Skip to content
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

@PreAuthorize in combination with kotlin coroutines and @Transactional does not proceed to invoke TransactionInterceptor #10252

Closed
Tracked by #11335
rolvraen opened this issue Sep 13, 2021 · 6 comments
Assignees
Labels
in: core An issue in spring-security-core type: bug A general bug
Milestone

Comments

@rolvraen
Copy link

Describe the bug

Unsure whether or not this is a bug or expected behaviour. When annotating a suspending kotlin function with both
@PreAuthorize and @Transactional
and not returing a Mono/Flux/Flow the PrePostAdviceReactiveMethodInterceptor does not proceed to invoke the transaction interceptor. It is possible to work around this issue by for example moving the @Transactional annotation to the RestController

To Reproduce

@Service
class Dummy(private val repository: FooRepository) {
    @PreAuthorize("hasAnyAuthority('FOO', 'BAR')")
    @Transactional
    suspend fun foo(name: String): FooDto {
        val rowsUpdated = repository.insertSomethingSuspended()
        return FooDto(name)
    }
    data class FooDto(val name: String)
}

Expected behavior
I would expect the PrePostAdviceReactiveMethodInterceptor to proceed to the next interceptor in the chain

@rolvraen rolvraen added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Sep 13, 2021
@eleftherias eleftherias self-assigned this Sep 13, 2021
@rolvraen rolvraen changed the title 2.5.5 @PreAuthorize in combination with kotlin coroutines and @Transactional does not proceed to invoke TransactionInterceptor 5.5.2 @PreAuthorize in combination with kotlin coroutines and @Transactional does not proceed to invoke TransactionInterceptor Sep 13, 2021
@eleftherias
Copy link
Contributor

Thanks for reaching out @rolvraen. Do you have a minimal sample that reproduces this issue?

@eleftherias eleftherias added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 1, 2021
rolvraen pushed a commit to rolvraen/spring-security-10252 that referenced this issue Oct 1, 2021
@rolvraen
Copy link
Author

rolvraen commented Oct 1, 2021

@eleftherias Thanks for getting back to me. I've created a small sample that should illustrate the issue:
https://github.com/rolvraen/spring-security-10252

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 1, 2021
@josteitv
Copy link

josteitv commented Oct 1, 2021

@eleftherias I have rewritten the sample project from @rolvraen to three unit tests and added them to KotlinEnableReactiveMethodSecurityTests.
Commit: josteitv@682febb#diff-423840da1fb0e93966c9f924ac3774caa7f4c54fd7697a391a842208dd59ede8

The problem seems to be that suspend functions are called with CoroutinesUtils.invokeSuspendingFunction in PrePostAdviceReactiveMethodInterceptor, and this will break the chain of interceptors. When using Flow without suspend, everything works correct, because flowProceed handles the invocation chain correct.

@eleftherias eleftherias added in: core An issue in spring-security-core and removed status: feedback-provided Feedback has been provided labels Oct 5, 2021
@eleftherias
Copy link
Contributor

Thanks @rolvraen and @josteitv, I can see the issue.
Are either of you interested in submitting a pull request to fix the issue?

@rwinch rwinch changed the title 5.5.2 @PreAuthorize in combination with kotlin coroutines and @Transactional does not proceed to invoke TransactionInterceptor @PreAuthorize in combination with kotlin coroutines and @Transactional does not proceed to invoke TransactionInterceptor Jun 3, 2022
@koenpunt
Copy link

Are either of you interested in submitting a pull request to fix the issue?

Since there apparently isn't in interest in fixing it, is this going to be picked up by the Spring team?

josteitv added a commit to josteitv/spring-security that referenced this issue Mar 29, 2023
@jzheaux jzheaux self-assigned this Jun 4, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Jun 4, 2024

This was addressed in the Spring Framework in #22462. Please see this comment for context specific to Spring Security. The corresponding code in Spring Security was updated in #12080 in the 6.2.0 release.

Since this is a specific issue addressed by #12080, I'll mark this as closed as of 6.2.0

@jzheaux jzheaux closed this as completed Jun 4, 2024
@jzheaux jzheaux added this to the 6.2.0 milestone Jun 4, 2024
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: bug A general bug
Projects
Archived in project
Development

No branches or pull requests

6 participants