Commit d8c77643e546e5e82306aa6756cdf9931e008cc2
Exists in
master
and in
29 other branches
Merge branch 'user_not_active_auth' into 'master'
Login - Show a message if a user is not active This new feature shows a message if a user that is not active try login on noosfero. A Refactory was did to allow two verifications on login: if a user exists and if him was activated! See merge request !656
Showing
3 changed files
with
53 additions
and
8 deletions
Show diff stats
app/controllers/public/account_controller.rb
| ... | ... | @@ -46,8 +46,12 @@ class AccountController < ApplicationController |
| 46 | 46 | |
| 47 | 47 | self.current_user = plugins_alternative_authentication |
| 48 | 48 | |
| 49 | - self.current_user ||= User.authenticate(params[:user][:login], params[:user][:password], environment) if params[:user] | |
| 50 | - | |
| 49 | + begin | |
| 50 | + self.current_user ||= User.authenticate(params[:user][:login], params[:user][:password], environment) if params[:user] | |
| 51 | + rescue User::UserNotActivated => e | |
| 52 | + session[:notice] = e.message | |
| 53 | + return | |
| 54 | + end | |
| 51 | 55 | if logged_in? |
| 52 | 56 | check_join_in_community(self.current_user) |
| 53 | 57 | ... | ... |
app/models/user.rb
| ... | ... | @@ -120,11 +120,17 @@ class User < ActiveRecord::Base |
| 120 | 120 | |
| 121 | 121 | 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 |
| 122 | 122 | |
| 123 | + scope :has_login?, lambda { |login,email,environment_id| | |
| 124 | + where('login = ? OR email = ?', login, email). | |
| 125 | + where(environment_id: environment_id) | |
| 126 | + } | |
| 127 | + | |
| 123 | 128 | # Authenticates a user by their login name or email and unencrypted password. Returns the user or nil. |
| 124 | 129 | def self.authenticate(login, password, environment = nil) |
| 125 | 130 | environment ||= Environment.default |
| 126 | - u = self.first :conditions => ['(login = ? OR email = ?) AND environment_id = ? AND activated_at IS NOT NULL', | |
| 127 | - login, login, environment.id] # need to get the salt | |
| 131 | + | |
| 132 | + u = self.has_login?(login, login, environment.id) | |
| 133 | + u = u.first if u.is_a?(ActiveRecord::Relation) | |
| 128 | 134 | u && u.authenticated?(password) ? u : nil |
| 129 | 135 | end |
| 130 | 136 | |
| ... | ... | @@ -236,7 +242,23 @@ class User < ActiveRecord::Base |
| 236 | 242 | password.crypt(salt) |
| 237 | 243 | end |
| 238 | 244 | |
| 245 | + class UserNotActivated < StandardError | |
| 246 | + attr_reader :user | |
| 247 | + | |
| 248 | + def initialize(message, user = nil) | |
| 249 | + @user = user | |
| 250 | + | |
| 251 | + super(message) | |
| 252 | + end | |
| 253 | + end | |
| 254 | + | |
| 239 | 255 | def authenticated?(password) |
| 256 | + | |
| 257 | + unless self.activated? | |
| 258 | + message = _('The user "%{login}" is not activated! Please check your email to activate your user') % {login: self.login} | |
| 259 | + raise UserNotActivated.new(message, self) | |
| 260 | + end | |
| 261 | + | |
| 240 | 262 | result = (crypted_password == encrypt(password)) |
| 241 | 263 | if (encryption_method != User.system_encryption_method) && result |
| 242 | 264 | self.password_type = User.system_encryption_method.to_s |
| ... | ... | @@ -275,9 +297,15 @@ class User < ActiveRecord::Base |
| 275 | 297 | # current password. |
| 276 | 298 | # * Saves the record unless it is a new one. |
| 277 | 299 | def change_password!(current, new, confirmation) |
| 278 | - unless self.authenticated?(current) | |
| 279 | - self.errors.add(:current_password, _('does not match.')) | |
| 280 | - raise IncorrectPassword | |
| 300 | + | |
| 301 | + begin | |
| 302 | + unless self.authenticated?(current) | |
| 303 | + self.errors.add(:current_password, _('does not match.')) | |
| 304 | + raise IncorrectPassword | |
| 305 | + end | |
| 306 | + rescue UserNotActivated => e | |
| 307 | + self.errors.add(:current_password, e.message) | |
| 308 | + raise UserNotActivated | |
| 281 | 309 | end |
| 282 | 310 | self.force_change_password!(new, confirmation) |
| 283 | 311 | end | ... | ... |
test/functional/account_controller_test.rb
| ... | ... | @@ -40,6 +40,14 @@ class AccountControllerTest < ActionController::TestCase |
| 40 | 40 | post :login, :user => { :login => 'fake', :password => 'fake' } |
| 41 | 41 | end |
| 42 | 42 | |
| 43 | + should 'fail login if a user is inactive and show a warning message' do | |
| 44 | + user = User.create!(login: 'testuser', email: 'test@email.com', password:'test', password_confirmation:'test', activation_code: nil) | |
| 45 | + post :login, :user => { :login => 'testuser', :password => 'test' } | |
| 46 | + | |
| 47 | + assert_match 'not activated', session[:notice] | |
| 48 | + assert_nil session[:user] | |
| 49 | + end | |
| 50 | + | |
| 43 | 51 | def test_should_fail_login_and_not_redirect |
| 44 | 52 | @request.env["HTTP_REFERER"] = 'bli' |
| 45 | 53 | post :login, :user => {:login => 'johndoe', :password => 'bad password'} |
| ... | ... | @@ -273,8 +281,9 @@ class AccountControllerTest < ActionController::TestCase |
| 273 | 281 | assert_template 'invalid_change_password_code' |
| 274 | 282 | end |
| 275 | 283 | |
| 276 | - should 'require password confirmation correctly to enter new pasword' do | |
| 284 | + should 'require password confirmation correctly to enter new password' do | |
| 277 | 285 | user = create_user('testuser', :email => 'testuser@example.com', :password => 'test', :password_confirmation => 'test') |
| 286 | + user.activate | |
| 278 | 287 | change = ChangePassword.create!(:requestor => user.person) |
| 279 | 288 | |
| 280 | 289 | post :new_password, :code => change.code, :change_password => { :password => 'onepass', :password_confirmation => 'another_pass' } |
| ... | ... | @@ -749,6 +758,8 @@ class AccountControllerTest < ActionController::TestCase |
| 749 | 758 | get :activate |
| 750 | 759 | assert_nil assigns(:message) |
| 751 | 760 | post :login, :user => {:login => 'testuser', :password => 'test123'} |
| 761 | + | |
| 762 | + assert_match 'not activated', session[:notice] | |
| 752 | 763 | assert_nil session[:user] |
| 753 | 764 | end |
| 754 | 765 | |
| ... | ... | @@ -758,6 +769,8 @@ class AccountControllerTest < ActionController::TestCase |
| 758 | 769 | get :activate, :activation_code => 'wrongcode' |
| 759 | 770 | assert_nil assigns(:message) |
| 760 | 771 | post :login, :user => {:login => 'testuser', :password => 'test123'} |
| 772 | + | |
| 773 | + assert_match 'not activated', session[:notice] | |
| 761 | 774 | assert_nil session[:user] |
| 762 | 775 | end |
| 763 | 776 | ... | ... |
-
mentioned in commit c6a2cc536afa7390c498ef50acc4a450d8e6c2e9