From cc93f72f0ad739d7744edef7eaff834836a3c441 Mon Sep 17 00:00:00 2001 From: Dan Milne Date: Sat, 2 May 2026 23:52:12 +1000 Subject: [PATCH] Notify users out-of-band when security settings change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously only TOTP-enabled triggered an email. Every other security-relevant change — password change, TOTP disable, passkey add/remove, API key create/revoke, email address change, backup-code regeneration — happened silently, so an attacker on a stolen session could quietly drop 2FA or hijack the email with no signal to the account holder. Add SecurityMailer with one method per event. Each email carries the request IP, user-agent, and timestamp so the user can spot unfamiliar activity. Email-address changes notify both the old and new addresses with directional language; the old-address copy explicitly warns that whoever made the change can now receive password reset emails. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/controllers/api_keys_controller.rb | 2 + app/controllers/application_controller.rb | 4 + app/controllers/passwords_controller.rb | 1 + app/controllers/profiles_controller.rb | 9 ++ app/controllers/totp_controller.rb | 2 + app/controllers/webauthn_controller.rb | 5 + app/mailers/security_mailer.rb | 59 ++++++++++ .../security_mailer/_event_metadata.html.erb | 11 ++ .../security_mailer/_event_metadata.text.erb | 7 ++ .../security_mailer/api_key_created.html.erb | 8 ++ .../security_mailer/api_key_created.text.erb | 6 + .../security_mailer/api_key_revoked.html.erb | 8 ++ .../security_mailer/api_key_revoked.text.erb | 6 + .../backup_codes_regenerated.html.erb | 9 ++ .../backup_codes_regenerated.text.erb | 6 + .../email_address_changed.html.erb | 22 ++++ .../email_address_changed.text.erb | 14 +++ .../security_mailer/passkey_added.html.erb | 8 ++ .../security_mailer/passkey_added.text.erb | 6 + .../security_mailer/passkey_removed.html.erb | 8 ++ .../security_mailer/passkey_removed.text.erb | 6 + .../security_mailer/password_changed.html.erb | 8 ++ .../security_mailer/password_changed.text.erb | 5 + .../security_mailer/totp_disabled.html.erb | 8 ++ .../security_mailer/totp_disabled.text.erb | 6 + test/mailers/security_mailer_test.rb | 111 ++++++++++++++++++ 26 files changed, 345 insertions(+) create mode 100644 app/mailers/security_mailer.rb create mode 100644 app/views/security_mailer/_event_metadata.html.erb create mode 100644 app/views/security_mailer/_event_metadata.text.erb create mode 100644 app/views/security_mailer/api_key_created.html.erb create mode 100644 app/views/security_mailer/api_key_created.text.erb create mode 100644 app/views/security_mailer/api_key_revoked.html.erb create mode 100644 app/views/security_mailer/api_key_revoked.text.erb create mode 100644 app/views/security_mailer/backup_codes_regenerated.html.erb create mode 100644 app/views/security_mailer/backup_codes_regenerated.text.erb create mode 100644 app/views/security_mailer/email_address_changed.html.erb create mode 100644 app/views/security_mailer/email_address_changed.text.erb create mode 100644 app/views/security_mailer/passkey_added.html.erb create mode 100644 app/views/security_mailer/passkey_added.text.erb create mode 100644 app/views/security_mailer/passkey_removed.html.erb create mode 100644 app/views/security_mailer/passkey_removed.text.erb create mode 100644 app/views/security_mailer/password_changed.html.erb create mode 100644 app/views/security_mailer/password_changed.text.erb create mode 100644 app/views/security_mailer/totp_disabled.html.erb create mode 100644 app/views/security_mailer/totp_disabled.text.erb create mode 100644 test/mailers/security_mailer_test.rb diff --git a/app/controllers/api_keys_controller.rb b/app/controllers/api_keys_controller.rb index 0caf305..40ea449 100644 --- a/app/controllers/api_keys_controller.rb +++ b/app/controllers/api_keys_controller.rb @@ -14,6 +14,7 @@ class ApiKeysController < ApplicationController @api_key = Current.session.user.api_keys.build(api_key_params) if @api_key.save + SecurityMailer.api_key_created(Current.session.user, name: @api_key.name, **security_event_context).deliver_later flash[:api_key_token] = @api_key.plaintext_token redirect_to api_key_path(@api_key) else @@ -31,6 +32,7 @@ class ApiKeysController < ApplicationController def destroy @api_key.revoke! + SecurityMailer.api_key_revoked(@api_key.user, name: @api_key.name, **security_event_context).deliver_later redirect_to api_keys_path, notice: "API key revoked." end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index eb41935..79e67e5 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -14,6 +14,10 @@ class ApplicationController < ActionController::Base private + def security_event_context + {ip: request.remote_ip, user_agent: request.user_agent, occurred_at: Time.current} + end + # Remove a query parameter from a URL using proper URI parsing # More robust than regex - handles URL encoding, edge cases, etc. # diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index 6b8bfad..5eaf946 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -20,6 +20,7 @@ class PasswordsController < ApplicationController def update if @user.update(params.permit(:password, :password_confirmation)) + SecurityMailer.password_changed(@user, **security_event_context).deliver_later @user.sessions.destroy_all redirect_to signin_path, notice: "Password has been reset." else diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index 89ce2bf..76dd0e7 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -15,6 +15,7 @@ class ProfilesController < ApplicationController end if @user.update(password_params) + SecurityMailer.password_changed(@user, **security_event_context).deliver_later redirect_to profile_path, notice: "Password updated successfully." else render :show, status: :unprocessable_entity @@ -27,7 +28,15 @@ class ProfilesController < ApplicationController return end + old_email = @user.email_address if @user.update(email_params) + new_email = @user.email_address + if old_email != new_email + context = security_event_context + [old_email, new_email].uniq.each do |recipient| + SecurityMailer.email_address_changed(@user, recipient: recipient, old_email: old_email, new_email: new_email, **context).deliver_later + end + end redirect_to profile_path, notice: "Email updated successfully." else render :show, status: :unprocessable_entity diff --git a/app/controllers/totp_controller.rb b/app/controllers/totp_controller.rb index 068d7f8..9808961 100644 --- a/app/controllers/totp_controller.rb +++ b/app/controllers/totp_controller.rb @@ -103,6 +103,7 @@ class TotpController < ApplicationController # Generate new backup codes and store BCrypt hashes plain_codes = @user.send(:generate_backup_codes) @user.save! + SecurityMailer.backup_codes_regenerated(@user, **security_event_context).deliver_later # Store plain codes temporarily in session for display session[:temp_backup_codes] = plain_codes @@ -136,6 +137,7 @@ class TotpController < ApplicationController end @user.disable_totp! + SecurityMailer.totp_disabled(@user, **security_event_context).deliver_later redirect_to profile_path, notice: "Two-factor authentication has been disabled." end diff --git a/app/controllers/webauthn_controller.rb b/app/controllers/webauthn_controller.rb index 535615f..770337b 100644 --- a/app/controllers/webauthn_controller.rb +++ b/app/controllers/webauthn_controller.rb @@ -91,6 +91,8 @@ class WebauthnController < ApplicationController backup_state: backup_state ) + SecurityMailer.passkey_added(user, nickname: @webauthn_credential.nickname, **security_event_context).deliver_later + render json: { success: true, message: "Passkey '#{nickname}' registered successfully", @@ -109,8 +111,11 @@ class WebauthnController < ApplicationController # Remove a passkey def destroy nickname = @webauthn_credential.nickname + user = @webauthn_credential.user @webauthn_credential.destroy + SecurityMailer.passkey_removed(user, nickname: nickname, **security_event_context).deliver_later + respond_to do |format| format.html { redirect_to profile_path, diff --git a/app/mailers/security_mailer.rb b/app/mailers/security_mailer.rb new file mode 100644 index 0000000..9630da5 --- /dev/null +++ b/app/mailers/security_mailer.rb @@ -0,0 +1,59 @@ +class SecurityMailer < ApplicationMailer + SUBJECT_PREFIX = "[Clinch security alert] ".freeze + + def password_changed(user, ip:, user_agent:, occurred_at:) + assign_context(user, ip, user_agent, occurred_at) + mail subject: "#{SUBJECT_PREFIX}Your password was changed", to: user.email_address + end + + def totp_disabled(user, ip:, user_agent:, occurred_at:) + assign_context(user, ip, user_agent, occurred_at) + mail subject: "#{SUBJECT_PREFIX}Two-factor authentication was disabled", to: user.email_address + end + + def backup_codes_regenerated(user, ip:, user_agent:, occurred_at:) + assign_context(user, ip, user_agent, occurred_at) + mail subject: "#{SUBJECT_PREFIX}Two-factor backup codes were regenerated", to: user.email_address + end + + def passkey_added(user, nickname:, ip:, user_agent:, occurred_at:) + assign_context(user, ip, user_agent, occurred_at) + @nickname = nickname + mail subject: "#{SUBJECT_PREFIX}A passkey was added to your account", to: user.email_address + end + + def passkey_removed(user, nickname:, ip:, user_agent:, occurred_at:) + assign_context(user, ip, user_agent, occurred_at) + @nickname = nickname + mail subject: "#{SUBJECT_PREFIX}A passkey was removed from your account", to: user.email_address + end + + def api_key_created(user, name:, ip:, user_agent:, occurred_at:) + assign_context(user, ip, user_agent, occurred_at) + @api_key_name = name + mail subject: "#{SUBJECT_PREFIX}An API key was created on your account", to: user.email_address + end + + def api_key_revoked(user, name:, ip:, user_agent:, occurred_at:) + assign_context(user, ip, user_agent, occurred_at) + @api_key_name = name + mail subject: "#{SUBJECT_PREFIX}An API key was revoked on your account", to: user.email_address + end + + def email_address_changed(user, recipient:, old_email:, new_email:, ip:, user_agent:, occurred_at:) + assign_context(user, ip, user_agent, occurred_at) + @recipient = recipient + @old_email = old_email + @new_email = new_email + mail subject: "#{SUBJECT_PREFIX}Your account email address was changed", to: recipient + end + + private + + def assign_context(user, ip, user_agent, occurred_at) + @user = user + @ip = ip + @user_agent = user_agent + @occurred_at = occurred_at + end +end diff --git a/app/views/security_mailer/_event_metadata.html.erb b/app/views/security_mailer/_event_metadata.html.erb new file mode 100644 index 0000000..4a6e21f --- /dev/null +++ b/app/views/security_mailer/_event_metadata.html.erb @@ -0,0 +1,11 @@ +
+

