Supporting basic authentication Token exchange in both the OAuth2 and OIDC handlers

[Tratcher]

#9448 (comment)

OAuth and OIDC have a standard flow of sending the clientid and secret to the token endpoint using a custom basic auth format. 4 of the auth handlers in aspnet-contrib require this flow and have to implement it manually. We expect many other providers also support this format since it's the one required in the spec.

Note the encoding is customized in the OAuth spec. (I don't think the FitBit handler is following that).
https://github.com/aspnet-contrib/AspNet.Security.OAuth.Providers/blob/aad5420654c65b5fb9908ddf298dbab17076338c/src/AspNet.Security.OAuth.Fitbit/FitbitAuthenticationHandler.cs#L66-L70

@PinpointTownes

[kevinchalet]

I don't think the FitBit handler is following that

Yeah, the logic is not standard-compliant. Not sure whether it's an oversight or not (I guess it is), but here's an example of a valid basic header construction: https://github.com/aspnet-contrib/AspNet.Security.OAuth.Extensions/blob/378348bb4efb1d277020e2a55f41a546368f7a31/src/AspNet.Security.OAuth.Introspection/OAuthIntrospectionHandler.cs#L414-L429

/cc @martincostello

[martincostello]

@PinpointTownes I'll look at updating that in aspnet-contrib/AspNet.Security.OAuth.Providers#280 based on the above sample.

[prochnowc]

@Tratcher Is there any plans to implement this in the OAuthHandler ? I need to add federation for some authority which only supports credentials via authorization header.

[Tratcher]

@prochnowc no plans at present. Only a few providers have required it, and even they don't use standard implementations.

You can handle this today the same way they did for FitBit.
https://github.com/aspnet-contrib/AspNet.Security.OAuth.Providers/blob/aad5420654c65b5fb9908ddf298dbab17076338c/src/AspNet.Security.OAuth.Fitbit/FitbitAuthenticationHandler.cs#L66-L70

[MrSmoke]

Just throwing my two cents on this ticket as I've also ran into this recently (also ended up using a similar method to the FitBit handler so no biggie).

Since its defined in the RFC it would be nice to have it as an option for those very few providers who do implement it as per the spec.

[dylantack]

Curiously, the documentation contains a description of TokenEndpointAuthMethodsSupported, but it appears to be unimplemented.

At least for me, this has no effect:

       options.Configuration = new OpenIdConnectConfiguration
       {
           TokenEndpointAuthMethodsSupported = { "client_secret_basic" },
       };

[lausitzer]

@Tratcher a whole rewrite of ExchangeCodeAsync() to support client_secret_basic is a bad idea (at least in my opinion) because you decouple from further improvements in the development and run the risk of incompatibilities. The linked FitBit code is a good example for that, it doesn't support PKCE verifier_code like the original implementation does. That results in trouble if your identity provider relies on PKCE (like ADFS) or expects verifier_code in the token request if you sent code_challenge in the authorization request (like GitLab).

If I may wish for something, implement client_secret_basic or split ExchangeCodeAsync() in one part for content creation and another one for transmission. That gives us a chance to adjust content in dedicated points before transmission without the need to generate it from scratch.

[kevinchalet]

The linked FitBit code is a good example for that, it doesn't support PKCE verifier_code like the original implementation does.

It's worth noting that the permalink present in the OP points to an old commit: the implementation has since been updated to optionally support PKCE (which isn't too terrible as it requires just 5 lines of code, braces included: https://github.com/aspnet-contrib/AspNet.Security.OAuth.Providers/blob/dev/src/AspNet.Security.OAuth.Fitbit/FitbitAuthenticationHandler.cs)

As part of the OpenIddict web providers development, I had to implement and test ~60 integrations (including a lot of providers we already support via aspnet-contrib), so I can shed some - fresh - light for those who are interested:

• Some providers - e.g Dropbox, LinkedIn, PayPal or Salesforce - have started offering OpenID Connect support (either by updating their existing OAuth 2.0 implementation or by providing new endpoints).

• Most of the providers that implemented OpenID Connect also implemented the optional discovery specification (which is a great thing). Sadly, I didn't see much appetite for the OAuth 2.0 equivalent/backport - OAuth 2.0 Authorization Server Metadata - that wasn't adopted by any major service.

• PKCE is slowly being implemented in popular services: ArcGIS, Discord, Facebook, Fitbit, Lichess, Twitter and Zoom now officially support it.

• There are still providers that only support client_secret_basic: Fitbit, Kroger, Notion, Reddit, ServiceChannel and Twitter. Reddit is still the weirdest one in that list as it requires sending the client_id using basic authentication even when there's no client_secret (in this case, the standard approach would be to send client_id in the form).

• The fact a provider offers a recent implementation doesn't mean it's standard-compliant, even if it implements OIDC. For instance:

  • The new OIDC-based LinkedIn implementation doesn't return a token_endpoint_auth_methods_supported node in its discovery document, so a standard client would have to use client_secret_basic (since it's the default client authentication method)... but LinkedIn doesn't support basic authentication AT ALL 🤣
  • The quite recent EpicGames service (OIDC-based too) doesn't implement the standard client_secret_basic encoding method as it doesn't support formURL-encoding AND returns chars that need to be escaped in its tokens...

Conclusion: it's sadly still a hot mess.