fix(authz): let permission interceptor handle nesting (#48)

* fix(authz): remove duplicate provide exports * fix(authz): remove alias of condition service * fix(authz): let interceptor handle nesting * Adapt `PermissionInterceptor` such that it also considers nested properties (objects as well as array). * Remove request scope from interceptor * Use proper constants for metadata
kittgen · Feb 9, 2021 · 8f2d5c9 · 8f2d5c9
1 parent 961c268
commit 8f2d5c9
Show file tree

Hide file tree

Showing 8 changed files with 240 additions and 91 deletions.
diff --git a/example-apps/my-nest-project/src/is-author.condition.ts b/example-apps/my-nest-project/src/is-author.condition.ts
@@ -1,19 +1,20 @@
 import { ExecutionContext, Injectable } from '@nestjs/common';
 import {
   AbstractCondition,
-  ConditionsService,
-  ConditionContext
+  ConditionService,
+  ConditionContext,
 } from '@kittgen/nestjs-authorization';
 
 @Injectable()
 export class IsAuthor extends AbstractCondition {
-  constructor(
-    conditionService: ConditionsService,
-  ) {
+  constructor(conditionService: ConditionService) {
     super(conditionService);
   }
 
-  async check(ctx: ExecutionContext, conditionCtx: ConditionContext): Promise<boolean> {
+  async check(
+    ctx: ExecutionContext,
+    conditionCtx: ConditionContext,
+  ): Promise<boolean> {
     const req = ctx.switchToHttp().getRequest();
     return req.body.authorId === 1;
   }

diff --git a/example-apps/with-db/src/is-author.condition.ts b/example-apps/with-db/src/is-author.condition.ts
@@ -1,19 +1,20 @@
 import { ExecutionContext, Injectable } from '@nestjs/common';
 import {
   AbstractCondition,
-  ConditionsService,
-  ConditionContext
+  ConditionService,
+  ConditionContext,
 } from '@kittgen/nestjs-authorization';
 
 @Injectable()
 export class IsAuthor extends AbstractCondition {
-  constructor(
-    conditionService: ConditionsService,
-  ) {
+  constructor(conditionService: ConditionService) {
     super(conditionService);
   }
 
-  async check(ctx: ExecutionContext, conditionCtx: ConditionContext): Promise<boolean> {
+  async check(
+    ctx: ExecutionContext,
+    conditionCtx: ConditionContext,
+  ): Promise<boolean> {
     const req = ctx.switchToHttp().getRequest();
     return req.body.authorId === 1;
   }

diff --git a/packages/nestjs-authorization/src/authorization.constants.ts b/packages/nestjs-authorization/src/authorization.constants.ts
@@ -1 +1,3 @@
 export const AUTHORIZATION_MODULE_OPTIONS = 'AUTHORIZATION_MODULE_OPTIONS';
+export const AUTHORIZATION_EXPOSED_WITH_PERMISSION_PROPS =
+  'AUTHORIZATION_EXPOSED_WITH_PERMISSION_PROPS ';
diff --git a/packages/nestjs-authorization/src/authorization.module.ts b/packages/nestjs-authorization/src/authorization.module.ts
@@ -1,12 +1,7 @@
 import { DynamicModule, Module } from '@nestjs/common';
-import { PermissionService } from './permissions/permission.service';
-import { ConditionService } from './conditions/condition.service';
 import { AuthorizationModuleAsyncOptions } from './authorization-module.interface';
 import { AuthorizationCoreModule } from './authorization-core.module';
-@Module({
-  providers: [ConditionService, PermissionService],
-  exports: [ConditionService, PermissionService],
-})
+@Module({})
 export class AuthorizationModule {
   static registerAsync(
     options: AuthorizationModuleAsyncOptions

diff --git a/packages/nestjs-authorization/src/index.ts b/packages/nestjs-authorization/src/index.ts
@@ -11,7 +11,7 @@ export {
   AbstractCondition,
   ConditionContext,
 } from './conditions/condition';
-export { ConditionService as ConditionsService } from './conditions/condition.service';
+export { ConditionService } from './conditions/condition.service';
 export {
   SimplePermissionSet,
   PermissionSet,

diff --git a/packages/nestjs-authorization/src/permissions/expose-with-permission.decorator.ts b/packages/nestjs-authorization/src/permissions/expose-with-permission.decorator.ts
@@ -1,11 +1,20 @@
 import { Action } from '../actions/action';
+import { AUTHORIZATION_EXPOSED_WITH_PERMISSION_PROPS } from '../authorization.constants';
 
 export function ExposeWithPermission(action: Action): PropertyDecorator {
   return (target, key) => {
-    const fields = Reflect.getMetadata('actions', target) || [];
-    if (!fields.includes(key)) {
-      fields.push([key, action]);
+    const props =
+      Reflect.getMetadata(
+        AUTHORIZATION_EXPOSED_WITH_PERMISSION_PROPS,
+        target
+      ) || [];
+    if (!props.includes(key)) {
+      props.push([key, action]);
     }
-    Reflect.defineMetadata('actions', fields, target);
+    Reflect.defineMetadata(
+      AUTHORIZATION_EXPOSED_WITH_PERMISSION_PROPS,
+      props,
+      target
+    );
   };
 }
diff --git a/packages/nestjs-authorization/src/permissions/permission.interceptor.spec.ts b/packages/nestjs-authorization/src/permissions/permission.interceptor.spec.ts
@@ -11,43 +11,46 @@ import { PermissionProvider } from './permission.provider';
 import { PermissionService } from './permission.service';
 import { ConditionService } from '../conditions/condition.service';
 
-const createMockContext = (req: any) =>
+const createMockContext = (req: any = {}) =>
   createMock<ExecutionContext>({
     switchToHttp: () =>
       createMock<HttpArgumentsHost>({
         getRequest: () => req,
       }),
   });
 
-const createCallHandler = () => ({
-  handle: jest.fn(),
+const createPermissionProvider = (
+  permissions: string[] = []
+): PermissionProvider => ({
+  getPermissionSet: async (_ctx: any): Promise<PermissionSet> => {
+    return Promise.resolve(
+      new SimplePermissionSet(
+        ...permissions.map(perm => new SimplePermission(perm))
+      )
+    );
+  },
 });
 
 const createPermissionInterceptor = (
-  permissionProvider: PermissionProvider
+  ...permissions: string[]
 ): PermissionInterceptor => {
   const interceptor = new PermissionInterceptor(
     new Reflector(),
-    undefined,
     {
-      permissionProvider,
+      permissionProvider: createPermissionProvider(permissions),
     },
     new PermissionService({} as ConditionService)
   );
   return interceptor;
 };
 
-const createPermissionProvider = (
-  permissions: string[] = []
-): PermissionProvider => ({
-  getPermissionSet: async (_ctx: any): Promise<PermissionSet> => {
-    return Promise.resolve(
-      new SimplePermissionSet(
-        ...permissions.map(perm => new SimplePermission(perm))
-      )
-    );
-  },
-});
+const mockCallHandler = (dto: any) => {
+  const callHandler = {
+    handle: jest.fn(),
+  };
+  callHandler.handle.mockReturnValue(from(Promise.resolve(dto)));
+  return callHandler;
+};
 
 class TestDto {
   @ExposeWithPermission('can-read-foo')
@@ -60,43 +63,148 @@ class TestDto {
   }
 }
 
+class UndecoratedTestDto {
+  baz: string;
+
+  constructor(_baz: string) {
+    this.baz = _baz;
+  }
+}
+
+class NestedTestDto {
+  quux: string;
+
+  dto: TestDto;
+
+  constructor(_quux: string, dto: TestDto) {
+    this.quux = _quux;
+    this.dto = dto;
+  }
+}
+class NestedTestWithConditionalArrayDto {
+  quux: string;
+
+  @ExposeWithPermission('can-read-dto')
+  dtos: TestDto[];
+
+  constructor(_quux: string, dtos: TestDto[]) {
+    this.quux = _quux;
+    this.dtos = dtos;
+  }
+}
+class NestedArrayTestDto {
+  quux: string;
+
+  dtos: TestDto[];
+
+  constructor(_quux: string, dtos: TestDto[]) {
+    this.quux = _quux;
+    this.dtos = dtos;
+  }
+}
+
 describe('PermissionInterceptor', () => {
-  describe('#intercept', () => {
-    it('should exclude properties is permissions are lacking', async () => {
-      const executionContext = createMockContext({});
-      const callHandler = createCallHandler();
-      const interceptor = createPermissionInterceptor(
-        createPermissionProvider()
-      );
-      callHandler.handle.mockReturnValue(
-        from(Promise.resolve(new TestDto('hello', 'world')))
+  describe('intercept', () => {
+    it('should exclude properties if permissions are lacking', async () => {
+      const callHandler = mockCallHandler(new TestDto('hello', 'world'));
+      const interceptor = createPermissionInterceptor();
+
+      const dto = await interceptor
+        .intercept(createMockContext(), callHandler)
+        .toPromise();
+
+      expect(dto).toHaveProperty('bar');
+      expect(dto).not.toHaveProperty('foo');
+    });
+
+    it('should include properties if permitted', async () => {
+      const callHandler = mockCallHandler(new TestDto('hello', 'world'));
+      const interceptor = createPermissionInterceptor('can-read-foo');
+
+      const dto = await interceptor
+        .intercept(createMockContext(), callHandler)
+        .toPromise();
+
+      expect(dto).toHaveProperty('bar');
+      expect(dto).toHaveProperty('foo');
+    });
+
+    it('should handle DTOs without decorator', async () => {
+      const callHandler = mockCallHandler(new UndecoratedTestDto('hello'));
+      const interceptor = createPermissionInterceptor('can-read-foo');
+
+      const dto = await interceptor
+        .intercept(createMockContext(), callHandler)
+        .toPromise();
+
+      expect(dto).toHaveProperty('baz');
+    });
+
+    it('should handle nested DTOs', async () => {
+      const callHandler = mockCallHandler(
+        new NestedTestDto('quuz', new TestDto('foo', 'bar'))
       );
-      const observable = await interceptor.intercept(
-        executionContext,
-        callHandler
+      const interceptor = createPermissionInterceptor('can-read-foo');
+
+      const dto = await interceptor
+        .intercept(createMockContext(), callHandler)
+        .toPromise();
+
+      expect(dto).toHaveProperty('quux');
+      expect(dto.dto).toEqual({
+        foo: 'foo',
+        bar: 'bar',
+      });
+    });
+
+    it('should handle nested array of DTOs', async () => {
+      const callHandler = mockCallHandler(
+        new NestedArrayTestDto('quuz', [new TestDto('foo', 'bar')])
       );
-      const serializedDto = await observable.toPromise();
-      expect(serializedDto).toHaveProperty('bar');
-      expect(serializedDto).not.toHaveProperty('foo');
+      const interceptor = createPermissionInterceptor('can-read-foo');
+
+      const dto = await interceptor
+        .intercept(createMockContext(), callHandler)
+        .toPromise();
+
+      expect(dto).toHaveProperty('quux');
+      expect(dto.dtos[0]).toEqual({
+        foo: 'foo',
+        bar: 'bar',
+      });
     });
 
-    it('should include properties if permitted', async () => {
+    it('should handle nested array of DTOs without permission', async () => {
       const executionContext = createMockContext({});
-      const callHandler = createCallHandler();
-      const interceptor = createPermissionInterceptor(
-        createPermissionProvider(['can-read-foo'])
+      const callHandler = mockCallHandler(
+        new NestedArrayTestDto('quuz', [new TestDto('foo', 'bar')])
       );
+      const interceptor = createPermissionInterceptor();
 
-      callHandler.handle.mockReturnValue(
-        from(Promise.resolve(new TestDto('hello', 'world')))
-      );
-      const observable = await interceptor.intercept(
-        executionContext,
-        callHandler
+      const dto = await interceptor
+        .intercept(executionContext, callHandler)
+        .toPromise();
+
+      expect(dto).toHaveProperty('quux');
+      expect(dto.dtos[0]).toEqual({
+        bar: 'bar',
+      });
+    });
+
+    it('should exclude nested array if no permission', async () => {
+      const callHandler = mockCallHandler(
+        new NestedTestWithConditionalArrayDto('quuz', [
+          new TestDto('foo', 'bar'),
+        ])
       );
-      const serializedDto = await observable.toPromise();
-      expect(serializedDto).toHaveProperty('bar');
-      expect(serializedDto).toHaveProperty('foo');
+      const interceptor = createPermissionInterceptor();
+
+      const dto = await interceptor
+        .intercept(createMockContext(), callHandler)
+        .toPromise();
+
+      expect(dto).toHaveProperty('quux');
+      expect(dto).not.toHaveProperty('dtos');
     });
   });
 });