Merge changes from topic "group-ref-advertisement"
* changes: Grant read access on group branches Advertise group refs to group owners Advertise refs/users/self if user branch is visible due to Access Database Test advertisement of user refs VisibleRefFilter: Skip fast advertising of all refs for All-Users VisibleRefFilter: Don't treat refs/meta/external-ids as meta ref
This commit is contained in:
@@ -224,6 +224,10 @@ public class RefNames {
|
||||
return ref.startsWith(REFS_USERS);
|
||||
}
|
||||
|
||||
public static boolean isRefsGroups(String ref) {
|
||||
return ref.startsWith(REFS_GROUPS);
|
||||
}
|
||||
|
||||
static Integer parseShardedRefPart(String name) {
|
||||
if (name == null) {
|
||||
return null;
|
||||
|
||||
@@ -144,6 +144,7 @@ public class GroupControl {
|
||||
return isOwner;
|
||||
}
|
||||
|
||||
// Keep this logic in sync with VisibleRefFilter#isOwner(...).
|
||||
if (group instanceof GroupDescription.Internal) {
|
||||
AccountGroup.UUID ownerUUID = ((GroupDescription.Internal) group).getOwnerGroupUUID();
|
||||
isOwner = getUser().getEffectiveGroups().contains(ownerUUID) || canAdministrateServer();
|
||||
|
||||
@@ -14,6 +14,7 @@
|
||||
|
||||
package com.google.gerrit.server.git;
|
||||
|
||||
import static com.google.common.base.Preconditions.checkNotNull;
|
||||
import static com.google.gerrit.reviewdb.client.RefNames.REFS_CACHE_AUTOMERGE;
|
||||
import static com.google.gerrit.reviewdb.client.RefNames.REFS_CHANGES;
|
||||
import static com.google.gerrit.reviewdb.client.RefNames.REFS_CONFIG;
|
||||
@@ -23,6 +24,7 @@ import static java.util.stream.Collectors.toMap;
|
||||
import com.google.gerrit.common.Nullable;
|
||||
import com.google.gerrit.extensions.restapi.AuthException;
|
||||
import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||
import com.google.gerrit.reviewdb.client.Branch;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.client.Project;
|
||||
@@ -30,6 +32,8 @@ import com.google.gerrit.reviewdb.client.RefNames;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.CurrentUser;
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.account.GroupCache;
|
||||
import com.google.gerrit.server.group.InternalGroup;
|
||||
import com.google.gerrit.server.notedb.ChangeNotes;
|
||||
import com.google.gerrit.server.notedb.ChangeNotes.Factory.ChangeNotesResult;
|
||||
import com.google.gerrit.server.permissions.ChangePermission;
|
||||
@@ -75,6 +79,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
@Nullable private final SearchingChangeCacheImpl changeCache;
|
||||
private final Provider<ReviewDb> db;
|
||||
private final Provider<CurrentUser> user;
|
||||
private final GroupCache groupCache;
|
||||
private final PermissionBackend permissionBackend;
|
||||
private final PermissionBackend.ForProject perm;
|
||||
private final ProjectState projectState;
|
||||
@@ -90,6 +95,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
@Nullable SearchingChangeCacheImpl changeCache,
|
||||
Provider<ReviewDb> db,
|
||||
Provider<CurrentUser> user,
|
||||
GroupCache groupCache,
|
||||
PermissionBackend permissionBackend,
|
||||
@Assisted ProjectState projectState,
|
||||
@Assisted Repository git) {
|
||||
@@ -98,6 +104,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
this.changeCache = changeCache;
|
||||
this.db = db;
|
||||
this.user = user;
|
||||
this.groupCache = groupCache;
|
||||
this.permissionBackend = permissionBackend;
|
||||
this.perm =
|
||||
permissionBackend.user(user).database(db).project(projectState.getProject().getNameKey());
|
||||
@@ -118,22 +125,29 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
|
||||
PermissionBackend.WithUser withUser = permissionBackend.user(user);
|
||||
PermissionBackend.ForProject forProject = withUser.project(projectState.getNameKey());
|
||||
if (checkProjectPermission(forProject, ProjectPermission.READ)) {
|
||||
return refs;
|
||||
} else if (checkProjectPermission(forProject, ProjectPermission.READ_NO_CONFIG)) {
|
||||
return fastHideRefsMetaConfig(refs);
|
||||
if (!projectState.isAllUsers()) {
|
||||
if (checkProjectPermission(forProject, ProjectPermission.READ)) {
|
||||
return refs;
|
||||
} else if (checkProjectPermission(forProject, ProjectPermission.READ_NO_CONFIG)) {
|
||||
return fastHideRefsMetaConfig(refs);
|
||||
}
|
||||
}
|
||||
|
||||
Account.Id userId;
|
||||
boolean viewMetadata;
|
||||
boolean isAdmin;
|
||||
Account.Id userId;
|
||||
IdentifiedUser identifiedUser;
|
||||
if (user.get().isIdentifiedUser()) {
|
||||
viewMetadata = withUser.testOrFalse(GlobalPermission.ACCESS_DATABASE);
|
||||
IdentifiedUser u = user.get().asIdentifiedUser();
|
||||
userId = u.getAccountId();
|
||||
isAdmin = withUser.testOrFalse(GlobalPermission.ADMINISTRATE_SERVER);
|
||||
identifiedUser = user.get().asIdentifiedUser();
|
||||
userId = identifiedUser.getAccountId();
|
||||
userEditPrefix = RefNames.refsEditPrefix(userId);
|
||||
} else {
|
||||
userId = null;
|
||||
viewMetadata = false;
|
||||
isAdmin = false;
|
||||
userId = null;
|
||||
identifiedUser = null;
|
||||
}
|
||||
|
||||
Map<String, Ref> result = new HashMap<>();
|
||||
@@ -143,6 +157,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
String name = ref.getName();
|
||||
Change.Id changeId;
|
||||
Account.Id accountId;
|
||||
AccountGroup.UUID accountGroupUuid;
|
||||
if (name.startsWith(REFS_CACHE_AUTOMERGE) || (!showMetadata && isMetadata(name))) {
|
||||
continue;
|
||||
} else if (RefNames.isRefsEdit(name)) {
|
||||
@@ -156,10 +171,19 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
result.put(name, ref);
|
||||
}
|
||||
} else if ((accountId = Account.Id.fromRef(name)) != null) {
|
||||
// Account ref is visible only to corresponding account.
|
||||
// Account ref is visible only to the corresponding account.
|
||||
if (viewMetadata || (accountId.equals(userId) && canReadRef(name))) {
|
||||
result.put(name, ref);
|
||||
}
|
||||
} else if ((accountGroupUuid = AccountGroup.UUID.fromRef(name)) != null) {
|
||||
// Group ref is visible only to the corresponding owner group.
|
||||
InternalGroup group = groupCache.get(accountGroupUuid).orElse(null);
|
||||
if (viewMetadata
|
||||
|| (group != null
|
||||
&& isGroupOwner(group, identifiedUser, isAdmin)
|
||||
&& canReadRef(name))) {
|
||||
result.put(name, ref);
|
||||
}
|
||||
} else if (isTag(ref)) {
|
||||
// If its a tag, consider it later.
|
||||
if (ref.getObjectId() != null) {
|
||||
@@ -180,6 +204,12 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
// symbolic we want the control around the final target. If its
|
||||
// not symbolic then getLeaf() is a no-op returning ref itself.
|
||||
result.put(name, ref);
|
||||
} else if (isRefsUsersSelf(ref)) {
|
||||
// viewMetadata allows to see all account refs, hence refs/users/self should be included as
|
||||
// well
|
||||
if (viewMetadata) {
|
||||
result.put(name, ref);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -327,15 +357,17 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
}
|
||||
|
||||
private boolean isMetadata(String name) {
|
||||
return name.startsWith(REFS_CHANGES)
|
||||
|| RefNames.isRefsEdit(name)
|
||||
|| (projectState.isAllUsers() && name.equals(RefNames.REFS_EXTERNAL_IDS));
|
||||
return name.startsWith(REFS_CHANGES) || RefNames.isRefsEdit(name);
|
||||
}
|
||||
|
||||
private static boolean isTag(Ref ref) {
|
||||
return ref.getLeaf().getName().startsWith(Constants.R_TAGS);
|
||||
}
|
||||
|
||||
private static boolean isRefsUsersSelf(Ref ref) {
|
||||
return ref.getName().startsWith(REFS_USERS_SELF);
|
||||
}
|
||||
|
||||
private boolean canReadRef(String ref) {
|
||||
try {
|
||||
perm.ref(ref).check(RefPermission.READ);
|
||||
@@ -364,4 +396,13 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
private boolean isGroupOwner(
|
||||
InternalGroup group, @Nullable IdentifiedUser user, boolean isAdmin) {
|
||||
checkNotNull(group);
|
||||
|
||||
// Keep this logic in sync with GroupControl#isOwner().
|
||||
return isAdmin
|
||||
|| (user != null && user.getEffectiveGroups().contains(group.getOwnerGroupUUID()));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -112,6 +112,12 @@ public class AllUsersCreator {
|
||||
grant(config, defaults, Permission.CREATE, admin);
|
||||
}
|
||||
|
||||
// Grant read permissions on the group branches to all users.
|
||||
// This allows group owners to see the group refs. VisibleRefFilter ensures that read
|
||||
// permissions for non-group-owners are ignored.
|
||||
AccessSection groups = config.getAccessSection(RefNames.REFS_GROUPS + "*", true);
|
||||
grant(config, groups, Permission.READ, false, true, registered);
|
||||
|
||||
config.commit(md);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -35,7 +35,7 @@ import java.util.concurrent.TimeUnit;
|
||||
/** A version of the database schema. */
|
||||
public abstract class SchemaVersion {
|
||||
/** The current schema version. */
|
||||
public static final Class<Schema_163> C = Schema_163.class;
|
||||
public static final Class<Schema_164> C = Schema_164.class;
|
||||
|
||||
public static int getBinaryVersion() {
|
||||
return guessVersion(C);
|
||||
|
||||
85
java/com/google/gerrit/server/schema/Schema_164.java
Normal file
85
java/com/google/gerrit/server/schema/Schema_164.java
Normal file
@@ -0,0 +1,85 @@
|
||||
// Copyright (C) 2017 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.server.schema;
|
||||
|
||||
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
|
||||
import static com.google.gerrit.server.schema.AclUtil.grant;
|
||||
|
||||
import com.google.gerrit.common.data.AccessSection;
|
||||
import com.google.gerrit.common.data.Permission;
|
||||
import com.google.gerrit.reviewdb.client.RefNames;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.GerritPersonIdent;
|
||||
import com.google.gerrit.server.config.AllUsersName;
|
||||
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
|
||||
import com.google.gerrit.server.git.GitRepositoryManager;
|
||||
import com.google.gerrit.server.git.MetaDataUpdate;
|
||||
import com.google.gerrit.server.git.ProjectConfig;
|
||||
import com.google.gerrit.server.group.SystemGroupBackend;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Provider;
|
||||
import java.io.IOException;
|
||||
import java.sql.SQLException;
|
||||
import org.eclipse.jgit.errors.ConfigInvalidException;
|
||||
import org.eclipse.jgit.lib.PersonIdent;
|
||||
import org.eclipse.jgit.lib.Repository;
|
||||
|
||||
/** Grant read on group branches */
|
||||
public class Schema_164 extends SchemaVersion {
|
||||
private static final String COMMIT_MSG = "Grant read permissions on group branches";
|
||||
|
||||
private final GitRepositoryManager repoManager;
|
||||
private final AllUsersName allUsersName;
|
||||
private final SystemGroupBackend systemGroupBackend;
|
||||
private final PersonIdent serverUser;
|
||||
|
||||
@Inject
|
||||
Schema_164(
|
||||
Provider<Schema_163> prior,
|
||||
GitRepositoryManager repoManager,
|
||||
AllUsersName allUsersName,
|
||||
SystemGroupBackend systemGroupBackend,
|
||||
@GerritPersonIdent PersonIdent serverUser) {
|
||||
super(prior);
|
||||
this.repoManager = repoManager;
|
||||
this.allUsersName = allUsersName;
|
||||
this.systemGroupBackend = systemGroupBackend;
|
||||
this.serverUser = serverUser;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException {
|
||||
try (Repository git = repoManager.openRepository(allUsersName);
|
||||
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, git)) {
|
||||
md.getCommitBuilder().setAuthor(serverUser);
|
||||
md.getCommitBuilder().setCommitter(serverUser);
|
||||
md.setMessage(COMMIT_MSG);
|
||||
|
||||
ProjectConfig config = ProjectConfig.read(md);
|
||||
AccessSection groups = config.getAccessSection(RefNames.REFS_GROUPS + "*", true);
|
||||
grant(
|
||||
config,
|
||||
groups,
|
||||
Permission.READ,
|
||||
false,
|
||||
true,
|
||||
systemGroupBackend.getGroup(REGISTERED_USERS));
|
||||
config.commit(md);
|
||||
} catch (IOException | ConfigInvalidException e) {
|
||||
throw new OrmException("Failed to grant read permissions on group branches", e);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -918,11 +918,11 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
@Test
|
||||
public void pushAccountConfigWithPrefEmailThatDoesNotExistAsExtIdToUserBranchForReviewAndSubmit()
|
||||
throws Exception {
|
||||
TestAccount foo = accountCreator.create(name("foo"));
|
||||
TestAccount foo = accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo");
|
||||
String userRef = RefNames.refsUsers(foo.id);
|
||||
accountIndexedCounter.clear();
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, foo);
|
||||
fetch(allUsersRepo, userRef + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
@@ -934,7 +934,7 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
foo.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
@@ -1058,7 +1058,10 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
@Sandboxed
|
||||
public void pushAccountConfigToUserBranchForReviewDeactivateOtherAccount() throws Exception {
|
||||
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
|
||||
|
||||
TestAccount foo = accountCreator.create(name("foo"));
|
||||
assertThat(gApi.accounts().id(foo.id.get()).getActive()).isTrue();
|
||||
String userRef = RefNames.refsUsers(foo.id);
|
||||
@@ -1229,18 +1232,15 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchInvalidPreferredEmailButNotChanged() throws Exception {
|
||||
TestAccount foo = accountCreator.create(name("foo"));
|
||||
TestAccount foo = accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo");
|
||||
String userRef = RefNames.refsUsers(foo.id);
|
||||
|
||||
String noEmail = "no.email";
|
||||
accountsUpdate.create().update(foo.id, a -> a.setPreferredEmail(noEmail));
|
||||
accountIndexedCounter.clear();
|
||||
|
||||
InternalGroup adminGroup =
|
||||
groupCache.get(new AccountGroup.NameKey("Administrators")).orElse(null);
|
||||
grant(allUsers, userRef, Permission.PUSH, false, adminGroup.getGroupUUID());
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
grant(allUsers, userRef, Permission.PUSH, false, REGISTERED_USERS);
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, foo);
|
||||
fetch(allUsersRepo, userRef + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
@@ -1251,7 +1251,7 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
foo.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
@@ -1268,7 +1268,7 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchIfPreferredEmailDoesNotExistAsExtId() throws Exception {
|
||||
TestAccount foo = accountCreator.create(name("foo"));
|
||||
TestAccount foo = accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo");
|
||||
String userRef = RefNames.refsUsers(foo.id);
|
||||
accountIndexedCounter.clear();
|
||||
|
||||
@@ -1276,7 +1276,7 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
groupCache.get(new AccountGroup.NameKey("Administrators")).orElse(null);
|
||||
grant(allUsers, userRef, Permission.PUSH, false, adminGroup.getGroupUUID());
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, foo);
|
||||
fetch(allUsersRepo, userRef + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
@@ -1287,7 +1287,7 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
foo.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
@@ -1326,7 +1326,10 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
@Sandboxed
|
||||
public void pushAccountConfigToUserBranchDeactivateOtherAccount() throws Exception {
|
||||
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
|
||||
|
||||
TestAccount foo = accountCreator.create(name("foo"));
|
||||
assertThat(gApi.accounts().id(foo.id.get()).getActive()).isTrue();
|
||||
String userRef = RefNames.refsUsers(foo.id);
|
||||
|
||||
@@ -33,8 +33,12 @@ import com.google.gerrit.acceptance.PushOneCommit;
|
||||
import com.google.gerrit.acceptance.Sandboxed;
|
||||
import com.google.gerrit.acceptance.TestAccount;
|
||||
import com.google.gerrit.common.TimeUtil;
|
||||
import com.google.gerrit.common.data.AccessSection;
|
||||
import com.google.gerrit.common.data.GroupDescription;
|
||||
import com.google.gerrit.common.data.GroupReference;
|
||||
import com.google.gerrit.common.data.Permission;
|
||||
import com.google.gerrit.common.data.PermissionRule;
|
||||
import com.google.gerrit.common.data.PermissionRule.Action;
|
||||
import com.google.gerrit.extensions.api.changes.ReviewInput;
|
||||
import com.google.gerrit.extensions.api.groups.GroupApi;
|
||||
import com.google.gerrit.extensions.api.groups.GroupInput;
|
||||
@@ -56,7 +60,9 @@ import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||
import com.google.gerrit.reviewdb.client.Project;
|
||||
import com.google.gerrit.reviewdb.client.RefNames;
|
||||
import com.google.gerrit.server.account.GroupBackend;
|
||||
import com.google.gerrit.server.account.GroupIncludeCache;
|
||||
import com.google.gerrit.server.git.ProjectConfig;
|
||||
import com.google.gerrit.server.group.InternalGroup;
|
||||
import com.google.gerrit.server.group.SystemGroupBackend;
|
||||
import com.google.gerrit.server.group.db.Groups;
|
||||
@@ -100,6 +106,7 @@ public class GroupsIT extends AbstractDaemonTest {
|
||||
|
||||
@Inject private Groups groups;
|
||||
@Inject private GroupIncludeCache groupIncludeCache;
|
||||
@Inject private GroupBackend groupBackend;
|
||||
|
||||
@Test
|
||||
public void systemGroupCanBeRetrievedFromIndex() throws Exception {
|
||||
@@ -935,6 +942,38 @@ public class GroupsIT extends AbstractDaemonTest {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void defaultPermissionsOnGroupBranches() throws Exception {
|
||||
assertPermission(
|
||||
allUsers, RefNames.REFS_GROUPS + "*", Permission.READ, groupRef(REGISTERED_USERS));
|
||||
}
|
||||
|
||||
private GroupReference groupRef(AccountGroup.UUID groupUuid) {
|
||||
GroupDescription.Basic groupDescription = groupBackend.get(groupUuid);
|
||||
return new GroupReference(groupDescription.getGroupUUID(), groupDescription.getName());
|
||||
}
|
||||
|
||||
private void assertPermission(
|
||||
Project.NameKey project, String ref, String permission, GroupReference groupReference)
|
||||
throws IOException {
|
||||
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
|
||||
AccessSection accessSection = cfg.getAccessSection(ref);
|
||||
assertThat(accessSection).isNotNull();
|
||||
Permission readPermission = accessSection.getPermission(permission);
|
||||
assertThat(readPermission).isNotNull();
|
||||
assertThat(readPermission.getName()).isEqualTo(permission);
|
||||
assertThat(readPermission.getExclusiveGroup()).isTrue();
|
||||
assertThat(readPermission.getLabel()).isNull();
|
||||
|
||||
PermissionRule rule = readPermission.getRule(groupReference);
|
||||
assertThat(rule).isNotNull();
|
||||
assertThat(rule.getGroup()).isEqualTo(groupReference);
|
||||
assertThat(rule.getAction()).isEqualTo(Action.ALLOW);
|
||||
assertThat(rule.getForce()).isFalse();
|
||||
assertThat(rule.getMin()).isEqualTo(0);
|
||||
assertThat(rule.getMax()).isEqualTo(0);
|
||||
}
|
||||
|
||||
private void pushToGroupBranchForReviewAndSubmit(Project.NameKey project, String expectedError)
|
||||
throws Exception {
|
||||
grantLabel(
|
||||
|
||||
@@ -21,18 +21,23 @@ import static com.google.gerrit.acceptance.GitUtil.fetch;
|
||||
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
|
||||
import static java.util.stream.Collectors.toList;
|
||||
|
||||
import com.google.common.base.Predicates;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.gerrit.acceptance.AbstractDaemonTest;
|
||||
import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
|
||||
import com.google.gerrit.acceptance.GerritConfig;
|
||||
import com.google.gerrit.acceptance.NoHttpd;
|
||||
import com.google.gerrit.acceptance.PushOneCommit;
|
||||
import com.google.gerrit.acceptance.Sandboxed;
|
||||
import com.google.gerrit.acceptance.TestAccount;
|
||||
import com.google.gerrit.common.Nullable;
|
||||
import com.google.gerrit.common.data.AccessSection;
|
||||
import com.google.gerrit.common.data.GlobalCapability;
|
||||
import com.google.gerrit.common.data.Permission;
|
||||
import com.google.gerrit.extensions.api.changes.DraftInput;
|
||||
import com.google.gerrit.extensions.api.groups.GroupInput;
|
||||
import com.google.gerrit.extensions.api.projects.BranchInput;
|
||||
import com.google.gerrit.extensions.restapi.RestApiException;
|
||||
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.client.Patch;
|
||||
@@ -53,11 +58,12 @@ import com.google.gerrit.testing.NoteDbMode;
|
||||
import com.google.gerrit.testing.TestChanges;
|
||||
import com.google.inject.Inject;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.function.Predicate;
|
||||
import org.eclipse.jgit.api.Git;
|
||||
import org.eclipse.jgit.api.LsRemoteCommand;
|
||||
import org.eclipse.jgit.junit.TestRepository;
|
||||
import org.eclipse.jgit.lib.ObjectId;
|
||||
import org.eclipse.jgit.lib.PersonIdent;
|
||||
@@ -75,6 +81,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
|
||||
@Inject private AllUsersName allUsersName;
|
||||
|
||||
private AccountGroup.UUID admins;
|
||||
private AccountGroup.UUID nonInteractiveUsers;
|
||||
|
||||
private ChangeData c1;
|
||||
private ChangeData c2;
|
||||
@@ -88,19 +95,32 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
|
||||
@Before
|
||||
public void setUp() throws Exception {
|
||||
admins = groupCache.get(new AccountGroup.NameKey("Administrators")).orElse(null).getGroupUUID();
|
||||
nonInteractiveUsers =
|
||||
groupCache
|
||||
.get(new AccountGroup.NameKey("Non-Interactive Users"))
|
||||
.orElse(null)
|
||||
.getGroupUUID();
|
||||
setUpPermissions();
|
||||
setUpChanges();
|
||||
}
|
||||
|
||||
private void setUpPermissions() throws Exception {
|
||||
// Remove read permissions for all users besides admin. This method is
|
||||
// idempotent, so is safe to call on every test setup.
|
||||
// Remove read permissions for all users besides admin. This method is idempotent, so is safe
|
||||
// to call on every test setup.
|
||||
ProjectConfig pc = projectCache.checkedGet(allProjects).getConfig();
|
||||
for (AccessSection sec : pc.getAccessSections()) {
|
||||
sec.removePermission(Permission.READ);
|
||||
}
|
||||
Util.allow(pc, Permission.READ, admins, "refs/*");
|
||||
saveProjectConfig(allProjects, pc);
|
||||
|
||||
// Remove all read permissions on All-Users. This method is idempotent, so is safe to call on
|
||||
// every test setup.
|
||||
pc = projectCache.checkedGet(allUsers).getConfig();
|
||||
for (AccessSection sec : pc.getAccessSections()) {
|
||||
sec.removePermission(Permission.READ);
|
||||
}
|
||||
saveProjectConfig(allUsers, pc);
|
||||
}
|
||||
|
||||
private static String changeRefPrefix(Change.Id id) {
|
||||
@@ -451,23 +471,114 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
|
||||
assertThat(getReceivePackRefs().additionalHaves()).containsExactly(obj(c4, 1));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void advertisedReferencesDontShowUserBranchWithoutRead() throws Exception {
|
||||
TestRepository<?> userTestRepository = cloneProject(allUsers, user);
|
||||
try (Git git = userTestRepository.git()) {
|
||||
assertThat(getUserRefs(git)).isEmpty();
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void advertisedReferencesOmitUserBranchesOfOtherUsers() throws Exception {
|
||||
allow(allUsersName, RefNames.REFS_USERS + "*", Permission.READ, REGISTERED_USERS);
|
||||
TestRepository<?> userTestRepository = cloneProject(allUsers, user);
|
||||
try (Git git = userTestRepository.git()) {
|
||||
assertThat(getUserRefs(git))
|
||||
.containsExactly(RefNames.REFS_USERS_SELF, RefNames.refsUsers(user.id));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void advertisedReferencesIncludeAllUserBranchesWithAccessDatabase() throws Exception {
|
||||
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
|
||||
try {
|
||||
TestRepository<?> userTestRepository = cloneProject(allUsers, user);
|
||||
try (Git git = userTestRepository.git()) {
|
||||
assertThat(getUserRefs(git))
|
||||
.containsExactly(
|
||||
RefNames.REFS_USERS_SELF,
|
||||
RefNames.refsUsers(user.id),
|
||||
RefNames.refsUsers(admin.id));
|
||||
}
|
||||
} finally {
|
||||
removeGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
@Sandboxed
|
||||
@GerritConfig(name = "user.writeGroupsToNoteDb", value = "true")
|
||||
public void advertisedReferencesDontShowGroupBranchToOwnerWithoutRead() throws Exception {
|
||||
createSelfOwnedGroup("Foos", user);
|
||||
TestRepository<?> userTestRepository = cloneProject(allUsers, user);
|
||||
try (Git git = userTestRepository.git()) {
|
||||
assertThat(getGroupRefs(git)).isEmpty();
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
@Sandboxed
|
||||
@GerritConfig(name = "user.writeGroupsToNoteDb", value = "true")
|
||||
public void advertisedReferencesOmitGroupBranchesOfNonOwnedGroups() throws Exception {
|
||||
allow(allUsersName, RefNames.REFS_GROUPS + "*", Permission.READ, REGISTERED_USERS);
|
||||
AccountGroup.UUID users = createGroup("Users", admins, user);
|
||||
AccountGroup.UUID foos = createGroup("Foos", users);
|
||||
AccountGroup.UUID bars = createSelfOwnedGroup("Bars", user);
|
||||
TestRepository<?> userTestRepository = cloneProject(allUsers, user);
|
||||
try (Git git = userTestRepository.git()) {
|
||||
assertThat(getGroupRefs(git))
|
||||
.containsExactly(RefNames.refsGroups(foos), RefNames.refsGroups(bars));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
@Sandboxed
|
||||
@GerritConfig(name = "user.writeGroupsToNoteDb", value = "true")
|
||||
public void advertisedReferencesIncludeAllGroupBranchesWithAccessDatabase() throws Exception {
|
||||
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
|
||||
AccountGroup.UUID users = createGroup("Users", admins);
|
||||
TestRepository<?> userTestRepository = cloneProject(allUsers, user);
|
||||
try (Git git = userTestRepository.git()) {
|
||||
assertThat(getGroupRefs(git))
|
||||
.containsExactly(
|
||||
RefNames.refsGroups(admins),
|
||||
RefNames.refsGroups(nonInteractiveUsers),
|
||||
RefNames.refsGroups(users));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
@GerritConfig(name = "user.writeGroupsToNoteDb", value = "true")
|
||||
public void advertisedReferencesIncludeAllGroupBranchesForAdmins() throws Exception {
|
||||
allow(allUsersName, RefNames.REFS_GROUPS + "*", Permission.READ, REGISTERED_USERS);
|
||||
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ADMINISTRATE_SERVER);
|
||||
try {
|
||||
AccountGroup.UUID users = createGroup("Users", admins);
|
||||
TestRepository<?> userTestRepository = cloneProject(allUsers, user);
|
||||
try (Git git = userTestRepository.git()) {
|
||||
assertThat(getGroupRefs(git))
|
||||
.containsExactly(
|
||||
RefNames.refsGroups(admins),
|
||||
RefNames.refsGroups(nonInteractiveUsers),
|
||||
RefNames.refsGroups(users));
|
||||
}
|
||||
} finally {
|
||||
removeGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ADMINISTRATE_SERVER);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void advertisedReferencesOmitPrivateChangesOfOtherUsers() throws Exception {
|
||||
allow("refs/heads/master", Permission.READ, REGISTERED_USERS);
|
||||
|
||||
TestRepository<?> userTestRepository = cloneProject(project, user);
|
||||
try (Git git = userTestRepository.git()) {
|
||||
LsRemoteCommand lsRemoteCommand = git.lsRemote();
|
||||
String change3RefName = c3.currentPatchSet().getRefName();
|
||||
|
||||
List<String> initialRefNames =
|
||||
lsRemoteCommand.call().stream().map(Ref::getName).collect(toList());
|
||||
assertWithMessage("Precondition violated").that(initialRefNames).contains(change3RefName);
|
||||
assertWithMessage("Precondition violated").that(getRefs(git)).contains(change3RefName);
|
||||
|
||||
gApi.changes().id(c3.getId().get()).setPrivate(true, null);
|
||||
|
||||
List<String> refNames = lsRemoteCommand.call().stream().map(Ref::getName).collect(toList());
|
||||
assertThat(refNames).doesNotContain(change3RefName);
|
||||
assertThat(getRefs(git)).doesNotContain(change3RefName);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -477,17 +588,11 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
|
||||
|
||||
TestRepository<?> userTestRepository = cloneProject(project, user);
|
||||
try (Git git = userTestRepository.git()) {
|
||||
LsRemoteCommand lsRemoteCommand = git.lsRemote();
|
||||
String change3RefName = c3.currentPatchSet().getRefName();
|
||||
|
||||
List<String> initialRefNames =
|
||||
lsRemoteCommand.call().stream().map(Ref::getName).collect(toList());
|
||||
assertWithMessage("Precondition violated").that(initialRefNames).contains(change3RefName);
|
||||
assertWithMessage("Precondition violated").that(getRefs(git)).contains(change3RefName);
|
||||
|
||||
gApi.changes().id(c3.getId().get()).setPrivate(true, null);
|
||||
|
||||
List<String> refNames = lsRemoteCommand.call().stream().map(Ref::getName).collect(toList());
|
||||
assertThat(refNames).contains(change3RefName);
|
||||
assertThat(getRefs(git)).contains(change3RefName);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -553,11 +658,12 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
|
||||
RefNames.refsUsers(admin.id),
|
||||
RefNames.refsUsers(user.id),
|
||||
RefNames.REFS_SEQUENCES + Sequences.NAME_ACCOUNTS,
|
||||
RefNames.REFS_SEQUENCES + Sequences.NAME_GROUPS);
|
||||
RefNames.REFS_SEQUENCES + Sequences.NAME_GROUPS,
|
||||
RefNames.REFS_EXTERNAL_IDS,
|
||||
RefNames.REFS_CONFIG);
|
||||
|
||||
List<String> expectedMetaRefs =
|
||||
new ArrayList<>(
|
||||
ImmutableList.of(RefNames.REFS_EXTERNAL_IDS, mr.getPatchSetId().toRefName()));
|
||||
new ArrayList<>(ImmutableList.of(mr.getPatchSetId().toRefName()));
|
||||
if (NoteDbMode.get() != NoteDbMode.OFF) {
|
||||
expectedMetaRefs.add(changeRefPrefix(mr.getChange().getId()) + "meta");
|
||||
}
|
||||
@@ -565,7 +671,6 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
|
||||
List<String> expectedAllRefs = new ArrayList<>(expectedNonMetaRefs);
|
||||
expectedAllRefs.addAll(expectedMetaRefs);
|
||||
|
||||
setApiUser(user);
|
||||
try (Repository repo = repoManager.openRepository(allUsers)) {
|
||||
Map<String, Ref> all = repo.getAllRefs();
|
||||
|
||||
@@ -587,6 +692,22 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
|
||||
}
|
||||
}
|
||||
|
||||
private List<String> getRefs(Git git) throws Exception {
|
||||
return getRefs(git, Predicates.alwaysTrue());
|
||||
}
|
||||
|
||||
private List<String> getUserRefs(Git git) throws Exception {
|
||||
return getRefs(git, RefNames::isRefsUsers);
|
||||
}
|
||||
|
||||
private List<String> getGroupRefs(Git git) throws Exception {
|
||||
return getRefs(git, RefNames::isRefsGroups);
|
||||
}
|
||||
|
||||
private List<String> getRefs(Git git, Predicate<String> predicate) throws Exception {
|
||||
return git.lsRemote().call().stream().map(Ref::getName).filter(predicate).collect(toList());
|
||||
}
|
||||
|
||||
/**
|
||||
* Assert that refs seen by a non-admin user match expected.
|
||||
*
|
||||
@@ -644,4 +765,20 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
|
||||
assertWithMessage("%s not found in %s", psId, cd.patchSets()).that(ps).isNotNull();
|
||||
return ObjectId.fromString(ps.getRevision().get());
|
||||
}
|
||||
|
||||
private AccountGroup.UUID createSelfOwnedGroup(String name, TestAccount... members)
|
||||
throws RestApiException {
|
||||
return createGroup(name, null, members);
|
||||
}
|
||||
|
||||
private AccountGroup.UUID createGroup(
|
||||
String name, @Nullable AccountGroup.UUID ownerGroup, TestAccount... members)
|
||||
throws RestApiException {
|
||||
GroupInput groupInput = new GroupInput();
|
||||
groupInput.name = name(name);
|
||||
groupInput.ownerId = ownerGroup != null ? ownerGroup.get() : null;
|
||||
groupInput.members =
|
||||
Arrays.stream(members).map(m -> String.valueOf(m.id.get())).collect(toList());
|
||||
return new AccountGroup.UUID(gApi.groups().create(groupInput).get().id);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -273,6 +273,8 @@ public class ExternalIdIT extends AbstractDaemonTest {
|
||||
|
||||
@Test
|
||||
public void pushToExternalIdsBranch() throws Exception {
|
||||
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, RefNames.REFS_EXTERNAL_IDS + ":" + RefNames.REFS_EXTERNAL_IDS);
|
||||
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
|
||||
@@ -296,6 +298,8 @@ public class ExternalIdIT extends AbstractDaemonTest {
|
||||
|
||||
@Test
|
||||
public void pushToExternalIdsBranchRejectsExternalIdWithoutAccountId() throws Exception {
|
||||
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, RefNames.REFS_EXTERNAL_IDS + ":" + RefNames.REFS_EXTERNAL_IDS);
|
||||
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
|
||||
@@ -312,6 +316,8 @@ public class ExternalIdIT extends AbstractDaemonTest {
|
||||
@Test
|
||||
public void pushToExternalIdsBranchRejectsExternalIdWithKeyThatDoesntMatchTheNoteId()
|
||||
throws Exception {
|
||||
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, RefNames.REFS_EXTERNAL_IDS + ":" + RefNames.REFS_EXTERNAL_IDS);
|
||||
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
|
||||
@@ -327,6 +333,8 @@ public class ExternalIdIT extends AbstractDaemonTest {
|
||||
|
||||
@Test
|
||||
public void pushToExternalIdsBranchRejectsExternalIdWithInvalidConfig() throws Exception {
|
||||
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, RefNames.REFS_EXTERNAL_IDS + ":" + RefNames.REFS_EXTERNAL_IDS);
|
||||
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
|
||||
@@ -342,6 +350,8 @@ public class ExternalIdIT extends AbstractDaemonTest {
|
||||
|
||||
@Test
|
||||
public void pushToExternalIdsBranchRejectsExternalIdWithEmptyNote() throws Exception {
|
||||
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, RefNames.REFS_EXTERNAL_IDS + ":" + RefNames.REFS_EXTERNAL_IDS);
|
||||
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
|
||||
@@ -380,6 +390,8 @@ public class ExternalIdIT extends AbstractDaemonTest {
|
||||
|
||||
private void testPushToExternalIdsBranchRejectsInvalidExternalId(ExternalId invalidExtId)
|
||||
throws Exception {
|
||||
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, RefNames.REFS_EXTERNAL_IDS + ":" + RefNames.REFS_EXTERNAL_IDS);
|
||||
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
|
||||
|
||||
Reference in New Issue
Block a user