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

fix(auth): update cookie deletion logic for chunked cookies

Browse Source 
Signed-off-by: Jessica He <jhe@redhat.com>
This commit is contained in:
Jessica He
2025-12-04 15:14:02 -05:00
parent f7aef07e50
commit e9dd634664
3 changed files with 207 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/sour-bats-press.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-auth-node': patch
---
fix flawed cookie removal logic with chunked tokens
plugins/auth-node/src/oauth/OAuthCookieManager.ts
+30 -13
View File
@@ -152,6 +152,36 @@ export class OAuthCookieManager {
};
const req = res.req;
let output = res;
const chunkedFormatExists = OAuthCookieManager.chunkedCookieExists(
req,
name,
);
// If using the default cookieConfigurer, delete old cookie with domain
// explicitly set to the callbackUrl's domain (legacy behavior)
if (this.cookieConfigurer === defaultCookieConfigurer) {
const { hostname: domain } = new URL(this.options.callbackUrl);
output = output.cookie(name, '', {
...this.getRemoveCookieOptions(),
domain: domain,
});
if (chunkedFormatExists) {
for (let chunkNumber = 0; ; chunkNumber++) {
const key = OAuthCookieManager.getCookieChunkName(name, chunkNumber);
const exists = !!req.cookies[key];
if (!exists) {
break;
}
output = output.cookie(key, '', {
...this.getRemoveCookieOptions(),
domain: domain,
});
}
}
}
if (val.length > MAX_COOKIE_SIZE_CHARACTERS) {
const nonChunkedFormatExists = !!req.cookies[name];
if (nonChunkedFormatExists) {
@@ -169,10 +199,6 @@ export class OAuthCookieManager {
return output;
}
const chunkedFormatExists = OAuthCookieManager.chunkedCookieExists(
req,
name,
);
if (chunkedFormatExists) {
for (let chunkNumber = 0; ; chunkNumber++) {
const key = OAuthCookieManager.getCookieChunkName(name, chunkNumber);
@@ -184,15 +210,6 @@ export class OAuthCookieManager {
}
}
// If using the default cookieConfigurer, delete old cookie with domain set to the callbackUrl's domain (legacy behavior)
if (this.cookieConfigurer === defaultCookieConfigurer) {
const { hostname: domain } = new URL(this.options.callbackUrl);
output = output.cookie(name, '', {
...this.getRemoveCookieOptions(),
domain: domain,
});
}
return output.cookie(name, val, options);
}
plugins/auth-node/src/oauth/createOAuthRouteHandlers.test.ts
+172
View File
@@ -337,6 +337,178 @@ describe('createOAuthRouteHandlers', () => {
expect(getGrantedScopesCookie(agent)).toBeUndefined();
});
it('should clean up old cookies (non-chunked) with domain attribute during migration', async () => {
const agent = request.agent(
wrapInApp(createOAuthRouteHandlers(baseConfig)),
);
agent.jar.setCookie(
'my-provider-nonce=123',
'127.0.0.1',
'/my-provider/handler',
);
agent.jar.setCookie(
'my-provider-refresh-token=old-refresh-token; Domain=127.0.0.1',
'127.0.0.1',
'/my-provider',
);
mockAuthenticator.authenticate.mockResolvedValue({
fullProfile: { id: 'id' } as PassportProfile,
session: mockSession,
});
const res = await agent.get('/my-provider/handler/frame').query({
state: encodeOAuthState({
env: 'development',
nonce: '123',
} as OAuthState),
});
expect(res.status).toBe(200);
const setCookieHeaders = [res.get('Set-Cookie') ?? []].flat();
const hasDeleteWithDomain = setCookieHeaders.some(
cookie =>
cookie.includes('my-provider-refresh-token=;') &&
cookie.includes('Max-Age=0') &&
cookie.includes('Domain=127.0.0.1'),
);
expect(hasDeleteWithDomain).toBe(true);
expect(getRefreshTokenCookie(agent).value).toBe('refresh-token');
});
it('should clean up old chunked cookies with domain attribute during migration', async () => {
const agent = request.agent(
wrapInApp(createOAuthRouteHandlers(baseConfig)),
);
agent.jar.setCookie(
'my-provider-nonce=123',
'127.0.0.1',
'/my-provider/handler',
);
// Simulate old chunked cookies with domain attribute (legacy format)
agent.jar.setCookie(
`my-provider-refresh-token-0=${fiveKilobyteRefreshToken.slice(
0,
4000,
)}; Domain=127.0.0.1`,
'/my-provider',
);
agent.jar.setCookie(
`my-provider-refresh-token-1=${fiveKilobyteRefreshToken.slice(
4000,
)}; Domain=127.0.0.1`,
'/my-provider',
);
mockAuthenticator.authenticate.mockResolvedValue({
fullProfile: { id: 'id' } as PassportProfile,
session: {
...mockSession,
refreshToken: fiveKilobyteRefreshToken,
},
});
const res = await agent.get('/my-provider/handler/frame').query({
state: encodeOAuthState({
env: 'development',
nonce: '123',
} as OAuthState),
});
expect(res.status).toBe(200);
const setCookieHeaders = [res.get('Set-Cookie') ?? []].flat();
const hasChunk0DeleteWithDomain = setCookieHeaders.some(
cookie =>
cookie.includes('my-provider-refresh-token-0=') &&
cookie.includes('Max-Age=0') &&
cookie.includes('Domain=127.0.0.1'),
);
const hasChunk1DeleteWithDomain = setCookieHeaders.some(
cookie =>
cookie.includes('my-provider-refresh-token-1=') &&
cookie.includes('Max-Age=0') &&
cookie.includes('Domain=127.0.0.1'),
);
expect(hasChunk0DeleteWithDomain).toBe(true);
expect(hasChunk1DeleteWithDomain).toBe(true);
expect(getRefreshTokenCookie(agent, 0).value).toBe(
fiveKilobyteRefreshToken.slice(0, 4000),
);
expect(getRefreshTokenCookie(agent, 1).value).toBe(
fiveKilobyteRefreshToken.slice(4000),
);
});
it('should clean up old chunked cookies with domain when migrating to non-chunked', async () => {
const agent = request.agent(
wrapInApp(createOAuthRouteHandlers(baseConfig)),
);
agent.jar.setCookie(
'my-provider-nonce=123',
'127.0.0.1',
'/my-provider/handler',
);
agent.jar.setCookie(
`my-provider-refresh-token-0=${fiveKilobyteRefreshToken.slice(
0,
4000,
)}; Domain=127.0.0.1`,
'/my-provider',
);
agent.jar.setCookie(
`my-provider-refresh-token-1=${fiveKilobyteRefreshToken.slice(
4000,
)}; Domain=127.0.0.1`,
'/my-provider',
);
mockAuthenticator.authenticate.mockResolvedValue({
fullProfile: { id: 'id' } as PassportProfile,
session: mockSession,
});
const res = await agent.get('/my-provider/handler/frame').query({
state: encodeOAuthState({
env: 'development',
nonce: '123',
} as OAuthState),
});
expect(res.status).toBe(200);
const setCookieHeaders = [res.get('Set-Cookie') ?? []].flat();
const hasChunk0DeleteWithDomain = setCookieHeaders.some(
cookie =>
cookie.includes('my-provider-refresh-token-0=;') &&
cookie.includes('Max-Age=0') &&
cookie.includes('Domain=127.0.0.1'),
);
const hasChunk1DeleteWithDomain = setCookieHeaders.some(
cookie =>
cookie.includes('my-provider-refresh-token-1=;') &&
cookie.includes('Max-Age=0') &&
cookie.includes('Domain=127.0.0.1'),
);
expect(hasChunk0DeleteWithDomain).toBe(true);
expect(hasChunk1DeleteWithDomain).toBe(true);
expect(getRefreshTokenCookie(agent).value).toBe('refresh-token');
expect(getRefreshTokenCookie(agent, 0)).toBeUndefined();
expect(getRefreshTokenCookie(agent, 1)).toBeUndefined();
});
it('should authenticate with sign-in, profile transform, and persisted scopes', async () => {
const agent = request.agent(
wrapInApp(