diff --git a/.changeset/kind-penguins-report.md b/.changeset/kind-penguins-report.md index 6596f727d2..9f2401491e 100644 --- a/.changeset/kind-penguins-report.md +++ b/.changeset/kind-penguins-report.md @@ -1,5 +1,8 @@ --- -'@backstage/plugin-auth-backend': patch +'@backstage/plugin-auth-backend': minor --- -Allow CookieConfigurer to optionally return the SameSite cookie attribute. Return `SameSite=None` in `defaultCookieConfigurer` for secure contexts to allow cookies to be included in third-party requests. +CookieConfigurer can optionally return the `SameSite` cookie attribute. +CookieConfigurer now requires an additional argument `appOrigin` - the origin URL of the app - which is used to calculate the `SameSite` attribute. +defaultCookieConfigurer returns the `SameSite` attribute which defaults to `Lax`. In cases where an auth-backend is running on a different domain than the App, `SameSite=None` is used - but only for secure contexts. This is so that cookies can be included in third-party requests. +OAuthAdapterOptions has been modified to require additional arguments, `baseUrl`, `cookieConfigurer` and `cookieConfig`. diff --git a/plugins/auth-backend/api-report.md b/plugins/auth-backend/api-report.md index ed3561d37c..8cb7e35e9b 100644 --- a/plugins/auth-backend/api-report.md +++ b/plugins/auth-backend/api-report.md @@ -183,6 +183,7 @@ export type CookieConfigurer = (ctx: { providerId: string; baseUrl: string; callbackUrl: string; + appOrigin: string; }) => { domain: string; path: string; @@ -283,7 +284,9 @@ export type OAuthAdapterOptions = { providerId: string; persistScopes?: boolean; appOrigin: string; + baseUrl: string; cookieConfig: ReturnType; + cookieConfigurer: CookieConfigurer; isOriginAllowed: (origin: string) => boolean; callbackUrl: string; }; diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts index a61554b600..d59a712205 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts @@ -37,6 +37,10 @@ const mockResponseData = { }; describe('OAuthAdapter', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + class MyAuthProvider implements OAuthHandlers { async start() { return { @@ -58,17 +62,19 @@ describe('OAuthAdapter', () => { } } const providerInstance = new MyAuthProvider(); - const cookieConfig = { + const mockCookieConfig: ReturnType = { domain: 'example.com', path: '/auth/test-provider', secure: false, - sameSite: 'lax', - } as ReturnType; + }; + const mockCookieConfigurer = jest.fn().mockReturnValue(mockCookieConfig); const oAuthProviderOptions = { providerId: 'test-provider', appOrigin: 'http://localhost:3000', - cookieConfig, + cookieConfig: mockCookieConfig, + cookieConfigurer: mockCookieConfigurer, + baseUrl: 'http://example.com:7007', tokenIssuer: { issueToken: async () => 'my-id-token', listPublicKeys: async () => ({ keys: [] }), @@ -135,11 +141,13 @@ describe('OAuthAdapter', () => { } as unknown as express.Response; await oauthProvider.frameHandler(mockRequest, mockResponse); + expect(mockCookieConfigurer).toHaveBeenCalledTimes(1); expect(mockResponse.cookie).toHaveBeenCalledTimes(1); expect(mockResponse.cookie).toHaveBeenCalledWith( expect.stringContaining('test-provider-refresh-token'), expect.stringContaining('token'), expect.objectContaining({ + httpOnly: true, path: '/auth/test-provider', maxAge: THOUSAND_DAYS_MS, secure: false, @@ -210,6 +218,7 @@ describe('OAuthAdapter', () => { } as unknown as express.Response; await oauthProvider.frameHandler(mockHandleReq, mockHandleRes); + expect(mockCookieConfigurer).toHaveBeenCalledTimes(1); expect(mockHandleRes.cookie).toHaveBeenCalledTimes(1); expect(mockHandleRes.cookie).toHaveBeenCalledWith( 'test-provider-granted-scope', @@ -303,7 +312,7 @@ describe('OAuthAdapter', () => { }); }); - it('sets the correct cookie configuration using a callbackUrl', async () => { + it('sets the correct cookie configuration using an unsecure callbackUrl', async () => { const config = { baseUrl: 'http://domain.org/auth', appUrl: 'http://domain.org', @@ -312,7 +321,7 @@ describe('OAuthAdapter', () => { const oauthProvider = OAuthAdapter.fromConfig(config, providerInstance, { ...oAuthProviderOptions, - callbackUrl: 'https://authdomain.org/auth/test-provider/handler/frame', + callbackUrl: 'http://authdomain.org/auth/test-provider/handler/frame', }); const mockRequest = { @@ -330,14 +339,112 @@ describe('OAuthAdapter', () => { } as unknown as express.Response; await oauthProvider.start(mockRequest, mockResponse); - + expect(mockCookieConfigurer).not.toHaveBeenCalled(); expect(mockResponse.cookie).toHaveBeenCalledTimes(1); expect(mockResponse.cookie).toHaveBeenCalledWith( `${oAuthProviderOptions.providerId}-nonce`, expect.any(String), expect.objectContaining({ + httpOnly: true, domain: 'authdomain.org', path: '/auth/test-provider/handler', + secure: false, + sameSite: 'lax', + }), + ); + }); + + it('sets the correct cookie configuration using an secure callbackUrl', async () => { + const config = { + baseUrl: 'https://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 state = { + nonce: 'nonce', + env: 'development', + }; + + const mockRequest = { + cookies: { + 'test-provider-nonce': 'nonce', + }, + query: { + state: encodeState(state), + }, + } as unknown as express.Request; + + const mockResponse = { + cookie: jest.fn().mockReturnThis(), + setHeader: jest.fn().mockReturnThis(), + end: jest.fn().mockReturnThis(), + } as unknown as express.Response; + + await oauthProvider.frameHandler(mockRequest, mockResponse); + expect(mockCookieConfigurer).not.toHaveBeenCalled(); + expect(mockResponse.cookie).toHaveBeenCalledTimes(1); + expect(mockResponse.cookie).toHaveBeenCalledWith( + expect.stringContaining('test-provider-refresh-token'), + expect.stringContaining('token'), + expect.objectContaining({ + httpOnly: true, + domain: 'authdomain.org', + path: '/auth/test-provider', + secure: true, + sameSite: 'none', + }), + ); + }); + + it('sets the correct cookie configuration using state', async () => { + const config = { + baseUrl: 'https://domain.org/auth', + appUrl: 'http://domain.org', + isOriginAllowed: () => true, + }; + + const oauthProvider = OAuthAdapter.fromConfig(config, providerInstance, { + ...oAuthProviderOptions, + callbackUrl: 'https://domain.org/auth/test-provider/handler/frame', + }); + + const state = { + nonce: 'nonce', + env: 'development', + origin: 'http://other.domain', + }; + + const mockRequest = { + cookies: { + 'test-provider-nonce': 'nonce', + }, + query: { + state: encodeState(state), + }, + } as unknown as express.Request; + + const mockResponse = { + cookie: jest.fn().mockReturnThis(), + setHeader: jest.fn().mockReturnThis(), + end: jest.fn().mockReturnThis(), + } as unknown as express.Response; + + await oauthProvider.frameHandler(mockRequest, mockResponse); + expect(mockCookieConfigurer).not.toHaveBeenCalled(); + expect(mockResponse.cookie).toHaveBeenCalledTimes(1); + expect(mockResponse.cookie).toHaveBeenCalledWith( + expect.stringContaining('test-provider-refresh-token'), + expect.stringContaining('token'), + expect.objectContaining({ + httpOnly: true, + domain: 'domain.org', + path: '/auth/test-provider', secure: true, sameSite: 'none', }), diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index c021104010..64dc4f70e3 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -50,7 +50,9 @@ export type OAuthAdapterOptions = { providerId: string; persistScopes?: boolean; appOrigin: string; + baseUrl: string; cookieConfig: ReturnType; + cookieConfigurer: CookieConfigurer; isOriginAllowed: (origin: string) => boolean; callbackUrl: string; }; @@ -65,24 +67,28 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { 'providerId' | 'persistScopes' | 'callbackUrl' >, ): OAuthAdapter { - const { origin: appOrigin } = new URL(config.appUrl); + const { appUrl, baseUrl, isOriginAllowed } = config; + const { origin: appOrigin } = new URL(appUrl); const cookieConfigurer = config.cookieConfigurer ?? defaultCookieConfigurer; const cookieConfig = cookieConfigurer({ providerId: options.providerId, baseUrl: config.baseUrl, callbackUrl: options.callbackUrl, + appOrigin, }); return new OAuthAdapter(handlers, { ...options, appOrigin, + baseUrl, cookieConfig, - isOriginAllowed: config.isOriginAllowed, + cookieConfigurer, + isOriginAllowed, }); } - private readonly baseCookieOptions: CookieOptions; + private baseCookieOptions: CookieOptions; constructor( private readonly handlers: OAuthHandlers, @@ -90,6 +96,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { ) { this.baseCookieOptions = { httpOnly: true, + sameSite: 'lax', ...this.options.cookieConfig, }; } @@ -147,6 +154,18 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { } } + // Update cookie options to reflect any changes for the appOrigin + const updatedCookieOptions = this.options.cookieConfigurer({ + providerId: this.options.providerId, + baseUrl: this.options.baseUrl, + callbackUrl: this.options.callbackUrl, + appOrigin, + }); + this.baseCookieOptions = { + ...this.baseCookieOptions, + ...updatedCookieOptions, + }; + // verify nonce cookie and state cookie on callback verifyNonce(req, this.options.providerId); diff --git a/plugins/auth-backend/src/lib/oauth/helpers.test.ts b/plugins/auth-backend/src/lib/oauth/helpers.test.ts index 2202fa60ea..58a3dcab05 100644 --- a/plugins/auth-backend/src/lib/oauth/helpers.test.ts +++ b/plugins/auth-backend/src/lib/oauth/helpers.test.ts @@ -117,6 +117,7 @@ describe('OAuthProvider Utils', () => { baseUrl: '', providerId: 'test-provider', callbackUrl: 'http://domain.org/auth', + appOrigin: 'http://domain.org', }), ).toMatchObject({ domain: 'domain.org', @@ -131,6 +132,7 @@ describe('OAuthProvider Utils', () => { baseUrl: '', providerId: 'test-provider', callbackUrl: 'http://domain.org/auth/test-provider/handler/frame', + appOrigin: 'http://domain.org', }), ).toMatchObject({ domain: 'domain.org', @@ -145,35 +147,67 @@ describe('OAuthProvider Utils', () => { baseUrl: '', providerId: 'test-provider', callbackUrl: 'https://domain.org/auth', + appOrigin: 'http://domain.org', }), ).toMatchObject({ secure: true, }); }); - it('should set sameSite to none for https', () => { + it('should set sameSite to lax for https on the same domain', () => { expect( defaultCookieConfigurer({ baseUrl: '', providerId: 'test-provider', callbackUrl: 'https://domain.org/auth', + appOrigin: 'http://domain.org', }), ).toMatchObject({ - sameSite: 'none', + sameSite: 'lax', secure: true, }); }); - it('should set sameSite to lax for http', () => { + + it('should set sameSite to lax for http on the same domain', () => { expect( defaultCookieConfigurer({ baseUrl: '', providerId: 'test-provider', callbackUrl: 'http://domain.org/auth', + appOrigin: 'http://domain.org', }), ).toMatchObject({ sameSite: 'lax', secure: false, }); }); + + it('should set sameSite to lax if not secure and on different domains', () => { + expect( + defaultCookieConfigurer({ + baseUrl: '', + providerId: 'test-provider', + callbackUrl: 'http://authdomain.org/auth', + appOrigin: 'http://domain.org', + }), + ).toMatchObject({ + sameSite: 'lax', + secure: false, + }); + }); + + it('should set sameSite to none if secure and on different domains', () => { + expect( + defaultCookieConfigurer({ + baseUrl: '', + providerId: 'test-provider', + callbackUrl: 'https://authdomain.org/auth', + appOrigin: 'http://domain.org', + }), + ).toMatchObject({ + sameSite: 'none', + secure: true, + }); + }); }); }); diff --git a/plugins/auth-backend/src/lib/oauth/helpers.ts b/plugins/auth-backend/src/lib/oauth/helpers.ts index b18db59ca7..2b4e50b477 100644 --- a/plugins/auth-backend/src/lib/oauth/helpers.ts +++ b/plugins/auth-backend/src/lib/oauth/helpers.ts @@ -65,10 +65,19 @@ export const verifyNonce = (req: express.Request, providerId: string) => { export const defaultCookieConfigurer: CookieConfigurer = ({ callbackUrl, providerId, + appOrigin, }) => { const { hostname: domain, pathname, protocol } = new URL(callbackUrl); const secure = protocol === 'https:'; - const sameSite = secure ? 'none' : 'lax'; + + // For situations where the auth-backend is running on a + // different domain than the app, we set the SameSite attribute + // to 'none' to allow third-party access to the cookie, but + // only if it's in a secure context (https). + let sameSite: ReturnType['sameSite'] = 'lax'; + if (new URL(appOrigin).hostname !== domain && secure) { + sameSite = 'none'; + } // If the provider supports callbackUrls, the pathname will // contain the complete path to the frame handler so we need diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index f22e6b0492..1459cff484 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -100,6 +100,8 @@ export type CookieConfigurer = (ctx: { baseUrl: string; /** The configured callback URL of the auth provider */ callbackUrl: string; + /** The origin URL of the app */ + appOrigin: string; }) => { domain: string; path: string;