Implement OIDC client AuthProvider #25270

bobbyrullo · 2016-05-06T17:47:44Z

@erictune @cjcullen @liggitt @sym3tri @philips @ericchiang

With this PR, kubectl and other RestClient's using the AuthProvider framework can make OIDC authenticated requests, and, if there is a refresh token present, the tokens will be refreshed as needed.

googlebot · 2016-05-06T17:47:48Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

k8s-bot · 2016-05-06T17:48:14Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-05-06T17:48:32Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-05-06T17:49:13Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

fabioy · 2016-05-09T22:47:43Z

@erictune Feel free to reassign as you see fit. Thank you.

erictune · 2016-05-11T14:08:24Z

fyi, you can probably do a git rebase -i and delete all the commits that are not yours or CJ's.

erictune · 2016-05-11T14:12:31Z

plugin/pkg/client/auth/oidc/oidc.go

+
+	clientConfig := restclient.Config{
+		TLSClientConfig: restclient.TLSClientConfig{
+			CAFile: cfg["idp-certifiate-authority"],


s/certifiate/certificate

erictune · 2016-05-11T14:31:31Z

The way this stores its config and refresh token in the .kube/config seems fine.

bobbyrullo · 2016-05-11T17:00:21Z

Thanks for your comments @erictune; I'm going to rebase onto master now that CJ's changes are merged...If the comments get lost, I'll paste 'em back in.

googlebot · 2016-05-11T23:51:13Z

CLAs look good, thanks!

k8s-github-robot · 2016-05-18T21:37:48Z

@yifan-gu
You must link to the test flake issue which caused you to request this manual re-test.
Re-test requests should be in the form of: k8s-bot test this issue: #<number>
Here is the list of open test flakes.

yifan-gu · 2016-05-18T22:16:37Z

Applying LGTM according to #25270 (comment)

yifan-gu · 2016-05-18T22:30:13Z

@k8s-bot ok to test

yifan-gu · 2016-05-18T23:11:26Z

@k8s-bot test this issue: #25791

erictune · 2016-05-18T23:44:32Z

I did an offline design discussion with @philips @bobbyrullo @sym3tri and I understand better how this PR will work.

They explained how my previous concerns were satisfied by this PR.

not using wierd apiserver refresh flow that seemed not OIDC compliant to me (at least to the spirit of the law)
- not assuming a single client_secret for all instances of kubectl.
- it will work with many IdPs, depending on how you configure it. different IdPs will support different modes.
they are going to document several ways to use this with different IdP implementations.

I am now fine with keeping the name the same.

I don't have time to review before deadine. I asked @sym3tri to review @bobbyrullo code, and @yifan-gu to manage the LGTM labels.

Ways to use this, as I now understand it:

a single client_id/client_secret
- should work with any IdP
- typically for use in a single organization where there is not a need to separately track clients (apps),
  and where redirect URL attacks are protected by localhost redirect and the corporation restricting
  what software can run on the nodes to avoid redirect-url-MITMs.
- client_id/secret is distributed as part of introducing users to the cluster (e.g. email telling you what to put in your .kube/config
- using same client_id for kubectl and apiserver
- with a single client_id allocated for the company/team and distributed offline ; for I am okay with the design and
cross-client on Google
- client_id for kubectl and different client_id for apiserver.
- since Google only supports same-project client-ids, this is probably for a single organization, but they can at least get some benefit from the apiserver client_secret being confidential
- other IdP may not support cross-client today, but maybe someday.
cross-client on Dex
- CoreOS working on something for Dex similar to Google cross-client
- they plan to have good setup instructions in open source for Dex + Kubernetes, and for it to be a top choice as an on-prem IdP.
- they may have a consent flow that is more customized to the kubernetes use case.

So, users have multiple IdP choices and the flow is OIDC-compliant.

yifan-gu · 2016-05-18T23:53:30Z

@k8s-bot test this issue: #25791

This commit handles: * Passing ID Token as Bearer token * Refreshing of tokens using refresh-tokens * Persisting refreshed tokens * ability to add arbitrary extra scopes via config * this is what enables the cross-client/azp stuff

This makes it easier to test other OIDC code.

* Use an interface for OIDC Client, so that we're testing the behavior of the client, not the go-oidc package itself * add backoff and retry when server rejects token

tests that tokens gets refreshed, passed along as bearers, etc.

k8s-bot · 2016-05-19T00:45:18Z

GCE e2e build/test passed for commit f575f89.

k8s-bot · 2016-05-19T18:30:22Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

bobbyrullo · 2016-05-19T23:12:13Z

@brendandburns - could you please tag this as p1 and 1.3 as per the Effort tracking Spreadsheet? It keeps getting bumped in the queue....

bobbyrullo · 2016-05-24T15:59:09Z

@erictune - I promised you some docs, but could use some guidance on where to put things.

Setting up OIDC on the API Server goes here, but not sure where to put the following:

Document the structure of the AuthProvider config; this is a map[string]string whose configuration depends on the specific AuthProvider plugin. There's no precedence here, since AuthProvider is new. I could put a README.md in plugin/pkg/client/auth/oidc/. Does that sound good? ( @cjcullen ?)
Notes on secure deployment of OIDC in your cluster (the stuff we talked about, @erictune ) - This makes sense to go in http://kubernetes.io/docs/admin/authentication as well yes? Maybe a separate document linked from the root auth page?

cjcullen · 2016-05-24T16:42:42Z

Re 1) I think a README next to the plugin with the meanings of the config fields is good. I should also put one together for the GCP plugin.

pwittrock · 2016-06-17T19:32:05Z

@bobbyrullo
@erictune
@yifan-gu

Would you provide an update on the status for the documentation for this feature as well as add any PRs as they are created?

Not Started / In Progress / In Review / Done

Thanks
@pwittrock

erictune · 2016-07-07T21:49:40Z

Done:
https://github.com/kubernetes/kubernetes.github.io/blob/master/docs/admin/authentication.md

hanikesn · 2016-08-15T13:30:37Z

plugin/pkg/client/auth/oidc/oidc.go

+		return nil, errors.New("Must provide idp-issuer-url")
+	}
+
+	providerCfg, err := oidc.FetchProviderConfig(hc, strings.TrimSuffix(issuer, "/"))


@bobbyrullo This seems to create the same issues as #29749

@bobbyrullo This seems to create the same issues as #29749

@ericchiang not sure if there's something you need to do here.

Sorry, I haven't looked at this code for a second. Will send a PR to not trim the trailing slash.

FIx CI for 3.11 Origin-commit: 64b44fbcfe4b692474a72d1ce2fa13c53a48ec85

googlebot added the cla: no label May 6, 2016

k8s-github-robot assigned brendandburns May 6, 2016

k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. kind/design Categorizes issue or PR as related to design. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. release-note-label-needed labels May 6, 2016

k8s-github-robot assigned rmmh and ixdy and unassigned brendandburns and rmmh May 6, 2016

ixdy removed their assignment May 6, 2016

fabioy assigned erictune May 9, 2016

erictune reviewed May 11, 2016
View reviewed changes

bobbyrullo force-pushed the deps branch from fbe451c to c15ca93 Compare May 11, 2016 23:51

googlebot added cla: yes and removed cla: no labels May 11, 2016

bobbyrullo force-pushed the deps branch 2 times, most recently from f4e8467 to e6f6776 Compare May 18, 2016 22:11

yifan-gu added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 18, 2016

erictune assigned yifan-gu and unassigned erictune May 18, 2016

Bobby Rullo added 5 commits May 18, 2016 17:02

Implement new OIDC client AuthProvider

f2135bd

This commit handles: * Passing ID Token as Bearer token * Refreshing of tokens using refresh-tokens * Persisting refreshed tokens * ability to add arbitrary extra scopes via config * this is what enables the cross-client/azp stuff

Refactor test oidc provider into its own package

c990462

This makes it easier to test other OIDC code.

add tests for newOIDCAuthProvider

e85940e

OIDC authprovider more testable, and add backoff

94ffa34

* Use an interface for OIDC Client, so that we're testing the behavior of the client, not the go-oidc package itself * add backoff and retry when server rejects token

add tests for the OIDC WrapTransport

f575f89

tests that tokens gets refreshed, passed along as bearers, etc.

bobbyrullo force-pushed the deps branch from e6f6776 to f575f89 Compare May 19, 2016 00:04

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 19, 2016

yifan-gu added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 19, 2016

erictune added this to the v1.3 milestone May 19, 2016

mikedanese merged commit 7170c89 into kubernetes:master May 20, 2016

hanikesn reviewed Aug 15, 2016
View reviewed changes

openshift-publish-robot pushed a commit to openshift/kubernetes that referenced this pull request Jul 14, 2020

Merge pull request kubernetes#25270 from soltysh/debug_pr

87690b6

FIx CI for 3.11 Origin-commit: 64b44fbcfe4b692474a72d1ce2fa13c53a48ec85

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement OIDC client AuthProvider #25270

Implement OIDC client AuthProvider #25270

bobbyrullo commented May 6, 2016 •

edited

Loading

googlebot commented May 6, 2016

k8s-bot commented May 6, 2016

k8s-bot commented May 6, 2016

k8s-bot commented May 6, 2016

fabioy commented May 9, 2016

erictune commented May 11, 2016

erictune May 11, 2016

erictune commented May 11, 2016

bobbyrullo commented May 11, 2016

googlebot commented May 11, 2016

k8s-github-robot commented May 18, 2016

yifan-gu commented May 18, 2016

yifan-gu commented May 18, 2016

yifan-gu commented May 18, 2016

erictune commented May 18, 2016

yifan-gu commented May 18, 2016

k8s-bot commented May 19, 2016

k8s-bot commented May 19, 2016

bobbyrullo commented May 19, 2016

bobbyrullo commented May 24, 2016

cjcullen commented May 24, 2016

pwittrock commented Jun 17, 2016

erictune commented Jul 7, 2016

hanikesn Aug 15, 2016

deads2k Aug 15, 2016

ericchiang Aug 15, 2016

Implement OIDC client AuthProvider #25270

Implement OIDC client AuthProvider #25270

Conversation

bobbyrullo commented May 6, 2016 • edited Loading

googlebot commented May 6, 2016

k8s-bot commented May 6, 2016

k8s-bot commented May 6, 2016

k8s-bot commented May 6, 2016

fabioy commented May 9, 2016

erictune commented May 11, 2016

erictune May 11, 2016

Choose a reason for hiding this comment

erictune commented May 11, 2016

bobbyrullo commented May 11, 2016

googlebot commented May 11, 2016

k8s-github-robot commented May 18, 2016

yifan-gu commented May 18, 2016

yifan-gu commented May 18, 2016

yifan-gu commented May 18, 2016

erictune commented May 18, 2016

yifan-gu commented May 18, 2016

k8s-bot commented May 19, 2016

k8s-bot commented May 19, 2016

bobbyrullo commented May 19, 2016

bobbyrullo commented May 24, 2016

cjcullen commented May 24, 2016

pwittrock commented Jun 17, 2016

erictune commented Jul 7, 2016

hanikesn Aug 15, 2016

Choose a reason for hiding this comment

deads2k Aug 15, 2016

Choose a reason for hiding this comment

ericchiang Aug 15, 2016

Choose a reason for hiding this comment

bobbyrullo commented May 6, 2016 •

edited

Loading