Handle 2FA error for OAuth client during login #10042
Conversation
For non-production WP.com OAuth clients, some APIs (like fetching settings) are disabled. In this case, we should still show a warning, but allow login to continue.
| @@ -1480,6 +1480,7 @@ | |||
| <string name="error_refresh_unauthorized_posts">You don\'t have permission to view or edit posts</string> | |||
| <string name="error_fetch_my_profile">Couldn\'t retrieve your profile</string> | |||
| <string name="error_fetch_account_settings">Couldn\'t retrieve your account settings</string> | |||
| <string name="error_disabled_apis">Could not fetch settings: Some APIs are disabled for this account + client ID combination.</string> | |||
There was a problem hiding this comment.
Some APIs are disabled for this account + client ID combination.
When I saw this, I thought there was something wrong with my actual WordPress user. What do you think about this?
Could not fetch settings: Some APIs are unavailable for this OAuth app ID and client ID combination.
The word "disable" connotes that it can be enabled somewhere but I believe it is impossible or we can't ever allow it, right?
There was a problem hiding this comment.
Hmm I see your point 🤔 but the OAuth app ID and client ID are the same thing. I think OAuth app ID is better for the message than client ID though since that's what we called the param in gradle.properties. 👍
What if we emphasized the 'OAuth app ID' part?
Could not fetch settings: Some APIs are unavailable for this OAuth app ID + account combination.
The word "disable" connotes that it can be enabled somewhere but I believe it is impossible or we can't ever allow it, right?
Basically, yes. I read it more as "works by default but not in this case", but "unavailable" works just as well for me and has less of a "default" connotation, which is maybe better. 👍 Will use it in the final message once we settle on one.
There was a problem hiding this comment.
Oh, oops. I meant secret instead of client ID there. 😅 But yep, your suggested message is great for me!
Could not fetch settings: Some APIs are unavailable for this OAuth app ID + account combination.
There was a problem hiding this comment.
Hey, @aforcier. Again, thank you for fixing this. This will significantly improve the experience for external contributors.
I responded to your comment in #10042 (comment) and I believe we have reached an agreement. I have also merged wordpress-mobile/WordPress-FluxC-Android#1283.
Feel free to apply the necessary changes and merge this. I might not be able to merge it myself since I will be AFK next week.
Thank you very much. 🙏
|
@shiki thanks for reviewing! I'll update the message in this PR and update with FluxC |
Fixes #8754.
Relies on wordpress-mobile/WordPress-FluxC-Android#1283, which should be reviewed and merged first.When logging into a 2FA-enabled account with personal (non-production) OAuth clients, login will no longer halt. Instead, you'll get a warning about disabled APIs and the flow will proceed:
See wordpress-mobile/WordPress-FluxC-Android#1283 for some background on the error and what it means.
To test:
wp.OAUTH.APP.IDandwp.OAUTH.APP.SECRETingradle.propertiesto those values.Me > Account SettingsandMe > App Settings, and confirm you get the same toast warning.Update release notes:
RELEASE-NOTES.txt.(This change only affects developers using personal OAuth credentials, no user-facing changes for WPAndroid.)