From 56c0f19440a07b80621b50f1d13113ffffc1a739 Mon Sep 17 00:00:00 2001 From: Ryan Diehl Date: Wed, 8 Jul 2020 14:36:22 -0400 Subject: [PATCH] feat(security): cross cutting security features exposed class interfaces for auth and token services --- libs/utils/security/src/index.ts | 8 +- .../src/lib/already-logged-in.guard.spec.ts | 75 ++++++++++ .../src/lib/already-logged-in.guard.ts | 30 ++++ .../security/src/lib/auth.interceptor.spec.ts | 134 ++++++++++++++++++ .../security/src/lib/auth.interceptor.ts | 56 ++++++++ .../security/src/lib/auth.service.spec.ts | 40 ++++++ libs/utils/security/src/lib/auth.service.ts | 26 ++++ libs/utils/security/src/lib/constants.spec.ts | 12 +- libs/utils/security/src/lib/constants.ts | 1 + libs/utils/security/src/lib/providers.ts | 11 ++ libs/utils/security/src/lib/token.service.ts | 32 +++++ ...userid-request-tracing-interceptor.spec.ts | 107 ++++++++++++++ .../lib/userid-request-tracing-interceptor.ts | 26 ++++ libs/utils/security/src/test-setup.ts | 1 + 14 files changed, 555 insertions(+), 4 deletions(-) create mode 100644 libs/utils/security/src/lib/already-logged-in.guard.spec.ts create mode 100644 libs/utils/security/src/lib/already-logged-in.guard.ts create mode 100644 libs/utils/security/src/lib/auth.interceptor.spec.ts create mode 100644 libs/utils/security/src/lib/auth.interceptor.ts create mode 100644 libs/utils/security/src/lib/auth.service.spec.ts create mode 100644 libs/utils/security/src/lib/auth.service.ts create mode 100644 libs/utils/security/src/lib/providers.ts create mode 100644 libs/utils/security/src/lib/token.service.ts create mode 100644 libs/utils/security/src/lib/userid-request-tracing-interceptor.spec.ts create mode 100644 libs/utils/security/src/lib/userid-request-tracing-interceptor.ts diff --git a/libs/utils/security/src/index.ts b/libs/utils/security/src/index.ts index b358e75..c0eebc0 100644 --- a/libs/utils/security/src/index.ts +++ b/libs/utils/security/src/index.ts @@ -1 +1,7 @@ -export { REQUIRE_AUTH_HEADER } from './lib/constants'; +export { AlreadyLoggedInGuard } from './lib/already-logged-in.guard'; +export { AuthInterceptor } from './lib/auth.interceptor'; +export { AuthService } from './lib/auth.service'; +export { HTTP_AUTHORIZATION_HEADER, REQUIRE_AUTH_HEADER } from './lib/constants'; +export { provideAuthService, provideTokenService } from './lib/providers'; +export { TokenService } from './lib/token.service'; +export { UseridRequestTracingInterceptor } from './lib/userid-request-tracing-interceptor'; diff --git a/libs/utils/security/src/lib/already-logged-in.guard.spec.ts b/libs/utils/security/src/lib/already-logged-in.guard.spec.ts new file mode 100644 index 0000000..1b6eae5 --- /dev/null +++ b/libs/utils/security/src/lib/already-logged-in.guard.spec.ts @@ -0,0 +1,75 @@ +import { TestBed } from '@angular/core/testing'; +import { Router } from '@angular/router'; +import { Mock } from 'ts-mocks'; +import { AlreadyLoggedInGuard } from './already-logged-in.guard'; +import { AuthService } from './auth.service'; + +describe('AlreadyLoggedInGuard', () => { + let guard: AlreadyLoggedInGuard; + let router: Mock; + let authService: Mock; + let isLoggedIn: boolean; + let result: boolean; + let complete: boolean; + let loadResult: boolean; + let loadComplete: boolean; + + const snapshot = { + data: { + targetRoute: ['test', 'route'], + queryParams: { + a: 'b' + } + } + }; + + beforeEach(() => { + result = complete = undefined; + loadResult = loadComplete = undefined; + isLoggedIn = undefined; + router = new Mock({ + navigate: () => ({} as any) + }); + authService = new Mock({ + isLoggedIn: () => isLoggedIn + }); + TestBed.configureTestingModule({ + providers: [ + { provide: Router, useValue: router.Object }, + { provide: AuthService, useValue: authService.Object }, + AlreadyLoggedInGuard + ] + }); + + guard = TestBed.get(AlreadyLoggedInGuard); + }); + + describe('when not logged in', () => { + beforeEach(() => { + isLoggedIn = false; + }); + + it('should emit true and not redirect', () => { + expect(guard.canActivate(snapshot as any)).toBe(true); + expect(guard.canLoad({ data: { targetRoute: ['lazy'] } })).toBe(true); + expect(router.Object.navigate).not.toHaveBeenCalled(); + }); + }); + + describe('when logged in', () => { + beforeEach(() => { + isLoggedIn = true; + }); + + it('should emit false and redirect to dashboard', () => { + expect(guard.canActivate(snapshot as any)).toBe(false); + expect(guard.canLoad({ data: { targetRoute: ['lazy'] } })).toBe(false); + expect(router.Object.navigate).toHaveBeenCalledWith(['test', 'route'], { + queryParams: { a: 'b' } + }); + expect(router.Object.navigate).toHaveBeenCalledWith(['lazy'], { + queryParams: {} + }); + }); + }); +}); diff --git a/libs/utils/security/src/lib/already-logged-in.guard.ts b/libs/utils/security/src/lib/already-logged-in.guard.ts new file mode 100644 index 0000000..b780f17 --- /dev/null +++ b/libs/utils/security/src/lib/already-logged-in.guard.ts @@ -0,0 +1,30 @@ +import { Injectable } from '@angular/core'; +import { ActivatedRouteSnapshot, CanActivate, CanLoad, Route, Router } from '@angular/router'; +import { Observable, of } from 'rxjs'; +import { map, take, tap } from 'rxjs/operators'; +import { AuthService } from './auth.service'; + +@Injectable({ + providedIn: 'root' +}) +export class AlreadyLoggedInGuard implements CanActivate, CanLoad { + constructor(private authService: AuthService, private router: Router) {} + + public canActivate(route: ActivatedRouteSnapshot): boolean { + return this.isLoggedIn(route.data); + } + + public canLoad(route: Route): boolean { + return this.isLoggedIn(route.data); + } + + private isLoggedIn(routeData: any): boolean { + const loggedIn = !!this.authService.isLoggedIn(); + if (loggedIn && routeData && routeData.targetRoute) { + this.router.navigate(routeData.targetRoute, { + queryParams: routeData.queryParams || {} + }); + } + return !loggedIn; + } +} diff --git a/libs/utils/security/src/lib/auth.interceptor.spec.ts b/libs/utils/security/src/lib/auth.interceptor.spec.ts new file mode 100644 index 0000000..f813cd7 --- /dev/null +++ b/libs/utils/security/src/lib/auth.interceptor.spec.ts @@ -0,0 +1,134 @@ +import { HttpClient, HttpHeaders, HttpRequest, HTTP_INTERCEPTORS } from '@angular/common/http'; +import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; +import { Injectable } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; +import { Observable, Subject } from 'rxjs'; +import { Mock } from 'ts-mocks'; +import { AuthInterceptor } from './auth.interceptor'; +import { REQUIRE_AUTH_HEADER } from './constants'; +import { TokenService } from './token.service'; + +@Injectable() +class TestService { + constructor(private httpClient: HttpClient) {} + + public getUnprotected(): Observable { + return this.httpClient.get('unprotected'); + } + + public getProtected(): Observable { + return this.httpClient.get('api', { + headers: new HttpHeaders().append(REQUIRE_AUTH_HEADER, 'true').append('Another', 'abc') + }); + } + + public getBasicAuth(): Observable { + return this.httpClient.get('api/1', { + headers: { + Authorization: 'Basic test', + [REQUIRE_AUTH_HEADER]: 'true' + } + }); + } +} + +describe('AuthInterceptor', () => { + let service: TestService; + let backend: HttpTestingController; + let tokenService: Mock; + let tokenCanAuth: boolean; + let acquireTokenResult$: Subject; + + beforeEach(() => { + tokenCanAuth = undefined; + acquireTokenResult$ = new Subject(); + tokenService = new Mock({ + requiresAuth: () => tokenCanAuth, + acquireToken: (req: HttpRequest) => acquireTokenResult$.asObservable(), + onUnauthorized: () => {} + }); + + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ + { provide: HTTP_INTERCEPTORS, useClass: AuthInterceptor, multi: true }, + TestService, + { provide: TokenService, useValue: tokenService.Object } + ] + }); + service = TestBed.get(TestService); + backend = TestBed.get(HttpTestingController); + }); + + afterEach(() => { + backend.verify(); + }); + + describe('intercept', () => { + it('should not add an Authorization header when request url is unprotected', () => { + service.getUnprotected().subscribe(); + const req = backend.expectOne('unprotected'); + expect(req.request.headers.has('Authorization')).toBe(false); + expect(req.request.headers.has(REQUIRE_AUTH_HEADER)).toBe(false); + req.flush({}); + expect(tokenService.Object.onUnauthorized).not.toHaveBeenCalled(); + }); + + it('should not add an Authorization header when token service cannot handle the request', () => { + tokenCanAuth = false; + service.getProtected().subscribe(); + const req = backend.expectOne('api'); + expect(req.request.headers.has('Authorization')).toBe(false); + expect(req.request.headers.has(REQUIRE_AUTH_HEADER)).toBe(false); + req.flush({}); + expect(tokenService.Object.onUnauthorized).not.toHaveBeenCalled(); + }); + + it('should add Authorization header when token service can handle the request', () => { + tokenCanAuth = true; + service.getProtected().subscribe(); + acquireTokenResult$.next('tok'); + const req = backend.expectOne('api'); + expect(req.request.headers.get('Authorization')).toEqual('Bearer tok'); + expect(req.request.headers.has(REQUIRE_AUTH_HEADER)).toBe(false); + expect(req.request.headers.get('Another')).toBe('abc'); + req.flush({}); + expect(tokenService.Object.onUnauthorized).not.toHaveBeenCalled(); + }); + + it('should not modify existing Authorization header when request already has an authorization header', () => { + tokenCanAuth = true; + service.getBasicAuth().subscribe(); + const req = backend.expectOne('api/1'); + expect(req.request.headers.get('Authorization')).toEqual('Basic test'); + expect(req.request.headers.has(REQUIRE_AUTH_HEADER)).toBe(false); + req.flush({}); + expect(tokenService.Object.onUnauthorized).not.toHaveBeenCalled(); + }); + + it('should not make request if token service throws an error', () => { + tokenCanAuth = true; + let error; + service.getProtected().subscribe({ + next: () => fail('Expected error to be thrown'), + error: e => (error = e) + }); + acquireTokenResult$.error('token error'); + backend.expectNone('api/1'); + expect(error).toEqual('token error'); + expect(tokenService.Object.onUnauthorized).not.toHaveBeenCalled(); + }); + + it('should call onUnauthorized if server errors with 401', () => { + tokenCanAuth = true; + service.getProtected().subscribe(); + acquireTokenResult$.next('tok'); + const req = backend.expectOne('api'); + req.flush('', { status: 401, statusText: 'Unauthorized' }); + expect(tokenService.Object.onUnauthorized).toHaveBeenCalledWith( + jasmine.objectContaining({ status: 401, statusText: 'Unauthorized' }), + 'tok' + ); + }); + }); +}); diff --git a/libs/utils/security/src/lib/auth.interceptor.ts b/libs/utils/security/src/lib/auth.interceptor.ts new file mode 100644 index 0000000..4b8063b --- /dev/null +++ b/libs/utils/security/src/lib/auth.interceptor.ts @@ -0,0 +1,56 @@ +import { HttpErrorResponse, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest } from '@angular/common/http'; +import { Injectable } from '@angular/core'; +import { Observable, throwError } from 'rxjs'; +import { catchError, mergeMap } from 'rxjs/operators'; +import { HTTP_AUTHORIZATION_HEADER, REQUIRE_AUTH_HEADER } from './constants'; +import { TokenService } from './token.service'; + +@Injectable() +export class AuthInterceptor implements HttpInterceptor { + constructor(private tokenService: TokenService) {} + + public intercept(req: HttpRequest, next: HttpHandler): Observable> { + // figure out if the request requires authentication. + const shouldAuth = this.requiresAuth(req); + + // always remove the custom x-auth header (cases where token handler cannot handle a request) + const headers = req.headers.delete(REQUIRE_AUTH_HEADER); + const newReq = req.clone({ headers }); + if (shouldAuth) { + return this.tokenService.acquireToken(newReq).pipe( + mergeMap(token => + next + .handle( + newReq.clone({ + setHeaders: { + [HTTP_AUTHORIZATION_HEADER]: `Bearer ${token}` + } + }) + ) + .pipe( + catchError(err => { + if (err instanceof HttpErrorResponse && err.status === 401 && this.tokenService.onUnauthorized) { + this.tokenService.onUnauthorized(err, token); + } + return throwError(err); + }) + ) + ) + ); + } else { + return next.handle(newReq); + } + } + + private requiresAuth(req: HttpRequest): boolean { + // 1. Does request have our custom X-Auth header? + // 2. Does request NOT already have an Authorization header? + // 3. Does token handler know how to handle the request? + // If all 3 are true, we can handle this request + return ( + req.headers.has(REQUIRE_AUTH_HEADER) && + !req.headers.has(HTTP_AUTHORIZATION_HEADER) && + this.tokenService.requiresAuth(req) + ); + } +} diff --git a/libs/utils/security/src/lib/auth.service.spec.ts b/libs/utils/security/src/lib/auth.service.spec.ts new file mode 100644 index 0000000..2f21e27 --- /dev/null +++ b/libs/utils/security/src/lib/auth.service.spec.ts @@ -0,0 +1,40 @@ +import { Injectable } from '@angular/core'; +import { AuthService } from './auth.service'; + +@Injectable() +class MockAuthService extends AuthService { + getUserName = () => 'mock'; +} + +@Injectable() +class MockAnonymousService extends AuthService { + getUserName = () => undefined; +} + +describe('AuthService', () => { + let service: MockAuthService; + let anonymousService: MockAnonymousService; + + beforeEach(() => { + service = new MockAuthService(); + anonymousService = new MockAnonymousService(); + }); + + it('should create', () => { + expect(service).toBeDefined(); + expect(anonymousService).toBeDefined(); + }); + + it('should return mock username', () => { + expect(service.getUserName()).toEqual('mock'); + expect(anonymousService.getUserName()).toBeUndefined(); + }); + + it('isLoggedIn should be true', () => { + expect(service.isLoggedIn()).toEqual(true); + }); + + it('anonymous service isLoggedIn should be false', () => { + expect(anonymousService.isLoggedIn()).toEqual(false); + }); +}); diff --git a/libs/utils/security/src/lib/auth.service.ts b/libs/utils/security/src/lib/auth.service.ts new file mode 100644 index 0000000..a3c6450 --- /dev/null +++ b/libs/utils/security/src/lib/auth.service.ts @@ -0,0 +1,26 @@ +/** + * This is a "class interface" for an authentication service. + * + * It exists to allow us to easily decouple the auth implementation from cross-cutting concerns + * that only need to be able to do things like get the current username. + */ +export abstract class AuthService { + /** + * Get the userName of the currently logged in user. + */ + getUserName: () => string; + + /** + * Trigger a login request to the auth provider. + * + * @param targetUrl the destination URL after logging in + */ + login: (targetUrl: string) => any; + + /** + * Determine if there is a logged in user. + */ + public isLoggedIn(): boolean { + return !!this.getUserName(); + } +} diff --git a/libs/utils/security/src/lib/constants.spec.ts b/libs/utils/security/src/lib/constants.spec.ts index 71e8d20..4507369 100644 --- a/libs/utils/security/src/lib/constants.spec.ts +++ b/libs/utils/security/src/lib/constants.spec.ts @@ -1,5 +1,11 @@ -describe('Need to have at least one test', () => { - it('should pass', () => { - expect(1).toEqual(1); +import { HTTP_AUTHORIZATION_HEADER, REQUIRE_AUTH_HEADER } from './constants'; + +describe('security constants', () => { + it('REQUIRE_AUTH_HEADER should be X-Auth', () => { + expect(REQUIRE_AUTH_HEADER).toEqual('X-Auth'); + }); + + it('HTTP_AUTHORIZATION_HEADER should be Authorization', () => { + expect(HTTP_AUTHORIZATION_HEADER).toEqual('Authorization'); }); }); diff --git a/libs/utils/security/src/lib/constants.ts b/libs/utils/security/src/lib/constants.ts index 11ec44a..9229ceb 100644 --- a/libs/utils/security/src/lib/constants.ts +++ b/libs/utils/security/src/lib/constants.ts @@ -1 +1,2 @@ export const REQUIRE_AUTH_HEADER = 'X-Auth'; +export const HTTP_AUTHORIZATION_HEADER = 'Authorization'; diff --git a/libs/utils/security/src/lib/providers.ts b/libs/utils/security/src/lib/providers.ts new file mode 100644 index 0000000..042744d --- /dev/null +++ b/libs/utils/security/src/lib/providers.ts @@ -0,0 +1,11 @@ +import { Provider, Type } from '@angular/core'; +import { AuthService } from './auth.service'; +import { TokenService } from './token.service'; + +export function provideAuthService(impl: Type): Provider { + return { provide: AuthService, useExisting: impl }; +} + +export function provideTokenService(impl: Type): Provider { + return { provide: TokenService, useExisting: impl }; +} diff --git a/libs/utils/security/src/lib/token.service.ts b/libs/utils/security/src/lib/token.service.ts new file mode 100644 index 0000000..bc5eb2d --- /dev/null +++ b/libs/utils/security/src/lib/token.service.ts @@ -0,0 +1,32 @@ +import { HttpErrorResponse, HttpRequest } from '@angular/common/http'; +import { Observable } from 'rxjs'; + +/** + * This is a "class interface" for a token service. + * + * It exists to allow us to easily decouple the token implementation from the + * HTTP interceptor that needs to acquire the token. + */ +export abstract class TokenService { + /** + * Determine whether this token handler is able to acquire a token for a given request. + * This method is called after initial checks have already passed, so implementations only + * need to worry about checking scopes, etc. + * + * @returns true if this token handler is able to acquire a token for the given request, false otherwise + */ + requiresAuth: (req: HttpRequest) => boolean; + + /** + * Acquire a token for the given request. + * + * @returns an Observable of a bearer token + */ + acquireToken: (req: HttpRequest) => Observable; + + /** + * Handle HTTP 401 Unauthorized responses from the server. This function is optional. + * The auth provider may want to clear token caches, etc. + */ + onUnauthorized?: (err: HttpErrorResponse, token: string) => void; +} diff --git a/libs/utils/security/src/lib/userid-request-tracing-interceptor.spec.ts b/libs/utils/security/src/lib/userid-request-tracing-interceptor.spec.ts new file mode 100644 index 0000000..28b86e2 --- /dev/null +++ b/libs/utils/security/src/lib/userid-request-tracing-interceptor.spec.ts @@ -0,0 +1,107 @@ +import { HttpClient, HttpHeaders, HTTP_INTERCEPTORS } from '@angular/common/http'; +import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; +import { Injectable } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; +import { REQUEST_TRACING_CONFIG, TRACE_HEADER } from '@psu/utils/browser'; +import { Observable } from 'rxjs'; +import { Mock } from 'ts-mocks'; +import { AuthService } from './auth.service'; +import { UseridRequestTracingInterceptor } from './userid-request-tracing-interceptor'; + +@Injectable() +class TestService { + constructor(private httpClient: HttpClient) {} + + public getStuff(trace = true): Observable { + let headers = new HttpHeaders({ + something: 'else' + }); + if (trace) { + headers = headers.append('X-Trace', 'true'); + } + return this.httpClient.get('http://stuff', { + headers + }); + } +} + +describe('UseridRequestTracingInterceptor', () => { + let backend: HttpTestingController; + let service: TestService; + let authService: Mock; + let mockUserName: string; + + beforeEach(() => { + mockUserName = undefined; + authService = new Mock({ + getUserName: () => mockUserName + }); + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ + { + provide: HTTP_INTERCEPTORS, + useClass: UseridRequestTracingInterceptor, + multi: true + }, + { + provide: REQUEST_TRACING_CONFIG, + useValue: { applicationId: 'my-app' } + }, + { provide: AuthService, useValue: authService.Object }, + TestService + ] + }); + + service = TestBed.get(TestService); + backend = TestBed.get(HttpTestingController); + }); + + afterEach(() => { + backend.verify(); + }); + + describe('intercept', () => { + describe('when there is no trace header', () => { + beforeEach(() => { + mockUserName = 'junk'; + service.getStuff(false).subscribe(); + }); + + it('should not add request id header', () => { + const req = backend.expectOne('http://stuff'); + expect(req.request.headers.has('something')).toBe(true); + expect(req.request.headers.has('x-request-id')).toBe(false); + expect(req.request.headers.has(TRACE_HEADER)).toBe(false); + }); + }); + + describe('when there is no user name', () => { + beforeEach(() => { + mockUserName = undefined; + service.getStuff().subscribe(); + }); + + it('should add default request id header', () => { + const req = backend.expectOne('http://stuff'); + expect(req.request.headers.has('something')).toBe(true); + expect(req.request.headers.get('x-request-id')).toMatch(/^my-app_\w+$/); + expect(req.request.headers.has(TRACE_HEADER)).toBe(false); + }); + }); + + describe('when there is a user name', () => { + beforeEach(() => { + mockUserName = 'me123'; + service.getStuff().subscribe(); + }); + + it('should add request id header with username', () => { + const req = backend.expectOne('http://stuff'); + expect(req.request.headers.has('something')).toBe(true); + expect(req.request.headers.get('x-request-id')).toMatch(/^my-app_me123_\w+$/); + expect(req.request.headers.has(TRACE_HEADER)).toBe(false); + }); + }); + }); +}); diff --git a/libs/utils/security/src/lib/userid-request-tracing-interceptor.ts b/libs/utils/security/src/lib/userid-request-tracing-interceptor.ts new file mode 100644 index 0000000..d91f435 --- /dev/null +++ b/libs/utils/security/src/lib/userid-request-tracing-interceptor.ts @@ -0,0 +1,26 @@ +import { Inject, Injectable } from '@angular/core'; +import { RequestTracingConfig, RequestTracingInterceptor, REQUEST_TRACING_CONFIG } from '@psu/utils/browser'; +import { AuthService } from './auth.service'; + +@Injectable() +export class UseridRequestTracingInterceptor extends RequestTracingInterceptor { + constructor( + @Inject(REQUEST_TRACING_CONFIG) protected config: RequestTracingConfig, + private authService: AuthService + ) { + super(config); + } + + protected generateRequestId(): string { + const user = this.authService.getUserName(); + if (!!user) { + return this._generate(user); + } else { + return super.generateRequestId(); + } + } + + private _generate(userid: string): string { + return `${super.getApplicationId()}_${userid}_${super.getShortUuid()}`; + } +} diff --git a/libs/utils/security/src/test-setup.ts b/libs/utils/security/src/test-setup.ts index 8d88704..ea6fbef 100644 --- a/libs/utils/security/src/test-setup.ts +++ b/libs/utils/security/src/test-setup.ts @@ -1 +1,2 @@ import 'jest-preset-angular'; +import '../../../../jestGlobalMocks'; -- GitLab