Merge pull request #731 from damienbod/fabiangosebrink/add-config-val…

…idation Fabiangosebrink/add config validation
damienbod · May 15, 2020 · e3e517d · e3e517d
2 parents 5c7dd4a + 3c061bd
commit e3e517d
Show file tree

Hide file tree

Showing 19 changed files with 279 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,13 @@
 ## Angular Lib for OpenID Connect/OAuth2 Changelog
 
+### 2020-05-xx Version 11.1.1
+
+-   Added validation for the lib configuration
+    -   [PR](https://github.com/damienbod/angular-auth-oidc-client/pull/731) // Fixes [#725](https://github.com/damienbod/angular-auth-oidc-client/issues/725)
+-   fixed some doc typos
+-   fixed bug 2 auth events emitter on STS callback
+    -   Fixes [PR](https://github.com/damienbod/angular-auth-oidc-client/pull/731) // [#734](https://github.com/damienbod/angular-auth-oidc-client/issues/734)
+
 ### 2020-05-14 Version 11.1.0
 
 -   Eager loading of well known endpoints can be configured: Made it possible to load the well known endpoints late (per configuration)

diff --git a/docs/features.md b/docs/features.md
@@ -117,7 +117,7 @@ If you want to pass dynamic custom parameters with the request url to the sts yo
 
 ```typescript
 login() {
-    this.oidcSecurityService.authorize({ customParams: { 'ui_locales: 'de-CH' });
+    this.oidcSecurityService.authorize({ customParams: { ui_locales: 'de-CH' });
 }
 
 ```

diff --git a/package.json b/package.json
@@ -8,7 +8,7 @@
     "bugs": {
         "url": "https://github.com/damienbod/angular-auth-oidc-client/issues"
     },
-    "version": "11.1.0",
+    "version": "11.1.1",
     "scripts": {
         "ng": "ng",
         "build": "npm run build-lib",

diff --git a/projects/angular-auth-oidc-client/package.json b/projects/angular-auth-oidc-client/package.json
@@ -36,6 +36,6 @@
         "authorization"
     ],
     "license": "MIT",
-    "version": "11.1.0",
+    "version": "11.1.1",
     "description": "Angular Lib for OpenID Connect & OAuth2"
 }
diff --git a/projects/angular-auth-oidc-client/src/lib/callback/callback.service.ts b/projects/angular-auth-oidc-client/src/lib/callback/callback.service.ts
@@ -40,6 +40,10 @@ export class CallbackService {
         private authStateService: AuthStateService
     ) {}
 
+    isCallback(): boolean {
+        return this.urlService.isCallbackFromSts();
+    }
+
     handlePossibleStsCallback(currentCallbackUrl: string) {
         let callback$: Observable<any>;
 

diff --git a/...ects/angular-auth-oidc-client/src/lib/config-validation/config-validation.service.spec.ts b/...ects/angular-auth-oidc-client/src/lib/config-validation/config-validation.service.spec.ts
@@ -0,0 +1,93 @@
+import { TestBed } from '@angular/core/testing';
+import { LogLevel } from '../logging/log-level';
+import { LoggerService } from '../logging/logger.service';
+import { LoggerServiceMock } from '../logging/logger.service-mock';
+import { ConfigValidationService } from './config-validation.service';
+
+describe('Config Validation Service', () => {
+    let configValidationService: ConfigValidationService;
+    let loggerService: LoggerService;
+
+    beforeEach(() => {
+        TestBed.configureTestingModule({
+            providers: [ConfigValidationService, { provide: LoggerService, useClass: LoggerServiceMock }],
+        });
+    });
+
+    const VALID_CONFIG = {
+        stsServer: 'https://offeringsolutions-sts.azurewebsites.net',
+        redirectUrl: window.location.origin,
+        postLogoutRedirectUri: window.location.origin,
+        clientId: 'angularClient',
+        scope: 'openid profile email',
+        responseType: 'code',
+        silentRenew: true,
+        silentRenewUrl: `${window.location.origin}/silent-renew.html`,
+        renewTimeBeforeTokenExpiresInSeconds: 10,
+        logLevel: LogLevel.Debug,
+    };
+
+    beforeEach(() => {
+        configValidationService = TestBed.inject(ConfigValidationService);
+        loggerService = TestBed.inject(LoggerService);
+    });
+
+    it('should create', () => {
+        expect(configValidationService).toBeTruthy();
+    });
+
+    it('should return false for empty config', () => {
+        const config = {};
+        const result = configValidationService.validateConfig(config);
+        expect(result).toBeFalse();
+    });
+
+    it('should return true for valid config', () => {
+        const result = configValidationService.validateConfig(VALID_CONFIG);
+        expect(result).toBeTrue();
+    });
+
+    describe('ensure-clientId.rule', () => {
+        it('return false when no clientId is set', () => {
+            const config = { ...VALID_CONFIG, clientId: null };
+            const result = configValidationService.validateConfig(config);
+            expect(result).toBeFalse();
+        });
+    });
+
+    describe('ensure-sts-server.rule', () => {
+        it('return false when no sts server is set', () => {
+            const config = { ...VALID_CONFIG, stsServer: null };
+            const result = configValidationService.validateConfig(config);
+            expect(result).toBeFalse();
+        });
+    });
+
+    describe('ensure-redirect-url.rule', () => {
+        it('return false for no redirect Url', () => {
+            const config = { ...VALID_CONFIG, redirectUrl: '' };
+            const result = configValidationService.validateConfig(config);
+            expect(result).toBeFalse();
+        });
+    });
+
+    describe('ensureSilentRenewUrlWhenNoRefreshTokenUsed', () => {
+        it('return false when silent renew is used with no useRefreshToken and no silentrenewUrl', () => {
+            const config = { ...VALID_CONFIG, silentRenew: true, useRefreshToken: false, silentRenewUrl: null };
+            const result = configValidationService.validateConfig(config);
+            expect(result).toBeFalse();
+        });
+    });
+
+    describe('use-offline-scope-with-silent-renew.rule', () => {
+        it('return true but warning when silent renew is used with useRefreshToken but no offline_access scope is given', () => {
+            const config = { ...VALID_CONFIG, silentRenew: true, useRefreshToken: true, scopes: 'scope1 scope2 but_no_offline_access' };
+
+            const loggerSpy = spyOn(loggerService, 'logError');
+
+            const result = configValidationService.validateConfig(config);
+            expect(result).toBeFalse();
+            expect(loggerSpy).toHaveBeenCalled();
+        });
+    });
+});
diff --git a/projects/angular-auth-oidc-client/src/lib/config-validation/config-validation.service.ts b/projects/angular-auth-oidc-client/src/lib/config-validation/config-validation.service.ts
@@ -0,0 +1,28 @@
+import { Injectable } from '@angular/core';
+import { OpenIdConfiguration } from '../angular-auth-oidc-client';
+import { LoggerService } from '../logging/logger.service';
+import { Level, RuleValidationResult } from './rule';
+import { allRules } from './rules';
+
+@Injectable({ providedIn: 'root' })
+export class ConfigValidationService {
+    constructor(private loggerService: LoggerService) {}
+
+    validateConfig(passedConfig: OpenIdConfiguration): boolean {
+        const allValidationResults = allRules.map((rule) => rule(passedConfig));
+
+        const allMessages = allValidationResults.filter((x) => x.messages.length > 0);
+
+        const allErrorMessages = this.getAllMessagesOfType('error', allMessages);
+        const allWarnings = this.getAllMessagesOfType('warning', allMessages);
+        allErrorMessages.map((message) => this.loggerService.logError(message));
+        allWarnings.map((message) => this.loggerService.logWarning(message));
+
+        return allErrorMessages.length === 0;
+    }
+
+    private getAllMessagesOfType(type: Level, results: RuleValidationResult[]) {
+        const allMessages = results.filter((x) => x.level === type).map((result) => result.messages);
+        return allMessages.reduce((acc, val) => acc.concat(val), []);
+    }
+}
diff --git a/projects/angular-auth-oidc-client/src/lib/config-validation/rule.ts b/projects/angular-auth-oidc-client/src/lib/config-validation/rule.ts
@@ -0,0 +1,19 @@
+import { OpenIdConfiguration } from '../config/openid-configuration';
+
+export interface Rule {
+    validate(passedConfig: OpenIdConfiguration): RuleValidationResult;
+}
+
+export interface RuleValidationResult {
+    result: boolean;
+    messages: string[];
+    level: Level;
+}
+
+export const POSITIVE_VALIDATION_RESULT = {
+    result: true,
+    messages: [],
+    level: null,
+};
+
+export type Level = 'warning' | 'error';
diff --git a/projects/angular-auth-oidc-client/src/lib/config-validation/rules/ensure-clientId.rule.ts b/projects/angular-auth-oidc-client/src/lib/config-validation/rules/ensure-clientId.rule.ts
@@ -0,0 +1,14 @@
+import { OpenIdConfiguration } from '../../config/openid-configuration';
+import { POSITIVE_VALIDATION_RESULT, RuleValidationResult } from '../rule';
+
+export function ensureClientId(passedConfig: OpenIdConfiguration): RuleValidationResult {
+    if (!passedConfig.clientId) {
+        return {
+            result: false,
+            messages: ['The clientId is required  and missing from your config!'],
+            level: 'error',
+        };
+    }
+
+    return POSITIVE_VALIDATION_RESULT;
+}
diff --git a/...ects/angular-auth-oidc-client/src/lib/config-validation/rules/ensure-redirect-url.rule.ts b/...ects/angular-auth-oidc-client/src/lib/config-validation/rules/ensure-redirect-url.rule.ts
@@ -0,0 +1,14 @@
+import { OpenIdConfiguration } from '../../config/openid-configuration';
+import { POSITIVE_VALIDATION_RESULT, RuleValidationResult } from '../rule';
+
+export function ensureRedirectRule(passedConfig: OpenIdConfiguration): RuleValidationResult {
+    if (!passedConfig.redirectUrl) {
+        return {
+            result: false,
+            messages: ['The redirectURL is required and missing from your config'],
+            level: 'error',
+        };
+    }
+
+    return POSITIVE_VALIDATION_RESULT;
+}
diff --git a/...lient/src/lib/config-validation/rules/ensure-silentRenewUrl-with-no-refreshtokens.rule.ts b/...lient/src/lib/config-validation/rules/ensure-silentRenewUrl-with-no-refreshtokens.rule.ts
@@ -0,0 +1,18 @@
+import { OpenIdConfiguration } from '../../config/openid-configuration';
+import { POSITIVE_VALIDATION_RESULT, RuleValidationResult } from '../rule';
+
+export function ensureSilentRenewUrlWhenNoRefreshTokenUsed(passedConfig: OpenIdConfiguration): RuleValidationResult {
+    const usesSilentRenew = passedConfig.silentRenew;
+    const usesRefreshToken = passedConfig.useRefreshToken;
+    const hasSilentRenewUrl = passedConfig.silentRenewUrl;
+
+    if (usesSilentRenew && !usesRefreshToken && !hasSilentRenewUrl) {
+        return {
+            result: false,
+            messages: ['Please provide a silent renew URL if using renew and not refresh tokens'],
+            level: 'error',
+        };
+    }
+
+    return POSITIVE_VALIDATION_RESULT;
+}
diff --git a/projects/angular-auth-oidc-client/src/lib/config-validation/rules/ensure-sts-server.rule.ts b/projects/angular-auth-oidc-client/src/lib/config-validation/rules/ensure-sts-server.rule.ts
@@ -0,0 +1,14 @@
+import { OpenIdConfiguration } from '../../config/openid-configuration';
+import { POSITIVE_VALIDATION_RESULT, RuleValidationResult } from '../rule';
+
+export function ensureStsServer(passedConfig: OpenIdConfiguration): RuleValidationResult {
+    if (!passedConfig.stsServer) {
+        return {
+            result: false,
+            messages: ['The STS URL MUST be provided in the configuration!'],
+            level: 'error',
+        };
+    }
+
+    return POSITIVE_VALIDATION_RESULT;
+}
diff --git a/projects/angular-auth-oidc-client/src/lib/config-validation/rules/index.ts b/projects/angular-auth-oidc-client/src/lib/config-validation/rules/index.ts
@@ -0,0 +1,13 @@
+import { ensureClientId } from './ensure-clientId.rule';
+import { ensureRedirectRule } from './ensure-redirect-url.rule';
+import { ensureSilentRenewUrlWhenNoRefreshTokenUsed } from './ensure-silentRenewUrl-with-no-refreshtokens.rule';
+import { ensureStsServer } from './ensure-sts-server.rule';
+import { useOfflineScopeWithSilentRenew } from './use-offline-scope-with-silent-renew.rule';
+
+export const allRules = [
+    ensureStsServer,
+    useOfflineScopeWithSilentRenew,
+    ensureRedirectRule,
+    ensureClientId,
+    ensureSilentRenewUrlWhenNoRefreshTokenUsed,
+];
diff --git a/...h-oidc-client/src/lib/config-validation/rules/use-offline-scope-with-silent-renew.rule.ts b/...h-oidc-client/src/lib/config-validation/rules/use-offline-scope-with-silent-renew.rule.ts
@@ -0,0 +1,19 @@
+import { OpenIdConfiguration } from '../../config/openid-configuration';
+import { POSITIVE_VALIDATION_RESULT, RuleValidationResult } from '../rule';
+
+export function useOfflineScopeWithSilentRenew(passedConfig: OpenIdConfiguration): RuleValidationResult {
+    const hasRefreshToken = passedConfig.useRefreshToken;
+    const hasSilentRenew = passedConfig.silentRenew;
+    const scope = passedConfig.scope || '';
+    const hasOfflineScope = scope.split(' ').includes('offline_access');
+
+    if (hasRefreshToken && hasSilentRenew && !hasOfflineScope) {
+        return {
+            result: false,
+            messages: ['When using silent renew and refresh tokens please set the `offline_access` scope'],
+            level: 'error',
+        };
+    }
+
+    return POSITIVE_VALIDATION_RESULT;
+}
diff --git a/projects/angular-auth-oidc-client/src/lib/config/config.service.spec.ts b/projects/angular-auth-oidc-client/src/lib/config/config.service.spec.ts
@@ -2,6 +2,7 @@ import { async, TestBed } from '@angular/core/testing';
 import { of } from 'rxjs';
 import { DataService } from '../api/data.service';
 import { DataServiceMock } from '../api/data.service-mock';
+import { ConfigValidationService } from '../config-validation/config-validation.service';
 import { LoggerService } from '../logging/logger.service';
 import { LoggerServiceMock } from '../logging/logger.service-mock';
 import { EventTypes } from '../public-events/event-types';
@@ -22,6 +23,7 @@ describe('Configuration Service', () => {
     let dataService: DataService;
     let authWellKnownService: AuthWellKnownService;
     let storagePersistanceService: StoragePersistanceService;
+    let configValidationService: ConfigValidationService;
 
     beforeEach(() => {
         TestBed.configureTestingModule({
@@ -48,6 +50,7 @@ describe('Configuration Service', () => {
                     useClass: StoragePersistanceServiceMock,
                 },
                 PublicEventsService,
+                ConfigValidationService,
             ],
         });
     });
@@ -60,6 +63,7 @@ describe('Configuration Service', () => {
         dataService = TestBed.inject(DataService);
         authWellKnownService = TestBed.inject(AuthWellKnownService);
         storagePersistanceService = TestBed.inject(StoragePersistanceService);
+        configValidationService = TestBed.inject(ConfigValidationService);
     });
 
     it('should create', () => {
@@ -71,9 +75,10 @@ describe('Configuration Service', () => {
     });
 
     describe('withConfig', () => {
-        it('no given sts server does nothing and logs error', async(() => {
+        it('not valid config does nothing and logs error', async(() => {
             const config = {};
             spyOn(loggerService, 'logError');
+            spyOn(configValidationService, 'validateConfig').and.returnValue(false);
 
             const promise = oidcConfigService.withConfig(config);
 
@@ -86,6 +91,7 @@ describe('Configuration Service', () => {
             const config = { stsServer: 'stsServerForTesting', authWellknownEndpoint: null };
             spyOnProperty(storagePersistanceService, 'authWellKnownEndPoints', 'get').and.returnValue({ any: 'thing' });
             const eventServiceSpy = spyOn(eventsService, 'fireEvent');
+            spyOn(configValidationService, 'validateConfig').and.returnValue(true);
 
             const promise = oidcConfigService.withConfig(config);
 
@@ -104,6 +110,7 @@ describe('Configuration Service', () => {
             const config = { stsServer: 'stsServerForTesting', authWellknownEndpoint: null };
             const authWellKnown = { issuer: 'issuerForTesting' };
             spyOnProperty(storagePersistanceService, 'authWellKnownEndPoints', 'get').and.returnValue(null);
+            spyOn(configValidationService, 'validateConfig').and.returnValue(true);
             const eventServiceSpy = spyOn(eventsService, 'fireEvent');
             const storeWellKnownEndpointsSpy = spyOn(oidcConfigService as any, 'storeWellKnownEndpoints');
 
@@ -125,6 +132,7 @@ describe('Configuration Service', () => {
             const config = { stsServer: 'stsServerForTesting', eagerLoadAuthWellKnownEndpoints: true };
             spyOnProperty(storagePersistanceService, 'authWellKnownEndPoints', 'get').and.returnValue(null);
             spyOn(configurationProvider, 'setConfig').and.returnValue(config);
+            spyOn(configValidationService, 'validateConfig').and.returnValue(true);
             const getWellKnownEndPointsFromUrlSpy = spyOn(authWellKnownService, 'getWellKnownEndPointsFromUrl').and.returnValue(of(null));
 
             const promise = oidcConfigService.withConfig(config);
@@ -139,6 +147,7 @@ describe('Configuration Service', () => {
             spyOnProperty(storagePersistanceService, 'authWellKnownEndPoints', 'get').and.returnValue(null);
             const storeWellKnownEndpointsSpy = spyOn(oidcConfigService as any, 'storeWellKnownEndpoints').and.returnValue(false);
             spyOn(configurationProvider, 'setConfig').and.returnValue(config);
+            spyOn(configValidationService, 'validateConfig').and.returnValue(true);
             spyOn(authWellKnownService, 'getWellKnownEndPointsFromUrl').and.returnValue(of({ issuer: 'issuerForTesting' }));
 
             const promise = oidcConfigService.withConfig(config);
@@ -153,6 +162,7 @@ describe('Configuration Service', () => {
             spyOnProperty(storagePersistanceService, 'authWellKnownEndPoints', 'get').and.returnValue(null);
             spyOn(oidcConfigService as any, 'storeWellKnownEndpoints').and.returnValue(false);
             spyOn(configurationProvider, 'setConfig').and.returnValue(config);
+            spyOn(configValidationService, 'validateConfig').and.returnValue(true);
             spyOn(authWellKnownService, 'getWellKnownEndPointsFromUrl').and.returnValue(of({ issuer: 'issuerForTesting' }));
             const eventServiceSpy = spyOn(eventsService, 'fireEvent');