From 0f50d24179aac5bf9c3b5c99fb53bc5930e2a5b2 Mon Sep 17 00:00:00 2001 From: ezimet-livefront Date: Mon, 18 Nov 2024 22:47:32 -0500 Subject: [PATCH 1/6] PM-14836 added new action when app re-enter foreground and re-evaluate the vault state --- .github/workflows/crowdin-pull.yml | 9 ++++++++- .../Core/Platform/Services/ServiceContainer.swift | 1 - .../UI/Auth/VaultUnlock/VaultUnlockEffect.swift | 3 +++ .../UI/Auth/VaultUnlock/VaultUnlockProcessor.swift | 2 ++ .../Auth/VaultUnlock/VaultUnlockProcessorTests.swift | 10 ++++++++++ .../UI/Auth/VaultUnlock/VaultUnlockView.swift | 5 +++++ 6 files changed, 28 insertions(+), 2 deletions(-) diff --git a/.github/workflows/crowdin-pull.yml b/.github/workflows/crowdin-pull.yml index 7eff03bf5..76a725dac 100644 --- a/.github/workflows/crowdin-pull.yml +++ b/.github/workflows/crowdin-pull.yml @@ -28,10 +28,17 @@ jobs: keyvault: "bitwarden-ci" secrets: "crowdin-api-token, github-gpg-private-key, github-gpg-private-key-passphrase" + - name: Generate GH App token + uses: actions/create-github-app-token@5d869da34e18e7287c1daad50e0b8ea0f506ce69 # v1.11.0 + id: app-token + with: + app-id: ${{ secrets.BW_GHAPP_ID }} + private-key: ${{ secrets.BW_GHAPP_KEY }} + - name: Download translations uses: crowdin/github-action@2d540f18b0a416b1fbf2ee5be35841bd380fc1da # v2.3.0 env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_TOKEN: ${{ steps.app-token.outputs.token }} CROWDIN_API_TOKEN: ${{ steps.retrieve-secrets.outputs.crowdin-api-token }} with: config: crowdin.yml diff --git a/BitwardenShared/Core/Platform/Services/ServiceContainer.swift b/BitwardenShared/Core/Platform/Services/ServiceContainer.swift index bde329894..575d20c96 100644 --- a/BitwardenShared/Core/Platform/Services/ServiceContainer.swift +++ b/BitwardenShared/Core/Platform/Services/ServiceContainer.swift @@ -484,7 +484,6 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le let twoStepLoginService = DefaultTwoStepLoginService(environmentService: environmentService) - let pasteboardService = DefaultPasteboardService( errorReporter: errorReporter, stateService: stateService diff --git a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockEffect.swift b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockEffect.swift index 496e5d897..4127658d3 100644 --- a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockEffect.swift +++ b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockEffect.swift @@ -4,6 +4,9 @@ enum VaultUnlockEffect: Equatable { /// The vault unlock view appeared. case appeared + /// The app has re-entered the foreground and is now visible to the user. + case didEnterForeground + /// A Profile Switcher Effect. case profileSwitcher(ProfileSwitcherEffect) diff --git a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessor.swift b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessor.swift index 33837c0c4..01ade90a9 100644 --- a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessor.swift +++ b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessor.swift @@ -58,6 +58,8 @@ class VaultUnlockProcessor: StateProcessor< await refreshProfileState() await checkIfPinUnlockIsAvailable() await loadData() + case .didEnterForeground: + await coordinator.handleEvent(.didStart) case let .profileSwitcher(profileEffect): await handleProfileSwitcherEffect(profileEffect) case .unlockVault: diff --git a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessorTests.swift b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessorTests.swift index 04acdc06d..e72dc4124 100644 --- a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessorTests.swift +++ b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessorTests.swift @@ -156,6 +156,16 @@ class VaultUnlockProcessorTests: BitwardenTestCase { // swiftlint:disable:this t XCTAssertEqual(3, subject.state.unsuccessfulUnlockAttemptsCount) } + /// `perform(.didEnterForeground)` triggers the coordinator to handle the `.didStart` event. + @MainActor + func test_perform_didEnterForeground() async { + // Perform the didEnterForeground effect + await subject.perform(.didEnterForeground) + + // Verify that the coordinator handled the .didStart event + XCTAssertEqual(coordinator.events.last, .didStart) + } + /// `perform(.appeared)` /// No active account and accounts should yield a profile switcher state without an active account. @MainActor diff --git a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockView.swift b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockView.swift index 0c4d550d3..c9bef4bec 100644 --- a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockView.swift +++ b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockView.swift @@ -54,6 +54,11 @@ struct VaultUnlockView: View { .task { await store.perform(.appeared) } + .onReceive(NotificationCenter.default.publisher(for: UIApplication.willEnterForegroundNotification)) { _ in + Task { + await store.perform(.didEnterForeground) + } + } .toast(store.binding( get: \.toast, send: VaultUnlockAction.toastShown From fb12a5d55f01f0bcfae6e4caeed0ff32fd9b27c8 Mon Sep 17 00:00:00 2001 From: ezimet-livefront Date: Mon, 18 Nov 2024 22:53:15 -0500 Subject: [PATCH 2/6] revert workflow change --- .github/workflows/crowdin-pull.yml | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/.github/workflows/crowdin-pull.yml b/.github/workflows/crowdin-pull.yml index 76a725dac..7eff03bf5 100644 --- a/.github/workflows/crowdin-pull.yml +++ b/.github/workflows/crowdin-pull.yml @@ -28,17 +28,10 @@ jobs: keyvault: "bitwarden-ci" secrets: "crowdin-api-token, github-gpg-private-key, github-gpg-private-key-passphrase" - - name: Generate GH App token - uses: actions/create-github-app-token@5d869da34e18e7287c1daad50e0b8ea0f506ce69 # v1.11.0 - id: app-token - with: - app-id: ${{ secrets.BW_GHAPP_ID }} - private-key: ${{ secrets.BW_GHAPP_KEY }} - - name: Download translations uses: crowdin/github-action@2d540f18b0a416b1fbf2ee5be35841bd380fc1da # v2.3.0 env: - GITHUB_TOKEN: ${{ steps.app-token.outputs.token }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} CROWDIN_API_TOKEN: ${{ steps.retrieve-secrets.outputs.crowdin-api-token }} with: config: crowdin.yml From 69092d6ccefb67bc55e0e94610bdc27b598e4ebb Mon Sep 17 00:00:00 2001 From: ezimet-livefront Date: Tue, 19 Nov 2024 20:20:14 -0500 Subject: [PATCH 3/6] moved the didStart logic to app processor --- .../Auth/VaultUnlock/VaultUnlockEffect.swift | 3 --- .../VaultUnlock/VaultUnlockProcessor.swift | 2 -- .../VaultUnlockProcessorTests.swift | 10 ---------- .../UI/Auth/VaultUnlock/VaultUnlockView.swift | 5 ----- .../Platform/Application/AppProcessor.swift | 1 + .../Application/AppProcessorTests.swift | 19 +++++++++++++------ 6 files changed, 14 insertions(+), 26 deletions(-) diff --git a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockEffect.swift b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockEffect.swift index 4127658d3..496e5d897 100644 --- a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockEffect.swift +++ b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockEffect.swift @@ -4,9 +4,6 @@ enum VaultUnlockEffect: Equatable { /// The vault unlock view appeared. case appeared - /// The app has re-entered the foreground and is now visible to the user. - case didEnterForeground - /// A Profile Switcher Effect. case profileSwitcher(ProfileSwitcherEffect) diff --git a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessor.swift b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessor.swift index 01ade90a9..33837c0c4 100644 --- a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessor.swift +++ b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessor.swift @@ -58,8 +58,6 @@ class VaultUnlockProcessor: StateProcessor< await refreshProfileState() await checkIfPinUnlockIsAvailable() await loadData() - case .didEnterForeground: - await coordinator.handleEvent(.didStart) case let .profileSwitcher(profileEffect): await handleProfileSwitcherEffect(profileEffect) case .unlockVault: diff --git a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessorTests.swift b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessorTests.swift index e72dc4124..04acdc06d 100644 --- a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessorTests.swift +++ b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessorTests.swift @@ -156,16 +156,6 @@ class VaultUnlockProcessorTests: BitwardenTestCase { // swiftlint:disable:this t XCTAssertEqual(3, subject.state.unsuccessfulUnlockAttemptsCount) } - /// `perform(.didEnterForeground)` triggers the coordinator to handle the `.didStart` event. - @MainActor - func test_perform_didEnterForeground() async { - // Perform the didEnterForeground effect - await subject.perform(.didEnterForeground) - - // Verify that the coordinator handled the .didStart event - XCTAssertEqual(coordinator.events.last, .didStart) - } - /// `perform(.appeared)` /// No active account and accounts should yield a profile switcher state without an active account. @MainActor diff --git a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockView.swift b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockView.swift index c9bef4bec..0c4d550d3 100644 --- a/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockView.swift +++ b/BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockView.swift @@ -54,11 +54,6 @@ struct VaultUnlockView: View { .task { await store.perform(.appeared) } - .onReceive(NotificationCenter.default.publisher(for: UIApplication.willEnterForegroundNotification)) { _ in - Task { - await store.perform(.didEnterForeground) - } - } .toast(store.binding( get: \.toast, send: VaultUnlockAction.toastShown diff --git a/BitwardenShared/UI/Platform/Application/AppProcessor.swift b/BitwardenShared/UI/Platform/Application/AppProcessor.swift index 83f0e7d58..a505bfbf1 100644 --- a/BitwardenShared/UI/Platform/Application/AppProcessor.swift +++ b/BitwardenShared/UI/Platform/Application/AppProcessor.swift @@ -84,6 +84,7 @@ public class AppProcessor { startEventTimer() await checkIfExtensionSwitchedAccounts() await checkAccountsForTimeout() + await coordinator?.handleEvent(.didStart) await completeAutofillAccountSetupIfEnabled() #if DEBUG debugWillEnterForeground?() diff --git a/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift b/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift index 86ef87a8e..e1ce8dc5c 100644 --- a/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift +++ b/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift @@ -260,7 +260,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body notificationCenterService.willEnterForegroundSubject.send() try await waitForAsync { self.willEnterForegroundCalled == 2 } - XCTAssertTrue(coordinator.events.isEmpty) + XCTAssertEqual(coordinator.events.last, .didStart) XCTAssertEqual(errorReporter.errors as? [BitwardenTestError], [.example]) } @@ -277,7 +277,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body notificationCenterService.willEnterForegroundSubject.send() try await waitForAsync { self.willEnterForegroundCalled == 2 } - XCTAssertTrue(coordinator.events.isEmpty) + XCTAssertEqual(coordinator.events.last, .didStart) } /// `init()` subscribes to will enter foreground events and handles switching accounts if the @@ -294,7 +294,14 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body notificationCenterService.willEnterForegroundSubject.send() try await waitForAsync { self.willEnterForegroundCalled == 2 } - XCTAssertEqual(coordinator.events, [.switchAccounts(userId: "2", isAutomatic: false)]) + XCTAssertEqual( + coordinator.events, + [ + .didStart, + .switchAccounts(userId: "2", isAutomatic: false), + .didStart, + ] + ) } /// `init()` subscribes to will enter foreground events and doesn't check for an account switch @@ -323,7 +330,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body notificationCenterService.willEnterForegroundSubject.send() try await waitForAsync { willEnterForegroundCalled == 2 } - XCTAssertTrue(coordinator.events.isEmpty) + XCTAssertEqual(coordinator.events.last, .didStart) } /// `init()` subscribes to will enter foreground events and restarts the app is there's no @@ -339,7 +346,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body notificationCenterService.willEnterForegroundSubject.send() try await waitForAsync { self.willEnterForegroundCalled == 2 } - XCTAssertEqual(coordinator.events, [.didStart]) + XCTAssertEqual(coordinator.events, [.didStart, .didStart, .didStart]) } /// `init()` sets the `AppProcessor` as the delegate of any necessary services. @@ -915,7 +922,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body waitFor(!coordinator.events.isEmpty) XCTAssertTrue(appModule.appCoordinator.isStarted) - XCTAssertEqual(appModule.appCoordinator.events, [.didStart]) + XCTAssertEqual(appModule.appCoordinator.events, [.didStart, .didStart]) XCTAssertEqual(migrationService.didPerformMigrations, true) } From eb634751b3b72332a68798cf24c0f44582d9f7a2 Mon Sep 17 00:00:00 2001 From: ezimet-livefront Date: Thu, 21 Nov 2024 16:02:42 -0500 Subject: [PATCH 4/6] address feedback --- .../Platform/Application/AppCoordinator.swift | 13 +++++ .../Platform/Application/AppProcessor.swift | 29 +++++++++++- .../Application/AppProcessorTests.swift | 47 ++++++++++++++----- .../UI/Platform/Application/AppRoute.swift | 13 +++++ 4 files changed, 88 insertions(+), 14 deletions(-) diff --git a/BitwardenShared/UI/Platform/Application/AppCoordinator.swift b/BitwardenShared/UI/Platform/Application/AppCoordinator.swift index 6ac66c5d6..7cfab0627 100644 --- a/BitwardenShared/UI/Platform/Application/AppCoordinator.swift +++ b/BitwardenShared/UI/Platform/Application/AppCoordinator.swift @@ -73,6 +73,19 @@ class AppCoordinator: Coordinator, HasRootNavigator { func handleEvent(_ event: AppEvent, context: AnyObject?) async { switch event { + case let .accountBecameActive( + account, + attemptAutomaticBiometricUnlock, + didSwitchAccountAutomatically + ): + await handleAuthEvent( + .accountBecameActive( + account, + animated: true, + attemptAutomaticBiometricUnlock: attemptAutomaticBiometricUnlock, + didSwitchAccountAutomatically: didSwitchAccountAutomatically + ) + ) case let .didLogout(userId, userInitiated): await handleAuthEvent(.didLogout(userId: userId, userInitiated: userInitiated)) case .didStart: diff --git a/BitwardenShared/UI/Platform/Application/AppProcessor.swift b/BitwardenShared/UI/Platform/Application/AppProcessor.swift index a505bfbf1..ad10e4ef0 100644 --- a/BitwardenShared/UI/Platform/Application/AppProcessor.swift +++ b/BitwardenShared/UI/Platform/Application/AppProcessor.swift @@ -84,7 +84,7 @@ public class AppProcessor { startEventTimer() await checkIfExtensionSwitchedAccounts() await checkAccountsForTimeout() - await coordinator?.handleEvent(.didStart) + await handleNeverTimeOutAccountBecameActive() await completeAutofillAccountSetupIfEnabled() #if DEBUG debugWillEnterForeground?() @@ -355,6 +355,33 @@ extension AppProcessor { } } + /// Handles the case where an session never timeout account becomes active. + /// + private func handleNeverTimeOutAccountBecameActive() async { + // we should be safe to unlock the vault: + // - If not in the extension + // - If vault is locked + // - If the user uses never lock + // - If the user hasn't manually locked their vault (because it was unlocked in the extension). + guard appExtensionDelegate?.isInAppExtension != true else { return } + guard let account = try? await services.stateService.getActiveAccount() else { return } + guard services.vaultTimeoutService.isLocked(userId: account.profile.userId) else { return } + guard let sessionTimeout = try? await services.vaultTimeoutService.sessionTimeoutValue( + userId: account.profile.userId + ), sessionTimeout == .never else { return } + guard let manuallyLocked = try? await services.stateService.getManuallyLockedAccount( + userId: account.profile.userId + ), !manuallyLocked else { return } + + await coordinator?.handleEvent( + .accountBecameActive( + account, + attemptAutomaticBiometricUnlock: true, + didSwitchAccountAutomatically: false + ) + ) + } + /// Checks if the active account was switched while in the extension. If this occurs, the app /// needs to also switch to the updated active account. /// diff --git a/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift b/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift index e1ce8dc5c..cf6eaa999 100644 --- a/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift +++ b/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift @@ -247,6 +247,34 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body XCTAssertEqual(stateService.accountSetupAutofill, ["1": .complete]) } + /// `init()` subscribes to will enter foreground events and handles accountBecameActive if the + /// never timeout account is unlocked in extension. + @MainActor + func test_init_appForeground_checkAccountBecomeActive() async throws { + // The processor checks for switched accounts when entering the foreground. Wait for the + // initial check to finish when the test starts before continuing. + try await waitForAsync { self.willEnterForegroundCalled == 1 } + let account: Account = .fixture(profile: .fixture(userId: "2")) + let userId = account.profile.userId + stateService.activeAccount = account + stateService.didAccountSwitchInExtensionResult = .success(true) + vaultTimeoutService.isClientLocked = [userId: true] + vaultTimeoutService.vaultTimeout = [userId: .never] + stateService.manuallyLockedAccounts = [userId: false] + + notificationCenterService.willEnterForegroundSubject.send() + try await waitForAsync { self.willEnterForegroundCalled == 2 } + + XCTAssertEqual( + coordinator.events.last, + AppEvent.accountBecameActive( + account, + attemptAutomaticBiometricUnlock: true, + didSwitchAccountAutomatically: false + ) + ) + } + /// `init()` subscribes to will enter foreground events and logs an error if one occurs while /// checking if the active account was changed in an extension. @MainActor @@ -260,7 +288,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body notificationCenterService.willEnterForegroundSubject.send() try await waitForAsync { self.willEnterForegroundCalled == 2 } - XCTAssertEqual(coordinator.events.last, .didStart) + XCTAssertTrue(coordinator.events.isEmpty) XCTAssertEqual(errorReporter.errors as? [BitwardenTestError], [.example]) } @@ -277,7 +305,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body notificationCenterService.willEnterForegroundSubject.send() try await waitForAsync { self.willEnterForegroundCalled == 2 } - XCTAssertEqual(coordinator.events.last, .didStart) + XCTAssertTrue(coordinator.events.isEmpty) } /// `init()` subscribes to will enter foreground events and handles switching accounts if the @@ -294,14 +322,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body notificationCenterService.willEnterForegroundSubject.send() try await waitForAsync { self.willEnterForegroundCalled == 2 } - XCTAssertEqual( - coordinator.events, - [ - .didStart, - .switchAccounts(userId: "2", isAutomatic: false), - .didStart, - ] - ) + XCTAssertEqual(coordinator.events, [.switchAccounts(userId: "2", isAutomatic: false)]) } /// `init()` subscribes to will enter foreground events and doesn't check for an account switch @@ -330,7 +351,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body notificationCenterService.willEnterForegroundSubject.send() try await waitForAsync { willEnterForegroundCalled == 2 } - XCTAssertEqual(coordinator.events.last, .didStart) + XCTAssertTrue(coordinator.events.isEmpty) } /// `init()` subscribes to will enter foreground events and restarts the app is there's no @@ -346,7 +367,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body notificationCenterService.willEnterForegroundSubject.send() try await waitForAsync { self.willEnterForegroundCalled == 2 } - XCTAssertEqual(coordinator.events, [.didStart, .didStart, .didStart]) + XCTAssertEqual(coordinator.events, [.didStart]) } /// `init()` sets the `AppProcessor` as the delegate of any necessary services. @@ -922,7 +943,7 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body waitFor(!coordinator.events.isEmpty) XCTAssertTrue(appModule.appCoordinator.isStarted) - XCTAssertEqual(appModule.appCoordinator.events, [.didStart, .didStart]) + XCTAssertEqual(appModule.appCoordinator.events, [.didStart]) XCTAssertEqual(migrationService.didPerformMigrations, true) } diff --git a/BitwardenShared/UI/Platform/Application/AppRoute.swift b/BitwardenShared/UI/Platform/Application/AppRoute.swift index 1c9acdf43..1ab1042b2 100644 --- a/BitwardenShared/UI/Platform/Application/AppRoute.swift +++ b/BitwardenShared/UI/Platform/Application/AppRoute.swift @@ -26,6 +26,19 @@ public enum AppRoute: Equatable { } public enum AppEvent: Equatable { + /// When the router should check the lock status of an account and propose a route. + /// + /// - Parameters: + /// - account: The account to unlock the vault for. + /// - attemptAutomaticBiometricUnlock: If `true` and biometric unlock is enabled/available, + /// the processor should attempt an automatic biometric unlock. + /// - didSwitchAccountAutomatically: A flag indicating if the active account was switched automatically. + /// + case accountBecameActive( + Account, + attemptAutomaticBiometricUnlock: Bool, + didSwitchAccountAutomatically: Bool + ) /// When the user logs out from an account. /// /// - Parameters: From 552ac902c63d8b6cc1dc4565b5ce980dd3c36b7e Mon Sep 17 00:00:00 2001 From: ezimet-livefront Date: Fri, 22 Nov 2024 12:15:20 -0500 Subject: [PATCH 5/6] address feedback, added test for coordinator, and updated app processor to have better doc and improved code --- .../Application/AppCoordinatorTests.swift | 18 ++++++++++++++ .../Platform/Application/AppProcessor.swift | 24 +++++++------------ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/BitwardenShared/UI/Platform/Application/AppCoordinatorTests.swift b/BitwardenShared/UI/Platform/Application/AppCoordinatorTests.swift index 28de1df89..1b34e0323 100644 --- a/BitwardenShared/UI/Platform/Application/AppCoordinatorTests.swift +++ b/BitwardenShared/UI/Platform/Application/AppCoordinatorTests.swift @@ -263,6 +263,24 @@ class AppCoordinatorTests: BitwardenTestCase { // swiftlint:disable:this type_bo ) } + /// `handleEvent(_:)` with `.accountBecameActive` has the router handle account activation. + @MainActor + func test_handleEvent_accountBecameActive() async { + let account = Account.fixtureAccountLogin() + await subject.handleEvent( + .accountBecameActive(account, attemptAutomaticBiometricUnlock: true, didSwitchAccountAutomatically: true) + ) + XCTAssertEqual( + router.events, + [.accountBecameActive( + account, + animated: true, + attemptAutomaticBiometricUnlock: true, + didSwitchAccountAutomatically: true + )] + ) + } + /// `navigate(to:)` with `.onboarding` starts the auth coordinator and navigates to the proper auth route. @MainActor func test_navigateTo_auth() throws { diff --git a/BitwardenShared/UI/Platform/Application/AppProcessor.swift b/BitwardenShared/UI/Platform/Application/AppProcessor.swift index ad10e4ef0..26f5d4eee 100644 --- a/BitwardenShared/UI/Platform/Application/AppProcessor.swift +++ b/BitwardenShared/UI/Platform/Application/AppProcessor.swift @@ -355,23 +355,17 @@ extension AppProcessor { } } - /// Handles the case where an session never timeout account becomes active. + /// Handles unlocking the vault for a manually locked account that uses never lock + /// and was previously unlocked in an extension. /// private func handleNeverTimeOutAccountBecameActive() async { - // we should be safe to unlock the vault: - // - If not in the extension - // - If vault is locked - // - If the user uses never lock - // - If the user hasn't manually locked their vault (because it was unlocked in the extension). - guard appExtensionDelegate?.isInAppExtension != true else { return } - guard let account = try? await services.stateService.getActiveAccount() else { return } - guard services.vaultTimeoutService.isLocked(userId: account.profile.userId) else { return } - guard let sessionTimeout = try? await services.vaultTimeoutService.sessionTimeoutValue( - userId: account.profile.userId - ), sessionTimeout == .never else { return } - guard let manuallyLocked = try? await services.stateService.getManuallyLockedAccount( - userId: account.profile.userId - ), !manuallyLocked else { return } + guard + appExtensionDelegate?.isInAppExtension != true, + await (try? services.authRepository.isLocked()) == true, + await (try? services.authRepository.sessionTimeoutValue()) == .never, + await (try? services.stateService.getManuallyLockedAccount(userId: nil)) == false, + let account = try? await services.stateService.getActiveAccount() + else { return } await coordinator?.handleEvent( .accountBecameActive( From 90662010f4777f1b68fc09421a797c62a8a756c6 Mon Sep 17 00:00:00 2001 From: ezimet-livefront Date: Fri, 22 Nov 2024 15:08:32 -0500 Subject: [PATCH 6/6] fix unit tests --- .../UI/Platform/Application/AppProcessorTests.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift b/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift index cf6eaa999..75d755e78 100644 --- a/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift +++ b/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift @@ -257,9 +257,10 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body let account: Account = .fixture(profile: .fixture(userId: "2")) let userId = account.profile.userId stateService.activeAccount = account + authRepository.activeAccount = account stateService.didAccountSwitchInExtensionResult = .success(true) - vaultTimeoutService.isClientLocked = [userId: true] - vaultTimeoutService.vaultTimeout = [userId: .never] + authRepository.vaultTimeout = [userId: .never] + authRepository.isLockedResult = .success(true) stateService.manuallyLockedAccounts = [userId: false] notificationCenterService.willEnterForegroundSubject.send()