From 0c59454867a24d6b7d8056ef87c47c8d63a59ca6 Mon Sep 17 00:00:00 2001 From: Irfan Habib Date: Tue, 19 Jun 2018 13:18:57 +0100 Subject: [PATCH 1/5] Disable removal of `org user` role if user has others --- .../cf-org-permission-cell.component.ts | 9 ++--- .../list-types/cf-users/cf-permission-cell.ts | 33 +++++++++++------ .../cf-space-permission-cell.component.ts | 4 +-- .../shared/data-services/cf-user.service.ts | 35 +++++++++++++++++++ 4 files changed, 65 insertions(+), 16 deletions(-) diff --git a/src/frontend/app/shared/components/list/list-types/cf-users/cf-org-permission-cell/cf-org-permission-cell.component.ts b/src/frontend/app/shared/components/list/list-types/cf-users/cf-org-permission-cell/cf-org-permission-cell.component.ts index ed635b8f70..10f4072f4e 100644 --- a/src/frontend/app/shared/components/list/list-types/cf-users/cf-org-permission-cell/cf-org-permission-cell.component.ts +++ b/src/frontend/app/shared/components/list/list-types/cf-users/cf-org-permission-cell/cf-org-permission-cell.component.ts @@ -25,11 +25,11 @@ import { CfPermissionCell, ICellPermissionList } from '../cf-permission-cell'; export class CfOrgPermissionCellComponent extends CfPermissionCell { constructor( public store: Store, - public cfUserService: CfUserService, + cfUserService: CfUserService, private userPerms: CurrentUserPermissionsService, confirmDialog: ConfirmationDialogService ) { - super(confirmDialog); + super(confirmDialog, cfUserService); } protected setChipConfig(row: APIResource) { @@ -76,8 +76,9 @@ export class CfOrgPermissionCellComponent extends CfPermissionCell - this.userPerms.can(CurrentUserPermissions.ORGANIZATION_CHANGE_ROLES, cfGuid, orgGuid) + public canRemovePermission = (cfGuid: string, orgGuid: string, spaceGuid: string) => { + return this.userPerms.can(CurrentUserPermissions.ORGANIZATION_CHANGE_ROLES, cfGuid, orgGuid); + } } diff --git a/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts b/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts index 3171bbce16..0504a48bfb 100644 --- a/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts +++ b/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts @@ -1,14 +1,16 @@ import { Input } from '@angular/core'; -import { Observable, of as observableOf } from 'rxjs'; -import { map } from 'rxjs/operators'; +import { Observable, of as observableOf, BehaviorSubject } from 'rxjs'; +import { map, filter } from 'rxjs/operators'; import { IUserRole } from '../../../../../features/cloud-foundry/cf.helpers'; import { APIResource } from '../../../../../store/types/api.types'; -import { CfUser } from '../../../../../store/types/user.types'; +import { CfUser, UserRoleInOrg } from '../../../../../store/types/user.types'; import { AppChip } from '../../../chips/chips.component'; import { TableCellCustom } from '../../list.types'; import { ConfirmationDialogService } from '../../../confirmation-dialog.service'; import { ConfirmationDialogConfig } from '../../../confirmation-dialog.config'; +import { UserRoleLabels } from '../../../../../store/types/users-roles.types'; +import { CfUserService } from '../../../../data-services/cf-user.service'; export interface ICellPermissionList extends IUserRole { @@ -27,22 +29,25 @@ interface ICellPermissionUpdates { } export abstract class CfPermissionCell extends TableCellCustom> { + userEntity: BehaviorSubject = new BehaviorSubject(null); @Input('row') set row(row: APIResource) { this.setChipConfig(row); this.guid = row.metadata.guid; + this.userEntity.next(row.entity); } public chipsConfig: AppChip>[]; protected guid: string; - constructor(private confirmDialog: ConfirmationDialogService) { + constructor( + private confirmDialog: ConfirmationDialogService, + public cfUserService: CfUserService + ) { super(); } - protected setChipConfig(user: APIResource) { - - } + protected setChipConfig(user: APIResource) {} protected getChipConfig(cellPermissionList: ICellPermissionList[]) { return cellPermissionList.map(perm => { @@ -54,9 +59,17 @@ export abstract class CfPermissionCell extends TableCellCustom !can), - ); + if (perm.string === UserRoleLabels.org.short.users) { + // If there are other roles, disable clear button + chipConfig.hideClearButton$ = this.userEntity.pipe( + filter(p => !!p), + map( (entity: CfUser) => this.cfUserService.hasRoles(entity)) + ); + } else { + chipConfig.hideClearButton$ = this.canRemovePermission(perm.cfGuid, perm.orgGuid, perm.spaceGuid).pipe( + map(can => !can), + ); + } return chipConfig; }); } diff --git a/src/frontend/app/shared/components/list/list-types/cf-users/cf-space-permission-cell/cf-space-permission-cell.component.ts b/src/frontend/app/shared/components/list/list-types/cf-users/cf-space-permission-cell/cf-space-permission-cell.component.ts index 90c222f941..5065810e44 100644 --- a/src/frontend/app/shared/components/list/list-types/cf-users/cf-space-permission-cell/cf-space-permission-cell.component.ts +++ b/src/frontend/app/shared/components/list/list-types/cf-users/cf-space-permission-cell/cf-space-permission-cell.component.ts @@ -26,11 +26,11 @@ export class CfSpacePermissionCellComponent extends CfPermissionCell, - public cfUserService: CfUserService, + cfUserService: CfUserService, private userPerms: CurrentUserPermissionsService, confirmDialog: ConfirmationDialogService ) { - super(confirmDialog); + super(confirmDialog, cfUserService); } protected setChipConfig(row: APIResource) { diff --git a/src/frontend/app/shared/data-services/cf-user.service.ts b/src/frontend/app/shared/data-services/cf-user.service.ts index e8957051ca..a55632ba81 100644 --- a/src/frontend/app/shared/data-services/cf-user.service.ts +++ b/src/frontend/app/shared/data-services/cf-user.service.ts @@ -32,6 +32,8 @@ import { IUserPermissionInSpace, UserRoleInOrg, UserRoleInSpace, + OrgUserRoleNames, + SpaceUserRoleNames, } from '../../store/types/user.types'; import { PaginationMonitorFactory } from '../monitors/pagination-monitor.factory'; import { ActiveRouteCfOrgSpace } from './../../features/cloud-foundry/cf-page.types'; @@ -131,6 +133,39 @@ export class CfUserService { return res; } + // Helper to determine if user has roles other than Org User + hasRoles(user: CfUser, excludeOrgUser = true): boolean { + + const orgRoles = this.getOrgRolesFromUser(user); + const spaceRoles = this.getSpaceRolesFromUser(user); + + let hasRoles = false; + orgRoles.forEach(roles => { + const permissions = roles.permissions; + hasRoles = !!(hasRoles || + ( + permissions[OrgUserRoleNames.MANAGER] || + permissions[OrgUserRoleNames.BILLING_MANAGERS] || + permissions[OrgUserRoleNames.AUDITOR] + )); + + if (!excludeOrgUser) { + hasRoles = !!(hasRoles || permissions[OrgUserRoleNames.USER]); + } + }); + spaceRoles.forEach(roles => { + const permissions = roles.permissions; + hasRoles = !!(hasRoles || + ( + permissions[SpaceUserRoleNames.MANAGER] || + permissions[SpaceUserRoleNames.AUDITOR] || + permissions[SpaceUserRoleNames.DEVELOPER] + + )); + }); + return hasRoles; + } + getUserRoleInOrg = ( userGuid: string, orgGuid: string, From f02076cd0af8a98e3ad11af7e1bd7d07a26d748d Mon Sep 17 00:00:00 2001 From: Irfan Habib Date: Tue, 19 Jun 2018 14:04:44 +0100 Subject: [PATCH 2/5] minor tweaks --- .../cf-org-permission-cell.component.ts | 6 ++---- .../list/list-types/cf-users/cf-permission-cell.ts | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/frontend/app/shared/components/list/list-types/cf-users/cf-org-permission-cell/cf-org-permission-cell.component.ts b/src/frontend/app/shared/components/list/list-types/cf-users/cf-org-permission-cell/cf-org-permission-cell.component.ts index 10f4072f4e..223ba7f026 100644 --- a/src/frontend/app/shared/components/list/list-types/cf-users/cf-org-permission-cell/cf-org-permission-cell.component.ts +++ b/src/frontend/app/shared/components/list/list-types/cf-users/cf-org-permission-cell/cf-org-permission-cell.component.ts @@ -76,9 +76,7 @@ export class CfOrgPermissionCellComponent extends CfPermissionCell { - return this.userPerms.can(CurrentUserPermissions.ORGANIZATION_CHANGE_ROLES, cfGuid, orgGuid); - } - + public canRemovePermission = (cfGuid: string, orgGuid: string, spaceGuid: string) => + this.userPerms.can(CurrentUserPermissions.ORGANIZATION_CHANGE_ROLES, cfGuid, orgGuid) } diff --git a/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts b/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts index 0504a48bfb..f32c43e2f3 100644 --- a/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts +++ b/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts @@ -60,7 +60,7 @@ export abstract class CfPermissionCell extends TableCellCustom !!p), map( (entity: CfUser) => this.cfUserService.hasRoles(entity)) From 2a307402e6279926d25410d8c91815e3934f8ca1 Mon Sep 17 00:00:00 2001 From: Irfan Habib Date: Wed, 20 Jun 2018 11:39:02 +0100 Subject: [PATCH 3/5] adding back change --- .../components/list/list-types/cf-users/cf-permission-cell.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts b/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts index 491c661e9d..8214e2667e 100644 --- a/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts +++ b/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts @@ -11,6 +11,7 @@ import { AppChip } from '../../../chips/chips.component'; import { ConfirmationDialogConfig } from '../../../confirmation-dialog.config'; import { ConfirmationDialogService } from '../../../confirmation-dialog.service'; import { TableCellCustom } from '../../list.types'; +import { IOrganization } from '../../../../../core/cf-api.types'; export interface ICellPermissionList extends IUserRole { @@ -56,8 +57,6 @@ export abstract class CfPermissionCell extends TableCellCustom) { } - protected getChipConfig(cellPermissionList: ICellPermissionList[]) { return cellPermissionList.map(perm => { const chipConfig = new AppChip>(); From 8d7d8aa115badaaacc21921ed4cdb39521b26056 Mon Sep 17 00:00:00 2001 From: Irfan Habib Date: Wed, 20 Jun 2018 12:00:20 +0100 Subject: [PATCH 4/5] Address comments --- .../list-types/cf-users/cf-permission-cell.ts | 29 ++++++----- .../shared/data-services/cf-user.service.ts | 51 ++++++++++++------- 2 files changed, 50 insertions(+), 30 deletions(-) diff --git a/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts b/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts index 8214e2667e..91aab59dd9 100644 --- a/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts +++ b/src/frontend/app/shared/components/list/list-types/cf-users/cf-permission-cell.ts @@ -1,6 +1,6 @@ import { Input } from '@angular/core'; import { BehaviorSubject, Observable, of as observableOf } from 'rxjs'; -import { filter, map } from 'rxjs/operators'; +import { filter, map, switchMap } from 'rxjs/operators'; import { IUserRole } from '../../../../../features/cloud-foundry/cf.helpers'; import { APIResource } from '../../../../../store/types/api.types'; @@ -11,7 +11,6 @@ import { AppChip } from '../../../chips/chips.component'; import { ConfirmationDialogConfig } from '../../../confirmation-dialog.config'; import { ConfirmationDialogService } from '../../../confirmation-dialog.service'; import { TableCellCustom } from '../../list.types'; -import { IOrganization } from '../../../../../core/cf-api.types'; export interface ICellPermissionList extends IUserRole { @@ -67,17 +66,21 @@ export abstract class CfPermissionCell extends TableCellCustom !!p), - map((entity: CfUser) => this.cfUserService.hasRoles(entity)) - ); - } else { - chipConfig.hideClearButton$ = this.canRemovePermission(perm.cfGuid, perm.orgGuid, perm.spaceGuid).pipe( - map(can => !can), - ); - } + chipConfig.hideClearButton$ = this.canRemovePermission(perm.cfGuid, perm.orgGuid, perm.spaceGuid).pipe( + map(can => !can), + switchMap(can => { + if (!can) { + if (perm.string === UserRoleLabels.org.short.users) { + // If there are other roles than Org User, disable clear button + return this.userEntity.pipe( + filter(p => !!p), + map((entity: CfUser) => this.cfUserService.hasRolesInOrg(entity, perm.orgGuid)), + ); + } + } + return observableOf(can); + }) + ); return chipConfig; }); } diff --git a/src/frontend/app/shared/data-services/cf-user.service.ts b/src/frontend/app/shared/data-services/cf-user.service.ts index a0c73269ce..1b71e94165 100644 --- a/src/frontend/app/shared/data-services/cf-user.service.ts +++ b/src/frontend/app/shared/data-services/cf-user.service.ts @@ -160,35 +160,52 @@ export class CfUserService { } // Helper to determine if user has roles other than Org User - hasRoles(user: CfUser, excludeOrgUser = true): boolean { + hasRolesInOrg(user: CfUser, orgGuid: string, excludeOrgUser = true): boolean { - const orgRoles = this.getOrgRolesFromUser(user); - const spaceRoles = this.getSpaceRolesFromUser(user); + const orgRoles = this.getOrgRolesFromUser(user).filter(o => o.orgGuid === orgGuid); + const spaceRoles = this.getSpaceRolesFromUser(user).filter(o => o.orgGuid === orgGuid); let hasRoles = false; - orgRoles.forEach(roles => { + for (const roleKey in orgRoles) { + if (!orgRoles.hasOwnProperty(roleKey)) { + continue; + } + const roles = orgRoles[roleKey]; const permissions = roles.permissions; hasRoles = !!(hasRoles || - ( - permissions[OrgUserRoleNames.MANAGER] || - permissions[OrgUserRoleNames.BILLING_MANAGERS] || - permissions[OrgUserRoleNames.AUDITOR] - )); + ( + permissions[OrgUserRoleNames.MANAGER] || + permissions[OrgUserRoleNames.BILLING_MANAGERS] || + permissions[OrgUserRoleNames.AUDITOR] + )); if (!excludeOrgUser) { hasRoles = !!(hasRoles || permissions[OrgUserRoleNames.USER]); } - }); - spaceRoles.forEach(roles => { + if (hasRoles) { + break; + } + } + + for (const roleKey in spaceRoles) { + if (!orgRoles.hasOwnProperty(roleKey)) { + continue; + } + const roles = spaceRoles[roleKey]; const permissions = roles.permissions; hasRoles = !!(hasRoles || - ( - permissions[SpaceUserRoleNames.MANAGER] || - permissions[SpaceUserRoleNames.AUDITOR] || - permissions[SpaceUserRoleNames.DEVELOPER] + ( + permissions[SpaceUserRoleNames.MANAGER] || + permissions[SpaceUserRoleNames.AUDITOR] || + permissions[SpaceUserRoleNames.DEVELOPER] + + )); + if (hasRoles) { + break; + } + } + - )); - }); return hasRoles; } From f1d922001768d20d5adf49d288ce40893bbbdef4 Mon Sep 17 00:00:00 2001 From: Richard Cox Date: Wed, 20 Jun 2018 16:10:55 +0100 Subject: [PATCH 5/5] Fixed c+p orgRoles.hasOwnProperty, tweaked return behaviour --- .../shared/data-services/cf-user.service.ts | 50 ++++++++----------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/src/frontend/app/shared/data-services/cf-user.service.ts b/src/frontend/app/shared/data-services/cf-user.service.ts index 1b71e94165..16880f2bc7 100644 --- a/src/frontend/app/shared/data-services/cf-user.service.ts +++ b/src/frontend/app/shared/data-services/cf-user.service.ts @@ -30,10 +30,10 @@ import { createUserRoleInSpace, IUserPermissionInOrg, IUserPermissionInSpace, - UserRoleInOrg, - UserRoleInSpace, OrgUserRoleNames, SpaceUserRoleNames, + UserRoleInOrg, + UserRoleInSpace, } from '../../store/types/user.types'; import { PaginationMonitorFactory } from '../monitors/pagination-monitor.factory'; import { ActiveRouteCfOrgSpace } from './../../features/cloud-foundry/cf-page.types'; @@ -159,54 +159,44 @@ export class CfUserService { return res; } - // Helper to determine if user has roles other than Org User + /** + * Helper to determine if user has roles other than Org User + */ hasRolesInOrg(user: CfUser, orgGuid: string, excludeOrgUser = true): boolean { const orgRoles = this.getOrgRolesFromUser(user).filter(o => o.orgGuid === orgGuid); const spaceRoles = this.getSpaceRolesFromUser(user).filter(o => o.orgGuid === orgGuid); - let hasRoles = false; for (const roleKey in orgRoles) { if (!orgRoles.hasOwnProperty(roleKey)) { continue; } - const roles = orgRoles[roleKey]; - const permissions = roles.permissions; - hasRoles = !!(hasRoles || - ( - permissions[OrgUserRoleNames.MANAGER] || - permissions[OrgUserRoleNames.BILLING_MANAGERS] || - permissions[OrgUserRoleNames.AUDITOR] - )); - if (!excludeOrgUser) { - hasRoles = !!(hasRoles || permissions[OrgUserRoleNames.USER]); + const permissions = orgRoles[roleKey].permissions; + if ( + permissions[OrgUserRoleNames.MANAGER] || + permissions[OrgUserRoleNames.BILLING_MANAGERS] || + permissions[OrgUserRoleNames.AUDITOR] + ) { + return true; } - if (hasRoles) { - break; + if (!excludeOrgUser && permissions[OrgUserRoleNames.USER]) { + return true; } } for (const roleKey in spaceRoles) { - if (!orgRoles.hasOwnProperty(roleKey)) { + if (!spaceRoles.hasOwnProperty(roleKey)) { continue; } - const roles = spaceRoles[roleKey]; - const permissions = roles.permissions; - hasRoles = !!(hasRoles || - ( - permissions[SpaceUserRoleNames.MANAGER] || - permissions[SpaceUserRoleNames.AUDITOR] || - permissions[SpaceUserRoleNames.DEVELOPER] - )); - if (hasRoles) { - break; + const permissions = spaceRoles[roleKey].permissions; + if (permissions[SpaceUserRoleNames.MANAGER] || + permissions[SpaceUserRoleNames.AUDITOR] || + permissions[SpaceUserRoleNames.DEVELOPER]) { + return true; } } - - - return hasRoles; } getUserRoleInOrg = (