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

fix(kubernetes-client-api): TokenRefreshInterceptor doesn't overwrite existing OAuth token with empty string #4461

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 @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ private CompletableFuture<String> 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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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<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, 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<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, 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 {
Expand Down