From 0a58137e1015c3791618b241d9bd5a2149feb459 Mon Sep 17 00:00:00 2001 From: Ben Langfeld Date: Mon, 15 Apr 2019 16:54:56 -0300 Subject: [PATCH 1/3] Consider OIDC tokens with an unknown kid to be expired When the authorization server rotates key IDs, we might encounter a token with a kid which the server no longer reports in discovery. Currently, json-jwt raises an exception and the user must expend a lot of effort debugging. We can help by simply assuming the token is expired and forcing token refresh. --- lib/kubeclient/oidc_auth_provider.rb | 16 +++++++++++----- test/test_oidc_auth_provider.rb | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/lib/kubeclient/oidc_auth_provider.rb b/lib/kubeclient/oidc_auth_provider.rb index 08605525..ffdfd7e2 100644 --- a/lib/kubeclient/oidc_auth_provider.rb +++ b/lib/kubeclient/oidc_auth_provider.rb @@ -21,9 +21,7 @@ def token(provider_config) discovery = OpenIDConnect::Discovery::Provider::Config.discover! issuer_url if provider_config.key? 'id-token' - id_token = OpenIDConnect::ResponseObject::IdToken.decode provider_config['id-token'], - discovery.jwks - return provider_config['id-token'] unless expired?(id_token) + return provider_config['id-token'] unless expired?(provider_config['id-token'], discovery) end client = OpenIDConnect::Client.new( @@ -37,9 +35,17 @@ def token(provider_config) client.access_token!.id_token end - def expired?(id_token) + def expired?(id_token, discovery) + decoded_token = OpenIDConnect::ResponseObject::IdToken.decode( + id_token, + discovery.jwks + ) # If token expired or expiring within 60 seconds - Time.now.to_i + 60 > id_token.exp.to_i + Time.now.to_i + 60 > decoded_token.exp.to_i + rescue JSON::JWK::Set::KidNotFound + # Token cannot be verified: the kid it was signed with is not available for discovery + # Consider it expired and fetch a new one. + true end end end diff --git a/test/test_oidc_auth_provider.rb b/test/test_oidc_auth_provider.rb index 9f16ff67..cdf325e9 100644 --- a/test/test_oidc_auth_provider.rb +++ b/test/test_oidc_auth_provider.rb @@ -57,6 +57,25 @@ def test_missing_id_token end end + def test_token_with_unknown_kid + OpenIDConnect::Discovery::Provider::Config.stub(:discover!, discovery_mock) do + OpenIDConnect::ResponseObject::IdToken.stub( + :decode, ->(_token, _jwks) { raise JSON::JWK::Set::KidNotFound } + ) do + OpenIDConnect::Client.stub(:new, openid_client_mock) do + retrieved_id_token = Kubeclient::OIDCAuthProvider.token( + 'client-id' => @client_id, + 'client-secret' => @client_secret, + 'id-token' => @id_token, + 'idp-issuer-url' => @idp_issuer_url, + 'refresh-token' => @refresh_token + ) + assert_equal(@new_id_token, retrieved_id_token) + end + end + end + end + private def openid_client_mock From 32a5c0233a063067b8e2c5bba50e3583ff1820ef Mon Sep 17 00:00:00 2001 From: Ben Langfeld Date: Tue, 16 Apr 2019 16:53:11 -0300 Subject: [PATCH 2/3] Exposes full access tokens from OIDCAuthProvider.token This allows consumers to get access to the new refresh token when the ID token is refreshed. This is important because refresh tokens are single-use. If the consumer code needs to refresh the ID token a second time, it will need to use an updated refresh token. --- lib/kubeclient/config.rb | 5 ++++- lib/kubeclient/oidc_auth_provider.rb | 7 +++++-- test/test_config.rb | 6 +++++- test/test_oidc_auth_provider.rb | 8 ++++---- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/lib/kubeclient/config.rb b/lib/kubeclient/config.rb index 339f4ab0..3d739501 100644 --- a/lib/kubeclient/config.rb +++ b/lib/kubeclient/config.rb @@ -159,7 +159,10 @@ def fetch_user_auth_options(user) when 'gcp' then Kubeclient::GoogleApplicationDefaultCredentials.token when 'oidc' - then Kubeclient::OIDCAuthProvider.token(auth_provider['config']) + then + Kubeclient::OIDCAuthProvider.token( + auth_provider['config'] + ).id_token end else %w[username password].each do |attr| diff --git a/lib/kubeclient/oidc_auth_provider.rb b/lib/kubeclient/oidc_auth_provider.rb index ffdfd7e2..a08077ca 100644 --- a/lib/kubeclient/oidc_auth_provider.rb +++ b/lib/kubeclient/oidc_auth_provider.rb @@ -6,6 +6,8 @@ class OIDCAuthProvider class OpenIDConnectDependencyError < LoadError # rubocop:disable Lint/InheritException end + AlreadyValidAccessToken = Struct.new(:id_token) + class << self def token(provider_config) begin @@ -21,7 +23,8 @@ def token(provider_config) discovery = OpenIDConnect::Discovery::Provider::Config.discover! issuer_url if provider_config.key? 'id-token' - return provider_config['id-token'] unless expired?(provider_config['id-token'], discovery) + access_token = AlreadyValidAccessToken.new(provider_config['id-token']) + return access_token unless expired?(access_token.id_token, discovery) end client = OpenIDConnect::Client.new( @@ -32,7 +35,7 @@ def token(provider_config) userinfo_endpoint: discovery.userinfo_endpoint ) client.refresh_token = provider_config['refresh-token'] - client.access_token!.id_token + client.access_token! end def expired?(id_token, discovery) diff --git a/test/test_config.rb b/test/test_config.rb index b769e3f3..1c45e588 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -153,7 +153,11 @@ def test_oidc_auth_provider 'id-token' => 'fake-id-token', 'idp-issuer-url' => 'https://accounts.google.com', 'refresh-token' => 'fake-refresh-token') - .returns('token1') + .returns( + Kubeclient::OIDCAuthProvider::AlreadyValidAccessToken.new( + 'token1' + ) + ) .once parsed = YAML.safe_load(File.read(config_file('oidcauth.kubeconfig'))) config = Kubeclient::Config.new(parsed, nil) diff --git a/test/test_oidc_auth_provider.rb b/test/test_oidc_auth_provider.rb index cdf325e9..ad42348e 100644 --- a/test/test_oidc_auth_provider.rb +++ b/test/test_oidc_auth_provider.rb @@ -21,7 +21,7 @@ def test_expired_token 'id-token' => @id_token, 'idp-issuer-url' => @idp_issuer_url, 'refresh-token' => @refresh_token - ) + ).id_token assert_equal(@new_id_token, retrieved_id_token) end end @@ -37,7 +37,7 @@ def test_valid_token 'id-token' => @id_token, 'idp-issuer-url' => @idp_issuer_url, 'refresh-token' => @refresh_token - ) + ).id_token assert_equal(@id_token, retrieved_id_token) end end @@ -51,7 +51,7 @@ def test_missing_id_token 'client-secret' => @client_secret, 'idp-issuer-url' => @idp_issuer_url, 'refresh-token' => @refresh_token - ) + ).id_token assert_equal(@new_id_token, retrieved_id_token) end end @@ -69,7 +69,7 @@ def test_token_with_unknown_kid 'id-token' => @id_token, 'idp-issuer-url' => @idp_issuer_url, 'refresh-token' => @refresh_token - ) + ).id_token assert_equal(@new_id_token, retrieved_id_token) end end From 65f7cf0bb43e0ca784d4ba64c9e43cf2d5c68ca7 Mon Sep 17 00:00:00 2001 From: Ben Langfeld Date: Tue, 16 Apr 2019 17:03:01 -0300 Subject: [PATCH 3/3] Exposes OIDC refresh token in Config::Context#auth_options In case the ID token is refreshed, client code will also need to know the new refresh token in order to pass it forward. --- lib/kubeclient/config.rb | 17 ++++++++--------- lib/kubeclient/oidc_auth_provider.rb | 7 +++++-- test/test_config.rb | 9 +++++++-- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/kubeclient/config.rb b/lib/kubeclient/config.rb index 3d739501..d4c28642 100644 --- a/lib/kubeclient/config.rb +++ b/lib/kubeclient/config.rb @@ -155,15 +155,14 @@ def fetch_user_auth_options(user) options[:bearer_token] = Kubeclient::ExecCredentials.token(exec_opts) elsif user.key?('auth-provider') auth_provider = user['auth-provider'] - options[:bearer_token] = case auth_provider['name'] - when 'gcp' - then Kubeclient::GoogleApplicationDefaultCredentials.token - when 'oidc' - then - Kubeclient::OIDCAuthProvider.token( - auth_provider['config'] - ).id_token - end + case auth_provider['name'] + when 'gcp' + options[:bearer_token] = Kubeclient::GoogleApplicationDefaultCredentials.token + when 'oidc' + access_token = Kubeclient::OIDCAuthProvider.token(auth_provider['config']) + options[:bearer_token] = access_token.id_token + options[:refresh_token] = access_token.refresh_token + end else %w[username password].each do |attr| options[attr.to_sym] = user[attr] if user.key?(attr) diff --git a/lib/kubeclient/oidc_auth_provider.rb b/lib/kubeclient/oidc_auth_provider.rb index a08077ca..a571c76b 100644 --- a/lib/kubeclient/oidc_auth_provider.rb +++ b/lib/kubeclient/oidc_auth_provider.rb @@ -6,7 +6,7 @@ class OIDCAuthProvider class OpenIDConnectDependencyError < LoadError # rubocop:disable Lint/InheritException end - AlreadyValidAccessToken = Struct.new(:id_token) + AlreadyValidAccessToken = Struct.new(:id_token, :refresh_token) class << self def token(provider_config) @@ -23,7 +23,10 @@ def token(provider_config) discovery = OpenIDConnect::Discovery::Provider::Config.discover! issuer_url if provider_config.key? 'id-token' - access_token = AlreadyValidAccessToken.new(provider_config['id-token']) + access_token = AlreadyValidAccessToken.new( + provider_config['id-token'], + provider_config['refresh-token'] + ) return access_token unless expired?(access_token.id_token, discovery) end diff --git a/test/test_config.rb b/test/test_config.rb index 1c45e588..feb9ce11 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -155,13 +155,18 @@ def test_oidc_auth_provider 'refresh-token' => 'fake-refresh-token') .returns( Kubeclient::OIDCAuthProvider::AlreadyValidAccessToken.new( - 'token1' + 'token1', + 'new-refresh-token' ) ) .once parsed = YAML.safe_load(File.read(config_file('oidcauth.kubeconfig'))) config = Kubeclient::Config.new(parsed, nil) - config.context(config.contexts.first) + context = config.context(config.contexts.first) + assert_equal( + { bearer_token: 'token1', refresh_token: 'new-refresh-token' }, + context.auth_options + ) end private