HeroicKatora / oxide-auth

A OAuth2 server library, for use in combination with actix or other frontends, featuring a set of configurable and pluggable backends.
682 stars 91 forks source link

Access Token Flow diverges from OAuth client implementations #199

Closed ImTheSquid closed 1 day ago

ImTheSquid commented 1 day ago

Bug report

The AccessTokenFlow requires a client_id in the request, but this violates the OAuth2 RFC.

The effects of this bug are that OAuth libraries don't work with this as a server.

Reproduction

Use any request library.

Steps to reproduce the behavior:

  1. Do the standard authorization flow with the authorization_code grant type
  2. Try to get a token without the client_id using AccessTokenFlow
  3. Error on InvalidClient

Expected behaviour

According to RFC 6749 4.1.D, it should not require the client ID, but it seems like it does.

HeroicKatora commented 1 day ago

(There is no 4.1.D but guessing you're referring to 4.1.3)

client_id REQUIRED, if the client is not authenticating with the authorization server as described in Section 3.2.1.

Do you have specific examples of libraries that don't work?

ImTheSquid commented 1 day ago

Here is my passage of interest:

The client requests an access token from the authorization server's token endpoint by including the authorization code received in the previous step. When making the request, the client authenticates with the authorization server. The client includes the redirection URI used to obtain the authorization code for verification.

It turns out this is from a list coming from a figure description, but it doesn't include the client ID. AuthJS doesn't work since it doesn't send the client ID in the token request it makes.

HeroicKatora commented 1 day ago

oxide-auth does accept either a client_id (for public clients) or an authorization header. It's written to the letter of the spec as far as I'm concerned. Please consider contributing to the test suite with HTTP request logs or minimal reproduction code that I can execute if you disagree, not name dropping. Wouldn't be the first JS library to be buggy / take an improperly liberal interpretation on this specific request.

Also have you verified the issue isn't on the library side? Is it a custom provider you're configuring?

ImTheSquid commented 1 day ago

Yes it is! Thank you so much for finding this, I was working on this for ages going around in circles. That fixed the issue. Sorry I implied oxide violated spec without knowing what I was talking about. Thanks for making such a great (and very spec compliant) library!