Skip to content

Commit

Permalink
Add drop auth headers redirect strategy for custom registry
Browse files Browse the repository at this point in the history 
- Add same auth header drop with custom registry as with s3
- Fixes gh-5548
  • Loading branch information
CG committed Nov 16, 2023
1 parent 556658b commit bacaf22
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 32 deletions.
20 changes: 14 additions & 6 deletions ...configuration/metadata/ApplicationConfigurationMetadataResolverAutoConfigurationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

import com.fasterxml.jackson.core.JsonProcessingException;
Expand Down Expand Up @@ -48,8 +49,7 @@
import org.springframework.web.client.RestTemplate;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -215,10 +215,18 @@ public ContainerImageRestTemplateFactory containerImageRestTemplateFactory(
@Qualifier("containerRestTemplate") RestTemplate containerRestTemplate,
@Qualifier("containerRestTemplateWithHttpProxy") RestTemplate containerRestTemplateWithHttpProxy) {
ContainerImageRestTemplateFactory containerImageRestTemplateFactory = Mockito.mock(ContainerImageRestTemplateFactory.class);
when(containerImageRestTemplateFactory.getContainerRestTemplate(eq(true), eq(false))).thenReturn(noSslVerificationContainerRestTemplate);
when(containerImageRestTemplateFactory.getContainerRestTemplate(eq(true), eq(true))).thenReturn(noSslVerificationContainerRestTemplateWithHttpProxy);
when(containerImageRestTemplateFactory.getContainerRestTemplate(eq(false), eq(false))).thenReturn(containerRestTemplate);
when(containerImageRestTemplateFactory.getContainerRestTemplate(eq(false), eq(true))).thenReturn(containerRestTemplateWithHttpProxy);
when(containerImageRestTemplateFactory.getContainerRestTemplate(eq(true), eq(false),
anyMap()))
.thenReturn(noSslVerificationContainerRestTemplate);
when(containerImageRestTemplateFactory.getContainerRestTemplate(eq(true), eq(true),
anyMap()))
.thenReturn(noSslVerificationContainerRestTemplateWithHttpProxy);
when(containerImageRestTemplateFactory.getContainerRestTemplate(eq(false), eq(false),
anyMap()))
.thenReturn(containerRestTemplate);
when(containerImageRestTemplateFactory.getContainerRestTemplate(eq(false), eq(true),
anyMap()))
.thenReturn(containerRestTemplateWithHttpProxy);
return containerImageRestTemplateFactory;
}

Expand Down
6 changes: 2 additions & 4 deletions ...ramework/cloud/dataflow/container/registry/DefaultContainerImageMetadataResolverTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@