+ This action was recorded at <%= @occurred_at.to_fs(:long) %> + from IP <%= @ip %> + using <%= @user_agent.presence || "an unknown client" %>. +

+ +

+ If you did not perform this action, reset your password + immediately and contact your administrator. +

diff --git a/app/views/security_mailer/_event_metadata.text.erb b/app/views/security_mailer/_event_metadata.text.erb new file mode 100644 index 0000000..cc172d2 --- /dev/null +++ b/app/views/security_mailer/_event_metadata.text.erb @@ -0,0 +1,7 @@ +--- +This action was recorded at <%= @occurred_at.to_fs(:long) %> +from IP <%= @ip %> +using <%= @user_agent.presence || "an unknown client" %>. + +If you did not perform this action, reset your password immediately +and contact your administrator. diff --git a/app/views/security_mailer/api_key_created.html.erb b/app/views/security_mailer/api_key_created.html.erb new file mode 100644 index 0000000..f847fc2 --- /dev/null +++ b/app/views/security_mailer/api_key_created.html.erb @@ -0,0 +1,8 @@ +

Hello,

+ +

+ A new API key (<%= @api_key_name %>) was just created + on your Clinch account (<%= @user.email_address %>). +

+ +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/api_key_created.text.erb b/app/views/security_mailer/api_key_created.text.erb new file mode 100644 index 0000000..6dab7c4 --- /dev/null +++ b/app/views/security_mailer/api_key_created.text.erb @@ -0,0 +1,6 @@ +Hello, + +A new API key ("<%= @api_key_name %>") was just created on your Clinch +account (<%= @user.email_address %>). + +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/api_key_revoked.html.erb b/app/views/security_mailer/api_key_revoked.html.erb new file mode 100644 index 0000000..5a91850 --- /dev/null +++ b/app/views/security_mailer/api_key_revoked.html.erb @@ -0,0 +1,8 @@ +

