diff --git a/gsec/src/main/java/gemma/gsec/acl/domain/AclDao.java b/gsec/src/main/java/gemma/gsec/acl/domain/AclDao.java index 74d24ac..a2c6e6b 100644 --- a/gsec/src/main/java/gemma/gsec/acl/domain/AclDao.java +++ b/gsec/src/main/java/gemma/gsec/acl/domain/AclDao.java @@ -15,12 +15,11 @@ package gemma.gsec.acl.domain; import org.springframework.security.acls.jdbc.LookupStrategy; -import org.springframework.security.acls.model.MutableAcl; -import org.springframework.security.acls.model.ObjectIdentity; -import org.springframework.security.acls.model.Sid; +import org.springframework.security.acls.model.Acl; +import org.springframework.security.acls.model.ChildrenExistException; +import javax.annotation.CheckReturnValue; import javax.annotation.Nullable; -import java.io.Serializable; import java.util.List; /** @@ -29,22 +28,56 @@ */ public interface AclDao extends LookupStrategy { - AclObjectIdentity createObjectIdentity( String type, Serializable identifier, Sid sid, boolean entriesInheriting ); + /** + * Find an ACL object identity confirming to the given object identity. + *

+ * If the provided object as a non-null ID, it is used, otherwise the type and identifier is used. + */ + @Nullable + AclObjectIdentity findObjectIdentity( AclObjectIdentity objectIdentity ); - void delete( ObjectIdentity objectIdentity, boolean deleteChildren ); + /** + * Find all the children of the given object identity. + */ + List findChildren( AclObjectIdentity parentIdentity ); - void delete( Sid sid ); + /** + * Create a new object identity. + */ + @CheckReturnValue + AclObjectIdentity createObjectIdentity( AclObjectIdentity oid ); - @Nullable - AclObjectIdentity find( ObjectIdentity oid ); + /** + * Update a given object identity so that it conforms to a given ACL object. + */ + void updateObjectIdentity( AclObjectIdentity aclObjectIdentity, Acl acl ); - @Nullable - AclSid find( Sid sid ); + /** + * Delete a given object identity. + * + * @param deleteChildren if true, the children are recursively deleted as well + * @throws ChildrenExistException if deleteChildren is false and there are children associated to the object + * identity, those must be removed beforehand + */ + void deleteObjectIdentity( AclObjectIdentity objectIdentity, boolean deleteChildren ) throws ChildrenExistException; - List findChildren( ObjectIdentity parentIdentity ); - - AclSid findOrCreate( Sid sid ); + /** + * Retrieve a SID conforming to the given object. + *

+ * If the provided object as a non-null ID, it is used, otherwise either the principal or granted authority is used + * depending on the type. + */ + @Nullable + AclSid findSid( AclSid sid ); - void update( MutableAcl acl ); + /** + * Create a given SID. + */ + @CheckReturnValue + AclSid createSid( AclSid sid ); + /** + * Delete a given SID. + */ + void deleteSid( AclSid sid ); } diff --git a/gsec/src/main/java/gemma/gsec/acl/domain/AclDaoImpl.java b/gsec/src/main/java/gemma/gsec/acl/domain/AclDaoImpl.java index b5e9eb8..3349d52 100644 --- a/gsec/src/main/java/gemma/gsec/acl/domain/AclDaoImpl.java +++ b/gsec/src/main/java/gemma/gsec/acl/domain/AclDaoImpl.java @@ -16,20 +16,15 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.hibernate.FlushMode; -import org.hibernate.Session; +import org.hibernate.Hibernate; +import org.hibernate.ObjectNotFoundException; import org.hibernate.SessionFactory; -import org.hibernate.criterion.Criterion; -import org.hibernate.criterion.Order; -import org.hibernate.criterion.Restrictions; -import org.hibernate.sql.JoinType; -import org.springframework.security.acls.domain.AclAuthorizationStrategy; +import org.springframework.security.acls.domain.*; import org.springframework.security.acls.model.*; +import org.springframework.security.util.FieldUtils; import org.springframework.util.Assert; import org.springframework.util.StringUtils; -import javax.persistence.EntityNotFoundException; -import java.io.Serializable; import java.util.*; import static java.util.Objects.requireNonNull; @@ -51,139 +46,160 @@ public class AclDaoImpl implements AclDao { private final SessionFactory sessionFactory; private final AclAuthorizationStrategy aclAuthorizationStrategy; - private final AclCache aclCache; + private final PermissionGrantingStrategy permissionGrantingStrategy; + private final PermissionFactory permissionFactory = new DefaultPermissionFactory(); - public AclDaoImpl( SessionFactory sessionFactory, AclAuthorizationStrategy aclAuthorizationStrategy, AclCache aclCache ) { + public AclDaoImpl( SessionFactory sessionFactory, AclAuthorizationStrategy aclAuthorizationStrategy, PermissionGrantingStrategy permissionGrantingStrategy ) { this.sessionFactory = sessionFactory; this.aclAuthorizationStrategy = aclAuthorizationStrategy; - this.aclCache = aclCache; + this.permissionGrantingStrategy = permissionGrantingStrategy; } @Override - public AclObjectIdentity createObjectIdentity( String type, Serializable identifier, Sid sid, boolean entriesInheriting ) { - Assert.isInstanceOf( AclSid.class, sid ); - AclObjectIdentity aoi = new AclObjectIdentity( type, ( Long ) identifier ); - aoi.setOwnerSid( sid ); - aoi.setEntriesInheriting( entriesInheriting ); + public AclObjectIdentity findObjectIdentity( AclObjectIdentity oid ) { + // fast path if we know the ID + if ( oid.getId() != null ) { + return ( AclObjectIdentity ) sessionFactory.getCurrentSession() + .get( AclObjectIdentity.class, oid.getId() ); + } + Assert.notNull( oid.getType() ); + Assert.notNull( oid.getIdentifier() ); + return ( AclObjectIdentity ) sessionFactory.getCurrentSession() + .byNaturalId( AclObjectIdentity.class ) + .using( "type", oid.getType() ) + .using( "identifier", oid.getIdentifier() ) + .load(); + } + + + @Override + public AclObjectIdentity createObjectIdentity( AclObjectIdentity aoi ) { + Assert.isNull( aoi.getId() ); sessionFactory.getCurrentSession().persist( aoi ); - log.trace( String.format( "Created %s", aoi ) ); + log.trace( "Created " + aoi ); return aoi; } @Override - public void delete( ObjectIdentity objectIdentity, boolean deleteChildren ) { - Assert.isInstanceOf( AclObjectIdentity.class, objectIdentity ); - + public void deleteObjectIdentity( AclObjectIdentity objectIdentity, boolean deleteChildren ) throws ChildrenExistException { + Assert.notNull( objectIdentity.getId() ); if ( deleteChildren ) { - List c = findChildren( objectIdentity ); - for ( ObjectIdentity cc : c ) { - delete( cc, true ); - } + deleteObjectIdentityAndChildren( objectIdentity, 0 ); + } else if ( !hasChildren( objectIdentity ) ) { + sessionFactory.getCurrentSession().delete( objectIdentity ); + log.trace( String.format( "Deleted %s", objectIdentity ) ); + } else { + throw new ChildrenExistException( objectIdentity + " has children, it cannot be deleted unless deleteChildren is set to true or the children are removed beforehand." ); } + sessionFactory.getCurrentSession().flush(); + } - // must do this first... - this.evictFromCache( objectIdentity ); + private boolean hasChildren( AclObjectIdentity aclObjectIdentity ) { + return ( Boolean ) sessionFactory.getCurrentSession() + .createQuery( "select count(*) > 0 from AclObjectIdentity where parentObject = :po" ) + .setParameter( "po", aclObjectIdentity ) + .uniqueResult(); + } + private void deleteObjectIdentityAndChildren( AclObjectIdentity objectIdentity, int indentSize ) { + String indent = String.join( "", Collections.nCopies( indentSize, "\t" ) ); + log.trace( String.format( indent + "Deleted %s", objectIdentity ) ); + List c = findChildren( objectIdentity ); + for ( AclObjectIdentity cc : c ) { + deleteObjectIdentityAndChildren( cc, indentSize + 1 ); + } sessionFactory.getCurrentSession().delete( objectIdentity ); - log.trace( String.format( "Deleted %s", objectIdentity ) ); } @Override - public void delete( Sid sid ) { - Assert.isInstanceOf( AclSid.class, sid ); - - Sid toDelete = this.find( sid ); - - if ( toDelete == null ) { - log.warn( "No such sid: " + sid ); - return; - } - - // delete any objectidentities owned - Session session = sessionFactory.getCurrentSession(); - List ownedOis = session - .createQuery( "select s from AclObjectIdentity oi join oi.ownerSid s where s = :sid " ) - .setParameter( "sid", toDelete ).list(); - - if ( !ownedOis.isEmpty() ) { - for ( Object oi : ownedOis ) { - this.evictFromCache( ( ObjectIdentity ) oi ); - session.delete( oi ); - } + public void deleteSid( AclSid sid ) { + Assert.notNull( sid.getId() ); + + // delete any ACL entries referring to this sid + int deletedEntries = sessionFactory.getCurrentSession() + .createQuery( "delete from AclEntry e where e.sid = :sid" ) + .setParameter( "sid", sid ) + .executeUpdate(); + if ( deletedEntries > 0 ) { + log.trace( String.format( "Deleted %d ACL entries owned by %s", deletedEntries, sid ) ); } - // delete any aclentries referring to this sid - List entries = session.createQuery( "select e from AclEntry e where e.sid = :sid" ) - .setParameter( "sid", toDelete ).list(); - - for ( Object e : entries ) { - session.delete( e ); + // delete any object identities owned by the sid + int deletedOis = sessionFactory.getCurrentSession() + .createQuery( "delete from AclObjectIdentity oi where oi.ownerSid = :sid" ) + .setParameter( "sid", sid ) + .executeUpdate(); + if ( deletedOis > 0 ) { + log.trace( String.format( "Deleted %d ACL object identities owned by %s", deletedOis, sid ) ); } - session.delete( toDelete ); + // now we can safely delete the sid + sessionFactory.getCurrentSession().delete( sid ); + log.trace( "Delete: " + sid ); } @Override - public AclObjectIdentity find( ObjectIdentity oid ) { - Assert.isInstanceOf( AclObjectIdentity.class, oid ); - return ( AclObjectIdentity ) sessionFactory.getCurrentSession() - .createQuery( "from AclObjectIdentity where type=:t and identifier=:i" ) - .setParameter( "t", oid.getType() ) - .setParameter( "i", oid.getIdentifier() ) - - .uniqueResult(); - } - - @Override - public AclSid find( Sid sid ) { - Assert.isInstanceOf( AclSid.class, sid ); + public AclSid findSid( AclSid sid ) { if ( sid instanceof AclPrincipalSid ) { AclPrincipalSid p = ( AclPrincipalSid ) sid; + if ( sid.getId() != null ) { + return ( AclSid ) sessionFactory.getCurrentSession().get( AclPrincipalSid.class, p.getId() ); + } + Assert.notNull( p.getPrincipal() ); return ( AclSid ) sessionFactory.getCurrentSession() - .createQuery( "from AclPrincipalSid where principal = :p" ) - .setParameter( "p", p.getPrincipal() ) - .uniqueResult(); + .createQuery( "from AclPrincipalSid where principal = :p" ) + .setParameter( "p", p.getPrincipal() ) + .uniqueResult(); } else if ( sid instanceof AclGrantedAuthoritySid ) { AclGrantedAuthoritySid g = ( AclGrantedAuthoritySid ) sid; + if ( sid.getId() != null ) { + return ( AclSid ) sessionFactory.getCurrentSession().get( AclGrantedAuthoritySid.class, g.getId() ); + } + Assert.notNull( g.getGrantedAuthority() ); return ( AclSid ) sessionFactory.getCurrentSession() - .createQuery( "from AclGrantedAuthoritySid where grantedAuthority = :g" ) - .setParameter( "g", g.getGrantedAuthority() ) - .uniqueResult(); + .createQuery( "from AclGrantedAuthoritySid where grantedAuthority = :g" ) + .setParameter( "g", g.getGrantedAuthority() ) + .uniqueResult(); } else { throw new IllegalArgumentException( "Unsupported ACL SID type: " + sid.getClass() ); } } @Override - public List findChildren( ObjectIdentity parentIdentity ) { - Assert.isInstanceOf( AclObjectIdentity.class, parentIdentity ); - Assert.notNull( parentIdentity, "ParentIdentity cannot be null" ); - - ObjectIdentity oi = this.find( parentIdentity ); - - if ( oi == null ) { - throw new EntityNotFoundException( "Failed to find: " + parentIdentity ); - } - - //noinspection unchecked - return sessionFactory.getCurrentSession() + public List findChildren( AclObjectIdentity parentIdentity ) { + // fast path if we know the ID + if ( parentIdentity.getId() != null ) { + //noinspection unchecked + return sessionFactory.getCurrentSession() .createQuery( "from AclObjectIdentity o where o.parentObject = :po" ) - .setParameter( "po", parentIdentity ).list(); + .setParameter( "po", parentIdentity ) + .list(); + } else { + Assert.notNull( parentIdentity.getType() ); + Assert.notNull( parentIdentity.getIdentifier() ); + //noinspection unchecked + return sessionFactory.getCurrentSession() + .createQuery( "from AclObjectIdentity o where o.parentObject.type = :type and o.parentObject.identifier = :identifier" ) + .setParameter( "type", parentIdentity.getType() ) + .setParameter( "identifier", parentIdentity.getIdentifier() ) + .list(); + } } @Override - public AclSid findOrCreate( Sid sid ) { - Assert.isInstanceOf( AclSid.class, sid ); - - AclSid fsid = this.find( sid ); - - if ( fsid != null ) return fsid; - + public AclSid createSid( AclSid sid ) { + Assert.isNull( sid.getId() ); + if ( sid instanceof AclPrincipalSid ) { + Assert.notNull( ( ( AclPrincipalSid ) sid ).getPrincipal() ); + } else if ( sid instanceof AclGrantedAuthoritySid ) { + Assert.notNull( ( ( AclGrantedAuthoritySid ) sid ).getGrantedAuthority() ); + } else { + throw new IllegalArgumentException( "Unsupported SID type " + sid.getClass().getName() ); + } sessionFactory.getCurrentSession().persist( sid ); - - return ( AclSid ) sid; - + log.trace( "Created " + sid ); + return sid; } /** @@ -195,310 +211,205 @@ public AclSid findOrCreate( Sid sid ) { public Map readAclsById( List objects, List sids ) { Assert.notEmpty( objects, "Objects to lookup required" ); - Map result = new HashMap<>(); - - Set aclsToLoad = new HashSet<>(); + // must use a list, otherwise the proxies will be initialized + List reloadedAclOids = new ArrayList<>(); for ( ObjectIdentity oid : objects ) { Assert.isInstanceOf( AclObjectIdentity.class, oid ); - - if ( result.containsKey( oid ) ) { - continue; - } - - // Check we don't already have this ACL in the results - - // Check cache for the present ACL entry - Acl acl = aclCache.getFromCache( oid ); - // Ensure any cached element supports all the requested SIDs - // (they should always, as our base impl doesn't filter on SID) - if ( acl != null ) { - result.put( acl.getObjectIdentity(), acl ); - continue; + AclObjectIdentity aclOid = ( AclObjectIdentity ) oid; + AclObjectIdentity reloadedAclOid; + if ( aclOid.getId() != null ) { + // load it from the first/second-level cache + reloadedAclOid = ( AclObjectIdentity ) sessionFactory.getCurrentSession() + .load( AclObjectIdentity.class, aclOid.getId() ); + } else { + // load it from the first/second-level cache using a natural ID (i.e. type + identifier) + Assert.notNull( aclOid.getType() ); + Assert.notNull( aclOid.getIdentifier() ); + reloadedAclOid = ( AclObjectIdentity ) sessionFactory.getCurrentSession() + .byNaturalId( AclObjectIdentity.class ) + .using( "type", aclOid.getType() ) + .using( "identifier", aclOid.getIdentifier() ) + .getReference(); + if ( reloadedAclOid == null ) { + // FIXME: for some reason, natural ID retrieval may return null without producing an invalid proxy + log.trace( "Failed to load an ACL object identity from persistent storage: " + + new ObjectNotFoundException( aclOid.getType() + ":" + aclOid.getIdentifier(), AclObjectIdentity.class.getName() ).getMessage() ); + continue; + } } - - aclsToLoad.add( ( AclObjectIdentity ) oid ); + reloadedAclOids.add( reloadedAclOid ); } - if ( !aclsToLoad.isEmpty() ) { - Map loadedBatch = loadAcls( aclsToLoad ); - - // Add loaded batch (all elements 100% initialized) to results - result.putAll( loadedBatch ); - - // Add the loaded batch to the cache - for ( Acl loadedAcl : loadedBatch.values() ) { - aclCache.putInCache( ( MutableAcl ) loadedAcl ); + // Hibernate uses batch-fetching for proxies, and we've set batch-size in AclObjectIdentity.hbm.xml, so this + // should be relatively efficient + Iterator iterator = reloadedAclOids.iterator(); + while ( iterator.hasNext() ) { + AclObjectIdentity reloadedAclOid = iterator.next(); + try { + Hibernate.initialize( reloadedAclOid ); + } catch ( ObjectNotFoundException e ) { + log.trace( "Failed to load an ACL object identity from persistent storage: " + e.getMessage() ); + iterator.remove(); } } - return result; + // Add loaded batch (all elements 100% initialized) to results + return loadAcls( reloadedAclOids ); } /** * This is an important method, and one that causes problems in the default JDBC-based service from spring-security. */ @Override - public void update( MutableAcl acl ) { + public void updateObjectIdentity( AclObjectIdentity aclObjectIdentity, Acl acl ) { + Assert.isInstanceOf( AclObjectIdentity.class, acl.getObjectIdentity() ); + Assert.isInstanceOf( AclSid.class, acl.getOwner() ); if ( log.isTraceEnabled() ) - log.trace( ">>>>>>>>>> starting database update of acl for: " + acl.getObjectIdentity() ); - /* - * This fixes problems with premature commits causing IDs to be erased on some entities. - */ - sessionFactory.getCurrentSession().setFlushMode( FlushMode.COMMIT ); - - AclObjectIdentity aclObjectIdentity = convert( acl ); - - // the ObjectIdentity might already be in the session. - aclObjectIdentity = ( AclObjectIdentity ) sessionFactory.getCurrentSession() - .merge( aclObjectIdentity ); - - if ( acl.getParentAcl() != null ) { - - if ( log.isTraceEnabled() ) - log.trace( " Updating ACL on parent: " + acl.getParentAcl().getObjectIdentity() ); - - update( ( MutableAcl ) acl.getParentAcl() ); - aclObjectIdentity.setParentObject( convert( ( MutableAcl ) acl.getParentAcl() ) ); - assert aclObjectIdentity.getParentObject() != null; - } else { - // should be impossible to go from non-null to null, but just in case ... - assert aclObjectIdentity.getParentObject() == null; - } - + log.trace( ">>>>>>>>>> Starting database update of ACL for: " + acl.getObjectIdentity() ); + updateObjectIdentity( aclObjectIdentity, acl, 0 ); + // persist the changes immediately sessionFactory.getCurrentSession().update( aclObjectIdentity ); - - evictFromCache( aclObjectIdentity ); - - // children are left out, no big deal. Eviction more important. - this.aclCache.putInCache( convertToAcl( aclObjectIdentity ) ); - if ( log.isTraceEnabled() ) - log.trace( " >>>>>>>>>> Done with database update of acl for: " + acl.getObjectIdentity() ); + log.trace( ">>>>>>>>>> Done with database update of ACL for: " + acl.getObjectIdentity() ); } /** - * @return synched-up and partly updated AclObjectIdentity + * Recursively update an ACL object identity and their parents, if any. */ - private AclObjectIdentity convert( MutableAcl acl ) { - assert acl instanceof AclImpl; - assert acl.getObjectIdentity() instanceof AclObjectIdentity; - - // these come back with the ace_order filled in. - List entriesFromAcl = acl.getEntries(); - - // repopulate the ID of the SIDs. May not have any if this is a secured child. - if ( log.isTraceEnabled() && !entriesFromAcl.isEmpty() ) - log.trace( "Preparing to update " + entriesFromAcl.size() + " aces on " + acl.getObjectIdentity() ); - for ( AccessControlEntry ace : entriesFromAcl ) { - if ( log.isTraceEnabled() ) log.trace( ace ); - AclSid sid = ( AclSid ) ace.getSid(); - if ( sid.getId() == null ) { - AclEntry aace = ( AclEntry ) ace; - aace.setSid( this.findOrCreate( sid ) ); - } - } + private void updateObjectIdentity( AclObjectIdentity aclObjectIdentity, Acl acl, int indentSize ) { + Assert.isInstanceOf( AclObjectIdentity.class, acl.getObjectIdentity() ); + Assert.isInstanceOf( AclSid.class, acl.getOwner() ); + Assert.notNull( ( ( AclSid ) acl.getOwner() ).getId() ); + Assert.isTrue( indentSize >= 0 ); - // synched up with the ACL, partly - AclObjectIdentity aclObjectIdentity = ( AclObjectIdentity ) acl.getObjectIdentity(); + String indent = String.join( "", Collections.nCopies( indentSize, "\t" ) ); - if ( aclObjectIdentity.getOwnerSid().getId() == null ) { - aclObjectIdentity.setOwnerSid( requireNonNull( this.find( acl.getOwner() ), - String.format( "Failed to locate owner SID %s for %s", aclObjectIdentity.getOwnerSid(), aclObjectIdentity ) ) ); - } + log.trace( indent + "Updating ACL on " + aclObjectIdentity ); - assert aclObjectIdentity.getOwnerSid() != null; - - /* - * Update the collection of entries. - */ - Collection entriesToUpdate = aclObjectIdentity.getEntries(); - entriesToUpdate.clear(); - for ( AccessControlEntry accessControlEntry : entriesFromAcl ) { - entriesToUpdate.add( ( AclEntry ) accessControlEntry ); + // update sids and entries of the main object + if ( !Objects.equals( aclObjectIdentity.getOwnerSid(), acl.getOwner() ) ) { + if ( log.isTraceEnabled() ) { + log.trace( indent + String.format( "Owner of %s is being changed from %s to %s", aclObjectIdentity, + aclObjectIdentity.getOwnerSid(), acl.getOwner() ) ); + } } + aclObjectIdentity.setOwnerSid( acl.getOwner() ); + aclObjectIdentity.setEntriesInheriting( acl.isEntriesInheriting() ); - return aclObjectIdentity; - } + int i = 0; + List entriesFromAcl = new ArrayList<>( acl.getEntries().size() ); + for ( AccessControlEntry accessControlEntry : acl.getEntries() ) { + AclEntry entry; + if ( accessControlEntry.getId() != null ) { + // this is cheap because it is already loaded + entry = ( AclEntry ) requireNonNull( sessionFactory.getCurrentSession().get( AclEntry.class, accessControlEntry.getId() ), + "The following ACL entry count not be found: " + accessControlEntry ); + } else { + entry = new AclEntry(); + } + entry.setSid( ( AclSid ) acl.getOwner() ); + entry.setMask( accessControlEntry.getPermission().getMask() ); + entry.setGranting( accessControlEntry.isGranting() ); + entry.setAceOrder( i++ ); + entriesFromAcl.add( entry ); + } - /** - * Does not check the cache; - */ - private MutableAcl convertToAcl( AclObjectIdentity oi ) { - return new AclImpl( oi, aclAuthorizationStrategy, oi.getParentObject() != null ? convertToAcl( oi.getParentObject() ) : null ); - } + // update the collection of entries. + aclObjectIdentity.getEntries().clear(); + aclObjectIdentity.getEntries().addAll( entriesFromAcl ); - /** - * ... including children, recursively. - */ - private void evictFromCache( ObjectIdentity aclObjectIdentity ) { - Assert.notNull( aclObjectIdentity, "aclObjectIdentity cannot be null" ); + // parent likely went from null -> non-null, update it + if ( acl.getParentAcl() != null ) { + if ( log.isTraceEnabled() ) + log.trace( indent + "Updating ACL on parent: " + acl.getParentAcl().getObjectIdentity() ); + AclObjectIdentity parentOi = ( AclObjectIdentity ) acl.getParentAcl().getObjectIdentity(); + // recursively update the parents + updateObjectIdentity( parentOi, acl.getParentAcl(), indentSize + 1 ); + aclObjectIdentity.setParentObject( parentOi ); + assert aclObjectIdentity.getParentObject() != null; + } - this.aclCache.evictFromCache( aclObjectIdentity ); - for ( ObjectIdentity c : this.findChildren( aclObjectIdentity ) ) { - evictFromCache( c ); + // parent went from non-null to null, unset it + else if ( aclObjectIdentity.getParentObject() != null ) { + if ( log.isTraceEnabled() ) + log.trace( indent + "Unsetting parent ACL of: " + aclObjectIdentity ); + aclObjectIdentity.setParentObject( null ); } } /** - * Looks up a batch of ObjectIdentitys directly from the database, when we only know the type and - * object's id (not the objectIdentity PK). + * Load a batch of {@link AclObjectIdentity} into a mapping of {@link ObjectIdentity} to {@link Acl}. *

- * The caller is responsible for optimization issues, such as selecting the identities to lookup, ensuring the cache - * doesn't contain them already, and adding the returned elements to the cache etc. - *

- * This is required to return fully valid Acls, including properly-configured parent ACLs. - * - * @param objectIdentities a batch of OIs to fetch ACLs for. + * Due to the way {@link AclObjectIdentity} are mapped with Hibernate, they are already fully initialized, so this + * method will proceed to populate the ACLs and their parents recursively. */ private Map loadAcls( final Collection objectIdentities ) { - final Map results = new HashMap<>(); - - // group by type so we can use in (...) clauses - Map> idsByType = new HashMap<>(); - for ( ObjectIdentity oi : objectIdentities ) { - idsByType.computeIfAbsent( oi.getType(), k -> new HashSet<>() ) - .add( oi.getIdentifier() ); - } - - Criterion[] clauses = new Criterion[idsByType.size()]; - int i = 0; - for ( Map.Entry> e : idsByType.entrySet() ) { - clauses[i++] = Restrictions.and( - Restrictions.eq( "type", e.getKey() ), - Restrictions.in( "identifier", e.getValue() ) ); - } - - //noinspection unchecked - List queryR = sessionFactory.getCurrentSession() - .createCriteria( AclObjectIdentity.class ) - // possibly has no entries yet, so left outer join? - .createAlias( "entries", "e", JoinType.LEFT_OUTER_JOIN ) - .add( Restrictions.or( clauses ) ) - .addOrder( Order.asc( "identifier" ) ) - .addOrder( Order.asc( "e.aceOrder" ) ) - .list(); - - // this is okay if we haven't added the objects yet. - // if ( queryR.size() < objectIdentities.size() ) { - // log.warn( "Expected " + objectIdentities.size() + " objectidentities from db, got " + queryR.size() - // + " from db" ); - // } - - Set parentIdsToLookup = new HashSet<>(); - for ( AclObjectIdentity oi : queryR ) { - - AclImpl parentAcl = null; - AclObjectIdentity parentObjectIdentity = oi.getParentObject(); - - if ( parentObjectIdentity != null ) { - - if ( !results.containsKey( parentObjectIdentity.getId() ) ) { - - // try to find parent in the cache - MutableAcl cachedParent = aclCache.getFromCache( parentObjectIdentity.getId() ); - - if ( cachedParent == null ) { - - parentIdsToLookup.add( parentObjectIdentity.getId() ); - - parentAcl = new AclImpl( parentObjectIdentity, aclAuthorizationStrategy, /* parent acl */null ); - - parentAcl.getEntries() - .addAll( AclEntry.convert( new ArrayList<>( oi.getEntries() ), parentAcl ) ); - } else { - parentAcl = ( AclImpl ) cachedParent; - - // Pop into the acls map, so our convert method doesn't - // need to deal with an unsynchronized AclCache, even though it might not be used directly. - - results.put( cachedParent.getId(), cachedParent ); - } - - } else { - parentAcl = ( AclImpl ) results.get( parentObjectIdentity.getId() ); - } - - assert parentAcl != null; - } - - Acl acl = new AclImpl( oi, aclAuthorizationStrategy, parentAcl ); - - results.put( oi.getId(), acl ); - - } - - // Lookup the parents - if ( parentIdsToLookup.size() > 0 ) { - loadAcls( results, parentIdsToLookup ); - } - - Map resultMap = new HashMap<>(); - for ( Acl inputAcl : results.values() ) { - resultMap.put( inputAcl.getObjectIdentity(), inputAcl ); - } - return resultMap; + Map acls = new HashMap<>(); + loadAcls( objectIdentities, acls ); + return acls; } /** - * Load ACLs when we know the primary key of the objectIdentity. Recursively fill in the parentAcls. + * Load ACLs when we know the primary key of the objectIdentity. Recursively fill in the parent ACLs. * - * @param acls the starting set of acls. - * @param objectIdentityIds primary keys of the object identities to be fetched. If these yield acls that are the - * parents of the given acls, they will be populated. + * @param objectIdentities primary keys of the object identities to be fetched. If these yield acls that are the + * parents of the given acls, they will be populated. + * @param acls the starting set of acls. */ - private void loadAcls( final Map acls, final Set objectIdentityIds ) { + private void loadAcls( final Collection objectIdentities, final Map acls ) { Assert.notNull( acls, "ACLs are required" ); - Assert.notEmpty( objectIdentityIds, "Items to find now required" ); - //noinspection unchecked - List queryR = sessionFactory.getCurrentSession() - .createQuery( "from AclObjectIdentity where id in (:ids)" ) - .setParameterList( "ids", objectIdentityIds ) - .list(); + if ( objectIdentities.isEmpty() ) { + return; + } - Set parentsToLookup = new HashSet<>(); - for ( AclObjectIdentity o : queryR ) { + Set parentAclsToLoad = new HashSet<>(); + for ( AclObjectIdentity o : objectIdentities ) { + MutableAcl acl = new AclImpl( o, o.getId(), aclAuthorizationStrategy, permissionGrantingStrategy, + null /* parents are filled later */, null, o.getEntriesInheriting(), o.getOwnerSid() ); + List orderedEntries = new ArrayList<>( o.getEntries() ); + orderedEntries.sort( Comparator.comparing( AclEntry::getAceOrder ) ); + List aces = new ArrayList<>( o.getEntries().size() ); + for ( AclEntry e : orderedEntries ) { + aces.add( new AccessControlEntryImpl( e.getId(), acl, e.getSid(), permissionFactory.buildFromMask( e.getMask() ), e.isGranting(), false, false ) ); + } + // unfortunately, there's no way to pass a pre-constructed list of ACEs to Spring ACL implementation without + // going through insertAce() + FieldUtils.setProtectedFieldValue( "aces", acl, aces ); + // we call setParent() later if ( o.getParentObject() != null ) { - assert !o.getParentObject().getId().equals( o.getId() ); - parentsToLookup.add( o.getParentObject().getId() ); + Acl parentAcl = acls.get( o.getParentObject() ); + if ( parentAcl != null ) { + acl.setParent( parentAcl ); + } else { + parentAclsToLoad.add( o.getParentObject() ); + } } - Acl acl = new AclImpl( o, aclAuthorizationStrategy, /* parentacl, to be filled in later */null ); - acls.put( o.getId(), acl ); + acls.put( o, acl ); } + // load the parent ACLs (recursively) + loadAcls( parentAclsToLoad, acls ); + /* * Fill in the parent Acls for the acls that need them. */ - for ( Long oiid : objectIdentityIds ) { + for ( AclObjectIdentity oiid : objectIdentities ) { MutableAcl acl = ( MutableAcl ) acls.get( oiid ); - if ( acl.getParentAcl() != null ) { // we already did it. continue; } - - ObjectIdentity oi = acl.getObjectIdentity(); - AclObjectIdentity aoi = ( AclObjectIdentity ) oi; - - if ( aoi.getParentObject() != null ) { + if ( oiid.getParentObject() != null ) { + Acl parentAcl = acls.get( oiid.getParentObject() ); // this used to be an assertion, source of failures not clear... - if ( !acls.containsKey( aoi.getParentObject().getId() ) ) { + if ( parentAcl == null ) { throw new IllegalStateException( - "ACLs did not contain key for parent object identity of " + aoi + "( parent = " + aoi.getParentObject() + "); " - + "ACLs being inspected: " + StringUtils.collectionToDelimitedString( acls.values(), "\n" ) ); + "ACLs did not contain key for parent object identity of " + oiid + "( parent = " + oiid.getParentObject() + "); " + + "ACLs being inspected: " + StringUtils.collectionToDelimitedString( acls.values(), "\n" ) ); } - // end assertion - - Acl parentAcl = acls.get( aoi.getParentObject().getId() ); - assert !acl.equals( parentAcl ); acl.setParent( parentAcl ); } } - - // Recurse; Lookup the parents of the newly fetched Acls. - if ( parentsToLookup.size() > 0 ) { - loadAcls( acls, parentsToLookup ); - } } - } diff --git a/gsec/src/main/java/gemma/gsec/acl/domain/AclEntry.java b/gsec/src/main/java/gemma/gsec/acl/domain/AclEntry.java index 2411761..3209ffd 100644 --- a/gsec/src/main/java/gemma/gsec/acl/domain/AclEntry.java +++ b/gsec/src/main/java/gemma/gsec/acl/domain/AclEntry.java @@ -18,17 +18,10 @@ */ package gemma.gsec.acl.domain; -import org.springframework.security.acls.domain.AccessControlEntryImpl; import org.springframework.security.acls.domain.DefaultPermissionFactory; import org.springframework.security.acls.domain.PermissionFactory; -import org.springframework.security.acls.model.AccessControlEntry; -import org.springframework.security.acls.model.Acl; -import org.springframework.security.acls.model.Permission; -import org.springframework.security.acls.model.Sid; -import org.springframework.util.Assert; - -import java.util.ArrayList; -import java.util.List; + +import java.io.Serializable; import java.util.Objects; /** @@ -36,37 +29,23 @@ * * @author paul */ -public class AclEntry implements AccessControlEntry, Comparable { - - private static final PermissionFactory permissionFactory = new DefaultPermissionFactory(); +public class AclEntry implements Serializable, Comparable { /** * The serial version UID of this class. Needed for serialization. */ private static final long serialVersionUID = -4697361841061166973L; - /** - * @param acl to be associated with the AccessControlEntries - */ - public static List convert( List entries, Acl acl ) { - List result = new ArrayList<>(); - for ( AclEntry e : entries ) { - result.add( new AccessControlEntryImpl( e.id, acl, e.sid, permissionFactory.buildFromMask( e.mask ), - e.granting, false, false ) ); - } - return result; - } + private static final PermissionFactory permissionFactory = new DefaultPermissionFactory(); private Long id; - private Sid sid; + private AclSid sid; private boolean granting; private int mask; - private Acl acl; - private int aceOrder; @SuppressWarnings("unused") @@ -74,18 +53,7 @@ public AclEntry() { } - public AclEntry( Acl acl, Sid sid, Permission permission, boolean granting ) { - Assert.notNull( acl, "Acl required" ); - Assert.notNull( sid, "Sid required" ); - Assert.notNull( permission, "Permission required" ); - this.acl = acl; - this.sid = sid; - this.mask = permission.getMask(); - this.granting = granting; - } - - @Override public Long getId() { return this.id; } @@ -94,25 +62,14 @@ public void setId( Long id ) { this.id = id; } - @Override - public Permission getPermission() { - return permissionFactory.buildFromMask( mask ); - } - - public void setPermission( Permission permission ) { - this.mask = permission.getMask(); - } - - @Override - public Sid getSid() { + public AclSid getSid() { return this.sid; } - public void setSid( Sid sid ) { + public void setSid( AclSid sid ) { this.sid = sid; } - @Override public boolean isGranting() { return this.granting; } @@ -129,11 +86,6 @@ public void setMask( int mask ) { this.mask = mask; } - @Override - public Acl getAcl() { - return this.acl; - } - @SuppressWarnings("unused") public int getAceOrder() { return aceOrder; diff --git a/gsec/src/main/java/gemma/gsec/acl/domain/AclImpl.java b/gsec/src/main/java/gemma/gsec/acl/domain/AclImpl.java deleted file mode 100644 index 0b8c3c8..0000000 --- a/gsec/src/main/java/gemma/gsec/acl/domain/AclImpl.java +++ /dev/null @@ -1,378 +0,0 @@ -/* - * The gemma-mda project - * - * Copyright (c) 2013 University of British Columbia - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ -package gemma.gsec.acl.domain; - -import org.springframework.security.acls.domain.AclAuthorizationStrategy; -import org.springframework.security.acls.model.*; -import org.springframework.util.Assert; - -import javax.annotation.Nullable; -import java.io.Serializable; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; - -/** - * Represents an access control list (ACL) for a domain object. Based on spring-security AclImpl. - * - * @author Paul - * @version $Id: AclImpl.java,v 1.1 2013/09/14 16:55:20 paul Exp $ - */ -public class AclImpl implements OwnershipAcl { - - private static final long serialVersionUID = -953242274878593548L; - - private final transient AclAuthorizationStrategy aclAuthorizationStrategy; - private final List entries; - private final AclObjectIdentity objectIdentity; - @Nullable - private Acl parentAcl; - - /** - * Full constructor - * - * @param objectIdentity the object identity this ACL relates to (required) - * @param aclAuthorizationStrategy authorization strategy (required) - * @param parentAcl the parent (may be null) - */ - public AclImpl( AclObjectIdentity objectIdentity, AclAuthorizationStrategy aclAuthorizationStrategy, - @Nullable Acl parentAcl ) { - Assert.notNull( objectIdentity, "Object Identity required" ); - Assert.notNull( aclAuthorizationStrategy, "AclAuthorizationStrategy required" ); - Assert.notNull( objectIdentity.getOwnerSid(), "Owner required" ); - this.objectIdentity = objectIdentity; - this.entries = new ArrayList<>( objectIdentity.getEntries() ); - this.aclAuthorizationStrategy = aclAuthorizationStrategy; - this.parentAcl = parentAcl; // may be null - } - - @Override - public void deleteAce( int aceIndex ) throws NotFoundException { - aclAuthorizationStrategy.securityCheck( this, AclAuthorizationStrategy.CHANGE_GENERAL ); - verifyAceIndexExists( aceIndex ); - - synchronized ( entries ) { - this.entries.remove( aceIndex ); - } - } - - @Override - public boolean equals( Object obj ) { - if ( obj == null ) return false; - if ( this == obj ) return true; - if ( !( obj instanceof AclImpl ) ) return false; - - AclImpl rhs = ( AclImpl ) obj; - - if ( ( this.getId() == null && rhs.getId() == null ) || ( this.getId().equals( rhs.getId() ) ) ) { - if ( ( this.getOwner() == null && rhs.getOwner() == null ) || this.getOwner().equals( rhs.getOwner() ) ) { - if ( ( this.objectIdentity == null && rhs.objectIdentity == null ) - || ( Objects.equals( this.objectIdentity, rhs.objectIdentity ) ) ) { - if ( ( this.parentAcl == null && rhs.parentAcl == null ) - || ( Objects.equals( this.parentAcl, rhs.parentAcl ) ) ) { - - // if ( ( this.loadedSids == null && rhs.loadedSids == null ) ) { - // return true; - // } - // if ( this.loadedSids.size() == rhs.loadedSids.size() ) { - // for ( int i = 0; i < this.loadedSids.size(); i++ ) { - // if ( !this.loadedSids.get( i ).equals( rhs.loadedSids.get( i ) ) ) { - // return false; - // } - // } - // return true; - // } - return this.isEntriesInheriting() == rhs.isEntriesInheriting(); - } - - } - } - } - return false; - } - - @Override - public List getEntries() { - // populate the ace_order. - int i = 0; - for ( AclEntry e : entries ) { - e.setAceOrder( i++ ); - } - return new ArrayList<>( entries ); - } - - @Override - public Serializable getId() { - return this.objectIdentity.getId(); - } - - @Override - public ObjectIdentity getObjectIdentity() { - return objectIdentity; - } - - @Override - public Sid getOwner() { - return this.objectIdentity.getOwnerSid(); - } - - @Override - public Acl getParentAcl() { - return parentAcl; - } - - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ( ( this.getId() == null ? 0 : this.getId().hashCode() ) ); - result = prime * result + ( ( objectIdentity == null ) ? 0 : objectIdentity.hashCode() ); - return result; - } - - @Override - public void insertAce( int atIndexLocation, Permission permission, Sid sid, boolean granting ) - throws NotFoundException { - - assert this.parentAcl == null; - - aclAuthorizationStrategy.securityCheck( this, AclAuthorizationStrategy.CHANGE_GENERAL ); - Assert.notNull( permission, "Permission required" ); - Assert.notNull( sid, "Sid required" ); - if ( atIndexLocation < 0 ) { - throw new NotFoundException( "atIndexLocation must be greater than or equal to zero" ); - } - if ( atIndexLocation > this.entries.size() ) { - throw new NotFoundException( - "atIndexLocation must be less than or equal to the size of the AccessControlEntry collection" ); - } - - AclEntry ace = new AclEntry( this, sid, permission, granting ); - - int osize = this.entries.size(); - synchronized ( entries ) { - this.entries.add( atIndexLocation, ace ); - } - - assert this.entries.size() == osize + 1; - } - - @Override - public boolean isEntriesInheriting() { - return this.objectIdentity.getEntriesInheriting(); - } - - /** - * Determines authorization. The order of the permission and sid arguments is - * extremely important! The method will iterate through each of the permissions in the order - * specified. For each iteration, all of the sids will be considered, again in the order they are - * presented. A search will then be performed for the first {@link AccessControlEntry} object that directly matches - * that permission:sid combination. When the first full match is found (ie an ACE that has the - * SID currently being searched for and the exact permission bit mask being search for), the grant or deny flag for - * that ACE will prevail. If the ACE specifies to grant access, the method will return true. If the ACE - * specifies to deny access, the loop will stop and the next permission iteration will be performed. If - * each permission indicates to deny access, the first deny ACE found will be considered the reason for the failure - * (as it was the first match found, and is therefore the one most logically requiring changes - although not - * always). If absolutely no matching ACE was found at all for any permission, the parent ACL will be tried - * (provided that there is a parent and {@link #isEntriesInheriting()} is true. The parent ACL will - * also scan its parent and so on. If ultimately no matching ACE is found, a NotFoundException will be - * thrown and the caller will need to decide how to handle the permission check. Similarly, if any of the SID - * arguments presented to the method were not loaded by the ACL, UnloadedSidException will be thrown. - * - * @param permission the exact permissions to scan for (order is important) - * @param sids the exact SIDs to scan for (order is important) - * @param administrativeMode if true denotes the query is for administrative purposes and no auditing - * will be undertaken - * @return true if one of the permissions has been granted, false if one of the - * permissions has been specifically revoked - * @return false if an exact ACE for one of the permission bit masks and SID combination could not be - * found - * @throws UnloadedSidException if the passed SIDs are unknown to this ACL because the ACL was only loaded for a - * subset of SIDs - */ - @Override - public boolean isGranted( List permission, List sids, boolean administrativeMode ) - throws NotFoundException, UnloadedSidException { - Assert.notEmpty( permission, "Permissions required" ); - Assert.notEmpty( sids, "SIDs required" ); - - if ( !this.isSidLoaded( sids ) ) { - throw new UnloadedSidException( "ACL was not loaded for one or more SID" ); - } - - AccessControlEntry firstRejection = null; - - for ( Permission p : permission ) { - for ( Sid sid : sids ) { - assert sid instanceof AclSid; - - // Attempt to find exact match for this permission mask and SID - boolean scanNextSid = true; - - for ( AccessControlEntry ace : entries ) { - - assert ace instanceof AclEntry; - - if ( ( ace.getPermission().getMask() == p.getMask() ) && ace.getSid().equals( sid ) ) { - // Found a matching ACE, so its authorization decision will prevail - if ( ace.isGranting() ) { - // Success - return true; - } - - // Failure for this permission, so stop search - // We will see if they have a different permission - // (this permission is 100% rejected for this SID) - if ( firstRejection == null ) { - // Store first rejection for auditing reasons - firstRejection = ace; - } - - scanNextSid = false; // helps break the loop - - break; // exit aces loop - } - } - - if ( !scanNextSid ) { - break; // exit SID for loop (now try next permission) - } - } - } - - if ( firstRejection != null ) { - // We found an ACE to reject the request at this point, as no - // other ACEs were found that granted a different permission - - return false; - } - - // No matches have been found so far - if ( isEntriesInheriting() && ( parentAcl != null ) ) { - // We have a parent, so let them try to find a matching ACE - return parentAcl.isGranted( permission, sids, false ); - } - - // We either have no parent, or we're the uppermost parent; this can be expected. - // throw new NotFoundException( "Unable to locate a matching ACE for passed permissions and SIDs: " + this - // + "\nPermissions: " + StringUtils.join( permission, "," ) + "\nSIDs: " + StringUtils.join( sids, "," ) ); - // - return false; - } - - @Override - public boolean isSidLoaded( List sids ) { - // // If loadedSids is null, this indicates all SIDs were loaded - // // Also return true if the caller didn't specify a SID to find - // if ( ( this.loadedSids == null ) || ( sids == null ) || ( sids.size() == 0 ) ) { - // return true; - // } - // - // // This ACL applies to a SID subset only. Iterate to check it applies. - // for ( Sid sid : sids ) { - // boolean found = false; - // - // for ( Sid loadedSid : loadedSids ) { - // if ( sid.equals( loadedSid ) ) { - // // this SID is OK - // found = true; - // break; // out of loadedSids for loop - // } - // } - // - // if ( !found ) { - // return false; - // } - // } - - return true; - } - - @Override - public void setEntriesInheriting( boolean entriesInheriting ) { - aclAuthorizationStrategy.securityCheck( this, AclAuthorizationStrategy.CHANGE_GENERAL ); - objectIdentity.setEntriesInheriting( entriesInheriting ); - } - - @Override - public void setOwner( Sid newOwner ) { - aclAuthorizationStrategy.securityCheck( this, AclAuthorizationStrategy.CHANGE_OWNERSHIP ); - Assert.notNull( newOwner, "Owner required" ); - this.objectIdentity.setOwnerSid( newOwner ); - } - - @Override - public void setParent( @Nullable Acl newParent ) { - aclAuthorizationStrategy.securityCheck( this, AclAuthorizationStrategy.CHANGE_GENERAL ); - Assert.isTrue( newParent == null || !newParent.equals( this ), "Cannot be the parent of yourself: " + newParent ); - this.parentAcl = ( AclImpl ) newParent; - } - - @Override - public String toString() { - StringBuilder sb = new StringBuilder(); - sb.append( "AclImpl[" ); - sb.append( "id: " ).append( this.objectIdentity.getId() ).append( "; " ); - sb.append( "objectIdentity: " ).append( this.objectIdentity ).append( "; " ); - sb.append( "owner: " ).append( this.objectIdentity.getOwnerSid() ).append( "; " ); - - int count = 0; - - for ( AccessControlEntry ace : this.entries ) { - count++; - - if ( count == 1 ) { - sb.append( "\r\n" ); - } - - sb.append( ace ).append( "\r\n" ); - } - - if ( count == 0 ) { - sb.append( "no ACEs; " ); - } - - sb.append( "inheriting: " ).append( this.objectIdentity.getEntriesInheriting() ).append( "; " ); - sb.append( "parent: " ).append( - ( this.parentAcl == null ) ? "Null" : this.parentAcl.getObjectIdentity().toString() ); - sb.append( "; " ); - sb.append( "aclAuthorizationStrategy: " ).append( this.aclAuthorizationStrategy ).append( "; " ); - // sb.append( "auditLogger: " ).append( this.auditLogger ); - sb.append( "]" ); - - return sb.toString(); - } - - @Override - public void updateAce( int aceIndex, Permission permission ) throws NotFoundException { - aclAuthorizationStrategy.securityCheck( this, AclAuthorizationStrategy.CHANGE_GENERAL ); - verifyAceIndexExists( aceIndex ); - - synchronized ( entries ) { - AclEntry ace = entries.get( aceIndex ); - ace.setPermission( permission ); - } - } - - private void verifyAceIndexExists( int aceIndex ) { - if ( aceIndex < 0 ) { - throw new NotFoundException( "aceIndex must be greater than or equal to zero" ); - } - if ( aceIndex >= this.entries.size() ) { - throw new NotFoundException( "aceIndex must refer to an index of the AccessControlEntry list. " - + "List size is " + entries.size() + ", index was " + aceIndex ); - } - } -} 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..f239155 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; } @@ -168,6 +171,6 @@ public boolean equals( Object o ) { @Override public String toString() { - return String.format( "%s[Type: %s; Identifier: %s]", this.getClass().getName(), this.type, this.identifier ); + return String.format( "%s[Id: %d; Type: %s; Identifier: %s]", this.getClass().getName(), this.id, this.type, this.identifier ); } } diff --git a/gsec/src/main/java/gemma/gsec/acl/domain/AclServiceImpl.java b/gsec/src/main/java/gemma/gsec/acl/domain/AclServiceImpl.java index 39ea59b..0ffca79 100644 --- a/gsec/src/main/java/gemma/gsec/acl/domain/AclServiceImpl.java +++ b/gsec/src/main/java/gemma/gsec/acl/domain/AclServiceImpl.java @@ -27,17 +27,20 @@ import org.springframework.util.Assert; import javax.annotation.Nullable; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; +import static java.util.Objects.requireNonNull; + /** * @author paul * @version $Id: AclServiceImpl.java,v 1.1 2013/09/14 16:55:19 paul Exp $ */ public class AclServiceImpl implements AclService { - private static final Log log = LogFactory.getLog( AclServiceImpl.class ); + private static final Log log = LogFactory.getLog( AclDaoImpl.class ); private final AclDao aclDao; @@ -48,52 +51,70 @@ public AclServiceImpl( AclDao aclDao ) { @Override @Transactional public MutableAcl createAcl( ObjectIdentity objectIdentity ) throws AlreadyExistsException { - // Check this object identity hasn't already been persisted - if ( aclDao.find( objectIdentity ) != null ) { - Acl acl = this.readAclById( objectIdentity ); - if ( acl != null ) { - log.warn( "Create called on object identity that already exists, and acl could be loaded; " + acl ); - /* - * This happens ... why? When we set a parent object earlier than needed? - */ - // return ( MutableAcl ) acl; - } + Assert.isInstanceOf( AclObjectIdentity.class, objectIdentity ); + + AclObjectIdentity aclObjectIdentity = ( AclObjectIdentity ) objectIdentity; + + // Check this object identity hasn't already been persisted (fast) or there's already a type/identifier registered (slow) + if ( aclObjectIdentity.getId() != null || aclDao.findObjectIdentity( aclObjectIdentity ) != null ) { throw new AlreadyExistsException( "Object identity '" + objectIdentity + "' already exists in the database" ); } // Need to retrieve the current principal, in order to know who "owns" this ACL (can be changed later on) - Authentication auth = SecurityContextHolder.getContext().getAuthentication(); - AclSid sid = new AclPrincipalSid( auth ); + if ( aclObjectIdentity.getOwnerSid() == null ) { + Authentication auth = SecurityContextHolder.getContext().getAuthentication(); + AclPrincipalSid owner = new AclPrincipalSid( auth ); + log.trace( String.format( "%s has no owner, assigning it to %s", aclObjectIdentity, owner ) ); + aclObjectIdentity.setOwnerSid( owner ); + } - // Create the acl_object_identity row - sid = aclDao.findOrCreate( sid ); - String type = objectIdentity.getType(); - objectIdentity = aclDao.createObjectIdentity( type, objectIdentity.getIdentifier(), sid, true ); + // create the SID + AclSid ownerSid = aclDao.findSid( aclObjectIdentity.getOwnerSid() ); + if ( ownerSid != null ) { + aclObjectIdentity.setOwnerSid( ownerSid ); + } else { + log.trace( String.format( "Owner of %s does not have an ID, finding or creating it...", aclObjectIdentity ) ); + aclObjectIdentity.setOwnerSid( aclDao.createSid( aclObjectIdentity.getOwnerSid() ) ); + } + + // if the parent is identified by type/identifier, we have to retrieve it + // we don't create parent ACLs however + if ( aclObjectIdentity.getParentObject() != null && aclObjectIdentity.getParentObject().getId() == null ) { + log.trace( String.format( "Parent of %s does not have an ID, finding or creating it...", aclObjectIdentity ) ); + aclObjectIdentity.setParentObject( requireNonNull( aclDao.findObjectIdentity( aclObjectIdentity.getParentObject() ), + "Could not find parent of " + aclObjectIdentity ) ); + } - Acl acl = this.readAclById( objectIdentity ); + // Create the acl_object_identity row + objectIdentity = aclDao.createObjectIdentity( aclObjectIdentity ); - return ( MutableAcl ) acl; + return ( MutableAcl ) this.readAclById( objectIdentity ); } @Override @Transactional - public void deleteAcl( ObjectIdentity objectIdentity, boolean deleteChildren ) throws ChildrenExistException { - objectIdentity = aclDao.find( objectIdentity ); - if ( objectIdentity != null ) { - aclDao.delete( objectIdentity, deleteChildren ); - } + public void deleteAcl( ObjectIdentity objectIdentity, boolean deleteChildren ) { + Assert.isInstanceOf( AclObjectIdentity.class, objectIdentity ); + // ensure that the aoi is in the session and also handle deleting by type/identifier combo + AclObjectIdentity aclObjectIdentity = aclDao.findObjectIdentity( ( AclObjectIdentity ) objectIdentity ); + Assert.notNull( aclObjectIdentity ); + aclDao.deleteObjectIdentity( aclObjectIdentity, deleteChildren ); } @Override @Transactional public void deleteSid( Sid sid ) { - aclDao.delete( sid ); + Assert.isInstanceOf( AclSid.class, sid ); + AclSid aclSid = aclDao.findSid( ( AclSid ) sid ); + Assert.notNull( aclSid ); + aclDao.deleteSid( aclSid ); } @Override @Transactional(readOnly = true) public List findChildren( final ObjectIdentity parentIdentity ) { - return aclDao.findChildren( parentIdentity ); + Assert.isInstanceOf( AclObjectIdentity.class, parentIdentity ); + return new ArrayList<>( aclDao.findChildren( ( AclObjectIdentity ) parentIdentity ) ); } @Override @@ -124,7 +145,16 @@ public Map readAclsById( final List objects @Transactional public MutableAcl updateAcl( final MutableAcl acl ) throws NotFoundException { Assert.notNull( acl.getId(), "Object Identity doesn't provide an identifier" ); - aclDao.update( acl ); + Assert.isInstanceOf( AclObjectIdentity.class, acl.getObjectIdentity() ); + Assert.isInstanceOf( AclSid.class, acl.getOwner() ); + AclSid ownerSid = aclDao.findSid( ( AclSid ) acl.getOwner() ); + if ( ownerSid != null ) { + acl.setOwner( ownerSid ); + } else { + log.trace( String.format( "Owner of %s does not exist, creating it...", acl ) ); + acl.setOwner( aclDao.createSid( ( AclSid ) acl.getOwner() ) ); + } + aclDao.updateObjectIdentity( ( AclObjectIdentity ) acl.getObjectIdentity(), acl ); return acl; } diff --git a/gsec/src/main/java/gemma/gsec/acl/domain/AuditLoggerImpl.java b/gsec/src/main/java/gemma/gsec/acl/domain/AuditLoggerImpl.java new file mode 100644 index 0000000..8ac638a --- /dev/null +++ b/gsec/src/main/java/gemma/gsec/acl/domain/AuditLoggerImpl.java @@ -0,0 +1,16 @@ +package gemma.gsec.acl.domain; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.security.acls.domain.AuditLogger; +import org.springframework.security.acls.model.AccessControlEntry; + +public class AuditLoggerImpl implements AuditLogger { + + private static final Log log = LogFactory.getLog( AuditLoggerImpl.class ); + + @Override + public void logIfNeeded( boolean granted, AccessControlEntry ace ) { + log.info( String.format( "%s due to ACE: %s", granted ? "GRANTED" : "DENIED", ace ) ); + } +} diff --git a/gsec/src/main/resources/gemma/gsec/applicationContext-gsec.xml b/gsec/src/main/resources/gemma/gsec/applicationContext-gsec.xml index 20bf6c5..2ac61c8 100644 --- a/gsec/src/main/resources/gemma/gsec/applicationContext-gsec.xml +++ b/gsec/src/main/resources/gemma/gsec/applicationContext-gsec.xml @@ -9,10 +9,17 @@ + + + + + + - + @@ -30,19 +37,6 @@ - - - - - - - - - - - - - diff --git a/gsec/src/main/resources/gemma/gsec/model/AclObjectIdentity.hbm.xml b/gsec/src/main/resources/gemma/gsec/model/AclObjectIdentity.hbm.xml index db67fbc..fb64b94 100644 --- a/gsec/src/main/resources/gemma/gsec/model/AclObjectIdentity.hbm.xml +++ b/gsec/src/main/resources/gemma/gsec/model/AclObjectIdentity.hbm.xml @@ -3,19 +3,21 @@ "http://www.hibernate.org/dtd/hibernate-mapping-3.0.dtd"> - + - - - - - - + + + + + + + + diff --git a/gsec/src/test/java/gemma/gsec/acl/domain/AclDaoTest.java b/gsec/src/test/java/gemma/gsec/acl/domain/AclDaoTest.java new file mode 100644 index 0000000..9b9bde6 --- /dev/null +++ b/gsec/src/test/java/gemma/gsec/acl/domain/AclDaoTest.java @@ -0,0 +1,45 @@ +package gemma.gsec.acl.domain; + +import gemma.gsec.model.Securable; +import org.hibernate.SessionFactory; +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.AbstractTransactionalJUnit4SpringContextTests; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; + +@ContextConfiguration(locations = { "classpath*:gemma/gsec/applicationContext-*.xml", "classpath:gemma/gsec/testContext.xml" }) +public class AclDaoTest extends AbstractTransactionalJUnit4SpringContextTests { + + @Autowired + private AclDao aclDao; + + @Autowired + private SessionFactory sessionFactory; + + public static class MyModel implements Securable { + + private final Long id; + + public MyModel( Long id ) { + this.id = id; + } + + @Override + public Long getId() { + return id; + } + } + + @Test + public void testFindObjectIdentityById() { + AclObjectIdentity oid = new AclObjectIdentity( MyModel.class, 1L ); + oid.setOwnerSid( aclDao.createSid( new AclPrincipalSid( "me" ) ) ); + oid = aclDao.createObjectIdentity( oid ); + assertNotNull( oid.getId() ); + assertSame( oid, aclDao.findObjectIdentity( oid ) ); // by ID + assertSame( oid, aclDao.findObjectIdentity( new AclObjectIdentity( MyModel.class, 1L ) ) ); // by natural ID + } +} \ No newline at end of file diff --git a/gsec/src/test/java/gemma/gsec/acl/domain/AclServiceTest.java b/gsec/src/test/java/gemma/gsec/acl/domain/AclServiceTest.java new file mode 100644 index 0000000..03c5db9 --- /dev/null +++ b/gsec/src/test/java/gemma/gsec/acl/domain/AclServiceTest.java @@ -0,0 +1,177 @@ +package gemma.gsec.acl.domain; + +import gemma.gsec.model.Securable; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.acls.domain.BasePermission; +import org.springframework.security.acls.model.*; +import org.springframework.security.authentication.TestingAuthenticationToken; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.AbstractJUnit4SpringContextTests; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.Assert.*; + +@ContextConfiguration(locations = { "classpath*:gemma/gsec/applicationContext-*.xml", "classpath:gemma/gsec/testContext.xml" }) +public class AclServiceTest extends AbstractJUnit4SpringContextTests { + + @Autowired + private AclService aclService; + + public static class MyModel implements Securable { + + private final Long id; + + public MyModel( Long id ) { + this.id = id; + } + + @Override + public Long getId() { + return id; + } + } + + @Before + public void setUpAuthentication() { + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication( new TestingAuthenticationToken( "admin", "1234", "GROUP_ADMIN" ) ); + SecurityContextHolder.setContext( context ); + } + + @After + public void clearAuthentication() { + SecurityContextHolder.clearContext(); + } + + @Test + public void test() { + AclSid me = new AclPrincipalSid( "me" ); + AclObjectIdentity oid = new AclObjectIdentity( new MyModel( 1L ) ); + + MutableAcl acl = aclService.createAcl( oid ); + assertNotNull( oid.getId() ); + assertEquals( oid, acl.getObjectIdentity() ); + assertEquals( new AclPrincipalSid( "admin" ), acl.getOwner() ); + assertFalse( acl.isEntriesInheriting() ); + + Acl reloadedAcl = aclService.readAclById( oid ); + assertNotNull( reloadedAcl.getOwner() ); + + acl.insertAce( 0, BasePermission.READ, me, false ); + acl.setEntriesInheriting( true ); + aclService.updateAcl( acl ); + + reloadedAcl = aclService.readAclById( oid ); + assertNotNull( reloadedAcl.getOwner() ); + assertTrue( reloadedAcl.isEntriesInheriting() ); + assertEquals( 1, reloadedAcl.getEntries().size() ); + AccessControlEntry entry = reloadedAcl.getEntries().iterator().next(); + assertNotNull( entry.getId() ); + assertSame( reloadedAcl, entry.getAcl() ); + assertNotNull( entry.getSid() ); + assertEquals( BasePermission.READ, entry.getPermission() ); + + acl.deleteAce( 0 ); + aclService.updateAcl( acl ); + + reloadedAcl = aclService.readAclById( oid ); + assertNotNull( reloadedAcl.getOwner() ); + assertTrue( reloadedAcl.getEntries().isEmpty() ); + } + + @Test + public void testAclWithParent() { + AclObjectIdentity oid = new AclObjectIdentity( new MyModel( 2L ) ); + MutableAcl acl = aclService.createAcl( oid ); + + AclObjectIdentity oidP = new AclObjectIdentity( new MyModel( 3L ) ); + MutableAcl parentAcl = aclService.createAcl( oidP ); + + acl.setParent( parentAcl ); + aclService.updateAcl( acl ); + + assertThatThrownBy( () -> aclService.deleteAcl( oidP, false ) ) + .isInstanceOf( ChildrenExistException.class ); + + // now, properly delete + aclService.deleteAcl( oid, false ); + aclService.deleteAcl( oidP, false ); + + assertThatThrownBy( () -> aclService.readAclById( new AclObjectIdentity( new MyModel( 3L ) ) ) ) + .isInstanceOf( NotFoundException.class ); + assertThatThrownBy( () -> aclService.readAclById( new AclObjectIdentity( new MyModel( 2L ) ) ) ) + .isInstanceOf( NotFoundException.class ); + } + + @Test + public void testAclWithParentOnCreation() { + AclObjectIdentity oidP = new AclObjectIdentity( new MyModel( 3L ) ); + MutableAcl parentAcl = aclService.createAcl( oidP ); + assertNotNull( oidP.getId() ); + + AclObjectIdentity oid = new AclObjectIdentity( new MyModel( 2L ) ); + oid.setParentObject( new AclObjectIdentity( new MyModel( 3L ) ) ); + MutableAcl acl = aclService.createAcl( oid ); + + assertEquals( parentAcl, acl.getParentAcl() ); + } + + @Test + public void testAclWithParentDeleteChildren() { + AclObjectIdentity oid = new AclObjectIdentity( new MyModel( 4L ) ); + MutableAcl acl = aclService.createAcl( oid ); + + AclObjectIdentity oidP = new AclObjectIdentity( new MyModel( 5L ) ); + MutableAcl parentAcl = aclService.createAcl( oidP ); + + acl.setParent( parentAcl ); + aclService.updateAcl( acl ); + + aclService.deleteAcl( oidP, true ); + assertThatThrownBy( () -> aclService.readAclById( oid ) ) + .isInstanceOf( NotFoundException.class ); + assertThatThrownBy( () -> aclService.readAclById( oidP ) ) + .isInstanceOf( NotFoundException.class ); + assertThatThrownBy( () -> aclService.readAclById( new AclObjectIdentity( new MyModel( 5L ) ) ) ) + .isInstanceOf( NotFoundException.class ); + assertThatThrownBy( () -> aclService.readAclById( new AclObjectIdentity( new MyModel( 4L ) ) ) ) + .isInstanceOf( NotFoundException.class ); + } + + @Test + public void testCreateDuplicateAcl() { + AclObjectIdentity oid = new AclObjectIdentity( new MyModel( 6L ) ); + aclService.createAcl( oid ); + assertNotNull( oid.getId() ); + // fast path, simply check the id + assertThatThrownBy( () -> aclService.createAcl( oid ) ) + .isInstanceOf( AlreadyExistsException.class ); + // slow path, type & identifier are looked up + assertThatThrownBy( () -> aclService.createAcl( new AclObjectIdentity( new MyModel( 6L ) ) ) ) + .isInstanceOf( AlreadyExistsException.class ); + } + + @Test + public void testDeleteSid() { + AclSid me = new AclPrincipalSid( "me" ); + AclObjectIdentity oid = new AclObjectIdentity( new MyModel( 7L ) ); + oid.setOwnerSid( me ); + + MutableAcl acl = aclService.createAcl( oid ); + + acl.insertAce( 0, BasePermission.READ, me, false ); + acl.setEntriesInheriting( true ); + aclService.updateAcl( acl ); + + aclService.deleteSid( me ); + + // there's no way to check the ACE has been removed, but that would have violated a FK constraint + assertThatThrownBy( () -> aclService.readAclById( oid ) ) + .isInstanceOf( NotFoundException.class ); + } +} \ No newline at end of file diff --git a/gsec/src/test/resources/gemma/gsec/testContext.xml b/gsec/src/test/resources/gemma/gsec/testContext.xml index 63816e0..e5486c5 100644 --- a/gsec/src/test/resources/gemma/gsec/testContext.xml +++ b/gsec/src/test/resources/gemma/gsec/testContext.xml @@ -1,26 +1,27 @@ - + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:tx="http://www.springframework.org/schema/tx" + xsi:schemaLocation=" + http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd + http://www.springframework.org/schema/tx http://www.springframework.org/schema/tx/spring-tx.xsd"> + + - - - - - - - - + + + + + + + + org.hibernate.dialect.H2Dialect create org.hibernate.cache.ehcache.SingletonEhCacheRegionFactory