opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.10'

Browse Source 
* stable-2.10:
  Update 2.10 release notes
  Fix faulty behaviour in BLOCK permission
  Let RefControlTest be responsible for creating own test projects
  Resource exhausted because of unclosed LDAP connection

Conflicts:
	gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java

Change-Id: I5e8a98365ce583cbd62c9289e49af8d5c1e755b1
This commit is contained in:
David Pursehouse
2015-01-09 09:44:33 +09:00
parent 0a7abf367f 7794b9e5e8
commit 31ba7fa943
6 changed files with 74 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
ReleaseNotes/ReleaseNotes-2.10.txt
View File

@@ -442,6 +442,23 @@ explicitly named on the command line (which may be empty) should be notified of
the change. Users watching the project should not be notified, as the change has
not yet been published.
* Fix resource exhaustion due to unclosed LDAP connection.
+
When `auth.type` is set to `LDAP` (not `LDAP_BIND`), two LDAP connections are
made, but one was not being closed. This eventually caused resource exhaustion
and LDAP authentications failed.
Access Permissions
~~~~~~~~~~~~~~~~~~
* link:https://code.google.com/p/gerrit/issues/detail?id=2995[Issue 2995]:
Fix faulty behaviour in `BLOCK` permission.
+
`BLOCK` can be overruled with `ALLOW` on the same project, however there was a
bug when a child of the above project duplicates the `ALLOW` permission. In this
case the `BLOCK` would always win for the child, even though the `BLOCK` was
overruled in the parent.
Web UI
~~~~~~

2
gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java
View File

