opendev/gerrit
Code Issues Proposed changes

Merge "Factor RefControl#canCreate(Repository, RevCommit) out"

Browse Source
This commit is contained in:
Edwin Kempin
2017-08-24 15:55:03 +00:00
committed by Gerrit Code Review
parent 61ead33fed aa8c1ddefa
commit caa5f7824f
4 changed files with 198 additions and 122 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
View File

@@ -128,7 +128,9 @@ 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.gerrit.server.project.CreateRefControl;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectState;
@@ -320,6 +322,7 @@ class ReceiveCommits {
private final String canonicalWebUrl;
private final SubmoduleOp.Factory subOpFactory;
private final TagCache tagCache;
private final CreateRefControl createRefControl;
// Assisted injected fields.
private final AllRefsWatcher allRefsWatcher;
@@ -402,6 +405,7 @@ class ReceiveCommits {
SshInfo sshInfo,
SubmoduleOp.Factory subOpFactory,
TagCache tagCache,
CreateRefControl createRefControl,
@Assisted ProjectControl projectControl,
@Assisted ReceivePack rp,
@Assisted AllRefsWatcher allRefsWatcher,
@@ -440,6 +444,7 @@ class ReceiveCommits {
this.sshInfo = sshInfo;
this.subOpFactory = subOpFactory;
this.tagCache = tagCache;
this.createRefControl = createRefControl;
// Assisted injected fields.
this.allRefsWatcher = allRefsWatcher;
@@ -524,7 +529,7 @@ class ReceiveCommits {
try {
parseCommands(commands);
} catch (PermissionBackendException err) {
} catch (PermissionBackendException | NoSuchProjectException | IOException err) {
for (ReceiveCommand cmd : actualCommands) {
if (cmd.getResult() == NOT_ATTEMPTED) {
cmd.setResult(REJECTED_OTHER_REASON, "internal server error");
@@ -772,7 +777,7 @@ class ReceiveCommits {
}
private void parseCommands(Collection<ReceiveCommand> commands)
throws PermissionBackendException {
throws PermissionBackendException, NoSuchProjectException, IOException {
List<String> optionList = rp.getPushOptions();
if (optionList != null) {
for (String option : optionList) {
@@ -977,7 +982,8 @@ class ReceiveCommits {
}
}
private void parseCreate(ReceiveCommand cmd) throws PermissionBackendException {
private void parseCreate(ReceiveCommand cmd)
throws PermissionBackendException, NoSuchProjectException, IOException {
RevObject obj;
try {
obj = rp.getRevWalk().parseAny(cmd.getNewId());
@@ -994,8 +1000,8 @@ class ReceiveCommits {
return;
}
RefControl ctl = projectControl.controlForRef(cmd.getRefName());
String rejectReason = ctl.canCreate(rp.getRepository(), obj);
Branch.NameKey branch = new Branch.NameKey(project.getName(), cmd.getRefName());
String rejectReason = createRefControl.canCreateRef(rp.getRepository(), obj, user, branch);
if (rejectReason != null) {
reject(cmd, "prohibited by Gerrit: " + rejectReason);
return;
@@ -1006,6 +1012,7 @@ class ReceiveCommits {
return;
}
RefControl ctl = projectControl.controlForRef(cmd.getRefName());
validateNewCommits(ctl, cmd);
actualCommands.add(cmd);
}

10
gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
View File

@@ -26,6 +26,7 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.util.MagicBranch;
import com.google.inject.Inject;
@@ -55,6 +56,7 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
private final GitRepositoryManager repoManager;
private final GitReferenceUpdated referenceUpdated;
private final RefValidationHelper refCreationValidator;
private final CreateRefControl createRefControl;
private String ref;
@Inject
@@ -64,18 +66,21 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
GitRepositoryManager repoManager,
GitReferenceUpdated referenceUpdated,
RefValidationHelper.Factory refHelperFactory,
CreateRefControl createRefControl,
@Assisted String ref) {
this.identifiedUser = identifiedUser;
this.permissionBackend = permissionBackend;
this.repoManager = repoManager;
this.referenceUpdated = referenceUpdated;
this.refCreationValidator = refHelperFactory.create(ReceiveCommand.Type.CREATE);
this.createRefControl = createRefControl;
this.ref = ref;
}
@Override
public BranchInfo apply(ProjectResource rsrc, BranchInput input)
throws BadRequestException, AuthException, ResourceConflictException, IOException {
throws BadRequestException, AuthException, ResourceConflictException, IOException,
PermissionBackendException, NoSuchProjectException {
if (input == null) {
input = new BranchInput();
}
@@ -100,7 +105,6 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
}
final Branch.NameKey name = new Branch.NameKey(rsrc.getNameKey(), ref);
final RefControl refControl = rsrc.getControl().controlForRef(name);
try (Repository repo = repoManager.openRepository(rsrc.getNameKey())) {
ObjectId revid = RefUtil.parseBaseRevision(repo, rsrc.getNameKey(), input.revision);
RevWalk rw = RefUtil.verifyConnected(repo, revid);
@@ -117,7 +121,7 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
}
}
String rejectReason = refControl.canCreate(repo, object);
String rejectReason = createRefControl.canCreateRef(repo, object, identifiedUser.get(), name);
if (rejectReason != null) {
throw new AuthException("Cannot create \"" + ref + "\": " + rejectReason);
}

179
gerrit-server/src/main/java/com/google/gerrit/server/project/CreateRefControl.java Normal file
View File

@@ -0,0 +1,179 @@
// Copyright (C) 2017 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.common.Nullable;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
import java.io.IOException;
import javax.inject.Inject;
import javax.inject.Singleton;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.revwalk.RevTag;
import org.eclipse.jgit.revwalk.RevWalk;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** Manages access control for creating Git references (aka branches, tags). */
@Singleton
public class CreateRefControl {
private static final Logger log = LoggerFactory.getLogger(CreateRefControl.class);
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
@Inject
CreateRefControl(PermissionBackend permissionBackend, ProjectCache projectCache) {
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
}
/**
* Determines whether the user can create a new Git ref.
*
* @param repo repository on which user want to create
* @param object the object the user will start the reference with
* @param user the current identified user
* @param branch the branch the new {@link RevObject} should be created on
* @return {@code null} if the user specified can create a new Git ref, or a String describing why
* the creation is not allowed.
* @throws PermissionBackendException on failure of permission checks
*/
@Nullable
public String canCreateRef(
Repository repo, RevObject object, IdentifiedUser user, Branch.NameKey branch)
throws PermissionBackendException, NoSuchProjectException, IOException {
ProjectState ps = projectCache.checkedGet(branch.getParentKey());
if (ps == null) {
throw new NoSuchProjectException(branch.getParentKey());
}
if (!ps.getProject()
.getState()
.equals(com.google.gerrit.extensions.client.ProjectState.ACTIVE)) {
return "project state does not permit write";
}
PermissionBackend.ForRef perm = permissionBackend.user(user).ref(branch);
if (object instanceof RevCommit) {
if (!testAuditLogged(perm, RefPermission.CREATE)) {
return user.getAccountId() + " lacks permission: " + Permission.CREATE;
}
return canCreateCommit(repo, (RevCommit) object, ps, user, perm);
} else if (object instanceof RevTag) {
final RevTag tag = (RevTag) object;
try (RevWalk rw = new RevWalk(repo)) {
rw.parseBody(tag);
} catch (IOException e) {
String msg =
String.format("RevWalk(%s) for pushing tag %s:", branch.getParentKey(), tag.name());
log.error(msg, e);
return "I/O exception for revwalk";
}
// If tagger is present, require it matches the user's email.
//
final PersonIdent tagger = tag.getTaggerIdent();
if (tagger != null) {
boolean valid;
if (user.isIdentifiedUser()) {
final String addr = tagger.getEmailAddress();
valid = user.asIdentifiedUser().hasEmailAddress(addr);
} else {
valid = false;
}
if (!valid && !testAuditLogged(perm, RefPermission.FORGE_COMMITTER)) {
return user.getAccountId() + " lacks permission: " + Permission.FORGE_COMMITTER;
}
}
RevObject tagObject = tag.getObject();
if (tagObject instanceof RevCommit) {
String rejectReason = canCreateCommit(repo, (RevCommit) tagObject, ps, user, perm);
if (rejectReason != null) {
return rejectReason;
}
} else {
String rejectReason = canCreateRef(repo, tagObject, user, branch);
if (rejectReason != null) {
return rejectReason;
}
}
// If the tag has a PGP signature, allow a lower level of permission
// than if it doesn't have a PGP signature.
//
RefControl refControl = ps.controlFor(user).controlForRef(branch);
if (tag.getFullMessage().contains("-----BEGIN PGP SIGNATURE-----\n")) {
return refControl.canPerform(Permission.CREATE_SIGNED_TAG)
? null
: user.getAccountId() + " lacks permission: " + Permission.CREATE_SIGNED_TAG;
}
return refControl.canPerform(Permission.CREATE_TAG)
? null
: user.getAccountId() + " lacks permission " + Permission.CREATE_TAG;
}
return null;
}
/**
* Check if the user is allowed to create a new commit object if this introduces a new commit to
* the project. If not allowed, returns a string describing why it's not allowed. The userId
* argument is only used for the error message.
*/
@Nullable
private String canCreateCommit(
Repository repo,
RevCommit commit,
ProjectState projectState,
IdentifiedUser user,
PermissionBackend.ForRef forRef)
throws PermissionBackendException {
if (projectState.controlFor(user).isReachableFromHeadsOrTags(repo, commit)) {
// If the user has no push permissions, check whether the object is
// merged into a branch or tag readable by this user. If so, they are
// not effectively "pushing" more objects, so they can create the ref
// even if they don't have push permission.
return null;
} else if (testAuditLogged(forRef, RefPermission.UPDATE)) {
// If the user has push permissions, they can create the ref regardless
// of whether they are pushing any new objects along with the create.
return null;
}
return user.getAccountId()
+ " lacks permission "
+ Permission.PUSH
+ " for creating new commit object";
}
private boolean testAuditLogged(PermissionBackend.ForRef forRef, RefPermission p)
throws PermissionBackendException {
try {
forRef.check(p);
} catch (AuthException e) {
return false;
}
return true;
}
}

114
gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
View File

@@ -16,7 +16,6 @@ package com.google.gerrit.server.project;
import static com.google.common.base.Preconditions.checkArgument;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.common.data.PermissionRule;
@@ -34,7 +33,6 @@ import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@@ -44,19 +42,9 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.revwalk.RevTag;
import org.eclipse.jgit.revwalk.RevWalk;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** Manages access control for Git references (aka branches, tags). */
public class RefControl {
private static final Logger log = LoggerFactory.getLogger(RefControl.class);
private final ProjectControl projectControl;
private final String refName;
@@ -230,108 +218,6 @@ public class RefControl {
return canForcePerform(Permission.PUSH);
}
/**
* Determines whether the user can create a new Git ref.
*
* @param repo repository on which user want to create
* @param object the object the user will start the reference with.
* @return {@code null} if the user specified can create a new Git ref, or a String describing why
* the creation is not allowed.
*/
@Nullable
public String canCreate(Repository repo, RevObject object) {
if (!isProjectStatePermittingWrite()) {
return "project state does not permit write";
}
String userId =
getUser().isIdentifiedUser() ? "account " + getUser().getAccountId() : "anonymous user";
if (object instanceof RevCommit) {
if (!canPerform(Permission.CREATE)) {
return userId + " lacks permission: " + Permission.CREATE;
}
return canCreateCommit(repo, (RevCommit) object, userId);
} else if (object instanceof RevTag) {
final RevTag tag = (RevTag) object;
try (RevWalk rw = new RevWalk(repo)) {
rw.parseBody(tag);
} catch (IOException e) {
String msg =
String.format(
"RevWalk(%s) for pushing tag %s:",
projectControl.getProject().getNameKey(), tag.name());
log.error(msg, e);
return "I/O exception for revwalk";
}
// If tagger is present, require it matches the user's email.
//
final PersonIdent tagger = tag.getTaggerIdent();
if (tagger != null) {
boolean valid;
if (getUser().isIdentifiedUser()) {
final String addr = tagger.getEmailAddress();
valid = getUser().asIdentifiedUser().hasEmailAddress(addr);
} else {
valid = false;
}
if (!valid && !canForgeCommitter()) {
return userId + " lacks permission: " + Permission.FORGE_COMMITTER;
}
}
RevObject tagObject = tag.getObject();
if (tagObject instanceof RevCommit) {
String rejectReason = canCreateCommit(repo, (RevCommit) tagObject, userId);
if (rejectReason != null) {
return rejectReason;
}
} else {
String rejectReason = canCreate(repo, tagObject);
if (rejectReason != null) {
return rejectReason;
}
}
// If the tag has a PGP signature, allow a lower level of permission
// than if it doesn't have a PGP signature.
//
if (tag.getFullMessage().contains("-----BEGIN PGP SIGNATURE-----\n")) {
return canPerform(Permission.CREATE_SIGNED_TAG)
? null
: userId + " lacks permission: " + Permission.CREATE_SIGNED_TAG;
}
return canPerform(Permission.CREATE_TAG)
? null
: userId + " lacks permission " + Permission.CREATE_TAG;
}
return null;
}
/**
* Check if the user is allowed to create a new commit object if this introduces a new commit to
* the project. If not allowed, returns a string describing why it's not allowed. The userId
* argument is only used for the error message.
*/
@Nullable
private String canCreateCommit(Repository repo, RevCommit commit, String userId) {
if (canUpdate()) {
// If the user has push permissions, they can create the ref regardless
// of whether they are pushing any new objects along with the create.
return null;
} else if (projectControl.isReachableFromHeadsOrTags(repo, commit)) {
// If the user has no push permissions, check whether the object is
// merged into a branch or tag readable by this user. If so, they are
// not effectively "pushing" more objects, so they can create the ref
// even if they don't have push permission.
return null;
}
return userId + " lacks permission " + Permission.PUSH + " for creating new commit object";
}
/**
* Determines whether the user can delete the Git ref controlled by this object.
*