search

node-oauth / node-oauth2-server

🚀 The successor to oauthjs/oauth2-server. 🔒 Complete, compliant, maintained and well tested OAuth2 Server for node.js. Includes native async await and PKCE.
https://www.npmjs.com/package/@node-oauth/oauth2-server
MIT License
286 stars 39 forks source link

PR #197 fix removed after merge #258

Closed mthlemos closed 10 months ago

mthlemos commented 10 months ago

Specify your setup

Describe the bug

PR #197 fix got removed after a merge commit.

More specifically this merge commit: 6758bffe4e53979ad86ad908c5f671002dcfe9e2

To Reproduce

This bug is currently present on the release-5.0.0 branch.

https://github.com/node-oauth/node-oauth2-server/blob/b97f6c773cecca3563b0b39c7179ed813d006c4f/lib/handlers/authorize-handler.js#L370

https://github.com/node-oauth/node-oauth2-server/blob/b97f6c773cecca3563b0b39c7179ed813d006c4f/lib/handlers/authorize-handler.js#L381

Expected behavior

Should also receive the code_challenge and code_challenge_method through the query params.

getCodeChallenge (request) {
  return request.body.code_challenge || request.query.code_challenge;
}
getCodeChallengeMethod (request) {
  const algorithm = request.body.code_challenge_method || request.query.code_challenge_method;
  ...
}

Additional context

None

jankapunkt commented 10 months ago

Thank you for reporting @mthlemos I will try to cherry pick this one and create a new RC (which is still pending since I was on vacation)

jankapunkt commented 10 months ago

@mthlemos added this again in https://github.com/node-oauth/node-oauth2-server/releases/tag/v5.0.0-rc.5 feel free to reopen if the issue persists in rc5

mthlemos commented 10 months ago

Thank you very much for the quick fix @jankapunkt!