Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of #4461 #4463

Merged
merged 1 commit into from
Sep 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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"));
Expand All @@ -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);
Expand All @@ -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<Config> 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<String> 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<Config> 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<String> 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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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.
Expand Down