Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: Updated URL service isCallbackFromSts #1976

Merged
merged 17 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,15 @@ Override the default Security Token Service well-known endpoint postfix.

This is the `redirect_url` which was configured on the Security Token Service (STS).

### `checkRedirectUrlWhenCheckingIfIsCallback`

- Type: `boolean`
- Required: `false`

Whether to check if current URL matches the redirect URI when determining if current URL is in fact the redirect URI.

Default = _true_

### `clientId`

- Type: `string`
Expand Down
22 changes: 17 additions & 5 deletions projects/angular-auth-oidc-client/src/lib/api/data.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import { HttpHeaders, provideHttpClient, withInterceptorsFromDi } from '@angular/common/http';
import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing';
import {
HttpHeaders,
provideHttpClient,
withInterceptorsFromDi,
} from '@angular/common/http';
import {
HttpTestingController,
provideHttpClientTesting,
} from '@angular/common/http/testing';
import { TestBed, waitForAsync } from '@angular/core/testing';
import { DataService } from './data.service';
import { HttpBaseService } from './http-base.service';
Expand All @@ -10,9 +17,14 @@ describe('Data Service', () => {

beforeEach(() => {
TestBed.configureTestingModule({
imports: [],
providers: [DataService, HttpBaseService, provideHttpClient(withInterceptorsFromDi()), provideHttpClientTesting()]
});
imports: [],
providers: [
DataService,
HttpBaseService,
provideHttpClient(withInterceptorsFromDi()),
provideHttpClientTesting(),
],
});
});

beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ export class CheckAuthService {
return of(result);
}

const isCallback = this.callbackService.isCallback(currentUrl);
const isCallback = this.callbackService.isCallback(currentUrl, config);

