From ab01673c01bd3056625a534f266171eebf37cc18 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Fri, 23 Feb 2024 13:18:41 +0100
Subject: [PATCH] backend-app-api: add tests for createCretentialsBarrier + fix

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
---
 .../createCredentialsBarrier.test.ts          | 122 ++++++++++++++++++
 .../httpRouter/createCredentialsBarrier.ts    |  17 ++-
 2 files changed, 132 insertions(+), 7 deletions(-)
 create mode 100644 packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts

diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts
new file mode 100644
index 0000000000..b8430d398e
--- /dev/null
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts
@@ -0,0 +1,122 @@
+/*
+ * Copyright 2024 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* eslint-disable jest/expect-expect */
+
+import express from 'express';
+import request from 'supertest';
+import { createCredentialsBarrier } from './createCredentialsBarrier';
+import { mockCredentials, mockServices } from '@backstage/backend-test-utils';
+import { MiddlewareFactory } from '../../../http';
+
+const errorMiddleware = MiddlewareFactory.create({
+  config: mockServices.rootConfig(),
+  logger: mockServices.rootLogger(),
+}).error();
+
+function setup() {
+  const barrier = createCredentialsBarrier({
+    httpAuth: mockServices.httpAuth({
+      defaultCredentials: mockCredentials.none(),
+    }),
+    config: mockServices.rootConfig(),
+  });
+
+  const app = express();
+  app.use(barrier.middleware);
+  app.use(errorMiddleware);
+  app.get('*', (_req, res) => res.status(200).end());
+
+  return { app, barrier };
+}
+
+describe('createCredentialsBarrier', () => {
+  it('should enforce default auth policy', async () => {
+    const { app } = setup();
+
+    await request(app)
+      .get('/')
+      .send()
+      .expect(401)
+      .expect(res =>
+        expect(res.body).toMatchObject({
+          error: { name: 'AuthenticationError', message: '' },
+        }),
+      );
+
+    await request(app)
+      .get('/')
+      .set('authorization', mockCredentials.user.invalidHeader())
+      .send()
+      .expect(401)
+      .expect(res =>
+        expect(res.body).toMatchObject({
+          error: {
+            name: 'AuthenticationError',
+            message: 'User token is invalid',
+          },
+        }),
+      );
+
+    await request(app)
+      .get('/')
+      .set('authorization', mockCredentials.service.invalidHeader())
+      .send()
+      .expect(401)
+      .expect(res =>
+        expect(res.body).toMatchObject({
+          error: {
+            name: 'AuthenticationError',
+            message: 'Service token is invalid',
+          },
+        }),
+      );
+
+    await request(app)
+      .get('/')
+      .set('authorization', mockCredentials.user.header())
+      .send()
+      .expect(200);
+
+    await request(app)
+      .get('/')
+      .set('authorization', mockCredentials.service.header())
+      .send()
+      .expect(200);
+  });
+
+  it('should allow exceptions to the default auth policy to be made', async () => {
+    const { app, barrier } = setup();
+
+    await request(app).get('/').send().expect(401);
+    await request(app).get('/public').send().expect(401);
+    await request(app).get('/other').send().expect(401);
+
+    barrier.addAuthPolicy({ allow: 'unauthenticated', path: '/public' });
+
+    await request(app).get('/').send().expect(401);
+    await request(app).get('/public').send().expect(200);
+    await request(app).get('/other').send().expect(401);
+
+    barrier.addAuthPolicy({ allow: 'unauthenticated', path: '/' });
+
+    await request(app).get('/').send().expect(200);
+    await request(app).get('/public').send().expect(200);
+    await request(app).get('/other').send().expect(200);
+  });
+
+  // TODO: cookie auth
+});
diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
index 801c87d430..a512c5ac9e 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
@@ -59,7 +59,7 @@ export function createCredentialsBarrier(options: {
   const unauthenticatedPredicates = new Array<(path: string) => boolean>();
   const cookiePredicates = new Array<(path: string) => boolean>();
 
-  const middleware: RequestHandler = async (req, _, next) => {
+  const middleware: RequestHandler = (req, _, next) => {
     const allowsUnauthenticated = unauthenticatedPredicates.some(predicate =>
       predicate(req.path),
     );
@@ -73,12 +73,15 @@ export function createCredentialsBarrier(options: {
       predicate(req.path),
     );
 
-    await httpAuth.credentials(req, {
-      allow: ['user', 'service'],
-      allowedAuthMethods: allowsCookie ? ['token', 'cookie'] : ['token'],
-    });
-
-    next();
+    httpAuth
+      .credentials(req, {
+        allow: ['user', 'service'],
+        allowedAuthMethods: allowsCookie ? ['token', 'cookie'] : ['token'],
+      })
+      .then(
+        () => next(),
+        err => next(err),
+      );
   };
 
   const addAuthPolicy = (policy: HttpRouterServiceAuthPolicy) => {