Global scopes and look for token with broader scope in the cache (#633)

jonesbusy · web-flow · commit 79f879c5e184 · 2026-03-08T15:09:00.000+01:00
Signed-off-by: Valentin Delaye &lt;jonesbusy@users.noreply.github.com&gt;
diff --git a/src/main/java/land/oras/auth/HttpClient.java b/src/main/java/land/oras/auth/HttpClient.java
@@ -486,11 +486,12 @@ private <T> ResponseWrapper<T> executeRequest(
 
             // Get scope based on method
             ContainerRef containerRef = scopes.getContainerRef();
+            LOG.debug("Scopes are adding registry scopes");
             Scopes newScopes =
                     switch (method) {
-                        case "GET", "HEAD" -> scopes.withNewRegistryScopes(Scope.PULL);
-                        case "POST", "PUT", "PATCH" -> scopes.withNewRegistryScopes(Scope.PUSH);
-                        case "DELETE" -> scopes.withNewRegistryScopes(Scope.DELETE);
+                        case "GET", "HEAD" -> scopes.withAddedRegistryScopes(Scope.PULL);
+                        case "POST", "PUT", "PATCH" -> scopes.withAddedRegistryScopes(Scope.PUSH);
+                        case "DELETE" -> scopes.withAddedRegistryScopes(Scope.DELETE);
                         default -> throw new OrasException("Unsupported HTTP method: " + method);
                     };
 
@@ -500,9 +501,9 @@ private <T> ResponseWrapper<T> executeRequest(
             // Check if token is present and reuse auth instead of passing auth provider
             TokenResponse cachedToken = TokenCache.get(newScopes);
             if (cachedToken == null) {
-                LOG.trace("No cached token found for scopes: {}", newScopes);
+                LOG.trace("No token found in cache for scopes: {}", newScopes);
             } else {
-                LOG.trace("Found cached token for scopes: {}", newScopes.withService(cachedToken.service()));
+                LOG.trace("Found token in cache for scopes: {}", newScopes.withService(cachedToken.service()));
             }
 
             // Add authentication header if any (from provider or cached token)
diff --git a/src/main/java/land/oras/auth/Scope.java b/src/main/java/land/oras/auth/Scope.java
@@ -25,6 +25,11 @@
  */
 public enum Scope {
 
+    /**
+     * Scope for all actions.
+     */
+    ALL,
+
     /**
      * Scope for pulling images.
      */
@@ -45,6 +50,9 @@ public enum Scope {
      * @return the lowercase name of the enum
      */
     public String toLowerCase() {
+        if (this == ALL) {
+            return "*";
+        }
         return name().toLowerCase();
     }
 }
diff --git a/src/main/java/land/oras/auth/Scopes.java b/src/main/java/land/oras/auth/Scopes.java
@@ -111,15 +111,53 @@ public Scopes withRegistryScopes(Scope... scopes) {
     }
 
     /**
-     * Return a new copy of the Scopes object with the given scopes
+     * Return a new copy of the Scopes object with the added scopes
      * @param newScopes The scopes to add
      * @return A new Scopes object with the given scopes
      */
-    public Scopes withNewRegistryScopes(Scope... newScopes) {
+    public Scopes withAddedRegistryScopes(Scope... newScopes) {
         return new Scopes(
                 containerRef, service, ScopeUtils.appendRepositoryScope(this.scopes, containerRef, newScopes));
     }
 
+    /**
+     * Return a new copy of the Scopes object with the given scopes
+     * @param globalScopes The global scopes to add
+     * @return A new Scopes object with the given scopes
+     */
+    public Scopes withAddedGlobalScopes(String... globalScopes) {
+        List<String> newScopes = new LinkedList<>(scopes);
+        newScopes.addAll(List.of(globalScopes));
+        return new Scopes(
+                containerRef, service, newScopes.stream().sorted().distinct().toList());
+    }
+
+    /**
+     * Return scopes that only contains global scopes (i.e. no repository or registry scopes)
+     * @return A new Scopes object with only global scopes
+     */
+    public Scopes withOnlyGlobalScopes() {
+        List<String> globalScopes = scopes.stream()
+                .filter(s -> !s.startsWith("repository:") && !s.startsWith("registry:"))
+                .sorted()
+                .distinct()
+                .toList();
+        return new Scopes(containerRef, service, globalScopes);
+    }
+
+    /**
+     * Return scopes that only contains non-global scopes (i.e. only repository or registry scopes)
+     * @return A new Scopes object with only non-global scopes
+     */
+    public Scopes withoutGlobalScopes() {
+        List<String> nonGlobalScopes = scopes.stream()
+                .filter(s -> s.startsWith("repository:") || s.startsWith("registry:"))
+                .sorted()
+                .distinct()
+                .toList();
+        return new Scopes(containerRef, service, nonGlobalScopes);
+    }
+
     /**
      * Return a new copy of the Scopes object with the given scope
      * @param scope The scope to add
@@ -172,6 +210,31 @@ public ContainerRef getContainerRef() {
         return containerRef;
     }
 
+    /**
+     * Return if these are global scopes (i.e. no repository or registry scopes)
+     * Typically AWS ECR uses global scopes for authentication, while Docker Hub uses repository scopes.
+     * @return True if these are global scopes, false otherwise
+     */
+    public boolean isGlobal() {
+        return scopes.stream().noneMatch(s -> s.startsWith("repository:") || s.startsWith("registry:"));
+    }
+
+    /**
+     * Return if these scopes include global scopes (i.e. at least one scope that is not a repository or registry scope)
+     * @return True if these scopes include global scopes, false otherwise
+     */
+    public boolean hasGlobalScopes() {
+        return scopes.stream().anyMatch(s -> !s.startsWith("repository:") && !s.startsWith("registry:"));
+    }
+
+    /**
+     * Return if these are pull-only scopes (i.e. all scopes end with ":pull" and there are no global scopes)
+     * @return True if these are pull-only scopes, false otherwise
+     */
+    public boolean isPullOnly() {
+        return !isGlobal() && scopes.stream().allMatch(s -> s.endsWith(":pull"));
+    }
+
     @Override
     public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) return false;
diff --git a/src/main/java/land/oras/auth/TokenCache.java b/src/main/java/land/oras/auth/TokenCache.java
@@ -89,11 +89,21 @@ public long expireAfterRead(
      * @param token The token response to be cached
      */
     public static void put(Scopes scopes, HttpClient.TokenResponse token) {
-        LOG.trace("Caching token for scopes: {}", scopes);
-        CACHE.put(scopes, token);
-        if (scopes.getService() != null) {
-            LOG.trace("Caching service for registry: {}", scopes.getRegistry());
-            SERVICE_CACHE.put(scopes.getRegistry(), scopes.getService());
+        Scopes newScopes = scopes.getService() != null ? scopes : scopes.withService(token.service());
+        LOG.trace("Caching token for scopes: {}", newScopes);
+        CACHE.put(newScopes, token);
+        if (newScopes.getService() != null) {
+            LOG.trace("Caching service '{}' for registry '{}'", newScopes.getService(), newScopes.getRegistry());
+            SERVICE_CACHE.put(scopes.getRegistry(), newScopes.getService());
+        }
+        // Store global scopes if present for future lookups
+        if (scopes.hasGlobalScopes()) {
+            Scopes newScopesWithService =
+                    scopes.withService(newScopes.getService()).withOnlyGlobalScopes();
+            CACHE.put(newScopesWithService, token);
+            Scopes newScopesWithoutGlobal =
+                    scopes.withService(newScopes.getService()).withoutGlobalScopes();
+            CACHE.put(newScopesWithoutGlobal, token);
         }
     }
 
@@ -103,16 +113,50 @@ public static void put(Scopes scopes, HttpClient.TokenResponse token) {
      * @return the token response associated with the scopes, or null if not found or expired
      */
     public static HttpClient.@Nullable TokenResponse get(Scopes scopes) {
+        String service =
+                scopes.getService() != null ? scopes.getService() : SERVICE_CACHE.getIfPresent(scopes.getRegistry());
         HttpClient.TokenResponse token = CACHE.getIfPresent(scopes);
-        if (token == null) {
-            // Lookup for specific service
-            String service = SERVICE_CACHE.getIfPresent(scopes.getRegistry());
-            if (service == null) {
-                return null;
+        if (token != null) {
+            LOG.trace("Direct cache hit for scopes: {}", scopes);
+            return token;
+        }
+        // Try lookup with service
+        token = CACHE.getIfPresent(scopes.withService(service));
+        if (token != null) {
+            LOG.trace("Cache lookup for scopes: {}, found with service '{}'", scopes, service);
+            return token;
+        }
+        // Check englobing scopes
+        if (scopes.isPullOnly() && !scopes.hasGlobalScopes()) {
+            Scopes newScopes = scopes.withService(service).withAddedRegistryScopes(Scope.PUSH); // Just add push scope
+            token = CACHE.getIfPresent(newScopes);
+            if (token != null) {
+                LOG.trace("Cache lookup for scopes: {}, found with push scope", scopes);
+                return token;
+            }
+            newScopes = newScopes.withService(service).withAddedRegistryScopes(Scope.DELETE); // Add delete scope
+            token = CACHE.getIfPresent(newScopes);
+            if (token != null) {
+                LOG.trace("Cache lookup for scopes: {}, found with push and delete scopes", scopes);
+                return token;
+            }
+            newScopes = newScopes.withService(service).withRegistryScopes(Scope.ALL); // All replace all scopes
+            token = CACHE.getIfPresent(newScopes);
+            if (token != null) {
+                LOG.trace("Cache lookup for scopes: {}, found with all scopes", scopes);
+                return token;
+            }
+            return null;
+        }
+        if (scopes.hasGlobalScopes()) {
+            token = CACHE.getIfPresent(scopes.withOnlyGlobalScopes());
+            if (token != null) {
+                LOG.trace("Cache lookup for scopes: {}, found with only global scopes", scopes);
+                return token;
             }
-            return CACHE.getIfPresent(scopes.withService(service));
         }
-        return token;
+        LOG.trace("Cache miss for scopes: {}", scopes);
+        return null;
     }
 
     /**
diff --git a/src/test/java/land/oras/PublicECRITCase.java b/src/test/java/land/oras/PublicECRITCase.java
@@ -22,15 +22,13 @@
 
 import static org.junit.jupiter.api.Assertions.*;
 
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 import org.junit.jupiter.api.parallel.Execution;
 import org.junit.jupiter.api.parallel.ExecutionMode;
-import uk.org.webcompere.systemstubs.environment.EnvironmentVariables;
 
 @Execution(ExecutionMode.SAME_THREAD)
 class PublicECRITCase {
@@ -69,25 +67,22 @@ void shouldRewriteDockerIOToPublicECR() throws Exception {
                 location = "public.ecr.aws/docker/library"
                 """;
 
-        Files.writeString(homeDir.resolve(".config").resolve("containers").resolve("registries.conf"), config);
-
-        new EnvironmentVariables()
-                .set("HOME", homeDir.toAbsolutePath().toString())
-                .execute(() -> {
-                    Registry registry = Registry.builder().defaults().build();
-                    ContainerRef containerRef = ContainerRef.parse("docker.io/library/alpine:latest");
-                    assertEquals("public.ecr.aws", containerRef.getEffectiveRegistry(registry));
-                    assertEquals(
-                            "public.ecr.aws/docker/library/alpine:latest",
-                            containerRef.forRegistry(registry).toString());
-                    assertEquals(
-                            "my-proxy/library/alpine:latest",
-                            containerRef
-                                    .forRegistry(Registry.builder()
-                                            .withRegistry("my-proxy")
-                                            .build())
-                                    .toString());
-                });
+        TestUtils.createRegistriesConfFile(homeDir, config);
+
+        TestUtils.withHome(homeDir, () -> {
+            Registry registry = Registry.builder().defaults().build();
+            ContainerRef containerRef = ContainerRef.parse("docker.io/library/alpine:latest");
+            assertEquals("public.ecr.aws", containerRef.getEffectiveRegistry(registry));
+            assertEquals(
+                    "public.ecr.aws/docker/library/alpine:latest",
+                    containerRef.forRegistry(registry).toString());
+            assertEquals(
+                    "my-proxy/library/alpine:latest",
+                    containerRef
+                            .forRegistry(
+                                    Registry.builder().withRegistry("my-proxy").build())
+                            .toString());
+        });
     }
 
     @Test
diff --git a/src/test/java/land/oras/auth/ScopesTest.java b/src/test/java/land/oras/auth/ScopesTest.java
@@ -48,6 +48,12 @@ void shouldBuildScopes() {
         assertEquals(
                 "Scopes{scopes=[repository:library/test:pull], service='docker', registry=localhost:5000}",
                 scopes.withService("docker").toString());
+        assertFalse(scopes.isGlobal(), "Scopes should not be global");
+        assertTrue(
+                Scopes.empty(containerRef, "aws.public").withNewScope("aws").isGlobal(),
+                "Scopes with global scope should be global");
+        assertTrue(scopes.isPullOnly(), "Scopes with only pull scope should be pull-only");
+        assertFalse(scopes.withRegistryScopes(Scope.PUSH).isPullOnly(), "Should not be pull only scope");
 
         Scopes newScopes = scopes.withRegistryScopes(Scope.PUSH);
         assertNotSame(scopes, newScopes, "Scopes should be immutable");
@@ -57,7 +63,7 @@ void shouldBuildScopes() {
         assertEquals("localhost:5000", newScopes.getRegistry());
         assertEquals("localhost:5000", scopes.withService("localhost:5000").getService());
 
-        Scopes newScopes2 = newScopes.withNewRegistryScopes(Scope.PULL);
+        Scopes newScopes2 = newScopes.withAddedRegistryScopes(Scope.PULL);
         assertNotSame(newScopes, newScopes2, "Scopes should be immutable");
         assertEquals(1, newScopes2.getScopes().size());
         assertEquals("repository:library/test:pull,push", newScopes2.getScopes().get(0));
diff --git a/src/test/java/land/oras/auth/TokenCacheTest.java b/src/test/java/land/oras/auth/TokenCacheTest.java
