From 84cd01165cda81af088885f261268ed9a45c28a2 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Wed, 10 Apr 2024 08:59:20 -0500 Subject: [PATCH] Auth `UserKeyDefinition` Migration (#8587) * Migrate DeviceTrustCryptoService * Migrate SsoLoginService * Migrate TokenService * Update libs/common/src/auth/services/token.state.ts Co-authored-by: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> * Fix Test * Actually Fix Tests --------- Co-authored-by: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> --- ...ice-trust-crypto.service.implementation.ts | 8 ++-- .../src/auth/services/sso-login.service.ts | 19 ++++++-- .../common/src/auth/services/token.service.ts | 4 +- .../src/auth/services/token.state.spec.ts | 11 +++-- libs/common/src/auth/services/token.state.ts | 43 +++++++++++++------ 5 files changed, 61 insertions(+), 24 deletions(-) diff --git a/libs/common/src/auth/services/device-trust-crypto.service.implementation.ts b/libs/common/src/auth/services/device-trust-crypto.service.implementation.ts index e65c5cd499..6fb58eab28 100644 --- a/libs/common/src/auth/services/device-trust-crypto.service.implementation.ts +++ b/libs/common/src/auth/services/device-trust-crypto.service.implementation.ts @@ -14,7 +14,7 @@ import { StorageLocation } from "../../platform/enums"; import { EncString } from "../../platform/models/domain/enc-string"; import { StorageOptions } from "../../platform/models/domain/storage-options"; import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypto-key"; -import { DEVICE_TRUST_DISK_LOCAL, KeyDefinition, StateProvider } from "../../platform/state"; +import { DEVICE_TRUST_DISK_LOCAL, StateProvider, UserKeyDefinition } from "../../platform/state"; import { UserId } from "../../types/guid"; import { UserKey, DeviceKey } from "../../types/key"; import { DeviceTrustCryptoServiceAbstraction } from "../abstractions/device-trust-crypto.service.abstraction"; @@ -27,16 +27,18 @@ import { } from "../models/request/update-devices-trust.request"; /** Uses disk storage so that the device key can persist after log out and tab removal. */ -export const DEVICE_KEY = new KeyDefinition(DEVICE_TRUST_DISK_LOCAL, "deviceKey", { +export const DEVICE_KEY = new UserKeyDefinition(DEVICE_TRUST_DISK_LOCAL, "deviceKey", { deserializer: (deviceKey) => SymmetricCryptoKey.fromJSON(deviceKey) as DeviceKey, + clearOn: [], // Device key is needed to log back into device, so we can't clear it automatically during lock or logout }); /** Uses disk storage so that the shouldTrustDevice bool can persist across login. */ -export const SHOULD_TRUST_DEVICE = new KeyDefinition( +export const SHOULD_TRUST_DEVICE = new UserKeyDefinition( DEVICE_TRUST_DISK_LOCAL, "shouldTrustDevice", { deserializer: (shouldTrustDevice) => shouldTrustDevice, + clearOn: [], // Need to preserve the user setting, so we can't clear it automatically during lock or logout }, ); diff --git a/libs/common/src/auth/services/sso-login.service.ts b/libs/common/src/auth/services/sso-login.service.ts index 99640e1c6c..3df6ef3540 100644 --- a/libs/common/src/auth/services/sso-login.service.ts +++ b/libs/common/src/auth/services/sso-login.service.ts @@ -6,6 +6,7 @@ import { KeyDefinition, SSO_DISK, StateProvider, + UserKeyDefinition, } from "../../platform/state"; import { SsoLoginServiceAbstraction } from "../abstractions/sso-login.service.abstraction"; @@ -26,7 +27,19 @@ const SSO_STATE = new KeyDefinition(SSO_DISK, "ssoState", { /** * Uses disk storage so that the organization sso identifier can be persisted across sso redirects. */ -const ORGANIZATION_SSO_IDENTIFIER = new KeyDefinition( +const USER_ORGANIZATION_SSO_IDENTIFIER = new UserKeyDefinition( + SSO_DISK, + "organizationSsoIdentifier", + { + deserializer: (organizationIdentifier) => organizationIdentifier, + clearOn: ["logout"], // Used for login, so not needed past logout + }, +); + +/** + * Uses disk storage so that the organization sso identifier can be persisted across sso redirects. + */ +const GLOBAL_ORGANIZATION_SSO_IDENTIFIER = new KeyDefinition( SSO_DISK, "organizationSsoIdentifier", { @@ -51,10 +64,10 @@ export class SsoLoginService implements SsoLoginServiceAbstraction { constructor(private stateProvider: StateProvider) { this.codeVerifierState = this.stateProvider.getGlobal(CODE_VERIFIER); this.ssoState = this.stateProvider.getGlobal(SSO_STATE); - this.orgSsoIdentifierState = this.stateProvider.getGlobal(ORGANIZATION_SSO_IDENTIFIER); + this.orgSsoIdentifierState = this.stateProvider.getGlobal(GLOBAL_ORGANIZATION_SSO_IDENTIFIER); this.ssoEmailState = this.stateProvider.getGlobal(SSO_EMAIL); this.activeUserOrgSsoIdentifierState = this.stateProvider.getActive( - ORGANIZATION_SSO_IDENTIFIER, + USER_ORGANIZATION_SSO_IDENTIFIER, ); } diff --git a/libs/common/src/auth/services/token.service.ts b/libs/common/src/auth/services/token.service.ts index fb13c21870..db39997663 100644 --- a/libs/common/src/auth/services/token.service.ts +++ b/libs/common/src/auth/services/token.service.ts @@ -15,8 +15,8 @@ import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypt import { GlobalState, GlobalStateProvider, - KeyDefinition, SingleUserStateProvider, + UserKeyDefinition, } from "../../platform/state"; import { UserId } from "../../types/guid"; import { TokenService as TokenServiceAbstraction } from "../abstractions/token.service"; @@ -863,7 +863,7 @@ export class TokenService implements TokenServiceAbstraction { private async getStateValueByUserIdAndKeyDef( userId: UserId, - storageLocation: KeyDefinition, + storageLocation: UserKeyDefinition, ): Promise { // read from single user state provider return await firstValueFrom(this.singleUserStateProvider.get(userId, storageLocation).state$); diff --git a/libs/common/src/auth/services/token.state.spec.ts b/libs/common/src/auth/services/token.state.spec.ts index 24eddc73f5..55f97b7e00 100644 --- a/libs/common/src/auth/services/token.state.spec.ts +++ b/libs/common/src/auth/services/token.state.spec.ts @@ -1,4 +1,4 @@ -import { KeyDefinition } from "../../platform/state"; +import { KeyDefinition, UserKeyDefinition } from "../../platform/state"; import { ACCESS_TOKEN_DISK, @@ -28,8 +28,8 @@ describe.each([ "deserializes state key definitions", ( keyDefinition: - | KeyDefinition - | KeyDefinition + | UserKeyDefinition + | UserKeyDefinition | KeyDefinition>, state: string | boolean | Record, ) => { @@ -50,7 +50,10 @@ describe.each([ return typeof value === "object" && value !== null && !Array.isArray(value); } - function testDeserialization(keyDefinition: KeyDefinition, state: T) { + function testDeserialization( + keyDefinition: KeyDefinition | UserKeyDefinition, + state: T, + ) { const deserialized = keyDefinition.deserializer(JSON.parse(JSON.stringify(state))); expect(deserialized).toEqual(state); } diff --git a/libs/common/src/auth/services/token.state.ts b/libs/common/src/auth/services/token.state.ts index 368f3c4ca2..a8c6878fbb 100644 --- a/libs/common/src/auth/services/token.state.ts +++ b/libs/common/src/auth/services/token.state.ts @@ -1,30 +1,41 @@ -import { KeyDefinition, TOKEN_DISK, TOKEN_DISK_LOCAL, TOKEN_MEMORY } from "../../platform/state"; +import { + KeyDefinition, + TOKEN_DISK, + TOKEN_DISK_LOCAL, + TOKEN_MEMORY, + UserKeyDefinition, +} from "../../platform/state"; // Note: all tokens / API key information must be cleared on logout. // because we are using secure storage, we must manually call to clean up our tokens. // See stateService.deAuthenticateAccount for where we call clearTokens(...) -export const ACCESS_TOKEN_DISK = new KeyDefinition(TOKEN_DISK, "accessToken", { +export const ACCESS_TOKEN_DISK = new UserKeyDefinition(TOKEN_DISK, "accessToken", { deserializer: (accessToken) => accessToken, + clearOn: [], // Manually handled }); -export const ACCESS_TOKEN_MEMORY = new KeyDefinition(TOKEN_MEMORY, "accessToken", { +export const ACCESS_TOKEN_MEMORY = new UserKeyDefinition(TOKEN_MEMORY, "accessToken", { deserializer: (accessToken) => accessToken, + clearOn: [], // Manually handled }); -export const REFRESH_TOKEN_DISK = new KeyDefinition(TOKEN_DISK, "refreshToken", { +export const REFRESH_TOKEN_DISK = new UserKeyDefinition(TOKEN_DISK, "refreshToken", { deserializer: (refreshToken) => refreshToken, + clearOn: [], // Manually handled }); -export const REFRESH_TOKEN_MEMORY = new KeyDefinition(TOKEN_MEMORY, "refreshToken", { +export const REFRESH_TOKEN_MEMORY = new UserKeyDefinition(TOKEN_MEMORY, "refreshToken", { deserializer: (refreshToken) => refreshToken, + clearOn: [], // Manually handled }); -export const REFRESH_TOKEN_MIGRATED_TO_SECURE_STORAGE = new KeyDefinition( +export const REFRESH_TOKEN_MIGRATED_TO_SECURE_STORAGE = new UserKeyDefinition( TOKEN_DISK, "refreshTokenMigratedToSecureStorage", { deserializer: (refreshTokenMigratedToSecureStorage) => refreshTokenMigratedToSecureStorage, + clearOn: [], // Don't clear on lock/logout so that we always check the correct place (secure storage) for the refresh token if it's been migrated }, ); @@ -36,26 +47,34 @@ export const EMAIL_TWO_FACTOR_TOKEN_RECORD_DISK_LOCAL = KeyDefinition.record(TOKEN_DISK, "apiKeyClientId", { +export const API_KEY_CLIENT_ID_DISK = new UserKeyDefinition(TOKEN_DISK, "apiKeyClientId", { deserializer: (apiKeyClientId) => apiKeyClientId, + clearOn: [], // Manually handled }); -export const API_KEY_CLIENT_ID_MEMORY = new KeyDefinition(TOKEN_MEMORY, "apiKeyClientId", { - deserializer: (apiKeyClientId) => apiKeyClientId, -}); +export const API_KEY_CLIENT_ID_MEMORY = new UserKeyDefinition( + TOKEN_MEMORY, + "apiKeyClientId", + { + deserializer: (apiKeyClientId) => apiKeyClientId, + clearOn: [], // Manually handled + }, +); -export const API_KEY_CLIENT_SECRET_DISK = new KeyDefinition( +export const API_KEY_CLIENT_SECRET_DISK = new UserKeyDefinition( TOKEN_DISK, "apiKeyClientSecret", { deserializer: (apiKeyClientSecret) => apiKeyClientSecret, + clearOn: [], // Manually handled }, ); -export const API_KEY_CLIENT_SECRET_MEMORY = new KeyDefinition( +export const API_KEY_CLIENT_SECRET_MEMORY = new UserKeyDefinition( TOKEN_MEMORY, "apiKeyClientSecret", { deserializer: (apiKeyClientSecret) => apiKeyClientSecret, + clearOn: [], // Manually handled }, );