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

Client side storage access is not wrapped in try catch block #13747

Closed
3 tasks done
sNishant011 opened this issue Aug 23, 2024 · 6 comments · Fixed by #13938
Closed
3 tasks done

Client side storage access is not wrapped in try catch block #13747

sNishant011 opened this issue Aug 23, 2024 · 6 comments · Fixed by #13938
Labels
bug Something isn't working Core Related to core Amplify issues

Comments

@sNishant011
Copy link

Before opening, please confirm:

JavaScript Framework

Next.js

Amplify APIs

Authentication

Amplify Version

v6

Amplify Categories

auth

Backend

None

Environment information

System:
OS: macOS 14.6.1
CPU: (8) arm64 Apple M1
Memory: 71.41 MB / 8.00 GB
Shell: 5.9 - /bin/zsh
Binaries:
Node: 20.16.0 - ~/.nvm/versions/node/v20.16.0/bin/node
npm: 10.8.1 - ~/.nvm/versions/node/v20.16.0/bin/npm
pnpm: 9.1.4 - ~/Library/pnpm/pnpm
Watchman: 2024.08.05.00 - /opt/homebrew/bin/watchman
Browsers:
Chrome: 128.0.6613.84
Safari: 17.6
npmPackages:
@aws-amplify/adapter-nextjs: 1.0.13 => 1.0.13
@benjc/rehype-enhanced-tables: ^0.1.1 => 0.1.1
@customerio/cdp-analytics-browser: ^0.1.3 => 0.1.3
@formkit/auto-animate: ^0.8.1 => 0.8.1
@gsap/react: ^2.1.1 => 2.1.1
@here/flexpolyline: ^0.1.0 => 0.1.0
@hookform/devtools: ^4.3.1 => 4.3.1
@hookform/resolvers: ^3.3.4 => 3.3.4
@microsoft/fetch-event-source: ^2.0.1 => 2.0.1
@next/third-parties: ^14.2.1 => 14.2.1
@notionhq/client: ^2.2.15 => 2.2.15
@radix-ui/react-accordion: ^1.1.2 => 1.1.2
@radix-ui/react-alert-dialog: ^1.0.5 => 1.0.5
@radix-ui/react-aspect-ratio: ^1.0.3 => 1.0.3
@radix-ui/react-avatar: ^1.0.4 => 1.0.4
@radix-ui/react-checkbox: ^1.0.4 => 1.0.4
@radix-ui/react-collapsible: ^1.0.3 => 1.0.3
@radix-ui/react-context-menu: ^2.1.5 => 2.1.5
@radix-ui/react-dialog: ^1.0.5 => 1.0.5
@radix-ui/react-dropdown-menu: ^2.0.6 => 2.0.6
@radix-ui/react-hover-card: ^1.0.7 => 1.0.7
@radix-ui/react-label: ^2.0.2 => 2.0.2
@radix-ui/react-menubar: ^1.0.4 => 1.0.4
@radix-ui/react-navigation-menu: ^1.1.4 => 1.1.4
@radix-ui/react-popover: ^1.0.7 => 1.0.7
@radix-ui/react-progress: ^1.0.3 => 1.0.3
@radix-ui/react-radio-group: ^1.1.3 => 1.1.3
@radix-ui/react-scroll-area: ^1.0.5 => 1.0.5
@radix-ui/react-select: ^2.0.0 => 2.0.0
@radix-ui/react-separator: ^1.0.3 => 1.0.3
@radix-ui/react-slider: ^1.1.2 => 1.1.2
@radix-ui/react-slot: ^1.0.2 => 1.0.2
@radix-ui/react-switch: ^1.0.3 => 1.0.3
@radix-ui/react-tabs: ^1.0.4 => 1.0.4
@radix-ui/react-toast: ^1.1.5 => 1.1.5
@radix-ui/react-toggle: ^1.0.3 => 1.0.3
@radix-ui/react-toggle-group: ^1.0.4 => 1.0.4
@radix-ui/react-tooltip: ^1.0.7 => 1.0.7
@react-google-maps/api: ^2.19.3 => 2.19.3
@sentry/nextjs: ^7.110.1 => 7.110.1
@tailwindcss/container-queries: ^0.1.1 => 0.1.1
@tailwindcss/typography: ^0.5.10 => 0.5.10
@tanstack/react-query: ^5.22.2 => 5.22.2
@tanstack/react-query-devtools: ^5.24.0 => 5.24.0
@types/aos: ^3.0.7 => 3.0.7
@types/dom-speech-recognition: ^0.0.4 => 0.0.4
@types/google.maps: ^3.55.3 => 3.55.3
@types/jsonwebtoken: ^9.0.6 => 9.0.6
@types/lodash: ^4.14.202 => 4.14.202
@types/node: ^20.11.20 => 20.11.20
@types/react: ^18.2.57 => 18.2.57
@types/react-dom: ^18.2.19 => 18.2.19
@types/uuid: ^9.0.8 => 9.0.8
@typescript-eslint/eslint-plugin: ^6.21.0 => 6.21.0
@vercel/edge: ^1.1.1 => 1.1.1
aos: ^2.3.4 => 2.3.4
autoprefixer: ^10.4.17 => 10.4.17
aws-amplify: ^6.0.17 => 6.0.17
axios: ^1.6.7 => 1.6.7
class-variance-authority: ^0.7.0 => 0.7.0
clsx: ^2.1.0 => 2.1.0
cmdk: ^0.2.1 => 0.2.1
date-fns: ^3.3.1 => 3.3.1
dayjs: ^1.11.10 => 1.11.10
embla-carousel: 8.0.0-rc23 => 8.0.0-rc23
embla-carousel-react: ^8.0.0 => 8.0.0
embla-carousel-wheel-gestures: 8.0.0-rc05 => 8.0.0-rc05
eslint: ^8.56.0 => 8.56.0
eslint-config-next: 14.1.0 => 14.1.0
eslint-config-prettier: ^9.1.0 => 9.1.0
eslint-plugin-prettier: ^5.1.3 => 5.1.3
eslint-plugin-simple-import-sort: ^10.0.0 => 10.0.0
eslint-plugin-tailwindcss: ^3.14.3 => 3.14.3
eslint-plugin-unused-imports: ^4.0.0 => 4.0.0
flagsmith: ^4.0.0 => 4.0.0
framer-motion: ^11.0.5 => 11.0.5
gsap: ^3.12.5 => 3.12.5
husky: ^9.0.11 => 9.0.11
input-otp: ^1.2.4 => 1.2.4
jsonwebtoken: ^9.0.2 => 9.0.2
lint-staged: ^15.2.2 => 15.2.2
lodash: ^4.17.21 => 4.17.21
lottie-react: ^2.4.0 => 2.4.0
lucide-react: ^0.319.0 => 0.319.0
mixpanel-browser: ^2.49.0 => 2.49.0
next: 14.2.3 => 14.2.3
next-nprogress-bar: ^2.1.2 => 2.1.2
next-themes: ^0.2.1 => 0.2.1
postcss: ^8.4.35 => 8.4.35
prettier: ^3.2.5 => 3.2.5
react: ^18.2.0 => 18.2.0
react-confetti: ^6.1.0 => 6.1.0
react-cookie-consent: ^9.0.0 => 9.0.0
react-day-picker: ^8.10.0 => 8.10.0
react-dom: ^18.2.0 => 18.2.0
react-error-boundary: ^4.0.12 => 4.0.12
react-fast-marquee: ^1.6.5 => 1.6.5
react-hook-form: ^7.50.1 => 7.50.1
react-icons: ^5.3.0 => 5.3.0
react-markdown: ^9.0.1 => 9.0.1
react-pdf: ^7.7.1 => 7.7.1
react-phone-number-input: ^3.4.1 => 3.4.1
react-remark: ^2.1.0 => 2.1.0
react-resizable-panels: ^2.0.13 => 2.0.13
react-share: ^5.1.0 => 5.1.0
react-textarea-autosize: ^8.5.3 => 8.5.3
react-virtualized-auto-sizer: ^1.0.24 => 1.0.24
recharts: ^2.12.1 => 2.12.1
remark-gfm: ^4.0.0 => 4.0.0
sharp: ^0.33.4 => 0.33.4
sonner: ^1.4.0 => 1.4.0
tailwind-merge: ^2.2.1 => 2.2.1
tailwindcss: ^3.4.1 => 3.4.1
tailwindcss-animate: ^1.0.7 => 1.0.7
typescript: ^5.3.3 => 5.3.3
uuid: ^9.0.1 => 9.0.1
vaul: ^0.8.9 => 0.8.9
zod: ^3.22.4 => 3.22.4
npmGlobalPackages:
corepack: 0.28.2
npm: 10.8.1
pnpm: 9.7.0

