Skip to content

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

@spring-projects-issues

Description

@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.

Activity

spring-projects-issues

spring-projects-issues commented on Dec 4, 2015

@spring-projects-issues
Author

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

spring-projects-issues commented on Dec 4, 2015

@spring-projects-issues
Author

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

spring-projects-issues commented on Dec 4, 2015

@spring-projects-issues
Author

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

spring-projects-issues commented on Dec 5, 2015

@spring-projects-issues
Author

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) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    in: aclAn issue in spring-security-acltype: enhancementA general enhancementtype: jiraAn issue that was migrated from JIRA

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @rwinch@spring-projects-issues

        Issue actions

          SEC-3165: Get domain object instance based on runtime class in voters · Issue #3374 · spring-projects/spring-security