Mijn-8373 added endpoint access security (#1262)

* MIJN-8373 - Add session id check * Improve error reporting and session id check * Add sid prop to tests * Remove unused comment + null value
Amsterdam · Apr 25, 2024 · cd1ad0b · cd1ad0b
1 parent d846a6c
commit cd1ad0b
Show file tree

Hide file tree

Showing 27 changed files with 289 additions and 85 deletions.
diff --git a/src/server/helpers/app.ts b/src/server/helpers/app.ts
@@ -33,8 +33,8 @@ import { captureException, captureMessage } from '../services/monitoring';
 export interface AuthProfile {
   authMethod: 'eherkenning' | 'digid' | 'yivi';
   profileType: ProfileType;
-  id?: string;
-  sid?: string; // TMA Session ID
+  id: string; // User id (bsn/kvknr)
+  sid: string; // TMA Session ID
 }
 
 export function getAuthProfile(tokenData: TokenData): AuthProfile {

diff --git a/src/server/helpers/source-api-request.test.ts b/src/server/helpers/source-api-request.test.ts
@@ -37,7 +37,12 @@ describe('requestData.ts', () => {
   const SESS_ID_2 = 'y2';
   const TOKEN = 'xxxxx';
   const AUTH_PROFILE_AND_TOKEN: AuthProfileAndToken = {
-    profile: { authMethod: 'digid', profileType: 'private', id: 'bsnxxxx' },
+    profile: {
+      authMethod: 'digid',
+      profileType: 'private',
+      id: 'bsnxxxx',
+      sid: '',
+    },
     token: TOKEN,
   };
 
@@ -175,12 +180,12 @@ describe('requestData.ts', () => {
 
     const error = new Error('Network Error');
 
-    expect(rs).toStrictEqual(apiErrorResult(error.toString(), null, null));
+    expect(rs).toStrictEqual(apiErrorResult(error.toString(), null));
   });
 
   test('Find corresponding api', () => {
     const entries: ApiUrlEntries = [
-      ['WMO', 'http://get/foo'],
+      ['WPI_AANVRAGEN', 'http://get/foo'],
       [
         'BRP',
         { private: 'http://get/bar', commercial: 'https://get/bar/commercial' },
@@ -194,7 +199,9 @@ describe('requestData.ts', () => {
       ],
     ];
 
-    expect(findApiByRequestUrl(entries, 'http://get/foo')).toBe('WMO');
+    expect(findApiByRequestUrl(entries, 'http://get/foo')).toBe(
+      'WPI_AANVRAGEN'
+    );
     expect(findApiByRequestUrl(entries, 'http://get/bar')).toBe('BRP');
     expect(findApiByRequestUrl(entries, 'http://get/bar/commercial')).toBe(
       'BRP'

diff --git a/src/server/router-protected.ts b/src/server/router-protected.ts
@@ -232,7 +232,7 @@ router.get(
 
 router.get(
   BffEndpoints.BEZWAREN_DOCUMENT_DOWNLOAD,
-  async (req: Request, res: Response) => {
+  async (req: Request<{ id: string }>, res: Response) => {
     const authProfileAndToken = await getAuth(req);
 
     const documentResponse = await fetchBezwaarDocument(
@@ -241,6 +241,16 @@ router.get(
       req.params.id
     );
 
+    if (documentResponse.status === 'ERROR') {
+      return res
+        .status(
+          typeof documentResponse.code === 'number'
+            ? documentResponse.code
+            : 500
+        )
+        .end();
+    }
+
     const contentType = documentResponse.headers['content-type'];
     res.setHeader('content-type', contentType);
     documentResponse.data.pipe(res);
@@ -272,7 +282,9 @@ router.get(
     );
 
     if (response.status === 'ERROR') {
-      res.status(500);
+      return res
+        .status(typeof response.code === 'number' ? response.code : 500)
+        .end();
     }
 
     return res.send(response);
@@ -281,7 +293,7 @@ router.get(
 
 router.get(
   BffEndpoints.STADSPAS_TRANSACTIONS,
-  async (req: Request, res: Response) => {
+  async (req: Request<{ transactionsKey: string }>, res: Response) => {
     const authProfileAndToken = await getAuth(req);
     const response = await fetchTransacties(
       res.locals.requestID,
@@ -290,7 +302,9 @@ router.get(
     );
 
     if (response.status === 'ERROR') {
-      res.status(500);
+      return res
+        .status(typeof response.code === 'number' ? response.code : 500)
+        .end();
     }
 
     return res.send(response);

diff --git a/src/server/services/avg/avg.test.ts b/src/server/services/avg/avg.test.ts
@@ -13,6 +13,7 @@ describe('AVG', () => {
       id: '123',
       authMethod: 'digid',
       profileType: 'private',
+      sid: '',
     },
     token: 'abc123',
   };

diff --git a/src/server/services/bag.test.ts b/src/server/services/bag.test.ts
@@ -23,7 +23,12 @@ describe('BAG service', () => {
       'x',
       {
         token: 'xxxx',
-        profile: { authMethod: 'digid', profileType: 'private' },
+        profile: {
+          authMethod: 'digid',
+          profileType: 'private',
+          sid: '',
+          id: '',
+        },
       },
       address
     );
@@ -38,7 +43,7 @@ describe('BAG service', () => {
     });
   });
 
-  it('Bag api should fail correct;y', async () => {
+  it('Bag api should fail correctly', async () => {
     nock('http://api.data.amsterdam.nl')
       .get('/bag', { params: { q: 'undefined' } })
       .reply(500);
@@ -47,7 +52,12 @@ describe('BAG service', () => {
       'x',
       {
         token: 'xxxx',
-        profile: { authMethod: 'digid', profileType: 'private' },
+        profile: {
+          authMethod: 'digid',
+          profileType: 'private',
+          id: '',
+          sid: '',
+        },
       },
       {} as any
     );

diff --git a/src/server/services/bezwaren/bezwaren.test.ts b/src/server/services/bezwaren/bezwaren.test.ts
@@ -18,9 +18,9 @@ vi.mock('../../../universal/helpers/encrypt-decrypt', async (requireActual) => {
   return {
     ...((await requireActual()) as object),
     encrypt: () => {
-      return ['test-encrypted-id'];
+      return ['session-id', 'test-encrypted-id'];
     },
-    decrypt: () => 'e6ed38c3-a44a-4c16-97c1-89d7ebfca095',
+    decrypt: () => 'session-id:e6ed38c3-a44a-4c16-97c1-89d7ebfca095',
   };
 });
 
@@ -34,6 +34,7 @@ describe('Bezwaren', () => {
       id: '123',
       authMethod: 'digid',
       profileType: 'private',
+      sid: 'session-id',
     },
     token: 'abc123',
   };

diff --git a/src/server/services/bezwaren/bezwaren.ts b/src/server/services/bezwaren/bezwaren.ts
@@ -1,18 +1,22 @@
 import axios from 'axios';
 import * as jose from 'jose';
+import memoizee from 'memoizee';
 import { generatePath } from 'react-router-dom';
 import { AppRoutes, Chapters } from '../../../universal/config';
 import {
   apiDependencyError,
+  apiErrorResult,
   apiSuccessResult,
   dateSort,
-  defaultDateFormat,
+  getFailedDependencies,
+  getSettledResult,
 } from '../../../universal/helpers';
 import { decrypt, encrypt } from '../../../universal/helpers/encrypt-decrypt';
 import { MyNotification } from '../../../universal/types';
 import { BffEndpoints, getApiConfig } from '../../config';
 import { requestData } from '../../helpers';
 import { AuthProfileAndToken } from '../../helpers/app';
+import { captureException } from '../monitoring';
 import {
   Bezwaar,
   BezwaarDocument,
@@ -25,8 +29,6 @@ import {
   Kenmerk,
   kenmerkKey,
 } from './types';
-import memoizee from 'memoizee';
-import { captureException } from '../monitoring';
 
 const MAX_PAGE_COUNT = 5; // Should amount to 5 * 20 (per page) = 100 bezwaren
 
@@ -87,28 +89,24 @@ function transformBezwarenDocumentsResults(
   response: BezwarenSourceResponse<BezwaarSourceDocument>
 ): BezwaarDocument[] {
   if (Array.isArray(response.results)) {
-    try {
-      return response.results.map(
-        ({ bestandsnaam, identificatie, dossiertype, verzenddatum }) => {
-          const [documentIdEncrypted] = encrypt(
-            identificatie,
-            process.env.BFF_GENERAL_ENCRYPTION_KEY ?? ''
-          );
-          return {
-            id: documentIdEncrypted,
-            title: bestandsnaam,
-            datePublished: defaultDateFormat(verzenddatum),
-            url: `${process.env.BFF_OIDC_BASE_URL}/api/v1${generatePath(
-              BffEndpoints.BEZWAREN_DOCUMENT_DOWNLOAD,
-              { id: documentIdEncrypted }
-            )}`,
-            dossiertype,
-          };
-        }
-      );
-    } catch (error) {
-      captureException(error);
-    }
+    return response.results.map(
+      ({ bestandsnaam, identificatie, dossiertype, verzenddatum }) => {
+        const [documentIdEncrypted] = encrypt(
+          identificatie,
+          process.env.BFF_GENERAL_ENCRYPTION_KEY ?? ''
+        );
+        return {
+          id: documentIdEncrypted,
+          title: bestandsnaam,
+          datePublished: verzenddatum,
+          url: `${process.env.BFF_OIDC_BASE_URL}/api/v1${generatePath(
+            BffEndpoints.BEZWAREN_DOCUMENT_DOWNLOAD,
+            { id: documentIdEncrypted }
+          )}`,
+          dossiertype,
+        };
+      }
+    );
   }
   return [];
 }
@@ -359,23 +357,40 @@ export type BezwaarDetail = {
 export async function fetchBezwaarDetail(
   requestID: requestID,
   authProfileAndToken: AuthProfileAndToken,
-  zaakId: string
+  zaakIdEncrypted: string
 ) {
+  const [sessionID, zaakId] = decrypt(zaakIdEncrypted).split(':');
+
+  if (sessionID !== authProfileAndToken.profile.sid) {
+    return apiErrorResult('Not authorized', null, 401);
+  }
+
   const bezwaarStatusRequest = fetchBezwaarStatus(zaakId, authProfileAndToken);
   const bezwaarDocumentsRequest = fetchBezwarenDocuments(
     zaakId,
     authProfileAndToken
   );
 
-  const [bezwaarStatusResponse, bezwaarDocumentsResponse] = await Promise.all([
+  const [statussenResponse, documentsResponse] = await Promise.allSettled([
     bezwaarStatusRequest,
     bezwaarDocumentsRequest,
   ]);
 
-  return apiSuccessResult({
-    statussen: bezwaarStatusResponse.content,
-    documents: bezwaarDocumentsResponse.content,
+  const statussen = getSettledResult(statussenResponse);
+  const documents = getSettledResult(documentsResponse);
+
+  const failedDependencies = getFailedDependencies({
+    statussen,
+    documents,
   });
+
+  return apiSuccessResult(
+    {
+      statussen: statussen.content,
+      documents: documents.content,
+    },
+    failedDependencies
+  );
 }
 
 export async function fetchBezwaarDocument(
@@ -384,10 +399,14 @@ export async function fetchBezwaarDocument(
   documentIdEncrypted: string,
   isDownload: boolean = true
 ) {
-  const documentId = decrypt(
+  const [sessionID, documentId] = decrypt(
     documentIdEncrypted,
     process.env.BFF_GENERAL_ENCRYPTION_KEY ?? ''
-  );
+  ).split(':');
+
+  if (sessionID !== authProfileAndToken.profile.sid) {
+    return apiErrorResult('Not authorized', null, 401);
+  }
 
   const url = generatePath(
     `${process.env.BFF_BEZWAREN_API}/zgw/v1/enkelvoudiginformatieobjecten/:id${

diff --git a/src/server/services/bodem/loodmeting.test.ts b/src/server/services/bodem/loodmeting.test.ts
@@ -18,6 +18,7 @@ describe('Loodmeting', () => {
       id: '123',
       authMethod: 'digid',
       profileType: 'private',
+      sid: '',
     },
     token: 'abc123',
   };

diff --git a/src/server/services/controller.test.ts b/src/server/services/controller.test.ts
@@ -97,6 +97,7 @@ describe('controller', () => {
         id: '123456789',
         profileType: 'private',
         authMethod: 'digid',
+        sid: '',
       },
       token: 'xxx==',
     });
@@ -124,6 +125,7 @@ describe('controller', () => {
         id: '90006178',
         profileType: 'commercial',
         authMethod: 'eherkenning',
+        sid: '',
       },
       token: 'xxx==',
     });
@@ -163,6 +165,7 @@ describe('controller', () => {
         id: '123456789',
         profileType: 'private',
         authMethod: 'digid',
+        sid: '',
       },
       token: 'xxx==',
     });
@@ -178,6 +181,7 @@ describe('controller', () => {
         id: '90006178',
         profileType: 'commercial',
         authMethod: 'eherkenning',
+        sid: '',
       },
       token: 'xxx==',
     });

diff --git a/src/server/services/horeca.test.ts b/src/server/services/horeca.test.ts
@@ -8,7 +8,7 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
 
 describe('Horeca service', () => {
   const authProfileAndToken: AuthProfileAndToken = {
-    profile: { authMethod: 'digid', profileType: 'private' },
+    profile: { authMethod: 'digid', profileType: 'private', id: '', sid: '' },
     token: 'xxxxxx',
   };
 

diff --git a/src/server/services/klachten/klachten.test.ts b/src/server/services/klachten/klachten.test.ts
@@ -17,6 +17,7 @@ describe('Klachten', () => {
       id: '123',
       authMethod: 'digid',
       profileType: 'private',
+      sid: '',
     },
     token: 'abc123',
   };

diff --git a/src/server/services/krefia.test.ts b/src/server/services/krefia.test.ts
@@ -9,7 +9,7 @@ import { fetchKrefia, fetchKrefiaNotifications, fetchSource } from './krefia';
 describe('Kredietbank & FIBU service', () => {
   const KREFIA_DUMMY_RESPONSE = jsonCopy(KrefiaData);
   const authProfileAndToken: AuthProfileAndToken = {
-    profile: { authMethod: 'digid', profileType: 'private' },
+    profile: { authMethod: 'digid', profileType: 'private', sid: '', id: '' },
     token: 'xxxxxx',
   };
 

diff --git a/src/server/services/sia.test.ts b/src/server/services/sia.test.ts
@@ -549,6 +549,7 @@ describe('sia service', () => {
           id: 'hm',
           authMethod: 'yivi',
           profileType: 'private-attributes',
+          sid: '',
         },
       },
       SIGNAL_ID_ENCRYPTED

diff --git a/src/server/services/simple-connect/belasting.test.ts b/src/server/services/simple-connect/belasting.test.ts
@@ -5,7 +5,7 @@ import { fetchBelasting, fetchBelastingNotifications } from './belasting';
 
 const REQUEST_ID = 'test-x-999';
 const authProfileAndToken: AuthProfileAndToken = {
-  profile: { authMethod: 'digid', profileType: 'private' },
+  profile: { authMethod: 'digid', profileType: 'private', id: '', sid: '' },
   token: 'xxxxxx',
 };