diff --git a/UPGRADE.md b/UPGRADE.md index 369ecae..436ed11 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,28 @@ # Upgrade Guide +## Releases + +There is no changelog; this list says what a release contains, and the sections below say what to change. + +**1.1.0** (tag `v1.1`: the release job drops a `.0` patch), the first release from `master` after 1.0.31. The +`v1.0.32` tag is not on `master`: it was cut from the merge ref of pull request #2 before the release job ran on +pushes only, so it holds that pull request's change and nothing after it. + +- Opt-in Azu integration: `require "session/azu"` and `Session::Web.use_store` (typed `context.session` and + `context.flash`), `config.cookie_secure` and `config.cookie_same_site`, and the session is saved before the first + byte of the response is written. +- Expired sessions are refused when loaded, for every store; `RedisStore` sessions end at `expires_at` unless + `sliding_expiration` is on. +- No published default secret: the secret comes from `SESSION_SECRET` (random per process without it, outside + production and staging); production and staging refuse a missing or published secret, a secret shorter than + 32 bytes is refused, and `Session.configure` checks it at boot when the store encrypts with it. A `MemoryStore` never + needs it. +- Isolation fixes: the current session, the flash messages and `CookieStore`'s request cookies are kept per request, + not per store. `bind_to_ip` and `bind_to_user_agent` now answer a session used from another client with a + fresh session (they never refused anything), with `trust_proxy_headers` for `X-Forwarded-For`; `Store#authenticate` changes privileges under + a new session id. +- Crystal 1.22.1 or newer. + ## No published default secret The secret used to default to a string published in this repository, so an app that never set one signed its diff --git a/docs/getting-started/quick-start.md b/docs/getting-started/quick-start.md index 456a13e..f6f2145 100644 --- a/docs/getting-started/quick-start.md +++ b/docs/getting-started/quick-start.md @@ -37,8 +37,9 @@ Session.use_store(Session::MemoryStore(UserSession)) Session.configure do |config| # The key comes from SESSION_SECRET (32 bytes or more). Without it, development gets a random - # key per process; in production and staging, a store that encrypts (CookieStore, or - # encrypt_redis_data) makes Session.configure raise Session::InsecureSecretException at boot. + # key per process; in production and staging, a store that encrypts with it (a Redis store with + # encrypt_redis_data, or CookieStore) makes Session.configure raise + # Session::InsecureSecretException at boot. This MemoryStore never needs it. # Session timeout config.timeout = 24.hours diff --git a/spec/configuration_spec.cr b/spec/configuration_spec.cr index cbb8b24..5d206ff 100644 --- a/spec/configuration_spec.cr +++ b/spec/configuration_spec.cr @@ -439,6 +439,48 @@ describe Session::Configuration do end end + describe "the default secret messages name where the secret came from" do + around_each do |example| + previous = ENV["SESSION_SECRET"]? + ENV.delete("SESSION_SECRET") + example.run + ensure + previous ? (ENV["SESSION_SECRET"] = previous) : ENV.delete("SESSION_SECRET") + end + + it "tell an explicit assignment to go, without an ENV[] that raises when the variable is unset" do + config = Session::Configuration.new + config.secret = Session::Configuration::DEFAULT_SECRET + config.require_secure_secret = true + + message = expect_raises(Session::InsecureSecretException) { config.validate_secret! }.message.to_s + message.should contain "remove the `secret =` assignment" + message.should_not contain %(ENV["SESSION_SECRET"]) + end + + it "say SESSION_SECRET holds the published value when it does" do + ENV["SESSION_SECRET"] = Session::Configuration::DEFAULT_SECRET + config = Session::Configuration.new + config.require_secure_secret = true + + message = expect_raises(Session::InsecureSecretException) { config.validate_secret! }.message.to_s + message.should contain "SESSION_SECRET holds the published default" + message.should_not contain "remove the `secret =` assignment" + end + + it "warn with the same advice" do + config = Session::Configuration.new + config.secret = Session::Configuration::DEFAULT_SECRET + config.require_secure_secret = false + + Log.capture do |logs| + config.validate_secret! + logs.check(:warn, /remove the `secret =` assignment/) + logs.entry.message.should_not contain %(ENV["SESSION_SECRET"]) + end + end + end + describe "#validate_secret_for!" do it "refuses a short secret for a store that encrypts with it" do Session.config.secret = "short" @@ -607,3 +649,24 @@ describe "Store#encrypts_with_secret?" do end end end + +describe "reset_config (spec helper)" do + it "puts back the lifecycle callbacks a spec replaced" do + calls = 0 + Session.config.on_started = ->(_sid : String, _data : Session::Base) { calls += 1; nil } + Session.config.on_deleted = ->(_sid : String, _data : Session::Base) { calls += 1; nil } + Session.config.on_loaded = ->(_sid : String, _data : Session::Base) { calls += 1; nil } + Session.config.on_client = ->(_sid : String, _data : Session::Base) { calls += 1; nil } + Session.config.on_regenerated = ->(_old : String, _new : String, _data : Session::Base) { calls += 1; nil } + + reset_config + + session = UserSession.new + Session.config.on_started.call("sid", session) + Session.config.on_deleted.call("sid", session) + Session.config.on_loaded.call("sid", session) + Session.config.on_client.call("sid", session) + Session.config.on_regenerated.call("old", "new", session) + calls.should eq 0 + end +end diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 0689098..2b4cdf1 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -57,6 +57,11 @@ def reset_config config.fail_fast_on_corruption = defaults.fail_fast_on_corruption config.metrics_backend = Session::Metrics::NullBackend.new config.cluster = Session::ClusterConfig.new + config.on_started = defaults.on_started + config.on_deleted = defaults.on_deleted + config.on_loaded = defaults.on_loaded + config.on_client = defaults.on_client + config.on_regenerated = defaults.on_regenerated end Spec.before_each do diff --git a/src/configuration.cr b/src/configuration.cr index 69da7f2..3a30c7b 100644 --- a/src/configuration.cr +++ b/src/configuration.cr @@ -294,22 +294,29 @@ module Session if require_secure_secret raise InsecureSecretException.new( - "Using default secret is not allowed when require_secure_secret is enabled. " \ - "Set SESSION_SECRET to a random value of at least #{MIN_SECRET_BYTES} bytes (`openssl rand -hex 32`), " \ - "or configure one with Session.configure { |c| c.secret = ENV[\"SESSION_SECRET\"] }" + "Using default secret is not allowed when require_secure_secret is enabled: #{default_secret_advice}" ) end # Log warning only once to avoid spam unless @secret_warning_logged Log.warn { - "SECURITY WARNING: Using default session secret. This is insecure for production. " \ - "Set SESSION_SECRET to a random value of at least #{MIN_SECRET_BYTES} bytes (`openssl rand -hex 32`), " \ - "or configure one with Session.configure { |c| c.secret = ENV[\"SESSION_SECRET\"] }" + "SECURITY WARNING: Using default session secret. This is insecure for production: #{default_secret_advice}" } @secret_warning_logged = true end end + + # The published secret is never a fallback: it was assigned, or `SESSION_SECRET` holds it (an + # assigned secret wins over the variable, so the assignment is what has to go). + private def default_secret_advice : String + replacement = "a random value of at least #{MIN_SECRET_BYTES} bytes (`openssl rand -hex 32`)" + if @secret + "remove the `secret =` assignment and set SESSION_SECRET to #{replacement}, or assign #{replacement}" + else + "SESSION_SECRET holds the published default; set it to #{replacement}" + end + end end class InvalidCookieOptions < Exception