Skip to content

Commit 1954eec

Browse files
itsmeichigoitsmeichigo
authored
Fix initialization of authenticator (#15953)
2 parents 9a24237 + df0a0df commit 1954eec

File tree

7 files changed

+45
-47
lines changed

7 files changed

+45
-47
lines changed

‎RELEASE-NOTES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
23.0
55
-----
6+
- [*] Fix initialization of authenticator to avoid crashes during login [https://github.com/woocommerce/woocommerce-ios/pull/15953]
67
- [*] Shipping Labels: Made HS tariff number field required in customs form for EU destinations [https://github.com/woocommerce/woocommerce-ios/pull/15946]
78

89
22.9

‎WooCommerce/Classes/Authentication/Epilogue/StorePickerViewController.swift‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,6 @@ private extension StorePickerViewController {
369369
}
370370

371371
func presentSiteDiscovery() {
372-
ServiceLocator.authenticationManager.initialize()
373372
guard let viewController = WordPressAuthenticator.siteDiscoveryUI() else {
374373
return
375374
}

‎WooCommerce/Classes/ViewRelated/AppCoordinator.swift‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,6 @@ final class AppCoordinator {
8989
self.displayLoggedInStateWithoutDefaultStore()
9090
case (true, false):
9191
self.validateRoleEligibility {
92-
self.configureAuthenticator()
9392
self.displayLoggedInUI()
9493
self.synchronizeAndShowWhatsNew()
9594
}
@@ -200,19 +199,13 @@ private extension AppCoordinator {
200199
presentLoginOnboarding { [weak self] in
201200
guard let self = self else { return }
202201
// Only displays the authenticator when dismissing onboarding to allow time for A/B test setup.
203-
self.configureAndDisplayAuthenticator()
202+
self.displayAuthenticator()
204203
}
205204
} else {
206-
configureAndDisplayAuthenticator()
205+
displayAuthenticator()
207206
}
208207
}
209208

210-
/// Configures the WPAuthenticator and sets the authenticator UI as the window's root view.
211-
func configureAndDisplayAuthenticator() {
212-
configureAuthenticator()
213-
displayAuthenticator()
214-
}
215-
216209
/// Configures the WPAuthenticator for usage in both logged-in and logged-out states.
217210
func configureAuthenticator() {
218211
authenticationManager.initialize()
@@ -296,7 +289,6 @@ private extension AppCoordinator {
296289
guard stores.isAuthenticatedWithoutWPCom == false else {
297290
return displayAuthenticatorWithOnboardingIfNeeded()
298291
}
299-
configureAuthenticator()
300292

301293
let matcher = ULAccountMatcher(storageManager: storageManager)
302294
matcher.refreshStoredSites()

‎WooCommerce/Classes/ViewRelated/JetpackSetup/JetpackSetupCoordinator.swift‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ final class JetpackSetupCoordinator {
1919
private let stores: StoresManager
2020
private let analytics: Analytics
2121
private let featureFlagService: FeatureFlagService
22-
private let dotcomAuthScheme: String
2322

2423
private var loginNavigationController: LoginNavigationController?
2524
private var setupStepsNavigationController: UINavigationController?
@@ -43,24 +42,18 @@ final class JetpackSetupCoordinator {
4342
}
4443

4544
init(site: Site,
46-
dotcomAuthScheme: String = ApiCredentials.dotcomAuthScheme,
4745
rootViewController: UIViewController,
4846
accountService: WordPressComAccountServiceProtocol = WordPressComAccountService(),
4947
stores: StoresManager = ServiceLocator.stores,
5048
analytics: Analytics = ServiceLocator.analytics,
5149
featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService) {
5250
self.site = site
53-
self.dotcomAuthScheme = dotcomAuthScheme
5451
self.requiresConnectionOnly = false // to be updated later after fetching Jetpack status
5552
self.rootViewController = rootViewController
5653
self.accountService = accountService
5754
self.stores = stores
5855
self.analytics = analytics
5956
self.featureFlagService = featureFlagService
60-
61-
/// the authenticator needs to be initialized with configs
62-
/// to be used for requesting authentication link and handle login later.
63-
WordPressAuthenticator.initializeWithCustomConfigs(dotcomAuthScheme: dotcomAuthScheme)
6457
}
6558

6659
func showBenefitModal() {
@@ -72,7 +65,7 @@ final class JetpackSetupCoordinator {
7265
rootViewController.present(benefitsController, animated: true, completion: nil)
7366
}
7467

75-
func handleAuthenticationUrl(_ url: URL) -> Bool {
68+
func handleAuthenticationUrl(_ url: URL, dotcomAuthScheme: String = ApiCredentials.dotcomAuthScheme) -> Bool {
7669
let expectedPrefix = dotcomAuthScheme + "://" + Constants.magicLinkUrlHostname
7770
guard url.absoluteString.hasPrefix(expectedPrefix) else {
7871
return false

‎WooCommerce/WooCommerceTests/ViewRelated/JetpackSetup/JetpackSetupCoordinatorTests.swift‎

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import WordPressAuthenticator
66
final class JetpackSetupCoordinatorTests: XCTestCase {
77

88
private var navigationController: UINavigationController!
9+
private let dotcomAuthScheme = "scheme"
910

1011
override func setUp() {
1112
navigationController = UINavigationController()
@@ -14,6 +15,9 @@ final class JetpackSetupCoordinatorTests: XCTestCase {
1415
window.rootViewController = UIViewController()
1516
window.makeKeyAndVisible()
1617
window.rootViewController = navigationController
18+
19+
AuthenticationManager().initialize()
20+
1721
super.setUp()
1822
}
1923

@@ -40,12 +44,11 @@ final class JetpackSetupCoordinatorTests: XCTestCase {
4044
func test_handleAuthenticationUrl_returns_false_for_unsupported_url_scheme() throws {
4145
// Given
4246
let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID)
43-
let expectedScheme = "scheme"
44-
let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController)
47+
let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController)
4548
let url = try XCTUnwrap(URL(string: "example://handle-authentication"))
4649

4750
// When
48-
let result = coordinator.handleAuthenticationUrl(url)
51+
let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme)
4952

5053
// Then
5154
XCTAssertFalse(result)
@@ -54,12 +57,11 @@ final class JetpackSetupCoordinatorTests: XCTestCase {
5457
func test_handleAuthenticationUrl_returns_false_for_missing_queries() throws {
5558
// Given
5659
let testSite = Site.fake().copy(siteID: -1)
57-
let expectedScheme = "scheme"
58-
let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController)
59-
let url = try XCTUnwrap(URL(string: "scheme://magic-login"))
60+
let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController)
61+
let url = try XCTUnwrap(URL(string: "\(dotcomAuthScheme)://magic-login"))
6062

6163
// When
62-
let result = coordinator.handleAuthenticationUrl(url)
64+
let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme)
6365

6466
// Then
6567
XCTAssertFalse(result)
@@ -68,12 +70,11 @@ final class JetpackSetupCoordinatorTests: XCTestCase {
6870
func test_handleAuthenticationUrl_returns_false_for_incorrect_host_name() throws {
6971
// Given
7072
let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID)
71-
let expectedScheme = "scheme"
72-
let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController)
73-
let url = try XCTUnwrap(URL(string: "scheme://handle-authentication?token=test"))
73+
let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController)
74+
let url = try XCTUnwrap(URL(string: "\(dotcomAuthScheme)://handle-authentication?token=test"))
7475

7576
// When
76-
let result = coordinator.handleAuthenticationUrl(url)
77+
let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme)
7778

7879
// Then
7980
XCTAssertFalse(result)
@@ -82,12 +83,11 @@ final class JetpackSetupCoordinatorTests: XCTestCase {
8283
func test_handleAuthenticationUrl_returns_true_for_correct_url_and_sufficient_queries() throws {
8384
// Given
8485
let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID)
85-
let expectedScheme = "scheme"
86-
let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController)
87-
let url = try XCTUnwrap(URL(string: "scheme://magic-login?token=test"))
86+
let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController)
87+
let url = try XCTUnwrap(URL(string: "\(dotcomAuthScheme)://magic-login?token=test"))
8888

8989
// When
90-
let result = coordinator.handleAuthenticationUrl(url)
90+
let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme)
9191

9292
// Then
9393
XCTAssertTrue(result)
@@ -97,9 +97,8 @@ final class JetpackSetupCoordinatorTests: XCTestCase {
9797
// Given
9898
let stores = MockStoresManager(sessionManager: .makeForTesting(authenticated: true, isWPCom: false, defaultRoles: [.shopManager]))
9999
let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID)
100-
let expectedScheme = "scheme"
101-
let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController, stores: stores)
102-
let url = try XCTUnwrap(URL(string: "scheme://magic-login?token=test"))
100+
let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController, stores: stores)
101+
let url = try XCTUnwrap(URL(string: "\(dotcomAuthScheme)://magic-login?token=test"))
103102

104103
let expectedAccount = Account(userID: 123, displayName: "Test", email: "[email protected]", username: "test", gravatarUrl: nil)
105104
stores.whenReceivingAction(ofType: JetpackConnectionAction.self) { action in
@@ -115,9 +114,10 @@ final class JetpackSetupCoordinatorTests: XCTestCase {
115114
stores.mockJetpackCheck()
116115

117116
// When
118-
_ = coordinator.handleAuthenticationUrl(url)
117+
let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme)
119118

120119
// Then
120+
XCTAssertTrue(result)
121121
waitUntil {
122122
(self.navigationController.presentedViewController as? UINavigationController)?.topViewController is AdminRoleRequiredHostingController
123123
}
@@ -127,9 +127,9 @@ final class JetpackSetupCoordinatorTests: XCTestCase {
127127
// Given
128128
let stores = MockStoresManager(sessionManager: .makeForTesting(authenticated: true, isWPCom: false))
129129
let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID)
130+
let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController, stores: stores)
130131
let expectedScheme = "scheme"
131-
let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController, stores: stores)
132-
let url = try XCTUnwrap(URL(string: "scheme://magic-login?token=test"))
132+
let url = try XCTUnwrap(URL(string: "\(dotcomAuthScheme)://magic-login?token=test"))
133133

134134
let expectedAccount = Account(userID: 123, displayName: "Test", email: "[email protected]", username: "test", gravatarUrl: nil)
135135
stores.whenReceivingAction(ofType: JetpackConnectionAction.self) { action in
@@ -145,9 +145,10 @@ final class JetpackSetupCoordinatorTests: XCTestCase {
145145
stores.mockJetpackCheck()
146146

147147
// When
148-
_ = coordinator.handleAuthenticationUrl(url)
148+
let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme)
149149

150150
// Then
151+
XCTAssertTrue(result)
151152
waitUntil {
152153
(self.navigationController.presentedViewController as? UINavigationController)?.topViewController is JetpackSetupHostingController
153154
}
@@ -158,9 +159,8 @@ final class JetpackSetupCoordinatorTests: XCTestCase {
158159
let stores = MockStoresManager(sessionManager: .makeForTesting(authenticated: true, isWPCom: false))
159160
let siteURL = "https://example.com"
160161
let testSite = Site.fake().copy(siteID: WooConstants.placeholderStoreID, url: siteURL)
161-
let expectedScheme = "scheme"
162-
let coordinator = JetpackSetupCoordinator(site: testSite, dotcomAuthScheme: expectedScheme, rootViewController: navigationController, stores: stores)
163-
let url = try XCTUnwrap(URL(string: "scheme://magic-login?token=test"))
162+
let coordinator = JetpackSetupCoordinator(site: testSite, rootViewController: navigationController, stores: stores)
163+
let url = try XCTUnwrap(URL(string: "\(dotcomAuthScheme)://magic-login?token=test"))
164164

165165
let expectedAccount = Account(userID: 123, displayName: "Test", email: "[email protected]", username: "test", gravatarUrl: nil)
166166
stores.whenReceivingAction(ofType: JetpackConnectionAction.self) { action in
@@ -200,9 +200,10 @@ final class JetpackSetupCoordinatorTests: XCTestCase {
200200
stores.mockJetpackCheck()
201201

202202
// When
203-
_ = coordinator.handleAuthenticationUrl(url)
203+
let result = coordinator.handleAuthenticationUrl(url, dotcomAuthScheme: dotcomAuthScheme)
204204

205205
// Then
206+
XCTAssertTrue(result)
206207
waitUntil {
207208
stores.sessionManager.defaultSite == expectedSite
208209
}

‎WooCommerce/WordPressAuthenticator/Authenticator/WordPressAuthenticator.swift‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,12 @@ import WordPressUI
8484
unifiedStyle: WordPressAuthenticatorUnifiedStyle?,
8585
displayImages: WordPressAuthenticatorDisplayImages = .defaultImages,
8686
displayStrings: WordPressAuthenticatorDisplayStrings = .defaultStrings) {
87+
guard privateInstance == nil else {
88+
if isRunningTests() == false { // avoid crashing in tests
89+
assertionFailure("Attempting to initialize WordPressAuthenticator more than once - this would cause the delegate to be reset. Aborting... 🙅⛔️")
90+
}
91+
return
92+
}
8793
privateInstance = WordPressAuthenticator(configuration: configuration,
8894
style: style,
8995
unifiedStyle: unifiedStyle,
@@ -487,3 +493,9 @@ public extension WordPressAuthenticator {
487493
appleIDCredentialObserver = nil
488494
}
489495
}
496+
497+
private extension WordPressAuthenticator {
498+
static func isRunningTests() -> Bool {
499+
return NSClassFromString("XCTestCase") != nil
500+
}
501+
}

‎WooCommerce/WordPressAuthenticator/Signin/LoginViewController.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ open class LoginViewController: NUXViewController, LoginFacadeDelegate {
3232

3333
var authenticationDelegate: WordPressAuthenticatorDelegate {
3434
guard let delegate = WordPressAuthenticator.shared.delegate else {
35-
fatalError()
35+
fatalError("No delegate found for WordPressAuthenticator")
3636
}
3737

3838
return delegate
@@ -140,7 +140,7 @@ open class LoginViewController: NUXViewController, LoginFacadeDelegate {
140140

141141
func showLoginEpilogue(for credentials: AuthenticatorCredentials) {
142142
guard let navigationController = navigationController else {
143-
fatalError()
143+
fatalError("No navigation controller found to show login epilogue")
144144
}
145145

146146
authenticationDelegate.presentLoginEpilogue(in: navigationController,

0 commit comments

Comments
 (0)