Allow update head with Fetch replication global capability
If a user has been granted the global capability to run
pull replication, then it should also be able to update
the project head, otherwise the replication would be
interrupted when the project's head default branch is
updated.
Change-Id: I6fa89490e406a7b966ab8e89c173e5bb47271745
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 161bcf4..77d0e0b 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
@@ -16,13 +16,17 @@
import static com.googlesource.gerrit.plugins.replication.pull.api.FetchApiCapability.CALL_FETCH_ACTION;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.api.access.PluginPermission;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.RefPermission;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import com.googlesource.gerrit.plugins.replication.pull.api.exception.UnauthorizedAuthException;
public class FetchPreconditions {
private final String pluginName;
@@ -39,11 +43,31 @@
this.permissionBackend = permissionBackend;
}
- public Boolean canCallFetchApi() {
- CurrentUser currentUser = userProvider.get();
- PermissionBackend.WithUser userPermission = permissionBackend.user(currentUser);
+ public Boolean canCallFetchApi() throws UnauthorizedAuthException {
+ CurrentUser currentUser = currentUser();
+ return canCallFetchApi(currentUser, permissionBackend.user(currentUser));
+ }
+
+ private Boolean canCallFetchApi(
+ CurrentUser currentUser, PermissionBackend.WithUser userPermission) {
return currentUser.isInternalUser()
|| userPermission.testOrFalse(GlobalPermission.ADMINISTRATE_SERVER)
|| userPermission.testOrFalse(new PluginPermission(pluginName, CALL_FETCH_ACTION));
}
+
+ public Boolean canCallUpdateHeadApi(Project.NameKey projectNameKey, String ref)
+ throws PermissionBackendException, UnauthorizedAuthException {
+ CurrentUser currentUser = currentUser();
+ PermissionBackend.WithUser userAcls = permissionBackend.user(currentUser);
+ return canCallFetchApi(currentUser, userAcls)
+ || userAcls.project(projectNameKey).ref(ref).test(RefPermission.SET_HEAD);
+ }
+
+ private CurrentUser currentUser() throws UnauthorizedAuthException {
+ CurrentUser currentUser = userProvider.get();
+ if (!currentUser.isIdentifiedUser() && !currentUser.isInternalUser()) {
+ throw new UnauthorizedAuthException();
+ }
+ return currentUser;
+ }
}
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 8feb825..030af76 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
@@ -23,6 +23,7 @@
import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
import static javax.servlet.http.HttpServletResponse.SC_OK;
+import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.annotations.PluginName;
@@ -52,6 +53,7 @@
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionInput;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionsInput;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.InitProjectException;
+import com.googlesource.gerrit.plugins.replication.pull.api.exception.UnauthorizedAuthException;
import java.io.BufferedReader;
import java.io.EOFException;
import java.io.IOException;
@@ -137,6 +139,9 @@
chain.doFilter(request, response);
}
+ } catch (UnauthorizedAuthException e) {
+ RestApiServlet.replyError(
+ httpRequest, httpResponse, SC_UNAUTHORIZED, e.getMessage(), e.caching(), e);
} catch (AuthException e) {
RestApiServlet.replyError(
httpRequest, httpResponse, SC_FORBIDDEN, e.getMessage(), e.caching(), e);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadAction.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadAction.java
index 4195435..3f6673b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadAction.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadAction.java
@@ -24,8 +24,6 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.server.permissions.PermissionBackend;
-import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.project.ProjectResource;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -37,12 +35,12 @@
@Singleton
public class UpdateHeadAction implements RestModifyView<ProjectResource, HeadInput> {
private final GerritConfigOps gerritConfigOps;
- private final PermissionBackend permissionBackend;
+ private final FetchPreconditions preconditions;
@Inject
- UpdateHeadAction(GerritConfigOps gerritConfigOps, PermissionBackend permissionBackend) {
+ UpdateHeadAction(GerritConfigOps gerritConfigOps, FetchPreconditions preconditions) {
this.gerritConfigOps = gerritConfigOps;
- this.permissionBackend = permissionBackend;
+ this.preconditions = preconditions;
}
@Override
@@ -53,11 +51,9 @@
}
String ref = RefNames.fullName(input.ref);
- permissionBackend
- .user(projectResource.getUser())
- .project(projectResource.getNameKey())
- .ref(ref)
- .check(RefPermission.SET_HEAD);
+ if (!preconditions.canCallUpdateHeadApi(projectResource.getNameKey(), ref)) {
+ throw new AuthException("Update head not permitted");
+ }
// TODO: the .git suffix should not be added here, but rather it should be
// dealt with by the caller, honouring the naming style from the
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/exception/UnauthorizedAuthException.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/exception/UnauthorizedAuthException.java
new file mode 100644
index 0000000..9afac22
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/exception/UnauthorizedAuthException.java
@@ -0,0 +1,25 @@
+// Copyright (C) 2023 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.exception;
+
+import com.google.gerrit.extensions.restapi.AuthException;
+
+public class UnauthorizedAuthException extends AuthException {
+ private static final long serialVersionUID = 1L;
+
+ public UnauthorizedAuthException() {
+ super("Unauthorized access");
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectActionTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectActionTest.java
index 7ae3a78..d0cd1b4 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectActionTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectActionTest.java
@@ -35,6 +35,7 @@
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionObjectData;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingParentObjectException;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.RefUpdateException;
+import com.googlesource.gerrit.plugins.replication.pull.api.exception.UnauthorizedAuthException;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
@@ -90,7 +91,7 @@
@Mock FetchPreconditions preConditions;
@Before
- public void setup() {
+ public void setup() throws UnauthorizedAuthException {
when(preConditions.canCallFetchApi()).thenReturn(true);
applyObjectAction = new ApplyObjectAction(applyObjectCommand, deleteRefCommand, preConditions);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java
index ce0b9d3..e7048f6 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java
@@ -33,6 +33,7 @@
import com.google.gerrit.server.git.WorkQueue.Task;
import com.google.gerrit.server.project.ProjectResource;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.RemoteConfigurationMissingException;
+import com.googlesource.gerrit.plugins.replication.pull.api.exception.UnauthorizedAuthException;
import java.util.Optional;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ScheduledExecutorService;
@@ -66,7 +67,7 @@
@Mock FetchPreconditions preConditions;
@Before
- public void setup() {
+ public void setup() throws UnauthorizedAuthException {
when(fetchJobFactory.create(any(), any(), any())).thenReturn(fetchJob);
when(workQueue.getDefaultQueue()).thenReturn(exceutorService);
when(urlFormatter.getRestUrl(anyString())).thenReturn(Optional.of(location));
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadActionIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadActionIT.java
index d0f3214..9ceae5a 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadActionIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadActionIT.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import com.google.gerrit.acceptance.config.GerritConfig;
@@ -25,19 +26,54 @@
import com.google.gerrit.extensions.api.projects.HeadInput;
import com.google.gson.Gson;
import com.google.inject.Inject;
+import java.io.IOException;
import javax.servlet.http.HttpServletResponse;
-import org.apache.http.client.methods.HttpRequestBase;
+import org.apache.http.auth.AuthenticationException;
+import org.apache.http.client.ClientProtocolException;
import org.junit.Ignore;
import org.junit.Test;
public class UpdateHeadActionIT extends ActionITBase {
private static final Gson gson = newGson();
+ private static final String PLUGIN_NAME = "pull-replication";
@Inject private ProjectOperations projectOperations;
@Test
@GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
- public void shouldReturnUnauthorizedForUserWithoutPermissions() throws Exception {
+ public void shouldReturnForbiddenForUserWithoutPermissions() throws Exception {
+ shouldReturnForbiddenForUserWithoutPermissionsTest();
+ }
+
+ @Test
+ @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
+ @GerritConfig(name = "container.replica", value = "true")
+ public void shouldReturnForbiddenForUserWithoutPermissionsInReplica() throws Exception {
+ shouldReturnForbiddenForUserWithoutPermissionsTest();
+ }
+
+ private void shouldReturnForbiddenForUserWithoutPermissionsTest() throws Exception {
+ httpClientFactory
+ .create(source)
+ .execute(
+ withBasicAuthenticationAsUser(createPutRequest(headInput("some/branch"))),
+ assertHttpResponseCode(HttpServletResponse.SC_FORBIDDEN));
+ }
+
+ @Test
+ @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
+ public void shouldReturnUnauthorizedForAnonymousUser() throws Exception {
+ shouldReturnUnauthorizedForAnonymousUserTest();
+ }
+
+ @Test
+ @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
+ @GerritConfig(name = "container.replica", value = "true")
+ public void shouldReturnUnauthorizedForAnonymousUserInReplica() throws Exception {
+ shouldReturnUnauthorizedForAnonymousUserTest();
+ }
+
+ private void shouldReturnUnauthorizedForAnonymousUserTest() throws Exception {
httpClientFactory
.create(source)
.execute(
@@ -106,6 +142,17 @@
@Test
@GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
public void shouldReturnForbiddenWhenMissingPermissions() throws Exception {
+ shouldReturnForbiddenWhenMissingPermissionsTest();
+ }
+
+ @Test
+ @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
+ @GerritConfig(name = "container.replica", value = "true")
+ public void shouldReturnForbiddenWhenMissingPermissionsInReplica() throws Exception {
+ shouldReturnForbiddenWhenMissingPermissionsTest();
+ }
+
+ private void shouldReturnForbiddenWhenMissingPermissionsTest() throws Exception {
httpClientFactory
.create(source)
.execute(
@@ -115,18 +162,36 @@
@Test
@GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
- public void shouldReturnOKWhenRegisteredUserHasPermissions() throws Exception {
- String testProjectName = project.get();
- String newBranch = "refs/heads/mybranch";
- String master = "refs/heads/master";
- BranchInput input = new BranchInput();
- input.revision = master;
- gApi.projects().name(testProjectName).branch(newBranch).create(input);
- HttpRequestBase put = withBasicAuthenticationAsUser(createPutRequest(headInput(newBranch)));
+ public void shouldReturnOKForUserWithPullReplicationCapability() throws Exception {
+ shouldReturnOKForUserWithPullReplicationCapabilityTest();
+ }
+
+ @Test
+ @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
+ @GerritConfig(name = "container.replica", value = "true")
+ public void shouldReturnOKForUserWithPullReplicationCapabilityInReplica() throws Exception {
+ shouldReturnOKForUserWithPullReplicationCapabilityTest();
+ }
+
+ private void shouldReturnOKForUserWithPullReplicationCapabilityTest()
+ throws ClientProtocolException, IOException, AuthenticationException {
+ projectOperations
+ .allProjectsForUpdate()
+ .add(
+ allowCapability(PLUGIN_NAME + "-" + FetchApiCapability.CALL_FETCH_ACTION)
+ .group(REGISTERED_USERS))
+ .update();
+
httpClientFactory
.create(source)
- .execute(put, assertHttpResponseCode(HttpServletResponse.SC_FORBIDDEN));
+ .execute(
+ withBasicAuthenticationAsUser(createPutRequest(headInput("refs/heads/master"))),
+ assertHttpResponseCode(HttpServletResponse.SC_OK));
+ }
+ @Test
+ @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
+ public void shouldReturnOKWhenRegisteredUserIsProjectOwner() throws Exception {
projectOperations
.project(project)
.forUpdate()
@@ -135,18 +200,9 @@
httpClientFactory
.create(source)
- .execute(put, assertHttpResponseCode(HttpServletResponse.SC_OK));
- }
-
- @Test
- @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId")
- @GerritConfig(name = "container.replica", value = "true")
- public void shouldReturnForbiddenWhenMissingPermissionsInReplica() throws Exception {
- httpClientFactory
- .create(source)
.execute(
- withBasicAuthenticationAsUser(createPutRequest(headInput("some/new/head"))),
- assertHttpResponseCode(HttpServletResponse.SC_FORBIDDEN));
+ withBasicAuthenticationAsUser(createPutRequest(headInput("refs/heads/master"))),
+ assertHttpResponseCode(HttpServletResponse.SC_OK));
}
@Test
