From ec0281ca44247dde99ee153e4aa44f472f3e2ce1 Mon Sep 17 00:00:00 2001
From: kaloyane <kaloyane>
Date: Tue, 1 Mar 2011 13:56:18 +0000
Subject: [PATCH] [LMS-2065] Unify DssComponent authorization checking
SVN: 20178
---
.../generic/server/AbstractDssServiceRpc.java | 46 +---------
.../server/api/v1/DssServiceRpcGeneric.java | 7 +-
.../systemtests/DssComponentTest.java | 90 +++++++++++++------
.../client/api/v1/impl/DssComponentTest.java | 22 ++---
.../generic/server/DssServiceRpcV1Test.java | 25 ------
.../api/v1/DssServiceRpcGenericTest.java | 31 +++----
6 files changed, 88 insertions(+), 133 deletions(-)
diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/AbstractDssServiceRpc.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/AbstractDssServiceRpc.java
index a677249a41d..884f3f287dd 100644
--- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/AbstractDssServiceRpc.java
+++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/AbstractDssServiceRpc.java
@@ -24,7 +24,6 @@ import java.util.Set;
import org.apache.log4j.Logger;
-import ch.systemsx.cisd.common.exceptions.UserFailureException;
import ch.systemsx.cisd.common.logging.LogCategory;
import ch.systemsx.cisd.common.logging.LogFactory;
import ch.systemsx.cisd.common.spring.AbstractServiceWithLogger;
@@ -120,31 +119,6 @@ public abstract class AbstractDssServiceRpc<T> extends AbstractServiceWithLogger
return homeDatabaseInstance;
}
- /**
- * Check with openBIS if the user with the given sessionToken is allowed to access the data set
- * specified by the dataSetCode.
- */
- protected boolean isDatasetAccessible(String sessionToken, String dataSetCode)
- {
- boolean access;
- if (operationLog.isInfoEnabled())
- {
- operationLog.info(String.format("Check access to the data set '%s' on openBIS server.",
- dataSetCode));
- }
-
- try
- {
- openBISService.checkDataSetAccess(sessionToken, dataSetCode);
- access = true;
- } catch (UserFailureException ex)
- {
- access = false;
- }
-
- return access;
- }
-
protected File getRootDirectory(String datasetCode)
{
return getRootDirectoryForDataSet(datasetCode, getShareIdManager().getShareId(datasetCode));
@@ -161,16 +135,6 @@ public abstract class AbstractDssServiceRpc<T> extends AbstractServiceWithLogger
return dataSetRootDirectory;
}
- protected File checkAccessAndGetRootDirectory(String sessionToken, String dataSetCode)
- throws IllegalArgumentException
- {
- if (isDatasetAccessible(sessionToken, dataSetCode) == false)
- {
- throw new IllegalArgumentException("Path does not exist.");
- }
- return getRootDirectory(dataSetCode);
- }
-
/**
* Return a map keyed by data set code with value root directory for that data set.
*/
@@ -192,15 +156,9 @@ public abstract class AbstractDssServiceRpc<T> extends AbstractServiceWithLogger
return rootDirectories;
}
- protected File checkAccessAndGetFile(String sessionToken, String dataSetCode, String path)
- throws IOException, IllegalArgumentException
- {
- File dataSetRootDirectory = checkAccessAndGetRootDirectory(sessionToken, dataSetCode);
- return getDatasetFile(path, dataSetRootDirectory);
- }
-
- private static File getDatasetFile(String path, File dataSetRootDirectory) throws IOException
+ protected File getDatasetFile(String dataSetCode, String path) throws IOException
{
+ File dataSetRootDirectory = getRootDirectory(dataSetCode);
String dataSetRootPath = dataSetRootDirectory.getCanonicalPath();
File requestedFile = new File(dataSetRootDirectory, path);
// Make sure the requested file is under the root of the data set
diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/api/v1/DssServiceRpcGeneric.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/api/v1/DssServiceRpcGeneric.java
index 2ab6cd42e07..09884135889 100644
--- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/api/v1/DssServiceRpcGeneric.java
+++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/api/v1/DssServiceRpcGeneric.java
@@ -83,7 +83,7 @@ public class DssServiceRpcGeneric extends AbstractDssServiceRpc<IDssServiceRpcGe
public FileInfoDssDTO[] listFilesForDataSet(String sessionToken, String dataSetCode,
String startPath, boolean isRecursive) throws IllegalArgumentException
{
- File dataSetRootDirectory = checkAccessAndGetRootDirectory(sessionToken, dataSetCode);
+ File dataSetRootDirectory = getRootDirectory(dataSetCode);
try
{
@@ -115,7 +115,7 @@ public class DssServiceRpcGeneric extends AbstractDssServiceRpc<IDssServiceRpcGe
{
try
{
- File requestedFile = checkAccessAndGetFile(sessionToken, dataSetCode, path);
+ File requestedFile = getDatasetFile(dataSetCode, path);
return new FileInputStream(requestedFile);
} catch (IOException ex)
{
@@ -175,7 +175,6 @@ public class DssServiceRpcGeneric extends AbstractDssServiceRpc<IDssServiceRpcGe
public void setDirectories(File store, File incoming)
{
// TODO Auto-generated method stub
-
}
@Override
@@ -195,7 +194,7 @@ public class DssServiceRpcGeneric extends AbstractDssServiceRpc<IDssServiceRpcGe
String overrideStoreRootPathOrNull) throws IOExceptionUnchecked,
IllegalArgumentException
{
- File rootDir = checkAccessAndGetRootDirectory(sessionToken, dataSetCode);
+ File rootDir = getRootDirectory(dataSetCode);
return convertPath(getStoreDirectory(), rootDir, overrideStoreRootPathOrNull);
}
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/DssComponentTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/DssComponentTest.java
index 0f0fc5a79be..fba43929280 100644
--- a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/DssComponentTest.java
+++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/DssComponentTest.java
@@ -30,6 +30,7 @@ import org.apache.commons.io.IOUtils;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
+import ch.systemsx.cisd.common.exceptions.AuthorizationFailureException;
import ch.systemsx.cisd.common.filesystem.FileUtilities;
import ch.systemsx.cisd.openbis.dss.client.api.v1.DssComponentFactory;
import ch.systemsx.cisd.openbis.dss.client.api.v1.IDataSetDss;
@@ -44,8 +45,6 @@ import ch.systemsx.cisd.openbis.dss.generic.shared.api.v1.NewDataSetDTO.DataSetO
import ch.systemsx.cisd.openbis.generic.shared.dto.SimpleDataSetInformationDTO;
/**
- *
- *
* @author Franz-Josef Elmer
*/
@Test(groups = "slow")
@@ -59,7 +58,11 @@ public class DssComponentTest extends SystemTestCase
return f1.getPathInDataSet().compareTo(f2.getPathInDataSet());
}
};
+
+ private static final String OPENBIS_URL = "http://localhost:8888";
+
private IDssComponent dss;
+
private File store;
@BeforeMethod
@@ -67,38 +70,25 @@ public class DssComponentTest extends SystemTestCase
{
store = new File(rootDir, "store");
store.mkdirs();
- dss = DssComponentFactory.tryCreate("test", "a", "http://localhost:8888");
+ dss = createDssComponent("test");
}
@Test
public void testPutDataSet() throws Exception
{
- DataSetOwner dataSetOwner = new DataSetOwner(DataSetOwnerType.SAMPLE, "CISD:/CISD/3VCP1");
File exampleDataSet = new File(workingDirectory, "my-data");
- exampleDataSet.mkdirs();
- FileUtilities.writeToFile(new File(exampleDataSet, "data.log"), "hello world");
- FileUtilities.writeToFile(new File(exampleDataSet, "data-set.properties"),
- "property\tvalue\nCOMMENT\thello");
- File subFolder = new File(exampleDataSet, "data");
- subFolder.mkdirs();
- FileUtilities.writeToFile(new File(subFolder, "1.data"), "1 2 3");
- FileUtilities.writeToFile(new File(subFolder, "2.data"), "4 5 6 7");
- String rootPath = exampleDataSet.getCanonicalPath();
- FileInfoDssBuilder builder = new FileInfoDssBuilder(rootPath, rootPath);
- ArrayList<FileInfoDssDTO> list = new ArrayList<FileInfoDssDTO>();
- builder.appendFileInfosForFile(exampleDataSet, list, true);
- IDataSetDss dataSet = dss.putDataSet(new NewDataSetDTO(dataSetOwner,
- exampleDataSet.getName(), list), exampleDataSet);
+ NewDataSetDTO newDataset = createNewDataSetDTO(exampleDataSet);
+ IDataSetDss dataSet = dss.putDataSet(newDataset, exampleDataSet);
checkDataSet(dataSet);
}
-
+
@Test(dependsOnMethods = "testPutDataSet")
public void testGetDataSetGetFile() throws Exception
{
String code = getCodeOfLatestDataSet().getDataSetCode();
-
+
IDataSetDss ds = dss.getDataSet(code);
-
+
assertEquals(code, ds.getCode());
checkDataSet(ds);
FileInfoDssDTO[] files = ds.listFiles("/original/my-data/data", false);
@@ -123,11 +113,12 @@ public class DssComponentTest extends SystemTestCase
SimpleDataSetInformationDTO dataSetInfo = getCodeOfLatestDataSet();
String code = dataSetInfo.getDataSetCode();
File fileIntoStore =
- new File(new File(store, ch.systemsx.cisd.openbis.dss.generic.shared.Constants.DEFAULT_SHARE_ID),
+ new File(new File(store,
+ ch.systemsx.cisd.openbis.dss.generic.shared.Constants.DEFAULT_SHARE_ID),
dataSetInfo.getDataSetLocation());
-
+
IDataSetDss ds = dss.getDataSet(code);
-
+
File link = ds.tryLinkToContents(null);
assertEquals(fileIntoStore.getAbsolutePath(), link.getAbsolutePath());
File file = ds.getLinkOrCopyOfContents(null, workingDirectory);
@@ -139,9 +130,9 @@ public class DssComponentTest extends SystemTestCase
{
SimpleDataSetInformationDTO dataSetInfo = getCodeOfLatestDataSet();
String code = dataSetInfo.getDataSetCode();
-
+
IDataSetDss ds = dss.getDataSet(code);
-
+
assertEquals(null, ds.tryLinkToContents("blabla"));
File file = ds.getLinkOrCopyOfContents("blabla", workingDirectory);
assertContent("hello world", file, "data.log");
@@ -149,12 +140,55 @@ public class DssComponentTest extends SystemTestCase
assertContent("4 5 6 7", file, "data/2.data");
}
+ @Test(expectedExceptions = AuthorizationFailureException.class)
+ public void testObserverHasNoWritePermissions() throws Exception
+ {
+ dss = createDssComponent("observer");
+ File exampleDataSet = new File(workingDirectory, "observer-data");
+ NewDataSetDTO newDataset = createNewDataSetDTO(exampleDataSet);
+ dss.putDataSet(newDataset, exampleDataSet);
+ }
+
+ @Test(dependsOnMethods = "testPutDataSet", expectedExceptions = AuthorizationFailureException.class)
+ public void testObserverHasNoReadPermissions() throws Exception
+ {
+ dss = createDssComponent("observer");
+ SimpleDataSetInformationDTO dataSetInfo = getCodeOfLatestDataSet();
+ String code = dataSetInfo.getDataSetCode();
+ IDataSetDss dataSet = dss.getDataSet(code);
+ dataSet.listFiles("/", true);
+ }
+
+ private IDssComponent createDssComponent(String userName)
+ {
+ return DssComponentFactory.tryCreate(userName, "a", OPENBIS_URL);
+ }
+
+ private NewDataSetDTO createNewDataSetDTO(File exampleDataSet) throws IOException
+ {
+ DataSetOwner dataSetOwner = new DataSetOwner(DataSetOwnerType.SAMPLE, "CISD:/CISD/3VCP1");
+ exampleDataSet.mkdirs();
+ FileUtilities.writeToFile(new File(exampleDataSet, "data.log"), "hello world");
+ FileUtilities.writeToFile(new File(exampleDataSet, "data-set.properties"),
+ "property\tvalue\nCOMMENT\thello");
+ File subFolder = new File(exampleDataSet, "data");
+ subFolder.mkdirs();
+ FileUtilities.writeToFile(new File(subFolder, "1.data"), "1 2 3");
+ FileUtilities.writeToFile(new File(subFolder, "2.data"), "4 5 6 7");
+ String rootPath = exampleDataSet.getCanonicalPath();
+ FileInfoDssBuilder builder = new FileInfoDssBuilder(rootPath, rootPath);
+ ArrayList<FileInfoDssDTO> list = new ArrayList<FileInfoDssDTO>();
+ builder.appendFileInfosForFile(exampleDataSet, list, true);
+ NewDataSetDTO newDataset = new NewDataSetDTO(dataSetOwner, exampleDataSet.getName(), list);
+ return newDataset;
+ }
+
private void assertContent(String expectedContent, File root, String path)
{
assertEquals(expectedContent,
FileUtilities.loadToString(new File(root, "original/my-data/" + path)).trim());
}
-
+
private SimpleDataSetInformationDTO getCodeOfLatestDataSet()
{
IEncapsulatedOpenBISService openBISService = ServiceProvider.getOpenBISService();
@@ -168,7 +202,7 @@ public class DssComponentTest extends SystemTestCase
});
return dataSets.get(0);
}
-
+
private void checkDataSet(IDataSetDss dataSet) throws IOException
{
assertEquals("hello world", getContent(dataSet, "data.log"));
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 3a1791b97ee..d43a5648d8e 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
@@ -402,13 +402,11 @@ public class DssComponentTest extends AbstractFileSystemTestCase
dssServiceV1_0 =
getAdvisedDssService(new MockDssServiceRpcV1_0(null, shareIdManager, fileInfos,
- new FileInputStream(randomDataFile), isDataSetAccessible == null ? true
- : isDataSetAccessible));
+ new FileInputStream(randomDataFile)));
dssServiceV1_1 =
getAdvisedDssService(new MockDssServiceRpcV1_1(null, shareIdManager, fileInfos,
- new FileInputStream(randomDataFile), isDataSetAccessible == null ? true
- : isDataSetAccessible));
+ new FileInputStream(randomDataFile)));
context.checking(new Expectations()
{
@@ -489,19 +487,16 @@ public class DssComponentTest extends AbstractFileSystemTestCase
private final FileInputStream fileInputStream;
- private final boolean isDataSetAccessible;
-
/**
* @param openBISService
*/
public MockDssServiceRpcV1_0(IEncapsulatedOpenBISService openBISService,
IShareIdManager shareIdManager, FileInfoDssDTO[] fileInfos,
- FileInputStream fileInputStream, boolean isDataSetAccessible)
+ FileInputStream fileInputStream)
{
super(openBISService, shareIdManager);
this.fileInfos = fileInfos;
this.fileInputStream = fileInputStream;
- this.isDataSetAccessible = isDataSetAccessible;
}
public InputStream getFileForDataSet(String sessionToken, DataSetFileDTO fileOrFolder)
@@ -545,12 +540,6 @@ public class DssComponentTest extends AbstractFileSystemTestCase
return 0;
}
- @Override
- protected boolean isDatasetAccessible(String sessionToken, String dataSetCode)
- {
- return isDataSetAccessible;
- }
-
public String getPathToDataSet(String sessionToken, String dataSetCode,
String overrideStoreRootPathOrNull) throws IOExceptionUnchecked,
IllegalArgumentException
@@ -571,13 +560,12 @@ public class DssComponentTest extends AbstractFileSystemTestCase
* @param openBISService
* @param fileInfos
* @param fileInputStream
- * @param isDataSetAccessible
*/
public MockDssServiceRpcV1_1(IEncapsulatedOpenBISService openBISService,
IShareIdManager shareIdManager, FileInfoDssDTO[] fileInfos,
- FileInputStream fileInputStream, boolean isDataSetAccessible)
+ FileInputStream fileInputStream)
{
- super(openBISService, shareIdManager, fileInfos, fileInputStream, isDataSetAccessible);
+ super(openBISService, shareIdManager, fileInfos, fileInputStream);
}
@Override
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/DssServiceRpcV1Test.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/DssServiceRpcV1Test.java
index 80018a18bbd..49e4e32b856 100644
--- a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/DssServiceRpcV1Test.java
+++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/DssServiceRpcV1Test.java
@@ -38,7 +38,6 @@ import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
import ch.systemsx.cisd.base.tests.AbstractFileSystemTestCase;
-import ch.systemsx.cisd.common.exceptions.UserFailureException;
import ch.systemsx.cisd.common.filesystem.QueueingPathRemoverService;
import ch.systemsx.cisd.common.io.ConcatenatedContentInputStream;
import ch.systemsx.cisd.common.io.FileBasedContent;
@@ -418,30 +417,6 @@ public class DssServiceRpcV1Test extends AbstractFileSystemTestCase
context.assertIsSatisfied();
}
- @Test
- public void testInaccessibleDataSetListing()
- {
- final String badDataSetCode = "BAD";
- context.checking(new Expectations()
- {
- {
- one(openBisService).checkDataSetAccess(SESSION_TOKEN, badDataSetCode);
- will(throwException(new UserFailureException("Data set not accessible")));
- }
- });
-
- try
- {
- rpcService.listFilesForDataSet(SESSION_TOKEN, badDataSetCode, "/", true);
- fail("IllegalArgumentException should have been thrown");
- } catch (IllegalArgumentException ex)
- {
- // correct
- }
-
- context.assertIsSatisfied();
- }
-
@Test
public void testDataSetListingWithSneakyPath()
{
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/api/v1/DssServiceRpcGenericTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/api/v1/DssServiceRpcGenericTest.java
index e93f0a52e36..fe65b4487a3 100644
--- a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/api/v1/DssServiceRpcGenericTest.java
+++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/api/v1/DssServiceRpcGenericTest.java
@@ -17,7 +17,7 @@
package ch.systemsx.cisd.openbis.dss.generic.server.api.v1;
import java.io.File;
-import java.util.Arrays;
+import java.util.Collections;
import org.jmock.Expectations;
import org.jmock.Mockery;
@@ -100,8 +100,8 @@ public class DssServiceRpcGenericTest extends AbstractFileSystemTestCase
public void testFilesForData()
{
final String dataSetCode = "ds-1";
- prepareCheckDataSetAccess(dataSetCode);
prepareLockDataSet(dataSetCode);
+ prepareAuthorizationCheck(dataSetCode);
prepareGetShareId(dataSetCode);
File location =
DatasetLocationUtil.getDatasetLocationPath(store, dataSetCode,
@@ -116,36 +116,37 @@ public class DssServiceRpcGenericTest extends AbstractFileSystemTestCase
context.assertIsSatisfied();
}
- private void prepareGetShareId(final String dataSetCode)
+ private void prepareLockDataSet(final String dataSetCode)
{
context.checking(new Expectations()
{
{
- one(shareIdManager).getShareId(dataSetCode);
- will(returnValue(Constants.DEFAULT_SHARE_ID));
+ one(shareIdManager).lock(dataSetCode);
+ one(shareIdManager).releaseLocks();
}
});
}
- private void prepareLockDataSet(final String dataSetCode)
+ private void prepareAuthorizationCheck(final String dataSetCode)
{
context.checking(new Expectations()
{
{
- one(shareIdManager).lock(dataSetCode);
- one(shareIdManager).releaseLocks();
+ one(service).checkDataSetCollectionAccess(SESSION_TOKEN,
+ Collections.singletonList(dataSetCode));
}
});
+
}
-
- private void prepareCheckDataSetAccess(final String dataSetCode)
+
+ private void prepareGetShareId(final String dataSetCode)
{
context.checking(new Expectations()
- {
{
- one(service).checkDataSetAccess(SESSION_TOKEN, dataSetCode);
- one(service).checkDataSetCollectionAccess(SESSION_TOKEN, Arrays.asList(dataSetCode));
- }
- });
+ {
+ one(shareIdManager).getShareId(dataSetCode);
+ will(returnValue(Constants.DEFAULT_SHARE_ID));
+ }
+ });
}
}
--
GitLab