From aad198494e6ae3ff83562fb89adc5913adff47cf Mon Sep 17 00:00:00 2001
From: brinn <brinn>
Date: Tue, 24 Jul 2012 22:30:59 +0000
Subject: [PATCH] [BIS-137] Implement a more secure SampleListPredicate and
replace SamplePredicate with a version that uses this new code. The new code
checks all aspects of the Sample object, rather than just checking the sample
identifier.
SVN: 26183
---
.../api/v1/GeneralInformationService.java | 7 +-
.../AuthorizationServiceUtils.java | 47 +++--
.../predicate/SampleListPredicate.java | 193 ++++++++++++++++++
.../predicate/SamplePredicate.java | 25 ++-
.../remoteapitest/RemoteApiTestCase.java | 1 +
5 files changed, 247 insertions(+), 26 deletions(-)
create mode 100644 openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SampleListPredicate.java
rename openbis/source/java/ch/systemsx/cisd/openbis/generic/{shared => server}/authorization/predicate/SamplePredicate.java (51%)
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/api/v1/GeneralInformationService.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/api/v1/GeneralInformationService.java
index a4e6fab166d..5f9ce1a4b68 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/api/v1/GeneralInformationService.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/api/v1/GeneralInformationService.java
@@ -39,6 +39,8 @@ import ch.systemsx.cisd.common.exceptions.UserFailureException;
import ch.systemsx.cisd.common.spring.IInvocationLoggerContext;
import ch.systemsx.cisd.openbis.generic.server.AbstractServer;
import ch.systemsx.cisd.openbis.generic.server.ComponentNames;
+import ch.systemsx.cisd.openbis.generic.server.authorization.predicate.SampleListPredicate;
+import ch.systemsx.cisd.openbis.generic.server.authorization.predicate.SamplePredicate;
import ch.systemsx.cisd.openbis.generic.server.business.IPropertiesBatchManager;
import ch.systemsx.cisd.openbis.generic.server.business.bo.ICommonBusinessObjectFactory;
import ch.systemsx.cisd.openbis.generic.server.business.bo.fetchoptions.datasetlister.DataSetLister;
@@ -76,7 +78,6 @@ import ch.systemsx.cisd.openbis.generic.shared.authorization.annotation.RolesAll
import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.ExperimentIdentifierPredicate;
import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.ExperimentPredicate;
import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.ProjectPredicate;
-import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.SamplePredicate;
import ch.systemsx.cisd.openbis.generic.shared.authorization.validator.DataSetByExperimentIdentifierValidator;
import ch.systemsx.cisd.openbis.generic.shared.authorization.validator.ExperimentByIdentiferValidator;
import ch.systemsx.cisd.openbis.generic.shared.authorization.validator.ProjectByIdentiferValidator;
@@ -422,7 +423,7 @@ public class GeneralInformationService extends AbstractServer<IGeneralInformatio
@RolesAllowed(RoleWithHierarchy.SPACE_OBSERVER)
@ReturnValueFilter(validatorClass = DataSetByExperimentIdentifierValidator.class)
public List<DataSet> listDataSets(String sessionToken,
- @AuthorizationGuard(guardClass = SamplePredicate.class)
+ @AuthorizationGuard(guardClass = SampleListPredicate.class)
List<Sample> samples)
{
return listDataSets(sessionToken, samples, EnumSet.noneOf(Connections.class));
@@ -651,7 +652,7 @@ public class GeneralInformationService extends AbstractServer<IGeneralInformatio
@RolesAllowed(RoleWithHierarchy.SPACE_OBSERVER)
@ReturnValueFilter(validatorClass = DataSetByExperimentIdentifierValidator.class)
public List<DataSet> listDataSets(String sessionToken,
- @AuthorizationGuard(guardClass = SamplePredicate.class)
+ @AuthorizationGuard(guardClass = SampleListPredicate.class)
List<Sample> samples, EnumSet<Connections> connections)
{
checkSession(sessionToken);
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/AuthorizationServiceUtils.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/AuthorizationServiceUtils.java
index 03a828a5359..269bac9b05a 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/AuthorizationServiceUtils.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/AuthorizationServiceUtils.java
@@ -24,13 +24,16 @@ import ch.systemsx.cisd.common.exceptions.StatusFlag;
import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDAOFactory;
import ch.systemsx.cisd.openbis.generic.shared.authorization.RoleWithIdentifier;
import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.DataSetCodePredicate;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.DelegatedPredicate;
import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.ExperimentIdentifierPredicate;
-import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.SamplePredicate;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.SampleOwnerIdentifierPredicate;
import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.SpaceIdentifierPredicate;
import ch.systemsx.cisd.openbis.generic.shared.basic.IIdentifierHolder;
import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy;
import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy.RoleLevel;
import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifierFactory;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleOwnerIdentifier;
import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SpaceIdentifier;
/**
@@ -175,13 +178,15 @@ public class AuthorizationServiceUtils
public List<String> filterSampleIds(String user, List<String> sampleIds)
{
- PersonPE person = getUserByName(user);
- List<RoleWithIdentifier> userRoles = DefaultAccessController.getUserRoles(person);
+ final DelegatedPredicate<SampleOwnerIdentifier, IIdentifierHolder> predicate =
+ createSampleOwnerPredicate();
+ final PersonPE person = getUserByName(user);
+ final List<RoleWithIdentifier> userRoles = DefaultAccessController.getUserRoles(person);
- LinkedList<String> resultList = new LinkedList<String>();
+ final LinkedList<String> resultList = new LinkedList<String>();
for (String sampleIdentifier : sampleIds)
{
- if (canAccessSample(person, userRoles, sampleIdentifier))
+ if (canAccessSample(predicate, person, userRoles, sampleIdentifier))
{
resultList.add(sampleIdentifier);
}
@@ -189,14 +194,11 @@ public class AuthorizationServiceUtils
return resultList;
}
- private boolean canAccessSample(PersonPE person, List<RoleWithIdentifier> allowedRoles,
+ private boolean canAccessSample(
+ final DelegatedPredicate<SampleOwnerIdentifier, IIdentifierHolder> predicate,
+ final PersonPE person, final List<RoleWithIdentifier> allowedRoles,
final String sampleIdentifier)
{
-
- SamplePredicate predicate = new SamplePredicate();
-
- predicate.init(new AuthorizationDataProvider(daoFactory));
-
final Status status = predicate.evaluate(person, allowedRoles, new IIdentifierHolder()
{
@Override
@@ -206,7 +208,28 @@ public class AuthorizationServiceUtils
}
});
- return (status.getFlag().equals(StatusFlag.OK));
+ return Status.OK.equals(status);
}
+ private DelegatedPredicate<SampleOwnerIdentifier, IIdentifierHolder> createSampleOwnerPredicate()
+ {
+ final DelegatedPredicate<SampleOwnerIdentifier, IIdentifierHolder> predicate =
+ new DelegatedPredicate<SampleOwnerIdentifier, IIdentifierHolder>(
+ new SampleOwnerIdentifierPredicate())
+ {
+ @Override
+ public SampleOwnerIdentifier tryConvert(IIdentifierHolder value)
+ {
+ return SampleIdentifierFactory.parse(value.getIdentifier());
+ }
+
+ @Override
+ public String getCandidateDescription()
+ {
+ return "sample";
+ }
+ };
+ predicate.init(new AuthorizationDataProvider(daoFactory));
+ return predicate;
+ }
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SampleListPredicate.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SampleListPredicate.java
new file mode 100644
index 00000000000..69f0d8eeee9
--- /dev/null
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SampleListPredicate.java
@@ -0,0 +1,193 @@
+/*
+ * Copyright 2010 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.generic.server.authorization.predicate;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import net.lemnik.eodsql.BaseQuery;
+import net.lemnik.eodsql.QueryTool;
+import net.lemnik.eodsql.Select;
+
+import ch.systemsx.cisd.common.exceptions.AuthorizationFailureException;
+import ch.systemsx.cisd.common.exceptions.Status;
+import ch.systemsx.cisd.openbis.generic.server.dataaccess.db.LongArrayMapper;
+import ch.systemsx.cisd.openbis.generic.server.dataaccess.db.StringArrayMapper;
+import ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.Sample;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.IAuthorizationDataProvider;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.RoleWithIdentifier;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.annotation.ShouldFlattenCollections;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.AbstractSpacePredicate;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.SampleOwnerIdentifierPredicate;
+import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifierFactory;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleOwnerIdentifier;
+
+/**
+ * A predicate for lists of entities of {@link Sample}s. This
+ * predicate authorizes for read-only access, i.e. it will allow access to shared samples for all
+ * users.
+ * <p>
+ * <i>This is an internal class. Do not use it as a user of the API.</i>
+ *
+ * @author Bernd Rinn
+ */
+@ShouldFlattenCollections(value = false)
+public class SampleListPredicate extends AbstractSpacePredicate<List<Sample>>
+{
+ private final SampleOwnerIdentifierPredicate idOwnerPredicate;
+
+ public SampleListPredicate()
+ {
+ idOwnerPredicate = new SampleOwnerIdentifierPredicate();
+ }
+
+ //
+ // AbstractPredicate
+ //
+
+ @Override
+ public final void init(IAuthorizationDataProvider provider)
+ {
+ super.init(provider);
+ idOwnerPredicate.init(provider);
+ }
+
+ @Override
+ public String getCandidateDescription()
+ {
+ return "sample";
+ }
+
+ @Override
+ protected Status doEvaluation(PersonPE person, List<RoleWithIdentifier> allowedRoles,
+ List<Sample> samples)
+ {
+ // All fields relevant for authorization are expected to be filled:
+ // - technical id
+ // - permanent id
+ // - space code
+ // - space identifier
+ final List<Long> ids = new ArrayList<Long>(samples.size());
+ final List<String> permIds = new ArrayList<String>(samples.size());
+ for (Sample sample : samples)
+ {
+ if (sample.getId() == null)
+ {
+ throw new AuthorizationFailureException("id is undefined.");
+ }
+ ids.add(sample.getId());
+ if (sample.getPermId() == null)
+ {
+ throw new AuthorizationFailureException("permId is undefined.");
+ }
+ permIds.add(sample.getPermId());
+
+ if (sample.getSpaceCode() != null) // == null represents a shared sample
+ {
+ final Status status =
+ evaluate(person, allowedRoles, authorizationDataProvider
+ .getHomeDatabaseInstance(), sample.getSpaceCode());
+ if (Status.OK.equals(status) == false)
+ {
+ return status;
+ }
+ }
+
+ final SampleOwnerIdentifier idOwner =
+ SampleIdentifierFactory.parse(sample.getIdentifier());
+ final Status status = idOwnerPredicate.evaluate(person, allowedRoles, idOwner);
+ if (Status.OK.equals(status) == false)
+ {
+ return status;
+ }
+ }
+ for (Long spaceId : getSampleSpaceIds(ids, permIds))
+ {
+ if (spaceId == null || spaceId == 0)
+ {
+ continue; // Shared samples will return a spaceId of null (or 0 in EoDSQL).
+ }
+ final Status status =
+ evaluate(person, allowedRoles, authorizationDataProvider
+ .getHomeDatabaseInstance(), spaceId);
+ if (Status.OK.equals(status) == false)
+ {
+ return status;
+ }
+ }
+ return Status.OK;
+ }
+
+ private final static int ARRAY_SIZE_LIMIT = 999;
+
+ interface ISampleToSpaceQuery extends BaseQuery
+ {
+ @Select(sql = "select distinct space_id from samples where id = any(?{1}) "
+ + "union select distinct space_id from samples where perm_id = any(?{2})", parameterBindings =
+ { LongArrayMapper.class, StringArrayMapper.class })
+ public List<Long> getSampleSpaceIds(long[] sampleIds, String[] samplePermIds);
+ }
+
+ private Collection<Long> getSampleSpaceIds(final List<Long> ids, final List<String> permIds)
+ {
+ if (ids.size() != permIds.size())
+ {
+ throw new IllegalArgumentException("Expect to get the same number of ids and permIds.");
+ }
+ final int size = ids.size();
+ if (size == 0)
+ {
+ return Collections.emptyList();
+ }
+ final ISampleToSpaceQuery query =
+ QueryTool.getQuery(authorizationDataProvider.getConnection(),
+ ISampleToSpaceQuery.class);
+ if (size > ARRAY_SIZE_LIMIT)
+ {
+ final Set<Long> spaceIds = new HashSet<Long>(size);
+ for (int startIdx = 0; startIdx < size; startIdx += ARRAY_SIZE_LIMIT)
+ {
+ final List<Long> idSubList = ids.subList(startIdx,
+ Math.min(size, startIdx + ARRAY_SIZE_LIMIT));
+ final List<String> permIdSubList = permIds.subList(startIdx,
+ Math.min(size, startIdx + ARRAY_SIZE_LIMIT));
+ spaceIds.addAll(query.getSampleSpaceIds(toArray(idSubList),
+ permIdSubList.toArray(new String[permIdSubList.size()])));
+ }
+ return spaceIds;
+ } else
+ {
+ return query.getSampleSpaceIds(toArray(ids), permIds.toArray(new String[size]));
+ }
+ }
+
+ private long[] toArray(List<Long> list)
+ {
+ final long[] result = new long[list.size()];
+ for (int i = 0; i < result.length; ++i)
+ {
+ result[i] = list.get(i);
+ }
+ return result;
+ }
+
+}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/predicate/SamplePredicate.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SamplePredicate.java
similarity index 51%
rename from openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/predicate/SamplePredicate.java
rename to openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SamplePredicate.java
index 4f750a63768..b983bf8e80d 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/authorization/predicate/SamplePredicate.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SamplePredicate.java
@@ -14,29 +14,32 @@
* limitations under the License.
*/
-package ch.systemsx.cisd.openbis.generic.shared.authorization.predicate;
+package ch.systemsx.cisd.openbis.generic.server.authorization.predicate;
-import ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.Experiment;
-import ch.systemsx.cisd.openbis.generic.shared.basic.IIdentifierHolder;
-import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifierFactory;
-import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleOwnerIdentifier;
+import java.util.Collections;
+import java.util.List;
+
+import ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.Sample;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.DelegatedPredicate;
/**
- * Predicate based on {@link Experiment}.
+ * Predicate based on {@link ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.Sample}. This
+ * predicate authorizes for read-only access, i.e. it will allow access to shared samples for all
+ * users.
*
- * @author Franz-Josef Elmer
+ * @author Bernd Rinn
*/
-public class SamplePredicate extends DelegatedPredicate<SampleOwnerIdentifier, IIdentifierHolder>
+public class SamplePredicate extends DelegatedPredicate<List<Sample>, Sample>
{
public SamplePredicate()
{
- super(new SampleOwnerIdentifierPredicate());
+ super(new SampleListPredicate());
}
@Override
- public SampleOwnerIdentifier tryConvert(IIdentifierHolder value)
+ public List<Sample> tryConvert(Sample value)
{
- return SampleIdentifierFactory.parse(value.getIdentifier());
+ return Collections.singletonList(value);
}
@Override
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/remoteapitest/RemoteApiTestCase.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/remoteapitest/RemoteApiTestCase.java
index f78e77f67fc..99baab190a4 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/remoteapitest/RemoteApiTestCase.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/remoteapitest/RemoteApiTestCase.java
@@ -53,6 +53,7 @@ public class RemoteApiTestCase extends AbstractTransactionalTestNGSpringContextT
server = new Server();
Connector connector = new SelectChannelConnector();
connector.setPort(8888);
+ connector.setMaxIdleTime(360000);
server.addConnector(connector);
DispatcherServlet dispatcherServlet = new DispatcherServlet()
{
--
GitLab