Skip to content

Commit f705a2a

Browse files
fedemkrfedemkr
authored
[PM-24290] Decrease intensive memory usage on iOS extensions by not syncing ASStore (#2046)
1 parent 567455e commit f705a2a

12 files changed

+279
-8
lines changed

‎BitwardenShared/Core/Autofill/Services/AutofillCredentialService+AppExtensionTests.swift‎

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
// swiftlint:disable:this file_name
2+
3+
import AuthenticationServices
4+
import BitwardenKit
5+
import BitwardenKitMocks
6+
import BitwardenSdk
7+
import TestHelpers
8+
import XCTest
9+
10+
@testable import BitwardenShared
11+
12+
/// The tests for `DefaultAutofillCredentialService` when the app context is `.appExtension`.
13+
/// This new file is needed given that the app context is necesary on `DefaultAutofillCredentialService`
14+
/// initialization to see if the subscription to the `VaultTimeoutService` is necessary. So it's easier to test
15+
/// having a new class test specifically for it.
16+
@MainActor
17+
class AutofillCredentialServiceAppExtensionTests: BitwardenTestCase {
18+
// MARK: Properties
19+
20+
var appContextHelper: MockAppContextHelper!
21+
var autofillCredentialServiceDelegate: MockAutofillCredentialServiceDelegate!
22+
var cipherService: MockCipherService!
23+
var clientService: MockClientService!
24+
var credentialIdentityFactory: MockCredentialIdentityFactory!
25+
var errorReporter: MockErrorReporter!
26+
var eventService: MockEventService!
27+
var fido2UserInterfaceHelperDelegate: MockFido2UserInterfaceHelperDelegate!
28+
var fido2CredentialStore: MockFido2CredentialStore!
29+
var fido2UserInterfaceHelper: MockFido2UserInterfaceHelper!
30+
var identityStore: MockCredentialIdentityStore!
31+
var pasteboardService: MockPasteboardService!
32+
var stateService: MockStateService!
33+
var timeProvider: MockTimeProvider!
34+
var totpService: MockTOTPService!
35+
var subject: DefaultAutofillCredentialService!
36+
var vaultTimeoutService: MockVaultTimeoutService!
37+
38+
// MARK: Setup & Teardown
39+
40+
override func setUp() {
41+
super.setUp()
42+
43+
appContextHelper = MockAppContextHelper()
44+
appContextHelper.appContext = .appExtension
45+
46+
autofillCredentialServiceDelegate = MockAutofillCredentialServiceDelegate()
47+
cipherService = MockCipherService()
48+
clientService = MockClientService()
49+
credentialIdentityFactory = MockCredentialIdentityFactory()
50+
errorReporter = MockErrorReporter()
51+
eventService = MockEventService()
52+
fido2UserInterfaceHelperDelegate = MockFido2UserInterfaceHelperDelegate()
53+
fido2CredentialStore = MockFido2CredentialStore()
54+
fido2UserInterfaceHelper = MockFido2UserInterfaceHelper()
55+
identityStore = MockCredentialIdentityStore()
56+
pasteboardService = MockPasteboardService()
57+
stateService = MockStateService()
58+
timeProvider = MockTimeProvider(.currentTime)
59+
totpService = MockTOTPService()
60+
vaultTimeoutService = MockVaultTimeoutService()
61+
62+
subject = DefaultAutofillCredentialService(
63+
appContextHelper: appContextHelper,
64+
cipherService: cipherService,
65+
clientService: clientService,
66+
credentialIdentityFactory: credentialIdentityFactory,
67+
errorReporter: errorReporter,
68+
eventService: eventService,
69+
fido2CredentialStore: fido2CredentialStore,
70+
fido2UserInterfaceHelper: fido2UserInterfaceHelper,
71+
identityStore: identityStore,
72+
pasteboardService: pasteboardService,
73+
stateService: stateService,
74+
timeProvider: timeProvider,
75+
totpService: totpService,
76+
vaultTimeoutService: vaultTimeoutService,
77+
)
78+
}
79+
80+
override func tearDown() async throws {
81+
try await super.tearDown()
82+
83+
appContextHelper = nil
84+
autofillCredentialServiceDelegate = nil
85+
cipherService = nil
86+
clientService = nil
87+
credentialIdentityFactory = nil
88+
errorReporter = nil
89+
eventService = nil
90+
fido2UserInterfaceHelperDelegate = nil
91+
fido2CredentialStore = nil
92+
fido2UserInterfaceHelper = nil
93+
identityStore = nil
94+
pasteboardService = nil
95+
stateService = nil
96+
timeProvider = nil
97+
totpService = nil
98+
subject = nil
99+
vaultTimeoutService = nil
100+
}
101+
102+
// MARK: Tests
103+
104+
/// `syncIdentities(vaultLockStatus:)` doesn't update the credential identity store with the identities
105+
/// from the user's vault when the app context is `.appExtension`.
106+
func test_syncIdentities_appExtensionContext() { // swiftlint:disable:this function_body_length
107+
prepareDataForIdentitiesReplacement()
108+
109+
vaultTimeoutService.vaultLockStatusSubject.send(VaultLockStatus(isVaultLocked: false, userId: "1"))
110+
111+
XCTAssertFalse(cipherService.fetchAllCiphersCalled)
112+
XCTAssertFalse(credentialIdentityFactory.createCredentialIdentitiesMocker.called)
113+
XCTAssertFalse(identityStore.replaceCredentialIdentitiesCalled)
114+
XCTAssertNil(identityStore.replaceCredentialIdentitiesIdentities)
115+
}
116+
117+
/// `updateCredentialsInStore()` replaces all identities in the identity Store correctly
118+
/// for the active user ID.
119+
func test_updateCredentialsInStore_succeedsActiveUserId() async throws {
120+
prepareDataForIdentitiesReplacement()
121+
stateService.activeAccount = .fixture(profile: .fixture(userId: "50"))
122+
123+
await subject.updateCredentialsInStore()
124+
try await waitForAsync { [weak self] in
125+
guard let self else { return false }
126+
return identityStore.replaceCredentialIdentitiesIdentities != nil
127+
}
128+
129+
XCTAssertEqual(
130+
identityStore.replaceCredentialIdentitiesIdentities,
131+
[
132+
.password(PasswordCredentialIdentity(id: "1", uri: "bitwarden.com", username: "[email protected]")),
133+
.password(PasswordCredentialIdentity(id: "3", uri: "example.com", username: "[email protected]")),
134+
],
135+
)
136+
XCTAssertEqual(subject.lastSyncedUserId, "50")
137+
}
138+
139+
/// `updateCredentialsInStore()` logs error when it throws.
140+
func test_updateCredentialsInStore_logOnThrow() async throws {
141+
stateService.activeAccount = nil
142+
143+
await subject.updateCredentialsInStore()
144+
145+
XCTAssertNil(identityStore.replaceCredentialIdentitiesIdentities)
146+
XCTAssertEqual(subject.lastSyncedUserId, nil)
147+
XCTAssertEqual(errorReporter.errors as? [StateServiceError], [.noActiveAccount])
148+
}
149+
150+
// MARK: Private methods
151+
152+
/// Prepares fixture data for identities replacement tests.
153+
func prepareDataForIdentitiesReplacement() {
154+
cipherService.fetchAllCiphersResult = .success([
155+
.fixture(
156+
id: "1",
157+
login: .fixture(
158+
password: "password123",
159+
uris: [.fixture(uri: "bitwarden.com")],
160+
username: "[email protected]",
161+
),
162+
),
163+
.fixture(id: "2", type: .identity),
164+
.fixture(
165+
id: "3",
166+
login: .fixture(
167+
password: "123321",
168+
uris: [.fixture(uri: "example.com")],
169+
username: "[email protected]",
170+
),
171+
),
172+
.fixture(deletedDate: .now, id: "4", type: .login),
173+
])
174+
175+
credentialIdentityFactory.createCredentialIdentitiesMocker
176+
.withResult { cipher in
177+
if cipher.id == "1" {
178+
[
179+
.password(
180+
PasswordCredentialIdentity(
181+
id: "1",
182+
uri: "bitwarden.com",
183+
username: "[email protected]",
184+
),
185+
),
186+
]
187+
} else if cipher.id == "3" {
188+
[
189+
.password(
190+
PasswordCredentialIdentity(
191+
id: "3",
192+
uri: "example.com",
193+
username: "[email protected]",
194+
),
195+
),
196+
]
197+
} else {
198+
[]
199+
}
200+
}
201+
}
202+
}

‎BitwardenShared/Core/Autofill/Services/AutofillCredentialService.swift‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,21 @@ protocol AutofillCredentialService: AnyObject {
7676
autofillCredentialServiceDelegate: AutofillCredentialServiceDelegate,
7777
repromptPasswordValidated: Bool,
7878
) async throws -> ASOneTimeCodeCredential
79+
80+
/// Updates all credential identities in the identity store with the current list of ciphers
81+
/// for the current user.
82+
///
83+
func updateCredentialsInStore() async
7984
}
8085

