search

aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.4k stars 2.12k forks source link

UniversalStorage overwrites third party localStorage values with cookie values #8608

Closed enigosi closed 8 months ago

enigosi commented 3 years ago

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

Authentication

Amplify Categories

auth

Environment information

``` # Put output below this line System: OS: macOS 11.2.1 CPU: (8) x64 Apple M1 Memory: 46.45 MB / 16.00 GB Shell: 5.8 - /bin/zsh Binaries: Node: 14.17.0 - /usr/local/bin/node Yarn: 1.22.10 - ~/.npm-packages/bin/yarn npm: 7.10.0 - /opt/homebrew/bin/npm Browsers: Chrome: 91.0.4472.164 Safari: 14.0.3 npmPackages: @ampproject/toolbox-optimizer: undefined () @ant-design/icons: 4.2.2 => 4.2.2 (4.6.2) @apollo/client: 3.1.4 => 3.1.4 (3.3.6) @apollo/client/cache: undefined () @apollo/client/core: undefined () @apollo/client/errors: undefined () @apollo/client/link/batch: undefined () @apollo/client/link/batch-http: undefined () @apollo/client/link/context: undefined () @apollo/client/link/core: undefined () @apollo/client/link/error: undefined () @apollo/client/link/http: undefined () @apollo/client/link/persisted-queries: undefined () @apollo/client/link/retry: undefined () @apollo/client/link/schema: undefined () @apollo/client/link/utils: undefined () @apollo/client/link/ws: undefined () @apollo/client/react: undefined () @apollo/client/react/components: undefined () @apollo/client/react/context: undefined () @apollo/client/react/data: undefined () @apollo/client/react/hoc: undefined () @apollo/client/react/hooks: undefined () @apollo/client/react/parser: undefined () @apollo/client/react/ssr: undefined () @apollo/client/testing: undefined () @apollo/client/utilities: undefined () @apollo/react-testing: ^4.0.0 => 4.0.0 @aws-amplify/core: 3.8.21 => 3.8.21 @aws-sdk/url-parser-browser: 3.1.0 => 3.1.0 @aws-sdk/url-parser-node: 3.1.0 => 3.1.0 @babel/core: ^7.12.3 => undefined (7.12.10, 7.12.9, ) @commitlint/cli: ^11.0.0 => 11.0.0 @commitlint/config-conventional: ^11.0.0 => 11.0.0 @contentful/rich-text-react-renderer: ^14.1.2 => 14.1.2 @graphql-codegen/cli: 1.19.4 => 1.19.4 @graphql-codegen/fragment-matcher: 2.0.1 => 2.0.1 @graphql-codegen/introspection: 1.18.1 => 1.18.1 @graphql-codegen/schema-ast: 1.18.1 => 1.18.1 @graphql-codegen/typescript: 1.19.0 => 1.19.0 (1.20.0) @graphql-codegen/typescript-compatibility: 2.0.1 => 2.0.1 @graphql-codegen/typescript-document-nodes: 1.17.9 => 1.17.9 @graphql-codegen/typescript-graphql-files-modules: 1.18.1 => 1.18.1 @graphql-codegen/typescript-operations: 1.17.12 => 1.17.12 @graphql-codegen/typescript-react-apollo: 2.0.6 => 2.0.6 @graphql-tools/mock: 6.2.0 => 6.2.0 @graphql-tools/schema: 7.1.2 => 7.1.2 (6.2.0) @graphql-tools/schema/es5: 7.1.2 @next/bundle-analyzer: 10.1.3 => 10.1.3 @segment/in-eu: 0.3.0 => 0.3.0 @segment/snippet: ^4.13.2 => 4.14.2 @sentry/browser: 6.9.0 => 6.9.0 @sentry/integrations: 6.9.0 => 6.9.0 @sentry/node: 6.9.0 => 6.9.0 @sentry/webpack-plugin: 1.16.0 => 1.16.0 @storybook/addon-essentials: ^6.0.26 => 6.1.14 @storybook/addon-links: ^6.0.26 => 6.1.14 @storybook/preset-ant-design: ^0.0.1 => 0.0.1 @storybook/react: ^6.0.26 => 6.1.14 @stripe/react-stripe-js: ^1.1.2 => 1.1.2 @stripe/stripe-js: ^1.11.0 => 1.11.0 @svgr/webpack: 5.5.0 => 5.5.0 @testing-library/jest-dom: 5.11.6 => 5.11.6 @testing-library/react: 11.2.2 => 11.2.2 @testing-library/react-hooks: ^7.0.0 => 7.0.0 @types/faker: 5.1.5 => 5.1.5 @types/jest: ^26.0.23 => 26.0.23 (24.9.1, 26.0.20) @types/node: 14.6.4 => 14.6.4 (14.14.20, 12.19.13) @types/react: 16.9.49 => 16.9.49 (17.0.9, 17.0.0) @types/react-lottie: ^1.2.5 => 1.2.5 @types/sinon: 9.0.9 => 9.0.9 @types/styled-components: 5.1.3 => 5.1.3 @types/styled-system: 5.1.10 => 5.1.10 @typescript-eslint/eslint-plugin: 4.13.0 => 4.13.0 @typescript-eslint/parser: 4.13.0 => 4.13.0 @venture-allaboutme/consent-manager: 5.4.17 => 5.4.17 amphtml-validator: undefined () antd: 4.15.0 => 4.15.0 arg: undefined () async-retry: undefined () async-sema: undefined () aws-amplify: 3.3.26 => 3.3.26 aws-amplify-react: 4.2.17 => 4.2.17 axios: 0.21.1 => 0.21.1 babel-loader: ^8.1.0 => 8.2.2 babel-plugin-transform-dynamic-import: ^2.1.0 => 2.1.0 bfj: undefined () cacache: undefined () cache-loader: undefined () ci-info: undefined () clsx: 1.1.1 => 1.1.1 comment-json: undefined () compression: undefined () concurrently: 5.3.0 => 5.3.0 conf: undefined () content-type: undefined () cookie: undefined () css-loader: undefined () date-fns: ^2.16.1 => 2.16.1 (1.30.1) date-fns-tz: ^1.0.12 => 1.0.12 debug: undefined () devalue: undefined () dotenv: ^8.2.0 => 8.2.0 (9.0.2, 6.2.0) escape-string-regexp: undefined () eslint: 7.0.0 => 7.0.0 eslint-config-prettier: 6.11.0 => 6.11.0 eslint-plugin-import: 2.20.2 => 2.20.2 eslint-plugin-prettier: 3.1.3 => 3.1.3 eslint-plugin-react: 7.20.0 => 7.20.0 faker: 5.2.0 => 5.2.0 file-loader: undefined () find-cache-dir: undefined () find-up: undefined () framer-motion: ^3.3.0 => 3.3.0 fresh: undefined () fs: ^0.0.1-security => 0.0.1-security graphql: 15.3.0 => 15.3.0 (14.0.0) graphql-tag: 2.12.3 => 2.12.3 (2.11.0) gzip-size: undefined () http-proxy: undefined () husky: 4.2.5 => 4.2.5 i18next: 19.4.4 => 19.4.4 i18next-browser-languagedetector: 6.0.1 => 6.0.1 identity-obj-proxy: ^3.0.0 => 3.0.0 ignore-loader: undefined () is-animated: undefined () is-docker: undefined () is-wsl: undefined () jest: 26.4.2 => 26.4.2 jest-canvas-mock: ^2.3.1 => 2.3.1 jest-environment-jsdom: ^26.6.2 => 26.6.2 jest-next-dynamic: ^1.0.1 => 1.0.1 jest-styled-components: ^7.0.3 => 7.0.3 jest-transform-stub: 2.0.0 => 2.0.0 json5: undefined () jsonwebtoken: undefined () less: 3.12.2 => 3.12.2 (3.13.1) lint-staged: 10.3.0 => 10.3.0 loader-utils: undefined () lodash.curry: undefined () lottie-react: ^2.1.0 => 2.1.0 lru-cache: undefined () memo-parser: 0.2.1 nanoid: undefined () neo-async: undefined () next: 10.1.3 => 10.1.3 next-compose-plugins: 2.2.0 => 2.2.0 next-secure-headers: 2.2.0 => 2.2.0 next-transpile-modules: 6.4.0 => 6.4.0 ora: undefined () postcss-flexbugs-fixes: undefined () postcss-loader: undefined () postcss-preset-env: 6.7.0 => 6.7.0 () postcss-pxtorem: 5.1.1 => 5.1.1 postcss-scss: undefined () prettier: 2.1.1 => 2.1.1 (2.0.5) react: 17.0.2 => 17.0.2 react-cookie: 4.0.3 => 4.0.3 react-dom: 17.0.2 => 17.0.2 react-i18next: 11.7.2 => 11.7.2 react-is: ^17.0.0 => 17.0.1 (16.13.1, 17.0.2, 16.10.2) react-player: ^2.6.2 => 2.7.2 react-test-renderer: 17.0.2 => 17.0.2 react-zendesk: 0.1.11 => 0.1.11 recast: undefined () recharts: 2.0.9 => 2.0.9 resolve-url-loader: undefined () sass-loader: undefined () schema-utils: undefined () semver: undefined () send: undefined () sinon: ^9.2.1 => 9.2.3 (7.5.0) source-map: undefined () storybook-addon-designs: ^5.4.2 => 5.4.2 string-hash: undefined () strip-ansi: undefined () stripe: ^8.128.0 => 8.130.0 style-loader: ^2.0.0 => 2.0.0 (1.3.0) styled-components: 5.2.3 => 5.2.3 styled-components/macro: undefined () styled-components/native: undefined () styled-components/primitives: undefined () styled-system: 5.1.5 => 5.1.5 terser: undefined () terser-webpack-plugin: 4.1.0 => 4.1.0 (3.1.0, 1.4.5) text-table: undefined () thread-loader: undefined () ts-jest: 26.3.0 => 26.3.0 typescript: ^4.0.3 => 4.1.3 unistore: undefined () web-vitals: undefined () webpack: undefined () webpack-sources: undefined () npmGlobalPackages: serverless: 2.32.1 yarn: 1.22.10 ```

