diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java index 17d8f8a3a9..7117b8c81b 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -56,6 +56,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.context.annotation.Role; +import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.annotation.AnnotationConfigurationException; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.PermissionEvaluator; @@ -1103,6 +1104,21 @@ public class PrePostMethodSecurityConfigurationTests { verifyNoInteractions(handler); } + // gh-16819 + @Test + void autowireWhenDefaultsThenAdvisorAnnotationsAreSorted() { + this.spring.register(MethodSecurityServiceConfig.class).autowire(); + AuthorizationAdvisorProxyFactory proxyFactory = this.spring.getContext() + .getBean(AuthorizationAdvisorProxyFactory.class); + AnnotationAwareOrderComparator comparator = AnnotationAwareOrderComparator.INSTANCE; + AuthorizationAdvisor previous = null; + for (AuthorizationAdvisor advisor : proxyFactory) { + boolean ordered = previous == null || comparator.compare(previous, advisor) < 0; + assertThat(ordered).isTrue(); + previous = advisor; + } + } + private static Consumer disallowBeanOverriding() { return (context) -> ((AnnotationConfigWebApplicationContext) context).setAllowBeanDefinitionOverriding(false); } diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java index 5cf36b5fa2..3b18466743 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -47,6 +47,7 @@ import org.springframework.aop.Advisor; import org.springframework.aop.Pointcut; import org.springframework.aop.framework.AopInfrastructureBean; import org.springframework.aop.framework.ProxyFactory; +import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.lang.NonNull; import org.springframework.security.authorization.AuthorizationProxyFactory; @@ -79,8 +80,8 @@ import org.springframework.util.ClassUtils; * @author Josh Cummings * @since 6.3 */ -public final class AuthorizationAdvisorProxyFactory - implements AuthorizationProxyFactory, Iterable, AopInfrastructureBean { +public final class AuthorizationAdvisorProxyFactory implements AuthorizationProxyFactory, + Iterable, AopInfrastructureBean, SmartInitializingSingleton { private static final boolean isReactivePresent = ClassUtils.isPresent("reactor.core.publisher.Mono", null); @@ -125,6 +126,7 @@ public final class AuthorizationAdvisorProxyFactory advisors.add(new PostFilterAuthorizationMethodInterceptor()); AuthorizationAdvisorProxyFactory proxyFactory = new AuthorizationAdvisorProxyFactory(advisors); proxyFactory.addAdvisor(new AuthorizeReturnObjectMethodInterceptor(proxyFactory)); + AnnotationAwareOrderComparator.sort(proxyFactory.advisors); return proxyFactory; } @@ -142,9 +144,15 @@ public final class AuthorizationAdvisorProxyFactory advisors.add(new PostFilterAuthorizationReactiveMethodInterceptor()); AuthorizationAdvisorProxyFactory proxyFactory = new AuthorizationAdvisorProxyFactory(advisors); proxyFactory.addAdvisor(new AuthorizeReturnObjectMethodInterceptor(proxyFactory)); + AnnotationAwareOrderComparator.sort(proxyFactory.advisors); return proxyFactory; } + @Override + public void afterSingletonsInstantiated() { + AnnotationAwareOrderComparator.sort(this.advisors); + } + /** * Proxy an object to enforce authorization advice. * @@ -165,7 +173,6 @@ public final class AuthorizationAdvisorProxyFactory */ @Override public Object proxy(Object target) { - AnnotationAwareOrderComparator.sort(this.advisors); if (target == null) { return null; } @@ -178,9 +185,9 @@ public final class AuthorizationAdvisorProxyFactory } ProxyFactory factory = new ProxyFactory(target); factory.addAdvisors(this.authorizationProxy); - for (Advisor advisor : this.advisors) { - factory.addAdvisors(advisor); - } + List advisors = new ArrayList<>(this.advisors); + AnnotationAwareOrderComparator.sort(advisors); + factory.addAdvisors(advisors); factory.addInterface(AuthorizationProxy.class); factory.setOpaque(true); factory.setProxyTargetClass(!Modifier.isFinal(target.getClass().getModifiers())); diff --git a/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java b/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java index aa5a249963..93d7ee1520 100644 --- a/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java +++ b/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -40,6 +40,7 @@ import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Test; import org.springframework.aop.Pointcut; +import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.authentication.TestAuthentication; @@ -360,6 +361,32 @@ public class AuthorizationAdvisorProxyFactoryTests { assertThat(target).isSameAs(this.flight); } + // gh-16819 + @Test + void advisorsWhenWithDefaultsThenAreSorted() { + AuthorizationAdvisorProxyFactory proxyFactory = AuthorizationAdvisorProxyFactory.withDefaults(); + AnnotationAwareOrderComparator comparator = AnnotationAwareOrderComparator.INSTANCE; + AuthorizationAdvisor previous = null; + for (AuthorizationAdvisor advisor : proxyFactory) { + boolean ordered = previous == null || comparator.compare(previous, advisor) < 0; + assertThat(ordered).isTrue(); + previous = advisor; + } + } + + // gh-16819 + @Test + void advisorsWhenWithReactiveDefaultsThenAreSorted() { + AuthorizationAdvisorProxyFactory proxyFactory = AuthorizationAdvisorProxyFactory.withReactiveDefaults(); + AnnotationAwareOrderComparator comparator = AnnotationAwareOrderComparator.INSTANCE; + AuthorizationAdvisor previous = null; + for (AuthorizationAdvisor advisor : proxyFactory) { + boolean ordered = previous == null || comparator.compare(previous, advisor) < 0; + assertThat(ordered).isTrue(); + previous = advisor; + } + } + private Authentication authenticated(String user, String... authorities) { return TestAuthentication.authenticated(TestAuthentication.withUsername(user).authorities(authorities).build()); }