diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index 751f0c2..85633da 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -53,6 +53,6 @@ def user_signed_in? end def store_location - session[:user_return_to] = request.original_url if request.get? && request.local? + session[:user_return_to] = request.original_url if request.get? end end diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index a65fc68..5656ed0 100644 --- a/app/controllers/confirmations_controller.rb +++ b/app/controllers/confirmations_controller.rb @@ -13,7 +13,16 @@ def create end def edit - @user = User.find_signed(params[:confirmation_token], purpose: :confirm_email) + expected_email = params[:email].strip.downcase + + # It's possible that the email address that we're confirming is the + # primary "email" field or the secondary "unconfirmed_email" field. + # We need to check both fields to find the user, but will only check + # the primary field if the secondary field is null. + @user = (User.where(unconfirmed_email: expected_email). + or(User.where(email: expected_email, unconfirmed_email: nil))). + find_signed(params[:confirmation_token], + purpose: User.email_confirmation_purpose_for(expected_email)) if @user.present? && @user.unconfirmed_or_reconfirming? if @user.confirm! login @user diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index ae2c631..f022963 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -6,9 +6,10 @@ class UserMailer < ApplicationMailer # # en.user_mailer.confirmation.subject # - def confirmation(user, confirmation_token) + def confirmation(user, confirmation_token, confirmable_email) @user = user @confirmation_token = confirmation_token + @confirmable_email = confirmable_email mail to: @user.confirmable_email, subject: "Confirmation Instructions" end diff --git a/app/models/user.rb b/app/models/user.rb index 578e289..0e0c0ac 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -53,8 +53,17 @@ def confirmable_email end end + def self.email_confirmation_purpose_for(email) + "confirm_email: #{email}" + end + + def email_confirmation_purpose + self.class.email_confirmation_purpose_for(confirmable_email) + end + def generate_confirmation_token - signed_id expires_in: CONFIRMATION_TOKEN_EXPIRATION, purpose: :confirm_email + signed_id expires_in: CONFIRMATION_TOKEN_EXPIRATION, + purpose: email_confirmation_purpose end def generate_password_reset_token @@ -63,7 +72,7 @@ def generate_password_reset_token def send_confirmation_email! confirmation_token = generate_confirmation_token - UserMailer.confirmation(self, confirmation_token).deliver_now + UserMailer.confirmation(self, confirmation_token, confirmable_email).deliver_now end def send_password_reset_email! diff --git a/app/views/user_mailer/confirmation.html.erb b/app/views/user_mailer/confirmation.html.erb index d064deb..b04c78a 100644 --- a/app/views/user_mailer/confirmation.html.erb +++ b/app/views/user_mailer/confirmation.html.erb @@ -1,3 +1,3 @@

Confirmation Instructions

