diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index fc1bcd758..938a53698 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -1,6 +1,6 @@ GIT remote: https://github.com/basecamp/fizzy-saas - revision: 7f392bbbf9f5170d334b6ee2f6d240569bd157ed + revision: f80da3c2faf34b94d65a41a501f19e8dba379012 specs: fizzy-saas (0.1.0) prometheus-client-mmap @@ -52,7 +52,7 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: 4f7ab01bb5d6be78c7447dbb230c55027d08ae34 + revision: 690ec8898318b8f50714e86676353ebe1551261e branch: main specs: actioncable (8.2.0.alpha) @@ -423,7 +423,7 @@ GEM rake-compiler-dock (1.9.1) rb_sys (0.9.117) rake-compiler-dock (= 1.9.1) - rdoc (6.15.1) + rdoc (6.16.1) erb psych (>= 4.0.0) tsort @@ -479,10 +479,10 @@ GEM rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2, < 4.0) websocket (~> 1.0) - sentry-rails (6.1.1) + sentry-rails (6.2.0) railties (>= 5.2.0) - sentry-ruby (~> 6.1.1) - sentry-ruby (6.1.1) + sentry-ruby (~> 6.2.0) + sentry-ruby (6.2.0) bigdecimal concurrent-ruby (~> 1.0, >= 1.0.2) sniffer (0.5.0) diff --git a/app/controllers/sessions/magic_links_controller.rb b/app/controllers/sessions/magic_links_controller.rb index 40de894df..dc55782af 100644 --- a/app/controllers/sessions/magic_links_controller.rb +++ b/app/controllers/sessions/magic_links_controller.rb @@ -9,9 +9,9 @@ class Sessions::MagicLinksController < ApplicationController end def create - if identity = MagicLink.consume(code) - start_new_session_for identity - redirect_to after_authentication_url + if magic_link = MagicLink.consume(code) + start_new_session_for magic_link.identity + redirect_to after_sign_in_url(magic_link) else redirect_to session_magic_link_path, alert: "Try another code." end @@ -21,4 +21,12 @@ class Sessions::MagicLinksController < ApplicationController def code params.expect(:code) end + + def after_sign_in_url(magic_link) + if magic_link.for_sign_up? + new_signup_completion_path + else + after_authentication_url + end + end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index aa765919d..72131e61c 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,13 +1,4 @@ class SessionsController < ApplicationController - # FIXME: Remove this before launch! - unless Rails.env.local? - http_basic_authenticate_with \ - name: Rails.application.credentials.account_signup_http_basic_auth.name, - password: Rails.application.credentials.account_signup_http_basic_auth.password, - realm: "Fizzy Signup", - only: :create, unless: -> { Identity.exists?(email_address: email_address) } - end - disallow_account_scope require_unauthenticated_access except: :destroy rate_limit to: 10, within: 3.minutes, only: :create, with: -> { redirect_to new_session_path, alert: "Try again later." } @@ -19,10 +10,11 @@ class SessionsController < ApplicationController def create if identity = Identity.find_by_email_address(email_address) - handle_existing_user(identity) - elsif - handle_new_signup + magic_link = identity.send_magic_link + flash[:magic_link_code] = magic_link&.code if Rails.env.development? end + + redirect_to session_magic_link_path end def destroy @@ -34,16 +26,4 @@ class SessionsController < ApplicationController def email_address params.expect(:email_address) end - - def handle_existing_user(identity) - magic_link = identity.send_magic_link - flash[:magic_link_code] = magic_link&.code if Rails.env.development? - redirect_to session_magic_link_path - end - - def handle_new_signup - Signup.new(email_address: email_address).create_identity - session[:return_to_after_authenticating] = new_signup_completion_path - redirect_to session_magic_link_path - end end diff --git a/app/controllers/signup/completions_controller.rb b/app/controllers/signups/completions_controller.rb similarity index 88% rename from app/controllers/signup/completions_controller.rb rename to app/controllers/signups/completions_controller.rb index d7f09c086..f0ea56ff9 100644 --- a/app/controllers/signup/completions_controller.rb +++ b/app/controllers/signups/completions_controller.rb @@ -1,4 +1,4 @@ -class Signup::CompletionsController < ApplicationController +class Signups::CompletionsController < ApplicationController layout "public" disallow_account_scope diff --git a/app/controllers/signups_controller.rb b/app/controllers/signups_controller.rb new file mode 100644 index 000000000..ea88383e0 --- /dev/null +++ b/app/controllers/signups_controller.rb @@ -0,0 +1,34 @@ +class SignupsController < ApplicationController + # FIXME: Remove this before launch! + unless Rails.env.local? + http_basic_authenticate_with \ + name: Rails.application.credentials.account_signup_http_basic_auth.name, + password: Rails.application.credentials.account_signup_http_basic_auth.password, + realm: "Fizzy Signup" + end + + disallow_account_scope + allow_unauthenticated_access + rate_limit to: 10, within: 3.minutes, only: :create, with: -> { redirect_to new_signup_path, alert: "Try again later." } + before_action :redirect_authenticated_user + + layout "public" + + def new + @signup = Signup.new + end + + def create + Signup.new(signup_params).create_identity + redirect_to session_magic_link_path + end + + private + def redirect_authenticated_user + redirect_to new_signup_completion_path if authenticated? + end + + def signup_params + params.expect signup: :email_address + end +end diff --git a/app/models/identity.rb b/app/models/identity.rb index e9ff507a1..135fdaae7 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -12,8 +12,10 @@ class Identity < ApplicationRecord normalizes :email_address, with: ->(value) { value.strip.downcase.presence } - def send_magic_link - magic_links.create!.tap do |magic_link| + def send_magic_link(**attributes) + attributes[:purpose] = attributes.delete(:for) if attributes.key?(:for) + + magic_links.create!(attributes).tap do |magic_link| MagicLinkMailer.sign_in_instructions(magic_link).deliver_later end end diff --git a/app/models/magic_link.rb b/app/models/magic_link.rb index 69961e9a5..987ca776a 100644 --- a/app/models/magic_link.rb +++ b/app/models/magic_link.rb @@ -4,6 +4,8 @@ class MagicLink < ApplicationRecord belongs_to :identity + enum :purpose, %w[ sign_in sign_up ], prefix: :for, default: :sign_in + scope :active, -> { where(expires_at: Time.current...) } scope :stale, -> { where(expires_at: ..Time.current) } @@ -24,7 +26,7 @@ class MagicLink < ApplicationRecord def consume destroy - identity + self end private diff --git a/app/models/signup.rb b/app/models/signup.rb index 230aafd0b..bdcf3490d 100644 --- a/app/models/signup.rb +++ b/app/models/signup.rb @@ -18,7 +18,7 @@ class Signup def create_identity @identity = Identity.find_or_create_by!(email_address: email_address) - @identity.send_magic_link + @identity.send_magic_link for: :sign_up end def complete 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 index 68ddeb165..40303c39e 100644 --- a/app/views/mailers/magic_link_mailer/sign_in_instructions.html.erb +++ b/app/views/mailers/magic_link_mailer/sign_in_instructions.html.erb @@ -1,5 +1,5 @@ -<% if @identity.users.any? %> +<% if @magic_link.for_sign_in? %>

