jro_apps/redmine Commit - r14763:5d70fce6ce4c

Security notifications when password or email adress is changed (#21421)....

Jean-Philippe Lang -

r14763:5d70fce6ce4c

parent child

Context file:

r14763:5d70fce6ce4c

Collapse all files

app/views/mailer/security_notification.html.erb created 644 +13 0

@@ -0,0 +1,13
	1	<p><%= @message %><br />
	2	<% if @url && @title -%>
	3	<%= link_to @title, @url -%>
	4	<% elsif @url -%>
	5	<%= link_to @url -%>
	6	<% elsif @title -%>
	7	<%= content_tag :h1, @title -%>
	8	<% end %></p>
	9
	10	<p><%= l(:field_user) %>: <strong><%= User.current.login %></strong><br/>
	11	<%= l(:field_remote_ip) %>: <strong><%= User.current.remote_ip %></strong><br/>
	12	<%= l(:label_date) %>: <strong><%= format_time Time.now, true, @user %></strong></p>
	13

app/views/mailer/security_notification.text.erb created 644 +8 0

@@ -0,0 +1,8
	1	<%= @message %>
	2
	3	<%= @url \|\| @title %>
	4
	5	<%= l(:field_user) %>: <%= User.current.login %>
	6	<%= l(:field_remote_ip) %>: <%= User.current.remote_ip %>
	7	<%= l(:label_date) %>: <%= format_time Time.now, true, @user %>
	8

app/controllers/account_controller.rb +6 0

                     @user.password, @user.password_confirmation = params[:new_password], params[:new_password_confirmation]
                     if @user.save
                       @token.destroy
+                      Mailer.security_notification(@user,
+                        message: :mail_body_security_notification_change,
+                        field: :field_password,
+                        title: :button_change_password,
+                        url: {controller: 'my', action: 'password'}
+                      ).deliver
                       flash[:notice] = l(:notice_account_password_updated)
                       redirect_to signin_path
                       return

app/controllers/application_controller.rb +2 0

                     end
                   end
                 end
+                # store current ip address in user object ephemerally
+                user.remote_ip = request.remote_ip if user
                 user
               end

app/controllers/my_controller.rb +6 0

                     if @user.save
                       # The session token was destroyed by the password change, generate a new one
                       session[:tk] = @user.generate_session_token
+                      Mailer.security_notification(@user,
+                        message: :mail_body_security_notification_change,
+                        field: :field_password,
+                        title: :button_change_password,
+                        url: {controller: 'my', action: 'password'}
+                      ).deliver
                       flash[:notice] = l(:notice_account_password_updated)
                       redirect_to my_account_path
                     end

app/models/email_address.rb +55 -2

@@ -19,8 +19,9 class EmailAddress < ActiveRecord::Base
19	belongs_to :user	19	belongs_to :user
20	attr_protected :id	20	attr_protected :id
21		21
22	after_update :destroy_tokens	22	after_create :deliver_security_notification_create
23	after_destroy :destroy_tokens	23	after_update :destroy_tokens, :deliver_security_notification_update
		24	after_destroy :destroy_tokens, :deliver_security_notification_destroy
24		25
25	validates_presence_of :address	26	validates_presence_of :address
26	validates_format_of :address, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i, :allow_blank => true	27	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
42		43
43	private	44	private
44		45
		46	# send a security notification to user that a new email address was added
		47	def deliver_security_notification_create
		48	# only deliver if this isn't the only address.
		49	# in that case, the user is just being created and
		50	# should not receive this email.
		51	if user.mails != [address]
		52	deliver_security_notification(user,
		53	message: :mail_body_security_notification_add,
		54	field: :field_mail,
		55	value: address
		56	)
		57	end
		58	end
		59
		60	# send a security notification to user that an email has been changed (notified/not notified)
		61	def deliver_security_notification_update
		62	if address_changed?
		63	recipients = [user, address_was]
		64	options = {
		65	message: :mail_body_security_notification_change_to,
		66	field: :field_mail,
		67	value: address
		68	}
		69	elsif notify_changed?
		70	recipients = [user, address]
		71	options = {
		72	message: notify_was ? :mail_body_security_notification_notify_disabled : :mail_body_security_notification_notify_enabled,
		73	value: address
		74	}
		75	end
		76	deliver_security_notification(recipients, options)
		77	end
		78
		79	# send a security notification to user that an email address was deleted
		80	def deliver_security_notification_destroy
		81	deliver_security_notification([user, address],
		82	message: :mail_body_security_notification_remove,
		83	field: :field_mail,
		84	value: address
		85	)
		86	end
		87
		88	# generic method to send security notifications for email addresses
		89	def deliver_security_notification(recipients, options={})
		90	Mailer.security_notification(recipients,
		91	options.merge(
		92	title: :label_my_account,
		93	url: {controller: 'my', action: 'account'}
		94	)
		95	).deliver
		96	end
		97
45	# Delete all outstanding password reset tokens on email change.	98	# Delete all outstanding password reset tokens on email change.
46	# This helps to keep the account secure in case the associated email account	99	# This helps to keep the account secure in case the associated email account
47	# was compromised.	100	# was compromised.

app/models/mailer.rb +14 0

@@ -318,6 +318,20 class Mailer < ActionMailer::Base
318	:subject => l(:mail_subject_register, Setting.app_title)	318	:subject => l(:mail_subject_register, Setting.app_title)
319	end	319	end
320		320
		321	def security_notification(recipients, options={})
		322	redmine_headers 'Sender' => User.current.login
		323	@user = Array(recipients).detect{\|r\| r.is_a? User }
		324	set_language_if_valid(@user.try :language)
		325	@message = l(options[:message],
		326	field: (options[:field] && l(options[:field])),
		327	value: options[:value]
		328	)
		329	@title = options[:title] && l(options[:title])
		330	@url = options[:url] && (options[:url].is_a?(Hash) ? url_for(options[:url]) : options[:url])
		331	mail :to => recipients,
		332	:subject => l(:mail_subject_security_notification)
		333	end
		334
321	def test_email(user)	335	def test_email(user)
322	set_language_if_valid(user.language)	336	set_language_if_valid(user.language)
323	@url = url_for(:controller => 'welcome')	337	@url = url_for(:controller => 'welcome')

app/models/user.rb +2 0

               attr_accessor :password, :password_confirmation, :generate_password
               attr_accessor :last_before_login_on
+              attr_accessor :remote_ip
               # Prevents unauthorized assignments
               attr_protected :login, :admin, :password, :password_confirmation, :hashed_password

config/locales/de.yml +8 0

               mail_subject_reminder: "%{count} Tickets müssen in den nächsten %{days} Tagen abgegeben werden"
               mail_subject_wiki_content_added: "Wiki-Seite '%{id}' hinzugefügt"
               mail_subject_wiki_content_updated: "Wiki-Seite '%{id}' erfolgreich aktualisiert"
+              mail_subject_security_notification: "Sicherheitshinweis"
+              mail_body_security_notification_change: "%{field} wurde geÃ¤ndert."
+              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.
               error_password_expired: Your password has expired or the administrator requires you
                 to change it.
               field_time_entries_visibility: Time logs visibility
+              field_remote_ip: IP-Adresse
               label_parent_task_attributes: Parent tasks attributes
               label_parent_task_attributes_derived: Calculated from subtasks
               label_parent_task_attributes_independent: Independent of subtasks

config/locales/en.yml +8 0

               mail_body_wiki_content_added: "The '%{id}' wiki page has been added by %{author}."
               mail_subject_wiki_content_updated: "'%{id}' wiki page has been updated"
               mail_body_wiki_content_updated: "The '%{id}' wiki page has been updated by %{author}."
+              mail_subject_security_notification: "Security notification"
+              mail_body_security_notification_change: "%{field} was changed."
+              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
               field_time_entries_visibility: Time logs visibility
               field_total_estimated_hours: Total estimated time
               field_default_version: Default version
+              field_remote_ip: IP address
               setting_app_title: Application title
               setting_app_subtitle: Application subtitle

test/functional/account_controller_test.rb +5 0

               end
               def test_post_lost_password_with_token_should_change_the_user_password
+                ActionMailer::Base.deliveries.clear
                 user = User.find(2)
                 token = Token.create!(:action => 'recovery', :user => user)
                 user.reload
                 assert user.check_password?('newpass123')
                 assert_nil Token.find_by_id(token.id), "Token was not deleted"
+                assert_not_nil (mail = ActionMailer::Base.deliveries.last)
+                assert_select_email do
+                  assert_select 'a[href^=?]', 'http://localhost:3000/my/password', :text => 'Change password'
+                end
               end
               def test_post_lost_password_with_token_for_non_active_user_should_fail

test/functional/email_addresses_controller_test.rb +45 0

@@ -92,6 +92,22 class EmailAddressesControllerTest < ActionController::TestCase
92	end	92	end
93	end	93	end
94		94
		95	def test_create_should_send_security_notification
		96	@request.session[:user_id] = 2
		97	ActionMailer::Base.deliveries.clear
		98	post :create, :user_id => 2, :email_address => {:address => 'something@example.fr'}
		99
		100	assert_not_nil (mail = ActionMailer::Base.deliveries.last)
		101	assert_mail_body_match '0.0.0.0', mail
		102	assert_mail_body_match I18n.t(:mail_body_security_notification_add, field: I18n.t(:field_mail), value: 'something@example.fr'), mail
		103	assert_select_email do
		104	assert_select 'a[href^=?]', 'http://localhost:3000/my/account', :text => 'My account'
		105	end
		106	# The old email address should be notified about a new address for security purposes
		107	assert [mail.bcc, mail.cc].flatten.include?(User.find(2).mail)
		108	assert [mail.bcc, mail.cc].flatten.include?('something@example.fr')
		109	end
		110
95	def test_update	111	def test_update
96	@request.session[:user_id] = 2	112	@request.session[:user_id] = 2
97	email = EmailAddress.create!(:user_id => 2, :address => 'another@somenet.foo')	113	email = EmailAddress.create!(:user_id => 2, :address => 'another@somenet.foo')
@@ -112,6 +128,21 class EmailAddressesControllerTest < ActionController::TestCase
112	assert_equal false, email.reload.notify	128	assert_equal false, email.reload.notify
113	end	129	end
114		130
		131	def test_update_should_send_security_notification
		132	@request.session[:user_id] = 2
		133	email = EmailAddress.create!(:user_id => 2, :address => 'another@somenet.foo')
		134
		135	ActionMailer::Base.deliveries.clear
		136	xhr :put, :update, :user_id => 2, :id => email.id, :notify => '0'
		137
		138	assert_not_nil (mail = ActionMailer::Base.deliveries.last)
		139	assert_mail_body_match I18n.t(:mail_body_security_notification_notify_disabled, value: 'another@somenet.foo'), mail
		140
		141	# The changed address should be notified for security purposes
		142	assert [mail.bcc, mail.cc].flatten.include?('another@somenet.foo')
		143	end
		144
		145
115	def test_destroy	146	def test_destroy
116	@request.session[:user_id] = 2	147	@request.session[:user_id] = 2
117	email = EmailAddress.create!(:user_id => 2, :address => 'another@somenet.foo')	148	email = EmailAddress.create!(:user_id => 2, :address => 'another@somenet.foo')
@@ -141,4 +172,18 class EmailAddressesControllerTest < ActionController::TestCase
141	assert_response 404	172	assert_response 404
142	end	173	end
143	end	174	end
		175
		176	def test_destroy_should_send_security_notification
		177	@request.session[:user_id] = 2
		178	email = EmailAddress.create!(:user_id => 2, :address => 'another@somenet.foo')
		179
		180	ActionMailer::Base.deliveries.clear
		181	xhr :delete, :destroy, :user_id => 2, :id => email.id
		182
		183	assert_not_nil (mail = ActionMailer::Base.deliveries.last)
		184	assert_mail_body_match I18n.t(:mail_body_security_notification_remove, field: I18n.t(:field_mail), value: 'another@somenet.foo'), mail
		185
		186	# The removed address should be notified for security purposes
		187	assert [mail.bcc, mail.cc].flatten.include?('another@somenet.foo')
		188	end
144	end	189	end

test/functional/my_controller_test.rb +31 0

@@ -117,6 +117,24 class MyControllerTest < ActionController::TestCase
117	assert user.groups.empty?	117	assert user.groups.empty?
118	end	118	end
119		119
		120	def test_update_account_should_send_security_notification
		121	ActionMailer::Base.deliveries.clear
		122	post :account,
		123	:user => {
		124	:mail => 'foobar@example.com'
		125	}
		126
		127	assert_not_nil (mail = ActionMailer::Base.deliveries.last)
		128	assert_mail_body_match '0.0.0.0', mail
		129	assert_mail_body_match I18n.t(:mail_body_security_notification_change_to, field: I18n.t(:field_mail), value: 'foobar@example.com'), mail
		130	assert_select_email do
		131	assert_select 'a[href^=?]', 'http://localhost:3000/my/account', :text => 'My account'
		132	end
		133	# The old email address should be notified about the change for security purposes
		134	assert [mail.bcc, mail.cc].flatten.include?(User.find(2).mail)
		135	assert [mail.bcc, mail.cc].flatten.include?('foobar@example.com')
		136	end
		137
120	def test_my_account_should_show_destroy_link	138	def test_my_account_should_show_destroy_link
121	get :account	139	get :account
122	assert_select 'a[href="/my/account/destroy"]'	140	assert_select 'a[href="/my/account/destroy"]'
@@ -193,6 +211,19 class MyControllerTest < ActionController::TestCase
193	assert_redirected_to '/my/account'	211	assert_redirected_to '/my/account'
194	end	212	end
195		213
		214	def test_change_password_should_send_security_notification
		215	ActionMailer::Base.deliveries.clear
		216	post :password, :password => 'jsmith',
		217	:new_password => 'secret123',
		218	:new_password_confirmation => 'secret123'
		219
		220	assert_not_nil (mail = ActionMailer::Base.deliveries.last)
		221	assert_mail_body_no_match 'secret123', mail # just to be sure: pw should never be sent!
		222	assert_select_email do
		223	assert_select 'a[href^=?]', 'http://localhost:3000/my/password', :text => 'Change password'
		224	end
		225	end
		226
196	def test_page_layout	227	def test_page_layout
197	get :page_layout	228	get :page_layout
198	assert_response :success	229	assert_response :success

test/unit/mailer_test.rb +45 0

@@ -666,6 +666,51 class MailerTest < ActiveSupport::TestCase
666	end	666	end
667	end	667	end
668		668
		669	def test_security_notification
		670	set_language_if_valid User.find(1).language
		671	with_settings :emails_footer => "footer without link" do
		672	User.current.remote_ip = '192.168.1.1'
		673	assert Mailer.security_notification(User.find(1), message: :notice_account_password_updated).deliver
		674	mail = last_email
		675	assert_not_nil mail
		676	assert_mail_body_match '192.168.1.1', mail
		677	assert_mail_body_match I18n.t(:notice_account_password_updated), mail
		678	assert_select_email do
		679	assert_select "h1", false
		680	assert_select "a", false
		681	end
		682	end
		683	end
		684
		685	def test_security_notification_should_include_title
		686	set_language_if_valid User.find(2).language
		687	with_settings :emails_footer => "footer without link" do
		688	assert Mailer.security_notification(User.find(2),
		689	message: :notice_account_password_updated,
		690	title: :label_my_account
		691	).deliver
		692	assert_select_email do
		693	assert_select "a", false
		694	assert_select "h1", :text => I18n.t(:label_my_account)
		695	end
		696	end
		697	end
		698
		699	def test_security_notification_should_include_link
		700	set_language_if_valid User.find(3).language
		701	with_settings :emails_footer => "footer without link" do
		702	assert Mailer.security_notification(User.find(3),
		703	message: :notice_account_password_updated,
		704	title: :label_my_account,
		705	url: {controller: 'my', action: 'account'}
		706	).deliver
		707	assert_select_email do
		708	assert_select "h1", false
		709	assert_select 'a[href=?]', 'http://mydomain.foo/my/account', :text => I18n.t(:label_my_account)
		710	end
		711	end
		712	end
		713
669	def test_mailer_should_not_change_locale	714	def test_mailer_should_not_change_locale
670	# Set current language to italian	715	# Set current language to italian
671	set_language_if_valid 'it'	716	set_language_if_valid 'it'

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages