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

Add client secret to refreshToken and remove ID token validation #433

Merged
merged 2 commits into from
Dec 6, 2019

Conversation

joshcanhelp
Copy link
Contributor

Changes

  • Add existing client secret from options
  • Remove ID token validation (refresh tokens can get access tokens without ID tokens)
  • Removed references to signing in

References

#432

Testing

  • This change adds test coverage

@joshcanhelp joshcanhelp requested review from stevehobbsdev and a team and removed request for stevehobbsdev October 21, 2019 23:39
@joshcanhelp joshcanhelp requested review from stevehobbsdev and removed request for stevehobbsdev October 29, 2019 15:02
src/auth/OAuthAuthenticator.js Show resolved Hide resolved
src/auth/OAuthAuthenticator.js Show resolved Hide resolved
expect(request.isDone()).to.be.true;
done();
})
.catch(done);
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't calling done imply the test is over successfully? And if so, I don't think we want that for the case where something goes wrong like here that an exception is thrown.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly, I'm not totally sure. This is only one of a few tests for refreshToken and we're just looking for something specific, that the secret is there.

@joshcanhelp
Copy link
Contributor Author

@lbalmaceda - Ready for you again!

@joshcanhelp joshcanhelp merged commit 13ddadc into master Dec 6, 2019
@joshcanhelp joshcanhelp deleted the add-secret-to-refresh-token-method branch December 6, 2019 19:09
@lbalmaceda lbalmaceda added this to the v2Next milestone Jan 10, 2020
@davidpatrick davidpatrick modified the milestones: v2Next, v2.21.0 Jan 15, 2020
@davidpatrick davidpatrick mentioned this pull request Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants