diff --git a/Gemfile b/Gemfile index 0ba84054..4964ccb3 100644 --- a/Gemfile +++ b/Gemfile @@ -1,5 +1,4 @@ source :gemcutter -source :rubyforge source "http://gems.github.com/" gem "rake", "~>0.8.7" diff --git a/Gemfile.lock b/Gemfile.lock index 7ca0f70e..b3d1d5d5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,7 +7,6 @@ GIT activerecord (>= 2.3.2) GEM - remote: http://rubygems.org/ remote: http://rubygems.org/ remote: http://gems.github.com/ specs: diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 30931609..7b0eb8b7 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -4,7 +4,7 @@ class UsersController < ApplicationController skip_before_filter :check_for_deprecated_password_hash, :only => [ :change_password, :update_password ] prepend_before_filter :login_optional, :only => [ :new, :create ] - + # GET /users GET /users.xml def index respond_to do |format| @@ -22,7 +22,7 @@ class UsersController < ApplicationController end end end - + # GET /users/id GET /users/id.xml def show @user = User.find_by_id(params[:id]) @@ -54,7 +54,7 @@ class UsersController < ApplicationController end render :layout => "login" end - + # Example usage: curl -H 'Accept: application/xml' -H 'Content-Type: # application/xml' # -u admin:up2n0g00d @@ -132,14 +132,14 @@ class UsersController < ApplicationController return end end - end - + end + # DELETE /users/id DELETE /users/id.xml def destroy @deleted_user = User.find_by_id(params[:id]) @saved = @deleted_user.destroy @total_users = User.find(:all).size - + respond_to do |format| format.html do if @saved @@ -153,14 +153,15 @@ class UsersController < ApplicationController format.xml { head :ok } end end - - + + def change_password @page_title = t('users.change_password_title') end - + def update_password - @user.change_password(params[:updateuser][:password], params[:updateuser][:password_confirmation]) + # is used for focing password change after sha->bcrypt upgrade + @user.change_password(params[:user][:password], params[:user][:password_confirmation]) notify :notice, t('users.password_updated') redirect_to preferences_path rescue Exception => error @@ -171,7 +172,7 @@ class UsersController < ApplicationController def change_auth_type @page_title = t('users.change_auth_type_title') end - + def update_auth_type if (params[:open_id_complete] || (params[:user][:auth_type] == 'open_id')) && openid_enabled? authenticate_with_open_id do |result, identity_url| @@ -203,16 +204,16 @@ class UsersController < ApplicationController redirect_to :action => 'change_auth_type' end end - + def refresh_token @user.generate_token @user.save! notify :notice, t('users.new_token_generated') redirect_to preferences_path end - + private - + def get_new_user if session['new_user'] user = session['new_user'] @@ -222,7 +223,7 @@ class UsersController < ApplicationController end user end - + def check_create_user_params return false unless params.has_key?(:request) return false unless params[:request].has_key?(:login) @@ -231,5 +232,5 @@ class UsersController < ApplicationController return false if params[:request][:password].empty? return true end - + end diff --git a/features/handling_users_with_deprecated_password_hashes.feature b/features/handling_users_with_deprecated_password_hashes.feature index 353afec5..4b1b8203 100644 --- a/features/handling_users_with_deprecated_password_hashes.feature +++ b/features/handling_users_with_deprecated_password_hashes.feature @@ -4,10 +4,10 @@ Feature: Handling users with deprecated passwords hashes I have to be redirected to the password resetting form Background: - Given the following user records - | login | password_with_algorithm | - | new_hash_user | first_secret bcrypt | - | old_hash_user | another_secret sha1 | + Given the following user records with hash algorithm + | login | password | algorithm | + | new_hash_user | first_secret | bcrypt | + | old_hash_user | another_secret | sha1 | Scenario Outline: A user with SHA1 password Given I have logged in as "old_hash_user" with password "another_secret" diff --git a/features/step_definitions/user_steps.rb b/features/step_definitions/user_steps.rb index 117b2b66..f11586cb 100644 --- a/features/step_definitions/user_steps.rb +++ b/features/step_definitions/user_steps.rb @@ -6,6 +6,40 @@ Given /^the following user records?$/ do |table| end end +Given /^the following user records with hash algorithm$/ do |table| + User.delete_all + table.hashes.each do | hash | + password = hash[:password] + algorithm = hash[:algorithm] + hash.delete("algorithm") + + user = Factory(:user, hash) + + case algorithm + when 'bcrypt' + user.change_password( password, password ) + user.reload + BCrypt::Password.new(user.crypted_password).should == password + when 'sha1' + user.password = user.password_confirmation = nil + user.write_attribute :crypted_password, User.sha1( password ) + user.save + user.reload + user.crypted_password.should == User.sha1(password) + else + raise "Unknown hashing algorithm: #{algorithm}" + end + + user.create_preference({:locale => 'en'}) + end +end + +When /^I change my password to "([^"]*)"$/ do |password| + Then 'I should be on the change password page' + %w{password password_confirmation}.each { |name| fill_in "user[#{name}]", :with => password } + click_button +end + Given "no users exists" do User.delete_all end @@ -32,9 +66,3 @@ Then "I should be an admin" do # just check on the presence of the menu item for managing users Then "I should see \"Manage users\"" end - -When /^I change my password to "([^"]*)"$/ do |password| - Then 'I should be on the change password page' - %w{new confirm}.each { |name| fill_in name + ' password', :with => password } - click_button -end diff --git a/features/support/user.rb b/features/support/user.rb deleted file mode 100644 index e40d3e89..00000000 --- a/features/support/user.rb +++ /dev/null @@ -1,18 +0,0 @@ -class User - # A method used in features' user records definitions. It accepts a string - # with a password and the name of a hashing algorithm ('sha1' or 'bcrypt') - # concatenated with a space. It encrypts user's password using the given - # mechanism and the given password value. - def password_with_algorithm=(x) - pass, algorithm = *x.split - case algorithm - when 'bcrypt' - change_password pass, pass - when 'sha1' - self.crypted_password = User.sha1 pass - self.password = self.password_confirmation = nil - else - raise "Unknown hashing algorithm: #{algorithm}" - end - end -end diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 17c4f31b..d9b461aa 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -6,7 +6,7 @@ class UsersController; def rescue_action(e) raise e end; end class UsersControllerTest < ActionController::TestCase fixtures :preferences, :users - + def setup assert_equal "test", ENV['RAILS_ENV'] assert_equal "change-me", Tracks::Config.salt @@ -14,23 +14,23 @@ class UsersControllerTest < ActionController::TestCase @request = ActionController::TestRequest.new @response = ActionController::TestResponse.new end - + def test_get_index_when_not_logged_in get :index assert_redirected_to :controller => 'login', :action => 'login' end - + def test_get_index_by_nonadmin login_as :other_user get :index assert_response 401 end - + def test_get_index_by_admin login_as :admin_user get :index assert_response :success - assert_equal "TRACKS::Manage Users", assigns['page_title'] + assert_equal "TRACKS::Manage Users", assigns['page_title'] assert_equal 5, assigns['total_users'] assert_equal "/users", session['return-to'] end @@ -48,7 +48,7 @@ class UsersControllerTest < ActionController::TestCase get :index, :page => 2 assert_equal assigns['users'],[User.find_by_login('jane')] end - + def test_destroy_user login_as :admin_user @no_users_before = User.find(:all).size @@ -56,7 +56,7 @@ class UsersControllerTest < ActionController::TestCase xhr :post, :destroy, :id => user_id.to_param assert_equal @no_users_before-1, User.find(:all).size end - + def test_update_password_successful get :change_password # should fail because no login assert_redirected_to :controller => 'login', :action => 'login' @@ -64,29 +64,29 @@ class UsersControllerTest < ActionController::TestCase @user = @request.session['user_id'] get :change_password # should now pass because we're logged in assert_response :success - assert_equal assigns['page_title'], "TRACKS::Change password" - post :update_password, :updateuser => {:password => 'newpassword', :password_confirmation => 'newpassword'} + assert_equal assigns['page_title'], "TRACKS::Change password" + post :update_password, :user => {:password => 'newpassword', :password_confirmation => 'newpassword'} assert_redirected_to preferences_path @updated_user = User.find(users(:admin_user).id) assert_not_nil User.authenticate(@updated_user.login, 'newpassword') assert_equal "Password updated.", flash[:notice] end - + def test_update_password_no_confirmation post :update_password # should fail because no login assert_redirected_to :controller => 'login', :action => 'login' login_as :admin_user - post :update_password, :updateuser => {:password => 'newpassword', :password_confirmation => 'wrong'} + post :update_password, :user => {:password => 'newpassword', :password_confirmation => 'wrong'} assert_redirected_to :controller => 'users', :action => 'change_password' assert users(:admin_user).save, false assert_equal 'Validation failed: Password doesn\'t match confirmation', flash[:error] end - + def test_update_password_validation_errors post :update_password # should fail because no login assert_redirected_to :controller => 'login', :action => 'login' login_as :admin_user - post :update_password, :updateuser => {:password => 'ba', :password_confirmation => 'ba'} + post :update_password, :user => {:password => 'ba', :password_confirmation => 'ba'} assert_redirected_to :controller => 'users', :action => 'change_password' assert users(:admin_user).save, false # For some reason, no errors are being raised now. @@ -94,11 +94,11 @@ class UsersControllerTest < ActionController::TestCase #assert_equal users(:admin_user).errors.on(:password), "is too short (min is 5 characters)" assert_equal 'Validation failed: Password is too short (minimum is 5 characters)', flash[:error] end - + # ============================================ # Signup and creation of new users # ============================================ - + def test_create_adds_a_new_nonadmin_user login_as :admin_user post :create, :user => {:login => 'newbie', :password => 'newbiepass', :password_confirmation => 'newbiepass'} @@ -108,28 +108,28 @@ class UsersControllerTest < ActionController::TestCase assert_not_nil newbie.preference # have user preferences been created? assert_not_nil User.authenticate('newbie', 'newbiepass') end - + def test_create_redirects_to_home_page login_as :admin_user post :create, :user => {:login => 'newbie', :password => 'newbiepass', :password_confirmation => 'newbiepass'} assert_redirected_to home_url end - + def test_create_sets_flash_message login_as :admin_user post :create, :user => {:login => 'newbie', :password => 'newbiepass', :password_confirmation => 'newbiepass'} assert_equal "Signup successful for user newbie.", flash[:notice], "expected flash notice not found" end - + def test_create_adds_a_user login_as :admin_user assert_difference 'User.count' do post :create, :user => {:login => 'newbie', :password => 'newbiepass', :password_confirmation => 'newbiepass'} end end - + # Test whether signup of new users is denied to a non-admin user - # + # def test_create_by_non_admin login_as :other_user assert_no_difference 'User.count' do @@ -138,47 +138,47 @@ class UsersControllerTest < ActionController::TestCase assert_response :success assert_template 'users/nosignup' end - + # ============================================ # Test validations # ============================================ - + def test_create_with_invalid_password_does_not_add_a_new_user login_as :admin_user assert_no_difference 'User.count' do post :create, :user => {:login => 'newbie', :password => '', :password_confirmation => ''} end end - + def test_create_with_invalid_password_redirects_to_new_user_page login_as :admin_user post :create, :user => {:login => 'newbie', :password => '', :password_confirmation => ''} assert_redirected_to signup_path end - + def test_create_with_invalid_login_does_not_add_a_new_user login_as :admin_user post :create, :user => {:login => 'n', :password => 'newbiepass', :password_confirmation => 'newbiepass'} assert_redirected_to signup_path end - + def test_create_with_invalid_login_redirects_to_new_user_page login_as :admin_user post :create, :user => {:login => 'n', :password => 'newbiepass', :password_confirmation => 'newbiepass'} assert_redirected_to signup_path end - + def test_create_with_duplicate_login_does_not_add_a_new_user login_as :admin_user assert_no_difference 'User.count' do post :create, :user => {:login => 'jane', :password => 'newbiepass', :password_confirmation => 'newbiepass'} end end - + def test_create_with_duplicate_login_redirects_to_new_user_page login_as :admin_user post :create, :user => {:login => 'jane', :password => 'newbiepass', :password_confirmation => 'newbiepass'} assert_redirected_to signup_path end - + end