Merge "ChangeField: Remove duplicate calls to stored() for int range fields"
diff --git a/java/com/google/gerrit/httpd/GitOverHttpServlet.java b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
index 05c1d58..739726e 100644
--- a/java/com/google/gerrit/httpd/GitOverHttpServlet.java
+++ b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
@@ -184,7 +184,7 @@
try {
Project.NameKey nameKey = new Project.NameKey(projectName);
ProjectState state = projectCache.checkedGet(nameKey);
- if (state == null) {
+ if (state == null || !state.statePermitsRead()) {
throw new RepositoryNotFoundException(nameKey.get());
}
req.setAttribute(ATT_STATE, state);
diff --git a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
index c51fb9b..f117b24 100644
--- a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
+++ b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
@@ -259,8 +259,14 @@
throws NoSuchProjectException, IOException, PermissionBackendException,
ResourceConflictException {
ProjectState state = projectCache.checkedGet(projectName);
+ // 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
+ // 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(projectName).check(ProjectPermission.ACCESS);
+ permissionBackend.currentUser().project(projectName).check(permissionToCheck);
} catch (AuthException e) {
throw new NoSuchProjectException(projectName);
}
diff --git a/java/com/google/gerrit/server/args4j/ProjectHandler.java b/java/com/google/gerrit/server/args4j/ProjectHandler.java
index e97c171..1d40b53 100644
--- a/java/com/google/gerrit/server/args4j/ProjectHandler.java
+++ b/java/com/google/gerrit/server/args4j/ProjectHandler.java
@@ -79,7 +79,13 @@
if (state == null) {
throw new CmdLineException(owner, String.format("project %s not found", nameWithoutSuffix));
}
- permissionBackend.currentUser().project(nameKey).check(ProjectPermission.ACCESS);
+ // 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
+ // 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;
+ permissionBackend.currentUser().project(nameKey).check(permissionToCheck);
} catch (AuthException e) {
throw new CmdLineException(owner, new NoSuchProjectException(nameKey).getMessage());
} catch (PermissionBackendException | IOException e) {
diff --git a/java/com/google/gerrit/server/events/EventBroker.java b/java/com/google/gerrit/server/events/EventBroker.java
index 8f12cb3..62e8d12 100644
--- a/java/com/google/gerrit/server/events/EventBroker.java
+++ b/java/com/google/gerrit/server/events/EventBroker.java
@@ -150,6 +150,11 @@
protected boolean isVisibleTo(Project.NameKey project, CurrentUser user) {
try {
+ ProjectState state = projectCache.get(project);
+ if (state == null || !state.statePermitsRead()) {
+ return false;
+ }
+
permissionBackend.user(user).project(project).check(ProjectPermission.ACCESS);
return true;
} catch (AuthException | PermissionBackendException e) {
diff --git a/java/com/google/gerrit/server/permissions/ProjectControl.java b/java/com/google/gerrit/server/permissions/ProjectControl.java
index 30ed180..dbd60ea 100644
--- a/java/com/google/gerrit/server/permissions/ProjectControl.java
+++ b/java/com/google/gerrit/server/permissions/ProjectControl.java
@@ -202,11 +202,6 @@
return false;
}
- /** Returns whether the project is hidden. */
- private boolean isHidden() {
- return getProject().getState().equals(com.google.gerrit.extensions.client.ProjectState.HIDDEN);
- }
-
private boolean canAddRefs() {
return (canPerformOnAnyRef(Permission.CREATE) || isAdmin());
}
@@ -400,8 +395,7 @@
private boolean can(ProjectPermission perm) throws PermissionBackendException {
switch (perm) {
case ACCESS:
- return (!isHidden() && (user.isInternalUser() || canPerformOnAnyRef(Permission.READ)))
- || isOwner();
+ return user.isInternalUser() || isOwner() || canPerformOnAnyRef(Permission.READ);
case READ:
return allRefsAreVisible(Collections.emptySet());
diff --git a/java/com/google/gerrit/server/project/ChildProjects.java b/java/com/google/gerrit/server/project/ChildProjects.java
index a29a152..868d0af 100644
--- a/java/com/google/gerrit/server/project/ChildProjects.java
+++ b/java/com/google/gerrit/server/project/ChildProjects.java
@@ -53,7 +53,7 @@
/** Gets all child projects recursively. */
public List<ProjectInfo> list(Project.NameKey parent) throws PermissionBackendException {
- Map<Project.NameKey, Project> projects = readAllProjects();
+ Map<Project.NameKey, Project> projects = readAllReadableProjects();
Multimap<Project.NameKey, Project.NameKey> children = parentToChildren(projects);
PermissionBackend.WithUser perm = permissionBackend.currentUser();
@@ -62,11 +62,11 @@
return results;
}
- private Map<Project.NameKey, Project> readAllProjects() {
+ private Map<Project.NameKey, Project> readAllReadableProjects() {
Map<Project.NameKey, Project> projects = new HashMap<>();
for (Project.NameKey name : projectCache.all()) {
ProjectState c = projectCache.get(name);
- if (c != null) {
+ if (c != null && c.statePermitsRead()) {
projects.put(c.getNameKey(), c.getProject());
}
}
diff --git a/java/com/google/gerrit/server/project/RefUtil.java b/java/com/google/gerrit/server/project/RefUtil.java
index 62e48be..e42a7df 100644
--- a/java/com/google/gerrit/server/project/RefUtil.java
+++ b/java/com/google/gerrit/server/project/RefUtil.java
@@ -39,6 +39,8 @@
public class RefUtil {
private static final Logger log = LoggerFactory.getLogger(RefUtil.class);
+ private RefUtil() {}
+
public static ObjectId parseBaseRevision(
Repository repo, Project.NameKey projectName, String baseRevision)
throws InvalidRevisionException {
diff --git a/java/com/google/gerrit/server/project/SuggestParentCandidates.java b/java/com/google/gerrit/server/project/SuggestParentCandidates.java
index 014029b..99833af 100644
--- a/java/com/google/gerrit/server/project/SuggestParentCandidates.java
+++ b/java/com/google/gerrit/server/project/SuggestParentCandidates.java
@@ -44,17 +44,17 @@
public List<Project.NameKey> getNameKeys() throws PermissionBackendException {
return permissionBackend
.currentUser()
- .filter(ProjectPermission.ACCESS, parents())
+ .filter(ProjectPermission.ACCESS, readableParents())
.stream()
.sorted()
.collect(toList());
}
- private Set<Project.NameKey> parents() {
+ private Set<Project.NameKey> readableParents() {
Set<Project.NameKey> parents = new HashSet<>();
for (Project.NameKey p : projectCache.all()) {
ProjectState ps = projectCache.get(p);
- if (ps != null) {
+ if (ps != null && ps.statePermitsRead()) {
Project.NameKey parent = ps.getProject().getParent();
if (parent != null) {
parents.add(parent);
diff --git a/java/com/google/gerrit/server/query/account/InternalAccountQuery.java b/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
index f1be580..02386ae 100644
--- a/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
+++ b/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
@@ -17,11 +17,9 @@
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
-import com.google.common.base.Joiner;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
-import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.IndexConfig;
@@ -37,8 +35,6 @@
import java.util.Arrays;
import java.util.List;
import java.util.Set;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* Query wrapper for the account index.
@@ -47,8 +43,6 @@
* holding on to a single instance.
*/
public class InternalAccountQuery extends InternalQuery<AccountState> {
- private static final Logger log = LoggerFactory.getLogger(InternalAccountQuery.class);
-
@Inject
InternalAccountQuery(
AccountQueryProcessor queryProcessor,
@@ -94,28 +88,6 @@
return query(AccountPredicates.externalIdIncludingSecondaryEmails(externalId.toString()));
}
- public AccountState oneByExternalId(String externalId) throws OrmException {
- return oneByExternalId(ExternalId.Key.parse(externalId));
- }
-
- public AccountState oneByExternalId(String scheme, String id) throws OrmException {
- return oneByExternalId(ExternalId.Key.create(scheme, id));
- }
-
- public AccountState oneByExternalId(ExternalId.Key externalId) throws OrmException {
- List<AccountState> accountStates = byExternalId(externalId);
- if (accountStates.size() == 1) {
- return accountStates.get(0);
- } else if (accountStates.size() > 0) {
- StringBuilder msg = new StringBuilder();
- msg.append("Ambiguous external ID ").append(externalId).append(" for accounts: ");
- Joiner.on(", ")
- .appendTo(msg, Lists.transform(accountStates, AccountState.ACCOUNT_ID_FUNCTION));
- log.warn(msg.toString());
- }
- return null;
- }
-
public List<AccountState> byFullName(String fullName) throws OrmException {
return query(AccountPredicates.fullName(fullName));
}
diff --git a/java/com/google/gerrit/server/restapi/config/ListTasks.java b/java/com/google/gerrit/server/restapi/config/ListTasks.java
index 71ee5ad..fb2819c 100644
--- a/java/com/google/gerrit/server/restapi/config/ListTasks.java
+++ b/java/com/google/gerrit/server/restapi/config/ListTasks.java
@@ -28,6 +28,8 @@
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.util.IdGenerator;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -46,13 +48,18 @@
private final PermissionBackend permissionBackend;
private final WorkQueue workQueue;
private final Provider<CurrentUser> self;
+ private final ProjectCache projectCache;
@Inject
public ListTasks(
- PermissionBackend permissionBackend, WorkQueue workQueue, Provider<CurrentUser> self) {
+ PermissionBackend permissionBackend,
+ WorkQueue workQueue,
+ Provider<CurrentUser> self,
+ ProjectCache projectCache) {
this.permissionBackend = permissionBackend;
this.workQueue = workQueue;
this.self = self;
+ this.projectCache = projectCache;
}
@Override
@@ -77,14 +84,17 @@
if (task.projectName != null) {
Boolean visible = visibilityCache.get(task.projectName);
if (visible == null) {
- try {
- permissionBackend
- .user(user)
- .project(new Project.NameKey(task.projectName))
- .check(ProjectPermission.ACCESS);
- visible = true;
- } catch (AuthException e) {
+ Project.NameKey nameKey = new Project.NameKey(task.projectName);
+ ProjectState state = projectCache.get(nameKey);
+ if (state == null || !state.statePermitsRead()) {
visible = false;
+ } else {
+ try {
+ permissionBackend.user(user).project(nameKey).check(ProjectPermission.ACCESS);
+ visible = true;
+ } catch (AuthException e) {
+ visible = false;
+ }
}
visibilityCache.put(task.projectName, visible);
}
diff --git a/java/com/google/gerrit/server/restapi/config/TasksCollection.java b/java/com/google/gerrit/server/restapi/config/TasksCollection.java
index f5b6e56..dda54a0 100644
--- a/java/com/google/gerrit/server/restapi/config/TasksCollection.java
+++ b/java/com/google/gerrit/server/restapi/config/TasksCollection.java
@@ -18,8 +18,10 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.config.TaskResource;
@@ -30,6 +32,8 @@
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -41,6 +45,7 @@
private final WorkQueue workQueue;
private final Provider<CurrentUser> self;
private final PermissionBackend permissionBackend;
+ private final ProjectCache projectCache;
@Inject
TasksCollection(
@@ -48,12 +53,14 @@
ListTasks list,
WorkQueue workQueue,
Provider<CurrentUser> self,
- PermissionBackend permissionBackend) {
+ PermissionBackend permissionBackend,
+ ProjectCache projectCache) {
this.views = views;
this.list = list;
this.workQueue = workQueue;
this.self = self;
this.permissionBackend = permissionBackend;
+ this.projectCache = projectCache;
}
@Override
@@ -63,7 +70,8 @@
@Override
public TaskResource parse(ConfigResource parent, IdString id)
- throws ResourceNotFoundException, AuthException, PermissionBackendException {
+ throws ResourceNotFoundException, AuthException, PermissionBackendException,
+ ResourceConflictException {
CurrentUser user = self.get();
if (!user.isIdentifiedUser()) {
throw new AuthException("Authentication required");
@@ -78,11 +86,16 @@
Task<?> task = workQueue.getTask(taskId);
if (task instanceof ProjectTask) {
+ Project.NameKey nameKey = ((ProjectTask<?>) task).getProjectNameKey();
+ ProjectState state = projectCache.get(nameKey);
+ if (state == null) {
+ throw new ResourceNotFoundException(String.format("project %s not found", nameKey));
+ }
+
+ state.checkStatePermitsRead();
+
try {
- permissionBackend
- .user(user)
- .project(((ProjectTask<?>) task).getProjectNameKey())
- .check(ProjectPermission.ACCESS);
+ permissionBackend.user(user).project(nameKey).check(ProjectPermission.ACCESS);
return new TaskResource(task);
} catch (AuthException e) {
// Fall through and try view queue permission.
diff --git a/java/com/google/gerrit/server/restapi/project/ListChildProjects.java b/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
index 7db4f84..3067c89 100644
--- a/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
@@ -80,7 +80,9 @@
Map<Project.NameKey, Project> children = new HashMap<>();
for (Project.NameKey name : projectCache.all()) {
ProjectState c = projectCache.get(name);
- if (c != null && parent.equals(c.getProject().getParent(allProjects))) {
+ if (c != null
+ && parent.equals(c.getProject().getParent(allProjects))
+ && c.statePermitsRead()) {
children.put(c.getNameKey(), c.getProject());
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/ListDashboards.java b/java/com/google/gerrit/server/restapi/project/ListDashboards.java
index 08b4069..882e922 100644
--- a/java/com/google/gerrit/server/restapi/project/ListDashboards.java
+++ b/java/com/google/gerrit/server/restapi/project/ListDashboards.java
@@ -88,8 +88,11 @@
private Collection<ProjectState> tree(ProjectResource rsrc) throws PermissionBackendException {
Map<Project.NameKey, ProjectState> tree = new LinkedHashMap<>();
for (ProjectState ps : rsrc.getProjectState().tree()) {
- tree.put(ps.getNameKey(), ps);
+ if (ps.statePermitsRead()) {
+ tree.put(ps.getNameKey(), ps);
+ }
}
+
tree.keySet()
.retainAll(permissionBackend.currentUser().filter(ProjectPermission.ACCESS, tree.keySet()));
return tree.values();
diff --git a/java/com/google/gerrit/server/restapi/project/ListProjects.java b/java/com/google/gerrit/server/restapi/project/ListProjects.java
index a1572c6..9a8232e 100644
--- a/java/com/google/gerrit/server/restapi/project/ListProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/ListProjects.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi.project;
+import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.extensions.client.ProjectState.HIDDEN;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;
@@ -521,11 +522,28 @@
if (type == FilterType.PARENT_CANDIDATES) {
matches = parentsOf(matches);
}
- // TODO(dborowitz): Streamified PermissionBackend#filter.
- return perm.filter(ProjectPermission.ACCESS, matches.collect(toList()))
- .stream()
- .sorted()
- .collect(toList());
+
+ List<Project.NameKey> results = new ArrayList<>();
+ List<Project.NameKey> projectNameKeys = matches.sorted().collect(toList());
+ for (Project.NameKey nameKey : projectNameKeys) {
+ ProjectState state = projectCache.get(nameKey);
+ checkNotNull(state, "Failed to load project %s", nameKey);
+
+ // 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
+ // 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 {
+ perm.project(nameKey).check(permissionToCheck);
+ results.add(nameKey);
+ } catch (AuthException e) {
+ // Not added to results.
+ }
+ }
+
+ return results;
}
private Stream<Project.NameKey> parentsOf(Stream<Project.NameKey> matches) {
@@ -551,13 +569,19 @@
}
private boolean isParentAccessible(
- Map<Project.NameKey, Boolean> checked, PermissionBackend.WithUser perm, ProjectState p)
+ Map<Project.NameKey, Boolean> checked, PermissionBackend.WithUser perm, ProjectState state)
throws PermissionBackendException {
- Project.NameKey name = p.getNameKey();
+ Project.NameKey name = state.getNameKey();
Boolean b = checked.get(name);
if (b == null) {
try {
- perm.project(name).check(ProjectPermission.ACCESS);
+ // 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
+ // 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;
+ perm.project(name).check(permissionToCheck);
b = true;
} catch (AuthException denied) {
b = false;
diff --git a/java/com/google/gerrit/server/restapi/project/ProjectsCollection.java b/java/com/google/gerrit/server/restapi/project/ProjectsCollection.java
index c5fb41e..3af8424 100644
--- a/java/com/google/gerrit/server/restapi/project/ProjectsCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/ProjectsCollection.java
@@ -150,8 +150,14 @@
}
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
+ // 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(ProjectPermission.ACCESS);
+ permissionBackend.currentUser().project(nameKey).check(permissionToCheck);
} catch (AuthException e) {
return null; // Pretend like not found on access denied.
}
diff --git a/java/com/google/gerrit/sshd/commands/KillCommand.java b/java/com/google/gerrit/sshd/commands/KillCommand.java
index a7e751a..ef12f5f 100644
--- a/java/com/google/gerrit/sshd/commands/KillCommand.java
+++ b/java/com/google/gerrit/sshd/commands/KillCommand.java
@@ -20,6 +20,7 @@
import com.google.gerrit.extensions.annotations.RequiresAnyCapability;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.config.TaskResource;
@@ -51,7 +52,10 @@
try {
TaskResource taskRsrc = tasksCollection.parse(cfgRsrc, IdString.fromDecoded(id));
deleteTask.apply(taskRsrc, null);
- } catch (AuthException | ResourceNotFoundException | PermissionBackendException e) {
+ } catch (AuthException
+ | ResourceNotFoundException
+ | ResourceConflictException
+ | PermissionBackendException e) {
stderr.print("kill: " + id + ": No such task\n");
}
}
diff --git a/plugins/replication b/plugins/replication
index dc1e7ff..9b08f32 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit dc1e7ff434fd7fce90dd6f6c4e312280bd884bd0
+Subproject commit 9b08f324462e62872ac071175e16e7553ba76e2b
