diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index 1b057a19d..08129d9c7 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -6,6 +6,7 @@ module Authentication before_action :require_authentication after_action :ensure_development_magic_link_not_leaked helper_method :authenticated? + helper_method :email_address_pending_authentication etag { Current.identity.id if authenticated? } @@ -107,8 +108,22 @@ 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 + session[:email_address_pending_authentication] + end + def redirect_to_session_magic_link(magic_link, return_to: nil) 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_url(script_name: nil) end diff --git a/app/controllers/sessions/magic_links_controller.rb b/app/controllers/sessions/magic_links_controller.rb index 71fa1b9ac..c0632407a 100644 --- a/app/controllers/sessions/magic_links_controller.rb +++ b/app/controllers/sessions/magic_links_controller.rb @@ -2,6 +2,7 @@ class Sessions::MagicLinksController < ApplicationController disallow_account_scope require_unauthenticated_access rate_limit to: 10, within: 15.minutes, only: :create, with: -> { redirect_to session_magic_link_path, alert: "Wait 15 minutes, then try again" } + before_action :ensure_that_email_address_pending_authentication_exists layout "public" @@ -10,14 +11,28 @@ class Sessions::MagicLinksController < ApplicationController def create if magic_link = MagicLink.consume(code) - start_new_session_for magic_link.identity - redirect_to after_sign_in_url(magic_link) + authenticate_with magic_link else redirect_to session_magic_link_path, flash: { shake: true } end end private + def ensure_that_email_address_pending_authentication_exists + unless email_address_pending_authentication.present? + redirect_to new_session_path, alert: "Enter your email address to sign in." + end + end + + def authenticate_with(magic_link) + if email_address_pending_authentication_matches?(magic_link.identity.email_address) + start_new_session_for magic_link.identity + redirect_to after_sign_in_url(magic_link) + else + redirect_to new_session_path, alert: "Authentication failed. Please try again." + end + end + def code params.expect(:code) end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index e32cf3829..f5044bd2a 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -14,8 +14,8 @@ class SessionsController < ApplicationController else signup = Signup.new(email_address: email_address) if signup.valid?(:identity_creation) - identity = signup.create_identity if Account.accepting_signups? - redirect_to_session_magic_link identity + magic_link = signup.create_identity if Account.accepting_signups? + redirect_to_session_magic_link magic_link else head :unprocessable_entity end diff --git a/app/views/sessions/magic_links/show.html.erb b/app/views/sessions/magic_links/show.html.erb index 947b3c426..26a28da9d 100644 --- a/app/views/sessions/magic_links/show.html.erb +++ b/app/views/sessions/magic_links/show.html.erb @@ -15,7 +15,7 @@ data: { magic_link_target: "input", action: "keydown.enter->magic-link#submitOnEnter paste->magic-link#submitOnPaste" } %> <% end %> -
The code you receive will work for <%= distance_of_time_in_words(MagicLink::EXPIRATION_TIME) %>.
+The code sent to <%= email_address_pending_authentication %> will work for <%= distance_of_time_in_words(MagicLink::EXPIRATION_TIME) %>.
<% if Rails.env.development? && flash[:magic_link_code].present? %> diff --git a/test/controllers/sessions/magic_links_controller_test.rb b/test/controllers/sessions/magic_links_controller_test.rb index dbde3723e..5a4a48e51 100644 --- a/test/controllers/sessions/magic_links_controller_test.rb +++ b/test/controllers/sessions/magic_links_controller_test.rb @@ -5,6 +5,14 @@ class Sessions::MagicLinksControllerTest < ActionDispatch::IntegrationTest untenanted do get session_magic_link_url + assert_response :redirect, "Without an email address pending authentication, should redirect" + assert_redirected_to new_session_path + end + + untenanted do + post session_path, params: { email_address: "test@example.com" } + get session_magic_link_url + assert_response :success end end @@ -14,6 +22,7 @@ class Sessions::MagicLinksControllerTest < ActionDispatch::IntegrationTest magic_link = MagicLink.create!(identity: identity) untenanted do + post session_path, params: { email_address: identity.email_address } post session_magic_link_url, params: { code: magic_link.code } assert_response :redirect @@ -28,6 +37,7 @@ class Sessions::MagicLinksControllerTest < ActionDispatch::IntegrationTest magic_link = MagicLink.create!(identity: identity, purpose: :sign_up) untenanted do + post session_path, params: { email_address: identity.email_address } post session_magic_link_url, params: { code: magic_link.code } assert_response :redirect @@ -37,6 +47,20 @@ class Sessions::MagicLinksControllerTest < ActionDispatch::IntegrationTest end end + test "create with cross-user code" do + identity = identities(:kevin) + other_identity = identities(:jason) + magic_link = MagicLink.create!(identity: other_identity) + + untenanted do + post session_path, params: { email_address: identity.email_address } + post session_magic_link_url, params: { code: magic_link.code } + + assert_redirected_to new_session_path + assert_not cookies[:session_token].present? + end + end + test "create with invalid code" do identity = identities(:kevin) magic_link = MagicLink.create!(identity: identity) diff --git a/test/test_helpers/session_test_helper.rb b/test/test_helpers/session_test_helper.rb index 57d697f75..d07d6612a 100644 --- a/test/test_helpers/session_test_helper.rb +++ b/test/test_helpers/session_test_helper.rb @@ -18,6 +18,7 @@ module SessionTestHelper magic_link = identity.magic_links.order(id: :desc).first untenanted do + post session_path, params: { email_address: identity.email_address } post session_magic_link_url, params: { code: magic_link.code } end