Merge changes from topic "remove-project-ctl-isHidden"

* changes:
  ProjectControl: remove "isHidden" when checking "READ" permission
  UploadArchive: check ProjectState readable before checking "READ" permission
  ListProjects: check ProjectState readable before checking "READ" permission
  ProjectIsVisibleToPredicate: check ProjectState readable before checking "READ" permission
  DefaultRefFilter: check ProjectState readable before checking "READ" permission
  AsyncReceiveCommits: check ProjectState readable before checking "READ" permission
  GitWebServlet: check ProjectState readable before checking "READ" permission
This commit is contained in:
Patrick Hiesel
2018-03-26 15:40:25 +00:00
committed by Gerrit Code Review
8 changed files with 63 additions and 26 deletions

View File

@@ -36,6 +36,7 @@ import com.google.common.base.CharMatcher;
import com.google.common.base.Splitter; import com.google.common.base.Splitter;
import com.google.gerrit.common.PageLinks; import com.google.gerrit.common.PageLinks;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.AnonymousUser;
@@ -51,6 +52,7 @@ import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission; import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.ssh.SshInfo; import com.google.gerrit.server.ssh.SshInfo;
import com.google.gwtexpui.server.CacheHeaders; import com.google.gwtexpui.server.CacheHeaders;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -412,33 +414,45 @@ class GitwebServlet extends HttpServlet {
} }
Project.NameKey nameKey = new Project.NameKey(name); Project.NameKey nameKey = new Project.NameKey(name);
ProjectState projectState;
try { try {
if (projectCache.checkedGet(nameKey) == null) { projectState = projectCache.checkedGet(nameKey);
notFound(req, rsp); if (projectState == null) {
sendErrorOrRedirect(req, rsp, HttpServletResponse.SC_NOT_FOUND);
return; return;
} }
projectState.checkStatePermitsRead();
permissionBackend.user(userProvider).project(nameKey).check(ProjectPermission.READ); permissionBackend.user(userProvider).project(nameKey).check(ProjectPermission.READ);
} catch (AuthException e) { } catch (AuthException e) {
notFound(req, rsp); sendErrorOrRedirect(req, rsp, HttpServletResponse.SC_NOT_FOUND);
return; return;
} catch (IOException | PermissionBackendException err) { } catch (IOException | PermissionBackendException err) {
log.error("cannot load " + name, err); log.error("cannot load " + name, err);
rsp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); rsp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
return; return;
} catch (ResourceConflictException e) {
sendErrorOrRedirect(req, rsp, HttpServletResponse.SC_CONFLICT);
return;
} }
try (Repository repo = repoManager.openRepository(nameKey)) { try (Repository repo = repoManager.openRepository(nameKey)) {
CacheHeaders.setNotCacheable(rsp); CacheHeaders.setNotCacheable(rsp);
exec(req, rsp, nameKey); exec(req, rsp, projectState);
} catch (RepositoryNotFoundException e) { } catch (RepositoryNotFoundException e) {
getServletContext().log("Cannot open repository", e); getServletContext().log("Cannot open repository", e);
rsp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); rsp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
} }
} }
private void notFound(HttpServletRequest req, HttpServletResponse rsp) throws IOException { /**
* Sends error response if the user is authenticated. Or redirect the user to the login page. By
* doing this, anonymous users cannot infer the existence of a resource from the status code.
*/
private void sendErrorOrRedirect(HttpServletRequest req, HttpServletResponse rsp, int statusCode)
throws IOException {
if (userProvider.get().isIdentifiedUser()) { if (userProvider.get().isIdentifiedUser()) {
rsp.sendError(HttpServletResponse.SC_NOT_FOUND); rsp.sendError(statusCode);
} else { } else {
rsp.sendRedirect(getLoginRedirectUrl(req)); rsp.sendRedirect(getLoginRedirectUrl(req));
} }
@@ -475,13 +489,13 @@ class GitwebServlet extends HttpServlet {
return params; return params;
} }
private void exec(HttpServletRequest req, HttpServletResponse rsp, Project.NameKey project) private void exec(HttpServletRequest req, HttpServletResponse rsp, ProjectState projectState)
throws IOException { throws IOException {
final Process proc = final Process proc =
Runtime.getRuntime() Runtime.getRuntime()
.exec( .exec(
new String[] {gitwebCgi.toAbsolutePath().toString()}, new String[] {gitwebCgi.toAbsolutePath().toString()},
makeEnv(req, project), makeEnv(req, projectState),
gitwebCgi.toAbsolutePath().getParent().toFile()); gitwebCgi.toAbsolutePath().getParent().toFile());
copyStderrToLog(proc.getErrorStream()); copyStderrToLog(proc.getErrorStream());
@@ -524,7 +538,7 @@ class GitwebServlet extends HttpServlet {
} }
} }
private String[] makeEnv(HttpServletRequest req, Project.NameKey nameKey) { private String[] makeEnv(HttpServletRequest req, ProjectState projectState) {
final EnvList env = new EnvList(_env); final EnvList env = new EnvList(_env);
final int contentLength = Math.max(0, req.getContentLength()); final int contentLength = Math.max(0, req.getContentLength());
@@ -562,15 +576,17 @@ class GitwebServlet extends HttpServlet {
env.set("HTTP_" + name.toUpperCase().replace('-', '_'), value); env.set("HTTP_" + name.toUpperCase().replace('-', '_'), value);
} }
Project.NameKey nameKey = projectState.getNameKey();
env.set("GERRIT_CONTEXT_PATH", req.getContextPath() + "/"); env.set("GERRIT_CONTEXT_PATH", req.getContextPath() + "/");
env.set("GERRIT_PROJECT_NAME", nameKey.get()); env.set("GERRIT_PROJECT_NAME", nameKey.get());
env.set("GITWEB_PROJECTROOT", repoManager.getBasePath(nameKey).toAbsolutePath().toString()); env.set("GITWEB_PROJECTROOT", repoManager.getBasePath(nameKey).toAbsolutePath().toString());
if (permissionBackend if (projectState.statePermitsRead()
.user(anonymousUserProvider) && permissionBackend
.project(nameKey) .user(anonymousUserProvider)
.testOrFalse(ProjectPermission.READ)) { .project(nameKey)
.testOrFalse(ProjectPermission.READ)) {
env.set("GERRIT_ANONYMOUS_READ", "1"); env.set("GERRIT_ANONYMOUS_READ", "1");
} }

View File

@@ -18,6 +18,7 @@ import com.google.common.collect.SetMultimap;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.Capable; import com.google.gerrit.common.data.Capable;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
@@ -228,8 +229,9 @@ public class AsyncReceiveCommits implements PreReceiveHook {
// Check objects mentioned inside the incoming pack file are reachable from visible refs. // Check objects mentioned inside the incoming pack file are reachable from visible refs.
this.perm = permissionBackend.user(user).project(projectName); this.perm = permissionBackend.user(user).project(projectName);
try { try {
projectState.checkStatePermitsRead();
this.perm.check(ProjectPermission.READ); this.perm.check(ProjectPermission.READ);
} catch (AuthException e) { } catch (AuthException | ResourceConflictException e) {
rp.setCheckReferencedObjectsAreReachable(receiveConfig.checkReferencedObjectsAreReachable); rp.setCheckReferencedObjectsAreReachable(receiveConfig.checkReferencedObjectsAreReachable);
} }

View File

@@ -114,7 +114,8 @@ class DefaultRefFilter {
PermissionBackend.WithUser withUser = permissionBackend.user(user); PermissionBackend.WithUser withUser = permissionBackend.user(user);
PermissionBackend.ForProject forProject = withUser.project(projectState.getNameKey()); PermissionBackend.ForProject forProject = withUser.project(projectState.getNameKey());
if (!projectState.isAllUsers()) { if (!projectState.isAllUsers()) {
if (checkProjectPermission(forProject, ProjectPermission.READ)) { if (projectState.statePermitsRead()
&& checkProjectPermission(forProject, ProjectPermission.READ)) {
return refs; return refs;
} else if (projectControl.allRefsAreVisible(ImmutableSet.of(RefNames.REFS_CONFIG))) { } else if (projectControl.allRefsAreVisible(ImmutableSet.of(RefNames.REFS_CONFIG))) {
return fastHideRefsMetaConfig(refs); return fastHideRefsMetaConfig(refs);

View File

@@ -404,7 +404,7 @@ class ProjectControl {
|| isOwner(); || isOwner();
case READ: case READ:
return !isHidden() && allRefsAreVisible(Collections.emptySet()); return allRefsAreVisible(Collections.emptySet());
case CREATE_REF: case CREATE_REF:
return canAddRefs(); return canAddRefs();

View File

@@ -35,6 +35,10 @@ public class ProjectIsVisibleToPredicate extends IsVisibleToPredicate<ProjectDat
@Override @Override
public boolean match(ProjectData pd) throws OrmException { public boolean match(ProjectData pd) throws OrmException {
if (!pd.getProject().getState().permitsRead()) {
return false;
}
return permissionBackend return permissionBackend
.user(user) .user(user)
.project(pd.getProject().getNameKey()) .project(pd.getProject().getNameKey())

View File

@@ -413,16 +413,19 @@ public class ListProjects implements RestReadView<TopLevelResource> {
if (!type.matches(git)) { if (!type.matches(git)) {
continue; continue;
} }
boolean canReadAllRefs;
try { boolean canReadAllRefs = e.statePermitsRead();
permissionBackend if (canReadAllRefs) {
.user(currentUser) try {
.project(e.getNameKey()) permissionBackend
.check(ProjectPermission.READ); .user(currentUser)
canReadAllRefs = true; .project(e.getNameKey())
} catch (AuthException ae) { .check(ProjectPermission.READ);
canReadAllRefs = false; } catch (AuthException exp) {
canReadAllRefs = false;
}
} }
List<Ref> refs = getBranchRefs(projectName, canReadAllRefs); List<Ref> refs = getBranchRefs(projectName, canReadAllRefs);
if (!hasValidRef(refs)) { if (!hasValidRef(refs)) {
continue; continue;

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.sshd.commands; package com.google.gerrit.sshd.commands;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.nio.charset.StandardCharsets.UTF_8; import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.Splitter; import com.google.common.base.Splitter;
@@ -23,6 +24,8 @@ import com.google.gerrit.server.change.ArchiveFormat;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission; 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.restapi.change.AllowedFormats; import com.google.gerrit.server.restapi.change.AllowedFormats;
import com.google.gerrit.server.restapi.project.CommitsCollection; import com.google.gerrit.server.restapi.project.CommitsCollection;
import com.google.gerrit.sshd.AbstractGitCommand; import com.google.gerrit.sshd.AbstractGitCommand;
@@ -123,6 +126,7 @@ public class UploadArchive extends AbstractGitCommand {
@Inject private PermissionBackend permissionBackend; @Inject private PermissionBackend permissionBackend;
@Inject private CommitsCollection commits; @Inject private CommitsCollection commits;
@Inject private AllowedFormats allowedFormats; @Inject private AllowedFormats allowedFormats;
@Inject private ProjectCache projectCache;
private Options options = new Options(); private Options options = new Options();
/** /**
@@ -242,6 +246,13 @@ public class UploadArchive extends AbstractGitCommand {
} }
private boolean canRead(ObjectId revId) throws IOException, PermissionBackendException { private boolean canRead(ObjectId revId) throws IOException, PermissionBackendException {
ProjectState projectState = projectCache.get(projectName);
checkNotNull(projectState, "Failed to load project %s", projectName);
if (!projectState.statePermitsRead()) {
return false;
}
try { try {
permissionBackend.user(user).project(projectName).check(ProjectPermission.READ); permissionBackend.user(user).project(projectName).check(ProjectPermission.READ);
return true; return true;