Do not include auth header when redirecting to other domain or when switching to insecure protocol (#334)

Author: jonesbusy

src/main/java/land/oras/auth/HttpClient.java

@@ -33,6 +33,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -150,6 +151,7 @@ public ResponseWrapper<String> get(URI uri, Map<String, String> headers, Scopes
         return executeRequest(
                 "GET",
                 uri,
+                true,
                 headers,
                 new byte[0],
                 HttpResponse.BodyHandlers.ofString(),
@@ -172,6 +174,7 @@ public ResponseWrapper<Path> download(
         return executeRequest(
                 "GET",
                 uri,
+                true,
                 headers,
                 new byte[0],
                 HttpResponse.BodyHandlers.ofFile(file),
@@ -193,6 +196,7 @@ public ResponseWrapper<InputStream> download(
         return executeRequest(
                 "GET",
                 uri,
+                true,
                 headers,
                 new byte[0],
                 HttpResponse.BodyHandlers.ofInputStream(),
@@ -217,6 +221,7 @@ public ResponseWrapper<String> upload(
             return executeRequest(
                     method,
                     uri,
+                    true,
                     headers,
                     new byte[0],
                     HttpResponse.BodyHandlers.ofString(),
@@ -241,6 +246,7 @@ public ResponseWrapper<String> head(
         return executeRequest(
                 "HEAD",
                 uri,
+                true,
                 headers,
                 new byte[0],
                 HttpResponse.BodyHandlers.ofString(),
@@ -262,6 +268,7 @@ public ResponseWrapper<String> delete(
         return executeRequest(
                 "DELETE",
                 uri,
+                true,
                 headers,
                 new byte[0],
                 HttpResponse.BodyHandlers.ofString(),
@@ -284,6 +291,7 @@ public ResponseWrapper<String> post(
         return executeRequest(
                 "POST",
                 uri,
+                true,
                 headers,
                 body,
                 HttpResponse.BodyHandlers.ofString(),
@@ -306,6 +314,7 @@ public ResponseWrapper<String> patch(
         return executeRequest(
                 "PATCH",
                 uri,
+                true,
                 headers,
                 body,
                 HttpResponse.BodyHandlers.ofString(),
@@ -328,6 +337,7 @@ public ResponseWrapper<String> put(
         return executeRequest(
                 "PUT",
                 uri,
+                true,
                 headers,
                 body,
                 HttpResponse.BodyHandlers.ofString(),
@@ -397,6 +407,22 @@ public <T> TokenResponse refreshToken(
         return JsonUtils.fromJson(responseWrapper.response(), TokenResponse.class);
     }
 
+    static boolean isSameOrigin(URI uri1, URI uri2) {
+        return Objects.equals(uri1.getScheme(), uri2.getScheme())
+                && Objects.equals(uri1.getHost(), uri2.getHost())
+                && getPort(uri1) == getPort(uri2);
+    }
+
+    static int getPort(URI uri) {
+        return uri.getPort() != -1 ? uri.getPort() : ("https".equals(uri.getScheme()) ? 443 : 80);
+    }
+
+    static <T> boolean shouldRedirect(HttpResponse<T> response) {
+        return response.statusCode() == HttpURLConnection.HTTP_MOVED_PERM
+                || response.statusCode() == HttpURLConnection.HTTP_MOVED_TEMP
+                || response.statusCode() == 307;
+    }
+
     /**
      * Execute a request
      * @param method The method
@@ -411,6 +437,7 @@ public <T> TokenResponse refreshToken(
     private <T> ResponseWrapper<T> executeRequest(
             String method,
             URI uri,
+            boolean includeAuthHeader,
             Map<String, String> headers,
             byte[] body,
             HttpResponse.BodyHandler<T> handler,
@@ -435,7 +462,8 @@ private <T> ResponseWrapper<T> executeRequest(
 
             // Add authentication header if any
             if (authProvider.getAuthHeader(containerRef) != null
-                    && !authProvider.getAuthScheme().equals(AuthScheme.NONE)) {
+                    && !authProvider.getAuthScheme().equals(AuthScheme.NONE)
+                    && includeAuthHeader) {
                 builder = builder.header(Const.AUTHORIZATION_HEADER, authProvider.getAuthHeader(containerRef));
             }
             headers.forEach(builder::header);
@@ -450,9 +478,22 @@ private <T> ResponseWrapper<T> executeRequest(
             // Follow redirect
             if (shouldRedirect(response)) {
                 String location = getLocationHeader(response);
-                LOG.debug("Redirecting to {}", location);
+                URI redirectUri = URI.create(location);
+                LOG.debug("Redirecting to {} from domain {} to domain {}", location, uri, redirectUri);
+                boolean includeAuthHeaderForRedirect = isSameOrigin(uri, redirectUri);
+                if (!includeAuthHeaderForRedirect) {
+                    LOG.debug("Skipping auth header for redirect from {} to {}", uri, redirectUri);
+                }
                 return executeRequest(
-                        method, URI.create(location), headers, body, handler, bodyPublisher, newScopes, authProvider);
+                        method,
+                        redirectUri,
+                        includeAuthHeaderForRedirect,
+                        headers,
+                        body,
+                        handler,
+                        bodyPublisher,
+                        newScopes,
+                        authProvider);
             }
             return redoRequest(response, builder, handler, newScopes, authProvider);
         } catch (Exception e) {
@@ -461,12 +502,6 @@ private <T> ResponseWrapper<T> executeRequest(
         }
     }
 
-    private <T> boolean shouldRedirect(HttpResponse<T> response) {
-        return response.statusCode() == HttpURLConnection.HTTP_MOVED_PERM
-                || response.statusCode() == HttpURLConnection.HTTP_MOVED_TEMP
-                || response.statusCode() == 307;
-    }
-
     private <T> String getLocationHeader(HttpResponse<T> response) {
         return response.headers()
                 .firstValue("Location")

src/test/java/land/oras/AnnotationsTest.java

@@ -28,7 +28,7 @@
 import org.junit.jupiter.api.parallel.ExecutionMode;
 
 @Execution(ExecutionMode.CONCURRENT)
-public class AnnotationsTest {
+class AnnotationsTest {
 
     @Test
     public void fromJson() {

src/test/java/land/oras/ArtifactTypeTest.java

@@ -30,7 +30,7 @@
 import org.junit.jupiter.api.parallel.ExecutionMode;
 
 @Execution(ExecutionMode.CONCURRENT)
-public class ArtifactTypeTest {
+class ArtifactTypeTest {
 
     @Test
     void validateArtifactType() {

src/test/java/land/oras/ClassAnnotationsTest.java

@@ -34,7 +34,7 @@
 import org.junit.jupiter.api.parallel.ExecutionMode;
 
 @Execution(ExecutionMode.CONCURRENT)
-public class ClassAnnotationsTest {
+class ClassAnnotationsTest {
 
     @Test
     void shouldHaveAnnotationOnModel() {

src/test/java/land/oras/ConfigTest.java

@@ -29,7 +29,7 @@
 import org.junit.jupiter.api.parallel.ExecutionMode;
 
 @Execution(ExecutionMode.CONCURRENT)
-public class ConfigTest {
+class ConfigTest {
 
     @Test
     void shouldSerializeEmptyConfig() {

src/test/java/land/oras/DockerIoITCase.java

@@ -30,7 +30,7 @@
 import org.junit.jupiter.api.parallel.ExecutionMode;
 
 @Execution(ExecutionMode.CONCURRENT)
-public class DockerIoITCase {
+class DockerIoITCase {
 
     @TempDir
     Path tempDir;

src/test/java/land/oras/GitHubContainerRegistryITCase.java

@@ -30,7 +30,7 @@
 import org.junit.jupiter.api.parallel.ExecutionMode;
 
 @Execution(ExecutionMode.CONCURRENT)
-public class GitHubContainerRegistryITCase {
+class GitHubContainerRegistryITCase {
 
     @TempDir
     Path tempDir;

src/test/java/land/oras/HarborS3ITCase.java

@@ -0,0 +1,48 @@
+/*-
+ * =LICENSE=
+ * ORAS Java SDK
+ * ===
+ * Copyright (C) 2024 - 2025 ORAS
+ * ===
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * =LICENSEEND=
+ */
+
+package land.oras;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.nio.file.Path;
+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;
+
+@Execution(ExecutionMode.CONCURRENT)
+class HarborS3ITCase {
+
+    @TempDir
+    Path tempDir;
+
+    @Test
+    @Disabled("Only to test with demo Harbor S3 instance")
+    void shouldPullOneBlob() {
+        Registry registry = Registry.builder().defaults().build();
+        ContainerRef containerRef1 = ContainerRef.parse("demo.goharbor.io/oras/lib:foo");
+        Manifest manifest = registry.getManifest(containerRef1);
+        Layer oneLayer = manifest.getLayers().get(0);
+        registry.fetchBlob(containerRef1.withDigest(oneLayer.getDigest()), tempDir.resolve("my-blob"));
+        assertNotNull(tempDir.resolve("my-blob"));
+    }
+}

src/test/java/land/oras/IndexTest.java

@@ -31,7 +31,7 @@
 import org.junit.jupiter.api.parallel.ExecutionMode;
 
 @Execution(ExecutionMode.CONCURRENT)
-public class IndexTest {
+class IndexTest {
 
     @Test
     void shouldReadAndWriteIndex() {

src/test/java/land/oras/JFrogArtifactoryITCase.java

@@ -30,7 +30,7 @@
 import org.junit.jupiter.api.parallel.ExecutionMode;
 
 @Execution(ExecutionMode.CONCURRENT)
-public class JFrogArtifactoryITCase {
+class JFrogArtifactoryITCase {
 
     @TempDir
     Path tempDir;