Repository navigation
fix(console): mark the session cookie Secure when the server serves TLS - #773
Open
LeeroyHannigan wants to merge 1 commit into
Open
LeeroyHannigan wants to merge 1 commit into
LeeroyHannigan wants to merge 1 commit into
Conversation
The console set its session cookie, and the cookie that clears it on logout, with HttpOnly, SameSite=Strict and Path=/console but without Secure. A browser attaches such a cookie to a plain http:// request to the same host. The login response also sends HSTS, but a browser does not record HSTS for a host given as an IP address or over a connection with certificate errors, which includes the self-signed certificate extenddb init creates. On such a host, an http:// request the browser is led to send carries the session token in clear text, ahead of the server's redirect to HTTPS. ConsoleState gains secure_cookies, set from AppState.tls_enabled, whose doc comment already said it affects the cookie's Secure flag. Both cookies come from session_cookie, which appends Secure when it is set. A dev-mode build serves plain HTTP, where Secure protects nothing and an HTTP client that does not treat the loopback origin as secure, such as the Python requests client the console tests use, would not send the cookie back, so it leaves Secure off. The security model guide lists the attribute. Unit tests check both cookies with and without Secure. The console login and logout tests check that Secure is present exactly when the endpoint is HTTPS. Against a TLS SQLite build and a dev-mode build on 127.0.0.1, the console auth and role tests pass. With secure_cookies forced false, the login and logout tests fail on the TLS build. With it forced true, they fail on the dev-mode build, and so does the dashboard test, because the client no longer sends the cookie over HTTP. Fixes #687.
LeeroyHannigan
requested review from
amrith,
c33howard,
eliseong,
hitesh96db,
jcshepherd,
pdf-amzn,
robinnsc and
yesyayen
as code owners
November 3, 2026 01:22
4 of 8 tasks
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The console set its session cookie, and the cookie that clears it on logout, as
extenddb_session=<token>; Path=/console; HttpOnly; SameSite=Strict; Max-Age=28800withoutSecure. A browser attaches such a cookie to a plainhttp://request to the same host. The login response also sends HSTS, but a browser does not record HSTS for a host given as an IP address or over a connection with certificate errors, which includes the self-signed certificateextenddb initcreates. On such a host, anhttp://request the browser is led to send carries the session token in clear text, ahead of the server's redirect to HTTPS.Change
ConsoleStategainssecure_cookies, set fromAppState.tls_enabled, whose doc comment already said it affects the cookie'sSecureflag.session_cookie, which appendsSecurewhensecure_cookiesis set.Secureoff. ThereSecureprotects nothing, and an HTTP client that does not treat the loopback origin as secure, such as the Pythonrequestsclient the console tests use, would not send the cookie back. Browsers exemptlocalhostfrom theSecurerule.docs/manuals/10-security-model.mdlists the attribute.The cookie keeps its name. A
__Secure-prefix, which the issue mentions as optional, would also changeextract_session_tokenand end every open session at upgrade.Tests
auth_pages.rscheck the login and logout cookies with and withoutSecure. They compare the attributes as a set after thename=valuepair.test_login_redirects_to_dashboardandtest_logout_clears_sessionintests/test_console_integration.pycheck thatSecureis present exactly when the endpoint is HTTPS. Every job that runs that file uses anhttps://endpoint.Testing
cargo +1.97.0 fmt --all -- --checkandcargo +1.97.0 clippy -p extenddb-server --all-targets -- -D warnings: clean.cargo +1.88.0 test --locked -p extenddb-server: 252 passed.devtools/run-tests --extenddb --pytest --filter "TestConsoleAuth or TestConsoleRoles": 10 passed against a TLS build (--features sqlite,extenddb init,serveonhttps://127.0.0.1) and 10 passed against a dev-mode build (plain HTTP). On the TLS build, login answeredPath=/console; HttpOnly; SameSite=Strict; Max-Age=28800; Secure, and the next request with the cookie got the dashboard.secure_cookiesforced false, the login and logout tests fail on the TLS build. With it forced true, they fail on the dev-mode build, and so doestest_dashboard_accessible_after_login, because the client no longer sends the cookie over HTTP.No browser was driven; the HSTS and
localhoststatements above follow RFC 6797 and theSet-Cookiedocumentation.Overlap with #704
#704 rewrites the same line of
docs/manuals/10-security-model.md(the session expiry wording), so the two conflict on that line only.Fixes #687.