diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 3a15f4fb8da..200d741221d 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -3,6 +3,7 @@ 23.0 ----- +- [*] Fix initialization of authenticator to avoid crashes during login [https://github.com/woocommerce/woocommerce-ios/pull/15953] - [*] Shipping Labels: Made HS tariff number field required in customs form for EU destinations [https://github.com/woocommerce/woocommerce-ios/pull/15946] 22.9 diff --git a/WooCommerce/Classes/Authentication/Epilogue/StorePickerViewController.swift b/WooCommerce/Classes/Authentication/Epilogue/StorePickerViewController.swift index 8ae0f6354e3..05e0d565d7d 100644 --- a/WooCommerce/Classes/Authentication/Epilogue/StorePickerViewController.swift +++ b/WooCommerce/Classes/Authentication/Epilogue/StorePickerViewController.swift @@ -369,7 +369,6 @@ private extension StorePickerViewController { } func presentSiteDiscovery() { - ServiceLocator.authenticationManager.initialize() guard let viewController = WordPressAuthenticator.siteDiscoveryUI() else { return } diff --git a/WooCommerce/Classes/ViewRelated/AppCoordinator.swift b/WooCommerce/Classes/ViewRelated/AppCoordinator.swift index 55ce1a42176..b400f9327c4 100644 --- a/WooCommerce/Classes/ViewRelated/AppCoordinator.swift +++ b/WooCommerce/Classes/ViewRelated/AppCoordinator.swift @@ -89,7 +89,6 @@ final class AppCoordinator { self.displayLoggedInStateWithoutDefaultStore() case (true, false): self.validateRoleEligibility { - self.configureAuthenticator() self.displayLoggedInUI() self.synchronizeAndShowWhatsNew() } @@ -200,19 +199,13 @@ private extension AppCoordinator { presentLoginOnboarding { [weak self] in guard let self = self else { return } // Only displays the authenticator when dismissing onboarding to allow time for A/B test setup. - self.configureAndDisplayAuthenticator() + self.displayAuthenticator() } } else { - configureAndDisplayAuthenticator() + displayAuthenticator() } } - /// Configures the WPAuthenticator and sets the authenticator UI as the window's root view. - func configureAndDisplayAuthenticator() { - configureAuthenticator() - displayAuthenticator() - } - /// Configures the WPAuthenticator for usage in both logged-in and logged-out states. func configureAuthenticator() { authenticationManager.initialize() @@ -296,7 +289,6 @@ private extension AppCoordinator { guard stores.isAuthenticatedWithoutWPCom == false else { return displayAuthenticatorWithOnboardingIfNeeded() } - configureAuthenticator() let matcher = ULAccountMatcher(storageManager: storageManager) matcher.refreshStoredSites() diff --git a/WooCommerce/Classes/ViewRelated/JetpackSetup/JetpackSetupCoordinator.swift b/WooCommerce/Classes/ViewRelated/JetpackSetup/JetpackSetupCoordinator.swift index 9884af711c7..8837f3a4e8b 100644 --- a/WooCommerce/Classes/ViewRelated/JetpackSetup/JetpackSetupCoordinator.swift +++ b/WooCommerce/Classes/ViewRelated/JetpackSetup/JetpackSetupCoordinator.swift @@ -19,7 +19,6 @@ final class JetpackSetupCoordinator { private let stores: StoresManager private let analytics: Analytics private let featureFlagService: FeatureFlagService - private let dotcomAuthScheme: String private var loginNavigationController: LoginNavigationController? private var setupStepsNavigationController: UINavigationController? @@ -43,24 +42,18 @@ final class JetpackSetupCoordinator { } init(site: Site, - dotcomAuthScheme: String = ApiCredentials.dotcomAuthScheme, rootViewController: UIViewController, accountService: WordPressComAccountServiceProtocol = WordPressComAccountService(), stores: StoresManager = ServiceLocator.stores, analytics: Analytics = ServiceLocator.analytics, featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService) { self.site = site - self.dotcomAuthScheme = dotcomAuthScheme self.requiresConnectionOnly = false // to be updated later after fetching Jetpack status self.rootViewController = rootViewController self.accountService = accountService self.stores = stores self.analytics = analytics self.featureFlagService = featureFlagService - - /// the authenticator needs to be initialized with configs - /// to be used for requesting authentication link and handle login later. - WordPressAuthenticator.initializeWithCustomConfigs(dotcomAuthScheme: dotcomAuthScheme) } func showBenefitModal() { @@ -72,7 +65,7 @@ final class JetpackSetupCoordinator { rootViewController.present(benefitsController, animated: true, completion: nil) } - func handleAuthenticationUrl(_ url: URL) -> Bool { + func handleAuthenticationUrl(_ url: URL, dotcomAuthScheme: String = ApiCredentials.dotcomAuthScheme) -> Bool { let expectedPrefix = dotcomAuthScheme + "://" + Constants.magicLinkUrlHostname guard url.absoluteString.hasPrefix(expectedPrefix) else { return false diff --git a/WooCommerce/WooCommerceTests/ViewRelated/JetpackSetup/JetpackSetupCoordinatorTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/JetpackSetup/JetpackSetupCoordinatorTests.swift index e67b99102ba..723459c1ca8 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/JetpackSetup/JetpackSetupCoordinatorTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/JetpackSetup/JetpackSetupCoordinatorTests.swift @@ -6,6 +6,7 @@ import WordPressAuthenticator final class JetpackSetupCoordinatorTests: XCTestCase { private var navigationController: UINavigationController! + private let dotcomAuthScheme = "scheme" override func setUp() { navigationController = UINavigationController() @@ -14,6 +15,9 @@ final class JetpackSetupCoordinatorTests: XCTestCase { window.rootViewController = UIViewController() window.makeKeyAndVisible() window.rootViewController = navigationController + + AuthenticationManager().initialize() + super.setUp() } @@ -40,12 +44,11 @@ final class JetpackSetupCoordinatorTests: XCTestCase { func test_handleAuthenticationUrl_returns_false_for_unsupported_url_scheme() throws { // Given let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID) - let expectedScheme = "scheme" - let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController) + let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController) let url = try XCTUnwrap(URL(string: "example://handle-authentication")) // When - let result = coordinator.handleAuthenticationUrl(url) + let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme) // Then XCTAssertFalse(result) @@ -54,12 +57,11 @@ final class JetpackSetupCoordinatorTests: XCTestCase { func test_handleAuthenticationUrl_returns_false_for_missing_queries() throws { // Given let testSite = Site.fake().copy(siteID: -1) - let expectedScheme = "scheme" - let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController) - let url = try XCTUnwrap(URL(string: "scheme://magic-login")) + let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController) + let url = try XCTUnwrap(URL(string: "\(dotcomAuthScheme)://magic-login")) // When - let result = coordinator.handleAuthenticationUrl(url) + let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme) // Then XCTAssertFalse(result) @@ -68,12 +70,11 @@ final class JetpackSetupCoordinatorTests: XCTestCase { func test_handleAuthenticationUrl_returns_false_for_incorrect_host_name() throws { // Given let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID) - let expectedScheme = "scheme" - let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController) - let url = try XCTUnwrap(URL(string: "scheme://handle-authentication?token=test")) + let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController) + let url = try XCTUnwrap(URL(string: "\(dotcomAuthScheme)://handle-authentication?token=test")) // When - let result = coordinator.handleAuthenticationUrl(url) + let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme) // Then XCTAssertFalse(result) @@ -82,12 +83,11 @@ final class JetpackSetupCoordinatorTests: XCTestCase { func test_handleAuthenticationUrl_returns_true_for_correct_url_and_sufficient_queries() throws { // Given let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID) - let expectedScheme = "scheme" - let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController) - let url = try XCTUnwrap(URL(string: "scheme://magic-login?token=test")) + let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController) + let url = try XCTUnwrap(URL(string: "\(dotcomAuthScheme)://magic-login?token=test")) // When - let result = coordinator.handleAuthenticationUrl(url) + let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme) // Then XCTAssertTrue(result) @@ -97,9 +97,8 @@ final class JetpackSetupCoordinatorTests: XCTestCase { // Given let stores = MockStoresManager(sessionManager: .makeForTesting(authenticated: true, isWPCom: false, defaultRoles: [.shopManager])) let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID) - let expectedScheme = "scheme" - let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController, stores: stores) - let url = try XCTUnwrap(URL(string: "scheme://magic-login?token=test")) + let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController, stores: stores) + let url = try XCTUnwrap(URL(string: "\(dotcomAuthScheme)://magic-login?token=test")) let expectedAccount = Account(userID: 123, displayName: "Test", email: "test@example.com", username: "test", gravatarUrl: nil) stores.whenReceivingAction(ofType: JetpackConnectionAction.self) { action in @@ -115,9 +114,10 @@ final class JetpackSetupCoordinatorTests: XCTestCase { stores.mockJetpackCheck() // When - _ = coordinator.handleAuthenticationUrl(url) + let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme) // Then + XCTAssertTrue(result) waitUntil { (self.navigationController.presentedViewController as? UINavigationController)?.topViewController is AdminRoleRequiredHostingController } @@ -127,9 +127,9 @@ final class JetpackSetupCoordinatorTests: XCTestCase { // Given let stores = MockStoresManager(sessionManager: .makeForTesting(authenticated: true, isWPCom: false)) let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID) + let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController, stores: stores) let expectedScheme = "scheme" - let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController, stores: stores) - let url = try XCTUnwrap(URL(string: "scheme://magic-login?token=test")) + let url = try XCTUnwrap(URL(string: "\(dotcomAuthScheme)://magic-login?token=test")) let expectedAccount = Account(userID: 123, displayName: "Test", email: "test@example.com", username: "test", gravatarUrl: nil) stores.whenReceivingAction(ofType: JetpackConnectionAction.self) { action in @@ -145,9 +145,10 @@ final class JetpackSetupCoordinatorTests: XCTestCase { stores.mockJetpackCheck() // When - _ = coordinator.handleAuthenticationUrl(url) + let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme) // Then + XCTAssertTrue(result) waitUntil { (self.navigationController.presentedViewController as? UINavigationController)?.topViewController is JetpackSetupHostingController } @@ -158,9 +159,8 @@ final class JetpackSetupCoordinatorTests: XCTestCase { let stores = MockStoresManager(sessionManager: .makeForTesting(authenticated: true, isWPCom: false)) let siteURL = "https://example.com" let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID, url: siteURL) - let expectedScheme = "scheme" - let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController, stores: stores) - let url = try XCTUnwrap(URL(string: "scheme://magic-login?token=test")) + let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController, stores: stores) + let url = try XCTUnwrap(URL(string: "\(dotcomAuthScheme)://magic-login?token=test")) let expectedAccount = Account(userID: 123, displayName: "Test", email: "test@example.com", username: "test", gravatarUrl: nil) stores.whenReceivingAction(ofType: JetpackConnectionAction.self) { action in @@ -200,9 +200,10 @@ final class JetpackSetupCoordinatorTests: XCTestCase { stores.mockJetpackCheck() // When - _ = coordinator.handleAuthenticationUrl(url) + let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme) // Then + XCTAssertTrue(result) waitUntil { stores.sessionManager.defaultSite == expectedSite } diff --git a/WooCommerce/WordPressAuthenticator/Authenticator/WordPressAuthenticator.swift b/WooCommerce/WordPressAuthenticator/Authenticator/WordPressAuthenticator.swift index 035112d9158..f8b46949157 100644 --- a/WooCommerce/WordPressAuthenticator/Authenticator/WordPressAuthenticator.swift +++ b/WooCommerce/WordPressAuthenticator/Authenticator/WordPressAuthenticator.swift @@ -84,6 +84,12 @@ import WordPressUI unifiedStyle: WordPressAuthenticatorUnifiedStyle?, displayImages: WordPressAuthenticatorDisplayImages = .defaultImages, displayStrings: WordPressAuthenticatorDisplayStrings = .defaultStrings) { + guard privateInstance == nil else { + if isRunningTests() == false { // avoid crashing in tests + assertionFailure("Attempting to initialize WordPressAuthenticator more than once - this would cause the delegate to be reset. Aborting... 🙅⛔️") + } + return + } privateInstance = WordPressAuthenticator(configuration: configuration, style: style, unifiedStyle: unifiedStyle, @@ -487,3 +493,9 @@ public extension WordPressAuthenticator { appleIDCredentialObserver = nil } } + +private extension WordPressAuthenticator { + static func isRunningTests() -> Bool { + return NSClassFromString("XCTestCase") != nil + } +} diff --git a/WooCommerce/WordPressAuthenticator/Signin/LoginViewController.swift b/WooCommerce/WordPressAuthenticator/Signin/LoginViewController.swift index 5def77a82a3..760f4cc9593 100644 --- a/WooCommerce/WordPressAuthenticator/Signin/LoginViewController.swift +++ b/WooCommerce/WordPressAuthenticator/Signin/LoginViewController.swift @@ -32,7 +32,7 @@ open class LoginViewController: NUXViewController, LoginFacadeDelegate { var authenticationDelegate: WordPressAuthenticatorDelegate { guard let delegate = WordPressAuthenticator.shared.delegate else { - fatalError() + fatalError("No delegate found for WordPressAuthenticator") } return delegate @@ -140,7 +140,7 @@ open class LoginViewController: NUXViewController, LoginFacadeDelegate { func showLoginEpilogue(for credentials: AuthenticatorCredentials) { guard let navigationController = navigationController else { - fatalError() + fatalError("No navigation controller found to show login epilogue") } authenticationDelegate.presentLoginEpilogue(in: navigationController,