Skip to content

Commit

Permalink
Fix #2529: SelfSubjectAccessReview broken with OpenShiftClient
Browse files Browse the repository at this point in the history
Right now all create only resources in `authorization.k8s.io` apiGroup
seem to be returning HTTP_OK/HTTP_CREATED even if there are no bearer
token headers set in request. Due to this, we were always skipping token
update since we only check for HTTP_UNAUTHORIZED/HTTP_FORBIDDEN response
codes.

Added an exception in OpenShiftOAuthInterceptor to always force token
update in case url contains Kubernetes/OpenShift authorization apiGroup
  • Loading branch information
rohanKanojia authored and manusa committed May 18, 2021
1 parent a2b67bf commit bb0c32d
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* Fix #3038: Upgrade TLS versions in mock servers to 1.2.
* Fix #3037: Account for JsonProperty annotations when computing properties' name
* Fix #3014: Resync Future is canceled and resync executor is shutdown on informer stop
* Fix #2529: SelfSubjectAccessReview not working with OpenShiftClient
* Fix #2978: Fix SharedInformer NPE on initial requests while syncing
* Fix #2989: serialization will generate valid yaml when using subtypes
* Fix #2991: reduced the level of ReflectWatcher event received log
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* 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;

import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReview;
import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReviewBuilder;
import io.fabric8.openshift.client.OpenShiftClient;
import org.arquillian.cube.kubernetes.api.Session;
import org.arquillian.cube.openshift.impl.requirement.RequiresOpenshift;
import org.arquillian.cube.requirement.ArquillianConditionalRunner;
import org.jboss.arquillian.test.api.ArquillianResource;
import org.junit.Test;
import org.junit.runner.RunWith;

import static org.junit.Assert.assertTrue;

@RunWith(ArquillianConditionalRunner.class)
@RequiresOpenshift
public class SelfSubjectAccessReviewIT {
@ArquillianResource
OpenShiftClient client;

@ArquillianResource
Session session;

@Test
public void create() {
// Given
SelfSubjectAccessReview ssar = new SelfSubjectAccessReviewBuilder()
.withNewSpec()
.withNewResourceAttributes()
.withGroup("apps")
.withResource("deployments")
.withVerb("create")
.withNamespace(session.getNamespace())
.endResourceAttributes()
.endSpec()
.build();

// When
ssar = client.authorization().v1().selfSubjectAccessReview().create(ssar);

// Then
assertTrue(ssar.getStatus().getAllowed());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/**
* 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;

import io.fabric8.openshift.api.model.SubjectAccessReview;
import io.fabric8.openshift.api.model.SubjectAccessReviewBuilder;
import io.fabric8.openshift.api.model.SubjectAccessReviewResponse;
import io.fabric8.openshift.client.OpenShiftClient;
import org.arquillian.cube.kubernetes.api.Session;
import org.arquillian.cube.openshift.impl.requirement.RequiresOpenshift;
import org.arquillian.cube.requirement.ArquillianConditionalRunner;
import org.jboss.arquillian.test.api.ArquillianResource;
import org.junit.Test;
import org.junit.runner.RunWith;

import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;


@RunWith(ArquillianConditionalRunner.class)
@RequiresOpenshift
public class SubjectAccessReviewIT {
@ArquillianResource
OpenShiftClient client;

@ArquillianResource
Session session;

@Test
public void testCreate() {
// Given
SubjectAccessReview sar = new SubjectAccessReviewBuilder()
.withResource("Pod")
.withVerb("get")
.withNamespace(session.getNamespace())
.build();

// When
SubjectAccessReviewResponse response = client.subjectAccessReviews().create(sar);

// Then
assertNotNull(response);
assertTrue(response.getAllowed());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
import java.net.URL;
import java.util.concurrent.atomic.AtomicReference;

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

public class OpenShiftOAuthInterceptor implements Interceptor {

private static final String AUTHORIZATION = "Authorization";
Expand All @@ -41,6 +44,8 @@ public class OpenShiftOAuthInterceptor implements Interceptor {

private static final String BEFORE_TOKEN = "access_token=";
private static final String AFTER_TOKEN = "&expires";
private static final String K8S_AUTHORIZATION = "authorization.k8s.io";
private static final String OPENSHIFT_AUTHORIZATION = "authorization.openshift.io";

private final OkHttpClient client;
private final OpenShiftConfig config;
Expand Down Expand Up @@ -68,7 +73,7 @@ public Response intercept(Chain chain) throws IOException {
Response response = chain.proceed(request);

//If response is Forbidden or Unauthorized, try to obtain a token via authorize() or via config.
if (response.code() != 401 && response.code() != 403) {
if (isResponseSuccessful(request, response)) {
return response;
} else if (Utils.isNotNullOrEmpty(config.getUsername()) && Utils.isNotNullOrEmpty(config.getPassword())) {
synchronized (client) {
Expand Down Expand Up @@ -141,4 +146,14 @@ private String authorize() {
throw KubernetesClientException.launderThrowable(e);
}
}

private boolean isResponseSuccessful(Request request, Response response) {
String url = request.url().toString();
// always retry in case of authorization endpoints; since they also return 200 when no
// authorization header is provided
if (url.contains(K8S_AUTHORIZATION) || url.contains(OPENSHIFT_AUTHORIZATION)) {
return false;
}
return response.code() != HTTP_UNAUTHORIZED && response.code() != HTTP_FORBIDDEN;
}
}

0 comments on commit bb0c32d

Please sign in to comment.