-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/adjust readme for 2fa issue #9781
Feature/adjust readme for 2fa issue #9781
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AmirGamilDev as I mentioned in our conversation, I'm not sure it's necessary to outline workarounds for a bug we have an open issue for in the README.
I do think it's called for to point out that there are some limitations applied to 2FA accounts when using personal credentials so it's less of a surprise. I suggested a rewording of the README.md
section you added - I think with that the DEV-BUILD-2FA-LOGIN-ISSUE.md
file isn't needed, especially since the workaround is outlined in the issue.
Incidentally I'm working on investigating our options server-side and will address #8754 in the near future one way or another 👍.
README.md
Outdated
the `Client cannot use "password" grant_type` error. | ||
|
||
**Note**: Due to a known [issue](https://github.com/wordpress-mobile/WordPress-Android/issues/8754), login with a wordpress account with 2FA enabled is not currently fully supported on _development builds_. If you _must_ login with an account with 2FA enabled on development builds, please follow instructions listed in the workaround [here](DEV-BUILD-2FA-LOGIN-ISSUE.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't completely accurate - there are server-side restrictions to using 2FA accounts with personal OAuth client credentials that would remain even after we resolve the issue to deal with the failed account settings fetch during login more gracefully.
I would change this to say something like:
For security reasons, some account-related actions aren't supported for development builds when using a WordPress.com account with 2-factor authentication enabled. There is also currently an issue where a restart of the app is required to complete login in this case: [issue]
DEV-BUILD-2FA-LOGIN-ISSUE.md
Outdated
|
||
1. Login with the 2FA enabled account as normal following the steps detailed [here](https://github.com/AmirGamilDev/WordPress-Android#oauth2-authentication). | ||
2. After entering your 6-digit verification code and tapping "NEXT", you will receive an error message stating "couldn't retrieve your profile". You are now logged into the app. If you perform a full close of the app (for example, using the back button to clear the back stack) and open it again you should find yourself logged in but you won't see sites associated with this account. You can manually add self-hosted sites at this point. Or, if you wish to auto-retrieve the sites, continue on to step 3. | ||
3. Create another client id and secret pair using a different wordpress.com account that doesn't have 2FA enabled as described in the [OAuth2 Authentication](https://github.com/AmirGamilDev/WordPress-Android#oauth2-authentication) section of the Readme. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this is the case - after login appears to fail and the app restarted, the site list should be refreshed after a few moments. Changing the client id/secret after login is completed doesn't have any effect on whether fetching works (as outlined under "Bypass" in #8754).
…FA enabled and remove file detailing workaround
Looks good, thanks @AmirGamilDev! |
Add note to Readme and create new file to detail issue with 2FA not being fully supported on development builds and the workaround required.
Related to this issue:
#8754
Once this issue is resolved, the Readme content may be reverted.
Update release notes:
RELEASE-NOTES.txt
.