Page MenuHomePhabricator

OAuth v2 client request form ignores default grant type values
Closed, ResolvedPublicBUG REPORT

Description

List of steps to reproduce (step by step, including full links if applicable):

What happens?:
The client has no right to authorize a token

requests.exceptions.HTTPError: 401 Client Error: Unauthorized for url: https://meta.wikimedia.org/w/rest.php/oauth2/access_token

From the database, it seems that the grant types got populated empty:

mysql:root@localhost [metawiki]> select oarc_oauth2_allowed_grants from oauth_registered_consumer where oarc_consumer_key = '0248bad72bec4d2847f2ef3a39ac77b4';
+----------------------------+
| oarc_oauth2_allowed_grants |
+----------------------------+
| []                         |
+----------------------------+
1 row in set (0.000 sec)

What should have happened instead?:

The grants types should be the ones displayed:

Screenshot from 2021-12-15 10-34-12.png (386×106 px, 11 KB)

When deselecting and selecting again the grant types, the values get populated, for example:

mysql:root@localhost [metawiki]> select oarc_oauth2_allowed_grants from oauth_registered_consumer where oarc_consumer_key = 'a0652706e2e016875fe831599e0c15ed';
+----------------------------------------+
| oarc_oauth2_allowed_grants             |
+----------------------------------------+
| ["authorization_code","refresh_token"] |
+----------------------------------------+
1 row in set (0.001 sec)

Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc:

Event Timeline

I suspect this is new (or maybe somehow browser-specific) - most people don't change these fields so if that always caused an error, we'd have noticed already.

cc @bd808 - OAuth admins should watch out for this (the "Allowed OAuth2 grant types" field being empty in the consumer approval form) until this gets fixed.

T245477: OAuth server should provide clear and useful feedback about client errors is related: if the client is using an invalid grant type, the error message should say that, not a nondescript "Client Error: Unauthorized".

The field is marked required so being allowed to submit the consumer proposal form with an empty set of grant types is already a bug.

(Not related to this bug, but the vocabulary conflict between grants and grant types is also confusing, they don't have a lot to do with each other. OAuth 2 actually calls the permission thingies scopes, not grants.)

The field is marked required so being allowed to submit the consumer proposal form with an empty set of grant types is already a bug.

Is there any real use case for the set of allowed grant types being anything other than authorization_code + refresh_token? Should this even be a user changeable field?

Is there any real use case for the set of allowed grant types being anything other than authorization_code + refresh_token? Should this even be a user changeable field?

Being able to enable client credentials grants, I suppose? Though that option seems to be disabled, for reasons I can't figure out (it works in no-JS mode, and I don't see any attempt for disabling it in the code).

FYI. I'm using firefox 91.4.1esr (64-bit), on debian bullseye (11.2):

ii  firefox-esr                             91.4.1esr-1~deb11u1                    amd64        Mozilla Firefox web browser - Extended Support Release (ESR)
• DAbad subscribed.

API Platform Bakclog Grooming

I wonder when exactly this happens. We don't get a lot of applications with an empty grant type list.

@Tgr wait you're right. i can replicate this every single time locally, but i just looked in Meta and looks like there are only 5 OAuth2 apps with empty grant type lists. Do you think its possible there are different settings applied to production that arent applied locally, affecting the ui ?

Or maybe it's browser-specific, or affected by asset loading timing which is very different in production. Not sure.

To accomplish this in a timely manner need help debugging production level things when we can't reproduce locally. See if we can get help on this.

I was able to reproduce this behaviour in a somewhat consistent way:

  • If you try to submit the form (but the form is invalid, and prompts you to change some field entries) the Grant Types input becomes enabled again, and therefore the Grant Types are successfully submitted to the database (although you have to re-select them from the dropdown)

I'm wondering if in production, users often had an invalid input which allowed them to submit the grant types appropriately.

Change 770607 had a related patch set uploaded (by TChin; author: TChin):

[mediawiki/extensions/OAuth@master] Split OAuth consumer registration form into two distinct forms for OAuth 1.0a and OAuth 2.0

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

Seen again with @putnik's 42ded93884cf4fac3eaec64cd0909092 consumer.

@putnik made a second consumer (74ec0c483b8d708113ad83a49da413b4) which also had the same empty oarc_oauth2_allowed_grants issue. Rather than make them try a third time I manually edited the database:

wikiadmin@10.64.48.109(metawiki)> update oauth_registered_consumer
    -> set oarc_oauth2_allowed_grants = '["authorization_code","refresh_token"]'
    -> where oarc_consumer_key = '74ec0c483b8d708113ad83a49da413b4';
Query OK, 1 row affected (0.002 sec)
Rows matched: 1  Changed: 1  Warnings: 0

There are still 6 other approved but broken OAuth 2 grants in the system:

wikiadmin@10.64.48.109(metawiki)> select oarc_consumer_key, oarc_name from oauth_registered_consumer where oarc_oauth_version = 2 and oarc_stage = 1 and oarc_oauth2_allowed_grants = '[]';
+----------------------------------+-----------------------+
| oarc_consumer_key                | oarc_name             |
+----------------------------------+-----------------------+
| 09c5453ed27db158507d463230242696 | wm-what-dev           |
| 0248bad72bec4d2847f2ef3a39ac77b4 | wm-what               |
| 25e3d1c428b0420ed24a688aabcaaf95 | Teyora (development)  |
| f5850d97247117ce9b10a8377338cf3e | test_wikiwix_archive2 |
| 2a05c3a9299978947fef4fdcb3c490be | side                  |
| 4ed42f9c5b03486cfde615a02496e542 | WikiSP Aula           |
+----------------------------------+-----------------------+
6 rows in set (0.007 sec)

