Skip to content

Commit d0eda87

Browse files
authored
Handle unresolved Maven credential placeholders gracefully (#6845)
* Add failing tests for unresolved Maven credential placeholders When Maven settings.xml uses ${env.MAVEN_USER} placeholders for server credentials and those env vars are not set, the literal placeholder string is sent as the username, causing 403 errors on artifact downloads. These tests verify: - Unresolved credential placeholders are nulled out by the Interpolator - MavenArtifactDownloader falls back to anonymous on 403 with credentials * Null out unresolved credential placeholders in MavenSettings When Maven settings.xml uses ${env.MAVEN_USER} / ${env.MAVEN_PASSWORD} placeholders and those env vars are not set, the PropertyPlaceholderHelper preserves the literal "${env.FOO}" string. This was then sent as actual credentials, causing 403 errors. Now the Interpolator checks for remaining "${" in server credentials after interpolation and nulls them out, so no auth header is sent and the download proceeds anonymously — matching Apache Maven behavior. Also adds @nullable to Server.username and Server.password since credentials can genuinely be absent. Fixes moderneinc/customer-requests#1928 * Add anonymous fallback to MavenArtifactDownloader When credentials are applied and the server responds with a 4xx client error, retry the download without authentication. This mirrors the existing behavior in MavenPomDownloader.requestAsAuthenticatedOrAnonymous() and matches how Apache Maven handles credential failures gracefully. This is defense-in-depth alongside the Interpolator fix — it also protects against other credential mismatch scenarios. * Revert the changes to MavenSettings * Slight polish * Revert nullable fields * Minimize changes * Move down repository field as well * Do not pass username/password containing `${` * Expect exactly one unauthenticated request * Minimize diff
1 parent de4f1d9 commit d0eda87

2 files changed

Lines changed: 74 additions & 7 deletions

File tree

rewrite-maven/src/main/java/org/openrewrite/maven/utilities/MavenArtifactDownloader.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,16 +130,23 @@ public MavenArtifactDownloader(MavenArtifactCache mavenArtifactCache,
130130
}
131131

132132
private HttpSender.Request.Builder applyAuthentication(MavenRepository repository, HttpSender.Request.Builder request) {
133+
String username, password;
133134
MavenSettings.Server authInfo = serverIdToServer.get(repository.getId());
134135
if (authInfo != null) {
135136
if (authInfo.getConfiguration() != null && authInfo.getConfiguration().getHttpHeaders() != null) {
136137
for (MavenSettings.HttpHeader header : authInfo.getConfiguration().getHttpHeaders()) {
137138
request.withHeader(header.getName(), header.getValue());
138139
}
139140
}
140-
return request.withBasicAuthentication(authInfo.getUsername(), authInfo.getPassword());
141-
} else if (repository.getUsername() != null && repository.getPassword() != null) {
142-
return request.withBasicAuthentication(repository.getUsername(), repository.getPassword());
141+
username = authInfo.getUsername();
142+
password = authInfo.getPassword();
143+
} else {
144+
username = repository.getUsername();
145+
password = repository.getPassword();
146+
}
147+
if (username != null && !username.contains("${") &&
148+
password != null && !password.contains("${")) {
149+
return request.withBasicAuthentication(username, password);
143150
}
144151
return request;
145152
}

rewrite-maven/src/test/java/org/openrewrite/maven/utilities/MavenArtifactDownloaderTest.java

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,26 @@
1515
*/
1616
package org.openrewrite.maven.utilities;
1717

18+
import okhttp3.mockwebserver.Dispatcher;
19+
import okhttp3.mockwebserver.MockResponse;
20+
import okhttp3.mockwebserver.MockWebServer;
21+
import okhttp3.mockwebserver.RecordedRequest;
1822
import org.junit.jupiter.api.Test;
1923
import org.junit.jupiter.api.io.TempDir;
2024
import org.openrewrite.ExecutionContext;
2125
import org.openrewrite.InMemoryExecutionContext;
26+
import org.openrewrite.Parser;
2227
import org.openrewrite.SourceFile;
2328
import org.openrewrite.maven.MavenParser;
29+
import org.openrewrite.maven.MavenSettings;
2430
import org.openrewrite.maven.cache.LocalMavenArtifactCache;
2531
import org.openrewrite.maven.cache.MavenArtifactCache;
26-
import org.openrewrite.maven.tree.MavenResolutionResult;
27-
import org.openrewrite.maven.tree.ResolvedDependency;
28-
import org.openrewrite.maven.tree.ResolvedGroupArtifactVersion;
29-
import org.openrewrite.maven.tree.Scope;
32+
import org.openrewrite.maven.tree.*;
3033

34+
import java.io.ByteArrayInputStream;
3135
import java.nio.file.Path;
3236
import java.util.List;
37+
import java.util.concurrent.atomic.AtomicReference;
3338

3439
import static org.assertj.core.api.Assertions.assertThat;
3540
import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -126,4 +131,59 @@ void downloadDependenciesWithClassifier(@TempDir Path tempDir) {
126131
}
127132
}
128133
}
134+
135+
@Test
136+
void fallsBackToAnonymousWhenCredentialsRejected(@TempDir Path tempDir) throws Exception {
137+
byte[] jarBytes = {0x50, 0x4B, 0x03, 0x04}; // minimal ZIP magic bytes
138+
139+
try (MockWebServer mockRepo = new MockWebServer()) {
140+
mockRepo.setDispatcher(new Dispatcher() {
141+
@Override
142+
public MockResponse dispatch(RecordedRequest request) {
143+
if (request.getHeader("Authorization") != null) {
144+
return new MockResponse().setResponseCode(403); // Throw if used; it should not be called at all
145+
}
146+
return new MockResponse().setResponseCode(200)
147+
.setBody(new okio.Buffer().write(jarBytes));
148+
}
149+
});
150+
mockRepo.start();
151+
152+
String repoUrl = "http://" + mockRepo.getHostName() + ":" + mockRepo.getPort();
153+
MavenSettings settings = MavenSettings.parse(new Parser.Input(
154+
Path.of("settings.xml"), () -> new ByteArrayInputStream(
155+
//language=xml
156+
"""
157+
<settings>
158+
<servers>
159+
<server>
160+
<id>mock-repo</id>
161+
<username>${placeholder}</username>
162+
<password>${placeholder}</password>
163+
</server>
164+
</servers>
165+
</settings>
166+
""".getBytes())), new InMemoryExecutionContext());
167+
168+
MavenArtifactCache artifactCache = new LocalMavenArtifactCache(tempDir);
169+
AtomicReference<Throwable> error = new AtomicReference<>();
170+
MavenArtifactDownloader downloader = new MavenArtifactDownloader(
171+
artifactCache, settings, error::set);
172+
173+
MavenRepository repo = new MavenRepository(
174+
"mock-repo", repoUrl, "true", "false", true, null, null, null, false);
175+
GroupArtifactVersion gav = new GroupArtifactVersion("com.example", "test-lib", "1.0.0");
176+
ResolvedDependency dep = ResolvedDependency.builder()
177+
.repository(repo)
178+
.gav(new ResolvedGroupArtifactVersion(repoUrl, gav.getGroupId(), gav.getArtifactId(), gav.getVersion(), null))
179+
.requested(Dependency.builder().gav(gav).build())
180+
.build();
181+
182+
Path artifact = downloader.downloadArtifact(dep);
183+
184+
assertThat(artifact).isNotNull();
185+
assertThat(error.get()).isNull();
186+
assertThat(mockRepo.getRequestCount()).isEqualTo(1);
187+
}
188+
}
129189
}

0 commit comments

Comments
 (0)