Describe the bug

getLocalStorageWithfallback and getSessionStorageWithfallback in this file are accessing the required storage but not wrapped in try catch block. Accessing those storage can throw Security Expections as mentioned in this doc.
image
This is causing our website to crash when localstorage is not accessible as seen in the sentry log below:
image

Expected behavior

Those code blocks should have been wrapped inside a try catch block and if we do not have localStorage access we can return new InMemoryStorage as a fallback as follows:

export const getLocalStorageWithFallback = (): Storage => {
  try {
    // Attempt to use localStorage directly
    if (typeof window !== 'undefined' && window.localStorage) {
      // Test if localStorage is accessible
      return window.localStorage;
    }
  } catch (e) {
    // Handle any errors related to localStorage access
    console.error('LocalStorage access failed:', e);
  }

  // Return in-memory storage as a fallback if localStorage is not accessible
  return new InMemoryStorage();
};

Similar approach can be taken for sessionStorage as well.
This will prevent crashing of website/web apps using amplify packages which depend on these functions.

Reproduction steps

The actual issue for me is related with authentication. To reproduce how the website crashes due to disabled localStorage:

  1. Disable site data storage in chrome or disable cookies in safari
  2. Visit amplify docs
image image

Code Snippet

No response

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

@sNishant011 sNishant011 added the pending-triage Issue is pending triage label Aug 23, 2024
@sNishant011
Copy link
Author

