Skip to content

Commit

Permalink
[PM-5963] Fix tde offboarding vault corruption (#9480)
Browse files Browse the repository at this point in the history
* Fix tde offboarding

* Add tde offboarding password request

* Add event for tde offboarding

* Update libs/auth/src/common/models/domain/user-decryption-options.ts

Co-authored-by: Jake Fink <[email protected]>

* Update libs/common/src/services/api.service.ts

Co-authored-by: Jake Fink <[email protected]>

* Make tde offboarding take priority

* Update tde offboarding message

* Fix unit tests

* Fix unit tests

* Fix typo

* Fix unit tests

---------

Co-authored-by: Jake Fink <[email protected]>
  • Loading branch information
quexten and jlf0dev authored Aug 1, 2024
1 parent d26ea1b commit c6229ab
Show file tree
Hide file tree
Showing 19 changed files with 94 additions and 17 deletions.
3 changes: 3 additions & 0 deletions apps/browser/src/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2327,6 +2327,9 @@
"updateWeakMasterPasswordWarning": {
"message": "Your master password does not meet one or more of your organization policies. In order to access the vault, you must update your master password now. Proceeding will log you out of your current session, requiring you to log back in. Active sessions on other devices may continue to remain active for up to one hour."
},
"tdeDisabledMasterPasswordRequired": {
"message": "Your organization has disabled trusted device encryption. Please set a master password to access your vault."
},
"resetPasswordPolicyAutoEnroll": {
"message": "Automatic enrollment"
},
Expand Down
3 changes: 3 additions & 0 deletions apps/desktop/src/locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2015,6 +2015,9 @@
"updateWeakMasterPasswordWarning": {
"message": "Your master password does not meet one or more of your organization policies. In order to access the vault, you must update your master password now. Proceeding will log you out of your current session, requiring you to log back in. Active sessions on other devices may continue to remain active for up to one hour."
},
"tdeDisabledMasterPasswordRequired": {
"message": "Your organization has disabled trusted device encryption. Please set a master password to access your vault."
},
"tryAgain": {
"message": "Try again"
},
Expand Down
3 changes: 3 additions & 0 deletions apps/web/src/app/core/event.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ export class EventService {
case EventType.User_RequestedDeviceApproval:
msg = humanReadableMsg = this.i18nService.t("requestedDeviceApproval");
break;
case EventType.User_TdeOffboardingPasswordSet:
msg = humanReadableMsg = this.i18nService.t("tdeOffboardingPasswordSet");
break;
// Cipher
case EventType.Cipher_Created:
msg = this.i18nService.t("createdItemId", this.formatCipherId(ev, options));
Expand Down
6 changes: 6 additions & 0 deletions apps/web/src/locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -5322,6 +5322,9 @@
"updateWeakMasterPasswordWarning": {
"message": "Your master password does not meet one or more of your organization policies. In order to access the vault, you must update your master password now. Proceeding will log you out of your current session, requiring you to log back in. Active sessions on other devices may continue to remain active for up to one hour."
},
"tdeDisabledMasterPasswordRequired": {
"message": "Your organization has updated your decryption options. Please set a master password to access your vault."
},
"maximumVaultTimeout": {
"message": "Vault timeout"
},
Expand Down Expand Up @@ -7674,6 +7677,9 @@
"requestedDeviceApproval": {
"message": "Requested device approval."
},
"tdeOffboardingPasswordSet": {
"message": "User set a master password during TDE offboarding."
},
"startYour7DayFreeTrialOfBitwardenFor": {
"message": "Start your 7-Day free trial of Bitwarden for $ORG$",
"placeholders": {
Expand Down
8 changes: 4 additions & 4 deletions libs/angular/src/auth/components/sso.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ describe("SsoComponent", () => {
}),
withMasterPasswordAndTrustedDevice: new UserDecryptionOptions({
hasMasterPassword: true,
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false),
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false, false),
keyConnectorOption: undefined,
}),
withMasterPasswordAndTrustedDeviceWithManageResetPassword: new UserDecryptionOptions({
hasMasterPassword: true,
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true),
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true, false),
keyConnectorOption: undefined,
}),
withMasterPasswordAndKeyConnector: new UserDecryptionOptions({
Expand All @@ -169,12 +169,12 @@ describe("SsoComponent", () => {
}),
noMasterPasswordWithTrustedDevice: new UserDecryptionOptions({
hasMasterPassword: false,
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false),
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false, false),
keyConnectorOption: undefined,
}),
noMasterPasswordWithTrustedDeviceWithManageResetPassword: new UserDecryptionOptions({
hasMasterPassword: false,
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true),
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true, false),
keyConnectorOption: undefined,
}),
noMasterPasswordWithKeyConnector: new UserDecryptionOptions({
Expand Down
12 changes: 11 additions & 1 deletion libs/angular/src/auth/components/sso.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,18 @@ export class SsoComponent {
orgIdentifier: string,
userDecryptionOpts: UserDecryptionOptions,
): Promise<void> {
// If user doesn't have a MP, but has reset password permission, they must set a MP
// Tde offboarding takes precedence
if (
!userDecryptionOpts.hasMasterPassword &&
userDecryptionOpts.trustedDeviceOption.isTdeOffboarding
) {
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
await this.masterPasswordService.setForceSetPasswordReason(
ForceSetPasswordReason.TdeOffboarding,
userId,
);
} else if (
// If user doesn't have a MP, but has reset password permission, they must set a MP
!userDecryptionOpts.hasMasterPassword &&
userDecryptionOpts.trustedDeviceOption.hasManageResetPasswordPermission
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,12 @@ describe("TwoFactorComponent", () => {
}),
withMasterPasswordAndTrustedDevice: new UserDecryptionOptions({
hasMasterPassword: true,
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false),
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false, false),
keyConnectorOption: undefined,
}),
withMasterPasswordAndTrustedDeviceWithManageResetPassword: new UserDecryptionOptions({
hasMasterPassword: true,
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true),
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true, false),
keyConnectorOption: undefined,
}),
withMasterPasswordAndKeyConnector: new UserDecryptionOptions({
Expand All @@ -142,12 +142,12 @@ describe("TwoFactorComponent", () => {
}),
noMasterPasswordWithTrustedDevice: new UserDecryptionOptions({
hasMasterPassword: false,
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false),
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false, false),
keyConnectorOption: undefined,
}),
noMasterPasswordWithTrustedDeviceWithManageResetPassword: new UserDecryptionOptions({
hasMasterPassword: false,
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true),
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true, false),
keyConnectorOption: undefined,
}),
noMasterPasswordWithKeyConnector: new UserDecryptionOptions({
Expand Down
8 changes: 4 additions & 4 deletions libs/angular/src/auth/components/two-factor.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,12 @@ describe("TwoFactorComponent", () => {
}),
withMasterPasswordAndTrustedDevice: new UserDecryptionOptions({
hasMasterPassword: true,
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false),
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false, false),
keyConnectorOption: undefined,
}),
withMasterPasswordAndTrustedDeviceWithManageResetPassword: new UserDecryptionOptions({
hasMasterPassword: true,
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true),
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true, false),
keyConnectorOption: undefined,
}),
withMasterPasswordAndKeyConnector: new UserDecryptionOptions({
Expand All @@ -135,12 +135,12 @@ describe("TwoFactorComponent", () => {
}),
noMasterPasswordWithTrustedDevice: new UserDecryptionOptions({
hasMasterPassword: false,
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false),
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false, false),
keyConnectorOption: undefined,
}),
noMasterPasswordWithTrustedDeviceWithManageResetPassword: new UserDecryptionOptions({
hasMasterPassword: false,
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true),
trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true, false),
keyConnectorOption: undefined,
}),
noMasterPasswordWithKeyConnector: new UserDecryptionOptions({
Expand Down
26 changes: 23 additions & 3 deletions libs/angular/src/auth/components/update-temp-password.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { UserVerificationService } from "@bitwarden/common/auth/abstractions/use
import { VerificationType } from "@bitwarden/common/auth/enums/verification-type";
import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason";
import { PasswordRequest } from "@bitwarden/common/auth/models/request/password.request";
import { UpdateTdeOffboardingPasswordRequest } from "@bitwarden/common/auth/models/request/update-tde-offboarding-password.request";
import { UpdateTempPasswordRequest } from "@bitwarden/common/auth/models/request/update-temp-password.request";
import { MasterPasswordVerification } from "@bitwarden/common/auth/types/verification";
import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
Expand Down Expand Up @@ -97,9 +98,13 @@ export class UpdateTempPasswordComponent extends BaseChangePasswordComponent {
}

get masterPasswordWarningText(): string {
return this.reason == ForceSetPasswordReason.WeakMasterPassword
? this.i18nService.t("updateWeakMasterPasswordWarning")
: this.i18nService.t("updateMasterPasswordWarning");
if (this.reason == ForceSetPasswordReason.WeakMasterPassword) {
return this.i18nService.t("weakMasterPasswordWarning");
} else if (this.reason == ForceSetPasswordReason.TdeOffboarding) {
return this.i18nService.t("tdeDisabledMasterPasswordRequired");
} else {
return this.i18nService.t("masterPasswordWarning");
}
}

togglePassword(confirmField: boolean) {
Expand Down Expand Up @@ -165,6 +170,9 @@ export class UpdateTempPasswordComponent extends BaseChangePasswordComponent {
case ForceSetPasswordReason.WeakMasterPassword:
this.formPromise = this.updatePassword(masterPasswordHash, userKey);
break;
case ForceSetPasswordReason.TdeOffboarding:
this.formPromise = this.updateTdeOffboardingPassword(masterPasswordHash, userKey);
break;
}

await this.formPromise;
Expand Down Expand Up @@ -211,4 +219,16 @@ export class UpdateTempPasswordComponent extends BaseChangePasswordComponent {

return this.apiService.postPassword(request);
}

private async updateTdeOffboardingPassword(
masterPasswordHash: string,
userKey: [UserKey, EncString],
) {
const request = new UpdateTdeOffboardingPasswordRequest();
request.key = userKey[1].encryptedString;
request.newMasterPasswordHash = masterPasswordHash;
request.masterPasswordHint = this.hint;

return this.apiService.putUpdateTdeOffboardingPassword(request);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ describe("SsoLoginStrategy", () => {
HasAdminApproval: true,
HasLoginApprovingDevice: true,
HasManageResetPasswordPermission: false,
IsTdeOffboarding: false,
EncryptedPrivateKey: mockEncDevicePrivateKey,
EncryptedUserKey: mockEncUserKey,
},
Expand Down Expand Up @@ -343,6 +344,7 @@ describe("SsoLoginStrategy", () => {
HasAdminApproval: true,
HasLoginApprovingDevice: false,
HasManageResetPasswordPermission: false,
IsTdeOffboarding: false,
EncryptedPrivateKey: mockEncDevicePrivateKey,
EncryptedUserKey: mockEncUserKey,
},
Expand Down
3 changes: 3 additions & 0 deletions libs/auth/src/common/models/domain/user-decryption-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ export class TrustedDeviceUserDecryptionOption {
hasLoginApprovingDevice: boolean;
/** True if the user has manage reset password permission, as these users must be forced to have a master password. */
hasManageResetPasswordPermission: boolean;
/** True if tde is disabled but user has not set a master password yet. */
isTdeOffboarding: boolean;

/**
* Initializes a new instance of the TrustedDeviceUserDecryptionOption from a response object.
Expand All @@ -70,6 +72,7 @@ export class TrustedDeviceUserDecryptionOption {
options.hasAdminApproval = response?.hasAdminApproval ?? false;
options.hasLoginApprovingDevice = response?.hasLoginApprovingDevice ?? false;
options.hasManageResetPasswordPermission = response?.hasManageResetPasswordPermission ?? false;
options.isTdeOffboarding = response?.isTdeOffboarding ?? false;
return options;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ export class FakeTrustedDeviceUserDecryptionOption extends TrustedDeviceUserDecr
hasAdminApproval: boolean,
hasLoginApprovingDevice: boolean,
hasManageResetPasswordPermission: boolean,
isTdeOffboarding: boolean,
) {
super();
this.hasAdminApproval = hasAdminApproval;
this.hasLoginApprovingDevice = hasLoginApprovingDevice;
this.hasManageResetPasswordPermission = hasManageResetPasswordPermission;
this.isTdeOffboarding = isTdeOffboarding;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
} from "@bitwarden/common/spec";
import { UserId } from "@bitwarden/common/types/guid";

import { UserDecryptionOptions } from "../../models/domain/user-decryption-options";

import {
USER_DECRYPTION_OPTIONS,
UserDecryptionOptionsService,
Expand All @@ -27,12 +29,13 @@ describe("UserDecryptionOptionsService", () => {
sut = new UserDecryptionOptionsService(fakeStateProvider);
});

const userDecryptionOptions = {
const userDecryptionOptions: UserDecryptionOptions = {
hasMasterPassword: true,
trustedDeviceOption: {
hasAdminApproval: false,
hasLoginApprovingDevice: false,
hasManageResetPasswordPermission: true,
isTdeOffboarding: false,
},
keyConnectorOption: {
keyConnectorUrl: "https://keyconnector.bitwarden.com",
Expand Down
2 changes: 2 additions & 0 deletions libs/common/src/abstractions/api.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { TwoFactorEmailRequest } from "../auth/models/request/two-factor-email.r
import { TwoFactorProviderRequest } from "../auth/models/request/two-factor-provider.request";
import { TwoFactorRecoveryRequest } from "../auth/models/request/two-factor-recovery.request";
import { UpdateProfileRequest } from "../auth/models/request/update-profile.request";
import { UpdateTdeOffboardingPasswordRequest } from "../auth/models/request/update-tde-offboarding-password.request";
import { UpdateTempPasswordRequest } from "../auth/models/request/update-temp-password.request";
import { UpdateTwoFactorAuthenticatorRequest } from "../auth/models/request/update-two-factor-authenticator.request";
import { UpdateTwoFactorDuoRequest } from "../auth/models/request/update-two-factor-duo.request";
Expand Down Expand Up @@ -181,6 +182,7 @@ export abstract class ApiService {
postUserApiKey: (id: string, request: SecretVerificationRequest) => Promise<ApiKeyResponse>;
postUserRotateApiKey: (id: string, request: SecretVerificationRequest) => Promise<ApiKeyResponse>;
putUpdateTempPassword: (request: UpdateTempPasswordRequest) => Promise<any>;
putUpdateTdeOffboardingPassword: (request: UpdateTdeOffboardingPasswordRequest) => Promise<any>;
postConvertToKeyConnector: () => Promise<void>;
//passwordless
postAuthRequest: (request: CreateAuthRequest) => Promise<AuthRequestResponse>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,9 @@ export enum ForceSetPasswordReason {
* Set post login & decryption client side and by server in sync (to catch logged in users).
*/
TdeUserWithoutPasswordHasPasswordResetPermission,

/**
* Occurs when TDE is disabled and master password has to be set.
*/
TdeOffboarding,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { OrganizationUserResetPasswordRequest } from "../../../admin-console/abstractions/organization-user/requests";

export class UpdateTdeOffboardingPasswordRequest extends OrganizationUserResetPasswordRequest {
masterPasswordHint: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export interface ITrustedDeviceUserDecryptionOptionServerResponse {
HasAdminApproval: boolean;
HasLoginApprovingDevice: boolean;
HasManageResetPasswordPermission: boolean;
IsTdeOffboarding: boolean;
EncryptedPrivateKey?: string;
EncryptedUserKey?: string;
}
Expand All @@ -13,6 +14,7 @@ export class TrustedDeviceUserDecryptionOptionResponse extends BaseResponse {
hasAdminApproval: boolean;
hasLoginApprovingDevice: boolean;
hasManageResetPasswordPermission: boolean;
isTdeOffboarding: boolean;
encryptedPrivateKey: EncString;
encryptedUserKey: EncString;

Expand All @@ -25,6 +27,8 @@ export class TrustedDeviceUserDecryptionOptionResponse extends BaseResponse {
"HasManageResetPasswordPermission",
);

this.isTdeOffboarding = this.getResponseProperty("IsTdeOffboarding");

if (response.EncryptedPrivateKey) {
this.encryptedPrivateKey = new EncString(this.getResponseProperty("EncryptedPrivateKey"));
}
Expand Down
1 change: 1 addition & 0 deletions libs/common/src/enums/event-type.enum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export enum EventType {
User_UpdatedTempPassword = 1008,
User_MigratedKeyToKeyConnector = 1009,
User_RequestedDeviceApproval = 1010,
User_TdeOffboardingPasswordSet = 1011,

Cipher_Created = 1100,
Cipher_Updated = 1101,
Expand Down
5 changes: 5 additions & 0 deletions libs/common/src/services/api.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import { TwoFactorEmailRequest } from "../auth/models/request/two-factor-email.r
import { TwoFactorProviderRequest } from "../auth/models/request/two-factor-provider.request";
import { TwoFactorRecoveryRequest } from "../auth/models/request/two-factor-recovery.request";
import { UpdateProfileRequest } from "../auth/models/request/update-profile.request";
import { UpdateTdeOffboardingPasswordRequest } from "../auth/models/request/update-tde-offboarding-password.request";
import { UpdateTempPasswordRequest } from "../auth/models/request/update-temp-password.request";
import { UpdateTwoFactorAuthenticatorRequest } from "../auth/models/request/update-two-factor-authenticator.request";
import { UpdateTwoFactorDuoRequest } from "../auth/models/request/update-two-factor-duo.request";
Expand Down Expand Up @@ -461,6 +462,10 @@ export class ApiService implements ApiServiceAbstraction {
return this.send("PUT", "/accounts/update-temp-password", request, true, false);
}

putUpdateTdeOffboardingPassword(request: UpdateTdeOffboardingPasswordRequest): Promise<void> {
return this.send("PUT", "/accounts/update-tde-offboarding-password", request, true, false);
}

postConvertToKeyConnector(): Promise<void> {
return this.send("POST", "/accounts/convert-to-key-connector", null, true, false);
}
Expand Down

0 comments on commit c6229ab

Please sign in to comment.