From 60a559f612e092c76e1499f93547248312db46f2 Mon Sep 17 00:00:00 2001 From: Hui Zhao <10602282+HuiSF@users.noreply.github.com> Date: Fri, 19 Jul 2024 16:36:55 -0700 Subject: [PATCH] fix(aws-amplify): createKeyValueStorageFromCookieStorageAdapter misses default path and secure values (#13508) * fix(aws-amplify): createKeyValueStorageFromCookieStorageAdapter misses default path and secure values * Ensure Path is being serialized * Delete cookie without path attr before setting it --- ...kieStorageAdapterFromNextServerContext.test.ts | 5 +++-- ...teCookieStorageAdapterFromNextServerContext.ts | 5 ++++- ...eyValueStorageFromCookieStorageAdapter.test.ts | 15 +++++++++++++++ ...eateKeyValueStorageFromCookieStorageAdapter.ts | 7 +++++++ .../serverContext/types/cookieStorage.ts | 8 +++++++- 5 files changed, 36 insertions(+), 4 deletions(-) diff --git a/packages/adapter-nextjs/__tests__/utils/createCookieStorageAdapterFromNextServerContext.test.ts b/packages/adapter-nextjs/__tests__/utils/createCookieStorageAdapterFromNextServerContext.test.ts index 858a8ad4b62..e27b4243511 100644 --- a/packages/adapter-nextjs/__tests__/utils/createCookieStorageAdapterFromNextServerContext.test.ts +++ b/packages/adapter-nextjs/__tests__/utils/createCookieStorageAdapterFromNextServerContext.test.ts @@ -147,6 +147,7 @@ describe('createCookieStorageAdapterFromNextServerContext', () => { sameSite: 'strict' as any, httpOnly: true, secure: true, + path: '/a-path', }; const result = createCookieStorageAdapterFromNextServerContext(mockContext); @@ -176,7 +177,7 @@ describe('createCookieStorageAdapterFromNextServerContext', () => { mockSerializeOptions.domain };Expires=${mockSerializeOptions.expires.toUTCString()};HttpOnly;SameSite=${ mockSerializeOptions.sameSite - };Secure`, + };Secure;Path=${mockSerializeOptions.path}`, ); }); @@ -188,7 +189,7 @@ describe('createCookieStorageAdapterFromNextServerContext', () => { mockSerializeOptions.domain };Expires=${mockSerializeOptions.expires.toUTCString()};HttpOnly;SameSite=${ mockSerializeOptions.sameSite - };Secure`, + };Secure;Path=${mockSerializeOptions.path}`, ); }); diff --git a/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts b/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts index 564259510e1..843235b7288 100644 --- a/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts +++ b/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts @@ -218,7 +218,7 @@ const createMutableCookieStoreFromHeaders = ( const serializeSetCookieOptions = ( options: CookieStorage.SetCookieOptions, ): string => { - const { expires, domain, httpOnly, sameSite, secure } = options; + const { expires, domain, httpOnly, sameSite, secure, path } = options; const serializedOptions: string[] = []; if (domain) { serializedOptions.push(`Domain=${domain}`); @@ -235,6 +235,9 @@ const serializeSetCookieOptions = ( if (secure) { serializedOptions.push(`Secure`); } + if (path) { + serializedOptions.push(`Path=${path}`); + } return serializedOptions.join(';'); }; diff --git a/packages/aws-amplify/__tests__/adapterCore/storageFactories/createKeyValueStorageFromCookieStorageAdapter.test.ts b/packages/aws-amplify/__tests__/adapterCore/storageFactories/createKeyValueStorageFromCookieStorageAdapter.test.ts index eae5ffe22a8..f2b900bfa87 100644 --- a/packages/aws-amplify/__tests__/adapterCore/storageFactories/createKeyValueStorageFromCookieStorageAdapter.test.ts +++ b/packages/aws-amplify/__tests__/adapterCore/storageFactories/createKeyValueStorageFromCookieStorageAdapter.test.ts @@ -40,6 +40,21 @@ describe('keyValueStorage', () => { ); }); + it('should remove item before setting item', async () => { + const testKey = 'testKey'; + const testValue = 'testValue'; + keyValueStorage.setItem(testKey, testValue); + expect(mockCookiesStorageAdapter.delete).toHaveBeenCalledWith(testKey); + expect(mockCookiesStorageAdapter.set).toHaveBeenCalledWith( + testKey, + testValue, + { + ...defaultSetCookieOptions, + expires: expect.any(Date), + }, + ); + }); + it('should set item with options', async () => { const testKey = 'testKey'; const testValue = 'testValue'; diff --git a/packages/aws-amplify/src/adapter-core/storageFactories/createKeyValueStorageFromCookieStorageAdapter.ts b/packages/aws-amplify/src/adapter-core/storageFactories/createKeyValueStorageFromCookieStorageAdapter.ts index 9cfd141c47c..dc6bd80fccf 100644 --- a/packages/aws-amplify/src/adapter-core/storageFactories/createKeyValueStorageFromCookieStorageAdapter.ts +++ b/packages/aws-amplify/src/adapter-core/storageFactories/createKeyValueStorageFromCookieStorageAdapter.ts @@ -10,6 +10,8 @@ import { export const defaultSetCookieOptions: CookieStorage.SetCookieOptions = { // TODO: allow configure with a public interface sameSite: 'lax', + secure: true, + path: '/', }; const ONE_YEAR_IN_MS = 365 * 24 * 60 * 60 * 1000; @@ -25,6 +27,11 @@ export const createKeyValueStorageFromCookieStorageAdapter = ( ): KeyValueStorageInterface => { return { setItem(key, value) { + // Delete the cookie item first then set it. This results: + // SetCookie: key=;expires=1970-01-01;(path='current-path') <- remove path'ed cookies + // SetCookie: key=value;expires=Date.now() + 365 days;path=/;secure=true + cookieStorageAdapter.delete(key); + // TODO(HuiSF): follow up the default CookieSerializeOptions values cookieStorageAdapter.set(key, value, { ...defaultSetCookieOptions, diff --git a/packages/core/src/adapterCore/serverContext/types/cookieStorage.ts b/packages/core/src/adapterCore/serverContext/types/cookieStorage.ts index 6069c749903..e93124d41d0 100644 --- a/packages/core/src/adapterCore/serverContext/types/cookieStorage.ts +++ b/packages/core/src/adapterCore/serverContext/types/cookieStorage.ts @@ -7,7 +7,13 @@ export declare namespace CookieStorage { export type SetCookieOptions = Partial< Pick< CookieSerializeOptions, - 'domain' | 'expires' | 'httpOnly' | 'maxAge' | 'sameSite' | 'secure' + | 'domain' + | 'expires' + | 'httpOnly' + | 'maxAge' + | 'sameSite' + | 'secure' + | 'path' > >;