Merge pull request #13146 from backstage/blam/fix-authorization-in-scaffolder

Fix authorization header invalid for scaffolder create

.changeset/smooth-planes-itch.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-scaffolder-backend': patch
+---
+
+Fail gracefully if an invalid `Authorization` header is passed to `POST /v2/tasks`

plugins/scaffolder-backend/src/service/router.test.ts

@@ -267,6 +267,57 @@ describe('createRouter', () => {
       );
     });
 
+    it('should not throw when an invalid authorization header is passed', async () => {
+      const broker = taskBroker.dispatch as jest.Mocked<TaskBroker>['dispatch'];
+      const mockToken = 'blob.eyJzdWIiOiIiLCJuYW1lIjoiSm9obiBEb2UifQ.blob';
+
+      await request(app)
+        .post('/v2/tasks')
+        .set('Authorization', `Bearer ${mockToken}`)
+        .send({
+          templateRef: stringifyEntityRef({
+            kind: 'template',
+            name: 'create-react-app-template',
+          }),
+          values: {
+            required: 'required-value',
+          },
+        });
+      expect(broker).toHaveBeenCalledWith(
+        expect.objectContaining({
+          createdBy: undefined,
+          secrets: {
+            backstageToken: undefined,
+          },
+
+          spec: {
+            apiVersion: mockTemplate.apiVersion,
+            steps: mockTemplate.spec.steps.map((step, index) => ({
+              ...step,
+              id: step.id ?? `step-${index + 1}`,
+              name: step.name ?? step.action,
+            })),
+            output: mockTemplate.spec.output ?? {},
+            parameters: {
+              required: 'required-value',
+            },
+            user: {
+              entity: undefined,
+              ref: undefined,
+            },
+            templateInfo: {
+              entityRef: stringifyEntityRef({
+                kind: 'Template',
+                namespace: 'Default',
+                name: mockTemplate.metadata?.name,
+              }),
+              baseUrl: 'https://dev.azure.com',
+            },
+          },
+        }),
+      );
+    });
+
     it('should not decorate a user when no backstage auth is passed', async () => {
       const broker = taskBroker.dispatch as jest.Mocked<TaskBroker>['dispatch'];
 

plugins/scaffolder-backend/src/service/router.ts

@@ -146,7 +146,10 @@ export async function createRouter(
       '/v2/templates/:namespace/:kind/:name/parameter-schema',
       async (req, res) => {
         const { namespace, kind, name } = req.params;
-        const { token } = parseBearerToken(req.headers.authorization);
+        const { token } = parseBearerToken({
+          header: req.headers.authorization,
+          logger,
+        });
         const template = await findTemplate({
           catalogApi: catalogClient,
           entityRef: { kind, namespace, name },
@@ -187,9 +190,10 @@ export async function createRouter(
       const { kind, namespace, name } = parseEntityRef(templateRef, {
         defaultKind: 'template',
       });
-      const { token, entityRef: userEntityRef } = parseBearerToken(
-        req.headers.authorization,
-      );
+      const { token, entityRef: userEntityRef } = parseBearerToken({
+        header: req.headers.authorization,
+        logger,
+      });
 
       const userEntity = userEntityRef
         ? await catalogClient.getEntityByRef(userEntityRef, { token })
@@ -389,7 +393,10 @@ export async function createRouter(
         throw new InputError('Input template is not a template');
       }
 
-      const { token } = parseBearerToken(req.headers.authorization);
+      const { token } = parseBearerToken({
+        header: req.headers.authorization,
+        logger,
+      });
 
       for (const parameters of [template.spec.parameters ?? []].flat()) {
         const result = validate(body.values, parameters);
@@ -440,7 +447,13 @@ export async function createRouter(
   return app;
 }
 
-function parseBearerToken(header?: string): {
+function parseBearerToken({
+  header,
+  logger,
+}: {
+  header?: string;
+  logger: Logger;
+}): {
   token?: string;
   entityRef?: string;
 } {
@@ -472,8 +485,12 @@ function parseBearerToken(header?: string): {
       throw new TypeError('Expected string sub claim');
     }
 
+    // Check that it's a valid ref, otherwise this will throw.
+    parseEntityRef(sub);
+
     return { entityRef: sub, token };
   } catch (e) {
-    throw new InputError(`Invalid authorization header: ${stringifyError(e)}`);
+    logger.error(`Invalid authorization header: ${stringifyError(e)}`);
+    return {};
   }
 }