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

Login page forwarding with security filter #5780

Closed
vaadin-bot opened this issue Oct 14, 2014 · 15 comments
Closed

Login page forwarding with security filter #5780

vaadin-bot opened this issue Oct 14, 2014 · 15 comments
Labels
Stale Stale bot label

Comments

@vaadin-bot
Copy link
Collaborator

Originally by @samulivaadin


Vaadin push throws an exception on client side when in our case the spring security redirects to login page.

Exception message: Uncaught message length "<>" is not a number vaadinPush.debug.js:11712

When this happens the user observed effect is that red spinner starts to spin indefinitely. This is similar to #8241 but happens with 7.3 and the environment here is not related to portals but instead using a security filter.


Imported from https://dev.vaadin.com/ issue #14866

@vaadin-bot
Copy link
Collaborator Author

Originally by proaccountapp


Updated prioritization date.

@vaadin-bot
Copy link
Collaborator Author

Originally by Markus Koivisto


What steps are required to reproduce this? Could you elaborate?

Does this happen only with session expiration? Does it happen if you log out? What container are you using? Is there a proxy involved?

@vaadin-bot
Copy link
Collaborator Author

Originally by @samulivaadin


This happens when session expires or server gets restarted. Server restart is not that common use case but with that it's really easy to reproduce this. What you need to setup in order to reproduce the issue is spring security with configuration like this:

HttpSecurity http = ...
http.formLogin().loginPage("/login").permitAll().and().logout().logoutSuccessUrl("/");

So you would have security filter in place that redirects you into login page if you don't have validly logged in. Long polling push is used here and for locally we have got with Jetty container and no proxies involved.

Then steps to reproduce.

  1. Open browser and see from network console that polling is done at background
  2. Restart your server
  3. Observe that earlier mentioned exception appears in log and red spinner starts to roll.

Can provide a live demo if required.

@vaadin-bot
Copy link
Collaborator Author

Originally by @jdahlstrom


As discussed on Skype, this is due to the _trackMessageSize function in vaadinPush.js throwing if it fails to parse the message size at the start of the message:

    if (isNaN(messageLength))
        throw 'message length "' + str + '" is not a number';

This should be changed to invoke the onError callback instead so we at least get a critical notification. For #8241 a special magic syntax was invented that, if included in a non-json response, would cause a refresh or redirect. Would be nice if this was supported with push as well, but implementing it may be a bit tricky.

@vaadin-bot
Copy link
Collaborator Author

Originally by Markus Koivisto


Apparently a pipe symbol in the content of the login page causes a failure (which is a separate issue that should have a ticket of it's own). A potential solution for this might be modeled after the solution to #12404.

@vaadin-bot
Copy link
Collaborator Author

Originally by @alvarezguille


We are currently trying to detect and fix the cases when responses are invalid. In this case it happens because push messages are redirected to login page.

@vaadin-bot
Copy link
Collaborator Author

Originally by @thinwire


Let me start by saying.. "Oh boy." This ticket turned out to be a real can of worms, and despite several hours of work by me and Elijah we're still figuring out what to make of it.

Problem 1: users aren't getting redirected to /login. This has a multitude of reasons and some half-solutions. If push is used, Atmosphere will misbehave in interesting ways after ingesting the login page contents. If the server replies with a 302 redirect to long polling requests, then Atmosphere goes into a CPU and network-heavy loop. With WebSockets, the session just hangs and new WS connections are attempted indefinitely. No red screen.

If push is NOT used, there's a possible half-solution in adding a Request Handler to VaadinSession and writing a magic redirect message to the response if the session is no longer valid: response.getWriter().write("<!-- Vaadin-Refresh: /login -->"); should do the trick - eventually.

Problem 2: Atmosphere goes into an uncontrolled reload loop when requests are being redirected by Security.
Detecting and halting the behavior after a sufficient amount of failures is feasible, but needs further design.

Problem 3: Atmosphere errors out when there's a pipe character in the returned login page.
This can be fixed, also. However, since this is the result of a much deeper configuration and architecture problem, do we really want to? At least we'd get that nice red box of death with a somewhat more sensible message.

Problem 4: Spring Security can desynchronize Vaadin, leaving the system in a completely broken state.
We're trying to create an example security filter and suitable attachments to Vaadin that would allow changes in Spring Security state to be notified to Vaadin, so that sessions can be cleanly terminated and users redirected to login without having communications break or red boxes appear.

In any case, it appears that getting Vaadin to play nice with Spring Security (and vice versa) isn't exactly trivial. We're hoping to improve this in the future.

@vaadin-bot
Copy link
Collaborator Author

Originally by @thinwire


Attachment added: vaadin-spring-sandbox.zip (42.8 KiB)
Simple test project (at some state) we're playing around with to test security configs. As-is, it demonstrates the 302 redirect issue.

@vaadin-bot
Copy link
Collaborator Author

Originally by @elmot


Attachment added: spring-security-vaadin-sandbox-workaround.zip (7.3 KiB)
Workaround for spring security. Does not 302, redirercts a browser to a login page after logout.

@vaadin-bot
Copy link
Collaborator Author

Originally by @thinwire


Changing this to "Needs Design". We're no longer actively working on this; instead, we'll try to focus on the issues this brought forth, and we need to investigate the viability of a Vaadin/Spring Security compatibility project (looks like this means code changes to Vaadin, Atmosphere and possibly even Spring).

As it is, Spring simply does not play nice with Vaadin (in fact, it does not play nice with anything using Atmosphere). If Spring Security is used, Vaadin application developers should be extremely careful in their actions regarding triggering Spring Security overrides, since doing so will cause the Vaadin application to go into inconsistent state as it's currently not possible to gracefully terminate all connections and notify the Vaadin session of its early demise. Therefore, it's up to the application developer to make sure that the Vaadin application stays aware of its surroundings.

Or, in other words, "Here be Dragons".

@vaadin-bot
Copy link
Collaborator Author

Originally by @samie


There is very little we can do more inside framework to fix this, but to document this one. Suggestions:

  • Check with Vaadin 7.6. It changes the communication logic and this might affect redirect behaviour.
  • Document the Spring configuration setup at vaadin.com/docs

@vaadin-bot
Copy link
Collaborator Author

Originally by @elmot


In 7.6 there is no more infinite request loop, but the application just shows "Communication problem" warning.

@vaadin-bot
Copy link
Collaborator Author

Originally by proaccountapp


Removed prioritization date.

@stale
Copy link

stale bot commented Mar 14, 2018

A lot of tickets have been left hanging in the issue tracker through the years. Some of them are still relevant, some of them have been fixed a long time ago and some are no longer valid. To get a better look on what is important and still relevant, we are closing old tickets which have not been touched in a long time. No further work will be done on this ticket. If the ticket seems to be still actual, please verify the problem existence over latest framework version and then open a new ticket in vaadin/framework with all the suitable information.

@stale stale bot added the Stale Stale bot label label Mar 14, 2018
@stale
Copy link

stale bot commented Aug 31, 2020

The issue was automatically closed due to inactivity. If you found some new details to it or started working on it, comment on the issue so that maintainers can re-open it.

@stale stale bot closed this as completed Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Stale bot label
Projects
None yet
Development

No branches or pull requests

1 participant