Fizzy verification code

Please enter this 6-character verification code on the Fizzy sign-in page:

<% else %> 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 index 2d5ba8f8b..b21f00509 100644 --- a/app/views/mailers/magic_link_mailer/sign_in_instructions.text.erb +++ b/app/views/mailers/magic_link_mailer/sign_in_instructions.text.erb @@ -1,4 +1,4 @@ -<% if @identity.users.any? %> +<% if @magic_link.for_sign_in? %> Please enter this 6-character verification code on the Fizzy sign-in page: <% else %> Please enter this 6-character verification code on the Fizzy sign-up page to create your new account: diff --git a/app/views/sessions/menus/show.html+menu_section.erb b/app/views/sessions/menus/show.html+menu_section.erb deleted file mode 100644 index e3468db65..000000000 --- a/app/views/sessions/menus/show.html+menu_section.erb +++ /dev/null @@ -1,9 +0,0 @@ -<%= turbo_frame_tag Current.identity, :account_menu do %> - <% cache [ Current.identity, @accounts ] do %> - <%= collapsible_nav_section "Accounts" do %> - <% @accounts.each do |account| %> - <%= filter_place_menu_item landing_url(script_name: account.slug, account.name, "marker", new_window: true %> - <% end %> - <% end %> - <% end %> -<% end %> diff --git a/app/views/sessions/menus/show.html.erb b/app/views/sessions/menus/show.html.erb index 8a36261bf..9c6867740 100644 --- a/app/views/sessions/menus/show.html.erb +++ b/app/views/sessions/menus/show.html.erb @@ -25,7 +25,7 @@ <% end %>
- <%= link_to new_signup_completion_path, class: "btn btn--plain txt-link center txt-small", data: { turbo_prefetch: false } do %> + <%= link_to new_signup_path, class: "btn btn--plain txt-link center txt-small", data: { turbo_prefetch: false } do %> Sign up for a new Fizzy account <% end %>
diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index f66d08736..b90809105 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -10,7 +10,7 @@ -

New here? Enter your email to create an account. Already have an account? Enter your email and we’ll get you signed in.

+

New here? <%= link_to "sign up", new_signup_path %> to create an account. Already have an account? Enter your email and we’ll get you signed in.

- <% end %> - - -<% content_for :footer do %> - <%= render "sessions/footer" %> -<% end %> diff --git a/app/views/signup/completions/new.html.erb b/app/views/signups/completions/new.html.erb similarity index 100% rename from app/views/signup/completions/new.html.erb rename to app/views/signups/completions/new.html.erb diff --git a/app/views/signups/new.html.erb b/app/views/signups/new.html.erb new file mode 100644 index 000000000..4578d020c --- /dev/null +++ b/app/views/signups/new.html.erb @@ -0,0 +1,24 @@ +<% @page_title = "Signup for Fizzy" %> + +
+

Sign up

+ + <%= form_with model: @signup, url: signup_path, scope: "signup", class: "flex flex-column gap", data: { turbo: false, controller: "form" } do |form| %> +
+ +
+ +

Enter your email to create an account.

+ + + <% end %> +
+ +<% content_for :footer do %> + <%= render "sessions/footer" %> +<% end %> diff --git a/config/routes.rb b/config/routes.rb index d4e4b17e6..6d13e219b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -155,10 +155,14 @@ Rails.application.routes.draw do end end - get "/signup/new", to: redirect("/session/new") + get "/signup", to: redirect("/signup/new") - namespace :signup do - resource :completion, only: %i[ new create ] + resource :signup, only: %i[ new create ] do + collection do + scope module: :signups, as: :signup do + resource :completion, only: %i[ new create ] + end + end end resource :landing diff --git a/db/migrate/20251129110120_add_purpose_to_magic_links.rb b/db/migrate/20251129110120_add_purpose_to_magic_links.rb new file mode 100644 index 000000000..52e332d72 --- /dev/null +++ b/db/migrate/20251129110120_add_purpose_to_magic_links.rb @@ -0,0 +1,11 @@ +class AddPurposeToMagicLinks < ActiveRecord::Migration[8.2] + def change + add_column :magic_links, :purpose, :integer, null: true + + execute <<-SQL + UPDATE magic_links SET purpose = 0 + SQL + + change_column_null :magic_links, :purpose, false + end +end diff --git a/db/schema.rb b/db/schema.rb index 236c61df0..ef0a4d166 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2025_11_27_000001) do +ActiveRecord::Schema[8.2].define(version: 2025_11_29_110120) do create_table "accesses", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -315,6 +315,7 @@ ActiveRecord::Schema[8.2].define(version: 2025_11_27_000001) do t.datetime "created_at", null: false t.datetime "expires_at", null: false t.uuid "identity_id" + t.integer "purpose", null: false t.datetime "updated_at", null: false t.index ["code"], name: "index_magic_links_on_code", unique: true t.index ["expires_at"], name: "index_magic_links_on_expires_at" diff --git a/db/schema_sqlite.rb b/db/schema_sqlite.rb index fd3cedd44..149862250 100644 --- a/db/schema_sqlite.rb +++ b/db/schema_sqlite.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2025_11_27_000001) do +ActiveRecord::Schema[8.2].define(version: 2025_11_29_110120) do create_table "accesses", id: :uuid, force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -315,6 +315,7 @@ ActiveRecord::Schema[8.2].define(version: 2025_11_27_000001) do t.datetime "created_at", null: false t.datetime "expires_at", null: false t.uuid "identity_id" + t.integer "purpose", null: false t.datetime "updated_at", null: false t.index ["code"], name: "index_magic_links_on_code", unique: true t.index ["expires_at"], name: "index_magic_links_on_expires_at" diff --git a/test/controllers/sessions/magic_links_controller_test.rb b/test/controllers/sessions/magic_links_controller_test.rb index a79027556..dbde3723e 100644 --- a/test/controllers/sessions/magic_links_controller_test.rb +++ b/test/controllers/sessions/magic_links_controller_test.rb @@ -9,17 +9,32 @@ class Sessions::MagicLinksControllerTest < ActionDispatch::IntegrationTest end end - test "create" do + test "create with sign in code" do identity = identities(:kevin) magic_link = MagicLink.create!(identity: identity) untenanted do post session_magic_link_url, params: { code: magic_link.code } - end - assert_response :redirect - assert cookies[:session_token].present? - assert_not MagicLink.exists?(magic_link.id), "The magic link should be consumed" + assert_response :redirect + assert cookies[:session_token].present? + assert_redirected_to landing_path, "Should redirect to after authentication path" + assert_not MagicLink.exists?(magic_link.id), "The magic link should be consumed" + end + end + + test "create with sign up code" do + identity = identities(:kevin) + magic_link = MagicLink.create!(identity: identity, purpose: :sign_up) + + untenanted do + post session_magic_link_url, params: { code: magic_link.code } + + assert_response :redirect + assert cookies[:session_token].present? + assert_redirected_to new_signup_completion_path, "Should redirect to signup completion" + assert_not MagicLink.exists?(magic_link.id), "The magic link should be consumed" + end end test "create with invalid code" do diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index e4a2163b1..8edf74573 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -23,11 +23,9 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest test "create for a new user" do untenanted do - assert_difference -> { Identity.count }, +1 do - assert_difference -> { MagicLink.count }, +1 do - post session_path, - params: { email_address: "nonexistent-#{SecureRandom.hex(6)}@example.com" } - end + assert_no_difference -> { MagicLink.count } do + post session_path, + params: { email_address: "nonexistent-#{SecureRandom.hex(6)}@example.com" } end assert_redirected_to session_magic_link_path diff --git a/test/controllers/signups_controller_test.rb b/test/controllers/signups_controller_test.rb new file mode 100644 index 000000000..9d65c7a55 --- /dev/null +++ b/test/controllers/signups_controller_test.rb @@ -0,0 +1,52 @@ +require "test_helper" + +class SignupsControllerTest < ActionDispatch::IntegrationTest + test "new" do + untenanted do + get new_signup_path + + assert_response :success + end + end + + test "new for an authenticated user" do + identity = identities(:kevin) + sign_in_as identity + + untenanted do + get new_signup_path + + assert_redirected_to new_signup_completion_path + end + end + + test "create" do + email_address = "newuser-#{SecureRandom.hex(6)}@example.com" + + untenanted do + assert_difference -> { Identity.count }, +1 do + assert_difference -> { MagicLink.count }, +1 do + post signup_path, params: { signup: { email_address: email_address } } + end + end + + assert_redirected_to session_magic_link_path + end + end + + test "create for an authenticated user" do + identity = identities(:kevin) + sign_in_as identity + + untenanted do + assert_no_difference -> { Identity.count } do + assert_no_difference -> { MagicLink.count } do + post signup_path, + params: { signup: { email_address: identity.email_address } } + end + end + + assert_redirected_to new_signup_completion_path + end + end +end diff --git a/test/models/magic_link_test.rb b/test/models/magic_link_test.rb index 4dec89737..d9cb1f73e 100644 --- a/test/models/magic_link_test.rb +++ b/test/models/magic_link_test.rb @@ -32,8 +32,8 @@ class MagicLinkTest < ActiveSupport::TestCase magic_link = MagicLink.create!(identity: identities(:kevin)) code_with_spaces = magic_link.code.downcase.chars.join(" ") - identity = MagicLink.consume(code_with_spaces) - assert_equal identities(:kevin), identity + consumed_magic_link = MagicLink.consume(code_with_spaces) + assert_equal magic_link, consumed_magic_link assert_not MagicLink.exists?(magic_link.id) expired_link = MagicLink.create!(identity: identities(:kevin))