Optimize authentication for base image

jib-core/CHANGELOG.md

@@ -20,6 +20,8 @@ All notable changes to this project will be documented in this file.
 - Fixed `NullPointerException` to return a helpful message when a server does not provide any message in certain error cases (400 Bad Request, 404 Not Found, and 405 Method Not Allowed). ([#2532](https://github.com/GoogleContainerTools/jib/issues/2532))
 - Now supports sending client certificate (for example, via the `javax.net.ssl.keyStore` and `javax.net.ssl.keyStorePassword` system properties) and thus enabling mutual TLS authentication. ([#2585](https://github.com/GoogleContainerTools/jib/issues/2585), [#2226](https://github.com/GoogleContainerTools/jib/issues/2226))
 - Fixed `NullPointerException` during input validation (in Java 9+) when configuring Jib parameters using certain immutable collections (such as `List.of()`). ([#2702](https://github.com/GoogleContainerTools/jib/issues/2702))
+- Fixed authentication failure with Azure Container Registry when using ["tokens"](https://docs.microsoft.com/en-us/azure/container-registry/container-registry-repository-scoped-permissions). ([#2784](https://github.com/GoogleContainerTools/jib/issues/2784))
+- Improved authentication flow for base image registry. ([#2134](https://github.com/GoogleContainerTools/jib/issues/2134))
 
 ## 0.15.0
 

jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java

@@ -137,7 +137,10 @@ public ImagesAndRegistryClient call()
             progressEventDispatcherFactory.create("pulling base image manifest", 2);
         TimerEventDispatcher ignored1 = new TimerEventDispatcher(eventHandlers, DESCRIPTION)) {
 
-      // First, try with no credentials.
+      // First, try with no credentials. This works with public GCR images (but not Docker Hub).
+      // TODO: investigate if we should just pass credentials up front. However, this involves
+      // some risk. https://github.com/GoogleContainerTools/jib/pull/2200#discussion_r359069026
+      // contains some related discussions.
       RegistryClient noAuthRegistryClient =
           buildContext.newBaseImageRegistryClientFactory().newRegistryClient();
       try {
@@ -149,40 +152,43 @@ public ImagesAndRegistryClient call()
             LogEvent.lifecycle(
                 "The base image requires auth. Trying again for " + imageReference + "..."));
 
-        Credential registryCredential =
+        Credential credential =
             RegistryCredentialRetriever.getBaseImageCredential(buildContext).orElse(null);
-
         RegistryClient registryClient =
             buildContext
                 .newBaseImageRegistryClientFactory()
-                .setCredential(registryCredential)
+                .setCredential(credential)
                 .newRegistryClient();
 
-        try {
-          // TODO: refactor the code (https://github.com/GoogleContainerTools/jib/pull/2202)
-          if (registryCredential == null || registryCredential.isOAuth2RefreshToken()) {
-            throw ex;
-          }
-
-          eventHandlers.dispatch(LogEvent.debug("Trying basic auth for " + imageReference + "..."));
-          registryClient.configureBasicAuth();
+        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, progressEventDispatcher), registryClient);
 
-        } catch (RegistryUnauthorizedException registryUnauthorizedException) {
-          // The registry requires us to authenticate using the Docker Token Authentication.
-          // See https://docs.docker.com/registry/spec/auth/token
-          eventHandlers.dispatch(
-              LogEvent.debug("Trying bearer auth for " + imageReference + "..."));
-          if (registryClient.doPullBearerAuth()) {
-            return new ImagesAndRegistryClient(
-                pullBaseImages(registryClient, progressEventDispatcher), registryClient);
+        } else {
+          // Not getting WWW-Authenticate is unexpected in practice, and we may just blame the
+          // server and fail. However, to keep some old behavior, try a few things as a last resort.
+          // TODO: consider removing this fallback branch.
+          if (credential != null && !credential.isOAuth2RefreshToken()) {
+            eventHandlers.dispatch(
+                LogEvent.debug("Trying basic auth as fallback for " + imageReference + "..."));
+            registryClient.configureBasicAuth();
+            try {
+              return new ImagesAndRegistryClient(
+                  pullBaseImages(registryClient, progressEventDispatcher), registryClient);
+            } catch (RegistryUnauthorizedException ignored) {
+              // Fall back to try bearer auth.
+            }
           }
+
           eventHandlers.dispatch(
-              LogEvent.error(
-                  "The registry asked for basic authentication, but the registry had refused basic "
-                      + "authentication previously"));
-          throw registryUnauthorizedException;
+              LogEvent.debug("Trying bearer auth as fallback for " + imageReference + "..."));
+          registryClient.doPullBearerAuth();
+          return new ImagesAndRegistryClient(
+              pullBaseImages(registryClient, progressEventDispatcher), registryClient);
         }
       }
     }

jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java

@@ -325,15 +325,26 @@ private boolean doBearerAuth(boolean readOnlyBearerAuth) throws IOException, Reg
       return false; // server returned "WWW-Authenticate: Basic ..."
     }
 
-    initialBearerAuthenticator.set(authenticator.get());
+    doBearerAuth(readOnlyBearerAuth, authenticator.get());
+    return true;
+  }
+
+  private void doBearerAuth(boolean readOnlyBearerAuth, RegistryAuthenticator authenticator)
+      throws RegistryException {
+    initialBearerAuthenticator.set(authenticator);
     if (readOnlyBearerAuth) {
-      authorization.set(authenticator.get().authenticatePull(credential));
+      authorization.set(authenticator.authenticatePull(credential));
     } else {
-      authorization.set(authenticator.get().authenticatePush(credential));
+      authorization.set(authenticator.authenticatePush(credential));
     }
     this.readOnlyBearerAuth = readOnlyBearerAuth;
-    eventHandlers.dispatch(LogEvent.debug("bearer auth succeeded for " + image));
-    return true;
+
+    eventHandlers.dispatch(
+        LogEvent.debug(
+            "bearer auth succeeded for "
+                + registryEndpointRequestProperties.getServerUrl()
+                + "/"
+                + registryEndpointRequestProperties.getImageName()));
   }
 
   private Authorization refreshBearerAuth(@Nullable String wwwAuthenticate)
@@ -366,6 +377,27 @@ private Authorization refreshBearerAuth(@Nullable String wwwAuthenticate)
     return Verify.verifyNotNull(initialBearerAuthenticator.get()).authenticatePush(credential);
   }
 
+  /**
+   * Configure basic authentication or attempts bearer authentication for pulling based on the
+   * specified authentication method in a server response.
+   *
+   * @param wwwAuthenticate {@code WWW-Authenticate} HTTP header value from a server response
+   *     specifying a required authentication method
+   * @throws RegistryException if communicating with the endpoint fails
+   * @throws RegistryAuthenticationFailedException if authentication fails
+   * @throws RegistryCredentialsNotSentException if authentication failed and credentials were not
+   */
+  public void authPullByWwwAuthenticate(String wwwAuthenticate) throws RegistryException {
+    Optional<RegistryAuthenticator> authenticator =
+        RegistryAuthenticator.fromAuthenticationMethod(
+            wwwAuthenticate, registryEndpointRequestProperties, getUserAgent(), httpClient);
+    if (authenticator.isPresent()) {
+      doBearerAuth(true, authenticator.get());
+    } else if (credential != null && !credential.isOAuth2RefreshToken()) {
+      configureBasicAuth();
+    }
+  }
+
   /**
    * Check if a manifest referred to by {@code imageQualifier} (tag or digest) exists on the
    * registry.

jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryClientTest.java

@@ -214,6 +214,93 @@ public void testConfigureBasicAuth()
         registry.getInputRead(), CoreMatchers.containsString("Authorization: Basic dXNlcjpwYXNz"));
   }
 
+  @Test
+  public void testAuthPullByWwwAuthenticate_bearerAuth()
+      throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException,
+          RegistryException {
+    String tokenResponse = "HTTP/1.1 200 OK\nContent-Length: 26\n\n{\"token\":\"awesome-token!\"}";
+    authServer = new TestWebServer(false, Arrays.asList(tokenResponse), 1);
+
+    String blobResponse = "HTTP/1.1 200 OK\nContent-Length: 5678\n\n";
+    registry = new TestWebServer(false, Arrays.asList(blobResponse), 1);
+
+    RegistryClient registryClient = createRegistryClient(Credential.from("user", "pass"));
+    registryClient.authPullByWwwAuthenticate("Bearer realm=\"" + authServer.getEndpoint() + "\"");
+
+    Optional<BlobDescriptor> digestAndSize = registryClient.checkBlob(digest);
+    Assert.assertEquals(5678, digestAndSize.get().getSize());
+
+    Mockito.verify(eventHandlers).dispatch(logContains("bearer auth succeeded"));
+  }
+
+  @Test
+  public void testAuthPullByWwwAuthenticate_basicAuth()
+      throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException,
+          RegistryException {
+    String blobResponse = "HTTP/1.1 200 OK\nContent-Length: 5678\n\n";
+    registry = new TestWebServer(false, Arrays.asList(blobResponse), 1);
+
+    RegistryClient registryClient = createRegistryClient(Credential.from("user", "pass"));
+    registryClient.authPullByWwwAuthenticate("Basic foo");
+
+    Optional<BlobDescriptor> digestAndSize = registryClient.checkBlob(digest);
+    Assert.assertEquals(5678, digestAndSize.get().getSize());
+
+    MatcherAssert.assertThat(
+        registry.getInputRead(), CoreMatchers.containsString("Authorization: Basic dXNlcjpwYXNz"));
+  }
+
+  @Test
+  public void testAuthPullByWwwAuthenticate_basicAuthRequestedButNullCredential()
+      throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException,
+          RegistryException {
+    String blobResponse = "HTTP/1.1 200 OK\nContent-Length: 5678\n\n";
+    registry = new TestWebServer(false, Arrays.asList(blobResponse), 1);
+
+    RegistryClient registryClient = createRegistryClient(null);
+    registryClient.authPullByWwwAuthenticate("Basic foo");
+
+    Optional<BlobDescriptor> digestAndSize = registryClient.checkBlob(digest);
+    Assert.assertEquals(5678, digestAndSize.get().getSize());
+
+    MatcherAssert.assertThat(
+        registry.getInputRead(), CoreMatchers.not(CoreMatchers.containsString("Authorization:")));
+  }
+
+  @Test
+  public void testAuthPullByWwwAuthenticate_basicAuthRequestedButOAuth2Credential()
+      throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException,
+          RegistryException {
+    String blobResponse = "HTTP/1.1 200 OK\nContent-Length: 5678\n\n";
+    registry = new TestWebServer(false, Arrays.asList(blobResponse), 1);
+
+    Credential credential = Credential.from(Credential.OAUTH2_TOKEN_USER_NAME, "pass");
+    Assert.assertTrue(credential.isOAuth2RefreshToken());
+    RegistryClient registryClient = createRegistryClient(credential);
+    registryClient.authPullByWwwAuthenticate("Basic foo");
+
+    Optional<BlobDescriptor> digestAndSize = registryClient.checkBlob(digest);
+    Assert.assertEquals(5678, digestAndSize.get().getSize());
+
+    MatcherAssert.assertThat(
+        registry.getInputRead(), CoreMatchers.not(CoreMatchers.containsString("Authorization:")));
+  }
+
+  @Test
+  public void testAuthPullByWwwAuthenticate_invalidAuthMethod() {
+    RegistryClient registryClient =
+        RegistryClient.factory(eventHandlers, "server", "foo/bar", null).newRegistryClient();
+    try {
+      registryClient.authPullByWwwAuthenticate("invalid WWW-Authenticate");
+      Assert.fail();
+    } catch (RegistryException ex) {
+      Assert.assertEquals(
+          "Failed to authenticate with registry server/foo/bar because: 'Bearer' was not found in "
+              + "the 'WWW-Authenticate' header, tried to parse: invalid WWW-Authenticate",
+          ex.getMessage());
+    }
+  }
+
   @Test
   public void testPullManifest()
       throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException,

jib-gradle-plugin/CHANGELOG.md

@@ -15,6 +15,8 @@ All notable changes to this project will be documented in this file.
 
 - Fixed `NullPointerException` during input validation (in Java 9+) when configuring Jib parameters using certain immutable collections (such as `List.of()`). ([#2702](https://github.com/GoogleContainerTools/jib/issues/2702))
 - Fixed an issue that configuring `jib.from.platforms` was always additive to the default `amd64/linux` platform. ([#2783](https://github.com/GoogleContainerTools/jib/issues/2783))
+- Fixed authentication failure with Azure Container Registry when using ["tokens"](https://docs.microsoft.com/en-us/azure/container-registry/container-registry-repository-scoped-permissions). ([#2784](https://github.com/GoogleContainerTools/jib/issues/2784))
+- Improved authentication flow for base image registry. ([#2134](https://github.com/GoogleContainerTools/jib/issues/2134))
 - Throw `IllegalArgumentException` with an error message instead of throwing a `NullPointerException` when `jib.to.tags` is set to a collection containing a `null` value. ([#2760](https://github.com/GoogleContainerTools/jib/issues/2760))
 
 ## 2.5.0

jib-maven-plugin/CHANGELOG.md

@@ -11,6 +11,9 @@ All notable changes to this project will be documented in this file.
 
 ### Fixed
 
+- Fixed authentication failure with Azure Container Registry when using ["tokens"](https://docs.microsoft.com/en-us/azure/container-registry/container-registry-repository-scoped-permissions). ([#2784](https://github.com/GoogleContainerTools/jib/issues/2784))
+- Improved authentication flow for base image registry. ([#2134](https://github.com/GoogleContainerTools/jib/issues/2134))
+
 ## 2.5.2
 
 ### Fixed