From 1be9ba6ee3937d48461a4968f5cfda59951683da Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Wed, 18 Jun 2025 15:24:50 -0400 Subject: [PATCH] Signup workflow for existing identity --- app/controllers/signup/accounts_controller.rb | 16 ++++---- app/controllers/signup/base_controller.rb | 30 ++++++++++++++ .../signup/completions_controller.rb | 23 +++++++++++ app/controllers/signup/sessions_controller.rb | 9 +++++ app/models/launchpad.rb | 4 ++ app/models/signup.rb | 18 +++++++++ app/views/signup/completions/new.html.erb | 1 + config/routes.rb | 2 + .../signup/accounts_controller_test.rb | 39 +++++++++++++++++++ test/models/signup_test.rb | 10 +++++ 10 files changed, 145 insertions(+), 7 deletions(-) create mode 100644 app/controllers/signup/base_controller.rb create mode 100644 app/controllers/signup/completions_controller.rb create mode 100644 app/controllers/signup/sessions_controller.rb create mode 100644 app/views/signup/completions/new.html.erb diff --git a/app/controllers/signup/accounts_controller.rb b/app/controllers/signup/accounts_controller.rb index e722684ea..614315ff3 100644 --- a/app/controllers/signup/accounts_controller.rb +++ b/app/controllers/signup/accounts_controller.rb @@ -1,5 +1,5 @@ -class Signup::AccountsController < ApplicationController - require_untenanted_access +class Signup::AccountsController < Signup::BaseController + before_action :reset_signup_storage def new @signup = Signup.new @@ -8,7 +8,10 @@ class Signup::AccountsController < ApplicationController def create @signup = Signup.new(signup_params) - if @signup.process + if @signup.recognized? + store_signup + redirect_to Launchpad.authentication_url(purpose: "signup", login_hint: @signup.email_address, redirect_uri: signup_session_url), allow_other_host: true + elsif @signup.process redirect_to_account(@signup.account) else render :new, status: :unprocessable_entity @@ -17,11 +20,10 @@ class Signup::AccountsController < ApplicationController private def signup_params - params.require(:signup).permit(:full_name, :email_address, :password, :company_name) + params.require(:signup).permit(*Signup::PERMITTED_KEYS) end - def redirect_to_account(account) - redirect_to account.signal_account.owner.remote_login_url(proceed_to: root_path), - allow_other_host: true + def store_signup + @signup.to_h.each { |key, value| signup_storage[key.to_s] = value } end end diff --git a/app/controllers/signup/base_controller.rb b/app/controllers/signup/base_controller.rb new file mode 100644 index 000000000..e3d112597 --- /dev/null +++ b/app/controllers/signup/base_controller.rb @@ -0,0 +1,30 @@ +class Signup::BaseController < ApplicationController + require_untenanted_access + + attr_reader :authenticated_identity + + private + def signup_storage + session[:signup] ||= {} + end + + def reset_signup_storage + session.delete(:signup) + end + + def authenticated_identity=(identity) + @authenticated_identity = identity + signup_storage["identity_id"] = identity.try(:id) + end + + def set_authenticated_identity + if identity_id = signup_storage["identity_id"] + @authenticated_identity = SignalId::Identity.find_by(id: identity_id) + end + end + + def redirect_to_account(account) + redirect_to account.signal_account.owner.remote_login_url(proceed_to: root_path), + allow_other_host: true + end +end diff --git a/app/controllers/signup/completions_controller.rb b/app/controllers/signup/completions_controller.rb new file mode 100644 index 000000000..4f97055a0 --- /dev/null +++ b/app/controllers/signup/completions_controller.rb @@ -0,0 +1,23 @@ +class Signup::CompletionsController < Signup::BaseController + before_action :set_authenticated_identity, only: :create + + def new + end + + def create + @signup = Signup.new(signup_params) + @signup.signal_identity = authenticated_identity + + if @signup.process + reset_signup_storage + redirect_to_account(@signup.account) + else + render plain: "Could not complete signup. This is probably a bug.", status: :unprocessable_entity, layout: false + end + end + + private + def signup_params + signup_storage.slice(*Signup::PERMITTED_KEYS.map(&:to_s)) + end +end diff --git a/app/controllers/signup/sessions_controller.rb b/app/controllers/signup/sessions_controller.rb new file mode 100644 index 000000000..b33c0cf2f --- /dev/null +++ b/app/controllers/signup/sessions_controller.rb @@ -0,0 +1,9 @@ +class Signup::SessionsController < Signup::BaseController + def create + if self.authenticated_identity = SignalId::Identity.authenticate(params.permit(:sig)) + redirect_to new_signup_completion_url + else + render plain: "Authentication failed. This is probably a bug.", status: :unauthorized + end + end +end diff --git a/app/models/launchpad.rb b/app/models/launchpad.rb index 8b437c809..6001dac82 100644 --- a/app/models/launchpad.rb +++ b/app/models/launchpad.rb @@ -8,6 +8,10 @@ module Launchpad url product_account_path("/signin", product: product, account: account), params end + def authentication_url(**params) + url "/authenticate", params.merge(product: :fizzy) + end + def product_account_path(path = nil, product: false, account: nil) product_path = "/fizzy" if product || account account_path = "/#{account.signal_account.id}" if account diff --git a/app/models/signup.rb b/app/models/signup.rb index 40996a1f4..58cdebcc3 100644 --- a/app/models/signup.rb +++ b/app/models/signup.rb @@ -3,6 +3,8 @@ class Signup include ActiveModel::Attributes include ActiveModel::Validations + PERMITTED_KEYS = %i[ full_name email_address password company_name ] + # Input attribute attr_accessor :company_name @@ -18,6 +20,10 @@ class Signup validate :validate_new_identity def initialize(...) + @full_name = nil + @email_address = nil + @password = nil + @company_name = nil @signal_identity = nil @queenbee_account = nil @account = nil @@ -44,10 +50,22 @@ class Signup false end + def recognized? + SignalId::Identity.find_by_email_address(email_address).present? + end + def tenant_name @tenant_name ||= signal_account.subdomain end + def to_h + { + full_name: full_name, + email_address: email_address, + company_name: company_name + }.compact_blank + end + private def create_signal_identity unless signal_identity.present? diff --git a/app/views/signup/completions/new.html.erb b/app/views/signup/completions/new.html.erb new file mode 100644 index 000000000..2fce66a3c --- /dev/null +++ b/app/views/signup/completions/new.html.erb @@ -0,0 +1 @@ +<%= auto_submit_form_with url: signup_completions_path, data: { turbo: false } %> diff --git a/config/routes.rb b/config/routes.rb index ad780b0da..ae77af74a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -91,6 +91,8 @@ Rails.application.routes.draw do namespace :signup do get "/" => "accounts#new" resources :accounts, only: %i[ new create ] + get "/session" => "sessions#create" # redirect from Launchpad after mid-signup authentication + resources :completions, only: %i[ new create ] end resources :users do diff --git a/test/controllers/signup/accounts_controller_test.rb b/test/controllers/signup/accounts_controller_test.rb index f7c2a6ad7..5e1eff360 100644 --- a/test/controllers/signup/accounts_controller_test.rb +++ b/test/controllers/signup/accounts_controller_test.rb @@ -36,4 +36,43 @@ class Signup::AccountsControllerTest < ActionDispatch::IntegrationTest signal_account = SignalId::Account.last assert_redirected_to(/#{signal_account.login_url}/) end + + test "create for an existing identity" do + integration_session.host = "example.com" # no subdomain + + identity = signal_identities(:david) + + post signup_accounts_url, params: { signup: { email_address: identity.email_address, company_name: "signup-accounts-controller-test-2" } } + assert_authentication_requested_for identity + + assert_no_difference -> { SignalId::Identity.count } do + assert_difference -> { SignalId::Account.count } do + authenticate_via_launchpad_as(identity) + assert_redirected_to_account + assert_nil session[:signup] + end + end + + signal_account = SignalId::Account.last + ApplicationRecord.with_tenant(signal_account.subdomain) do + assert_equal Account.last, signal_account.peer + end + end + + def assert_authentication_requested_for(identity) + assert_response :redirect + assert_match(/#{Regexp.escape(Launchpad.url("/authenticate", login_hint: identity.email_address))}.*&purpose=signup/, redirect_to_url) + end + + def authenticate_via_launchpad_as(identity) + get signup_session_url, params: { sig: identity.perishable_signature } + assert_equal identity.id, session[:signup]["identity_id"] + assert_redirected_to new_signup_completion_url + post signup_completions_url + end + + def assert_redirected_to_account(signal_account = SignalId::Account.last) + assert_response :redirect + assert_match(/#{Regexp.escape(signal_account.url("/session/launchpad"))}.*&sig=/, redirect_to_url) + end end diff --git a/test/models/signup_test.rb b/test/models/signup_test.rb index 0dfd2bd87..fc9f89d7a 100644 --- a/test/models/signup_test.rb +++ b/test/models/signup_test.rb @@ -10,6 +10,16 @@ class SignupTest < ActiveSupport::TestCase ) end + test "#to_h allows persistence of the signup data" do + actual = @signup.to_h + expected = { + email_address: @signup.email_address, + full_name: @signup.full_name, + company_name: @signup.company_name + } + assert_equal expected, actual + end + test "#process creates all the necessary objects for a new identity" do Account.any_instance.expects(:setup_basic_template).once