OAuth 2.0 behavior deviates from standard practice

cyyynthia commented 5 months ago

Describe the bug

The current implementation of the OAuth 2.0 flow deviates in certain key areas making it hard to interop with generic clients. I encountered these while using my own generic OAuth handler.

Most notably,

Client authentication (RFC 6749 §2.3.) is done using a non-standard authentication system where the client secret is sent "as-is" as the authentication header. The general standard is to use HTTP Basic authentication, which is mentioned in §2.3.1..
The API returns a Bearer token (RFC 6750), but does not support Bearer <token> authorization format despite being the format defined in RFC 6750 §2.1.. Only the prefix-less token is supported.

Steps to reproduce

Create an OAuth application
Initiate an authotization flow
Request the bearer access token (with Authorization: Basic base64(client_id:client_secret))
Request the authenticated user data (with Authorization: Bearer access_token)

Expected behavior

The flow to succeed and to get information about the authenticated user.

Additional context

No response

triphora commented 5 months ago

@cyyynthia Would you be able to review #872 to make sure I implemented it correctly?

newtykip commented 2 months ago

Hello! I was just wondering how you worked around this - did you just completely write a new client to interop with Modrinth's implementation? I am currently facing the same issue @cyyynthia

cyyynthia commented 2 months ago

The OAuth client I'm using is already my own, so I could just attach extra logic for Modrinth specifically - I didn't though and I just don't have support yet 😄

Modrinth's OAuth implementation is mostly compliant except for the client secret part. You need to send the client_id in the request payload (probably already happening since many implementations expect client credentials to be sent via the request payload), and the client secret in the Authorization header as a plain string (Authorization: <client_secret>).

In my own implementation I would only have to add a special case to the Authorization header, everything else works as expected using a generic implementation.

newtykip commented 2 months ago

Oh thank you! That's what I was missing! I was just about to scour through the codebase but you saved me some time - thank you :]

modrinth / labrinth