Merge pull request #9344 from backstage/rugvip/auth-cookies

auth-backend: make it possible to tweak the cookie configuration logic

.changeset/breezy-windows-jump.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-auth-backend': minor
+---
+
+The `callbackUrl` option of `OAuthAdapter` is now required.

.changeset/metal-lions-fix.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-auth-backend': patch
+---
+
+Added a new `cookieConfigurer` option to `AuthProviderConfig` that makes it possible to override the default logic for configuring OAuth provider cookies.

plugins/auth-backend/api-report.md

@@ -214,6 +214,17 @@ export class CatalogIdentityClient {
   resolveCatalogMembership(query: MemberClaimQuery): Promise<string[]>;
 }
 
+// @public
+export type CookieConfigurer = (ctx: {
+  providerId: string;
+  baseUrl: string;
+  callbackUrl: string;
+}) => {
+  domain: string;
+  path: string;
+  secure: boolean;
+};
+
 // Warning: (ae-missing-release-tag) "createAtlassianProvider" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
 //
 // @public (undocumented)
@@ -736,5 +747,5 @@ export type WebMessageResponse =
 // src/identity/types.d.ts:31:9 - (ae-forgotten-export) The symbol "AnyJWK" needs to be exported by the entry point index.d.ts
 // src/providers/aws-alb/provider.d.ts:77:5 - (ae-forgotten-export) The symbol "AwsAlbResult" needs to be exported by the entry point index.d.ts
 // src/providers/github/provider.d.ts:97:5 - (ae-forgotten-export) The symbol "StateEncoder" needs to be exported by the entry point index.d.ts
