feat(api): trap forwarded origin related errors with a dedicated Forw…

…ardedOriginError

and return HTTP status code 400 instead of 500 when those errors arise.

api/src/identity-access-management/infrastructure/utils/network.js

@@ -1,3 +1,5 @@
+import { HttpErrors } from '../../../shared/application/http-errors.js';
+
 const PIX_APP_APPLICATION_NAME = 'app';
 const PIX_ADMIN_APPLICATION_NAME = 'admin';
 const PIX_ORGA_APPLICATION_NAME = 'orga';
@@ -22,7 +24,7 @@ function getForwardedOrigin(headers) {
   const protoHeader = headers['x-forwarded-proto'];
   const hostHeader = headers['x-forwarded-host'];
   if (!protoHeader || !hostHeader) {
-    return '';
+    throw new ForwardedOriginError('Missing forwarded header(s)');
   }
 
   return `${_getHeaderFirstValue(protoHeader)}://${_getHeaderFirstValue(hostHeader)}`;
@@ -61,23 +63,47 @@ class RequestedApplication {
    * @returns {RequestedApplication}
    */
   static fromOrigin(origin) {
-    const url = new URL(origin);
+    let url;
+    try {
+      url = new URL(origin);
+    } catch {
+      throw new ForwardedOriginError(`Invalid URL: "${origin}"`);
+    }
+
     let applicationName;
 
     if (url.hostname == 'localhost') {
       applicationName = localhostApplicationPortMapping[url.port];
       return new RequestedApplication(applicationName);
     }
 
-    const urlFirstLabel = url.hostname.split('.')[0];
-    const reviewAppSubPart = urlFirstLabel.split('-')[0];
-    applicationName = reviewAppSubPart;
+    const hostnameParts = url.hostname.split('.');
+    if (hostnameParts.length < 2) {
+      throw new ForwardedOriginError(`Unsupported hostname: "${url.hostname}"`);
+    }
+
+    const urlFirstLabel = hostnameParts[0];
+
+    const urlFirstLabelParts = urlFirstLabel.split('-');
+    if (urlFirstLabelParts.length == 2) {
+      const reviewAppSubPart = urlFirstLabelParts[0];
+      applicationName = reviewAppSubPart;
+    } else {
+      applicationName = urlFirstLabel;
+    }
+
     return new RequestedApplication(applicationName);
   }
 }
 
+class ForwardedOriginError extends HttpErrors.BadRequestError {
+  constructor(message) {
+    super(message);
+  }
+}
+
 function _getHeaderFirstValue(headerValue) {
   return headerValue.split(',')[0];
 }
 
-export { getForwardedOrigin, RequestedApplication };
+export { ForwardedOriginError, getForwardedOrigin, RequestedApplication };

api/src/shared/application/healthcheck/healthcheck-controller.js

@@ -39,8 +39,12 @@ const checkRedisStatus = async function () {
 };
 
 const checkForwardedOriginStatus = async function (request, h) {
-  const forwardedOrigin = network.getForwardedOrigin(request.headers);
-  if (!forwardedOrigin) {
+  let forwardedOrigin;
+  try {
+    // network.getForwardedOrigin throws ForwardedOriginError which maps to a HTTP status code 400,
+    // but for monitoring purpose we want this error to produce a 500.
+    forwardedOrigin = network.getForwardedOrigin(request.headers);
+  } catch {
     return h.response('Obtaining Forwarded Origin failed').code(500);
   }
 

api/src/shared/infrastructure/plugins/pino.js

@@ -22,9 +22,14 @@ function requestSerializer(req) {
   const context = monitoringTools.getContext();
   if (context?.request?.route?.path === '/api/token') {
     const { username, refresh_token, grant_type } = context.request.payload || {};
-    const origin = getForwardedOrigin(context.request.headers);
+    let origin;
+    try {
+      origin = getForwardedOrigin(context.request.headers);
+    } catch {
+      origin = '-';
+    }
+    enhancedReq.audience = origin;
     enhancedReq.grantType = grant_type || '-';
-    enhancedReq.audience = origin || '-';
     enhancedReq.usernameHash = generateHash(username) || '-';
     enhancedReq.refreshTokenHash = generateHash(refresh_token) || '-';
   }

api/tests/identity-access-management/unit/infrastructure/utils/network.test.js

@@ -1,4 +1,5 @@
 import {
+  ForwardedOriginError,
   getForwardedOrigin,
   RequestedApplication,
 } from '../../../../../src/identity-access-management/infrastructure/utils/network.js';
@@ -75,15 +76,12 @@ describe('Unit | Identity Access Management | Infrastructure | Utils | network',
     });
 
     context('when x-forwarded-proto and x-forwarded-port are not defined', function () {
-      it('doesn’t choke and returns an empty string', async function () {
+      it('throws a ForwardedOriginError', function () {
         // given
         const headers = {};
 
-        // when
-        const origin = getForwardedOrigin(headers);
-
-        // then
-        expect(origin).to.equal('');
+        // when & then
+        expect(() => getForwardedOrigin(headers)).to.throw(ForwardedOriginError, 'Missing forwarded header(s)');
       });
     });
   });
@@ -235,6 +233,19 @@ describe('Unit | Identity Access Management | Infrastructure | Utils | network',
           });
         });
       });
