From b764e398475c26217bcca8ac9063f053bc1cf627 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Mon, 5 Aug 2013 17:58:33 +0000 Subject: [PATCH] Option to force a user to change his password (#3872). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@12081 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/account_controller.rb | 2 +- app/controllers/application_controller.rb | 19 +++++++++++- app/controllers/my_controller.rb | 11 +++++-- app/models/user.rb | 5 ++++ app/views/my/password.html.erb | 4 +++ app/views/users/_form.html.erb | 3 +- config/locales/en.yml | 1 + config/locales/fr.yml | 1 + ...0729070143_add_users_must_change_passwd.rb | 9 ++++++ test/integration/account_test.rb | 29 +++++++++++++++++++ 10 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20130729070143_add_users_must_change_passwd.rb diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb index 7089c176f..d39fc2ace 100644 --- a/app/controllers/account_controller.rb +++ b/app/controllers/account_controller.rb @@ -20,7 +20,7 @@ class AccountController < ApplicationController include CustomFieldsHelper # prevents login action to be filtered by check_if_login_required application scope filter - skip_before_filter :check_if_login_required + skip_before_filter :check_if_login_required, :check_password_change # Login request and validation def login diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index bb8dae56f..6e53ffe01 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -38,7 +38,7 @@ class ApplicationController < ActionController::Base cookies.delete(autologin_cookie_name) end - before_filter :session_expiration, :user_setup, :check_if_login_required, :set_localization + before_filter :session_expiration, :user_setup, :check_if_login_required, :check_password_change, :set_localization rescue_from ActionController::InvalidAuthenticityToken, :with => :invalid_authenticity_token rescue_from ::Unauthorized, :with => :deny_access @@ -78,6 +78,9 @@ class ApplicationController < ActionController::Base session[:user_id] = user.id session[:ctime] = Time.now.utc.to_i session[:atime] = Time.now.utc.to_i + if user.must_change_password? + session[:pwd] = '1' + end end def user_setup @@ -112,6 +115,10 @@ class ApplicationController < ActionController::Base authenticate_with_http_basic do |username, password| user = User.try_to_login(username, password) || User.find_by_api_key(username) end + if user && user.must_change_password? + render_error :message => 'You must change your password', :status => 403 + return + end end # Switch user if requested by an admin user if user && user.admin? && (username = api_switch_user_from_request) @@ -170,6 +177,16 @@ class ApplicationController < ActionController::Base require_login if Setting.login_required? end + def check_password_change + if session[:pwd] + if User.current.must_change_password? + redirect_to my_password_path + else + session.delete(:pwd) + end + end + end + def set_localization lang = nil if User.current.logged? diff --git a/app/controllers/my_controller.rb b/app/controllers/my_controller.rb index 5328991b3..82532918a 100644 --- a/app/controllers/my_controller.rb +++ b/app/controllers/my_controller.rb @@ -17,6 +17,8 @@ class MyController < ApplicationController before_filter :require_login + # let user change his password when he has to + skip_before_filter :check_password_change, :only => :password helper :issues helper :users @@ -90,14 +92,17 @@ class MyController < ApplicationController return end if request.post? - if @user.check_password?(params[:password]) + if !@user.check_password?(params[:password]) + flash.now[:error] = l(:notice_account_wrong_password) + elsif params[:password] == params[:new_password] + flash.now[:error] = 'Your new password must be different from your current password' + else @user.password, @user.password_confirmation = params[:new_password], params[:new_password_confirmation] + @user.must_change_passwd = false if @user.save flash[:notice] = l(:notice_account_password_updated) redirect_to my_account_path end - else - flash[:error] = l(:notice_account_wrong_password) end end end diff --git a/app/models/user.rb b/app/models/user.rb index 441e6b5b8..9020295d0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -280,6 +280,10 @@ class User < Principal return auth_source.allow_password_changes? end + def must_change_password? + must_change_passwd? && change_password_allowed? + end + def generate_password? generate_password == '1' || generate_password == true end @@ -568,6 +572,7 @@ class User < Principal safe_attributes 'status', 'auth_source_id', 'generate_password', + 'must_change_passwd', :if => lambda {|user, current_user| current_user.admin?} safe_attributes 'group_ids', diff --git a/app/views/my/password.html.erb b/app/views/my/password.html.erb index de9a459e4..5dbf24cb0 100644 --- a/app/views/my/password.html.erb +++ b/app/views/my/password.html.erb @@ -17,6 +17,10 @@ <%= submit_tag l(:button_apply) %> <% end %> +<% unless @user.must_change_passwd? %> <% content_for :sidebar do %> <%= render :partial => 'sidebar' %> <% end %> +<% end %> + +<%= javascript_tag "$('#password').focus();" %> diff --git a/app/views/users/_form.html.erb b/app/views/users/_form.html.erb index 57c2f29b2..bb762c1ee 100644 --- a/app/views/users/_form.html.erb +++ b/app/views/users/_form.html.erb @@ -28,10 +28,11 @@

<%= f.select :auth_source_id, ([[l(:label_internal), ""]] + @auth_sources.collect { |a| [a.name, a.id] }), {}, :onchange => "if (this.value=='') {$('#password_fields').show();} else {$('#password_fields').hide();}" %>

<% end %>
-

<%= f.check_box :generate_password %>

<%= f.password_field :password, :required => true, :size => 25 %> <%= l(:text_caracters_minimum, :count => Setting.password_min_length) %>

<%= f.password_field :password_confirmation, :required => true, :size => 25 %>

+

<%= f.check_box :generate_password %>

+

<%= f.check_box :must_change_passwd %>

diff --git a/config/locales/en.yml b/config/locales/en.yml index 18221e837..e63a724cb 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -335,6 +335,7 @@ en: field_private_notes: Private notes field_inherit_members: Inherit members field_generate_password: Generate password + field_must_change_passwd: Must change password at next logon setting_app_title: Application title setting_app_subtitle: Application subtitle diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 13fcbf0f1..c2e72c45b 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -335,6 +335,7 @@ fr: field_private_notes: Notes privées field_inherit_members: Hériter les membres field_generate_password: Générer un mot de passe + field_must_change_passwd: Doit changer de mot de passe à la prochaine connexion setting_app_title: Titre de l'application setting_app_subtitle: Sous-titre de l'application diff --git a/db/migrate/20130729070143_add_users_must_change_passwd.rb b/db/migrate/20130729070143_add_users_must_change_passwd.rb new file mode 100644 index 000000000..13c29292e --- /dev/null +++ b/db/migrate/20130729070143_add_users_must_change_passwd.rb @@ -0,0 +1,9 @@ +class AddUsersMustChangePasswd < ActiveRecord::Migration + def up + add_column :users, :must_change_passwd, :boolean, :default => false, :null => false + end + + def down + remove_column :users, :must_change_passwd + end +end diff --git a/test/integration/account_test.rb b/test/integration/account_test.rb index 1f9741b33..2f96005f5 100644 --- a/test/integration/account_test.rb +++ b/test/integration/account_test.rb @@ -128,6 +128,35 @@ class AccountTest < ActionController::IntegrationTest assert_equal 0, Token.count end + def test_user_with_must_change_passwd_should_be_forced_to_change_its_password + User.find_by_login('jsmith').update_attribute :must_change_passwd, true + + post '/login', :username => 'jsmith', :password => 'jsmith' + assert_redirected_to '/my/page' + follow_redirect! + assert_redirected_to '/my/password' + + get '/issues' + assert_redirected_to '/my/password' + end + + def test_user_with_must_change_passwd_should_be_able_to_change_its_password + User.find_by_login('jsmith').update_attribute :must_change_passwd, true + + post '/login', :username => 'jsmith', :password => 'jsmith' + assert_redirected_to '/my/page' + follow_redirect! + assert_redirected_to '/my/password' + follow_redirect! + assert_response :success + post '/my/password', :password => 'jsmith', :new_password => 'newpassword', :new_password_confirmation => 'newpassword' + assert_redirected_to '/my/account' + follow_redirect! + assert_response :success + + assert_equal false, User.find_by_login('jsmith').must_change_passwd? + end + def test_register_with_automatic_activation Setting.self_registration = '3'