Commit 47a60ac7 authored by jplang's avatar jplang

Security notification on password recovery is empty (#28302).

Patch by Felix Schäfer.

git-svn-id: https://svn.redmine.org/redmine/trunk@17269 e93f8b46-1217-0410-a6f0-8f06a7374b81
parent dd09d7b5
......@@ -87,7 +87,7 @@ class AccountController < ApplicationController
@user.must_change_passwd = false
if @user.save
@token.destroy
Mailer.password_updated(@user)
Mailer.password_updated(@user, { remote_ip: request.remote_ip })
flash[:notice] = l(:notice_account_password_updated)
redirect_to signin_path
return
......
......@@ -311,7 +311,7 @@ class Mailer < ActionMailer::Base
end
# Notifies user that his password was updated
def self.password_updated(user)
def self.password_updated(user, options={})
# Don't send a notification to the dummy email address when changing the password
# of the default admin account which is required after the first login
# TODO: maybe not the best way to handle this
......@@ -320,6 +320,8 @@ class Mailer < ActionMailer::Base
security_notification(user,
message: :mail_body_password_updated,
title: :button_change_password,
remote_ip: options[:remote_ip],
originator: user,
url: {controller: 'my', action: 'password'}
).deliver
end
......@@ -333,7 +335,6 @@ class Mailer < ActionMailer::Base
end
def security_notification(recipients, options={})
redmine_headers 'Sender' => User.current.login
@user = Array(recipients).detect{|r| r.is_a? User }
set_language_if_valid(@user.try :language)
@message = l(options[:message],
......@@ -341,7 +342,11 @@ class Mailer < ActionMailer::Base
value: options[:value]
)
@title = options[:title] && l(options[:title])
@originator = options[:originator] || User.current
@remote_ip = options[:remote_ip] || @originator.remote_ip
@url = options[:url] && (options[:url].is_a?(Hash) ? url_for(options[:url]) : options[:url])
redmine_headers 'Sender' => @originator.login
redmine_headers 'Url' => @url
mail :to => recipients,
:subject => "[#{Setting.app_title}] #{l(:mail_subject_security_notification)}"
end
......
......@@ -7,7 +7,7 @@
<%= content_tag :h1, @title -%>
<% end %></p>
<p><%= l(:field_user) %>: <strong><%= User.current.login %></strong><br/>
<%= l(:field_remote_ip) %>: <strong><%= User.current.remote_ip %></strong><br/>
<p><%= l(:field_user) %>: <strong><%= @originator.login %></strong><br/>
<%= l(:field_remote_ip) %>: <strong><%= @remote_ip %></strong><br/>
<%= l(:label_date) %>: <strong><%= format_time Time.now, true, @user %></strong></p>
......@@ -2,7 +2,7 @@
<%= @url || @title %>
<%= l(:field_user) %>: <%= User.current.login %>
<%= l(:field_remote_ip) %>: <%= User.current.remote_ip %>
<%= l(:field_user) %>: <%= @originator.login %>
<%= l(:field_remote_ip) %>: <%= @remote_ip %>
<%= l(:label_date) %>: <%= format_time Time.now, true, @user %>
......@@ -721,6 +721,23 @@ class MailerTest < ActiveSupport::TestCase
end
end
def test_security_notification_with_overridden_originator_and_remote_ip
set_language_if_valid User.find(1).language
with_settings :emails_footer => "footer without link" do
User.current.remote_ip = '192.168.1.1'
assert Mailer.security_notification(User.find(1), message: :notice_account_password_updated, originator: User.find(2), remote_ip: '10.0.0.42').deliver
mail = last_email
assert_not_nil mail
assert_mail_body_match User.find(2).login, mail
assert_mail_body_match '10.0.0.42', mail
assert_mail_body_match I18n.t(:notice_account_password_updated), mail
assert_select_email do
assert_select "h1", false
assert_select "a", false
end
end
end
def test_security_notification_should_include_title
set_language_if_valid User.find(2).language
with_settings :emails_footer => "footer without link" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment