CreateRefControl: Pass explicit user

When used from ReceiveCommits, the injected Provider<CurrentUser> always
returns an anonymous user. While this behavior may not be intended, it
at least is not new, dating back at least to 2.14. The simplest fix to
avoid depending on the injected provider is to pass an explicit user.
Other approaches (e.g. introducing a factory) may result in cleaner code
overall, but are more invasive and thus less suitable for a stable
branch.

I eyeballed the diff of ReceiveCommits between stable-2.14 and
stable-2.15 to see if there were any other permission checks which might
be implicitly depending on the Provider<CurrentUser>, and I did not find
any. CreateRefControl is kind of a one-off, and the remainder of the
checks are handled by the PermissionBackend that is initialized in the
ReceiveCommits constructor with the correct user.

Bug: Issue 7385
Change-Id: I60dc41be1a91f26e1dfc3493997e72b837b881b0
This commit is contained in:
Dave Borowitz 2017-10-25 13:33:17 -04:00 committed by David Pursehouse
parent 1dd9eebfbc
commit c3bd56f6e5
3 changed files with 20 additions and 11 deletions

View File

@ -154,6 +154,7 @@ import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.util.Providers;
import java.io.IOException;
import java.io.StringWriter;
import java.util.ArrayList;
@ -1014,7 +1015,9 @@ class ReceiveCommits {
Branch.NameKey branch = new Branch.NameKey(project.getName(), cmd.getRefName());
try {
createRefControl.checkCreateRef(rp.getRepository(), branch, obj);
// Must pass explicit user instead of injecting a provider into CreateRefControl, since
// Provider<CurrentUser> within ReceiveCommits will always return anonymous.
createRefControl.checkCreateRef(Providers.of(user), rp.getRepository(), branch, obj);
} catch (AuthException denied) {
reject(cmd, "prohibited by Gerrit: " + denied.getMessage());
return;

View File

@ -121,7 +121,7 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
}
}
createRefControl.checkCreateRef(repo, name, object);
createRefControl.checkCreateRef(identifiedUser, repo, name, object);
try {
final RefUpdate u = repo.updateRef(ref);

View File

@ -41,26 +41,28 @@ public class CreateRefControl {
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
private final Provider<CurrentUser> user;
@Inject
CreateRefControl(
PermissionBackend permissionBackend, ProjectCache projectCache, Provider<CurrentUser> user) {
CreateRefControl(PermissionBackend permissionBackend, ProjectCache projectCache) {
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
this.user = user;
}
/**
* Checks whether the {@link CurrentUser} can create a new Git ref.
*
* @param user the user performing the operation
* @param repo repository on which user want to create
* @param branch the branch the new {@link RevObject} should be created on
* @param object the object the user will start the reference with
* @throws AuthException if creation is denied; the message explains the denial.
* @throws PermissionBackendException on failure of permission checks.
*/
public void checkCreateRef(Repository repo, Branch.NameKey branch, RevObject object)
public void checkCreateRef(
Provider<? extends CurrentUser> user,
Repository repo,
Branch.NameKey branch,
RevObject object)
throws AuthException, PermissionBackendException, NoSuchProjectException, IOException {
ProjectState ps = projectCache.checkedGet(branch.getParentKey());
if (ps == null) {
@ -73,7 +75,7 @@ public class CreateRefControl {
PermissionBackend.ForRef perm = permissionBackend.user(user).ref(branch);
if (object instanceof RevCommit) {
perm.check(RefPermission.CREATE);
checkCreateCommit(repo, (RevCommit) object, ps, perm);
checkCreateCommit(user, repo, (RevCommit) object, ps, perm);
} else if (object instanceof RevTag) {
RevTag tag = (RevTag) object;
try (RevWalk rw = new RevWalk(repo)) {
@ -93,9 +95,9 @@ public class CreateRefControl {
RevObject target = tag.getObject();
if (target instanceof RevCommit) {
checkCreateCommit(repo, (RevCommit) target, ps, perm);
checkCreateCommit(user, repo, (RevCommit) target, ps, perm);
} else {
checkCreateRef(repo, branch, target);
checkCreateRef(user, repo, branch, target);
}
// If the tag has a PGP signature, allow a lower level of permission
@ -116,7 +118,11 @@ public class CreateRefControl {
* new commit to the repository.
*/
private void checkCreateCommit(
Repository repo, RevCommit commit, ProjectState projectState, PermissionBackend.ForRef forRef)
Provider<? extends CurrentUser> user,
Repository repo,
RevCommit commit,
ProjectState projectState,
PermissionBackend.ForRef forRef)
throws AuthException, PermissionBackendException {
try {
// If the user has update (push) permission, they can create the ref regardless