Skip to content

Commit

Permalink
Extract createResponseURI
Browse files Browse the repository at this point in the history 
Remove comments from test controller.
Reverted ContainerRegistryService
Add SpecialRedirectExec to replace RedirectExec.
Ensure DropAuthorizationHeaderRequestRedirectStrategy changes request headers used to create the redirect request.

Fixes #5989
  • Loading branch information
@corneil
corneil committed Oct 17, 2024
1 parent 9e93935 commit 8b90386
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 98 deletions.
5 changes: 0 additions & 5 deletions .../dataflow/container/registry/authorization/support/S3SignedRedirectRequestController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
*/
@RestController
public class S3SignedRedirectRequestController {
private final static Logger logger = LoggerFactory.getLogger(S3SignedRedirectRequestController.class);

@RequestMapping("/service/token")
public ResponseEntity<Map<String, String>> getToken() {
Expand All @@ -57,7 +56,6 @@ 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 @@ -68,15 +66,12 @@ 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) {
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
12 changes: 5 additions & 7 deletions .../springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -216,14 +217,11 @@ private RestTemplate initRestTemplate(HttpClientBuilder clientBuilder, boolean w
routePlanner = new DefaultRoutePlanner(DefaultSchemePortResolver.INSTANCE);
}

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

Expand All @@ -232,7 +230,7 @@ private RestTemplate initRestTemplate(HttpClientBuilder clientBuilder, boolean w
// Therefore we extend the MappingJackson2HttpMessageConverter media-types to
// include application/octet-stream and text/plain.
MappingJackson2HttpMessageConverter octetSupportJsonConverter = new MappingJackson2HttpMessageConverter();
ArrayList<MediaType> mediaTypeList = new ArrayList(octetSupportJsonConverter.getSupportedMediaTypes());
List<MediaType> mediaTypeList = new ArrayList(octetSupportJsonConverter.getSupportedMediaTypes());
mediaTypeList.add(MediaType.APPLICATION_OCTET_STREAM);
mediaTypeList.add(MediaType.TEXT_PLAIN);
octetSupportJsonConverter.setSupportedMediaTypes(mediaTypeList);
Expand Down
95 changes: 42 additions & 53 deletions .../java/org/springframework/cloud/dataflow/container/registry/ContainerRegistryService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import org.springframework.http.HttpMethod;
import org.springframework.http.ResponseEntity;
import org.springframework.util.StringUtils;
import org.springframework.web.client.RestClientException;
import org.springframework.web.client.RestTemplate;
import org.springframework.web.util.UriComponents;
import org.springframework.web.util.UriComponentsBuilder;
Expand All @@ -47,9 +46,9 @@ public class ContainerRegistryService {

private static final Logger logger = LoggerFactory.getLogger(ContainerRegistryService.class);

private static final List<String> SUPPORTED_MANIFEST_MEDIA_TYPES = Collections
.unmodifiableList(Arrays.asList(ContainerRegistryProperties.OCI_IMAGE_MANIFEST_MEDIA_TYPE,
ContainerRegistryProperties.DOCKER_IMAGE_MANIFEST_MEDIA_TYPE));
private static final List<String> SUPPORTED_MANIFEST_MEDIA_TYPES =
Collections.unmodifiableList(Arrays.asList(ContainerRegistryProperties.OCI_IMAGE_MANIFEST_MEDIA_TYPE,
ContainerRegistryProperties.DOCKER_IMAGE_MANIFEST_MEDIA_TYPE));

private static final String HTTPS_SCHEME = "https";

Expand Down Expand Up @@ -89,38 +88,35 @@ public Map<String, ContainerRegistryConfiguration> getContainerRegistryConfigura
}

/**
* Get the tag information for the given container image identified by its repository
* and registry. The registry information is expected to be set via container registry
* configuration in SCDF.
* Get the tag information for the given container image identified by its repository and registry.
* The registry information is expected to be set via container registry configuration in SCDF.
* @param registryName the container registry name
* @param repositoryName the image repository name
* @return the list of tags for the image
*/
public List<String> getTags(String registryName, String repositoryName) {
try {
ContainerRegistryConfiguration containerRegistryConfiguration = this.registryConfigurations
.get(registryName);
ContainerRegistryConfiguration containerRegistryConfiguration = this.registryConfigurations.get(registryName);
Map<String, String> properties = new HashMap<>();
properties.put(DockerOAuth2RegistryAuthorizer.DOCKER_REGISTRY_REPOSITORY_FIELD_KEY, repositoryName);
HttpHeaders httpHeaders = new HttpHeaders(
this.registryAuthorizerMap.get(containerRegistryConfiguration.getAuthorizationType())
.getAuthorizationHeaders(containerRegistryConfiguration, properties));
HttpHeaders httpHeaders = new HttpHeaders(this.registryAuthorizerMap.get(containerRegistryConfiguration.getAuthorizationType()).getAuthorizationHeaders(
containerRegistryConfiguration, properties));
httpHeaders.set(HttpHeaders.ACCEPT, "application/json");

UriComponents manifestUriComponents = UriComponentsBuilder.newInstance()
.scheme(HTTPS_SCHEME)
.host(containerRegistryConfiguration.getRegistryHost())
.path(TAGS_LIST_PATH)
.build()
.expand(repositoryName);
.scheme(HTTPS_SCHEME)
.host(containerRegistryConfiguration.getRegistryHost())
.path(TAGS_LIST_PATH)
.build().expand(repositoryName);

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

ResponseEntity<Map> manifest = requestRestTemplate.exchange(manifestUriComponents.toUri(), HttpMethod.GET,
new HttpEntity<>(httpHeaders), Map.class);
return (List<String>) manifest.getBody().get(TAGS_FIELD);
ResponseEntity<Map> manifest = requestRestTemplate.exchange(manifestUriComponents.toUri(),
HttpMethod.GET, new HttpEntity<>(httpHeaders), Map.class);
return (List<String>) manifest.getBody().get(TAGS_FIELD);
}
catch (Exception e) {
logger.error("Exception getting tag information for the {} from {}", repositoryName, registryName);
Expand All @@ -136,25 +132,28 @@ public List<String> getTags(String registryName, String repositoryName) {
public Map getRepositories(String registryName) {
try {
ContainerRegistryConfiguration containerRegistryConfiguration = this.registryConfigurations
.get(registryName);
.get(registryName);
Map<String, String> properties = new HashMap<>();
properties.put(DockerOAuth2RegistryAuthorizer.DOCKER_REGISTRY_REPOSITORY_FIELD_KEY, registryName);
HttpHeaders httpHeaders = new HttpHeaders(
this.registryAuthorizerMap.get(containerRegistryConfiguration.getAuthorizationType())
.getAuthorizationHeaders(containerRegistryConfiguration, properties));
.getAuthorizationHeaders(
containerRegistryConfiguration, properties));
httpHeaders.set(HttpHeaders.ACCEPT, "application/json");
UriComponents manifestUriComponents = UriComponentsBuilder.newInstance()
.scheme(HTTPS_SCHEME)
.host(containerRegistryConfiguration.getRegistryHost())
.path(CATALOG_LIST_PATH)
.build();
.scheme(HTTPS_SCHEME)
.host(containerRegistryConfiguration.getRegistryHost())
.path(CATALOG_LIST_PATH)
.build();


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

ResponseEntity<Map> manifest = requestRestTemplate.exchange(manifestUriComponents.toUri(), HttpMethod.GET,
new HttpEntity<>(httpHeaders), Map.class);
ResponseEntity<Map> manifest = requestRestTemplate.exchange(manifestUriComponents.toUri(),
HttpMethod.GET, new HttpEntity<>(httpHeaders), Map.class);
return manifest.getBody();
}
catch (Exception e) {
Expand Down Expand Up @@ -207,20 +206,18 @@ public <T> T getImageManifest(ContainerRegistryRequest registryRequest, Class<T>
// Docker Registry HTTP V2 API pull manifest
ContainerImage containerImage = registryRequest.getContainerImage();
UriComponents manifestUriComponents = UriComponentsBuilder.newInstance()
.scheme(HTTPS_SCHEME)
.host(containerImage.getHostname())
.port(StringUtils.hasText(containerImage.getPort()) ? containerImage.getPort() : null)
.path(IMAGE_MANIFEST_REFERENCE_PATH)
.build()
.expand(containerImage.getRepository(), containerImage.getRepositoryReference());
.scheme(HTTPS_SCHEME)
.host(containerImage.getHostname())
.port(StringUtils.hasText(containerImage.getPort()) ? containerImage.getPort() : null)
.path(IMAGE_MANIFEST_REFERENCE_PATH)
.build().expand(containerImage.getRepository(), containerImage.getRepositoryReference());

ResponseEntity<T> manifest = registryRequest.getRestTemplate()
.exchange(manifestUriComponents.toUri(), HttpMethod.GET, new HttpEntity<>(httpHeaders), responseClassType);
ResponseEntity<T> manifest = registryRequest.getRestTemplate().exchange(manifestUriComponents.toUri(),
HttpMethod.GET, new HttpEntity<>(httpHeaders), responseClassType);
return manifest.getBody();
}

public <T> T getImageBlob(ContainerRegistryRequest registryRequest, String configDigest,
Class<T> responseClassType) {
public <T> T getImageBlob(ContainerRegistryRequest registryRequest, String configDigest, Class<T> responseClassType) {
ContainerImage containerImage = registryRequest.getContainerImage();
HttpHeaders httpHeaders = new HttpHeaders(registryRequest.getAuthHttpHeaders());

Expand All @@ -230,19 +227,11 @@ public <T> T getImageBlob(ContainerRegistryRequest registryRequest, String confi
.host(containerImage.getHostname())
.port(StringUtils.hasText(containerImage.getPort()) ? containerImage.getPort() : null)
.path(IMAGE_BLOB_DIGEST_PATH)
.build()
.expand(containerImage.getRepository(), configDigest);
try {
logger.info("getImageBlob:request:{},{}", blobUriComponents.toUri(), httpHeaders);
ResponseEntity<T> blob = registryRequest.getRestTemplate()
.exchange(blobUriComponents.toUri(), HttpMethod.GET, new HttpEntity<>(httpHeaders), responseClassType);
.build().expand(containerImage.getRepository(), configDigest);

return blob.getStatusCode().is2xxSuccessful() ? blob.getBody() : null;
}
catch (RestClientException x) {
logger.error("getImageBlob:exception:" + x, x);
return null;
}
}
ResponseEntity<T> blob = registryRequest.getRestTemplate().exchange(blobUriComponents.toUri(),
HttpMethod.GET, new HttpEntity<>(httpHeaders), responseClassType);

return blob.getBody();
}
}
53 changes: 20 additions & 33 deletions ...flow/container/registry/authorization/DropAuthorizationHeaderRequestRedirectStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,63 +94,50 @@ public URI getLocationURI(final HttpRequest request, final HttpResponse response
// Handle Amazon requests
if (StringUtils.hasText(query) && query.contains(AMZ_CREDENTIAL)) {
if (isHeadOrGetMethod(method)) {
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);
}
removeAuthorizationHeader(request, false);
return createResponseURI(method, httpUriRequest);
}
}

// Handle Azure requests
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();
}
removeAuthorizationHeader(request, true);
return createResponseURI(method, httpUriRequest);
}
}

// 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();
}
removeAuthorizationHeader(request, false);
return createResponseURI(method, httpUriRequest);
}
}
}
catch (URISyntaxException e) {
throw new HttpException("Unable to get Locaction URI", e);
throw new HttpException("Unable to get Location URI", e);
}
return httpUriRequest;
}

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;
private URI createResponseURI(String method, URI httpUriRequest) throws HttpException {
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);
}
}

private static void removeAuthorizationHeader(HttpRequest request, boolean onlyBasicAuth) {
for (Header header : request.getHeaders()) {
if (header.getName().equalsIgnoreCase(AUTHORIZATION_HEADER)
&& (!onlyBasicAuth || (onlyBasicAuth && header.getValue().contains(BASIC_AUTH)))) {
Expand Down

0 comments on commit 8b90386

Please sign in to comment.