From 979732e0fa1997a35090c704cd96945dcabfa21f Mon Sep 17 00:00:00 2001 From: Marcel Eisentraut Date: Thu, 30 Mar 2023 17:43:11 +0200 Subject: [PATCH] feat: improve token endpoint handling * own service, which is responsible for all token handling with the ICM * rename ICMTokenEndpoint Service to TokenService and moved it files to core/services folder * create utility class to instantiate custom oauthService instance * adapt documentation for new authentication behavior closes: #1374 --- docs/concepts/authentication.md | 42 +++-- docs/guides/authentication_icm.md | 2 +- docs/guides/authentication_punchout.md | 2 +- docs/guides/migrations.md | 5 + src/app/core/identity-provider.module.ts | 26 +-- .../auth0.identity-provider.spec.ts | 27 ++- .../auth0.identity-provider.ts | 164 +++++++----------- .../icm.identity-provider.spec.ts | 10 +- .../icm.identity-provider.ts | 52 ++---- .../core/services/token/token.service.spec.ts | 59 +++++++ src/app/core/services/token/token.service.ts | 102 +++++++++++ .../core/services/user/user.service.spec.ts | 15 +- src/app/core/services/user/user.service.ts | 37 +--- .../store/customer/customer-store.spec.ts | 16 +- .../store/customer/user/user.effects.spec.ts | 16 +- .../core/store/customer/user/user.effects.ts | 19 +- .../core/utils/api-token/api-token.service.ts | 15 +- src/app/core/utils/instance-creators.ts | 9 + .../oauth-configuration.service.spec.ts | 75 -------- .../oauth-configuration.service.ts | 50 ------ .../punchout-identity-provider.spec.ts | 8 +- .../punchout-identity-provider.ts | 150 ++++++---------- 22 files changed, 388 insertions(+), 513 deletions(-) create mode 100644 src/app/core/services/token/token.service.spec.ts create mode 100644 src/app/core/services/token/token.service.ts create mode 100644 src/app/core/utils/instance-creators.ts delete mode 100644 src/app/core/utils/oauth-configuration/oauth-configuration.service.spec.ts delete mode 100644 src/app/core/utils/oauth-configuration/oauth-configuration.service.ts diff --git a/docs/concepts/authentication.md b/docs/concepts/authentication.md index d992f636ba..3611df9cf2 100644 --- a/docs/concepts/authentication.md +++ b/docs/concepts/authentication.md @@ -20,8 +20,7 @@ The following identity providers are supported: The default [ICM server](../guid There is a lot of functionality related to authentication, e.g., logging a user in and out, registering a new user, keeping the user identified even if the user opens further browser tabs, etc. The PWA uses the library [angular-oauth2-oidc](https://github.com/manfredsteyer/angular-oauth2-oidc#readme) to support the implementation of these functionalities. -It can be configured to provide access to identity providers. -You can find the initialization of this library in the [oauth-configuration-service.ts](../../src/app/shared/../core/utils/oauth-configuration/oauth-configuration.service.ts). +It is used to fetching data from the the [icm token endpoint service](../../src/app/core/services/token/token.service.ts) and can be configured to provide access to other identity providers. ## Implementation and Configuration of Identity Providers @@ -31,31 +30,36 @@ To add or change the functionality of an identity provider, the following steps In the following code you see a typical implementation of the init method of an IdP class. - Note that all authentication-related functionality must not be executed before the oAuth service has been configured. - ```typescript @Injectable({ providedIn: 'root' }) export class ExampleIdentityProvider implements IdentityProvider { - private configured$ = new BehaviorSubject(false); - - constructor(private oAuthService: OAuthService, private configService: OAuthConfigurationService) {} + constructor( + private router: Router, + private apiTokenService: ApiTokenService, + private accountFacade: AccountFacade + ) {} init() { - this.configService.config$.subscribe(config => { - this.oAuthService.configure(config); - this.configured.next(true); + this.apiTokenService.restore$().subscribe(noop); + + this.apiTokenService.cookieVanishes$.subscribe(([type]) => { + this.accountFacade.logoutUser({ revokeApiToken: false }); + if (type === 'user') { + this.router.navigate(['/login'], { + queryParams: { returnUrl: this.router.url, messageKey: 'session_timeout' }, + }); + } }); - - this.configured - .pipe( - whenTruthy(), - switchMap(() => from(this.oAuthService.fetchTokenUsingGrant('anonymous'))) - ) - .subscribe(); } } ``` + > **Note** + > + > If a identity provider is using the OAuthService for authentication, then the identity provider have to inject the OAuthService with a new instance. + > Otherwise difficult side effects with the [TokenService](../../src/app/core/services/token/token.service.ts) will occur. + > Please checkout the [Auth0IdentityProvider](../../src/app/core/identity-provider/auth0.identity-provider.ts) for an example. + 2. Register the `.identity-provider.ts` in the [`IdentityProviderModule`](../../src/app/core/identity-provider.module.ts). The `APP_INITIALIZER` injection token is used to configure and initialize the identity provider before app initialization. 3. Set the environment variables `IdentityProviders` and `IdentityProvider` accordingly. @@ -63,8 +67,8 @@ To add or change the functionality of an identity provider, the following steps ## PWA Initialization A PWA user has to be identified by the ICM server by a unique authentication token, even if it is an anonymous user. -Once a user opens the PWA for the first time, an authentication token is requested by the [ICM Token REST endpoint](https://support.intershop.com/kb/index.php?c=Display&q1=U29770&q2=Text). -This happens in the [`init()`](../../src/app/core/identity-provider/icm.identity-provider.ts) method of the active identity provider. +Once a unknown user create a basket in the PWA, an anonymous authentication token is requested by the [ICM Token REST endpoint](https://support.intershop.com/kb/index.php?c=Display&q1=U29770&q2=Text). +This happens in the [`apiToken http interceptor`](../../src/app/core/utils/api-token/api-token.service.ts) method. Subsequently, this token will be saved as `apiToken` cookie and added to all REST requests in the request header, e.g.: ```typescript diff --git a/docs/guides/authentication_icm.md b/docs/guides/authentication_icm.md index 51b07a1c6f..1fad0f419e 100644 --- a/docs/guides/authentication_icm.md +++ b/docs/guides/authentication_icm.md @@ -29,7 +29,7 @@ Afterwards, the authentication token is requested from the server and the user w Each authentication token has a predefined lifetime. That means, the token has to be refreshed to prevent it from expiring. Once 75% of the token's lifetime have passed (this time can be configured in the oAuth library), an info event is emitted. -This event is used to call the [refresh mechanism `setupRefreshTokenMechanism$`](../../src/app/core/utils/oauth-configuration/oauth-configuration.service.ts) of the oAuth configuration service and the authentication token will be renewed. +This event is used to call the [refresh mechanism `setupRefreshTokenMechanism$`](../../src/app/core/services/token/token.service.ts) of the oAuth configuration service and the authentication token will be renewed. Hence, the token will not expire as long as the user keeps the PWA open in the browser. ## Logout diff --git a/docs/guides/authentication_punchout.md b/docs/guides/authentication_punchout.md index 1444f9330b..8de698c63d 100644 --- a/docs/guides/authentication_punchout.md +++ b/docs/guides/authentication_punchout.md @@ -67,7 +67,7 @@ There is currently no possibility to register a new punchout user in the PWA. Each authentication token has a predefined lifetime. That means, the token has to be refreshed to prevent it from expiring. Once 75% of the token's lifetime have passed ( this time can be configured in the oAuth library), an info event is emitted. -This event is used to call the [refresh mechanism `setupRefreshTokenMechanism$`](../../src/app/core/utils/oauth-configuration/oauth-configuration.service.ts) of the oAuth configuration service and the authentication token will be renewed. +This event is used to call the [refresh mechanism `setupRefreshTokenMechanism$`](../../src/app/core/services/token/token.service.ts) of the oAuth configuration service and the authentication token will be renewed. Hence, the token will not expire as long as the user keeps the PWA open in the browser. ## Logout diff --git a/docs/guides/migrations.md b/docs/guides/migrations.md index 977d9f9a74..65eada69f6 100644 --- a/docs/guides/migrations.md +++ b/docs/guides/migrations.md @@ -27,6 +27,11 @@ Existing projects that do not want to use a configurable theme do not need to ap To use the new [configurable theme](./themes.md#configurable-theme) feature, the feature toggle `extraConfiguration` needs to be enabled. +A new `TokenService` is introduced to be only responsible for fetching token data from the ICM. +However all necessary adaptions for the identity providers and the `fetchToken()` method of the UserService are removed in order to be completely independent of `TokenService`. +If your identity providers should use the `OAuthService` to handle the authentication, please make sure to instantiate a new `OAuthService` entity within the identity provider. +The `getOAuthServiceInstance()` static method from the `InstanceCreators` class can be used for that. + ## 3.3 to 4.0 The Intershop PWA now uses Node.js 18.15.0 LTS with the corresponding npm version 9.5.0 and the `"lockfileVersion": 3,`. diff --git a/src/app/core/identity-provider.module.ts b/src/app/core/identity-provider.module.ts index a5f9ebe2bd..1686259cb4 100644 --- a/src/app/core/identity-provider.module.ts +++ b/src/app/core/identity-provider.module.ts @@ -1,7 +1,7 @@ import { HttpHandler, HttpRequest } from '@angular/common/http'; -import { APP_INITIALIZER, ModuleWithProviders, NgModule } from '@angular/core'; +import { ModuleWithProviders, NgModule } from '@angular/core'; import { OAuthModule, OAuthStorage } from 'angular-oauth2-oidc'; -import { BehaviorSubject, noop, of, race, timer } from 'rxjs'; +import { noop } from 'rxjs'; import { PunchoutIdentityProviderModule } from '../extensions/punchout/identity-provider/punchout-identity-provider.module'; @@ -9,7 +9,6 @@ import { Auth0IdentityProvider } from './identity-provider/auth0.identity-provid import { ICMIdentityProvider } from './identity-provider/icm.identity-provider'; import { IDENTITY_PROVIDER_IMPLEMENTOR, IdentityProviderFactory } from './identity-provider/identity-provider.factory'; import { IdentityProviderCapabilities } from './identity-provider/identity-provider.interface'; -import { OAuthConfigurationService } from './utils/oauth-configuration/oauth-configuration.service'; /** * provider factory for storage @@ -21,23 +20,9 @@ export function storageFactory(): OAuthStorage { } } -/** - * load configuration object for OAuth Service - * OAuth Service should be configured, when app is initialized - */ -function loadOAuthConfig(configService: OAuthConfigurationService) { - return () => race(configService.loadConfig$, timer(4000)); -} - @NgModule({ imports: [OAuthModule.forRoot({ resourceServer: { sendAccessToken: false } }), PunchoutIdentityProviderModule], providers: [ - { - provide: APP_INITIALIZER, - useFactory: loadOAuthConfig, - deps: [OAuthConfigurationService], - multi: true, - }, { provide: OAuthStorage, useFactory: storageFactory }, { provide: IDENTITY_PROVIDER_IMPLEMENTOR, @@ -78,13 +63,6 @@ export class IdentityProviderModule { getType: () => 'ICM', }, }, - { - provide: OAuthConfigurationService, - useValue: { - loadConfig$: of({}), - config$: new BehaviorSubject({}), - }, - }, ], }; } diff --git a/src/app/core/identity-provider/auth0.identity-provider.spec.ts b/src/app/core/identity-provider/auth0.identity-provider.spec.ts index ad3dbc4c5e..9925918a1f 100644 --- a/src/app/core/identity-provider/auth0.identity-provider.spec.ts +++ b/src/app/core/identity-provider/auth0.identity-provider.spec.ts @@ -4,7 +4,7 @@ import { Router } from '@angular/router'; import { RouterTestingModule } from '@angular/router/testing'; import { MockStore, provideMockStore } from '@ngrx/store/testing'; import { OAuthService } from 'angular-oauth2-oidc'; -import { BehaviorSubject, of } from 'rxjs'; +import { of } from 'rxjs'; import { anything, capture, instance, mock, resetCalls, spy, verify, when } from 'ts-mockito'; import { Customer } from 'ish-core/models/customer/customer.model'; @@ -13,7 +13,7 @@ import { ApiService } from 'ish-core/services/api/api.service'; import { getSsoRegistrationCancelled, getSsoRegistrationRegistered } from 'ish-core/store/customer/sso-registration'; import { getLoggedInCustomer, getUserAuthorized, getUserLoading } from 'ish-core/store/customer/user'; import { ApiTokenService } from 'ish-core/utils/api-token/api-token.service'; -import { OAuthConfigurationService } from 'ish-core/utils/oauth-configuration/oauth-configuration.service'; +import { InstanceCreators } from 'ish-core/utils/instance-creators'; import { Auth0Config, Auth0IdentityProvider } from './auth0.identity-provider'; @@ -36,7 +36,7 @@ describe('Auth0 Identity Provider', () => { const oAuthService = mock(OAuthService); const apiService = mock(ApiService); const apiTokenService = mock(ApiTokenService); - const oAuthConfigurationService = mock(OAuthConfigurationService); + const instanceCreators = spy(InstanceCreators); let auth0IdentityProvider: Auth0IdentityProvider; let store$: MockStore; let storeSpy$: MockStore; @@ -57,20 +57,10 @@ describe('Auth0 Identity Provider', () => { { provide: ApiService, useFactory: () => instance(apiService) }, { provide: ApiTokenService, useFactory: () => instance(apiTokenService) }, { provide: APP_BASE_HREF, useValue: baseHref }, - { provide: OAuthConfigurationService, useFactory: () => instance(oAuthConfigurationService) }, - { provide: OAuthService, useFactory: () => instance(oAuthService) }, provideMockStore(), ], }).compileComponents(); - auth0IdentityProvider = TestBed.inject(Auth0IdentityProvider); - router = TestBed.inject(Router); - store$ = TestBed.inject(MockStore); - storeSpy$ = spy(store$); - }); - - beforeEach(() => { - when(apiTokenService.restore$(anything(), anything())).thenReturn(of(true)); when(oAuthService.getIdToken()).thenReturn(idToken); when(oAuthService.loadDiscoveryDocumentAndTryLogin()).thenReturn( new Promise((res, _) => { @@ -78,7 +68,16 @@ describe('Auth0 Identity Provider', () => { }) ); when(oAuthService.state).thenReturn(undefined); - when(oAuthConfigurationService.config$).thenReturn(new BehaviorSubject({})); + when(instanceCreators.getOAuthServiceInstance(anything())).thenReturn(instance(oAuthService)); + + auth0IdentityProvider = TestBed.inject(Auth0IdentityProvider); + router = TestBed.inject(Router); + store$ = TestBed.inject(MockStore); + storeSpy$ = spy(store$); + }); + + beforeEach(() => { + when(apiTokenService.restore$(anything())).thenReturn(of(true)); when(apiService.post(anything(), anything())).thenReturn(of(userData)); }); diff --git a/src/app/core/identity-provider/auth0.identity-provider.ts b/src/app/core/identity-provider/auth0.identity-provider.ts index de279d2230..600bab7b79 100644 --- a/src/app/core/identity-provider/auth0.identity-provider.ts +++ b/src/app/core/identity-provider/auth0.identity-provider.ts @@ -1,10 +1,10 @@ import { APP_BASE_HREF } from '@angular/common'; import { HttpEvent, HttpHandler, HttpRequest } from '@angular/common/http'; -import { Inject, Injectable } from '@angular/core'; +import { Inject, Injectable, Injector } from '@angular/core'; import { ActivatedRouteSnapshot, Router } from '@angular/router'; import { Store, select } from '@ngrx/store'; import { OAuthService } from 'angular-oauth2-oidc'; -import { BehaviorSubject, Observable, combineLatest, from, of, race, timer } from 'rxjs'; +import { Observable, combineLatest, from, of, race, timer } from 'rxjs'; import { catchError, filter, first, map, switchMap, take, tap } from 'rxjs/operators'; import { HttpError } from 'ish-core/models/http-error/http-error.model'; @@ -18,8 +18,8 @@ import { loadUserByAPIToken, } from 'ish-core/store/customer/user'; import { ApiTokenService } from 'ish-core/utils/api-token/api-token.service'; -import { OAuthConfigurationService } from 'ish-core/utils/oauth-configuration/oauth-configuration.service'; -import { delayUntil, whenTruthy } from 'ish-core/utils/operators'; +import { InstanceCreators } from 'ish-core/utils/instance-creators'; +import { whenTruthy } from 'ish-core/utils/operators'; import { IdentityProvider, IdentityProviderCapabilities, TriggerReturnType } from './identity-provider.interface'; @@ -33,17 +33,18 @@ export interface Auth0Config { export class Auth0IdentityProvider implements IdentityProvider { // emits true, when OAuth Service is successfully configured // used as an additional condition to check that the OAuth Service is configured before OAuth Service actions are used - private oAuthServiceConfigured$ = new BehaviorSubject(false); + private oauthService: OAuthService; constructor( private apiService: ApiService, private store: Store, private router: Router, private apiTokenService: ApiTokenService, - private oauthService: OAuthService, - private configService: OAuthConfigurationService, + parent: Injector, @Inject(APP_BASE_HREF) private baseHref: string - ) {} + ) { + this.oauthService = InstanceCreators.getOAuthServiceInstance(parent); + } getCapabilities(): IdentityProviderCapabilities { return { @@ -57,77 +58,62 @@ export class Auth0IdentityProvider implements IdentityProvider { const effectiveOrigin = this.baseHref === '/' ? window.location.origin : window.location.origin + this.baseHref; // use internal OAuth configuration service for tokenEndpoint configuration - this.configService.config$.pipe(whenTruthy(), take(1)).subscribe(serviceConf => { - this.oauthService.configure({ - // Your Auth0 app's domain - // Important: Don't forget to start with https:// AND the trailing slash! - issuer: `https://${config.domain}/`, - - // The app's clientId configured in Auth0 - clientId: config.clientID, - - // The app's redirectUri configured in Auth0 - redirectUri: `${effectiveOrigin}/loading`, + this.oauthService.configure({ + // Your Auth0 app's domain + // Important: Don't forget to start with https:// AND the trailing slash! + issuer: `https://${config.domain}/`, - // logout redirect URL - postLogoutRedirectUri: effectiveOrigin, + // The app's clientId configured in Auth0 + clientId: config.clientID, - // Scopes ("rights") the Angular application wants get delegated - // https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims - scope: 'openid email profile offline_access', + // The app's redirectUri configured in Auth0 + redirectUri: `${effectiveOrigin}/loading`, - // Using Authorization Code Flow - // (PKCE is activated by default for authorization code flow) - responseType: 'code', + // logout redirect URL + postLogoutRedirectUri: effectiveOrigin, - // Your Auth0 account's logout url - // Derive it from your application's domain - logoutUrl: `https://${config.domain}/v2/logout`, + // Scopes ("rights") the Angular application wants get delegated + // https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims + scope: 'openid email profile offline_access', - sessionChecksEnabled: true, + // Using Authorization Code Flow + // (PKCE is activated by default for authorization code flow) + responseType: 'code', - // ICM token endpoint to retrieve a valid token for an anonymous user - tokenEndpoint: serviceConf?.tokenEndpoint, + // Your Auth0 account's logout url + // Derive it from your application's domain + logoutUrl: `https://${config.domain}/v2/logout`, - requireHttps: serviceConf?.requireHttps, - }); - this.oauthService.setupAutomaticSilentRefresh(); - this.oAuthServiceConfigured$.next(true); + sessionChecksEnabled: true, }); + this.oauthService.setupAutomaticSilentRefresh(); - // OAuth Service should be configured before apiToken information are restored - this.oAuthServiceConfigured$ + // anonymous user token should only be fetched when no user is logged in + this.apiTokenService + .restore$(['user', 'order'], !this.oauthService.getIdToken()) .pipe( - whenTruthy(), - take(1), + switchMap(() => from(this.oauthService.loadDiscoveryDocumentAndTryLogin())), switchMap(() => - // anonymous user token should only be fetched when no user is logged in - this.apiTokenService.restore$(['user', 'order'], !this.oauthService.getIdToken()).pipe( - delayUntil(this.oAuthServiceConfigured$), - switchMap(() => from(this.oauthService.loadDiscoveryDocumentAndTryLogin())), - switchMap(() => - timer(0, 200).pipe( - map(() => this.oauthService.getIdToken()), - take(100), - whenTruthy(), - take(1) - ) - ), + timer(0, 200).pipe( + map(() => this.oauthService.getIdToken()), + take(100), whenTruthy(), - switchMap(idToken => { - const inviteUserId = window.sessionStorage.getItem('invite-userid'); - const inviteHash = window.sessionStorage.getItem('invite-hash'); - return inviteUserId && inviteHash - ? this.inviteRegistration(idToken, inviteUserId, inviteHash).pipe( - tap(() => { - window.sessionStorage.removeItem('invite-userid'); - window.sessionStorage.removeItem('invite-hash'); - }) - ) - : this.normalSignInRegistration(idToken); - }) + take(1) ) - ) + ), + whenTruthy(), + switchMap(idToken => { + const inviteUserId = window.sessionStorage.getItem('invite-userid'); + const inviteHash = window.sessionStorage.getItem('invite-hash'); + return inviteUserId && inviteHash + ? this.inviteRegistration(idToken, inviteUserId, inviteHash).pipe( + tap(() => { + window.sessionStorage.removeItem('invite-userid'); + window.sessionStorage.removeItem('invite-hash'); + }) + ) + : this.normalSignInRegistration(idToken); + }) ) .subscribe(() => { this.apiTokenService.removeApiToken(); @@ -220,45 +206,27 @@ export class Auth0IdentityProvider implements IdentityProvider { if (route.queryParamMap.get('userid')) { return of(true); } else { - return this.oAuthServiceConfigured$.pipe( - whenTruthy(), - take(1), - tap(() => { - this.router.navigateByUrl('/loading'); - this.oauthService.loadDiscoveryDocumentAndLogin({ - state: route.queryParams.returnUrl, - }); - }) - ); + this.router.navigateByUrl('/loading'); + return this.oauthService.loadDiscoveryDocumentAndLogin({ + state: route.queryParams.returnUrl, + }); } } triggerLogin(route: ActivatedRouteSnapshot): TriggerReturnType { - return this.oAuthServiceConfigured$.pipe( - whenTruthy(), - take(1), - tap(() => { - this.router.navigateByUrl('/loading'); - this.oauthService.loadDiscoveryDocumentAndLogin({ - state: route.queryParams.returnUrl, - }); - }) - ); + this.router.navigateByUrl('/loading'); + return this.oauthService.loadDiscoveryDocumentAndLogin({ + state: route.queryParams.returnUrl, + }); } triggerInvite(route: ActivatedRouteSnapshot): TriggerReturnType { - return this.oAuthServiceConfigured$.pipe( - whenTruthy(), - take(1), - tap(() => { - this.router.navigateByUrl('/loading'); - window.sessionStorage.setItem('invite-userid', route.queryParams.uid); - window.sessionStorage.setItem('invite-hash', route.queryParams.Hash); - this.oauthService.loadDiscoveryDocumentAndLogin({ - state: route.queryParams.returnUrl, - }); - }) - ); + this.router.navigateByUrl('/loading'); + window.sessionStorage.setItem('invite-userid', route.queryParams.uid); + window.sessionStorage.setItem('invite-hash', route.queryParams.Hash); + return this.oauthService.loadDiscoveryDocumentAndLogin({ + state: route.queryParams.returnUrl, + }); } triggerLogout(): TriggerReturnType { diff --git a/src/app/core/identity-provider/icm.identity-provider.spec.ts b/src/app/core/identity-provider/icm.identity-provider.spec.ts index 8b3695e978..e558625b67 100644 --- a/src/app/core/identity-provider/icm.identity-provider.spec.ts +++ b/src/app/core/identity-provider/icm.identity-provider.spec.ts @@ -1,21 +1,18 @@ import { TestBed } from '@angular/core/testing'; import { Router, UrlTree } from '@angular/router'; import { MockStore, provideMockStore } from '@ngrx/store/testing'; -import { OAuthService } from 'angular-oauth2-oidc'; -import { BehaviorSubject, Observable, Subject, of } from 'rxjs'; +import { Observable, Subject, of } from 'rxjs'; import { anything, instance, mock, resetCalls, verify, when } from 'ts-mockito'; import { AccountFacade } from 'ish-core/facades/account.facade'; import { selectQueryParam } from 'ish-core/store/core/router'; import { ApiTokenService } from 'ish-core/utils/api-token/api-token.service'; -import { OAuthConfigurationService } from 'ish-core/utils/oauth-configuration/oauth-configuration.service'; import { ICMIdentityProvider } from './icm.identity-provider'; describe('Icm Identity Provider', () => { const apiTokenService = mock(ApiTokenService); const accountFacade = mock(AccountFacade); - const oAuthConfigurationService = mock(OAuthConfigurationService); let icmIdentityProvider: ICMIdentityProvider; let store$: MockStore; @@ -26,10 +23,6 @@ describe('Icm Identity Provider', () => { providers: [ { provide: AccountFacade, useFactory: () => instance(accountFacade) }, { provide: ApiTokenService, useFactory: () => instance(apiTokenService) }, - - { provide: OAuthConfigurationService, useFactory: () => instance(oAuthConfigurationService) }, - { provide: OAuthService, useFactory: () => instance(mock(OAuthService)) }, - provideMockStore(), ], }).compileComponents(); @@ -42,7 +35,6 @@ describe('Icm Identity Provider', () => { beforeEach(() => { when(apiTokenService.restore$()).thenReturn(of(true)); when(apiTokenService.cookieVanishes$).thenReturn(new Subject()); - when(oAuthConfigurationService.config$).thenReturn(new BehaviorSubject({})); resetCalls(apiTokenService); resetCalls(accountFacade); diff --git a/src/app/core/identity-provider/icm.identity-provider.ts b/src/app/core/identity-provider/icm.identity-provider.ts index dc567cd29b..6391976180 100644 --- a/src/app/core/identity-provider/icm.identity-provider.ts +++ b/src/app/core/identity-provider/icm.identity-provider.ts @@ -2,31 +2,22 @@ import { HttpEvent, HttpHandler, HttpRequest } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { ActivatedRouteSnapshot, Router } from '@angular/router'; import { Store, select } from '@ngrx/store'; -import { OAuthService } from 'angular-oauth2-oidc'; -import { BehaviorSubject, Observable, merge, noop } from 'rxjs'; -import { filter, map, switchMap, take, tap, withLatestFrom } from 'rxjs/operators'; +import { Observable, noop } from 'rxjs'; +import { filter, map, switchMap, take, withLatestFrom } from 'rxjs/operators'; import { AccountFacade } from 'ish-core/facades/account.facade'; import { selectQueryParam } from 'ish-core/store/core/router'; import { ApiTokenService } from 'ish-core/utils/api-token/api-token.service'; -import { OAuthConfigurationService } from 'ish-core/utils/oauth-configuration/oauth-configuration.service'; -import { whenTruthy } from 'ish-core/utils/operators'; import { IdentityProvider, TriggerReturnType } from './identity-provider.interface'; @Injectable({ providedIn: 'root' }) export class ICMIdentityProvider implements IdentityProvider { - // emits true, when OAuth Service is successfully configured - // used as an additional condition to check that the OAuth Service is configured before OAuth Service actions are used - private oAuthServiceConfigured$ = new BehaviorSubject(false); - constructor( private router: Router, private store: Store, private apiTokenService: ApiTokenService, - private accountFacade: AccountFacade, - private oAuthService: OAuthService, - private configService: OAuthConfigurationService + private accountFacade: AccountFacade ) {} getCapabilities() { @@ -38,11 +29,7 @@ export class ICMIdentityProvider implements IdentityProvider { } init() { - // OAuth Service should be configured by internal OAuth configuration service - this.configService.config$.pipe(whenTruthy(), take(1)).subscribe(config => { - this.oAuthService.configure(config); - this.oAuthServiceConfigured$.next(true); - }); + this.apiTokenService.restore$().subscribe(noop); this.apiTokenService.cookieVanishes$ .pipe(withLatestFrom(this.apiTokenService.apiToken$)) @@ -57,15 +44,6 @@ export class ICMIdentityProvider implements IdentityProvider { }); } }); - - // OAuth Service should be configured before apiToken information are restored and the refresh token mechanism is setup - this.oAuthServiceConfigured$ - .pipe( - whenTruthy(), - take(1), - switchMap(() => merge(this.apiTokenService.restore$(), this.configService.setupRefreshTokenMechanism$())) - ) - .subscribe(noop); } triggerLogin(): TriggerReturnType { @@ -73,22 +51,16 @@ export class ICMIdentityProvider implements IdentityProvider { } triggerLogout(): TriggerReturnType { - return this.oAuthServiceConfigured$.pipe( - whenTruthy(), + this.accountFacade.logoutUser(); + return this.accountFacade.isLoggedIn$.pipe( + // wait until the user is logged out before you go to homepage to prevent unnecessary REST calls + filter(loggedIn => !loggedIn), take(1), - tap(() => this.accountFacade.logoutUser()), // user will be logged out and related refresh token is revoked on server switchMap(() => - this.accountFacade.isLoggedIn$.pipe( - // wait until the user is logged out before you go to homepage to prevent unnecessary REST calls - filter(loggedIn => !loggedIn), - take(1), - switchMap(() => - this.store.pipe( - select(selectQueryParam('returnUrl')), - map(returnUrl => returnUrl || '/home'), - map(returnUrl => this.router.parseUrl(returnUrl)) - ) - ) + this.store.pipe( + select(selectQueryParam('returnUrl')), + map(returnUrl => returnUrl || '/home'), + map(returnUrl => this.router.parseUrl(returnUrl)) ) ) ); diff --git a/src/app/core/services/token/token.service.spec.ts b/src/app/core/services/token/token.service.spec.ts new file mode 100644 index 0000000000..4451569971 --- /dev/null +++ b/src/app/core/services/token/token.service.spec.ts @@ -0,0 +1,59 @@ +import { TestBed } from '@angular/core/testing'; +import { OAuthService, TokenResponse } from 'angular-oauth2-oidc'; +import { firstValueFrom, of } from 'rxjs'; +import { anyString, anything, instance, mock, spy, verify, when } from 'ts-mockito'; + +import { ApiService } from 'ish-core/services/api/api.service'; +import { ApiTokenService } from 'ish-core/utils/api-token/api-token.service'; +import { InstanceCreators } from 'ish-core/utils/instance-creators'; + +import { TokenService } from './token.service'; + +describe('Token Service', () => { + let tokenService: TokenService; + const apiService = mock(ApiService); + const oAuthService = mock(OAuthService); + const instanceCreators = spy(InstanceCreators); + + when(apiService.constructUrlForPath('token', anything())).thenReturn(of('https://icm-test-url.com/token')); + + when(oAuthService.fetchTokenUsingGrant(anyString(), anything(), anything())).thenReturn( + firstValueFrom(of({ access_token: 'access-token', expires_in: 123456 } as TokenResponse)) + ); + when(oAuthService.refreshToken()).thenResolve(); + when(oAuthService.getAccessToken()).thenReturn('access-token'); + when(oAuthService.getAccessTokenExpiration()).thenReturn(123456); + when(oAuthService.configure(anything())).thenResolve(); + when(oAuthService.events).thenReturn(of(undefined)); + + when(instanceCreators.getOAuthServiceInstance(anything())).thenReturn(instance(oAuthService)); + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [ + { + provide: ApiService, + useFactory: () => instance(apiService), + }, + { provide: ApiTokenService, useFactory: () => instance(mock(ApiTokenService)) }, + ], + }).compileComponents(); + + tokenService = TestBed.inject(TokenService); + }); + + describe('constructor', () => { + it('should configure oAuthService', () => { + verify(oAuthService.configure(anything())).once(); + }); + }); + + describe('fetchToken', () => { + it('should call the fetchTokenUsingGrant method', done => { + tokenService.fetchToken('anonymous').subscribe(() => { + verify(oAuthService.fetchTokenUsingGrant(anyString(), anything(), anything())).once(); + done(); + }); + }); + }); +}); diff --git a/src/app/core/services/token/token.service.ts b/src/app/core/services/token/token.service.ts new file mode 100644 index 0000000000..14376b4b03 --- /dev/null +++ b/src/app/core/services/token/token.service.ts @@ -0,0 +1,102 @@ +import { HttpHeaders } from '@angular/common/http'; +import { Injectable, Injector } from '@angular/core'; +import { AuthConfig, OAuthInfoEvent, OAuthService, OAuthSuccessEvent, TokenResponse } from 'angular-oauth2-oidc'; +import { BehaviorSubject, Observable, filter, first, from, map, noop, switchMap, take } from 'rxjs'; + +import { FetchTokenOptions, GrantType } from 'ish-core/models/token/token.interface'; +import { ApiService } from 'ish-core/services/api/api.service'; +import { ApiTokenService } from 'ish-core/utils/api-token/api-token.service'; +import { InstanceCreators } from 'ish-core/utils/instance-creators'; +import { whenTruthy } from 'ish-core/utils/operators'; + +@Injectable({ + providedIn: 'root', +}) +export class TokenService { + private oAuthService: OAuthService; + private serviceConfigured$ = new BehaviorSubject(false); + + constructor(private apiService: ApiService, private apiTokenService: ApiTokenService, parent: Injector) { + this.oAuthService = InstanceCreators.getOAuthServiceInstance(parent); + + this.apiService + .constructUrlForPath('token', { + sendCurrency: true, + sendLocale: true, + }) + .pipe( + whenTruthy(), + filter(url => !url.startsWith('/')), // url should not be relative + take(1), + map(url => ({ + tokenEndpoint: url, + requireHttps: url.startsWith('https'), + timeoutFactor: 0.5, + })) + ) + .subscribe(conf => { + this.oAuthService.configure(conf); + this.serviceConfigured$.next(true); + }); + + this.setApiTokenCookie$().subscribe(noop); + this.setupRefreshTokenMechanism$().subscribe(noop); + } + + /** + * Fetches a new user token. Based on the grantType the user has to apply certain token options to the method. + * + * @param grantType The given type ('anonymous', 'password', 'client_credentials', 'refresh_token') is used to specify, how the user token should be fetched. + */ + fetchToken(grantType: T): Observable; + fetchToken>(grantType: T, options: R): Observable; + fetchToken>( + grantType: T, + options?: R + ): Observable { + return this.serviceConfigured$.pipe( + whenTruthy(), + first(), + switchMap(() => + from( + this.oAuthService?.fetchTokenUsingGrant( + grantType, + options ?? {}, + new HttpHeaders({ 'content-type': 'application/x-www-form-urlencoded' }) + ) + ) + ) + ); + } + + /** + * Refresh existing tokens, when token is about to expire + * + * @returns {TokenResponse} updated tokens + */ + private setupRefreshTokenMechanism$(): Observable { + return this.serviceConfigured$.pipe( + whenTruthy(), + first(), + switchMap(() => + this.oAuthService.events.pipe( + filter( + event => event instanceof OAuthInfoEvent && event.type === 'token_expires' && event.info === 'access_token' + ), + switchMap(() => from(this.oAuthService.refreshToken())) + ) + ) + ); + } + + private setApiTokenCookie$() { + return this.oAuthService.events.pipe( + filter(event => event instanceof OAuthSuccessEvent && event.type === 'token_received'), + map(() => + this.apiTokenService.setApiToken(this.oAuthService.getAccessToken(), { + expires: new Date(this.oAuthService.getAccessTokenExpiration()), + }) + ) + ); + } +} diff --git a/src/app/core/services/user/user.service.spec.ts b/src/app/core/services/user/user.service.spec.ts index d77b706b2d..b068ae59c1 100644 --- a/src/app/core/services/user/user.service.spec.ts +++ b/src/app/core/services/user/user.service.spec.ts @@ -1,6 +1,6 @@ import { TestBed } from '@angular/core/testing'; import { MockStore, provideMockStore } from '@ngrx/store/testing'; -import { OAuthService, TokenResponse } from 'angular-oauth2-oidc'; +import { TokenResponse } from 'angular-oauth2-oidc'; import { of, throwError } from 'rxjs'; import { anyString, anything, capture, instance, mock, verify, when } from 'ts-mockito'; @@ -11,6 +11,7 @@ import { CustomerData } from 'ish-core/models/customer/customer.interface'; import { Customer, CustomerRegistrationType, CustomerUserType } from 'ish-core/models/customer/customer.model'; import { User } from 'ish-core/models/user/user.model'; import { ApiService, AvailableOptions } from 'ish-core/services/api/api.service'; +import { TokenService } from 'ish-core/services/token/token.service'; import { getUserPermissions } from 'ish-core/store/customer/authorization'; import { getLoggedInCustomer, getLoggedInUser } from 'ish-core/store/customer/user'; import { ApiTokenService } from 'ish-core/utils/api-token/api-token.service'; @@ -30,7 +31,7 @@ describe('User Service', () => { let userService: UserService; let apiServiceMock: ApiService; let apiTokenServiceMock: ApiTokenService; - let oAuthServiceMock: OAuthService; + let tokenServiceMock: TokenService; let appFacade: AppFacade; let store$: MockStore; @@ -38,9 +39,9 @@ describe('User Service', () => { apiServiceMock = mock(ApiService); apiTokenServiceMock = mock(ApiTokenService); appFacade = mock(AppFacade); - oAuthServiceMock = mock(OAuthService); + tokenServiceMock = mock(TokenService); - when(oAuthServiceMock.fetchTokenUsingGrant(anyString(), anything(), anything())).thenResolve(token); + when(tokenServiceMock.fetchToken(anyString(), anything())).thenReturn(of(token)); when(appFacade.isAppTypeREST$).thenReturn(of(true)); when(appFacade.currentLocale$).thenReturn(of('en_US')); when(appFacade.customerRestResource$).thenReturn(of('customers')); @@ -50,7 +51,7 @@ describe('User Service', () => { { provide: ApiService, useFactory: () => instance(apiServiceMock) }, { provide: ApiTokenService, useFactory: () => instance(apiTokenServiceMock) }, { provide: AppFacade, useFactory: () => instance(appFacade) }, - { provide: OAuthService, useFactory: () => instance(oAuthServiceMock) }, + { provide: TokenService, useFactory: () => instance(tokenServiceMock) }, provideMockStore({ selectors: [{ selector: getLoggedInCustomer, value: undefined }] }), ], }); @@ -71,7 +72,7 @@ describe('User Service', () => { when(apiServiceMock.get('personalization', anything())).thenReturn(of({ pgid: '6FGMJtFU2xuRpG9I3CpTS7fc0000' })); userService.signInUser(loginDetail).subscribe(data => { - verify(oAuthServiceMock.fetchTokenUsingGrant(anyString(), anything(), anything())).once(); + verify(tokenServiceMock.fetchToken(anyString(), anything())).once(); expect(data).toHaveProperty('customer.customerNo', 'PC'); expect(data).toHaveProperty('pgid', '6FGMJtFU2xuRpG9I3CpTS7fc0000'); done(); @@ -88,7 +89,7 @@ describe('User Service', () => { when(apiServiceMock.get('personalization', anything())).thenReturn(of({ pgid: '6FGMJtFU2xuRpG9I3CpTS7fc0000' })); userService.signInUser(undefined).subscribe(data => { - verify(oAuthServiceMock.fetchTokenUsingGrant(anyString(), anything(), anything())).never(); + verify(tokenServiceMock.fetchToken(anyString(), anything())).never(); expect(data).toHaveProperty('customer.customerNo', 'PC'); expect(data).toHaveProperty('pgid', '6FGMJtFU2xuRpG9I3CpTS7fc0000'); done(); diff --git a/src/app/core/services/user/user.service.ts b/src/app/core/services/user/user.service.ts index 0ce951adef..1b30250f22 100644 --- a/src/app/core/services/user/user.service.ts +++ b/src/app/core/services/user/user.service.ts @@ -1,9 +1,8 @@ import { HttpHeaders } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { Store, select } from '@ngrx/store'; -import { OAuthService, TokenResponse } from 'angular-oauth2-oidc'; import { pick } from 'lodash-es'; -import { Observable, combineLatest, defer, forkJoin, from, of, throwError } from 'rxjs'; +import { Observable, combineLatest, defer, forkJoin, of, throwError } from 'rxjs'; import { concatMap, first, map, switchMap, take, tap, withLatestFrom } from 'rxjs/operators'; import { AppFacade } from 'ish-core/facades/app.facade'; @@ -20,11 +19,11 @@ import { } from 'ish-core/models/customer/customer.model'; import { PasswordReminderUpdate } from 'ish-core/models/password-reminder-update/password-reminder-update.model'; import { PasswordReminder } from 'ish-core/models/password-reminder/password-reminder.model'; -import { FetchTokenOptions, GrantType } from 'ish-core/models/token/token.interface'; import { UserCostCenter } from 'ish-core/models/user-cost-center/user-cost-center.model'; import { UserMapper } from 'ish-core/models/user/user.mapper'; import { User } from 'ish-core/models/user/user.model'; import { ApiService, AvailableOptions, unpackEnvelope } from 'ish-core/services/api/api.service'; +import { TokenService } from 'ish-core/services/token/token.service'; import { getUserPermissions } from 'ish-core/store/customer/authorization'; import { getLoggedInCustomer, getLoggedInUser } from 'ish-core/store/customer/user'; import { ApiTokenService } from 'ish-core/utils/api-token/api-token.service'; @@ -58,7 +57,7 @@ export class UserService { private apiTokenService: ApiTokenService, private appFacade: AppFacade, private store: Store, - private oauthService: OAuthService + private tokenService: TokenService ) {} /** @@ -72,9 +71,9 @@ export class UserService { signInUser(loginCredentials: Credentials): Observable { return defer(() => loginCredentials - ? this.fetchToken('password', { username: loginCredentials.login, password: loginCredentials.password }).pipe( - switchMap(() => this.fetchCustomer()) - ) + ? this.tokenService + .fetchToken('password', { username: loginCredentials.login, password: loginCredentials.password }) + .pipe(switchMap(() => this.fetchCustomer())) : this.fetchCustomer() ); } @@ -89,7 +88,9 @@ export class UserService { */ signInUserByToken(token?: string): Observable { if (token) { - return this.fetchToken('refresh_token', { refresh_token: token }).pipe(switchMap(() => this.fetchCustomer())); + return this.tokenService + .fetchToken('refresh_token', { refresh_token: token }) + .pipe(switchMap(() => this.fetchCustomer())); } else { return this.fetchCustomer({ skipApiErrorHandling: true }); } @@ -110,26 +111,6 @@ export class UserService { ); } - /** - * Fetches a new user token. Based on the grantType the user has to apply certain token options to the method. - * - * @param grantType The given type ('anonymous', 'password', 'client_credentials', 'refresh_token') is used to specify, how the user token should be fetched. - */ - fetchToken(grantType: T): Observable; - fetchToken>(grantType: T, options: R): Observable; - fetchToken>( - grantType: T, - options?: R - ): Observable { - return from( - this.oauthService.fetchTokenUsingGrant( - grantType, - options ?? {}, - new HttpHeaders({ 'content-type': 'application/x-www-form-urlencoded' }) - ) - ); - } - /** * Creates a new user for the given data. * diff --git a/src/app/core/store/customer/customer-store.spec.ts b/src/app/core/store/customer/customer-store.spec.ts index bb98cf117c..3b89194c35 100644 --- a/src/app/core/store/customer/customer-store.spec.ts +++ b/src/app/core/store/customer/customer-store.spec.ts @@ -1,9 +1,9 @@ import { TestBed } from '@angular/core/testing'; import { RouterTestingModule } from '@angular/router/testing'; import { TranslateModule } from '@ngx-translate/core'; -import { OAuthService, TokenResponse } from 'angular-oauth2-oidc'; +import { OAuthService } from 'angular-oauth2-oidc'; import { EMPTY, of } from 'rxjs'; -import { anyNumber, anyString, anything, instance, mock, when } from 'ts-mockito'; +import { anyNumber, anything, instance, mock, when } from 'ts-mockito'; import { Basket } from 'ish-core/models/basket/basket.model'; import { Credentials } from 'ish-core/models/credentials/credentials.model'; @@ -26,6 +26,7 @@ import { PricesService } from 'ish-core/services/prices/prices.service'; import { ProductsService } from 'ish-core/services/products/products.service'; import { PromotionsService } from 'ish-core/services/promotions/promotions.service'; import { SuggestService } from 'ish-core/services/suggest/suggest.service'; +import { TokenService } from 'ish-core/services/token/token.service'; import { UserService } from 'ish-core/services/user/user.service'; import { CoreStoreModule } from 'ish-core/store/core/core-store.module'; import { CustomerStoreModule } from 'ish-core/store/customer/customer-store.module'; @@ -107,14 +108,6 @@ describe('Customer Store', () => { useExternalUrl: false, } as Promotion; - const token = { - access_token: 'DEMO@access-token', - token_type: 'user', - expires_in: 3600, - refresh_token: 'DEMO@refresh-token', - id_token: 'DEMO@id-token', - } as TokenResponse; - beforeEach(() => { const categoriesServiceMock = mock(CategoriesService); when(categoriesServiceMock.getTopLevelCategories(anyNumber())).thenReturn(of(categoryTree())); @@ -149,7 +142,6 @@ describe('Customer Store', () => { const userServiceMock = mock(UserService); when(userServiceMock.signInUser(anything())).thenReturn(of({ customer, user, pgid })); - when(userServiceMock.fetchToken(anyString(), anything())).thenReturn(of(token)); const dataRequestsServiceMock = mock(DataRequestsService); const filterServiceMock = mock(FilterService); @@ -187,13 +179,13 @@ describe('Customer Store', () => { { provide: CookiesService, useFactory: () => instance(mock(CookiesService)) }, { provide: DataRequestsService, useFactory: () => instance(dataRequestsServiceMock) }, { provide: FilterService, useFactory: () => instance(filterServiceMock) }, - { provide: OAuthService, useFactory: () => instance(oAuthService) }, { provide: OrderService, useFactory: () => instance(orderServiceMock) }, { provide: PaymentService, useFactory: () => instance(mock(PaymentService)) }, { provide: PricesService, useFactory: () => instance(productPriceServiceMock) }, { provide: ProductsService, useFactory: () => instance(productsServiceMock) }, { provide: PromotionsService, useFactory: () => instance(promotionsServiceMock) }, { provide: SuggestService, useFactory: () => instance(mock(SuggestService)) }, + { provide: TokenService, useFactory: () => instance(mock(TokenService)) }, { provide: UserService, useFactory: () => instance(userServiceMock) }, provideStoreSnapshots(), ], diff --git a/src/app/core/store/customer/user/user.effects.spec.ts b/src/app/core/store/customer/user/user.effects.spec.ts index 7230e00bea..bacd3d2ccf 100644 --- a/src/app/core/store/customer/user/user.effects.spec.ts +++ b/src/app/core/store/customer/user/user.effects.spec.ts @@ -14,6 +14,7 @@ import { Customer, CustomerRegistrationType, CustomerUserType } from 'ish-core/m import { PasswordReminder } from 'ish-core/models/password-reminder/password-reminder.model'; import { User } from 'ish-core/models/user/user.model'; import { PaymentService } from 'ish-core/services/payment/payment.service'; +import { TokenService } from 'ish-core/services/token/token.service'; import { UserService } from 'ish-core/services/user/user.service'; import { CoreStoreModule } from 'ish-core/store/core/core-store.module'; import { displaySuccessMessage } from 'ish-core/store/core/messages'; @@ -73,6 +74,7 @@ describe('User Effects', () => { let paymentServiceMock: PaymentService; let apiTokenServiceMock: ApiTokenService; let oAuthServiceMock: OAuthService; + let tokenServiceMock: TokenService; let router: Router; let location: Location; @@ -102,10 +104,11 @@ describe('User Effects', () => { paymentServiceMock = mock(PaymentService); apiTokenServiceMock = mock(ApiTokenService); oAuthServiceMock = mock(OAuthService); + tokenServiceMock = mock(TokenService); when(userServiceMock.signInUser(anything())).thenReturn(of(loginResponseData)); - when(userServiceMock.fetchToken(anyString(), anything())).thenReturn(of(token)); - when(userServiceMock.fetchToken(anyString())).thenReturn(of(token)); + when(tokenServiceMock.fetchToken(anyString(), anything())).thenReturn(of(token)); + when(tokenServiceMock.fetchToken(anyString())).thenReturn(of(token)); when(userServiceMock.signInUserByToken(anything())).thenReturn(of(loginResponseData)); when(userServiceMock.createUser(anything())).thenReturn(of(undefined)); when(userServiceMock.updateUser(anything(), anything())).thenReturn(of({ firstName: 'Patricia' } as User)); @@ -129,8 +132,8 @@ describe('User Effects', () => { ], providers: [ { provide: ApiTokenService, useFactory: () => instance(apiTokenServiceMock) }, - { provide: OAuthService, useFactory: () => instance(oAuthServiceMock) }, { provide: PaymentService, useFactory: () => instance(paymentServiceMock) }, + { provide: TokenService, useFactory: () => instance(tokenServiceMock) }, { provide: UserService, useFactory: () => instance(userServiceMock) }, provideMockActions(() => actions$), UserEffects, @@ -217,11 +220,10 @@ describe('User Effects', () => { it('should dispatch a success action on a successful request and should fetch a new anonymous user token', () => { const action = logoutUser(); - const completion1 = logoutUserSuccess(); - const completion2 = fetchAnonymousUserToken(); + const completion = logoutUserSuccess(); actions$ = hot('-a', { a: action }); - const expected$ = cold('-(bc)', { b: completion1, c: completion2 }); + const expected$ = cold('-b', { b: completion }); expect(effects.logoutUser$).toBeObservable(expected$); }); @@ -247,7 +249,7 @@ describe('User Effects', () => { actions$ = of(action); effects.fetchAnonymousUserToken$.subscribe(() => { - verify(userServiceMock.fetchToken('anonymous')).once(); + verify(tokenServiceMock.fetchToken('anonymous')).once(); done(); }); }); diff --git a/src/app/core/store/customer/user/user.effects.ts b/src/app/core/store/customer/user/user.effects.ts index 518fd8bf62..be947bea7c 100644 --- a/src/app/core/store/customer/user/user.effects.ts +++ b/src/app/core/store/customer/user/user.effects.ts @@ -3,7 +3,6 @@ import { Router } from '@angular/router'; import { Actions, createEffect, ofType } from '@ngrx/effects'; import { routerNavigatedAction } from '@ngrx/router-store'; import { Store, select } from '@ngrx/store'; -import { OAuthService, OAuthSuccessEvent } from 'angular-oauth2-oidc'; import { from } from 'rxjs'; import { concatMap, @@ -20,6 +19,7 @@ import { import { CustomerRegistrationType } from 'ish-core/models/customer/customer.model'; import { PaymentService } from 'ish-core/services/payment/payment.service'; +import { TokenService } from 'ish-core/services/token/token.service'; import { UserService } from 'ish-core/services/user/user.service'; import { displaySuccessMessage } from 'ish-core/store/core/messages'; import { selectQueryParam, selectUrl } from 'ish-core/store/core/router'; @@ -83,7 +83,7 @@ export class UserEffects { private paymentService: PaymentService, private router: Router, private apiTokenService: ApiTokenService, - private oAuthService: OAuthService + private tokenService: TokenService ) {} loginUser$ = createEffect(() => @@ -115,20 +115,7 @@ export class UserEffects { () => this.actions$.pipe( ofType(fetchAnonymousUserToken), - switchMap(() => this.userService.fetchToken('anonymous')) - ), - { dispatch: false } - ); - - setApiToken$ = createEffect( - () => - this.oAuthService.events.pipe( - filter(event => event instanceof OAuthSuccessEvent && event.type === 'token_received'), - map(() => - this.apiTokenService.setApiToken(this.oAuthService.getAccessToken(), { - expires: new Date(this.oAuthService.getAccessTokenExpiration()), - }) - ) + switchMap(() => this.tokenService.fetchToken('anonymous')) ), { dispatch: false } ); diff --git a/src/app/core/utils/api-token/api-token.service.ts b/src/app/core/utils/api-token/api-token.service.ts index a7c81e18c4..f502d8a6cb 100644 --- a/src/app/core/utils/api-token/api-token.service.ts +++ b/src/app/core/utils/api-token/api-token.service.ts @@ -36,12 +36,7 @@ import { User } from 'ish-core/models/user/user.model'; import { ApiService } from 'ish-core/services/api/api.service'; import { getCurrentBasket, getCurrentBasketId, loadBasketByAPIToken } from 'ish-core/store/customer/basket'; import { getOrder, getSelectedOrderId, loadOrderByAPIToken } from 'ish-core/store/customer/orders'; -import { - fetchAnonymousUserToken, - getLoggedInUser, - getUserAuthorized, - loadUserByAPIToken, -} from 'ish-core/store/customer/user'; +import { getLoggedInUser, getUserAuthorized, loadUserByAPIToken } from 'ish-core/store/customer/user'; import { CookiesService } from 'ish-core/utils/cookies/cookies.service'; import { mapToProperty, whenTruthy } from 'ish-core/utils/operators'; @@ -185,17 +180,13 @@ export class ApiTokenService { return apiTokenCookie?.type === 'user' && !apiTokenCookie?.isAnonymous; } - restore$(types: ApiTokenCookieType[] = ['user', 'order'], fetchAnonymousToken = true): Observable { + restore$(types: ApiTokenCookieType[] = ['user', 'order']): Observable { if (SSR) { return of(true); } return this.initialCookie$.pipe( first(), - withLatestFrom(this.apiToken$), - switchMap(([cookie, apiToken]) => { - if (!apiToken && fetchAnonymousToken) { - this.store.dispatch(fetchAnonymousUserToken()); - } + switchMap(cookie => { if (types.includes(cookie?.type)) { switch (cookie?.type) { case 'user': { diff --git a/src/app/core/utils/instance-creators.ts b/src/app/core/utils/instance-creators.ts new file mode 100644 index 0000000000..b9039e9447 --- /dev/null +++ b/src/app/core/utils/instance-creators.ts @@ -0,0 +1,9 @@ +import { Injector } from '@angular/core'; +import { OAuthService } from 'angular-oauth2-oidc'; + +export class InstanceCreators { + static getOAuthServiceInstance(parent: Injector): OAuthService { + const injector = Injector.create({ providers: [{ provide: OAuthService }], parent }); + return injector.get(OAuthService); + } +} diff --git a/src/app/core/utils/oauth-configuration/oauth-configuration.service.spec.ts b/src/app/core/utils/oauth-configuration/oauth-configuration.service.spec.ts deleted file mode 100644 index 1004b61869..0000000000 --- a/src/app/core/utils/oauth-configuration/oauth-configuration.service.spec.ts +++ /dev/null @@ -1,75 +0,0 @@ -import { TestBed } from '@angular/core/testing'; -import { AuthConfig, OAuthInfoEvent, OAuthService, TokenResponse } from 'angular-oauth2-oidc'; -import { combineLatest, lastValueFrom, of } from 'rxjs'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; - -import { ApiService } from 'ish-core/services/api/api.service'; - -import { OAuthConfigurationService } from './oauth-configuration.service'; - -describe('Oauth Configuration Service', () => { - const TOKEN_ENDPOINT = 'http://test-icm-url.de/token'; - - const config: AuthConfig = { - tokenEndpoint: TOKEN_ENDPOINT, - requireHttps: false, - }; - - const apiService = mock(ApiService); - const oAuthService = mock(OAuthService); - - let component: OAuthConfigurationService; - - beforeEach(() => { - when(apiService.constructUrlForPath('token', anything())).thenReturn(of(TOKEN_ENDPOINT)); - const infoEvent = Object.create(OAuthInfoEvent.prototype); - when(oAuthService.events).thenReturn( - // eslint-disable-next-line ban/ban - of(Object.assign(infoEvent, { type: 'token_expires', info: 'access_token' })) - ); - when(oAuthService.refreshToken()).thenReturn( - lastValueFrom(of({ access_token: 'access', expires_in: 10, refresh_token: 'refresh' } as TokenResponse)) - ); - - TestBed.configureTestingModule({ - providers: [ - { provide: ApiService, useFactory: () => instance(apiService) }, - { provide: OAuthService, useFactory: () => instance(oAuthService) }, - ], - }).compileComponents(); - - component = TestBed.inject(OAuthConfigurationService); - }); - - it('should be created', () => { - expect(component).toBeTruthy(); - }); - - describe('loadConfig$', () => { - it('should calculate correct configuration object', done => { - component.loadConfig$.subscribe(authConf => { - verify(apiService.constructUrlForPath('token', anything())).once(); - expect(authConf).toEqual(config); - done(); - }); - }); - }); - - describe('config$', () => { - it('should contain the calculated authConfig after successful loadConfig$ action', done => { - combineLatest([component.loadConfig$, component.config$]).subscribe(([, authConf]) => { - expect(authConf).toEqual(config); - done(); - }); - }); - }); - - describe('setupRefreshTokenMechanism$', () => { - it('should refresh access token, when access token is about to expire', done => { - component.setupRefreshTokenMechanism$().subscribe(() => { - verify(oAuthService.refreshToken()).once(); - done(); - }); - }); - }); -}); diff --git a/src/app/core/utils/oauth-configuration/oauth-configuration.service.ts b/src/app/core/utils/oauth-configuration/oauth-configuration.service.ts deleted file mode 100644 index bf58651fba..0000000000 --- a/src/app/core/utils/oauth-configuration/oauth-configuration.service.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { Injectable } from '@angular/core'; -import { AuthConfig, OAuthInfoEvent, OAuthService, TokenResponse } from 'angular-oauth2-oidc'; -import { BehaviorSubject, Observable, filter, from, map, switchMap, take, tap } from 'rxjs'; - -import { ApiService } from 'ish-core/services/api/api.service'; -import { whenTruthy } from 'ish-core/utils/operators'; - -@Injectable({ - providedIn: 'root', -}) -export class OAuthConfigurationService { - config$ = new BehaviorSubject(undefined); - - constructor(private apiService: ApiService, private oAuthService: OAuthService) {} - - /** - * load an AuthConfig configuration object with specified tokenEndpoint - */ - get loadConfig$(): Observable { - return this.apiService - .constructUrlForPath('token', { - sendCurrency: true, - sendLocale: true, - }) - .pipe( - whenTruthy(), - filter(url => !url.startsWith('/')), // url should not be relative - take(1), - map(url => ({ - tokenEndpoint: url, - requireHttps: url.startsWith('https'), - })), - tap(config => this.config$.next(config)) - ); - } - - /** - * Refresh existing tokens, when token is about to expire - * - * @returns {TokenResponse} updated tokens - */ - setupRefreshTokenMechanism$(): Observable { - return this.oAuthService.events.pipe( - filter( - event => event instanceof OAuthInfoEvent && event.type === 'token_expires' && event.info === 'access_token' - ), - switchMap(() => from(this.oAuthService.refreshToken())) - ); - } -} diff --git a/src/app/extensions/punchout/identity-provider/punchout-identity-provider.spec.ts b/src/app/extensions/punchout/identity-provider/punchout-identity-provider.spec.ts index f43b65d6e0..27b0f50212 100644 --- a/src/app/extensions/punchout/identity-provider/punchout-identity-provider.spec.ts +++ b/src/app/extensions/punchout/identity-provider/punchout-identity-provider.spec.ts @@ -1,8 +1,7 @@ import { TestBed, fakeAsync, tick } from '@angular/core/testing'; import { ActivatedRouteSnapshot, Params, Router, UrlTree, convertToParamMap } from '@angular/router'; import { MockStore, provideMockStore } from '@ngrx/store/testing'; -import { OAuthService } from 'angular-oauth2-oidc'; -import { BehaviorSubject, EMPTY, Observable, Subject, noop, of, timer } from 'rxjs'; +import { EMPTY, Observable, Subject, noop, of, timer } from 'rxjs'; import { switchMap } from 'rxjs/operators'; import { anyString, anything, instance, mock, resetCalls, verify, when } from 'ts-mockito'; @@ -14,7 +13,6 @@ import { ApiTokenService } from 'ish-core/utils/api-token/api-token.service'; import { CookiesService } from 'ish-core/utils/cookies/cookies.service'; import { makeHttpError } from 'ish-core/utils/dev/api-service-utils'; import { BasketMockData } from 'ish-core/utils/dev/basket-mock-data'; -import { OAuthConfigurationService } from 'ish-core/utils/oauth-configuration/oauth-configuration.service'; import { PunchoutSession } from '../models/punchout-session/punchout-session.model'; import { PunchoutService } from '../services/punchout/punchout.service'; @@ -34,7 +32,6 @@ describe('Punchout Identity Provider', () => { const accountFacade = mock(AccountFacade); const checkoutFacade = mock(CheckoutFacade); const cookiesService = mock(CookiesService); - const oAuthConfigurationService = mock(OAuthConfigurationService); let punchoutIdentityProvider: PunchoutIdentityProvider; let store$: MockStore; @@ -48,8 +45,6 @@ describe('Punchout Identity Provider', () => { { provide: AppFacade, useFactory: () => instance(appFacade) }, { provide: CheckoutFacade, useFactory: () => instance(checkoutFacade) }, { provide: CookiesService, useFactory: () => instance(cookiesService) }, - { provide: OAuthConfigurationService, useFactory: () => instance(oAuthConfigurationService) }, - { provide: OAuthService, useFactory: () => instance(mock(OAuthService)) }, { provide: PunchoutService, useFactory: () => instance(punchoutService) }, provideMockStore(), ], @@ -64,7 +59,6 @@ describe('Punchout Identity Provider', () => { when(apiTokenService.restore$(anything())).thenReturn(of(true)); when(apiTokenService.cookieVanishes$).thenReturn(new Subject()); when(checkoutFacade.basket$).thenReturn(EMPTY); - when(oAuthConfigurationService.config$).thenReturn(new BehaviorSubject({})); resetCalls(apiTokenService); resetCalls(punchoutService); diff --git a/src/app/extensions/punchout/identity-provider/punchout-identity-provider.ts b/src/app/extensions/punchout/identity-provider/punchout-identity-provider.ts index 1364a6fef2..bbaf35fcb2 100644 --- a/src/app/extensions/punchout/identity-provider/punchout-identity-provider.ts +++ b/src/app/extensions/punchout/identity-provider/punchout-identity-provider.ts @@ -2,8 +2,7 @@ import { HttpEvent, HttpHandler, HttpRequest } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { ActivatedRouteSnapshot, Router, UrlTree } from '@angular/router'; import { Store, select } from '@ngrx/store'; -import { OAuthService } from 'angular-oauth2-oidc'; -import { BehaviorSubject, Observable, merge, noop, of, race, throwError } from 'rxjs'; +import { Observable, noop, of, race, throwError } from 'rxjs'; import { catchError, concatMap, delay, filter, first, map, switchMap, take, tap, withLatestFrom } from 'rxjs/operators'; import { AccountFacade } from 'ish-core/facades/account.facade'; @@ -13,17 +12,12 @@ import { IdentityProvider, TriggerReturnType } from 'ish-core/identity-provider/ import { selectQueryParam } from 'ish-core/store/core/router'; import { ApiTokenService } from 'ish-core/utils/api-token/api-token.service'; import { CookiesService } from 'ish-core/utils/cookies/cookies.service'; -import { OAuthConfigurationService } from 'ish-core/utils/oauth-configuration/oauth-configuration.service'; import { whenTruthy } from 'ish-core/utils/operators'; import { PunchoutService } from '../services/punchout/punchout.service'; @Injectable({ providedIn: 'root' }) export class PunchoutIdentityProvider implements IdentityProvider { - // emits true, when OAuth Service is successfully configured - // used as an additional condition to check that the OAuth Service is configured before OAuth Service actions are used - private oAuthServiceConfigured$ = new BehaviorSubject(false); - constructor( protected router: Router, protected store: Store, @@ -32,9 +26,7 @@ export class PunchoutIdentityProvider implements IdentityProvider { private accountFacade: AccountFacade, private punchoutService: PunchoutService, private cookiesService: CookiesService, - private checkoutFacade: CheckoutFacade, - private oAuthService: OAuthService, - private configService: OAuthConfigurationService + private checkoutFacade: CheckoutFacade ) {} getCapabilities() { @@ -46,12 +38,6 @@ export class PunchoutIdentityProvider implements IdentityProvider { } init() { - // OAuth Service should be configured by internal OAuth configuration service - this.configService.config$.pipe(whenTruthy(), take(1)).subscribe(config => { - this.oAuthService.configure(config); - this.oAuthServiceConfigured$.next(true); - }); - this.apiTokenService.cookieVanishes$ .pipe(withLatestFrom(this.apiTokenService.apiToken$)) .subscribe(([type, apiToken]) => { @@ -64,15 +50,7 @@ export class PunchoutIdentityProvider implements IdentityProvider { }); // OAuth Service should be configured before apiToken information are restored and the refresh token mechanism is setup - this.oAuthServiceConfigured$ - .pipe( - whenTruthy(), - take(1), - switchMap(() => - merge(this.apiTokenService.restore$(['user', 'order']), this.configService.setupRefreshTokenMechanism$()) - ) - ) - .subscribe(noop); + this.apiTokenService.restore$(['user', 'order']).subscribe(noop); this.checkoutFacade.basket$.pipe(whenTruthy(), first()).subscribe(basketView => { window.sessionStorage.setItem('basket-id', basketView.id); @@ -94,85 +72,71 @@ export class PunchoutIdentityProvider implements IdentityProvider { return false; } - return this.oAuthServiceConfigured$.pipe( - whenTruthy(), - take(1), - tap(() => { - // initiate the punchout user login with the access-token (cXML) or the given credentials (OCI) - if (route.queryParamMap.has('access-token')) { - this.accountFacade.loginUserWithToken(route.queryParamMap.get('access-token')); - } else { - this.accountFacade.loginUser({ - login: route.queryParamMap.get('USERNAME'), - password: route.queryParamMap.get('PASSWORD'), - }); - } - }), - switchMap(() => - race( - // throw an error if a user login error occurs - this.accountFacade.userError$.pipe( - whenTruthy(), - take(1), - concatMap(userError => throwError(() => userError)) - ), - - // handle the punchout functions once the punchout user is logged in - this.accountFacade.isLoggedIn$.pipe( - whenTruthy(), - take(1), + if (route.queryParamMap.has('access-token')) { + this.accountFacade.loginUserWithToken(route.queryParamMap.get('access-token')); + } else { + this.accountFacade.loginUser({ + login: route.queryParamMap.get('USERNAME'), + password: route.queryParamMap.get('PASSWORD'), + }); + } + + return race( + // throw an error if a user login error occurs + this.accountFacade.userError$.pipe( + whenTruthy(), + take(1), + concatMap(userError => throwError(() => userError)) + ), + + // handle the punchout functions once the punchout user is logged in + this.accountFacade.isLoggedIn$.pipe( + whenTruthy(), + take(1), + switchMap(() => { + // handle cXML punchout with sid + if (route.queryParamMap.get('sid')) { + return this.handleCxmlPunchoutLogin(route); + // handle OCI punchout with HOOK_URL + } else if (route.queryParamMap.get('HOOK_URL')) { + return this.handleOciPunchoutLogin(route); + } + }), + // punchout error after successful authentication (needs to logout) + catchError(error => + this.accountFacade.userLoading$.pipe( + first(loading => !loading), + delay(0), switchMap(() => { - // handle cXML punchout with sid - if (route.queryParamMap.get('sid')) { - return this.handleCxmlPunchoutLogin(route); - // handle OCI punchout with HOOK_URL - } else if (route.queryParamMap.get('HOOK_URL')) { - return this.handleOciPunchoutLogin(route); - } - }), - // punchout error after successful authentication (needs to logout) - catchError(error => - this.accountFacade.userLoading$.pipe( - first(loading => !loading), - delay(0), - switchMap(() => { - this.accountFacade.logoutUser(); - this.apiTokenService.removeApiToken(); - this.appFacade.setBusinessError(error); - return of(this.router.parseUrl('/error')); - }) - ) - ) + this.accountFacade.logoutUser(); + this.apiTokenService.removeApiToken(); + this.appFacade.setBusinessError(error); + return of(this.router.parseUrl('/error')); + }) ) - ).pipe( - // general punchout error handling (parameter missing, authentication error) - catchError(error => { - this.appFacade.setBusinessError(error); - return of(this.router.parseUrl('/error')); - }) ) ) + ).pipe( + // general punchout error handling (parameter missing, authentication error) + catchError(error => { + this.appFacade.setBusinessError(error); + return of(this.router.parseUrl('/error')); + }) ); } triggerLogout(): TriggerReturnType { window.sessionStorage.removeItem('basket-id'); - return this.oAuthServiceConfigured$.pipe( - whenTruthy(), + this.accountFacade.logoutUser(); // user will be logged out and related refresh token is revoked on server + return this.accountFacade.isLoggedIn$.pipe( + // wait until the user is logged out before you go to homepage to prevent unnecessary REST calls + filter(loggedIn => !loggedIn), take(1), - tap(() => this.accountFacade.logoutUser()), // user will be logged out and related refresh token is revoked on server switchMap(() => - this.accountFacade.isLoggedIn$.pipe( - // wait until the user is logged out before you go to homepage to prevent unnecessary REST calls - filter(loggedIn => !loggedIn), - take(1), - switchMap(() => - this.store.pipe( - select(selectQueryParam('returnUrl')), - map(returnUrl => returnUrl || '/home'), - map(returnUrl => this.router.parseUrl(returnUrl)) - ) - ) + this.store.pipe( + select(selectQueryParam('returnUrl')), + map(returnUrl => returnUrl || '/home'), + map(returnUrl => this.router.parseUrl(returnUrl)) ) ) );