From d3d17f14961445d91be9317283ac525ad48ef08a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Fri, 16 Jun 2023 16:38:55 +0100 Subject: [PATCH] [AC-1144] Warn admins when removing or revoking users without master password (#5494) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [AC-1144] Added new messages for warning removing/revoking user without master password * [AC-1144] Added property 'hasMasterPassword' to OrganizationUserUserDetailsResponse and OrganizationUserView * [AC-1144] Added user's name to 'No master password' warning * [AC-1144] Added property 'hasMasterPassword' to ProviderUserResponse * [AC-1144] Added alert to bulk "remove/revoke users" action when a selected user has no master password * [AC-1144] Moved 'noMasterPasswordConfirmationDialog' method to BasePeopleComponent * [AC-1144] Removed await from noMasterPasswordConfirmationDialog * [AC-1144] Changed ApiService.getProviderUser to output ProviderUserUserDetailsResponse * [AC-1144] Added warning on removing a provider user without master password * [AC-1144] Added "No Master password" warning to provider users * [AC-1144] Added "no master password" warning when removing/revoking user in modal view * [AC-1144] Reverted changes made to ProviderUsers * [AC-1144] Converted showNoMasterPasswordWarning() into a property * [AC-1144] Fixed issue when opening invite member modal --- .../core/services/user-admin.service.ts | 1 + .../views/organization-user-admin-view.ts | 1 + .../core/views/organization-user.view.ts | 1 + .../bulk/bulk-remove.component.html | 15 ++++++- .../components/bulk/bulk-remove.component.ts | 12 ++++- .../bulk/bulk-restore-revoke.component.html | 15 ++++++- .../bulk/bulk-restore-revoke.component.ts | 2 + .../components/bulk/bulk-status.component.ts | 1 + .../member-dialog/member-dialog.component.ts | 37 +++++++++++++++- .../organizations/members/people.component.ts | 44 ++++++++++++++++++- .../src/app/common/base.people.component.ts | 8 +++- apps/web/src/locales/en/messages.json | 18 ++++++++ .../responses/organization-user.response.ts | 2 + 13 files changed, 149 insertions(+), 8 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/core/services/user-admin.service.ts b/apps/web/src/app/admin-console/organizations/core/services/user-admin.service.ts index 2d7635c5c6..4e83d76f7d 100644 --- a/apps/web/src/app/admin-console/organizations/core/services/user-admin.service.ts +++ b/apps/web/src/app/admin-console/organizations/core/services/user-admin.service.ts @@ -83,6 +83,7 @@ export class UserAdminService { })); view.groups = u.groups; view.accessSecretsManager = u.accessSecretsManager; + view.hasMasterPassword = u.hasMasterPassword; return view; }); diff --git a/apps/web/src/app/admin-console/organizations/core/views/organization-user-admin-view.ts b/apps/web/src/app/admin-console/organizations/core/views/organization-user-admin-view.ts index 11bf026967..3bcb4767a8 100644 --- a/apps/web/src/app/admin-console/organizations/core/views/organization-user-admin-view.ts +++ b/apps/web/src/app/admin-console/organizations/core/views/organization-user-admin-view.ts @@ -16,6 +16,7 @@ export class OrganizationUserAdminView { accessAll: boolean; permissions: PermissionsApi; resetPasswordEnrolled: boolean; + hasMasterPassword: boolean; collections: CollectionAccessSelectionView[] = []; groups: string[] = []; diff --git a/apps/web/src/app/admin-console/organizations/core/views/organization-user.view.ts b/apps/web/src/app/admin-console/organizations/core/views/organization-user.view.ts index 16b3a60150..f723fcc32e 100644 --- a/apps/web/src/app/admin-console/organizations/core/views/organization-user.view.ts +++ b/apps/web/src/app/admin-console/organizations/core/views/organization-user.view.ts @@ -20,6 +20,7 @@ export class OrganizationUserView { avatarColor: string; twoFactorEnabled: boolean; usesKeyConnector: boolean; + hasMasterPassword: boolean; collections: CollectionAccessSelectionView[] = []; groups: string[] = []; diff --git a/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-remove.component.html b/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-remove.component.html index 65230f6c05..d2adef98ee 100644 --- a/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-remove.component.html +++ b/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-remove.component.html @@ -23,12 +23,16 @@ - {{ removeUsersWarning }} +

{{ removeUsersWarning }}

+

+ {{ "removeMembersWithoutMasterPasswordWarning" | i18n }} +

+ @@ -39,6 +43,15 @@ {{ user.email }} {{ user.name }} +
{{ "user" | i18n }}{{ "details" | i18n }}
+ + - + + + {{ "noMasterPassword" | i18n }} + + +
diff --git a/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-remove.component.ts b/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-remove.component.ts index d083d0cd1e..665d245e9f 100644 --- a/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-remove.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-remove.component.ts @@ -12,13 +12,23 @@ import { BulkUserDetails } from "./bulk-status.component"; }) export class BulkRemoveComponent { @Input() organizationId: string; - @Input() users: BulkUserDetails[]; + @Input() set users(value: BulkUserDetails[]) { + this._users = value; + this.showNoMasterPasswordWarning = this._users.some((u) => u.hasMasterPassword === false); + } + + get users(): BulkUserDetails[] { + return this._users; + } + + private _users: BulkUserDetails[]; statuses: Map = new Map(); loading = false; done = false; error: string; + showNoMasterPasswordWarning = false; constructor( protected apiService: ApiService, diff --git a/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-restore-revoke.component.html b/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-restore-revoke.component.html index 9d29121037..db8af13ed2 100644 --- a/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-restore-revoke.component.html +++ b/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-restore-revoke.component.html @@ -23,12 +23,16 @@ - {{ "revokeUsersWarning" | i18n }} +

{{ "revokeUsersWarning" | i18n }}

+

+ {{ "removeMembersWithoutMasterPasswordWarning" | i18n }} +

+ @@ -39,6 +43,15 @@ {{ user.email }} {{ user.name }} +
{{ "user" | i18n }}{{ "details" | i18n }}
+ + - + + + {{ "noMasterPassword" | i18n }} + + +
diff --git a/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-restore-revoke.component.ts b/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-restore-revoke.component.ts index 82f3a4d7e3..8005efcb8d 100644 --- a/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-restore-revoke.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-restore-revoke.component.ts @@ -20,6 +20,7 @@ export class BulkRestoreRevokeComponent { loading = false; done = false; error: string; + showNoMasterPasswordWarning = false; constructor( protected i18nService: I18nService, @@ -29,6 +30,7 @@ export class BulkRestoreRevokeComponent { this.isRevoking = config.data.isRevoking; this.organizationId = config.data.organizationId; this.users = config.data.users; + this.showNoMasterPasswordWarning = this.users.some((u) => u.hasMasterPassword === false); } get bulkTitle() { diff --git a/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-status.component.ts b/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-status.component.ts index f2920c398b..ff0a0903df 100644 --- a/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-status.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-status.component.ts @@ -10,6 +10,7 @@ export interface BulkUserDetails { name: string; email: string; status: OrganizationUserStatusType | ProviderUserStatusType; + hasMasterPassword?: boolean; } type BulkStatusEntry = { diff --git a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts index 5a707b795f..91d0fc97e6 100644 --- a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts @@ -72,6 +72,7 @@ export class MemberDialogComponent implements OnInit, OnDestroy { canUseCustomPermissions: boolean; PermissionMode = PermissionMode; canUseSecretsManager: boolean; + showNoMasterPasswordWarning = false; protected organization: Organization; protected collectionAccessItems: AccessItemView[] = []; @@ -179,6 +180,9 @@ export class MemberDialogComponent implements OnInit, OnDestroy { throw new Error("Could not find user to edit."); } this.isRevoked = userDetails.status === OrganizationUserStatusType.Revoked; + this.showNoMasterPasswordWarning = + userDetails.status > OrganizationUserStatusType.Invited && + userDetails.hasMasterPassword === false; const assignedCollectionsPermissions = { editAssignedCollections: userDetails.permissions.editAssignedCollections, deleteAssignedCollections: userDetails.permissions.deleteAssignedCollections, @@ -366,7 +370,7 @@ export class MemberDialogComponent implements OnInit, OnDestroy { ? "removeUserConfirmationKeyConnector" : "removeOrgUserConfirmation"; - const confirmed = await this.dialogService.openSimpleDialog({ + let confirmed = await this.dialogService.openSimpleDialog({ title: { key: "removeUserIdAccess", placeholders: [this.params.name] }, content: { key: message }, type: SimpleDialogType.WARNING, @@ -376,6 +380,14 @@ export class MemberDialogComponent implements OnInit, OnDestroy { return false; } + if (this.showNoMasterPasswordWarning) { + confirmed = await this.noMasterPasswordConfirmationDialog(); + + if (!confirmed) { + return false; + } + } + await this.organizationUserService.deleteOrganizationUser( this.params.organizationId, this.params.organizationUserId @@ -394,7 +406,7 @@ export class MemberDialogComponent implements OnInit, OnDestroy { return; } - const confirmed = await this.dialogService.openSimpleDialog({ + let confirmed = await this.dialogService.openSimpleDialog({ title: { key: "revokeUserId", placeholders: [this.params.name] }, content: { key: "revokeUserConfirmation" }, acceptButtonText: { key: "revokeAccess" }, @@ -405,6 +417,14 @@ export class MemberDialogComponent implements OnInit, OnDestroy { return false; } + if (this.showNoMasterPasswordWarning) { + confirmed = await this.noMasterPasswordConfirmationDialog(); + + if (!confirmed) { + return false; + } + } + await this.organizationUserService.revokeOrganizationUser( this.params.organizationId, this.params.organizationUserId @@ -450,6 +470,19 @@ export class MemberDialogComponent implements OnInit, OnDestroy { private close(result: MemberDialogResult) { this.dialogRef.close(result); } + + private noMasterPasswordConfirmationDialog() { + return this.dialogService.openSimpleDialog({ + title: { + key: "removeOrgUserNoMasterPasswordTitle", + }, + content: { + key: "removeOrgUserNoMasterPasswordDesc", + placeholders: [this.params.name], + }, + type: SimpleDialogType.WARNING, + }); + } } function mapCollectionToAccessItemView( diff --git a/apps/web/src/app/admin-console/organizations/members/people.component.ts b/apps/web/src/app/admin-console/organizations/members/people.component.ts index 64b7f2c20c..58a02274f8 100644 --- a/apps/web/src/app/admin-console/organizations/members/people.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/people.component.ts @@ -546,7 +546,7 @@ export class PeopleComponent ? "removeUserConfirmationKeyConnector" : "removeOrgUserConfirmation"; - return await this.dialogService.openSimpleDialog({ + const confirmed = await this.dialogService.openSimpleDialog({ title: { key: "removeUserIdAccess", placeholders: [this.userNamePipe.transform(user)], @@ -554,6 +554,35 @@ export class PeopleComponent content: { key: content }, type: SimpleDialogType.WARNING, }); + + if (!confirmed) { + return false; + } + + if (user.status > OrganizationUserStatusType.Invited && user.hasMasterPassword === false) { + return await this.noMasterPasswordConfirmationDialog(user); + } + + return true; + } + + protected async revokeUserConfirmationDialog(user: OrganizationUserView) { + const confirmed = await this.dialogService.openSimpleDialog({ + title: { key: "revokeAccess", placeholders: [this.userNamePipe.transform(user)] }, + content: this.revokeWarningMessage(), + acceptButtonText: { key: "revokeAccess" }, + type: SimpleDialogType.WARNING, + }); + + if (!confirmed) { + return false; + } + + if (user.status > OrganizationUserStatusType.Invited && user.hasMasterPassword === false) { + return await this.noMasterPasswordConfirmationDialog(user); + } + + return true; } private async showBulkStatus( @@ -608,4 +637,17 @@ export class PeopleComponent modal.close(); } } + + private async noMasterPasswordConfirmationDialog(user: OrganizationUserView) { + return this.dialogService.openSimpleDialog({ + title: { + key: "removeOrgUserNoMasterPasswordTitle", + }, + content: { + key: "removeOrgUserNoMasterPasswordDesc", + placeholders: [this.userNamePipe.transform(user)], + }, + type: SimpleDialogType.WARNING, + }); + } } diff --git a/apps/web/src/app/common/base.people.component.ts b/apps/web/src/app/common/base.people.component.ts index 200e0e762b..d6044bc9ef 100644 --- a/apps/web/src/app/common/base.people.component.ts +++ b/apps/web/src/app/common/base.people.component.ts @@ -247,13 +247,17 @@ export abstract class BasePeopleComponent< this.actionPromise = null; } - async revoke(user: UserType) { - const confirmed = await this.dialogService.openSimpleDialog({ + protected async revokeUserConfirmationDialog(user: UserType) { + return this.dialogService.openSimpleDialog({ title: { key: "revokeAccess", placeholders: [this.userNamePipe.transform(user)] }, content: this.revokeWarningMessage(), acceptButtonText: { key: "revokeAccess" }, type: SimpleDialogType.WARNING, }); + } + + async revoke(user: UserType) { + const confirmed = await this.revokeUserConfirmationDialog(user); if (!confirmed) { return false; diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 4e08fa65ba..f80bccdf60 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -6884,5 +6884,23 @@ }, "loginRequestApproved": { "message": "Login request approved" + }, + "removeOrgUserNoMasterPasswordTitle": { + "message": "Account does not have master password" + }, + "removeOrgUserNoMasterPasswordDesc": { + "message": "Removing $USER$ without setting a master password for them may restrict access to their full account. Are you sure you want to continue?", + "placeholders": { + "user": { + "content": "$1", + "example": "John Smith" + } + } + }, + "noMasterPassword": { + "message": "No master password" + }, + "removeMembersWithoutMasterPasswordWarning": { + "message": "Removing members who do not have master passwords without setting one for them may restrict access to their full account." } } diff --git a/libs/common/src/abstractions/organization-user/responses/organization-user.response.ts b/libs/common/src/abstractions/organization-user/responses/organization-user.response.ts index b5af4253c5..973049968a 100644 --- a/libs/common/src/abstractions/organization-user/responses/organization-user.response.ts +++ b/libs/common/src/abstractions/organization-user/responses/organization-user.response.ts @@ -14,6 +14,7 @@ export class OrganizationUserResponse extends BaseResponse { accessSecretsManager: boolean; permissions: PermissionsApi; resetPasswordEnrolled: boolean; + hasMasterPassword: boolean; collections: SelectionReadOnlyResponse[] = []; groups: string[] = []; @@ -28,6 +29,7 @@ export class OrganizationUserResponse extends BaseResponse { this.accessAll = this.getResponseProperty("AccessAll"); this.accessSecretsManager = this.getResponseProperty("AccessSecretsManager"); this.resetPasswordEnrolled = this.getResponseProperty("ResetPasswordEnrolled"); + this.hasMasterPassword = this.getResponseProperty("HasMasterPassword"); const collections = this.getResponseProperty("Collections"); if (collections != null) {