MIJN-9699-Bug - Bypasss IDP logout if token is expired

Dockerfile

@@ -30,6 +30,7 @@ WORKDIR /build-space
 COPY package-lock.json /build-space/
 COPY package.json /build-space/
 COPY vite.config.ts /build-space/
+COPY .env.local.template /build-space/
 COPY vendor /build-space/vendor
 COPY mocks/fixtures /build-space/mocks/fixtures
 
@@ -74,9 +75,6 @@ ENV MA_TEST_ACCOUNTS=$MA_TEST_ACCOUNTS
 ARG REACT_APP_BFF_API_URL=/api/v1
 ENV REACT_APP_BFF_API_URL=$REACT_APP_BFF_API_URL
 
-ARG REACT_APP_SENTRY_DSN=
-ENV REACT_APP_SENTRY_DSN=$REACT_APP_SENTRY_DSN
-
 ARG REACT_APP_ANALYTICS_ID=
 ENV REACT_APP_ANALYTICS_ID=$REACT_APP_ANALYTICS_ID
 

src/server/app.test.ts

@@ -25,27 +25,35 @@ describe('app', async () => {
     vi.resetModules();
   });
 
-  test('app middleware OT', async () => {
-    const appModule = await import('./app');
-    const app = appModule.forTesting.app;
-    expect(
-      app._router.stack.some((layer: ILayer) =>
-        'BFF_ID' in layer.handle ? layer.handle.BFF_ID === 'router-dev' : false
-      )
-    ).toBe(true);
-    expect(
-      app._router.stack.some((layer: ILayer) =>
-        'BFF_ID' in layer.handle ? layer.handle.BFF_ID === 'router-oidc' : false
-      )
-    ).toBe(false);
-    expect(
-      app._router.stack.some((layer: ILayer) =>
-        'BFF_ID' in layer.handle
-          ? layer.handle.BFF_ID === 'router-protected'
-          : false
-      )
-    ).toBe(true);
-  });
+  test(
+    'app middleware OT',
+    async () => {
+      const appModule = await import('./app');
+      const app = appModule.forTesting.app;
+      expect(
+        app._router.stack.some((layer: ILayer) =>
+          'BFF_ID' in layer.handle
+            ? layer.handle.BFF_ID === 'router-dev'
+            : false
+        )
+      ).toBe(true);
+      expect(
+        app._router.stack.some((layer: ILayer) =>
+          'BFF_ID' in layer.handle
+            ? layer.handle.BFF_ID === 'router-oidc'
+            : false
+        )
+      ).toBe(false);
+      expect(
+        app._router.stack.some((layer: ILayer) =>
+          'BFF_ID' in layer.handle
+            ? layer.handle.BFF_ID === 'router-protected'
+            : false
+        )
+      ).toBe(true);
+    },
+    { timeout: 10000 }
+  );
 
   test('app middleware AP', async () => {
     mocks.IS_AP = true;

src/server/auth/auth-config.ts

@@ -51,7 +51,6 @@ export const oidcConfigBase: ConfigParams = {
       getFromEnv('MA_APP_MODE') !== 'unittest'
         ? getSessionStore(openIdAuth as typeof expressSession, {
             tableName: OIDC_SESSIONS_TABLE_NAME,
-            ttlSeconds: OIDC_SESSION_MAX_AGE_SECONDS, // NOTE: Not sure if this works with rollingDuration
           })
         : undefined,
   },

src/server/auth/auth-helpers.test.ts

@@ -1,3 +1,4 @@
+import { millisecondsToSeconds } from 'date-fns';
 import { Request } from 'express';
 
 import {
@@ -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', () => {
@@ -56,6 +58,7 @@ describe('auth-helpers', () => {
           aud: 'test1',
           [EH_ATTR_PRIMARY_ID]: 'EHERKENNING-KVK',
           sid: 'test',
+          id: 'xx-bsn-xx',
         } as TokenData
       );
 
@@ -123,6 +126,7 @@ describe('auth-helpers', () => {
           aud: 'test1',
           [EH_ATTR_PRIMARY_ID]: 'EH-KVK1',
           sid: 'test3',
+          id: 'xx-bsn-xx',
         } as TokenData
       );
 
@@ -146,6 +150,7 @@ describe('auth-helpers', () => {
           aud: 'test1',
           [EH_ATTR_PRIMARY_ID_LEGACY]: 'EH-KVK1',
           sid: 'test4',
+          id: 'xx-bsn-xx',
         } as TokenData
       );
 
@@ -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
       );
 
@@ -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({
@@ -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({

src/server/auth/auth-helpers.ts

@@ -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';
@@ -89,6 +90,7 @@ export function getAuth(req: Request) {
   return {
     token: oidcToken,
     profile,
+    expiresAt: maSession.expires_at,
   };
 }
 
@@ -126,31 +128,39 @@ export function decodeToken<T extends Record<string, string>>(
   return jose.decodeJwt(jwtToken) as unknown as T;
 }
 
+function isIDPSessionExpired(expiresAtInSeconds: number) {
+  return expiresAtInSeconds < millisecondsToSeconds(Date.now());
+}
+
 export function createLogoutHandler(
   postLogoutRedirectUrl: string,
   doIDPLogout: boolean = true
 ) {
   return async (req: AuthenticatedRequest, res: Response) => {
-    if (req.oidc.isAuthenticated() && doIDPLogout) {
-      const auth = getAuth(req);
-      if (auth) {
-        // 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) {
-          await addToBlackList(auth.profile.sid);
-        }
-
-        return res.oidc.logout({
-          returnTo: postLogoutRedirectUrl,
-          logoutParams: {
-            id_token_hint: !FeatureToggle.oidcLogoutHintActive
-              ? auth.token
-              : null,
-            logout_hint: FeatureToggle.oidcLogoutHintActive
-              ? req[OIDC_SESSION_COOKIE_NAME]?.TMASessionID
-              : null,
-          },
-        });
+    const auth = getAuth(req);
+    if (
+      doIDPLogout &&
+      auth &&
+      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) {
+        await addToBlackList(auth.profile.sid);
       }
+
+      return res.oidc.logout({
+        returnTo: postLogoutRedirectUrl,
+        logoutParams: {
+          id_token_hint: !FeatureToggle.oidcLogoutHintActive
+            ? auth.token
+            : null,
+          logout_hint: FeatureToggle.oidcLogoutHintActive
+            ? req[OIDC_SESSION_COOKIE_NAME]?.TMASessionID
+            : null,
+        },
+      });
     }
 
     if (hasSessionCookie(req)) {

src/server/auth/auth-session-store.ts

@@ -10,7 +10,6 @@ import { pool } from '../services/db/postgres';
 
 type SessionStoreOptions = {
   tableName: string;
-  ttlSeconds?: number;
 };
 
 export function getSessionStore<T extends typeof expressSession>(
@@ -25,7 +24,6 @@ export function getSessionStore<T extends typeof expressSession>(
       tableName: options.tableName,
       pool,
       createTableIfMissing: true,
-      ttl: options.ttlSeconds,
     }) as unknown as SessionStore<Session>;
   }
 

src/server/auth/auth-types.ts

@@ -10,11 +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: number;
 }
 
 export interface AuthProfileAndToken {

src/server/helpers/file-cache.test.ts

@@ -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],

src/server/routing/route-handlers.ts

@@ -58,11 +58,13 @@ export function verifyAuthenticated(
 ) {
   return async (req: Request, res: Response) => {
     if (await isRequestAuthenticated(req, authMethod)) {
+      const auth = getAuth(req);
       return res.send(
         apiSuccessResult({
           isAuthenticated: true,
           profileType,
           authMethod,
+          expiresAt: auth?.expiresAt,
         })
       );
     }

src/server/routing/router-oidc.test.ts

@@ -47,6 +47,7 @@ describe('router-oids', () => {
           authMethod: 'digid',
           isAuthenticated: true,
           profileType: 'private',
+          expiresAt: expect.any(Number),
         },
         status: 'OK',
       });
@@ -64,6 +65,7 @@ describe('router-oids', () => {
           authMethod: 'eherkenning',
           isAuthenticated: true,
           profileType: 'commercial',
+          expiresAt: expect.any(Number),
         },
         status: 'OK',
       });

src/server/services/buurt/buurt.test.ts

@@ -7,6 +7,7 @@ import {
   DEFAULT_TRIES_UNTIL_CONSIDERED_STALE,
   DatasetConfig,
   DatasetFeatureProperties,
+  DatasetFeatures,
 } from './datasets';
 import {
   createDynamicFilterConfig,
@@ -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 } },
@@ -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,
@@ -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,
@@ -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,
@@ -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([