-<%= link_to "Click here to confirm your email.", edit_confirmation_url(@confirmation_token) %> \ No newline at end of file +<%= link_to "Click here to confirm your email.", edit_confirmation_url(@confirmation_token, email: @confirmable_email) %> diff --git a/app/views/user_mailer/confirmation.text.erb b/app/views/user_mailer/confirmation.text.erb index a34b236..54bee56 100644 --- a/app/views/user_mailer/confirmation.text.erb +++ b/app/views/user_mailer/confirmation.text.erb @@ -1,3 +1,3 @@ Confirmation Instructions -<%= edit_confirmation_url(@confirmation_token) %> \ No newline at end of file +<%= edit_confirmation_url(@confirmation_token, email: @confirmable_email) %> diff --git a/test/controllers/confirmations_controller_test.rb b/test/controllers/confirmations_controller_test.rb index d196308..d5a9c81 100644 --- a/test/controllers/confirmations_controller_test.rb +++ b/test/controllers/confirmations_controller_test.rb @@ -11,7 +11,7 @@ class ConfirmationsControllerTest < ActionDispatch::IntegrationTest freeze_time do confirmation_token = @unconfirmed_user.generate_confirmation_token - get edit_confirmation_path(confirmation_token) + get edit_confirmation_path(confirmation_token, email: @unconfirmed_user.confirmable_email) assert @unconfirmed_user.reload.confirmed? assert_equal Time.now, @unconfirmed_user.confirmed_at @@ -26,7 +26,7 @@ class ConfirmationsControllerTest < ActionDispatch::IntegrationTest freeze_time do confirmation_token = @reconfirmed_user.generate_confirmation_token - get edit_confirmation_path(confirmation_token) + get edit_confirmation_path(confirmation_token, email: unconfirmed_email) assert @reconfirmed_user.reload.confirmed? assert_equal Time.current, @reconfirmed_user.reload.confirmed_at @@ -44,7 +44,7 @@ class ConfirmationsControllerTest < ActionDispatch::IntegrationTest freeze_time do confirmation_token = @reconfirmed_user.generate_confirmation_token - get edit_confirmation_path(confirmation_token) + get edit_confirmation_path(confirmation_token, email: @confirmed_user.confirmable_email) assert_equal original_email, @reconfirmed_user.reload.email assert_redirected_to new_confirmation_path @@ -56,7 +56,7 @@ class ConfirmationsControllerTest < ActionDispatch::IntegrationTest confirmation_token = @unconfirmed_user.generate_confirmation_token travel_to 601.seconds.from_now do - get edit_confirmation_path(confirmation_token) + get edit_confirmation_path(confirmation_token, email: @unconfirmed_user.confirmable_email) assert_nil @unconfirmed_user.reload.confirmed_at assert_not @unconfirmed_user.reload.confirmed? @@ -66,7 +66,15 @@ class ConfirmationsControllerTest < ActionDispatch::IntegrationTest end test "should redirect if confirmation link is incorrect" do - get edit_confirmation_path("not_a_real_token") + get edit_confirmation_path("not_a_real_token", email: @unconfirmed_user.confirmable_email) + assert_redirected_to new_confirmation_path + assert_not_nil flash[:alert] + end + + test "should redirect if confirmation email is incorrect" do + confirmation_token = @unconfirmed_user.generate_confirmation_token + + get edit_confirmation_path(confirmation_token, email: "not_the_email@gmail.com") assert_redirected_to new_confirmation_path assert_not_nil flash[:alert] end @@ -99,7 +107,7 @@ class ConfirmationsControllerTest < ActionDispatch::IntegrationTest login @confirmed_user - get edit_confirmation_path(confirmation_token) + get edit_confirmation_path(confirmation_token, email: @confirmed_user.confirmable_email) assert_not_equal Time.current, @confirmed_user.reload.confirmed_at assert_redirected_to new_confirmation_path @@ -115,7 +123,7 @@ class ConfirmationsControllerTest < ActionDispatch::IntegrationTest confirmation_token = @reconfirmed_user.generate_confirmation_token - get edit_confirmation_path(confirmation_token) + get edit_confirmation_path(confirmation_token, email: unconfirmed_email) assert_equal Time.current, @reconfirmed_user.reload.confirmed_at assert @reconfirmed_user.reload.confirmed? @@ -126,6 +134,19 @@ class ConfirmationsControllerTest < ActionDispatch::IntegrationTest end end + test "should prevent reuse of confirmation token" do + login @unconfirmed_user + + confirmation_token = @unconfirmed_user.generate_confirmation_token + + get edit_confirmation_path(confirmation_token, email: @unconfirmed_user.confirmable_email) + assert @unconfirmed_user.reload.confirmed? + @unconfirmed_user.update(unconfirmed_email: "not_the_same@example.com") + + get edit_confirmation_path(confirmation_token, email: @unconfirmed_user.unconfirmed_email) + assert_redirected_to new_confirmation_path + end + test "should prevent authenticated user from submitting the confirmation form" do login @confirmed_user diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 928d560..09ceea5 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -44,6 +44,12 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest assert_not_nil current_user assert_not_nil cookies[:remember_token] + + remember_me_cookie = cookies.get_cookie("remember_token") + + assert remember_me_cookie.http_only? + assert remember_me_cookie.secure? + assert_equal "Strict", remember_me_cookie.to_h["SameSite"] end test "should forget user when logging out" do diff --git a/test/mailers/user_mailer_test.rb b/test/mailers/user_mailer_test.rb index 9f9c9e0..0439fd8 100644 --- a/test/mailers/user_mailer_test.rb +++ b/test/mailers/user_mailer_test.rb @@ -7,7 +7,7 @@ class UserMailerTest < ActionMailer::TestCase test "confirmation" do confirmation_token = @user.generate_confirmation_token - mail = UserMailer.confirmation(@user, confirmation_token) + mail = UserMailer.confirmation(@user, confirmation_token, @user.unconfirmed_email) assert_equal "Confirmation Instructions", mail.subject assert_equal [@user.email], mail.to assert_equal [User::MAILER_FROM_EMAIL], mail.from