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

supertokens · Oct 23, 2023 · 8ea3a39 · 8ea3a39
1 parent 9460b38
commit 8ea3a39
Show file tree

Hide file tree

Showing 22 changed files with 297 additions and 107 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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

diff --git a/lib/build/framework/awsLambda/framework.js b/lib/build/framework/awsLambda/framework.js
@@ -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
@@ -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;
                 }

diff --git a/lib/build/framework/custom/framework.js b/lib/build/framework/custom/framework.js
@@ -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);
             }

diff --git a/lib/build/framework/fastify/framework.js b/lib/build/framework/fastify/framework.js
@@ -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);
@@ -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

diff --git a/lib/build/framework/fastify/index.d.ts b/lib/build/framework/fastify/index.d.ts
@@ -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
     >

diff --git a/lib/build/framework/koa/framework.js b/lib/build/framework/koa/framework.js
@@ -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);
@@ -108,6 +115,7 @@ class KoaResponse extends response_1.BaseResponse {
                 expires: new Date(expires),
                 domain,
                 path,
+                overwrite: true,
             });
         };
         /**

diff --git a/lib/build/framework/utils.d.ts b/lib/build/framework/utils.d.ts
@@ -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[],

diff --git a/lib/build/framework/utils.js b/lib/build/framework/utils.js
@@ -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

diff --git a/lib/build/version.d.ts b/lib/build/version.d.ts
diff --git a/lib/build/version.js b/lib/build/version.js
diff --git a/lib/ts/framework/awsLambda/framework.ts b/lib/ts/framework/awsLambda/framework.ts
@@ -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,
+        ];
     };
 
     /**
@@ -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;
             }

diff --git a/lib/ts/framework/custom/framework.ts b/lib/ts/framework/custom/framework.ts
@@ -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);
         }

diff --git a/lib/ts/framework/fastify/framework.ts b/lib/ts/framework/fastify/framework.ts
@@ -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);
@@ -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,
+        ]);
     };
 
     /**

diff --git a/lib/ts/framework/koa/framework.ts b/lib/ts/framework/koa/framework.ts
@@ -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);
@@ -136,6 +143,7 @@ export class KoaResponse extends BaseResponse {
             expires: new Date(expires),
             domain,
             path,
+            overwrite: true,
         });
     };
 

diff --git a/lib/ts/framework/utils.ts b/lib/ts/framework/utils.ts
@@ -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