From 5cef4ffeb0c97a8eff1718f968ab8157221039e6 Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Tue, 7 Oct 2025 19:33:51 +0200 Subject: [PATCH] Add sign in flow using magic links --- app/controllers/concerns/authentication.rb | 53 ++++++-- .../sessions/login_menus_controller.rb | 25 ++++ .../sessions/magic_links_controller.rb | 34 ++++++ app/controllers/sessions_controller.rb | 16 ++- app/helpers/login_helper.rb | 2 +- app/helpers/magic_link_helper.rb | 5 + app/jobs/magic_link/cleanup_job.rb | 5 + app/mailers/magic_link_mailer.rb | 15 +++ app/models/account.rb | 2 + app/models/identity.rb | 6 +- app/models/magic_link.rb | 41 +++++++ app/models/magic_link/code.rb | 31 +++++ app/models/membership.rb | 11 +- app/models/user/identifiable.rb | 2 + app/models/user/role.rb | 1 + .../sign_in_instructions.html.erb | 13 ++ .../sign_in_instructions.text.erb | 8 ++ app/views/sessions/login_menus/show.html.erb | 27 ++++ .../sessions/magic_links/create.html.erb | 22 ++++ app/views/sessions/magic_links/show.html.erb | 29 +++++ app/views/sessions/new.html.erb | 13 +- config/recurring.yml | 3 + config/routes.rb | 2 + ...1011080030_add_setup_status_to_accounts.rb | 5 + db/seeds.rb | 13 +- ...51007112917_create_identity_magic_links.rb | 11 ++ .../signups/completions_controller.rb | 46 +++++++ .../app/controllers/signups_controller.rb | 6 +- gems/fizzy-saas/app/models/signup.rb | 68 ++++++++--- .../views/signups/completions/new.html.erb | 42 +++++++ .../fizzy-saas/app/views/signups/new.html.erb | 42 ++----- gems/fizzy-saas/config/routes.rb | 8 +- .../signups/completions_controller_test.rb | 40 ++++++ .../controllers/signups_controller_test.rb | 45 +------ gems/fizzy-saas/test/models/signup_test.rb | 115 +++++++----------- .../20251013-mark-all-accounts-as-setup.rb | 8 ++ .../controller_authentication_test.rb | 7 +- .../sessions/login_menus_controller_test.rb | 21 ++++ .../sessions/magic_links_controller_test.rb | 36 ++++++ test/controllers/sessions_controller_test.rb | 38 +++--- test/fixtures/accesses.yml | 2 + test/fixtures/identities.yml | 2 + test/fixtures/memberships.yml | 12 ++ test/integration/identity_membership_test.rb | 37 ++---- test/mailers/magic_link_mailer_test.rb | 16 +++ test/mailers/previews/magic_link_preview.rb | 8 ++ test/models/magic_link/code_test.rb | 17 +++ test/models/magic_link_test.rb | 58 +++++++++ test/models/membership_test.rb | 30 ++++- test/models/user/identifiable_test.rb | 24 ++-- test/test_helpers/session_test_helper.rb | 53 +++++++- 51 files changed, 908 insertions(+), 268 deletions(-) create mode 100644 app/controllers/sessions/login_menus_controller.rb create mode 100644 app/controllers/sessions/magic_links_controller.rb create mode 100644 app/helpers/magic_link_helper.rb create mode 100644 app/jobs/magic_link/cleanup_job.rb create mode 100644 app/mailers/magic_link_mailer.rb create mode 100644 app/models/magic_link.rb create mode 100644 app/models/magic_link/code.rb create mode 100644 app/views/mailers/magic_link_mailer/sign_in_instructions.html.erb create mode 100644 app/views/mailers/magic_link_mailer/sign_in_instructions.text.erb create mode 100644 app/views/sessions/login_menus/show.html.erb create mode 100644 app/views/sessions/magic_links/create.html.erb create mode 100644 app/views/sessions/magic_links/show.html.erb create mode 100644 db/migrate/20251011080030_add_setup_status_to_accounts.rb create mode 100644 db/untenanted_migrate/20251007112917_create_identity_magic_links.rb create mode 100644 gems/fizzy-saas/app/controllers/signups/completions_controller.rb create mode 100644 gems/fizzy-saas/app/views/signups/completions/new.html.erb create mode 100644 gems/fizzy-saas/test/controllers/signups/completions_controller_test.rb create mode 100755 script/migrations/20251013-mark-all-accounts-as-setup.rb create mode 100644 test/controllers/sessions/login_menus_controller_test.rb create mode 100644 test/controllers/sessions/magic_links_controller_test.rb create mode 100644 test/mailers/magic_link_mailer_test.rb create mode 100644 test/mailers/previews/magic_link_preview.rb create mode 100644 test/models/magic_link/code_test.rb create mode 100644 test/models/magic_link_test.rb diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index f98808b6e..f9a462b7f 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -38,13 +38,31 @@ module Authentication def require_tenant unless ApplicationRecord.current_tenant.present? - set_current_identity_token - render "sessions/login_menu" + resume_identity + redirect_to session_login_menu_url(script_name: nil) end end + def require_identity + resume_identity || request_authentication + end + def require_authentication - resume_session || request_authentication + if resume_identity + resume_session || request_session_for_identity + else + request_authentication + end + end + + def request_session_for_identity + redirect_to session_login_menu_url(script_name: nil) + end + + def resume_identity + if identity = find_identity_by_cookie + set_current_identity(identity) + end end def resume_session @@ -53,11 +71,15 @@ module Authentication end end + def find_identity_by_cookie + Identity.find_signed(cookies.signed[:identity_token]&.dig("id")) + end + def find_session_by_cookie Session.find_signed(cookies.signed[:session_token]) end - def request_authentication(untenanted: false) + def request_authentication if ApplicationRecord.current_tenant.present? session[:return_to_after_authenticating] = request.url end @@ -84,26 +106,37 @@ module Authentication end end - def link_identity(user) + def link_identity(user_or_membership) token_value = cookies.signed[:identity_token] identity = Identity.find_signed(token_value["id"]) if token_value.present? - identity = user.set_identity(identity) - cookies.signed.permanent[:identity_token] = { value: { "id" => identity.signed_id, "updated_at" => identity.updated_at }, httponly: true, same_site: :lax } + + if user_or_membership.is_a?(User) + identity = user_or_membership.set_identity(identity) + elsif user_or_membership.is_a?(Membership) && identity + user_or_membership.update!(identity: identity) + end + + set_current_identity(identity) end - def set_current_identity_token - Current.identity_token = Identity::Mock.new(**cookies.signed[:identity_token]) + def set_current_identity(identity) + Current.identity_token = if identity + cookies.signed.permanent[:identity_token] = { value: { "id" => identity.signed_id, "updated_at" => identity.updated_at }, httponly: true, same_site: :lax } + Identity::Mock.new(**cookies.signed[:identity_token]) + else + nil + end end def set_current_session(session) logger.struct " Authorized User##{session.user.id}", authentication: { user: { id: session.user.id } } Current.session = session - set_current_identity_token cookies.signed.permanent[:session_token] = { value: session.signed_id, httponly: true, same_site: :lax, path: Account.sole.slug } end def terminate_session Current.session.destroy cookies.delete(:session_token) + cookies.delete(:identity_token) end end diff --git a/app/controllers/sessions/login_menus_controller.rb b/app/controllers/sessions/login_menus_controller.rb new file mode 100644 index 000000000..f65ab3c5c --- /dev/null +++ b/app/controllers/sessions/login_menus_controller.rb @@ -0,0 +1,25 @@ +class Sessions::LoginMenusController < ApplicationController + require_untenanted_access only: :show + allow_unauthenticated_access only: :create + before_action :require_identity + before_action :set_identity + + def show + @memberships = @identity.memberships + end + + def create + membership = @identity.memberships.find(membership_id) + start_new_session_for(membership.user) + redirect_to after_authentication_url + end + + private + def set_identity + @identity = Current.identity_token.identity + end + + def membership_id + params.expect :membership_id + end +end diff --git a/app/controllers/sessions/magic_links_controller.rb b/app/controllers/sessions/magic_links_controller.rb new file mode 100644 index 000000000..2039f527c --- /dev/null +++ b/app/controllers/sessions/magic_links_controller.rb @@ -0,0 +1,34 @@ +class Sessions::MagicLinksController < ApplicationController + require_untenanted_access + rate_limit to: 10, within: 15.minutes, only: :create, with: -> { redirect_to session_magic_link_path, alert: "Try again in 15 minutes." } + + def show + end + + def create + membership = MagicLink.consume(code) + + if membership.blank? + redirect_to session_magic_link_path, alert: "Try another code." + else + resume_identity + + if Current.identity_token + link_identity(membership) + else + set_current_identity(membership.identity) + end + + if membership.account.setup_pending? + redirect_to saas.new_signup_completion_url(script_name: "/#{membership.user_tenant}") + else + redirect_to session_login_menu_path + end + end + end + + private + def code + params.expect(:code) + end +end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 16d368c59..5db073e97 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,21 +1,25 @@ class SessionsController < ApplicationController - require_unauthenticated_access only: %i[ new create ] + require_untenanted_access only: %i[ new create ] rate_limit to: 10, within: 3.minutes, only: :create, with: -> { redirect_to new_session_path, alert: "Try again later." } def new end def create - if user = User.active.authenticate_by(params.permit(:email_address, :password)) - start_new_session_for user - redirect_to after_authentication_url - else - redirect_to new_session_path, alert: "Try another email address or password." + if membership = Membership.find_by(email_address: email_address) + membership.send_magic_link end + + redirect_to session_magic_link_path end def destroy terminate_session redirect_to_logout_url end + + private + def email_address + params.expect(:email_address) + end end diff --git a/app/helpers/login_helper.rb b/app/helpers/login_helper.rb index 0eed1781b..56627d968 100644 --- a/app/helpers/login_helper.rb +++ b/app/helpers/login_helper.rb @@ -1,6 +1,6 @@ module LoginHelper def login_url - new_session_path + new_session_path(script_name: nil) end def logout_url diff --git a/app/helpers/magic_link_helper.rb b/app/helpers/magic_link_helper.rb new file mode 100644 index 000000000..d34d71115 --- /dev/null +++ b/app/helpers/magic_link_helper.rb @@ -0,0 +1,5 @@ +module MagicLinkHelper + def magic_link_code(magic_link) + magic_link.code.scan(/.{3}/).join("-") + end +end diff --git a/app/jobs/magic_link/cleanup_job.rb b/app/jobs/magic_link/cleanup_job.rb new file mode 100644 index 000000000..eea054fd0 --- /dev/null +++ b/app/jobs/magic_link/cleanup_job.rb @@ -0,0 +1,5 @@ +class MagicLink::CleanupJob < ApplicationJob + def perform + MagicLink.cleanup + end +end diff --git a/app/mailers/magic_link_mailer.rb b/app/mailers/magic_link_mailer.rb new file mode 100644 index 000000000..11f0f5705 --- /dev/null +++ b/app/mailers/magic_link_mailer.rb @@ -0,0 +1,15 @@ +class MagicLinkMailer < ApplicationMailer + helper MagicLinkHelper + + def sign_in_instructions(magic_link) + @magic_link = magic_link + @membership = @magic_link.membership + + mail to: @membership.email_address, subject: "Sign in to Fizzy" + end + + private + def default_url_options + Rails.application.config.action_mailer.default_url_options + end +end diff --git a/app/models/account.rb b/app/models/account.rb index da2298c88..3e9b0fe19 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -3,6 +3,8 @@ class Account < ApplicationRecord has_many_attached :uploads + enum :setup_status, %w[ pending complete ].index_by(&:itself), prefix: :setup, default: :pending + class << self def create_with_admin_user(account:, owner:) User.system diff --git a/app/models/identity.rb b/app/models/identity.rb index bbf798dd1..74aa3830b 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -1,7 +1,11 @@ class Identity < UntenantedRecord # This is used to instantiate an Identity-like object from the `identity_token` without hitting # the untenanted database. It is intended to be used with caching/etagging methods. - Mock = Struct.new(:id, :updated_at) + Mock = Struct.new(:id, :updated_at) do + def identity + Identity.find_signed(id) + end + end has_many :memberships, dependent: :destroy end diff --git a/app/models/magic_link.rb b/app/models/magic_link.rb new file mode 100644 index 000000000..bb06313f7 --- /dev/null +++ b/app/models/magic_link.rb @@ -0,0 +1,41 @@ +class MagicLink < UntenantedRecord + CODE_LENGTH = 6 + EXPIRATION_TIME = 15.minutes + + belongs_to :membership + + scope :active, -> { where(expires_at: Time.current...) } + scope :stale, -> { where(expires_at: ..Time.current) } + + before_validation :generate_code, on: :create + before_validation :set_expiration, on: :create + + validates :code, uniqueness: true, presence: true + + class << self + def consume(code) + active.find_by(code: Code.sanitize(code))&.consume + end + + def cleanup + stale.delete_all + end + end + + def consume + destroy + membership + end + + private + def generate_code + self.code = loop do + candidate = Code.generate(CODE_LENGTH) + break candidate unless self.class.exists?(code: candidate) + end + end + + def set_expiration + self.expires_at = EXPIRATION_TIME.from_now + end +end diff --git a/app/models/magic_link/code.rb b/app/models/magic_link/code.rb new file mode 100644 index 000000000..6af1619d8 --- /dev/null +++ b/app/models/magic_link/code.rb @@ -0,0 +1,31 @@ +module MagicLink::Code + CODE_ALPHABET = "0123456789ABCDEFGHJKMNPQRSTVWXYZ".chars.freeze + CODE_SUBSTITUTIONS = { "O" => "0", "I" => "1", "L" => "1" }.freeze + + class << self + def generate(length) + length.times.map { CODE_ALPHABET.sample }.join + end + + def sanitize(code) + return nil if code.blank? + + normalize_code(code) + .then { apply_substitutions(_1) } + .then { remove_invalid_characters(_1) } + end + + private + def normalize_code(code) + code.to_s.upcase + end + + def apply_substitutions(code) + CODE_SUBSTITUTIONS.reduce(code) { |result, (from, to)| result.gsub(from, to) } + end + + def remove_invalid_characters(code) + code.gsub(/[^#{CODE_ALPHABET.join}]/, "") + end + end +end diff --git a/app/models/membership.rb b/app/models/membership.rb index 2289443f1..9233de99c 100644 --- a/app/models/membership.rb +++ b/app/models/membership.rb @@ -1,6 +1,6 @@ class Membership < UntenantedRecord belongs_to :identity, touch: true - has_many :magic_links, dependent: :delete + has_many :magic_links, dependent: :delete_all # I want this to be `belongs_to :user`, but ActiveRecord::Tenanted doesn't yet support # associations from untenanted to untenanted models. @@ -11,4 +11,13 @@ class Membership < UntenantedRecord def user User.with_tenant(user_tenant) { User.find_by(id: user_id) } end + + def account + Account.with_tenant(user_tenant) { Account.sole } + end + + def send_magic_link + magic_link = magic_links.create! + MagicLinkMailer.sign_in_instructions(magic_link).deliver_later + end end diff --git a/app/models/user/identifiable.rb b/app/models/user/identifiable.rb index 0b3f0abbc..c90247b74 100644 --- a/app/models/user/identifiable.rb +++ b/app/models/user/identifiable.rb @@ -4,6 +4,8 @@ module User::Identifiable included do has_one :membership, ->(user) { where(user_tenant: user.tenant) } has_one :identity, through: :membership + + scope :with_identity, ->(identity) { where(id: identity.memberships.where(user_tenant: ApplicationRecord.current_tenant).pluck(:user_id)) } end def set_identity(token_identity) diff --git a/app/models/user/role.rb b/app/models/user/role.rb index aaa4bd5ff..5c89fdd25 100644 --- a/app/models/user/role.rb +++ b/app/models/user/role.rb @@ -4,6 +4,7 @@ module User::Role included do enum :role, %i[ admin member system ].index_by(&:itself), scopes: false + scope :admin, -> { where(role: :admin) } scope :member, -> { where(role: :member) } scope :active, -> { where(active: true, role: %i[ admin member ]) } end diff --git a/app/views/mailers/magic_link_mailer/sign_in_instructions.html.erb b/app/views/mailers/magic_link_mailer/sign_in_instructions.html.erb new file mode 100644 index 000000000..69cb84cf8 --- /dev/null +++ b/app/views/mailers/magic_link_mailer/sign_in_instructions.html.erb @@ -0,0 +1,13 @@ +

