Convert ProjectControl.isVisible to PermissionBackend
Remove isReadable and isVisible, relying only on PermissionBackend.
Change-Id: I478119601acfe661da4c164a55e3642b020c4cd7
diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/EventBroker.java b/gerrit-server/src/main/java/com/google/gerrit/common/EventBroker.java
index 87cfea1..96c70c0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/common/EventBroker.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/common/EventBroker.java
@@ -16,6 +16,7 @@
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
@@ -28,6 +29,9 @@
import com.google.gerrit.server.events.ProjectEvent;
import com.google.gerrit.server.events.RefEvent;
import com.google.gerrit.server.notedb.ChangeNotes;
+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.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectState;
@@ -54,6 +58,7 @@
/** Listeners to receive all changes as they happen. */
protected final DynamicSet<EventListener> unrestrictedListeners;
+ private final PermissionBackend permissionBackend;
protected final ProjectCache projectCache;
protected final ChangeNotes.Factory notesFactory;
@@ -64,11 +69,13 @@
public EventBroker(
DynamicSet<UserScopedEventListener> listeners,
DynamicSet<EventListener> unrestrictedListeners,
+ PermissionBackend permissionBackend,
ProjectCache projectCache,
ChangeNotes.Factory notesFactory,
Provider<ReviewDb> dbProvider) {
this.listeners = listeners;
this.unrestrictedListeners = unrestrictedListeners;
+ this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
this.notesFactory = notesFactory;
this.dbProvider = dbProvider;
@@ -137,11 +144,12 @@
}
protected boolean isVisibleTo(Project.NameKey project, CurrentUser user) {
- ProjectState pe = projectCache.get(project);
- if (pe == null) {
+ try {
+ permissionBackend.user(user).project(project).check(ProjectPermission.ACCESS);
+ return true;
+ } catch (AuthException | PermissionBackendException e) {
return false;
}
- return pe.controlFor(user).isVisible();
}
protected boolean isVisibleTo(Change change, CurrentUser user) throws OrmException {
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 73d5383..3aaca53 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
@@ -476,7 +476,7 @@
// If the user lacks READ permission, some references may be filtered and hidden from view.
// Check objects mentioned inside the incoming pack file are reachable from visible refs.
try {
- permissions.check(ProjectPermission.READ);
+ permissionBackend.user(user).project(project.getNameKey()).check(ProjectPermission.READ);
} catch (AuthException e) {
rp.setCheckReferencedObjectsAreReachable(receiveConfig.checkReferencedObjectsAreReachable);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
index d4c5edc..916637f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
@@ -76,9 +76,6 @@
/** Access control management for a user accessing a project's data. */
public class ProjectControl {
- public static final int VISIBLE = 1 << 0;
- public static final int OWNER = 1 << 1;
-
private static final Logger log = LoggerFactory.getLogger(ProjectControl.class);
public static class GenericFactory {
@@ -97,18 +94,6 @@
}
return p.controlFor(user);
}
-
- public ProjectControl validateFor(Project.NameKey nameKey, int need, CurrentUser user)
- throws NoSuchProjectException, IOException {
- final ProjectControl c = controlFor(nameKey, user);
- if ((need & VISIBLE) == VISIBLE && c.isVisible()) {
- return c;
- }
- if ((need & OWNER) == OWNER && c.isOwner()) {
- return c;
- }
- throw new NoSuchProjectException(nameKey);
- }
}
public static class Factory {
@@ -122,26 +107,6 @@
public ProjectControl controlFor(final Project.NameKey nameKey) throws NoSuchProjectException {
return userCache.get().get(nameKey);
}
-
- public ProjectControl validateFor(final Project.NameKey nameKey) throws NoSuchProjectException {
- return validateFor(nameKey, VISIBLE);
- }
-
- public ProjectControl ownerFor(final Project.NameKey nameKey) throws NoSuchProjectException {
- return validateFor(nameKey, OWNER);
- }
-
- public ProjectControl validateFor(final Project.NameKey nameKey, final int need)
- throws NoSuchProjectException {
- final ProjectControl c = controlFor(nameKey);
- if ((need & VISIBLE) == VISIBLE && c.isVisible()) {
- return c;
- }
- if ((need & OWNER) == OWNER && c.isOwner()) {
- return c;
- }
- throw new NoSuchProjectException(nameKey);
- }
}
public interface AssistedFactory {
@@ -280,21 +245,6 @@
return getProject().getState().equals(com.google.gerrit.extensions.client.ProjectState.HIDDEN);
}
- /**
- * Returns whether the project is readable to the current user. Note that the project could still
- * be hidden.
- */
- public boolean isReadable() {
- return (user.isInternalUser() || canPerformOnAnyRef(Permission.READ));
- }
-
- /**
- * Returns whether the project is accessible to the current user, i.e. readable and not hidden.
- */
- public boolean isVisible() {
- return isReadable() && !isHidden();
- }
-
public boolean canAddRefs() {
return (canPerformOnAnyRef(Permission.CREATE) || isOwnerAnyRef());
}
@@ -312,16 +262,11 @@
return false;
}
- /** Can this user see all the refs in this projects? */
- public boolean allRefsAreVisible() {
- return allRefsAreVisible(Collections.<String>emptySet());
- }
-
public boolean allRefsAreVisible(Set<String> ignore) {
return user.isInternalUser() || canPerformOnAllRefs(Permission.READ, ignore);
}
- /** Is this user a project owner? Ownership does not imply {@link #isVisible()} */
+ /** Is this user a project owner? */
public boolean isOwner() {
return (isDeclaredOwner() && !controlForRef("refs/*").isBlocked(Permission.OWNER))
|| user.getCapabilities().isAdmin_DoNotUse();
@@ -609,10 +554,11 @@
private boolean can(ProjectPermission perm) throws PermissionBackendException {
switch (perm) {
case ACCESS:
- return (!isHidden() && isReadable()) || isOwner();
+ return (!isHidden() && (user.isInternalUser() || canPerformOnAnyRef(Permission.READ)))
+ || isOwner();
case READ:
- return (!isHidden() && allRefsAreVisible()) || isOwner();
+ return (!isHidden() && allRefsAreVisible(Collections.emptySet())) || isOwner();
}
throw new PermissionBackendException(perm + " unsupported");
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
index 60d6f71..ebd5b49 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
@@ -58,6 +58,7 @@
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.index.SingleVersionModule.SingleVersionListener;
+import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.schema.SchemaCreator;
@@ -110,12 +111,14 @@
assertThat(u.controlForRef(ref).isOwner()).named("NOT OWN " + ref).isFalse();
}
- private void assertCanRead(ProjectControl u) {
- assertThat(u.isVisible()).named("can read").isTrue();
+ private void assertCanAccess(ProjectControl u) {
+ boolean access = u.asForProject().testOrFalse(ProjectPermission.ACCESS);
+ assertThat(access).named("can access").isTrue();
}
- private void assertCannotRead(ProjectControl u) {
- assertThat(u.isVisible()).named("cannot read").isFalse();
+ private void assertAccessDenied(ProjectControl u) {
+ boolean access = u.asForProject().testOrFalse(ProjectPermission.ACCESS);
+ assertThat(access).named("cannot access").isFalse();
}
private void assertCanRead(String ref, ProjectControl u) {
@@ -443,13 +446,13 @@
public void inheritDuplicateSections() throws Exception {
allow(parent, READ, ADMIN, "refs/*");
allow(local, READ, DEVS, "refs/heads/*");
- assertCanRead(user(local, "a", ADMIN));
+ assertCanAccess(user(local, "a", ADMIN));
local = new ProjectConfig(localKey);
local.load(newRepository(localKey));
local.getProject().setParentName(parentKey);
allow(local, READ, DEVS, "refs/*");
- assertCanRead(user(local, "d", DEVS));
+ assertCanAccess(user(local, "d", DEVS));
}
@Test
@@ -457,7 +460,7 @@
allow(parent, READ, REGISTERED_USERS, "refs/*");
deny(local, READ, REGISTERED_USERS, "refs/*");
- assertCannotRead(user(local));
+ assertAccessDenied(user(local));
}
@Test
@@ -466,7 +469,7 @@
deny(local, READ, REGISTERED_USERS, "refs/heads/*");
ProjectControl u = user(local);
- assertCanRead(u);
+ assertCanAccess(u);
assertCanRead("refs/master", u);
assertCanRead("refs/tags/foobar", u);
assertCanRead("refs/heads/master", u);
@@ -479,7 +482,7 @@
allow(local, READ, REGISTERED_USERS, "refs/heads/*");
ProjectControl u = user(local);
- assertCanRead(u);
+ assertCanAccess(u);
assertCannotRead("refs/foobar", u);
assertCannotRead("refs/tags/foobar", u);
assertCanRead("refs/heads/foobar", u);
diff --git a/plugins/replication b/plugins/replication
index 032d195..a6cba7b 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 032d195285b11213821ed695ecadce51238d9bb8
+Subproject commit a6cba7b3ab4aa1e1ad0f0deb95078aacaa29b37d
