1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-24 08:33:29 +00:00

Merge branch 'main' into km/pm-27331

This commit is contained in:
Thomas Avery
2026-02-23 09:29:30 -06:00
committed by GitHub
647 changed files with 37702 additions and 10151 deletions

View File

@@ -15,11 +15,13 @@ import { MasterPasswordApiService } from "@bitwarden/common/auth/abstractions/ma
import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason";
import { SetPasswordRequest } from "@bitwarden/common/auth/models/request/set-password.request";
import { UpdateTdeOffboardingPasswordRequest } from "@bitwarden/common/auth/models/request/update-tde-offboarding-password.request";
import { assertNonNullish, assertTruthy } from "@bitwarden/common/auth/utils";
import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service";
import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction";
import {
MasterPasswordAuthenticationData,
MasterPasswordSalt,
MasterPasswordUnlockData,
} from "@bitwarden/common/key-management/master-password/types/master-password.types";
@@ -45,6 +47,7 @@ import {
SetInitialPasswordService,
SetInitialPasswordTdeOffboardingCredentials,
SetInitialPasswordUserType,
SetInitialPasswordTdeUserWithPermissionCredentials,
} from "./set-initial-password.service.abstraction";
export class DefaultSetInitialPasswordService implements SetInitialPasswordService {
@@ -212,7 +215,7 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi
await this.masterPasswordService.setMasterKeyHash(newLocalMasterKeyHash, userId);
if (resetPasswordAutoEnroll) {
await this.handleResetPasswordAutoEnroll(newServerMasterKeyHash, orgId, userId);
await this.handleResetPasswordAutoEnrollOld(newServerMasterKeyHash, orgId, userId);
}
}
@@ -336,6 +339,86 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi
);
}
async setInitialPasswordTdeUserWithPermission(
credentials: SetInitialPasswordTdeUserWithPermissionCredentials,
userId: UserId,
): Promise<void> {
const ctx =
"Could not set initial password for TDE user with Manage Account Recovery permission.";
assertTruthy(credentials.newPassword, "newPassword", ctx);
assertTruthy(credentials.salt, "salt", ctx);
assertNonNullish(credentials.kdfConfig, "kdfConfig", ctx);
assertNonNullish(credentials.newPasswordHint, "newPasswordHint", ctx); // can have an empty string as a valid value, so check non-nullish
assertTruthy(credentials.orgSsoIdentifier, "orgSsoIdentifier", ctx);
assertTruthy(credentials.orgId, "orgId", ctx);
assertNonNullish(credentials.resetPasswordAutoEnroll, "resetPasswordAutoEnroll", ctx); // can have `false` as a valid value, so check non-nullish
assertTruthy(userId, "userId", ctx);
const {
newPassword,
salt,
kdfConfig,
newPasswordHint,
orgSsoIdentifier,
orgId,
resetPasswordAutoEnroll,
} = credentials;
const userKey = await firstValueFrom(this.keyService.userKey$(userId));
if (!userKey) {
throw new Error("userKey not found.");
}
const authenticationData: MasterPasswordAuthenticationData =
await this.masterPasswordService.makeMasterPasswordAuthenticationData(
newPassword,
kdfConfig,
salt,
);
const unlockData: MasterPasswordUnlockData =
await this.masterPasswordService.makeMasterPasswordUnlockData(
newPassword,
kdfConfig,
salt,
userKey,
);
const request = SetPasswordRequest.newConstructor(
authenticationData,
unlockData,
newPasswordHint,
orgSsoIdentifier,
null, // no KeysRequest for TDE user because they already have a key pair
);
await this.masterPasswordApiService.setPassword(request);
// Clear force set password reason to allow navigation back to vault.
await this.masterPasswordService.setForceSetPasswordReason(ForceSetPasswordReason.None, userId);
// User now has a password so update decryption state
await this.masterPasswordService.setMasterPasswordUnlockData(unlockData, userId);
await this.updateLegacyState(
newPassword,
unlockData.kdf,
new EncString(unlockData.masterKeyWrappedUserKey),
userId,
unlockData,
);
if (resetPasswordAutoEnroll) {
await this.handleResetPasswordAutoEnroll(
authenticationData.masterPasswordAuthenticationHash,
orgId,
userId,
userKey,
);
}
}
/**
* @deprecated To be removed in PM-28143
*/
@@ -441,7 +524,19 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi
await this.masterPasswordService.setMasterPasswordUnlockData(masterPasswordUnlockData, userId);
}
private async handleResetPasswordAutoEnroll(
/**
* @deprecated To be removed in PM-28143
*
* This method is now deprecated because it is used with the deprecated `setInitialPassword()` method,
* which handles both JIT MP and TDE + Permission user flows.
*
* Since these methods can handle the JIT MP flow - which creates a new user key and sets it to state - we
* must retreive that user key here in this method.
*
* But the new handleResetPasswordAutoEnroll() method is only used in the TDE + Permission user case, in which
* case we already have the user key and can simply pass it through via method parameter ( @see handleResetPasswordAutoEnroll )
*/
private async handleResetPasswordAutoEnrollOld(
masterKeyHash: string,
orgId: string,
userId: UserId,
@@ -483,4 +578,43 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi
enrollmentRequest,
);
}
private async handleResetPasswordAutoEnroll(
masterKeyHash: string,
orgId: string,
userId: UserId,
userKey: UserKey,
) {
const organizationKeys = await this.organizationApiService.getKeys(orgId);
if (organizationKeys == null) {
throw new Error(
"Organization keys response is null. Could not handle reset password auto enroll.",
);
}
const orgPublicKey = Utils.fromB64ToArray(organizationKeys.publicKey);
// RSA encrypt user key with organization public key
const orgPublicKeyEncryptedUserKey = await this.encryptService.encapsulateKeyUnsigned(
userKey,
orgPublicKey,
);
if (orgPublicKeyEncryptedUserKey == null || !orgPublicKeyEncryptedUserKey.encryptedString) {
throw new Error(
"orgPublicKeyEncryptedUserKey not found. Could not handle reset password auto enroll.",
);
}
const enrollmentRequest = new OrganizationUserResetPasswordEnrollmentRequest();
enrollmentRequest.masterPasswordHash = masterKeyHash;
enrollmentRequest.resetPasswordKey = orgPublicKeyEncryptedUserKey.encryptedString;
await this.organizationUserApiService.putOrganizationUserResetPasswordEnrollment(
orgId,
userId,
enrollmentRequest,
);
}
}

View File

@@ -31,6 +31,9 @@ import {
} from "@bitwarden/common/key-management/crypto/models/enc-string";
import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction";
import {
MasterKeyWrappedUserKey,
MasterPasswordAuthenticationData,
MasterPasswordAuthenticationHash,
MasterPasswordSalt,
MasterPasswordUnlockData,
} from "@bitwarden/common/key-management/master-password/types/master-password.types";
@@ -62,6 +65,7 @@ import {
SetInitialPasswordCredentials,
SetInitialPasswordService,
SetInitialPasswordTdeOffboardingCredentials,
SetInitialPasswordTdeUserWithPermissionCredentials,
SetInitialPasswordUserType,
} from "./set-initial-password.service.abstraction";
@@ -237,7 +241,7 @@ describe("DefaultSetInitialPasswordService", () => {
}
}
// Mock handleResetPasswordAutoEnroll() values
// Mock handleResetPasswordAutoEnrollOld() values
if (config.resetPasswordAutoEnroll) {
organizationApiService.getKeys.mockResolvedValue(organizationKeys);
encryptService.encapsulateKeyUnsigned.mockResolvedValue(orgPublicKeyEncryptedUserKey);
@@ -1104,4 +1108,285 @@ describe("DefaultSetInitialPasswordService", () => {
await expect(promise).rejects.toThrow("Unexpected V2 account cryptographic state");
});
});
describe("setInitialPasswordTdeUserWithPermission()", () => {
// Mock method parameters
let credentials: SetInitialPasswordTdeUserWithPermissionCredentials;
// Mock method data
let authenticationData: MasterPasswordAuthenticationData;
let unlockData: MasterPasswordUnlockData;
let setPasswordRequest: SetPasswordRequest;
let userDecryptionOptions: UserDecryptionOptions;
beforeEach(() => {
// Mock method parameters
credentials = {
newPassword: "newPassword123!",
salt: "user@example.com" as MasterPasswordSalt,
kdfConfig: DEFAULT_KDF_CONFIG,
newPasswordHint: "newPasswordHint",
orgSsoIdentifier: "orgSsoIdentifier",
orgId: "orgId" as OrganizationId,
resetPasswordAutoEnroll: false,
};
// Mock method data
userKey = makeSymmetricCryptoKey(64) as UserKey;
keyService.userKey$.mockReturnValue(of(userKey));
authenticationData = {
salt: credentials.salt,
kdf: credentials.kdfConfig,
masterPasswordAuthenticationHash:
"masterPasswordAuthenticationHash" as MasterPasswordAuthenticationHash,
};
masterPasswordService.makeMasterPasswordAuthenticationData.mockResolvedValue(
authenticationData,
);
unlockData = {
salt: credentials.salt,
kdf: credentials.kdfConfig,
masterKeyWrappedUserKey: "masterKeyWrappedUserKey" as MasterKeyWrappedUserKey,
} as MasterPasswordUnlockData;
masterPasswordService.makeMasterPasswordUnlockData.mockResolvedValue(unlockData);
setPasswordRequest = SetPasswordRequest.newConstructor(
authenticationData,
unlockData,
credentials.newPasswordHint,
credentials.orgSsoIdentifier,
null, // no KeysRequest for TDE user because they already have a key pair
);
userDecryptionOptions = new UserDecryptionOptions({ hasMasterPassword: false });
userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue(
of(userDecryptionOptions),
);
});
describe("general error handling", () => {
["newPassword", "salt", "orgSsoIdentifier", "orgId"].forEach((key) => {
it(`should throw if ${key} is an empty string (falsy) on the SetInitialPasswordTdeUserWithPermissionCredentials object`, async () => {
// Arrange
const invalidCredentials: SetInitialPasswordTdeUserWithPermissionCredentials = {
...credentials,
[key]: "",
};
// Act
const promise = sut.setInitialPasswordTdeUserWithPermission(invalidCredentials, userId);
// Assert
await expect(promise).rejects.toThrow(
`${key} is falsy. Could not set initial password for TDE user with Manage Account Recovery permission.`,
);
});
});
["kdfConfig", "newPasswordHint", "resetPasswordAutoEnroll"].forEach((key) => {
it(`should throw if ${key} is null on the SetInitialPasswordTdeUserWithPermissionCredentials object`, async () => {
// Arrange
const invalidCredentials: SetInitialPasswordTdeUserWithPermissionCredentials = {
...credentials,
[key]: null,
};
// Act
const promise = sut.setInitialPasswordTdeUserWithPermission(invalidCredentials, userId);
// Assert
await expect(promise).rejects.toThrow(
`${key} is null or undefined. Could not set initial password for TDE user with Manage Account Recovery permission.`,
);
});
});
it("should throw if userId is not given", async () => {
// Arrange
userId = null;
// Act
const promise = sut.setInitialPasswordTdeUserWithPermission(credentials, userId);
// Assert
await expect(promise).rejects.toThrow(
"userId is falsy. Could not set initial password for TDE user with Manage Account Recovery permission.",
);
});
});
it("should throw if the userKey is not found", async () => {
// Arrange
keyService.userKey$.mockReturnValue(of(null));
// Act
const promise = sut.setInitialPasswordTdeUserWithPermission(credentials, userId);
// Assert
await expect(promise).rejects.toThrow("userKey not found.");
});
it("should call makeMasterPasswordAuthenticationData and makeMasterPasswordUnlockData with the correct parameters", async () => {
// Act
await sut.setInitialPasswordTdeUserWithPermission(credentials, userId);
// Assert
expect(masterPasswordService.makeMasterPasswordAuthenticationData).toHaveBeenCalledWith(
credentials.newPassword,
credentials.kdfConfig,
credentials.salt,
);
expect(masterPasswordService.makeMasterPasswordUnlockData).toHaveBeenCalledWith(
credentials.newPassword,
credentials.kdfConfig,
credentials.salt,
userKey,
);
});
it("should call the API method to set a master password", async () => {
// Act
await sut.setInitialPasswordTdeUserWithPermission(credentials, userId);
// Assert
expect(masterPasswordApiService.setPassword).toHaveBeenCalledTimes(1);
expect(masterPasswordApiService.setPassword).toHaveBeenCalledWith(setPasswordRequest);
});
describe("given the initial password has been successfully set", () => {
it("should clear the ForceSetPasswordReason by setting it to None", async () => {
// Act
await sut.setInitialPasswordTdeUserWithPermission(credentials, userId);
// Assert
expect(masterPasswordService.setForceSetPasswordReason).toHaveBeenCalledWith(
ForceSetPasswordReason.None,
userId,
);
});
it("should set MasterPasswordUnlockData to state", async () => {
// Act
await sut.setInitialPasswordTdeUserWithPermission(credentials, userId);
// Assert
expect(masterPasswordService.setMasterPasswordUnlockData).toHaveBeenCalledWith(
unlockData,
userId,
);
});
it("should update legacy state", async () => {
// Act
await sut.setInitialPasswordTdeUserWithPermission(credentials, userId);
// Assert
expect(userDecryptionOptionsService.setUserDecryptionOptionsById).toHaveBeenCalledWith(
userId,
expect.objectContaining({ hasMasterPassword: true }),
);
expect(kdfConfigService.setKdfConfig).toHaveBeenCalledWith(userId, credentials.kdfConfig);
expect(masterPasswordService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(
new EncString(unlockData.masterKeyWrappedUserKey),
userId,
);
expect(masterPasswordService.setLegacyMasterKeyFromUnlockData).toHaveBeenCalledWith(
credentials.newPassword,
unlockData,
userId,
);
});
describe("given resetPasswordAutoEnroll is false", () => {
it("should NOT handle reset password (account recovery) auto enroll", async () => {
// Act
await sut.setInitialPasswordTdeUserWithPermission(credentials, userId);
// Assert
expect(
organizationUserApiService.putOrganizationUserResetPasswordEnrollment,
).not.toHaveBeenCalled();
});
});
describe("given resetPasswordAutoEnroll is true", () => {
let organizationKeys: OrganizationKeysResponse;
let orgPublicKeyEncryptedUserKey: EncString;
let enrollmentRequest: OrganizationUserResetPasswordEnrollmentRequest;
beforeEach(() => {
credentials.resetPasswordAutoEnroll = true;
organizationKeys = {
privateKey: "orgPrivateKey",
publicKey: "orgPublicKey",
} as OrganizationKeysResponse;
organizationApiService.getKeys.mockResolvedValue(organizationKeys);
orgPublicKeyEncryptedUserKey = new EncString("orgPublicKeyEncryptedUserKey");
encryptService.encapsulateKeyUnsigned.mockResolvedValue(orgPublicKeyEncryptedUserKey);
enrollmentRequest = new OrganizationUserResetPasswordEnrollmentRequest();
enrollmentRequest.masterPasswordHash =
authenticationData.masterPasswordAuthenticationHash;
enrollmentRequest.resetPasswordKey = orgPublicKeyEncryptedUserKey.encryptedString;
});
it("should throw if organization keys are not found", async () => {
// Arrange
organizationApiService.getKeys.mockResolvedValue(null);
// Act
const promise = sut.setInitialPasswordTdeUserWithPermission(credentials, userId);
// Assert
await expect(promise).rejects.toThrow(
"Organization keys response is null. Could not handle reset password auto enroll.",
);
});
it("should throw if orgPublicKeyEncryptedUserKey is not found", async () => {
// Arrange
encryptService.encapsulateKeyUnsigned.mockResolvedValue(null);
// Act
const promise = sut.setInitialPasswordTdeUserWithPermission(credentials, userId);
// Assert
await expect(promise).rejects.toThrow(
"orgPublicKeyEncryptedUserKey not found. Could not handle reset password auto enroll.",
);
});
it("should throw if orgPublicKeyEncryptedUserKey.encryptedString is not found", async () => {
// Arrange
orgPublicKeyEncryptedUserKey.encryptedString = null;
// Act
const promise = sut.setInitialPasswordTdeUserWithPermission(credentials, userId);
// Assert
await expect(promise).rejects.toThrow(
"orgPublicKeyEncryptedUserKey not found. Could not handle reset password auto enroll.",
);
});
it("should call the API method to handle reset password (account recovery) auto enroll", async () => {
// Act
await sut.setInitialPasswordTdeUserWithPermission(credentials, userId);
// Assert
expect(
organizationUserApiService.putOrganizationUserResetPasswordEnrollment,
).toHaveBeenCalledTimes(1);
expect(
organizationUserApiService.putOrganizationUserResetPasswordEnrollment,
).toHaveBeenCalledWith(credentials.orgId, userId, enrollmentRequest);
});
});
});
});
});

View File

