Fix redirect failures from apache client 5.x

spring-cloud-dataflow-configuration-metadata/pom.xml

@@ -35,6 +35,10 @@
 			<groupId>org.springframework</groupId>
 			<artifactId>spring-web</artifactId>
 		</dependency>
+		<dependency>
+			<groupId>io.projectreactor.netty</groupId>
+			<artifactId>reactor-netty</artifactId>
+		</dependency>
 		<dependency>
 			<groupId>com.fasterxml.jackson.core</groupId>
 			<artifactId>jackson-core</artifactId>

spring-cloud-dataflow-configuration-metadata/src/test/java/org/springframework/cloud/dataflow/container/registry/authorization/DropAuthorizationHeaderOnContainerTestManual.java

@@ -0,0 +1,82 @@
+/*
+ * Copyright 2020-2020 the original author or authors.
+ *
+ * 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
+ *
+ *      https://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 org.springframework.cloud.dataflow.container.registry.authorization;
+
+import java.util.Collections;
+import java.util.Map;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.boot.test.autoconfigure.web.client.AutoConfigureWebClient;
+import org.springframework.cloud.dataflow.configuration.metadata.ApplicationConfigurationMetadataResolverAutoConfiguration;
+import org.springframework.cloud.dataflow.configuration.metadata.container.DefaultContainerImageMetadataResolver;
+import org.springframework.cloud.dataflow.container.registry.ContainerRegistryAutoConfiguration;
+import org.springframework.cloud.dataflow.container.registry.ContainerRegistryConfiguration;
+import org.springframework.cloud.dataflow.container.registry.ContainerRegistryProperties;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Import;
+import org.springframework.context.annotation.Primary;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * @author Adam J. Weigold
+ * @author Corneil du Plessis
+ */
+public class DropAuthorizationHeaderOnContainerTestManual {
+
+	private AnnotationConfigApplicationContext context;
+
+	@AfterEach
+	void clean() {
+		if (context != null) {
+			context.close();
+		}
+		context = null;
+	}
+
+	@Test
+	void testContainerImageLabels() {
+		context = new AnnotationConfigApplicationContext(TestApplication.class);
+		ContainerRegistryProperties registryProperties = context.getBean(ContainerRegistryProperties.class);
+		DefaultContainerImageMetadataResolver imageMetadataResolver = context.getBean(DefaultContainerImageMetadataResolver.class);
+		String imageNameAndTag = "springcloudstream/s3-sink-rabbit:5.0.0";
+		Map<String, String> imageLabels = imageMetadataResolver.getImageLabels(registryProperties.getDefaultRegistryHost() + "/" + imageNameAndTag);
+		System.out.println("imageLabels:" + imageLabels.keySet());
+		assertThat(imageLabels).containsKey("org.springframework.boot.spring-configuration-metadata.json");
+	}
+
+	@Import({ContainerRegistryAutoConfiguration.class, ApplicationConfigurationMetadataResolverAutoConfiguration.class})
+	@AutoConfigureWebClient
+	static class TestApplication {
+		@Bean
+		@Primary
+		ContainerRegistryProperties containerRegistryProperties() {
+			ContainerRegistryProperties properties = new ContainerRegistryProperties();
+			ContainerRegistryConfiguration registryConfiguration = new ContainerRegistryConfiguration();
+			registryConfiguration.setRegistryHost("registry-1.docker.io");
+			registryConfiguration.setAuthorizationType(ContainerRegistryConfiguration.AuthorizationType.dockeroauth2);
+			registryConfiguration.setUser("<Docker username>");
+			registryConfiguration.setSecret("<Docker PAT>");
+			properties.setRegistryConfigurations(Collections.singletonMap("registry-1.docker.io", registryConfiguration));
+
+			return properties;
+		}
+	}
+}

spring-cloud-dataflow-configuration-metadata/src/test/java/org/springframework/cloud/dataflow/container/registry/authorization/DropAuthorizationHeaderOnSignedS3RequestRedirectStrategyTest.java

@@ -42,8 +42,6 @@
  * @author Adam J. Weigold
  * @author Corneil du Plessis
  */
