New email new me! #151

24c02 · 2026-01-01T22:23:15Z

emails should be changeable...

Users can now change their primary email address. The flow requires: - Verification links sent to BOTH old and new email addresses - Both must be clicked before the change takes effect - Requests expire after 24 hours - IP address logging and paper_trail for audit Security measures: - Encrypted tokens with blind indexes - ValidEmail2 validation (rejects disposable emails, checks MX) - New email uniqueness check against existing identities - Activity tracking on completion

- Fix mailer route helpers (verify_old_email_changes_url) - Add server-side enforcement blocking email step-up for email_change - Extract StepUpAuthenticatable concern with consume_step_up! - Add clear_step_up! to IdentitySession (single-use step-up) - Add DB unique index on identities.primary_email - Fix race condition in complete_if_ready! with row lock - Add mailer previews and tests

Copilot

Pull request overview

This PR implements a secure email change feature for user accounts, requiring verification of both the old and new email addresses before completing the change. The implementation includes step-up authentication for users with 2FA enabled, comprehensive security measures like rate limiting and blind token indexing, and activity tracking throughout the email change process.

Key changes:

Adds a double-verification email change workflow requiring confirmation from both old and new email addresses
Integrates step-up authentication that blocks email-based verification for email changes (to prevent circular verification)
Implements security enhancements including IP tracking, unique pending request constraints, and action-specific step-up binding

Reviewed changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated 21 comments.

Show a summary per file

