Kotlin reactive apps using coroutines broken after upgrade to spring security 6.0.2

[petergphillips]

Under 6.0.2 when trying to call any of our endpoints we now get errors such as:

The returnType class java.lang.Object on public java.lang.Object package.resource.ActivityMappingResource.deleteMapping(long,kotlin.coroutines.Continuation) must return an instance of org.reactivestreams.Publisher (for example, a Mono or Flux) in order to support Reactor Context

Investigating it looks that AuthorizationManagerBeforeReactiveMethodInterceptor is now being used under 6.0.2 as a result of the changes for #12506 setting the default for useAuthorizationManager in EnableReactiveMethodSecurity to be true now instead of false.

In the above example our method is

  @PreAuthorize("hasRole('ROLE_NOMIS_ACTIVITIES')")
  @DeleteMapping("/mapping/activities/activity-schedule-id/{id}")
  suspend fun deleteMapping(@PathVariable id: Long) = mappingService.deleteMapping(id)

which therefore appears that the issue is that the authorisation manager doesn't support kotlin coroutines? Support for coroutines was added in #8143, which we've been using successfully up to this point.

Setting

@EnableReactiveMethodSecurity(useAuthorizationManager = false)

reverts to the old behaviour of using the PrePostAdviceReactiveMethodInterceptor instead, which does provide kotlin coroutine support but is deprecated.

To Reproduce
With @EnableReactiveMethodSecurity enabled call an kotlin coroutine endpoint with a @PreAuthorize annotation. See above for example.

Expected behavior
Endpoint can still be called successfully after upgrade to 6.0.2. I'm not sure what the bug is here - but would have thought that if the default is to use the authorization manager then it should support kotlin coroutines.

Sample
Happy to provide a smaller sample if required, but the project with the failed upgrade branch is pgp-SDIT-513-upgrade-latest. Example test that is now failing is ActivityMappingResourceIntTest

[jzheaux]

Thanks for the report @petergphillips. I'm marking it as a duplicate of #12080, but feel free to correct me if it appears to be a different issue.

[petergphillips]

Thanks for investigating @jzheaux. I agree in that the underlying issue is that the new authorization manager doesn't support kotlin coroutines so from that point of view it is a duplicate.

However I'm surprised to see an application breaking feature making its way into a point release, especially as the comment in #12506 (comment) states:

Thanks, @anschnapp. I believe useAuthorizationManager should be false. We'll take care of this in the next point release.

Since the new authorization manager doesn't support kotlin coroutines, does it make sense for it to default to true in the first place? If someone was trying to add in method based security to a kotlin coroutines app, would they know that they need to turn off the authorization manager in order to get anything to work?

Furthermore I would have thought that the migration documentation should really be the other way round i.e. it should read:

So, to complete migration and preserve existing behaviour, @EnableReactiveMethodSecurity add in the useAuthorizationManager attribute:

@EnableReactiveMethodSecurity

changes to:

@EnableReactiveMethodSecurity(useAuthorizationManager = false)

I do remember reading that item of the migration guide at the time and thinking it was pointless since the behaviour isn't altered if useAuthorizationManager was specified as true in the first place!