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

VaadinWebSecurity is broken with Hilla 2 #681

Closed
simasch opened this issue Dec 1, 2022 · 9 comments · Fixed by vaadin/flow#15804
Closed

VaadinWebSecurity is broken with Hilla 2 #681

simasch opened this issue Dec 1, 2022 · 9 comments · Fixed by vaadin/flow#15804

Comments

@simasch
Copy link
Contributor

simasch commented Dec 1, 2022

I have a simple Hilla application created on https://start.vaadin.com with VaadinWebSecurity configured.

Login redirect to the login view and navigating to / results in a 401:

Connect.ts:422          POST http://localhost:8080/connect/UserEndpoint/getAuthenticatedUser 401
@simasch simasch added bug Something isn't working hilla Issues related to Hilla labels Dec 1, 2022
@simasch
Copy link
Contributor Author

simasch commented Dec 1, 2022

The issue seems to be vaadin/flow#14923

@Artur-
Copy link
Member

Artur- commented Dec 1, 2022

Looks like some other problem as CsrfTokenRequestAttributeHandler is used and not XorCsrfTokenRequestAttributeHandler

@simasch
Copy link
Contributor Author

simasch commented Dec 1, 2022

Alright. Thank you

@simasch
Copy link
Contributor Author

simasch commented Dec 21, 2022

Any news on that? I'm working on a Hilla talk and it would be great to show Hilla 2 with Spring Boot 3

@platosha platosha added this to the 2.0 milestone Jan 30, 2023
@cromoteca cromoteca self-assigned this Jan 31, 2023
@cromoteca
Copy link
Contributor

I think that the problem comes from vaadin/flow#14853: if I do the same change on 23.3 and try to use it with Hilla 1.3, integration tests pass, but applications can no longer log in. The code removed in that PR cannot be restored because it no longer works (requestResponseHolder.getResponse() is null), but must be replaced with a different implementation. The fact of calling saveContext while loading the context is no longer possible, but Hilla 1.3 was counting on that to pass the context between calls.

@Artur-
Copy link
Member

Artur- commented Feb 1, 2023

Where in the code can you see the problem when debugging? Or in other words, what is not available at which point?

@cromoteca
Copy link
Contributor

I was wrong. The issue is not there. It's this response header: XSRF-TOKEN=; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:10 GMT; Path=/ which resets that cookie and causes a mismatch when verifying the POST request for the authenticated user, which then returns a 401 and the client application resorts to an infinite login. Now I must search where that empty cookie is set.

@cromoteca
Copy link
Contributor

That header is set in the authentication request, and only in that case. Otherwise, the XSRF-TOKEN is expected to have a value. Hilla 1.3 correctly resets that cookie only in the authentication response.

But with Hilla 2, other requests are seen as authentication ones, even some completely unrelated like http://localhost:8080/VAADIN/@vite/client or http://localhost:8080/VAADIN/generated/vite-devmode.ts, but not others like http://localhost:8080/VAADIN/generated/vaadin-featureflags.ts. I have the feeling that those empty cookies are sent for some time after the first auth response.

There are also some JSESSIONID cookies sent, all different.

I still haven't been able to understand what causes this behavior. Anyway, the wrong path passes by this: https://github.com/spring-projects/spring-security/blob/19c55372a3f2f97d1f0ee235f90b38187a8b3a69/web/src/main/java/org/springframework/security/web/session/SessionManagementFilter.java#L97-L102

Only the first http://localhost:8081/login request should show that behavior.

Artur- added a commit to vaadin/flow that referenced this issue Feb 2, 2023
…on (#15804)

The shared objects need to be available when other configurers are run and because of spring-projects/spring-security#12579 a workaround is needed to actually apply the correct SecurityContextRepository

Fixes vaadin/hilla#681
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.0.0.alpha10 and is also targeting the upcoming stable 24.0.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants