strimzi / strimzi-kafka-oauth

OAuth2 support for Apache Kafka® to work with many OAuth2 authorization servers
Apache License 2.0
146 stars 90 forks source link

Configured enable to use dynamic oauth introspection url parameter #143

Closed Eric84626 closed 2 years ago

Eric84626 commented 2 years ago

In this lib it is using hardcode parameter(token=) for oauth introspection endpoint, but some people want to use some other parameter name such as "atoken=", "btoken=" etc. We configured enable to use dynamic oauth introspection endpoint parameter.

mstruk commented 2 years ago

I'm not sure about this proposal. OAuth is a specification because it locks down a number of things. One of the major ones is how to invoke each endpoint.

It is very clearly defined, and it says one should use token as a parameter. See: https://datatracker.ietf.org/doc/html/rfc7662#page-4

Could you describe in more detail your authorization server setup, and why you don't want to use standard parameter names?

Eric84626 commented 2 years ago

I'm not sure about this proposal. OAuth is a specification because it locks down a number of things. One of the major ones is how to invoke each endpoint.

It is very clearly defined, and it says one should use token as a parameter. See: https://datatracker.ietf.org/doc/html/rfc7662#page-4

Could you describe in more detail your authorization server setup, and why you don't want to use standard parameter names?

Hello, Some companies/orgs own their internal oauth server, they could customize their introspection endpoint, and don't use standard parameter name as below. curl POST "https://XXXXX.com/validation/XXXX?access_token=XXXXX"

scholzj commented 2 years ago

Some companies/orgs own their internal oauth server, they could customize their introspection endpoint, and don't use standard parameter name as below. curl POST "https://xxxxx.com/validation/XXXX?access_token=XXXXX"

I leave it to Marko to decide whether this is something what makes sense or not. But in general, please understand that this project aims to provide OAuth authentication. If someone decides to not follow the specs, then it is probably their mistake which should be fixed on their side. This is not something what can be offloaded on open source projects and change them to match whatever someone decided to do contrary to the standard. While it is great that you contributed this as a PR, if this is merged there will be ongoing cost to support this non-standard thing in terms of increased testing effort, increased efforts when doing other improvements, etc. So we cannot always do something just because someone somewhere decided to deviate from the OAuth specs.

mstruk commented 2 years ago

This is exactly the sentiment I share. It's very easy to address this issue on the side of the user. You just add a few lines of code to your in-house authorization server so that it treats token the same way as access_token. Problem solved.

If that's not something you can do, there are ways to add rewrite rules to web servers, so you can front your authorization server with a reverse proxy and add a rewrite rule that changes token parameter in the url to access_token before forwarding the request on to the authorization server.

Also, doing a little research I could find no OAuth integration project that allows such a configuration. That tells me that non-compliant (in this aspect) OAuth authorization servers are virtually non-existent, and require all clients to write custom code rather than using an existing library for integration. That's simply the price you pay for not being standards compliant.

If this is the only point of standards non-compliance then it's easy to fix in the authorization server. If not, then this PR wouldn't help anyway, as the next problem would almost certainly be incompatibility in response attributes.

For these reasons it doesn't make much sense to me to support non-standard authorization servers in the way this PR proposes.

mstruk commented 2 years ago

This proposal isn't the appropriate way to address the issue. I'm closing it. @Eric84626 It would be interesting to hear how you addressed the issue.