Sign in to Fizzy

+ +
+

Hit the button below to sign in to Fizzy on this device

+ +
<%= link_to "Sign in to Fizzy", session_magic_link_url(code: @magic_link.code), class: "btn" %>
+
+ +
+

If you're signing in on another device, enter the code below:

+ + <%= magic_link_code(@magic_link) %> +
diff --git a/app/views/mailers/magic_link_mailer/sign_in_instructions.text.erb b/app/views/mailers/magic_link_mailer/sign_in_instructions.text.erb new file mode 100644 index 000000000..129bce961 --- /dev/null +++ b/app/views/mailers/magic_link_mailer/sign_in_instructions.text.erb @@ -0,0 +1,8 @@ +Sign in to Fizzy +<%= "=" * 80 %> + +Open this link in your browser to login on this device: +<%= session_magic_link_url(code: @magic_link.code) %> + +If you're signing in on another device, enter the code below: +<%= magic_link_code(@magic_link) %> diff --git a/app/views/sessions/login_menus/show.html.erb b/app/views/sessions/login_menus/show.html.erb new file mode 100644 index 000000000..b66b2490d --- /dev/null +++ b/app/views/sessions/login_menus/show.html.erb @@ -0,0 +1,27 @@ +<% cache [ @identity, @memberships ] do %> +
+
+ <% if @memberships.present? %> +

