Skip to content

Commit 195efe7

Browse files
Grisha Kruglovpocmo
Grisha Kruglov
authored andcommitted
Make sure device finalization succeeds before proceeding to postAuth
1 parent 5000c01 commit 195efe7

File tree

4 files changed

+25
-15
lines changed

4 files changed

+25
-15
lines changed

components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,7 @@ open class FxaAccountManager(
598598
}
599599
AccountState.NotAuthenticated -> {
600600
when (via) {
601-
Event.Logout -> {
601+
Event.FailedToAuthenticate, Event.Logout -> {
602602
// Clean up internal account state and destroy the current FxA device record.
603603
if (account.disconnectAsync().await()) {
604604
logger.info("Disconnected FxA account")
@@ -619,7 +619,9 @@ open class FxaAccountManager(
619619
// Re-initialize account.
620620
account = createAccount(serverConfig)
621621

622-
notifyObservers { onLoggedOut() }
622+
if (via is Event.Logout) {
623+
notifyObservers { onLoggedOut() }
624+
}
623625

624626
null
625627
}
@@ -743,13 +745,14 @@ open class FxaAccountManager(
743745

744746
logger.info("Initializing device")
745747
// NB: underlying API is expected to 'ensureCapabilities' as part of device initialization.
746-
account.deviceConstellation().initDeviceAsync(
748+
if (account.deviceConstellation().initDeviceAsync(
747749
deviceConfig.name, deviceConfig.type, deviceConfig.capabilities
748-
).await()
749-
750-
postAuthenticated(via.authData.authType, via.authData.declinedEngines)
751-
752-
Event.FetchProfile
750+
).await()) {
751+
postAuthenticated(via.authData.authType, via.authData.declinedEngines)
752+
Event.FetchProfile
753+
} else {
754+
Event.FailedToAuthenticate
755+
}
753756
}
754757
Event.AccountRestored -> {
755758
logger.info("Registering persistence callback")
@@ -778,7 +781,7 @@ open class FxaAccountManager(
778781
logger.info("Registering device constellation observer")
779782
account.deviceConstellation().register(accountEventsIntegration)
780783

781-
if (via.reuseAccount) {
784+
val deviceFinalizeSuccess = if (via.reuseAccount) {
782785
logger.info("Configuring migrated account's device")
783786
// At the minimum, we need to "ensure capabilities" - that is, register for Send Tab, etc.
784787
account.deviceConstellation().ensureCapabilitiesAsync(deviceConfig.capabilities).await()
@@ -792,9 +795,12 @@ open class FxaAccountManager(
792795
).await()
793796
}
794797

795-
postAuthenticated(AuthType.Shared)
796-
797-
Event.FetchProfile
798+
if (deviceFinalizeSuccess) {
799+
postAuthenticated(AuthType.Shared)
800+
Event.FetchProfile
801+
} else {
802+
Event.FailedToAuthenticate
803+
}
798804
}
799805

800806
Event.RecoveredFromAuthenticationProblem -> {

components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/State.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ internal object FxaStateMatrix {
151151
Event.FetchProfile -> AccountState.AuthenticatedNoProfile
152152
Event.FetchedProfile -> AccountState.AuthenticatedWithProfile
153153
Event.FailedToFetchProfile -> AccountState.AuthenticatedNoProfile
154+
Event.FailedToAuthenticate -> AccountState.NotAuthenticated
154155
Event.Logout -> AccountState.NotAuthenticated
155156
else -> null
156157
}

components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaAccountManagerTest.kt

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,11 +1304,12 @@ class FxaAccountManagerTest {
13041304

13051305
assertNull(manager.authenticatedAccount())
13061306
assertNull(manager.accountProfile())
1307+
verify(accountStorage, times(1)).clear()
13071308

13081309
assertTrue(manager.finishAuthenticationAsync(FxaAuthData(AuthType.Signin, "dummyCode", EXPECTED_AUTH_STATE)).await())
13091310

13101311
verify(accountStorage, times(1)).read()
1311-
verify(accountStorage, never()).clear()
1312+
verify(accountStorage, times(1)).clear()
13121313

13131314
verify(accountObserver, times(1)).onAuthenticated(mockAccount, AuthType.Signin)
13141315
verify(accountObserver, times(1)).onProfileUpdated(profile)
@@ -1350,11 +1351,12 @@ class FxaAccountManagerTest {
13501351

13511352
assertNull(manager.authenticatedAccount())
13521353
assertNull(manager.accountProfile())
1354+
verify(accountStorage, times(1)).clear()
13531355

13541356
assertTrue(manager.finishAuthenticationAsync(FxaAuthData(AuthType.Signin, "dummyCode", EXPECTED_AUTH_STATE)).await())
13551357

13561358
verify(accountStorage, times(1)).read()
1357-
verify(accountStorage, never()).clear()
1359+
verify(accountStorage, times(1)).clear()
13581360

13591361
verify(accountObserver, times(1)).onAuthenticated(mockAccount, AuthType.Signin)
13601362
verify(accountObserver, times(1)).onProfileUpdated(profile)
@@ -1930,6 +1932,7 @@ class FxaAccountManagerTest {
19301932
): FxaAccountManager {
19311933
`when`(mockAccount.getProfileAsync(ignoreCache = false)).thenReturn(CompletableDeferred(profile))
19321934

1935+
`when`(mockAccount.disconnectAsync()).thenReturn(CompletableDeferred(true))
19331936
`when`(mockAccount.beginOAuthFlowAsync(any())).thenReturn(CompletableDeferred(value = null))
19341937
`when`(mockAccount.beginPairingFlowAsync(anyString(), any())).thenReturn(CompletableDeferred(value = null))
19351938
`when`(mockAccount.completeOAuthFlowAsync(anyString(), anyString())).thenReturn(CompletableDeferred(true))

components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/manager/FxaStateMatrixTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ class FxaStateMatrixTest {
6868
assertEquals(AccountState.AuthenticatedNoProfile, FxaStateMatrix.nextState(state, Event.FetchProfile))
6969
assertEquals(AccountState.AuthenticatedWithProfile, FxaStateMatrix.nextState(state, Event.FetchedProfile))
7070
assertEquals(AccountState.AuthenticatedNoProfile, FxaStateMatrix.nextState(state, Event.FailedToFetchProfile))
71-
assertNull(FxaStateMatrix.nextState(state, Event.FailedToAuthenticate))
71+
assertEquals(AccountState.NotAuthenticated, FxaStateMatrix.nextState(state, Event.FailedToAuthenticate))
7272
assertEquals(AccountState.NotAuthenticated, FxaStateMatrix.nextState(state, Event.Logout))
7373
assertEquals(AccountState.AuthenticationProblem, FxaStateMatrix.nextState(state, Event.AuthenticationError("test")))
7474
assertNull(FxaStateMatrix.nextState(state, Event.SignInShareableAccount(mock(), false)))

0 commit comments

Comments
 (0)