From 6c6c3eda6fc2ba0328e38173f08e28ea6e6f0947 Mon Sep 17 00:00:00 2001 From: Federico Maccaroni Date: Mon, 23 Dec 2024 13:30:04 -0300 Subject: [PATCH] [PM-15554] Added cipher-key-encryption remote feature flag logic (#1205) --- .../Auth/Repositories/AuthRepository.swift | 2 +- .../Domain/EnvironmentUrlDataTests.swift | 10 ++- .../Platform/Models/Enum/FeatureFlag.swift | 7 ++- .../Models/Enum/FeatureFlagTests.swift | 6 ++ .../Platform/Services/ClientService.swift | 12 +++- .../Services/ClientServiceTests.swift | 63 +++++++++++++++++-- .../Platform/Services/ConfigService.swift | 6 +- .../Services/EnvironmentServiceTests.swift | 9 ++- .../GeneratorState+UsernameState.swift | 2 +- ...VaultAutofillListProcessor+TotpTests.swift | 2 +- .../AuthenticatorKeyCaptureCoordinator.swift | 2 +- 11 files changed, 102 insertions(+), 19 deletions(-) diff --git a/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift b/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift index db85a1a6c..cc8df17c8 100644 --- a/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift +++ b/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift @@ -877,7 +877,7 @@ extension DefaultAuthRepository: AuthRepository { } func unlockVaultFromLoginWithDevice(privateKey: String, key: String, masterPasswordHash: String?) async throws { - let method: AuthRequestMethod = + let method = if masterPasswordHash != nil, let encUserKey = try await stateService.getAccountEncryptionKeys().encryptedUserKey { AuthRequestMethod.masterKey(protectedMasterKey: key, authRequestKey: encUserKey) diff --git a/BitwardenShared/Core/Platform/Models/Domain/EnvironmentUrlDataTests.swift b/BitwardenShared/Core/Platform/Models/Domain/EnvironmentUrlDataTests.swift index 820eb34b8..c5740d94a 100644 --- a/BitwardenShared/Core/Platform/Models/Domain/EnvironmentUrlDataTests.swift +++ b/BitwardenShared/Core/Platform/Models/Domain/EnvironmentUrlDataTests.swift @@ -193,7 +193,10 @@ class EnvironmentUrlDataTests: XCTestCase { /// `setUpTwoFactorURL` returns the change email URL for the base URL. func test_setUpTwoFactorURL_baseURL() { let subject = EnvironmentUrlData(base: URL(string: "https://vault.example.com")) - XCTAssertEqual(subject.setUpTwoFactorURL?.absoluteString, "https://vault.example.com/#/settings/security/two-factor") + XCTAssertEqual( + subject.setUpTwoFactorURL?.absoluteString, + "https://vault.example.com/#/settings/security/two-factor" + ) } /// `setUpTwoFactorURL` returns the default change email base URL. @@ -208,7 +211,10 @@ class EnvironmentUrlDataTests: XCTestCase { base: URL(string: "https://vault.example.com"), webVault: URL(string: "https://web.vault.example.com") ) - XCTAssertEqual(subject.setUpTwoFactorURL?.absoluteString, "https://web.vault.example.com/#/settings/security/two-factor") + XCTAssertEqual( + subject.setUpTwoFactorURL?.absoluteString, + "https://web.vault.example.com/#/settings/security/two-factor" + ) } /// `webVaultHost` returns the host for the base URL if no web vault URL is set. diff --git a/BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift b/BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift index a9d20028e..e6dc7cbdf 100644 --- a/BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift +++ b/BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift @@ -13,6 +13,9 @@ enum FeatureFlag: String, CaseIterable, Codable { /// Flag to enable/disable Credential Exchange import flow. case cxpImportMobile = "cxp-import-mobile" + /// Flag to enable/disable individual cipher encryption configured remotely. + case cipherKeyEncryption = "cipher-key-encryption" + /// Flag to enable/disable email verification during registration /// This flag introduces a new flow for account creation case emailVerification = "email-verification" @@ -23,7 +26,7 @@ enum FeatureFlag: String, CaseIterable, Codable { /// Flag to enable/disable the ability to sync TOTP codes with the Authenticator app. case enableAuthenticatorSync = "enable-authenticator-sync-ios" - /// A flag that enables individual cipher encryption. + /// An SDK flag that enables individual cipher encryption. case enableCipherKeyEncryption /// A feature flag for the import logins flow for new accounts. @@ -79,6 +82,7 @@ enum FeatureFlag: String, CaseIterable, Codable { /// but if `isRemotelyConfigured` is false for the flag, then the value here will be used. /// This is a helpful way to manage local feature flags. static let initialValues: [FeatureFlag: AnyCodable] = [ + .cipherKeyEncryption: .bool(true), .testLocalInitialBoolFlag: .bool(true), .testLocalInitialIntFlag: .int(42), .testLocalInitialStringFlag: .string("Test String"), @@ -103,6 +107,7 @@ enum FeatureFlag: String, CaseIterable, Codable { .testLocalInitialStringFlag: false case .appReviewPrompt, + .cipherKeyEncryption, .cxpExportMobile, .cxpImportMobile, .emailVerification, diff --git a/BitwardenShared/Core/Platform/Models/Enum/FeatureFlagTests.swift b/BitwardenShared/Core/Platform/Models/Enum/FeatureFlagTests.swift index 43d4cab68..85fe09ffb 100644 --- a/BitwardenShared/Core/Platform/Models/Enum/FeatureFlagTests.swift +++ b/BitwardenShared/Core/Platform/Models/Enum/FeatureFlagTests.swift @@ -12,9 +12,15 @@ final class FeatureFlagTests: BitwardenTestCase { XCTAssertEqual(filtered, []) } + /// `initialValues` returns the correct value for each flag. + func test_initialValues() { + XCTAssertTrue(FeatureFlag.initialValues[.cipherKeyEncryption]?.boolValue == true) + } + /// `getter:isRemotelyConfigured` returns the correct value for each flag. func test_isRemotelyConfigured() { XCTAssertTrue(FeatureFlag.appReviewPrompt.isRemotelyConfigured) + XCTAssertTrue(FeatureFlag.cipherKeyEncryption.isRemotelyConfigured) XCTAssertTrue(FeatureFlag.cxpExportMobile.isRemotelyConfigured) XCTAssertTrue(FeatureFlag.cxpImportMobile.isRemotelyConfigured) XCTAssertTrue(FeatureFlag.emailVerification.isRemotelyConfigured) diff --git a/BitwardenShared/Core/Platform/Services/ClientService.swift b/BitwardenShared/Core/Platform/Services/ClientService.swift index 140cd0ffc..4db363bdd 100644 --- a/BitwardenShared/Core/Platform/Services/ClientService.swift +++ b/BitwardenShared/Core/Platform/Services/ClientService.swift @@ -255,7 +255,7 @@ actor DefaultClientService: ClientService { // Get the current config and load the flags. let config = await configService.getConfig() - loadFlags(config, for: newClient) + await loadFlags(config, for: newClient) return newClient } @@ -288,14 +288,20 @@ actor DefaultClientService: ClientService { /// Loads the flags into the SDK. /// - Parameter config: Config to update the flags. - private func loadFlags(_ config: ServerConfig?, for client: BitwardenSdkClient) { + private func loadFlags(_ config: ServerConfig?, for client: BitwardenSdkClient) async { do { guard let config else { return } + let cipherKeyEncryptionFlagEnabled: Bool = await configService.getFeatureFlag( + .cipherKeyEncryption, + defaultValue: true + ) + let enableCipherKeyEncryption = cipherKeyEncryptionFlagEnabled && config.supportsCipherKeyEncryption() + try client.platform().loadFlags([ - FeatureFlag.enableCipherKeyEncryption.rawValue: config.supportsCipherKeyEncryption(), + FeatureFlag.enableCipherKeyEncryption.rawValue: enableCipherKeyEncryption, ]) } catch { errorReporter.log(error: error) diff --git a/BitwardenShared/Core/Platform/Services/ClientServiceTests.swift b/BitwardenShared/Core/Platform/Services/ClientServiceTests.swift index c37d0eccf..7abc20b80 100644 --- a/BitwardenShared/Core/Platform/Services/ClientServiceTests.swift +++ b/BitwardenShared/Core/Platform/Services/ClientServiceTests.swift @@ -105,6 +105,7 @@ final class ClientServiceTests: BitwardenTestCase { // swiftlint:disable:this ty /// `client(for:)` loads flags into the SDK. @MainActor func test_client_loadFlags() async throws { + configService.featureFlagsBool[.cipherKeyEncryption] = true configService.configMocker.withResult(ServerConfig( date: Date(year: 2024, month: 2, day: 14, hour: 7, minute: 50, second: 0), responseModel: ConfigResponseModel( @@ -125,9 +126,11 @@ final class ClientServiceTests: BitwardenTestCase { // swiftlint:disable:this ty ) } - /// `client(for:)` loads `enableCipherKeyEncryption` flag as `false` into the SDK. + /// `client(for:)` loads `enableCipherKeyEncryption` flag as `false` into the SDK + /// when the server version is old. @MainActor - func test_client_loadFlagsEnableCipherKeyEncryptionFalse() async throws { + func test_client_loadFlagsEnableCipherKeyEncryptionFalseBecauseOfServerVersion() async throws { + configService.featureFlagsBool[.cipherKeyEncryption] = true configService.configMocker.withResult(ServerConfig( date: Date(year: 2024, month: 2, day: 14, hour: 7, minute: 50, second: 0), responseModel: ConfigResponseModel( @@ -148,6 +151,31 @@ final class ClientServiceTests: BitwardenTestCase { // swiftlint:disable:this ty ) } + /// `client(for:)` loads `enableCipherKeyEncryption` flag as `false` into the SDK + /// when the server version is old. + @MainActor + func test_client_loadFlagsEnableCipherKeyEncryptionFalseBecauseOfFeatureFlag() async throws { + configService.featureFlagsBool[.cipherKeyEncryption] = false + configService.configMocker.withResult(ServerConfig( + date: Date(year: 2024, month: 2, day: 14, hour: 7, minute: 50, second: 0), + responseModel: ConfigResponseModel( + environment: nil, + featureStates: [:], + gitHash: "75238191", + server: nil, + version: "2024.4.0" + ) + )) + + _ = try await subject.auth(for: "1") + + let client = try XCTUnwrap(clientBuilder.clients.first) + XCTAssertEqual( + client.clientPlatform.featureFlags, + ["enableCipherKeyEncryption": false] + ) + } + /// `client(for:)` loading flags throws. @MainActor func test_client_loadFlagsThrows() async throws { @@ -195,7 +223,7 @@ final class ClientServiceTests: BitwardenTestCase { // swiftlint:disable:this ty date: Date(year: 2024, month: 2, day: 14, hour: 7, minute: 50, second: 0), responseModel: ConfigResponseModel( environment: nil, - featureStates: [:], + featureStates: ["cipher-key-encryption": .bool(true)], gitHash: "75238191", server: nil, version: "2024.4.0" @@ -213,9 +241,10 @@ final class ClientServiceTests: BitwardenTestCase { // swiftlint:disable:this ty } } - /// `configPublisher` loads flags into the SDK on a already created client. + /// `configPublisher` loads flags into the SDK on a already created client taking into account + /// changing the cipher-key-encryption feature flag. @MainActor - func test_configPublisher_loadFlagsOverride() async throws { + func test_configPublisher_loadFlagsOverride() async throws { // swiftlint:disable:this function_body_length configService.configMocker.withResult(ServerConfig( date: Date(year: 2024, month: 2, day: 14, hour: 7, minute: 50, second: 0), responseModel: ConfigResponseModel( @@ -256,6 +285,30 @@ final class ClientServiceTests: BitwardenTestCase { // swiftlint:disable:this ty return client?.clientPlatform.featureFlags == ["enableCipherKeyEncryption": true] } XCTAssertEqual(clientBuilder.clients.count, 1) + + configService.featureFlagsBool[.cipherKeyEncryption] = false + configService.configSubject.send( + MetaServerConfig( + isPreAuth: false, + userId: "1", + serverConfig: ServerConfig( + date: Date(year: 2024, month: 2, day: 14, hour: 7, minute: 50, second: 0), + responseModel: ConfigResponseModel( + environment: nil, + featureStates: [:], + gitHash: "75238191", + server: nil, + version: "2024.4.0" + ) + ) + ) + ) + + try await waitForAsync { + let client = try? XCTUnwrap(self.clientBuilder.clients.first) + return client?.clientPlatform.featureFlags == ["enableCipherKeyEncryption": false] + } + XCTAssertEqual(clientBuilder.clients.count, 1) } /// `configPublisher` does not load flags into the SDK when the config sent is pre authentication. diff --git a/BitwardenShared/Core/Platform/Services/ConfigService.swift b/BitwardenShared/Core/Platform/Services/ConfigService.swift index a3a26eb66..c9785bbc9 100644 --- a/BitwardenShared/Core/Platform/Services/ConfigService.swift +++ b/BitwardenShared/Core/Platform/Services/ConfigService.swift @@ -228,12 +228,16 @@ class DefaultConfigService: ConfigService { ?? defaultValue } + // MARK: Debug Feature Flags + func getDebugFeatureFlags() async -> [DebugMenuFeatureFlag] { let remoteFeatureFlags = await getConfig()?.featureStates ?? [:] let flags = FeatureFlag.debugMenuFeatureFlags.map { feature in let userDefaultValue = appSettingsStore.debugFeatureFlag(name: feature.rawValue) - let remoteFlagValue = remoteFeatureFlags[feature]?.boolValue ?? false + let remoteFlagValue = remoteFeatureFlags[feature]?.boolValue + ?? FeatureFlag.initialValues[feature]?.boolValue + ?? false return DebugMenuFeatureFlag( feature: feature, diff --git a/BitwardenShared/Core/Platform/Services/EnvironmentServiceTests.swift b/BitwardenShared/Core/Platform/Services/EnvironmentServiceTests.swift index 07e4a8e3b..14b743759 100644 --- a/BitwardenShared/Core/Platform/Services/EnvironmentServiceTests.swift +++ b/BitwardenShared/Core/Platform/Services/EnvironmentServiceTests.swift @@ -216,8 +216,12 @@ class EnvironmentServiceTests: XCTestCase { XCTAssertEqual(subject.region, .selfHosted) XCTAssertEqual(subject.sendShareURL, URL(string: "https://example.com/#/send")) XCTAssertEqual(subject.settingsURL, URL(string: "https://example.com/#/settings")) - // swiftlint:disable:next line_length - XCTAssertEqual(subject.setUpTwoFactorURL, URL(string: "https://example.com/#/settings/security/two-factor")) + XCTAssertEqual( + subject.setUpTwoFactorURL, + URL( + string: "https://example.com/#/settings/security/two-factor" + ) + ) XCTAssertEqual(subject.webVaultURL, URL(string: "https://example.com")) XCTAssertEqual(stateService.preAuthEnvironmentUrls, urls) @@ -242,7 +246,6 @@ class EnvironmentServiceTests: XCTestCase { XCTAssertEqual(subject.region, .selfHosted) XCTAssertEqual(subject.sendShareURL, URL(string: "https://example.com/#/send")) XCTAssertEqual(subject.settingsURL, URL(string: "https://example.com/#/settings")) - // swiftlint:disable:next line_length XCTAssertEqual(subject.setUpTwoFactorURL, URL(string: "https://example.com/#/settings/security/two-factor")) XCTAssertEqual(subject.webVaultURL, URL(string: "https://example.com")) XCTAssertEqual(stateService.preAuthEnvironmentUrls, urls) diff --git a/BitwardenShared/UI/Tools/Generator/Generator/GeneratorState+UsernameState.swift b/BitwardenShared/UI/Tools/Generator/Generator/GeneratorState+UsernameState.swift index c3144cf2b..caca9b2a5 100644 --- a/BitwardenShared/UI/Tools/Generator/Generator/GeneratorState+UsernameState.swift +++ b/BitwardenShared/UI/Tools/Generator/Generator/GeneratorState+UsernameState.swift @@ -216,7 +216,7 @@ extension GeneratorState.UsernameState { /// Returns a `UsernameGeneratorRequest` used to generate a forwarded email alias username. /// private func forwardedEmailGeneratorRequest() -> UsernameGeneratorRequest { - let service: ForwarderServiceType = switch forwardedEmailService { + let service = switch forwardedEmailService { case .addyIO: ForwarderServiceType.addyIo( apiToken: addyIOAPIAccessToken, diff --git a/BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor+TotpTests.swift b/BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor+TotpTests.swift index ddb1d1396..1a8abdacb 100644 --- a/BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor+TotpTests.swift +++ b/BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor+TotpTests.swift @@ -402,4 +402,4 @@ class VaultAutofillListProcessorTotpTests: BitwardenTestCase { // swiftlint:disa waitFor(errorReporter.errors.last as? BitwardenTestError == BitwardenTestError.example) } -} +} // swiftlint:disable:this file_length diff --git a/BitwardenShared/UI/Vault/VaultItem/AuthenticatorKeyCapture/AuthenticatorKeyCaptureCoordinator.swift b/BitwardenShared/UI/Vault/VaultItem/AuthenticatorKeyCapture/AuthenticatorKeyCaptureCoordinator.swift index a9a2269f9..d09102443 100644 --- a/BitwardenShared/UI/Vault/VaultItem/AuthenticatorKeyCapture/AuthenticatorKeyCaptureCoordinator.swift +++ b/BitwardenShared/UI/Vault/VaultItem/AuthenticatorKeyCapture/AuthenticatorKeyCaptureCoordinator.swift @@ -175,7 +175,7 @@ final class AuthenticatorKeyCaptureCoordinator: Coordinator, HasStackNavigator { /// private func showManualTotp() { let deviceSupportsCamera = services.cameraService.deviceSupportsCamera() && - appExtensionDelegate?.isInAppExtension != true // Extensions don't allow camera access. + appExtensionDelegate?.isInAppExtension != true // Extensions don't allow camera access. let processor = ManualEntryProcessor( coordinator: asAnyCoordinator(), services: services,