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

localStorage Safari Security Error when users have "Block Cookies" setting enabled #232

Open
zachequi opened this issue Sep 8, 2023 · 5 comments

Comments

@zachequi
Copy link

zachequi commented Sep 8, 2023

Hi,

Safari does this funny thing where if you try and use localStorage when the user has enabled the "Block Cookies" setting the browser throws a Security Error, which bubbles up and can crash the parent application. We've seen 4 or 5 of these in production in the past 2 weeks, so its rare but does happen.

Some relevant links with background on this:

https://stackoverflow.com/questions/46632093/getting-security-error-on-iphone-when-using-localstorage

https://michalzalecki.com/why-using-localStorage-directly-is-a-bad-idea/#:~:text=SecurityError%3A%20The%20operation%20is%20insecure,localStorage%20object%20causes%20the%20error

https://hackernoon.com/why-localstorage-still-crashes-your-website-in-2023


I've also seen errors related to localStorage in browser-tabs-lock from Chrome Mobile Webviews. Looks like localStorage is null somehow on that platform.

image

@nkshah2
Copy link
Contributor

nkshah2 commented Sep 12, 2023

Hi @zachequi

We will look into the Safari issue, thanks. About the issue in chrome webviews, is this a webview launched inside of your mobile app?

@nkshah2
Copy link
Contributor

nkshah2 commented Sep 14, 2023

Hi @zachequi

We looked into the issue with Safari and the best way to get around this would be to provide a windowHandler when initialising SuperTokens.

windowHandler: (original) => {
        return {
            ...original,
            localStorage: {
                clear: () => {},
                clearSync: () => {},
                // TODO: add remaining functions
            },
        };
    },

For example if you are using store2 you could provide your own implementations for all the localStorage functions to use that instead of using the browser's localStorage.

You can find the full type of the object that the window handler expects here: https://github.com/supertokens/supertokens-website/blob/master/lib/ts/utils/windowHandler/types.ts.

Also about the issue in Android mobile webviews where localstorage is null, you may need to specifically enable storage in the webview when you launch it to make it available on Android. You can check here for how to do this: https://stackoverflow.com/questions/5899087/android-webview-localstorage/5934650#5934650

Closing this issue for now but feel free to re-open if you need more help

@nkshah2 nkshah2 closed this as completed Sep 14, 2023
@zachequi
Copy link
Author

zachequi commented Sep 14, 2023 via email

@nkshah2
Copy link
Contributor

nkshah2 commented Sep 15, 2023

Hey @zachequi

While we agree this is not the best solution to the problem and we have added this to our pipeline we just cant promise when we will pick this up as a priority because so far this is the first complaint we have had about this. If we get more complaints about this we will move this up in terms of priority, in the meantime leaving this open so others can also see it. The suggestion for overriding it was just so that you can move past this issue in the meantime.

@rishabhpoddar rishabhpoddar reopened this Sep 15, 2023
@zachequi
Copy link
Author

zachequi commented Oct 6, 2023

Posting for the benefit of anyone who may find this down the road --- we've implemented store2 as the window handler as below. This mostly works, however we believe that there is an interaction between browser-tabs-lock and store2 that occasionally causes the whole tab to lock up. Still working through a solution to that.

 windowHandler: (original) => {
      return {
        ...original,
        // Override localstorage usage within supertokens to use store2 so we don't have
        // localStorage edgecase crashes.  https://github.com/supertokens/supertokens-website/issues/232#issuecomment-1719213092
        localStorage: {
          clear: () => (store2.clear() ? Promise.resolve() : Promise.resolve()),
          clearSync: store2.clear.bind(store2),

          key: (index) => Promise.resolve(store2.keys()[index]),
          keySync: (index) => store2.keys()[index],

          setItem: (key, value) => Promise.resolve(store2.set(key, value)),
          setItemSync: store2.set.bind(store2),

          getItem: (key) => Promise.resolve(store2.get(key)),
          getItemSync: store2.get.bind(store2),

          removeItem: (key) => Promise.resolve(store2.remove(key)),
          removeItemSync: store2.remove.bind(store2),
        },
      };

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants