refactor(oidc): drop single-subject allowlist, authenticate-only (ADR-012)
ADR-011's single-user authz (ID-token subject must equal AllowedSubject, else 403) is replaced by ADR-012's model: Dex authentication is the only gate — any Dex-authenticated subject may establish a session. Whether that subject has a tapir user, and routing to registration if not, is decided downstream in internal/web (next commit). Removals (noted): oidc.Config.AllowedSubject + its required-field check + the callback 403 branch; config.Config.AllowedSubject + TAPIR_ALLOWED_SUBJECT env wiring; the AllowedSubject arg in cmdServe. ui-spec.md updated to reflect the supersession. Sessions, cookie signing, login/callback/logout unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -57,15 +57,14 @@ type Config struct {
|
||||
// HTTPAddr is the listen address for `tapir serve` (the Stage-0 web UI).
|
||||
HTTPAddr string
|
||||
|
||||
// Dex OIDC (web login, ADR-011). When OIDCIssuer is empty, `serve` falls back
|
||||
// to the allow-all StubAuth (local dev). When set, serve uses Dex with
|
||||
// single-user allowlist authz.
|
||||
// Dex OIDC (web login, ADR-011/012). When OIDCIssuer is empty, `serve` falls
|
||||
// back to the allow-all StubAuth (local dev). When set, serve uses Dex: any
|
||||
// Dex-authenticated subject may sign in, then registers a tapir user (ADR-012).
|
||||
OIDCIssuer string
|
||||
DexClientID string
|
||||
DexClientSecret string
|
||||
OIDCRedirectURL string
|
||||
SessionSecret string
|
||||
AllowedSubject string
|
||||
}
|
||||
|
||||
// DexConfigured reports whether Dex OIDC login is wired (issuer present). When
|
||||
@@ -104,7 +103,6 @@ func Load() (Config, error) {
|
||||
DexClientSecret: os.Getenv("TAPIR_DEX_CLIENT_SECRET"),
|
||||
OIDCRedirectURL: os.Getenv("TAPIR_OIDC_REDIRECT_URL"),
|
||||
SessionSecret: os.Getenv("TAPIR_SESSION_SECRET"),
|
||||
AllowedSubject: os.Getenv("TAPIR_ALLOWED_SUBJECT"),
|
||||
}
|
||||
|
||||
timeout, err := durationOr("TAPIR_SUMMARIZER_TIMEOUT", defaultSummarizerTimeout)
|
||||
|
||||
+17
-24
@@ -4,11 +4,13 @@
|
||||
// interface, so swapping the stub for Dex is a wiring choice in cmd/tapir, not
|
||||
// a code change (ADR-003).
|
||||
//
|
||||
// Authentication is real (Dex OIDC); authorization is single-user — the ID
|
||||
// token's subject must equal Config.AllowedSubject or the request is refused
|
||||
// with 403. Sessions are server-side (in-memory, fine for the single Stage-0
|
||||
// replica) addressed by an HMAC-signed (HS256) HttpOnly Secure SameSite=Lax
|
||||
// cookie with a short TTL and sliding refresh. Tokens are never logged.
|
||||
// Authentication is real (Dex OIDC) and is the only gate: any Dex-authenticated
|
||||
// subject may sign in (ADR-012 dropped ADR-011's single-subject allowlist).
|
||||
// Authorization/registration is layered on top in internal/web (an authenticated
|
||||
// subject with no tapir user is routed to registration). Sessions are server-side
|
||||
// (in-memory, fine for the single Stage-1 replica) addressed by an HMAC-signed
|
||||
// (HS256) HttpOnly Secure SameSite=Lax cookie with a short TTL and sliding
|
||||
// refresh. Tokens are never logged.
|
||||
//
|
||||
// This is mcp-chassis's cousin but NOT the same code: mcp-chassis validates
|
||||
// inbound Bearer JWTs for MCP APIs; this is a browser session login.
|
||||
@@ -28,8 +30,8 @@ import (
|
||||
)
|
||||
|
||||
// Config is the OIDC + session configuration. cmd/tapir maps these from
|
||||
// TAPIR_OIDC_*/TAPIR_DEX_*/TAPIR_SESSION_SECRET/TAPIR_ALLOWED_SUBJECT; this
|
||||
// package takes the resolved struct.
|
||||
// TAPIR_OIDC_*/TAPIR_DEX_*/TAPIR_SESSION_SECRET; this package takes the resolved
|
||||
// struct.
|
||||
type Config struct {
|
||||
// Issuer is the Dex issuer URL, e.g. https://auth.d-ma.be. Discovery
|
||||
// (.well-known/openid-configuration) runs against it in New.
|
||||
@@ -42,9 +44,6 @@ type Config struct {
|
||||
RedirectURL string
|
||||
// SessionSecret keys the HS256 session-cookie signature. Never logged.
|
||||
SessionSecret string
|
||||
// AllowedSubject is the single Dex subject permitted to sign in. Everyone
|
||||
// else is refused 403 (single-user authz, ADR-011).
|
||||
AllowedSubject string
|
||||
}
|
||||
|
||||
const (
|
||||
@@ -102,12 +101,11 @@ func WithInsecureCookies() Option {
|
||||
// discovery request only.
|
||||
func New(ctx context.Context, cfg Config, opts ...Option) (*DexAuth, error) {
|
||||
for name, val := range map[string]string{
|
||||
"issuer": cfg.Issuer,
|
||||
"client id": cfg.ClientID,
|
||||
"client secret": cfg.ClientSecret,
|
||||
"redirect url": cfg.RedirectURL,
|
||||
"session secret": cfg.SessionSecret,
|
||||
"allowed subject": cfg.AllowedSubject,
|
||||
"issuer": cfg.Issuer,
|
||||
"client id": cfg.ClientID,
|
||||
"client secret": cfg.ClientSecret,
|
||||
"redirect url": cfg.RedirectURL,
|
||||
"session secret": cfg.SessionSecret,
|
||||
} {
|
||||
if strings.TrimSpace(val) == "" {
|
||||
return nil, fmt.Errorf("oidc: missing %s", name)
|
||||
@@ -242,14 +240,9 @@ func (d *DexAuth) handleCallback(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// Single-user authz: only the allowlisted subject may sign in. On mismatch
|
||||
// we echo the caller's own subject (an opaque id, not a secret) so the
|
||||
// maintainer can bootstrap TAPIR_ALLOWED_SUBJECT on first login.
|
||||
if idToken.Subject != d.cfg.AllowedSubject {
|
||||
http.Error(w, "forbidden — not the allowlisted subject. your subject is: "+idToken.Subject, http.StatusForbidden)
|
||||
return
|
||||
}
|
||||
|
||||
// Authentication is the only gate (ADR-012): any Dex-authenticated subject may
|
||||
// establish a session. Whether that subject has a tapir user — and routing to
|
||||
// registration if not — is decided downstream in internal/web, not here.
|
||||
var claims struct {
|
||||
Email string `json:"email"`
|
||||
}
|
||||
|
||||
@@ -20,7 +20,7 @@ import (
|
||||
|
||||
const (
|
||||
testClientID = "tapir-web"
|
||||
allowedSub = "allowed-subject-123"
|
||||
testSubject = "dex-subject-123"
|
||||
)
|
||||
|
||||
// fakeIssuer is an httptest-backed OIDC provider: it serves a discovery
|
||||
@@ -115,12 +115,11 @@ func writeJSON(t *testing.T, w http.ResponseWriter, v any) {
|
||||
func newAuth(t *testing.T, f *fakeIssuer) *oidc.DexAuth {
|
||||
t.Helper()
|
||||
auth, err := oidc.New(context.Background(), oidc.Config{
|
||||
Issuer: f.server.URL,
|
||||
ClientID: testClientID,
|
||||
ClientSecret: "test-client-secret",
|
||||
RedirectURL: "http://tapir.test/auth/callback",
|
||||
SessionSecret: "test-session-secret-please-change",
|
||||
AllowedSubject: allowedSub,
|
||||
Issuer: f.server.URL,
|
||||
ClientID: testClientID,
|
||||
ClientSecret: "test-client-secret",
|
||||
RedirectURL: "http://tapir.test/auth/callback",
|
||||
SessionSecret: "test-session-secret-please-change",
|
||||
}, oidc.WithInsecureCookies())
|
||||
require.NoError(t, err)
|
||||
return auth
|
||||
@@ -140,12 +139,12 @@ func login(t *testing.T, auth *oidc.DexAuth) (state, nonce string) {
|
||||
return q.Get("state"), q.Get("nonce")
|
||||
}
|
||||
|
||||
// authenticate completes a full login+callback for the allowlisted subject and
|
||||
// returns the resulting session cookie.
|
||||
// authenticate completes a full login+callback for the test subject and returns
|
||||
// the resulting session cookie.
|
||||
func authenticate(t *testing.T, auth *oidc.DexAuth, f *fakeIssuer) *http.Cookie {
|
||||
t.Helper()
|
||||
state, nonce := login(t, auth)
|
||||
f.sub, f.email, f.nonce = allowedSub, "maintainer@d-ma.be", nonce
|
||||
f.sub, f.email, f.nonce = testSubject, "maintainer@d-ma.be", nonce
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
auth.Routes().ServeHTTP(rec, httptest.NewRequest(http.MethodGet,
|
||||
@@ -189,7 +188,7 @@ func TestLoginRedirectsToAuthorize(t *testing.T) {
|
||||
require.Contains(t, q.Get("scope"), "openid")
|
||||
}
|
||||
|
||||
func TestCallbackAllowedSubjectSetsSession(t *testing.T) {
|
||||
func TestCallbackSetsSession(t *testing.T) {
|
||||
f := newFakeIssuer(t)
|
||||
auth := newAuth(t, f)
|
||||
|
||||
@@ -199,26 +198,35 @@ func TestCallbackAllowedSubjectSetsSession(t *testing.T) {
|
||||
req.AddCookie(cookie)
|
||||
user, ok := auth.CurrentUser(req)
|
||||
require.True(t, ok)
|
||||
require.Equal(t, allowedSub, user.Subject)
|
||||
require.Equal(t, testSubject, user.Subject)
|
||||
require.Equal(t, "maintainer@d-ma.be", user.Email)
|
||||
|
||||
require.True(t, cookie.HttpOnly)
|
||||
require.Equal(t, http.SameSiteLaxMode, cookie.SameSite)
|
||||
}
|
||||
|
||||
func TestCallbackNonAllowedSubjectForbidden(t *testing.T) {
|
||||
// TestCallbackAnySubjectAuthenticates proves the single-subject allowlist is gone
|
||||
// (ADR-012): a subject other than any prior allowlist still gets a session.
|
||||
func TestCallbackAnySubjectAuthenticates(t *testing.T) {
|
||||
f := newFakeIssuer(t)
|
||||
auth := newAuth(t, f)
|
||||
|
||||
state, nonce := login(t, auth)
|
||||
f.sub, f.email, f.nonce = "intruder-999", "intruder@elsewhere.test", nonce
|
||||
f.sub, f.email, f.nonce = "some-other-subject-999", "other@elsewhere.test", nonce
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
auth.Routes().ServeHTTP(rec, httptest.NewRequest(http.MethodGet,
|
||||
"/auth/callback?code=valid-code&state="+state, nil))
|
||||
|
||||
require.Equal(t, http.StatusForbidden, rec.Code)
|
||||
require.Empty(t, rec.Result().Cookies(), "no session for a rejected subject")
|
||||
require.Equal(t, http.StatusFound, rec.Code)
|
||||
require.Equal(t, "/", rec.Header().Get("Location"))
|
||||
|
||||
cookie := sessionCookie(t, rec.Result())
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.AddCookie(cookie)
|
||||
user, ok := auth.CurrentUser(req)
|
||||
require.True(t, ok)
|
||||
require.Equal(t, "some-other-subject-999", user.Subject)
|
||||
}
|
||||
|
||||
func TestCallbackUnknownStateRejected(t *testing.T) {
|
||||
@@ -304,12 +312,11 @@ func TestExpiredSessionRejected(t *testing.T) {
|
||||
f := newFakeIssuer(t)
|
||||
clock := time.Now()
|
||||
auth, err := oidc.New(context.Background(), oidc.Config{
|
||||
Issuer: f.server.URL,
|
||||
ClientID: testClientID,
|
||||
ClientSecret: "test-client-secret",
|
||||
RedirectURL: "http://tapir.test/auth/callback",
|
||||
SessionSecret: "test-session-secret-please-change",
|
||||
AllowedSubject: allowedSub,
|
||||
Issuer: f.server.URL,
|
||||
ClientID: testClientID,
|
||||
ClientSecret: "test-client-secret",
|
||||
RedirectURL: "http://tapir.test/auth/callback",
|
||||
SessionSecret: "test-session-secret-please-change",
|
||||
}, oidc.WithInsecureCookies(),
|
||||
oidc.WithSessionTTL(time.Minute),
|
||||
oidc.WithClock(func() time.Time { return clock }))
|
||||
|
||||
Reference in New Issue
Block a user