Home » Authentication » Auth » Bug #9407

[2006-11-21 16:17 UTC] jonw at improov dot com

Description:
------------
Doing a checkAuth() seems to store the password into session to be used until logout. No further password checks are done after checkAuth().

I have an application that allows a user to log in, and then an administrator (another user) to change the user's password, or even delete the user altogether. Since Pear::Auth does not do anymore password checks after the initial checkAuth(), Pear::Auth assumes that the user is still logged on, even though the user's password may already have been changed, or the user may have been deleted.

Test script:
---------------
Just do a checkAuth(), which uses POST parameters 'username' and 'password'. So populate those parameters before calling checkAuth().

Now, delete the logged in user (from your datastore) or change the password.

Then do a checkAuth() again. The logged in user is still logged in, though all records of the user in the datastore are already purged (or the password changed).

Expected result:
----------------
checkAuth() should be sensitive to any changes to the logged in user's password.

Actual result:
--------------
The logged in user is still logged in, though all records of the user in the datastore are already purged (or the password changed).

[2006-11-21 23:05 UTC] aashley at php dot net

Umm, how?

Auth is only aware of the users password at the point the form submission is processed. After it validates the password it is not stored anywhere.

Plus the definition of a 'deleted' or 'changed' account will change depending upon application and backend used. There is no way to reliably implement this sort of functionality in a generic way.

[2006-11-22 01:36 UTC] jonw at improov dot com

Speaking of generic methods, how about making it generic to have an "last_updated" variable? You already have 2 generic variables: "userid" and "password" (plus "dsn" and some other I may have forgotten). It's very common, if not actually de facto, for applications to have such mechanisms.

Of course, you may say that different applications may view the "last_updated" variable differently. Sure, let applications override the generic processing of "last_updated". But isn't it generic/common to process it thus?: force a re-login if password was last updated AFTER the login time.

Seems like a trivial extension, probably no more than 10-20 lines of code. That's the extension I had to do in my application for now.

[2006-11-22 01:51 UTC] aashley at php dot net

That still is adding a forced interrogation of the authentication backend. Which is completely against the design of Auth. The backend is checked once at login and then thats it.

The best I'd be willing to add is another callback that enables extra checks which is called from checkAuth(). it is supplied the username and the Auth object and it's up to the developer to provide a true/false response on whether to continue the session.

[2006-11-22 05:07 UTC] jonw at improov dot com

Sounds great!

How about tying default functions (say the "last_updated" mechanism I mentioned) to this checkAuth()'s callback you're suggesting, and letting users override this default generic behavior?

[2006-11-22 06:53 UTC] aashley at php dot net

A checkAuth() call back will be added. That is all. Behaviour like your 'last_updated' stuff should be implemented using said callback.

[2006-12-06 00:44 UTC] aashley at php dot net

This bug has been fixed in CVS.

If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET).

If this was a problem with the pear.php.net website, the change should be live shortly.

Otherwise, the fix will appear in the package's next release.

Thank you for the report and for helping us make PEAR better.