opendev/gerrit
Code Issues Proposed changes

Remove PermissionBackend#user(Provider<CurrentUser>)

Browse Source 
Checking permissions of users that aren't the caller on the current request
can have implications on the security of the system. The most prominent
one is creating a group-oracle.

To limit the cases where we could potentially expose Gerrit to these
threats, PermissionBackend removes the method that was operating solely
on the provider of the current user.

Change-Id: I601ea1200a15a5f262ca0770b23cc1c7bee126b1
This commit is contained in:
Patrick Hiesel
2018-02-20 12:19:33 +01:00
parent 659ea71969
commit 4bdef6c030
78 changed files with 124 additions and 245 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
java/com/google/gerrit/server/args4j/ProjectHandler.java
View File

@@ -17,7 +17,6 @@ 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;
@@ -25,7 +24,6 @@ import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
import org.kohsuke.args4j.CmdLineException;
@@ -42,20 +40,17 @@ public class ProjectHandler extends OptionHandler<ProjectState> {
private final ProjectCache projectCache;
private final PermissionBackend permissionBackend;
private final Provider<CurrentUser> user;
@Inject
public ProjectHandler(
ProjectCache projectCache,
PermissionBackend permissionBackend,
Provider<CurrentUser> user,
@Assisted final CmdLineParser parser,
@Assisted final OptionDef option,
@Assisted final Setter<ProjectState> setter) {
super(parser, option, setter);
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
this.user = user;
}
@Override
@@ -84,7 +79,7 @@ public class ProjectHandler extends OptionHandler<ProjectState> {
if (state == null) {
throw new CmdLineException(owner, String.format("project %s not found", nameWithoutSuffix));
}
permissionBackend.user(user).project(nameKey).check(ProjectPermission.ACCESS);
permissionBackend.currentUser().project(nameKey).check(ProjectPermission.ACCESS);
} catch (AuthException e) {
throw new CmdLineException(owner, new NoSuchProjectException(nameKey).getMessage());
} catch (PermissionBackendException | IOException e) {