From 01ae7f4b2969d9ef3bb69db818a7efffbb0ffb53 Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Thu, 2 Mar 2023 21:57:52 +0530 Subject: [PATCH] fix (kubernetes-client-api) : Track sources from where OAuthToken gets set (#4802) + Add enum OAuthTokenSource in Config which tell us about the source from which Config's OAuthToken was set. + Skip refresh in case current OAuthTokenSource is set to USER (the scenario reported in the linked issue) Signed-off-by: Rohan Kumar --- CHANGELOG.md | 1 + .../io/fabric8/kubernetes/client/Config.java | 47 +++++++- .../kubernetes/client/OAuthTokenSource.java | 50 ++++++++ .../client/utils/TokenRefreshInterceptor.java | 32 +++++- .../fabric8/kubernetes/client/ConfigTest.java | 77 ++++++++----- .../utils/TokenRefreshInterceptorTest.java | 108 ++++++++++++++---- .../openshift/client/OpenShiftConfig.java | 7 +- .../internal/OpenShiftOAuthInterceptor.java | 44 ++++--- .../OpenShiftOAuthInterceptorTest.java | 53 +++++++++ 9 files changed, 350 insertions(+), 69 deletions(-) create mode 100644 kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/OAuthTokenSource.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ed83128669..804e9b6cd65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ * Fix #4794: improving the semantics of manually calling informer stop * Fix #4797: OkHttpClientFactory.additionalConfig can be used to override the default OkHttp Dispatcher * Fix #4798: fix leader election release on cancel +* Fix #4802: config.refresh() erases token specified when building initial config * Fix #4815: (java-generator) create target download directory if it doesn't exist * Fix #4846: allowed for pod read / copy operations to distinguish when the target doesn't exist * Fix #4818: [java-generator] Escape `*/` in generated JavaDocs diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java index 1c01181ed58..7b9299be62d 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java @@ -180,6 +180,7 @@ public class Config { private int connectionTimeout = 10 * 1000; private int maxConcurrentRequests = DEFAULT_MAX_CONCURRENT_REQUESTS; private int maxConcurrentRequestsPerHost = DEFAULT_MAX_CONCURRENT_REQUESTS_PER_HOST; + private OAuthTokenSource oAuthTokenSource; private RequestConfig requestConfig = new RequestConfig(); @@ -330,7 +331,7 @@ public Config(String masterUrl, String apiVersion, String namespace, boolean tru loggingInterval, maxConcurrentRequests, maxConcurrentRequestsPerHost, false, httpProxy, httpsProxy, noProxy, errorMessages, userAgent, tlsVersions, websocketTimeout, websocketPingInterval, proxyUsername, proxyPassword, trustStoreFile, trustStorePassphrase, keyStoreFile, keyStorePassphrase, impersonateUsername, impersonateGroups, - impersonateExtras, null, null, DEFAULT_REQUEST_RETRY_BACKOFFLIMIT, DEFAULT_REQUEST_RETRY_BACKOFFINTERVAL, + impersonateExtras, null, null, null, DEFAULT_REQUEST_RETRY_BACKOFFLIMIT, DEFAULT_REQUEST_RETRY_BACKOFFINTERVAL, DEFAULT_UPLOAD_REQUEST_TIMEOUT); } @@ -344,7 +345,8 @@ public Config(String masterUrl, String apiVersion, String namespace, boolean tru String userAgent, TlsVersion[] tlsVersions, long websocketTimeout, long websocketPingInterval, String proxyUsername, String proxyPassword, String trustStoreFile, String trustStorePassphrase, String keyStoreFile, String keyStorePassphrase, String impersonateUsername, String[] impersonateGroups, Map> impersonateExtras, - OAuthTokenProvider oauthTokenProvider, Map customHeaders, int requestRetryBackoffLimit, + OAuthTokenProvider oauthTokenProvider, OAuthTokenSource oAuthTokenSource, Map customHeaders, + int requestRetryBackoffLimit, int requestRetryBackoffInterval, int uploadRequestTimeout) { this.apiVersion = apiVersion; this.namespace = namespace; @@ -361,6 +363,7 @@ public Config(String masterUrl, String apiVersion, String namespace, boolean tru this.username = username; this.password = password; this.oauthToken = oauthToken; + this.oAuthTokenSource = determineOAuthTokenSource(oauthToken, oAuthTokenSource, oauthTokenProvider); this.websocketPingInterval = websocketPingInterval; this.connectionTimeout = connectionTimeout; @@ -426,7 +429,11 @@ public static void configFromSysPropsOrEnvVars(Config config) { Utils.getSystemPropertyOrEnvVar(KUBERNETES_KEYSTORE_PASSPHRASE_PROPERTY, config.getKeyStorePassphrase())); config.setKeyStoreFile(Utils.getSystemPropertyOrEnvVar(KUBERNETES_KEYSTORE_FILE_PROPERTY, config.getKeyStoreFile())); - config.setOauthToken(Utils.getSystemPropertyOrEnvVar(KUBERNETES_OAUTH_TOKEN_SYSTEM_PROPERTY, config.getOauthToken())); + String tokenFromPropertyOrEnvVar = Utils.getSystemPropertyOrEnvVar(KUBERNETES_OAUTH_TOKEN_SYSTEM_PROPERTY, ""); + if (!tokenFromPropertyOrEnvVar.isEmpty()) { + config.setOauthToken(tokenFromPropertyOrEnvVar); + config.setOAuthTokenSource(OAuthTokenSource.SYSTEM_PROPERTIES_OR_ENVIRONMENT_VARIABLES); + } config.setUsername(Utils.getSystemPropertyOrEnvVar(KUBERNETES_AUTH_BASIC_USERNAME_SYSTEM_PROPERTY, config.getUsername())); config.setPassword(Utils.getSystemPropertyOrEnvVar(KUBERNETES_AUTH_BASIC_PASSWORD_SYSTEM_PROPERTY, config.getPassword())); @@ -552,6 +559,7 @@ private static boolean tryServiceAccount(Config config) { String serviceTokenCandidate = new String(Files.readAllBytes(saTokenPathFile.toPath())); LOGGER.debug("Found service account token at: [{}].", saTokenPathLocation); config.setOauthToken(serviceTokenCandidate); + config.setOAuthTokenSource(OAuthTokenSource.SERVICEACCOUNT_TOKEN_FILE); String txt = "Configured service account doesn't have access. Service account may have been revoked."; config.getErrorMessages().put(401, "Unauthorized! " + txt); config.getErrorMessages().put(403, "Forbidden!" + txt); @@ -632,6 +640,9 @@ public static Config fromKubeconfig(String context, String kubeconfigContents, S * @return */ public Config refresh() { + if (isTokenNonRefreshable()) { + return this; + } final String currentContextName = this.getCurrentContext() != null ? this.getCurrentContext().getName() : null; if (this.autoConfigure) { return Config.autoConfigure(currentContextName); @@ -731,6 +742,7 @@ private static boolean loadFromKubeconfig(Config config, String context, String config.setClientKeyData(currentAuthInfo.getClientKeyData()); config.setClientKeyAlgo(getKeyAlgorithm(config.getClientKeyFile(), config.getClientKeyData())); config.setOauthToken(currentAuthInfo.getToken()); + config.setOAuthTokenSource(OAuthTokenSource.KUBECONFIG); config.setUsername(currentAuthInfo.getUsername()); config.setPassword(currentAuthInfo.getPassword()); @@ -740,9 +752,11 @@ private static boolean loadFromKubeconfig(Config config, String context, String if (!Utils.isNullOrEmpty(currentAuthInfo.getAuthProvider().getConfig().get(ACCESS_TOKEN))) { // GKE token config.setOauthToken(currentAuthInfo.getAuthProvider().getConfig().get(ACCESS_TOKEN)); + config.setOAuthTokenSource(OAuthTokenSource.KUBECONFIG); } else if (!Utils.isNullOrEmpty(currentAuthInfo.getAuthProvider().getConfig().get(ID_TOKEN))) { // OpenID Connect token config.setOauthToken(currentAuthInfo.getAuthProvider().getConfig().get(ID_TOKEN)); + config.setOAuthTokenSource(OAuthTokenSource.KUBECONFIG); } } } else if (config.getOauthTokenProvider() == null) { // https://kubernetes.io/docs/reference/access-authn-authz/authentication/#client-go-credential-plugins @@ -751,6 +765,7 @@ private static boolean loadFromKubeconfig(Config config, String context, String ExecCredential ec = getExecCredentialFromExecConfig(exec, configFile); if (ec != null && ec.status != null && ec.status.token != null) { config.setOauthToken(ec.status.token); + config.setOAuthTokenSource(OAuthTokenSource.EXEC_CREDENTIAL_PLUGIN); } else { LOGGER.warn("No token returned"); } @@ -1405,6 +1420,18 @@ public boolean getAutoConfigure() { return autoConfigure; } + public void setOAuthTokenSource(OAuthTokenSource oAuthTokenSource) { + this.oAuthTokenSource = oAuthTokenSource; + } + + public OAuthTokenSource getOAuthTokenSource() { + return oAuthTokenSource; + } + + public boolean isTokenNonRefreshable() { + return oAuthTokenSource != null && oAuthTokenSource.equals(OAuthTokenSource.USER); + } + /** * Returns all the {@link NamedContext}s that exist in the kube config * @@ -1476,4 +1503,18 @@ public void setAutoConfigure(boolean autoConfigure) { this.autoConfigure = autoConfigure; } + private static OAuthTokenSource determineOAuthTokenSource(String oAuthToken, OAuthTokenSource oAuthTokenSource, + OAuthTokenProvider oAuthTokenProvider) { + if (oAuthTokenProvider != null) { + return OAuthTokenSource.OAUTHTOKEN_PROVIDER; + } + if (oAuthToken != null) { + if (Utils.getSystemPropertyOrEnvVar(KUBERNETES_OAUTH_TOKEN_SYSTEM_PROPERTY) != null) { + return OAuthTokenSource.SYSTEM_PROPERTIES_OR_ENVIRONMENT_VARIABLES; + } else { + return OAuthTokenSource.USER; + } + } + return oAuthTokenSource; + } } diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/OAuthTokenSource.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/OAuthTokenSource.java new file mode 100644 index 00000000000..dfbdc34fd7b --- /dev/null +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/OAuthTokenSource.java @@ -0,0 +1,50 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.kubernetes.client; + +/** + * Enum to track source of OAuthToken + */ +public enum OAuthTokenSource { + /** + * OAuthToken that comes from Config's OAuthTokenProvider. If token is coming from this source + * it would not be considered for refresh. + */ + OAUTHTOKEN_PROVIDER, + /** + * OAuthToken directly provided by user during building initial Config. It would not be considered + * for refresh. It is responsibility of caller to refresh it on its own. + */ + USER, + /** + * OAuthToken picked up from System properties or environment variables + */ + SYSTEM_PROPERTIES_OR_ENVIRONMENT_VARIABLES, + /** + * OAuthToken picked up from KubeConfig file + */ + KUBECONFIG, + /** + * OAuthToken picked from + * client-go + * credential plugins + */ + EXEC_CREDENTIAL_PLUGIN, + /** + * OAuthToken picked from mounted ServiceAccount file inside container + */ + SERVICEACCOUNT_TOKEN_FILE +} diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptor.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptor.java index 78311265138..bc0b9b2617c 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptor.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptor.java @@ -16,6 +16,7 @@ package io.fabric8.kubernetes.client.utils; import io.fabric8.kubernetes.client.Config; +import io.fabric8.kubernetes.client.OAuthTokenSource; import io.fabric8.kubernetes.client.http.BasicBuilder; import io.fabric8.kubernetes.client.http.HttpClient; import io.fabric8.kubernetes.client.http.HttpRequest; @@ -59,7 +60,7 @@ public void before(BasicBuilder headerBuilder, HttpRequest request, RequestTags if (Utils.isNotNullOrEmpty(config.getOauthToken())) { headerBuilder.header(AUTHORIZATION, "Bearer " + config.getOauthToken()); } - if (isTimeToRefresh()) { + if (!config.isTokenNonRefreshable() && isTimeToRefresh()) { refreshToken(headerBuilder); } } @@ -77,6 +78,15 @@ public CompletableFuture afterFailure(BasicBuilder headerBuilder, HttpR if (isBasicAuth()) { return CompletableFuture.completedFuture(false); } + if (config.isTokenNonRefreshable()) { + return CompletableFuture.completedFuture(false); + } + if (config.getOAuthTokenSource() != null && config.getOAuthTokenSource().equals(OAuthTokenSource.OAUTHTOKEN_PROVIDER)) { + String tokenFromProvider = config.getOauthTokenProvider().getToken(); + if (tokenFromProvider != null && !tokenFromProvider.isEmpty()) { + return CompletableFuture.completedFuture(overrideNewAccessTokenToConfig(tokenFromProvider, headerBuilder, config)); + } + } if (response.code() == HttpURLConnection.HTTP_UNAUTHORIZED) { return refreshToken(headerBuilder); } @@ -85,13 +95,28 @@ public CompletableFuture afterFailure(BasicBuilder headerBuilder, HttpR private CompletableFuture refreshToken(BasicBuilder headerBuilder) { Config newestConfig = config.refresh(); + if (shouldUseTokenFromNewestConfig(newestConfig)) { + return CompletableFuture + .completedFuture(overrideNewAccessTokenToConfig(newestConfig.getOauthToken(), headerBuilder, newestConfig)); + } final CompletableFuture newAccessToken = extractNewAccessTokenFrom(newestConfig); return newAccessToken.thenApply(token -> overrideNewAccessTokenToConfig(token, headerBuilder, config)); } + private static boolean shouldUseTokenFromNewestConfig(Config newestConfig) { + if (newestConfig.getOauthToken() != null) { + OAuthTokenSource oAuthTokenSource = newestConfig.getOAuthTokenSource(); + return oAuthTokenSource != null && + (oAuthTokenSource.equals(OAuthTokenSource.EXEC_CREDENTIAL_PLUGIN) || + oAuthTokenSource.equals(OAuthTokenSource.SERVICEACCOUNT_TOKEN_FILE) || + oAuthTokenSource.equals(OAuthTokenSource.SYSTEM_PROPERTIES_OR_ENVIRONMENT_VARIABLES)); + } + return false; + } + private CompletableFuture extractNewAccessTokenFrom(Config newestConfig) { - if (newestConfig.getAuthProvider() != null && newestConfig.getAuthProvider().getName().equalsIgnoreCase("oidc")) { + if (isAuthProviderOidc(newestConfig)) { return OpenIDConnectionUtils.resolveOIDCTokenFromAuthConfig(config, newestConfig.getAuthProvider().getConfig(), factory.newBuilder()); } @@ -116,4 +141,7 @@ private void updateLatestRefreshTimestamp() { latestRefreshTimestamp = Instant.now(); } + private static boolean isAuthProviderOidc(Config newestConfig) { + return newestConfig.getAuthProvider() != null && newestConfig.getAuthProvider().getName().equalsIgnoreCase("oidc"); + } } diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java index 0d6bddd24b5..912fa1bdc94 100644 --- a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java @@ -39,6 +39,7 @@ import java.util.List; import java.util.Map; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; @@ -49,7 +50,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.condition.OS.WINDOWS; -public class ConfigTest { +class ConfigTest { private static final String TEST_KUBECONFIG_FILE = Utils.filePath(ConfigTest.class.getResource("/test-kubeconfig")); private static final String TEST_EC_KUBECONFIG_FILE = Utils.filePath(ConfigTest.class.getResource("/test-ec-kubeconfig")); @@ -148,9 +149,11 @@ void testWithSystemProperties() { Config config = new Config(); assertConfig(config); + assertThat(config.getOAuthTokenSource()).isEqualTo(OAuthTokenSource.SYSTEM_PROPERTIES_OR_ENVIRONMENT_VARIABLES); config = new ConfigBuilder().build(); assertConfig(config); + assertThat(config.getOAuthTokenSource()).isEqualTo(OAuthTokenSource.SYSTEM_PROPERTIES_OR_ENVIRONMENT_VARIABLES); } @Test @@ -230,6 +233,7 @@ void testWithBuilderAndSystemProperties() { .build(); assertConfig(config); + assertThat(config.getOAuthTokenSource()).isEqualTo(OAuthTokenSource.SYSTEM_PROPERTIES_OR_ENVIRONMENT_VARIABLES); } @Test @@ -281,6 +285,7 @@ void testWithKubeConfig() { assertTrue(config.getCaCertFile().endsWith("testns/ca.pem".replace("/", File.separator))); assertTrue(new File(config.getCaCertFile()).isAbsolute()); assertEquals(new File(TEST_KUBECONFIG_FILE), config.getFile()); + assertThat(config.getOAuthTokenSource()).isEqualTo(OAuthTokenSource.KUBECONFIG); } @Test @@ -469,6 +474,7 @@ void honorClientAuthenticatorCommands() throws Exception { Config config = Config.autoConfigure(null); assertNotNull(config); assertEquals("HELLO WORLD", config.getOauthToken()); + assertThat(config.getOAuthTokenSource()).isEqualTo(OAuthTokenSource.EXEC_CREDENTIAL_PLUGIN); } @Test @@ -497,6 +503,7 @@ void shouldBeUsedTokenSuppliedByProvider() { .build(); assertEquals("PROVIDER_TOKEN", config.getOauthToken()); + assertThat(config.getOAuthTokenSource()).isEqualTo(OAuthTokenSource.OAUTHTOKEN_PROVIDER); } @Test @@ -515,6 +522,7 @@ void testKubeConfigWithAuthConfigProvider() throws URISyntaxException { assertEquals( "eyJraWQiOiJDTj1vaWRjaWRwLnRyZW1vbG8ubGFuLCBPVT1EZW1vLCBPPVRybWVvbG8gU2VjdXJpdHksIEw9QXJsaW5ndG9uLCBTVD1WaXJnaW5pYSwgQz1VUy1DTj1rdWJlLWNhLTEyMDIxNDc5MjEwMzYwNzMyMTUyIiwiYWxnIjoiUlMyNTYifQ.eyJpc3MiOiJodHRwczovL29pZGNpZHAudHJlbW9sby5sYW46ODQ0My9hdXRoL2lkcC9PaWRjSWRQIiwiYXVkIjoia3ViZXJuZXRlcyIsImV4cCI6MTQ4MzU0OTUxMSwianRpIjoiMm96US15TXdFcHV4WDlHZUhQdy1hZyIsImlhdCI6MTQ4MzU0OTQ1MSwibmJmIjoxNDgzNTQ5MzMxLCJzdWIiOiI0YWViMzdiYS1iNjQ1LTQ4ZmQtYWIzMC0xYTAxZWU0MWUyMTgifQ.w6p4J_6qQ1HzTG9nrEOrubxIMb9K5hzcMPxc9IxPx2K4xO9l-oFiUw93daH3m5pluP6K7eOE6txBuRVfEcpJSwlelsOsW8gb8VJcnzMS9EnZpeA0tW_p-mnkFc3VcfyXuhe5R3G7aa5d8uHv70yJ9Y3-UhjiN9EhpMdfPAoEB9fYKKkJRzF7utTTIPGrSaSU6d2pcpfYKaxIwePzEkT4DfcQthoZdy9ucNvvLoi1DIC-UocFD8HLs8LYKEqSxQvOcvnThbObJ9af71EwmuE21fO5KzMW20KtAeget1gnldOosPtz1G5EwvaQ401-RPQzPGMVBld0_zMCAwZttJ4knw", config.getOauthToken()); + assertEquals(OAuthTokenSource.KUBECONFIG, config.getOAuthTokenSource()); } @Test @@ -526,31 +534,32 @@ void testEmptyConfig() { emptyConfig = Config.empty(); // Then - assertNotNull(emptyConfig); - assertEquals("https://kubernetes.default.svc", emptyConfig.getMasterUrl()); - assertTrue(emptyConfig.getContexts().isEmpty()); - assertNull(emptyConfig.getCurrentContext()); - assertEquals(64, emptyConfig.getMaxConcurrentRequests()); - assertEquals(5, emptyConfig.getMaxConcurrentRequestsPerHost()); - assertFalse(emptyConfig.isTrustCerts()); - assertFalse(emptyConfig.isDisableHostnameVerification()); - assertEquals("RSA", emptyConfig.getClientKeyAlgo()); - assertEquals("changeit", emptyConfig.getClientKeyPassphrase()); - assertEquals(1000, emptyConfig.getWatchReconnectInterval()); - assertEquals(-1, emptyConfig.getWatchReconnectLimit()); - assertEquals(10000, emptyConfig.getConnectionTimeout()); - assertEquals(10000, emptyConfig.getRequestTimeout()); - assertEquals(600000, emptyConfig.getScaleTimeout()); - assertEquals(20000, emptyConfig.getLoggingInterval()); - assertEquals(5000, emptyConfig.getWebsocketTimeout()); - assertEquals(30000, emptyConfig.getWebsocketPingInterval()); - assertEquals(120000, emptyConfig.getUploadRequestTimeout()); - assertTrue(emptyConfig.getImpersonateExtras().isEmpty()); - assertEquals(0, emptyConfig.getImpersonateGroups().length); - assertFalse(emptyConfig.isHttp2Disable()); - assertEquals(1, emptyConfig.getTlsVersions().length); - assertTrue(emptyConfig.getErrorMessages().isEmpty()); - assertNotNull(emptyConfig.getUserAgent()); + assertThat(emptyConfig) + .hasFieldOrPropertyWithValue("masterUrl", "https://kubernetes.default.svc") + .hasFieldOrPropertyWithValue("contexts", Collections.emptyList()) + .hasFieldOrPropertyWithValue("maxConcurrentRequests", 64) + .hasFieldOrPropertyWithValue("maxConcurrentRequestsPerHost", 5) + .hasFieldOrPropertyWithValue("trustCerts", false) + .hasFieldOrPropertyWithValue("disableHostnameVerification", false) + .hasFieldOrPropertyWithValue("clientKeyAlgo", "RSA") + .hasFieldOrPropertyWithValue("clientKeyPassphrase", "changeit") + .hasFieldOrPropertyWithValue("watchReconnectInterval", 1000) + .hasFieldOrPropertyWithValue("watchReconnectLimit", -1) + .hasFieldOrPropertyWithValue("connectionTimeout", 10000) + .hasFieldOrPropertyWithValue("requestTimeout", 10000) + .hasFieldOrPropertyWithValue("scaleTimeout", 600000L) + .hasFieldOrPropertyWithValue("loggingInterval", 20000) + .hasFieldOrPropertyWithValue("websocketTimeout", 5000L) + .hasFieldOrPropertyWithValue("websocketPingInterval", 30000L) + .hasFieldOrPropertyWithValue("uploadRequestTimeout", 120000) + .hasFieldOrPropertyWithValue("impersonateExtras", Collections.emptyMap()) + .hasFieldOrPropertyWithValue("http2Disable", false) + .hasFieldOrPropertyWithValue("tlsVersions", new TlsVersion[] {TlsVersion.TLS_1_2}) + .hasFieldOrPropertyWithValue("errorMessages", Collections.emptyMap()) + .satisfies(e -> assertThat(e.getCurrentContext()).isNull()) + .satisfies(e -> assertThat(e.getImpersonateGroups()).isEmpty()) + .satisfies(e -> assertThat(e.getUserAgent()).isNotNull()) + .satisfies(e -> assertThat(e.getOAuthTokenSource()).isNull()); } private void assertConfig(Config config) { @@ -750,4 +759,20 @@ void getHomeDir_shouldReturnUserHomeProp_WhenHomeEnvVariablesAreNotSet() { System.setProperty("user.home", userHomePropToRestore); } } + + @Test + void refresh_whenOAuthTokenSourceSetToUser_thenConfigUnchanged() { + // Given + Config config = new ConfigBuilder() + .withOauthToken("token-from-user") + .build(); + + // When + Config updatedConfig = config.refresh(); + + // Then + assertThat(updatedConfig) + .isSameAs(config) + .hasFieldOrPropertyWithValue("oauthToken", "token-from-user"); + } } diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptorTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptorTest.java index fc895099109..5b77c49c057 100644 --- a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptorTest.java +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptorTest.java @@ -17,6 +17,8 @@ import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.ConfigBuilder; +import io.fabric8.kubernetes.client.OAuthTokenProvider; +import io.fabric8.kubernetes.client.OAuthTokenSource; import io.fabric8.kubernetes.client.http.HttpClient; import io.fabric8.kubernetes.client.http.HttpRequest; import io.fabric8.kubernetes.client.http.StandardHttpRequest; @@ -32,13 +34,15 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Objects; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import static io.fabric8.kubernetes.client.Config.KUBERNETES_AUTH_SERVICEACCOUNT_TOKEN_FILE_SYSTEM_PROPERTY; import static io.fabric8.kubernetes.client.Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY; import static io.fabric8.kubernetes.client.Config.KUBERNETES_KUBECONFIG_FILE; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; /** @@ -55,7 +59,7 @@ void shouldAutoconfigureAfter401() throws Exception { Paths.get(tempFile.getPath()), StandardCopyOption.REPLACE_EXISTING); System.setProperty(KUBERNETES_KUBECONFIG_FILE, tempFile.getAbsolutePath()); - HttpRequest.Builder builder = Mockito.mock(HttpRequest.Builder.class, Mockito.RETURNS_SELF); + HttpRequest.Builder builder = mock(HttpRequest.Builder.class, Mockito.RETURNS_SELF); // Call boolean reissue = new TokenRefreshInterceptor(Config.autoConfigure(null), null, Instant.now()) @@ -77,7 +81,7 @@ void shouldAutoconfigureAfter1Minute() throws Exception { Paths.get(tempFile.getPath()), StandardCopyOption.REPLACE_EXISTING); System.setProperty(KUBERNETES_KUBECONFIG_FILE, tempFile.getAbsolutePath()); - HttpRequest.Builder builder = Mockito.mock(HttpRequest.Builder.class, Mockito.RETURNS_SELF); + HttpRequest.Builder builder = mock(HttpRequest.Builder.class, Mockito.RETURNS_SELF); // Call TokenRefreshInterceptor tokenRefreshInterceptor = new TokenRefreshInterceptor(Config.autoConfigure(null), @@ -99,13 +103,9 @@ void shouldAutoconfigureAfter1Minute() throws Exception { @DisplayName("#4442 token auto refresh should not overwrite existing token when not applicable") void refreshShouldNotOverwriteExistingToken() throws Exception { // Given - final Config originalConfig = spy(new ConfigBuilder(Config.empty()) + final Config originalConfig = new ConfigBuilder(Config.empty()) .withOauthToken("existing-token") - .build()); - final Config autoConfig = new ConfigBuilder(Config.empty()) - .withOauthToken("") // empty token .build(); - when(originalConfig.refresh()).thenReturn(autoConfig); final TokenRefreshInterceptor tokenRefreshInterceptor = new TokenRefreshInterceptor( originalConfig, null, Instant.now().minusSeconds(61)); // When @@ -117,24 +117,20 @@ void refreshShouldNotOverwriteExistingToken() throws Exception { } @Test - @DisplayName("#4442 token auto refresh should overwrite existing token when applicable") - void refreshShouldOverwriteExistingToken() throws Exception { + @DisplayName("#4442 token auto refresh should not overwrite existing token when provided by user") + void refresh_whenNoAuthProvider_thenShouldInheritTokenFromOldConfig() throws Exception { // Given - final Config originalConfig = spy(new ConfigBuilder(Config.empty()) + final Config originalConfig = new ConfigBuilder(Config.empty()) .withOauthToken("existing-token") - .build()); - final Config autoConfig = new ConfigBuilder(Config.empty()) - .withOauthToken("new-token") .build(); - when(originalConfig.refresh()).thenReturn(autoConfig); final TokenRefreshInterceptor tokenRefreshInterceptor = new TokenRefreshInterceptor( originalConfig, null, Instant.now().minusSeconds(61)); // When final boolean result = tokenRefreshInterceptor .afterFailure(new StandardHttpRequest.Builder(), new TestHttpResponse<>().withCode(401), null).get(); // Then - assertThat(result).isTrue(); - assertThat(originalConfig).hasFieldOrPropertyWithValue("oauthToken", "new-token"); + assertThat(result).isFalse(); + assertThat(originalConfig).hasFieldOrPropertyWithValue("oauthToken", "existing-token"); } @Test @@ -147,7 +143,7 @@ void shouldReloadInClusterServiceAccount() throws Exception { System.setProperty(KUBERNETES_AUTH_SERVICEACCOUNT_TOKEN_FILE_SYSTEM_PROPERTY, tokenFile.getAbsolutePath()); System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false"); - HttpRequest.Builder builder = Mockito.mock(HttpRequest.Builder.class, Mockito.RETURNS_SELF); + HttpRequest.Builder builder = mock(HttpRequest.Builder.class, Mockito.RETURNS_SELF); // The expired token will be read at auto configure. TokenRefreshInterceptor interceptor = new TokenRefreshInterceptor(Config.autoConfigure(null), null, Instant.now()); @@ -176,7 +172,7 @@ void shouldRefreshOIDCToken() throws Exception { System.setProperty(KUBERNETES_KUBECONFIG_FILE, tempFile.getAbsolutePath()); // Prepare HTTP call that will fail with 401 Unauthorized to trigger OIDC token renewal. - HttpRequest.Builder builder = Mockito.mock(HttpRequest.Builder.class, Mockito.RETURNS_SELF); + HttpRequest.Builder builder = mock(HttpRequest.Builder.class, Mockito.RETURNS_SELF); // Loads the initial kubeconfig, including initial token value. Config config = Config.autoConfigure(null); @@ -190,7 +186,7 @@ void shouldRefreshOIDCToken() throws Exception { Files.copy(Objects.requireNonNull(getClass().getResourceAsStream("/token-refresh-interceptor/kubeconfig-oidc")), Paths.get(tempFile.getPath()), StandardCopyOption.REPLACE_EXISTING); - TokenRefreshInterceptor interceptor = new TokenRefreshInterceptor(config, Mockito.mock(HttpClient.Factory.class), + TokenRefreshInterceptor interceptor = new TokenRefreshInterceptor(config, mock(HttpClient.Factory.class), Instant.now()); boolean reissue = interceptor.afterFailure(builder, new TestHttpResponse<>().withCode(401), null).get(); @@ -203,4 +199,76 @@ void shouldRefreshOIDCToken() throws Exception { } } + + @Test + void afterFailure_whenTokenUpdatedPostRefreshUsingExecCredentials_thenUseUpdatedToken() + throws ExecutionException, InterruptedException { + // Given + final Config oldConfig = mock(Config.class); + final Config newConfig = mock(Config.class); + HttpRequest.Builder builder = mock(HttpRequest.Builder.class, Mockito.RETURNS_SELF); + when(oldConfig.isTokenNonRefreshable()).thenReturn(false); + when(oldConfig.refresh()).thenReturn(newConfig); + when(newConfig.getOauthToken()).thenReturn("token-from-exec-credentials"); + when(newConfig.getOAuthTokenSource()).thenReturn(OAuthTokenSource.EXEC_CREDENTIAL_PLUGIN); + final TokenRefreshInterceptor tokenRefreshInterceptor = new TokenRefreshInterceptor( + oldConfig, null, Instant.now().minusSeconds(61)); + // When + final boolean result = tokenRefreshInterceptor + .afterFailure(builder, new TestHttpResponse<>().withCode(401), null).get(); + // Then + assertThat(result).isTrue(); + Mockito.verify(builder).setHeader("Authorization", "Bearer token-from-exec-credentials"); + } + + @Test + void afterFailure_whenTokenFromOAuthTokenProvider_thenUseUpdatedToken() throws ExecutionException, InterruptedException { + // Given + final Config oldConfig = mock(Config.class); + HttpRequest.Builder builder = mock(HttpRequest.Builder.class, Mockito.RETURNS_SELF); + when(oldConfig.isTokenNonRefreshable()).thenReturn(false); + when(oldConfig.getOAuthTokenSource()).thenReturn(OAuthTokenSource.OAUTHTOKEN_PROVIDER); + when(oldConfig.getOauthTokenProvider()).thenReturn(() -> "token-from-oauthtokenprovider"); + final TokenRefreshInterceptor tokenRefreshInterceptor = new TokenRefreshInterceptor( + oldConfig, null, Instant.now().minusSeconds(61)); + // When + final boolean result = tokenRefreshInterceptor + .afterFailure(builder, new TestHttpResponse<>().withCode(401), null).get(); + // Then + assertThat(result).isTrue(); + Mockito.verify(builder).setHeader("Authorization", "Bearer token-from-oauthtokenprovider"); + } + + @Test + void afterFailure_whenBasicAuth_thenCompleteWithFalse() { + // Given + final Config config = mock(Config.class); + when(config.getUsername()).thenReturn("kubeadmin"); + when(config.getPassword()).thenReturn("secret"); + final TokenRefreshInterceptor tokenRefreshInterceptor = new TokenRefreshInterceptor( + config, null, Instant.now().minusSeconds(61)); + + // When + CompletableFuture result = tokenRefreshInterceptor.afterFailure(null, null, null); + + // Then + assertThat(result).isCompletedWithValue(false); + } + + @Test + void before_whenBasicAuth_thenUseCredentialsInHeader() { + // Given + final Config config = mock(Config.class); + HttpRequest.Builder builder = mock(HttpRequest.Builder.class, Mockito.RETURNS_SELF); + when(config.getUsername()).thenReturn("kubeadmin"); + when(config.getPassword()).thenReturn("secret"); + final TokenRefreshInterceptor tokenRefreshInterceptor = new TokenRefreshInterceptor( + config, null, Instant.now().minusSeconds(61)); + + // When + tokenRefreshInterceptor.before(builder, null, null); + + // Then + Mockito.verify(builder).header("Authorization", HttpClientUtils.basicCredentials("kubeadmin", "secret")); + } } diff --git a/openshift-client-api/src/main/java/io/fabric8/openshift/client/OpenShiftConfig.java b/openshift-client-api/src/main/java/io/fabric8/openshift/client/OpenShiftConfig.java index bd4daba4d2e..5cbc3ff22fb 100644 --- a/openshift-client-api/src/main/java/io/fabric8/openshift/client/OpenShiftConfig.java +++ b/openshift-client-api/src/main/java/io/fabric8/openshift/client/OpenShiftConfig.java @@ -20,6 +20,7 @@ import com.fasterxml.jackson.annotation.JsonInclude; import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.OAuthTokenProvider; +import io.fabric8.kubernetes.client.OAuthTokenSource; import io.fabric8.kubernetes.client.http.TlsVersion; import io.fabric8.kubernetes.client.readiness.Readiness; import io.fabric8.kubernetes.client.utils.URLUtils; @@ -85,6 +86,7 @@ public OpenShiftConfig(String openShiftUrl, String oapiVersion, String masterUrl long websocketPingInterval, String proxyUsername, String proxyPassword, String trustStoreFile, String trustStorePassphrase, String keyStoreFile, String keyStorePassphrase, String impersonateUsername, String[] impersonateGroups, Map> impersonateExtras, OAuthTokenProvider oauthTokenProvider, + OAuthTokenSource oAuthTokenSource, Map customHeaders, int requestRetryBackoffLimit, int requestRetryBackoffInterval, int uploadRequestTimeout, long buildTimeout, boolean disableApiGroupCheck) { @@ -97,7 +99,8 @@ public OpenShiftConfig(String openShiftUrl, String oapiVersion, String masterUrl noProxy, errorMessages, userAgent, tlsVersions, websocketTimeout, websocketPingInterval, proxyUsername, proxyPassword, trustStoreFile, trustStorePassphrase, keyStoreFile, keyStorePassphrase, impersonateUsername, impersonateGroups, - impersonateExtras, oauthTokenProvider, customHeaders, requestRetryBackoffLimit, requestRetryBackoffInterval, + impersonateExtras, oauthTokenProvider, oAuthTokenSource, customHeaders, requestRetryBackoffLimit, + requestRetryBackoffInterval, uploadRequestTimeout); this.setOapiVersion(oapiVersion); this.setBuildTimeout(buildTimeout); @@ -135,7 +138,7 @@ public OpenShiftConfig(Config kubernetesConfig, String openShiftUrl, String oapi kubernetesConfig.getTrustStorePassphrase(), kubernetesConfig.getKeyStoreFile(), kubernetesConfig.getKeyStorePassphrase(), kubernetesConfig.getImpersonateUsername(), kubernetesConfig.getImpersonateGroups(), kubernetesConfig.getImpersonateExtras(), - kubernetesConfig.getOauthTokenProvider(), kubernetesConfig.getCustomHeaders(), + kubernetesConfig.getOauthTokenProvider(), kubernetesConfig.getOAuthTokenSource(), kubernetesConfig.getCustomHeaders(), kubernetesConfig.getRequestRetryBackoffLimit(), kubernetesConfig.getRequestRetryBackoffInterval(), kubernetesConfig.getUploadRequestTimeout(), buildTimeout, diff --git a/openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptor.java b/openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptor.java index 358877e3bbc..4d280673591 100644 --- a/openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptor.java +++ b/openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptor.java @@ -21,6 +21,7 @@ import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReview; import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.KubernetesClientException; +import io.fabric8.kubernetes.client.OAuthTokenSource; import io.fabric8.kubernetes.client.http.BasicBuilder; import io.fabric8.kubernetes.client.http.HttpClient; import io.fabric8.kubernetes.client.http.HttpRequest; @@ -94,6 +95,15 @@ public void before(BasicBuilder builder, HttpRequest httpRequest, RequestTags ta @Override public CompletableFuture afterFailure(Builder builder, HttpResponse response, RequestTags tags) { + if (config.isTokenNonRefreshable()) { + return CompletableFuture.completedFuture(false); + } + if (config.getOAuthTokenSource() != null && config.getOAuthTokenSource().equals(OAuthTokenSource.OAUTHTOKEN_PROVIDER)) { + String tokenFromProvider = config.getOauthTokenProvider().getToken(); + if (tokenFromProvider != null && !tokenFromProvider.isEmpty()) { + setAuthHeader(builder, tokenFromProvider); + } + } if (shouldProceed(response.request(), response)) { return CompletableFuture.completedFuture(false); } @@ -101,22 +111,7 @@ public CompletableFuture afterFailure(Builder builder, HttpResponse // use the original config, not the refreshed, as the username / password could be programmatically set on the Config or RequestConfig if (Utils.isNotNullOrEmpty(config.getUsername()) && Utils.isNotNullOrEmpty(config.getPassword())) { // TODO: we could make all concurrent refresh requests return the same future - return authorize().thenApply(t -> { - if (t != null) { - config.setOauthToken(t); - try { - // TODO: we may need some protection here or in the persistKubeConfigWithUpdatedAuthInfo - // if the user has modified the username via the requestconfig are we writing a valid value? - OpenIDConnectionUtils.persistKubeConfigWithUpdatedAuthInfo(config, a -> a.setToken(t)); - } catch (IOException e) { - LOGGER.warn("failure while persisting new token into KUBECONFIG", e); - } - // If token was obtained, then retry request using the obtained token. - return setAuthHeader(builder, t); - } - - return refreshFromConfig(builder); - }); + return authorize().thenApply(t -> persistNewOAuthTokenIntoKubeConfig(builder, t)); } return CompletableFuture.completedFuture(refreshFromConfig(builder)); } @@ -198,4 +193,21 @@ private boolean shouldProceed(HttpRequest request, HttpResponse response) { return response.code() != HTTP_UNAUTHORIZED && response.code() != HTTP_FORBIDDEN; } + private boolean persistNewOAuthTokenIntoKubeConfig(Builder builder, String token) { + if (token != null) { + config.setOauthToken(token); + try { + // TODO: we may need some protection here or in the persistKubeConfigWithUpdatedAuthInfo + // if the user has modified the username via the requestconfig are we writing a valid value? + OpenIDConnectionUtils.persistKubeConfigWithUpdatedAuthInfo(config, a -> a.setToken(token)); + } catch (IOException e) { + LOGGER.warn("failure while persisting new token into KUBECONFIG", e); + } + // If token was obtained, then retry request using the obtained token. + return setAuthHeader(builder, token); + } + + return refreshFromConfig(builder); + } + } diff --git a/openshift-client/src/test/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptorTest.java b/openshift-client/src/test/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptorTest.java index 6ce8e15d94b..6948a5bc6ec 100644 --- a/openshift-client/src/test/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptorTest.java +++ b/openshift-client/src/test/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptorTest.java @@ -16,7 +16,9 @@ package io.fabric8.openshift.client.internal; import io.fabric8.kubernetes.client.Config; +import io.fabric8.kubernetes.client.OAuthTokenSource; import io.fabric8.kubernetes.client.http.HttpClient; +import io.fabric8.kubernetes.client.http.HttpRequest; import io.fabric8.kubernetes.client.http.StandardHttpRequest; import io.fabric8.kubernetes.client.http.TestHttpResponse; import io.fabric8.kubernetes.client.utils.TokenRefreshInterceptor; @@ -26,10 +28,16 @@ import java.net.HttpURLConnection; import java.net.URI; import java.util.Collections; +import java.util.concurrent.CompletableFuture; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.RETURNS_DEEP_STUBS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; class OpenShiftOAuthInterceptorTest { @@ -89,4 +97,49 @@ void testTokenRefreshedFromConfig() { Mockito.verify(config).setOauthToken("token"); } + @Test + void afterFailure_whenTokenSetByUser_thenNoRefresh() { + // Given + Config config = Mockito.mock(Config.class, RETURNS_DEEP_STUBS); + when(config.getOauthToken()).thenReturn("token-set-by-user"); + when(config.isTokenNonRefreshable()).thenReturn(true); + HttpClient client = Mockito.mock(HttpClient.class); + StandardHttpRequest.Builder builder = new StandardHttpRequest.Builder(); + builder.uri("http://localhost"); + OpenShiftOAuthInterceptor interceptor = new OpenShiftOAuthInterceptor(client, config); + + // When + CompletableFuture result = interceptor.afterFailure(builder, + TestHttpResponse.from(HttpURLConnection.HTTP_UNAUTHORIZED, "not for you") + .withRequest(new StandardHttpRequest(null, URI.create("http://localhost"), "GET", null)), + null); + + // Then + assertThat(result).isCompletedWithValue(false); + verify(config, times(0)).refresh(); + } + + @Test + void afterFailure_whenOAuthTokenProviderPresent_thenUseTokenFromProvider() { + // Given + Config config = Mockito.mock(Config.class, RETURNS_DEEP_STUBS); + when(config.isTokenNonRefreshable()).thenReturn(false); + when(config.getOAuthTokenSource()).thenReturn(OAuthTokenSource.OAUTHTOKEN_PROVIDER); + when(config.getOauthTokenProvider()).thenReturn(() -> "token-from-oauthtokenprovider"); + HttpClient client = Mockito.mock(HttpClient.class); + HttpRequest.Builder builder = mock(HttpRequest.Builder.class, Mockito.RETURNS_SELF); + builder.uri("http://localhost"); + OpenShiftOAuthInterceptor interceptor = new OpenShiftOAuthInterceptor(client, config); + + // When + CompletableFuture result = interceptor.afterFailure(builder, + TestHttpResponse.from(HttpURLConnection.HTTP_UNAUTHORIZED, "not for you") + .withRequest(new StandardHttpRequest(null, URI.create("http://localhost"), "GET", null)), + null); + + // Then + assertThat(result).isCompletedWithValue(false); + verify(builder).setHeader("Authorization", "Bearer token-from-oauthtokenprovider"); + } + }