Skip to content

Commit

Permalink
Replace RedirectExec with modified chain handler.
Browse files Browse the repository at this point in the history 
Fixes #5989
  • Loading branch information
@corneil
corneil committed Oct 16, 2024
1 parent 88ddf2b commit 9e93935
Show file tree
Hide file tree
Showing 4 changed files with 315 additions and 98 deletions.
11 changes: 9 additions & 2 deletions .../dataflow/container/registry/authorization/support/S3SignedRedirectRequestController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
import java.util.Collections;
import java.util.Map;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.springframework.cloud.dataflow.container.registry.ContainerRegistryProperties;
import org.springframework.core.io.ByteArrayResource;
import org.springframework.core.io.Resource;
Expand All @@ -36,6 +39,7 @@
*/
@RestController
public class S3SignedRedirectRequestController {
private final static Logger logger = LoggerFactory.getLogger(S3SignedRedirectRequestController.class);

@RequestMapping("/service/token")
public ResponseEntity<Map<String, String>> getToken() {
Expand All @@ -53,6 +57,7 @@ public ResponseEntity<Resource> getManifests(@RequestHeader("Authorization") Str
@RequestMapping("/v2/test/s3-redirect-image/blobs/signed_redirect_digest")
public ResponseEntity<Map<String, String>> getBlobRedirect(@RequestHeader("Authorization") String token) {
if (!"bearer my_token_999".equals(token.trim().toLowerCase())) {
logger.info("getBlobRedirect=BAD_REQUEST");
return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
}
HttpHeaders redirectHeaders = new HttpHeaders();
Expand All @@ -63,13 +68,15 @@ public ResponseEntity<Map<String, String>> getBlobRedirect(@RequestHeader("Autho
"&X-Amz-Expires=1200" +
"&X-Amz-SignedHeaders=host" +
"&X-Amz-Signature=test");

logger.info("getBlobRedirect:{}", redirectHeaders);
return new ResponseEntity<>(redirectHeaders, HttpStatus.TEMPORARY_REDIRECT);
}

@RequestMapping("/test/docker/registry/v2/blobs/test/data")
public ResponseEntity<Resource> getSignedBlob(@RequestHeader Map<String, String> headers) {
if (!headers.containsKey("authorization")) {
logger.info("getSignedBlob:{}", headers);
if (headers.containsKey("authorization")) {
logger.info("getSignedBlob=BAD_REQUEST");
return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
}
return buildFromString("{\"config\": {\"Labels\": {\"foo\": \"bar\"} } }");
Expand Down
30 changes: 21 additions & 9 deletions .../springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,13 @@

import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.client5.http.cookie.StandardCookieSpec;
import org.apache.hc.client5.http.impl.ChainElement;
import org.apache.hc.client5.http.impl.DefaultSchemePortResolver;
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.impl.routing.DefaultProxyRoutePlanner;
import org.apache.hc.client5.http.impl.routing.DefaultRoutePlanner;
import org.apache.hc.client5.http.socket.ConnectionSocketFactory;
import org.apache.hc.client5.http.socket.PlainConnectionSocketFactory;
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
Expand All @@ -44,12 +48,12 @@

import org.springframework.boot.web.client.RestTemplateBuilder;
import org.springframework.cloud.dataflow.container.registry.authorization.DropAuthorizationHeaderRequestRedirectStrategy;
import org.springframework.cloud.dataflow.container.registry.authorization.SpecialRedirectExec;
import org.springframework.http.MediaType;
import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
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.
Expand Down Expand Up @@ -83,6 +87,7 @@
*
* @author Christian Tzolov
* @author Cheng Guan Poh
* @author Corneil du Plessis
*/
public class ContainerImageRestTemplateFactory {

Expand Down Expand Up @@ -197,23 +202,30 @@ private HttpClientBuilder httpClientBuilder(SSLContext sslContext) {
private RestTemplate initRestTemplate(HttpClientBuilder clientBuilder, boolean withHttpProxy, Map<String, String> extra) {

clientBuilder.setDefaultRequestConfig(RequestConfig.custom().setCookieSpec(StandardCookieSpec.RELAXED).build());

DefaultRoutePlanner routePlanner;
// 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);
routePlanner = new DefaultProxyRoutePlanner(proxy, DefaultSchemePortResolver.INSTANCE);
}
else {
routePlanner = new DefaultRoutePlanner(DefaultSchemePortResolver.INSTANCE);
}

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());
DropAuthorizationHeaderRequestRedirectStrategy redirectStrategy = new DropAuthorizationHeaderRequestRedirectStrategy(
extra);
HttpComponentsClientHttpRequestFactory customRequestFactory = new HttpComponentsClientHttpRequestFactory(
clientBuilder.setRedirectStrategy(redirectStrategy)
.replaceExecInterceptor(ChainElement.REDIRECT.name(),
new SpecialRedirectExec(routePlanner, redirectStrategy))
// Azure redirects may contain double slashes and on default those are
// normilised
.setDefaultRequestConfig(RequestConfig.custom().build())
.build());

// 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.
Expand Down
146 changes: 59 additions & 87 deletions ...flow/container/registry/authorization/DropAuthorizationHeaderRequestRedirectStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,10 @@

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

import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.classic.methods.HttpHead;
import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase;
import org.apache.hc.client5.http.impl.DefaultRedirectStrategy;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpException;
Expand Down Expand Up @@ -62,6 +60,7 @@
* @author Janne Valkealahti
* @author Christian Tzolov
* @author Cheng Guan Poh
* @author Corneil du Plessis
*/
public class DropAuthorizationHeaderRequestRedirectStrategy extends DefaultRedirectStrategy {

Expand Down Expand Up @@ -92,109 +91,82 @@ public URI getLocationURI(final HttpRequest request, final HttpResponse response
URI httpUriRequest = super.getLocationURI(request, response, context);
String query = httpUriRequest.getQuery();
String method = request.getMethod();

// Handle Amazon requests
if (StringUtils.hasText(query) && query.contains(AMZ_CREDENTIAL)) {
if (isHeadOrGetMethod(method)) {
try {
return new DropAuthorizationHeaderHttpRequestBase(httpUriRequest, method).getUri();
} catch (URISyntaxException e) {
throw new HttpException("Unable to get location URI", e);
}
}
removeAuthorizationHeader(request, response, false);
try {
if (isHeadMethod(method)) {
return new HttpHead(httpUriRequest).getUri();
}
else {
return new HttpGet(httpUriRequest).getUri();
}
}
catch (URISyntaxException e) {
throw new HttpException("Unable to get location URI", e);
}
}
}

// Handle Azure requests
try {
if (request.getUri().getRawPath().contains(AZURECR_URI_SUFFIX)) {
if (isHeadOrGetMethod(method)) {
return (new DropAuthorizationHeaderHttpRequestBase(httpUriRequest, method) {
// Drop headers only for the Basic Auth and leave unchanged for OAuth2
@Override
protected boolean isDropHeader(String name, Object value) {
return name.equalsIgnoreCase(AUTHORIZATION_HEADER) && StringUtils.hasText((String) value) && ((String)value).contains(BASIC_AUTH);
}
}).getUri();
}
}


// Handle Custom requests
if (extra.containsKey(CUSTOM_REGISTRY) && request.getUri().getRawPath().contains(extra.get(CUSTOM_REGISTRY))) {
if (isHeadOrGetMethod(method)) {
return new DropAuthorizationHeaderHttpRequestBase(httpUriRequest, method).getUri();
try {
if (request.getUri().getRawPath().contains(AZURECR_URI_SUFFIX)) {
if (isHeadOrGetMethod(method)) {
removeAuthorizationHeader(request, response, true);
if (isHeadMethod(method)) {
return new HttpHead(httpUriRequest).getUri();
}
else {
return new HttpGet(httpUriRequest).getUri();
}
}
}

// Handle Custom requests
if (extra.containsKey(CUSTOM_REGISTRY)
&& request.getUri().getRawPath().contains(extra.get(CUSTOM_REGISTRY))) {
if (isHeadOrGetMethod(method)) {
removeAuthorizationHeader(request, response, false);
if (isHeadMethod(method)) {
return new HttpHead(httpUriRequest).getUri();
}
else {
return new HttpGet(httpUriRequest).getUri();
}
}
}
}
} catch (URISyntaxException e) {
catch (URISyntaxException e) {
throw new HttpException("Unable to get Locaction URI", e);
}
return httpUriRequest;
}

private boolean isHeadOrGetMethod(String method) {
return StringUtils.hasText(method)
&& (method.equalsIgnoreCase(HttpHead.METHOD_NAME) || method.equalsIgnoreCase(HttpGet.METHOD_NAME));
}

/**
* Overrides all header setter methods to filter out the Authorization headers.
*/
static class DropAuthorizationHeaderHttpRequestBase extends HttpUriRequestBase {

private final String method;

DropAuthorizationHeaderHttpRequestBase(URI uri, String method) {
super(method, uri);
this.method = method;
}

@Override
public String getMethod() {
return this.method;
}

@Override
public void addHeader(Header header) {
if (!isDropHeader(header)) {
super.addHeader(header);
private static void removeAuthorizationHeader(HttpRequest request, HttpResponse response, boolean onlyBasicAuth) {
for (Header header : response.getHeaders()) {
if (header.getName().equalsIgnoreCase(AUTHORIZATION_HEADER)
&& (!onlyBasicAuth || (onlyBasicAuth && header.getValue().contains(BASIC_AUTH)))) {
response.removeHeaders(header.getName());
break;
}
}

@Override
public void addHeader(String name, Object value) {
if (!isDropHeader(name, value)) {
super.addHeader(name, value);
for (Header header : request.getHeaders()) {
if (header.getName().equalsIgnoreCase(AUTHORIZATION_HEADER)
&& (!onlyBasicAuth || (onlyBasicAuth && header.getValue().contains(BASIC_AUTH)))) {
request.removeHeaders(header.getName());
break;
}
}
}

@Override
public void setHeader(Header header) {
if (!isDropHeader(header)) {
super.setHeader(header);
}
}

@Override
public void setHeader(String name, Object value) {
if (!isDropHeader(name, value)) {
super.setHeader(name, value);
}
}

@Override
public void setHeaders(Header[] headers) {
Header[] filteredHeaders = Arrays.stream(headers)
.filter(header -> !isDropHeader(header))
.toArray(Header[]::new);
super.setHeaders(filteredHeaders);
}

protected boolean isDropHeader(Header header) {
return isDropHeader(header.getName(), header.getValue());
}
private boolean isHeadOrGetMethod(String method) {
return StringUtils.hasText(method)
&& (method.equalsIgnoreCase(HttpHead.METHOD_NAME) || method.equalsIgnoreCase(HttpGet.METHOD_NAME));
}

protected boolean isDropHeader(String name, Object value) {
return name.equalsIgnoreCase(AUTHORIZATION_HEADER);
}
private boolean isHeadMethod(String method) {
return StringUtils.hasText(method) && method.equalsIgnoreCase(HttpHead.METHOD_NAME);
}

}
Loading

0 comments on commit 9e93935

Please sign in to comment.