Skip to content

Commit

Permalink
[PM-15554] Added cipher-key-encryption remote feature flag logic (#1205)
Browse files Browse the repository at this point in the history
  • Loading branch information
fedemkr authored Dec 23, 2024
1 parent fe4ba4b commit 6c6c3ed
Show file tree
Hide file tree
Showing 11 changed files with 102 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand Down Expand Up @@ -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"),
Expand All @@ -103,6 +107,7 @@ enum FeatureFlag: String, CaseIterable, Codable {
.testLocalInitialStringFlag:
false
case .appReviewPrompt,
.cipherKeyEncryption,
.cxpExportMobile,
.cxpImportMobile,
.emailVerification,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 9 additions & 3 deletions BitwardenShared/Core/Platform/Services/ClientService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
63 changes: 58 additions & 5 deletions BitwardenShared/Core/Platform/Services/ClientServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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 {
Expand Down Expand Up @@ -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"
Expand All @@ -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(
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion BitwardenShared/Core/Platform/Services/ConfigService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,4 +402,4 @@ class VaultAutofillListProcessorTotpTests: BitwardenTestCase { // swiftlint:disa

waitFor(errorReporter.errors.last as? BitwardenTestError == BitwardenTestError.example)
}
}
} // swiftlint:disable:this file_length
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 6c6c3ed

Please sign in to comment.