Spring Security
  1. Spring Security
  2. SEC-1837

Problem appear when add @PreAuthorize to a controller using @PathVariable

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Cannot Reproduce
    • Affects Version/s: 3.1.0.M2
    • Fix Version/s: 3.1.0
    • Component/s: Core
    • Labels:
      None

      Description

      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

        Activity

        Hide
        Luke Taylor added a comment -

        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?

        Show
        Luke Taylor added a comment - 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?
        Hide
        Davide added a comment -

        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

        <aop:aspectj-autoproxy proxy-target-class="true" />
        <security:global-method-security pre-post-annotations="enabled"/>

        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;
        }

        Show
        Davide added a comment - 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 <aop:aspectj-autoproxy proxy-target-class="true" /> <security:global-method-security pre-post-annotations="enabled"/> 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; }
        Hide
        Davide added a comment -

        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;
        }

        Show
        Davide added a comment - 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; }
        Hide
        Rob Winch added a comment -

        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.

        Show
        Rob Winch added a comment - 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.
        Hide
        Davide added a comment -

        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

        Show
        Davide added a comment - 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
        Hide
        Rob Winch added a comment -

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

        Show
        Rob Winch added a comment - Marking as cannot reproduce since the latest versions of code do not exhibit this issue.

          People

          • Assignee:
            Rob Winch
            Reporter:
            Davide
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: