Skip to content

Commit d4fff1d

Browse files
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. Improvements for ACL advice
1 parent 90ecb28 commit d4fff1d

6 files changed

Lines changed: 30 additions & 90 deletions

File tree

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

Lines changed: 16 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -19,32 +19,26 @@
1919
package ubic.gemma.core.security.authorization.acl;
2020

2121
import gemma.gsec.acl.BaseAclAdvice;
22+
import gemma.gsec.acl.ObjectTransientnessRetrievalStrategy;
23+
import gemma.gsec.acl.ParentIdentityRetrievalStrategy;
2224
import gemma.gsec.acl.domain.AclService;
23-
import gemma.gsec.model.GroupAuthority;
2425
import gemma.gsec.model.Securable;
2526
import gemma.gsec.model.User;
2627
import gemma.gsec.model.UserGroup;
2728
import org.apache.commons.logging.Log;
2829
import org.apache.commons.logging.LogFactory;
2930
import org.hibernate.SessionFactory;
3031
import org.springframework.beans.factory.annotation.Autowired;
31-
import org.springframework.security.acls.model.*;
32-
import org.springframework.security.core.GrantedAuthority;
33-
import org.springframework.security.core.authority.SimpleGrantedAuthority;
32+
import org.springframework.security.acls.model.ObjectIdentityRetrievalStrategy;
3433
import org.springframework.stereotype.Component;
3534
import ubic.gemma.model.analysis.Investigation;
36-
import ubic.gemma.model.analysis.SingleExperimentAnalysis;
3735
import ubic.gemma.model.common.auditAndSecurity.AuditTrail;
3836
import ubic.gemma.model.common.auditAndSecurity.curation.CurationDetails;
3937
import ubic.gemma.model.expression.arrayDesign.ArrayDesign;
4038
import ubic.gemma.model.expression.bioAssay.BioAssay;
41-
import ubic.gemma.model.expression.experiment.BioAssaySet;
4239
import ubic.gemma.model.expression.experiment.ExpressionExperiment;
4340
import ubic.gemma.persistence.util.Pointcuts;
4441

45-
import javax.annotation.Nullable;
46-
import java.util.Collection;
47-
4842
/**
4943
* For permissions modification to be triggered, the method name must match certain patterns, which include "create", or
5044
* "remove". These patterns are defined in the {@link Pointcuts}. Other methods that would require
@@ -58,14 +52,17 @@ public class AclAdvice extends BaseAclAdvice {
5852
private static final Log log = LogFactory.getLog( AclAdvice.class );
5953

6054
@Autowired
61-
public AclAdvice( AclService aclService, SessionFactory sessionFactory, ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy ) {
62-
super( aclService, sessionFactory, objectIdentityRetrievalStrategy );
55+
public AclAdvice( AclService aclService, SessionFactory sessionFactory,
56+
ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy,
57+
ParentIdentityRetrievalStrategy parentIdentityRetrievalStrategy,
58+
ObjectTransientnessRetrievalStrategy objectTransientnessRetrievalStrategy ) {
59+
super( aclService, sessionFactory, objectIdentityRetrievalStrategy, parentIdentityRetrievalStrategy,
60+
objectTransientnessRetrievalStrategy );
6361
}
6462

6563
@Override
6664
protected boolean canSkipAclCheck( Object object ) {
67-
return AuditTrail.class.isAssignableFrom( object.getClass() ) || CurationDetails.class
68-
.isAssignableFrom( object.getClass() );
65+
return object instanceof AuditTrail || object instanceof CurationDetails;
6966
}
7067

7168
@Override
@@ -75,7 +72,7 @@ protected boolean canSkipAssociationCheck( Object object, String propertyName )
7572
* If this is an expression experiment, don't go down the data vectors - it has no securable associations and
7673
* would be expensive to traverse.
7774
*/
78-
if ( ExpressionExperiment.class.isAssignableFrom( object.getClass() )
75+
if ( object instanceof ExpressionExperiment
7976
&& ( propertyName.equals( "rawExpressionDataVectors" )
8077
|| propertyName.equals( "processedExpressionDataVectors" )
8178
|| propertyName.equals( "singleCellExpressionDataVectors" ) ) ) {
@@ -87,7 +84,7 @@ protected boolean canSkipAssociationCheck( Object object, String propertyName )
8784
/*
8885
* Array design has some non (directly) securable associations that would be expensive to load
8986
*/
90-
if ( ArrayDesign.class.isAssignableFrom( object.getClass() ) && propertyName.equals( "compositeSequences" ) ) {
87+
if ( object instanceof ArrayDesign && propertyName.equals( "compositeSequences" ) ) {
9188
if ( AclAdvice.log.isTraceEnabled() )
9289
AclAdvice.log.trace( "Skipping checking acl on probes on " + object );
9390
return true;
@@ -97,60 +94,13 @@ protected boolean canSkipAssociationCheck( Object object, String propertyName )
9794
}
9895

9996
@Override
100-
protected void createOrUpdateAclSpecialCases( MutableAcl acl, @Nullable Acl parentAcl, Sid sid, Securable object ) {
101-
102-
// Treating Analyses as special case. It'll inherit ACL from ExpressionExperiment
103-
// If aclParent is passed to this method we overwrite it.
104-
if ( SingleExperimentAnalysis.class.isAssignableFrom( object.getClass() ) ) {
105-
SingleExperimentAnalysis<?> experimentAnalysis = ( SingleExperimentAnalysis<?> ) object;
106-
107-
BioAssaySet bioAssaySet = experimentAnalysis.getExperimentAnalyzed();
108-
ObjectIdentity oi_temp = this.makeObjectIdentity( bioAssaySet );
109-
110-
parentAcl = this.getAclService().readAclById( oi_temp );
111-
if ( parentAcl == null ) {
112-
// This is possible if making an EESubSet is part of the transaction.
113-
parentAcl = this.getAclService().createAcl( oi_temp );
114-
}
115-
acl.setEntriesInheriting( true );
116-
acl.setParent( parentAcl );
117-
//noinspection UnusedAssignment //Owner of the experiment owns analyses even if administrator ran them.
118-
sid = parentAcl.getOwner();
119-
}
120-
121-
}
122-
123-
@Override
124-
protected GrantedAuthority getUserGroupGrantedAuthority( Securable object ) {
125-
Collection<? extends GroupAuthority> authorities = ( ( UserGroup ) object ).getAuthorities();
126-
assert authorities.size() == 1;
127-
return new SimpleGrantedAuthority( authorities.iterator().next().getAuthority() );
128-
}
129-
130-
@Override
131-
protected String getUserName( Securable user ) {
132-
return ( ( User ) user ).getUserName();
133-
}
134-
135-
@Override
136-
protected boolean objectIsUser( Securable object ) {
137-
return User.class.isAssignableFrom( object.getClass() );
138-
}
139-
140-
@Override
141-
protected boolean objectIsUserGroup( Securable object ) {
142-
return UserGroup.class.isAssignableFrom( object.getClass() );
143-
}
144-
145-
@Override
146-
protected boolean specialCaseForAssociationFollow( Object object, String property ) {
147-
return BioAssay.class.isAssignableFrom( object.getClass() ) && ( property.equals( "sampleUsed" ) || property
148-
.equals( "arrayDesignUsed" ) );
97+
protected boolean canFollowAssociation( Object object, String property ) {
98+
return object instanceof BioAssay && ( property.equals( "sampleUsed" ) || property.equals( "arrayDesignUsed" ) );
14999
}
150100

151101
@Override
152-
protected boolean specialCaseToKeepPrivateOnCreation( Securable object ) {
153-
return super.specialCaseToKeepPrivateOnCreation( object )
102+
protected boolean isKeepPrivateOnCreation( Securable object ) {
103+
return super.isKeepPrivateOnCreation( object )
154104
|| object instanceof UserGroup
155105
|| object instanceof User
156106
|| object instanceof Investigation;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package ubic.gemma.core.security.authorization.acl;
22

3+
import gemma.gsec.acl.ParentIdentityRetrievalStrategy;
34
import gemma.gsec.acl.domain.AclGrantedAuthoritySid;
45
import gemma.gsec.acl.domain.AclObjectIdentity;
56
import gemma.gsec.acl.domain.AclService;

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

Lines changed: 0 additions & 21 deletions
This file was deleted.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package ubic.gemma.core.security.authorization.acl;
22

3+
import gemma.gsec.acl.ParentIdentityRetrievalStrategy;
34
import gemma.gsec.acl.domain.AclObjectIdentity;
45
import lombok.extern.apachecommons.CommonsLog;
56
import org.hibernate.Hibernate;

gemma-core/src/main/resources/ubic/gemma/applicationContext-security.xml

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,17 @@
8181
<!-- Aspect that adds ACLs for new entities, and deletes them for deleted entities. -->
8282
<aop:config>
8383
<aop:aspect id="aclAspect" ref="aclAdvice" order="4">
84-
<aop:after-returning method="doAclAdvice"
85-
pointcut="ubic.gemma.persistence.util.Pointcuts.modifier()"
84+
<aop:after-returning method="doAclCreateAdvice"
85+
pointcut="ubic.gemma.persistence.util.Pointcuts.creator()"
86+
returning="retValue"/>
87+
<aop:after-returning method="doAclUpdateAdvice"
88+
pointcut="ubic.gemma.persistence.util.Pointcuts.updater()"
89+
returning="retValue"/>
90+
<aop:after-returning method="doAclSaveAdvice"
91+
pointcut="ubic.gemma.persistence.util.Pointcuts.saver()"
92+
returning="retValue"/>
93+
<aop:after-returning method="doAclDeleteAdvice"
94+
pointcut="ubic.gemma.persistence.util.Pointcuts.deleter()"
8695
returning="retValue"/>
8796
</aop:aspect>
8897
</aop:config>

pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -875,7 +875,7 @@
875875
</plugins>
876876
</reporting>
877877
<properties>
878-
<gsec.version>0.0.22</gsec.version>
878+
<gsec.version>0.0.23-SNAPSHOT</gsec.version>
879879
<spring.version>3.2.18.RELEASE</spring.version>
880880
<spring.security.version>3.2.10.RELEASE</spring.security.version>
881881
<jersey.version>2.25.1</jersey.version>

0 commit comments

Comments
 (0)