Pin sign in attempts to the current session
This makes it way more difficult to brute-force a magic link. Original implementation by udiudi. Ref: https://github.com/udiudi/fizzy/pull/1/files Discussion: https://github.com/basecamp/fizzy/discussions/1981 Co-Authored-By: Udi <udi@udi.codes>
This commit is contained in:
@@ -107,8 +107,20 @@ module Authentication
|
||||
end
|
||||
end
|
||||
|
||||
def email_address_pending_authentication_matches?(email_address)
|
||||
pending_email_address = session.fetch(:email_address_pending_authentication, "")
|
||||
|
||||
if ActiveSupport::SecurityUtils.secure_compare(email_address, pending_email_address)
|
||||
session.delete(:email_address_pending_authentication)
|
||||
true
|
||||
else
|
||||
false
|
||||
end
|
||||
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 session_magic_link_url(script_name: nil)
|
||||
end
|
||||
|
||||
@@ -10,14 +10,22 @@ 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 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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -14,6 +14,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 +29,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 +39,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)
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user