this.loggerService.logDebug(
config,
Expand Down
13 changes: 10 additions & 3 deletions projects/angular-auth-oidc-client/src/lib/auth.module.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
import { CommonModule } from '@angular/common';
import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http';
import {
provideHttpClient,
withInterceptorsFromDi,
} from '@angular/common/http';
import { ModuleWithProviders, NgModule } from '@angular/core';
import { PassedInitialConfig } from './auth-config';
import { _provideAuth } from './provide-auth';

@NgModule({ declarations: [],
exports: [], imports: [CommonModule], providers: [provideHttpClient(withInterceptorsFromDi())] })
@NgModule({
declarations: [],
exports: [],
imports: [CommonModule],
providers: [provideHttpClient(withInterceptorsFromDi())],
})
export class AuthModule {
static forRoot(
passedConfig: PassedInitialConfig
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,8 +452,10 @@ describe(`AutoLoginPartialRoutesGuard`, () => {
);
const loginSpy = spyOn(loginService, 'login');

const guard$ = TestBed.runInInjectionContext(
() => autoLoginPartialRoutesGuard({data: {custom: 'param'}} as unknown as ActivatedRouteSnapshot)
const guard$ = TestBed.runInInjectionContext(() =>
autoLoginPartialRoutesGuard({
data: { custom: 'param' },
} as unknown as ActivatedRouteSnapshot)
);

guard$.subscribe(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ export class AutoLoginPartialRoutesGuard {
}
}

export function autoLoginPartialRoutesGuard(route?: ActivatedRouteSnapshot): Observable<boolean> {
export function autoLoginPartialRoutesGuard(
route?: ActivatedRouteSnapshot
): Observable<boolean> {
const configurationService = inject(ConfigurationService);
const authStateService = inject(AuthStateService);
const loginService = inject(LoginService);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('CallbackService ', () => {
const urlServiceSpy = spyOn(urlService, 'isCallbackFromSts');

callbackService.isCallback('anyUrl');
expect(urlServiceSpy).toHaveBeenCalledOnceWith('anyUrl');
expect(urlServiceSpy).toHaveBeenCalledOnceWith('anyUrl', undefined);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ export class CallbackService {
return this.stsCallbackInternal$.asObservable();
}

isCallback(currentUrl: string): boolean {
isCallback(currentUrl: string, config?: OpenIdConfiguration): boolean {
if (!currentUrl) {
return false;
}

return this.urlService.isCallbackFromSts(currentUrl);
return this.urlService.isCallbackFromSts(currentUrl, config);
}

handleCallbackAndFireEvents(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,3 @@ export class IntervalService {
});
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ describe('AuthWellKnownDataService', () => {
const urlWithSuffix = `${urlWithoutSuffix}/.well-known/test-openid-configuration`;

(service as any)
.getWellKnownDocument(urlWithoutSuffix, { configId: 'configId1', authWellknownUrlSuffix: '/.well-known/test-openid-configuration' })
.getWellKnownDocument(urlWithoutSuffix, {
configId: 'configId1',
authWellknownUrlSuffix: '/.well-known/test-openid-configuration',
})
.subscribe(() => {
expect(dataServiceSpy).toHaveBeenCalledOnceWith(urlWithSuffix, {
configId: 'configId1',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,17 @@ describe('AuthWellKnownService', () => {

describe('getAuthWellKnownEndPoints', () => {
it('getAuthWellKnownEndPoints throws an error if not config provided', waitForAsync(() => {
service
.queryAndStoreAuthWellKnownEndPoints(null)
.subscribe({
error: (error) => {
expect(error).toEqual(new Error('Please provide a configuration before setting up the module'))
}
});
service.queryAndStoreAuthWellKnownEndPoints(null).subscribe({
error: (error) => {
expect(error).toEqual(
new Error(
'Please provide a configuration before setting up the module'
)
);
},
});
}));


it('getAuthWellKnownEndPoints calls always dataservice', waitForAsync(() => {
const dataServiceSpy = spyOn(
dataService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export const DEFAULT_CONFIG: OpenIdConfiguration = {
authWellknownEndpointUrl: '',
authWellknownEndpoints: undefined,
redirectUrl: 'https://please_set',
checkRedirectUrlWhenCheckingIfIsCallback: true,
clientId: 'please_set',
responseType: 'code',
scope: 'openid email profile',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,21 @@ export interface OpenIdConfiguration {
authWellknownEndpointUrl?: string;
authWellknownEndpoints?: AuthWellKnownEndpoints;

/**
* Override the default Security Token Service wellknown endpoint postfix.
*
* @default /.well-known/openid-configuration
*/
/**
* Override the default Security Token Service wellknown endpoint postfix.
*
* @default /.well-known/openid-configuration
*/
authWellknownUrlSuffix?: string;

/** The redirect URL defined on the Security Token Service. */
redirectUrl?: string;
/**
* Whether to check if current URL matches the redirect URI when determining
* if current URL is in fact the redirect URI.
* Default: true
*/
checkRedirectUrlWhenCheckingIfIsCallback?: boolean;
/**
* The Client MUST validate that the aud (audience) Claim contains its `client_id` value
* registered at the Issuer identified by the iss (issuer) Claim as an audience.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ export class ConfigValidationService {

private validateConfigsInternal(
passedConfigs: OpenIdConfiguration[],
allRulesToUse: ((passedConfig: OpenIdConfiguration[]) => RuleValidationResult)[]
allRulesToUse: ((
passedConfig: OpenIdConfiguration[]
) => RuleValidationResult)[]
): boolean {
if (passedConfigs.length === 0) {
return false;
Expand All @@ -47,7 +49,9 @@ export class ConfigValidationService {

private validateConfigInternal(
passedConfig: OpenIdConfiguration,
allRulesToUse: ((passedConfig: OpenIdConfiguration) => RuleValidationResult)[]
allRulesToUse: ((
passedConfig: OpenIdConfiguration
) => RuleValidationResult)[]
): boolean {
const allValidationResults = allRulesToUse.map((rule) =>
rule(passedConfig)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ describe('CodeFlowCallbackHandlerService', () => {

getUrlParameterSpy.withArgs('test-url', 'state').and.returnValue('');

service.codeFlowCallback('test-url', { configId: 'configId1' }).subscribe({
error: (err) => {
expect(err).toBeTruthy();
},
});
service
.codeFlowCallback('test-url', { configId: 'configId1' })
.subscribe({
error: (err) => {
expect(err).toBeTruthy();
},
});
}));

it('throws error if no code is given', waitForAsync(() => {
Expand All @@ -69,11 +71,13 @@ describe('CodeFlowCallbackHandlerService', () => {

getUrlParameterSpy.withArgs('test-url', 'code').and.returnValue('');

service.codeFlowCallback('test-url', { configId: 'configId1' }).subscribe({
error: (err) => {
expect(err).toBeTruthy();
},
});
service
.codeFlowCallback('test-url', { configId: 'configId1' })
.subscribe({
error: (err) => {
expect(err).toBeTruthy();
},
});
}));

it('returns callbackContext if all params are good', waitForAsync(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,12 @@ export class HistoryJwtKeysCallbackHandlerService {
): void {
let validationResult = ValidationResult.SecureTokenServerError;

if (result && typeof result === 'object' && 'error' in result && (result.error as string) === 'login_required') {
if (
result &&
typeof result === 'object' &&
'error' in result &&
(result.error as string) === 'login_required'
) {
validationResult = ValidationResult.LoginRequired;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class CheckSessionService implements OnDestroy {

private checkSessionReceived = false;

private scheduledHeartBeatRunning: number|null= null
private scheduledHeartBeatRunning: number | null = null;

private lastIFrameRefresh = 0;

Expand All @@ -45,7 +45,10 @@ export class CheckSessionService implements OnDestroy {
false
);

private iframeMessageEventListener?: (this:Window, ev: MessageEvent<any>) => any;
private iframeMessageEventListener?: (
this: Window,
ev: MessageEvent<any>
) => any;

get checkSessionChanged$(): Observable<boolean> {
return this.checkSessionChangedInternal$.asObservable();
Expand Down Expand Up @@ -224,10 +227,11 @@ export class CheckSessionService implements OnDestroy {
}

this.zone.runOutsideAngular(() => {
this.scheduledHeartBeatRunning = this.document?.defaultView?.setTimeout(
() => this.zone.run(pollServerSessionRecur),
this.heartBeatInterval
) ?? null;
this.scheduledHeartBeatRunning =
this.document?.defaultView?.setTimeout(
() => this.zone.run(pollServerSessionRecur),
this.heartBeatInterval
) ?? null;
});
});
};
Expand All @@ -236,7 +240,7 @@ export class CheckSessionService implements OnDestroy {
}

private clearScheduledHeartBeat(): void {
if(this.scheduledHeartBeatRunning !== null) {
if (this.scheduledHeartBeatRunning !== null) {
clearTimeout(this.scheduledHeartBeatRunning);
this.scheduledHeartBeatRunning = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import {
withInterceptors,
withInterceptorsFromDi,
} from '@angular/common/http';
import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing';
import {
HttpTestingController,
provideHttpClientTesting,
} from '@angular/common/http/testing';
import { TestBed, waitForAsync } from '@angular/core/testing';
import { mockProvider } from '../../test/auto-mock';
import { AuthStateService } from '../auth-state/auth-state.service';
Expand Down Expand Up @@ -233,7 +236,7 @@ describe(`AuthHttpInterceptor`, () => {
]);
spyOn(
closestMatchingRouteService,
'getConfigIdForClosestMatchingRoute',
'getConfigIdForClosestMatchingRoute'
).and.returnValue({
matchingRoute: null,
matchingConfig: null,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { HttpEvent, HttpHandler, HttpHandlerFn, HttpInterceptor, HttpInterceptorFn, HttpRequest } from '@angular/common/http';
import {
HttpEvent,
HttpHandler,
HttpHandlerFn,
HttpInterceptor,
HttpInterceptorFn,
HttpRequest,
} from '@angular/common/http';
import { Injectable, inject } from '@angular/core';
import { Observable } from 'rxjs';
import { AuthStateService } from '../auth-state/auth-state.service';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ import { Injectable } from '@angular/core';
*/
@Injectable({ providedIn: 'root' })
export abstract class AbstractLoggerService {
abstract logError(message: string|object, ...args: any[]): void;
abstract logError(message: string | object, ...args: any[]): void;

abstract logWarning(message: string|object, ...args: any[]): void;
abstract logWarning(message: string | object, ...args: any[]): void;

abstract logDebug(message: string|object, ...args: any[]): void;
abstract logDebug(message: string | object, ...args: any[]): void;
}

Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ import { AbstractLoggerService } from './abstract-logger.service';

@Injectable({ providedIn: 'root' })
export class ConsoleLoggerService implements AbstractLoggerService {
logError(message: string|object, ...args: any[]): void {
logError(message: string | object, ...args: any[]): void {
console.error(message, ...args);
}

logWarning(message: string|object, ...args: any[]): void {
logWarning(message: string | object, ...args: any[]): void {
console.warn(message, ...args);
}

logDebug(message: string|object, ...args: any[]): void {
logDebug(message: string | object, ...args: any[]): void {
console.debug(message, ...args);
}
}
Loading
Loading