Implement OIDC Auth Provider #396

rhysm · 2019-02-25T10:27:18Z

Adds ability for Kubeclient to utilize the oidc auth-provider both by
re-using the id-token that is inside the kubeconfig if it is not expired
and refreshing it if the token has passed expiry.

Adds ability for Kubeclient to utilize the oidc auth-provider both by re-using the id-token that is inside the kubeconfig if it is not expired and refreshing it if the token has passed expiry.

cben · 2019-02-26T21:24:34Z

Sorry for delay, reviewing...
This is cool!
k8s docs: https://kubernetes.io/docs/reference/access-authn-authz/authentication/#openid-connect-tokens

cben

Excellent 💯, very minor comments

cben · 2019-02-27T09:15:50Z

README.md

+If the id-token specified in your `$KUBECONFIG` file has not expired the OIDC Auth Provider will simply use that token.
+If it has expired then the provider will refresh the token and use that.
+
+Currently tokens are only valid for one hour and the provider will not automatically refresh them. Fresh id-tokens are also


Is it always 1 hour? Isn't expiration up to the oidc server?

Updated for clarity.

cben · 2019-02-27T09:16:20Z

README.md

+#### OIDC Auth Provider
+
+If the cluster you are using has OIDC authentication enabed you can use the `openid_connect` gem to refresh
+id-tokens when they have expired.


See next comment about the wording "refresh".

Updated for clarity.

cben · 2019-02-27T10:50:14Z

lib/kubeclient/oidc_auth_provider.rb

+
+      def expired?(id_token)
+        # If token expired or expiring within 60 seconds
+        Time.now.to_i + 60 > id_token.exp.to_i


Is this timezone independent? 👍 looks good, Time.now knows its timezone and Time.now.to_i is same either way:

~ $ env TZ= ruby -e 'p Time.now' 2019-02-27 10:47:22 +0000 ~ $ ruby -e 'p Time.now' 2019-02-27 12:47:26 +0200 ~ $ env TZ= ruby -e 'p Time.now.to_i' 1551264454 ~ $ env ruby -e 'p Time.now.to_i' 1551264463

The openid_connect gem performs the same check except for the 60 second grace so I believe it will be fine.

cben · 2019-02-27T11:01:05Z

lib/kubeclient/oidc_auth_provider.rb

+    end
+
+    class << self
+      def token(auth_provider)


[optional nit, your call] The name auth_provider sounds like this would contain the ['auth-provider'] part of the config, but it contains the ['auth-provider']['config'] sub-part. Maybe config? auth_provider_config (ouch)? provider_config?

Good point, I've gone with provider_config.

cben · 2019-02-27T11:06:25Z

lib/kubeclient/oidc_auth_provider.rb

+        issuer_url = auth_provider['idp-issuer-url']
+        discovery = OpenIDConnect::Discovery::Provider::Config.discover! issuer_url
+
+        id_token = OpenIDConnect::ResponseObject::IdToken.decode auth_provider['id-token'],


What if the config doesn't contain id-token at all? (Is that possible?)
As we can always obtain a new one, let's allow this?

I'm not sure how you'd get into that state but kubectl supports it so I've implemented it too.

cben · 2019-02-27T11:10:57Z

README.md

+like [`dexter`](https://github.com/gini/dexter) in order to configure the auth-provider in your `$KUBECONFIG` file.
+
+If the id-token specified in your `$KUBECONFIG` file has not expired the OIDC Auth Provider will simply use that token.
+If it has expired then the provider will refresh the token and use that.


The wordings "will refresh" ... "will not automatically refresh" sounds confusing on a quick read.
I propose here saying:

If you use Config.context(...).auth_options and the kubeconfig file has user: {auth-provider: {name: gcp}}, kubeclient will automatically obtain a token (or use id-token if still valid)

to me "obtain" is one-time action, doesn't imply ability to keep refreshing it, nor sounds as action done to the token so less expectation it'll write it back to the config file (the disclaimer below is still good).

The other change is because this section talks abstractly about "the OIDC Auth Provider" but I wanted to say specifically how to invoke this (Client doesn't do this, Config does)

(one day if we implement #393, we can reword these sections in terms of "refresh" or "renewal")

Updated README for more clarity, let me know what you think.

…better clarity

cben

perfect!

cben · 2019-02-28T23:55:32Z

BTW, a security question:
Kubeclient supports opt-out from some "dangerous" config handling — reading local files, and exec:.
(For scenarios where you take config from untrusted source, which I warn against anyway, and I haven't seen anyone doing, but on general principles...)
Do you think this opt-out should also disable OIDC (and GCP #394) auth-providers?

On one hand, making a network request is not risky to your machine(?).
On the other, that's still "acting on data" vs passively reading it. However the goal of Config is to feed it to Client which will act on a lot of it by network requests anyway so that's hardly a criterion...

added changelog for ManageIQ#396

cben · 2019-03-03T11:09:28Z

released in gem 4.3.0.

rhysm added 3 commits February 25, 2019 23:23

Implement OIDC Auth Provider

ebb12da

Adds ability for Kubeclient to utilize the oidc auth-provider both by re-using the id-token that is inside the kubeconfig if it is not expired and refreshing it if the token has passed expiry.

Ignore Metrics/BlockLength cop in kubeclient.gemspec

d787766

Fix Complexity related rubocops in fetch_user_auth_options

741f363

cben reviewed Feb 27, 2019

View reviewed changes

rhysm added 2 commits March 1, 2019 12:27

Add information about OIDCAuthProvider to README.md

184b7d3

Support missing id-token for OIDC provider. Update variable name for …

f0a750c

…better clarity

rhysm force-pushed the oidcAuthProvider branch from 22c43ef to f0a750c Compare February 28, 2019 23:42

cben approved these changes Feb 28, 2019

View reviewed changes

cben merged commit c335035 into ManageIQ:master Feb 28, 2019

cben mentioned this pull request Mar 1, 2019

Don't promise that Config#context is what acts on exec:/auth-provider: #400

Merged

cben added a commit to cben/kubeclient that referenced this pull request Mar 3, 2019

CHANGELOG for 4.3.0

067ca33

added changelog for ManageIQ#396

cben mentioned this pull request Mar 3, 2019

Release 4.3.0 #401

Merged

benlangfeld mentioned this pull request Apr 16, 2019

Refreshed OIDC tokens should be persisted to configuration file #409

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement OIDC Auth Provider #396

Implement OIDC Auth Provider #396

rhysm commented Feb 25, 2019

cben commented Feb 26, 2019 •

edited

Loading

cben left a comment

cben Feb 27, 2019

rhysm Feb 28, 2019

cben Feb 27, 2019

rhysm Feb 28, 2019

cben Feb 27, 2019

rhysm Feb 28, 2019

cben Feb 27, 2019

rhysm Feb 28, 2019

cben Feb 27, 2019

rhysm Feb 28, 2019

cben Feb 27, 2019 •

edited

Loading

cben Feb 27, 2019

rhysm Feb 28, 2019

cben left a comment

cben commented Feb 28, 2019

cben commented Mar 3, 2019

Implement OIDC Auth Provider #396

Implement OIDC Auth Provider #396

Conversation

rhysm commented Feb 25, 2019

cben commented Feb 26, 2019 • edited Loading

cben left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cben Feb 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cben left a comment

Choose a reason for hiding this comment

cben commented Feb 28, 2019

cben commented Mar 3, 2019

cben commented Feb 26, 2019 •

edited

Loading

cben Feb 27, 2019 •

edited

Loading