Fix CredentialsFactory and CredentialsProvider abstraction leak
The CredentialsFactory returned SecureCredentialsProvider implementation
class instead of the abstract interface CredentialsProvider.
The SecureCredentialsProvider exposed its getUser and getPass methods
which were used by the replication plugin and thus hardcoded
username/password authentication for the REST API based admin
operations.
Fix the CredentialsFactory to create CredentialsProvider. Remove the
getUser and getPass methods from the SecureCredentialsProvider. Use the
abstract CredentialItem to get username and password from the
CredentialsProvider.
Change-Id: I8b6ef89694db4b7fd6e3116e69f562133f6ad5e8
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java
index 29a7ee6..98f364d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java
@@ -23,6 +23,7 @@
import java.nio.file.Files;
import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.transport.CredentialsProvider;
public class AutoReloadSecureCredentialsFactoryDecorator implements CredentialsFactory {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -50,7 +51,7 @@
}
@Override
- public SecureCredentialsProvider create(String remoteName) {
+ public CredentialsProvider create(String remoteName) {
try {
if (needsReload()) {
secureCredentialsFactory.compareAndSet(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/CredentialsFactory.java b/src/main/java/com/googlesource/gerrit/plugins/replication/CredentialsFactory.java
index 10719c1..3bb64ab 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/CredentialsFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/CredentialsFactory.java
@@ -13,7 +13,9 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.replication;
+import org.eclipse.jgit.transport.CredentialsProvider;
+
public interface CredentialsFactory {
- SecureCredentialsProvider create(String remoteName);
+ CredentialsProvider create(String remoteName);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/GerritRestApi.java b/src/main/java/com/googlesource/gerrit/plugins/replication/GerritRestApi.java
index cb05bca..91fe20a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/GerritRestApi.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/GerritRestApi.java
@@ -33,6 +33,7 @@
import org.apache.http.impl.client.BasicCredentialsProvider;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.message.BasicHeader;
+import org.eclipse.jgit.transport.CredentialItem;
import org.eclipse.jgit.transport.RemoteConfig;
import org.eclipse.jgit.transport.URIish;
@@ -101,13 +102,23 @@
private HttpClientContext getContext() {
HttpClientContext ctx = HttpClientContext.create();
- CredentialsProvider cp = new BasicCredentialsProvider();
- SecureCredentialsProvider scp = credentials.create(remoteConfig.getName());
- cp.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(scp.getUser(), scp.getPass()));
- ctx.setCredentialsProvider(cp);
+ ctx.setCredentialsProvider(adapt(credentials.create(remoteConfig.getName())));
return ctx;
}
+ private CredentialsProvider adapt(org.eclipse.jgit.transport.CredentialsProvider cp) {
+ CredentialItem.Username user = new CredentialItem.Username();
+ CredentialItem.Password pass = new CredentialItem.Password();
+ if (cp.supports(user, pass) && cp.get(uri, user, pass)) {
+ CredentialsProvider adapted = new BasicCredentialsProvider();
+ adapted.setCredentials(
+ AuthScope.ANY,
+ new UsernamePasswordCredentials(user.getValue(), new String(pass.getValue())));
+ return adapted;
+ }
+ return null;
+ }
+
private static String toHttpUri(URIish uri) {
String u = uri.toString();
if (u.startsWith(GERRIT_ADMIN_PROTOCOL_PREFIX)) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsProvider.java b/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsProvider.java
index 691cc4e..c4294a9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsProvider.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsProvider.java
@@ -77,12 +77,4 @@
}
return true;
}
-
- public String getUser() {
- return cfgUser;
- }
-
- public String getPass() {
- return cfgPass;
- }
}
