diff --git a/app/controllers/public/account_controller.rb b/app/controllers/public/account_controller.rb index b4b80a9..5775657 100644 --- a/app/controllers/public/account_controller.rb +++ b/app/controllers/public/account_controller.rb @@ -46,8 +46,12 @@ class AccountController < ApplicationController self.current_user = plugins_alternative_authentication - self.current_user ||= User.authenticate(params[:user][:login], params[:user][:password], environment) if params[:user] - + begin + self.current_user ||= User.authenticate(params[:user][:login], params[:user][:password], environment) if params[:user] + rescue User::UserNotActivated => e + session[:notice] = e.message + return + end if logged_in? check_join_in_community(self.current_user) diff --git a/app/models/user.rb b/app/models/user.rb index e613513..fe8a563 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -120,11 +120,17 @@ class User < ActiveRecord::Base validates_inclusion_of :terms_accepted, :in => [ '1' ], :if => lambda { |u| ! u.terms_of_use.blank? }, :message => N_('{fn} must be checked in order to signup.').fix_i18n + scope :has_login?, lambda { |login,email,environment_id| + where('login = ? OR email = ?', login, email). + where(environment_id: environment_id) + } + # Authenticates a user by their login name or email and unencrypted password. Returns the user or nil. def self.authenticate(login, password, environment = nil) environment ||= Environment.default - u = self.first :conditions => ['(login = ? OR email = ?) AND environment_id = ? AND activated_at IS NOT NULL', - login, login, environment.id] # need to get the salt + + u = self.has_login?(login, login, environment.id) + u = u.first if u.is_a?(ActiveRecord::Relation) u && u.authenticated?(password) ? u : nil end @@ -236,7 +242,23 @@ class User < ActiveRecord::Base password.crypt(salt) end + class UserNotActivated < StandardError + attr_reader :user + + def initialize(message, user = nil) + @user = user + + super(message) + end + end + def authenticated?(password) + + unless self.activated? + message = _('The user "%{login}" is not activated! Please check your email to activate your user') % {login: self.login} + raise UserNotActivated.new(message, self) + end + result = (crypted_password == encrypt(password)) if (encryption_method != User.system_encryption_method) && result self.password_type = User.system_encryption_method.to_s @@ -275,9 +297,15 @@ class User < ActiveRecord::Base # current password. # * Saves the record unless it is a new one. def change_password!(current, new, confirmation) - unless self.authenticated?(current) - self.errors.add(:current_password, _('does not match.')) - raise IncorrectPassword + + begin + unless self.authenticated?(current) + self.errors.add(:current_password, _('does not match.')) + raise IncorrectPassword + end + rescue UserNotActivated => e + self.errors.add(:current_password, e.message) + raise UserNotActivated end self.force_change_password!(new, confirmation) end diff --git a/test/functional/account_controller_test.rb b/test/functional/account_controller_test.rb index 19ebdce..e5b5696 100644 --- a/test/functional/account_controller_test.rb +++ b/test/functional/account_controller_test.rb @@ -40,6 +40,14 @@ class AccountControllerTest < ActionController::TestCase post :login, :user => { :login => 'fake', :password => 'fake' } end + should 'fail login if a user is inactive and show a warning message' do + user = User.create!(login: 'testuser', email: 'test@email.com', password:'test', password_confirmation:'test', activation_code: nil) + post :login, :user => { :login => 'testuser', :password => 'test' } + + assert_match 'not activated', session[:notice] + assert_nil session[:user] + end + def test_should_fail_login_and_not_redirect @request.env["HTTP_REFERER"] = 'bli' post :login, :user => {:login => 'johndoe', :password => 'bad password'} @@ -273,8 +281,9 @@ class AccountControllerTest < ActionController::TestCase assert_template 'invalid_change_password_code' end - should 'require password confirmation correctly to enter new pasword' do + should 'require password confirmation correctly to enter new password' do user = create_user('testuser', :email => 'testuser@example.com', :password => 'test', :password_confirmation => 'test') + user.activate change = ChangePassword.create!(:requestor => user.person) post :new_password, :code => change.code, :change_password => { :password => 'onepass', :password_confirmation => 'another_pass' } @@ -749,6 +758,8 @@ class AccountControllerTest < ActionController::TestCase get :activate assert_nil assigns(:message) post :login, :user => {:login => 'testuser', :password => 'test123'} + + assert_match 'not activated', session[:notice] assert_nil session[:user] end @@ -758,6 +769,8 @@ class AccountControllerTest < ActionController::TestCase get :activate, :activation_code => 'wrongcode' assert_nil assigns(:message) post :login, :user => {:login => 'testuser', :password => 'test123'} + + assert_match 'not activated', session[:notice] assert_nil session[:user] end diff --git a/test/unit/change_password_test.rb b/test/unit/change_password_test.rb index ab06cdb..44d4bc0 100644 --- a/test/unit/change_password_test.rb +++ b/test/unit/change_password_test.rb @@ -30,7 +30,8 @@ class ChangePasswordTest < ActiveSupport::TestCase change.password_confirmation = 'newpass' change.finish - assert User.find(person.user.id).authenticated?('newpass') + person.user.activate + assert person.user.authenticated?('newpass') end should 'not require password and password confirmation when cancelling' do diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index af36f18..76c54dc 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -123,6 +123,7 @@ class UserTest < ActiveSupport::TestCase def test_should_change_password user = create_user('changetest', :password => 'test', :password_confirmation => 'test', :email => 'changetest@example.com') + user.activate assert_nothing_raised do user.change_password!('test', 'newpass', 'newpass') end @@ -132,6 +133,7 @@ class UserTest < ActiveSupport::TestCase def test_should_give_correct_current_password_for_changing_password user = create_user('changetest', :password => 'test', :password_confirmation => 'test', :email => 'changetest@example.com') + user.activate assert_raise User::IncorrectPassword do user.change_password!('wrong', 'newpass', 'newpass') end @@ -141,6 +143,8 @@ class UserTest < ActiveSupport::TestCase should 'require matching confirmation when changing password by force' do user = create_user('changetest', :password => 'test', :password_confirmation => 'test', :email => 'changetest@example.com') + user.activate + assert_raise ActiveRecord::RecordInvalid do user.force_change_password!('newpass', 'newpasswrong') end @@ -153,6 +157,8 @@ class UserTest < ActiveSupport::TestCase assert_nothing_raised do user.force_change_password!('newpass', 'newpass') end + + user.activate assert user.authenticated?('newpass') end @@ -256,6 +262,7 @@ class UserTest < ActiveSupport::TestCase # when the user logs in, her password must be reencrypted with the new # method + user.activate user.authenticated?('test') # and the new password must be saved back to the database @@ -273,6 +280,7 @@ class UserTest < ActiveSupport::TestCase User.expects(:system_encryption_method).returns(:md5).at_least_once # but the user provided the wrong password + user.activate user.authenticated?('WRONG_PASSWORD') # and then her password is not updated @@ -520,7 +528,9 @@ class UserTest < ActiveSupport::TestCase should 'not authenticate a not activated user' do user = new_user :login => 'testuser', :password => 'test123', :password_confirmation => 'test123' - assert_nil User.authenticate('testuser', 'test123') + assert_raises User::UserNotActivated do + User.authenticate('testuser', 'test123') + end end should 'have activation code but no activated at when created' do -- libgit2 0.21.2