Skip to content

Conversation

@platosha
Copy link

Fixes #924

@platosha platosha added the fusion Changes required for fusion label Oct 26, 2021
@platosha platosha marked this pull request as ready for review October 27, 2021 18:41
@ZheSun88
Copy link
Contributor

validation tests failed with Caused by: java.io.NotSerializableException: com.vaadin.flow.spring.security.VaadinWebSecurityConfigurerAdapter$Http401UnauthorizedAccessDeniedHandler

@haijian-vaadin
Copy link

seems it requires a page refresh after the fix? If so, since the InvalidSessionMiddleWare is mostly about redirecting to login view. Should we just refresh the page in the server-side AccessDeniedHandler? then InvalidSessionMiddleWare is no longer needed?

@platosha
Copy link
Author

@haijian-vaadin the main concern in #924 is that InvalidSessionMiddleware was not invoked due to server response is a redirect instead of an expected 401. So the authentication expiration was hard to somehow address on the client side. Concerns both the session-based and stateless authentication.

Because with the session-based authentication Spring uses default session-based CSRF protection, when the session expires on the server we need to update the CSRF token on the client. In that case reloading the page is required, as it is currently the only way to obtain a fresh CSRF token.

Note that an anonymous session expires on the server, there will be invalid CSRF exceptions for anonymous endpoints too, which triggers InvalidSessionMiddleware. In that case prompting the user with the login screen is not appropriate, the user was not logged in previously, and the endpoint action does not require a login.

Should we just refresh the page in the server-side AccessDeniedHandler?

Redirect was the default behaviour of Spring Security before this change. But this does not fit endpoint requests: redirecting in AccessDeniedHandler for endpoint requests does not automatically refresh the page, but rather the endpoint client receives an unexpected HTML response which it cannot parse. The server is expected to respond with a JSON or an error here.

then InvalidSessionMiddleware is no longer needed?

I think it is still needed, or we need something else to cover a use case of handling expired CSRF and authentication on the client. Both sessions and JWT expire over time, so one API can cover both, I think. With stateless authentication, a non-reloading redirect on the client side to the login view should be a working way to deal with expired authentication, whereas with session-based CSRF you still have to reload the page.

@haijian-vaadin haijian-vaadin merged commit dee2bb9 into master Oct 29, 2021
@haijian-vaadin haijian-vaadin deleted the fix/ap/stateless-csrf branch October 29, 2021 10:10
manolo pushed a commit to vaadin/flow that referenced this pull request Feb 8, 2022
vaadin/spring#925

* fix: configure 401 unauthorized response for endpoints

Fixes: vaadin/spring#924

* fix formatting

* Add CSRF access denied handler for endpoints and tests

* Don’t ignore obscure browser error

* Fix SpringClassesSerializableTest

* Fix stale PublicView element reference in test

* Fix formatting, hope one day there will be a pre-commit hook

* Handle StaleElementReferenceException

* Fix open in SecurityIT

* Remove waitForClientRouter

Co-authored-by: ZheSun88 <zhe@vaadin.com>
manolo pushed a commit to vaadin/flow that referenced this pull request Feb 8, 2022
vaadin/spring#925

* fix: configure 401 unauthorized response for endpoints

Fixes: vaadin/spring#924

* fix formatting

* Add CSRF access denied handler for endpoints and tests

* Don’t ignore obscure browser error

* Fix SpringClassesSerializableTest

* Fix stale PublicView element reference in test

* Fix formatting, hope one day there will be a pre-commit hook

* Handle StaleElementReferenceException

* Fix open in SecurityIT

* Remove waitForClientRouter

Co-authored-by: ZheSun88 <zhe@vaadin.com>
platosha added a commit to vaadin/hilla that referenced this pull request May 24, 2022
vaadin/spring#925

* fix: configure 401 unauthorized response for endpoints

Fixes: vaadin/spring#924

* fix formatting

* Add CSRF access denied handler for endpoints and tests

* Don’t ignore obscure browser error

* Fix SpringClassesSerializableTest

* Fix stale PublicView element reference in test

* Fix formatting, hope one day there will be a pre-commit hook

* Handle StaleElementReferenceException

* Fix open in SecurityIT

* Remove waitForClientRouter

Co-authored-by: ZheSun88 <zhe@vaadin.com>
platosha added a commit to vaadin/hilla that referenced this pull request May 31, 2022
vaadin/spring#925

* fix: configure 401 unauthorized response for endpoints

Fixes: vaadin/spring#924

* fix formatting

* Add CSRF access denied handler for endpoints and tests

* Don’t ignore obscure browser error

* Fix SpringClassesSerializableTest

* Fix stale PublicView element reference in test

* Fix formatting, hope one day there will be a pre-commit hook

* Handle StaleElementReferenceException

* Fix open in SecurityIT

* Remove waitForClientRouter

Co-authored-by: ZheSun88 <zhe@vaadin.com>
vercel-talented added a commit to vercel-talented/hilla-react that referenced this pull request May 4, 2024
vaadin/spring#925

* fix: configure 401 unauthorized response for endpoints

Fixes: vaadin/spring#924

* fix formatting

* Add CSRF access denied handler for endpoints and tests

* Don’t ignore obscure browser error

* Fix SpringClassesSerializableTest

* Fix stale PublicView element reference in test

* Fix formatting, hope one day there will be a pre-commit hook

* Handle StaleElementReferenceException

* Fix open in SecurityIT

* Remove waitForClientRouter

Co-authored-by: ZheSun88 <zhe@vaadin.com>
byte-dev-hubs added a commit to byte-dev-hubs/hila-java that referenced this pull request May 12, 2024
vaadin/spring#925

* fix: configure 401 unauthorized response for endpoints

Fixes: vaadin/spring#924

* fix formatting

* Add CSRF access denied handler for endpoints and tests

* Don’t ignore obscure browser error

* Fix SpringClassesSerializableTest

* Fix stale PublicView element reference in test

* Fix formatting, hope one day there will be a pre-commit hook

* Handle StaleElementReferenceException

* Fix open in SecurityIT

* Remove waitForClientRouter

Co-authored-by: ZheSun88 <zhe@vaadin.com>
AceDev24 pushed a commit to AceDev24/hiliaGround that referenced this pull request Sep 3, 2024
vaadin/spring#925

* fix: configure 401 unauthorized response for endpoints

Fixes: vaadin/spring#924

* fix formatting

* Add CSRF access denied handler for endpoints and tests

* Don’t ignore obscure browser error

* Fix SpringClassesSerializableTest

* Fix stale PublicView element reference in test

* Fix formatting, hope one day there will be a pre-commit hook

* Handle StaleElementReferenceException

* Fix open in SecurityIT

* Remove waitForClientRouter

Co-authored-by: ZheSun88 <zhe@vaadin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fusion Changes required for fusion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

In v22, unauthorized request for Fusion endpoints redirects to /login instead of having 401 response

3 participants