Commit c6a2cc536afa7390c498ef50acc4a450d8e6c2e9
1 parent
d8c77643
Exists in
master
and in
6 other branches
Revert "Merge branch 'user_not_active_auth' into 'master'"
This reverts commit d8c77643e546e5e82306aa6756cdf9931e008cc2, reversing changes made to 950a07082d85808991b63c99f0c30386af945a86.
Showing
3 changed files
with
8 additions
and
53 deletions
Show diff stats
app/controllers/public/account_controller.rb
@@ -46,12 +46,8 @@ class AccountController < ApplicationController | @@ -46,12 +46,8 @@ class AccountController < ApplicationController | ||
46 | 46 | ||
47 | self.current_user = plugins_alternative_authentication | 47 | self.current_user = plugins_alternative_authentication |
48 | 48 | ||
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 | 49 | + self.current_user ||= User.authenticate(params[:user][:login], params[:user][:password], environment) if params[:user] |
50 | + | ||
55 | if logged_in? | 51 | if logged_in? |
56 | check_join_in_community(self.current_user) | 52 | check_join_in_community(self.current_user) |
57 | 53 |
app/models/user.rb
@@ -120,17 +120,11 @@ class User < ActiveRecord::Base | @@ -120,17 +120,11 @@ class User < ActiveRecord::Base | ||
120 | 120 | ||
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 | 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 | - | ||
128 | # Authenticates a user by their login name or email and unencrypted password. Returns the user or nil. | 123 | # Authenticates a user by their login name or email and unencrypted password. Returns the user or nil. |
129 | def self.authenticate(login, password, environment = nil) | 124 | def self.authenticate(login, password, environment = nil) |
130 | environment ||= Environment.default | 125 | environment ||= Environment.default |
131 | - | ||
132 | - u = self.has_login?(login, login, environment.id) | ||
133 | - u = u.first if u.is_a?(ActiveRecord::Relation) | 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 | ||
134 | u && u.authenticated?(password) ? u : nil | 128 | u && u.authenticated?(password) ? u : nil |
135 | end | 129 | end |
136 | 130 | ||
@@ -242,23 +236,7 @@ class User < ActiveRecord::Base | @@ -242,23 +236,7 @@ class User < ActiveRecord::Base | ||
242 | password.crypt(salt) | 236 | password.crypt(salt) |
243 | end | 237 | end |
244 | 238 | ||
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 | - | ||
255 | def authenticated?(password) | 239 | 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 | - | ||
262 | result = (crypted_password == encrypt(password)) | 240 | result = (crypted_password == encrypt(password)) |
263 | if (encryption_method != User.system_encryption_method) && result | 241 | if (encryption_method != User.system_encryption_method) && result |
264 | self.password_type = User.system_encryption_method.to_s | 242 | self.password_type = User.system_encryption_method.to_s |
@@ -297,15 +275,9 @@ class User < ActiveRecord::Base | @@ -297,15 +275,9 @@ class User < ActiveRecord::Base | ||
297 | # current password. | 275 | # current password. |
298 | # * Saves the record unless it is a new one. | 276 | # * Saves the record unless it is a new one. |
299 | def change_password!(current, new, confirmation) | 277 | def change_password!(current, new, confirmation) |
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 | 278 | + unless self.authenticated?(current) |
279 | + self.errors.add(:current_password, _('does not match.')) | ||
280 | + raise IncorrectPassword | ||
309 | end | 281 | end |
310 | self.force_change_password!(new, confirmation) | 282 | self.force_change_password!(new, confirmation) |
311 | end | 283 | end |
test/functional/account_controller_test.rb
@@ -40,14 +40,6 @@ class AccountControllerTest < ActionController::TestCase | @@ -40,14 +40,6 @@ class AccountControllerTest < ActionController::TestCase | ||
40 | post :login, :user => { :login => 'fake', :password => 'fake' } | 40 | post :login, :user => { :login => 'fake', :password => 'fake' } |
41 | end | 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 | - | ||
51 | def test_should_fail_login_and_not_redirect | 43 | def test_should_fail_login_and_not_redirect |
52 | @request.env["HTTP_REFERER"] = 'bli' | 44 | @request.env["HTTP_REFERER"] = 'bli' |
53 | post :login, :user => {:login => 'johndoe', :password => 'bad password'} | 45 | post :login, :user => {:login => 'johndoe', :password => 'bad password'} |
@@ -281,9 +273,8 @@ class AccountControllerTest < ActionController::TestCase | @@ -281,9 +273,8 @@ class AccountControllerTest < ActionController::TestCase | ||
281 | assert_template 'invalid_change_password_code' | 273 | assert_template 'invalid_change_password_code' |
282 | end | 274 | end |
283 | 275 | ||
284 | - should 'require password confirmation correctly to enter new password' do | 276 | + should 'require password confirmation correctly to enter new pasword' do |
285 | user = create_user('testuser', :email => 'testuser@example.com', :password => 'test', :password_confirmation => 'test') | 277 | user = create_user('testuser', :email => 'testuser@example.com', :password => 'test', :password_confirmation => 'test') |
286 | - user.activate | ||
287 | change = ChangePassword.create!(:requestor => user.person) | 278 | change = ChangePassword.create!(:requestor => user.person) |
288 | 279 | ||
289 | post :new_password, :code => change.code, :change_password => { :password => 'onepass', :password_confirmation => 'another_pass' } | 280 | post :new_password, :code => change.code, :change_password => { :password => 'onepass', :password_confirmation => 'another_pass' } |
@@ -758,8 +749,6 @@ class AccountControllerTest < ActionController::TestCase | @@ -758,8 +749,6 @@ class AccountControllerTest < ActionController::TestCase | ||
758 | get :activate | 749 | get :activate |
759 | assert_nil assigns(:message) | 750 | assert_nil assigns(:message) |
760 | post :login, :user => {:login => 'testuser', :password => 'test123'} | 751 | post :login, :user => {:login => 'testuser', :password => 'test123'} |
761 | - | ||
762 | - assert_match 'not activated', session[:notice] | ||
763 | assert_nil session[:user] | 752 | assert_nil session[:user] |
764 | end | 753 | end |
765 | 754 | ||
@@ -769,8 +758,6 @@ class AccountControllerTest < ActionController::TestCase | @@ -769,8 +758,6 @@ class AccountControllerTest < ActionController::TestCase | ||
769 | get :activate, :activation_code => 'wrongcode' | 758 | get :activate, :activation_code => 'wrongcode' |
770 | assert_nil assigns(:message) | 759 | assert_nil assigns(:message) |
771 | post :login, :user => {:login => 'testuser', :password => 'test123'} | 760 | post :login, :user => {:login => 'testuser', :password => 'test123'} |
772 | - | ||
773 | - assert_match 'not activated', session[:notice] | ||
774 | assert_nil session[:user] | 761 | assert_nil session[:user] |
775 | end | 762 | end |
776 | 763 |