summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGELOG1
-rw-r--r--app/controllers/passwords_controller.rb20
-rw-r--r--app/helpers/emails_helper.rb19
-rw-r--r--app/views/devise/passwords/new.html.haml2
-rw-r--r--app/views/notify/new_user_email.html.haml4
-rw-r--r--app/views/notify/new_user_email.text.erb2
-rw-r--r--spec/helpers/emails_helper_spec.rb46
-rw-r--r--spec/mailers/notify_spec.rb58
8 files changed, 119 insertions, 33 deletions
diff --git a/CHANGELOG b/CHANGELOG
index ade877feb9a..15bfe570f1a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -32,6 +32,7 @@ v 7.11.0 (unreleased)
- Show Atom feed buttons everywhere where applicable.
- Add project activity atom feed.
- Don't crash when an MR from a fork has a cross-reference comment from the target project on one of its commits.
+ - Explain how to get a new password reset token in welcome emails
- Include commit comments in MR from a forked project.
- Fix adding new group members from admin area
- Group milestones by title in the dashboard and all other issue views.
diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb
index 88459d4080a..145f27b67dd 100644
--- a/app/controllers/passwords_controller.rb
+++ b/app/controllers/passwords_controller.rb
@@ -36,4 +36,24 @@ class PasswordsController < Devise::PasswordsController
end
end
end
+
+ def edit
+ super
+ reset_password_token = Devise.token_generator.digest(
+ User,
+ :reset_password_token,
+ resource.reset_password_token
+ )
+
+ unless reset_password_token.nil?
+ user = User.where(
+ reset_password_token: reset_password_token
+ ).first_or_initialize
+
+ unless user.reset_password_period_valid?
+ flash[:alert] = 'Your password reset token has expired.'
+ redirect_to(new_user_password_url(user_email: user['email']))
+ end
+ end
+ end
end
diff --git a/app/helpers/emails_helper.rb b/app/helpers/emails_helper.rb
index 0df3ecc90b7..128de18bc47 100644
--- a/app/helpers/emails_helper.rb
+++ b/app/helpers/emails_helper.rb
@@ -35,4 +35,23 @@ module EmailsHelper
lexer = Rugments::Lexers::Diff.new
raw formatter.format(lexer.lex(diffcontent))
end
+
+ def password_reset_token_valid_time
+ valid_hours = Devise.reset_password_within / 60 / 60
+ if valid_hours >= 24
+ unit = 'day'
+ valid_length = (valid_hours / 24).floor
+ else
+ unit = 'hour'
+ valid_length = valid_hours.floor
+ end
+
+ pluralize(valid_length, unit)
+ end
+
+ def reset_token_expire_message
+ link_tag = link_to('request a new one', new_user_password_url(user_email: @user.email))
+ msg = "This link is valid for #{password_reset_token_valid_time}. "
+ msg << "After it expires, you can #{link_tag}."
+ end
end
diff --git a/app/views/devise/passwords/new.html.haml b/app/views/devise/passwords/new.html.haml
index e8820daf58f..29ffe8a8be3 100644
--- a/app/views/devise/passwords/new.html.haml
+++ b/app/views/devise/passwords/new.html.haml
@@ -6,7 +6,7 @@
.devise-errors
= devise_error_messages!
.clearfix.append-bottom-20
- = f.email_field :email, placeholder: "Email", class: "form-control", required: true
+ = f.email_field :email, placeholder: "Email", class: "form-control", required: true, value: params[:user_email]
.clearfix
= f.submit "Reset password", class: "btn-primary btn"
diff --git a/app/views/notify/new_user_email.html.haml b/app/views/notify/new_user_email.html.haml
index ebbe98dd472..4feacdaacff 100644
--- a/app/views/notify/new_user_email.html.haml
+++ b/app/views/notify/new_user_email.html.haml
@@ -11,4 +11,6 @@
- if @user.created_by_id
%p
- = link_to "Click here to set your password", edit_password_url(@user, :reset_password_token => @token)
+ = link_to "Click here to set your password", edit_password_url(@user, reset_password_token: @token)
+ %p
+ = reset_token_expire_message
diff --git a/app/views/notify/new_user_email.text.erb b/app/views/notify/new_user_email.text.erb
index 96b26879a77..dd9b71e3b84 100644
--- a/app/views/notify/new_user_email.text.erb
+++ b/app/views/notify/new_user_email.text.erb
@@ -5,4 +5,6 @@ The Administrator created an account for you. Now you are a member of the compan
login.................. <%= @user.email %>
<% if @user.created_by_id %>
<%= link_to "Click here to set your password", edit_password_url(@user, :reset_password_token => @token) %>
+
+ <%= reset_token_expire_message %>
<% end %>
diff --git a/spec/helpers/emails_helper_spec.rb b/spec/helpers/emails_helper_spec.rb
new file mode 100644
index 00000000000..7a3e38d7e63
--- /dev/null
+++ b/spec/helpers/emails_helper_spec.rb
@@ -0,0 +1,46 @@
+require 'spec_helper'
+
+describe EmailsHelper do
+ describe 'password_reset_token_valid_time' do
+ def validate_time_string(time_limit, expected_string)
+ Devise.reset_password_within = time_limit
+ expect(password_reset_token_valid_time).to eq(expected_string)
+ end
+
+ context 'when time limit is less than 2 hours' do
+ it 'should display the time in hours using a singular unit' do
+ validate_time_string(1.hour, '1 hour')
+ end
+ end
+
+ context 'when time limit is 2 or more hours' do
+ it 'should display the time in hours using a plural unit' do
+ validate_time_string(2.hours, '2 hours')
+ end
+ end
+
+ context 'when time limit contains fractions of an hour' do
+ it 'should round down to the nearest hour' do
+ validate_time_string(96.minutes, '1 hour')
+ end
+ end
+
+ context 'when time limit is 24 or more hours' do
+ it 'should display the time in days using a singular unit' do
+ validate_time_string(24.hours, '1 day')
+ end
+ end
+
+ context 'when time limit is 2 or more days' do
+ it 'should display the time in days using a plural unit' do
+ validate_time_string(2.days, '2 days')
+ end
+ end
+
+ context 'when time limit contains fractions of a day' do
+ it 'should round down to the nearest day' do
+ validate_time_string(57.hours, '2 days')
+ end
+ end
+ end
+end
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index dbcf7286e45..37607b55eb1 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -5,6 +5,8 @@ describe Notify do
include EmailSpec::Matchers
include RepoHelpers
+ new_user_address = 'newguy@example.com'
+
let(:gitlab_sender_display_name) { Gitlab.config.gitlab.email_display_name }
let(:gitlab_sender) { Gitlab.config.gitlab.email_from }
let(:gitlab_sender_reply_to) { Gitlab.config.gitlab.email_reply_to }
@@ -55,18 +57,9 @@ describe Notify do
end
end
- describe 'for new users, the email' do
- let(:example_site_path) { root_path }
- let(:new_user) { create(:user, email: 'newguy@example.com', created_by_id: 1) }
-
- token = 'kETLwRaayvigPq_x3SNM'
-
- subject { Notify.new_user_email(new_user.id, token) }
-
- it_behaves_like 'an email sent from GitLab'
-
+ shared_examples 'a new user email' do |user_email, site_path|
it 'is sent to the new user' do
- is_expected.to deliver_to new_user.email
+ is_expected.to deliver_to user_email
end
it 'has the correct subject' do
@@ -74,9 +67,25 @@ describe Notify do
end
it 'contains the new user\'s login name' do
- is_expected.to have_body_text /#{new_user.email}/
+ is_expected.to have_body_text /#{user_email}/
end
+ it 'includes a link to the site' do
+ is_expected.to have_body_text /#{site_path}/
+ end
+ end
+
+ describe 'for new users, the email' do
+ let(:example_site_path) { root_path }
+ let(:new_user) { create(:user, email: new_user_address, created_by_id: 1) }
+
+ token = 'kETLwRaayvigPq_x3SNM'
+
+ subject { Notify.new_user_email(new_user.id, token) }
+
+ it_behaves_like 'an email sent from GitLab'
+ it_behaves_like 'a new user email', new_user_address
+
it 'contains the password text' do
is_expected.to have_body_text /Click here to set your password/
end
@@ -88,39 +97,26 @@ describe Notify do
)
end
- it 'includes a link to the site' do
- is_expected.to have_body_text /#{example_site_path}/
+ it 'explains the reset link expiration' do
+ is_expected.to have_body_text(/This link is valid for \d+ (hours?|days?)/)
+ is_expected.to have_body_text(new_user_password_url)
+ is_expected.to have_body_text(/\?user_email=.*%40.*/)
end
end
describe 'for users that signed up, the email' do
let(:example_site_path) { root_path }
- let(:new_user) { create(:user, email: 'newguy@example.com', password: "securePassword") }
+ let(:new_user) { create(:user, email: new_user_address, password: "securePassword") }
subject { Notify.new_user_email(new_user.id) }
it_behaves_like 'an email sent from GitLab'
-
- it 'is sent to the new user' do
- is_expected.to deliver_to new_user.email
- end
-
- it 'has the correct subject' do
- is_expected.to have_subject /^Account was created for you$/i
- end
-
- it 'contains the new user\'s login name' do
- is_expected.to have_body_text /#{new_user.email}/
- end
+ it_behaves_like 'a new user email', new_user_address
it 'should not contain the new user\'s password' do
is_expected.not_to have_body_text /password/
end
-
- it 'includes a link to the site' do
- is_expected.to have_body_text /#{example_site_path}/
- end
end
describe 'user added ssh key' do
View Lorry Controller Status