Inject EntityService<E> into EntityAuthorizationService

tschaffter · tschaffter · commit a6e4d71490a6 · 2019-10-05T11:36:12.000-07:00
- Adapt project, tool, data catalog authorization service and guard
diff --git a/client/app/data-catalog/data-catalog-authorization.service.ts b/client/app/data-catalog/data-catalog-authorization.service.ts
@@ -1,16 +1,19 @@
 import { Injectable } from '@angular/core';
-import { Observable } from 'rxjs';
-import { filter, mapTo } from 'rxjs/operators';
+import { DataCatalog } from 'models/entities/data-catalog.model';
 import { UserPermissionDataService } from 'components/auth/user-permission-data.service';
 import { EntityAuthorizationService } from 'components/authorization/entity-authorization.service';
+import { DataCatalogService } from './data-catalog.service';
 import config from '../app.constants';
 
 @Injectable()
-export class DataCatalogAuthorizationService extends EntityAuthorizationService {
+export class DataCatalogAuthorizationService extends EntityAuthorizationService<DataCatalog> {
     // TODO Find a way to not have to inject the service in the child service, only the parent service.
-    static parameters = [UserPermissionDataService];
-    constructor(protected userPermissionDataService: UserPermissionDataService) {
-        super(userPermissionDataService);
+    static parameters = [UserPermissionDataService, DataCatalogService];
+    constructor(
+        protected userPermissionDataService: UserPermissionDataService,
+        protected entityService: DataCatalogService
+    ) {
+        super(userPermissionDataService, entityService);
     }
 
     getEntityType(): string {
@@ -20,13 +23,4 @@ export class DataCatalogAuthorizationService extends EntityAuthorizationService
     getCreateActionPermissionType(): string {
         return config.actionPermissionTypes.CREATE_DATA_CATALOG.value;
     }
-
-    // TODO Remove when DataCatalog can be made private
-    /** @override */
-    canRead(catalogId: string): Observable<boolean> {
-        return this.userPermissionDataService.permissions().pipe(
-            filter(auth => !!auth),
-            mapTo(true)
-        );
-    }
 }
diff --git a/client/app/data-catalog/data-catalog-guard.service.ts b/client/app/data-catalog/data-catalog-guard.service.ts
@@ -2,9 +2,10 @@ import { Injectable } from '@angular/core';
 import { Router } from '@angular/router';
 import { DataCatalogAuthorizationService } from './data-catalog-authorization.service';
 import { EntityGuard } from 'components/authorization/entity-guard.service';
+import { DataCatalog } from 'models/entities/data-catalog.model';
 
 @Injectable()
-export class DataCatalogGuard extends EntityGuard {
+export class DataCatalogGuard extends EntityGuard<DataCatalog> {
     static parameters = [Router, DataCatalogAuthorizationService];
     constructor(protected router: Router, protected authorizationService: DataCatalogAuthorizationService) {
         super(router, authorizationService);
diff --git a/client/app/data-catalog/data-catalog.module.ts b/client/app/data-catalog/data-catalog.module.ts
@@ -39,8 +39,7 @@ export const ROUTES: Routes = [
     {
         path: 'data-catalogs/:id',
         component: DataCatalogComponent,
-        canActivate: [AuthGuard, DataCatalogGuard],
-        data: { authorization: EntityAuthorizationTypes.READ },
+        canActivate: [AuthGuard],
     },
     {
         path: 'data-catalogs/:id/discussion',
diff --git a/client/app/data-catalog/data-catalog.service.ts b/client/app/data-catalog/data-catalog.service.ts
@@ -8,6 +8,8 @@ import { DataCatalog } from 'models/entities/data-catalog.model';
 import { Patch } from 'models/patch.model';
 import { QueryListResponse } from 'models/query-list-response.model';
 import { EntityAttachment } from 'models/entities/entity-attachment.model';
+import { StringValue } from 'models/string-value.model';
+import config from '../app.constants';
 
 @Injectable()
 export class DataCatalogService implements EntityService<DataCatalog> {
@@ -38,6 +40,15 @@ export class DataCatalogService implements EntityService<DataCatalog> {
         return this.httpClient.delete(`/api/data-catalogs/${dataCatalog._id}`).pipe(map(() => dataCatalog));
     }
 
+    isPublic(id: string): Observable<boolean> {
+        // TODO Restore meaningful code when data catalogs fully support ACL
+        // For now data catalogs are considered public
+        return of(true);
+        // return this.httpClient
+            // .get<StringValue>(`/api/data-catalogs/${id}/visibility`)
+            // .pipe(map(res => res.value === config.entityVisibility.PUBLIC.value));
+    }
+
     makePublic(entity: DataCatalog): Observable<DataCatalog> {
         throw new Error('Method not implemented.');
     }
diff --git a/client/app/project/project-authorization.service.ts b/client/app/project/project-authorization.service.ts
@@ -1,14 +1,19 @@
 import { Injectable } from '@angular/core';
+import { Project } from 'models/entities/project.model';
 import { UserPermissionDataService } from 'components/auth/user-permission-data.service';
 import { EntityAuthorizationService } from 'components/authorization/entity-authorization.service';
+import { ProjectService } from './project.service';
 import config from '../app.constants';
 
 @Injectable()
-export class ProjectAuthorizationService extends EntityAuthorizationService {
+export class ProjectAuthorizationService extends EntityAuthorizationService<Project> {
     // TODO Find a way to not have to inject the service in the child service, only the parent service.
-    static parameters = [UserPermissionDataService];
-    constructor(protected userPermissionDataService: UserPermissionDataService) {
-        super(userPermissionDataService);
+    static parameters = [UserPermissionDataService, ProjectService];
+    constructor(
+        protected userPermissionDataService: UserPermissionDataService,
+        protected entityService: ProjectService
+    ) {
+        super(userPermissionDataService, entityService);
     }
 
     getEntityType(): string {
diff --git a/client/app/project/project-guard.service.ts b/client/app/project/project-guard.service.ts
@@ -1,10 +1,11 @@
 import { Injectable } from '@angular/core';
 import { Router } from '@angular/router';
-import { ProjectAuthorizationService } from './project-authorization.service';
+import { Project } from 'models/entities/project.model';
 import { EntityGuard } from 'components/authorization/entity-guard.service';
+import { ProjectAuthorizationService } from './project-authorization.service';
 
 @Injectable()
-export class ProjectGuard extends EntityGuard {
+export class ProjectGuard extends EntityGuard<Project> {
     static parameters = [Router, ProjectAuthorizationService];
     constructor(protected router: Router, protected authorizationService: ProjectAuthorizationService) {
         super(router, authorizationService);
diff --git a/client/app/project/project.module.ts b/client/app/project/project.module.ts
@@ -65,8 +65,8 @@ export const ROUTES: Routes = [
     {
         path: 'projects/:id',
         component: ProjectComponent,
-        canActivate: [AuthGuard/*, ProjectGuard*/],
-        // data: { authorization: EntityAuthorizationTypes.READ }, // TODO Need to add a step to check whether a project is public
+        canActivate: [AuthGuard, ProjectGuard],
+        data: { authorization: EntityAuthorizationTypes.READ },
         children: [
             { path: '', redirectTo: 'home', pathMatch: 'full' },
             { path: 'home', component: ProjectHomeComponent },
diff --git a/client/app/project/project.service.ts b/client/app/project/project.service.ts
@@ -1,3 +1,4 @@
+import { BooleanValue } from './../../../shared/interfaces/boolean-value.model';
 import { Injectable } from '@angular/core';
 import { Observable, of } from 'rxjs';
 import { debounceTime, distinctUntilChanged, map, switchMap, tap } from 'rxjs/operators';
@@ -10,6 +11,8 @@ import { EntityVisibility, Entity } from 'models/entities/entity.model';
 import { EntityService } from 'components/entity/entity.service';
 import { QueryListResponse } from 'models/query-list-response.model';
 import { EntityAttachment } from 'models/entities/entity-attachment.model';
+import { StringValue } from 'models/string-value.model';
+import config from '../app.constants';
 
 @Injectable()
 export class ProjectService implements EntityService<Project> {
@@ -40,6 +43,12 @@ export class ProjectService implements EntityService<Project> {
         return this.httpClient.delete(`/api/projects/${project._id}`).pipe(map(() => project));
     }
 
+    isPublic(id: string): Observable<boolean> {
+        return this.httpClient
+            .get<StringValue>(`/api/projects/${id}/visibility`)
+            .pipe(map(res => res.value === config.entityVisibility.PUBLIC.value));
+    }
+
     makePublic(entity: Project): Observable<Project> {
         return this.httpClient.patch<Project>(`/api/projects/${entity._id}/visibility/public`, []);
     }
diff --git a/client/app/tool/tool-authorization.service.ts b/client/app/tool/tool-authorization.service.ts
@@ -1,16 +1,16 @@
 import { Injectable } from '@angular/core';
-import { Observable } from 'rxjs';
-import { filter, mapTo } from 'rxjs/operators';
+import { Tool } from 'models/entities/tool.model';
 import { UserPermissionDataService } from 'components/auth/user-permission-data.service';
 import { EntityAuthorizationService } from 'components/authorization/entity-authorization.service';
+import { ToolService } from './tool.service';
 import config from '../app.constants';
 
 @Injectable()
-export class ToolAuthorizationService extends EntityAuthorizationService {
+export class ToolAuthorizationService extends EntityAuthorizationService<Tool> {
     // TODO Find a way to not have to inject the service in the child service, only the parent service.
-    static parameters = [UserPermissionDataService];
-    constructor(protected userPermissionDataService: UserPermissionDataService) {
-        super(userPermissionDataService);
+    static parameters = [UserPermissionDataService, ToolService];
+    constructor(protected userPermissionDataService: UserPermissionDataService, protected entityService: ToolService) {
+        super(userPermissionDataService, entityService);
     }
 
     getEntityType(): string {
@@ -20,13 +20,4 @@ export class ToolAuthorizationService extends EntityAuthorizationService {
     getCreateActionPermissionType(): string {
         return config.actionPermissionTypes.CREATE_TOOL.value;
     }
-
-    // TODO Remove when Tool can be made private
-    /** @override */
-    canRead(toolId: string): Observable<boolean> {
-        return this.userPermissionDataService.permissions().pipe(
-            filter(auth => !!auth),
-            mapTo(true)
-        );
-    }
 }
diff --git a/client/app/tool/tool-guard.service.ts b/client/app/tool/tool-guard.service.ts
@@ -2,9 +2,10 @@ import { Injectable } from '@angular/core';
 import { Router } from '@angular/router';
 import { ToolAuthorizationService } from './tool-authorization.service';
 import { EntityGuard } from 'components/authorization/entity-guard.service';
+import { Tool } from 'models/entities/tool.model';
 
 @Injectable()
-export class ToolGuard extends EntityGuard {
+export class ToolGuard extends EntityGuard<Tool> {
     static parameters = [Router, ToolAuthorizationService];
     constructor(protected router: Router, protected authorizationService: ToolAuthorizationService) {
         super(router, authorizationService);
diff --git a/client/app/tool/tool.service.ts b/client/app/tool/tool.service.ts
@@ -11,6 +11,8 @@ import { EntityService } from 'components/entity/entity.service';
 import { QueryListResponse } from 'models/query-list-response.model';
 import { Patch } from 'models/patch.model';
 import { EntityAttachment } from 'models/entities/entity-attachment.model';
+import { StringValue } from 'models/string-value.model';
+import config from '../app.constants';
 
 @Injectable()
 export class ToolService implements EntityService<Tool> {
@@ -41,6 +43,15 @@ export class ToolService implements EntityService<Tool> {
         return this.httpClient.delete(`/api/tools/${tool._id}`).pipe(map(() => tool));
     }
 
+    isPublic(id: string): Observable<boolean> {
+        // TODO Restore meaningful code when tools fully support ACL
+        // For now tools are considered public
+        return of(true);
+        // return this.httpClient
+        //     .get<StringValue>(`/api/tools/${id}/visibility`)
+        //     .pipe(map(res => res.value === config.entityVisibility.PUBLIC.value));
+    }
+
     makePublic(entity: Tool): Observable<Tool> {
         throw new Error('Method not implemented.');
     }
diff --git a/client/components/authorization/entity-authorization.service.ts b/client/components/authorization/entity-authorization.service.ts
@@ -1,7 +1,9 @@
 import { Injectable } from '@angular/core';
-import { Observable, BehaviorSubject, Subscription } from 'rxjs';
-import { filter, map } from 'rxjs/operators';
+import { Observable, BehaviorSubject, Subscription, of } from 'rxjs';
+import { filter, map, switchMap } from 'rxjs/operators';
 import { UserPermissionDataService } from 'components/auth/user-permission-data.service';
+import { EntityService } from 'components/entity/entity.service';
+import { Entity } from 'models/entities/entity.model';
 
 export interface EntityAuthorization {
     canCreate: boolean;
@@ -11,9 +13,8 @@ export interface EntityAuthorization {
 }
 
 @Injectable()
-export abstract class EntityAuthorizationService {
-    static parameters = [UserPermissionDataService];
-    constructor(protected userPermissionDataService: UserPermissionDataService) {}
+export abstract class EntityAuthorizationService<E extends Entity> {
+    constructor(protected userPermissionDataService: UserPermissionDataService, protected entityService: EntityService<E>) {}
 
     abstract getEntityType(): string; // TODO use enum
 
@@ -39,9 +40,15 @@ export abstract class EntityAuthorizationService {
      * @param entityId
      */
     canRead(entityId: string): Observable<boolean> {
-        return this.userPermissionDataService.permissions().pipe(
-            filter(auth => !!auth),
-            map(auth => auth.canReadEntity(entityId, this.getEntityType()))
+        return this.entityService.isPublic(entityId).pipe(
+            switchMap(isPublic =>
+                isPublic
+                    ? of(true)
+                    : this.userPermissionDataService.permissions().pipe(
+                          filter(auth => !!auth),
+                          map(auth => auth.canReadEntity(entityId, this.getEntityType()))
+                      )
+            )
         );
     }
 
diff --git a/client/components/authorization/entity-guard.service.ts b/client/components/authorization/entity-guard.service.ts
@@ -3,6 +3,7 @@ import { CanActivate, Router, ActivatedRouteSnapshot } from '@angular/router';
 import { Observable, of } from 'rxjs';
 import { map, catchError } from 'rxjs/operators';
 import { EntityAuthorizationService } from './entity-authorization.service';
+import { Entity } from 'models/entities/entity.model';
 
 export enum EntityAuthorizationTypes {
     CREATE,
@@ -12,8 +13,8 @@ export enum EntityAuthorizationTypes {
 }
 
 @Injectable()
-export abstract class EntityGuard implements CanActivate {
-    constructor(protected router: Router, protected authorizationService: EntityAuthorizationService) {}
+export abstract class EntityGuard<E extends Entity> implements CanActivate {
+    constructor(protected router: Router, protected authorizationService: EntityAuthorizationService<E>) {}
 
     canActivate(route: ActivatedRouteSnapshot): Observable<boolean> {
         let authorized$: Observable<boolean>;
diff --git a/client/components/entity/entity.service.ts b/client/components/entity/entity.service.ts
@@ -19,11 +19,11 @@ export abstract class EntityService<E extends Entity> {
      */
     abstract get(id: string): Observable<E>;
 
-    /**
-     * Returns the entity with the slug specified.
-     * @param slug
-     */
-    abstract getBySlug(slug: string): Observable<E>;
+    // /**
+    //  * Returns the entity with the slug specified.
+    //  * @param slug
+    //  */
+    // abstract getBySlug(slug: string): Observable<E>;
 
     /**
      * Creates a new entity.
@@ -44,6 +44,12 @@ export abstract class EntityService<E extends Entity> {
      */
     abstract remove(entity: E): Observable<E>;
 
+    /**
+     * Returns true if the entity is public.
+     * @param id
+     */
+    abstract isPublic(id: string): Observable<boolean>;
+
     /**
      * Makes an entity public.
      * @param entity
diff --git a/client/components/insight/insight.service.ts b/client/components/insight/insight.service.ts
@@ -14,6 +14,8 @@ import { ShareSidenavComponent } from 'components/share/share-sidenav/share-side
 import { Entity } from 'models/entities/entity.model';
 import { Patch } from 'models/patch.model';
 import { EntityAttachment } from 'models/entities/entity-attachment.model';
+import { StringValue } from 'models/string-value.model';
+import config from '../../app/app.constants';
 
 @Injectable()
 export class InsightService implements EntityService<Insight> {
@@ -44,6 +46,12 @@ export class InsightService implements EntityService<Insight> {
         return this.httpClient.delete(`/api/insights/${insight._id}`).pipe(map(() => insight));
     }
 
+    isPublic(id: string): Observable<boolean> {
+        return this.httpClient
+            .get<StringValue>(`/api/insights/${id}/visibility`)
+            .pipe(map(res => res.value === config.entityVisibility.PUBLIC.value));
+    }
+
     makePublic(entity: Insight): Observable<Insight> {
         throw new Error('Method not implemented.');
     }
diff --git a/client/components/resource/resource.service.ts b/client/components/resource/resource.service.ts
@@ -11,6 +11,8 @@ import { ActivitySidenavComponent } from 'components/activity/activity-sidenav/a
 import { EntityService } from 'components/entity/entity.service';
 import { Patch } from 'models/patch.model';
 import { EntityAttachment } from 'models/entities/entity-attachment.model';
+import { StringValue } from 'models/string-value.model';
+import config from '../../app/app.constants';
 
 @Injectable()
 export class ResourceService implements EntityService<Resource> {
@@ -41,6 +43,12 @@ export class ResourceService implements EntityService<Resource> {
         return this.httpClient.delete(`/api/resources/${resource._id}`).pipe(map(() => resource));
     }
 
+    isPublic(id: string): Observable<boolean> {
+        return this.httpClient
+            .get<StringValue>(`/api/resources/${id}/visibility`)
+            .pipe(map(res => res.value === config.entityVisibility.PUBLIC.value));
+    }
+
     makePublic(entity: Resource): Observable<Resource> {
         throw new Error('Method not implemented.');
     }
diff --git a/server/api/project/project.controller.js b/server/api/project/project.controller.js
diff --git a/shared/interfaces/boolean-value.model.ts b/shared/interfaces/boolean-value.model.ts
diff --git a/shared/interfaces/string-value.model.ts b/shared/interfaces/string-value.model.ts

Original file line number	Diff line number	Diff line change
`@@ -39,8 +39,7 @@ export const ROUTES: Routes = [`
`39`	`39`	`{`
`40`	`40`	`path: 'data-catalogs/:id',`
`41`	`41`	`component: DataCatalogComponent,`
`42`		`- canActivate: [AuthGuard, DataCatalogGuard],`
`43`		`- data: { authorization: EntityAuthorizationTypes.READ },`
	`42`	`+ canActivate: [AuthGuard],`
`44`	`43`	`},`
`45`	`44`	`{`
`46`	`45`	`path: 'data-catalogs/:id/discussion',`