[PM-16687] Sync before determining if we need 2FA notice (#1251)

Author: KatherineInCode

BitwardenShared/Core/Platform/Services/Services.swift

@@ -37,6 +37,7 @@ typealias Services = HasAPIService
     & HasSendRepository
     & HasSettingsRepository
     & HasStateService
+    & HasSyncService
     & HasSystemDevice
     & HasTOTPExpirationManagerFactory
     & HasTOTPService
@@ -300,6 +301,13 @@ protocol HasStateService {
     var stateService: StateService { get }
 }
 
+/// Protocol for an object that has a `SyncService`.
+///
+protocol HasSyncService {
+    /// The service used by the application to sync account data.
+    var syncService: SyncService { get }
+}
+
 /// Protocol for an object that provides a `SystemDevice`.
 ///
 protocol HasSystemDevice {

BitwardenShared/UI/Auth/TwoFactorNotice/SetUpTwoFactor/SetUpTwoFactorAction.swift

@@ -5,10 +5,4 @@
 enum SetUpTwoFactorAction: Equatable, Sendable {
     /// The url has been opened so clear the value in the state.
     case clearURL
-
-    /// The user tapped the button to turn on two-factor authentication.
-    case turnOnTwoFactorTapped
-
-    /// The user tapped the button to change email.
-    case changeAccountEmailTapped
 }

BitwardenShared/UI/Auth/TwoFactorNotice/SetUpTwoFactor/SetUpTwoFactorEffect.swift

@@ -3,6 +3,12 @@
 /// Effects that can be processed by a `SetUpTwoFactorProcessor`.
 ///
 enum SetUpTwoFactorEffect: Equatable, Sendable {
+    /// The user tapped the button to change email.
+    case changeAccountEmailTapped
+
     /// The user tapped the "remind me later" button.
     case remindMeLaterTapped
+
+    /// The user tapped the button to turn on two-factor authentication.
+    case turnOnTwoFactorTapped
 }

BitwardenShared/UI/Auth/TwoFactorNotice/SetUpTwoFactor/SetUpTwoFactorProcessor.swift

@@ -45,28 +45,39 @@ class SetUpTwoFactorProcessor: StateProcessor<SetUpTwoFactorState, SetUpTwoFacto
 
     override func perform(_ effect: SetUpTwoFactorEffect) async {
         switch effect {
+        case .changeAccountEmailTapped:
+            await handleChangeAccountEmail()
         case .remindMeLaterTapped:
             await handleDismiss()
+        case .turnOnTwoFactorTapped:
+            await handleTurnOnTwoFactor()
         }
     }
 
     override func receive(_ action: SetUpTwoFactorAction) {
         switch action {
-        case .changeAccountEmailTapped:
-            coordinator.showAlert(.changeEmailAlert {
-                self.state.url = self.services.environmentService.changeEmailURL
-            })
         case .clearURL:
             state.url = nil
-        case .turnOnTwoFactorTapped:
-            coordinator.showAlert(.turnOnTwoFactorLoginAlert {
-                self.state.url = self.services.environmentService.setUpTwoFactorURL
-            })
         }
     }
 
     // MARK: Private Methods
 
+    /// Handles when the user taps the Change Account Email button. This will
+    /// give the user the ability to go to the website to change their email, and
+    /// will in the process nil the last sync time so the next time the app launches
+    /// it will be able to sync immediately.
+    private func handleChangeAccountEmail() async {
+        do {
+            try await services.stateService.setLastSyncTime(nil)
+            coordinator.showAlert(.changeEmailAlert {
+                self.state.url = self.services.environmentService.changeEmailURL
+            })
+        } catch {
+            services.errorReporter.log(error: error)
+        }
+    }
+
     /// Saves the current time to disk when the user dismisses the notice.
     private func handleDismiss() async {
         do {
@@ -77,4 +88,19 @@ class SetUpTwoFactorProcessor: StateProcessor<SetUpTwoFactorState, SetUpTwoFacto
             services.errorReporter.log(error: error)
         }
     }
+
+    /// Handles when the user taps the Turn On Two-Step button. This will
+    /// give the user the ability to go to the website to change their email, and
+    /// will in the process nil the last sync time so the next time the app launches
+    /// it will be able to sync immediately.
+    private func handleTurnOnTwoFactor() async {
+        do {
+            try await services.stateService.setLastSyncTime(nil)
+            coordinator.showAlert(.turnOnTwoFactorLoginAlert {
+                self.state.url = self.services.environmentService.setUpTwoFactorURL
+            })
+        } catch {
+            services.errorReporter.log(error: error)
+        }
+    }
 }

BitwardenShared/UI/Auth/TwoFactorNotice/SetUpTwoFactor/SetUpTwoFactorProcessorTests.swift

@@ -37,6 +37,10 @@ class SetUpTwoFactorProcessorTests: BitwardenTestCase {
             services: services,
             state: SetUpTwoFactorState(allowDelay: true, emailAddress: "person@example.com")
         )
+
+        let account = Account.fixture()
+        stateService.activeAccount = account
+        stateService.lastSyncTimeByUserId["1"] = timeProvider.presentTime
     }
 
     override func tearDown() {
@@ -56,8 +60,6 @@ class SetUpTwoFactorProcessorTests: BitwardenTestCase {
     /// then dismisses
     @MainActor
     func test_perform_remindMeLaterTapped() async {
-        let account = Account.fixture()
-        stateService.activeAccount = account
         await subject.perform(.remindMeLaterTapped)
         XCTAssertEqual(
             stateService.twoFactorNoticeDisplayState["1"],
@@ -69,8 +71,6 @@ class SetUpTwoFactorProcessorTests: BitwardenTestCase {
     /// `.perform(_:)` with `.remindMeLaterTapped` handles errors
     @MainActor
     func test_perform_remindMeLaterTapped_error() async {
-        let account = Account.fixture()
-        stateService.activeAccount = account
         stateService.setTwoFactorNoticeDisplayStateError = BitwardenTestError.example
         await subject.perform(.remindMeLaterTapped)
         XCTAssertEqual(
@@ -85,10 +85,12 @@ class SetUpTwoFactorProcessorTests: BitwardenTestCase {
     func test_receive_changeAccountEmailTapped() async throws {
         let url = URL("https://www.example.com")!
         environmentService.changeEmailURL = url
-        subject.receive(.changeAccountEmailTapped)
+        await subject.perform(.changeAccountEmailTapped)
+        waitFor(!coordinator.alertShown.isEmpty)
         let alert = try XCTUnwrap(coordinator.alertShown.last)
         try await alert.tapAction(title: Localizations.continue)
         XCTAssertEqual(subject.state.url, url)
+        XCTAssertNil(stateService.lastSyncTimeByUserId["1"])
     }
 
     /// `receive(_:)` with `.clearURL` clears the URL in the state.
@@ -105,9 +107,11 @@ class SetUpTwoFactorProcessorTests: BitwardenTestCase {
     func test_receive_turnOnTwoFactorTapped() async throws {
         let url = URL("https://www.example.com")!
         environmentService.setUpTwoFactorURL = url
-        subject.receive(.turnOnTwoFactorTapped)
+        await subject.perform(.turnOnTwoFactorTapped)
+        waitFor(!coordinator.alertShown.isEmpty)
         let alert = try XCTUnwrap(coordinator.alertShown.last)
         try await alert.tapAction(title: Localizations.continue)
         XCTAssertEqual(subject.state.url, url)
+        XCTAssertNil(stateService.lastSyncTimeByUserId["1"])
     }
 }

BitwardenShared/UI/Auth/TwoFactorNotice/SetUpTwoFactor/SetUpTwoFactorView.swift

@@ -27,8 +27,8 @@ struct SetUpTwoFactorView: View {
             .padding(.top, 16)
 
             VStack(spacing: 12) {
-                Button {
-                    store.send(.turnOnTwoFactorTapped)
+                AsyncButton {
+                    await store.perform(.turnOnTwoFactorTapped)
                 } label: {
                     Label {
                         Text(Localizations.turnOnTwoStepLogin)
@@ -38,8 +38,8 @@ struct SetUpTwoFactorView: View {
                 }
                 .buttonStyle(.primary())
 
-                Button {
-                    store.send(.changeAccountEmailTapped)
+                AsyncButton {
+                    await store.perform(.changeAccountEmailTapped)
                 } label: {
                     Label {
                         Text(Localizations.changeAccountEmail)

BitwardenShared/UI/Auth/TwoFactorNotice/SetUpTwoFactor/SetUpTwoFactorViewTests.swift

@@ -33,11 +33,11 @@ class SetUpTwoFactorViewTests: BitwardenTestCase {
 
     /// Tapping the change email button sends `.changeAccountEmailTapped`
     @MainActor
-    func test_changeEmail_tap() throws {
-        let button = try subject.inspect().find(button: Localizations.changeAccountEmail)
-        try button.tap()
+    func test_changeEmail_tap() async throws {
+        let button = try subject.inspect().find(asyncButton: Localizations.changeAccountEmail)
+        try await button.tap()
 
-        XCTAssertEqual(processor.dispatchedActions.last, .changeAccountEmailTapped)
+        XCTAssertEqual(processor.effects.last, .changeAccountEmailTapped)
     }
 
     /// Tapping the remind me later button sends `.remindMeLater`
@@ -51,11 +51,11 @@ class SetUpTwoFactorViewTests: BitwardenTestCase {
 
     /// Tapping the turn on two-step login button sends `.turnOnTwoFactorTapped`
     @MainActor
-    func test_turnOnTwoStep_tap() throws {
-        let button = try subject.inspect().find(button: Localizations.turnOnTwoStepLogin)
-        try button.tap()
+    func test_turnOnTwoStep_tap() async throws {
+        let button = try subject.inspect().find(asyncButton: Localizations.turnOnTwoStepLogin)
+        try await button.tap()
 
-        XCTAssertEqual(processor.dispatchedActions.last, .turnOnTwoFactorTapped)
+        XCTAssertEqual(processor.effects.last, .turnOnTwoFactorTapped)
     }
 
     // MARK: Previews
@@ -70,7 +70,7 @@ class SetUpTwoFactorViewTests: BitwardenTestCase {
         )
     }
 
-    /// The set up two factor view renders correctly when delay is allowed
+    /// The set up two factor view renders correctly when delay is not allowed
     @MainActor
     func test_snapshot_setUpTwoFactorView_allowDelay_false() {
         processor.state.allowDelay = false

BitwardenShared/UI/Auth/TwoFactorNotice/TwoFactorNoticeHelper.swift

@@ -42,6 +42,7 @@ class DefaultTwoFactorNoticeHelper: TwoFactorNoticeHelper {
         & HasErrorReporter
         & HasPolicyService
         & HasStateService
+        & HasSyncService
         & HasTimeProvider
 
     // MARK: Private Properties
@@ -74,20 +75,23 @@ class DefaultTwoFactorNoticeHelper: TwoFactorNoticeHelper {
     /// and displays the notice if necessary
     ///
     func maybeShowTwoFactorNotice() async {
-        let temporary = await services.configService.getFeatureFlag(
-            .newDeviceVerificationTemporaryDismiss,
-            defaultValue: false
-        )
-        let permanent = await services.configService.getFeatureFlag(
-            .newDeviceVerificationPermanentDismiss,
-            defaultValue: false
-        )
-        let ignoreEnvironmentCheck = await services.configService.getFeatureFlag(
-            .ignore2FANoticeEnvironmentCheck,
-            defaultValue: false
-        )
-        guard temporary || permanent else { return }
         do {
+            try await services.syncService.fetchSync(forceSync: false)
+
+            let temporary = await services.configService.getFeatureFlag(
+                .newDeviceVerificationTemporaryDismiss,
+                defaultValue: false
+            )
+            let permanent = await services.configService.getFeatureFlag(
+                .newDeviceVerificationPermanentDismiss,
+                defaultValue: false
+            )
+            let ignoreEnvironmentCheck = await services.configService.getFeatureFlag(
+                .ignore2FANoticeEnvironmentCheck,
+                defaultValue: false
+            )
+            guard temporary || permanent else { return }
+
             guard services.environmentService.region != .selfHosted || ignoreEnvironmentCheck else {
                 return
             }

BitwardenShared/UI/Auth/TwoFactorNotice/TwoFactorNoticeHelperTests.swift

@@ -14,6 +14,7 @@ class TwoFactorNoticeHelperTests: BitwardenTestCase {
     var policyService: MockPolicyService!
     var stateService: MockStateService!
     var subject: DefaultTwoFactorNoticeHelper!
+    var syncService: MockSyncService!
     var timeProvider: MockTimeProvider!
 
     // MARK: Setup & Teardown
@@ -27,6 +28,7 @@ class TwoFactorNoticeHelperTests: BitwardenTestCase {
         errorReporter = MockErrorReporter()
         policyService = MockPolicyService()
         stateService = MockStateService()
+        syncService = MockSyncService()
         timeProvider = MockTimeProvider(.mockTime(Date(year: 2024, month: 6, day: 15, hour: 12, minute: 0)))
 
         let services = ServiceContainer.withMocks(
@@ -35,6 +37,7 @@ class TwoFactorNoticeHelperTests: BitwardenTestCase {
             errorReporter: errorReporter,
             policyService: policyService,
             stateService: stateService,
+            syncService: syncService,
             timeProvider: timeProvider
         )
 
@@ -70,6 +73,7 @@ class TwoFactorNoticeHelperTests: BitwardenTestCase {
         policyService = nil
         stateService = nil
         subject = nil
+        syncService = nil
         timeProvider = nil
     }
 
@@ -86,6 +90,7 @@ class TwoFactorNoticeHelperTests: BitwardenTestCase {
     func test_maybeShow() async {
         await subject.maybeShowTwoFactorNotice()
 
+        XCTAssertTrue(syncService.didFetchSync)
         XCTAssertEqual(coordinator.routes, [.twoFactorNotice(allowDelay: false, emailAddress: "user@bitwarden.com")])
     }
 
@@ -98,6 +103,7 @@ class TwoFactorNoticeHelperTests: BitwardenTestCase {
 
         await subject.maybeShowTwoFactorNotice()
 
+        XCTAssertTrue(syncService.didFetchSync)
         XCTAssertEqual(coordinator.routes, [.twoFactorNotice(allowDelay: false, emailAddress: "user@bitwarden.com")])
     }
 
@@ -109,6 +115,7 @@ class TwoFactorNoticeHelperTests: BitwardenTestCase {
 
         await subject.maybeShowTwoFactorNotice()
 
+        XCTAssertTrue(syncService.didFetchSync)
         XCTAssertEqual(coordinator.routes, [])
     }
 
@@ -120,6 +127,7 @@ class TwoFactorNoticeHelperTests: BitwardenTestCase {
 
         await subject.maybeShowTwoFactorNotice()
 
+        XCTAssertTrue(syncService.didFetchSync)
         XCTAssertEqual(coordinator.routes, [])
     }
 
@@ -131,6 +139,7 @@ class TwoFactorNoticeHelperTests: BitwardenTestCase {
 
         await subject.maybeShowTwoFactorNotice()
 
+        XCTAssertTrue(syncService.didFetchSync)
         XCTAssertEqual(coordinator.routes, [])
     }
 
@@ -143,6 +152,7 @@ class TwoFactorNoticeHelperTests: BitwardenTestCase {
 
         await subject.maybeShowTwoFactorNotice()
 
+        XCTAssertTrue(syncService.didFetchSync)
         XCTAssertEqual(coordinator.routes, [.twoFactorNotice(allowDelay: false, emailAddress: "user@bitwarden.com")])
     }
 

BitwardenShared/UI/Vault/Vault/VaultCoordinator.swift

@@ -54,7 +54,7 @@ public protocol VaultCoordinatorDelegate: AnyObject {
 
 /// A coordinator that manages navigation in the vault tab.
 ///
-final class VaultCoordinator: Coordinator, HasStackNavigator {
+final class VaultCoordinator: Coordinator, HasStackNavigator { // swiftlint:disable: this type_body_length
     // MARK: Types
 
     typealias Module = GeneratorModule
@@ -79,6 +79,7 @@ final class VaultCoordinator: Coordinator, HasStackNavigator {
         & HasReviewPromptService
         & HasSettingsRepository
         & HasStateService
+        & HasSyncService
         & HasTOTPExpirationManagerFactory
         & HasTextAutofillHelperFactory
         & HasTimeProvider

BitwardenShared/UI/Vault/Vault/VaultCoordinatorTests.swift

@@ -313,7 +313,10 @@ class VaultCoordinatorTests: BitwardenTestCase {
         let action = try XCTUnwrap(stackNavigator.actions.last)
         XCTAssertEqual(action.type, .presented)
         XCTAssertTrue(module.twoFactorNoticeCoordinator.isStarted)
-        XCTAssertEqual(module.twoFactorNoticeCoordinator.routes.last, .emailAccess(allowDelay: true, emailAddress: "person@example.com"))
+        XCTAssertEqual(
+            module.twoFactorNoticeCoordinator.routes.last,
+            .emailAccess(allowDelay: true, emailAddress: "person@example.com")
+        )
     }
 
     /// `.navigate(to:)` with `.vaultItemSelection` presents the vault item selection screen.