opendev/gerrit
Code Issues Proposed changes

Cleanup redundant access control evaluation

Browse Source 
During a security audit of Gerrit, Julien Tinnes identified that this
conditional is redundant with the else block, and can be removed.  So
drop it from the code to simplify the logic.

Change-Id: I917f88e63ade3ebf0bc18575fc84f8d2885032e9
Suggested-by: Julien Tinnes
Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2010-05-11 11:22:48 -07:00
parent 9ee35ef724
commit 8fa42c6cc8
2 changed files with 2 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
View File

@@ -157,11 +157,7 @@ public class ProjectControl {
for (final RefRight pr : state.getLocalRights(actionId)) {
if (groups.contains(pr.getAccountGroupId())) {
if (val < 0 && pr.getMaxValue() > 0) {
val = pr.getMaxValue();
} else {
val = Math.max(pr.getMaxValue(), val);
}
val = Math.max(pr.getMaxValue(), val);
}
}

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

@@ -241,18 +241,7 @@ public class RefControl {
for (RefRight right : filterMostSpecific(allRights)) {
if (groups.contains(right.getAccountGroupId())) {
if (val < 0 && right.getMaxValue() > 0) {
// If one of the user's groups had denied them access, but
// this group grants them access, prefer the grant over
// the denial. We have to break the tie somehow and we
// prefer being "more open" to being "more closed".
//
val = right.getMaxValue();
} else {
// Otherwise we use the largest value we can get.
//
val = Math.max(right.getMaxValue(), val);
}
val = Math.max(right.getMaxValue(), val);
}
}
return val >= level;