From 9179d6f5bdeacbe6b2d7aedff1e7aa24db0c2f85 Mon Sep 17 00:00:00 2001 From: Guillaume Poirier-Morency Date: Sat, 14 Feb 2026 20:20:28 -0800 Subject: [PATCH] Add support for filtering ACLs using subqueries Rename EE2CAclQueryUtils to AclDenormalizedTableQueryUtils since it's also being used for other denormalized tables. Make the mask column and requested authority configurable when querying denormalized tables. --- .../BibliographicReferenceDaoImpl.java | 8 +- .../description/CharacteristicDaoImpl.java | 12 ++- .../arrayDesign/ArrayDesignDaoImpl.java | 26 +++-- .../CompositeSequenceDaoImpl.java | 6 +- .../ExpressionExperimentDaoImpl.java | 38 ++++--- .../experiment/FactorValueDaoImpl.java | 10 +- .../service/genome/GeneDaoImpl.java | 4 +- .../maintenance/TableMaintenanceUtil.java | 10 ++ .../util/AclDenormalizedTableQueryUtils.java | 102 ++++++++++++++++++ .../gemma/persistence/util/AclQueryUtils.java | 100 +++++++++++------ .../persistence/util/EE2CAclQueryUtils.java | 50 --------- .../persistence/util/AclQueryUtilsTest.java | 14 +-- 12 files changed, 246 insertions(+), 134 deletions(-) create mode 100644 gemma-core/src/main/java/ubic/gemma/persistence/util/AclDenormalizedTableQueryUtils.java delete mode 100644 gemma-core/src/main/java/ubic/gemma/persistence/util/EE2CAclQueryUtils.java diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/service/common/description/BibliographicReferenceDaoImpl.java b/gemma-core/src/main/java/ubic/gemma/persistence/service/common/description/BibliographicReferenceDaoImpl.java index 92b23f067e..e72954d538 100644 --- a/gemma-core/src/main/java/ubic/gemma/persistence/service/common/description/BibliographicReferenceDaoImpl.java +++ b/gemma-core/src/main/java/ubic/gemma/persistence/service/common/description/BibliographicReferenceDaoImpl.java @@ -75,7 +75,7 @@ public long countDistinctWithRelatedExperiments() { .createQuery( "select count(" + ( AclQueryUtils.requiresCountDistinct() ? "distinct " : "" ) + " b)" + " " + "from ExpressionExperiment e join e.primaryPublication b " + AclQueryUtils.formAclRestrictionClause( "e.id" ) ); - AclQueryUtils.addAclParameters( q, ExpressionExperiment.class ); + AclQueryUtils.setAclParameters( q, ExpressionExperiment.class ); return ( Long ) q.uniqueResult(); } @@ -87,7 +87,7 @@ public long countWithRelatedExperiments() { .createQuery( "select count(" + ( AclQueryUtils.requiresCountDistinct() ? "distinct " : "" ) + " e)" + " " + "from ExpressionExperiment e join e.primaryPublication b" + AclQueryUtils.formAclRestrictionClause( "e.id" ) ); - AclQueryUtils.addAclParameters( q, ExpressionExperiment.class ); + AclQueryUtils.setAclParameters( q, ExpressionExperiment.class ); return ( Long ) q.uniqueResult(); } @@ -99,7 +99,7 @@ public LinkedHashMap os = q .setFirstResult( offset ) @@ -126,7 +126,7 @@ public LinkedHashMap> g + "and b in (:recs) " + ( AclQueryUtils.requiresGroupBy() ? "group by b, e " : "" ) + "order by b.authorList nulls last, b.title nulls last" ); - AclQueryUtils.addAclParameters( query, ExpressionExperiment.class ); + AclQueryUtils.setAclParameters( query, ExpressionExperiment.class ); List os = QueryUtils.listByIdentifiableBatch( query, "recs", records, eeBatchSize ); LinkedHashMap> result = new LinkedHashMap<>(); for ( Object[] o : os ) { diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/service/common/description/CharacteristicDaoImpl.java b/gemma-core/src/main/java/ubic/gemma/persistence/service/common/description/CharacteristicDaoImpl.java index 45415335b8..fdbfc4727c 100644 --- a/gemma-core/src/main/java/ubic/gemma/persistence/service/common/description/CharacteristicDaoImpl.java +++ b/gemma-core/src/main/java/ubic/gemma/persistence/service/common/description/CharacteristicDaoImpl.java @@ -29,6 +29,7 @@ import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.type.StandardBasicTypes; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.acls.domain.BasePermission; import org.springframework.stereotype.Repository; import org.springframework.util.Assert; import ubic.gemma.core.ontology.OntologyUtils; @@ -45,7 +46,7 @@ import ubic.gemma.model.genome.Taxon; import ubic.gemma.model.genome.gene.GeneSet; import ubic.gemma.persistence.service.AbstractNoopFilteringVoEnabledDao; -import ubic.gemma.persistence.util.EE2CAclQueryUtils; +import ubic.gemma.persistence.util.AclDenormalizedTableQueryUtils; import ubic.gemma.persistence.util.IdentifiableUtils; import ubic.gemma.persistence.util.QueryUtils; @@ -56,6 +57,7 @@ import static org.hibernate.criterion.Restrictions.like; import static org.hibernate.criterion.Restrictions.not; +import static ubic.gemma.persistence.service.maintenance.TableMaintenanceUtil.EE2C_IS_AUTHENTICATED_ANONYMOUSLY_MASK_COLUMN; import static ubic.gemma.persistence.service.maintenance.TableMaintenanceUtil.EE2C_QUERY_SPACE; import static ubic.gemma.persistence.util.IdentifiableUtils.toIdentifiableSet; import static ubic.gemma.persistence.util.QueryUtils.*; @@ -242,11 +244,13 @@ public Map, Map> private Map, Map>> findExperimentsByUrisInternal( Collection uris, boolean includeSubjects, boolean includePredicates, boolean includeObjects, @Nullable Taxon taxon, boolean rankByLevel, int limit ) { String qs = "select T.`LEVEL`, T.VALUE_URI, T.PREDICATE_URI, T.OBJECT_URI, T.SECOND_PREDICATE_URI, T.SECOND_OBJECT_URI, T.EXPRESSION_EXPERIMENT_FK from EXPRESSION_EXPERIMENT2CHARACTERISTIC T" + ( taxon != null ? " join INVESTIGATION I on T.EXPRESSION_EXPERIMENT_FK = I.ID " : "" ) - + EE2CAclQueryUtils.formNativeAclJoinClause( "T.EXPRESSION_EXPERIMENT_FK" ) + " " + + AclDenormalizedTableQueryUtils.formNativeAclJoinClause( "T.EXPRESSION_EXPERIMENT_FK", "IS_AUTHENTICATED_ANONYMOUSLY" ) + " " + "where " + createPredicates( "T", "uris", includeSubjects, includePredicates, includeObjects ) + ( taxon != null ? " and I.TAXON_FK = :taxonId" : "" ) - + EE2CAclQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), "T.ACL_IS_AUTHENTICATED_ANONYMOUSLY_MASK" ) + + AclDenormalizedTableQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), + "IS_AUTHENTICATED_ANONYMOUSLY", "T." + EE2C_IS_AUTHENTICATED_ANONYMOUSLY_MASK_COLUMN, + BasePermission.READ ) + ( rankByLevel ? " order by FIELD(T.LEVEL, :eeClass, :edClass, :bmClass)" : "" ); Query query = getSessionFactory().getCurrentSession().createSQLQuery( qs ) @@ -274,7 +278,7 @@ private Map, Map>> findExperimen query.setParameter( "taxonId", taxon.getId() ); } - EE2CAclQueryUtils.addAclParameters( query, ExpressionExperiment.class ); + AclDenormalizedTableQueryUtils.setAclParameters( query, "IS_AUTHENTICATED_ANONYMOUSLY", ExpressionExperiment.class ); query.setCacheable( true ); diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/arrayDesign/ArrayDesignDaoImpl.java b/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/arrayDesign/ArrayDesignDaoImpl.java index 185a481902..a20b3f42fe 100644 --- a/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/arrayDesign/ArrayDesignDaoImpl.java +++ b/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/arrayDesign/ArrayDesignDaoImpl.java @@ -25,6 +25,7 @@ import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.type.StandardBasicTypes; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.acls.domain.BasePermission; import org.springframework.stereotype.Repository; import org.springframework.util.Assert; import ubic.gemma.model.common.auditAndSecurity.AuditEvent; @@ -43,6 +44,7 @@ import ubic.gemma.model.genome.sequenceAnalysis.BlatAssociation; import ubic.gemma.model.genome.sequenceAnalysis.BlatResult; import ubic.gemma.persistence.service.common.auditAndSecurity.curation.AbstractCuratableDao; +import ubic.gemma.persistence.service.maintenance.TableMaintenanceUtil; import ubic.gemma.persistence.util.*; import ubic.gemma.persistence.util.Filter; @@ -492,7 +494,7 @@ public long countExpressionExperiments( ArrayDesign arrayDesign ) { + AclQueryUtils.formAclRestrictionClause( "ee.id" ) + " " + "and ad = :ad" ) .setParameter( "ad", arrayDesign ); - AclQueryUtils.addAclParameters( query, ExpressionExperiment.class ); + AclQueryUtils.setAclParameters( query, ExpressionExperiment.class ); return ( Long ) query.uniqueResult(); } @@ -503,7 +505,7 @@ public Map getPerTaxonCount() { + "join ad.primaryTaxon t " + AclQueryUtils.formAclRestrictionClause( "ad.id" ) + " " + "group by t" ); - AclQueryUtils.addAclParameters( query, ArrayDesign.class ); + AclQueryUtils.setAclParameters( query, ArrayDesign.class ); //noinspection unchecked List csList = query.setCacheable( true ).list(); return csList.stream().collect( Collectors.toMap( row -> ( Taxon ) row[0], row -> ( Long ) row[1] ) ); @@ -536,7 +538,7 @@ public long countSwitchedExpressionExperiments( ArrayDesign arrayDesign ) { + AclQueryUtils.formAclRestrictionClause( "e.id" ) + " " + "and b.originalPlatform = :arrayDesign" ) .setParameter( "arrayDesign", arrayDesign ); - AclQueryUtils.addAclParameters( query, ExpressionExperiment.class ); + AclQueryUtils.setAclParameters( query, ExpressionExperiment.class ); return ( Long ) query .setCacheable( true ) .uniqueResult(); @@ -1059,7 +1061,7 @@ private Query finishFilteringQuery( String queryString, @Nullable Filters filter Query query = this.getSessionFactory().getCurrentSession().createQuery( queryString ); - AclQueryUtils.addAclParameters( query, ArrayDesign.class ); + AclQueryUtils.setAclParameters( query, ArrayDesign.class ); FilterQueryUtils.addRestrictionParameters( query, filters ); return query; @@ -1136,10 +1138,12 @@ private void populateBlacklisted( Collection vos ) { private void populateExpressionExperimentCount( Collection entities ) { Query query = this.getSessionFactory().getCurrentSession() .createSQLQuery( "select ee2ad.ARRAY_DESIGN_FK as ID, count(distinct ee2ad.EXPRESSION_EXPERIMENT_FK) as EE_COUNT from EXPRESSION_EXPERIMENT2ARRAY_DESIGN ee2ad " - + EE2CAclQueryUtils.formNativeAclJoinClause( "ee2ad.EXPRESSION_EXPERIMENT_FK" ) + " " + + AclDenormalizedTableQueryUtils.formNativeAclJoinClause( "ee2ad.EXPRESSION_EXPERIMENT_FK", "IS_AUTHENTICATED_ANONYMOUSLY" ) + " " + "where ee2ad.ARRAY_DESIGN_FK in :ids " + "and not ee2ad.IS_ORIGINAL_PLATFORM" - + EE2CAclQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), "ee2ad.ACL_IS_AUTHENTICATED_ANONYMOUSLY_MASK" ) + + AclDenormalizedTableQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), + "IS_AUTHENTICATED_ANONYMOUSLY", "ee2ad." + TableMaintenanceUtil.EE2AD_IS_AUTHENTICATED_ANONYMOUSLY_MASK_COLUMN, + BasePermission.READ ) + formNativeNonTroubledClause( "ee2ad.EXPRESSION_EXPERIMENT_FK", ExpressionExperiment.class ) + " group by ee2ad.ARRAY_DESIGN_FK" ) .addScalar( "ID", StandardBasicTypes.LONG ) @@ -1150,7 +1154,7 @@ private void populateExpressionExperimentCount( Collection countById = QueryUtils.streamByBatch( query, "ids", IdentifiableUtils.getIds( entities ), 2048 ) .collect( Collectors.toMap( o -> ( Long ) o[0], o -> ( Long ) o[1] ) ); for ( ArrayDesignValueObject vo : entities ) { @@ -1162,12 +1166,14 @@ private void populateExpressionExperimentCount( Collection entities ) { Query query = this.getSessionFactory().getCurrentSession() .createSQLQuery( "select ee2ad.ARRAY_DESIGN_FK as ID, count(distinct ee2ad.EXPRESSION_EXPERIMENT_FK) as EE_COUNT from EXPRESSION_EXPERIMENT2ARRAY_DESIGN ee2ad " - + EE2CAclQueryUtils.formNativeAclJoinClause( "ee2ad.EXPRESSION_EXPERIMENT_FK" ) + " " + + AclDenormalizedTableQueryUtils.formNativeAclJoinClause( "ee2ad.EXPRESSION_EXPERIMENT_FK", "IS_AUTHENTICATED_ANONYMOUSLY" ) + " " + "where ee2ad.ARRAY_DESIGN_FK in :ids " + "and ee2ad.IS_ORIGINAL_PLATFORM " // ignore noop switches + "and ee2ad.ARRAY_DESIGN_FK not in (select ARRAY_DESIGN_FK from EXPRESSION_EXPERIMENT2ARRAY_DESIGN where EXPRESSION_EXPERIMENT_FK = ee2ad.EXPRESSION_EXPERIMENT_FK and ARRAY_DESIGN_FK = ee2ad.ARRAY_DESIGN_FK and not IS_ORIGINAL_PLATFORM)" - + EE2CAclQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), "ee2ad.ACL_IS_AUTHENTICATED_ANONYMOUSLY_MASK" ) + + AclDenormalizedTableQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), + "IS_AUTHENTICATED_ANONYMOUSLY", "ee2ad." + TableMaintenanceUtil.EE2AD_IS_AUTHENTICATED_ANONYMOUSLY_MASK_COLUMN, + BasePermission.READ ) + formNativeNonTroubledClause( "ee2ad.EXPRESSION_EXPERIMENT_FK", ExpressionExperiment.class ) + " group by ee2ad.ARRAY_DESIGN_FK" ) .addScalar( "ID", StandardBasicTypes.LONG ) @@ -1178,7 +1184,7 @@ private void populateSwitchedExpressionExperimentCount( Collection switchedCountById = QueryUtils.streamByBatch( query, "ids", IdentifiableUtils.getIds( entities ), 2048 ) .collect( Collectors.toMap( row -> ( Long ) row[0], row -> ( Long ) row[1] ) ); for ( ArrayDesignValueObject vo : entities ) { diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/designElement/CompositeSequenceDaoImpl.java b/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/designElement/CompositeSequenceDaoImpl.java index f3476362be..1071c4ba7a 100644 --- a/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/designElement/CompositeSequenceDaoImpl.java +++ b/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/designElement/CompositeSequenceDaoImpl.java @@ -99,7 +99,7 @@ protected Query getFilteringQuery( @Nullable Filters filters, @Nullable Sort sor + ( !SecurityUtil.isUserAdmin() ? " and ad.curationDetails.troubled = false" : "" ) + FilterQueryUtils.formRestrictionClause( filters ) + FilterQueryUtils.formOrderByClause( sort ) ); - AclQueryUtils.addAclParameters( query, ArrayDesign.class ); + AclQueryUtils.setAclParameters( query, ArrayDesign.class ); FilterQueryUtils.addRestrictionParameters( query, filters ); return query; } @@ -119,7 +119,7 @@ protected Query getFilteringIdQuery( @Nullable Filters filters, @Nullable Sort s + ( !SecurityUtil.isUserAdmin() ? " and ad.curationDetails.troubled = false" : "" ) + FilterQueryUtils.formRestrictionClause( filters ) + FilterQueryUtils.formOrderByClause( sort ) ); - AclQueryUtils.addAclParameters( query, ArrayDesign.class ); + AclQueryUtils.setAclParameters( query, ArrayDesign.class ); FilterQueryUtils.addRestrictionParameters( query, filters ); return query; } @@ -132,7 +132,7 @@ protected Query getFilteringCountQuery( @Nullable Filters filters ) { + AclQueryUtils.formAclRestrictionClause( "ad.id" ) + ( !SecurityUtil.isUserAdmin() ? " and ad.curationDetails.troubled = false" : "" ) + FilterQueryUtils.formRestrictionClause( filters ) ); - AclQueryUtils.addAclParameters( query, ArrayDesign.class ); + AclQueryUtils.setAclParameters( query, ArrayDesign.class ); FilterQueryUtils.addRestrictionParameters( query, filters ); return query; } diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/ExpressionExperimentDaoImpl.java b/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/ExpressionExperimentDaoImpl.java index 5a72437049..ece0492db7 100644 --- a/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/ExpressionExperimentDaoImpl.java +++ b/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/ExpressionExperimentDaoImpl.java @@ -35,6 +35,7 @@ import org.hibernate.type.StandardBasicTypes; import org.hibernate.type.Type; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.acls.domain.BasePermission; import org.springframework.stereotype.Repository; import org.springframework.util.Assert; import ubic.gemma.core.analysis.singleCell.SingleCellMaskUtils; @@ -174,7 +175,7 @@ public List loadAllIdentifiers() { .createQuery( "select ee.id as id, ee.shortName as shortName, ee.name as name, accession.accession as accession from ExpressionExperiment ee " + "left join ee.accession accession " + AclQueryUtils.formAclRestrictionClause( "ee.id" ) ); - AclQueryUtils.addAclParameters( query, ExpressionExperiment.class ); + AclQueryUtils.setAclParameters( query, ExpressionExperiment.class ); //noinspection unchecked return query.setResultTransformer( aliasToBean( Identifiers.class ) ).list(); } @@ -828,7 +829,7 @@ public Map getCategoriesUsageFrequency( @Nullable Collecti } String query = "select T.CATEGORY as CATEGORY, T.CATEGORY_URI as CATEGORY_URI, count(distinct T.EXPRESSION_EXPERIMENT_FK) as EE_COUNT from EXPRESSION_EXPERIMENT2CHARACTERISTIC T "; if ( doAclFiltering ) { - query += EE2CAclQueryUtils.formNativeAclJoinClause( "T.EXPRESSION_EXPERIMENT_FK" ) + " "; + query += AclDenormalizedTableQueryUtils.formNativeAclJoinClause( "T.EXPRESSION_EXPERIMENT_FK", "IS_AUTHENTICATED_ANONYMOUSLY" ) + " "; } if ( eeIds != null ) { query += "where T.EXPRESSION_EXPERIMENT_FK in :eeIds"; @@ -846,7 +847,9 @@ public Map getCategoriesUsageFrequency( @Nullable Collecti query += ")"; } if ( doAclFiltering ) { - query += EE2CAclQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), "T.ACL_IS_AUTHENTICATED_ANONYMOUSLY_MASK" ); + query += AclDenormalizedTableQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), + "IS_AUTHENTICATED_ANONYMOUSLY", "T." + EE2C_IS_AUTHENTICATED_ANONYMOUSLY_MASK_COLUMN, + BasePermission.READ ); // troubled filtering query += formNativeNonTroubledClause( "T.EXPRESSION_EXPERIMENT_FK", ExpressionExperiment.class ); } @@ -871,7 +874,7 @@ public Map getCategoriesUsageFrequency( @Nullable Collecti q.setParameterList( "retainedTermUris", optimizeParameterList( retainedTermUris ) ); } if ( doAclFiltering ) { - EE2CAclQueryUtils.addAclParameters( q, ExpressionExperiment.class ); + AclDenormalizedTableQueryUtils.setAclParameters( q, "IS_AUTHENTICATED_ANONYMOUSLY", ExpressionExperiment.class ); } q.setCacheable( true ); List result; @@ -990,7 +993,7 @@ public Map getAnnotationsUsageFrequencyInternal( } String query = "select T." + valueColumn + " as `VALUE`, T." + valueUriColumn + " as VALUE_URI, " + ( isCategorized ? "T.CATEGORY" : "NULL" ) + " as CATEGORY, " + ( isCategorized ? "T.CATEGORY_URI" : "NULL" ) + " as CATEGORY_URI, T.EVIDENCE_CODE as EVIDENCE_CODE, count(distinct T.EXPRESSION_EXPERIMENT_FK) as EE_COUNT from EXPRESSION_EXPERIMENT2CHARACTERISTIC T "; if ( doAclFiltering ) { - query += EE2CAclQueryUtils.formNativeAclJoinClause( "T.EXPRESSION_EXPERIMENT_FK" ) + " "; + query += AclDenormalizedTableQueryUtils.formNativeAclJoinClause( "T.EXPRESSION_EXPERIMENT_FK", "IS_AUTHENTICATED_ANONYMOUSLY" ) + " "; } if ( eeIds != null ) { query += "where T.EXPRESSION_EXPERIMENT_FK in :eeIds"; @@ -1029,7 +1032,8 @@ else if ( category.startsWith( "http://" ) ) { query += ")"; } if ( doAclFiltering ) { - query += EE2CAclQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), "T.ACL_IS_AUTHENTICATED_ANONYMOUSLY_MASK" ); + query += AclDenormalizedTableQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), + "IS_AUTHENTICATED_ANONYMOUSLY", "T." + EE2C_IS_AUTHENTICATED_ANONYMOUSLY_MASK_COLUMN, BasePermission.READ ); query += formNativeNonTroubledClause( "T.EXPRESSION_EXPERIMENT_FK", ExpressionExperiment.class ); } //language=HQL @@ -1079,7 +1083,7 @@ else if ( category.startsWith( "http://" ) ) { q.setParameter( "minFrequency", minFrequency ); } if ( doAclFiltering ) { - EE2CAclQueryUtils.addAclParameters( q, ExpressionExperiment.class ); + AclDenormalizedTableQueryUtils.setAclParameters( q, "IS_AUTHENTICATED_ANONYMOUSLY", ExpressionExperiment.class ); } q.setCacheable( true ); List result; @@ -1244,9 +1248,10 @@ public Map getTechnologyTypeUsageFrequency() { Query q = getSessionFactory().getCurrentSession() .createSQLQuery( "select AD.TECHNOLOGY_TYPE as TT, count(distinct EE2AD.EXPRESSION_EXPERIMENT_FK) as EE_COUNT from EXPRESSION_EXPERIMENT2ARRAY_DESIGN EE2AD " + "join ARRAY_DESIGN AD on EE2AD.ARRAY_DESIGN_FK = AD.ID " - + EE2CAclQueryUtils.formNativeAclJoinClause( "EE2AD.EXPRESSION_EXPERIMENT_FK" ) + " " + + AclDenormalizedTableQueryUtils.formNativeAclJoinClause( "EE2AD.EXPRESSION_EXPERIMENT_FK", "IS_AUTHENTICATED_ANONYMOUSLY" ) + " " + "where EE2AD.EXPRESSION_EXPERIMENT_FK is not NULL" - + EE2CAclQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), "EE2AD.ACL_IS_AUTHENTICATED_ANONYMOUSLY_MASK" ) + + AclDenormalizedTableQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), + "IS_AUTHENTICATED_ANONYMOUSLY", "EE2AD." + EE2AD_IS_AUTHENTICATED_ANONYMOUSLY_MASK_COLUMN, BasePermission.READ ) + formNativeNonTroubledClause( "EE2AD.ARRAY_DESIGN_FK", ArrayDesign.class ) + formNativeNonTroubledClause( "EE2AD.EXPRESSION_EXPERIMENT_FK", ExpressionExperiment.class ) + " group by AD.TECHNOLOGY_TYPE" ) @@ -1256,7 +1261,7 @@ public Map getTechnologyTypeUsageFrequency() { .addSynchronizedEntityClass( ExpressionExperiment.class ) .addSynchronizedEntityClass( ArrayDesign.class ) .setCacheable( true ); - EE2CAclQueryUtils.addAclParameters( q, ExpressionExperiment.class ); + AclDenormalizedTableQueryUtils.setAclParameters( q, "IS_AUTHENTICATED_ANONYMOUSLY", ExpressionExperiment.class ); //noinspection unchecked List results = q.list(); return results.stream().collect( Collectors.groupingBy( row -> TechnologyType.valueOf( ( String ) row[0] ), Collectors.summingLong( row -> ( Long ) row[1] ) ) ); @@ -1306,11 +1311,12 @@ private Map getPlatformsUsageFrequency( boolean original, int Query query = getSessionFactory().getCurrentSession() .createSQLQuery( "select ad.*, count(distinct ee2ad.EXPRESSION_EXPERIMENT_FK) EE_COUNT from EXPRESSION_EXPERIMENT2ARRAY_DESIGN ee2ad " + "join ARRAY_DESIGN ad on ee2ad.ARRAY_DESIGN_FK = ad.ID " - + EE2CAclQueryUtils.formNativeAclJoinClause( "ee2ad.EXPRESSION_EXPERIMENT_FK" ) + " " + + AclDenormalizedTableQueryUtils.formNativeAclJoinClause( "ee2ad.EXPRESSION_EXPERIMENT_FK", "IS_AUTHENTICATED_ANONYMOUSLY" ) + " " + "where ee2ad.IS_ORIGINAL_PLATFORM = :original" // exclude noop switch + ( original ? " and ee2ad.ARRAY_DESIGN_FK not in (select ARRAY_DESIGN_FK from EXPRESSION_EXPERIMENT2ARRAY_DESIGN where EXPRESSION_EXPERIMENT_FK = ee2ad.EXPRESSION_EXPERIMENT_FK and ARRAY_DESIGN_FK = ee2ad.ARRAY_DESIGN_FK and not IS_ORIGINAL_PLATFORM)" : "" ) - + EE2CAclQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), "ee2ad.ACL_IS_AUTHENTICATED_ANONYMOUSLY_MASK" ) + " " + + AclDenormalizedTableQueryUtils.formNativeAclRestrictionClause( ( SessionFactoryImplementor ) getSessionFactory(), + "IS_AUTHENTICATED_ANONYMOUSLY", "ee2ad." + EE2AD_IS_AUTHENTICATED_ANONYMOUSLY_MASK_COLUMN, BasePermission.READ ) + " " // exclude troubled platforms or experiments for non-admins + formNativeNonTroubledClause( "ee2ad.ARRAY_DESIGN_FK", ArrayDesign.class ) + formNativeNonTroubledClause( "ee2ad.EXPRESSION_EXPERIMENT_FK", ExpressionExperiment.class ) @@ -1325,8 +1331,8 @@ private Map getPlatformsUsageFrequency( boolean original, int .addSynchronizedEntityClass( ExpressionExperiment.class ) .addSynchronizedEntityClass( ArrayDesign.class ); query.setParameter( "original", original ); - EE2CAclQueryUtils.addAclParameters( query, ExpressionExperiment.class ); - EE2CAclQueryUtils.addAclParameters( query, ExpressionExperiment.class ); + AclDenormalizedTableQueryUtils.setAclParameters( query, "IS_AUTHENTICATED_ANONYMOUSLY", ExpressionExperiment.class ); + AclDenormalizedTableQueryUtils.setAclParameters( query, "IS_AUTHENTICATED_ANONYMOUSLY", ExpressionExperiment.class ); query.setCacheable( true ); List result; //noinspection unchecked @@ -1596,7 +1602,7 @@ public Map getPerTaxonCount() { Query query = this.getSessionFactory().getCurrentSession().createQuery( queryString ); - AclQueryUtils.addAclParameters( query, ExpressionExperiment.class ); + AclQueryUtils.setAclParameters( query, ExpressionExperiment.class ); // it is important to cache this, as it gets called on the home page. Though it's actually fast. //noinspection unchecked @@ -3868,7 +3874,7 @@ private Query finishFilteringQuery( String queryString, @Nullable Filters filter Query query = this.getSessionFactory().getCurrentSession().createQuery( queryString ); - AclQueryUtils.addAclParameters( query, ExpressionExperiment.class ); + AclQueryUtils.setAclParameters( query, ExpressionExperiment.class ); FilterQueryUtils.addRestrictionParameters( query, filters ); return query; diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/FactorValueDaoImpl.java b/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/FactorValueDaoImpl.java index a52cc48909..e9c2393272 100644 --- a/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/FactorValueDaoImpl.java +++ b/gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/FactorValueDaoImpl.java @@ -82,7 +82,7 @@ public Slice loadAll( int offset, int limit ) { + AclQueryUtils.formAclRestrictionClause( "ee.id" ) + " " + "and ee.experimentalDesign = ed " + ( AclQueryUtils.requiresGroupBy() ? " group by fv" : "" ) ); - AclQueryUtils.addAclParameters( query, ExpressionExperiment.class ); + AclQueryUtils.setAclParameters( query, ExpressionExperiment.class ); //noinspection unchecked return new Slice<>( ( List ) query .setFirstResult( offset ) @@ -97,7 +97,7 @@ public Collection loadAllIds() { + AclQueryUtils.formAclRestrictionClause( "ee.id" ) + " " + "and ee.experimentalDesign = ed " + ( AclQueryUtils.requiresGroupBy() ? " group by fv" : "" ) ); - AclQueryUtils.addAclParameters( query, ExpressionExperiment.class ); + AclQueryUtils.setAclParameters( query, ExpressionExperiment.class ); //noinspection unchecked return query.list(); } @@ -109,7 +109,7 @@ public Slice loadAllIds( int offset, int limit ) { + AclQueryUtils.formAclRestrictionClause( "ee.id" ) + " " + "and ee.experimentalDesign = ed" + ( AclQueryUtils.requiresGroupBy() ? " group by fv" : "" ) ); - AclQueryUtils.addAclParameters( query, ExpressionExperiment.class ); + AclQueryUtils.setAclParameters( query, ExpressionExperiment.class ); //noinspection unchecked return new Slice<>( ( List ) query .setFirstResult( offset ) @@ -123,7 +123,7 @@ private long countAllWithAcls() { + "join fv.experimentalFactor ef join ef.experimentalDesign ed, ExpressionExperiment ee " + AclQueryUtils.formAclRestrictionClause( "ee.id" ) + " " + "and ee.experimentalDesign = ed" ); - AclQueryUtils.addAclParameters( countQuery, ExpressionExperiment.class ); + AclQueryUtils.setAclParameters( countQuery, ExpressionExperiment.class ); return ( Long ) countQuery.uniqueResult(); } @@ -135,7 +135,7 @@ public Collection findByValueStartingWith( String valuePrefix, int + "and ee.experimentalDesign = ed " + "and fv.value like :q" + ( AclQueryUtils.requiresGroupBy() ? " group by fv" : "" ) ); - AclQueryUtils.addAclParameters( query, ExpressionExperiment.class ); + AclQueryUtils.setAclParameters( query, ExpressionExperiment.class ); //noinspection unchecked return query .setParameter( "q", escapeLike( valuePrefix ) + "%" ) diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/service/genome/GeneDaoImpl.java b/gemma-core/src/main/java/ubic/gemma/persistence/service/genome/GeneDaoImpl.java index 5457024011..ac9c449876 100644 --- a/gemma-core/src/main/java/ubic/gemma/persistence/service/genome/GeneDaoImpl.java +++ b/gemma-core/src/main/java/ubic/gemma/persistence/service/genome/GeneDaoImpl.java @@ -228,7 +228,7 @@ public long getCompositeSequenceCount( Gene gene, boolean includeDummyProducts ) + "and gp.gene = :gene" + ( includeDummyProducts ? "" : " and gp.dummy = false" ) ) .setParameter( "gene", gene ); - AclQueryUtils.addAclParameters( query, ArrayDesign.class ); + AclQueryUtils.setAclParameters( query, ArrayDesign.class ); return ( Long ) query.uniqueResult(); } @@ -247,7 +247,7 @@ public long getCompositeSequenceCountById( long id, boolean includeDummyProducts + "and gp.gene.id = :id" + ( includeDummyProducts ? "" : " and gp.dummy = false" ) ) .setParameter( "id", id ); - AclQueryUtils.addAclParameters( query, ArrayDesign.class ); + AclQueryUtils.setAclParameters( query, ArrayDesign.class ); return ( Long ) query.uniqueResult(); } diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/service/maintenance/TableMaintenanceUtil.java b/gemma-core/src/main/java/ubic/gemma/persistence/service/maintenance/TableMaintenanceUtil.java index 175bb78eca..5d1c300de4 100644 --- a/gemma-core/src/main/java/ubic/gemma/persistence/service/maintenance/TableMaintenanceUtil.java +++ b/gemma-core/src/main/java/ubic/gemma/persistence/service/maintenance/TableMaintenanceUtil.java @@ -53,6 +53,11 @@ public interface TableMaintenanceUtil { */ String EE2C_QUERY_SPACE = "EXPRESSION_EXPERIMENT2CHARACTERISTIC"; + /** + * Column in the EE2C table that contains a permission mask for anonymous users. + */ + String EE2C_IS_AUTHENTICATED_ANONYMOUSLY_MASK_COLUMN = "ACL_IS_AUTHENTICATED_ANONYMOUSLY_MASK"; + /** * Query space used by the {@code EXPRESSION_EXPERIMENT2ARRAY_DESIGN} table. *

@@ -62,6 +67,11 @@ public interface TableMaintenanceUtil { */ String EE2AD_QUERY_SPACE = "EXPRESSION_EXPERIMENT2_ARRAY_DESIGN"; + /** + * Column in the EE2AD table that contains a permission mask for anonymous users. + */ + String EE2AD_IS_AUTHENTICATED_ANONYMOUSLY_MASK_COLUMN = "ACL_IS_AUTHENTICATED_ANONYMOUSLY_MASK"; + /** * If necessary, update the GENE2CS table. */ diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/util/AclDenormalizedTableQueryUtils.java b/gemma-core/src/main/java/ubic/gemma/persistence/util/AclDenormalizedTableQueryUtils.java new file mode 100644 index 0000000000..e25768c5fe --- /dev/null +++ b/gemma-core/src/main/java/ubic/gemma/persistence/util/AclDenormalizedTableQueryUtils.java @@ -0,0 +1,102 @@ +package ubic.gemma.persistence.util; + +import gemma.gsec.util.SecurityUtil; +import org.apache.commons.collections4.CollectionUtils; +import org.apache.commons.lang3.StringUtils; +import org.hibernate.Query; +import org.hibernate.dialect.function.SQLFunction; +import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.type.IntegerType; +import org.springframework.security.acls.model.Permission; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.util.Assert; +import ubic.gemma.model.common.auditAndSecurity.Securable; +import ubic.gemma.model.common.auditAndSecurity.SecuredChild; + +import java.util.Arrays; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * This class provides a fast-path to {@link AclQueryUtils} that uses denormalized permission masks for a specified + * granted authority. + * @author poirigui + */ +public class AclDenormalizedTableQueryUtils { + + /** + * Form a native ACL join clause for the given AOI ID column and granted authorities. + *

+ * The fast path is only taken if the user is an administrator or if it has at least one of the granted authorities + * that are relevant to the query. + * + * @see AclQueryUtils#formNativeAclJoinClause(String) + */ + public static String formNativeAclJoinClause( String aoiIdColumn, String grantedAuthority ) { + Assert.isTrue( StringUtils.isNotBlank( aoiIdColumn ), "The ACL object identity column cannot be empty." ); + Assert.isTrue( StringUtils.isNotBlank( grantedAuthority ), "The granted authority cannot be empty." ); + if ( SecurityUtil.isUserAdmin() || CollectionUtils.containsAny( getGrantedAuthorities(), grantedAuthority ) ) { + // nothing to do, we're in the fast path + return ""; + } else { + return AclQueryUtils.formNativeAclJoinClause( aoiIdColumn ); + } + } + + /** + * Form a native ACL restriction clause. + * @param sessionFactoryImplementor a session factory, used for generating queries + * @param grantedAuthority a granted authority relevant to the query + * @param grantedAuthorityPermissionMaskAlias the corresponding permission mask alias in the query + * @param permission a requested permission + */ + public static String formNativeAclRestrictionClause( SessionFactoryImplementor sessionFactoryImplementor, String grantedAuthority, String grantedAuthorityPermissionMaskAlias, Permission permission ) { + Assert.isTrue( StringUtils.isNotBlank( grantedAuthority ), "The granted authority cannot be empty." ); + Assert.isTrue( StringUtils.isNotBlank( grantedAuthorityPermissionMaskAlias ), "The granted authority permission mask alias must not be empty." ); + Assert.isTrue( permission.getMask() > 0, "At least one bit must be set in the permission mask." ); + if ( SecurityUtil.isUserAdmin() ) { + return ""; + } else { + if ( !getGrantedAuthorities().contains( grantedAuthority ) ) { + SQLFunction bitwiseAnd = sessionFactoryImplementor.getSqlFunctionRegistry().findSQLFunction( "bitwise_and" ); + // user has at least one relevant authority, use it to check permissions + return " and " + "(" + bitwiseAnd.render( new IntegerType(), Arrays.asList( grantedAuthorityPermissionMaskAlias, permission.getMask() ), sessionFactoryImplementor ) + " <> 0)"; + } else { + // user has no relevant authorities, default to the inefficient ACL check + return AclQueryUtils.formNativeAclRestrictionClause( sessionFactoryImplementor, permission ); + } + } + } + + /** + * Add ACL parameters to the query with an ACL clause for the given granted authorities by {@link #formNativeAclJoinClause(String, String)}. + * + * @param query a query on which ACL parameters should be set + * @param aoiType the type of secured object being queried + * @see AclQueryUtils#setAclParameters(Query, Class) + */ + public static void setAclParameters( Query query, String grantedAuthority, Class aoiType ) { + Assert.isTrue( StringUtils.isNotBlank( grantedAuthority ), "The granted authority cannot be empty." ); + Assert.isTrue( !SecuredChild.class.isAssignableFrom( aoiType ), + "ACL filtering cannot be done on a SecuredChild; instead identify the owner and apply ACLs on it." ); + //noinspection StatementWithEmptyBody + if ( SecurityUtil.isUserAdmin() || getGrantedAuthorities().contains( grantedAuthority ) ) { + // nothing to do, we're in the fast path + } else { + AclQueryUtils.setAclParameters( query, aoiType ); + } + } + + /** + * Obtain a set of granted authorities for the current user. + */ + private static Set getGrantedAuthorities() { + return SecurityContextHolder.getContext() + .getAuthentication() + .getAuthorities() + .stream() + .map( GrantedAuthority::getAuthority ) + .collect( Collectors.toSet() ); + } +} diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/util/AclQueryUtils.java b/gemma-core/src/main/java/ubic/gemma/persistence/util/AclQueryUtils.java index 87dd1ff86a..687e39e57d 100644 --- a/gemma-core/src/main/java/ubic/gemma/persistence/util/AclQueryUtils.java +++ b/gemma-core/src/main/java/ubic/gemma/persistence/util/AclQueryUtils.java @@ -25,7 +25,7 @@ *

  • form where clause and add your constraints
    • *
  • concatenate {@link #formNativeAclRestrictionClause(SessionFactoryImplementor)} in the clause section (only for native queries)
    • *
  • bind all your parameters
    • - *
  • bind ACL-specific parameters with {@link #addAclParameters(Query, Class)} to the query object
    • + *
  • bind ACL-specific parameters with {@link #setAclParameters(Query, Class)} to the query object
    • * * * @author poirigui @@ -83,7 +83,8 @@ public class AclQueryUtils { /** * Indicate if the ACL query requires a {@code count(distinct ...)} clause. *

    - * FIXME: remove the need for a distinct altogether by using a sub-query to apply ACLs (see #784) + * This is only applicable to {@link #formAclRestrictionClause(String, Permission)} and {@link #formNativeAclRestrictionClause(SessionFactoryImplementor, Permission)}, + * you do not need to use distinct clause when using a sub-query. */ public static boolean requiresCountDistinct() { return !SecurityUtil.isUserAdmin(); @@ -92,7 +93,8 @@ public static boolean requiresCountDistinct() { /** * Indicate if the ACL query requires a {@code group by} clause. *

    - * FIXME: remove the need for a count distinct altogether by using a sub-query to apply ACLs (see #784) + * This is only applicable to {@link #formAclRestrictionClause(String, Permission)} and {@link #formNativeAclRestrictionClause(SessionFactoryImplementor, Permission)}, + * you do not need to use a group by clause when using a sub-query. */ public static boolean requiresGroupBy() { return !SecurityUtil.isUserAdmin(); @@ -110,7 +112,7 @@ public static String formAclRestrictionClause( String aoiIdColumn ) { * Create an HQL join clause for {@link gemma.gsec.acl.domain.AclObjectIdentity}, {@link gemma.gsec.acl.domain.AclGrantedAuthoritySid} * and a restriction clause to limit the result only to objects the current user can access. *

    - * Ensure that you use {@link #addAclParameters(Query, Class)} afterward to bind the query parameters. + * Ensure that you use {@link #setAclParameters(Query, Class)} afterward to bind the query parameters. *

    * Important note: when using this, ensure that you have a {@code group by} clause in your query, otherwise * entities with multiple ACL entries will be duplicated in the results. You can use {@link #requiresGroupBy()} to @@ -120,15 +122,13 @@ public static String formAclRestrictionClause( String aoiIdColumn ) { * would be preferable? * * @param aoiIdColumn column name to match against the ACL object identity, the object class is passed via - * {@link #addAclParameters(Query, Class)} afterward + * {@link #setAclParameters(Query, Class)} afterward * @param permission requested permission(s) * @return clause to add to the query after any jointure */ public static String formAclRestrictionClause( String aoiIdColumn, Permission permission ) { - if ( StringUtils.isBlank( aoiIdColumn ) ) { - throw new IllegalArgumentException( "Object identity column cannot be empty." ); - } - Assert.isTrue( permission.getMask() > 0, "The mask must have at least one bit set." ); + Assert.isTrue( permission.getMask() > 0, "At least one bit must be set in the permission mask." ); + Assert.isTrue( StringUtils.isNotBlank( aoiIdColumn ), "Object identity column cannot be empty." ); //language=HQL String q = ", AclObjectIdentity as " + AOI_ALIAS + " join " + AOI_ALIAS + ".ownerSid " + SID_ALIAS; // for non-admin, we have to include aoi.entries @@ -138,22 +138,33 @@ public static String formAclRestrictionClause( String aoiIdColumn, Permission pe } q += " where (" + AOI_ALIAS + ".identifier = " + aoiIdColumn + " and " + AOI_ALIAS + ".type = :" + AOI_TYPE_PARAM + ")"; // add ACL restrictions - if ( !SecurityUtil.isUserAdmin() ) { - if ( SecurityUtil.isUserAnonymous() ) { - //language=HQL - q += " and (bitwise_and(" + ACE_ALIAS + ".mask, " + permission.getMask() + ") <> 0 and " + ACE_ALIAS + ".sid in (" + ANONYMOUS_SID_HQL + "))"; - } else { - q += " and (" - // user own the object - + SID_ALIAS + ".principal = :" + USER_NAME_PARAM + " " - // specific rights to the object - + "or (" + ACE_ALIAS + ".sid in (" + CURRENT_USER_SIDS_HQL + ") and bitwise_and(" + ACE_ALIAS + ".mask, " + permission.getMask() + ") <> 0) " - // publicly available - + "or (" + ACE_ALIAS + ".sid in (" + ANONYMOUS_SID_HQL + ") and bitwise_and(" + ACE_ALIAS + ".mask, " + permission.getMask() + ") <> 0)" - + ")"; - } + if ( SecurityUtil.isUserAnonymous() ) { + //language=HQL + return q + " and (bitwise_and(" + ACE_ALIAS + ".mask, " + permission.getMask() + ") <> 0 and " + ACE_ALIAS + ".sid in (" + ANONYMOUS_SID_HQL + "))"; + } else if ( !SecurityUtil.isUserAdmin() ) { + return q + " and (" + // user own the object + + SID_ALIAS + ".principal = :" + USER_NAME_PARAM + " " + // specific rights to the object + + "or (" + ACE_ALIAS + ".sid in (" + CURRENT_USER_SIDS_HQL + ") and bitwise_and(" + ACE_ALIAS + ".mask, " + permission.getMask() + ") <> 0) " + // publicly available + + "or (" + ACE_ALIAS + ".sid in (" + ANONYMOUS_SID_HQL + ") and bitwise_and(" + ACE_ALIAS + ".mask, " + permission.getMask() + ") <> 0)" + + ")"; + } else { + return q; + } + } + + public static String formAclRestrictionSubqueryClause( String aoiIdColumn, Permission permission ) { + Assert.isTrue( StringUtils.isNotBlank( aoiIdColumn ), "ACL object identity column cannot be empty." ); + Assert.isTrue( permission.getMask() > 0, "At least one bit must be set in the permission mask." ); + if ( SecurityUtil.isUserAnonymous() ) { + return " and "; + } else if ( !SecurityUtil.isUserAdmin() ) { + return " and "; + } else { + return ""; } - return q; } /** @@ -165,18 +176,15 @@ public static String formAclRestrictionClause( String aoiIdColumn, Permission pe * Important note: when using this, ensure that you have a {@code group by} clause in your query, otherwise * entities with multiple ACL entries will be duplicated in the results. * @param aoiIdColumn column name to match against the ACL object identity, the object class is passed via - * {@link #addAclParameters(Query, Class)} afterward + * {@link #setAclParameters(Query, Class)} afterward * * @see #formAclRestrictionClause(String) */ public static String formNativeAclJoinClause( String aoiIdColumn ) { - if ( StringUtils.isBlank( aoiIdColumn ) ) { - throw new IllegalArgumentException( "Object identity column cannot be empty." ); - } + Assert.isTrue( StringUtils.isNotBlank( aoiIdColumn ), "ACL object identity ID column cannot be empty." ); //language=SQL String q = " join ACLOBJECTIDENTITY " + AOI_ALIAS + " on (" + AOI_ALIAS + ".OBJECT_CLASS = :" + AOI_TYPE_PARAM + " and " + AOI_ALIAS + ".OBJECT_ID = " + aoiIdColumn + ") " + "join ACLSID " + SID_ALIAS + " on (" + SID_ALIAS + ".ID = " + AOI_ALIAS + ".OWNER_SID_FK)"; - // for non-admin, we have to include aoi.entries // if aoi.entries is empty, the user might still be the owner, so we use a left join if ( !SecurityUtil.isUserAdmin() ) { @@ -201,6 +209,7 @@ public static String formNativeAclRestrictionClause( SessionFactoryImplementor s * @see #formAclRestrictionClause(String, Permission) */ public static String formNativeAclRestrictionClause( SessionFactoryImplementor sessionFactoryImplementor, Permission permission ) { + Assert.isTrue( permission.getMask() > 0, "At least one bit must be set in the permission mask." ); SQLFunction bitwiseAnd = sessionFactoryImplementor.getSqlFunctionRegistry().findSQLFunction( "bitwise_and" ); String renderedMask = bitwiseAnd.render( new IntegerType(), Arrays.asList( ACE_ALIAS + ".MASK", permission.getMask() ), sessionFactoryImplementor ); //language=SQL @@ -221,6 +230,31 @@ public static String formNativeAclRestrictionClause( SessionFactoryImplementor s } } + /** + * Native flavour of the ACL restriction subquery clause. + * + * @see #formAclRestrictionSubqueryClause(String, Permission) + */ + public static String formNativeAclRestrictionSubqueryClause( SessionFactoryImplementor sessionFactoryImplementor, String aoiIdColumn, Permission permission ) { + Assert.isTrue( StringUtils.isNotBlank( aoiIdColumn ), "ACL object identity ID column cannot be empty." ); + Assert.isTrue( permission.getMask() > 0, "At least one bit must be set in the permission mask." ); + SQLFunction bitwiseAnd = sessionFactoryImplementor.getSqlFunctionRegistry().findSQLFunction( "bitwise_and" ); + String renderedMask = bitwiseAnd.render( new IntegerType(), Arrays.asList( ACE_ALIAS + ".MASK", permission.getMask() ), sessionFactoryImplementor ); + //language=SQL + if ( SecurityUtil.isUserAnonymous() ) { + return " and " + aoiIdColumn + " in (select ACLOBJECTIDENTITY.OBJECT_ID from ACLOBJECTIDENTITY aoi " + + "left join ACLENTRY " + ACE_ALIAS + " on aoi.ID = " + ACE_ALIAS + ".OBJECTIDENTITY_FK " + + "where aoi.OBJECT_CLASS = :" + AOI_TYPE_PARAM + " and " + renderedMask + " <> 0))"; + } else if ( !SecurityUtil.isUserAdmin() ) { + return " and " + aoiIdColumn + " in (select ACLOBJECTIDENTITY.OBJECT_ID from ACLOBJECTIDENTITY aoi " + + "left join ACLENTRY " + ACE_ALIAS + " on aoi.ID = " + ACE_ALIAS + ".OBJECTIDENTITY_FK " + + "where aoi.OBJECT_CLASS = :" + AOI_TYPE_PARAM + " and " + renderedMask + " <> 0))"; + } else { + // For administrators, no filtering is needed, so the ACE is completely skipped from the where clause. + return ""; + } + } + /** * Bind {@link Query} parameters to a join clause generated with {@link #formAclRestrictionClause(String)} and add ACL * restriction parameters defined in {@link #formAclRestrictionClause(String)}. @@ -234,10 +268,9 @@ public static String formNativeAclRestrictionClause( SessionFactoryImplementor s * {@link #formAclRestrictionClause(String)}. */ @SuppressWarnings("StatementWithEmptyBody") - public static void addAclParameters( Query query, Class aoiType ) throws QueryParameterException { - if ( SecuredChild.class.isAssignableFrom( aoiType ) ) { - throw new IllegalArgumentException( "ACL filtering cannot be done on a SecuredChild; instead identify the owner and apply ACLs on it." ); - } + public static Query setAclParameters( Query query, Class aoiType ) throws QueryParameterException { + Assert.isTrue( !SecuredChild.class.isAssignableFrom( aoiType ), + "ACL filtering cannot be done on a SecuredChild; instead identify the owner and apply ACLs on it." ); query.setParameter( AOI_TYPE_PARAM, aoiType.getCanonicalName() ); if ( SecurityUtil.isUserAnonymous() ) { // a constant is used directly in ANONYMOUS_SID_SQL, so no binding is necessary @@ -246,5 +279,6 @@ public static void addAclParameters( Query query, Class aoi } else { // For administrators, no filtering is needed, so the ACE is completely skipped from the where clause. } + return query; } } diff --git a/gemma-core/src/main/java/ubic/gemma/persistence/util/EE2CAclQueryUtils.java b/gemma-core/src/main/java/ubic/gemma/persistence/util/EE2CAclQueryUtils.java deleted file mode 100644 index b03dba9552..0000000000 --- a/gemma-core/src/main/java/ubic/gemma/persistence/util/EE2CAclQueryUtils.java +++ /dev/null @@ -1,50 +0,0 @@ -package ubic.gemma.persistence.util; - -import gemma.gsec.util.SecurityUtil; -import org.hibernate.Query; -import org.hibernate.dialect.function.SQLFunction; -import org.hibernate.engine.spi.SessionFactoryImplementor; -import org.hibernate.type.IntegerType; -import org.springframework.security.acls.domain.BasePermission; -import org.springframework.security.acls.model.Permission; -import ubic.gemma.model.common.auditAndSecurity.Securable; - -import java.util.Arrays; - -/** - * This class provides a fast-path to {@link AclQueryUtils} that uses the denormalized mask for anonymous users. - * @author poirigui - */ -public class EE2CAclQueryUtils { - - public static String formNativeAclJoinClause( String aoiIdColumn ) { - // ACLs are only necessary for regular, non-admin users - if ( SecurityUtil.isUserAnonymous() || SecurityUtil.isUserAdmin() ) { - return ""; - } else { - return AclQueryUtils.formNativeAclJoinClause( aoiIdColumn ); - } - } - - public static String formNativeAclRestrictionClause( SessionFactoryImplementor sessionFactoryImplementor, String anonymousMaskColumn ) { - return formNativeAclRestrictionClause( sessionFactoryImplementor, anonymousMaskColumn, BasePermission.READ ); - } - - public static String formNativeAclRestrictionClause( SessionFactoryImplementor sessionFactoryImplementor, String anonymousMaskColumn, Permission permission ) { - if ( SecurityUtil.isUserAnonymous() ) { - SQLFunction bitwiseAnd = sessionFactoryImplementor.getSqlFunctionRegistry().findSQLFunction( "bitwise_and" ); - String renderedMask = bitwiseAnd.render( new IntegerType(), Arrays.asList( anonymousMaskColumn, permission.getMask() ), sessionFactoryImplementor ); - return " and " + renderedMask + " <> 0"; - } else if ( SecurityUtil.isUserAdmin() ) { - return ""; - } else { - return AclQueryUtils.formNativeAclRestrictionClause( sessionFactoryImplementor, permission ); - } - } - - public static void addAclParameters( Query query, Class aoiType ) { - if ( !SecurityUtil.isUserAdmin() && !SecurityUtil.isUserAnonymous() ) { - AclQueryUtils.addAclParameters( query, aoiType ); - } - } -} diff --git a/gemma-core/src/test/java/ubic/gemma/persistence/util/AclQueryUtilsTest.java b/gemma-core/src/test/java/ubic/gemma/persistence/util/AclQueryUtilsTest.java index 36c83916bb..c715676c97 100644 --- a/gemma-core/src/test/java/ubic/gemma/persistence/util/AclQueryUtilsTest.java +++ b/gemma-core/src/test/java/ubic/gemma/persistence/util/AclQueryUtilsTest.java @@ -71,7 +71,7 @@ public void testFormAclJoinClauseAsNonAdminIncludesAoiEntriesInnerJointure() { @Test public void testAddAclJoinParameters() { Query query = mock( Query.class ); - addAclParameters( query, ExpressionExperiment.class ); + setAclParameters( query, ExpressionExperiment.class ); verify( query ).setParameter( "aclQueryUtils_aoiType", "ubic.gemma.model.expression.experiment.ExpressionExperiment" ); } @@ -116,7 +116,7 @@ public void testAsAdmin() { Query q = session.createQuery( "select ee from ExpressionExperiment ee" + formAclRestrictionClause( "ee.id" ) ); - addAclParameters( q, ExpressionExperiment.class ); + setAclParameters( q, ExpressionExperiment.class ); q.setMaxResults( 1 ); q.list(); } @@ -127,7 +127,7 @@ public void testAsUser() { Query q = session.createQuery( "select ee from ExpressionExperiment ee" + formAclRestrictionClause( "ee.id" ) ); - addAclParameters( q, ExpressionExperiment.class ); + setAclParameters( q, ExpressionExperiment.class ); q.setMaxResults( 1 ); q.list(); } @@ -138,7 +138,7 @@ public void testAsAnonymous() { Query q = session.createQuery( "select ee from ExpressionExperiment ee" + formAclRestrictionClause( "ee.id" ) ); - addAclParameters( q, ExpressionExperiment.class ); + setAclParameters( q, ExpressionExperiment.class ); q.setMaxResults( 1 ); q.list(); } @@ -151,7 +151,7 @@ public void testNative() { + "where {I}.class = 'ExpressionExperiment'" + formNativeAclRestrictionClause( ( SessionFactoryImplementor ) sessionFactory ) ) .addEntity( "I", ExpressionExperiment.class ); - addAclParameters( q, ExpressionExperiment.class ); + setAclParameters( q, ExpressionExperiment.class ); q.setMaxResults( 1 ); q.list(); } @@ -165,7 +165,7 @@ public void testNativeAsUser() { + "where {I}.class = 'ExpressionExperiment'" + formNativeAclRestrictionClause( ( SessionFactoryImplementor ) sessionFactory ) ) .addEntity( "I", ExpressionExperiment.class ); - addAclParameters( q, ExpressionExperiment.class ); + setAclParameters( q, ExpressionExperiment.class ); q.setMaxResults( 1 ); q.list(); } @@ -179,7 +179,7 @@ public void testNativeAsAnonymous() { + "where {I}.class = 'ExpressionExperiment'" + formNativeAclRestrictionClause( ( SessionFactoryImplementor ) sessionFactory ) ) .addEntity( "I", ExpressionExperiment.class ); - addAclParameters( q, ExpressionExperiment.class ); + setAclParameters( q, ExpressionExperiment.class ); q.setMaxResults( 1 ); q.list(); }