Implement permission filters as decorator for servlets

This commit is contained in:
René Pfeuffer
2018-09-14 10:21:00 +02:00
parent 7de9f69061
commit e88639ab81
14 changed files with 122 additions and 71 deletions

View File

@@ -6,7 +6,6 @@ import sonia.scm.api.v2.resources.ScmPathInfoStore;
import sonia.scm.config.ScmConfiguration;
import sonia.scm.repository.Repository;
import sonia.scm.repository.api.ScmProtocolProvider;
import sonia.scm.web.filter.PermissionFilter;
import javax.inject.Provider;
import javax.servlet.ServletConfig;
@@ -24,16 +23,14 @@ public abstract class InitializingHttpScmProtocolWrapper implements ScmProtocolP
private static final Logger logger = LoggerFactory.getLogger(InitializingHttpScmProtocolWrapper.class);
private final Provider<? extends ScmProviderHttpServlet> delegateProvider;
private final Provider<? extends PermissionFilter> permissionFilterProvider;
private final Provider<ScmPathInfoStore> uriInfoStore;
private final ScmConfiguration scmConfiguration;
private volatile boolean isInitialized = false;
protected InitializingHttpScmProtocolWrapper(Provider<? extends ScmProviderHttpServlet> delegateProvider, Provider<? extends PermissionFilter> permissionFilterProvider, Provider<ScmPathInfoStore> uriInfoStore, ScmConfiguration scmConfiguration) {
protected InitializingHttpScmProtocolWrapper(Provider<? extends ScmProviderHttpServlet> delegateProvider, Provider<ScmPathInfoStore> uriInfoStore, ScmConfiguration scmConfiguration) {
this.delegateProvider = delegateProvider;
this.permissionFilterProvider = permissionFilterProvider;
this.uriInfoStore = uriInfoStore;
this.scmConfiguration = scmConfiguration;
}
@@ -89,11 +86,7 @@ public abstract class InitializingHttpScmProtocolWrapper implements ScmProtocolP
}
}
permissionFilterProvider.get().executeIfPermitted(
request,
response,
repository,
() -> delegateProvider.get().service(request, response, repository));
delegateProvider.get().service(request, response, repository);
}
}

View File

@@ -4,6 +4,7 @@ import com.google.inject.Inject;
import sonia.scm.util.Decorators;
import javax.inject.Provider;
import java.util.List;
import java.util.Set;
import static java.util.stream.Collectors.toList;
@@ -21,7 +22,11 @@ public abstract class ScmProviderHttpServletProvider implements Provider<ScmProv
@Override
public ScmProviderHttpServlet get() {
return Decorators.decorate(getRootServlet(), decoratorFactories.stream().filter(d -> d.handlesScmType(type)).collect(toList()));
return Decorators.decorate(getRootServlet(), getDecoratorsForType());
}
private List<ScmProviderHttpServletDecoratorFactory> getDecoratorsForType() {
return decoratorFactories.stream().filter(d -> d.handlesScmType(type)).collect(toList());
}
protected abstract ScmProviderHttpServlet getRootServlet();

View File

