node-oauth / express-oauth-server

🔒 Complete, compliant and well tested module for implementing an OAuth2 Server/Provider with express in node.js
https://www.npmjs.com/package/@node-oauth/express-oauth-server
MIT License
29 stars 8 forks source link

Errors problems with @node-oauth/express-oauth-server and @node-oauth/oauth2-server version 5 #23

Closed kanelloc closed 9 months ago

kanelloc commented 10 months ago

After the recent update to the @node-oauth/oauth2-server package to version 5.0.x, there is a bug related to error evaluation.

The main issue arises when an error is thrown from @node-oauth/express-oauth-server, which still uses @node-oauth/oauth2-server version 4.3.0. Due to this version discrepancy, the error instances from the older version do not match those from the new version 5.0.x, leading to compatibility issues.

Just pointing out the issue as there is already a PR which updates the version of @node-oauth/oauth2-server to the latest one.

jankapunkt commented 10 months ago

@kanelloc, thank you for reporting. Until the PR is merged I can publish a release candidate with it's most recent state. Would be interested in such to check if this resolve the issue on your side?

kanelloc commented 10 months ago

@kanelloc, thank you for reporting. Until the PR is merged I can publish a release candidate with it's most recent state. Would be interested in such to check if this resolve the issue on your side?

Sure I can test it! 👍

From my side I can also wait for the major release v.4.0.0

jankapunkt commented 10 months ago

@kanelloc https://github.com/node-oauth/express-oauth-server/releases/tag/v4.0.0-rc.0 any feedback is appreciated

jankapunkt commented 9 months ago

@kanelloc any issues encountered during your tests of the RC?

kanelloc commented 9 months ago

Hey @jankapunkt sorry for the late reply I missed the mentions. I'll test it today and reply here

jankapunkt commented 9 months ago

@kanelloc no worries, let me know if something unusual came up. Otherwise I'm going to merge and release with beginning of February :rocket:

uf0h commented 9 months ago

Tested v4.0.0-rc.0, no issues 👍. Waiting for official release.

kanelloc commented 9 months ago

@jankapunkt I just found one small issue on the latest version v4.0.0

In the previous version in case an invalid token was used the library was throwing an InvalidTokenError with message invalid_token: Invalid token: access token is invalid and code: 401

Currently for the same use case a 400 error is thrown which is an InvalidRequestError with message: invalid_request: Invalid request: malformed authorization header

To give you a bit of context this is thrown only when an invalid token is used not a expired one. Is this change on purpose?

jankapunkt commented 9 months ago

The change itself does not come from this package but from the updated OAuth2 server package that is now in use.

The error is correct, because the RFC requires different errors for different cases:

https://datatracker.ietf.org/doc/html/rfc6750#section-3.1

   invalid_request
         The request is missing a required parameter, includes an
         unsupported parameter or parameter value, repeats the same
         parameter, uses more than one method for including an access
         token, or is otherwise malformed.  The resource server SHOULD
         respond with the HTTP 400 (Bad Request) status code.

   invalid_token
         The access token provided is expired, revoked, malformed, or
         invalid for other reasons.  The resource SHOULD respond with
         the HTTP 401 (Unauthorized) status code.  The client MAY
         request a new access token and retry the protected resource
         request.

The core library now validates token schema against /^Bearer ([0-9a-zA-Z-._~+/]+=*)$/ before validating if the token is valid. In previous versions it was not differentiable, whether the token itself is invalid or doesn't conform the Bearer schema

A little more context as I know this section may be interpreted differently. The idea is, that a malformed token is assumed to be an invalid parameter and should not even reach your model, as opposed to a schematically valid token that may not exist or is outdated, revoked etc.