Enable plugin-based validation of user's ref related operations
After user is authorized to perform a given ref operation, but just
before it is performed, invoke validation from plugins.
Validation is invoked for ref create, delete and update (including
non-ff) operations caused by user through either push or call to
existing ref related Gerrit endpoints (create/delete branch/es).
Change-Id: I11cbe51f54e7d133b336ab503577653b6fe0f443
Signed-off-by: Jacek Centkowski <geminica.programs@gmail.com>
diff --git a/Documentation/config-validation.txt b/Documentation/config-validation.txt
index 27b39eb..2707e5c 100644
--- a/Documentation/config-validation.txt
+++ b/Documentation/config-validation.txt
@@ -21,16 +21,18 @@
Out of the box, Gerrit includes a plugin that checks the length of the
subject and body lines of commit messages on uploaded commits.
-[[ref-operation-validation]]
-== Ref operation validation
+[[user-ref-operations-validation]]
+== User ref operations validation
Plugins implementing the `RefOperationValidationListener` interface can
-perform additional validation checks against ref creation/deletion operation
-before it is applied to the git repository.
+perform additional validation checks against user ref operations (resulting
+from either push or corresponding Gerrit REST/SSH endpoints call e.g.
+create branch etc.). Namely including ref creation, deletion and update
+(also non-fast-forward) before they are applied to the git repository.
-If the ref operation fails the validation, the plugin can throw an exception
-which will cause the operation to fail.
+The plugin can throw an exception which will cause the operation to fail,
+and prevent the ref update from being applied.
[[pre-merge-validation]]
== Pre-merge validation
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index 21fe05c..2f73360 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -100,6 +100,9 @@
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidationMessage;
import com.google.gerrit.server.git.validators.CommitValidators;
+import com.google.gerrit.server.git.validators.RefOperationValidationException;
+import com.google.gerrit.server.git.validators.RefOperationValidators;
+import com.google.gerrit.server.git.validators.ValidationMessage;
import com.google.gerrit.server.mail.MailUtil.MailRecipients;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NotesMigration;
@@ -289,6 +292,7 @@
private final ProjectCache projectCache;
private final String canonicalWebUrl;
private final CommitValidators.Factory commitValidatorsFactory;
+ private final RefOperationValidators.Factory refValidatorsFactory;
private final TagCache tagCache;
private final AccountCache accountCache;
private final ChangeInserter.Factory changeInserterFactory;
@@ -329,7 +333,7 @@
private final NotesMigration notesMigration;
private final ChangeEditUtil editUtil;
- private final List<CommitValidationMessage> messages = new ArrayList<>();
+ private final List<ValidationMessage> messages = new ArrayList<>();
private ListMultimap<Error, String> errors = LinkedListMultimap.create();
private Task newProgress;
private Task replaceProgress;
@@ -355,6 +359,7 @@
@Nullable SearchingChangeCacheImpl changeCache,
ChangeInserter.Factory changeInserterFactory,
CommitValidators.Factory commitValidatorsFactory,
+ RefOperationValidators.Factory refValidatorsFactory,
@CanonicalWebUrl String canonicalWebUrl,
RequestScopePropagator requestScopePropagator,
SshInfo sshInfo,
@@ -392,6 +397,7 @@
this.accountCache = accountCache;
this.changeInserterFactory = changeInserterFactory;
this.commitValidatorsFactory = commitValidatorsFactory;
+ this.refValidatorsFactory = refValidatorsFactory;
this.requestScopePropagator = requestScopePropagator;
this.sshInfo = sshInfo;
this.allProjectsName = allProjectsName;
@@ -544,7 +550,7 @@
}
void sendMessages() {
- for (CommitValidationMessage m : messages) {
+ for (ValidationMessage m : messages) {
if (m.isError()) {
messageSender.sendError(m.getMessage());
} else {
@@ -1096,6 +1102,9 @@
RefControl ctl = projectControl.controlForRef(cmd.getRefName());
if (ctl.canCreate(db, rp.getRepository(), obj)) {
+ if (!validRefOperation(cmd)) {
+ return;
+ }
validateNewCommits(ctl, cmd);
batch.addCommand(cmd);
} else {
@@ -1111,6 +1120,9 @@
return;
}
+ if (!validRefOperation(cmd)) {
+ return;
+ }
validateNewCommits(ctl, cmd);
batch.addCommand(cmd);
} else {
@@ -1148,6 +1160,9 @@
errors.put(Error.DELETE_CHANGES, ctl.getRefName());
reject(cmd, "cannot delete changes");
} else if (ctl.canDelete()) {
+ if (!validRefOperation(cmd)) {
+ return;
+ }
batch.addCommand(cmd);
} else {
if (RefNames.REFS_CONFIG.equals(ctl.getRefName())) {
@@ -1182,6 +1197,9 @@
}
if (ctl.canForceUpdate()) {
+ if (!validRefOperation(cmd)) {
+ return;
+ }
batch.setAllowNonFastForwards(true).addCommand(cmd);
} else {
cmd.setResult(REJECTED_NONFASTFORWARD, " need '"
@@ -2399,6 +2417,21 @@
}
}
+ private boolean validRefOperation(ReceiveCommand cmd) {
+ RefOperationValidators refValidators =
+ refValidatorsFactory.create(getProject(), user, cmd);
+
+ try {
+ messages.addAll(refValidators.validateForRefOperation());
+ } catch (RefOperationValidationException e) {
+ messages.addAll(Lists.newArrayList(e.getMessages()));
+ reject(cmd, e.getMessage());
+ return false;
+ }
+
+ return true;
+ }
+
private void validateNewCommits(RefControl ctl, ReceiveCommand cmd) {
if (ctl.canForgeAuthor()
&& ctl.canForgeCommitter()
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
index 674e788..c7b2922 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
@@ -55,6 +55,7 @@
private final GitRepositoryManager repoManager;
private final Provider<ReviewDb> db;
private final GitReferenceUpdated referenceUpdated;
+ private final RefValidationHelper refCreationValidator;
private String ref;
@Inject
@@ -62,11 +63,14 @@
GitRepositoryManager repoManager,
Provider<ReviewDb> db,
GitReferenceUpdated referenceUpdated,
+ RefValidationHelper.Factory refHelperFactory,
@Assisted String ref) {
this.identifiedUser = identifiedUser;
this.repoManager = repoManager;
this.db = db;
this.referenceUpdated = referenceUpdated;
+ this.refCreationValidator =
+ refHelperFactory.create(ReceiveCommand.Type.CREATE);
this.ref = ref;
}
@@ -123,6 +127,8 @@
u.setNewObjectId(object.copy());
u.setRefLogIdent(identifiedUser.get().newRefLogIdent());
u.setRefLogMessage("created via REST from " + input.revision, false);
+ refCreationValidator.validateRefOperation(
+ rsrc.getName(), identifiedUser.get(), u);
final RefUpdate.Result result = u.update(rw);
switch (result) {
case FAST_FORWARD:
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java
index 9db12b8..091cba3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java
@@ -50,16 +50,20 @@
private final GitRepositoryManager repoManager;
private final Provider<InternalChangeQuery> queryProvider;
private final GitReferenceUpdated referenceUpdated;
+ private final RefValidationHelper refDeletionValidator;
@Inject
DeleteBranch(Provider<IdentifiedUser> identifiedUser,
GitRepositoryManager repoManager,
Provider<InternalChangeQuery> queryProvider,
- GitReferenceUpdated referenceUpdated) {
+ GitReferenceUpdated referenceUpdated,
+ RefValidationHelper.Factory refHelperFactory) {
this.identifiedUser = identifiedUser;
this.repoManager = repoManager;
this.queryProvider = queryProvider;
this.referenceUpdated = referenceUpdated;
+ this.refDeletionValidator =
+ refHelperFactory.create(ReceiveCommand.Type.DELETE);
}
@Override
@@ -78,6 +82,8 @@
RefUpdate.Result result;
RefUpdate u = r.updateRef(rsrc.getRef());
u.setForceUpdate(true);
+ refDeletionValidator.validateRefOperation(
+ rsrc.getName(), identifiedUser.get(), u);
int remainingLockFailureCalls = MAX_LOCK_FAILURE_CALLS;
for (;;) {
try {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java
index e0a84eb..f4fa446 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java
@@ -35,6 +35,7 @@
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
@@ -53,16 +54,20 @@
private final GitRepositoryManager repoManager;
private final Provider<InternalChangeQuery> queryProvider;
private final GitReferenceUpdated referenceUpdated;
+ private final RefValidationHelper refDeletionValidator;
@Inject
DeleteBranches(Provider<IdentifiedUser> identifiedUser,
GitRepositoryManager repoManager,
Provider<InternalChangeQuery> queryProvider,
- GitReferenceUpdated referenceUpdated) {
+ GitReferenceUpdated referenceUpdated,
+ RefValidationHelper.Factory refHelperFactory) {
this.identifiedUser = identifiedUser;
this.repoManager = repoManager;
this.queryProvider = queryProvider;
this.referenceUpdated = referenceUpdated;
+ this.refDeletionValidator =
+ refHelperFactory.create(ReceiveCommand.Type.DELETE);
}
@Override
@@ -100,7 +105,8 @@
}
private ReceiveCommand createDeleteCommand(ProjectResource project,
- Repository r, String branch) throws OrmException, IOException {
+ Repository r, String branch)
+ throws OrmException, IOException, ResourceConflictException {
Ref ref = r.getRefDatabase().getRef(branch);
ReceiveCommand command;
if (ref == null) {
@@ -120,6 +126,10 @@
if (!queryProvider.get().setLimit(1).byBranchOpen(branchKey).isEmpty()) {
command.setResult(Result.REJECTED_OTHER_REASON, "it has open changes");
}
+ RefUpdate u = r.updateRef(branch);
+ u.setForceUpdate(true);
+ refDeletionValidator.validateRefOperation(
+ project.getName(), identifiedUser.get(), u);
return command;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/Module.java
index 9706298..8a6145a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/Module.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/Module.java
@@ -69,6 +69,7 @@
post(PROJECT_KIND, "branches:delete").to(DeleteBranches.class);
factory(CreateBranch.Factory.class);
get(BRANCH_KIND, "mergeable").to(CheckMergeability.class);
+ factory(RefValidationHelper.Factory.class);
get(BRANCH_KIND, "reflog").to(GetReflog.class);
child(BRANCH_KIND, "files").to(FilesCollection.class);
get(FILE_KIND, "content").to(GetContent.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefValidationHelper.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefValidationHelper.java
new file mode 100644
index 0000000..6e2fd5d
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefValidationHelper.java
@@ -0,0 +1,56 @@
+// Copyright (C) 2014 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.google.gerrit.server.project;
+
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.git.validators.RefOperationValidators;
+import com.google.gerrit.server.validators.ValidationException;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.transport.ReceiveCommand.Type;
+
+public class RefValidationHelper {
+ public interface Factory {
+ RefValidationHelper create(Type operationType);
+ }
+
+ private final RefOperationValidators.Factory refValidatorsFactory;
+ private final Type operationType;
+
+ @Inject
+ RefValidationHelper(RefOperationValidators.Factory refValidatorsFactory,
+ @Assisted Type operationType) {
+ this.refValidatorsFactory = refValidatorsFactory;
+ this.operationType = operationType;
+ }
+
+ public void validateRefOperation(String projectName, IdentifiedUser user,
+ RefUpdate update) throws ResourceConflictException {
+ RefOperationValidators refValidators =
+ refValidatorsFactory.create(
+ new Project(new Project.NameKey(projectName)),
+ user,
+ RefOperationValidators.getCommand(update, operationType));
+ try {
+ refValidators.validateForRefOperation();
+ } catch (ValidationException e) {
+ throw new ResourceConflictException(e.getMessage());
+ }
+ }
+}
