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 223fc3f8fc2..b0fb127e88c 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. @@ -47,6 +47,7 @@ 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; @@ -1007,6 +1008,21 @@ public void methodWhenMetaAnnotationPropertiesHasClassProperties() { assertThat(service.getIdPath("uid")).isEqualTo("uid"); } + // 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 3bc69949c81..57731a35982 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. @@ -42,6 +42,7 @@ import org.springframework.aop.Advisor; 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; @@ -75,7 +76,7 @@ * @since 6.3 */ public final class AuthorizationAdvisorProxyFactory - implements AuthorizationProxyFactory, Iterable { + implements AuthorizationProxyFactory, Iterable, SmartInitializingSingleton { private static final boolean isReactivePresent = ClassUtils.isPresent("reactor.core.publisher.Mono", null); @@ -108,7 +109,9 @@ public static AuthorizationAdvisorProxyFactory withDefaults() { advisors.add(AuthorizationManagerAfterMethodInterceptor.postAuthorize()); advisors.add(new PreFilterAuthorizationMethodInterceptor()); advisors.add(new PostFilterAuthorizationMethodInterceptor()); - return new AuthorizationAdvisorProxyFactory(advisors); + AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(advisors); + AnnotationAwareOrderComparator.sort(factory.advisors); + return factory; } /** @@ -123,7 +126,14 @@ public static AuthorizationAdvisorProxyFactory withReactiveDefaults() { advisors.add(AuthorizationManagerAfterReactiveMethodInterceptor.postAuthorize()); advisors.add(new PreFilterAuthorizationReactiveMethodInterceptor()); advisors.add(new PostFilterAuthorizationReactiveMethodInterceptor()); - return new AuthorizationAdvisorProxyFactory(advisors); + AuthorizationAdvisorProxyFactory factory = new AuthorizationAdvisorProxyFactory(advisors); + AnnotationAwareOrderComparator.sort(factory.advisors); + return factory; + } + + @Override + public void afterSingletonsInstantiated() { + AnnotationAwareOrderComparator.sort(this.advisors); } /** @@ -146,7 +156,6 @@ public static AuthorizationAdvisorProxyFactory withReactiveDefaults() { */ @Override public Object proxy(Object target) { - AnnotationAwareOrderComparator.sort(this.advisors); if (target == null) { return null; } @@ -155,9 +164,9 @@ public Object proxy(Object target) { return proxied; } ProxyFactory factory = new ProxyFactory(target); - for (Advisor advisor : this.advisors) { - factory.addAdvisors(advisor); - } + List advisors = new ArrayList<>(this.advisors); + AnnotationAwareOrderComparator.sort(advisors); + factory.addAdvisors(advisors); factory.setProxyTargetClass(!Modifier.isFinal(target.getClass().getModifiers())); return factory.getProxy(); } 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 15389c2ec2c..0b325cbd0cb 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. @@ -38,6 +38,7 @@ 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; @@ -336,6 +337,32 @@ public void setTargetVisitorIgnoreValueTypesThenIgnores() { assertThat(factory.proxy(35)).isEqualTo(35); } + // 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()); }