strimzi-kafka-oauth icon indicating copy to clipboard operation
strimzi-kafka-oauth copied to clipboard

support client_credentials with client_assertion

Open robbertvanwaveren opened this issue 2 years ago • 3 comments

PR for issue #164

All builds and tests are working.

I added the following abilities:

  1. support for client assertion (private_key_jwt) based authentication on the token endpoint as per https://www.rfc-editor.org/rfc/rfc7523#section-2.2
  2. support for acquiring token, refresh token, client assertion (token) from filesystem on-demand (to support refreshment by an external process such as Azure AD Workload Identity).
  3. support for acquiring config values based on references to environment variables (to support pointing to externally injected environment variables such as those from Azure AD Workload Identity)

I was not yet able to test the changing token in real scenario as part of a re-authentication request. So the fact that a re-authentication re-triggers JaasClientOauthLoginCallbackHandler.handle is an untested assumption at this point.

robbertvanwaveren avatar Oct 13 '22 08:10 robbertvanwaveren

This PR has been tested in a real deployment and hourly re-authentication is working as expected.

robbertvanwaveren avatar Oct 17 '22 07:10 robbertvanwaveren

@robbertvanwaveren Thanks for the PR. Could you also add some documentation in README.md to explain the circumstances when one would want to use this, and how the workflow works in such a case?

I also had one problem running the tests. See the proposed fix.

mstruk avatar Oct 17 '22 08:10 mstruk

I've made a few suggested additions.

robbertvanwaveren avatar Oct 17 '22 09:10 robbertvanwaveren

@mstruk Do you require anything further from my side?

robbertvanwaveren avatar Oct 24 '22 07:10 robbertvanwaveren

@robbertvanwaveren Not at the moment. This PR still has to be thoroughly reviewed.

Currently I'm busy doing the 0.11.0 release, and integrating the functionality into the main Strimzi Project. I'll get back to this when I find the time.

mstruk avatar Oct 24 '22 08:10 mstruk

@mstruk What is the plan for this?

scholzj avatar Mar 09 '23 08:03 scholzj

@scholzj The plan is to go back to this very soon now.

mstruk avatar Mar 13 '23 09:03 mstruk

@robbertvanwaveren I'm looking at this issue again. There's been a lot of changes by other PRs resulting in conflicts. I can merge the changes in some branch, and you can hard reset your branch from there so we can then continue this PR and get it merged, if that's ok with you.

mstruk avatar Jul 14 '23 09:07 mstruk

@robbertvanwaveren I merged the changes here: https://github.com/mstruk/strimzi-kafka-oauth/commits/oauth-client-assertion-support

Feel free to hard reset your branch to https://github.com/mstruk/strimzi-kafka-oauth/commit/d280c414b6aae0565dbdd2fcba8c8ea6ef2f17ad

mstruk avatar Jul 25 '23 11:07 mstruk

@mstruk Did you managed to make any progress around this? Any ideas what to do with it?

scholzj avatar Sep 21 '23 08:09 scholzj

@robbertvanwaveren Are you still interested in getting this PR merged?

If yes, we can get it merged quickly. I can prepare another rebase to which you just hard reset your PR and we can merge it.

mstruk avatar Sep 21 '23 10:09 mstruk

Discussed on the Community cal of 19.10.2023: There was no reply for a long time. This PR should be closed. If there is some renewed interest later, we can reopen it.

scholzj avatar Oct 19 '23 08:10 scholzj

This PR would allow clients to use OpenID Connect with federated identity via client assertion issued by another IdP to get a token. Kubernetes workloads in Azure cloud provider are migrating to this authentication model. It would be great to reopen the review of this PR. Best regards.

https://curity.io/resources/learn/jwt-assertion/ https://www.rfc-editor.org/rfc/rfc7523 https://www.rfc-editor.org/rfc/rfc7521

shinji avatar Oct 25 '23 03:10 shinji

@shinji The PR has stalled and without the DCO sign-off there is not much we can do with it. We either need the DCO sign-off to be fixed and then the existing code here might be used. Or it needs to be implemented from scratch.

scholzj avatar Oct 26 '23 02:10 scholzj

Any specific action required from me?

robbertvanwaveren avatar Oct 27 '23 16:10 robbertvanwaveren

If you could at least fix the DCO signoff, it would be possible to re-use the code. The instructions should be here (assuming the link works): https://github.com/strimzi/strimzi-kafka-oauth/runs/8930450983

scholzj avatar Oct 27 '23 21:10 scholzj

Any specific action required from me?

@robbertvanwaveren You can just use your oauth-client-assertion-support branch and run the following:

git rebase HEAD~3 --signoff
git push --force-with-lease origin oauth-client-assertion-support

I will then do the rebase, because there are many conflicts at this point due to other PRs that were merged in the mean time.

You can read more about DCO here: https://github.com/apps/dco

Another thing you can do is enable commits into your branch as described here which will allow me to also merge the PR. Otherwise I'll have to ask you to hard reset your PR to my rebased branch and force push it again before we can merge.

Thanks.

mstruk avatar Oct 30 '23 09:10 mstruk

I had to create a new PR to grant rights: https://github.com/strimzi/strimzi-kafka-oauth/pull/211

robbertvanwaveren avatar Oct 30 '23 13:10 robbertvanwaveren