From 5ed33259378bcdfe0f5b345d5f7c30e32ea57ba0 Mon Sep 17 00:00:00 2001 From: Matt Czech Date: Tue, 18 Aug 2026 16:33:33 -0500 Subject: [PATCH] [PM-40314] fix: Enforce Master Password policy on SSO/TDE vault unlock --- .../Auth/Repositories/AuthRepository.swift | 10 + .../Repositories/AuthRepositoryTests.swift | 55 +++++ .../API/Auth/Fixtures/APITestData+Auth.swift | 3 + ...TrustedDeviceWithMasterPasswordPolicy.json | 33 +++ .../Core/Auth/Services/AuthService.swift | 48 ++++ .../Core/Auth/Services/AuthServiceTests.swift | 205 ++++++++++++++++++ .../TestHelpers/MockAuthService.swift | 12 + 7 files changed, 366 insertions(+) create mode 100644 BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/IdentityTokenTrustedDeviceWithMasterPasswordPolicy.json diff --git a/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift b/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift index 7ff536f81e..0fbc343fba 100644 --- a/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift +++ b/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift @@ -1158,6 +1158,16 @@ extension DefaultAuthRepository: AuthRepository { ) try await stateService.setMasterPasswordHash(hashedPassword) await updateKdfToMinimumsIfNeeded(password: password) + + do { + try await authService.checkMasterPasswordPolicyAfterUnlock( + email: account.profile.email, + masterPassword: password, + ) + } catch { + // Don't let a policy-check failure block an unlock that already succeeded. + errorReporter.log(error: error) + } } func unlockVaultWithPIN(pin: String) async throws { diff --git a/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift b/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift index 7987112909..3f09557475 100644 --- a/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift +++ b/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift @@ -2866,6 +2866,61 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo XCTAssertTrue(errorReporter.errors.isEmpty) } + /// `unlockVaultWithPassword(password:)` checks the entered master password against any + /// active organization Master Password policy after a successful unlock. + func test_unlockVaultWithPassword_checksMasterPasswordPolicy() async throws { + let account = Account.fixture(profile: .fixture( + userDecryptionOptions: UserDecryptionOptions( + hasMasterPassword: true, + masterPasswordUnlock: .fixture(), + keyConnectorOption: nil, + trustedDeviceOption: nil, + ), + )) + stateService.activeAccount = account + stateService.accountEncryptionKeys = [ + "1": AccountEncryptionKeys( + cryptographicState: .fixtureV2(), + encryptedUserKey: "USER_KEY", + ), + ] + + try await subject.unlockVaultWithPassword(password: "password") + + XCTAssertEqual(authService.checkMasterPasswordPolicyAfterUnlockEmail, "user@bitwarden.com") + XCTAssertEqual(authService.checkMasterPasswordPolicyAfterUnlockMasterPassword, "password") + XCTAssertTrue(errorReporter.errors.isEmpty) + } + + /// `unlockVaultWithPassword(password:)` does not let a failure in the Master Password policy + /// check propagate out or block an unlock that already succeeded — it logs the error instead. + func test_unlockVaultWithPassword_masterPasswordPolicyCheckError_doesNotThrow() async throws { + let account = Account.fixture(profile: .fixture( + userDecryptionOptions: UserDecryptionOptions( + hasMasterPassword: true, + masterPasswordUnlock: .fixture(), + keyConnectorOption: nil, + trustedDeviceOption: nil, + ), + )) + stateService.activeAccount = account + stateService.accountEncryptionKeys = [ + "1": AccountEncryptionKeys( + cryptographicState: .fixtureV2(), + encryptedUserKey: "USER_KEY", + ), + ] + authService.checkMasterPasswordPolicyAfterUnlockResult = .failure(BitwardenTestError.example) + + await assertAsyncDoesNotThrow { + try await subject.unlockVaultWithPassword(password: "password") + } + + XCTAssertFalse(vaultTimeoutService.isLocked(userId: "1")) + XCTAssertEqual(stateService.masterPasswordHashes["1"], "hashed") + XCTAssertEqual(errorReporter.errors.last as? BitwardenTestError, .example) + } + /// `logout` throws an error with no accounts. func test_logout_noAccounts() async { stateService.accounts = [] diff --git a/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/APITestData+Auth.swift b/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/APITestData+Auth.swift index 61f8e5ec6b..5742f6bb80 100644 --- a/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/APITestData+Auth.swift +++ b/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/APITestData+Auth.swift @@ -25,6 +25,9 @@ public extension APITestData { ) static let identityTokenRefreshStubError = loadFromJsonBundle(resource: "IdentityTokenRefreshStubError") static let identityTokenTrustedDevice = loadFromJsonBundle(resource: "IdentityTokenTrustedDevice") + static let identityTokenTrustedDeviceWithMPPolicy = loadFromJsonBundle( + resource: "IdentityTokenTrustedDeviceWithMasterPasswordPolicy", + ) static let identityTokenTwoFactorError = loadFromJsonBundle(resource: "IdentityTokenTwoFactorFailure") static let preValidateSingleSignOn = loadFromJsonBundle(resource: "preValidateSingleSignOn") static let identityTokenNewDeviceError = loadFromJsonBundle(resource: "IdentityTokenNewDeviceError") diff --git a/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/IdentityTokenTrustedDeviceWithMasterPasswordPolicy.json b/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/IdentityTokenTrustedDeviceWithMasterPasswordPolicy.json new file mode 100644 index 0000000000..681d8dd54d --- /dev/null +++ b/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/IdentityTokenTrustedDeviceWithMasterPasswordPolicy.json @@ -0,0 +1,33 @@ +{ + "access_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE2OTY5MDg4NzksInN1YiI6IjEzNTEyNDY3LTljZmUtNDNiMC05NjlmLTA3NTM0MDg0NzY0YiIsIm5hbWUiOiJCaXR3YXJkZW4gVXNlciIsImVtYWlsIjoidXNlckBiaXR3YXJkZW4uY29tIiwiZW1haWxfdmVyaWZpZWQiOnRydWUsImlhdCI6MTUxNjIzOTAyMiwicHJlbWl1bSI6ZmFsc2UsImFtciI6WyJBcHBsaWNhdGlvbiJdfQ.KDqC8kUaOAgBiUY8eeLa0a4xYWN8GmheXTFXmataFwM", + "expires_in": 3600, + "token_type": "Bearer", + "refresh_token": "REFRESH_TOKEN", + "scope": "api offline_access", + "MasterPasswordPolicy": { + "MinComplexity": 1, + "MinLength": 22, + "RequireLower": true, + "RequireUpper": true, + "RequireNumbers": false, + "RequireSpecial": false, + "EnforceOnLogin": true, + "Object": "masterPasswordPolicy" + }, + "ForcePasswordReset": false, + "Kdf": 0, + "KdfIterations": 600000, + "KdfMemory": null, + "KdfParallelism": null, + "UserDecryptionOptions": { + "HasMasterPassword": true, + "TrustedDeviceOption": { + "EncryptedPrivateKey": "private-key", + "EncryptedUserKey": "user-key", + "HasAdminApproval": false, + "HasLoginApprovingDevice": true, + "HasManageResetPasswordPermission": false + }, + "Object": "userDecryptionOptions" + } +} diff --git a/BitwardenShared/Core/Auth/Services/AuthService.swift b/BitwardenShared/Core/Auth/Services/AuthService.swift index b3f106b231..85e5f197dc 100644 --- a/BitwardenShared/Core/Auth/Services/AuthService.swift +++ b/BitwardenShared/Core/Auth/Services/AuthService.swift @@ -133,6 +133,18 @@ protocol AuthService { /// func answerLoginRequest(_ request: LoginRequest, approve: Bool) async throws + /// Checks the supplied master password against any active organization Master Password + /// policy after the vault has been unlocked with it, setting `forcePasswordResetReason` if + /// the password does not satisfy the policy. Uses the policy captured during the most recent + /// SSO/TDE login for this email if one is cached, otherwise falls back to whatever policy + /// data is already available via `PolicyService`. + /// + /// - Parameters: + /// - email: The email of the account that was unlocked. + /// - masterPassword: The master password used to unlock the vault. + /// + func checkMasterPasswordPolicyAfterUnlock(email: String, masterPassword: String) async throws + /// Check the status of the pending login request for the unauthenticated user. /// func checkPendingLoginRequest(withId id: String) async throws -> LoginRequest @@ -355,6 +367,12 @@ class DefaultAuthService: AuthService { // swiftlint:disable:this type_body_leng /// the callback the `ASWebAuthenticationSession` is configured to match. private var singleSignOnCallbackUrl: String { callbackUrl(for: .singleSignOn) } + /// The master password policy captured from the identity token response during an SSO/TDE + /// login, cached (keyed by email) until the user unlocks the vault with their master + /// password. Only consumed when the email matches the account being unlocked, to avoid a + /// stale policy from one account leaking into a later unlock for a different account. + private var ssoMasterPasswordPolicy: (email: String, policy: MasterPasswordPolicyResponseModel?)? + /// The service used by the application to manage account state. private let stateService: StateService @@ -456,6 +474,19 @@ class DefaultAuthService: AuthService { // swiftlint:disable:this type_body_leng _ = try await authAPIService.answerLoginRequest(loginRequest.id, requestModel: requestModel) } + func checkMasterPasswordPolicyAfterUnlock(email: String, masterPassword: String) async throws { + let cached = ssoMasterPasswordPolicy + ssoMasterPasswordPolicy = nil + let policy = cached?.email == email ? cached?.policy : nil + + try await checkMasterPasswordPolicies( + isPreAuth: false, + masterPassword: masterPassword, + masterPasswordPolicy: policy, + username: email, + ) + } + func checkPendingLoginRequest(withId id: String) async throws -> LoginRequest { guard let loginWithDeviceData else { throw AuthError.missingLoginWithDeviceData } @@ -756,6 +787,12 @@ class DefaultAuthService: AuthService { // swiftlint:disable:this type_body_leng email: email, ) + // Cache the master password policy so it can be checked once the user unlocks the vault + // with their master password (covers both the direct master password unlock method and + // the TDE "approve with master password" flow, which is reached via the + // `AuthError.requireDecryptionOptions` throw below). + ssoMasterPasswordPolicy = (email, response.masterPasswordPolicy) + return try await unlockMethod(for: response) } @@ -779,6 +816,17 @@ class DefaultAuthService: AuthService { // swiftlint:disable:this type_body_leng // Get the identity token to log in to Bitwarden. let response = try await getIdentityTokenResponse(email: email, request: twoFactorRequest) + // Cache the master password policy so it can be checked once the user unlocks the vault + // with their master password. Only applies to SSO/TDE logins that also required two-factor + // authentication, where the policy from the first (pre-2FA) identity token response was + // never captured because that attempt threw `twoFactorRequired` before reaching the + // caching in `loginWithSingleSignOn`. Plain password logins already have their policy + // checked above via `preAuthForcePasswordResetReason`, so caching it again here would + // just cause it to be redundantly re-checked once the vault is unlocked. + if case .authorizationCode = twoFactorRequest.authenticationMethod { + ssoMasterPasswordPolicy = (email, response.masterPasswordPolicy) + } + // If it's assigned then we need to update the required reset password and remove the cache. if preAuthForcePasswordResetReason != nil { try await stateService.setForcePasswordResetReason(.weakMasterPasswordOnLogin) diff --git a/BitwardenShared/Core/Auth/Services/AuthServiceTests.swift b/BitwardenShared/Core/Auth/Services/AuthServiceTests.swift index 835d48b2b8..6ca2d5a7da 100644 --- a/BitwardenShared/Core/Auth/Services/AuthServiceTests.swift +++ b/BitwardenShared/Core/Auth/Services/AuthServiceTests.swift @@ -138,6 +138,64 @@ class AuthServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body_ XCTAssertEqual(subject.callbackUrlScheme, "bitwarden") } + /// `checkMasterPasswordPolicyAfterUnlock(email:masterPassword:)` falls back to + /// `PolicyService.getMasterPasswordPolicyOptions()` when there's no cached SSO/TDE policy for + /// the account (e.g. a non-SSO login, or an ordinary vault-timeout re-lock). + func test_checkMasterPasswordPolicyAfterUnlock_noCachedPolicy_fallsBackToPolicyService() async throws { + clientService.mockAuth.satisfiesPolicyReturnValue = false + policyService.getMasterPasswordPolicyOptionsResult = .success( + MasterPasswordPolicyOptions( + minComplexity: 2, + minLength: 8, + requireUpper: true, + requireLower: false, + requireNumbers: true, + requireSpecial: false, + enforceOnLogin: true, + ), + ) + stateService.activeAccount = .fixture() + + try await subject.checkMasterPasswordPolicyAfterUnlock( + email: "user@bitwarden.com", + masterPassword: "weak password", + ) + + XCTAssertEqual( + stateService.forcePasswordResetReason["1"], + .weakMasterPasswordOnLogin, + ) + } + + /// `checkMasterPasswordPolicyAfterUnlock(email:masterPassword:)` ignores (and clears) a + /// cached SSO/TDE policy that belongs to a different account, so a stale policy from one + /// account's login can never be misapplied to a later unlock for a different account. + func test_checkMasterPasswordPolicyAfterUnlock_cachedPolicyForDifferentAccount_isIgnored() async throws { + policyService.getMasterPasswordPolicyOptionsResult = .success(nil) + clientService.mockAuth.satisfiesPolicyReturnValue = false + client.result = .httpSuccess(testData: .identityTokenWithMasterPasswordPolicy) + + // Cache a policy for "user@bitwarden.com" via an SSO login. + _ = try await subject.loginWithSingleSignOn(code: "super_cool_secret_code", email: "user@bitwarden.com") + + // Consume it for a different account. + stateService.activeAccount = .fixture(profile: .fixture(email: "other@bitwarden.com", userId: "2")) + try await subject.checkMasterPasswordPolicyAfterUnlock( + email: "other@bitwarden.com", + masterPassword: "weak password", + ) + + XCTAssertNil(stateService.forcePasswordResetReason["2"]) + + // The cache should also have been cleared, even for the original account. + stateService.activeAccount = .fixture(profile: .fixture(userId: "13512467-9cfe-43b0-969f-07534084764b")) + try await subject.checkMasterPasswordPolicyAfterUnlock( + email: "user@bitwarden.com", + masterPassword: "weak password", + ) + XCTAssertNil(stateService.forcePasswordResetReason["13512467-9cfe-43b0-969f-07534084764b"]) + } + /// `checkPendingLoginRequest(withId:)` returns the result of the API request. func test_checkPendingLoginRequest() async throws { // First initiate the login with device flow so that the necessary data is cached. @@ -809,6 +867,60 @@ class AuthServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body_ assertGetConfig() } + /// `loginWithSingleSignOn(code:email:)` caches the master password policy from the identity + /// token response, and `checkMasterPasswordPolicyAfterUnlock(email:masterPassword:)` applies + /// it once the vault is unlocked with the master password. + func test_loginSingleSignOn_masterPasswordPolicy_appliesOnUnlock() async throws { + policyService.getMasterPasswordPolicyOptionsResult = .success(nil) + clientService.mockAuth.satisfiesPolicyReturnValue = false + client.result = .httpSuccess(testData: .identityTokenWithMasterPasswordPolicy) + + let unlockMethod = try await subject.loginWithSingleSignOn( + code: "super_cool_secret_code", + email: "user@bitwarden.com", + ) + guard case let .masterPassword(account) = unlockMethod else { + return XCTFail("Expected the masterPassword unlock method.") + } + stateService.activeAccount = account + + try await subject.checkMasterPasswordPolicyAfterUnlock( + email: "user@bitwarden.com", + masterPassword: "weak password", + ) + + XCTAssertEqual( + stateService.forcePasswordResetReason[account.profile.userId], + .weakMasterPasswordOnLogin, + ) + } + + /// `loginWithSingleSignOn(code:email:)` caches the master password policy from the identity + /// token response even when the response leads to the TDE `requireDecryptionOptions` flow, so + /// it can still be applied later if the user chooses to approve with their master password. + func test_loginSingleSignOn_masterPasswordPolicy_capturedForTrustedDeviceApproval() async throws { + policyService.getMasterPasswordPolicyOptionsResult = .success(nil) + clientService.mockAuth.satisfiesPolicyReturnValue = false + trustDeviceService.isDeviceTrustedResult = .success(false) + client.result = .httpSuccess(testData: .identityTokenTrustedDeviceWithMPPolicy) + + await assertAsyncThrows(error: AuthError.requireDecryptionOptions) { + _ = try await subject.loginWithSingleSignOn(code: "super_cool_secret_code", email: "user@bitwarden.com") + } + + // Simulate the user choosing "approve with master password" on the decryption options screen. + stateService.activeAccount = .fixture(profile: .fixture(userId: "13512467-9cfe-43b0-969f-07534084764b")) + try await subject.checkMasterPasswordPolicyAfterUnlock( + email: "user@bitwarden.com", + masterPassword: "weak password", + ) + + XCTAssertEqual( + stateService.forcePasswordResetReason["13512467-9cfe-43b0-969f-07534084764b"], + .weakMasterPasswordOnLogin, + ) + } + /// `loginWithTwoFactorCode(email:code:method:remember:)` uses the cached request but with two factor /// codes added in to authenticate. func test_loginWithTwoFactorCode() async throws { // swiftlint:disable:this function_body_length @@ -883,6 +995,99 @@ class AuthServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body_ assertGetConfig() } + /// `loginWithTwoFactorCode(email:code:method:remember:)` caches the master password policy + /// from the final identity token response for an SSO login that also required two-factor + /// authentication, since the pre-2FA `loginWithSingleSignOn` attempt threw before it could + /// cache anything. + func test_loginWithTwoFactorCode_ssoMasterPasswordPolicy_appliesOnUnlock() async throws { + policyService.getMasterPasswordPolicyOptionsResult = .success(nil) + clientService.mockAuth.satisfiesPolicyReturnValue = false + client.results = [ + .httpFailure( + statusCode: 400, + headers: [:], + data: APITestData.identityTokenTwoFactorError.data, + ), + .httpSuccess(testData: .identityTokenWithMasterPasswordPolicy), + ] + + await assertAsyncThrows( + error: IdentityTokenRequestError.twoFactorRequired(AuthMethodsData.fixture(), nil, "exampleToken"), + ) { + _ = try await subject.loginWithSingleSignOn(code: "super_cool_secret_code", email: "user@bitwarden.com") + } + + let unlockMethod = try await subject.loginWithTwoFactorCode( + email: "user@bitwarden.com", + code: "just_a_lil_code", + method: .email, + remember: false, + ) + guard case let .masterPassword(account) = unlockMethod else { + return XCTFail("Expected the masterPassword unlock method.") + } + stateService.activeAccount = account + + try await subject.checkMasterPasswordPolicyAfterUnlock( + email: "user@bitwarden.com", + masterPassword: "weak password", + ) + + XCTAssertEqual( + stateService.forcePasswordResetReason[account.profile.userId], + .weakMasterPasswordOnLogin, + ) + } + + /// `loginWithTwoFactorCode(email:code:method:remember:)` does not cache a master password + /// policy for a plain (non-SSO) password + two-factor login, since that flow already checks + /// the policy via `preAuthForcePasswordResetReason`. Caching it here too would cause it to be + /// redundantly re-checked once the vault is unlocked. + func test_loginWithTwoFactorCode_passwordLogin_doesNotCacheMasterPasswordPolicy() async throws { + // No local policy is available, so any policy applied would have to come from the + // (incorrectly) cached SSO policy. + policyService.getMasterPasswordPolicyOptionsResult = .success(nil) + clientService.mockAuth.satisfiesPolicyReturnValue = false + client.results = [ + .httpSuccess(testData: .preLoginSuccess), + .httpFailure( + statusCode: 400, + headers: [:], + data: APITestData.identityTokenTwoFactorError.data, + ), + .httpSuccess(testData: .identityTokenWithMasterPasswordPolicy), + ] + clientService.mockAuth.hashPasswordReturnValue = "hashed password" + + await assertAsyncThrows( + error: IdentityTokenRequestError.twoFactorRequired(AuthMethodsData.fixture(), nil, "exampleToken"), + ) { + try await subject.loginWithMasterPassword( + "Password1234!", + username: "user@bitwarden.com", + isNewAccount: false, + ) + } + + let unlockMethod = try await subject.loginWithTwoFactorCode( + email: "user@bitwarden.com", + code: "just_a_lil_code", + method: .email, + remember: false, + ) + guard case let .masterPassword(account) = unlockMethod else { + return XCTFail("Expected the masterPassword unlock method.") + } + stateService.activeAccount = account + + try await subject.checkMasterPasswordPolicyAfterUnlock( + email: "user@bitwarden.com", + masterPassword: "weak password", + ) + + XCTAssertNil(stateService.forcePasswordResetReason[account.profile.userId]) + } + /// `loginWithTwoFactorCode(email:code:method:remember:)` uses the cached request /// but with device verification code added in to authenticate. func test_loginWithNewDeviceVerificationCode() async throws { // swiftlint:disable:this function_body_length diff --git a/BitwardenShared/Core/Auth/Services/TestHelpers/MockAuthService.swift b/BitwardenShared/Core/Auth/Services/TestHelpers/MockAuthService.swift index 445a804f7b..300da9fd4e 100644 --- a/BitwardenShared/Core/Auth/Services/TestHelpers/MockAuthService.swift +++ b/BitwardenShared/Core/Auth/Services/TestHelpers/MockAuthService.swift @@ -12,6 +12,12 @@ class MockAuthService: AuthService { var callbackUrlScheme: String = "callback" + // swiftlint:disable identifier_name + var checkMasterPasswordPolicyAfterUnlockEmail: String? + var checkMasterPasswordPolicyAfterUnlockMasterPassword: String? + var checkMasterPasswordPolicyAfterUnlockResult: Result = .success(()) + // swiftlint:enable identifier_name + var checkPendingLoginRequestId: String? var checkPendingLoginRequestResult: Result = .success(.fixture()) @@ -75,6 +81,12 @@ class MockAuthService: AuthService { try answerLoginRequestResult.get() } + func checkMasterPasswordPolicyAfterUnlock(email: String, masterPassword: String) async throws { + checkMasterPasswordPolicyAfterUnlockEmail = email + checkMasterPasswordPolicyAfterUnlockMasterPassword = masterPassword + try checkMasterPasswordPolicyAfterUnlockResult.get() + } + func checkPendingLoginRequest(withId id: String) async throws -> BitwardenShared.LoginRequest { checkPendingLoginRequestId = id return try checkPendingLoginRequestResult.get()