diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 23a979ea2..2cc43919c 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -80,6 +80,7 @@ class UsersController < ApplicationController def new @user = User.new(:language => Setting.default_language, :mail_notification => Setting.default_notification_option) + @user.safe_attributes = params[:user] @auth_sources = AuthSource.all end @@ -96,13 +97,14 @@ class UsersController < ApplicationController @user.pref.save @user.notified_project_ids = (@user.mail_notification == 'selected' ? params[:notified_project_ids] : []) - Mailer.account_information(@user, params[:user][:password]).deliver if params[:send_information] + Mailer.account_information(@user, @user.password).deliver if params[:send_information] respond_to do |format| format.html { flash[:notice] = l(:notice_user_successful_create, :id => view_context.link_to(@user.login, user_path(@user))) if params[:continue] - redirect_to new_user_path + attrs = params[:user].slice(:generate_password) + redirect_to new_user_path(:user => attrs) else redirect_to edit_user_path(@user) end @@ -145,8 +147,8 @@ class UsersController < ApplicationController if was_activated Mailer.account_activated(@user).deliver - elsif @user.active? && params[:send_information] && !params[:user][:password].blank? && @user.auth_source_id.nil? - Mailer.account_information(@user, params[:user][:password]).deliver + elsif @user.active? && params[:send_information] && @user.password.present? && @user.auth_source_id.nil? + Mailer.account_information(@user, @user.password).deliver end respond_to do |format| diff --git a/app/models/user.rb b/app/models/user.rb index 3670ad2a1..3acd4bbe2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -81,7 +81,7 @@ class User < Principal acts_as_customizable - attr_accessor :password, :password_confirmation + attr_accessor :password, :password_confirmation, :generate_password attr_accessor :last_before_login_on # Prevents unauthorized assignments attr_protected :login, :admin, :password, :password_confirmation, :hashed_password @@ -103,7 +103,7 @@ class User < Principal validate :validate_password_length before_create :set_mail_notification - before_save :update_hashed_password + before_save :generate_password_if_needed, :update_hashed_password before_destroy :remove_references_before_destroy scope :in_group, lambda {|group| @@ -274,13 +274,16 @@ class User < Principal return auth_source.allow_password_changes? end - # Generate and set a random password. Useful for automated user creation - # Based on Token#generate_token_value - # - def random_password + def generate_password? + generate_password == '1' || generate_password == true + end + + # Generate and set a random password on given length + def random_password(length=40) chars = ("a".."z").to_a + ("A".."Z").to_a + ("0".."9").to_a + chars -= %w(0 O 1 l) password = '' - 40.times { |i| password << chars[rand(chars.size-1)] } + length.times {|i| password << chars[SecureRandom.random_number(chars.size)] } self.password = password self.password_confirmation = password self @@ -541,6 +544,7 @@ class User < Principal safe_attributes 'status', 'auth_source_id', + 'generate_password', :if => lambda {|user, current_user| current_user.admin?} safe_attributes 'group_ids', @@ -610,6 +614,7 @@ class User < Principal protected def validate_password_length + return if password.blank? && generate_password? # Password length validation based on setting if !password.nil? && password.size < Setting.password_min_length.to_i errors.add(:password, :too_short, :count => Setting.password_min_length.to_i) @@ -618,6 +623,13 @@ class User < Principal private + def generate_password_if_needed + if generate_password? && auth_source.nil? + length = [Setting.password_min_length.to_i + 2, 10].max + random_password(length) + end + end + # Removes references that are not handled by associations # Things that are not deleted are reassociated with the anonymous user def remove_references_before_destroy diff --git a/app/views/users/_form.html.erb b/app/views/users/_form.html.erb index ddc826fa8..57c2f29b2 100644 --- a/app/views/users/_form.html.erb +++ b/app/views/users/_form.html.erb @@ -28,6 +28,7 @@

<%= 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 %>

@@ -49,3 +50,16 @@
+ +<%= javascript_tag do %> +$(document).ready(function(){ + $('#user_generate_password').change(function(){ + var passwd = $('#user_password, #user_password_confirmation'); + if ($(this).is(':checked')){ + passwd.val('').attr('disabled', true); + }else{ + passwd.removeAttr('disabled'); + } + }).trigger('change'); +}); +<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 39583b583..4dcc3defd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -331,6 +331,7 @@ en: field_board_parent: Parent forum field_private_notes: Private notes field_inherit_members: Inherit members + field_generate_password: Generate password setting_app_title: Application title setting_app_subtitle: Application subtitle diff --git a/config/locales/fr.yml b/config/locales/fr.yml index d9d79938b..9d74f31ec 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -331,6 +331,7 @@ fr: field_board_parent: Forum parent field_private_notes: Notes privées field_inherit_members: Hériter les membres + field_generate_password: Générer un mot de passe setting_app_title: Titre de l'application setting_app_subtitle: Sous-titre de l'application diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 37e58d500..65aa2d7c0 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -218,6 +218,30 @@ class UsersControllerTest < ActionController::TestCase assert_equal '0', user.pref[:warn_on_leaving_unsaved] end + def test_create_with_generate_password_should_email_the_password + assert_difference 'User.count' do + post :create, :user => { + :login => 'randompass', + :firstname => 'Random', + :lastname => 'Pass', + :mail => 'randompass@example.net', + :language => 'en', + :generate_password => '1', + :password => '', + :password_confirmation => '' + }, :send_information => 1 + end + user = User.order('id DESC').first + assert_equal 'randompass', user.login + + mail = ActionMailer::Base.deliveries.last + assert_not_nil mail + m = mail_body(mail).match(/Password: ([a-zA-Z0-9]+)/) + assert m + password = m[1] + assert user.check_password?(password) + end + def test_create_with_failure assert_no_difference 'User.count' do post :create, :user => {} @@ -290,6 +314,37 @@ class UsersControllerTest < ActionController::TestCase assert_mail_body_match 'newpass123', mail end + def test_update_with_generate_password_should_email_the_password + ActionMailer::Base.deliveries.clear + Setting.bcc_recipients = '1' + + put :update, :id => 2, :user => { + :generate_password => '1', + :password => '', + :password_confirmation => '' + }, :send_information => '1' + + mail = ActionMailer::Base.deliveries.last + assert_not_nil mail + m = mail_body(mail).match(/Password: ([a-zA-Z0-9]+)/) + assert m + password = m[1] + assert User.find(2).check_password?(password) + end + + def test_update_without_generate_password_should_not_change_password + put :update, :id => 2, :user => { + :firstname => 'changed', + :generate_password => '0', + :password => '', + :password_confirmation => '' + }, :send_information => '1' + + user = User.find(2) + assert_equal 'changed', user.firstname + assert user.check_password?('jsmith') + end + def test_update_user_switchin_from_auth_source_to_password_authentication # Configure as auth source u = User.find(2) diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 28d09518e..9ea57c722 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -70,6 +70,27 @@ class UserTest < ActiveSupport::TestCase assert user.save end + def test_generate_password_on_create_should_set_password + user = User.new(:firstname => "new", :lastname => "user", :mail => "newuser@somenet.foo") + user.login = "newuser" + user.generate_password = true + assert user.save + + password = user.password + assert user.check_password?(password) + end + + def test_generate_password_on_update_should_update_password + user = User.find(2) + hash = user.hashed_password + user.generate_password = true + assert user.save + + password = user.password + assert user.check_password?(password) + assert_not_equal hash, user.reload.hashed_password + end + def test_create user = User.new(:firstname => "new", :lastname => "user", :mail => "newuser@somenet.foo")