From 1b200ae69dfa6e07078c363430f43ded7e79ed20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 10 Sep 2018 16:58:52 +0200 Subject: [PATCH] Remove http request filter implementation from permission filters --- .../InitializingHttpScmProtocolWrapper.java | 34 ++-------- .../scm/web/filter/PermissionFilter.java | 63 +++++++------------ .../web/filter/ProviderPermissionFilter.java | 49 +-------------- .../sonia/scm/web/GitPermissionFilter.java | 6 +- .../scm/web/GitPermissionFilterTest.java | 8 +-- .../sonia/scm/web/HgPermissionFilter.java | 7 +-- .../sonia/scm/web/SvnPermissionFilter.java | 7 +-- 7 files changed, 35 insertions(+), 139 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapper.java b/scm-core/src/main/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapper.java index ba63cf8501..12a62d13c5 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapper.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapper.java @@ -1,12 +1,9 @@ package sonia.scm.repository.spi; -import org.apache.shiro.SecurityUtils; -import org.apache.shiro.subject.Subject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.api.v2.resources.UriInfoStore; import sonia.scm.repository.Repository; -import sonia.scm.web.filter.PermissionFilter; import sonia.scm.web.filter.ProviderPermissionFilter; import javax.inject.Provider; @@ -61,32 +58,11 @@ public abstract class InitializingHttpScmProtocolWrapper { } } - if (getRepository() != null) - { - Subject subject = SecurityUtils.getSubject(); - - PermissionFilter permissionFilter = permissionFilterProvider.get(); - boolean writeRequest = permissionFilter.isWriteRequest(request); - - if (permissionFilter.hasPermission(getRepository(), writeRequest)) - { -// logger.trace("{} access to repository {} for user {} granted", -// getActionAsString(writeRequest), repository.getName(), -// getUserName(subject)); - - delegateProvider.get().service(request, response); - } - else - { -// logger.info("{} access to repository {} for user {} denied", -// getActionAsString(writeRequest), repository.getName(), -// getUserName(subject)); - - permissionFilter.sendAccessDenied(request, response, subject); - } - } - + permissionFilterProvider.get().executeIfPermitted( + request, + response, + getRepository(), + () -> delegateProvider.get().service(request, response)); } } - } 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 732bac1c57..8831756234 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 @@ -51,7 +51,6 @@ import sonia.scm.security.ScmSecurityException; import sonia.scm.util.HttpUtil; import sonia.scm.util.Util; -import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -65,7 +64,7 @@ import java.util.Iterator; * * @author Sebastian Sdorra */ -public abstract class PermissionFilter extends HttpFilter +public abstract class PermissionFilter { /** the logger for PermissionFilter */ @@ -81,23 +80,13 @@ public abstract class PermissionFilter extends HttpFilter * * @since 1.21 */ - public PermissionFilter(ScmConfiguration configuration) + protected PermissionFilter(ScmConfiguration configuration) { this.configuration = configuration; } //~--- get methods ---------------------------------------------------------- - /** - * Returns the requested repository. - * - * - * @param request current http request - * - * @return requested repository - */ - protected abstract Repository getRepository(HttpServletRequest request); - /** * Returns true if the current request is a write request. * @@ -106,7 +95,7 @@ public abstract class PermissionFilter extends HttpFilter * * @return returns true if the current request is a write request */ - public abstract boolean isWriteRequest(HttpServletRequest request); + protected abstract boolean isWriteRequest(HttpServletRequest request); //~--- methods -------------------------------------------------------------- @@ -117,48 +106,35 @@ public abstract class PermissionFilter extends HttpFilter * * @param request http request * @param response http response - * @param chain filter chain * * @throws IOException * @throws ServletException */ - @Override - protected void doFilter(HttpServletRequest request, - HttpServletResponse response, FilterChain chain) + public void executeIfPermitted(HttpServletRequest request, + HttpServletResponse response, Repository repository, ContinuationServlet continuation) throws IOException, ServletException { Subject subject = SecurityUtils.getSubject(); try { - Repository repository = getRepository(request); + boolean writeRequest = isWriteRequest(request); - if (repository != null) + if (hasPermission(repository, writeRequest)) { - boolean writeRequest = isWriteRequest(request); + logger.trace("{} access to repository {} for user {} granted", + getActionAsString(writeRequest), repository.getName(), + getUserName(subject)); - if (hasPermission(repository, writeRequest)) - { - logger.trace("{} access to repository {} for user {} granted", - getActionAsString(writeRequest), repository.getName(), - getUserName(subject)); - - chain.doFilter(request, response); - } - else - { - logger.info("{} access to repository {} for user {} denied", - getActionAsString(writeRequest), repository.getName(), - getUserName(subject)); - - sendAccessDenied(request, response, subject); - } + continuation.serve(); } else { - logger.debug("repository not found"); + logger.info("{} access to repository {} for user {} denied", + getActionAsString(writeRequest), repository.getName(), + getUserName(subject)); - response.sendError(HttpServletResponse.SC_NOT_FOUND); + sendAccessDenied(request, response, subject); } } catch (ArgumentIsInvalidException ex) @@ -249,7 +225,7 @@ public abstract class PermissionFilter extends HttpFilter * * @throws IOException */ - public void sendAccessDenied(HttpServletRequest request, + private void sendAccessDenied(HttpServletRequest request, HttpServletResponse response, Subject subject) throws IOException { @@ -328,7 +304,7 @@ public abstract class PermissionFilter extends HttpFilter * * @return true if the current user has the required permissions */ - public boolean hasPermission(Repository repository, boolean writeRequest) + private boolean hasPermission(Repository repository, boolean writeRequest) { boolean permitted; @@ -348,4 +324,9 @@ public abstract class PermissionFilter extends HttpFilter /** scm-manager global configuration */ private final ScmConfiguration configuration; + + @FunctionalInterface + public interface ContinuationServlet { + void serve() throws ServletException, IOException; + } } diff --git a/scm-core/src/main/java/sonia/scm/web/filter/ProviderPermissionFilter.java b/scm-core/src/main/java/sonia/scm/web/filter/ProviderPermissionFilter.java index ea0d90c915..280ea0a77b 100644 --- a/scm-core/src/main/java/sonia/scm/web/filter/ProviderPermissionFilter.java +++ b/scm-core/src/main/java/sonia/scm/web/filter/ProviderPermissionFilter.java @@ -35,22 +35,12 @@ package sonia.scm.web.filter; //~--- non-JDK imports -------------------------------------------------------- -import com.google.common.base.Throwables; -import com.google.inject.ProvisionException; - -import org.apache.shiro.authz.AuthorizationException; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import sonia.scm.config.ScmConfiguration; -import sonia.scm.repository.Repository; -import sonia.scm.repository.RepositoryProvider; //~--- JDK imports ------------------------------------------------------------ -import javax.servlet.http.HttpServletRequest; - /** * * @author Sebastian Sdorra @@ -72,47 +62,10 @@ public abstract class ProviderPermissionFilter extends PermissionFilter * * * @param configuration - * @param repositoryProvider * @since 1.21 */ - public ProviderPermissionFilter(ScmConfiguration configuration, - RepositoryProvider repositoryProvider) + public ProviderPermissionFilter(ScmConfiguration configuration) { super(configuration); - this.repositoryProvider = repositoryProvider; } - - //~--- get methods ---------------------------------------------------------- - - /** - * Method description - * - * - * @param request - * - * @return - */ - @Override - protected Repository getRepository(HttpServletRequest request) - { - Repository repository = null; - - try - { - repository = repositoryProvider.get(); - } - catch (ProvisionException ex) - { - Throwables.propagateIfPossible(ex.getCause(), - IllegalStateException.class, AuthorizationException.class); - logger.error("could not get repository from request", ex); - } - - return repository; - } - - //~--- fields --------------------------------------------------------------- - - /** Field description */ - private final RepositoryProvider repositoryProvider; } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitPermissionFilter.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitPermissionFilter.java index ca88e06a41..5606b73762 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitPermissionFilter.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitPermissionFilter.java @@ -43,7 +43,6 @@ import sonia.scm.Priority; import sonia.scm.config.ScmConfiguration; import sonia.scm.filter.Filters; import sonia.scm.repository.GitUtil; -import sonia.scm.repository.RepositoryProvider; import sonia.scm.web.filter.ProviderPermissionFilter; import javax.servlet.http.HttpServletRequest; @@ -78,11 +77,10 @@ public class GitPermissionFilter extends ProviderPermissionFilter * Constructs a new instance of the GitPermissionFilter. * * @param configuration scm main configuration - * @param repositoryProvider repository provider */ @Inject - public GitPermissionFilter(ScmConfiguration configuration, RepositoryProvider repositoryProvider) { - super(configuration, repositoryProvider); + public GitPermissionFilter(ScmConfiguration configuration) { + super(configuration); } @Override diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/GitPermissionFilterTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/GitPermissionFilterTest.java index fede8842a9..8e7ff3d954 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/GitPermissionFilterTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/GitPermissionFilterTest.java @@ -5,7 +5,6 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import sonia.scm.config.ScmConfiguration; -import sonia.scm.repository.RepositoryProvider; import sonia.scm.util.HttpUtil; import javax.servlet.ServletOutputStream; @@ -29,12 +28,7 @@ import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class GitPermissionFilterTest { - @Mock - private RepositoryProvider repositoryProvider; - - private final GitPermissionFilter permissionFilter = new GitPermissionFilter( - new ScmConfiguration(), repositoryProvider - ); + private final GitPermissionFilter permissionFilter = new GitPermissionFilter(new ScmConfiguration()); @Mock private HttpServletResponse response; 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 9350f8399c..3a1aba93e4 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 @@ -38,7 +38,6 @@ package sonia.scm.web; import com.google.common.collect.ImmutableSet; import com.google.inject.Inject; import sonia.scm.config.ScmConfiguration; -import sonia.scm.repository.RepositoryProvider; import sonia.scm.web.filter.ProviderPermissionFilter; import javax.servlet.http.HttpServletRequest; @@ -62,13 +61,11 @@ public class HgPermissionFilter extends ProviderPermissionFilter * Constructs a new instance. * * @param configuration scm configuration - * @param repositoryProvider repository provider */ @Inject - public HgPermissionFilter(ScmConfiguration configuration, - RepositoryProvider repositoryProvider) + public HgPermissionFilter(ScmConfiguration configuration) { - super(configuration, repositoryProvider); + super(configuration); } //~--- get methods ---------------------------------------------------------- diff --git a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/web/SvnPermissionFilter.java b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/web/SvnPermissionFilter.java index 43a4f1baec..2939b44753 100644 --- a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/web/SvnPermissionFilter.java +++ b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/web/SvnPermissionFilter.java @@ -39,7 +39,6 @@ import com.google.common.collect.ImmutableSet; import com.google.inject.Inject; import sonia.scm.ClientMessages; import sonia.scm.config.ScmConfiguration; -import sonia.scm.repository.RepositoryProvider; import sonia.scm.repository.ScmSvnErrorCode; import sonia.scm.repository.SvnUtil; import sonia.scm.web.filter.ProviderPermissionFilter; @@ -71,13 +70,11 @@ public class SvnPermissionFilter extends ProviderPermissionFilter * Constructs ... * * @param configuration - * @param repository */ @Inject - public SvnPermissionFilter(ScmConfiguration configuration, - RepositoryProvider repository) + public SvnPermissionFilter(ScmConfiguration configuration) { - super(configuration, repository); + super(configuration); } //~--- methods --------------------------------------------------------------