diff --git a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy index c72de8c7a..a8a423907 100644 --- a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy +++ b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy @@ -21,7 +21,6 @@ package grails.plugin.springsecurity.acl import grails.gorm.DetachedCriteria import grails.gorm.transactions.ReadOnly -import groovy.transform.CompileDynamic import groovy.transform.CompileStatic import org.springframework.security.acls.model.ObjectIdentity @@ -42,14 +41,9 @@ class AclObjectIdentityGormService { AclObjectIdentity.get(id) } - @CompileDynamic @ReadOnly List findAllByParentObjectIdAndParentAclClassName(Long objectId, String aclClassName) { - //findQueryByParentObjectIdAndParentAclClassName(objectId, aclClassName).list() - List aclObjectIdentityList = findAll() - aclObjectIdentityList.findAll { AclObjectIdentity oid -> - (oid?.parent?.aclClass?.className == aclClassName) && ( oid?.parent?.objectId == objectId) - } + findQueryByParentObjectIdAndParentAclClassName(objectId, aclClassName).list() } @ReadOnly diff --git a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy index 91961a4c3..602fe74b6 100644 --- a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy +++ b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy @@ -22,6 +22,7 @@ import grails.gorm.transactions.ReadOnly import groovy.transform.CompileDynamic import groovy.transform.CompileStatic import groovy.util.logging.Slf4j +import org.grails.datastore.gorm.GormEntity import org.springframework.context.MessageSource import org.springframework.security.acls.domain.AccessControlEntryImpl import org.springframework.security.acls.domain.GrantedAuthoritySid @@ -97,10 +98,7 @@ class AclService implements MutableAclService, WarnErros { objectId: object.identifier as Long, owner: ownerSid, entriesInheriting: true) - if ( !aclObjectIdentity.save() ) { - log.error '{}', errorsBeanBeingSaved(messageSource, aclObjectIdentity) - } - aclObjectIdentity + save(aclObjectIdentity) } @Transactional @@ -124,9 +122,7 @@ class AclService implements MutableAclService, WarnErros { AclSid aclSid = aclSidGormService.findBySidAndPrincipal(sidName, principal) if (!aclSid && allowCreate) { aclSid = new AclSid(sid: sidName, principal: principal) - if ( !aclSid.save() ) { - log.error '{}', errorsBeanBeingSaved(messageSource, aclSid) - } + save(aclSid) } aclSid } @@ -136,9 +132,7 @@ class AclService implements MutableAclService, WarnErros { AclClass aclClass = aclClassGormService.findByClassName(className) if (!aclClass && allowCreate) { aclClass = new AclClass(className: className) - if ( !aclClass.save() ) { - log.error '{}', errorsBeanBeingSaved(messageSource, aclClass) - } + save(aclClass) } aclClass } @@ -209,7 +203,7 @@ class AclService implements MutableAclService, WarnErros { log.trace 'Checking ace for delete: {}', ace !acl.entries.find { AccessControlEntry entry -> log.trace 'Checking entry for delete: {}', entry - entry.permission.mask == ace.mask && entry.sid == ace.sid.sid + entryEqual(ace, entry) } } @@ -217,15 +211,17 @@ class AclService implements MutableAclService, WarnErros { log.trace 'Checking entry for create: {}', entry !existingAces.find { AclEntry ace -> log.trace 'Checking ace for create: {}', ace - entry.permission.mask == ace.mask && entry.sid == ace.sid.sid + entryEqual(ace, entry) } } + Integer maxAceOrder = existingAces.max { it.aceOrder }?.aceOrder + // Delete this ACL's ACEs in the acl_entry table deleteEntries toDelete // Create this ACL's ACEs in the acl_entry table - createEntries(acl, toCreate as List) + createEntries(acl, maxAceOrder, toCreate as List) // Change the mutable columns in acl_object_identity updateObjectIdentity acl @@ -237,10 +233,9 @@ class AclService implements MutableAclService, WarnErros { } @Transactional - protected void createEntries(MutableAcl acl, List entries = null) { - List entryList = entries ?: acl.entries as List - int i = 0 - for (AuditableAccessControlEntry entry in entryList) { + protected void createEntries(MutableAcl acl, Integer maxOrder, List entries) { + int i = maxOrder != null ? maxOrder + 1 : 0 + for (AuditableAccessControlEntry entry in entries) { Assert.isInstanceOf AccessControlEntryImpl, entry, 'Unknown ACE class' AclEntry aclEntryInstance = new AclEntry( aclObjectIdentity: AclObjectIdentity.load(acl.id), @@ -250,9 +245,7 @@ class AclService implements MutableAclService, WarnErros { granting: entry.isGranting(), auditSuccess: entry.isAuditSuccess(), auditFailure: entry.isAuditFailure()) - if ( !aclEntryInstance.save() ) { - log.error "{}", errorsBeanBeingSaved(messageSource, aclEntryInstance) - } + save(aclEntryInstance) } } @@ -272,9 +265,7 @@ class AclService implements MutableAclService, WarnErros { aclObjectIdentity.parent = parent aclObjectIdentity.owner = createOrRetrieveSid(acl.owner, true) aclObjectIdentity.entriesInheriting = acl.isEntriesInheriting() - if ( !aclObjectIdentity.save() ) { - log.error "{}", errorsBeanBeingSaved(messageSource, aclObjectIdentity) - } + save(aclObjectIdentity) AclObjectIdentity.withSession { it.flush() } } @@ -335,4 +326,17 @@ class AclService implements MutableAclService, WarnErros { } return result } + + @Transactional + protected > T save(T bean) { + if (!bean.save()) { + log.warn errorsBeanBeingSaved(messageSource, bean) + } + bean + } + + private static boolean entryEqual(AclEntry ace, AccessControlEntry entry) { + Sid sid = ace.sid.principal ? new PrincipalSid(ace.sid.sid) : new GrantedAuthoritySid(ace.sid.sid) + return entry.permission.mask == ace.mask && entry.sid == sid && entry.granting == ace.granting + } } diff --git a/plugin-acl/plugin/src/main/groovy/grails/plugin/springsecurity/acl/AbstractAclObjectIdentity.groovy b/plugin-acl/plugin/src/main/groovy/grails/plugin/springsecurity/acl/AbstractAclObjectIdentity.groovy index ec5d12751..bb717fb51 100644 --- a/plugin-acl/plugin/src/main/groovy/grails/plugin/springsecurity/acl/AbstractAclObjectIdentity.groovy +++ b/plugin-acl/plugin/src/main/groovy/grails/plugin/springsecurity/acl/AbstractAclObjectIdentity.groovy @@ -18,6 +18,7 @@ */ package grails.plugin.springsecurity.acl +import grails.gorm.dirty.checking.DirtyCheck import groovy.transform.EqualsAndHashCode import groovy.transform.ToString @@ -29,6 +30,7 @@ import groovy.transform.ToString */ @EqualsAndHashCode(includes=['aclClass', 'parent', 'owner', 'entriesInheriting']) @ToString(includeNames=true) +@DirtyCheck abstract class AbstractAclObjectIdentity implements Serializable { AclClass aclClass diff --git a/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclObjectIdentity.groovy b/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclObjectIdentity.groovy index 306f43a1d..061bc98c7 100644 --- a/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclObjectIdentity.groovy +++ b/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclObjectIdentity.groovy @@ -18,12 +18,14 @@ */ package grails.plugin.springsecurity.acl +import grails.gorm.dirty.checking.DirtyCheck import groovy.transform.ToString /** * @author Burt Beckwith */ @ToString(excludes='version', includeNames=true) +@DirtyCheck class AclObjectIdentity { private static final long serialVersionUID = 1 diff --git a/plugin-ui/examples/extended/src/integration-test/groovy/spec/AclEntrySpec.groovy b/plugin-ui/examples/extended/src/integration-test/groovy/spec/AclEntrySpec.groovy index 630874ac0..c01313dba 100644 --- a/plugin-ui/examples/extended/src/integration-test/groovy/spec/AclEntrySpec.groovy +++ b/plugin-ui/examples/extended/src/integration-test/groovy/spec/AclEntrySpec.groovy @@ -54,9 +54,9 @@ class AclEntrySpec extends AbstractSecuritySpec { aclEntrySearchPage.assertResults(1, 3, 3) assertContentContains('60') - assertContentContains('398') - assertContentContains('399') - assertContentContains('400') + assertContentContains('62') + assertContentContains('194') + assertContentContains('195') assertContentContains('user1') assertContentContains('admin') assertContentDoesNotContain('>user2') @@ -74,7 +74,7 @@ class AclEntrySpec extends AbstractSecuritySpec { then: browser.at(AclEntrySearchPage) aclEntrySearchPage.assertResults(1, 10, 67) - ['104', '111', '119', '126', '131', '136', '141', '146', '152', '159'].each { + ['75', '76', '78', '80', '82', '87', '89', '91', '93', '95'].each { assertContentContains it } }