From 556656d09079d275e4e8d1f041c8e70fb3be7b8d Mon Sep 17 00:00:00 2001
From: Dan Milne <d@nmilne.com>
Date: Sat, 2 May 2026 23:54:09 +1000
Subject: [PATCH] Drop Remember-me cookie's Expires when the box is unchecked
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Without Remember-me the session cookie was still being written via
`cookies.signed.permanent`, so it survived browser restart on shared
devices — surprising for a user who explicitly opted out of Remember-me.
Issue a browser-session cookie (no Expires) when remember_me is off;
the server-side Session#expires_at still bounds the 24h / 30d window.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 app/controllers/concerns/authentication.rb   | 10 +++++++++-
 test/controllers/sessions_controller_test.rb | 18 ++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb
index 799fefd..f9f51ae 100644
--- a/app/controllers/concerns/authentication.rb
+++ b/app/controllers/concerns/authentication.rb
@@ -73,7 +73,15 @@ module Authentication
       # Set domain for cross-subdomain authentication if we can extract it
       cookie_options[:domain] = domain if domain.present?
 
-      cookies.signed.permanent[:session_id] = cookie_options
+      # When "Remember me" is off, issue a browser-session cookie (no Expires)
+      # so closing the browser signs the user out — especially important on
+      # shared devices. The server Session#expires_at still enforces the
+      # 24h / 30d window regardless.
+      if remember_me
+        cookies.signed.permanent[:session_id] = cookie_options
+      else
+        cookies.signed[:session_id] = cookie_options
+      end
 
       # Create a one-time token for immediate forward auth after authentication
       # This solves the race condition where browser hasn't processed cookie yet
diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb
index c273f59..90d6b74 100644
--- a/test/controllers/sessions_controller_test.rb
+++ b/test/controllers/sessions_controller_test.rb
@@ -30,4 +30,22 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to signin_path
     assert_empty cookies[:session_id]
   end
+
+  test "session cookie has no Expires attribute when remember_me is off" do
+    post session_path, params: {email_address: @user.email_address, password: "password", remember_me: "0"}
+
+    set_cookie = Array(response.headers["Set-Cookie"]).find { |c| c.start_with?("session_id=") }
+    assert set_cookie, "session_id cookie should be set"
+    refute_match(/expires=/i, set_cookie,
+      "without Remember me, the session cookie must be a browser-session cookie (no Expires)")
+  end
+
+  test "session cookie has long-lived Expires attribute when remember_me is on" do
+    post session_path, params: {email_address: @user.email_address, password: "password", remember_me: "1"}
+
+    set_cookie = Array(response.headers["Set-Cookie"]).find { |c| c.start_with?("session_id=") }
+    assert set_cookie, "session_id cookie should be set"
+    assert_match(/expires=/i, set_cookie,
+      "with Remember me, the cookie should have an Expires attribute")
+  end
 end