Page MenuHomePhabricator

OAuth 2.0 consumer form is not consistent with implementation
Closed, ResolvedPublicBUG REPORT

Description

Steps to Reproduce:
Propose a new OAuth 2.0 consumer at https://meta.wikimedia.org/wiki/Special:OAuthConsumerRegistration/propose. A checkbox is visible: "Allow consumer to specify a callback in requests and use "callback" URL above as a required prefix."

Actual Results:
The checkbox has no effect. The callback URL must match exactly.

Expected Results:
The form should offer options consistent with what OAuth 2.0 consumers can actually do. Either the prefix functionality should be implemented, or the checkbox should be removed for cases where it doesn't apply.

Note that apparently the author of oauth2-server has rejected callback URL prefixes as being inconsistent with the OAuth 2.0 spec: https://github.com/thephpleague/oauth2-server/issues/1085. If WMF wants to override this behavior and support prefixes, the behavior of AbstractGrant::validateRedirectUri needs to be changed. Otherwise, the form needs to be fixed.

Event Timeline

The RFC says

3.1.2.2.  Registration Requirements
   ...

   The authorization server SHOULD require the client to provide the
   complete redirection URI (the client MAY use the "state" request
   parameter to achieve per-request customization).  If requiring the
   registration of the complete redirection URI is not possible, the
   authorization server SHOULD require the registration of the URI
   scheme, authority, and path (allowing the client to dynamically vary
   only the query component of the redirection URI when requesting
   authorization).

   The authorization server MAY allow the client to register multiple
   redirection endpoints.

so it's a SHALL requirement. (See also the full 3.1.2 which is about the redirect URI.) Elsewhere the RFC mentions that these requirements are to prevent attacks taking advantage of unintentional open redirectors on the target site.

One option would be to split the callback URL into a domain part and a path part, and allow the developer to update the path part as needed. See also T241867: Better support for callback URL updates.

The easily fixable issue I had was that I had registered the callback URI as http://localhost:3000 but the library I was using was passing it to the server as http://localhost:3000/, which was enough to cause a mismatch. I registered a new consumer with the slash on the callback URI and it works now.

The more interesting question is how to save application state on the return from the authorization page. Assuming the state was part of a query string on http://localhost:3000/, I initially was going to pass in the whole URI with query string in redirect_uri, but that isn't possible with a strict match. It turns out you're supposed to use the state parameter to handle that, which I'm now doing.

Based on recent comments in the above-referenced GitHub issue for oauth2-server, it looks like current best practice is not to allow callback prefixes of any kind. That means it's just the OAuth consumer proposal form that needs to be updated.

Change 710701 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/OAuth@master] Hide \"callback as prefix\" checkbox for OAuth 2.0 consumers

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

Change 710701 merged by jenkins-bot:

[mediawiki/extensions/OAuth@master] Hide \"callback as prefix\" checkbox for OAuth 2.0 consumers

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

Tgr claimed this task.

I don't think there's anything else to do here.