Skip to content

Commit e6bea1c

Browse files
DerChris173jzheaux
authored andcommitted
Polish RoleHierarchy Bean Usage
Issue gh-12783
1 parent b76f7c0 commit e6bea1c

File tree

6 files changed

+44
-35
lines changed

6 files changed

+44
-35
lines changed

config/src/main/java/org/springframework/security/config/annotation/method/configuration/Jsr250MethodSecurityConfiguration.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
import org.springframework.beans.factory.ObjectProvider;
2424
import org.springframework.beans.factory.config.BeanDefinition;
25-
import org.springframework.context.ApplicationContext;
2625
import org.springframework.context.annotation.Bean;
2726
import org.springframework.context.annotation.Configuration;
2827
import org.springframework.context.annotation.Role;
@@ -53,11 +52,10 @@ final class Jsr250MethodSecurityConfiguration {
5352
static MethodInterceptor jsr250AuthorizationMethodInterceptor(
5453
ObjectProvider<GrantedAuthorityDefaults> defaultsProvider,
5554
ObjectProvider<SecurityContextHolderStrategy> strategyProvider,
56-
ObjectProvider<ObservationRegistry> registryProvider, ApplicationContext context) {
55+
ObjectProvider<ObservationRegistry> registryProvider, ObjectProvider<RoleHierarchy> roleHierarchyProvider) {
5756
Jsr250AuthorizationManager jsr250 = new Jsr250AuthorizationManager();
5857
AuthoritiesAuthorizationManager authoritiesAuthorizationManager = new AuthoritiesAuthorizationManager();
59-
RoleHierarchy roleHierarchy = (context.getBeanNamesForType(RoleHierarchy.class).length > 0)
60-
? context.getBean(RoleHierarchy.class) : new NullRoleHierarchy();
58+
RoleHierarchy roleHierarchy = roleHierarchyProvider.getIfAvailable(NullRoleHierarchy::new);
6159
authoritiesAuthorizationManager.setRoleHierarchy(roleHierarchy);
6260
jsr250.setAuthoritiesAuthorizationManager(authoritiesAuthorizationManager);
6361
defaultsProvider.ifAvailable((d) -> jsr250.setRolePrefix(d.getRolePrefix()));

config/src/main/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfiguration.java

+23-18
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,12 @@ final class PrePostMethodSecurityConfiguration {
6565
static MethodInterceptor preFilterAuthorizationMethodInterceptor(
6666
ObjectProvider<GrantedAuthorityDefaults> defaultsProvider,
6767
ObjectProvider<MethodSecurityExpressionHandler> expressionHandlerProvider,
68-
ObjectProvider<SecurityContextHolderStrategy> strategyProvider, ApplicationContext context) {
68+
ObjectProvider<SecurityContextHolderStrategy> strategyProvider,
69+
ObjectProvider<RoleHierarchy> roleHierarchyProvider, ApplicationContext context) {
6970
PreFilterAuthorizationMethodInterceptor preFilter = new PreFilterAuthorizationMethodInterceptor();
7071
strategyProvider.ifAvailable(preFilter::setSecurityContextHolderStrategy);
71-
preFilter.setExpressionHandler(
72-
new DeferringMethodSecurityExpressionHandler(expressionHandlerProvider, defaultsProvider, context));
72+
preFilter.setExpressionHandler(new DeferringMethodSecurityExpressionHandler(expressionHandlerProvider,
73+
defaultsProvider, roleHierarchyProvider, context));
7374
return preFilter;
7475
}
7576

@@ -80,10 +81,11 @@ static MethodInterceptor preAuthorizeAuthorizationMethodInterceptor(
8081
ObjectProvider<MethodSecurityExpressionHandler> expressionHandlerProvider,
8182
ObjectProvider<SecurityContextHolderStrategy> strategyProvider,
8283
ObjectProvider<AuthorizationEventPublisher> eventPublisherProvider,
83-
ObjectProvider<ObservationRegistry> registryProvider, ApplicationContext context) {
84+
ObjectProvider<ObservationRegistry> registryProvider, ObjectProvider<RoleHierarchy> roleHierarchyProvider,
85+
ApplicationContext context) {
8486
PreAuthorizeAuthorizationManager manager = new PreAuthorizeAuthorizationManager();
85-
manager.setExpressionHandler(
86-
new DeferringMethodSecurityExpressionHandler(expressionHandlerProvider, defaultsProvider, context));
87+
manager.setExpressionHandler(new DeferringMethodSecurityExpressionHandler(expressionHandlerProvider,
88+
defaultsProvider, roleHierarchyProvider, context));
8789
AuthorizationManagerBeforeMethodInterceptor preAuthorize = AuthorizationManagerBeforeMethodInterceptor
8890
.preAuthorize(manager(manager, registryProvider));
8991
strategyProvider.ifAvailable(preAuthorize::setSecurityContextHolderStrategy);
@@ -98,10 +100,11 @@ static MethodInterceptor postAuthorizeAuthorizationMethodInterceptor(
98100
ObjectProvider<MethodSecurityExpressionHandler> expressionHandlerProvider,
99101
ObjectProvider<SecurityContextHolderStrategy> strategyProvider,
100102
ObjectProvider<AuthorizationEventPublisher> eventPublisherProvider,
101-
ObjectProvider<ObservationRegistry> registryProvider, ApplicationContext context) {
103+
ObjectProvider<ObservationRegistry> registryProvider, ObjectProvider<RoleHierarchy> roleHierarchyProvider,
104+
ApplicationContext context) {
102105
PostAuthorizeAuthorizationManager manager = new PostAuthorizeAuthorizationManager();
103-
manager.setExpressionHandler(
104-
new DeferringMethodSecurityExpressionHandler(expressionHandlerProvider, defaultsProvider, context));
106+
manager.setExpressionHandler(new DeferringMethodSecurityExpressionHandler(expressionHandlerProvider,
107+
defaultsProvider, roleHierarchyProvider, context));
105108
AuthorizationManagerAfterMethodInterceptor postAuthorize = AuthorizationManagerAfterMethodInterceptor
106109
.postAuthorize(manager(manager, registryProvider));
107110
strategyProvider.ifAvailable(postAuthorize::setSecurityContextHolderStrategy);
@@ -114,19 +117,20 @@ static MethodInterceptor postAuthorizeAuthorizationMethodInterceptor(
114117
static MethodInterceptor postFilterAuthorizationMethodInterceptor(
115118
ObjectProvider<GrantedAuthorityDefaults> defaultsProvider,
116119
ObjectProvider<MethodSecurityExpressionHandler> expressionHandlerProvider,
117-
ObjectProvider<SecurityContextHolderStrategy> strategyProvider, ApplicationContext context) {
120+
ObjectProvider<SecurityContextHolderStrategy> strategyProvider,
121+
ObjectProvider<RoleHierarchy> roleHierarchyProvider, ApplicationContext context) {
118122
PostFilterAuthorizationMethodInterceptor postFilter = new PostFilterAuthorizationMethodInterceptor();
119123
strategyProvider.ifAvailable(postFilter::setSecurityContextHolderStrategy);
120-
postFilter.setExpressionHandler(
121-
new DeferringMethodSecurityExpressionHandler(expressionHandlerProvider, defaultsProvider, context));
124+
postFilter.setExpressionHandler(new DeferringMethodSecurityExpressionHandler(expressionHandlerProvider,
125+
defaultsProvider, roleHierarchyProvider, context));
122126
return postFilter;
123127
}
124128

125129
private static MethodSecurityExpressionHandler defaultExpressionHandler(
126-
ObjectProvider<GrantedAuthorityDefaults> defaultsProvider, ApplicationContext context) {
130+
ObjectProvider<GrantedAuthorityDefaults> defaultsProvider,
131+
ObjectProvider<RoleHierarchy> roleHierarchyProvider, ApplicationContext context) {
127132
DefaultMethodSecurityExpressionHandler handler = new DefaultMethodSecurityExpressionHandler();
128-
RoleHierarchy roleHierarchy = (context.getBeanNamesForType(RoleHierarchy.class).length > 0)
129-
? context.getBean(RoleHierarchy.class) : new NullRoleHierarchy();
133+
RoleHierarchy roleHierarchy = roleHierarchyProvider.getIfAvailable(NullRoleHierarchy::new);
130134
handler.setRoleHierarchy(roleHierarchy);
131135
defaultsProvider.ifAvailable((d) -> handler.setDefaultRolePrefix(d.getRolePrefix()));
132136
handler.setApplicationContext(context);
@@ -144,9 +148,10 @@ private static final class DeferringMethodSecurityExpressionHandler implements M
144148

145149
private DeferringMethodSecurityExpressionHandler(
146150
ObjectProvider<MethodSecurityExpressionHandler> expressionHandlerProvider,
147-
ObjectProvider<GrantedAuthorityDefaults> defaultsProvider, ApplicationContext applicationContext) {
148-
this.expressionHandler = SingletonSupplier.of(() -> expressionHandlerProvider
149-
.getIfAvailable(() -> defaultExpressionHandler(defaultsProvider, applicationContext)));
151+
ObjectProvider<GrantedAuthorityDefaults> defaultsProvider,
152+
ObjectProvider<RoleHierarchy> roleHierarchyProvider, ApplicationContext applicationContext) {
153+
this.expressionHandler = SingletonSupplier.of(() -> expressionHandlerProvider.getIfAvailable(
154+
() -> defaultExpressionHandler(defaultsProvider, roleHierarchyProvider, applicationContext)));
150155
}
151156

152157
@Override

config/src/main/java/org/springframework/security/config/annotation/method/configuration/SecuredMethodSecurityConfiguration.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
import org.springframework.beans.factory.ObjectProvider;
2424
import org.springframework.beans.factory.config.BeanDefinition;
25-
import org.springframework.context.ApplicationContext;
2625
import org.springframework.context.annotation.Bean;
2726
import org.springframework.context.annotation.Configuration;
2827
import org.springframework.context.annotation.Role;
@@ -52,11 +51,10 @@ final class SecuredMethodSecurityConfiguration {
5251
@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
5352
static MethodInterceptor securedAuthorizationMethodInterceptor(
5453
ObjectProvider<SecurityContextHolderStrategy> strategyProvider,
55-
ObjectProvider<ObservationRegistry> registryProvider, ApplicationContext context) {
54+
ObjectProvider<ObservationRegistry> registryProvider, ObjectProvider<RoleHierarchy> roleHierarchyProvider) {
5655
SecuredAuthorizationManager secured = new SecuredAuthorizationManager();
5756
AuthoritiesAuthorizationManager authoritiesAuthorizationManager = new AuthoritiesAuthorizationManager();
58-
RoleHierarchy roleHierarchy = (context.getBeanNamesForType(RoleHierarchy.class).length > 0)
59-
? context.getBean(RoleHierarchy.class) : new NullRoleHierarchy();
57+
RoleHierarchy roleHierarchy = roleHierarchyProvider.getIfAvailable(NullRoleHierarchy::new);
6058
authoritiesAuthorizationManager.setRoleHierarchy(roleHierarchy);
6159
secured.setAuthoritiesAuthorizationManager(authoritiesAuthorizationManager);
6260
SecurityContextHolderStrategy strategy = strategyProvider

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -50,9 +50,12 @@ public interface MethodSecurityService {
5050
@PermitAll
5151
String jsr250PermitAll();
5252

53-
@RolesAllowed({ "ADMIN", "USER" })
53+
@RolesAllowed("ADMIN")
5454
String jsr250RolesAllowed();
5555

56+
@RolesAllowed("USER")
57+
String jsr250RolesAllowedUser();
58+
5659
@Secured({ "ROLE_USER", "RUN_AS_SUPER" })
5760
Authentication runAs();
5861

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

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -56,6 +56,11 @@ public String jsr250RolesAllowed() {
5656
return null;
5757
}
5858

59+
@Override
60+
public String jsr250RolesAllowedUser() {
61+
return null;
62+
}
63+
5964
@Override
6065
public Authentication runAs() {
6166
return SecurityContextHolder.getContext().getAuthentication();

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

+6-6
Original file line numberDiff line numberDiff line change
@@ -453,9 +453,9 @@ public void configureWhenBeanOverridingDisallowedThenWorks() {
453453
@Test
454454
public void methodSecurityAdminWhenRoleHierarchyBeanAvailableThenUses() {
455455
this.spring.register(RoleHierarchyConfig.class, MethodSecurityServiceConfig.class).autowire();
456-
this.methodSecurityService.preAuthorizeAdmin();
457-
this.methodSecurityService.secured();
458-
this.methodSecurityService.jsr250RolesAllowed();
456+
this.methodSecurityService.preAuthorizeUser();
457+
this.methodSecurityService.securedUser();
458+
this.methodSecurityService.jsr250RolesAllowedUser();
459459
}
460460

461461
@WithMockUser
@@ -464,7 +464,7 @@ public void methodSecurityUserWhenRoleHierarchyBeanAvailableThenUses() {
464464
this.spring.register(RoleHierarchyConfig.class, MethodSecurityServiceConfig.class).autowire();
465465
this.methodSecurityService.preAuthorizeUser();
466466
this.methodSecurityService.securedUser();
467-
this.methodSecurityService.jsr250RolesAllowed();
467+
this.methodSecurityService.jsr250RolesAllowedUser();
468468
}
469469

470470
private static Consumer<ConfigurableWebApplicationContext> disallowBeanOverriding() {
@@ -652,9 +652,9 @@ Authz authz() {
652652
static class RoleHierarchyConfig {
653653

654654
@Bean
655-
RoleHierarchy roleHierarchy() {
655+
static RoleHierarchy roleHierarchy() {
656656
RoleHierarchyImpl roleHierarchyImpl = new RoleHierarchyImpl();
657-
roleHierarchyImpl.setHierarchy("ADMIN > USER");
657+
roleHierarchyImpl.setHierarchy("ROLE_ADMIN > ROLE_USER");
658658
return roleHierarchyImpl;
659659
}
660660

0 commit comments

Comments
 (0)