From 5747445977d9feb73c9b4224ce47881b40afad84 Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Sat, 29 Nov 2025 10:15:48 +0100 Subject: [PATCH 1/8] Create signups controller --- Gemfile.saas | 2 +- Gemfile.saas.lock | 3 ++- app/controllers/sessions_controller.rb | 19 ++++--------------- app/controllers/signups_controller.rb | 25 +++++++++++++++++++++++++ config/routes.rb | 10 +++++++--- 5 files changed, 39 insertions(+), 20 deletions(-) create mode 100644 app/controllers/signups_controller.rb diff --git a/Gemfile.saas b/Gemfile.saas index a56a7b682..b986ecafe 100644 --- a/Gemfile.saas +++ b/Gemfile.saas @@ -6,7 +6,7 @@ git_source(:bc) { |repo| "https://github.com/basecamp/#{repo}" } gem "activeresource", require: "active_resource" gem "queenbee", bc: "queenbee-plugin" -gem "fizzy-saas", bc: "fizzy-saas" +gem "fizzy-saas", bc: "fizzy-saas", ref: "remove-unused-route" gem "rails_structured_logging", bc: "rails-structured-logging" gem "sentry-ruby" gem "sentry-rails" diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index fc1bcd758..0e4a78540 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -1,6 +1,7 @@ GIT remote: https://github.com/basecamp/fizzy-saas - revision: 7f392bbbf9f5170d334b6ee2f6d240569bd157ed + revision: 4740eeaa3ac652bfb695282a963a79fa1a1409e3 + ref: remove-unused-route specs: fizzy-saas (0.1.0) prometheus-client-mmap diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index aa765919d..facc98e17 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -19,10 +19,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 +35,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/signups_controller.rb b/app/controllers/signups_controller.rb new file mode 100644 index 000000000..5d4aa0eef --- /dev/null +++ b/app/controllers/signups_controller.rb @@ -0,0 +1,25 @@ +class SignupsController < ApplicationController + 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." } + + layout "public" + + def new + @signup = Signup.new + end + + def create + if Current.identity + Signup.new(signup_params).create_identity + redirect_to session_magic_link_path + else + redirect_to new_signup_completion_path + end + end + + private + def signup_params + params.expect signup: :email_address + end +end diff --git a/config/routes.rb b/config/routes.rb index d4e4b17e6..91ae0055c 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: :signup, as: :signup do + resource :completion, only: %i[ new create ] + end + end end resource :landing From 658b5a07cfe339f58c889b41a5ff207309251145 Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Sat, 29 Nov 2025 11:46:09 +0100 Subject: [PATCH 2/8] Organize everything under Signups --- Gemfile.saas.lock | 12 ++--- app/controllers/sessions_controller.rb | 9 ---- .../completions_controller.rb | 2 +- app/controllers/signups_controller.rb | 21 +++++--- .../completions/new.html.erb | 0 app/views/{signup => signups}/new.html.erb | 0 config/routes.rb | 2 +- test/controllers/sessions_controller_test.rb | 8 ++- test/controllers/signups_controller_test.rb | 52 +++++++++++++++++++ 9 files changed, 78 insertions(+), 28 deletions(-) rename app/controllers/{signup => signups}/completions_controller.rb (88%) rename app/views/{signup => signups}/completions/new.html.erb (100%) rename app/views/{signup => signups}/new.html.erb (100%) create mode 100644 test/controllers/signups_controller_test.rb diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index 0e4a78540..e9a585a26 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -1,6 +1,6 @@ GIT remote: https://github.com/basecamp/fizzy-saas - revision: 4740eeaa3ac652bfb695282a963a79fa1a1409e3 + revision: 0c6f9f6c7fa767fb83645286cdaf6705f9b753a5 ref: remove-unused-route specs: fizzy-saas (0.1.0) @@ -53,7 +53,7 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: 4f7ab01bb5d6be78c7447dbb230c55027d08ae34 + revision: 690ec8898318b8f50714e86676353ebe1551261e branch: main specs: actioncable (8.2.0.alpha) @@ -424,7 +424,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 @@ -480,10 +480,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_controller.rb b/app/controllers/sessions_controller.rb index facc98e17..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." } 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 index 5d4aa0eef..ea88383e0 100644 --- a/app/controllers/signups_controller.rb +++ b/app/controllers/signups_controller.rb @@ -1,7 +1,16 @@ 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" @@ -10,15 +19,15 @@ class SignupsController < ApplicationController end def create - if Current.identity - Signup.new(signup_params).create_identity - redirect_to session_magic_link_path - else - redirect_to new_signup_completion_path - end + 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 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/signup/new.html.erb b/app/views/signups/new.html.erb similarity index 100% rename from app/views/signup/new.html.erb rename to app/views/signups/new.html.erb diff --git a/config/routes.rb b/config/routes.rb index 91ae0055c..6d13e219b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -159,7 +159,7 @@ Rails.application.routes.draw do resource :signup, only: %i[ new create ] do collection do - scope module: :signup, as: :signup do + scope module: :signups, as: :signup do resource :completion, only: %i[ new create ] end end 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 From 354dd1e95ab39252b5c134f68a3ad59e1915d8df Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Sat, 29 Nov 2025 11:58:48 +0100 Subject: [PATCH 3/8] Update signup with the design from signin --- app/views/sessions/menus/show.html.erb | 2 +- app/views/sessions/new.html.erb | 2 +- app/views/signups/new.html.erb | 26 +++++++++++--------------- 3 files changed, 13 insertions(+), 17 deletions(-) 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 %> From e302e79b557560136c5d97173163bc0c87bed901 Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Sat, 29 Nov 2025 11:59:10 +0100 Subject: [PATCH 4/8] Remove unused view --- app/views/sessions/menus/show.html+menu_section.erb | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 app/views/sessions/menus/show.html+menu_section.erb 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 %> From 2311efd03e1edff86f14cf86b63fbced769cf3a0 Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Sat, 29 Nov 2025 12:36:00 +0100 Subject: [PATCH 5/8] Make sign up Magic Links work across devices Previously if someone started signing in on their phone, but finished it on their laptop, they'd end up on the menu screen with no account. Bu tracking the purpose of a Magic Link we can always direct the user to sign up if they requested a magic link through sign up. This also makes the logic for changing the copy in the email more robust. --- .../sessions/magic_links_controller.rb | 14 ++++++++--- app/models/identity.rb | 6 +++-- app/models/magic_link.rb | 4 ++- app/models/signup.rb | 2 +- .../sign_in_instructions.html.erb | 2 +- .../sign_in_instructions.text.erb | 2 +- ...251129110120_add_purpose_to_magic_links.rb | 11 ++++++++ db/schema.rb | 3 ++- .../sessions/magic_links_controller_test.rb | 25 +++++++++++++++---- test/models/magic_link_test.rb | 4 +-- 10 files changed, 56 insertions(+), 17 deletions(-) create mode 100644 db/migrate/20251129110120_add_purpose_to_magic_links.rb 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/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/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/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/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)) From 7df30534f40150709f3a338c6b7d429865332c49 Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Sat, 29 Nov 2025 15:20:21 +0100 Subject: [PATCH 6/8] Change copy --- app/views/signups/new.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/signups/new.html.erb b/app/views/signups/new.html.erb index 0020e0589..4578d020c 100644 --- a/app/views/signups/new.html.erb +++ b/app/views/signups/new.html.erb @@ -10,7 +10,7 @@ -

New here? Enter your email to create an account.

+

Enter your email to create an account.