Convert parsing projects to use PermissionBackend

When parsing a project name from a command line argument or in the
REST API, check the caller has ACCESS permission using
PermissionBackend, failing if they don't.

In UploadArchive check READ permission to determine if the
reachability check can be skipped.

Change-Id: I8b9155834a4ab36b964e339f5d9e1d110f771158
This commit is contained in:
Shawn Pearce
2017-02-23 11:39:01 -08:00
committed by David Pursehouse
parent 0527c60251
commit 8ea6df30a2
12 changed files with 88 additions and 34 deletions

View File

@@ -80,7 +80,8 @@ public class PostWatchedProjects
}
private Map<ProjectWatchKey, Set<NotifyType>> asMap(List<ProjectWatchInfo> input)
throws BadRequestException, UnprocessableEntityException, IOException {
throws BadRequestException, UnprocessableEntityException, IOException,
PermissionBackendException {
Map<ProjectWatchKey, Set<NotifyType>> m = new HashMap<>();
for (ProjectWatchInfo info : input) {
if (info.project == null) {

View File

@@ -32,6 +32,7 @@ import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.CreateChange;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.query.change.QueryChanges;
import com.google.gerrit.server.update.UpdateException;
@@ -87,7 +88,11 @@ class ChangesImpl implements Changes {
try {
ChangeInfo out = createChange.apply(TopLevelResource.INSTANCE, in).value();
return api.create(changes.parse(new Change.Id(out._number)));
} catch (OrmException | IOException | InvalidChangeOperationException | UpdateException e) {
} catch (OrmException
| IOException
| InvalidChangeOperationException
| UpdateException
| PermissionBackendException e) {
throw new RestApiException("Cannot create change", e);
}
}

View File

@@ -122,7 +122,7 @@ class GroupsImpl implements Groups {
for (String project : req.getProjects()) {
try {
list.addProject(projects.parse(tlr, IdString.fromDecoded(project)).getControl());
} catch (IOException e) {
} catch (IOException | PermissionBackendException e) {
throw new RestApiException("Error looking up project " + project, e);
}
}

View File

@@ -389,7 +389,7 @@ public class ProjectApiImpl implements ProjectApi {
public ChildProjectApi child(String name) throws RestApiException {
try {
return childApi.create(children.parse(checkExists(), IdString.fromDecoded(name)));
} catch (IOException e) {
} catch (IOException | PermissionBackendException e) {
throw new RestApiException("Cannot parse child project", e);
}
}

View File

@@ -21,6 +21,7 @@ import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ListProjects;
import com.google.gerrit.server.project.ListProjects.FilterType;
import com.google.gerrit.server.project.ProjectsCollection;
@@ -52,8 +53,8 @@ class ProjectsImpl implements Projects {
return api.create(projects.parse(name));
} catch (UnprocessableEntityException e) {
return api.create(name);
} catch (IOException e) {
throw new RestApiException("Cannot retrieve project");
} catch (IOException | PermissionBackendException e) {
throw new RestApiException("Cannot retrieve project", e);
}
}

View File

@@ -15,8 +15,12 @@
package com.google.gerrit.server.args4j;
import com.google.gerrit.common.ProjectUtil;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.inject.Inject;
@@ -34,18 +38,22 @@ import org.slf4j.LoggerFactory;
public class ProjectControlHandler extends OptionHandler<ProjectControl> {
private static final Logger log = LoggerFactory.getLogger(ProjectControlHandler.class);
private final ProjectControl.GenericFactory projectControlFactory;
private final PermissionBackend permissionBackend;
private final Provider<CurrentUser> user;
@Inject
public ProjectControlHandler(
final ProjectControl.GenericFactory projectControlFactory,
ProjectControl.GenericFactory projectControlFactory,
PermissionBackend permissionBackend,
Provider<CurrentUser> user,
@Assisted final CmdLineParser parser,
@Assisted final OptionDef option,
@Assisted final Setter<ProjectControl> setter) {
super(parser, option, setter);
this.projectControlFactory = projectControlFactory;
this.permissionBackend = permissionBackend;
this.user = user;
}
@@ -69,14 +77,15 @@ public class ProjectControlHandler extends OptionHandler<ProjectControl> {
String nameWithoutSuffix = ProjectUtil.stripGitSuffix(projectName);
Project.NameKey nameKey = new Project.NameKey(nameWithoutSuffix);
final ProjectControl control;
ProjectControl control;
try {
control =
projectControlFactory.validateFor(
nameKey, ProjectControl.OWNER | ProjectControl.VISIBLE, user.get());
control = projectControlFactory.controlFor(nameKey, user.get());
permissionBackend.user(user).project(nameKey).check(ProjectPermission.ACCESS);
} catch (AuthException e) {
throw new CmdLineException(owner, new NoSuchProjectException(nameKey).getMessage());
} catch (NoSuchProjectException e) {
throw new CmdLineException(owner, e.getMessage());
} catch (IOException e) {
} catch (PermissionBackendException | IOException e) {
log.warn("Cannot load project " + nameWithoutSuffix, e);
throw new CmdLineException(owner, new NoSuchProjectException(nameKey).getMessage());
}

View File

@@ -53,6 +53,7 @@ import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.ProjectControl;
@@ -145,7 +146,7 @@ public class CreateChange implements RestModifyView<TopLevelResource, ChangeInpu
@Override
public Response<ChangeInfo> apply(TopLevelResource parent, ChangeInput input)
throws OrmException, IOException, InvalidChangeOperationException, RestApiException,
UpdateException {
UpdateException, PermissionBackendException {
if (Strings.isNullOrEmpty(input.project)) {
throw new BadRequestException("project must be non-empty");
}

View File

@@ -21,6 +21,7 @@ import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -50,7 +51,7 @@ public class ChildProjectsCollection
@Override
public ChildProjectResource parse(ProjectResource parent, IdString id)
throws ResourceNotFoundException, IOException {
throws ResourceNotFoundException, IOException, PermissionBackendException {
ProjectResource p = projectsCollection.parse(TopLevelResource.INSTANCE, id);
for (ProjectState pp : p.getControl().getProjectState().parents()) {
if (parent.getNameKey().equals(pp.getProject().getNameKey())) {

View File

@@ -55,6 +55,7 @@ import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.RepositoryCaseMismatchException;
import com.google.gerrit.server.group.GroupsCollection;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.validators.ProjectCreationValidationListener;
import com.google.gerrit.server.validators.ValidationException;
import com.google.inject.Inject;
@@ -148,7 +149,8 @@ public class CreateProject implements RestModifyView<TopLevelResource, ProjectIn
@Override
public Response<ProjectInfo> apply(TopLevelResource resource, ProjectInput input)
throws BadRequestException, UnprocessableEntityException, ResourceConflictException,
ResourceNotFoundException, IOException, ConfigInvalidException {
ResourceNotFoundException, IOException, ConfigInvalidException,
PermissionBackendException {
if (input == null) {
input = new ProjectInput();
}

View File

@@ -14,8 +14,10 @@
package com.google.gerrit.server.project;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AcceptsCreate;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestCollection;
@@ -25,6 +27,9 @@ import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.OutputFormat;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -37,6 +42,7 @@ public class ProjectsCollection
private final DynamicMap<RestView<ProjectResource>> views;
private final Provider<ListProjects> list;
private final ProjectControl.GenericFactory controlFactory;
private final PermissionBackend permissionBackend;
private final Provider<CurrentUser> user;
private final CreateProject.Factory createProjectFactory;
@@ -45,11 +51,13 @@ public class ProjectsCollection
DynamicMap<RestView<ProjectResource>> views,
Provider<ListProjects> list,
ProjectControl.GenericFactory controlFactory,
PermissionBackend permissionBackend,
CreateProject.Factory factory,
Provider<CurrentUser> user) {
this.views = views;
this.list = list;
this.controlFactory = controlFactory;
this.permissionBackend = permissionBackend;
this.user = user;
this.createProjectFactory = factory;
}
@@ -61,7 +69,7 @@ public class ProjectsCollection
@Override
public ProjectResource parse(TopLevelResource parent, IdString id)
throws ResourceNotFoundException, IOException {
throws ResourceNotFoundException, IOException, PermissionBackendException {
ProjectResource rsrc = _parse(id.get(), true);
if (rsrc == null) {
throw new ResourceNotFoundException(id);
@@ -77,8 +85,10 @@ public class ProjectsCollection
* @throws UnprocessableEntityException thrown if the project ID cannot be resolved or if the
* project is not visible to the calling user
* @throws IOException thrown when there is an error.
* @throws PermissionBackendException
*/
public ProjectResource parse(String id) throws UnprocessableEntityException, IOException {
public ProjectResource parse(String id)
throws UnprocessableEntityException, IOException, PermissionBackendException {
return parse(id, true);
}
@@ -86,33 +96,43 @@ public class ProjectsCollection
* Parses a project ID from a request body and returns the project.
*
* @param id ID of the project, can be a project name
* @param checkVisibility Whether to check or not that project is visible to the calling user
* @param checkAccess if true, check the project is accessible by the current user
* @return the project
* @throws UnprocessableEntityException thrown if the project ID cannot be resolved or if the
* project is not visible to the calling user and checkVisibility is true.
* @throws IOException thrown when there is an error.
* @throws PermissionBackendException
*/
public ProjectResource parse(String id, boolean checkVisibility)
throws UnprocessableEntityException, IOException {
ProjectResource rsrc = _parse(id, checkVisibility);
public ProjectResource parse(String id, boolean checkAccess)
throws UnprocessableEntityException, IOException, PermissionBackendException {
ProjectResource rsrc = _parse(id, checkAccess);
if (rsrc == null) {
throw new UnprocessableEntityException(String.format("Project Not Found: %s", id));
}
return rsrc;
}
private ProjectResource _parse(String id, boolean checkVisibility) throws IOException {
@Nullable
private ProjectResource _parse(String id, boolean checkAccess)
throws IOException, PermissionBackendException {
if (id.endsWith(Constants.DOT_GIT_EXT)) {
id = id.substring(0, id.length() - Constants.DOT_GIT_EXT.length());
}
Project.NameKey nameKey = new Project.NameKey(id);
ProjectControl ctl;
try {
ctl = controlFactory.controlFor(new Project.NameKey(id), user.get());
ctl = controlFactory.controlFor(nameKey, user.get());
} catch (NoSuchProjectException e) {
return null;
}
if (checkVisibility && !ctl.isVisible() && !ctl.isOwner()) {
return null;
if (checkAccess) {
try {
permissionBackend.user(user).project(nameKey).check(ProjectPermission.ACCESS);
} catch (AuthException e) {
return null; // Pretend like not found on access denied.
}
}
return new ProjectResource(ctl);
}

View File

@@ -18,6 +18,7 @@ import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.sshd.SshScope.Context;
import com.google.inject.Inject;
@@ -84,7 +85,7 @@ public abstract class AbstractGitCommand extends BaseCommand {
return n;
}
private void service() throws IOException, Failure {
private void service() throws IOException, PermissionBackendException, Failure {
project = projectControl.getProjectState().getProject();
try {
@@ -100,5 +101,5 @@ public abstract class AbstractGitCommand extends BaseCommand {
}
}
protected abstract void runImpl() throws IOException, Failure;
protected abstract void runImpl() throws IOException, PermissionBackendException, Failure;
}

View File

@@ -17,9 +17,14 @@ package com.google.gerrit.sshd.commands;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.AllowedFormats;
import com.google.gerrit.server.change.ArchiveFormat;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.sshd.AbstractGitCommand;
import com.google.inject.Inject;
import java.io.IOException;
@@ -115,6 +120,8 @@ public class UploadArchive extends AbstractGitCommand {
private List<String> path;
}
@Inject private PermissionBackend permissionBackend;
@Inject private IdentifiedUser user;
@Inject private AllowedFormats allowedFormats;
@Inject private ReviewDb db;
private Options options = new Options();
@@ -156,7 +163,7 @@ public class UploadArchive extends AbstractGitCommand {
}
@Override
protected void runImpl() throws IOException, Failure {
protected void runImpl() throws IOException, PermissionBackendException, Failure {
PacketLineOut packetOut = new PacketLineOut(out);
packetOut.setFlushOnEnd(true);
packetOut.writeString("ACK");
@@ -177,8 +184,8 @@ public class UploadArchive extends AbstractGitCommand {
throw new Failure(4, "fatal: reference not found");
}
// Verify the user has permissions to read the specified reference
if (!projectControl.allRefsAreVisible() && !canRead(treeId)) {
// Verify the user has permissions to read the specified tree.
if (!canRead(treeId)) {
throw new Failure(5, "fatal: cannot perform upload-archive operation");
}
@@ -235,10 +242,16 @@ public class UploadArchive extends AbstractGitCommand {
return Collections.emptyMap();
}
private boolean canRead(ObjectId revId) throws IOException {
try (RevWalk rw = new RevWalk(repo)) {
RevCommit commit = rw.parseCommit(revId);
return projectControl.canReadCommit(db, repo, commit);
private boolean canRead(ObjectId revId) throws IOException, PermissionBackendException {
try {
permissionBackend.user(user).project(project.getNameKey()).check(ProjectPermission.READ);
return true;
} catch (AuthException e) {
// Check reachability of the specific revision.
try (RevWalk rw = new RevWalk(repo)) {
RevCommit commit = rw.parseCommit(revId);
return projectControl.canReadCommit(db, repo, commit);
}
}
}
}