Remove http request filter implementation from permission filters

This commit is contained in:
René Pfeuffer
2018-09-10 16:58:52 +02:00
parent eb378de87f
commit 1b200ae69d
7 changed files with 35 additions and 139 deletions

View File

@@ -1,12 +1,9 @@
package sonia.scm.repository.spi; package sonia.scm.repository.spi;
import org.apache.shiro.SecurityUtils;
import org.apache.shiro.subject.Subject;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import sonia.scm.api.v2.resources.UriInfoStore; import sonia.scm.api.v2.resources.UriInfoStore;
import sonia.scm.repository.Repository; import sonia.scm.repository.Repository;
import sonia.scm.web.filter.PermissionFilter;
import sonia.scm.web.filter.ProviderPermissionFilter; import sonia.scm.web.filter.ProviderPermissionFilter;
import javax.inject.Provider; import javax.inject.Provider;
@@ -61,32 +58,11 @@ public abstract class InitializingHttpScmProtocolWrapper {
} }
} }
if (getRepository() != null) permissionFilterProvider.get().executeIfPermitted(
{ request,
Subject subject = SecurityUtils.getSubject(); response,
getRepository(),
PermissionFilter permissionFilter = permissionFilterProvider.get(); () -> delegateProvider.get().service(request, response));
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);
} }
} }
}
}
} }

View File