import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand All @@ -68,7 +66,7 @@ public class DefaultContainerImageMetadataResolverTest {
public void init() {
MockitoAnnotations.initMocks(this);

when(containerImageRestTemplateFactory.getContainerRestTemplate(anyBoolean(), anyBoolean())).thenReturn(mockRestTemplate);
when(containerImageRestTemplateFactory.getContainerRestTemplate(anyBoolean(), anyBoolean(), anyMap())).thenReturn(mockRestTemplate);

// DockerHub registry configuration by default.
ContainerRegistryConfiguration dockerHubAuthConfig = new ContainerRegistryConfiguration();
Expand Down
6 changes: 2 additions & 4 deletions ...er/registry/authorization/DockerConfigJsonSecretToRegistryConfigurationConverterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.when;

/**
Expand All @@ -61,7 +59,7 @@ public class DockerConfigJsonSecretToRegistryConfigurationConverterTest {
@Before
public void init() {
MockitoAnnotations.initMocks(this);
when(containerImageRestTemplateFactory.getContainerRestTemplate(anyBoolean(), anyBoolean())).thenReturn(mockRestTemplate);
when(containerImageRestTemplateFactory.getContainerRestTemplate(anyBoolean(), anyBoolean(), anyMap())).thenReturn(mockRestTemplate);
converter = new DockerConfigJsonSecretToRegistryConfigurationConverter(new ContainerRegistryProperties(), containerImageRestTemplateFactory);
}

Expand Down
17 changes: 10 additions & 7 deletions .../springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import java.security.NoSuchAlgorithmException;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;

Expand Down Expand Up @@ -123,11 +125,11 @@ public ContainerImageRestTemplateFactory(RestTemplateBuilder restTemplateBuilder
this.restTemplateCache = new ConcurrentHashMap();
}

public RestTemplate getContainerRestTemplate(boolean skipSslVerification, boolean withHttpProxy) {
public RestTemplate getContainerRestTemplate(boolean skipSslVerification, boolean withHttpProxy, Map<String, String> extra) {
try {
CacheKey cacheKey = CacheKey.of(skipSslVerification, withHttpProxy);
if (!this.restTemplateCache.containsKey(cacheKey)) {
RestTemplate restTemplate = createContainerRestTemplate(skipSslVerification, withHttpProxy);
RestTemplate restTemplate = createContainerRestTemplate(skipSslVerification, withHttpProxy, extra);
this.restTemplateCache.putIfAbsent(cacheKey, restTemplate);
}
return this.restTemplateCache.get(cacheKey);
Expand All @@ -139,12 +141,12 @@ public RestTemplate getContainerRestTemplate(boolean skipSslVerification, boolea
}
}

private RestTemplate createContainerRestTemplate(boolean skipSslVerification, boolean withHttpProxy)
private RestTemplate createContainerRestTemplate(boolean skipSslVerification, boolean withHttpProxy, Map<String, String> extra)
throws NoSuchAlgorithmException, KeyManagementException {

if (!skipSslVerification) {
// Create a RestTemplate that uses custom request factory
return this.initRestTemplate(HttpClients.custom(), withHttpProxy);
return this.initRestTemplate(HttpClients.custom(), withHttpProxy, extra);
}

// Trust manager that blindly trusts all SSL certificates.
Expand All @@ -170,10 +172,11 @@ public void checkServerTrusted(java.security.cert.X509Certificate[] certs, Strin
HttpClients.custom()
.setSSLContext(sslContext)
.setSSLHostnameVerifier(NoopHostnameVerifier.INSTANCE),
withHttpProxy);
withHttpProxy,
extra);
}

private RestTemplate initRestTemplate(HttpClientBuilder clientBuilder, boolean withHttpProxy) {
private RestTemplate initRestTemplate(HttpClientBuilder clientBuilder, boolean withHttpProxy, Map<String, String> extra) {

clientBuilder.setDefaultRequestConfig(RequestConfig.custom().setCookieSpec(CookieSpecs.STANDARD).build());

Expand All @@ -189,7 +192,7 @@ private RestTemplate initRestTemplate(HttpClientBuilder clientBuilder, boolean w
HttpComponentsClientHttpRequestFactory customRequestFactory =
new HttpComponentsClientHttpRequestFactory(
clientBuilder
.setRedirectStrategy(new DropAuthorizationHeaderRequestRedirectStrategy())
.setRedirectStrategy(new DropAuthorizationHeaderRequestRedirectStrategy(extra))
// Azure redirects may contain double slashes and on default those are normilised
.setDefaultRequestConfig(RequestConfig.custom().setNormalizeUri(false).build())
.build());
Expand Down
10 changes: 7 additions & 3 deletions .../java/org/springframework/cloud/dataflow/container/registry/ContainerRegistryService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ public List<String> getTags(String registryName, String repositoryName) {
.build().expand(repositoryName);

RestTemplate requestRestTemplate = this.containerImageRestTemplateFactory.getContainerRestTemplate(
containerRegistryConfiguration.isDisableSslVerification(), containerRegistryConfiguration.isUseHttpProxy());
containerRegistryConfiguration.isDisableSslVerification(),
containerRegistryConfiguration.isUseHttpProxy(),
containerRegistryConfiguration.getExtra());

ResponseEntity<Map> manifest = requestRestTemplate.exchange(manifestUriComponents.toUri(),
HttpMethod.GET, new HttpEntity<>(httpHeaders), Map.class);
Expand Down Expand Up @@ -145,7 +147,9 @@ public Map getRepositories(String registryName) {


RestTemplate requestRestTemplate = this.containerImageRestTemplateFactory.getContainerRestTemplate(
containerRegistryConfiguration.isDisableSslVerification(), containerRegistryConfiguration.isUseHttpProxy());
containerRegistryConfiguration.isDisableSslVerification(),
containerRegistryConfiguration.isUseHttpProxy(),
containerRegistryConfiguration.getExtra());

ResponseEntity<Map> manifest = requestRestTemplate.exchange(manifestUriComponents.toUri(),
HttpMethod.GET, new HttpEntity<>(httpHeaders), Map.class);
Expand Down Expand Up @@ -184,7 +188,7 @@ public ContainerRegistryRequest getRegistryRequest(String imageName) {
}

RestTemplate requestRestTemplate = this.containerImageRestTemplateFactory.getContainerRestTemplate(
registryConf.isDisableSslVerification(), registryConf.isUseHttpProxy());
registryConf.isDisableSslVerification(), registryConf.isUseHttpProxy(), registryConf.getExtra());

return new ContainerRegistryRequest(containerImage, registryConf, authHttpHeaders, requestRestTemplate);
}
Expand Down
2 changes: 1 addition & 1 deletion ...tainer/registry/authorization/DockerConfigJsonSecretToRegistryConfigurationConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ private String replaceDefaultDockerRegistryServerUrl(String dockerConfigJsonRegi
public Optional<String> getDockerTokenServiceUri(String registryHost, boolean disableSSl, boolean useHttpProxy) {

try {
RestTemplate restTemplate = this.containerImageRestTemplate.getContainerRestTemplate(disableSSl, useHttpProxy);
RestTemplate restTemplate = this.containerImageRestTemplate.getContainerRestTemplate(disableSSl, useHttpProxy, Collections.emptyMap());
String host = registryHost;
Integer port = null;
if (registryHost.contains(":")) {
Expand Down
6 changes: 4 additions & 2 deletions ...ework/cloud/dataflow/container/registry/authorization/DockerOAuth2RegistryAuthorizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ public HttpHeaders getAuthorizationHeaders(ContainerImage containerImage, Contai
}

private RestTemplate getRestTemplate(ContainerRegistryConfiguration registryConfiguration) {
return this.containerImageRestTemplate.getContainerRestTemplate(registryConfiguration.isDisableSslVerification(),
registryConfiguration.isUseHttpProxy());
return this.containerImageRestTemplate.getContainerRestTemplate(
registryConfiguration.isDisableSslVerification(),
registryConfiguration.isUseHttpProxy(),
registryConfiguration.getExtra());
}
}
31 changes: 30 additions & 1 deletion ...flow/container/registry/authorization/DropAuthorizationHeaderRequestRedirectStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import java.net.URI;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;

import org.apache.commons.lang3.StringUtils;
import org.apache.http.Header;
Expand All @@ -30,9 +32,10 @@
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.impl.client.DefaultRedirectStrategy;
import org.apache.http.protocol.HttpContext;
import org.springframework.cloud.dataflow.container.registry.ContainerRegistryConfiguration;

/**
* Both Amazon and Azure Container Registry services require special treatment for the Authorization headers when the
* Amazon, Azure and Custom Container Registry services require special treatment for the Authorization headers when the
* HTTP request are forwarded to 3rd party services.
*
* Amazon:
Expand All @@ -53,11 +56,16 @@
* Azure have same type of issues as S3 so header needs to be dropped as well.
* (https://docs.microsoft.com/en-us/azure/container-registry/container-registry-faq#authentication-information-is-not-given-in-the-correct-format-on-direct-rest-api-calls)
*
* Custom:
* Custom Container Registry may have same type of issues as S3 so header needs to be dropped as well.
*
* @author Adam J. Weigold
* @author Janne Valkealahti
* @author Christian Tzolov
* @author Cheng Guan Poh
*/
public class DropAuthorizationHeaderRequestRedirectStrategy extends DefaultRedirectStrategy {
public static final String CUSTOM_REGISTRY = "custom-registry";

private static final String AMZ_CREDENTIAL = "X-Amz-Credential";

Expand All @@ -67,6 +75,16 @@ public class DropAuthorizationHeaderRequestRedirectStrategy extends DefaultRedir

private static final String BASIC_AUTH = "Basic";

/**
* Additional registry specific configuration properties.
* Usually used inside the Registry authorizer implementations. For example check the AwsEcrAuthorizer implementation.
*/
private Map<String, String> extra = new HashMap<>();

public DropAuthorizationHeaderRequestRedirectStrategy(Map<String, String> extra) {
this.extra = extra;
}

@Override
public HttpUriRequest getRedirect(final HttpRequest request, final HttpResponse response,
final HttpContext context) throws ProtocolException {
Expand Down Expand Up @@ -100,6 +118,17 @@ protected boolean isDropHeader(String name, String value) {
}
}

// Handle custom requests
if (extra.containsKey(CUSTOM_REGISTRY)) {
if (request.getRequestLine().getUri().contains(extra.get(CUSTOM_REGISTRY))) {
final String method = request.getRequestLine().getMethod();
if (StringUtils.isNoneEmpty(method)
&& (method.equalsIgnoreCase(HttpHead.METHOD_NAME) || method.equalsIgnoreCase(HttpGet.METHOD_NAME))) {
return new DropAuthorizationHeaderHttpRequestBase(httpUriRequest.getURI(), method);
}
}
}

return httpUriRequest;
}

Expand Down
6 changes: 2 additions & 4 deletions ...ry/authorization/DockerConfigJsonSecretToContainerRegistryConfigurationConverterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.when;

/**
Expand All @@ -62,7 +60,7 @@ public class DockerConfigJsonSecretToContainerRegistryConfigurationConverterTest
@Before
public void init() {
MockitoAnnotations.initMocks(this);
when(containerImageRestTemplateFactory.getContainerRestTemplate(anyBoolean(), anyBoolean())).thenReturn(mockRestTemplate);
when(containerImageRestTemplateFactory.getContainerRestTemplate(anyBoolean(), anyBoolean(), anyMap())).thenReturn(mockRestTemplate);
converter = new DockerConfigJsonSecretToRegistryConfigurationConverter(new ContainerRegistryProperties(), containerImageRestTemplateFactory);
}

Expand Down

0 comments on commit bacaf22

Please sign in to comment.