Use PullReplicationFilter in Gerrit primary setup
The use of the Gerrit REST-API framework for the pull-replication
APIs is problematic because it relies on the underlying project
status and ACLs.
When performing replication, we need to allow the pull replication
user to access the API regardless of the current status or ACLs of
the project, hence the standard REST-API are not suitable.
The PullReplicationFilter was already implemented for the Gerrit
replicas, therefore it is already suitable for use with the
primary as well, after being made independent from the project
resource parsing.
Remove in the documentation the note about being owner of the
project or administrator for replicating hidden projects, because
the adoption of the PullReplicationFilter would bypass the ACLs
altogether when resolving the project URL component.
Change-Id: I0c0217985f67098f913d496a57f68b44932d951e
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationModule.java
index d1c5ca1..03345e2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationModule.java
@@ -15,6 +15,7 @@
package com.googlesource.gerrit.plugins.replication.pull;
import static com.googlesource.gerrit.plugins.replication.StartReplicationCapability.START_REPLICATION;
+import static com.googlesource.gerrit.plugins.replication.pull.api.FetchApiCapability.CALL_FETCH_ACTION;
import com.google.common.eventbus.EventBus;
import com.google.gerrit.extensions.annotations.Exports;
@@ -42,8 +43,8 @@
import com.googlesource.gerrit.plugins.replication.ReplicationConfig;
import com.googlesource.gerrit.plugins.replication.ReplicationFileBasedConfig;
import com.googlesource.gerrit.plugins.replication.StartReplicationCapability;
+import com.googlesource.gerrit.plugins.replication.pull.api.FetchApiCapability;
import com.googlesource.gerrit.plugins.replication.pull.api.FetchJob;
-import com.googlesource.gerrit.plugins.replication.pull.api.PullReplicationApiModule;
import com.googlesource.gerrit.plugins.replication.pull.auth.PullReplicationGroupModule;
import com.googlesource.gerrit.plugins.replication.pull.client.FetchApiClient;
import com.googlesource.gerrit.plugins.replication.pull.client.FetchRestApiClient;
@@ -75,13 +76,16 @@
@Override
protected void configure() {
+ bind(CapabilityDefinition.class)
+ .annotatedWith(Exports.named(CALL_FETCH_ACTION))
+ .to(FetchApiCapability.class);
+
install(new PullReplicationGroupModule());
bind(BearerTokenProvider.class).in(Scopes.SINGLETON);
bind(RevisionReader.class).in(Scopes.SINGLETON);
bind(ApplyObject.class);
install(new FactoryModuleBuilder().build(FetchJob.Factory.class));
install(new ApplyObjectCacheModule());
- install(new PullReplicationApiModule());
install(new FetchRefReplicatedEventModule());
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectAction.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectAction.java
index 72f0266..d535ac9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectAction.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectAction.java
@@ -26,6 +26,7 @@
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.project.ProjectResource;
import com.google.inject.Inject;
+import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionInput;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingParentObjectException;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.RefUpdateException;
@@ -33,6 +34,7 @@
import java.util.Objects;
import javax.servlet.http.HttpServletResponse;
+@Singleton
public class ApplyObjectAction implements RestModifyView<ProjectResource, RevisionInput> {
private final ApplyObjectCommand applyObjectCommand;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchAction.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchAction.java
index fdb4f8f..9817f2c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchAction.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchAction.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.ioutil.HexFormat;
import com.google.gerrit.server.project.ProjectResource;
import com.google.inject.Inject;
+import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.replication.pull.api.FetchAction.Input;
import com.googlesource.gerrit.plugins.replication.pull.api.FetchJob.Factory;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.RemoteConfigurationMissingException;
@@ -37,6 +38,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeoutException;
+@Singleton
public class FetchAction implements RestModifyView<ProjectResource, Input> {
private final FetchCommand command;
private final WorkQueue workQueue;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchApiCapability.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchApiCapability.java
index 73a4ac5..27afcfd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchApiCapability.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchApiCapability.java
@@ -17,7 +17,7 @@
import com.google.gerrit.extensions.config.CapabilityDefinition;
public class FetchApiCapability extends CapabilityDefinition {
- static final String CALL_FETCH_ACTION = "callFetchAction";
+ public static final String CALL_FETCH_ACTION = "callFetchAction";
@Override
public String getDescription() {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchPreconditions.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchPreconditions.java
index 77d0e0b..7ca8805 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchPreconditions.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchPreconditions.java
@@ -26,8 +26,10 @@
import com.google.gerrit.server.permissions.RefPermission;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.UnauthorizedAuthException;
+@Singleton
public class FetchPreconditions {
private final String pluginName;
private final PermissionBackend permissionBackend;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/HttpModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/HttpModule.java
index 0f3e1e8..95082b8 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/HttpModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/HttpModule.java
@@ -16,7 +16,6 @@
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.httpd.AllRequestFilter;
-import com.google.gerrit.server.config.GerritIsReplica;
import com.google.inject.Inject;
import com.google.inject.Scopes;
import com.google.inject.name.Names;
@@ -24,12 +23,10 @@
import com.googlesource.gerrit.plugins.replication.pull.BearerTokenProvider;
public class HttpModule extends ServletModule {
- private boolean isReplica;
private final BearerTokenProvider bearerTokenProvider;
@Inject
- public HttpModule(@GerritIsReplica Boolean isReplica, BearerTokenProvider bearerTokenProvider) {
- this.isReplica = isReplica;
+ public HttpModule(BearerTokenProvider bearerTokenProvider) {
this.bearerTokenProvider = bearerTokenProvider;
}
@@ -49,12 +46,8 @@
.in(Scopes.SINGLETON);
});
- if (isReplica) {
- DynamicSet.bind(binder(), AllRequestFilter.class)
- .to(PullReplicationFilter.class)
- .in(Scopes.SINGLETON);
- } else {
- serveRegex("/init-project/.*$").with(ProjectInitializationAction.class);
- }
+ DynamicSet.bind(binder(), AllRequestFilter.class)
+ .to(PullReplicationFilter.class)
+ .in(Scopes.SINGLETON);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectDeletionAction.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectDeletionAction.java
index 2e1c5d4..adb333c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectDeletionAction.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectDeletionAction.java
@@ -27,11 +27,13 @@
import com.google.gerrit.server.project.ProjectResource;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.replication.LocalFS;
import com.googlesource.gerrit.plugins.replication.pull.GerritConfigOps;
import java.util.Optional;
import org.eclipse.jgit.transport.URIish;
+@Singleton
class ProjectDeletionAction
implements RestModifyView<ProjectResource, ProjectDeletionAction.DeleteInput> {
private static final PluginPermission DELETE_PROJECT =
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationApiModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationApiModule.java
deleted file mode 100644
index d1d28a6..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationApiModule.java
+++ /dev/null
@@ -1,43 +0,0 @@
-// Copyright (C) 2020 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.googlesource.gerrit.plugins.replication.pull.api;
-
-import static com.google.gerrit.server.project.ProjectResource.PROJECT_KIND;
-import static com.googlesource.gerrit.plugins.replication.pull.api.FetchApiCapability.CALL_FETCH_ACTION;
-
-import com.google.gerrit.extensions.annotations.Exports;
-import com.google.gerrit.extensions.config.CapabilityDefinition;
-import com.google.gerrit.extensions.restapi.RestApiModule;
-import com.google.inject.Scopes;
-
-public class PullReplicationApiModule extends RestApiModule {
- @Override
- protected void configure() {
- bind(FetchAction.class).in(Scopes.SINGLETON);
- bind(ApplyObjectAction.class).in(Scopes.SINGLETON);
- bind(ProjectDeletionAction.class).in(Scopes.SINGLETON);
- bind(UpdateHeadAction.class).in(Scopes.SINGLETON);
- post(PROJECT_KIND, "fetch").to(FetchAction.class);
- post(PROJECT_KIND, "apply-object").to(ApplyObjectAction.class);
- post(PROJECT_KIND, "apply-objects").to(ApplyObjectsAction.class);
- delete(PROJECT_KIND, "delete-project").to(ProjectDeletionAction.class);
- put(PROJECT_KIND, "HEAD").to(UpdateHeadAction.class);
-
- bind(FetchPreconditions.class).in(Scopes.SINGLETON);
- bind(CapabilityDefinition.class)
- .annotatedWith(Exports.named(CALL_FETCH_ACTION))
- .to(FetchApiCapability.class);
- }
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationFilter.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationFilter.java
index e54d408..ca90c02 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationFilter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationFilter.java
@@ -26,6 +26,7 @@
import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.api.projects.HeadInput;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -35,19 +36,22 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.httpd.AllRequestFilter;
import com.google.gerrit.httpd.restapi.RestApiServlet;
import com.google.gerrit.json.OutputFormat;
+import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.restapi.project.ProjectsCollection;
+import com.google.gerrit.server.project.ProjectState;
import com.google.gson.Gson;
import com.google.gson.JsonParseException;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.MalformedJsonException;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.TypeLiteral;
import com.googlesource.gerrit.plugins.replication.pull.api.FetchAction.Input;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionInput;
@@ -83,9 +87,10 @@
private ProjectInitializationAction projectInitializationAction;
private UpdateHeadAction updateHEADAction;
private ProjectDeletionAction projectDeletionAction;
- private ProjectsCollection projectsCollection;
+ private ProjectCache projectCache;
private Gson gson;
private String pluginName;
+ private final Provider<CurrentUser> currentUserProvider;
@Inject
public PullReplicationFilter(
@@ -95,17 +100,19 @@
ProjectInitializationAction projectInitializationAction,
UpdateHeadAction updateHEADAction,
ProjectDeletionAction projectDeletionAction,
- ProjectsCollection projectsCollection,
- @PluginName String pluginName) {
+ ProjectCache projectCache,
+ @PluginName String pluginName,
+ Provider<CurrentUser> currentUserProvider) {
this.fetchAction = fetchAction;
this.applyObjectAction = applyObjectAction;
this.applyObjectsAction = applyObjectsAction;
this.projectInitializationAction = projectInitializationAction;
this.updateHEADAction = updateHEADAction;
this.projectDeletionAction = projectDeletionAction;
- this.projectsCollection = projectsCollection;
+ this.projectCache = projectCache;
this.pluginName = pluginName;
this.gson = OutputFormat.JSON.newGsonBuilder().create();
+ this.currentUserProvider = currentUserProvider;
}
@Override
@@ -120,19 +127,25 @@
HttpServletRequest httpRequest = (HttpServletRequest) request;
try {
if (isFetchAction(httpRequest)) {
+ failIfcurrentUserIsAnonymous();
writeResponse(httpResponse, doFetch(httpRequest));
} else if (isApplyObjectAction(httpRequest)) {
+ failIfcurrentUserIsAnonymous();
writeResponse(httpResponse, doApplyObject(httpRequest));
} else if (isApplyObjectsAction(httpRequest)) {
+ failIfcurrentUserIsAnonymous();
writeResponse(httpResponse, doApplyObjects(httpRequest));
} else if (isInitProjectAction(httpRequest)) {
+ failIfcurrentUserIsAnonymous();
if (!checkAcceptHeader(httpRequest, httpResponse)) {
return;
}
doInitProject(httpRequest, httpResponse);
} else if (isUpdateHEADAction(httpRequest)) {
+ failIfcurrentUserIsAnonymous();
writeResponse(httpResponse, doUpdateHEAD(httpRequest));
} else if (isDeleteProjectAction(httpRequest)) {
+ failIfcurrentUserIsAnonymous();
writeResponse(httpResponse, doDeleteProject(httpRequest));
} else {
chain.doFilter(request, response);
@@ -173,6 +186,13 @@
}
}
+ private void failIfcurrentUserIsAnonymous() throws UnauthorizedAuthException {
+ CurrentUser currentUser = currentUserProvider.get();
+ if (currentUser instanceof AnonymousUser) {
+ throw new UnauthorizedAuthException();
+ }
+ }
+
private void doInitProject(HttpServletRequest httpRequest, HttpServletResponse httpResponse)
throws RestApiException, IOException, PermissionBackendException {
@@ -191,9 +211,8 @@
throws RestApiException, IOException, PermissionBackendException {
RevisionInput input = readJson(httpRequest, TypeLiteral.get(RevisionInput.class));
IdString id = getProjectName(httpRequest).get();
- ProjectResource projectResource = projectsCollection.parse(TopLevelResource.INSTANCE, id);
- return (Response<String>) applyObjectAction.apply(projectResource, input);
+ return (Response<String>) applyObjectAction.apply(parseProjectResource(id), input);
}
@SuppressWarnings("unchecked")
@@ -201,26 +220,24 @@
throws RestApiException, IOException, PermissionBackendException {
RevisionsInput input = readJson(httpRequest, TypeLiteral.get(RevisionsInput.class));
IdString id = getProjectName(httpRequest).get();
- ProjectResource projectResource = projectsCollection.parse(TopLevelResource.INSTANCE, id);
- return (Response<String>) applyObjectsAction.apply(projectResource, input);
+ return (Response<String>) applyObjectsAction.apply(parseProjectResource(id), input);
}
@SuppressWarnings("unchecked")
private Response<String> doUpdateHEAD(HttpServletRequest httpRequest) throws Exception {
HeadInput input = readJson(httpRequest, TypeLiteral.get(HeadInput.class));
IdString id = getProjectName(httpRequest).get();
- ProjectResource projectResource = projectsCollection.parse(TopLevelResource.INSTANCE, id);
- return (Response<String>) updateHEADAction.apply(projectResource, input);
+ return (Response<String>) updateHEADAction.apply(parseProjectResource(id), input);
}
@SuppressWarnings("unchecked")
private Response<String> doDeleteProject(HttpServletRequest httpRequest) throws Exception {
IdString id = getProjectName(httpRequest).get();
- ProjectResource projectResource = projectsCollection.parse(TopLevelResource.INSTANCE, id);
return (Response<String>)
- projectDeletionAction.apply(projectResource, new ProjectDeletionAction.DeleteInput());
+ projectDeletionAction.apply(
+ parseProjectResource(id), new ProjectDeletionAction.DeleteInput());
}
@SuppressWarnings("unchecked")
@@ -228,9 +245,16 @@
throws IOException, RestApiException, PermissionBackendException {
Input input = readJson(httpRequest, TypeLiteral.get(Input.class));
IdString id = getProjectName(httpRequest).get();
- ProjectResource projectResource = projectsCollection.parse(TopLevelResource.INSTANCE, id);
- return (Response<Map<String, Object>>) fetchAction.apply(projectResource, input);
+ return (Response<Map<String, Object>>) fetchAction.apply(parseProjectResource(id), input);
+ }
+
+ private ProjectResource parseProjectResource(IdString id) throws ResourceNotFoundException {
+ Optional<ProjectState> project = projectCache.get(Project.nameKey(id.get()));
+ if (project.isEmpty()) {
+ throw new ResourceNotFoundException(id);
+ }
+ return new ProjectResource(project.get(), currentUserProvider.get());
}
private <T> void writeResponse(HttpServletResponse httpResponse, Response<T> response)
diff --git a/src/main/resources/Documentation/about.md b/src/main/resources/Documentation/about.md
index 73c5c05..22b52e7 100644
--- a/src/main/resources/Documentation/about.md
+++ b/src/main/resources/Documentation/about.md
@@ -16,10 +16,6 @@
group that is granted the 'Pull Replication' capability (provided
by this plugin) or the 'Administrate Server' capability.
-When replicating hidden projects, the pull replication user needs to have
-the 'Administrate Server' capability or being added as the owner of each
-individual project that is supposed to be replicated.
-
Change Indexing
--------
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java
index b9ea0c8..4ede1ae 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java
@@ -50,7 +50,8 @@
@UseLocalDisk
@TestPlugin(
name = "pull-replication",
- sysModule = "com.googlesource.gerrit.plugins.replication.pull.PullReplicationModule")
+ sysModule = "com.googlesource.gerrit.plugins.replication.pull.PullReplicationModule",
+ httpModule = "com.googlesource.gerrit.plugins.replication.pull.api.HttpModule")
public class PullReplicationFanoutConfigIT extends LightweightPluginDaemonTest {
private static final Optional<String> ALL_PROJECTS = Optional.empty();
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectDeletionActionIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectDeletionActionIT.java
index 10415e4..cf7515e 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectDeletionActionIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectDeletionActionIT.java
@@ -94,10 +94,11 @@
@Test
@GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
@GerritConfig(name = "container.replica", value = "true")
- public void shouldReturnForbiddenForUserWithoutPermissionsOnReplica() throws Exception {
+ public void shouldReturnUnauthorizedForUserWithoutPermissionsOnReplica() throws Exception {
httpClientFactory
.create(source)
- .execute(createDeleteRequest(), assertHttpResponseCode(HttpServletResponse.SC_FORBIDDEN));
+ .execute(
+ createDeleteRequest(), assertHttpResponseCode(HttpServletResponse.SC_UNAUTHORIZED));
}
@Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectInitializationActionIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectInitializationActionIT.java
index c543969..0f13881 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectInitializationActionIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectInitializationActionIT.java
@@ -184,12 +184,13 @@
@Test
@GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
@GerritConfig(name = "container.replica", value = "true")
- public void shouldReturnForbiddenForUserWithoutPermissionsWhenNodeIsAReplica() throws Exception {
+ public void shouldReturnUnauthorizedForUserWithoutPermissionsWhenNodeIsAReplica()
+ throws Exception {
httpClientFactory
.create(source)
.execute(
createPutRequestWithHeaders(),
- assertHttpResponseCode(HttpServletResponse.SC_FORBIDDEN));
+ assertHttpResponseCode(HttpServletResponse.SC_UNAUTHORIZED));
}
@Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationFilterTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationFilterTest.java
index 9e5ca8d..5ede668 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationFilterTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/PullReplicationFilterTest.java
@@ -5,17 +5,25 @@
import static javax.servlet.http.HttpServletResponse.SC_CONFLICT;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.internal.verification.VerificationModeFactory.atLeastOnce;
import static org.mockito.internal.verification.VerificationModeFactory.times;
import com.google.common.net.MediaType;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.restapi.*;
+import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.restapi.project.ProjectsCollection;
+import com.google.gerrit.server.project.ProjectState;
+import com.google.inject.util.Providers;
import java.io.*;
import java.nio.charset.StandardCharsets;
+import java.util.Optional;
import javax.servlet.FilterChain;
import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServletRequest;
@@ -37,10 +45,12 @@
@Mock private ProjectInitializationAction projectInitializationAction;
@Mock private UpdateHeadAction updateHEADAction;
@Mock private ProjectDeletionAction projectDeletionAction;
- @Mock private ProjectsCollection projectsCollection;
- @Mock private ProjectResource projectResource;
+ @Mock private ProjectCache projectCache;
+ @Mock private ProjectState projectState;
@Mock private ServletOutputStream outputStream;
@Mock private PrintWriter printWriter;
+ @Mock private IdentifiedUser identifiedUserMock;
+ @Mock private AnonymousUser anonymousUserMock;
private final String PLUGIN_NAME = "pull-replication";
private final String PROJECT_NAME = "some-project";
private final String PROJECT_NAME_GIT = "some-project.git";
@@ -60,6 +70,10 @@
private final Response OK_RESPONSE = Response.ok();
private PullReplicationFilter createPullReplicationFilter() {
+ return createPullReplicationFilter(identifiedUserMock);
+ }
+
+ private PullReplicationFilter createPullReplicationFilter(CurrentUser currentUser) {
return new PullReplicationFilter(
fetchAction,
applyObjectAction,
@@ -67,8 +81,9 @@
projectInitializationAction,
updateHEADAction,
projectDeletionAction,
- projectsCollection,
- PLUGIN_NAME);
+ projectCache,
+ PLUGIN_NAME,
+ Providers.of(currentUser));
}
private void defineBehaviours(byte[] payload, String uri) throws Exception {
@@ -76,15 +91,14 @@
InputStream is = new ByteArrayInputStream(payload);
BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(is));
when(request.getReader()).thenReturn(bufferedReader);
- when(projectsCollection.parse(TopLevelResource.INSTANCE, IdString.fromDecoded(PROJECT_NAME)))
- .thenReturn(projectResource);
+ when(projectCache.get(Project.nameKey(PROJECT_NAME))).thenReturn(Optional.of(projectState));
when(response.getWriter()).thenReturn(printWriter);
}
private void verifyBehaviours() throws Exception {
verify(request, atLeastOnce()).getRequestURI();
verify(request).getReader();
- verify(projectsCollection).parse(TopLevelResource.INSTANCE, IdString.fromDecoded(PROJECT_NAME));
+ verify(projectCache).get(Project.nameKey(PROJECT_NAME));
verify(response).getWriter();
verify(response).setContentType("application/json");
verify(response).setStatus(HttpServletResponse.SC_OK);
@@ -107,7 +121,7 @@
pullReplicationFilter.doFilter(request, response, filterChain);
verifyBehaviours();
- verify(fetchAction).apply(eq(projectResource), any());
+ verify(fetchAction).apply(any(ProjectResource.class), any());
}
@Test
@@ -130,7 +144,7 @@
pullReplicationFilter.doFilter(request, response, filterChain);
verifyBehaviours();
- verify(applyObjectAction).apply(eq(projectResource), any());
+ verify(applyObjectAction).apply(any(ProjectResource.class), any());
}
@Test
@@ -152,7 +166,7 @@
pullReplicationFilter.doFilter(request, response, filterChain);
verifyBehaviours();
- verify(applyObjectsAction).apply(eq(projectResource), any());
+ verify(applyObjectsAction).apply(any(ProjectResource.class), any());
}
@Test
@@ -183,15 +197,14 @@
pullReplicationFilter.doFilter(request, response, filterChain);
verifyBehaviours();
- verify(updateHEADAction).apply(eq(projectResource), any());
+ verify(updateHEADAction).apply(any(ProjectResource.class), any());
}
@Test
public void shouldFilterProjectDeletionAction() throws Exception {
when(request.getRequestURI()).thenReturn(DELETE_PROJECT_URI);
when(request.getMethod()).thenReturn("DELETE");
- when(projectsCollection.parse(TopLevelResource.INSTANCE, IdString.fromDecoded(PROJECT_NAME)))
- .thenReturn(projectResource);
+ when(projectCache.get(Project.nameKey(PROJECT_NAME))).thenReturn(Optional.of(projectState));
when(projectDeletionAction.apply(any(), any())).thenReturn(OK_RESPONSE);
when(response.getWriter()).thenReturn(printWriter);
@@ -199,8 +212,8 @@
pullReplicationFilter.doFilter(request, response, filterChain);
verify(request, times(7)).getRequestURI();
- verify(projectsCollection).parse(TopLevelResource.INSTANCE, IdString.fromDecoded(PROJECT_NAME));
- verify(projectDeletionAction).apply(eq(projectResource), any());
+ verify(projectCache).get(Project.nameKey(PROJECT_NAME));
+ verify(projectDeletionAction).apply(any(ProjectResource.class), any());
verify(response).getWriter();
verify(response).setContentType("application/json");
verify(response).setStatus(OK_RESPONSE.statusCode());
@@ -215,6 +228,17 @@
}
@Test
+ public void shouldGoNextInChainWhenAnonymousRequestUriDoesNotMatch() throws Exception {
+ when(request.getRequestURI()).thenReturn("any-url");
+ lenient().when(response.getOutputStream()).thenReturn(outputStream);
+
+ final PullReplicationFilter pullReplicationFilter =
+ createPullReplicationFilter(anonymousUserMock);
+ pullReplicationFilter.doFilter(request, response, filterChain);
+ verify(filterChain).doFilter(request, response);
+ }
+
+ @Test
public void shouldBe404WhenJsonIsMalformed() throws Exception {
byte[] payloadMalformedJson = "some-json-malformed".getBytes(StandardCharsets.UTF_8);
InputStream is = new ByteArrayInputStream(payloadMalformedJson);
@@ -246,8 +270,7 @@
public void shouldBe500WhenResourceNotFound() throws Exception {
when(request.getRequestURI()).thenReturn(DELETE_PROJECT_URI);
when(request.getMethod()).thenReturn("DELETE");
- when(projectsCollection.parse(TopLevelResource.INSTANCE, IdString.fromDecoded(PROJECT_NAME)))
- .thenReturn(projectResource);
+ when(projectCache.get(Project.nameKey(PROJECT_NAME))).thenReturn(Optional.of(projectState));
when(projectDeletionAction.apply(any(), any()))
.thenThrow(new ResourceNotFoundException("resource not found"));
when(response.getOutputStream()).thenReturn(outputStream);
@@ -280,6 +303,19 @@
}
@Test
+ public void shouldBe401WhenUserIsAnonymous() throws Exception {
+ byte[] payloadFetchAction = "{}".getBytes(StandardCharsets.UTF_8);
+
+ defineBehaviours(payloadFetchAction, FETCH_URI);
+ when(response.getOutputStream()).thenReturn(outputStream);
+
+ PullReplicationFilter pullReplicationFilter = createPullReplicationFilter(anonymousUserMock);
+ pullReplicationFilter.doFilter(request, response, filterChain);
+
+ verify(response).setStatus(HttpServletResponse.SC_UNAUTHORIZED);
+ }
+
+ @Test
public void shouldBe422WhenEntityCannotBeProcessed() throws Exception {
byte[] payloadFetchAction =
("{"
@@ -304,8 +340,7 @@
public void shouldBe409WhenThereIsResourceConflict() throws Exception {
when(request.getRequestURI()).thenReturn(DELETE_PROJECT_URI);
when(request.getMethod()).thenReturn("DELETE");
- when(projectsCollection.parse(TopLevelResource.INSTANCE, IdString.fromDecoded(PROJECT_NAME)))
- .thenReturn(projectResource);
+ when(projectCache.get(Project.nameKey(PROJECT_NAME))).thenReturn(Optional.of(projectState));
when(projectDeletionAction.apply(any(), any()))
.thenThrow(new ResourceConflictException("Resource conflict"));
