diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java index 8dd418e990..0aa80e2c5c 100644 --- a/java/com/google/gerrit/server/permissions/ChangeControl.java +++ b/java/com/google/gerrit/server/permissions/ChangeControl.java @@ -318,6 +318,13 @@ class ChangeControl { return user().equals(user) ? this : forUser(user).asForChange(cd, db); } + @Override + public String resourcePath() { + return String.format( + "/projects/%s/+changes/%s", + getProjectControl().getProjectState().getName(), changeData().getId().get()); + } + @Override public void check(ChangePermissionOrLabel perm) throws AuthException, PermissionBackendException { diff --git a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java index 152fdf967d..01d5fa3f2f 100644 --- a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java +++ b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java @@ -70,6 +70,11 @@ public class DefaultPermissionBackend extends PermissionBackend { this.user = checkNotNull(user, "user"); } + @Override + public CurrentUser user() { + return user; + } + @Override public ForProject project(Project.NameKey project) { try { diff --git a/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java b/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java index 4c6e6753fc..5144833c7c 100644 --- a/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java +++ b/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java @@ -72,11 +72,22 @@ public class FailedPermissionBackend { return this; } + @Override + public CurrentUser user() { + throw new UnsupportedOperationException("FailedPermissionBackend is not scoped to user"); + } + @Override public ForProject user(CurrentUser user) { return this; } + @Override + public String resourcePath() { + throw new UnsupportedOperationException( + "FailedPermissionBackend is not scoped to a resource"); + } + @Override public ForRef ref(String ref) { return new FailedRef(message, cause); @@ -108,11 +119,22 @@ public class FailedPermissionBackend { return this; } + @Override + public CurrentUser user() { + throw new UnsupportedOperationException("FailedPermissionBackend is not scoped to user"); + } + @Override public ForRef user(CurrentUser user) { return this; } + @Override + public String resourcePath() { + throw new UnsupportedOperationException( + "FailedPermissionBackend is not scoped to a resource"); + } + @Override public ForChange change(ChangeData cd) { return new FailedChange(message, cause); @@ -159,6 +181,12 @@ public class FailedPermissionBackend { return this; } + @Override + public String resourcePath() { + throw new UnsupportedOperationException( + "FailedPermissionBackend is not scoped to a resource"); + } + @Override public void check(ChangePermissionOrLabel perm) throws PermissionBackendException { throw new PermissionBackendException(message, cause); diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java index 2a652d9605..0cd9a754be 100644 --- a/java/com/google/gerrit/server/permissions/PermissionBackend.java +++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java @@ -135,6 +135,9 @@ public abstract class PermissionBackend { /** PermissionBackend scoped to a specific user. */ public abstract static class WithUser extends AcceptsReviewDb { + /** @return user this instance is scoped to. */ + public abstract CurrentUser user(); + /** @return instance scoped for the specified project. */ public abstract ForProject project(Project.NameKey project); @@ -247,6 +250,12 @@ public abstract class PermissionBackend { /** PermissionBackend scoped to a user and project. */ public abstract static class ForProject extends AcceptsReviewDb { + /** @return user this instance is scoped to. */ + public abstract CurrentUser user(); + + /** @return fully qualified resource path that this instance is scoped to. */ + public abstract String resourcePath(); + /** @return new instance rescoped to same project, but different {@code user}. */ public abstract ForProject user(CurrentUser user); @@ -304,6 +313,12 @@ public abstract class PermissionBackend { /** PermissionBackend scoped to a user, project and reference. */ public abstract static class ForRef extends AcceptsReviewDb { + /** @return user this instance is scoped to. */ + public abstract CurrentUser user(); + + /** @return fully qualified resource path that this instance is scoped to. */ + public abstract String resourcePath(); + /** @return new instance rescoped to same reference, but different {@code user}. */ public abstract ForRef user(CurrentUser user); @@ -359,6 +374,9 @@ public abstract class PermissionBackend { /** @return user this instance is scoped to. */ public abstract CurrentUser user(); + /** @return fully qualified resource path that this instance is scoped to. */ + public abstract String resourcePath(); + /** @return new instance rescoped to same change, but different {@code user}. */ public abstract ForChange user(CurrentUser user); diff --git a/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java b/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java index 8d66e50546..58e78b905a 100644 --- a/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java +++ b/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java @@ -17,6 +17,8 @@ package com.google.gerrit.server.permissions; import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission; import com.google.gerrit.extensions.conditions.BooleanCondition; import com.google.gerrit.extensions.conditions.PrivateInternals_BooleanCondition; +import com.google.gerrit.server.CurrentUser; +import java.util.Objects; /** {@link BooleanCondition} to evaluate a permission. */ public abstract class PermissionBackendCondition @@ -64,6 +66,20 @@ public abstract class PermissionBackendCondition public String toString() { return "PermissionBackendCondition.WithUser(" + perm + ")"; } + + @Override + public int hashCode() { + return Objects.hash(perm, hashForUser(impl.user())); + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof WithUser)) { + return false; + } + WithUser other = (WithUser) obj; + return Objects.equals(perm, other.perm) && usersAreEqual(impl.user(), other.impl.user()); + } } public static class ForProject extends PermissionBackendCondition { @@ -92,6 +108,22 @@ public abstract class PermissionBackendCondition public String toString() { return "PermissionBackendCondition.ForProject(" + perm + ")"; } + + @Override + public int hashCode() { + return Objects.hash(perm, impl.resourcePath(), hashForUser(impl.user())); + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof ForProject)) { + return false; + } + ForProject other = (ForProject) obj; + return Objects.equals(perm, other.perm) + && Objects.equals(impl.resourcePath(), other.impl.resourcePath()) + && usersAreEqual(impl.user(), other.impl.user()); + } } public static class ForRef extends PermissionBackendCondition { @@ -120,6 +152,22 @@ public abstract class PermissionBackendCondition public String toString() { return "PermissionBackendCondition.ForRef(" + perm + ")"; } + + @Override + public int hashCode() { + return Objects.hash(perm, impl.resourcePath(), hashForUser(impl.user())); + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof ForRef)) { + return false; + } + ForRef other = (ForRef) obj; + return Objects.equals(perm, other.perm) + && Objects.equals(impl.resourcePath(), other.impl.resourcePath()) + && usersAreEqual(impl.user(), other.impl.user()); + } } public static class ForChange extends PermissionBackendCondition { @@ -148,5 +196,35 @@ public abstract class PermissionBackendCondition public String toString() { return "PermissionBackendCondition.ForChange(" + perm + ")"; } + + @Override + public int hashCode() { + return Objects.hash(perm, impl.resourcePath(), hashForUser(impl.user())); + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof ForChange)) { + return false; + } + ForChange other = (ForChange) obj; + return Objects.equals(perm, other.perm) + && Objects.equals(impl.resourcePath(), other.impl.resourcePath()) + && usersAreEqual(impl.user(), other.impl.user()); + } + } + + private static int hashForUser(CurrentUser user) { + if (!user.isIdentifiedUser()) { + return 0; + } + return user.getAccountId().get(); + } + + private static boolean usersAreEqual(CurrentUser user1, CurrentUser user2) { + if (user1.isIdentifiedUser() && user2.isIdentifiedUser()) { + return user1.getAccountId().equals(user2.getAccountId()); + } + return false; } } diff --git a/java/com/google/gerrit/server/permissions/ProjectControl.java b/java/com/google/gerrit/server/permissions/ProjectControl.java index d37b25465f..a3bb4244a2 100644 --- a/java/com/google/gerrit/server/permissions/ProjectControl.java +++ b/java/com/google/gerrit/server/permissions/ProjectControl.java @@ -312,11 +312,21 @@ class ProjectControl { } private class ForProjectImpl extends ForProject { + @Override + public CurrentUser user() { + return getUser(); + } + @Override public ForProject user(CurrentUser user) { return forUser(user).asForProject().database(db); } + @Override + public String resourcePath() { + return "/projects/" + getProjectState().getName(); + } + @Override public ForRef ref(String ref) { return controlForRef(ref).asForRef().database(db); diff --git a/java/com/google/gerrit/server/permissions/RefControl.java b/java/com/google/gerrit/server/permissions/RefControl.java index 66d1db84cc..94f1acfe2f 100644 --- a/java/com/google/gerrit/server/permissions/RefControl.java +++ b/java/com/google/gerrit/server/permissions/RefControl.java @@ -431,11 +431,22 @@ class RefControl { } private class ForRefImpl extends ForRef { + @Override + public CurrentUser user() { + return getUser(); + } + @Override public ForRef user(CurrentUser user) { return forUser(user).asForRef().database(db); } + @Override + public String resourcePath() { + return String.format( + "/projects/%s/+refs/%s", getProjectControl().getProjectState().getName(), refName); + } + @Override public ForChange change(ChangeData cd) { try { diff --git a/javatests/com/google/gerrit/acceptance/server/permissions/PermissionBackendConditionIT.java b/javatests/com/google/gerrit/acceptance/server/permissions/PermissionBackendConditionIT.java new file mode 100644 index 0000000000..720eeeda17 --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/server/permissions/PermissionBackendConditionIT.java @@ -0,0 +1,170 @@ +// Copyright (C) 2018 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.acceptance.server.permissions; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.extensions.conditions.BooleanCondition; +import com.google.gerrit.reviewdb.client.Branch; +import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.permissions.ChangePermission; +import com.google.gerrit.server.permissions.GlobalPermission; +import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.ProjectPermission; +import com.google.gerrit.server.permissions.RefPermission; +import com.google.gerrit.server.query.change.ChangeData; +import com.google.inject.Inject; +import org.junit.Test; + +public class PermissionBackendConditionIT extends AbstractDaemonTest { + + @Inject PermissionBackend pb; + + @Test + public void globalPermissions_sameUserAndPermissionEquals() throws Exception { + BooleanCondition cond1 = pb.user(user()).testCond(GlobalPermission.CREATE_GROUP); + BooleanCondition cond2 = pb.user(user()).testCond(GlobalPermission.CREATE_GROUP); + assertEquals(cond1, cond2); + assertEquals(cond1.hashCode(), cond2.hashCode()); + } + + @Test + public void globalPermissions_differentPermissionDoesNotEquals() throws Exception { + BooleanCondition cond1 = pb.user(user()).testCond(GlobalPermission.CREATE_GROUP); + BooleanCondition cond2 = pb.user(user()).testCond(GlobalPermission.ACCESS_DATABASE); + assertNotEquals(cond1, cond2); + assertNotEquals(cond1.hashCode(), cond2.hashCode()); + } + + @Test + public void globalPermissions_differentUserDoesNotEqual() throws Exception { + BooleanCondition cond1 = pb.user(user()).testCond(GlobalPermission.CREATE_GROUP); + BooleanCondition cond2 = pb.user(admin()).testCond(GlobalPermission.CREATE_GROUP); + assertNotEquals(cond1, cond2); + assertNotEquals(cond1.hashCode(), cond2.hashCode()); + } + + @Test + public void changePermissions_sameResourceAndUserEquals() throws Exception { + ChangeData change = createChange().getChange(); + BooleanCondition cond1 = pb.user(user()).change(change).testCond(ChangePermission.READ); + BooleanCondition cond2 = pb.user(user()).change(change).testCond(ChangePermission.READ); + + assertEquals(cond1, cond2); + assertEquals(cond1.hashCode(), cond2.hashCode()); + } + + @Test + public void changePermissions_sameResourceDifferentUserDoesNotEqual() throws Exception { + ChangeData change = createChange().getChange(); + BooleanCondition cond1 = pb.user(user()).change(change).testCond(ChangePermission.READ); + BooleanCondition cond2 = pb.user(admin()).change(change).testCond(ChangePermission.READ); + + assertNotEquals(cond1, cond2); + assertNotEquals(cond1.hashCode(), cond2.hashCode()); + } + + @Test + public void changePermissions_differentResourceSameUserDoesNotEqual() throws Exception { + ChangeData change1 = createChange().getChange(); + ChangeData change2 = createChange().getChange(); + BooleanCondition cond1 = pb.user(user()).change(change1).testCond(ChangePermission.READ); + BooleanCondition cond2 = pb.user(user()).change(change2).testCond(ChangePermission.READ); + + assertNotEquals(cond1, cond2); + assertNotEquals(cond1.hashCode(), cond2.hashCode()); + } + + @Test + public void projectPermissions_sameResourceAndUserEquals() throws Exception { + BooleanCondition cond1 = pb.user(user()).project(project).testCond(ProjectPermission.READ); + BooleanCondition cond2 = pb.user(user()).project(project).testCond(ProjectPermission.READ); + + assertEquals(cond1, cond2); + assertEquals(cond1.hashCode(), cond2.hashCode()); + } + + @Test + public void projectPermissions_sameResourceDifferentUserDoesNotEqual() throws Exception { + BooleanCondition cond1 = pb.user(user()).project(project).testCond(ProjectPermission.READ); + BooleanCondition cond2 = pb.user(admin()).project(project).testCond(ProjectPermission.READ); + + assertNotEquals(cond1, cond2); + assertNotEquals(cond1.hashCode(), cond2.hashCode()); + } + + @Test + public void projectPermissions_differentResourceSameUserDoesNotEqual() throws Exception { + Project.NameKey project2 = createProject("p2"); + BooleanCondition cond1 = pb.user(user()).project(project).testCond(ProjectPermission.READ); + BooleanCondition cond2 = pb.user(user()).project(project2).testCond(ProjectPermission.READ); + + assertNotEquals(cond1, cond2); + assertNotEquals(cond1.hashCode(), cond2.hashCode()); + } + + @Test + public void refPermissions_sameResourceAndUserEquals() throws Exception { + Branch.NameKey branch = new Branch.NameKey(project, "branch"); + BooleanCondition cond1 = pb.user(user()).ref(branch).testCond(RefPermission.READ); + BooleanCondition cond2 = pb.user(user()).ref(branch).testCond(RefPermission.READ); + + assertEquals(cond1, cond2); + assertEquals(cond1.hashCode(), cond2.hashCode()); + } + + @Test + public void refPermissions_sameResourceAndDifferentUserDoesNotEqual() throws Exception { + Branch.NameKey branch = new Branch.NameKey(project, "branch"); + BooleanCondition cond1 = pb.user(user()).ref(branch).testCond(RefPermission.READ); + BooleanCondition cond2 = pb.user(admin()).ref(branch).testCond(RefPermission.READ); + + assertNotEquals(cond1, cond2); + assertNotEquals(cond1.hashCode(), cond2.hashCode()); + } + + @Test + public void refPermissions_differentResourceAndSameUserDoesNotEqual() throws Exception { + Branch.NameKey branch1 = new Branch.NameKey(project, "branch"); + Branch.NameKey branch2 = new Branch.NameKey(project, "branch2"); + BooleanCondition cond1 = pb.user(user()).ref(branch1).testCond(RefPermission.READ); + BooleanCondition cond2 = pb.user(user()).ref(branch2).testCond(RefPermission.READ); + + assertNotEquals(cond1, cond2); + assertNotEquals(cond1.hashCode(), cond2.hashCode()); + } + + @Test + public void refPermissions_differentResourceAndSameUserDoesNotEqual2() throws Exception { + Branch.NameKey branch1 = new Branch.NameKey(project, "branch"); + Branch.NameKey branch2 = new Branch.NameKey(createProject("p2"), "branch"); + BooleanCondition cond1 = pb.user(user()).ref(branch1).testCond(RefPermission.READ); + BooleanCondition cond2 = pb.user(user()).ref(branch2).testCond(RefPermission.READ); + + assertNotEquals(cond1, cond2); + assertNotEquals(cond1.hashCode(), cond2.hashCode()); + } + + private CurrentUser user() { + return identifiedUserFactory.create(user.id); + } + + private CurrentUser admin() { + return identifiedUserFactory.create(admin.id); + } +}