veradekok wrote:

Correction: the blank page says "Error retrieving token: mwoauthdatastore-request-token-not-found"

What is the exact user agent string on the phone? Do you literally refer to the string "PC"? Exact strings welcome in order to reproduce...

veradekok wrote:

The user agent resulting in the error, eg. "Android":

Mozilla/5.0 (Linux; U; Android 4.0.1; ja-jp; Galaxy Nexus Build/ITL41D) AppleWebKit/534.30 (KHTML, like Gecko) Version/4.0 Mobile Safari/534.30

The "PC" user agent not resulting in error:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_3; ja-jp) AppleWebKit/533.16 (KHTML, like Gecko) Version/5.0 Safari/533.16

Verified, when editing from a user-agent that would be redirected to the mobile site, after authorizing, page gets redirected to mobile site and errors.

E006

Steps to reproduce:
*From a cell phone, where you have not opted out of mobile interface and it sends a mobile UA
*Go to http://tools.wmflabs.org/oauth-hello-world/index.php?action=authorize
*Follow instructions, eventually get an error.

(In reply to Bawolff (Brian Wolff) from comment #4)

Verified, when editing from a user-agent that would be redirected to the
mobile site, after authorizing, page gets redirected to mobile site and
errors.

I had suspected this, thanks for confirming.

After some further investigation, it looks to me like it's actually a bug in WMF's varnish layer trying to redirect mobile clients to the mobile site: it doesn't hit on the first request that uses /w/index.php?title=Special:OAuth/authorize&oauth_token=abc123&oauth_consumer_key=abc123, but the POST back to /wiki/Special:OAuth/authorize is caught and since browsers treat a 302 redirect as 303 rather than 307 this breaks everything. The relevant code appears to be in the operations/puppet repo, templates/varnish/text-frontend.inc.vcl.erb, sub mobile_redirect.

Change 167453 had a related patch set uploaded by MaxSem:
Perform mobile redirect only for GET requests

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

(In reply to Brad Jorsch from comment #5)

(In reply to Bawolff (Brian Wolff) from comment #4)

Verified, when editing from a user-agent that would be redirected to the
mobile site, after authorizing, page gets redirected to mobile site and
errors.

I had suspected this, thanks for confirming.

After some further investigation, it looks to me like it's actually a bug in
WMF's varnish layer trying to redirect mobile clients to the mobile site: it
doesn't hit on the first request that uses
/w/index.php?title=Special:OAuth/
authorize&oauth_token=abc123&oauth_consumer_key=abc123, but the POST back to
/wiki/Special:OAuth/authorize is caught and since browsers treat a 302
redirect as 303 rather than 307 this breaks everything. The relevant code
appears to be in the operations/puppet repo,
templates/varnish/text-frontend.inc.vcl.erb, sub mobile_redirect.

Yep, this is actually a known issue.

Mobile can't really redirect calls to /w/index.php, so OAuth app authors need to redirect their users to "/wiki/Special:OAuth/authorize?oauth_token=..." instead of "/w/index.php?title=...".

So Max'es patch will probably work, although then the login experience on mobile isn't great. And we'll have to make sure the centralauth handshake continues to work. Or OAuth app authors can use /wiki/Special:OAuth urls, and the experience is better, but we can't control their code. Or we make a special varnish rule to allow mobile redirecting for this specific url pattern ("/w/index.php?title=Special:OAuth/authorize")... But I haven't fully thought through what else that would impact.

I thought we had trouble with OAuth getting confused by internal rewriting somewhere that changed /wiki/Special:OAuth to /w/index.php?title=Special:OAuth and broke the signature validation. Did that get fixed?

(In reply to Brad Jorsch from comment #8)

I thought we had trouble with OAuth getting confused by internal rewriting
somewhere that changed /wiki/Special:OAuth to
/w/index.php?title=Special:OAuth and broke the signature validation. Did
that get fixed?

Yeah, that's one of the confusing parts. For any calls that are signed, that is the case, so title=Special:OAuth is the best format for the url. The /authorize call is the only one not signed, since it's just redirecting the user, so the clean url can be used.

Change 167453 merged by Faidon Liambotis:
Perform mobile redirect only for GET and HEAD requests

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

Is this still an issue after the (long since) merge of that gerrit changeset?

In T74186#1058964, @ArielGlenn wrote:

Is this still an issue after the (long since) merge of that gerrit changeset?

That change didn't affect this particular issue. This issue occurs because the mobile redirect only happens for /wiki/ urls, but to work around some badly designed OAuth libraries (like the one in phabricator...), server-side OAuth calls have to use /w/index.php?title=Special:OAuth, so they redirect the user to the same url structure for the authorization, and mobile doesn't redirect them.

So, Chris, what needs to happen here?

I think line 61 should become something like,

&& ( req.url ~ "^/(wiki|(gan|ike|iu|kk|ku|shi|sr|tg|uz|zh)(-[a-z]+)?)[/\?]"
    || req.url ~ "^/w/index.php\?title=Special:OAuth/authorize" ) ) {

In T74186#1064090, @csteipp wrote:

I think line 61 should become something like,

&& ( req.url ~ "^/(wiki|(gan|ike|iu|kk|ku|shi|sr|tg|uz|zh)(-[a-z]+)?)[/\?]"
    || req.url ~ "^/w/index.php\?title=Special:OAuth/authorize" ) ) {

Uh, so you want to redirect some ugly URLs to mobile? I thought the opposite is what is needed. Also, special page names (or at least the Special: namespace) can be localised, rendring this regex unmanageable.

In T74186#1064100, @MaxSem wrote:

Uh, so you want to redirect some ugly URLs to mobile? I thought the opposite is what is needed. Also, special page names (or at least the Special: namespace) can be localised, rendring this regex unmanageable.

Right, that's why this is really best to push onto the app owner to implement. Unfortunately, things like Phabricator's OAuth library are horrible broken, but upstream won't accept patches to let us work around this issue... so we may need to do something ugly on our end too.

Adding Traffic for visibility

The swap of Traffic for Traffic-Icebox in this ticket's set of tags was based on a bulk action for all such tickets that haven't been updated in 6 months or more. This does not imply any human judgement about the validity or importance of the task, and is simply the first step in a larger task cleanup effort. Further manual triage and/or requests for updates will happen this month for all such tickets. For more detail, have a look at the extended explanation on the main page of Traffic-Icebox . Thank you!

Note that I'm baking this bug into the ATS config in gerrit 817086. If this bug is ever fixed, the rule will have to be updated.

In T74186#8103651, @tstarling wrote:

Note that I'm baking this bug into the ATS config in gerrit 817086. If this bug is ever fixed, the rule will have to be updated.

The bug only affects mobile domains/devices. We do say in all OAuth docs to use nice URL, but apparently quite a few apps don't, so probably the config should handle both cases.

As pointed out in T332650#8715452, if the user is not logged in, they get redirected to the login form, and then back via returnto/returntoquery and that will always use the non-pretty format, so this problem can't fully be fixed on the OAuth app's side.

I think this is easy enough to solve: we just need to make Special:OAuth/authorize (and authenticate) redirect to the nice URL. There's some performance cost (an extra hop, but only after login/autologin, which is very slow anyway), and some risk of an infinite redirect loop, but it seems manageable.

After re-reading the comments, I'm not really sure about the current state of the task. Varnish does a redirect to the mobile domain on GET requests from a mobile device for nice (/wiki/) URLs but not index.php URLs. Originally (as per T74186#750517), the redirect was done on POST requests as well and the authorization form always posted to a nice URL, and browsers don't handle well redirects on POST, so this broke the submission of the authorization form. That was fixed in rOPUPcbbb9f8e480f: Perform mobile redirect only for GET and HEAD requests though. So what's left? If you use a non-nice URL, the mobile redirect won't happen and so the user will see the desktop authorization form on their mobile device, which requires heavy zooming to use?

T112730: Failure to OAuth after login on mobile implies this issue somehow also interfered with login - I guess that only happened on mediawiki.org because of the cross-wiki mobile domain generation problems that were fixed in T257852?

In T74186#750538, @Anomie wrote:

I thought we had trouble with OAuth getting confused by internal rewriting somewhere that changed /wiki/Special:OAuth to /w/index.php?title=Special:OAuth and broke the signature validation. Did that get fixed?

For the record, that's T59500: Impossible to use https://www.mediawiki.org/wiki/Special:OAuth/initiate?format=&oauth_callback= style URL.

In T74186#8716436, @Tgr wrote:

As pointed out in T332650#8715452, if the user is not logged in, they get redirected to the login form, and then back via returnto/returntoquery and that will always use the non-pretty format, so this problem can't fully be fixed on the OAuth app's side.

On reflection I think this is incorrect - the mobile redirect happens in Varnish before MediaWiki ever receives the request. T112730#9347006 also confirms that login on mobile is using the mobile site (as long as the app uses the nice URL). So the problem can be fully fixed on the OAuth app's side. Still would be nice if they didn't have to care about it though.