fix(provider/docker): Support multiple challenges from `WWW-Authentic…

…ate` header (#6139) (#6175) * test(provider/docker): Add test for `WWW-Authenticate` header parsing * fix(provider/docker): Support multiple challenges from `WWW-Authenticate` header (cherry picked from commit 2aeea5a) Co-authored-by: Pieter Dirk Soels <p.d.soels@hotmail.com>
spinnaker · Mar 27, 2024 · 54856aa · 54856aa
1 parent 8f41451
commit 54856aa
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 14 deletions.
diff --git a/...m/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryClient.groovy b/...m/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryClient.groovy
@@ -497,35 +497,37 @@ 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<String> authenticateHeader = null
 
           error.headers.entrySet().forEach { header ->
             if (header.key.equalsIgnoreCase("www-authenticate")) {
               authenticateHeader = header.value
             }
           }
 
-          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
           }
 
           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/...tflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryClientSpec.groovy b/...tflix/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
+  }
 }