SEC-1837: Problem appear when add @PreAuthorize to a controller using @PathVariable #2068

Closed
spring-issuemaster opened this Issue Oct 11, 2011 · 6 comments

2 participants

@spring-issuemaster

Davide (Migrated from SEC-1837) said:

I'm currently using @PathVariable annotation in this method without any problem

@RequestMapping(value={"/account/{pk}"},method=RequestMethod.DELETE)
@ResponseBody
public JsonResponse deleteAccount(final @PathEntity("orgName") Organization org, @PathVariable("pk") Long pk){

but if I try to add also @PreAuthorize at the class it throws an IllegalArgumentException before entering the method.
Seams there's something wrong with MethodParameter#getParameterAnnotations in the core library since return null when trying to get
the annotation, works with @PathEntity but not with @PathVariable

@spring-issuemaster

Luke Taylor said:

Could you provide a test case please, clarifying where you are putting the annotation and what it contains, as well as what you have in the rest of your configuration?

@spring-issuemaster

Davide said:

The annotation is used in this class

@Controller
@RequestMapping("/{orgName}")
@PreAuthorize("(#org == null) or hasAnyRole('ROLE1:' + #org.id,'ADMIN')")
public class OrganizationController { // Note - extending AbstractC breaks PreAuthorize

then we have these features enabled in the application context config file


I need to clean the config files before to publish them and it'll take me a while, for the moment maybe is also useful
for you to know that we temporarily patched the method MethodParameter.getParameterAnnotations and is working

public Annotation[] getParameterAnnotations() {
    if (this.parameterAnnotations == null) {
        Annotation[][] annotationArray = (this.method != null ?
                this.method.getParameterAnnotations() : this.constructor.getParameterAnnotations());
        if (this.parameterIndex >= 0 && this.parameterIndex < annotationArray.length) {
            this.parameterAnnotations = annotationArray[this.parameterIndex];
        }
        else {
            this.parameterAnnotations = new Annotation[0];
        }
    }

// return this.parameterAnnotations;
if (this.parameterAnnotations == null) {
Annotation[][] annotationArray = (this.method != null ?
this.method.getParameterAnnotations() : this.constructor.getParameterAnnotations());

        // Patch to support parameter annotations on CGLIB classes
        if(this.method != null && net.sf.cglib.proxy.Factory.class.isAssignableFrom(this.method.getDeclaringClass())){
            // get parameter annotations from super class
            for(Method m : this.getDeclaringClass().getMethods()){
                if(m.getName().equals(this.method.getName()) && m.getReturnType().equals(this.method.getReturnType()) && Arrays.equals(m.getParameterTypes(),this.method.getParameterTypes())){
                    annotationArray = m.getParameterAnnotations();
                    break;
                }
            }
        }

        if (this.parameterIndex >= 0 && this.parameterIndex < annotationArray.length) {
            this.parameterAnnotations = annotationArray[this.parameterIndex];
        }
        else {
            this.parameterAnnotations = new Annotation[0];
        }
    }
    return this.parameterAnnotations;
}
@spring-issuemaster

Davide said:

I'm sorry I messed up with the class source and don't know how to change my comment, this is the patched source

public Annotation[] getParameterAnnotations() {
    if (this.parameterAnnotations == null) {
        Annotation[][] annotationArray = (this.method != null ?
                this.method.getParameterAnnotations() : this.constructor.getParameterAnnotations());

        // Patch to support parameter annotations on CGLIB classes
        if(this.method != null && net.sf.cglib.proxy.Factory.class.isAssignableFrom(this.method.getDeclaringClass())){
            // get parameter annotations from super class
            for(Method m : this.getDeclaringClass().getMethods()){
                if(m.getName().equals(this.method.getName()) && m.getReturnType().equals(this.method.getReturnType()) && Arrays.equals(m.getParameterTypes(),this.method.getParameterTypes())){
                    annotationArray = m.getParameterAnnotations();
                    break;
                }
            }
        }

        if (this.parameterIndex >= 0 && this.parameterIndex < annotationArray.length) {
            this.parameterAnnotations = annotationArray[this.parameterIndex];
        }
        else {
            this.parameterAnnotations = new Annotation[0];
        }
    }
    return this.parameterAnnotations;
}
@spring-issuemaster

Rob Winch said:

I tried to reproduce this issue with Spring Security 3.1.0.RELEASE and Spring 3.1.1.RELEASE and was unsuccessful. Perhaps this has since been fixed. Can you please provide a complete (as simple as possible) example of the problem with the latest versions of code? If you are unable to do it with the most recent version of code, it may be nice to have a sample with the versions that caused the issue.

@spring-issuemaster

Davide said:

Hi Rob,

I had the problem with spring 3.1.0.M2, lately I moved to Spring 3.1.1 and haven't had that issue anymore.

Thanks

@spring-issuemaster

Rob Winch said:

Marking as cannot reproduce since the latest versions of code do not exhibit this issue.

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