-
Notifications
You must be signed in to change notification settings - Fork 203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Client Credentials Flow #1231
base: master
Are you sure you want to change the base?
Conversation
It looks like the docker tests are expected to fail? I see the following which seems reasonable:
|
docker/build-push-action was failing on forked pull request. I have fixed the problem as #1233. |
pkg/oidc/client/client.go
Outdated
return &oidc.TokenSet{ | ||
IDToken: token.AccessToken, | ||
RefreshToken: token.RefreshToken, | ||
}, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it needs to verify the retrieved token.
For example,
kubelogin/pkg/oidc/client/client.go
Line 165 in 9263439
return c.verifyToken(ctx, token, "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I've implemented this now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately in my extended testing, verifyToken
fails to accept my valid OAuth 2.0 token. Perhaps you can advise which path you prefer for me to follow?
Per the OAuth2 RFC, where the Client Credential flow is defined, it looks like access_token
is a required response per the RFC8693 Successful Response definition but no longer is id_token
mentioned. (Similarly Auth0 and Okta have diagrams pointing out an access_token
is returned for Client Credentials.)
Meanwhile, I do see in the OIDC Core Spec, id_token
is required it its Token Response section.
Using verifyToken
my exchange works but I fail to accept the token:
./kubelogin get-token \
--grant-type=client-credentials \
--oidc-issuer-url=https://<elided> \
--oidc-client-id=<elided> \
--oidc-client-secret=<elided> \
--no-keyring
error: get-token: authentication error: client-credentials error: authorization error: id_token is missing in the token response: &oauth2.Token{AccessToken:"... elided ...", TokenType:"Bearer", RefreshToken:"", Expiry:time.Date(2025, time.January, 16, 23, 31, 55, 191375292, time.Local), ExpiresIn:0, raw:map[string]interface {}{"access_token":"... elided ...", "expires_in":59, "token_type":"Bearer"}, expiryDelta:0}
The code does work for me, as originally implemented, returning the access_token
in my client.authentication.k8s.io/v1beta1
JSON payload as token
which my API server accepts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about using this feature with --use-access-token
flag? It will use the access_token as the token response for Kubernetes API server. See also #1083.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, the structure of the token is different than what VerifyToken
is expecting.
The Client Credentials RFC token specification does not return an OIDC IDToken? (Specifically the response specification in RFC 6749 does not include even an optional IDToken field.)
As such, at least as configured by my IdP, there is no IDToken envelope to pull the AccessToken out of; the AccessToken is all I receive:
&oauth2.Token{AccessToken:"[...]", TokenType:"Bearer", RefreshToken:"", Expiry:time.Date(2025, time.January, 26, 9, 43, 35, 530935198, time.Local)
Questions to confirm:
- I don't think it will be possible to use
--grant-type=client-credentials
without--use-access-token
as I've implemented? VerifyToken
would need to parse a different token type to still be used but I also do not believe we would have the need for verification like with the other grant types (the token could be fully opaque per the specification)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@int128 I think I understand questions 1 and 2 now. I think this code should be good.
Does it look okay to you?
This comment was marked as resolved.
This comment was marked as resolved.
9043b19
to
307212c
Compare
c58369d
to
c6d31d7
Compare
This provides support for the OAuth 2.0 Client Credentials authentication login flow.
The Client Credentials flow has been asked about in #931. I have been working with a piece of software which only accepts a K8s configuration for connection setup and authentication, so client credentials works great for me for machine-to-machine communications. I've used this code against a private IdP successfully.