indieweb / indieauth

IndieAuth.net website code and IndieAuth Specification
52 stars 7 forks source link

Client may provide the `client_id` in the Authorization header for the authorization code exchange #76

Closed jamietanna closed 3 years ago

jamietanna commented 3 years ago

When using an OAuth2, not an IndieAuth, client, it's possible that the client may provide the client_id on the authorization code exchange in the Authorization header in the request, rather than in the body of the request.

Did we want to add something, as a MAY for instance, to allow servers to add support, for better OAuth2 interop?

sknebel commented 3 years ago

Do you have a (ideally spec) reference for that? To my knowledge, the Authorization header can be used for a client password if one has been given, but is never used for client_id in OAuth 2, but I could very well be missing an extension.

jamietanna commented 3 years ago

For example, in https://tools.ietf.org/html/rfc6749#section-4.1.3:


     POST /token HTTP/1.1
     Host: server.example.com
     Authorization: Basic czZCaGRSa3F0MzpnWDFmQmF0M2JW
     Content-Type: application/x-www-form-urlencoded

     grant_type=authorization_code&code=SplxlOBeZQQYbYS6WxSbIA
     &redirect_uri=https%3A%2F%2Fclient%2Eexample%2Ecom%2Fcb

The client_id is not referenced in the request body, but as part of the authentication.

As we only have public clients, it doesn't make sense for IndieAuth clients to send just their client_id in the Authorization header, but I've definitely seen it with an OAuth2 client that doesn't have a client_secret set

sknebel commented 3 years ago

Right, because it is using username:password in the Authorization header, missed that. So you are asking about doing Basic auth with an empty password to pass the client_id, not to pass the client_id as the value of the Authorization header and treating the client as "authenticated", allowing it to ignore the requirement for unauthenticated clients to pass the client_id in the request body?

jamietanna commented 3 years ago

Yep that's the thought :smile:

Zegnat commented 3 years ago

I would be against this unless there are clear examples of OAuth2 clients that could be used with IndieAuth out-of-the-box if this was added. If there is no immediate win in compatibility with existing projects, then I personally do not think adding an optional implementation detail to the spec helps.

I feel that the OAuth framework makes it relatively clear that authenticating to the AS in this way is aimed at “confidential clients or other clients issued client credentials”. (3.2.1.) IndieAuth by itself has no concept of this. Instead the standard complies to that section by explicitly requiring that “an unauthenticated client MUST send its "client_id"” request parameter.

The framework tells us “the authorization server MUST NOT rely on public client authentication for the purpose of identifying the client.” (2.3.) The IndieAuth specification defines all its clients as public clients. I do not think it makes sense to both make the IndieAuth specification say that all clients are public clients while also stating that the AS may implement client authentication.

For token requests (4.1.3.), it doubles down on this (emphasis mine):

The authorization server MUST:

[…]

  • ensure that the authorization code was issued to the authenticated confidential client, or if the client is public, ensure that the code was issued to "client_id" in the request,

This to me seems to say that even if you do the token request as quoted in this issue before, you should still add client_id to the request body. Then the client is just duplicating data.

Unless the IndieAuth specification is tweaked to also allow authenticated/confidential clients, I see no reason to tack authentication onto it.

As a side-note outside of OAuth specific logic: URLs are invalid usernames for HTTP Basic Auth, so IndieAuth’s client IDs are not allowed for client password (2.3.1.) authentication. From RFC 7617: 2. The 'Basic' Authentication Scheme: “a user-id containing a colon character is invalid, as the first colon in a user-pass string separates user-id and password”.

This does not mean an AS can’t implement this. IndieAuth is an OAuth 2.0 extension and usually compatible with other OAuth 2.0 use cases. If an AS has a use for allowing authenticated clients, the developer should add it. I only think that, as long as IndieAuth only specifies public clients, there is no reason for the IndieAuth extension to talk about OAuth 2.0 features that exist outside of its own scope. Does that make sense? 🤔

jamietanna commented 3 years ago

I would be against this unless there are clear examples of OAuth2 clients that could be used with IndieAuth out-of-the-box if this was added. If there is no immediate win in compatibility with existing projects, then I personally do not think adding an optional implementation detail to the spec helps.

I've definitely found this as part of implementing https://gitlab.com/jamietanna/micropub-postman with rack-oauth2, as that needed me to send the client authentication in the body.

That is a very good reading, though, and I appreciate the detailed response - I think we can instead put this down to the fact that this library does not expect a public client.