Only require 'redirect_uri' in token/authorization_code when one was provided in authorize/code

[scagood]

This PR is to prevent redirect_uri being saved using saveAuthorizationCode when its not directly requested, this is so that AuthorizationCodeGrantType#validateRedirectUri does not try to validate against the fallback of client.redirectUris[0]

This is the comment that made me think this makes sense: https://github.com/oauthjs/node-oauth2-server/blob/master/lib/grant-types/authorization-code-grant-type.js#L125-L134

[scagood]

For reference this is an example of what I think is incorrect:

First request a token:

[~/github/saml-oauth2 (master)] $ curl -v 'http://localhost:8080/oauth/authorize?client_id=123&response_type=code&scope=read&state=31232' 
> GET /oauth/authorize?client_id=123&response_type=code&scope=read&state=31232 HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.68.0
> Accept: */*
> 
< HTTP/1.1 302 Found
< X-Powered-By: Express
< location: http://localhost:8080/success?code=7abc490fda5133d7d401b41e911ad1736e8ae1fe&state=31232
< Content-Type: application/json; charset=utf-8
< Content-Length: 2
< ETag: W/"2-vyGp6PvFo4RvsFtPoIWeCReyIC8"
< Date: Sat, 21 Aug 2021 02:25:51 GMT
< Connection: keep-alive
< Keep-Alive: timeout=5
< 
{}

The request that I think is incorrect because I never sent a redirect_uri

[~/github/saml-oauth2 (master)] $ curl --request POST --url 'http://localhost:8080/oauth/token' \
  --header 'content-type: application/x-www-form-urlencoded' \
  --data grant_type=authorization_code \
  --data client_id=123 \
  --data scope='read write' \
  --data code=7abc490fda5133d7d401b41e911ad1736e8ae1fe
invalid_request: Invalid request: `redirect_uri` is not a valid URI
    at new InvalidRequest (/home/scagood/github/saml-oauth2/node_modules/oauth2-server/lib/errors/invalid-request-error.js:26:14)
    at AuthorizationCodeGrantType.validateRedirectUri (/home/scagood/github/saml-oauth2/node_modules/oauth2-server/lib/grant-types/authorization-code-grant-type.js:144:12)
    at AuthorizationCodeGrantType.<anonymous> (/home/scagood/github/saml-oauth2/node_modules/oauth2-server/lib/grant-types/authorization-code-grant-type.js:69:19)
    at PassThroughHandlerContext.finallyHandler (/home/scagood/github/saml-oauth2/node_modules/oauth2-server/node_modules/bluebird/js/release/finally.js:57:23)
    at PassThroughHandlerContext.tryCatcher (/home/scagood/github/saml-oauth2/node_modules/oauth2-server/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/home/scagood/github/saml-oauth2/node_modules/oauth2-server/node_modules/bluebird/js/release/promise.js:547:31)
    at Promise._settlePromise (/home/scagood/github/saml-oauth2/node_modules/oauth2-server/node_modules/bluebird/js/release/promise.js:604:18)
    at Promise._settlePromise0 (/home/scagood/github/saml-oauth2/node_modules/oauth2-server/node_modules/bluebird/js/release/promise.js:649:10)
    at Promise._settlePromises (/home/scagood/github/saml-oauth2/node_modules/oauth2-server/node_modules/bluebird/js/release/promise.js:729:18)
    at _drainQueueStep (/home/scagood/github/saml-oauth2/node_modules/oauth2-server/node_modules/bluebird/js/release/async.js:93:12)
    at _drainQueue (/home/scagood/github/saml-oauth2/node_modules/oauth2-server/node_modules/bluebird/js/release/async.js:86:9)
    at Async._drainQueues (/home/scagood/github/saml-oauth2/node_modules/oauth2-server/node_modules/bluebird/js/release/async.js:102:5)
    at Immediate.Async.drainQueues [as _onImmediate] (/home/scagood/github/saml-oauth2/node_modules/oauth2-server/node_modules/bluebird/js/release/async.js:15:14)
    at processImmediate (node:internal/timers:464:21)

The request that actually works as it contains redirect_uri:

[~/github/saml-oauth2 (master)] $ curl --request POST --url 'http://localhost:8080/oauth/token' \
  --header 'content-type: application/x-www-form-urlencoded' \
  --data grant_type=authorization_code \
  --data client_id=123 \
  --data scope='read write' \
  --data code=7abc490fda5133d7d401b41e911ad1736e8ae1fe \
  --data redirect_uri=http://localhost:8080/success
{
  "access_token": "5c67a9a07a2b8c6d18bbe592e7f5f215a57e4b1e",
  "token_type": "Bearer",
  "expires_in": 3599,
  "refresh_token": "acd822f2c8b2f3bbfd2baf9ef7131a84359e4fc9",
  "scope": "read",
  "_id": "612064638b41291f191d2c33",
  "clientId": "123",
  "userId": null,
  "expiresAt": "2021-09-04T02:26:43.252Z"
}

[whatwewant]

Maybe you are wrong. You should read RFC 6749 The OAuth 2.0 Authorization Framework - Authorization Code Grant

And check your model on getClient and https://github.com/oauthjs/node-oauth2-server/blob/6d4c98794bc024a8cf93cf9e79f92f93766da9f4/lib/handlers/authorize-handler.js#L197

[scagood]

@whatwewant I referenced the relevent comment to the relevent part of the spec in my description.

To quote: [redirect_uri] REQUIRED, if the "redirect_uri" parameter was included in the authorization request

The redirect_uri is only REQUIRED IF it was included in the authorization request, not if it is in the client.

Full quote:

redirect_uri
         REQUIRED, if the "redirect_uri" parameter was included in the
         authorization request as described in Section 4.1.1, and their
         values MUST be identical.

https://tools.ietf.org/html/rfc6749#section-4.1.3

[HappyZombies]

Hello, due to this project appearing to be dead and no maintainers responding, I went ahead and forked the project under a new organization, and will continue the work over there. https://github.com/node-oauth/node-oauth2-server

Feel free to move over there to further the discussion