Fix updating password

Signed-off-by: Reinier Balt <lrbalt@gmail.com>
This commit is contained in:
Reinier Balt 2011-09-09 17:49:42 +02:00
parent 50875cfa40
commit 998c14fa71
7 changed files with 83 additions and 74 deletions

View file

@ -1,5 +1,4 @@
source :gemcutter
source :rubyforge
source "http://gems.github.com/"
gem "rake", "~>0.8.7"

View file

@ -7,7 +7,6 @@ GIT
activerecord (>= 2.3.2)
GEM
remote: http://rubygems.org/
remote: http://rubygems.org/
remote: http://gems.github.com/
specs:

View file

@ -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

View file

@ -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"

View file

@ -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

View file

@ -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

View file

@ -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