Skip to content

@EnableMethodSecurity doesn't resolve annotations on interfaces through a Proxy #11177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import org.aopalliance.intercept.MethodInvocation;

import org.springframework.aop.support.AopUtils;
import org.springframework.core.MethodClassKey;
import org.springframework.lang.NonNull;
import org.springframework.security.authorization.AuthorizationManager;
Expand All @@ -46,7 +45,7 @@ abstract class AbstractAuthorizationManagerRegistry {
final AuthorizationManager<MethodInvocation> getManager(MethodInvocation methodInvocation) {
Method method = methodInvocation.getMethod();
Object target = methodInvocation.getThis();
Class<?> targetClass = (target != null) ? AopUtils.getTargetClass(target) : null;
Class<?> targetClass = (target != null) ? target.getClass() : null;
MethodClassKey cacheKey = new MethodClassKey(method, targetClass);
return this.cachedManagers.computeIfAbsent(cacheKey, (k) -> resolveManager(method, targetClass));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import org.aopalliance.intercept.MethodInvocation;

import org.springframework.aop.support.AopUtils;
import org.springframework.core.MethodClassKey;
import org.springframework.lang.NonNull;

Expand All @@ -43,7 +42,7 @@ abstract class AbstractExpressionAttributeRegistry<T extends ExpressionAttribute
final T getAttribute(MethodInvocation mi) {
Method method = mi.getMethod();
Object target = mi.getThis();
Class<?> targetClass = (target != null) ? AopUtils.getTargetClass(target) : null;
Class<?> targetClass = (target != null) ? target.getClass() : null;
return getAttribute(method, targetClass);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import org.junit.jupiter.api.Test;

import org.springframework.aop.TargetClassAware;
import org.springframework.core.annotation.AnnotationConfigurationException;
import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler;
Expand Down Expand Up @@ -133,6 +134,19 @@ public void checkInheritedAnnotationsWhenConflictingThenAnnotationConfigurationE
.isThrownBy(() -> manager.check(authentication, methodInvocation));
}

@Test
public void checkTargetClassAwareWhenInterfaceLevelAnnotationsThenApplies() throws Exception {
MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestTargetClassAware(),
TestTargetClassAware.class, "doSomething");
PreAuthorizeAuthorizationManager manager = new PreAuthorizeAuthorizationManager();
AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation);
assertThat(decision).isNotNull();
assertThat(decision.isGranted()).isFalse();
decision = manager.check(TestAuthentication::authenticatedAdmin, methodInvocation);
assertThat(decision).isNotNull();
assertThat(decision.isGranted()).isTrue();
}

public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {

public void doSomething() {
Expand Down Expand Up @@ -198,4 +212,33 @@ public interface InterfaceAnnotationsThree {

}

@PreAuthorize("hasRole('ADMIN')")
public interface InterfaceLevelAnnotations {

}

public static class TestTargetClassAware extends TestClass implements TargetClassAware, InterfaceLevelAnnotations {

@Override
public Class<?> getTargetClass() {
return TestClass.class;
}

@Override
public void doSomething() {
super.doSomething();
}

@Override
public String doSomethingString(String s) {
return super.doSomethingString(s);
}

@Override
public void inheritedAnnotations() {
super.inheritedAnnotations();
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import org.junit.jupiter.api.Test;

import org.springframework.aop.TargetClassAware;
import org.springframework.core.annotation.AnnotationConfigurationException;
import org.springframework.security.access.annotation.Secured;
import org.springframework.security.access.intercept.method.MockMethodInvocation;
Expand Down Expand Up @@ -127,6 +128,19 @@ public void checkInheritedAnnotationsWhenConflictingThenAnnotationConfigurationE
.isThrownBy(() -> manager.check(authentication, methodInvocation));
}

@Test
public void checkTargetClassAwareWhenInterfaceLevelAnnotationsThenApplies() throws Exception {
MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestTargetClassAware(),
TestTargetClassAware.class, "doSomething");
SecuredAuthorizationManager manager = new SecuredAuthorizationManager();
AuthorizationDecision decision = manager.check(TestAuthentication::authenticatedUser, methodInvocation);
assertThat(decision).isNotNull();
assertThat(decision.isGranted()).isFalse();
decision = manager.check(TestAuthentication::authenticatedAdmin, methodInvocation);
assertThat(decision).isNotNull();
assertThat(decision.isGranted()).isTrue();
}

public static class TestClass implements InterfaceAnnotationsOne, InterfaceAnnotationsTwo {

public void doSomething() {
Expand Down Expand Up @@ -192,4 +206,33 @@ public interface InterfaceAnnotationsThree {

}

@Secured("ROLE_ADMIN")
public interface InterfaceLevelAnnotations {

}

public static class TestTargetClassAware extends TestClass implements TargetClassAware, InterfaceLevelAnnotations {

@Override
public Class<?> getTargetClass() {
return TestClass.class;
}

@Override
public void doSomething() {
super.doSomething();
}

@Override
public void securedUserOrAdmin() {
super.securedUserOrAdmin();
}

@Override
public void inheritedAnnotations() {
super.inheritedAnnotations();
}

}

}