diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/TrashBO.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/TrashBO.java index c363f25562baa512f17efe396bc0c8ce283992f1..b97d6920c1bb0c7fc8a978825ef19900691da213 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/TrashBO.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/TrashBO.java @@ -28,9 +28,19 @@ import java.util.Map.Entry; import java.util.Set; import java.util.TreeMap; +import org.apache.commons.lang.StringUtils; import org.springframework.dao.DataAccessException; +import ch.systemsx.cisd.common.collection.CollectionUtils; +import ch.systemsx.cisd.common.exceptions.AuthorizationFailureException; +import ch.systemsx.cisd.common.exceptions.Status; import ch.systemsx.cisd.common.exceptions.UserFailureException; +import ch.systemsx.cisd.openbis.generic.server.authorization.DefaultAccessController; +import ch.systemsx.cisd.openbis.generic.server.authorization.RoleWithIdentifier; +import ch.systemsx.cisd.openbis.generic.server.authorization.predicate.DataPEPredicate; +import ch.systemsx.cisd.openbis.generic.server.authorization.predicate.ExperimentPEPredicate; +import ch.systemsx.cisd.openbis.generic.server.authorization.predicate.PersistentEntityPredicate; +import ch.systemsx.cisd.openbis.generic.server.authorization.predicate.SamplePEPredicate; import ch.systemsx.cisd.openbis.generic.server.batch.BatchOperationExecutor; import ch.systemsx.cisd.openbis.generic.server.batch.IBatchOperation; import ch.systemsx.cisd.openbis.generic.server.business.IRelationshipService; @@ -44,6 +54,7 @@ import ch.systemsx.cisd.openbis.generic.server.dataaccess.ISampleDAO; import ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.DataSetFetchOption; import ch.systemsx.cisd.openbis.generic.shared.basic.IIdAndCodeHolder; import ch.systemsx.cisd.openbis.generic.shared.basic.IIdHolder; +import ch.systemsx.cisd.openbis.generic.shared.basic.IIdentifierHolder; import ch.systemsx.cisd.openbis.generic.shared.basic.TechId; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.AbstractExternalData; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataSetArchivingStatus; @@ -54,6 +65,7 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Sample; import ch.systemsx.cisd.openbis.generic.shared.dto.DataPE; import ch.systemsx.cisd.openbis.generic.shared.dto.DeletionPE; import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE; +import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE; import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE; import ch.systemsx.cisd.openbis.generic.shared.dto.Session; import ch.systemsx.cisd.openbis.generic.shared.dto.properties.EntityKind; @@ -513,6 +525,11 @@ public class TrashBO extends AbstractBusinessObject implements ITrashBO private final Map<EntityKind, Set<TechId>> entityIdsByKind = new HashMap<EntityKind, Set<TechId>>(); private final List<TrashBatchOperation> operations = new ArrayList<TrashBatchOperation>(); + private final PersonPE person; + private final List<RoleWithIdentifier> allowedRoles; + private final ExperimentPEPredicate experimentPredicate; + private final SamplePEPredicate samplesPredicate; + private final DataPEPredicate dataSetPredicate; TrashOperationsManager(Session session, DeletionPE deletion, IDAOFactory daoFactory) { @@ -524,17 +541,59 @@ public class TrashBO extends AbstractBusinessObject implements ITrashBO { entityIdsByKind.put(entityKind, new HashSet<TechId>()); } + person = session.tryGetPerson(); + allowedRoles = DefaultAccessController.getUserRoles(person); + experimentPredicate = new ExperimentPEPredicate(); + samplesPredicate = new SamplePEPredicate(); + dataSetPredicate = new DataPEPredicate(); } void addTrashOperation(EntityKind entityKind, List<TechId> entityIds, boolean isOriginalDeletion) { if (entityIds.isEmpty() == false) { + List<Long> ids = TechId.asLongs(entityIds); + if (entityKind == EntityKind.EXPERIMENT) + { + List<ExperimentPE> experiments = daoFactory.getExperimentDAO().listByIDs(ids); + assertAuthorization(experimentPredicate, experiments, "experiments"); + } else if (entityKind == EntityKind.SAMPLE) + { + List<SamplePE> samples = daoFactory.getSampleDAO().listByIDs(ids); + assertAuthorization(samplesPredicate, samples, "samples"); + } else if (entityKind == EntityKind.DATA_SET) + { + List<DataPE> dataSets = daoFactory.getDataDAO().listByIDs(ids); + assertAuthorization(dataSetPredicate, dataSets, "data sets"); + } entityIdsByKind.get(entityKind).addAll(entityIds); IDeletionDAO deletionDAO = daoFactory.getDeletionDAO(); operations.add(0, new TrashBatchOperation(entityKind, entityIds, deletion, deletionDAO, isOriginalDeletion)); } } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private void assertAuthorization(PersistentEntityPredicate entityPredicate, + List<? extends IIdentifierHolder> entities, String entityKindName) + { + List<String> identifiers = new ArrayList<String>(); + String errorMessage = ""; + for (IIdentifierHolder identifierHolder : entities) + { + Status status = entityPredicate.evaluate(person, allowedRoles, identifierHolder); + if (status.isOK() == false) + { + identifiers.add(identifierHolder.getIdentifier()); + errorMessage = status.tryGetErrorMessage(); + } + } + if (identifiers.isEmpty() == false) + { + throw new AuthorizationFailureException("Can not delete " + entityKindName + " " + + CollectionUtils.abbreviate(identifiers, 10) + " because user " + + errorMessage); + } + } void trash() { @@ -576,6 +635,7 @@ public class TrashBO extends AbstractBusinessObject implements ITrashBO RelationshipUtils.updateModificationDateAndModifierOfRelatedEntitiesOfDataSets(dataSets, session); } } + } private static class TrashBatchOperation implements IBatchOperation<TechId> diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/EntityDeletionTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/EntityDeletionTest.java index e765e9db9b0d1e9def7d990dcbaf11d2c56cc331..aa0ab22c5a087f35ba2d094e463680deef56417e 100644 --- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/EntityDeletionTest.java +++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/EntityDeletionTest.java @@ -35,7 +35,7 @@ import ch.systemsx.cisd.openbis.systemtest.AbstractEntityDeletionTestCase; /** - * Implementation of {@link AbstractEntityDeletionTestCase} based in V3 API. + * Implementation of {@link AbstractEntityDeletionTestCase} based on V3 API. * * @author Franz-Josef Elmer */ @@ -44,6 +44,7 @@ public class EntityDeletionTest extends AbstractEntityDeletionTestCase { private static final String CONTEXT_DESCRIPTION = " (Context: [])"; + @Autowired protected IApplicationServerApi v3api; @@ -59,6 +60,12 @@ public class EntityDeletionTest extends AbstractEntityDeletionTestCase return super.createExpectedErrorMessage(originalNode, relatedEntity, outsiderNode) + CONTEXT_DESCRIPTION; } + @Override + protected String createExpectedErrorMessage(String userID, String... samples) + { + return super.createExpectedErrorMessage(userID, samples) + CONTEXT_DESCRIPTION; + } + @Override protected void deleteExperiments(List<String> experimentIdentifiers, String userSessionToken) { diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/TrashBOTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/TrashBOTest.java index 89ef529b3886c6b07293198767c6a34a67086772..1f45068e04f846bc97ecfc75bfa9a3e63124226e 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/TrashBOTest.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/TrashBOTest.java @@ -49,13 +49,16 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.IIdHolder; import ch.systemsx.cisd.openbis.generic.shared.basic.TechId; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.AbstractExternalData; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ListOrSearchSampleCriteria; +import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy.RoleCode; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Sample; import ch.systemsx.cisd.openbis.generic.shared.dto.DataPE; import ch.systemsx.cisd.openbis.generic.shared.dto.DeletionPE; import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE; import ch.systemsx.cisd.openbis.generic.shared.dto.ExternalDataPE; import ch.systemsx.cisd.openbis.generic.shared.dto.RelationshipTypePE; +import ch.systemsx.cisd.openbis.generic.shared.dto.RoleAssignmentPE; import ch.systemsx.cisd.openbis.generic.shared.dto.ScriptPE; +import ch.systemsx.cisd.openbis.generic.shared.dto.Session; import ch.systemsx.cisd.openbis.generic.shared.dto.properties.EntityKind; /** @@ -85,6 +88,8 @@ public final class TrashBOTest extends AbstractBOTest private int dataSetTableSequenceId; + private Session session; + @Override @BeforeMethod public void beforeMethod() @@ -92,13 +97,15 @@ public final class TrashBOTest extends AbstractBOTest super.beforeMethod(); boFactory = context.mock(ICommonBusinessObjectFactory.class); dataSetTable = context.mock(IDataSetTable.class); - trashBO = - new TrashBO(daoFactory, boFactory, ManagerTestTool.EXAMPLE_SESSION, - managedPropertyEvaluatorFactory, null, null); + session = ManagerTestTool.createSession(); + RoleAssignmentPE roleAssignment = new RoleAssignmentPE(); + roleAssignment.setRole(RoleCode.ADMIN); + session.tryGetPerson().addRoleAssignment(roleAssignment); + trashBO = new TrashBO(daoFactory, boFactory, session, managedPropertyEvaluatorFactory, null, null); context.checking(new Expectations() { { - allowing(boFactory).createDataSetTable(ManagerTestTool.EXAMPLE_SESSION); + allowing(boFactory).createDataSetTable(session); will(returnValue(dataSetTable)); allowing(relationshipTypeDAO).tryFindRelationshipTypeByCode(BasicConstant.CONTAINER_COMPONENT_INTERNAL_RELATIONSHIP); @@ -111,10 +118,10 @@ public final class TrashBOTest extends AbstractBOTest type.setId(CHILDREN_PARENT_RELATIONSHIP_ID); will(returnValue(type)); - allowing(boFactory).createSampleLister(ManagerTestTool.EXAMPLE_SESSION); + allowing(boFactory).createSampleLister(session); will(returnValue(sampleLister)); - allowing(boFactory).createDatasetLister(ManagerTestTool.EXAMPLE_SESSION); + allowing(boFactory).createDatasetLister(session); will(returnValue(datasetLister)); } }); @@ -144,7 +151,7 @@ public final class TrashBOTest extends AbstractBOTest final String reason = EXAMPLE_REASON; DeletionPE deletionPE = createDeletion(reason); assertEquals(reason, deletionPE.getReason()); - assertEquals(ManagerTestTool.EXAMPLE_SESSION.tryGetPerson(), deletionPE.getRegistrator()); + assertEquals(session.tryGetPerson(), deletionPE.getRegistrator()); context.assertIsSatisfied(); } @@ -161,8 +168,7 @@ public final class TrashBOTest extends AbstractBOTest one(deletionDAO).getByTechId(deletionId); will(returnValue(dummyDeletion)); - one(deletionDAO).revert(dummyDeletion, - ManagerTestTool.EXAMPLE_SESSION.tryGetPerson()); + one(deletionDAO).revert(dummyDeletion, session.tryGetPerson()); } }); trashBO.revertDeletion(deletionId); diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/entitygraph/Utils.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/entitygraph/Utils.java index 09b425c71eea67bd3389a2560fb3c88ad54949a1..525abd23f8d45fc73df80f4f5785f7966802b6a9 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/entitygraph/Utils.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/entitygraph/Utils.java @@ -28,6 +28,8 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.ExternalDataPE; import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectPE; import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE; import ch.systemsx.cisd.openbis.generic.shared.dto.SpacePE; +import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifier; +import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifierFactory; /** * @@ -142,7 +144,19 @@ public class Utils } ExperimentPE experiment = new ExperimentPE(); experiment.setId(experimentNode.getId()); - experiment.setCode(experimentNode.getCode()); + String identifier = experimentNode.getIdentifier(); + if (identifier.startsWith("///")) + { + identifier = "/S0/P0/" + experimentNode.getCode(); + } + ExperimentIdentifier experimentIdentifier = ExperimentIdentifierFactory.parse(identifier); + experiment.setCode(experimentIdentifier.getExperimentCode()); + ProjectPE project = new ProjectPE(); + project.setCode(experimentIdentifier.getProjectCode()); + SpacePE space = new SpacePE(); + space.setCode(experimentIdentifier.getSpaceCode()); + project.setSpace(space); + experiment.setProject(project); return experiment; } diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/AbstractEntityDeletionTestCase.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/AbstractEntityDeletionTestCase.java index ee78c8d3e0e4c6745618084dbd236e7e8d1f8e76..1979ed90fd70018d4df47226662fb3a75f4a2654 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/AbstractEntityDeletionTestCase.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/AbstractEntityDeletionTestCase.java @@ -20,6 +20,7 @@ import static org.testng.AssertJUnit.assertEquals; import static org.testng.AssertJUnit.fail; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import org.springframework.test.annotation.Rollback; @@ -32,8 +33,10 @@ import ch.systemsx.cisd.openbis.generic.server.business.bo.entitygraph.EntityGra import ch.systemsx.cisd.openbis.generic.server.business.bo.entitygraph.EntityNode; import ch.systemsx.cisd.openbis.generic.server.business.bo.entitygraph.ExperimentNode; import ch.systemsx.cisd.openbis.generic.server.business.bo.entitygraph.SampleNode; +import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy.RoleCode; import ch.systemsx.cisd.openbis.systemtest.base.BaseTest; +import ch.systemsx.cisd.openbis.systemtest.base.builder.SessionBuilder; /** * Abstract super class of all entity deletion system tests. @@ -454,6 +457,23 @@ public abstract class AbstractEntityDeletionTestCase extends BaseTest + "DS4[NECT], components: DS5[NET] DS6[NET]\n", renderGraph(g)); assertUnmodifiedAndUndeleted(g); } + + @Test + @Rollback(true) + public void testTrashSampleWithComponentsFromAnInvisibleSpace() + { + EntityGraphGenerator g = parseAndCreateGraph("/S1/S1, components: /S2/S2\n"); + SessionBuilder sessionBuilder = aSession().withSpaceRole(RoleWithHierarchy.SPACE_ADMIN, + entityGraphManager.getSample(g.s(1)).getSpace()); + String userSessionToken = create(sessionBuilder); + + String userID = sessionBuilder.getUserID(); + String sample = entityGraphManager.getSample(g.s(2)).getIdentifier(); + failTrashSample(userSessionToken, g.s(1), createExpectedErrorMessage(userID, sample)); + + assertEquals("/S1/S1, components: /S2/S2\n", renderGraph(g)); + assertUnmodifiedAndUndeleted(g); + } @Test public final void testTrashDataSets() @@ -565,10 +585,16 @@ public abstract class AbstractEntityDeletionTestCase extends BaseTest } private void failTrashSample(SampleNode sampleNode, String expectedErrorMessage) + { + String userSessionToken = createAdminUser(); + failTrashSample(userSessionToken, sampleNode, expectedErrorMessage); + } + + private void failTrashSample(String userSessionToken, SampleNode sampleNode, String expectedErrorMessage) { try { - deleteSamples(sampleNode); + deleteSamples(userSessionToken, sampleNode); fail("UserFailureException expected"); } catch (UserFailureException ex) { @@ -589,6 +615,13 @@ public abstract class AbstractEntityDeletionTestCase extends BaseTest + " is a component of the " + render(relatedEntity) + " which belongs to the " + render(outsiderNode) + " which is outside the deletion set."; } + + protected String createExpectedErrorMessage(String userID, String... samples) + { + return "Authorization failure: Can not delete samples " + Arrays.asList(samples) + + " because user " + userID + " does not have enough privileges."; + } + private String render(EntityNode entityNode) { @@ -622,13 +655,18 @@ public abstract class AbstractEntityDeletionTestCase extends BaseTest } private void deleteSamples(SampleNode...sampleNodes) + { + deleteSamples(createAdminUser(), sampleNodes); + } + + private void deleteSamples(String userSessionToken, SampleNode... sampleNodes) { List<String> samplePermIds = new ArrayList<String>(); for (SampleNode sampleNode : sampleNodes) { samplePermIds.add(entityGraphManager.getSamplePermIdOrNull(sampleNode)); } - deleteSamples(samplePermIds, createAdminUser()); + deleteSamples(samplePermIds, userSessionToken); flushAndClearHibernateSession(); }