Skip to content

Commit 90ecb28

Browse files
arteymixarteymix
committed
More improvements and fixes for the ACL linter
Add a strategy for resolving parent ACL identity. The interface will be added to gsec, but we need it now for clarity. Fix Hibernate.getClass() called incorrectly on a class. Support for linting parent of assays and samples, looking up both datasets and subsets. In the latter case, the assay must have the source experiment as parent in the ACL structure.
1 parent 3258c11 commit 90ecb28

8 files changed

Lines changed: 314 additions & 129 deletions

File tree

‎gemma-core/src/main/java/ubic/gemma/core/security/authorization/acl/AclLinterServiceImpl.java‎

Lines changed: 69 additions & 127 deletions
Large diffs are not rendered by default.

‎gemma-core/src/main/java/ubic/gemma/core/security/authorization/acl/ParentIdentityRetrievalStrategy.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package ubic.gemma.core.security.authorization.acl;
2+
3+
import org.springframework.security.acls.model.ObjectIdentity;
4+
5+
import javax.annotation.Nullable;
6+
7+
/**
8+
* Strategy for locating parent ACL identities.
9+
*
10+
* @author poirigui
11+
*/
12+
public interface ParentIdentityRetrievalStrategy {
13+
14+
/**
15+
* Obtain the parent ACL identity for the given ACL identity.
16+
*
17+
* @return the parent ACL identity if it can be determined, null otherwise
18+
*/
19+
@Nullable
20+
ObjectIdentity getParentIdentity( ObjectIdentity aoi );
21+
}

‎gemma-core/src/main/java/ubic/gemma/core/security/authorization/acl/ParentIdentityRetrievalStrategyImpl.java‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
package ubic.gemma.core.security.authorization.acl;
2+
3+
import gemma.gsec.acl.domain.AclObjectIdentity;
4+
import lombok.extern.apachecommons.CommonsLog;
5+
import org.hibernate.Hibernate;
6+
import org.hibernate.SessionFactory;
7+
import org.springframework.beans.factory.annotation.Autowired;
8+
import org.springframework.security.acls.model.ObjectIdentity;
9+
import org.springframework.stereotype.Service;
10+
import org.springframework.transaction.annotation.Transactional;
11+
import ubic.gemma.model.common.auditAndSecurity.Securable;
12+
import ubic.gemma.model.common.auditAndSecurity.SecuredChild;
13+
import ubic.gemma.model.expression.bioAssay.BioAssay;
14+
import ubic.gemma.model.expression.bioAssayData.MeanVarianceRelation;
15+
import ubic.gemma.model.expression.biomaterial.BioMaterial;
16+
import ubic.gemma.model.expression.experiment.ExperimentalDesign;
17+
import ubic.gemma.model.expression.experiment.ExperimentalFactor;
18+
import ubic.gemma.model.expression.experiment.ExpressionExperiment;
19+
import ubic.gemma.model.expression.experiment.FactorValue;
20+
import ubic.gemma.persistence.service.expression.experiment.ExpressionExperimentService;
21+
22+
import java.util.Collection;
23+
24+
/**
25+
* Use domain-specific logic to resolve parent ACL identities.
26+
*
27+
* @author poirigui
28+
*/
29+
@Service
30+
@CommonsLog
31+
public class ParentIdentityRetrievalStrategyImpl implements ParentIdentityRetrievalStrategy {
32+
33+
@Autowired
34+
private SessionFactory sessionFactory;
35+
36+
@Autowired
37+
private ExpressionExperimentService expressionExperimentService;
38+
39+
@Override
40+
@Transactional(readOnly = true)
41+
public ObjectIdentity getParentIdentity( ObjectIdentity aoi ) {
42+
//noinspection unchecked
43+
Class<? extends SecuredChild<?>> clazz = sessionFactory.getClassMetadata( aoi.getType() ).getMappedClass();
44+
Class<? extends Securable> parentType;
45+
Long parentIdentifier;
46+
if ( ExperimentalDesign.class.isAssignableFrom( clazz ) ) {
47+
ExperimentalDesign design = ( ExperimentalDesign ) sessionFactory.getCurrentSession()
48+
.get( ExperimentalDesign.class, aoi.getIdentifier() );
49+
if ( design == null ) {
50+
log.warn( "Could not find " + clazz.getSimpleName() + " with ID " + aoi.getIdentifier() + "." );
51+
return null;
52+
}
53+
ExpressionExperiment ee = expressionExperimentService.findByDesign( design );
54+
parentType = ExpressionExperiment.class;
55+
parentIdentifier = ee != null ? ee.getId() : null;
56+
} else if ( ExperimentalFactor.class.isAssignableFrom( clazz ) ) {
57+
ExperimentalFactor factor = ( ExperimentalFactor ) sessionFactory.getCurrentSession()
58+
.get( ExperimentalFactor.class, aoi.getIdentifier() );
59+
if ( factor == null ) {
60+
log.warn( "Could not find " + clazz.getSimpleName() + " with ID " + aoi.getIdentifier() + "." );
61+
return null;
62+
}
63+
ExpressionExperiment ee = expressionExperimentService.findByFactor( factor );
64+
parentType = ExpressionExperiment.class;
65+
parentIdentifier = ee != null ? ee.getId() : null;
66+
} else if ( FactorValue.class.isAssignableFrom( clazz ) ) {
67+
FactorValue factor = ( FactorValue ) sessionFactory.getCurrentSession()
68+
.get( FactorValue.class, aoi.getIdentifier() );
69+
if ( factor == null ) {
70+
log.warn( "Could not find " + clazz.getSimpleName() + " with ID " + aoi.getIdentifier() + "." );
71+
return null;
72+
}
73+
ExpressionExperiment ee = expressionExperimentService.findByFactorValue( factor );
74+
parentType = ExpressionExperiment.class;
75+
parentIdentifier = ee != null ? ee.getId() : null;
76+
} else if ( BioAssay.class.isAssignableFrom( clazz ) ) {
77+
BioAssay ba = ( BioAssay ) sessionFactory.getCurrentSession()
78+
.get( BioAssay.class, aoi.getIdentifier() );
79+
if ( ba == null ) {
80+
log.warn( "Could not find " + clazz.getSimpleName() + " with ID " + aoi.getIdentifier() + "." );
81+
return null;
82+
}
83+
ExpressionExperiment ee = expressionExperimentService.findByBioAssay( ba, true );
84+
parentType = ExpressionExperiment.class;
85+
parentIdentifier = ee != null ? ee.getId() : null;
86+
} else if ( BioMaterial.class.isAssignableFrom( clazz ) ) {
87+
BioMaterial bm = ( BioMaterial ) sessionFactory.getCurrentSession()
88+
.get( BioMaterial.class, aoi.getIdentifier() );
89+
if ( bm == null ) {
90+
log.warn( "Could not find " + clazz.getSimpleName() + " with ID " + aoi.getIdentifier() + "." );
91+
return null;
92+
}
93+
Collection<ExpressionExperiment> ees = expressionExperimentService.findByBioMaterial( bm, true );
94+
parentType = ExpressionExperiment.class;
95+
if ( ees.size() == 1 ) {
96+
parentIdentifier = ees.iterator().next().getId();
97+
} else if ( ees.size() > 1 ) {
98+
log.warn( "More than one ExpressionExperiment refer to " + bm + ", cannot pick its parent ACL identity." );
99+
parentIdentifier = null;
100+
} else {
101+
log.warn( "Could not find " + clazz.getSimpleName() + " with ID " + aoi.getIdentifier() + "." );
102+
parentIdentifier = null;
103+
}
104+
} else if ( MeanVarianceRelation.class.isAssignableFrom( clazz ) ) {
105+
MeanVarianceRelation mvr = ( MeanVarianceRelation ) sessionFactory.getCurrentSession()
106+
.get( MeanVarianceRelation.class, aoi.getIdentifier() );
107+
if ( mvr == null ) {
108+
log.warn( "Could not find " + clazz.getSimpleName() + " with ID " + aoi.getIdentifier() + "." );
109+
return null;
110+
}
111+
ExpressionExperiment ee = expressionExperimentService.findByMeanVarianceRelation( mvr );
112+
parentType = ExpressionExperiment.class;
113+
parentIdentifier = ee != null ? ee.getId() : null;
114+
} else {
115+
// try automated!
116+
SecuredChild<?> sc = ( SecuredChild<?> ) sessionFactory.getCurrentSession()
117+
.get( clazz, aoi.getIdentifier() );
118+
if ( sc == null ) {
119+
log.warn( "Could not find " + clazz.getSimpleName() + " with ID " + aoi.getIdentifier() + "." );
120+
return null;
121+
}
122+
if ( sc.getSecurityOwner() != null ) {
123+
// this is necessary because we rely on the class name for querying
124+
//noinspection unchecked
125+
parentType = Hibernate.getClass( sc.getSecurityOwner() );
126+
parentIdentifier = sc.getSecurityOwner().getId();
127+
} else {
128+
log.warn( String.format( "Cannot resolve parent ACL identity for %s Id=%s: its security owner is not populated.",
129+
clazz.getSimpleName(), aoi.getIdentifier() ) );
130+
parentType = null;
131+
parentIdentifier = null;
132+
}
133+
}
134+
135+
if ( parentIdentifier != null ) {
136+
return ( AclObjectIdentity ) sessionFactory.getCurrentSession()
137+
.createQuery( "select aoi from AclObjectIdentity aoi where aoi.type = :type and aoi.identifier = :identifier" )
138+
.setParameter( "type", parentType.getName() )
139+
.setParameter( "identifier", parentIdentifier )
140+
.uniqueResult();
141+
} else {
142+
log.warn( String.format( "Could not locate parent identifier for %s Id=%s.", clazz.getSimpleName(), aoi.getIdentifier() ) );
143+
return null;
144+
}
145+
}
146+
}

‎gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/ExpressionExperimentDao.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,13 @@ class Identifiers {
9797
@Nullable
9898
ExpressionExperiment findByBioAssay( BioAssay ba );
9999

100+
@Nullable
101+
ExpressionExperiment findByBioAssay( BioAssay ba, boolean includeSubSets );
102+
100103
Collection<ExpressionExperiment> findByBioMaterial( BioMaterial bm );
101104

105+
Collection<ExpressionExperiment> findByBioMaterial( BioMaterial bm, boolean includeSubSets );
106+
102107
Map<ExpressionExperiment, Collection<BioMaterial>> findByBioMaterials( Collection<BioMaterial> bms );
103108

104109
Collection<ExpressionExperiment> findByExpressedGene( Gene gene, Double rank );

‎gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/ExpressionExperimentDaoImpl.java‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,24 +283,54 @@ public Collection<ExpressionExperiment> findByBibliographicReference( Bibliograp
283283

284284
@Override
285285
public ExpressionExperiment findByBioAssay( BioAssay ba ) {
286-
return ( ExpressionExperiment ) this.getSessionFactory().getCurrentSession()
286+
return findByBioAssay( ba, false );
287+
}
288+
289+
@Override
290+
public ExpressionExperiment findByBioAssay( BioAssay ba, boolean includeSubSets ) {
291+
ExpressionExperiment ee = ( ExpressionExperiment ) this.getSessionFactory().getCurrentSession()
287292
.createQuery( "select ee from ExpressionExperiment as ee "
288293
+ "join ee.bioAssays as ba "
289294
+ "where ba = :ba "
290295
+ "group by ee" )
291296
.setParameter( "ba", ba )
292297
.uniqueResult();
298+
if ( ee == null && includeSubSets ) {
299+
ee = ( ExpressionExperiment ) this.getSessionFactory().getCurrentSession()
300+
.createQuery( "select eess.sourceExperiment from ExpressionExperimentSubSet as eess "
301+
+ "join eess.bioAssays as ba "
302+
+ "where ba = :ba "
303+
+ "group by eess.sourceExperiment" )
304+
.setParameter( "ba", ba )
305+
.uniqueResult();
306+
}
307+
return ee;
293308
}
294309

295310
@Override
296311
public Collection<ExpressionExperiment> findByBioMaterial( BioMaterial bm ) {
312+
return findByBioMaterial( bm, false );
313+
}
314+
315+
@Override
316+
public Collection<ExpressionExperiment> findByBioMaterial( BioMaterial bm, boolean includeSubSets ) {
297317
//noinspection unchecked
298-
return this.getSessionFactory().getCurrentSession()
318+
List<ExpressionExperiment> results = this.getSessionFactory().getCurrentSession()
299319
.createQuery( "select ee from ExpressionExperiment as ee "
300320
+ "join ee.bioAssays as ba join ba.sampleUsed as sample where sample = :bm "
301321
+ "group by ee" )
302322
.setParameter( "bm", bm )
303323
.list();
324+
if ( results == null && includeSubSets ) {
325+
//noinspection unchecked
326+
results = this.getSessionFactory().getCurrentSession()
327+
.createQuery( "select eess.sourceExperiment from ExpressionExperimentSubSet as eess "
328+
+ "join eess.bioAssays as ba join ba.sampleUsed as sample where sample = :bm "
329+
+ "group by eess.sourceExperiment" )
330+
.setParameter( "bm", bm )
331+
.list();
332+
}
333+
return results;
304334
}
305335

306336
@Override

‎gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/ExpressionExperimentService.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,13 +354,26 @@ public interface ExpressionExperimentService extends SecurableBaseService<Expres
354354
@Secured({ "IS_AUTHENTICATED_ANONYMOUSLY", "AFTER_ACL_READ" })
355355
ExpressionExperiment findByBioAssay( BioAssay ba );
356356

357+
/**
358+
* @param includeSubSets include assays that belong to subsets of the experiment
359+
*/
360+
@Secured({ "IS_AUTHENTICATED_ANONYMOUSLY", "AFTER_ACL_READ" })
361+
ExpressionExperiment findByBioAssay( BioAssay ba, boolean includeSubSets );
362+
357363
/**
358364
* @param bm bio material
359365
* @return experiment the given biomaterial is associated with
360366
*/
361367
@Secured({ "IS_AUTHENTICATED_ANONYMOUSLY", "AFTER_ACL_COLLECTION_READ" })
362368
Collection<ExpressionExperiment> findByBioMaterial( BioMaterial bm );
363369

370+
/**
371+
*
372+
* @param includeSubSets include samples that are associated to assays that belong to subsets of the experiment
373+
*/
374+
@Secured({ "IS_AUTHENTICATED_ANONYMOUSLY", "AFTER_ACL_COLLECTION_READ" })
375+
Collection<ExpressionExperiment> findByBioMaterial( BioMaterial bm, boolean includeSubSets );
376+
364377
@Secured({ "IS_AUTHENTICATED_ANONYMOUSLY", "AFTER_ACL_COLLECTION_READ" })
365378
Map<ExpressionExperiment, Collection<BioMaterial>> findByBioMaterials( Collection<BioMaterial> biomaterials );
366379

‎gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/ExpressionExperimentServiceImpl.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,12 @@ public ExpressionExperiment findByBioAssay( final BioAssay ba ) {
578578
return this.expressionExperimentDao.findByBioAssay( ba );
579579
}
580580

581+
@Override
582+
@Transactional(readOnly = true)
583+
public ExpressionExperiment findByBioAssay( BioAssay ba, boolean includeSubSets ) {
584+
return this.expressionExperimentDao.findByBioAssay( ba, includeSubSets );
585+
}
586+
581587
/**
582588
* @see ExpressionExperimentService#findByBioMaterial(BioMaterial)
583589
*/
@@ -587,6 +593,12 @@ public Collection<ExpressionExperiment> findByBioMaterial( final BioMaterial bm
587593
return this.expressionExperimentDao.findByBioMaterial( bm );
588594
}
589595

596+
@Override
597+
@Transactional(readOnly = true)
598+
public Collection<ExpressionExperiment> findByBioMaterial( BioMaterial bm, boolean includeSubSets ) {
599+
return this.expressionExperimentDao.findByBioMaterial( bm, includeSubSets );
600+
}
601+
590602
@Override
591603
public Map<ExpressionExperiment, Collection<BioMaterial>> findByBioMaterials( Collection<BioMaterial> biomaterials ) {
592604
return this.expressionExperimentDao.findByBioMaterials( biomaterials );

‎gemma-core/src/test/java/ubic/gemma/core/security/authorization/acl/AclLinterServiceTest.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
import ubic.gemma.core.util.test.BaseDatabaseTest;
1515
import ubic.gemma.model.analysis.expression.diff.DifferentialExpressionAnalysis;
1616
import ubic.gemma.model.analysis.expression.diff.ExpressionAnalysisResultSet;
17+
import ubic.gemma.model.expression.bioAssay.BioAssay;
18+
import ubic.gemma.model.expression.biomaterial.BioMaterial;
1719
import ubic.gemma.model.expression.experiment.ExpressionExperiment;
1820
import ubic.gemma.persistence.service.expression.experiment.ExpressionExperimentService;
1921

@@ -37,6 +39,11 @@ public ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy() {
3739
return new ObjectIdentityRetrievalStrategyImpl();
3840
}
3941

42+
@Bean
43+
public ParentIdentityRetrievalStrategy parentObjectRetrievalStrategy() {
44+
return new ParentIdentityRetrievalStrategyImpl();
45+
}
46+
4047
@Bean
4148
public ExpressionExperimentService expressionExperimentService() {
4249
return mock();
@@ -62,9 +69,18 @@ public void test() {
6269
aclLinterService.lintAcls( ExpressionExperiment.class, config );
6370
aclLinterService.lintAcls( ExpressionExperiment.class, 1L, config );
6471

72+
aclLinterService.lintAcls( ExpressionAnalysisResultSet.class, config );
6573
aclLinterService.lintAcls( ExpressionAnalysisResultSet.class, 1L, config );
74+
75+
aclLinterService.lintAcls( DifferentialExpressionAnalysis.class, config );
6676
aclLinterService.lintAcls( DifferentialExpressionAnalysis.class, 1L, config );
6777

78+
aclLinterService.lintAcls( BioAssay.class, config );
79+
aclLinterService.lintAcls( BioAssay.class, 1L, config );
80+
81+
aclLinterService.lintAcls( BioMaterial.class, config );
82+
aclLinterService.lintAcls( BioMaterial.class, 1L, config );
83+
6884
config = AclLinterConfig.builder()
6985
.lintDanglingIdentities( true )
7086
.lintSecurablesLackingIdentities( true )

0 commit comments

Comments
 (0)