@@ -42,6 +42,8 @@ import sonia.scm.SCMContext;
import sonia.scm.config.ScmConfiguration;
import sonia.scm.repository.Repository;
import sonia.scm.repository.RepositoryPermissions;
import sonia.scm.repository.spi.ScmProviderHttpServlet;
import sonia.scm.repository.spi.ScmProviderHttpServletDecorator;
import sonia.scm.security.Role;
import sonia.scm.security.ScmSecurityException;
import sonia.scm.util.HttpUtil;
@@ -56,7 +58,7 @@ import java.io.IOException;
*
* @author Sebastian Sdorra
*/
public abstract class PermissionFilter
public abstract class PermissionFilter extends ScmProviderHttpServletDecorator
{
/** the logger for PermissionFilter */
@@ -72,8 +74,9 @@ public abstract class PermissionFilter
*
* @since 1.21
*/
protected PermissionFilter(ScmConfiguration configuration)
protected PermissionFilter(ScmConfiguration configuration, ScmProviderHttpServlet delegate)
{
super(delegate);
this.configuration = configuration;
}
@@ -102,8 +105,9 @@ public abstract class PermissionFilter
* @throws IOException
* @throws ServletException
*/
public void executeIfPermitted(HttpServletRequest request,
HttpServletResponse response, Repository repository, ContinuationServlet continuation)
@Override
public void service(HttpServletRequest request,
HttpServletResponse response, Repository repository)
throws IOException, ServletException
{
Subject subject = SecurityUtils.getSubject();
@@ -118,7 +122,7 @@ public abstract class PermissionFilter
getActionAsString(writeRequest), repository.getName(),
getUserName(subject));
continuation.doService();
super.service(request, response, repository);
}
else
{
@@ -258,9 +262,4 @@ public abstract class PermissionFilter
/** scm-manager global configuration */
private final ScmConfiguration configuration;
@FunctionalInterface
public interface ContinuationServlet {
void doService() throws ServletException, IOException;
}
}

View File