Your Fizzy Accounts

+
+ <% @memberships.each do |membership| %> + <%= button_to session_login_menu_url(membership_id: membership, script_name: "/#{membership.user_tenant}"), method: :post, class: "btn center txt-medium" do %> + <%= membership.account_name %> + : + <%= membership.email_address %> + <% end %> + <% end %> +
+ <% else %> +

You don't have any existing Fizzy accounts.

+ <% end %> +
+ +
+ <%= link_to login_url, class: "btn center btn--reversed" do %> + <%= icon_tag "add" %> + Add another account + <% end %> +
+
+<% end %> diff --git a/app/views/sessions/magic_links/create.html.erb b/app/views/sessions/magic_links/create.html.erb new file mode 100644 index 000000000..c73b393eb --- /dev/null +++ b/app/views/sessions/magic_links/create.html.erb @@ -0,0 +1,22 @@ +<% @page_title = "Log in" %> + +
+

Log in

+ +

Pick where you want to log in.

+ +
+ <% @memberships.each do |membership| %> + <%= link_to membership.email_address, session_transfer_path(membership.user.transfer_id) %> + <% end %> +
+
+ +<% content_for :footer do %> +
+ Fizzy™ version 0 +
+<% end %> diff --git a/app/views/sessions/magic_links/show.html.erb b/app/views/sessions/magic_links/show.html.erb new file mode 100644 index 000000000..a47db86fb --- /dev/null +++ b/app/views/sessions/magic_links/show.html.erb @@ -0,0 +1,29 @@ +<% @page_title = "Magic link" %> + +
" + style="--panel-size: 55ch;" +> +

Check your email

+ +

We just emailed you a special code.

