Skip to content

Commit

Permalink
Add tests for bypassing expiry
Browse files Browse the repository at this point in the history
  • Loading branch information
timvanoostrom committed Nov 28, 2024
1 parent 3cbfd0a commit 6addd1a
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 30 deletions.
34 changes: 27 additions & 7 deletions src/server/auth/auth-helpers.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { millisecondsToSeconds } from 'date-fns';
import { Request } from 'express';

import {
Expand All @@ -23,6 +24,7 @@ import {
RequestMock,
ResponseMock,
} from '../../testing/utils';
import { ONE_MINUTE_SECONDS } from '../config/app';
import * as blacklist from '../services/session-blacklist';

describe('auth-helpers', () => {
Expand Down Expand Up @@ -56,6 +58,7 @@ describe('auth-helpers', () => {
aud: 'test1',
[EH_ATTR_PRIMARY_ID]: 'EHERKENNING-KVK',
sid: 'test',
id: 'xx-bsn-xx',
} as TokenData
);

Expand Down Expand Up @@ -123,6 +126,7 @@ describe('auth-helpers', () => {
aud: 'test1',
[EH_ATTR_PRIMARY_ID]: 'EH-KVK1',
sid: 'test3',
id: 'xx-bsn-xx',
} as TokenData
);

Expand All @@ -146,6 +150,7 @@ describe('auth-helpers', () => {
aud: 'test1',
[EH_ATTR_PRIMARY_ID_LEGACY]: 'EH-KVK1',
sid: 'test4',
id: 'xx-bsn-xx',
} as TokenData
);

Expand All @@ -170,6 +175,7 @@ describe('auth-helpers', () => {
[EH_ATTR_INTERMEDIATE_PRIMARY_ID]: 'EH-KVK1',
[EH_ATTR_INTERMEDIATE_SECONDARY_ID]: 'EH-KVK2',
sid: 'test5',
id: 'xx-bsn-xx',
} as TokenData
);

Expand All @@ -192,19 +198,21 @@ describe('auth-helpers', () => {

const resMock = ResponseMock.new();

(resMock as any).oidc = {
logout: vi.fn(),
};

const addToBlackListSpy = vi.spyOn(blacklist, 'addToBlackList');

const nowInSeconds = millisecondsToSeconds(Date.now());

beforeEach(() => {
resMock.oidc.logout.mockClear();
addToBlackListSpy.mockClear();
});

test('Authenticated IDP logout', async () => {
test('Authenticated IDP logout calls oidc.logout', async () => {
const handler = createLogoutHandler('http://foo.bar');

reqMock[OIDC_SESSION_COOKIE_NAME]!.expires_at =
nowInSeconds + ONE_MINUTE_SECONDS; // Expires in one minute

await handler(reqMock, resMock);

expect(resMock.oidc.logout).toHaveBeenCalledWith({
Expand All @@ -220,16 +228,28 @@ describe('auth-helpers', () => {
);
});

test('Local logout', async () => {
const handler2 = createLogoutHandler('http://foo.bar', false);
test('Expired authenticated IDP logout should not call oidc.logout', async () => {
const handler = createLogoutHandler('http://foo.bar');

reqMock[OIDC_SESSION_COOKIE_NAME]!.expires_at =
nowInSeconds - ONE_MINUTE_SECONDS; // Expired one minute ago

await handler(reqMock, resMock);

expect(resMock.oidc.logout).not.toHaveBeenCalled();
expect(addToBlackListSpy).not.toHaveBeenCalled();
});

test('Local logout should not call oidc.logout', async () => {
const handler2 = createLogoutHandler('http://foo.bar', false);
await handler2(reqMock, resMock);

expect(resMock.clearCookie).toHaveBeenCalled();
expect(resMock.redirect).toHaveBeenCalledWith('http://foo.bar');
expect(addToBlackListSpy).not.toHaveBeenCalled();
});
});

describe('getReturnToUrl', () => {
test('getReturnToUrl should return the corrent zaak-status url', () => {
const url = getReturnToUrl({
Expand Down
12 changes: 7 additions & 5 deletions src/server/auth/auth-helpers.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { millisecondsToSeconds } from 'date-fns/millisecondsToSeconds';
import type { Request, Response } from 'express';
import * as jose from 'jose';
import { ParsedQs } from 'qs';
Expand All @@ -18,7 +19,6 @@ import {
} from './auth-types';
import { FeatureToggle } from '../../universal/config/feature-toggles';
import { AppRoutes } from '../../universal/config/routes';
import { ONE_SECOND_MS } from '../config/app';
import { ExternalConsumerEndpoints } from '../routing/bff-routes';
import { generateFullApiUrlBFF } from '../routing/route-helpers';
import { captureException } from '../services/monitoring';
Expand Down Expand Up @@ -128,8 +128,8 @@ export function decodeToken<T extends Record<string, string>>(
return jose.decodeJwt(jwtToken) as unknown as T;
}

function isIDPSessionExpired(expiresAt: string) {
return new Date(parseInt(expiresAt, 10) * ONE_SECOND_MS) < new Date();
function isIDPSessionExpired(expiresAtInSeconds: number) {
return expiresAtInSeconds < millisecondsToSeconds(Date.now());
}

export function createLogoutHandler(
Expand All @@ -139,9 +139,11 @@ export function createLogoutHandler(
return async (req: AuthenticatedRequest, res: Response) => {
const auth = getAuth(req);
if (
doIDPLogout &&
auth &&
req.oidc.isAuthenticated() &&
(doIDPLogout ? isIDPSessionExpired(auth.expiresAt) : false)
auth.expiresAt &&
!isIDPSessionExpired(auth.expiresAt) &&
req.oidc.isAuthenticated()
) {
// Add the session ID to a blacklist. This way the jwt id_token, which itself has longer lifetime, cannot be reused after logging out at IDP.
if (auth.profile.sid) {
Expand Down
4 changes: 2 additions & 2 deletions src/server/auth/auth-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ export interface AuthProfile {
sid: SessionID;
}

export interface MaSession extends Session {
export interface MaSession extends Omit<Session, 'expires_at'> {
sid: SessionID;
TMASessionID: string; // TMA Session ID
profileType: ProfileType;
authMethod: AuthMethod;
expires_at: string;
expires_at: number;
}

export interface AuthProfileAndToken {
Expand Down
4 changes: 2 additions & 2 deletions src/server/helpers/file-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ import { describe, expect, it, vi } from 'vitest';
import FileCache from './file-cache';

vi.mock('flat-cache', () => {
const cache: { [key: string]: any } = {};
const cache: { [key: string]: unknown } = {};

return {
default: vi.fn(),
setKey: vi.fn(),
create: () => ({
set: (key: string, data: any) => {
set: (key: string, data: unknown) => {
cache[key] = data;
},
get: (key: string) => cache[key],
Expand Down
2 changes: 2 additions & 0 deletions src/server/routing/router-oidc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ describe('router-oids', () => {
authMethod: 'digid',
isAuthenticated: true,
profileType: 'private',
expiresAt: expect.any(Number),
},
status: 'OK',
});
Expand All @@ -64,6 +65,7 @@ describe('router-oids', () => {
authMethod: 'eherkenning',
isAuthenticated: true,
profileType: 'commercial',
expiresAt: expect.any(Number),
},
status: 'OK',
});
Expand Down
16 changes: 9 additions & 7 deletions src/server/services/buurt/buurt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
DEFAULT_TRIES_UNTIL_CONSIDERED_STALE,
DatasetConfig,
DatasetFeatureProperties,
DatasetFeatures,
} from './datasets';
import {
createDynamicFilterConfig,
Expand All @@ -28,7 +29,6 @@ import { jsonCopy, omit } from '../../../universal/helpers/utils';
import FileCache from '../../helpers/file-cache';
import { requestData } from '../../helpers/source-api-request';


const DUMMY_DATA_RESPONSE = apiSuccessResult([
{ properties: { foo: 'bar', bar: undefined } },
{ properties: { foo: 'hop', bar: true } },
Expand All @@ -54,7 +54,7 @@ const datasetId3 = 'test-dataset-error';
const datasetConfig: DatasetConfig = {
listUrl: 'http://url.to/api/foo',
detailUrl: 'http://url.to/api/detail/foo/',
transformList: (r: any) => r,
transformList: (r: unknown) => r as DatasetFeatures<DatasetFeatureProperties>,
featureType: 'Point',
cacheTimeMinutes: BUURT_CACHE_TTL_1_DAY_IN_MINUTES,
triesUntilConsiderdStale: 5,
Expand All @@ -63,7 +63,7 @@ const datasetConfig: DatasetConfig = {
const datasetConfig2: DatasetConfig = {
listUrl: 'http://url.to/api/hello',
detailUrl: 'http://url.to/api/detail/hello/',
transformList: (r: any) => r,
transformList: (r: unknown) => r as DatasetFeatures<DatasetFeatureProperties>,
featureType: 'Point',
cacheTimeMinutes: BUURT_CACHE_TTL_1_DAY_IN_MINUTES,
triesUntilConsiderdStale: 5,
Expand All @@ -72,7 +72,7 @@ const datasetConfig2: DatasetConfig = {
const datasetConfig3: DatasetConfig = {
listUrl: 'http://url.to/api/not-found',
detailUrl: 'http://url.to/api/not-found/hello/',
transformList: (r: any) => r,
transformList: (r: unknown) => r as DatasetFeatures<DatasetFeatureProperties>,
featureType: 'Point',
cacheTimeMinutes: BUURT_CACHE_TTL_1_DAY_IN_MINUTES,
triesUntilConsiderdStale: 5,
Expand Down Expand Up @@ -382,15 +382,17 @@ describe('Buurt services', () => {
const detailItemId = 'x-detail';

datasetConfig2.transformDetail = (
responseData: any,
responseData: unknown,
options: DatasetFeatureProperties
) => {
const cachedFeatures = options.datasetCache.getKey('features');
const cachedFeatures = (options.datasetCache as FileCache).getKey<
string[]
>('features');
expect(responseData).toBe(null);
expect(options.id).toBe(detailItemId);
expect(options.datasetId).toBe(datasetId);
expect(cachedFeatures).toStrictEqual(features);
return cachedFeatures[0];
return cachedFeatures?.[0];
};

(getDatasetEndpointConfig as Mock).mockReturnValueOnce([
Expand Down
30 changes: 23 additions & 7 deletions src/testing/utils.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import { millisecondsToSeconds } from 'date-fns';
import { Request, Response } from 'express';
import nock from 'nock';
import UID from 'uid-safe';

import { bffApiHost, remoteApiHost } from './setup';
import { AuthProfile, AuthProfileAndToken } from '../server/auth/auth-types';
import { OIDC_SESSION_COOKIE_NAME } from '../server/auth/auth-config';
import {
AuthenticatedRequest,
AuthProfile,
AuthProfileAndToken,
} from '../server/auth/auth-types';
import { createOIDCStub } from '../server/routing/router-development';
import { HTTP_STATUS_CODES } from '../universal/constants/errorCodes';

Expand Down Expand Up @@ -50,9 +56,10 @@ export function getAuthProfileAndToken(
export class ResponseMock {
statusCode: number;
locals: { requestID: string };
oidc = { logout: vi.fn() };

static new() {
return new ResponseMock() as unknown as Response;
return new ResponseMock() as unknown as Response & ResponseMock;
}

private constructor() {
Expand Down Expand Up @@ -108,9 +115,13 @@ export class RequestMock {
return this;
}

async createOIDCStub(authProfile: AuthProfile) {
const self = this as unknown as Request;
async createOIDCStub(authProfile: AuthProfile, expiresAt?: number) {
const self = this as unknown as AuthenticatedRequest;
await createOIDCStub(self, authProfile);
const sessionObj = self[OIDC_SESSION_COOKIE_NAME];
if (sessionObj) {
sessionObj.expires_at = expiresAt ?? millisecondsToSeconds(Date.now());
}
return this;
}

Expand All @@ -119,10 +130,15 @@ export class RequestMock {
}
}

export async function getReqMockWithOidc(profile: AuthProfile) {
export async function getReqMockWithOidc(
profile: AuthProfile,
expiresAt?: number
) {
const reqMockWithOidc = RequestMock.new();
await reqMockWithOidc.createOIDCStub(profile);
return reqMockWithOidc.get();
await reqMockWithOidc.createOIDCStub(profile, expiresAt);
const mock = reqMockWithOidc.get() as unknown as AuthenticatedRequest;

return mock;
}

export const DEV_JWT =
Expand Down

0 comments on commit 6addd1a

Please sign in to comment.