From 161c1c63ffee1d91df3cf2acf53b948e894cb7ba Mon Sep 17 00:00:00 2001 From: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Date: Fri, 3 Nov 2023 11:33:10 -0400 Subject: [PATCH] Auth/PM-3275 - Changes to support TDE User without MP being able to Set a Password (#6281) * PM-3275 - Policy.service - Refactor existing mapPoliciesFromToken internal logic to provide public mapPolicyFromResponse method * PM-3275 - Add new PolicyApiService.getMasterPasswordPolicyOptsForOrgUser method for use in the set password comp * PM-3275 - Update set-password.comp to use new policyApiService.getMasterPasswordPoliciesForInvitedUsers method * PM-3275 - (1) Remove post TDE AuthN set password routing logic from SSO/2FA comps as we cannot set an initial user password until after decryption in order to avoid losing the ability to decrypt existing vault items (a new user key would be created if one didn't exist in memory) (2) Add set password routing logic post TDE decryption in LoginWithDevice/Lock components (3) Add new ForceResetPasswordReason to capture this case so that we can guard against users manually navigating away from the set password screen * PM-3275 - SyncSvc - Add logic for setting forcePasswordReset reason if TDE user w/out MP went from not having MP reset permission to having it. * PM-3275 - Rename ForceResetPasswordReason enum to ForceSetPasswordReason + update all references. * PM-3275 - Removing client deprecated calls to getPoliciesByInvitedUser and helper call getMasterPasswordPoliciesForInvitedUsers * PM-3275 - PolicyAPI service - remove no longer necessary getPoliciesByInvitedUser method * PM-3275 - LockComp - TODO cleanup * PM-3275 - SSO & 2FA comp - cleanup of incorrect routing path * PM-3275 - (1) State service refactor - change getForcePasswordResetReason / setForcePasswordResetReason to be getForceSetPasswordReason / setForceSetPasswordReason (2) Sync Service - encapsulate setForceSetPasswordReasonIfNeeded logic into own method * PM-3275 - SetPassword Comp - Rename "identifier" to be "orgSsoIdentifier" for clarity * PM-3275 - SetPasswordComp - Moving routing from SSO / 2FA comps to Lock / LoginWithDevice comps results in a loss of the the OrgSsoId. However, as part of the TDE work, we added the OrgSsoId to state so use that as a fallback so we can accurately evaluate if the user needs to be auto enrolled in admin account recovery. * PM-3275 - SetPasswordComp - add a bit more context to why/when we are reading the user org sso id out of state * PM-3275 - SetPassword Comp - (1) Add forceSetPasswordReason and ForceSetPasswordReason enum as public props on the class so we can change copy text based on which is set + set forceSetPasswordReason on ngOnInit (2) Refactor ngOnInit to use a single RxJs observable chain for primary logic as the auto enroll check was occurring before the async getUserSsoOrganizationIdentifier could finish. * PM-3275 - Desktop - App comp - missed replacing getForcePasswordResetReason with getForceSetPasswordReason * PM-3275 - TDE Decryption Option Comps - must set ForceSetPasswordReason so that we can properly enforce keeping the user on the component + display the correct copy explaining the scenario to the user. * PM-3275 - All Clients - SetPasswordComp html - Update page description per product + remove no longer used ssoCompleteRegistration translation. * PM-3275 - SetPasswordComp - hopefully the final puzzle piece - must clear ForceSetPasswordReason in order to let user navigate back to vault. * PM-3275 - SyncService - Remove check for previous value of account decryption options hasManageResetPasswordPermission as when a user logged in on a trusted device after having their permissions updated, the initial setting would be true and it would cause the flag to NOT be set when it should have. * PM-3275 - TDE User Context - (1) Remove explicit navigation to set password screen from post decryption success scenarios on lock & login w/ device comps (2) Move TdeUserWithoutPasswordHasPasswordResetPermission flag setting to SSO / 2FA components to support both trusted and untrusted device scenarios (both of which are now caught by the auth guard). * PM-3275 - (1) SetPassword comp - adjust set password logic for TDE users to avoid creating a new user asymmetric key pair and setting a new private key in memory. (2) Adjust SetPasswordRequest to allow null keys * PM-3275 - Remove unused route from login with device comp * PM-3275 - Sso & 2FA comp tests - Update tests to reflect new routing logic when TDE user needs to set a password * PM-3275 - Lock comp - per PR feedback, remove unused setPasswordRoute property. * PM-3275 - SetPasswordComp - Per PR feedback, use explicit null check * PM-3275 - Per PR Feedback, rename missed forcePasswordResetReason to be forceSetPasswordReason on account model * PM-3275 - Auth guard - rename forcePasswordResetReason to forceSetPasswordReason * PM-3275 - SSO / 2FA comps - Per PR feedback, refactor Admin Force Password reset handling to be in one place above the TDE user flows and standard user flows as it applies to both. * PM-3275 - Per PR feedback, clarify 2FA routing comment * PM-3275 - Per PR feedback, update set-password comp ngOnInit switchMaps to just return promises as switchMap converts promises to observables internally. * PM-3275 - Per PR feedback, refactor set password ngOnInit observable chain to avoid using async subscribe and instead simply sequence the calls via switchMap and tap for side effects. * PM-3275 - Per PR feedback, move tap after filter so we can remove if check * PM-3275 - Per PR feedback, update policy service mapping methods to use shorthand null checking. * PM-3275 - SetPassword comp - (1) Move force set password reason logic into onSetPasswordSuccess(...) (2) On onSetPasswordSuccess, must set hasMasterPassword to true for user verification scenarios. * PM-3275 - Per PR feedback, remove new hasManageResetPasswordPermission flag from profile response and instead simply read the information off the existing profile.organizations data as the information I needed was already present. * PM-4633 - PolicyService - mapPolicyFromResponse(...) - remove incorrect null check for data. Policies with internal null data property should still be evaluated and turned into Policy objects or the policy array ends up having null values in it and it causes errors down the line on login after acct creation. --- apps/browser/src/_locales/en/messages.json | 9 +- .../auth/popup/set-password.component.html | 16 ++- apps/cli/src/auth/commands/login.command.ts | 8 +- apps/desktop/src/app/app.component.ts | 6 +- .../src/auth/set-password.component.html | 15 ++- apps/desktop/src/locales/en/messages.json | 9 +- .../src/app/auth/set-password.component.html | 15 ++- apps/web/src/locales/en/messages.json | 11 +- .../src/auth/components/lock.component.ts | 6 +- .../login-via-auth-request.component.ts | 5 +- .../src/auth/components/login.component.ts | 4 +- .../auth/components/set-password.component.ts | 114 +++++++++++++----- .../src/auth/components/sso.component.spec.ts | 36 ++++-- .../src/auth/components/sso.component.ts | 29 ++--- .../components/two-factor.component.spec.ts | 33 +++-- .../auth/components/two-factor.component.ts | 49 +++++--- .../update-temp-password.component.ts | 18 +-- libs/angular/src/auth/guards/auth.guard.ts | 16 ++- .../policy/policy-api.service.abstraction.ts | 7 +- .../policy/policy.service.abstraction.ts | 1 + .../services/policy/policy-api.service.ts | 50 +++++--- .../services/policy/policy.service.ts | 10 +- .../login-strategies/login.strategy.spec.ts | 4 +- .../auth/login-strategies/login.strategy.ts | 4 +- .../password-login.strategy.spec.ts | 20 +-- .../password-login.strategy.ts | 16 +-- .../login-strategies/sso-login.strategy.ts | 6 +- .../src/auth/models/domain/auth-result.ts | 4 +- ...reason.ts => force-set-password-reason.ts} | 10 +- .../models/request/set-password.request.ts | 4 +- .../platform/abstractions/state.service.ts | 8 +- .../src/platform/models/domain/account.ts | 4 +- .../src/platform/services/state.service.ts | 12 +- .../src/vault/services/sync/sync.service.ts | 50 ++++++-- 34 files changed, 422 insertions(+), 187 deletions(-) rename libs/common/src/auth/models/domain/{force-reset-password-reason.ts => force-set-password-reason.ts} (65%) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 529ead2d6c..355c1ec3b8 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -1925,8 +1925,13 @@ "selectFolder": { "message": "Select folder..." }, - "ssoCompleteRegistration": { - "message": "In order to complete logging in with SSO, please set a master password to access and protect your vault." + "orgPermissionsUpdatedMustSetPassword": { + "message": "Your organization permissions were updated, requiring you to set a master password.", + "description": "Used as a card title description on the set password page to explain why the user is there" + }, + "orgRequiresYouToSetPassword": { + "message": "Your organization requires you to set a master password.", + "description": "Used as a card title description on the set password page to explain why the user is there" }, "hours": { "message": "Hours" diff --git a/apps/browser/src/auth/popup/set-password.component.html b/apps/browser/src/auth/popup/set-password.component.html index 656664facb..6261608c34 100644 --- a/apps/browser/src/auth/popup/set-password.component.html +++ b/apps/browser/src/auth/popup/set-password.component.html @@ -19,7 +19,21 @@
- {{ "ssoCompleteRegistration" | i18n }} +

+ {{ "orgPermissionsUpdatedMustSetPassword" | i18n }} +

+ + +

{{ "orgRequiresYouToSetPassword" | i18n }}

+ +
- {{ "ssoCompleteRegistration" | i18n }} +

+ {{ "orgPermissionsUpdatedMustSetPassword" | i18n }} +

+ + +

{{ "orgRequiresYouToSetPassword" | i18n }}

+ +
- {{ "ssoCompleteRegistration" | i18n }} +

+ {{ "orgPermissionsUpdatedMustSetPassword" | i18n }} +

+ + +

{{ "orgRequiresYouToSetPassword" | i18n }}

+ + Promise; successRoute = "vault"; + forceSetPasswordReason: ForceSetPasswordReason = ForceSetPasswordReason.None; + ForceSetPasswordReason = ForceSetPasswordReason; + constructor( i18nService: I18nService, cryptoService: CryptoService, @@ -67,30 +74,49 @@ export class SetPasswordComponent extends BaseChangePasswordComponent { } async ngOnInit() { + super.ngOnInit(); + await this.syncService.fullSync(true); this.syncLoading = false; - // eslint-disable-next-line rxjs/no-async-subscribe - this.route.queryParams.pipe(first()).subscribe(async (qParams) => { - if (qParams.identifier != null) { - this.identifier = qParams.identifier; - } - }); + this.forceSetPasswordReason = await this.stateService.getForceSetPasswordReason(); - // Automatic Enrollment Detection - if (this.identifier != null) { - try { - const response = await this.organizationApiService.getAutoEnrollStatus(this.identifier); - this.orgId = response.id; - this.resetPasswordAutoEnroll = response.resetPasswordEnabled; - this.enforcedPolicyOptions = - await this.policyApiService.getMasterPasswordPoliciesForInvitedUsers(this.orgId); - } catch { - this.platformUtilsService.showToast("error", null, this.i18nService.t("errorOccurred")); - } - } - - super.ngOnInit(); + this.route.queryParams + .pipe( + first(), + switchMap((qParams) => { + if (qParams.identifier != null) { + return of(qParams.identifier); + } else { + // Try to get orgSsoId from state as fallback + // Note: this is primarily for the TDE user w/out MP obtains admin MP reset permission scenario. + return this.stateService.getUserSsoOrganizationIdentifier(); + } + }), + filter((orgSsoId) => orgSsoId != null), + tap((orgSsoId: string) => { + this.orgSsoIdentifier = orgSsoId; + }), + switchMap((orgSsoId: string) => this.organizationApiService.getAutoEnrollStatus(orgSsoId)), + tap((orgAutoEnrollStatusResponse: OrganizationAutoEnrollStatusResponse) => { + this.orgId = orgAutoEnrollStatusResponse.id; + this.resetPasswordAutoEnroll = orgAutoEnrollStatusResponse.resetPasswordEnabled; + }), + switchMap((orgAutoEnrollStatusResponse: OrganizationAutoEnrollStatusResponse) => + // Must get org id from response to get master password policy options + this.policyApiService.getMasterPasswordPolicyOptsForOrgUser( + orgAutoEnrollStatusResponse.id + ) + ), + tap((masterPasswordPolicyOptions: MasterPasswordPolicyOptions) => { + this.enforcedPolicyOptions = masterPasswordPolicyOptions; + }) + ) + .subscribe({ + error: () => { + this.platformUtilsService.showToast("error", null, this.i18nService.t("errorOccurred")); + }, + }); } async setupSubmitActions() { @@ -104,13 +130,26 @@ export class SetPasswordComponent extends BaseChangePasswordComponent { masterKey: MasterKey, userKey: [UserKey, EncString] ) { - const newKeyPair = await this.cryptoService.makeKeyPair(userKey[0]); + let keysRequest: KeysRequest | null = null; + let newKeyPair: [string, EncString] | null = null; + + if ( + this.forceSetPasswordReason != + ForceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission + ) { + // Existing JIT provisioned user in a MP encryption org setting first password + // Users in this state will not already have a user asymmetric key pair so must create it for them + // We don't want to re-create the user key pair if the user already has one (TDE user case) + newKeyPair = await this.cryptoService.makeKeyPair(userKey[0]); + keysRequest = new KeysRequest(newKeyPair[0], newKeyPair[1].encryptedString); + } + const request = new SetPasswordRequest( masterPasswordHash, userKey[1].encryptedString, this.hint, - this.identifier, - new KeysRequest(newKeyPair[0], newKeyPair[1].encryptedString), + this.orgSsoIdentifier, + keysRequest, this.kdf, this.kdfConfig.iterations, this.kdfConfig.memory, @@ -171,13 +210,32 @@ export class SetPasswordComponent extends BaseChangePasswordComponent { protected async onSetPasswordSuccess( masterKey: MasterKey, userKey: [UserKey, EncString], - keyPair: [string, EncString] + keyPair: [string, EncString] | null ) { + // Clear force set password reason to allow navigation back to vault. + await this.stateService.setForceSetPasswordReason(ForceSetPasswordReason.None); + + // User now has a password so update account decryption options in state + const acctDecryptionOpts: AccountDecryptionOptions = + await this.stateService.getAccountDecryptionOptions(); + + acctDecryptionOpts.hasMasterPassword = true; + await this.stateService.setAccountDecryptionOptions(acctDecryptionOpts); + await this.stateService.setKdfType(this.kdf); await this.stateService.setKdfConfig(this.kdfConfig); await this.cryptoService.setMasterKey(masterKey); await this.cryptoService.setUserKey(userKey[0]); - await this.cryptoService.setPrivateKey(keyPair[1].encryptedString); + + // Set private key only for new JIT provisioned users in MP encryption orgs + // Existing TDE users will have private key set on sync or on login + if ( + keyPair !== null && + this.forceSetPasswordReason != + ForceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission + ) { + await this.cryptoService.setPrivateKey(keyPair[1].encryptedString); + } const localMasterKeyHash = await this.cryptoService.hashMasterKey( this.masterPassword, diff --git a/libs/angular/src/auth/components/sso.component.spec.ts b/libs/angular/src/auth/components/sso.component.spec.ts index 6d81b3d61e..3667e62905 100644 --- a/libs/angular/src/auth/components/sso.component.spec.ts +++ b/libs/angular/src/auth/components/sso.component.spec.ts @@ -8,7 +8,7 @@ import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type"; import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; -import { ForceResetPasswordReason } from "@bitwarden/common/auth/models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { KeyConnectorUserDecryptionOption } from "@bitwarden/common/auth/models/domain/user-decryption-options/key-connector-user-decryption-option"; import { TrustedDeviceUserDecryptionOption } from "@bitwarden/common/auth/models/domain/user-decryption-options/trusted-device-user-decryption-option"; import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.service.abstraction"; @@ -345,16 +345,32 @@ describe("SsoComponent", () => { mockAuthService.logIn.mockResolvedValue(authResult); }); - testChangePasswordOnSuccessfulLogin(); - testOnSuccessfulLoginChangePasswordNavigate(); + it("navigates to the component's defined trustedDeviceEncRoute route and sets correct flag when onSuccessfulLoginTdeNavigate is undefined ", async () => { + await _component.logIn(code, codeVerifier, orgIdFromState); + expect(mockAuthService.logIn).toHaveBeenCalledTimes(1); + + expect(mockStateService.setForceSetPasswordReason).toHaveBeenCalledWith( + ForceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission + ); + + expect(mockOnSuccessfulLoginTdeNavigate).not.toHaveBeenCalled(); + + expect(mockRouter.navigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledWith( + [_component.trustedDeviceEncRoute], + undefined + ); + + expect(mockLogService.error).not.toHaveBeenCalled(); + }); }); describe("Given Trusted Device Encryption is enabled, user doesn't need to set a MP, and forcePasswordReset is required", () => { [ - ForceResetPasswordReason.AdminForcePasswordReset, + ForceSetPasswordReason.AdminForcePasswordReset, // ForceResetPasswordReason.WeakMasterPassword, -- not possible in SSO flow as set client side ].forEach((forceResetPasswordReason) => { - const reasonString = ForceResetPasswordReason[forceResetPasswordReason]; + const reasonString = ForceSetPasswordReason[forceResetPasswordReason]; let authResult; beforeEach(() => { mockStateService.getAccountDecryptionOptions.mockResolvedValue( @@ -362,7 +378,7 @@ describe("SsoComponent", () => { ); authResult = new AuthResult(); - authResult.forcePasswordReset = ForceResetPasswordReason.AdminForcePasswordReset; + authResult.forcePasswordReset = ForceSetPasswordReason.AdminForcePasswordReset; mockAuthService.logIn.mockResolvedValue(authResult); }); @@ -379,7 +395,7 @@ describe("SsoComponent", () => { ); authResult = new AuthResult(); - authResult.forcePasswordReset = ForceResetPasswordReason.None; + authResult.forcePasswordReset = ForceSetPasswordReason.None; mockAuthService.logIn.mockResolvedValue(authResult); }); @@ -448,10 +464,10 @@ describe("SsoComponent", () => { describe("Force Master Password Reset scenarios", () => { [ - ForceResetPasswordReason.AdminForcePasswordReset, + ForceSetPasswordReason.AdminForcePasswordReset, // ForceResetPasswordReason.WeakMasterPassword, -- not possible in SSO flow as set client side ].forEach((forceResetPasswordReason) => { - const reasonString = ForceResetPasswordReason[forceResetPasswordReason]; + const reasonString = ForceSetPasswordReason[forceResetPasswordReason]; beforeEach(() => { // use standard user with MP because this test is not concerned with password reset. @@ -477,7 +493,7 @@ describe("SsoComponent", () => { mockStateService.getAccountDecryptionOptions.mockResolvedValue( mockAcctDecryptionOpts.withMasterPassword ); - authResult.forcePasswordReset = ForceResetPasswordReason.None; + authResult.forcePasswordReset = ForceSetPasswordReason.None; mockAuthService.logIn.mockResolvedValue(authResult); }); diff --git a/libs/angular/src/auth/components/sso.component.ts b/libs/angular/src/auth/components/sso.component.ts index 8909c1c1c4..fe02304a4e 100644 --- a/libs/angular/src/auth/components/sso.component.ts +++ b/libs/angular/src/auth/components/sso.component.ts @@ -5,7 +5,7 @@ import { first } from "rxjs/operators"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; -import { ForceResetPasswordReason } from "@bitwarden/common/auth/models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { SsoLoginCredentials } from "@bitwarden/common/auth/models/domain/login-credentials"; import { TrustedDeviceUserDecryptionOption } from "@bitwarden/common/auth/models/domain/user-decryption-options/trusted-device-user-decryption-option"; import { SsoPreValidateResponse } from "@bitwarden/common/auth/models/response/sso-pre-validate.response"; @@ -201,12 +201,19 @@ export class SsoComponent { // Everything after the 2FA check is considered a successful login // Just have to figure out where to send the user - // Save off the OrgSsoIdentifier for use in the TDE flows + // Save off the OrgSsoIdentifier for use in the TDE flows (or elsewhere) // - TDE login decryption options component // - Browser SSO on extension open // Note: you cannot set this in state before 2FA b/c there won't be an account in state. await this.stateService.setUserSsoOrganizationIdentifier(orgSsoIdentifier); + // Users enrolled in admin acct recovery can be forced to set a new password after + // having the admin set a temp password for them (affects TDE & standard users) + if (authResult.forcePasswordReset == ForceSetPasswordReason.AdminForcePasswordReset) { + // Weak password is not a valid scenario here b/c we cannot have evaluated a MP yet + return await this.handleForcePasswordReset(orgSsoIdentifier); + } + const tdeEnabled = await this.isTrustedDeviceEncEnabled( acctDecryptionOpts.trustedDeviceOption ); @@ -231,12 +238,6 @@ export class SsoComponent { return await this.handleChangePasswordRequired(orgSsoIdentifier); } - // Users enrolled in admin acct recovery can be forced to set a new password after - // having the admin set a temp password for them - if (authResult.forcePasswordReset == ForceResetPasswordReason.AdminForcePasswordReset) { - return await this.handleForcePasswordReset(orgSsoIdentifier); - } - // Standard SSO login success case return await this.handleSuccessfulLogin(); } catch (e) { @@ -277,12 +278,12 @@ export class SsoComponent { !acctDecryptionOpts.hasMasterPassword && acctDecryptionOpts.trustedDeviceOption.hasManageResetPasswordPermission ) { - // Change implies going no password -> password in this case - return await this.handleChangePasswordRequired(orgIdentifier); - } - - if (authResult.forcePasswordReset !== ForceResetPasswordReason.None) { - return await this.handleForcePasswordReset(orgIdentifier); + // Set flag so that auth guard can redirect to set password screen after decryption (trusted or untrusted device) + // Note: we cannot directly navigate in this scenario as we are in a pre-decryption state, and + // if you try to set a new MP before decrypting, you will invalidate the user's data by making a new user key. + await this.stateService.setForceSetPasswordReason( + ForceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission + ); } if (this.onSuccessfulLoginTde != null) { diff --git a/libs/angular/src/auth/components/two-factor.component.spec.ts b/libs/angular/src/auth/components/two-factor.component.spec.ts index 9e147f3357..49bc25d2b6 100644 --- a/libs/angular/src/auth/components/two-factor.component.spec.ts +++ b/libs/angular/src/auth/components/two-factor.component.spec.ts @@ -10,7 +10,7 @@ import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { LoginService } from "@bitwarden/common/auth/abstractions/login.service"; import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service"; import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; -import { ForceResetPasswordReason } from "@bitwarden/common/auth/models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { KeyConnectorUserDecryptionOption } from "@bitwarden/common/auth/models/domain/user-decryption-options/key-connector-user-decryption-option"; import { TrustedDeviceUserDecryptionOption } from "@bitwarden/common/auth/models/domain/user-decryption-options/trusted-device-user-decryption-option"; import { TokenTwoFactorRequest } from "@bitwarden/common/auth/models/request/identity-token/token-two-factor.request"; @@ -310,10 +310,10 @@ describe("TwoFactorComponent", () => { describe("Force Master Password Reset scenarios", () => { [ - ForceResetPasswordReason.AdminForcePasswordReset, - ForceResetPasswordReason.WeakMasterPassword, + ForceSetPasswordReason.AdminForcePasswordReset, + ForceSetPasswordReason.WeakMasterPassword, ].forEach((forceResetPasswordReason) => { - const reasonString = ForceResetPasswordReason[forceResetPasswordReason]; + const reasonString = ForceSetPasswordReason[forceResetPasswordReason]; beforeEach(() => { // use standard user with MP because this test is not concerned with password reset. @@ -389,15 +389,30 @@ describe("TwoFactorComponent", () => { mockAuthService.logInTwoFactor.mockResolvedValue(authResult); }); - testChangePasswordOnSuccessfulLogin(); + it("navigates to the component's defined trusted device encryption route and sets correct flag when user doesn't have a MP and key connector isn't enabled", async () => { + // Act + await component.doSubmit(); + + // Assert + + expect(mockStateService.setForceSetPasswordReason).toHaveBeenCalledWith( + ForceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission + ); + + expect(mockRouter.navigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledWith( + [_component.trustedDeviceEncRoute], + undefined + ); + }); }); describe("Given Trusted Device Encryption is enabled, user doesn't need to set a MP, and forcePasswordReset is required", () => { [ - ForceResetPasswordReason.AdminForcePasswordReset, - ForceResetPasswordReason.WeakMasterPassword, + ForceSetPasswordReason.AdminForcePasswordReset, + ForceSetPasswordReason.WeakMasterPassword, ].forEach((forceResetPasswordReason) => { - const reasonString = ForceResetPasswordReason[forceResetPasswordReason]; + const reasonString = ForceSetPasswordReason[forceResetPasswordReason]; beforeEach(() => { // use standard user with MP because this test is not concerned with password reset. @@ -422,7 +437,7 @@ describe("TwoFactorComponent", () => { ); authResult = new AuthResult(); - authResult.forcePasswordReset = ForceResetPasswordReason.None; + authResult.forcePasswordReset = ForceSetPasswordReason.None; mockAuthService.logInTwoFactor.mockResolvedValue(authResult); }); diff --git a/libs/angular/src/auth/components/two-factor.component.ts b/libs/angular/src/auth/components/two-factor.component.ts index 756218a1e1..303c0adc16 100644 --- a/libs/angular/src/auth/components/two-factor.component.ts +++ b/libs/angular/src/auth/components/two-factor.component.ts @@ -11,7 +11,7 @@ import { LoginService } from "@bitwarden/common/auth/abstractions/login.service" import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service"; import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type"; import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; -import { ForceResetPasswordReason } from "@bitwarden/common/auth/models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { TrustedDeviceUserDecryptionOption } from "@bitwarden/common/auth/models/domain/user-decryption-options/trusted-device-user-decryption-option"; import { TokenTwoFactorRequest } from "@bitwarden/common/auth/models/request/identity-token/token-two-factor.request"; import { TwoFactorEmailRequest } from "@bitwarden/common/auth/models/request/two-factor-email.request"; @@ -239,9 +239,13 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI // - TDE login decryption options component // - Browser SSO on extension open await this.stateService.setUserSsoOrganizationIdentifier(this.orgIdentifier); - this.loginService.clearValues(); + // note: this flow affects both TDE & standard users + if (this.isForcePasswordResetRequired(authResult)) { + return await this.handleForcePasswordReset(this.orgIdentifier); + } + const acctDecryptionOpts: AccountDecryptionOptions = await this.stateService.getAccountDecryptionOptions(); @@ -264,12 +268,6 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI return await this.handleChangePasswordRequired(this.orgIdentifier); } - // Users can be forced to reset their password via an admin or org policy - // disallowing weak passwords - if (authResult.forcePasswordReset !== ForceResetPasswordReason.None) { - return await this.handleForcePasswordReset(this.orgIdentifier); - } - return await this.handleSuccessfulLogin(); } @@ -298,16 +296,12 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI !acctDecryptionOpts.hasMasterPassword && acctDecryptionOpts.trustedDeviceOption.hasManageResetPasswordPermission ) { - // Change implies going no password -> password in this case - return await this.handleChangePasswordRequired(orgIdentifier); - } - - // Users can be forced to reset their password via an admin or org policy disallowing weak passwords - // Note: this is different from SSO component login flow as a user can - // login with MP and then have to pass 2FA to finish login and we can actually - // evaluate if they have a weak password at this time. - if (authResult.forcePasswordReset !== ForceResetPasswordReason.None) { - return await this.handleForcePasswordReset(orgIdentifier); + // Set flag so that auth guard can redirect to set password screen after decryption (trusted or untrusted device) + // Note: we cannot directly navigate to the set password screen in this scenario as we are in a pre-decryption state, and + // if you try to set a new MP before decrypting, you will invalidate the user's data by making a new user key. + await this.stateService.setForceSetPasswordReason( + ForceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission + ); } if (this.onSuccessfulLoginTde != null) { @@ -332,6 +326,25 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI }); } + /** + * Determines if a user needs to reset their password based on certain conditions. + * Users can be forced to reset their password via an admin or org policy disallowing weak passwords. + * Note: this is different from the SSO component login flow as a user can + * login with MP and then have to pass 2FA to finish login and we can actually + * evaluate if they have a weak password at that time. + * + * @param {AuthResult} authResult - The authentication result. + * @returns {boolean} Returns true if a password reset is required, false otherwise. + */ + private isForcePasswordResetRequired(authResult: AuthResult): boolean { + const forceResetReasons = [ + ForceSetPasswordReason.AdminForcePasswordReset, + ForceSetPasswordReason.WeakMasterPassword, + ]; + + return forceResetReasons.includes(authResult.forcePasswordReset); + } + private async handleForcePasswordReset(orgIdentifier: string) { this.router.navigate([this.forcePasswordResetRoute], { queryParams: { diff --git a/libs/angular/src/auth/components/update-temp-password.component.ts b/libs/angular/src/auth/components/update-temp-password.component.ts index 7bb65ab68d..4c27bfa4f0 100644 --- a/libs/angular/src/auth/components/update-temp-password.component.ts +++ b/libs/angular/src/auth/components/update-temp-password.component.ts @@ -6,7 +6,7 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli import { MasterPasswordPolicyOptions } from "@bitwarden/common/admin-console/models/domain/master-password-policy-options"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { VerificationType } from "@bitwarden/common/auth/enums/verification-type"; -import { ForceResetPasswordReason } from "@bitwarden/common/auth/models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { PasswordRequest } from "@bitwarden/common/auth/models/request/password.request"; import { UpdateTempPasswordRequest } from "@bitwarden/common/auth/models/request/update-temp-password.request"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; @@ -30,7 +30,7 @@ export class UpdateTempPasswordComponent extends BaseChangePasswordComponent { key: string; enforcedPolicyOptions: MasterPasswordPolicyOptions; showPassword = false; - reason: ForceResetPasswordReason = ForceResetPasswordReason.None; + reason: ForceSetPasswordReason = ForceSetPasswordReason.None; verification: Verification = { type: VerificationType.MasterPassword, secret: "", @@ -39,7 +39,7 @@ export class UpdateTempPasswordComponent extends BaseChangePasswordComponent { onSuccessfulChangePassword: () => Promise; get requireCurrentPassword(): boolean { - return this.reason === ForceResetPasswordReason.WeakMasterPassword; + return this.reason === ForceSetPasswordReason.WeakMasterPassword; } constructor( @@ -72,10 +72,10 @@ export class UpdateTempPasswordComponent extends BaseChangePasswordComponent { async ngOnInit() { await this.syncService.fullSync(true); - this.reason = await this.stateService.getForcePasswordResetReason(); + this.reason = await this.stateService.getForceSetPasswordReason(); // If we somehow end up here without a reason, go back to the home page - if (this.reason == ForceResetPasswordReason.None) { + if (this.reason == ForceSetPasswordReason.None) { this.router.navigate(["/"]); return; } @@ -84,7 +84,7 @@ export class UpdateTempPasswordComponent extends BaseChangePasswordComponent { } get masterPasswordWarningText(): string { - return this.reason == ForceResetPasswordReason.WeakMasterPassword + return this.reason == ForceSetPasswordReason.WeakMasterPassword ? this.i18nService.t("updateWeakMasterPasswordWarning") : this.i18nService.t("updateMasterPasswordWarning"); } @@ -146,10 +146,10 @@ export class UpdateTempPasswordComponent extends BaseChangePasswordComponent { ) { try { switch (this.reason) { - case ForceResetPasswordReason.AdminForcePasswordReset: + case ForceSetPasswordReason.AdminForcePasswordReset: this.formPromise = this.updateTempPassword(masterPasswordHash, userKey); break; - case ForceResetPasswordReason.WeakMasterPassword: + case ForceSetPasswordReason.WeakMasterPassword: this.formPromise = this.updatePassword(masterPasswordHash, userKey); break; } @@ -161,7 +161,7 @@ export class UpdateTempPasswordComponent extends BaseChangePasswordComponent { this.i18nService.t("updatedMasterPassword") ); - await this.stateService.setForcePasswordResetReason(ForceResetPasswordReason.None); + await this.stateService.setForceSetPasswordReason(ForceSetPasswordReason.None); if (this.onSuccessfulChangePassword != null) { this.onSuccessfulChangePassword(); diff --git a/libs/angular/src/auth/guards/auth.guard.ts b/libs/angular/src/auth/guards/auth.guard.ts index b8535da937..705ec02335 100644 --- a/libs/angular/src/auth/guards/auth.guard.ts +++ b/libs/angular/src/auth/guards/auth.guard.ts @@ -4,7 +4,7 @@ import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot } from import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; -import { ForceResetPasswordReason } from "@bitwarden/common/auth/models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; @@ -40,9 +40,19 @@ export class AuthGuard implements CanActivate { return this.router.createUrlTree(["/remove-password"]); } + const forceSetPasswordReason = await this.stateService.getForceSetPasswordReason(); + if ( - !routerState.url.includes("update-temp-password") && - (await this.stateService.getForcePasswordResetReason()) != ForceResetPasswordReason.None + forceSetPasswordReason === + ForceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission && + !routerState.url.includes("set-password") + ) { + return this.router.createUrlTree(["/set-password"]); + } + + if ( + forceSetPasswordReason !== ForceSetPasswordReason.None && + !routerState.url.includes("update-temp-password") ) { return this.router.createUrlTree(["/update-temp-password"]); } diff --git a/libs/common/src/admin-console/abstractions/policy/policy-api.service.abstraction.ts b/libs/common/src/admin-console/abstractions/policy/policy-api.service.abstraction.ts index a941569563..ef042b0d5a 100644 --- a/libs/common/src/admin-console/abstractions/policy/policy-api.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/policy/policy-api.service.abstraction.ts @@ -14,10 +14,7 @@ export class PolicyApiServiceAbstraction { email: string, organizationUserId: string ) => Promise>; - getPoliciesByInvitedUser: ( - organizationId: string, - userId: string - ) => Promise>; - getMasterPasswordPoliciesForInvitedUsers: (orgId: string) => Promise; + + getMasterPasswordPolicyOptsForOrgUser: (orgId: string) => Promise; putPolicy: (organizationId: string, type: PolicyType, request: PolicyRequest) => Promise; } diff --git a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts index e43b124dcd..0683bbd585 100644 --- a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts @@ -30,6 +30,7 @@ export abstract class PolicyService { policies: Policy[], orgId: string ) => [ResetPasswordPolicyOptions, boolean]; + mapPolicyFromResponse: (policyResponse: PolicyResponse) => Policy; mapPoliciesFromToken: (policiesResponse: ListResponse) => Policy[]; policyAppliesToUser: ( policyType: PolicyType, diff --git a/libs/common/src/admin-console/services/policy/policy-api.service.ts b/libs/common/src/admin-console/services/policy/policy-api.service.ts index c0a572ae8d..648bd53dcb 100644 --- a/libs/common/src/admin-console/services/policy/policy-api.service.ts +++ b/libs/common/src/admin-console/services/policy/policy-api.service.ts @@ -1,6 +1,8 @@ import { firstValueFrom } from "rxjs"; import { ApiService } from "../../../abstractions/api.service"; +import { HttpStatusCode } from "../../../enums"; +import { ErrorResponse } from "../../../models/response/error.response"; import { ListResponse } from "../../../models/response/list.response"; import { StateService } from "../../../platform/abstractions/state.service"; import { Utils } from "../../../platform/misc/utils"; @@ -65,27 +67,47 @@ export class PolicyApiService implements PolicyApiServiceAbstraction { return new ListResponse(r, PolicyResponse); } - async getPoliciesByInvitedUser( - organizationId: string, - userId: string - ): Promise> { - const r = await this.apiService.send( + private async getMasterPasswordPolicyResponseForOrgUser( + organizationId: string + ): Promise { + const response = await this.apiService.send( "GET", - "/organizations/" + organizationId + "/policies/invited-user?" + "userId=" + userId, + "/organizations/" + organizationId + "/policies/master-password", null, - false, + true, true ); - return new ListResponse(r, PolicyResponse); + + return new PolicyResponse(response); } - async getMasterPasswordPoliciesForInvitedUsers( + async getMasterPasswordPolicyOptsForOrgUser( orgId: string - ): Promise { - const userId = await this.stateService.getUserId(); - const response = await this.getPoliciesByInvitedUser(orgId, userId); - const policies = await this.policyService.mapPoliciesFromToken(response); - return await firstValueFrom(this.policyService.masterPasswordPolicyOptions$(policies)); + ): Promise { + try { + const masterPasswordPolicyResponse = await this.getMasterPasswordPolicyResponseForOrgUser( + orgId + ); + + const masterPasswordPolicy = this.policyService.mapPolicyFromResponse( + masterPasswordPolicyResponse + ); + + if (!masterPasswordPolicy) { + return null; + } + + return await firstValueFrom( + this.policyService.masterPasswordPolicyOptions$([masterPasswordPolicy]) + ); + } catch (error) { + // If policy not found, return null + if (error instanceof ErrorResponse && error.statusCode === HttpStatusCode.NotFound) { + return null; + } + // otherwise rethrow error + throw error; + } } async putPolicy(organizationId: string, type: PolicyType, request: PolicyRequest): Promise { diff --git a/libs/common/src/admin-console/services/policy/policy.service.ts b/libs/common/src/admin-console/services/policy/policy.service.ts index 4ef3356c18..d57a7fef3c 100644 --- a/libs/common/src/admin-console/services/policy/policy.service.ts +++ b/libs/common/src/admin-console/services/policy/policy.service.ts @@ -218,13 +218,17 @@ export class PolicyService implements InternalPolicyServiceAbstraction { return [resetPasswordPolicyOptions, policy?.enabled ?? false]; } + mapPolicyFromResponse(policyResponse: PolicyResponse): Policy { + const policyData = new PolicyData(policyResponse); + return new Policy(policyData); + } + mapPoliciesFromToken(policiesResponse: ListResponse): Policy[] { - if (policiesResponse == null || policiesResponse.data == null) { + if (policiesResponse?.data == null) { return null; } - const policiesData = policiesResponse.data.map((p) => new PolicyData(p)); - return policiesData.map((p) => new Policy(p)); + return policiesResponse.data.map((response) => this.mapPolicyFromResponse(response)); } async policyAppliesToUser( diff --git a/libs/common/src/auth/login-strategies/login.strategy.spec.ts b/libs/common/src/auth/login-strategies/login.strategy.spec.ts index d691f934d4..4e7e7b216a 100644 --- a/libs/common/src/auth/login-strategies/login.strategy.spec.ts +++ b/libs/common/src/auth/login-strategies/login.strategy.spec.ts @@ -33,7 +33,7 @@ import { TokenService } from "../abstractions/token.service"; import { TwoFactorService } from "../abstractions/two-factor.service"; import { TwoFactorProviderType } from "../enums/two-factor-provider-type"; import { AuthResult } from "../models/domain/auth-result"; -import { ForceResetPasswordReason } from "../models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "../models/domain/force-set-password-reason"; import { PasswordLoginCredentials } from "../models/domain/login-credentials"; import { PasswordTokenRequest } from "../models/request/identity-token/password-token.request"; import { TokenTwoFactorRequest } from "../models/request/identity-token/token-two-factor.request"; @@ -229,7 +229,7 @@ describe("LoginStrategy", () => { const result = await passwordLoginStrategy.logIn(credentials); expect(result).toEqual({ - forcePasswordReset: ForceResetPasswordReason.AdminForcePasswordReset, + forcePasswordReset: ForceSetPasswordReason.AdminForcePasswordReset, resetMasterPassword: true, twoFactorProviders: null, captchaSiteKey: "", diff --git a/libs/common/src/auth/login-strategies/login.strategy.ts b/libs/common/src/auth/login-strategies/login.strategy.ts index 09e3641f54..fb328c865c 100644 --- a/libs/common/src/auth/login-strategies/login.strategy.ts +++ b/libs/common/src/auth/login-strategies/login.strategy.ts @@ -18,7 +18,7 @@ import { TokenService } from "../abstractions/token.service"; import { TwoFactorService } from "../abstractions/two-factor.service"; import { TwoFactorProviderType } from "../enums/two-factor-provider-type"; import { AuthResult } from "../models/domain/auth-result"; -import { ForceResetPasswordReason } from "../models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "../models/domain/force-set-password-reason"; import { AuthRequestLoginCredentials, PasswordLoginCredentials, @@ -166,7 +166,7 @@ export abstract class LoginStrategy { // Convert boolean to enum if (response.forcePasswordReset) { - result.forcePasswordReset = ForceResetPasswordReason.AdminForcePasswordReset; + result.forcePasswordReset = ForceSetPasswordReason.AdminForcePasswordReset; } // Must come before setting keys, user key needs email to update additional keys diff --git a/libs/common/src/auth/login-strategies/password-login.strategy.spec.ts b/libs/common/src/auth/login-strategies/password-login.strategy.spec.ts index 03f2a26b1c..a0f8091bca 100644 --- a/libs/common/src/auth/login-strategies/password-login.strategy.spec.ts +++ b/libs/common/src/auth/login-strategies/password-login.strategy.spec.ts @@ -24,7 +24,7 @@ import { AuthService } from "../abstractions/auth.service"; import { TokenService } from "../abstractions/token.service"; import { TwoFactorService } from "../abstractions/two-factor.service"; import { TwoFactorProviderType } from "../enums/two-factor-provider-type"; -import { ForceResetPasswordReason } from "../models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "../models/domain/force-set-password-reason"; import { PasswordLoginCredentials } from "../models/domain/login-credentials"; import { IdentityTokenResponse } from "../models/response/identity-token.response"; import { IdentityTwoFactorResponse } from "../models/response/identity-two-factor.response"; @@ -154,7 +154,7 @@ describe("PasswordLoginStrategy", () => { const result = await passwordLoginStrategy.logIn(credentials); expect(policyService.evaluateMasterPassword).not.toHaveBeenCalled(); - expect(result.forcePasswordReset).toEqual(ForceResetPasswordReason.None); + expect(result.forcePasswordReset).toEqual(ForceSetPasswordReason.None); }); it("does not force the user to update their master password when it meets requirements", async () => { @@ -164,7 +164,7 @@ describe("PasswordLoginStrategy", () => { const result = await passwordLoginStrategy.logIn(credentials); expect(policyService.evaluateMasterPassword).toHaveBeenCalled(); - expect(result.forcePasswordReset).toEqual(ForceResetPasswordReason.None); + expect(result.forcePasswordReset).toEqual(ForceSetPasswordReason.None); }); it("forces the user to update their master password on successful login when it does not meet master password policy requirements", async () => { @@ -174,10 +174,10 @@ describe("PasswordLoginStrategy", () => { const result = await passwordLoginStrategy.logIn(credentials); expect(policyService.evaluateMasterPassword).toHaveBeenCalled(); - expect(stateService.setForcePasswordResetReason).toHaveBeenCalledWith( - ForceResetPasswordReason.WeakMasterPassword + expect(stateService.setForceSetPasswordReason).toHaveBeenCalledWith( + ForceSetPasswordReason.WeakMasterPassword ); - expect(result.forcePasswordReset).toEqual(ForceResetPasswordReason.WeakMasterPassword); + expect(result.forcePasswordReset).toEqual(ForceSetPasswordReason.WeakMasterPassword); }); it("forces the user to update their master password on successful 2FA login when it does not meet master password policy requirements", async () => { @@ -210,12 +210,12 @@ describe("PasswordLoginStrategy", () => { ); // First login attempt should not save the force password reset options - expect(firstResult.forcePasswordReset).toEqual(ForceResetPasswordReason.None); + expect(firstResult.forcePasswordReset).toEqual(ForceSetPasswordReason.None); // Second login attempt should save the force password reset options and return in result - expect(stateService.setForcePasswordResetReason).toHaveBeenCalledWith( - ForceResetPasswordReason.WeakMasterPassword + expect(stateService.setForceSetPasswordReason).toHaveBeenCalledWith( + ForceSetPasswordReason.WeakMasterPassword ); - expect(secondResult.forcePasswordReset).toEqual(ForceResetPasswordReason.WeakMasterPassword); + expect(secondResult.forcePasswordReset).toEqual(ForceSetPasswordReason.WeakMasterPassword); }); }); diff --git a/libs/common/src/auth/login-strategies/password-login.strategy.ts b/libs/common/src/auth/login-strategies/password-login.strategy.ts index 788116cf11..6475bbe5a7 100644 --- a/libs/common/src/auth/login-strategies/password-login.strategy.ts +++ b/libs/common/src/auth/login-strategies/password-login.strategy.ts @@ -14,7 +14,7 @@ import { AuthService } from "../abstractions/auth.service"; import { TokenService } from "../abstractions/token.service"; import { TwoFactorService } from "../abstractions/two-factor.service"; import { AuthResult } from "../models/domain/auth-result"; -import { ForceResetPasswordReason } from "../models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "../models/domain/force-set-password-reason"; import { PasswordLoginCredentials } from "../models/domain/login-credentials"; import { PasswordTokenRequest } from "../models/request/identity-token/password-token.request"; import { TokenTwoFactorRequest } from "../models/request/identity-token/token-two-factor.request"; @@ -42,7 +42,7 @@ export class PasswordLoginStrategy extends LoginStrategy { * Options to track if the user needs to update their password due to a password that does not meet an organization's * master password policy. */ - private forcePasswordResetReason: ForceResetPasswordReason = ForceResetPasswordReason.None; + private forcePasswordResetReason: ForceSetPasswordReason = ForceSetPasswordReason.None; constructor( cryptoService: CryptoService, @@ -82,9 +82,9 @@ export class PasswordLoginStrategy extends LoginStrategy { if ( !result.requiresTwoFactor && !result.requiresCaptcha && - this.forcePasswordResetReason != ForceResetPasswordReason.None + this.forcePasswordResetReason != ForceSetPasswordReason.None ) { - await this.stateService.setForcePasswordResetReason(this.forcePasswordResetReason); + await this.stateService.setForceSetPasswordReason(this.forcePasswordResetReason); result.forcePasswordReset = this.forcePasswordResetReason; } @@ -128,13 +128,13 @@ export class PasswordLoginStrategy extends LoginStrategy { if (!meetsRequirements) { if (authResult.requiresCaptcha || authResult.requiresTwoFactor) { // Save the flag to this strategy for later use as the master password is about to pass out of scope - this.forcePasswordResetReason = ForceResetPasswordReason.WeakMasterPassword; + this.forcePasswordResetReason = ForceSetPasswordReason.WeakMasterPassword; } else { // Authentication was successful, save the force update password options with the state service - await this.stateService.setForcePasswordResetReason( - ForceResetPasswordReason.WeakMasterPassword + await this.stateService.setForceSetPasswordReason( + ForceSetPasswordReason.WeakMasterPassword ); - authResult.forcePasswordReset = ForceResetPasswordReason.WeakMasterPassword; + authResult.forcePasswordReset = ForceSetPasswordReason.WeakMasterPassword; } } } diff --git a/libs/common/src/auth/login-strategies/sso-login.strategy.ts b/libs/common/src/auth/login-strategies/sso-login.strategy.ts index 9c550b833f..f285676751 100644 --- a/libs/common/src/auth/login-strategies/sso-login.strategy.ts +++ b/libs/common/src/auth/login-strategies/sso-login.strategy.ts @@ -14,7 +14,7 @@ import { DeviceTrustCryptoServiceAbstraction } from "../abstractions/device-trus import { KeyConnectorService } from "../abstractions/key-connector.service"; import { TokenService } from "../abstractions/token.service"; import { TwoFactorService } from "../abstractions/two-factor.service"; -import { ForceResetPasswordReason } from "../models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "../models/domain/force-set-password-reason"; import { SsoLoginCredentials } from "../models/domain/login-credentials"; import { SsoTokenRequest } from "../models/request/identity-token/sso-token.request"; import { IdentityTokenResponse } from "../models/response/identity-token.response"; @@ -75,8 +75,8 @@ export class SsoLoginStrategy extends LoginStrategy { this.ssoEmail2FaSessionToken = ssoAuthResult.ssoEmail2FaSessionToken; // Auth guard currently handles redirects for this. - if (ssoAuthResult.forcePasswordReset == ForceResetPasswordReason.AdminForcePasswordReset) { - await this.stateService.setForcePasswordResetReason(ssoAuthResult.forcePasswordReset); + if (ssoAuthResult.forcePasswordReset == ForceSetPasswordReason.AdminForcePasswordReset) { + await this.stateService.setForceSetPasswordReason(ssoAuthResult.forcePasswordReset); } return ssoAuthResult; diff --git a/libs/common/src/auth/models/domain/auth-result.ts b/libs/common/src/auth/models/domain/auth-result.ts index 6900cba1c4..16f58c6459 100644 --- a/libs/common/src/auth/models/domain/auth-result.ts +++ b/libs/common/src/auth/models/domain/auth-result.ts @@ -1,7 +1,7 @@ import { Utils } from "../../../platform/misc/utils"; import { TwoFactorProviderType } from "../../enums/two-factor-provider-type"; -import { ForceResetPasswordReason } from "./force-reset-password-reason"; +import { ForceSetPasswordReason } from "./force-set-password-reason"; export class AuthResult { captchaSiteKey = ""; @@ -13,7 +13,7 @@ export class AuthResult { * */ resetMasterPassword = false; - forcePasswordReset: ForceResetPasswordReason = ForceResetPasswordReason.None; + forcePasswordReset: ForceSetPasswordReason = ForceSetPasswordReason.None; twoFactorProviders: Map = null; ssoEmail2FaSessionToken?: string; email: string; diff --git a/libs/common/src/auth/models/domain/force-reset-password-reason.ts b/libs/common/src/auth/models/domain/force-set-password-reason.ts similarity index 65% rename from libs/common/src/auth/models/domain/force-reset-password-reason.ts rename to libs/common/src/auth/models/domain/force-set-password-reason.ts index 99e461c2ea..a6b407d2f1 100644 --- a/libs/common/src/auth/models/domain/force-reset-password-reason.ts +++ b/libs/common/src/auth/models/domain/force-set-password-reason.ts @@ -1,8 +1,8 @@ /* - * This enum is used to determine if a user should be forced to reset their password + * This enum is used to determine if a user should be forced to initially set or reset their password * on login (server flag) or unlock via MP (client evaluation). */ -export enum ForceResetPasswordReason { +export enum ForceSetPasswordReason { /** * A password reset should not be forced. */ @@ -20,4 +20,10 @@ export enum ForceResetPasswordReason { * Only set client side b/c server can't evaluate MP. */ WeakMasterPassword, + + /** + * Occurs when a TDE user without a password obtains the password reset permission. + * Set post login & decryption client side and by server in sync (to catch logged in users). + */ + TdeUserWithoutPasswordHasPasswordResetPermission, } diff --git a/libs/common/src/auth/models/request/set-password.request.ts b/libs/common/src/auth/models/request/set-password.request.ts index 1fa6b4c7d2..30fa05662f 100644 --- a/libs/common/src/auth/models/request/set-password.request.ts +++ b/libs/common/src/auth/models/request/set-password.request.ts @@ -5,7 +5,7 @@ export class SetPasswordRequest { masterPasswordHash: string; key: string; masterPasswordHint: string; - keys: KeysRequest; + keys: KeysRequest | null; kdf: KdfType; kdfIterations: number; kdfMemory?: number; @@ -17,7 +17,7 @@ export class SetPasswordRequest { key: string, masterPasswordHint: string, orgIdentifier: string, - keys: KeysRequest, + keys: KeysRequest | null, kdf: KdfType, kdfIterations: number, kdfMemory?: number, diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 571dad6478..a40f6b36b1 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -7,7 +7,7 @@ import { ProviderData } from "../../admin-console/models/data/provider.data"; import { Policy } from "../../admin-console/models/domain/policy"; import { AdminAuthRequestStorable } from "../../auth/models/domain/admin-auth-req-storable"; import { EnvironmentUrls } from "../../auth/models/domain/environment-urls"; -import { ForceResetPasswordReason } from "../../auth/models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "../../auth/models/domain/force-set-password-reason"; import { KdfConfig } from "../../auth/models/domain/kdf-config"; import { BiometricKey } from "../../auth/types/biometric-key"; import { KdfType, ThemeType, UriMatchType } from "../../enums"; @@ -391,9 +391,9 @@ export abstract class StateService { setEverHadUserKey: (value: boolean, options?: StorageOptions) => Promise; getEverBeenUnlocked: (options?: StorageOptions) => Promise; setEverBeenUnlocked: (value: boolean, options?: StorageOptions) => Promise; - getForcePasswordResetReason: (options?: StorageOptions) => Promise; - setForcePasswordResetReason: ( - value: ForceResetPasswordReason, + getForceSetPasswordReason: (options?: StorageOptions) => Promise; + setForceSetPasswordReason: ( + value: ForceSetPasswordReason, options?: StorageOptions ) => Promise; getInstalledVersion: (options?: StorageOptions) => Promise; diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 6d85d6501f..2583c07461 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -8,7 +8,7 @@ import { Policy } from "../../../admin-console/models/domain/policy"; import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; import { AdminAuthRequestStorable } from "../../../auth/models/domain/admin-auth-req-storable"; import { EnvironmentUrls } from "../../../auth/models/domain/environment-urls"; -import { ForceResetPasswordReason } from "../../../auth/models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason"; import { KeyConnectorUserDecryptionOption } from "../../../auth/models/domain/user-decryption-options/key-connector-user-decryption-option"; import { TrustedDeviceUserDecryptionOption } from "../../../auth/models/domain/user-decryption-options/trusted-device-user-decryption-option"; import { IdentityTokenResponse } from "../../../auth/models/response/identity-token.response"; @@ -194,7 +194,7 @@ export class AccountProfile { entityType?: string; everHadUserKey?: boolean; everBeenUnlocked?: boolean; - forcePasswordResetReason?: ForceResetPasswordReason; + forceSetPasswordReason?: ForceSetPasswordReason; hasPremiumPersonally?: boolean; hasPremiumFromOrganization?: boolean; lastSync?: string; diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 78bfddd6e9..d9dcb93a36 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -10,7 +10,7 @@ import { AccountService } from "../../auth/abstractions/account.service"; import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { AdminAuthRequestStorable } from "../../auth/models/domain/admin-auth-req-storable"; import { EnvironmentUrls } from "../../auth/models/domain/environment-urls"; -import { ForceResetPasswordReason } from "../../auth/models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "../../auth/models/domain/force-set-password-reason"; import { KdfConfig } from "../../auth/models/domain/kdf-config"; import { BiometricKey } from "../../auth/types/biometric-key"; import { @@ -2072,24 +2072,24 @@ export class StateService< ); } - async getForcePasswordResetReason(options?: StorageOptions): Promise { + async getForceSetPasswordReason(options?: StorageOptions): Promise { return ( ( await this.getAccount( this.reconcileOptions(options, await this.defaultOnDiskMemoryOptions()) ) - )?.profile?.forcePasswordResetReason ?? ForceResetPasswordReason.None + )?.profile?.forceSetPasswordReason ?? ForceSetPasswordReason.None ); } - async setForcePasswordResetReason( - value: ForceResetPasswordReason, + async setForceSetPasswordReason( + value: ForceSetPasswordReason, options?: StorageOptions ): Promise { const account = await this.getAccount( this.reconcileOptions(options, await this.defaultOnDiskMemoryOptions()) ); - account.profile.forcePasswordResetReason = value; + account.profile.forceSetPasswordReason = value; await this.saveAccount( account, this.reconcileOptions(options, await this.defaultOnDiskMemoryOptions()) diff --git a/libs/common/src/vault/services/sync/sync.service.ts b/libs/common/src/vault/services/sync/sync.service.ts index c8bd274a4d..7368d82f73 100644 --- a/libs/common/src/vault/services/sync/sync.service.ts +++ b/libs/common/src/vault/services/sync/sync.service.ts @@ -3,12 +3,13 @@ import { SettingsService } from "../../../abstractions/settings.service"; import { InternalOrganizationServiceAbstraction } from "../../../admin-console/abstractions/organization/organization.service.abstraction"; import { InternalPolicyService } from "../../../admin-console/abstractions/policy/policy.service.abstraction"; import { ProviderService } from "../../../admin-console/abstractions/provider.service"; +import { OrganizationUserType } from "../../../admin-console/enums"; import { OrganizationData } from "../../../admin-console/models/data/organization.data"; import { PolicyData } from "../../../admin-console/models/data/policy.data"; import { ProviderData } from "../../../admin-console/models/data/provider.data"; import { PolicyResponse } from "../../../admin-console/models/response/policy.response"; import { KeyConnectorService } from "../../../auth/abstractions/key-connector.service"; -import { ForceResetPasswordReason } from "../../../auth/models/domain/force-reset-password-reason"; +import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason"; import { DomainsResponse } from "../../../models/response/domains.response"; import { SyncCipherNotification, @@ -21,6 +22,7 @@ import { LogService } from "../../../platform/abstractions/log.service"; import { MessagingService } from "../../../platform/abstractions/messaging.service"; import { StateService } from "../../../platform/abstractions/state.service"; import { sequentialize } from "../../../platform/misc/sequentialize"; +import { AccountDecryptionOptions } from "../../../platform/models/domain/account"; import { SendData } from "../../../tools/send/models/data/send.data"; import { SendResponse } from "../../../tools/send/models/response/send.response"; import { SendApiService } from "../../../tools/send/services/send-api.service.abstraction"; @@ -314,12 +316,7 @@ export class SyncService implements SyncServiceAbstraction { await this.stateService.setHasPremiumFromOrganization(response.premiumFromOrganization); await this.keyConnectorService.setUsesKeyConnector(response.usesKeyConnector); - // The `forcePasswordReset` flag indicates an admin has reset the user's password and must be updated - if (response.forcePasswordReset) { - await this.stateService.setForcePasswordResetReason( - ForceResetPasswordReason.AdminForcePasswordReset - ); - } + await this.setForceSetPasswordReasonIfNeeded(response); await this.syncProfileOrganizations(response); @@ -338,6 +335,45 @@ export class SyncService implements SyncServiceAbstraction { } } + private async setForceSetPasswordReasonIfNeeded(profileResponse: ProfileResponse) { + // The `forcePasswordReset` flag indicates an admin has reset the user's password and must be updated + if (profileResponse.forcePasswordReset) { + await this.stateService.setForceSetPasswordReason( + ForceSetPasswordReason.AdminForcePasswordReset + ); + } + + const acctDecryptionOpts: AccountDecryptionOptions = + await this.stateService.getAccountDecryptionOptions(); + + // Even though TDE users should only be in a single org (per single org policy), check + // through all orgs for the manageResetPassword permission. If they have it in any org, + // they should be forced to set a password. + let hasManageResetPasswordPermission = false; + for (const org of profileResponse.organizations) { + const isAdmin = org.type === OrganizationUserType.Admin; + const isOwner = org.type === OrganizationUserType.Owner; + + // Note: apparently permissions only come down populated for custom roles. + if (isAdmin || isOwner || (org.permissions && org.permissions.manageResetPassword)) { + hasManageResetPasswordPermission = true; + break; + } + } + + if ( + acctDecryptionOpts.trustedDeviceOption !== undefined && + !acctDecryptionOpts.hasMasterPassword && + hasManageResetPasswordPermission + ) { + // TDE user w/out MP went from having no password reset permission to having it. + // Must set the force password reset reason so the auth guard will redirect to the set password page. + await this.stateService.setForceSetPasswordReason( + ForceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission + ); + } + } + private async syncProfileOrganizations(response: ProfileResponse) { const organizations: { [id: string]: OrganizationData } = {}; response.organizations.forEach((o) => {