SEC-1198: MethodSecurityPriviligeEvaluator Returns True When Principal Does Not Have Required Role #1446

Closed
spring-issuemaster opened this Issue Jul 16, 2009 · 9 comments

Projects

None yet

1 participant

@spring-issuemaster

Ole Ersoy (Migrated from SEC-1198) said:

We've implemented an IAuthorizationService.isCallable(Object object, String methodName) service.

The isCallable method looks up the object's security interceptor in a map and then sets the security interceptor on the MethodInvocationPriviliegeEvaluator instance. MethodInvocationPriviliegeEvaluator is then called to see whether the method invocation is allowed.

True is returned regardless of whether the authentication has the required role or not.

I'm attaching a maven project with all source code and tests.

The test to look at is:
spring.security.jira/src/test/java/com/example/security/authorization/tests/AuthorizationServiceTest.java

SIDE NOTE:
I believe the method MethodInvocationPriviliegeEvaluator currently requires a MethodSecurityInterceptor instance for it's initialization. This required us to wire in "Dummy" classes just to get the container to start up. When isCallable is called, we replace the "Dummy" MethodSecurityInterceptor instance with the instance the container created for the secured object.

@spring-issuemaster

Luke Taylor said:

I've had a look at your code and I believe this is because of the way your AuthorizationServiceImpl does the method lookups. The SecurityMetadataSource has the attributes registered against the interface INeedsMethodAuthorization, and to get a match, the Method object you use to create the SimpleMethodInvocation would have to also be on the interface. In fact, the object you are invoking the method on is a Java dynamic proxy, so the class is of this type.

You can cast the object to "Advised", then use getTargetSource().getTarget() to get the actual target object.

Similarly you can call getProxiedInterfaces() and check through these for the method name in order to locate the method (if you want to register it by interface). If to check for metadata registered against the the implementation, then you can call getClass() method on the actual target object.

@spring-issuemaster

Ole Ersoy said:

Yes! Luke thanks a gazillion for explaining that. I gave it a shot and I think it works fine now, both for interface based security metadata and implementation based security meta data. I added a property to the service called implementationBasedSecurityMetaData, which defaults to true. If it's set to false, then the service expects the meta data to be keyed on an interface.

I'll upload the project with the modified service and test cases next.

Thanks again!

@spring-issuemaster

Luke Taylor said:

Do you still think there is a bug? If so, please explain what it is and supply a test case (just against Spring Security code) that demonstrates the problem. If not, I'll close the issue.

@spring-issuemaster

Ole Ersoy said:

Please close it. It works great now - Thanks.

@spring-issuemaster

Ole Ersoy said:

Actually there is one thing that might be of interest still, and that is that we had to create a "Dummy" MethodSecurityInterceptor, etc in order to create the MethodInvocationPrivilegeEvaluator (See Side Note in description).

@spring-issuemaster

Luke Taylor said:

You can either inject the interceptor directly into it if you have configured the inteceptor explicitly, autowire it by type or use a BeanPostProcessor to configure it.

@spring-issuemaster

Ole Ersoy said:

It's just that the service as is currently chooses which security interceptor to use during the isCallable method invocation, so all the service needs is a "vanilla" MethodInvocationPriviligeEvaluator.

@spring-issuemaster

Ole Ersoy said:

That might be a moo point though, if proper framework way to do it is to either use or manually configure a single method security interceptor that gets applied via aop. If I understand correctly in both of these cases there will only be a single security interceptor, so one might as well just wire it during startup?

@spring-issuemaster

Luke Taylor said:

There isn't really a "proper" way to do it. It depends on your requirements. If a single interceptor is sufficient (and often it will be), then there's no reason why you can't just define the MethodInvocationPrivilegeEvaluator in the app context and inject the interceptor. If for some reason you need more than one interceptor then you can have multiple MethodInvocationPrivilegeEvaluators too.

@spring-issuemaster spring-issuemaster added this to the 3.0.0 M2 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment