Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: repeated headers/entries in access-control-expose-headers #726

Merged
merged 1 commit into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html)

## [16.3.4] - 2023-10-22

### Fixes

- Fixes an issue where sometimes the `Access-Control-Expose-Headers` header value would contain duplicates

## [16.3.3] - 2023-10-19

- Tests `null` values in `ProviderConfig` saved in core
Expand Down
19 changes: 16 additions & 3 deletions lib/build/framework/awsLambda/framework.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,10 @@ class AWSResponse extends response_1.BaseResponse {
path,
sameSite
);
this.event.supertokens.response.cookies.push(serialisedCookie);
this.event.supertokens.response.cookies = [
...this.event.supertokens.response.cookies.filter((c) => !c.startsWith(key + "=")),
serialisedCookie,
];
};
/**
* @param {number} statusCode
Expand Down Expand Up @@ -187,8 +190,18 @@ class AWSResponse extends response_1.BaseResponse {
}
}
if (supertokensHeaders[i].allowDuplicateKey && currentValue !== undefined) {
let newValue = `${currentValue}, ${supertokensHeaders[i].value}`;
headers[supertokensHeaders[i].key] = newValue;
/**
We only want to append if it does not already exist
For example if the caller is trying to add front token to the access control exposed headers property
we do not want to append if something else had already added it
*/
if (
typeof currentValue !== "string" ||
!currentValue.includes(supertokensHeaders[i].value.toString())
) {
let newValue = `${currentValue}, ${supertokensHeaders[i].value}`;
headers[supertokensHeaders[i].key] = newValue;
}
} else {
headers[supertokensHeaders[i].key] = supertokensHeaders[i].value;
}
Expand Down
10 changes: 9 additions & 1 deletion lib/build/framework/custom/framework.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,16 @@ class CollectingResponse extends response_1.BaseResponse {
this.body = html;
};
this.setHeader = (key, value, allowDuplicateKey) => {
var _a;
if (allowDuplicateKey) {
this.headers.append(key, value);
/**
We only want to append if it does not already exist
For example if the caller is trying to add front token to the access control exposed headers property
we do not want to append if something else had already added it
*/
if (!((_a = this.headers.get(key)) === null || _a === void 0 ? void 0 : _a.includes(value))) {
this.headers.append(key, value);
}
} else {
this.headers.set(key, value);
}
Expand Down
54 changes: 16 additions & 38 deletions lib/build/framework/fastify/framework.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,14 @@ class FastifyResponse extends response_1.BaseResponse {
if (existingValue === undefined) {
this.response.header(key, value);
} else if (allowDuplicateKey) {
this.response.header(key, existingValue + ", " + value);
/**
We only want to append if it does not already exist
For example if the caller is trying to add front token to the access control exposed headers property
we do not want to append if something else had already added it
*/
if (typeof existingValue !== "string" || !existingValue.includes(value)) {
this.response.header(key, existingValue + ", " + value);
}
} else {
// we overwrite the current one with the new one
this.response.header(key, value);
Expand All @@ -102,43 +109,14 @@ class FastifyResponse extends response_1.BaseResponse {
path,
sameSite
);
/**
* lets say if current value is undefined, prev -> undefined
*
* now if add AT,
* cookieValueToSetInHeader -> AT
* response header object will be:
*
* 'set-cookie': AT
*
* now if add RT,
*
* prev -> AT
* cookieValueToSetInHeader -> AT + RT
* and response header object will be:
*
* 'set-cookie': AT + AT + RT
*
* now if add IRT,
*
* prev -> AT + AT + RT
* cookieValueToSetInHeader -> IRT + AT + AT + RT
* and response header object will be:
*
* 'set-cookie': AT + AT + RT + IRT + AT + AT + RT
*
* To avoid this, we no longer get and use the previous value
*
* Old code:
*
* let prev: string | string[] | undefined = this.response.getHeader(COOKIE_HEADER) as
* | string
* | string[]
* | undefined;
* let cookieValueToSetInHeader = getCookieValueToSetInHeader(prev, serialisedCookie, key);
* this.response.header(COOKIE_HEADER, cookieValueToSetInHeader);
*/
this.response.header(constants_1.COOKIE_HEADER, serialisedCookie);
let oldHeaders = this.response.getHeader(constants_1.COOKIE_HEADER);
if (oldHeaders === undefined) oldHeaders = [];
else if (!(oldHeaders instanceof Array)) oldHeaders = [oldHeaders];
this.response.removeHeader(constants_1.COOKIE_HEADER);
this.response.header(constants_1.COOKIE_HEADER, [
...oldHeaders.filter((h) => !h.startsWith(key + "=")),
serialisedCookie,
]);
};
/**
* @param {number} statusCode
Expand Down
11 changes: 7 additions & 4 deletions lib/build/framework/fastify/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
// @ts-nocheck
/// <reference types="node" />
export type { SessionRequest } from "./framework";
export declare const plugin: import("fastify").FastifyPluginCallback<Record<never, never>, import("http").Server>;
export declare const plugin: import("fastify").FastifyPluginCallback<
Record<never, never>,
import("fastify").RawServerDefault
>;
export declare const errorHandler: () => (
err: any,
req: import("fastify").FastifyRequest<
import("fastify/types/route").RouteGenericInterface,
import("http").Server,
import("fastify").RawServerDefault,
import("http").IncomingMessage
>,
res: import("fastify").FastifyReply<
import("http").Server,
import("fastify").RawServerDefault,
import("http").IncomingMessage,
import("http").ServerResponse,
import("http").ServerResponse<import("http").IncomingMessage>,
import("fastify/types/route").RouteGenericInterface,
unknown
>
Expand Down
10 changes: 9 additions & 1 deletion lib/build/framework/koa/framework.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,14 @@ class KoaResponse extends response_1.BaseResponse {
if (existingValue === undefined) {
this.ctx.set(key, value);
} else if (allowDuplicateKey) {
this.ctx.set(key, existingValue + ", " + value);
/**
We only want to append if it does not already exist
For example if the caller is trying to add front token to the access control exposed headers property
we do not want to append if something else had already added it
*/
if (typeof existingValue !== "string" || !existingValue.includes(value)) {
this.ctx.set(key, existingValue + ", " + value);
}
} else {
// we overwrite the current one with the new one
this.ctx.set(key, value);
Expand All @@ -108,6 +115,7 @@ class KoaResponse extends response_1.BaseResponse {
expires: new Date(expires),
domain,
path,
overwrite: true,
});
};
/**
Expand Down
2 changes: 1 addition & 1 deletion lib/build/framework/utils.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export declare function setCookieForServerResponse(
expires: number,
path: string,
sameSite: "strict" | "lax" | "none"
): ServerResponse;
): ServerResponse<IncomingMessage>;
export declare function getCookieValueToSetInHeader(
prev: string | string[] | undefined,
val: string | string[],
Expand Down
15 changes: 11 additions & 4 deletions lib/build/framework/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,17 @@ function setHeaderForExpressLikeResponse(res, key, value, allowDuplicateKey) {
res.setHeader(key, value);
}
} else if (allowDuplicateKey) {
if (res.header !== undefined) {
res.header(key, existingValue + ", " + value);
} else {
res.setHeader(key, existingValue + ", " + value);
/**
We only want to append if it does not already exist
For example if the caller is trying to add front token to the access control exposed headers property
we do not want to append if something else had already added it
*/
if (typeof existingValue !== "string" || !existingValue.includes(value)) {
if (res.header !== undefined) {
res.header(key, existingValue + ", " + value);
} else {
res.setHeader(key, existingValue + ", " + value);
}
}
} else {
// we overwrite the current one with the new one
Expand Down
2 changes: 1 addition & 1 deletion lib/build/version.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/build/version.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 16 additions & 3 deletions lib/ts/framework/awsLambda/framework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,10 @@ export class AWSResponse extends BaseResponse {
sameSite: "strict" | "lax" | "none"
) => {
let serialisedCookie = serializeCookieValue(key, value, domain, secure, httpOnly, expires, path, sameSite);
this.event.supertokens.response.cookies.push(serialisedCookie);
this.event.supertokens.response.cookies = [
...this.event.supertokens.response.cookies.filter((c) => !c.startsWith(key + "=")),
serialisedCookie,
];
};

/**
Expand Down Expand Up @@ -265,8 +268,18 @@ export class AWSResponse extends BaseResponse {
}
}
if (supertokensHeaders[i].allowDuplicateKey && currentValue !== undefined) {
let newValue = `${currentValue}, ${supertokensHeaders[i].value}`;
headers[supertokensHeaders[i].key] = newValue;
/**
We only want to append if it does not already exist
For example if the caller is trying to add front token to the access control exposed headers property
we do not want to append if something else had already added it
*/
if (
typeof currentValue !== "string" ||
!currentValue.includes(supertokensHeaders[i].value.toString())
) {
let newValue = `${currentValue}, ${supertokensHeaders[i].value}`;
headers[supertokensHeaders[i].key] = newValue;
}
} else {
headers[supertokensHeaders[i].key] = supertokensHeaders[i].value;
}
Expand Down
9 changes: 8 additions & 1 deletion lib/ts/framework/custom/framework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,14 @@ export class CollectingResponse extends BaseResponse {

setHeader = (key: string, value: string, allowDuplicateKey: boolean) => {
if (allowDuplicateKey) {
this.headers.append(key, value);
/**
We only want to append if it does not already exist
For example if the caller is trying to add front token to the access control exposed headers property
we do not want to append if something else had already added it
*/
if (!this.headers.get(key)?.includes(value)) {
this.headers.append(key, value);
}
} else {
this.headers.set(key, value);
}
Expand Down
56 changes: 18 additions & 38 deletions lib/ts/framework/fastify/framework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,14 @@ export class FastifyResponse extends BaseResponse {
if (existingValue === undefined) {
this.response.header(key, value);
} else if (allowDuplicateKey) {
this.response.header(key, existingValue + ", " + value);
/**
We only want to append if it does not already exist
For example if the caller is trying to add front token to the access control exposed headers property
we do not want to append if something else had already added it
*/
if (typeof existingValue !== "string" || !existingValue.includes(value)) {
this.response.header(key, existingValue + ", " + value);
}
} else {
// we overwrite the current one with the new one
this.response.header(key, value);
Expand All @@ -125,43 +132,16 @@ export class FastifyResponse extends BaseResponse {
sameSite: "strict" | "lax" | "none"
) => {
let serialisedCookie = serializeCookieValue(key, value, domain, secure, httpOnly, expires, path, sameSite);
/**
* lets say if current value is undefined, prev -> undefined
*
* now if add AT,
* cookieValueToSetInHeader -> AT
* response header object will be:
*
* 'set-cookie': AT
*
* now if add RT,
*
* prev -> AT
* cookieValueToSetInHeader -> AT + RT
* and response header object will be:
*
* 'set-cookie': AT + AT + RT
*
* now if add IRT,
*
* prev -> AT + AT + RT
* cookieValueToSetInHeader -> IRT + AT + AT + RT
* and response header object will be:
*
* 'set-cookie': AT + AT + RT + IRT + AT + AT + RT
*
* To avoid this, we no longer get and use the previous value
*
* Old code:
*
* let prev: string | string[] | undefined = this.response.getHeader(COOKIE_HEADER) as
* | string
* | string[]
* | undefined;
* let cookieValueToSetInHeader = getCookieValueToSetInHeader(prev, serialisedCookie, key);
* this.response.header(COOKIE_HEADER, cookieValueToSetInHeader);
*/
this.response.header(COOKIE_HEADER, serialisedCookie);

let oldHeaders: string | string[] | undefined = this.response.getHeader(COOKIE_HEADER);
if (oldHeaders === undefined) oldHeaders = [];
else if (!((oldHeaders as any) instanceof Array)) oldHeaders = [oldHeaders];

this.response.removeHeader(COOKIE_HEADER);
this.response.header(COOKIE_HEADER, [
...(oldHeaders as string[]).filter((h) => !h.startsWith(key + "=")),
serialisedCookie,
]);
};

/**
Expand Down
10 changes: 9 additions & 1 deletion lib/ts/framework/koa/framework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,14 @@ export class KoaResponse extends BaseResponse {
if (existingValue === undefined) {
this.ctx.set(key, value);
} else if (allowDuplicateKey) {
this.ctx.set(key, existingValue + ", " + value);
/**
We only want to append if it does not already exist
For example if the caller is trying to add front token to the access control exposed headers property
we do not want to append if something else had already added it
*/
if (typeof existingValue !== "string" || !existingValue.includes(value)) {
this.ctx.set(key, existingValue + ", " + value);
}
} else {
// we overwrite the current one with the new one
this.ctx.set(key, value);
Expand Down Expand Up @@ -136,6 +143,7 @@ export class KoaResponse extends BaseResponse {
expires: new Date(expires),
domain,
path,
overwrite: true,
});
};

Expand Down
15 changes: 11 additions & 4 deletions lib/ts/framework/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,17 @@ export function setHeaderForExpressLikeResponse(res: Response, key: string, valu
res.setHeader(key, value);
}
} else if (allowDuplicateKey) {
if (res.header !== undefined) {
res.header(key, existingValue + ", " + value);
} else {
res.setHeader(key, existingValue + ", " + value);
/**
We only want to append if it does not already exist
For example if the caller is trying to add front token to the access control exposed headers property
we do not want to append if something else had already added it
*/
if (typeof existingValue !== "string" || !existingValue.includes(value)) {
if (res.header !== undefined) {
res.header(key, existingValue + ", " + value);
} else {
res.setHeader(key, existingValue + ", " + value);
}
}
} else {
// we overwrite the current one with the new one
Expand Down
Loading
Loading