From 1a1f4a077b5306427d8c8375c8ba6ca1a9769040 Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Tue, 16 Dec 2025 18:23:04 +0100 Subject: [PATCH] Simplify auth logic --- app/controllers/concerns/authentication.rb | 45 ++++++++++--------- .../sessions/magic_links_controller.rb | 15 ++++++- app/controllers/sessions_controller.rb | 30 +++++-------- test/controllers/magic_link_api_test.rb | 2 +- test/controllers/sessions_controller_test.rb | 3 +- 5 files changed, 50 insertions(+), 45 deletions(-) diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index dbea14ada..aa3563e90 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -108,36 +108,39 @@ module Authentication end end - def email_address_pending_authentication_matches?(email_address) - if ActiveSupport::SecurityUtils.secure_compare(email_address, email_address_pending_authentication || "") - session.delete(:email_address_pending_authentication) - true - else - false - end - end - def email_address_pending_authentication - if request.format.json? - verified_pending_authentication_token - else - session[:email_address_pending_authentication] - end + pending_authentication_token_verifier.verified(pending_authentication_token) end - def pending_authentication_token_for(email_address) - Rails.application.message_verifier(:pending_authentication).generate(email_address, expires_in: 10.minutes) + def pending_authentication_token_verifier + Rails.application.message_verifier(:pending_authentication) end - def verified_pending_authentication_token - Rails.application.message_verifier(:pending_authentication).verified(params[:pending_authentication_token]) + def pending_authentication_token + cookies[:pending_authentication_token] end - def redirect_to_session_magic_link(magic_link, return_to: nil) + def clear_pending_authentication_token + cookies.delete(:pending_authentication_token) + end + + def redirect_to_session_magic_link(magic_link, email_address: magic_link&.identity&.email_address, return_to: nil) + expires_at = magic_link&.expires_at || MagicLink::EXPIRATION_TIME.from_now + + cookies[:pending_authentication_token] = { + value: pending_authentication_token_verifier.generate(email_address, expires_at: expires_at), + httponly: true, + same_site: :lax, + expires: expires_at + } + serve_development_magic_link(magic_link) - session[:email_address_pending_authentication] = magic_link.identity.email_address if magic_link session[:return_to_after_authenticating] = return_to if return_to - redirect_to main_app.session_magic_link_path(script_name: nil) + + respond_to do |format| + format.html { redirect_to main_app.session_magic_link_url(script_name: nil) } + format.json { render json: { pending_authentication_token: pending_authentication_token }, status: :created } + end end def serve_development_magic_link(magic_link) diff --git a/app/controllers/sessions/magic_links_controller.rb b/app/controllers/sessions/magic_links_controller.rb index d83e7f807..38d1350bb 100644 --- a/app/controllers/sessions/magic_links_controller.rb +++ b/app/controllers/sessions/magic_links_controller.rb @@ -33,7 +33,8 @@ class Sessions::MagicLinksController < ApplicationController end def respond_to_valid_code_from(magic_link) - if email_address_pending_authentication_matches?(magic_link.identity.email_address) + if ActiveSupport::SecurityUtils.secure_compare(email_address_pending_authentication || "", magic_link.identity.email_address) + clear_pending_authentication_token start_new_session_for magic_link.identity respond_to do |format| @@ -41,7 +42,9 @@ class Sessions::MagicLinksController < ApplicationController format.json { render json: { session_token: cookies[:session_token] } } end else - alert_message = "Authentication failed. Please try again." + clear_pending_authentication_token + alert_message = "Something went wrong. Please try again." + respond_to do |format| format.html { redirect_to new_session_path, alert: alert_message } format.json { render json: { message: alert_message }, status: :unauthorized } @@ -56,6 +59,14 @@ class Sessions::MagicLinksController < ApplicationController end end + def pending_authentication_token + if request.format.json? + params[:pending_authentication_token] + else + super + end + end + def after_sign_in_url(magic_link) if magic_link.for_sign_up? new_signup_completion_path diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 1781b40bc..29d795fb2 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -11,29 +11,19 @@ class SessionsController < ApplicationController def create if identity = Identity.find_by_email_address(email_address) magic_link = identity.send_magic_link - else + elsif Account.accepting_signups? signup = Signup.new(email_address: email_address) - if signup.valid?(:identity_creation) - magic_link = signup.create_identity if Account.accepting_signups? - else - head :unprocessable_entity - return - end + magic_link = signup.create_identity if signup.valid?(:identity_creation) end - serve_development_magic_link magic_link - - respond_to do |format| - format.html do - if magic_link.present? - redirect_to_session_magic_link magic_link - else - redirect_to session_magic_link_path - end - end - - format.json do - render json: { pending_authentication_token: pending_authentication_token_for(email_address) }, status: :created + if magic_link + redirect_to_session_magic_link magic_link + elsif !Account.accepting_signups? + redirect_to_session_magic_link nil, email_address: email_address + else + respond_to do |format| + format.html { redirect_to new_session_path, alert: "Something went wrong" } + format.json { render json: { message: "Something went wrong" }, status: :unprocessable_entity } end end end diff --git a/test/controllers/magic_link_api_test.rb b/test/controllers/magic_link_api_test.rb index a1d494d48..4b7f7f444 100644 --- a/test/controllers/magic_link_api_test.rb +++ b/test/controllers/magic_link_api_test.rb @@ -67,7 +67,7 @@ class MagicLinkApiTest < ActionDispatch::IntegrationTest untenanted do post session_magic_link_path(format: :json), params: { code: magic_link.code, pending_authentication_token: pending_token } assert_response :unauthorized - assert_equal "Authentication failed. Please try again.", @response.parsed_body["message"] + assert_equal "Something went wrong. Please try again.", @response.parsed_body["message"] end end diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 74b336a4f..07c58ed74 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -70,7 +70,8 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest post session_path, params: { email_address: "not-a-valid-email" } end - assert_response :unprocessable_entity + assert_response :redirect + assert_redirected_to new_session_path end end end