auth: clear session cookie under both host-only and parent-domain scope
Logout emitted a single Set-Cookie scoped to the configured cookie domain (parodia.dev). A browser holding the session under the older host-only scope (news.parodia.dev, from before the cookie domain widened for the games site) was never cleared, so logout looked like a no-op and stranded the user on a stale session logout couldn't reach. Clear both scopes. Also surface the who-page owner unlock's previously-silent misses: a genuine lookup/decode error, and a signed-in session that carries no username. Both used to fail with err discarded and no log, making a stuck owner undiagnosable.
This commit is contained in:
+15
-3
@@ -195,11 +195,23 @@ func (a *Authenticator) setCookie(w http.ResponseWriter, name, value string, ttl
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (a *Authenticator) clearCookie(w http.ResponseWriter, name string) {
|
func (a *Authenticator) clearCookie(w http.ResponseWriter, name string) {
|
||||||
http.SetCookie(w, &http.Cookie{
|
// A cookie delete only lands when its Domain matches the one the cookie was
|
||||||
|
// set with. The session cookie's scope has changed over Pete's life: host-only
|
||||||
|
// before the games site, then widened to the parent domain so news and games
|
||||||
|
// could share one login. A browser may still hold it under the older scope, and
|
||||||
|
// a clear under only the current scope leaves the other in place — a stranded
|
||||||
|
// cookie that keeps someone signed in with a session logout can't reach. So
|
||||||
|
// clear both: always host-only, plus the parent domain when one is configured.
|
||||||
|
base := http.Cookie{
|
||||||
Name: name, Value: "", Path: "/", MaxAge: -1,
|
Name: name, Value: "", Path: "/", MaxAge: -1,
|
||||||
Domain: a.cookieDomain(name),
|
|
||||||
HttpOnly: true, Secure: true, SameSite: http.SameSiteLaxMode,
|
HttpOnly: true, Secure: true, SameSite: http.SameSiteLaxMode,
|
||||||
})
|
}
|
||||||
|
http.SetCookie(w, &base) // host-only (no Domain attribute)
|
||||||
|
if d := a.cookieDomain(name); d != "" {
|
||||||
|
scoped := base
|
||||||
|
scoped.Domain = d
|
||||||
|
http.SetCookie(w, &scoped) // parent-domain scope
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// oauthFor returns the OAuth config to use for this request. The configured
|
// oauthFor returns the OAuth config to use for this request. The configured
|
||||||
|
|||||||
@@ -8,6 +8,56 @@ import (
|
|||||||
"golang.org/x/oauth2"
|
"golang.org/x/oauth2"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// TestClearCookieClearsBothScopes: when a parent cookie domain is configured,
|
||||||
|
// clearCookie must emit a delete for BOTH the parent-domain scope and the
|
||||||
|
// host-only scope. A browser holding a session under the older host-only scope
|
||||||
|
// (from before the cookie domain widened for the games site) would otherwise
|
||||||
|
// survive logout and keep the user signed in with a session logout can't reach.
|
||||||
|
func TestClearCookieClearsBothScopes(t *testing.T) {
|
||||||
|
a := &Authenticator{domain: "parodia.dev"}
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
a.clearCookie(rec, sessionCookie)
|
||||||
|
|
||||||
|
var hostOnly, scoped bool
|
||||||
|
for _, c := range rec.Result().Cookies() {
|
||||||
|
if c.Name != sessionCookie {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if c.MaxAge >= 0 {
|
||||||
|
t.Errorf("clear cookie should expire the session, got MaxAge=%d", c.MaxAge)
|
||||||
|
}
|
||||||
|
switch c.Domain {
|
||||||
|
case "":
|
||||||
|
hostOnly = true
|
||||||
|
case "parodia.dev":
|
||||||
|
scoped = true
|
||||||
|
default:
|
||||||
|
t.Errorf("unexpected clear Domain %q", c.Domain)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !hostOnly {
|
||||||
|
t.Error("missing host-only clear (no Domain) — stale host-only sessions stay stranded")
|
||||||
|
}
|
||||||
|
if !scoped {
|
||||||
|
t.Error("missing parent-domain clear (Domain=parodia.dev)")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// With no cookie domain configured, only the host-only clear is emitted.
|
||||||
|
func TestClearCookieHostOnlyWhenNoDomain(t *testing.T) {
|
||||||
|
a := &Authenticator{}
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
a.clearCookie(rec, sessionCookie)
|
||||||
|
|
||||||
|
got := rec.Result().Cookies()
|
||||||
|
if len(got) != 1 {
|
||||||
|
t.Fatalf("want exactly one clear cookie, got %d", len(got))
|
||||||
|
}
|
||||||
|
if got[0].Domain != "" {
|
||||||
|
t.Errorf("want host-only clear, got Domain=%q", got[0].Domain)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestSignVerifyRoundTrip(t *testing.T) {
|
func TestSignVerifyRoundTrip(t *testing.T) {
|
||||||
a := &Authenticator{secret: []byte("test-secret-key-at-least-16")}
|
a := &Authenticator{secret: []byte("test-secret-key-at-least-16")}
|
||||||
payload := []byte(`{"sub":"abc","exp":123}`)
|
payload := []byte(`{"sub":"abc","exp":123}`)
|
||||||
|
|||||||
+12
-1
@@ -221,7 +221,18 @@ func (s *Server) handleAdventureWho(w http.ResponseWriter, r *http.Request) {
|
|||||||
// reversing the token, so no visitor can unlock another player's self extras.
|
// reversing the token, so no visitor can unlock another player's self extras.
|
||||||
if s.auth != nil {
|
if s.auth != nil {
|
||||||
if u := s.auth.userFromRequest(r); u != nil {
|
if u := s.auth.userFromRequest(r); u != nil {
|
||||||
if self, ok, err := storage.PlayerDetailByOwner(buyerLocalpart(u), token); err == nil && ok {
|
// A signed-in visitor viewing someone else's page legitimately fails the
|
||||||
|
// ownership join (ok=false), so that alone is not worth a log. Two other
|
||||||
|
// misses are: a genuine lookup/decode error, and a session that carries no
|
||||||
|
// username (minted before the game economy existed) — for the latter the
|
||||||
|
// join can never match because buyerLocalpart is empty. Both used to fail
|
||||||
|
// silently here; surface them so a stuck owner is diagnosable.
|
||||||
|
lp := buyerLocalpart(u)
|
||||||
|
if lp == "" {
|
||||||
|
slog.Warn("who: signed-in session has no username; owner unlock skipped", "sub", u.Sub)
|
||||||
|
} else if self, ok, err := storage.PlayerDetailByOwner(lp, token); err != nil {
|
||||||
|
slog.Error("who: owner detail lookup failed", "localpart", lp, "token", token, "err", err)
|
||||||
|
} else if ok {
|
||||||
page.HasSelf = true
|
page.HasSelf = true
|
||||||
page.Self = self
|
page.Self = self
|
||||||
page.Worn = itemRows(self.Equipped, "worn")
|
page.Worn = itemRows(self.Equipped, "worn")
|
||||||
|
|||||||
Reference in New Issue
Block a user