works for me

candrews wrote:

I oversimplified the test case:

$wgHooks['UserLoadFromSession'][] = 'testFn';

function testFn(&$user){

//Give us a user, see if we're around
$tmpuser = User::newFromSession();

//They already with us?  If so, quit this function.
if( $tmpuser->isLoggedIn() ) {
        $wgAuth->printDebug( "User is already logged in.", 1 );
        return true;
}
// more login actions...

}

After some more debugging, I learned that UserLoadFromSession isn't a simple rename of the AutoAuthenticate hook - if you call User::newFromSession() from within a UserLoadFromSession hook, an infinite loop results. It seems some more documentation or something may be needed :-)

How do I determine if a user is already logged in from within the UserLoadFromSession hook function?

Yeah, I see, that code breaks

Still, I don't see why you need to be loading another user.

candrews wrote:

I just want to figure out if the user is already logged in, (perhaps he was logged in via another method, like cookies), so I can avoid logging them in again (which is expensive).

Is there a way to do that without using the method I previously described? Calling "$user->isLoggedIn()" inside testFn always returns false.

$user->isLoggedIn() should not fail if $user is the object passed from the hook. This is because $user->load() shortcircuits since it was already loaded. Calling most methods on User triggers $user->load(). When you make a new user object, it isn't already loaded, so $newuser->load() calls the session function and hook, which causes recursion.

candrews wrote:

$wgHooks['UserLoadFromSession'][] = 'testFn';

function testFn(&$user){

if( $user->isLoggedIn() ) {
  die("Logged in");
}

}

Even after logging in, that "die" will never execute. From my reading of your last statement, it sounds like after logging in, it should execute.... right?

You can run the code from comment #2, with $tmpuser replaced with $user

(In reply to comment #8)

You can run the code from comment #2, with $tmpuser replaced with $user

Well not exactly. As long as User::newFromSession() is not called of the user is already loaded.

(In reply to comment #6)

$user->isLoggedIn() should not fail if $user is the object passed from the
hook. This is because $user->load() shortcircuits since it was already loaded.
Calling most methods on User triggers $user->load(). When you make a new user
object, it isn't already loaded, so $newuser->load() calls the session function
and hook, which causes recursion.

$user-isLogedIn() will always return false in this situation.

The UserLoadFromSession hook is called before the user is loaded from the session. As such, the hook is always going to pass an anonymous user.

A workaround for this problem in the past (when the hook was still AutoAuthenticate), was to create a temporary user, call User::loadFromSession, and then check if that user was logged in. There is no way of doing this now.

Auto-auth plugins now either have to reimplement User::loadFromSession in their plugin, or re-authenticate users on every page view. I've chosen to do the former.

My suggested solution is to have User::loadFromSession call the UserLoadFromSession hook, then call another function (User::loadFromSessionBody?) that isn't marked as private to do the rest. That way auto-auth plugins can call $user->loadFromSessionBody(), *then* call $user->isLoggedIn().

Looking at this hook, the above uses just seem to be beyond what it was intended for, so I'm not sure it's worth changing. If a new hook is needed, then that can be filed as a another bug report.

Maybe you don't realize this, but this wasn't originally the hook used for this. The hook originally used was AutoAuthenticate, and it was renamed to this, and moved to this spot.

This hook is fully usable for auto-authentication, after all, the CentralAuth plugin uses it. The only issue with it, is that auto-authentication plugins can't check to see if a user has been logged in or not. The solution I offered gives the auto-authentication plugins a way to check this (the way they previously were).

I don't necessarily have an issue with making a new hook, but we need a stable api for auto-authentication; we can't keep changing how it works every release.

If you'd like to fix the problem, please fix it permanently this time.

Obviously it is used for authentication, but it is there to fetch the cookie data not to call another loadFromSession(), which is just recursive.

There is a difference between MediaWiki core loading something from session, and from an extension doing it via a hook. Some plugins would like to allow core to do session checking for them.

I don't mind if another hook is made, but as it is a solution to this problem, let's leave the bug open until we actually fix the problem.

(In reply to comment #14)

There is a difference between MediaWiki core loading something from session,
and from an extension doing it via a hook. Some plugins would like to allow
core to do session checking for them.

I don't mind if another hook is made, but as it is a solution to this problem,
let's leave the bug open until we actually fix the problem.

The problem is assumed to have to do with the bug summary and initial report. Unless the fix is to stop it from segfaulting, then this bug is a wontfix. Adding a new hook could solve the issue of authenticating by checking for an existing initialized session first, but not the issue of segfaults.

I suppose one could change the bug summary...

New hook called after loadFromSession is called

Here's a patch to make a new hook (UserLoadAfterLoadFromSession), which is called in load, after loadFromSession() is called.

Using this hook, auto-authentication plugins won't need to call $user->loadFromSession(), and will instead just be able to check $user->isLoggedIn().

Attached:

auto-authentication plugins that wish to have core load the user from session should use the UserLoadAfterLoadSession hook now instead of UserLoadFromSession. Also, they should no longer call $user->loadFromSession() before checking whether a user is logged in or not.