-
-
-
-
+
+ (args)}>
+
+ `,
+ }),
+};
+
+export const Variants: Story = {
+ render: (args) => ({
+ props: args,
+ template: `
+
+ (args)} variant="success">
+ (args)} variant="info">
+ (args)} variant="warning">
+ (args)} variant="error">
`,
}),
@@ -93,8 +105,8 @@ export const LongContent: Story = {
args: {
title: "Foo",
message: [
- "Lorem ipsum dolor sit amet, consectetur adipisci",
- "Lorem ipsum dolor sit amet, consectetur adipisci",
+ "Maecenas commodo posuere quam, vel malesuada nulla accumsan ac.",
+ "Pellentesque interdum ligula ante, eget bibendum ante lacinia congue.",
],
},
};
diff --git a/libs/components/src/toast/toastr.component.ts b/libs/components/src/toast/toastr.component.ts
index 75124ceb4b3..06182f094aa 100644
--- a/libs/components/src/toast/toastr.component.ts
+++ b/libs/components/src/toast/toastr.component.ts
@@ -4,6 +4,9 @@ import { Toast as BaseToastrComponent, ToastPackage, ToastrService } from "ngx-t
import { ToastComponent } from "./toast.component";
+/**
+ * Toasts are ephemeral notifications. They most often communicate the result of a user action. Due to their ephemeral nature, long messages and critical alerts should not utilize toasts.
+ */
@Component({
template: `
Date: Fri, 30 May 2025 11:40:55 -0500
Subject: [PATCH 22/33] [PM-21443] Require userId for KeyService's
everHadUserKey$ (#14712)
* Require userId for KeyService's everHadUserKey$
* handle null active user in tdeDecryptionRequiredGuard
---
.../src/auth/guards/lock.guard.spec.ts | 2 +-
libs/angular/src/auth/guards/lock.guard.ts | 2 +-
.../angular/src/auth/guards/redirect.guard.ts | 6 +-
.../tde-decryption-required.guard.spec.ts | 107 ++++++++++++++++++
.../guards/tde-decryption-required.guard.ts | 11 +-
.../src/auth/guards/unauth.guard.spec.ts | 4 +-
libs/angular/src/auth/guards/unauth.guard.ts | 2 +-
.../src/abstractions/key.service.ts | 6 +-
libs/key-management/src/key.service.spec.ts | 11 +-
libs/key-management/src/key.service.ts | 16 ++-
10 files changed, 142 insertions(+), 25 deletions(-)
create mode 100644 libs/angular/src/auth/guards/tde-decryption-required.guard.spec.ts
diff --git a/libs/angular/src/auth/guards/lock.guard.spec.ts b/libs/angular/src/auth/guards/lock.guard.spec.ts
index 32b8ecbb9dd..ed77f9bdebf 100644
--- a/libs/angular/src/auth/guards/lock.guard.spec.ts
+++ b/libs/angular/src/auth/guards/lock.guard.spec.ts
@@ -44,7 +44,7 @@ describe("lockGuard", () => {
const keyService: MockProxy = mock();
keyService.isLegacyUser.mockResolvedValue(setupParams.isLegacyUser);
- keyService.everHadUserKey$ = of(setupParams.everHadUserKey);
+ keyService.everHadUserKey$.mockReturnValue(of(setupParams.everHadUserKey));
const platformUtilService: MockProxy = mock();
platformUtilService.getClientType.mockReturnValue(setupParams.clientType);
diff --git a/libs/angular/src/auth/guards/lock.guard.ts b/libs/angular/src/auth/guards/lock.guard.ts
index 10ad4917f32..01d03dc718d 100644
--- a/libs/angular/src/auth/guards/lock.guard.ts
+++ b/libs/angular/src/auth/guards/lock.guard.ts
@@ -84,7 +84,7 @@ export function lockGuard(): CanActivateFn {
}
// If authN user with TDE directly navigates to lock, reject that navigation
- const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$);
+ const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$(activeUser.id));
if (tdeEnabled && !everHadUserKey) {
return false;
}
diff --git a/libs/angular/src/auth/guards/redirect.guard.ts b/libs/angular/src/auth/guards/redirect.guard.ts
index 00dd20c9909..b893614b405 100644
--- a/libs/angular/src/auth/guards/redirect.guard.ts
+++ b/libs/angular/src/auth/guards/redirect.guard.ts
@@ -2,8 +2,10 @@ import { inject } from "@angular/core";
import { CanActivateFn, Router } from "@angular/router";
import { firstValueFrom } from "rxjs";
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
+import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { KeyService } from "@bitwarden/key-management";
@@ -33,6 +35,7 @@ export function redirectGuard(overrides: Partial = {}): CanActiv
const authService = inject(AuthService);
const keyService = inject(KeyService);
const deviceTrustService = inject(DeviceTrustServiceAbstraction);
+ const accountService = inject(AccountService);
const logService = inject(LogService);
const router = inject(Router);
@@ -49,7 +52,8 @@ export function redirectGuard(overrides: Partial = {}): CanActiv
// If locked, TDE is enabled, and the user hasn't decrypted yet, then redirect to the
// login decryption options component.
const tdeEnabled = await firstValueFrom(deviceTrustService.supportsDeviceTrust$);
- const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$);
+ const userId = await firstValueFrom(accountService.activeAccount$.pipe(getUserId));
+ const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$(userId));
if (authStatus === AuthenticationStatus.Locked && tdeEnabled && !everHadUserKey) {
logService.info(
"Sending user to TDE decryption options. AuthStatus is %s. TDE support is %s. Ever had user key is %s.",
diff --git a/libs/angular/src/auth/guards/tde-decryption-required.guard.spec.ts b/libs/angular/src/auth/guards/tde-decryption-required.guard.spec.ts
new file mode 100644
index 00000000000..4408452a2a2
--- /dev/null
+++ b/libs/angular/src/auth/guards/tde-decryption-required.guard.spec.ts
@@ -0,0 +1,107 @@
+import { TestBed } from "@angular/core/testing";
+import { Router, provideRouter } from "@angular/router";
+import { mock } from "jest-mock-extended";
+import { BehaviorSubject, of } from "rxjs";
+
+import { EmptyComponent } from "@bitwarden/angular/platform/guard/feature-flag.guard.spec";
+import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service";
+import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
+import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
+import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction";
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { UserId } from "@bitwarden/common/types/guid";
+import { KeyService } from "@bitwarden/key-management";
+
+import { tdeDecryptionRequiredGuard } from "./tde-decryption-required.guard";
+
+describe("tdeDecryptionRequiredGuard", () => {
+ const activeUser: Account = {
+ id: "fake_user_id" as UserId,
+ email: "test@email.com",
+ emailVerified: true,
+ name: "Test User",
+ };
+
+ const setup = (
+ activeUser: Account | null,
+ authStatus: AuthenticationStatus | null = null,
+ tdeEnabled: boolean = false,
+ everHadUserKey: boolean = false,
+ ) => {
+ const accountService = mock();
+ const authService = mock();
+ const keyService = mock();
+ const deviceTrustService = mock();
+ const logService = mock();
+
+ accountService.activeAccount$ = new BehaviorSubject(activeUser);
+ if (authStatus !== null) {
+ authService.getAuthStatus.mockResolvedValue(authStatus);
+ }
+ keyService.everHadUserKey$.mockReturnValue(of(everHadUserKey));
+ deviceTrustService.supportsDeviceTrust$ = of(tdeEnabled);
+
+ const testBed = TestBed.configureTestingModule({
+ providers: [
+ { provide: AccountService, useValue: accountService },
+ { provide: AuthService, useValue: authService },
+ { provide: KeyService, useValue: keyService },
+ { provide: DeviceTrustServiceAbstraction, useValue: deviceTrustService },
+ { provide: LogService, useValue: logService },
+ provideRouter([
+ { path: "", component: EmptyComponent },
+ {
+ path: "protected-route",
+ component: EmptyComponent,
+ canActivate: [tdeDecryptionRequiredGuard()],
+ },
+ ]),
+ ],
+ });
+
+ return {
+ router: testBed.inject(Router),
+ };
+ };
+
+ it("redirects to root when the active account is null", async () => {
+ const { router } = setup(null, null);
+ await router.navigate(["protected-route"]);
+ expect(router.url).toBe("/");
+ });
+
+ test.each([AuthenticationStatus.Unlocked, AuthenticationStatus.LoggedOut])(
+ "redirects to root when the user isn't locked",
+ async (authStatus) => {
+ const { router } = setup(activeUser, authStatus);
+
+ await router.navigate(["protected-route"]);
+
+ expect(router.url).toBe("/");
+ },
+ );
+
+ it("redirects to root when TDE is not enabled", async () => {
+ const { router } = setup(activeUser, AuthenticationStatus.Locked, false, true);
+
+ await router.navigate(["protected-route"]);
+
+ expect(router.url).toBe("/");
+ });
+
+ it("redirects to root when user has had a user key", async () => {
+ const { router } = setup(activeUser, AuthenticationStatus.Locked, true, true);
+
+ await router.navigate(["protected-route"]);
+
+ expect(router.url).toBe("/");
+ });
+
+ it("allows access when user is locked, TDE is enabled, and user has never had a user key", async () => {
+ const { router } = setup(activeUser, AuthenticationStatus.Locked, true, false);
+
+ const result = await router.navigate(["protected-route"]);
+ expect(result).toBe(true);
+ expect(router.url).toBe("/protected-route");
+ });
+});
diff --git a/libs/angular/src/auth/guards/tde-decryption-required.guard.ts b/libs/angular/src/auth/guards/tde-decryption-required.guard.ts
index 1d98b1fa740..13e7c6d04e1 100644
--- a/libs/angular/src/auth/guards/tde-decryption-required.guard.ts
+++ b/libs/angular/src/auth/guards/tde-decryption-required.guard.ts
@@ -5,8 +5,9 @@ import {
RouterStateSnapshot,
CanActivateFn,
} from "@angular/router";
-import { firstValueFrom } from "rxjs";
+import { firstValueFrom, map } from "rxjs";
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction";
@@ -24,12 +25,18 @@ export function tdeDecryptionRequiredGuard(): CanActivateFn {
const authService = inject(AuthService);
const keyService = inject(KeyService);
const deviceTrustService = inject(DeviceTrustServiceAbstraction);
+ const accountService = inject(AccountService);
const logService = inject(LogService);
const router = inject(Router);
+ const userId = await firstValueFrom(accountService.activeAccount$.pipe(map((a) => a?.id)));
+ if (userId == null) {
+ return router.createUrlTree(["/"]);
+ }
+
const authStatus = await authService.getAuthStatus();
const tdeEnabled = await firstValueFrom(deviceTrustService.supportsDeviceTrust$);
- const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$);
+ const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$(userId));
// We need to determine if we should bypass the decryption options and send the user to the vault.
// The ONLY time that we want to send a user to the decryption options is when:
diff --git a/libs/angular/src/auth/guards/unauth.guard.spec.ts b/libs/angular/src/auth/guards/unauth.guard.spec.ts
index ad0ce680a1f..c696b849558 100644
--- a/libs/angular/src/auth/guards/unauth.guard.spec.ts
+++ b/libs/angular/src/auth/guards/unauth.guard.spec.ts
@@ -2,7 +2,7 @@ import { TestBed } from "@angular/core/testing";
import { Router } from "@angular/router";
import { RouterTestingModule } from "@angular/router/testing";
import { MockProxy, mock } from "jest-mock-extended";
-import { BehaviorSubject } from "rxjs";
+import { BehaviorSubject, of } from "rxjs";
import { EmptyComponent } from "@bitwarden/angular/platform/guard/feature-flag.guard.spec";
import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service";
@@ -43,7 +43,7 @@ describe("UnauthGuard", () => {
authService.authStatusFor$.mockReturnValue(activeAccountStatusObservable);
}
- keyService.everHadUserKey$ = new BehaviorSubject(everHadUserKey);
+ keyService.everHadUserKey$.mockReturnValue(of(everHadUserKey));
deviceTrustService.supportsDeviceTrustByUserId$.mockReturnValue(
new BehaviorSubject(tdeEnabled),
);
diff --git a/libs/angular/src/auth/guards/unauth.guard.ts b/libs/angular/src/auth/guards/unauth.guard.ts
index 6764b46843e..3fcfd38349b 100644
--- a/libs/angular/src/auth/guards/unauth.guard.ts
+++ b/libs/angular/src/auth/guards/unauth.guard.ts
@@ -50,7 +50,7 @@ async function unauthGuard(
const tdeEnabled = await firstValueFrom(
deviceTrustService.supportsDeviceTrustByUserId$(activeUser.id),
);
- const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$);
+ const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$(activeUser.id));
// If locked, TDE is enabled, and the user hasn't decrypted yet, then redirect to the
// login decryption options component.
diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts
index 95b79890c6a..51a99421967 100644
--- a/libs/key-management/src/abstractions/key.service.ts
+++ b/libs/key-management/src/abstractions/key.service.ts
@@ -85,11 +85,13 @@ export abstract class KeyService {
* (such as auto, biometrics, or pin)
*/
abstract refreshAdditionalKeys(): Promise;
+
/**
- * Observable value that returns whether or not the currently active user has ever had auser key,
+ * Observable value that returns whether or not the user has ever had a userKey,
* i.e. has ever been unlocked/decrypted. This is key for differentiating between TDE locked and standard locked states.
*/
- abstract everHadUserKey$: Observable;
+ abstract everHadUserKey$(userId: UserId): Observable;
+
/**
* Retrieves the user key
* @param userId The desired user
diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts
index 6d2e8fd20ec..400d7279a30 100644
--- a/libs/key-management/src/key.service.spec.ts
+++ b/libs/key-management/src/key.service.spec.ts
@@ -34,7 +34,6 @@ import {
FakeAccountService,
mockAccountServiceWith,
FakeStateProvider,
- FakeActiveUserState,
FakeSingleUserState,
} from "@bitwarden/common/spec";
import { CsprngArray } from "@bitwarden/common/types/csprng";
@@ -190,28 +189,28 @@ describe("keyService", () => {
});
describe("everHadUserKey$", () => {
- let everHadUserKeyState: FakeActiveUserState;
+ let everHadUserKeyState: FakeSingleUserState;
beforeEach(() => {
- everHadUserKeyState = stateProvider.activeUser.getFake(USER_EVER_HAD_USER_KEY);
+ everHadUserKeyState = stateProvider.singleUser.getFake(mockUserId, USER_EVER_HAD_USER_KEY);
});
it("should return true when stored value is true", async () => {
everHadUserKeyState.nextState(true);
- expect(await firstValueFrom(keyService.everHadUserKey$)).toBe(true);
+ expect(await firstValueFrom(keyService.everHadUserKey$(mockUserId))).toBe(true);
});
it("should return false when stored value is false", async () => {
everHadUserKeyState.nextState(false);
- expect(await firstValueFrom(keyService.everHadUserKey$)).toBe(false);
+ expect(await firstValueFrom(keyService.everHadUserKey$(mockUserId))).toBe(false);
});
it("should return false when stored value is null", async () => {
everHadUserKeyState.nextState(null);
- expect(await firstValueFrom(keyService.everHadUserKey$)).toBe(false);
+ expect(await firstValueFrom(keyService.everHadUserKey$(mockUserId))).toBe(false);
});
});
diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts
index 1d4fcc86a0c..fe288adeb88 100644
--- a/libs/key-management/src/key.service.ts
+++ b/libs/key-management/src/key.service.ts
@@ -41,7 +41,7 @@ import {
USER_EVER_HAD_USER_KEY,
USER_KEY,
} from "@bitwarden/common/platform/services/key-state/user-key.state";
-import { ActiveUserState, StateProvider } from "@bitwarden/common/platform/state";
+import { StateProvider } from "@bitwarden/common/platform/state";
import { CsprngArray } from "@bitwarden/common/types/csprng";
import { OrganizationId, ProviderId, UserId } from "@bitwarden/common/types/guid";
import {
@@ -63,10 +63,6 @@ import {
import { KdfConfig } from "./models/kdf-config";
export class DefaultKeyService implements KeyServiceAbstraction {
- private readonly activeUserEverHadUserKey: ActiveUserState;
-
- readonly everHadUserKey$: Observable;
-
readonly activeUserOrgKeys$: Observable>;
constructor(
@@ -82,10 +78,6 @@ export class DefaultKeyService implements KeyServiceAbstraction {
protected stateProvider: StateProvider,
protected kdfConfigService: KdfConfigService,
) {
- // User Key
- this.activeUserEverHadUserKey = stateProvider.getActive(USER_EVER_HAD_USER_KEY);
- this.everHadUserKey$ = this.activeUserEverHadUserKey.state$.pipe(map((x) => x ?? false));
-
this.activeUserOrgKeys$ = this.stateProvider.activeUserId$.pipe(
switchMap((userId) => (userId != null ? this.orgKeys$(userId) : NEVER)),
) as Observable>;
@@ -141,6 +133,12 @@ export class DefaultKeyService implements KeyServiceAbstraction {
await this.setUserKey(key, activeUserId);
}
+ everHadUserKey$(userId: UserId): Observable {
+ return this.stateProvider
+ .getUser(userId, USER_EVER_HAD_USER_KEY)
+ .state$.pipe(map((x) => x ?? false));
+ }
+
getInMemoryUserKeyFor$(userId: UserId): Observable {
return this.stateProvider.getUserState$(USER_KEY, userId);
}
From 874fe0fd1ebde3392695183edba294a25451a0d8 Mon Sep 17 00:00:00 2001
From: Tom <144813356+ttalty@users.noreply.github.com>
Date: Fri, 30 May 2025 12:55:14 -0400
Subject: [PATCH 23/33] Adding userGuid to the member details object (#14899)
---
.../dirt/reports/risk-insights/models/password-health.ts | 1 +
.../response/member-cipher-details.response.ts | 2 ++
.../risk-insights/services/risk-insights-report.service.ts | 6 +++++-
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/password-health.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/password-health.ts
index d24d8386ecd..acb4a116b8f 100644
--- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/password-health.ts
+++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/password-health.ts
@@ -97,6 +97,7 @@ export type ExposedPasswordDetail = {
* organization member to a cipher
*/
export type MemberDetailsFlat = {
+ userGuid: string;
userName: string;
email: string;
cipherId: string;
diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/response/member-cipher-details.response.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/response/member-cipher-details.response.ts
index fcf5ada4b2c..7aa52330663 100644
--- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/response/member-cipher-details.response.ts
+++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/response/member-cipher-details.response.ts
@@ -1,6 +1,7 @@
import { BaseResponse } from "@bitwarden/common/models/response/base.response";
export class MemberCipherDetailsResponse extends BaseResponse {
+ userGuid: string;
userName: string;
email: string;
useKeyConnector: boolean;
@@ -8,6 +9,7 @@ export class MemberCipherDetailsResponse extends BaseResponse {
constructor(response: any) {
super(response);
+ this.userGuid = this.getResponseProperty("UserGuid");
this.userName = this.getResponseProperty("UserName");
this.email = this.getResponseProperty("Email");
this.useKeyConnector = this.getResponseProperty("UseKeyConnector");
diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.ts
index 6fdab58115d..afd246e1836 100644
--- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.ts
+++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.ts
@@ -48,7 +48,9 @@ export class RiskInsightsReportService {
const results$ = zip(allCiphers$, memberCiphers$).pipe(
map(([allCiphers, memberCiphers]) => {
const details: MemberDetailsFlat[] = memberCiphers.flatMap((dtl) =>
- dtl.cipherIds.map((c) => this.getMemberDetailsFlat(dtl.userName, dtl.email, c)),
+ dtl.cipherIds.map((c) =>
+ this.getMemberDetailsFlat(dtl.userGuid, dtl.userName, dtl.email, c),
+ ),
);
return [allCiphers, details] as const;
}),
@@ -408,11 +410,13 @@ export class RiskInsightsReportService {
}
private getMemberDetailsFlat(
+ userGuid: string,
userName: string,
email: string,
cipherId: string,
): MemberDetailsFlat {
return {
+ userGuid: userGuid,
userName: userName,
email: email,
cipherId: cipherId,
From 4e112e2daaba5141060f8d630e7ac232d722127a Mon Sep 17 00:00:00 2001
From: tangowithfoxtrot <5676771+tangowithfoxtrot@users.noreply.github.com>
Date: Fri, 30 May 2025 10:30:08 -0700
Subject: [PATCH 24/33] feat: enable running as non-root user (#13887)
---
apps/web/entrypoint.sh | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/apps/web/entrypoint.sh b/apps/web/entrypoint.sh
index 16d1c78fb77..53e8af235fb 100644
--- a/apps/web/entrypoint.sh
+++ b/apps/web/entrypoint.sh
@@ -19,20 +19,29 @@ then
LGID=65534
fi
-# Create user and group
+if [ "$(id -u)" = "0" ]; then
+ # Create user and group
-groupadd -o -g $LGID $GROUPNAME >/dev/null 2>&1 ||
-groupmod -o -g $LGID $GROUPNAME >/dev/null 2>&1
-useradd -o -u $LUID -g $GROUPNAME -s /bin/false $USERNAME >/dev/null 2>&1 ||
-usermod -o -u $LUID -g $GROUPNAME -s /bin/false $USERNAME >/dev/null 2>&1
-mkhomedir_helper $USERNAME
+ groupadd -o -g $LGID $GROUPNAME >/dev/null 2>&1 ||
+ groupmod -o -g $LGID $GROUPNAME >/dev/null 2>&1
+ useradd -o -u $LUID -g $GROUPNAME -s /bin/false $USERNAME >/dev/null 2>&1 ||
+ usermod -o -u $LUID -g $GROUPNAME -s /bin/false $USERNAME >/dev/null 2>&1
+ mkhomedir_helper $USERNAME
-# The rest...
+ # The rest...
-chown -R $USERNAME:$GROUPNAME /etc/bitwarden
-cp /etc/bitwarden/web/app-id.json /app/app-id.json
-chown -R $USERNAME:$GROUPNAME /app
-chown -R $USERNAME:$GROUPNAME /bitwarden_server
+ chown -R $USERNAME:$GROUPNAME /etc/bitwarden
+ chown -R $USERNAME:$GROUPNAME /app
+ chown -R $USERNAME:$GROUPNAME /bitwarden_server
-exec gosu $USERNAME:$GROUPNAME dotnet /bitwarden_server/Server.dll \
- /contentRoot=/app /webRoot=. /serveUnknown=false /webVault=true
+ gosu_cmd="gosu $USERNAME:$GROUPNAME"
+else
+ gosu_cmd=""
+fi
+
+exec $gosu_cmd /bitwarden_server/Server \
+ /contentRoot=/app \
+ /webRoot=. \
+ /serveUnknown=false \
+ /webVault=true \
+ /appIdLocation=/etc/bitwarden/web/app-id.json
From 9f9cb0d13d0bc88c24e3adcac0b864a0ec336fc3 Mon Sep 17 00:00:00 2001
From: Matt Gibson
Date: Fri, 30 May 2025 10:50:54 -0700
Subject: [PATCH 25/33] Add-userid-to-encryption-methods (#14844)
* Get userId from response if available
This is a small improvement for the Auth team which avoids inspection of the access token, sometimes.
* Initialize sdk clients with a userId
* return both Cipher and encryptedFor when encrypting a cipher
Update cipher api requests to include encryptedFor attribute
* Prefer named types with documentation
* Update sdk to latest
* Fixup types
* Fixup tests
* Revert getting userId from identity token response
---------
Co-authored-by: Shane
---
.../notification.background.spec.ts | 17 +-
.../background/notification.background.ts | 5 +-
.../autofill/popup/fido2/fido2.component.ts | 6 +-
.../vault-popup-autofill.service.spec.ts | 2 +-
.../vault/components/add-edit.component.ts | 14 +-
.../fido2/fido2-authenticator.service.spec.ts | 15 +-
.../services/sdk/default-sdk.service.ts | 8 +-
.../src/vault/abstractions/cipher.service.ts | 15 +-
.../src/vault/models/data/field.data.ts | 2 +-
.../request/cipher-bulk-share.request.ts | 9 +-
.../models/request/cipher-create.request.ts | 6 +-
.../models/request/cipher-share.request.ts | 6 +-
.../models/request/cipher-with-id.request.ts | 6 +-
.../vault/models/request/cipher.request.ts | 7 +-
.../src/vault/models/view/cipher.view.ts | 2 +-
.../src/vault/services/cipher.service.spec.ts | 168 +++++++++---------
.../src/vault/services/cipher.service.ts | 140 ++++++---------
.../services/default-cipher-form.service.ts | 9 +-
.../assign-collections.component.ts | 2 +-
19 files changed, 212 insertions(+), 227 deletions(-)
diff --git a/apps/browser/src/autofill/background/notification.background.spec.ts b/apps/browser/src/autofill/background/notification.background.spec.ts
index 009efd7ff36..b161200215a 100644
--- a/apps/browser/src/autofill/background/notification.background.spec.ts
+++ b/apps/browser/src/autofill/background/notification.background.spec.ts
@@ -69,8 +69,9 @@ describe("NotificationBackground", () => {
const accountService = mock();
const organizationService = mock();
+ const userId = "testId" as UserId;
const activeAccountSubject = new BehaviorSubject<{ id: UserId } & AccountInfo>({
- id: "testId" as UserId,
+ id: userId,
email: "test@example.com",
emailVerified: true,
name: "Test User",
@@ -1141,8 +1142,11 @@ describe("NotificationBackground", () => {
convertAddLoginQueueMessageToCipherViewSpy.mockReturnValueOnce(cipherView);
editItemSpy.mockResolvedValueOnce(undefined);
cipherEncryptSpy.mockResolvedValueOnce({
- ...cipherView,
- id: "testId",
+ cipher: {
+ ...cipherView,
+ id: "testId",
+ },
+ encryptedFor: userId,
});
sendMockExtensionMessage(message, sender);
@@ -1188,6 +1192,13 @@ describe("NotificationBackground", () => {
folderExistsSpy.mockResolvedValueOnce(true);
convertAddLoginQueueMessageToCipherViewSpy.mockReturnValueOnce(cipherView);
editItemSpy.mockResolvedValueOnce(undefined);
+ cipherEncryptSpy.mockResolvedValueOnce({
+ cipher: {
+ ...cipherView,
+ id: "testId",
+ },
+ encryptedFor: userId,
+ });
const errorMessage = "fetch error";
createWithServerSpy.mockImplementation(() => {
throw new Error(errorMessage);
diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts
index a73141b7e4d..cb6a67c8137 100644
--- a/apps/browser/src/autofill/background/notification.background.ts
+++ b/apps/browser/src/autofill/background/notification.background.ts
@@ -719,9 +719,10 @@ export default class NotificationBackground {
return;
}
- const cipher = await this.cipherService.encrypt(newCipher, activeUserId);
+ const encrypted = await this.cipherService.encrypt(newCipher, activeUserId);
+ const { cipher } = encrypted;
try {
- await this.cipherService.createWithServer(cipher);
+ await this.cipherService.createWithServer(encrypted);
await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", {
itemName: newCipher?.name && String(newCipher?.name),
cipherId: cipher?.id && String(cipher?.id),
diff --git a/apps/browser/src/autofill/popup/fido2/fido2.component.ts b/apps/browser/src/autofill/popup/fido2/fido2.component.ts
index 6b7d9120195..996d1bb6176 100644
--- a/apps/browser/src/autofill/popup/fido2/fido2.component.ts
+++ b/apps/browser/src/autofill/popup/fido2/fido2.component.ts
@@ -442,10 +442,10 @@ export class Fido2Component implements OnInit, OnDestroy {
);
this.buildCipher(name, username);
- const cipher = await this.cipherService.encrypt(this.cipher, activeUserId);
+ const encrypted = await this.cipherService.encrypt(this.cipher, activeUserId);
try {
- await this.cipherService.createWithServer(cipher);
- this.cipher.id = cipher.id;
+ await this.cipherService.createWithServer(encrypted);
+ this.cipher.id = encrypted.cipher.id;
} catch (e) {
this.logService.error(e);
}
diff --git a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts
index 415aeb31081..73c3fed3276 100644
--- a/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts
+++ b/apps/browser/src/vault/popup/services/vault-popup-autofill.service.spec.ts
@@ -353,7 +353,7 @@ describe("VaultPopupAutofillService", () => {
});
it("should add a URI to the cipher and save with the server", async () => {
- const mockEncryptedCipher = {} as Cipher;
+ const mockEncryptedCipher = { cipher: {} as Cipher, encryptedFor: mockUserId };
mockCipherService.encrypt.mockResolvedValue(mockEncryptedCipher);
const result = await service.doAutofillAndSave(mockCipher);
expect(result).toBe(true);
diff --git a/libs/angular/src/vault/components/add-edit.component.ts b/libs/angular/src/vault/components/add-edit.component.ts
index 8175372cae5..8cc79a22dfd 100644
--- a/libs/angular/src/vault/components/add-edit.component.ts
+++ b/libs/angular/src/vault/components/add-edit.component.ts
@@ -26,11 +26,13 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl
import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { CollectionId, UserId } from "@bitwarden/common/types/guid";
-import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
+import {
+ CipherService,
+ EncryptionContext,
+} from "@bitwarden/common/vault/abstractions/cipher.service";
import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
import { CipherType, SecureNoteType } from "@bitwarden/common/vault/enums";
import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type";
-import { Cipher } from "@bitwarden/common/vault/models/domain/cipher";
import { CardView } from "@bitwarden/common/vault/models/view/card.view";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
@@ -740,17 +742,17 @@ export class AddEditComponent implements OnInit, OnDestroy {
return this.cipherService.encrypt(this.cipher, userId);
}
- protected saveCipher(cipher: Cipher) {
+ protected saveCipher(data: EncryptionContext) {
let orgAdmin = this.organization?.canEditAllCiphers;
// if a cipher is unassigned we want to check if they are an admin or have permission to edit any collection
- if (!cipher.collectionIds) {
+ if (!data.cipher.collectionIds) {
orgAdmin = this.organization?.canEditUnassignedCiphers;
}
return this.cipher.id == null
- ? this.cipherService.createWithServer(cipher, orgAdmin)
- : this.cipherService.updateWithServer(cipher, orgAdmin);
+ ? this.cipherService.createWithServer(data, orgAdmin)
+ : this.cipherService.updateWithServer(data, orgAdmin);
}
protected deleteCipher(userId: UserId) {
diff --git a/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts
index 5c377e1a980..78ae8253ee2 100644
--- a/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts
+++ b/libs/common/src/platform/services/fido2/fido2-authenticator.service.spec.ts
@@ -6,7 +6,7 @@ import { BehaviorSubject, of } from "rxjs";
import { mockAccountServiceWith } from "../../../../spec";
import { Account } from "../../../auth/abstractions/account.service";
import { UserId } from "../../../types/guid";
-import { CipherService } from "../../../vault/abstractions/cipher.service";
+import { CipherService, EncryptionContext } from "../../../vault/abstractions/cipher.service";
import { SyncService } from "../../../vault/abstractions/sync/sync.service.abstraction";
import { CipherRepromptType } from "../../../vault/enums/cipher-reprompt-type";
import { CipherType } from "../../../vault/enums/cipher-type";
@@ -36,8 +36,9 @@ type ParentWindowReference = string;
const RpId = "bitwarden.com";
describe("FidoAuthenticatorService", () => {
+ const userId = "testId" as UserId;
const activeAccountSubject = new BehaviorSubject({
- id: "testId" as UserId,
+ id: userId,
email: "test@example.com",
emailVerified: true,
name: "Test User",
@@ -254,7 +255,7 @@ describe("FidoAuthenticatorService", () => {
cipherId: existingCipher.id,
userVerified: false,
});
- cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as Cipher);
+ cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as EncryptionContext);
await authenticator.makeCredential(params, windowReference);
@@ -325,7 +326,7 @@ describe("FidoAuthenticatorService", () => {
cipherId: existingCipher.id,
userVerified: false,
});
- cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as Cipher);
+ cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as EncryptionContext);
cipherService.updateWithServer.mockRejectedValue(new Error("Internal error"));
const result = async () => await authenticator.makeCredential(params, windowReference);
@@ -357,13 +358,13 @@ describe("FidoAuthenticatorService", () => {
cipherService.decrypt.mockResolvedValue(cipher);
cipherService.encrypt.mockImplementation(async (cipher) => {
cipher.login.fido2Credentials[0].credentialId = credentialId; // Replace id for testability
- return {} as any;
+ return { cipher: {} as any as Cipher, encryptedFor: userId };
});
- cipherService.createWithServer.mockImplementation(async (cipher) => {
+ cipherService.createWithServer.mockImplementation(async ({ cipher }) => {
cipher.id = cipherId;
return cipher;
});
- cipherService.updateWithServer.mockImplementation(async (cipher) => {
+ cipherService.updateWithServer.mockImplementation(async ({ cipher }) => {
cipher.id = cipherId;
return cipher;
});
diff --git a/libs/common/src/platform/services/sdk/default-sdk.service.ts b/libs/common/src/platform/services/sdk/default-sdk.service.ts
index 6be89a4b376..d9f7ba19a6f 100644
--- a/libs/common/src/platform/services/sdk/default-sdk.service.ts
+++ b/libs/common/src/platform/services/sdk/default-sdk.service.ts
@@ -180,9 +180,7 @@ export class DefaultSdkService implements SdkService {
return () => client?.markForDisposal();
});
}),
- tap({
- finalize: () => this.sdkClientCache.delete(userId),
- }),
+ tap({ finalize: () => this.sdkClientCache.delete(userId) }),
shareReplay({ refCount: true, bufferSize: 1 }),
);
@@ -205,9 +203,7 @@ export class DefaultSdkService implements SdkService {
method: { decryptedKey: { decrypted_user_key: userKey.keyB64 } },
kdfParams:
kdfParams.kdfType === KdfType.PBKDF2_SHA256
- ? {
- pBKDF2: { iterations: kdfParams.iterations },
- }
+ ? { pBKDF2: { iterations: kdfParams.iterations } }
: {
argon2id: {
iterations: kdfParams.iterations,
diff --git a/libs/common/src/vault/abstractions/cipher.service.ts b/libs/common/src/vault/abstractions/cipher.service.ts
index fc809058161..91f8006d15e 100644
--- a/libs/common/src/vault/abstractions/cipher.service.ts
+++ b/libs/common/src/vault/abstractions/cipher.service.ts
@@ -21,6 +21,12 @@ import { CipherView } from "../models/view/cipher.view";
import { FieldView } from "../models/view/field.view";
import { AddEditCipherInfo } from "../types/add-edit-cipher-info";
+export type EncryptionContext = {
+ cipher: Cipher;
+ /** The Id of the user that encrypted the cipher. It should always represent a UserId, even for Organization-owned ciphers */
+ encryptedFor: UserId;
+};
+
export abstract class CipherService implements UserKeyRotationDataProvider {
abstract cipherViews$(userId: UserId): Observable;
abstract ciphers$(userId: UserId): Observable>;
@@ -42,7 +48,7 @@ export abstract class CipherService implements UserKeyRotationDataProvider;
+ ): Promise;
abstract encryptFields(fieldsModel: FieldView[], key: SymmetricCryptoKey): Promise;
abstract encryptField(fieldModel: FieldView, key: SymmetricCryptoKey): Promise;
abstract get(id: string, userId: UserId): Promise;
@@ -94,7 +100,10 @@ export abstract class CipherService implements UserKeyRotationDataProvider;
+ abstract createWithServer(
+ { cipher, encryptedFor }: EncryptionContext,
+ orgAdmin?: boolean,
+ ): Promise;
/**
* Update a cipher with the server
* @param cipher The cipher to update
@@ -104,7 +113,7 @@ export abstract class CipherService implements UserKeyRotationDataProvider;
diff --git a/libs/common/src/vault/models/data/field.data.ts b/libs/common/src/vault/models/data/field.data.ts
index b9daf7fa423..cf9df69a6b0 100644
--- a/libs/common/src/vault/models/data/field.data.ts
+++ b/libs/common/src/vault/models/data/field.data.ts
@@ -7,7 +7,7 @@ export class FieldData {
type: FieldType;
name: string;
value: string;
- linkedId: LinkedIdType;
+ linkedId: LinkedIdType | null;
constructor(response?: FieldApi) {
if (response == null) {
diff --git a/libs/common/src/vault/models/request/cipher-bulk-share.request.ts b/libs/common/src/vault/models/request/cipher-bulk-share.request.ts
index 4f56297d0a5..d0c394bea00 100644
--- a/libs/common/src/vault/models/request/cipher-bulk-share.request.ts
+++ b/libs/common/src/vault/models/request/cipher-bulk-share.request.ts
@@ -1,5 +1,6 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
+import { UserId } from "../../../types/guid";
import { Cipher } from "../domain/cipher";
import { CipherWithIdRequest } from "./cipher-with-id.request";
@@ -8,11 +9,15 @@ export class CipherBulkShareRequest {
ciphers: CipherWithIdRequest[];
collectionIds: string[];
- constructor(ciphers: Cipher[], collectionIds: string[]) {
+ constructor(
+ ciphers: Cipher[],
+ collectionIds: string[],
+ readonly encryptedFor: UserId,
+ ) {
if (ciphers != null) {
this.ciphers = [];
ciphers.forEach((c) => {
- this.ciphers.push(new CipherWithIdRequest(c));
+ this.ciphers.push(new CipherWithIdRequest({ cipher: c, encryptedFor }));
});
}
this.collectionIds = collectionIds;
diff --git a/libs/common/src/vault/models/request/cipher-create.request.ts b/libs/common/src/vault/models/request/cipher-create.request.ts
index 9c3be5544b9..e992ebed9b2 100644
--- a/libs/common/src/vault/models/request/cipher-create.request.ts
+++ b/libs/common/src/vault/models/request/cipher-create.request.ts
@@ -1,4 +1,4 @@
-import { Cipher } from "../domain/cipher";
+import { EncryptionContext } from "../../abstractions/cipher.service";
import { CipherRequest } from "./cipher.request";
@@ -6,8 +6,8 @@ export class CipherCreateRequest {
cipher: CipherRequest;
collectionIds: string[];
- constructor(cipher: Cipher) {
- this.cipher = new CipherRequest(cipher);
+ constructor({ cipher, encryptedFor }: EncryptionContext) {
+ this.cipher = new CipherRequest({ cipher, encryptedFor });
this.collectionIds = cipher.collectionIds;
}
}
diff --git a/libs/common/src/vault/models/request/cipher-share.request.ts b/libs/common/src/vault/models/request/cipher-share.request.ts
index 4043599ce05..17c46168efe 100644
--- a/libs/common/src/vault/models/request/cipher-share.request.ts
+++ b/libs/common/src/vault/models/request/cipher-share.request.ts
@@ -1,4 +1,4 @@
-import { Cipher } from "../domain/cipher";
+import { EncryptionContext } from "../../abstractions/cipher.service";
import { CipherRequest } from "./cipher.request";
@@ -6,8 +6,8 @@ export class CipherShareRequest {
cipher: CipherRequest;
collectionIds: string[];
- constructor(cipher: Cipher) {
- this.cipher = new CipherRequest(cipher);
+ constructor({ cipher, encryptedFor }: EncryptionContext) {
+ this.cipher = new CipherRequest({ cipher, encryptedFor });
this.collectionIds = cipher.collectionIds;
}
}
diff --git a/libs/common/src/vault/models/request/cipher-with-id.request.ts b/libs/common/src/vault/models/request/cipher-with-id.request.ts
index f291e342640..0b04f50fb1e 100644
--- a/libs/common/src/vault/models/request/cipher-with-id.request.ts
+++ b/libs/common/src/vault/models/request/cipher-with-id.request.ts
@@ -1,12 +1,12 @@
-import { Cipher } from "../domain/cipher";
+import { EncryptionContext } from "../../abstractions/cipher.service";
import { CipherRequest } from "./cipher.request";
export class CipherWithIdRequest extends CipherRequest {
id: string;
- constructor(cipher: Cipher) {
- super(cipher);
+ constructor({ cipher, encryptedFor }: EncryptionContext) {
+ super({ cipher, encryptedFor });
this.id = cipher.id;
}
}
diff --git a/libs/common/src/vault/models/request/cipher.request.ts b/libs/common/src/vault/models/request/cipher.request.ts
index 5b77ee7508e..2e3b2efbedc 100644
--- a/libs/common/src/vault/models/request/cipher.request.ts
+++ b/libs/common/src/vault/models/request/cipher.request.ts
@@ -1,5 +1,7 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
+import { UserId } from "../../../types/guid";
+import { EncryptionContext } from "../../abstractions/cipher.service";
import { CipherRepromptType } from "../../enums/cipher-reprompt-type";
import { CipherType } from "../../enums/cipher-type";
import { CardApi } from "../api/card.api";
@@ -10,12 +12,12 @@ import { LoginUriApi } from "../api/login-uri.api";
import { LoginApi } from "../api/login.api";
import { SecureNoteApi } from "../api/secure-note.api";
import { SshKeyApi } from "../api/ssh-key.api";
-import { Cipher } from "../domain/cipher";
import { AttachmentRequest } from "./attachment.request";
import { PasswordHistoryRequest } from "./password-history.request";
export class CipherRequest {
+ encryptedFor: UserId;
type: CipherType;
folderId: string;
organizationId: string;
@@ -36,8 +38,9 @@ export class CipherRequest {
reprompt: CipherRepromptType;
key: string;
- constructor(cipher: Cipher) {
+ constructor({ cipher, encryptedFor }: EncryptionContext) {
this.type = cipher.type;
+ this.encryptedFor = encryptedFor;
this.folderId = cipher.folderId;
this.organizationId = cipher.organizationId;
this.name = cipher.name ? cipher.name.encryptedString : null;
diff --git a/libs/common/src/vault/models/view/cipher.view.ts b/libs/common/src/vault/models/view/cipher.view.ts
index 1f73903a5bc..e182025a332 100644
--- a/libs/common/src/vault/models/view/cipher.view.ts
+++ b/libs/common/src/vault/models/view/cipher.view.ts
@@ -25,7 +25,7 @@ export class CipherView implements View, InitializerMetadata {
readonly initializerKey = InitializerKey.CipherView;
id: string = null;
- organizationId: string = null;
+ organizationId: string | undefined = null;
folderId: string = null;
name: string = null;
notes: string = null;
diff --git a/libs/common/src/vault/services/cipher.service.spec.ts b/libs/common/src/vault/services/cipher.service.spec.ts
index 9e56bac2ca0..1a0b1568775 100644
--- a/libs/common/src/vault/services/cipher.service.spec.ts
+++ b/libs/common/src/vault/services/cipher.service.spec.ts
@@ -27,6 +27,7 @@ import { ContainerService } from "../../platform/services/container.service";
import { CipherId, UserId } from "../../types/guid";
import { CipherKey, OrgKey, UserKey } from "../../types/key";
import { CipherEncryptionService } from "../abstractions/cipher-encryption.service";
+import { EncryptionContext } from "../abstractions/cipher.service";
import { CipherFileUploadService } from "../abstractions/file-upload/cipher-file-upload.service";
import { FieldType } from "../enums";
import { CipherRepromptType } from "../enums/cipher-reprompt-type";
@@ -78,36 +79,12 @@ const cipherData: CipherData = {
},
passwordHistory: [{ password: "EncryptedString", lastUsedDate: "2022-01-31T12:00:00.000Z" }],
attachments: [
- {
- id: "a1",
- url: "url",
- size: "1100",
- sizeName: "1.1 KB",
- fileName: "file",
- key: "EncKey",
- },
- {
- id: "a2",
- url: "url",
- size: "1100",
- sizeName: "1.1 KB",
- fileName: "file",
- key: "EncKey",
- },
+ { id: "a1", url: "url", size: "1100", sizeName: "1.1 KB", fileName: "file", key: "EncKey" },
+ { id: "a2", url: "url", size: "1100", sizeName: "1.1 KB", fileName: "file", key: "EncKey" },
],
fields: [
- {
- name: "EncryptedString",
- value: "EncryptedString",
- type: FieldType.Text,
- linkedId: null,
- },
- {
- name: "EncryptedString",
- value: "EncryptedString",
- type: FieldType.Hidden,
- linkedId: null,
- },
+ { name: "EncryptedString", value: "EncryptedString", type: FieldType.Text, linkedId: null },
+ { name: "EncryptedString", value: "EncryptedString", type: FieldType.Hidden, linkedId: null },
],
};
const mockUserId = Utils.newGuid() as UserId;
@@ -133,7 +110,7 @@ describe("Cipher Service", () => {
const userId = "TestUserId" as UserId;
let cipherService: CipherService;
- let cipherObj: Cipher;
+ let encryptionContext: EncryptionContext;
beforeEach(() => {
encryptService.encryptFileData.mockReturnValue(Promise.resolve(ENCRYPTED_BYTES));
@@ -159,7 +136,7 @@ describe("Cipher Service", () => {
cipherEncryptionService,
);
- cipherObj = new Cipher(cipherData);
+ encryptionContext = { cipher: new Cipher(cipherData), encryptedFor: userId };
});
afterEach(() => {
@@ -192,33 +169,33 @@ describe("Cipher Service", () => {
it("should call apiService.postCipherAdmin when orgAdmin param is true and the cipher orgId != null", async () => {
const spy = jest
.spyOn(apiService, "postCipherAdmin")
- .mockImplementation(() => Promise.resolve(cipherObj.toCipherData()));
- await cipherService.createWithServer(cipherObj, true);
- const expectedObj = new CipherCreateRequest(cipherObj);
+ .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData()));
+ await cipherService.createWithServer(encryptionContext, true);
+ const expectedObj = new CipherCreateRequest(encryptionContext);
expect(spy).toHaveBeenCalled();
expect(spy).toHaveBeenCalledWith(expectedObj);
});
it("should call apiService.postCipher when orgAdmin param is true and the cipher orgId is null", async () => {
- cipherObj.organizationId = null;
+ encryptionContext.cipher.organizationId = null!;
const spy = jest
.spyOn(apiService, "postCipher")
- .mockImplementation(() => Promise.resolve(cipherObj.toCipherData()));
- await cipherService.createWithServer(cipherObj, true);
- const expectedObj = new CipherRequest(cipherObj);
+ .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData()));
+ await cipherService.createWithServer(encryptionContext, true);
+ const expectedObj = new CipherRequest(encryptionContext);
expect(spy).toHaveBeenCalled();
expect(spy).toHaveBeenCalledWith(expectedObj);
});
it("should call apiService.postCipherCreate if collectionsIds != null", async () => {
- cipherObj.collectionIds = ["123"];
+ encryptionContext.cipher.collectionIds = ["123"];
const spy = jest
.spyOn(apiService, "postCipherCreate")
- .mockImplementation(() => Promise.resolve(cipherObj.toCipherData()));
- await cipherService.createWithServer(cipherObj);
- const expectedObj = new CipherCreateRequest(cipherObj);
+ .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData()));
+ await cipherService.createWithServer(encryptionContext);
+ const expectedObj = new CipherCreateRequest(encryptionContext);
expect(spy).toHaveBeenCalled();
expect(spy).toHaveBeenCalledWith(expectedObj);
@@ -227,9 +204,9 @@ describe("Cipher Service", () => {
it("should call apiService.postCipher when orgAdmin and collectionIds logic is false", async () => {
const spy = jest
.spyOn(apiService, "postCipher")
- .mockImplementation(() => Promise.resolve(cipherObj.toCipherData()));
- await cipherService.createWithServer(cipherObj);
- const expectedObj = new CipherRequest(cipherObj);
+ .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData()));
+ await cipherService.createWithServer(encryptionContext);
+ const expectedObj = new CipherRequest(encryptionContext);
expect(spy).toHaveBeenCalled();
expect(spy).toHaveBeenCalledWith(expectedObj);
@@ -240,36 +217,36 @@ describe("Cipher Service", () => {
it("should call apiService.putCipherAdmin when orgAdmin param is true", async () => {
const spy = jest
.spyOn(apiService, "putCipherAdmin")
- .mockImplementation(() => Promise.resolve(cipherObj.toCipherData()));
- await cipherService.updateWithServer(cipherObj, true);
- const expectedObj = new CipherRequest(cipherObj);
+ .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData()));
+ await cipherService.updateWithServer(encryptionContext, true);
+ const expectedObj = new CipherRequest(encryptionContext);
expect(spy).toHaveBeenCalled();
- expect(spy).toHaveBeenCalledWith(cipherObj.id, expectedObj);
+ expect(spy).toHaveBeenCalledWith(encryptionContext.cipher.id, expectedObj);
});
it("should call apiService.putCipher if cipher.edit is true", async () => {
- cipherObj.edit = true;
+ encryptionContext.cipher.edit = true;
const spy = jest
.spyOn(apiService, "putCipher")
- .mockImplementation(() => Promise.resolve(cipherObj.toCipherData()));
- await cipherService.updateWithServer(cipherObj);
- const expectedObj = new CipherRequest(cipherObj);
+ .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData()));
+ await cipherService.updateWithServer(encryptionContext);
+ const expectedObj = new CipherRequest(encryptionContext);
expect(spy).toHaveBeenCalled();
- expect(spy).toHaveBeenCalledWith(cipherObj.id, expectedObj);
+ expect(spy).toHaveBeenCalledWith(encryptionContext.cipher.id, expectedObj);
});
it("should call apiService.putPartialCipher when orgAdmin, and edit are false", async () => {
- cipherObj.edit = false;
+ encryptionContext.cipher.edit = false;
const spy = jest
.spyOn(apiService, "putPartialCipher")
- .mockImplementation(() => Promise.resolve(cipherObj.toCipherData()));
- await cipherService.updateWithServer(cipherObj);
- const expectedObj = new CipherPartialRequest(cipherObj);
+ .mockImplementation(() => Promise.resolve(encryptionContext.cipher.toCipherData()));
+ await cipherService.updateWithServer(encryptionContext);
+ const expectedObj = new CipherPartialRequest(encryptionContext.cipher);
expect(spy).toHaveBeenCalled();
- expect(spy).toHaveBeenCalledWith(cipherObj.id, expectedObj);
+ expect(spy).toHaveBeenCalledWith(encryptionContext.cipher.id, expectedObj);
});
});
@@ -293,6 +270,15 @@ describe("Cipher Service", () => {
jest.spyOn(cipherService as any, "getAutofillOnPageLoadDefault").mockResolvedValue(true);
});
+ it("should return the encrypting user id", async () => {
+ keyService.getOrgKey.mockReturnValue(
+ Promise.resolve(new SymmetricCryptoKey(new Uint8Array(32)) as OrgKey),
+ );
+
+ const { encryptedFor } = await cipherService.encrypt(cipherView, userId);
+ expect(encryptedFor).toEqual(userId);
+ });
+
describe("login encryption", () => {
it("should add a uri hash to login uris", async () => {
encryptService.hash.mockImplementation((value) => Promise.resolve(`${value} hash`));
@@ -304,9 +290,9 @@ describe("Cipher Service", () => {
Promise.resolve(new SymmetricCryptoKey(new Uint8Array(32)) as OrgKey),
);
- const domain = await cipherService.encrypt(cipherView, userId);
+ const { cipher } = await cipherService.encrypt(cipherView, userId);
- expect(domain.login.uris).toEqual([
+ expect(cipher.login.uris).toEqual([
{
uri: new EncString("uri has been encrypted"),
uriChecksum: new EncString("uri hash has been encrypted"),
@@ -325,7 +311,7 @@ describe("Cipher Service", () => {
it("is null when feature flag is false", async () => {
configService.getFeatureFlag.mockResolvedValue(false);
- const cipher = await cipherService.encrypt(cipherView, userId);
+ const { cipher } = await cipherService.encrypt(cipherView, userId);
expect(cipher.key).toBeNull();
});
@@ -338,7 +324,7 @@ describe("Cipher Service", () => {
it("is null when the cipher is not viewPassword", async () => {
cipherView.viewPassword = false;
- const cipher = await cipherService.encrypt(cipherView, userId);
+ const { cipher } = await cipherService.encrypt(cipherView, userId);
expect(cipher.key).toBeNull();
});
@@ -346,7 +332,7 @@ describe("Cipher Service", () => {
it("is defined when the cipher is viewPassword", async () => {
cipherView.viewPassword = true;
- const cipher = await cipherService.encrypt(cipherView, userId);
+ const { cipher } = await cipherService.encrypt(cipherView, userId);
expect(cipher.key).toBeDefined();
});
@@ -393,7 +379,13 @@ describe("Cipher Service", () => {
it("is called when cipher viewPassword is false and original cipher has a key", async () => {
cipherView.viewPassword = false;
- await cipherService.encrypt(cipherView, userId, undefined, undefined, cipherObj);
+ await cipherService.encrypt(
+ cipherView,
+ userId,
+ undefined,
+ undefined,
+ encryptionContext.cipher,
+ );
expect(cipherService["encryptCipherWithCipherKey"]).toHaveBeenCalled();
});
@@ -416,22 +408,17 @@ describe("Cipher Service", () => {
stateService.getUserId.mockResolvedValue(mockUserId);
- const keys = {
- userKey: originalUserKey,
- } as CipherDecryptionKeys;
+ const keys = { userKey: originalUserKey } as CipherDecryptionKeys;
keyService.cipherDecryptionKeys$.mockReturnValue(of(keys));
- const cipher1 = new CipherView(cipherObj);
- cipher1.id = "Cipher 1";
+ const cipher1 = new CipherView(encryptionContext.cipher);
+ cipher1.id = "Cipher 1" as CipherId;
cipher1.organizationId = null;
- const cipher2 = new CipherView(cipherObj);
- cipher2.id = "Cipher 2";
+ const cipher2 = new CipherView(encryptionContext.cipher);
+ cipher2.id = "Cipher 2" as CipherId;
cipher2.organizationId = null;
- decryptedCiphers = new BehaviorSubject({
- Cipher1: cipher1,
- Cipher2: cipher2,
- });
+ decryptedCiphers = new BehaviorSubject({ [cipher1.id]: cipher1, [cipher2.id]: cipher2 });
jest
.spyOn(cipherService, "cipherViews$")
.mockImplementation((userId: UserId) =>
@@ -462,19 +449,19 @@ describe("Cipher Service", () => {
});
it("throws if the original user key is null", async () => {
- await expect(cipherService.getRotatedData(null, newUserKey, mockUserId)).rejects.toThrow(
+ await expect(cipherService.getRotatedData(null!, newUserKey, mockUserId)).rejects.toThrow(
"Original user key is required to rotate ciphers",
);
});
it("throws if the new user key is null", async () => {
- await expect(cipherService.getRotatedData(originalUserKey, null, mockUserId)).rejects.toThrow(
- "New user key is required to rotate ciphers",
- );
+ await expect(
+ cipherService.getRotatedData(originalUserKey, null!, mockUserId),
+ ).rejects.toThrow("New user key is required to rotate ciphers");
});
it("throws if the user has any failed to decrypt ciphers", async () => {
- const badCipher = new CipherView(cipherObj);
+ const badCipher = new CipherView(encryptionContext.cipher);
badCipher.id = "Cipher 3";
badCipher.organizationId = null;
badCipher.decryptionFailure = true;
@@ -488,12 +475,15 @@ describe("Cipher Service", () => {
describe("decrypt", () => {
it("should call decrypt method of CipherEncryptionService when feature flag is true", async () => {
configService.getFeatureFlag.mockResolvedValue(true);
- cipherEncryptionService.decrypt.mockResolvedValue(new CipherView(cipherObj));
+ cipherEncryptionService.decrypt.mockResolvedValue(new CipherView(encryptionContext.cipher));
- const result = await cipherService.decrypt(cipherObj, userId);
+ const result = await cipherService.decrypt(encryptionContext.cipher, userId);
- expect(result).toEqual(new CipherView(cipherObj));
- expect(cipherEncryptionService.decrypt).toHaveBeenCalledWith(cipherObj, userId);
+ expect(result).toEqual(new CipherView(encryptionContext.cipher));
+ expect(cipherEncryptionService.decrypt).toHaveBeenCalledWith(
+ encryptionContext.cipher,
+ userId,
+ );
});
it("should call legacy decrypt when feature flag is false", async () => {
@@ -501,12 +491,14 @@ describe("Cipher Service", () => {
configService.getFeatureFlag.mockResolvedValue(false);
cipherService.getKeyForCipherKeyDecryption = jest.fn().mockResolvedValue(mockUserKey);
encryptService.decryptToBytes.mockResolvedValue(new Uint8Array(32));
- jest.spyOn(cipherObj, "decrypt").mockResolvedValue(new CipherView(cipherObj));
+ jest
+ .spyOn(encryptionContext.cipher, "decrypt")
+ .mockResolvedValue(new CipherView(encryptionContext.cipher));
- const result = await cipherService.decrypt(cipherObj, userId);
+ const result = await cipherService.decrypt(encryptionContext.cipher, userId);
- expect(result).toEqual(new CipherView(cipherObj));
- expect(cipherObj.decrypt).toHaveBeenCalledWith(mockUserKey);
+ expect(result).toEqual(new CipherView(encryptionContext.cipher));
+ expect(encryptionContext.cipher.decrypt).toHaveBeenCalledWith(mockUserKey);
});
});
diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts
index 2693d9d4644..0c948fe0c6b 100644
--- a/libs/common/src/vault/services/cipher.service.ts
+++ b/libs/common/src/vault/services/cipher.service.ts
@@ -33,7 +33,10 @@ import { CipherId, CollectionId, OrganizationId, UserId } from "../../types/guid
import { OrgKey, UserKey } from "../../types/key";
import { filterOutNullish, perUserCache$ } from "../../vault/utils/observable-utilities";
import { CipherEncryptionService } from "../abstractions/cipher-encryption.service";
-import { CipherService as CipherServiceAbstraction } from "../abstractions/cipher.service";
+import {
+ CipherService as CipherServiceAbstraction,
+ EncryptionContext,
+} from "../abstractions/cipher.service";
import { CipherFileUploadService } from "../abstractions/file-upload/cipher-file-upload.service";
import { FieldType } from "../enums";
import { CipherType } from "../enums/cipher-type";
@@ -196,7 +199,7 @@ export class CipherService implements CipherServiceAbstraction {
keyForCipherEncryption?: SymmetricCryptoKey,
keyForCipherKeyDecryption?: SymmetricCryptoKey,
originalCipher: Cipher = null,
- ): Promise {
+ ): Promise {
if (model.id != null) {
if (originalCipher == null) {
originalCipher = await this.get(model.id, userId);
@@ -230,18 +233,24 @@ export class CipherService implements CipherServiceAbstraction {
keyForCipherEncryption ||= userOrOrgKey;
// If the caller has provided a key for cipher key decryption, use it. Otherwise, use the user or org key.
keyForCipherKeyDecryption ||= userOrOrgKey;
- return this.encryptCipherWithCipherKey(
- model,
- cipher,
- keyForCipherEncryption,
- keyForCipherKeyDecryption,
- );
+ return {
+ cipher: await this.encryptCipherWithCipherKey(
+ model,
+ cipher,
+ keyForCipherEncryption,
+ keyForCipherKeyDecryption,
+ ),
+ encryptedFor: userId,
+ };
} else {
keyForCipherEncryption ||= await this.getKeyForCipherKeyDecryption(cipher, userId);
// We want to ensure that the cipher key is null if cipher key encryption is disabled
// so that decryption uses the proper key.
cipher.key = null;
- return this.encryptCipher(model, cipher, keyForCipherEncryption);
+ return {
+ cipher: await this.encryptCipher(model, cipher, keyForCipherEncryption),
+ encryptedFor: userId,
+ };
}
}
@@ -261,19 +270,14 @@ export class CipherService implements CipherServiceAbstraction {
attachment.size = model.size;
attachment.sizeName = model.sizeName;
attachment.url = model.url;
- const promise = this.encryptObjProperty(
- model,
- attachment,
- {
- fileName: null,
+ const promise = this.encryptObjProperty(model, attachment, { fileName: null }, key).then(
+ async () => {
+ if (model.key != null) {
+ attachment.key = await this.encryptService.wrapSymmetricKey(model.key, key);
+ }
+ encAttachments.push(attachment);
},
- key,
- ).then(async () => {
- if (model.key != null) {
- attachment.key = await this.encryptService.wrapSymmetricKey(model.key, key);
- }
- encAttachments.push(attachment);
- });
+ );
promises.push(promise);
});
@@ -306,15 +310,7 @@ export class CipherService implements CipherServiceAbstraction {
fieldModel.value = "false";
}
- await this.encryptObjProperty(
- fieldModel,
- field,
- {
- name: null,
- value: null,
- },
- key,
- );
+ await this.encryptObjProperty(fieldModel, field, { name: null, value: null }, key);
return field;
}
@@ -345,14 +341,7 @@ export class CipherService implements CipherServiceAbstraction {
const ph = new Password();
ph.lastUsedDate = phModel.lastUsedDate;
- await this.encryptObjProperty(
- phModel,
- ph,
- {
- password: null,
- },
- key,
- );
+ await this.encryptObjProperty(phModel, ph, { password: null }, key);
return ph;
}
@@ -705,9 +694,7 @@ export class CipherService implements CipherServiceAbstraction {
if (ciphersLocalData[cipherId]) {
ciphersLocalData[cipherId].lastUsedDate = new Date().getTime();
} else {
- ciphersLocalData[cipherId] = {
- lastUsedDate: new Date().getTime(),
- };
+ ciphersLocalData[cipherId] = { lastUsedDate: new Date().getTime() };
}
await this.localDataState(userId).update(() => ciphersLocalData);
@@ -735,10 +722,7 @@ export class CipherService implements CipherServiceAbstraction {
}
const currentTime = new Date().getTime();
- ciphersLocalData[id as CipherId] = {
- lastLaunched: currentTime,
- lastUsedDate: currentTime,
- };
+ ciphersLocalData[id as CipherId] = { lastLaunched: currentTime, lastUsedDate: currentTime };
await this.localDataState(userId).update(() => ciphersLocalData);
@@ -770,18 +754,21 @@ export class CipherService implements CipherServiceAbstraction {
await this.domainSettingsService.setNeverDomains(domains);
}
- async createWithServer(cipher: Cipher, orgAdmin?: boolean): Promise {
+ async createWithServer(
+ { cipher, encryptedFor }: EncryptionContext,
+ orgAdmin?: boolean,
+ ): Promise {
let response: CipherResponse;
if (orgAdmin && cipher.organizationId != null) {
- const request = new CipherCreateRequest(cipher);
+ const request = new CipherCreateRequest({ cipher, encryptedFor });
response = await this.apiService.postCipherAdmin(request);
const data = new CipherData(response, cipher.collectionIds);
return new Cipher(data);
} else if (cipher.collectionIds != null) {
- const request = new CipherCreateRequest(cipher);
+ const request = new CipherCreateRequest({ cipher, encryptedFor });
response = await this.apiService.postCipherCreate(request);
} else {
- const request = new CipherRequest(cipher);
+ const request = new CipherRequest({ cipher, encryptedFor });
response = await this.apiService.postCipher(request);
}
cipher.id = response.id;
@@ -792,15 +779,18 @@ export class CipherService implements CipherServiceAbstraction {
return new Cipher(updated[cipher.id as CipherId]);
}
- async updateWithServer(cipher: Cipher, orgAdmin?: boolean): Promise {
+ async updateWithServer(
+ { cipher, encryptedFor }: EncryptionContext,
+ orgAdmin?: boolean,
+ ): Promise {
let response: CipherResponse;
if (orgAdmin) {
- const request = new CipherRequest(cipher);
+ const request = new CipherRequest({ cipher, encryptedFor });
response = await this.apiService.putCipherAdmin(cipher.id, request);
const data = new CipherData(response, cipher.collectionIds);
return new Cipher(data, cipher.localData);
} else if (cipher.edit) {
- const request = new CipherRequest(cipher);
+ const request = new CipherRequest({ cipher, encryptedFor });
response = await this.apiService.putCipher(cipher.id, request);
} else {
const request = new CipherPartialRequest(cipher);
@@ -854,12 +844,12 @@ export class CipherService implements CipherServiceAbstraction {
cipher.collectionIds = collectionIds;
promises.push(
this.encryptSharedCipher(cipher, userId).then((c) => {
- encCiphers.push(c);
+ encCiphers.push(c.cipher);
}),
);
}
await Promise.all(promises);
- const request = new CipherBulkShareRequest(encCiphers, collectionIds);
+ const request = new CipherBulkShareRequest(encCiphers, collectionIds, userId);
try {
await this.apiService.putShareCiphers(request);
} catch (e) {
@@ -921,8 +911,8 @@ export class CipherService implements CipherServiceAbstraction {
//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);
- cipher = await this.encrypt(model, userId);
- await this.updateWithServer(cipher);
+ const reEncrypted = await this.encrypt(model, userId);
+ await this.updateWithServer(reEncrypted);
}
const encFileName = await this.encryptService.encryptString(filename, cipherEncKey);
@@ -1482,7 +1472,7 @@ export class CipherService implements CipherServiceAbstraction {
// In the case of a cipher that is being shared with an organization, we want to decrypt the
// cipher key with the user's key and then re-encrypt it with the organization's key.
- private async encryptSharedCipher(model: CipherView, userId: UserId): Promise {
+ private async encryptSharedCipher(model: CipherView, userId: UserId): Promise {
const keyForCipherKeyDecryption = await this.keyService.getUserKeyWithLegacySupport(userId);
return await this.encrypt(model, userId, null, keyForCipherKeyDecryption);
}
@@ -1584,10 +1574,7 @@ export class CipherService implements CipherServiceAbstraction {
fd.append(
"data",
Buffer.from(encData.buffer) as any,
- {
- filepath: encFileName.encryptedString,
- contentType: "application/octet-stream",
- } as any,
+ { filepath: encFileName.encryptedString, contentType: "application/octet-stream" } as any,
);
} else {
throw e;
@@ -1649,11 +1636,7 @@ export class CipherService implements CipherServiceAbstraction {
await this.encryptObjProperty(
model.login,
cipher.login,
- {
- username: null,
- password: null,
- totp: null,
- },
+ { username: null, password: null, totp: null },
key,
);
@@ -1663,14 +1646,7 @@ export class CipherService implements CipherServiceAbstraction {
for (let i = 0; i < model.login.uris.length; i++) {
const loginUri = new LoginUri();
loginUri.match = model.login.uris[i].match;
- await this.encryptObjProperty(
- model.login.uris[i],
- loginUri,
- {
- uri: null,
- },
- key,
- );
+ await this.encryptObjProperty(model.login.uris[i], loginUri, { uri: null }, key);
const uriHash = await this.encryptService.hash(model.login.uris[i].uri, "sha256");
loginUri.uriChecksum = await this.encryptService.encryptString(uriHash, key);
cipher.login.uris.push(loginUri);
@@ -1766,11 +1742,7 @@ export class CipherService implements CipherServiceAbstraction {
await this.encryptObjProperty(
model.sshKey,
cipher.sshKey,
- {
- privateKey: null,
- publicKey: null,
- keyFingerprint: null,
- },
+ { privateKey: null, publicKey: null, keyFingerprint: null },
key,
);
return;
@@ -1855,15 +1827,7 @@ export class CipherService implements CipherServiceAbstraction {
}
await Promise.all([
- this.encryptObjProperty(
- model,
- cipher,
- {
- name: null,
- notes: null,
- },
- key,
- ),
+ this.encryptObjProperty(model, cipher, { name: null, notes: null }, key),
this.encryptCipherData(cipher, model, key),
this.encryptFields(model.fields, key).then((fields) => {
cipher.fields = fields;
diff --git a/libs/vault/src/cipher-form/services/default-cipher-form.service.ts b/libs/vault/src/cipher-form/services/default-cipher-form.service.ts
index 68eac4f0da2..99f853d4c86 100644
--- a/libs/vault/src/cipher-form/services/default-cipher-form.service.ts
+++ b/libs/vault/src/cipher-form/services/default-cipher-form.service.ts
@@ -29,19 +29,20 @@ export class DefaultCipherFormService implements CipherFormService {
async saveCipher(cipher: CipherView, config: CipherFormConfig): Promise {
// Passing the original cipher is important here as it is responsible for appending to password history
const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
- const encryptedCipher = await this.cipherService.encrypt(
+ const encrypted = await this.cipherService.encrypt(
cipher,
activeUserId,
null,
null,
config.originalCipher ?? null,
);
+ const encryptedCipher = encrypted.cipher;
let savedCipher: Cipher;
// Creating a new cipher
if (cipher.id == null) {
- savedCipher = await this.cipherService.createWithServer(encryptedCipher, config.admin);
+ savedCipher = await this.cipherService.createWithServer(encrypted, config.admin);
return await this.cipherService.decrypt(savedCipher, activeUserId);
}
@@ -64,13 +65,13 @@ export class DefaultCipherFormService implements CipherFormService {
);
// If the collectionIds are the same, update the cipher normally
} else if (isSetEqual(originalCollectionIds, newCollectionIds)) {
- savedCipher = await this.cipherService.updateWithServer(encryptedCipher, config.admin);
+ savedCipher = await this.cipherService.updateWithServer(encrypted, config.admin);
} else {
// Updating a cipher with collection changes is not supported with a single request currently
// First update the cipher with the original collectionIds
encryptedCipher.collectionIds = config.originalCipher.collectionIds;
await this.cipherService.updateWithServer(
- encryptedCipher,
+ encrypted,
config.admin || originalCollectionIds.size === 0,
);
diff --git a/libs/vault/src/components/assign-collections.component.ts b/libs/vault/src/components/assign-collections.component.ts
index faa2dae072a..4a0bd1fc670 100644
--- a/libs/vault/src/components/assign-collections.component.ts
+++ b/libs/vault/src/components/assign-collections.component.ts
@@ -506,7 +506,7 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI
private async updateAssignedCollections(cipherView: CipherView, userId: UserId) {
const { collections } = this.formGroup.getRawValue();
cipherView.collectionIds = collections.map((i) => i.id as CollectionId);
- const cipher = await this.cipherService.encrypt(cipherView, userId);
+ const { cipher } = await this.cipherService.encrypt(cipherView, userId);
if (this.params.isSingleCipherAdmin) {
await this.cipherService.saveCollectionsWithServerAdmin(cipher);
} else {
From eba22cf5f8b7f9bdcbc9e37f820165d6f04819b1 Mon Sep 17 00:00:00 2001
From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>
Date: Fri, 30 May 2025 13:45:31 -0500
Subject: [PATCH 26/33] [PM-21797] Require userID for keyService's
getUserKeyFromStorage (#14855)
* require userID for keyService's getUserKeyFromStorage
---
.../src/abstractions/key.service.ts | 3 +-
libs/key-management/src/key.service.spec.ts | 81 +++++++++++++++++++
libs/key-management/src/key.service.ts | 5 +-
3 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts
index 51a99421967..7a9c076a8bb 100644
--- a/libs/key-management/src/abstractions/key.service.ts
+++ b/libs/key-management/src/abstractions/key.service.ts
@@ -128,10 +128,11 @@ export abstract class KeyService {
* @param keySuffix The desired version of the user's key to retrieve
* @param userId The desired user
* @returns The user key
+ * @throws Error when userId is null or undefined.
*/
abstract getUserKeyFromStorage(
keySuffix: KeySuffixOptions,
- userId?: string,
+ userId: string,
): Promise;
/**
diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts
index 400d7279a30..cd5458e9a1f 100644
--- a/libs/key-management/src/key.service.spec.ts
+++ b/libs/key-management/src/key.service.spec.ts
@@ -14,6 +14,7 @@ import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/ke
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
+import { KeySuffixOptions } from "@bitwarden/common/platform/enums";
import { Encrypted } from "@bitwarden/common/platform/interfaces/encrypted";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { EncString, EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string";
@@ -896,4 +897,84 @@ describe("keyService", () => {
});
});
});
+
+ describe("getUserKeyFromStorage", () => {
+ let mockUserKey: UserKey;
+ let validateUserKeySpy: jest.SpyInstance;
+
+ beforeEach(() => {
+ mockUserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey;
+ validateUserKeySpy = jest.spyOn(keyService, "validateUserKey");
+ });
+
+ afterEach(() => {
+ validateUserKeySpy.mockRestore();
+ });
+
+ describe("input validation", () => {
+ const invalidUserIdTestCases = [
+ { keySuffix: KeySuffixOptions.Auto, userId: null as unknown as UserId },
+ { keySuffix: KeySuffixOptions.Auto, userId: undefined as unknown as UserId },
+ { keySuffix: KeySuffixOptions.Pin, userId: null as unknown as UserId },
+ { keySuffix: KeySuffixOptions.Pin, userId: undefined as unknown as UserId },
+ ];
+
+ test.each(invalidUserIdTestCases)(
+ "throws when keySuffix is $keySuffix and userId is $userId",
+ async ({ keySuffix, userId }) => {
+ await expect(keyService.getUserKeyFromStorage(keySuffix, userId)).rejects.toThrow(
+ "UserId is required",
+ );
+ },
+ );
+ });
+
+ describe("with Pin keySuffix", () => {
+ it("returns null and doesn't validate the key", async () => {
+ const result = await keyService.getUserKeyFromStorage(KeySuffixOptions.Pin, mockUserId);
+
+ expect(result).toBeNull();
+ expect(validateUserKeySpy).not.toHaveBeenCalled();
+ });
+ });
+
+ describe("with Auto keySuffix", () => {
+ it("returns validated key from storage when key exists and is valid", async () => {
+ stateService.getUserKeyAutoUnlock.mockResolvedValue(mockUserKey.keyB64);
+ validateUserKeySpy.mockResolvedValue(true);
+
+ const result = await keyService.getUserKeyFromStorage(KeySuffixOptions.Auto, mockUserId);
+
+ expect(result).toEqual(mockUserKey);
+ expect(validateUserKeySpy).toHaveBeenCalledWith(mockUserKey, mockUserId);
+ expect(stateService.getUserKeyAutoUnlock).toHaveBeenCalledWith({
+ userId: mockUserId,
+ });
+ });
+
+ it("returns null when no key is found in storage", async () => {
+ stateService.getUserKeyAutoUnlock.mockResolvedValue(null as unknown as string);
+
+ const result = await keyService.getUserKeyFromStorage(KeySuffixOptions.Auto, mockUserId);
+
+ expect(result).toBeNull();
+ expect(validateUserKeySpy).not.toHaveBeenCalled();
+ });
+
+ it("clears stored keys when userKey validation fails", async () => {
+ stateService.getUserKeyAutoUnlock.mockResolvedValue(mockUserKey.keyB64);
+ validateUserKeySpy.mockResolvedValue(false);
+
+ const result = await keyService.getUserKeyFromStorage(KeySuffixOptions.Auto, mockUserId);
+
+ expect(result).toEqual(mockUserKey);
+ expect(validateUserKeySpy).toHaveBeenCalledWith(mockUserKey, mockUserId);
+ expect(logService.warning).toHaveBeenCalledWith("Invalid key, throwing away stored keys");
+ expect(pinService.clearPinKeyEncryptedUserKeyEphemeral).toHaveBeenCalledWith(mockUserId);
+ expect(stateService.setUserKeyAutoUnlock).toHaveBeenCalledWith(null, {
+ userId: mockUserId,
+ });
+ });
+ });
+ });
});
diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts
index fe288adeb88..4a48d00f568 100644
--- a/libs/key-management/src/key.service.ts
+++ b/libs/key-management/src/key.service.ts
@@ -178,11 +178,10 @@ export class DefaultKeyService implements KeyServiceAbstraction {
async getUserKeyFromStorage(
keySuffix: KeySuffixOptions,
- userId?: UserId,
+ userId: UserId,
): Promise {
- userId ??= await firstValueFrom(this.stateProvider.activeUserId$);
if (userId == null) {
- throw new Error("No active user id found.");
+ throw new Error("UserId is required");
}
const userKey = await this.getKeyFromStorage(keySuffix, userId);
From 721657a5c30802edef34a20309c01ae2952b1da1 Mon Sep 17 00:00:00 2001
From: Todd Martin <106564991+trmartin4@users.noreply.github.com>
Date: Fri, 30 May 2025 15:31:44 -0400
Subject: [PATCH 27/33] Update syntax for Github. (#14845)
---
.../angular/src/platform/view-cache/README.md | 20 +++++++------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/libs/angular/src/platform/view-cache/README.md b/libs/angular/src/platform/view-cache/README.md
index c1f80da5800..d98222c4bde 100644
--- a/libs/angular/src/platform/view-cache/README.md
+++ b/libs/angular/src/platform/view-cache/README.md
@@ -43,12 +43,9 @@ on any component.
The persistence layer ensures that the popup will open at the same route as was active when it
closed, provided that none of the lifetime expiration events have occurred.
-:::tip Excluding a route
-
-If a particular route should be excluded from the history and not persisted, add
-`doNotSaveUrl: true` to the `data` property on the route.
-
-:::
+> [!TIP]
+> If a particular route should be excluded from the history and not persisted, add
+> `doNotSaveUrl: true` to the `data` property on the route.
### View data persistence
@@ -85,13 +82,10 @@ const mySignal = this.viewCacheService.signal({
mySignal.set("value")
```
-:::note Equality comparison
-
-By default, signals use `Object.is` to determine equality, and `set()` will only trigger updates if
-the updated value is not equal to the current signal state. See documentation
-[here](https://angular.dev/guide/signals#signal-equality-functions).
-
-:::
+> [!NOTE]
+> By default, signals use `Object.is` to determine equality, and `set()` will only trigger updates if
+> the updated value is not equal to the current signal state. See documentation
+> [here](https://angular.dev/guide/signals#signal-equality-functions).
Putting this together, the most common implementation pattern would be:
From f55f315ca15df09772e957e0e8b089a2d45b04f7 Mon Sep 17 00:00:00 2001
From: Kevinw778
Date: Sat, 31 May 2025 05:18:28 -0400
Subject: [PATCH 28/33] [PM-21868] Send limit reached icon + message now show
(#14860)
* Send limit reached icon + message now show
* Fix en/messages.json
---------
Co-authored-by: Daniel James Smith
---
apps/browser/src/_locales/en/messages.json | 4 ++++
.../send-list-items-container.component.html | 10 ++++++++++
2 files changed, 14 insertions(+)
diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json
index feb5a7706f3..29223942fd6 100644
--- a/apps/browser/src/_locales/en/messages.json
+++ b/apps/browser/src/_locales/en/messages.json
@@ -2677,6 +2677,10 @@
"message": "All Sends",
"description": "'Send' is a noun and the name of a feature called 'Bitwarden Send'. It should not be translated."
},
+ "maxAccessCountReached": {
+ "message": "Max access count reached",
+ "description": "This text will be displayed after a Send has been accessed the maximum amount of times."
+ },
"hideTextByDefault": {
"message": "Hide text by default"
},
diff --git a/libs/tools/send/send-ui/src/send-list-items-container/send-list-items-container.component.html b/libs/tools/send/send-ui/src/send-list-items-container/send-list-items-container.component.html
index 9b0f0fca26f..94ebfc3e5e6 100644
--- a/libs/tools/send/send-ui/src/send-list-items-container/send-list-items-container.component.html
+++ b/libs/tools/send/send-ui/src/send-list-items-container/send-list-items-container.component.html
@@ -26,6 +26,16 @@
>