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

ReactiveSecurityContextHolder.getContext().block() is null with custom AuthenticationToken (Sample provided) #5207

Closed
eximius313 opened this issue Apr 4, 2018 · 10 comments
Assignees
Labels
status: invalid An issue that we don't feel is valid

Comments

@eximius313
Copy link

eximius313 commented Apr 4, 2018

Summary

Documentation here and here shows example of how to get Authentication token using ReactiveSecurityContextHolder.getContext() but this is misleading because it always returns null!
I spent days on trying to figure out what I'm doing wrong in my AuthenticationManager, ServerSecurityContextRepository and/or WebFilters to get current logged user in my Controller:

@GetMapping
@PreAuthorize("hasRole('USER')")
Mono<String> getMyProfile() {
    return ReactiveSecurityContextHolder.getContext()
            .map(SecurityContext::getAuthentication)
            .map(Authentication::getName);
}

and finally I found this issue: #4790, changed my Controller to:

@GetMapping
@PreAuthorize("hasRole('USER')")
Mono<String> getMyProfile(@AuthenticationPrincipal Mono<UserDetails> details) {
        return details
        .cast(User.class)
        .map(User::getLogin);
}

and it magically worked (still I have no idea why....)

Actual Behavior

Documentation is not clear on how to obtain UserDetails/Principle in Controller

Expected Behavior

Documentation should explain exactly what is written in this comment: #4790 (comment)

@eximius313
Copy link
Author

It occurred, that this was not a problem with documentation, but actually a bug!
I was blocking on Context:
final SecurityContext context = ReactiveSecurityContextHolder.getContext().block(); and the context was always null.
Because I was using custom AuthenticationToken, in tests I used: SecurityMockServerConfigurers.mockUser; and it also was returning null.
Surprisingly, when I tried WithMockUser instead - it worked!

  1. Run the sample: SpringSecurityReactiveSample.zip with ./gradlew test and you'll get NPE.
  2. Go to ControllerTest, comment the line 65 and uncomment 62
  3. Run ./gradlew test again, and tests pass

@eximius313 eximius313 changed the title Documentation is misleading about getting UserDetails/Principle in Controller ReactiveSecurityContextHolder.getContext().block() is null with custom AuthenticationToken (Sample provided) Apr 6, 2018
@yyunikov
Copy link

+1 Same for me, I always get a null in ReactiveSecurityContextHolder for some reason.

@LBoraz
Copy link

LBoraz commented Feb 6, 2020

Still a bug in 5.2.2, ReactiveSecurityContextHolder returns NEVER a context, not even when mocked in unit tests

@luvarqpp
Copy link

I have an issue, which pointed me here. I am trying to use CsrfMutator and it fails on NPE. Can this issue be connected to #7778 ?

@rwinch
Copy link
Member

rwinch commented Mar 24, 2020

You cannot use .block() because it subscribes and does not have the security context as part of the subscriber context that is established by Spring Security's WebFilter.

If you believe this is a bug, please provide a complete and minimal sample (i.e. GitHub repository) of the problem with details on how to reproduce the issue, what you expect to happen, and what actually happens.

@rwinch rwinch 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 Mar 24, 2020
@spring-projects-issues
Copy link

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 spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Mar 31, 2020
@eximius313
Copy link
Author

@rwinch - I have provided the sample project in my second comment two years ago (#5207 (comment))

@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 status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Mar 31, 2020
@rwinch rwinch added status: invalid An issue that we don't feel is valid and removed status: feedback-provided Feedback has been provided labels Apr 1, 2020
@rwinch rwinch self-assigned this Apr 1, 2020
@rwinch
Copy link
Member

rwinch commented Apr 1, 2020

Thanks for the pointer. Sorry I wasn't clear. I wasn't sure if someone had an example that does not use .block(). Some comments seem to imply that ReactiveSecurityContextHolder never works for them.

ReactiveSecurityContextHolder uses Reactor's Context and does not allow .block() because the subscriber context won't be propagated. For understanding how Reactor's Context works refer to Adding a Context to a Reactive Sequence section of their reference.

More generally, you should not invoke .block() in async threads because that breaks reactive semantics (which should not be blocking). In newer versions of Reactor it will throw an Exception if you try to .block() any Publisher in an async thread.

I'm going to close this issue as invalid since the example uses .block(). If someone has an issue leveraging ReactiveSecurityContextHolder without using .block() then please create a new ticket.

@rwinch rwinch closed this as completed Apr 1, 2020
@Nida96
Copy link

Nida96 commented Aug 18, 2022

Still not fixed without using block

@sohelnaikawadi1
Copy link

I can confirm it still doesn't work without using block().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

8 participants