Page MenuHomePhabricator
Create Task
Maniphest T100413

"You are centrally logged in." toast on every page view on commons
Closed, ResolvedPublic
Actions

Description

Tested on Chrome and Firefox (both desktop, not tested on mobile so far...) and two different computers:
Logged in using the desktop site of some wiki.
Visit http://commons.m.wikimedia.org, and get a login toast notification with message "Central login
You are centrally logged in. Reload the page to apply your user settings."
If i reload the page, i get the same toast again (and on every other page view).

Maybe related to T88860: Authentication sharing between desktop and mobile Commons is broken ?

Related Objects
Search...

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
csteipp claimed this task.Jun 5 2015, 12:51 AM

@kaldari, I'm still not certain what the best way is to detect that a request came to a mobile domain. Everything I've tried feels horribly hacky. But probably better to just get something working for now on WMF sites, and we can generalize it in code review.

I'll try to get something up tomorrow.

csteipp added a project: Security-Team.Jun 5 2015, 12:51 AM
csteipp moved this task from Backlog to In Progress on the Security-Team board.
csteipp added a comment.Jun 10 2015, 8:14 PM

The interactions with mobile frontend are turning out to be more complicated than I thought. Can I pair with someone on mobile to work through the patch sometime in the next week?

Jdlrobson added a comment.Jun 16 2015, 12:33 AM

We sat down on Friday and worked out a way to do this. Patch coming soon I suspect :)!

csteipp added a comment.Jun 16 2015, 4:52 PM

Yes soon! Should be end of the week at the latest.

gerritbot added a subscriber: gerritbot.Jun 18 2015, 10:26 PM

Change 219272 had a related patch set uploaded (by CSteipp):
Add detection for mobile domain request

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

gerritbot added a comment.Jun 18 2015, 10:31 PM

Change 219275 had a related patch set uploaded (by CSteipp):
Autologin for m. domains

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

Restricted Application added a subscriber: Steinsplitter. · View Herald TranscriptJun 26 2015, 5:49 PM
Steinsplitter added a project: Commons.Jun 26 2015, 6:47 PM
Steinsplitter moved this task from Incoming to Backlog on the Commons board.
gerritbot added a comment.Jul 1 2015, 10:15 PM

Change 219272 merged by jenkins-bot:
Add detection for mobile domain request

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

Restricted Application added a subscriber: Luke081515. · View Herald TranscriptJul 2 2015, 10:39 AM
Restricted Application added a subscriber: MGChecker. · View Herald TranscriptJul 6 2015, 4:07 PM
Jdlrobson added a subscriber: phuedx.Jul 6 2015, 8:57 PM

@csteipp there is an open question from @phuedx on your patchset

gerritbot added a comment.Jul 15 2015, 10:13 AM

Change 219275 merged by jenkins-bot:
Autologin for m. domains

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

Jdlrobson updated the task description. (Show Details)Jul 27 2015, 11:17 PM

@csteipp bad news this seems to still be a problem on wmf15 despite the fact that both patches should have been deployed.

csteipp added a comment.Jul 28 2015, 12:00 AM

It works for me... without javascript.

The issue is ext.centralauth.centralautologin doesn't have logic to add mobile=1 when it constructs the url for loginwiki. Jon and I had briefly talked about loading a different version of that script for mobile using resource loader targets.

Jdlrobson added a comment.Jul 31 2015, 6:52 PM

@csteipp do you need help with this?

csteipp added a comment.Aug 1 2015, 12:10 AM

@Jdlrobson, yeah, help would be appreciated! I didn't see an obvious, clean way to handle it, but if you have ideas, I'd be happy to review.

Jdlrobson changed the task status from Open to Stalled.Aug 19 2015, 12:23 AM
gerritbot added a comment.Aug 22 2015, 5:07 AM

Change 233091 had a related patch set uploaded (by CSteipp):
Set mobile flag for autologin js

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

csteipp moved this task from In Progress to Waiting on the Security-Team board.Aug 24 2015, 7:11 PM
csteipp moved this task from Waiting to In Progress on the Security-Team board.Sep 11 2015, 2:09 PM
jrbs added a subscriber: jrbs.Sep 12 2015, 11:04 AM
csteipp moved this task from In Progress to Waiting on the Security-Team board.Sep 14 2015, 11:14 PM
Florian added a comment.Oct 26 2015, 6:48 PM

Hmm, I'm wondering if this is already fixed? (I haven't seen such a notification for a long time now), because there is one open change here :)

Aklapper added a comment.Nov 30 2015, 11:20 AM
In T100413#1754527, @Jdlrobson wrote:

https://gerrit.wikimedia.org/r/#/c/233091/ is waiting for feedback from @Krinkle afaik

That patch got a +1. Anything else blocking this "Unbreak now"-priority task?

Krinkle added a comment.Nov 30 2015, 12:12 PM

The patch looks fine (added +1), though it does work around two problems that will remain unsolved:

  • Internal domain variation is not verified. So whenever a new way comes up to view a page, it will result in the same bug again. Curious if this affects Wikipedia Zero, for example. It might make sense to validate the hostname server-side instead of passing mobile=1 which doesn't scale and leaves the same bug to happen if other "mobile" domains appear. It also negatively affects caching proxies and mirrors (which already have this bug for both mobile and desktop origins).
  • Silently attempting to log-in from a any domain where the code runs is fine (that one request isn't a problem), but we should at least validate the domain as part of that request so it doesn't fan-out further then needed.
  • MobileFrontend toast "You've been logged-in,.. reload .." message is displayed regardless of whether the login succeeded. This is the root cause of this bug and will likely happen again in the future.
csteipp moved this task from Waiting to Done on the Security-Team board.Dec 1 2015, 7:54 PM
Jdlrobson changed the task status from Stalled to Open.Jan 8 2016, 10:12 PM
gerritbot added a comment.Jan 8 2016, 10:14 PM

Change 233091 merged by jenkins-bot:
Set mobile flag for autologin js

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

csteipp closed this task as Resolved.Jan 26 2016, 4:52 PM

The original issue seems to be resolved now

Restricted Application added subscribers: Jay8g, TerraCodes. · View Herald TranscriptDec 15 2016, 4:20 PM
Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL