From ad7c825f4b408a85cf9d3ffefb2de96b874bebe4 Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Thu, 29 Sep 2022 12:33:48 +0200 Subject: [PATCH] fix(kubernetes-client-api): TokenRefreshInterceptor doesn't overwrite existing OAuth token with empty string Signed-off-by: Marc Nuri --- CHANGELOG.md | 1 + .../client/utils/TokenRefreshInterceptor.java | 2 +- .../utils/TokenRefreshInterceptorTest.java | 54 +++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27ab1048997..32f4a0847d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * Fix #4350: SchemaSwap annotation is now repeatable and is applied multiple times if classes are used more than once in the class hierarchy. * Fix #3733: The authentication command from the .kube/config won't be discarded if no arguments are specified * Fix #4441: corrected patch base handling for the patch methods available from a Resource - resource(item).patch() will be evaluated as resource(latest).patch(item). Also undeprecated patch(item), which is consistent with leaving patch(context, item) undeprecated as well. For consistency with the other operations (such as edit), patch(item) will use the context item as the base when available, or the server side item when not. This means that patch(item) is only the same as resource(item).patch() when the patch(item) is called when the context item is missing or is the same as the latest. +* Fix #4442: TokenRefreshInterceptor doesn't overwrite existing OAuth token with empty string #### Improvements * Fix #4348: Introduce specific annotations for the generators 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 7c320167aa2..2448ef8cebd 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 @@ -84,7 +84,7 @@ private CompletableFuture extractNewAccessTokenFrom(Config newestConfig) } private boolean overrideNewAccessTokenToConfig(String newAccessToken, BasicBuilder headerBuilder, Config existConfig) { - if (newAccessToken != null) { + if (Utils.isNotNullOrEmpty(newAccessToken)) { headerBuilder.setHeader("Authorization", "Bearer " + newAccessToken); existConfig.setOauthToken(newAccessToken); 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 e24a2265d54..cf95ec10bf4 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 @@ -16,10 +16,14 @@ package io.fabric8.kubernetes.client.utils; import io.fabric8.kubernetes.client.Config; +import io.fabric8.kubernetes.client.ConfigBuilder; 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 org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; import org.mockito.Mockito; import java.io.File; @@ -33,7 +37,11 @@ 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.ArgumentMatchers.any; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mockStatic; /** * Ignoring for now - the token refresh should be based upon the java 11 client or the provided client library and not okhttp @@ -89,6 +97,52 @@ void shouldAutoconfigureAfter1Minute() throws Exception { } } + @Test + @DisplayName("#4442 token auto refresh should not overwrite existing token when not applicable") + void refreshShouldNotOverwriteExistingToken() throws Exception { + try (MockedStatic configMock = mockStatic(Config.class, CALLS_REAL_METHODS)) { + // Given + final Config autoConfig = new ConfigBuilder(Config.empty()) + .withOauthToken("") // empty token + .build(); + configMock.when(() -> Config.autoConfigure(any())).thenReturn(autoConfig); + final Config config = new ConfigBuilder(Config.empty()) + .withOauthToken("existing-token") + .build(); + final TokenRefreshInterceptor tokenRefreshInterceptor = new TokenRefreshInterceptor( + config, null, Instant.now().minusSeconds(61)); + // When + final boolean result = tokenRefreshInterceptor + .afterFailure(new StandardHttpRequest.Builder(), new TestHttpResponse<>().withCode(401)).get(); + // Then + assertThat(result).isFalse(); + assertThat(config).hasFieldOrPropertyWithValue("oauthToken", "existing-token"); + } + } + + @Test + @DisplayName("#4442 token auto refresh should overwrite existing token when applicable") + void refreshShouldOverwriteExistingToken() throws Exception { + try (MockedStatic configMock = mockStatic(Config.class, CALLS_REAL_METHODS)) { + // Given + final Config autoConfig = new ConfigBuilder(Config.empty()) + .withOauthToken("new-token") + .build(); + configMock.when(() -> Config.autoConfigure(any())).thenReturn(autoConfig); + final Config config = new ConfigBuilder(Config.empty()) + .withOauthToken("existing-token") + .build(); + final TokenRefreshInterceptor tokenRefreshInterceptor = new TokenRefreshInterceptor( + config, null, Instant.now().minusSeconds(61)); + // When + final boolean result = tokenRefreshInterceptor + .afterFailure(new StandardHttpRequest.Builder(), new TestHttpResponse<>().withCode(401)).get(); + // Then + assertThat(result).isTrue(); + assertThat(config).hasFieldOrPropertyWithValue("oauthToken", "new-token"); + } + } + @Test void shouldReloadInClusterServiceAccount() throws Exception { try {