Cleanup hg permissions

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;
 }

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<String> 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());
-  }
 }