Skip to content

Commit a3c5059

Browse files
committed
Do not include auth header when redirecting to other domain
Signed-off-by: Valentin Delaye <jonesbusy@users.noreply.github.com>
1 parent 0161a89 commit a3c5059

3 files changed

Lines changed: 141 additions & 3 deletions

File tree

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

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.HashMap;
3434
import java.util.List;
3535
import java.util.Map;
36+
import java.util.Objects;
3637
import java.util.regex.Matcher;
3738
import java.util.regex.Pattern;
3839
import java.util.stream.Collectors;
@@ -150,6 +151,7 @@ public ResponseWrapper<String> get(URI uri, Map<String, String> headers, Scopes
150151
return executeRequest(
151152
"GET",
152153
uri,
154+
true,
153155
headers,
154156
new byte[0],
155157
HttpResponse.BodyHandlers.ofString(),
@@ -172,6 +174,7 @@ public ResponseWrapper<Path> download(
172174
return executeRequest(
173175
"GET",
174176
uri,
177+
true,
175178
headers,
176179
new byte[0],
177180
HttpResponse.BodyHandlers.ofFile(file),
@@ -193,6 +196,7 @@ public ResponseWrapper<InputStream> download(
193196
return executeRequest(
194197
"GET",
195198
uri,
199+
true,
196200
headers,
197201
new byte[0],
198202
HttpResponse.BodyHandlers.ofInputStream(),
@@ -217,6 +221,7 @@ public ResponseWrapper<String> upload(
217221
return executeRequest(
218222
method,
219223
uri,
224+
true,
220225
headers,
221226
new byte[0],
222227
HttpResponse.BodyHandlers.ofString(),
@@ -241,6 +246,7 @@ public ResponseWrapper<String> head(
241246
return executeRequest(
242247
"HEAD",
243248
uri,
249+
true,
244250
headers,
245251
new byte[0],
246252
HttpResponse.BodyHandlers.ofString(),
@@ -262,6 +268,7 @@ public ResponseWrapper<String> delete(
262268
return executeRequest(
263269
"DELETE",
264270
uri,
271+
true,
265272
headers,
266273
new byte[0],
267274
HttpResponse.BodyHandlers.ofString(),
@@ -284,6 +291,7 @@ public ResponseWrapper<String> post(
284291
return executeRequest(
285292
"POST",
286293
uri,
294+
true,
287295
headers,
288296
body,
289297
HttpResponse.BodyHandlers.ofString(),
@@ -306,6 +314,7 @@ public ResponseWrapper<String> patch(
306314
return executeRequest(
307315
"PATCH",
308316
uri,
317+
true,
309318
headers,
310319
body,
311320
HttpResponse.BodyHandlers.ofString(),
@@ -328,6 +337,7 @@ public ResponseWrapper<String> put(
328337
return executeRequest(
329338
"PUT",
330339
uri,
340+
true,
331341
headers,
332342
body,
333343
HttpResponse.BodyHandlers.ofString(),
@@ -411,6 +421,7 @@ public <T> TokenResponse refreshToken(
411421
private <T> ResponseWrapper<T> executeRequest(
412422
String method,
413423
URI uri,
424+
boolean includeAuthHeader,
414425
Map<String, String> headers,
415426
byte[] body,
416427
HttpResponse.BodyHandler<T> handler,
@@ -435,7 +446,8 @@ private <T> ResponseWrapper<T> executeRequest(
435446

436447
// Add authentication header if any
437448
if (authProvider.getAuthHeader(containerRef) != null
438-
&& !authProvider.getAuthScheme().equals(AuthScheme.NONE)) {
449+
&& !authProvider.getAuthScheme().equals(AuthScheme.NONE)
450+
&& includeAuthHeader) {
439451
builder = builder.header(Const.AUTHORIZATION_HEADER, authProvider.getAuthHeader(containerRef));
440452
}
441453
headers.forEach(builder::header);
@@ -450,9 +462,22 @@ private <T> ResponseWrapper<T> executeRequest(
450462
// Follow redirect
451463
if (shouldRedirect(response)) {
452464
String location = getLocationHeader(response);
453-
LOG.debug("Redirecting to {}", location);
465+
URI redirectUri = URI.create(location);
466+
LOG.debug("Redirecting to {} from domain {} to domain {}", location, uri, redirectUri);
467+
boolean includeAuthHeaderForRedirect = isSameOrigin(uri, redirectUri);
468+
if (!includeAuthHeaderForRedirect) {
469+
LOG.debug("Skipping auth header for redirect from {} to {}", uri, redirectUri);
470+
}
454471
return executeRequest(
455-
method, URI.create(location), headers, body, handler, bodyPublisher, newScopes, authProvider);
472+
method,
473+
redirectUri,
474+
includeAuthHeaderForRedirect,
475+
headers,
476+
body,
477+
handler,
478+
bodyPublisher,
479+
newScopes,
480+
authProvider);
456481
}
457482
return redoRequest(response, builder, handler, newScopes, authProvider);
458483
} catch (Exception e) {
@@ -461,6 +486,20 @@ private <T> ResponseWrapper<T> executeRequest(
461486
}
462487
}
463488

489+
private static boolean isSameOrigin(URI uri1, URI uri2) {
490+
return Objects.equals(uri1.getScheme(), uri2.getScheme())
491+
&& Objects.equals(uri1.getHost(), uri2.getHost())
492+
&& getPort(uri1) == getPort(uri2);
493+
}
494+
495+
private static int getPort(URI uri) {
496+
int port = uri.getPort();
497+
if (port == -1) {
498+
return uri.getScheme().equals("https") ? 443 : 80;
499+
}
500+
return port;
501+
}
502+
464503
private <T> boolean shouldRedirect(HttpResponse<T> response) {
465504
return response.statusCode() == HttpURLConnection.HTTP_MOVED_PERM
466505
|| response.statusCode() == HttpURLConnection.HTTP_MOVED_TEMP
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*-
2+
* =LICENSE=
3+
* ORAS Java SDK
4+
* ===
5+
* Copyright (C) 2024 - 2025 ORAS
6+
* ===
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
* =LICENSEEND=
19+
*/
20+
21+
package land.oras;
22+
23+
import static org.junit.jupiter.api.Assertions.assertNotNull;
24+
25+
import java.nio.file.Path;
26+
import org.junit.jupiter.api.Disabled;
27+
import org.junit.jupiter.api.Test;
28+
import org.junit.jupiter.api.io.TempDir;
29+
import org.junit.jupiter.api.parallel.Execution;
30+
import org.junit.jupiter.api.parallel.ExecutionMode;
31+
32+
@Execution(ExecutionMode.CONCURRENT)
33+
public class HarborS3ITCase {
34+
35+
@TempDir
36+
Path tempDir;
37+
38+
@Test
39+
@Disabled("Only to test with demo Harbor S3 instance")
40+
void shouldPullOneBlob() {
41+
Registry registry = Registry.builder().defaults().build();
42+
ContainerRef containerRef1 = ContainerRef.parse("demo.goharbor.io/oras/lib:foo");
43+
Manifest manifest = registry.getManifest(containerRef1);
44+
Layer oneLayer = manifest.getLayers().get(0);
45+
registry.fetchBlob(containerRef1.withDigest(oneLayer.getDigest()), tempDir.resolve("my-blob"));
46+
assertNotNull(tempDir.resolve("my-blob"));
47+
}
48+
}

src/test/java/land/oras/RegistryWireMockTest.java

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@
2525
import static org.junit.jupiter.api.Assertions.assertNotNull;
2626
import static org.junit.jupiter.api.Assertions.assertThrows;
2727

28+
import com.github.tomakehurst.wiremock.WireMockServer;
2829
import com.github.tomakehurst.wiremock.client.WireMock;
30+
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
2931
import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
3032
import com.github.tomakehurst.wiremock.junit5.WireMockTest;
3133
import com.github.tomakehurst.wiremock.stubbing.Scenario;
@@ -134,6 +136,55 @@ void shouldRedirectWhenPushingBlob(WireMockRuntimeInfo wmRuntimeInfo) throws IOE
134136
registry.pushBlob(containerRef, testFile);
135137
}
136138

139+
@Test
140+
void shouldNotSendAuthHeaderOnRedirectToDifferentDomain(WireMockRuntimeInfo wmRuntimeInfo) {
141+
String digest = SupportedAlgorithm.SHA256.digest("blob-data".getBytes());
142+
143+
// Setup second WireMock instance on a different port
144+
WireMockServer redirectTarget =
145+
new WireMockServer(WireMockConfiguration.options().dynamicPort());
146+
redirectTarget.start();
147+
148+
try {
149+
String redirectUrl = "http://localhost:%d/v2/other/blobs/sha256:other".formatted(redirectTarget.port());
150+
151+
WireMock mainMock = wmRuntimeInfo.getWireMock();
152+
153+
// Main mock responds with redirect to different domain
154+
mainMock.register(WireMock.any(WireMock.urlEqualTo("/v2/library/artifact/blobs/%s".formatted(digest)))
155+
.willReturn(WireMock.temporaryRedirect(redirectUrl)));
156+
157+
// Secondary server returns blob, we inspect headers here
158+
redirectTarget.stubFor(WireMock.get(WireMock.urlEqualTo("/v2/other/blobs/sha256:other"))
159+
.willReturn(WireMock.ok()
160+
.withBody("blob-data")
161+
.withHeader(Const.DOCKER_CONTENT_DIGEST_HEADER, digest)));
162+
163+
redirectTarget.stubFor(WireMock.head(WireMock.urlEqualTo("/v2/other/blobs/sha256:other"))
164+
.willReturn(WireMock.ok()));
165+
166+
// Registry setup with auth that would inject an Authorization header
167+
Registry registry = Registry.Builder.builder()
168+
.withAuthProvider(authProvider)
169+
.withInsecure(true)
170+
.build();
171+
172+
ContainerRef containerRef =
173+
ContainerRef.parse("localhost:%d/library/artifact".formatted(wmRuntimeInfo.getHttpPort()));
174+
byte[] blob = registry.getBlob(containerRef.withDigest(digest));
175+
176+
assertEquals("blob-data", new String(blob));
177+
178+
// Assert Authorization header was not sent to the redirect target
179+
redirectTarget.verify(
180+
1,
181+
WireMock.getRequestedFor(WireMock.urlEqualTo("/v2/other/blobs/sha256:other"))
182+
.withoutHeader("Authorization"));
183+
} finally {
184+
redirectTarget.stop();
185+
}
186+
}
187+
137188
@Test
138189
void shouldListTags(WireMockRuntimeInfo wmRuntimeInfo) {
139190

0 commit comments

Comments
 (0)