From de3e99d6933ed3dc07fc70daa35dfbbd7ba2a05d Mon Sep 17 00:00:00 2001
From: pkupczyk <pkupczyk>
Date: Sun, 21 Jan 2018 17:14:02 +0000
Subject: [PATCH] SSDM-4997 : Project Authorization bugfixes: - project user
cannot create/update a project sample - deleting a project that is used in a
project role assignment of the currently logged user breaks the project grid
- project user can turn a sample attached to an experiment or a project into
a space sample even though he/she does not have access to that space - v3 api
allows to fetch/modify project role assignments even if project authorization
feature is disabled
SVN: 39119
---
...edicateWithSampleIdentifierSystemTest.java | 80 -------------------
...edicateWithSampleIdentifierSystemTest.java | 70 ----------------
.../RoleAssignmentAuthorizationExecutor.java | 24 ++++--
.../executor/sample/UpdateSampleExecutor.java | 23 ++++++
.../translator/AbstractCachingTranslator.java | 12 +++
.../RoleAssignmentTranslator.java | 59 ++++++++++----
.../generic/server/AbstractServer.java | 24 +++---
.../predicate/NewSamplePredicate.java | 13 +--
.../SampleUpdatesCollectionPredicate.java | 8 +-
.../predicate/SampleUpdatesPredicate.java | 5 --
.../generic/server/business/bo/SampleBO.java | 10 +++
.../server/business/bo/SampleTable.java | 20 ++++-
.../systemtest/asapi/v3/UpdateSampleTest.java | 27 +++++++
.../ProjectAuthorizationUser.java | 32 ++++++--
14 files changed, 201 insertions(+), 206 deletions(-)
delete mode 100644 datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/authorization/predicate/sample/SampleUpdatesCollectionPredicateWithSampleIdentifierSystemTest.java
delete mode 100644 datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/authorization/predicate/sample/SampleUpdatesPredicateWithSampleIdentifierSystemTest.java
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/authorization/predicate/sample/SampleUpdatesCollectionPredicateWithSampleIdentifierSystemTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/authorization/predicate/sample/SampleUpdatesCollectionPredicateWithSampleIdentifierSystemTest.java
deleted file mode 100644
index 180e98511a4..00000000000
--- a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/authorization/predicate/sample/SampleUpdatesCollectionPredicateWithSampleIdentifierSystemTest.java
+++ /dev/null
@@ -1,80 +0,0 @@
-/*
- * Copyright 2017 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.datastoreserver.systemtests.authorization.predicate.sample;
-
-import java.util.List;
-
-import ch.systemsx.cisd.common.exceptions.UserFailureException;
-import ch.systemsx.cisd.openbis.datastoreserver.systemtests.authorization.ProjectAuthorizationUser;
-import ch.systemsx.cisd.openbis.datastoreserver.systemtests.authorization.predicate.CommonPredicateSystemTestAssertions;
-import ch.systemsx.cisd.openbis.datastoreserver.systemtests.authorization.predicate.CommonPredicateSystemTestAssertionsDelegate;
-import ch.systemsx.cisd.openbis.generic.shared.dto.SampleUpdatesDTO;
-import ch.systemsx.cisd.openbis.systemtest.authorization.predicate.sample.SamplePredicateTestService;
-
-/**
- * @author pkupczyk
- */
-public class SampleUpdatesCollectionPredicateWithSampleIdentifierSystemTest extends SampleUpdatesPredicateWithSampleIdentifierSystemTest
-{
-
- @Override
- protected boolean isCollectionPredicate()
- {
- return true;
- }
-
- @Override
- protected void evaluateObjects(ProjectAuthorizationUser user, List<SampleUpdatesDTO> objects, Object param)
- {
- getBean(SamplePredicateTestService.class).testSampleUpdatesCollectionPredicate(user.getSessionProvider(), objects);
- }
-
- @Override
- protected CommonPredicateSystemTestAssertions<SampleUpdatesDTO> getAssertions()
- {
- return new CommonPredicateSystemTestAssertionsDelegate<SampleUpdatesDTO>(super.getAssertions())
- {
- @Override
- public void assertWithNullObject(ProjectAuthorizationUser user, Throwable t, Object param)
- {
- if (user.isDisabledProjectUser())
- {
- assertAuthorizationFailureExceptionThatNoRoles(t);
- } else if (user.isInstanceUser())
- {
- assertNoException(t);
- } else
- {
- assertException(t, UserFailureException.class, "No sample updates specified.");
- }
- }
-
- @Override
- public void assertWithNullCollection(ProjectAuthorizationUser user, Throwable t, Object param)
- {
- if (user.isDisabledProjectUser())
- {
- assertAuthorizationFailureExceptionThatNoRoles(t);
- } else
- {
- assertException(t, UserFailureException.class, "No sample updates collection specified.");
- }
- }
- };
- }
-
-}
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/authorization/predicate/sample/SampleUpdatesPredicateWithSampleIdentifierSystemTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/authorization/predicate/sample/SampleUpdatesPredicateWithSampleIdentifierSystemTest.java
deleted file mode 100644
index c65d9d414f3..00000000000
--- a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/datastoreserver/systemtests/authorization/predicate/sample/SampleUpdatesPredicateWithSampleIdentifierSystemTest.java
+++ /dev/null
@@ -1,70 +0,0 @@
-/*
- * Copyright 2017 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.datastoreserver.systemtests.authorization.predicate.sample;
-
-import ch.systemsx.cisd.openbis.datastoreserver.systemtests.authorization.ProjectAuthorizationUser;
-import ch.systemsx.cisd.openbis.datastoreserver.systemtests.authorization.common.SampleIdentifierUtil;
-import ch.systemsx.cisd.openbis.datastoreserver.systemtests.authorization.predicate.CommonPredicateSystemTestAssertions;
-import ch.systemsx.cisd.openbis.datastoreserver.systemtests.authorization.predicate.CommonPredicateSystemTestAssertionsDelegate;
-import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectPE;
-import ch.systemsx.cisd.openbis.generic.shared.dto.SampleUpdatesDTO;
-import ch.systemsx.cisd.openbis.generic.shared.dto.SpacePE;
-import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifier;
-
-/**
- * @author pkupczyk
- */
-public class SampleUpdatesPredicateWithSampleIdentifierSystemTest extends SampleUpdatesPredicateWithSampleSystemTest
-{
-
- @Override
- protected SampleUpdatesDTO createNonexistentObject(Object param)
- {
- SampleIdentifier identifier = SampleIdentifierUtil.createNonexistentObject(param);
- return new SampleUpdatesDTO(null, null, null, null, null, 0, identifier, null, null);
- }
-
- @Override
- protected SampleUpdatesDTO createObject(SpacePE spacePE, ProjectPE projectPE, Object param)
- {
- SampleIdentifier identifier = SampleIdentifierUtil.createObject(this, spacePE, projectPE, param);
- return new SampleUpdatesDTO(null, null, null, null, null, 0, identifier, null, null);
- }
-
- @Override
- protected CommonPredicateSystemTestAssertions<SampleUpdatesDTO> getAssertions()
- {
- return new CommonPredicateSystemTestAssertionsDelegate<SampleUpdatesDTO>(super.getAssertions())
- {
- @Override
- public void assertWithNonexistentObject(ProjectAuthorizationUser user, Throwable t, Object param)
- {
- if (user.isDisabledProjectUser())
- {
- assertAuthorizationFailureExceptionThatNoRoles(t);
- } else if (user.isInstanceUser())
- {
- assertNoException(t);
- } else
- {
- assertAuthorizationFailureExceptionThatNotEnoughPrivileges(t);
- }
- }
- };
- }
-
-}
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/roleassignment/RoleAssignmentAuthorizationExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/roleassignment/RoleAssignmentAuthorizationExecutor.java
index 0e2c1f9aa6d..39cdc810fca 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/roleassignment/RoleAssignmentAuthorizationExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/roleassignment/RoleAssignmentAuthorizationExecutor.java
@@ -16,27 +16,31 @@
package ch.ethz.sis.openbis.generic.server.asapi.v3.executor.roleassignment;
+import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import ch.ethz.sis.openbis.generic.server.asapi.v3.executor.IOperationContext;
+import ch.systemsx.cisd.common.exceptions.UserFailureException;
import ch.systemsx.cisd.openbis.generic.server.authorization.annotation.AuthorizationGuard;
import ch.systemsx.cisd.openbis.generic.server.authorization.annotation.Capability;
import ch.systemsx.cisd.openbis.generic.server.authorization.annotation.RolesAllowed;
import ch.systemsx.cisd.openbis.generic.server.authorization.predicate.ProjectPEPredicate;
import ch.systemsx.cisd.openbis.generic.server.authorization.predicate.SpacePEPredicate;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.IAuthorizationConfig;
import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy;
import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.SpacePE;
/**
- *
- *
* @author Franz-Josef Elmer
*/
@Component
public class RoleAssignmentAuthorizationExecutor implements IRoleAssignmentAuthorizationExecutor
{
+ @Autowired
+ private IAuthorizationConfig authorizationConfig;
+
@Override
@RolesAllowed(RoleWithHierarchy.PROJECT_ADMIN)
@Capability("GET_ROLE_ASSIGNMENT")
@@ -54,7 +58,7 @@ public class RoleAssignmentAuthorizationExecutor implements IRoleAssignmentAutho
@Override
@RolesAllowed(RoleWithHierarchy.SPACE_ADMIN)
@Capability("CREATE_SPACE_ROLE")
- public void canCreateSpaceRole(IOperationContext context,
+ public void canCreateSpaceRole(IOperationContext context,
@AuthorizationGuard(guardClass = SpacePEPredicate.class) SpacePE space)
{
}
@@ -62,9 +66,13 @@ public class RoleAssignmentAuthorizationExecutor implements IRoleAssignmentAutho
@Override
@RolesAllowed(RoleWithHierarchy.PROJECT_ADMIN)
@Capability("CREATE_PROJECT_ROLE")
- public void canCreateProjectRole(IOperationContext context,
+ public void canCreateProjectRole(IOperationContext context,
@AuthorizationGuard(guardClass = ProjectPEPredicate.class) ProjectPE project)
{
+ if (false == authorizationConfig.isProjectLevelEnabled())
+ {
+ throw new UserFailureException("Project authorization is disabled. Project level roles cannot be manipulated.");
+ }
}
@Override
@@ -84,7 +92,7 @@ public class RoleAssignmentAuthorizationExecutor implements IRoleAssignmentAutho
@Override
@RolesAllowed(RoleWithHierarchy.SPACE_ADMIN)
@Capability("DELETE_SPACE_ROLE")
- public void canDeleteSpaceRole(IOperationContext context,
+ public void canDeleteSpaceRole(IOperationContext context,
@AuthorizationGuard(guardClass = SpacePEPredicate.class) SpacePE space)
{
}
@@ -92,9 +100,13 @@ public class RoleAssignmentAuthorizationExecutor implements IRoleAssignmentAutho
@Override
@RolesAllowed(RoleWithHierarchy.PROJECT_ADMIN)
@Capability("DELETE_PROJECT_ROLE")
- public void canDeleteProjectRole(IOperationContext context,
+ public void canDeleteProjectRole(IOperationContext context,
@AuthorizationGuard(guardClass = ProjectPEPredicate.class) ProjectPE project)
{
+ if (false == authorizationConfig.isProjectLevelEnabled())
+ {
+ throw new UserFailureException("Project authorization is disabled. Project level roles cannot be manipulated.");
+ }
}
}
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/sample/UpdateSampleExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/sample/UpdateSampleExecutor.java
index 8e82dd36d76..7a0a071c884 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/sample/UpdateSampleExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/sample/UpdateSampleExecutor.java
@@ -16,6 +16,7 @@
package ch.ethz.sis.openbis.generic.server.asapi.v3.executor.sample;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Date;
import java.util.List;
@@ -36,6 +37,7 @@ import ch.ethz.sis.openbis.generic.server.asapi.v3.helper.common.batch.MapBatch;
import ch.ethz.sis.openbis.generic.server.asapi.v3.helper.common.batch.MapBatchProcessor;
import ch.ethz.sis.openbis.generic.server.asapi.v3.helper.entity.progress.UpdateRelationProgress;
import ch.systemsx.cisd.common.exceptions.UserFailureException;
+import ch.systemsx.cisd.openbis.generic.server.business.IRelationshipService;
import ch.systemsx.cisd.openbis.generic.server.business.bo.DataAccessExceptionTranslator;
import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDAOFactory;
import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
@@ -81,6 +83,9 @@ public class UpdateSampleExecutor extends AbstractUpdateEntityExecutor<SampleUpd
@Autowired
private IUpdateSampleAttachmentExecutor updateSampleAttachmentExecutor;
+ @Autowired
+ protected IRelationshipService relationshipService;
+
@Override
protected ISampleId getId(SampleUpdate update)
{
@@ -111,6 +116,16 @@ public class UpdateSampleExecutor extends AbstractUpdateEntityExecutor<SampleUpd
@Override
protected void updateBatch(IOperationContext context, MapBatch<SampleUpdate, SamplePE> batch)
{
+ Collection<SamplePE> experimentOrProjectSamples = new ArrayList<SamplePE>();
+
+ for (SamplePE entity : batch.getObjects().values())
+ {
+ if (entity.getExperiment() != null || entity.getProject() != null)
+ {
+ experimentOrProjectSamples.add(entity);
+ }
+ }
+
updateSampleSpaceExecutor.update(context, batch);
updateSampleProjectExecutor.update(context, batch);
updateSampleExperimentExecutor.update(context, batch);
@@ -118,6 +133,14 @@ public class UpdateSampleExecutor extends AbstractUpdateEntityExecutor<SampleUpd
updateTags(context, batch);
updateAttachments(context, batch);
+ for (SamplePE entity : experimentOrProjectSamples)
+ {
+ if (entity.getExperiment() == null && entity.getProject() == null)
+ {
+ relationshipService.assignSampleToSpace(context.getSession(), entity, entity.getSpace());
+ }
+ }
+
PersonPE person = context.getSession().tryGetPerson();
Date timeStamp = daoFactory.getTransactionTimestamp();
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/translator/AbstractCachingTranslator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/translator/AbstractCachingTranslator.java
index cc67d6fd3f2..b75aefb8d65 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/translator/AbstractCachingTranslator.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/translator/AbstractCachingTranslator.java
@@ -87,6 +87,8 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>>
}
}
+ filterTranslated(context, translated);
+
return translated;
}
@@ -264,6 +266,16 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>>
return true;
}
+ /**
+ * Override this method if you want to filter out some translated objects (e.g. because they should not be visible to a user the translation is
+ * performed for). Because of performance overhead related with unnecessary translation do use this method only when the number of objects is
+ * small. In other cases do use shouldTranslate instead.
+ */
+ protected void filterTranslated(TranslationContext context, Map<I, O> translated)
+ {
+
+ }
+
/**
* Implementation of this method should create a translated version of the input object. Only basic attributes of the input object should be
* translated here. Parts that have a corresponding fetch option should be translated in the
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/translator/roleassignment/RoleAssignmentTranslator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/translator/roleassignment/RoleAssignmentTranslator.java
index 50bf247df77..1c643c58d61 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/translator/roleassignment/RoleAssignmentTranslator.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/translator/roleassignment/RoleAssignmentTranslator.java
@@ -18,6 +18,7 @@ package ch.ethz.sis.openbis.generic.server.asapi.v3.translator.roleassignment;
import java.util.Collection;
import java.util.HashSet;
+import java.util.Map;
import java.util.Set;
import org.springframework.beans.factory.annotation.Autowired;
@@ -34,38 +35,40 @@ import ch.ethz.sis.openbis.generic.server.asapi.v3.translator.AbstractCachingTra
import ch.ethz.sis.openbis.generic.server.asapi.v3.translator.TranslationContext;
import ch.ethz.sis.openbis.generic.server.asapi.v3.translator.TranslationResults;
import ch.systemsx.cisd.common.exceptions.AuthorizationFailureException;
+import ch.systemsx.cisd.openbis.generic.shared.authorization.IAuthorizationConfig;
/**
- *
- *
* @author Franz-Josef Elmer
*/
@Component
-public class RoleAssignmentTranslator
- extends AbstractCachingTranslator<Long, RoleAssignment, RoleAssignmentFetchOptions>
+public class RoleAssignmentTranslator
+ extends AbstractCachingTranslator<Long, RoleAssignment, RoleAssignmentFetchOptions>
implements IRoleAssignmentTranslator
{
@Autowired
private IRoleAssignmentAuthorizationExecutor authorizationExecutor;
-
+
@Autowired
private IRoleAssignmentBaseTranslator baseTranslator;
-
+
@Autowired
private IRoleAssignmentRegistratorTranslator registratorTranslator;
-
+
@Autowired
private IRoleAssignmentPersonTranslator userTranslator;
-
+
@Autowired
private IRoleAssignmentAuthorizationGroupTranslator authorizationGroupTranslator;
-
+
@Autowired
private IRoleAssignmentSpaceTranslator spaceTranslator;
-
+
@Autowired
private IRoleAssignmentProjectTranslator projectTranslator;
+ @Autowired
+ private IAuthorizationConfig authorizationConfig;
+
@Override
protected Set<Long> shouldTranslate(TranslationContext context, Collection<Long> inputs, RoleAssignmentFetchOptions fetchOptions)
{
@@ -79,6 +82,30 @@ public class RoleAssignmentTranslator
}
}
+ @Override
+ protected void filterTranslated(TranslationContext context, Map<Long, RoleAssignment> translated)
+ {
+ if (authorizationConfig.isProjectLevelEnabled())
+ {
+ return;
+ }
+
+ Collection<Long> projectRoleIds = new HashSet<Long>();
+
+ for (Map.Entry<Long, RoleAssignment> entry : translated.entrySet())
+ {
+ if (RoleLevel.PROJECT.equals(entry.getValue().getRoleLevel()))
+ {
+ projectRoleIds.add(entry.getKey());
+ }
+ }
+
+ for (Long projectRoleId : projectRoleIds)
+ {
+ translated.remove(projectRoleId);
+ }
+ }
+
@Override
protected RoleAssignment createObject(TranslationContext context, Long input, RoleAssignmentFetchOptions fetchOptions)
{
@@ -91,11 +118,12 @@ public class RoleAssignmentTranslator
protected Object getObjectsRelations(TranslationContext context, Collection<Long> inputs, RoleAssignmentFetchOptions fetchOptions)
{
TranslationResults relations = new TranslationResults();
-
+
relations.put(IRoleAssignmentBaseTranslator.class, baseTranslator.translate(context, inputs, null));
if (fetchOptions.hasRegistrator())
{
- relations.put(IRoleAssignmentRegistratorTranslator.class, registratorTranslator.translate(context, inputs, fetchOptions.withRegistrator()));
+ relations.put(IRoleAssignmentRegistratorTranslator.class,
+ registratorTranslator.translate(context, inputs, fetchOptions.withRegistrator()));
}
if (fetchOptions.hasUser())
{
@@ -103,7 +131,8 @@ public class RoleAssignmentTranslator
}
if (fetchOptions.hasAuthorizationGroup())
{
- relations.put(IRoleAssignmentAuthorizationGroupTranslator.class, authorizationGroupTranslator.translate(context, inputs, fetchOptions.withAuthorizationGroup()));
+ relations.put(IRoleAssignmentAuthorizationGroupTranslator.class,
+ authorizationGroupTranslator.translate(context, inputs, fetchOptions.withAuthorizationGroup()));
}
if (fetchOptions.hasSpace())
{
@@ -113,7 +142,7 @@ public class RoleAssignmentTranslator
{
relations.put(IRoleAssignmentProjectTranslator.class, projectTranslator.translate(context, inputs, fetchOptions.withProject()));
}
-
+
return relations;
}
@@ -127,7 +156,7 @@ public class RoleAssignmentTranslator
output.setRole(Role.valueOf(baseRecord.role_code));
output.setRoleLevel(extractRoleLevel(baseRecord));
output.setRegistrationDate(baseRecord.registrationDate);
-
+
if (fetchOptions.hasRegistrator())
{
output.setRegistrator(relations.get(IRoleAssignmentRegistratorTranslator.class, input));
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java
index d8333c424c1..d17c567bd75 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/AbstractServer.java
@@ -800,8 +800,11 @@ public abstract class AbstractServer<T> extends AbstractServiceWithLogger<T> imp
final PersonPE person = session.tryGetPerson();
if (person != null)
{
- getDAOFactory().getPersonDAO().lock(person);
- displaySettingsProvider.executeActionWithPersonLock(person, new IDelegatedActionWithResult<Void>()
+ org.hibernate.Session hibernateSession = getDAOFactory().getSessionFactory().getCurrentSession();
+ PersonPE attachedPerson = (PersonPE) hibernateSession.get(PersonPE.class, person.getId());
+
+ getDAOFactory().getPersonDAO().lock(attachedPerson);
+ displaySettingsProvider.executeActionWithPersonLock(attachedPerson, new IDelegatedActionWithResult<Void>()
{
@Override
public Void execute(boolean didOperationSucceed)
@@ -815,8 +818,8 @@ public abstract class AbstractServer<T> extends AbstractServiceWithLogger<T> imp
visits.remove(i);
}
}
- displaySettingsProvider.replaceRegularDisplaySettings(person, displaySettings);
- getDAOFactory().getPersonDAO().updatePerson(person);
+ displaySettingsProvider.replaceRegularDisplaySettings(attachedPerson, displaySettings);
+ getDAOFactory().getPersonDAO().updatePerson(attachedPerson);
return null;
}
});
@@ -845,19 +848,22 @@ public abstract class AbstractServer<T> extends AbstractServiceWithLogger<T> imp
final PersonPE person = session.tryGetPerson();
if (person != null)
{
- getDAOFactory().getPersonDAO().lock(person);
- displaySettingsProvider.executeActionWithPersonLock(person, new IDelegatedActionWithResult<Void>()
+ org.hibernate.Session hibernateSession = getDAOFactory().getSessionFactory().getCurrentSession();
+ PersonPE attachedPerson = (PersonPE) hibernateSession.get(PersonPE.class, person.getId());
+
+ getDAOFactory().getPersonDAO().lock(attachedPerson);
+ displaySettingsProvider.executeActionWithPersonLock(attachedPerson, new IDelegatedActionWithResult<Void>()
{
@Override
public Void execute(boolean didOperationSucceed)
{
DisplaySettings currentDisplaySettings =
- displaySettingsProvider.getCurrentDisplaySettings(person);
+ displaySettingsProvider.getCurrentDisplaySettings(attachedPerson);
DisplaySettings newDisplaySettings =
displaySettingsUpdate.update(currentDisplaySettings);
- displaySettingsProvider.replaceCurrentDisplaySettings(person,
+ displaySettingsProvider.replaceCurrentDisplaySettings(attachedPerson,
newDisplaySettings);
- getDAOFactory().getPersonDAO().updatePerson(person);
+ getDAOFactory().getPersonDAO().updatePerson(attachedPerson);
return null;
}
});
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/NewSamplePredicate.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/NewSamplePredicate.java
index 1058c287970..01f65db4f20 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/NewSamplePredicate.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/NewSamplePredicate.java
@@ -69,13 +69,16 @@ public final class NewSamplePredicate extends AbstractPredicate<NewSample>
Status status = Status.OK;
- if (value.getIdentifier() != null)
+ if (value.getProjectIdentifier() == null && value.getExperimentIdentifier() == null)
{
- status = newSampleIdentifierPredicate.doEvaluation(person, allowedRoles, value);
-
- if (false == status.equals(Status.OK))
+ if (value.getIdentifier() != null)
{
- return status;
+ status = newSampleIdentifierPredicate.doEvaluation(person, allowedRoles, value);
+
+ if (false == status.equals(Status.OK))
+ {
+ return status;
+ }
}
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SampleUpdatesCollectionPredicate.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SampleUpdatesCollectionPredicate.java
index c7d3b48987c..c121aaaf3f1 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SampleUpdatesCollectionPredicate.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SampleUpdatesCollectionPredicate.java
@@ -81,7 +81,6 @@ public class SampleUpdatesCollectionPredicate extends AbstractPredicate<List<Sam
IServiceConversationProgressListener progressListener =
ServiceConversationsThreadContext.getProgressListener();
List<TechId> techIds = new ArrayList<TechId>(value.size());
- List<SampleIdentifier> sampleIdentifiers = new ArrayList<SampleIdentifier>(value.size());
List<SampleIdentifier> containerIdentifiers = new ArrayList<SampleIdentifier>();
int index = 0;
@@ -102,11 +101,6 @@ public class SampleUpdatesCollectionPredicate extends AbstractPredicate<List<Sam
{
return result;
}
- SampleIdentifier sampleIdentifier = sampleUpdates.getSampleIdentifier();
- if (sampleIdentifier != null)
- {
- sampleIdentifiers.add(sampleIdentifier);
- }
if (sampleUpdates.getContainerIdentifierOrNull() != null)
{
@@ -132,7 +126,7 @@ public class SampleUpdatesCollectionPredicate extends AbstractPredicate<List<Sam
}
}
- return sampleIdentifierCollectionPredicate.doEvaluation(person, allowedRoles, sampleIdentifiers);
+ return result;
}
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SampleUpdatesPredicate.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SampleUpdatesPredicate.java
index dec331cf900..35b05ba3e48 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SampleUpdatesPredicate.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/predicate/SampleUpdatesPredicate.java
@@ -95,11 +95,6 @@ public class SampleUpdatesPredicate extends AbstractSamplePredicate<SampleUpdate
}
}
- if (updates.getSampleIdentifier() != null)
- {
- status = sampleIdentifierPredicate.doEvaluation(person, allowedRoles, updates.getSampleIdentifier());
- }
-
return status;
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleBO.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleBO.java
index ffa01b3ee19..f2f75d8a227 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleBO.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleBO.java
@@ -40,6 +40,7 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.dto.id.sample.SampleTechIdI
import ch.systemsx.cisd.openbis.generic.shared.dto.AttachmentPE;
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.ProjectPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE;
import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePropertyPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.SampleTypePE;
@@ -297,6 +298,10 @@ public final class SampleBO extends AbstractSampleBusinessObject implements ISam
{
throwModifiedEntityException("Sample");
}
+
+ ExperimentPE oldExperiment = sample.getExperiment();
+ ProjectPE oldProject = sample.getProject();
+
updateProperties(sample.getSampleType(), updates.getProperties(), extractPropertiesCodes(updates.getProperties()), sample, sample);
spaceUpdated = updateSpace(sample, updates.getSampleIdentifier(), null);
if (updates.isUpdateExperimentLink())
@@ -310,6 +315,11 @@ public final class SampleBO extends AbstractSampleBusinessObject implements ISam
updateParents(updates);
setMetaprojects(sample, updates.getMetaprojectsOrNull());
+ if ((oldExperiment != null || oldProject != null) && (sample.getExperiment() == null && sample.getProject() == null))
+ {
+ relationshipService.assignSampleToSpace(session, sample, sample.getSpace());
+ }
+
dataChanged = true;
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleTable.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleTable.java
index f388d7ee44f..2432d8f1e8f 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleTable.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleTable.java
@@ -271,6 +271,10 @@ public final class SampleTable extends AbstractSampleBusinessObject implements I
"No sample could be found with given identifier '%s'.",
updates.getOldSampleIdentifierOrNull());
}
+
+ ExperimentPE oldExperiment = sample.getExperiment();
+ ProjectPE oldProject = sample.getProject();
+
SampleBatchUpdateDetails details = updates.getDetails();
batchUpdateProperties(sample, updates.getProperties(), details.getPropertiesToUpdate());
checkPropertiesBusinessRules(sample, propertiesCache);
@@ -307,6 +311,11 @@ public final class SampleTable extends AbstractSampleBusinessObject implements I
{
checkContainerBusinessRules(sample);
}
+
+ if ((oldExperiment != null || oldProject != null) && (sample.getExperiment() == null && sample.getProject() == null))
+ {
+ relationshipService.assignSampleToSpace(session, sample, sample.getSpace());
+ }
}
/**
@@ -325,6 +334,9 @@ public final class SampleTable extends AbstractSampleBusinessObject implements I
updates.getSampleIdentifier());
}
+ ExperimentPE oldExperiment = sample.getExperiment();
+ ProjectPE oldProject = sample.getProject();
+
updateProperties(sample, updates.getProperties());
checkPropertiesBusinessRules(sample, propertiesCache);
@@ -335,7 +347,7 @@ public final class SampleTable extends AbstractSampleBusinessObject implements I
updateExperiment(sample, experimentIdentifier, experimentCache);
checkExperimentBusinessRules(sample);
}
-
+
if (experimentIdentifier == null)
{
updateProject(sample, updates.getProjectIdentifier(), projectCache);
@@ -357,6 +369,12 @@ public final class SampleTable extends AbstractSampleBusinessObject implements I
{
checkContainerBusinessRules(sample);
}
+
+ if ((oldExperiment != null || oldProject != null) && (sample.getExperiment() == null && sample.getProject() == null))
+ {
+ relationshipService.assignSampleToSpace(session, sample, sample.getSpace());
+ }
+
RelationshipUtils.updateModificationDateAndModifier(sample, session, getTransactionTimeStamp());
}
diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/UpdateSampleTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/UpdateSampleTest.java
index 1c6f755fa57..23bca7c6a96 100644
--- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/UpdateSampleTest.java
+++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/UpdateSampleTest.java
@@ -518,6 +518,33 @@ public class UpdateSampleTest extends AbstractSampleTest
Assert.assertNull(sample.getExperiment());
}
+ @Test
+ public void testUpdateWithExperimentSampleTurnedIntoSpaceSampleAsProjectUser()
+ {
+ String sessionToken = v3api.login(ProjectAuthorizationUser.TEST_PROJECT_PA_ON, PASSWORD);
+
+ SampleCreation creation = new SampleCreation();
+ creation.setCode("SAMPLE");
+ creation.setTypeId(new EntityTypePermId("CELL_PLATE"));
+ creation.setSpaceId(new SpacePermId("TEST-SPACE"));
+ creation.setExperimentId(new ExperimentIdentifier("/TEST-SPACE/TEST-PROJECT/EXP-SPACE-TEST"));
+
+ List<SamplePermId> ids = v3api.createSamples(sessionToken, Arrays.asList(creation));
+
+ SampleUpdate update = new SampleUpdate();
+ update.setSampleId(ids.get(0));
+ update.setExperimentId(null);
+
+ assertAuthorizationFailureException(new IDelegatedAction()
+ {
+ @Override
+ public void execute()
+ {
+ v3api.updateSamples(sessionToken, Arrays.asList(update));
+ }
+ });
+ }
+
@Test
public void testUpdateWithExperimentNullForSampleWithDataSets()
{
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/authorization/ProjectAuthorizationUser.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/authorization/ProjectAuthorizationUser.java
index 05f2fcd2e5d..7b3c1fe83fc 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/authorization/ProjectAuthorizationUser.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/authorization/ProjectAuthorizationUser.java
@@ -28,6 +28,22 @@ import org.testng.annotations.DataProvider;
public class ProjectAuthorizationUser
{
+ public static final String INSTANCE_ADMIN = "instance_admin";
+
+ public static final String TEST_SPACE_PA_ON = "test_space_pa_on";
+
+ public static final String TEST_SPACE_PA_OFF = "test_space_pa_off";
+
+ public static final String TEST_PROJECT_PA_ON = "test_project_pa_on";
+
+ public static final String TEST_PROJECT_PA_OFF = "test_project_pa_off";
+
+ public static final String ETL_SERVER = "etlserver";
+
+ public static final String TEST_SPACE_ETL_SERVER = "test_space_etl_server";
+
+ public static final String TEST_GROUP_ETL_SERVER = "test_group_etl_server";
+
public static final String PROVIDER = "project-authorization-users-provider";
public static final String PROVIDER_WITH_ETL = "project-authorization-users-provider-with-etl";
@@ -149,20 +165,20 @@ public class ProjectAuthorizationUser
@DataProvider(name = PROVIDER)
public static Object[][] providerUsers()
{
- ProjectAuthorizationUser instanceAdmin = new ProjectAuthorizationUser("instance_admin");
+ ProjectAuthorizationUser instanceAdmin = new ProjectAuthorizationUser(INSTANCE_ADMIN);
instanceAdmin.setInstanceUser(true);
- ProjectAuthorizationUser testSpacePAOff = new ProjectAuthorizationUser("test_space_pa_off");
+ ProjectAuthorizationUser testSpacePAOff = new ProjectAuthorizationUser(TEST_SPACE_PA_OFF);
testSpacePAOff.setTestSpaceUser(true);
- ProjectAuthorizationUser testSpacePAOn = new ProjectAuthorizationUser("test_space_pa_on");
+ ProjectAuthorizationUser testSpacePAOn = new ProjectAuthorizationUser(TEST_SPACE_PA_ON);
testSpacePAOn.setTestSpaceUser(true);
testSpacePAOn.setPAEnabled(true);
- ProjectAuthorizationUser testProjectPAOff = new ProjectAuthorizationUser("test_project_pa_off");
+ ProjectAuthorizationUser testProjectPAOff = new ProjectAuthorizationUser(TEST_PROJECT_PA_OFF);
testProjectPAOff.setTestProjectUser(true);
- ProjectAuthorizationUser testProjectPAOn = new ProjectAuthorizationUser("test_project_pa_on");
+ ProjectAuthorizationUser testProjectPAOn = new ProjectAuthorizationUser(TEST_PROJECT_PA_ON);
testProjectPAOn.setTestProjectUser(true);
testProjectPAOn.setPAEnabled(true);
@@ -182,15 +198,15 @@ public class ProjectAuthorizationUser
@DataProvider(name = PROVIDER_WITH_ETL)
public static Object[][] provideUsersWithETL()
{
- ProjectAuthorizationUser instanceETLServer = new ProjectAuthorizationUser("etlserver");
+ ProjectAuthorizationUser instanceETLServer = new ProjectAuthorizationUser(ETL_SERVER);
instanceETLServer.setInstanceUser(true);
instanceETLServer.setETLServerUser(true);
- ProjectAuthorizationUser testSpaceETLServer = new ProjectAuthorizationUser("test_space_etl_server");
+ ProjectAuthorizationUser testSpaceETLServer = new ProjectAuthorizationUser(TEST_SPACE_ETL_SERVER);
testSpaceETLServer.setTestSpaceUser(true);
testSpaceETLServer.setETLServerUser(true);
- ProjectAuthorizationUser testGroupETLServer = new ProjectAuthorizationUser("test_group_etl_server");
+ ProjectAuthorizationUser testGroupETLServer = new ProjectAuthorizationUser(TEST_GROUP_ETL_SERVER);
testGroupETLServer.setTestGroupUser(true);
testGroupETLServer.setETLServerUser(true);
--
GitLab