From 0002b481bf5e044b4732d4308f501c742d2505da Mon Sep 17 00:00:00 2001 From: Dan Percic Date: Sat, 29 Oct 2022 14:41:03 +0300 Subject: [PATCH 1/3] remove promise from validation functions --- .../permissions/permissions.directive.spec.ts | 260 ++---------------- src/lib/permissions/permissions.directive.ts | 9 +- .../services/permissions-guard.service.ts | 41 +-- .../services/permissions.service.spec.ts | 70 +---- .../services/permissions.service.ts | 20 +- .../services/roles.service.spec.ts | 48 ++-- src/lib/permissions/services/roles.service.ts | 8 +- src/lib/permissions/types.ts | 4 +- 8 files changed, 95 insertions(+), 365 deletions(-) diff --git a/src/lib/permissions/permissions.directive.spec.ts b/src/lib/permissions/permissions.directive.spec.ts index 6be1b0e..11ee281 100644 --- a/src/lib/permissions/permissions.directive.spec.ts +++ b/src/lib/permissions/permissions.directive.spec.ts @@ -58,8 +58,6 @@ describe('Permission directive', () => { expect(content.innerHTML).toEqual('123'); expect(isAuthorizedSpy).toHaveBeenCalledTimes(1); - expect(isUnauthorizedSpy).toHaveBeenCalledTimes(1); - expect(isUnauthorizedSpy).toHaveBeenCalledBefore(isAuthorizedSpy); })); it('should not show the component', fakeAsync(() => { @@ -68,7 +66,6 @@ describe('Permission directive', () => { expect(getFixtureContent()).toEqual(null); expect(isAuthorizedSpy).toHaveBeenCalledTimes(0); - expect(isUnauthorizedSpy).toHaveBeenCalledTimes(1); })); it('should show component when permission added', fakeAsync(() => { @@ -84,9 +81,7 @@ describe('Permission directive', () => { expect(content).toBeTruthy(); expect(content.innerHTML).toEqual('123'); - expect(isUnauthorizedSpy).toHaveBeenCalledTimes(1); expect(isAuthorizedSpy).toHaveBeenCalledTimes(1); - expect(isUnauthorizedSpy).toHaveBeenCalledBefore(isAuthorizedSpy); })); it('should hide component when permission removed', fakeAsync(() => { @@ -96,15 +91,13 @@ describe('Permission directive', () => { const content = getFixtureContent(); expect(content).toBeTruthy(); expect(content.innerHTML).toEqual('123'); - expect(isUnauthorizedSpy).toHaveBeenCalledTimes(1); expect(isAuthorizedSpy).toHaveBeenCalledTimes(1); - expect(isUnauthorizedSpy).toHaveBeenCalledBefore(isAuthorizedSpy); permissionsService.remove(ADMIN); tick(); expect(getFixtureContent()).toEqual(null); - expect(isUnauthorizedSpy).toHaveBeenCalledTimes(2); + expect(isUnauthorizedSpy).toHaveBeenCalledTimes(1); expect(isAuthorizedSpy).toHaveBeenCalledTimes(1); })); }); @@ -287,26 +280,6 @@ describe('Permission directive angular testing different async functions in role expect(getFixtureContent()).toEqual(null); })); - - it('should show the component when promise returns truthy value', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - rolesService.add({ ADMIN: () => Promise.resolve(true) }); - tick(); - - const content = getFixtureContent(); - expect(content).toBeTruthy(); - expect(content.innerHTML).toEqual('
123
'); - })); - - it('should not show the component when promise rejects', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - rolesService.add({ ADMIN: () => Promise.reject() }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); }); describe('Permission directive angular testing different async functions in roles via array', () => { @@ -319,11 +292,11 @@ describe('Permission directive angular testing different async functions in role beforeEach(() => configureTestBed(TestComponent)); - it('should show the component when promise returns truthy value', fakeAsync(() => { + it('should show the component when returns truthy value', fakeAsync(() => { expect(getFixtureContent()).toEqual(null); - rolesService.add({ ADMIN: () => Promise.resolve(true) }); - rolesService.add({ GUEST: () => Promise.resolve(true) }); + rolesService.add({ ADMIN: () => true }); + rolesService.add({ GUEST: () => true }); tick(); const content = getFixtureContent(); @@ -331,57 +304,11 @@ describe('Permission directive angular testing different async functions in role expect(content.innerHTML).toEqual('
123
'); })); - it('should not show the component when promise returns false value', fakeAsync(() => { + it('should not show the component when returns false value', fakeAsync(() => { expect(getFixtureContent()).toEqual(null); - rolesService.add({ ADMIN: () => Promise.resolve(false) }); - rolesService.add({ GUEST: () => Promise.resolve(true) }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - - it('should not show the component when promise rejects', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - rolesService.add({ ADMIN: () => Promise.resolve(true) }); - rolesService.add({ GUEST: () => Promise.reject() }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - - it('should hide the component when one of the promises fulfills', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - rolesService.add({ ADMIN: () => Promise.reject(), GUEST: () => Promise.resolve(true) }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - - it('should hide the component when one of the promises fulfills with true value', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - rolesService.add({ ADMIN: () => Promise.reject(), GUEST: () => Promise.resolve(true) }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - - it('should not show the component when all promises fails', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - rolesService.add({ ADMIN: () => Promise.reject(), GUEST: () => Promise.reject() }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - - it('should hide the component when one of promises returns true', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - rolesService.add({ GUEST: () => true, ADMIN: () => Promise.reject() }); + rolesService.add({ ADMIN: () => false }); + rolesService.add({ GUEST: () => true }); tick(); expect(getFixtureContent()).toEqual(null); @@ -390,7 +317,7 @@ describe('Permission directive angular testing different async functions in role it('should hide the component when 1 passes second fails', fakeAsync(() => { expect(getFixtureContent()).toEqual(null); - rolesService.add({ ADMIN: () => Promise.reject() }); + rolesService.add({ ADMIN: () => false }); rolesService.add({ GUEST: ['AWESOME'] }); tick(); @@ -408,11 +335,11 @@ describe('Permission directive with different async functions in permissions via beforeEach(() => configureTestBed(TestComponent)); - it('should show the component when promise returns truthy value', fakeAsync(() => { + it('should show the component when returns truthy value', fakeAsync(() => { expect(getFixtureContent()).toEqual(null); permissionsService.add({ ADMIN: () => true }); - permissionsService.add({ GUEST: () => Promise.resolve(true) }); + permissionsService.add({ GUEST: () => true }); tick(); const content = getFixtureContent(); @@ -420,7 +347,7 @@ describe('Permission directive with different async functions in permissions via expect(content.innerHTML).toEqual('
123
'); })); - it('should not show the component when promise returns false value', fakeAsync(() => { + it('should not show the component when returns false value', fakeAsync(() => { expect(getFixtureContent()).toEqual(null); permissionsService.add({ ADMIN: () => false }); @@ -429,12 +356,12 @@ describe('Permission directive with different async functions in permissions via expect(getFixtureContent()).toEqual(null); })); - it('should show the component when promises returns truthy value', fakeAsync(() => { + it('should show the component when returns truthy value', fakeAsync(() => { expect(getFixtureContent()).toEqual(null); permissionsService.add({ - ADMIN: () => Promise.resolve(true), - GUEST: () => Promise.resolve(true), + ADMIN: () => true, + GUEST: () => true, }); tick(); @@ -443,85 +370,6 @@ describe('Permission directive with different async functions in permissions via expect(content2.innerHTML).toEqual('
123
'); })); - it('should not show the component when promise rejects', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ ADMIN: () => Promise.reject() }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - - it('should not show the component when only one of the promises fulfills ', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ ADMIN: () => Promise.resolve(true) }); - permissionsService.add({ GUEST: () => Promise.reject() }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - - it('should not show the component when one of the promises fulfills with false value', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ ADMIN: () => Promise.resolve(true) }); - permissionsService.add({ GUEST: () => Promise.resolve(false) }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - - it('should not show the component when all promises fails', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ ADMIN: () => Promise.reject() }); - permissionsService.add({ GUEST: () => Promise.reject() }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - - it('should not show the component when one of promises rejects', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ GUEST: () => true }); - permissionsService.add({ ADMIN: () => Promise.reject() }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - - it('should not show the component when a promise fails', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ ADMIN: () => Promise.reject() }); - permissionsService.add({ GUEST: () => Promise.resolve(true) }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - - it('should not show the component when one rejects but another one fulfils', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ ADMIN: () => Promise.reject() }); - permissionsService.add({ GUEST: () => true }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - - it('should not show the component when one rejects but another one fulfils', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ ADMIN: () => true }); - permissionsService.add({ GUEST: () => Promise.reject() }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - it('should not show the component when functions with name and store fulfils', fakeAsync(() => { expect(getFixtureContent()).toEqual(null); @@ -533,7 +381,7 @@ describe('Permission directive with different async functions in permissions via }, }); - permissionsService.add({ GUEST: () => Promise.reject() }); + permissionsService.add({ GUEST: () => false }); tick(); expect(getFixtureContent()).toEqual(null); @@ -570,94 +418,26 @@ describe('Permission directive testing different async functions in permissions expect(getFixtureContent()).toEqual(null); })); - it('should show the component when promise returns truthy value', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ ADMIN: () => Promise.resolve(true) }); - tick(); - - const content = getFixtureContent(); - expect(content).toBeTruthy(); - expect(content.innerHTML).toEqual('
123
'); - })); - - it('should not show the component when promise rejects', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ ADMIN: () => Promise.reject() }); - tick(); - - expect(getFixtureContent()).toEqual(null); - })); - it('should not show the component when only one of the promises fulfills', fakeAsync(() => { expect(getFixtureContent()).toEqual(null); - permissionsService.add({ ADMIN: () => Promise.resolve(false) }); - permissionsService.add({ GUEST: () => Promise.resolve(true) }); + permissionsService.add({ ADMIN: () => false }); + permissionsService.add({ GUEST: () => true }); tick(); expect(getFixtureContent()).toEqual(null); })); - it('should show the component when one of the promises fulfills with 0 value', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ ADMIN: () => Promise.resolve(true) }); - permissionsService.add({ GUEST: () => Promise.reject() }); - tick(); - - const content = getFixtureContent(); - expect(content).toBeTruthy(); - expect(content.innerHTML).toEqual('
123
'); - })); - it('should not show the component when all promises fails', fakeAsync(() => { expect(getFixtureContent()).toEqual(null); - permissionsService.add({ ADMIN: () => Promise.reject() }); - permissionsService.add({ GUEST: () => Promise.reject() }); + permissionsService.add({ ADMIN: () => false }); + permissionsService.add({ GUEST: () => false }); tick(); expect(getFixtureContent()).toEqual(null); })); - it('should show the component when one of promises returns true', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ GUEST: () => Promise.reject() }); - permissionsService.add({ ADMIN: () => true }); - tick(); - - const content = getFixtureContent(); - expect(content).toBeTruthy(); - expect(content.innerHTML).toEqual('
123
'); - })); - - it('should not show the component when all promises fails', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ ADMIN: () => Promise.resolve(true) }); - permissionsService.add({ GUEST: () => Promise.reject() }); - tick(); - - const content = getFixtureContent(); - expect(content).toBeTruthy(); - expect(content.innerHTML).toEqual('
123
'); - })); - - it('should show the component when one rejects but another one fulfils', fakeAsync(() => { - expect(getFixtureContent()).toEqual(null); - - permissionsService.add({ ADMIN: () => true }); - permissionsService.add({ GUEST: () => Promise.reject() }); - tick(); - - const content = getFixtureContent(); - expect(content).toBeTruthy(); - expect(content.innerHTML).toEqual('
123
'); - })); - it('should show the component when functions with name and store fulfils', fakeAsync(() => { expect(getFixtureContent()).toEqual(null); @@ -669,7 +449,7 @@ describe('Permission directive testing different async functions in permissions }, }); - permissionsService.add({ GUEST: () => Promise.reject() }); + permissionsService.add({ GUEST: () => false }); tick(); const content = getFixtureContent(); diff --git a/src/lib/permissions/permissions.directive.ts b/src/lib/permissions/permissions.directive.ts index de1a1a1..1617523 100644 --- a/src/lib/permissions/permissions.directive.ts +++ b/src/lib/permissions/permissions.directive.ts @@ -111,18 +111,15 @@ export class IqserPermissionsDirective implements OnDestroy, OnInit { } #validateRolesAndPermissions() { - return merge(this._permissionsService.permissions$, this._rolesService.roles$).pipe(switchMap(() => this.#validate())); + return merge(this._permissionsService.permissions$, this._rolesService.roles$).pipe(map(() => this.#validate())); } #validate() { if (!this.#permissions) { - return Promise.resolve(true); + return true; } - const promises = [this._permissionsService.has(this.#permissions), this._rolesService.has(this.#permissions)]; - return Promise.all(promises) - .then(([hasPermission, hasRole]) => hasPermission || hasRole) - .catch(() => false); + return this._permissionsService.has(this.#permissions) || this._rolesService.has(this.#permissions); } #showElseBlock() { diff --git a/src/lib/permissions/services/permissions-guard.service.ts b/src/lib/permissions/services/permissions-guard.service.ts index 9d8cf59..14e25e6 100644 --- a/src/lib/permissions/services/permissions-guard.service.ts +++ b/src/lib/permissions/services/permissions-guard.service.ts @@ -1,8 +1,8 @@ import { Injectable } from '@angular/core'; import { ActivatedRouteSnapshot, CanActivate, CanActivateChild, CanLoad, Route, Router, RouterStateSnapshot } from '@angular/router'; -import { firstValueFrom, forkJoin, from, of } from 'rxjs'; -import { first, mergeMap, tap } from 'rxjs/operators'; +import { firstValueFrom, from, of } from 'rxjs'; +import { first, mergeMap } from 'rxjs/operators'; import { DEFAULT_REDIRECT_KEY, @@ -95,15 +95,12 @@ export class IqserPermissionsGuard implements CanActivate, CanLoad, CanActivateC let failedPermission = ''; const res = from(permissions.allow).pipe( mergeMap(permission => { - return forkJoin([this._permissionsService.has(permission), this._rolesService.has(permission)]).pipe( - tap(hasPermissions => { - const failed = hasPermissions.every(hasPermission => hasPermission === false); - - if (failed) { - failedPermission = permission; - } - }), - ); + const results = [this._permissionsService.has(permission), this._rolesService.has(permission)]; + const failed = results.every(result => !result); + if (failed) { + failedPermission = permission; + } + return of(results); }), first(hasPermissions => { if (isFunction(permissions.redirectTo)) { @@ -177,22 +174,14 @@ export class IqserPermissionsGuard implements CanActivate, CanLoad, CanActivateC return undefined; } - #validatePermissions( - permissions: IqserPermissionsData, - route: ActivatedRouteSnapshot | Route, - state?: RouterStateSnapshot, - ): Promise { - const results = Promise.all([this._permissionsService.has(permissions.allow), this._rolesService.has(permissions.allow)]); + #validatePermissions(permissions: IqserPermissionsData, route: ActivatedRouteSnapshot | Route, state?: RouterStateSnapshot) { + const isAllowed = this._permissionsService.has(permissions.allow) || this._rolesService.has(permissions.allow); - return results - .then(([hasPermission, hasRole]) => hasPermission || hasRole) - .then(isAllowed => { - if (!isAllowed && permissions.redirectTo) { - const redirect = this.#redirectToAnotherRoute(permissions.redirectTo, route, permissions.allow[0], state); - return redirect.then(() => false); - } + if (!isAllowed && permissions.redirectTo) { + const redirect = this.#redirectToAnotherRoute(permissions.redirectTo, route, permissions.allow[0], state); + return redirect.then(() => false); + } - return isAllowed; - }); + return isAllowed; } } diff --git a/src/lib/permissions/services/permissions.service.spec.ts b/src/lib/permissions/services/permissions.service.spec.ts index 44a36a5..46d7480 100644 --- a/src/lib/permissions/services/permissions.service.spec.ts +++ b/src/lib/permissions/services/permissions.service.spec.ts @@ -57,103 +57,63 @@ describe('Permissions Service', () => { expect(Object.keys(permissionsService.get())).toEqual([ADMIN, GUEST]); }); - it('should return true when permission name is present in permissions object', async () => { + it('should return true when permission name is present in permissions object', () => { expect(getPermissionsLength()).toEqual(0); permissionsService.add([ADMIN, GUEST]); expect(getPermissionsLength()).toEqual(2); - let result = await permissionsService.has('ADMIN'); + let result = permissionsService.has('ADMIN'); expect(result).toEqual(true); - result = await permissionsService.has('SHOULDNOTHAVEROLE'); + result = permissionsService.has('SHOULDNOTHAVEROLE'); expect(result).toEqual(false); - result = await permissionsService.has(['ADMIN']); + result = permissionsService.has(['ADMIN']); expect(result).toEqual(true); - result = await permissionsService.has(['ADMIN', 'IRIISISTABLE']); + result = permissionsService.has(['ADMIN', 'IRIISISTABLE']); expect(result).toEqual(false); }); - it('should return true when permission function return true', async () => { + it('should return true when function return true', () => { expect(getPermissionsLength()).toEqual(0); permissionsService.add({ ADMIN: () => true }); expect(getPermissionsLength()).toEqual(1); - let result = await permissionsService.has('ADMIN'); + let result = permissionsService.has('ADMIN'); expect(result).toEqual(true); permissionsService.add({ GUEST: () => false }); expect(getPermissionsLength()).toEqual(2); - result = await permissionsService.has('GUEST'); - expect(result).toEqual(false); - - permissionsService.add({ TEST1: () => Promise.resolve(true) }); - expect(getPermissionsLength()).toEqual(3); - - result = await permissionsService.has('TEST1'); - expect(result).toEqual(true); - - permissionsService.add({ TEST2: () => Promise.resolve(false) }); - expect(getPermissionsLength()).toEqual(4); - - result = await permissionsService.has('TEST2'); + result = permissionsService.has('GUEST'); expect(result).toEqual(false); }); - it('should return true when permissions array function return true', async () => { - expect(getPermissionsLength()).toEqual(0); - - permissionsService.add({ ADMIN: () => true }); - expect(getPermissionsLength()).toEqual(1); - - let result = await permissionsService.has('ADMIN'); - expect(result).toEqual(true); - - permissionsService.add({ GUEST: () => false }); - expect(getPermissionsLength()).toEqual(2); - - result = await permissionsService.has('GUEST'); - expect(result).toEqual(false); - - permissionsService.add({ TEST1: () => Promise.resolve(true) }); - expect(getPermissionsLength()).toEqual(3); - - result = await permissionsService.has('TEST1'); - expect(result).toEqual(true); - - permissionsService.add({ TEST9: () => Promise.resolve(false) }); - expect(getPermissionsLength()).toEqual(4); - - result = await permissionsService.has(['TEST9']); - expect(result).toEqual(false); - }); - - it('should call validationFn with permission name and store', async () => { + it('should call validationFn with permission name and store', () => { permissionsService.add({ TEST11: (name, store) => { expect(name).toEqual('TEST11'); expect(store['TEST11']).toBeTruthy(); - return Promise.resolve(true); + return true; }, }); expect(getPermissionsLength()).toEqual(1); - const result = await permissionsService.has(['TEST11']); + const result = permissionsService.has(['TEST11']); expect(result).toEqual(true); }); - it('should return true when called with empty parameters', async () => { - const result = await permissionsService.has(''); + it('should return true when called with empty parameters', () => { + const result = permissionsService.has(''); expect(result).toEqual(true); }); - it('should return true when called with empty array', async () => { - const result = await permissionsService.has([]); + it('should return true when called with empty array', () => { + const result = permissionsService.has([]); expect(result).toEqual(true); }); }); diff --git a/src/lib/permissions/services/permissions.service.ts b/src/lib/permissions/services/permissions.service.ts index b2e3eb7..1ac4946 100644 --- a/src/lib/permissions/services/permissions.service.ts +++ b/src/lib/permissions/services/permissions.service.ts @@ -1,6 +1,6 @@ import { Injectable } from '@angular/core'; -import { BehaviorSubject, firstValueFrom, Observable, of, switchMap } from 'rxjs'; +import { BehaviorSubject, Observable, of } from 'rxjs'; import { isArray, isString, toArray } from '../utils'; import { IqserPermissions, PermissionValidationFn } from '../types'; @@ -20,11 +20,18 @@ export class IqserPermissionsService { this.#permissions$.next({}); } - has(permission: string): Promise; - has(permissions: List): Promise; - has(permissions: string | List): Promise; - has(permissions: string | List): Promise { - return firstValueFrom(this.has$(permissions)); + has(permission: string): boolean; + has(permissions: List): boolean; + has(permissions: string | List): boolean; + has(permissions: string | List): boolean { + const isEmpty = !permissions || permissions.length === 0; + if (isEmpty) { + return true; + } + + const all = this.#permissions$.value; + const results = toArray(permissions).map(permission => all[permission]?.(permission, all) ?? false); + return results.every(result => result); } load(permissions: IqserPermissions | List) { @@ -61,7 +68,6 @@ export class IqserPermissionsService { return this.permissions$.pipe( map(all => toArray(permissions).map(permission => all[permission]?.(permission, all) ?? false)), - switchMap(promises => Promise.all(promises)), map(results => results.every(result => result)), ); } diff --git a/src/lib/permissions/services/roles.service.spec.ts b/src/lib/permissions/services/roles.service.spec.ts index 908e50a..6f6d786 100644 --- a/src/lib/permissions/services/roles.service.spec.ts +++ b/src/lib/permissions/services/roles.service.spec.ts @@ -77,7 +77,7 @@ describe('Roles Service', () => { }); }); - it('return true when role name is present in roles object', async () => { + it('return true when role name is present in roles object', () => { expect(getRolesLength()).toEqual(0); rolesService.add({ @@ -87,17 +87,17 @@ describe('Roles Service', () => { expect(getRolesLength()).toEqual(2); - let result = await rolesService.has(ADMIN); + let result = rolesService.has(ADMIN); expect(result).toEqual(true); - result = await rolesService.has('SHOULDNOTHAVEROLE'); + result = rolesService.has('SHOULDNOTHAVEROLE'); expect(result).toEqual(false); - result = await rolesService.has([ADMIN, 'IRIISISTABLE']); + result = rolesService.has([ADMIN, 'IRIISISTABLE']); expect(result).toEqual(false); }); - it('return true when role permission name is present in Roles object', async () => { + it('return true when role permission name is present in Roles object', () => { expect(getRolesLength()).toEqual(0); rolesService.add({ @@ -107,16 +107,16 @@ describe('Roles Service', () => { expect(getRolesLength()).toEqual(2); - let result = await rolesService.has(ADMIN); + let result = rolesService.has(ADMIN); expect(result).toEqual(true); - result = await rolesService.has([ADMIN, 'IRRISISTABLE']); + result = rolesService.has([ADMIN, 'IRRISISTABLE']); expect(result).toEqual(false); - result = await rolesService.has('SHOULDNOTHAVEROLE'); + result = rolesService.has('SHOULDNOTHAVEROLE'); expect(result).toEqual(false); - result = await rolesService.has(['SHOULDNOTHAVEROLE']); + result = rolesService.has(['SHOULDNOTHAVEROLE']); expect(result).toEqual(false); }); @@ -128,53 +128,53 @@ describe('Roles Service', () => { expect(validationFn('role', rolesService.get())).toEqual(true); }); - it('should return true when checking with empty permission(not specified)', async () => { - const result = await rolesService.has(''); + it('should return true when checking with empty permission(not specified)', () => { + const result = rolesService.has(''); expect(result).toEqual(true); }); - it('should return false when permission array is empty', async () => { - const result = await rolesService.has('Empty'); + it('should return false when permission array is empty', () => { + const result = rolesService.has('Empty'); expect(result).toEqual(false); }); - it('should return false when role is not specified in the list', async () => { + it('should return false when role is not specified in the list', () => { rolesService.add({ test: ['One'] }); - const result = await rolesService.has('nice'); + const result = rolesService.has('nice'); expect(result).toBe(false); }); - it('should return true when passing empty array', async () => { + it('should return true when passing empty array', () => { rolesService.add({ test: ['One'] }); - const result = await rolesService.has([]); + const result = rolesService.has([]); expect(result).toBe(true); }); - it('should add permissions to roles automatically', async () => { + it('should add permissions to roles automatically', () => { rolesService.add({ test: ['one', 'two'] }); - const result = await rolesService.has('test'); + const result = rolesService.has('test'); expect(result).toBe(true); }); - it('should remove roles and permissions add the same time', async () => { + it('should remove roles and permissions add the same time', () => { rolesService.add({ test: ['one', 'two'] }); - let result = await rolesService.has('test'); + let result = rolesService.has('test'); expect(result).toBe(true); - result = await permissionsService.has('one'); + result = permissionsService.has('one'); expect(result).toBe(true); rolesService.clear(); permissionsService.clear(); - result = await rolesService.has('test'); + result = rolesService.has('test'); expect(result).toBe(false); - result = await permissionsService.has('one'); + result = permissionsService.has('one'); expect(result).toBe(false); }); diff --git a/src/lib/permissions/services/roles.service.ts b/src/lib/permissions/services/roles.service.ts index de3b396..5527c3e 100644 --- a/src/lib/permissions/services/roles.service.ts +++ b/src/lib/permissions/services/roles.service.ts @@ -40,17 +40,15 @@ export class IqserRolesService { return role ? this.#roles$.value[role] : this.#roles$.value; } - has(roles: string | List): Promise { + has(roles: string | List): boolean { const isEmpty = !roles || roles.length === 0; if (isEmpty) { - return Promise.resolve(true); + return true; } const validations = toArray(roles).map(role => this.#runValidation(role)); - return Promise.all(validations) - .then(results => this.#checkPermissionsIfNeeded(results)) - .then(results => results.every(result => result === true)); + return this.#checkPermissionsIfNeeded(validations).every(result => result); } #checkPermissionsIfNeeded(results: List) { diff --git a/src/lib/permissions/types.ts b/src/lib/permissions/types.ts index 68fe07c..bd8a109 100644 --- a/src/lib/permissions/types.ts +++ b/src/lib/permissions/types.ts @@ -25,8 +25,8 @@ export type RedirectTo = export type RedirectToFn = (failedPermission: string, route?: ActivatedRouteSnapshot | Route, state?: RouterStateSnapshot) => RedirectTo; export type NavigationCommandsFn = (route: ActivatedRouteSnapshot | Route, state?: RouterStateSnapshot) => any[]; export type NavigationExtrasFn = (route: ActivatedRouteSnapshot | Route, state?: RouterStateSnapshot) => NavigationExtras; -export type RoleValidationFn = (name: string, store: IqserRoles) => Promise | boolean | List; -export type PermissionValidationFn = (name: string, store: IqserPermissions) => Promise | boolean; +export type RoleValidationFn = (name: string, store: IqserRoles) => boolean | List; +export type PermissionValidationFn = (name: string, store: IqserPermissions) => boolean; export type IqserActivatedRouteSnapshot = ActivatedRouteSnapshot & { data?: { From 3658c6567292738eb474e2552222c59fdd8f685c Mon Sep 17 00:00:00 2001 From: Dan Percic Date: Sat, 29 Oct 2022 18:50:18 +0300 Subject: [PATCH 2/3] update user service --- src/lib/permissions/types.ts | 4 ++++ src/lib/users/services/iqser-user.service.ts | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/lib/permissions/types.ts b/src/lib/permissions/types.ts index bd8a109..77aec2a 100644 --- a/src/lib/permissions/types.ts +++ b/src/lib/permissions/types.ts @@ -38,6 +38,10 @@ export type IqserRoute = Route & { data?: { permissions?: IqserPermissionsRouterData; }; + + children?: List; }; +export type IqserRoutes = IqserRoute[]; + export const DEFAULT_REDIRECT_KEY = 'default'; diff --git a/src/lib/users/services/iqser-user.service.ts b/src/lib/users/services/iqser-user.service.ts index 78048e3..a7a90d1 100644 --- a/src/lib/users/services/iqser-user.service.ts +++ b/src/lib/users/services/iqser-user.service.ts @@ -1,4 +1,4 @@ -import { inject, Injectable, InjectFlags } from '@angular/core'; +import { inject, Injectable } from '@angular/core'; import { KeycloakService } from 'keycloak-angular'; import { BehaviorSubject, firstValueFrom, Observable } from 'rxjs'; import { tap } from 'rxjs/operators'; @@ -34,8 +34,8 @@ export abstract class IqserUserService< protected readonly _baseHref = inject(BASE_HREF); protected readonly _keycloakService = inject(KeycloakService); protected readonly _cacheApiService = inject(CacheApiService); - protected readonly _permissionsService = inject(IqserPermissionsService, InjectFlags.Optional); - protected readonly _rolesService = inject(IqserRolesService, InjectFlags.Optional); + protected readonly _permissionsService = inject(IqserPermissionsService, { optional: true }); + protected readonly _rolesService = inject(IqserRolesService, { optional: true }); constructor() { super(); @@ -86,7 +86,7 @@ export abstract class IqserUserService< const decoded: IToken = jwt_decode(token); const userId = decoded.sub; const realm = this._keycloakService.getKeycloakInstance().realm || ''; - this._permissionsService?.load(decoded.resource_access[realm].roles); + this._permissionsService?.load(decoded.resource_access[realm]?.roles ?? []); const roles = this._keycloakService.getUserRoles(true).filter(role => this._rolesFilter(role)); this._rolesService?.load(roles); From 45627ed3338011c64d1b328e12bdcac107a19280 Mon Sep 17 00:00:00 2001 From: Dan Percic Date: Sat, 5 Nov 2022 19:02:50 +0200 Subject: [PATCH 3/3] use id & roles from keycloak service --- src/lib/users/services/iqser-user.service.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/lib/users/services/iqser-user.service.ts b/src/lib/users/services/iqser-user.service.ts index a7a90d1..2eda00d 100644 --- a/src/lib/users/services/iqser-user.service.ts +++ b/src/lib/users/services/iqser-user.service.ts @@ -2,7 +2,6 @@ import { inject, Injectable } from '@angular/core'; import { KeycloakService } from 'keycloak-angular'; import { BehaviorSubject, firstValueFrom, Observable } from 'rxjs'; import { tap } from 'rxjs/operators'; -import jwt_decode from 'jwt-decode'; import { BASE_HREF, List, mapEach, RequiredParam, Validate } from '../../utils'; import { QueryParam } from '../../services'; import { CacheApiService } from '../../caching'; @@ -73,27 +72,25 @@ export abstract class IqserUserService< } async loadCurrentUser(): Promise { - let profile; + let profile: KeycloakProfile | undefined; try { profile = await this._keycloakService.loadUserProfile(true); + if (!profile.id) { + throw new Error('No user id'); + } } catch (e) { console.log(e); await this.logout(); return; } - const token = await this._keycloakService.getToken(); - const decoded: IToken = jwt_decode(token); - const userId = decoded.sub; - const realm = this._keycloakService.getKeycloakInstance().realm || ''; - this._permissionsService?.load(decoded.resource_access[realm]?.roles ?? []); - const roles = this._keycloakService.getUserRoles(true).filter(role => this._rolesFilter(role)); + this._permissionsService?.load(roles); this._rolesService?.load(roles); - const user = new this._entityClass(profile, roles, userId); + const user = new this._entityClass(profile, roles, profile.id); this.replace(user); - this._currentUser$.next(this.find(userId)); + this._currentUser$.next(this.find(profile.id)); return user; }