File	Description
db/migrate/20260101170716_add_email_change_security_enhancements.rb	Adds security columns for IP tracking and step-up action binding
db/migrate/20251212204707_create_identity_email_change_requests.rb	Creates table for email change requests with encrypted tokens
db/migrate/20251212171457_add_email_uniqueness_indexes.rb	Adds case-insensitive unique index on primary_email
db/schema.rb	Updates schema with email change tables and session step-up columns
app/models/identity/email_change_request.rb	Core model implementing double-verification workflow with validations
app/models/identity_session.rb	Adds action-specific step-up tracking methods
app/models/identity.rb	Adds association to email_change_requests
app/controllers/email_changes_controller.rb	Handles email change flow including verification endpoints
app/controllers/step_up_controller.rb	Blocks email verification method for email_change actions
app/controllers/concerns/step_up_authenticatable.rb	Adds step-up authentication concern for controllers
app/controllers/application_controller.rb	Includes step-up concern and adds paper_trail metadata
app/mailers/email_change_mailer.rb	Sends verification and notification emails
app/views/email_changes/*.html.erb	UI for initiating, monitoring, and canceling email changes
app/views/email_change_mailer/*.erb	Email templates for verification and notifications
app/views/identities/edit.html.erb	Adds "Change email" button to profile page
app/views/step_up/new.html.erb	Updates cancel path to be action-aware
app/views/public_activity/identity/*.html.erb	Activity feed snippets for email change events
config/routes.rb	Adds email_changes resource with verification routes
config/locales/en.yml	Adds translations for email change UI and emails
config/initializers/rack_attack.rb	Adds rate limiting for email change endpoints
app/frontend/stylesheets/snippets/email_changes.scss	Styles for email change UI components
spec/models/identity/email_change_request_spec.rb	Comprehensive model tests for email change workflow
spec/requests/step_up_spec.rb	Tests for step-up blocking of email method during email changes
spec/factories/identity_email_change_requests.rb	Factory definitions for testing email change requests
spec/mailers/previews/email_change_mailer_preview.rb	Preview helpers for email templates

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-01T22:29:04Z

spec/mailers/previews/email_change_mailer_preview.rb

+    Identity::EmailChangeRequest.new(
+      id: 1,
+      identity: identity,
+      old_email: identity.primary_email,
+      new_email: "[email protected]",
+      old_email_token: SecureRandom.urlsafe_base64(32),
+      new_email_token: SecureRandom.urlsafe_base64(32),
+      expires_at: 24.hours.from_now
+    )


Calling unsaved model instances in a mailer preview can lead to errors if the mailer code expects persisted records. Consider using create instead of new or ensuring the mailer handles unpersisted records gracefully.

Copilot · 2026-01-01T22:29:04Z

app/controllers/email_changes_controller.rb

+  end
+
+  def create
+    new_email = email_change_params[:new_email]&.downcase&.strip


Email normalization in the create action happens outside of the model's validation. This duplicates the normalization logic that's already in the model's normalize_emails callback. Consider removing this duplication and relying solely on the model's before_validation callback for consistency.

Suggested change

new_email = email_change_params[:new_email]&.downcase&.strip

new_email = email_change_params[:new_email]

Copilot · 2026-01-01T22:29:04Z

app/models/identity/email_change_request.rb

+  def verify_old_email!(token, verified_from_ip: nil)
+    return false unless pending?
+    return false unless ActiveSupport::SecurityUtils.secure_compare(old_email_token.to_s, token.to_s)
+
+    update!(old_email_verified_at: Time.current, old_email_verified_from_ip: verified_from_ip)
+    identity.create_activity :email_change_verified_old,
+      owner: identity,
+      recipient: identity,
+      parameters: { old_email: old_email, new_email: new_email }
+    complete_if_ready!
+    true
+  end


The verify_old_email! and verify_new_email! methods perform multiple database operations (update, create_activity, and potentially complete_if_ready!) without wrapping them in a transaction. If any operation fails after the update, the state could be inconsistent. Consider wrapping these operations in a transaction block for atomicity.

Copilot · 2026-01-01T22:29:04Z

spec/mailers/previews/email_change_mailer_preview.rb

+    request = identity.email_change_requests.first
+    return request if request


The variable name "request" shadows the controller's request method. Consider renaming this variable to something more specific like "email_change_request" or "change_request" to avoid potential confusion.

Copilot · 2026-01-01T22:29:05Z

app/views/email_change_mailer/verify_old_email.html.erb

+
+<p class="secondary-text">
+  <strong><%= t(".not_you_title") %></strong>
+  <%= t(".not_you_body_html", cancel_url: @cancel_url).html_safe %>


Using html_safe with interpolated variables (@cancel_url) in the translation could introduce XSS vulnerabilities if the URL is not properly sanitized. Consider using Rails' safe string interpolation or sanitize helpers to ensure the URL is properly escaped before marking as html_safe.

Suggested change

<%= t(".not_you_body_html", cancel_url: @cancel_url).html_safe %>

<%= sanitize(t(".not_you_body_html", cancel_url: @cancel_url)) %>

Copilot · 2026-01-01T22:29:08Z

app/frontend/stylesheets/snippets/email_changes.scss

+.email-arrow .arrow {
+  color: var(--text-muted-strong);
+  font-size: 1.25rem;
+  flex-shrink: 0;


Missing semicolon after the flex property. While not strictly required for the last property in a rule, it's best practice to include it for consistency and to avoid issues when adding new properties later.

Copilot · 2026-01-01T22:29:08Z

app/models/identity_session.rb

+  def recently_stepped_up?(for_action: nil)
+    return false unless last_step_up_at.present? && last_step_up_at > STEP_UP_DURATION.ago
+
+    # If a specific action is required, verify the step-up was for that action
+    return true if for_action.nil?
+
+    last_step_up_action == for_action.to_s
+  end
+
+  def record_step_up!(action:)
+    update!(last_step_up_at: Time.current, last_step_up_action: action.to_s)
+  end
+
+  def clear_step_up!
+    update!(last_step_up_at: nil, last_step_up_action: nil)
+  end


The public methods recently_stepped_up?, record_step_up!, and clear_step_up! lack documentation. Consider adding comments or docstrings to explain their purpose, parameters (especially the for_action parameter), and return values.

Copilot · 2026-01-01T22:29:08Z

app/controllers/application_controller.rb

@@ -18,6 +19,8 @@ def current_user = nil # TODO: this is a temp hack to fix partials until /backen

  def user_for_paper_trail = current_identity&.id



The info_for_paper_trail method provides audit trail information, but the method lacks documentation explaining its purpose and what extra_data fields are being tracked. Consider adding a comment to describe this security-critical functionality.

Suggested change

# Provide additional metadata for PaperTrail version records.

# The extra_data payload includes:

# - ip: the requester's remote IP address

# - user_agent: the HTTP User-Agent header

# This information is used for security/audit purposes and should be treated as

# sensitive logging data.

Copilot · 2026-01-01T22:29:08Z

app/views/email_change_mailer/email_changed_notification.html.erb

+
+<p class="secondary-text">
+  <strong><%= t(".not_you_title") %></strong>
+  <%= t(".not_you_body_html").html_safe %>


Using html_safe on the translation output could introduce XSS vulnerabilities if the translation contains unescaped user input or URLs. Verify that all interpolated values in this translation are properly sanitized.

Suggested change

<%= t(".not_you_body_html").html_safe %>

<%= sanitize(t(".not_you_body_html")) %>

Copilot · 2026-01-01T22:29:08Z

spec/models/identity/email_change_request_spec.rb

+    end
+
+    it "rejects email already taken by another identity" do
+      other_identity = create(:identity, primary_email: "[email protected]")


This assignment to other_identity is useless, since its value is never read.

24c02 added 13 commits January 1, 2026 16:47

another pass on mailers

ed0d8df

emailll

3f36ed6

screw it

d2c8821

fix backend i18n

0cf52e6

ratelimit email changes bc why not

afd6041

waow

5b55f36

ifpt on frontend too

fe12a01

secure_compare!

9a32f4f

rate limit verfs too

d6961f7

no generate_tokens race

d7c4059

minor security improvements

106ddd3

24c02 requested a review from Copilot January 1, 2026 22:23

Copilot started reviewing on behalf of 24c02 January 1, 2026 22:23 View session

Copilot AI reviewed Jan 1, 2026

View reviewed changes

24c02 added 4 commits January 1, 2026 17:35

left

4ccf0da

wrap that in a transaction

9783349

lint ts

46891df

put emc behind flipper

da5784e

24c02 merged commit 94858d5 into main Jan 1, 2026
0 of 2 checks passed

24c02 deleted the new-email-new-me branch January 1, 2026 22:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New email new me! #151

New email new me! #151

Uh oh!

24c02 commented Jan 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	new_email = email_change_params[:new_email]&.downcase&.strip
	new_email = email_change_params[:new_email]

		request = identity.email_change_requests.first
		return request if request

	<%= t(".not_you_body_html", cancel_url: @cancel_url).html_safe %>
	<%= sanitize(t(".not_you_body_html", cancel_url: @cancel_url)) %>

		@@ -18,6 +19,8 @@ def current_user = nil # TODO: this is a temp hack to fix partials until /backen

		def user_for_paper_trail = current_identity&.id

+  # Provide additional metadata for PaperTrail version records.
+  # The extra_data payload includes:
+  #   - ip:         the requester's remote IP address
+  #   - user_agent: the HTTP User-Agent header
+  # This information is used for security/audit purposes and should be treated as
+  # sensitive logging data.

	<%= t(".not_you_body_html").html_safe %>
	<%= sanitize(t(".not_you_body_html")) %>

New email new me! #151

New email new me! #151

Uh oh!

Conversation

24c02 commented Jan 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants