From 5fa831ce551eda0fb14ac4d05a372b8450048e07 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Tue, 6 Sep 2022 13:12:50 +0200 Subject: [PATCH] Add optional SameSite attribute to CookieConfigurer and return from defaultCookieConfigurer for secure contexts. Signed-off-by: Marcus Eide --- .changeset/kind-penguins-report.md | 5 ++++ plugins/auth-backend/api-report.md | 5 ++-- .../src/lib/oauth/OAuthAdapter.test.ts | 15 ++++++++--- .../src/lib/oauth/OAuthAdapter.ts | 16 ++++-------- .../src/lib/oauth/helpers.test.ts | 25 +++++++++++++++++++ plugins/auth-backend/src/lib/oauth/helpers.ts | 3 ++- plugins/auth-backend/src/providers/types.ts | 7 +++++- 7 files changed, 57 insertions(+), 19 deletions(-) create mode 100644 .changeset/kind-penguins-report.md diff --git a/.changeset/kind-penguins-report.md b/.changeset/kind-penguins-report.md new file mode 100644 index 0000000000..6596f727d2 --- /dev/null +++ b/.changeset/kind-penguins-report.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': patch +--- + +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. diff --git a/plugins/auth-backend/api-report.md b/plugins/auth-backend/api-report.md index d47abeee83..ed3561d37c 100644 --- a/plugins/auth-backend/api-report.md +++ b/plugins/auth-backend/api-report.md @@ -187,6 +187,7 @@ export type CookieConfigurer = (ctx: { domain: string; path: string; secure: boolean; + sameSite?: 'none' | 'lax' | 'strict'; }; // @public @@ -280,11 +281,9 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { // @public (undocumented) export type OAuthAdapterOptions = { providerId: string; - secure: boolean; persistScopes?: boolean; - cookieDomain: string; - cookiePath: string; appOrigin: string; + cookieConfig: ReturnType; 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 9dcd76e92e..a61554b600 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts @@ -18,6 +18,7 @@ import express from 'express'; import { THOUSAND_DAYS_MS, TEN_MINUTES_MS, OAuthAdapter } from './OAuthAdapter'; import { encodeState } from './helpers'; import { OAuthHandlers, OAuthState } from './types'; +import { CookieConfigurer } from '../../providers/types'; const mockResponseData = { providerInfo: { @@ -57,12 +58,17 @@ describe('OAuthAdapter', () => { } } const providerInstance = new MyAuthProvider(); + const cookieConfig = { + domain: 'example.com', + path: '/auth/test-provider', + secure: false, + sameSite: 'lax', + } as ReturnType; + const oAuthProviderOptions = { providerId: 'test-provider', - secure: false, appOrigin: 'http://localhost:3000', - cookieDomain: 'example.com', - cookiePath: '/auth/test-provider', + cookieConfig, tokenIssuer: { issueToken: async () => 'my-id-token', listPublicKeys: async () => ({ keys: [] }), @@ -136,6 +142,8 @@ describe('OAuthAdapter', () => { expect.objectContaining({ path: '/auth/test-provider', maxAge: THOUSAND_DAYS_MS, + secure: false, + sameSite: 'lax', }), ); }); @@ -331,6 +339,7 @@ describe('OAuthAdapter', () => { domain: 'authdomain.org', path: '/auth/test-provider/handler', 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 6caa9fed12..c021104010 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -24,6 +24,7 @@ import { import { AuthProviderRouteHandlers, AuthProviderConfig, + CookieConfigurer, } from '../../providers/types'; import { AuthenticationError, @@ -47,11 +48,9 @@ export const TEN_MINUTES_MS = 600 * 1000; /** @public */ export type OAuthAdapterOptions = { providerId: string; - secure: boolean; persistScopes?: boolean; - cookieDomain: string; - cookiePath: string; appOrigin: string; + cookieConfig: ReturnType; isOriginAllowed: (origin: string) => boolean; callbackUrl: string; }; @@ -78,9 +77,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { return new OAuthAdapter(handlers, { ...options, appOrigin, - cookieDomain: cookieConfig.domain, - cookiePath: cookieConfig.path, - secure: cookieConfig.secure, + cookieConfig, isOriginAllowed: config.isOriginAllowed, }); } @@ -93,10 +90,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { ) { this.baseCookieOptions = { httpOnly: true, - sameSite: 'lax', - secure: this.options.secure, - path: this.options.cookiePath, - domain: this.options.cookieDomain, + ...this.options.cookieConfig, }; } @@ -265,7 +259,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { res.cookie(`${this.options.providerId}-nonce`, nonce, { maxAge: TEN_MINUTES_MS, ...this.baseCookieOptions, - path: `${this.options.cookiePath}/handler`, + path: `${this.options.cookieConfig.path}/handler`, }); }; diff --git a/plugins/auth-backend/src/lib/oauth/helpers.test.ts b/plugins/auth-backend/src/lib/oauth/helpers.test.ts index b3052c93e1..2202fa60ea 100644 --- a/plugins/auth-backend/src/lib/oauth/helpers.test.ts +++ b/plugins/auth-backend/src/lib/oauth/helpers.test.ts @@ -150,5 +150,30 @@ describe('OAuthProvider Utils', () => { secure: true, }); }); + + it('should set sameSite to none for https', () => { + expect( + defaultCookieConfigurer({ + baseUrl: '', + providerId: 'test-provider', + callbackUrl: 'https://domain.org/auth', + }), + ).toMatchObject({ + sameSite: 'none', + secure: true, + }); + }); + it('should set sameSite to lax for http', () => { + expect( + defaultCookieConfigurer({ + baseUrl: '', + providerId: 'test-provider', + callbackUrl: 'http://domain.org/auth', + }), + ).toMatchObject({ + sameSite: 'lax', + secure: false, + }); + }); }); }); diff --git a/plugins/auth-backend/src/lib/oauth/helpers.ts b/plugins/auth-backend/src/lib/oauth/helpers.ts index 63d5d62ef9..b18db59ca7 100644 --- a/plugins/auth-backend/src/lib/oauth/helpers.ts +++ b/plugins/auth-backend/src/lib/oauth/helpers.ts @@ -68,6 +68,7 @@ export const defaultCookieConfigurer: CookieConfigurer = ({ }) => { const { hostname: domain, pathname, protocol } = new URL(callbackUrl); const secure = protocol === 'https:'; + const sameSite = secure ? 'none' : 'lax'; // If the provider supports callbackUrls, the pathname will // contain the complete path to the frame handler so we need @@ -76,5 +77,5 @@ export const defaultCookieConfigurer: CookieConfigurer = ({ ? pathname.slice(0, -'/handler/frame'.length) : `${pathname}/${providerId}`; - return { domain, path, secure }; + return { domain, path, secure, sameSite }; }; diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 574afade79..f22e6b0492 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -100,7 +100,12 @@ export type CookieConfigurer = (ctx: { baseUrl: string; /** The configured callback URL of the auth provider */ callbackUrl: string; -}) => { domain: string; path: string; secure: boolean }; +}) => { + domain: string; + path: string; + secure: boolean; + sameSite?: 'none' | 'lax' | 'strict'; +}; /** @public */ export type AuthProviderConfig = {