@@ -51,7 +51,6 @@ import sonia.scm.security.ScmSecurityException;
import sonia.scm.util.HttpUtil; import sonia.scm.util.HttpUtil;
import sonia.scm.util.Util; import sonia.scm.util.Util;
import javax.servlet.FilterChain;
import javax.servlet.ServletException; import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;
@@ -65,7 +64,7 @@ import java.util.Iterator;
* *
* @author Sebastian Sdorra * @author Sebastian Sdorra
*/ */
public abstract class PermissionFilter extends HttpFilter public abstract class PermissionFilter
{ {
/** the logger for PermissionFilter */ /** the logger for PermissionFilter */
@@ -81,23 +80,13 @@ public abstract class PermissionFilter extends HttpFilter
* *
* @since 1.21 * @since 1.21
*/ */
public PermissionFilter(ScmConfiguration configuration) protected PermissionFilter(ScmConfiguration configuration)
{ {
this.configuration = configuration; this.configuration = configuration;
} }
//~--- get methods ---------------------------------------------------------- //~--- 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. * 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 * @return returns true if the current request is a write request
*/ */
public abstract boolean isWriteRequest(HttpServletRequest request); protected abstract boolean isWriteRequest(HttpServletRequest request);
//~--- methods -------------------------------------------------------------- //~--- methods --------------------------------------------------------------
@@ -117,23 +106,17 @@ public abstract class PermissionFilter extends HttpFilter
* *
* @param request http request * @param request http request
* @param response http response * @param response http response
* @param chain filter chain
* *
* @throws IOException * @throws IOException
* @throws ServletException * @throws ServletException
*/ */
@Override public void executeIfPermitted(HttpServletRequest request,
protected void doFilter(HttpServletRequest request, HttpServletResponse response, Repository repository, ContinuationServlet continuation)
HttpServletResponse response, FilterChain chain)
throws IOException, ServletException throws IOException, ServletException
{ {
Subject subject = SecurityUtils.getSubject(); Subject subject = SecurityUtils.getSubject();
try try
{
Repository repository = getRepository(request);
if (repository != null)
{ {
boolean writeRequest = isWriteRequest(request); boolean writeRequest = isWriteRequest(request);
@@ -143,7 +126,7 @@ public abstract class PermissionFilter extends HttpFilter
getActionAsString(writeRequest), repository.getName(), getActionAsString(writeRequest), repository.getName(),
getUserName(subject)); getUserName(subject));
chain.doFilter(request, response); continuation.serve();
} }
else else
{ {
@@ -154,13 +137,6 @@ public abstract class PermissionFilter extends HttpFilter
sendAccessDenied(request, response, subject); sendAccessDenied(request, response, subject);
} }
} }
else
{
logger.debug("repository not found");
response.sendError(HttpServletResponse.SC_NOT_FOUND);
}
}
catch (ArgumentIsInvalidException ex) catch (ArgumentIsInvalidException ex)
{ {
if (logger.isTraceEnabled()) if (logger.isTraceEnabled())
@@ -249,7 +225,7 @@ public abstract class PermissionFilter extends HttpFilter
* *
* @throws IOException * @throws IOException
*/ */
public void sendAccessDenied(HttpServletRequest request, private void sendAccessDenied(HttpServletRequest request,
HttpServletResponse response, Subject subject) HttpServletResponse response, Subject subject)
throws IOException throws IOException
{ {
@@ -328,7 +304,7 @@ public abstract class PermissionFilter extends HttpFilter
* *
* @return true if the current user has the required permissions * @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; boolean permitted;
@@ -348,4 +324,9 @@ public abstract class PermissionFilter extends HttpFilter
/** scm-manager global configuration */ /** scm-manager global configuration */
private final ScmConfiguration configuration; private final ScmConfiguration configuration;
@FunctionalInterface
public interface ContinuationServlet {
void serve() throws ServletException, IOException;
}
} }

View File

@@ -35,22 +35,12 @@ package sonia.scm.web.filter;
//~--- non-JDK imports -------------------------------------------------------- //~--- 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.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import sonia.scm.config.ScmConfiguration; import sonia.scm.config.ScmConfiguration;
import sonia.scm.repository.Repository;
import sonia.scm.repository.RepositoryProvider;
//~--- JDK imports ------------------------------------------------------------ //~--- JDK imports ------------------------------------------------------------
import javax.servlet.http.HttpServletRequest;
/** /**
* *
* @author Sebastian Sdorra * @author Sebastian Sdorra
@@ -72,47 +62,10 @@ public abstract class ProviderPermissionFilter extends PermissionFilter
* *
* *
* @param configuration * @param configuration
* @param repositoryProvider
* @since 1.21 * @since 1.21
*/ */
public ProviderPermissionFilter(ScmConfiguration configuration, public ProviderPermissionFilter(ScmConfiguration configuration)
RepositoryProvider repositoryProvider)
{ {
super(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;
} }

View File

@@ -43,7 +43,6 @@ import sonia.scm.Priority;
import sonia.scm.config.ScmConfiguration; import sonia.scm.config.ScmConfiguration;
import sonia.scm.filter.Filters; import sonia.scm.filter.Filters;
import sonia.scm.repository.GitUtil; import sonia.scm.repository.GitUtil;
import sonia.scm.repository.RepositoryProvider;
import sonia.scm.web.filter.ProviderPermissionFilter; import sonia.scm.web.filter.ProviderPermissionFilter;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
@@ -78,11 +77,10 @@ public class GitPermissionFilter extends ProviderPermissionFilter
* Constructs a new instance of the GitPermissionFilter. * Constructs a new instance of the GitPermissionFilter.
* *
* @param configuration scm main configuration * @param configuration scm main configuration
* @param repositoryProvider repository provider
*/ */
@Inject @Inject
public GitPermissionFilter(ScmConfiguration configuration, RepositoryProvider repositoryProvider) { public GitPermissionFilter(ScmConfiguration configuration) {
super(configuration, repositoryProvider); super(configuration);
} }
@Override @Override

View File

@@ -5,7 +5,6 @@ import org.junit.runner.RunWith;
import org.mockito.Mock; import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner; import org.mockito.junit.MockitoJUnitRunner;
import sonia.scm.config.ScmConfiguration; import sonia.scm.config.ScmConfiguration;
import sonia.scm.repository.RepositoryProvider;
import sonia.scm.util.HttpUtil; import sonia.scm.util.HttpUtil;
import javax.servlet.ServletOutputStream; import javax.servlet.ServletOutputStream;
@@ -29,12 +28,7 @@ import static org.mockito.Mockito.when;
@RunWith(MockitoJUnitRunner.class) @RunWith(MockitoJUnitRunner.class)
public class GitPermissionFilterTest { public class GitPermissionFilterTest {
@Mock private final GitPermissionFilter permissionFilter = new GitPermissionFilter(new ScmConfiguration());
private RepositoryProvider repositoryProvider;
private final GitPermissionFilter permissionFilter = new GitPermissionFilter(
new ScmConfiguration(), repositoryProvider
);
@Mock @Mock
private HttpServletResponse response; private HttpServletResponse response;

View File

@@ -38,7 +38,6 @@ package sonia.scm.web;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.inject.Inject; import com.google.inject.Inject;
import sonia.scm.config.ScmConfiguration; import sonia.scm.config.ScmConfiguration;
import sonia.scm.repository.RepositoryProvider;
import sonia.scm.web.filter.ProviderPermissionFilter; import sonia.scm.web.filter.ProviderPermissionFilter;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
@@ -62,13 +61,11 @@ public class HgPermissionFilter extends ProviderPermissionFilter
* Constructs a new instance. * Constructs a new instance.
* *
* @param configuration scm configuration * @param configuration scm configuration
* @param repositoryProvider repository provider
*/ */
@Inject @Inject
public HgPermissionFilter(ScmConfiguration configuration, public HgPermissionFilter(ScmConfiguration configuration)
RepositoryProvider repositoryProvider)
{ {
super(configuration, repositoryProvider); super(configuration);
} }
//~--- get methods ---------------------------------------------------------- //~--- get methods ----------------------------------------------------------

View File

@@ -39,7 +39,6 @@ import com.google.common.collect.ImmutableSet;
import com.google.inject.Inject; import com.google.inject.Inject;
import sonia.scm.ClientMessages; import sonia.scm.ClientMessages;
import sonia.scm.config.ScmConfiguration; import sonia.scm.config.ScmConfiguration;
import sonia.scm.repository.RepositoryProvider;
import sonia.scm.repository.ScmSvnErrorCode; import sonia.scm.repository.ScmSvnErrorCode;
import sonia.scm.repository.SvnUtil; import sonia.scm.repository.SvnUtil;
import sonia.scm.web.filter.ProviderPermissionFilter; import sonia.scm.web.filter.ProviderPermissionFilter;
@@ -71,13 +70,11 @@ public class SvnPermissionFilter extends ProviderPermissionFilter
* Constructs ... * Constructs ...
* *
* @param configuration * @param configuration
* @param repository
*/ */
@Inject @Inject
public SvnPermissionFilter(ScmConfiguration configuration, public SvnPermissionFilter(ScmConfiguration configuration)
RepositoryProvider repository)
{ {
super(configuration, repository); super(configuration);
} }
//~--- methods -------------------------------------------------------------- //~--- methods --------------------------------------------------------------