Make it easier to create a WebExpressionAuthorizationManager with a custom expressionHandler

[ghusta]

It's not easy to customize WebExpressionAuthorizationManager with a custom expressionHandler, when defining the web security.

For example if we want to secure multiple uri in Spring Security 5.8 we have to do something like this :

@Bean
SecurityFilterChain web(HttpSecurity http) throws Exception {
        SecurityExpressionHandler<RequestAuthorizationContext> securityExpressionHandler = <custom>;
        WebExpressionAuthorizationManager expr1 = new WebExpressionAuthorizationManager("hasRole('X') and hasRole('Y')");
        expr1.setExpressionHandler(securityExpressionHandler);
        WebExpressionAuthorizationManager expr2 = new WebExpressionAuthorizationManager("hasRole('Y') and hasRole('Z')"));
        expr2.setExpressionHandler(securityExpressionHandler);

	http
		// ...
		.authorizeHttpRequests(authorize -> authorize                                  
			.requestMatchers("/x/**").access(expr1)
			.requestMatchers("/y/**").access(expr2)
			.anyRequest().denyAll()                                                
		);

	return http.build();
}

It would be easier to have either a new WebExpressionAuthorizationManager constructor with this signature :

public WebExpressionAuthorizationManager(String expressionString, SecurityExpressionHandler<RequestAuthorizationContext> expressionHandler)

Or a factory method like :

public static WebExpressionAuthorizationManager createWithExpressionHandler(String expression,
                                                                            SecurityExpressionHandler<RequestAuthorizationContext> securityExpressionHandler) {
    WebExpressionAuthorizationManager weam = new WebExpressionAuthorizationManager(expression);
    weam.setExpressionHandler(securityExpressionHandler);
    return weam;
}

References

• https://docs.spring.io/spring-security/reference/5.8/servlet/authorization/authorize-http-requests.html#servlet-authorize-requests-defaults
• Add AuthorizationManager that uses ExpressionHandler #11105

[jzheaux]

Hi, @ghusta, for the suggestion.

Spring Security prefers to have optional fields as setters and required fields in the constructor. Further, this class is intended to give folks who wish to continue using SpEL authorization expressions a way to do so, even as the codebase migrates away from it. Given these two considerations, I'm not inclined to make configuring a WebExpressionAuthorizationManager easier in this way.

The recommended path at this point is that filter-based authorization rules get evaluated programmatically. I understand you may have legacy reasons to stick with SpEL authorization rules; however, I'd be happy to look into this with you and help you migrate away from that. Can you explain what you need a custom expression handler for?

[ghusta]

Hi @jzheaux

Thank you for giving some details about the orientation of Spring Security. I understand your point of view.

I'll take some time to give you more context about my current use case, after the Christmas holidays.

Best regards

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[spring-projects-issues]

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

[JohnZ1385]

Hi @jzheaux. I'd be interested if you have any additional documentation or samples available for moving away from large scale SpEL expressions to a filter-based authorization rule. I think my app is in the same boat as @ghusta .. i have a large number of endpoints all secured via SpEL expressions and approaching this with the WebExpressionAuthorizationManager seems like I'd have to clutter the code up even more with a unique WebExpressionAuthorizationManager for each request.

