From cde02954c84591c66c575f0d76d44c18ab6edf95 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 12 Dec 2010 13:39:55 +0000 Subject: [PATCH] Moves password param to user hash param so that it can be set using the User API. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4493 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/users_controller.rb | 14 +++++++++----- app/views/users/_form.rhtml | 6 ++---- test/functional/users_controller_test.rb | 15 ++++++++++----- test/integration/api_test/users_test.rb | 5 +++-- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 07c807ce..9c7f2a17 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -101,7 +101,7 @@ class UsersController < ApplicationController @user.safe_attributes = params[:user] @user.admin = params[:user][:admin] || false @user.login = params[:user][:login] - @user.password, @user.password_confirmation = params[:password], params[:password_confirmation] unless @user.auth_source_id + @user.password, @user.password_confirmation = params[:user][:password], params[:user][:password_confirmation] unless @user.auth_source_id # TODO: Similar to My#account @user.mail_notification = params[:notification_option] || 'only_my_events' @@ -127,6 +127,8 @@ class UsersController < ApplicationController else @auth_sources = AuthSource.find(:all) @notification_option = @user.mail_notification + # Clear password input + @user.password = @user.password_confirmation = nil respond_to do |format| format.html { render :action => 'new' } @@ -152,8 +154,8 @@ class UsersController < ApplicationController @user.admin = params[:user][:admin] if params[:user][:admin] @user.login = params[:user][:login] if params[:user][:login] - if params[:password].present? && (@user.auth_source_id.nil? || params[:user][:auth_source_id].blank?) - @user.password, @user.password_confirmation = params[:password], params[:password_confirmation] + if params[:user][:password].present? && (@user.auth_source_id.nil? || params[:user][:auth_source_id].blank?) + @user.password, @user.password_confirmation = params[:user][:password], params[:user][:password_confirmation] end @user.group_ids = params[:user][:group_ids] if params[:user][:group_ids] @user.safe_attributes = params[:user] @@ -170,8 +172,8 @@ class UsersController < ApplicationController if was_activated Mailer.deliver_account_activated(@user) - elsif @user.active? && params[:send_information] && !params[:password].blank? && @user.auth_source_id.nil? - Mailer.deliver_account_information(@user, params[:password]) + elsif @user.active? && params[:send_information] && !params[:user][:password].blank? && @user.auth_source_id.nil? + Mailer.deliver_account_information(@user, params[:user][:password]) end respond_to do |format| @@ -184,6 +186,8 @@ class UsersController < ApplicationController else @auth_sources = AuthSource.find(:all) @membership ||= Member.new + # Clear password input + @user.password = @user.password_confirmation = nil respond_to do |format| format.html { render :action => :edit } diff --git a/app/views/users/_form.rhtml b/app/views/users/_form.rhtml index 7e50fcdc..4e788f6c 100644 --- a/app/views/users/_form.rhtml +++ b/app/views/users/_form.rhtml @@ -25,11 +25,9 @@

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

<% end %>
-

-<%= password_field_tag 'password', nil, :size => 25 %>
+

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

-

-<%= password_field_tag 'password_confirmation', nil, :size => 25 %>

+

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

diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 100ec8de..8b879cc3 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -152,6 +152,11 @@ class UsersControllerTest < ActionController::TestCase user = User.last assert_equal 'none', user.mail_notification end + + should 'set the password' do + user = User.first(:order => 'id DESC') + assert user.check_password?('test') + end end context "when unsuccessful" do @@ -194,13 +199,13 @@ class UsersControllerTest < ActionController::TestCase assert mail.body.include?(ll('fr', :notice_account_activated)) end - def test_updat_with_password_change_should_send_a_notification + def test_update_with_password_change_should_send_a_notification ActionMailer::Base.deliveries.clear Setting.bcc_recipients = '1' + put :update, :id => 2, :user => {:password => 'newpass', :password_confirmation => 'newpass'}, :send_information => '1' u = User.find(2) - put :update, :id => u.id, :user => {}, :password => 'newpass', :password_confirmation => 'newpass', :send_information => '1' - assert_equal User.hash_password('newpass'), u.reload.hashed_password + assert u.check_password?('newpass') mail = ActionMailer::Base.deliveries.last assert_not_nil mail @@ -214,10 +219,10 @@ class UsersControllerTest < ActionController::TestCase u.auth_source = AuthSource.find(1) u.save! - put :update, :id => u.id, :user => {:auth_source_id => ''}, :password => 'newpass', :password_confirmation => 'newpass' + put :update, :id => u.id, :user => {:auth_source_id => '', :password => 'newpass'}, :password_confirmation => 'newpass' assert_equal nil, u.reload.auth_source - assert_equal User.hash_password('newpass'), u.reload.hashed_password + assert u.check_password?('newpass') end def test_edit_membership diff --git a/test/integration/api_test/users_test.rb b/test/integration/api_test/users_test.rb index 1fc4b203..83be388a 100644 --- a/test/integration/api_test/users_test.rb +++ b/test/integration/api_test/users_test.rb @@ -54,13 +54,13 @@ class ApiTest::UsersTest < ActionController::IntegrationTest context "POST /users" do context "with valid parameters" do setup do - @parameters = {:user => {:login => 'foo', :firstname => 'Firstname', :lastname => 'Lastname', :mail => 'foo@example.net'}} + @parameters = {:user => {:login => 'foo', :firstname => 'Firstname', :lastname => 'Lastname', :mail => 'foo@example.net', :password => 'secret'}} end context ".xml" do should_allow_api_authentication(:post, '/users.xml', - {:user => {:login => 'foo', :firstname => 'Firstname', :lastname => 'Lastname', :mail => 'foo@example.net'}}, + {:user => {:login => 'foo', :firstname => 'Firstname', :lastname => 'Lastname', :mail => 'foo@example.net', :password => 'secret'}}, {:success_code => :created}) should "create a user with the attributes" do @@ -74,6 +74,7 @@ class ApiTest::UsersTest < ActionController::IntegrationTest assert_equal 'Lastname', user.lastname assert_equal 'foo@example.net', user.mail assert !user.admin? + assert user.check_password?('secret') assert_response :created assert_equal 'application/xml', @response.content_type