feat: improve forbidden error handling (#1104)

* feat: do not throw 500 on forbidden * refactor: remove useless code --------- Co-authored-by: Nicolas Moreau <nicolas.moreau76@gmail.com>
ForestAdmin · Apr 11, 2024 · 6e5cf71 · 6e5cf71
1 parent fb76378
commit 6e5cf71
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 3 deletions.
diff --git a/packages/agent/src/routes/security/authentication.ts b/packages/agent/src/routes/security/authentication.ts
@@ -1,5 +1,5 @@
 import { ValidationError } from '@forestadmin/datasource-toolkit';
-import { AuthenticationError } from '@forestadmin/forestadmin-client';
+import { AuthenticationError, ForbiddenError } from '@forestadmin/forestadmin-client';
 import Router from '@koa/router';
 import jsonwebtoken from 'jsonwebtoken';
 import { Context, Next } from 'koa';
@@ -81,6 +81,16 @@ export default class Authentication extends BaseRoute {
     try {
       await next();
     } catch (e) {
+      if (e instanceof ForbiddenError) {
+        context.response.status = 403;
+        context.response.body = {
+          error: 403,
+          error_description: e.message,
+        };
+
+        return;
+      }
+
       if (e instanceof AuthenticationError) {
         context.response.status = 401;
         context.response.body = {

diff --git a/packages/agent/test/routes/security/authentication.test.ts b/packages/agent/test/routes/security/authentication.test.ts
@@ -1,4 +1,4 @@
-import { AuthenticationError } from '@forestadmin/forestadmin-client';
+import { AuthenticationError, ForbiddenError } from '@forestadmin/forestadmin-client';
 import { createMockContext } from '@shopify/jest-koa-mocks';
 import { errors } from 'openid-client';
 
@@ -186,6 +186,33 @@ describe('Authentication', () => {
           });
         });
 
+        test('returns a translated error if it is ForbiddenError', async () => {
+          route.setupRoutes(router);
+
+          const getMock = router.get as jest.Mock;
+
+          const errorHandler = getMock.mock.calls[0][1];
+          const callbackHandler = getMock.mock.calls[0][2];
+
+          const context = createMockContext({
+            customProperties: { query: { state: '{"renderingId": 1}' } },
+          });
+
+          jest
+            .spyOn(options.forestAdminClient.authService, 'generateTokens')
+            .mockRejectedValue(new ForbiddenError('access denied'));
+
+          await errorHandler(context, async () => {
+            await callbackHandler(context);
+          });
+
+          expect(context.response.status).toEqual(403);
+          expect(context.response.body).toEqual({
+            error: 403,
+            error_description: 'access denied',
+          });
+        });
+
         test('rethrow the error if it is not an AuthenticationError', async () => {
           route.setupRoutes(router);
 

diff --git a/packages/forestadmin-client/src/auth/errors.ts b/packages/forestadmin-client/src/auth/errors.ts
@@ -1,6 +1,6 @@
+/* eslint-disable max-classes-per-file */
 import type { errors } from 'openid-client';
 
-// eslint-disable-next-line import/prefer-default-export
 export class AuthenticationError extends Error {
   public readonly description: string;
   public readonly state: string;
@@ -15,3 +15,5 @@ export class AuthenticationError extends Error {
     this.stack = e.stack;
   }
 }
+
+export class ForbiddenError extends Error {}
diff --git a/packages/forestadmin-client/src/utils/server.ts b/packages/forestadmin-client/src/utils/server.ts
@@ -1,5 +1,6 @@
 import superagent, { ResponseError } from 'superagent';
 
+import { ForbiddenError } from '..';
 import { ForestAdminClientOptionsWithDefaults } from '../types';
 
 type HttpOptions = Pick<ForestAdminClientOptionsWithDefaults, 'envSecret' | 'forestServerUrl'>;
@@ -52,6 +53,10 @@ export default class ServerUtils {
         throw new Error('Failed to reach Forest Admin server. Are you online?');
       }
 
+      if (status === 403) {
+        throw new ForbiddenError(message);
+      }
+
       if (status === 404) {
         throw new Error(
           'Forest Admin server failed to find the' +

diff --git a/packages/forestadmin-client/test/utils/server.test.ts b/packages/forestadmin-client/test/utils/server.test.ts
@@ -1,5 +1,6 @@
 import nock from 'nock';
 
+import { ForbiddenError } from '../../src';
 import ServerUtils from '../../src/utils/server';
 
 const options = { envSecret: '123', forestServerUrl: 'http://forestadmin-server.com' };
@@ -34,6 +35,16 @@ describe('ServerUtils', () => {
     expect(result).toStrictEqual({ data: 'ok' });
   });
 
+  it('should throw a ForbiddenError if the server returns a 403', async () => {
+    nock(options.forestServerUrl)
+      .get('/endpoint')
+      .reply(403, { errors: [{ detail: 'project access forbidden' }] });
+
+    await expect(ServerUtils.query(options, 'get', '/endpoint')).rejects.toEqual(
+      new ForbiddenError('project access forbidden'),
+    );
+  });
+
   it('should fail if project does not exists', async () => {
     nock(options.forestServerUrl)
       .get('/endpoint')