Refreshed OIDC tokens should be persisted to configuration file

[benlangfeld]

OIDC auth support was added in https://github.com/abonas/kubeclient/pull/396. It includes support for expired ID tokens (expanded in https://github.com/abonas/kubeclient/pull/407 to handle certificate rotation on the authentication server) and refreshes them when fetching a context from configuration.

The README states:

Note: id-tokens retrieved via this provider are not written back to the $KUBECONFIG file as they would be when using kubectl.

This in itself would be ok if it were the whole story, since it would imply simply repeated token refreshes on each use.

What's not taken into account here is that refresh tokens are single-use. If kubeclient refreshes an ID token, it "spends" the corresponding refresh token such that it can never be used again, but it doesn't record the new refresh token anywhere. This means that any other tool which uses the configuration file, or indeed a follow-up use of the config file by kubeclient (even in the same process, even using the very same instance of Kubeclient::Config), will encounter both an expired ID token and an invalid refresh token, and will raise an exception like this one:

...
         4: from /var/lib/gems/2.5.0/gems/kubeclient-4.3.0/lib/kubeclient/config.rb:162:in `fetch_user_auth_options'
         3: from /var/lib/gems/2.5.0/gems/kubeclient-4.3.0/lib/kubeclient/oidc_auth_provider.rb:37:in `token'
         2: from /var/lib/gems/2.5.0/gems/rack-oauth2-1.9.3/lib/rack/oauth2/client.rb:122:in `access_token!'
         1: from /var/lib/gems/2.5.0/gems/rack-oauth2-1.9.3/lib/rack/oauth2/client.rb:148:in `handle_response'
/var/lib/gems/2.5.0/gems/rack-oauth2-1.9.3/lib/rack/oauth2/client.rb:171:in `handle_error_response': invalid_request :: Refresh token is invalid or has already been claimed by another client. (Rack::OAuth2::Client::Error)

In order to behave correctly, similarly to kubectl, kubeclient must update the configuration file it consumes with the new tokens when a refresh happens.

Is there any reason not to implement this? Does anyone have any thoughts on how it might be implemented? I made something of a start at https://github.com/abonas/kubeclient/pull/408, but I'm not really happy with the direction it's going right now. I wonder if it should be the responsibility of Kubeclient::OIDCAuthProvider to update the Kubeclient::Config instance's values?

[cben]

What's not taken into account here is that refresh tokens are single-use.

Interesting! Is that always true in OIDC?
I'm now using Keycloak (in an non-kubernetes context) which I believe implements OIDC and I think it's refresh token can be reused (?)

[benlangfeld]

Is that always true in OIDC?

I don't think every authorization server implementation includes it, but the one we're using does.

My testing:

require 'openid_connect'

issuer_url = '<snip/>'
client_id = 'APP-HQ'
client_secret = '<snip/>'
refresh_token = '<snip/>'

discovery = OpenIDConnect::Discovery::Provider::Config.discover!(issuer_url)

client = OpenIDConnect::Client.new(
  identifier: client_id,
  secret: client_secret,
  authorization_endpoint: discovery.authorization_endpoint,
  token_endpoint: discovery.token_endpoint,
  userinfo_endpoint: discovery.userinfo_endpoint
)
client.refresh_token = refresh_token

2.times do |attempt|
  puts "Getting new token (attempt #{attempt})..."
  access_token = client.access_token!

  puts "ID Token: #{access_token.id_token}"
  puts "Refresh token: #{access_token.refresh_token}"
end

➭ ruby refresh.rb
Getting new token (attempt 0)...
ID Token: <snip/>
Refresh token: <snip/>
Getting new token (attempt 1)...
Traceback (most recent call last):
        5: from refresh.rb:19:in `<main>'
        4: from refresh.rb:19:in `times'
        3: from refresh.rb:21:in `block in <main>'
        2: from /usr/local/Cellar/rbenv/1.0.0/versions/2.5.0/lib/ruby/gems/2.5.0/gems/rack-oauth2-1.9.3/lib/rack/oauth2/client.rb:122:in `access_token!'
        1: from /usr/local/Cellar/rbenv/1.0.0/versions/2.5.0/lib/ruby/gems/2.5.0/gems/rack-oauth2-1.9.3/lib/rack/oauth2/client.rb:148:in `handle_response'
/usr/local/Cellar/rbenv/1.0.0/versions/2.5.0/lib/ruby/gems/2.5.0/gems/rack-oauth2-1.9.3/lib/rack/oauth2/client.rb:171:in `handle_error_response': invalid_request :: Refresh token is invalid or has already been claimed by another client. (Rack::OAuth2::Client::Error)

On this second attempt, Dex logged the following:

time="2019-04-17T13:52:25Z" level=error msg="refresh token with id uppqscpzhkx4uxk5espb2gwk3 claimed twice" 

[cben]

Ack. There are several risks (network cut off during renewal => locked out; several processes reading same token from file and all trying to renew & update the file might be messy...). But there's no alternative.

It does add an important constraint to #393 -- to support renewal without re-creating Client objects, renewal state better be centralized, allowing single renewal object to serve multiple Client objects. (I kinda knew that already, but extra motivation / constraint is helpful.)

[joshbranham]

This is of interest to me as well. Looking to create some kubectl plugins using this library, and we utilize OIDC authentication. If I am not mistaken, if a user runs kubectl <my-plugin> which utilizes this library, and it refreshes the token, then if they go to use kubectl get pods say an hour+ later, kubectl will be unable to refresh the token because the currently defined refresh token in the kubeconfig file has been used?