There are still 6 other approved but broken OAuth 2 grants in the system:

wikiadmin@10.64.48.109(metawiki)> select oarc_consumer_key, oarc_name from oauth_registered_consumer where oarc_oauth_version = 2 and oarc_stage = 1 and oarc_oauth2_allowed_grants = '[]';
+----------------------------------+-----------------------+
| oarc_consumer_key                | oarc_name             |
+----------------------------------+-----------------------+
| 09c5453ed27db158507d463230242696 | wm-what-dev           |
| 0248bad72bec4d2847f2ef3a39ac77b4 | wm-what               |
| 25e3d1c428b0420ed24a688aabcaaf95 | Teyora (development)  |
| f5850d97247117ce9b10a8377338cf3e | test_wikiwix_archive2 |
| 2a05c3a9299978947fef4fdcb3c490be | side                  |
| 4ed42f9c5b03486cfde615a02496e542 | WikiSP Aula           |
+----------------------------------+-----------------------+
6 rows in set (0.007 sec)

I have fixed these manually as well because it did not feel right to punish the users for bugs in the app:

wikiadmin@10.64.48.109(metawiki)> update oauth_registered_consumer
    -> set oarc_oauth2_allowed_grants = '["authorization_code","refresh_token"]'
    -> where oarc_oauth_version = 2 and oarc_stage = 1 and oarc_oauth2_allowed_grants = '[]';
Query OK, 6 rows affected (0.006 sec)

I have fixed these manually as well because it did not feel right to punish the users for bugs in the app:

Thanks! two of those are mine :}

related to other ticket that has a patch that will address this

My consumers seem to be affected by this bug. I've tried the workaround suggested here, to delete and re-add the grant pills as I propose a consumer, but still I see the field is empty in the database:

select oarc_oauth2_allowed_grants from oauth_registered_consumer where oarc_consumer_key='45559ef445748c9740b48b171391e483';

    [oarc_oauth2_allowed_grants] => []

Hopefully there's something better we can do than recreating the consumer and having someone repeatedly approve, to see if the bug goes away? If anyone is up for running the update to add the usual grants, that would be much appreciated.

I see that other tools are stuck in the same state:

select oarc_consumer_key, oarc_name from oauth_registered_consumer where oarc_oauth_version = 2 and oarc_stage = 1 and oarc_oauth2_allowed_grants = '[]';

(
    [oarc_consumer_key] => ac65b92975ab436bbed9692396caf4da
    [oarc_name] => CertificateBot
)
stdClass Object
(
    [oarc_consumer_key] => 77d1ed44108c72978262d135c2fcd461
    [oarc_name] => videocuttool
)
stdClass Object
(
    [oarc_consumer_key] => 45559ef445748c9740b48b171391e483
    [oarc_name] => Capacity Exchange
)

If anyone is up for running the update to add the usual grants, that would be much appreciated.

wikiadmin@10.64.32.139(metawiki)> select oarc_consumer_key, oarc_name from oauth_registered_consumer where oarc_oauth_version = 2 and oarc_stage = 1 and oarc_oauth2_allowed_grants = '[]';
+----------------------------------+-------------------+
| oarc_consumer_key                | oarc_name         |
+----------------------------------+-------------------+
| ac65b92975ab436bbed9692396caf4da | CertificateBot    |
| 77d1ed44108c72978262d135c2fcd461 | videocuttool      |
| 45559ef445748c9740b48b171391e483 | Capacity Exchange |
+----------------------------------+-------------------+
3 rows in set (0.005 sec)

wikiadmin@10.64.32.139(metawiki)> update oauth_registered_consumer set oarc_oauth2_allowed_grants = '["authorization_code","refresh_token"]' where oarc_oauth_version = 2 and oarc_stage = 1 and oarc_oauth2_allowed_grants = '[]';
Query OK, 3 rows affected (0.010 sec)
Rows matched: 3  Changed: 3  Warnings: 0

wikiadmin@10.64.32.139(metawiki)> select oarc_consumer_key, oarc_name from oauth_registered_consumer where oarc_oauth_version = 2 and oarc_stage = 1 and oarc_oauth2_allowed_grants = '[]';
Empty set (0.005 sec)

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

[mediawiki/extensions/OAuth@master] Fix broken grant type field

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

Change 796932 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/OAuth@master] Change field casing to allow validation

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

Change 796932 abandoned by Awight:

[mediawiki/extensions/OAuth@master] Change field casing to allow validation

Reason:

Will try the simpler approach of changing banana lint rules.

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

That was fixed, so this is fixed too. I think it would be still nice to:

  • fail the form validation when there are no grant types - accepting a dysfunctional consumer definition and putting it in the approvals queue is a waste of everyone's time.
  • show the grant types in Special:OAuthListConsumers. Missing from there made debugging this issue hard for non-admins.

Change 796056 merged by jenkins-bot:

[mediawiki/extensions/OAuth@master] Fix broken grant type field

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

Change 770607 merged by jenkins-bot:

[mediawiki/extensions/OAuth@master] Split OAuth consumer registration form into two distinct forms for OAuth 1.0a and OAuth 2.0

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