SshScheme: Provide an option to omit usernames in SSH URLs

Including the username in the download URL can prevent IT admins from
configuring hosts to redirect read-only traffic to regional replicas
when the admins rely on the git url.insteadOf configuration. This is
because git doesn't support anything except simple replacement of
matching strings, so there's no way to account for variable usernames in
a host-wide /etc/gitconfig file.

The default stays as always including the username in the SSH download
commands.

Release-Notes: Added an option to omit SSH usernames from download commands
Change-Id: I49f3b85adc3dae2d440366c919fb656ca95b812f
diff --git a/src/main/java/com/googlesource/gerrit/plugins/download/scheme/SshScheme.java b/src/main/java/com/googlesource/gerrit/plugins/download/scheme/SshScheme.java
index 6e23ef9..cb41f9b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/download/scheme/SshScheme.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/download/scheme/SshScheme.java
@@ -44,6 +44,7 @@
   private final Provider<CurrentUser> userProvider;
   private final boolean schemeAllowed;
   private final boolean schemeHidden;
+  private final boolean includeUserName;
 
   @Inject
   @VisibleForTesting
@@ -101,6 +102,7 @@
     this.userProvider = userProvider;
     this.schemeAllowed = downloadConfig.getDownloadSchemes().contains(SSH);
     this.schemeHidden = downloadConfig.getHiddenSchemes().contains(SSH);
+    this.includeUserName = config.getBoolean("sshIncludeUserName", true);
   }
 
   @Nullable
@@ -127,12 +129,16 @@
 
     StringBuilder r = new StringBuilder();
     r.append("ssh://");
-    try {
-      r.append(URLEncoder.encode(username.get(), StandardCharsets.UTF_8.name()));
-    } catch (UnsupportedEncodingException e) {
-      throw new IllegalStateException("No UTF-8 support", e);
+
+    if (includeUserName) {
+      try {
+        r.append(URLEncoder.encode(username.get(), StandardCharsets.UTF_8.name()));
+      } catch (UnsupportedEncodingException e) {
+        throw new IllegalStateException("No UTF-8 support", e);
+      }
+      r.append("@");
     }
-    r.append("@");
+
     r.append(ensureSlash(address));
     r.append(project);
     return r.toString();
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 3386747..5580f87 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -144,6 +144,7 @@
 ```
 [plugin "@PLUGIN@"]
 	sshdAdvertisedPrimaryAddress = host:port
+	sshIncludeUserName = true
 ```
 
 <a id="plugin.@PLUGIN@.sshdAdvertisedPrimaryAddress">plugin.@PLUGIN@.sshdAdvertisedPrimaryAddress</a>
@@ -164,3 +165,7 @@
 * `['IPv6']:'port'` (for example `[ff02::1]:29418`)
 
 By default unset.
+
+<a id="plugin.@PLUGIN@.sshIncludeUserName">plugin.@PLUGIN@.sshIncludeUserName</a>
+Whether the SSH scheme's download commands should include the current user's username
+or not. By default `true`.
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 c39bf25..cc10b95 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/download/DownloadCommandTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/download/DownloadCommandTest.java
@@ -81,6 +81,7 @@
     }
   }
 
+  protected static final String PLUGIN_NAME = "download-commands";
   protected static TestEnvironment ENV = new TestEnvironment();
 
   protected HttpScheme httpScheme;
@@ -94,11 +95,9 @@
 
   @Before
   public void setUp() {
-    final String pluginName = "download-commands";
-
     PluginConfigFactory configFactory = Mockito.mock(PluginConfigFactory.class);
-    Mockito.when(configFactory.getFromGerritConfig(pluginName))
-        .thenReturn(PluginConfig.createFromGerritConfig(pluginName, new Config()));
+    Mockito.when(configFactory.getFromGerritConfig(PLUGIN_NAME))
+        .thenReturn(PluginConfig.createFromGerritConfig(PLUGIN_NAME, new Config()));
 
     urlProvider = Providers.of(ENV.canonicalUrl());
 
@@ -111,7 +110,7 @@
     sshScheme =
         new SshScheme(
             ImmutableList.of(String.format("%s:%d", ENV.fqdn, ENV.sshPort)),
-            pluginName,
+            PLUGIN_NAME,
             configFactory,
             urlProvider,
             userProvider,
diff --git a/src/test/java/com/googlesource/gerrit/plugins/download/scheme/SchemeTest.java b/src/test/java/com/googlesource/gerrit/plugins/download/scheme/SchemeTest.java
index eaae634..74e57b5 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/download/scheme/SchemeTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/download/scheme/SchemeTest.java
@@ -15,11 +15,24 @@
 package com.googlesource.gerrit.plugins.download.scheme;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.when;
 
+import com.google.common.collect.ImmutableList;
+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 org.eclipse.jgit.lib.Config;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
 
+@RunWith(MockitoJUnitRunner.class)
 public class SchemeTest extends DownloadCommandTest {
+  @Mock
+  private PluginConfigFactory pluginConfigFactoryMock;
+
   @Test
   public void ensureHttpSchemeEncodedInUrl() throws Exception {
     assertThat(httpScheme.getUrl(ENV.projectName))
@@ -29,11 +42,33 @@
   }
 
   @Test
-  public void ensureSshSchemeEncodedInUrl() throws Exception {
+  public void ensureSshSchemeEncodedInUrlWithUserName() throws Exception {
     assertThat(sshScheme.getUrl(ENV.projectName))
         .isEqualTo(
             String.format(
                 "ssh://%s@%s:%d/%s",
                 ENV.urlEncodedUserName(), ENV.fqdn, ENV.sshPort, ENV.projectName));
   }
+
+  @Test
+  public void ensureSshSchemeNameEncodedInUrlWithoutUserName() {
+    assertThat(getSshSchemeUrlWithoutUserName())
+        .isEqualTo(String.format("ssh://%s:%d/%s", ENV.fqdn, ENV.sshPort, ENV.projectName));
+  }
+
+  private String getSshSchemeUrlWithoutUserName() {
+    PluginConfig.Update pluginConfig = PluginConfig.Update.forTest(PLUGIN_NAME, new Config());
+    pluginConfig.setBoolean("sshIncludeUserName", false);
+    when(pluginConfigFactoryMock.getFromGerritConfig(PLUGIN_NAME))
+        .thenReturn(pluginConfig.asPluginConfig());
+    SshScheme scheme =
+        new SshScheme(
+            ImmutableList.of(String.format("%s:%d", ENV.fqdn, ENV.sshPort)),
+            PLUGIN_NAME,
+            pluginConfigFactoryMock,
+            urlProvider,
+            userProvider,
+            new DownloadConfig(new Config()));
+    return scheme.getUrl(ENV.projectName);
+  }
 }