Skip to content

Commit 1a03924

Browse files
committed
Address Feedback
- Changed to template delimiters - Don't synthesize unnecessarily
1 parent 5f440d1 commit 1a03924

File tree

3 files changed

+28
-17
lines changed

3 files changed

+28
-17
lines changed

config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java

+7-7
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ public void methodRoleWhenPreAuthorizeMetaAnnotationHardcodedParameterThenPasses
614614
public void methodWhenParameterizedAnnotationThenFails() {
615615
this.spring.register(MetaAnnotationPlaceholderConfig.class).autowire();
616616
MetaAnnotationService service = this.spring.getContext().getBean(MetaAnnotationService.class);
617-
assertThatExceptionOfType(SpelParseException.class)
617+
assertThatExceptionOfType(IllegalArgumentException.class)
618618
.isThrownBy(service::placeholdersOnlyResolvedByMetaAnnotations);
619619
}
620620

@@ -988,7 +988,7 @@ boolean hasUserRole() {
988988
return true;
989989
}
990990

991-
@PreAuthorize("hasRole(${role})")
991+
@PreAuthorize("hasRole({role})")
992992
void placeholdersOnlyResolvedByMetaAnnotations() {
993993
}
994994

@@ -1015,15 +1015,15 @@ List<String> resultsContainDave(List<String> list) {
10151015
}
10161016

10171017
@Retention(RetentionPolicy.RUNTIME)
1018-
@PreAuthorize("hasRole(${role})")
1018+
@PreAuthorize("hasRole({role})")
10191019
@interface RequireRole {
10201020

10211021
String role();
10221022

10231023
}
10241024

