From c824942484eceb492a88a382c8455a5f2f84aad3 Mon Sep 17 00:00:00 2001
From: dev_agent <dev_agent@cubecraftlabs.com>
Date: Sat, 3 Oct 2026 10:25:59 -0400
Subject: [PATCH] End any existing session when a sign-in is refused

A browser that still holds a session and signs in as an account outside
the household's group got the 403 page but kept its old session. The
refusal now ends that session and clears its cookie first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/auth/auth.go      |  7 ++++++
 internal/auth/gate_test.go | 50 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/internal/auth/auth.go b/internal/auth/auth.go
index b1cf2c9..4d3c1fb 100644
--- a/internal/auth/auth.go
+++ b/internal/auth/auth.go
@@ -277,6 +277,13 @@ func (a *Auth) callback(w http.ResponseWriter, r *http.Request) {
 		} else {
 			a.logger.WarnContext(ctx, "sign-in refused: not in an allowed group", "subject", claims.Subject)
 		}
+		// A session this browser already holds ends too, so a refused
+		// sign-in never leaves it signed in.
+		if err := a.sessions.EndSession(w, r); err != nil {
+			a.logger.ErrorContext(ctx, "end session", "error", err)
+			http.Error(w, "internal error", http.StatusInternalServerError)
+			return
+		}
 		a.refuse(w, r)
 		return
 	}
diff --git a/internal/auth/gate_test.go b/internal/auth/gate_test.go
index fffa832..0f18362 100644
--- a/internal/auth/gate_test.go
+++ b/internal/auth/gate_test.go
@@ -2,9 +2,11 @@ package auth
 
 import (
 	"context"
+	"errors"
 	"io"
 	"log/slog"
 	"net/http"
+	"net/url"
 	"slices"
 	"strings"
 	"testing"
@@ -19,7 +21,8 @@ func (h *harness) signInWith(t *testing.T, edit func(claims map[string]any)) *ht
 	return h.callback(l, l.query.Get("state"), h.provider.signRS256(c))
 }
 
-// assertRefused checks resp is the refused-sign-in page and sets no session.
+// assertRefused checks resp is the refused-sign-in page and starts no
+// session.
 func assertRefused(t *testing.T, resp *http.Response) {
 	t.Helper()
 	if resp.StatusCode != http.StatusForbidden {
@@ -42,7 +45,8 @@ func assertRefused(t *testing.T, resp *http.Response) {
 		}
 	}
 	for _, c := range resp.Cookies() {
-		if c.Name == SessionCookieName || c.Name == "test_session" {
+		// Clearing a session cookie (negative Max-Age) is expected.
+		if (c.Name == SessionCookieName || c.Name == "test_session") && c.MaxAge >= 0 {
 			t.Errorf("refused sign-in set session cookie %s", c.Name)
 		}
 	}
@@ -185,6 +189,48 @@ func TestGroupGateRemovedMemberRefusedAtNextLogin(t *testing.T) {
 	}
 }
 
+// A refused sign-in from a browser that still holds a session ends that
+// session and clears its cookie.
+func TestGroupGateRefusalEndsExistingSession(t *testing.T) {
+	t.Parallel()
+	h := newSessionHarness(t)
+	_, cookie := h.signIn(t)
+
+	l := h.login(t, LoginPath)
+	c := h.provider.claims(l.query.Get("nonce"))
+	c["groups"] = []string{"former-household"}
+	code := h.provider.authorize(l.query.Get("code_challenge"), h.provider.signRS256(c))
+	q := url.Values{"code": {code}, "state": {l.query.Get("state")}}
+	resp := h.get(CallbackPath+"?"+q.Encode(), l.cookie, cookie)
+
+	if resp.StatusCode != http.StatusForbidden {
+		t.Fatalf("callback status = %d, want 403", resp.StatusCode)
+	}
+	if n := len(h.rows(t)); n != 0 {
+		t.Errorf("sessions after refused login = %d, want 0", n)
+	}
+	if _, err := h.use(cookie); !errors.Is(err, ErrNoSession) {
+		t.Errorf("old session lookup error = %v, want ErrNoSession", err)
+	}
+	cleared := false
+	for _, c := range resp.Cookies() {
+		cleared = cleared || (c.Name == SessionCookieName && c.MaxAge < 0)
+	}
+	if !cleared {
+		t.Error("refused sign-in did not clear the session cookie")
+	}
+}
+
+func TestGroupGateRefusalEndSessionError(t *testing.T) {
+	t.Parallel()
+	h := newHarness(t)
+	h.sessions.err = errors.New("database down")
+	resp := h.signInWith(t, func(c map[string]any) { c["groups"] = []string{"guests"} })
+	if resp.StatusCode != http.StatusInternalServerError {
+		t.Errorf("callback status = %d, want 500", resp.StatusCode)
+	}
+}
+
 func TestNewRequiresAllowedGroups(t *testing.T) {
 	t.Parallel()
 	p := newFakeProvider(t)