From 024e0f0ca18d8a1e885bf81af41affa277a0a58d Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Thu, 24 Sep 2026 22:55:38 +0200 Subject: [PATCH] Issue #13265: let a settings server declare the repository origins its credentials may be used with Server credentials are scoped to the origin (scheme, host and port) of the repositories and mirrors declared with the same server id. Repositories declared inside a settings profile only reach that set when the profile id is listed in ; a profile activated through or -P contributes no origin, so its credentials are warned about under the default origin scope and refused under strict, with nothing the operator can do about it. The session is built before any project exists, so profile activation cannot be evaluated there. A can now declare those origins explicitly: internal u p https://repo.example.org https://mirror.example.org:8443 The declared origins are added to the ones Maven derives on its own; nothing is replaced. Values are bare origins, not repository URLs, and are validated: a value no origin can be derived from is an error, while a full repository URL is accepted with a warning naming the origin actually used. The deprecated V3 validator skips values with a property placeholder, as it runs before interpolation. The element is dropped from project settings, like credentials are. This is a security requirement rather than tidiness: settings list fields merge by union and a project-settings of the same id survives as a separate entry, so without it a project could widen the origins its user's credentials are sent to. The warnings of the origin binding now name as the way to declare a missing origin. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/main/mdo/settings.mdo | 19 ++++ .../validation/DefaultSettingsValidator.java | 79 +++++++++++++ .../DefaultSettingsValidatorTest.java | 69 ++++++++++++ ...DefaultRepositorySystemSessionFactory.java | 25 ++++- .../OriginBoundAuthenticationSelector.java | 30 +++-- ...ultRepositorySystemSessionFactoryTest.java | 102 +++++++++++++++++ .../maven/impl/DefaultSettingsBuilder.java | 1 + .../maven/impl/DefaultSettingsValidator.java | 89 +++++++++++++++ .../DefaultSettingsBuilderFactoryTest.java | 17 +++ .../maven/impl/DefaultSettingsParserTest.java | 28 +++++ .../impl/DefaultSettingsValidatorTest.java | 106 ++++++++++++++++++ .../resources/settings/settings-servers-4.xml | 38 +++++++ 12 files changed, 588 insertions(+), 15 deletions(-) create mode 100644 impl/maven-impl/src/test/resources/settings/settings-servers-4.xml diff --git a/api/maven-api-settings/src/main/mdo/settings.mdo b/api/maven-api-settings/src/main/mdo/settings.mdo index b9f37bda2277..d8b61aee9d12 100644 --- a/api/maven-api-settings/src/main/mdo/settings.mdo +++ b/api/maven-api-settings/src/main/mdo/settings.mdo @@ -545,6 +545,25 @@ * + + repositoryOrigins + 1.3.0+ + List of repository origins these credentials may be used with, each written as + {@code scheme://host} or {@code scheme://host:port}, for example {@code https://repo.example.org} + or {@code https://mirror.example.org:8443} - an origin, not a full repository URL. + Maven binds the credentials of a server to the origins of the repositories and mirrors declared + with the same id, see the {@code maven.repository.credentialScope} user property. The origins + listed here are added to those; nothing is replaced. Declaring them is needed when the repository + using these credentials is not known when the session is created, for example a repository + declared in a {@code settings.xml} profile that is not listed in {@code <activeProfiles>} + but activated through {@code <activation>} or {@code -P}. + Comparison is case insensitive and the default ports 80 (http) and 443 (https) are equivalent to + no port. This element is not supported in project settings and is dropped there. + + String + * + + diff --git a/compat/maven-settings-builder/src/main/java/org/apache/maven/settings/validation/DefaultSettingsValidator.java b/compat/maven-settings-builder/src/main/java/org/apache/maven/settings/validation/DefaultSettingsValidator.java index e274a2887f66..b89f8767df28 100644 --- a/compat/maven-settings-builder/src/main/java/org/apache/maven/settings/validation/DefaultSettingsValidator.java +++ b/compat/maven-settings-builder/src/main/java/org/apache/maven/settings/validation/DefaultSettingsValidator.java @@ -21,6 +21,8 @@ import javax.inject.Named; import javax.inject.Singleton; +import java.net.URI; +import java.net.URISyntaxException; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -109,6 +111,8 @@ public void validate(Settings settings, SettingsProblemCollector problems) { "must be unique across all server ids and aliases but found duplicate alias " + alias); } } + + validateRepositoryOrigins(problems, server, i); } } @@ -239,6 +243,81 @@ private void validateRepositories(SettingsProblemCollector problems, Liststring.length > 0 * */ + private static void validateRepositoryOrigins(SettingsProblemCollector problems, Server server, int index) { + for (int o = 0; o < server.getRepositoryOrigins().size(); o++) { + String repositoryOrigin = server.getRepositoryOrigins().get(o); + String fieldName = "servers.server[" + index + "].repositoryOrigins[" + o + "]"; + + if (!validateStringNotEmpty(problems, fieldName, repositoryOrigin, server.getId())) { + continue; + } + + // settings are validated before they are interpolated here, so a placeholder is not something + // this validator can judge + if (repositoryOrigin.contains("${")) { + continue; + } + + String invalid = invalidRepositoryOriginReason(repositoryOrigin); + if (invalid != null) { + addViolation(problems, Severity.ERROR, fieldName, server.getId(), invalid); + continue; + } + + String ignored = ignoredRepositoryOriginPartsReason(repositoryOrigin); + if (ignored != null) { + addViolation(problems, Severity.WARNING, fieldName, server.getId(), ignored); + } + } + } + + /** + * Parses a {@code } value the way the credential scoping does; see + * {@code OriginBoundAuthenticationSelector#originOf(String)} in maven-core, which is the authority + * on what an origin is. Returns {@code null} when the value is not a URI at all. + */ + private static URI parseRepositoryOrigin(String value) { + try { + return new URI(value).parseServerAuthority(); + } catch (URISyntaxException e) { + return null; + } + } + + /** + * @return the reason why no origin can be derived from the given value, or {@code null} if one can + */ + private static String invalidRepositoryOriginReason(String value) { + URI uri = parseRepositoryOrigin(value); + if (uri == null) { + return "must be a repository origin of the form scheme://host[:port] but found '" + value + "'"; + } + if (uri.getScheme() == null) { + return "must start with a scheme, for example https://repo.example.org, but found '" + value + "'"; + } + if (uri.getHost() == null) { + return "must name a host, for example https://repo.example.org, but found '" + value + "'"; + } + if (uri.getUserInfo() != null) { + return "must not carry user information but found '" + value + "'"; + } + return null; + } + + /** + * @return the reason why parts of the given value are ignored, or {@code null} if it is a bare origin + */ + private static String ignoredRepositoryOriginPartsReason(String value) { + URI uri = parseRepositoryOrigin(value); + String path = uri.getRawPath(); + boolean extraPath = path != null && !path.isEmpty() && !"/".equals(path); + if (!extraPath && uri.getRawQuery() == null && uri.getRawFragment() == null) { + return null; + } + return "is a repository origin, not a repository URL; only '" + uri.getScheme() + "://" + uri.getAuthority() + + "' of '" + value + "' is used"; + } + private static boolean validateStringNotEmpty( SettingsProblemCollector problems, String fieldName, String string, String sourceHint) { if (!validateNotNull(problems, fieldName, string, sourceHint)) { diff --git a/compat/maven-settings-builder/src/test/java/org/apache/maven/settings/validation/DefaultSettingsValidatorTest.java b/compat/maven-settings-builder/src/test/java/org/apache/maven/settings/validation/DefaultSettingsValidatorTest.java index 4bec76a359bf..a16af1a4e31b 100644 --- a/compat/maven-settings-builder/src/test/java/org/apache/maven/settings/validation/DefaultSettingsValidatorTest.java +++ b/compat/maven-settings-builder/src/test/java/org/apache/maven/settings/validation/DefaultSettingsValidatorTest.java @@ -271,6 +271,75 @@ void testValidateServerIdAliasesWithEmptyValue() { assertContains(problems.messages.get(0), "'servers.server[0].aliases[0]' for server-1 is missing"); } + @Test + void testValidateServerRepositoryOrigins() { + SimpleProblemCollector problems = validateRepositoryOrigins( + "https://repo.example.org", "https://mirror.example.org:8443", "HTTP://Repo.Example.Org:80"); + assertEquals(0, problems.messages.size()); + } + + @Test + void testValidateServerRepositoryOriginWithoutScheme() { + SimpleProblemCollector problems = validateRepositoryOrigins("repo.example.org"); + assertEquals(1, problems.messages.size()); + assertContains( + problems.messages.get(0), + "'servers.server[0].repositoryOrigins[0]' for server-1 must start with a scheme," + + " for example https://repo.example.org, but found 'repo.example.org'"); + } + + @Test + void testValidateServerRepositoryOriginWithoutHost() { + SimpleProblemCollector problems = validateRepositoryOrigins("file:/tmp/repo"); + assertEquals(1, problems.messages.size()); + assertContains(problems.messages.get(0), "must name a host"); + } + + @Test + void testValidateServerRepositoryOriginWithUserInfo() { + SimpleProblemCollector problems = validateRepositoryOrigins("https://user:pwd@repo.example.org"); + assertEquals(1, problems.messages.size()); + assertContains(problems.messages.get(0), "must not carry user information"); + } + + @Test + void testValidateServerRepositoryOriginEmpty() { + SimpleProblemCollector problems = validateRepositoryOrigins(""); + assertEquals(1, problems.messages.size()); + assertContains(problems.messages.get(0), "'servers.server[0].repositoryOrigins[0]' for server-1 is missing"); + } + + @Test + void testValidateServerRepositoryOriginWithPath() { + SimpleProblemCollector problems = validateRepositoryOrigins("https://repo.example.org/releases/"); + assertEquals(1, problems.messages.size()); + assertContains( + problems.messages.get(0), + "is a repository origin, not a repository URL; only 'https://repo.example.org'" + + " of 'https://repo.example.org/releases/' is used"); + } + + @Test + void testValidateServerRepositoryOriginWithPlaceholderIsNotValidated() { + // this validator runs before the settings are interpolated + SimpleProblemCollector problems = validateRepositoryOrigins("${env.REPO_URL}"); + assertEquals(0, problems.messages.size()); + } + + private SimpleProblemCollector validateRepositoryOrigins(String... repositoryOrigins) { + Settings settings = new Settings(); + Server server = new Server(); + server.setId("server-1"); + for (String repositoryOrigin : repositoryOrigins) { + server.addRepositoryOrigin(repositoryOrigin); + } + settings.addServer(server); + + SimpleProblemCollector problems = new SimpleProblemCollector(); + validator.validate(settings, problems); + return problems; + } + private static class SimpleProblemCollector implements SettingsProblemCollector { List messages = new ArrayList<>(); diff --git a/impl/maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java b/impl/maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java index 45ca17114bd3..a99cbc2780d8 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java +++ b/impl/maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java @@ -105,15 +105,20 @@ public class DefaultRepositorySystemSessionFactory implements RepositorySystemSe * User property selecting how server credentials configured in settings are scoped to repositories: *
    *
  • {@code origin} (default): credentials for a server id are only used with a repository whose - * origin (protocol, host and port) matches a repository or mirror declared with the same id in - * settings or on the command line. For server ids without any such declared repository (for - * example pure deployment servers whose URL comes from the project's + * origin (protocol, host and port) matches an origin declared for the same id, either by a + * repository or mirror declared with that id in settings or on the command line, or by the + * {@code } of that server in settings. For server ids without any such declared + * origin (for example pure deployment servers whose URL comes from the project's * {@code distributionManagement}), credentials are used as before, but a warning identifying the * target origin is emitted.
    • *
  • {@code strict}: like {@code origin}, but credentials are refused for server ids that have no - * repository or mirror declared in settings or on the command line.
    • + * declared origin at all. *
  • {@code id}: legacy behavior, credentials are matched by server id only.
    • *
