From 9084ef3d1d2cd879a7cf3afd7e9feb5c7d69f095 Mon Sep 17 00:00:00 2001 From: Pieter Dirk Soels Date: Tue, 23 Jan 2024 17:49:59 +0100 Subject: [PATCH] fix(provider/docker): Support multiple challenges from `WWW-Authenticate` header (#6139) * test(provider/docker): Add test for `WWW-Authenticate` header parsing * fix(provider/docker): Support multiple challenges from `WWW-Authenticate` header (cherry picked from commit 2aeea5a1ee9018f74035d1d84a457b6622c8a1f6) --- .../api/v2/client/DockerRegistryClient.groovy | 30 ++++++++++--------- .../v2/client/DockerRegistryClientSpec.groovy | 22 ++++++++++++++ 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryClient.groovy b/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryClient.groovy index d13a91b17a7..d8200dfcfab 100644 --- a/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryClient.groovy +++ b/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryClient.groovy @@ -497,7 +497,7 @@ class DockerRegistryClient { // note, this is a workaround for registries that should be returning // 401 when a token expires if ([400, 401].contains(status)) { - String authenticateHeader = null + List authenticateHeader = null error.headers.entrySet().forEach { header -> if (header.key.equalsIgnoreCase("www-authenticate")) { @@ -505,7 +505,7 @@ class DockerRegistryClient { } } - if (!authenticateHeader) { + if (!authenticateHeader || authenticateHeader.isEmpty()) { log.warn "Registry $address returned status $status for request '$target' without a WWW-Authenticate header" tokenService.clearToken(target) throw error @@ -513,19 +513,21 @@ class DockerRegistryClient { String bearerPrefix = "bearer " String basicPrefix = "basic " - if (bearerPrefix.equalsIgnoreCase(authenticateHeader.substring(0, bearerPrefix.length()))) { - // If we got a 401 and the request requires bearer auth, get a new token and try again - dockerToken = tokenService.getToken(target, authenticateHeader.substring(bearerPrefix.length())) - token = "Bearer ${(dockerToken.bearer_token ?: dockerToken.token) ?: dockerToken.access_token}" - response = withToken(token) - } else if (basicPrefix.equalsIgnoreCase(authenticateHeader.substring(0, basicPrefix.length()))) { - // If we got a 401 and the request requires basic auth, there's no point in trying again - tokenService.clearToken(target) - throw error - } else { - tokenService.clearToken(target) - throw new DockerRegistryAuthenticationException("Docker registry must support 'Bearer' or 'Basic' authentication.") + for (String headerValue in authenticateHeader) { + if (bearerPrefix.equalsIgnoreCase(headerValue.substring(0, bearerPrefix.length()))) { + // If we got a 401 and the request requires bearer auth, get a new token and try again + dockerToken = tokenService.getToken(target, headerValue.substring(bearerPrefix.length())) + token = "Bearer ${(dockerToken.bearer_token ?: dockerToken.token) ?: dockerToken.access_token}" + return withToken(token) + } else if (basicPrefix.equalsIgnoreCase(headerValue.substring(0, basicPrefix.length()))) { + // If we got a 401 and the request requires basic auth, there's no point in trying again + tokenService.clearToken(target) + throw error + } } + + tokenService.clearToken(target) + throw new DockerRegistryAuthenticationException("Docker registry must support 'Bearer' or 'Basic' authentication.") } else { throw error } diff --git a/clouddriver-docker/src/test/groovy/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryClientSpec.groovy b/clouddriver-docker/src/test/groovy/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryClientSpec.groovy index 96570b5dc90..31b5e31fee7 100644 --- a/clouddriver-docker/src/test/groovy/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryClientSpec.groovy +++ b/clouddriver-docker/src/test/groovy/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryClientSpec.groovy @@ -16,7 +16,12 @@ package com.netflix.spinnaker.clouddriver.docker.registry.api.v2.client +import com.netflix.spinnaker.clouddriver.docker.registry.api.v2.auth.DockerBearerToken import com.netflix.spinnaker.clouddriver.docker.registry.api.v2.auth.DockerBearerTokenService +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException +import org.springframework.http.HttpStatus +import retrofit.RetrofitError +import retrofit.client.Header import retrofit.client.Response import retrofit.mime.TypedByteArray import retrofit.mime.TypedInput @@ -182,4 +187,21 @@ class DockerRegistryClientSpec extends Specification { results?.config?.Labels != null results?.config?.Labels?.commitId == "b48e2cf960de545597411c99ec969e47a7635ba3" } + + void "DockerRegistryClient should honor the www-authenticate header"() { + setup: + def authenticateDetails = "realm=\"https://auth.docker.io/token\",service=\"registry.docker.io\",scope=\"repository:${REPOSITORY1}:pull\"" + def unauthorizedRetroFitError = RetrofitError.httpError("url", + new Response("url", HttpStatus.UNAUTHORIZED.value(), "authentication required", [new Header("www-authenticate", "Bearer ${authenticateDetails}")], null), + null, null) + DockerBearerToken token = new DockerBearerToken() + token.bearer_token = "bearer-token" + + when: + client = new DockerRegistryClient("https://index.docker.io", 100, "", "", stubbedRegistryService, dockerBearerTokenService) + client.request(() -> {throw new SpinnakerHttpException(unauthorizedRetroFitError)}, (_) -> null, REPOSITORY1) + + then: + 1 * dockerBearerTokenService.getToken(REPOSITORY1, authenticateDetails) >> token + } }