diff --git a/app.rb b/app.rb index b4a7a49..7a4aae3 100644 --- a/app.rb +++ b/app.rb @@ -15,6 +15,40 @@ require_relative "lib/maptool/plugins" require_relative "lib/maptool/pmtiles_builder" module Maptool + # Only /login, /auth/callback and /logout ever write to session + # (oauth_state, user, session.clear) — everything else 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, a 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] with + # nothing logged anywhere to explain it. + # + # A plain Rack middleware, not a Sinatra `before` filter: static assets + # (public_folder — every JS/CSS/font/vendor file) are served by + # Sinatra's own built-in static-file shortcut, which bypasses route and + # filter dispatch entirely but is still wrapped by this, same as every + # other response — each one was independently capable of winning the + # clobbering race a `before` filter alone left uncovered. + class SkipSessionWrite + WRITE_PATHS = %w[/login /auth/callback /logout].freeze + + def initialize(app) + @app = app + end + + def call(env) + env["rack.session.options"][:skip] = true unless WRITE_PATHS.include?(env["PATH_INFO"]) + @app.call(env) + end + end + class App < Sinatra::Base set :public_folder, File.join(__dir__, "public") set :views, File.join(__dir__, "views") @@ -33,6 +67,7 @@ module Maptool # don't survive a restart there, which is fine outside production. set :sessions, httponly: true, secure: production?, same_site: :lax, secret: ENV.fetch("SESSION_SECRET") { SecureRandom.hex(32) } + use SkipSessionWrite # Cache-busts /js and /css requests: browsers (and a caching proxy in # front of production, see docs/deployment.adoc) hang onto static @@ -150,28 +185,6 @@ 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(