diff --git a/go.mod b/go.mod index 8fd3bfa3c..7425aa134 100644 --- a/go.mod +++ b/go.mod @@ -103,6 +103,8 @@ require ( github.com/aws/aws-sdk-go-v2/service/sesv2 v1.42.0 github.com/aws/aws-sdk-go-v2/service/sns v1.34.0 github.com/aws/aws-sdk-go-v2/service/sts v1.26.6 + github.com/coreos/go-oidc/v3 v3.18.0 + github.com/go-jose/go-jose/v4 v4.1.4 github.com/golang-migrate/migrate/v4 v4.19.1 github.com/google/uuid v1.6.0 github.com/jackc/pgx/v5 v5.8.0 @@ -148,7 +150,6 @@ require ( github.com/ebitengine/purego v0.8.4 // indirect github.com/envoyproxy/go-control-plane/envoy v1.36.0 // indirect github.com/envoyproxy/protoc-gen-validate v1.3.0 // indirect - github.com/go-jose/go-jose/v4 v4.1.3 // indirect github.com/go-ole/go-ole v1.2.6 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.6 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect diff --git a/go.sum b/go.sum index 09f5bcae7..41a86df56 100644 --- a/go.sum +++ b/go.sum @@ -174,6 +174,8 @@ github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I= github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3EhrzVo= github.com/containerd/platforms v0.2.1 h1:zvwtM3rz2YHPQsF2CHYM8+KtB5dvhISiXh5ZpSBQv6A= github.com/containerd/platforms v0.2.1/go.mod h1:XHCb+2/hzowdiut9rkudds9bE5yJ7npe7dG/wG+uFPw= +github.com/coreos/go-oidc/v3 v3.18.0 h1:V9orjXynvu5wiC9SemFTWnG4F45v403aIcjWo0d41+A= +github.com/coreos/go-oidc/v3 v3.18.0/go.mod h1:DYCf24+ncYi+XkIH97GY1+dqoRlbaSI26KVTCI9SrY4= github.com/cpuguy83/dockercfg v0.3.2 h1:DlJTyZGBDlXqUZ2Dk2Q3xHs/FtnooJJVaad2S9GKorA= github.com/cpuguy83/dockercfg v0.3.2/go.mod h1:sugsbF4//dDlL/i+S+rtpIWp+5h0BHJHfjj5/jFyUJc= github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= @@ -207,8 +209,8 @@ github.com/envoyproxy/protoc-gen-validate v1.3.0 h1:TvGH1wof4H33rezVKWSpqKz5NXWg github.com/envoyproxy/protoc-gen-validate v1.3.0/go.mod h1:HvYl7zwPa5mffgyeTUHA9zHIH36nmrm7oCbo4YKoSWA= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= -github.com/go-jose/go-jose/v4 v4.1.3 h1:CVLmWDhDVRa6Mi/IgCgaopNosCaHz7zrMeF9MlZRkrs= -github.com/go-jose/go-jose/v4 v4.1.3/go.mod h1:x4oUasVrzR7071A4TnHLGSPpNOm2a21K9Kf04k1rs08= +github.com/go-jose/go-jose/v4 v4.1.4 h1:moDMcTHmvE6Groj34emNPLs/qtYXRVcd6S7NHbHz3kA= +github.com/go-jose/go-jose/v4 v4.1.4/go.mod h1:x4oUasVrzR7071A4TnHLGSPpNOm2a21K9Kf04k1rs08= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= diff --git a/internal/server/app.go b/internal/server/app.go index 14a2dca32..b132d3177 100644 --- a/internal/server/app.go +++ b/internal/server/app.go @@ -27,6 +27,7 @@ import ( "github.com/LeanerCloud/CUDly/internal/runtime" "github.com/LeanerCloud/CUDly/internal/scheduler" "github.com/LeanerCloud/CUDly/internal/secrets" + "github.com/LeanerCloud/CUDly/internal/server/scheduledauth" "github.com/LeanerCloud/CUDly/pkg/logging" "github.com/aws/aws-sdk-go-v2/aws" awsconfig "github.com/aws/aws-sdk-go-v2/config" @@ -77,6 +78,17 @@ type Application struct { // federated credential path). Nil when the deployment has not // opted into the federated flow. signer oidc.Signer + + // scheduledAuth authenticates inbound /api/scheduled/* requests. + // Non-nil after NewApplicationFromDeps — disabled mode allows + // everything through with a loud WARN log; oidc mode (GCP) + // validates the Cloud Scheduler-signed ID token; bearer mode + // (Azure) constant-time-compares the shared secret resolved at + // startup from Key Vault. May be nil in tests that build + // Application directly without the constructor; the + // scheduledAuthMiddleware helper passes through unmodified in + // that case so handler-only tests stay focused. + scheduledAuth *scheduledauth.Validator } // ApplicationConfig holds all env-based configuration for the application @@ -267,6 +279,35 @@ func resolveScheduledTaskSecret(ctx context.Context, cfg ApplicationConfig, reso return resolved } +// envSourceOS implements scheduledauth.EnvSource against os.Getenv. The +// scheduledauth package owns the env var names and parse rules so they +// stay aligned with Terraform. +type envSourceOS struct{} + +func (envSourceOS) Get(key string) string { return os.Getenv(key) } + +// buildScheduledAuth wires up the /api/scheduled/* validator. It pulls +// mode + OIDC params from env (they're per-deployment Terraform inputs) +// but injects the bearer secret from cfg — that path goes through the +// SecretResolver (Key Vault) for production deployments and never lives +// in a container env var. Returns ErrConfigInvalid on misconfig so the +// container fails fast rather than silently accepting everything. +func buildScheduledAuth(cfg ApplicationConfig) (*scheduledauth.Validator, error) { + saCfg, err := scheduledauth.LoadConfig(envSourceOS{}) + if err != nil { + return nil, err + } + // In bearer mode, override the env-supplied secret with the one + // already resolved from KV / SM. LoadConfig reads SCHEDULED_TASK_SECRET + // directly which is fine for local dev where the env carries the + // plaintext, but in prod cfg.ScheduledTaskSecret is the authoritative + // value (resolved by resolveScheduledTaskSecret upstream). + if saCfg.Mode == scheduledauth.ModeBearer { + saCfg.Bearer = cfg.ScheduledTaskSecret + } + return scheduledauth.New(saCfg) +} + // NewApplicationFromDeps creates an Application from pre-built configuration and dependencies. // This is the testable constructor - all external I/O is done before calling this. func NewApplicationFromDeps(ctx context.Context, cfg ApplicationConfig, deps ExternalDeps) (*Application, error) { @@ -276,6 +317,20 @@ func NewApplicationFromDeps(ctx context.Context, cfg ApplicationConfig, deps Ext cfg.ScheduledTaskSecret = resolveScheduledTaskSecret(ctx, cfg, deps.SecretResolver) + // Build the /api/scheduled/* auth validator. Fail-fast on bad + // config (empty subjects in oidc mode, unknown mode, etc.) — better + // to crash on startup than to silently accept unauthenticated + // scheduled-task calls in production. + scheduledAuth, err := buildScheduledAuth(cfg) + if err != nil { + return nil, fmt.Errorf("scheduled-task auth init: %w", err) + } + // Best-effort JWKS warmup — surfaces misconfiguration in startup + // logs without blocking startup if Google's CDN is unreachable. + warmCtx, warmCancel := context.WithTimeout(ctx, 5*time.Second) + scheduledAuth.Warmup(warmCtx) + warmCancel() + // Construct the OIDC issuer signer once per deployment. Nil means // the deployment has not opted into the federated flow yet — all // OIDC-dependent paths (handler_oidc.go, purchase manager Azure @@ -381,6 +436,7 @@ func NewApplicationFromDeps(ctx context.Context, cfg ApplicationConfig, deps Ext secretResolver: deps.SecretResolver, appConfig: cfg, signer: signer, + scheduledAuth: scheduledAuth, }, nil } diff --git a/internal/server/app_test.go b/internal/server/app_test.go index a1311b7af..4de388bca 100644 --- a/internal/server/app_test.go +++ b/internal/server/app_test.go @@ -368,6 +368,12 @@ func TestLoadApplicationConfig(t *testing.T) { func TestNewApplicationFromDeps(t *testing.T) { ctx := testutil.TestContext(t) + // SCHEDULED_TASK_AUTH_MODE is required at startup (fail-closed default + // per CR pass-4 review on PR #161). Tests boot the app without + // scheduled-task wiring, so explicitly opt into "disabled" — the same + // thing local-dev tfvars do. + t.Setenv("SCHEDULED_TASK_AUTH_MODE", "disabled") + validDBConfig := &database.Config{ Host: "localhost", Port: 5432, diff --git a/internal/server/http.go b/internal/server/http.go index 6046401a6..9bf6bc232 100644 --- a/internal/server/http.go +++ b/internal/server/http.go @@ -2,7 +2,6 @@ package server import ( "context" - "crypto/subtle" "encoding/base64" "encoding/json" "fmt" @@ -20,9 +19,13 @@ import ( func CreateHTTPServer(app *Application, port int) *http.Server { mux := http.NewServeMux() - // Register health and scheduled task routes (always available) + // Register health and scheduled task routes (always available). + // /api/scheduled/* sits behind the scheduledauth middleware — see + // internal/server/scheduledauth. Mode is selected by + // SCHEDULED_TASK_AUTH_MODE: oidc on GCP, bearer on Azure, disabled + // for local dev. mux.HandleFunc("/health", app.handleHealthCheck) - mux.HandleFunc("/api/scheduled/", app.handleScheduledHTTP) + mux.Handle("/api/scheduled/", app.scheduledAuthMiddleware(http.HandlerFunc(app.handleScheduledHTTP))) // When STATIC_DIR is set, serve static files for non-API paths // and route only /api/ to the API handler. @@ -105,24 +108,29 @@ func (app *Application) handleHTTPRequest(w http.ResponseWriter, r *http.Request lambdaResponseToHTTP(w, lambdaResp) } +// scheduledAuthMiddleware returns the configured scheduledauth middleware, +// or a passthrough no-op if app.scheduledAuth is nil. Tests that build +// an Application directly (without NewApplicationFromDeps) leave it +// nil — they exercise the handler in isolation, with the middleware +// covered separately in scheduledauth's own tests. +func (app *Application) scheduledAuthMiddleware(next http.Handler) http.Handler { + if app.scheduledAuth == nil { + return next + } + return app.scheduledAuth.Middleware(next) +} + // handleScheduledHTTP handles scheduled tasks via HTTP endpoint -// This is used by GCP Cloud Scheduler and Azure Logic Apps +// This is used by GCP Cloud Scheduler and Azure Logic Apps. Auth is +// enforced upstream by scheduledAuthMiddleware (see CreateHTTPServer); +// by the time we reach here the request is already authenticated for +// the configured mode. func (app *Application) handleScheduledHTTP(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPost { http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) return } - // Verify shared secret for scheduled task authentication - if secret := app.appConfig.ScheduledTaskSecret; secret != "" { - provided := r.Header.Get("Authorization") - expected := "Bearer " + secret - if subtle.ConstantTimeCompare([]byte(provided), []byte(expected)) != 1 { - http.Error(w, "Unauthorized", http.StatusUnauthorized) - return - } - } - ctx := r.Context() // Ensure database connection is established (lazy initialization) diff --git a/internal/server/http_test.go b/internal/server/http_test.go index 3df54ba5a..b13e5da9a 100644 --- a/internal/server/http_test.go +++ b/internal/server/http_test.go @@ -12,6 +12,7 @@ import ( "github.com/LeanerCloud/CUDly/internal/api" "github.com/LeanerCloud/CUDly/internal/scheduler" + "github.com/LeanerCloud/CUDly/internal/server/scheduledauth" "github.com/LeanerCloud/CUDly/internal/testutil" "github.com/aws/aws-lambda-go/events" ) @@ -216,12 +217,25 @@ func TestLambdaResponseToHTTP(t *testing.T) { } func TestHandleScheduledHTTP(t *testing.T) { + // newBearerValidator constructs a bearer-mode validator and fails + // the test on construction error — preferred over `panic` because + // failures stay scoped to the offending case. + newBearerValidator := func(t *testing.T, secret string) *scheduledauth.Validator { + t.Helper() + v, err := scheduledauth.New(scheduledauth.Config{ + Mode: scheduledauth.ModeBearer, + Bearer: secret, + }) + testutil.AssertNoError(t, err) + return v + } + tests := []struct { name string method string path string authHeader string - setupApp func(*Application) + setupApp func(*testing.T, *Application) expectedStatus int expectError bool }{ @@ -229,7 +243,7 @@ func TestHandleScheduledHTTP(t *testing.T) { name: "valid scheduled task", method: "POST", path: "/api/scheduled/collect_recommendations", - setupApp: func(app *Application) { + setupApp: func(_ *testing.T, app *Application) { app.Scheduler = &testutil.MockScheduler{ CollectRecommendationsFunc: func(ctx context.Context) (*scheduler.CollectResult, error) { return &scheduler.CollectResult{ @@ -246,7 +260,7 @@ func TestHandleScheduledHTTP(t *testing.T) { name: "invalid method (GET instead of POST)", method: "GET", path: "/api/scheduled/collect_recommendations", - setupApp: func(app *Application) {}, + setupApp: func(_ *testing.T, _ *Application) {}, expectedStatus: 405, expectError: false, }, @@ -254,7 +268,7 @@ func TestHandleScheduledHTTP(t *testing.T) { name: "invalid path (missing task type)", method: "POST", path: "/api/scheduled/", - setupApp: func(app *Application) {}, + setupApp: func(_ *testing.T, _ *Application) {}, expectedStatus: 400, expectError: false, }, @@ -262,8 +276,8 @@ func TestHandleScheduledHTTP(t *testing.T) { name: "auth required but missing", method: "POST", path: "/api/scheduled/collect_recommendations", - setupApp: func(app *Application) { - app.appConfig.ScheduledTaskSecret = "my-secret" + setupApp: func(t *testing.T, app *Application) { + app.scheduledAuth = newBearerValidator(t, "my-secret") }, expectedStatus: 401, }, @@ -272,8 +286,8 @@ func TestHandleScheduledHTTP(t *testing.T) { method: "POST", path: "/api/scheduled/collect_recommendations", authHeader: "Bearer my-secret", - setupApp: func(app *Application) { - app.appConfig.ScheduledTaskSecret = "my-secret" + setupApp: func(t *testing.T, app *Application) { + app.scheduledAuth = newBearerValidator(t, "my-secret") app.Scheduler = &testutil.MockScheduler{ CollectRecommendationsFunc: func(ctx context.Context) (*scheduler.CollectResult, error) { return &scheduler.CollectResult{}, nil @@ -288,7 +302,7 @@ func TestHandleScheduledHTTP(t *testing.T) { t.Run(tt.name, func(t *testing.T) { app := &Application{} if tt.setupApp != nil { - tt.setupApp(app) + tt.setupApp(t, app) } req := httptest.NewRequest(tt.method, tt.path, nil) @@ -297,7 +311,10 @@ func TestHandleScheduledHTTP(t *testing.T) { } w := httptest.NewRecorder() - app.handleScheduledHTTP(w, req) + // Drive the request through the same middleware chain + // that CreateHTTPServer wires up, so auth is enforced + // upstream of handleScheduledHTTP. + app.scheduledAuthMiddleware(http.HandlerFunc(app.handleScheduledHTTP)).ServeHTTP(w, req) testutil.AssertEqual(t, tt.expectedStatus, w.Code) diff --git a/internal/server/scheduledauth/config.go b/internal/server/scheduledauth/config.go new file mode 100644 index 000000000..242f52b3d --- /dev/null +++ b/internal/server/scheduledauth/config.go @@ -0,0 +1,85 @@ +package scheduledauth + +import ( + "fmt" + "strings" +) + +// EnvSource is the minimal interface NewFromEnv depends on. The default +// implementation (envOS) reads from os.Getenv; tests inject a map. +type EnvSource interface { + Get(string) string +} + +// EnvMap is an EnvSource backed by a map. Useful in tests. +type EnvMap map[string]string + +// Get returns the value for key, or "" if absent. +func (m EnvMap) Get(key string) string { return m[key] } + +// Environment variable names. Centralised so tests and Terraform stay +// aligned with the Go code. +const ( + EnvAuthMode = "SCHEDULED_TASK_AUTH_MODE" // "oidc" | "bearer" | "disabled" + EnvOIDCIssuer = "SCHEDULED_TASK_OIDC_ISSUER" // default GoogleIssuer + EnvOIDCJWKSURL = "SCHEDULED_TASK_OIDC_JWKS_URL" // default GoogleJWKSURL + EnvOIDCAudience = "SCHEDULED_TASK_OIDC_AUDIENCE" // comma-separated + EnvOIDCSubjects = "SCHEDULED_TASK_OIDC_SUBJECTS" // comma-separated; REQUIRED in oidc mode + EnvBearerSecret = "SCHEDULED_TASK_SECRET" // bearer mode only +) + +// LoadConfig parses Config from env. SCHEDULED_TASK_AUTH_MODE MUST be +// set explicitly to one of "oidc", "bearer", or "disabled" — an unset +// value returns ErrConfigInvalid so a missed Terraform/Kubernetes env +// wiring fails server startup instead of silently booting +// /api/scheduled/* unauthenticated. "disabled" is reserved for an +// explicit local-dev choice and must be opted into deliberately. +func LoadConfig(env EnvSource) (Config, error) { + mode := strings.ToLower(strings.TrimSpace(env.Get(EnvAuthMode))) + if mode == "" { + return Config{}, fmt.Errorf("%w: %s is unset (set to oidc, bearer, or disabled)", + ErrConfigInvalid, EnvAuthMode) + } + + cfg := Config{Mode: Mode(mode)} + + switch cfg.Mode { + case ModeOIDC: + cfg.Issuer = strings.TrimSpace(env.Get(EnvOIDCIssuer)) + cfg.JWKSURL = strings.TrimSpace(env.Get(EnvOIDCJWKSURL)) + cfg.Audiences = splitCSV(env.Get(EnvOIDCAudience)) + cfg.Subjects = splitCSV(env.Get(EnvOIDCSubjects)) + + case ModeBearer: + cfg.Bearer = env.Get(EnvBearerSecret) + // Permit other env vars to be set without complaint — the + // terraform module ships SCHEDULED_TASK_OIDC_* across all + // platforms and they should be ignored on Azure. + + case ModeDisabled: + // Nothing to parse. + + default: + return Config{}, fmt.Errorf("%w: %s=%q (expected oidc, bearer, or disabled)", + ErrConfigInvalid, EnvAuthMode, mode) + } + + return cfg, nil +} + +// splitCSV splits a comma-separated value into a trimmed list, dropping +// empty entries (so trailing commas / whitespace are tolerated). +func splitCSV(raw string) []string { + if raw == "" { + return nil + } + parts := strings.Split(raw, ",") + out := make([]string, 0, len(parts)) + for _, p := range parts { + p = strings.TrimSpace(p) + if p != "" { + out = append(out, p) + } + } + return out +} diff --git a/internal/server/scheduledauth/errors.go b/internal/server/scheduledauth/errors.go new file mode 100644 index 000000000..db28d96a1 --- /dev/null +++ b/internal/server/scheduledauth/errors.go @@ -0,0 +1,29 @@ +// Package scheduledauth provides authentication for the /api/scheduled/* +// endpoints invoked by Cloud Scheduler / Logic Apps. Two modes are +// supported: +// +// - oidc: Google-issued OIDC ID tokens (RS256, JWKS-validated). Used +// by GCP Cloud Run and GKE deployments where Cloud Scheduler signs an +// OIDC token with a service-account audience. Subject pinning to the +// scheduler service account email(s) is REQUIRED — the audience alone +// is not sufficient if other tenants in the same GCP org could mint +// tokens with the same `aud` value. +// +// - bearer: shared-secret HMAC-style bearer (constant-time compare). +// Used by Azure Container Apps + Logic Apps where the workflow +// pulls the secret from Key Vault at invocation time. +// +// Design context: GitHub PR #161 / issue #159. +package scheduledauth + +import "errors" + +// ErrConfigInvalid signals a fatal configuration parse error. Callers are +// expected to fail-fast on this — the deploy is misconfigured and the +// scheduled endpoint would silently accept or reject everything. +var ErrConfigInvalid = errors.New("scheduledauth: invalid configuration") + +// ErrUnauthorized is the validation failure returned by Validate. The +// detailed reason is wrapped via fmt.Errorf so it gets logged but not +// returned to the client (which only sees 401). +var ErrUnauthorized = errors.New("scheduledauth: unauthorized") diff --git a/internal/server/scheduledauth/integration_test.go b/internal/server/scheduledauth/integration_test.go new file mode 100644 index 000000000..7a9c1e143 --- /dev/null +++ b/internal/server/scheduledauth/integration_test.go @@ -0,0 +1,151 @@ +package scheduledauth + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "testing" + "time" +) + +// TestIntegration_FullHTTPRoundtrip drives the validator through an +// actual HTTP server with a mocked JWKS endpoint, exercising the same +// path the running container uses: middleware in front of an inner +// handler, JWKS fetched lazily, JWT verified end-to-end. +// +// The test is split into table-driven sub-cases so each scenario gets +// an independent verdict in `go test` output. It reuses the +// helpers (newTestKey, signToken, jwks, newJWKSServer, baseClaims) from +// validator_test.go since both files share the same test package. +func TestIntegration_FullHTTPRoundtrip(t *testing.T) { + // Cloud Scheduler uses one signing key per Google identity; rotate + // is handled by refresh-on-unknown-kid and covered in unit tests. + signingKey := newTestKey(t, "kid-prod-2026") + otherKey := newTestKey(t, "kid-attacker") + + // JWKS server publishes the prod key only. otherKey is not in the + // JWKS, so tokens signed by it must fail. + jwksSrv := newJWKSServer(t, jwks(signingKey)) + + v, err := New(Config{ + Mode: ModeOIDC, + Issuer: "https://accounts.example.com", + JWKSURL: jwksSrv.URL, + Audiences: []string{"https://cudly.example.com"}, + Subjects: []string{testSchedulerSubject}, + }) + if err != nil { + t.Fatalf("New: %v", err) + } + + // Inner handler the middleware fronts. Returns 200 + body so we can + // confirm the request reached the handler in the success case. + const handlerBody = "scheduled-task-ran" + handlerCalls := 0 + inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + handlerCalls++ + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(handlerBody)) + }) + + // Wire validator middleware + inner handler behind a real HTTP + // server so we exercise net/http header parsing, query routing, + // and the actual JWKS fetch over the wire. + srv := httptest.NewServer(v.Middleware(inner)) + t.Cleanup(srv.Close) + + type tc struct { + name string + buildAuth func(t *testing.T) string + wantStatus int + wantHandlerHit bool + } + + now := time.Now() + cases := []tc{ + { + name: "valid token signed by published key", + buildAuth: func(t *testing.T) string { + return "Bearer " + signToken(t, signingKey, baseClaims(now, + testSchedulerSubject, + "https://cudly.example.com", + "https://accounts.example.com")) + }, + wantStatus: http.StatusOK, + wantHandlerHit: true, + }, + { + name: "token signed by key not in JWKS", + buildAuth: func(t *testing.T) string { + return "Bearer " + signToken(t, otherKey, baseClaims(now, + testSchedulerSubject, + "https://cudly.example.com", + "https://accounts.example.com")) + }, + wantStatus: http.StatusUnauthorized, + wantHandlerHit: false, + }, + { + name: "missing Authorization header", + buildAuth: func(t *testing.T) string { return "" }, + wantStatus: http.StatusUnauthorized, + wantHandlerHit: false, + }, + { + name: "wrong subject (audience matches but sub does not)", + buildAuth: func(t *testing.T) string { + return "Bearer " + signToken(t, signingKey, baseClaims(now, + testNonSchedulerSubject, + "https://cudly.example.com", + "https://accounts.example.com")) + }, + wantStatus: http.StatusUnauthorized, + wantHandlerHit: false, + }, + { + name: "wrong audience (sub matches but aud does not)", + buildAuth: func(t *testing.T) string { + return "Bearer " + signToken(t, signingKey, baseClaims(now, + testSchedulerSubject, + "https://attacker.example.com", + "https://accounts.example.com")) + }, + wantStatus: http.StatusUnauthorized, + wantHandlerHit: false, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + callsBefore := handlerCalls + + req, err := http.NewRequestWithContext(context.Background(), + http.MethodPost, srv.URL+"/api/scheduled/recommendations", nil) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + if authz := tt.buildAuth(t); authz != "" { + req.Header.Set("Authorization", authz) + } + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("client Do: %v", err) + } + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + + if resp.StatusCode != tt.wantStatus { + t.Fatalf("status = %d (want %d), body=%q", resp.StatusCode, tt.wantStatus, string(body)) + } + gotHit := handlerCalls > callsBefore + if gotHit != tt.wantHandlerHit { + t.Fatalf("handler hit = %v, want %v", gotHit, tt.wantHandlerHit) + } + if tt.wantHandlerHit && string(body) != handlerBody { + t.Fatalf("body = %q, want %q", string(body), handlerBody) + } + }) + } +} diff --git a/internal/server/scheduledauth/validator.go b/internal/server/scheduledauth/validator.go new file mode 100644 index 000000000..25a4c7094 --- /dev/null +++ b/internal/server/scheduledauth/validator.go @@ -0,0 +1,441 @@ +package scheduledauth + +import ( + "context" + "crypto/subtle" + "encoding/json" + "errors" + "fmt" + "io" + "log" + "net/http" + "net/url" + "strings" + "time" + + "github.com/coreos/go-oidc/v3/oidc" +) + +// Mode is the authentication mode used for scheduled-task requests. +type Mode string + +const ( + // ModeOIDC verifies a Google-issued OIDC ID token (RS256 only) on + // the Authorization header. Subject pinning is required. + ModeOIDC Mode = "oidc" + + // ModeBearer compares the Authorization header to a shared secret + // using crypto/subtle.ConstantTimeCompare. Used on Azure where Logic + // Apps fetch the secret from Key Vault at runtime. + ModeBearer Mode = "bearer" + + // ModeDisabled disables auth entirely — every request is allowed + // through. Intended for local development only. The validator logs + // a WARN at startup and on every request to make this loud. + ModeDisabled Mode = "disabled" +) + +// DefaultClockSkew is the tolerance applied to exp/nbf checks. Matches +// the leeway most OIDC providers (Microsoft, Auth0) recommend. +const DefaultClockSkew = 60 * time.Second + +// GoogleIssuer is the canonical OIDC issuer URL for Google-signed ID +// tokens (Cloud Scheduler, Cloud Functions, etc.). +const GoogleIssuer = "https://accounts.google.com" + +// GoogleJWKSURL is Google's JWKS endpoint. Used as the default when the +// caller does not override it via SCHEDULED_TASK_OIDC_JWKS_URL. +const GoogleJWKSURL = "https://www.googleapis.com/oauth2/v3/certs" + +// Config holds the parsed configuration for a Validator. NewValidator +// applies defaults and rejects invalid combinations. +type Config struct { + Mode Mode + Issuer string // OIDC issuer; defaults to GoogleIssuer in oidc mode + JWKSURL string // OIDC JWKS endpoint; defaults to GoogleJWKSURL in oidc mode + Audiences []string // accepted aud claims (must be non-empty in oidc mode) + Subjects []string // accepted sub claims (REQUIRED non-empty in oidc mode — defence in depth) + Skew time.Duration + Bearer string // shared secret for bearer mode (must be non-empty) +} + +// Validator authenticates inbound requests against the configured mode. +type Validator struct { + mode Mode + verifier *oidc.IDTokenVerifier // nil unless mode == ModeOIDC + keySet *oidc.RemoteKeySet // nil unless mode == ModeOIDC; powered by go-oidc's single-flight cache + jwksURL string // remembered for Warmup; empty unless mode == ModeOIDC + audiences map[string]struct{} + subjects map[string]struct{} + skew time.Duration + bearer []byte + now func() time.Time // pluggable for tests +} + +// claims captures the timestamp claims we need beyond what go-oidc's +// IDToken exposes directly. Issuer/Subject/Audience are read off the +// IDToken value returned by Verify; we only re-parse the payload to +// pick up exp/iat/nbf as int64 timestamps so we can apply our own +// skew-tolerant checks (go-oidc's built-in expiry check has no skew). +type claims struct { + ExpiresAt int64 `json:"exp"` + IssuedAt int64 `json:"iat"` + NotBefore int64 `json:"nbf"` +} + +// New builds a Validator from a parsed Config. The caller is responsible +// for invoking NewFromEnv (or a similar config loader) to populate +// Config from environment variables. +// +// In oidc mode, the underlying *oidc.RemoteKeySet performs JWKS retrieval +// lazily on the first request and refreshes on unknown kid (per +// https://openid.net/specs/openid-connect-core-1_0.html#RotateSigKeys). +// Single-flight is built in. There is no fixed TTL — refresh-on- +// unknown-kid is sufficient because providers always sign new tokens +// with the new kid before retiring the old key. +// +// To pre-warm the cache and surface JWKS endpoint misconfiguration at +// startup, callers should invoke (*Validator).Warmup. Failure is logged +// but does not abort startup — Google's CDN sometimes hiccups and we +// prefer the validator come up with a stale-on-error fetch on the first +// real request rather than crashloop the container. +func New(cfg Config) (*Validator, error) { + if cfg.Skew <= 0 { + cfg.Skew = DefaultClockSkew + } + + v := &Validator{ + mode: cfg.Mode, + skew: cfg.Skew, + now: time.Now, + } + + switch cfg.Mode { + case ModeOIDC: + return configureOIDC(v, cfg) + case ModeBearer: + return configureBearer(v, cfg) + case ModeDisabled: + // No setup; every request passes (with a per-request WARN log). + log.Printf("scheduledauth: WARN — auth mode is disabled; /api/scheduled/* is unauthenticated. " + + "Set SCHEDULED_TASK_AUTH_MODE=oidc (GCP) or =bearer (Azure) in production.") + return v, nil + default: + return nil, fmt.Errorf("%w: unknown mode %q (expected oidc, bearer, or disabled)", ErrConfigInvalid, cfg.Mode) + } +} + +// configureOIDC validates OIDC config, builds set-lookups, and wires the +// go-oidc RemoteKeySet + IDTokenVerifier. Split out of New to keep the +// per-mode setup below the cyclomatic-complexity gate. +func configureOIDC(v *Validator, cfg Config) (*Validator, error) { + if cfg.Issuer == "" { + cfg.Issuer = GoogleIssuer + } + if cfg.JWKSURL == "" { + cfg.JWKSURL = GoogleJWKSURL + } + if err := validateAbsoluteURL(cfg.JWKSURL, "SCHEDULED_TASK_OIDC_JWKS_URL"); err != nil { + return nil, err + } + if len(cfg.Audiences) == 0 { + return nil, fmt.Errorf("%w: oidc mode requires SCHEDULED_TASK_OIDC_AUDIENCE", ErrConfigInvalid) + } + // Subject pinning is REQUIRED in oidc mode. Any GCP service account + // in the same org can mint OIDC tokens with arbitrary `aud` values + // — we MUST also pin the issuer SA unique_id via `sub`. Google uses + // the service-account unique_id, not the email address, as the `sub` + // claim for Cloud Scheduler-signed ID tokens. That is what + // SCHEDULED_TASK_OIDC_SUBJECTS must contain. + if len(cfg.Subjects) == 0 { + return nil, fmt.Errorf("%w: oidc mode requires SCHEDULED_TASK_OIDC_SUBJECTS (defence in depth)", ErrConfigInvalid) + } + + auds, err := cleanSet(cfg.Audiences, "SCHEDULED_TASK_OIDC_AUDIENCE") + if err != nil { + return nil, err + } + subs, err := cleanSet(cfg.Subjects, "SCHEDULED_TASK_OIDC_SUBJECTS") + if err != nil { + return nil, err + } + + v.audiences = auds + v.subjects = subs + v.jwksURL = cfg.JWKSURL + v.keySet = oidc.NewRemoteKeySet(context.Background(), cfg.JWKSURL) + v.verifier = oidc.NewVerifier(cfg.Issuer, v.keySet, &oidc.Config{ + // Pin to RS256. Google's tokens are RS256; rejecting anything + // else closes the alg=none / alg=HS256 confusion family. + SupportedSigningAlgs: []string{string(oidc.RS256)}, + // We validate audience manually (multiple values supported) + // and expiry manually (skew tolerance). Issuer is checked by + // go-oidc. + SkipClientIDCheck: true, + SkipExpiryCheck: true, + }) + return v, nil +} + +func configureBearer(v *Validator, cfg Config) (*Validator, error) { + if cfg.Bearer == "" { + return nil, fmt.Errorf("%w: bearer mode requires SCHEDULED_TASK_SECRET", ErrConfigInvalid) + } + v.bearer = []byte("Bearer " + cfg.Bearer) + return v, nil +} + +// cleanSet trims and deduplicates input, rejecting blank entries that +// would silently match a missing claim. +func cleanSet(in []string, label string) (map[string]struct{}, error) { + out := make(map[string]struct{}, len(in)) + for _, s := range in { + s = strings.TrimSpace(s) + if s == "" { + return nil, fmt.Errorf("%w: %s contains empty value", ErrConfigInvalid, label) + } + out[s] = struct{}{} + } + return out, nil +} + +func validateAbsoluteURL(raw, label string) error { + u, err := url.Parse(raw) + if err != nil { + return fmt.Errorf("%w: %s must be an absolute URL: %v", ErrConfigInvalid, label, err) + } + if u.Scheme == "" || u.Host == "" { + return fmt.Errorf("%w: %s must be an absolute URL", ErrConfigInvalid, label) + } + return nil +} + +// Mode returns the validator's auth mode. Useful for /health. +func (v *Validator) Mode() Mode { + return v.mode +} + +// warmupTimeout is the fallback deadline for the JWKS warmup probe +// when the caller passes a context without one. http.DefaultClient +// has no timeout, so without this guard a misconfigured / unreachable +// JWKS endpoint would block startup indefinitely. +const warmupTimeout = 5 * time.Second + +// Warmup performs a best-effort sanity check on the JWKS endpoint at +// startup. Failure is non-fatal — it is logged and the validator stays +// operable; the underlying *oidc.RemoteKeySet retries on the first real +// request. This avoids crashlooping the container when Google's CDN +// hiccups, while still surfacing misconfiguration (wrong URL, blocked +// egress) prominently in the startup logs. +// +// If ctx has no deadline, a 5s fallback is applied — startup must +// always make forward progress, even if the caller forgot to bound the +// warmup. Callers that DO want to wait longer are respected. +// +// Implementation note: go-oidc's RemoteKeySet does not expose a public +// "fetch now" method — it only fetches on demand from VerifySignature +// (which requires a parseable JWT). We issue an independent HTTP GET +// to the JWKS URL to keep the warmup probe lightweight and reusable. +func (v *Validator) Warmup(ctx context.Context) { + if v.mode != ModeOIDC || v.jwksURL == "" { + return + } + if _, ok := ctx.Deadline(); !ok { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, warmupTimeout) + defer cancel() + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, v.jwksURL, nil) + if err != nil { + log.Printf("scheduledauth: WARN — JWKS warmup request build failed: %v", err) + return + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + log.Printf("scheduledauth: WARN — JWKS warmup fetch failed for %s: %v "+ + "(validator will retry on first request)", v.jwksURL, err) + return + } + defer resp.Body.Close() + if resp.StatusCode >= 400 { + log.Printf("scheduledauth: WARN — JWKS warmup got %d from %s "+ + "(validator will retry on first request)", resp.StatusCode, v.jwksURL) + return + } + if err := validateJWKSBody(resp.Body); err != nil { + log.Printf("scheduledauth: WARN — JWKS warmup got invalid JWKS payload from %s: %v "+ + "(validator will retry on first request)", v.jwksURL, err) + } +} + +func validateJWKSBody(r io.Reader) error { + var doc struct { + Keys []json.RawMessage `json:"keys"` + } + if err := json.NewDecoder(r).Decode(&doc); err != nil { + return err + } + if doc.Keys == nil { + return errors.New(`missing "keys" array`) + } + return nil +} + +// Middleware wraps next with auth enforcement. On failure, writes a 401 +// with no body (avoiding any implementation-detail leakage) and logs the +// reason. Successful requests pass through unmodified. +func (v *Validator) Middleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if v.mode == ModeDisabled { + log.Printf("scheduledauth: WARN — request to %s allowed without auth (mode=disabled)", r.URL.Path) + next.ServeHTTP(w, r) + return + } + + authz := r.Header.Get("Authorization") + if err := v.Validate(r.Context(), authz); err != nil { + log.Printf("scheduledauth: rejected %s %s: %v", r.Method, r.URL.Path, err) + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return + } + next.ServeHTTP(w, r) + }) +} + +// Validate is the unit-testable core. It returns nil iff the +// Authorization header satisfies the configured mode's requirements. +func (v *Validator) Validate(ctx context.Context, authz string) error { + switch v.mode { + case ModeDisabled: + return nil + case ModeBearer: + return v.validateBearer(authz) + case ModeOIDC: + return v.validateOIDC(ctx, authz) + default: + // Should be impossible — New rejects unknown modes. + return fmt.Errorf("%w: misconfigured validator", ErrUnauthorized) + } +} + +func (v *Validator) validateBearer(authz string) error { + if authz == "" { + return fmt.Errorf("%w: missing Authorization header", ErrUnauthorized) + } + if subtle.ConstantTimeCompare([]byte(authz), v.bearer) != 1 { + return fmt.Errorf("%w: bearer secret mismatch", ErrUnauthorized) + } + return nil +} + +func (v *Validator) validateOIDC(ctx context.Context, authz string) error { + rawToken, err := extractBearerToken(authz) + if err != nil { + return err + } + + // Verify signature, issuer, and algorithm via go-oidc. SkipClientIDCheck + // and SkipExpiryCheck are enabled because we apply our own multi-aud + // and skew-tolerant expiry checks below. + idToken, err := v.verifier.Verify(ctx, rawToken) + if err != nil { + return fmt.Errorf("%w: %v", ErrUnauthorized, err) + } + + // Re-parse the payload to access iat / nbf — IDToken exposes Expiry + // and IssuedAt but go-oidc's expiry check has no skew tolerance, and + // nbf is not exposed as a typed field. + var c claims + if err := idToken.Claims(&c); err != nil { + return fmt.Errorf("%w: malformed claims: %v", ErrUnauthorized, err) + } + + if err := v.checkTimestamps(c); err != nil { + return err + } + + // Audience: at least one of the token's aud values must be in our + // allow-list. The token typically has exactly one audience but the + // spec permits multiple. + if !audienceMatches(idToken.Audience, v.audiences) { + return fmt.Errorf("%w: audience %v not in allowlist", ErrUnauthorized, idToken.Audience) + } + + // Subject pinning: required defence-in-depth — any GCP SA in the + // org could mint a token with our `aud` value, but only the + // scheduler SA has our `sub`. + if _, ok := v.subjects[idToken.Subject]; !ok { + return fmt.Errorf("%w: subject %q not in allowlist", ErrUnauthorized, idToken.Subject) + } + + log.Printf("scheduledauth: oidc token accepted (sub=%s, aud=%v)", idToken.Subject, idToken.Audience) + return nil +} + +// extractBearerToken pulls the JWT out of an `Authorization: Bearer ` +// header. Returns ErrUnauthorized for any shape that isn't a non-empty +// bearer token. +func extractBearerToken(authz string) (string, error) { + if authz == "" { + return "", fmt.Errorf("%w: missing Authorization header", ErrUnauthorized) + } + const prefix = "Bearer " + if !strings.HasPrefix(authz, prefix) { + return "", fmt.Errorf("%w: Authorization is not a Bearer token", ErrUnauthorized) + } + raw := strings.TrimSpace(authz[len(prefix):]) + if raw == "" { + return "", fmt.Errorf("%w: empty bearer token", ErrUnauthorized) + } + return raw, nil +} + +// checkTimestamps applies skew-tolerant exp/nbf/iat checks. exp is +// required (Cloud Scheduler always sets it); nbf and iat are optional +// per RFC 7519. The iat-in-future check defends against horizontally- +// spread clock-skew attacks. +func (v *Validator) checkTimestamps(c claims) error { + now := v.now() + + if c.ExpiresAt == 0 { + return fmt.Errorf("%w: missing exp claim", ErrUnauthorized) + } + exp := time.Unix(c.ExpiresAt, 0) + if now.After(exp.Add(v.skew)) { + return fmt.Errorf("%w: token expired at %s (now=%s, skew=%s)", + ErrUnauthorized, exp.Format(time.RFC3339), now.Format(time.RFC3339), v.skew) + } + + if c.NotBefore != 0 { + nbf := time.Unix(c.NotBefore, 0) + if now.Add(v.skew).Before(nbf) { + return fmt.Errorf("%w: token not yet valid (nbf=%s, now=%s)", + ErrUnauthorized, nbf.Format(time.RFC3339), now.Format(time.RFC3339)) + } + } + + if c.IssuedAt != 0 { + iat := time.Unix(c.IssuedAt, 0) + if iat.After(now.Add(v.skew)) { + return fmt.Errorf("%w: token iat in the future (iat=%s, now=%s)", + ErrUnauthorized, iat.Format(time.RFC3339), now.Format(time.RFC3339)) + } + } + return nil +} + +func audienceMatches(tokenAud []string, allowed map[string]struct{}) bool { + for _, a := range tokenAud { + if _, ok := allowed[a]; ok { + return true + } + } + return false +} + +// IsUnauthorized reports whether err originated from a Validate failure. +// Useful for callers that need to distinguish auth failures from config +// errors. +func IsUnauthorized(err error) bool { + return errors.Is(err, ErrUnauthorized) +} diff --git a/internal/server/scheduledauth/validator_test.go b/internal/server/scheduledauth/validator_test.go new file mode 100644 index 000000000..d35a61241 --- /dev/null +++ b/internal/server/scheduledauth/validator_test.go @@ -0,0 +1,828 @@ +package scheduledauth + +import ( + "bytes" + "context" + "crypto/rand" + "crypto/rsa" + "encoding/json" + "errors" + "fmt" + "io" + "log" + "net/http" + "net/http/httptest" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + jose "github.com/go-jose/go-jose/v4" + "github.com/go-jose/go-jose/v4/jwt" +) + +// testSchedulerSubject is the SA unique_id format Google puts in the +// JWT `sub` claim for SA-signed ID tokens (21-digit numeric). The +// production SCHEDULED_TASK_OIDC_SUBJECTS expects this format, NOT the +// SA email — wiring tests around the email shape would let a "sub-as- +// email" regression slip past the suite. +const testSchedulerSubject = "112233445566778899001" + +// testNonSchedulerSubject is a stand-in for any non-listed SA's unique_id, +// used by negative-path tests that verify the validator rejects subjects +// outside SCHEDULED_TASK_OIDC_SUBJECTS. Same shape as testSchedulerSubject +// (21-digit numeric) — using an email-shaped fixture in the rejection +// tests would have made them pass for the wrong reason (subject +// mismatch by FORMAT, not by VALUE), masking a regression that +// silently widened the validator to accept any string. +const testNonSchedulerSubject = "999888777666555444333" + +// testKey wraps an RSA keypair plus the kid we'll publish in the JWKS. +type testKey struct { + priv *rsa.PrivateKey + pub *rsa.PublicKey + kid string +} + +func newTestKey(t *testing.T, kid string) *testKey { + t.Helper() + priv, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("rsa keygen: %v", err) + } + return &testKey{priv: priv, pub: &priv.PublicKey, kid: kid} +} + +// jwks renders a JWKS document containing the given public keys. +func jwks(keys ...*testKey) []byte { + type jwk struct { + Kty string `json:"kty"` + Use string `json:"use"` + Kid string `json:"kid"` + Alg string `json:"alg"` + N string `json:"n"` + E string `json:"e"` + } + out := struct { + Keys []jwk `json:"keys"` + }{} + for _, k := range keys { + j := jose.JSONWebKey{Key: k.pub, KeyID: k.kid, Algorithm: string(jose.RS256), Use: "sig"} + raw, _ := j.MarshalJSON() + var parsed jwk + _ = json.Unmarshal(raw, &parsed) + out.Keys = append(out.Keys, parsed) + } + b, _ := json.Marshal(out) + return b +} + +// jwksServer returns an httptest.Server that serves a JWKS document at +// "/" and counts how many times it has been hit (atomic, so concurrent +// fetches under the single-flight test see a stable value). +type jwksServer struct { + *httptest.Server + hits *atomic.Int64 +} + +func newJWKSServer(t *testing.T, body []byte) *jwksServer { + t.Helper() + hits := &atomic.Int64{} + mu := &sync.Mutex{} + current := body + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + mu.Lock() + defer mu.Unlock() + w.Header().Set("Content-Type", "application/jwk-set+json") + _, _ = w.Write(current) + }) + mux.HandleFunc("/swap", func(w http.ResponseWriter, r *http.Request) { + // Body of POST /swap replaces the served JWKS. Used to test + // refresh-on-unknown-kid. io.ReadAll handles short Reads + // correctly — a single Body.Read call may return less than + // ContentLength and silently truncate the JWKS. + buf, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + mu.Lock() + current = buf + mu.Unlock() + w.WriteHeader(http.StatusOK) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return &jwksServer{Server: srv, hits: hits} +} + +// signToken signs a JWT with the given key and claim set. +func signToken(t *testing.T, key *testKey, c map[string]interface{}) string { + t.Helper() + signer, err := jose.NewSigner( + jose.SigningKey{Algorithm: jose.RS256, Key: key.priv}, + (&jose.SignerOptions{}).WithType("JWT").WithHeader("kid", key.kid), + ) + if err != nil { + t.Fatalf("signer: %v", err) + } + tok, err := jwt.Signed(signer).Claims(c).Serialize() + if err != nil { + t.Fatalf("sign: %v", err) + } + return tok +} + +// signTokenAlg signs with a non-RS256 algorithm. Used to confirm the +// validator rejects unexpected algs. +func signTokenAlg(t *testing.T, alg jose.SignatureAlgorithm, key interface{}, kid string, c map[string]interface{}) string { + t.Helper() + signer, err := jose.NewSigner( + jose.SigningKey{Algorithm: alg, Key: key}, + (&jose.SignerOptions{}).WithType("JWT").WithHeader("kid", kid), + ) + if err != nil { + t.Fatalf("signer: %v", err) + } + tok, err := jwt.Signed(signer).Claims(c).Serialize() + if err != nil { + t.Fatalf("sign: %v", err) + } + return tok +} + +func baseClaims(now time.Time, sub, aud, iss string) map[string]interface{} { + return map[string]interface{}{ + "iss": iss, + "sub": sub, + "aud": aud, + "exp": now.Add(5 * time.Minute).Unix(), + "iat": now.Add(-1 * time.Minute).Unix(), + } +} + +func newOIDCValidator(t *testing.T, jwksURL string) *Validator { + t.Helper() + v, err := New(Config{ + Mode: ModeOIDC, + Issuer: "https://accounts.example.com", + JWKSURL: jwksURL, + Audiences: []string{"https://api.example.com"}, + Subjects: []string{testSchedulerSubject}, + }) + if err != nil { + t.Fatalf("New: %v", err) + } + return v +} + +func TestValidate_OIDC_Valid(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + tok := signToken(t, key, baseClaims(time.Now(), + testSchedulerSubject, + "https://api.example.com", + "https://accounts.example.com")) + + if err := v.Validate(context.Background(), "Bearer "+tok); err != nil { + t.Fatalf("expected valid, got: %v", err) + } +} + +func TestValidate_OIDC_MissingHeader(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + if err := v.Validate(context.Background(), ""); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("expected ErrUnauthorized, got: %v", err) + } +} + +func TestValidate_OIDC_NotBearer(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + if err := v.Validate(context.Background(), "Basic Zm9vOmJhcg=="); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("expected ErrUnauthorized, got: %v", err) + } +} + +func TestValidate_OIDC_Expired(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + now := time.Now() + claims := baseClaims(now, + testSchedulerSubject, + "https://api.example.com", + "https://accounts.example.com") + // 2 minutes past expiry, well beyond the 60s skew. + claims["exp"] = now.Add(-2 * time.Minute).Unix() + tok := signToken(t, key, claims) + + if err := v.Validate(context.Background(), "Bearer "+tok); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("expected ErrUnauthorized, got: %v", err) + } +} + +func TestValidate_OIDC_ExpiryWithinSkew(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + now := time.Now() + claims := baseClaims(now, + testSchedulerSubject, + "https://api.example.com", + "https://accounts.example.com") + // Just expired but within the 60s skew window. + claims["exp"] = now.Add(-30 * time.Second).Unix() + tok := signToken(t, key, claims) + + // Pin time so the test is deterministic. + v.now = func() time.Time { return now } + if err := v.Validate(context.Background(), "Bearer "+tok); err != nil { + t.Fatalf("expected accepted within skew, got: %v", err) + } +} + +func TestValidate_OIDC_WrongAudience(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + tok := signToken(t, key, baseClaims(time.Now(), + testSchedulerSubject, + "https://attacker.example.com", + "https://accounts.example.com")) + + if err := v.Validate(context.Background(), "Bearer "+tok); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("expected ErrUnauthorized, got: %v", err) + } +} + +func TestValidate_OIDC_WrongIssuer(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + tok := signToken(t, key, baseClaims(time.Now(), + testSchedulerSubject, + "https://api.example.com", + "https://attacker-iss.com")) + + if err := v.Validate(context.Background(), "Bearer "+tok); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("expected ErrUnauthorized, got: %v", err) + } +} + +func TestValidate_OIDC_WrongSubject(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + tok := signToken(t, key, baseClaims(time.Now(), + testNonSchedulerSubject, + "https://api.example.com", + "https://accounts.example.com")) + + if err := v.Validate(context.Background(), "Bearer "+tok); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("expected ErrUnauthorized, got: %v", err) + } +} + +func TestValidate_OIDC_BadSignature(t *testing.T) { + key := newTestKey(t, "kid-1") + other := newTestKey(t, "kid-1") // same kid, different key — sig won't verify + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + tok := signToken(t, other, baseClaims(time.Now(), + testSchedulerSubject, + "https://api.example.com", + "https://accounts.example.com")) + + if err := v.Validate(context.Background(), "Bearer "+tok); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("expected ErrUnauthorized, got: %v", err) + } +} + +func TestValidate_OIDC_AlgorithmConfusion(t *testing.T) { + // Server publishes only an RS256 key. Client tries to sign an HS256 + // token using the public key as a shared secret (the classic alg + // confusion attack). The verifier MUST reject this because we + // pinned SupportedSigningAlgs to ["RS256"]. + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + // We can't easily sign an HS256 with an RSA public key via go-jose + // (it expects []byte), but signing with HS256 + a random secret + + // kid="kid-1" is a strictly easier case for the attacker; if the + // validator rejects this it covers the core alg-confusion gap. + tok := signTokenAlg(t, jose.HS256, []byte("attacker-secret-32-bytes-padding!!"), key.kid, baseClaims(time.Now(), + testSchedulerSubject, + "https://api.example.com", + "https://accounts.example.com")) + + err := v.Validate(context.Background(), "Bearer "+tok) + if !errors.Is(err, ErrUnauthorized) { + t.Fatalf("expected ErrUnauthorized for HS256 token, got: %v", err) + } +} + +func TestValidate_OIDC_IATInFuture(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + now := time.Now() + claims := baseClaims(now, + testSchedulerSubject, + "https://api.example.com", + "https://accounts.example.com") + // 2 minutes in the future, well beyond the 60s skew. + claims["iat"] = now.Add(2 * time.Minute).Unix() + tok := signToken(t, key, claims) + + v.now = func() time.Time { return now } + if err := v.Validate(context.Background(), "Bearer "+tok); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("expected ErrUnauthorized, got: %v", err) + } +} + +func TestValidate_OIDC_NotBeforeFuture(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + now := time.Now() + claims := baseClaims(now, + testSchedulerSubject, + "https://api.example.com", + "https://accounts.example.com") + claims["nbf"] = now.Add(2 * time.Minute).Unix() + tok := signToken(t, key, claims) + + v.now = func() time.Time { return now } + if err := v.Validate(context.Background(), "Bearer "+tok); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("expected ErrUnauthorized, got: %v", err) + } +} + +func TestValidate_OIDC_AudienceListClaim(t *testing.T) { + // `aud` may be either a string or a JSON list. Make sure the list + // form is accepted as long as one entry matches. + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + now := time.Now() + claims := baseClaims(now, + testSchedulerSubject, + "placeholder", // overridden below + "https://accounts.example.com") + claims["aud"] = []string{"https://other.example.com", "https://api.example.com"} + tok := signToken(t, key, claims) + + if err := v.Validate(context.Background(), "Bearer "+tok); err != nil { + t.Fatalf("expected valid, got: %v", err) + } +} + +func TestValidate_OIDC_KeyRotation_RefreshOnUnknownKid(t *testing.T) { + // 1. Server starts with key A. Token signed by A → validates. + // 2. Provider rotates: server now publishes key B. Token signed by B + // arrives with a previously-unseen kid. The validator MUST refresh + // the JWKS and accept the new token. + keyA := newTestKey(t, "kid-A") + keyB := newTestKey(t, "kid-B") + srv := newJWKSServer(t, jwks(keyA)) + v := newOIDCValidator(t, srv.URL) + + tokA := signToken(t, keyA, baseClaims(time.Now(), + testSchedulerSubject, + "https://api.example.com", + "https://accounts.example.com")) + if err := v.Validate(context.Background(), "Bearer "+tokA); err != nil { + t.Fatalf("kid A: %v", err) + } + + // Swap the JWKS to publish kid B. + // + // Both the request build and the response status are checked: if the + // /swap handler 5xx's (or — more subtly — returns a non-200 because + // the body short-read), the JWKS would silently NOT update. The test + // would then fail later at "unknown kid" instead of pointing at the + // real cause. Surfacing the swap failure here keeps the diagnostic + // chain short. + jwksB := jwks(keyB) + swap, err := http.NewRequest(http.MethodPost, srv.URL+"/swap", strings.NewReader(string(jwksB))) + if err != nil { + t.Fatalf("build swap request: %v", err) + } + swap.ContentLength = int64(len(jwksB)) + resp, err := http.DefaultClient.Do(swap) + if err != nil { + t.Fatalf("swap: %v", err) + } + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + t.Fatalf("swap: unexpected status %d, body: %s", resp.StatusCode, string(body)) + } + resp.Body.Close() + + tokB := signToken(t, keyB, baseClaims(time.Now(), + testSchedulerSubject, + "https://api.example.com", + "https://accounts.example.com")) + if err := v.Validate(context.Background(), "Bearer "+tokB); err != nil { + t.Fatalf("kid B (post-rotation): %v", err) + } +} + +func TestValidate_OIDC_SingleFlight_StampedeProtection(t *testing.T) { + // 50 concurrent verifications of fresh tokens after a cold start + // should result in at most a small handful of JWKS fetches (the + // underlying RemoteKeySet uses single-flight). Without the + // protection we'd see ~50 hits. + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + tok := signToken(t, key, baseClaims(time.Now(), + testSchedulerSubject, + "https://api.example.com", + "https://accounts.example.com")) + + const N = 50 + var wg sync.WaitGroup + wg.Add(N) + errCh := make(chan error, N) + for i := 0; i < N; i++ { + go func() { + defer wg.Done() + if err := v.Validate(context.Background(), "Bearer "+tok); err != nil { + errCh <- err + } + }() + } + wg.Wait() + close(errCh) + for err := range errCh { + t.Errorf("concurrent validate: %v", err) + } + hits := srv.hits.Load() + if hits > 5 { + t.Errorf("expected <=5 JWKS fetches under single-flight, got %d", hits) + } +} + +func TestValidate_Bearer_OK(t *testing.T) { + v, err := New(Config{Mode: ModeBearer, Bearer: "topsecret"}) + if err != nil { + t.Fatalf("New: %v", err) + } + if err := v.Validate(context.Background(), "Bearer topsecret"); err != nil { + t.Fatalf("expected ok, got: %v", err) + } +} + +func TestValidate_Bearer_Mismatch(t *testing.T) { + v, err := New(Config{Mode: ModeBearer, Bearer: "topsecret"}) + if err != nil { + t.Fatalf("New: %v", err) + } + if err := v.Validate(context.Background(), "Bearer wrong"); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("expected ErrUnauthorized, got: %v", err) + } +} + +func TestValidate_Bearer_MissingHeader(t *testing.T) { + v, err := New(Config{Mode: ModeBearer, Bearer: "topsecret"}) + if err != nil { + t.Fatalf("New: %v", err) + } + if err := v.Validate(context.Background(), ""); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("expected ErrUnauthorized, got: %v", err) + } +} + +func TestValidate_Disabled_AlwaysAllows(t *testing.T) { + v, err := New(Config{Mode: ModeDisabled}) + if err != nil { + t.Fatalf("New: %v", err) + } + if err := v.Validate(context.Background(), ""); err != nil { + t.Fatalf("expected nil, got: %v", err) + } +} + +func TestNew_Rejects_OIDCWithoutSubjects(t *testing.T) { + _, err := New(Config{ + Mode: ModeOIDC, + Audiences: []string{"https://api.example.com"}, + // Subjects intentionally empty. + }) + if !errors.Is(err, ErrConfigInvalid) { + t.Fatalf("expected ErrConfigInvalid, got: %v", err) + } +} + +func TestNew_Rejects_OIDCWithoutAudiences(t *testing.T) { + _, err := New(Config{ + Mode: ModeOIDC, + Subjects: []string{testSchedulerSubject}, + }) + if !errors.Is(err, ErrConfigInvalid) { + t.Fatalf("expected ErrConfigInvalid, got: %v", err) + } +} + +func TestNew_RejectsOIDCMalformedJWKSURL(t *testing.T) { + for _, jwksURL := range []string{"://bad", "/relative/path", "https://"} { + t.Run(jwksURL, func(t *testing.T) { + _, err := New(Config{ + Mode: ModeOIDC, + JWKSURL: jwksURL, + Audiences: []string{"https://api.example.com"}, + Subjects: []string{testSchedulerSubject}, + }) + if !errors.Is(err, ErrConfigInvalid) { + t.Fatalf("expected ErrConfigInvalid, got: %v", err) + } + }) + } +} + +func TestNew_Rejects_BearerWithoutSecret(t *testing.T) { + _, err := New(Config{Mode: ModeBearer}) + if !errors.Is(err, ErrConfigInvalid) { + t.Fatalf("expected ErrConfigInvalid, got: %v", err) + } +} + +func TestNew_Rejects_UnknownMode(t *testing.T) { + _, err := New(Config{Mode: Mode("bogus")}) + if !errors.Is(err, ErrConfigInvalid) { + t.Fatalf("expected ErrConfigInvalid, got: %v", err) + } +} + +func TestLoadConfig_RejectsMissingAuthMode(t *testing.T) { + _, err := LoadConfig(EnvMap{}) + if !errors.Is(err, ErrConfigInvalid) { + t.Fatalf("expected ErrConfigInvalid for unset SCHEDULED_TASK_AUTH_MODE, got: %v", err) + } +} + +func TestLoadConfig_ExplicitDisabledIsAccepted(t *testing.T) { + cfg, err := LoadConfig(EnvMap{EnvAuthMode: "disabled"}) + if err != nil { + t.Fatalf("LoadConfig(disabled): %v", err) + } + if cfg.Mode != ModeDisabled { + t.Fatalf("mode = %s, want %s", cfg.Mode, ModeDisabled) + } +} + +func TestLoadConfig_OIDCParsesCommaSeparated(t *testing.T) { + cfg, err := LoadConfig(EnvMap{ + EnvAuthMode: "oidc", + EnvOIDCAudience: " a@example.com , , b@example.com ", + EnvOIDCSubjects: "x@example.com,y@example.com", + }) + if err != nil { + t.Fatalf("LoadConfig: %v", err) + } + if cfg.Mode != ModeOIDC { + t.Fatalf("mode = %s", cfg.Mode) + } + if got, want := strings.Join(cfg.Audiences, ","), "a@example.com,b@example.com"; got != want { + t.Fatalf("audiences = %q, want %q", got, want) + } + if got, want := strings.Join(cfg.Subjects, ","), "x@example.com,y@example.com"; got != want { + t.Fatalf("subjects = %q, want %q", got, want) + } +} + +func TestLoadConfig_RejectsUnknownMode(t *testing.T) { + _, err := LoadConfig(EnvMap{EnvAuthMode: "bogus"}) + if !errors.Is(err, ErrConfigInvalid) { + t.Fatalf("expected ErrConfigInvalid, got: %v", err) + } +} + +func TestLoadConfig_BearerParse(t *testing.T) { + cfg, err := LoadConfig(EnvMap{ + EnvAuthMode: "bearer", + EnvBearerSecret: "topsecret", + }) + if err != nil { + t.Fatalf("LoadConfig: %v", err) + } + if cfg.Mode != ModeBearer || cfg.Bearer != "topsecret" { + t.Fatalf("got %+v", cfg) + } +} + +func TestMiddleware_OIDC_RejectsAndLogsButDoesNotCallNext(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + called := false + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { called = true }) + mw := v.Middleware(next) + + req := httptest.NewRequest("POST", "/api/scheduled/foo", nil) + rr := httptest.NewRecorder() + mw.ServeHTTP(rr, req) + + if rr.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401", rr.Code) + } + if called { + t.Fatalf("next handler must not be called on auth failure") + } +} + +func TestMiddleware_OIDC_AllowsAndCallsNextOnSuccess(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + tok := signToken(t, key, baseClaims(time.Now(), + testSchedulerSubject, + "https://api.example.com", + "https://accounts.example.com")) + + called := false + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + w.WriteHeader(http.StatusOK) + }) + mw := v.Middleware(next) + + req := httptest.NewRequest("POST", "/api/scheduled/foo", nil) + req.Header.Set("Authorization", "Bearer "+tok) + rr := httptest.NewRecorder() + mw.ServeHTTP(rr, req) + + if !called { + t.Fatalf("next handler should be called on success") + } + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rr.Code) + } +} + +func TestMiddleware_Disabled_PassesThroughWithWarn(t *testing.T) { + v, err := New(Config{Mode: ModeDisabled}) + if err != nil { + t.Fatalf("New: %v", err) + } + called := false + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + w.WriteHeader(http.StatusNoContent) + }) + mw := v.Middleware(next) + + req := httptest.NewRequest("POST", "/api/scheduled/foo", nil) + rr := httptest.NewRecorder() + mw.ServeHTTP(rr, req) + + if !called { + t.Fatalf("disabled middleware must call next") + } + if rr.Code != http.StatusNoContent { + t.Fatalf("status = %d", rr.Code) + } +} + +func TestWarmup_HitsJWKSEndpoint(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + + before := srv.hits.Load() + v.Warmup(context.Background()) + after := srv.hits.Load() + if after <= before { + t.Fatalf("Warmup should have hit JWKS endpoint at least once (before=%d after=%d)", before, after) + } +} + +func TestValidateJWKSBody_RequiresKeysArray(t *testing.T) { + tests := []struct { + name string + body string + wantErr bool + }{ + {name: "valid", body: `{"keys":[]}`}, + {name: "invalid json", body: `{`, wantErr: true}, + {name: "missing keys", body: `{}`, wantErr: true}, + {name: "null keys", body: `{"keys":null}`, wantErr: true}, + {name: "object keys", body: `{"keys":{}}`, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateJWKSBody(strings.NewReader(tt.body)) + if tt.wantErr && err == nil { + t.Fatalf("expected error") + } + if !tt.wantErr && err != nil { + t.Fatalf("expected nil, got: %v", err) + } + }) + } +} + +func TestWarmup_LoggedAndNonFatal_OnDeadEndpoint(t *testing.T) { + // Capture the global logger so we can assert the failure path was + // actually exercised. Without this, the test passes just as well if + // Warmup were to short-circuit before the fetch ever ran (CR pass on + // PR #161 — "did not hang" alone doesn't pin the non-fatal branch). + // No tests in this package use t.Parallel(), so swapping the global + // log writer here is safe; t.Cleanup restores it. + var logBuf bytes.Buffer + origOut := log.Writer() + log.SetOutput(&logBuf) + t.Cleanup(func() { log.SetOutput(origOut) }) + + v, err := New(Config{ + Mode: ModeOIDC, + Issuer: "https://accounts.example.com", + JWKSURL: "http://127.0.0.1:1/this-port-is-closed", // ECONNREFUSED + Audiences: []string{"https://api.example.com"}, + Subjects: []string{testSchedulerSubject}, + }) + if err != nil { + t.Fatalf("New: %v", err) + } + // Warmup must not panic or block. Use a short timeout to make the + // test fast even if Warmup were to misbehave. + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + v.Warmup(ctx) // non-fatal — only logs + + // Assert the failure-path log fired. The validator emits + // "scheduledauth: WARN — JWKS warmup fetch failed for : " + // on connect refused (validator.go:255). We pin on the substring + // "JWKS warmup" so a future log-message tweak doesn't break the + // test for cosmetic reasons, while still proving the code path + // reached the post-fetch error branch. + logs := logBuf.String() + if !strings.Contains(logs, "JWKS warmup") { + t.Fatalf("expected a JWKS-warmup failure log, got: %q", logs) + } +} + +// guard: sanity-check that Mode is reported back. +func TestMode(t *testing.T) { + for _, m := range []Mode{ModeOIDC, ModeBearer, ModeDisabled} { + var v *Validator + var err error + switch m { + case ModeOIDC: + v, err = New(Config{ + Mode: ModeOIDC, + JWKSURL: "http://127.0.0.1:1", + Audiences: []string{"a"}, + Subjects: []string{"s"}, + }) + case ModeBearer: + v, err = New(Config{Mode: ModeBearer, Bearer: "x"}) + case ModeDisabled: + v, err = New(Config{Mode: ModeDisabled}) + } + if err != nil { + t.Fatalf("New(%s): %v", m, err) + } + if v.Mode() != m { + t.Fatalf("Mode() = %s, want %s", v.Mode(), m) + } + } +} + +// guard: ensure typed sentinel works with errors.Is. +func TestIsUnauthorized(t *testing.T) { + wrapped := fmt.Errorf("wrap: %w", ErrUnauthorized) + if !IsUnauthorized(wrapped) { + t.Fatalf("IsUnauthorized failed for wrapped sentinel") + } + if IsUnauthorized(errors.New("other")) { + t.Fatalf("IsUnauthorized matched unrelated error") + } +} diff --git a/terraform/environments/azure/dev.tfvars.example b/terraform/environments/azure/dev.tfvars.example index 318bcdc85..1734e03fa 100644 --- a/terraform/environments/azure/dev.tfvars.example +++ b/terraform/environments/azure/dev.tfvars.example @@ -78,9 +78,24 @@ key_vault_default_network_acl_action = "Allow" # ============================================== # Scheduled Tasks (Logic Apps) # ============================================== - -enable_scheduled_tasks = true -recommendation_schedule = "0 2 * * *" # Cron: Daily at 2 AM +# +# Logic Apps fetch a shared bearer secret from Key Vault at invocation +# time and call /api/scheduled/* with `Authorization: Bearer `. +# The CUDly app validates via SCHEDULED_TASK_AUTH_MODE=bearer (set +# automatically by the container-apps / aks modules) — the secret name +# itself is wired through `module.secrets.scheduled_task_secret_name` +# and never lives in the container env. + +enable_scheduled_tasks = true + +# recommendation_schedule: a cron-shaped string, but ONLY the hour field +# (index [1]) is honored — the Azure module converts it into a Logic +# Apps recurrence trigger of frequency=Day + interval=1 starting at +# that hour (see modules/compute/azure/container-apps/scheduled-tasks.tf). +# Sub-daily / day-of-week / day-of-month patterns are silently ignored. +# Stick to daily-at-hour-N values; if you need finer granularity, +# extend the parser before pinning a different schedule. +recommendation_schedule = "0 2 * * *" # Daily at 02:00 UTC # ============================================== # Frontend (Azure CDN) diff --git a/terraform/environments/azure/variables.tf b/terraform/environments/azure/variables.tf index df517c6f9..dc26f8570 100644 --- a/terraform/environments/azure/variables.tf +++ b/terraform/environments/azure/variables.tf @@ -378,9 +378,15 @@ variable "enable_scheduled_tasks" { } variable "recommendation_schedule" { - description = "Cron schedule for recommendations task" + description = <<-EOT + Cron-shaped schedule for the recommendations Logic App. Only the + hour field is parsed (the Azure module converts it to Logic Apps + frequency=Day + interval=1 + startTime=:00). Sub-daily, + day-of-week, and day-of-month patterns are silently ignored; + use daily-at-hour-N values only. + EOT type = string - default = "0 2 * * *" # 2 AM daily + default = "0 2 * * *" # Daily at 02:00 UTC } variable "enable_ri_exchange_schedule" { diff --git a/terraform/environments/gcp/compute.tf b/terraform/environments/gcp/compute.tf index 38219ded8..7026586fd 100644 --- a/terraform/environments/gcp/compute.tf +++ b/terraform/environments/gcp/compute.tf @@ -49,10 +49,17 @@ module "compute_cloud_run" { # VPC Access (for Cloud SQL) vpc_connector_id = module.networking.vpc_connector_id - # Scheduled tasks + # Scheduled tasks. Per #159 the cloud-run module no longer takes a + # plaintext scheduled_task_secret — Cloud Scheduler signs an OIDC + # ID token with the scheduler SA, and the CUDly app validates that + # token at /api/scheduled/* via internal/server/scheduledauth + # (signature, issuer, audience, sub-pin). Cloud Run's IAM gate + # (roles/run.invoker, gated by cloud_run_allow_unauthenticated — + # tracked separately in #78) acts as defence in depth on top. + # Azure stays on bearer + Key Vault because Logic Apps' HTTP + # Connector does not emit Entra OIDC tokens. enable_scheduled_tasks = var.enable_scheduled_tasks recommendation_schedule = var.recommendation_schedule - scheduled_task_secret = module.secrets.scheduled_task_secret_value # RI exchange automation enable_ri_exchange_schedule = var.enable_ri_exchange_schedule @@ -73,7 +80,6 @@ module "compute_cloud_run" { FROM_EMAIL = var.subdomain_zone_name != "" ? "noreply@${var.subdomain_zone_name}" : "noreply@${var.project_name}.example.com" DASHBOARD_URL = local.dashboard_url CORS_ALLOWED_ORIGIN = local.dashboard_url != "" ? local.dashboard_url : "http://localhost:3000" - SCHEDULED_TASK_SECRET = module.secrets.scheduled_task_secret_value CUDLY_MAX_ACCOUNT_PARALLELISM = tostring(var.max_account_parallelism) CUDLY_SOURCE_CLOUD = "gcp" }, @@ -147,7 +153,6 @@ module "compute_gke" { FROM_EMAIL = var.subdomain_zone_name != "" ? "noreply@${var.subdomain_zone_name}" : "noreply@${var.project_name}.example.com" DASHBOARD_URL = local.dashboard_url CORS_ALLOWED_ORIGIN = local.dashboard_url != "" ? local.dashboard_url : "http://localhost:3000" - SCHEDULED_TASK_SECRET = module.secrets.scheduled_task_secret_value CUDLY_MAX_ACCOUNT_PARALLELISM = tostring(var.max_account_parallelism) CUDLY_SOURCE_CLOUD = "gcp" }, diff --git a/terraform/environments/gcp/dev.tfvars.example b/terraform/environments/gcp/dev.tfvars.example index 8a6619982..508a0b89c 100644 --- a/terraform/environments/gcp/dev.tfvars.example +++ b/terraform/environments/gcp/dev.tfvars.example @@ -65,6 +65,12 @@ enable_nat_logging = false # ============================================== # Scheduled Tasks (Cloud Scheduler) # ============================================== +# +# Cloud Scheduler signs OIDC ID tokens with `audience` and `sub` set +# from the per-job service account. The CUDly app validates these +# tokens via SCHEDULED_TASK_AUTH_MODE=oidc + SCHEDULED_TASK_OIDC_* +# env vars — those are wired automatically by the cloud-run / gke +# modules and do not need to be set here. enable_scheduled_tasks = true recommendation_schedule = "0 2 * * *" # Cron: Daily at 2 AM diff --git a/terraform/modules/compute/azure/aks/main.tf b/terraform/modules/compute/azure/aks/main.tf index cecae74cc..f9fe58ccc 100644 --- a/terraform/modules/compute/azure/aks/main.tf +++ b/terraform/modules/compute/azure/aks/main.tf @@ -360,6 +360,15 @@ resource "kubernetes_deployment" "app" { value = join(",", var.allowed_origins) } + # Scheduled-task auth — Azure stays on bearer mode (Logic Apps + # fetch the shared secret from Key Vault at invocation time; + # Logic Apps' HTTP Connector does not emit Entra OIDC tokens). + # Validated app-side via internal/server/scheduledauth. + env { + name = "SCHEDULED_TASK_AUTH_MODE" + value = "bearer" + } + dynamic "env" { for_each = var.additional_env_vars content { diff --git a/terraform/modules/compute/azure/container-apps/main.tf b/terraform/modules/compute/azure/container-apps/main.tf index 1a18fb0f7..4c367d5f9 100644 --- a/terraform/modules/compute/azure/container-apps/main.tf +++ b/terraform/modules/compute/azure/container-apps/main.tf @@ -137,6 +137,13 @@ resource "azurerm_container_app" "main" { CUDLY_SOURCE_CLOUD = "azure" CUDLY_SIGNING_KEY_VAULT_URL = var.key_vault_uri CUDLY_SIGNING_KEY_NAME = var.signing_key_name + # Scheduled-task auth — Azure stays on the bearer mode where + # Logic Apps fetch the shared secret from Key Vault at + # invocation time and the app constant-time-compares it via + # internal/server/scheduledauth. Switching to oidc here + # would require Entra-issued tokens, which Logic Apps does + # not emit on the HTTP Connector trigger we use. + SCHEDULED_TASK_AUTH_MODE = "bearer" }, var.additional_env_vars ) diff --git a/terraform/modules/compute/azure/container-apps/variables.tf b/terraform/modules/compute/azure/container-apps/variables.tf index b493154ac..354f669bd 100644 --- a/terraform/modules/compute/azure/container-apps/variables.tf +++ b/terraform/modules/compute/azure/container-apps/variables.tf @@ -223,7 +223,14 @@ variable "scheduled_task_secret_name" { } variable "recommendation_schedule" { - description = "Cron schedule for recommendations refresh (default: daily at 2 AM UTC)" + description = <<-EOT + Cron-shaped schedule for the recommendations Logic App. Only the + hour field (index [1]) is parsed — see scheduled-tasks.tf — and + the value is materialized as a frequency=Day + interval=1 + recurrence starting at that hour. Sub-daily, day-of-week, and + day-of-month cron patterns are silently ignored; stick to + daily-at-hour-N values. Default: daily at 02:00 UTC. + EOT type = string default = "0 2 * * *" } diff --git a/terraform/modules/compute/gcp/cloud-run/main.tf b/terraform/modules/compute/gcp/cloud-run/main.tf index 40802dfdc..686d1404e 100644 --- a/terraform/modules/compute/gcp/cloud-run/main.tf +++ b/terraform/modules/compute/gcp/cloud-run/main.tf @@ -34,6 +34,56 @@ resource "google_service_account" "cloud_run" { project = var.project_id } +# ============================================== +# Scheduled-task auth wiring +# ============================================== +# +# Both Cloud Scheduler jobs below sign OIDC tokens with `audience` set to +# the scheduler SA email and `sub` set to the SA's unique numeric ID. +# The Go validator (internal/server/scheduledauth) checks signature + +# issuer + audience + subject; pinning these here keeps the allow-list +# in the container env in sync with what Cloud Scheduler actually emits. +# +# `compact` drops empty strings produced when a scheduler is disabled +# (count = 0 → google_service_account.scheduler[0] is unavailable, so +# we use try() to short-circuit). `join(",")` produces the CSV format +# the validator parses. +locals { + # Per-schedule audiences: each Cloud Scheduler job mints an OIDC + # token whose `aud` claim names the receiving endpoint, NOT the + # scheduler SA. `sub` already pins the caller to the SA's unique_id; + # using the SA email for `aud` made both checks describe the caller + # and was replay-prone if the SA was ever reused for a different + # service. The new shape keeps the token recipient-bound. (CodeRabbit + # nitpick on PR #161.) + # + # We deliberately use ${var.service_name}/api/scheduled/ + # rather than the Cloud Run URL: the URL would create a Terraform + # cycle (google_cloud_run_v2_service.main.uri → env vars on the + # service itself → the audience locals → back to the service). + # `service_name` is a module input, so the audience is fully + # resolvable before the Cloud Run resource is planned. Any string + # that's unique-per-(service, endpoint) satisfies the OIDC + # recipient-bound contract — the validator only does string-equality + # against this same value (built once into the env var below). + scheduled_task_recommendations_audience = "${var.service_name}/api/scheduled/recommendations" + scheduled_task_ri_exchange_audience = "${var.service_name}/api/scheduled/ri-exchange" + + scheduled_task_oidc_audiences = join(",", compact([ + var.enable_scheduled_tasks ? local.scheduled_task_recommendations_audience : "", + var.enable_ri_exchange_schedule ? local.scheduled_task_ri_exchange_audience : "", + ])) + scheduled_task_oidc_subjects = join(",", compact([ + var.enable_scheduled_tasks ? try(google_service_account.scheduler[0].unique_id, "") : "", + var.enable_ri_exchange_schedule ? try(google_service_account.scheduler_ri_exchange[0].unique_id, "") : "", + ])) + # Auth mode selector. When neither scheduler is enabled there's + # nothing to validate; staying on "disabled" avoids fail-fast on an + # empty allow-list in the validator. The resulting WARN log is + # harmless when /api/scheduled/* is never hit. + scheduled_task_auth_mode = (var.enable_scheduled_tasks || var.enable_ri_exchange_schedule) ? "oidc" : "disabled" +} + # ============================================== # Cloud Run Service # ============================================== @@ -99,6 +149,11 @@ resource "google_cloud_run_v2_service" "main" { # OIDC issuer signing key — see internal/oidc/gcp_signer.go. CUDLY_SOURCE_CLOUD = "gcp" CUDLY_SIGNING_KEY_RESOURCE = local.signing_key_version_resource + # Scheduled-task OIDC validator config — see + # internal/server/scheduledauth and the locals block above. + SCHEDULED_TASK_AUTH_MODE = local.scheduled_task_auth_mode + SCHEDULED_TASK_OIDC_AUDIENCE = local.scheduled_task_oidc_audiences + SCHEDULED_TASK_OIDC_SUBJECTS = local.scheduled_task_oidc_subjects }, var.additional_env_vars ) @@ -269,12 +324,25 @@ resource "google_cloud_scheduler_job" "recommendations" { http_method = "POST" uri = "${google_cloud_run_v2_service.main.uri}/api/scheduled/recommendations" - headers = { - "Authorization" = "Bearer ${var.scheduled_task_secret}" - } - + # Auth: oidc_token below is signed by the scheduler's service + # account at invocation time. Two complementary defences: + # 1. Cloud Run's IAM gate via roles/run.invoker (when + # cloud_run_allow_unauthenticated = false; tracked separately + # in #78). + # 2. App-level OIDC validation on /api/scheduled/* — the Go + # validator (internal/server/scheduledauth) checks the JWT + # signature, issuer, audience, and pins the subject to this + # SA's unique_id. Audience is pinned to the receiving endpoint + # URL (the OAuth/OIDC convention — keeps the token recipient- + # bound). The validator's whitelist reads the same value via + # local.scheduled_task_oidc_audiences so the JWT claim and + # the env-var match by construction. + # The previous static `Authorization: Bearer + # ${var.scheduled_task_secret}` header leaked the shared secret into + # the scheduler resource definition + Terraform state — closes #159. oidc_token { service_account_email = google_service_account.scheduler[0].email + audience = local.scheduled_task_recommendations_audience } } } @@ -322,12 +390,14 @@ resource "google_cloud_scheduler_job" "ri_exchange" { http_method = "POST" uri = "${google_cloud_run_v2_service.main.uri}/api/scheduled/ri-exchange" - headers = { - "Authorization" = "Bearer ${var.scheduled_task_secret}" - } - + # Same OIDC-only model as the recommendations scheduler above — + # see #159 for the rationale. Audience pinned to the receiving + # endpoint URL so the token is recipient-bound and the validator's + # whitelist is matched by construction (the env var includes this + # value via local.scheduled_task_oidc_audiences). oidc_token { service_account_email = google_service_account.scheduler_ri_exchange[0].email + audience = local.scheduled_task_ri_exchange_audience } } } diff --git a/terraform/modules/compute/gcp/cloud-run/variables.tf b/terraform/modules/compute/gcp/cloud-run/variables.tf index c4adcea19..6da6d4bf6 100644 --- a/terraform/modules/compute/gcp/cloud-run/variables.tf +++ b/terraform/modules/compute/gcp/cloud-run/variables.tf @@ -161,13 +161,6 @@ variable "recommendation_schedule" { default = "0 2 * * *" } -variable "scheduled_task_secret" { - description = "Shared secret for authenticating scheduled task HTTP calls" - type = string - default = "" - sensitive = true -} - variable "additional_secret_accessor_ids" { description = <<-EOT Map of Secret Manager secret IDs (full resource names) the Cloud Run diff --git a/terraform/modules/compute/gcp/gke/main.tf b/terraform/modules/compute/gcp/gke/main.tf index 61b475565..f193944d3 100644 --- a/terraform/modules/compute/gcp/gke/main.tf +++ b/terraform/modules/compute/gcp/gke/main.tf @@ -18,6 +18,35 @@ locals { managed_by = "terraform" } ) + + # Scheduled-task OIDC validator wiring — see + # internal/server/scheduledauth. + # + # `aud` (audience) names the RECEIVING endpoint — the URL Cloud + # Scheduler is calling. Pinning aud to the endpoint keeps tokens + # recipient-bound (the OAuth/OIDC convention) and limits cross-service + # replay if the scheduler SA is ever reused. + # + # `sub` (subject) names the CALLER — the scheduler SA's unique + # numeric ID. Google puts the SA unique_id (not its email) in the + # JWT `sub` claim for SA-signed ID tokens, so the validator's + # SCHEDULED_TASK_OIDC_SUBJECTS env var must contain that numeric form. + # + # Auth mode is derived from scheduler SA presence, NOT from + # var.enable_scheduled_tasks. kubernetes_ingress_v1.app exposes + # /api/scheduled/* via the catch-all rule even when the scheduler is + # disabled, so tying auth to enable_scheduled_tasks would turn a + # disabled scheduler into an unauthenticated public trigger. With no + # SA, var.scheduled_task_auth_mode_override decides — its default is + # the fail-closed "oidc" so an unauthenticated boot requires opting + # in to "disabled" deliberately. + scheduled_task_oidc_audience = "${var.app_url}/api/scheduled/recommendations" + scheduled_task_oidc_subject = try(google_service_account.scheduler[0].unique_id, "") + scheduled_task_auth_mode = ( + length(google_service_account.scheduler) > 0 + ? "oidc" + : var.scheduled_task_auth_mode_override + ) } # ============================================== @@ -448,6 +477,23 @@ resource "kubernetes_deployment" "app" { value = join(",", var.allowed_origins) } + # Scheduled-task OIDC validator config — see locals block above + # and internal/server/scheduledauth. + env { + name = "SCHEDULED_TASK_AUTH_MODE" + value = local.scheduled_task_auth_mode + } + + env { + name = "SCHEDULED_TASK_OIDC_AUDIENCE" + value = local.scheduled_task_oidc_audience + } + + env { + name = "SCHEDULED_TASK_OIDC_SUBJECTS" + value = local.scheduled_task_oidc_subject + } + dynamic "env" { for_each = var.additional_env_vars content { @@ -670,14 +716,25 @@ resource "google_cloud_scheduler_job" "recommendations" { http_target { http_method = "POST" - uri = "${var.app_url}/api/scheduled/recommendations" - - headers = { - "Authorization" = "Bearer ${var.scheduled_task_secret}" - } - + # Both the request target and the OIDC audience read the same + # local so they cannot drift. If they ever did, Cloud Scheduler + # would mint a token with `aud` for the old endpoint while + # actually POSTing to the new one — the validator would 401 + # silently. Sharing the local makes that class of regression a + # syntax-level mismatch rather than a runtime auth failure. + uri = local.scheduled_task_oidc_audience + + # Auth: oidc_token below is signed by the scheduler's service + # account at invocation time. GKE Ingress has no Cloud-Run-style + # IAM gate, so the OIDC token is validated by the app at + # /api/scheduled/* via internal/server/scheduledauth — signature, + # issuer, audience, and `sub` pinned to this SA's unique_id. + # The previous static `Authorization: Bearer + # ${var.scheduled_task_secret}` header leaked the shared secret into + # the scheduler resource definition + Terraform state — closes #159. oidc_token { service_account_email = google_service_account.scheduler[0].email + audience = local.scheduled_task_oidc_audience } } } diff --git a/terraform/modules/compute/gcp/gke/variables.tf b/terraform/modules/compute/gcp/gke/variables.tf index 38ef98b3a..8c3602433 100644 --- a/terraform/modules/compute/gcp/gke/variables.tf +++ b/terraform/modules/compute/gcp/gke/variables.tf @@ -261,15 +261,31 @@ variable "recommendation_schedule" { default = "0 2 * * *" } -variable "scheduled_task_secret" { - description = "Shared secret for authenticating scheduled task HTTP calls" +variable "app_url" { + description = "Application URL for scheduled task HTTP triggers (e.g., https://app.example.com). Required when enable_scheduled_tasks is true." type = string default = "" - sensitive = true } -variable "app_url" { - description = "Application URL for scheduled task HTTP triggers (e.g., https://app.example.com). Required when enable_scheduled_tasks is true." +variable "scheduled_task_auth_mode_override" { + description = <<-EOT + Override for SCHEDULED_TASK_AUTH_MODE used ONLY when no scheduler SA + is created (var.enable_scheduled_tasks = false). When the SA exists, + auth mode is always derived as "oidc" — the override is ignored. + + Why this exists: kubernetes_ingress_v1.app exposes /api/scheduled/* + via the catch-all rule even when the scheduler is disabled. Tying + auth to var.enable_scheduled_tasks would silently boot those + endpoints unauthenticated. The fail-closed default ("oidc") here + means a deploy without scheduler SA still requires the validator to + be configured (or it rejects every request). Set this to "disabled" + deliberately for local-dev / dry-run only. + EOT type = string - default = "" + default = "oidc" + + validation { + condition = contains(["oidc", "bearer", "disabled"], var.scheduled_task_auth_mode_override) + error_message = "scheduled_task_auth_mode_override must be one of: \"oidc\", \"bearer\", \"disabled\"." + } }