Page MenuHomePhabricator

Authentication sharing between desktop and mobile Commons is broken
Closed, ResolvedPublic

Description

Logging into desktop Commons does not also log you into mobile Commons. Logging into mobile Commons does not also log you into desktop Commons. Authentication sharing between mobile and desktop versions of the other projects seems to work fine.

Steps to reproduce:

  1. Go to https://commons.wikimedia.org/wiki/Main_Page
  2. Log in
  3. Go to https://commons.m.wikimedia.org/wiki/Main_Page

Expected result: You are still logged in

Actual result: You are not logged in

Event Timeline

kaldari created this task.Feb 6 2015, 10:41 PM
kaldari raised the priority of this task from to Needs Triage.
kaldari updated the task description. (Show Details)
kaldari added a subscriber: kaldari.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 6 2015, 10:41 PM
Matanya added a subscriber: Matanya.Feb 7 2015, 5:56 PM

We can't set the cookie for *.wikimedia.org since there are private wikis on that domain. That's why we individually login to each of the public wikimedia.org domains.

In this case though, it should be similar to the case where 3rd-party cookies are disabled, and the attempt to set the user's cookies on login fails. In that case, when the user gets to the site, and doesn't have any cookies, they should get an image / javascript that attempts to log them in to the current wiki if they are logged in to loginwiki (see the isAnon() code in CentralAuthHooks::onBeforePageDisplay() for the code). If that isn't working on the mobile site, then that's an entire issue that needs to be addressed.

Thanks for the info Chris!

KLans_WMF triaged this task as Medium priority.Mar 13 2015, 8:55 PM
KLans_WMF set Security to None.
Jdlrobson closed this task as Declined.Apr 10 2015, 6:22 PM
Jdlrobson claimed this task.
Jdlrobson added a subscriber: Jdlrobson.

Sounds like there is no work to do on our side here.

Sounds like there is no work to do on our side here.

@Jdlrobson, is this working now? My last comment was, "If that isn't working on the mobile site, then that's an entire issue that needs to be addressed." And it would need to be addressed in mobile frontend. Just want to make sure you're not waiting on CentralAuth to fix this.

Jdlrobson reopened this task as Open.Apr 10 2015, 8:31 PM

Sorry I binge bug triaged and yes you're right nope.. still not working and I missed your last message. I'm not clear what isn't happening here. Maybe @kaldari has some ideas

No worries!

CentralAuthHooks::onBeforePageDisplay() tries in two ways to auto-login anonymous users the first time they hit a page while logged out.

$out->addModules( 'ext.centralauth.centralautologin' );

loads the javascript version. Then we add a fallback <noscript> element with a 1x1 image (pointing at Special:CentralAutoLogin/start, with a bunch of other parameters) that does the handshake.

If either/both of those are being stripped by MobileFrontend, then the autologin won't work. The javascript also assumes that the skin is Vector, so that might be failing too.

There's your bug then - ext.centralauth.centralautologin needs to set 'targets' => array( 'desktop', 'mobile' ) on its ResourceLoader module definition.

(which extension provides this module - that's where the bug needs to be fixed?)

@Jdlrobson, adding 'target' => array( 'desktop', 'mobile' ) is easy enough, but the autologin script eventually loads in

https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FCentralAuth/c20e942e124b124075b44ba1e01789a1fa5ba44f/modules%2Finline%2Fautologin.js

which looks like it is going to fail on mobile. The user will be logged in one the browser actually downloads that javascript and has the cookies set, but the user won't get a notification.

Can you help with updating autologin.js to handle both mobile or desktop? Or is there someone else on mobile who would be better for that?

Add the targets and add me as a reviewer and I'll merge

I ran the autologin script on mobile and it doesn't do anything catastrophic to the UI.
Even though it doesn't update the ui to look like the user is logged in, since mobile is more JavaScripty than desktop this would probably be a bad experience anyway - it wouldn't show Echo notifications for some reason.

I think it's better to force the user to reload anyway - the notification message shows so we're good to go there.

Change 205771 had a related patch set uploaded (by CSteipp):
Mark centralautologin for mobile

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

Change 205771 merged by jenkins-bot:
Mark centralautologin for mobile

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

phuedx closed this task as Resolved.May 1 2015, 2:59 PM
phuedx removed a project: Patch-For-Review.
phuedx added a subscriber: phuedx.