From 36f257f3010a1f1b6da52d42295c0bff9b9feeed Mon Sep 17 00:00:00 2001 From: Jan Schulz-Hofen Date: Wed, 2 Dec 2015 18:36:46 +0800 Subject: [PATCH 4/7] Send a security notification when a user's email address is changed --- app/models/email_address.rb | 57 +++++++++++++++++++++- config/locales/de.yml | 2 + config/locales/en.yml | 2 + test/functional/email_addresses_controller_test.rb | 45 +++++++++++++++++ test/functional/my_controller_test.rb | 18 +++++++ 5 files changed, 122 insertions(+), 2 deletions(-) diff --git a/app/models/email_address.rb b/app/models/email_address.rb index 01fd75b..8348d03 100644 --- a/app/models/email_address.rb +++ b/app/models/email_address.rb @@ -19,8 +19,9 @@ class EmailAddress < ActiveRecord::Base belongs_to :user attr_protected :id - after_update :destroy_tokens - after_destroy :destroy_tokens + after_create :deliver_security_notification_create + after_update :destroy_tokens, :deliver_security_notification_update + after_destroy :destroy_tokens, :deliver_security_notification_destroy validates_presence_of :address validates_format_of :address, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i, :allow_blank => true @@ -42,6 +43,58 @@ class EmailAddress < ActiveRecord::Base private + # send a security notification to user that a new email address was added + def deliver_security_notification_create + # only deliver if this isn't the only address. + # in that case, the user is just being created and + # should not receive this email. + if user.mails != [address] + deliver_security_notification(user, + message: :mail_body_security_notification_add, + field: :field_mail, + value: address + ) + end + end + + # send a security notification to user that an email has been changed (notified/not notified) + def deliver_security_notification_update + if address_changed? + recipients = [user, address_was] + options = { + message: :mail_body_security_notification_change_to, + field: :field_mail, + value: address + } + elsif notify_changed? + recipients = [user, address] + options = { + message: notify_was ? :mail_body_security_notification_notify_disabled : :mail_body_security_notification_notify_enabled, + value: address + } + end + deliver_security_notification(recipients, options) + end + + # send a security notification to user that an email address was deleted + def deliver_security_notification_destroy + deliver_security_notification([user, address], + message: :mail_body_security_notification_remove, + field: :field_mail, + value: address + ) + end + + # generic method to send security notifications for email addresses + def deliver_security_notification(recipients, options={}) + Mailer.security_notification(recipients, + options.merge( + title: :label_my_account, + url: {controller: 'my', action: 'account'} + ) + ).deliver + end + # Delete all outstanding password reset tokens on email change. # This helps to keep the account secure in case the associated email account # was compromised. diff --git a/config/locales/de.yml b/config/locales/de.yml index e827e3d..4f43f1b 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -853,6 +853,8 @@ de: mail_body_security_notification_change_to: "%{field} wurde geändert zu %{value}." mail_body_security_notification_add: "%{field} %{value} wurde hinzugefügt." mail_body_security_notification_remove: "%{field} %{value} wurde entfernt." + mail_body_security_notification_notify_enabled: "E-Mail-Adresse %{value} erhält nun Benachrichtigungen." + mail_body_security_notification_notify_disabled: "E-Mail-Adresse %{value} erhält keine Benachrichtigungen mehr." notice_account_activated: Ihr Konto ist aktiviert. Sie können sich jetzt anmelden. notice_account_deleted: Ihr Benutzerkonto wurde unwiderruflich gelöscht. diff --git a/config/locales/en.yml b/config/locales/en.yml index 0b74c2e..4495a7a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -232,6 +232,8 @@ en: mail_body_security_notification_change_to: "%{field} was changed to %{value}." mail_body_security_notification_add: "%{field} %{value} was added." mail_body_security_notification_remove: "%{field} %{value} was removed." + mail_body_security_notification_notify_enabled: "Email address %{value} now receives notifications." + mail_body_security_notification_notify_disabled: "Email address %{value} no longer receives notifications." field_name: Name field_description: Description diff --git a/test/functional/email_addresses_controller_test.rb b/test/functional/email_addresses_controller_test.rb index 7c52d9c..3d2d6de 100644 --- a/test/functional/email_addresses_controller_test.rb +++ b/test/functional/email_addresses_controller_test.rb @@ -92,6 +92,22 @@ class EmailAddressesControllerTest < ActionController::TestCase end end + def test_create_should_send_security_notification + @request.session[:user_id] = 2 + ActionMailer::Base.deliveries.clear + post :create, :user_id => 2, :email_address => {:address => 'something@example.fr'} + + assert_not_nil (mail = ActionMailer::Base.deliveries.last) + assert_mail_body_match '0.0.0.0', mail + assert_mail_body_match I18n.t(:mail_body_security_notification_add, field: I18n.t(:field_mail), value: 'something@example.fr'), mail + assert_select_email do + assert_select 'a[href^=?]', 'http://localhost:3000/my/account', :text => 'My account' + end + # The old email address should be notified about a new address for security purposes + assert [mail.bcc, mail.cc].flatten.include?(User.find(2).mail) + assert [mail.bcc, mail.cc].flatten.include?('something@example.fr') + end + def test_update @request.session[:user_id] = 2 email = EmailAddress.create!(:user_id => 2, :address => 'another@somenet.foo') @@ -112,6 +128,21 @@ class EmailAddressesControllerTest < ActionController::TestCase assert_equal false, email.reload.notify end + def test_update_should_send_security_notification + @request.session[:user_id] = 2 + email = EmailAddress.create!(:user_id => 2, :address => 'another@somenet.foo') + + ActionMailer::Base.deliveries.clear + xhr :put, :update, :user_id => 2, :id => email.id, :notify => '0' + + assert_not_nil (mail = ActionMailer::Base.deliveries.last) + assert_mail_body_match I18n.t(:mail_body_security_notification_notify_disabled, value: 'another@somenet.foo'), mail + + # The changed address should be notified for security purposes + assert [mail.bcc, mail.cc].flatten.include?('another@somenet.foo') + end + + def test_destroy @request.session[:user_id] = 2 email = EmailAddress.create!(:user_id => 2, :address => 'another@somenet.foo') @@ -141,4 +172,18 @@ class EmailAddressesControllerTest < ActionController::TestCase assert_response 404 end end + + def test_destroy_should_send_security_notification + @request.session[:user_id] = 2 + email = EmailAddress.create!(:user_id => 2, :address => 'another@somenet.foo') + + ActionMailer::Base.deliveries.clear + xhr :delete, :destroy, :user_id => 2, :id => email.id + + assert_not_nil (mail = ActionMailer::Base.deliveries.last) + assert_mail_body_match I18n.t(:mail_body_security_notification_remove, field: I18n.t(:field_mail), value: 'another@somenet.foo'), mail + + # The removed address should be notified for security purposes + assert [mail.bcc, mail.cc].flatten.include?('another@somenet.foo') + end end diff --git a/test/functional/my_controller_test.rb b/test/functional/my_controller_test.rb index f048b62..4f3f2e2 100644 --- a/test/functional/my_controller_test.rb +++ b/test/functional/my_controller_test.rb @@ -117,6 +117,24 @@ class MyControllerTest < ActionController::TestCase assert user.groups.empty? end + def test_update_account_should_send_security_notification + ActionMailer::Base.deliveries.clear + post :account, + :user => { + :mail => 'foobar@example.com' + } + + assert_not_nil (mail = ActionMailer::Base.deliveries.last) + assert_mail_body_match '0.0.0.0', mail + assert_mail_body_match I18n.t(:mail_body_security_notification_change_to, field: I18n.t(:field_mail), value: 'foobar@example.com'), mail + assert_select_email do + assert_select 'a[href^=?]', 'http://localhost:3000/my/account', :text => 'My account' + end + # The old email address should be notified about the change for security purposes + assert [mail.bcc, mail.cc].flatten.include?(User.find(2).mail) + assert [mail.bcc, mail.cc].flatten.include?('foobar@example.com') + end + def test_my_account_should_show_destroy_link get :account assert_select 'a[href="/my/account/destroy"]' -- 2.4.0