Describe the bug

UniversalStorage merges all values extracted from cookie to localStorage which overwrites values existing in both.

https://github.com/aws-amplify/amplify-js/blob/main/packages/core/src/UniversalStorage/index.ts on line 13 it first assigns (by reference) window.localStorage to this.store and then on line 20 it's merging cookies to it Object.assign(this.store, this.cookies.getAll()); which is causing all values from cookies to be saved in LS

In my case, it's causing a bug because it overwrites Segment's ajs_user_id in localStorage (stringified user id) with Segment's ajs_user_id from cookie (not stringified user id).

Expected behavior

UniversalStorage shouldn't modify third-party localStorage values and explicitly set only ones used by Amplify.

Reproduction steps

  1. add value to cookie
  2. add different value to localStorage with same key as a cookie
  3. init amplify
  4. LS value is now equal to value from cookie

Code Snippet


  setCookie('foo', 'bar')
  window.localStorage.setItem('foo', 'biz')
  debugger // cookie.foo === 'bar' localStorage.foo = 'biz'
  const SSR = withSSRContext() 
  debugger // cookie.foo === 'bar' localStorage.foo = 'bar'

Log output

No response

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

chrisbonifacio commented 2 years ago

@enigosi Apologies for the delay. Would you be willing to create a PR to address this issue? We'd be happy to include it in a release in the near future.

robbieaverill commented 1 year ago

I've run into this issue and found this thread - it seems a little over-zealous to me that Amplify stores all cookies in local storage. Perhaps Amplify could whitelist the cookies it needs and store those in local storage, or prefix the local storage keys it writes with something to identify them. In my case, Amplify is reading JSON encoded cookies and writing them to local storage as [object Object].

cwomack commented 10 months ago

Hello, @enigosi and @robbieaverill 👋. Just wanted to follow up on this issue to say that we are working on an improvements to SSR for the next major version of Amplify that should resolve this issue. We will circle back on this and provide an update soon!

cwomack commented 9 months ago

The developer preview for v6 of Amplify has officially been released with SSR improvements and much more! Please check out our announcement and updated documentation to see what has changed.

This issue should be resolved within the dev preview and upcoming General Availability for Amplify v6, but let us know with a comment if there are further issues.

cwomack commented 8 months ago

With the release of the latest major version of Amplify (aws-amplify@>6), this issue should now be resolved! Please refer to our release announcement, migration guide, and documentation for more information.