10251025
@Retention(RetentionPolicy.RUNTIME)
1026-
@PreAuthorize("hasAuthority('SCOPE_${claim}') || hasAnyRole(${roles})")
1026+
@PreAuthorize("hasAuthority('SCOPE_{claim}') || hasAnyRole({roles})")
10271027
@interface HasClaim {
10281028

10291029
String claim();
@@ -1033,23 +1033,23 @@ List<String> resultsContainDave(List<String> list) {
10331033
}
10341034

10351035
@Retention(RetentionPolicy.RUNTIME)
1036-
@PostAuthorize("returnObject.startsWith('${value}')")
1036+
@PostAuthorize("returnObject.startsWith('{value}')")
10371037
@interface ResultStartsWith {
10381038

10391039
String value();
10401040

10411041
}
10421042

10431043
@Retention(RetentionPolicy.RUNTIME)
1044-
@PreFilter("filterObject.contains('${value}')")
1044+
@PreFilter("filterObject.contains('{value}')")
10451045
@interface ParameterContains {
10461046

10471047
String value();
10481048

10491049
}
10501050

10511051
@Retention(RetentionPolicy.RUNTIME)
1052-
@PostFilter("filterObject.contains('${value}')")
1052+
@PostFilter("filterObject.contains('{value}')")
10531053
@interface ResultContains {
10541054

10551055
String value();

core/src/main/java/org/springframework/security/authorization/method/AuthorizationAnnotationUtils.java

+17-6
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.lang.reflect.AnnotatedElement;
2121
import java.lang.reflect.Method;
2222
import java.util.Collections;
23+
import java.util.HashMap;
2324
import java.util.List;
2425
import java.util.Map;
2526
import java.util.function.Function;
@@ -29,9 +30,14 @@
2930
import org.springframework.core.annotation.MergedAnnotations;
3031
import org.springframework.core.annotation.MergedAnnotations.SearchStrategy;
3132
import org.springframework.core.annotation.RepeatableContainers;
33+
import org.springframework.core.convert.ConversionService;
34+
import org.springframework.core.convert.support.DefaultConversionService;
35+
import org.springframework.core.env.AbstractPropertyResolver;
3236
import org.springframework.core.env.MapPropertySource;
3337
import org.springframework.core.env.MutablePropertySources;
3438
import org.springframework.core.env.PropertySourcesPropertyResolver;
39+
import org.springframework.util.PropertyPlaceholderHelper;
40+
import org.springframework.util.SystemPropertyUtils;
3541

3642
/**
3743
* A collection of utility methods that check for, and error on, conflicting annotations.
@@ -58,17 +64,22 @@ final class AuthorizationAnnotationUtils {
5864

5965
static <A extends Annotation> Function<AnnotatedElement, A> withPlaceholderResolver(Class<A> type) {
6066
Function<MergedAnnotation<A>, A> map = (mergedAnnotation) -> {
61-
A annotation = mergedAnnotation.synthesize();
6267
if (mergedAnnotation.getMetaSource() == null) {
63-
return annotation;
68+
return mergedAnnotation.synthesize();
6469
}
70+
PropertyPlaceholderHelper helper = new PropertyPlaceholderHelper("{", "}");
6571
String expression = (String) mergedAnnotation.asMap().get("value");
66-
MutablePropertySources propertySources = new MutablePropertySources();
6772
Map<String, Object> annotationProperties = mergedAnnotation.getMetaSource().asMap();
68-
propertySources.addFirst(new MapPropertySource("annotation", annotationProperties));
69-
PropertySourcesPropertyResolver propertyResolver = new PropertySourcesPropertyResolver(propertySources);
73+
Map<String, String> stringProperties = new HashMap<>();
74+
for (Map.Entry<String, Object> property : annotationProperties.entrySet()) {
75+
String key = property.getKey();
76+
Object value = property.getValue();
77+
String asString = (value instanceof String) ? (String) value
78+
: DefaultConversionService.getSharedInstance().convert(value, String.class);
79+
stringProperties.put(key, asString);
80+
}
7081
AnnotatedElement annotatedElement = (AnnotatedElement) mergedAnnotation.getSource();
71-
String value = propertyResolver.resolvePlaceholders(expression);
82+
String value = helper.replacePlaceholders(expression, stringProperties::get);
7283
return MergedAnnotation.of(annotatedElement, type, Collections.singletonMap("value", value)).synthesize();
7384
};
7485
return (annotatedElement) -> findDistinctAnnotation(annotatedElement, type, map);

docs/modules/ROOT/pages/servlet/authorization/method-security.adoc

+4-4
Original file line numberDiff line numberDiff line change
@@ -911,7 +911,7 @@ Java::
911911
----
912912
@Target({ ElementType.METHOD, ElementType.TYPE })
913913
@Retention(RetentionPolicy.RUNTIME)
914-
@PreAuthorize("hasRole('${value}')")
914+
@PreAuthorize("hasRole('{value}')")
915915
public @interface HasRole {
916916
String value();
917917
}
@@ -923,7 +923,7 @@ Kotlin::
923923
----
924924
@Target(ElementType.METHOD, ElementType.TYPE)
925925
@Retention(RetentionPolicy.RUNTIME)
926-
@PreAuthorize("hasRole('${value}')")
926+
@PreAuthorize("hasRole('{value}')")
927927
annotation class IsAdmin(val value: String)
928928
----
929929
======
@@ -971,7 +971,7 @@ Java::
971971
----
972972
@Target({ ElementType.METHOD, ElementType.TYPE })
973973
@Retention(RetentionPolicy.RUNTIME)
974-
@PreAuthorize("hasAnyRole(${roles})")
974+
@PreAuthorize("hasAnyRole({roles})")
975975
public @interface HasAnyRole {
976976
String roles();
977977
}
@@ -983,7 +983,7 @@ Kotlin::
983983
----
984984
@Target(ElementType.METHOD, ElementType.TYPE)
985985
@Retention(RetentionPolicy.RUNTIME)
986-
@PreAuthorize("hasAnyRole(${roles})")
986+
@PreAuthorize("hasAnyRole({roles})")
987987
annotation class HasAnyRole(val roles: Array<String>)
988988
----
989989
======

0 commit comments

Comments
 (0)