Merge pull request #9136 from backstage/eide/auth-callback-urls

[auth-backend]: Support callbackUrl when setting cookie configuration

.changeset/quiet-rivers-wave.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-auth-backend': patch
+---
+
+Running the `auth-backend` on multiple domains, perhaps different domains depending on the `auth.environment`, was previously not possible as the `domain` name of the cookie was taken from `backend.baseUrl`. This prevented any cookies to be set in the start of the auth flow as the domain of the cookie would not match the domain of the callbackUrl configured in the OAuth app. This change checks if a provider supports custom `callbackUrl`'s to be configured in the application configuration and uses the domain from that, allowing the `domain`'s to match and the cookie to be set.

plugins/auth-backend/api-report.md

@@ -478,7 +478,11 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
     handlers: OAuthHandlers,
     options: Pick<
       Options,
-      'providerId' | 'persistScopes' | 'disableRefresh' | 'tokenIssuer'
+      | 'providerId'
+      | 'persistScopes'
+      | 'disableRefresh'
+      | 'tokenIssuer'
+      | 'callbackUrl'
     >,
   ): OAuthAdapter;
   // (undocumented)

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

@@ -347,4 +347,85 @@ 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',
+      appUrl: 'http://domain.org',
+      isOriginAllowed: () => false,
+    };
+
+    const oauthProvider = OAuthAdapter.fromConfig(config, providerInstance, {
+      ...oAuthProviderOptions,
+      callbackUrl: 'https://authdomain.org/auth/test-provider/handler/frame',
+    });
+
+    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: 'authdomain.org',
+        path: '/auth/test-provider/handler',
+        secure: true,
+      }),
+    );
+  });
 });

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

@@ -35,7 +35,7 @@ import {
   NotAllowedError,
 } from '@backstage/errors';
 import { TokenIssuer } from '../../identity/types';
-import { readState, verifyNonce } from './helpers';
+import { getCookieConfig, readState, verifyNonce } from './helpers';
 import { postMessageResponse, ensuresXRequestedWith } from '../flow';
 import {
   OAuthHandlers,
@@ -58,25 +58,32 @@ export type Options = {
   appOrigin: string;
   tokenIssuer: TokenIssuer;
   isOriginAllowed: (origin: string) => boolean;
+  callbackUrl?: string;
 };
-
 export class OAuthAdapter implements AuthProviderRouteHandlers {
   static fromConfig(
     config: AuthProviderConfig,
     handlers: OAuthHandlers,
     options: Pick<
       Options,
-      'providerId' | 'persistScopes' | 'disableRefresh' | 'tokenIssuer'
+      | 'providerId'
+      | 'persistScopes'
+      | 'disableRefresh'
+      | 'tokenIssuer'
+      | 'callbackUrl'
     >,
   ): OAuthAdapter {
     const { origin: appOrigin } = new URL(config.appUrl);
-    const secure = config.baseUrl.startsWith('https://');
-    const url = new URL(config.baseUrl);
-    const cookiePath = `${url.pathname}/${options.providerId}`;
+    const authUrl = new URL(options.callbackUrl ?? config.baseUrl);
+    const { cookieDomain, cookiePath, secure } = getCookieConfig(
+      authUrl,
+      options.providerId,
+    );
+
     return new OAuthAdapter(handlers, {
       ...options,
       appOrigin,
-      cookieDomain: url.hostname,
+      cookieDomain,
       cookiePath,
       secure,
       isOriginAllowed: config.isOriginAllowed,

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

@@ -15,7 +15,12 @@
  */
 
 import express from 'express';
-import { verifyNonce, encodeState, readState } from './helpers';
+import {
+  verifyNonce,
+  encodeState,
+  readState,
+  getCookieConfig,
+} from './helpers';
 
 describe('OAuthProvider Utils', () => {
   describe('encodeState', () => {
@@ -104,4 +109,33 @@ describe('OAuthProvider Utils', () => {
       }).not.toThrow();
     });
   });
+
+  describe('getCookieConfig', () => {
+    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',
+        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',
+        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({
+        secure: true,
+      });
+    });
+  });
 });

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

@@ -57,3 +57,21 @@ export const verifyNonce = (req: express.Request, providerId: string) => {
     throw new Error('Invalid nonce');
   }
 };
+
+export const getCookieConfig = (authUrl: URL, providerId: string) => {
+  const { hostname: cookieDomain, pathname, protocol } = authUrl;
+  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`)
+    ? pathname.slice(0, -'/handler/frame'.length)
+    : `${pathname}/${providerId}`;
+
+  return {
+    cookieDomain,
+    cookiePath,
+    secure,
+  };
+};

plugins/auth-backend/src/providers/github/provider.ts

@@ -314,6 +314,7 @@ export const createGithubProvider = (
         persistScopes: true,
         providerId,
         tokenIssuer,
+        callbackUrl,
       });
     });
 };