Skip to content

Commit

Permalink
backend: Refactor authentication and email models
Browse files Browse the repository at this point in the history
- Introduced `UserDatabaseAuthentication` model with secure password management.
- Created `UserEmail` model to manage email addresses independently.
- Updated login, signup, and set password mutations to use the new models.
- Modified invitation mailer to work with `UserEmail`.
- Adjusted tests to reflect the separation of user authentication and email logic.
- Removed `pgcrypto` extension as UUIDs are now handled without it.
  • Loading branch information
hoshinotsuyoshi committed Nov 1, 2024
1 parent 537e2eb commit 1118c43
Show file tree
Hide file tree
Showing 24 changed files with 149 additions and 100 deletions.
12 changes: 8 additions & 4 deletions backend/app/graphql/mutations/login.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,16 @@ class Login < BaseMutation

def resolve(email_address:, password:)
errors = []
user = User.authenticate_by(email_address:, password:)
user = UserEmail.verified.find_by(email_address:)&.user
if user
start_new_session_for(user)
auth = UserDatabaseAuthentication.authenticate_by(user:, password:)
if auth
start_new_session_for(auth.user)
else
errors << :something_wrong
end
else
error = :something_wrong
errors << error
errors << :something_wrong
end

{ success: !!user, errors: }
Expand Down
4 changes: 2 additions & 2 deletions backend/app/graphql/mutations/set_password.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ class SetPassword < BaseMutation
def resolve(password:)
user = User.transaction do
current_user.lock!
if current_user.before_set_own_password_status?
current_user.update!(onboarding_status: :onboarded, password:)
if current_user.database_authentication.password.nil?
current_user.database_authentication.update!(password:)
current_user
else
nil
Expand Down
13 changes: 4 additions & 9 deletions backend/app/graphql/mutations/signup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,13 @@ def resolve(email_address:)
user = nil
errors = []
ApplicationRecord.transaction do
user = User.find_by(
email_address:, onboarding_status: :before_verify_email_address
) ||
User.create(
email_address:, password: SecureRandom.uuid, onboarding_status: :before_verify_email_address
)
user = User.create_with!(email_address:) unless UserEmail.find_by(email_address:)
end
if user.valid?
InvitationMailer.invite(user.id).deliver_later
if user&.valid?
InvitationMailer.invite(user.email.id).deliver_later
success = true
else
errors += user.errors.errors
errors << :taken
success = false
end
{ success:, errors: }
Expand Down
14 changes: 7 additions & 7 deletions backend/app/graphql/mutations/verify_email_address.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ class VerifyEmailAddress < BaseMutation
field :success, GraphQL::Types::Boolean, null: false

def resolve(signed_id:)
user = nil
User.transaction do
user = User
user_email = nil
UserEmail.transaction do
user_email = UserEmail
.lock
.find_signed(signed_id, purpose: :invite)
next unless user
user.update!(onboarding_status: :before_set_own_password) if user.before_verify_email_address_status?
start_new_session_for(user)
next unless user_email
user_email.touch(:confirmed_at) # rubocop:disable Rails/SkipsModelValidations
start_new_session_for(user_email.user)
end
{ success: !!user }
{ success: !!user_email }
end
end
end
8 changes: 4 additions & 4 deletions backend/app/mailers/invitation_mailer.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class InvitationMailer < ApplicationMailer
def invite(user_id)
@user = User.find(user_id)
@signed_id = @user.signed_id(purpose: :invite, expires_in: 1.hour)
mail to: @user.email_address
def invite(user_email_id)
@user_email = UserEmail.find(user_email_id)
@signed_id = @user_email.signed_id(purpose: :invite, expires_in: 1.hour)
mail to: @user_email.email
end
end
19 changes: 10 additions & 9 deletions backend/app/models/user.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
class User < ApplicationRecord
has_secure_password
has_many :sessions, dependent: :destroy
has_one :database_authentication, dependent: :destroy, class_name: :UserDatabaseAuthentication, inverse_of: :user
has_one :email, dependent: :destroy, class_name: :UserEmail, inverse_of: :user

normalizes :email_address, with: -> e { e.strip.downcase }
delegate :email_address, to: :email

validates :email_address, uniqueness: true

enum :onboarding_status, %w[
before_verify_email_address
before_set_own_password
onboarded
].index_by(&:itself), suffix: 'status'
class << self
def create_with!(email_address:)
create!
.tap { _1.create_email!(email_address:) }
.tap { _1.create_database_authentication!(password: SecureRandom.uuid) }
end
end
end
5 changes: 5 additions & 0 deletions backend/app/models/user_database_authentication.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class UserDatabaseAuthentication < ApplicationRecord
has_secure_password

belongs_to :user, inverse_of: :database_authentication
end
8 changes: 8 additions & 0 deletions backend/app/models/user_email.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class UserEmail < ApplicationRecord
belongs_to :user, inverse_of: :email
alias_attribute :email, :email_address

normalizes :email_address, with: -> e { e.strip.downcase }
validates :email_address, uniqueness: true
scope :verified, -> { where.not(confirmed_at: nil) }
end
2 changes: 2 additions & 0 deletions backend/config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class Application < Rails::Application
files = files.reject { _1.end_with?(".erb") }
system("bin/rubocop --autocorrect-all " + files.join(" "), exception: true)
end

