LMS-1702 Annotation-based authentication for DSS Services.

SVN: 17923

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

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