From 456604ab459d1e44311458042e283ae02727fd99 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Thu, 27 Mar 2025 15:29:44 -0600 Subject: [PATCH] Sort Default Advisors and Added Advisors This commit ensures that the default advisors and added advisors are sorted in the event that this component is not being published as a Spring bean. Issue gh-16819 --- .../AuthorizationAdvisorProxyFactory.java | 14 +++++---- ...AuthorizationAdvisorProxyFactoryTests.java | 29 ++++++++++++++++++- 2 files changed, 37 insertions(+), 6 deletions(-) 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 16566997c0..57731a3598 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 @@ -109,7 +109,9 @@ public final class AuthorizationAdvisorProxyFactory 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; } /** @@ -124,7 +126,9 @@ public final class AuthorizationAdvisorProxyFactory 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 @@ -160,9 +164,9 @@ public final class AuthorizationAdvisorProxyFactory 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 15389c2ec2..0b325cbd0c 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.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; @@ -336,6 +337,32 @@ public class AuthorizationAdvisorProxyFactoryTests { 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()); }