From 619e9b3382f1bea6314b50193158dcf541bd1acc Mon Sep 17 00:00:00 2001 From: Richard Cox Date: Thu, 4 Jun 2020 13:30:00 +0100 Subject: [PATCH] Change following review #1 --- .../cf-services-list-config.service.ts | 5 +- .../cf-org-space-service.service.ts | 5 +- .../src/store/cloud-foundry.store.module.ts | 2 - .../src/store/effects/roles.effects.ts | 28 -------- .../src/store/effects/users-roles.effects.ts | 14 ++++ .../store/selectors/cloud-foundry.selector.ts | 4 -- .../cf-user-permissions-checkers.ts | 4 +- .../user-permissions/cf-user-roles-fetch.ts | 5 +- .../current-user-permissions.service.ts | 4 +- .../current-user-permissions.types.ts | 61 ++++++++++++++++ .../stratos-user-permissions.checker.ts | 72 ++----------------- .../current-user-roles.reducer.ts | 4 +- .../src/types/current-user-roles.types.ts | 5 +- 13 files changed, 99 insertions(+), 114 deletions(-) delete mode 100644 src/frontend/packages/cloud-foundry/src/store/effects/roles.effects.ts delete mode 100644 src/frontend/packages/cloud-foundry/src/store/selectors/cloud-foundry.selector.ts create mode 100644 src/frontend/packages/core/src/core/permissions/current-user-permissions.types.ts diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-services/cf-services-list-config.service.ts b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-services/cf-services-list-config.service.ts index 954c2562d0..90fdcad893 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-services/cf-services-list-config.service.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/cf-services/cf-services-list-config.service.ts @@ -11,10 +11,11 @@ import { ListViewTypes, } from '../../../../../../../core/src/shared/components/list/list.component.types'; import { ListView } from '../../../../../../../store/src/actions/list.actions'; +import { connectedEndpointsOfTypesSelector } from '../../../../../../../store/src/selectors/endpoint.selectors'; import { APIResource } from '../../../../../../../store/src/types/api.types'; +import { CF_ENDPOINT_TYPE } from '../../../../../cf-types'; import { ActiveRouteCfOrgSpace } from '../../../../../features/cloud-foundry/cf-page.types'; import { haveMultiConnectedCfs } from '../../../../../features/cloud-foundry/cf.helpers'; -import { endpointsCfEntitiesConnectedSelector } from '../../../../../store/selectors/cloud-foundry.selector'; import { CfOrgSpaceItem, createCfOrgSpaceFilterConfig } from '../../../../data-services/cf-org-space-service.service'; import { CfServiceCardComponent } from './cf-service-card/cf-service-card.component'; import { CfServicesDataSource } from './cf-services-data-source'; @@ -38,7 +39,7 @@ export class CfServicesListConfigService implements IListConfig { ) { this.dataSource = new CfServicesDataSource(this.store, activeRouteCfOrgSpace.cfGuid, this); this.cf = { - list$: this.store.select(endpointsCfEntitiesConnectedSelector).pipe( + list$: this.store.select(connectedEndpointsOfTypesSelector(CF_ENDPOINT_TYPE)).pipe( first(), map(endpoints => Object.values(endpoints)) ), diff --git a/src/frontend/packages/cloud-foundry/src/shared/data-services/cf-org-space-service.service.ts b/src/frontend/packages/cloud-foundry/src/shared/data-services/cf-org-space-service.service.ts index 8a8951d87c..e8ce2c3184 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/data-services/cf-org-space-service.service.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/data-services/cf-org-space-service.service.ts @@ -27,6 +27,7 @@ import { ResetPagination, SetParams } from '../../../../store/src/actions/pagina import { PaginationMonitorFactory } from '../../../../store/src/monitors/pagination-monitor.factory'; import { getPaginationObservables } from '../../../../store/src/reducers/pagination-reducer/pagination-reducer.helper'; import { getCurrentPageRequestInfo } from '../../../../store/src/reducers/pagination-reducer/pagination-reducer.types'; +import { connectedEndpointsOfTypesSelector } from '../../../../store/src/selectors/endpoint.selectors'; import { selectPaginationState } from '../../../../store/src/selectors/pagination.selectors'; import { APIResource } from '../../../../store/src/types/api.types'; import { EndpointModel } from '../../../../store/src/types/endpoint.types'; @@ -34,7 +35,7 @@ import { PaginatedAction, PaginationParam } from '../../../../store/src/types/pa import { IOrganization, ISpace } from '../../cf-api.types'; import { cfEntityCatalog } from '../../cf-entity-catalog'; import { cfEntityFactory } from '../../cf-entity-factory'; -import { endpointsCfEntitiesConnectedSelector } from '../../store/selectors/cloud-foundry.selector'; +import { CF_ENDPOINT_TYPE } from '../../cf-types'; import { QParam, QParamJoiners } from '../q-param'; export function spreadPaginationParams(params: PaginationParam): PaginationParam { @@ -221,7 +222,7 @@ export class CfOrgSpaceDataService implements OnDestroy { } private createCf() { - const list$ = this.store.select(endpointsCfEntitiesConnectedSelector).pipe( + const list$ = this.store.select(connectedEndpointsOfTypesSelector(CF_ENDPOINT_TYPE)).pipe( // Ensure we have endpoints filter(endpoints => endpoints && !!Object.keys(endpoints).length), publishReplay(1), diff --git a/src/frontend/packages/cloud-foundry/src/store/cloud-foundry.store.module.ts b/src/frontend/packages/cloud-foundry/src/store/cloud-foundry.store.module.ts index 45ddef3acb..d209acc642 100644 --- a/src/frontend/packages/cloud-foundry/src/store/cloud-foundry.store.module.ts +++ b/src/frontend/packages/cloud-foundry/src/store/cloud-foundry.store.module.ts @@ -10,7 +10,6 @@ import { CreateAppPageEffects } from './effects/create-app-effects'; import { DeployAppEffects } from './effects/deploy-app.effects'; import { GithubEffects } from './effects/github.effects'; import { CfValidateEffects } from './effects/request.effects'; -import { PermissionEffects } from './effects/roles.effects'; import { RouteEffect } from './effects/route.effects'; import { ServiceInstanceEffects } from './effects/service-instance.effects'; import { UpdateAppEffects } from './effects/update-app-effects'; @@ -26,7 +25,6 @@ import { UsersRolesEffects } from './effects/users-roles.effects'; GithubEffects, CloudFoundryEffects, RouteEffect, - PermissionEffects, ServiceInstanceEffects, AppEffects, UpdateAppEffects, diff --git a/src/frontend/packages/cloud-foundry/src/store/effects/roles.effects.ts b/src/frontend/packages/cloud-foundry/src/store/effects/roles.effects.ts deleted file mode 100644 index 9181245b63..0000000000 --- a/src/frontend/packages/cloud-foundry/src/store/effects/roles.effects.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { HttpClient } from '@angular/common/http'; -import { Injectable } from '@angular/core'; -import { Actions, Effect, ofType } from '@ngrx/effects'; -import { Store } from '@ngrx/store'; -import { catchError, map } from 'rxjs/operators'; - -import { GET_CURRENT_CF_USER_RELATION, GetCurrentCfUserRelations } from '../../actions/permissions.actions'; -import { CFAppState } from '../../cf-app-state'; -import { fetchCfUserRole } from '../../user-permissions/cf-user-roles-fetch'; - -@Injectable() -export class PermissionEffects { - constructor( - private httpClient: HttpClient, - private actions$: Actions, - private store: Store - ) { } - - @Effect() getCurrentUsersPermissions$ = this.actions$.pipe( - ofType(GET_CURRENT_CF_USER_RELATION), - map(action => { - return fetchCfUserRole(this.store, action, this.httpClient).pipe( - map(() => ({ type: action.actions[1] })), - catchError(() => [{ type: action.actions[2] }]) - ); - }) - ); -} diff --git a/src/frontend/packages/cloud-foundry/src/store/effects/users-roles.effects.ts b/src/frontend/packages/cloud-foundry/src/store/effects/users-roles.effects.ts index 8b84913970..7ed0d48c09 100644 --- a/src/frontend/packages/cloud-foundry/src/store/effects/users-roles.effects.ts +++ b/src/frontend/packages/cloud-foundry/src/store/effects/users-roles.effects.ts @@ -1,3 +1,4 @@ +import { HttpClient } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { Actions, Effect, ofType } from '@ngrx/effects'; import { Store } from '@ngrx/store'; @@ -14,12 +15,14 @@ import { selectSessionData } from '../../../../store/src/reducers/auth.reducer'; import { SessionDataEndpoint } from '../../../../store/src/types/auth.types'; import { PaginatedAction } from '../../../../store/src/types/pagination.types'; import { ICFAction, UpdateCfAction } from '../../../../store/src/types/request.types'; +import { GET_CURRENT_CF_USER_RELATION, GetCurrentCfUserRelations } from '../../actions/permissions.actions'; import { UsersRolesActions, UsersRolesClearUpdateState, UsersRolesExecuteChanges } from '../../actions/users-roles.actions'; import { AddCfUserRole, ChangeCfUserRole, RemoveCfUserRole } from '../../actions/users.actions'; import { CFAppState } from '../../cf-app-state'; import { organizationEntityType, spaceEntityType } from '../../cf-entity-types'; import { CF_ENDPOINT_TYPE } from '../../cf-types'; import { CfUserService } from '../../shared/data-services/cf-user.service'; +import { fetchCfUserRole } from '../../user-permissions/cf-user-roles-fetch'; import { selectCfUsersRoles } from '../selectors/cf-users-roles.selector'; import { OrgUserRoleNames } from '../types/cf-user.types'; import { CfRoleChange, UsersRolesState } from '../types/users-roles.types'; @@ -29,11 +32,22 @@ import { CfRoleChange, UsersRolesState } from '../types/users-roles.types'; export class UsersRolesEffects { constructor( + private httpClient: HttpClient, private actions$: Actions, private store: Store, private cfUserService: CfUserService, ) { } + @Effect() getCurrentUsersPermissions$ = this.actions$.pipe( + ofType(GET_CURRENT_CF_USER_RELATION), + map(action => { + return fetchCfUserRole(this.store, action, this.httpClient).pipe( + map(() => ({ type: action.actions[1] })), + catchError(() => [{ type: action.actions[2] }]) + ); + }) + ); + @Effect() clearEntityUpdates$ = this.actions$.pipe( ofType(UsersRolesActions.ClearUpdateState), mergeMap(action => { diff --git a/src/frontend/packages/cloud-foundry/src/store/selectors/cloud-foundry.selector.ts b/src/frontend/packages/cloud-foundry/src/store/selectors/cloud-foundry.selector.ts deleted file mode 100644 index 3df60ec938..0000000000 --- a/src/frontend/packages/cloud-foundry/src/store/selectors/cloud-foundry.selector.ts +++ /dev/null @@ -1,4 +0,0 @@ -import { connectedEndpointsOfTypesSelector } from '../../../../store/src/selectors/endpoint.selectors'; -import { CF_ENDPOINT_TYPE } from '../../cf-types'; - -export const endpointsCfEntitiesConnectedSelector = connectedEndpointsOfTypesSelector(CF_ENDPOINT_TYPE); diff --git a/src/frontend/packages/cloud-foundry/src/user-permissions/cf-user-permissions-checkers.ts b/src/frontend/packages/cloud-foundry/src/user-permissions/cf-user-permissions-checkers.ts index def80a3110..b38816caf9 100644 --- a/src/frontend/packages/cloud-foundry/src/user-permissions/cf-user-permissions-checkers.ts +++ b/src/frontend/packages/cloud-foundry/src/user-permissions/cf-user-permissions-checkers.ts @@ -19,7 +19,7 @@ import { IConfigGroups, ICurrentUserPermissionsChecker, IPermissionCheckCombiner, -} from '../../../core/src/core/permissions/stratos-user-permissions.checker'; +} from '../../../core/src/core/permissions/current-user-permissions.types'; import { GeneralEntityAppState } from '../../../store/src/app-state'; import { connectedEndpointsSelector } from '../../../store/src/selectors/endpoint.selectors'; import { CFFeatureFlagTypes, IFeatureFlag } from '../cf-api.types'; @@ -397,7 +397,7 @@ export class CfUserPermissionsChecker extends BaseCurrentUserPermissionsChecker private getAllEndpointGuids() { return this.store.select(connectedEndpointsSelector).pipe( - map(endpoints => Object.values(endpoints).filter(e => e.cnsi_type === 'cf').map(endpoint => endpoint.guid)) + map(endpoints => Object.values(endpoints).filter(e => e.cnsi_type === CF_ENDPOINT_TYPE).map(endpoint => endpoint.guid)) ); } diff --git a/src/frontend/packages/cloud-foundry/src/user-permissions/cf-user-roles-fetch.ts b/src/frontend/packages/cloud-foundry/src/user-permissions/cf-user-roles-fetch.ts index 24ae18d80f..ec6687ced1 100644 --- a/src/frontend/packages/cloud-foundry/src/user-permissions/cf-user-roles-fetch.ts +++ b/src/frontend/packages/cloud-foundry/src/user-permissions/cf-user-roles-fetch.ts @@ -16,6 +16,7 @@ import { PaginationFlattener, } from '../../../store/src/helpers/paginated-request-helpers'; import { ActionState } from '../../../store/src/reducers/api-request-reducer/types'; +import { connectedEndpointsOfTypesSelector } from '../../../store/src/selectors/endpoint.selectors'; import { selectPaginationState } from '../../../store/src/selectors/pagination.selectors'; import { BasePaginatedAction, PaginationEntityState } from '../../../store/src/types/pagination.types'; import { @@ -28,14 +29,14 @@ import { GetCurrentCfUserRelationsComplete, } from '../actions/permissions.actions'; import { cfEntityCatalog } from '../cf-entity-catalog'; -import { endpointsCfEntitiesConnectedSelector } from '../store/selectors/cloud-foundry.selector'; +import { CF_ENDPOINT_TYPE } from '../cf-types'; import { CFResponse } from '../store/types/cf-api.types'; const createEndpointArray = (store: Store, endpoints: string[] | EntityUserRolesEndpoint[]): Observable => { // If there's no endpoints get all from store. Alternatively fetch specific endpoint id's from store if (!endpoints || !endpoints.length || typeof (endpoints[0]) === 'string') { const endpointIds = endpoints as string[]; - return store.select(endpointsCfEntitiesConnectedSelector).pipe( + return store.select(connectedEndpointsOfTypesSelector(CF_ENDPOINT_TYPE)).pipe( first(), map(cfEndpoints => endpointIds.length === 0 ? Object.values(cfEndpoints) : diff --git a/src/frontend/packages/core/src/core/permissions/current-user-permissions.service.ts b/src/frontend/packages/core/src/core/permissions/current-user-permissions.service.ts index 009a965899..3d407620cd 100644 --- a/src/frontend/packages/core/src/core/permissions/current-user-permissions.service.ts +++ b/src/frontend/packages/core/src/core/permissions/current-user-permissions.service.ts @@ -19,8 +19,8 @@ import { BaseCurrentUserPermissionsChecker, ICurrentUserPermissionsChecker, IPermissionCheckCombiner, - StratosUserPermissionsChecker, -} from './stratos-user-permissions.checker'; +} from './current-user-permissions.types'; +import { StratosUserPermissionsChecker } from './stratos-user-permissions.checker'; export const CUSTOM_USER_PERMISSION_CHECKERS = 'custom_user_perm_checkers' diff --git a/src/frontend/packages/core/src/core/permissions/current-user-permissions.types.ts b/src/frontend/packages/core/src/core/permissions/current-user-permissions.types.ts new file mode 100644 index 0000000000..1314130007 --- /dev/null +++ b/src/frontend/packages/core/src/core/permissions/current-user-permissions.types.ts @@ -0,0 +1,61 @@ +import { combineLatest, Observable, of } from 'rxjs'; +import { distinctUntilChanged, map } from 'rxjs/operators'; + +import { PermissionConfig, PermissionConfigType, PermissionTypes } from './current-user-permissions.config'; + +export interface IConfigGroups { + [permissionType: string]: IConfigGroup; +} + +export type IConfigGroup = PermissionConfig[]; + +export type IPermissionCheckCombineTypes = '||' | '&&'; + +export interface IPermissionCheckCombiner { + checks: Observable[]; + combineType?: IPermissionCheckCombineTypes; +} +export interface ICurrentUserPermissionsChecker { + /** + * For the given permission action find the checker configuration that will determine if the user can or cannot do the action + * If this is not supported by the the checker null is returned. If another checker also lays claim to the same string the check will + * always return denied + */ + getPermissionConfig: (action: string) => PermissionConfigType + /** + * Simple checks are used when the permission config contains a single thing to check + */ + getSimpleCheck: ( + permissionConfig: PermissionConfig, + endpointGuid?: string, + ...args: any[] + ) => Observable; + /** + * Used when the permission config contains multiple things to check + */ + getComplexCheck: ( + permissionConfig: PermissionConfig[], + permission: PermissionTypes, + ...args: any[] + ) => IPermissionCheckCombiner[]; + /** + * If no checker provides simple + */ + getFallbackCheck: ( + endpointGuid: string, + endpointType: string + ) => Observable; +} + +export abstract class BaseCurrentUserPermissionsChecker { + public static reduceChecks(checks: Observable[], type: IPermissionCheckCombineTypes = '||') { + const func = type === '||' ? 'some' : 'every'; + if (!checks || !checks.length) { + return of(true); + } + return combineLatest(checks).pipe( + map(flags => flags[func](flag => flag)), + distinctUntilChanged() + ); + } +} \ No newline at end of file diff --git a/src/frontend/packages/core/src/core/permissions/stratos-user-permissions.checker.ts b/src/frontend/packages/core/src/core/permissions/stratos-user-permissions.checker.ts index ff977c886c..a0bcb58695 100644 --- a/src/frontend/packages/core/src/core/permissions/stratos-user-permissions.checker.ts +++ b/src/frontend/packages/core/src/core/permissions/stratos-user-permissions.checker.ts @@ -1,78 +1,20 @@ import { Store } from '@ngrx/store'; -import { combineLatest, Observable, of as observableOf } from 'rxjs'; -import { distinctUntilChanged, map } from 'rxjs/operators'; +import { Observable } from 'rxjs'; import { GeneralEntityAppState } from '../../../../store/src/app-state'; import { getCurrentUserStratosHasScope, getCurrentUserStratosRole, } from '../../../../store/src/selectors/current-user-role.selectors'; +import { IPermissionConfigs, PermissionConfig, PermissionTypes, PermissionValues } from './current-user-permissions.config'; import { - IPermissionConfigs, - PermissionConfig, - PermissionConfigType, - PermissionTypes, - PermissionValues, -} from './current-user-permissions.config'; + BaseCurrentUserPermissionsChecker, + IConfigGroups, + ICurrentUserPermissionsChecker, + IPermissionCheckCombiner, +} from './current-user-permissions.types'; -export interface IConfigGroups { - [permissionType: string]: IConfigGroup; -} - -export type IConfigGroup = PermissionConfig[]; - -export type IPermissionCheckCombineTypes = '||' | '&&'; - -export interface IPermissionCheckCombiner { - checks: Observable[]; - combineType?: IPermissionCheckCombineTypes; -} -export interface ICurrentUserPermissionsChecker { - /** - * For the given permission action find the checker configuration that will determine if the user can or cannot do the action - * If this is not supported by the the checker null is returned. If another checker also lays claim to the same string the check will - * always return denied - */ - getPermissionConfig: (action: string) => PermissionConfigType - /** - * Simple checks are used when the permission config contains a single thing to check - */ - getSimpleCheck: ( - permissionConfig: PermissionConfig, - endpointGuid?: string, - ...args: any[] - ) => Observable; - /** - * Used when the permission config contains multiple things to check - */ - getComplexCheck: ( - permissionConfig: PermissionConfig[], - permission: PermissionTypes, - ...args: any[] - ) => IPermissionCheckCombiner[]; - /** - * If no checker provides simple - */ - getFallbackCheck: ( - endpointGuid: string, - endpointType: string - ) => Observable; -} - -export abstract class BaseCurrentUserPermissionsChecker { - public static reduceChecks(checks: Observable[], type: IPermissionCheckCombineTypes = '||') { - const func = type === '||' ? 'some' : 'every'; - if (!checks || !checks.length) { - return observableOf(true); - } - return combineLatest(checks).pipe( - map(flags => flags[func](flag => flag)), - distinctUntilChanged() - ); - } -} - export enum StratosCurrentUserPermissions { ENDPOINT_REGISTER = 'register.endpoint', PASSWORD_CHANGE = 'change-password', diff --git a/src/frontend/packages/store/src/reducers/current-user-roles-reducer/current-user-roles.reducer.ts b/src/frontend/packages/store/src/reducers/current-user-roles-reducer/current-user-roles.reducer.ts index c75ebfbc4e..ad4ffc58b4 100644 --- a/src/frontend/packages/store/src/reducers/current-user-roles-reducer/current-user-roles.reducer.ts +++ b/src/frontend/packages/store/src/reducers/current-user-roles-reducer/current-user-roles.reducer.ts @@ -24,8 +24,8 @@ const getDefaultState = () => ({ }); export function currentUserRolesReducer(state: ICurrentUserRolesState = getDefaultState(), action: Action): ICurrentUserRolesState { - const stateAfterStratosChanges = coreCurrentUserRolesReducer(state, action); - return entityCatalog.getAllCurrentUserReducers(stateAfterStratosChanges, action); + const stateAfterCoreChanges = coreCurrentUserRolesReducer(state, action); + return entityCatalog.getAllCurrentUserReducers(stateAfterCoreChanges, action); } function coreCurrentUserRolesReducer(state: ICurrentUserRolesState, action: Action): ICurrentUserRolesState { diff --git a/src/frontend/packages/store/src/types/current-user-roles.types.ts b/src/frontend/packages/store/src/types/current-user-roles.types.ts index 08243b9ed7..90c41efcb4 100644 --- a/src/frontend/packages/store/src/types/current-user-roles.types.ts +++ b/src/frontend/packages/store/src/types/current-user-roles.types.ts @@ -19,11 +19,10 @@ export interface IStratosRolesState { scopes: UserScopeStrings[]; } -export interface ICurrentUserRolesState { +export interface ICurrentUserRolesState { internal: IStratosRolesState; endpoints: { - // T could be different in each endpoint type, however supplying a type makes it nicer to use when looking at a specific type - [endpointType: string]: T + [endpointType: string]: any } state: RolesRequestState; }