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

.getItem from a local/session storage throws when 3rd party cookies/storage is disabled - chrome mobile & other similar behaving browsers #280

Closed
AVVS opened this issue Oct 31, 2017 · 12 comments

Comments

@AVVS
Copy link

AVVS commented Oct 31, 2017

Describe your environment

  • Operating System version: OS X 10.13
  • Firebase SDK version: 4.5.2
  • Firebase Product: auth, database & storage

Describe the problem

Accessing window.localStorage && localStorage.getItem may throw security error based on settings of chrome, code is minified so cant really point to where it happens, but search via .getItem in the auth dist and it will be there

@bojeil-google
Copy link
Contributor

Can you provide exact steps how to recreate this? Right now, I am not sure what you are experiencing.
We currently persist the Auth state in web storage (localStorage). Before we do so, we try and catch setting and getting an item from localStorage.
3rd party cookies/data needs to be enabled for the state to persist, but if they are not enabled, we fallback to in memory state persistence.

@AVVS
Copy link
Author

AVVS commented Oct 31, 2017

This is the best I dug out of the logs, you can see auth service is called, and then it goes on from that. I assume somewhere its not checking properly for localstorage availability - needs to be wrapped in the try/catch even for simple window.localStorage calls - since that throws in certain modes

screen shot 2017-10-31 at 12 50 33 am

@AVVS
Copy link
Author

AVVS commented Oct 31, 2017

Dont have unminified version since it still wasnt open sourced per say - dist was just a minified version, used @firebase/auth package

@AVVS
Copy link
Author

AVVS commented Oct 31, 2017

Another log - might be helpful

screen shot 2017-10-31 at 12 56 41 am

@bojeil-google
Copy link
Contributor

Tried testing it with the Chrome emulator but can't reproduce it as you report it.
Are you running some operation like signInWithPopup or does this trigger on firebase.initializeApp? Are you changing Auth state persistence?
Some snippets would make my life easier as I am still forced to speculate.

@AVVS
Copy link
Author

AVVS commented Oct 31, 2017

I do this in 2 cases:

Initializing redux store:

export const firebaseApp = firebase.initializeApp(firebaseConfig);
const auth = firebaseApp.auth();

// make sure we only persist session and nothing else
auth.setPersistence(firebase.auth.Auth.Persistence.NONE);

...

// enable listener for id token and update http auth header accordingly
auth.onIdTokenChanged((user) => {
  if (!user) return setHttpAuth();
  return user
    .getIdToken(false)
    .catch((e) => {
      window.Raven.captureException(e);
      store.dispatch(showModal());
      return null;
    })
    .then(setHttpAuth);
});

Second case:

export const mintToken = createAction('@@auth0/firebase-mint', () => (dispatch, getState, getFirebase) => {
  if (getState().auth.isFirebaseLoaded === false) return null;

  dispatch(firebaseStartedLoading());

  const token = getState().auth.id_token_payload[FIREBASE_TOKEN_KEY];
  const fb = getFirebase();

  return fb.auth()
    .signInWithCustomToken(token)
    .catch((e) => {
      // Try again.
      if (e.code === 'auth/network-request-failed') {
        return fb.auth().signInWithCustomToken(token);
      }

      throw e;
    })
    .then(currentUser => currentUser.getIdToken())
    .catch((e) => {
      window.Raven.captureException(e, {
        extra: {
          context: 'auth/firebase-mint',
          code: e.code,
        },
      });

      throw e;
    });
});

Likely its somewhere during 1st case, because the second one would've been reported as an error inside the reducer, and it did not

@AVVS
Copy link
Author

AVVS commented Oct 31, 2017

This is similar, but when SecurityError is thrown, ie some1 accesses window.localStorage (simple as that) and I think I know where it is:

https://github.com/firebase/firebase-js-sdk/blob/master/packages/auth/src/storage/localstorage.js#L57-L59 - access to localstorage object, I assume goog.global is alias for window in the browser

https://github.com/firebase/firebase-js-sdk/blob/master/packages/auth/src/storage/localstorage.js#L76 - likely throws

screen shot 2017-10-31 at 11 44 34 am

@bojeil-google
Copy link
Contributor

I am still unable to recreate this. I am also not familiar with the debugger you are using.
When I disable 3rd party cookies in Chrome I only get:
iframe.js:98 Uncaught DOMException: Failed to read the 'localStorage' property from 'Window': Access is denied for this document. which is expected and gets caught in the main code as auth/web-storage-unsupported.
3rd party cookies should only impact the signInWithPopup/Redirect flow as you are embedding a 3rd party iframe. You are the first party app and you are running the code directly on your site for operations like signInWithCustomToken. Anyway, I can add a try/catch there when accessing localStorage. However, it will be a shot in the dark. So I can't guarantee it will fix it. Though it doesn't hurt to have that check.

@bojeil-google bojeil-google self-assigned this Oct 31, 2017
@AVVS
Copy link
Author

AVVS commented Oct 31, 2017

I'm using sentry.io / raven for capturing errors that do propagate up to the window and not get caught.

Stacktrack is on the minified source code because there was no original code to map this to (ie you've just opensourced it a few days ago?)

@bojeil-google
Copy link
Contributor

Yeah, we just open sourced yesterday.

@AVVS
Copy link
Author

AVVS commented Nov 21, 2017

seems to have fixed it!

@AVVS AVVS closed this as completed Nov 21, 2017
@bojeil-google
Copy link
Contributor

Sorry, I forgot to update the bug. Thanks for confirming.

@firebase firebase locked and limited conversation to collaborators Oct 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants