Skip to content
Snippets Groups Projects
Commit 02fb6794 authored by cramakri's avatar cramakri
Browse files

LMS-1153 Switch data set access checking to use new, more efficient query.

SVN: 15668
parent 00f6ae4c
No related branches found
No related tags found
No related merge requests found
...@@ -20,17 +20,21 @@ import java.util.HashMap; ...@@ -20,17 +20,21 @@ import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import org.hibernate.Query;
import org.hibernate.Session;
import ch.rinn.restrictions.Private; import ch.rinn.restrictions.Private;
import ch.systemsx.cisd.common.exceptions.Status; import ch.systemsx.cisd.common.exceptions.Status;
import ch.systemsx.cisd.openbis.generic.server.dataaccess.IAuthorizationDAOFactory; import ch.systemsx.cisd.openbis.generic.server.dataaccess.IAuthorizationDAOFactory;
import ch.systemsx.cisd.openbis.generic.shared.authorization.SpaceOwnerKind;
import ch.systemsx.cisd.openbis.generic.shared.authorization.IAuthorizationDataProvider; 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.RoleWithIdentifier;
import ch.systemsx.cisd.openbis.generic.shared.authorization.SpaceOwnerKind;
import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.ArrayPredicate; import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.ArrayPredicate;
import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.CollectionPredicate; import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.CollectionPredicate;
import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.IPredicate; import ch.systemsx.cisd.openbis.generic.shared.authorization.predicate.IPredicate;
import ch.systemsx.cisd.openbis.generic.shared.basic.TechId; import ch.systemsx.cisd.openbis.generic.shared.basic.TechId;
import ch.systemsx.cisd.openbis.generic.shared.dto.DataPE; import ch.systemsx.cisd.openbis.generic.shared.dto.DataPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.DataSetAccessPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE; import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE;
import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE; 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.ExternalDataPE;
...@@ -242,6 +246,18 @@ public final class PredicateExecutor ...@@ -242,6 +246,18 @@ public final class PredicateExecutor
} }
} }
@SuppressWarnings("unchecked")
public DataSetAccessPE tryGetDatasetAccessData(String dataSetCode)
{
Session sess = daoFactory.getSessionFactory().getCurrentSession();
Query query = sess.getNamedQuery(DataSetAccessPE.DATASET_ACCESS_QUERY_NAME);
query = query.setReadOnly(true);
List<DataSetAccessPE> results = query.setString(0, dataSetCode).list();
if (results.size() < 1)
return null;
return results.get(0);
}
public GroupPE tryToGetGroup(SpaceOwnerKind kind, TechId techId) public GroupPE tryToGetGroup(SpaceOwnerKind kind, TechId techId)
{ {
switch (kind) switch (kind)
......
...@@ -20,6 +20,7 @@ import java.util.List; ...@@ -20,6 +20,7 @@ import java.util.List;
import ch.systemsx.cisd.openbis.generic.shared.IDatabaseInstanceFinder; import ch.systemsx.cisd.openbis.generic.shared.IDatabaseInstanceFinder;
import ch.systemsx.cisd.openbis.generic.shared.basic.TechId; import ch.systemsx.cisd.openbis.generic.shared.basic.TechId;
import ch.systemsx.cisd.openbis.generic.shared.dto.DataSetAccessPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.GridCustomColumnPE; import ch.systemsx.cisd.openbis.generic.shared.dto.GridCustomColumnPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.GridCustomFilterPE; import ch.systemsx.cisd.openbis.generic.shared.dto.GridCustomFilterPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.GroupPE; import ch.systemsx.cisd.openbis.generic.shared.dto.GroupPE;
...@@ -46,6 +47,11 @@ public interface IAuthorizationDataProvider extends IDatabaseInstanceFinder ...@@ -46,6 +47,11 @@ public interface IAuthorizationDataProvider extends IDatabaseInstanceFinder
*/ */
public ProjectPE tryToGetProject(String dataSetCode); public ProjectPE tryToGetProject(String dataSetCode);
/**
* Returns the information necessary to determine if a user is allowed to access this dataset.
*/
public DataSetAccessPE tryGetDatasetAccessData(String dataSetCode);
/** /**
* Returns the group of an entity with given <var>entityKind</var> and <var>techId</var> * Returns the group of an entity with given <var>entityKind</var> and <var>techId</var>
* *
...@@ -67,7 +73,7 @@ public interface IAuthorizationDataProvider extends IDatabaseInstanceFinder ...@@ -67,7 +73,7 @@ public interface IAuthorizationDataProvider extends IDatabaseInstanceFinder
* Returns the grid custom column with given <var>techId</var> * Returns the grid custom column with given <var>techId</var>
*/ */
public GridCustomColumnPE getGridCustomColumn(TechId techId); public GridCustomColumnPE getGridCustomColumn(TechId techId);
/** /**
* Returns the query with specified ID. * Returns the query with specified ID.
*/ */
......
...@@ -49,8 +49,16 @@ abstract class AbstractGroupPredicate<T> extends AbstractDatabaseInstancePredica ...@@ -49,8 +49,16 @@ abstract class AbstractGroupPredicate<T> extends AbstractDatabaseInstancePredica
final DatabaseInstancePE databaseInstance, final String groupCodeOrNull) final DatabaseInstancePE databaseInstance, final String groupCodeOrNull)
{ {
final String databaseInstanceUUID = databaseInstance.getUuid(); final String databaseInstanceUUID = databaseInstance.getUuid();
return evaluate(person, allowedRoles, databaseInstanceUUID, databaseInstance.getCode(),
groupCodeOrNull);
}
protected Status evaluate(final PersonPE person, final List<RoleWithIdentifier> allowedRoles,
final String databaseInstanceUUID, final String databaseInstanceCode,
final String groupCodeOrNull)
{
final GroupIdentifier fullGroupIdentifier = final GroupIdentifier fullGroupIdentifier =
new GroupIdentifier(databaseInstance.getCode(), groupCodeOrNull); new GroupIdentifier(databaseInstanceCode, groupCodeOrNull);
ensureGroupExists(fullGroupIdentifier, databaseInstanceUUID, groupCodeOrNull); ensureGroupExists(fullGroupIdentifier, databaseInstanceUUID, groupCodeOrNull);
final boolean matching = isMatching(allowedRoles, databaseInstanceUUID, groupCodeOrNull); final boolean matching = isMatching(allowedRoles, databaseInstanceUUID, groupCodeOrNull);
if (matching) if (matching)
...@@ -59,7 +67,7 @@ abstract class AbstractGroupPredicate<T> extends AbstractDatabaseInstancePredica ...@@ -59,7 +67,7 @@ abstract class AbstractGroupPredicate<T> extends AbstractDatabaseInstancePredica
} }
return Status.createError(String.format( return Status.createError(String.format(
"User '%s' does not have enough privileges to access data in the space '%s'.", "User '%s' does not have enough privileges to access data in the space '%s'.",
person.getUserId(), new GroupIdentifier(databaseInstance.getCode(), groupCodeOrNull))); person.getUserId(), new GroupIdentifier(databaseInstanceCode, groupCodeOrNull)));
} }
private void ensureGroupExists(final GroupIdentifier groupIdentifier, private void ensureGroupExists(final GroupIdentifier groupIdentifier,
...@@ -91,7 +99,8 @@ abstract class AbstractGroupPredicate<T> extends AbstractDatabaseInstancePredica ...@@ -91,7 +99,8 @@ abstract class AbstractGroupPredicate<T> extends AbstractDatabaseInstancePredica
{ {
final RoleLevel roleGroup = role.getRoleLevel(); final RoleLevel roleGroup = role.getRoleLevel();
if (roleGroup.equals(RoleLevel.SPACE) if (roleGroup.equals(RoleLevel.SPACE)
&& equalIdentifier(role.getAssignedGroup(), databaseInstanceUUID, groupCodeOrNull)) && equalIdentifier(role.getAssignedGroup(), databaseInstanceUUID,
groupCodeOrNull))
{ {
return true; return true;
} else if (roleGroup.equals(RoleLevel.INSTANCE) } else if (roleGroup.equals(RoleLevel.INSTANCE)
......
...@@ -20,15 +20,13 @@ import java.util.List; ...@@ -20,15 +20,13 @@ import java.util.List;
import ch.systemsx.cisd.common.exceptions.Status; import ch.systemsx.cisd.common.exceptions.Status;
import ch.systemsx.cisd.openbis.generic.shared.authorization.RoleWithIdentifier; import ch.systemsx.cisd.openbis.generic.shared.authorization.RoleWithIdentifier;
import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE; import ch.systemsx.cisd.openbis.generic.shared.dto.DataSetAccessPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.GroupPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE; import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectPE;
/** /**
* A {@link IPredicate} based on a list of data set codes. * A {@link IPredicate} based on a list of data set codes.
* *
* @author Franz-Josef Elmer * @author Franz-Josef Elmer
*/ */
public class DataSetCodePredicate extends AbstractGroupPredicate<String> public class DataSetCodePredicate extends AbstractGroupPredicate<String>
{ {
...@@ -37,23 +35,23 @@ public class DataSetCodePredicate extends AbstractGroupPredicate<String> ...@@ -37,23 +35,23 @@ public class DataSetCodePredicate extends AbstractGroupPredicate<String>
{ {
return "data set code"; return "data set code";
} }
@Override @Override
Status doEvaluation(PersonPE person, List<RoleWithIdentifier> allowedRoles, String dataSetCode) Status doEvaluation(PersonPE person, List<RoleWithIdentifier> allowedRoles, String dataSetCode)
{ {
assert initialized : "Predicate has not been initialized"; assert initialized : "Predicate has not been initialized";
ProjectPE project = authorizationDataProvider.tryToGetProject(dataSetCode); DataSetAccessPE accessData = authorizationDataProvider.tryGetDatasetAccessData(dataSetCode);
if (project != null)
if (accessData != null)
{ {
GroupPE group = project.getGroup(); String dbInstanceUUID = accessData.getDatabaseInstanceUuid();
DatabaseInstancePE databaseInstance = group.getDatabaseInstance(); String dbInstanceCode = accessData.getDatabaseInstanceCode();
String code = group.getCode(); String groupCode = accessData.getGroupCode();
return evaluate(person, allowedRoles, databaseInstance, code); Status result =
evaluate(person, allowedRoles, dbInstanceUUID, dbInstanceCode, groupCode);
return result;
} }
return Status.OK; return Status.OK;
} }
} }
...@@ -23,6 +23,9 @@ import javax.persistence.NamedNativeQuery; ...@@ -23,6 +23,9 @@ import javax.persistence.NamedNativeQuery;
import javax.persistence.SqlResultSetMapping; import javax.persistence.SqlResultSetMapping;
/** /**
* A PE for retrieving only the information necessary to determine if a user/person can access a
* data set.
*
* @author Chandrasekhar Ramakrishnan * @author Chandrasekhar Ramakrishnan
*/ */
@Entity @Entity
...@@ -55,6 +58,22 @@ public class DataSetAccessPE ...@@ -55,6 +58,22 @@ public class DataSetAccessPE
public final static String DATASET_ACCESS_QUERY_NAME = "dataset_access"; public final static String DATASET_ACCESS_QUERY_NAME = "dataset_access";
/**
* A factory method that should only be used for testing.
*/
public static DataSetAccessPE createDataSetAccessPEForTest(String dataSetId,
String dataSetCode, String groupCode, String databaseInstanceUuid,
String databaseInstanceCode)
{
DataSetAccessPE newMe = new DataSetAccessPE();
newMe.setDataSetId(dataSetId);
newMe.setDataSetCode(dataSetCode);
newMe.setGroupCode(groupCode);
newMe.setDatabaseInstanceUuid(databaseInstanceUuid);
newMe.setDatabaseInstanceCode(databaseInstanceCode);
return newMe;
}
void setDataSetId(String dataSetId) void setDataSetId(String dataSetId)
{ {
this.dataSetId = dataSetId; this.dataSetId = dataSetId;
......
...@@ -21,6 +21,7 @@ import org.testng.annotations.Test; ...@@ -21,6 +21,7 @@ import org.testng.annotations.Test;
import ch.systemsx.cisd.common.exceptions.Status; import ch.systemsx.cisd.common.exceptions.Status;
import ch.systemsx.cisd.openbis.generic.shared.authorization.AuthorizationTestCase; import ch.systemsx.cisd.openbis.generic.shared.authorization.AuthorizationTestCase;
import ch.systemsx.cisd.openbis.generic.shared.dto.DataSetAccessPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectPE; import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectPE;
/** /**
...@@ -33,11 +34,15 @@ public class DataSetCodePredicateTest extends AuthorizationTestCase ...@@ -33,11 +34,15 @@ public class DataSetCodePredicateTest extends AuthorizationTestCase
{ {
final ProjectPE project = new ProjectPE(); final ProjectPE project = new ProjectPE();
project.setGroup(createGroup()); project.setGroup(createGroup());
final DataSetAccessPE accessData =
DataSetAccessPE.createDataSetAccessPEForTest("1", "d1", SPACE_CODE, "global_"
+ INSTANCE_CODE, INSTANCE_CODE);
context.checking(new Expectations() context.checking(new Expectations()
{ {
{ {
one(provider).tryToGetProject("d1"); one(provider).tryGetDatasetAccessData("d1");
will(returnValue(project)); will(returnValue(accessData));
} }
}); });
DataSetCodePredicate predicate = new DataSetCodePredicate(); DataSetCodePredicate predicate = new DataSetCodePredicate();
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment