Skip to content

Fixes ignoring scope on refresh token call #238

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

Merged

Conversation

jorenvandeweyer
Copy link
Member

Summary

  • Fixes Scope in refresh_token grant type is ignored. #104
  • Rewrote scopes to only allow an array of strings instead of where before both a string and an array of strings were allowed.
  • Validate scopes safely
    BREAKING: validateScope must now only return an array of strings or a falsy value.
  • Updated docs (also removed places still mentioning callbacks)

Linked issue(s)

#104 #194

Involved parts of the project

Scopes

Added tests?

Yes & updated existing tests

OAuth2 standard

See #104

@jorenvandeweyer
Copy link
Member Author

@jankapunkt I targeted the PR to the release-5.0.0 branch to avoid merge conflicts since this branch is ahead of development

I think we should asap merge release-5.0.0 into development. We can do this before or after merging this PR.

@shrihari-prakash
Copy link
Contributor

@jankapunkt , @jorenvandeweyer , will there be one more RC to test this change? I think this is already breaking with my project and maybe I can do a code change and test this to see if things are smooth before 5.0.0?

@jorenvandeweyer
Copy link
Member Author

@jankapunkt can there be a RC of this PR?

@jankapunkt
Copy link
Member

Hi @jorenvandeweyer @shrihari-prakash I was sick the last days jost got back today 💫 I will merge later today and create an rc

Copy link
Member

@jankapunkt jankapunkt left a comment

Choose a reason for hiding this comment

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

@jorenvandeweyer I added some comments, can we shortly discuss before merging?

@jankapunkt
Copy link
Member

@jorenvandeweyer would you mind shortly reviewing my comments? I think it's ready to merge I just want to make sure there is nothing missing etc. I can create a new release ASAP once it's merged

@jorenvandeweyer
Copy link
Member Author

@jankapunkt sorry for the delay, I updated the PR.

It should be ready to be merged now!

@jankapunkt jankapunkt merged commit d50cb2d into node-oauth:release-5.0.0 Sep 28, 2023
@jorenvandeweyer jorenvandeweyer deleted the feature/scope-validation branch September 28, 2023 08:01
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.

None yet

3 participants