Skip to content
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

[SIWA] Handle credential state changes #12585

Merged

Conversation

@ScoutHarris
Copy link
Contributor

commented Sep 30, 2019

Ref #12399
Related WPAuth PR: wordpress-mobile/WordPressAuthenticator-iOS#139

This uses the WPAuth changes to:

  • Accept the CredentialState provided by WPAuth.
  • If the user id is revoked, log out of the account.

There should be no visible difference from the previous implementation.

To test:

This should be tested on a real device for proper keychain handling.

  • Log in / sign up with Apple.
  • Background the app.
  • Disconnect in your Apple account on the device.
    • Go to iOS Settings.
    • Select your iCloud account at the top.
    • Go to Password & Security > Apps Using Your Apple ID.
    • Select WordPress > Stop Using Apple ID.
  • Open the app.
  • The app should log you out.

Update release notes:

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.
Copy link
Contributor

left a comment

Looks good! One request for a change to fix a bug I found. We can address it separately if you like, but it might make sense to roll it in here :)

DDLogInfo("checkAppleIDCredentialState: Unauthorized Apple ID. User signed out.")
switch state {
case .revoked:
DDLogInfo("checkAppleIDCredentialState: Revoked Apple ID. User signed out.")
self?.logOutDefaultWordPressComAccount()

This comment has been minimized.

Copy link
@frosty

frosty Oct 1, 2019

Contributor

We should also ensure we delete the Apple ID from the keychain on log out. I noticed an issue with my device where it somehow still has the Apple ID in the keychain, even though I logged out and logged back in with a different account. Because of this, each time we do the apple credential state check here, the app sees I have a revoked ID and logs me out again.

This comment has been minimized.

Copy link
@ScoutHarris

ScoutHarris Oct 1, 2019

Author Contributor

Hey @frosty . Nice catch! It has been done. See what you think now. Thanks!

@ScoutHarris ScoutHarris requested a review from frosty Oct 1, 2019
@frosty
frosty approved these changes Oct 2, 2019
@ScoutHarris ScoutHarris merged commit d3ac4a0 into develop Oct 2, 2019
6 checks passed
6 checks passed
Hound No violations found. Woof!
Peril All green. Good on 'ya.
Details
ci/circleci: Build Tests Your tests passed on CircleCI!
Details
ci/circleci: UI Tests (iPad Air 3rd generation) Your tests passed on CircleCI!
Details
ci/circleci: UI Tests (iPhone 11) Your tests passed on CircleCI!
Details
ci/circleci: Unit Tests Your tests passed on CircleCI!
Details
@ScoutHarris ScoutHarris deleted the issue/12399-siwa_handle_credential_state_changes branch Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.