Skip to content

Commit

Permalink
fix fabric8io#4698: adding tests, addressing smells, and adding docs
Browse files Browse the repository at this point in the history
  • Loading branch information
shawkins committed Jan 10, 2023
1 parent da3d243 commit 19c651e
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#### Bugs

#### Improvements
* Fix #4698: changes were made to improve authentication logic. If a username and password are specified and you are using a base KuberentesClient, then that will always be used as a basic auth header value. If a username and password are specified and you are using an OpenShiftClient, then a token will still be used if present, but upon an auth failure the username and password will be used to obtain a fresh token. If a new token is obtained it will be saved in the kubeconfig if one were used to create the Config.

#### Dependency Upgrade

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ public static CompletableFuture<String> resolveOIDCTokenFromAuthConfig(Config cu
String clientSecret = currentAuthProviderConfig.getOrDefault(CLIENT_SECRET_KUBECONFIG, "");
String idpCert = currentAuthProviderConfig.get(IDP_CERT_DATA);
if (isTokenRefreshSupported(currentAuthProviderConfig)) {
return getOIDCProviderTokenEndpointAndRefreshToken(issuer, clientId, refreshToken, clientSecret, accessToken, idpCert,
clientBuilder).thenApply(map -> {
return getOIDCProviderTokenEndpointAndRefreshToken(issuer, clientId, refreshToken, clientSecret, idpCert, clientBuilder)
.thenApply(map -> {
Object token = map.get(ID_TOKEN_PARAM);
if (token == null) {
LOGGER.warn("token response did not contain an id_token, either the scope \\\"openid\\\" wasn't " +
Expand Down Expand Up @@ -307,8 +307,7 @@ private static Map<String, String> getRequestBodyContentForRefresh(String client
}

private static CompletableFuture<Map<String, Object>> getOIDCProviderTokenEndpointAndRefreshToken(String issuer,
String clientId, String refreshToken, String clientSecret, String accessToken, String idpCert,
HttpClient.Builder clientBuilder) {
String clientId, String refreshToken, String clientSecret, String idpCert, HttpClient.Builder clientBuilder) {
HttpClient newClient = getDefaultHttpClientWithPemCert(idpCert, clientBuilder);
CompletableFuture<Map<String, Object>> result = getOIDCDiscoveryDocumentAsMap(newClient, issuer)
.thenCompose(wellKnownOpenIdConfiguration -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
*/
public class TokenRefreshInterceptor implements Interceptor {

public static final String AUTHORIZATION = "Authorization";

public static final String NAME = "TOKEN";

private final Config config;
Expand All @@ -56,11 +58,11 @@ public Interceptor withConfig(Config config) {
@Override
public void before(BasicBuilder headerBuilder, HttpHeaders headers) {
if (isBasicAuth()) {
headerBuilder.header("Authorization", HttpClientUtils.basicCredentials(config.getUsername(), config.getPassword()));
headerBuilder.header(AUTHORIZATION, HttpClientUtils.basicCredentials(config.getUsername(), config.getPassword()));
return;
}
if (Utils.isNotNullOrEmpty(config.getOauthToken())) {
headerBuilder.header("Authorization", "Bearer " + config.getOauthToken());
headerBuilder.header(AUTHORIZATION, "Bearer " + config.getOauthToken());
}
if (isTimeToRefresh()) {
refreshToken(headerBuilder);
Expand Down Expand Up @@ -104,7 +106,7 @@ private CompletableFuture<String> extractNewAccessTokenFrom(Config newestConfig)

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

updateLatestRefreshTimestamp();
Expand Down
9 changes: 8 additions & 1 deletion openshift-client/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>

<dependency>
<groupId>io.fabric8</groupId>
<artifactId>kubernetes-client-api</artifactId>
<type>test-jar</type>
<scope>test</scope>
</dependency>


<dependency>
<groupId>io.fabric8</groupId>
Expand All @@ -110,7 +118,6 @@
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-simple</artifactId>
<version>${slf4j.version}</version>
<scope>test</scope>
</dependency>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,7 @@ public CompletableFuture<Boolean> afterFailure(Builder builder, HttpResponse<?>
try {
// TODO: we may need some protection here or in the persistKubeConfigWithUpdatedAuthInfo
// if the user has modified the username via the requestconfig are we writing a valid value?
OpenIDConnectionUtils
.persistKubeConfigWithUpdatedAuthInfo(config, a -> {
a.setToken(t);
});
OpenIDConnectionUtils.persistKubeConfigWithUpdatedAuthInfo(config, a -> a.setToken(t));
} catch (IOException e) {
LOGGER.warn("failure while persisting new token into KUBECONFIG", e);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* Copyright (C) 2015 Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.fabric8.openshift.client.internal;

import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.http.HttpClient;
import io.fabric8.kubernetes.client.http.StandardHttpHeaders;
import io.fabric8.kubernetes.client.http.StandardHttpRequest;
import io.fabric8.kubernetes.client.http.TestHttpResponse;
import io.fabric8.kubernetes.client.utils.TokenRefreshInterceptor;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import java.net.HttpURLConnection;
import java.net.URI;
import java.util.Collections;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;

class OpenShiftOAuthInterceptorTest {

@Test
void testBasicAuthNotUsed() {
Config config = Config.empty();
config.setUsername("user");
config.setPassword("pass");

HttpClient client = Mockito.mock(HttpClient.class);

OpenShiftOAuthInterceptor interceptor = new OpenShiftOAuthInterceptor(client, config);

StandardHttpRequest.Builder builder = new StandardHttpRequest.Builder();
builder.uri("http://localhost");

interceptor.before(builder, new StandardHttpHeaders());

assertTrue(builder.build().headers(TokenRefreshInterceptor.AUTHORIZATION).isEmpty());
}

@Test
void testTokenUsed() {
Config config = Config.empty();
config.setUsername("user");
config.setPassword("pass");
config.setOauthToken("token");

HttpClient client = Mockito.mock(HttpClient.class);

OpenShiftOAuthInterceptor interceptor = new OpenShiftOAuthInterceptor(client, config);

StandardHttpRequest.Builder builder = new StandardHttpRequest.Builder();
builder.uri("http://localhost");

interceptor.before(builder, new StandardHttpHeaders());

assertEquals(Collections.singletonList("Bearer token"), builder.build().headers(TokenRefreshInterceptor.AUTHORIZATION));
}

@Test
void testTokenRefreshedFromConfig() {
Config config = Mockito.mock(Config.class, RETURNS_DEEP_STUBS);
Mockito.when(config.refresh().getOauthToken()).thenReturn("token");

HttpClient client = Mockito.mock(HttpClient.class);

OpenShiftOAuthInterceptor interceptor = new OpenShiftOAuthInterceptor(client, config);

StandardHttpRequest.Builder builder = new StandardHttpRequest.Builder();
builder.uri("http://localhost");

interceptor.afterFailure(builder, TestHttpResponse.from(HttpURLConnection.HTTP_UNAUTHORIZED, "not for you")
.withRequest(new StandardHttpRequest(null, URI.create("http://localhost"), "GET", null)));

assertEquals(Collections.singletonList("Bearer token"), builder.build().headers(TokenRefreshInterceptor.AUTHORIZATION));
Mockito.verify(config).setOauthToken("token");
}

}

0 comments on commit 19c651e

Please sign in to comment.