From ea9ab96f84851e4847aed7c408241a20efcafc0f Mon Sep 17 00:00:00 2001 From: cjimti Date: Wed, 13 May 2026 10:36:01 -0700 Subject: [PATCH] server,auth/inbound: accept OIDC JWTs on /v1/* via inbound chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inbound chain previously held only api-key + static-bearer authenticators, so an Authorization: Bearer hitting /v1/* was rejected with 401 even when oidc.enabled was true and the JWT carried the right issuer/audience — the OIDC validator was wired only into the portal session flow. Adds an OIDCBearerAuthenticator adapter that returns ErrNoCredential on non-JWT bearers (so static dev tokens still fall through) and ErrInvalidCredential only on JWTs that fail verification; the server now builds one OIDC validator at startup and shares it between the chain and BrowserAuth. Fixes #10. --- internal/server/server.go | 41 +++-- pkg/auth/inbound/oidc.go | 99 ++++++++++++ pkg/auth/inbound/oidc_test.go | 291 ++++++++++++++++++++++++++++++++++ 3 files changed, 420 insertions(+), 11 deletions(-) create mode 100644 pkg/auth/inbound/oidc.go create mode 100644 pkg/auth/inbound/oidc_test.go diff --git a/internal/server/server.go b/internal/server/server.go index b78d00a..4c8e517 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -99,7 +99,28 @@ func Build(ctx context.Context, cfg *config.Config, logger *slog.Logger) (*Appli } apikeyAuth := inbound.NewAPIKey(keyStore, cfg.APIKeys.HeaderName, cfg.APIKeys.QueryParamName) bearerAuth := inbound.NewBearer(cfg.Bearer.Tokens) - app.chain = inbound.NewChain(cfg.Auth.AllowAnonymous, apikeyAuth, bearerAuth) + + // The OIDC validator is shared with the portal's BrowserAuth so we hit + // discovery + JWKS once. When portal is disabled but oidc.enabled is + // true the validator still belongs in the chain so /v1/* accepts JWTs. + var oidcValidator *auth.OIDCAuthenticator + if cfg.OIDC.Enabled { + v, err := auth.NewOIDC(ctx, cfg.OIDC) + if err != nil { + return nil, fmt.Errorf("oidc validator: %w", err) + } + oidcValidator = v + } + + auths := []inbound.Authenticator{apikeyAuth} + if oidcValidator != nil { + // Insert OIDC before the static bearer so JWTs hit the IdP-aware + // validator first. A non-JWT bearer returns ErrNoCredential and + // falls through to the static list. + auths = append(auths, inbound.NewOIDCBearer(oidcValidator)) + } + auths = append(auths, bearerAuth) + app.chain = inbound.NewChain(cfg.Auth.AllowAnonymous, auths...) // --- Endpoint registry --- app.registry = buildRegistry(cfg) @@ -119,7 +140,7 @@ func Build(ctx context.Context, cfg *config.Config, logger *slog.Logger) (*Appli app.readiness = httpsrv.NewReadiness() // --- Portal (M3+) --- - portalDeps, err := buildPortal(ctx, cfg, app.chain, app.auditLog, app.registry, app.dbKeys, logger) + portalDeps, err := buildPortal(ctx, cfg, app.chain, app.auditLog, app.registry, app.dbKeys, oidcValidator, logger) if err != nil { return nil, fmt.Errorf("portal: %w", err) } @@ -160,9 +181,10 @@ func buildOpenAPI(cfg *config.Config, registry *endpoints.Registry) oapi.Documen // true. Returns (nil, nil) when the portal is disabled — the mux falls back // to the bare /v1/* + /healthz surface. // -// The OIDC validator + BrowserAuth construction will hit the configured -// issuer's discovery URL at startup; misconfiguration (wrong issuer, IdP -// down) fails Build() rather than the first portal request. +// oidcValidator is the same instance the inbound chain uses; the portal +// reuses it rather than running discovery + JWKS fetch a second time. When +// cfg.OIDC.Enabled is false the validator is nil and BrowserAuth is not +// mounted. func buildPortal( ctx context.Context, cfg *config.Config, @@ -170,6 +192,7 @@ func buildPortal( auditLog audit.Logger, registry *endpoints.Registry, keys *apikeys.Store, + oidcValidator *auth.OIDCAuthenticator, logger *slog.Logger, ) (*httpsrv.PortalDeps, error) { if !cfg.Portal.Enabled { @@ -189,12 +212,8 @@ func buildPortal( PortalAuth: httpsrv.NewPortalAuth(sessions, chain), PortalAPI: httpsrv.NewPortalAPI(cfg, registry, auditLog, keys), } - if cfg.OIDC.Enabled { - validator, err := auth.NewOIDC(ctx, cfg.OIDC) - if err != nil { - return nil, fmt.Errorf("oidc validator: %w", err) - } - ba, err := httpsrv.NewBrowserAuth(ctx, cfg, validator, sessions, logger) + if oidcValidator != nil { + ba, err := httpsrv.NewBrowserAuth(ctx, cfg, oidcValidator, sessions, logger) if err != nil { return nil, fmt.Errorf("browser auth: %w", err) } diff --git a/pkg/auth/inbound/oidc.go b/pkg/auth/inbound/oidc.go new file mode 100644 index 0000000..3e0d68a --- /dev/null +++ b/pkg/auth/inbound/oidc.go @@ -0,0 +1,99 @@ +package inbound + +import ( + "context" + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "strings" + + "github.com/plexara/api-test/pkg/auth" +) + +// OIDCValidator is the subset of pkg/auth.OIDCAuthenticator the inbound +// adapter needs. Defined as an interface so tests can stub validation +// without standing up a fake IdP. +type OIDCValidator interface { + ValidateBearer(ctx context.Context, token string) (*auth.Identity, error) +} + +// OIDCBearerAuthenticator adapts an OIDC JWT validator to the inbound +// chain. It guards the /v1/* surface so callers can authenticate with +// `Authorization: Bearer ` issued by the configured IdP. +// +// Chain semantics: +// - No Authorization header / non-Bearer scheme → ErrNoCredential. +// - Bearer value that is not a structurally-valid JWT → ErrNoCredential. +// This lets a static dev token fall through to BearerAuthenticator +// when both are registered in the chain. +// - Structurally-valid JWT that fails signature/issuer/audience/expiry +// checks → ErrInvalidCredential (no fallthrough — a real JWT that +// fails verification must 401, not be retried as a static bearer). +type OIDCBearerAuthenticator struct { + validator OIDCValidator +} + +// NewOIDCBearer returns an OIDCBearerAuthenticator wrapping v. A nil +// validator yields an authenticator that always returns ErrNoCredential, +// so callers can register the adapter unconditionally. +func NewOIDCBearer(v OIDCValidator) *OIDCBearerAuthenticator { + return &OIDCBearerAuthenticator{validator: v} +} + +// Authenticate implements Authenticator. +func (a *OIDCBearerAuthenticator) Authenticate(ctx context.Context, r *http.Request) (*Identity, error) { + if a == nil || a.validator == nil { + return nil, ErrNoCredential + } + token := extractBearer(r.Header.Get("Authorization")) + if token == "" { + return nil, ErrNoCredential + } + if !looksLikeJWT(token) { + return nil, ErrNoCredential + } + aid, err := a.validator.ValidateBearer(ctx, token) + if err != nil { + // Wrap both the sentinel (so callers can errors.Is for chain + // semantics) and the underlying verifier error (so operators see + // why the JWT was rejected in logs). + return nil, fmt.Errorf("oidc: %w: %w", ErrInvalidCredential, err) + } + subject := aid.Subject + if subject == "" { + // ValidateBearer already falls back to preferred_username; this is + // just a safety net so an audit row never has an empty subject. + subject = "oidc-subject-missing" + } + return &Identity{ + Subject: subject, + Email: aid.Email, + AuthType: "oidc", + KeyName: subject, + Claims: aid.Claims, + }, nil +} + +// looksLikeJWT is a cheap structural check: three dot-separated segments +// where the first segment base64url-decodes to JSON carrying a non-empty +// "alg" header. We gate the IdP-aware validator behind this so a static +// dev token like "abc123" returns ErrNoCredential and the chain falls +// through to BearerAuthenticator. +func looksLikeJWT(s string) bool { + parts := strings.Split(s, ".") + if len(parts) != 3 { + return false + } + header, err := base64.RawURLEncoding.DecodeString(parts[0]) + if err != nil { + return false + } + var h struct { + Alg string `json:"alg"` + } + if err := json.Unmarshal(header, &h); err != nil { + return false + } + return h.Alg != "" +} diff --git a/pkg/auth/inbound/oidc_test.go b/pkg/auth/inbound/oidc_test.go new file mode 100644 index 0000000..53af010 --- /dev/null +++ b/pkg/auth/inbound/oidc_test.go @@ -0,0 +1,291 @@ +package inbound + +import ( + "context" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/plexara/api-test/pkg/auth" + "github.com/plexara/api-test/pkg/config" +) + +// stubValidator is a hand-rolled OIDCValidator: tokens whose payload +// segment decodes to the configured "iss" / "aud" pair succeed; anything +// else returns the configured error. The point is to exercise the +// adapter's chain semantics, not the real JWT verifier (which has its own +// tests in pkg/auth). +type stubValidator struct { + wantIss string + wantAud string +} + +func (s stubValidator) ValidateBearer(_ context.Context, token string) (*auth.Identity, error) { + parts := splitDots(token) + if len(parts) != 3 { + return nil, errors.New("not a jwt") + } + payload, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + return nil, fmt.Errorf("decode payload: %w", err) + } + var claims map[string]any + if err := json.Unmarshal(payload, &claims); err != nil { + return nil, fmt.Errorf("decode claims: %w", err) + } + if iss, _ := claims["iss"].(string); iss != s.wantIss { + return nil, fmt.Errorf("issuer mismatch: got %q want %q", iss, s.wantIss) + } + if aud, _ := claims["aud"].(string); aud != s.wantAud { + return nil, fmt.Errorf("audience mismatch: got %q want %q", aud, s.wantAud) + } + sub, _ := claims["sub"].(string) + email, _ := claims["email"].(string) + return &auth.Identity{ + Subject: sub, + Email: email, + AuthType: "oidc", + Claims: claims, + }, nil +} + +func splitDots(s string) []string { + out := []string{""} + for _, c := range s { + if c == '.' { + out = append(out, "") + continue + } + out[len(out)-1] += string(c) + } + return out +} + +// makeJWT crafts a structurally-valid JWT-shaped string (header.payload.sig) +// from the supplied claims. The signature is opaque since stubValidator +// does not verify it. +func makeJWT(t *testing.T, claims map[string]any) string { + t.Helper() + header, err := json.Marshal(map[string]any{"alg": "RS256", "typ": "JWT", "kid": "k1"}) + if err != nil { + t.Fatalf("marshal header: %v", err) + } + payload, err := json.Marshal(claims) + if err != nil { + t.Fatalf("marshal payload: %v", err) + } + return base64.RawURLEncoding.EncodeToString(header) + "." + + base64.RawURLEncoding.EncodeToString(payload) + "." + + base64.RawURLEncoding.EncodeToString([]byte("sig")) +} + +func TestLooksLikeJWT(t *testing.T) { + t.Run("valid shape", func(t *testing.T) { + tok := makeJWT(t, map[string]any{"sub": "x"}) + if !looksLikeJWT(tok) { + t.Error("crafted jwt should be detected") + } + }) + t.Run("static dev token", func(t *testing.T) { + if looksLikeJWT("abc123") { + t.Error("plain bearer token is not a jwt") + } + }) + t.Run("two segments", func(t *testing.T) { + if looksLikeJWT("a.b") { + t.Error("two-segment value is not a jwt") + } + }) + t.Run("header missing alg", func(t *testing.T) { + header := base64.RawURLEncoding.EncodeToString([]byte(`{"typ":"JWT"}`)) + tok := header + ".aGVsbG8.c2ln" + if looksLikeJWT(tok) { + t.Error("header without alg is not a usable jwt") + } + }) + t.Run("non-base64 header", func(t *testing.T) { + if looksLikeJWT("@@@.b.c") { + t.Error("non-base64 header is not a jwt") + } + }) +} + +func TestOIDCBearer_NoCredentialPaths(t *testing.T) { + a := NewOIDCBearer(stubValidator{wantIss: "https://idp", wantAud: "api-test"}) + + t.Run("no authorization header", func(t *testing.T) { + r := httptest.NewRequest(http.MethodGet, "/v1/whoami", nil) + if _, err := a.Authenticate(context.Background(), r); !errors.Is(err, ErrNoCredential) { + t.Errorf("err=%v want ErrNoCredential", err) + } + }) + t.Run("non-bearer scheme", func(t *testing.T) { + r := httptest.NewRequest(http.MethodGet, "/v1/whoami", nil) + r.Header.Set("Authorization", "Basic dXNlcjpwYXNz") + if _, err := a.Authenticate(context.Background(), r); !errors.Is(err, ErrNoCredential) { + t.Errorf("err=%v want ErrNoCredential", err) + } + }) + t.Run("bearer but not a jwt (static dev token)", func(t *testing.T) { + // This is the regression the ticket calls out: when a static dev + // token reaches the OIDC adapter it must yield to the next + // authenticator instead of failing the chain. + r := httptest.NewRequest(http.MethodGet, "/v1/whoami", nil) + r.Header.Set("Authorization", "Bearer abc123") + if _, err := a.Authenticate(context.Background(), r); !errors.Is(err, ErrNoCredential) { + t.Errorf("err=%v want ErrNoCredential", err) + } + }) +} + +func TestOIDCBearer_ValidJWT(t *testing.T) { + a := NewOIDCBearer(stubValidator{wantIss: "https://idp", wantAud: "api-test"}) + tok := makeJWT(t, map[string]any{ + "iss": "https://idp", + "aud": "api-test", + "sub": "alice", + "email": "alice@example.com", + }) + r := httptest.NewRequest(http.MethodGet, "/v1/whoami", nil) + r.Header.Set("Authorization", "Bearer "+tok) + id, err := a.Authenticate(context.Background(), r) + if err != nil { + t.Fatalf("Authenticate: %v", err) + } + if id.Subject != "alice" || id.Email != "alice@example.com" || id.AuthType != "oidc" || id.KeyName != "alice" { + t.Errorf("identity = %+v", id) + } + if id.Claims["iss"] != "https://idp" { + t.Errorf("claims = %+v want iss=https://idp", id.Claims) + } +} + +func TestOIDCBearer_ForeignIssuer(t *testing.T) { + // A real JWT that didn't come from our IdP must 401, not fall through + // to BearerAuthenticator — otherwise a foreign-issued JWT could be + // silently retried as a static bearer. + a := NewOIDCBearer(stubValidator{wantIss: "https://idp", wantAud: "api-test"}) + tok := makeJWT(t, map[string]any{ + "iss": "https://attacker.example", + "aud": "api-test", + "sub": "mallory", + }) + r := httptest.NewRequest(http.MethodGet, "/v1/whoami", nil) + r.Header.Set("Authorization", "Bearer "+tok) + if _, err := a.Authenticate(context.Background(), r); !errors.Is(err, ErrInvalidCredential) { + t.Errorf("err=%v want ErrInvalidCredential", err) + } +} + +func TestOIDCBearer_NilValidator(t *testing.T) { + // Allow callers to register the adapter unconditionally even when OIDC + // is disabled. Treat nil as "no opinion" rather than panicking. + a := NewOIDCBearer(nil) + r := httptest.NewRequest(http.MethodGet, "/v1/whoami", nil) + r.Header.Set("Authorization", "Bearer "+makeJWT(t, map[string]any{"sub": "x"})) + if _, err := a.Authenticate(context.Background(), r); !errors.Is(err, ErrNoCredential) { + t.Errorf("err=%v want ErrNoCredential", err) + } +} + +// TestChain_OIDCBeforeBearer is the end-to-end composition test the ticket +// asks for: register apikey + oidc + bearer and assert each credential type +// resolves to the right authenticator without the others stealing it. +func TestChain_OIDCBeforeBearer(t *testing.T) { + apikeys := NewFileAPIKeyStore([]config.FileAPIKey{{Name: "ops", Key: "K"}}) + apikeyAuth := NewAPIKey(apikeys, "X-API-Key", "api_key") + oidcAuth := NewOIDCBearer(stubValidator{wantIss: "https://idp", wantAud: "api-test"}) + bearerAuth := NewBearer([]config.FileBearerToken{{Name: "dev", Token: "DEVTOKEN"}}) + + chain := NewChain(false, apikeyAuth, oidcAuth, bearerAuth) + + cases := []struct { + name string + req func() *http.Request + wantSubject string + wantType string + wantErr error + }{ + { + name: "valid jwt resolves via oidc", + req: func() *http.Request { + tok := makeJWT(t, map[string]any{ + "iss": "https://idp", + "aud": "api-test", + "sub": "alice", + }) + r := httptest.NewRequest(http.MethodGet, "/v1/whoami", nil) + r.Header.Set("Authorization", "Bearer "+tok) + return r + }, + wantSubject: "alice", + wantType: "oidc", + }, + { + name: "foreign-issuer jwt aborts at oidc (no bearer fallthrough)", + req: func() *http.Request { + tok := makeJWT(t, map[string]any{ + "iss": "https://attacker.example", + "aud": "api-test", + "sub": "mallory", + }) + r := httptest.NewRequest(http.MethodGet, "/v1/whoami", nil) + r.Header.Set("Authorization", "Bearer "+tok) + return r + }, + wantErr: ErrInvalidCredential, + }, + { + name: "static dev bearer falls through oidc to bearer", + req: func() *http.Request { + r := httptest.NewRequest(http.MethodGet, "/v1/whoami", nil) + r.Header.Set("Authorization", "Bearer DEVTOKEN") + return r + }, + wantSubject: "dev", + wantType: "bearer", + }, + { + name: "api key still works alongside oidc", + req: func() *http.Request { + r := httptest.NewRequest(http.MethodGet, "/v1/whoami", nil) + r.Header.Set("X-API-Key", "K") + return r + }, + wantSubject: "ops", + wantType: "apikey", + }, + { + name: "no credential and anonymous disabled", + req: func() *http.Request { + return httptest.NewRequest(http.MethodGet, "/v1/whoami", nil) + }, + wantErr: ErrNoCredential, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + id, err := chain.Authenticate(context.Background(), tc.req()) + if tc.wantErr != nil { + if !errors.Is(err, tc.wantErr) { + t.Fatalf("err=%v want %v", err, tc.wantErr) + } + return + } + if err != nil { + t.Fatalf("Authenticate: %v", err) + } + if id.Subject != tc.wantSubject { + t.Errorf("subject=%q want %q", id.Subject, tc.wantSubject) + } + if id.AuthType != tc.wantType { + t.Errorf("authType=%q want %q", id.AuthType, tc.wantType) + } + }) + } +}