+ *

+ * Repositories declared inside a settings {@code } only contribute an origin when the profile + * id is listed in {@code }; profiles activated through {@code } or + * {@code -P} contribute none, which is what {@code } is for. * * @since 4.0.0 */ @@ -214,8 +219,9 @@ public SessionBuilder newRepositorySessionBuilder(MavenExecutionRequest request) .buildVersionFilter(mergedProps.get(Constants.MAVEN_VERSION_FILTER), this::parseVersionConstraint) .ifPresent(sessionBuilder::setVersionFilter); - // origins of the repositories and mirrors the operator declared for a given server id, used below - // to scope that id's credentials to the origin(s) it was actually configured for + // origins of the repositories and mirrors the operator declared for a given server id, completed + // below with the origins declared on the servers themselves, used to scope that id's credentials + // to the origin(s) it was actually configured for Map> declaredRepositoryOrigins = new HashMap<>(); DefaultMirrorSelector mirrorSelector = new DefaultMirrorSelector(); @@ -260,6 +266,13 @@ public SessionBuilder newRepositorySessionBuilder(MavenExecutionRequest request) authBuilder.addPrivateKey(server.getPrivateKey(), server.getPassphrase()); authSelector.add(server.getId(), authBuilder.build()); + // origins the operator bound to this id explicitly, added to the ones collected above from the + // mirrors and the repositories of the request + for (String repositoryOrigin : server.getRepositoryOrigins()) { + OriginBoundAuthenticationSelector.addOrigin( + declaredRepositoryOrigins, server.getId(), repositoryOrigin); + } + if (server.getConfiguration() != null) { XmlNode dom = server.getDelegate().getConfiguration(); List children = dom.children().stream() diff --git a/impl/maven-core/src/main/java/org/apache/maven/internal/aether/OriginBoundAuthenticationSelector.java b/impl/maven-core/src/main/java/org/apache/maven/internal/aether/OriginBoundAuthenticationSelector.java index 030b38742d32..a9f16621af34 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/internal/aether/OriginBoundAuthenticationSelector.java +++ b/impl/maven-core/src/main/java/org/apache/maven/internal/aether/OriginBoundAuthenticationSelector.java @@ -38,10 +38,11 @@ * port) of the repository or mirror the operator declared for the same server id. *