g.orm :active_record, primary_key_type: :uuid
end
# Initialize configuration defaults for originally generated Rails version.
config.load_defaults 8.0
Expand Down
7 changes: 7 additions & 0 deletions backend/db/migrate/00000000000001_create_users.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class CreateUsers < ActiveRecord::Migration[8.0]
def change
create_table :users, id: :uuid do |t|
t.timestamps
end
end
end

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class CreateUserDatabaseAuthentications < ActiveRecord::Migration[8.0]
def change
create_table :user_database_authentications, id: :uuid do |t|
t.references :user, null: false, foreign_key: true, type: :uuid, index: { unique: true }
t.string :password_digest, null: false

t.timestamps
end
end
end
12 changes: 0 additions & 12 deletions backend/db/migrate/00000000000002_create_users.rb

This file was deleted.

13 changes: 13 additions & 0 deletions backend/db/migrate/00000000000003_create_user_emails.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class CreateUserEmails < ActiveRecord::Migration[8.0]
def change
create_table :user_emails, id: :uuid do |t|
t.references :user, null: false, foreign_key: true, type: :uuid, index: { unique: true }
t.string :email_address, null: false
t.datetime :confirmed_at

t.timestamps
end

add_index :user_emails, :email_address, unique: true
end
end
27 changes: 21 additions & 6 deletions backend/db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions backend/spec/factories/user_database_authentications.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
FactoryBot.define do
factory :user_database_authentication do
association(:user)
password { SecureRandom.alphanumeric }
end
end
7 changes: 7 additions & 0 deletions backend/spec/factories/user_emails.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
FactoryBot.define do
factory :user_email do
association(:user)
email_address { "#{SecureRandom.alphanumeric}@example.com" }
confirmed_at { Time.current }
end
end
9 changes: 6 additions & 3 deletions backend/spec/factories/users.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
FactoryBot.define do
factory :user do
email_address { "#{SecureRandom.alphanumeric}@example.com" }
password { SecureRandom.alphanumeric }
onboarding_status { :onboarded }
trait :onboarded do
after(:create) do |instance|
create(:user_database_authentication, user: instance)
create(:user_email, user: instance)
end
end
end
end
6 changes: 5 additions & 1 deletion backend/spec/graphql/mutations/login_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@
end

context "when password is correct" do
before { User.create!(email_address: "[email protected]", password: "password", onboarding_status: :before_verify_email_address) }
before do
user = create(:user)
create(:user_email, user:, email: "[email protected]")
create(:user_database_authentication, user:, password: "password")
end
it "no errors" do
expect { subject }.to change(Session, :count).by(1)
expect(subject_response_to_hash).to match(
Expand Down
45 changes: 15 additions & 30 deletions backend/spec/graphql/mutations/signup_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,39 +65,24 @@
let!(:email_address) { '[email protected]' }
let!(:context) { {} }

context 'when onboarding_status: :before_verify_email_address' do
before { create(:user, email_address:, onboarding_status: :before_verify_email_address) }
it 'sends an invitation email' do
expect { subject }
.to have_enqueued_job(ActionMailer::MailDeliveryJob)
.with('InvitationMailer', 'invite', 'deliver_now', { args: [be_a(String)] })
expect(subject_response_to_hash).to match(
data: {
signup: {
success: true,
errors: [],
},
},
)
end
before do
user = create(:user)
create(:user_email, user:, email: "[email protected]")
end

context 'when onboarding_status: :before_set_own_password' do
before { create(:user, email_address:, onboarding_status: :before_set_own_password) }
it 'does not send invitation email' do
expect { subject }
.not_to have_enqueued_job(ActionMailer::MailDeliveryJob)
expect(subject_response_to_hash).to match(
data: {
signup: {
success: false,
errors: [
{ __typename: "Taken" },
],
},
it 'does not send invitation email' do
expect { subject }
.not_to have_enqueued_job(ActionMailer::MailDeliveryJob)
expect(subject_response_to_hash).to match(
data: {
signup: {
success: false,
errors: [
{ __typename: "Taken" },
],
},
)
end
},
)
end
end
end
Expand Down
5 changes: 3 additions & 2 deletions backend/spec/graphql/resolvers/me_resolver_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
}
GRAPHQL
end
let!(:current_user) { User.new(email_address: "[email protected]") }
let!(:user) { create(:user) }
let!(:user_email) { create(:user_email, user:, email: '[email protected]') }
let!(:variables) { {} }
let!(:context) { { current_user: } }
let!(:context) { { current_user: user } }

it "returns me" do
expect(subject_response_to_hash).to eq(
Expand Down
4 changes: 2 additions & 2 deletions backend/spec/models/user_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'rails_helper'

RSpec.describe User, type: :model do
it { is_expected.to have_secure_password }
it { is_expected.to have_many(:sessions).dependent(:destroy) }
it { is_expected.to normalize(:email_address).from(" [email protected] ").to("[email protected]") }
it { is_expected.to have_one(:database_authentication).dependent(:destroy) }
it { is_expected.to have_one(:email).dependent(:destroy) }
end
8 changes: 4 additions & 4 deletions backend/spec/system/scenarios/login_flow_spec.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
require 'rails_helper'

RSpec.describe "login flow", type: :system do
let!(:user) { create(:user) }
let!(:user) { create(:user, :onboarded) }

it 'me -> login -> me -> logout -> me -> login' do
visit '/me'
expect(page).to have_no_content("hello, It's me!")

fill_in "email", with: user.email_address
fill_in "password", with: user.password
fill_in "email", with: user.email.email
fill_in "password", with: user.database_authentication.password
click_button "Login"

retry_on_error { expect(page).to have_content(user.email_address) }
retry_on_error { expect(page).to have_content(user.email.email) }
expect(page).to have_content("hello, It's me!")
accept_alert do
click_button "Logout"
Expand Down

0 comments on commit 1118c43

Please sign in to comment.