badgateway / oauth2-client

OAuth2 client for Node and browsers
https://www.npmjs.com/package/@badgateway/oauth2-client
MIT License
285 stars 34 forks source link

Add support for client_secret_post auth method #84

Closed parkerduckworth closed 1 year ago

parkerduckworth commented 1 year ago

Adds support for client_secret_post authentication method for the client_credentials and password grants. This is useful for those who are unable to allow client_secret_basic on their auth server.

Tests have been added to assert correct behavior for:

Additionally, I ran a formatter over the source files containing my changes, and fixed some typos.


Closes #85

parkerduckworth commented 1 year ago

One more thing to note. I built a new dist and ran the changes with the project that I am working on. I can confirm that these changes behave as expected when authenticating with an OIDC server

evert commented 1 year ago

Great PR, but I'm wondering if it's worth taking it one step further and also supporting token_endpoint_auth_methods_supported from the OAuth 2 discovery doc, in one swoop.

As much as possible I'd want to encourage people to use the discovery doc so there's less configuration on the client. Only reason I'd hold off with this, is because it would suck to release a feature and then soon after have to break compatibility (or add some (minor) tech debt).

If you're up for tackling this more generally via the discovery doc, let me know. If not, also happy to pick this up in the next few days.

evert commented 1 year ago

P.S.: make fix should correct most of the linting issues

parkerduckworth commented 1 year ago

Hey @evert I'd love to take a shot at taking the discovery doc route. Thanks!

parkerduckworth commented 1 year ago

@evert after pondering this for a bit, I'd like your opinion on this:

I understand the motivation to offload as much client configuration as we can to the discovery document, but consider this scenario. Let's say the target oauth server returns both client_secret_post and client_secret_basic as token_endpoint_auth_methods_supported.

Without client-side configuration, what is the best way to communicate the client's desired auth method, without an explicit property on the ClientSettings?

evert commented 1 year ago

Let's say the target oauth server returns both client_secret_post and client_secret_basic

I think for those cases we can use our own preference (which for me would be 'basic auth').

Without client-side configuration, what is the best way to communicate the client's desired auth method, without an explicit property on the ClientSettings?

Did you mean 'without server-side configuration' ?

parkerduckworth commented 1 year ago

Did you mean 'without server-side configuration' ?

What I mean is, if the server supports both auth methods, are we interested in allowing the client to specify which method they prefer to use?

evert commented 1 year ago

Ah!

What I mean is, if the server supports both auth methods, are we interested in allowing the client to specify which method they prefer to use?

Do you have a need? I'm inclined to not add features until someone has a motivation for them, or they are somewhat obvious. So I would just pick 'Basic' if both are available, and wait for the first issue in case that's not good enough for someone.