oauthjs / node-oauth2-server

Complete, compliant and well tested module for implementing an OAuth2 Server/Provider with express in node.js
https://npmjs.org/package/oauth2-server
MIT License
4.02k stars 931 forks source link

Incorrect handling of redirect_uri in AuthorizeHandler.prototype.buildErrorRedirectUri #455

Open kryysler opened 6 years ago

kryysler commented 6 years ago

The optional url query string is not parsed, but replaced with an object containing the error params. However, as the "search" property of the parsed Url object is not changed, the url when formatted again will contain the original params only and not the error params.

https://github.com/oauthjs/node-oauth2-server/blob/master/lib/handlers/authorize-handler.js#L300

In order to work, the function would have to do something like this:

AuthorizeHandler.prototype.buildErrorRedirectUri = function(redirectUri, error) {
  var uri = url.parse(redirectUri, true);

  uri.search = null;
  uri.query.error = error.name;

  if (error.message) {
    uri.query.error_description = error.message;
  }

  return uri;
};
markstos commented 6 years ago

@kryysler If you are clear what the correct behavior is, could your provide a text that exercises the bug, code updates to fix it, and references to the related parts of the OAuth RFC that justify your change as more spec compliant?