-//TODO: Boot3x followup
-@Disabled("TODO: Boot3x `org.springframework.web.client.HttpClientErrorException$BadRequest: 400 : [no body]` is thrown by REST Template")
 public class DropAuthorizationHeaderOnSignedS3RequestRedirectStrategyTest {
 	@RegisterExtension
 	public final static S3SignedRedirectRequestServerResource s3SignedRedirectRequestServerResource =

spring-cloud-dataflow-container-registry/pom.xml

@@ -42,6 +42,10 @@
 			<groupId>org.springframework</groupId>
 			<artifactId>spring-web</artifactId>
 		</dependency>
+		<dependency>
+			<groupId>io.projectreactor.netty</groupId>
+			<artifactId>reactor-netty</artifactId>
+		</dependency>
 		<dependency>
 			<groupId>com.fasterxml.jackson.core</groupId>
 			<artifactId>jackson-core</artifactId>

spring-cloud-dataflow-container-registry/src/main/java/org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java

@@ -16,53 +16,44 @@
 
 package org.springframework.cloud.dataflow.container.registry;
 
-import java.security.KeyManagementException;
-import java.security.NoSuchAlgorithmException;
-import java.security.cert.X509Certificate;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.ConcurrentHashMap;
 
-import javax.net.ssl.SSLContext;
-import javax.net.ssl.TrustManager;
-import javax.net.ssl.X509TrustManager;
-
-import org.apache.hc.client5.http.config.RequestConfig;
-import org.apache.hc.client5.http.cookie.StandardCookieSpec;
-import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
-import org.apache.hc.client5.http.impl.classic.HttpClients;
-import org.apache.hc.client5.http.impl.io.BasicHttpClientConnectionManager;
-import org.apache.hc.client5.http.socket.ConnectionSocketFactory;
-import org.apache.hc.client5.http.socket.PlainConnectionSocketFactory;
-import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
-import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
-import org.apache.hc.core5.http.HttpHost;
-import org.apache.hc.core5.http.config.Lookup;
-import org.apache.hc.core5.http.config.RegistryBuilder;
+import javax.net.ssl.SSLException;
+
+import io.netty.handler.codec.http.HttpHeaders;
+import io.netty.handler.codec.http.HttpMethod;
+import io.netty.handler.ssl.SslContext;
+import io.netty.handler.ssl.SslContextBuilder;
+import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
+import reactor.netty.http.client.HttpClient;
+import reactor.netty.http.client.HttpClientRequest;
+import reactor.netty.transport.ProxyProvider;
 
 import org.springframework.boot.web.client.RestTemplateBuilder;
-import org.springframework.cloud.dataflow.container.registry.authorization.DropAuthorizationHeaderRequestRedirectStrategy;
 import org.springframework.http.MediaType;
-import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
+import org.springframework.http.client.ClientHttpRequestFactory;
+import org.springframework.http.client.ReactorNettyClientRequestFactory;
 import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
 import org.springframework.web.client.RestTemplate;
 
-
 /**
- * On demand creates a cacheable {@link RestTemplate} instances for the purpose of the Container Registry access.
- * Created RestTemplates can be configured to use Http Proxy and/or bypassing the SSL verification.
+ * On demand creates a cacheable {@link RestTemplate} instances for the purpose of the
+ * Container Registry access. Created RestTemplates can be configured to use Http Proxy
+ * and/or bypassing the SSL verification.
  *
- * For configuring a Http Proxy in need to:
- *  1. Add http proxy configuration using the spring.cloud.dataflow.container.httpProxy.* properties.
- *  2. For every {@link ContainerRegistryConfiguration} that has to interact via the http proxy set the use-http-proxy
- *     flag to true. For example:
- *     <code>spring.cloud.dataflow.container.registry-configurations[reg-name].use-http-proxy=ture</code>
+ * For configuring a Http Proxy in need to: 1. Add http proxy configuration using the
+ * spring.cloud.dataflow.container.httpProxy.* properties. 2. For every
+ * {@link ContainerRegistryConfiguration} that has to interact via the http proxy set the
+ * use-http-proxy flag to true. For example:
+ * <code>spring.cloud.dataflow.container.registry-configurations[reg-name].use-http-proxy=ture</code>
  *
- * Following example configures the default (e.g. DockerHub) registry to use the HTTP Proxy (my-proxy.test:8080)
- * while the dockerhub-mirror and the private-snapshots registry configurations allow direct communication:
- * <code>
+ * Following example configures the default (e.g. DockerHub) registry to use the HTTP
+ * Proxy (my-proxy.test:8080) while the dockerhub-mirror and the private-snapshots
+ * registry configurations allow direct communication: <code>
  *     spring:
  *   cloud:
  *     dataflow:
@@ -86,6 +77,12 @@
  */
 public class ContainerImageRestTemplateFactory {
 
+	private static final String CUSTOM_REGISTRY = "custom-registry";
+	private static final String AMZ_CREDENTIAL = "X-Amz-Credential";
+	private static final String AUTHORIZATION_HEADER = "Authorization";
+	private static final String AZURECR_URI_SUFFIX = "azurecr.io";
+	private static final String BASIC_AUTH = "Basic";
+
 	private final RestTemplateBuilder restTemplateBuilder;
 
 	private final ContainerRegistryProperties properties;
@@ -153,67 +150,104 @@ public RestTemplate getContainerRestTemplate(boolean skipSslVerification, boolea
 		}
 	}
 
-	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, extra);
-		}
+	private RestTemplate createContainerRestTemplate(boolean skipSslVerification, boolean withHttpProxy, Map<String, String> extra) {
+		HttpClient client = httpClientBuilder(skipSslVerification, extra);
+		return initRestTemplate(client, withHttpProxy, extra);
+	}
 