+ + <%= form_with url: session_magic_link_path, method: :post, html: { class: "flex flex-column gap", data: { controller: token_list("auto-submit" => params[:code].present?)} } do |form| %> +
+ +
+ + + <% end %> +
+ +<% content_for :footer do %> +
+ Fizzy™ version 0 +
+<% end %> diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index 85cbddb81..9c0cf2464 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -11,16 +11,15 @@ -
- -
- <% end %> + +<% content_for :footer do %> +
+ Fizzy™ version 0 +
+<% end %> diff --git a/config/recurring.yml b/config/recurring.yml index 4f56c420f..8ed13f58d 100644 --- a/config/recurring.yml +++ b/config/recurring.yml @@ -25,6 +25,9 @@ production: &production cleanup_webhook_deliveries: class: Webhook::CleanupDeliveriesJob schedule: every 4 hours at minute 51 + cleanup_magic_links: + class: "MagicLink::CleanupJob" + schedule: every 4 hours sqlite_backups: class: SQLiteBackupsJob schedule: every day at 05:02 diff --git a/config/routes.rb b/config/routes.rb index 7593c0bcd..be32d7119 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -129,6 +129,8 @@ Rails.application.routes.draw do resource :session do scope module: "sessions" do resources :transfers, only: %i[ show update ] + resource :magic_link, only: %i[ show create ] + resource :login_menu, only: %i[ show create ] end end diff --git a/db/migrate/20251011080030_add_setup_status_to_accounts.rb b/db/migrate/20251011080030_add_setup_status_to_accounts.rb new file mode 100644 index 000000000..db5f5c537 --- /dev/null +++ b/db/migrate/20251011080030_add_setup_status_to_accounts.rb @@ -0,0 +1,5 @@ +class AddSetupStatusToAccounts < ActiveRecord::Migration[8.1] + def change + add_column :accounts, :setup_status, :string + end +end diff --git a/db/seeds.rb b/db/seeds.rb index b000de36f..76bcbd253 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -18,7 +18,8 @@ def create_tenant(signal_account_name) account = Account.create_with_admin_user( account: { external_account_id: tenant_id, - name: signal_account_name + name: signal_account_name, + setup_status: :complete }, owner: { name: "David Heinemeier Hansson", @@ -30,16 +31,24 @@ def create_tenant(signal_account_name) end ApplicationRecord.current_tenant = tenant_id + + identity = Membership.find_by(email_address: User.first.email_address)&.identity + User.first.set_identity(identity) end def find_or_create_user(full_name, email_address) if user = User.find_by(email_address: email_address) user else - User.create! \ + user = User.create! \ name: full_name, email_address: email_address, password: "secret123456" + + identity = Membership.find_by(email_address: email_address)&.identity || Identity.create! + user.set_identity(identity) + + user end end diff --git a/db/untenanted_migrate/20251007112917_create_identity_magic_links.rb b/db/untenanted_migrate/20251007112917_create_identity_magic_links.rb new file mode 100644 index 000000000..cdd99228d --- /dev/null +++ b/db/untenanted_migrate/20251007112917_create_identity_magic_links.rb @@ -0,0 +1,11 @@ +class CreateIdentityMagicLinks < ActiveRecord::Migration[8.1] + def change + create_table :magic_links do |t| + t.string :code, index: { unique: true }, null: false + t.belongs_to :membership, null: false, foreign_key: true + t.datetime :expires_at, index: true, null: false + + t.timestamps + end + end +end diff --git a/gems/fizzy-saas/app/controllers/signups/completions_controller.rb b/gems/fizzy-saas/app/controllers/signups/completions_controller.rb new file mode 100644 index 000000000..bf322354a --- /dev/null +++ b/gems/fizzy-saas/app/controllers/signups/completions_controller.rb @@ -0,0 +1,46 @@ +class Signups::CompletionsController < ApplicationController + allow_unauthenticated_access + + before_action :require_identity + before_action :ensure_setup_pending + before_action :ensure_identity_of_an_admin + + def new + @signup = Signup.new + end + + def create + @signup = Signup.new(signup_params) + + if @signup.complete + start_new_session_for(@signup.user) + redirect_to root_path + else + render :new, status: :unprocessable_entity + end + end + + private + def ensure_setup_pending + unless Account.sole.setup_pending? + redirect_to root_path + end + end + + def ensure_identity_of_an_admin + unless user&.admin? + redirect_to root_path + end + end + + def signup_params + params.expect(signup: %i[ full_name company_name ]).with_defaults( + tenant: ApplicationRecord.current_tenant, + user: user + ) + end + + def user + @user ||= User.all.admin.with_identity(Current.identity_token.identity).first + end +end diff --git a/gems/fizzy-saas/app/controllers/signups_controller.rb b/gems/fizzy-saas/app/controllers/signups_controller.rb index 73bcdeada..8421270e2 100644 --- a/gems/fizzy-saas/app/controllers/signups_controller.rb +++ b/gems/fizzy-saas/app/controllers/signups_controller.rb @@ -17,8 +17,8 @@ class SignupsController < ApplicationController def create @signup = Signup.new(signup_params) - if @signup.process - redirect_to root_url(script_name: @signup.account.slug) + if @signup.create_account + redirect_to session_magic_link_path else render :new, status: :unprocessable_entity end @@ -26,6 +26,6 @@ class SignupsController < ApplicationController private def signup_params - params.require(:signup).permit(*Signup::PERMITTED_KEYS) + params.expect(signup: %i[ email_address ]) end end diff --git a/gems/fizzy-saas/app/models/signup.rb b/gems/fizzy-saas/app/models/signup.rb index f9ead90be..e9d9dfbeb 100644 --- a/gems/fizzy-saas/app/models/signup.rb +++ b/gems/fizzy-saas/app/models/signup.rb @@ -3,14 +3,19 @@ class Signup include ActiveModel::Attributes include ActiveModel::Validations - PERMITTED_KEYS = %i[ full_name email_address password company_name ] + PERMITTED_KEYS = %i[ full_name email_address company_name ] - # Input attributes - attr_accessor :company_name, :full_name, :email_address, :password - validates_presence_of :company_name, :full_name, :email_address, :password + attr_accessor :company_name, :full_name, :email_address, :user, :tenant + attr_reader :queenbee_account, :account + + with_options on: :account_creation do + validates_presence_of :email_address + end + + with_options on: :completion do + validates_presence_of :company_name, :full_name + end - # Output attributes - attr_reader :tenant, :account, :user, :queenbee_account def initialize(...) @company_name = nil @@ -25,22 +30,43 @@ class Signup super end - def process - return false unless valid? + def create_account + return false unless valid?(:account_creation) - create_queenbee_account - create_tenant + if membership = Membership.find_by(email_address: email_address) + membership.send_magic_link + else + create_queenbee_account + create_tenant + user.membership.send_magic_link + end - true rescue => error destroy_tenant destroy_queenbee_account errors.add(:base, "An error occurred during signup: #{error.message}") + Rails.logger.error(error) + Rails.logger.error(error.backtrace.join("\n")) false end + def complete + return false unless valid?(:completion) + + ApplicationRecord.with_tenant(tenant) do + @account = Account.sole + + ApplicationRecord.transaction do + user.update!(name: full_name) + account.update!(name: company_name, setup_status: :complete) + user.membership.update!(account_name: company_name) + end + end + # TODO: Update company and user name in QB + end + private def create_queenbee_account @queenbee_account = Queenbee::Remote::Account.create!(queenbee_account_attributes) @@ -52,17 +78,18 @@ class Signup end def create_tenant - @tenant = queenbee_account.id.to_s + self.tenant = queenbee_account.id.to_s + ApplicationRecord.create_tenant(tenant) do @account = Account.create_with_admin_user( account: { external_account_id: tenant, - name: company_name + name: "New Account", + setup_status: :pending }, owner: { - name: full_name, - email_address: email_address, - password: password + name: email_address, + email_address: email_address } ) @user = User.find_by!(role: :admin) @@ -74,9 +101,10 @@ class Signup if tenant.present? && ApplicationRecord.tenant_exist?(tenant) ApplicationRecord.destroy_tenant(tenant) end - @user = nil + @account = nil - @tenant = nil + self.user = nil + self.tenant = nil end def queenbee_account_attributes @@ -92,8 +120,8 @@ class Signup # attributes[:terms_of_service] = true attributes[:product_name] = "fizzy" - attributes[:name] = company_name - attributes[:owner_name] = full_name + attributes[:name] = email_address + attributes[:owner_name] = email_address attributes[:owner_email] = email_address attributes[:trial] = true diff --git a/gems/fizzy-saas/app/views/signups/completions/new.html.erb b/gems/fizzy-saas/app/views/signups/completions/new.html.erb new file mode 100644 index 000000000..b78b84e76 --- /dev/null +++ b/gems/fizzy-saas/app/views/signups/completions/new.html.erb @@ -0,0 +1,42 @@ +<% @page_title = "Sign up for Fizzy" %> + +
" + style="--panel-size: 65ch;" +> +

Create your account

+ +

+ Great! We just need two more things... +

+ + <%= form_with model: @signup, url: saas.signup_completion_path, scope: "signup", class: "flex flex-column gap txt-large", data: { turbo: false } do |form| %> +
+ +
+ +
+ +
+ + <% if @signup.errors.any? %> +
+
    + <% @signup.errors.full_messages.each do |message| %> +
  • <%= message %>
    • + <% end %> +
+
+ <% end %> + + + <% end %> +
diff --git a/gems/fizzy-saas/app/views/signups/new.html.erb b/gems/fizzy-saas/app/views/signups/new.html.erb index c357d3c0f..11d4c926e 100644 --- a/gems/fizzy-saas/app/views/signups/new.html.erb +++ b/gems/fizzy-saas/app/views/signups/new.html.erb @@ -1,17 +1,16 @@ <% @page_title = "Sign up for Fizzy" %> -
" style="--panel-size: 65ch;"> +
" + style="--panel-size: 65ch;" +>

Fizzy

-

Create your account

+ +

+ Enter your email address to get started. +

<%= form_with model: @signup, url: saas.signup_path, scope: "signup", class: "flex flex-column gap txt-large", data: { turbo: false } do |form| %> -
- -
-
-
- -
- -
- -
- <% if @signup.errors.any? %>
    @@ -44,17 +29,8 @@ <% end %> <% end %> - -
diff --git a/gems/fizzy-saas/config/routes.rb b/gems/fizzy-saas/config/routes.rb index 698095350..172eb6e6f 100644 --- a/gems/fizzy-saas/config/routes.rb +++ b/gems/fizzy-saas/config/routes.rb @@ -1,4 +1,10 @@ Fizzy::Saas::Engine.routes.draw do - resource :signup, only: [ :new, :create ] + resource :signup, only: %i[ new create ] do + scope module: :signups do + collection do + resource :completion, only: %i[ new create ], as: :signup_completion + end + end + end Queenbee.routes(self) end diff --git a/gems/fizzy-saas/test/controllers/signups/completions_controller_test.rb b/gems/fizzy-saas/test/controllers/signups/completions_controller_test.rb new file mode 100644 index 000000000..5b24277dc --- /dev/null +++ b/gems/fizzy-saas/test/controllers/signups/completions_controller_test.rb @@ -0,0 +1,40 @@ +require "test_helper" + +class Signups::CompletionsControllerTest < ActionDispatch::IntegrationTest + setup do + Account.sole.update!(setup_status: :pending) + set_identity_as :kevin + end + + test "new" do + get saas.new_signup_completion_path + + assert_response :success + end + + test "create" do + post saas.signup_completion_path, params: { + signup: { + full_name: "Kevin Systrom", + company_name: "37signals" + } + } + + assert_redirected_to root_path, "Successful completion should redirect to root" + assert cookies[:session_token].present?, "Successful completion should create a session" + assert_equal "complete", Account.sole.reload.setup_status, "Account setup status should be complete" + assert_equal "Kevin Systrom", users(:kevin).reload.name, "User name should be updated" + assert_equal "37signals", Account.sole.reload.name, "Account name should be updated" + + Account.sole.update!(setup_status: :pending) + + post saas.signup_completion_path, params: { + signup: { + full_name: "", + company_name: "" + } + } + + assert_response :unprocessable_entity, "Invalid params should return unprocessable entity" + end +end diff --git a/gems/fizzy-saas/test/controllers/signups_controller_test.rb b/gems/fizzy-saas/test/controllers/signups_controller_test.rb index 8cbe8ec9f..d23ad07e4 100644 --- a/gems/fizzy-saas/test/controllers/signups_controller_test.rb +++ b/gems/fizzy-saas/test/controllers/signups_controller_test.rb @@ -5,8 +5,7 @@ class SignupsControllerTest < ActionDispatch::IntegrationTest @signup_params = { full_name: "Brian Wilson", email_address: "brian@example.com", - company_name: "Beach Boys", - password: SecureRandom.hex(16) + company_name: "Beach Boys" } @starting_tenants = ApplicationRecord.tenants @@ -24,39 +23,20 @@ class SignupsControllerTest < ActionDispatch::IntegrationTest get saas.new_signup_url, headers: http_basic_auth_headers assert_response :success - assert_select "h2", "Create your account" - assert_select "input[name='signup[full_name]']" + assert_select "h2", "Enter your email address to get started." assert_select "input[name='signup[email_address]']" - assert_select "input[name='signup[company_name]']" - assert_select "input[name='signup[password]']" end - test "should create signup and redirect to tenant root on success" do - Account.any_instance.expects(:setup_basic_template).once - + test "should create signup and redirect to magic link page" do assert_difference -> { ApplicationRecord.tenants.count }, 1 do - post saas.signup_url, params: { signup: @signup_params }, headers: http_basic_auth_headers + post saas.signup_url, params: { signup: { email_address: @signup_params[:email_address] } }, headers: http_basic_auth_headers end - assert_response :redirect - - new_tenant = (ApplicationRecord.tenants - @starting_tenants).first - ApplicationRecord.with_tenant(new_tenant) do - account = Account.sole - assert account, "Account should have been created" - assert_equal @signup_params[:company_name], account.name - - user = User.find_by(email_address: @signup_params[:email_address]) - assert user, "User should have been created" - assert_equal @signup_params[:full_name], user.name - assert_equal @signup_params[:email_address], user.email_address - - assert_redirected_to root_url(script_name: account.slug) - end + assert_redirected_to session_magic_link_path end test "should render new with errors when signup fails validation" do - invalid_params = @signup_params.merge(password: "") + invalid_params = { email_address: "" } assert_no_difference -> { ApplicationRecord.tenants.count } do post saas.signup_url, params: { signup: invalid_params }, headers: http_basic_auth_headers @@ -64,19 +44,6 @@ class SignupsControllerTest < ActionDispatch::IntegrationTest assert_response :unprocessable_entity assert_select ".alert--error" - assert_select ".alert--error li", /Password can't be blank/ - end - - test "should render new with errors when signup processing fails" do - Queenbee::Remote::Account.stubs(:create!).raises(RuntimeError, "Invalid account data") - - assert_no_difference -> { ApplicationRecord.tenants.count } do - post saas.signup_url, params: { signup: @signup_params }, headers: http_basic_auth_headers - end - - assert_response :unprocessable_entity - assert_select ".alert--error" - assert_select ".alert--error li", /An error occurred during signup/ end private diff --git a/gems/fizzy-saas/test/models/signup_test.rb b/gems/fizzy-saas/test/models/signup_test.rb index 9566fc0b4..48c6f499b 100644 --- a/gems/fizzy-saas/test/models/signup_test.rb +++ b/gems/fizzy-saas/test/models/signup_test.rb @@ -3,92 +3,67 @@ require "test_helper" class SignupTest < ActiveSupport::TestCase setup do @starting_tenants = ApplicationRecord.tenants - @signup = Signup.new( - email_address: "brian@example.com", - full_name: "Brian Wilson", - company_name: "Beach Boys", - password: SecureRandom.hex(16) - ) end - test "#process creates all the necessary objects for a new Fizzy account" do + test "#create_account" do Account.any_instance.expects(:setup_basic_template).once - assert @signup.process, @signup.errors.full_messages.to_sentence(words_connector: ". ") - assert_empty @signup.errors + signup = Signup.new(email_address: "brian@example.com") - assert @signup.tenant - assert_includes ApplicationRecord.tenants, @signup.tenant + assert_difference -> { Membership.count }, 1 do + assert_difference -> { MagicLink.count }, 1 do + assert signup.create_account, signup.errors.full_messages.to_sentence(words_connector: ". ") + end + end - assert @signup.account - assert @signup.account.persisted? - assert @signup.account.external_account_id - assert_equal @signup.company_name, @signup.account.name - assert_equal @signup.tenant, @signup.account.external_account_id.to_s - assert_equal @signup.tenant, @signup.account.tenant + assert_empty signup.errors + assert signup.tenant + assert_includes ApplicationRecord.tenants, signup.tenant + assert signup.account + assert signup.account.persisted? + assert signup.user + assert signup.user.persisted? - assert @signup.user - assert @signup.user.persisted? - assert_equal @signup.full_name, @signup.user.name - assert_equal @signup.email_address, @signup.user.email_address + signup_existing = Signup.new(email_address: "brian@example.com") - auth_params = { email_address: @signup.email_address, password: @signup.password } - user = ApplicationRecord.with_tenant(@signup.tenant) { User.authenticate_by(**auth_params) } + assert_no_difference -> { Membership.count } do + assert_difference -> { MagicLink.count }, 1 do + assert signup_existing.create_account, "Should send magic link for existing membership" + end + end - assert user, "User should be able to authenticate with #{auth_params.inspect}" - assert_equal @signup.user, user - assert_equal @signup.tenant, @signup.user.tenant - end + signup_invalid = Signup.new(email_address: "") + assert_not signup_invalid.create_account, "Should fail with invalid email" + assert_not_empty signup_invalid.errors[:email_address], "Should have validation error for email_address" - test "#process does nothing if a basic validation error occurs" do - @signup.password = "" - - assert_not @signup.process - assert_not_empty @signup.errors[:password] - - assert_nil @signup.tenant - assert_nil @signup.account - assert_nil @signup.user - assert_equal @starting_tenants, ApplicationRecord.tenants - end - - test "#process does nothing if an error occurs creating the queenbee record" do Queenbee::Remote::Account.stubs(:create!).raises(RuntimeError, "Invalid account data") + signup_error = Signup.new(email_address: "error@example.com") - assert_not @signup.process - assert_not_empty @signup.errors[:base] - - assert_nil @signup.tenant - assert_nil @signup.account - assert_nil @signup.user - assert_equal @starting_tenants, ApplicationRecord.tenants + assert_not signup_error.create_account, "Should fail when error occurs" + assert_not_empty signup_error.errors[:base], "Should have base error" + assert_nil signup_error.tenant + assert_nil signup_error.account + assert_nil signup_error.user end - test "#process does nothing if an error occurs creating the tenant" do - ApplicationRecord.stubs(:create_tenant).raises(RuntimeError, "Tenant already exists") + test "#complete" do + Account.sole.update!(setup_status: :pending) + signup = Signup.new( + tenant: ApplicationRecord.current_tenant, + user: users(:kevin), + full_name: "Kevin Systrom", + company_name: "37signals" + ) - Queenbee::Remote::Account.any_instance.expects(:cancel).once + assert signup.complete, signup.errors.full_messages.to_sentence(words_connector: ". ") - assert_not @signup.process - assert_not_empty @signup.errors[:base] + assert_equal "complete", Account.sole.reload.setup_status, "Account setup status should be complete" + assert_equal "Kevin Systrom", users(:kevin).reload.name, "User name should be updated" + assert_equal "37signals", Account.sole.reload.name, "Account name should be updated" + assert_equal "37signals", users(:kevin).membership.reload.account_name, "Membership account name should be updated" - assert_nil @signup.tenant - assert_nil @signup.account - assert_nil @signup.user - assert_equal @starting_tenants, ApplicationRecord.tenants - end - - test "#process does nothing if an error occurs creating the tenanted records" do - Account.stubs(:create_with_admin_user).raises(ActiveRecord::RecordInvalid, "Validation failed: Name can't be blank") - - Queenbee::Remote::Account.any_instance.expects(:cancel).once - - assert_not @signup.process - assert_not_empty @signup.errors[:base] - - assert_nil @signup.tenant - assert_nil @signup.account - assert_nil @signup.user - assert_equal @starting_tenants, ApplicationRecord.tenants + signup.full_name = "" + assert_not signup.complete, "Complete should fail with invalid params" + assert_not_empty signup.errors[:full_name], "Should have validation error for full_name" end end diff --git a/script/migrations/20251013-mark-all-accounts-as-setup.rb b/script/migrations/20251013-mark-all-accounts-as-setup.rb new file mode 100755 index 000000000..792fcb288 --- /dev/null +++ b/script/migrations/20251013-mark-all-accounts-as-setup.rb @@ -0,0 +1,8 @@ +#!/usr/bin/env ruby + +require_relative "../../config/environment" + +ApplicationRecord.with_each_tenant do |tenant| + puts "✅ #{tenant}" + Account.sole.setup_complete! +end diff --git a/test/controllers/controller_authentication_test.rb b/test/controllers/controller_authentication_test.rb index b3b2c75d3..509ce26c1 100644 --- a/test/controllers/controller_authentication_test.rb +++ b/test/controllers/controller_authentication_test.rb @@ -1,19 +1,18 @@ require "test_helper" class ControllerAuthenticationTest < ActionDispatch::IntegrationTest - test "access without an account slug redirects to new session" do + test "access without an account slug redirects to login menu" do integration_session.default_url_options[:script_name] = "" # no tenant get cards_path - assert_response :success - assert_dom "p", text: "You don't have any existing BOXCAR accounts." + assert_redirected_to session_login_menu_path end test "access with an account slug but no session redirects to new session" do get cards_path - assert_redirected_to new_session_path + assert_redirected_to new_session_path(script_name: nil) end test "access with an account slug and a session allows functional access" do diff --git a/test/controllers/sessions/login_menus_controller_test.rb b/test/controllers/sessions/login_menus_controller_test.rb new file mode 100644 index 000000000..8527b1fdd --- /dev/null +++ b/test/controllers/sessions/login_menus_controller_test.rb @@ -0,0 +1,21 @@ +require "test_helper" + +class Sessions::LoginMenusControllerTest < ActionDispatch::IntegrationTest + test "show" do + untenanted do + set_identity_as :kevin + + get session_login_menu_url + + assert_response :success + end + end + + test "create" do + untenanted do + sign_in_as :kevin + + assert cookies[:session_token].present? + end + end +end diff --git a/test/controllers/sessions/magic_links_controller_test.rb b/test/controllers/sessions/magic_links_controller_test.rb new file mode 100644 index 000000000..837107309 --- /dev/null +++ b/test/controllers/sessions/magic_links_controller_test.rb @@ -0,0 +1,36 @@ +require "test_helper" + +class Sessions::MagicLinksControllerTest < ActionDispatch::IntegrationTest + test "show" do + untenanted do + get session_magic_link_url + + assert_response :success + end + end + + test "create" do + untenanted do + membership = memberships(:kevin_in_37signals) + magic_link = MagicLink.create!(membership: membership) + + post session_magic_link_url, params: { code: magic_link.code } + + assert_response :redirect, "Valid magic link should redirect" + assert cookies[:identity_token].present?, "Valid magic link should set identity token" + assert_not MagicLink.exists?(magic_link.id), "Valid magic link should be consumed" + + post session_magic_link_url, params: { code: "INVALID" } + + assert_response :redirect, "Invalid code should redirect" + + expired_link = MagicLink.create!(membership: membership) + expired_link.update_column(:expires_at, 1.hour.ago) + + post session_magic_link_url, params: { code: expired_link.code } + + assert_response :redirect, "Expired magic link should redirect" + assert MagicLink.exists?(expired_link.id), "Expired magic link should not be consumed" + end + end +end diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 09f1f13f0..b6a9b7b83 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -11,26 +11,32 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest end test "new" do - get new_session_path + untenanted do + get new_session_path - assert_response :success - end - - test "create with valid credentials" do - assert_difference -> { Identity.count }, 1 do - assert_difference -> { Membership.count }, 1 do - post session_path, params: { email_address: "david@37signals.com", password: "secret123456" } - end + assert_response :success end - - assert_redirected_to root_path - assert cookies[:session_token].present? end - test "create with invalid credentials" do - post session_path, params: { email_address: "david@37signals.com", password: "wrong" } + test "create with existing membership" do + untenanted do + membership = memberships(:kevin_in_37signals) - assert_redirected_to new_session_path - assert_not cookies[:session_token].present? + assert_difference -> { MagicLink.count }, 1 do + post session_path, params: { email_address: membership.email_address } + end + + assert_redirected_to session_magic_link_path + end + end + + test "create with non-existent email" do + untenanted do + assert_no_difference -> { MagicLink.count } do + post session_path, params: { email_address: "nonexistent@example.com" } + end + + assert_redirected_to session_magic_link_path + end end end diff --git a/test/fixtures/accesses.yml b/test/fixtures/accesses.yml index 75e6fdf02..ad1a82b47 100644 --- a/test/fixtures/accesses.yml +++ b/test/fixtures/accesses.yml @@ -1,6 +1,7 @@ writebook_david: collection: writebook user: david + involvement: watching writebook_jz: collection: writebook @@ -9,6 +10,7 @@ writebook_jz: writebook_kevin: collection: writebook user: kevin + involvement: watching private_kevin: collection: private diff --git a/test/fixtures/identities.yml b/test/fixtures/identities.yml index 8b1378917..0da6dd9ae 100644 --- a/test/fixtures/identities.yml +++ b/test/fixtures/identities.yml @@ -1 +1,3 @@ +kevin_identity: {} +jz_identity: {} diff --git a/test/fixtures/memberships.yml b/test/fixtures/memberships.yml index 8b1378917..fd9924c21 100644 --- a/test/fixtures/memberships.yml +++ b/test/fixtures/memberships.yml @@ -1 +1,13 @@ +kevin_in_37signals: + identity: kevin_identity + email_address: kevin@37signals.com + user_tenant: <%= Rails.application.config.active_record_tenanted.default_tenant %> + user_id: <%= ActiveRecord::FixtureSet.identify(:kevin) %> + account_name: Fizzy +jz_in_37signals: + identity: jz_identity + email_address: jz@37signals.com + user_tenant: <%= Rails.application.config.active_record_tenanted.default_tenant %> + user_id: <%= ActiveRecord::FixtureSet.identify(:jz) %> + account_name: Fizzy diff --git a/test/integration/identity_membership_test.rb b/test/integration/identity_membership_test.rb index 03d8a33f4..11a28fba5 100644 --- a/test/integration/identity_membership_test.rb +++ b/test/integration/identity_membership_test.rb @@ -1,37 +1,16 @@ require "test_helper" class IdentityMembershipTest < ActionDispatch::IntegrationTest - setup do - @tenants = [ ActiveRecord::FixtureSet.identify("identity-tenant-1"), - ActiveRecord::FixtureSet.identify("identity-tenant-2") ] - @tenant_paths = @tenants.map { "/#{_1}" } - @tenant_urls = @tenant_paths.map { root_url(script_name: _1) } - @user_params = { email_address: "user@example.com", password: "password1234" } - @users = @tenants.map do |tenant| - ApplicationRecord.create_tenant(tenant) do - Account.create! name: "Account for #{tenant}" - User.create! @user_params.merge(name: "Harold Hancox") - end - end - end - test "multiple signins on the same browser" do - post session_path(script_name: @tenant_paths[0], params: @user_params) - assert_redirected_to root_path(script_name: @tenant_paths[0]) + # Sign in as kevin to first account + kevin = users(:kevin) + sign_in_as(kevin) - post session_path(script_name: @tenant_paths[1], params: @user_params) - assert_redirected_to root_path(script_name: @tenant_paths[1]) + # Then sign in as JZ + jz = users(:jz) + set_identity_as(jz) - # Render links for other Fizzies in the jump menu - get my_menu_path(script_name: @tenant_paths[0]) - assert_select "#my_menu ul li a[href='#{@tenant_urls[1]}']", "Account for #{@tenants[1]}: user@example.com" - - get my_menu_path(script_name: @tenant_paths[1]) - assert_select "#my_menu ul li a[href='#{@tenant_urls[0]}']", "Account for #{@tenants[0]}: user@example.com" - - # Render links for all the identity's Fizzies - get root_path(script_name: nil) - assert_select "ul li a[href='#{@tenant_urls[0]}']", "Account for #{@tenants[0]}: user@example.com" - assert_select "ul li a[href='#{@tenant_urls[1]}']", "Account for #{@tenants[1]}: user@example.com" + expected_membership_ids = [ memberships(:kevin_in_37signals), memberships(:jz_in_37signals) ].map(&:id) + assert_equal expected_membership_ids.sort, jz.reload.identity.memberships.pluck(:id).sort end end diff --git a/test/mailers/magic_link_mailer_test.rb b/test/mailers/magic_link_mailer_test.rb new file mode 100644 index 000000000..2c6785ec4 --- /dev/null +++ b/test/mailers/magic_link_mailer_test.rb @@ -0,0 +1,16 @@ +require "test_helper" + +class MagicLinkMailerTest < ActionMailer::TestCase + test "sign_in_instructions" do + magic_link = MagicLink.create!(membership: memberships(:kevin_in_37signals)) + email = MagicLinkMailer.sign_in_instructions(magic_link) + + assert_emails 1 do + email.deliver_now + end + + assert_equal [ "kevin@37signals.com" ], email.to + assert_equal "Sign in to Fizzy", email.subject + assert_match magic_link.code, email.body.encoded + end +end diff --git a/test/mailers/previews/magic_link_preview.rb b/test/mailers/previews/magic_link_preview.rb new file mode 100644 index 000000000..af638f373 --- /dev/null +++ b/test/mailers/previews/magic_link_preview.rb @@ -0,0 +1,8 @@ +class MagicLinkPreview < ActionMailer::Preview + def magic_link + membership = Membership.new email_address: "test@example.com" + magic_link = MagicLink.new(membership: membership) + magic_link.valid? + MagicLinkMailer.magic_link(magic_link) + end +end diff --git a/test/models/magic_link/code_test.rb b/test/models/magic_link/code_test.rb new file mode 100644 index 000000000..685f26d69 --- /dev/null +++ b/test/models/magic_link/code_test.rb @@ -0,0 +1,17 @@ +require "test_helper" + +class MagicLink::CodeTest < ActiveSupport::TestCase + test "generate" do + code = MagicLink::Code.generate(6) + + assert_equal 6, code.length + assert_match(/\A[#{MagicLink::Code::CODE_ALPHABET.join}]+\z/, code) + end + + test "sanitize" do + assert_equal "011123", MagicLink::Code.sanitize("OIL123") + assert_equal "ABC123", MagicLink::Code.sanitize("ABC-123 !@#") + assert_nil MagicLink::Code.sanitize(nil) + assert_nil MagicLink::Code.sanitize("") + end +end diff --git a/test/models/magic_link_test.rb b/test/models/magic_link_test.rb new file mode 100644 index 000000000..0e921096e --- /dev/null +++ b/test/models/magic_link_test.rb @@ -0,0 +1,58 @@ +require "test_helper" + +class MagicLinkTest < ActiveSupport::TestCase + test "new" do + magic_link = MagicLink.create!(membership: memberships(:kevin_in_37signals)) + + assert magic_link.code.present? + assert_equal MagicLink::CODE_LENGTH, magic_link.code.length + assert magic_link.expires_at.present? + assert_in_delta MagicLink::EXPIRATION_TIME.from_now, magic_link.expires_at, 1.second + end + + test "active" do + active_link = MagicLink.create!(membership: memberships(:kevin_in_37signals)) + expired_link = MagicLink.create!(membership: memberships(:kevin_in_37signals)) + expired_link.update_column(:expires_at, 1.hour.ago) + + assert_includes MagicLink.active, active_link + assert_not_includes MagicLink.active, expired_link + end + + test "stale" do + active_link = MagicLink.create!(membership: memberships(:kevin_in_37signals)) + expired_link = MagicLink.create!(membership: memberships(:kevin_in_37signals)) + expired_link.update_column(:expires_at, 1.hour.ago) + + assert_includes MagicLink.stale, expired_link + assert_not_includes MagicLink.stale, active_link + end + + test "consume" do + magic_link = MagicLink.create!(membership: memberships(:kevin_in_37signals)) + code_with_spaces = magic_link.code.downcase.chars.join(" ") + + membership = MagicLink.consume(code_with_spaces) + assert_equal memberships(:kevin_in_37signals), membership + assert_not MagicLink.exists?(magic_link.id) + + expired_link = MagicLink.create!(membership: memberships(:kevin_in_37signals)) + expired_link.update_column(:expires_at, 1.hour.ago) + assert_nil MagicLink.consume(expired_link.code) + assert MagicLink.exists?(expired_link.id) + + assert_nil MagicLink.consume("INVALID") + assert_nil MagicLink.consume(nil) + end + + test "cleanup" do + active_link = MagicLink.create!(membership: memberships(:kevin_in_37signals)) + expired_link = MagicLink.create!(membership: memberships(:kevin_in_37signals)) + expired_link.update_column(:expires_at, 1.hour.ago) + + MagicLink.cleanup + + assert MagicLink.exists?(active_link.id) + assert_not MagicLink.exists?(expired_link.id) + end +end diff --git a/test/models/membership_test.rb b/test/models/membership_test.rb index 8506331ed..f544b0c25 100644 --- a/test/models/membership_test.rb +++ b/test/models/membership_test.rb @@ -1,7 +1,31 @@ require "test_helper" class MembershipTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end + test "send_magic_link" do + membership = memberships(:kevin_in_37signals) + + assert_difference -> { membership.magic_links.count }, 1 do + membership.send_magic_link + end + + assert_enqueued_jobs 1, only: ActionMailer::MailDeliveryJob + end + + test "user" do + membership = memberships(:kevin_in_37signals) + + user = membership.user + + assert_equal users(:kevin).id, user.id + assert_equal users(:kevin).email_address, user.email_address + end + + test "account" do + membership = memberships(:kevin_in_37signals) + + account = membership.account + + assert_equal Account.sole.id, account.id + assert_equal Account.sole.name, account.name + end end diff --git a/test/models/user/identifiable_test.rb b/test/models/user/identifiable_test.rb index fb0529a06..4ff50783d 100644 --- a/test/models/user/identifiable_test.rb +++ b/test/models/user/identifiable_test.rb @@ -30,26 +30,16 @@ class User::IdentifiableTest < ActiveSupport::TestCase end test "set_identity when token is an identity and user has a different identity" do - user = users(:david) - user2 = users(:jz) - token_identity = Identity.create! - user_identity = Identity.create! - user_identity.memberships.create!(user_id: user.id, user_tenant: user.tenant, email_address: user.email_address, account_name: "asdf") - user_identity.memberships.create!(user_id: user2.id, user_tenant: user2.tenant, email_address: user2.email_address, account_name: "qwer") - assert_equal(user_identity, user.identity) - assert_equal(user_identity, user2.identity) + kevin = users(:kevin) + jz = users(:jz) - result = user.set_identity(token_identity) - user.reload - user2.reload + assert_not_equal(kevin.identity, jz.identity, "Kevin and JZ should start with different identities") - assert_equal(token_identity, result) - assert_equal(token_identity, user.identity) - assert_equal(token_identity, user2.identity) + kevin.set_identity(jz.identity) + kevin.reload + jz.reload - assert_raises(ActiveRecord::RecordNotFound) do - user_identity.reload - end + assert_equal(kevin.identity, jz.identity, "Kevin and JZ should now share the same identity") end test "set_identity when token is nil and user has an identity" do diff --git a/test/test_helpers/session_test_helper.rb b/test/test_helpers/session_test_helper.rb index ecc489b57..7e7e16feb 100644 --- a/test/test_helpers/session_test_helper.rb +++ b/test/test_helpers/session_test_helper.rb @@ -7,12 +7,39 @@ module SessionTestHelper cookies.delete :session_token user = users(user) unless user.is_a? User - post session_path, params: { email_address: user.email_address, password: "secret123456" } - assert_response :redirect + set_identity_as user - cookie = cookies.get_cookie "session_token" - assert_not_nil cookie, "Expected session_token cookie to be set after sign in" - assert_equal Account.sole.slug, cookie.path, "Expected session_token cookie to be scoped to account slug" + user.reload + membership = user.membership + tenanted do + post session_login_menu_url, params: { membership_id: membership.id } + assert_response :redirect, "Login should succeed" + + cookie = cookies.get_cookie "session_token" + assert_not_nil cookie, "Expected session_token cookie to be set after sign in" + assert_equal Account.sole.slug, cookie.path, "Expected session_token cookie to be scoped to account slug" + end + end + + def set_identity_as(user_or_identity) + user = if user_or_identity.is_a?(User) + user_or_identity + else + users(user_or_identity) + end + + membership = user.membership || user.set_identity(nil).memberships.find_by(user_id: user.id, user_tenant: user.tenant) + membership.send_magic_link + + magic_link = membership.magic_links.order(id: :desc).first + + untenanted do + post session_magic_link_url, params: { code: magic_link.code } + assert_response :redirect, "Magic link should succeed" + + cookie = cookies.get_cookie "identity_token" + assert_not_nil cookie, "Expected identity_token cookie to be set after magic link consumption" + end end def sign_out @@ -27,4 +54,20 @@ module SessionTestHelper ensure Current.clear_all end + + def untenanted(&block) + original_script_name = integration_session.default_url_options[:script_name] + integration_session.default_url_options[:script_name] = "" + yield + ensure + integration_session.default_url_options[:script_name] = original_script_name + end + + def tenanted(tenant = ApplicationRecord.current_tenant, &block) + original_script_name = integration_session.default_url_options[:script_name] + integration_session.default_url_options[:script_name] = "/#{tenant}" + yield + ensure + integration_session.default_url_options[:script_name] = original_script_name + end end