opendev
/
gerrit
Code Issues Proposed changes

Merge "Use WRITE_CONFIG in ProjectsCollection"

This commit is contained in:
Patrick Hiesel 2018-09-24 10:47:30 +00:00 committed by Gerrit Code Review
parent 37a903a993 ca4b395488
commit 26f524df55
2 changed files with 29 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -145,23 +145,16 @@ public class ProjectsCollection
if (checkAccess) {
// Hidden projects(permitsRead = false) should only be accessible by the project owners.
// READ_CONFIG is checked here because it's only allowed to project owners(ACCESS may also
// WRITE_CONFIG is checked here because it's only allowed to project owners (ACCESS may also
// be allowed for other users). Allowing project owners to access here will help them to view
// and update the config of hidden projects easily.
ProjectPermission permissionToCheck =
state.statePermitsRead() ? ProjectPermission.ACCESS : ProjectPermission.READ_CONFIG;
try {
permissionBackend.currentUser().project(nameKey).check(permissionToCheck);
} catch (AuthException e) {
return null; // Pretend like not found on access denied.
}
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.
if (state.statePermitsRead()) {
try {
permissionBackend.currentUser().project(nameKey).check(ProjectPermission.ACCESS);
} catch (AuthException e) {
return null;
}
} else {
try {
permissionBackend.currentUser().project(nameKey).check(ProjectPermission.WRITE_CONFIG);
} catch (AuthException e) {

21
javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
View File

@ -49,6 +49,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.IndexExecutor;
import com.google.inject.Inject;
import org.eclipse.jgit.revwalk.RevCommit;
@ -395,6 +396,26 @@ public class ProjectIT extends AbstractDaemonTest {
}
}
@Test
public void nonActiveProjectCanBeMadeActiveByHostAdmin() throws Exception {
// ACTIVE => HIDDEN
ConfigInput ci1 = new ConfigInput();
ci1.state = ProjectState.HIDDEN;
gApi.projects().name(project.get()).config(ci1);
assertThat(gApi.projects().name(project.get()).config().state).isEqualTo(ProjectState.HIDDEN);
// Revoke OWNER permission for admin and block them from reading the project's refs
block(project, RefNames.REFS + "*", Permission.OWNER, SystemGroupBackend.REGISTERED_USERS);
block(project, RefNames.REFS + "*", Permission.READ, SystemGroupBackend.REGISTERED_USERS);
// HIDDEN => ACTIVE
ConfigInput ci2 = new ConfigInput();
ci2.state = ProjectState.ACTIVE;
gApi.projects().name(project.get()).config(ci2);
// ACTIVE is represented as null in the API
assertThat(gApi.projects().name(project.get()).config().state).isNull();
}
@Test
public void reindexProject() throws Exception {
createProject("child", project);