diff --git a/gsec/pom.xml b/gsec/pom.xml index 1f52d04..b1398a1 100644 --- a/gsec/pom.xml +++ b/gsec/pom.xml @@ -8,7 +8,7 @@ pavlab gemma-gsec - 0.0.22 + 0.0.23-SNAPSHOT jar gsec @@ -49,6 +49,12 @@ commons-logging + + org.apache.commons + commons-collections4 + 4.5.0 + + org.hibernate @@ -137,7 +143,7 @@ org.json json - 20250107 + 20251224 true @@ -318,7 +324,7 @@ org.codehaus.mojo versions-maven-plugin - 2.20.1 + 2.21.0 org.hibernate:hibernate-core:* diff --git a/gsec/src/main/java/gemma/gsec/acl/BaseAclAdvice.java b/gsec/src/main/java/gemma/gsec/acl/BaseAclAdvice.java index b7ccc37..7d2dabd 100644 --- a/gsec/src/main/java/gemma/gsec/acl/BaseAclAdvice.java +++ b/gsec/src/main/java/gemma/gsec/acl/BaseAclAdvice.java @@ -18,14 +18,14 @@ import gemma.gsec.acl.domain.AclGrantedAuthoritySid; import gemma.gsec.acl.domain.AclPrincipalSid; import gemma.gsec.acl.domain.AclService; -import gemma.gsec.model.Securable; -import gemma.gsec.model.SecuredChild; -import gemma.gsec.model.SecuredNotChild; +import gemma.gsec.model.*; import gemma.gsec.util.SecurityUtil; +import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.aspectj.lang.JoinPoint; import org.aspectj.lang.Signature; +import org.aspectj.lang.annotation.AfterReturning; import org.hibernate.Hibernate; import org.hibernate.LazyInitializationException; import org.hibernate.SessionFactory; @@ -44,9 +44,8 @@ import javax.annotation.Nullable; import java.beans.PropertyDescriptor; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.Collection; +import java.util.*; /** * Adds security controls to newly created objects (including those created by updates to other objects via cascades), @@ -61,72 +60,49 @@ * @author pavlidis * @version $Id: BaseAclAdvice.java,v 1.1 2013/09/14 16:56:03 paul Exp $ */ -@SuppressWarnings("unused") public abstract class BaseAclAdvice { private static final Log log = LogFactory.getLog( BaseAclAdvice.class ); + private enum AclMode { + CREATE, + UPDATE, + SAVE, + DELETE + } + private final AclService aclService; private final SessionFactory sessionFactory; private final ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy; + private final ParentIdentityRetrievalStrategy parentIdentityRetrievalStrategy; + private final ObjectTransientnessRetrievalStrategy objectTransientnessRetrievalStrategy; - protected BaseAclAdvice( AclService aclService, SessionFactory sessionFactory, ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy ) { + protected BaseAclAdvice( AclService aclService, SessionFactory sessionFactory, ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy, ParentIdentityRetrievalStrategy parentIdentityRetrievalStrategy, ObjectTransientnessRetrievalStrategy objectTransientnessRetrievalStrategy ) { this.aclService = aclService; this.sessionFactory = sessionFactory; this.objectIdentityRetrievalStrategy = objectIdentityRetrievalStrategy; + this.parentIdentityRetrievalStrategy = parentIdentityRetrievalStrategy; + this.objectTransientnessRetrievalStrategy = objectTransientnessRetrievalStrategy; } - public final void doAclAdvice( JoinPoint jp, Object retValue ) { - // - - final Object[] args = jp.getArgs(); - Signature signature = jp.getSignature(); - final String methodName = signature.getName(); - - // Class[] clazzes = new Class[jp.getArgs().length]; - // for ( int i = 0; i < jp.getArgs().length; i++ ) { - // clazzes[i] = jp.getArgs().getClass(); - // } - // Method m = signature.getDeclaringType().getDeclaredMethod( signature.getName(), clazzes ); - // Transactional annotation = m.getAnnotation( Transactional.class ); - // if ( annotation != null && !annotation.readOnly() ) { - // return; - // } - - assert args != null; - final Object persistentObject = getPersistentObject( retValue, methodName, args ); - - if ( persistentObject == null ) return; - - final boolean isUpdate = methodIsUpdate( methodName ); - final boolean isDelete = methodIsDelete( methodName ); + @AfterReturning + public void doAclCreateAdvice( JoinPoint jp, @Nullable Object retValue ) { + doAclAdvice( jp, retValue, AclMode.CREATE ); + } - // Case 1: collection of securables. - if ( Collection.class.isAssignableFrom( persistentObject.getClass() ) ) { - for ( final Object o : ( Collection ) persistentObject ) { - if ( isIneligibleForAcl( o ) ) { - continue; // possibly could return, if we assume collection is homogeneous in type. - } - process( o, methodName, isUpdate, isDelete ); - } - } else { - // Case 2: single securable - if ( isIneligibleForAcl( persistentObject ) ) { - /* - * This fails for methods that delete entities given an id. - */ - return; - } - process( persistentObject, methodName, isUpdate, isDelete ); - } + @AfterReturning + public void doAclUpdateAdvice( JoinPoint jp, @Nullable Object retValue ) { + doAclAdvice( jp, retValue, AclMode.UPDATE ); + } + @AfterReturning + public void doAclSaveAdvice( JoinPoint jp, @Nullable Object retValue ) { + doAclAdvice( jp, retValue, AclMode.SAVE ); } - // from gemma reflectionutil - private Object getProperty( Object object, PropertyDescriptor descriptor ) throws IllegalAccessException, - InvocationTargetException { - Method getter = descriptor.getReadMethod(); - return getter.invoke( object ); + @AfterReturning + public void doAclDeleteAdvice( JoinPoint jp, @Nullable Object retValue ) { + doAclAdvice( jp, retValue, AclMode.DELETE ); } /** @@ -148,165 +124,176 @@ protected boolean canSkipAssociationCheck( Object object, String propertyName ) } /** - * Subclasses can modify this to address special cases. Default implementation is a no-op. - *

- * FIXME this might not be necessary. + * For cases where don't have a cascade but the other end is securable, so we must check the association. + * For example, when we persist an EE we also persist any new ADs in the same transaction. Thus the ADs need ACL + * attention at the same time (via the BioAssays). * - * @param acl may be modified by this call - * @param parentAcl value may be changed by this call - * @param sid value may be changed by this call + * @param object we are checking + * @param property of the object + * @return true if the association should be followed (even though it might not be based on cascade status) */ - protected void createOrUpdateAclSpecialCases( MutableAcl acl, @Nullable Acl parentAcl, Sid sid, Securable object ) { - } - - protected boolean currentUserIsAdmin() { - return SecurityUtil.isUserAdmin(); - } - - protected boolean currentUserIsAnonymous() { - return SecurityUtil.isUserAnonymous(); - } - - protected boolean currentUserIsRunningAsAdmin() { - return SecurityUtil.isRunningAsAdmin(); + protected boolean canFollowAssociation( Object object, String property ) { + return false; } /** - * For use by other overridden methods. + * For cases in which the object is not a SecuredChild, but we still want to erase ACEs on it when it has a parent. + * Implementers will check the class of the object, and the class of the parent (e.g. using Class.forName( + * parentAcl.getObjectIdentity().getType() )) and decide what to do. + * + * @return false if ACEs should be retained. True if ACEs should be removed (if possible). */ - protected final AclService getAclService() { - return aclService; + protected boolean canRemoveAcesFromChild( Securable object, Acl parentAcl ) { + return false; } - protected abstract GrantedAuthority getUserGroupGrantedAuthority( Securable object ); - - protected abstract String getUserName( Securable object ); - /** - * Called during create. Default implementation returns null unless the object is a SecuredChild, in which case it - * returns the value of s.getSecurityOwner() (which will be null unless it is filled in) + * Indicate if the given object should not be made public immediately on creation by administrators. *

- * For some cases, we want to find the parent so we don't have to rely on updates later to catch it and fill it in. - * Implementers must decide which cases can be handled this way. Care is required: the parent might not be created - * yet, in which case the cascade to s is surely going to fix it later. The best situation is when s has an accessor - * to reach the parent. + * The default implementation returns true if the object is a {@link SecuredChild}; otherwise false. * - * @param s which might have a parent already in the system + * @return true if it's a special case to be kept private on creation. */ - protected Acl locateParentAcl( SecuredChild s ) { - Securable parent = locateSecuredParent( s ); - - if ( parent != null ) return this.getAclService().readAclById( makeObjectIdentity( parent ) ); - - return null; + protected boolean isKeepPrivateOnCreation( Securable object ) { + return object instanceof SecuredChild; } - /** - * Forms the object identity to be inserted in acl_object_identity table. Note that this does not add an - * ObjectIdentity to the database; it just calls 'new'. - * - * @param object A persistent object - * @return object identity. - */ - protected final ObjectIdentity makeObjectIdentity( Securable object ) { - return objectIdentityRetrievalStrategy.getObjectIdentity( object ); - } + private void doAclAdvice( JoinPoint jp, @Nullable Object retValue, AclMode aclMode ) { + final Object[] args = jp.getArgs(); + Signature signature = jp.getSignature(); + final String methodName = signature.getName(); - protected abstract boolean objectIsUser( Securable object ); + assert args != null; + Object persistentObject; + if ( aclMode == AclMode.UPDATE || aclMode == AclMode.DELETE ) { + if ( args.length >= 1 ) { + persistentObject = args[0]; + if ( persistentObject == null ) { + log.warn( "First argument of " + jp + " is null, cannot update/delete ACLs." ); + return; + } + } else { + log.warn( jp + " has no argument; cannot update/delete ACLs." ); + return; + } + } else { + // SAVE and CREATE return a value + if ( retValue != null ) { + persistentObject = retValue; + } else { + log.warn( jp + " returned null, cannot create/update ACLs." ); + return; + } + } - protected abstract boolean objectIsUserGroup( Securable object ); + for ( Securable securable : extractSecurables( persistentObject ) ) { + process( securable, methodName, aclMode ); + } + } /** - * Called when objects are first created in the system and need their permissions initialized. Insert the access - * control entries that all objects should have (unless they inherit from another object). - *

- * Default implementation does the following: - *

    - *
  • All objects are administratable by GROUP_ADMIN - *
  • GROUP_AGENT has READ permissions on all objects - *
  • If the current user is an adminisrator, and keepPrivateEvenWhenAdmin is false, the object gets READ - * permissions for ANONYMOUS. - *
  • If the current user is a "regular user" (non-admin) give them read/write permissions. + * Do necessary ACL operations on the object. */ - protected void setupBaseAces( MutableAcl acl, ObjectIdentity oi, Sid sid, boolean keepPrivateEvenWhenAdmin ) { - - /* - * All objects must have administration permissions on them. - */ - if ( log.isDebugEnabled() ) log.debug( "Making administratable by GROUP_ADMIN: " + oi ); - grant( acl, BasePermission.ADMINISTRATION, new AclGrantedAuthoritySid( new SimpleGrantedAuthority( - AuthorityConstants.ADMIN_GROUP_AUTHORITY ) ) ); + private void process( final Securable s, final String methodName, final AclMode aclMode ) { + if ( log.isTraceEnabled() ) + log.trace( "*********** Start " + aclMode.name().toLowerCase() + " ACL on " + s + " *************" ); + switch ( aclMode ) { + case CREATE: + startCreate( methodName, s ); + break; + case UPDATE: + startUpdate( methodName, s ); + break; + case SAVE: + if ( objectTransientnessRetrievalStrategy.isObjectTransient( s ) ) { + startCreate( methodName, s ); + } else { + startUpdate( methodName, s ); + } + break; + case DELETE: + deleteAcl( s ); + break; + default: + throw new IllegalArgumentException( "Unknown ACL mode: " + aclMode + "." ); + } + if ( log.isTraceEnabled() ) log.trace( "*========* End ACL on " + s + " *=========*" ); + } - /* - * Let agent read anything - */ - if ( log.isDebugEnabled() ) log.debug( "Making readable by GROUP_AGENT: " + oi ); - grant( acl, BasePermission.READ, new AclGrantedAuthoritySid( new SimpleGrantedAuthority( - AuthorityConstants.AGENT_GROUP_AUTHORITY ) ) ); + private void startCreate( String methodName, Securable s ) { + // Note that if the method is findOrCreate, we'll return quickly. + ObjectIdentity oi = objectIdentityRetrievalStrategy.getObjectIdentity( s ); - /* - * If admin, and the object is not a user or group, make it readable by anonymous. - */ - boolean makeAnonymousReadable = this.currentUserIsAdmin() && !keepPrivateEvenWhenAdmin; + if ( oi == null ) { + throw new IllegalStateException( + "On 'create' methods, object should have a valid objectIdentity available. Method=" + methodName + + " on " + s ); + } - if ( makeAnonymousReadable ) { - if ( log.isDebugEnabled() ) log.debug( "Making readable by IS_AUTHENTICATED_ANONYMOUSLY: " + oi ); - grant( acl, BasePermission.READ, new AclGrantedAuthoritySid( new SimpleGrantedAuthority( - AuthenticatedVoter.IS_AUTHENTICATED_ANONYMOUSLY ) ) ); + Acl parentAcl; + if ( s instanceof SecuredChild ) { + ObjectIdentity parentAoi = parentIdentityRetrievalStrategy.getParentIdentity( s ); + parentAcl = parentAoi != null ? aclService.readAclById( parentAoi ) : null; + } else { + parentAcl = null; } - /* - * Don't add more permissions for the administrator. But whatever it is, the person who created it can - * read/write it. User will only be anonymous if they are registering (AFAIK) - */ - if ( !this.currentUserIsAdmin() && !this.currentUserIsAnonymous() ) { + addOrUpdateAcl( null, s, parentAcl ); + processAssociations( s, null ); + } - if ( log.isDebugEnabled() ) log.debug( "Giving read/write permissions on " + oi + " to " + sid ); - grant( acl, BasePermission.READ, sid ); + /** + * Kick off an update. This is executed when we call fooService.update(s). The basic issue is to add permissions for + * any new associated objects. + * + * @param m the update method + * @param s the securable being updated. + */ + private void startUpdate( String m, Securable s ) { + ObjectIdentity oi = objectIdentityRetrievalStrategy.getObjectIdentity( s ); + // this is not persistent. + assert oi != null; + MutableAcl acl = null; + Acl parentAcl = null; + try { + acl = ( MutableAcl ) aclService.readAclById( oi ); + assert acl != null; + parentAcl = acl.getParentAcl(); // can be null. + } catch ( NotFoundException nfe ) { /* - * User who created something can edit it. + * This really should be an error. */ - grant( acl, BasePermission.WRITE, sid ); + /* + * Then, this shouldn't be an update. + */ + log.warn( "On 'update' methods, there should be a ACL on the passed object already. Method=" + m + " on " + + s ); } + addOrUpdateAcl( acl, s, parentAcl ); + processAssociations( s, parentAcl ); } /** - * For cases where don't have a cascade but the other end is securable, so we must check the association. - * For example, when we persist an EE we also persist any new ADs in the same transaction. Thus the ADs need ACL - * attention at the same time (via the BioAssays). - * - * @param object we are checking - * @param property of the object - * @return true if the association should be followed (even though it might not be based on cascade status) + * Delete acl permissions for an object. */ - protected boolean specialCaseForAssociationFollow( Object object, String property ) { - return false; + private void deleteAcl( Securable object ) throws DataAccessException, IllegalArgumentException { + ObjectIdentity oi = objectIdentityRetrievalStrategy.getObjectIdentity( object ); - } + if ( oi == null ) { + log.warn( "Null object identity for : " + object ); + } - /** - * For cases in which the object is not a SecuredChild, but we still want to erase ACEs on it when it has a parent. - * Implementers will check the class of the object, and the class of the parent (e.g. using Class.forName( - * parentAcl.getObjectIdentity().getType() )) and decide what to do. - * - * @return false if ACEs should be retained. True if ACEs should be removed (if possible). - */ - protected boolean specialCaseToAllowRemovingAcesFromChild( Securable object, Acl parentAcl ) { - return false; - } + if ( log.isDebugEnabled() ) { + log.debug( "Deleting ACL for " + object ); + } - /** - * Indicate if the given object should not be made public immediately on creation by administrators. - *

    - * The default implementation returns true if the object is a {@link SecuredChild}; otherwise false. - * - * @return true if it's a special case to be kept private on creation. - */ - protected boolean specialCaseToKeepPrivateOnCreation( Securable object ) { - return object instanceof SecuredChild; + /* + * This deletes children with the second parameter = true. + */ + aclService.deleteAcl( oi, true ); } /** @@ -324,14 +311,14 @@ private void addOrUpdateAcl( @Nullable MutableAcl acl, Securable object, @Nullab } if ( log.isTraceEnabled() ) log.trace( "Checking for ACLS on " + object ); - ObjectIdentity oi = makeObjectIdentity( object ); + ObjectIdentity oi = objectIdentityRetrievalStrategy.getObjectIdentity( object ); boolean create = false; if ( acl == null ) { // usually create, but could be update. try { // this is probably redundant. We shouldn't have ACLs already. - acl = ( MutableAcl ) getAclService().readAclById( oi ); // throws exception if not found + acl = ( MutableAcl ) aclService.readAclById( oi ); // throws exception if not found /* * If we get here, we're in update mode after all. Could be findOrCreate, or could be a second pass that * will let us fill in parent ACLs for associated objects missed earlier in a persist cycle. E.g. @@ -345,7 +332,7 @@ private void addOrUpdateAcl( @Nullable MutableAcl acl, Securable object, @Nullab } } catch ( NotFoundException nfe ) { // the current user will be the owner. - acl = getAclService().createAcl( oi ); + acl = aclService.createAcl( oi ); create = true; assert acl != null; assert acl.getOwner() != null; @@ -366,15 +353,11 @@ private void addOrUpdateAcl( @Nullable MutableAcl acl, Securable object, @Nullab AclPrincipalSid sid = new AclPrincipalSid( p.toString() ); - boolean isAdmin = currentUserIsAdmin(); - - boolean isRunningAsAdmin = currentUserIsRunningAsAdmin(); + boolean isAdmin = SecurityUtil.isUserAdmin(); - boolean objectIsAUser = objectIsUser( object ); + boolean isRunningAsAdmin = SecurityUtil.isRunningAsAdmin(); - boolean objectIsAGroup = objectIsUserGroup( object ); - - boolean keepPrivateEvenWhenAdmin = this.specialCaseToKeepPrivateOnCreation( object ); + boolean keepPrivateEvenWhenAdmin = this.isKeepPrivateOnCreation( object ); /* * The only case where we absolutely disallow inheritance is for SecuredNotChild. @@ -400,8 +383,8 @@ private void addOrUpdateAcl( @Nullable MutableAcl acl, Securable object, @Nullab /* * Make sure user groups can be read by future members of the group */ - if ( objectIsAGroup ) { - GrantedAuthority ga = getUserGroupGrantedAuthority( object ); + if ( object instanceof UserGroup ) { + GrantedAuthority ga = getUserGroupGrantedAuthority( ( UserGroup ) object ); if ( log.isDebugEnabled() ) log.debug( "Making group readable by " + ga + ": " + oi ); grant( acl, BasePermission.READ, new AclGrantedAuthoritySid( ga ) ); } @@ -417,8 +400,8 @@ private void addOrUpdateAcl( @Nullable MutableAcl acl, Securable object, @Nullab * fact, user creation runs with RUN_AS_ADMIN privileges. */ - if ( create && objectIsAUser ) { - String userName = getUserName( object ); + if ( create && object instanceof User ) { + String userName = ( ( User ) object ).getUserName(); if ( sid.getPrincipal().equals( userName ) ) { /* * This case should actually never happen. "we" are the user who is creating this user. We've already @@ -449,8 +432,6 @@ private void addOrUpdateAcl( @Nullable MutableAcl acl, Securable object, @Nullab } } - createOrUpdateAclSpecialCases( acl, parentAcl, sid, object ); - /* * Only the owner or an administrator can do these operations, and only in those cases would they be necessary * anyway (primarily in creating the objects in the first place, there's nearly no conceivable reason to change @@ -474,79 +455,91 @@ private void addOrUpdateAcl( @Nullable MutableAcl acl, Securable object, @Nullab } // finalize. - getAclService().updateAcl( acl ); + aclService.updateAcl( acl ); + } + private GrantedAuthority getUserGroupGrantedAuthority( UserGroup object ) { + Collection authorities = object.getAuthorities(); + assert authorities.size() == 1; + return new SimpleGrantedAuthority( authorities.iterator().next().getAuthority() ); } /** - * Determine which ACL is going to be the parent of the associations of the given object. + * Called when objects are first created in the system and need their permissions initialized. Insert the access + * control entries that all objects should have (unless they inherit from another object). *

    - * If the object is a SecuredNotChild, then it will be treated as the parent. For example, ArrayDesigns associated - * with an Experiment has 'parent status' for securables associated with the AD, such as LocalFiles. + * Default implementation does the following: + *

      + *
    • All objects are administratable by GROUP_ADMIN + *
    • GROUP_AGENT has READ permissions on all objects + *
    • If the current user is an adminisrator, and keepPrivateEvenWhenAdmin is false, the object gets READ + * permissions for ANONYMOUS. + *
    • If the current user is a "regular user" (non-admin) give them read/write permissions. */ - private Acl chooseParentForAssociations( Object object, @Nullable Acl previousParent ) { - Acl parentAcl; - if ( SecuredNotChild.class.isAssignableFrom( object.getClass() ) - || ( previousParent == null && Securable.class.isAssignableFrom( object.getClass() ) && !SecuredChild.class - .isAssignableFrom( object.getClass() ) ) ) { + private void setupBaseAces( MutableAcl acl, ObjectIdentity oi, Sid sid, boolean keepPrivateEvenWhenAdmin ) { + // All objects must have administration permissions on them. + if ( log.isDebugEnabled() ) log.debug( "Making administratable by GROUP_ADMIN: " + oi ); + grant( acl, BasePermission.ADMINISTRATION, new AclGrantedAuthoritySid( new SimpleGrantedAuthority( + AuthorityConstants.ADMIN_GROUP_AUTHORITY ) ) ); + + // Let agent read anything + if ( log.isDebugEnabled() ) log.debug( "Making readable by GROUP_AGENT: " + oi ); + grant( acl, BasePermission.READ, new AclGrantedAuthoritySid( new SimpleGrantedAuthority( + AuthorityConstants.AGENT_GROUP_AUTHORITY ) ) ); + + // If admin, and the object is not a user or group, make it readable by anonymous. + boolean makeAnonymousReadable = SecurityUtil.isUserAdmin() && !keepPrivateEvenWhenAdmin; + + if ( makeAnonymousReadable ) { + if ( log.isDebugEnabled() ) log.debug( "Making readable by IS_AUTHENTICATED_ANONYMOUSLY: " + oi ); + grant( acl, BasePermission.READ, new AclGrantedAuthoritySid( new SimpleGrantedAuthority( + AuthenticatedVoter.IS_AUTHENTICATED_ANONYMOUSLY ) ) ); + } + + // Don't add more permissions for the administrator. But whatever it is, the person who created it can + // read/write it. User will only be anonymous if they are registering (AFAIK) + if ( !SecurityUtil.isUserAdmin() && !SecurityUtil.isUserAnonymous() ) { + + if ( log.isDebugEnabled() ) log.debug( "Giving read/write permissions on " + oi + " to " + sid ); + grant( acl, BasePermission.READ, sid ); - parentAcl = getAcl( ( Securable ) object ); - } else { /* - * Keep the previous parent. This means we 'pass through' and the parent is basically going to be the - * top-most object: there isn't a hierarchy of parenthood. This also means that the parent might be kept as - * null. + * User who created something can edit it. */ - parentAcl = previousParent; + grant( acl, BasePermission.WRITE, sid ); } - return parentAcl; } /** - * Delete acl permissions for an object. + * Determine which ACL is going to be the parent of the associations of the given object. + *

      + * If the object is a SecuredNotChild, then it will be treated as the parent. For example, ArrayDesigns associated + * with an Experiment has 'parent status' for securables associated with the AD, such as LocalFiles. */ - private void deleteAcl( Securable object ) throws DataAccessException, IllegalArgumentException { - ObjectIdentity oi = makeObjectIdentity( object ); - - if ( oi == null ) { - log.warn( "Null object identity for : " + object ); - } - - if ( log.isDebugEnabled() ) { - log.debug( "Deleting ACL for " + object ); + @Nullable + private Acl chooseParentForAssociations( Object object, @Nullable Acl previousParent ) { + if ( object instanceof SecuredNotChild + || ( previousParent == null && object instanceof Securable && !( object instanceof SecuredChild ) ) ) { + return getAcl( ( Securable ) object ); + } else { + // Keep the previous parent. This means we 'pass through' and the parent is basically going to be the + // top-most object: there isn't a hierarchy of parenthood. This also means that the parent might be kept as + // null. + return previousParent; } - - /* - * This deletes children with the second parameter = true. - */ - this.getAclService().deleteAcl( oi, true ); } + @Nullable private MutableAcl getAcl( Securable s ) { ObjectIdentity oi = objectIdentityRetrievalStrategy.getObjectIdentity( s ); - try { - return ( MutableAcl ) getAclService().readAclById( oi ); + return ( MutableAcl ) aclService.readAclById( oi ); } catch ( NotFoundException e ) { return null; } } - private Object getPersistentObject( Object retValue, String methodName, Object[] args ) { - if ( methodIsDelete( methodName ) || methodIsUpdate( methodName ) ) { - - /* - * Only deal with single-argument update methods. MIGHT WANT TO RETURN THE FIRST ARGUMENT - */ - if ( args.length > 1 ) return null; - - assert args.length > 0; - return args[0]; - } - return retValue; - } - /** * Add ACE granting permission to sid to ACL (does not persist the change, you have to call update!) * @@ -558,27 +551,6 @@ private void grant( MutableAcl acl, Permission permission, Sid sid ) { acl.insertAce( acl.getEntries().size(), permission, sid, true ); } - private boolean isIneligibleForAcl( Object c ) { - return !( c instanceof Securable ); - } - - /** - * Recursively locate the actual secured parent. - */ - private Securable locateSecuredParent( SecuredChild s ) { - if ( s.getSecurityOwner() == null ) { - return null; - - } - Securable securityOwner = s.getSecurityOwner(); - - if ( securityOwner instanceof SecuredChild ) { - return locateSecuredParent( ( SecuredChild ) securityOwner ); - } - return securityOwner; - - } - /** * When setting the parent, we check to see if we can delete the ACEs on the 'child', if any. This is because we * want permissions to be managed by the parent, so ACEs on the child are redundant and possibly a source of later @@ -597,13 +569,13 @@ private boolean maybeClearACEsOnChild( Securable object, MutableAcl childAcl, @N int aceCount = childAcl.getEntries().size(); if ( aceCount == 0 ) { - if ( parentAcl.getEntries().size() == 0 ) { + if ( parentAcl.getEntries().isEmpty() ) { throw new IllegalStateException( "Either the child or the parent has to have ACEs" ); } return false; } - boolean force = specialCaseToAllowRemovingAcesFromChild( object, parentAcl ); + boolean force = canRemoveAcesFromChild( object, parentAcl ); if ( parentAcl.getEntries().size() == aceCount || force ) { @@ -635,7 +607,7 @@ private boolean maybeClearACEsOnChild( Securable object, MutableAcl childAcl, @N if ( log.isTraceEnabled() ) log.trace( "Erasing ACEs from child " + object ); - while ( childAcl.getEntries().size() > 0 ) { + while ( !childAcl.getEntries().isEmpty() ) { childAcl.deleteAce( 0 ); } @@ -670,7 +642,7 @@ private boolean maybeClearACEsOnChild( Securable object, MutableAcl childAcl, @N * @param parentAcl - the potential parent */ private void maybeSetParentACL( final Securable object, MutableAcl childAcl, @Nullable final Acl parentAcl ) { - if ( parentAcl != null && !SecuredNotChild.class.isAssignableFrom( object.getClass() ) ) { + if ( parentAcl != null && !( object instanceof SecuredNotChild ) ) { Acl currentParentAcl = childAcl.getParentAcl(); @@ -691,44 +663,18 @@ private void maybeSetParentACL( final Securable object, MutableAcl childAcl, @Nu boolean clearedACEs = maybeClearACEsOnChild( object, childAcl, parentAcl ); if ( changedParentAcl || clearedACEs ) { - getAclService().updateAcl( childAcl ); + aclService.updateAcl( childAcl ); } } childAcl.getParentAcl(); } - /** - * Do necessary ACL operations on the object. - */ - private void process( final Object o, final String methodName, final boolean isUpdate, final boolean isDelete ) { - - Securable s = ( Securable ) o; - - assert s != null; - - if ( isUpdate ) { - if ( log.isTraceEnabled() ) log.trace( "*********** Start update ACL on " + o + " *************" ); - startUpdate( methodName, s ); - } else if ( isDelete ) { - if ( log.isTraceEnabled() ) log.trace( "*********** Start delete ACL on " + o + " *************" ); - deleteAcl( s ); - } else { - if ( log.isTraceEnabled() ) log.trace( "*********** Start create ACL on " + o + " *************" ); - startCreate( methodName, s ); - } - - if ( log.isTraceEnabled() ) log.trace( "*========* End ACL on " + o + " *=========*" ); - } - /** * Walk the tree of associations and add (or update) acls. * - * @param methodName method name * @param previousParent The parent ACL of the given object (if it is a Securable) or of the last visited Securable. */ - @SuppressWarnings("unchecked") - private void processAssociations( String methodName, Object object, @Nullable Acl previousParent ) { - + private void processAssociations( Object object, @Nullable Acl previousParent ) { if ( canSkipAclCheck( object ) ) { return; } @@ -755,8 +701,8 @@ private void processAssociations( String methodName, Object object, @Nullable Ac * be a bit tricky because there are exceptions. This is kind of inelegant, but the alternative is to check * _every_ association, which will often not be reachable. */ - if ( !specialCaseForAssociationFollow( object, propertyName ) - && ( canSkipAssociationCheck( object, propertyName ) || !needCascade( methodName, cs ) ) ) { + if ( !canFollowAssociation( object, propertyName ) + && ( canSkipAssociationCheck( object, propertyName ) || !( cs.doCascade( CascadingAction.PERSIST ) || cs.doCascade( CascadingAction.SAVE_UPDATE ) || cs.doCascade( CascadingAction.MERGE ) ) ) ) { // if ( log.isTraceEnabled() ) // log.trace( "Skipping checking association: " + propertyName + " on " + object ); continue; @@ -767,7 +713,8 @@ private void processAssociations( String methodName, Object object, @Nullable Ac Object associatedObject; try { // FieldUtils DOES NOT WORK correctly with proxies - associatedObject = getProperty( object, descriptor ); + Method getter = descriptor.getReadMethod(); + associatedObject = getter.invoke( object ); } catch ( LazyInitializationException e ) { /* * This is not a problem. If this was reached via a create, the associated objects must not be new so @@ -788,17 +735,17 @@ private void processAssociations( String methodName, Object object, @Nullable Ac if ( associatedObject == null ) continue; if ( associatedObject instanceof Collection ) { - Collection associatedObjects = ( Collection ) associatedObject; + Collection associatedObjects = ( Collection ) associatedObject; try { for ( Object object2 : associatedObjects ) { - if ( Securable.class.isAssignableFrom( object2.getClass() ) ) { + if ( object2 instanceof Securable ) { addOrUpdateAcl( null, ( Securable ) object2, parentAcl ); } else if ( log.isTraceEnabled() ) { log.trace( object2 + ": not securable, skipping" ); } - processAssociations( methodName, object2, parentAcl ); + processAssociations( object2, parentAcl ); } } catch ( LazyInitializationException ok ) { /* @@ -819,81 +766,44 @@ private void processAssociations( String methodName, Object object, @Nullable Ac if ( Securable.class.isAssignableFrom( propertyType ) ) { addOrUpdateAcl( null, ( Securable ) associatedObject, parentAcl ); } - processAssociations( methodName, associatedObject, parentAcl ); + processAssociations( associatedObject, parentAcl ); } } } - private void startCreate( String methodName, Securable s ) { - - /* - * Note that if the method is findOrCreate, we'll return quickly. - */ - - ObjectIdentity oi = makeObjectIdentity( s ); - - if ( oi == null ) { - throw new IllegalStateException( - "On 'create' methods, object should have a valid objectIdentity available. Method=" + methodName - + " on " + s ); - } - - Acl parentAcl = SecuredChild.class.isAssignableFrom( s.getClass() ) ? locateParentAcl( ( SecuredChild ) s ) - : null; - - addOrUpdateAcl( null, s, parentAcl ); - processAssociations( methodName, s, null ); - - } - /** - * Kick off an update. This is executed when we call fooService.update(s). The basic issue is to add permissions for - * any new associated objects. - * - * @param m the update method - * @param s the securable being updated. + * Efficiently extract all securable of a given type in an object's tree. + *

      + * This method traverses {@link Map}, {@link Collection}, {@link Iterable} and Java arrays, but not properties and + * fields of objects. + *

      + * This was borrowed from Gemma's AuditAdvice. */ - private void startUpdate( String m, Securable s ) { - - ObjectIdentity oi = makeObjectIdentity( s ); - - // this is not persistent. - assert oi != null; - MutableAcl acl = null; - Acl parentAcl = null; - try { - acl = ( MutableAcl ) getAclService().readAclById( oi ); - assert acl != null; - parentAcl = acl.getParentAcl(); // can be null. - } catch ( NotFoundException nfe ) { - /* - * This really should be an error. - */ - - /* - * Then, this shouldn't be an update. - */ - log.warn( "On 'update' methods, there should be a ACL on the passed object already. Method=" + m + " on " - + s ); - } - - addOrUpdateAcl( acl, s, parentAcl ); - processAssociations( m, s, parentAcl ); - } - - - private boolean methodIsDelete( String s ) { - return s.equals( "remove" ) || s.equals( "delete" ); - } - - private boolean methodIsUpdate( String s ) { - return s.equals( "update" ) || s.equals( "persistOrUpdate" ); - } - - private boolean needCascade( String m, CascadeStyle cs ) { - if ( methodIsDelete( m ) ) { - return cs.doCascade( CascadingAction.DELETE ); + public static Collection extractSecurables( Object object ) { + // necessary as ArrayQueue does not accept nulls + Queue fringe = new LinkedList<>(); + // use identity hashcode since auditable might rely on a potentially null ID for hashing + Set visited = Collections.newSetFromMap( new IdentityHashMap<>() ); + Collection found = new ArrayList<>(); + fringe.add( object ); + while ( !fringe.isEmpty() ) { + Object o = fringe.remove(); + if ( o == null ) + continue; + if ( visited.contains( o ) ) + continue; + visited.add( o ); + if ( o instanceof Securable ) { + found.add( ( Securable ) o ); + } else if ( o.getClass().isArray() ) { + CollectionUtils.addAll( fringe, ( Object[] ) o ); + } else if ( o instanceof Iterable ) { + CollectionUtils.addAll( fringe, ( Iterable ) o ); + } else if ( o instanceof Map ) { + CollectionUtils.addAll( fringe, ( ( Map ) o ).keySet() ); + CollectionUtils.addAll( fringe, ( ( Map ) o ).values() ); + } } - return cs.doCascade( CascadingAction.PERSIST ) || cs.doCascade( CascadingAction.SAVE_UPDATE ); + return found; } } \ No newline at end of file diff --git a/gsec/src/main/java/gemma/gsec/acl/ObjectTransientnessRetrievalStrategyImpl.java b/gsec/src/main/java/gemma/gsec/acl/ObjectTransientnessRetrievalStrategyImpl.java index 74161d9..02a2bcb 100644 --- a/gsec/src/main/java/gemma/gsec/acl/ObjectTransientnessRetrievalStrategyImpl.java +++ b/gsec/src/main/java/gemma/gsec/acl/ObjectTransientnessRetrievalStrategyImpl.java @@ -1,6 +1,5 @@ package gemma.gsec.acl; -import gemma.gsec.acl.ObjectTransientnessRetrievalStrategy; import gemma.gsec.model.Securable; import org.springframework.util.Assert; diff --git a/gsec/src/main/java/gemma/gsec/acl/ParentIdentityRetrievalStrategy.java b/gsec/src/main/java/gemma/gsec/acl/ParentIdentityRetrievalStrategy.java new file mode 100644 index 0000000..29355fa --- /dev/null +++ b/gsec/src/main/java/gemma/gsec/acl/ParentIdentityRetrievalStrategy.java @@ -0,0 +1,21 @@ +package gemma.gsec.acl; + +import org.springframework.security.acls.model.ObjectIdentity; + +import javax.annotation.Nullable; + +/** + * Strategy for locating parent ACL identities. + * + * @author poirigui + */ +public interface ParentIdentityRetrievalStrategy { + + /** + * Obtain the parent ACL identity for the given domain object. + * + * @return the parent ACL identity if it can be determined, null otherwise + */ + @Nullable + ObjectIdentity getParentIdentity( Object domainObject ); +} diff --git a/gsec/src/main/java/gemma/gsec/acl/domain/AclObjectIdentity.java b/gsec/src/main/java/gemma/gsec/acl/domain/AclObjectIdentity.java index 9564283..d40a11d 100644 --- a/gsec/src/main/java/gemma/gsec/acl/domain/AclObjectIdentity.java +++ b/gsec/src/main/java/gemma/gsec/acl/domain/AclObjectIdentity.java @@ -25,6 +25,7 @@ import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import javax.annotation.Nullable; import java.util.HashSet; import java.util.Objects; import java.util.Set; @@ -49,6 +50,7 @@ public class AclObjectIdentity implements ObjectIdentity { private AclSid ownerSid; + @Nullable private AclObjectIdentity parentObject; private Set entries = new HashSet<>(); @@ -138,11 +140,12 @@ public void setOwnerSid( Sid ownerSid ) { this.ownerSid = ( AclSid ) ownerSid; } + @Nullable public AclObjectIdentity getParentObject() { return parentObject; } - public void setParentObject( AclObjectIdentity parentObject ) { + public void setParentObject( @Nullable AclObjectIdentity parentObject ) { assert parentObject != this && !this.equals( parentObject ); this.parentObject = parentObject; } diff --git a/gsec/src/main/java/gemma/gsec/model/Securable.java b/gsec/src/main/java/gemma/gsec/model/Securable.java index 212999a..6c318f1 100644 --- a/gsec/src/main/java/gemma/gsec/model/Securable.java +++ b/gsec/src/main/java/gemma/gsec/model/Securable.java @@ -19,7 +19,10 @@ package gemma.gsec.model; /** - * Interface that indicates an entity can be secured. By default, permissions are inherited by associated objects. + * Interface that indicates an entity can be secured. + *

      + * Securables have ACLs associated with them and may inherit permissions from parent securables (see {@link SecuredChild}), or + * not (see {@link SecuredNotChild}). * * @author paul * @version $Id: Securable.java,v 1.4 2013/03/16 00:39:24 paul Exp $ diff --git a/gsec/src/main/java/gemma/gsec/model/SecuredChild.java b/gsec/src/main/java/gemma/gsec/model/SecuredChild.java index eb71f66..0d26387 100644 --- a/gsec/src/main/java/gemma/gsec/model/SecuredChild.java +++ b/gsec/src/main/java/gemma/gsec/model/SecuredChild.java @@ -18,14 +18,16 @@ */ package gemma.gsec.model; +import javax.annotation.Nullable; + /** - * Indicates a securable that must have a parent that holds the permissons. For example, BioAssays are given the same - * permissions as the holding Experiment, and no object should have the BioAssay's ACL as its parent. + * Indicates a {@link Securable} must have a parent from which it inherits permissions. * * @author paul * @version $Id: SecuredChild.java,v 1.3 2013/03/16 00:39:24 paul Exp $ */ public interface SecuredChild extends Securable { + @Nullable Securable getSecurityOwner(); } diff --git a/gsec/src/main/java/gemma/gsec/model/SecuredNotChild.java b/gsec/src/main/java/gemma/gsec/model/SecuredNotChild.java index 0904b3c..48d57e7 100644 --- a/gsec/src/main/java/gemma/gsec/model/SecuredNotChild.java +++ b/gsec/src/main/java/gemma/gsec/model/SecuredNotChild.java @@ -19,8 +19,7 @@ package gemma.gsec.model; /** - * Interface to mark entities which are secured, and which should not have 'parent's, and therefore do not inherit - * permissions from other objects. + * Indicates that a {@link Securable} cannot have a parent. * * @author paul * @version $Id: SecuredNotChild.java,v 1.2 2009/11/23 20:26:42 paul Exp $