Added sameGroupVisibility to All-Projects project.config.
The sameGroupVisiblity is needed to restrict visibility of accounts
when accountVisibility is SAME_GROUP. Namely, this is a way to make
sure the autoVerify group in a contributor-agreements section is
never suggested.
Change-Id: I0d5d55793ca55fc5929ca093637a27c309ca38d9
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountControl.java
index b297ed8..e297ad7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountControl.java
@@ -14,11 +14,14 @@
package com.google.gerrit.server.account;
+import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.git.AccountsSection;
+import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -27,16 +30,19 @@
/** Access control management for one account's access to other accounts. */
public class AccountControl {
public static class Factory {
+ private final ProjectCache projectCache;
private final GroupControl.Factory groupControlFactory;
private final Provider<CurrentUser> user;
private final IdentifiedUser.GenericFactory userFactory;
private final AccountVisibility accountVisibility;
@Inject
- Factory(final GroupControl.Factory groupControlFactory,
+ Factory(final ProjectCache projectCache,
+ final GroupControl.Factory groupControlFactory,
final Provider<CurrentUser> user,
final IdentifiedUser.GenericFactory userFactory,
final AccountVisibility accountVisibility) {
+ this.projectCache = projectCache;
this.groupControlFactory = groupControlFactory;
this.user = user;
this.userFactory = userFactory;
@@ -44,20 +50,24 @@
}
public AccountControl get() {
- return new AccountControl(groupControlFactory, user.get(), userFactory,
- accountVisibility);
+ return new AccountControl(projectCache, groupControlFactory, user.get(),
+ userFactory, accountVisibility);
}
}
+ private final AccountsSection accountsSection;
private final GroupControl.Factory groupControlFactory;
private final CurrentUser currentUser;
private final IdentifiedUser.GenericFactory userFactory;
private final AccountVisibility accountVisibility;
- AccountControl(final GroupControl.Factory groupControlFactory,
+ AccountControl(final ProjectCache projectCache,
+ final GroupControl.Factory groupControlFactory,
final CurrentUser currentUser,
final IdentifiedUser.GenericFactory userFactory,
final AccountVisibility accountVisibility) {
+ this.accountsSection =
+ projectCache.getAllProjects().getConfig().getAccountsSection();
this.groupControlFactory = groupControlFactory;
this.currentUser = currentUser;
this.userFactory = userFactory;
@@ -87,6 +97,12 @@
Set<AccountGroup.UUID> usersGroups = groupsOf(otherUser);
usersGroups.remove(AccountGroup.ANONYMOUS_USERS);
usersGroups.remove(AccountGroup.REGISTERED_USERS);
+ for (PermissionRule rule : accountsSection.getSameGroupVisibility()) {
+ if (rule.isBlock() || rule.isDeny()) {
+ usersGroups.remove(rule.getGroup().getUUID());
+ }
+ }
+
if (currentUser.getEffectiveGroups().containsAnyOf(usersGroups)) {
return true;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/ListGroupMembership.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/ListGroupMembership.java
index 237d381..346f406 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/ListGroupMembership.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/ListGroupMembership.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.account;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import com.google.gerrit.reviewdb.client.AccountGroup;
import java.util.Set;
@@ -47,6 +48,6 @@
@Override
public Set<AccountGroup.UUID> getKnownGroups() {
- return ImmutableSet.copyOf(groups);
+ return Sets.newHashSet(groups);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/AccountsSection.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/AccountsSection.java
new file mode 100644
index 0000000..7d868bf
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/AccountsSection.java
@@ -0,0 +1,35 @@
+// Copyright (C) 2010 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.git;
+
+import com.google.gerrit.common.data.PermissionRule;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class AccountsSection {
+ protected List<PermissionRule> sameGroupVisibility;
+
+ public List<PermissionRule> getSameGroupVisibility() {
+ if (sameGroupVisibility == null) {
+ sameGroupVisibility = new ArrayList<PermissionRule>();
+ }
+ return sameGroupVisibility;
+ }
+
+ public void setSameGroupVisibility(List<PermissionRule> sameGroupVisibility) {
+ this.sameGroupVisibility = sameGroupVisibility;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
index b380276..2b36259 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
@@ -58,6 +58,9 @@
private static final String KEY_INHERIT_FROM = "inheritFrom";
private static final String KEY_GROUP_PERMISSIONS = "exclusiveGroupPermissions";
+ private static final String ACCOUNTS = "accounts";
+ private static final String KEY_SAME_GROUP_VISIBILITY = "sameGroupVisibility";
+
private static final String CONTRIBUTOR_AGREEMENT = "contributor-agreement";
private static final String KEY_ACCEPTED = "accepted";
private static final String KEY_REQUIRE_CONTACT_INFORMATION = "requireContactInformation";
@@ -84,6 +87,7 @@
private Project.NameKey projectName;
private Project project;
+ private AccountsSection accountsSection;
private Map<AccountGroup.UUID, GroupReference> groupsByUUID;
private Map<String, AccessSection> accessSections;
private Map<String, ContributorAgreement> contributorAgreements;
@@ -112,6 +116,10 @@
return project;
}
+ public AccountsSection getAccountsSection() {
+ return accountsSection;
+ }
+
public AccessSection getAccessSection(String name) {
return getAccessSection(name, false);
}
@@ -266,6 +274,20 @@
p.setUseContentMerge(getBoolean(rc, SUBMIT, KEY_MERGE_CONTENT, false));
p.setState(getEnum(rc, PROJECT, null, KEY_STATE, defaultStateValue));
+ loadAccountsSection(rc, groupsByName);
+ loadContributorAgreements(rc, groupsByName);
+ loadAccessSections(rc, groupsByName);
+ }
+
+ private void loadAccountsSection(
+ Config rc, Map<String, GroupReference> groupsByName) {
+ accountsSection = new AccountsSection();
+ accountsSection.setSameGroupVisibility(loadPermissionRules(
+ rc, ACCOUNTS, null, KEY_SAME_GROUP_VISIBILITY, groupsByName, false));
+ }
+
+ private void loadContributorAgreements(
+ Config rc, Map<String, GroupReference> groupsByName) {
contributorAgreements = new HashMap<String, ContributorAgreement>();
for (String name : rc.getSubsections(CONTRIBUTOR_AGREEMENT)) {
ContributorAgreement ca = getContributorAgreement(name, true);
@@ -273,32 +295,33 @@
ca.setRequireContactInformation(
rc.getBoolean(CONTRIBUTOR_AGREEMENT, name, KEY_REQUIRE_CONTACT_INFORMATION, false));
ca.setAgreementUrl(rc.getString(CONTRIBUTOR_AGREEMENT, name, KEY_AGREEMENT_URL));
+ ca.setAccepted(loadPermissionRules(
+ rc, CONTRIBUTOR_AGREEMENT, name, KEY_ACCEPTED, groupsByName, false));
- Permission perm = new Permission(KEY_ACCEPTED);
- loadPermissionRules(rc, CONTRIBUTOR_AGREEMENT, name, KEY_ACCEPTED, groupsByName, perm, false);
- ca.setAccepted(perm.getRules());
-
- perm = new Permission(KEY_AUTO_VERIFY);
- loadPermissionRules(rc, CONTRIBUTOR_AGREEMENT, name, KEY_AUTO_VERIFY, groupsByName, perm, false);
- if (perm.getRules().isEmpty()) {
+ List<PermissionRule> rules = loadPermissionRules(
+ rc, CONTRIBUTOR_AGREEMENT, name, KEY_AUTO_VERIFY, groupsByName, false);
+ if (rules.isEmpty()) {
ca.setAutoVerify(null);
- } else if (perm.getRules().size() > 1) {
+ } else if (rules.size() > 1) {
error(new ValidationError(PROJECT_CONFIG, "Invalid rule in "
+ CONTRIBUTOR_AGREEMENT
+ "." + name
+ "." + KEY_AUTO_VERIFY
+ ": at most one group may be set"));
- } else if (perm.getRules().get(0).getAction() != Action.ALLOW) {
+ } else if (rules.get(0).getAction() != Action.ALLOW) {
error(new ValidationError(PROJECT_CONFIG, "Invalid rule in "
+ CONTRIBUTOR_AGREEMENT
+ "." + name
+ "." + KEY_AUTO_VERIFY
+ ": the group must be allowed"));
} else {
- ca.setAutoVerify(perm.getRules().get(0).getGroup());
+ ca.setAutoVerify(rules.get(0).getGroup());
}
}
+ }
+ private void loadAccessSections(
+ Config rc, Map<String, GroupReference> groupsByName) {
accessSections = new HashMap<String, AccessSection>();
for (String refName : rc.getSubsections(ACCESS)) {
if (RefConfigSection.isValid(refName)) {
@@ -336,6 +359,15 @@
}
}
+ private List<PermissionRule> loadPermissionRules(Config rc, String section,
+ String subsection, String varName,
+ Map<String, GroupReference> groupsByName,
+ boolean useRange) {
+ Permission perm = new Permission(varName);
+ loadPermissionRules(rc, section, subsection, varName, groupsByName, perm, useRange);
+ return perm.getRules();
+ }
+
private void loadPermissionRules(Config rc, String section,
String subsection, String varName,
Map<String, GroupReference> groupsByName, Permission perm,
@@ -425,6 +457,24 @@
set(rc, PROJECT, null, KEY_STATE, p.getState(), null);
Set<AccountGroup.UUID> keepGroups = new HashSet<AccountGroup.UUID>();
+ saveAccountsSection(rc, keepGroups);
+ saveContributorAgreements(rc, keepGroups);
+ saveAccessSections(rc, keepGroups);
+ groupsByUUID.keySet().retainAll(keepGroups);
+
+ saveConfig(PROJECT_CONFIG, rc);
+ saveGroupList();
+ }
+
+ private void saveAccountsSection(Config rc, Set<AccountGroup.UUID> keepGroups) {
+ if (accountsSection != null) {
+ rc.setStringList(ACCOUNTS, null, KEY_SAME_GROUP_VISIBILITY,
+ ruleToStringList(accountsSection.getSameGroupVisibility(), keepGroups));
+ }
+ }
+
+ private void saveContributorAgreements(
+ Config rc, Set<AccountGroup.UUID> keepGroups) {
for (ContributorAgreement ca : sort(contributorAgreements.values())) {
set(rc, CONTRIBUTOR_AGREEMENT, ca.getName(), KEY_DESCRIPTION, ca.getDescription());
set(rc, CONTRIBUTOR_AGREEMENT, ca.getName(), KEY_REQUIRE_CONTACT_INFORMATION, ca.isRequireContactInformation());
@@ -440,16 +490,25 @@
rc.unset(CONTRIBUTOR_AGREEMENT, ca.getName(), KEY_AUTO_VERIFY);
}
- List<String> rules = new ArrayList<String>();
- for (PermissionRule rule : sort(ca.getAccepted())) {
- if (rule.getGroup().getUUID() != null) {
- keepGroups.add(rule.getGroup().getUUID());
- }
- rules.add(rule.asString(false));
- }
- rc.setStringList(CONTRIBUTOR_AGREEMENT, ca.getName(), KEY_ACCEPTED, rules);
+ rc.setStringList(CONTRIBUTOR_AGREEMENT, ca.getName(), KEY_ACCEPTED,
+ ruleToStringList(ca.getAccepted(), keepGroups));
}
+ }
+ private List<String> ruleToStringList(
+ List<PermissionRule> list, Set<AccountGroup.UUID> keepGroups) {
+ List<String> rules = new ArrayList<String>();
+ for (PermissionRule rule : sort(list)) {
+ if (rule.getGroup().getUUID() != null) {
+ keepGroups.add(rule.getGroup().getUUID());
+ }
+ rules.add(rule.asString(false));
+ }
+ return rules;
+ }
+
+ private void saveAccessSections(
+ Config rc, Set<AccountGroup.UUID> keepGroups) {
AccessSection capability = accessSections.get(AccessSection.GLOBAL_CAPABILITIES);
if (capability != null) {
Set<String> have = new HashSet<String>();
@@ -526,10 +585,6 @@
rc.unsetSection(ACCESS, name);
}
}
- groupsByUUID.keySet().retainAll(keepGroups);
-
- saveConfig(PROJECT_CONFIG, rc);
- saveGroupList();
}
private void saveGroupList() throws IOException {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java
index e8f9b1a..2bc5d9f 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java
@@ -73,6 +73,9 @@
+ " submit = group Developers\n" //
+ " push = group Developers\n" //
+ " read = group Developers\n" //
+ + "[accounts]\n" //
+ + " sameGroupVisibility = deny group Developers\n" //
+ + " sameGroupVisibility = block group Staff\n" //
+ "[contributor-agreement \"Individual\"]\n" //
+ " description = A simple description\n" //
+ " accepted = group Developers\n" //
@@ -83,6 +86,7 @@
));
ProjectConfig cfg = read(rev);
+ assertEquals(2, cfg.getAccountsSection().getSameGroupVisibility().size());
ContributorAgreement ca = cfg.getContributorAgreement("Individual");
assertEquals("Individual", ca.getName());
assertEquals("A simple description", ca.getDescription());
@@ -118,6 +122,9 @@
+ " submit = group Developers\n" //
+ " upload = group Developers\n" //
+ " read = group Developers\n" //
+ + "[accounts]\n" //
+ + " sameGroupVisibility = deny group Developers\n" //
+ + " sameGroupVisibility = block group Staff\n" //
+ "[contributor-agreement \"Individual\"]\n" //
+ " description = A simple description\n" //
+ " accepted = group Developers\n" //
@@ -129,6 +136,8 @@
ProjectConfig cfg = read(rev);
AccessSection section = cfg.getAccessSection("refs/heads/*");
+ cfg.getAccountsSection().setSameGroupVisibility(
+ Collections.singletonList(new PermissionRule(cfg.resolve(staff))));
Permission submit = section.getPermission(Permission.SUBMIT);
submit.add(new PermissionRule(cfg.resolve(staff)));
ContributorAgreement ca = cfg.getContributorAgreement("Individual");
@@ -144,6 +153,8 @@
+ "\tsubmit = group Staff\n" //
+ " upload = group Developers\n" //
+ " read = group Developers\n"//
+ + "[accounts]\n" //
+ + " sameGroupVisibility = group Staff\n" //
+ "[contributor-agreement \"Individual\"]\n" //
+ " description = A new description\n" //
+ " accepted = group Staff\n" //
