##// END OF EJS Templates
jro_apps » redmine
RSS
Security notifications when password or email adress is changed (#21421)....
Jean-Philippe Lang -
r14763:5d70fce6ce4c
parent child
Show More
Side by Side Unified
Context file:
r14763:5d70fce6ce4c Loading diff...:
Show file before | Show file after | Hide comments
@@ -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
Show file before | Show file after | Hide comments
@@ -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
Show file before | Show file after | Hide comments
@@ -73,6 +73,12 class AccountController < ApplicationController
73 73 @user.password, @user.password_confirmation = params[:new_password], params[:new_password_confirmation]
74 74 if @user.save
75 75 @token.destroy
76 Mailer.security_notification(@user,
77 message: :mail_body_security_notification_change,
78 field: :field_password,
79 title: :button_change_password,
80 url: {controller: 'my', action: 'password'}
81 ).deliver
76 82 flash[:notice] = l(:notice_account_password_updated)
77 83 redirect_to signin_path
78 84 return
Show file before | Show file after | Hide comments
@@ -133,6 +133,8 class ApplicationController < ActionController::Base
133 133 end
134 134 end
135 135 end
136 # store current ip address in user object ephemerally
137 user.remote_ip = request.remote_ip if user
136 138 user
137 139 end
138 140
Show file before | Show file after | Hide comments
@@ -105,6 +105,12 class MyController < ApplicationController
105 105 if @user.save
106 106 # The session token was destroyed by the password change, generate a new one
107 107 session[:tk] = @user.generate_session_token
108 Mailer.security_notification(@user,
109 message: :mail_body_security_notification_change,
110 field: :field_password,
111 title: :button_change_password,
112 url: {controller: 'my', action: 'password'}
113 ).deliver
108 114 flash[:notice] = l(:notice_account_password_updated)
109 115 redirect_to my_account_path
110 116 end
Show file before | Show file after | Hide comments
@@ -19,8 +19,9 class EmailAddress < ActiveRecord::Base
19 19 belongs_to :user
20 20 attr_protected :id
21 21
22 after_update :destroy_tokens
23 after_destroy :destroy_tokens
22 after_create :deliver_security_notification_create
23 after_update :destroy_tokens, :deliver_security_notification_update
24 after_destroy :destroy_tokens, :deliver_security_notification_destroy
24 25
25 26 validates_presence_of :address
26 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 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 98 # Delete all outstanding password reset tokens on email change.
46 99 # This helps to keep the account secure in case the associated email account
47 100 # was compromised.
Show file before | Show file after | Hide comments
@@ -318,6 +318,20 class Mailer < ActionMailer::Base
318 318 :subject => l(:mail_subject_register, Setting.app_title)
319 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 335 def test_email(user)
322 336 set_language_if_valid(user.language)
323 337 @url = url_for(:controller => 'welcome')
Show file before | Show file after | Hide comments
@@ -97,6 +97,8 class User < Principal
97 97
98 98 attr_accessor :password, :password_confirmation, :generate_password
99 99 attr_accessor :last_before_login_on
100 attr_accessor :remote_ip
101
100 102 # Prevents unauthorized assignments
101 103 attr_protected :login, :admin, :password, :password_confirmation, :hashed_password
102 104
Show file before | Show file after | Hide comments
@@ -848,6 +848,13 de:
848 848 mail_subject_reminder: "%{count} Tickets müssen in den nächsten %{days} Tagen abgegeben werden"
849 849 mail_subject_wiki_content_added: "Wiki-Seite '%{id}' hinzugefügt"
850 850 mail_subject_wiki_content_updated: "Wiki-Seite '%{id}' erfolgreich aktualisiert"
851 mail_subject_security_notification: "Sicherheitshinweis"
852 mail_body_security_notification_change: "%{field} wurde geändert."
853 mail_body_security_notification_change_to: "%{field} wurde geändert zu %{value}."
854 mail_body_security_notification_add: "%{field} %{value} wurde hinzugefügt."
855 mail_body_security_notification_remove: "%{field} %{value} wurde entfernt."
856 mail_body_security_notification_notify_enabled: "E-Mail-Adresse %{value} erhält nun Benachrichtigungen."
857 mail_body_security_notification_notify_disabled: "E-Mail-Adresse %{value} erhält keine Benachrichtigungen mehr."
851 858
852 859 notice_account_activated: Ihr Konto ist aktiviert. Sie können sich jetzt anmelden.
853 860 notice_account_deleted: Ihr Benutzerkonto wurde unwiderruflich gelöscht.
@@ -1148,6 +1155,7 de:
1148 1155 error_password_expired: Your password has expired or the administrator requires you
1149 1156 to change it.
1150 1157 field_time_entries_visibility: Time logs visibility
1158 field_remote_ip: IP-Adresse
1151 1159 label_parent_task_attributes: Parent tasks attributes
1152 1160 label_parent_task_attributes_derived: Calculated from subtasks
1153 1161 label_parent_task_attributes_independent: Independent of subtasks
Show file before | Show file after | Hide comments
@@ -228,6 +228,13 en:
228 228 mail_body_wiki_content_added: "The '%{id}' wiki page has been added by %{author}."
229 229 mail_subject_wiki_content_updated: "'%{id}' wiki page has been updated"
230 230 mail_body_wiki_content_updated: "The '%{id}' wiki page has been updated by %{author}."
231 mail_subject_security_notification: "Security notification"
232 mail_body_security_notification_change: "%{field} was changed."
233 mail_body_security_notification_change_to: "%{field} was changed to %{value}."
234 mail_body_security_notification_add: "%{field} %{value} was added."
235 mail_body_security_notification_remove: "%{field} %{value} was removed."
236 mail_body_security_notification_notify_enabled: "Email address %{value} now receives notifications."
237 mail_body_security_notification_notify_disabled: "Email address %{value} no longer receives notifications."
231 238
232 239 field_name: Name
233 240 field_description: Description
@@ -352,6 +359,7 en:
352 359 field_time_entries_visibility: Time logs visibility
353 360 field_total_estimated_hours: Total estimated time
354 361 field_default_version: Default version
362 field_remote_ip: IP address
355 363
356 364 setting_app_title: Application title
357 365 setting_app_subtitle: Application subtitle
Show file before | Show file after | Hide comments
@@ -400,6 +400,7 class AccountControllerTest < ActionController::TestCase
400 400 end
401 401
402 402 def test_post_lost_password_with_token_should_change_the_user_password
403 ActionMailer::Base.deliveries.clear
403 404 user = User.find(2)
404 405 token = Token.create!(:action => 'recovery', :user => user)
405 406
@@ -408,6 +409,10 class AccountControllerTest < ActionController::TestCase
408 409 user.reload
409 410 assert user.check_password?('newpass123')
410 411 assert_nil Token.find_by_id(token.id), "Token was not deleted"
412 assert_not_nil (mail = ActionMailer::Base.deliveries.last)
413 assert_select_email do
414 assert_select 'a[href^=?]', 'http://localhost:3000/my/password', :text => 'Change password'
415 end
411 416 end
412 417
413 418 def test_post_lost_password_with_token_for_non_active_user_should_fail
Show file before | Show file after | Hide comments
@@ -92,6 +92,22 class EmailAddressesControllerTest < ActionController::TestCase
92 92 end
93 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 111 def test_update
96 112 @request.session[:user_id] = 2
97 113 email = EmailAddress.create!(:user_id => 2, :address => 'another@somenet.foo')
@@ -112,6 +128,21 class EmailAddressesControllerTest < ActionController::TestCase
112 128 assert_equal false, email.reload.notify
113 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 146 def test_destroy
116 147 @request.session[:user_id] = 2
117 148 email = EmailAddress.create!(:user_id => 2, :address => 'another@somenet.foo')
@@ -141,4 +172,18 class EmailAddressesControllerTest < ActionController::TestCase
141 172 assert_response 404
142 173 end
143 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 189 end
Show file before | Show file after | Hide comments
@@ -117,6 +117,24 class MyControllerTest < ActionController::TestCase
117 117 assert user.groups.empty?
118 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 138 def test_my_account_should_show_destroy_link
121 139 get :account
122 140 assert_select 'a[href="/my/account/destroy"]'
@@ -193,6 +211,19 class MyControllerTest < ActionController::TestCase
193 211 assert_redirected_to '/my/account'
194 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 227 def test_page_layout
197 228 get :page_layout
198 229 assert_response :success
Show file before | Show file after | Hide comments
@@ -666,6 +666,51 class MailerTest < ActiveSupport::TestCase
666 666 end
667 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 714 def test_mailer_should_not_change_locale
670 715 # Set current language to italian
671 716 set_language_if_valid 'it'
General Comments 0
You need to be logged in to leave comments. Login now