While debugging T409901: TypeError: array_keys(): Argument #1 ($array) must be of type array, null given by $resourceServer->getScopes(), we uncovered an interesting bug in the "update" feature of Special:OAuthManageMyGrants. This special page is not recommended for use by application developers, but no programmatic restrictions are enforced. So, if a developer doing local testing somehow finds their way to the aforementioned special page and tries to update their application grants (manually), it results in overriding their grants in the DB with an empty array ([]).
This has been tested for cases where the application is an OAuth2 client, and the grant type is either authonlyprivate or authonly. More testing is needed for consumers with requested permissions, etc.
Steps to replicate the issue (include links if applicable):
- Visit: <local-wiki-URL>/wiki/Special:OAuthManageMyGrants/update/<oaac_id>
- The ooac_id can be gotten quickly by trying to revoke/de-authorize the application or from the DB
- Click "Update grants" on the form presented
- Check the DB for the specific accepted consumer and notice that grants is now an empty array ([])
What happens?:
The oauth_accepted_consumer.oaac_grants field becomes an empty array ([]).
What should have happened instead?:
It (oauth_accepted_consumer.oaac_grants) should be whatever oauth_registered_consumer.oarc_grants is at the very least, since the user merely just clicked update without changing any data in the form.
Software version (on Special:Version page; skip for WMF-hosted wikis like Wikipedia):
Affecting master (which means it has been around for a while, I suppose).
Proposed solutions
Figure out what in ConsumerAcceptanceSubmitControl alters the grants on update, and at the very least, reuse the grants from oauth_registered_consumer.
But a better approach may be to avoid copying the grants from one table to another, since if they ever change, we'll need to keep the two copies in sync. Maybe we should review oauth_accepted_consumer and oauth_registered_consumer tables, and normalize so that the grants are in a central place of truth?
We already have ooac_user_id and ooac_consumer_id in oauth_accepted_consumer, so we can use those to query oauth_registered_consumer to get the grants, and related updates (like changes to grants, etc.) go to the consumer registrations table instead. Other ideas are welcome!
Notes
- Notice how the user of an example consumer app updated their grants: https://logstash.wikimedia.org/goto/df637852f48cfdeb8124430993a0a513 (timestamp: 15:56:...)
- Then notice how shortly after that, the app started logging warnings in Logstash: https://logstash.wikimedia.org/goto/07091fc5ab16e563a453adfaed239651 (timestamp: 15:58:...)
