Fix updating replication credentials using te RepicationRemotesApi

The implementation of the ReplicationRemotesApi for updating a remote
introduced with I8003ec6c827 has wrongly saved the credentials (username)
in the repication.config rather than in the SecureStore, causing
the generation of non-functioning replication configs.

The credentials are read from the secure.config (or other SecureStore)
as documented in [1].

[1] https://gerrit.googlesource.com/plugins/replication/+/refs/heads/master/src/main/resources/Documentation/config.md#file-2

Change-Id: I644d73bc69589189a8762de98d1c58c263abcfd4

src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java

diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java
index e4ef318..cc6bcad 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java
@@ -59,19 +59,19 @@
           "configuration update must have at least one 'remote' section");
     }
 
-    SeparatedRemoteConfigs configs = onlyRemoteSectionsWithSeparatedPasswords(remoteConfig);
-    persistRemotesPasswords(configs);
+    SeparatedRemoteConfigs configs = onlyRemoteSectionsWithSeparatedCredentials(remoteConfig);
+    persistRemotesCredentials(configs);
 
     mergedConfigResource.update(configs.remotes);
   }
 
-  private SeparatedRemoteConfigs onlyRemoteSectionsWithSeparatedPasswords(Config configUpdates) {
+  private SeparatedRemoteConfigs onlyRemoteSectionsWithSeparatedCredentials(Config configUpdates) {
     SeparatedRemoteConfigs configs = new SeparatedRemoteConfigs();
     for (String subSection : configUpdates.getSubsections("remote")) {
       for (String name : configUpdates.getNames("remote", subSection)) {
         List<String> values = List.of(configUpdates.getStringList("remote", subSection, name));
-        if ("password".equals(name)) {
-          configs.passwords.setStringList("remote", subSection, "password", values);
+        if ("password".equals(name) || "username".equals(name)) {
+          configs.credentials.setStringList("remote", subSection, name, values);
         } else {
           configs.remotes.setStringList("remote", subSection, name, values);
         }
@@ -81,16 +81,20 @@
     return configs;
   }
 
-  private void persistRemotesPasswords(SeparatedRemoteConfigs configs) {
-    for (String subSection : configs.passwords.getSubsections("remote")) {
-      List<String> values =
-          List.of(configs.passwords.getStringList("remote", subSection, "password"));
-      secureStore.setList("remote", subSection, "password", values);
+  private void persistRemotesCredentials(SeparatedRemoteConfigs configs) {
+    for (String subSection : configs.credentials.getSubsections("remote")) {
+      copyRemoteStringList(configs.credentials, subSection, "username");
+      copyRemoteStringList(configs.credentials, subSection, "password");
     }
   }
 
+  private void copyRemoteStringList(Config config, String subSection, String key) {
+    secureStore.setList(
+        "remote", subSection, key, List.of(config.getStringList("remote", subSection, key)));
+  }
+
   private static class SeparatedRemoteConfigs {
     private final Config remotes = new Config();
-    private final Config passwords = new Config();
+    private final Config credentials = new Config();
   }
 }

src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java

diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java
index a806875..40d9846 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java
@@ -148,9 +148,9 @@
   }
 
   @Test
-  public void shouldEncryptPasswordButNotStoreInConfig() throws Exception {
+  public void shouldSetPasswordViaSecureStoreButNotStoreInConfig() throws Exception {
     Config update = new Config();
-    String password = "my_secret_password";
+    String password = "encrypted-password";
     String remoteName = "site";
     setRemoteSite(update, remoteName, "password", password);
     ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi();
@@ -163,6 +163,22 @@
     assertRemoteSite(objectUnderTest.get(remoteName), remoteName, "password").isNull();
   }
 
+  @Test
+  public void shouldSetUsernameViaSecureStoreButNotStoreInConfig() throws Exception {
+    Config update = new Config();
+    String username = "encrypted-username";
+    String remoteName = "site";
+    setRemoteSite(update, remoteName, "username", username);
+    ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi();
+
+    objectUnderTest.update(update);
+
+    verify(secureStoreMock).setList("remote", remoteName, "username", List.of(username));
+    assertRemoteSite(baseConfig.getConfig(), remoteName, "username").isNull();
+    assertRemoteSite(testOverrides.get().getConfig(), remoteName, "username").isNull();
+    assertRemoteSite(objectUnderTest.get(remoteName), remoteName, "username").isNull();
+  }
+
   private void setRemoteSite(Config config, String remoteName, String name, String value) {
     config.setString("remote", remoteName, name, value);
   }