@@ -47,6 +47,7 @@ import {
SetInitialPasswordCredentials,
SetInitialPasswordService,
SetInitialPasswordTdeOffboardingCredentials,
SetInitialPasswordTdeUserWithPermissionCredentials,
SetInitialPasswordUserType,
} from "./set-initial-password.service.abstraction";
@@ -183,7 +184,13 @@ export class SetInitialPasswordComponent implements OnInit {
break;
}
case SetInitialPasswordUserType.TDE_ORG_USER_RESET_PASSWORD_PERMISSION_REQUIRES_MP:
if (passwordInputResult.newApisWithInputPasswordFlagEnabled) {
await this.setInitialPasswordTdeUserWithPermission(passwordInputResult);
return; // EARLY RETURN for flagged logic
}
await this.setInitialPassword(passwordInputResult);
break;
case SetInitialPasswordUserType.OFFBOARDED_TDE_ORG_USER:
await this.setInitialPasswordTdeOffboarding(passwordInputResult);
@@ -382,6 +389,46 @@ export class SetInitialPasswordComponent implements OnInit {
}
}
private async setInitialPasswordTdeUserWithPermission(passwordInputResult: PasswordInputResult) {
const ctx =
"Could not set initial password for TDE user with Manage Account Recovery permission.";
assertTruthy(passwordInputResult.newPassword, "newPassword", ctx);
assertTruthy(passwordInputResult.salt, "salt", ctx);
assertNonNullish(passwordInputResult.kdfConfig, "kdfConfig", ctx);
assertNonNullish(passwordInputResult.newPasswordHint, "newPasswordHint", ctx); // can have an empty string as a valid value, so check non-nullish
assertTruthy(this.orgSsoIdentifier, "orgSsoIdentifier", ctx);
assertTruthy(this.orgId, "orgId", ctx);
assertNonNullish(this.resetPasswordAutoEnroll, "resetPasswordAutoEnroll", ctx); // can have `false` as a valid value, so check non-nullish
assertTruthy(this.userId, "userId", ctx);
try {
const credentials: SetInitialPasswordTdeUserWithPermissionCredentials = {
newPassword: passwordInputResult.newPassword,
salt: passwordInputResult.salt,
kdfConfig: passwordInputResult.kdfConfig,
newPasswordHint: passwordInputResult.newPasswordHint,
orgSsoIdentifier: this.orgSsoIdentifier,
orgId: this.orgId as OrganizationId,
resetPasswordAutoEnroll: this.resetPasswordAutoEnroll,
};
await this.setInitialPasswordService.setInitialPasswordTdeUserWithPermission(
credentials,
this.userId,
);
this.showSuccessToastByUserType();
this.submitting = false;
await this.router.navigate(["vault"]);
} catch (e) {
this.logService.error("Error setting initial password", e);
this.validationService.showError(e);
this.submitting = false;
}
}
private async setInitialPasswordTdeOffboarding(passwordInputResult: PasswordInputResult) {
const ctx = "Could not set initial password.";
assertTruthy(passwordInputResult.newMasterKey, "newMasterKey", ctx);

View File

@@ -55,6 +55,16 @@ export interface SetInitialPasswordCredentials {
salt: MasterPasswordSalt;
}
export interface SetInitialPasswordTdeUserWithPermissionCredentials {
newPassword: string;
salt: MasterPasswordSalt;
kdfConfig: KdfConfig;
newPasswordHint: string;
orgSsoIdentifier: string;
orgId: OrganizationId;
resetPasswordAutoEnroll: boolean;
}
export interface SetInitialPasswordTdeOffboardingCredentials {
newMasterKey: MasterKey;
newServerMasterKeyHash: string;
@@ -103,6 +113,19 @@ export abstract class SetInitialPasswordService {
userId: UserId,
) => Promise<void>;
/**
* Sets an initial password for an existing authed TDE user who has been given the
* Manage Account Recovery permission:
* - {@link SetInitialPasswordUserType.TDE_ORG_USER_RESET_PASSWORD_PERMISSION_REQUIRES_MP}
*
* @param credentials An object of the credentials needed to set the initial password
* @throws If any property on the `credentials` object not found, or if userKey is not found
*/
abstract setInitialPasswordTdeUserWithPermission: (
credentials: SetInitialPasswordTdeUserWithPermissionCredentials,
userId: UserId,
) => Promise<void>;
/**
* Sets an initial password for a user who logs in after their org offboarded from
* trusted device encryption and is now a master-password-encryption org:

View File

@@ -56,6 +56,10 @@ import {
UserDecryptionOptionsService,
UserDecryptionOptionsServiceAbstraction,
} from "@bitwarden/auth/common";
import {
AutomaticUserConfirmationService,
DefaultAutomaticUserConfirmationService,
} from "@bitwarden/auto-confirm";
import { ApiService as ApiServiceAbstraction } from "@bitwarden/common/abstractions/api.service";
import { AuditService as AuditServiceAbstraction } from "@bitwarden/common/abstractions/audit.service";
import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service";
@@ -1060,6 +1064,19 @@ const safeProviders: SafeProvider[] = [
PendingAuthRequestsStateService,
],
}),
safeProvider({
provide: AutomaticUserConfirmationService,
useClass: DefaultAutomaticUserConfirmationService,
deps: [
ConfigService,
ApiServiceAbstraction,
OrganizationUserService,
StateProvider,
InternalOrganizationServiceAbstraction,
OrganizationUserApiService,
InternalPolicyService,
],
}),
safeProvider({
provide: ServerNotificationsService,
useClass: devFlagEnabled("noopNotifications")
@@ -1079,6 +1096,7 @@ const safeProviders: SafeProvider[] = [
AuthRequestAnsweringService,
ConfigService,
InternalPolicyService,
AutomaticUserConfirmationService,
],
}),
safeProvider({
@@ -1510,7 +1528,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({
provide: OrganizationMetadataServiceAbstraction,
useClass: DefaultOrganizationMetadataService,
deps: [BillingApiServiceAbstraction, ConfigService, PlatformUtilsServiceAbstraction],
deps: [BillingApiServiceAbstraction, PlatformUtilsServiceAbstraction],
}),
safeProvider({
provide: BillingAccountProfileStateService,

View File

@@ -143,6 +143,17 @@ describe("VaultFilter", () => {
expect(result).toBe(true);
});
it("should return true when filtering on unassigned folder via empty string id", () => {
const filterFunction = createFilterFunction({
selectedFolder: true,
selectedFolderId: "",
});
const result = filterFunction(cipher);
expect(result).toBe(true);
});
});
describe("given an organizational cipher (with organization and collections)", () => {

View File

@@ -63,10 +63,19 @@ export class VaultFilter {
if (this.cipherType != null && cipherPassesFilter) {
cipherPassesFilter = CipherViewLikeUtils.getType(cipher) === this.cipherType;
}
if (this.selectedFolder && this.selectedFolderId == null && cipherPassesFilter) {
cipherPassesFilter = cipher.folderId == null;
if (
this.selectedFolder &&
(this.selectedFolderId == null || this.selectedFolderId === "") &&
cipherPassesFilter
) {
cipherPassesFilter = cipher.folderId == null || cipher.folderId === "";
}
if (this.selectedFolder && this.selectedFolderId != null && cipherPassesFilter) {
if (
this.selectedFolder &&
this.selectedFolderId != null &&
this.selectedFolderId !== "" &&
cipherPassesFilter
) {
cipherPassesFilter = cipher.folderId === this.selectedFolderId;
}
if (this.selectedCollection && this.selectedCollectionId == null && cipherPassesFilter) {

View File

@@ -83,7 +83,7 @@ export class VaultFilterService implements DeprecatedVaultFilterServiceAbstracti
const ciphers = await this.cipherService.getAllDecrypted(userId);
const orgCiphers = ciphers.filter((c) => c.organizationId == organizationId);
folders = storedFolders.filter(
(f) => orgCiphers.some((oc) => oc.folderId == f.id) || f.id == null,
(f) => orgCiphers.some((oc) => oc.folderId == f.id) || !f.id,
);
}

View File

@@ -13,16 +13,9 @@
## Standard Auth Request Flows
### Flow 1: Unauthed user requests approval from device; Approving device has a masterKey in memory
### Flow 1: This flow was removed
1. Unauthed user clicks "Login with device"
2. Navigates to `/login-with-device` which creates a `StandardAuthRequest`
3. Receives approval from a device with authRequestPublicKey(masterKey)
4. Decrypts masterKey
5. Decrypts userKey
6. Proceeds to vault
### Flow 2: Unauthed user requests approval from device; Approving device does NOT have a masterKey in memory
### Flow 2: Unauthed user requests approval from device; Approving device does NOT need to have a masterKey in memory
1. Unauthed user clicks "Login with device"
2. Navigates to `/login-with-device` which creates a `StandardAuthRequest`
@@ -33,28 +26,18 @@
**Note:** This flow is an uncommon scenario and relates to TDE off-boarding. The following describes how a user could
get into this flow:
1. An SSO TD user logs into a device via an Admin auth request approval, therefore this device does NOT have a masterKey
1. An SSO TD user logs into a device via an Admin auth request approval, therefore this device does NOT need to have a masterKey
in memory
2. The org admin:
- Changes the member decryption options from "Trusted devices" to "Master password" AND
- Turns off the "Require single sign-on authentication" policy
3. On another device, the user clicks "Login with device", which they can do because the org no longer requires SSO
4. The user approves from the device they had previously logged into with SSO TD, which does NOT have a masterKey in
4. The user approves from the device they had previously logged into with SSO TD, which does NOT need to have a masterKey in
memory
### Flow 3: Authed SSO TD user requests approval from device; Approving device has a masterKey in memory
### Flow 3: This flow was removed
1. SSO TD user authenticates via SSO
2. Navigates to `/login-initiated`
3. Clicks "Approve from your other device"
4. Navigates to `/login-with-device` which creates a `StandardAuthRequest`
5. Receives approval from device with authRequestPublicKey(masterKey)
6. Decrypts masterKey
7. Decrypts userKey
8. Establishes trust (if required)
9. Proceeds to vault
### Flow 4: Authed SSO TD user requests approval from device; Approving device does NOT have a masterKey in memory
### Flow 4: Authed SSO TD user requests approval from device; Approving device does NOT need to have a masterKey in memory
1. SSO TD user authenticates via SSO
2. Navigates to `/login-initiated`
@@ -89,9 +72,7 @@ userKey. This is how admins are able to send over the authRequestPublicKey(userK
| Flow | Auth Status | Clicks Button [active route] | Navigates to | Approving device has masterKey in memory\* |
| --------------- | ----------- | ----------------------------------------------------- | --------------------------- | ------------------------------------------------- |
| Standard Flow 1 | unauthed | "Login with device" [`/login`] | `/login-with-device` | yes |
| Standard Flow 2 | unauthed | "Login with device" [`/login`] | `/login-with-device` | no |
| Standard Flow 3 | authed | "Approve from your other device" [`/login-initiated`] | `/login-with-device` | yes |
| Standard Flow 4 | authed | "Approve from your other device" [`/login-initiated`] | `/login-with-device` | no |
| Admin Flow | authed | "Request admin approval"<br>[`/login-initiated`] | `/admin-approval-requested` | NA - admin requests always send encrypted userKey |

View File

@@ -605,10 +605,10 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy {
if (authRequestResponse.requestApproved) {
const userHasAuthenticatedViaSSO = this.authStatus === AuthenticationStatus.Locked;
if (userHasAuthenticatedViaSSO) {
// [Standard Flow 3-4] Handle authenticated SSO TD user flows
// [Standard Flow 4] Handle authenticated SSO TD user flows
return await this.handleAuthenticatedFlows(authRequestResponse);
} else {
// [Standard Flow 1-2] Handle unauthenticated user flows
// [Standard Flow 2] Handle unauthenticated user flows
return await this.handleUnauthenticatedFlows(authRequestResponse, requestId);
}
}
@@ -629,7 +629,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy {
}
private async handleAuthenticatedFlows(authRequestResponse: AuthRequestResponse) {
// [Standard Flow 3-4] Handle authenticated SSO TD user flows
// [Standard Flow 4] Handle authenticated SSO TD user flows
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
if (!userId) {
this.logService.error(
@@ -654,7 +654,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy {
authRequestResponse: AuthRequestResponse,
requestId: string,
) {
// [Standard Flow 1-2] Handle unauthenticated user flows
// [Standard Flow 2] Handle unauthenticated user flows
const authRequestLoginCredentials = await this.buildAuthRequestLoginCredentials(
requestId,
authRequestResponse,
@@ -676,30 +676,15 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy {
private async decryptViaApprovedAuthRequest(
authRequestResponse: AuthRequestResponse,
privateKey: ArrayBuffer,
privateKey: Uint8Array,
userId: UserId,
): Promise<void> {
/**
* [Flow Type Detection]
* We determine the type of `key` based on the presence or absence of `masterPasswordHash`:
* - If `masterPasswordHash` exists: Standard Flow 1 or 3 (device has masterKey)
* - If no `masterPasswordHash`: Standard Flow 2, 4, or Admin Flow (device sends userKey)
*/
if (authRequestResponse.masterPasswordHash) {
// [Standard Flow 1 or 3] Device has masterKey
await this.authRequestService.setKeysAfterDecryptingSharedMasterKeyAndHash(
authRequestResponse,
privateKey,
userId,
);
} else {
// [Standard Flow 2, 4, or Admin Flow] Device sends userKey
await this.authRequestService.setUserKeyAfterDecryptingSharedUserKey(
authRequestResponse,
privateKey,
userId,
);
}
// [Standard Flow 2, 4, or Admin Flow] Device sends userKey
await this.authRequestService.setUserKeyAfterDecryptingSharedUserKey(
authRequestResponse,
privateKey,
userId,
);
// [Admin Flow Cleanup] Clear one-time use admin auth request
// clear the admin auth request from state so it cannot be used again (it's a one time use)
@@ -758,43 +743,13 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy {
/**
* See verifyAndHandleApprovedAuthReq() for flow details.
*
* We determine the type of `key` based on the presence or absence of `masterPasswordHash`:
* - If `masterPasswordHash` has a value, we receive the `key` as an authRequestPublicKey(masterKey) [plus we have authRequestPublicKey(masterPasswordHash)]
* - If `masterPasswordHash` does not have a value, we receive the `key` as an authRequestPublicKey(userKey)
*/
if (authRequestResponse.masterPasswordHash) {
// ...in Standard Auth Request Flow 1
const { masterKey, masterKeyHash } =
await this.authRequestService.decryptPubKeyEncryptedMasterKeyAndHash(
authRequestResponse.key,
authRequestResponse.masterPasswordHash,
this.authRequestKeyPair.privateKey,
);
return new AuthRequestLoginCredentials(
this.email,
this.accessCode,
requestId,
null, // no userKey
masterKey,
masterKeyHash,
);
} else {
// ...in Standard Auth Request Flow 2
const userKey = await this.authRequestService.decryptPubKeyEncryptedUserKey(
authRequestResponse.key,
this.authRequestKeyPair.privateKey,
);
return new AuthRequestLoginCredentials(
this.email,
this.accessCode,
requestId,
userKey,
null, // no masterKey
null, // no masterKeyHash
);
}
// ...in Standard Auth Request Flow 2
const userKey = await this.authRequestService.decryptPubKeyEncryptedUserKey(
authRequestResponse.key,
this.authRequestKeyPair.privateKey,
);
return new AuthRequestLoginCredentials(this.email, this.accessCode, requestId, userKey);
}
private async clearExistingAdminAuthRequestAndStartNewRequest(userId: UserId) {

View File

@@ -4,7 +4,7 @@ import { AdminAuthRequestStorable } from "@bitwarden/common/auth/models/domain/a
import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response";
import { AuthRequestPushNotification } from "@bitwarden/common/models/response/notification.response";
import { UserId } from "@bitwarden/common/types/guid";
import { UserKey, MasterKey } from "@bitwarden/common/types/key";
import { UserKey } from "@bitwarden/common/types/key";
export abstract class AuthRequestServiceAbstraction {
/** Emits an auth request id when an auth request has been approved. */
@@ -72,18 +72,7 @@ export abstract class AuthRequestServiceAbstraction {
*/
abstract setUserKeyAfterDecryptingSharedUserKey(
authReqResponse: AuthRequestResponse,
authReqPrivateKey: ArrayBuffer,
userId: UserId,
): Promise<void>;
/**
* Sets the `MasterKey` and `MasterKeyHash` from an auth request. Auth request must have a `MasterKey` and `MasterKeyHash`.
* @param authReqResponse The auth request.
* @param authReqPrivateKey The private key corresponding to the public key sent in the auth request.
* @param userId The ID of the user for whose account we will set the keys.
*/
abstract setKeysAfterDecryptingSharedMasterKeyAndHash(
authReqResponse: AuthRequestResponse,
authReqPrivateKey: ArrayBuffer,
authReqPrivateKey: Uint8Array,
userId: UserId,
): Promise<void>;
/**
@@ -94,20 +83,8 @@ export abstract class AuthRequestServiceAbstraction {
*/
abstract decryptPubKeyEncryptedUserKey(
pubKeyEncryptedUserKey: string,
privateKey: ArrayBuffer,
privateKey: Uint8Array,
): Promise<UserKey>;
/**
* Decrypts a `MasterKey` and `MasterKeyHash` from a public key encrypted `MasterKey` and `MasterKeyHash`.
* @param pubKeyEncryptedMasterKey The public key encrypted `MasterKey`.
* @param pubKeyEncryptedMasterKeyHash The public key encrypted `MasterKeyHash`.
* @param privateKey The private key corresponding to the public key used to encrypt the `MasterKey` and `MasterKeyHash`.
* @returns The decrypted `MasterKey` and `MasterKeyHash`.
*/
abstract decryptPubKeyEncryptedMasterKeyAndHash(
pubKeyEncryptedMasterKey: string,
pubKeyEncryptedMasterKeyHash: string,
privateKey: ArrayBuffer,
): Promise<{ masterKey: MasterKey; masterKeyHash: string }>;
/**
* Handles incoming auth request push server notifications.

View File

@@ -26,7 +26,7 @@ import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/sym
import { makeEncString, FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec";
import { CsprngArray } from "@bitwarden/common/types/csprng";
import { UserId } from "@bitwarden/common/types/guid";
import { MasterKey, UserKey } from "@bitwarden/common/types/key";
import { UserKey } from "@bitwarden/common/types/key";
import { KdfConfigService, KeyService } from "@bitwarden/key-management";
import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction";
@@ -73,11 +73,7 @@ describe("AuthRequestLoginStrategy", () => {
const email = "EMAIL";
const accessCode = "ACCESS_CODE";
const authRequestId = "AUTH_REQUEST_ID";
const decMasterKey = new SymmetricCryptoKey(
new Uint8Array(64).buffer as CsprngArray,
) as MasterKey;
const decUserKey = new SymmetricCryptoKey(new Uint8Array(64).buffer as CsprngArray) as UserKey;
const decMasterKeyHash = "LOCAL_PASSWORD_HASH";
beforeEach(async () => {
keyService = mock<KeyService>();
@@ -150,42 +146,6 @@ describe("AuthRequestLoginStrategy", () => {
);
});
it("sets keys after a successful authentication when masterKey and masterKeyHash provided in login credentials", async () => {
credentials = new AuthRequestLoginCredentials(
email,
accessCode,
authRequestId,
null,
decMasterKey,
decMasterKeyHash,
);
const masterKey = new SymmetricCryptoKey(new Uint8Array(64).buffer as CsprngArray) as MasterKey;
const userKey = new SymmetricCryptoKey(new Uint8Array(64).buffer as CsprngArray) as UserKey;
masterPasswordService.masterKeySubject.next(masterKey);
masterPasswordService.mock.decryptUserKeyWithMasterKey.mockResolvedValue(userKey);
tokenService.decodeAccessToken.mockResolvedValue({ sub: mockUserId });
await authRequestLoginStrategy.logIn(credentials);
expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith(masterKey, mockUserId);
expect(masterPasswordService.mock.setMasterKeyHash).toHaveBeenCalledWith(
decMasterKeyHash,
mockUserId,
);
expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(
tokenResponse.key,
mockUserId,
);
expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, mockUserId);
expect(deviceTrustService.trustDeviceIfRequired).toHaveBeenCalled();
expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith(
{ V1: { private_key: tokenResponse.privateKey } },
mockUserId,
);
});
it("sets keys after a successful authentication when only userKey provided in login credentials", async () => {
// Initialize credentials with only userKey
credentials = new AuthRequestLoginCredentials(
@@ -193,8 +153,6 @@ describe("AuthRequestLoginStrategy", () => {
accessCode,
authRequestId,
decUserKey, // Pass userKey
null, // No masterKey
null, // No masterKeyHash
);
// Call logIn
@@ -240,7 +198,6 @@ describe("AuthRequestLoginStrategy", () => {
};
apiService.postIdentityToken.mockResolvedValue(tokenResponse);
masterPasswordService.masterKeySubject.next(decMasterKey);
masterPasswordService.mock.decryptUserKeyWithMasterKey.mockResolvedValue(decUserKey);
await authRequestLoginStrategy.logIn(credentials);

View File

@@ -72,20 +72,7 @@ export class AuthRequestLoginStrategy extends LoginStrategy {
}
protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) {
const authRequestCredentials = this.cache.value.authRequestCredentials;
if (
authRequestCredentials.decryptedMasterKey &&
authRequestCredentials.decryptedMasterKeyHash
) {
await this.masterPasswordService.setMasterKey(
authRequestCredentials.decryptedMasterKey,
userId,
);
await this.masterPasswordService.setMasterKeyHash(
authRequestCredentials.decryptedMasterKeyHash,
userId,
);
}
// This login strategy does not use a master key
}
protected override async setUserKey(

View File

@@ -416,24 +416,6 @@ describe("SsoLoginStrategy", () => {
);
});
it("sets the user key using master key and hash from approved admin request if exists", async () => {
apiService.postIdentityToken.mockResolvedValue(tokenResponse);
keyService.hasUserKey.mockResolvedValue(true);
const adminAuthResponse = {
id: "1",
publicKey: "PRIVATE" as any,
key: "KEY" as any,
masterPasswordHash: "HASH" as any,
requestApproved: true,
};
apiService.getAuthRequest.mockResolvedValue(adminAuthResponse as AuthRequestResponse);
await ssoLoginStrategy.logIn(credentials);
expect(authRequestService.setKeysAfterDecryptingSharedMasterKeyAndHash).toHaveBeenCalled();
expect(deviceTrustService.decryptUserKeyWithDeviceKey).not.toHaveBeenCalled();
});
it("sets the user key from approved admin request if exists", async () => {
apiService.postIdentityToken.mockResolvedValue(tokenResponse);
keyService.hasUserKey.mockResolvedValue(true);
@@ -475,9 +457,6 @@ describe("SsoLoginStrategy", () => {
await ssoLoginStrategy.logIn(credentials);
expect(authRequestService.clearAdminAuthRequest).toHaveBeenCalled();
expect(
authRequestService.setKeysAfterDecryptingSharedMasterKeyAndHash,
).not.toHaveBeenCalled();
expect(authRequestService.setUserKeyAfterDecryptingSharedUserKey).not.toHaveBeenCalled();
expect(deviceTrustService.trustDeviceIfRequired).not.toHaveBeenCalled();
});

View File

@@ -239,23 +239,11 @@ export class SsoLoginStrategy extends LoginStrategy {
}
if (adminAuthReqResponse?.requestApproved) {
// if masterPasswordHash has a value, we will always receive authReqResponse.key
// as authRequestPublicKey(masterKey) + authRequestPublicKey(masterPasswordHash)
if (adminAuthReqResponse.masterPasswordHash) {
await this.authRequestService.setKeysAfterDecryptingSharedMasterKeyAndHash(
adminAuthReqResponse,
adminAuthReqStorable.privateKey,
userId,
);
} else {
// if masterPasswordHash is null, we will always receive authReqResponse.key
// as authRequestPublicKey(userKey)
await this.authRequestService.setUserKeyAfterDecryptingSharedUserKey(
adminAuthReqResponse,
adminAuthReqStorable.privateKey,
userId,
);
}
await this.authRequestService.setUserKeyAfterDecryptingSharedUserKey(
adminAuthReqResponse,
adminAuthReqStorable.privateKey,
userId,
);
if (await this.keyService.hasUserKey(userId)) {
// Now that we have a decrypted user key in memory, we can check if we

View File

@@ -7,7 +7,7 @@ import { AuthenticationType } from "@bitwarden/common/auth/enums/authentication-
import { TokenTwoFactorRequest } from "@bitwarden/common/auth/models/request/identity-token/token-two-factor.request";
import { WebAuthnLoginAssertionResponseRequest } from "@bitwarden/common/auth/services/webauthn-login/request/webauthn-login-assertion-response.request";
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
import { UserKey, MasterKey } from "@bitwarden/common/types/key";
import { UserKey } from "@bitwarden/common/types/key";
export class PasswordLoginCredentials {
readonly type = AuthenticationType.Password;
@@ -54,8 +54,6 @@ export class AuthRequestLoginCredentials {
public accessCode: string,
public authRequestId: string,
public decryptedUserKey: UserKey | null,
public decryptedMasterKey: MasterKey | null,
public decryptedMasterKeyHash: string | null,
public twoFactor?: TokenTwoFactorRequest,
) {}
@@ -66,8 +64,6 @@ export class AuthRequestLoginCredentials {
json.accessCode,
json.authRequestId,
null,
null,
json.decryptedMasterKeyHash,
json.twoFactor
? new TokenTwoFactorRequest(
json.twoFactor.provider,
@@ -78,7 +74,6 @@ export class AuthRequestLoginCredentials {
),
{
decryptedUserKey: SymmetricCryptoKey.fromJSON(json.decryptedUserKey) as UserKey,
decryptedMasterKey: SymmetricCryptoKey.fromJSON(json.decryptedMasterKey) as MasterKey,
},
);
}

View File

@@ -13,7 +13,7 @@ import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.ser
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
import { StateProvider } from "@bitwarden/common/platform/state";
import { UserId } from "@bitwarden/common/types/guid";
import { MasterKey, UserKey } from "@bitwarden/common/types/key";
import { UserKey } from "@bitwarden/common/types/key";
import { newGuid } from "@bitwarden/guid";
import { KeyService } from "@bitwarden/key-management";
@@ -154,60 +154,6 @@ describe("AuthRequestService", () => {
});
});
describe("setKeysAfterDecryptingSharedMasterKeyAndHash", () => {
it("decrypts and sets master key and hash and user key when given valid auth request response and private key", async () => {
// Arrange
const mockAuthReqResponse = {
key: "authReqPublicKeyEncryptedMasterKey",
masterPasswordHash: "authReqPublicKeyEncryptedMasterKeyHash",
} as AuthRequestResponse;
const mockDecryptedMasterKey = {} as MasterKey;
const mockDecryptedMasterKeyHash = "mockDecryptedMasterKeyHash";
const mockDecryptedUserKey = {} as UserKey;
jest.spyOn(sut, "decryptPubKeyEncryptedMasterKeyAndHash").mockResolvedValueOnce({
masterKey: mockDecryptedMasterKey,
masterKeyHash: mockDecryptedMasterKeyHash,
});
masterPasswordService.masterKeySubject.next(undefined);
masterPasswordService.masterKeyHashSubject.next(undefined);
masterPasswordService.mock.decryptUserKeyWithMasterKey.mockResolvedValue(
mockDecryptedUserKey,
);
keyService.setUserKey.mockResolvedValueOnce(undefined);
// Act
await sut.setKeysAfterDecryptingSharedMasterKeyAndHash(
mockAuthReqResponse,
mockPrivateKey,
mockUserId,
);
// Assert
expect(sut.decryptPubKeyEncryptedMasterKeyAndHash).toBeCalledWith(
mockAuthReqResponse.key,
mockAuthReqResponse.masterPasswordHash,
mockPrivateKey,
);
expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith(
mockDecryptedMasterKey,
mockUserId,
);
expect(masterPasswordService.mock.setMasterKeyHash).toHaveBeenCalledWith(
mockDecryptedMasterKeyHash,
mockUserId,
);
expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith(
mockDecryptedMasterKey,
mockUserId,
undefined,
);
expect(keyService.setUserKey).toHaveBeenCalledWith(mockDecryptedUserKey, mockUserId);
});
});
describe("decryptAuthReqPubKeyEncryptedUserKey", () => {
it("returns a decrypted user key when given valid public key encrypted user key and an auth req private key", async () => {
// Arrange

View File

@@ -16,14 +16,13 @@ import { ListResponse } from "@bitwarden/common/models/response/list.response";
import { AuthRequestPushNotification } from "@bitwarden/common/models/response/notification.response";
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
import {
AUTH_REQUEST_DISK_LOCAL,
StateProvider,
UserKeyDefinition,
} from "@bitwarden/common/platform/state";
import { UserId } from "@bitwarden/common/types/guid";
import { MasterKey, UserKey } from "@bitwarden/common/types/key";
import { UserKey } from "@bitwarden/common/types/key";
import { KeyService } from "@bitwarden/key-management";
import { AuthRequestApiServiceAbstraction } from "../../abstractions/auth-request-api.service";
@@ -163,27 +162,6 @@ export class AuthRequestService implements AuthRequestServiceAbstraction {
await this.keyService.setUserKey(userKey, userId);
}
async setKeysAfterDecryptingSharedMasterKeyAndHash(
authReqResponse: AuthRequestResponse,
authReqPrivateKey: Uint8Array,
userId: UserId,
) {
const { masterKey, masterKeyHash } = await this.decryptPubKeyEncryptedMasterKeyAndHash(
authReqResponse.key,
authReqResponse.masterPasswordHash,
authReqPrivateKey,
);
// Decrypt and set user key in state
const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey, userId);
// Set masterKey + masterKeyHash in state after decryption (in case decryption fails)
await this.masterPasswordService.setMasterKey(masterKey, userId);
await this.masterPasswordService.setMasterKeyHash(masterKeyHash, userId);
await this.keyService.setUserKey(userKey, userId);
}
// Decryption helpers
async decryptPubKeyEncryptedUserKey(
pubKeyEncryptedUserKey: string,
@@ -197,30 +175,6 @@ export class AuthRequestService implements AuthRequestServiceAbstraction {
return decryptedUserKey as UserKey;
}
async decryptPubKeyEncryptedMasterKeyAndHash(
pubKeyEncryptedMasterKey: string,
pubKeyEncryptedMasterKeyHash: string,
privateKey: Uint8Array,
): Promise<{ masterKey: MasterKey; masterKeyHash: string }> {
const decryptedMasterKeyArrayBuffer = await this.encryptService.rsaDecrypt(
new EncString(pubKeyEncryptedMasterKey),
privateKey,
);
const decryptedMasterKeyHashArrayBuffer = await this.encryptService.rsaDecrypt(
new EncString(pubKeyEncryptedMasterKeyHash),
privateKey,
);
const masterKey = new SymmetricCryptoKey(decryptedMasterKeyArrayBuffer) as MasterKey;
const masterKeyHash = Utils.fromBufferToUtf8(decryptedMasterKeyHashArrayBuffer);
return {
masterKey,
masterKeyHash,
};
}
sendAuthRequestPushNotification(notification: AuthRequestPushNotification): void {
if (notification.id != null) {
this.authRequestPushNotificationSubject.next(notification.id);

View File

@@ -93,8 +93,6 @@ describe("LOGIN_STRATEGY_CACHE_KEY", () => {
"ACCESS_CODE",
"AUTH_REQUEST_ID",
new SymmetricCryptoKey(new Uint8Array(64)) as UserKey,
new SymmetricCryptoKey(new Uint8Array(64)) as MasterKey,
"MASTER_KEY_HASH",
);
const result = sut.deserializer(JSON.parse(JSON.stringify(actual)));

View File

@@ -1,6 +1,6 @@
import { Observable } from "rxjs";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { OrganizationId } from "@bitwarden/common/types/guid";
import { UserId } from "@bitwarden/user-core";
import { AutoConfirmState } from "../models/auto-confirm-state.model";
@@ -27,12 +27,12 @@ export abstract class AutomaticUserConfirmationService {
/**
* Calls the API endpoint to initiate automatic user confirmation.
* @param userId The userId of the logged in admin performing auto confirmation. This is neccesary to perform the key exchange and for permissions checks.
* @param confirmingUserId The userId of the user being confirmed.
* @param organization the organization the user is being auto confirmed to.
* @param confirmedUserId The userId of the member being confirmed.
* @param organization the organization the member is being auto confirmed to.
**/
abstract autoConfirmUser(
userId: UserId,
confirmingUserId: UserId,
organization: Organization,
confirmedUserId: UserId,
organization: OrganizationId,
): Promise<void>;
}

View File

@@ -3,14 +3,13 @@ import { Router, UrlTree } from "@angular/router";
import { mock, MockProxy } from "jest-mock-extended";
import { BehaviorSubject, firstValueFrom, Observable, of } from "rxjs";
import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm";
import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { UserId } from "@bitwarden/common/types/guid";
import { ToastService } from "@bitwarden/components";
import { newGuid } from "@bitwarden/guid";
import { AutomaticUserConfirmationService } from "../abstractions";
import { canAccessAutoConfirmSettings } from "./automatic-user-confirmation-settings.guard";
describe("canAccessAutoConfirmSettings", () => {

View File

@@ -2,13 +2,12 @@ import { inject } from "@angular/core";
import { CanActivateFn, Router } from "@angular/router";
import { map, switchMap } from "rxjs";
import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { filterOutNullish } from "@bitwarden/common/vault/utils/observable-utilities";
import { ToastService } from "@bitwarden/components";
import { AutomaticUserConfirmationService } from "../abstractions";
export const canAccessAutoConfirmSettings: CanActivateFn = () => {
const accountService = inject(AccountService);
const autoConfirmService = inject(AutomaticUserConfirmationService);

View File

@@ -0,0 +1,8 @@
// Re-export core auto-confirm functionality for convenience
export * from "../abstractions";
export * from "../models";
export * from "../services";
// Angular-specific exports
export * from "./components";
export * from "./guards";

View File

@@ -1,5 +1,3 @@
export * from "./abstractions";
export * from "./components";
export * from "./guards";
export * from "./models";
export * from "./services";

View File

@@ -377,48 +377,85 @@ describe("DefaultAutomaticUserConfirmationService", () => {
defaultUserCollectionName: "encrypted-collection",
} as OrganizationUserConfirmRequest;
beforeEach(() => {
beforeEach(async () => {
const organizations$ = new BehaviorSubject<Organization[]>([mockOrganization]);
organizationService.organizations$.mockReturnValue(organizations$);
configService.getFeatureFlag$.mockReturnValue(of(true));
policyService.policyAppliesToUser$.mockReturnValue(of(true));
// Enable auto-confirm configuration for the user
const enabledConfig = new AutoConfirmState();
enabledConfig.enabled = true;
await stateProvider.setUserState(
AUTO_CONFIRM_STATE,
{ [mockUserId]: enabledConfig },
mockUserId,
);
apiService.getUserPublicKey.mockResolvedValue({
publicKey: mockPublicKey,
} as UserKeyResponse);
jest.spyOn(Utils, "fromB64ToArray").mockReturnValue(mockPublicKeyArray);
organizationUserService.buildConfirmRequest.mockReturnValue(of(mockConfirmRequest));
organizationUserApiService.postOrganizationUserConfirm.mockResolvedValue(undefined);
organizationUserApiService.postOrganizationUserAutoConfirm.mockResolvedValue(undefined);
});
it("should successfully auto-confirm a user", async () => {
await service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganization);
it("should successfully auto-confirm a user with organizationId", async () => {
await service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganizationId);
expect(apiService.getUserPublicKey).toHaveBeenCalledWith(mockUserId);
expect(organizationUserService.buildConfirmRequest).toHaveBeenCalledWith(
mockOrganization,
mockPublicKeyArray,
);
expect(organizationUserApiService.postOrganizationUserConfirm).toHaveBeenCalledWith(
expect(organizationUserApiService.postOrganizationUserAutoConfirm).toHaveBeenCalledWith(
mockOrganizationId,
mockConfirmingUserId,
mockConfirmRequest,
);
});
it("should not confirm user when canManageAutoConfirm returns false", async () => {
it("should return early when canManageAutoConfirm returns false", async () => {
configService.getFeatureFlag$.mockReturnValue(of(false));
await expect(
service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganization),
).rejects.toThrow("Cannot automatically confirm user (insufficient permissions)");
await service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganizationId);
expect(apiService.getUserPublicKey).not.toHaveBeenCalled();
expect(organizationUserApiService.postOrganizationUserConfirm).not.toHaveBeenCalled();
expect(organizationUserApiService.postOrganizationUserAutoConfirm).not.toHaveBeenCalled();
});
it("should return early when auto-confirm is disabled in configuration", async () => {
const disabledConfig = new AutoConfirmState();
disabledConfig.enabled = false;
await stateProvider.setUserState(
AUTO_CONFIRM_STATE,
{ [mockUserId]: disabledConfig },
mockUserId,
);
await service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganizationId);
expect(apiService.getUserPublicKey).not.toHaveBeenCalled();
expect(organizationUserApiService.postOrganizationUserAutoConfirm).not.toHaveBeenCalled();
});
it("should return early when auto-confirm is disabled in configuration", async () => {
const disabledConfig = new AutoConfirmState();
disabledConfig.enabled = false;
await stateProvider.setUserState(
AUTO_CONFIRM_STATE,
{ [mockUserId]: disabledConfig },
mockUserId,
);
await service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganizationId);
expect(apiService.getUserPublicKey).not.toHaveBeenCalled();
expect(organizationUserApiService.postOrganizationUserAutoConfirm).not.toHaveBeenCalled();
});
it("should build confirm request with organization and public key", async () => {
await service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganization);
await service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganizationId);
expect(organizationUserService.buildConfirmRequest).toHaveBeenCalledWith(
mockOrganization,
@@ -427,10 +464,10 @@ describe("DefaultAutomaticUserConfirmationService", () => {
});
it("should call API with correct parameters", async () => {
await service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganization);
await service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganizationId);
expect(organizationUserApiService.postOrganizationUserConfirm).toHaveBeenCalledWith(
mockOrganization.id,
expect(organizationUserApiService.postOrganizationUserAutoConfirm).toHaveBeenCalledWith(
mockOrganizationId,
mockConfirmingUserId,
mockConfirmRequest,
);
@@ -441,10 +478,10 @@ describe("DefaultAutomaticUserConfirmationService", () => {
apiService.getUserPublicKey.mockRejectedValue(apiError);
await expect(
service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganization),
service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganizationId),
).rejects.toThrow("API Error");
expect(organizationUserApiService.postOrganizationUserConfirm).not.toHaveBeenCalled();
expect(organizationUserApiService.postOrganizationUserAutoConfirm).not.toHaveBeenCalled();
});
it("should handle buildConfirmRequest errors gracefully", async () => {
@@ -452,10 +489,10 @@ describe("DefaultAutomaticUserConfirmationService", () => {
organizationUserService.buildConfirmRequest.mockReturnValue(throwError(() => buildError));
await expect(
service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganization),
service.autoConfirmUser(mockUserId, mockConfirmingUserId, mockOrganizationId),
).rejects.toThrow("Build Error");
expect(organizationUserApiService.postOrganizationUserConfirm).not.toHaveBeenCalled();
expect(organizationUserApiService.postOrganizationUserAutoConfirm).not.toHaveBeenCalled();
});
});
});

View File

@@ -8,10 +8,11 @@ import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { InternalOrganizationServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { getById } from "@bitwarden/common/platform/misc";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { OrganizationId } from "@bitwarden/common/types/guid";
import { StateProvider } from "@bitwarden/state";
import { UserId } from "@bitwarden/user-core";
@@ -66,26 +67,44 @@ export class DefaultAutomaticUserConfirmationService implements AutomaticUserCon
async autoConfirmUser(
userId: UserId,
confirmingUserId: UserId,
organization: Organization,
confirmedUserId: UserId,
organizationId: OrganizationId,
): Promise<void> {
const canManage = await firstValueFrom(this.canManageAutoConfirm$(userId));
if (!canManage) {
return;
}
// Only initiate auto confirmation if the local client setting has been turned on
const autoConfirmEnabled = await firstValueFrom(
this.configuration$(userId).pipe(map((state) => state.enabled)),
);
if (!autoConfirmEnabled) {
return;
}
const organization$ = this.organizationService.organizations$(userId).pipe(
getById(organizationId),
map((organization) => {
if (organization == null) {
throw new Error("Organization not found");
}
return organization;
}),
);
const publicKeyResponse = await this.apiService.getUserPublicKey(userId);
const publicKey = Utils.fromB64ToArray(publicKeyResponse.publicKey);
await firstValueFrom(
this.canManageAutoConfirm$(userId).pipe(
map((canManage) => {
if (!canManage) {
throw new Error("Cannot automatically confirm user (insufficient permissions)");
}
return canManage;
}),
switchMap(() => this.apiService.getUserPublicKey(userId)),
map((publicKeyResponse) => Utils.fromB64ToArray(publicKeyResponse.publicKey)),
switchMap((publicKey) =>
this.organizationUserService.buildConfirmRequest(organization, publicKey),
),
organization$.pipe(
switchMap((org) => this.organizationUserService.buildConfirmRequest(org, publicKey)),
switchMap((request) =>
this.organizationUserApiService.postOrganizationUserConfirm(
organization.id,
confirmingUserId,
this.organizationUserApiService.postOrganizationUserAutoConfirm(
organizationId,
confirmedUserId,
request,
),
),

View File

@@ -92,6 +92,7 @@ import { CipherRequest } from "../vault/models/request/cipher.request";
import { AttachmentUploadDataResponse } from "../vault/models/response/attachment-upload-data.response";
import { AttachmentResponse } from "../vault/models/response/attachment.response";
import { CipherMiniResponse, CipherResponse } from "../vault/models/response/cipher.response";
import { DeleteAttachmentResponse } from "../vault/models/response/delete-attachment.response";
import { OptionalCipherResponse } from "../vault/models/response/optional-cipher.response";
/**
@@ -243,8 +244,14 @@ export abstract class ApiService {
id: string,
request: AttachmentRequest,
): Promise<AttachmentUploadDataResponse>;
abstract deleteCipherAttachment(id: string, attachmentId: string): Promise<any>;
abstract deleteCipherAttachmentAdmin(id: string, attachmentId: string): Promise<any>;
abstract deleteCipherAttachment(
id: string,
attachmentId: string,
): Promise<DeleteAttachmentResponse>;
abstract deleteCipherAttachmentAdmin(
id: string,
attachmentId: string,
): Promise<DeleteAttachmentResponse>;
abstract postShareCipherAttachment(
id: string,
attachmentId: string,

View File

@@ -11,8 +11,7 @@ export class AuthRequestResponse extends BaseResponse {
requestDeviceIdentifier: string;
requestIpAddress: string;
requestCountryName: string;
key: string; // could be either an encrypted MasterKey or an encrypted UserKey
masterPasswordHash: string; // if hash is present, the `key` above is an encrypted MasterKey (else `key` is an encrypted UserKey)
key: string; // Auth-request public-key encrypted user-key. Note: No sender authenticity provided!
creationDate: string;
requestApproved?: boolean;
responseDate?: string;
@@ -30,7 +29,6 @@ export class AuthRequestResponse extends BaseResponse {
this.requestIpAddress = this.getResponseProperty("RequestIpAddress");
this.requestCountryName = this.getResponseProperty("RequestCountryName");
this.key = this.getResponseProperty("Key");
this.masterPasswordHash = this.getResponseProperty("MasterPasswordHash");
this.creationDate = this.getResponseProperty("CreationDate");
this.requestApproved = this.getResponseProperty("RequestApproved");
this.responseDate = this.getResponseProperty("ResponseDate");

View File

@@ -21,11 +21,7 @@ export abstract class BillingApiServiceAbstraction {
organizationId: OrganizationId,
): Promise<OrganizationBillingMetadataResponse>;
abstract getOrganizationBillingMetadataVNext(
organizationId: OrganizationId,
): Promise<OrganizationBillingMetadataResponse>;
abstract getOrganizationBillingMetadataVNextSelfHost(
abstract getOrganizationBillingMetadataSelfHost(
organizationId: OrganizationId,
): Promise<OrganizationBillingMetadataResponse>;

View File

@@ -36,20 +36,6 @@ export class BillingApiService implements BillingApiServiceAbstraction {
async getOrganizationBillingMetadata(
organizationId: OrganizationId,
): Promise<OrganizationBillingMetadataResponse> {
const r = await this.apiService.send(
"GET",
"/organizations/" + organizationId + "/billing/metadata",
null,
true,
true,
);
return new OrganizationBillingMetadataResponse(r);
}
async getOrganizationBillingMetadataVNext(
organizationId: OrganizationId,
): Promise<OrganizationBillingMetadataResponse> {
const r = await this.apiService.send(
"GET",
@@ -62,7 +48,7 @@ export class BillingApiService implements BillingApiServiceAbstraction {
return new OrganizationBillingMetadataResponse(r);
}
async getOrganizationBillingMetadataVNextSelfHost(
async getOrganizationBillingMetadataSelfHost(
organizationId: OrganizationId,
): Promise<OrganizationBillingMetadataResponse> {
const r = await this.apiService.send(

View File

@@ -1,13 +1,11 @@
import { mock } from "jest-mock-extended";
import { BehaviorSubject, firstValueFrom } from "rxjs";
import { firstValueFrom } from "rxjs";
import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions";
import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { newGuid } from "@bitwarden/guid";
import { FeatureFlag } from "../../../enums/feature-flag.enum";
import { OrganizationId } from "../../../types/guid";
import { DefaultOrganizationMetadataService } from "./organization-metadata.service";
@@ -15,9 +13,7 @@ import { DefaultOrganizationMetadataService } from "./organization-metadata.serv
describe("DefaultOrganizationMetadataService", () => {
let service: DefaultOrganizationMetadataService;
let billingApiService: jest.Mocked<BillingApiServiceAbstraction>;
let configService: jest.Mocked<ConfigService>;
let platformUtilsService: jest.Mocked<PlatformUtilsService>;
let featureFlagSubject: BehaviorSubject<boolean>;
const mockOrganizationId = newGuid() as OrganizationId;
const mockOrganizationId2 = newGuid() as OrganizationId;
@@ -34,182 +30,114 @@ describe("DefaultOrganizationMetadataService", () => {
beforeEach(() => {
billingApiService = mock<BillingApiServiceAbstraction>();
configService = mock<ConfigService>();
platformUtilsService = mock<PlatformUtilsService>();
featureFlagSubject = new BehaviorSubject<boolean>(false);
configService.getFeatureFlag$.mockReturnValue(featureFlagSubject.asObservable());
platformUtilsService.isSelfHost.mockReturnValue(false);
service = new DefaultOrganizationMetadataService(
billingApiService,
configService,
platformUtilsService,
);
service = new DefaultOrganizationMetadataService(billingApiService, platformUtilsService);
});
afterEach(() => {
jest.resetAllMocks();
featureFlagSubject.complete();
});
describe("getOrganizationMetadata$", () => {
describe("feature flag OFF", () => {
beforeEach(() => {
featureFlagSubject.next(false);
});
it("calls getOrganizationBillingMetadata for cloud-hosted", async () => {
const mockResponse = createMockMetadataResponse(false, 10);
billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockResponse);
it("calls getOrganizationBillingMetadata when feature flag is off", async () => {
const mockResponse = createMockMetadataResponse(false, 10);
billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockResponse);
const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
expect(configService.getFeatureFlag$).toHaveBeenCalledWith(
FeatureFlag.PM25379_UseNewOrganizationMetadataStructure,
);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledWith(
mockOrganizationId,
);
expect(billingApiService.getOrganizationBillingMetadataVNext).not.toHaveBeenCalled();
expect(result).toEqual(mockResponse);
});
it("does not cache metadata when feature flag is off", async () => {
const mockResponse1 = createMockMetadataResponse(false, 10);
const mockResponse2 = createMockMetadataResponse(false, 15);
billingApiService.getOrganizationBillingMetadata
.mockResolvedValueOnce(mockResponse1)
.mockResolvedValueOnce(mockResponse2);
const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2);
expect(result1).toEqual(mockResponse1);
expect(result2).toEqual(mockResponse2);
});
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledWith(
mockOrganizationId,
);
expect(result).toEqual(mockResponse);
});
describe("feature flag ON", () => {
beforeEach(() => {
featureFlagSubject.next(true);
});
it("calls getOrganizationBillingMetadataSelfHost when isSelfHost is true", async () => {
platformUtilsService.isSelfHost.mockReturnValue(true);
const mockResponse = createMockMetadataResponse(true, 25);
billingApiService.getOrganizationBillingMetadataSelfHost.mockResolvedValue(mockResponse);
it("calls getOrganizationBillingMetadataVNext when feature flag is on", async () => {
const mockResponse = createMockMetadataResponse(true, 15);
billingApiService.getOrganizationBillingMetadataVNext.mockResolvedValue(mockResponse);
const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
expect(configService.getFeatureFlag$).toHaveBeenCalledWith(
FeatureFlag.PM25379_UseNewOrganizationMetadataStructure,
);
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledWith(
mockOrganizationId,
);
expect(billingApiService.getOrganizationBillingMetadata).not.toHaveBeenCalled();
expect(result).toEqual(mockResponse);
});
it("caches metadata by organization ID when feature flag is on", async () => {
const mockResponse = createMockMetadataResponse(true, 10);
billingApiService.getOrganizationBillingMetadataVNext.mockResolvedValue(mockResponse);
const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(1);
expect(result1).toEqual(mockResponse);
expect(result2).toEqual(mockResponse);
});
it("maintains separate cache entries for different organization IDs", async () => {
const mockResponse1 = createMockMetadataResponse(true, 10);
const mockResponse2 = createMockMetadataResponse(false, 20);
billingApiService.getOrganizationBillingMetadataVNext
.mockResolvedValueOnce(mockResponse1)
.mockResolvedValueOnce(mockResponse2);
const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId2));
const result3 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result4 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId2));
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(2);
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenNthCalledWith(
1,
mockOrganizationId,
);
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenNthCalledWith(
2,
mockOrganizationId2,
);
expect(result1).toEqual(mockResponse1);
expect(result2).toEqual(mockResponse2);
expect(result3).toEqual(mockResponse1);
expect(result4).toEqual(mockResponse2);
});
it("calls getOrganizationBillingMetadataVNextSelfHost when feature flag is on and isSelfHost is true", async () => {
platformUtilsService.isSelfHost.mockReturnValue(true);
const mockResponse = createMockMetadataResponse(true, 25);
billingApiService.getOrganizationBillingMetadataVNextSelfHost.mockResolvedValue(
mockResponse,
);
const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
expect(platformUtilsService.isSelfHost).toHaveBeenCalled();
expect(billingApiService.getOrganizationBillingMetadataVNextSelfHost).toHaveBeenCalledWith(
mockOrganizationId,
);
expect(billingApiService.getOrganizationBillingMetadataVNext).not.toHaveBeenCalled();
expect(billingApiService.getOrganizationBillingMetadata).not.toHaveBeenCalled();
expect(result).toEqual(mockResponse);
});
expect(platformUtilsService.isSelfHost).toHaveBeenCalled();
expect(billingApiService.getOrganizationBillingMetadataSelfHost).toHaveBeenCalledWith(
mockOrganizationId,
);
expect(billingApiService.getOrganizationBillingMetadata).not.toHaveBeenCalled();
expect(result).toEqual(mockResponse);
});
describe("shareReplay behavior", () => {
beforeEach(() => {
featureFlagSubject.next(true);
});
it("caches metadata by organization ID", async () => {
const mockResponse = createMockMetadataResponse(true, 10);
billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockResponse);
it("does not call API multiple times when the same cached observable is subscribed to multiple times", async () => {
const mockResponse = createMockMetadataResponse(true, 10);
billingApiService.getOrganizationBillingMetadataVNext.mockResolvedValue(mockResponse);
const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const metadata$ = service.getOrganizationMetadata$(mockOrganizationId);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(1);
expect(result1).toEqual(mockResponse);
expect(result2).toEqual(mockResponse);
});
const subscription1Promise = firstValueFrom(metadata$);
const subscription2Promise = firstValueFrom(metadata$);
const subscription3Promise = firstValueFrom(metadata$);
it("maintains separate cache entries for different organization IDs", async () => {
const mockResponse1 = createMockMetadataResponse(true, 10);
const mockResponse2 = createMockMetadataResponse(false, 20);
billingApiService.getOrganizationBillingMetadata
.mockResolvedValueOnce(mockResponse1)
.mockResolvedValueOnce(mockResponse2);
const [result1, result2, result3] = await Promise.all([
subscription1Promise,
subscription2Promise,
subscription3Promise,
]);
const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId2));
const result3 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId));
const result4 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId2));
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(1);
expect(result1).toEqual(mockResponse);
expect(result2).toEqual(mockResponse);
expect(result3).toEqual(mockResponse);
});
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenNthCalledWith(
1,
mockOrganizationId,
);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenNthCalledWith(
2,
mockOrganizationId2,
);
expect(result1).toEqual(mockResponse1);
expect(result2).toEqual(mockResponse2);
expect(result3).toEqual(mockResponse1);
expect(result4).toEqual(mockResponse2);
});
it("does not call API multiple times when the same cached observable is subscribed to multiple times", async () => {
const mockResponse = createMockMetadataResponse(true, 10);
billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockResponse);
const metadata$ = service.getOrganizationMetadata$(mockOrganizationId);
const subscription1Promise = firstValueFrom(metadata$);
const subscription2Promise = firstValueFrom(metadata$);
const subscription3Promise = firstValueFrom(metadata$);
const [result1, result2, result3] = await Promise.all([
subscription1Promise,
subscription2Promise,
subscription3Promise,
]);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(1);
expect(result1).toEqual(mockResponse);
expect(result2).toEqual(mockResponse);
expect(result3).toEqual(mockResponse);
});
});
describe("refreshMetadataCache", () => {
beforeEach(() => {
featureFlagSubject.next(true);
});
it("refreshes cached metadata when called with feature flag on", (done) => {
it("refreshes cached metadata when called", (done) => {
const mockResponse1 = createMockMetadataResponse(true, 10);
const mockResponse2 = createMockMetadataResponse(true, 20);
let invocationCount = 0;
billingApiService.getOrganizationBillingMetadataVNext
billingApiService.getOrganizationBillingMetadata
.mockResolvedValueOnce(mockResponse1)
.mockResolvedValueOnce(mockResponse2);
@@ -221,7 +149,7 @@ describe("DefaultOrganizationMetadataService", () => {
expect(result).toEqual(mockResponse1);
} else if (invocationCount === 2) {
expect(result).toEqual(mockResponse2);
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(2);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2);
subscription.unsubscribe();
done();
}
@@ -234,45 +162,13 @@ describe("DefaultOrganizationMetadataService", () => {
}, 10);
});
it("does trigger refresh when feature flag is disabled", async () => {
featureFlagSubject.next(false);
const mockResponse1 = createMockMetadataResponse(false, 10);
const mockResponse2 = createMockMetadataResponse(false, 20);
let invocationCount = 0;
billingApiService.getOrganizationBillingMetadata
.mockResolvedValueOnce(mockResponse1)
.mockResolvedValueOnce(mockResponse2);
const subscription = service.getOrganizationMetadata$(mockOrganizationId).subscribe({
next: () => {
invocationCount++;
},
});
// wait for initial invocation
await new Promise((resolve) => setTimeout(resolve, 10));
expect(invocationCount).toBe(1);
service.refreshMetadataCache();
await new Promise((resolve) => setTimeout(resolve, 10));
expect(invocationCount).toBe(2);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2);
subscription.unsubscribe();
});
it("bypasses cache when refreshing metadata", (done) => {
const mockResponse1 = createMockMetadataResponse(true, 10);
const mockResponse2 = createMockMetadataResponse(true, 20);
const mockResponse3 = createMockMetadataResponse(true, 30);
let invocationCount = 0;
billingApiService.getOrganizationBillingMetadataVNext
billingApiService.getOrganizationBillingMetadata
.mockResolvedValueOnce(mockResponse1)
.mockResolvedValueOnce(mockResponse2)
.mockResolvedValueOnce(mockResponse3);
@@ -289,7 +185,7 @@ describe("DefaultOrganizationMetadataService", () => {
service.refreshMetadataCache();
} else if (invocationCount === 3) {
expect(result).toEqual(mockResponse3);
expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(3);
expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(3);
subscription.unsubscribe();
done();
}

View File

@@ -1,10 +1,8 @@
import { BehaviorSubject, combineLatest, from, Observable, shareReplay, switchMap } from "rxjs";
import { BehaviorSubject, from, Observable, shareReplay, switchMap } from "rxjs";
import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { FeatureFlag } from "../../../enums/feature-flag.enum";
import { ConfigService } from "../../../platform/abstractions/config/config.service";
import { OrganizationId } from "../../../types/guid";
import { OrganizationMetadataServiceAbstraction } from "../../abstractions/organization-metadata.service.abstraction";
import { OrganizationBillingMetadataResponse } from "../../models/response/organization-billing-metadata.response";
@@ -17,7 +15,6 @@ export class DefaultOrganizationMetadataService implements OrganizationMetadataS
constructor(
private billingApiService: BillingApiServiceAbstraction,
private configService: ConfigService,
private platformUtilsService: PlatformUtilsService,
) {}
private refreshMetadataTrigger = new BehaviorSubject<void>(undefined);
@@ -28,50 +25,26 @@ export class DefaultOrganizationMetadataService implements OrganizationMetadataS
};
getOrganizationMetadata$(orgId: OrganizationId): Observable<OrganizationBillingMetadataResponse> {
return combineLatest([
this.refreshMetadataTrigger,
this.configService.getFeatureFlag$(FeatureFlag.PM25379_UseNewOrganizationMetadataStructure),
]).pipe(
switchMap(([_, featureFlagEnabled]) =>
featureFlagEnabled
? this.vNextGetOrganizationMetadataInternal$(orgId)
: this.getOrganizationMetadataInternal$(orgId),
),
);
}
private vNextGetOrganizationMetadataInternal$(
orgId: OrganizationId,
): Observable<OrganizationBillingMetadataResponse> {
const cacheHit = this.metadataCache.get(orgId);
if (cacheHit) {
return cacheHit;
}
const result = from(this.fetchMetadata(orgId, true)).pipe(
shareReplay({ bufferSize: 1, refCount: false }),
);
this.metadataCache.set(orgId, result);
return result;
}
private getOrganizationMetadataInternal$(
organizationId: OrganizationId,
): Observable<OrganizationBillingMetadataResponse> {
return from(this.fetchMetadata(organizationId, false)).pipe(
shareReplay({ bufferSize: 1, refCount: false }),
return this.refreshMetadataTrigger.pipe(
switchMap(() => {
const cacheHit = this.metadataCache.get(orgId);
if (cacheHit) {
return cacheHit;
}
const result = from(this.fetchMetadata(orgId)).pipe(
shareReplay({ bufferSize: 1, refCount: false }),
);
this.metadataCache.set(orgId, result);
return result;
}),
);
}
private async fetchMetadata(
organizationId: OrganizationId,
featureFlagEnabled: boolean,
): Promise<OrganizationBillingMetadataResponse> {
return featureFlagEnabled
? this.platformUtilsService.isSelfHost()
? await this.billingApiService.getOrganizationBillingMetadataVNextSelfHost(organizationId)
: await this.billingApiService.getOrganizationBillingMetadataVNext(organizationId)
return this.platformUtilsService.isSelfHost()
? await this.billingApiService.getOrganizationBillingMetadataSelfHost(organizationId)
: await this.billingApiService.getOrganizationBillingMetadata(organizationId);
}
}

View File

@@ -231,6 +231,7 @@ describe("DefaultSubscriptionPricingService", () => {
},
storage: {
price: 4,
provided: 1,
},
} as PremiumPlanResponse;
@@ -350,7 +351,7 @@ describe("DefaultSubscriptionPricingService", () => {
billingApiService.getPlans.mockResolvedValue(mockPlansResponse);
billingApiService.getPremiumPlan.mockResolvedValue(mockPremiumPlanResponse);
configService.getFeatureFlag$.mockReturnValue(of(false)); // Default to false (use hardcoded value)
configService.getFeatureFlag$.mockReturnValue(of(false));
setupEnvironmentService(environmentService);
service = new DefaultSubscriptionPricingService(
@@ -915,7 +916,7 @@ describe("DefaultSubscriptionPricingService", () => {
const testError = new Error("Premium plan API error");
errorBillingApiService.getPlans.mockResolvedValue(mockPlansResponse);
errorBillingApiService.getPremiumPlan.mockRejectedValue(testError);
errorConfigService.getFeatureFlag$.mockReturnValue(of(true)); // Enable feature flag to use premium plan API
errorConfigService.getFeatureFlag$.mockReturnValue(of(false));
setupEnvironmentService(errorEnvironmentService);
const errorService = new DefaultSubscriptionPricingService(
@@ -959,71 +960,16 @@ describe("DefaultSubscriptionPricingService", () => {
expect(getPlansResponse).toHaveBeenCalledTimes(1);
});
it("should share premium plan API response between multiple subscriptions when feature flag is enabled", () => {
// Create a new mock to avoid conflicts with beforeEach setup
const newBillingApiService = mock<BillingApiServiceAbstraction>();
const newConfigService = mock<ConfigService>();
const newEnvironmentService = mock<EnvironmentService>();
newBillingApiService.getPlans.mockResolvedValue(mockPlansResponse);
newBillingApiService.getPremiumPlan.mockResolvedValue(mockPremiumPlanResponse);
newConfigService.getFeatureFlag$.mockReturnValue(of(true));
setupEnvironmentService(newEnvironmentService);
const getPremiumPlanSpy = jest.spyOn(newBillingApiService, "getPremiumPlan");
// Create a new service instance with the feature flag enabled
const newService = new DefaultSubscriptionPricingService(
newBillingApiService,
newConfigService,
i18nService,
logService,
newEnvironmentService,
);
it("should share premium plan API response between multiple subscriptions", () => {
const getPremiumPlanSpy = jest.spyOn(billingApiService, "getPremiumPlan");
// Subscribe to the premium pricing tier multiple times
newService.getPersonalSubscriptionPricingTiers$().subscribe();
newService.getPersonalSubscriptionPricingTiers$().subscribe();
service.getPersonalSubscriptionPricingTiers$().subscribe();
service.getPersonalSubscriptionPricingTiers$().subscribe();
// API should only be called once due to shareReplay on premiumPlanResponse$
expect(getPremiumPlanSpy).toHaveBeenCalledTimes(1);
});
it("should use hardcoded premium price when feature flag is disabled", (done) => {
// Create a new mock to test from scratch
const newBillingApiService = mock<BillingApiServiceAbstraction>();
const newConfigService = mock<ConfigService>();
const newEnvironmentService = mock<EnvironmentService>();
newBillingApiService.getPlans.mockResolvedValue(mockPlansResponse);
newBillingApiService.getPremiumPlan.mockResolvedValue({
seat: { price: 999 }, // Different price to verify hardcoded value is used
storage: { price: 999 },
} as PremiumPlanResponse);
newConfigService.getFeatureFlag$.mockReturnValue(of(false));
setupEnvironmentService(newEnvironmentService);
// Create a new service instance with the feature flag disabled
const newService = new DefaultSubscriptionPricingService(
newBillingApiService,
newConfigService,
i18nService,
logService,
newEnvironmentService,
);
// Subscribe with feature flag disabled
newService.getPersonalSubscriptionPricingTiers$().subscribe((tiers) => {
const premiumTier = tiers.find(
(tier) => tier.id === PersonalSubscriptionPricingTierIds.Premium,
);
// Should use hardcoded value of 10, not the API response value of 999
expect(premiumTier!.passwordManager.annualPrice).toBe(10);
expect(premiumTier!.passwordManager.annualPricePerAdditionalStorageGB).toBe(4);
done();
});
});
});
describe("Self-hosted environment behavior", () => {
@@ -1035,7 +981,7 @@ describe("DefaultSubscriptionPricingService", () => {
const getPlansSpy = jest.spyOn(selfHostedBillingApiService, "getPlans");
const getPremiumPlanSpy = jest.spyOn(selfHostedBillingApiService, "getPremiumPlan");
selfHostedConfigService.getFeatureFlag$.mockReturnValue(of(true));
selfHostedConfigService.getFeatureFlag$.mockReturnValue(of(false));
setupEnvironmentService(selfHostedEnvironmentService, Region.SelfHosted);
const selfHostedService = new DefaultSubscriptionPricingService(
@@ -1061,7 +1007,7 @@ describe("DefaultSubscriptionPricingService", () => {
const selfHostedConfigService = mock<ConfigService>();
const selfHostedEnvironmentService = mock<EnvironmentService>();
selfHostedConfigService.getFeatureFlag$.mockReturnValue(of(true));
selfHostedConfigService.getFeatureFlag$.mockReturnValue(of(false));
setupEnvironmentService(selfHostedEnvironmentService, Region.SelfHosted);
const selfHostedService = new DefaultSubscriptionPricingService(

View File

@@ -33,16 +33,6 @@ import {
} from "../types/subscription-pricing-tier";
export class DefaultSubscriptionPricingService implements SubscriptionPricingServiceAbstraction {
/**
* Fallback premium pricing used when the feature flag is disabled.
* These values represent the legacy pricing model and will not reflect
* server-side price changes. They are retained for backward compatibility
* during the feature flag rollout period.
*/
private static readonly FALLBACK_PREMIUM_SEAT_PRICE = 10;
private static readonly FALLBACK_PREMIUM_STORAGE_PRICE = 4;
private static readonly FALLBACK_PREMIUM_PROVIDED_STORAGE_GB = 1;
constructor(
private billingApiService: BillingApiServiceAbstraction,
private configService: ConfigService,
@@ -123,45 +113,27 @@ export class DefaultSubscriptionPricingService implements SubscriptionPricingSer
shareReplay({ bufferSize: 1, refCount: false }),
);
private premium$: Observable<PersonalSubscriptionPricingTier> = this.configService
.getFeatureFlag$(FeatureFlag.PM26793_FetchPremiumPriceFromPricingService)
.pipe(
take(1), // Lock behavior at first subscription to prevent switching data sources mid-stream
switchMap((fetchPremiumFromPricingService) =>
fetchPremiumFromPricingService
? this.premiumPlanResponse$.pipe(
map((premiumPlan) => ({
seat: premiumPlan.seat?.price,
storage: premiumPlan.storage?.price,
provided: premiumPlan.storage?.provided,
})),
)
: of({
seat: DefaultSubscriptionPricingService.FALLBACK_PREMIUM_SEAT_PRICE,
storage: DefaultSubscriptionPricingService.FALLBACK_PREMIUM_STORAGE_PRICE,
provided: DefaultSubscriptionPricingService.FALLBACK_PREMIUM_PROVIDED_STORAGE_GB,
}),
),
map((premiumPrices) => ({
id: PersonalSubscriptionPricingTierIds.Premium,
name: this.i18nService.t("premium"),
description: this.i18nService.t("advancedOnlineSecurity"),
availableCadences: [SubscriptionCadenceIds.Annually],
passwordManager: {
type: "standalone",
annualPrice: premiumPrices.seat,
annualPricePerAdditionalStorageGB: premiumPrices.storage,
providedStorageGB: premiumPrices.provided,
features: [
this.featureTranslations.builtInAuthenticator(),
this.featureTranslations.secureFileStorage(),
this.featureTranslations.emergencyAccess(),
this.featureTranslations.breachMonitoring(),
this.featureTranslations.andMoreFeatures(),
],
},
})),
);
private premium$: Observable<PersonalSubscriptionPricingTier> = this.premiumPlanResponse$.pipe(
map((premiumPlan) => ({
id: PersonalSubscriptionPricingTierIds.Premium,
name: this.i18nService.t("premium"),
description: this.i18nService.t("advancedOnlineSecurity"),
availableCadences: [SubscriptionCadenceIds.Annually],
passwordManager: {
type: "standalone",
annualPrice: premiumPlan.seat?.price,
annualPricePerAdditionalStorageGB: premiumPlan.storage?.price,
providedStorageGB: premiumPlan.storage?.provided,
features: [
this.featureTranslations.builtInAuthenticator(),
this.featureTranslations.secureFileStorage(),
this.featureTranslations.emergencyAccess(),
this.featureTranslations.breachMonitoring(),
this.featureTranslations.andMoreFeatures(),
],
},
})),
);
private families$: Observable<PersonalSubscriptionPricingTier> =
this.organizationPlansResponse$.pipe(

View File

@@ -5,4 +5,5 @@ export enum EventSystemUser {
SCIM = 1,
DomainVerification = 2,
PublicApi = 3,
BitwardenPortal = 5,
}

View File

@@ -60,6 +60,7 @@ export enum EventType {
OrganizationUser_RejectedAuthRequest = 1514,
OrganizationUser_Deleted = 1515,
OrganizationUser_Left = 1516,
OrganizationUser_AutomaticallyConfirmed = 1517,
Organization_Updated = 1600,
Organization_PurgedVault = 1601,
@@ -81,6 +82,10 @@ export enum EventType {
Organization_CollectionManagement_AllowAdminAccessToAllCollectionItemsDisabled = 1617,
Organization_ItemOrganization_Accepted = 1618,
Organization_ItemOrganization_Declined = 1619,
Organization_AutoConfirmEnabled_Admin = 1620,
Organization_AutoConfirmDisabled_Admin = 1621,
Organization_AutoConfirmEnabled_Portal = 1622,
Organization_AutoConfirmDisabled_Portal = 1623,
Policy_Updated = 1700,

View File

@@ -12,15 +12,14 @@ import { ServerConfig } from "../platform/abstractions/config/server-config";
export enum FeatureFlag {
/* Admin Console Team */
AutoConfirm = "pm-19934-auto-confirm-organization-users",
BlockClaimedDomainAccountCreation = "pm-28297-block-uninvited-claimed-domain-registration",
DefaultUserCollectionRestore = "pm-30883-my-items-restored-users",
MembersComponentRefactor = "pm-29503-refactor-members-inheritance",
BulkReinviteUI = "pm-28416-bulk-reinvite-ux-improvements",
/* Auth */
PM23801_PrefetchPasswordPrelogin = "pm-23801-prefetch-password-prelogin",
PM27086_UpdateAuthenticationApisForInputPassword = "pm-27086-update-authentication-apis-for-input-password",
SafariAccountSwitching = "pm-5594-safari-account-switching",
PM31088_MasterPasswordServiceEmitSalt = "pm-31088-master-password-service-emit-salt",
/* Autofill */
UseUndeterminedCipherScenarioTriggeringLogic = "undetermined-cipher-scenario-logic",
@@ -32,8 +31,6 @@ export enum FeatureFlag {
/* Billing */
TrialPaymentOptional = "PM-8163-trial-payment",
PM24032_NewNavigationPremiumUpgradeButton = "pm-24032-new-navigation-premium-upgrade-button",
PM25379_UseNewOrganizationMetadataStructure = "pm-25379-use-new-organization-metadata-structure",
PM26793_FetchPremiumPriceFromPricingService = "pm-26793-fetch-premium-price-from-pricing-service",
PM23713_PremiumBadgeOpensNewPremiumUpgradeDialog = "pm-23713-premium-badge-opens-new-premium-upgrade-dialog",
PM26462_Milestone_3 = "pm-26462-milestone-3",
PM23341_Milestone_2 = "pm-23341-milestone-2",
@@ -44,6 +41,7 @@ export enum FeatureFlag {
PrivateKeyRegeneration = "pm-12241-private-key-regeneration",
EnrollAeadOnKeyRotation = "enroll-aead-on-key-rotation",
ForceUpdateKDFSettings = "pm-18021-force-update-kdf-settings",
SdkKeyRotation = "pm-30144-sdk-key-rotation",
LinuxBiometricsV2 = "pm-26340-linux-biometrics-v2",
NoLogoutOnKdfChange = "pm-23995-no-logout-on-kdf-change",
PasskeyUnlock = "pm-2035-passkey-unlock",
@@ -55,7 +53,6 @@ export enum FeatureFlag {
/* Tools */
UseSdkPasswordGenerators = "pm-19976-use-sdk-password-generators",
ChromiumImporterWithABE = "pm-25855-chromium-importer-abe",
SendUIRefresh = "pm-28175-send-ui-refresh",
SendEmailOTP = "pm-19051-send-email-verification",
@@ -73,6 +70,11 @@ export enum FeatureFlag {
BrowserPremiumSpotlight = "pm-23384-browser-premium-spotlight",
MigrateMyVaultToMyItems = "pm-20558-migrate-myvault-to-myitems",
PM27632_SdkCipherCrudOperations = "pm-27632-cipher-crud-operations-to-sdk",
PM30521_AutofillButtonViewLoginScreen = "pm-30521-autofill-button-view-login-screen",
PM29438_WelcomeDialogWithExtensionPrompt = "pm-29438-welcome-dialog-with-extension-prompt",
PM29438_DialogWithExtensionPromptAccountAge = "pm-29438-dialog-with-extension-prompt-account-age",
PM29437_WelcomeDialog = "pm-29437-welcome-dialog-no-ext-prompt",
PM31039ItemActionInExtension = "pm-31039-item-action-in-extension",
/* Platform */
ContentScriptIpcChannelFramework = "content-script-ipc-channel-framework",
@@ -108,9 +110,7 @@ const FALSE = false as boolean;
export const DefaultFeatureFlagValue = {
/* Admin Console Team */
[FeatureFlag.AutoConfirm]: FALSE,
[FeatureFlag.BlockClaimedDomainAccountCreation]: FALSE,
[FeatureFlag.DefaultUserCollectionRestore]: FALSE,
[FeatureFlag.MembersComponentRefactor]: FALSE,
[FeatureFlag.BulkReinviteUI]: FALSE,
/* Autofill */
@@ -119,10 +119,10 @@ export const DefaultFeatureFlagValue = {
[FeatureFlag.WindowsDesktopAutotype]: FALSE,
[FeatureFlag.WindowsDesktopAutotypeGA]: FALSE,
[FeatureFlag.SSHAgentV2]: FALSE,
[FeatureFlag.PM31039ItemActionInExtension]: FALSE,
/* Tools */
[FeatureFlag.UseSdkPasswordGenerators]: FALSE,
[FeatureFlag.ChromiumImporterWithABE]: FALSE,
[FeatureFlag.SendUIRefresh]: FALSE,
[FeatureFlag.SendEmailOTP]: FALSE,
@@ -140,17 +140,20 @@ export const DefaultFeatureFlagValue = {
[FeatureFlag.BrowserPremiumSpotlight]: FALSE,
[FeatureFlag.PM27632_SdkCipherCrudOperations]: FALSE,
[FeatureFlag.MigrateMyVaultToMyItems]: FALSE,
[FeatureFlag.PM30521_AutofillButtonViewLoginScreen]: FALSE,
[FeatureFlag.PM29438_WelcomeDialogWithExtensionPrompt]: FALSE,
[FeatureFlag.PM29438_DialogWithExtensionPromptAccountAge]: 5,
[FeatureFlag.PM29437_WelcomeDialog]: FALSE,
/* Auth */
[FeatureFlag.PM23801_PrefetchPasswordPrelogin]: FALSE,
[FeatureFlag.PM27086_UpdateAuthenticationApisForInputPassword]: FALSE,
[FeatureFlag.SafariAccountSwitching]: FALSE,
[FeatureFlag.PM31088_MasterPasswordServiceEmitSalt]: FALSE,
/* Billing */
[FeatureFlag.TrialPaymentOptional]: FALSE,
[FeatureFlag.PM24032_NewNavigationPremiumUpgradeButton]: FALSE,
[FeatureFlag.PM25379_UseNewOrganizationMetadataStructure]: FALSE,
[FeatureFlag.PM26793_FetchPremiumPriceFromPricingService]: FALSE,
[FeatureFlag.PM23713_PremiumBadgeOpensNewPremiumUpgradeDialog]: FALSE,
[FeatureFlag.PM26462_Milestone_3]: FALSE,
[FeatureFlag.PM23341_Milestone_2]: FALSE,
@@ -161,6 +164,7 @@ export const DefaultFeatureFlagValue = {
[FeatureFlag.PrivateKeyRegeneration]: FALSE,
[FeatureFlag.EnrollAeadOnKeyRotation]: FALSE,
[FeatureFlag.ForceUpdateKDFSettings]: FALSE,
[FeatureFlag.SdkKeyRotation]: FALSE,
[FeatureFlag.LinuxBiometricsV2]: FALSE,
[FeatureFlag.NoLogoutOnKdfChange]: FALSE,
[FeatureFlag.PasskeyUnlock]: FALSE,

View File

@@ -35,4 +35,5 @@ export enum NotificationType {
ProviderBankAccountVerified = 24,
SyncPolicy = 25,
AutoConfirmMember = 26,
}

View File

@@ -161,13 +161,6 @@ export abstract class EncryptService {
decapsulationKey: Uint8Array,
): Promise<SymmetricCryptoKey>;
/**
* @deprecated Use @see {@link decapsulateKeyUnsigned} instead
* @param data - The ciphertext to decrypt
* @param privateKey - The privateKey to decrypt with
*/
abstract rsaDecrypt(data: EncString, privateKey: Uint8Array): Promise<Uint8Array>;
/**
* Generates a base64-encoded hash of the given value
* @param value The value to hash

View File

@@ -219,24 +219,4 @@ export class EncryptServiceImplementation implements EncryptService {
);
return new SymmetricCryptoKey(keyBytes);
}
async rsaDecrypt(data: EncString, privateKey: Uint8Array): Promise<Uint8Array> {
if (data == null) {
throw new Error("[Encrypt service] rsaDecrypt: No data provided for decryption.");
}
switch (data.encryptionType) {
case EncryptionType.Rsa2048_OaepSha1_B64:
case EncryptionType.Rsa2048_OaepSha1_HmacSha256_B64:
break;
default:
throw new Error("Invalid encryption type.");
}
if (privateKey == null) {
throw new Error("[Encrypt service] rsaDecrypt: No private key provided for decryption.");
}
return this.cryptoFunctionService.rsaDecrypt(data.dataBytes, privateKey, "sha1");
}
}

View File

@@ -17,8 +17,11 @@ import {
mockAccountServiceWith,
} from "../../../../spec";
import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason";
import { FeatureFlag } from "../../../enums/feature-flag.enum";
import { ServerConfig } from "../../../platform/abstractions/config/server-config";
import { LogService } from "../../../platform/abstractions/log.service";
import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key";
import { USER_SERVER_CONFIG } from "../../../platform/services/config/default-config.service";
import { UserId } from "../../../types/guid";
import { MasterKey, UserKey } from "../../../types/key";
import { KeyGenerationService } from "../../crypto";
@@ -92,14 +95,52 @@ describe("MasterPasswordService", () => {
sut.saltForUser$(null as unknown as UserId);
}).toThrow("userId is null or undefined.");
});
// Removable with unwinding of PM31088_MasterPasswordServiceEmitSalt
it("throws when userid present but not in account service", async () => {
await expect(
firstValueFrom(sut.saltForUser$("00000000-0000-0000-0000-000000000001" as UserId)),
).rejects.toThrow("Cannot read properties of undefined (reading 'email')");
});
it("returns salt", async () => {
const salt = await firstValueFrom(sut.saltForUser$(userId));
expect(salt).toBeDefined();
// Removable with unwinding of PM31088_MasterPasswordServiceEmitSalt
it("returns email-derived salt for legacy path", async () => {
const result = await firstValueFrom(sut.saltForUser$(userId));
// mockAccountServiceWith defaults email to "email"
expect(result).toBe("email" as MasterPasswordSalt);
});
describe("saltForUser$ master password unlock data migration path", () => {
// Flagged with PM31088_MasterPasswordServiceEmitSalt PM-31088
beforeEach(() => {
stateProvider.singleUser.getFake(userId, USER_SERVER_CONFIG).nextState({
featureStates: {
[FeatureFlag.PM31088_MasterPasswordServiceEmitSalt]: true,
},
} as unknown as ServerConfig);
});
// Unwinding should promote these tests as part of saltForUser suite.
it("returns salt from master password unlock data", async () => {
const expectedSalt = "custom-salt" as MasterPasswordSalt;
const unlockData = new MasterPasswordUnlockData(
expectedSalt,
new PBKDF2KdfConfig(600_000),
makeEncString().toSdk() as MasterKeyWrappedUserKey,
);
stateProvider.singleUser
.getFake(userId, MASTER_PASSWORD_UNLOCK_KEY)
.nextState(unlockData.toJSON());
const result = await firstValueFrom(sut.saltForUser$(userId));
expect(result).toBe(expectedSalt);
});
it("throws when master password unlock data is null", async () => {
stateProvider.singleUser.getFake(userId, MASTER_PASSWORD_UNLOCK_KEY).nextState(null);
await expect(firstValueFrom(sut.saltForUser$(userId))).rejects.toThrow(
"Master password unlock data not found for user.",
);
});
});
});

View File

@@ -1,6 +1,6 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { firstValueFrom, map, Observable } from "rxjs";
import { firstValueFrom, iif, map, Observable, switchMap } from "rxjs";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { assertNonNullish } from "@bitwarden/common/auth/utils";
@@ -12,8 +12,10 @@ import { KdfConfig } from "@bitwarden/key-management";
import { PureCrypto } from "@bitwarden/sdk-internal";
import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason";
import { FeatureFlag, getFeatureFlagValue } from "../../../enums/feature-flag.enum";
import { LogService } from "../../../platform/abstractions/log.service";
import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key";
import { USER_SERVER_CONFIG } from "../../../platform/services/config/default-config.service";
import {
MASTER_PASSWORD_DISK,
MASTER_PASSWORD_MEMORY,
@@ -102,9 +104,29 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr
saltForUser$(userId: UserId): Observable<MasterPasswordSalt> {
assertNonNullish(userId, "userId");
return this.accountService.accounts$.pipe(
map((accounts) => accounts[userId].email),
map((email) => this.emailToSalt(email)),
// Note: We can't use the config service as an abstraction here because it creates a circular dependency: ConfigService -> ConfigApiService -> ApiService -> VaultTimeoutSettingsService -> KeyService -> MP service.
return this.stateProvider.getUser(userId, USER_SERVER_CONFIG).state$.pipe(
map((serverConfig) =>
getFeatureFlagValue(serverConfig, FeatureFlag.PM31088_MasterPasswordServiceEmitSalt),
),
switchMap((enabled) =>
iif(
() => enabled,
this.masterPasswordUnlockData$(userId).pipe(
map((unlockData) => {
if (unlockData == null) {
throw new Error("Master password unlock data not found for user.");
}
return unlockData.salt;
}),
),
this.accountService.accounts$.pipe(
map((accounts) => accounts[userId].email),
map((email) => this.emailToSalt(email)),
),
),
),
);
}

View File

@@ -1,5 +1,3 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { EncString } from "../../key-management/crypto/models/enc-string";
import { Folder as FolderDomain } from "../../vault/models/domain/folder";
import { FolderView } from "../../vault/models/view/folder.view";
@@ -7,6 +5,8 @@ import { FolderView } from "../../vault/models/view/folder.view";
import { safeGetString } from "./utils";
export class FolderExport {
name: string = "";
static template(): FolderExport {
const req = new FolderExport();
req.name = "Folder name";
@@ -19,14 +19,12 @@ export class FolderExport {
}
static toDomain(req: FolderExport, domain = new FolderDomain()) {
domain.name = req.name != null ? new EncString(req.name) : null;
domain.name = new EncString(req.name);
return domain;
}
name: string;
// Use build method instead of ctor so that we can control order of JSON stringify for pretty print
build(o: FolderView | FolderDomain) {
this.name = safeGetString(o.name);
this.name = safeGetString(o.name ?? "") ?? "";
}
}

View File

@@ -0,0 +1,63 @@
import { NotificationType } from "../../enums";
import { AutoConfirmMemberNotification, NotificationResponse } from "./notification.response";
describe("NotificationResponse", () => {
describe("AutoConfirmMemberNotification", () => {
it("should parse AutoConfirmMemberNotification payload", () => {
const response = {
ContextId: "context-123",
Type: NotificationType.AutoConfirmMember,
Payload: {
TargetUserId: "target-user-id",
UserId: "user-id",
OrganizationId: "org-id",
},
};
const notification = new NotificationResponse(response);
expect(notification.type).toBe(NotificationType.AutoConfirmMember);
expect(notification.payload).toBeInstanceOf(AutoConfirmMemberNotification);
expect(notification.payload.targetUserId).toBe("target-user-id");
expect(notification.payload.userId).toBe("user-id");
expect(notification.payload.organizationId).toBe("org-id");
});
it("should handle stringified JSON payload", () => {
const response = {
ContextId: "context-123",
Type: NotificationType.AutoConfirmMember,
Payload: JSON.stringify({
TargetUserId: "target-user-id-2",
UserId: "user-id-2",
OrganizationId: "org-id-2",
}),
};
const notification = new NotificationResponse(response);
expect(notification.type).toBe(NotificationType.AutoConfirmMember);
expect(notification.payload).toBeInstanceOf(AutoConfirmMemberNotification);
expect(notification.payload.targetUserId).toBe("target-user-id-2");
expect(notification.payload.userId).toBe("user-id-2");
expect(notification.payload.organizationId).toBe("org-id-2");
});
});
describe("AutoConfirmMemberNotification constructor", () => {
it("should extract all properties from response", () => {
const response = {
TargetUserId: "target-user-id",
UserId: "user-id",
OrganizationId: "org-id",
};
const notification = new AutoConfirmMemberNotification(response);
expect(notification.targetUserId).toBe("target-user-id");
expect(notification.userId).toBe("user-id");
expect(notification.organizationId).toBe("org-id");
});
});
});

View File

@@ -75,6 +75,9 @@ export class NotificationResponse extends BaseResponse {
case NotificationType.SyncPolicy:
this.payload = new SyncPolicyNotification(payload);
break;
case NotificationType.AutoConfirmMember:
this.payload = new AutoConfirmMemberNotification(payload);
break;
default:
break;
}
@@ -210,3 +213,16 @@ export class LogOutNotification extends BaseResponse {
this.reason = this.getResponseProperty("Reason");
}
}
export class AutoConfirmMemberNotification extends BaseResponse {
userId: string;
targetUserId: string;
organizationId: string;
constructor(response: any) {
super(response);
this.targetUserId = this.getResponseProperty("TargetUserId");
this.userId = this.getResponseProperty("UserId");
this.organizationId = this.getResponseProperty("OrganizationId");
}
}

View File

@@ -417,6 +417,142 @@ describe("Utils Service", () => {
// });
});
describe("fromArrayToHex(...)", () => {
const originalIsNode = Utils.isNode;
afterEach(() => {
Utils.isNode = originalIsNode;
});
runInBothEnvironments("should convert a Uint8Array to a hex string", () => {
const arr = new Uint8Array([0x00, 0x01, 0x02, 0x0a, 0xff]);
const hexString = Utils.fromArrayToHex(arr);
expect(hexString).toBe("0001020aff");
});
runInBothEnvironments("should return null for null input", () => {
const hexString = Utils.fromArrayToHex(null);
expect(hexString).toBeNull();
});
runInBothEnvironments("should return empty string for an empty Uint8Array", () => {
const arr = new Uint8Array([]);
const hexString = Utils.fromArrayToHex(arr);
expect(hexString).toBe("");
});
});
describe("fromArrayToB64(...)", () => {
const originalIsNode = Utils.isNode;
afterEach(() => {
Utils.isNode = originalIsNode;
});
runInBothEnvironments("should convert a Uint8Array to a b64 string", () => {
const arr = new Uint8Array(asciiHelloWorldArray);
const b64String = Utils.fromArrayToB64(arr);
expect(b64String).toBe(b64HelloWorldString);
});
runInBothEnvironments("should return null for null input", () => {
const b64String = Utils.fromArrayToB64(null);
expect(b64String).toBeNull();
});
runInBothEnvironments("should return empty string for an empty Uint8Array", () => {
const arr = new Uint8Array([]);
const b64String = Utils.fromArrayToB64(arr);
expect(b64String).toBe("");
});
});
describe("fromArrayToUrlB64(...)", () => {
const originalIsNode = Utils.isNode;
afterEach(() => {
Utils.isNode = originalIsNode;
});
runInBothEnvironments("should convert a Uint8Array to a URL-safe b64 string", () => {
// Input that produces +, /, and = in standard base64
const arr = new Uint8Array([251, 255, 254]);
const urlB64String = Utils.fromArrayToUrlB64(arr);
// Standard b64 would be "+//+" with padding, URL-safe removes padding and replaces chars
expect(urlB64String).not.toContain("+");
expect(urlB64String).not.toContain("/");
expect(urlB64String).not.toContain("=");
});
runInBothEnvironments("should return null for null input", () => {
const urlB64String = Utils.fromArrayToUrlB64(null);
expect(urlB64String).toBeNull();
});
runInBothEnvironments("should return empty string for an empty Uint8Array", () => {
const arr = new Uint8Array([]);
const urlB64String = Utils.fromArrayToUrlB64(arr);
expect(urlB64String).toBe("");
});
});
describe("fromArrayToByteString(...)", () => {
const originalIsNode = Utils.isNode;
afterEach(() => {
Utils.isNode = originalIsNode;
});
runInBothEnvironments("should convert a Uint8Array to a byte string", () => {
const arr = new Uint8Array(asciiHelloWorldArray);
const byteString = Utils.fromArrayToByteString(arr);
expect(byteString).toBe(asciiHelloWorld);
});
runInBothEnvironments("should return null for null input", () => {
const byteString = Utils.fromArrayToByteString(null);
expect(byteString).toBeNull();
});
runInBothEnvironments("should return empty string for an empty Uint8Array", () => {
const arr = new Uint8Array([]);
const byteString = Utils.fromArrayToByteString(arr);
expect(byteString).toBe("");
});
});
describe("fromArrayToUtf8(...)", () => {
const originalIsNode = Utils.isNode;
afterEach(() => {
Utils.isNode = originalIsNode;
});
runInBothEnvironments("should convert a Uint8Array to a UTF-8 string", () => {
const arr = new Uint8Array(asciiHelloWorldArray);
const utf8String = Utils.fromArrayToUtf8(arr);
expect(utf8String).toBe(asciiHelloWorld);
});
runInBothEnvironments("should return null for null input", () => {
const utf8String = Utils.fromArrayToUtf8(null);
expect(utf8String).toBeNull();
});
runInBothEnvironments("should return empty string for an empty Uint8Array", () => {
const arr = new Uint8Array([]);
const utf8String = Utils.fromArrayToUtf8(arr);
expect(utf8String).toBe("");
});
runInBothEnvironments("should handle multi-byte UTF-8 characters", () => {
// "日本" in UTF-8 bytes
const arr = new Uint8Array([0xe6, 0x97, 0xa5, 0xe6, 0x9c, 0xac]);
const utf8String = Utils.fromArrayToUtf8(arr);
expect(utf8String).toBe("日本");
});
});
describe("Base64 and ArrayBuffer round trip conversions", () => {
const originalIsNode = Utils.isNode;
@@ -447,10 +583,10 @@ describe("Utils Service", () => {
"should correctly round trip convert from base64 to ArrayBuffer and back",
() => {
// Convert known base64 string to ArrayBuffer
const bufferFromB64 = Utils.fromB64ToArray(b64HelloWorldString).buffer;
const bufferFromB64 = Utils.fromB64ToArray(b64HelloWorldString);
// Convert the ArrayBuffer back to a base64 string
const roundTrippedB64String = Utils.fromBufferToB64(bufferFromB64);
const roundTrippedB64String = Utils.fromArrayToB64(bufferFromB64);
// Compare the original base64 string with the round-tripped base64 string
expect(roundTrippedB64String).toBe(b64HelloWorldString);

View File

@@ -8,6 +8,8 @@ import { Observable, of, switchMap } from "rxjs";
import { getHostname, parse } from "tldts";
import { Merge } from "type-fest";
import "core-js/proposals/array-buffer-base64";
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
// eslint-disable-next-line no-restricted-imports
import { KeyService } from "@bitwarden/key-management";
@@ -129,6 +131,78 @@ export class Utils {
return arr;
}
/**
* Converts a Uint8Array to a hexadecimal string.
* @param arr - The Uint8Array to convert.
* @returns The hexadecimal string representation, or null if the input is null.
*/
static fromArrayToHex(arr: Uint8Array | null): string | null {
if (arr == null) {
return null;
}
// @ts-expect-error - polyfilled by core-js
return arr.toHex();
}
/**
* Converts a Uint8Array to a Base64 encoded string.
* @param arr - The Uint8Array to convert.
* @returns The Base64 encoded string, or null if the input is null.
*/
static fromArrayToB64(arr: Uint8Array | null): string | null {
if (arr == null) {
return null;
}
// @ts-expect-error - polyfilled by core-js
return arr.toBase64({ alphabet: "base64" });
}
/**
* Converts a Uint8Array to a URL-safe Base64 encoded string.
* @param arr - The Uint8Array to convert.
* @returns The URL-safe Base64 encoded string, or null if the input is null.
*/
static fromArrayToUrlB64(arr: Uint8Array | null): string | null {
if (arr == null) {
return null;
}
// @ts-expect-error - polyfilled by core-js
return arr.toBase64({ alphabet: "base64url" });
}
/**
* Converts a Uint8Array to a byte string (each byte as a character).
* @param arr - The Uint8Array to convert.
* @returns The byte string representation, or null if the input is null.
*/
static fromArrayToByteString(arr: Uint8Array | null): string | null {
if (arr == null) {
return null;
}
let byteString = "";
for (let i = 0; i < arr.length; i++) {
byteString += String.fromCharCode(arr[i]);
}
return byteString;
}
/**
* Converts a Uint8Array to a UTF-8 decoded string.
* @param arr - The Uint8Array containing UTF-8 encoded bytes.
* @returns The decoded UTF-8 string, or null if the input is null.
*/
static fromArrayToUtf8(arr: Uint8Array | null): string | null {
if (arr == null) {
return null;
}
return BufferLib.from(arr).toString("utf8");
}
/**
* Convert binary data into a Base64 string.
*
@@ -302,7 +376,7 @@ export class Utils {
}
static fromUtf8ToUrlB64(utfStr: string): string {
return Utils.fromBufferToUrlB64(Utils.fromUtf8ToArray(utfStr));
return Utils.fromArrayToUrlB64(Utils.fromUtf8ToArray(utfStr));
}
static fromB64ToUtf8(b64Str: string): string {

View File

@@ -3,6 +3,7 @@ import { BehaviorSubject, bufferCount, firstValueFrom, Subject, ObservedValueOf
// eslint-disable-next-line no-restricted-imports
import { LogoutReason } from "@bitwarden/auth/common";
import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm";
import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction";
@@ -36,6 +37,7 @@ describe("DefaultServerNotificationsService (multi-user)", () => {
let authRequestAnsweringService: MockProxy<AuthRequestAnsweringService>;
let configService: MockProxy<ConfigService>;
let policyService: MockProxy<InternalPolicyService>;
let autoConfirmService: MockProxy<AutomaticUserConfirmationService>;
let activeUserAccount$: BehaviorSubject<ObservedValueOf<AccountService["activeAccount$"]>>;
let userAccounts$: BehaviorSubject<ObservedValueOf<AccountService["accounts$"]>>;
@@ -131,6 +133,8 @@ describe("DefaultServerNotificationsService (multi-user)", () => {
policyService = mock<InternalPolicyService>();
autoConfirmService = mock<AutomaticUserConfirmationService>();
defaultServerNotificationsService = new DefaultServerNotificationsService(
mock<LogService>(),
syncService,
@@ -145,6 +149,7 @@ describe("DefaultServerNotificationsService (multi-user)", () => {
authRequestAnsweringService,
configService,
policyService,
autoConfirmService,
);
});

View File

@@ -4,6 +4,7 @@ import { BehaviorSubject, bufferCount, firstValueFrom, ObservedValueOf, of, Subj
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
// eslint-disable-next-line no-restricted-imports
import { LogoutReason } from "@bitwarden/auth/common";
import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm";
import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction";
@@ -45,6 +46,7 @@ describe("NotificationsService", () => {
let authRequestAnsweringService: MockProxy<AuthRequestAnsweringService>;
let configService: MockProxy<ConfigService>;
let policyService: MockProxy<InternalPolicyService>;
let autoConfirmService: MockProxy<AutomaticUserConfirmationService>;
let activeAccount: BehaviorSubject<ObservedValueOf<AccountService["activeAccount$"]>>;
let accounts: BehaviorSubject<ObservedValueOf<AccountService["accounts$"]>>;
@@ -75,6 +77,7 @@ describe("NotificationsService", () => {
authRequestAnsweringService = mock<AuthRequestAnsweringService>();
configService = mock<ConfigService>();
policyService = mock<InternalPolicyService>();
autoConfirmService = mock<AutomaticUserConfirmationService>();
// For these tests, use the active-user implementation (feature flag disabled)
configService.getFeatureFlag$.mockImplementation(() => of(true));
@@ -128,6 +131,7 @@ describe("NotificationsService", () => {
authRequestAnsweringService,
configService,
policyService,
autoConfirmService,
);
});
@@ -507,5 +511,29 @@ describe("NotificationsService", () => {
});
});
});
describe("NotificationType.AutoConfirmMember", () => {
it("should call autoConfirmService.autoConfirmUser with correct parameters", async () => {
autoConfirmService.autoConfirmUser.mockResolvedValue();
const notification = new NotificationResponse({
type: NotificationType.AutoConfirmMember,
payload: {
UserId: mockUser1,
TargetUserId: "target-user-id",
OrganizationId: "org-id",
},
contextId: "different-app-id",
});
await sut["processNotification"](notification, mockUser1);
expect(autoConfirmService.autoConfirmUser).toHaveBeenCalledWith(
mockUser1,
"target-user-id",
"org-id",
);
});
});
});
});

View File

@@ -15,6 +15,7 @@ import {
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
// eslint-disable-next-line no-restricted-imports
import { LogoutReason } from "@bitwarden/auth/common";
import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm";
import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyData } from "@bitwarden/common/admin-console/models/data/policy.data";
import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction";
@@ -49,6 +50,7 @@ export const DISABLED_NOTIFICATIONS_URL = "http://-";
export const AllowedMultiUserNotificationTypes = new Set<NotificationType>([
NotificationType.AuthRequest,
NotificationType.AutoConfirmMember,
]);
export class DefaultServerNotificationsService implements ServerNotificationsService {
@@ -70,6 +72,7 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer
private readonly authRequestAnsweringService: AuthRequestAnsweringService,
private readonly configService: ConfigService,
private readonly policyService: InternalPolicyService,
private autoConfirmService: AutomaticUserConfirmationService,
) {
this.notifications$ = this.accountService.accounts$.pipe(
map((accounts: Record<UserId, AccountInfo>): Set<UserId> => {
@@ -292,6 +295,13 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer
case NotificationType.SyncPolicy:
await this.policyService.syncPolicy(PolicyData.fromPolicy(notification.payload.policy));
break;
case NotificationType.AutoConfirmMember:
await this.autoConfirmService.autoConfirmUser(
notification.payload.userId,
notification.payload.targetUserId,
notification.payload.organizationId,
);
break;
default:
break;
}

View File

@@ -183,6 +183,8 @@ export class DefaultSyncService extends CoreSyncService {
const response = await this.inFlightApiCalls.sync;
await this.cipherService.clear(response.profile.id);
await this.syncUserDecryption(response.profile.id, response.userDecryption);
await this.syncProfile(response.profile);
await this.syncFolders(response.folders, response.profile.id);

View File

@@ -115,6 +115,7 @@ import { CipherRequest } from "../vault/models/request/cipher.request";
import { AttachmentUploadDataResponse } from "../vault/models/response/attachment-upload-data.response";
import { AttachmentResponse } from "../vault/models/response/attachment.response";
import { CipherResponse } from "../vault/models/response/cipher.response";
import { DeleteAttachmentResponse } from "../vault/models/response/delete-attachment.response";
import { OptionalCipherResponse } from "../vault/models/response/optional-cipher.response";
import { InsecureUrlNotAllowedError } from "./api-errors";
@@ -590,18 +591,32 @@ export class ApiService implements ApiServiceAbstraction {
return new AttachmentUploadDataResponse(r);
}
deleteCipherAttachment(id: string, attachmentId: string): Promise<any> {
return this.send("DELETE", "/ciphers/" + id + "/attachment/" + attachmentId, null, true, true);
async deleteCipherAttachment(
id: string,
attachmentId: string,
): Promise<DeleteAttachmentResponse> {
const r = await this.send(
"DELETE",
"/ciphers/" + id + "/attachment/" + attachmentId,
null,
true,
true,
);
return new DeleteAttachmentResponse(r);
}
deleteCipherAttachmentAdmin(id: string, attachmentId: string): Promise<any> {
return this.send(
async deleteCipherAttachmentAdmin(
id: string,
attachmentId: string,
): Promise<DeleteAttachmentResponse> {
const r = await this.send(
"DELETE",
"/ciphers/" + id + "/attachment/" + attachmentId + "/admin",
null,
true,
true,
);
return new DeleteAttachmentResponse(r);
}
postShareCipherAttachment(

View File

@@ -105,7 +105,7 @@ export class SendApiService implements SendApiServiceAbstraction {
"POST",
"/sends/access/file/" + send.file.id,
null,
true,
false,
true,
apiUrl,
setAuthTokenHeader,

View File

@@ -2,7 +2,6 @@ import { Observable } from "rxjs";
import { SendView } from "../../tools/send/models/view/send.view";
import { IndexedEntityId, UserId } from "../../types/guid";
import { CipherView } from "../models/view/cipher.view";
import { CipherViewLike } from "../utils/cipher-view-like-utils";
export abstract class SearchService {
@@ -20,7 +19,7 @@ export abstract class SearchService {
abstract isSearchable(userId: UserId, query: string | null): Promise<boolean>;
abstract indexCiphers(
userId: UserId,
ciphersToIndex: CipherView[],
ciphersToIndex: CipherViewLike[],
indexedEntityGuid?: string,
): Promise<void>;
abstract searchCiphers<C extends CipherViewLike>(

View File

@@ -1,5 +1,3 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { Jsonify } from "type-fest";
import { FolderResponse } from "../response/folder.response";
@@ -10,12 +8,19 @@ export class FolderData {
revisionDate: string;
constructor(response: Partial<FolderResponse>) {
this.name = response?.name;
this.id = response?.id;
this.revisionDate = response?.revisionDate;
this.name = response.name ?? "";
this.id = response.id ?? "";
this.revisionDate = response.revisionDate ?? new Date().toISOString();
}
static fromJSON(obj: Jsonify<FolderData>) {
return Object.assign(new FolderData({}), obj);
static fromJSON(obj: Jsonify<FolderData | null>) {
if (obj == null) {
return null;
}
return new FolderData({
id: obj.id,
name: obj.name,
revisionDate: obj.revisionDate,
});
}
}

View File

@@ -8,7 +8,7 @@ import {
mockFromJson,
} from "../../../../spec";
import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service";
import { EncryptedString, EncString } from "../../../key-management/crypto/models/enc-string";
import { EncString } from "../../../key-management/crypto/models/enc-string";
import { FolderData } from "../../models/data/folder.data";
import { Folder } from "../../models/domain/folder";
@@ -49,6 +49,30 @@ describe("Folder", () => {
});
});
describe("constructor", () => {
it("initializes properties from FolderData", () => {
const revisionDate = new Date("2022-08-04T01:06:40.441Z");
const folder = new Folder({
id: "id",
name: "name",
revisionDate: revisionDate.toISOString(),
});
expect(folder.id).toBe("id");
expect(folder.revisionDate).toEqual(revisionDate);
expect(folder.name).toBeInstanceOf(EncString);
expect((folder.name as EncString).encryptedString).toBe("name");
});
it("initializes empty properties when no FolderData is provided", () => {
const folder = new Folder();
expect(folder.id).toBe("");
expect(folder.name).toBeInstanceOf(EncString);
expect(folder.revisionDate).toBeInstanceOf(Date);
});
});
describe("fromJSON", () => {
jest.mock("../../../key-management/crypto/models/enc-string");
jest.spyOn(EncString, "fromJSON").mockImplementation(mockFromJson);
@@ -57,17 +81,13 @@ describe("Folder", () => {
const revisionDate = new Date("2022-08-04T01:06:40.441Z");
const actual = Folder.fromJSON({
revisionDate: revisionDate.toISOString(),
name: "name" as EncryptedString,
name: "name",
id: "id",
});
const expected = {
revisionDate: revisionDate,
name: "name_fromJSON",
id: "id",
};
expect(actual).toMatchObject(expected);
expect(actual?.id).toBe("id");
expect(actual?.revisionDate).toEqual(revisionDate);
expect(actual?.name).toBe("name_fromJSON");
});
});
@@ -89,9 +109,7 @@ describe("Folder", () => {
const view = await folder.decryptWithKey(key, encryptService);
expect(view).toEqual({
name: "encName",
});
expect(view.name).toBe("encName");
});
it("assigns the folder id and revision date", async () => {

View File

@@ -1,5 +1,3 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { Jsonify } from "type-fest";
import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service";
@@ -9,16 +7,10 @@ import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-cr
import { FolderData } from "../data/folder.data";
import { FolderView } from "../view/folder.view";
export class Test extends Domain {
id: string;
name: EncString;
revisionDate: Date;
}
export class Folder extends Domain {
id: string;
name: EncString;
revisionDate: Date;
id: string = "";
name: EncString = new EncString("");
revisionDate: Date = new Date();
constructor(obj?: FolderData) {
super();
@@ -26,17 +18,9 @@ export class Folder extends Domain {
return;
}
this.buildDomainModel(
this,
obj,
{
id: null,
name: null,
},
["id"],
);
this.revisionDate = obj.revisionDate != null ? new Date(obj.revisionDate) : null;
this.id = obj.id;
this.name = new EncString(obj.name);
this.revisionDate = new Date(obj.revisionDate);
}
decrypt(key: SymmetricCryptoKey): Promise<FolderView> {
@@ -62,7 +46,14 @@ export class Folder extends Domain {
}
static fromJSON(obj: Jsonify<Folder>) {
const revisionDate = obj.revisionDate == null ? null : new Date(obj.revisionDate);
return Object.assign(new Folder(), obj, { name: EncString.fromJSON(obj.name), revisionDate });
if (obj == null) {
return null;
}
const folder = new Folder();
folder.id = obj.id;
folder.name = EncString.fromJSON(obj.name);
folder.revisionDate = new Date(obj.revisionDate);
return folder;
}
}

View File

@@ -1,12 +1,25 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { Folder } from "../domain/folder";
import { FolderRequest } from "./folder.request";
export class FolderWithIdRequest extends FolderRequest {
/**
* Declared as `string` (not `string | null`) to satisfy the
* {@link UserKeyRotationDataProvider}`<TRequest extends { id: string } | { organizationId: string }>`
* constraint on `FolderService`.
*
* At runtime this is `null` for new import folders. PR #17077 enforced strict type-checking on
* folder models, changing this assignment to `folder.id ?? ""` — causing the importer to send
* `{"id":""}` instead of `{"id":null}`, which the server rejected.
* The `|| null` below restores the pre-migration behavior while `@ts-strict-ignore` above
* allows the `null` assignment against the `string` declaration.
*/
id: string;
constructor(folder: Folder) {
super(folder);
this.id = folder.id;
this.id = folder.id || null;
}
}

View File

@@ -0,0 +1,12 @@
import { BaseResponse } from "../../../models/response/base.response";
import { CipherResponse } from "./cipher.response";
export class DeleteAttachmentResponse extends BaseResponse {
cipher: CipherResponse;
constructor(response: any) {
super(response);
this.cipher = new CipherResponse(this.getResponseProperty("Cipher"));
}
}

View File

@@ -1,19 +1,17 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { Jsonify } from "type-fest";
import { View } from "../../../models/view/view";
import { DecryptedObject } from "../../../platform/models/domain/domain-base";
import { Folder } from "../domain/folder";
import { ITreeNodeObject } from "../domain/tree-node";
export class FolderView implements View, ITreeNodeObject {
id: string = null;
name: string = null;
revisionDate: Date = null;
id: string = "";
name: string = "";
revisionDate: Date;
constructor(f?: Folder | DecryptedObject<Folder, "name">) {
constructor(f?: Folder) {
if (!f) {
this.revisionDate = new Date();
return;
}
@@ -22,7 +20,12 @@ export class FolderView implements View, ITreeNodeObject {
}
static fromJSON(obj: Jsonify<FolderView>) {
const revisionDate = obj.revisionDate == null ? null : new Date(obj.revisionDate);
return Object.assign(new FolderView(), obj, { revisionDate });
const folderView = new FolderView();
folderView.id = obj.id ?? "";
folderView.name = obj.name ?? "";
if (obj.revisionDate != null) {
folderView.revisionDate = new Date(obj.revisionDate);
}
return folderView;
}
}

View File

@@ -77,6 +77,7 @@ import { CipherShareRequest } from "../models/request/cipher-share.request";
import { CipherWithIdRequest } from "../models/request/cipher-with-id.request";
import { CipherRequest } from "../models/request/cipher.request";
import { CipherResponse } from "../models/response/cipher.response";
import { DeleteAttachmentResponse } from "../models/response/delete-attachment.response";
import { AttachmentView } from "../models/view/attachment.view";
import { CipherView } from "../models/view/cipher.view";
import { FieldView } from "../models/view/field.view";
@@ -173,13 +174,14 @@ export class CipherService implements CipherServiceAbstraction {
decryptStartTime = performance.now();
}),
switchMap(async (ciphers) => {
const [decrypted, failures] = await this.decryptCiphersWithSdk(ciphers, userId, false);
void this.setFailedDecryptedCiphers(failures, userId);
// Trigger full decryption and indexing in background
void this.getAllDecrypted(userId);
return decrypted;
return await this.decryptCiphersWithSdk(ciphers, userId, false);
}),
tap((decrypted) => {
tap(([decrypted, failures]) => {
void Promise.all([
this.setFailedDecryptedCiphers(failures, userId),
this.searchService.indexCiphers(userId, decrypted),
]);
this.logService.measure(
decryptStartTime,
"Vault",
@@ -188,10 +190,11 @@ export class CipherService implements CipherServiceAbstraction {
[["Items", decrypted.length]],
);
}),
map(([decrypted]) => decrypted),
);
}),
);
});
}, this.clearCipherViewsForUser$);
/**
* Observable that emits an array of decrypted ciphers for the active user.
@@ -530,6 +533,10 @@ export class CipherService implements CipherServiceAbstraction {
ciphers: Cipher[],
userId: UserId,
): Promise<[CipherView[], CipherView[]] | null> {
if (ciphers.length === 0) {
return [[], []];
}
if (await this.configService.getFeatureFlag(FeatureFlag.PM19941MigrateCipherDomainToSdk)) {
const decryptStartTime = performance.now();
@@ -1191,33 +1198,28 @@ export class CipherService implements CipherServiceAbstraction {
userId: UserId,
admin = false,
): Promise<Cipher> {
const encKey = await this.getKeyForCipherKeyDecryption(cipher, userId);
const cipherKeyEncryptionEnabled = await this.getCipherKeyEncryptionEnabled();
// The organization's symmetric key or the user's user key
const vaultKey = await this.getKeyForCipherKeyDecryption(cipher, userId);
const cipherEncKey =
cipherKeyEncryptionEnabled && cipher.key != null
? ((await this.encryptService.unwrapSymmetricKey(cipher.key, encKey)) as UserKey)
: encKey;
const cipherKeyOrVaultKey =
cipher.key != null
? ((await this.encryptService.unwrapSymmetricKey(cipher.key, vaultKey)) as UserKey)
: vaultKey;
//if cipher key encryption is disabled but the item has an individual key,
//then we rollback to using the user key as the main key of encryption of the item
//in order to keep item and it's attachments with the same encryption level
if (cipher.key != null && !cipherKeyEncryptionEnabled) {
const model = await this.decrypt(cipher, userId);
await this.updateWithServer(model, userId);
}
const encFileName = await this.encryptService.encryptString(filename, cipherKeyOrVaultKey);
const encFileName = await this.encryptService.encryptString(filename, cipherEncKey);
const dataEncKey = await this.keyService.makeDataEncKey(cipherEncKey);
const encData = await this.encryptService.encryptFileData(new Uint8Array(data), dataEncKey[0]);
const attachmentKey = await this.keyService.makeDataEncKey(cipherKeyOrVaultKey);
const encData = await this.encryptService.encryptFileData(
new Uint8Array(data),
attachmentKey[0],
);
const response = await this.cipherFileUploadService.upload(
cipher,
encFileName,
encData,
admin,
dataEncKey,
attachmentKey,
);
const cData = new CipherData(response, cipher.collectionIds);
@@ -1481,16 +1483,16 @@ export class CipherService implements CipherServiceAbstraction {
userId: UserId,
admin: boolean = false,
): Promise<CipherData> {
let cipherResponse = null;
let response: DeleteAttachmentResponse;
try {
cipherResponse = admin
response = admin
? await this.apiService.deleteCipherAttachmentAdmin(id, attachmentId)
: await this.apiService.deleteCipherAttachment(id, attachmentId);
} catch (e) {
return Promise.reject((e as ErrorResponse).getSingleMessage());
}
const cipherData = CipherData.fromJSON(cipherResponse?.cipher);
const cipherData = new CipherData(response.cipher);
return await this.deleteAttachment(id, cipherData.revisionDate, attachmentId, userId);
}
@@ -2406,6 +2408,12 @@ export class CipherService implements CipherServiceAbstraction {
userId: UserId,
fullDecryption: boolean = true,
): Promise<[CipherViewLike[], CipherView[]]> {
// Short-circuit if there are no ciphers to decrypt
// Observables reacting to key changes may attempt to decrypt with a stale SDK reference.
if (ciphers.length === 0) {
return [[], []];
}
if (fullDecryption) {
const [decryptedViews, failedViews] = await this.cipherEncryptionService.decryptManyLegacy(
ciphers,

View File

@@ -95,6 +95,7 @@ describe("DefaultCipherEncryptionService", () => {
vault: jest.fn().mockReturnValue({
ciphers: jest.fn().mockReturnValue({
encrypt: jest.fn(),
encrypt_list: jest.fn(),
encrypt_cipher_for_rotation: jest.fn(),
set_fido2_credentials: jest.fn(),
decrypt: jest.fn(),
@@ -280,10 +281,23 @@ describe("DefaultCipherEncryptionService", () => {
name: "encrypted-name-3",
} as unknown as Cipher;
mockSdkClient.vault().ciphers().encrypt.mockReturnValue({
cipher: sdkCipher,
encryptedFor: userId,
});
mockSdkClient
.vault()
.ciphers()
.encrypt_list.mockReturnValue([
{
cipher: sdkCipher,
encryptedFor: userId,
},
{
cipher: sdkCipher,
encryptedFor: userId,
},
{
cipher: sdkCipher,
encryptedFor: userId,
},
]);
jest
.spyOn(Cipher, "fromSdkCipher")
@@ -299,7 +313,8 @@ describe("DefaultCipherEncryptionService", () => {
expect(results[1].cipher).toEqual(expectedCipher2);
expect(results[2].cipher).toEqual(expectedCipher3);
expect(mockSdkClient.vault().ciphers().encrypt).toHaveBeenCalledTimes(3);
expect(mockSdkClient.vault().ciphers().encrypt_list).toHaveBeenCalledTimes(1);
expect(mockSdkClient.vault().ciphers().encrypt).not.toHaveBeenCalled();
expect(results[0].encryptedFor).toBe(userId);
expect(results[1].encryptedFor).toBe(userId);
@@ -311,7 +326,7 @@ describe("DefaultCipherEncryptionService", () => {
expect(results).toBeDefined();
expect(results.length).toBe(0);
expect(mockSdkClient.vault().ciphers().encrypt).not.toHaveBeenCalled();
expect(mockSdkClient.vault().ciphers().encrypt_list).not.toHaveBeenCalled();
});
});

View File

@@ -65,21 +65,14 @@ export class DefaultCipherEncryptionService implements CipherEncryptionService {
using ref = sdk.take();
const results: EncryptionContext[] = [];
// TODO: https://bitwarden.atlassian.net/browse/PM-30580
// Replace this loop with a native SDK encryptMany method for better performance.
for (const model of models) {
const sdkCipherView = this.toSdkCipherView(model, ref.value);
const encryptionContext = ref.value.vault().ciphers().encrypt(sdkCipherView);
results.push({
return ref.value
.vault()
.ciphers()
.encrypt_list(models.map((model) => this.toSdkCipherView(model, ref.value)))
.map((encryptionContext) => ({
cipher: Cipher.fromSdkCipher(encryptionContext.cipher)!,
encryptedFor: uuidAsString(encryptionContext.encryptedFor) as UserId,
});
}
return results;
}));
}),
catchError((error: unknown) => {
this.logService.error(`Failed to encrypt ciphers in batch: ${error}`);

View File

@@ -93,12 +93,12 @@ export class CipherFileUploadService implements CipherFileUploadServiceAbstracti
response: CipherResponse,
uploadData: AttachmentUploadDataResponse,
isAdmin: boolean,
) {
return () => {
): () => Promise<void> {
return async () => {
if (isAdmin) {
return this.apiService.deleteCipherAttachmentAdmin(response.id, uploadData.attachmentId);
await this.apiService.deleteCipherAttachmentAdmin(response.id, uploadData.attachmentId);
} else {
return this.apiService.deleteCipherAttachment(response.id, uploadData.attachmentId);
await this.apiService.deleteCipherAttachment(response.id, uploadData.attachmentId);
}
};
}

View File

@@ -17,11 +17,11 @@ export class FolderApiService implements FolderApiServiceAbstraction {
const request = new FolderRequest(folder);
let response: FolderResponse;
if (folder.id == null) {
if (folder.id) {
response = await this.putFolder(folder.id, request);
} else {
response = await this.postFolder(request);
folder.id = response.id;
} else {
response = await this.putFolder(folder.id, request);
}
const data = new FolderData(response);

View File

@@ -122,6 +122,7 @@ describe("Folder Service", () => {
encryptedString: "ENC",
encryptionType: 0,
},
revisionDate: expect.any(Date),
});
});
@@ -132,7 +133,7 @@ describe("Folder Service", () => {
expect(result).toEqual({
id: "1",
name: makeEncString("ENC_STRING_" + 1),
revisionDate: null,
revisionDate: expect.any(Date),
});
});
@@ -150,12 +151,12 @@ describe("Folder Service", () => {
{
id: "1",
name: makeEncString("ENC_STRING_" + 1),
revisionDate: null,
revisionDate: expect.any(Date),
},
{
id: "2",
name: makeEncString("ENC_STRING_" + 2),
revisionDate: null,
revisionDate: expect.any(Date),
},
]);
});
@@ -167,7 +168,7 @@ describe("Folder Service", () => {
{
id: "4",
name: makeEncString("ENC_STRING_" + 4),
revisionDate: null,
revisionDate: expect.any(Date),
},
]);
});
@@ -203,7 +204,7 @@ describe("Folder Service", () => {
const folderViews = await firstValueFrom(folderService.folderViews$(mockUserId));
expect(folderViews.length).toBe(1);
expect(folderViews[0].id).toBeNull(); // Should be the "No Folder" folder
expect(folderViews[0].id).toEqual(""); // Should be the "No Folder" folder
});
describe("getRotatedData", () => {

View File

@@ -21,7 +21,6 @@ import { IndexedEntityId, UserId } from "../../types/guid";
import { SearchService as SearchServiceAbstraction } from "../abstractions/search.service";
import { FieldType } from "../enums";
import { CipherType } from "../enums/cipher-type";
import { CipherView } from "../models/view/cipher.view";
import { CipherViewLike, CipherViewLikeUtils } from "../utils/cipher-view-like-utils";
// Time to wait before performing a search after the user stops typing.
@@ -169,7 +168,7 @@ export class SearchService implements SearchServiceAbstraction {
async indexCiphers(
userId: UserId,
ciphers: CipherView[],
ciphers: CipherViewLike[],
indexedEntityId?: string,
): Promise<void> {
if (await this.getIsIndexing(userId)) {
@@ -182,34 +181,47 @@ export class SearchService implements SearchServiceAbstraction {
const builder = new lunr.Builder();
builder.pipeline.add(this.normalizeAccentsPipelineFunction);
builder.ref("id");
builder.field("shortid", { boost: 100, extractor: (c: CipherView) => c.id.substr(0, 8) });
builder.field("shortid", {
boost: 100,
extractor: (c: CipherViewLike) => uuidAsString(c.id).substr(0, 8),
});
builder.field("name", {
boost: 10,
});
builder.field("subtitle", {
boost: 5,
extractor: (c: CipherView) => {
if (c.subTitle != null && c.type === CipherType.Card) {
return c.subTitle.replace(/\*/g, "");
extractor: (c: CipherViewLike) => {
const subtitle = CipherViewLikeUtils.subtitle(c);
if (subtitle != null && CipherViewLikeUtils.getType(c) === CipherType.Card) {
return subtitle.replace(/\*/g, "");
}
return c.subTitle;
return subtitle;
},
});
builder.field("notes");
builder.field("notes", { extractor: (c: CipherViewLike) => CipherViewLikeUtils.getNotes(c) });
builder.field("login.username", {
extractor: (c: CipherView) =>
c.type === CipherType.Login && c.login != null ? c.login.username : null,
extractor: (c: CipherViewLike) => {
const login = CipherViewLikeUtils.getLogin(c);
return login?.username ?? null;
},
});
builder.field("login.uris", {
boost: 2,
extractor: (c: CipherViewLike) => this.uriExtractor(c),
});
builder.field("fields", {
extractor: (c: CipherViewLike) => this.fieldExtractor(c, false),
});
builder.field("fields_joined", {
extractor: (c: CipherViewLike) => this.fieldExtractor(c, true),
});
builder.field("login.uris", { boost: 2, extractor: (c: CipherView) => this.uriExtractor(c) });
builder.field("fields", { extractor: (c: CipherView) => this.fieldExtractor(c, false) });
builder.field("fields_joined", { extractor: (c: CipherView) => this.fieldExtractor(c, true) });
builder.field("attachments", {
extractor: (c: CipherView) => this.attachmentExtractor(c, false),
extractor: (c: CipherViewLike) => this.attachmentExtractor(c, false),
});
builder.field("attachments_joined", {
extractor: (c: CipherView) => this.attachmentExtractor(c, true),
extractor: (c: CipherViewLike) => this.attachmentExtractor(c, true),
});
builder.field("organizationid", { extractor: (c: CipherView) => c.organizationId });
builder.field("organizationid", { extractor: (c: CipherViewLike) => c.organizationId });
ciphers = ciphers || [];
ciphers.forEach((c) => builder.add(c));
const index = builder.build();
@@ -400,37 +412,44 @@ export class SearchService implements SearchServiceAbstraction {
return await firstValueFrom(this.searchIsIndexing$(userId));
}
private fieldExtractor(c: CipherView, joined: boolean) {
if (!c.hasFields) {
private fieldExtractor(c: CipherViewLike, joined: boolean) {
const fields = CipherViewLikeUtils.getFields(c);
if (!fields || fields.length === 0) {
return null;
}
let fields: string[] = [];
c.fields.forEach((f) => {
let fieldStrings: string[] = [];
fields.forEach((f) => {
if (f.name != null) {
fields.push(f.name);
fieldStrings.push(f.name);
}
if (f.type === FieldType.Text && f.value != null) {
fields.push(f.value);
// For CipherListView, value is only populated for Text fields
// For CipherView, we check the type explicitly
if (f.value != null) {
const fieldType = (f as { type?: FieldType }).type;
if (fieldType === undefined || fieldType === FieldType.Text) {
fieldStrings.push(f.value);
}
}
});
fields = fields.filter((f) => f.trim() !== "");
if (fields.length === 0) {
fieldStrings = fieldStrings.filter((f) => f.trim() !== "");
if (fieldStrings.length === 0) {
return null;
}
return joined ? fields.join(" ") : fields;
return joined ? fieldStrings.join(" ") : fieldStrings;
}
private attachmentExtractor(c: CipherView, joined: boolean) {
if (!c.hasAttachments) {
private attachmentExtractor(c: CipherViewLike, joined: boolean) {
const attachmentNames = CipherViewLikeUtils.getAttachmentNames(c);
if (!attachmentNames || attachmentNames.length === 0) {
return null;
}
let attachments: string[] = [];
c.attachments.forEach((a) => {
if (a != null && a.fileName != null) {
if (joined && a.fileName.indexOf(".") > -1) {
attachments.push(a.fileName.substr(0, a.fileName.lastIndexOf(".")));
attachmentNames.forEach((fileName) => {
if (fileName != null) {
if (joined && fileName.indexOf(".") > -1) {
attachments.push(fileName.substring(0, fileName.lastIndexOf(".")));
} else {
attachments.push(a.fileName);
attachments.push(fileName);
}
}
});
@@ -441,43 +460,39 @@ export class SearchService implements SearchServiceAbstraction {
return joined ? attachments.join(" ") : attachments;
}
private uriExtractor(c: CipherView) {
if (c.type !== CipherType.Login || c.login == null || !c.login.hasUris) {
private uriExtractor(c: CipherViewLike) {
if (CipherViewLikeUtils.getType(c) !== CipherType.Login) {
return null;
}
const login = CipherViewLikeUtils.getLogin(c);
if (!login?.uris?.length) {
return null;
}
const uris: string[] = [];
c.login.uris.forEach((u) => {
login.uris.forEach((u) => {
if (u.uri == null || u.uri === "") {
return;
}
// Match ports
// Extract port from URI
const portMatch = u.uri.match(/:(\d+)(?:[/?#]|$)/);
const port = portMatch?.[1];
let uri = u.uri;
if (u.hostname !== null) {
uris.push(u.hostname);
const hostname = CipherViewLikeUtils.getUriHostname(u);
if (hostname !== undefined) {
uris.push(hostname);
if (port) {
uris.push(`${u.hostname}:${port}`);
uris.push(port);
}
return;
} else {
const slash = uri.indexOf("/");
const hostPart = slash > -1 ? uri.substring(0, slash) : uri;
uris.push(hostPart);
if (port) {
uris.push(`${hostPart}`);
uris.push(`${hostname}:${port}`);
uris.push(port);
}
}
// Add processed URI (strip protocol and query params for non-regex matches)
let uri = u.uri;
if (u.match !== UriMatchStrategy.RegularExpression) {
const protocolIndex = uri.indexOf("://");
if (protocolIndex > -1) {
uri = uri.substr(protocolIndex + 3);
uri = uri.substring(protocolIndex + 3);
}
const queryIndex = uri.search(/\?|&|#/);
if (queryIndex > -1) {
@@ -486,6 +501,7 @@ export class SearchService implements SearchServiceAbstraction {
}
uris.push(uri);
});
return uris.length > 0 ? uris : null;
}

View File

@@ -651,4 +651,198 @@ describe("CipherViewLikeUtils", () => {
expect(CipherViewLikeUtils.decryptionFailure(cipherListView)).toBe(false);
});
});
describe("getNotes", () => {
describe("CipherView", () => {
it("returns notes when present", () => {
const cipherView = createCipherView();
cipherView.notes = "This is a test note";
expect(CipherViewLikeUtils.getNotes(cipherView)).toBe("This is a test note");
});
it("returns undefined when notes are not present", () => {
const cipherView = createCipherView();
cipherView.notes = undefined;
expect(CipherViewLikeUtils.getNotes(cipherView)).toBeUndefined();
});
});
describe("CipherListView", () => {
it("returns notes when present", () => {
const cipherListView = {
type: "secureNote",
notes: "List view notes",
} as CipherListView;
expect(CipherViewLikeUtils.getNotes(cipherListView)).toBe("List view notes");
});
it("returns undefined when notes are not present", () => {
const cipherListView = {
type: "secureNote",
} as CipherListView;
expect(CipherViewLikeUtils.getNotes(cipherListView)).toBeUndefined();
});
});
});
describe("getFields", () => {
describe("CipherView", () => {
it("returns fields when present", () => {
const cipherView = createCipherView();
cipherView.fields = [
{ name: "Field1", value: "Value1" } as any,
{ name: "Field2", value: "Value2" } as any,
];
const fields = CipherViewLikeUtils.getFields(cipherView);
expect(fields).toHaveLength(2);
expect(fields?.[0].name).toBe("Field1");
expect(fields?.[0].value).toBe("Value1");
expect(fields?.[1].name).toBe("Field2");
expect(fields?.[1].value).toBe("Value2");
});
it("returns empty array when fields array is empty", () => {
const cipherView = createCipherView();
cipherView.fields = [];
expect(CipherViewLikeUtils.getFields(cipherView)).toEqual([]);
});
});
describe("CipherListView", () => {
it("returns fields when present", () => {
const cipherListView = {
type: { login: {} },
fields: [
{ name: "Username", value: "user@example.com" },
{ name: "API Key", value: "abc123" },
],
} as CipherListView;
const fields = CipherViewLikeUtils.getFields(cipherListView);
expect(fields).toHaveLength(2);
expect(fields?.[0].name).toBe("Username");
expect(fields?.[0].value).toBe("user@example.com");
expect(fields?.[1].name).toBe("API Key");
expect(fields?.[1].value).toBe("abc123");
});
it("returns empty array when fields array is empty", () => {
const cipherListView = {
type: "secureNote",
fields: [],
} as unknown as CipherListView;
expect(CipherViewLikeUtils.getFields(cipherListView)).toEqual([]);
});
it("returns undefined when fields are not present", () => {
const cipherListView = {
type: "secureNote",
} as CipherListView;
expect(CipherViewLikeUtils.getFields(cipherListView)).toBeUndefined();
});
});
});
describe("getAttachmentNames", () => {
describe("CipherView", () => {
it("returns attachment filenames when present", () => {
const cipherView = createCipherView();
const attachment1 = new AttachmentView();
attachment1.id = "1";
attachment1.fileName = "document.pdf";
const attachment2 = new AttachmentView();
attachment2.id = "2";
attachment2.fileName = "image.png";
const attachment3 = new AttachmentView();
attachment3.id = "3";
attachment3.fileName = "spreadsheet.xlsx";
cipherView.attachments = [attachment1, attachment2, attachment3];
const attachmentNames = CipherViewLikeUtils.getAttachmentNames(cipherView);
expect(attachmentNames).toEqual(["document.pdf", "image.png", "spreadsheet.xlsx"]);
});
it("filters out null and undefined filenames", () => {
const cipherView = createCipherView();
const attachment1 = new AttachmentView();
attachment1.id = "1";
attachment1.fileName = "valid.pdf";
const attachment2 = new AttachmentView();
attachment2.id = "2";
attachment2.fileName = null as any;
const attachment3 = new AttachmentView();
attachment3.id = "3";
attachment3.fileName = undefined;
const attachment4 = new AttachmentView();
attachment4.id = "4";
attachment4.fileName = "another.txt";
cipherView.attachments = [attachment1, attachment2, attachment3, attachment4];
const attachmentNames = CipherViewLikeUtils.getAttachmentNames(cipherView);
expect(attachmentNames).toEqual(["valid.pdf", "another.txt"]);
});
it("returns empty array when attachments have no filenames", () => {
const cipherView = createCipherView();
const attachment1 = new AttachmentView();
attachment1.id = "1";
const attachment2 = new AttachmentView();
attachment2.id = "2";
cipherView.attachments = [attachment1, attachment2];
const attachmentNames = CipherViewLikeUtils.getAttachmentNames(cipherView);
expect(attachmentNames).toEqual([]);
});
it("returns empty array for empty attachments array", () => {
const cipherView = createCipherView();
cipherView.attachments = [];
expect(CipherViewLikeUtils.getAttachmentNames(cipherView)).toEqual([]);
});
});
describe("CipherListView", () => {
it("returns attachment names when present", () => {
const cipherListView = {
type: "secureNote",
attachmentNames: ["report.pdf", "photo.jpg", "data.csv"],
} as CipherListView;
const attachmentNames = CipherViewLikeUtils.getAttachmentNames(cipherListView);
expect(attachmentNames).toEqual(["report.pdf", "photo.jpg", "data.csv"]);
});
it("returns empty array when attachmentNames is empty", () => {
const cipherListView = {
type: "secureNote",
attachmentNames: [],
} as unknown as CipherListView;
expect(CipherViewLikeUtils.getAttachmentNames(cipherListView)).toEqual([]);
});
it("returns undefined when attachmentNames is not present", () => {
const cipherListView = {
type: "secureNote",
} as CipherListView;
expect(CipherViewLikeUtils.getAttachmentNames(cipherListView)).toBeUndefined();
});
});
});
});

View File

@@ -10,6 +10,7 @@ import {
LoginUriView as LoginListUriView,
} from "@bitwarden/sdk-internal";
import { Utils } from "../../platform/misc/utils";
import { CipherType } from "../enums";
import { Cipher } from "../models/domain/cipher";
import { CardView } from "../models/view/card.view";
@@ -290,6 +291,71 @@ export class CipherViewLikeUtils {
static decryptionFailure = (cipher: CipherViewLike): boolean => {
return "decryptionFailure" in cipher ? cipher.decryptionFailure : false;
};
/**
* Returns the notes from the cipher.
*
* @param cipher - The cipher to extract notes from (either `CipherView` or `CipherListView`)
* @returns The notes string if present, or `undefined` if not set
*/
static getNotes = (cipher: CipherViewLike): string | undefined => {
return cipher.notes;
};
/**
* Returns the fields from the cipher.
*
* @param cipher - The cipher to extract fields from (either `CipherView` or `CipherListView`)
* @returns Array of field objects with `name` and `value` properties, `undefined` if not set
*/
static getFields = (
cipher: CipherViewLike,
): { name?: string | null; value?: string | undefined }[] | undefined => {
if (this.isCipherListView(cipher)) {
return cipher.fields;
}
return cipher.fields;
};
/**
* Returns attachment filenames from the cipher.
*
* @param cipher - The cipher to extract attachment names from (either `CipherView` or `CipherListView`)
* @returns Array of attachment filenames, `undefined` if attachments are not present
*/
static getAttachmentNames = (cipher: CipherViewLike): string[] | undefined => {
if (this.isCipherListView(cipher)) {
return cipher.attachmentNames;
}
return cipher.attachments
?.map((a) => a.fileName)
.filter((name): name is string => name != null);
};
/**
* Extracts hostname from a login URI.
*
* @param uri - The URI object (either `LoginUriView` class or `LoginListUriView`)
* @returns The hostname if available, `undefined` otherwise
*
* @remarks
* - For `LoginUriView` (CipherView): Uses the built-in `hostname` getter
* - For `LoginListUriView` (CipherListView): Computes hostname using `Utils.getHostname()`
* - Returns `undefined` for RegularExpression match types or when hostname cannot be extracted
*/
static getUriHostname = (uri: LoginListUriView | LoginUriView): string | undefined => {
if ("hostname" in uri && typeof uri.hostname !== "undefined") {
return uri.hostname ?? undefined;
}
if (uri.match !== UriMatchStrategy.RegularExpression && uri.uri) {
const hostname = Utils.getHostname(uri.uri);
return hostname === "" ? undefined : hostname;
}
return undefined;
};
}
/**

View File

@@ -0,0 +1,69 @@
import { Meta } from "@storybook/addon-docs/blocks";
<Meta title="Documentation/Router Focus Management" />
# Router Focus Management
On a normal non-SPA (Single Page Application) webpage, a page navigation / route change will cause
the full page to reload, and a user's focus is placed at the top of the page when the new page
loads.
Bitwarden's Angular apps are SPAs using the Angular router to manage internal routing and page
navigation. When the Angular router performs a page navigation / route change to another internal
SPA route, the full page does not reload, and the user's focus does not move from the trigger
element unless the trigger element no longer exists. There is no other built-in notification to a
screenreader user that a navigation has occured, if the focus is not moved.
## Web
We handle router focus management in the web app by moving the user's focus at the end of a SPA
Angular router navigation.
See `router-focus-manager.service.ts` for the implementation.
### Default behavior
By default, we focus the `main` element.
Consumers can change or opt out of the focus management using the `state` input to the
[Angular route](https://angular.dev/api/router/RouterLink). Using `state` allows us to access the
value between browser back/forward arrows.
### Change focus target
In template: `<a [routerLink]="route()" [state]="{ focusAfterNav: '#selector' }"></a>`
In typescript: `this.router.navigate([], { state: { focusAfterNav: '#selector' }})`
Any valid `querySelector` selector can be passed. If the element is not found, no focus management
occurs as we cannot make the assumption that the default `main` element is the next best option.
Examples of where you might want to change the target:
- A full page navigation occurs where you do want the user to be placed at the top of the page (aka
the body element) like a non-SPA app, such as going from Password Manager to Secrets Manager
- A routed dialog needs to manually specify where the focus should return to once it is closed
### Opt out of focus management
In template: `<a [routerLink]="route()" [state]="{ focusAfterNav: false }"></a>`
In typescript: `this.router.navigate([], { state: { focusAfterNav: false }})`
Example of where you might want to manually opt out:
- Tab component causes a route navigation, and the focus will be handled by the tab component itself
### Autofocus directive
Consumers can use the autofocus directive on an applicable interactive element. The autofocus
directive will take precedence over the router focus management system. See the
[Autofocus Directive docs](?path=/docs/component-library-form-autofocus-directive--docs).
## Browser
Not implemented yet.
## Desktop
Not implemented yet.

View File

@@ -0,0 +1,323 @@
import {
computed,
DestroyRef,
EventEmitter,
Injectable,
NgZone,
Signal,
signal,
} from "@angular/core";
import { TestBed } from "@angular/core/testing";
import { Event, Navigation, NavigationEnd, Router } from "@angular/router";
import { mock } from "jest-mock-extended";
import { BehaviorSubject, Subject } from "rxjs";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { RouterFocusManagerService } from "./router-focus-manager.service";
describe("RouterFocusManagerService", () => {
@Injectable()
class MockNgZone extends NgZone {
onStable: EventEmitter<any> = new EventEmitter(false);
constructor() {
super({ enableLongStackTrace: false });
}
run(fn: any): any {
return fn();
}
runOutsideAngular(fn: any): any {
return fn();
}
simulateZoneExit(): void {
this.onStable.emit(null);
}
isStable: boolean = true;
}
@Injectable()
class MockRouter extends Router {
readonly currentNavigationExtras = signal({});
readonly currentNavigation: Signal<Navigation> = computed(() => ({
...mock<Navigation>(),
extras: this.currentNavigationExtras(),
}));
// eslint-disable-next-line rxjs/no-exposed-subjects
readonly routerEventsSubject = new Subject<Event>();
override get events() {
return this.routerEventsSubject.asObservable();
}
}
let service: RouterFocusManagerService;
let featureFlagSubject: BehaviorSubject<boolean>;
let mockRouter: MockRouter;
let mockConfigService: Partial<ConfigService>;
let mockNgZoneRef: MockNgZone;
let querySelectorSpy: jest.SpyInstance;
let consoleWarnSpy: jest.SpyInstance;
beforeEach(() => {
// Mock ConfigService
featureFlagSubject = new BehaviorSubject<boolean>(true);
mockConfigService = {
getFeatureFlag$: jest.fn((flag: FeatureFlag) => {
if (flag === FeatureFlag.RouterFocusManagement) {
return featureFlagSubject.asObservable();
}
return new BehaviorSubject(false).asObservable();
}) as ConfigService["getFeatureFlag$"],
};
// Spy on document.querySelector and console.warn
querySelectorSpy = jest.spyOn(document, "querySelector");
consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation();
TestBed.configureTestingModule({
providers: [
RouterFocusManagerService,
{ provide: Router, useClass: MockRouter },
{ provide: ConfigService, useValue: mockConfigService },
{ provide: NgZone, useClass: MockNgZone },
{ provide: DestroyRef, useValue: { onDestroy: jest.fn() } },
],
});
service = TestBed.inject(RouterFocusManagerService);
mockNgZoneRef = TestBed.inject(NgZone) as MockNgZone;
mockRouter = TestBed.inject(Router) as MockRouter;
});
afterEach(() => {
querySelectorSpy.mockRestore();
consoleWarnSpy.mockRestore();
mockNgZoneRef.isStable = true;
TestBed.resetTestingModule();
});
describe("default behavior", () => {
it("should focus main element after navigation", () => {
const mainElement = document.createElement("main");
mainElement.focus = jest.fn();
querySelectorSpy.mockReturnValue(mainElement);
// Subscribe to start the service
service.start$.subscribe();
// Emit first navigation (should be skipped)
mockRouter.routerEventsSubject.next(new NavigationEnd(1, "/first", "/first"));
// Emit second navigation (should trigger focus)
mockRouter.routerEventsSubject.next(new NavigationEnd(2, "/test", "/test"));
expect(querySelectorSpy).toHaveBeenCalledWith("main");
expect(mainElement.focus).toHaveBeenCalled();
});
});
describe("custom selector", () => {
it("should focus custom element when focusAfterNav selector is provided", () => {
const customElement = document.createElement("button");
customElement.id = "custom-btn";
customElement.focus = jest.fn();
querySelectorSpy.mockReturnValue(customElement);
// Subscribe to start the service
service.start$.subscribe();
// Emit first navigation (should be skipped)
mockRouter.routerEventsSubject.next(new NavigationEnd(1, "/first", "/first"));
// Emit second navigation with custom selector
mockRouter.currentNavigationExtras.set({ state: { focusAfterNav: "#custom-btn" } });
mockRouter.routerEventsSubject.next(new NavigationEnd(2, "/test", "/test"));
expect(querySelectorSpy).toHaveBeenCalledWith("#custom-btn");
expect(customElement.focus).toHaveBeenCalled();
});
});
describe("opt-out", () => {
it("should not focus when focusAfterNav is false", () => {
const mainElement = document.createElement("main");
mainElement.focus = jest.fn();
querySelectorSpy.mockReturnValue(mainElement);
// Subscribe to start the service
service.start$.subscribe();
// Emit first navigation (should be skipped)
mockRouter.routerEventsSubject.next(new NavigationEnd(1, "/first", "/first"));
// Emit second navigation with opt-out
mockRouter.currentNavigationExtras.set({ state: { focusAfterNav: false } });
mockRouter.routerEventsSubject.next(new NavigationEnd(2, "/test", "/test"));
expect(querySelectorSpy).not.toHaveBeenCalled();
expect(mainElement.focus).not.toHaveBeenCalled();
});
});
describe("element not found", () => {
it("should log warning when custom selector does not match any element", () => {
querySelectorSpy.mockReturnValue(null);
// Subscribe to start the service
service.start$.subscribe();
// Emit first navigation (should be skipped)
mockRouter.routerEventsSubject.next(new NavigationEnd(1, "/first", "/first"));
// Emit second navigation with non-existent selector
mockRouter.currentNavigationExtras.set({ state: { focusAfterNav: "#non-existent" } });
mockRouter.routerEventsSubject.next(new NavigationEnd(2, "/test", "/test"));
expect(querySelectorSpy).toHaveBeenCalledWith("#non-existent");
expect(consoleWarnSpy).toHaveBeenCalledWith(
'RouterFocusManager: Could not find element with selector "#non-existent"',
);
});
});
// Remove describe block when FeatureFlag.RouterFocusManagement is removed
describe("feature flag", () => {
it("should not activate when RouterFocusManagement flag is disabled", () => {
const mainElement = document.createElement("main");
mainElement.focus = jest.fn();
querySelectorSpy.mockReturnValue(mainElement);
// Disable feature flag
featureFlagSubject.next(false);
// Subscribe to start the service
service.start$.subscribe();
// Emit first navigation (should be skipped)
mockRouter.routerEventsSubject.next(new NavigationEnd(1, "/first", "/first"));
// Emit second navigation with flag disabled
mockRouter.routerEventsSubject.next(new NavigationEnd(2, "/test", "/test"));
expect(querySelectorSpy).not.toHaveBeenCalled();
expect(mainElement.focus).not.toHaveBeenCalled();
});
it("should activate when RouterFocusManagement flag is enabled", () => {
const mainElement = document.createElement("main");
mainElement.focus = jest.fn();
querySelectorSpy.mockReturnValue(mainElement);
// Ensure feature flag is enabled
featureFlagSubject.next(true);
// Subscribe to start the service
service.start$.subscribe();
// Emit first navigation (should be skipped)
mockRouter.routerEventsSubject.next(new NavigationEnd(1, "/first", "/first"));
// Emit second navigation with flag enabled
mockRouter.routerEventsSubject.next(new NavigationEnd(2, "/test", "/test"));
expect(querySelectorSpy).toHaveBeenCalledWith("main");
expect(mainElement.focus).toHaveBeenCalled();
});
});
describe("first navigation skip", () => {
it("should not trigger focus management on first navigation after page load", () => {
const mainElement = document.createElement("main");
mainElement.focus = jest.fn();
querySelectorSpy.mockReturnValue(mainElement);
// Subscribe to start the service
service.start$.subscribe();
// Emit first navigation (should be skipped)
mockRouter.routerEventsSubject.next(new NavigationEnd(1, "/first", "/first"));
expect(querySelectorSpy).not.toHaveBeenCalled();
expect(mainElement.focus).not.toHaveBeenCalled();
});
it("should trigger focus management on second and subsequent navigations", () => {
const mainElement = document.createElement("main");
mainElement.focus = jest.fn();
querySelectorSpy.mockReturnValue(mainElement);
// Subscribe to start the service
service.start$.subscribe();
// Emit first navigation (should be skipped)
mockRouter.routerEventsSubject.next(new NavigationEnd(1, "/first", "/first"));
// Emit second navigation (should trigger focus)
mockRouter.routerEventsSubject.next(new NavigationEnd(2, "/second", "/second"));
expect(querySelectorSpy).toHaveBeenCalledWith("main");
expect(mainElement.focus).toHaveBeenCalledTimes(1);
// Emit third navigation (should also trigger focus)
mainElement.focus = jest.fn(); // Reset mock
mockRouter.routerEventsSubject.next(new NavigationEnd(3, "/third", "/third"));
expect(mainElement.focus).toHaveBeenCalledTimes(1);
});
});
describe("NgZone stability", () => {
it("should focus immediately when zone is stable", () => {
const mainElement = document.createElement("main");
mainElement.focus = jest.fn();
querySelectorSpy.mockReturnValue(mainElement);
// Subscribe to start the service
service.start$.subscribe();
// Emit first navigation (should be skipped)
mockRouter.routerEventsSubject.next(new NavigationEnd(1, "/first", "/first"));
// Emit second navigation
mockRouter.routerEventsSubject.next(new NavigationEnd(2, "/test", "/test"));
expect(mainElement.focus).toHaveBeenCalled();
});
it("should wait for zone stability before focusing when zone is not stable", async () => {
const mainElement = document.createElement("main");
mainElement.focus = jest.fn();
querySelectorSpy.mockReturnValue(mainElement);
// Set zone as not stable
mockNgZoneRef.isStable = false;
// Subscribe to start the service
service.start$.subscribe();
// Emit first navigation (should be skipped)
mockRouter.routerEventsSubject.next(new NavigationEnd(1, "/first", "/first"));
// Emit second navigation
mockRouter.routerEventsSubject.next(new NavigationEnd(2, "/test", "/test"));
// Focus should not happen yet
expect(mainElement.focus).not.toHaveBeenCalled();
// Emit zone stability
mockNgZoneRef.onStable.emit(true);
// flush promises
await Promise.resolve();
// Now focus should have happened
expect(mainElement.focus).toHaveBeenCalled();
});
});
});

View File

@@ -1,40 +1,23 @@
import { inject, Injectable } from "@angular/core";
import { inject, Injectable, NgZone } from "@angular/core";
import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
import { NavigationEnd, Router } from "@angular/router";
import { skip, filter, combineLatestWith, tap } from "rxjs";
import { skip, filter, combineLatestWith, tap, map, firstValueFrom } from "rxjs";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { queryForAutofocusDescendents } from "../input";
@Injectable({ providedIn: "root" })
export class RouterFocusManagerService {
private router = inject(Router);
private ngZone = inject(NgZone);
private configService = inject(ConfigService);
/**
* Handles SPA route focus management. SPA apps don't automatically notify screenreader
* users that navigation has occured or move the user's focus to the content they are
* navigating to, so we need to do it.
*
* By default, we focus the `main` after an internal route navigation.
*
* Consumers can opt out of the passing the following to the `state` input. Using `state`
* allows us to access the value between browser back/forward arrows.
* In template: `<a [routerLink]="route()" [state]="{ focusMainAfterNav: false }"></a>`
* In typescript: `this.router.navigate([], { state: { focusMainAfterNav: false }})`
*
* Or, consumers can use the autofocus directive on an applicable interactive element.
* The autofocus directive will take precedence over this route focus pipeline.
*
* Example of where you might want to manually opt out:
* - Tab component causes a route navigation, but the tab content should be focused,
* not the whole `main`
*
* Note that router events that cause a fully new page to load (like switching between
* products) will not follow this pipeline. Instead, those will automatically bring
* focus to the top of the html document as if it were a full page load. So those links
* do not need to manually opt out of this pipeline.
* See associated router-focus-manager.mdx page for documentation on what this pipeline does and
* how to customize focus behavior.
*/
start$ = this.router.events.pipe(
takeUntilDestroyed(),
@@ -46,19 +29,47 @@ export class RouterFocusManagerService {
skip(1),
combineLatestWith(this.configService.getFeatureFlag$(FeatureFlag.RouterFocusManagement)),
filter(([_navEvent, flagEnabled]) => flagEnabled),
filter(() => {
map(() => {
const currentNavExtras = this.router.currentNavigation()?.extras;
const focusMainAfterNav: boolean | undefined = currentNavExtras?.state?.focusMainAfterNav;
const focusAfterNav: boolean | string | undefined = currentNavExtras?.state?.focusAfterNav;
return focusMainAfterNav !== false;
return focusAfterNav;
}),
tap(() => {
const mainEl = document.querySelector<HTMLElement>("main");
filter((focusAfterNav) => {
return focusAfterNav !== false;
}),
tap(async (focusAfterNav) => {
let elSelector: string = "main";
if (mainEl) {
mainEl.focus();
if (typeof focusAfterNav === "string" && focusAfterNav.length > 0) {
elSelector = focusAfterNav;
}
if (this.ngZone.isStable) {
this.focusTargetEl(elSelector);
} else {
await firstValueFrom(this.ngZone.onStable);
this.focusTargetEl(elSelector);
}
}),
);
private focusTargetEl(elSelector: string) {
const targetEl = document.querySelector<HTMLElement>(elSelector);
const mainEl = document.querySelector<HTMLElement>("main");
const pageHasAutofocusEl = mainEl && queryForAutofocusDescendents(mainEl).length > 0;
if (pageHasAutofocusEl) {
// do nothing because autofocus will handle the focus
return;
} else if (targetEl) {
targetEl.focus();
} else {
// eslint-disable-next-line no-console
console.warn(`RouterFocusManager: Could not find element with selector "${elSelector}"`);
}
}
}

View File

@@ -38,7 +38,7 @@ export class BerryComponent {
});
protected readonly textColor = computed(() => {
return this.variant() === "contrast" ? "tw-text-fg-dark" : "tw-text-fg-white";
return this.variant() === "contrast" ? "tw-text-fg-heading" : "tw-text-fg-contrast";
});
protected readonly padding = computed(() => {
@@ -67,7 +67,7 @@ export class BerryComponent {
warning: "tw-bg-bg-warning",
danger: "tw-bg-bg-danger",
accentPrimary: "tw-bg-fg-accent-primary-strong",
contrast: "tw-bg-bg-white",
contrast: "tw-bg-bg-primary",
};
return [

View File

@@ -75,7 +75,9 @@ export const statusType: Story = {
<bit-berry [type]="'status'" variant="warning"></bit-berry>
<bit-berry [type]="'status'" variant="danger"></bit-berry>
<bit-berry [type]="'status'" variant="accentPrimary"></bit-berry>
<bit-berry [type]="'status'" variant="contrast"></bit-berry>
<div class="tw-p-2 tw-bg-bg-contrast">
<bit-berry [type]="'status'" variant="contrast"></bit-berry>
</div>
</div>
`,
}),
@@ -153,8 +155,8 @@ export const AllVariants: Story = {
<bit-berry variant="accentPrimary" [value]="5000"></bit-berry>
</div>
<div class="tw-flex tw-items-center tw-gap-4 tw-bg-bg-dark">
<span class="tw-w-20 tw-text-fg-white">Contrast:</span>
<div class="tw-flex tw-items-center tw-gap-4 tw-bg-bg-contrast">
<span class="tw-w-20 tw-text-fg-contrast">Contrast:</span>
<bit-berry type="status" variant="contrast"></bit-berry>
<bit-berry variant="contrast" [value]="5"></bit-berry>
<bit-berry variant="contrast" [value]="50"></bit-berry>

View File

@@ -1,4 +1,3 @@
import { NgClass, NgTemplateOutlet } from "@angular/common";
import {
input,
HostBinding,
@@ -72,7 +71,7 @@ const buttonStyles: Record<ButtonType, string[]> = {
selector: "button[bitButton], a[bitButton]",
templateUrl: "button.component.html",
providers: [{ provide: ButtonLikeAbstraction, useExisting: ButtonComponent }],
imports: [NgClass, NgTemplateOutlet, SpinnerComponent],
imports: [SpinnerComponent],
hostDirectives: [AriaDisableDirective],
})
export class ButtonComponent implements ButtonLikeAbstraction {
@@ -124,14 +123,31 @@ export class ButtonComponent implements ButtonLikeAbstraction {
return this.showLoadingStyle() || (this.disabledAttr() && this.loading() === false);
});
/**
* Style variant of the button.
*/
readonly buttonType = input<ButtonType>("secondary");
/**
* Bitwarden icon displayed **before** the button label.
* Spacing between the icon and label is handled automatically.
*/
readonly startIcon = input<BitwardenIcon | undefined>(undefined);
/**
* Bitwarden icon (`bwi-*`) displayed **after** the button label.
* Spacing between the label and icon is handled automatically.
*/
readonly endIcon = input<BitwardenIcon | undefined>(undefined);
/**
* Size variant of the button.
*/
readonly size = input<ButtonSize>("default");
/**
* When `true`, the button expands to fill the full width of its container.
*/
readonly block = input(false, { transform: booleanAttribute });
readonly loading = model<boolean>(false);

View File

@@ -80,21 +80,20 @@ where the width is fixed and the text wraps to 2 lines if exceeding the button
## With Icon
To ensure consistent icon spacing, the icon should have .5rem spacing on left or right(depending on
placement).
Use the `startIcon` and `endIcon` inputs to add a Bitwarden icon (`bwi-*`) before or after the
button label. Do not use a `<bit-icon>` component inside the button as this may not have the correct
styling and spacing.
> NOTE: Use logical css properties to ensure LTR/RTL support.
**If icon is placed before button label**
### Icon before the label
```html
<i class="bwi bwi-plus tw-me-2"></i>
<button bitButton startIcon="bwi-plus">Add item</button>
```
**If icon is placed after button label**
### Icon after the label
```html
<i class="bwi bwi-plus tw-ms-2"></i>
<button bitButton endIcon="bwi-angle-right">Next</button>
```
<Canvas of={stories.WithIcon} />

View File

@@ -1,4 +1,4 @@
import { ChangeDetectionStrategy, Component, signal } from "@angular/core";
import { ChangeDetectionStrategy, Component, signal, computed } from "@angular/core";
import { ComponentFixture, TestBed } from "@angular/core/testing";
import { FormControl } from "@angular/forms";
import { By } from "@angular/platform-browser";
@@ -502,3 +502,157 @@ class TestAppComponent {
readonly disabled = signal(false);
readonly fullWidth = signal(false);
}
describe("ChipSelectComponentWithDynamicOptions", () => {
let component: ChipSelectComponent<string>;
let fixture: ComponentFixture<TestAppWithDynamicOptionsComponent>;
const getChipButton = () =>
fixture.debugElement.query(By.css("[data-fvw-target]"))?.nativeElement as HTMLButtonElement;
beforeEach(async () => {
await TestBed.configureTestingModule({
imports: [TestAppWithDynamicOptionsComponent, NoopAnimationsModule],
providers: [{ provide: I18nService, useValue: mockI18nService }],
}).compileComponents();
fixture = TestBed.createComponent(TestAppWithDynamicOptionsComponent);
fixture.detectChanges();
component = fixture.debugElement.query(By.directive(ChipSelectComponent)).componentInstance;
fixture.componentInstance.firstCounter.set(0);
fixture.componentInstance.secondCounter.set(0);
fixture.detectChanges();
});
describe("User-Facing Behavior", () => {
it("should update available options when they change", () => {
const first = 5;
const second = 10;
const testApp = fixture.componentInstance;
testApp.firstCounter.set(first);
testApp.secondCounter.set(second);
fixture.detectChanges();
getChipButton().click();
fixture.detectChanges();
const menuItems = Array.from(document.querySelectorAll<HTMLButtonElement>("[bitMenuItem]"));
expect(menuItems.some((el) => el.textContent?.includes(`Option - ${first}`))).toBe(true);
expect(menuItems.some((el) => el.textContent?.includes(`Option - ${second}`))).toBe(true);
});
});
describe("Form Integration Behavior", () => {
it("should display selected option when form control value is set", () => {
const testApp = fixture.componentInstance;
testApp.firstCounter.set(1);
testApp.secondCounter.set(2);
component.writeValue("opt2"); // select second menu option which has dynamic label
fixture.detectChanges();
const button = getChipButton();
expect(button.textContent?.trim()).toContain("Option - 2"); // verify that the label reflects the dynamic value
// change the dynamic values and verify that the menu still shows the correct labels for the options
// it should also keep opt2 selected since it's the same value, just with an updated label
const first = 10;
const second = 20;
testApp.firstCounter.set(first);
testApp.secondCounter.set(second);
fixture.detectChanges();
// again, verify that the label reflects the dynamic value
expect(button.textContent?.trim()).toContain(`Option - ${second}`);
// click the button to open the menu
getChipButton().click();
fixture.detectChanges();
// verify that the menu items also reflect the updated dynamic values
const menuItems = Array.from(document.querySelectorAll<HTMLButtonElement>("[bitMenuItem]"));
expect(menuItems.some((el) => el.textContent?.includes(`Option - ${first}`))).toBe(true);
expect(menuItems.some((el) => el.textContent?.includes(`Option - ${second}`))).toBe(true);
});
it("should find and display nested option when form control value is set", () => {
const testApp = fixture.componentInstance;
testApp.firstCounter.set(1);
testApp.secondCounter.set(2);
component.writeValue("child1"); // select a child menu item
fixture.detectChanges();
const button = getChipButton();
// verify that the label reflects the dynamic value for the child option
expect(button.textContent?.trim()).toContain("Child - 1");
const first = 10;
const second = 20;
testApp.firstCounter.set(first);
testApp.secondCounter.set(second);
fixture.detectChanges();
// again, verify that the label reflects the dynamic value
expect(button.textContent?.trim()).toContain(`Child - ${first}`);
});
it("should clear selection when form control value is set to null", () => {
const testApp = fixture.componentInstance;
testApp.firstCounter.set(1);
testApp.secondCounter.set(2);
component.writeValue("opt1");
fixture.detectChanges();
expect(getChipButton().textContent).toContain("Option - 1");
component.writeValue(null as any);
fixture.detectChanges();
expect(getChipButton().textContent).toContain("Select an option");
});
});
}); /* end of ChipSelectComponentWithDynamicOptions tests */
@Component({
selector: "test-app-with-dynamic-options",
template: `
<bit-chip-select
placeholderText="Select an option"
placeholderIcon="bwi-filter"
[options]="options()"
[disabled]="disabled()"
[fullWidth]="fullWidth()"
/>
`,
imports: [ChipSelectComponent],
changeDetection: ChangeDetectionStrategy.OnPush,
})
class TestAppWithDynamicOptionsComponent {
readonly firstCounter = signal(1);
readonly secondCounter = signal(2);
readonly options = computed(() => {
const first = this.firstCounter();
const second = this.secondCounter();
return [
{ label: `Option - ${first}`, value: "opt1", icon: "bwi-folder" },
{ label: `Option - ${second}`, value: "opt2" },
{
label: "Parent Option",
value: "parent",
children: [
{ label: `Child - ${first}`, value: "child1" },
{ label: `Child - ${second}`, value: "child2" },
],
},
];
});
readonly disabled = signal(false);
readonly fullWidth = signal(false);
}

View File

@@ -106,8 +106,19 @@ export class ChipSelectComponent<T = unknown> implements ControlValueAccessor {
constructor() {
// Initialize the root tree whenever options change
effect(() => {
const currentSelection = this.selectedOption;
// when the options change, clear the childParentMap
this.childParentMap.clear();
this.initializeRootTree(this.options());
// when the options change, we need to change our selectedOption
// to reflect the changed options.
if (currentSelection?.value != null) {
this.selectedOption = this.findOption(this.rootTree, currentSelection.value);
}
// If there's a pending value, apply it now that options are available
if (this.pendingValue !== undefined) {
this.selectedOption = this.findOption(this.rootTree, this.pendingValue);

View File

@@ -20,6 +20,7 @@ import { combineLatest, firstValueFrom, switchMap } from "rxjs";
import { I18nPipe } from "@bitwarden/ui-common";
import { BitIconButtonComponent } from "../../icon-button/icon-button.component";
import { queryForAutofocusDescendents } from "../../input";
import { SpinnerComponent } from "../../spinner";
import { TypographyDirective } from "../../typography/typography.directive";
import { hasScrollableContent$ } from "../../utils/";
@@ -45,7 +46,7 @@ const drawerSizeToWidth = {
// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
@Component({
selector: "bit-dialog",
selector: "bit-dialog, [bit-dialog]",
templateUrl: "./dialog.component.html",
host: {
"[class]": "classes()",
@@ -67,7 +68,7 @@ const drawerSizeToWidth = {
export class DialogComponent implements AfterViewInit {
private readonly destroyRef = inject(DestroyRef);
private readonly ngZone = inject(NgZone);
private readonly el = inject(ElementRef);
private readonly el = inject<ElementRef<HTMLElement>>(ElementRef);
private readonly dialogHeader =
viewChild.required<ElementRef<HTMLHeadingElement>>("dialogHeader");
@@ -187,8 +188,7 @@ export class DialogComponent implements AfterViewInit {
* AutofocusDirective.
*/
const dialogRef = this.el.nativeElement;
// Must match selectors of AutofocusDirective
const autofocusDescendants = dialogRef.querySelectorAll("[appAutofocus], [bitAutofocus]");
const autofocusDescendants = queryForAutofocusDescendents(dialogRef);
const hasAutofocusDescendants = autofocusDescendants.length > 0;
if (!hasAutofocusDescendants) {

View File

@@ -82,3 +82,12 @@ The `background` input can be set to `alt` to change the background color. This
dialogs that contain multiple card sections.
<Canvas of={stories.WithCards} />
## Using Forms with Dialogs
When using forms with dialogs, apply the `bit-dialog` attribute directly to the `<form>` element
instead of wrapping the dialog in a form. This ensures proper styling.
```html
<form bit-dialog>...</form>
```

View File

@@ -225,8 +225,7 @@ export const WithCards: Story = {
...args,
},
template: /*html*/ `
<form [formGroup]="formObj">
<bit-dialog [dialogSize]="dialogSize" [background]="background" [title]="title" [subtitle]="subtitle" [loading]="loading" [disablePadding]="disablePadding" [disableAnimations]="disableAnimations">
<form [formGroup]="formObj" bit-dialog [dialogSize]="dialogSize" [background]="background" [title]="title" [subtitle]="subtitle" [loading]="loading" [disablePadding]="disablePadding" [disableAnimations]="disableAnimations">
<ng-container bitDialogContent>
<bit-section>
<bit-section-header>
@@ -270,7 +269,7 @@ export const WithCards: Story = {
</bit-section>
</ng-container>
<ng-container bitDialogFooter>
<button type="button" bitButton buttonType="primary" [disabled]="loading">Save</button>
<button type="submit" bitButton buttonType="primary" [disabled]="loading">Save</button>
<button type="button" bitButton buttonType="secondary" [disabled]="loading">Cancel</button>
<button
type="button"
@@ -281,8 +280,7 @@ export const WithCards: Story = {
size="default"
label="Delete"></button>
</ng-container>
</bit-dialog>
</form>
</form>
`,
}),
args: {

View File

@@ -92,3 +92,12 @@ Once closed, focus should remain on the element which triggered the Dialog.
**Note:** If a Simple Dialog is triggered from a main Dialog, be sure to make sure focus is moved to
the Simple Dialog.
## Using Forms with Dialogs
When using forms with dialogs, apply the `bit-dialog` attribute directly to the `<form>` element
instead of wrapping the dialog in a form. This ensures proper styling.
```html
<form bit-dialog>...</form>
```

View File

@@ -1,27 +1,25 @@
<form [formGroup]="formGroup" [bitSubmit]="accept">
<bit-simple-dialog>
<i bitDialogIcon class="bwi tw-text-3xl" [class]="iconClasses" aria-hidden="true"></i>
<form [formGroup]="formGroup" [bitSubmit]="accept" bit-simple-dialog>
<i bitDialogIcon class="bwi tw-text-3xl" [class]="iconClasses" aria-hidden="true"></i>
<span bitDialogTitle>{{ title }}</span>
<span bitDialogTitle>{{ title }}</span>
<div bitDialogContent>{{ content }}</div>
<div bitDialogContent>{{ content }}</div>
<ng-container bitDialogFooter>
<button type="submit" bitButton bitFormButton buttonType="primary">
{{ acceptButtonText }}
<ng-container bitDialogFooter>
<button type="submit" bitButton bitFormButton buttonType="primary">
{{ acceptButtonText }}
</button>
@if (showCancelButton) {
<button
type="button"
bitButton
bitFormButton
buttonType="secondary"
(click)="dialogRef.close(false)"
>
{{ cancelButtonText }}
</button>
@if (showCancelButton) {
<button
type="button"
bitButton
bitFormButton
buttonType="secondary"
(click)="dialogRef.close(false)"
>
{{ cancelButtonText }}
</button>
}
</ng-container>
</bit-simple-dialog>
}
</ng-container>
</form>

View File

@@ -12,7 +12,7 @@ export class IconDirective {}
// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
@Component({
selector: "bit-simple-dialog",
selector: "bit-simple-dialog, [bit-simple-dialog]",
templateUrl: "./simple-dialog.component.html",
animations: [fadeIn],
imports: [DialogTitleContainerDirective, TypographyDirective],

View File

@@ -49,3 +49,12 @@ Simple dialogs can support scrolling content if necessary, but typically with la
content a [Dialog component](?path=/docs/component-library-dialogs-dialog--docs).
<Canvas of={stories.ScrollingContent} />
## Using Forms with Dialogs
When using forms with dialogs, apply the `bit-simple-dialog` attribute directly to the `<form>`
element instead of wrapping the dialog in a form. This ensures proper styling.
```html
<form bit-simple-dialog>...</form>
```

View File

@@ -126,3 +126,21 @@ export const TextOverflow: Story = {
`,
}),
};
export const WithForm: Story = {
render: (args) => ({
props: args,
template: /*html*/ `
<form bit-simple-dialog>
<span bitDialogTitle>Confirm Action</span>
<span bitDialogContent>
Are you sure you want to proceed with this action? This cannot be undone.
</span>
<ng-container bitDialogFooter>
<button type="submit" bitButton buttonType="primary">Confirm</button>
<button type="button" bitButton buttonType="secondary">Cancel</button>
</ng-container>
</form>
`,
}),
};

View File

@@ -13,6 +13,18 @@ import { Utils } from "@bitwarden/common/platform/misc/utils";
import { FocusableElement } from "../shared/focusable-element";
/**
* Helper function to query for descendents of a given el that have the AutofocusDirective
* applied to them
*
* @param el element that supports querySelectorAll
* @returns querySelectorAll results
*/
export function queryForAutofocusDescendents(el: Document | Element) {
// ensure selectors match the directive selectors
return el.querySelectorAll("[appAutofocus], [bitAutofocus]");
}
/**
* Directive to focus an element.
*
@@ -21,9 +33,7 @@ import { FocusableElement } from "../shared/focusable-element";
* Will focus the element once, when it becomes visible.
*
* If the component provides the `FocusableElement` interface, the `focus`
* method will be called. Otherwise, the native element will be focused.
*
* If selector changes, `dialog.component.ts` must also be updated
* method will be called. Otherwise, the native element will be focused. *
*/
@Directive({
selector: "[appAutofocus], [bitAutofocus]",

View File

@@ -58,13 +58,12 @@ const commonStyles = [
"[&.tw-test-hover_span]:tw-underline",
"[&:hover_span]:tw-decoration-[.125em]",
"[&.tw-test-hover_span]:tw-decoration-[.125em]",
"disabled:tw-no-underline",
"disabled:tw-cursor-not-allowed",
"disabled:!tw-text-fg-disabled",
"disabled:hover:!tw-text-fg-disabled",
"disabled:hover:tw-no-underline",
"focus-visible:tw-outline-none",
"focus-visible:before:tw-ring-border-focus",
"[&:focus-visible_span]:tw-underline",
"[&:focus-visible_span]:tw-decoration-[.125em]",
"[&.tw-test-focus-visible_span]:tw-underline",
"[&.tw-test-focus-visible_span]:tw-decoration-[.125em]",
// Workaround for html button tag not being able to be set to `display: inline`
// and at the same time not being able to use `tw-ring-offset` because of box-shadow issue.
@@ -93,6 +92,7 @@ const commonStyles = [
"aria-disabled:!tw-text-fg-disabled",
"aria-disabled:hover:!tw-text-fg-disabled",
"aria-disabled:hover:tw-no-underline",
"[&[aria-disabled]:focus-visible_span]:!tw-no-underline",
];
@Component({

View File

@@ -1,4 +1,12 @@
import { Meta, Story, Primary, Controls, Title, Description } from "@storybook/addon-docs/blocks";
import {
Meta,
Story,
Canvas,
Primary,
Controls,
Title,
Description,
} from "@storybook/addon-docs/blocks";
import * as stories from "./link.stories";
@@ -33,15 +41,25 @@ You can use one of the following variants by providing it as the `linkType` inpu
If you want to display a link with a smaller text size, apply the `tw-text-sm` class. This will
match the `body2` variant of the Typography directive.
## With icons
## With Icon
Text Links/buttons can have icons on left or the right.
Use the `startIcon` and `endIcon` inputs to add a Bitwarden icon (`bwi-*`) before or after the link
label. Do not use a `<bit-icon>` component inside the link as this may not have the correct styling
and spacing.
To indicate a new or add action, the <i class="bwi bwi-plus-circle"></i> icon on is used on the
left.
### Icon before the label
An angle icon, <i class="bwi bwi-angle-right"></i>, is used on the left to indicate an expand to
show/hide additional content.
```html
<a bitLink startIcon="bwi-plus-circle">Add item</a>
```
### Icon after the label
```html
<a bitLink endIcon="bwi-angle-right">Next</a>
```
<Canvas of={stories.WithIcons} />
## Accessibility

View File

@@ -32,6 +32,9 @@
[ariaCurrentWhenActive]="ariaCurrentWhenActive()"
(isActiveChange)="setIsActive($event)"
(click)="mainContentClicked.emit()"
[state]="{
focusAfterNav: focusAfterNavTarget(),
}"
>
<ng-container *ngTemplateOutlet="anchorAndButtonContent"></ng-container>
</a>

View File

@@ -91,6 +91,18 @@ export class NavItemComponent extends NavBaseComponent {
*/
readonly ariaCurrentWhenActive = input<RouterLinkActive["ariaCurrentWhenActive"]>("page");
/**
* By default, a navigation will put the user's focus on the `main` element.
*
* If the user's focus should be moved to another element upon navigation end, pass a selector
* here (i.e. `#elementId`).
*
* Pass `false` to opt out of moving the focus entirely. Focus will stay on the nav item.
*
* See router-focus-manager.service for implementation of focus management
*/
readonly focusAfterNavTarget = input<string | boolean>();
/**
* The design spec calls for the an outline to wrap the entire element when the template's
* anchor/button has :focus-visible. Usually, we would use :focus-within for this. However, that

View File

@@ -5,7 +5,7 @@
[routerLinkActiveOptions]="routerLinkMatchOptions"
#rla="routerLinkActive"
[active]="rla.isActive"
[state]="{ focusMainAfterNav: false }"
[state]="{ focusAfterNav: false }"
[disabled]="disabled"
[attr.aria-disabled]="disabled"
ariaCurrentWhenActive="page"

View File

@@ -1,11 +1,13 @@
import requireLabelOnBiticonbutton from "./require-label-on-biticonbutton.mjs";
import requireThemeColorsInSvg from "./require-theme-colors-in-svg.mjs";
import noBwiClassUsage from "./no-bwi-class-usage.mjs";
import noIconChildrenInBitButton from "./no-icon-children-in-bit-button.mjs";
export default {
rules: {
"require-label-on-biticonbutton": requireLabelOnBiticonbutton,
"require-theme-colors-in-svg": requireThemeColorsInSvg,
"no-bwi-class-usage": noBwiClassUsage,
"no-icon-children-in-bit-button": noIconChildrenInBitButton,
},
};

View File

@@ -0,0 +1,74 @@
export const errorMessage =
'Avoid placing icon elements (<i class="bwi ..."> or <bit-icon>) inside a bitButton or bitLink. ' +
"Use the [startIcon] or [endIcon] inputs instead. " +
'Example: <button bitButton startIcon="bwi-plus">Label</button>';
export default {
meta: {
type: "suggestion",
docs: {
description:
"Discourage using icon child elements inside bitButton; use startIcon/endIcon inputs instead",
category: "Best Practices",
recommended: true,
},
schema: [],
},
create(context) {
return {
Element(node) {
if (node.name !== "button" && node.name !== "a") {
return;
}
const allAttrNames = [
...(node.attributes?.map((attr) => attr.name) ?? []),
...(node.inputs?.map((input) => input.name) ?? []),
];
if (!allAttrNames.includes("bitButton") && !allAttrNames.includes("bitLink")) {
return;
}
for (const child of node.children ?? []) {
if (!child.name) {
continue;
}
// <bit-icon> child
if (child.name === "bit-icon") {
context.report({
node: child,
message: errorMessage,
});
continue;
}
// <i> child with bwi class
if (child.name === "i") {
const classAttrs = [
...(child.attributes?.filter((attr) => attr.name === "class") ?? []),
...(child.inputs?.filter((input) => input.name === "class") ?? []),
];
for (const classAttr of classAttrs) {
const classValue = classAttr.value || "";
if (typeof classValue !== "string") {
continue;
}
if (/\bbwi\b/.test(classValue)) {
context.report({
node: child,
message: errorMessage,
});
break;
}
}
}
}
},
};
},
};

View File

@@ -0,0 +1,97 @@
import { RuleTester } from "@typescript-eslint/rule-tester";
import rule, { errorMessage } from "./no-icon-children-in-bit-button.mjs";
const ruleTester = new RuleTester({
languageOptions: {
parser: require("@angular-eslint/template-parser"),
},
});
ruleTester.run("no-icon-children-in-bit-button", rule.default, {
valid: [
{
name: "should allow bitButton with startIcon input",
code: `<button bitButton startIcon="bwi-plus">Add</button>`,
},
{
name: "should allow bitButton with endIcon input",
code: `<button bitButton endIcon="bwi-external-link">Open</button>`,
},
{
name: "should allow a[bitButton] with startIcon input",
code: `<a bitButton startIcon="bwi-external-link" href="https://example.com">Link</a>`,
},
{
name: "should allow <i> with bwi inside a regular button (no bitButton)",
code: `<button type="button"><i class="bwi bwi-lock"></i> Lock</button>`,
},
{
name: "should allow <bit-icon> inside a regular div",
code: `<div><bit-icon name="bwi-lock"></bit-icon></div>`,
},
{
name: "should allow bitButton with only text content",
code: `<button bitButton buttonType="primary">Save</button>`,
},
{
name: "should allow <i> without bwi class inside bitButton",
code: `<button bitButton><i class="fa fa-lock"></i> Lock</button>`,
},
{
name: "should allow bitLink with startIcon input",
code: `<a bitLink startIcon="bwi-external-link" href="https://example.com">Link</a>`,
},
{
name: "should allow bitLink with only text content",
code: `<a bitLink href="https://example.com">Link</a>`,
},
],
invalid: [
{
name: "should warn on <i> with bwi class inside button[bitButton]",
code: `<button bitButton buttonType="primary"><i class="bwi bwi-plus"></i> Add</button>`,
errors: [{ message: errorMessage }],
},
{
name: "should warn on <i> with bwi class and extra classes inside button[bitButton]",
code: `<button bitButton><i class="bwi bwi-lock tw-me-2" aria-hidden="true"></i> Lock</button>`,
errors: [{ message: errorMessage }],
},
{
name: "should warn on <i> with bwi class inside a[bitButton]",
code: `<a bitButton buttonType="secondary"><i class="bwi bwi-external-link"></i> Link</a>`,
errors: [{ message: errorMessage }],
},
{
name: "should warn on <bit-icon> inside button[bitButton]",
code: `<button bitButton buttonType="primary"><bit-icon name="bwi-lock"></bit-icon> Lock</button>`,
errors: [{ message: errorMessage }],
},
{
name: "should warn on <bit-icon> inside a[bitButton]",
code: `<a bitButton><bit-icon name="bwi-clone"></bit-icon> Copy</a>`,
errors: [{ message: errorMessage }],
},
{
name: "should warn on multiple icon children inside bitButton",
code: `<button bitButton><i class="bwi bwi-plus"></i> Add <i class="bwi bwi-angle-down"></i></button>`,
errors: [{ message: errorMessage }, { message: errorMessage }],
},
{
name: "should warn on both <i> and <bit-icon> children",
code: `<button bitButton><i class="bwi bwi-plus"></i><bit-icon name="bwi-lock"></bit-icon></button>`,
errors: [{ message: errorMessage }, { message: errorMessage }],
},
{
name: "should warn on <i> with bwi class inside a[bitLink]",
code: `<a bitLink><i class="bwi bwi-external-link"></i> Link</a>`,
errors: [{ message: errorMessage }],
},
{
name: "should warn on <bit-icon> inside button[bitLink]",
code: `<button bitLink><bit-icon name="bwi-lock"></bit-icon> Lock</button>`,
errors: [{ message: errorMessage }],
},
],
});

Some files were not shown because too many files have changed in this diff Show More