SEC-3165: Get domain object instance based on runtime class in voters

[spring-projects-issues]

Olivier Ailloud (Migrated from SEC-3165) said:

The method getDomainObjectInstance in AbstractAclVoter loops through the parameters and looks for any parameter whose +declared+ class is assignable from the processDomainObjectClass.
But when the parameter's type is an interface, this is annoying as the voter will throw an AuthorizationServiceException even if implementations are eligible for this voter.
It seems to me that it should rather be based on the +runtime+ class.

The patch seems fairly easy, I may provide it.

[spring-projects-issues]

Rob Winch said:

Thank you for logging this issue and your willingness to submit the changes!

This seems like a reasonable enhancement. However, we would want to ensure that this is passive. To implement it, we would want to have a flag set that determines if resolution is done by the parameter types or the argument types. We want to prevent issues if a user has a method like this:

public interface MyInterface {
// ...
}

public class MyInterfaceImpl implements MyInterface {
// ...
}


public void run(MyInterface i, MyInterfaceImpl impl)

Right now the domain object instance would resolve the second argument if processDomainObjectClass is set to MyInterfaceImpl. We would want to be sure this (by default) remains the same to make sure we do not break users.

Additionally, we would need to determine what we do if the argument passed in is null. Perhaps, if the argument is null we should fall back to the parameter type. Thoughts?

[spring-projects-issues]

Olivier Ailloud said:

Good points indeed.
In case of a null argument, falling back on the parameter type would be the most error proof but would not that be overkill ? Since the voter is not going to be able to decide much about a null domainObject, is he ?
IMHO, I would rather skip to the next argument and throw the exception if no one matched. But maybe skipping null arguments is too much of a decision at that point, I don't know.

As an alternative, what about simply looping through the argument types as a fallback if resolution failed on parameters ? This would keep the current behavior without the complexity of a new flag.

[spring-projects-issues]

Rob Winch said:

Thanks for the fast response!

I think the problem is if we skip the argument, then the exception is thrown. However, most likely the user wants to make a decision based on the null value (which would likely be abstain).

[spring-projects-issues]

Olivier Ailloud said:

In that case, falling back on the parameter type as you suggested seems a good solution.

And so you would prefer a flag to enable lookup on arguments ? Rather than looping on parameter types first and then looping on arguments (if no parameter matched) ?