From d581f06b321d5217130cfdfca99684b2fdc0f896 Mon Sep 17 00:00:00 2001 From: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Date: Wed, 3 Dec 2025 17:23:17 -0500 Subject: [PATCH] refactor(IdentityTokenResponse): [Auth/PM-3537] Remove deprecated KeyConnectorUrl from of IdentityTokenResponse + misc TDE cleanup (#17593) * PM-3537 - Remove KeyConnectorUrl from IdentityTokenResponse and clean up other flagged behavior * PM-3537 - SSO Login Strategy tests - remove key connector url * PM-3537 - Update LoginStrategyService tests to pass --- .../login-strategies/login.strategy.spec.ts | 2 +- .../common/login-strategies/login.strategy.ts | 2 +- .../sso-login.strategy.spec.ts | 61 ------------------- .../login-strategies/sso-login.strategy.ts | 12 +--- .../models/domain/user-decryption-options.ts | 21 +++---- .../login-strategy.service.spec.ts | 4 ++ .../response/identity-token.response.ts | 3 +- 7 files changed, 15 insertions(+), 90 deletions(-) diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index ceb36a44633..82e1183a1d6 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -259,7 +259,7 @@ describe("LoginStrategy", () => { expect(userDecryptionOptionsService.setUserDecryptionOptionsById).toHaveBeenCalledWith( userId, - UserDecryptionOptions.fromResponse(idTokenResponse), + UserDecryptionOptions.fromIdentityTokenResponse(idTokenResponse), ); expect(masterPasswordService.mock.setMasterPasswordUnlockData).toHaveBeenCalledWith( new MasterPasswordUnlockData( diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index 2e3c41da900..08d5ae6246f 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -199,7 +199,7 @@ export abstract class LoginStrategy { // as the user decryption options help determine the available timeout actions. await this.userDecryptionOptionsService.setUserDecryptionOptionsById( userId, - UserDecryptionOptions.fromResponse(tokenResponse), + UserDecryptionOptions.fromIdentityTokenResponse(tokenResponse), ); if (tokenResponse.userDecryptionOptions?.masterPasswordUnlock != null) { diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts index acbb680ae6d..3dbce6500a8 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts @@ -503,67 +503,6 @@ describe("SsoLoginStrategy", () => { 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); - masterPasswordService.masterKeySubject.next(masterKey); - - await ssoLoginStrategy.logIn(credentials); - - expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl, userId); - }); - - it("converts new SSO user with no master password to Key Connector on first login", async () => { - tokenResponse.key = undefined; - tokenResponse.kdfConfig = new Argon2KdfConfig(10, 64, 4); - - apiService.postIdentityToken.mockResolvedValue(tokenResponse); - - await ssoLoginStrategy.logIn(credentials); - - expect(keyConnectorService.setNewSsoUserKeyConnectorConversionData).toHaveBeenCalledWith( - { - kdfConfig: new Argon2KdfConfig(10, 64, 4), - keyConnectorUrl: keyConnectorUrl, - organizationId: ssoOrgId, - }, - userId, - ); - }); - - 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); - masterPasswordService.masterKeySubject.next(masterKey); - masterPasswordService.mock.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); - - await ssoLoginStrategy.logIn(credentials); - - expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith( - masterKey, - userId, - undefined, - ); - expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, userId); - }); - }); - - describe("Key Connector Pre-TDE", () => { - let tokenResponse: IdentityTokenResponse; - beforeEach(() => { - tokenResponse = identityTokenResponseFactory(); - tokenResponse.userDecryptionOptions = null; - 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 () => { diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.ts index d806f6d733e..33802765aca 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -157,22 +157,12 @@ export class SsoLoginStrategy extends LoginStrategy { // 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 { - // TODO: remove tokenResponse.keyConnectorUrl reference after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537) const userDecryptionOptions = tokenResponse?.userDecryptionOptions; - return ( - tokenResponse.keyConnectorUrl ?? userDecryptionOptions?.keyConnectorOption?.keyConnectorUrl - ); + return userDecryptionOptions?.keyConnectorOption?.keyConnectorUrl; } // TODO: future passkey login strategy will need to support setting user key (decrypting via TDE or admin approval request) diff --git a/libs/auth/src/common/models/domain/user-decryption-options.ts b/libs/auth/src/common/models/domain/user-decryption-options.ts index 7653a3b77ff..44d8bff4d2c 100644 --- a/libs/auth/src/common/models/domain/user-decryption-options.ts +++ b/libs/auth/src/common/models/domain/user-decryption-options.ts @@ -112,10 +112,11 @@ export class UserDecryptionOptions { * @throws If the response is nullish, this method will throw an error. User decryption options * are required for client initialization. */ - // TODO: Change response type to `UserDecryptionOptionsResponse` after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537) - static fromResponse(response: IdentityTokenResponse): UserDecryptionOptions { + static fromIdentityTokenResponse(response: IdentityTokenResponse): UserDecryptionOptions { if (response == null) { - throw new Error("User Decryption Options are required for client initialization."); + throw new Error( + "User Decryption Options are required for client initialization. Response is nullish.", + ); } const decryptionOptions = new UserDecryptionOptions(); @@ -134,17 +135,9 @@ export class UserDecryptionOptions { responseOptions.keyConnectorOption, ); } 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; - decryptionOptions.hasMasterPassword = !usingKeyConnector; - if (usingKeyConnector) { - decryptionOptions.keyConnectorOption = new KeyConnectorUserDecryptionOption(); - decryptionOptions.keyConnectorOption.keyConnectorUrl = response.keyConnectorUrl; - } + throw new Error( + "User Decryption Options are required for client initialization. userDecryptionOptions is missing in response.", + ); } return decryptionOptions; } diff --git a/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts b/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts index 831692c160f..20251e339a5 100644 --- a/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts +++ b/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts @@ -10,6 +10,7 @@ import { TokenTwoFactorRequest } from "@bitwarden/common/auth/models/request/ide import { IdentityTokenResponse } from "@bitwarden/common/auth/models/response/identity-token.response"; import { IdentityTwoFactorResponse } from "@bitwarden/common/auth/models/response/identity-two-factor.response"; import { PreloginResponse } from "@bitwarden/common/auth/models/response/prelogin.response"; +import { UserDecryptionOptionsResponse } from "@bitwarden/common/auth/models/response/user-decryption-options/user-decryption-options.response"; import { TwoFactorService } from "@bitwarden/common/auth/two-factor"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; @@ -496,6 +497,7 @@ describe("LoginStrategyService", () => { refresh_token: "REFRESH_TOKEN", scope: "api offline_access", token_type: "Bearer", + userDecryptionOptions: new UserDecryptionOptionsResponse({ HasMasterPassword: true }), }), ); apiService.postPrelogin.mockResolvedValue( @@ -563,6 +565,7 @@ describe("LoginStrategyService", () => { refresh_token: "REFRESH_TOKEN", scope: "api offline_access", token_type: "Bearer", + userDecryptionOptions: new UserDecryptionOptionsResponse({ HasMasterPassword: true }), }), ); @@ -692,6 +695,7 @@ describe("LoginStrategyService", () => { refresh_token: "REFRESH_TOKEN", scope: "api offline_access", token_type: "Bearer", + userDecryptionOptions: new UserDecryptionOptionsResponse({ HasMasterPassword: true }), }), ); diff --git a/libs/common/src/auth/models/response/identity-token.response.ts b/libs/common/src/auth/models/response/identity-token.response.ts index dab96f6cf8c..59e7eb98ec2 100644 --- a/libs/common/src/auth/models/response/identity-token.response.ts +++ b/libs/common/src/auth/models/response/identity-token.response.ts @@ -26,7 +26,6 @@ export class IdentityTokenResponse extends BaseResponse { forcePasswordReset: boolean; masterPasswordPolicy: MasterPasswordPolicyResponse; apiUseKeyConnector: boolean; - keyConnectorUrl: string; userDecryptionOptions?: UserDecryptionOptionsResponse; @@ -70,7 +69,7 @@ export class IdentityTokenResponse extends BaseResponse { : new Argon2KdfConfig(kdfIterations, kdfMemory, kdfParallelism); this.forcePasswordReset = this.getResponseProperty("ForcePasswordReset"); this.apiUseKeyConnector = this.getResponseProperty("ApiUseKeyConnector"); - this.keyConnectorUrl = this.getResponseProperty("KeyConnectorUrl"); + this.masterPasswordPolicy = new MasterPasswordPolicyResponse( this.getResponseProperty("MasterPasswordPolicy"), );