password reset tokens expire after two hours (configurable)

test plan:
 - click "Forgot password" on the login screen and enter
   a valid login for a user
 - the reset password link sent to the user should work
   as expected (you can use /users/X/messages as an admin
   in a separate browser to get the link for testing purposes)
   - after the password is reset, you should be returned to the
     login screen with a flash message indicating your password
     was reset.
 - click "Forgot password" again, but wait > 2 hours
   before trying to use the reset-password link
   (or change the timeout by using the console to, e.g., 5 minutes:
    Setting.set('password_reset_token_expiration_minutes', '5')
    Note that this expiration applies only to password resets
    requested after the setting is changed.)
 - you should get a message indicating the code is no longer
   valid

fixes CNVS-2757

Change-Id: I6c3beec9a5b59e089997c46d518645357eec3b08
Reviewed-on: https://gerrit.instructure.com/119868
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Heath Hales <hhales@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
Jeremy Stanley 2017-07-19 15:38:59 -06:00
parent d29e3b0b4f
commit 030c2f422f
6 changed files with 59 additions and 7 deletions

View File

@ -29,6 +29,7 @@ class Login::CanvasController < ApplicationController
@pseudonym_session = PseudonymSession.new
@headers = false
flash.now[:error] = params[:message] if params[:message]
flash.now[:notice] = t('Your password has been changed.') if params[:password_changed] == '1'
maybe_render_mobile_login
end

View File

@ -118,8 +118,12 @@ class PseudonymsController < ApplicationController
# and finish the registration process?
if !@cc || @cc.path_type != 'email'
flash[:error] = t 'errors.cant_change_password', "Cannot change the password for that login, or login does not exist"
redirect_to root_url
redirect_to canvas_login_url
else
if @cc.confirmation_code_expires_at.present? && @cc.confirmation_code_expires_at <= Time.now.utc
flash[:error] = t 'The link you used has expired. Click "Forgot Password?" to get a new reset-password link.'
redirect_to canvas_login_url
end
@password_pseudonyms = @cc.user.pseudonyms.active.select{|p| p.account.canvas_authentication? }
js_env :PASSWORD_POLICY => @domain_root_account.password_policy,
:PASSWORD_POLICIES => Hash[@password_pseudonyms.map{ |p| [p.id, p.account.password_policy]}]
@ -128,7 +132,8 @@ class PseudonymsController < ApplicationController
def change_password
@pseudonym = Pseudonym.find(params[:pseudonym][:id] || params[:pseudonym_id])
if @cc = @pseudonym.user.communication_channels.where(confirmation_code: params[:nonce]).first
if @cc = @pseudonym.user.communication_channels.where(confirmation_code: params[:nonce]).
where('confirmation_code_expires_at IS NULL OR confirmation_code_expires_at > ?', Time.now.utc).first
@pseudonym.require_password = true
@pseudonym.password = params[:pseudonym][:password]
@pseudonym.password_confirmation = params[:pseudonym][:password_confirmation]
@ -145,13 +150,11 @@ class PseudonymsController < ApplicationController
reset_session
@pseudonym_session = PseudonymSession.new(@pseudonym, true)
flash[:notice] = t 'notices.password_changed', "Password changed"
render :json => @pseudonym, :status => :ok # -> dashboard
else
render :json => {:pseudonym => @pseudonym.errors.as_json[:errors]}, :status => :bad_request
end
else
flash[:notice] = t 'notices.link_invalid', "The link you used is no longer valid. If you can't log in, click \"Don't know your password?\" to reset your password."
render :json => {:errors => {:nonce => 'expired'}}, :status => :bad_request # -> login url
end
end

View File

@ -28,7 +28,7 @@ $form.formSubmit({
return registrationErrors(errors, ENV.PASSWORD_POLICIES[pseudonymId] != null ? ENV.PASSWORD_POLICIES[pseudonymId] : ENV.PASSWORD_POLICY)
},
success () {
location.href = '/login/canvas'
location.href = '/login/canvas?password_changed=1'
},
error (errors) {
if (errors.nonce) location.href = '/login/canvas'

View File

@ -253,7 +253,7 @@ class CommunicationChannel < ActiveRecord::Base
def forgot_password!
@request_password = true
set_confirmation_code(true)
set_confirmation_code(true, Setting.get('password_reset_token_expiration_minutes', '120').to_i.minutes.from_now)
self.save!
@request_password = false
end
@ -301,13 +301,14 @@ class CommunicationChannel < ActiveRecord::Base
# works. If you are resetting the confirmation_code, call @cc.
# set_confirmation_code(true), or just save the record to leave the old
# confirmation code in place.
def set_confirmation_code(reset=false)
def set_confirmation_code(reset=false, expires_at=nil)
self.confirmation_code = nil if reset
if self.path_type == TYPE_EMAIL or self.path_type.nil?
self.confirmation_code ||= CanvasSlug.generate(nil, 25)
else
self.confirmation_code ||= CanvasSlug.generate
end
self.confirmation_code_expires_at = expires_at if reset
true
end

View File

@ -0,0 +1,24 @@
#
# Copyright (C) 2017 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
class AddConfirmationCodeExpiresAtToCommunicationChannels < ActiveRecord::Migration[5.0]
tag :predeploy
def change
add_column :communication_channels, :confirmation_code_expires_at, :datetime
end
end

View File

@ -46,6 +46,7 @@ describe PseudonymsController do
@cc.confirm
@cc.reload
expect(@cc).to be_active
expect(@cc.confirmation_code_expires_at).to be_nil
pword = @pseudonym.crypted_password
code = @cc.confirmation_code
post 'change_password', params: {:pseudonym_id => @pseudonym.id, :nonce => @cc.confirmation_code, :pseudonym => {:password => '12341234', :password_confirmation => '12341234'}}
@ -73,6 +74,21 @@ describe PseudonymsController do
expect(@cc).not_to be_active
end
it "accepts a non-expired password-change token" do
Setting.set('password_reset_token_expiration_minutes', '60')
@cc.forgot_password!
expect(@cc.confirmation_code_expires_at).to be_between(58.minutes.from_now, 62.minutes.from_now)
post 'change_password', :params => { :pseudonym_id => @pseudonym.id, :nonce => @cc.confirmation_code, :pseudonym => {:password => '12341234', :password_confirmation => '12341234'} }
expect(response).to be_success
end
it "rejects an expired password-change token" do
@cc.forgot_password!
@cc.update_attributes :confirmation_code_expires_at => 1.hour.ago
post 'change_password', :params => { :pseudonym_id => @pseudonym.id, :nonce => @cc.confirmation_code, :pseudonym => {:password => '12341234', :password_confirmation => '12341234'} }
assert_status(400)
end
describe "forgot password" do
before :once do
Notification.create(:name => 'Forgot Password')
@ -132,6 +148,13 @@ describe PseudonymsController do
get 'confirm_change_password', params: {:pseudonym_id => @pseudonym.id, :nonce => @cc.confirmation_code}
expect(response).to be_success
end
it "should not render confirm change password view if token is expired" do
@user.register
@cc.update_attributes :confirmation_code_expires_at => 1.hour.ago
get 'confirm_change_password', :params => { :pseudonym_id => @pseudonym.id, :nonce => @cc.confirmation_code }
expect(response).to be_redirect
end
end
describe "destroy" do