program logic seems ok, all tests pass, ACK from me ----- Original Message ----- From: "Tomáš Hrčka" <[email protected]> To: [email protected] Cc: "Tomáš Hrčka" <[email protected]> Sent: Thursday, October 11, 2012 11:15:44 AM Subject: [PATCH conductor] BZ#864393&BZ#864382 Fixed number of logins and number of failed logins for LDAP authentication
http://bugzilla.redhat.com/show_bug.cgi?id=864393 http://bugzilla.redhat.com/show_bug.cgi?id=864382 --- src/app/models/user.rb | 7 +++++-- src/spec/models/user_spec.rb | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/app/models/user.rb b/src/app/models/user.rb index 5db8e95..27eed83 100644 --- a/src/app/models/user.rb +++ b/src/app/models/user.rb @@ -122,9 +122,13 @@ class User < ActiveRecord::Base def self.authenticate_using_ldap(username, password, ipaddress) if Ldap.valid_ldap_authentication?(username, password) u = User.find_by_username(username) || create_ldap_user!(username) - u.login_count += 1 update_login_attributes(u, ipaddress) else + u = User.find_by_username(username) + if u.present? + u.failed_login_count += 1 + u.save! + end u = nil end u.save! unless u.nil? @@ -133,7 +137,6 @@ class User < ActiveRecord::Base def self.authenticate_using_krb(username, ipaddress) u = User.find_by_username(username) || create_krb_user!(username) - u.login_count += 1 update_login_attributes(u, ipaddress) u.save! u diff --git a/src/spec/models/user_spec.rb b/src/spec/models/user_spec.rb index 9e38278..fd8e9f6 100644 --- a/src/spec/models/user_spec.rb +++ b/src/spec/models/user_spec.rb @@ -111,6 +111,23 @@ describe User do u.id.should == user.id end + it "should authenticate a existing user against LDAP and increment login count" do + user = FactoryGirl.create(:tuser, :ignore_password => true, :password => nil) + Ldap.should_receive(:valid_ldap_authentication?).and_return(true) + u = User.authenticate_using_ldap(user.username, 'random', user.last_login_ip) + u.failed_login_count.should be(0) + u.login_count.should be(1) + end + + it "should not authenticate a existing user against LDAP and increment failed login count" do + user = FactoryGirl.create(:tuser, :ignore_password => true, :password => nil) + Ldap.should_receive(:valid_ldap_authentication?).and_return(false) + User.authenticate_using_ldap(user.username, 'random', user.last_login_ip) + user.reload + user.login_count.should be(0) + user.failed_login_count.should be(1) + end + it "should authenticate a user with valid kerberos ticket" do User.authenticate_using_krb('krbuser', '192.168.1.1').should_not be_nil u = User.find_by_username('krbuser') -- 1.7.11.7
