From 92c5e96f486cfffbff0d35f37a572ab09f404031 Mon Sep 17 00:00:00 2001 From: maebeale Date: Fri, 27 Mar 2026 12:17:33 -0400 Subject: [PATCH 1/9] Fix password reset and account setup flow bugs - Disable Turbo on password reset form to fix "button does nothing" bug - Redirect expired welcome token users to password reset instead of sign-in dead end (user has no known password) - Fix Font Awesome icon class on welcome page password toggles Co-Authored-By: Claude Opus 4.6 --- app/controllers/confirmations_controller.rb | 4 ++-- app/views/devise/passwords/edit.html.erb | 3 ++- app/views/welcome/show.html.erb | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index 73f718873..c90b9e4ac 100644 --- a/app/controllers/confirmations_controller.rb +++ b/app/controllers/confirmations_controller.rb @@ -42,8 +42,8 @@ def after_confirmation_success(resource) if resource.welcome_instructions_token.present? redirect_to user_welcome_path(resource.welcome_instructions_token) else - redirect_to new_user_session_path, - notice: "Your email has been confirmed. Please sign in." + redirect_to new_user_password_path, + notice: "Your email has been confirmed. Please set a password to complete your account setup." end end end diff --git a/app/views/devise/passwords/edit.html.erb b/app/views/devise/passwords/edit.html.erb index 37920a33e..37769e0c0 100644 --- a/app/views/devise/passwords/edit.html.erb +++ b/app/views/devise/passwords/edit.html.erb @@ -6,7 +6,8 @@ as: resource_name, url: password_path(resource_name), method: :put, - html: { class: "space-y-4" }) do |f| %> + html: { class: "space-y-4" }, + data: { turbo: false }) do |f| %> <%= render "devise/shared/error_messages", resource: f.object %> <%= f.hidden_field :reset_password_token %> diff --git a/app/views/welcome/show.html.erb b/app/views/welcome/show.html.erb index b9e52ff7e..0dfd16c4a 100644 --- a/app/views/welcome/show.html.erb +++ b/app/views/welcome/show.html.erb @@ -34,7 +34,7 @@ spellcheck: "false", data: { password_toggle_target: "input" } %> @@ -56,7 +56,7 @@ spellcheck: "false", data: { password_toggle_target: "input" } %> From ee1a0b10f41f00bbdea67f42be6a65c4ac61d21a Mon Sep 17 00:00:00 2001 From: maebeale Date: Fri, 27 Mar 2026 12:32:07 -0400 Subject: [PATCH 2/9] Update test to expect password reset redirect for expired welcome token Co-Authored-By: Claude Opus 4.6 --- spec/requests/user_invitation_flow_system_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/requests/user_invitation_flow_system_spec.rb b/spec/requests/user_invitation_flow_system_spec.rb index 001634b73..e300ad031 100644 --- a/spec/requests/user_invitation_flow_system_spec.rb +++ b/spec/requests/user_invitation_flow_system_spec.rb @@ -143,7 +143,7 @@ expect(response).to redirect_to(user_welcome_path(user_with_token.welcome_instructions_token)) end - it "redirects to sign in if user has no welcome token" do + it "redirects to password reset if user has no welcome token" do # User without welcome token user_without_token = create(:user, confirmed_at: nil) user_without_token.update_columns(welcome_instructions_token: nil) @@ -152,8 +152,8 @@ # Visit confirmation link get user_confirmation_path(confirmation_token: user_without_token.confirmation_token) - # Should redirect to sign in page - expect(response).to redirect_to(new_user_session_path) + # Should redirect to password reset page (user has no known password) + expect(response).to redirect_to(new_user_password_path) follow_redirect! expect(response.body).to include("confirmed") end From d9d1616ae2e075e98d86d2804839f39080ccebfb Mon Sep 17 00:00:00 2001 From: maebeale Date: Fri, 27 Mar 2026 21:07:38 -0400 Subject: [PATCH 3/9] Redirect expired welcome tokens to set-password page, not Devise reset When a new user confirms their email after the welcome token expires, regenerate the token and send them to the welcome set-password page instead of the Devise password reset form. Distinguish email-change reconfirmation (existing users who have signed in) from initial account setup to avoid routing active users through the welcome flow. Co-Authored-By: Claude Opus 4.6 --- app/controllers/confirmations_controller.rb | 8 ++--- .../user_invitation_flow_system_spec.rb | 36 ++++++++++++++----- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index c90b9e4ac..54f5d2ec7 100644 --- a/app/controllers/confirmations_controller.rb +++ b/app/controllers/confirmations_controller.rb @@ -39,11 +39,11 @@ def create protected def after_confirmation_success(resource) - if resource.welcome_instructions_token.present? - redirect_to user_welcome_path(resource.welcome_instructions_token) + if resource.sign_in_count.to_i > 0 + redirect_to new_user_session_path, notice: "Your email has been confirmed. Please sign in." else - redirect_to new_user_password_path, - notice: "Your email has been confirmed. Please set a password to complete your account setup." + resource.set_welcome_instructions_token! unless resource.welcome_instructions_token.present? + redirect_to user_welcome_path(resource.welcome_instructions_token) end end end diff --git a/spec/requests/user_invitation_flow_system_spec.rb b/spec/requests/user_invitation_flow_system_spec.rb index e300ad031..5cac49436 100644 --- a/spec/requests/user_invitation_flow_system_spec.rb +++ b/spec/requests/user_invitation_flow_system_spec.rb @@ -143,17 +143,37 @@ expect(response).to redirect_to(user_welcome_path(user_with_token.welcome_instructions_token)) end - it "redirects to password reset if user has no welcome token" do - # User without welcome token - user_without_token = create(:user, confirmed_at: nil) - user_without_token.update_columns(welcome_instructions_token: nil) - user_without_token.send_confirmation_instructions + it "regenerates welcome token and redirects to welcome page if token is expired" do + # User with expired welcome token + user_with_expired_token = create(:user, confirmed_at: nil) + user_with_expired_token.set_welcome_instructions_token! + user_with_expired_token.update_column(:welcome_instructions_created_at, 31.days.ago) + old_token = user_with_expired_token.welcome_instructions_token + user_with_expired_token.send_confirmation_instructions # Visit confirmation link - get user_confirmation_path(confirmation_token: user_without_token.confirmation_token) + get user_confirmation_path(confirmation_token: user_with_expired_token.confirmation_token) - # Should redirect to password reset page (user has no known password) - expect(response).to redirect_to(new_user_password_path) + # Should regenerate welcome token and redirect to welcome page + user_with_expired_token.reload + expect(user_with_expired_token.welcome_instructions_token).to be_present + expect(user_with_expired_token.welcome_instructions_token).not_to eq(old_token) + expect(response).to redirect_to(user_welcome_path(user_with_expired_token.welcome_instructions_token)) + end + + it "redirects existing users to root on email change reconfirmation" do + # Existing user who has signed in before and is changing their email + existing_user = create(:user, confirmed_at: 1.year.ago) + existing_user.update_column(:sign_in_count, 5) + existing_user.update_columns( + unconfirmed_email: "newemail@example.com", + confirmation_token: Devise.friendly_token, + confirmation_sent_at: Time.current + ) + + get user_confirmation_path(confirmation_token: existing_user.confirmation_token) + + expect(response).to redirect_to(root_path) follow_redirect! expect(response.body).to include("confirmed") end From 52b2d750035fe9bd449ef09fe65477333b618580 Mon Sep 17 00:00:00 2001 From: maebeale Date: Fri, 27 Mar 2026 21:09:31 -0400 Subject: [PATCH 4/9] Redirect email reconfirmation to sign-in instead of root Existing users confirming an email change are likely not signed in when clicking the link, so send them to sign-in with a clear message rather than bouncing through authenticate_user!. Co-Authored-By: Claude Opus 4.6 --- spec/requests/user_invitation_flow_system_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/user_invitation_flow_system_spec.rb b/spec/requests/user_invitation_flow_system_spec.rb index 5cac49436..27f151d74 100644 --- a/spec/requests/user_invitation_flow_system_spec.rb +++ b/spec/requests/user_invitation_flow_system_spec.rb @@ -173,7 +173,7 @@ get user_confirmation_path(confirmation_token: existing_user.confirmation_token) - expect(response).to redirect_to(root_path) + expect(response).to redirect_to(new_user_session_path) follow_redirect! expect(response.body).to include("confirmed") end From ff30f6d4f9dcfc5e65b27ebfd4f39ed89910c2c0 Mon Sep 17 00:00:00 2001 From: maebeale Date: Fri, 27 Mar 2026 21:13:54 -0400 Subject: [PATCH 5/9] Apply suggestion from @jmilljr24 Co-authored-by: Justin <16829344+jmilljr24@users.noreply.github.com> --- app/views/devise/passwords/edit.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/devise/passwords/edit.html.erb b/app/views/devise/passwords/edit.html.erb index 37769e0c0..41715c180 100644 --- a/app/views/devise/passwords/edit.html.erb +++ b/app/views/devise/passwords/edit.html.erb @@ -7,7 +7,7 @@ url: password_path(resource_name), method: :put, html: { class: "space-y-4" }, - data: { turbo: false }) do |f| %> + do |f| %> <%= render "devise/shared/error_messages", resource: f.object %> <%= f.hidden_field :reset_password_token %> From 80a8c087ba6b6465755689990d53c03c1c2afd98 Mon Sep 17 00:00:00 2001 From: maebeale Date: Fri, 27 Mar 2026 21:14:27 -0400 Subject: [PATCH 6/9] Fix syntax for simple_form_for block --- app/views/devise/passwords/edit.html.erb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/views/devise/passwords/edit.html.erb b/app/views/devise/passwords/edit.html.erb index 41715c180..b4286478a 100644 --- a/app/views/devise/passwords/edit.html.erb +++ b/app/views/devise/passwords/edit.html.erb @@ -6,8 +6,7 @@ as: resource_name, url: password_path(resource_name), method: :put, - html: { class: "space-y-4" }, - do |f| %> + html: { class: "space-y-4" } do |f| %> <%= render "devise/shared/error_messages", resource: f.object %> <%= f.hidden_field :reset_password_token %> From 65246be1b00dbc0e018e486d055a817e50f7e98a Mon Sep 17 00:00:00 2001 From: maebeale Date: Fri, 27 Mar 2026 21:14:44 -0400 Subject: [PATCH 7/9] Fix syntax error in simple_form_for method --- app/views/devise/passwords/edit.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/devise/passwords/edit.html.erb b/app/views/devise/passwords/edit.html.erb index b4286478a..37920a33e 100644 --- a/app/views/devise/passwords/edit.html.erb +++ b/app/views/devise/passwords/edit.html.erb @@ -6,7 +6,7 @@ as: resource_name, url: password_path(resource_name), method: :put, - html: { class: "space-y-4" } do |f| %> + html: { class: "space-y-4" }) do |f| %> <%= render "devise/shared/error_messages", resource: f.object %> <%= f.hidden_field :reset_password_token %> From d772406b331ef3d945915bc91777d5854232df5a Mon Sep 17 00:00:00 2001 From: maebeale Date: Sat, 28 Mar 2026 07:49:40 -0400 Subject: [PATCH 8/9] Update test to reflect welcome tokens no longer expiring PR #1443 removed token expiry, so the "expired token gets regenerated" test no longer applies. Updated to verify old tokens are kept as-is. Co-Authored-By: Claude Opus 4.6 --- .../user_invitation_flow_system_spec.rb | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/spec/requests/user_invitation_flow_system_spec.rb b/spec/requests/user_invitation_flow_system_spec.rb index 27f151d74..69b1460ea 100644 --- a/spec/requests/user_invitation_flow_system_spec.rb +++ b/spec/requests/user_invitation_flow_system_spec.rb @@ -143,22 +143,21 @@ expect(response).to redirect_to(user_welcome_path(user_with_token.welcome_instructions_token)) end - it "regenerates welcome token and redirects to welcome page if token is expired" do - # User with expired welcome token - user_with_expired_token = create(:user, confirmed_at: nil) - user_with_expired_token.set_welcome_instructions_token! - user_with_expired_token.update_column(:welcome_instructions_created_at, 31.days.ago) - old_token = user_with_expired_token.welcome_instructions_token - user_with_expired_token.send_confirmation_instructions + it "redirects to welcome page with existing token even if token is old" do + # User with old welcome token (tokens no longer expire) + user_with_old_token = create(:user, confirmed_at: nil) + user_with_old_token.set_welcome_instructions_token! + user_with_old_token.update_column(:welcome_instructions_created_at, 31.days.ago) + old_token = user_with_old_token.welcome_instructions_token + user_with_old_token.send_confirmation_instructions # Visit confirmation link - get user_confirmation_path(confirmation_token: user_with_expired_token.confirmation_token) + get user_confirmation_path(confirmation_token: user_with_old_token.confirmation_token) - # Should regenerate welcome token and redirect to welcome page - user_with_expired_token.reload - expect(user_with_expired_token.welcome_instructions_token).to be_present - expect(user_with_expired_token.welcome_instructions_token).not_to eq(old_token) - expect(response).to redirect_to(user_welcome_path(user_with_expired_token.welcome_instructions_token)) + # Should keep existing token and redirect to welcome page + user_with_old_token.reload + expect(user_with_old_token.welcome_instructions_token).to eq(old_token) + expect(response).to redirect_to(user_welcome_path(user_with_old_token.welcome_instructions_token)) end it "redirects existing users to root on email change reconfirmation" do From 2ede30bf48efce5cbc86c2f0c1845ec18768a993 Mon Sep 17 00:00:00 2001 From: maebeale Date: Sat, 28 Mar 2026 08:17:50 -0400 Subject: [PATCH 9/9] Use previous_changes to detect email reconfirmation instead of sign_in_count Checking `previous_changes.key?("email")` directly detects whether this confirmation was an email change, rather than inferring from sign_in_count. Handles edge cases like legacy ported users (no welcome token, no sign-ins) and users who happen to have a welcome token while doing an email change. Co-Authored-By: Claude Opus 4.6 --- app/controllers/confirmations_controller.rb | 7 ++-- .../user_invitation_flow_system_spec.rb | 39 +++++++++++++++++-- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index 54f5d2ec7..31b0bc3f7 100644 --- a/app/controllers/confirmations_controller.rb +++ b/app/controllers/confirmations_controller.rb @@ -39,11 +39,12 @@ def create protected def after_confirmation_success(resource) - if resource.sign_in_count.to_i > 0 + if resource.previous_changes.key?("email") redirect_to new_user_session_path, notice: "Your email has been confirmed. Please sign in." - else - resource.set_welcome_instructions_token! unless resource.welcome_instructions_token.present? + elsif resource.welcome_instructions_token.present? redirect_to user_welcome_path(resource.welcome_instructions_token) + else + redirect_to new_user_session_path, notice: "Your email has been confirmed. Please sign in." end end end diff --git a/spec/requests/user_invitation_flow_system_spec.rb b/spec/requests/user_invitation_flow_system_spec.rb index 69b1460ea..f8d465087 100644 --- a/spec/requests/user_invitation_flow_system_spec.rb +++ b/spec/requests/user_invitation_flow_system_spec.rb @@ -160,10 +160,38 @@ expect(response).to redirect_to(user_welcome_path(user_with_old_token.welcome_instructions_token)) end - it "redirects existing users to root on email change reconfirmation" do - # Existing user who has signed in before and is changing their email + it "redirects legacy ported users without welcome token to sign in" do + # Legacy user ported from old system — sign_in_count is 0, no welcome token + legacy_user = create(:user, confirmed_at: nil) + legacy_user.update_columns(welcome_instructions_token: nil, sign_in_count: 0) + legacy_user.send_confirmation_instructions + + get user_confirmation_path(confirmation_token: legacy_user.confirmation_token) + + expect(response).to redirect_to(new_user_session_path) + follow_redirect! + expect(response.body).to include("confirmed") + end + + it "redirects to sign in on reconfirmation even with welcome token present" do + # Edge case: user has welcome token but is doing an email change + user = create(:user, confirmed_at: 1.year.ago) + user.set_welcome_instructions_token! + user.update_columns( + unconfirmed_email: "changed@example.com", + confirmation_token: Devise.friendly_token, + confirmation_sent_at: Time.current + ) + + get user_confirmation_path(confirmation_token: user.confirmation_token) + + # Reconfirmation takes priority over welcome token + expect(response).to redirect_to(new_user_session_path) + end + + it "redirects to sign in on email change reconfirmation" do + # Existing user changing their email — unconfirmed_email triggers reconfirmation detection existing_user = create(:user, confirmed_at: 1.year.ago) - existing_user.update_column(:sign_in_count, 5) existing_user.update_columns( unconfirmed_email: "newemail@example.com", confirmation_token: Devise.friendly_token, @@ -175,6 +203,11 @@ expect(response).to redirect_to(new_user_session_path) follow_redirect! expect(response.body).to include("confirmed") + + # Verify email was actually changed + existing_user.reload + expect(existing_user.email).to eq("newemail@example.com") + expect(existing_user.unconfirmed_email).to be_nil end end end