diff --git a/app.rb b/app.rb index 0845779..b4a7a49 100644 --- a/app.rb +++ b/app.rb @@ -34,21 +34,6 @@ module Maptool set :sessions, httponly: true, secure: production?, same_site: :lax, secret: ENV.fetch("SESSION_SECRET") { SecureRandom.hex(32) } - # Sinatra enables Rack::Protection::SessionHijacking by default once - # sessions are on: it fingerprints a session to the User-Agent header - # seen on the request that first wrote to it, and *silently* empties - # the session (no exception, nothing logged) on any later request - # where that header doesn't match — which is exactly what a login - # behind production's reverse proxy hit: the OAuth callback set - # session[:user] fine, but the very next ordinary page load (or even a - # plain refresh) came back logged out with nothing in the logs to - # explain it. Its own docs already admit every header it can check is - # spoofable anyway ("will not prevent determined hijacking attempts") - # — losing it is a minor tradeoff for a login that actually stays - # logged in. Every other Rack::Protection check (XSS header, path - # traversal, JSON CSRF, frame options, ...) stays on. - set :protection, except: [:session_hijacking] - # Cache-busts /js and /css requests: browsers (and a caching proxy in # front of production, see docs/deployment.adoc) hang onto static # assets for hours, so without this a deploy that changes app.js/css @@ -165,6 +150,28 @@ module Maptool end end + # Only these three routes ever write to session (oauth_state, user, + # session.clear below) — every other route only ever reads it, if it + # touches session at all. That still matters: Rack::Session::Cookie + # re-serializes and re-sets the cookie on *every* response that so + # much as read session (Rack::Protection's own CSRF check does, on + # nearly every request) — reflecting whatever cookie that particular + # request happened to arrive with, whether or not anything changed. + # A HAR capture caught the actual failure mode this caused: seconds + # after a successful login, offline_data.js's _checkUpdate — an + # unrelated background poll fired the instant the page loads — was + # still carrying the browser's pre-login cookie (a client-side timing + # race across the OAuth redirect chain, not something this server + # controls), and its own re-issued Set-Cookie landed *after* the + # login's, silently overwriting session[:user] back out with nothing + # logged anywhere to explain it. Skipping the write everywhere except + # the routes that actually need it removes every other response from + # that race — a stale request can still be served from a stale + # cookie, but it can no longer clobber a fresher one. + before do + session.options[:skip] = true unless %w[/login /auth/callback /logout].include?(request.path_info) + end + get "/login" do session[:oauth_state] = SecureRandom.hex(16) redirect OAUTH_CLIENT.auth_code.authorize_url(