Skip to content

hasAuthority and custom Mono<Boolean> method in @PreAuthorize leads to ConverterNotFoundException error #15209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bskorka opened this issue Jun 6, 2024 · 3 comments
Labels
in: core An issue in spring-security-core type: enhancement A general enhancement

Comments

@bskorka
Copy link

bskorka commented Jun 6, 2024

Describe the bug
After upgrading to Spring Boot 3.3.0 and Spring Security 6.3.0 I've tried to migrate my single Mono<Boolean> @PreAuthorize calls to more complex ones as I thought that these tasks make it possible:

However, if we want to use the @PreAuthorize connected with a Mono<Boolean> and hasAuthority we are getting an error No converter found capable of converting from type [reactor.core.publisher.MonoFlatMap<java.lang.Boolean, ?>] to type [java.lang.Boolean]

To Reproduce

  • Create a WebFlux controller and use @EnableReactiveMethodSecurity configuration
  • Define Controller with a @PreAuthorize usage
  • Add verification returning Mono<Boolean> and call the endpoint - it works
  • Connect the custom method authorization with built-in @PreAuthorize - the exception will be thrown on call

Expected behaviour
The "complex" @PreAuthorize expression should be handled without isues if we use methods returning Mono<Boolean> and built-in hasAuthority calls.

Sample

@Configuration
@EnableWebFluxSecurity
@EnableReactiveMethodSecurity
public class SecurityConfig {
}
@Slf4j
@RestController
@RequiredArgsConstructor
@RequestMapping(value = "/test")
public class TestController {
    @GetMapping("/values")
    @PreAuthorize("hasAuthority('ppcv:browse') || "
                  + "(hasAuthority('ppcv:browse-scoped') && @userConnectionAuthorizerService.isUserConnected(#id))")
    public Mono<ApiResponse<List<TestObject>>> getAllValues(@RequestParam Integer id,
                                                                                       @RequestParam List<Status> statuses,
                                                                                       @RequestParam(required = false, defaultValue = "false") Boolean latest) {
        return ...
    }
}
public interface UserConnectionAuthorizerService {

    Mono<Boolean> isUserConnected(Long id);

}

When running code in the integration tests I receive:

2024-06-06 18:55:42,135 [parallel-4] ERROR o.s.b.a.w.r.e.AbstractErrorWebExceptionHandler - [5f1ceeb9-3]  500 Server Error for HTTP GET "/test/values?id=1&statuses=APPROVED"
java.lang.IllegalArgumentException: Failed to evaluate expression 'hasAuthority('ppcv:browse') || (hasAuthority('ppcv:browse-scoped') && @userConnectionAuthorizerService.isUserConnected(#id))'
	at org.springframework.security.authorization.method.ReactiveExpressionUtils.lambda$evaluate$0(ReactiveExpressionUtils.java:43)
	Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Error has been observed at the following site(s):
	*__checkpoint ⇢ Handler test.TestController#getAllValues(Integer, List, Boolean) [DispatcherHandler]
	*__checkpoint ⇢ test.core.infrastructure.actuator.spring.DefaultActuatorWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ test.core.application.spring.http.filter.AdminSecurityFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ AuthorizationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ ExceptionTranslationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ LogoutWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ ServerRequestCacheWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ SecurityContextServerWebExchangeWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ UserIdAuthenticationFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ ReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ HttpHeaderWriterWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ ServerWebExchangeReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.WebFilterChainProxy [DefaultWebFilterChain]
	*__checkpoint ⇢ HTTP GET "/test/values?id=1&statuses=APPROVED" [ExceptionHandlingWebHandler]
Original Stack Trace:
		at org.springframework.security.authorization.method.ReactiveExpressionUtils.lambda$evaluate$0(ReactiveExpressionUtils.java:43)
		...
Caused by: org.springframework.expression.spel.SpelEvaluationException: EL1001E: Type conversion problem, cannot convert from reactor.core.publisher.MonoJust<java.lang.Boolean> to java.lang.Boolean
	at org.springframework.expression.spel.support.StandardTypeConverter.convertValue(StandardTypeConverter.java:87)
	at org.springframework.expression.common.ExpressionUtils.convertTypedValue(ExpressionUtils.java:57)
	at org.springframework.expression.spel.ast.SpelNodeImpl.getValue(SpelNodeImpl.java:222)
	at org.springframework.expression.spel.ast.OpAnd.getBooleanValue(OpAnd.java:57)
	at org.springframework.expression.spel.ast.OpAnd.getValueInternal(OpAnd.java:52)
	at org.springframework.expression.spel.ast.SpelNodeImpl.getValue(SpelNodeImpl.java:222)
	at org.springframework.expression.spel.ast.OpOr.getBooleanValue(OpOr.java:56)
	at org.springframework.expression.spel.ast.OpOr.getValueInternal(OpOr.java:51)
	at org.springframework.expression.spel.ast.OpOr.getValueInternal(OpOr.java:37)
	at org.springframework.expression.spel.ast.SpelNodeImpl.getValue(SpelNodeImpl.java:114)
	at org.springframework.expression.spel.standard.SpelExpression.getValue(SpelExpression.java:273)
	at org.springframework.security.authorization.method.ReactiveExpressionUtils.lambda$evaluate$2(ReactiveExpressionUtils.java:39)
	...
