From aa78cb6f03fe89703060fbc74df2ae2a4eccafed Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Tue, 11 Apr 2023 09:02:09 +0200 Subject: [PATCH] Cleanup hg permissions --- .../scm/web/filter/PermissionFilter.java | 121 +++++------------- .../sonia/scm/web/HgPermissionFilter.java | 14 +- 2 files changed, 35 insertions(+), 100 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/web/filter/PermissionFilter.java b/scm-core/src/main/java/sonia/scm/web/filter/PermissionFilter.java index dd586881ff..e82dde656c 100644 --- a/scm-core/src/main/java/sonia/scm/web/filter/PermissionFilter.java +++ b/scm-core/src/main/java/sonia/scm/web/filter/PermissionFilter.java @@ -49,155 +49,113 @@ import java.io.IOException; * * @author Sebastian Sdorra */ -public abstract class PermissionFilter extends ScmProviderHttpServletDecorator -{ +public abstract class PermissionFilter extends ScmProviderHttpServletDecorator { - /** the logger for PermissionFilter */ - private static final Logger logger = - LoggerFactory.getLogger(PermissionFilter.class); - - //~--- constructors --------------------------------------------------------- + private static final Logger LOG = LoggerFactory.getLogger(PermissionFilter.class); + private final ScmConfiguration configuration; /** * Constructs a new permission filter * * @param configuration global scm-manager configuration - * * @since 1.21 */ - protected PermissionFilter(ScmConfiguration configuration, ScmProviderHttpServlet delegate) - { + protected PermissionFilter(ScmConfiguration configuration, ScmProviderHttpServlet delegate) { super(delegate); this.configuration = configuration; } - //~--- get methods ---------------------------------------------------------- - /** * Returns true if the current request is a write request. * - * * @param request - * * @return returns true if the current request is a write request */ protected abstract boolean isWriteRequest(HttpServletRequest request); - //~--- methods -------------------------------------------------------------- - /** * Checks the permission for the requested repository. If the user has enough * permission, then the filter chain is called. * - * - * @param request http request + * @param request http request * @param response http response - * * @throws IOException * @throws ServletException */ @Override - public void service(HttpServletRequest request, - HttpServletResponse response, Repository repository) - throws IOException, ServletException - { + public void service(HttpServletRequest request, HttpServletResponse response, Repository repository) + throws IOException, ServletException { Subject subject = SecurityUtils.getSubject(); - try - { + try { boolean writeRequest = isWriteRequest(request); - if (hasPermission(repository, writeRequest)) - { - logger.trace("{} access to repository {} for user {} granted", + if (hasPermission(repository, writeRequest)) { + LOG.trace("{} access to repository {} for user {} granted", getActionAsString(writeRequest), repository.getName(), getUserName(subject)); super.service(request, response, repository); - } - else - { - logger.info("{} access to repository {} for user {} denied", + } else { + LOG.info("{} access to repository {} for user {} denied", getActionAsString(writeRequest), repository.getName(), getUserName(subject)); sendAccessDenied(request, response); } - } - catch (ScmSecurityException | AuthorizationException ex) - { - logger.warn("user " + subject.getPrincipal() + " has not enough permissions", ex); + } catch (ScmSecurityException | AuthorizationException ex) { + LOG.warn("user " + subject.getPrincipal() + " has not enough permissions", ex); sendAccessDenied(request, response); } - } /** * Sends a "not enough privileges" error back to client. * - * - * @param request http request + * @param request http request * @param response http response - * * @throws IOException */ - protected void sendNotEnoughPrivilegesError(HttpServletRequest request, - HttpServletResponse response) - throws IOException - { + protected void sendNotEnoughPrivilegesError(HttpServletRequest request, HttpServletResponse response) + throws IOException { response.sendError(HttpServletResponse.SC_FORBIDDEN); } /** * Sends an unauthorized error back to client. * - * - * @param request http request + * @param request http request * @param response http response - * * @throws IOException */ - protected void sendUnauthorizedError(HttpServletRequest request, - HttpServletResponse response) - throws IOException - { + protected void sendUnauthorizedError(HttpServletRequest request, HttpServletResponse response) + throws IOException { HttpUtil.sendUnauthorized(response, configuration.getRealmDescription()); } /** * Send access denied to the servlet response. * - * @param request current http request object + * @param request current http request object * @param response current http response object - * * @throws IOException */ - private void sendAccessDenied(HttpServletRequest request, - HttpServletResponse response) - throws IOException - { - if (!Authentications.isAuthenticatedSubjectAnonymous()) - { + private void sendAccessDenied(HttpServletRequest request, HttpServletResponse response) + throws IOException { + if (!Authentications.isAuthenticatedSubjectAnonymous()) { sendNotEnoughPrivilegesError(request, response); - } - else - { + } else { sendUnauthorizedError(request, response); } } - //~--- get methods ---------------------------------------------------------- - /** * Returns action as string. * - * * @param writeRequest true if the action is a write action - * * @return action as string */ - private String getActionAsString(boolean writeRequest) - { + private String getActionAsString(boolean writeRequest) { return writeRequest ? "write" : "read"; @@ -206,17 +164,13 @@ public abstract class PermissionFilter extends ScmProviderHttpServletDecorator /** * Returns the username from the given subject or anonymous. * - * * @param subject user subject - * * @return username username from subject or anonymous */ - private Object getUserName(Subject subject) - { + private Object getUserName(Subject subject) { Object principal = subject.getPrincipal(); - if (principal == null) - { + if (principal == null) { principal = SCMContext.USER_ANONYMOUS; } @@ -226,30 +180,19 @@ public abstract class PermissionFilter extends ScmProviderHttpServletDecorator /** * Returns true if the current user has the required permissions. * - * - * @param repository repository for the permissions check + * @param repository repository for the permissions check * @param writeRequest true if request is a write request - * * @return true if the current user has the required permissions */ - public static boolean hasPermission(Repository repository, boolean writeRequest) - { + public static boolean hasPermission(Repository repository, boolean writeRequest) { boolean permitted; - if (writeRequest) - { + if (writeRequest) { permitted = RepositoryPermissions.push(repository).isPermitted(); - } - else - { + } else { permitted = RepositoryPermissions.pull(repository).isPermitted(); } return permitted; } - - //~--- fields --------------------------------------------------------------- - - /** scm-manager global configuration */ - private final ScmConfiguration configuration; } diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgPermissionFilter.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgPermissionFilter.java index 56a0f65e60..6e8449113e 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgPermissionFilter.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgPermissionFilter.java @@ -35,7 +35,6 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; -import java.util.Set; /** * Permission filter for mercurial repositories. @@ -44,8 +43,6 @@ import java.util.Set; */ public class HgPermissionFilter extends PermissionFilter { - private static final Set READ_METHODS = Set.of("GET", "HEAD", "OPTIONS", "TRACE"); - private final HgRepositoryHandler repositoryHandler; public HgPermissionFilter(ScmConfiguration configuration, ScmProviderHttpServlet delegate, HgRepositoryHandler repositoryHandler) { @@ -68,17 +65,12 @@ public class HgPermissionFilter extends PermissionFilter { @Override public boolean isWriteRequest(HttpServletRequest request) { - if (isHttpPostArgsEnabled()) { - return true; - } - return isDefaultWriteRequest(request); + // The repository permissions for read and write are handled by hg internally. + // Therefore, we ignore the checks here. + return false; } private boolean isHttpPostArgsEnabled() { return repositoryHandler.getConfig().isEnableHttpPostArgs(); } - - private boolean isDefaultWriteRequest(HttpServletRequest request) { - return !READ_METHODS.contains(request.getMethod()); - } }