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 flash messages only work because you're talking to localhost #234

Open
rlpowell opened this issue Nov 22, 2023 · 3 comments
Open

login flash messages only work because you're talking to localhost #234

rlpowell opened this issue Nov 22, 2023 · 3 comments

Comments

@rlpowell
Copy link

rlpowell commented Nov 22, 2023

I was very confused when after switching to actix-web-flash-messages, both the login::an_error_flash_message_is_set_on_failure test and the functionality in question stopped working.

I eventually discovered that this is a side effect of my setup; I have host: in the local config set to 0.0.0.0 because I'm running my tests on my linux box but my browser is on my windows box (and publishing ports from a container doesn't work when the port is opened on loopback, at least in my podman config).

The specific issue is that actix-web-flash-messages sets Secure on the cookies, which the previous manual cookie solution did not. This appears to be important to you (see LukeMathWalker/actix-web-flash-messages#6 ), but you should be aware it makes things weird in testing situations.

FWIW, the reason the test works with 127.0.0.1 is https://github.com/pfernie/cookie_store/blob/403db6b2bd5acb4844aa9ac5b10f909d273678ee/src/cookie_store.rs#L1552 , so, pretty hacky. :D

To get the browser to work I had to use SSH port forwarding so that I could go to http://localhost/ in the browser, even though it's not actually on the local host.

Fun side note: the version of curl in Debian bullseye also fails to be usable with these flash cookies, even on localhost, because curl/curl@c495dcd

Anyway, having lost ~3 hours of my life to this, I thought you should be aware that it's a bit fragile.

@rlpowell
Copy link
Author

I question whether flash messages actually need the Secure flag at all, but I haven't actually thought about the threat model there at all.

@LukeMathWalker
Copy link
Owner

It really depends on what information is travelling in those messages, which is highly application-specific.
In our newsletter, we could downgrade them to be non-secure and we wouldn't really diminish our security posture.

Thanks for the investigation btw! It's probably worth to disable the Secure flag in our tests (and provide the necessary hooks in the flash message crate to support it). I'll keep it in mind for the next revision of the book while keeping this issue open for others that may bump into the same issues.

@rlpowell
Copy link
Author

It is worth noting that the same issue exists with manually testing the admin dashboard at the end of 10.8.1. I was able to fix it like so:

    let server = HttpServer::new(move || {
        App::new()
            .wrap(message_framework.clone())
            .wrap(
                SessionMiddleware::builder(redis_store.clone(), secret_key.clone())
                    .cookie_secure(false)
                    .build(),
            )

Obviously not OK for production, but 🤷

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

No branches or pull requests

2 participants