Hello,

+ +

+ The API key <%= @api_key_name %> was just revoked + on your Clinch account (<%= @user.email_address %>). +

+ +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/api_key_revoked.text.erb b/app/views/security_mailer/api_key_revoked.text.erb new file mode 100644 index 0000000..dbbc42c --- /dev/null +++ b/app/views/security_mailer/api_key_revoked.text.erb @@ -0,0 +1,6 @@ +Hello, + +The API key "<%= @api_key_name %>" was just revoked on your Clinch +account (<%= @user.email_address %>). + +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/backup_codes_regenerated.html.erb b/app/views/security_mailer/backup_codes_regenerated.html.erb new file mode 100644 index 0000000..1586212 --- /dev/null +++ b/app/views/security_mailer/backup_codes_regenerated.html.erb @@ -0,0 +1,9 @@ +

Hello,

+ +

+ A new set of two-factor backup codes was generated on your Clinch + account (<%= @user.email_address %>). + Any previous backup codes are now invalid. +

+ +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/backup_codes_regenerated.text.erb b/app/views/security_mailer/backup_codes_regenerated.text.erb new file mode 100644 index 0000000..94231a2 --- /dev/null +++ b/app/views/security_mailer/backup_codes_regenerated.text.erb @@ -0,0 +1,6 @@ +Hello, + +A new set of two-factor backup codes was generated on your Clinch account +(<%= @user.email_address %>). Any previous backup codes are now invalid. + +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/email_address_changed.html.erb b/app/views/security_mailer/email_address_changed.html.erb new file mode 100644 index 0000000..cdc2f70 --- /dev/null +++ b/app/views/security_mailer/email_address_changed.html.erb @@ -0,0 +1,22 @@ +