-// src/providers/types.d.ts:98:5 - (ae-forgotten-export) The symbol "AuthProviderConfig" needs to be exported by the entry point index.d.ts
+// src/providers/types.d.ts:118:5 - (ae-forgotten-export) The symbol "AuthProviderConfig" needs to be exported by the entry point index.d.ts
 ```

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

@@ -69,13 +69,14 @@ describe('OAuthAdapter', () => {
     secure: false,
     disableRefresh: true,
     appOrigin: 'http://localhost:3000',
-    cookieDomain: 'localhost',
+    cookieDomain: 'example.com',
     cookiePath: '/auth/test-provider',
     tokenIssuer: {
       issueToken: async () => 'my-id-token',
       listPublicKeys: async () => ({ keys: [] }),
     },
     isOriginAllowed: () => false,
+    callbackUrl: 'http://example.com:7007/auth/test-provider/frame/handler',
   };
 
   it('sets the correct headers in start', async () => {
@@ -444,47 +445,6 @@ describe('OAuthAdapter', () => {
     });
   });
 
-  it('sets the correct cookie configuration using the base url', async () => {
-    const config = {
-      baseUrl: 'http://domain.org/auth',
-      appUrl: 'http://domain.org',
-      isOriginAllowed: () => false,
-    };
-
-    const oauthProvider = OAuthAdapter.fromConfig(
-      config,
-      providerInstance,
-      oAuthProviderOptions,
-    );
-
-    const mockRequest = {
-      query: {
-        scope: 'user',
-        env: 'development',
-      },
-    } as unknown as express.Request;
-
-    const mockResponse = {
-      cookie: jest.fn().mockReturnThis(),
-      end: jest.fn().mockReturnThis(),
-      setHeader: jest.fn().mockReturnThis(),
-      statusCode: jest.fn().mockReturnThis(),
-    } as unknown as express.Response;
-
-    await oauthProvider.start(mockRequest, mockResponse);
-
-    expect(mockResponse.cookie).toBeCalledTimes(1);
-    expect(mockResponse.cookie).toBeCalledWith(
-      `${oAuthProviderOptions.providerId}-nonce`,
-      expect.any(String),
-      expect.objectContaining({
-        domain: 'domain.org',
-        path: '/auth/test-provider/handler',
-        secure: false,
-      }),
-    );
-  });
-
   it('sets the correct cookie configuration using a callbackUrl', async () => {
     const config = {
       baseUrl: 'http://domain.org/auth',

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

@@ -35,7 +35,7 @@ import {
   NotAllowedError,
 } from '@backstage/errors';
 import { TokenIssuer } from '../../identity/types';
-import { getCookieConfig, readState, verifyNonce } from './helpers';
+import { defaultCookieConfigurer, readState, verifyNonce } from './helpers';
 import { postMessageResponse, ensuresXRequestedWith } from '../flow';
 import {
   OAuthHandlers,
@@ -58,7 +58,7 @@ export type Options = {
   appOrigin: string;
   tokenIssuer: TokenIssuer;
   isOriginAllowed: (origin: string) => boolean;
-  callbackUrl?: string;
+  callbackUrl: string;
 };
 export class OAuthAdapter implements AuthProviderRouteHandlers {
   static fromConfig(
@@ -74,18 +74,20 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
     >,
   ): OAuthAdapter {
     const { origin: appOrigin } = new URL(config.appUrl);
-    const authUrl = new URL(options.callbackUrl ?? config.baseUrl);
-    const { cookieDomain, cookiePath, secure } = getCookieConfig(
-      authUrl,
-      options.providerId,
-    );
+
+    const cookieConfigurer = config.cookieConfigurer ?? defaultCookieConfigurer;
+    const cookieConfig = cookieConfigurer({
+      providerId: options.providerId,
+      baseUrl: config.baseUrl,
+      callbackUrl: options.callbackUrl,
+    });
 
     return new OAuthAdapter(handlers, {
       ...options,
       appOrigin,
-      cookieDomain,
-      cookiePath,
-      secure,
+      cookieDomain: cookieConfig.domain,
+      cookiePath: cookieConfig.path,
+      secure: cookieConfig.secure,
       isOriginAllowed: config.isOriginAllowed,
     });
   }

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

@@ -19,7 +19,7 @@ import {
   verifyNonce,
   encodeState,
   readState,
-  getCookieConfig,
+  defaultCookieConfigurer,
 } from './helpers';
 
 describe('OAuthProvider Utils', () => {
@@ -110,30 +110,43 @@ describe('OAuthProvider Utils', () => {
     });
   });
 
-  describe('getCookieConfig', () => {
+  describe('defaultCookieConfigurer', () => {
     it('should set the correct domain and path for a base url', () => {
-      const mockAuthUrl = new URL('http://domain.org/auth');
-      expect(getCookieConfig(mockAuthUrl, 'test-provider')).toMatchObject({
-        cookieDomain: 'domain.org',
-        cookiePath: '/auth/test-provider',
+      expect(
+        defaultCookieConfigurer({
+          baseUrl: '',
+          providerId: 'test-provider',
+          callbackUrl: 'http://domain.org/auth',
+        }),
+      ).toMatchObject({
+        domain: 'domain.org',
+        path: '/auth/test-provider',
         secure: false,
       });
     });
 
     it('should set the correct domain and path for a url containing a frame handler', () => {
-      const mockAuthUrl = new URL(
-        'http://domain.org/auth/test-provider/handler/frame',
-      );
-      expect(getCookieConfig(mockAuthUrl, 'test-provider')).toMatchObject({
-        cookieDomain: 'domain.org',
-        cookiePath: '/auth/test-provider',
+      expect(
+        defaultCookieConfigurer({
+          baseUrl: '',
+          providerId: 'test-provider',
+          callbackUrl: 'http://domain.org/auth/test-provider/handler/frame',
+        }),
+      ).toMatchObject({
+        domain: 'domain.org',
+        path: '/auth/test-provider',
         secure: false,
       });
     });
 
     it('should set the secure flag if url is using https', () => {
-      const mockAuthUrl = new URL('https://domain.org/auth');
-      expect(getCookieConfig(mockAuthUrl, 'test-provider')).toMatchObject({
+      expect(
+        defaultCookieConfigurer({
+          baseUrl: '',
+          providerId: 'test-provider',
+          callbackUrl: 'https://domain.org/auth',
+        }),
+      ).toMatchObject({
         secure: true,
       });
     });

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

@@ -17,6 +17,7 @@
 import express from 'express';
 import { OAuthState } from './types';
 import pickBy from 'lodash/pickBy';
+import { CookieConfigurer } from '../../providers/types';
 
 export const readState = (stateString: string): OAuthState => {
   const state = Object.fromEntries(
@@ -58,20 +59,19 @@ export const verifyNonce = (req: express.Request, providerId: string) => {
   }
 };
 
-export const getCookieConfig = (authUrl: URL, providerId: string) => {
-  const { hostname: cookieDomain, pathname, protocol } = authUrl;
+export const defaultCookieConfigurer: CookieConfigurer = ({
+  callbackUrl,
+  providerId,
+}) => {
+  const { hostname: domain, pathname, protocol } = new URL(callbackUrl);
   const secure = protocol === 'https:';
 
   // If the provider supports callbackUrls, the pathname will
   // contain the complete path to the frame handler so we need
   // to slice off the trailing part of the path.
-  const cookiePath = pathname.endsWith(`${providerId}/handler/frame`)
+  const path = pathname.endsWith(`${providerId}/handler/frame`)
     ? pathname.slice(0, -'/handler/frame'.length)
     : `${pathname}/${providerId}`;
 
-  return {
-    cookieDomain,
-    cookiePath,
-    secure,
-  };
+  return { domain, path, secure };
 };

plugins/auth-backend/src/providers/index.ts

@@ -43,6 +43,7 @@ export type {
   AuthHandlerResult,
   SignInResolver,
   SignInInfo,
+  CookieConfigurer,
 } from './types';
 
 // These types are needed for a postMessage from the login pop-up

plugins/auth-backend/src/providers/types.ts

@@ -38,6 +38,19 @@ export type AuthResolverContext = {
   logger: Logger;
 };
 
+/**
+ * The callback used to resolve the cookie configuration for auth providers that use cookies.
+ * @public
+ */
+export type CookieConfigurer = (ctx: {
+  /** ID of the auth provider that this configuration applies to */
+  providerId: string;
+  /** The externally reachable base URL of the auth-backend plugin */
+  baseUrl: string;
+  /** The configured callback URL of the auth provider */
+  callbackUrl: string;
+}) => { domain: string; path: string; secure: boolean };
+
 export type AuthProviderConfig = {
   /**
    * The protocol://domain[:port] where the app is hosted. This is used to construct the
@@ -54,6 +67,11 @@ export type AuthProviderConfig = {
    * A function that is called to check whether an origin is allowed to receive the authentication result.
    */
   isOriginAllowed: (origin: string) => boolean;
+
+  /**
+   * The function used to resolve cookie configuration based on the auth provider options.
+   */
+  cookieConfigurer?: CookieConfigurer;
 };
 
 export type RedirectInfo = {

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

@@ -111,7 +111,11 @@ export async function createRouter(
       try {
         const provider = providerFactory({
           providerId,
-          globalConfig: { baseUrl: authUrl, appUrl, isOriginAllowed },
+          globalConfig: {
+            baseUrl: authUrl,
+            appUrl,
+            isOriginAllowed,
+          },
           config: providersConfig.getConfig(providerId),
           logger,
           tokenManager,