auth-backend: use more standardized error responses

Co-authored-by: Johan Haals <johan.haals@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

.changeset/forty-teachers-argue.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-auth-backend': patch
+---
+
+Switched to using the standardized JSON error responses for all provider endpoints.

plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts

@@ -176,7 +176,7 @@ describe('OAuthAdapter', () => {
 
     const mockResponse = {
       cookie: jest.fn().mockReturnThis(),
-      send: jest.fn().mockReturnThis(),
+      end: jest.fn().mockReturnThis(),
       status: jest.fn().mockReturnThis(),
     } as unknown as express.Response;
 
@@ -187,6 +187,7 @@ describe('OAuthAdapter', () => {
       '',
       expect.objectContaining({ path: '/auth/test-provider' }),
     );
+    expect(mockResponse.end).toHaveBeenCalledTimes(1);
   });
 
   it('gets new access-token when refreshing', async () => {
@@ -230,21 +231,14 @@ describe('OAuthAdapter', () => {
 
     const mockRequest = {
       header: () => 'XMLHttpRequest',
-      cookies: {
-        'test-provider-refresh-token': 'token',
-      },
-      query: {},
     } as unknown as express.Request;
 
-    const mockResponse = {
-      send: jest.fn().mockReturnThis(),
-      status: jest.fn().mockReturnThis(),
-    } as unknown as express.Response;
+    const mockResponse = {} as unknown as express.Response;
 
-    await oauthProvider.refresh(mockRequest, mockResponse);
-    expect(mockResponse.send).toHaveBeenCalledTimes(1);
-    expect(mockResponse.send).toHaveBeenCalledWith(
-      'Refresh token not supported for provider: test-provider',
+    await expect(
+      oauthProvider.refresh(mockRequest, mockResponse),
+    ).rejects.toThrow(
+      'Refresh token is not supported for provider test-provider',
     );
   });
 });

plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts

@@ -22,7 +22,12 @@ import {
   BackstageIdentity,
   AuthProviderConfig,
 } from '../../providers/types';
-import { InputError, isError, NotAllowedError } from '@backstage/errors';
+import {
+  AuthenticationError,
+  InputError,
+  isError,
+  NotAllowedError,
+} from '@backstage/errors';
 import { TokenIssuer } from '../../identity/types';
 import { readState, verifyNonce } from './helpers';
 import { postMessageResponse, ensuresXRequestedWith } from '../flow';
@@ -166,29 +171,24 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
 
   async logout(req: express.Request, res: express.Response): Promise<void> {
     if (!ensuresXRequestedWith(req)) {
-      res.status(401).send('Invalid X-Requested-With header');
-      return;
+      throw new AuthenticationError('Invalid X-Requested-With header');
     }
 
     // remove refresh token cookie if it is set
     this.removeRefreshTokenCookie(res);
 
-    res.status(200).send('logout!');
+    res.status(200).end();
   }
 
   async refresh(req: express.Request, res: express.Response): Promise<void> {
     if (!ensuresXRequestedWith(req)) {
-      res.status(401).send('Invalid X-Requested-With header');
-      return;
+      throw new AuthenticationError('Invalid X-Requested-With header');
     }
 
     if (!this.handlers.refresh || this.options.disableRefresh) {
-      res
-        .status(400)
-        .send(
-          `Refresh token not supported for provider: ${this.options.providerId}`,
-        );
-      return;
+      throw new InputError(
+        `Refresh token is not supported for provider ${this.options.providerId}`,
+      );
     }
 
     try {
@@ -197,7 +197,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
 
       // throw error if refresh token is missing in the request
       if (!refreshToken) {
-        throw new Error('Missing session cookie');
+        throw new InputError('Missing session cookie');
       }
 
       const scope = req.query.scope?.toString() ?? '';
@@ -220,7 +220,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
 
       res.status(200).json(response);
     } catch (error) {
-      res.status(401).send(String(error));
+      throw new AuthenticationError('Refresh failed', error);
     }
   }
 

plugins/auth-backend/src/lib/oauth/OAuthEnvironmentHandler.ts

@@ -16,7 +16,7 @@
 
 import express from 'express';
 import { Config } from '@backstage/config';
-import { InputError } from '@backstage/errors';
+import { InputError, NotFoundError } from '@backstage/errors';
 import { readState } from './helpers';
 import { AuthProviderRouteHandlers } from '../../providers/types';
 
@@ -42,26 +42,26 @@ export class OAuthEnvironmentHandler implements AuthProviderRouteHandlers {
   ) {}
 
   async start(req: express.Request, res: express.Response): Promise<void> {
-    const provider = this.getProviderForEnv(req, res);
-    await provider?.start(req, res);
+    const provider = this.getProviderForEnv(req);
+    await provider.start(req, res);
   }
 
   async frameHandler(
     req: express.Request,
     res: express.Response,
   ): Promise<void> {
-    const provider = this.getProviderForEnv(req, res);
-    await provider?.frameHandler(req, res);
+    const provider = this.getProviderForEnv(req);
+    await provider.frameHandler(req, res);
   }
 
   async refresh(req: express.Request, res: express.Response): Promise<void> {
-    const provider = this.getProviderForEnv(req, res);
-    await provider?.refresh?.(req, res);
+    const provider = this.getProviderForEnv(req);
+    await provider.refresh?.(req, res);
   }
 
   async logout(req: express.Request, res: express.Response): Promise<void> {
-    const provider = this.getProviderForEnv(req, res);
-    await provider?.logout?.(req, res);
+    const provider = this.getProviderForEnv(req);
+    await provider.logout?.(req, res);
   }
 
   private getRequestFromEnv(req: express.Request): string | undefined {
@@ -77,26 +77,20 @@ export class OAuthEnvironmentHandler implements AuthProviderRouteHandlers {
     return env;
   }
 
-  private getProviderForEnv(
-    req: express.Request,
-    res: express.Response,
-  ): AuthProviderRouteHandlers | undefined {
+  private getProviderForEnv(req: express.Request): AuthProviderRouteHandlers {
     const env: string | undefined = this.getRequestFromEnv(req);
 
     if (!env) {
       throw new InputError(`Must specify 'env' query to select environment`);
     }
 
-    if (!this.handlers.has(env)) {
-      res.status(404).send(
-        `Missing configuration.
-    <br>
-    <br>
-    For this flow to work you need to supply a valid configuration for the "${env}" environment of provider.`,
+    const handler = this.handlers.get(env);
+    if (!handler) {
+      throw new NotFoundError(
+        `No configuration available for the '${env}' environment of this provider.`,
       );
-      return undefined;
     }
 
-    return this.handlers.get(env);
+    return handler;
   }
 }