Hello,

+ +<% if @recipient == @new_email %> +

+ The email address on your Clinch account is now + <%= @new_email %>. + It was previously <%= @old_email %>. +

+<% else %> +

+ The email address on your Clinch account was changed away from this + address (<%= @old_email %>) to + <%= @new_email %>. +

+

+ If this was not you, contact your administrator + immediately — whoever made the change can now receive password + reset emails for the account. +

+<% end %> + +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/email_address_changed.text.erb b/app/views/security_mailer/email_address_changed.text.erb new file mode 100644 index 0000000..2b60dcf --- /dev/null +++ b/app/views/security_mailer/email_address_changed.text.erb @@ -0,0 +1,14 @@ +Hello, + +<% if @recipient == @new_email %> +The email address on your Clinch account is now <%= @new_email %>. +It was previously <%= @old_email %>. +<% else %> +The email address on your Clinch account was changed away from this +address (<%= @old_email %>) to <%= @new_email %>. + +If this was not you, contact your administrator immediately — whoever +made the change can now receive password reset emails for the account. +<% end %> + +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/passkey_added.html.erb b/app/views/security_mailer/passkey_added.html.erb new file mode 100644 index 0000000..f1de4fd --- /dev/null +++ b/app/views/security_mailer/passkey_added.html.erb @@ -0,0 +1,8 @@ +

Hello,

+ +

+ A new passkey (<%= @nickname %>) was just added to your + Clinch account (<%= @user.email_address %>). +

+ +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/passkey_added.text.erb b/app/views/security_mailer/passkey_added.text.erb new file mode 100644 index 0000000..a3c72d5 --- /dev/null +++ b/app/views/security_mailer/passkey_added.text.erb @@ -0,0 +1,6 @@ +Hello, + +A new passkey ("<%= @nickname %>") was just added to your Clinch account +(<%= @user.email_address %>). + +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/passkey_removed.html.erb b/app/views/security_mailer/passkey_removed.html.erb new file mode 100644 index 0000000..666cbe4 --- /dev/null +++ b/app/views/security_mailer/passkey_removed.html.erb @@ -0,0 +1,8 @@ +

Hello,

+ +

+ A passkey (<%= @nickname %>) was just removed from your + Clinch account (<%= @user.email_address %>). +

+ +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/passkey_removed.text.erb b/app/views/security_mailer/passkey_removed.text.erb new file mode 100644 index 0000000..8fc6814 --- /dev/null +++ b/app/views/security_mailer/passkey_removed.text.erb @@ -0,0 +1,6 @@ +Hello, + +A passkey ("<%= @nickname %>") was just removed from your Clinch account +(<%= @user.email_address %>). + +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/password_changed.html.erb b/app/views/security_mailer/password_changed.html.erb new file mode 100644 index 0000000..670acfd --- /dev/null +++ b/app/views/security_mailer/password_changed.html.erb @@ -0,0 +1,8 @@ +

Hello,

+ +

+ The password on your Clinch account + (<%= @user.email_address %>) was just changed. +

+ +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/password_changed.text.erb b/app/views/security_mailer/password_changed.text.erb new file mode 100644 index 0000000..86fdbaf --- /dev/null +++ b/app/views/security_mailer/password_changed.text.erb @@ -0,0 +1,5 @@ +Hello, + +The password on your Clinch account (<%= @user.email_address %>) was just changed. + +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/totp_disabled.html.erb b/app/views/security_mailer/totp_disabled.html.erb new file mode 100644 index 0000000..e9d2148 --- /dev/null +++ b/app/views/security_mailer/totp_disabled.html.erb @@ -0,0 +1,8 @@ +

