From 182d5bf5ace604f93f17032b580efa399e3b862d Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Mon, 4 Sep 2023 22:07:14 -0400 Subject: [PATCH] [PM-3758] Handle user decryption options from pre-TDE server response (#6180) * Mapped pre-TDE server response to UserDecryptionOptions. * Updated logic on SsoLoginStrategy to match account. * Linting. * Adjusted tests. * Fixed tests. --- .../login-strategies/login.strategy.spec.ts | 11 +--- .../auth/login-strategies/login.strategy.ts | 4 +- .../sso-login.strategy.spec.ts | 56 ++++++++++++++++++- .../login-strategies/sso-login.strategy.ts | 24 +++++--- .../src/platform/models/domain/account.ts | 50 +++++++++++------ 5 files changed, 107 insertions(+), 38 deletions(-) diff --git a/libs/common/src/auth/login-strategies/login.strategy.spec.ts b/libs/common/src/auth/login-strategies/login.strategy.spec.ts index f7128b35df..735135f406 100644 --- a/libs/common/src/auth/login-strategies/login.strategy.spec.ts +++ b/libs/common/src/auth/login-strategies/login.strategy.spec.ts @@ -41,10 +41,7 @@ import { IdentityCaptchaResponse } from "../models/response/identity-captcha.res import { IdentityTokenResponse } from "../models/response/identity-token.response"; import { IdentityTwoFactorResponse } from "../models/response/identity-two-factor.response"; import { MasterPasswordPolicyResponse } from "../models/response/master-password-policy.response"; -import { - IUserDecryptionOptionsServerResponse, - UserDecryptionOptionsResponse, -} from "../models/response/user-decryption-options/user-decryption-options.response"; +import { IUserDecryptionOptionsServerResponse } from "../models/response/user-decryption-options/user-decryption-options.response"; import { PasswordLogInStrategy } from "./password-login.strategy"; @@ -65,10 +62,6 @@ const name = "NAME"; const defaultUserDecryptionOptionsServerResponse: IUserDecryptionOptionsServerResponse = { HasMasterPassword: true, }; -const userDecryptionOptions = new UserDecryptionOptionsResponse( - defaultUserDecryptionOptionsServerResponse -); -const acctDecryptionOptions = AccountDecryptionOptions.fromResponse(userDecryptionOptions); const decodedToken = { sub: userId, @@ -197,7 +190,7 @@ describe("LogInStrategy", () => { }, }, keys: new AccountKeys(), - decryptionOptions: acctDecryptionOptions, + decryptionOptions: AccountDecryptionOptions.fromResponse(idTokenResponse), }) ); expect(messagingService.send).toHaveBeenCalledWith("loggedIn"); diff --git a/libs/common/src/auth/login-strategies/login.strategy.ts b/libs/common/src/auth/login-strategies/login.strategy.ts index 7bc8358016..6e51f21501 100644 --- a/libs/common/src/auth/login-strategies/login.strategy.ts +++ b/libs/common/src/auth/login-strategies/login.strategy.ts @@ -143,9 +143,7 @@ export abstract class LogInStrategy { }, }, keys: accountKeys, - decryptionOptions: AccountDecryptionOptions.fromResponse( - tokenResponse.userDecryptionOptions - ), + decryptionOptions: AccountDecryptionOptions.fromResponse(tokenResponse), adminAuthRequest: adminAuthRequest?.toJSON(), }) ); diff --git a/libs/common/src/auth/login-strategies/sso-login.strategy.spec.ts b/libs/common/src/auth/login-strategies/sso-login.strategy.spec.ts index 099a3a02a2..f078a7b86b 100644 --- a/libs/common/src/auth/login-strategies/sso-login.strategy.spec.ts +++ b/libs/common/src/auth/login-strategies/sso-login.strategy.spec.ts @@ -266,7 +266,61 @@ describe("SsoLogInStrategy", () => { describe("Key Connector", () => { let tokenResponse: IdentityTokenResponse; beforeEach(() => { - tokenResponse = identityTokenResponseFactory(null, { HasMasterPassword: false }); + tokenResponse = identityTokenResponseFactory(null, { + HasMasterPassword: false, + KeyConnectorOption: { KeyConnectorUrl: keyConnectorUrl }, + }); + tokenResponse.keyConnectorUrl = keyConnectorUrl; + }); + + it("gets and sets the master key if Key Connector is enabled and the user doesn't have a master password", async () => { + const masterKey = new SymmetricCryptoKey( + new Uint8Array(64).buffer as CsprngArray + ) as MasterKey; + + apiService.postIdentityToken.mockResolvedValue(tokenResponse); + cryptoService.getMasterKey.mockResolvedValue(masterKey); + + await ssoLogInStrategy.logIn(credentials); + + expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl); + }); + + it("converts new SSO user with no master password to Key Connector on first login", async () => { + tokenResponse.key = null; + + apiService.postIdentityToken.mockResolvedValue(tokenResponse); + + await ssoLogInStrategy.logIn(credentials); + + expect(keyConnectorService.convertNewSsoUserToKeyConnector).toHaveBeenCalledWith( + tokenResponse, + ssoOrgId + ); + }); + + it("decrypts and sets the user key if Key Connector is enabled and the user doesn't have a master password", async () => { + const userKey = new SymmetricCryptoKey(new Uint8Array(64).buffer as CsprngArray) as UserKey; + const masterKey = new SymmetricCryptoKey( + new Uint8Array(64).buffer as CsprngArray + ) as MasterKey; + + apiService.postIdentityToken.mockResolvedValue(tokenResponse); + cryptoService.getMasterKey.mockResolvedValue(masterKey); + cryptoService.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); + + await ssoLogInStrategy.logIn(credentials); + + expect(cryptoService.decryptUserKeyWithMasterKey).toHaveBeenCalledWith(masterKey); + expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey); + }); + }); + + describe("Key Connector Pre-TDE", () => { + let tokenResponse: IdentityTokenResponse; + beforeEach(() => { + tokenResponse = identityTokenResponseFactory(); + tokenResponse.userDecryptionOptions = null; tokenResponse.keyConnectorUrl = keyConnectorUrl; }); diff --git a/libs/common/src/auth/login-strategies/sso-login.strategy.ts b/libs/common/src/auth/login-strategies/sso-login.strategy.ts index 3e9a7e33f3..09dbca72fe 100644 --- a/libs/common/src/auth/login-strategies/sso-login.strategy.ts +++ b/libs/common/src/auth/login-strategies/sso-login.strategy.ts @@ -101,16 +101,22 @@ export class SsoLogInStrategy extends LogInStrategy { private shouldSetMasterKeyFromKeyConnector(tokenResponse: IdentityTokenResponse): boolean { const userDecryptionOptions = tokenResponse?.userDecryptionOptions; - // If the user has a master password, this means that they need to migrate to Key Connector, so we won't set the key here. - // We default to false here because old server versions won't have hasMasterPassword and in that case we want to rely solely on the keyConnectorUrl. - // TODO: remove null default after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537) - const userHasMasterPassword = userDecryptionOptions?.hasMasterPassword ?? false; + if (userDecryptionOptions != null) { + const userHasMasterPassword = userDecryptionOptions.hasMasterPassword; + const userHasKeyConnectorUrl = + userDecryptionOptions.keyConnectorOption?.keyConnectorUrl != null; - const keyConnectorUrl = this.getKeyConnectorUrl(tokenResponse); - - // In order for us to set the master key from Key Connector, we need to have a Key Connector URL - // and the user must not have a master password. - return keyConnectorUrl != null && !userHasMasterPassword; + // In order for us to set the master key from Key Connector, we need to have a Key Connector URL + // and the user must not have a master password. + return userHasKeyConnectorUrl && !userHasMasterPassword; + } else { + // In pre-TDE versions of the server, the userDecryptionOptions will not be present. + // In this case, we can determine if the user has a master password and has a Key Connector URL by + // just checking the keyConnectorUrl property. This is because the server short-circuits on the response + // and will not pass back the URL in the response if the user has a master password. + // TODO: remove compatibility check after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537) + return tokenResponse.keyConnectorUrl != null; + } } private getKeyConnectorUrl(tokenResponse: IdentityTokenResponse): string { diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 95a5a89912..09dc6971dc 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -11,7 +11,7 @@ import { EnvironmentUrls } from "../../../auth/models/domain/environment-urls"; import { ForceResetPasswordReason } from "../../../auth/models/domain/force-reset-password-reason"; import { KeyConnectorUserDecryptionOption } from "../../../auth/models/domain/user-decryption-options/key-connector-user-decryption-option"; import { TrustedDeviceUserDecryptionOption } from "../../../auth/models/domain/user-decryption-options/trusted-device-user-decryption-option"; -import { UserDecryptionOptionsResponse } from "../../../auth/models/response/user-decryption-options/user-decryption-options.response"; +import { IdentityTokenResponse } from "../../../auth/models/response/identity-token.response"; import { KdfType, UriMatchType } from "../../../enums"; import { EventData } from "../../../models/data/event.data"; import { GeneratedPasswordHistory } from "../../../tools/generator/password"; @@ -311,28 +311,46 @@ export class AccountDecryptionOptions { // return this.keyConnectorOption !== null && this.keyConnectorOption !== undefined; // } - static fromResponse(response: UserDecryptionOptionsResponse): AccountDecryptionOptions { + static fromResponse(response: IdentityTokenResponse): AccountDecryptionOptions { if (response == null) { return null; } const accountDecryptionOptions = new AccountDecryptionOptions(); - accountDecryptionOptions.hasMasterPassword = response.hasMasterPassword; - if (response.trustedDeviceOption) { - accountDecryptionOptions.trustedDeviceOption = new TrustedDeviceUserDecryptionOption( - response.trustedDeviceOption.hasAdminApproval, - response.trustedDeviceOption.hasLoginApprovingDevice, - response.trustedDeviceOption.hasManageResetPasswordPermission - ); + if (response.userDecryptionOptions) { + // If the response has userDecryptionOptions, this means it's on a post-TDE server version and can interrogate + // the new decryption options. + const responseOptions = response.userDecryptionOptions; + accountDecryptionOptions.hasMasterPassword = responseOptions.hasMasterPassword; + + if (responseOptions.trustedDeviceOption) { + accountDecryptionOptions.trustedDeviceOption = new TrustedDeviceUserDecryptionOption( + responseOptions.trustedDeviceOption.hasAdminApproval, + responseOptions.trustedDeviceOption.hasLoginApprovingDevice, + responseOptions.trustedDeviceOption.hasManageResetPasswordPermission + ); + } + + if (responseOptions.keyConnectorOption) { + accountDecryptionOptions.keyConnectorOption = new KeyConnectorUserDecryptionOption( + responseOptions.keyConnectorOption.keyConnectorUrl + ); + } + } else { + // If the response does not have userDecryptionOptions, this means it's on a pre-TDE server version and so + // we must base our decryption options on the presence of the keyConnectorUrl. + // Note that the presence of keyConnectorUrl implies that the user does not have a master password, as in pre-TDE + // server versions, a master password short-circuited the addition of the keyConnectorUrl to the response. + // TODO: remove this check after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537) + const usingKeyConnector = response.keyConnectorUrl != null; + accountDecryptionOptions.hasMasterPassword = !usingKeyConnector; + if (usingKeyConnector) { + accountDecryptionOptions.keyConnectorOption = new KeyConnectorUserDecryptionOption( + response.keyConnectorUrl + ); + } } - - if (response.keyConnectorOption) { - accountDecryptionOptions.keyConnectorOption = new KeyConnectorUserDecryptionOption( - response.keyConnectorOption.keyConnectorUrl - ); - } - return accountDecryptionOptions; }