From a93f0bd0699cf99302e453f1881383729de34c46 Mon Sep 17 00:00:00 2001 From: Wojciech Duda <69160975+wojd0@users.noreply.github.com> Date: Tue, 8 Oct 2024 23:20:41 +0200 Subject: [PATCH] AAE-26321 Add an injection token to JwtHelperService for OAuthStorage (#10288) * AAE-26321 Add an injection token to JwtHelperService for OAuthStorage * AAE-26321 remove unneeded method spies * AAE-26321 Add missing providers to depending tests --- .../guard/auth-guard-sso-role.service.spec.ts | 6 +- .../lib/auth/guard/auth-guard.service.spec.ts | 3 +- .../oidc/oidc-authentication.service.spec.ts | 5 +- .../services/identity-user.service.spec.ts | 4 +- .../auth/services/jwt-helper.service.spec.ts | 89 +++++++++---------- .../lib/auth/services/jwt-helper.service.ts | 24 ++--- .../auth/services/user-access.service.spec.ts | 5 +- lib/core/src/lib/core.module.ts | 4 +- ...dit-process-filter-cloud.component.spec.ts | 5 +- 9 files changed, 74 insertions(+), 71 deletions(-) diff --git a/lib/core/src/lib/auth/guard/auth-guard-sso-role.service.spec.ts b/lib/core/src/lib/auth/guard/auth-guard-sso-role.service.spec.ts index 8b7c0fb2cf..0ca49540a9 100644 --- a/lib/core/src/lib/auth/guard/auth-guard-sso-role.service.spec.ts +++ b/lib/core/src/lib/auth/guard/auth-guard-sso-role.service.spec.ts @@ -18,9 +18,10 @@ import { TestBed } from '@angular/core/testing'; import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; import { AuthGuardSsoRoleService } from './auth-guard-sso-role.service'; -import { JwtHelperService } from '../services/jwt-helper.service'; +import { JWT_STORAGE_SERVICE, JwtHelperService } from '../services/jwt-helper.service'; import { MatDialog, MatDialogModule } from '@angular/material/dialog'; import { NoopTranslateModule } from '../../testing/noop-translate.module'; +import { StorageService } from '../../common'; describe('Auth Guard SSO role service', () => { let jwtHelperService: JwtHelperService; @@ -29,7 +30,8 @@ describe('Auth Guard SSO role service', () => { beforeEach(() => { TestBed.configureTestingModule({ - imports: [NoopTranslateModule, MatDialogModule] + imports: [NoopTranslateModule, MatDialogModule], + providers: [{ provide: JWT_STORAGE_SERVICE, useClass: StorageService }] }); localStorage.clear(); jwtHelperService = TestBed.inject(JwtHelperService); diff --git a/lib/core/src/lib/auth/guard/auth-guard.service.spec.ts b/lib/core/src/lib/auth/guard/auth-guard.service.spec.ts index ef3314e45e..48b0ee7086 100644 --- a/lib/core/src/lib/auth/guard/auth-guard.service.spec.ts +++ b/lib/core/src/lib/auth/guard/auth-guard.service.spec.ts @@ -28,6 +28,7 @@ import { EMPTY, of } from 'rxjs'; import { MatDialogModule } from '@angular/material/dialog'; import { RouterTestingModule } from '@angular/router/testing'; import { NoopTranslateModule } from '../../testing/noop-translate.module'; +import { JWT_STORAGE_SERVICE } from '../public-api'; describe('AuthGuardService', () => { let state: RouterStateSnapshot; @@ -45,7 +46,7 @@ describe('AuthGuardService', () => { imports: [NoopTranslateModule, MatDialogModule, RouterTestingModule], providers: [ AppConfigService, - StorageService, + { provide: JWT_STORAGE_SERVICE, useClass: StorageService }, { provide: RedirectAuthService, useValue: { onLogin: EMPTY, onTokenReceived: of() } }, { provide: OidcAuthenticationService, diff --git a/lib/core/src/lib/auth/oidc/oidc-authentication.service.spec.ts b/lib/core/src/lib/auth/oidc/oidc-authentication.service.spec.ts index a6fbadb23a..0a9b426e2f 100644 --- a/lib/core/src/lib/auth/oidc/oidc-authentication.service.spec.ts +++ b/lib/core/src/lib/auth/oidc/oidc-authentication.service.spec.ts @@ -18,8 +18,10 @@ import { TestBed } from '@angular/core/testing'; import { OidcAuthenticationService } from './oidc-authentication.service'; import { OAuthService, OAuthStorage } from 'angular-oauth2-oidc'; -import { AppConfigService, AuthService } from '@alfresco/adf-core'; import { AUTH_MODULE_CONFIG } from './auth-config'; +import { StorageService } from '../../common'; +import { AuthService, JWT_STORAGE_SERVICE } from '../public-api'; +import { AppConfigService } from '../../app-config'; interface MockAppConfigOAuth2 { oauth2: { @@ -62,6 +64,7 @@ describe('OidcAuthenticationService', () => { OidcAuthenticationService, { provide: AppConfigService, useClass: MockAppConfigService }, { provide: OAuthService, useClass: MockOAuthService }, + { provide: JWT_STORAGE_SERVICE, useValue: StorageService }, { provide: OAuthStorage, useValue: {} }, { provide: AUTH_MODULE_CONFIG, useValue: {} }, { provide: AuthService, useValue: {} } diff --git a/lib/core/src/lib/auth/services/identity-user.service.spec.ts b/lib/core/src/lib/auth/services/identity-user.service.spec.ts index 858843c9d9..a5af6efb2a 100644 --- a/lib/core/src/lib/auth/services/identity-user.service.spec.ts +++ b/lib/core/src/lib/auth/services/identity-user.service.spec.ts @@ -29,7 +29,7 @@ import { } from '../mock/identity-user.mock'; import { mockGroups, mockJoinGroupRequest } from '../mock/identity-group.mock'; import { IdentityUserService } from './identity-user.service'; -import { JwtHelperService } from './jwt-helper.service'; +import { JWT_STORAGE_SERVICE, JwtHelperService } from './jwt-helper.service'; import { mockToken } from '../mock/jwt-helper.service.spec'; import { IdentityRoleModel } from '../models/identity-role.model'; import { AdfHttpClient } from '../../../../api/src'; @@ -53,7 +53,7 @@ describe('IdentityUserService', () => { beforeEach(() => { TestBed.configureTestingModule({ imports: [NoopTranslateModule], - providers: [StorageService, AdfHttpClient] + providers: [StorageService, AdfHttpClient, { provide: JWT_STORAGE_SERVICE, useClass: StorageService }] }); storageService = TestBed.inject(StorageService); service = TestBed.inject(IdentityUserService); diff --git a/lib/core/src/lib/auth/services/jwt-helper.service.spec.ts b/lib/core/src/lib/auth/services/jwt-helper.service.spec.ts index 0c56406fef..06a899322b 100644 --- a/lib/core/src/lib/auth/services/jwt-helper.service.spec.ts +++ b/lib/core/src/lib/auth/services/jwt-helper.service.spec.ts @@ -15,17 +15,29 @@ * limitations under the License. */ -import { JwtHelperService } from './jwt-helper.service'; +import { JWT_STORAGE_SERVICE, JwtHelperService } from './jwt-helper.service'; import { mockToken } from '../mock/jwt-helper.service.spec'; import { TestBed } from '@angular/core/testing'; -describe('JwtHelperService', () => { +const mockLocalStorage = { + access_token: 'my-access_token', + id_token: 'my-id_token' +}; +describe('JwtHelperService', () => { let jwtHelperService: JwtHelperService; beforeEach(() => { TestBed.configureTestingModule({ - providers: [JwtHelperService] + providers: [ + JwtHelperService, + { + provide: JWT_STORAGE_SERVICE, + useValue: { + getItem: (key: string) => mockLocalStorage[key] + } + } + ] }); jwtHelperService = TestBed.inject(JwtHelperService); }); @@ -44,26 +56,19 @@ describe('JwtHelperService', () => { }); describe('RealmRole ', () => { - it('Should be true if the realm_access contains the single role', () => { - spyOn(jwtHelperService, 'getAccessToken').and.returnValue('my-access_token'); - - spyOn(jwtHelperService, 'decodeToken').and.returnValue( - { - realm_access: { roles: ['role1'] } - }); + spyOn(jwtHelperService, 'decodeToken').and.returnValue({ + realm_access: { roles: ['role1'] } + }); const result = jwtHelperService.hasRealmRole('role1'); expect(result).toBeTruthy(); }); it('Should be true if the realm_access contains at least one of the roles', () => { - spyOn(jwtHelperService, 'getAccessToken').and.returnValue('my-access_token'); - - spyOn(jwtHelperService, 'decodeToken').and.returnValue( - { - realm_access: { roles: ['role1'] } - }); + spyOn(jwtHelperService, 'decodeToken').and.returnValue({ + realm_access: { roles: ['role1'] } + }); const result = jwtHelperService.hasRealmRoles(['role1', 'role2']); expect(result).toBeTruthy(); @@ -71,46 +76,37 @@ describe('JwtHelperService', () => { it('Should be false if the realm_access does not contain the role', () => { spyOn(jwtHelperService, 'getAccessToken').and.returnValue('my-access_token'); - spyOn(jwtHelperService, 'decodeToken').and.returnValue( - { - realm_access: { roles: ['role3'] } - }); + spyOn(jwtHelperService, 'decodeToken').and.returnValue({ + realm_access: { roles: ['role3'] } + }); const result = jwtHelperService.hasRealmRole('role1'); expect(result).toBeFalsy(); }); it('Should be false if the realm_access does not contain at least one of the roles', () => { spyOn(jwtHelperService, 'getAccessToken').and.returnValue('my-access_token'); - spyOn(jwtHelperService, 'decodeToken').and.returnValue( - { - realm_access: { roles: ['role1'] } - }); + spyOn(jwtHelperService, 'decodeToken').and.returnValue({ + realm_access: { roles: ['role1'] } + }); const result = jwtHelperService.hasRealmRoles(['role3', 'role2']); expect(result).toBeFalsy(); }); - }); + }); describe('ClientRole ', () => { - it('Should be true if the resource_access contains the single role', () => { - spyOn(jwtHelperService, 'getAccessToken').and.returnValue('my-access_token'); - - spyOn(jwtHelperService, 'decodeToken').and.returnValue( - { - resource_access: { fakeApp: { roles: ['role1'] } } - }); + spyOn(jwtHelperService, 'decodeToken').and.returnValue({ + resource_access: { fakeApp: { roles: ['role1'] } } + }); const result = jwtHelperService.hasRealmRolesForClientRole('fakeApp', ['role1']); expect(result).toBeTruthy(); }); it('Should be true if the resource_access contains at least one of the roles', () => { - spyOn(jwtHelperService, 'getAccessToken').and.returnValue('my-access_token'); - - spyOn(jwtHelperService, 'decodeToken').and.returnValue( - { - resource_access: { fakeApp: { roles: ['role1'] } } - }); + spyOn(jwtHelperService, 'decodeToken').and.returnValue({ + resource_access: { fakeApp: { roles: ['role1'] } } + }); const result = jwtHelperService.hasRealmRolesForClientRole('fakeApp', ['role1', 'role2']); expect(result).toBeTruthy(); @@ -118,22 +114,19 @@ describe('JwtHelperService', () => { it('Should be false if the resource_access does not contain the role', () => { spyOn(jwtHelperService, 'getAccessToken').and.returnValue('my-access_token'); - spyOn(jwtHelperService, 'decodeToken').and.returnValue( - { - resource_access: { fakeApp: { roles: ['role3'] } } - }); + spyOn(jwtHelperService, 'decodeToken').and.returnValue({ + resource_access: { fakeApp: { roles: ['role3'] } } + }); const result = jwtHelperService.hasRealmRolesForClientRole('fakeApp', ['role1', 'role2']); expect(result).toBeFalsy(); }); it('Should be false if the resource_access does not contain the client role related to the app', () => { - spyOn(jwtHelperService, 'getAccessToken').and.returnValue('my-access_token'); - spyOn(jwtHelperService, 'decodeToken').and.returnValue( - { - resource_access: { anotherFakeApp: { roles: ['role1'] } } - }); + spyOn(jwtHelperService, 'decodeToken').and.returnValue({ + resource_access: { anotherFakeApp: { roles: ['role1'] } } + }); const result = jwtHelperService.hasRealmRolesForClientRole('fakeApp', ['role1', 'role2']); expect(result).toBeFalsy(); }); - }); + }); }); diff --git a/lib/core/src/lib/auth/services/jwt-helper.service.ts b/lib/core/src/lib/auth/services/jwt-helper.service.ts index d992f48278..1b2d635b00 100644 --- a/lib/core/src/lib/auth/services/jwt-helper.service.ts +++ b/lib/core/src/lib/auth/services/jwt-helper.service.ts @@ -15,14 +15,15 @@ * limitations under the License. */ -import { Injectable } from '@angular/core'; -import { StorageService } from '../../common/services/storage.service'; +import { Inject, Injectable, InjectionToken } from '@angular/core'; +import { OAuthStorage } from 'angular-oauth2-oidc'; + +export const JWT_STORAGE_SERVICE = new InjectionToken('JWT_STORAGE_SERVICE'); @Injectable({ providedIn: 'root' }) export class JwtHelperService { - static USER_NAME = 'name'; static FAMILY_NAME = 'family_name'; static GIVEN_NAME = 'given_name'; @@ -34,8 +35,7 @@ export class JwtHelperService { static USER_PREFERRED_USERNAME = 'preferred_username'; static HXP_AUTHORIZATION = 'hxp_authorization'; - constructor(private storageService: StorageService) { - } + constructor(@Inject(JWT_STORAGE_SERVICE) private jwtStorage: OAuthStorage) {} /** * Decodes a JSON web token into a JS object. @@ -85,7 +85,7 @@ export class JwtHelperService { * @param key Key name of the field to retrieve * @returns Value from the token */ - getValueFromLocalToken(key: string): T { + getValueFromLocalToken(key: string): T { return this.getValueFromToken(this.getAccessToken(), key) || this.getValueFromToken(this.getIdToken(), key); } @@ -105,7 +105,7 @@ export class JwtHelperService { * @returns access token */ getAccessToken(): string { - return this.storageService.getItem(JwtHelperService.USER_ACCESS_TOKEN); + return this.jwtStorage.getItem(JwtHelperService.USER_ACCESS_TOKEN); } /** @@ -114,7 +114,7 @@ export class JwtHelperService { * @param key Key name of the field to retrieve * @returns Value from the token */ - getValueFromLocalIdToken(key: string): T { + getValueFromLocalIdToken(key: string): T { return this.getValueFromToken(this.getIdToken(), key); } @@ -123,8 +123,8 @@ export class JwtHelperService { * * @returns id token */ - getIdToken(): string { - return this.storageService.getItem(JwtHelperService.USER_ID_TOKEN); + getIdToken(): string { + return this.jwtStorage.getItem(JwtHelperService.USER_ID_TOKEN); } /** @@ -186,7 +186,7 @@ export class JwtHelperService { * @param rolesToCheck List of role names to check * @returns True if it contains at least one of the given roles, false otherwise */ - hasRealmRoles(rolesToCheck: string []): boolean { + hasRealmRoles(rolesToCheck: string[]): boolean { return rolesToCheck.some((currentRole) => this.hasRealmRole(currentRole)); } @@ -197,7 +197,7 @@ export class JwtHelperService { * @param rolesToCheck List of role names to check * @returns True if it contains at least one of the given roles, false otherwise */ - hasRealmRolesForClientRole(clientName: string, rolesToCheck: string []): boolean { + hasRealmRolesForClientRole(clientName: string, rolesToCheck: string[]): boolean { return rolesToCheck.some((currentRole) => this.hasClientRole(clientName, currentRole)); } diff --git a/lib/core/src/lib/auth/services/user-access.service.spec.ts b/lib/core/src/lib/auth/services/user-access.service.spec.ts index bde2e79579..64360b595f 100644 --- a/lib/core/src/lib/auth/services/user-access.service.spec.ts +++ b/lib/core/src/lib/auth/services/user-access.service.spec.ts @@ -17,9 +17,10 @@ import { TestBed } from '@angular/core/testing'; import { UserAccessService } from './user-access.service'; -import { JwtHelperService } from './jwt-helper.service'; +import { JWT_STORAGE_SERVICE, JwtHelperService } from './jwt-helper.service'; import { AppConfigService } from '../../app-config'; import { HttpClientTestingModule } from '@angular/common/http/testing'; +import { StorageService } from '../../common'; describe('UserAccessService', () => { let userAccessService: UserAccessService; @@ -29,7 +30,7 @@ describe('UserAccessService', () => { beforeEach(() => { TestBed.configureTestingModule({ imports: [HttpClientTestingModule], - providers: [UserAccessService] + providers: [UserAccessService, { provide: JWT_STORAGE_SERVICE, useClass: StorageService }] }); userAccessService = TestBed.inject(UserAccessService); jwtHelperService = TestBed.inject(JwtHelperService); diff --git a/lib/core/src/lib/core.module.ts b/lib/core/src/lib/core.module.ts index fcc5acac55..d0b14f2f12 100644 --- a/lib/core/src/lib/core.module.ts +++ b/lib/core/src/lib/core.module.ts @@ -56,6 +56,7 @@ import { DynamicChipListComponent } from './dynamic-chip-list'; import { IdentityUserInfoComponent } from './identity-user-info'; import { UnsavedChangesDialogComponent } from './dialogs'; import { MaterialModule } from './material.module'; +import { JWT_STORAGE_SERVICE } from './auth/services/jwt-helper.service'; @NgModule({ imports: [ @@ -150,7 +151,8 @@ export class CoreModule { useValue: { duration: 10000 } - } + }, + { provide: JWT_STORAGE_SERVICE, useExisting: StorageService } ] }; } diff --git a/lib/process-services-cloud/src/lib/process/process-filters/components/edit-process-filter-cloud.component.spec.ts b/lib/process-services-cloud/src/lib/process/process-filters/components/edit-process-filter-cloud.component.spec.ts index d94aa24dc8..c068c4eb96 100644 --- a/lib/process-services-cloud/src/lib/process/process-filters/components/edit-process-filter-cloud.component.spec.ts +++ b/lib/process-services-cloud/src/lib/process/process-filters/components/edit-process-filter-cloud.component.spec.ts @@ -16,7 +16,7 @@ */ import { AlfrescoApiService } from '@alfresco/adf-content-services'; -import { ADF_DATE_FORMATS, FullNamePipe, NoopTranslateModule, UserPreferencesService } from '@alfresco/adf-core'; +import { ADF_DATE_FORMATS, FullNamePipe, JWT_STORAGE_SERVICE, NoopTranslateModule, StorageService, UserPreferencesService } from '@alfresco/adf-core'; import { HarnessLoader } from '@angular/cdk/testing'; import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed'; import { SimpleChange } from '@angular/core'; @@ -121,7 +121,8 @@ describe('EditProcessFilterCloudComponent', () => { { provide: DateAdapter, useClass: DateFnsAdapter }, { provide: NotificationCloudService, useValue: { makeGQLQuery: () => of([]) } }, { provide: MAT_DATE_FORMATS, useValue: ADF_DATE_FORMATS }, - { provide: IDENTITY_USER_SERVICE_TOKEN, useExisting: IdentityUserServiceMock } + { provide: IDENTITY_USER_SERVICE_TOKEN, useExisting: IdentityUserServiceMock }, + { provide: JWT_STORAGE_SERVICE, useClass: StorageService } ], declarations: [PeopleCloudComponent, DateRangeFilterComponent] });