From 898a3882c58ed4a1a7e01682052b2078cae7ffd8 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Sat, 28 Oct 2023 12:00:57 +0100 Subject: [PATCH] Avoid storing user records in the session during signup This works around an issue with rails failing to preserve attribute change flags and is in line with upstream advice against storing models in the session in this way. https://github.com/rails/rails/issues/49826 https://github.com/rails/rails/issues/49827 --- .rubocop_todo.yml | 2 +- app/controllers/users_controller.rb | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index db94a610b8..6f25cfeb33 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -81,7 +81,7 @@ Metrics/ParameterLists: # Offense count: 56 # Configuration parameters: AllowedMethods, AllowedPatterns. Metrics/PerceivedComplexity: - Max: 27 + Max: 29 # Offense count: 2394 # This cop supports safe autocorrection (--autocorrect). diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5ba1b702bf..36c9f4e228 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -104,11 +104,11 @@ def create render :action => "new" elsif current_user.auth_provider.present? # Verify external authenticator before moving on - session[:new_user] = current_user + session[:new_user] = current_user.attributes.slice("email", "display_name", "pass_crypt") redirect_to auth_url(current_user.auth_provider, current_user.auth_uid), :status => :temporary_redirect else # Save the user record - session[:new_user] = current_user + session[:new_user] = current_user.attributes.slice("email", "display_name", "pass_crypt") redirect_to :action => :terms end end @@ -170,7 +170,10 @@ def save redirect_to referer || edit_account_path else - self.current_user = session.delete(:new_user) + new_user = session.delete(:new_user) + verified_email = new_user.delete("verified_email") + + self.current_user = User.new(new_user) if check_signup_allowed(current_user.email) current_user.data_public = true @@ -184,6 +187,8 @@ def save if current_user.auth_uid.blank? current_user.auth_provider = nil current_user.auth_uid = nil + elsif current_user.email == verified_email + current_user.activate end if current_user.save @@ -272,10 +277,9 @@ def auth_success redirect_to edit_account_path elsif session[:new_user] - session[:new_user].auth_provider = provider - session[:new_user].auth_uid = uid - - session[:new_user].activate if email_verified && email == session[:new_user].email + session[:new_user]["auth_provider"] = provider + session[:new_user]["auth_uid"] = uid + session[:new_user]["verified_email"] = email if email_verified redirect_to :action => "terms" else