Skip to content

Commit 54ba608

Browse files
committed
Address Feedback
- Changed to template delimiters - Don't synthesize unnecessarily
1 parent ed5305c commit 54ba608

File tree

3 files changed

+25
-21
lines changed

3 files changed

+25
-21
lines changed

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import org.springframework.context.annotation.Import;
4242
import org.springframework.context.annotation.Role;
4343
import org.springframework.core.annotation.AnnotationConfigurationException;
44-
import org.springframework.expression.spel.SpelParseException;
4544
import org.springframework.security.access.AccessDeniedException;
4645
import org.springframework.security.access.PermissionEvaluator;
4746
import org.springframework.security.access.annotation.BusinessService;
@@ -614,7 +613,7 @@ public void methodRoleWhenPreAuthorizeMetaAnnotationHardcodedParameterThenPasses
614613
public void methodWhenParameterizedAnnotationThenFails() {
615614
this.spring.register(MetaAnnotationPlaceholderConfig.class).autowire();
616615
MetaAnnotationService service = this.spring.getContext().getBean(MetaAnnotationService.class);
617-
assertThatExceptionOfType(SpelParseException.class)
616+
assertThatExceptionOfType(IllegalArgumentException.class)
618617
.isThrownBy(service::placeholdersOnlyResolvedByMetaAnnotations);
619618
}
620619

@@ -988,7 +987,7 @@ boolean hasUserRole() {
988987
return true;
989988
}
990989

991-
@PreAuthorize("hasRole(${role})")
990+
@PreAuthorize("hasRole({role})")
992991
void placeholdersOnlyResolvedByMetaAnnotations() {
993992
}
994993

@@ -1015,15 +1014,15 @@ List<String> resultsContainDave(List<String> list) {
10151014
}
10161015

10171016
@Retention(RetentionPolicy.RUNTIME)
1018-
@PreAuthorize("hasRole(${role})")
1017+
@PreAuthorize("hasRole({role})")
10191018
@interface RequireRole {
10201019

10211020
String role();
10221021

10231022
}
10241023

10251024
@Retention(RetentionPolicy.RUNTIME)
1026-
@PreAuthorize("hasAuthority('SCOPE_${claim}') || hasAnyRole(${roles})")
1025+
@PreAuthorize("hasAuthority('SCOPE_{claim}') || hasAnyRole({roles})")
10271026
@interface HasClaim {
10281027

10291028
String claim();
@@ -1033,23 +1032,23 @@ List<String> resultsContainDave(List<String> list) {
10331032
}
10341033

10351034
@Retention(RetentionPolicy.RUNTIME)
1036-
@PostAuthorize("returnObject.startsWith('${value}')")
1035+
@PostAuthorize("returnObject.startsWith('{value}')")
10371036
@interface ResultStartsWith {
10381037

10391038
String value();
10401039

10411040
}
10421041

10431042
@Retention(RetentionPolicy.RUNTIME)
1044-
@PreFilter("filterObject.contains('${value}')")
1043+
@PreFilter("filterObject.contains('{value}')")
10451044
@interface ParameterContains {
10461045

10471046
String value();
10481047

10491048
}
10501049

10511050
@Retention(RetentionPolicy.RUNTIME)
1052-
@PostFilter("filterObject.contains('${value}')")
1051+
@PostFilter("filterObject.contains('{value}')")
10531052
@interface ResultContains {
10541053

10551054
String value();

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

Lines changed: 14 additions & 9 deletions
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,8 @@
2930
import org.springframework.core.annotation.MergedAnnotations;
3031
import org.springframework.core.annotation.MergedAnnotations.SearchStrategy;
3132
import org.springframework.core.annotation.RepeatableContainers;
32-
import org.springframework.core.env.MapPropertySource;
33-
import org.springframework.core.env.MutablePropertySources;
34-
import org.springframework.core.env.PropertySourcesPropertyResolver;
33+
import org.springframework.core.convert.support.DefaultConversionService;
34+
import org.springframework.util.PropertyPlaceholderHelper;
3535

3636
/**
3737
* A collection of utility methods that check for, and error on, conflicting annotations.
@@ -58,17 +58,22 @@ final class AuthorizationAnnotationUtils {
5858

5959
static <A extends Annotation> Function<AnnotatedElement, A> withPlaceholderResolver(Class<A> type) {
6060
Function<MergedAnnotation<A>, A> map = (mergedAnnotation) -> {
61-
A annotation = mergedAnnotation.synthesize();
6261
if (mergedAnnotation.getMetaSource() == null) {
63-
return annotation;
62+
return mergedAnnotation.synthesize();
6463
}
64+
PropertyPlaceholderHelper helper = new PropertyPlaceholderHelper("{", "}");
6565
String expression = (String) mergedAnnotation.asMap().get("value");
66-
MutablePropertySources propertySources = new MutablePropertySources();
6766
Map<String, Object> annotationProperties = mergedAnnotation.getMetaSource().asMap();
68-
propertySources.addFirst(new MapPropertySource("annotation", annotationProperties));
69-
PropertySourcesPropertyResolver propertyResolver = new PropertySourcesPropertyResolver(propertySources);
67+
Map<String, String> stringProperties = new HashMap<>();
68+
for (Map.Entry<String, Object> property : annotationProperties.entrySet()) {
69+
String key = property.getKey();
70+
Object value = property.getValue();
71+
String asString = (value instanceof String) ? (String) value
72+
: DefaultConversionService.getSharedInstance().convert(value, String.class);
73+
stringProperties.put(key, asString);
74+
}
7075
AnnotatedElement annotatedElement = (AnnotatedElement) mergedAnnotation.getSource();
71-
String value = propertyResolver.resolvePlaceholders(expression);
76+
String value = helper.replacePlaceholders(expression, stringProperties::get);
7277
return MergedAnnotation.of(annotatedElement, type, Collections.singletonMap("value", value)).synthesize();
7378
};
7479
return (annotatedElement) -> findDistinctAnnotation(annotatedElement, type, map);

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

Lines changed: 4 additions & 4 deletions
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)