Compare commits

...
Author SHA1 Message Date
dev_agentandClaude Opus 5.5 c824942484 End any existing session when a sign-in is refused
CI / ci (pull_request) Successful in 1m24s
Independent review Codex gpt-6-sol high: APPROVED (0 findings)
CI / ci (push) Successful in 1m28s
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>
2026-10-03 10:25:59 -04:00
dev_agentandClaude Opus 5.5 f9f10c3a53 Gate sign-in on household group and require a session for /api/v1 and /media
The callback now starts a session only when the verified groups claim
contains a group from MEALQUEST_OIDC_ALLOWED_GROUPS. When the ID token has
no groups claim, groups come from the userinfo endpoint, whose subject must
match the ID token. A refused sign-in gets a 403 HTML page ("This kitchen is
for the household only") with a sign-out button, and no session. The check
runs on every login, so an account removed from the group is refused at its
next one. auth.New requires at least one allowed group and a discovered
userinfo endpoint that passes the endpoint rule.

The RequireSession middleware wraps the /api/v1 and /media subrouters: no
live session gets 401 {"error":{"code":"unauthenticated",...}}, a lookup
failure 500, and a live session reaches the handler with the actor in the
context. It wraps the subrouters instead of using chi's Use because chi
skips middleware on a subrouter with no routes. /health, /auth and the web
build stay public.

auth.WithActor and auth.ActorFrom are the one way to read the signed-in
household member; handlers record Actor.Subject as updated_by.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-03 10:24:35 -04:00
12 changed files with 757 additions and 56 deletions
+7 -6
View File
@@ -57,17 +57,18 @@ func run(cfg config.Config, logger *slog.Logger) error {
return err
}
authFlow, err := auth.New(ctx, auth.Config{
Issuer: cfg.OIDCIssuer.String(),
ClientID: cfg.OIDCClientID,
ClientSecret: cfg.OIDCClientSecret.Reveal(),
PublicURL: cfg.PublicURL,
SecretKey: []byte(cfg.SecretKey.Reveal()),
Issuer: cfg.OIDCIssuer.String(),
ClientID: cfg.OIDCClientID,
ClientSecret: cfg.OIDCClientSecret.Reveal(),
PublicURL: cfg.PublicURL,
SecretKey: []byte(cfg.SecretKey.Reveal()),
AllowedGroups: cfg.OIDCAllowedGroups,
}, sessions, logger)
if err != nil {
return err
}
router, err := server.NewRouter(logger, version, authFlow, frontend.Assets())
router, err := server.NewRouter(logger, version, authFlow, sessions, frontend.Assets())
if err != nil {
return err
}
+35
View File
@@ -0,0 +1,35 @@
package auth
import (
"context"
"slices"
)
// Actor is the signed-in household member a request acts for. Handlers
// record Actor.Subject as updated_by on edits.
type Actor struct {
Subject string
Email string
Name string
Groups []string
}
// Actor returns the household member s belongs to.
func (s Session) Actor() Actor {
return Actor{Subject: s.Subject, Email: s.Email, Name: s.Name, Groups: slices.Clone(s.Groups)}
}
type actorKey struct{}
// WithActor returns a copy of ctx carrying actor. The session middleware
// calls it for every authenticated request.
func WithActor(ctx context.Context, actor Actor) context.Context {
return context.WithValue(ctx, actorKey{}, actor)
}
// ActorFrom returns the actor WithActor stored in ctx. ok is false when ctx
// has none, which behind the session middleware means a wiring bug.
func ActorFrom(ctx context.Context) (actor Actor, ok bool) {
actor, ok = ctx.Value(actorKey{}).(Actor)
return actor, ok
}
+32
View File
@@ -0,0 +1,32 @@
package auth
import (
"context"
"slices"
"testing"
)
func TestActorContext(t *testing.T) {
t.Parallel()
if _, ok := ActorFrom(context.Background()); ok {
t.Error("ActorFrom found an actor in an empty context")
}
want := Actor{Subject: "user-123", Email: "cook@example.com", Name: "Home Cook", Groups: []string{"mealquest-admin"}}
got, ok := ActorFrom(WithActor(context.Background(), want))
if !ok || got.Subject != want.Subject || got.Email != want.Email || got.Name != want.Name || !slices.Equal(got.Groups, want.Groups) {
t.Errorf("ActorFrom = %+v (ok=%v), want %+v", got, ok, want)
}
}
func TestSessionActor(t *testing.T) {
t.Parallel()
s := Session{Subject: "user-123", Email: "cook@example.com", Name: "Home Cook", Groups: []string{"mealquest-admin"}, CSRFToken: "t"}
a := s.Actor()
if a.Subject != s.Subject || a.Email != s.Email || a.Name != s.Name || !slices.Equal(a.Groups, s.Groups) {
t.Errorf("Actor = %+v, want the session's identity", a)
}
a.Groups[0] = "changed"
if s.Groups[0] != "mealquest-admin" {
t.Error("Actor shares the session's groups slice")
}
}
+75 -2
View File
@@ -12,6 +12,7 @@ import (
"log/slog"
"net/http"
"net/url"
"slices"
"time"
"github.com/coreos/go-oidc/v3/oidc"
@@ -65,15 +66,20 @@ type Config struct {
PublicURL *url.URL
// SecretKey is the server key the login cookie's signing key derives from.
SecretKey []byte
// AllowedGroups are the household's groups; a sign-in is refused unless
// the account is in at least one of them.
AllowedGroups []string
}
// Auth serves the login and callback routes.
type Auth struct {
oauth oauth2.Config
verifier *oidc.IDTokenVerifier
provider *oidc.Provider
client *http.Client
cookie loginCookie
sessions Sessions
allowed []string
// endSessionURL is the provider's end_session_endpoint; logout ends
// there so the provider's own session ends too.
endSessionURL string
@@ -92,6 +98,8 @@ func New(ctx context.Context, cfg Config, sessions Sessions, logger *slog.Logger
return nil, errors.New("auth: secret key is required")
case sessions == nil:
return nil, errors.New("auth: session layer is required")
case len(cfg.AllowedGroups) == 0:
return nil, errors.New("auth: at least one allowed group is required")
}
if err := checkEndpoint("issuer", cfg.Issuer); err != nil {
@@ -118,6 +126,9 @@ func New(ctx context.Context, cfg Config, sessions Sessions, logger *slog.Logger
{"token endpoint", endpoint.TokenURL},
{"JWKS endpoint", discovered.JWKSURI},
{"end session endpoint", discovered.EndSessionURI},
// Groups come from userinfo when the ID token carries none, and the
// access token is sent there.
{"userinfo endpoint", provider.UserInfoEndpoint()},
} {
if err := checkEndpoint(e.name, e.raw); err != nil {
return nil, err
@@ -138,7 +149,9 @@ func New(ctx context.Context, cfg Config, sessions Sessions, logger *slog.Logger
ClientID: cfg.ClientID,
SupportedSigningAlgs: []string{oidc.RS256},
}),
provider: provider,
client: client,
allowed: slices.Clone(cfg.AllowedGroups),
cookie: newLoginCookie(cfg.SecretKey),
sessions: sessions,
endSessionURL: discovered.EndSessionURI,
@@ -218,7 +231,9 @@ func (a *Auth) login(w http.ResponseWriter, r *http.Request) {
}
// callback completes the flow: it checks state against the login cookie,
// exchanges the code, verifies the ID token and nonce, then starts a session.
// exchanges the code, verifies the ID token and nonce, then starts a session
// if the account is in an allowed group. Membership is checked on every
// login, so an account removed from the group is refused at its next one.
func (a *Auth) callback(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Cache-Control", "no-store")
ctx := r.Context()
@@ -256,6 +271,22 @@ func (a *Auth) callback(w http.ResponseWriter, r *http.Request) {
http.Error(w, "sign-in failed", http.StatusUnauthorized)
return
}
if !a.member(claims.Groups) {
if claims.Groups == nil {
a.logger.WarnContext(ctx, "sign-in refused: no groups claim in the ID token or userinfo", "subject", claims.Subject)
} 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
}
if err := a.sessions.StartSession(w, r, claims); err != nil {
a.logger.ErrorContext(ctx, "start session", "error", err)
http.Error(w, "internal error", http.StatusInternalServerError)
@@ -278,7 +309,8 @@ func (a *Auth) logout(w http.ResponseWriter, r *http.Request) {
// verify exchanges code for tokens and returns the verified ID token claims.
// The verifier enforces RS256, issuer, audience and expiry; nonce is checked
// here.
// here. When the ID token has no groups claim, Groups comes from userinfo;
// it stays nil when neither has one.
func (a *Auth) verify(ctx context.Context, code string, state loginState) (Claims, error) {
token, err := a.oauth.Exchange(context.WithValue(ctx, oauth2.HTTPClient, a.client), code,
oauth2.VerifierOption(state.Verifier))
@@ -303,9 +335,50 @@ func (a *Auth) verify(ctx context.Context, code string, state loginState) (Claim
if claims.Subject == "" {
return Claims{}, errors.New("ID token has no subject")
}
if claims.Groups == nil {
groups, err := a.userinfoGroups(ctx, token, claims.Subject)
if err != nil {
return Claims{}, err
}
claims.Groups = groups
}
return claims, nil
}
// userinfoGroups returns the groups claim from the userinfo endpoint, nil if
// it has none. The userinfo subject must match the ID token's subject.
func (a *Auth) userinfoGroups(ctx context.Context, token *oauth2.Token, subject string) ([]string, error) {
info, err := a.provider.UserInfo(oidc.ClientContext(ctx, a.client), oauth2.StaticTokenSource(token))
if err != nil {
return nil, userinfoError(err)
}
if info.Subject != subject {
return nil, errors.New("userinfo subject does not match the ID token")
}
var claims struct {
Groups []string `json:"groups"`
}
if err := info.Claims(&claims); err != nil {
return nil, fmt.Errorf("decode userinfo claims: %w", err)
}
return claims.Groups, nil
}
// userinfoError reduces a failed userinfo request to a fixed category, like
// exchangeError: go-oidc's error text can carry the response body.
func userinfoError(err error) error {
var transport *url.Error
if errors.As(err, &transport) {
return fmt.Errorf("userinfo: request failed: %s: %w", transport.Op, transport.Err)
}
return errors.New("userinfo: endpoint returned an unusable response")
}
// member reports whether groups contains an allowed group.
func (a *Auth) member(groups []string) bool {
return slices.ContainsFunc(groups, func(g string) bool { return slices.Contains(a.allowed, g) })
}
// exchangeError reduces a failed code exchange to a fixed category and safe
// metadata. The oauth2 error text can carry the token endpoint's response
// body or error_description, which may echo credentials, so it is dropped.
+83 -30
View File
@@ -36,6 +36,9 @@ const (
var testPublicURL = &url.URL{Scheme: "https", Host: "meals.example.com"}
// testAllowedGroups are the household's groups in tests.
var testAllowedGroups = []string{"mealquest-admin", "mealquest-family"}
// fakeProvider is an OIDC provider serving discovery, JWKS and a token
// endpoint. Each issued code maps to the ID token the token endpoint returns.
type fakeProvider struct {
@@ -50,6 +53,12 @@ type fakeProvider struct {
// tokenReply, when set, is returned by the token endpoint instead of a
// token.
tokenReply *tokenReply
// userinfo is the userinfo endpoint's response; userinfoHits counts
// requests to it.
userinfo map[string]any
userinfoHits int
// userinfoReply, when set, is returned by the userinfo endpoint instead.
userinfoReply *tokenReply
}
type tokenReply struct {
@@ -73,6 +82,7 @@ func newFakeProvider(t *testing.T) *fakeProvider {
mux.HandleFunc("GET /.well-known/openid-configuration", p.discovery)
mux.HandleFunc("GET /jwks", p.jwks)
mux.HandleFunc("POST /token", p.token)
mux.HandleFunc("GET /userinfo", p.userinfoEndpoint)
p.srv = httptest.NewServer(mux)
t.Cleanup(p.srv.Close)
return p
@@ -86,6 +96,7 @@ func (p *fakeProvider) discovery(w http.ResponseWriter, _ *http.Request) {
"authorization_endpoint": p.issuer() + "/authorize",
"token_endpoint": p.issuer() + "/token",
"jwks_uri": p.issuer() + "/jwks",
"userinfo_endpoint": p.issuer() + "/userinfo",
"end_session_endpoint": p.issuer() + "/end-session/",
"response_types_supported": []string{"code"},
"subject_types_supported": []string{"public"},
@@ -148,6 +159,41 @@ func (p *fakeProvider) token(w http.ResponseWriter, r *http.Request) {
})
}
// userinfoEndpoint checks the access token and returns p.userinfo, which
// defaults to the test user without a groups claim.
func (p *fakeProvider) userinfoEndpoint(w http.ResponseWriter, r *http.Request) {
if r.Header.Get("Authorization") != "Bearer access-token" {
http.Error(w, `{"error":"invalid_token"}`, http.StatusUnauthorized)
return
}
p.mu.Lock()
p.userinfoHits++
info, reply := p.userinfo, p.userinfoReply
p.mu.Unlock()
if reply != nil {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(reply.status)
io.WriteString(w, reply.body)
return
}
if info == nil {
info = map[string]any{"sub": "user-123", "email": "cook@example.com"}
}
writeJSON(w, info)
}
func (p *fakeProvider) setUserinfo(info map[string]any) {
p.mu.Lock()
defer p.mu.Unlock()
p.userinfo = info
}
func (p *fakeProvider) userinfoRequests() int {
p.mu.Lock()
defer p.mu.Unlock()
return p.userinfoHits
}
// authorize stands in for the user signing in at the provider: it records a
// code bound to the PKCE challenge and returns it.
func (p *fakeProvider) authorize(challenge, idToken string) string {
@@ -291,11 +337,12 @@ func newHarness(t *testing.T) *harness {
sessions := &memorySessions{}
logs := &syncBuffer{}
a, err := New(context.Background(), Config{
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("server-secret-key"),
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("server-secret-key"),
AllowedGroups: testAllowedGroups,
}, sessions, slog.New(slog.NewTextHandler(logs, nil)))
if err != nil {
t.Fatal(err)
@@ -730,11 +777,12 @@ func TestNewFailsWhenDiscoveryFails(t *testing.T) {
srv := httptest.NewServer(http.NotFoundHandler())
t.Cleanup(srv.Close)
_, err := New(context.Background(), Config{
Issuer: srv.URL,
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("k"),
Issuer: srv.URL,
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("k"),
AllowedGroups: testAllowedGroups,
}, &memorySessions{}, slog.New(slog.DiscardHandler))
if err == nil {
t.Fatal("New succeeded without a discovery document")
@@ -747,11 +795,12 @@ func TestNewFailsOnIssuerMismatch(t *testing.T) {
// Discovery trims the trailing slash, so this fetches the provider's
// document, which reports the issuer without it.
_, err := New(context.Background(), Config{
Issuer: p.issuer() + "/",
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("k"),
Issuer: p.issuer() + "/",
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("k"),
AllowedGroups: testAllowedGroups,
}, &memorySessions{}, slog.New(slog.DiscardHandler))
if err == nil {
t.Fatal("New accepted a provider reporting a different issuer")
@@ -772,11 +821,12 @@ func TestNewRejectsRemoteHTTPIssuer(t *testing.T) {
t.Parallel()
for _, issuer := range []string{"http://auth.example.com/application/o/mealquest/", "http://192.0.2.10"} {
_, err := New(context.Background(), Config{
Issuer: issuer,
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("k"),
Issuer: issuer,
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("k"),
AllowedGroups: testAllowedGroups,
}, &memorySessions{}, slog.New(slog.DiscardHandler))
if err == nil || !strings.Contains(err.Error(), "issuer") {
t.Errorf("New(issuer %q) error = %v, want issuer rejected", issuer, err)
@@ -793,6 +843,7 @@ func TestNewRejectsRemoteHTTPEndpoint(t *testing.T) {
{"token_endpoint", "token endpoint"},
{"jwks_uri", "JWKS endpoint"},
{"end_session_endpoint", "end session endpoint"},
{"userinfo_endpoint", "userinfo endpoint"},
}
for _, tc := range tests {
t.Run(tc.field, func(t *testing.T) {
@@ -800,11 +851,12 @@ func TestNewRejectsRemoteHTTPEndpoint(t *testing.T) {
p := newFakeProvider(t)
p.discoveryOverrides = map[string]any{tc.field: "http://auth.example.com/" + tc.field}
_, err := New(context.Background(), Config{
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("k"),
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("k"),
AllowedGroups: testAllowedGroups,
}, &memorySessions{}, slog.New(slog.DiscardHandler))
if err == nil || !strings.Contains(err.Error(), tc.name) {
t.Errorf("New error = %v, want %s rejected", err, tc.name)
@@ -840,11 +892,12 @@ func TestCallbackRefusesTokenRedirectToHTTP(t *testing.T) {
sessions := &memorySessions{}
logs := &syncBuffer{}
a, err := New(context.Background(), Config{
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("server-secret-key"),
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("server-secret-key"),
AllowedGroups: testAllowedGroups,
}, sessions, slog.New(slog.NewTextHandler(logs, nil)))
if err != nil {
t.Fatal(err)
+264
View File
@@ -0,0 +1,264 @@
package auth
import (
"context"
"errors"
"io"
"log/slog"
"net/http"
"net/url"
"slices"
"strings"
"testing"
)
// signInWith completes a login whose ID token has claims edited by edit.
func (h *harness) signInWith(t *testing.T, edit func(claims map[string]any)) *http.Response {
t.Helper()
l := h.login(t, LoginPath)
c := h.provider.claims(l.query.Get("nonce"))
edit(c)
return h.callback(l, l.query.Get("state"), h.provider.signRS256(c))
}
// 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 {
t.Fatalf("callback status = %d, want 403", resp.StatusCode)
}
if ct := resp.Header.Get("Content-Type"); ct != "text/html; charset=utf-8" {
t.Errorf("Content-Type = %q, want text/html; charset=utf-8", ct)
}
body, err := io.ReadAll(resp.Body)
if err != nil {
t.Fatal(err)
}
for _, want := range []string{
"This kitchen is for the household only",
`<form method="post" action="/auth/logout">`,
"Sign out",
} {
if !strings.Contains(string(body), want) {
t.Errorf("refused page lacks %q:\n%s", want, body)
}
}
for _, c := range resp.Cookies() {
// 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)
}
}
assertLoginCookieCleared(t, resp)
}
func TestGroupGateMember(t *testing.T) {
t.Parallel()
// Any one allowed group is enough, alongside groups that are not allowed.
h := newHarness(t)
resp := h.signInWith(t, func(c map[string]any) { c["groups"] = []string{"guests", "mealquest-family"} })
if resp.StatusCode != http.StatusSeeOther {
t.Fatalf("callback status = %d, want 303", resp.StatusCode)
}
if started := h.sessions.started(); len(started) != 1 || !slices.Equal(started[0].Groups, []string{"guests", "mealquest-family"}) {
t.Errorf("sessions started = %+v, want one with the ID token groups", started)
}
if n := h.provider.userinfoRequests(); n != 0 {
t.Errorf("userinfo requested %d times, want 0 when the ID token has groups", n)
}
}
func TestGroupGateRefused(t *testing.T) {
t.Parallel()
tests := []struct {
name string
groups any // ID token groups claim; nil removes it
userinfo map[string]any
}{
{name: "non-member", groups: []string{"guests", "MEALQUEST-ADMIN"}},
{name: "empty groups claim", groups: []string{}},
{name: "groups claim missing from ID token and userinfo", groups: nil},
{
name: "non-member in userinfo",
groups: nil,
userinfo: map[string]any{"sub": "user-123", "groups": []string{"guests"}},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
h := newHarness(t)
if tc.userinfo != nil {
h.provider.setUserinfo(tc.userinfo)
}
resp := h.signInWith(t, func(c map[string]any) {
if tc.groups == nil {
delete(c, "groups")
} else {
c["groups"] = tc.groups
}
})
assertRefused(t, resp)
if started := h.sessions.started(); len(started) != 0 {
t.Errorf("sessions started = %+v, want none", started)
}
if !strings.Contains(h.logs.String(), "sign-in refused") {
t.Errorf("refusal not logged:\n%s", h.logs.String())
}
})
}
}
func TestGroupGateUsesUserinfoWhenIDTokenHasNoGroups(t *testing.T) {
t.Parallel()
h := newHarness(t)
h.provider.setUserinfo(map[string]any{"sub": "user-123", "groups": []string{"mealquest-admin"}})
resp := h.signInWith(t, func(c map[string]any) { delete(c, "groups") })
if resp.StatusCode != http.StatusSeeOther {
t.Fatalf("callback status = %d, want 303", resp.StatusCode)
}
if started := h.sessions.started(); len(started) != 1 || !slices.Equal(started[0].Groups, []string{"mealquest-admin"}) {
t.Errorf("sessions started = %+v, want one with the userinfo groups", started)
}
if n := h.provider.userinfoRequests(); n != 1 {
t.Errorf("userinfo requested %d times, want 1", n)
}
}
func TestGroupGateUserinfoFailures(t *testing.T) {
t.Parallel()
tests := []struct {
name string
userinfo map[string]any
}{
{"subject mismatch", map[string]any{"sub": "someone-else", "groups": []string{"mealquest-admin"}}},
{"malformed groups", map[string]any{"sub": "user-123", "groups": "mealquest-admin"}},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
h := newHarness(t)
h.provider.setUserinfo(tc.userinfo)
resp := h.signInWith(t, func(c map[string]any) { delete(c, "groups") })
if resp.StatusCode != http.StatusUnauthorized {
t.Errorf("callback status = %d, want 401", resp.StatusCode)
}
if started := h.sessions.started(); len(started) != 0 {
t.Errorf("sessions started = %+v, want none", started)
}
})
}
}
// A failed userinfo request fails the sign-in without logging the
// endpoint's response, which may echo credentials.
func TestGroupGateUserinfoErrorNotLogged(t *testing.T) {
t.Parallel()
h := newHarness(t)
h.provider.mu.Lock()
h.provider.userinfoReply = &tokenReply{status: http.StatusInternalServerError, body: `{"error":"leaked-s3cret"}`}
h.provider.mu.Unlock()
resp := h.signInWith(t, func(c map[string]any) { delete(c, "groups") })
if resp.StatusCode != http.StatusUnauthorized {
t.Errorf("callback status = %d, want 401", resp.StatusCode)
}
if started := h.sessions.started(); len(started) != 0 {
t.Errorf("sessions started = %+v, want none", started)
}
logs := h.logs.String()
if !strings.Contains(logs, "userinfo") || strings.Contains(logs, "leaked-s3cret") {
t.Errorf("logs = %s, want the userinfo failure without the response body", logs)
}
}
// Membership is checked at every login: after removal from the group, the
// next login is refused and starts no session.
func TestGroupGateRemovedMemberRefusedAtNextLogin(t *testing.T) {
t.Parallel()
h := newSessionHarness(t)
h.signIn(t)
if n := len(h.rows(t)); n != 1 {
t.Fatalf("sessions after first login = %d, want 1", n)
}
resp := h.signInWith(t, func(c map[string]any) { c["groups"] = []string{"former-household"} })
assertRefused(t, resp)
if n := len(h.rows(t)); n != 1 {
t.Errorf("sessions after refused login = %d, want still 1", n)
}
}
// 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)
_, err := New(context.Background(), Config{
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("k"),
}, &memorySessions{}, slog.New(slog.DiscardHandler))
if err == nil || !strings.Contains(err.Error(), "allowed group") {
t.Errorf("New error = %v, want missing allowed groups named", err)
}
}
func TestNewRequiresUserinfoEndpoint(t *testing.T) {
t.Parallel()
p := newFakeProvider(t)
p.discoveryOverrides = map[string]any{"userinfo_endpoint": nil}
_, err := New(context.Background(), Config{
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("k"),
AllowedGroups: testAllowedGroups,
}, &memorySessions{}, slog.New(slog.DiscardHandler))
if err == nil || !strings.Contains(err.Error(), "userinfo endpoint") {
t.Errorf("New error = %v, want the missing userinfo endpoint named", err)
}
}
+37
View File
@@ -0,0 +1,37 @@
package auth
import (
"io"
"net/http"
)
// refusedPage is shown when a sign-in succeeds at the provider but the
// account is not in the household's group. Signing out ends the provider
// session too, so another account can sign in.
const refusedPage = `<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>MealQuest</title>
</head>
<body>
<main>
<h1>This kitchen is for the household only</h1>
<p>The account you signed in with is not part of our kitchen.</p>
<form method="post" action="` + LogoutPath + `">
<button type="submit">Sign out</button>
</form>
</main>
</body>
</html>
`
// refuse writes the refused-sign-in page with 403.
func (a *Auth) refuse(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/html; charset=utf-8")
w.WriteHeader(http.StatusForbidden)
if _, err := io.WriteString(w, refusedPage); err != nil {
a.logger.WarnContext(r.Context(), "write refused sign-in page", "error", err)
}
}
+12 -10
View File
@@ -62,11 +62,12 @@ func newSessionHarness(t *testing.T) *sessionHarness {
p := newFakeProvider(t)
logs := &syncBuffer{}
a, err := New(ctx, Config{
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("server-secret-key"),
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("server-secret-key"),
AllowedGroups: testAllowedGroups,
}, sessions, slog.New(slog.NewTextHandler(logs, nil)))
if err != nil {
t.Fatal(err)
@@ -490,11 +491,12 @@ func TestNewRequiresEndSessionEndpoint(t *testing.T) {
p := newFakeProvider(t)
p.discoveryOverrides = map[string]any{"end_session_endpoint": nil}
_, err := New(context.Background(), Config{
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("k"),
Issuer: p.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("k"),
AllowedGroups: testAllowedGroups,
}, &memorySessions{}, slog.New(slog.DiscardHandler))
if err == nil || !strings.Contains(err.Error(), "end session endpoint") {
t.Errorf("New error = %v, want the missing end session endpoint named", err)
+17 -6
View File
@@ -22,9 +22,10 @@ type RouteMounter interface {
}
// NewRouter returns the HTTP handler for the service. version is reported by
// GET /health; auth mounts the /auth routes; assets is the web build served
// for every other GET.
func NewRouter(logger *slog.Logger, version string, auth RouteMounter, assets fs.FS) (http.Handler, error) {
// GET /health; auth mounts the /auth routes; sessions guards /api/v1 and
// /media; assets is the web build served for every other GET. /health, /auth
// and the web build are public.
func NewRouter(logger *slog.Logger, version string, auth RouteMounter, sessions SessionLookup, assets fs.FS) (http.Handler, error) {
notFound := apiErrorHandler(logger, http.StatusNotFound, "not_found", "not found")
methodNotAllowed := apiErrorHandler(logger, http.StatusMethodNotAllowed, "method_not_allowed", "method not allowed")
@@ -39,13 +40,23 @@ func NewRouter(logger *slog.Logger, version string, auth RouteMounter, assets fs
r.Get("/health", health(logger, version))
auth.Mount(r)
// The session middleware wraps the whole /api/v1 and /media subrouters
// rather than being added with Use: chi skips a subrouter's middleware
// when it has no routes, which would leave it unguarded.
signedIn := requireSession(logger, sessions)
apiV1, media := chi.NewRouter(), chi.NewRouter()
for _, sub := range []*chi.Mux{apiV1, media} {
sub.MethodNotAllowed(methodNotAllowed)
sub.NotFound(notFound)
}
r.Route("/api", func(r chi.Router) {
r.MethodNotAllowed(methodNotAllowed)
r.NotFound(notFound)
r.Mount("/v1", signedIn(apiV1))
})
// Catch-all for the PWA. Unknown /auth paths and /media (no routes yet)
// reach the frontend handler, which passes them to notFound rather than
// the SPA fallback.
r.Mount("/media", signedIn(media))
// Catch-all for the PWA. Unknown /auth paths reach the frontend handler,
// which passes them to notFound rather than the SPA fallback.
r.Method(http.MethodGet, "/*", web)
r.Method(http.MethodHead, "/*", web)
return r, nil
+7 -2
View File
@@ -27,7 +27,7 @@ func newTestRouter(t *testing.T, version string) http.Handler {
"index.html": {Data: []byte(fixtureIndex)},
"assets/index-AbC123.js": {Data: []byte(fixtureAsset)},
}
r, err := NewRouter(logger, version, stubAuth{}, assets)
r, err := NewRouter(logger, version, stubAuth{}, stubSessions{}, assets)
if err != nil {
t.Fatal(err)
}
@@ -126,6 +126,8 @@ func TestReservedPathsNotFound(t *testing.T) {
}{
{http.MethodGet, "/api"},
{http.MethodGet, "/api/unknown"},
{http.MethodGet, "/api/v1/unknown"},
{http.MethodPost, "/api/v1/unknown"},
{http.MethodPost, "/api/unknown"},
{http.MethodGet, "/auth/unknown"},
{http.MethodGet, "/media/photo.jpg"},
@@ -137,8 +139,11 @@ func TestReservedPathsNotFound(t *testing.T) {
t.Run(tt.method+" "+tt.path, func(t *testing.T) {
t.Parallel()
// Signed in, so /api/v1 and /media reach their routers.
req := httptest.NewRequest(tt.method, tt.path, nil)
req.AddCookie(validSessionCookie)
rec := httptest.NewRecorder()
router.ServeHTTP(rec, httptest.NewRequest(tt.method, tt.path, nil))
router.ServeHTTP(rec, req)
if rec.Code != http.StatusNotFound {
t.Errorf("status = %d, want 404", rec.Code)
+37
View File
@@ -0,0 +1,37 @@
package server
import (
"errors"
"log/slog"
"net/http"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth"
)
// SessionLookup finds the live session a request carries; it returns
// auth.ErrNoSession when there is none. *auth.SessionManager implements it.
type SessionLookup interface {
SessionFor(r *http.Request) (auth.Session, error)
}
// requireSession is the RequireSession middleware. It passes requests with a
// live session to next with the session's actor in the context (read it with
// auth.ActorFrom); others get 401 unauthenticated.
func requireSession(logger *slog.Logger, sessions SessionLookup) func(http.Handler) http.Handler {
unauthenticated := apiErrorHandler(logger, http.StatusUnauthorized, "unauthenticated", "sign in required")
internal := apiErrorHandler(logger, http.StatusInternalServerError, "internal", "internal error")
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
s, err := sessions.SessionFor(r)
switch {
case errors.Is(err, auth.ErrNoSession):
unauthenticated(w, r)
case err != nil:
logger.ErrorContext(r.Context(), "look up session", "error", err)
internal(w, r)
default:
next.ServeHTTP(w, r.WithContext(auth.WithActor(r.Context(), s.Actor())))
}
})
}
}
+151
View File
@@ -0,0 +1,151 @@
package server
import (
"errors"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"slices"
"testing"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth"
)
const (
unauthenticatedJSON = `{"error":{"code":"unauthenticated","message":"sign in required","retryable":false}}`
internalJSON = `{"error":{"code":"internal","message":"internal error","retryable":false}}`
)
var (
validSessionCookie = &http.Cookie{Name: auth.SessionCookieName, Value: "valid"}
brokenSessionCookie = &http.Cookie{Name: auth.SessionCookieName, Value: "broken"}
testSession = auth.Session{
Subject: "user-123",
Email: "cook@example.com",
Name: "Home Cook",
Groups: []string{"mealquest-admin"},
}
)
// stubSessions knows one valid session cookie; the broken cookie fails the
// lookup as a database error would.
type stubSessions struct{}
func (stubSessions) SessionFor(r *http.Request) (auth.Session, error) {
c, err := r.Cookie(auth.SessionCookieName)
switch {
case err != nil:
return auth.Session{}, auth.ErrNoSession
case c.Value == validSessionCookie.Value:
return testSession, nil
case c.Value == brokenSessionCookie.Value:
return auth.Session{}, errors.New("database down")
default:
return auth.Session{}, auth.ErrNoSession
}
}
func TestProtectedPathsRequireSession(t *testing.T) {
t.Parallel()
router := newTestRouter(t, "dev")
for _, tc := range []struct {
method, path string
cookie *http.Cookie
}{
{http.MethodGet, "/api/v1/settings/household", nil},
{http.MethodPatch, "/api/v1/settings/household", nil},
{http.MethodGet, "/api/v1", nil},
{http.MethodGet, "/media/photo.jpg", nil},
{http.MethodGet, "/api/v1/settings/household", &http.Cookie{Name: auth.SessionCookieName, Value: "unknown"}},
} {
t.Run(tc.method+" "+tc.path, func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest(tc.method, tc.path, nil)
if tc.cookie != nil {
req.AddCookie(tc.cookie)
}
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
if rec.Code != http.StatusUnauthorized {
t.Errorf("status = %d, want 401", rec.Code)
}
if ct := rec.Header().Get("Content-Type"); ct != "application/json" {
t.Errorf("Content-Type = %q, want application/json", ct)
}
if got := rec.Body.String(); got != unauthenticatedJSON {
t.Errorf("body = %s, want %s", got, unauthenticatedJSON)
}
})
}
}
func TestPublicPathsNeedNoSession(t *testing.T) {
t.Parallel()
router := newTestRouter(t, "dev")
for _, path := range []string{"/health", "/auth/login", "/", "/assets/index-AbC123.js", "/quests/42"} {
t.Run(path, func(t *testing.T) {
t.Parallel()
rec := httptest.NewRecorder()
router.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
if rec.Code != http.StatusOK {
t.Errorf("GET %s without a cookie = %d, want 200", path, rec.Code)
}
})
}
}
func TestSessionLookupFailure(t *testing.T) {
t.Parallel()
req := httptest.NewRequest(http.MethodGet, "/api/v1/settings/household", nil)
req.AddCookie(brokenSessionCookie)
rec := httptest.NewRecorder()
newTestRouter(t, "dev").ServeHTTP(rec, req)
if rec.Code != http.StatusInternalServerError || rec.Body.String() != internalJSON {
t.Errorf("response = %d %s, want 500 %s", rec.Code, rec.Body.String(), internalJSON)
}
}
func TestRequireSessionAttachesActor(t *testing.T) {
t.Parallel()
var got auth.Actor
var ok bool
next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
got, ok = auth.ActorFrom(r.Context())
w.WriteHeader(http.StatusNoContent)
})
h := requireSession(slog.New(slog.NewTextHandler(io.Discard, nil)), stubSessions{})(next)
req := httptest.NewRequest(http.MethodGet, "/api/v1/settings/household", nil)
req.AddCookie(validSessionCookie)
rec := httptest.NewRecorder()
h.ServeHTTP(rec, req)
if rec.Code != http.StatusNoContent {
t.Fatalf("status = %d, want the handler's 204", rec.Code)
}
if !ok || got.Subject != "user-123" || got.Email != "cook@example.com" || got.Name != "Home Cook" ||
!slices.Equal(got.Groups, []string{"mealquest-admin"}) {
t.Errorf("actor = %+v (ok=%v), want the session's household member", got, ok)
}
}
func TestRequireSessionStopsUnauthenticated(t *testing.T) {
t.Parallel()
called := false
next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { called = true })
h := requireSession(slog.New(slog.NewTextHandler(io.Discard, nil)), stubSessions{})(next)
rec := httptest.NewRecorder()
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/v1/settings/household", nil))
if called || rec.Code != http.StatusUnauthorized {
t.Errorf("handler called = %v, status = %d; want not called and 401", called, rec.Code)
}
}