Caused by: org.springframework.core.convert.ConverterNotFoundException: No converter found capable of converting from type [reactor.core.publisher.MonoJust<java.lang.Boolean>] to type [java.lang.Boolean]
	at org.springframework.core.convert.support.GenericConversionService.handleConverterNotFound(GenericConversionService.java:294)
	at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:185)
	at org.springframework.expression.spel.support.StandardTypeConverter.convertValue(StandardTypeConverter.java:82)
	... 260 common frames omitted

@bskorka bskorka added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jun 6, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Jun 6, 2024

@bskorka, thanks for the report. Will you please try the following?

Change @PreAuthorize to reference only one bean like so:

@PreAuthorize("@authz.checkEverything(#root)"`)

Then, introduce the above bean in such a way that it checks all three conditions. Can you confirm that this allows you to return Mono<Boolean> as expected?

In the meantime, I will take a look and see what can be done about supporting expression elements that return a mixture of Boolean and Mono<Boolean>. I'm not convinced that this should be added, but it's worth a look. And, we can at least try and improve the error message with additional context.

@jzheaux jzheaux added in: core An issue in spring-security-core type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jun 6, 2024
@jzheaux jzheaux assigned jzheaux and unassigned jzheaux Jun 6, 2024
@bskorka
Copy link
Author

bskorka commented Jun 7, 2024

Hey @jzheaux!
I can confirm that moving all the expressions to one method returning the Mono<Boolean> works fine, as I have used it since support for Mono<Boolean> was introduced in Spring Security 5.8.9(?).

The implementation of this approach looks like this:

public class TestController {
    @GetMapping("/values")
    @PreAuthorize("@userConnectionAuthorizerService.hasUserAccessToConnectionData(#id))")
    public Mono<ApiResponse<List<TestObject>>> getAllValues(@RequestParam Integer id,
                                                            @RequestParam List<Status> statuses,
                                                            @RequestParam(required = false, defaultValue = "false") Boolean latest) {
        return ...
    }
}
public class UserConnectionAuthorizerServiceImpl implements UserConnectionAuthorizerService {
    private final PreAuthService preAuthService;

    @Override
    public Mono<Boolean> hasUserAccessToConnectionData(Long id) {
        return Mono.zip(
                           preAuthService.hasAuthority(Permissions.PPCV_BROWSE.getValue()),
                           preAuthService.hasAuthority(Permissions.PPCV_BROWSE_SCOPED.getValue())
                   )
                   .flatMap(tuple -> tuple.getT1() ? Mono.just(true) :
                           tuple.getT2() ? checkUserConnection(id) : // business check, same as isUserConnected in previous example
                                   Mono.just(false)
                   )
                   .switchIfEmpty(Mono.just(false));
    }
}
public class PreAuthServiceImpl implements PreAuthService {
    public Mono<Boolean> hasAuthority(String authority) {
        return ReactiveSecurityContextHolder.getContext()
                                            .map(SecurityContext::getAuthentication)
                                            .map(authentication -> authentication.getAuthorities().stream()
                                                                                 .anyMatch(grantedAuthority -> grantedAuthority.getAuthority().equals(authority)));
    }
}

I understand that handling the mix of Boolean and Mono<Boolean> might not be easy to implement understandably and satisfyingly. Maybe a reactive variant of the @PreAuthorize annotation or reactiveHasAuthority returning the Mono<Boolean> will work?
I agree that the error message can be improved as I spent some time thinking that the issue is related to my reactive pipe.

@rrrship
Copy link

rrrship commented Dec 19, 2024

Ideally we should be able to look at a method and see what are the authorities needed for executing this method. With this current approach, all the checks are hidden. It also looks like we can't define two reactive methods in the @PreAuthorize SpEl as well, which maybe would've helped with this a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core An issue in spring-security-core type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants