Skip to content

SIWA: fix the app not logging out after Apple credential is revoked when account has 2FA #2961

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 20, 2020
Merged

SIWA: fix the app not logging out after Apple credential is revoked when account has 2FA #2961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bdeca43
Save Apple ID to Keychain only when authentication is complete. Reset…
jaclync Oct 13, 2020
2b9fa18
Remove Apple ID from Keychain on logout.
jaclync Oct 13, 2020
a54db83
On `credentialRevokedNotification` from Apple, only log out the app i…
jaclync Oct 13, 2020
1269c85
Remove unused code from debugging.
jaclync Oct 13, 2020
2dafc0a
Fix trailing whitespace warning.
jaclync Oct 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion WooCommerce/Classes/Authentication/AppleIDCredentialChecker.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ final class AppleIDCredentialChecker {
if isLoggedIn {
self?.startObservingAppleIDCredentialRevoked()
} else {
self?.removeAppleIDFromKeychain()
self?.stopObservingAppleIDCredentialRevoked()
}
}
Expand Down Expand Up @@ -90,7 +91,9 @@ private extension AppleIDCredentialChecker {
guard let self = self else {
return
}
if self.isLoggedIn() {
// The user could have SIWA'ed earlier then changed to authenticate with another method, and thus the app still receives notifications on
// revoked Apple credentials. We only want to log out the app when the app is currently signed in with Apple (Apple ID saved in Keychain).
if self.isLoggedIn() && self.keychain.wooAppleID != nil {
DDLogInfo("Apple credentialRevokedNotification received. User signed out.")
self.logOutRevokedAppleAccount()
}
Expand Down
18 changes: 16 additions & 2 deletions WooCommerce/Classes/Authentication/AuthenticationManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ class AuthenticationManager: Authentication {
///
private lazy var keychain = Keychain(service: WooConstants.keychainServiceName)

/// Apple ID is temporarily stored in memory until we can save it to Keychain when the authentication is complete.
///
private var appleUserID: String?

/// Initializes the WordPress Authenticator.
///
func initialize() {
Expand Down Expand Up @@ -101,7 +105,11 @@ class AuthenticationManager: Authentication {
/// Displays the Login Flow using the specified UIViewController as presenter.
///
func displayAuthentication(from presenter: UIViewController, animated: Bool, onCompletion: @escaping () -> Void) {
WordPressAuthenticator.showLogin(from: presenter, animated: animated, onLoginButtonTapped: {
WordPressAuthenticator.showLogin(from: presenter, animated: animated, onLoginButtonTapped: { [weak self] in
guard let self = self else { return }
// Resets Apple ID at the beginning of the authentication.
self.appleUserID = nil

ServiceLocator.analytics.track(.loginPrologueContinueTapped)
}, onCompletion: onCompletion)
}
Expand Down Expand Up @@ -130,7 +138,7 @@ class AuthenticationManager: Authentication {
//
extension AuthenticationManager: WordPressAuthenticatorDelegate {
func userAuthenticatedWithAppleUserID(_ appleUserID: String) {
keychain.wooAppleID = appleUserID
self.appleUserID = appleUserID
}

var allowWPComLogin: Bool {
Expand Down Expand Up @@ -259,6 +267,12 @@ extension AuthenticationManager: WordPressAuthenticatorDelegate {
fatalError("Self Hosted sites are not supported. Please review the Authenticator settings!")
}

// If Apple ID is previously set, saves it to Keychain now that authentication is complete.
if let appleUserID = appleUserID {
keychain.wooAppleID = appleUserID
}
appleUserID = nil

ServiceLocator.stores.authenticate(credentials: .init(authToken: wpcom.authToken))
let action = AccountAction.synchronizeAccount { (account, error) in
if let account = account {
Expand Down
10 changes: 0 additions & 10 deletions WooCommerce/Classes/Authentication/Prologue/LoginPrologueViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,3 @@ private extension LoginPrologueViewController {
return disclaimerAttrText
}
}


// MARK: - (Private) Constants!
//
private enum Settings {

/// Login Button's Corner Radius
///
static let buttonCornerRadius = CGFloat(8.0)
}