From ab61c85ddc5eae33e68cf41f1d472b311097ca7e Mon Sep 17 00:00:00 2001
From: cramakri <cramakri>
Date: Tue, 21 Sep 2010 08:04:12 +0000
Subject: [PATCH] LMS-1702 Annotation-based authentication for DSS Services.
SVN: 17923
---
.../DssServiceRpcAuthorizationAdvisor.java | 76 +++++++-
.../client/api/v1/impl/DssComponentTest.java | 178 ++++++++++++++++--
2 files changed, 236 insertions(+), 18 deletions(-)
diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/DssServiceRpcAuthorizationAdvisor.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/DssServiceRpcAuthorizationAdvisor.java
index 5c1ed2a384f..e5a5c410265 100644
--- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/DssServiceRpcAuthorizationAdvisor.java
+++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/DssServiceRpcAuthorizationAdvisor.java
@@ -18,16 +18,32 @@ package ch.systemsx.cisd.openbis.dss.generic.server;
import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;
+import org.apache.log4j.Logger;
import org.springframework.aop.support.DefaultPointcutAdvisor;
import org.springframework.aop.support.annotation.AnnotationMatchingPointcut;
+import ch.systemsx.cisd.common.exceptions.AuthorizationFailureException;
+import ch.systemsx.cisd.common.logging.LogCategory;
+import ch.systemsx.cisd.common.logging.LogFactory;
+import ch.systemsx.cisd.common.utilities.MethodUtils;
import ch.systemsx.cisd.openbis.dss.generic.shared.api.authorization.DataSetAccessGuard;
+import ch.systemsx.cisd.openbis.dss.generic.shared.api.v1.DataSetFileDTO;
/**
* The advisor for authorization in the DSS RPC interfaces.
* <p>
- * This AOP advisor ensures that invocations to the DSS RPC services pass the authorization
- * requirements.
+ * This AOP advisor ensures that invocations of methods on DSS RPC services with the
+ * {@link DataSetAccessGuard} annotation conform to the following the authorization requirements:
+ * <ul>
+ * <li>The session token (String) is the the first parameter</li>
+ * <li>The second parameter is either a data set code (String) or a DataSetFileDTO object</li>
+ * <li>The user who owns the session token (first argument) has access to the specified data set
+ * code (second argument)</li>
+ * </ul>
+ * <p>
+ * It does this check by invoking the method
+ * {@link AbstractDssServiceRpc#isDatasetAccessible(String, String)} on the receiver of the method
+ * containing the join point.
* <p>
* Though it is not necessary to subclass DefaultPointcutAdvisor for the implementation, we subclass
* here because to make the configuration in spring a bit simpler.
@@ -38,6 +54,9 @@ public class DssServiceRpcAuthorizationAdvisor extends DefaultPointcutAdvisor
{
private static final long serialVersionUID = 1L;
+ private static final Logger authorizationLog =
+ LogFactory.getLogger(LogCategory.AUTH, DssServiceRpcAuthorizationAdvisor.class);
+
/**
* The public constructor.
*/
@@ -61,9 +80,62 @@ public class DssServiceRpcAuthorizationAdvisor extends DefaultPointcutAdvisor
public Object invoke(MethodInvocation methodInvocation) throws Throwable
{
+ final Object[] args = methodInvocation.getArguments();
+ String sessionToken = getSessionToken(args);
+ String dataSetCode = getDataSetCode(args, methodInvocation);
+ AbstractDssServiceRpc recv = (AbstractDssServiceRpc) methodInvocation.getThis();
+ if (false == recv.isDatasetAccessible(sessionToken, dataSetCode))
+ {
+ authorizationLog.info(String.format(
+ "[SESSION:'%s' DATA_SET:%s]: Authorization failure while "
+ + "invoking method '%s'", sessionToken, dataSetCode, MethodUtils
+ .describeMethod(methodInvocation.getMethod())));
+
+ throw new AuthorizationFailureException("User does not have access to data set "
+ + dataSetCode);
+ }
+
return methodInvocation.proceed();
}
+ /**
+ * Get the session token from the MethodInvocation
+ */
+ private static String getSessionToken(final Object[] args)
+ {
+ final int len = args.length;
+ assert len > 0 : "The session token should be the first argument.";
+ final Object firstObject = args[0];
+ assert firstObject instanceof String : "The session token should be the first argument.";
+
+ return (String) firstObject;
+ }
+
+ private static String getDataSetCode(final Object[] args, MethodInvocation methodInvocation)
+ {
+ final int len = args.length;
+ assert len > 1 : "The data set code should be contained in the second argument.";
+ final Object secondObject = args[1];
+ String dataSetCode;
+ if (secondObject instanceof String)
+ {
+ dataSetCode = (String) secondObject;
+ } else if (secondObject instanceof DataSetFileDTO)
+ {
+ DataSetFileDTO dsFile = (DataSetFileDTO) secondObject;
+ dataSetCode = dsFile.getDataSetCode();
+ } else
+ {
+ String methodDesc = MethodUtils.describeMethod(methodInvocation.getMethod());
+ authorizationLog.info(String.format("Authorization failure while "
+ + "invoking method '%s': %s", methodDesc));
+
+ throw new IllegalArgumentException("Method (" + methodDesc
+ + ") does not include the data set code as an argument");
+ }
+
+ return dataSetCode;
+ }
}
}
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/client/api/v1/impl/DssComponentTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/client/api/v1/impl/DssComponentTest.java
index c6d1f33bcee..fa350052e22 100644
--- a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/client/api/v1/impl/DssComponentTest.java
+++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/client/api/v1/impl/DssComponentTest.java
@@ -27,29 +27,47 @@ import java.util.Random;
import org.apache.commons.io.IOUtils;
import org.jmock.Expectations;
import org.jmock.Mockery;
+import org.springframework.aop.Advisor;
+import org.springframework.aop.TargetSource;
+import org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator;
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.config.BeanPostProcessor;
+import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
+import ch.systemsx.cisd.base.exceptions.IOExceptionUnchecked;
import ch.systemsx.cisd.base.tests.AbstractFileSystemTestCase;
+import ch.systemsx.cisd.common.api.IRpcService;
import ch.systemsx.cisd.common.api.IRpcServiceFactory;
import ch.systemsx.cisd.common.api.RpcServiceInterfaceDTO;
import ch.systemsx.cisd.common.api.RpcServiceInterfaceVersionDTO;
+import ch.systemsx.cisd.common.exceptions.AuthorizationFailureException;
import ch.systemsx.cisd.openbis.dss.client.api.v1.IDataSetDss;
import ch.systemsx.cisd.openbis.dss.client.api.v1.impl.DssComponent;
+import ch.systemsx.cisd.openbis.dss.generic.server.AbstractDssServiceRpc;
+import ch.systemsx.cisd.openbis.dss.generic.server.DssServiceRpcAuthorizationAdvisor;
+import ch.systemsx.cisd.openbis.dss.generic.shared.IEncapsulatedOpenBISService;
+import ch.systemsx.cisd.openbis.dss.generic.shared.api.v1.DataSetFileDTO;
import ch.systemsx.cisd.openbis.dss.generic.shared.api.v1.DataStoreApiUrlUtilities;
import ch.systemsx.cisd.openbis.dss.generic.shared.api.v1.FileInfoDssBuilder;
import ch.systemsx.cisd.openbis.dss.generic.shared.api.v1.FileInfoDssDTO;
import ch.systemsx.cisd.openbis.dss.generic.shared.api.v1.IDssServiceRpcGeneric;
+import ch.systemsx.cisd.openbis.dss.generic.shared.api.v1.NewDataSetDTO;
import ch.systemsx.cisd.openbis.generic.shared.IETLLIMSService;
import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataStore;
import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ExternalData;
import ch.systemsx.cisd.openbis.generic.shared.dto.SessionContextDTO;
/**
+ * A test of the DSS component.
+ *
* @author Chandrasekhar Ramakrishnan
*/
public class DssComponentTest extends AbstractFileSystemTestCase
{
+ private static final String DUMMY_DATA_SET_CODE = "DummyDataSetCode";
+
private Mockery context;
private IETLLIMSService openBisService;
@@ -65,6 +83,34 @@ public class DssComponentTest extends AbstractFileSystemTestCase
private static final String DUMMY_DSS_URL =
DataStoreApiUrlUtilities.getDataStoreUrlFromServerUrl("http://localhost/");
+ private IDssServiceRpcGeneric dssService;
+
+ @SuppressWarnings("unchecked")
+ public static <T extends IRpcService> T getAdvisedDssService(final T service)
+ {
+ final Advisor advisor = new DssServiceRpcAuthorizationAdvisor();
+ final BeanPostProcessor processor = new AbstractAutoProxyCreator()
+ {
+ private static final long serialVersionUID = 1L;
+
+ //
+ // AbstractAutoProxyCreator
+ //
+ @Override
+ protected final Object[] getAdvicesAndAdvisorsForBean(final Class beanClass,
+ final String beanName, final TargetSource customTargetSource)
+ throws BeansException
+ {
+ return new Object[]
+ { advisor };
+ }
+ };
+ final Object proxy =
+ processor.postProcessAfterInitialization(service, "proxy of "
+ + service.getClass().getName());
+ return (T) proxy;
+ }
+
public DssComponentTest()
{
@@ -82,6 +128,13 @@ public class DssComponentTest extends AbstractFileSystemTestCase
randomDataFile = getFileWithRandomData(1);
}
+ @AfterMethod
+ public void tearDown()
+ {
+ // Clear the dss service
+ dssService = null;
+ }
+
@Test
public void testLogin()
{
@@ -106,7 +159,7 @@ public class DssComponentTest extends AbstractFileSystemTestCase
setupExpectations();
dssComponent.login("foo", "bar");
- IDataSetDss dataSetProxy = dssComponent.getDataSet("DummyDataSetCode");
+ IDataSetDss dataSetProxy = dssComponent.getDataSet(DUMMY_DATA_SET_CODE);
FileInfoDssDTO[] fileInfos = dataSetProxy.listFiles("/", true);
assertEquals(1, fileInfos.length);
@@ -118,22 +171,41 @@ public class DssComponentTest extends AbstractFileSystemTestCase
{
dssComponent = new DssComponent(openBisService, dssServiceFactory, DUMMY_SESSSION_TOKEN);
setupExpectationsNoLogin();
- IDataSetDss dataSetProxy = dssComponent.getDataSet("DummyDataSetCode");
+ IDataSetDss dataSetProxy = dssComponent.getDataSet(DUMMY_DATA_SET_CODE);
FileInfoDssDTO[] fileInfos = dataSetProxy.listFiles("/", true);
assertEquals(1, fileInfos.length);
context.assertIsSatisfied();
}
+ @Test
+ public void testListDataSetFilesUnauthorized() throws IOException
+ {
+ setupExpectations(false);
+ dssComponent.login("foo", "bar");
+ try
+ {
+ IDataSetDss dataSetProxy = dssComponent.getDataSet(DUMMY_DATA_SET_CODE);
+ dataSetProxy.listFiles("/", true);
+ fail("Unauthorized access to data set should have thrown an exception.");
+ } catch (AuthorizationFailureException ex)
+ {
+ assertEquals("Authorization failure: User does not have access to data set "
+ + DUMMY_DATA_SET_CODE + ".", ex.getMessage());
+ }
+
+ context.assertIsSatisfied();
+ }
+
@Test
public void testUnsupportedInterface() throws IOException
{
- setupExpectations("Some Server Interface", true);
+ setupExpectations("Some Server Interface", true, true);
dssComponent.login("foo", "bar");
try
{
- dssComponent.getDataSet("DummyDataSetCode");
+ dssComponent.getDataSet(DUMMY_DATA_SET_CODE);
fail("Should have thrown an IllegalArgumentException");
} catch (IllegalArgumentException e)
{
@@ -149,7 +221,7 @@ public class DssComponentTest extends AbstractFileSystemTestCase
setupExpectations();
dssComponent.login("foo", "bar");
- IDataSetDss dataSetProxy = dssComponent.getDataSet("DummyDataSetCode");
+ IDataSetDss dataSetProxy = dssComponent.getDataSet(DUMMY_DATA_SET_CODE);
FileInfoDssDTO[] fileInfos = dataSetProxy.listFiles("/", true);
FileInfoDssDTO fileFileInfo = null;
for (FileInfoDssDTO fid : fileInfos)
@@ -178,22 +250,28 @@ public class DssComponentTest extends AbstractFileSystemTestCase
private void setupExpectations() throws IOException
{
- setupExpectations(IDssServiceRpcGeneric.DSS_SERVICE_NAME, true);
+ setupExpectations(true);
+ }
+
+ private void setupExpectations(boolean isDataSetAccessible) throws IOException
+ {
+ setupExpectations(IDssServiceRpcGeneric.DSS_SERVICE_NAME, true, isDataSetAccessible);
}
private void setupExpectationsNoLogin() throws IOException
{
- setupExpectations(IDssServiceRpcGeneric.DSS_SERVICE_NAME, false);
+ setupExpectations(IDssServiceRpcGeneric.DSS_SERVICE_NAME, false, true);
}
- private void setupExpectations(String serviceName, final boolean needsLogin) throws IOException
+ private void setupExpectations(String serviceName, final boolean needsLogin,
+ boolean isDataSetAccessible) throws IOException
{
final SessionContextDTO session = getDummySession();
final ExternalData dataSetExternalData = new ExternalData();
DataStore dataStore = new DataStore();
dataStore.setDownloadUrl(DUMMY_DSS_URL);
dataSetExternalData.setDataStore(dataStore);
- final IDssServiceRpcGeneric dssService = context.mock(IDssServiceRpcGeneric.class);
+
ArrayList<FileInfoDssDTO> list = new ArrayList<FileInfoDssDTO>();
FileInfoDssBuilder builder =
new FileInfoDssBuilder(workingDirectory.getCanonicalPath(), workingDirectory
@@ -202,6 +280,10 @@ public class DssComponentTest extends AbstractFileSystemTestCase
final FileInfoDssDTO[] fileInfos = new FileInfoDssDTO[list.size()];
list.toArray(fileInfos);
+ dssService =
+ getAdvisedDssService(new MockDssServiceRpc(null, fileInfos, new FileInputStream(
+ randomDataFile), isDataSetAccessible));
+
final ArrayList<RpcServiceInterfaceDTO> ifaces = new ArrayList<RpcServiceInterfaceDTO>(1);
final RpcServiceInterfaceDTO iface = new RpcServiceInterfaceDTO(serviceName);
final RpcServiceInterfaceVersionDTO ifaceVersion =
@@ -214,7 +296,7 @@ public class DssComponentTest extends AbstractFileSystemTestCase
context.checking(new Expectations()
{
{
- final String dataSetCode = "DummyDataSetCode";
+ final String dataSetCode = DUMMY_DATA_SET_CODE;
if (needsLogin)
{
@@ -228,12 +310,6 @@ public class DssComponentTest extends AbstractFileSystemTestCase
allowing(dssServiceFactory).getService(ifaceVersion,
IDssServiceRpcGeneric.class, DUMMY_DSS_URL, false);
will(returnValue(dssService));
- allowing(dssService).listFilesForDataSet(DUMMY_SESSSION_TOKEN, dataSetCode,
- "/", true);
- will(returnValue(fileInfos));
- allowing(dssService).getFileForDataSet(DUMMY_SESSSION_TOKEN, dataSetCode,
- "/random.txt");
- will(returnValue(new FileInputStream(randomDataFile)));
}
});
}
@@ -273,4 +349,74 @@ public class DssComponentTest extends AbstractFileSystemTestCase
return file;
}
+ private static class MockDssServiceRpc extends AbstractDssServiceRpc implements
+ IDssServiceRpcGeneric
+ {
+ private final FileInfoDssDTO[] fileInfos;
+
+ private final FileInputStream fileInputStream;
+
+ private final boolean isDataSetAccessible;
+
+ /**
+ * @param openBISService
+ */
+ public MockDssServiceRpc(IEncapsulatedOpenBISService openBISService,
+ FileInfoDssDTO[] fileInfos, FileInputStream fileInputStream,
+ boolean isDataSetAccessible)
+ {
+ super(openBISService);
+ this.fileInfos = fileInfos;
+ this.fileInputStream = fileInputStream;
+ this.isDataSetAccessible = isDataSetAccessible;
+ }
+
+ public InputStream getFileForDataSet(String sessionToken, DataSetFileDTO fileOrFolder)
+ throws IOExceptionUnchecked, IllegalArgumentException
+ {
+ return fileInputStream;
+ }
+
+ public InputStream getFileForDataSet(String sessionToken, String dataSetCode, String path)
+ throws IOExceptionUnchecked, IllegalArgumentException
+ {
+ return fileInputStream;
+ }
+
+ public FileInfoDssDTO[] listFilesForDataSet(String sessionToken, DataSetFileDTO fileOrFolder)
+ throws IOExceptionUnchecked, IllegalArgumentException
+ {
+ return fileInfos;
+ }
+
+ public FileInfoDssDTO[] listFilesForDataSet(String sessionToken, String dataSetCode,
+ String path, boolean isRecursive) throws IOExceptionUnchecked,
+ IllegalArgumentException
+ {
+ return fileInfos;
+ }
+
+ public String putDataSet(String sessionToken, NewDataSetDTO newDataset,
+ InputStream inputStream) throws IOExceptionUnchecked, IllegalArgumentException
+ {
+ return null;
+ }
+
+ public int getMajorVersion()
+ {
+ return 1;
+ }
+
+ public int getMinorVersion()
+ {
+ return 0;
+ }
+
+ @Override
+ protected boolean isDatasetAccessible(String sessionToken, String dataSetCode)
+ {
+ return isDataSetAccessible;
+ }
+ }
+
}
--
GitLab