Skip to content

Commit b0a55c4

Browse files
PM-9155: Hide the vault filter when personal ownership and single organization policies apply (#777)
1 parent 7673d18 commit b0a55c4

16 files changed

+236
-27
lines changed

BitwardenShared/Core/Platform/Services/ServiceContainer.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le
523523
errorReporter: errorReporter,
524524
folderService: folderService,
525525
organizationService: organizationService,
526+
policyService: policyService,
526527
settingsService: settingsService,
527528
stateService: stateService,
528529
syncService: syncService,

BitwardenShared/Core/Vault/Repositories/TestHelpers/MockVaultRepository.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ class MockVaultRepository: VaultRepository {
1010
var addCipherCiphers = [CipherView]()
1111
var addCipherResult: Result<Void, Error> = .success(())
1212

13+
var canShowVaultFilter = true
14+
1315
var ciphersAutofillPublisherUriCalled: String?
1416
var ciphersSubject = CurrentValueSubject<[CipherListView], Error>([])
1517
var ciphersAutofillSubject = CurrentValueSubject<[BitwardenShared.VaultListSection], Error>([])
@@ -113,6 +115,10 @@ class MockVaultRepository: VaultRepository {
113115
try addCipherResult.get()
114116
}
115117

118+
func canShowVaultFilter() async -> Bool {
119+
canShowVaultFilter
120+
}
121+
116122
func cipherPublisher() async throws -> AsyncThrowingPublisher<AnyPublisher<[CipherListView], Error>> {
117123
ciphersSubject.eraseToAnyPublisher().values
118124
}

BitwardenShared/Core/Vault/Repositories/VaultRepository.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@ public protocol VaultRepository: AnyObject {
2727
///
2828
func addCipher(_ cipher: CipherView) async throws
2929

30+
/// Whether the vault filter can be shown to the user. It might not be shown to the user if the
31+
/// policies are set up to disable personal vault ownership and only allow the user to be in a
32+
/// single organization.
33+
///
34+
/// - Returns: `true` if the vault filter can be shown.
35+
///
36+
func canShowVaultFilter() async -> Bool
37+
3038
/// Removes any temporarily downloaded attachments.
3139
func clearTemporaryDownloads()
3240

@@ -322,6 +330,9 @@ class DefaultVaultRepository { // swiftlint:disable:this type_body_length
322330
/// The service used to manage syncing and updates to the user's organizations.
323331
private let organizationService: OrganizationService
324332

333+
/// The service for managing the polices for the user.
334+
private let policyService: PolicyService
335+
325336
/// The service used by the application to manage user settings.
326337
let settingsService: SettingsService
327338

@@ -350,6 +361,7 @@ class DefaultVaultRepository { // swiftlint:disable:this type_body_length
350361
/// - errorReporter: The service used by the application to report non-fatal errors.
351362
/// - folderService: The service used to manage syncing and updates to the user's folders.
352363
/// - organizationService: The service used to manage syncing and updates to the user's organizations.
364+
/// - policyService: The service for managing the polices for the user.
353365
/// - settingsService: The service used by the application to manage user settings.
354366
/// - stateService: The service used by the application to manage account state.
355367
/// - syncService: The service used to handle syncing vault data with the API.
@@ -365,6 +377,7 @@ class DefaultVaultRepository { // swiftlint:disable:this type_body_length
365377
errorReporter: ErrorReporter,
366378
folderService: FolderService,
367379
organizationService: OrganizationService,
380+
policyService: PolicyService,
368381
settingsService: SettingsService,
369382
stateService: StateService,
370383
syncService: SyncService,
@@ -379,6 +392,7 @@ class DefaultVaultRepository { // swiftlint:disable:this type_body_length
379392
self.errorReporter = errorReporter
380393
self.folderService = folderService
381394
self.organizationService = organizationService
395+
self.policyService = policyService
382396
self.settingsService = settingsService
383397
self.stateService = stateService
384398
self.syncService = syncService
@@ -919,6 +933,12 @@ extension DefaultVaultRepository: VaultRepository {
919933
try await cipherService.addCipherWithServer(cipher)
920934
}
921935

936+
func canShowVaultFilter() async -> Bool {
937+
let disablePersonalOwnership = await policyService.policyAppliesToUser(.personalOwnership)
938+
let singleOrg = await policyService.policyAppliesToUser(.onlyOrg)
939+
return !(disablePersonalOwnership && singleOrg)
940+
}
941+
922942
func clearTemporaryDownloads() {
923943
Task {
924944
do {

BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
2121
var now: Date!
2222
var premiumAccount = Account.fixture(profile: .fixture(hasPremiumPersonally: true))
2323
var organizationService: MockOrganizationService!
24+
var policyService: MockPolicyService!
2425
var stateService: MockStateService!
2526
var subject: DefaultVaultRepository!
2627
var syncService: MockSyncService!
@@ -44,6 +45,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
4445
folderService = MockFolderService()
4546
now = Date(year: 2024, month: 1, day: 18)
4647
organizationService = MockOrganizationService()
48+
policyService = MockPolicyService()
4749
syncService = MockSyncService()
4850
timeProvider = MockTimeProvider(.mockTime(now))
4951
vaultTimeoutService = MockVaultTimeoutService()
@@ -59,6 +61,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
5961
errorReporter: errorReporter,
6062
folderService: folderService,
6163
organizationService: organizationService,
64+
policyService: policyService,
6265
settingsService: MockSettingsService(),
6366
stateService: stateService,
6467
syncService: syncService,
@@ -81,6 +84,7 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
8184
fido2UserInterfaceHelper = nil
8285
folderService = nil
8386
organizationService = nil
87+
policyService = nil
8488
now = nil
8589
stateService = nil
8690
subject = nil
@@ -109,6 +113,42 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
109113
}
110114
}
111115

116+
/// `canShowVaultFilter()` returns true if only org and personal ownership policies are disabled.
117+
func test_canShowVaultFilter_onlyOrgAndPersonalOwnershipDisabled() async {
118+
policyService.policyAppliesToUserResult[.onlyOrg] = false
119+
policyService.policyAppliesToUserResult[.personalOwnership] = false
120+
121+
let canShowVaultFilter = await subject.canShowVaultFilter()
122+
XCTAssertTrue(canShowVaultFilter)
123+
}
124+
125+
/// `canShowVaultFilter()` returns false if the only org and personal ownership policies are enabled.
126+
func test_canShowVaultFilter_onlyOrgAndPersonalOwnershipEnabled() async {
127+
policyService.policyAppliesToUserResult[.onlyOrg] = true
128+
policyService.policyAppliesToUserResult[.personalOwnership] = true
129+
130+
let canShowVaultFilter = await subject.canShowVaultFilter()
131+
XCTAssertFalse(canShowVaultFilter)
132+
}
133+
134+
/// `canShowVaultFilter()` returns false if the only org is enabled but not personal ownership.
135+
func test_canShowVaultFilter_onlyOrgEnabled() async {
136+
policyService.policyAppliesToUserResult[.onlyOrg] = true
137+
policyService.policyAppliesToUserResult[.personalOwnership] = false
138+
139+
let canShowVaultFilter = await subject.canShowVaultFilter()
140+
XCTAssertTrue(canShowVaultFilter)
141+
}
142+
143+
/// `canShowVaultFilter()` returns false if the personal ownership is enabled but not only org.
144+
func test_canShowVaultFilter_personalOwnershipEnabled() async {
145+
policyService.policyAppliesToUserResult[.onlyOrg] = false
146+
policyService.policyAppliesToUserResult[.personalOwnership] = true
147+
148+
let canShowVaultFilter = await subject.canShowVaultFilter()
149+
XCTAssertTrue(canShowVaultFilter)
150+
}
151+
112152
/// `cipherPublisher()` returns a publisher for the list of a user's ciphers.
113153
func test_cipherPublisher() async throws {
114154
let ciphers: [Cipher] = [.fixture(name: "Bitwarden")]

BitwardenShared/UI/Platform/Tabs/TabCoordinator.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@ final class TabCoordinator: Coordinator, HasTabNavigator {
164164
do {
165165
for try await organizations in try await vaultRepository.organizationsPublisher() {
166166
guard let navigator = tabNavigator?.navigator(for: TabRoute.vault(.list)) else { return }
167-
if organizations.isEmpty {
167+
let canShowVaultFilter = await vaultRepository.canShowVaultFilter()
168+
if organizations.isEmpty || !canShowVaultFilter {
168169
navigator.rootViewController?.title = Localizations.myVault
169170
} else {
170171
navigator.rootViewController?.title = Localizations.vaults

BitwardenShared/UI/Platform/Tabs/TabCoordinatorTests.swift

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,24 @@ class TabCoordinatorTests: BitwardenTestCase {
167167
XCTAssertEqual(viewController.title, Localizations.vaults)
168168
}
169169

170+
/// `start()` subscribes to the organization publisher and updates the vault navigation bar
171+
/// title if the vault filter can't be shown.
172+
func test_start_organizationsCanShowVaultFilterDisabled() {
173+
let mockRoot = MockRootNavigator()
174+
let viewController = UIViewController()
175+
mockRoot.rootViewController = viewController
176+
tabNavigator.navigatorForTabReturns = mockRoot
177+
vaultRepository.canShowVaultFilter = false
178+
vaultRepository.organizationsSubject = .init([
179+
.fixture(),
180+
])
181+
182+
subject.start()
183+
184+
waitFor(viewController.title != nil)
185+
XCTAssertEqual(viewController.title, Localizations.myVault)
186+
}
187+
170188
/// `start()` presents the tab navigator within the root navigator and starts the child-coordinators.
171189
func test_start_organizationsError() throws {
172190
let mockRoot = MockRootNavigator()

BitwardenShared/UI/Vault/Vault/VaultGroup/VaultGroupProcessor.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ final class VaultGroupProcessor: StateProcessor<
158158
private func checkPersonalOwnershipPolicy() async {
159159
let isPersonalOwnershipDisabled = await services.policyService.policyAppliesToUser(.personalOwnership)
160160
state.isPersonalOwnershipDisabled = isPersonalOwnershipDisabled
161+
state.canShowVaultFilter = await services.vaultRepository.canShowVaultFilter()
161162
}
162163

163164
/// Refreshes the vault group's TOTP Codes.

BitwardenShared/UI/Vault/Vault/VaultGroup/VaultGroupProcessorTests.swift

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,43 @@ class VaultGroupProcessorTests: BitwardenTestCase { // swiftlint:disable:this ty
143143
XCTAssertTrue(subject.state.isPersonalOwnershipDisabled)
144144
}
145145

146+
/// `perform(_:)` with `appeared` determines whether the vault filter can be shown based on
147+
/// policy settings.
148+
func test_perform_appeared_policyCanShowVaultFilterDisabled() {
149+
vaultRepository.canShowVaultFilter = false
150+
subject.state.organizations = [.fixture()]
151+
152+
let task = Task {
153+
await subject.perform(.appeared)
154+
}
155+
waitFor(subject.state.loadingState == .data([]))
156+
task.cancel()
157+
158+
XCTAssertFalse(subject.state.canShowVaultFilter)
159+
XCTAssertFalse(subject.state.vaultFilterState.canShowVaultFilter)
160+
XCTAssertEqual(subject.state.vaultFilterState.vaultFilterOptions, [])
161+
}
162+
163+
/// `perform(_:)` with `appeared` determines whether the vault filter can be shown based on
164+
/// policy settings.
165+
func test_perform_appeared_policyCanShowVaultFilterEnabled() {
166+
vaultRepository.canShowVaultFilter = true
167+
subject.state.organizations = [.fixture()]
168+
169+
let task = Task {
170+
await subject.perform(.appeared)
171+
}
172+
waitFor(subject.state.loadingState == .data([]))
173+
task.cancel()
174+
175+
XCTAssertTrue(subject.state.canShowVaultFilter)
176+
XCTAssertTrue(subject.state.vaultFilterState.canShowVaultFilter)
177+
XCTAssertEqual(
178+
subject.state.vaultFilterState.vaultFilterOptions,
179+
[.allVaults, .myVault, .organization(.fixture())]
180+
)
181+
}
182+
146183
/// `perform(_:)` with `.morePressed` has the vault item more options helper display the alert.
147184
func test_perform_morePressed() async throws {
148185
await subject.perform(.morePressed(.fixture()))

BitwardenShared/UI/Vault/Vault/VaultGroup/VaultGroupState.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import Foundation
66
struct VaultGroupState: Equatable {
77
// MARK: Properties
88

9+
/// Whether the vault filter can be shown.
10+
var canShowVaultFilter = true
11+
912
/// Whether there is data for the vault group.
1013
var emptyData: Bool {
1114
loadingState.data.isEmptyOrNil
@@ -83,6 +86,16 @@ struct VaultGroupState: Equatable {
8386
/// The url to open in the device's web browser.
8487
var url: URL?
8588

89+
/// The state for showing the vault filter.
90+
var vaultFilterState: SearchVaultFilterRowState {
91+
SearchVaultFilterRowState(
92+
canShowVaultFilter: canShowVaultFilter,
93+
isPersonalOwnershipDisabled: isPersonalOwnershipDisabled,
94+
organizations: organizations,
95+
searchVaultFilterType: searchVaultFilterType
96+
)
97+
}
98+
8699
/// The vault filter used to display a single or all vaults for the user.
87100
let vaultFilterType: VaultFilterType
88101
}

BitwardenShared/UI/Vault/Vault/VaultGroup/VaultGroupView.swift

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -156,13 +156,7 @@ struct VaultGroupView: View {
156156
private var searchVaultFilterRow: some View {
157157
SearchVaultFilterRowView(
158158
hasDivider: true, store: store.child(
159-
state: { state in
160-
SearchVaultFilterRowState(
161-
organizations: state.organizations,
162-
searchVaultFilterType: state.searchVaultFilterType,
163-
isPersonalOwnershipDisabled: state.isPersonalOwnershipDisabled
164-
)
165-
},
159+
state: \.vaultFilterState,
166160
mapAction: { action in
167161
switch action {
168162
case let .searchVaultFilterChanged(type):

BitwardenShared/UI/Vault/Vault/VaultList/SearchVaultFilterRowView/SearchVaultFilterRowState.swift

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,22 @@
55
struct SearchVaultFilterRowState: Equatable {
66
// MARK: Properties
77

8+
/// Whether the vault filter can be shown.
9+
var canShowVaultFilter = true
10+
11+
/// Whether the policy is enforced to disable personal vault ownership.
12+
var isPersonalOwnershipDisabled: Bool = false
13+
814
/// The list of organizations the user is a member of.
915
var organizations: [Organization] = []
1016

1117
/// The search vault filter used to display a single or all vaults for the user.
1218
var searchVaultFilterType: VaultFilterType = .allVaults
1319

14-
/// Whether the policy is enforced to disable personal vault ownership.
15-
var isPersonalOwnershipDisabled: Bool = false
16-
1720
/// The list of vault filter options that can be used to filter the vault, if the user is a
1821
/// member of any organizations.
1922
var vaultFilterOptions: [VaultFilterType] {
20-
guard !organizations.isEmpty else { return [] }
23+
guard !organizations.isEmpty, canShowVaultFilter else { return [] }
2124

2225
let sortedOrganizations = organizations
2326
.sorted { $0.name.localizedStandardCompare($1.name) == .orderedAscending }

BitwardenShared/UI/Vault/Vault/VaultList/SearchVaultFilterRowView/SearchVaultFilterRowStateTests.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,34 @@ class SearchVaultFilterRowStateTests: BitwardenTestCase {
4545
]
4646
)
4747
}
48+
49+
/// `vaultFilterOptions` returns no filter options if organizations exist but the filter should be hidden.
50+
func test_vaultFilterOptions_organizationsShouldNotShowVaultFilter() {
51+
subject.organizations = [
52+
Organization.fixture(id: "1", name: "Org 1"),
53+
Organization.fixture(id: "2", name: "Test Org"),
54+
Organization.fixture(id: "3", name: "ABC Org"),
55+
]
56+
subject.canShowVaultFilter = false
57+
XCTAssertEqual(subject.vaultFilterOptions, [])
58+
}
59+
60+
/// `vaultFilterOptions` returns the filter organization filter options if personal ownership is disabled.
61+
func test_vaultFilterOptions_personalOwnershipDisabled() {
62+
subject.organizations = [
63+
Organization.fixture(id: "1", name: "Org 1"),
64+
Organization.fixture(id: "2", name: "Test Org"),
65+
Organization.fixture(id: "3", name: "ABC Org"),
66+
]
67+
subject.isPersonalOwnershipDisabled = true
68+
XCTAssertEqual(
69+
subject.vaultFilterOptions,
70+
[
71+
.allVaults,
72+
.organization(Organization.fixture(id: "3", name: "ABC Org")),
73+
.organization(Organization.fixture(id: "1", name: "Org 1")),
74+
.organization(Organization.fixture(id: "2", name: "Test Org")),
75+
]
76+
)
77+
}
4878
}

BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessor.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ extension VaultListProcessor {
179179
private func checkPersonalOwnershipPolicy() async {
180180
let isPersonalOwnershipDisabled = await services.policyService.policyAppliesToUser(.personalOwnership)
181181
state.isPersonalOwnershipDisabled = isPersonalOwnershipDisabled
182+
state.canShowVaultFilter = await services.vaultRepository.canShowVaultFilter()
182183
}
183184

184185
/// Checks if we need to display the unassigned ciphers alert, and displays if necessary.

0 commit comments

Comments
 (0)