Remove unnecessary Basic Auth in Openshift requests.

[ismadirolas]

Is your enhancement related to a problem? Please describe

Related with #4673
When execute a request with Openshift client, the token requested after a failure (403) is stored in an AtomicReference for future requests in OpenShiftOAuthInterceptor.

But in the before method in the interceptor, there is a condition:

if (Utils.isNotNullOrEmpty(token)
        && (headers.headers(AUTHORIZATION).isEmpty() || Utils.isNullOrEmpty(headers.headers(AUTHORIZATION).get(0)))) {
      setAuthHeader(builder, token);
    }

that will never be true because the HttpClientUtils.HeaderInterceptor add a Basic Authorization header if your client has username and password configured; that it is necessary for the authentication requests performed by the OpenShiftOAuthInterceptor.
So, the result is the token stored is never reused, and every request to openshift performs the authentication.

Describe the solution you'd like

A way to avoid the Basic header for Openshift.

If I try to walkaround like this:

....
new KubernetesClientBuilder().withConfig(config)
        .withHttpClientBuilderConsumer(builder -> {
          builder.addOrReplaceInterceptor("HEADER", new NoBasicHeaderInterceptor(config));
        })
        .withHttpClientFactory(factory)
        .withTaskExecutor(executorService).build().adapt(OpenShiftClient.class);

....

  private static final class NoBasicHeaderInterceptor implements Interceptor {
    private final Config config;

    private NoBasicHeaderInterceptor(final Config config) {
      this.config = config;
    }

    @Override
    public Interceptor withConfig(final Config config) {
      return new NoBasicHeaderInterceptor(config);
    }

    @Override
    public void before(final BasicBuilder builder, final HttpHeaders headers) {
      if (Utils.isNotNullOrEmpty(this.config.getOauthToken())) {
        builder.header("Authorization", "Bearer " + this.config.getOauthToken());
      }
      if ((this.config.getCustomHeaders() != null) && !this.config.getCustomHeaders().isEmpty()) {
        for (final Map.Entry<String, String> entry : this.config.getCustomHeaders().entrySet()) {
          builder.header(entry.getKey(), entry.getValue());
        }
      }
      if ((this.config.getUserAgent() != null) && !this.config.getUserAgent().isEmpty()) {
        builder.setHeader("User-Agent", this.config.getUserAgent());
      }
    }
  }

then, the first Authentication request to Openshift fails with 401 (the one to .well-known/oauth-authorization-server endpoint). Curiously, the second one, to the url returned by the previous request, it adds manually the Basic header.

A possible solution could be avoid Basic header filter for openshift, and add it manually for the first request too.

Describe alternatives you've considered

No response

Additional context

No response

[shawkins]

Current behavior if you specify a username and password:

• base kubernetes: the basic auth header will be added. If auth fails, then the TokenRefreshInterceptor will refresh the token and set the header to use the bearer auth header.

• openshift: the basic auth header will be added. If auth fails, then the OpenShiftOAuthInterceptor will refresh the token and set the header to use the bearer token. It looks like it's always been this way, even in older releases.

So it looks like neither was designed to reuse in the case of setting a username and password.

My understanding at this point is that for base kubernetes if username and password are supplied, then only basic auth should be used - that would match the behavior of kubectl.

For openshift basic auth is not supported, so the default logic in HeaderInterceptor is definitely wrong. To confirm your expectation is that if the config already contains a token it should be used, then if it needs to be refreshed the username and password provided will be used?

[ismadirolas]

Correct, I think the username and password is required for OAuth authentication flux against Openshift, but I'm not an expert on that, when I try to overwrite the HeaderInterceptor to not send the basic header, I get a 401 from openshift when OpenShiftOAuthInterceptor gets the token for the first time.

[shawkins]

@ismadirolas see if #4702 makes sense for you. In particular - do you expect to use both the config username/password and the token/tokeprovider? My guess is that those should be separate mechanisms.

[ismadirolas]

hello @shawkins ,
username and password will be used only to get a new token from openshift oauth server, and this token should be added as Bearer token in the next requests to openshift, right?

#4702 looks good, we're looking forward to test it as soon as possible.

[shawkins]

username and password will be used only to get a new token from openshift oauth server, and this token should be added as Bearer token in the next requests to openshift, right?

Yes and saved / shared for future use.