opendev/gerrit
Code Issues Proposed changes

ProjectsCollection: Check WRITE_CONFIG permission only if project is hidden

Browse Source 
Doing this permission check is unneeded if the project state permits
read.

Also permission checks that are done through the check method may result
in audit logs if the permission is denied. If the project state permits
read and the user doesn't have the WRITE_CONFIG permission we don't want
to have such an audit log.

This issue was discovered by looking at debug traces.

Change-Id: I5d07993c0932b024fb55431b0036fc619815534d
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2018-09-21 10:51:37 +02:00
parent 60f59f1cdf
commit 92f4c244d7
1 changed files with 12 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
java/com/google/gerrit/server/restapi/project/ProjectsCollection.java
View File

@@ -155,15 +155,18 @@ public class ProjectsCollection
} catch (AuthException e) {
return null; // Pretend like not found on access denied.
}
// If the project's state does not permit reading, we want to hide it from all callers. The
// only exception to that are users who are allowed to mutate the project's configuration.
// This enables these users to still mutate the project's state (e.g. set a HIDDEN project to
// ACTIVE). Individual views should still check for checkStatePermitsRead() and this should
// just serve as a safety net in case the individual check is forgotten.
try {
permissionBackend.currentUser().project(nameKey).check(ProjectPermission.WRITE_CONFIG);
} catch (AuthException e) {
state.checkStatePermitsRead();
if (!state.statePermitsRead()) {
// If the project's state does not permit reading, we want to hide it from all callers. The
// only exception to that are users who are allowed to mutate the project's configuration.
// This enables these users to still mutate the project's state (e.g. set a HIDDEN project
// to ACTIVE). Individual views should still check for checkStatePermitsRead() and this
// should just serve as a safety net in case the individual check is forgotten.
try {
permissionBackend.currentUser().project(nameKey).check(ProjectPermission.WRITE_CONFIG);
} catch (AuthException e) {
state.checkStatePermitsRead();
}
}
}
return new ProjectResource(state, user.get());