Expected Behavior
AuthorizationManager[Before/After]ReactiveMethodInterceptor should suport Kotlin coroutines
Current Behavior
The now deprecated PrePostAdviceReactiveMethodInterceptor did support Kotlin coroutines but the newer counterparts don't
Context
At least when using a custom annotation with the new AuthorizationManager[Before/After]ReactiveMethodInterceptor like:
@Bean
@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
fun customAuthorize(): Advisor {
val pointcut = Pointcuts.union(
AnnotationMatchingPointcut(null, Can::class.java, true),
AnnotationMatchingPointcut(Can::class.java, true)
)
val interceptor = AuthorizationManagerBeforeReactiveMethodInterceptor(
pointcut,
CanDoAuthorizationManager()
)
interceptor.order = AuthorizationInterceptorsOrder.PRE_AUTHORIZE.order + 1
return interceptor
}
I can't annotated suspended functions in Kotlin, looking at the deprecated PrePostAdviceReactiveMethodInterceptor coroutines functionality was added a while back, but looks like the same functionality wasn't ported to the new classes.
Workaround is to use mono {} to wrap the suspended functions and return Mono
Comment From: jzheaux
This is blocked until https://github.com/spring-projects/spring-framework/issues/22462 is addressed.
See https://github.com/spring-projects/spring-security/pull/9867#pullrequestreview-1020729196 for additional context.
Comment From: martinformi
May I ask about the status? We made some effort to upgrade to Spring Boot 3.1.0 where reactive/suspend functions are heavily used (with @EnableReactiveMethodSecurity and @PreAuthorize annotations). I am afraid that the bug https://github.com/spring-projects/spring-framework/issues/22462 will not be resolved soon. What could be a workaround here?
Comment From: F43nd1r
Any updates here? The underlying issue https://github.com/spring-projects/spring-framework/issues/22462 is closed since august
Comment From: sjohnr
No updates @martinformi @F43nd1r. I opened gh-13770 with a list of the classes that require enhancements to support this, but I just closed it as a duplicate of this issue. If anyone is interested in contributing the enhancements, it would be most welcome.
Comment From: F43nd1r
@sjohnr took a look at the code of AuthorizationManagerBeforeReactiveMethodInterceptor as an example.
However, to me it looks like there are competing concepts in the code:
On the one hand, there's a check which enforces the return type to extend Publisher, meaning only Mono, Flux and friends are supported.
On the other hand, there's a ReactiveAdapterRegistry after that check, which could convert other return types (e.g. Flow) to Mono or Flux.
Also, the ReactiveAdapterRegistry identifies adapters by return type, which is fundamentally incompatible with suspend functions as they only return Object and instead should be identified by the last parameter being a Continuation.
Am I misunderstanding what the ReactiveAdapterRegistry is for? Could you give some pointers how the team expects this to be implemented?
Comment From: sjohnr
@F43nd1r it's tricky indeed. I am not an expert on reactive types so I had to work it out as well.
The support in PrePostAdviceReactiveMethodInterceptor (@EnableReactiveMethodSecurity(useAuthorizationManager = false)) has already been updated with gh-13764 via 92256f0522dbdfbfd3f9f9b403278bc97240b791. I believe the updates we're discussing here would be similar. The main difference being that the newer classes don't have any support for suspend so it needs to be added, whereas the older class was only missing support for nested suspend calls.