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>
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user