Skip to content

Commit

Permalink
fix (openshift-client) : OpenShiftOAuthInterceptor should not refresh…
Browse files Browse the repository at this point in the history
… on `403` response code (#4970)

Do not do token refresh on HTTP_FORBIDDEN response code in order to
align better with kubectl and oc

Signed-off-by: Rohan Kumar <[email protected]>
  • Loading branch information
rohanKanojia authored and manusa committed Mar 20, 2023
1 parent 3710714 commit 700dab3
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 5 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 #4970: OpenShiftOAuthInterceptor should not refresh on `403` response code

#### Dependency Upgrade

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
import java.util.Set;
import java.util.concurrent.CompletableFuture;

import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED;

/**
Expand Down Expand Up @@ -195,7 +194,7 @@ private boolean shouldProceed(HttpRequest request, HttpResponse<?> response) {
if (method.equals("POST") && RETRIABLE_RESOURCES.stream().anyMatch(url::endsWith)) {
return false;
}
return response.code() != HTTP_UNAUTHORIZED && response.code() != HTTP_FORBIDDEN;
return response.code() != HTTP_UNAUTHORIZED;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.http.HttpClient;
import io.fabric8.kubernetes.client.http.HttpRequest;
import io.fabric8.kubernetes.client.http.HttpResponse;
import io.fabric8.kubernetes.client.http.StandardHttpRequest;
import io.fabric8.kubernetes.client.http.TestHttpResponse;
import io.fabric8.kubernetes.client.utils.TokenRefreshInterceptor;
Expand All @@ -26,10 +28,13 @@
import java.net.HttpURLConnection;
import java.net.URI;
import java.util.Collections;
import java.util.concurrent.CompletableFuture;

import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED;
import static org.assertj.core.api.Assertions.assertThat;
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 {

Expand Down Expand Up @@ -72,8 +77,7 @@ void testTokenUsed() {

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

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

Expand All @@ -89,4 +93,54 @@ void testTokenRefreshedFromConfig() {
Mockito.verify(config).setOauthToken("token");
}

@Test
void afterFailure_whenResponseCode403_thenShouldNotRefresh() {
// Given
HttpClient client = Mockito.mock(HttpClient.class);
Config config = mockConfigRefresh();
HttpResponse<?> httpResponse = mockHttpResponse(HTTP_FORBIDDEN);
HttpRequest.Builder httpRequestBuilder = Mockito.mock(HttpRequest.Builder.class);
OpenShiftOAuthInterceptor interceptor = new OpenShiftOAuthInterceptor(client, config);

// When
CompletableFuture<Boolean> result = interceptor.afterFailure(httpRequestBuilder, httpResponse, null);

// Then
assertThat(result).isCompletedWithValue(false);
}

@Test
void afterFailure_whenResponseCode401_thenShouldRefresh() {
// Given
HttpClient client = Mockito.mock(HttpClient.class);
Config config = mockConfigRefresh();
HttpRequest.Builder httpRequestBuilder = Mockito.mock(HttpRequest.Builder.class);
HttpResponse<?> httpResponse = mockHttpResponse(HTTP_UNAUTHORIZED);
OpenShiftOAuthInterceptor interceptor = new OpenShiftOAuthInterceptor(client, config);

// When
CompletableFuture<Boolean> result = interceptor.afterFailure(httpRequestBuilder, httpResponse, null);

// Then
assertThat(result).isCompletedWithValue(true);
}

private Config mockConfigRefresh() {
Config config = Mockito.mock(Config.class);
Mockito.when(config.refresh()).thenReturn(config);
Mockito.when(config.getOauthToken()).thenReturn("token");
return config;
}

private HttpResponse<?> mockHttpResponse(int responseCode) {
HttpRequest httpRequest = Mockito.mock(HttpRequest.class);
HttpResponse<?> httpResponse = Mockito.mock(HttpResponse.class);
Mockito.when(httpRequest.method()).thenReturn("GET");
Mockito.when(httpRequest.uri())
.thenReturn(URI.create("http://www.example.com/apis/routes.openshift.io/namespaces/foo/routes"));
Mockito.when(httpResponse.request()).thenReturn(httpRequest);
Mockito.when(httpResponse.code()).thenReturn(responseCode);
return httpResponse;
}

}

0 comments on commit 700dab3

Please sign in to comment.