Fix initialization of authenticator

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

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
         }

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()

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

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: "[email protected]", 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: "[email protected]", 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: "[email protected]", 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
         }

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
+    }
+}

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,