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

Prevent getTokenSilently to be called concurrently #125

Closed
wants to merge 9 commits into from

Conversation

luisrudge
Copy link
Contributor

@luisrudge luisrudge commented Jul 31, 2019

fix #109

Description

Today, if you call getTokenSilently in parallel, there's no guarantee that the message we receive from the iframe will be received by the correct iframe handler, which causes issues like #109. To fix this behavior and prepare our SDK for rotation of Refresh Token, we need to explicitly allow only a single call at a time.

Testing

  • This change adds test coverage for new/changed/fixed functionality

@luisrudge luisrudge requested a review from a team July 31, 2019 01:24
static/index.html Outdated Show resolved Hide resolved
src/utils.ts Show resolved Hide resolved
const IFRAME_ID = 'a0-spajs-iframe';
if (document.getElementById(IFRAME_ID)) {
throw new InternalError(
"`getTokenSilently` can only be called once at a time. Check your code to make sure you're not calling it multiple times."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when in this function's code are you calling twice getTokenSilently? if the answer is "nowhere" I think this error should be different. Even the tests mention "throws an error when the iframe already exists". Then why is this error talking about the specific use case of calling getTokenSilently multiple times? That's out of this method's scope IMO. Maybe the caller should be checking that and re-throwing a similar error up.

src/utils.ts Show resolved Hide resolved
@nojaf
Copy link

nojaf commented Aug 28, 2019

Hey @luisrudge, what still needs to be done here?
I'm having this issue and am looking forward to this fix.
Can I assist somehow?

@luisrudge
Copy link
Contributor Author

@nojaf we're having internal conversations about this. I'll ping this thread once we release this

@luisrudge
Copy link
Contributor Author

After reevaluating this issue and our goals, it became clear that we need to support multiple calls of this method in parallel (even though the requests won't be able to run in parallel). We'll discuss another approach to internally serialize those requests. Thanks for the patience.

@ipzKellyR
Copy link

Any progress on this?

@iamchathu
Copy link

@luisrudge Any update on this?

@stevehobbsdev
Copy link
Contributor

@ipzKellyR @iamchathu This feature was released in v1.4.0, see also #238

Add 'lock' to prevent getTokenSilently to be invoked in parallel

@DaleGardner
Copy link

DaleGardner commented Nov 26, 2019

@stevehobbsdev Thanks! But in your release notes, it isn't clear what super-tokens are. And the link you put does not work: https://github.com/super-tokens
https://github.com/auth0/auth0-spa-js/releases/tag/v1.6.0

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

Successfully merging this pull request may close these issues.

Unable to call getTokenSilently concurrently
8 participants