Use LDAP username in HTTP URLs when gitBasicAuthPolicy=LDAP
When Gerrit is configured with `gitBasicAuthPolicy = LDAP`,
Git-over-HTTP authentication is performed directly against the LDAP
backend.
In this mode, the LDAP login name (not the Gerrit username) is the
credential that must be used for HTTP BasicAuth to succeed.
However, the download-commands plugin previously embedded the Gerrit
username in generated HTTP clone URLs, regardless of authentication
policy.
This change updates `HttpScheme` to select the LDAP login name (the
`gerrit:` external ID) whenever `gitBasicAuthPolicy` is set to `LDAP`.
If no LDAP username exists, or LDAP authentication is not active, the
existing behavior is preserved and the Gerrit username continues to be
used.
This ensures that download commands always embed a username that can
actually authenticate under the configured Git-over-HTTP authentication
policy.
Bug: Issue 466122136
Change-Id: I267b9ca01045056e540bd928d649321bb6c9ddc1
diff --git a/src/main/java/com/googlesource/gerrit/plugins/download/scheme/HttpScheme.java b/src/main/java/com/googlesource/gerrit/plugins/download/scheme/HttpScheme.java
index 923470d..8ef7957 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/download/scheme/HttpScheme.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/download/scheme/HttpScheme.java
@@ -15,10 +15,16 @@
package com.googlesource.gerrit.plugins.download.scheme;
import static com.google.gerrit.entities.CoreDownloadSchemes.HTTP;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GERRIT;
+import static java.util.stream.Collectors.toList;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.client.GitBasicAuthPolicy;
import com.google.gerrit.extensions.config.DownloadScheme;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.DownloadConfig;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -27,27 +33,33 @@
import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Optional;
import org.eclipse.jgit.lib.Config;
public class HttpScheme extends DownloadScheme {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final String gitHttpUrl;
private final String canonicalWebUrl;
private final Provider<CurrentUser> userProvider;
private final boolean schemeAllowed;
private final boolean schemeHidden;
+ private final AuthConfig authConfig;
@Inject
public HttpScheme(
@GerritServerConfig Config cfg,
@CanonicalWebUrl @Nullable Provider<String> urlProvider,
Provider<CurrentUser> userProvider,
- DownloadConfig downloadConfig) {
+ DownloadConfig downloadConfig,
+ AuthConfig authConfig) {
this.gitHttpUrl = ensureSlash(cfg.getString("gerrit", null, "gitHttpUrl"));
this.canonicalWebUrl = urlProvider != null ? urlProvider.get() : null;
this.userProvider = userProvider;
this.schemeAllowed = downloadConfig.getDownloadSchemes().contains(HTTP);
this.schemeHidden = downloadConfig.getHiddenSchemes().contains(HTTP);
+ this.authConfig = authConfig;
}
@Nullable
@@ -69,16 +81,16 @@
}
String host = base.substring(p + 3, s);
r.append(base.substring(0, p + 3));
- if (userProvider.get().getUserName().isPresent()) {
- try {
- r.append(
- URLEncoder.encode(
- userProvider.get().getUserName().get(), StandardCharsets.UTF_8.name()));
- } catch (UnsupportedEncodingException e) {
- throw new IllegalStateException("No UTF-8 support", e);
- }
- r.append("@");
- }
+ getHttpUserName()
+ .ifPresent(
+ u -> {
+ try {
+ r.append(URLEncoder.encode(u, StandardCharsets.UTF_8.name()));
+ } catch (UnsupportedEncodingException e) {
+ throw new IllegalStateException("No UTF-8 support", e);
+ }
+ r.append("@");
+ });
r.append(host);
r.append(base.substring(s));
r.append("a/");
@@ -89,6 +101,39 @@
return r.toString();
}
+ /**
+ * Returns the username to embed in HTTP URLs.
+ *
+ * <p>If {@code gitBasicAuthPolicy == LDAP}, always prefer the LDAP login name (the {@code
+ * gerrit:} external ID) when available, since this is the username guaranteed to authenticate
+ * against LDAP.
+ *
+ * <p>If no such LDAP username exists, or if the policy is not LDAP, fall back to the Gerrit
+ * username.
+ */
+ private Optional<String> getHttpUserName() {
+ CurrentUser user = userProvider.get();
+ Optional<String> gerritUserName = user.getUserName();
+
+ if (authConfig.getGitBasicAuthPolicy() != GitBasicAuthPolicy.LDAP) {
+ return gerritUserName;
+ }
+
+ List<String> ldapUserNames =
+ user.getExternalIdKeys().stream()
+ .filter(k -> k.isScheme(SCHEME_GERRIT))
+ .map(ExternalId.Key::id)
+ .collect(toList());
+
+ if (ldapUserNames.size() > 1) {
+ logger.atWarning().log(
+ "Found %d LDAP identities for user %s. Exactly one was expected.",
+ ldapUserNames.size(), user.getLoggableName());
+ }
+
+ return ldapUserNames.stream().findFirst().or(() -> gerritUserName);
+ }
+
@Override
public boolean isEnabled() {
return schemeAllowed && (gitHttpUrl != null || canonicalWebUrl != null);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/download/DownloadCommandTest.java b/src/test/java/com/googlesource/gerrit/plugins/download/DownloadCommandTest.java
index cc10b95..aeb7008 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/download/DownloadCommandTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/download/DownloadCommandTest.java
@@ -14,9 +14,13 @@
package com.googlesource.gerrit.plugins.download;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.extensions.client.GitBasicAuthPolicy;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.GroupMembership;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.DownloadConfig;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.PluginConfigFactory;
@@ -28,41 +32,56 @@
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Optional;
+import java.util.Set;
import org.eclipse.jgit.lib.Config;
import org.junit.Before;
import org.junit.Ignore;
+import org.mockito.Mock;
import org.mockito.Mockito;
@Ignore
public class DownloadCommandTest {
+ public static class TestUser extends CurrentUser {
+ private ImmutableSet<ExternalId.Key> externalIds = ImmutableSet.of();
+
+ public void setExternalIds(Set<ExternalId.Key> keys) {
+ this.externalIds = ImmutableSet.copyOf(keys);
+ }
+
+ @Override
+ public Optional<String> getUserName() {
+ return Optional.of(ENV.userName);
+ }
+
+ @Override
+ public GroupMembership getEffectiveGroups() {
+ throw new UnsupportedOperationException("not implemented");
+ }
+
+ @Override
+ public Object getCacheKey() {
+ return new Object();
+ }
+
+ @Override
+ public boolean isIdentifiedUser() {
+ return true;
+ }
+
+ @Override
+ public Account.Id getAccountId() {
+ return Account.id(1);
+ }
+
+ @Override
+ public ImmutableSet<ExternalId.Key> getExternalIdKeys() {
+ return externalIds;
+ }
+ }
+
static CurrentUser fakeUser() {
- return new CurrentUser() {
- @Override
- public Optional<String> getUserName() {
- return Optional.of(ENV.userName);
- }
-
- @Override
- public GroupMembership getEffectiveGroups() {
- throw new UnsupportedOperationException("not implemented");
- }
-
- @Override
- public Object getCacheKey() {
- return new Object();
- }
-
- @Override
- public boolean isIdentifiedUser() {
- return true;
- }
-
- @Override
- public Account.Id getAccountId() {
- return Account.id(1);
- }
- };
+ return new TestUser();
}
public static class TestEnvironment {
@@ -88,6 +107,7 @@
protected SshScheme sshScheme;
protected Provider<String> urlProvider;
protected Provider<CurrentUser> userProvider;
+ protected @Mock AuthConfig authConfig;
public DownloadCommandTest() {
super();
@@ -106,7 +126,10 @@
userProvider = Providers.of(fakeUser());
- httpScheme = new HttpScheme(cfg, urlProvider, userProvider, downloadConfig);
+ authConfig = Mockito.mock(AuthConfig.class);
+ Mockito.when(authConfig.getGitBasicAuthPolicy()).thenReturn(GitBasicAuthPolicy.HTTP);
+
+ httpScheme = new HttpScheme(cfg, urlProvider, userProvider, downloadConfig, authConfig);
sshScheme =
new SshScheme(
ImmutableList.of(String.format("%s:%d", ENV.fqdn, ENV.sshPort)),
diff --git a/src/test/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHookTest.java b/src/test/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHookTest.java
index 9762ab7..26badab 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHookTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHookTest.java
@@ -19,11 +19,15 @@
import static com.googlesource.gerrit.plugins.download.command.CloneWithCommitMsgHook.HOOK_COMMAND_KEY;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.extensions.client.GitBasicAuthPolicy;
+import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.config.DownloadConfig;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.googlesource.gerrit.plugins.download.DownloadCommandTest;
import com.googlesource.gerrit.plugins.download.scheme.SshScheme;
+import java.io.UnsupportedEncodingException;
import org.eclipse.jgit.lib.Config;
import org.junit.Test;
import org.mockito.Mockito;
@@ -161,6 +165,30 @@
extraCommand));
}
+ @Test
+ public void usesLdapUsernameWhenLdapPolicyAndLdapExternalIdPresent() {
+ DownloadCommandTest.TestUser user = (DownloadCommandTest.TestUser) userProvider.get();
+
+ String ldapUserName = "ldap_username";
+ ExternalId.Key ldapKey = ExternalId.Key.create(ExternalId.SCHEME_GERRIT, ldapUserName, true);
+ user.setExternalIds(ImmutableSet.of(ldapKey));
+ Mockito.when(authConfig.getGitBasicAuthPolicy()).thenReturn(GitBasicAuthPolicy.LDAP);
+
+ String url = httpScheme.getUrl(ENV.projectName);
+
+ assertThat(url).contains(ldapUserName + "@");
+ }
+
+ @Test
+ public void fallsBackToGerritUsernameWhenLdapPolicyButNoLdapExternalId()
+ throws UnsupportedEncodingException {
+ Mockito.when(authConfig.getGitBasicAuthPolicy()).thenReturn(GitBasicAuthPolicy.LDAP);
+
+ String url = httpScheme.getUrl(ENV.projectName);
+
+ assertThat(url).contains(ENV.urlEncodedUserName() + "@");
+ }
+
private String baseName(String projectName) {
return projectName.substring(projectName.lastIndexOf('/') + 1);
}
