paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 30 Packages Projects Releases Wiki Activity

Calculate SameSite attribute more carefully, defaulting to lax in most cases

Browse Source 
Signed-off-by: Marcus Eide <eide@spotify.com>
This commit is contained in:
Marcus Eide
2022-09-09 11:16:04 +02:00
parent 5fa831ce55
commit 12943d1ade
7 changed files with 193 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/kind-penguins-report.md
+5 -2
View File
@@ -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`.
plugins/auth-backend/api-report.md
+3
View File
@@ -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: CookieConfigurer;
isOriginAllowed: (origin: string) => boolean;
callbackUrl: string;
};
plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts
+114 -7
View File
@@ -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<CookieConfigurer> = {
domain: 'example.com',
path: '/auth/test-provider',
secure: false,
sameSite: 'lax',
} as ReturnType<CookieConfigurer>;
};
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',
}),
plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts
+22 -3
View File
@@ -50,7 +50,9 @@ export type OAuthAdapterOptions = {
providerId: string;
persistScopes?: boolean;
appOrigin: string;
baseUrl: string;
cookieConfig: ReturnType<CookieConfigurer>;
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);
plugins/auth-backend/src/lib/oauth/helpers.test.ts
+37 -3
View File
@@ -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,
});
});
});
});
plugins/auth-backend/src/lib/oauth/helpers.ts
+10 -1
View File
@@ -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<CookieConfigurer>['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
plugins/auth-backend/src/providers/types.ts
+2
View File
@@ -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;