SEC-1185: MethodInvocationPrivilegeEvaluator.isAllowed() Returns True When Authentication is null #1434

Closed
spring-issuemaster opened this Issue Jun 17, 2009 · 11 comments

1 participant

@spring-issuemaster

Ole Ersoy (Migrated from SEC-1185) said:

The 2.0.4 MethodInvocationPrivilegeEvaluator has the following code block in it:

if ((authentication == null) || (authentication.getAuthorities() == null)
|| (authentication.getAuthorities().length == 0)) {
return false;
}

But I'm getting true returned when authentication is null.

I'll upload a project containing a AuthorizationServiceTest. The test that shows this is:
testAuthorization

The test looks like this:

public void testAuthorization() throws Exception
{
    Authentication authentication = 
        SecurityContextHolder.
        getContext().
        getAuthentication();

    assertNull(authentication);

    boolean authenticationNotFoundExceptionTriggered = false;

    try
    {
        needsAuthorizedMethod.returnString();
    }
    catch (AuthenticationCredentialsNotFoundException e)
    {
        authenticationNotFoundExceptionTriggered = true;
    }

    assertTrue(authenticationNotFoundExceptionTriggered);

    boolean isAllowedResult = 
        authorizationService.
        isCallable(
                needsAuthorizedMethod, 
                "returnString");

    assertTrue(isAllowedResult);
}
@spring-issuemaster

Ole Ersoy said:

Actually maybe this is not a bug, but just me being confused about how to configure the ObjectDefinitionSource property on the MethodSecurityInterceptor instance in service-context.xml.

Here's what happens (attrs is null and securityInterceptor.isRejectPublicInvocations() is true) :

    ConfigAttributeDefinition attrs = securityInterceptor.obtainObjectDefinitionSource().getAttributes(mi);

    if (attrs == null) 
    {
        if (securityInterceptor.isRejectPublicInvocations()) 
        {
            return false;
        }
        return true;

So true is returned.

The reason this confuses me is that Spring knows that the method is protected like this:

<bean
 id="needsAuthorizedMethod"
 class="com.example.protect.NeedsAuthorizedMethod">
    <security:intercept-methods>
        <security:protect method="returnString" access="ROLE_ADMIN" />
    </security:intercept-methods>
</bean>

So it seems like it has the information it needs to provide the ConfigAttributeDefinition being retrieved start of the MethodInvocationPrivilegeEvaluator.isAllowed() code. Do I need to do something so that this information is used to create the ConfigAttributeDefinition instance?

@spring-issuemaster

Luke Taylor said:

MethodInvocationPrivilegeEvaluator returns true for isAllowed if there are no attributes configured for the resource and the interceptor is set to allow public invocations (i.e. not deny by default). It will return false if the resource is protected by the configured interceptor and the authentication object is null.

MethodInvocationPrivilegeEvaluator has to be explicitly configured with an interceptor - it won't pick one up at random from a Spring configuration and thus won't work with the intercept-methods namespace element where the interceptor is hidden. You can configure and interceptor explicitly and apply it using Spring AOP if you need a reference. It would also be possible for us to expose a MethodInvocationPrivilegeEvaluator through the namespace (e.g. as an attribute on but it's not obvious that there would be enough of a demand to justify this.

@spring-issuemaster

Ole Ersoy said:

Since we wanted to avoid duplicating configuration information, we looked up the hidden security interceptors and applied them dynamically at runtime. Further description here:
http://jira.springsource.org/browse/SEC-1198

I really like your idea of exposing the service through .
If anyone else is interested I'd be glad to help with further planning and implementation.

@spring-issuemaster

Luke Taylor said:

It's more a question of whether adding it to the namespace is justified, given that this isn't something people use much - it is cluttered enough as it is :).

You can also work round this kind of configuration issue by using autowiring by type, or by adding a BeanPostProcessor to the context (as well as the bean you want to configure):

MethodInvocationPrivilegeEvaluatorPostProcessor implements BeanPostProcessor, BeanfactoryAware {
private BeanFactory beanFactory;

public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException {

    if ("myPrivilegeEvalBean".equals(beanName)) {
        // Get the interceptor(s)
         beanFactory.getBeansOfType(MethodSecurityInterceptor.class);
       // Do whatever else you want here
    }
    return bean;
}

...

}

@spring-issuemaster

Ole Ersoy said:

Aha - Thanks - I like the getBeansOfType method, as this gets us around the need to reimplement the service if Spring changes the naming convention of the security interceptors. I see what you are saying though. I imagine most projects are simply using authentication as their method security. We are building a single page RIA (Rich internet application), so we need more advanced capabilities.

@spring-issuemaster

Luke Taylor said:

The naming convention has already changed, in that there is no convention :). I've been removing the use of global bean names where possible as it's reminiscent of building an application where everything is implemented using the Singleton pattern.

@spring-issuemaster

Ole Ersoy said:

Oh Oh...I just realized "No naming convention" breaks the way we implemented the AuthorizationService here:

http://jira.springsource.org/browse/SEC-1198

Since the service has a list of secured object ID's and we use these IDs to lookup the corresponding security interceptors.

Is there some other way to know which security interceptor corresponds to a specific bean, so that we can create Map?

@spring-issuemaster

Luke Taylor said:

Either use the namespace element to apply security (then there is only one interceptor). Otherwise you can configure one explicitly and apply it using Spring AOP, or BeanNameAutoProxyCreator. See SEC-1204 for an example. To use Spring's AOP namespace, you could do something like

  <aop:config>
        <aop:pointcut id='targetMethods' expression='execution(* org.springframework.security.TargetObject.*(..))'/>
        <aop:advisor advice-ref='securityInterceptor' pointcut-ref='targetMethods' />
  </aop:config>
@spring-issuemaster

Ole Ersoy said:

Thanks for pointing out these options. I think once we upgrade I may miss the simplicity of just adding the security:intercept-methods elements directly to the bean definitions. Would it conflict with other implementation efficiencies / designs if the naming convention is kept?

@spring-issuemaster

Luke Taylor said:

The interceptor name will be decided by Spring's AbstractInterceptorDrivenBeanDefinitionDecorator, so you can use the convention from there to work it out.

@spring-issuemaster

Ole Ersoy said:

Super - Thanks!

@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