From 01bc3ab09ad877226ec37314596f29afd6627ba5 Mon Sep 17 00:00:00 2001 From: kosi2801 Date: Fri, 16 Feb 2024 17:05:38 +0100 Subject: [PATCH] fix: primitive support for multiple `WWW-Authenticate` response headers (GoogleContainerTools#4187) * fix: try all authenticate methods from the `WWW-Authenticate` response header until one succeeds --- .../jib/builder/steps/PullBaseImageStep.java | 37 +++++++++++++------ .../AuthenticationMethodRetriever.java | 31 ++++++++++------ .../tools/jib/registry/RegistryClient.java | 36 +++++++++++++++++- .../AuthenticationMethodRetrieverTest.java | 33 +++++++++++++++-- 4 files changed, 109 insertions(+), 28 deletions(-) diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java index 6ce49ba41a..9df893acde 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java @@ -171,14 +171,30 @@ public ImagesAndRegistryClient call() .setCredential(credential) .newRegistryClient(); - String wwwAuthenticate = ex.getHttpResponseException().getHeaders().getAuthenticate(); - if (wwwAuthenticate != null) { - eventHandlers.dispatch( - LogEvent.debug("WWW-Authenticate for " + imageReference + ": " + wwwAuthenticate)); - registryClient.authPullByWwwAuthenticate(wwwAuthenticate); - return new ImagesAndRegistryClient( - pullBaseImages(registryClient, progressDispatcher.newChildProducer()), - registryClient); + List wwwAuthenticateList = + ex.getHttpResponseException().getHeaders().getAuthenticateAsList(); + if (wwwAuthenticateList != null && !wwwAuthenticateList.isEmpty()) { + RegistryException storedEx = null; + // try all WWW-Authenticate headers until one succeeds + for (String wwwAuthenticate : wwwAuthenticateList) { + eventHandlers.dispatch( + LogEvent.debug("WWW-Authenticate for " + imageReference + ": " + wwwAuthenticate)); + try { + storedEx = null; + registryClient.authPullByWwwAuthenticate(wwwAuthenticate); + break; + } catch (RegistryException exc) { + eventHandlers.dispatch( + LogEvent.debug( + "WWW-Authenticate failed for " + imageReference + ": " + wwwAuthenticate)); + storedEx = exc; + } + } + // if none of the WWW-Authenticate headers worked for the authPullByWwwAuthenticate, throw + // the last stored exception + if (storedEx != null) { + throw storedEx; + } } else { // Not getting WWW-Authenticate is unexpected in practice, and we may just blame the @@ -200,10 +216,9 @@ public ImagesAndRegistryClient call() eventHandlers.dispatch( LogEvent.debug("Trying bearer auth as fallback for " + imageReference + "...")); registryClient.doPullBearerAuth(); - return new ImagesAndRegistryClient( - pullBaseImages(registryClient, progressDispatcher.newChildProducer()), - registryClient); } + return new ImagesAndRegistryClient( + pullBaseImages(registryClient, progressDispatcher.newChildProducer()), registryClient); } } } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetriever.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetriever.java index 20910e73c3..2dec7115c3 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetriever.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetriever.java @@ -93,22 +93,31 @@ public Optional handleHttpResponseException( } // Checks if the 'WWW-Authenticate' header is present. - String authenticationMethod = responseException.getHeaders().getAuthenticate(); - if (authenticationMethod == null) { + List authList = responseException.getHeaders().getAuthenticateAsList(); + if (authList == null || authList.isEmpty()) { throw new RegistryErrorExceptionBuilder(getActionDescription(), responseException) .addReason("'WWW-Authenticate' header not found") .build(); } - // Parses the header to retrieve the components. - try { - return RegistryAuthenticator.fromAuthenticationMethod( - authenticationMethod, registryEndpointRequestProperties, userAgent, httpClient); - - } catch (RegistryAuthenticationFailedException ex) { - throw new RegistryErrorExceptionBuilder(getActionDescription(), ex) - .addReason("Failed get authentication method from 'WWW-Authenticate' header") - .build(); + // try all 'WWW-Authenticate' headers until a working RegistryAuthenticator can be created + RegistryErrorException lastExc = null; + for (String authenticationMethod : authList) { + try { + return RegistryAuthenticator.fromAuthenticationMethod( + authenticationMethod, registryEndpointRequestProperties, userAgent, httpClient); + } catch (RegistryAuthenticationFailedException ex) { + if (lastExc == null) { + lastExc = + new RegistryErrorExceptionBuilder(getActionDescription(), ex) + .addReason( + "Failed getting supported authentication method from 'WWW-Authenticate' header") + .build(); + } + } } + + // if none of the RegistryAuthenticators worked, throw the last stored exception + throw lastExc; } } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java index a2d97e35bb..53611c2852 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java @@ -632,8 +632,40 @@ private T callRegistryEndpoint(RegistryEndpointProvider registryEndpointP // Because we successfully did bearer authentication initially, getting 401 here probably // means the token was expired. - String wwwAuthenticate = ex.getHttpResponseException().getHeaders().getAuthenticate(); - authorization.set(refreshBearerAuth(wwwAuthenticate)); + // There may be multiple WWW-Authenticate headers, so we iterate them until + // refreshBearerAuth() succeeds + List wwwAuthenticateList = + ex.getHttpResponseException().getHeaders().getAuthenticateAsList(); + Authorization auth = null; + if (wwwAuthenticateList != null && !wwwAuthenticateList.isEmpty()) { + Exception storedEx = null; + // try all WWW-Authenticate headers until one succeeds + for (String wwwAuthenticate : wwwAuthenticateList) { + try { + storedEx = null; + auth = refreshBearerAuth(wwwAuthenticate); + break; + } catch (Exception exc) { + storedEx = exc; + } + } + // if none of the headers worked, throw the last exception that occurred + if (storedEx != null) { + if (storedEx instanceof IllegalStateException) { + throw (IllegalStateException) storedEx; + } else if (storedEx instanceof RegistryException) { + throw (RegistryException) storedEx; + } else { + throw new IllegalStateException( + "unexpected exception during handling of WWW-Authenticate headers: " + storedEx); + } + } + } + // if no WWW-Authenticate header was provided, perform a refreshBearerAuth without it anyway + if (auth == null) { + auth = refreshBearerAuth(null); + } + authorization.set(auth); } } } diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetrieverTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetrieverTest.java index 44d3fd7c9f..980c09a5d1 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetrieverTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetrieverTest.java @@ -22,6 +22,7 @@ import com.google.cloud.tools.jib.http.FailoverHttpClient; import com.google.cloud.tools.jib.http.Response; import com.google.cloud.tools.jib.http.ResponseException; +import com.google.common.collect.Lists; import java.net.MalformedURLException; import java.net.URL; import java.util.Collections; @@ -102,7 +103,7 @@ public void testHandleHttpResponseException_noHeader() throws ResponseException Mockito.when(mockResponseException.getStatusCode()) .thenReturn(HttpStatusCodes.STATUS_CODE_UNAUTHORIZED); Mockito.when(mockResponseException.getHeaders()).thenReturn(mockHeaders); - Mockito.when(mockHeaders.getAuthenticate()).thenReturn(null); + Mockito.when(mockHeaders.getAuthenticateAsList()).thenReturn(null); try { testAuthenticationMethodRetriever.handleHttpResponseException(mockResponseException); @@ -122,7 +123,8 @@ public void testHandleHttpResponseException_badAuthenticationMethod() throws Res Mockito.when(mockResponseException.getStatusCode()) .thenReturn(HttpStatusCodes.STATUS_CODE_UNAUTHORIZED); Mockito.when(mockResponseException.getHeaders()).thenReturn(mockHeaders); - Mockito.when(mockHeaders.getAuthenticate()).thenReturn(authenticationMethod); + Mockito.when(mockHeaders.getAuthenticateAsList()) + .thenReturn(Lists.newArrayList(authenticationMethod)); try { testAuthenticationMethodRetriever.handleHttpResponseException(mockResponseException); @@ -133,7 +135,7 @@ public void testHandleHttpResponseException_badAuthenticationMethod() throws Res MatcherAssert.assertThat( ex.getMessage(), CoreMatchers.containsString( - "Failed get authentication method from 'WWW-Authenticate' header")); + "Failed getting supported authentication method from 'WWW-Authenticate' header")); } } @@ -146,7 +148,30 @@ public void testHandleHttpResponseException_pass() Mockito.when(mockResponseException.getStatusCode()) .thenReturn(HttpStatusCodes.STATUS_CODE_UNAUTHORIZED); Mockito.when(mockResponseException.getHeaders()).thenReturn(mockHeaders); - Mockito.when(mockHeaders.getAuthenticate()).thenReturn(authenticationMethod); + Mockito.when(mockHeaders.getAuthenticateAsList()) + .thenReturn(Lists.newArrayList(authenticationMethod)); + + RegistryAuthenticator registryAuthenticator = + testAuthenticationMethodRetriever.handleHttpResponseException(mockResponseException).get(); + + Assert.assertEquals( + new URL("https://somerealm?service=someservice&scope=repository:someImageName:someScope"), + registryAuthenticator.getAuthenticationUrl( + null, Collections.singletonMap("someImageName", "someScope"))); + } + + @Test + public void testHandleHttpResponseExceptionWithKerberosFirst_pass() + throws RegistryErrorException, ResponseException, MalformedURLException { + String authenticationMethodNegotiate = "Negotiate"; + String authenticationMethodBearer = + "Bearer realm=\"https://somerealm\",service=\"someservice\",scope=\"somescope\""; + + Mockito.when(mockResponseException.getStatusCode()) + .thenReturn(HttpStatusCodes.STATUS_CODE_UNAUTHORIZED); + Mockito.when(mockResponseException.getHeaders()).thenReturn(mockHeaders); + Mockito.when(mockHeaders.getAuthenticateAsList()) + .thenReturn(Lists.newArrayList(authenticationMethodNegotiate, authenticationMethodBearer)); RegistryAuthenticator registryAuthenticator = testAuthenticationMethodRetriever.handleHttpResponseException(mockResponseException).get();