Home » Authentication » LiveUser » Bug #1020

[2004-03-16 08:16 UTC] alain_d99 at freesurf dot ch

Description:
------------
Hello

LiveUser.php: v1.82

The LiveUser::isInactive() function doesn't work as expected.

Example:
$nLiveUser =& LiveUser::factory($nConfig);
$nLiveUser->init();

if(!$nLiveUser->isLoggedIn()) {
if($nLiveUser->isInactive())
echo "Inactive";
else
echo "Active but wrong password";
}

In the example above, the "Inactive" term is always printed if the user exists and the given password is wrong. Must be "Active but wrong password".

The problem comes from the following calls:
LiveUser::init() -> LiveUser::tryLogin(...., ...., true) -> Container_DB::readUserData('handle', 'password').

The Container_DB::readUserData() gets a password, then in this function the user is searched with handle AND password. If the password is wrong, no user is found, and the following condition is never met :

$result = $this->dbc->getRow($sql, null, DB_FETCHMODE_ASSOC);

// If a user was found, read data into class variables and set
// return value to true
if (!DB::isError($result) && is_array($result)) {
$udata = $result; // NEVER GO INTO CONDITION

$this->isActive = ((!isset($udata['is_active']) || $udata['is_active'] == 'Y') ? true : false); // NEVER SET THE ISACTIVE FIELD
.....
}

In this case, $this->isActive is always false. Then LiveUser::isInactive() returns always true.

Then, impossible in my example to differenciate between a wrong login (handle correct but password not) and an inactive state.

Thank for your work,
Alain

Expected result:
----------------
The LiveUser::isInactive() must return false (if the 'is_active' bit is set to 'Y') when a correct handle is given, but its password is wrong.

Actual result:
--------------
The LiveUser::isInactive() returns always true (the 'is_active' bit is set to 'Y') when a correct handle is given, but its password is wrong.

[2004-04-04 08:21 UTC] alain_d99 at freesurf dot ch

Hello Dufuz,

thank for your message. I have made a workaround for my problem (in Common.php: ~310):

if ($this->allowDuplicateHandles == true || $checkpw == true) {
// If duplicate handles are allowed or the password _has_
// to be checked, only read in data if a matching user is found
$success = $this->readUserData($handle, $passwd);
} else {
// If duplicate handles are not allowed or the password
// doesn't need to be checked, just read in the data based
// on the handle
$success = $this->readUserData($handle);
}

Change to:

if ($this->allowDuplicateHandles == true) { // REMOVED, PASSWORD IS CHECKED AFTER || $checkpw == true) {
// If duplicate handles are allowed,
// only read in data if a matching user is found
$success = $this->readUserData($handle, $passwd);
} else {
// If duplicate handles are not allowed,
// just read in the data based
// on the handle
$success = $this->readUserData($handle);
}

And now, works correctly. But I believe that the whole condition must be removed, and keep only:

$success = $this->readUserData($handle);

And test after the password if duplicate handler is allowed or password must be checked, ie :

...
if($checkpw == true || $this->allowDuplicateHandles == true) {
if($this->passwd == $this->encryptPW($passwd)) {
...

I will discuss about this into devs mailing list.

The status member must be only valid for init() function. It will be a status_init member. Actually it is.

Greetings,
Alain

[2004-04-22 13:24 UTC] smith at backendmedia dot com

This is the intended behaviour.
A failed login attempt can be determined by the return value of the init() method.

However since LiveUser tries to potentially authenticate against multiple auth containers it would be a bad idea if we would consider a matching handle a match if passwords are actually required. It is very likely that one person will use the same handle in those multiple containers. Therefore if you need this feature you will have to create a custom container.