@@ -5,13 +5,11 @@ import com.google.inject.util.Providers;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.stubbing.Answer;
import org.mockito.stubbing.OngoingStubbing;
import sonia.scm.api.v2.resources.ScmPathInfo;
import sonia.scm.api.v2.resources.ScmPathInfoStore;
import sonia.scm.config.ScmConfiguration;
import sonia.scm.repository.Repository;
import sonia.scm.web.filter.PermissionFilter;
import javax.inject.Provider;
import javax.servlet.ServletConfig;
@@ -22,12 +20,7 @@ import java.io.IOException;
import java.net.URI;
import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.same;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -40,8 +33,6 @@ public class InitializingHttpScmProtocolWrapperTest {
@Mock
private ScmProviderHttpServlet delegateServlet;
@Mock
private PermissionFilter permissionFilter;
@Mock
private ScmPathInfoStore pathInfoStore;
@Mock
private ScmConfiguration scmConfiguration;
@@ -62,7 +53,7 @@ public class InitializingHttpScmProtocolWrapperTest {
pathInfoStoreProvider = mock(Provider.class);
when(pathInfoStoreProvider.get()).thenReturn(pathInfoStore);
wrapper = new InitializingHttpScmProtocolWrapper(Providers.of(this.delegateServlet), Providers.of(permissionFilter), pathInfoStoreProvider, scmConfiguration) {
wrapper = new InitializingHttpScmProtocolWrapper(Providers.of(this.delegateServlet), pathInfoStoreProvider, scmConfiguration) {
@Override
public String getType() {
return "git";
@@ -98,8 +89,6 @@ public class InitializingHttpScmProtocolWrapperTest {
@Test
public void shouldInitializeAndDelegateRequestThroughFilter() throws ServletException, IOException {
doAnswer(proceedInvocation()).
when(permissionFilter).executeIfPermitted(same(request), same(response), same(REPOSITORY), any());
HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY);
httpScmProtocol.serve(request, response, servletConfig);
@@ -108,21 +97,8 @@ public class InitializingHttpScmProtocolWrapperTest {
verify(delegateServlet).service(request, response, REPOSITORY);
}
@Test
public void shouldNotDelegateRequestWhenFilterBlocks() throws ServletException, IOException {
doNothing().
when(permissionFilter).executeIfPermitted(same(request), same(response), same(REPOSITORY), any());
HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY);
httpScmProtocol.serve(request, response, servletConfig);
verify(delegateServlet, never()).service(request, response, REPOSITORY);
}
@Test
public void shouldInitializeOnlyOnce() throws ServletException, IOException {
doAnswer(proceedInvocation()).
when(permissionFilter).executeIfPermitted(same(request), same(response), same(REPOSITORY), any());
HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY);
httpScmProtocol.serve(request, response, servletConfig);
@@ -137,13 +113,6 @@ public class InitializingHttpScmProtocolWrapperTest {
HttpScmProtocol httpScmProtocol = wrapper.get(new Repository("", "other", "space", "name"));
}
private Answer proceedInvocation() {
return invocation -> {
((PermissionFilter.ContinuationServlet) invocation.getArgument(3)).doService();
return null;
};
}
private OngoingStubbing<ScmPathInfo> mockSetPathInfo() {
return when(pathInfoStore.get()).thenReturn(() -> URI.create("http://example.com/scm/api/rest/"));
}

View File

@@ -34,11 +34,11 @@
package sonia.scm.web;
import com.google.common.annotations.VisibleForTesting;
import com.google.inject.Inject;
import org.eclipse.jgit.http.server.GitSmartHttpTools;
import sonia.scm.ClientMessages;
import sonia.scm.config.ScmConfiguration;
import sonia.scm.repository.GitUtil;
import sonia.scm.repository.spi.ScmProviderHttpServlet;
import sonia.scm.web.filter.PermissionFilter;
import javax.servlet.http.HttpServletRequest;
@@ -70,9 +70,8 @@ public class GitPermissionFilter extends PermissionFilter
*
* @param configuration scm main configuration
*/
@Inject
public GitPermissionFilter(ScmConfiguration configuration) {
super(configuration);
public GitPermissionFilter(ScmConfiguration configuration, ScmProviderHttpServlet delegate) {
super(configuration, delegate);
}
@Override

View File

@@ -0,0 +1,29 @@
package sonia.scm.web;
import sonia.scm.config.ScmConfiguration;
import sonia.scm.plugin.Extension;
import sonia.scm.repository.spi.ScmProviderHttpServlet;
import sonia.scm.repository.spi.ScmProviderHttpServletDecoratorFactory;
import javax.inject.Inject;
@Extension
public class GitPermissionFilterFactory implements ScmProviderHttpServletDecoratorFactory {
private final ScmConfiguration configuration;
@Inject
public GitPermissionFilterFactory(ScmConfiguration configuration) {
this.configuration = configuration;
}
@Override
public boolean handlesScmType(String type) {
return "git".equals(type);
}
@Override
public ScmProviderHttpServlet createDecorator(ScmProviderHttpServlet delegate) {
return new GitPermissionFilter(configuration, delegate);
}
}

View File

@@ -14,8 +14,8 @@ import javax.inject.Singleton;
@Extension
public class GitScmProtocolProviderWrapper extends InitializingHttpScmProtocolWrapper {
@Inject
public GitScmProtocolProviderWrapper(@Git Provider<ScmProviderHttpServlet> servletProvider, Provider<GitPermissionFilter> permissionFilter, Provider<ScmPathInfoStore> uriInfoStore, ScmConfiguration scmConfiguration) {
super(servletProvider, permissionFilter, uriInfoStore, scmConfiguration);
public GitScmProtocolProviderWrapper(@Git Provider<ScmProviderHttpServlet> servletProvider, Provider<ScmPathInfoStore> uriInfoStore, ScmConfiguration scmConfiguration) {
super(servletProvider, uriInfoStore, scmConfiguration);
}
@Override

View File

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

View File

@@ -34,8 +34,8 @@
package sonia.scm.web;
import com.google.common.collect.ImmutableSet;
import com.google.inject.Inject;
import sonia.scm.config.ScmConfiguration;
import sonia.scm.repository.spi.ScmProviderHttpServlet;
import sonia.scm.web.filter.PermissionFilter;
import javax.servlet.http.HttpServletRequest;
@@ -51,10 +51,9 @@ public class HgPermissionFilter extends PermissionFilter
private static final Set<String> READ_METHODS = ImmutableSet.of("GET", "HEAD", "OPTIONS", "TRACE");
@Inject
public HgPermissionFilter(ScmConfiguration configuration)
public HgPermissionFilter(ScmConfiguration configuration, ScmProviderHttpServlet delegate)
{
super(configuration);
super(configuration, delegate);
}
@Override

View File

@@ -0,0 +1,29 @@
package sonia.scm.web;
import sonia.scm.config.ScmConfiguration;
import sonia.scm.plugin.Extension;
import sonia.scm.repository.spi.ScmProviderHttpServlet;
import sonia.scm.repository.spi.ScmProviderHttpServletDecoratorFactory;
import javax.inject.Inject;
@Extension
public class HgPermissionFilterFactory implements ScmProviderHttpServletDecoratorFactory {
private final ScmConfiguration configuration;
@Inject
public HgPermissionFilterFactory(ScmConfiguration configuration) {
this.configuration = configuration;
}
@Override
public boolean handlesScmType(String type) {
return "hg".equals(type);
}
@Override
public ScmProviderHttpServlet createDecorator(ScmProviderHttpServlet delegate) {
return new HgPermissionFilter(configuration, delegate);
}
}

View File

@@ -14,8 +14,8 @@ import javax.inject.Singleton;
@Extension
public class HgScmProtocolProviderWrapper extends InitializingHttpScmProtocolWrapper {
@Inject
public HgScmProtocolProviderWrapper(@Hg Provider<ScmProviderHttpServlet> servletProvider, Provider<HgPermissionFilter> permissionFilter, Provider<ScmPathInfoStore> uriInfoStore, ScmConfiguration scmConfiguration) {
super(servletProvider, permissionFilter, uriInfoStore, scmConfiguration);
public HgScmProtocolProviderWrapper(@Hg Provider<ScmProviderHttpServlet> servletProvider, Provider<ScmPathInfoStore> uriInfoStore, ScmConfiguration scmConfiguration) {
super(servletProvider, uriInfoStore, scmConfiguration);
}
@Override

View File

@@ -34,11 +34,11 @@
package sonia.scm.web;
import com.google.common.collect.ImmutableSet;
import com.google.inject.Inject;
import sonia.scm.ClientMessages;
import sonia.scm.config.ScmConfiguration;
import sonia.scm.repository.ScmSvnErrorCode;
import sonia.scm.repository.SvnUtil;
import sonia.scm.repository.spi.ScmProviderHttpServlet;
import sonia.scm.web.filter.PermissionFilter;
import javax.servlet.http.HttpServletRequest;
@@ -65,10 +65,9 @@ public class SvnPermissionFilter extends PermissionFilter
*
* @param configuration
*/
@Inject
public SvnPermissionFilter(ScmConfiguration configuration)
public SvnPermissionFilter(ScmConfiguration configuration, ScmProviderHttpServlet delegate)
{
super(configuration);
super(configuration, delegate);
}
//~--- methods --------------------------------------------------------------

View File

@@ -0,0 +1,29 @@
package sonia.scm.web;
import sonia.scm.config.ScmConfiguration;
import sonia.scm.plugin.Extension;
import sonia.scm.repository.spi.ScmProviderHttpServlet;
import sonia.scm.repository.spi.ScmProviderHttpServletDecoratorFactory;
import javax.inject.Inject;
@Extension
public class SvnPermissionFilterFactory implements ScmProviderHttpServletDecoratorFactory {
private final ScmConfiguration configuration;
@Inject
public SvnPermissionFilterFactory(ScmConfiguration configuration) {
this.configuration = configuration;
}
@Override
public boolean handlesScmType(String type) {
return "svn".equals(type);
}
@Override
public ScmProviderHttpServlet createDecorator(ScmProviderHttpServlet delegate) {
return new SvnPermissionFilter(configuration, delegate);
}
}

View File

@@ -26,8 +26,8 @@ public class SvnScmProtocolProviderWrapper extends InitializingHttpScmProtocolWr
}
@Inject
public SvnScmProtocolProviderWrapper(@Svn Provider<ScmProviderHttpServlet> servletProvider, Provider<SvnPermissionFilter> permissionFilter, Provider<ScmPathInfoStore> uriInfoStore, ScmConfiguration scmConfiguration) {
super(servletProvider, permissionFilter, uriInfoStore, scmConfiguration);
public SvnScmProtocolProviderWrapper(@Svn Provider<ScmProviderHttpServlet> servletProvider, Provider<ScmPathInfoStore> uriInfoStore, ScmConfiguration scmConfiguration) {
super(servletProvider, uriInfoStore, scmConfiguration);
}
@Override