From db029155f59313e8a5b05d0bad84944a0d0445bb Mon Sep 17 00:00:00 2001 From: felmer <felmer> Date: Mon, 28 Feb 2011 13:23:19 +0000 Subject: [PATCH] LMS-2077 locking data sets for IDssServiceRpcGeneric and IDssServiceRpcScreening SVN: 20144 --- .../DssServiceRpcAuthorizationAdvisor.java | 102 ++++++++++++------ .../api/internal/IDataSetPredicate.java | 36 +++++++ .../AbstractDataSetAccessPredicate.java | 39 +++++++ .../authorization/DataSetAccessGuard.java | 7 +- .../DataSetCodeStringPredicate.java | 14 +-- .../DataSetFileDTOPredicate.java | 15 ++- .../IAuthorizationGuardPredicate.java | 3 +- .../authorization/NewDataSetPredicate.java | 9 ++ .../shared/api/v1/IDssServiceRpcGeneric.java | 2 +- .../client/api/v1/impl/DssComponentTest.java | 44 ++++---- .../generic/server/DssServiceRpcV1Test.java | 30 +++++- .../api/v1/DssServiceRpcGenericTest.java | 21 +++- .../internal/DatasetIdentifierPredicate.java | 18 +--- .../SingleDataSetIdentifierPredicate.java | 20 ++-- .../server/DssServiceRpcScreeningTest.java | 33 +++++- 15 files changed, 287 insertions(+), 106 deletions(-) create mode 100644 datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/IDataSetPredicate.java create mode 100644 datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/AbstractDataSetAccessPredicate.java 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 2bebc529497..2938718720d 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 @@ -77,9 +77,17 @@ public class DssServiceRpcAuthorizationAdvisor extends DefaultPointcutAdvisor */ public DssServiceRpcAuthorizationAdvisor() { - this(new DssServiceRpcAuthorizationMethodInterceptor()); + this(new DssServiceRpcAuthorizationMethodInterceptor(null)); } + /** + * Constructor for testing purposes. + */ + public DssServiceRpcAuthorizationAdvisor(IShareIdManager shareIdManager) + { + this(new DssServiceRpcAuthorizationMethodInterceptor(shareIdManager)); + } + /** * Constructor for testing purposes. * @@ -99,6 +107,11 @@ public class DssServiceRpcAuthorizationAdvisor extends DefaultPointcutAdvisor { private IShareIdManager shareIdManager; + public DssServiceRpcAuthorizationMethodInterceptor(IShareIdManager shareIdManager) + { + this.shareIdManager = shareIdManager; + } + /** * Get the session token and any guarded parameters and invoke the guards on those * parameters. @@ -110,43 +123,56 @@ public class DssServiceRpcAuthorizationAdvisor extends DefaultPointcutAdvisor List<Parameter<AuthorizationGuard>> annotatedParameters = AnnotationUtils.getAnnotatedParameters(methodInvocation.getMethod(), AuthorizationGuard.class); + IShareIdManager manager = getShareIdManager(); for (Parameter<AuthorizationGuard> param : annotatedParameters) { + Object guarded = args[param.getIndex()]; + List<String> dataSetCodes = getDataSetCodes(param, guarded); + for (String dataSetCode : dataSetCodes) + { + manager.lock(dataSetCode); + } } - final boolean requiresInstanceAdmin = - checkRequiresInstanceAdmin(methodInvocation.getMethod(), sessionToken); - // At least one of the parameters must be annotated - assert requiresInstanceAdmin || annotatedParameters.size() > 0 : "No guard defined"; - - if (requiresInstanceAdmin) - { - // An instance admin is allowed to work on all data sets. - return methodInvocation.proceed(); - } - - final Object recv = methodInvocation.getThis(); - - for (Parameter<AuthorizationGuard> param : annotatedParameters) + try { - Object guarded = args[param.getIndex()]; - Status status = evaluateGuard(sessionToken, recv, param, guarded); - if (status != Status.OK) + final boolean requiresInstanceAdmin = + checkRequiresInstanceAdmin(methodInvocation.getMethod(), sessionToken); + // At least one of the parameters must be annotated + assert requiresInstanceAdmin || annotatedParameters.size() > 0 : "No guard defined"; + + if (requiresInstanceAdmin) { - authorizationLog.info(String.format( - "[SESSION:'%s' DATA_SET:%s]: Authorization failure while " - + "invoking method '%s'", sessionToken, guarded, - MethodUtils.describeMethod(methodInvocation.getMethod()))); - String errorMessage = "Data set does not exist."; - if (null != status.tryGetErrorMessage()) + // An instance admin is allowed to work on all data sets. + return methodInvocation.proceed(); + } + + final Object recv = methodInvocation.getThis(); + + for (Parameter<AuthorizationGuard> param : annotatedParameters) + { + Object guarded = args[param.getIndex()]; + Status status = evaluateGuard(sessionToken, recv, param, guarded); + if (status != Status.OK) { - errorMessage = status.tryGetErrorMessage(); + authorizationLog.info(String.format( + "[SESSION:'%s' DATA_SET:%s]: Authorization failure while " + + "invoking method '%s'", sessionToken, guarded, + MethodUtils.describeMethod(methodInvocation.getMethod()))); + String errorMessage = "Data set does not exist."; + if (null != status.tryGetErrorMessage()) + { + errorMessage = status.tryGetErrorMessage(); + } + + throw new AuthorizationFailureException(errorMessage); } - - throw new AuthorizationFailureException(errorMessage); } + + return methodInvocation.proceed(); + } finally + { + manager.releaseLocks(); } - - return methodInvocation.proceed(); } private boolean checkRequiresInstanceAdmin(final Method method, final String sessionToken) @@ -188,8 +214,20 @@ public class DssServiceRpcAuthorizationAdvisor extends DefaultPointcutAdvisor */ @SuppressWarnings( { "rawtypes", "unchecked" }) - private Status evaluateGuard(String sessionToken, Object recv, - Parameter<AuthorizationGuard> param, Object guarded) + private List<String> getDataSetCodes(Parameter<AuthorizationGuard> param, Object guarded) + { + IAuthorizationGuardPredicate predicate = createPredicate(param); + return predicate.getDataSetCodes(guarded); + } + + /** + * Because the predicate is being invoked in a context in which its types are not known, + * there is no way to do this in a statically type-safe way. + */ + @SuppressWarnings( + { "rawtypes", "unchecked" }) + private Status evaluateGuard(String sessionToken, Object recv, + Parameter<AuthorizationGuard> param, Object guarded) { IAuthorizationGuardPredicate predicate = createPredicate(param); return predicate.evaluate(recv, sessionToken, guarded); @@ -218,7 +256,7 @@ public class DssServiceRpcAuthorizationAdvisor extends DefaultPointcutAdvisor private IShareIdManager getShareIdManager() { - if (shareIdManager != null) + if (shareIdManager == null) { shareIdManager = ServiceProvider.getShareIdManager(); } diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/IDataSetPredicate.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/IDataSetPredicate.java new file mode 100644 index 00000000000..bd56744a7fb --- /dev/null +++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/IDataSetPredicate.java @@ -0,0 +1,36 @@ +/* + * Copyright 2011 ETH Zuerich, CISD + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package ch.systemsx.cisd.openbis.dss.generic.shared.api.internal; + +import java.util.List; + +/** + * Interface implemented by classes which are able to extract data set codes from a + * method argument of type <code>A</code>. + * <p> + * <i>This is an internal class. Do not use it as a user of the API.</i> + * + * @author Franz-Josef Elmer + */ +public interface IDataSetPredicate<A> +{ + /** + * Extracts from the specified argument data set codes. Returns an empty list if there is no + * data set code to extract. + */ + public List<String> getDataSetCodes(A argument); +} diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/AbstractDataSetAccessPredicate.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/AbstractDataSetAccessPredicate.java new file mode 100644 index 00000000000..400bad2c02b --- /dev/null +++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/AbstractDataSetAccessPredicate.java @@ -0,0 +1,39 @@ +/* + * Copyright 2011 ETH Zuerich, CISD + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package ch.systemsx.cisd.openbis.dss.generic.shared.api.internal.authorization; + +import java.util.List; + +import ch.systemsx.cisd.common.exceptions.Status; +import ch.systemsx.cisd.common.exceptions.UserFailureException; + +/** + * Abstract super class of all implementations checking data set access. + * + * @author Franz-Josef Elmer + */ +public abstract class AbstractDataSetAccessPredicate<T, D> implements IAuthorizationGuardPredicate<T, D> +{ + + public Status evaluate(T receiver, String sessionToken, D argument) throws UserFailureException + { + List<String> dataSetCodes = getDataSetCodes(argument); + return DssSessionAuthorizationHolder.getAuthorizer().checkDatasetAccess(sessionToken, + dataSetCodes); + } + +} diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/DataSetAccessGuard.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/DataSetAccessGuard.java index aaba434856d..4ab636eb4da 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/DataSetAccessGuard.java +++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/DataSetAccessGuard.java @@ -24,7 +24,7 @@ import java.lang.annotation.Target; /** * Annotation for service methods to automagically check that the user invoking the method has - * access to the data set. + * access to the data set. It also locks data sets specified by method arguments. * <p> * <i>This is an internal class. Do not use it as a user of the API.</i> * @@ -39,4 +39,9 @@ public @interface DataSetAccessGuard * If calling this method requires instance admin privileges. */ boolean requiresInstanceAdmin() default false; + + /** + * By default locks on data sets are released after method invocation. + */ + boolean releaseDataSetLocks() default true; } diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/DataSetCodeStringPredicate.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/DataSetCodeStringPredicate.java index cc791785ef7..bf0faebb830 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/DataSetCodeStringPredicate.java +++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/DataSetCodeStringPredicate.java @@ -16,8 +16,8 @@ package ch.systemsx.cisd.openbis.dss.generic.shared.api.internal.authorization; -import ch.systemsx.cisd.common.exceptions.Status; -import ch.systemsx.cisd.common.exceptions.UserFailureException; +import java.util.Arrays; +import java.util.List; /** * Predicate for checking that the current user has access to a data set specified by code. @@ -26,14 +26,10 @@ import ch.systemsx.cisd.common.exceptions.UserFailureException; * * @author Chandrasekhar Ramakrishnan */ -public class DataSetCodeStringPredicate implements - IAuthorizationGuardPredicate<IDssServiceRpcGenericInternal, String> +public class DataSetCodeStringPredicate extends AbstractDataSetAccessPredicate<IDssServiceRpcGenericInternal, String> { - public Status evaluate(IDssServiceRpcGenericInternal receiver, String sessionToken, - String datasetCode) throws UserFailureException + public List<String> getDataSetCodes(String argument) { - return DssSessionAuthorizationHolder.getAuthorizer().checkDatasetAccess(sessionToken, - datasetCode); + return Arrays.asList(argument); } - } diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/DataSetFileDTOPredicate.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/DataSetFileDTOPredicate.java index 9660f92379f..1d72ab2001d 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/DataSetFileDTOPredicate.java +++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/DataSetFileDTOPredicate.java @@ -16,8 +16,9 @@ package ch.systemsx.cisd.openbis.dss.generic.shared.api.internal.authorization; -import ch.systemsx.cisd.common.exceptions.Status; -import ch.systemsx.cisd.common.exceptions.UserFailureException; +import java.util.Arrays; +import java.util.List; + import ch.systemsx.cisd.openbis.dss.generic.shared.api.v1.DataSetFileDTO; import ch.systemsx.cisd.openbis.dss.generic.shared.api.v1.IDssServiceRpcGeneric; @@ -29,14 +30,12 @@ import ch.systemsx.cisd.openbis.dss.generic.shared.api.v1.IDssServiceRpcGeneric; * * @author Chandrasekhar Ramakrishnan */ -public class DataSetFileDTOPredicate implements - IAuthorizationGuardPredicate<IDssServiceRpcGeneric, DataSetFileDTO> +public class DataSetFileDTOPredicate extends +AbstractDataSetAccessPredicate<IDssServiceRpcGeneric, DataSetFileDTO> { - public Status evaluate(IDssServiceRpcGeneric receiver, String sessionToken, - DataSetFileDTO dataSetFile) throws UserFailureException + public List<String> getDataSetCodes(DataSetFileDTO argument) { - return DssSessionAuthorizationHolder.getAuthorizer().checkDatasetAccess(sessionToken, - dataSetFile.getDataSetCode()); + return Arrays.asList(argument.getDataSetCode()); } } diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/IAuthorizationGuardPredicate.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/IAuthorizationGuardPredicate.java index f4f54e20616..49e0955a8c9 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/IAuthorizationGuardPredicate.java +++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/IAuthorizationGuardPredicate.java @@ -18,6 +18,7 @@ package ch.systemsx.cisd.openbis.dss.generic.shared.api.internal.authorization; import ch.systemsx.cisd.common.exceptions.Status; import ch.systemsx.cisd.common.exceptions.UserFailureException; +import ch.systemsx.cisd.openbis.dss.generic.shared.api.internal.IDataSetPredicate; /** * Interface for objects that can function as guardClasses in an AuthorizationGuard. @@ -29,7 +30,7 @@ import ch.systemsx.cisd.common.exceptions.UserFailureException; * * @author Chandrasekhar Ramakrishnan */ -public interface IAuthorizationGuardPredicate<T /* Receiver */, D /* Argument */> +public interface IAuthorizationGuardPredicate<T /* Receiver */, D /* Argument */> extends IDataSetPredicate<D> { /** diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/NewDataSetPredicate.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/NewDataSetPredicate.java index 94bdd3e1626..1915978473f 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/NewDataSetPredicate.java +++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/internal/authorization/NewDataSetPredicate.java @@ -16,6 +16,9 @@ package ch.systemsx.cisd.openbis.dss.generic.shared.api.internal.authorization; +import java.util.Arrays; +import java.util.List; + import ch.systemsx.cisd.common.exceptions.Status; import ch.systemsx.cisd.common.exceptions.UserFailureException; import ch.systemsx.cisd.openbis.dss.generic.shared.api.v1.IDssServiceRpcGeneric; @@ -38,6 +41,12 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SpaceIdentifier; public class NewDataSetPredicate implements IAuthorizationGuardPredicate<IDssServiceRpcGeneric, NewDataSetDTO> { + + public List<String> getDataSetCodes(NewDataSetDTO argument) + { + return Arrays.asList(); + } + public Status evaluate(IDssServiceRpcGeneric receiver, String sessionToken, NewDataSetDTO newDataSet) throws UserFailureException { diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/v1/IDssServiceRpcGeneric.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/v1/IDssServiceRpcGeneric.java index 34be6908c0b..066b27291bb 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/v1/IDssServiceRpcGeneric.java +++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/api/v1/IDssServiceRpcGeneric.java @@ -121,7 +121,7 @@ public interface IDssServiceRpcGeneric extends IRpcService * @throws IllegalArgumentException Thrown if the dataSetCode or startPath are not valid * @since 1.1 */ - @DataSetAccessGuard + @DataSetAccessGuard(releaseDataSetLocks = false) public String getPathToDataSet(String sessionToken, @AuthorizationGuard(guardClass = DataSetCodeStringPredicate.class) String dataSetCode, String overrideStoreRootPathOrNull) throws IOExceptionUnchecked, 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 4017bd90730..e333df8afcf 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 @@ -22,6 +22,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; +import java.util.Arrays; import java.util.Random; import org.apache.commons.io.IOUtils; @@ -103,9 +104,9 @@ public class DssComponentTest extends AbstractFileSystemTestCase private IShareIdManager shareIdManager; @SuppressWarnings("unchecked") - public static <T extends IRpcService> T getAdvisedDssService(final T service) + private <T extends IRpcService> T getAdvisedDssService(final T service) { - final Advisor advisor = new DssServiceRpcAuthorizationAdvisor(); + final Advisor advisor = new DssServiceRpcAuthorizationAdvisor(shareIdManager); final BeanPostProcessor processor = new AbstractAutoProxyCreator() { private static final long serialVersionUID = 1L; @@ -180,7 +181,7 @@ public class DssComponentTest extends AbstractFileSystemTestCase @Test public void testListDataSetFiles() throws IOException { - setupExpectations(); + setupExpectations(true, 1); dssComponent.login("foo", "bar"); IDataSetDss dataSetProxy = dssComponent.getDataSet(DUMMY_DATA_SET_CODE); @@ -206,7 +207,7 @@ public class DssComponentTest extends AbstractFileSystemTestCase @Test public void testListDataSetFilesUnauthorized() throws IOException { - setupExpectations(false); + setupExpectations(false, 1); dssComponent.login("foo", "bar"); try { @@ -224,7 +225,7 @@ public class DssComponentTest extends AbstractFileSystemTestCase @Test public void testLinkToContents() throws IOException { - setupExpectations(true, false); + setupExpectations(true, false, 3); dssComponent.login("foo", "bar"); IDataSetDss dataSetProxy = dssComponent.getDataSet(DUMMY_DATA_SET_CODE); @@ -254,7 +255,7 @@ public class DssComponentTest extends AbstractFileSystemTestCase @Test public void testLinkToContentsEarlierVersion() throws IOException { - setupExpectations(null, true); + setupExpectations(null, true, 1); dssComponent.login("foo", "bar"); IDataSetDss dataSetProxy = dssComponent.getDataSet(DUMMY_DATA_SET_CODE); @@ -273,7 +274,7 @@ public class DssComponentTest extends AbstractFileSystemTestCase @Test public void testUnsupportedInterface() throws IOException { - setupExpectations("Some Server Interface", true, null, false); + setupExpectations("Some Server Interface", true, null, false, 1); dssComponent.login("foo", "bar"); try @@ -291,7 +292,7 @@ public class DssComponentTest extends AbstractFileSystemTestCase @Test public void testGetFileContents() throws IOException { - setupExpectations(); + setupExpectations(2); dssComponent.login("foo", "bar"); IDataSetDss dataSetProxy = dssComponent.getDataSet(DUMMY_DATA_SET_CODE); @@ -319,32 +320,35 @@ public class DssComponentTest extends AbstractFileSystemTestCase } assertEquals(fileFileInfo.getFileSize(), byteCount); + context.assertIsSatisfied(); } - private void setupExpectations() throws IOException + private void setupExpectations(int lockingCount) throws IOException { - setupExpectations(true); + setupExpectations(true, lockingCount); } - private void setupExpectations(Boolean isDataSetAccessible) throws IOException + private void setupExpectations(Boolean isDataSetAccessible, int lockingCount) + throws IOException { - setupExpectations(IDssServiceRpcGeneric.DSS_SERVICE_NAME, true, isDataSetAccessible, false); + setupExpectations(IDssServiceRpcGeneric.DSS_SERVICE_NAME, true, isDataSetAccessible, false, + lockingCount); } - private void setupExpectations(Boolean isDataSetAccessible, boolean returnEarlierVersion) - throws IOException + private void setupExpectations(Boolean isDataSetAccessible, boolean returnEarlierVersion, + int lockingCount) throws IOException { setupExpectations(IDssServiceRpcGeneric.DSS_SERVICE_NAME, true, isDataSetAccessible, - returnEarlierVersion); + returnEarlierVersion, lockingCount); } private void setupExpectationsNoLogin() throws IOException { - setupExpectations(IDssServiceRpcGeneric.DSS_SERVICE_NAME, false, true, false); + setupExpectations(IDssServiceRpcGeneric.DSS_SERVICE_NAME, false, true, false, 1); } private void setupExpectations(String serviceName, final boolean needsLogin, - final Boolean isDataSetAccessible, boolean returnEarlierVersion) throws IOException + final Boolean isDataSetAccessible, boolean returnEarlierVersion, final int lockingCount) throws IOException { final SessionContextDTO session = getDummySession(); @@ -380,12 +384,14 @@ public class DssComponentTest extends AbstractFileSystemTestCase context.checking(new Expectations() { { - atLeast(1).of(etlService).checkDataSetAccess(DUMMY_SESSION_TOKEN, - DUMMY_DATA_SET_CODE); + one(etlService).checkDataSetCollectionAccess(DUMMY_SESSION_TOKEN, + Arrays.asList(DUMMY_DATA_SET_CODE)); if (isDataSetAccessible == false) { will(throwException(new UserFailureException("Not allowed."))); } + exactly(lockingCount).of(shareIdManager).lock(DUMMY_DATA_SET_CODE); + exactly(lockingCount).of(shareIdManager).releaseLocks(); } }); } 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 e28a880bf4e..9056dcbd271 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 @@ -23,6 +23,7 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.io.PrintWriter; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Properties; @@ -230,6 +231,7 @@ public class DssServiceRpcV1Test extends AbstractFileSystemTestCase { // Expectations for getting allowing(openBisService).checkDataSetAccess(SESSION_TOKEN, DATA_SET_CODE); + allowing(openBisService).checkDataSetCollectionAccess(SESSION_TOKEN, Arrays.asList(DATA_SET_CODE)); allowing(openBisService).getHomeDatabaseInstance(); will(returnValue(homeDatabaseInstance)); } @@ -497,6 +499,7 @@ public class DssServiceRpcV1Test extends AbstractFileSystemTestCase @Test public void testDataSetUpload() throws IOException { + prepareLockDataSet(); setupPutExpectations(); QueueingPathRemoverService.start(); File fileToUpload = createDummyFile(workingDirectory, "to-upload.txt", 80); @@ -508,7 +511,7 @@ public class DssServiceRpcV1Test extends AbstractFileSystemTestCase new ConcatenatedContentInputStream(true, getContentForFileInfos( fileToUpload.getPath(), fileInfos)); - TestMethodInterceptor testMethodInterceptor = new TestMethodInterceptor(); + TestMethodInterceptor testMethodInterceptor = new TestMethodInterceptor(shareIdManager); IDssServiceRpcGenericInternal service = getAdvisedService(testMethodInterceptor); String result = service.putDataSet(SESSION_TOKEN, newDataSet, fileInputStream); @@ -522,6 +525,11 @@ public class DssServiceRpcV1Test extends AbstractFileSystemTestCase private static class TestMethodInterceptor extends DssServiceRpcAuthorizationMethodInterceptor implements MethodInterceptor { + public TestMethodInterceptor(IShareIdManager shareIdManager) + { + super(shareIdManager); + } + private boolean methodInvoked = false; @Override @@ -547,7 +555,8 @@ public class DssServiceRpcV1Test extends AbstractFileSystemTestCase public void testAuthorizationForStringCode() { prepareGetShareId(); - TestMethodInterceptor testMethodInterceptor = new TestMethodInterceptor(); + prepareLockDataSet(DATA_SET_CODE); + TestMethodInterceptor testMethodInterceptor = new TestMethodInterceptor(shareIdManager); IDssServiceRpcGenericInternal service = getAdvisedService(testMethodInterceptor); service.listFilesForDataSet(SESSION_TOKEN, DATA_SET_CODE, "/", false); assertTrue("Advice should have been invoked.", testMethodInterceptor.methodInvoked); @@ -557,7 +566,8 @@ public class DssServiceRpcV1Test extends AbstractFileSystemTestCase public void testAuthorizationDataSetFile() { prepareGetShareId(); - TestMethodInterceptor testMethodInterceptor = new TestMethodInterceptor(); + prepareLockDataSet(DATA_SET_CODE); + TestMethodInterceptor testMethodInterceptor = new TestMethodInterceptor(shareIdManager); IDssServiceRpcGenericInternal service = getAdvisedService(testMethodInterceptor); DataSetFileDTO dataSetFile = new DataSetFileDTO(DATA_SET_CODE, "/", false); service.listFilesForDataSet(SESSION_TOKEN, dataSetFile); @@ -640,5 +650,19 @@ public class DssServiceRpcV1Test extends AbstractFileSystemTestCase } }); } + + private void prepareLockDataSet(final String... dataSetCodes) + { + context.checking(new Expectations() + { + { + for (String dataSetCode : dataSetCodes) + { + one(shareIdManager).lock(dataSetCode); + } + one(shareIdManager).releaseLocks(); + } + }); + } } 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 8af314465cd..e93f0a52e36 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,6 +17,7 @@ package ch.systemsx.cisd.openbis.dss.generic.server.api.v1; import java.io.File; +import java.util.Arrays; import org.jmock.Expectations; import org.jmock.Mockery; @@ -72,7 +73,7 @@ public class DssServiceRpcGenericTest extends AbstractFileSystemTestCase proxyFactoryBean.setInterfaces(new Class[] {IDssServiceRpcGeneric.class}); DssServiceRpcGeneric nakedDssService = new DssServiceRpcGeneric(service, shareIdManager); proxyFactoryBean.setTarget(nakedDssService); - proxyFactoryBean.addAdvisor(new DssServiceRpcAuthorizationAdvisor()); + proxyFactoryBean.addAdvisor(new DssServiceRpcAuthorizationAdvisor(shareIdManager)); dssService = (IDssServiceRpcGeneric) proxyFactoryBean.getObject(); context.checking(new Expectations() { @@ -100,7 +101,7 @@ public class DssServiceRpcGenericTest extends AbstractFileSystemTestCase { final String dataSetCode = "ds-1"; prepareCheckDataSetAccess(dataSetCode); - prepareCheckDataSetAccess(dataSetCode); + prepareLockDataSet(dataSetCode); prepareGetShareId(dataSetCode); File location = DatasetLocationUtil.getDatasetLocationPath(store, dataSetCode, @@ -126,13 +127,25 @@ public class DssServiceRpcGenericTest extends AbstractFileSystemTestCase }); } - private void prepareCheckDataSetAccess(final String dataSetCode) + private void prepareLockDataSet(final String dataSetCode) { context.checking(new Expectations() { { - one(service).checkDataSetAccess(SESSION_TOKEN, dataSetCode); + one(shareIdManager).lock(dataSetCode); + one(shareIdManager).releaseLocks(); } }); } + + private void prepareCheckDataSetAccess(final String dataSetCode) + { + context.checking(new Expectations() + { + { + one(service).checkDataSetAccess(SESSION_TOKEN, dataSetCode); + one(service).checkDataSetCollectionAccess(SESSION_TOKEN, Arrays.asList(dataSetCode)); + } + }); + } } diff --git a/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/shared/api/authorization/internal/DatasetIdentifierPredicate.java b/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/shared/api/authorization/internal/DatasetIdentifierPredicate.java index ebbcd5d3741..c79797211b6 100644 --- a/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/shared/api/authorization/internal/DatasetIdentifierPredicate.java +++ b/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/shared/api/authorization/internal/DatasetIdentifierPredicate.java @@ -19,10 +19,7 @@ package ch.systemsx.cisd.openbis.dss.screening.shared.api.authorization.internal import java.util.ArrayList; import java.util.List; -import ch.systemsx.cisd.common.exceptions.Status; -import ch.systemsx.cisd.common.exceptions.UserFailureException; -import ch.systemsx.cisd.openbis.dss.generic.shared.api.internal.authorization.DssSessionAuthorizationHolder; -import ch.systemsx.cisd.openbis.dss.generic.shared.api.internal.authorization.IAuthorizationGuardPredicate; +import ch.systemsx.cisd.openbis.dss.generic.shared.api.internal.authorization.AbstractDataSetAccessPredicate; import ch.systemsx.cisd.openbis.dss.screening.shared.api.v1.IDssServiceRpcScreening; import ch.systemsx.cisd.openbis.plugin.screening.shared.api.v1.dto.IDatasetIdentifier; @@ -33,18 +30,11 @@ import ch.systemsx.cisd.openbis.plugin.screening.shared.api.v1.dto.IDatasetIdent * * @author Chandrasekhar Ramakrishnan */ -public class DatasetIdentifierPredicate implements - IAuthorizationGuardPredicate<IDssServiceRpcScreening, List<? extends IDatasetIdentifier>> +public class DatasetIdentifierPredicate extends + AbstractDataSetAccessPredicate<IDssServiceRpcScreening, List<? extends IDatasetIdentifier>> { - public Status evaluate(IDssServiceRpcScreening receiver, String sessionToken, - List<? extends IDatasetIdentifier> datasetIdentifiers) throws UserFailureException - { - return DssSessionAuthorizationHolder.getAuthorizer().checkDatasetAccess( - sessionToken, getDatasetCodes(datasetIdentifiers)); - } - - private List<String> getDatasetCodes(List<? extends IDatasetIdentifier> datasetIdentifiers) + public List<String> getDataSetCodes(List<? extends IDatasetIdentifier> datasetIdentifiers) { final List<String> result = new ArrayList<String>(); for (IDatasetIdentifier id : datasetIdentifiers) diff --git a/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/shared/api/authorization/internal/SingleDataSetIdentifierPredicate.java b/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/shared/api/authorization/internal/SingleDataSetIdentifierPredicate.java index 393f58851db..b7a52989607 100644 --- a/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/shared/api/authorization/internal/SingleDataSetIdentifierPredicate.java +++ b/screening/source/java/ch/systemsx/cisd/openbis/dss/screening/shared/api/authorization/internal/SingleDataSetIdentifierPredicate.java @@ -16,10 +16,10 @@ package ch.systemsx.cisd.openbis.dss.screening.shared.api.authorization.internal; -import ch.systemsx.cisd.common.exceptions.Status; -import ch.systemsx.cisd.common.exceptions.UserFailureException; -import ch.systemsx.cisd.openbis.dss.generic.shared.api.internal.authorization.DssSessionAuthorizationHolder; -import ch.systemsx.cisd.openbis.dss.generic.shared.api.internal.authorization.IAuthorizationGuardPredicate; +import java.util.Arrays; +import java.util.List; + +import ch.systemsx.cisd.openbis.dss.generic.shared.api.internal.authorization.AbstractDataSetAccessPredicate; import ch.systemsx.cisd.openbis.dss.screening.shared.api.v1.IDssServiceRpcScreening; import ch.systemsx.cisd.openbis.plugin.screening.shared.api.v1.dto.IDatasetIdentifier; @@ -30,15 +30,11 @@ import ch.systemsx.cisd.openbis.plugin.screening.shared.api.v1.dto.IDatasetIdent * * @author Franz-Josef Elmer */ -public class SingleDataSetIdentifierPredicate implements - IAuthorizationGuardPredicate<IDssServiceRpcScreening, IDatasetIdentifier> - +public class SingleDataSetIdentifierPredicate extends + AbstractDataSetAccessPredicate<IDssServiceRpcScreening, IDatasetIdentifier> { - public Status evaluate(IDssServiceRpcScreening receiver, String sessionToken, - IDatasetIdentifier datasetIdentifier) throws UserFailureException + public List<String> getDataSetCodes(IDatasetIdentifier argument) { - return DssSessionAuthorizationHolder.getAuthorizer().checkDatasetAccess( - sessionToken, datasetIdentifier.getDatasetCode()); + return Arrays.asList(argument.getDatasetCode()); } - } diff --git a/screening/sourceTest/java/ch/systemsx/cisd/openbis/dss/screening/server/DssServiceRpcScreeningTest.java b/screening/sourceTest/java/ch/systemsx/cisd/openbis/dss/screening/server/DssServiceRpcScreeningTest.java index fc45bbe8f21..253726658c1 100644 --- a/screening/sourceTest/java/ch/systemsx/cisd/openbis/dss/screening/server/DssServiceRpcScreeningTest.java +++ b/screening/sourceTest/java/ch/systemsx/cisd/openbis/dss/screening/server/DssServiceRpcScreeningTest.java @@ -176,7 +176,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit will(returnValue(imageParameters)); } }); - testMethodInterceptor = new TestMethodInterceptor(); + testMethodInterceptor = new TestMethodInterceptor(shareIdManager); DssServiceRpcScreening rawScreeningService = new DssServiceRpcScreening("targets", dao, transformerDAO, service, shareIdManager, false) @@ -225,6 +225,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit prepareGetHomeDatabaseInstance(); prepareAssetDataSetIsAccessible(ds.getPermId()); prepareGetShareId(); + prepareLockDataSet(DATASET_CODE); List<PlateImageReference> plateImageReferences = screeningService @@ -242,6 +243,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit public void testListAvailableFeatureNames() { prepareAssetDataSetsAreAccessible(); + prepareLockDataSet("ds1", "ds2"); prepareGetFeatureDefinitions(1, "f1", "f2"); prepareGetFeatureDefinitions(2, "f2", "f3"); @@ -259,6 +261,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit public void testAuthorization() { prepareAssetDataSetsAreAccessible(); + prepareLockDataSet("ds1", "ds2"); prepareGetFeatureDefinitions(1, "f1", "f2"); prepareGetFeatureDefinitions(2, "f2", "f3"); @@ -275,6 +278,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit public void testLoadFeatures() { prepareAssetDataSetsAreAccessible(); + prepareLockDataSet("ds1", "ds2"); FeatureVectorDatasetReference r1 = createFeatureVectorDatasetReference(DATASET_CODE); FeatureVectorDatasetReference r2 = createFeatureVectorDatasetReference("ds2"); prepareCreateFeatureVectorDataSet(1, "F1", "F2"); @@ -309,6 +313,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit prepareGetHomeDatabaseInstance(); prepareAssetDataSetIsAccessible(DATASET_CODE); prepareGetShareId(); + prepareLockDataSet(DATASET_CODE); context.checking(new Expectations() { { @@ -364,6 +369,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit final String channel = "dapi"; prepareAssetDataSetsAreAccessible(); prepareGetExperimentPermIDs(ds1, ds2); + prepareLockDataSet("ds1", "ds2"); context.checking(new Expectations() { { @@ -392,6 +398,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit final DatasetIdentifier ds2 = new DatasetIdentifier("ds2", "url1"); prepareAssetDataSetsAreAccessible(); prepareGetExperimentPermIDs(ds1, ds2); + prepareLockDataSet("ds1", "ds2"); context.checking(new Expectations() { { @@ -419,6 +426,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit final DatasetIdentifier ds1 = new DatasetIdentifier(DATASET_CODE, "url1"); final DatasetIdentifier ds2 = new DatasetIdentifier("ds2", "url1"); final String channel = "dapi"; + prepareLockDataSet(DATASET_CODE, "ds2"); context.checking(new Expectations() { { @@ -447,6 +455,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit final DatasetIdentifier ds1 = new DatasetIdentifier(DATASET_CODE, "url1"); final DatasetIdentifier ds2 = new DatasetIdentifier("ds2", "url1"); final String channel = "dapi"; + prepareLockDataSet(DATASET_CODE, "ds2"); context.checking(new Expectations() { { @@ -490,6 +499,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit public void testSaveImageTransformerFactoryForExperiment() { final DatasetIdentifier ds1 = new DatasetIdentifier(DATASET_CODE, "url1"); + prepareLockDataSet(DATASET_CODE); context.checking(new Expectations() { { @@ -647,7 +657,7 @@ public class DssServiceRpcScreeningTest extends AssertJUnit context.checking(new Expectations() { { - one(service).checkDataSetAccess(SESSION_TOKEN, dsCode); + one(service).checkDataSetCollectionAccess(SESSION_TOKEN, Arrays.asList(dsCode)); } }); } @@ -679,6 +689,20 @@ public class DssServiceRpcScreeningTest extends AssertJUnit }); } + private void prepareLockDataSet(final String... dataSetCodes) + { + context.checking(new Expectations() + { + { + for (String dataSetCode : dataSetCodes) + { + one(shareIdManager).lock(dataSetCode); + } + one(shareIdManager).releaseLocks(); + } + }); + } + private FeatureVectorDatasetReference createFeatureVectorDatasetReference(String dataSetCode) { return new FeatureVectorDatasetReference(dataSetCode, "", null, null, null, null, null, @@ -689,6 +713,11 @@ public class DssServiceRpcScreeningTest extends AssertJUnit private static class TestMethodInterceptor extends DssServiceRpcAuthorizationMethodInterceptor implements MethodInterceptor { + public TestMethodInterceptor(IShareIdManager shareIdManager) + { + super(shareIdManager); + } + private boolean methodInvoked = false; @Override -- GitLab