Skip to content

Commit

Permalink
fix #4802 further refinements & creating a common base for token refresh
Browse files Browse the repository at this point in the history
  • Loading branch information
@shawkins @manusa
shawkins authored and manusa committed May 24, 2023
1 parent bc190ae commit 39b526c
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 118 deletions.
13 changes: 2 additions & 11 deletions kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@ public Config(String masterUrl, String apiVersion, String namespace, boolean tru
this.masterUrl = ensureEndsWithSlash(ensureHttps(masterUrl, this));
this.maxConcurrentRequests = maxConcurrentRequests;
this.maxConcurrentRequestsPerHost = maxConcurrentRequestsPerHost;
this.autoOAuthToken = autoOAuthToken;
}

public static void configFromSysPropsOrEnvVars(Config config) {
Expand Down Expand Up @@ -966,19 +967,9 @@ public static String getKeyAlgorithm(String clientKeyFile, String clientKeyData)
return null;
}

public String getUserConfiguredOauthToken() {
return oauthToken;
}

@JsonProperty("oauthToken")
public String getOauthToken() {
if (this.oauthTokenProvider != null) {
return this.oauthTokenProvider.getToken();
}
if (this.oauthToken != null) {
return oauthToken;
}
return autoOAuthToken;
return oauthToken;
}

