Page MenuHomePhabricator

Prevent re-authentication when visiting /index.php?title=Special:Login&returnto=Main_Page?
Closed, ResolvedPublic

Description

(I've originally posted this on the discussion page of the PluggableAuth extension: https://www.mediawiki.org/wiki/Topic:U63590t4cy1djubd)

I'm using the OpenID Connect extension with a setup like this:

  • anonymous users are allowed to read, but they're not allowed to edit or to register
  • $wgPluggableAuth_EnableAutoLogin is disabled so that anonymous users that cannot authenticate can read the wiki
  • the users that can log in usually access the wiki while it is iframed in the site that serves as the OP
  • I'm using /index.php?title=Special:Login&returnto=Main_Page as the iframe src to ensure that these users are automatically logged in

Unfortuately, visiting /index.php?title=Special:Login&returnto=Main_Page causes the OpenID Connect extension to re-authenticate against the IdP, even if the user was already logged in. This causes an annoying delay of several seconds each time the user clicks on the navigation link that opens the wiki in the iframe.

Is there any way to prevent re-authentication when the user is already logged in, e.g. with another Special site, or a parameter for Special:Login? As far as I can tell from includes/specialpage/LoginSignupSpecialPage.php, the core code does have the behaviour I want:

/*
 * In the case where the user is already logged in, and was redirected to
 * the login form from a page that requires login, do not show the login
 * page. The use case scenario for this is when a user opens a large number
 * of tabs, is redirected to the login page on all of them, and then logs
 * in on one, expecting all the others to work properly.
 *
 * However, do show the form if it was visited intentionally (no 'returnto'
 * is present). People who often switch between several accounts have grown
 * accustomed to this behavior.
 *
 * Also make an exception when force=<level> is set in the URL, which means the user must
 * reauthenticate for security reasons.
 */
if ( !$this->isSignup() && !$this->mPosted && !$this->securityLevel &&
         ( $this->mReturnTo !== '' || $this->mReturnToQuery !== '' ) &&
         $this->getUser()->isLoggedIn()
) {
        $this->successfulAction();
}

Is it possible that PluggableAuth accidentally breaks this behaviour?

Details

Related Gerrit Patches:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 25 2018, 3:00 PM

I have reproduced this behavior and have confirmed that the code above is executed. However, the redirect location that is set inside successfulAction() is later overwritten by Authentication Providers that redirect. A quick fix would be to add a "return" inside that if statement after successfulAction(), but the implications of that change would have to be assessed.

cicalese added a subscriber: Tgr.Jan 25 2018, 3:18 PM

This is probably as simple as adding a return; after successfulAction(); I'll have to check more closely if there was a reason for not doing that, but probably just an oversight.

Sending people to the login page as a means of autologin is not great but MediaWiki does not provide anything better for now. CentralAuth does autologin via an AJAX call; it would be nice to provide something like that for remote login providers with a simple "clickthrough" workflow.

Change 446702 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/core@master] Fix handling of already logged-in user in Special:Userlogin

https://gerrit.wikimedia.org/r/446702

Change 446702 merged by jenkins-bot:
[mediawiki/core@master] Fix handling of already logged-in user in Special:Userlogin

https://gerrit.wikimedia.org/r/446702

CCicalese_WMF closed this task as Resolved.Jul 19 2018, 8:44 AM
CCicalese_WMF removed a project: Patch-For-Review.
Tgr added a comment.Jul 19 2018, 10:13 AM

Filed T199984: Support background autologin in MediaWiki core about the more generic issue of better supporting the "check whether user is logged in centrally, log them in locally if possible" flow in core.