feat: Always use the security context from VaadinSession when one is available #907
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ble. This ensures that the security context is the expected (the one from the UI you run access() on) if you run UI.access from a request to another VaadinSession. Practical use case is e.g. sending a global 'refresh' event and the receipient updating the UI as a result. Fixes #906
Artur-
force-pushed
the
use-security-context-from-access
branch
from
c966948 to
b0cb13a
Compare
mshabarov
reviewed
Sep 28, 2021
...pring-security-flow/src/main/java/com/vaadin/flow/spring/flowsecurity/views/Broadcaster.java
Show resolved
Hide resolved
mshabarov
reviewed
Sep 28, 2021
.../test-spring-security-flow/src/test/java/com/vaadin/flow/spring/flowsecurity/AbstractIT.java
Show resolved
Hide resolved
mshabarov
reviewed
Sep 28, 2021
...pring-security-flow/src/test/java/com/vaadin/flow/spring/flowsecurity/UIAccessContextIT.java
Show resolved
Hide resolved
mshabarov
reviewed
Sep 28, 2021
.../src/main/java/com/vaadin/flow/spring/security/VaadinAwareSecurityContextHolderStrategy.java
Show resolved
Hide resolved
mshabarov
reviewed
Sep 28, 2021
|
|
||
| import static java.util.Objects.requireNonNull; | ||
|
|
||
| public final class VaadinAwareSecurityContextHolderStrategy |
Contributor
There was a problem hiding this comment.
Javadoc is needed. Although this is internally used class, a couple of sentences describing that this implementation looks for the VaadinSession for security context and it provides the security context for UI.access() calls.
mshabarov
reviewed
Sep 28, 2021
...pring-security-flow/src/main/java/com/vaadin/flow/spring/flowsecurity/views/Broadcaster.java
Outdated
Show resolved
Hide resolved
mshabarov
reviewed
Sep 28, 2021
.../src/main/java/com/vaadin/flow/spring/security/VaadinAwareSecurityContextHolderStrategy.java
Outdated
Show resolved
Hide resolved
mshabarov
reviewed
Sep 28, 2021
| balanceSpan.setId("balanceText"); | ||
| add(balanceSpan); | ||
| add(new Button("Apply for a loan", this::applyForLoan)); | ||
| add(new Button("Apply for a huge loan", |
Contributor
There was a problem hiding this comment.
Apply for a huge loan button is not used in a tests. Is it for demo only?
Member
Author
There was a problem hiding this comment.
Yeah it is for manual testing only. At least for now
mshabarov
requested changes
Sep 28, 2021
Contributor
mshabarov
left a comment
mshabarov
left a comment
There was a problem hiding this comment.
I would add the javadoc and some missed headers, but otherwise it look good to me: VaadinSession is used to lookup the security context and this security context is available within UI::access calls, even if the same thread is used for broadcast UI updates, so the developer doesn't need to execute the update in a separate thread (if I understood this enhancement correctly).
mshabarov
changed the title
Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>
Artur-
force-pushed
the
use-security-context-from-access
branch
from
a0f1184 to
8bf6e8a
Compare
Collaborator
|
SonarQube analysis reported 1 issue
|
mshabarov
approved these changes
Sep 28, 2021
Collaborator
|
This ticket/PR has been released with platform 22.0.0.alpha5 and is also targeting the upcoming stable 22.0.0 version. |
manolo
pushed a commit
to vaadin/flow
that referenced
this pull request
Feb 8, 2022
…available vaadin/spring#907 This ensures that the security context is the expected (the one from the UI you run access() on) if you run UI.access from a request to another VaadinSession. Practical use case is e.g. sending a global 'refresh' event and the recipient updating the UI as a result. Fixes: vaadin/spring#906
manolo
pushed a commit
to vaadin/flow
that referenced
this pull request
Feb 8, 2022
…available vaadin/spring#907 This ensures that the security context is the expected (the one from the UI you run access() on) if you run UI.access from a request to another VaadinSession. Practical use case is e.g. sending a global 'refresh' event and the recipient updating the UI as a result. Fixes: vaadin/spring#906
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This ensures that the security context is the expected (the one from the UI you run access() on) if you run UI.access from a request to another VaadinSession.
Practical use case is e.g. sending a global 'refresh' event and the receipient updating the UI as a result.
Fixes #906