Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BIT-2284: Display unassigned ciphers banner #638

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ class DefaultAuthRepository {
/// The service that handles common client functionality such as encryption and decryption.
private let clientService: ClientService

/// The services to get server-specified configuration.
/// The service to get server-specified configuration.
private let configService: ConfigService

/// The service used by the application to manage the environment settings.
Expand Down
12 changes: 12 additions & 0 deletions BitwardenShared/Core/Platform/Services/ConfigService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@ extension ConfigService {
func getConfig() async -> ServerConfig? {
await getConfig(forceRefresh: false)
}

func getFeatureFlag(_ flag: FeatureFlag, defaultValue: Bool = false) async -> Bool {
await getFeatureFlag(flag, defaultValue: defaultValue, forceRefresh: false)
}

func getFeatureFlag(_ flag: FeatureFlag, defaultValue: Int = 0) async -> Int {
await getFeatureFlag(flag, defaultValue: defaultValue, forceRefresh: false)
}

func getFeatureFlag(_ flag: FeatureFlag, defaultValue: String? = nil) async -> String? {
await getFeatureFlag(flag, defaultValue: defaultValue, forceRefresh: false)
}
}

// MARK: - DefaultConfigService
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -470,10 +470,10 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le
)

let vaultRepository = DefaultVaultRepository(
cipherAPIService: apiService,
cipherService: cipherService,
clientService: clientService,
collectionService: collectionService,
configService: configService,
environmentService: environmentService,
errorReporter: errorReporter,
folderService: folderService,
Expand Down Expand Up @@ -536,10 +536,6 @@ extension ServiceContainer {
apiService
}

var cipherAPIService: CipherAPIService {
apiService
}

var configAPIService: ConfigAPIService {
apiService
}
Expand Down
8 changes: 0 additions & 8 deletions BitwardenShared/Core/Platform/Services/Services.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ typealias Services = HasAPIService
& HasBiometricsRepository
& HasCameraService
& HasCaptchaService
& HasCipherAPIService
& HasClientService
& HasConfigService
& HasDeviceAPIService
Expand Down Expand Up @@ -115,13 +114,6 @@ protocol HasClientService {
var clientService: ClientService { get }
}

/// Protocol for an object that provides a `CipherAPIService`.
///
protocol HasCipherAPIService {
/// The service used by the application to make cipher-related API requests.
var cipherAPIService: CipherAPIService { get }
}

/// Protocol for an object that provides a `ConfigService`.
///
protocol HasConfigService {
Expand Down
26 changes: 26 additions & 0 deletions BitwardenShared/Core/Platform/Services/StateService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,13 @@ protocol StateService: AnyObject {
///
func getServerConfig(userId: String?) async throws -> ServerConfig?

/// Gets whether we should check for unassigned items for the user.
///
/// - Parameter userId: The user ID associated with the flag.
/// - Returns: `false` if the user has seen and acknowledged the unassigned items alert.
///
func getShouldCheckOrganizationUnassignedItems(userId: String?) async throws -> Bool

/// Get whether the device should be trusted.
///
/// - Returns: Whether to trust the device.
Expand Down Expand Up @@ -443,6 +450,15 @@ protocol StateService: AnyObject {
///
func setServerConfig(_ config: ServerConfig?, userId: String?) async throws

/// Sets whether or not we should check for unassigned ciphers in an organization for
/// a particular user.
///
/// - Parameters:
/// - shouldCheck: Whether or not we should check for unassigned ciphers.
/// - userId: The user ID that acknowledged the alert.
///
func setShouldCheckOrganizationUnassignedItems(_ shouldCheck: Bool?, userId: String?) async throws

/// Set whether to trust the device.
///
/// - Parameter shouldTrustDevice: Whether to trust the device.
Expand Down Expand Up @@ -1116,6 +1132,11 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
return appSettingsStore.serverConfig(userId: userId)
}

func getShouldCheckOrganizationUnassignedItems(userId: String?) async throws -> Bool {
let userId = try userId ?? getActiveAccountUserId()
return appSettingsStore.shouldCheckOrganizationUnassignedItems(userId: userId) ?? true
}

func getShouldTrustDevice(userId: String) async -> Bool? {
appSettingsStore.shouldTrustDevice(userId: userId)
}
Expand Down Expand Up @@ -1312,6 +1333,11 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
appSettingsStore.setServerConfig(config, userId: userId)
}

func setShouldCheckOrganizationUnassignedItems(_ shouldCheck: Bool?, userId: String?) async throws {
let userId = try userId ?? getActiveAccountUserId()
appSettingsStore.setShouldCheckOrganizationUnassignedItems(shouldCheck, userId: userId)
}

func setShouldTrustDevice(_ shouldTrustDevice: Bool?, userId: String) {
appSettingsStore.setShouldTrustDevice(shouldTrustDevice: shouldTrustDevice, userId: userId)
}
Expand Down
22 changes: 22 additions & 0 deletions BitwardenShared/Core/Platform/Services/StateServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,22 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertEqual(value, model)
}

/// `getShouldCheckOrganizationUnassignedItems` returns the config value
func test_getShouldCheckOrganizationUnassignedItems() async throws {
appSettingsStore.shouldCheckOrganizationUnassignedItems["1"] = false
var shouldCheck = try await subject.getShouldCheckOrganizationUnassignedItems(userId: "1")
XCTAssertFalse(shouldCheck)
appSettingsStore.shouldCheckOrganizationUnassignedItems["1"] = true
shouldCheck = try await subject.getShouldCheckOrganizationUnassignedItems(userId: "1")
XCTAssertTrue(shouldCheck)
}

/// `getShouldCheckOrganizationUnassignedItems` returns true if it hasn't been set
func test_getShouldCheckOrganizationUnassignedItems_notSet() async throws {
let shouldCheck = try await subject.getShouldCheckOrganizationUnassignedItems(userId: "1")
XCTAssertTrue(shouldCheck)
}

/// `getShowWebIcons` gets the show web icons value.
func test_getShowWebIcons() async {
appSettingsStore.disableWebIcons = true
Expand Down Expand Up @@ -1262,6 +1278,12 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertEqual(appSettingsStore.serverConfig["1"], model)
}

/// `setShouldCheckOrganizationUnassignedItems` saves the should check value.
func test_setShouldCheckOrganizationUnassignedItems() async throws {
try await subject.setShouldCheckOrganizationUnassignedItems(true, userId: "1")
XCTAssertEqual(appSettingsStore.shouldCheckOrganizationUnassignedItems["1"], true)
}

/// `setShouldTrustDevice` saves the should trust device value.
func test_setShouldTrustDevice() async {
await subject.setShouldTrustDevice(true, userId: "1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,15 @@ protocol AppSettingsStore: AnyObject {
///
func setServerConfig(_ config: ServerConfig?, userId: String)

/// Sets whether or not we should check for unassigned ciphers in an organization for
/// a particular user.
///
/// - Parameters:
/// - shouldCheck: Whether or not we should check for unassigned ciphers.
/// - userId: The user ID to track.
///
func setShouldCheckOrganizationUnassignedItems(_ shouldCheck: Bool?, userId: String)

/// Set whether to trust the device.
///
/// - Parameter shouldTrustDevice: Whether to trust the device.
Expand Down Expand Up @@ -361,20 +370,27 @@ protocol AppSettingsStore: AnyObject {
///
func setUsernameGenerationOptions(_ options: UsernameGenerationOptions?, userId: String)

/// Returns the action taken upon a session timeout.
/// Gets whether or not we should check for unassigned ciphers in an organization for
/// a particular user.
///
/// - Parameter userId: The user ID associated with the session timeout action.
/// - Returns: The user's session timeout action.
/// - Parameter userId: The user ID to check.
///
func timeoutAction(userId: String) -> Int?
func shouldCheckOrganizationUnassignedItems(userId: String) -> Bool?

/// Get whether the device should be trusted.
///
/// - Returns: Whether to trust the device.
///
func shouldTrustDevice(userId: String) -> Bool?

/// Get the two-factor token associated with a user's email..
/// Returns the action taken upon a session timeout.
///
/// - Parameter userId: The user ID associated with the session timeout action.
/// - Returns: The user's session timeout action.
///
func timeoutAction(userId: String) -> Int?

/// Get the two-factor token associated with a user's email.
///
/// - Parameter email: The user's email.
/// - Returns: The two-factor token.
Expand Down Expand Up @@ -555,6 +571,7 @@ extension DefaultAppSettingsStore: AppSettingsStore {
case rememberedEmail
case rememberedOrgIdentifier
case serverConfig(userId: String)
case shouldCheckOrganizationUnassignedItems(userId: String)
case shouldTrustDevice(userId: String)
case state
case twoFactorToken(email: String)
Expand Down Expand Up @@ -625,6 +642,8 @@ extension DefaultAppSettingsStore: AppSettingsStore {
key = "rememberedOrgIdentifier"
case let .serverConfig(userId):
key = "serverConfig_\(userId)"
case let .shouldCheckOrganizationUnassignedItems(userId):
key = "shouldCheckOrganizationUnassignedItems_\(userId)"
case let .shouldTrustDevice(userId):
key = "shouldTrustDevice_\(userId)"
case .state:
Expand Down Expand Up @@ -863,6 +882,10 @@ extension DefaultAppSettingsStore: AppSettingsStore {
store(config, for: .serverConfig(userId: userId))
}

func setShouldCheckOrganizationUnassignedItems(_ shouldCheck: Bool?, userId: String) {
store(shouldCheck, for: .shouldCheckOrganizationUnassignedItems(userId: userId))
}

func setShouldTrustDevice(shouldTrustDevice: Bool?, userId: String) {
store(shouldTrustDevice, for: .shouldTrustDevice(userId: userId))
}
Expand All @@ -883,6 +906,10 @@ extension DefaultAppSettingsStore: AppSettingsStore {
store(minutes, for: .vaultTimeout(userId: userId))
}

func shouldCheckOrganizationUnassignedItems(userId: String) -> Bool? {
fetch(for: .shouldCheckOrganizationUnassignedItems(userId: userId))
matt-livefront marked this conversation as resolved.
Show resolved Hide resolved
}

func timeoutAction(userId: String) -> Int? {
fetch(for: .vaultTimeoutAction(userId: userId))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,19 @@ class AppSettingsStoreTests: BitwardenTestCase { // swiftlint:disable:this type_
)
}

/// `shouldCheckOrganizationUnassignedItems(:)` is initially nil {
func test_shouldCheckOrganizationUnassignedItems_isInitiallyNil() {
XCTAssertNil(subject.shouldCheckOrganizationUnassignedItems(userId: "1"))
}

/// `shouldCheckOrganizationUnassignedItems(:)` can be used to get and set the persisted value.
func test_shouldCheckOrganizationUnassignedItems_withValue() {
subject.setShouldCheckOrganizationUnassignedItems(true, userId: "1")
XCTAssertEqual(subject.shouldCheckOrganizationUnassignedItems(userId: "1"), true)
subject.setShouldCheckOrganizationUnassignedItems(false, userId: "1")
XCTAssertEqual(subject.shouldCheckOrganizationUnassignedItems(userId: "1"), false)
}

/// `twoFactorToken(email:)` returns `nil` if there isn't a previously stored value.
func test_twoFactorToken_isInitiallyNil() {
XCTAssertNil(subject.twoFactorToken(email: "[email protected]"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class MockAppSettingsStore: AppSettingsStore {
var passwordGenerationOptions = [String: PasswordGenerationOptions]()
var pinProtectedUserKey = [String: String]()
var serverConfig = [String: ServerConfig]()
var shouldCheckOrganizationUnassignedItems = [String: Bool?]()
var shouldTrustDevice = [String: Bool?]()
var timeoutAction = [String: Int]()
var twoFactorTokens = [String: String]()
Expand Down Expand Up @@ -185,6 +186,10 @@ class MockAppSettingsStore: AppSettingsStore {
serverConfig[userId] = config
}

func setShouldCheckOrganizationUnassignedItems(_ shouldCheck: Bool?, userId: String) {
shouldCheckOrganizationUnassignedItems[userId] = shouldCheck
}

func setShouldTrustDevice(shouldTrustDevice: Bool?, userId: String) {
self.shouldTrustDevice[userId] = shouldTrustDevice
}
Expand Down Expand Up @@ -213,6 +218,10 @@ class MockAppSettingsStore: AppSettingsStore {
vaultTimeout[userId] = minutes
}

func shouldCheckOrganizationUnassignedItems(userId: String) -> Bool? {
shouldCheckOrganizationUnassignedItems[userId] ?? nil
}

func shouldTrustDevice(userId: String) -> Bool? {
shouldTrustDevice[userId] ?? false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
var serverConfig = [String: ServerConfig]()
var setBiometricAuthenticationEnabledResult: Result<Void, Error> = .success(())
var setBiometricIntegrityStateError: Error?
var shouldCheckOrganizationUnassignedItems = [String: Bool?]()
var shouldTrustDevice = [String: Bool?]()
var twoFactorTokens = [String: String]()
var unsuccessfulUnlockAttempts = [String: Int]()
Expand Down Expand Up @@ -211,6 +212,11 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
return serverConfig[userId]
}

func getShouldCheckOrganizationUnassignedItems(userId: String?) async throws -> Bool {
let userId = try unwrapUserId(userId)
return (shouldCheckOrganizationUnassignedItems[userId] ?? false) ?? false
}

func getShouldTrustDevice(userId: String) async -> Bool? {
shouldTrustDevice[userId] ?? false
}
Expand Down Expand Up @@ -395,6 +401,11 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
serverConfig[userId] = config
}

func setShouldCheckOrganizationUnassignedItems(_ shouldCheck: Bool?, userId: String?) async throws {
let userId = try unwrapUserId(userId)
shouldCheckOrganizationUnassignedItems[userId] = shouldCheck
}

func setShouldTrustDevice(_ shouldTrustDevice: Bool?, userId: String) async {
self.shouldTrustDevice[userId] = shouldTrustDevice
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class MockVaultRepository: VaultRepository {

var getDisableAutoTotpCopyResult: Result<Bool, Error> = .success(false)

var hasUnassignedCiphersResult: Result<Bool, Error> = .success(false)

var needsSyncCalled = false
var needsSyncResult: Result<Bool, Error> = .success(false)

Expand Down Expand Up @@ -80,6 +82,8 @@ class MockVaultRepository: VaultRepository {
var shareCipherCiphers = [CipherView]()
var shareCipherResult: Result<Void, Error> = .success(())

var shouldShowUnassignedCiphersAlert = false

var softDeletedCipher = [CipherView]()
var softDeleteCipherResult: Result<Void, Error> = .success(())

Expand Down Expand Up @@ -178,6 +182,10 @@ class MockVaultRepository: VaultRepository {
try getDisableAutoTotpCopyResult.get()
}

func hasUnassignedCiphers() async throws -> Bool {
try hasUnassignedCiphersResult.get()
}

func needsSync() async throws -> Bool {
needsSyncCalled = true
return try needsSyncResult.get()
Expand Down Expand Up @@ -237,6 +245,10 @@ class MockVaultRepository: VaultRepository {
try shareCipherResult.get()
}

func shouldShowUnassignedCiphersAlert() async -> Bool {
shouldShowUnassignedCiphersAlert
}

func softDeleteCipher(_ cipher: CipherView) async throws {
softDeletedCipher.append(cipher)
try softDeleteCipherResult.get()
Expand Down
Loading
Loading