There was a related open issue #10396 not sure why it was closed without proper solution.

@cwomack cwomack added the Auth Related to Auth components/category label Aug 23, 2024
@cwomack cwomack self-assigned this Aug 23, 2024
@cwomack
Copy link
Member

cwomack commented Aug 26, 2024

Hello, @sNishant011 👋 and thanks for opening this issue. Very easily reproducible (on Firefox as well as the browsers you mentioned), and will review this with our team. Marking this as a bug at this point and we'll follow up with progress or further questions.

@cwomack cwomack added bug Something isn't working and removed pending-triage Issue is pending triage labels Aug 26, 2024
@cwomack cwomack removed their assignment Aug 26, 2024
@cwomack cwomack added Core Related to core Amplify issues and removed Auth Related to Auth components/category labels Aug 26, 2024
@sNishant011
Copy link
Author

Thank you for taking this into consideration. Hoping to get this resolved soon. 🎉

@cwomack
Copy link
Member

cwomack commented Sep 3, 2024

Absolutely. Thanks for catching this and we'll keep you updated.

@github-actions github-actions bot added the pending-maintainer-response Issue is pending a response from the Amplify team. label Sep 3, 2024
@cwomack cwomack removed the pending-maintainer-response Issue is pending a response from the Amplify team. label Sep 3, 2024
@jgo80
Copy link

jgo80 commented Nov 13, 2024

The "Fix" #13938 pops up nasty in my React Native application, how to handle it there?

CoreStorageUtils - SessionStorage access failed: [Error: sessionStorage is not defined]

I understand why the error appears as there is no window property in React Native.

Trying to implement a custom storage according to the documentation does not work, even if the custom storage is implemented before initializing with Amplify.configure

Thanks for any hint!

This is my implementation:

import { LibraryOptions } from '@aws-amplify/core';
import { Amplify } from 'aws-amplify';
import { fetchAuthSession } from 'aws-amplify/auth';
import { cognitoUserPoolsTokenProvider } from 'aws-amplify/auth/cognito';
import { KeyValueStorageInterface } from 'aws-amplify/utils';

class MyCustomStorage implements KeyValueStorageInterface {
  storageObject: Record<string, string> = {};
  async setItem(key: string, value: string): Promise<void> {
    this.storageObject[key] = value;
  }
  async getItem(key: string): Promise<string | null> {
    return this.storageObject[key];
  }
  async removeItem(key: string): Promise<void> {
    delete this.storageObject[key];
  }
  async clear(): Promise<void> {
    this.storageObject = {};
  }
}

cognitoUserPoolsTokenProvider.setKeyValueStorage(new MyCustomStorage());

const libraryOptions: LibraryOptions = {
  API: {
    GraphQL: {
      headers: async () => {
        const Authorization = (
          await fetchAuthSession()
        ).tokens?.idToken?.toString();
        if (Authorization) {
          return {
            Authorization,
          };
        } else {
          return {};
        }
      },
    },
  },
};

const init = (resourceConfig: any) => {
  Amplify.configure(resourceConfig, libraryOptions);
};

export const App = { init };

EDIT: I realized that a SessionStorage is instanciated in any case in node_modules/@aws-amplify/core/src/storage/index.ts with:

export const sessionStorage = new SessionStorage();

So there is no chance to get rid of this error in React Native...

@github-actions github-actions bot added the pending-maintainer-response Issue is pending a response from the Amplify team. label Nov 13, 2024
@yuhengshs
Copy link
Contributor

Hi @jgo80 ,
new version of aws-amplify v6.8.2 is released. This should solve the issue you mentioned. Please upgrade to the latest version and let us know if there's any other issues.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending a response from the Amplify team. label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Core Related to core Amplify issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants