SEC-3081: @PreAuthorize not inherited on overridden generic method

[spring-projects-issues]

Christopher Smith (Migrated from SEC-3081) said:

I have mappings and security annotations defined in an abstract base controller class, then subclass that controller to provide HTML and versioned JSON implementations. Up until now, I've been able to apply @PreAuthorize to an abstract method on the superclass:

@PreAuthorize(AuthorizationExpressions.SUPPORT_ROLE)
@RequestMapping('/{digitizer}')
abstract digitizerDetail(@PathVariable Digitizer digitizer)

and the correct restrictions are applied to the subclass:

@Override
def digitizerDetail(@PathVariable Digitizer digitizer) {
    new ModelAndView('digitizers/profile')
}

However, if the superclass is generic (in this case, to facilitate REST API versioning by permitting different DTO classes to be mapped in method parameters), the path mapping works but the security annotation does not:

<R extends ResourceSupport>...
@PreAuthorize(AuthorizationExpressions.SUPPORT_ROLE)
@RequestMapping(value = '/{digitizer}', method = RequestMethod.POST)
abstract modifyDigitizer(@PathVariable Digitizer digitizer, R resource)

@Override
def modifyDigitizer(@PathVariable Digitizer digitizer, @ModelAttribute DigitizerResource updatedDigitizer) {
    // this method does not have security restrictions applied
}

I expected the generic override to either succeed or fail in its entirety, but the partial success means that a method can get mapped in appropriately but not secured as intended.

[spring-projects-issues]

Juergen Hoeller said:

This is probably due to the way Spring Security performs its annotation lookup. Moving the issue to the Spring Security project for that reason.

Juergen

[spring-projects-issues]

Rob Winch said:

w_c_smith It appears you are using Groovy. Does this problem still happen if you are using Java?

[spring-projects-issues]

Christopher Smith said:

I will attempt a replication in pure Java, but it will probably be a bit.

[christor]

I'm seeing something pretty similar to this, where there are multiple @PreAuthorize-annotated methods defined in an interface (A), which is implemented by (B), and this implementation is extended by (C).

What I'm seeing is that, for tests anyway, one of the annotated methods on (C) -- which is called save -- behaves as if the annotation isn't there while the rest -- find, update, etc -- function as expected, enforcing the authorization. All methods are overridden in (C).

If I add that same annotation (copy & paste) to the save method in (C), then things work as epected (authorization is enforced).

This is in Java, where I'm upgrading from an old version of Spring libraries where these tests passed to current ones where I'm seeing this problem. Versions:

spring-boot: 1.5.7 (upgraded from 1.3.1) spring-core: 4.3.11 (upgraded from 4.2.4) spring-security: 4.2.3 (upgraded from 4.0.3)

I've reviewed the dependencies list and everything looks like it should work together, but it's possible I've missed something.

I'll see if I can find time to create a shareable bit of code to reproduce it. Any thoughts? Thanks.

[christor]

I spent a bunch of time stepping through the debugger and it looks like it may be related to this call in the searchOnInterfaces method in AnnotationUtils.java ...

Method equivalentMethod = iface.getMethod(method.getName(), method.getParameterTypes());

The interface class (A) is generic, and the parameterTypes don't match up with the actual parameters from (C), where the generic type is narrower. This causes this annotation to not be seen.

This doesn't explain why this is only a problem for one method, and I still have more debugging to do. Hopefully someone more familiar with the code sees this and can offer some suggestions.

Thanks

[christor]

Dependencies:

