From ea9dd2728ec9df922fe6844aeae0c851d74a4c96 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Mon, 21 Jul 2025 09:27:27 -0500 Subject: [PATCH 1/3] Support add nested security configurers during builder initialization Closes gh-17011 Signed-off-by: DingHao --- .../AbstractConfiguredSecurityBuilder.java | 15 ++++++++-- ...bstractConfiguredSecurityBuilderTests.java | 30 ++++++++++++++++++- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/AbstractConfiguredSecurityBuilder.java b/config/src/main/java/org/springframework/security/config/annotation/AbstractConfiguredSecurityBuilder.java index f89e06ca796..2fd72457c24 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/AbstractConfiguredSecurityBuilder.java +++ b/config/src/main/java/org/springframework/security/config/annotation/AbstractConfiguredSecurityBuilder.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. @@ -50,6 +50,7 @@ * @param The object that this builder returns * @param The type of this builder (that is returned by the base class) * @author Rob Winch + * @author DingHao * @see WebSecurity */ public abstract class AbstractConfiguredSecurityBuilder> @@ -386,8 +387,10 @@ private void init() throws Exception { for (SecurityConfigurer configurer : configurers) { configurer.init((B) this); } - for (SecurityConfigurer configurer : this.configurersAddedInInitializing) { - configurer.init((B) this); + while (!this.configurersAddedInInitializing.isEmpty()) { + for (SecurityConfigurer configurer : getConfigurersInInitializing()) { + configurer.init((B) this); + } } } @@ -407,6 +410,12 @@ private Collection> getConfigurers() { return result; } + private List> getConfigurersInInitializing() { + List> result = new ArrayList<>(this.configurersAddedInInitializing); + this.configurersAddedInInitializing.clear(); + return result; + } + /** * Determines if the object is unbuilt. * @return true, if unbuilt else false diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/AbstractConfiguredSecurityBuilderTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/AbstractConfiguredSecurityBuilderTests.java index 322459e33ac..3dc572f2b40 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/AbstractConfiguredSecurityBuilderTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/AbstractConfiguredSecurityBuilderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 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. @@ -163,6 +163,34 @@ public void withWhenDuplicateConfigurerAddedThenDuplicateConfigurerRemoved() thr assertThat(this.builder.getConfigurers(TestSecurityConfigurer.class)).hasSize(1); } + @Test + public void withWhenConfigurerAddInitializing() throws Exception { + this.builder.with(new FooConfigurer(), Customizer.withDefaults()); + assertThat(this.builder.build()).isEqualTo("success"); + } + + private static class FooConfigurer extends SecurityConfigurerAdapter { + + @Override + public void init(TestConfiguredSecurityBuilder builder) throws Exception { + builder.with(new BarConfigurer(), Customizer.withDefaults()); + } + + } + + private static class BarConfigurer extends SecurityConfigurerAdapter { + + @Override + public void init(TestConfiguredSecurityBuilder http) throws Exception { + http.with(new CooConfigurer(), Customizer.withDefaults()); + } + + } + + private static class CooConfigurer extends SecurityConfigurerAdapter { + + } + private static class ApplyAndRemoveSecurityConfigurer extends SecurityConfigurerAdapter { From fca704e61f80e1959fa3034a84eafe1ebe7c4b4c Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Mon, 21 Jul 2025 09:01:48 -0500 Subject: [PATCH 2/3] Fix getConfigurersInInitializing Semantics A getter should not mutate state. This removes getConfigurersInInitializing in favor of inline code since this is just used once. Issue gh-17020 gh-17011 Signed-off-by: Rob Winch <362503+rwinch@users.noreply.github.com> --- .../AbstractConfiguredSecurityBuilder.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/AbstractConfiguredSecurityBuilder.java b/config/src/main/java/org/springframework/security/config/annotation/AbstractConfiguredSecurityBuilder.java index 2fd72457c24..29b4e7be145 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/AbstractConfiguredSecurityBuilder.java +++ b/config/src/main/java/org/springframework/security/config/annotation/AbstractConfiguredSecurityBuilder.java @@ -60,7 +60,7 @@ public abstract class AbstractConfiguredSecurityBuilder>, List>> configurers = new LinkedHashMap<>(); - private final List> configurersAddedInInitializing = new ArrayList<>(); + private List> configurersAddedInInitializing = new ArrayList<>(); private final Map, Object> sharedObjects = new HashMap<>(); @@ -388,7 +388,9 @@ private void init() throws Exception { configurer.init((B) this); } while (!this.configurersAddedInInitializing.isEmpty()) { - for (SecurityConfigurer configurer : getConfigurersInInitializing()) { + List> toInit = this.configurersAddedInInitializing; + this.configurersAddedInInitializing = new ArrayList<>(); + for (SecurityConfigurer configurer : toInit) { configurer.init((B) this); } } @@ -410,12 +412,6 @@ private Collection> getConfigurers() { return result; } - private List> getConfigurersInInitializing() { - List> result = new ArrayList<>(this.configurersAddedInInitializing); - this.configurersAddedInInitializing.clear(); - return result; - } - /** * Determines if the object is unbuilt. * @return true, if unbuilt else false From 829af961f03a83053b03680b063c794935439bc0 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Mon, 21 Jul 2025 09:18:42 -0500 Subject: [PATCH 3/3] Use Meaningful Configurer Names in Test This just renames the Configurer names used in AbstractConfiguredSecurityBuilderTests to be more meaningful. Issue gh-17020 gh-17011 Signed-off-by: Rob Winch <362503+rwinch@users.noreply.github.com> --- .../AbstractConfiguredSecurityBuilderTests.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/AbstractConfiguredSecurityBuilderTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/AbstractConfiguredSecurityBuilderTests.java index 3dc572f2b40..283637bcb76 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/AbstractConfiguredSecurityBuilderTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/AbstractConfiguredSecurityBuilderTests.java @@ -165,29 +165,31 @@ public void withWhenDuplicateConfigurerAddedThenDuplicateConfigurerRemoved() thr @Test public void withWhenConfigurerAddInitializing() throws Exception { - this.builder.with(new FooConfigurer(), Customizer.withDefaults()); + this.builder.with(new AppliesNestedConfigurer(), Customizer.withDefaults()); assertThat(this.builder.build()).isEqualTo("success"); } - private static class FooConfigurer extends SecurityConfigurerAdapter { + private static class AppliesNestedConfigurer + extends SecurityConfigurerAdapter { @Override public void init(TestConfiguredSecurityBuilder builder) throws Exception { - builder.with(new BarConfigurer(), Customizer.withDefaults()); + builder.with(new NestedConfigurer(), Customizer.withDefaults()); } } - private static class BarConfigurer extends SecurityConfigurerAdapter { + private static class NestedConfigurer extends SecurityConfigurerAdapter { @Override public void init(TestConfiguredSecurityBuilder http) throws Exception { - http.with(new CooConfigurer(), Customizer.withDefaults()); + http.with(new DoubleNestedConfigurer(), Customizer.withDefaults()); } } - private static class CooConfigurer extends SecurityConfigurerAdapter { + private static class DoubleNestedConfigurer + extends SecurityConfigurerAdapter { }