Hello,

+ +

+ Two-factor authentication was just disabled on your + Clinch account (<%= @user.email_address %>). +

+ +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/totp_disabled.text.erb b/app/views/security_mailer/totp_disabled.text.erb new file mode 100644 index 0000000..ceaa9e4 --- /dev/null +++ b/app/views/security_mailer/totp_disabled.text.erb @@ -0,0 +1,6 @@ +Hello, + +Two-factor authentication was just disabled on your Clinch account +(<%= @user.email_address %>). + +<%= render "event_metadata" %> diff --git a/test/mailers/security_mailer_test.rb b/test/mailers/security_mailer_test.rb new file mode 100644 index 0000000..f89e6d5 --- /dev/null +++ b/test/mailers/security_mailer_test.rb @@ -0,0 +1,111 @@ +require "test_helper" + +class SecurityMailerTest < ActionMailer::TestCase + CONTEXT = { + ip: "203.0.113.42", + user_agent: "Mozilla/5.0 (TestBrowser)", + occurred_at: Time.utc(2026, 5, 2, 13, 37) + }.freeze + + def setup + @user = User.create!(email_address: "security_mailer_test@example.com", password: "password123") + end + + def teardown + @user.destroy + end + + test "password_changed names the user and includes request metadata" do + email = SecurityMailer.password_changed(@user, **CONTEXT) + + assert_equal [@user.email_address], email.to + assert_match(/password was changed/i, email.subject) + assert_bodies_contain email, @user.email_address + assert_bodies_contain email, "203.0.113.42" + assert_bodies_contain email, "TestBrowser" + end + + test "totp_disabled describes the change" do + email = SecurityMailer.totp_disabled(@user, **CONTEXT) + + assert_equal [@user.email_address], email.to + assert_match(/two-factor.*disabled/i, email.subject) + assert_bodies_contain email, "203.0.113.42" + end + + test "backup_codes_regenerated mentions previous codes are invalid" do + email = SecurityMailer.backup_codes_regenerated(@user, **CONTEXT) + + assert_match(/backup codes/i, email.subject) + assert_bodies_match email, /previous backup codes are now invalid/i + end + + test "passkey_added includes the nickname" do + email = SecurityMailer.passkey_added(@user, nickname: "Yubikey-5", **CONTEXT) + + assert_match(/passkey.*added/i, email.subject) + assert_bodies_contain email, "Yubikey-5" + end + + test "passkey_removed includes the nickname" do + email = SecurityMailer.passkey_removed(@user, nickname: "Old MacBook", **CONTEXT) + + assert_match(/passkey.*removed/i, email.subject) + assert_bodies_contain email, "Old MacBook" + end + + test "api_key_created includes the key name" do + email = SecurityMailer.api_key_created(@user, name: "CI bot", **CONTEXT) + + assert_match(/api key.*created/i, email.subject) + assert_bodies_contain email, "CI bot" + end + + test "api_key_revoked includes the key name" do + email = SecurityMailer.api_key_revoked(@user, name: "Old token", **CONTEXT) + + assert_match(/api key.*revoked/i, email.subject) + assert_bodies_contain email, "Old token" + end + + test "email_address_changed sent to new address confirms the new value" do + email = SecurityMailer.email_address_changed(@user, + recipient: "new@example.com", + old_email: "old@example.com", + new_email: "new@example.com", + **CONTEXT) + + assert_equal ["new@example.com"], email.to + assert_bodies_contain email, "new@example.com" + assert_bodies_contain email, "old@example.com" + assert_bodies_no_match email, /reset emails for the account/ + end + + test "email_address_changed sent to old address warns about reset emails" do + email = SecurityMailer.email_address_changed(@user, + recipient: "old@example.com", + old_email: "old@example.com", + new_email: "new@example.com", + **CONTEXT) + + assert_equal ["old@example.com"], email.to + assert_bodies_match email, /reset emails for the account/ + end + + private + + def assert_bodies_contain(email, fragment) + assert_match fragment, email.text_part.body.to_s, "expected text body to contain #{fragment.inspect}" + assert_match fragment, email.html_part.body.to_s, "expected html body to contain #{fragment.inspect}" + end + + def assert_bodies_match(email, regex) + assert_match regex, email.text_part.body.to_s + assert_match regex, email.html_part.body.to_s + end + + def assert_bodies_no_match(email, regex) + refute_match regex, email.text_part.body.to_s + refute_match regex, email.html_part.body.to_s + end +end