public void setOauthToken(String oauthToken) {
Expand Down
80 changes: 54 additions & 26 deletions ...-client-api/src/main/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.concurrent.CompletableFuture;
import java.util.function.Function;

/**
* Interceptor for handling kube authentication. It will either be basic auth, or token based. This class takes responsibility
Expand All @@ -37,34 +38,58 @@ public class TokenRefreshInterceptor implements Interceptor {

public static final String NAME = "TOKEN";

private final Config config;
private HttpClient.Factory factory;
protected final Config config;
private final Function<Config, CompletableFuture<String>> remoteRefresh;

private static final int REFRESH_INTERVAL_MINUTE = 1;

private Instant latestRefreshTimestamp;
private volatile Instant latestRefreshTimestamp;

public TokenRefreshInterceptor(Config config, HttpClient.Factory factory, Instant latestRefreshTimestamp) {
this(config, latestRefreshTimestamp,
newestConfig -> OpenIDConnectionUtils.resolveOIDCTokenFromAuthConfig(config, newestConfig.getAuthProvider().getConfig(),
factory.newBuilder()));
}

public TokenRefreshInterceptor(Config config, Instant latestRefreshTimestamp,
Function<Config, CompletableFuture<String>> remoteRefresh) {
this.config = config;
this.remoteRefresh = remoteRefresh;
this.latestRefreshTimestamp = latestRefreshTimestamp;
this.factory = factory;
}

@Override
public void before(BasicBuilder headerBuilder, HttpRequest request, RequestTags tags) {
if (isBasicAuth()) {
if (useBasicAuth()) {
headerBuilder.header(AUTHORIZATION, HttpClientUtils.basicCredentials(config.getUsername(), config.getPassword()));
return;
}
if (Utils.isNotNullOrEmpty(config.getOauthToken())) {
headerBuilder.header(AUTHORIZATION, "Bearer " + config.getOauthToken());

String token = getEffectiveOauthToken(config);

if (Utils.isNotNullOrEmpty(token)) {
headerBuilder.header(AUTHORIZATION, "Bearer " + token);
}
if (isTimeToRefresh()) {
refreshToken(headerBuilder);
}
}

private boolean isBasicAuth() {
private static String getEffectiveOauthToken(Config config) {
if (config.getOauthTokenProvider() != null) {
return config.getOauthTokenProvider().getToken();
}
if (config.getOauthToken() != null) {
return config.getOauthToken();
}
return config.getAutoOAuthToken();
}

protected boolean useBasicAuth() {
return isBasicAuth();
}

final protected boolean isBasicAuth() {
return Utils.isNotNullOrEmpty(config.getUsername()) && Utils.isNotNullOrEmpty(config.getPassword());
}

Expand All @@ -74,45 +99,48 @@ private boolean isTimeToRefresh() {

@Override
public CompletableFuture<Boolean> afterFailure(BasicBuilder headerBuilder, HttpResponse<?> response, RequestTags tags) {
if (isBasicAuth()) {
if (shouldFail(response)) {
return CompletableFuture.completedFuture(false);
}
if (response.code() == HttpURLConnection.HTTP_UNAUTHORIZED) {
return refreshToken(headerBuilder);
}
return CompletableFuture.completedFuture(false);
return refreshToken(headerBuilder);
}

protected boolean shouldFail(HttpResponse<?> response) {
return useBasicAuth() || response.code() != HttpURLConnection.HTTP_UNAUTHORIZED;
}

private CompletableFuture<Boolean> refreshToken(BasicBuilder headerBuilder) {
if (config.getOauthTokenProvider() != null) {
String tokenFromProvider = config.getOauthTokenProvider().getToken();
if (tokenFromProvider != null && !tokenFromProvider.isEmpty()) {
return CompletableFuture.completedFuture(overrideNewAccessTokenToConfig(tokenFromProvider, headerBuilder, config));
}
return CompletableFuture.completedFuture(overrideNewAccessTokenToConfig(tokenFromProvider, headerBuilder));
}
if (config.getUserConfiguredOauthToken() != null && !config.getUserConfiguredOauthToken().isEmpty()) {
return CompletableFuture
.completedFuture(overrideNewAccessTokenToConfig(config.getUserConfiguredOauthToken(), headerBuilder, config));
if (config.getOauthToken() != null) {
return CompletableFuture.completedFuture(false);
}
Config newestConfig = config.refresh();
final CompletableFuture<String> newAccessToken = extractNewAccessTokenFrom(newestConfig);

return newAccessToken.thenApply(token -> overrideNewAccessTokenToConfig(token, headerBuilder, config));
return newAccessToken.thenApply(token -> overrideNewAccessTokenToConfig(token, headerBuilder));
}

private CompletableFuture<String> extractNewAccessTokenFrom(Config newestConfig) {
if (isAuthProviderOidc(newestConfig) && OpenIDConnectionUtils.idTokenExpired(newestConfig)) {
return OpenIDConnectionUtils.resolveOIDCTokenFromAuthConfig(config, newestConfig.getAuthProvider().getConfig(),
factory.newBuilder());
if (useRemoteRefresh(newestConfig)) {
// TODO: determine the appropriate fall-back behavior. If the result here is null, do we use the non-remote token
return remoteRefresh.apply(newestConfig);
}

return CompletableFuture.completedFuture(newestConfig.getOauthToken());
return CompletableFuture.completedFuture(getEffectiveOauthToken(newestConfig));
}

protected boolean useRemoteRefresh(Config newestConfig) {
// TODO: in a hard failure scenario, should we skip the expired check
return isAuthProviderOidc(newestConfig) && OpenIDConnectionUtils.idTokenExpired(newestConfig);
}

private boolean overrideNewAccessTokenToConfig(String newAccessToken, BasicBuilder headerBuilder, Config existConfig) {
private boolean overrideNewAccessTokenToConfig(String newAccessToken, BasicBuilder headerBuilder) {
if (Utils.isNotNullOrEmpty(newAccessToken)) {
headerBuilder.setHeader(AUTHORIZATION, "Bearer " + newAccessToken);
existConfig.setAutoOAuthToken(newAccessToken);
config.setAutoOAuthToken(newAccessToken);

updateLatestRefreshTimestamp();

Expand Down
40 changes: 23 additions & 17 deletions kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,10 @@ void testWithSystemProperties() {
System.setProperty(Config.KUBERNETES_UPLOAD_REQUEST_TIMEOUT_SYSTEM_PROPERTY, "600000");

Config config = new Config();
assertConfig(config);
assertConfig(config, true);

config = new ConfigBuilder().build();
assertConfig(config);
assertConfig(config, true);
}

@Test
Expand Down Expand Up @@ -201,7 +201,7 @@ void testWithBuilder() {
.withKeyStorePassphrase("keystorePassphrase")
.build();

assertConfig(config);
assertConfig(config, false);
}

@Test
Expand Down Expand Up @@ -244,7 +244,7 @@ void testWithBuilderAndSystemProperties() {
.withNamespace("testns")
.build();

assertConfig(config);
assertConfig(config, true);
}

@Test
Expand Down Expand Up @@ -292,7 +292,7 @@ void testWithKubeConfig() {

assertEquals("https://172.28.128.4:8443/", config.getMasterUrl());
assertEquals("testns", config.getNamespace());
assertEquals("token", config.getOauthToken());
assertEquals("token", config.getAutoOAuthToken());
assertTrue(config.getCaCertFile().endsWith("testns/ca.pem".replace("/", File.separator)));
assertTrue(new File(config.getCaCertFile()).isAbsolute());
assertEquals(new File(TEST_KUBECONFIG_FILE), config.getFile());
Expand All @@ -306,7 +306,7 @@ void testWithKubeConfigAndOverrideContext() {

assertEquals("https://172.28.128.4:8443/", config.getMasterUrl());
assertEquals("production", config.getNamespace());
assertEquals("supertoken", config.getOauthToken());
assertEquals("supertoken", config.getAutoOAuthToken());
assertTrue(config.getCaCertFile().endsWith("testns/ca.pem".replace("/", File.separator)));
assertTrue(new File(config.getCaCertFile()).isAbsolute());
}
Expand All @@ -320,7 +320,7 @@ void testWithMultipleKubeConfigAndOverrideContext() {

assertEquals("https://172.28.128.4:8443/", config.getMasterUrl());
assertEquals("production", config.getNamespace());
assertEquals("supertoken", config.getOauthToken());
assertEquals("supertoken", config.getAutoOAuthToken());
assertTrue(config.getCaCertFile().endsWith("testns/ca.pem".replace("/", File.separator)));
assertTrue(new File(config.getCaCertFile()).isAbsolute());
}
Expand All @@ -334,7 +334,7 @@ void testWithKubeConfigAndSystemProperties() {
assertNotNull(config);
assertEquals("http://somehost:80/", config.getMasterUrl());
assertEquals("testns", config.getNamespace());
assertEquals("token", config.getOauthToken());
assertEquals("token", config.getAutoOAuthToken());
assertEquals(new File(TEST_KUBECONFIG_FILE), config.getFile());
}

Expand All @@ -349,7 +349,7 @@ void testWithKubeConfigAndSytemPropertiesAndBuilder() {

assertNotNull(config);
assertEquals("http://somehost:80/", config.getMasterUrl());
assertEquals("token", config.getOauthToken());
assertEquals("token", config.getAutoOAuthToken());
assertEquals("testns2", config.getNamespace());
}

Expand Down Expand Up @@ -483,7 +483,7 @@ void honorClientAuthenticatorCommands() throws Exception {

Config config = Config.autoConfigure(null);
assertNotNull(config);
assertEquals("HELLO WORLD", config.getOauthToken());
assertEquals("HELLO WORLD", config.getAutoOAuthToken());
}

@Test
Expand All @@ -498,7 +498,7 @@ void should_accept_client_authentication_commands_with_null_args() throws Except

Config config = Config.autoConfigure(null);
assertNotNull(config);
assertEquals("HELLO", config.getOauthToken());
assertEquals("HELLO", config.getAutoOAuthToken());
} finally {
System.clearProperty(Config.KUBERNETES_KUBECONFIG_FILE);
}
Expand All @@ -517,7 +517,7 @@ void should_accept_client_authentication_commands_args_with_spaces() throws Exce

Config config = Config.autoConfigure(null);
assertNotNull(config);
assertEquals("HELLO W O R L D", config.getOauthToken());
assertEquals("HELLO W O R L D", config.getAutoOAuthToken());
} finally {
System.clearProperty(Config.KUBERNETES_KUBECONFIG_FILE);
}
Expand All @@ -536,7 +536,7 @@ void should_accept_client_authentication_commands_with_spaces() throws Exception

Config config = Config.autoConfigure(null);
assertNotNull(config);
assertEquals("HELLO WORLD", config.getOauthToken());
assertEquals("HELLO WORLD", config.getAutoOAuthToken());
} finally {
System.clearProperty(Config.KUBERNETES_KUBECONFIG_FILE);
}
Expand All @@ -549,7 +549,9 @@ void shouldBeUsedTokenSuppliedByProvider() {
.withOauthTokenProvider(() -> "PROVIDER_TOKEN")
.build();

assertEquals("PROVIDER_TOKEN", config.getOauthToken());
// this is mostly a configuration error, and
// the provider does not modify the oauthtoken field
assertEquals("oauthToken", config.getOauthToken());
}

@Test
Expand All @@ -567,7 +569,7 @@ void testKubeConfigWithAuthConfigProvider() throws URISyntaxException {
assertEquals("https://172.28.128.4:8443/", config.getMasterUrl());
assertEquals(
"eyJraWQiOiJDTj1vaWRjaWRwLnRyZW1vbG8ubGFuLCBPVT1EZW1vLCBPPVRybWVvbG8gU2VjdXJpdHksIEw9QXJsaW5ndG9uLCBTVD1WaXJnaW5pYSwgQz1VUy1DTj1rdWJlLWNhLTEyMDIxNDc5MjEwMzYwNzMyMTUyIiwiYWxnIjoiUlMyNTYifQ.eyJpc3MiOiJodHRwczovL29pZGNpZHAudHJlbW9sby5sYW46ODQ0My9hdXRoL2lkcC9PaWRjSWRQIiwiYXVkIjoia3ViZXJuZXRlcyIsImV4cCI6MTQ4MzU0OTUxMSwianRpIjoiMm96US15TXdFcHV4WDlHZUhQdy1hZyIsImlhdCI6MTQ4MzU0OTQ1MSwibmJmIjoxNDgzNTQ5MzMxLCJzdWIiOiI0YWViMzdiYS1iNjQ1LTQ4ZmQtYWIzMC0xYTAxZWU0MWUyMTgifQ.w6p4J_6qQ1HzTG9nrEOrubxIMb9K5hzcMPxc9IxPx2K4xO9l-oFiUw93daH3m5pluP6K7eOE6txBuRVfEcpJSwlelsOsW8gb8VJcnzMS9EnZpeA0tW_p-mnkFc3VcfyXuhe5R3G7aa5d8uHv70yJ9Y3-UhjiN9EhpMdfPAoEB9fYKKkJRzF7utTTIPGrSaSU6d2pcpfYKaxIwePzEkT4DfcQthoZdy9ucNvvLoi1DIC-UocFD8HLs8LYKEqSxQvOcvnThbObJ9af71EwmuE21fO5KzMW20KtAeget1gnldOosPtz1G5EwvaQ401-RPQzPGMVBld0_zMCAwZttJ4knw",
config.getOauthToken());
config.getAutoOAuthToken());
}

@Test
Expand Down Expand Up @@ -605,13 +607,17 @@ void testEmptyConfig() {
.satisfies(e -> assertThat(e.getUserAgent()).isNotNull());
}

private void assertConfig(Config config) {
private void assertConfig(Config config, boolean autoToken) {
assertNotNull(config);
assertTrue(config.isTrustCerts());
assertTrue(config.isDisableHostnameVerification());
assertEquals("http://somehost:80/", config.getMasterUrl());
assertEquals("testns", config.getNamespace());
assertEquals("token", config.getOauthToken());
if (autoToken) {
assertEquals("token", config.getAutoOAuthToken());
} else {
assertEquals("token", config.getOauthToken());
}
assertEquals("user", config.getUsername());
assertEquals("pass", config.getPassword());
assertEquals("/path/to/cert", config.getCaCertFile());
Expand Down
4 changes: 2 additions & 2 deletions ...ent-api/src/test/java/io/fabric8/kubernetes/client/utils/TokenRefreshInterceptorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ void refreshShouldNotOverwriteExistingToken() throws Exception {
final boolean result = tokenRefreshInterceptor
.afterFailure(new StandardHttpRequest.Builder(), new TestHttpResponse<>().withCode(401), null).get();
// Then
assertThat(result).isTrue();
assertThat(result).isFalse();
assertThat(originalConfig).hasFieldOrPropertyWithValue("oauthToken", "existing-token");
}

Expand All @@ -132,7 +132,7 @@ void refresh_whenNoAuthProvider_thenShouldInheritTokenFromOldConfig() throws Exc
final boolean result = tokenRefreshInterceptor
.afterFailure(new StandardHttpRequest.Builder(), new TestHttpResponse<>().withCode(401), null).get();
// Then
assertThat(result).isTrue();
assertThat(result).isFalse();
assertThat(originalConfig).hasFieldOrPropertyWithValue("oauthToken", "existing-token");
}

Expand Down
Loading

0 comments on commit 39b526c

Please sign in to comment.