Page MenuHomePhabricator

PluggableAuth::login should call $user->setCookies()
Closed, ResolvedPublic


(added @Anomie because I'm not sure if the following comment of his in CookieSessionProvider::sessionDataToExport() indicates my proposal is wrong.)

		// If we're calling the legacy hook, we should populate $session
		// like User::setCookies() did.

While working to solve a problem with VisualEditor, I discovered that the UserID and UserName cookies are not set when using PluggableAuth.

This problem occurs if you are using SSO which does not call a (MediaWiki-managed) login form since $user->setCookies() is only called in a few places SpecialChangePasswordPreAuthManager::attemptReset(), LoginFormPreAuthManager::processLogin(), and ApiLogin::execute().

Since the SSO system I'm coding against (CA's SiteMinder) depends on http headers, and the api uses User's loadFromSession, my SSO system can't authenticate VE users (since the headers SSO uses aren't passed in, but the UserID and UserName cookies are.

It may be that the $user->setCookies() call should happen in my use of PluggableAuth, not in PluggableAuth, but users of PluggableAuth should be aware of the potential problem.

Event Timeline

Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJul 12 2016, 3:10 PM

Change 298489 had a related patch set uploaded (by MarkAHershberger):
PluggableAuth::login should call $user->setCookies()

Change 298489 merged by Cicalese:
PluggableAuth::login should call $user->setCookies()

Interesting! Thanks for the patch. I have merged it.

I note that the UserLoadFromSession hook is deprecated, as are all the mentioned callers of $user->setCookies().

See also T110464: Update PluggableAuth to use AuthManager.

I coincidentally just started today working on T110464.

cicalese closed this task as Resolved.Sep 12 2016, 1:18 AM

Change 317148 had a related patch set uploaded (by Cicalese):
PluggableAuth::login should call $user->setCookies()

Change 317148 abandoned by Cicalese:
PluggableAuth::login should call $user->setCookies()

Will cherry-pick/edit later commit instead, since only one line of a deleted file is affected.