From 69ed6ce1f5ed42eaecb38cfc46bb9e9cf6482963 Mon Sep 17 00:00:00 2001 From: Jake Fink Date: Fri, 3 May 2024 11:54:29 -0400 Subject: [PATCH 1/5] [PM-6727] Part 1: pass userId in login strategies (#9030) * add validation to initAccount * pass userId to setMasterKey * fix key connector tests --- .../auth-request-login.strategy.ts | 8 ++- .../login-strategies/login.strategy.spec.ts | 1 + .../common/login-strategies/login.strategy.ts | 5 +- .../password-login.strategy.spec.ts | 7 ++- .../password-login.strategy.ts | 53 ++++++++++--------- .../sso-login.strategy.spec.ts | 11 ++-- .../login-strategies/sso-login.strategy.ts | 18 +++---- .../user-api-login.strategy.spec.ts | 4 +- .../user-api-login.strategy.ts | 8 +-- .../webauthn-login.strategy.spec.ts | 7 ++- .../webauthn-login.strategy.ts | 6 +-- .../abstractions/key-connector.service.ts | 4 +- .../src/auth/models/domain/auth-result.ts | 2 + .../services/key-connector.service.spec.ts | 4 +- .../auth/services/key-connector.service.ts | 15 +++--- .../src/platform/services/crypto.service.ts | 13 +++++ 16 files changed, 101 insertions(+), 65 deletions(-) diff --git a/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts b/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts index a66d987984..c8acc6c24b 100644 --- a/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts @@ -117,13 +117,12 @@ export class AuthRequestLoginStrategy extends LoginStrategy { return super.logInTwoFactor(twoFactor); } - protected override async setMasterKey(response: IdentityTokenResponse) { + protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) { const authRequestCredentials = this.cache.value.authRequestCredentials; if ( authRequestCredentials.decryptedMasterKey && authRequestCredentials.decryptedMasterKeyHash ) { - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; await this.masterPasswordService.setMasterKey( authRequestCredentials.decryptedMasterKey, userId, @@ -147,15 +146,14 @@ export class AuthRequestLoginStrategy extends LoginStrategy { if (authRequestCredentials.decryptedUserKey) { await this.cryptoService.setUserKey(authRequestCredentials.decryptedUserKey); } else { - await this.trySetUserKeyWithMasterKey(); + await this.trySetUserKeyWithMasterKey(userId); // Establish trust if required after setting user key await this.deviceTrustService.trustDeviceIfRequired(userId); } } - private async trySetUserKeyWithMasterKey(): Promise { - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; + private async trySetUserKeyWithMasterKey(userId: UserId): Promise { const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); if (masterKey) { const userKey = await this.cryptoService.decryptUserKeyWithMasterKey(masterKey); 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 c3a8f61d78..612222c10e 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -244,6 +244,7 @@ describe("LoginStrategy", () => { const result = await passwordLoginStrategy.logIn(credentials); expect(result).toEqual({ + userId: userId, forcePasswordReset: ForceSetPasswordReason.AdminForcePasswordReset, resetMasterPassword: true, twoFactorProviders: null, diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index 3a3109349e..96f7b73cab 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -241,6 +241,7 @@ export abstract class LoginStrategy { // Must come before setting keys, user key needs email to update additional keys const userId = await this.saveAccountInformation(response); + result.userId = userId; if (response.twoFactorToken != null) { // note: we can read email from access token b/c it was saved in saveAccountInformation @@ -249,7 +250,7 @@ export abstract class LoginStrategy { await this.tokenService.setTwoFactorToken(userEmail, response.twoFactorToken); } - await this.setMasterKey(response); + await this.setMasterKey(response, userId); await this.setUserKey(response, userId); await this.setPrivateKey(response); @@ -259,7 +260,7 @@ export abstract class LoginStrategy { } // The keys comes from different sources depending on the login strategy - protected abstract setMasterKey(response: IdentityTokenResponse): Promise; + protected abstract setMasterKey(response: IdentityTokenResponse, userId: UserId): Promise; protected abstract setUserKey(response: IdentityTokenResponse, userId: UserId): Promise; protected abstract setPrivateKey(response: IdentityTokenResponse): Promise; diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts index c97639f102..e85d01c5d7 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts @@ -173,8 +173,11 @@ describe("PasswordLoginStrategy", () => { localHashedPassword, userId, ); - expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(tokenResponse.key); - expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey); + expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( + tokenResponse.key, + userId, + ); + expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey, userId); expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey); }); diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.ts b/libs/auth/src/common/login-strategies/password-login.strategy.ts index d3ce8fa9e8..ec2a4850fe 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.ts @@ -147,6 +147,10 @@ export class PasswordLoginStrategy extends LoginStrategy { const [authResult, identityResponse] = await this.startLogIn(); + if (identityResponse instanceof IdentityCaptchaResponse) { + return authResult; + } + const masterPasswordPolicyOptions = this.getMasterPasswordPolicyOptionsFromResponse(identityResponse); @@ -157,23 +161,23 @@ export class PasswordLoginStrategy extends LoginStrategy { credentials, masterPasswordPolicyOptions, ); + if (meetsRequirements) { + return authResult; + } - if (!meetsRequirements) { - if (authResult.requiresCaptcha || authResult.requiresTwoFactor) { - // Save the flag to this strategy for later use as the master password is about to pass out of scope - this.cache.next({ - ...this.cache.value, - forcePasswordResetReason: ForceSetPasswordReason.WeakMasterPassword, - }); - } else { - // Authentication was successful, save the force update password options with the state service - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; - await this.masterPasswordService.setForceSetPasswordReason( - ForceSetPasswordReason.WeakMasterPassword, - userId, - ); - authResult.forcePasswordReset = ForceSetPasswordReason.WeakMasterPassword; - } + if (identityResponse instanceof IdentityTwoFactorResponse) { + // Save the flag to this strategy for use in 2fa login as the master password is about to pass out of scope + this.cache.next({ + ...this.cache.value, + forcePasswordResetReason: ForceSetPasswordReason.WeakMasterPassword, + }); + } else { + // Authentication was successful, save the force update password options with the state service + await this.masterPasswordService.setForceSetPasswordReason( + ForceSetPasswordReason.WeakMasterPassword, + authResult.userId, // userId is only available on successful login + ); + authResult.forcePasswordReset = ForceSetPasswordReason.WeakMasterPassword; } } return authResult; @@ -196,17 +200,18 @@ export class PasswordLoginStrategy extends LoginStrategy { !result.requiresCaptcha && forcePasswordResetReason != ForceSetPasswordReason.None ) { - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; - await this.masterPasswordService.setForceSetPasswordReason(forcePasswordResetReason, userId); + await this.masterPasswordService.setForceSetPasswordReason( + forcePasswordResetReason, + result.userId, + ); result.forcePasswordReset = forcePasswordResetReason; } return result; } - protected override async setMasterKey(response: IdentityTokenResponse) { + protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) { const { masterKey, localMasterKeyHash } = this.cache.value; - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; await this.masterPasswordService.setMasterKey(masterKey, userId); await this.masterPasswordService.setMasterKeyHash(localMasterKeyHash, userId); } @@ -219,12 +224,12 @@ export class PasswordLoginStrategy extends LoginStrategy { if (this.encryptionKeyMigrationRequired(response)) { return; } - await this.cryptoService.setMasterKeyEncryptedUserKey(response.key); + await this.cryptoService.setMasterKeyEncryptedUserKey(response.key, userId); const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); if (masterKey) { const userKey = await this.cryptoService.decryptUserKeyWithMasterKey(masterKey); - await this.cryptoService.setUserKey(userKey); + await this.cryptoService.setUserKey(userKey, userId); } } @@ -239,9 +244,9 @@ export class PasswordLoginStrategy extends LoginStrategy { } private getMasterPasswordPolicyOptionsFromResponse( - response: IdentityTokenResponse | IdentityTwoFactorResponse | IdentityCaptchaResponse, + response: IdentityTokenResponse | IdentityTwoFactorResponse, ): MasterPasswordPolicyOptions { - if (response == null || response instanceof IdentityCaptchaResponse) { + if (response == null) { return null; } return MasterPasswordPolicyOptions.fromResponse(response.masterPasswordPolicy); 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 416e910b47..1e4d867603 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 @@ -163,7 +163,10 @@ describe("SsoLoginStrategy", () => { // Assert expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledTimes(1); - expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(tokenResponse.key); + expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( + tokenResponse.key, + userId, + ); }); describe("Trusted Device Decryption", () => { @@ -417,7 +420,7 @@ describe("SsoLoginStrategy", () => { await ssoLoginStrategy.logIn(credentials); - expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl); + expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl, userId); }); it("converts new SSO user with no master password to Key Connector on first login", async () => { @@ -430,6 +433,7 @@ describe("SsoLoginStrategy", () => { expect(keyConnectorService.convertNewSsoUserToKeyConnector).toHaveBeenCalledWith( tokenResponse, ssoOrgId, + userId, ); }); @@ -468,7 +472,7 @@ describe("SsoLoginStrategy", () => { await ssoLoginStrategy.logIn(credentials); - expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl); + expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl, userId); }); it("converts new SSO user with no master password to Key Connector on first login", async () => { @@ -481,6 +485,7 @@ describe("SsoLoginStrategy", () => { expect(keyConnectorService.convertNewSsoUserToKeyConnector).toHaveBeenCalledWith( tokenResponse, ssoOrgId, + userId, ); }); 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 ad56d1ae51..37f616c98a 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -8,6 +8,7 @@ import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-con import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service"; +import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { SsoTokenRequest } from "@bitwarden/common/auth/models/request/identity-token/sso-token.request"; import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response"; @@ -124,7 +125,7 @@ export class SsoLoginStrategy extends LoginStrategy { this.ssoEmail2FaSessionToken$ = this.cache.pipe(map((state) => state.ssoEmail2FaSessionToken)); } - async logIn(credentials: SsoLoginCredentials) { + async logIn(credentials: SsoLoginCredentials): Promise { const data = new SsoLoginStrategyData(); data.orgId = credentials.orgId; @@ -147,10 +148,9 @@ export class SsoLoginStrategy extends LoginStrategy { // Auth guard currently handles redirects for this. if (ssoAuthResult.forcePasswordReset == ForceSetPasswordReason.AdminForcePasswordReset) { - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; await this.masterPasswordService.setForceSetPasswordReason( ssoAuthResult.forcePasswordReset, - userId, + ssoAuthResult.userId, ); } @@ -163,7 +163,7 @@ export class SsoLoginStrategy extends LoginStrategy { return ssoAuthResult; } - protected override async setMasterKey(tokenResponse: IdentityTokenResponse) { + protected override async setMasterKey(tokenResponse: IdentityTokenResponse, userId: UserId) { // The only way we can be setting a master key at this point is if we are using Key Connector. // First, check to make sure that we should do so based on the token response. if (this.shouldSetMasterKeyFromKeyConnector(tokenResponse)) { @@ -175,10 +175,11 @@ export class SsoLoginStrategy extends LoginStrategy { await this.keyConnectorService.convertNewSsoUserToKeyConnector( tokenResponse, this.cache.value.orgId, + userId, ); } else { const keyConnectorUrl = this.getKeyConnectorUrl(tokenResponse); - await this.keyConnectorService.setMasterKeyFromUrl(keyConnectorUrl); + await this.keyConnectorService.setMasterKeyFromUrl(keyConnectorUrl, userId); } } } @@ -231,7 +232,7 @@ export class SsoLoginStrategy extends LoginStrategy { if (masterKeyEncryptedUserKey) { // set the master key encrypted user key if it exists - await this.cryptoService.setMasterKeyEncryptedUserKey(masterKeyEncryptedUserKey); + await this.cryptoService.setMasterKeyEncryptedUserKey(masterKeyEncryptedUserKey, userId); } const userDecryptionOptions = tokenResponse?.userDecryptionOptions; @@ -251,7 +252,7 @@ export class SsoLoginStrategy extends LoginStrategy { this.getKeyConnectorUrl(tokenResponse) != null ) { // Key connector enabled for user - await this.trySetUserKeyWithMasterKey(); + await this.trySetUserKeyWithMasterKey(userId); } // Note: In the traditional SSO flow with MP without key connector, the lock component @@ -338,8 +339,7 @@ export class SsoLoginStrategy extends LoginStrategy { } } - private async trySetUserKeyWithMasterKey(): Promise { - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; + private async trySetUserKeyWithMasterKey(userId: UserId): Promise { const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); // There is a scenario in which the master key is not set here. That will occur if the user diff --git a/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts index 130e6c2d89..03dfa4f5fa 100644 --- a/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts @@ -174,7 +174,7 @@ describe("UserApiLoginStrategy", () => { await apiLogInStrategy.logIn(credentials); - expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl); + expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl, userId); }); it("decrypts and sets the user key if Key Connector is enabled", async () => { @@ -195,6 +195,6 @@ describe("UserApiLoginStrategy", () => { await apiLogInStrategy.logIn(credentials); expect(cryptoService.decryptUserKeyWithMasterKey).toHaveBeenCalledWith(masterKey); - expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey); + expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey, userId); }); }); diff --git a/libs/auth/src/common/login-strategies/user-api-login.strategy.ts b/libs/auth/src/common/login-strategies/user-api-login.strategy.ts index d7ee6fdc4b..b9d0c9e588 100644 --- a/libs/auth/src/common/login-strategies/user-api-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/user-api-login.strategy.ts @@ -93,11 +93,11 @@ export class UserApiLoginStrategy extends LoginStrategy { return authResult; } - protected override async setMasterKey(response: IdentityTokenResponse) { + protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) { if (response.apiUseKeyConnector) { const env = await firstValueFrom(this.environmentService.environment$); const keyConnectorUrl = env.getKeyConnectorUrl(); - await this.keyConnectorService.setMasterKeyFromUrl(keyConnectorUrl); + await this.keyConnectorService.setMasterKeyFromUrl(keyConnectorUrl, userId); } } @@ -108,11 +108,10 @@ export class UserApiLoginStrategy extends LoginStrategy { await this.cryptoService.setMasterKeyEncryptedUserKey(response.key); if (response.apiUseKeyConnector) { - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); if (masterKey) { const userKey = await this.cryptoService.decryptUserKeyWithMasterKey(masterKey); - await this.cryptoService.setUserKey(userKey); + await this.cryptoService.setUserKey(userKey, userId); } } } @@ -123,6 +122,7 @@ export class UserApiLoginStrategy extends LoginStrategy { ); } + // Overridden to save client ID and secret to token service protected async saveAccountInformation(tokenResponse: IdentityTokenResponse): Promise { const userId = await super.saveAccountInformation(tokenResponse); diff --git a/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts index bbcd3bafdd..9e977193dd 100644 --- a/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts @@ -208,7 +208,10 @@ describe("WebAuthnLoginStrategy", () => { // Assert // Master key encrypted user key should be set expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledTimes(1); - expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(idTokenResponse.key); + expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( + idTokenResponse.key, + userId, + ); expect(cryptoService.decryptToBytes).toHaveBeenCalledTimes(1); expect(cryptoService.decryptToBytes).toHaveBeenCalledWith( @@ -220,7 +223,7 @@ describe("WebAuthnLoginStrategy", () => { idTokenResponse.userDecryptionOptions.webAuthnPrfOption.encryptedUserKey.encryptedString, mockPrfPrivateKey, ); - expect(cryptoService.setUserKey).toHaveBeenCalledWith(mockUserKey); + expect(cryptoService.setUserKey).toHaveBeenCalledWith(mockUserKey, userId); expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(idTokenResponse.privateKey); // Master key and private key should not be set diff --git a/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts b/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts index ac487b3a82..714edabc1e 100644 --- a/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts @@ -98,7 +98,7 @@ export class WebAuthnLoginStrategy extends LoginStrategy { throw new Error("2FA not supported yet for WebAuthn Login."); } - protected override async setMasterKey() { + protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) { return Promise.resolve(); } @@ -107,7 +107,7 @@ export class WebAuthnLoginStrategy extends LoginStrategy { if (masterKeyEncryptedUserKey) { // set the master key encrypted user key if it exists - await this.cryptoService.setMasterKeyEncryptedUserKey(masterKeyEncryptedUserKey); + await this.cryptoService.setMasterKeyEncryptedUserKey(masterKeyEncryptedUserKey, userId); } const userDecryptionOptions = idTokenResponse?.userDecryptionOptions; @@ -134,7 +134,7 @@ export class WebAuthnLoginStrategy extends LoginStrategy { ); if (userKey) { - await this.cryptoService.setUserKey(new SymmetricCryptoKey(userKey) as UserKey); + await this.cryptoService.setUserKey(new SymmetricCryptoKey(userKey) as UserKey, userId); } } } diff --git a/libs/common/src/auth/abstractions/key-connector.service.ts b/libs/common/src/auth/abstractions/key-connector.service.ts index 36f413d70c..b1b6727cd1 100644 --- a/libs/common/src/auth/abstractions/key-connector.service.ts +++ b/libs/common/src/auth/abstractions/key-connector.service.ts @@ -1,8 +1,9 @@ import { Organization } from "../../admin-console/models/domain/organization"; +import { UserId } from "../../types/guid"; import { IdentityTokenResponse } from "../models/response/identity-token.response"; export abstract class KeyConnectorService { - setMasterKeyFromUrl: (url?: string) => Promise; + setMasterKeyFromUrl: (url: string, userId: UserId) => Promise; getManagingOrganization: () => Promise; getUsesKeyConnector: () => Promise; migrateUser: () => Promise; @@ -10,6 +11,7 @@ export abstract class KeyConnectorService { convertNewSsoUserToKeyConnector: ( tokenResponse: IdentityTokenResponse, orgId: string, + userId: UserId, ) => Promise; setUsesKeyConnector: (enabled: boolean) => Promise; setConvertAccountRequired: (status: boolean) => Promise; diff --git a/libs/common/src/auth/models/domain/auth-result.ts b/libs/common/src/auth/models/domain/auth-result.ts index f45466777e..bc828d3e86 100644 --- a/libs/common/src/auth/models/domain/auth-result.ts +++ b/libs/common/src/auth/models/domain/auth-result.ts @@ -1,9 +1,11 @@ import { Utils } from "../../../platform/misc/utils"; +import { UserId } from "../../../types/guid"; import { TwoFactorProviderType } from "../../enums/two-factor-provider-type"; import { ForceSetPasswordReason } from "./force-set-password-reason"; export class AuthResult { + userId: UserId; captchaSiteKey = ""; // TODO: PM-3287 - Remove this after 3 releases of backwards compatibility. - Target release 2023.12 for removal /** diff --git a/libs/common/src/auth/services/key-connector.service.spec.ts b/libs/common/src/auth/services/key-connector.service.spec.ts index e3e5fbdbe7..0fc0267a53 100644 --- a/libs/common/src/auth/services/key-connector.service.spec.ts +++ b/libs/common/src/auth/services/key-connector.service.spec.ts @@ -215,7 +215,7 @@ describe("KeyConnectorService", () => { const masterKey = new SymmetricCryptoKey(keyArr) as MasterKey; // Act - await keyConnectorService.setMasterKeyFromUrl(url); + await keyConnectorService.setMasterKeyFromUrl(url, mockUserId); // Assert expect(apiService.getMasterKeyFromKeyConnector).toHaveBeenCalledWith(url); @@ -235,7 +235,7 @@ describe("KeyConnectorService", () => { try { // Act - await keyConnectorService.setMasterKeyFromUrl(url); + await keyConnectorService.setMasterKeyFromUrl(url, mockUserId); } catch { // Assert expect(logService.error).toHaveBeenCalledWith(error); diff --git a/libs/common/src/auth/services/key-connector.service.ts b/libs/common/src/auth/services/key-connector.service.ts index c19185ae91..5065e58c76 100644 --- a/libs/common/src/auth/services/key-connector.service.ts +++ b/libs/common/src/auth/services/key-connector.service.ts @@ -16,6 +16,7 @@ import { StateProvider, UserKeyDefinition, } from "../../platform/state"; +import { UserId } from "../../types/guid"; import { MasterKey } from "../../types/key"; import { AccountService } from "../abstractions/account.service"; import { KeyConnectorService as KeyConnectorServiceAbstraction } from "../abstractions/key-connector.service"; @@ -100,12 +101,11 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { } // TODO: UserKey should be renamed to MasterKey and typed accordingly - async setMasterKeyFromUrl(url: string) { + async setMasterKeyFromUrl(url: string, userId: UserId) { try { const masterKeyResponse = await this.apiService.getMasterKeyFromKeyConnector(url); const keyArr = Utils.fromB64ToArray(masterKeyResponse.key); const masterKey = new SymmetricCryptoKey(keyArr) as MasterKey; - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; await this.masterPasswordService.setMasterKey(masterKey, userId); } catch (e) { this.handleKeyConnectorError(e); @@ -123,7 +123,11 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { ); } - async convertNewSsoUserToKeyConnector(tokenResponse: IdentityTokenResponse, orgId: string) { + async convertNewSsoUserToKeyConnector( + tokenResponse: IdentityTokenResponse, + orgId: string, + userId: UserId, + ) { // TODO: Remove after tokenResponse.keyConnectorUrl is deprecated in 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537) const { kdf, @@ -145,12 +149,11 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { kdfConfig, ); const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64); - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; await this.masterPasswordService.setMasterKey(masterKey, userId); const userKey = await this.cryptoService.makeUserKey(masterKey); - await this.cryptoService.setUserKey(userKey[0]); - await this.cryptoService.setMasterKeyEncryptedUserKey(userKey[1].encryptedString); + await this.cryptoService.setUserKey(userKey[0], userId); + await this.cryptoService.setMasterKeyEncryptedUserKey(userKey[1].encryptedString, userId); const [pubKey, privKey] = await this.cryptoService.makeKeyPair(); diff --git a/libs/common/src/platform/services/crypto.service.ts b/libs/common/src/platform/services/crypto.service.ts index 70f10a4b98..6a5ae71529 100644 --- a/libs/common/src/platform/services/crypto.service.ts +++ b/libs/common/src/platform/services/crypto.service.ts @@ -771,6 +771,19 @@ export class CryptoService implements CryptoServiceAbstraction { publicKey: string; privateKey: EncString; }> { + // Verify keys don't exist + const existingUserKey = await this.getUserKey(); + const existingPrivateKey = await this.getPrivateKey(); + if (existingUserKey != null || existingPrivateKey != null) { + if (existingUserKey != null) { + this.logService.error("Tried to initialize account with existing user key."); + } + if (existingPrivateKey != null) { + this.logService.error("Tried to initialize account with existing private key."); + } + throw new Error("Cannot initialize account, keys already exist."); + } + const userKey = (await this.keyGenerationService.createKey(512)) as UserKey; const [publicKey, privateKey] = await this.makeKeyPair(userKey); await this.setUserKey(userKey); From 4c860e12d7c3189ba132b74b9d807ef07214dde6 Mon Sep 17 00:00:00 2001 From: Jake Fink Date: Fri, 3 May 2024 12:23:12 -0400 Subject: [PATCH 2/5] fix init account validation (#9034) --- libs/common/src/platform/services/crypto.service.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/libs/common/src/platform/services/crypto.service.ts b/libs/common/src/platform/services/crypto.service.ts index 6a5ae71529..b90fab6b49 100644 --- a/libs/common/src/platform/services/crypto.service.ts +++ b/libs/common/src/platform/services/crypto.service.ts @@ -771,16 +771,10 @@ export class CryptoService implements CryptoServiceAbstraction { publicKey: string; privateKey: EncString; }> { - // Verify keys don't exist + // Verify user key doesn't exist const existingUserKey = await this.getUserKey(); - const existingPrivateKey = await this.getPrivateKey(); - if (existingUserKey != null || existingPrivateKey != null) { - if (existingUserKey != null) { - this.logService.error("Tried to initialize account with existing user key."); - } - if (existingPrivateKey != null) { - this.logService.error("Tried to initialize account with existing private key."); - } + if (existingUserKey != null) { + this.logService.error("Tried to initialize account with existing user key."); throw new Error("Cannot initialize account, keys already exist."); } From 0b02d2ee1c2a5ec33e3ec3c85f5298d6e4101d11 Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Fri, 3 May 2024 12:36:10 -0400 Subject: [PATCH 3/5] [AC-1970] Add billing navigation group to provider layout (#8941) * Add billing navigation item to provider layout with empty subscription page behind FF. * Fixing tests * Missed build error * Addison's feedback * Remove unused function * Missed one get$ conversion * Fixed background failure --- .../providers/clients/clients.component.ts | 25 +++----- .../providers/providers-layout.component.html | 27 +++++--- .../providers/providers-layout.component.ts | 63 ++++++++----------- .../providers/providers-routing.module.ts | 23 ++++++- .../providers/providers.module.ts | 2 + .../manage-client-organizations.component.ts | 31 ++++----- .../guards/has-consolidated-billing.guard.ts | 30 +++++++++ .../src/app/billing/providers/index.ts | 2 + .../provider-subscription.component.html | 1 + .../provider-subscription.component.ts | 7 +++ .../abstractions/provider.service.ts | 3 + .../services/provider.service.spec.ts | 19 ++++++ .../services/provider.service.ts | 8 ++- .../provider-billing.service.abstraction.ts | 25 ++++++++ 14 files changed, 181 insertions(+), 85 deletions(-) create mode 100644 bitwarden_license/bit-web/src/app/billing/providers/guards/has-consolidated-billing.guard.ts create mode 100644 bitwarden_license/bit-web/src/app/billing/providers/index.ts create mode 100644 bitwarden_license/bit-web/src/app/billing/providers/provider-subscription.component.html create mode 100644 bitwarden_license/bit-web/src/app/billing/providers/provider-subscription.component.ts create mode 100644 libs/common/src/billing/abstractions/provider-billing.service.abstraction.ts diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts index 6875c3816b..7a96bdc7c7 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts @@ -1,17 +1,17 @@ import { Component } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; -import { combineLatest, firstValueFrom, from } from "rxjs"; -import { concatMap, switchMap, takeUntil } from "rxjs/operators"; +import { firstValueFrom, from, map } from "rxjs"; +import { switchMap, takeUntil } from "rxjs/operators"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service"; -import { ProviderStatusType, ProviderUserType } from "@bitwarden/common/admin-console/enums"; +import { ProviderUserType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { canAccessBilling } from "@bitwarden/common/billing/abstractions/provider-billing.service.abstraction"; import { PlanType } from "@bitwarden/common/billing/enums"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; @@ -40,10 +40,6 @@ export class ClientsComponent extends BaseClientsComponent { manageOrganizations = false; showAddExisting = false; - protected consolidatedBillingEnabled$ = this.configService.getFeatureFlag$( - FeatureFlag.EnableConsolidatedBilling, - ); - constructor( private router: Router, private providerService: ProviderService, @@ -75,15 +71,10 @@ export class ClientsComponent extends BaseClientsComponent { .pipe( switchMap((params) => { this.providerId = params.providerId; - return combineLatest([ - this.providerService.get(this.providerId), - this.consolidatedBillingEnabled$, - ]).pipe( - concatMap(([provider, consolidatedBillingEnabled]) => { - if ( - consolidatedBillingEnabled && - provider.providerStatus === ProviderStatusType.Billable - ) { + return this.providerService.get$(this.providerId).pipe( + canAccessBilling(this.configService), + map((canAccessBilling) => { + if (canAccessBilling) { return from( this.router.navigate(["../manage-client-organizations"], { relativeTo: this.activatedRoute, diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.html b/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.html index 55efbe1386..a1cf2cc5aa 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.html +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.html @@ -1,5 +1,5 @@ -