-		// Trust manager that blindly trusts all SSL certificates.
-		TrustManager[] trustAllCerts = new TrustManager[] {
-				new X509TrustManager() {
-					public java.security.cert.X509Certificate[] getAcceptedIssuers() {
-						return new X509Certificate[0];
-					}
+	/**
+	 * 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:
+	 *   The Amazon S3 API supports two Authentication Methods (https://amzn.to/2Dg9sga):
+	 *   (1) HTTP Authorization header and (2) Query string parameters (often referred to as a pre-signed URL).
+	 *
+	 *   But only one auth mechanism is allowed at a time. If the http request contains both an Authorization header and
+	 *   an pre-signed URL parameters then an error is thrown.
+	 *
+	 *   Container Registries often use AmazonS3 as a backend object store. If HTTP Authorization header
+	 *   is used to authenticate with the Container Registry and then this registry redirect the request to a S3 storage
+	 *   using pre-signed URL authentication, the redirection will fail.
+	 *
+	 *   Solution is to implement a HTTP redirect strategy that removes the original Authorization headers when the request is
+	 *   redirected toward an Amazon signed URL.
+	 *
+	 * Azure:
+	 *   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
+	 * @author Corneil du Plessis
+	 */
+	private HttpClient httpClientBuilder(boolean skipSslVerification, Map<String, String> extra) {
 
-					public void checkClientTrusted(java.security.cert.X509Certificate[] certs, String authType) {
-					}
+		try {
+			SslContextBuilder builder = skipSslVerification
+					? SslContextBuilder.forClient().trustManager(InsecureTrustManagerFactory.INSTANCE)
+					: SslContextBuilder.forClient();
+			SslContext sslContext = builder.build();
+			HttpClient client = HttpClient.create().secure(sslContextSpec -> sslContextSpec.sslContext(sslContext));
 
-					public void checkServerTrusted(java.security.cert.X509Certificate[] certs, String authType) {
-					}
+			return client.followRedirect(true, (entries, httpClientRequest) -> {
+				if (shouldRemoveAuthorization(httpClientRequest, extra)) {
+					HttpHeaders httpHeaders = httpClientRequest.requestHeaders();
+					removeAuthorization(httpHeaders);
+					removeAuthorization(entries);
+					httpClientRequest.headers(httpHeaders);
 				}
-		};
-		SSLContext sslContext = SSLContext.getInstance("SSL");
-		// Install trust manager to SSL Context.
-		sslContext.init(null, trustAllCerts, new java.security.SecureRandom());
-
-		// Create a RestTemplate that uses custom request factory
-		return initRestTemplate(
-				httpClientBuilder(sslContext),
-				withHttpProxy,
-				extra);
+			});
+		}
+		catch (SSLException e) {
+			throw new RuntimeException(e);
+		}
+	}
+
+	private boolean shouldRemoveAuthorization(HttpClientRequest request, Map<String, String> extra) {
+		HttpMethod method = request.method();
+		if(method.equals(HttpMethod.GET) || method.equals(HttpMethod.HEAD)) {
+			if (request.uri().contains(AMZ_CREDENTIAL)) {
+				return true;
+			}
+			if (request.uri().contains(AZURECR_URI_SUFFIX)) {
+				return request.requestHeaders()
+					.entries()
+					.stream()
+					.anyMatch(entry -> entry.getKey().equalsIgnoreCase(AUTHORIZATION_HEADER)
+							&& entry.getValue().contains(BASIC_AUTH));
+			}
+			return extra.containsKey(CUSTOM_REGISTRY) && request.uri().contains(extra.get(CUSTOM_REGISTRY));
+		}
+		return false;
 	}
-	private HttpClientBuilder httpClientBuilder(SSLContext sslContext) {
-		// Register http/s connection factories
-		Lookup<ConnectionSocketFactory> connSocketFactoryLookup = RegistryBuilder.<ConnectionSocketFactory> create()
-			.register("https", new SSLConnectionSocketFactory(sslContext, NoopHostnameVerifier.INSTANCE))
-			.register("http", new PlainConnectionSocketFactory())
-			.build();
-		return HttpClients.custom()
-			.setConnectionManager(new BasicHttpClientConnectionManager(connSocketFactoryLookup));
+
+	private static void removeAuthorization(HttpHeaders headers) {
+		for(Map.Entry<String,String> entry: headers.entries()) {
+			if(entry.getKey().equalsIgnoreCase(org.springframework.http.HttpHeaders.AUTHORIZATION)) {
+				headers.remove(entry.getKey());
+				break;
+			}
+		}
 	}
-	private RestTemplate initRestTemplate(HttpClientBuilder clientBuilder, boolean withHttpProxy, Map<String, String> extra) {
 
-		clientBuilder.setDefaultRequestConfig(RequestConfig.custom().setCookieSpec(StandardCookieSpec.RELAXED).build());
+	private RestTemplate initRestTemplate(HttpClient client, boolean withHttpProxy, Map<String, String> extra) {
 
 		// Set the HTTP proxy if configured.
 		if (withHttpProxy) {
 			if (!properties.getHttpProxy().isEnabled()) {
 				throw new ContainerRegistryException("Registry Configuration uses a HttpProxy but non is configured!");
 			}
-			HttpHost proxy = new HttpHost(properties.getHttpProxy().getHost(), properties.getHttpProxy().getPort());
-			clientBuilder.setProxy(proxy);
+			ProxyProvider.Builder builder = ProxyProvider.builder().type(ProxyProvider.Proxy.HTTP).host(properties.getHttpProxy().getHost()).port(properties.getHttpProxy().getPort());
+			client.proxy(typeSpec -> builder.build());
 		}
-
-		HttpComponentsClientHttpRequestFactory customRequestFactory =
-				new HttpComponentsClientHttpRequestFactory(
-						clientBuilder
-								.setRedirectStrategy(new DropAuthorizationHeaderRequestRedirectStrategy(extra))
-								// Azure redirects may contain double slashes and on default those are normilised
-								.setDefaultRequestConfig(RequestConfig.custom().build())
-								.build());
+		// TODO what do we do with extra?
+		ClientHttpRequestFactory customRequestFactory = new ReactorNettyClientRequestFactory(client);
 
 		// DockerHub response's media-type is application/octet-stream although the content is in JSON.
 		// Similarly the Github CR response's media-type is always text/plain although the content is in JSON.