diff --git a/.changeset/breezy-windows-jump.md b/.changeset/breezy-windows-jump.md new file mode 100644 index 0000000000..0d839614ff --- /dev/null +++ b/.changeset/breezy-windows-jump.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': minor +--- + +The `callbackUrl` option of `OAuthAdapter` is now required. diff --git a/.changeset/metal-lions-fix.md b/.changeset/metal-lions-fix.md new file mode 100644 index 0000000000..913098bbf7 --- /dev/null +++ b/.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. diff --git a/plugins/auth-backend/api-report.md b/plugins/auth-backend/api-report.md index 88b75d99b0..aaacd05815 100644 --- a/plugins/auth-backend/api-report.md +++ b/plugins/auth-backend/api-report.md @@ -214,6 +214,17 @@ export class CatalogIdentityClient { resolveCatalogMembership(query: MemberClaimQuery): Promise; } +// @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 ``` diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts index d1057b19a8..c1130b4270 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts +++ b/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', diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index 2e2d26db61..82a43c7c64 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/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, }); } diff --git a/plugins/auth-backend/src/lib/oauth/helpers.test.ts b/plugins/auth-backend/src/lib/oauth/helpers.test.ts index a98b684141..2f79cdef98 100644 --- a/plugins/auth-backend/src/lib/oauth/helpers.test.ts +++ b/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, }); }); diff --git a/plugins/auth-backend/src/lib/oauth/helpers.ts b/plugins/auth-backend/src/lib/oauth/helpers.ts index 878083e679..eec25696a3 100644 --- a/plugins/auth-backend/src/lib/oauth/helpers.ts +++ b/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 }; }; diff --git a/plugins/auth-backend/src/providers/index.ts b/plugins/auth-backend/src/providers/index.ts index 7779b67e04..1207254e11 100644 --- a/plugins/auth-backend/src/providers/index.ts +++ b/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 diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index b02e8d74b8..f3a1a95919 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/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 = { diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index bdb68929b1..ec1a52ffc9 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/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,