SEC-3066: DefaultMethodSecurityExpressionHandler does not handle NotFoundException when using a PermissionCacheOptimizer

[spring-projects-issues]

Steven Pearce (Migrated from SEC-3066) said:

To Trigger:
When DefaultMethodSecurityExpressionHandler is used with AclPermissionCacheOptimizer and a method is annotated with
@PostFilter("hasPermission(filterObject, 'read')")

If any of the returned objects do not have a corresponding record in acl_object_identity, when AclPermissionCacheOptimizer tries to save them via JdbcAclService it throws a NotFoundException.

If DefaultMethodSecurityExpressionHandler is created without using a Cache, then this Exception is not raised.

Here is where I think the problem is

spring-security/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java

Lines 103 to 106 in 969f3a7

	if (permissionCacheOptimizer != null) {
	permissionCacheOptimizer.cachePermissionsFor(
	rootObject.getAuthentication(), collection);
	}

If the Cache is set, then it tries to save the permissions for the whole collection. At this point, the collection will contain 3 types of objects,

• Objects with ACL records that will evaluate true,
• Objects with ACL records that will evaluate false,
• Objects with no ACL records at all.

The last item is causing the problem, but in the use case of caching, should not be.
A quick fix would be to move those highlighted lines lower and only cache the retainList collection, not ideal as this would only cache objects that evaluate as true.

But I think a better fix would require a new method in JdBcAclService similar to readAclsById but that doesn't throw an exception when an ACL record isn't found.

The high-level view of this issue is that ACL caching isn't possible if you don't have an ACL record for every single entity you are likely to return in collections.

[spring-projects-issues]

Steven Pearce said:

Have created a pull request implementing the mentioned new method in AclService, to see what people thought.
#214

[spring-projects-issues]

Steven Pearce said:

Much Better solution after thinking it through some more.

By the time the Exception has been raised in AclService, the PermissionCacheOptimiser has done its job and already loaded the ACLs present.
So all thats required is to handle the Exception by a log entry and nothing else.

New Pull
#215