for example I have a setup such as this:
`

     .antMatchers(HttpMethod.GET, "/login").permitAll()
     .antMatchers(HttpMethod.GET, "/login/**").permitAll()
     .antMatchers(HttpMethod.GET, "/logout").permitAll()
     .antMatchers(HttpMethod.GET, "/logout/**").permitAll()
     .antMatchers(HttpMethod.GET, "/accessDenied").permitAll()
     .antMatchers(HttpMethod.GET, "/accessDenied/**").permitAll()
     .antMatchers("/changepassword").access("isChangePasswordEnabled() or userMustChangePassword()")
     .antMatchers("/changepassword/**").access("isChangePasswordEnabled() or userMustChangePassword()")
     .antMatchers("/forgotpassword").access("isPasswordRecoveryEnabled()")
     .antMatchers("/forgotpassword/**").access("isPasswordRecoveryEnabled()")

     .antMatchers("/batch/batchJobMonitor/**").access("fullyAuthenticated and canView('" + Item.RUN_BATCH_JOB_MONITOR_APP + "')")
     .antMatchers("/batch/configImportExport/**").access("fullyAuthenticated and canView('" + Item.RUN_CONFIG_EXPORT_APP + "')")

     .antMatchers("/tools/createClient/**").access("fullyAuthenticated and canView('" + Item.RUN_CREATE_CLIENT + "')")
     .antMatchers("/tools/dataDictionary/**").access("fullyAuthenticated and canView('" + Item.RUN_DATA_DICTIONARY + "')")
     .antMatchers("/tools/systemLog/**").access("fullyAuthenticated and canView('" + Item.RUN_SYSTEM_LOG_APP + "')")
     .antMatchers("/tools/versionDetail/**").access("fullyAuthenticated and canView('" + Item.VERSION_BUTTON + "')")

Based on the current WebExpressionAuthorizationManager constructor it seems I'd have to construct a new WebExpressionAuthorizationManager for each URI I'm trying to secure, where as in the past I could just do this:

  @Bean
  public AccessDecisionManager accessDecisionManager() {
    List<AccessDecisionVoter<? extends Object>> decisionVoters = Arrays.asList(webExpressionVoter());
    return new UnanimousBased(decisionVoters);
  }

  @Bean
  WebSecurityExpressionHandler webSecurityExpressionHandler() {
    return new WebSecurityExpressionHandler(webAuthorizationService());
  }

  @Bean
  AccessDecisionVoter<FilterInvocation> webExpressionVoter() {
    WebExpressionVoter webExpressionVoter = new WebExpressionVoter();
    webExpressionVoter.setExpressionHandler(webSecurityExpressionHandler());
    return webExpressionVoter;
  }
  
  public interface WebAuthorizationService {
    boolean canViewItem(HttpServletRequest request, String item);
    boolean canEditItem(HttpServletRequest request, String item);
    boolean isChangePasswordEnabled();
    boolean isPasswordRecoveryEnabled();
    boolean userMustChangePassword(HttpServletRequest request);
}

[jzheaux]

Thanks for reaching out @JohnZ1385. My recommendation is that you move to programmatic authorization.

In your case, you'd do:

@Component("authz")
public class RequestAuthorizationManagerFactory {
    private final WebAuthorizationService service;

    public AuthorizationManager<RequestAuthorizationContext> isChangePasswordEnabled() {
        return (authentication, context) -> new AuthorizationDecision(this.service.isChangePasswordEnabled());
    }

    // ...

    public AuthorizationManager<RequestAuthorizationContext> canViewItem(String item) {
        return (authentication, context) -> new AuthorizationDecision(this.service.canViewItem(context.getRequest(), item));
    }
}

And then declare your rules like so:

import static org.springframework.security.authorization.AuthorizationManagers.anyOf;

.requestMatchers("/changepassword").access(anyOf(authz.isChangePasswordEnabled(), authz.userMustChangePassword()))

Alternatively, you could achieve something closer to what you have now using the same pattern:

@Component("authz")
public class RequestAuthorizationManagerFactory {
    private final WebSecurityExpressionHandler expressionHandler;

    public AuthorizationManager<RequestAuthorizationContext) spel(String expression) {
        WebExpressionAuthorizationManager manager = new WebExpressionAuthorizationManager(expression);
        manager.setExpressionHandler(expressionHandler);
        return manager;
    }
}

And then change to:

.requestMatchers("/changepassword").access(authz.spel("isChangePasswordEnabled() or userMustChangePassword()"))

[JohnZ1385]

that's not so bad, thanks for your help @jzheaux