8186
/// A default implementation of an `AutofillCredentialService`.
8287
///
8388
class DefaultAutofillCredentialService {
8489
// MARK: Private Properties
8590

91+
/// Helper to know about the app context.
92+
private let appContextHelper: AppContextHelper
93+
8694
/// The service used to manage syncing and updates to the user's ciphers.
8795
private let cipherService: CipherService
8896

@@ -109,7 +117,7 @@ class DefaultAutofillCredentialService {
109117
private let identityStore: CredentialIdentityStore
110118

111119
/// The last user ID that had their identities synced.
112-
private var lastSyncedUserId: String?
120+
private(set) var lastSyncedUserId: String?
113121

114122
/// The service used to manage copy/pasting from the device's clipboard.
115123
private let pasteboardService: PasteboardService
@@ -135,6 +143,7 @@ class DefaultAutofillCredentialService {
135143
/// Initialize an `AutofillCredentialService`.
136144
///
137145
/// - Parameters:
146+
/// - appContextHelper: The helper to know about the app context.
138147
/// - cipherService: The service used to manage syncing and updates to the user's ciphers.
139148
/// - clientService: The service that handles common client functionality such as encryption and decryption.
140149
/// - credentialIdentityFactory: The factory to create credential identities.
@@ -151,6 +160,7 @@ class DefaultAutofillCredentialService {
151160
/// - vaultTimeoutService: The service used to manage vault access.
152161
///
153162
init(
163+
appContextHelper: AppContextHelper,
154164
cipherService: CipherService,
155165
clientService: ClientService,
156166
credentialIdentityFactory: CredentialIdentityFactory,
@@ -165,6 +175,7 @@ class DefaultAutofillCredentialService {
165175
totpService: TOTPService,
166176
vaultTimeoutService: VaultTimeoutService,
167177
) {
178+
self.appContextHelper = appContextHelper
168179
self.cipherService = cipherService
169180
self.clientService = clientService
170181
self.credentialIdentityFactory = credentialIdentityFactory
@@ -179,6 +190,10 @@ class DefaultAutofillCredentialService {
179190
self.totpService = totpService
180191
self.vaultTimeoutService = vaultTimeoutService
181192

193+
guard appContextHelper.appContext == .mainApp else {
194+
return
195+
}
196+
182197
Task {
183198
for await vaultLockStatus in await self.vaultTimeoutService.vaultLockStatusPublisher().values {
184199
syncIdentities(vaultLockStatus: vaultLockStatus)
@@ -419,6 +434,15 @@ extension DefaultAutofillCredentialService: AutofillCredentialService {
419434
return ASOneTimeCodeCredential(code: code.code)
420435
}
421436

437+
func updateCredentialsInStore() async {
438+
do {
439+
let userId = try await stateService.getActiveAccountId()
440+
await replaceAllIdentities(userId: userId)
441+
} catch {
442+
errorReporter.log(error: error)
443+
}
444+
}
445+
422446
// MARK: Private
423447

424448
/// Checks if the vault is locked, unlocking if never session timeout and gets the decrypted cipher

‎BitwardenShared/Core/Autofill/Services/AutofillCredentialServiceTests.swift‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import XCTest
1111
class AutofillCredentialServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body_length
1212
// MARK: Properties
1313

14+
var appContextHelper: MockAppContextHelper!
1415
var autofillCredentialServiceDelegate: MockAutofillCredentialServiceDelegate!
1516
var cipherService: MockCipherService!
1617
var clientService: MockClientService!
@@ -33,6 +34,7 @@ class AutofillCredentialServiceTests: BitwardenTestCase { // swiftlint:disable:t
3334
override func setUp() {
3435
super.setUp()
3536

37+
appContextHelper = MockAppContextHelper()
3638
autofillCredentialServiceDelegate = MockAutofillCredentialServiceDelegate()
3739
cipherService = MockCipherService()
3840
clientService = MockClientService()
@@ -50,6 +52,7 @@ class AutofillCredentialServiceTests: BitwardenTestCase { // swiftlint:disable:t
5052
vaultTimeoutService = MockVaultTimeoutService()
5153

5254
subject = DefaultAutofillCredentialService(
55+
appContextHelper: appContextHelper,
5356
cipherService: cipherService,
5457
clientService: clientService,
5558
credentialIdentityFactory: credentialIdentityFactory,
@@ -75,6 +78,7 @@ class AutofillCredentialServiceTests: BitwardenTestCase { // swiftlint:disable:t
7578
override func tearDown() async throws {
7679
try await super.tearDown()
7780

81+
appContextHelper = nil
7882
autofillCredentialServiceDelegate = nil
7983
cipherService = nil
8084
clientService = nil

‎BitwardenShared/Core/Autofill/Services/TestHelpers/MockAutofillCredentialService.swift‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ class MockAutofillCredentialService: AutofillCredentialService {
1212
var provideCredentialError: Error?
1313
var provideFido2CredentialResult: Result<PasskeyAssertionCredential, Error> = .failure(BitwardenTestError.example)
1414
var provideOTPCredentialResult: Result<OneTimeCodeCredential, Error> = .failure(BitwardenTestError.example)
15+
var updateCredentialsInStoreCalled = false
1516

1617
func isAutofillCredentialsEnabled() async -> Bool {
1718
isAutofillCredentialsEnabled
@@ -65,6 +66,10 @@ class MockAutofillCredentialService: AutofillCredentialService {
6566
}
6667
return credential
6768
}
69+
70+
func updateCredentialsInStore() async {
71+
updateCredentialsInStoreCalled = true
72+
}
6873
}
6974

7075
// MARK: - PasskeyAssertionCredential

‎BitwardenShared/Core/Platform/Services/ServiceContainer.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -807,6 +807,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le
807807

808808
let credentialIdentityFactory = DefaultCredentialIdentityFactory()
809809
let autofillCredentialService = DefaultAutofillCredentialService(
810+
appContextHelper: appContextHelper,
810811
cipherService: cipherService,
811812
clientService: clientService,
812813
credentialIdentityFactory: credentialIdentityFactory,

‎BitwardenShared/UI/Platform/ExtensionSetup/ExtensionActivation/ExtensionActivationEffect.swift‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// MARK: - ExtensionActivationEffect
2+
3+
/// Effects that can be performed by a `ExtensionActivationProcessor`.
4+
///
5+
enum ExtensionActivationEffect: Equatable {
6+
/// The view appeared on screen.
7+
case appeared
8+
}

‎BitwardenShared/UI/Platform/ExtensionSetup/ExtensionActivation/ExtensionActivationProcessor.swift‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@
55
class ExtensionActivationProcessor: StateProcessor<
66
ExtensionActivationState,
77
ExtensionActivationAction,
8-
Void,
8+
ExtensionActivationEffect,
99
> {
1010
// MARK: Types
1111

12-
typealias Services = HasConfigService
12+
typealias Services = HasAutofillCredentialService
13+
& HasConfigService
14+
& HasErrorReporter
1315

1416
// MARK: Private Properties
1517

@@ -40,6 +42,13 @@ class ExtensionActivationProcessor: StateProcessor<
4042

4143
// MARK: Methods
4244

45+
override func perform(_ effect: ExtensionActivationEffect) async {
46+
switch effect {
47+
case .appeared:
48+
await services.autofillCredentialService.updateCredentialsInStore()
49+
}
50+
}
51+
4352
override func receive(_ action: ExtensionActivationAction) {
4453
switch action {
4554
case .cancelTapped:

0 commit comments

Comments
 (0)