* A repository's id and its origin are independent: this selector serves a server id's credentials - * only to a repository whose origin matches one the operator declared for that id, in settings or on - * the command line. Ids with no operator-declared origin keep the previous behaviour unless - * {@code strict} scope is requested, and a warning naming the target origin is emitted once per - * id/origin pair. + * only to a repository whose origin matches one the operator declared for that id, either by declaring + * a repository or mirror with that id in settings or on the command line, or by listing the origin in + * that server's {@code } in settings. Ids with no operator-declared origin keep the + * previous behaviour unless {@code strict} scope is requested, and a warning naming the target origin + * is emitted once per id/origin pair. * * @see DefaultRepositorySystemSessionFactory#MAVEN_REPOSITORY_CREDENTIAL_SCOPE */ @@ -135,8 +136,10 @@ public Authentication getAuthentication(RemoteRepository repository) { id, origin, "Not using credentials of server '" + id + "' for repository " + repository.getUrl() - + ": the repository or mirror declared for this id resides at " + origins - + ". Set " + + ": the origins declared for this id are " + origins + + ". Add " + originHint(origin) + + " to of that server in settings if these credentials belong" + + " there, or set " + DefaultRepositorySystemSessionFactory.MAVEN_REPOSITORY_CREDENTIAL_SCOPE + "=" + SCOPE_ID + " to restore legacy id-only credential matching."); return null; @@ -147,8 +150,10 @@ public Authentication getAuthentication(RemoteRepository repository) { origin, "Not using credentials of server '" + id + "' for repository " + repository.getUrl() + ": no repository or mirror with this id is declared in settings or on the command" - + " line, and " + DefaultRepositorySystemSessionFactory.MAVEN_REPOSITORY_CREDENTIAL_SCOPE - + "=" + SCOPE_STRICT + " is in effect."); + + " line and the server declares no , and " + + DefaultRepositorySystemSessionFactory.MAVEN_REPOSITORY_CREDENTIAL_SCOPE + + "=" + SCOPE_STRICT + " is in effect. Declare " + originHint(origin) + + " in of that server in settings to allow it."); return null; } warnOnce( @@ -156,12 +161,19 @@ public Authentication getAuthentication(RemoteRepository repository) { origin, "Using credentials of server '" + id + "' for repository " + repository.getUrl() + ", although no repository or mirror with this id is declared in settings or on the" - + " command line. Set " + + " command line and the server declares no . Declare " + + originHint(origin) + + " in of that server in settings to bind these credentials" + + " explicitly, or set " + DefaultRepositorySystemSessionFactory.MAVEN_REPOSITORY_CREDENTIAL_SCOPE + "=" + SCOPE_STRICT + " to refuse such credential use."); return auth; } + private static String originHint(String origin) { + return origin != null ? "'" + origin + "'" : "its origin"; + } + private void warnOnce(String id, String origin, String message) { if (reported.add(id + "->" + origin)) { logger.warn(message); diff --git a/impl/maven-core/src/test/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactoryTest.java b/impl/maven-core/src/test/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactoryTest.java index c6d30a13e562..aae3f8c5a352 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactoryTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactoryTest.java @@ -40,12 +40,15 @@ import org.apache.maven.impl.InternalSession; import org.apache.maven.internal.impl.DefaultTypeRegistry; import org.apache.maven.rtinfo.RuntimeInformation; +import org.apache.maven.settings.Mirror; import org.apache.maven.settings.Server; import org.codehaus.plexus.configuration.PlexusConfiguration; import org.codehaus.plexus.testing.PlexusTest; import org.codehaus.plexus.util.xml.Xpp3Dom; import org.eclipse.aether.ConfigurationProperties; import org.eclipse.aether.collection.VersionFilterBuilder; +import org.eclipse.aether.repository.AuthenticationSelector; +import org.eclipse.aether.repository.RemoteRepository; import org.eclipse.aether.repository.RepositoryPolicy; import org.eclipse.aether.version.VersionScheme; import org.junit.jupiter.api.Test; @@ -437,6 +440,105 @@ void transportConfigurationTest() throws InvalidRepositoryException { properties.remove("maven.resolver.transport"); } + @Test + void credentialsServedForOriginDeclaredOnlyByServer() throws InvalidRepositoryException { + MavenExecutionRequest request = requestWithServer( + serverWithRepositoryOrigins("internal", "https://repo.example.org", "https://mirror.example.org:8443")); + + AuthenticationSelector selector = authenticationSelector(request); + + assertNotNull(selector.getAuthentication(repository("internal", "https://repo.example.org/releases/"))); + assertNotNull(selector.getAuthentication(repository("internal", "https://mirror.example.org:8443/repo/"))); + assertNull(selector.getAuthentication(repository("internal", "https://evil.example.org/releases/"))); + } + + @Test + void credentialsServedForOriginDeclaredOnlyByServerInStrictScope() throws InvalidRepositoryException { + MavenExecutionRequest request = + requestWithServer(serverWithRepositoryOrigins("internal", "https://repo.example.org")); + Properties properties = new Properties(); + properties.put(DefaultRepositorySystemSessionFactory.MAVEN_REPOSITORY_CREDENTIAL_SCOPE, "strict"); + request.setSystemProperties(properties); + + AuthenticationSelector selector = authenticationSelector(request); + + assertNotNull(selector.getAuthentication(repository("internal", "https://repo.example.org/releases/"))); + assertNull(selector.getAuthentication(repository("internal", "https://evil.example.org/releases/"))); + } + + @Test + void serverWithoutRepositoryOriginsIsRefusedInStrictScope() throws InvalidRepositoryException { + MavenExecutionRequest request = requestWithServer(serverWithRepositoryOrigins("internal")); + Properties properties = new Properties(); + properties.put(DefaultRepositorySystemSessionFactory.MAVEN_REPOSITORY_CREDENTIAL_SCOPE, "strict"); + request.setSystemProperties(properties); + + AuthenticationSelector selector = authenticationSelector(request); + + assertNull(selector.getAuthentication(repository("internal", "https://repo.example.org/releases/"))); + } + + @Test + void serverRepositoryOriginsAddToMirrorOrigins() throws InvalidRepositoryException { + MavenExecutionRequest request = + requestWithServer(serverWithRepositoryOrigins("internal", "https://repo.example.org")); + Mirror mirror = new Mirror(); + mirror.setId("internal"); + mirror.setUrl("https://mirror.example.org/repo/"); + mirror.setMirrorOf("*"); + request.setMirrors(new ArrayList<>(List.of(mirror))); + + AuthenticationSelector selector = authenticationSelector(request); + + assertNotNull(selector.getAuthentication(repository("internal", "https://mirror.example.org/repo/"))); + assertNotNull(selector.getAuthentication(repository("internal", "https://repo.example.org/releases/"))); + assertNull(selector.getAuthentication(repository("internal", "https://evil.example.org/releases/"))); + } + + @Test + void malformedServerRepositoryOriginIsIgnored() throws InvalidRepositoryException { + MavenExecutionRequest request = requestWithServer(serverWithRepositoryOrigins("internal", "not an origin")); + Properties properties = new Properties(); + properties.put(DefaultRepositorySystemSessionFactory.MAVEN_REPOSITORY_CREDENTIAL_SCOPE, "strict"); + request.setSystemProperties(properties); + + AuthenticationSelector selector = authenticationSelector(request); + + assertNull(selector.getAuthentication(repository("internal", "https://repo.example.org/releases/"))); + } + + private static Server serverWithRepositoryOrigins(String id, String... repositoryOrigins) { + Server server = new Server(); + server.setId(id); + server.setUsername("jason"); + server.setPassword("abc123"); + server.setRepositoryOrigins(List.of(repositoryOrigins)); + return server; + } + + private MavenExecutionRequest requestWithServer(Server server) throws InvalidRepositoryException { + MavenExecutionRequest request = new DefaultMavenExecutionRequest(); + request.setLocalRepository(getLocalRepository()); + request.setServers(new ArrayList<>(List.of(server))); + return request; + } + + private AuthenticationSelector authenticationSelector(MavenExecutionRequest request) { + DefaultRepositorySystemSessionFactory systemSessionFactory = new DefaultRepositorySystemSessionFactory( + aetherRepositorySystem, + eventSpyDispatcher, + information, + defaultTypeRegistry, + versionScheme, + Collections.emptyMap(), + versionFilterBuilder); + return systemSessionFactory.newRepositorySession(request).getAuthenticationSelector(); + } + + private static RemoteRepository repository(String id, String url) { + return new RemoteRepository.Builder(id, "default", url).build(); + } + protected ArtifactRepository getLocalRepository() throws InvalidRepositoryException { File repoDir = new File(getBasedir(), "target/local-repo").getAbsoluteFile(); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSettingsBuilder.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSettingsBuilder.java index e061346900c5..39f8d47bfc94 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSettingsBuilder.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSettingsBuilder.java @@ -228,6 +228,7 @@ private Settings readSettings( .filePermissions(null) .directoryPermissions(null) .aliases(List.of()) + .repositoryOrigins(List.of()) .build()) .toList()) .build(); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSettingsValidator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSettingsValidator.java index 4887dffe725f..f79e15c19817 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSettingsValidator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSettingsValidator.java @@ -18,6 +18,8 @@ */ package org.apache.maven.impl; +import java.net.URI; +import java.net.URISyntaxException; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -76,6 +78,10 @@ public void validate(Settings settings, boolean isProjectSettings, ProblemCollec if (!server.getAliases().isEmpty()) { addViolation(problems, BuilderProblem.Severity.WARNING, serverField + ".aliases", null, msgP); } + if (!server.getRepositoryOrigins().isEmpty()) { + addViolation( + problems, BuilderProblem.Severity.WARNING, serverField + ".repositoryOrigins", null, msgP); + } } } @@ -142,6 +148,8 @@ public void validate(Settings settings, boolean isProjectSettings, ProblemCollec "must be unique across all server ids and aliases but found duplicate alias " + alias); } } + + validateRepositoryOrigins(problems, server, i); } } @@ -289,6 +297,87 @@ private void validateRepositories( *

  • string.length == 0 * */ + private static void validateRepositoryOrigins(ProblemCollector problems, Server server, int index) { + for (int o = 0; o < server.getRepositoryOrigins().size(); o++) { + String repositoryOrigin = server.getRepositoryOrigins().get(o); + String fieldName = "servers.server[" + index + "].repositoryOrigins[" + o + "]"; + + if (!validateStringNotEmpty(problems, fieldName, repositoryOrigin, server.getId())) { + continue; + } + + // settings are interpolated before they are validated, so a placeholder left here is a defect + // and would silently never match any repository + if (repositoryOrigin.contains("${")) { + addViolation( + problems, + BuilderProblem.Severity.ERROR, + fieldName, + server.getId(), + "contains an unresolved property placeholder: '" + repositoryOrigin + "'"); + continue; + } + + String invalid = invalidRepositoryOriginReason(repositoryOrigin); + if (invalid != null) { + addViolation(problems, BuilderProblem.Severity.ERROR, fieldName, server.getId(), invalid); + continue; + } + + String ignored = ignoredRepositoryOriginPartsReason(repositoryOrigin); + if (ignored != null) { + addViolation(problems, BuilderProblem.Severity.WARNING, fieldName, server.getId(), ignored); + } + } + } + + /** + * Parses a {@code } value the way the credential scoping does; see + * {@code OriginBoundAuthenticationSelector#originOf(String)} in maven-core, which is the authority + * on what an origin is. Returns {@code null} when the value is not a URI at all. + */ + private static URI parseRepositoryOrigin(String value) { + try { + return new URI(value).parseServerAuthority(); + } catch (URISyntaxException e) { + return null; + } + } + + /** + * @return the reason why no origin can be derived from the given value, or {@code null} if one can + */ + private static String invalidRepositoryOriginReason(String value) { + URI uri = parseRepositoryOrigin(value); + if (uri == null) { + return "must be a repository origin of the form scheme://host[:port] but found '" + value + "'"; + } + if (uri.getScheme() == null) { + return "must start with a scheme, for example https://repo.example.org, but found '" + value + "'"; + } + if (uri.getHost() == null) { + return "must name a host, for example https://repo.example.org, but found '" + value + "'"; + } + if (uri.getUserInfo() != null) { + return "must not carry user information but found '" + value + "'"; + } + return null; + } + + /** + * @return the reason why parts of the given value are ignored, or {@code null} if it is a bare origin + */ + private static String ignoredRepositoryOriginPartsReason(String value) { + URI uri = parseRepositoryOrigin(value); + String path = uri.getRawPath(); + boolean extraPath = path != null && !path.isEmpty() && !"/".equals(path); + if (!extraPath && uri.getRawQuery() == null && uri.getRawFragment() == null) { + return null; + } + return "is a repository origin, not a repository URL; only '" + uri.getScheme() + "://" + uri.getAuthority() + + "' of '" + value + "' is used"; + } + private static boolean validateStringEmpty( ProblemCollector problems, String fieldName, String string, String message) { if (string == null || string.isEmpty()) { diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsBuilderFactoryTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsBuilderFactoryTest.java index f4a9fef36553..16210f65f46d 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsBuilderFactoryTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsBuilderFactoryTest.java @@ -161,6 +161,23 @@ void testSettingsWithDuplicateServersIds() throws Exception { problems.problems().findFirst().orElseThrow().getMessage()); } + @Test + void testSettingsWithServerRepositoryOrigins() { + Settings settings = execute("settings-servers-4").getEffectiveSettings(); + + List servers = settings.getServers(); + assertEquals(2, servers.size()); + + List repositoryOrigins = List.of("https://repo.example.org", "https://mirror.example.org:8443"); + + Server server1 = getServerById(servers, "server-1"); + assertEquals(repositoryOrigins, server1.getRepositoryOrigins()); + + // an alias is the same credentials under another id, so it is bound to the same origins + Server server11 = getServerById(servers, "server-11"); + assertEquals(repositoryOrigins, server11.getRepositoryOrigins()); + } + @Test void testRelativeLocalRepositoryIsResolvedToAbsolute() { Settings settings = execute("settings-relative-local-repo").getEffectiveSettings(); diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsParserTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsParserTest.java index 0fd89f4c4935..ecfe6a21f947 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsParserTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsParserTest.java @@ -410,6 +410,34 @@ void projectSettingsRestrictionsApplyToCustomParser() throws Exception { assertTrue(result.getProblems().hasWarningProblems()); } + @Test + void projectSettingsCannotWidenServerCredentialOrigins() throws Exception { + var result = builder(Map.of()) + .build(SettingsBuilderRequest.builder() + .session(mock(Session.class)) + .userSettingsSource(source( + "user.xml", + "repository" + + "user" + + "https://good.example.org" + + "")) + .projectSettingsSource(source( + "project.xml", + "repository" + + "https://evil.example.org" + + "")) + .build()); + + // a server of the project settings is kept as a separate entry of the same id, so the origins of + // every entry matter: none of them may come from the project + List repositoryOrigins = result.getEffectiveSettings().getServers().stream() + .filter(server -> "repository".equals(server.getId())) + .flatMap(server -> server.getRepositoryOrigins().stream()) + .toList(); + assertEquals(List.of("https://good.example.org"), repositoryOrigins); + assertTrue(result.getProblems().hasWarningProblems()); + } + @Test void customSettingsDecryptionFailureDoesNotExposeCredentials() throws Exception { String encrypted = "{L6L/HbmrY+cH+sNkphn-corrupted-q3fguYepTpM04WlIXb8nB1pk=}"; diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsValidatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsValidatorTest.java index 46ae074de0eb..aaee1eecad3e 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsValidatorTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsValidatorTest.java @@ -117,4 +117,110 @@ void testValidateServerIdAliasesWithEmptyValue() { "'servers.server[0].aliases[0]' for server-1 is missing", problems.problems().findFirst().orElseThrow().getMessage()); } + + @Test + void testValidateServerRepositoryOrigins() { + Server server = Server.newBuilder() + .id("server-1") + .repositoryOrigins(List.of( + "https://repo.example.org", + "https://mirror.example.org:8443", + "HTTP://Repo.Example.Org:80", + "https://repo.example.org/")) + .build(); + + Settings settings = Settings.newBuilder().servers(List.of(server)).build(); + + ProblemCollector problems = validator.validate(settings); + assertEquals(0, problems.totalProblemsReported()); + } + + @Test + void testValidateServerRepositoryOriginWithoutScheme() { + ProblemCollector problems = validateRepositoryOrigin("repo.example.org"); + assertEquals(1, problems.totalProblemsReported()); + assertEquals( + "'servers.server[0].repositoryOrigins[0]' for server-1 must start with a scheme," + + " for example https://repo.example.org, but found 'repo.example.org'", + problems.problems().findFirst().orElseThrow().getMessage()); + } + + @Test + void testValidateServerRepositoryOriginWithoutHost() { + ProblemCollector problems = validateRepositoryOrigin("file:/tmp/repo"); + assertEquals(1, problems.totalProblemsReported()); + assertEquals( + "'servers.server[0].repositoryOrigins[0]' for server-1 must name a host," + + " for example https://repo.example.org, but found 'file:/tmp/repo'", + problems.problems().findFirst().orElseThrow().getMessage()); + } + + @Test + void testValidateServerRepositoryOriginWithUserInfo() { + ProblemCollector problems = validateRepositoryOrigin("https://user:pwd@repo.example.org"); + assertEquals(1, problems.totalProblemsReported()); + assertEquals( + "'servers.server[0].repositoryOrigins[0]' for server-1 must not carry user information" + + " but found 'https://user:pwd@repo.example.org'", + problems.problems().findFirst().orElseThrow().getMessage()); + } + + @Test + void testValidateServerRepositoryOriginWithPlaceholder() { + ProblemCollector problems = validateRepositoryOrigin("${env.REPO_URL}"); + assertEquals(1, problems.totalProblemsReported()); + assertEquals( + "'servers.server[0].repositoryOrigins[0]' for server-1 contains an unresolved property" + + " placeholder: '${env.REPO_URL}'", + problems.problems().findFirst().orElseThrow().getMessage()); + } + + @Test + void testValidateServerRepositoryOriginEmpty() { + ProblemCollector problems = validateRepositoryOrigin(""); + assertEquals(1, problems.totalProblemsReported()); + assertEquals( + "'servers.server[0].repositoryOrigins[0]' for server-1 is missing", + problems.problems().findFirst().orElseThrow().getMessage()); + } + + @Test + void testValidateServerRepositoryOriginWithPathIsOnlyWarned() { + ProblemCollector problems = validateRepositoryOrigin("https://repo.example.org/releases/"); + assertEquals(1, problems.totalProblemsReported()); + BuilderProblem problem = problems.problems().findFirst().orElseThrow(); + assertEquals(BuilderProblem.Severity.WARNING, problem.getSeverity()); + assertEquals( + "'servers.server[0].repositoryOrigins[0]' for server-1 is a repository origin," + + " not a repository URL; only 'https://repo.example.org' of" + + " 'https://repo.example.org/releases/' is used", + problem.getMessage()); + } + + @Test + void testValidateServerRepositoryOriginsOnProjectSettings() { + Server server = Server.newBuilder() + .id("server-1") + .repositoryOrigins(List.of("https://repo.example.org")) + .build(); + + Settings settings = Settings.newBuilder().servers(List.of(server)).build(); + + ProblemCollector problems = validator.validate(settings, true); + assertEquals(1, problems.totalProblemsReported()); + assertEquals( + "'servers.server[0].repositoryOrigins' are not supported on project settings.", + problems.problems().findFirst().orElseThrow().getMessage()); + } + + private ProblemCollector validateRepositoryOrigin(String repositoryOrigin) { + Server server = Server.newBuilder() + .id("server-1") + .repositoryOrigins(List.of(repositoryOrigin)) + .build(); + + Settings settings = Settings.newBuilder().servers(List.of(server)).build(); + + return validator.validate(settings); + } } diff --git a/impl/maven-impl/src/test/resources/settings/settings-servers-4.xml b/impl/maven-impl/src/test/resources/settings/settings-servers-4.xml new file mode 100644 index 000000000000..4c3923b1ef93 --- /dev/null +++ b/impl/maven-impl/src/test/resources/settings/settings-servers-4.xml @@ -0,0 +1,38 @@ + + + + + + + + server-1 + username1 + password1 + + server-11 + + + https://repo.example.org + https://mirror.example.org:8443 + + + + +