@@ -89,7 +89,7 @@ public class LdapAuthBackend implements AuthBackend {
// We found the user account, but we need to verify
// the password matches it before we can continue.
//
helper.authenticate(m.getDN(), req.getPassword());
helper.authenticate(m.getDN(), req.getPassword()).close();
}
return new AuthUser(new AuthUser.UUID(username), username);
} finally {

2
gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
View File

@@ -221,7 +221,7 @@ public class LdapRealm extends AbstractRealm {
// We found the user account, but we need to verify
// the password matches it before we can continue.
//
helper.authenticate(m.getDN(), who.getPassword());
helper.authenticate(m.getDN(), who.getPassword()).close();
}
who.setDisplayName(apply(schema.accountFullName, m));

25
gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.project;
import static com.google.common.base.Objects.firstNonNull;
import static com.google.gerrit.server.project.RefControl.isRE;
import com.google.common.collect.Lists;
@@ -117,6 +118,7 @@ public class PermissionCollection {
Set<String> exclusiveGroupPermissions = new HashSet<>();
HashMap<String, List<PermissionRule>> permissions = new HashMap<>();
HashMap<String, List<PermissionRule>> overridden = new HashMap<>();
Map<PermissionRule, ProjectRef> ruleProps = Maps.newIdentityHashMap();
for (AccessSection section : sections) {
Project.NameKey project = sectionToProject.get(section);
@@ -132,11 +134,19 @@ public class PermissionCollection {
} else {
addRule = seen.add(s) && !rule.isDeny() && !exclusivePermissionExists;
}
HashMap<String, List<PermissionRule>> p = null;
if (addRule) {
List<PermissionRule> r = permissions.get(permission.getName());
p = permissions;
} else if (!rule.isDeny() && !exclusivePermissionExists) {
p = overridden;
}
if (p != null) {
List<PermissionRule> r = p.get(permission.getName());
if (r == null) {
r = new ArrayList<>(2);
permissions.put(permission.getName(), r);
p.put(permission.getName(), r);
}
r.add(rule);
ruleProps.put(rule, new ProjectRef(project, section.getName()));
@@ -149,18 +159,22 @@ public class PermissionCollection {
}
}
return new PermissionCollection(permissions, ruleProps, perUser);
return new PermissionCollection(permissions, overridden, ruleProps,
perUser);
}
}
private final Map<String, List<PermissionRule>> rules;
private final Map<String, List<PermissionRule>> overridden;
private final Map<PermissionRule, ProjectRef> ruleProps;
private final boolean perUser;
private PermissionCollection(Map<String, List<PermissionRule>> rules,
Map<String, List<PermissionRule>> overridden,
Map<PermissionRule, ProjectRef> ruleProps,
boolean perUser) {
this.rules = rules;
this.overridden = overridden;
this.ruleProps = ruleProps;
this.perUser = perUser;
}
@@ -186,6 +200,11 @@ public class PermissionCollection {
return r != null ? r : Collections.<PermissionRule> emptyList();
}
List<PermissionRule> getOverridden(String permissionName) {
return firstNonNull(
overridden.get(permissionName), Collections.<PermissionRule> emptyList());
}
ProjectRef getRuleProps(PermissionRule rule) {
return ruleProps.get(rule);
}

22
gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
View File

@@ -127,6 +127,7 @@ public class RefControl {
*/
public boolean isVisibleByRegisteredUsers() {
List<PermissionRule> access = relevant.getPermission(Permission.READ);
List<PermissionRule> overridden = relevant.getOverridden(Permission.READ);
Set<ProjectRef> allows = Sets.newHashSet();
Set<ProjectRef> blocks = Sets.newHashSet();
for (PermissionRule rule : access) {
@@ -136,6 +137,11 @@ public class RefControl {
allows.add(relevant.getRuleProps(rule));
}
}
for (PermissionRule rule : overridden) {
if (SystemGroupBackend.isAnonymousOrRegistered(rule.getGroup())) {
blocks.remove(relevant.getRuleProps(rule));
}
}
blocks.removeAll(allows);
return blocks.isEmpty() && !allows.isEmpty();
}
@@ -541,6 +547,7 @@ public class RefControl {
private boolean doCanPerform(String permissionName, boolean blockOnly) {
List<PermissionRule> access = access(permissionName);
List<PermissionRule> overridden = relevant.getOverridden(permissionName);
Set<ProjectRef> allows = Sets.newHashSet();
Set<ProjectRef> blocks = Sets.newHashSet();
for (PermissionRule rule : access) {
@@ -550,6 +557,9 @@ public class RefControl {
allows.add(relevant.getRuleProps(rule));
}
}
for (PermissionRule rule : overridden) {
blocks.remove(relevant.getRuleProps(rule));
}
blocks.removeAll(allows);
return blocks.isEmpty() && (!allows.isEmpty() || blockOnly);
}
@@ -557,6 +567,7 @@ public class RefControl {
/** True if the user has force this permission. Works only for non labels. */
private boolean canForcePerform(String permissionName) {
List<PermissionRule> access = access(permissionName);
List<PermissionRule> overridden = relevant.getOverridden(permissionName);
Set<ProjectRef> allows = Sets.newHashSet();
Set<ProjectRef> blocks = Sets.newHashSet();
for (PermissionRule rule : access) {
@@ -566,6 +577,11 @@ public class RefControl {
allows.add(relevant.getRuleProps(rule));
}
}
for (PermissionRule rule : overridden) {
if (rule.getForce()) {
blocks.remove(relevant.getRuleProps(rule));
}
}
blocks.removeAll(allows);
return blocks.isEmpty() && !allows.isEmpty();
}
@@ -573,6 +589,7 @@ public class RefControl {
/** True if for this permission force is blocked for the user. Works only for non labels. */
private boolean isForceBlocked(String permissionName) {
List<PermissionRule> access = access(permissionName);
List<PermissionRule> overridden = relevant.getOverridden(permissionName);
Set<ProjectRef> allows = Sets.newHashSet();
Set<ProjectRef> blocks = Sets.newHashSet();
for (PermissionRule rule : access) {
@@ -582,6 +599,11 @@ public class RefControl {
allows.add(relevant.getRuleProps(rule));
}
}
for (PermissionRule rule : overridden) {
if (rule.getForce()) {
blocks.remove(relevant.getRuleProps(rule));
}
}
blocks.removeAll(allows);
return !blocks.isEmpty();
}

11
gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
View File

@@ -370,6 +370,17 @@ public class RefControlTest {
assertFalse("u can't vote 2", range.contains(2));
}
@Test
public void testInheritSubmit_AllowInChildDoesntAffectUnblockInParent() {
block(parent, SUBMIT, ANONYMOUS_USERS, "refs/heads/*");
allow(parent, SUBMIT, REGISTERED_USERS, "refs/heads/*");
allow(local, SUBMIT, REGISTERED_USERS, "refs/heads/*");
ProjectControl u = util.user(local);
assertFalse("not blocked from submitting", u.controlForRef(
"refs/heads/master").isBlocked(SUBMIT));
}
@Test
public void testUnblockNoForce() {
block(local, PUSH, ANONYMOUS_USERS, "refs/heads/*");