antlr:antlr:2.7.7 aopalliance:aopalliance:1.0 c3p0:c3p0:0.9.1.1 ca.juliusdavies:not-yet-commons-ssl:0.3.9 ch.qos.logback:logback-classic:1.1.11 ch.qos.logback:logback-core:1.1.11 com.amazonaws:aws-java-sdk-cloudformation:1.11.125 com.amazonaws:aws-java-sdk-core:1.11.125 com.amazonaws:aws-java-sdk-ec2:1.11.125 com.amazonaws:aws-java-sdk-elasticache:1.11.38 com.amazonaws:aws-java-sdk-kms:1.11.125 com.amazonaws:aws-java-sdk-s3:1.11.125 com.amazonaws:aws-java-sdk-sqs:1.11.38 com.amazonaws:jmespath-java:1.11.125 com.auth0:java-jwt:2.1.0 com.fasterxml.jackson.core:jackson-annotations:2.8.0 com.fasterxml.jackson.core:jackson-core:2.8.10 com.fasterxml.jackson.core:jackson-databind:2.8.10 com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.8.10 com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.8.6 com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.8.10 com.fasterxml.jackson.module:jackson-module-afterburner:2.8.10 com.fasterxml:classmate:1.3.4 com.google.code.findbugs:jsr305:3.0.1 com.google.guava:guava:19.0 com.mangofactory:swagger-models:1.0.2 com.mangofactory:swagger-springmvc:1.0.2 com.netflix.archaius:archaius-core:0.7.4 com.netflix.hystrix:hystrix-core:1.5.12 com.netflix.hystrix:hystrix-javanica:1.5.12 com.netflix.hystrix:hystrix-metrics-event-stream:1.5.12 com.netflix.hystrix:hystrix-serialization:1.5.12 com.netflix.netflix-commons:netflix-commons-util:0.1.1 com.netflix.netflix-commons:netflix-statistics:0.1.1 com.netflix.ribbon:ribbon-core:2.2.2 com.netflix.ribbon:ribbon-httpclient:2.2.2 com.netflix.ribbon:ribbon-loadbalancer:2.2.2 com.netflix.ribbon:ribbon-transport:2.2.2 com.netflix.ribbon:ribbon:2.2.2 com.netflix.servo:servo-core:0.10.1 com.netflix.servo:servo-internal:0.10.1 com.opencsv:opencsv:3.2 com.segment.backo:backo:1.0.0 com.squareup.okhttp:okhttp:2.5.0 com.squareup.okio:okio:1.6.0 com.squareup.retrofit:converter-jackson:2.0.0-beta1 com.squareup.retrofit:retrofit:2.0.0-beta1 com.sun.jersey.contribs:jersey-apache-client4:1.19.1 com.sun.jersey:jersey-client:1.19.1 com.sun.jersey:jersey-core:1.19.1 com.wordnik:swagger-annotations:1.3.11 commons-beanutils:commons-beanutils:1.9.3 commons-codec:commons-codec:1.10 commons-collections:commons-collections:3.2.2 commons-configuration:commons-configuration:1.8 commons-digester:commons-digester:2.1 commons-httpclient:commons-httpclient:3.1 commons-io:commons-io:2.4 commons-lang:commons-lang:2.6 commons-logging:commons-logging:1.2 commons-net:commons-net:3.4 commons-validator:commons-validator:1.5.0 dom4j:dom4j:1.6.1 io.netty:netty-buffer:4.0.27.Final io.netty:netty-codec-http:4.0.27.Final io.netty:netty-codec:4.0.27.Final io.netty:netty-common:4.0.27.Final io.netty:netty-handler:4.0.27.Final io.netty:netty-transport-native-epoll:4.0.27.Final io.netty:netty-transport:4.0.27.Final io.reactivex:rxjava:1.2.0 io.reactivex:rxnetty-contexts:0.4.9 io.reactivex:rxnetty-servo:0.4.9 io.reactivex:rxnetty:0.4.9 javax.annotation:javax.annotation-api:1.2 javax.inject:javax.inject:1 javax.money:money-api:1.0 javax.transaction:javax.transaction-api:1.2 javax.validation:validation-api:1.1.0.Final javax.ws.rs:jsr311-api:1.1.1 joda-time:joda-time:2.9.9 mysql:mysql-connector-java:5.1.44 net.logstash.logback:logstash-logback-encoder:4.1 net.sf.dozer:dozer:5.5.1 org.apache.commons:commons-collections4:4.1 org.apache.commons:commons-lang3:3.4 org.apache.commons:commons-math3:3.6 org.apache.httpcomponents:httpclient:4.5.3 org.apache.httpcomponents:httpcore:4.4.6 org.apache.santuario:xmlsec:1.5.7 org.apache.tomcat.embed:tomcat-embed-core:8.5.20 org.apache.tomcat.embed:tomcat-embed-el:8.5.20 org.apache.tomcat.embed:tomcat-embed-websocket:8.5.20 org.apache.tomcat:tomcat-jdbc:8.5.20 org.apache.tomcat:tomcat-juli:8.5.20 org.apache.velocity:velocity:1.7 org.aspectj:aspectjweaver:1.8.10 org.bouncycastle:bcpkix-jdk15on:1.55 org.bouncycastle:bcprov-jdk15on:1.55 org.codehaus.janino:commons-compiler:2.7.8 org.codehaus.janino:janino:2.7.8 org.hdrhistogram:HdrHistogram:2.1.9 org.hibernate.common:hibernate-commons-annotations:5.0.1.Final org.hibernate.javax.persistence:hibernate-jpa-2.1-api:1.0.0.Final org.hibernate:hibernate-core:5.0.12.Final org.hibernate:hibernate-entitymanager:5.0.12.Final org.hibernate:hibernate-envers:5.0.12.Final org.hibernate:hibernate-validator:5.3.5.Final org.javamoney:moneta:1.0 org.javassist:javassist:3.21.0-GA org.jboss.logging:jboss-logging:3.3.1.Final org.jboss:jandex:2.0.0.Final org.liquibase:liquibase-core:3.5.3 org.opensaml:opensaml:2.6.4 org.opensaml:openws:1.5.4 org.opensaml:xmltooling:1.4.4 org.ow2.asm:asm:5.0.4 org.owasp.esapi:esapi:2.0.1 org.projectlombok:lombok:1.16.18 org.quartz-scheduler:quartz:2.2.1 org.scala-lang:scala-library:2.10.4 org.slf4j:jcl-over-slf4j:1.7.25 org.slf4j:jul-to-slf4j:1.7.25 org.slf4j:log4j-over-slf4j:1.7.25 org.slf4j:slf4j-api:1.7.25 org.springframework.boot:spring-boot-actuator:1.5.7.RELEASE org.springframework.boot:spring-boot-autoconfigure:1.5.7.RELEASE org.springframework.boot:spring-boot-starter-actuator:1.5.7.RELEASE org.springframework.boot:spring-boot-starter-aop:1.5.7.RELEASE org.springframework.boot:spring-boot-starter-data-jpa:1.5.7.RELEASE org.springframework.boot:spring-boot-starter-jdbc:1.5.7.RELEASE org.springframework.boot:spring-boot-starter-logging:1.5.7.RELEASE org.springframework.boot:spring-boot-starter-security:1.5.7.RELEASE org.springframework.boot:spring-boot-starter-tomcat:1.5.7.RELEASE org.springframework.boot:spring-boot-starter-web:1.5.7.RELEASE org.springframework.boot:spring-boot-starter:1.5.7.RELEASE org.springframework.boot:spring-boot:1.5.7.RELEASE org.springframework.cloud:spring-cloud-aws-autoconfigure:1.2.1.RELEASE org.springframework.cloud:spring-cloud-aws-context:1.2.1.RELEASE org.springframework.cloud:spring-cloud-aws-core:1.2.1.RELEASE org.springframework.cloud:spring-cloud-commons:1.2.3.RELEASE org.springframework.cloud:spring-cloud-context:1.2.3.RELEASE org.springframework.cloud:spring-cloud-netflix-core:1.3.4.RELEASE org.springframework.cloud:spring-cloud-starter-archaius:1.3.4.RELEASE org.springframework.cloud:spring-cloud-starter-aws:1.2.1.RELEASE org.springframework.cloud:spring-cloud-starter-hystrix:1.3.4.RELEASE org.springframework.cloud:spring-cloud-starter-ribbon:1.3.4.RELEASE org.springframework.cloud:spring-cloud-starter:1.2.3.RELEASE org.springframework.data:spring-data-commons:1.13.7.RELEASE org.springframework.data:spring-data-jpa:1.11.7.RELEASE org.springframework.security:spring-security-aspects:4.2.3.RELEASE org.springframework.security:spring-security-config:4.2.3.RELEASE org.springframework.security:spring-security-core:4.2.3.RELEASE org.springframework.security:spring-security-crypto:4.2.3.RELEASE org.springframework.security:spring-security-rsa:1.0.3.RELEASE org.springframework.security:spring-security-web:4.2.3.RELEASE org.springframework:spring-aop:4.3.11.RELEASE org.springframework:spring-aspects:4.3.11.RELEASE org.springframework:spring-beans:4.3.11.RELEASE org.springframework:spring-context-support:4.3.11.RELEASE org.springframework:spring-context:4.3.11.RELEASE org.springframework:spring-core:4.3.11.RELEASE org.springframework:spring-expression:4.3.11.RELEASE org.springframework:spring-jdbc:4.3.11.RELEASE org.springframework:spring-orm:4.3.11.RELEASE org.springframework:spring-tx:4.3.11.RELEASE org.springframework:spring-web:4.3.11.RELEASE org.springframework:spring-webmvc:4.3.11.RELEASE org.togglz:togglz-console:2.2.0.Final org.togglz:togglz-core:2.2.0.Final org.togglz:togglz-servlet:2.2.0.Final org.togglz:togglz-spring-core:2.2.0.Final org.togglz:togglz-spring-web:2.2.0.Final org.yaml:snakeyaml:1.17 project :foundation-core project :foundation-core-web project :foundation-data project :foundation-services software.amazon.ion:ion-java:1.0.2

[christor]

I've reimplemented searchOnInterfaces using commons-lang's MethodUtils.getMatchingAccessibleMethod method.

https://commons.apache.org/proper/commons-lang/javadocs/api-3.1/org/apache/commons/lang3/reflect/MethodUtils.html#getMatchingAccessibleMethod(java.lang.Class, java.lang.String, java.lang.Class...)

    private static <A extends Annotation> A searchOnInterfaces(Method method, Class<A> annotationType, Class<?>... ifcs) {
        A annotation = null;
        for (Class<?> iface : ifcs) {
            if (isInterfaceWithAnnotatedMethods(iface)) {
                Method equivalentMethod = MethodUtils.getMatchingAccessibleMethod(iface, method.getName(), method.getParameterTypes());
                if (equivalentMethod != null) {
                    annotation = getAnnotation(equivalentMethod, annotationType);
                }
                if (annotation != null) {
                    break;
                }
            }
        }
        return annotation;
    }

This corrects the issue for me, where the annotation is on an interface method. There may still be cases where the same problem exists extending a class with generic method arguments.

[christor]

Creted a pull request https://github.com/spring-projects/spring-framework/pull/1553

[izeye]

This looks resolved via https://jira.spring.io/browse/SPR-16060.

See https://github.com/spring-projects/spring-framework/pull/1553

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[spring-projects-issues]

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.