From 8d0097247b35d2eddea366b7b9aee638c291a973 Mon Sep 17 00:00:00 2001 From: Sudipto Sarkar Date: Sat, 2 Sep 2023 19:28:29 +0530 Subject: [PATCH 1/2] Fixed Cookie Authentication * Fixed Cookie Removal for Logouts * Fixed Setting of Cookie by specifying the path * Adding secure only when target is SSL/TLS * Added Unit Tests --- src/core/plugins/auth/wrap-actions.js | 10 ++- test/unit/core/plugins/auth/wrap-actions.js | 79 ++++++++++++++++++++- 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/src/core/plugins/auth/wrap-actions.js b/src/core/plugins/auth/wrap-actions.js index 4c4026069e2..8be692b3f74 100644 --- a/src/core/plugins/auth/wrap-actions.js +++ b/src/core/plugins/auth/wrap-actions.js @@ -24,7 +24,11 @@ export const authorize = (oriAction, system) => (payload) => { const isApiKeyInCookie = isApiKeyAuth && isInCookie if (isApiKeyInCookie) { - document.cookie = `${schema.get("name")}=${value}; SameSite=None; Secure` + const secure = `${configs.url?.split("/")[0] === "https:" ? ";secure" : ""}` + const urlBasePath = configs.url?.split("/")[3] + const path = `${urlBasePath === undefined ? ";path=/" : ";path=/".concat(urlBasePath)}` + let cookieStr = `${schema.get("name")}=${value};samesite=None${secure}${path}` + document.cookie = cookieStr } } catch (error) { console.error( @@ -49,7 +53,9 @@ export const logout = (oriAction, system) => (payload) => { if (isApiKeyInCookie) { const cookieName = auth.getIn(["schema", "name"]) - document.cookie = `${cookieName}=; Max-Age=-99999999` + const urlBasePath = configs.url?.split("/")[3] + const path = `${urlBasePath === undefined ? ";path=/" : ";path=/".concat(urlBasePath)}` + document.cookie = `${cookieName}=;max-age=-99999999${path}` } }) } diff --git a/test/unit/core/plugins/auth/wrap-actions.js b/test/unit/core/plugins/auth/wrap-actions.js index cd1327d7be2..ff2e8f6269d 100644 --- a/test/unit/core/plugins/auth/wrap-actions.js +++ b/test/unit/core/plugins/auth/wrap-actions.js @@ -38,7 +38,82 @@ describe("Cookie based apiKey persistence in document.cookie", () => { authorize(jest.fn(), system)(payload) expect(document.cookie).toEqual( - "apiKeyCookie=test; SameSite=None; Secure" + "apiKeyCookie=test;samesite=None;path=/" + ) + }) + + it("should persist secure cookie in document.cookie for non-SSL targets", () => { + const system = { + getConfigs: () => ({ + persistAuthorization: true, + url: "http://example.org" + }), + } + const payload = { + api_key: { + schema: fromJS({ + type: "apiKey", + name: "apiKeyCookie", + in: "cookie", + }), + value: "test", + }, + } + + authorize(jest.fn(), system)(payload) + + expect(document.cookie).toEqual( + "apiKeyCookie=test;samesite=None;path=/" + ) + }) + + it("should persist secure cookie in document.cookie for SSL targets", () => { + const system = { + getConfigs: () => ({ + persistAuthorization: true, + url: "https://example.org" + }), + } + const payload = { + api_key: { + schema: fromJS({ + type: "apiKey", + name: "apiKeyCookie", + in: "cookie", + }), + value: "test", + }, + } + + authorize(jest.fn(), system)(payload) + + expect(document.cookie).toEqual( + "apiKeyCookie=test;samesite=None;secure;path=/" + ) + }) + + it("should persist secure cookie in document.cookie for non-root SSL targets", () => { + const system = { + getConfigs: () => ({ + persistAuthorization: true, + url: "https://example.org/api" + }), + } + const payload = { + api_key: { + schema: fromJS({ + type: "apiKey", + name: "apiKeyCookie", + in: "cookie", + }), + value: "test", + }, + } + + authorize(jest.fn(), system)(payload) + + expect(document.cookie).toEqual( + "apiKeyCookie=test;samesite=None;secure;path=/api" ) }) @@ -64,7 +139,7 @@ describe("Cookie based apiKey persistence in document.cookie", () => { logout(jest.fn(), system)(["api_key"]) - expect(document.cookie).toEqual("apiKeyCookie=; Max-Age=-99999999") + expect(document.cookie).toEqual("apiKeyCookie=;max-age=-99999999;path=/") }) }) From a326f67b5dbc07fac4441e7cc649d217974ace69 Mon Sep 17 00:00:00 2001 From: Sudipto Sarkar Date: Sat, 2 Sep 2023 19:58:33 +0530 Subject: [PATCH 2/2] Fixed Cookie Authentication for Multi level target base Url * Fixed Cookie Path for Multi level target base Url * Fixed Cookie Removal for Multi level target base Url * Added Unit Tests --- src/core/plugins/auth/wrap-actions.js | 4 +- test/unit/core/plugins/auth/wrap-actions.js | 77 +++++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/src/core/plugins/auth/wrap-actions.js b/src/core/plugins/auth/wrap-actions.js index 8be692b3f74..a39775cf0d4 100644 --- a/src/core/plugins/auth/wrap-actions.js +++ b/src/core/plugins/auth/wrap-actions.js @@ -25,7 +25,7 @@ export const authorize = (oriAction, system) => (payload) => { if (isApiKeyInCookie) { const secure = `${configs.url?.split("/")[0] === "https:" ? ";secure" : ""}` - const urlBasePath = configs.url?.split("/")[3] + const urlBasePath = configs.url?.split("/").splice(3).join("/") const path = `${urlBasePath === undefined ? ";path=/" : ";path=/".concat(urlBasePath)}` let cookieStr = `${schema.get("name")}=${value};samesite=None${secure}${path}` document.cookie = cookieStr @@ -53,7 +53,7 @@ export const logout = (oriAction, system) => (payload) => { if (isApiKeyInCookie) { const cookieName = auth.getIn(["schema", "name"]) - const urlBasePath = configs.url?.split("/")[3] + const urlBasePath = configs.url?.split("/").splice(3).join("/") const path = `${urlBasePath === undefined ? ";path=/" : ";path=/".concat(urlBasePath)}` document.cookie = `${cookieName}=;max-age=-99999999${path}` } diff --git a/test/unit/core/plugins/auth/wrap-actions.js b/test/unit/core/plugins/auth/wrap-actions.js index ff2e8f6269d..ed789518495 100644 --- a/test/unit/core/plugins/auth/wrap-actions.js +++ b/test/unit/core/plugins/auth/wrap-actions.js @@ -117,6 +117,31 @@ describe("Cookie based apiKey persistence in document.cookie", () => { ) }) + it("should persist secure cookie in document.cookie for SSL targets with non-root multi-level base path", () => { + const system = { + getConfigs: () => ({ + persistAuthorization: true, + url: "https://example.org/api/production" + }), + } + const payload = { + api_key: { + schema: fromJS({ + type: "apiKey", + name: "apiKeyCookie", + in: "cookie", + }), + value: "test", + }, + } + + authorize(jest.fn(), system)(payload) + + expect(document.cookie).toEqual( + "apiKeyCookie=test;samesite=None;secure;path=/api/production" + ) + }) + it("should delete cookie from document.cookie", () => { const payload = fromJS({ api_key: { @@ -141,6 +166,32 @@ describe("Cookie based apiKey persistence in document.cookie", () => { expect(document.cookie).toEqual("apiKeyCookie=;max-age=-99999999;path=/") }) + + it("should delete cookie from document.cookie for targets with non-root multi-level base path", () => { + const payload = fromJS({ + api_key: { + schema: { + type: "apiKey", + name: "apiKeyCookie", + in: "cookie", + }, + value: "test", + }, + }) + const system = { + getConfigs: () => ({ + persistAuthorization: true, + url: "https://example.org/api/production" + }), + authSelectors: { + authorized: () => payload, + }, + } + + logout(jest.fn(), system)(["api_key"]) + + expect(document.cookie).toEqual("apiKeyCookie=;max-age=-99999999;path=/api/production") + }) }) describe("given persistAuthorization=false", () => { @@ -191,4 +242,30 @@ describe("Cookie based apiKey persistence in document.cookie", () => { expect(document.cookie).toEqual("") }) }) + + it("should delete cookie from document.cookie for targets with non-root multi-level base path", () => { + const payload = fromJS({ + api_key: { + schema: { + type: "apiKey", + name: "apiKeyCookie", + in: "cookie", + }, + value: "test", + }, + }) + const system = { + getConfigs: () => ({ + persistAuthorization: false, + url: "https://example.org/api/production" + }), + authSelectors: { + authorized: () => payload, + }, + } + + logout(jest.fn(), system)(["api_key"]) + + expect(document.cookie).toEqual("") + }) })