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

Bug: prevent concurrent access token refresh attempts #29

Open
chasecaleb opened this issue Nov 3, 2021 · 2 comments
Open

Bug: prevent concurrent access token refresh attempts #29

chasecaleb opened this issue Nov 3, 2021 · 2 comments

Comments

@chasecaleb
Copy link

Making multiple fetch calls using OAuth2AuthCodePKCE.decorateFetchHTTPClient(fetch) while the access token is expired will result in multiple token refresh calls. Unfortunately this causes issues such as invalid_grant error responses and potentially even rate limiting. For example, gitlab.com rate limits this particular API call to 10 requests/minute.

Here's how I worked around the issue, but it would be ideal to implement this in your library instead:

export const createGitlabOAuth = () => {
  let expiryPromise;
  let invalidGrantPromise;
  return new OAuth2AuthCodePKCE({
    authorizationUrl: 'https://gitlab.com/oauth/authorize',
    tokenUrl: 'https://gitlab.com/oauth/token',
    clientId: process.env.REACT_APP_GITLAB_CLIENT_ID,
    redirectUrl: window.location.origin,
    scopes: ['api'],
    extraAuthorizationParams: {
      clientSecret: process.env.REACT_APP_GITLAB_SECRET,
    },
    onAccessTokenExpiry: async (refreshToken) => {
      if (!expiryPromise) {
        expiryPromise = refreshToken();
      }
      const result = await expiryPromise;
      expiryPromise = undefined;
      return result;
    },
    onInvalidGrant: async (refreshAuthCodeOrToken) => {
      if (!invalidGrantPromise) {
        invalidGrantPromise = refreshAuthCodeOrToken();
      }
      // This is a void promise, so don't need to return the result. Refer to the TypeScript source
      // of OAuth2AuthCodePKCE. Types are great.
      await invalidGrantPromise;
      invalidGrantPromise = undefined;
    },
  });
};

Note: the onAccessTokenExpiry workaround was the important part for the specific scenario I encountered, but I think it makes sense to do in onInvalidGrant too.

chasecaleb added a commit to chasecaleb/organice that referenced this issue Nov 3, 2021
Prevent multiple concurrent token refresh calls if multiple GitLab API
calls are triggered while token is expired.

For more info, see issue I submitted here:
BitySA/oauth2-auth-code-pkce#29
chasecaleb added a commit to chasecaleb/organice that referenced this issue Nov 3, 2021
Prevent multiple concurrent token refresh calls if multiple GitLab API
calls are triggered while token is expired.

For more info, see issue I submitted here:
BitySA/oauth2-auth-code-pkce#29
chasecaleb added a commit to chasecaleb/organice that referenced this issue Nov 3, 2021
Prevent multiple concurrent token refresh calls if multiple GitLab API
calls are triggered while token is expired.

For more info, see issue I submitted here:
BitySA/oauth2-auth-code-pkce#29

Fixes 200ok-ch#739.
@chasecaleb
Copy link
Author

For additional context, check out the linked PR in organice: 200ok-ch/organice#740

@lf94
Copy link

lf94 commented Feb 14, 2022

Thank you again for your explanation and suggested fix! I've just been really busy the past months. Bity development team is practically 2 people for the past year, and yeah, I'm leaving now, so this may not see any updates for awhile. I know personally if I ever need an OAuth2 solution though, I'll be using this one I've created 🙂

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

2 participants