+
+      context('when the origin is unsupported', function () {
+        it('throws a ForwardedOriginError', function () {
+          // given
+          const origin = 'https://someUnsupportedName';
+
+          // when & then
+          expect(() => RequestedApplication.fromOrigin(origin)).to.throw(
+            ForwardedOriginError,
+            'Unsupported hostname: "someunsupportedname"',
+          );
+        });
+      });
     });
   });
 });

api/tests/integration/infrastructure/plugins/pino_test.js

@@ -150,53 +150,81 @@ describe('Integration | Infrastructure | plugins | pino', function () {
       });
 
       context('when calling /api/token', function () {
-        it('should log the message, version, user id, route, metrics and hashed username', async function () {
-          // given
-          const messages = [];
-          await registerWithPlugin((data) => {
-            messages.push(data);
+        context('when there is a username', function () {
+          it('logs the message, version, user id, route, metrics and hashed username', async function () {
+            // given
+            const messages = [];
+            await registerWithPlugin((data) => {
+              messages.push(data);
+            });
+
+            const method = 'POST';
+            const url = '/api/token';
+            const payload = {
+              username: 'toto',
+            };
+            const headers = { 'x-forwarded-proto': 'https', 'x-forwarded-host': 'app.pix.org' };
+
+            // when
+            const response = await httpTestServer.request(method, url, payload, null, headers);
+
+            // then
+            expect(response.statusCode).to.equal(200);
+            expect(messages).to.have.lengthOf(1);
+            expect(messages[0].msg).to.equal('request completed');
+            expect(messages[0].req.version).to.equal('development');
+            expect(messages[0].req.user_id).to.equal('-');
+            expect(messages[0].req.route).to.equal('/api/token');
+            expect(messages[0].req.usernameHash).to.equal(
+              '31f7a65e315586ac198bd798b6629ce4903d0899476d5741a9f32e2e521b6a66', // echo -n 'toto'| shasum -a 256
+            );
           });
+        });
 
-          const method = 'POST';
-          const url = '/api/token';
-          const payload = {
-            username: 'toto',
-          };
-
-          // when
-          const response = await httpTestServer.request(method, url, payload);
-          // then
-          expect(response.statusCode).to.equal(200);
-          expect(messages).to.have.lengthOf(1);
-          expect(messages[0].msg).to.equal('request completed');
-          expect(messages[0].req.version).to.equal('development');
-          expect(messages[0].req.user_id).to.equal('-');
-          expect(messages[0].req.route).to.equal('/api/token');
-          expect(messages[0].req.usernameHash).to.equal(
-            '31f7a65e315586ac198bd798b6629ce4903d0899476d5741a9f32e2e521b6a66', // echo -n 'toto'| shasum -a 256
-          );
+        context('when there is no username', function () {
+          it('logs the message, version, user id, route, metrics and default value for username', async function () {
+            // given
+            const messages = [];
+            await registerWithPlugin((data) => {
+              messages.push(data);
+            });
+            const method = 'POST';
+            const url = '/api/token';
+            const headers = { 'x-forwarded-proto': 'https', 'x-forwarded-host': 'app.pix.org' };
+
+            // when
+            const response = await httpTestServer.request(method, url, null, null, headers);
+
+            // then
+            expect(response.statusCode).to.equal(200);
+            expect(messages).to.have.lengthOf(1);
+            expect(messages[0].msg).to.equal('request completed');
+            expect(messages[0].req.version).to.equal('development');
+            expect(messages[0].req.user_id).to.equal('-');
+            expect(messages[0].req.route).to.equal('/api/token');
+            expect(messages[0].req.usernameHash).to.equal('-');
+          });
         });
 
-        it('should log the message, version, user id, route, metrics and default value for username when not specified', async function () {
-          // given
-          const messages = [];
-          await registerWithPlugin((data) => {
-            messages.push(data);
+        context('when there is no forwarded origin (no x-forwarded headers)', function () {
+          it('handles the ForwardedOriginError error', async function () {
+            // given
+            const messages = [];
+            await registerWithPlugin((data) => {
+              messages.push(data);
+            });
+            const method = 'POST';
+            const url = '/api/token';
+            const noForwardedHeaders = {};
+
+            // when
+            const response = await httpTestServer.request(method, url, null, null, noForwardedHeaders);
+
+            // then
+            expect(response.statusCode).to.equal(200);
+            expect(messages).to.have.lengthOf(1);
+            expect(messages[0].msg).to.equal('request completed');
           });
-          const method = 'POST';
-          const url = '/api/token';
-
-          // when
-          const response = await httpTestServer.request(method, url);
-
-          // then
-          expect(response.statusCode).to.equal(200);
-          expect(messages).to.have.lengthOf(1);
-          expect(messages[0].msg).to.equal('request completed');
-          expect(messages[0].req.version).to.equal('development');
-          expect(messages[0].req.user_id).to.equal('-');
-          expect(messages[0].req.route).to.equal('/api/token');
-          expect(messages[0].req.usernameHash).to.equal('-');
         });
       });
     });

api/tests/unit/infrastructure/authentication_test.js

@@ -1,6 +1,7 @@
 import { authentication, validateClientApplication, validateUser } from '../../../lib/infrastructure/authentication.js';
 import { RevokedUserAccess } from '../../../src/identity-access-management/domain/models/RevokedUserAccess.js';
 import { revokedUserAccessRepository } from '../../../src/identity-access-management/infrastructure/repositories/revoked-user-access.repository.js';
+import { ForwardedOriginError } from '../../../src/identity-access-management/infrastructure/utils/network.js';
 import { config } from '../../../src/shared/config.js';
 import { tokenService } from '../../../src/shared/domain/services/token-service.js';
 import { expect, sinon } from '../../test-helper.js';
@@ -221,7 +222,7 @@ describe('Unit | Infrastructure | Authentication', function () {
               aud: 'https://app.pix.fr',
             });
 
-            // when
+            // when & then
             const { authenticate } = authentication.schemes.jwt.scheme(undefined, {
               key: 'dummy-secret',
               validate: (decodedAccessToken, options) =>
@@ -230,14 +231,7 @@ describe('Unit | Infrastructure | Authentication', function () {
                   revokedUserAccessRepository,
                 }),
             });
-            const response = await authenticate(request, h);
-
-            // then
-            expect(response.output.payload).to.include({
-              statusCode: 401,
-              error: 'Unauthorized',
-              message: 'Unauthorized',
-            });
+            await expect(authenticate(request, h)).to.be.rejectedWith(ForwardedOriginError);
           });
         });