diff --git a/CHANGELOG.md b/CHANGELOG.md index db3de7f736d..76abbd7b0da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * * Fix #4206: KubernetesDeserializer can now handle any valid object. If the object lacks type information, it will be deserialized as a GenericKubernetesResource. * Fix #4365: backport of stopped future for informers to obtain the termination exception * Fix #4383: bump snakeyaml from 1.28 to 1.33 +* Fix #4442: TokenRefreshInterceptor doesn't overwrite existing OAuth token with empty string #### _**Note**_: Behavior changes * Fix #4206: The Serialization utility class will throw an Exception, instead of returning null, if an untyped unmarshall method is used on something that lacks type information diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptor.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptor.java index f1d2c52895e..208a75f1c7e 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptor.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptor.java @@ -74,7 +74,7 @@ private boolean refreshToken(BasicBuilder headerBuilder) { newAccessToken = newestConfig.getOauthToken(); } - if (newAccessToken != null) { + if (Utils.isNotNullOrEmpty(newAccessToken)) { // Delete old Authorization header and append new one headerBuilder .setHeader("Authorization", "Bearer " + newAccessToken); diff --git a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptorTest.java b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptorTest.java index e6cfae5ea99..478d3c2615d 100644 --- a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptorTest.java +++ b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptorTest.java @@ -16,9 +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.BasicBuilder; import io.fabric8.kubernetes.client.http.HttpClient; import io.fabric8.kubernetes.client.http.HttpRequest; +import io.fabric8.kubernetes.client.http.HttpResponse; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; import org.mockito.Mockito; import java.io.File; @@ -35,7 +40,13 @@ import static io.fabric8.kubernetes.client.Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY; import static io.fabric8.kubernetes.client.Config.KUBERNETES_KUBECONFIG_FILE; import static io.fabric8.kubernetes.client.MockHttpClientUtils.buildResponse; +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.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; /** * Ignoring for now - the token refresh should be based upon the java 11 client or the provided client library and not okhttp @@ -50,7 +61,7 @@ public void shouldAutoconfigureAfter401() throws IOException { Files.copy(Objects.requireNonNull(getClass().getResourceAsStream("/test-kubeconfig-tokeninterceptor")), 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).afterFailure(builder, buildResponse(HttpURLConnection.HTTP_UNAUTHORIZED, "foo")); @@ -71,7 +82,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), null); @@ -87,6 +98,54 @@ 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); + tokenRefreshInterceptor.setLastRefresh(Instant.now().minusSeconds(61)); + final HttpResponse response = mock(HttpResponse.class); + when(response.code()).thenReturn(401); + // When + final boolean result = tokenRefreshInterceptor.afterFailure(mock(BasicBuilder.class), response); + // 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); + tokenRefreshInterceptor.setLastRefresh(Instant.now().minusSeconds(61)); + final HttpResponse response = mock(HttpResponse.class); + when(response.code()).thenReturn(401); + // When + final boolean result = tokenRefreshInterceptor.afterFailure(mock(BasicBuilder.class), response); + // Then + assertThat(result).isTrue(); + assertThat(config).hasFieldOrPropertyWithValue("oauthToken", "new-token"); + } + } + @Test void shouldReloadInClusterServiceAccount() throws IOException { try { @@ -97,7 +156,7 @@ void shouldReloadInClusterServiceAccount() throws IOException { 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); @@ -126,7 +185,7 @@ void shouldRefreshOIDCToken() throws IOException { 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); @@ -140,7 +199,7 @@ void shouldRefreshOIDCToken() throws IOException { Files.copy(Objects.requireNonNull(getClass().getResourceAsStream("/test-kubeconfig-tokeninterceptor-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)); boolean reissue = interceptor.afterFailure(builder, buildResponse(HttpURLConnection.HTTP_UNAUTHORIZED, "foo")); // Make the call and check that renewed token was read at 401 Unauthorized.