From 6e3c3155077a4067d6bdf17bb032b9d823c5a34b Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Tue, 24 Jun 2025 14:17:47 -0400 Subject: [PATCH] feat(change-password-component): Change Password Update [18720] - Stopping work and going to switch to a new branch to pare down some of the solutions that were made to get this over the finish line --- .../src/popup/services/services.module.ts | 14 +++++ .../src/app/services/services.module.ts | 19 +++++-- apps/web/src/app/core/core.module.ts | 11 ++++ .../change-password.component.ts | 53 ++++++++++--------- ...ter-password-policy.service.abstraction.ts | 6 +-- .../policy/policy-api.service.abstraction.ts | 9 ++-- .../default-master-password-policy.service.ts | 36 +++++++++++++ libs/components/src/dialog/dialog.service.ts | 2 +- 8 files changed, 112 insertions(+), 38 deletions(-) create mode 100644 libs/common/src/admin-console/services/policy/default-master-password-policy.service.ts diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 028287df210..f41427e74df 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -191,6 +191,15 @@ import { ExtensionAnonLayoutWrapperDataService } from "../components/extension-a import { DebounceNavigationService } from "./debounce-navigation.service"; import { InitService } from "./init.service"; import { PopupCloseWarningService } from "./popup-close-warning.service"; +import { + MasterPasswordPolicyServiceAbstraction +} from "@bitwarden/common/admin-console/abstractions/policy/master-password-policy.service.abstraction"; +import { + DefaultMasterPasswordPolicyService +} from "@bitwarden/common/admin-console/services/policy/default-master-password-policy.service"; +import { + PolicyApiServiceAbstraction +} from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; const OBSERVABLE_LARGE_OBJECT_MEMORY_STORAGE = new SafeInjectionToken< AbstractStorageService & ObservableStorageService @@ -554,6 +563,11 @@ const safeProviders: SafeProvider[] = [ useClass: DefaultOrganizationInviteService, deps: [], }), + safeProvider({ + provide: MasterPasswordPolicyServiceAbstraction, + useClass: DefaultMasterPasswordPolicyService, + deps: [PolicyApiServiceAbstraction], + }), safeProvider({ provide: LockComponentService, useClass: ExtensionLockComponentService, diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 8d3d59a95b3..a4f61454fa2 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -142,6 +142,15 @@ import { DesktopSetPasswordJitService } from "./desktop-set-password-jit.service import { InitService } from "./init.service"; import { NativeMessagingManifestService } from "./native-messaging-manifest.service"; import { RendererCryptoFunctionService } from "./renderer-crypto-function.service"; +import { + MasterPasswordPolicyServiceAbstraction +} from "@bitwarden/common/admin-console/abstractions/policy/master-password-policy.service.abstraction"; +import { + DefaultMasterPasswordPolicyService +} from "@bitwarden/common/admin-console/services/policy/default-master-password-policy.service"; +import { + PolicyApiServiceAbstraction +} from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; const RELOAD_CALLBACK = new SafeInjectionToken<() => any>("RELOAD_CALLBACK"); @@ -383,16 +392,20 @@ const safeProviders: SafeProvider[] = [ useValue: DefaultOrganizationInviteService, deps: [], }), + safeProvider({ + provide: MasterPasswordPolicyServiceAbstraction, + useClass: DefaultMasterPasswordPolicyService, + deps: [PolicyApiServiceAbstraction], + }), safeProvider({ provide: SetPasswordJitService, useClass: DesktopSetPasswordJitService, deps: [ - ApiService, - MasterPasswordApiService, - KeyService, EncryptService, I18nServiceAbstraction, KdfConfigService, + KeyService, + MasterPasswordApiService, InternalMasterPasswordServiceAbstraction, OrganizationApiServiceAbstraction, OrganizationUserApiService, diff --git a/apps/web/src/app/core/core.module.ts b/apps/web/src/app/core/core.module.ts index 50bacccbfe3..7b9096e6607 100644 --- a/apps/web/src/app/core/core.module.ts +++ b/apps/web/src/app/core/core.module.ts @@ -41,11 +41,17 @@ import { } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction"; +import { + MasterPasswordPolicyServiceAbstraction +} from "@bitwarden/common/admin-console/abstractions/policy/master-password-policy.service.abstraction"; import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { InternalPolicyService, PolicyService, } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { + DefaultMasterPasswordPolicyService +} from "@bitwarden/common/admin-console/services/policy/default-master-password-policy.service"; import { AccountApiService as AccountApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/account-api.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; @@ -250,6 +256,11 @@ const safeProviders: SafeProvider[] = [ useClass: WebOrganizationInviteService, deps: [GlobalStateProvider], }), + safeProvider({ + provide: MasterPasswordPolicyServiceAbstraction, + useClass: DefaultMasterPasswordPolicyService, + deps: [PolicyApiServiceAbstraction], + }), safeProvider({ provide: RegistrationFinishServiceAbstraction, useClass: WebRegistrationFinishService, diff --git a/libs/auth/src/angular/change-password/change-password.component.ts b/libs/auth/src/angular/change-password/change-password.component.ts index 1174c68e8e9..e9e60df42fc 100644 --- a/libs/auth/src/angular/change-password/change-password.component.ts +++ b/libs/auth/src/angular/change-password/change-password.component.ts @@ -15,11 +15,9 @@ import { SyncService } from "@bitwarden/common/platform/sync"; import { UserId } from "@bitwarden/common/types/guid"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports -import { DialogService, ToastService } from "@bitwarden/components"; +import { AnonLayoutWrapperDataService, DialogService, ToastService, Icons } from "@bitwarden/components"; import { I18nPipe } from "@bitwarden/ui-common"; -import { AnonLayoutWrapperDataService } from "../anon-layout/anon-layout-wrapper-data.service"; -import { LockIcon } from "../icons"; import { InputPasswordComponent, InputPasswordFlow, @@ -94,38 +92,41 @@ export class ChangePasswordComponent implements OnInit { ); // New Master Password Policy Options service - const orgInvite = await this.organizationInviteService.getOrganizationInvite(); + // const orgInvite = await this.organizationInviteService.getOrganizationInvite(); - switch (this.forceSetPasswordReason) { - case ForceSetPasswordReason.WeakMasterPassword: - if (orgInvite) { - if (!orgInvite.token) { - this.logService.error("No org token found when trying to retrieve policies."); - return; - } - this.masterPasswordPolicyOptions = - await this.masterPasswordPolicyOptionsService.getForInvitedMember(orgInvite.token); - } else { - this.masterPasswordPolicyOptions = - await this.masterPasswordPolicyOptionsService.getByUserId(this.userId); - } - break; - case ForceSetPasswordReason.AdminForcePasswordReset: - default: - this.masterPasswordPolicyOptions = - await this.masterPasswordPolicyOptionsService.getByUserId(this.userId); - break; - } + // switch (this.forceSetPasswordReason) { + // case ForceSetPasswordReason.WeakMasterPassword: + // if (orgInvite) { + // if (!orgInvite.token) { + // this.logService.error("No org token found when trying to retrieve policies."); + // return; + // } + // // Jared I think you wanted this to fetch the token inside the function call but we have + // // it here and it could make sense to just pass it in? but it does go against the whole + // // self-sufficient notion of this service. + // this.masterPasswordPolicyOptions = + // await this.masterPasswordPolicyOptionsService.getForInvitedMember(orgInvite.token); + // } else { + // this.masterPasswordPolicyOptions = + // await this.masterPasswordPolicyOptionsService.getByUserId(this.userId); + // } + // break; + // case ForceSetPasswordReason.AdminForcePasswordReset: + // default: + // this.masterPasswordPolicyOptions = + // await this.masterPasswordPolicyOptionsService.getByUserId(this.userId); + // break; + // } if (this.forceSetPasswordReason === ForceSetPasswordReason.AdminForcePasswordReset) { this.anonLayoutWrapperDataService.setAnonLayoutWrapperData({ - pageIcon: LockIcon, + pageIcon: Icons.LockIcon, pageTitle: { key: "updateMasterPassword" }, pageSubtitle: { key: "accountRecoveryUpdateMasterPasswordSubtitle" }, }); } else if (this.forceSetPasswordReason === ForceSetPasswordReason.WeakMasterPassword) { this.anonLayoutWrapperDataService.setAnonLayoutWrapperData({ - pageIcon: LockIcon, + pageIcon: Icons.LockIcon, pageTitle: { key: "updateMasterPassword" }, pageSubtitle: { key: "updateMasterPasswordSubtitle" }, maxWidth: "lg", diff --git a/libs/common/src/admin-console/abstractions/policy/master-password-policy.service.abstraction.ts b/libs/common/src/admin-console/abstractions/policy/master-password-policy.service.abstraction.ts index ab58cd827d6..6e613858a11 100644 --- a/libs/common/src/admin-console/abstractions/policy/master-password-policy.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/policy/master-password-policy.service.abstraction.ts @@ -6,7 +6,7 @@ export abstract class MasterPasswordPolicyServiceAbstraction { * Used for a logged-in user is changing their password. Would probably be deprecated once * PolicyService is refactored. */ - abstract getByUserId: (loggedInUserId: UserId) => Promise; + abstract getByUserId: (userId: UserId) => Promise; /** * Used to evaluate compliance before accepting an invite. @@ -16,7 +16,7 @@ export abstract class MasterPasswordPolicyServiceAbstraction { /** * Used when resetting a grantor's password during an emergency access takeover. */ - abstract getForEmergencyAccessGrantor: (grantorUserId: UserId) => MasterPasswordPolicyOptions; + // abstract getForEmergencyAccessGrantor: (grantorUserId: UserId) => MasterPasswordPolicyOptions; /** * Used when resetting a member's password during an account recovery. Gets the master @@ -24,5 +24,5 @@ export abstract class MasterPasswordPolicyServiceAbstraction { * that an admin is keeping the user in compliance with the users whose password * they are setting and not using the policies that they are a part of. */ - abstract getForAccountRecoveryMember: (userId: UserId) => Promise; + // abstract getForAccountRecoveryMember: (userId: UserId) => Promise; } 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 7e37aceeee3..67c693d882d 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 @@ -1,5 +1,3 @@ -import { MasterPasswordPolicyResponse } from "@bitwarden/common/auth/models/response/master-password-policy.response"; - import { ListResponse } from "../../../models/response/list.response"; import { PolicyType } from "../../enums"; import { MasterPasswordPolicyOptions } from "../../models/domain/master-password-policy-options"; @@ -11,8 +9,6 @@ export abstract class PolicyApiServiceAbstraction { abstract getPolicy: (organizationId: string, type: PolicyType) => Promise; abstract getPolicies: (organizationId: string) => Promise>; - abstract getMasterPasswordPoliciesForAcceptedOrConfirmedUser: () => Promise; - abstract getPoliciesByToken: ( organizationId: string, token: string, @@ -20,8 +16,11 @@ export abstract class PolicyApiServiceAbstraction { organizationUserId: string, ) => Promise; + /** + * This takes an organization id to get org policies for. The user is inferred by the + */ abstract getMasterPasswordPolicyOptsForOrgUser: ( - orgId: string, + organizationId: string, ) => Promise; abstract putPolicy: ( organizationId: string, diff --git a/libs/common/src/admin-console/services/policy/default-master-password-policy.service.ts b/libs/common/src/admin-console/services/policy/default-master-password-policy.service.ts new file mode 100644 index 00000000000..acc6058b633 --- /dev/null +++ b/libs/common/src/admin-console/services/policy/default-master-password-policy.service.ts @@ -0,0 +1,36 @@ +import { + MasterPasswordPolicyServiceAbstraction +} from "@bitwarden/common/admin-console/abstractions/policy/master-password-policy.service.abstraction"; +import { + PolicyApiServiceAbstraction +} from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; +import { + MasterPasswordPolicyOptions +} from "@bitwarden/common/admin-console/models/domain/master-password-policy-options"; +import { UserId } from "@bitwarden/common/types/guid"; + + +export class DefaultMasterPasswordPolicyService implements MasterPasswordPolicyServiceAbstraction { + + constructor( + private readonly policyApiService: PolicyApiServiceAbstraction, + ) { + } + + getByUserId(userId: UserId): Promise { + // + // const result = await this.policyApiService.getMasterPasswordPolicyOptsForOrgUser(userId); + // + // if (!result) { + // throw new Error("No policy found for user id."); + // } + // + // return result; + return new Promise((resolve, reject) => resolve(new MasterPasswordPolicyOptions())); + } + + getForInvitedMember(inviteToken: string): Promise { + return new Promise((resolve, reject) => resolve(new MasterPasswordPolicyOptions())); + } + +} diff --git a/libs/components/src/dialog/dialog.service.ts b/libs/components/src/dialog/dialog.service.ts index 409bf0a5b55..2f2a1daf7cd 100644 --- a/libs/components/src/dialog/dialog.service.ts +++ b/libs/components/src/dialog/dialog.service.ts @@ -160,7 +160,7 @@ export class DialogService { * but we get the base CDK DialogRef instance *from* `Dialog.open`. * * To break the circle, we define CDKDialogRef as a wrapper for the CDKDialogRefBase. - * This allows us to create the class instance and provide the base instance later, almost like "deferred inheritance". + provide the base instance later, almost like "deferred inheritance". **/ const ref = new CdkDialogRef(); const injector = this.createInjector({