diff --git a/.golangci.yml b/.golangci.yml index c5bb12f6a0..b404319318 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -784,11 +784,26 @@ linters: list-mode: strict files: - '**/internal/identitybroker/adapter/**.go' + - '!**/internal/identitybroker/adapter/as1server/**.go' - '!$test' allow: - $gostd - github.com/stacklok/mecatl/internal/identitybroker$ - github.com/golang-jwt/jwt/v5$ + # AS-1 is hosted on ToolHive's embedded authorization server; only the + # authorization server packages it constructs are allowed. + internal-identitybroker-adapter-as1server: + list-mode: strict + files: + - '**/internal/identitybroker/adapter/as1server/**.go' + - '!$test' + allow: + - $gostd + - github.com/stacklok/toolhive/pkg/authserver$ + - github.com/stacklok/toolhive/pkg/authserver/runner$ + - github.com/stacklok/toolhive/pkg/authserver/server/registration$ + - github.com/stacklok/toolhive/pkg/authserver/storage$ + - github.com/stacklok/toolhive/pkg/oauthproto$ # Only the composition root wires the identity broker's adapters; the # rest of Mecatl reaches the broker through its authentication boundary. internal-identitybroker-adapter-importers: diff --git a/cmd/mecak8s/as1_sign_in_harness_test.go b/cmd/mecak8s/as1_sign_in_harness_test.go new file mode 100644 index 0000000000..4845c97cbe --- /dev/null +++ b/cmd/mecak8s/as1_sign_in_harness_test.go @@ -0,0 +1,518 @@ +package main + +import ( + "bytes" + "context" + "crypto/rand" + "errors" + "io" + "maps" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + mecatuiclient "github.com/stacklok/mecatl/cmd/mecatui/client" + mecatlv1 "github.com/stacklok/mecatl/contracts/gen/go/mecatl/v1" + "github.com/stacklok/mecatl/engine/port" + "github.com/stacklok/mecatl/internal/adapter/clientauth" + "github.com/stacklok/mecatl/internal/adapter/credentialstore" + "github.com/stacklok/mecatl/internal/adapter/mcpbroker" + "github.com/stacklok/mecatl/internal/adapter/server" + "github.com/stacklok/mecatl/internal/app" + "github.com/stacklok/mecatl/internal/identitybroker" + "github.com/stacklok/mecatl/internal/testutil/identityharness" + "github.com/stacklok/mecatl/mcp/oauthlogin" +) + +// TestHostedAS1RoutesMountWithoutConflicts pins that the hosted AS-1's routes +// are served anonymously on the normal HTTP listener, that a route colliding +// with another mecak8s route refuses startup, and that nothing answers on +// AS-1's routes when it is not hosted. +func TestHostedAS1RoutesMountWithoutConflicts(t *testing.T) { + built, err := buildIsolated(t, context.Background(), app.Config{ + Workspace: t.TempDir(), UseMock: true, NoSoul: true, NoUserModel: true, Diagnostics: port.NopDiagnostics{}, + }) + if err != nil { + t.Fatalf("app.Build: %v", err) + } + defer built.Close() + auth := server.NewAuthenticator(server.SecurityConfig{AuthToken: "test-token"}) + defer auth.Close() + profile := server.ProtectedResourceProfile{Resource: "https://mecatl.test/api", Issuer: "https://mecatl.test/as1", Audience: "https://mecatl.test/api", ClientID: "mecatui"} + as1Handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte("as1")) }) + mux := func(routes ...string) (http.Handler, error) { + var as1 *app.IdentityBrokerAS1Server + if routes != nil { + as1 = &app.IdentityBrokerAS1Server{Issuer: "https://mecatl.test/as1", Routes: routes, Handler: as1Handler} + } + return normalHTTPMux(built.Service, auth, profile, "", true, builtServing{brokerHandlers: built.MCPBrokerHandlers, brokerCallbackPath: built.MCPBrokerCallbackPath, as1: as1}) + } + + hosted, err := mux("/as1/oauth/token", "/as1/.well-known/openid-configuration") + if err != nil { + t.Fatalf("normalHTTPMux: %v", err) + } + srv := httptest.NewServer(hosted) + defer srv.Close() + for _, path := range []string{"/as1/oauth/token", "/as1/.well-known/openid-configuration"} { + if code := probe(t, srv.URL+path); code != http.StatusOK { + t.Errorf("hosted AS-1 %s = %d, want 200 without credentials", path, code) + } + } + if code := probe(t, srv.URL+"/readyz"); code != http.StatusOK { + t.Errorf("/readyz beside the hosted AS-1 = %d, want 200", code) + } + + for name, route := range map[string]string{ + "health": "/readyz", + "API namespace": "/v1/oauth/token", + "API root": "/v1", + "protected-resource metadata": "/.well-known/oauth-protected-resource/api", + } { + t.Run(name, func(t *testing.T) { + if _, err := mux(route); err == nil || !strings.Contains(err.Error(), "conflicts with a mecak8s route") { + t.Fatalf("normalHTTPMux with AS-1 route %s = %v, want a conflict", route, err) + } + }) + } + + for _, method := range []string{http.MethodConnect, http.MethodTrace} { + t.Run(method+" route", func(t *testing.T) { + methodMux := http.NewServeMux() + methodMux.Handle(method+" /as1/oauth/token", as1Handler) + hosted := &app.IdentityBrokerAS1Server{Issuer: "https://mecatl.test/as1", Routes: []string{"/as1/oauth/token"}, Handler: as1Handler} + if err := mountAS1(methodMux, hosted, server.ProtectedResourceProfile{}); err == nil || !strings.Contains(err.Error(), "conflicts with a mecak8s route") { + t.Fatalf("mountAS1 over a %s-only route = %v, want a conflict", method, err) + } + }) + } + + off, err := mux() + if err != nil { + t.Fatalf("normalHTTPMux: %v", err) + } + offSrv := httptest.NewServer(off) + defer offSrv.Close() + for _, path := range []string{"/as1/oauth/token", "/as1/.well-known/openid-configuration", "/.well-known/openid-configuration", "/oauth/authorize"} { + req, err := http.NewRequest(http.MethodGet, offSrv.URL+path, nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Authorization", "Bearer test-token") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusNotFound { + t.Errorf("%s without a hosted AS-1 = %d, want 404", path, resp.StatusCode) + } + } +} + +// TestHostedAS1ServesBesideTheMCPBroker mounts AS-1 beside a complete +// protected MCP broker bundle through the real broker mount: each answers on +// its own issuer's routes, and an AS-1 route taking the broker's callback path +// refuses startup. +func TestHostedAS1ServesBesideTheMCPBroker(t *testing.T) { + built, err := buildIsolated(t, context.Background(), app.Config{ + Workspace: t.TempDir(), UseMock: true, NoSoul: true, NoUserModel: true, Diagnostics: port.NopDiagnostics{}, + }) + if err != nil { + t.Fatalf("app.Build: %v", err) + } + defer built.Close() + auth := server.NewAuthenticator(server.SecurityConfig{AuthToken: "test-token"}) + defer auth.Close() + answer := func(body string) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(body)) }) + } + broker := answer("broker") + bundle := mcpbroker.HandlerBundle{ + Authorization: broker, Token: broker, UpstreamCallback: broker, Discovery: broker, + JWKS: broker, ProtectedResource: broker, VMCP: broker, Callback: broker, + } + const brokerCallback = "/oauth/callback" + as1Routes := []string{"/as1/oauth/authorize", "/as1/oauth/token", "/as1/oauth/callback", "/as1/.well-known/openid-configuration", "/as1/.well-known/jwks.json"} + as1 := &app.IdentityBrokerAS1Server{Issuer: "https://mecatl.test/as1", Routes: as1Routes, Handler: answer("as1")} + mux, err := normalHTTPMux(built.Service, auth, server.ProtectedResourceProfile{}, "", true, builtServing{brokerHandlers: bundle, brokerCallbackPath: brokerCallback, as1: as1}) + if err != nil { + t.Fatalf("normalHTTPMux: %v", err) + } + srv := httptest.NewServer(mux) + defer srv.Close() + get := func(path string) string { + t.Helper() + resp, err := http.Get(srv.URL + path) + if err != nil { + t.Fatal(err) + } + defer func() { _ = resp.Body.Close() }() + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + return string(body) + } + for _, path := range as1Routes { + if got := get(path); got != "as1" { + t.Errorf("GET %s = %q, want AS-1", path, got) + } + } + for _, path := range []string{ + "/v1/mcp/broker/oauth/authorize", "/v1/mcp/broker/oauth/token", "/v1/mcp/broker/oauth/callback", + "/v1/mcp/broker/.well-known/openid-configuration", "/v1/mcp/broker/.well-known/jwks.json", "/v1/mcp/broker/mcp", brokerCallback, + } { + if got := get(path); got != "broker" { + t.Errorf("GET %s = %q, want the MCP broker", path, got) + } + } + + rootAS1 := &app.IdentityBrokerAS1Server{Issuer: "https://mecatl.test", Routes: []string{"/oauth/authorize", "/oauth/callback"}, Handler: answer("as1")} + if _, err := normalHTTPMux(built.Service, auth, server.ProtectedResourceProfile{}, "", true, builtServing{brokerHandlers: bundle, brokerCallbackPath: brokerCallback, as1: rootAS1}); err == nil || !strings.Contains(err.Error(), "conflicts with a mecak8s route") { + t.Fatalf("normalHTTPMux with AS-1 on the broker's callback path = %v, want a conflict", err) + } +} + +// TestHostedAS1AnonymousSurfaceIsBounded pins the limits on AS-1's anonymous +// routes: an oversized query never reaches AS-1, where /oauth/authorize would +// store it, and the body is capped. +func TestHostedAS1AnonymousSurfaceIsBounded(t *testing.T) { + built, err := buildIsolated(t, context.Background(), app.Config{ + Workspace: t.TempDir(), UseMock: true, NoSoul: true, NoUserModel: true, Diagnostics: port.NopDiagnostics{}, + }) + if err != nil { + t.Fatalf("app.Build: %v", err) + } + defer built.Close() + auth := server.NewAuthenticator(server.SecurityConfig{AuthToken: "test-token"}) + defer auth.Close() + var reached atomic.Int32 + var bodyCapped atomic.Bool + as1Handler := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + reached.Add(1) + if _, err := io.ReadAll(r.Body); err != nil { + var tooLarge *http.MaxBytesError + bodyCapped.Store(errors.As(err, &tooLarge)) + } + }) + as1 := &app.IdentityBrokerAS1Server{Issuer: "https://mecatl.test/as1", Routes: []string{"/as1/oauth/authorize", "/as1/oauth/token"}, Handler: as1Handler} + mux, err := normalHTTPMux(built.Service, auth, server.ProtectedResourceProfile{}, "", true, builtServing{brokerHandlers: built.MCPBrokerHandlers, brokerCallbackPath: built.MCPBrokerCallbackPath, as1: as1}) + if err != nil { + t.Fatalf("normalHTTPMux: %v", err) + } + srv := httptest.NewServer(mux) + defer srv.Close() + + if code := probe(t, srv.URL+"/as1/oauth/authorize?state="+strings.Repeat("s", as1MaxQueryBytes)); code != http.StatusRequestURITooLong { + t.Errorf("authorize with an oversized query = %d, want 414", code) + } + if n := reached.Load(); n != 0 { + t.Fatalf("an oversized query reached AS-1 %d times", n) + } + resp, err := http.Post(srv.URL+"/as1/oauth/token", "application/x-www-form-urlencoded", strings.NewReader(strings.Repeat("a", as1MaxBodyBytes+1))) + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + if !bodyCapped.Load() { + t.Error("an oversized body was not capped") + } +} + +// TestHostedAS1RateLimitsAreGlobalPerSurface pins AS-1's rate limits: one +// peer, as every user is behind an ingress, gets each whole burst, and the +// browser sign-in routes and the token endpoint have separate buckets, so +// exhausting either leaves the other serving. +func TestHostedAS1RateLimitsAreGlobalPerSurface(t *testing.T) { + const authorize, callback, token = "/as1/oauth/authorize", "/as1/oauth/callback", "/as1/oauth/token" + serve := func(t *testing.T) string { + mux := http.NewServeMux() + ok := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) + as1 := &app.IdentityBrokerAS1Server{Issuer: "https://mecatl.test/as1", Routes: []string{authorize, callback, token}, Handler: ok} + if err := mountAS1(mux, as1, server.ProtectedResourceProfile{}); err != nil { + t.Fatalf("mountAS1: %v", err) + } + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv.URL + } + // exhaust sends requests to path until a 429 and returns how many passed. + // The bucket refills while it runs, so at least its burst passes. + exhaust := func(t *testing.T, target string, burst int) int { + t.Helper() + for passed := range 10 * burst { + switch code := probe(t, target); code { + case http.StatusTooManyRequests: + return passed + case http.StatusOK: + default: + t.Fatalf("GET %s = %d, want 200 or 429", target, code) + } + } + t.Fatalf("GET %s: no 429 after %d requests", target, 10*burst) + return 0 + } + + t.Run("sign-in flood", func(t *testing.T) { + base := serve(t) + if passed := exhaust(t, base+authorize, as1SignInBurst); passed < as1SignInBurst { + t.Fatalf("%d sign-in requests from one peer passed before a 429, want the global burst of %d", passed, as1SignInBurst) + } + if passed := exhaust(t, base+callback, as1SignInBurst); passed > 5 { + t.Errorf("%d callback requests passed after authorize exhausted the sign-in bucket, want it shared", passed) + } + if passed := exhaust(t, base+token, as1TokenBurst); passed < as1TokenBurst { + t.Errorf("%d token requests passed while sign-in was exhausted, want the token burst of %d", passed, as1TokenBurst) + } + }) + t.Run("token flood", func(t *testing.T) { + base := serve(t) + if passed := exhaust(t, base+token, as1TokenBurst); passed < as1TokenBurst { + t.Fatalf("%d token requests from one peer passed before a 429, want the global burst of %d", passed, as1TokenBurst) + } + if passed := exhaust(t, base+authorize, as1SignInBurst); passed < as1SignInBurst { + t.Errorf("%d sign-in requests passed while the token endpoint was exhausted, want the sign-in burst of %d", passed, as1SignInBurst) + } + }) +} + +// TestHostedAS1SignsTheTUIIn hosts AS-1 in mecak8s, with the harness IdP as +// its upstream, and signs the TUI's real client code in there: mecak8s's +// protected-resource metadata names AS-1, the token AS-1 issues passes +// mecak8s's sub_id checks for each session create, each with a refreshed +// token, and session open converts nothing at AS-1. With a signing key file, +// a token AS-1 issued before a restart still passes after it. +func TestHostedAS1SignsTheTUIIn(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + dir := t.TempDir() + certFile, keyFile := h.WriteServerKeyPair(t, dir) + caFile := h.WriteCACert(t, dir) + httpAddr := freeLoopbackPort(t) + issuer := "https://" + httpAddr + "/as1" + secret := rand.Text() + secretFile := filepath.Join(dir, "upstream-client-secret") + if err := os.WriteFile(secretFile, []byte(secret), 0o600); err != nil { + t.Fatal(err) + } + h.IdP.RegisterClient("mecak8s-as1", secret, issuer+"/oauth/callback") + cfg, err := parseFlags([]string{ + "--mock", + "--enable-identity-broker", "--no-shell", + "--permission-mode=yolo", "--guardrails=off", + "--grpc-addr", freeLoopbackPort(t), + "--http-addr", httpAddr, + "--drain-addr", freeLoopbackPort(t), + "--session-lease-k8s-namespace", "", + "--tls-cert", certFile, "--tls-key", keyFile, + "--identity-broker-as1-issuer", issuer, + "--identity-broker-as1-signing-key-file", h.WriteSigningKey(t, dir), + "--identity-broker-as1-tui-client-id", identityharness.DefaultClientID, + "--identity-broker-as1-upstream-issuer", h.IdP.Issuer(), + "--identity-broker-as1-upstream-client-id", "mecak8s-as1", + "--identity-broker-as1-upstream-client-secret-file", secretFile, + "--identity-broker-subject-claim", "sub", + "--oidc-audience", identityharness.DefaultAudience, + "--oidc-resource", "https://mecatl.test", + "--oidc-owner-identity=sub_id", + }) + if err != nil { + t.Fatalf("parseFlags: %v", err) + } + cfg.diagnostics = port.NopDiagnostics{} + httpsClient := h.HTTPClient() + // as1Grants counts the grant types of token requests that reach the + // hosted AS-1 over the listener. + var as1Mu sync.Mutex + as1Grants := map[string]int{} + + // start builds and serves mecak8s as run does, and returns its broker and + // a function that shuts it down. + start := func() (identitybroker.Service, func()) { + composition := appConfig(cfg, port.NopDiagnostics{}, observability{}) + composition.IdentityBrokerHostedAS1.UpstreamCAFile = caFile + composition.IdentityBrokerHostedAS1.UpstreamAllowPrivateAddresses = true + built, err := buildIsolated(t, context.Background(), composition) + if err != nil { + t.Fatalf("app.Build: %v", err) + } + hosted := *built.IdentityBrokerHostedAS1 + hosted.Handler = recordGrants(hosted.Handler, func(grant string) { + as1Mu.Lock() + defer as1Mu.Unlock() + as1Grants[grant]++ + }) + ctx, cancel := context.WithCancel(context.Background()) + serveErr := make(chan error, 1) + go func() { + serveErr <- serveWithDrainWait(ctx, cfg, built.Service, observability{}, func(time.Duration) {}, builtServing{brokerHandlers: built.MCPBrokerHandlers, brokerCallbackPath: built.MCPBrokerCallbackPath, as1: &hosted}) + }() + waitForHTTPStatus(t, httpsClient, "https://"+httpAddr+"/readyz", http.StatusOK) + return built.IdentityBroker, func() { + cancel() + select { + case err := <-serveErr: + if err != nil { + t.Errorf("serve: %v", err) + } + case <-time.After(10 * time.Second): + t.Error("serve did not stop after cancellation") + } + built.Close() + } + } + broker, stop := start() + stopped := false + defer func() { + if !stopped { + stop() + } + }() + + profile := fetchProtectedResourceProfile(t, httpsClient, "https://"+httpAddr+"/.well-known/oauth-protected-resource") + if len(profile.AuthorizationServers) != 1 || profile.AuthorizationServers[0] != issuer || profile.Audience != identityharness.DefaultAudience || + profile.ClientID != identityharness.DefaultClientID || !profile.IdentityBrokerSignIn { + t.Fatalf("protected-resource metadata = %+v, want the hosted AS-1 as the only authorization server, its audience and TUI client, and broker sign-in", profile) + } + identity := clientauth.Identity{ + Target: cfg.grpcAddr, Issuer: profile.AuthorizationServers[0], ClientID: profile.ClientID, + Audience: profile.Audience, RedirectURI: oauthlogin.ExactRedirectURL, Scopes: []string{"openid", "offline_access"}, + } + asClient := h.HTTPClient() + tokenRequests := &resourceRecorder{next: asClient.Transport} + asClient.Transport = tokenRequests + presenter := clientauth.PresenterFunc(func(ctx context.Context, authorizationURL string) (oauthlogin.Result, error) { + back, err := h.AuthorizeWithConsent(ctx, authorizationURL) + if err != nil { + return oauthlogin.Result{}, err + } + if back.Get("error") != "" { + return oauthlogin.Result{}, errors.New("authorization refused: " + back.Get("error")) + } + return oauthlogin.Result{Code: back.Get("code"), State: back.Get("state"), Iss: back.Get("iss")}, nil + }) + login := clientauth.LoginConfig{Identity: identity, Presenter: presenter, HTTPClient: asClient, IdentityBrokerSignIn: profile.IdentityBrokerSignIn} + token, err := clientauth.Login(t.Context(), login) + if err != nil { + t.Fatalf("Login: %v", err) + } + store, err := credentialstore.NewMemoryBackend().Open("hosted-as1-sign-in") + if err != nil { + t.Fatal(err) + } + creds, err := clientauth.NewCredentials(store) + if err != nil { + t.Fatal(err) + } + if _, err := creds.Save(t.Context(), identity, token, nil); err != nil { + t.Fatalf("save credential: %v", err) + } + registry, err := clientauth.OpenRegistry(t.TempDir()) + if err != nil { + t.Fatal(err) + } + login.Presenter, login.Registry = nil, registry + source, err := clientauth.NewRefreshSource(t.Context(), creds, login) + if err != nil { + t.Fatalf("NewRefreshSource: %v", err) + } + defer func() { _ = source.Close() }() + recorder := &refreshRecorder{source: source} + tui, err := mecatuiclient.Dial(mecatuiclient.DialConfig{Server: cfg.grpcAddr, UseTLS: true, TLSCAFile: caFile, TokenSource: recorder, IdentityBrokerSignIn: profile.IdentityBrokerSignIn}) + if err != nil { + t.Fatalf("Dial: %v", err) + } + defer func() { _ = tui.Close() }() + + aliceCompany := identitybroker.Owner{Issuer: h.IdP.Issuer(), Subject: identityharness.Alice} + checkSession := func(broker identitybroker.Service, id string) { + t.Helper() + brokerSession, ok, err := broker.FindSession(t.Context(), identitybroker.FindSessionRequest{RootHostSessionID: id}) + if err != nil || !ok || brokerSession.Owner != aliceCompany { + t.Fatalf("session %s broker session: found=%v err=%v owner=%+v; want one owned by %+v", id, ok, err, brokerSession.Owner, aliceCompany) + } + // Session open is unchanged: the broker is not pointed at the hosted + // AS-1, so it converts no token and holds no refresh token. + if brokerSession.HoldsRefreshToken { + t.Fatalf("session %s broker session holds a refresh token; session open must not convert at the hosted AS-1", id) + } + } + for range 2 { + id, _, _, err := tui.CreateSession(t.Context(), mecatlv1.PermissionMode_PERMISSION_MODE_UNSPECIFIED, mecatuiclient.ModelSelection{}) + if err != nil { + t.Fatalf("CreateSession through the TUI client: %v", err) + } + checkSession(broker, id) + } + jtis := recorder.refreshedJTIs() + if len(jtis) != 2 || jtis[0] == "" || jtis[0] == jtis[1] { + t.Fatalf("session creates refreshed %d times with distinct jti=%v; want 2 distinct", len(jtis), len(jtis) == 2 && jtis[0] != jtis[1]) + } + if grants := tokenRequests.grantsWithResource(profile.Audience); grants["authorization_code"] != 1 || grants["refresh_token"] < 2 || grants["missing"] != 0 { + t.Fatalf("token requests naming the audience as resource = %v, want the code exchange and every refresh", grants) + } + if got := h.IdP.RefreshTokensIssued(identityharness.Alice); got != 1 { + t.Fatalf("upstream refresh tokens issued = %d, want 1: AS-1 asks the IdP for offline_access with prompt=consent", got) + } + as1Mu.Lock() + reachedAS1 := maps.Clone(as1Grants) + as1Mu.Unlock() + if reachedAS1["authorization_code"] != 1 || reachedAS1[tokenExchangeGrant] != 0 { + t.Fatalf("token requests at the hosted AS-1 by grant = %v, want the code exchange and no token exchange", reachedAS1) + } + + // A token AS-1 issued before a restart, never presented, still passes + // after it: the signing key comes from the file. + beforeRestart, err := source.Refresh(t.Context()) + if err != nil { + t.Fatalf("Refresh: %v", err) + } + _ = tui.Close() + stop() + stopped = true + broker, stop = start() + stopped = false + restarted, err := mecatuiclient.Dial(mecatuiclient.DialConfig{Server: cfg.grpcAddr, UseTLS: true, TLSCAFile: caFile, AuthToken: beforeRestart}) + if err != nil { + t.Fatalf("Dial after restart: %v", err) + } + defer func() { _ = restarted.Close() }() + id, _, _, err := restarted.CreateSession(t.Context(), mecatlv1.PermissionMode_PERMISSION_MODE_UNSPECIFIED, mecatuiclient.ModelSelection{}) + if err != nil { + t.Fatalf("CreateSession after restart with a token issued before it: %v", err) + } + checkSession(broker, id) + as1Mu.Lock() + defer as1Mu.Unlock() + if as1Grants[tokenExchangeGrant] != 0 { + t.Fatalf("token requests at the hosted AS-1 by grant = %v after restart, want no token exchange", as1Grants) + } +} + +// tokenExchangeGrant is the RFC 8693 grant type session open would use. +const tokenExchangeGrant = "urn:ietf:params:oauth:grant-type:token-exchange" + +// recordGrants passes each token-endpoint request's grant_type to record +// before next serves it. +func recordGrants(next http.Handler, record func(grant string)) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/oauth/token") { + body, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, "unreadable body", http.StatusBadRequest) + return + } + r.Body = io.NopCloser(bytes.NewReader(body)) + form, _ := url.ParseQuery(string(body)) + record(form.Get("grant_type")) + } + next.ServeHTTP(w, r) + }) +} diff --git a/cmd/mecak8s/flags.go b/cmd/mecak8s/flags.go index c88e145a45..fd72a7e240 100644 --- a/cmd/mecak8s/flags.go +++ b/cmd/mecak8s/flags.go @@ -44,6 +44,7 @@ import ( "github.com/stacklok/mecatl/internal/app" "github.com/stacklok/mecatl/internal/cliconfig" "github.com/stacklok/mecatl/internal/flaghelp" + "github.com/stacklok/mecatl/mcp/oauthlogin" ) // k8s-native listen defaults. A pod binds 0.0.0.0 (not loopback — the @@ -309,6 +310,12 @@ type config struct { // enableIdentityBroker starts the experimental agent identity broker. enableIdentityBroker bool + // as1 hosts AS-1 beside the identity broker when its issuer is set. + as1 as1Flags + // identityBrokerSubjectClaim is the IdP claim naming the company + // identity, explicitly set or not. + identityBrokerSubjectClaim string + identityBrokerSubjectClaimSet bool // Scheduled tasks (issue #189): the in-process scheduler. // mecak8s is the multi-replica home — the leader-lease (the k8s session-lease @@ -346,6 +353,16 @@ type config struct { installationID string } +// as1Flags are the --identity-broker-as1-* flags. +type as1Flags struct { + issuer string + signingKeyFile string + tuiClientID string + upstreamIssuer string + upstreamClientID string + upstreamClientSecretFile string +} + // stringList is a repeatable string flag.Value, preserving order across // multiple occurrences (mirrors cmd/mecated's stringList). type stringList []string @@ -531,6 +548,13 @@ func parseFlags(argv []string) (config, error) { fs.BoolVar(&cfg.enableParallel, "enable-parallel", false, "Enable the Parallel tool for isolated child branches") fs.BoolVar(&cfg.enableTeams, "enable-teams", false, "Enable the experimental agent-team capability") fs.BoolVar(&cfg.enableIdentityBroker, "enable-identity-broker", false, "Start the experimental agent identity broker, which gives each signed-in user's session a broker session. Supports a single replica only, requires no shell in this process (--no-shell, or --shell \"\" with Kubernetes execution, which keeps the remote Shell), and blocks the tool calls of agent definitions that have hooks") + fs.StringVar(&cfg.as1.issuer, "identity-broker-as1-issuer", "", "HTTPS issuer URL of AS-1, the identity broker's sign-in authorization server, hosted under the URL's path on the HTTP listener. Requires --enable-identity-broker, --oidc-audience, --oidc-resource, and --oidc-owner-identity=sub_id; sets --oidc-issuer, --oidc-client-id, and --oidc-accepted-client-ids when unset, and refuses other values. AS-1 keeps sign-ins and refresh tokens in memory. Empty does not host AS-1") + fs.StringVar(&cfg.as1.signingKeyFile, "identity-broker-as1-signing-key-file", "", "PEM private key (EC P-256 or RSA) AS-1 signs access tokens with. Empty generates a key at startup, so access tokens stop verifying when mecak8s restarts") + fs.StringVar(&cfg.as1.tuiClientID, "identity-broker-as1-tui-client-id", "", "OAuth client ID of mecatui at AS-1, a public client. Required with --identity-broker-as1-issuer") + fs.StringVar(&cfg.as1.upstreamIssuer, "identity-broker-as1-upstream-issuer", "", "HTTPS issuer URL of the OIDC identity provider users sign in at through AS-1. Required with --identity-broker-as1-issuer") + fs.StringVar(&cfg.as1.upstreamClientID, "identity-broker-as1-upstream-client-id", "", "AS-1's client ID at the upstream identity provider, registered with the redirect URI ISSUER/oauth/callback. Required with --identity-broker-as1-issuer") + fs.StringVar(&cfg.as1.upstreamClientSecretFile, "identity-broker-as1-upstream-client-secret-file", "", "File holding AS-1's client secret at the upstream identity provider. Empty makes AS-1 a public client there") + fs.StringVar(&cfg.identityBrokerSubjectClaim, "identity-broker-subject-claim", "", "Upstream identity provider ID-token claim that names the company identity, the same for a user in every application: oid for Entra ID, whose sub differs per application, or sub for a provider whose sub does not. Required with, and only allowed with, --identity-broker-as1-issuer") // Headless telemetry (issue #343): OPT-IN. --metrics-addr mounts a // SEPARATE loopback /metrics listener (the admin mux — Prometheus scrape). @@ -588,6 +612,8 @@ func parseFlags(argv []string) (config, error) { cfg.subagentModelRouterSet = true case "product-metrics": cfg.productMetricsSet = true + case "identity-broker-subject-claim": + cfg.identityBrokerSubjectClaimSet = true } markRetentionCLIFlag(&cfg.retentionCLISet, fl.Name) if fl.Name == "schedule-fire-retention" { @@ -686,6 +712,9 @@ func parseFlags(argv []string) (config, error) { return config{}, errors.New("remote execution does not support --enable-parallel or --enable-teams") } } + if err := resolveAS1Flags(&cfg); err != nil { + return config{}, err + } // The broker's owner is the company identity AS-1 witnessed in sub_id; // without this it would be AS-1's own issuer and pseudonymous subject. if cfg.enableIdentityBroker && cfg.oidc.Enabled() && cfg.oidc.OwnerIdentity != oidcauthn.OwnerIdentitySubID { @@ -719,6 +748,67 @@ func parseFlags(argv []string) (config, error) { return cfg, nil } +// resolveAS1Flags checks the hosted AS-1 flags and, with AS-1 on, points +// caller identity at it: mecak8s validates the access tokens AS-1 issues, for +// the TUI client, with the --oidc-audience AS-1 issues them for. AS-1's own +// configuration is checked again when app.Build constructs it. +// +//nolint:gocyclo // the startup refusals are one explicit list. +func resolveAS1Flags(cfg *config) error { + as1, oidc := cfg.as1, &cfg.oidc + if as1.issuer == "" { + if as1 != (as1Flags{}) || cfg.identityBrokerSubjectClaimSet { + return errors.New("--identity-broker-as1-* and --identity-broker-subject-claim require --identity-broker-as1-issuer") + } + return nil + } + switch { + case !cfg.enableIdentityBroker: + return errors.New("--identity-broker-as1-issuer requires --enable-identity-broker") + case as1.tuiClientID == "" || as1.upstreamIssuer == "" || as1.upstreamClientID == "": + return errors.New("--identity-broker-as1-issuer requires --identity-broker-as1-tui-client-id, --identity-broker-as1-upstream-issuer, and --identity-broker-as1-upstream-client-id") + case cfg.identityBrokerSubjectClaim == "": + return errors.New("--identity-broker-as1-issuer requires --identity-broker-subject-claim: oid for Entra ID, whose sub differs per application, or sub for an identity provider whose sub is the same across applications") + case !strings.HasPrefix(as1.issuer, "https://") || !validIssuerIdentifier(as1.issuer): + return errors.New("--identity-broker-as1-issuer must be an https URL with a host and no credentials, query, or fragment") + case !strings.HasPrefix(as1.upstreamIssuer, "https://") || !validIssuerIdentifier(as1.upstreamIssuer): + return errors.New("--identity-broker-as1-upstream-issuer must be an https URL with a host and no credentials, query, or fragment") + case oidc.Audience == "": + return errors.New("--identity-broker-as1-issuer requires --oidc-audience, the Mecatl API audience AS-1 issues access tokens for") + case oidc.Resource == "": + return errors.New("--identity-broker-as1-issuer requires --oidc-resource: mecatui finds AS-1 through the protected-resource metadata") + case oidc.AllowPrivateHTTPSIssuer: + return errors.New("--oidc-allow-private-https-issuer cannot be used with --identity-broker-as1-issuer: mecak8s reaches the AS-1 it hosts without the network") + case oidc.JWKSURI != "": + return errors.New("--oidc-jwks-uri cannot be used with --identity-broker-as1-issuer: AS-1's discovery document supplies its keys") + case oidc.Issuer != "" && oidc.Issuer != as1.issuer: + return errors.New("--oidc-issuer must be unset or equal --identity-broker-as1-issuer") + case oidc.ClientID != "" && oidc.ClientID != as1.tuiClientID: + return errors.New("--oidc-client-id must be unset or equal --identity-broker-as1-tui-client-id") + case len(oidc.AcceptedClientIDs) != 0 && !slices.Contains(oidc.AcceptedClientIDs, as1.tuiClientID): + return errors.New("--oidc-accepted-client-ids must include --identity-broker-as1-tui-client-id") + } + oidc.Issuer, oidc.ClientID = as1.issuer, as1.tuiClientID + if len(oidc.AcceptedClientIDs) == 0 { + oidc.AcceptedClientIDs = []string{as1.tuiClientID} + } + return nil +} + +// identityBrokerAS1Config is AS-1's composition configuration, or nil when +// AS-1 is not hosted. +func identityBrokerAS1Config(cfg config) *app.IdentityBrokerAS1Config { + if cfg.as1.issuer == "" { + return nil + } + return &app.IdentityBrokerAS1Config{ + Issuer: cfg.as1.issuer, Audience: cfg.oidc.Audience, SigningKeyFile: cfg.as1.signingKeyFile, + PublicClients: []app.IdentityBrokerAS1PublicClient{{ID: cfg.as1.tuiClientID, RedirectURIs: []string{oauthlogin.ExactRedirectURL}}}, + UpstreamIssuer: cfg.as1.upstreamIssuer, UpstreamClientID: cfg.as1.upstreamClientID, + UpstreamClientSecretFile: cfg.as1.upstreamClientSecretFile, SubjectClaim: cfg.identityBrokerSubjectClaim, + } +} + func selectMockScript(cfg *config) { if cfg.mockScript != "" { cfg.useMock = true @@ -901,6 +991,7 @@ func appConfig(cfg config, diag port.Diagnostics, obs observability) app.Config EnableParallel: cfg.enableParallel, EnableTeams: cfg.enableTeams, IdentityBroker: cfg.enableIdentityBroker, + IdentityBrokerHostedAS1: identityBrokerAS1Config(cfg), SoulPath: cfg.soulFile, NoSoul: cfg.noSoul, UserModelDir: cfg.userModelDir, diff --git a/cmd/mecak8s/identity_broker_flag_test.go b/cmd/mecak8s/identity_broker_flag_test.go index a143d55030..bd827b8775 100644 --- a/cmd/mecak8s/identity_broker_flag_test.go +++ b/cmd/mecak8s/identity_broker_flag_test.go @@ -1,12 +1,17 @@ package main import ( + "context" "errors" + "net/http" "slices" "strings" "testing" + "github.com/stacklok/mecatl/internal/adapter/server" + "github.com/stacklok/mecatl/internal/app" "github.com/stacklok/mecatl/internal/cliconfig" + "github.com/stacklok/mecatl/mcp/oauthlogin" ) // TestIdentityBrokerRequiresOwnerIdentitySubID pins fail-closed startup: with @@ -61,13 +66,134 @@ func TestOwnerIdentityMisconfigurationRefusesStartup(t *testing.T) { if err != nil { t.Fatalf("parseFlags(%v): %v", args, err) } - if auth, err := newAuthenticator(t.Context(), cfg); auth != nil || !errors.Is(err, cliconfig.ErrOIDCMisconfigured) { + if auth, err := newAuthenticator(t.Context(), cfg, nil); auth != nil || !errors.Is(err, cliconfig.ErrOIDCMisconfigured) { t.Fatalf("newAuthenticator(%v) = (%v, %v), want ErrOIDCMisconfigured", args, auth, err) } }) } } +// as1Args host AS-1 with every required companion flag. +var as1Args = []string{ + "--mock", "--enable-identity-broker", + "--identity-broker-as1-issuer=https://mecatl.example/as1", + "--identity-broker-as1-tui-client-id=mecatui", + "--identity-broker-as1-upstream-issuer=https://idp.example", + "--identity-broker-as1-upstream-client-id=mecatl-as1", + "--identity-broker-subject-claim=sub", + "--oidc-audience=https://mecatl.example/api", + "--oidc-resource=https://mecatl.example", + "--oidc-owner-identity=sub_id", +} + +// TestIdentityBrokerAS1FlagRules pins which hosted AS-1 configurations +// mecak8s starts with, and that caller identity is derived from AS-1's. +func TestIdentityBrokerAS1FlagRules(t *testing.T) { + without := func(drop ...string) []string { + return slices.DeleteFunc(slices.Clone(as1Args), func(arg string) bool { + return slices.ContainsFunc(drop, func(prefix string) bool { return strings.HasPrefix(arg, prefix) }) + }) + } + with := func(extra ...string) []string { return slices.Concat(as1Args, extra) } + for _, c := range []struct { + name string + args []string + want string + }{ + {name: "all companions", args: as1Args}, + {name: "matching explicit caller identity", args: with( + "--oidc-issuer=https://mecatl.example/as1", "--oidc-client-id=mecatui", + "--oidc-accepted-client-ids=studio,mecatui", + "--identity-broker-subject-claim=oid", "--identity-broker-as1-signing-key-file=/keys/as1.pem", + "--identity-broker-as1-upstream-client-secret-file=/secrets/as1")}, + {name: "signing key without issuer", args: []string{"--mock", "--identity-broker-as1-signing-key-file=/keys/as1.pem"}, want: "require --identity-broker-as1-issuer"}, + {name: "TUI client without issuer", args: []string{"--mock", "--identity-broker-as1-tui-client-id=mecatui"}, want: "require --identity-broker-as1-issuer"}, + {name: "upstream issuer without issuer", args: []string{"--mock", "--identity-broker-as1-upstream-issuer=https://idp.example"}, want: "require --identity-broker-as1-issuer"}, + {name: "upstream client without issuer", args: []string{"--mock", "--identity-broker-as1-upstream-client-id=x"}, want: "require --identity-broker-as1-issuer"}, + {name: "upstream secret without issuer", args: []string{"--mock", "--identity-broker-as1-upstream-client-secret-file=/s"}, want: "require --identity-broker-as1-issuer"}, + {name: "subject claim without issuer", args: []string{"--mock", "--identity-broker-subject-claim=oid"}, want: "require --identity-broker-as1-issuer"}, + {name: "without the broker", args: without("--enable-identity-broker"), want: "requires --enable-identity-broker"}, + {name: "without the TUI client", args: without("--identity-broker-as1-tui-client-id"), want: "requires --identity-broker-as1-tui-client-id"}, + {name: "without the upstream issuer", args: without("--identity-broker-as1-upstream-issuer"), want: "requires --identity-broker-as1-tui-client-id"}, + {name: "without the upstream client", args: without("--identity-broker-as1-upstream-client-id"), want: "requires --identity-broker-as1-tui-client-id"}, + {name: "without the subject claim", args: without("--identity-broker-subject-claim"), want: "requires --identity-broker-subject-claim: oid for Entra ID, whose sub differs per application, or sub for an identity provider whose sub is the same across applications"}, + {name: "empty subject claim", args: with("--identity-broker-subject-claim="), want: "requires --identity-broker-subject-claim"}, + {name: "http issuer", args: with("--identity-broker-as1-issuer=http://mecatl.example/as1"), want: "--identity-broker-as1-issuer must be an https URL"}, + {name: "issuer with a query", args: with("--identity-broker-as1-issuer=https://mecatl.example/as1?x=1"), want: "--identity-broker-as1-issuer must be an https URL"}, + {name: "http upstream issuer", args: with("--identity-broker-as1-upstream-issuer=http://idp.example"), want: "--identity-broker-as1-upstream-issuer must be an https URL"}, + {name: "without the audience", args: without("--oidc-audience"), want: "requires --oidc-audience"}, + {name: "without the resource", args: without("--oidc-resource"), want: "requires --oidc-resource"}, + {name: "private HTTPS issuer admission", args: with("--oidc-allow-private-https-issuer", "--oidc-ca-cert-file=/ca.pem"), want: "--oidc-allow-private-https-issuer cannot be used"}, + {name: "another JWKS URI", args: with("--oidc-jwks-uri=https://idp.example/jwks"), want: "--oidc-jwks-uri cannot be used with --identity-broker-as1-issuer"}, + {name: "AS-1's own JWKS URI", args: with("--oidc-jwks-uri=https://mecatl.example/as1/.well-known/jwks.json"), want: "--oidc-jwks-uri cannot be used with --identity-broker-as1-issuer"}, + {name: "another OIDC issuer", args: with("--oidc-issuer=https://idp.example"), want: "--oidc-issuer must be unset or equal"}, + {name: "another client ID", args: with("--oidc-client-id=studio"), want: "--oidc-client-id must be unset or equal"}, + {name: "accepted client IDs without the TUI", args: with("--oidc-accepted-client-ids=studio"), want: "--oidc-accepted-client-ids must include"}, + {name: "without owner identity sub_id", args: without("--oidc-owner-identity"), want: "requires --oidc-owner-identity=sub_id"}, + } { + t.Run(c.name, func(t *testing.T) { + cfg, err := parseFlags(c.args) + if c.want != "" { + if err == nil || !strings.Contains(err.Error(), c.want) { + t.Fatalf("parseFlags(%v) = %v, want error containing %q", c.args, err, c.want) + } + return + } + if err != nil { + t.Fatalf("parseFlags(%v): %v", c.args, err) + } + if cfg.oidc.Issuer != "https://mecatl.example/as1" || cfg.oidc.ClientID != "mecatui" || !slices.Contains(cfg.oidc.AcceptedClientIDs, "mecatui") { + t.Fatalf("caller identity = issuer %q, client %q, accepted %v; want AS-1's", cfg.oidc.Issuer, cfg.oidc.ClientID, cfg.oidc.AcceptedClientIDs) + } + got := appConfig(cfg, nil, observability{}).IdentityBrokerHostedAS1 + if got == nil || got.Issuer != "https://mecatl.example/as1" || got.Audience != "https://mecatl.example/api" || len(got.PublicClients) != 1 || got.PublicClients[0].ID != "mecatui" || + !slices.Equal(got.PublicClients[0].RedirectURIs, []string{oauthlogin.ExactRedirectURL}) || got.UpstreamIssuer != "https://idp.example" || + got.UpstreamClientID != "mecatl-as1" || got.UpstreamCAFile != "" || got.UpstreamAllowPrivateAddresses { + t.Fatalf("IdentityBrokerHostedAS1 = %+v", got) + } + }) + } + cfg, err := parseFlags(as1Args) + if err != nil { + t.Fatal(err) + } + if got := appConfig(cfg, nil, observability{}).IdentityBrokerHostedAS1; got.SubjectClaim != "sub" || got.SigningKeyFile != "" || got.UpstreamClientSecretFile != "" { + t.Fatalf("IdentityBrokerHostedAS1 defaults = %+v, want the explicit subject claim sub and no files", got) + } + cfg, err = parseFlags(with("--identity-broker-subject-claim=oid", "--identity-broker-as1-signing-key-file=/keys/as1.pem", "--identity-broker-as1-upstream-client-secret-file=/secrets/as1")) + if err != nil { + t.Fatal(err) + } + if got := appConfig(cfg, nil, observability{}).IdentityBrokerHostedAS1; got.SubjectClaim != "oid" || got.SigningKeyFile != "/keys/as1.pem" || got.UpstreamClientSecretFile != "/secrets/as1" { + t.Fatalf("IdentityBrokerHostedAS1 = %+v, want the configured subject claim and files", got) + } +} + +// TestNewAuthenticatorFetchesThroughTheHostedAS1 pins that the validator +// fetches through the passed hosted AS-1's in-process client, and through no +// AS-1 client when none is passed. TestHostedAS1SignsTheTUIIn proves the +// in-process fetch end to end. +func TestNewAuthenticatorFetchesThroughTheHostedAS1(t *testing.T) { + cfg, err := parseFlags(as1Args) + if err != nil { + t.Fatal(err) + } + errStop := errors.New("validator construction stopped") + var got *http.Client + cfg.oidc.NewValidator = func(_ context.Context, c cliconfig.OIDCConfig) (server.PrincipalValidator, error) { + got = c.HTTPClient + return nil, errStop + } + hosted := &app.IdentityBrokerAS1Server{Issuer: cfg.oidc.Issuer, HTTPClient: &http.Client{}} + if _, err := newAuthenticator(t.Context(), cfg, hosted); !errors.Is(err, errStop) || got != hosted.HTTPClient { + t.Fatalf("newAuthenticator with the hosted AS-1: err %v, client %p; want the AS-1's client %p", err, got, hosted.HTTPClient) + } + got = &http.Client{} + if _, err := newAuthenticator(t.Context(), cfg, nil); !errors.Is(err, errStop) || got != nil { + t.Fatalf("newAuthenticator without a hosted AS-1: err %v, client %p; want none", err, got) + } +} + func TestIdentityBrokerFlagMapsToConfig(t *testing.T) { for _, c := range []struct { args []string @@ -81,8 +207,8 @@ func TestIdentityBrokerFlagMapsToConfig(t *testing.T) { if err != nil { t.Fatalf("parseFlags(%v): %v", c.args, err) } - if got := appConfig(cfg, nil, observability{}).IdentityBroker; got != c.want { - t.Errorf("parseFlags(%v): IdentityBroker = %v, want %v", c.args, got, c.want) + if got := appConfig(cfg, nil, observability{}); got.IdentityBroker != c.want || got.IdentityBrokerHostedAS1 != nil { + t.Errorf("parseFlags(%v): IdentityBroker = %v, IdentityBrokerHostedAS1 = %+v; want %v and none", c.args, got.IdentityBroker, got.IdentityBrokerHostedAS1, c.want) } } } diff --git a/cmd/mecak8s/identity_broker_harness_test.go b/cmd/mecak8s/identity_broker_harness_test.go index f5b6e7deac..c3540535db 100644 --- a/cmd/mecak8s/identity_broker_harness_test.go +++ b/cmd/mecak8s/identity_broker_harness_test.go @@ -80,10 +80,10 @@ func TestIdentityBrokerAgainstOfflineHarness(t *testing.T) { defer cancel() serveErr := make(chan error, 1) go func() { - serveErr <- serveWithDrainWait(ctx, cfg, built.Service, observability{}, func(time.Duration) {}, built.MCPBrokerHandlers, built.MCPBrokerCallbackPath) + serveErr <- serveWithDrainWait(ctx, cfg, built.Service, observability{}, func(time.Duration) {}, builtServing{brokerHandlers: built.MCPBrokerHandlers, brokerCallbackPath: built.MCPBrokerCallbackPath}) }() api := "http://" + cfg.httpAddr - waitForHTTPStatus(t, api+"/readyz", http.StatusOK) + waitForHTTPStatus(t, http.DefaultClient, api+"/readyz", http.StatusOK) alice, err := h.SignIn(t.Context(), identityharness.SignInRequest{User: identityharness.Alice}) if err != nil { diff --git a/cmd/mecak8s/main.go b/cmd/mecak8s/main.go index 74b8be00c4..db320e65ce 100644 --- a/cmd/mecak8s/main.go +++ b/cmd/mecak8s/main.go @@ -114,5 +114,9 @@ func run() error { defer boundedClose(built.Close, cfg.closeTimeout) defer flushTelemetry(os.Stderr, obs, cfg.otlpShutdownTimeout) - return serve(ctx, cfg, built.Service, obs, built.MCPBrokerHandlers, built.MCPBrokerCallbackPath) + return serve(ctx, cfg, built.Service, obs, builtServing{ + brokerHandlers: built.MCPBrokerHandlers, + brokerCallbackPath: built.MCPBrokerCallbackPath, + as1: built.IdentityBrokerHostedAS1, + }) } diff --git a/cmd/mecak8s/main_test.go b/cmd/mecak8s/main_test.go index ce47bca437..0835f6cfd3 100644 --- a/cmd/mecak8s/main_test.go +++ b/cmd/mecak8s/main_test.go @@ -549,7 +549,7 @@ func TestDrainHTTPRouting(t *testing.T) { auth := server.NewAuthenticator(server.SecurityConfig{AuthToken: "test-token"}) defer auth.Close() - normalMux, err := normalHTTPMux(built.Service, auth, server.ProtectedResourceProfile{}, "", true, built.MCPBrokerHandlers, built.MCPBrokerCallbackPath) + normalMux, err := normalHTTPMux(built.Service, auth, server.ProtectedResourceProfile{}, "", true, builtServing{brokerHandlers: built.MCPBrokerHandlers, brokerCallbackPath: built.MCPBrokerCallbackPath}) if err != nil { t.Fatalf("normalHTTPMux: %v", err) } @@ -646,14 +646,14 @@ func TestServeWiresSeparateDrainListener(t *testing.T) { defer cancel() serveErr := make(chan error, 1) go func() { - serveErr <- serveWithDrainWait(ctx, cfg, built.Service, observability{}, func(time.Duration) {}, built.MCPBrokerHandlers, built.MCPBrokerCallbackPath) + serveErr <- serveWithDrainWait(ctx, cfg, built.Service, observability{}, func(time.Duration) {}, builtServing{brokerHandlers: built.MCPBrokerHandlers, brokerCallbackPath: built.MCPBrokerCallbackPath}) }() - waitForHTTPStatus(t, "http://"+cfg.httpAddr+"/drain", http.StatusNotFound) + waitForHTTPStatus(t, http.DefaultClient, "http://"+cfg.httpAddr+"/drain", http.StatusNotFound) if built.Service.IsDraining() { t.Fatal("normal listener drained the service") } - waitForHTTPStatus(t, "http://"+cfg.drainAddr+"/drain", http.StatusOK) + waitForHTTPStatus(t, http.DefaultClient, "http://"+cfg.drainAddr+"/drain", http.StatusOK) if !built.Service.IsDraining() { t.Fatal("drain listener did not drain the service") } @@ -669,11 +669,11 @@ func TestServeWiresSeparateDrainListener(t *testing.T) { } } -func waitForHTTPStatus(t *testing.T, url string, want int) { +func waitForHTTPStatus(t *testing.T, client *http.Client, url string, want int) { t.Helper() deadline := time.Now().Add(5 * time.Second) for time.Now().Before(deadline) { - resp, err := http.Get(url) + resp, err := client.Get(url) if err == nil { _ = resp.Body.Close() if resp.StatusCode == want { diff --git a/cmd/mecak8s/serve.go b/cmd/mecak8s/serve.go index e6a6924265..8c35182690 100644 --- a/cmd/mecak8s/serve.go +++ b/cmd/mecak8s/serve.go @@ -9,10 +9,13 @@ import ( "log/slog" "net" "net/http" + "net/url" "os/signal" + "strings" "syscall" "time" + "golang.org/x/time/rate" "google.golang.org/grpc" "google.golang.org/grpc/credentials" "google.golang.org/grpc/health" @@ -23,6 +26,7 @@ import ( "github.com/stacklok/mecatl/internal/adapter/server" "github.com/stacklok/mecatl/internal/adapter/telemetry" "github.com/stacklok/mecatl/internal/adapter/tlsreload" + "github.com/stacklok/mecatl/internal/app" "github.com/stacklok/mecatl/internal/cliconfig" ) @@ -95,7 +99,10 @@ func mountBrokerHandlers(mux *http.ServeMux, addr string, verifiedIdentity, owne // non-empty) is mounted in front of the authenticated API, same as mecated — // it must stay reachable without credentials, and outside the drain listener // since it is discovery metadata, not a lifecycle operation. -func normalHTTPMux(svc *server.Service, auth *server.Authenticator, profile server.ProtectedResourceProfile, addr string, authed bool, brokerHandlers mcpbroker.HandlerBundle, brokerCallbackPath string) (http.Handler, error) { +// +// The hosted AS-1's anonymous OAuth routes, when built.as1 is non-nil, are +// mounted after the broker's with the same kind of conflict check. +func normalHTTPMux(svc *server.Service, auth *server.Authenticator, profile server.ProtectedResourceProfile, addr string, authed bool, built builtServing) (http.Handler, error) { ready := server.ReadyFunc(func() bool { if svc.IsDraining() { return false @@ -111,12 +118,90 @@ func normalHTTPMux(svc *server.Service, auth *server.Authenticator, profile serv // Broker routes are mounted LAST, after every other route above, so // HandlerBundle.Mount's own route-conflict check is checked against the // real, fully-populated mux — see mountBrokerHandlers' doc comment. - if err := mountBrokerHandlers(mux, addr, authed, false, brokerHandlers, brokerCallbackPath); err != nil { + if err := mountBrokerHandlers(mux, addr, authed, false, built.brokerHandlers, built.brokerCallbackPath); err != nil { return nil, fmt.Errorf("mount MCP broker handlers: %w", err) } + if err := mountAS1(mux, built.as1, profile); err != nil { + return nil, fmt.Errorf("mount AS-1: %w", err) + } return mux, nil } +// mountAS1 registers the hosted AS-1's routes on mux, which must already hold +// every other route. A route another pattern on mux already serves is a +// conflict, as is one the API behind the "/" fallback owns: its /v1 namespace +// and the protected-resource metadata document. +func mountAS1(mux *http.ServeMux, as1 *app.IdentityBrokerAS1Server, profile server.ProtectedResourceProfile) error { + if as1 == nil { + return nil + } + metadataPath := "" + if profile.Resource != "" { + if u, err := url.Parse(server.WellKnownProtectedResourceURL(profile.Resource)); err == nil { + metadataPath = u.Path + } + } + conflicts := func(route string) bool { + if route == "/v1" || strings.HasPrefix(route, "/v1/") || route == metadataPath { + return true + } + for _, method := range []string{http.MethodGet, http.MethodHead, http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete, http.MethodConnect, http.MethodOptions, http.MethodTrace} { + if _, pattern := mux.Handler(&http.Request{Method: method, URL: &url.URL{Path: route}}); pattern != "" && pattern != "/" { + return true + } + } + return false + } + for _, route := range as1.Routes { + if conflicts(route) { + return fmt.Errorf("route %q conflicts with a mecak8s route; choose another --identity-broker-as1-issuer path", route) + } + } + signIn := limitAS1(as1.Handler, rate.NewLimiter(as1SignInRatePerSec, as1SignInBurst)) + token := limitAS1(as1.Handler, rate.NewLimiter(as1TokenRatePerSec, as1TokenBurst)) + for _, route := range as1.Routes { + if strings.HasSuffix(route, "/oauth/token") { + mux.Handle(route, token) + } else { + mux.Handle(route, signIn) + } + } + return nil +} + +// Bounds on AS-1's anonymous routes. /oauth/authorize stores a pending +// authorization before anyone signs in, so request size and rate bound the +// memory an unauthenticated caller can make AS-1 hold. mecak8s does not trust +// forwarded headers, so behind an ingress every user has the same peer +// address: the rates are global, with the browser sign-in routes and the +// token endpoint in separate buckets so a flood of one cannot starve the +// other. +const ( + as1MaxQueryBytes = 8 << 10 + as1MaxBodyBytes = 64 << 10 + as1SignInRatePerSec = 50 + as1SignInBurst = 200 + as1TokenRatePerSec = 50 + as1TokenBurst = 200 +) + +// limitAS1 refuses a request limiter has no room for with 429, refuses an +// oversized query, and caps the request body. +func limitAS1(next http.Handler, limiter *rate.Limiter) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !limiter.Allow() { + http.Error(w, "rate limit exceeded", http.StatusTooManyRequests) + return + } + if len(r.URL.RawQuery) > as1MaxQueryBytes { + http.Error(w, "query too long", http.StatusRequestURITooLong) + return + } + r.Body = http.MaxBytesReader(w, r.Body, as1MaxBodyBytes) + next.ServeHTTP(w, r) + }) +} + func drainHTTPMux(svc *server.Service, wait func(time.Duration)) http.Handler { mux := http.NewServeMux() mux.HandleFunc("/drain", func(w http.ResponseWriter, r *http.Request) { @@ -179,15 +264,22 @@ func startMetricsServer(addr string, obs observability, errCh chan<- error) (*ht return metricsSrv, metricsLis.Addr(), nil } -func serve(ctx context.Context, cfg config, svc *server.Service, obs observability, brokerHandlers mcpbroker.HandlerBundle, brokerCallbackPath string) error { - return serveWithDrainWait(ctx, cfg, svc, obs, time.Sleep, brokerHandlers, brokerCallbackPath) +// builtServing is what serve takes from app.Build. +type builtServing struct { + brokerHandlers mcpbroker.HandlerBundle + brokerCallbackPath string + as1 *app.IdentityBrokerAS1Server } -func serveWithDrainWait(ctx context.Context, cfg config, svc *server.Service, obs observability, drainWait func(time.Duration), brokerHandlers mcpbroker.HandlerBundle, brokerCallbackPath string) error { - return serveWithHooks(ctx, cfg, svc, obs, drainWait, nil, brokerHandlers, brokerCallbackPath) +func serve(ctx context.Context, cfg config, svc *server.Service, obs observability, built builtServing) error { + return serveWithDrainWait(ctx, cfg, svc, obs, time.Sleep, built) } -func serveWithHooks(ctx context.Context, cfg config, svc *server.Service, obs observability, drainWait func(time.Duration), metricsListening func(net.Addr), brokerHandlers mcpbroker.HandlerBundle, brokerCallbackPath string) error { +func serveWithDrainWait(ctx context.Context, cfg config, svc *server.Service, obs observability, drainWait func(time.Duration), built builtServing) error { + return serveWithHooks(ctx, cfg, svc, obs, drainWait, nil, built) +} + +func serveWithHooks(ctx context.Context, cfg config, svc *server.Service, obs observability, drainWait func(time.Duration), metricsListening func(net.Addr), built builtServing) error { tlsCfg, tlsLifecycle, err := buildTLSConfig(cfg) if err != nil { return err @@ -200,7 +292,7 @@ func serveWithHooks(ctx context.Context, cfg config, svc *server.Service, obs ob } } - auth, err := newAuthenticator(ctx, cfg) + auth, err := newAuthenticator(ctx, cfg, built.as1) if err != nil { return err } @@ -227,7 +319,7 @@ func serveWithHooks(ctx context.Context, cfg config, svc *server.Service, obs ob // A bearer token, OIDC, or verified mTLS authenticates callers. Ordinary // server TLS only authenticates the server. authed := callerAuthenticationConfigured(cfg, tlsCfg) - normalMux, err := normalHTTPMux(svc, auth, protectedResourceProfile(cfg.oidc), cfg.httpAddr, authed, brokerHandlers, brokerCallbackPath) + normalMux, err := normalHTTPMux(svc, auth, protectedResourceProfile(cfg.oidc), cfg.httpAddr, authed, built) if err != nil { return err } @@ -319,14 +411,22 @@ func protectedResourceProfile(c cliconfig.OIDCConfig) server.ProtectedResourcePr // newAuthenticator constructs the caller-identity boundary with the server-root // context so its JWKS refresh survives individual requests. A broken OIDC setup // is fatal: the pod must not silently serve unauthenticated traffic. -func newAuthenticator(ctx context.Context, cfg config) (*server.Authenticator, error) { +// +// A non-nil as1 is the hosted AS-1, whose discovery document and keys the +// validator fetches in process: listeners are not serving yet. Without it a +// hosted issuer is fetched over the network, which fails startup closed. +func newAuthenticator(ctx context.Context, cfg config, as1 *app.IdentityBrokerAS1Server) (*server.Authenticator, error) { // Log before construction so an insecure test-only relaxation is visible even // when validator construction then fails. warnInsecureIssuer(cfg.oidc) if err := cliconfig.ValidateOIDCAuthToken(cfg.oidc, cfg.authToken); err != nil { return nil, err } - validator, err := cliconfig.OIDCValidator(ctx, cfg.oidc) + oidc := cfg.oidc + if as1 != nil { + oidc.HTTPClient = as1.HTTPClient + } + validator, err := cliconfig.OIDCValidator(ctx, oidc) if err != nil { return nil, err } diff --git a/cmd/mecak8s/telemetry_test.go b/cmd/mecak8s/telemetry_test.go index cbaa11bed2..344f6547f2 100644 --- a/cmd/mecak8s/telemetry_test.go +++ b/cmd/mecak8s/telemetry_test.go @@ -277,7 +277,7 @@ func TestTelemetryMetricsAddrServesPrometheus(t *testing.T) { defer cancel() serveErr = serveWithHooks(ctx, cfg, built.Service, obs, time.Sleep, func(addr net.Addr) { metricsListening <- addr - }, built.MCPBrokerHandlers, built.MCPBrokerCallbackPath) + }, builtServing{brokerHandlers: built.MCPBrokerHandlers, brokerCallbackPath: built.MCPBrokerCallbackPath}) }() defer func() { cancel() @@ -367,7 +367,7 @@ func TestTelemetryMetricsAddrBindFailure(t *testing.T) { listening := false err = serveWithHooks(ctx, cfg, built.Service, obs, time.Sleep, func(net.Addr) { listening = true - }, built.MCPBrokerHandlers, built.MCPBrokerCallbackPath) + }, builtServing{brokerHandlers: built.MCPBrokerHandlers, brokerCallbackPath: built.MCPBrokerCallbackPath}) if err == nil || !strings.Contains(err.Error(), "listen metrics") || !strings.Contains(err.Error(), addr) { t.Fatalf("serve with occupied metrics address = %v, want listen metrics error naming %s", err, addr) } @@ -459,7 +459,7 @@ func TestTelemetryPushesRunMetricsOnExit(t *testing.T) { defer cancel() serveErr := make(chan error, 1) go func() { - serveErr <- serve(ctx, cfg, built.Service, obs, built.MCPBrokerHandlers, built.MCPBrokerCallbackPath) + serveErr <- serve(ctx, cfg, built.Service, obs, builtServing{brokerHandlers: built.MCPBrokerHandlers, brokerCallbackPath: built.MCPBrokerCallbackPath}) }() cancel() select { diff --git a/cmd/mecak8s/tui_sign_in_harness_test.go b/cmd/mecak8s/tui_sign_in_harness_test.go index aa5a5493fc..c9cc8a4c3d 100644 --- a/cmd/mecak8s/tui_sign_in_harness_test.go +++ b/cmd/mecak8s/tui_sign_in_harness_test.go @@ -66,13 +66,13 @@ type protectedResourceDocument struct { IdentityBrokerSignIn bool `json:"com.stacklok.mecatl.identity_broker_sign_in"` } -func fetchProtectedResourceProfile(t *testing.T, metadataURL string) protectedResourceDocument { +func fetchProtectedResourceProfile(t *testing.T, client *http.Client, metadataURL string) protectedResourceDocument { t.Helper() req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, metadataURL, nil) if err != nil { t.Fatal(err) } - res, err := http.DefaultClient.Do(req) + res, err := client.Do(req) if err != nil { t.Fatalf("fetch protected-resource metadata: %v", err) } @@ -169,13 +169,13 @@ func TestTUISignsInAtAS1AndOpensEachSessionWithAFreshToken(t *testing.T) { defer cancel() serveErr := make(chan error, 1) go func() { - serveErr <- serveWithDrainWait(ctx, cfg, built.Service, observability{}, func(time.Duration) {}, built.MCPBrokerHandlers, built.MCPBrokerCallbackPath) + serveErr <- serveWithDrainWait(ctx, cfg, built.Service, observability{}, func(time.Duration) {}, builtServing{brokerHandlers: built.MCPBrokerHandlers, brokerCallbackPath: built.MCPBrokerCallbackPath}) }() - waitForHTTPStatus(t, "http://"+cfg.httpAddr+"/readyz", http.StatusOK) + waitForHTTPStatus(t, http.DefaultClient, "http://"+cfg.httpAddr+"/readyz", http.StatusOK) // The client learns its sign-in profile, including whether to send the // RFC 8707 resource, from mecak8s's protected-resource metadata. - profile := fetchProtectedResourceProfile(t, "http://"+cfg.httpAddr+"/.well-known/oauth-protected-resource") + profile := fetchProtectedResourceProfile(t, http.DefaultClient, "http://"+cfg.httpAddr+"/.well-known/oauth-protected-resource") if len(profile.AuthorizationServers) != 1 || profile.AuthorizationServers[0] != h.AS1.Issuer() || profile.Audience != h.AS1.Audience() || profile.ClientID != identityharness.DefaultClientID { t.Fatalf("protected-resource metadata = %+v, want the AS-1 issuer, audience, and client", profile) } diff --git a/deploy/helm/mecak8s/chart_test.go b/deploy/helm/mecak8s/chart_test.go index 83d6e86eae..9e7b6f1643 100644 --- a/deploy/helm/mecak8s/chart_test.go +++ b/deploy/helm/mecak8s/chart_test.go @@ -643,21 +643,21 @@ func TestMecak8sHelmChart_OwnerIdentity(t *testing.T) { t.Fatalf("template: render = %v, want failure %q:\n%s", err, wantUnknown, output) } - // The identity broker, enabled through extraArgs, needs the owner identity - // when OIDC is on; mecak8s refuses it at startup, so the chart fails first. - for _, arg := range []string{"--enable-identity-broker", "-enable-identity-broker", "--enable-identity-broker=true", "--enable-identity-broker=1"} { - want := fmt.Sprintf("extraArgs entry %q enables the identity broker, which requires oidc.ownerIdentity=sub_id when oidc.enabled is true", arg) - if output, err := helm(t, append(slices.Clone(base), "--set-string", "extraArgs[0]="+arg)...); err == nil || !strings.Contains(output, want) { - t.Fatalf("broker %s without owner identity: render = %v, want failure %q:\n%s", arg, err, want, output) - } - } - for name, extra := range map[string][]string{ - "broker explicitly off": {"--set-string", "extraArgs[0]=--enable-identity-broker=false"}, - "broker with identity": {"--set-string", "extraArgs[0]=--enable-identity-broker", "--set", "oidc.ownerIdentity=sub_id", "--set", "oidc.acceptedClientIDs={mecatui}"}, - "broker without OIDC on": {"--set-string", "extraArgs[0]=--enable-identity-broker", "--set", "oidc.enabled=false"}, - } { - if output, err := helm(t, append(slices.Clone(base), extra...)...); err != nil { - t.Fatalf("%s: render failed: %v\n%s", name, err, output) + // The identity broker is enabled only through identityBroker.enabled, which + // also renders its single replica: extraArgs may not enable it in any + // spelling, whatever the owner identity, OIDC, or identityBroker.enabled. + for _, arg := range []string{"--enable-identity-broker", "-enable-identity-broker", "--enable-identity-broker=true", "-enable-identity-broker=1", "--enable-identity-broker=false"} { + want := fmt.Sprintf("extraArgs entry %q enables the identity broker without the single replica it needs; remove it and set identityBroker.enabled=true", arg) + for name, extra := range map[string][]string{ + "without owner identity": nil, + "with owner identity": {"--set", "oidc.ownerIdentity=sub_id", "--set", "oidc.acceptedClientIDs={mecatui}"}, + "without OIDC": {"--set", "oidc.enabled=false"}, + "with the broker on": {"--set", "oidc.ownerIdentity=sub_id", "--set", "oidc.acceptedClientIDs={mecatui}", "--set", "identityBroker.enabled=true,replicaCount=1", "--set-string", "extraArgs[1]=--no-shell"}, + } { + args := slices.Concat(base, []string{"--set-string", "extraArgs[0]=" + arg}, extra) + if output, err := helm(t, args...); err == nil || !strings.Contains(output, want) { + t.Fatalf("extraArgs %s %s: render = %v, want failure %q:\n%s", arg, name, err, want, output) + } } } @@ -690,6 +690,156 @@ func TestMecak8sHelmChart_OwnerIdentity(t *testing.T) { } } +func TestMecak8sHelmChart_IdentityBrokerAS1(t *testing.T) { + base := []string{"template", "as1", ".", "--set", "mockProvider=true,permissionMode=yolo,replicaCount=1", "--set", "redis.local.enabled=true", + "--set", "extraArgs={--no-shell}", + "--set", "oidc.enabled=true,oidc.audience=mecatl,oidc.ownerIdentity=sub_id,oidc.resource=https://mecatl.example.com", + "--set", "identityBroker.enabled=true,identityBroker.subjectClaim=sub", + "--set", "identityBroker.as1.issuer=https://mecatl.example.com/as1,identityBroker.as1.tuiClientID=mecatui", + "--set", "identityBroker.as1.upstream.issuer=https://login.example.com/tenant/v2.0,identityBroker.as1.upstream.clientID=as1"} + with := func(values ...string) []string { + args := slices.Clone(base) + for _, value := range values { + args = append(args, "--set", value) + } + return args + } + + // mecak8s derives the unset OIDC issuer, client ID, and accepted client IDs + // from AS-1; the chart renders the same values. + rendered, err := helm(t, base...) + if err != nil { + t.Fatalf("render minimal AS-1: %v\n%s", err, rendered) + } + deployment := deploymentFromRender(t, rendered) + args := deployment.Spec.Template.Spec.Containers[0].Args + for _, want := range []string{ + "--oidc-issuer=https://mecatl.example.com/as1", "--oidc-client-id=mecatui", "--oidc-accepted-client-ids=mecatui", "--oidc-owner-identity=sub_id", + "--enable-identity-broker", "--identity-broker-as1-issuer=https://mecatl.example.com/as1", "--identity-broker-as1-tui-client-id=mecatui", + "--identity-broker-as1-upstream-issuer=https://login.example.com/tenant/v2.0", "--identity-broker-as1-upstream-client-id=as1", + "--identity-broker-subject-claim=sub", + } { + if !slices.Contains(args, want) { + t.Fatalf("rendered args missing %q: %v", want, args) + } + } + for _, arg := range args { + if strings.HasPrefix(arg, "--identity-broker-as1-signing-key-file") || strings.HasPrefix(arg, "--identity-broker-as1-upstream-client-secret-file") { + t.Fatalf("minimal AS-1 rendered optional flag %q", arg) + } + } + if deployment.Spec.Strategy.Type != appsv1.RecreateDeploymentStrategyType { + t.Fatalf("identity broker strategy = %q, want Recreate", deployment.Spec.Strategy.Type) + } + + // Explicit OIDC values equal to the derived ones, and the optional values. + rendered, err = helm(t, with( + "oidc.issuer=https://mecatl.example.com/as1", "oidc.clientID=mecatui", "oidc.acceptedClientIDs={studio,mecatui}", + "identityBroker.subjectClaim=oid", + "identityBroker.as1.signingKey.secretName=as1-signing", "identityBroker.as1.signingKey.key=key.pem", + "identityBroker.as1.upstream.clientSecret.secretName=as1-upstream", "identityBroker.as1.upstream.clientSecret.key=client-secret", + )...) + if err != nil { + t.Fatalf("render full AS-1: %v\n%s", err, rendered) + } + spec := deploymentFromRender(t, rendered).Spec.Template.Spec + args = spec.Containers[0].Args + for _, want := range []string{ + "--oidc-accepted-client-ids=studio,mecatui", "--identity-broker-subject-claim=oid", + "--identity-broker-as1-signing-key-file=/var/run/secrets/as1-signing-key/key.pem", + "--identity-broker-as1-upstream-client-secret-file=/var/run/secrets/as1-upstream-client-secret/client-secret", + } { + if !slices.Contains(args, want) { + t.Fatalf("rendered args missing %q: %v", want, args) + } + } + if n := len(slices.DeleteFunc(slices.Clone(args), func(arg string) bool { return arg != "--enable-identity-broker" })); n != 1 { + t.Fatalf("--enable-identity-broker rendered %d times: %v", n, args) + } + for volume, secret := range map[string]struct{ name, key string }{"as1-signing-key": {"as1-signing", "key.pem"}, "as1-upstream-client-secret": {"as1-upstream", "client-secret"}} { + i := slices.IndexFunc(spec.Volumes, func(v corev1.Volume) bool { return v.Name == volume }) + if i < 0 || spec.Volumes[i].Secret == nil || spec.Volumes[i].Secret.SecretName != secret.name || len(spec.Volumes[i].Secret.Items) != 1 || spec.Volumes[i].Secret.Items[0].Key != secret.key { + t.Fatalf("volume %s does not project %s/%s: %+v", volume, secret.name, secret.key, spec.Volumes) + } + m := slices.IndexFunc(spec.Containers[0].VolumeMounts, func(m corev1.VolumeMount) bool { return m.Name == volume }) + if m < 0 || !spec.Containers[0].VolumeMounts[m].ReadOnly || spec.Containers[0].VolumeMounts[m].MountPath != "/var/run/secrets/"+volume { + t.Fatalf("volume %s is not mounted read-only at /var/run/secrets/%s: %+v", volume, volume, spec.Containers[0].VolumeMounts) + } + } + + // The identity broker without AS-1 keeps the company IdP as the OIDC issuer, + // and, like mecak8s, needs no OIDC at all. + brokerOnly := []string{"template", "broker", ".", "--set", "mockProvider=true,permissionMode=yolo,replicaCount=1", "--set", "redis.local.enabled=true", + "--set", "identityBroker.enabled=true"} + for name, extra := range map[string][]string{ + "with OIDC": {"--set", "extraArgs={--no-shell}", "--set", "oidc.enabled=true,oidc.issuer=https://as1.example.com,oidc.audience=mecatl,oidc.ownerIdentity=sub_id", "--set", "oidc.acceptedClientIDs={mecatui}"}, + "without OIDC": {"--set", "extraArgs={--no-shell}"}, + "-no-shell": {"--set", "extraArgs={-no-shell}"}, + "no-shell=1": {"--set", "extraArgs={--no-shell=1}"}, + "shell=": {"--set-string", "extraArgs[0]=--shell="}, + "shell-less Redis workspace": {"--set", "redis.filesystem.enabled=true"}, + } { + rendered, err = helm(t, slices.Concat(brokerOnly, extra)...) + if err != nil { + t.Fatalf("render broker without AS-1 %s: %v\n%s", name, err, rendered) + } + args = deploymentFromRender(t, rendered).Spec.Template.Spec.Containers[0].Args + if !slices.Contains(args, "--enable-identity-broker") || slices.ContainsFunc(args, func(arg string) bool { return strings.HasPrefix(arg, "--identity-broker-") }) { + t.Fatalf("broker without AS-1 %s rendered %v", name, args) + } + } + for name, extra := range map[string][]string{ + "no shell flag": nil, + "no-shell turned off": {"--set", "extraArgs={--no-shell,--no-shell=false}"}, + "non-empty shell": {"--set", "extraArgs={--shell=/bin/bash}"}, + "shell value next": {"--set", "extraArgs={--shell,/bin/bash}"}, + "shell reset after empty": {"--set-string", "extraArgs[0]=--shell=", "--set-string", "extraArgs[1]=-shell=/bin/sh"}, + } { + if output, err := helm(t, slices.Concat(brokerOnly, extra)...); err == nil || !strings.Contains(output, "identityBroker.enabled requires no shell in the mecak8s process") { + t.Fatalf("render broker %s = %v, want the shell refusal:\n%s", name, err, output) + } + } + + for name, tc := range map[string]struct { + args []string + want string + }{ + "AS-1 without OIDC": {with("oidc.enabled=false"), "identityBroker.as1.issuer requires oidc.enabled=true"}, + "broker without owner identity": {with("oidc.ownerIdentity="), "identityBroker.enabled with oidc.enabled requires oidc.ownerIdentity=sub_id"}, + "broker with a shell": {with("extraArgs[0]=--shell=/bin/sh"), "identityBroker.enabled requires no shell in the mecak8s process"}, + "subject claim without AS-1": {with("identityBroker.subjectClaim=oid", "identityBroker.as1.issuer=", "identityBroker.as1.tuiClientID=", "identityBroker.as1.upstream.issuer=", "identityBroker.as1.upstream.clientID="), "identityBroker.subjectClaim requires identityBroker.as1.issuer"}, + "AS-1 without subject claim": {with("identityBroker.subjectClaim="), "identityBroker.subjectClaim is required when identityBroker.as1.issuer is set: oid for Microsoft Entra ID, or sub for an identity provider whose sub is the same across applications"}, + "broker flag also in extraArgs": {with("extraArgs[0]=--enable-identity-broker"), `extraArgs entry "--enable-identity-broker" enables the identity broker without the single replica it needs`}, + "AS-1 flag also in extraArgs": {with("extraArgs[0]=-identity-broker-as1-issuer=https://x.example.com"), `extraArgs entry "-identity-broker-as1-issuer=https://x.example.com" sets an identity broker flag`}, + "subject claim in extraArgs": {with("extraArgs[0]=--identity-broker-subject-claim=oid"), `extraArgs entry "--identity-broker-subject-claim=oid" sets an identity broker flag`}, + "AS-1 without broker": {with("identityBroker.enabled=false"), "identityBroker.as1.issuer requires identityBroker.enabled=true"}, + "AS-1 values without issuer": {with("identityBroker.as1.issuer=", "identityBroker.subjectClaim="), "identityBroker.as1 values require identityBroker.as1.issuer"}, + "without TUI client ID": {with("identityBroker.as1.tuiClientID="), "identityBroker.as1.tuiClientID is required"}, + "without upstream issuer": {with("identityBroker.as1.upstream.issuer="), "identityBroker.as1.upstream.issuer is required"}, + "without upstream client ID": {with("identityBroker.as1.upstream.clientID="), "identityBroker.as1.upstream.clientID is required"}, + "plain HTTP issuer": {with("identityBroker.as1.issuer=http://mecatl.example.com/as1"), "identityBroker.as1.issuer must be an https URL"}, + "upstream issuer with query": {with("identityBroker.as1.upstream.issuer=https://login.example.com?x=1"), "identityBroker.as1.upstream.issuer must be an https URL"}, + "without audience": {with("oidc.audience="), "oidc.audience is required with identityBroker.as1.issuer"}, + "without resource": {with("oidc.resource="), "oidc.resource is required with identityBroker.as1.issuer"}, + "private HTTPS issuer": {with("oidc.allowPrivateHTTPSIssuer=true", "oidc.caSecret=ca", "oidc.caKey=ca.crt"), "oidc.allowPrivateHTTPSIssuer cannot be used with identityBroker.as1.issuer"}, + "foreign JWKS URI": {with("oidc.jwksURI=https://idp.example.com/keys"), "oidc.jwksURI cannot be used with identityBroker.as1.issuer"}, + "AS-1's own JWKS URI": {with("oidc.jwksURI=https://mecatl.example.com/as1/.well-known/jwks.json"), "oidc.jwksURI cannot be used with identityBroker.as1.issuer"}, + "different OIDC issuer": {with("oidc.issuer=https://idp.example.com"), "oidc.issuer must be empty or equal identityBroker.as1.issuer"}, + "different OIDC client ID": {with("oidc.clientID=studio"), "oidc.clientID must be empty or equal identityBroker.as1.tuiClientID"}, + "accepted IDs without TUI": {with("oidc.acceptedClientIDs={studio}"), "oidc.acceptedClientIDs must include identityBroker.as1.tuiClientID"}, + "signing key Secret without key": {with("identityBroker.as1.signingKey.secretName=as1-signing"), "identityBroker.as1.signingKey.key is required"}, + "client secret key without name": {with("identityBroker.as1.upstream.clientSecret.key=client-secret"), "identityBroker.as1.upstream.clientSecret.secretName is required"}, + "two replicas": {with("replicaCount=2"), "replicaCount"}, + } { + t.Run(name, func(t *testing.T) { + output, err := helm(t, tc.args...) + if err == nil || !strings.Contains(output, tc.want) { + t.Fatalf("render = %v, want failure %q:\n%s", err, tc.want, output) + } + }) + } +} + func TestMecak8sHelmChart_EdgeTerminatedTLS(t *testing.T) { rendered, err := helm(t, "template", "production", ".", "-f", "ci/production-edge-tls-values.yaml") if err != nil { diff --git a/deploy/helm/mecak8s/ci/production-identity-broker-values.yaml b/deploy/helm/mecak8s/ci/production-identity-broker-values.yaml new file mode 100644 index 0000000000..4436d1d09f --- /dev/null +++ b/deploy/helm/mecak8s/ci/production-identity-broker-values.yaml @@ -0,0 +1,47 @@ +# task deploy:check fixture: a secure real-provider profile with the identity +# broker hosting AS-1 in front of the company identity provider. +image: + tag: v0.0.0 +defaultProvider: openrouter +model: anthropic/claude-sonnet-4-6 +guardrails: + model: anthropic/claude-haiku-4-5 +# The identity broker keeps its state in this process and needs no in-process shell. +replicaCount: 1 +extraArgs: + - --no-shell +extraEnv: + # The chart references these operator-provisioned Secrets; it never creates them. + - name: OPENROUTER_API_KEY + valueFrom: + secretKeyRef: + name: provider-credentials + key: openrouter-api-key +redis: + endpoint: redis.example.internal:6379 + caKey: "" +# oidc.issuer, oidc.clientID, and oidc.acceptedClientIDs are derived from AS-1. +oidc: + enabled: true + audience: mecatl + resource: https://mecatl.example.com + ownerIdentity: sub_id +identityBroker: + enabled: true + # Required with as1.issuer: Entra ID gives each application its own sub. + subjectClaim: oid + as1: + issuer: https://mecatl.example.com/as1 + tuiClientID: mecatui + signingKey: + secretName: as1-signing-key + key: signing-key.pem + upstream: + issuer: https://login.microsoftonline.com/00000000-0000-0000-0000-000000000000/v2.0 + clientID: 11111111-1111-1111-1111-111111111111 + clientSecret: + secretName: as1-upstream + key: client-secret +tls: + enabled: true + secretName: mecak8s-tls diff --git a/deploy/helm/mecak8s/templates/_helpers.tpl b/deploy/helm/mecak8s/templates/_helpers.tpl index 00a65cd8e6..1adb169879 100644 --- a/deploy/helm/mecak8s/templates/_helpers.tpl +++ b/deploy/helm/mecak8s/templates/_helpers.tpl @@ -61,13 +61,90 @@ mounted {{- end -}} {{- end -}} {{- end -}} +{{/* +With a hosted AS-1, mecak8s derives an unset OIDC issuer, client ID, and accepted +client IDs from it; the chart renders the same derived values. +*/}} +{{- define "mecak8s.oidcIssuer" -}} +{{- .Values.oidc.issuer | default .Values.identityBroker.as1.issuer -}} +{{- end -}} +{{- define "mecak8s.oidcClientID" -}} +{{- .Values.oidc.clientID | default .Values.identityBroker.as1.tuiClientID -}} +{{- end -}} +{{- define "mecak8s.oidcAcceptedClientIDs" -}} +{{- if .Values.oidc.acceptedClientIDs -}}{{ toJson .Values.oidc.acceptedClientIDs }}{{- else if .Values.identityBroker.as1.issuer -}}{{ toJson (list .Values.identityBroker.as1.tuiClientID) }}{{- else -}}[]{{- end -}} +{{- end -}} +{{- define "mecak8s.validateIdentityBroker" -}} +{{- /* mecak8s refuses these combinations at startup; fail at render time instead of crash-looping. */ -}} +{{- $broker := .Values.identityBroker -}} +{{- $as1 := $broker.as1 -}} +{{- range $arg := .Values.extraArgs -}} +{{- /* Only identityBroker.enabled also renders the one replica and Recreate strategy the process-local broker state needs. */ -}} +{{- if regexMatch "^--?enable-identity-broker(=|$)" (toString $arg) -}} +{{- fail (printf "extraArgs entry %q enables the identity broker without the single replica it needs; remove it and set identityBroker.enabled=true" (toString $arg)) -}} +{{- end -}} +{{- end -}} +{{- if $broker.enabled -}} +{{- if and .Values.oidc.enabled (ne .Values.oidc.ownerIdentity "sub_id") -}}{{ fail "identityBroker.enabled with oidc.enabled requires oidc.ownerIdentity=sub_id" }}{{- end -}} +{{- /* Go's flag parser takes one or two leading dashes, a bool value only after "=", and a string value after "=" or in the next argument; the last setting wins. */ -}} +{{- $noShell := false -}} +{{- $emptyShell := false -}} +{{- $shellValueNext := false -}} +{{- range $arg := .Values.extraArgs -}} +{{- $a := toString $arg -}} +{{- if $shellValueNext -}} +{{- $emptyShell = eq $a "" -}} +{{- $shellValueNext = false -}} +{{- else if regexMatch "^--?(identity-broker-as1-[a-z-]+|identity-broker-subject-claim)(=|$)" $a -}} +{{- fail (printf "extraArgs entry %q sets an identity broker flag, which the chart owns when identityBroker.enabled is true; remove it and set identityBroker values instead" $a) -}} +{{- else if regexMatch "^--?no-shell(=|$)" $a -}} +{{- $noShell = regexMatch "^--?no-shell(=(1|t|T|TRUE|true|True))?$" $a -}} +{{- else if regexMatch "^--?shell$" $a -}} +{{- $shellValueNext = true -}} +{{- else if regexMatch "^--?shell=" $a -}} +{{- $emptyShell = regexMatch "^--?shell=$" $a -}} +{{- end -}} +{{- end -}} +{{- if not (or $noShell $emptyShell .Values.redis.filesystem.enabled) -}}{{ fail "identityBroker.enabled requires no shell in the mecak8s process: add --no-shell to extraArgs, or --shell= with execution.enabled to keep the remote Shell" }}{{- end -}} +{{- end -}} +{{- if and $broker.subjectClaim (not $as1.issuer) -}}{{ fail "identityBroker.subjectClaim requires identityBroker.as1.issuer" }}{{- end -}} +{{- $as1Set := or $as1.tuiClientID $as1.signingKey.secretName $as1.signingKey.key $as1.upstream.issuer $as1.upstream.clientID $as1.upstream.clientSecret.secretName $as1.upstream.clientSecret.key -}} +{{- if and $as1Set (not $as1.issuer) -}}{{ fail "identityBroker.as1 values require identityBroker.as1.issuer" }}{{- end -}} +{{- if $as1.issuer -}} +{{- if not $broker.enabled -}}{{ fail "identityBroker.as1.issuer requires identityBroker.enabled=true" }}{{- end -}} +{{- if not .Values.oidc.enabled -}}{{ fail "identityBroker.as1.issuer requires oidc.enabled=true" }}{{- end -}} +{{- $_ := required "identityBroker.as1.tuiClientID is required when identityBroker.as1.issuer is set" $as1.tuiClientID -}} +{{- $_ := required "identityBroker.as1.upstream.issuer is required when identityBroker.as1.issuer is set" $as1.upstream.issuer -}} +{{- $_ := required "identityBroker.as1.upstream.clientID is required when identityBroker.as1.issuer is set" $as1.upstream.clientID -}} +{{- $_ := required "identityBroker.subjectClaim is required when identityBroker.as1.issuer is set: oid for Microsoft Entra ID, or sub for an identity provider whose sub is the same across applications" $broker.subjectClaim -}} +{{- range $name, $url := dict "identityBroker.as1.issuer" $as1.issuer "identityBroker.as1.upstream.issuer" $as1.upstream.issuer -}} +{{- if not (regexMatch "^https://[^/?#@[:space:]]+(/[^?#[:space:]]*)?$" $url) -}}{{ fail (printf "%s must be an https URL with a host and no credentials, query, or fragment" $name) }}{{- end -}} +{{- end -}} +{{- $_ := required "oidc.audience is required with identityBroker.as1.issuer: it is the Mecatl API audience AS-1 issues access tokens for" .Values.oidc.audience -}} +{{- $_ := required "oidc.resource is required with identityBroker.as1.issuer: mecatui finds AS-1 through the protected-resource metadata" .Values.oidc.resource -}} +{{- if .Values.oidc.allowPrivateHTTPSIssuer -}}{{ fail "oidc.allowPrivateHTTPSIssuer cannot be used with identityBroker.as1.issuer: mecak8s reaches the AS-1 it hosts without the network" }}{{- end -}} +{{- if .Values.oidc.jwksURI -}}{{ fail "oidc.jwksURI cannot be used with identityBroker.as1.issuer: AS-1's discovery document supplies its keys" }}{{- end -}} +{{- if and .Values.oidc.issuer (ne .Values.oidc.issuer $as1.issuer) -}}{{ fail "oidc.issuer must be empty or equal identityBroker.as1.issuer" }}{{- end -}} +{{- if and .Values.oidc.clientID (ne .Values.oidc.clientID $as1.tuiClientID) -}}{{ fail "oidc.clientID must be empty or equal identityBroker.as1.tuiClientID" }}{{- end -}} +{{- if and .Values.oidc.acceptedClientIDs (not (has $as1.tuiClientID .Values.oidc.acceptedClientIDs)) -}}{{ fail "oidc.acceptedClientIDs must include identityBroker.as1.tuiClientID" }}{{- end -}} +{{- end -}} +{{- range $name, $ref := dict "identityBroker.as1.signingKey" $as1.signingKey "identityBroker.as1.upstream.clientSecret" $as1.upstream.clientSecret -}} +{{- if or $ref.secretName $ref.key -}} +{{- $_ := required (printf "%s.secretName is required when %s.key is set" $name $name) $ref.secretName -}} +{{- $_ := required (printf "%s.key is required when %s.secretName is set" $name $name) $ref.key -}} +{{- end -}} +{{- end -}} +{{- end -}} {{- define "mecak8s.validateOIDC" -}} -{{- $profileSet := or .Values.oidc.resource .Values.oidc.clientID (gt (len .Values.oidc.scopes) 0) -}} +{{- $issuer := include "mecak8s.oidcIssuer" . -}} +{{- $clientID := include "mecak8s.oidcClientID" . -}} +{{- $acceptedClientIDs := include "mecak8s.oidcAcceptedClientIDs" . | fromJsonArray -}} +{{- $profileSet := or .Values.oidc.resource $clientID (gt (len .Values.oidc.scopes) 0) -}} {{- if $profileSet -}} {{- if not .Values.oidc.enabled -}}{{ fail "oidc protected-resource profile requires oidc.enabled=true" }}{{- end -}} -{{- if not (regexMatch "^https://[^/?#[:space:]]+" .Values.oidc.issuer) -}}{{ fail "oidc protected-resource profile requires an https:// oidc.issuer with a non-empty host" }}{{- end -}} +{{- if not (regexMatch "^https://[^/?#[:space:]]+" $issuer) -}}{{ fail "oidc protected-resource profile requires an https:// oidc.issuer with a non-empty host" }}{{- end -}} {{- if not .Values.oidc.resource -}}{{ fail "oidc.resource is required when protected-resource profile is set" }}{{- end -}} -{{- if not .Values.oidc.clientID -}}{{ fail "oidc.clientID is required when protected-resource profile is set" }}{{- end -}} +{{- if not $clientID -}}{{ fail "oidc.clientID is required when protected-resource profile is set" }}{{- end -}} {{- if or (gt (len .Values.oidc.resource) 1024) (regexMatch "[\\x00-\\x1f\\x7f-\\x9f]" .Values.oidc.resource) (regexMatch "[\\p{Cf}]" .Values.oidc.resource) (not (regexMatch "^https://[^/?#[:space:]]+(/[^?#[:space:]]*)?$" .Values.oidc.resource)) (contains "@" .Values.oidc.resource) (contains "," .Values.oidc.resource) (contains "\"" .Values.oidc.resource) (contains "\\" .Values.oidc.resource) (regexMatch "%[^0-9A-Fa-f]|%[0-9A-Fa-f][^0-9A-Fa-f]|%[0-9A-Fa-f]$" .Values.oidc.resource) (regexMatch "(^|/|%2[fF])(\\.|\\.\\.|%2[eE]|%2[eE]%2[eE]|\\.%2[eE]|%2[eE]\\.)(/|%2[fF]|$)" .Values.oidc.resource) (regexMatch "^https://:" .Values.oidc.resource) (regexMatch "^https://[^/?#:\\[]+:([^0-9/]|$)" .Values.oidc.resource) (regexMatch "^https://[^/?#:\\[]+:[0-9]+[^0-9/]" .Values.oidc.resource) (regexMatch "^https://\\[[^\\]/?#]+\\]:([^0-9/]|$)" .Values.oidc.resource) (regexMatch "^https://\\[[^\\]/?#]+\\]:[0-9]+[^0-9/]" .Values.oidc.resource) -}}{{ fail "oidc.resource must be an absolute HTTPS URL of at most 1024 bytes without credentials, query, fragment, commas, quotes, backslashes, malformed escapes, dot segments, invalid authority, control, or Unicode format characters" }}{{- end -}} {{- $resourcePort := regexFind "^https://[^/?#:\\[]+:[0-9]+" .Values.oidc.resource -}} {{- if $resourcePort -}} @@ -79,7 +156,7 @@ mounted {{- $port6 := regexFind "\\]:[0-9]+$" $resourceIPv6Port | trimPrefix "]:" | int -}} {{- if or (lt $port6 1) (gt $port6 65535) -}}{{ fail "oidc.resource port must be between 1 and 65535" }}{{- end -}} {{- end -}} -{{- if or (gt (len .Values.oidc.clientID) 1024) (regexMatch "[\\x00-\\x1f\\x7f-\\x9f]" .Values.oidc.clientID) (regexMatch "[\\p{Cf}]" .Values.oidc.clientID) -}}{{ fail "oidc.clientID must be non-empty, at most 1024 bytes, and contain no control or Unicode format characters" }}{{- end -}} +{{- if or (gt (len $clientID) 1024) (regexMatch "[\\x00-\\x1f\\x7f-\\x9f]" $clientID) (regexMatch "[\\p{Cf}]" $clientID) -}}{{ fail "oidc.clientID must be non-empty, at most 1024 bytes, and contain no control or Unicode format characters" }}{{- end -}} {{- range $scope := .Values.oidc.scopes -}} {{- if or (eq (trim $scope) "") (not (regexMatch "^[!-~]+$" $scope)) (contains "," $scope) (contains "\"" $scope) (contains "\\" $scope) -}}{{ fail (printf "oidc.scopes entry %q is invalid" $scope) }}{{- end -}} {{- end -}} @@ -87,24 +164,16 @@ mounted {{- if not (has .Values.oidc.ownerIdentity (list "" "sub_id")) -}}{{ fail (printf "oidc.ownerIdentity %q is not supported; accepted values: sub_id" .Values.oidc.ownerIdentity) }}{{- end -}} {{- if .Values.oidc.ownerIdentity -}} {{- if not .Values.oidc.enabled -}}{{ fail "oidc.ownerIdentity requires oidc.enabled=true" }}{{- end -}} -{{- if not .Values.oidc.acceptedClientIDs -}}{{ fail "oidc.ownerIdentity=sub_id requires at least one oidc.acceptedClientIDs entry" }}{{- end -}} -{{- if and .Values.oidc.clientID (not (has .Values.oidc.clientID .Values.oidc.acceptedClientIDs)) -}}{{ fail (printf "oidc.clientID %q must be listed in oidc.acceptedClientIDs when oidc.ownerIdentity is sub_id" .Values.oidc.clientID) }}{{- end -}} +{{- if not $acceptedClientIDs -}}{{ fail "oidc.ownerIdentity=sub_id requires at least one oidc.acceptedClientIDs entry" }}{{- end -}} +{{- if and $clientID (not (has $clientID $acceptedClientIDs)) -}}{{ fail (printf "oidc.clientID %q must be listed in oidc.acceptedClientIDs when oidc.ownerIdentity is sub_id" $clientID) }}{{- end -}} {{- else if .Values.oidc.acceptedClientIDs -}} {{- fail "oidc.acceptedClientIDs requires oidc.ownerIdentity=sub_id" -}} {{- end -}} -{{- if and .Values.oidc.enabled (not .Values.oidc.ownerIdentity) -}} -{{- range $arg := .Values.extraArgs -}} -{{- /* mecak8s refuses this combination at startup; fail at render time instead of crash-looping. */ -}} -{{- if regexMatch "^--?enable-identity-broker(=(1|t|T|true|TRUE|True))?$" (toString $arg) -}} -{{- fail (printf "extraArgs entry %q enables the identity broker, which requires oidc.ownerIdentity=sub_id when oidc.enabled is true" (toString $arg)) -}} -{{- end -}} -{{- end -}} -{{- end -}} -{{- range $clientID := .Values.oidc.acceptedClientIDs -}} -{{- if or (eq (trim $clientID) "") (ne (trim $clientID) $clientID) (contains "," $clientID) (regexMatch "[\\x00-\\x1f\\x7f-\\x9f]" $clientID) (regexMatch "[\\p{Cf}]" $clientID) -}}{{ fail (printf "oidc.acceptedClientIDs entry %q must be non-blank and contain no surrounding spaces, commas, control, or Unicode format characters" $clientID) }}{{- end -}} +{{- range $accepted := $acceptedClientIDs -}} +{{- if or (eq (trim $accepted) "") (ne (trim $accepted) $accepted) (contains "," $accepted) (regexMatch "[\\x00-\\x1f\\x7f-\\x9f]" $accepted) (regexMatch "[\\p{Cf}]" $accepted) -}}{{ fail (printf "oidc.acceptedClientIDs entry %q must be non-blank and contain no surrounding spaces, commas, control, or Unicode format characters" $accepted) }}{{- end -}} {{- end -}} {{- if .Values.oidc.enabled -}} -{{- $_ := required "oidc.issuer is required when oidc.enabled is true" .Values.oidc.issuer -}} +{{- $_ := required "oidc.issuer is required when oidc.enabled is true" $issuer -}} {{- $_ := required "oidc.audience is required when oidc.enabled is true" .Values.oidc.audience -}} {{- end -}} {{- if .Values.oidc.allowPrivateHTTPSIssuer -}} diff --git a/deploy/helm/mecak8s/templates/deployment.yaml b/deploy/helm/mecak8s/templates/deployment.yaml index be56e6b4c4..dc1d091560 100644 --- a/deploy/helm/mecak8s/templates/deployment.yaml +++ b/deploy/helm/mecak8s/templates/deployment.yaml @@ -1,5 +1,6 @@ {{- include "mecak8s.validateImage" . -}} {{- include "mecak8s.validateRedis" . -}} +{{- include "mecak8s.validateIdentityBroker" . -}} {{- include "mecak8s.validateOIDC" . -}} {{- include "mecak8s.validateExecution" . -}} {{- include "mecak8s.validateTLS" . -}} @@ -20,8 +21,9 @@ metadata: spec: replicas: {{ .Values.replicaCount }} strategy: - {{- if $mcpOAuth }} - # Broker mode: session/grant/authorization state is process-local. A surge + {{- if or $mcpOAuth .Values.identityBroker.enabled }} + # Broker mode (the MCP OAuth broker or the identity broker and its AS-1): + # session/grant/authorization state is process-local. A surge # rollout (the RollingUpdate default) would briefly run a second, empty # broker incarnation alongside the old one before it drains, which can # receive traffic bound to the old pod's in-flight state. Recreate retires @@ -184,7 +186,7 @@ spec: {{- end }} {{- end }} {{- if .Values.oidc.enabled }} - - {{ printf "--oidc-issuer=%s" .Values.oidc.issuer | quote }} + - {{ printf "--oidc-issuer=%s" (include "mecak8s.oidcIssuer" .) | quote }} - {{ printf "--oidc-audience=%s" .Values.oidc.audience | quote }} {{- if .Values.oidc.jwksURI }} - {{ printf "--oidc-jwks-uri=%s" .Values.oidc.jwksURI | quote }} @@ -198,14 +200,34 @@ spec: - {{ printf "--oidc-max-jwks-staleness=%s" .Values.oidc.maxJWKSStaleness | quote }} {{- if .Values.oidc.resource }} - {{ printf "--oidc-resource=%s" .Values.oidc.resource | quote }} - - {{ printf "--oidc-client-id=%s" .Values.oidc.clientID | quote }} + - {{ printf "--oidc-client-id=%s" (include "mecak8s.oidcClientID" .) | quote }} {{- if .Values.oidc.scopes }} - {{ printf "--oidc-scopes=%s" (join "," .Values.oidc.scopes) | quote }} {{- end }} {{- end }} {{- if .Values.oidc.ownerIdentity }} - {{ printf "--oidc-owner-identity=%s" .Values.oidc.ownerIdentity | quote }} - - {{ printf "--oidc-accepted-client-ids=%s" (join "," .Values.oidc.acceptedClientIDs) | quote }} + - {{ printf "--oidc-accepted-client-ids=%s" (join "," (include "mecak8s.oidcAcceptedClientIDs" . | fromJsonArray)) | quote }} + {{- end }} + {{- end }} + {{- if .Values.identityBroker.enabled }} + - --enable-identity-broker + {{- with .Values.identityBroker.as1 }} + {{- if .issuer }} + - {{ printf "--identity-broker-as1-issuer=%s" .issuer | quote }} + - {{ printf "--identity-broker-as1-tui-client-id=%s" .tuiClientID | quote }} + - {{ printf "--identity-broker-as1-upstream-issuer=%s" .upstream.issuer | quote }} + - {{ printf "--identity-broker-as1-upstream-client-id=%s" .upstream.clientID | quote }} + {{- if .upstream.clientSecret.secretName }} + - {{ printf "--identity-broker-as1-upstream-client-secret-file=/var/run/secrets/as1-upstream-client-secret/%s" .upstream.clientSecret.key | quote }} + {{- end }} + {{- if .signingKey.secretName }} + - {{ printf "--identity-broker-as1-signing-key-file=/var/run/secrets/as1-signing-key/%s" .signingKey.key | quote }} + {{- end }} + {{- end }} + {{- end }} + {{- with .Values.identityBroker.subjectClaim }} + - {{ printf "--identity-broker-subject-claim=%s" . | quote }} {{- end }} {{- end }} {{- if .Values.tls.enabled }} @@ -323,6 +345,12 @@ spec: {{- if .Values.oidc.caSecret }} - {name: oidc-ca, mountPath: /var/run/secrets/oidc-ca, readOnly: true} {{- end }} + {{- if .Values.identityBroker.as1.signingKey.secretName }} + - {name: as1-signing-key, mountPath: /var/run/secrets/as1-signing-key, readOnly: true} + {{- end }} + {{- if .Values.identityBroker.as1.upstream.clientSecret.secretName }} + - {name: as1-upstream-client-secret, mountPath: /var/run/secrets/as1-upstream-client-secret, readOnly: true} + {{- end }} {{- if .Values.execution.enabled }} - {name: execution-mtls, mountPath: /var/run/secrets/execution, readOnly: true} {{- end }} @@ -374,6 +402,26 @@ spec: items: - {key: {{ .Values.oidc.caKey | quote }}, path: {{ .Values.oidc.caKey | quote }}} {{- end }} + {{- with .Values.identityBroker.as1.signingKey }} + {{- if .secretName }} + - name: as1-signing-key + secret: + secretName: {{ .secretName }} + defaultMode: 0440 + items: + - {key: {{ .key | quote }}, path: {{ .key | quote }}} + {{- end }} + {{- end }} + {{- with .Values.identityBroker.as1.upstream.clientSecret }} + {{- if .secretName }} + - name: as1-upstream-client-secret + secret: + secretName: {{ .secretName }} + defaultMode: 0440 + items: + - {key: {{ .key | quote }}, path: {{ .key | quote }}} + {{- end }} + {{- end }} {{- if .Values.execution.enabled }} - name: execution-mtls secret: diff --git a/deploy/helm/mecak8s/tests/identity_broker_test.yaml b/deploy/helm/mecak8s/tests/identity_broker_test.yaml new file mode 100644 index 0000000000..504b91f3b6 --- /dev/null +++ b/deploy/helm/mecak8s/tests/identity_broker_test.yaml @@ -0,0 +1,341 @@ +suite: identity broker and hosted AS-1 +templates: + - templates/deployment.yaml +release: + name: unit + namespace: test +set: + guardrails.fromSettings: true + redis.endpoint: redis.example.internal:6379 + redis.credentialsSecret: redis-credentials + mockProvider: true + replicaCount: 1 + extraArgs: [--no-shell] + oidc.enabled: true + oidc.audience: mecatl + oidc.ownerIdentity: sub_id + oidc.resource: https://mecatl.example.com + identityBroker.enabled: true + identityBroker.subjectClaim: sub + identityBroker.as1.issuer: https://mecatl.example.com/as1 + identityBroker.as1.tuiClientID: mecatui + identityBroker.as1.upstream.issuer: https://login.example.com/tenant/v2.0 + identityBroker.as1.upstream.clientID: as1 +tests: + - it: renders AS-1 and derives the OIDC issuer and clients from it + asserts: + - equal: + path: spec.strategy.type + value: Recreate + - contains: + path: spec.template.spec.containers[0].args + content: --enable-identity-broker + - contains: + path: spec.template.spec.containers[0].args + content: --identity-broker-as1-issuer=https://mecatl.example.com/as1 + - contains: + path: spec.template.spec.containers[0].args + content: --oidc-issuer=https://mecatl.example.com/as1 + - contains: + path: spec.template.spec.containers[0].args + content: --oidc-client-id=mecatui + - contains: + path: spec.template.spec.containers[0].args + content: --oidc-accepted-client-ids=mecatui + - contains: + path: spec.template.spec.containers[0].args + content: --identity-broker-subject-claim=sub + - notExists: + path: spec.template.spec.volumes[?(@.name == "as1-signing-key")] + + - it: projects the signing key and upstream client secret read-only + set: + identityBroker.subjectClaim: oid + identityBroker.as1.signingKey.secretName: as1-signing + identityBroker.as1.signingKey.key: key.pem + identityBroker.as1.upstream.clientSecret.secretName: as1-upstream + identityBroker.as1.upstream.clientSecret.key: client-secret + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --identity-broker-as1-signing-key-file=/var/run/secrets/as1-signing-key/key.pem + - contains: + path: spec.template.spec.containers[0].args + content: --identity-broker-as1-upstream-client-secret-file=/var/run/secrets/as1-upstream-client-secret/client-secret + - contains: + path: spec.template.spec.containers[0].args + content: --identity-broker-subject-claim=oid + - equal: + path: spec.template.spec.volumes[?(@.name == "as1-signing-key")].secret + value: + secretName: as1-signing + defaultMode: 288 + items: + - key: key.pem + path: key.pem + - equal: + path: spec.template.spec.volumes[?(@.name == "as1-upstream-client-secret")].secret + value: + secretName: as1-upstream + defaultMode: 288 + items: + - key: client-secret + path: client-secret + - equal: + path: spec.template.spec.containers[0].volumeMounts[?(@.name == "as1-signing-key")].readOnly + value: true + - equal: + path: spec.template.spec.containers[0].volumeMounts[?(@.name == "as1-upstream-client-secret")].readOnly + value: true + + - it: rejects the identity broker without the sub_id owner identity + set: + oidc.ownerIdentity: "" + asserts: + - failedTemplate: + errorMessage: identityBroker.enabled with oidc.enabled requires oidc.ownerIdentity=sub_id + + - it: renders the identity broker without OIDC when AS-1 is off + set: + identityBroker.subjectClaim: "" + oidc.enabled: false + oidc.audience: "" + oidc.ownerIdentity: "" + oidc.resource: "" + identityBroker.as1.issuer: "" + identityBroker.as1.tuiClientID: "" + identityBroker.as1.upstream.issuer: "" + identityBroker.as1.upstream.clientID: "" + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --enable-identity-broker + - notContains: + path: spec.template.spec.containers[0].args + content: --identity-broker-as1-issuer=https://mecatl.example.com/as1 + + - it: rejects AS-1 without OIDC + set: + oidc.enabled: false + asserts: + - failedTemplate: + errorMessage: identityBroker.as1.issuer requires oidc.enabled=true + + - it: rejects the identity broker with a shell in the process + set: + extraArgs: [] + asserts: + - failedTemplate: + errorPattern: identityBroker.enabled requires no shell in the mecak8s process + + - it: rejects a no-shell flag a later flag turns off + set: + extraArgs: [--no-shell, --no-shell=false] + asserts: + - failedTemplate: + errorPattern: identityBroker.enabled requires no shell + + - it: rejects a non-empty shell + set: + extraArgs: [--shell, /bin/bash] + asserts: + - failedTemplate: + errorPattern: identityBroker.enabled requires no shell + + - it: accepts each Go form of disabling the shell + set: + extraArgs: [-no-shell] + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: -no-shell + + - it: accepts --no-shell=True + set: + extraArgs: [--no-shell=True] + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --no-shell=True + + - it: accepts an empty --shell= + set: + extraArgs: [--shell=] + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --shell= + + - it: reads the argument after --shell as its value + set: + extraArgs: [--shell, --no-shell] + asserts: + - failedTemplate: + errorPattern: identityBroker.enabled requires no shell + + - it: accepts the shell-less Redis workspace + set: + extraArgs: [] + redis.filesystem.enabled: true + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --redis-filesystem + + - it: rejects a subject claim without AS-1 + set: + identityBroker.subjectClaim: oid + identityBroker.as1.issuer: "" + identityBroker.as1.tuiClientID: "" + identityBroker.as1.upstream.issuer: "" + identityBroker.as1.upstream.clientID: "" + asserts: + - failedTemplate: + errorMessage: identityBroker.subjectClaim requires identityBroker.as1.issuer + + - it: rejects the broker flag in extraArgs as well + set: + extraArgs: [--no-shell, --enable-identity-broker] + asserts: + - failedTemplate: + errorMessage: extraArgs entry "--enable-identity-broker" enables the identity broker without the single replica it needs; remove it and set identityBroker.enabled=true + + - it: rejects the broker flag in extraArgs with the identity broker off + set: + extraArgs: [-enable-identity-broker=true] + replicaCount: 3 + oidc.enabled: false + oidc.audience: "" + oidc.ownerIdentity: "" + oidc.resource: "" + identityBroker.enabled: false + identityBroker.subjectClaim: "" + identityBroker.as1.issuer: "" + identityBroker.as1.tuiClientID: "" + identityBroker.as1.upstream.issuer: "" + identityBroker.as1.upstream.clientID: "" + asserts: + - failedTemplate: + errorPattern: enables the identity broker without the single replica it needs; remove it and set identityBroker.enabled=true + + - it: rejects another identity broker flag in extraArgs + set: + extraArgs: [--no-shell, --identity-broker-subject-claim=oid] + asserts: + - failedTemplate: + errorPattern: sets an identity broker flag + + - it: rejects more than one replica + set: + replicaCount: 2 + asserts: + - failedTemplate: + errorPattern: replicaCount + + - it: rejects AS-1 without the identity broker + set: + identityBroker.enabled: false + asserts: + - failedTemplate: + errorMessage: identityBroker.as1.issuer requires identityBroker.enabled=true + + - it: rejects AS-1 values without its issuer + set: + identityBroker.as1.issuer: "" + identityBroker.subjectClaim: "" + asserts: + - failedTemplate: + errorMessage: identityBroker.as1 values require identityBroker.as1.issuer + + - it: requires the TUI client ID + set: + identityBroker.as1.tuiClientID: "" + asserts: + - failedTemplate: + errorPattern: identityBroker.as1.tuiClientID is required + + - it: requires the subject claim with AS-1 + set: + identityBroker.subjectClaim: "" + asserts: + - failedTemplate: + errorMessage: "identityBroker.subjectClaim is required when identityBroker.as1.issuer is set: oid for Microsoft Entra ID, or sub for an identity provider whose sub is the same across applications" + + - it: requires the upstream issuer and client ID + set: + identityBroker.as1.upstream.clientID: "" + asserts: + - failedTemplate: + errorPattern: identityBroker.as1.upstream.clientID is required + + - it: requires an HTTPS issuer + set: + identityBroker.as1.issuer: http://mecatl.example.com/as1 + asserts: + - failedTemplate: + errorPattern: identityBroker.as1.issuer must be an https URL + + - it: requires the Mecatl API audience + set: + oidc.audience: "" + asserts: + - failedTemplate: + errorPattern: oidc.audience is required with identityBroker.as1.issuer + + - it: requires the protected-resource identifier + set: + oidc.resource: "" + asserts: + - failedTemplate: + errorPattern: oidc.resource is required with identityBroker.as1.issuer + + - it: rejects a different OIDC issuer + set: + oidc.issuer: https://idp.example.com + asserts: + - failedTemplate: + errorMessage: oidc.issuer must be empty or equal identityBroker.as1.issuer + + - it: rejects a different OIDC client ID + set: + oidc.clientID: studio + asserts: + - failedTemplate: + errorMessage: oidc.clientID must be empty or equal identityBroker.as1.tuiClientID + + - it: rejects accepted client IDs without the TUI client + set: + oidc.acceptedClientIDs: [studio] + asserts: + - failedTemplate: + errorMessage: oidc.acceptedClientIDs must include identityBroker.as1.tuiClientID + + - it: rejects a JWKS URI outside AS-1 + set: + oidc.jwksURI: https://idp.example.com/keys + asserts: + - failedTemplate: + errorMessage: "oidc.jwksURI cannot be used with identityBroker.as1.issuer: AS-1's discovery document supplies its keys" + + - it: rejects even AS-1's own JWKS URI + set: + oidc.jwksURI: https://mecatl.example.com/as1/.well-known/jwks.json + asserts: + - failedTemplate: + errorPattern: oidc.jwksURI cannot be used with identityBroker.as1.issuer + + - it: rejects a private HTTPS issuer + set: + oidc.allowPrivateHTTPSIssuer: true + oidc.caSecret: oidc-ca + oidc.caKey: ca.crt + asserts: + - failedTemplate: + errorPattern: oidc.allowPrivateHTTPSIssuer cannot be used with identityBroker.as1.issuer + + - it: rejects a signing key Secret without a key + set: + identityBroker.as1.signingKey.secretName: as1-signing + asserts: + - failedTemplate: + errorMessage: identityBroker.as1.signingKey.key is required when identityBroker.as1.signingKey.secretName is set diff --git a/deploy/helm/mecak8s/values.schema.json b/deploy/helm/mecak8s/values.schema.json index 2f1d61bdad..9a20011f57 100644 --- a/deploy/helm/mecak8s/values.schema.json +++ b/deploy/helm/mecak8s/values.schema.json @@ -62,6 +62,35 @@ "acceptedClientIDs": {"type": "array", "items": {"type": "string", "minLength": 1}} } }, + "identityBroker": { + "type": "object", + "additionalProperties": false, + "required": ["enabled", "as1"], + "properties": { + "enabled": {"type": "boolean"}, + "subjectClaim": {"type": "string", "pattern": "^\\S*$"}, + "as1": { + "type": "object", + "additionalProperties": false, + "required": ["issuer", "tuiClientID", "signingKey", "upstream"], + "properties": { + "issuer": {"type": "string"}, + "tuiClientID": {"type": "string"}, + "signingKey": {"$ref": "#/definitions/optionalSecretKey"}, + "upstream": { + "type": "object", + "additionalProperties": false, + "required": ["issuer", "clientID", "clientSecret"], + "properties": { + "issuer": {"type": "string"}, + "clientID": {"type": "string"}, + "clientSecret": {"$ref": "#/definitions/optionalSecretKey"} + } + } + } + } + } + }, "tls": { "type": "object", "additionalProperties": false, @@ -553,9 +582,28 @@ "required": ["replicaCount"], "properties": {"replicaCount": {"const": 1}} } + }, + { + "if": { + "required": ["identityBroker"], + "properties": {"identityBroker": {"required": ["enabled"], "properties": {"enabled": {"const": true}}}} + }, + "then": { + "required": ["replicaCount"], + "properties": {"replicaCount": {"const": 1}} + } } ], "definitions": { + "optionalSecretKey": { + "type": "object", + "additionalProperties": false, + "required": ["secretName", "key"], + "properties": { + "secretName": {"type": "string", "maxLength": 253, "pattern": "^(|[a-z0-9]([-.a-z0-9]*[a-z0-9])?)$"}, + "key": {"type": "string", "maxLength": 253, "pattern": "^(|[-._a-zA-Z0-9]+)$"} + } + }, "secretKeyRef": { "type": "object", "additionalProperties": false, diff --git a/deploy/helm/mecak8s/values.yaml b/deploy/helm/mecak8s/values.yaml index 117fad4a2f..7c5886935f 100644 --- a/deploy/helm/mecak8s/values.yaml +++ b/deploy/helm/mecak8s/values.yaml @@ -279,13 +279,62 @@ oidc: # identifier). sub_id requires RFC 9068 access tokens (typ at+jwt) whose # client_id is listed in acceptedClientIDs; other tokens are rejected. It # requires oidc.enabled and at least one acceptedClientIDs entry. The identity - # broker (--enable-identity-broker in extraArgs) requires sub_id when OIDC is on. + # broker (identityBroker.enabled) requires sub_id when OIDC is on. ownerIdentity: "" # acceptedClientIDs are the OAuth clients (for example the TUI and Studio) # whose access tokens ownerIdentity sub_id accepts. When clientID is set it must be # listed here. acceptedClientIDs: [] +# The experimental agent identity broker. enabled renders --enable-identity-broker +# and requires oidc.ownerIdentity=sub_id when oidc.enabled, replicaCount: 1 (broker +# state is process-local; the Deployment then rolls out with Recreate), and no shell +# in the process: add --no-shell to extraArgs, or --shell= with execution.enabled to +# keep the remote Shell (redis.filesystem.enabled also has no shell). The chart +# refuses to render without one of these rather than let the pod crash-loop. +# enabled is the only way to turn the broker on: the chart refuses +# --enable-identity-broker in extraArgs, which would skip the single replica and +# Recreate rollout. With enabled, the chart also owns the other identity broker +# flags and refuses them in extraArgs. +identityBroker: + enabled: false + # Upstream ID-token claim that names the company identity, required with + # as1.issuer and allowed only with it. It must identify a user the same way in + # every IdP application the identity broker uses: oid for Microsoft Entra ID, + # whose sub differs per application; sub for an IdP whose sub is the same + # across applications. + subjectClaim: "" + # as1 hosts AS-1, Mecatl's sign-in authorization server in front of the company + # identity provider, on mecak8s's HTTP listener under the issuer URL's path. A + # non-empty issuer turns it on and requires oidc.enabled, oidc.audience, and + # oidc.resource; oidc.jwksURI must stay empty. + # oidc.issuer, oidc.clientID, and oidc.acceptedClientIDs may stay empty: the chart + # renders them as issuer, tuiClientID, and [tuiClientID]. When set, oidc.issuer + # must equal issuer, oidc.clientID must equal tuiClientID, and + # oidc.acceptedClientIDs must include tuiClientID. AS-1 keeps users, sign-ins, and + # refresh tokens in memory, so a restart signs everyone out. + as1: + # HTTPS issuer URL. Do not change it once users have signed in. + issuer: "" + # OAuth client ID of mecatui at AS-1 (a public client). + tuiClientID: "" + # Optional existing Secret key holding AS-1's PEM signing key (EC P-256 or + # RSA), mounted read-only. Empty generates a key at startup, so access tokens + # stop verifying when the pod restarts. + signingKey: + secretName: "" + key: "" + # The company identity provider. Register AS-1 there as a web application with + # the redirect URI /oauth/callback. + upstream: + issuer: "" + clientID: "" + # Optional existing Secret key holding AS-1's client secret at the upstream, + # mounted read-only. Empty makes AS-1 a public client there. + clientSecret: + secretName: "" + key: "" + tls: # Server TLS is optional for mock and explicit unsafe-bypass profiles, but # required for a real provider under the secure default. To enable it, create diff --git a/internal/adapter/mcpbroker/toolhive_boundary_test.go b/internal/adapter/mcpbroker/toolhive_boundary_test.go index 2799c23677..0906e1b6e7 100644 --- a/internal/adapter/mcpbroker/toolhive_boundary_test.go +++ b/internal/adapter/mcpbroker/toolhive_boundary_test.go @@ -27,6 +27,8 @@ func TestToolHiveImportsStayBehindApprovedAdapterLeaves(t *testing.T) { // import leaf unrelated to the MCP broker's own construction/discovery. "cmd/mecatui/resource_discovery.go": true, "internal/adapter/resourceurl/resourceurl.go": true, + // AS-1, hosted on ToolHive's embedded authorization server (#2490). + "internal/identitybroker/adapter/as1server/as1server.go": true, } const toolHiveModule = "github.com/stacklok/toolhive" if err := filepath.WalkDir(root, func(sourcePath string, entry fs.DirEntry, walkErr error) error { diff --git a/internal/app/build.go b/internal/app/build.go index 33e579364d..22dcc4734a 100644 --- a/internal/app/build.go +++ b/internal/app/build.go @@ -982,10 +982,14 @@ type Config struct { // configuration in tests. identityBrokerConfig *identitybroker.Config // IdentityBrokerAS1 connects the real identity broker's session open to an - // AS-1, where it converts users' access tokens. No flag sets it yet: #2490 - // fills it from operator configuration for AS-1 hosted in mecak8s. Nil, as - // in every deployment until then, converts no access tokens. + // AS-1, where it converts users' access tokens. No flag sets it yet: #2607 + // fills it for AS-1 hosted in mecak8s. Nil, as in every deployment until + // then, converts no access tokens. IdentityBrokerAS1 *IdentityBrokerAS1 + // IdentityBrokerHostedAS1 hosts AS-1 beside the identity broker; nil hosts + // none. Only mecak8s sets it. Hosting AS-1 does not connect session open + // to it: IdentityBrokerAS1 stays nil until #2607. + IdentityBrokerHostedAS1 *IdentityBrokerAS1Config // MCPProfileLoader resolves operator-tier profiles with the same permission // resolver Build already owns. Command roots install it so settings are not // parsed a second time and secret lookup remains a runtime-only operation. @@ -1551,6 +1555,9 @@ type Built struct { // IdentityBroker is the agent identity broker, as Mecatl calls it; nil when // it is not enabled. Close releases it. IdentityBroker identitybroker.Service + // IdentityBrokerHostedAS1 is the hosted AS-1; nil when + // Config.IdentityBrokerHostedAS1 is. Close releases it. + IdentityBrokerHostedAS1 *IdentityBrokerAS1Server // GuardrailsAdvisory reports a configured checker demoted to advisory for // sessions in the server's default mode (yolo); other modes keep it enforcing. GuardrailsAdvisory bool @@ -2284,7 +2291,7 @@ func Build(ctx context.Context, cfg Config) (*Built, error) { // The identity broker is released after the MCP broker, on startup failure // and on shutdown. - identityBroker, releaseIdentityBroker, err := buildIdentityBroker(cfg) + identityBroker, identityBrokerAS1, releaseIdentityBroker, err := buildIdentityBroker(ctx, cfg) if err != nil { childLiveness.Close() mcpClose() @@ -3132,7 +3139,7 @@ func Build(ctx context.Context, cfg Config) (*Built, error) { diag.Log(ctx, port.LevelWarn, "durable automatic reservation reconciliation unavailable") }) attempts = startAttemptRecovery(ctx, cfg, reg, store, eventLog, assets.attemptRepository, assets.reflectionRepository, assets, placementProvider, placementScope) - return &Built{Service: svc, MCPBroker: brokerRuntime, MCPBrokerHandlers: brokerHandlers, MCPBrokerCallbackPath: brokerCallbackPath, IdentityBroker: identityBroker, GuardrailsAdvisory: guardrailConfigured && guardrailsDemoted(cfg), Close: closeAll}, nil + return &Built{Service: svc, MCPBroker: brokerRuntime, MCPBrokerHandlers: brokerHandlers, MCPBrokerCallbackPath: brokerCallbackPath, IdentityBroker: identityBroker, IdentityBrokerHostedAS1: identityBrokerAS1, GuardrailsAdvisory: guardrailConfigured && guardrailsDemoted(cfg), Close: closeAll}, nil } // resolveAgentSeam resolves the agent-definition registry from cfg: the diff --git a/internal/app/identitybroker.go b/internal/app/identitybroker.go index d2c6bbecee..7704772d02 100644 --- a/internal/app/identitybroker.go +++ b/internal/app/identitybroker.go @@ -1,6 +1,7 @@ package app import ( + "context" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" @@ -11,6 +12,7 @@ import ( "time" "github.com/stacklok/mecatl/internal/identitybroker" + "github.com/stacklok/mecatl/internal/identitybroker/adapter/as1server" "github.com/stacklok/mecatl/internal/identitybroker/adapter/authserverclient" ) @@ -34,22 +36,87 @@ var interimIdentityBrokerConfig = identitybroker.Config{ // signing key from the process's memory or environment. var errIdentityBrokerWithShell = errors.New("the identity broker holds its signing key in this process, so it cannot run while sessions have a shell in this process; use --no-shell, or --shell \"\" with Kubernetes execution to keep the remote Shell") -// buildIdentityBroker returns the identity broker Mecatl calls, and a function -// that releases it. Both are no-ops when the broker is not enabled. Mecatl gets -// only the authentication boundary, never the core behind it. A construction -// failure fails startup, as does a configuration that gives sessions a shell. -func buildIdentityBroker(cfg Config) (identitybroker.Service, func(), error) { +// IdentityBrokerAS1Config hosts AS-1, the identity broker's sign-in +// authorization server, in this process. It requires Config.IdentityBroker. +// AS-1 keeps pending sign-ins and refresh tokens in memory. +type IdentityBrokerAS1Config struct { + // Issuer is AS-1's https issuer URL; its routes are served under its path. + Issuer string + // Audience is the Mecatl API audience AS-1 issues access tokens for. + Audience string + // SigningKeyFile is a PEM key that signs access tokens. Empty generates + // one, so tokens stop verifying when the process restarts. + SigningKeyFile string + // PublicClients are the sign-in clients AS-1 registers: no secret, PKCE, + // and tokens for Audience. At least one is required. + PublicClients []IdentityBrokerAS1PublicClient + // UpstreamIssuer, UpstreamClientID, and UpstreamClientSecretFile are + // AS-1's registration at the IdP users sign in at. An empty secret file + // makes AS-1 a public client there. + UpstreamIssuer string + UpstreamClientID string + UpstreamClientSecretFile string + // SubjectClaim is the IdP ID-token claim naming the company identity. + // Empty selects "sub". + SubjectClaim string + // UpstreamCAFile is a PEM bundle trusted for the IdP's TLS certificate in + // addition to the system roots. Empty trusts the system roots only. + UpstreamCAFile string + // UpstreamAllowPrivateAddresses lets AS-1 reach the IdP on a private or + // link-local address, which its SSRF protection otherwise refuses. + UpstreamAllowPrivateAddresses bool +} + +// IdentityBrokerAS1PublicClient is a public sign-in client of AS-1. +type IdentityBrokerAS1PublicClient struct { + // ID is the client ID, unique among the configured clients. + ID string + // RedirectURIs are the client's absolute redirect URIs, matched exactly. + RedirectURIs []string +} + +// IdentityBrokerAS1Server is a hosted AS-1 as a command root serves it. +type IdentityBrokerAS1Server struct { + // Issuer is AS-1's issuer URL. + Issuer string + // Routes are the exact paths Handler serves, to mount on the listener + // that serves Issuer after checking they collide with no other route. + Routes []string + // Handler serves Routes and nothing else. + Handler http.Handler + // HTTPClient reaches AS-1 without a listener, so the process's own token + // validator can fetch its discovery document and keys before listeners + // start. + HTTPClient *http.Client +} + +// errIdentityBrokerAS1WithoutBroker refuses a hosted AS-1 the identity broker +// would not be running beside. +var errIdentityBrokerAS1WithoutBroker = errors.New("hosting AS-1 requires the identity broker") + +// buildIdentityBroker returns the identity broker Mecatl calls, the hosted +// AS-1 when configured, and a function that releases both. All are nil or +// no-ops when the broker is not enabled. Mecatl gets only the authentication +// boundary, never the core behind it. A construction failure fails startup, +// as does a configuration that gives sessions a shell. +// +// Session open does not convert tokens at the hosted AS-1: the broker is not +// pointed at it here. +func buildIdentityBroker(ctx context.Context, cfg Config) (identitybroker.Service, *IdentityBrokerAS1Server, func(), error) { if !cfg.IdentityBroker { - return nil, func() {}, nil + if cfg.IdentityBrokerHostedAS1 != nil { + return nil, nil, nil, errIdentityBrokerAS1WithoutBroker + } + return nil, nil, func() {}, nil } if shellAvailable(cfg) { - return nil, nil, errIdentityBrokerWithShell + return nil, nil, nil, errIdentityBrokerWithShell } core := cfg.identityBrokerCore if core == nil { broker, err := newIdentityBroker(cfg) if err != nil { - return nil, nil, fmt.Errorf("build identity broker: %w", err) + return nil, nil, nil, fmt.Errorf("build identity broker: %w", err) } core = broker.core } @@ -57,13 +124,39 @@ func buildIdentityBroker(cfg Config) (identitybroker.Service, func(), error) { if closer, ok := core.(io.Closer); ok { release = func() { _ = closer.Close() } } - return identitybroker.NewInProcess(identitybroker.UnverifiedAuthenticator{Name: identityBrokerWorkload}, core), release, nil + var hosted *IdentityBrokerAS1Server + if as1 := cfg.IdentityBrokerHostedAS1; as1 != nil { + clients := make([]as1server.PublicClient, len(as1.PublicClients)) + for i, c := range as1.PublicClients { + clients[i] = as1server.PublicClient{ID: c.ID, RedirectURIs: c.RedirectURIs} + } + server, err := as1server.New(ctx, as1server.Config{ + Issuer: as1.Issuer, Audience: as1.Audience, SigningKeyFile: as1.SigningKeyFile, + PublicClients: clients, + Upstream: as1server.UpstreamConfig{ + Issuer: as1.UpstreamIssuer, ClientID: as1.UpstreamClientID, + ClientSecretFile: as1.UpstreamClientSecretFile, SubjectClaim: as1.SubjectClaim, + CAFile: as1.UpstreamCAFile, AllowPrivateAddresses: as1.UpstreamAllowPrivateAddresses, + }, + }) + if err != nil { + release() + return nil, nil, nil, fmt.Errorf("build AS-1: %w", err) + } + hosted = &IdentityBrokerAS1Server{Issuer: server.Issuer(), Routes: server.Routes(), Handler: server.Handler(), HTTPClient: server.InProcessClient()} + releaseCore := release + release = func() { + _ = server.Close() + releaseCore() + } + } + return identitybroker.NewInProcess(identitybroker.UnverifiedAuthenticator{Name: identityBrokerWorkload}, core), hosted, release, nil } // identityBroker is the real identity broker's startup state: its core and // the signer for its client authentication at AS-1 and AS-2. With an AS-1 // configured, session open authenticates there with the signer; without one, -// which is every deployment until hosted AS-1 configuration exists (#2490), +// which is every deployment until session open uses the hosted AS-1 (#2607), // nothing calls AS-1 or AS-2. type identityBroker struct { core *identitybroker.Broker diff --git a/internal/app/identitybroker_test.go b/internal/app/identitybroker_test.go index abba68bcf6..516581b2ff 100644 --- a/internal/app/identitybroker_test.go +++ b/internal/app/identitybroker_test.go @@ -8,6 +8,11 @@ import ( "encoding/base64" "encoding/json" "errors" + "net/http" + "os" + "path/filepath" + "runtime" + "slices" "strings" "sync" "testing" @@ -28,6 +33,7 @@ import ( "github.com/stacklok/mecatl/internal/adapter/permconfig" "github.com/stacklok/mecatl/internal/adapter/server" "github.com/stacklok/mecatl/internal/identitybroker" + "github.com/stacklok/mecatl/internal/identitybroker/adapter/as1server" "github.com/stacklok/mecatl/internal/syscaller" "github.com/stacklok/mecatl/internal/testutil/identityharness" ) @@ -70,8 +76,8 @@ func TestIdentityBrokerIsOffByDefault(t *testing.T) { t.Fatal(err) } t.Cleanup(built.Close) - if built.IdentityBroker != nil { - t.Fatal("an identity broker was constructed with the flag off") + if built.IdentityBroker != nil || built.IdentityBrokerHostedAS1 != nil { + t.Fatal("an identity broker or AS-1 was constructed with the flag off") } } @@ -151,6 +157,9 @@ func TestIdentityBrokerDefaultWiringOpensSessions(t *testing.T) { if built.IdentityBroker == nil { t.Fatal("no identity broker was constructed with the flag on") } + if built.IdentityBrokerHostedAS1 != nil { + t.Fatal("AS-1 was hosted without its configuration") + } owner := identitybroker.Owner{Issuer: "https://login.example.com", Subject: "alice-204"} opened, err := built.IdentityBroker.OpenSession(t.Context(), identitybroker.OpenSessionRequest{HostSessionID: "hostS42", Incarnation: string(session.NewIncarnationID()), Owner: owner}) if err != nil { @@ -379,6 +388,119 @@ func TestIdentityBrokerSessionsRegisterTheirAgent(t *testing.T) { } } +// hostedAS1Config configures a hosted AS-1 for issuer with h's IdP as its +// upstream, registering it there. +func hostedAS1Config(t *testing.T, h *identityharness.Harness, issuer string) *IdentityBrokerAS1Config { + t.Helper() + dir := t.TempDir() + secret := rand.Text() + secretFile := filepath.Join(dir, "upstream-client-secret") + if err := os.WriteFile(secretFile, []byte(secret), 0o600); err != nil { + t.Fatal(err) + } + h.IdP.RegisterClient("hosted-as1", secret, issuer+"/oauth/callback") + return &IdentityBrokerAS1Config{ + Issuer: issuer, Audience: identityharness.DefaultAudience, + PublicClients: []IdentityBrokerAS1PublicClient{{ID: identityharness.DefaultClientID, RedirectURIs: []string{identityharness.DefaultRedirectURI}}}, + UpstreamIssuer: h.IdP.Issuer(), UpstreamClientID: "hosted-as1", UpstreamClientSecretFile: secretFile, + UpstreamCAFile: h.WriteCACert(t, dir), UpstreamAllowPrivateAddresses: true, + } +} + +// as1StorageRunning reports whether a hosted AS-1's in-memory storage is +// still running its cleanup loop, the one goroutine Close stops. +func as1StorageRunning() bool { + buf := make([]byte, 1<<20) + for { + n := runtime.Stack(buf, true) + if n < len(buf) { + return strings.Contains(string(buf[:n]), "storage.(*MemoryStorage).cleanupLoop") + } + buf = make([]byte, 2*len(buf)) + } +} + +// TestIdentityBrokerHostsAS1 builds a hosted AS-1 beside the broker, serving +// its routes under the issuer path and reachable in process, releases it with +// the broker, and leaves session open unchanged: the broker converts no token +// at it. +func TestIdentityBrokerHostsAS1(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + const issuer = "https://mecatl.test/as1" + built, err := buildIsolated(t, t.Context(), Config{Workspace: t.TempDir(), UseMock: true, NoSoul: true, IdentityBroker: true, IdentityBrokerHostedAS1: hostedAS1Config(t, h, issuer)}) + if err != nil { + t.Fatal(err) + } + closed := false + t.Cleanup(func() { + if !closed { + built.Close() + } + }) + as1 := built.IdentityBrokerHostedAS1 + if as1 == nil || as1.Issuer != issuer || !slices.Contains(as1.Routes, "/as1/oauth/token") || as1.Handler == nil { + t.Fatalf("hosted AS-1 = %+v, want one for %s serving under /as1", as1, issuer) + } + resp, err := as1.HTTPClient.Get(issuer + "/.well-known/openid-configuration") + if err != nil { + t.Fatalf("in-process discovery: %v", err) + } + var doc struct { + Issuer string `json:"issuer"` + } + err = json.NewDecoder(resp.Body).Decode(&doc) + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK || err != nil || doc.Issuer != issuer { + t.Fatalf("in-process discovery: status %d, issuer %q, err %v", resp.StatusCode, doc.Issuer, err) + } + alice := h.Owner(identityharness.Alice) + opened, err := built.IdentityBroker.OpenSession(t.Context(), identitybroker.OpenSessionRequest{ + HostSessionID: "hostS42", Incarnation: string(session.NewIncarnationID()), + Owner: identitybroker.Owner{Issuer: alice.Issuer, Subject: alice.Subject}, + AccessToken: identitybroker.NewToken(h.AS1.MintAccessToken(identityharness.AccessTokenSpec{User: identityharness.Alice})), + }) + if err != nil || opened.HoldsRefreshToken { + t.Fatalf("OpenSession with an access token = %+v, %v; want a session holding no refresh token", opened, err) + } + if !as1StorageRunning() { + t.Fatal("the hosted AS-1's storage is not running") + } + built.Close() + closed = true + if as1StorageRunning() { + t.Fatal("the hosted AS-1 was not released on shutdown") + } +} + +func TestIdentityBrokerAS1RequiresTheBroker(t *testing.T) { + built, err := buildIsolated(t, t.Context(), Config{Workspace: t.TempDir(), UseMock: true, NoSoul: true, IdentityBrokerHostedAS1: &IdentityBrokerAS1Config{Issuer: "https://mecatl.test/as1"}}) + if err == nil { + built.Close() + t.Fatal("Build hosted AS-1 without the identity broker") + } + if !errors.Is(err, errIdentityBrokerAS1WithoutBroker) { + t.Fatalf("Build error = %v, want %v", err, errIdentityBrokerAS1WithoutBroker) + } +} + +func TestIdentityBrokerIsReleasedWhenAS1ConstructionFails(t *testing.T) { + core := newClosableCore(t) + invalid := &IdentityBrokerAS1Config{Issuer: "https://mecatl.test/as1"} + built, err := buildIsolated(t, t.Context(), Config{Workspace: t.TempDir(), UseMock: true, NoSoul: true, IdentityBroker: true, identityBrokerCore: core, IdentityBrokerHostedAS1: invalid}) + if err == nil { + built.Close() + t.Fatal("Build succeeded with an invalid AS-1 configuration") + } + if !errors.Is(err, as1server.ErrInvalidConfig) { + t.Fatalf("Build error = %v, want AS-1's invalid configuration", err) + } + core.mu.Lock() + defer core.mu.Unlock() + if core.closed != 1 { + t.Fatalf("identity broker closed %d times after AS-1 failed to build, want once", core.closed) + } +} + func TestIdentityBrokerConstructionFailureStopsStartup(t *testing.T) { // The session store's driver connection is closed on this failure only by // the identity broker's own error branch: no deferred cleanup covers the diff --git a/internal/cliconfig/authnvalidator.go b/internal/cliconfig/authnvalidator.go index 6ff0b77cee..b6270f3a3f 100644 --- a/internal/cliconfig/authnvalidator.go +++ b/internal/cliconfig/authnvalidator.go @@ -60,7 +60,7 @@ func defaultNewValidator(ctx context.Context, c OIDCConfig) (server.PrincipalVal AllowPrivateHTTPSIssuer: c.AllowPrivateHTTPSIssuer, TrustedCAFile: c.TrustedCAFile, TrustedCAPEM: caPEM, - HTTPClient: c.httpClient, + HTTPClient: c.HTTPClient, OwnerIdentity: c.OwnerIdentity, AcceptedClientIDs: c.AcceptedClientIDs, }) diff --git a/internal/cliconfig/authnvalidator_test.go b/internal/cliconfig/authnvalidator_test.go index 0efe417ae5..8cf811586e 100644 --- a/internal/cliconfig/authnvalidator_test.go +++ b/internal/cliconfig/authnvalidator_test.go @@ -111,7 +111,7 @@ func (f *rootJWKSFixture) validator(t *testing.T) server.PrincipalValidator { t.Helper() validator, err := OIDCValidator(context.Background(), OIDCConfig{ Issuer: f.srv.URL, Audience: rootTestAudience, JWKSURI: f.srv.URL + "/keys", - httpClient: f.srv.Client(), + HTTPClient: f.srv.Client(), }) if err != nil { t.Fatalf("OIDCValidator: %v", err) @@ -152,7 +152,7 @@ func TestDefaultOIDCValidatorPassesOwnerIdentity(t *testing.T) { validator, err := OIDCValidator(context.Background(), OIDCConfig{ Issuer: fixture.srv.URL, Audience: rootTestAudience, JWKSURI: fixture.srv.URL + "/keys", OwnerIdentity: oidcauthn.OwnerIdentitySubID, AcceptedClientIDs: []string{"mecatui"}, - httpClient: fixture.srv.Client(), + HTTPClient: fixture.srv.Client(), }) if err != nil { t.Fatalf("OIDCValidator: %v", err) diff --git a/internal/cliconfig/insecure_issuer_test.go b/internal/cliconfig/insecure_issuer_test.go index e1285f4cc5..97ca113a8b 100644 --- a/internal/cliconfig/insecure_issuer_test.go +++ b/internal/cliconfig/insecure_issuer_test.go @@ -48,7 +48,7 @@ func TestCallerIdentityE2E_Scenario3_PrivateIssuerRefusedByDefault(t *testing.T) build := func(t *testing.T, srv *httptest.Server, insecure bool) error { t.Helper() - // No httpClient override: this asserts what the validator's OWN client + // No HTTPClient override: this asserts what the validator's OWN client // does, which is the only thing a real deployment has. _, err := OIDCValidator(context.Background(), OIDCConfig{ Issuer: srv.URL, diff --git a/internal/cliconfig/oidc.go b/internal/cliconfig/oidc.go index cab1e48966..05a6458f02 100644 --- a/internal/cliconfig/oidc.go +++ b/internal/cliconfig/oidc.go @@ -60,9 +60,12 @@ type OIDCConfig struct { // force a startup failure; it is not a deployment extension point. NewValidator func(ctx context.Context, c OIDCConfig) (server.PrincipalValidator, error) - // httpClient is an in-package test seam for a local TLS issuer's private CA. - // Production flag parsing leaves it nil, preserving the hardened client. - httpClient *http.Client + // HTTPClient, when set, fetches the issuer's discovery document and keys + // in place of the hardened client. It is for an issuer this process + // serves itself, such as mecak8s's hosted AS-1, reached without a + // listener. Flag parsing leaves it nil, and authn/oidc refuses it with + // AllowPrivateHTTPSIssuer. + HTTPClient *http.Client // InsecureAllowPrivateIssuer is the deprecated legacy escape hatch. It permits // BOTH HTTP and private issuer/JWKS addresses; use AllowPrivateHTTPSIssuer diff --git a/internal/identitybroker/adapter/as1server/as1server.go b/internal/identitybroker/adapter/as1server/as1server.go new file mode 100644 index 0000000000..1e2192586a --- /dev/null +++ b/internal/identitybroker/adapter/as1server/as1server.go @@ -0,0 +1,352 @@ +// Package as1server hosts AS-1, Mecatl's sign-in authorization server, on +// ToolHive's embedded authorization server. AS-1 signs users in through one +// upstream OIDC IdP and issues RFC 9068 (typ at+jwt) access tokens for the +// Mecatl API audience that carry the company identity as the RFC 9493 sub_id +// claim. Its clients are the configured public clients, which sign in with +// PKCE. +// +// The server keeps its own in-memory storage, so pending sign-ins and refresh +// tokens do not survive a restart, and caps the sign-ins pending there. Its +// signing key survives when Config.SigningKeyFile is set. +// +// Only the sign-in surface is served: dynamic client registration and the +// device flow are not, and the discovery documents do not advertise them. +package as1server + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "maps" + "net/http" + "net/url" + "path" + "path/filepath" + "slices" + "strconv" + "strings" + "sync" + + "github.com/stacklok/toolhive/pkg/authserver" + "github.com/stacklok/toolhive/pkg/authserver/runner" + "github.com/stacklok/toolhive/pkg/authserver/server/registration" + "github.com/stacklok/toolhive/pkg/authserver/storage" + "github.com/stacklok/toolhive/pkg/oauthproto" +) + +// accessTokenType is the JOSE typ of AS-1 access tokens (RFC 9068). +const accessTokenType = "at+jwt" + +// ErrInvalidConfig reports a Config that New refuses before starting anything. +var ErrInvalidConfig = errors.New("as1server: invalid configuration") + +// Config configures AS-1. +type Config struct { + // Issuer is AS-1's issuer URL: https, without query, fragment, or trailing + // slash. It may have a path, under which every route is served. + Issuer string + // Audience is the Mecatl API identifier, the only audience AS-1 issues + // tokens for and the one whose tokens carry sub_id. + Audience string + // SigningKeyFile is a PEM private key (EC P-256 for ES256, or RSA) that + // signs access tokens. Empty generates a key at startup, so tokens stop + // verifying when the server restarts. + SigningKeyFile string + // PublicClients are the sign-in clients AS-1 registers. At least one is + // required. + PublicClients []PublicClient + // Upstream is the IdP users sign in at. + Upstream UpstreamConfig +} + +// PublicClient is a public sign-in client of AS-1: it has no secret and uses +// PKCE with the authorization-code and refresh-token grants. It may ask for +// the openid, profile, and offline_access scopes, and its access tokens are +// for Config.Audience. +type PublicClient struct { + // ID is the client ID, unique among Config.PublicClients. + ID string + // RedirectURIs are the client's absolute redirect URIs, matched exactly. + RedirectURIs []string +} + +// UpstreamConfig is AS-1's registration at the upstream OIDC IdP. +type UpstreamConfig struct { + // Issuer is the IdP's https issuer URL, used for OIDC discovery. + Issuer string + // ClientID is AS-1's client ID at the IdP. + ClientID string + // ClientSecretFile holds AS-1's client secret at the IdP. Empty makes AS-1 + // a public client of the IdP. + ClientSecretFile string + // SubjectClaim is the ID-token claim that names the company identity. + // Empty selects "sub"; Entra ID needs "oid". + SubjectClaim string + // CAFile is a PEM bundle trusted for the IdP's TLS certificate in + // addition to the system roots. Empty trusts the system roots only. + CAFile string + // AllowPrivateAddresses lets AS-1 reach the IdP on a private or + // link-local address, which its SSRF protection otherwise refuses. + // ToolHive admits a loopback issuer host (localhost, 127.0.0.0/8, ::1) + // either way. + AllowPrivateAddresses bool +} + +// Server is a running AS-1. Close releases it. +type Server struct { + issuer *url.URL + routes []string + handler http.Handler + auth *runner.EmbeddedAuthServer +} + +// options are test-only construction settings. +type options struct { + // extraAudiences are further allowed audiences whose tokens carry no + // sub_id. Test-only. + extraAudiences []string + // maxPendingAuthorizations replaces the pending sign-in cap when + // positive. Test-only. + maxPendingAuthorizations int +} + +// New validates cfg, discovers the upstream IdP, and builds AS-1. +func New(ctx context.Context, cfg Config) (*Server, error) { + return newServer(ctx, cfg, options{}) +} + +func newServer(ctx context.Context, cfg Config, opts options) (*Server, error) { + issuer, err := cfg.validate() + if err != nil { + return nil, err + } + audiences := append([]string{cfg.Audience}, opts.extraAudiences...) + store := &cappedStorage{MemoryStorage: storage.NewMemoryStorage(), limit: maxPendingAuthorizations} + if opts.maxPendingAuthorizations > 0 { + store.limit = opts.maxPendingAuthorizations + } + for _, pc := range cfg.PublicClients { + client, err := registration.New(registration.Config{ + ID: pc.ID, + RedirectURIs: slices.Clone(pc.RedirectURIs), + TokenEndpointAuthMethod: oauthproto.TokenEndpointAuthMethodNone, + GrantTypes: []string{oauthproto.GrantTypeAuthorizationCode, oauthproto.GrantTypeRefreshToken}, + ResponseTypes: []string{oauthproto.ResponseTypeCode}, + Scopes: []string{"openid", "profile", "offline_access"}, + Audience: audiences, + }) + if err != nil { + _ = store.Close() + return nil, fmt.Errorf("as1server: construct client %q: %w", pc.ID, err) + } + if err := store.RegisterClient(ctx, client); err != nil { + _ = store.Close() + return nil, fmt.Errorf("as1server: register client %q: %w", pc.ID, err) + } + } + runConfig := &authserver.RunConfig{ + SchemaVersion: "v1", + Issuer: cfg.Issuer, + AllowedAudiences: audiences, + Upstreams: []authserver.UpstreamRunConfig{{Type: authserver.UpstreamProviderTypeOIDC, OIDCConfig: cfg.upstreamRunConfig()}}, + } + if cfg.SigningKeyFile != "" { + runConfig.SigningKeyConfig = &authserver.SigningKeyRunConfig{ + KeyDir: filepath.Dir(cfg.SigningKeyFile), SigningKeyFile: filepath.Base(cfg.SigningKeyFile), + } + } + // On error the constructor closes store. + auth, err := runner.NewEmbeddedAuthServerWithStorage(ctx, runConfig, store, + runner.WithSubjectIdentifierAudiences([]string{cfg.Audience}), + runner.WithAccessTokenType(accessTokenType)) + if err != nil { + return nil, fmt.Errorf("as1server: create authorization server: %w", err) + } + s := &Server{issuer: issuer, auth: auth} + s.routes, s.handler = signInRoutes(issuer, auth.Handler()) + return s, nil +} + +// maxPendingAuthorizations caps the sign-ins AS-1 holds before the user has +// approved them: /oauth/authorize stores one for every anonymous request. +// Sign-ins past that step may use a further tenth so a flood of new ones does +// not fail those already in progress. ToolHive keeps an entry for its +// ten-minute lifetime plus up to one five-minute cleanup interval. +const maxPendingAuthorizations = 10_000 + +// errTooManyPendingAuthorizations refuses a sign-in at the cap. ToolHive +// answers it with a server_error redirect to the client and does not show +// this text to the browser. +var errTooManyPendingAuthorizations = errors.New("as1server: too many pending sign-ins") + +// cappedStorage is ToolHive's memory storage with pending authorizations +// capped. The embedded storage serves every other method, including the +// optional interfaces ToolHive looks for. +type cappedStorage struct { + *storage.MemoryStorage + // mu makes the count and the store one step. + mu sync.Mutex + limit int +} + +// StorePendingAuthorization refuses a new sign-in once limit are pending, and +// one past consent once a further tenth are. +func (s *cappedStorage) StorePendingAuthorization(ctx context.Context, state string, pending *storage.PendingAuthorization) error { + s.mu.Lock() + defer s.mu.Unlock() + limit := s.limit + if pending != nil && pending.ConsentStage != storage.ConsentStageAwaiting { + limit += s.limit / 10 + } + if s.Stats().PendingAuthorizations >= limit { + return errTooManyPendingAuthorizations + } + return s.MemoryStorage.StorePendingAuthorization(ctx, state, pending) +} + +// upstreamRunConfig is ToolHive's configuration of AS-1's upstream IdP. It +// asks for offline access the way OIDC Core requires. +func (cfg Config) upstreamRunConfig() *authserver.OIDCUpstreamRunConfig { + return &authserver.OIDCUpstreamRunConfig{ + IssuerURL: cfg.Upstream.Issuer, + ClientID: cfg.Upstream.ClientID, + ClientSecretFile: cfg.Upstream.ClientSecretFile, + RedirectURI: cfg.Issuer + "/oauth/callback", + Scopes: []string{"openid", "offline_access"}, + AdditionalAuthorizationParams: map[string]string{"prompt": "consent"}, + SubjectClaim: cfg.Upstream.SubjectClaim, + CAFilePath: cfg.Upstream.CAFile, + AllowPrivateIPs: cfg.Upstream.AllowPrivateAddresses, + } +} + +// validate checks cfg and returns the parsed issuer. +func (cfg Config) validate() (*url.URL, error) { + issuer, err := httpsURL("issuer", cfg.Issuer) + if err != nil { + return nil, err + } + if strings.HasSuffix(issuer.Path, "/") || (issuer.Path != "" && path.Clean(issuer.Path) != issuer.Path) || + issuer.EscapedPath() != issuer.Path { + return nil, fmt.Errorf("%w: issuer path must be clean, unescaped, and without a trailing slash", ErrInvalidConfig) + } + switch { + case cfg.Audience == "": + return nil, fmt.Errorf("%w: audience is required", ErrInvalidConfig) + case len(cfg.PublicClients) == 0: + return nil, fmt.Errorf("%w: at least one public client is required", ErrInvalidConfig) + case cfg.Upstream.ClientID == "": + return nil, fmt.Errorf("%w: upstream client ID is required", ErrInvalidConfig) + } + seen := make(map[string]bool, len(cfg.PublicClients)) + for _, pc := range cfg.PublicClients { + switch { + case pc.ID == "": + return nil, fmt.Errorf("%w: public client ID is required", ErrInvalidConfig) + case seen[pc.ID]: + return nil, fmt.Errorf("%w: duplicate public client ID %q", ErrInvalidConfig, pc.ID) + case len(pc.RedirectURIs) == 0: + return nil, fmt.Errorf("%w: public client %q needs at least one redirect URI", ErrInvalidConfig, pc.ID) + } + seen[pc.ID] = true + for _, raw := range pc.RedirectURIs { + if u, err := url.Parse(raw); err != nil || !u.IsAbs() || u.Host == "" || u.Fragment != "" { + return nil, fmt.Errorf("%w: public client %q redirect URI %q must be an absolute URL without a fragment", ErrInvalidConfig, pc.ID, raw) + } + } + } + if _, err := httpsURL("upstream issuer", cfg.Upstream.Issuer); err != nil { + return nil, err + } + return issuer, nil +} + +// httpsURL parses an issuer-style URL: https, with a host, and without +// userinfo, query, or fragment. +func httpsURL(name, raw string) (*url.URL, error) { + if raw == "" { + return nil, fmt.Errorf("%w: %s is required", ErrInvalidConfig, name) + } + u, err := url.Parse(raw) + if err != nil || u.Scheme != "https" || u.Host == "" || u.User != nil || u.RawQuery != "" || u.ForceQuery || u.Fragment != "" { + return nil, fmt.Errorf("%w: %s must be an https URL with a host and without userinfo, query, or fragment", ErrInvalidConfig, name) + } + return u, nil +} + +// signInRoutes lists the routes AS-1 serves under issuer's path and returns a +// handler that serves only those, passing them to embedded with the issuer +// path stripped. Everything else, including registration and the device flow, +// is not found, and the discovery documents leave out the endpoints that +// would be. +func signInRoutes(issuer *url.URL, embedded http.Handler) ([]string, http.Handler) { + issuerPath := issuer.Path + routes := []string{ + issuerPath + "/oauth/authorize", + issuerPath + "/oauth/consent", + issuerPath + "/oauth/token", + issuerPath + "/oauth/callback", + issuerPath + "/.well-known/openid-configuration", + issuerPath + "/.well-known/oauth-authorization-server", + issuerPath + "/.well-known/jwks.json", + } + routed := make(map[string]bool, len(routes)) + for _, route := range routes { + routed[issuer.Scheme+"://"+issuer.Host+route] = true + } + stripped := http.StripPrefix(issuerPath, embedded) + discovery := withoutUnroutedEndpoints(stripped, routed) + mux := http.NewServeMux() + for _, route := range routes { + if strings.HasPrefix(route, issuerPath+"/.well-known/") && !strings.HasSuffix(route, "/jwks.json") { + mux.Handle(route, discovery) + } else { + mux.Handle(route, stripped) + } + } + return routes, mux +} + +// withoutUnroutedEndpoints serves next's discovery document without the +// *_endpoint fields whose URL is not one of routed, such as ToolHive's +// registration and device-flow endpoints. Any other response passes through. +func withoutUnroutedEndpoints(next http.Handler, routed map[string]bool) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + recorded := &bufferedResponse{header: make(http.Header)} + next.ServeHTTP(recorded, r) + if recorded.status == 0 { + recorded.status = http.StatusOK + } + body := recorded.body.Bytes() + var doc map[string]json.RawMessage + if recorded.status == http.StatusOK && json.Unmarshal(body, &doc) == nil { + for key, raw := range doc { + var endpoint string + if strings.HasSuffix(key, "_endpoint") && (json.Unmarshal(raw, &endpoint) != nil || !routed[endpoint]) { + delete(doc, key) + } + } + if rewritten, err := json.Marshal(doc); err == nil { + body = rewritten + } + } + maps.Copy(w.Header(), recorded.header) + w.Header().Set("Content-Length", strconv.Itoa(len(body))) + w.WriteHeader(recorded.status) + _, _ = w.Write(body) + }) +} + +// Issuer is AS-1's issuer URL. +func (s *Server) Issuer() string { return s.issuer.String() } + +// Routes are the exact paths Handler serves, for the caller to mount on its +// mux after checking they collide with nothing already there. +func (s *Server) Routes() []string { return slices.Clone(s.routes) } + +// Handler serves AS-1's routes and nothing else. +func (s *Server) Handler() http.Handler { return s.handler } + +// Close releases the authorization server and its storage. It is idempotent. +func (s *Server) Close() error { return s.auth.Close() } diff --git a/internal/identitybroker/adapter/as1server/as1server_test.go b/internal/identitybroker/adapter/as1server/as1server_test.go new file mode 100644 index 0000000000..c6c84061f2 --- /dev/null +++ b/internal/identitybroker/adapter/as1server/as1server_test.go @@ -0,0 +1,820 @@ +package as1server + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "errors" + "io" + "net" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "slices" + "strconv" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/stacklok/toolhive/pkg/authserver/storage" + + "github.com/stacklok/mecatl/internal/testutil/identityharness" +) + +const ( + testAudience = identityharness.DefaultAudience + otherAudience = "https://other.test/api" + testClientID = "cli" + testRedirect = "http://127.0.0.1:18473/oauth/callback" + testIssuerPath = "/as1" +) + +type fixture struct { + h *identityharness.Harness + server *Server + issuer string +} + +// startAS1 hosts a real AS-1 on loopback TLS with the harness IdP as its +// upstream. The listener exists before the server so the issuer names it. +func startAS1(t *testing.T, h *identityharness.Harness, edit func(*Config)) *fixture { + t.Helper() + return startAS1At(t, h, testIssuerPath, edit) +} + +// startAS1At is startAS1 with the issuer path issuerPath. +func startAS1At(t *testing.T, h *identityharness.Harness, issuerPath string, edit func(*Config)) *fixture { + t.Helper() + return startAS1With(t, h, issuerPath, edit, options{}, nil) +} + +// startAS1With is startAS1At with opts, serving server.Handler() through +// wrap when it is set. +func startAS1With(t *testing.T, h *identityharness.Harness, issuerPath string, edit func(*Config), opts options, wrap func(http.Handler) http.Handler) *fixture { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + issuer := "https://" + ln.Addr().String() + issuerPath + server := newTestServerWith(t, h, issuer, edit, opts) + handler := server.Handler() + if wrap != nil { + handler = wrap(handler) + } + ts := &httptest.Server{Listener: ln, Config: &http.Server{Handler: handler, ReadHeaderTimeout: 10 * time.Second}} + ts.StartTLS() + t.Cleanup(ts.Close) + return &fixture{h: h, server: server, issuer: issuer} +} + +// testConfig configures AS-1 for issuer with the harness IdP, which it +// trusts and admits on loopback, as its upstream, and registers it there. +// Registering again for the same issuer replaces the upstream client secret. +func testConfig(t *testing.T, h *identityharness.Harness, issuer string) Config { + t.Helper() + dir := t.TempDir() + clientID := "as1-" + strings.NewReplacer(":", "-", "/", "-", ".", "-").Replace(strings.TrimPrefix(issuer, "https://")) + secret := rand.Text() + secretFile := filepath.Join(dir, "upstream-client-secret") + if err := os.WriteFile(secretFile, []byte(secret), 0o600); err != nil { + t.Fatal(err) + } + h.IdP.RegisterClient(clientID, secret, issuer+"/oauth/callback") + return Config{ + Issuer: issuer, Audience: testAudience, + PublicClients: []PublicClient{{ID: testClientID, RedirectURIs: []string{testRedirect}}}, + Upstream: UpstreamConfig{ + Issuer: h.IdP.Issuer(), ClientID: clientID, ClientSecretFile: secretFile, + CAFile: h.WriteCACert(t, dir), AllowPrivateAddresses: true, + }, + } +} + +// newTestServer builds AS-1 for issuer from testConfig after edit. +func newTestServer(t *testing.T, h *identityharness.Harness, issuer string, edit func(*Config)) *Server { + t.Helper() + return newTestServerWith(t, h, issuer, edit, options{}) +} + +// newTestServerWith is newTestServer with opts, to which it adds the other +// test audience. +func newTestServerWith(t *testing.T, h *identityharness.Harness, issuer string, edit func(*Config), opts options) *Server { + t.Helper() + cfg := testConfig(t, h, issuer) + if edit != nil { + edit(&cfg) + } + opts.extraAudiences = append(opts.extraAudiences, otherAudience) + server, err := newServer(t.Context(), cfg, opts) + if err != nil { + t.Fatalf("newServer: %v", err) + } + t.Cleanup(func() { + if err := server.Close(); err != nil { + t.Errorf("Close: %v", err) + } + }) + return server +} + +type tokenResponse struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + TokenType string `json:"token_type"` + Error string `json:"error"` + status int +} + +// signIn signs the default harness user in as the test client, with PKCE, +// asking for a token for resource. +func (f *fixture) signIn(t *testing.T, resource string) tokenResponse { + t.Helper() + return f.signInAs(t, PublicClient{ID: testClientID, RedirectURIs: []string{testRedirect}}, resource) +} + +// signInAs is signIn as client, at its first redirect URI. +func (f *fixture) signInAs(t *testing.T, client PublicClient, resource string) tokenResponse { + t.Helper() + q, verifier := authorizationRequest(client, resource) + state := q.Get("state") + back, err := f.h.AuthorizeWithConsent(t.Context(), f.issuer+"/oauth/authorize?"+q.Encode()) + if err != nil { + t.Fatalf("authorize: %v", err) + } + if back.Get("error") != "" || back.Get("state") != state || back.Get("code") == "" { + t.Fatalf("authorize redirect: error=%q description=%q state match=%v", back.Get("error"), back.Get("error_description"), back.Get("state") == state) + } + tokens := f.token(t, url.Values{ + "grant_type": {"authorization_code"}, + "client_id": {client.ID}, + "code": {back.Get("code")}, + "redirect_uri": {client.RedirectURIs[0]}, + "code_verifier": {verifier}, + "resource": {resource}, + }) + if tokens.status != http.StatusOK || tokens.AccessToken == "" { + t.Fatalf("code exchange: status %d error %q", tokens.status, tokens.Error) + } + return tokens +} + +// authorizationRequest is the query of client's PKCE authorization request +// for resource, and its code verifier. +func authorizationRequest(client PublicClient, resource string) (url.Values, string) { + verifier := rand.Text() + rand.Text() + sum := sha256.Sum256([]byte(verifier)) + return url.Values{ + "response_type": {"code"}, + "client_id": {client.ID}, + "redirect_uri": {client.RedirectURIs[0]}, + "scope": {"openid offline_access"}, + "state": {rand.Text()}, + "code_challenge": {base64.RawURLEncoding.EncodeToString(sum[:])}, + "code_challenge_method": {"S256"}, + "resource": {resource}, + }, verifier +} + +// startAuthorization sends the test client's authorization request without +// following redirects and returns the response status, its Location (nil +// without one), and its body. +func (f *fixture) startAuthorization(t *testing.T) (int, *url.URL, string) { + t.Helper() + q, _ := authorizationRequest(PublicClient{ID: testClientID, RedirectURIs: []string{testRedirect}}, testAudience) + client := f.h.HTTPClient() + client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } + resp, err := client.Get(f.issuer + "/oauth/authorize?" + q.Encode()) + if err != nil { + t.Errorf("authorize: %v", err) + return 0, nil, "" + } + defer func() { _ = resp.Body.Close() }() + body, _ := io.ReadAll(resp.Body) + location, _ := resp.Location() + return resp.StatusCode, location, string(body) +} + +func (f *fixture) refresh(t *testing.T, refreshToken, resource string) tokenResponse { + t.Helper() + return f.token(t, url.Values{ + "grant_type": {"refresh_token"}, + "client_id": {testClientID}, + "refresh_token": {refreshToken}, + "resource": {resource}, + }) +} + +func (f *fixture) token(t *testing.T, form url.Values) tokenResponse { + t.Helper() + resp, err := f.h.HTTPClient().PostForm(f.issuer+"/oauth/token", form) + if err != nil { + t.Fatalf("token request: %v", err) + } + defer func() { _ = resp.Body.Close() }() + var out tokenResponse + if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { + t.Fatalf("token response: %v", err) + } + out.status = resp.StatusCode + return out +} + +// parse decodes an access token without checking its signature. +func parse(t *testing.T, token string) (map[string]any, jwt.MapClaims) { + t.Helper() + claims := jwt.MapClaims{} + parsed, _, err := jwt.NewParser().ParseUnverified(token, claims) + if err != nil { + t.Fatalf("parse access token: %v", err) + } + return parsed.Header, claims +} + +// jwks fetches server's key set through its in-process client. +func jwks(t *testing.T, server *Server) map[string]*ecdsa.PublicKey { + t.Helper() + resp, err := server.InProcessClient().Get(server.Issuer() + "/.well-known/jwks.json") + if err != nil { + t.Fatalf("JWKS: %v", err) + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusOK { + t.Fatalf("JWKS: status %d", resp.StatusCode) + } + var set struct { + Keys []struct{ Kid, Kty, Crv, X, Y string } `json:"keys"` + } + if err := json.NewDecoder(resp.Body).Decode(&set); err != nil { + t.Fatalf("JWKS: %v", err) + } + keys := make(map[string]*ecdsa.PublicKey, len(set.Keys)) + for _, k := range set.Keys { + if k.Kty != "EC" || k.Crv != "P-256" { + t.Fatalf("JWKS: unexpected key type %s %s", k.Kty, k.Crv) + } + x, xErr := base64.RawURLEncoding.DecodeString(k.X) + y, yErr := base64.RawURLEncoding.DecodeString(k.Y) + if xErr != nil || yErr != nil { + t.Fatal("JWKS: malformed key") + } + key, err := ecdsa.ParseUncompressedPublicKey(elliptic.P256(), slices.Concat([]byte{4}, x, y)) + if err != nil { + t.Fatalf("JWKS: %v", err) + } + keys[k.Kid] = key + } + if len(keys) == 0 { + t.Fatal("JWKS: no keys") + } + return keys +} + +// verify checks token's signature against keys. +func verify(token string, keys map[string]*ecdsa.PublicKey) error { + _, err := jwt.Parse(token, func(tok *jwt.Token) (any, error) { + kid, _ := tok.Header["kid"].(string) + key, ok := keys[kid] + if !ok { + return nil, errors.New("unknown kid") + } + return key, nil + }, jwt.WithValidMethods([]string{"ES256"})) + return err +} + +func TestSignInIssuesAudienceTokenWithCompanyIdentity(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + f := startAS1(t, h, nil) + + tokens := f.signIn(t, testAudience) + + header, claims := parse(t, tokens.AccessToken) + if header["typ"] != "at+jwt" { + t.Errorf("typ = %v, want at+jwt", header["typ"]) + } + if aud, _ := claims.GetAudience(); !slices.Equal(aud, jwt.ClaimStrings{testAudience}) { + t.Errorf("aud = %v, want [%s]", aud, testAudience) + } + if iss, _ := claims.GetIssuer(); iss != f.issuer { + t.Errorf("iss = %q, want %q", iss, f.issuer) + } + if claims["client_id"] != testClientID { + t.Errorf("client_id = %v, want %s", claims["client_id"], testClientID) + } + if jti, _ := claims["jti"].(string); jti == "" { + t.Error("jti missing") + } + if sub, _ := claims.GetSubject(); sub == "" { + t.Error("sub missing") + } + wantSubID := map[string]any{"format": "iss_sub", "iss": h.IdP.Issuer(), "sub": identityharness.Alice} + if subID, _ := claims["sub_id"].(map[string]any); !mapsEqual(subID, wantSubID) { + t.Errorf("sub_id = %v, want %v", claims["sub_id"], wantSubID) + } + if tokens.RefreshToken == "" { + t.Error("no refresh token for offline_access") + } + if got := h.IdP.RefreshTokensIssued(identityharness.Alice); got != 1 { + t.Errorf("upstream refresh tokens issued = %d, want 1 (offline_access with prompt=consent)", got) + } + if err := verify(tokens.AccessToken, jwks(t, f.server)); err != nil { + t.Errorf("access token does not verify against AS-1's JWKS: %v", err) + } +} + +func mapsEqual(got, want map[string]any) bool { + if len(got) != len(want) { + return false + } + for k, v := range want { + if got[k] != v { + return false + } + } + return true +} + +func TestEveryPublicClientCanSignIn(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + second := PublicClient{ID: "studio", RedirectURIs: []string{"http://127.0.0.1:18474/callback"}} + f := startAS1(t, h, func(c *Config) { c.PublicClients = append(c.PublicClients, second) }) + + for _, client := range []PublicClient{{ID: testClientID, RedirectURIs: []string{testRedirect}}, second} { + _, claims := parse(t, f.signInAs(t, client, testAudience).AccessToken) + if claims["client_id"] != client.ID { + t.Errorf("client_id = %v, want %s", claims["client_id"], client.ID) + } + } +} + +// TestSubjectClaimReachesTheUpstreamSubject: the harness IdP's ID tokens +// carry an oid claim that differs from sub, and the subject claim picks which +// one AS-1 keys the user on, which ToolHive's upstream_subject claim carries. +func TestSubjectClaimReachesTheUpstreamSubject(t *testing.T) { + for name, c := range map[string]struct { + claim, wantClaim, wantValue string + }{ + "default": {claim: "", wantClaim: "sub", wantValue: identityharness.Alice}, + "oid": {claim: "oid", wantClaim: "oid", wantValue: identityharness.OID(identityharness.Alice)}, + } { + t.Run(name, func(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + f := startAS1(t, h, func(cfg *Config) { cfg.Upstream.SubjectClaim = c.claim }) + + _, claims := parse(t, f.signIn(t, testAudience).AccessToken) + + want := map[string]any{"iss": h.IdP.Issuer(), "claim": c.wantClaim, "value": c.wantValue} + if got, _ := claims["https://toolhive.dev/upstream_subject"].(map[string]any); !mapsEqual(got, want) { + t.Errorf("upstream_subject = %v, want %v", claims["https://toolhive.dev/upstream_subject"], want) + } + }) + } +} + +// TestRootIssuerServesAtTheRoot hosts AS-1 at an issuer without a path: its +// routes sit at the root, sign-in works, and the in-process client reaches it. +func TestRootIssuerServesAtTheRoot(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + f := startAS1At(t, h, "", nil) + + wantRoutes := []string{ + "/oauth/authorize", "/oauth/consent", "/oauth/token", "/oauth/callback", + "/.well-known/openid-configuration", "/.well-known/oauth-authorization-server", "/.well-known/jwks.json", + } + if got := f.server.Routes(); !slices.Equal(got, wantRoutes) { + t.Errorf("Routes() = %v, want %v", got, wantRoutes) + } + tokens := f.signIn(t, testAudience) + if _, claims := parse(t, tokens.AccessToken); claims["iss"] != f.issuer { + t.Errorf("iss = %v, want %s", claims["iss"], f.issuer) + } + if err := verify(tokens.AccessToken, jwks(t, f.server)); err != nil { + t.Errorf("access token does not verify against the in-process JWKS: %v", err) + } + + issuer := "https://as1.mecatl.test" + client := newTestServer(t, h, issuer, nil).InProcessClient() + resp, err := client.Get(issuer + "/.well-known/openid-configuration") + if err != nil { + t.Fatalf("in-process discovery: %v", err) + } + var discovery struct { + Issuer string `json:"issuer"` + JWKSURI string `json:"jwks_uri"` + } + err = json.NewDecoder(resp.Body).Decode(&discovery) + _ = resp.Body.Close() + if err != nil || resp.StatusCode != http.StatusOK || discovery.Issuer != issuer || discovery.JWKSURI != issuer+"/.well-known/jwks.json" { + t.Fatalf("in-process discovery: status %d, issuer %q, jwks_uri %q, err %v", resp.StatusCode, discovery.Issuer, discovery.JWKSURI, err) + } + if resp, err := client.Get("https://other.mecatl.test/.well-known/jwks.json"); err == nil { + _ = resp.Body.Close() + t.Error("in-process client served another host") + } +} + +// TestUpstreamCATrustIsSeparateFromPrivateAddresses: trusting a CA for the +// upstream does not switch off its SSRF protection, and only the CA file makes +// the harness IdP's certificate trusted. ToolHive admits a loopback issuer +// host whatever AllowPrivateAddresses says, so the private-address half is +// checked on the configuration AS-1 hands ToolHive. +func TestUpstreamCATrustIsSeparateFromPrivateAddresses(t *testing.T) { + caOnly := Config{Issuer: "https://as1.mecatl.test", Upstream: UpstreamConfig{CAFile: "/ca.pem"}}.upstreamRunConfig() + if caOnly.CAFilePath != "/ca.pem" || caOnly.AllowPrivateIPs { + t.Errorf("CA file alone: CAFilePath %q AllowPrivateIPs %v, want the CA and no private addresses", caOnly.CAFilePath, caOnly.AllowPrivateIPs) + } + privateOnly := Config{Upstream: UpstreamConfig{AllowPrivateAddresses: true}}.upstreamRunConfig() + if privateOnly.CAFilePath != "" || !privateOnly.AllowPrivateIPs { + t.Errorf("private addresses alone: CAFilePath %q AllowPrivateIPs %v", privateOnly.CAFilePath, privateOnly.AllowPrivateIPs) + } + + h := identityharness.New(t, identityharness.Options{}) + cfg := testConfig(t, h, "https://as1.mecatl.test"+testIssuerPath) + cfg.Upstream.CAFile = "" + if server, err := New(t.Context(), cfg); err == nil { + _ = server.Close() + t.Fatal("New trusted the harness IdP without its CA file") + } + cfg.Upstream.CAFile, cfg.Upstream.AllowPrivateAddresses = h.WriteCACert(t, t.TempDir()), false + server, err := New(t.Context(), cfg) + if err != nil { + t.Fatalf("New with the CA file and a loopback upstream: %v", err) + } + _ = server.Close() +} + +func TestTokenForAnotherAudienceCarriesNoSubjectIdentifier(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + f := startAS1(t, h, nil) + + _, claims := parse(t, f.signIn(t, otherAudience).AccessToken) + + if aud, _ := claims.GetAudience(); !slices.Equal(aud, jwt.ClaimStrings{otherAudience}) { + t.Fatalf("aud = %v, want [%s]", aud, otherAudience) + } + if _, ok := claims["sub_id"]; ok { + t.Errorf("sub_id present for audience %s: %v", otherAudience, claims["sub_id"]) + } +} + +func TestRepeatedSignInKeepsSubject(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + f := startAS1(t, h, nil) + + _, first := parse(t, f.signIn(t, testAudience).AccessToken) + _, second := parse(t, f.signIn(t, testAudience).AccessToken) + + firstSub, _ := first.GetSubject() + secondSub, _ := second.GetSubject() + if firstSub == "" || firstSub != secondSub { + t.Errorf("sub changed across sign-ins: %q then %q", firstSub, secondSub) + } + if first["jti"] == second["jti"] { + t.Error("two sign-ins share a jti") + } +} + +func TestRefreshRotatesAndChecksResource(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + f := startAS1(t, h, nil) + signedIn := f.signIn(t, testAudience) + _, signedInClaims := parse(t, signedIn.AccessToken) + + if wrong := f.refresh(t, signedIn.RefreshToken, "https://elsewhere.test/api"); wrong.status == http.StatusOK { + t.Fatal("refresh for a resource that is not an allowed audience succeeded") + } + + refreshed := f.refresh(t, signedIn.RefreshToken, testAudience) + if refreshed.status != http.StatusOK { + t.Fatalf("refresh: status %d error %q", refreshed.status, refreshed.Error) + } + if refreshed.RefreshToken == "" || refreshed.RefreshToken == signedIn.RefreshToken { + t.Error("refresh token did not rotate") + } + _, claims := parse(t, refreshed.AccessToken) + if aud, _ := claims.GetAudience(); !slices.Equal(aud, jwt.ClaimStrings{testAudience}) { + t.Errorf("refreshed aud = %v", aud) + } + if claims["sub"] != signedInClaims["sub"] || claims["sub_id"] == nil { + t.Errorf("refreshed token lost the identity: sub %v sub_id %v", claims["sub"], claims["sub_id"]) + } + + if reused := f.refresh(t, signedIn.RefreshToken, testAudience); reused.status == http.StatusOK { + t.Error("a rotated refresh token was accepted again") + } +} + +func TestSigningKeyFileSurvivesRestart(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + keyFile := h.WriteSigningKey(t, t.TempDir()) + withKey := func(cfg *Config) { cfg.SigningKeyFile = keyFile } + before := startAS1(t, h, withKey) + token := before.signIn(t, testAudience).AccessToken + + // The restarted instance has the same issuer and key file, and is never + // served: its JWKS comes through the in-process client. + after := newTestServer(t, h, before.issuer, withKey) + + if err := verify(token, jwks(t, after)); err != nil { + t.Errorf("token from the first instance does not verify against the restarted instance's JWKS: %v", err) + } +} + +func TestGeneratedSigningKeysDifferPerInstance(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + f := startAS1(t, h, nil) + token := f.signIn(t, testAudience).AccessToken + other := newTestServer(t, h, f.issuer, nil) + + first, second := jwks(t, f.server), jwks(t, other) + for kid := range first { + if _, shared := second[kid]; shared { + t.Errorf("generated key %q shared by two instances", kid) + } + } + if err := verify(token, second); err == nil { + t.Error("token verified against another instance's generated keys") + } +} + +func TestServesOnlySignInRoutes(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + f := startAS1(t, h, nil) + client := h.HTTPClient() + origin := strings.TrimSuffix(f.issuer, testIssuerPath) + + for _, path := range []string{ + testIssuerPath + "/.well-known/openid-configuration", + testIssuerPath + "/.well-known/oauth-authorization-server", + testIssuerPath + "/.well-known/jwks.json", + } { + resp, err := client.Get(origin + path) + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Errorf("GET %s: status %d, want 200", path, resp.StatusCode) + } + } + for _, path := range []string{ + testIssuerPath + "/oauth/register", + testIssuerPath + "/oauth/device_authorization", + testIssuerPath + "/oauth/device", + testIssuerPath + "/oauth/device/confirm", + "/oauth/token", + "/.well-known/openid-configuration", + } { + for _, method := range []string{http.MethodGet, http.MethodPost} { + req, err := http.NewRequestWithContext(t.Context(), method, origin+path, strings.NewReader("{}")) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Content-Type", "application/json") + resp, err := client.Do(req) + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusNotFound { + t.Errorf("%s %s: status %d, want 404", method, path, resp.StatusCode) + } + } + } + + wantRoutes := []string{ + testIssuerPath + "/oauth/authorize", + testIssuerPath + "/oauth/consent", + testIssuerPath + "/oauth/token", + testIssuerPath + "/oauth/callback", + testIssuerPath + "/.well-known/openid-configuration", + testIssuerPath + "/.well-known/oauth-authorization-server", + testIssuerPath + "/.well-known/jwks.json", + } + if got := f.server.Routes(); !slices.Equal(got, wantRoutes) { + t.Errorf("Routes() = %v, want %v", got, wantRoutes) + } +} + +// TestDiscoveryOmitsUnroutedEndpoints: both discovery documents keep the +// endpoints AS-1 serves and drop every other endpoint ToolHive advertises, +// with a Content-Length that matches the rewritten body. +func TestDiscoveryOmitsUnroutedEndpoints(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + f := startAS1(t, h, nil) + for _, doc := range []string{"/.well-known/openid-configuration", "/.well-known/oauth-authorization-server"} { + resp, err := h.HTTPClient().Get(f.issuer + doc) + if err != nil { + t.Fatal(err) + } + body, err := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if err != nil || resp.StatusCode != http.StatusOK { + t.Fatalf("GET %s: status %d: %v", doc, resp.StatusCode, err) + } + if resp.Header.Get("Content-Length") != strconv.Itoa(len(body)) || resp.Header.Get("Content-Type") != "application/json" { + t.Errorf("GET %s: Content-Length %q Content-Type %q for a %d-byte body", doc, resp.Header.Get("Content-Length"), resp.Header.Get("Content-Type"), len(body)) + } + var got map[string]any + if err := json.Unmarshal(body, &got); err != nil { + t.Fatalf("GET %s: %v", doc, err) + } + for key, want := range map[string]string{ + "authorization_endpoint": f.issuer + "/oauth/authorize", + "token_endpoint": f.issuer + "/oauth/token", + "jwks_uri": f.issuer + "/.well-known/jwks.json", + "issuer": f.issuer, + } { + if got[key] != want { + t.Errorf("GET %s: %s = %v, want %s", doc, key, got[key], want) + } + } + for key := range got { + if strings.HasSuffix(key, "_endpoint") && key != "authorization_endpoint" && key != "token_endpoint" { + t.Errorf("GET %s advertises %s = %v, which AS-1 does not serve", doc, key, got[key]) + } + } + } + + // Endpoints ToolHive advertises in other configurations, such as the + // device flow, are dropped the same way. + routed := map[string]bool{"https://as1.test/oauth/token": true} + upstream := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Length", "999") + _, _ = w.Write([]byte(`{"token_endpoint":"https://as1.test/oauth/token","device_authorization_endpoint":"https://as1.test/oauth/device_authorization",` + + `"revocation_endpoint":"https://elsewhere.test/oauth/token","odd_endpoint":7,"scopes_supported":["openid"]}`)) + }) + rec := httptest.NewRecorder() + withoutUnroutedEndpoints(upstream, routed).ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/", nil)) + want := `{"scopes_supported":["openid"],"token_endpoint":"https://as1.test/oauth/token"}` + if rec.Body.String() != want || rec.Header().Get("Content-Length") != strconv.Itoa(len(want)) { + t.Errorf("rewritten document = %s (Content-Length %s), want %s", rec.Body, rec.Header().Get("Content-Length"), want) + } +} + +// TestPendingSignInsAreCapped: once the cap of pending sign-ins is reached a +// new authorization request gets a clean server_error redirect, a sign-in +// already in progress still completes, and finished sign-ins free capacity. +func TestPendingSignInsAreCapped(t *testing.T) { + const limit = 3 + h := identityharness.New(t, identityharness.Options{}) + var f *fixture + var filled atomic.Bool + // When the sign-in in progress approves its consent page, it holds one + // pending authorization: fill the rest and check the next is refused. + fillAtConsent := func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/oauth/consent") && filled.CompareAndSwap(false, true) { + for range limit - 1 { + if status, _, _ := f.startAuthorization(t); status != http.StatusOK { + t.Errorf("authorization below the cap: status %d, want the consent page", status) + } + } + checkRefused(t, f) + } + next.ServeHTTP(w, r) + }) + } + f = startAS1With(t, h, testIssuerPath, nil, options{maxPendingAuthorizations: limit}, fillAtConsent) + + f.signIn(t, testAudience) + if !filled.Load() { + t.Fatal("the sign-in never posted its consent page") + } + // The completed sign-in freed its entry; the next one takes it. + if status, _, _ := f.startAuthorization(t); status != http.StatusOK { + t.Errorf("authorization after a sign-in completed: status %d, want the consent page", status) + } + checkRefused(t, f) +} + +// checkRefused checks that a new authorization request at f gets ToolHive's +// server_error redirect to the client, without a 500 or the storage error. +func checkRefused(t *testing.T, f *fixture) { + t.Helper() + status, location, body := f.startAuthorization(t) + if status != http.StatusFound && status != http.StatusSeeOther { + t.Errorf("authorization at the cap: status %d, want a redirect to the client", status) + return + } + if location == nil || !strings.HasPrefix(location.String(), testRedirect+"?") || location.Query().Get("error") != "server_error" { + t.Errorf("authorization at the cap redirected to %v, want server_error at the client", location) + } + if strings.Contains(location.String()+body, errTooManyPendingAuthorizations.Error()) { + t.Errorf("authorization at the cap leaked the storage error: %v %s", location, body) + } +} + +// TestPendingCapReservesRoomPastConsent: new sign-ins stop at the cap, and +// those past consent may use a further tenth. +func TestPendingCapReservesRoomPastConsent(t *testing.T) { + store := &cappedStorage{MemoryStorage: storage.NewMemoryStorage(), limit: 10} + t.Cleanup(func() { _ = store.Close() }) + pending := func(stage string) *storage.PendingAuthorization { + return &storage.PendingAuthorization{ConsentStage: stage, CreatedAt: time.Now()} + } + for i := range 10 { + if err := store.StorePendingAuthorization(t.Context(), rand.Text(), pending(storage.ConsentStageAwaiting)); err != nil { + t.Fatalf("new sign-in %d below the cap: %v", i, err) + } + } + if err := store.StorePendingAuthorization(t.Context(), rand.Text(), pending(storage.ConsentStageAwaiting)); !errors.Is(err, errTooManyPendingAuthorizations) { + t.Fatalf("new sign-in at the cap: %v, want refused", err) + } + if err := store.StorePendingAuthorization(t.Context(), rand.Text(), pending(storage.ConsentStageApproved)); err != nil { + t.Fatalf("sign-in past consent at the cap: %v", err) + } + if err := store.StorePendingAuthorization(t.Context(), rand.Text(), pending(storage.ConsentStageApproved)); !errors.Is(err, errTooManyPendingAuthorizations) { + t.Fatalf("sign-in past consent beyond the reserve: %v, want refused", err) + } +} + +func TestInProcessClientServesOnlyTheIssuer(t *testing.T) { + h := identityharness.New(t, identityharness.Options{}) + issuer := "https://as1.mecatl.test" + testIssuerPath + server := newTestServer(t, h, issuer, nil) + client := server.InProcessClient() + + resp, err := client.Get(issuer + "/.well-known/openid-configuration") + if err != nil { + t.Fatalf("discovery: %v", err) + } + var discovery struct { + Issuer string `json:"issuer"` + JWKSURI string `json:"jwks_uri"` + } + err = json.NewDecoder(resp.Body).Decode(&discovery) + _ = resp.Body.Close() + if err != nil || resp.StatusCode != http.StatusOK { + t.Fatalf("discovery: status %d: %v", resp.StatusCode, err) + } + if discovery.Issuer != issuer || discovery.JWKSURI != issuer+"/.well-known/jwks.json" { + t.Errorf("discovery issuer %q jwks_uri %q", discovery.Issuer, discovery.JWKSURI) + } + jwks(t, server) + + for _, refused := range []string{ + "http://as1.mecatl.test" + testIssuerPath + "/.well-known/jwks.json", + "https://other.mecatl.test" + testIssuerPath + "/.well-known/jwks.json", + "https://as1.mecatl.test:8443" + testIssuerPath + "/.well-known/jwks.json", + "https://as1.mecatl.test/.well-known/openid-configuration", + "https://as1.mecatl.test" + testIssuerPath + "x/.well-known/jwks.json", + "https://as1.mecatl.test" + testIssuerPath + "/../.well-known/jwks.json", + } { + resp, err := client.Get(refused) + if err == nil { + _ = resp.Body.Close() + t.Errorf("in-process client served %s: status %d", refused, resp.StatusCode) + } + } +} + +func TestConfigValidation(t *testing.T) { + valid := Config{ + Issuer: "https://as1.mecatl.test/as1", Audience: testAudience, + Upstream: UpstreamConfig{Issuer: "https://idp.example", ClientID: "mecatl-as1"}, + } + client := PublicClient{ID: testClientID, RedirectURIs: []string{testRedirect}} + for name, edit := range map[string]func(*Config){ + "empty issuer": func(c *Config) { c.Issuer = "" }, + "http issuer": func(c *Config) { c.Issuer = "http://as1.mecatl.test/as1" }, + "issuer with query": func(c *Config) { c.Issuer = "https://as1.mecatl.test/as1?x=1" }, + "issuer with fragment": func(c *Config) { c.Issuer = "https://as1.mecatl.test/as1#x" }, + "issuer with userinfo": func(c *Config) { c.Issuer = "https://u@as1.mecatl.test/as1" }, + "issuer trailing slash": func(c *Config) { c.Issuer = "https://as1.mecatl.test/as1/" }, + "issuer unclean path": func(c *Config) { c.Issuer = "https://as1.mecatl.test/a/../as1" }, + "empty audience": func(c *Config) { c.Audience = "" }, + "no public clients": func(c *Config) { c.PublicClients = nil }, + "empty client ID": func(c *Config) { c.PublicClients[0].ID = "" }, + "duplicate client IDs": func(c *Config) { c.PublicClients = append(c.PublicClients, client) }, + "no redirect URIs": func(c *Config) { c.PublicClients[0].RedirectURIs = nil }, + "relative redirect URI": func(c *Config) { c.PublicClients[0].RedirectURIs = []string{"/oauth/callback"} }, + "redirect URI fragment": func(c *Config) { c.PublicClients[0].RedirectURIs = []string{testRedirect + "#x"} }, + "empty upstream issuer": func(c *Config) { c.Upstream.Issuer = "" }, + "http upstream issuer": func(c *Config) { c.Upstream.Issuer = "http://idp.example" }, + "empty upstream client": func(c *Config) { c.Upstream.ClientID = "" }, + "upstream issuer query": func(c *Config) { c.Upstream.Issuer = "https://idp.example?x=1" }, + "upstream issuer no host": func(c *Config) { c.Upstream.Issuer = "https:///tenant" }, + } { + t.Run(name, func(t *testing.T) { + cfg := valid + cfg.PublicClients = []PublicClient{client} + edit(&cfg) + server, err := New(t.Context(), cfg) + if server != nil { + _ = server.Close() + } + if !errors.Is(err, ErrInvalidConfig) { + t.Errorf("New: %v, want ErrInvalidConfig", err) + } + }) + } +} diff --git a/internal/identitybroker/adapter/as1server/inprocess.go b/internal/identitybroker/adapter/as1server/inprocess.go new file mode 100644 index 0000000000..9dbfff184d --- /dev/null +++ b/internal/identitybroker/adapter/as1server/inprocess.go @@ -0,0 +1,77 @@ +package as1server + +import ( + "bytes" + "fmt" + "io" + "net/http" + "path" + "strings" +) + +// InProcessClient returns a client that serves requests for AS-1's issuer +// straight from Handler, without a network listener, so a validator in the +// same process can fetch discovery and keys before listeners start. It +// refuses any request outside the issuer's scheme, host, and path. +func (s *Server) InProcessClient() *http.Client { + return &http.Client{ + Transport: inProcessTransport{server: s}, + CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, + } +} + +type inProcessTransport struct{ server *Server } + +func (t inProcessTransport) RoundTrip(req *http.Request) (*http.Response, error) { + if req.Body != nil { + defer func() { _ = req.Body.Close() }() + } + issuer := t.server.issuer + p := req.URL.Path + if req.URL.Scheme != issuer.Scheme || req.URL.Host != issuer.Host || req.URL.User != nil || + path.Clean(p) != p || !strings.HasPrefix(p, issuer.Path+"/") { + return nil, fmt.Errorf("as1server: in-process client refuses %s://%s%s: outside the issuer", req.URL.Scheme, req.URL.Host, p) + } + inner := req.Clone(req.Context()) + inner.RequestURI = req.URL.RequestURI() + inner.Host = issuer.Host + if inner.Body == nil { + inner.Body = http.NoBody + } + recorded := &bufferedResponse{header: make(http.Header)} + t.server.handler.ServeHTTP(recorded, inner) + if recorded.status == 0 { + recorded.status = http.StatusOK + } + return &http.Response{ + Status: fmt.Sprintf("%d %s", recorded.status, http.StatusText(recorded.status)), + StatusCode: recorded.status, + Proto: "HTTP/1.1", + ProtoMajor: 1, + ProtoMinor: 1, + Header: recorded.header, + Body: io.NopCloser(bytes.NewReader(recorded.body.Bytes())), + ContentLength: int64(recorded.body.Len()), + Request: req, + }, nil +} + +// bufferedResponse collects a handler's response in memory. +type bufferedResponse struct { + header http.Header + status int + body bytes.Buffer +} + +func (b *bufferedResponse) Header() http.Header { return b.header } + +func (b *bufferedResponse) WriteHeader(status int) { + if b.status == 0 { + b.status = status + } +} + +func (b *bufferedResponse) Write(p []byte) (int, error) { + b.WriteHeader(http.StatusOK) + return b.body.Write(p) +} diff --git a/internal/testutil/identityharness/browser.go b/internal/testutil/identityharness/browser.go new file mode 100644 index 0000000000..db87178775 --- /dev/null +++ b/internal/testutil/identityharness/browser.go @@ -0,0 +1,98 @@ +package identityharness + +import ( + "context" + "fmt" + "html" + "io" + "net/http" + "net/http/cookiejar" + "net/url" + "regexp" + "strings" +) + +var ( + consentFormAction = regexp.MustCompile(`]*\saction="([^"]*)"`) + consentFormHandle = regexp.MustCompile(``) +) + +// AuthorizeWithConsent plays the browser for a real client's authorization +// request at a real AS-1 that shows a consent page, as ToolHive's embedded +// authorization server does for public clients. Unlike Authorize, it keeps +// cookies for the one sign-in (ToolHive binds the sign-in to the browser with +// a cookie), follows 302 and 303 redirects, and approves each consent page by +// posting its form with the page's Origin, as a browser would. It returns the +// query of the redirect to the request's redirect_uri (code, state, or error) +// without contacting the client's redirect URI. +func (h *Harness) AuthorizeWithConsent(ctx context.Context, authorizationURL string) (url.Values, error) { + u, err := url.Parse(authorizationURL) + if err != nil { + return nil, err + } + redirectURI := u.Query().Get("redirect_uri") + jar, err := cookiejar.New(nil) + if err != nil { + return nil, err + } + client := h.HTTPClient() + client.Jar = jar + req, err := http.NewRequestWithContext(ctx, http.MethodGet, authorizationURL, nil) + if err != nil { + return nil, err + } + for range 10 { + resp, err := client.Do(req) + if err != nil { + return nil, err + } + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + _ = resp.Body.Close() + if err != nil { + return nil, err + } + switch resp.StatusCode { + case http.StatusFound, http.StatusSeeOther: + location, err := resp.Location() + if err != nil { + return nil, err + } + if strings.HasPrefix(location.String(), redirectURI+"?") { + return location.Query(), nil + } + req, err = http.NewRequestWithContext(ctx, http.MethodGet, location.String(), nil) + if err != nil { + return nil, err + } + case http.StatusOK: + req, err = approveConsent(ctx, req.URL, body) + if err != nil { + return nil, err + } + default: + return nil, fmt.Errorf("identityharness: %s %s: status %d", req.Method, req.URL.Path, resp.StatusCode) + } + } + return nil, fmt.Errorf("identityharness: too many redirects") +} + +// approveConsent builds the browser's approval of the consent page at page. +func approveConsent(ctx context.Context, page *url.URL, body []byte) (*http.Request, error) { + action := consentFormAction.FindSubmatch(body) + handle := consentFormHandle.FindSubmatch(body) + if action == nil || handle == nil { + return nil, fmt.Errorf("identityharness: GET %s: status 200 without a consent form", page.Path) + } + target, err := page.Parse(html.UnescapeString(string(action[1]))) + if err != nil { + return nil, err + } + form := url.Values{"handle": {html.UnescapeString(string(handle[1]))}, "decision": {"approve"}} + req, err := http.NewRequestWithContext(ctx, http.MethodPost, target.String(), strings.NewReader(form.Encode())) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Origin", page.Scheme+"://"+page.Host) + return req, nil +} diff --git a/internal/testutil/identityharness/harness.go b/internal/testutil/identityharness/harness.go index 215eab4fbe..80b9abd535 100644 --- a/internal/testutil/identityharness/harness.go +++ b/internal/testutil/identityharness/harness.go @@ -32,7 +32,9 @@ // forced grant expiry; and ScriptedBroker, a // hand-driven caller holding the broker's credentials for exercising // AS-1 directly (broker.go). Production code under test uses its own -// client, not ScriptedBroker. +// client, not ScriptedBroker; +// - a browser for tests hosting the real AS-1, which keeps cookies and +// approves ToolHive's consent page (browser.go). // // Keeping one live refresh token per session, checking the owner, and // refusing to reopen a closed session are the broker's job, so AS-1 does @@ -61,6 +63,9 @@ package identityharness import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" "crypto/tls" "crypto/x509" "encoding/pem" @@ -201,6 +206,45 @@ func (h *Harness) WriteCACert(t testing.TB, dir string) string { return path } +// WriteSigningKey writes a new P-256 private key in PEM to a file in dir and +// returns its path, for a real AS-1's signing key file. +func (*Harness) WriteSigningKey(t testing.TB, dir string) string { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("identityharness: signing key: %v", err) + } + der, err := x509.MarshalECPrivateKey(key) + if err != nil { + t.Fatalf("identityharness: marshal signing key: %v", err) + } + path := filepath.Join(dir, "as1-signing-key.pem") + if err := os.WriteFile(path, pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: der}), 0o600); err != nil { + t.Fatalf("identityharness: write signing key: %v", err) + } + return path +} + +// WriteServerKeyPair writes the stand-ins' loopback TLS certificate and its +// private key to files in dir and returns their paths, so a server under test +// (such as mecak8s with --tls-cert and --tls-key) is trusted wherever the +// stand-ins are, including by HTTPClient. +func (h *Harness) WriteServerKeyPair(t testing.TB, dir string) (certFile, keyFile string) { + t.Helper() + der, err := x509.MarshalPKCS8PrivateKey(h.IdP.server.TLS.Certificates[0].PrivateKey) + if err != nil { + t.Fatalf("identityharness: marshal TLS key: %v", err) + } + certFile, keyFile = filepath.Join(dir, "identityharness-tls.crt"), filepath.Join(dir, "identityharness-tls.key") + if err := os.WriteFile(certFile, h.certPEM, 0o600); err != nil { + t.Fatalf("identityharness: write TLS certificate: %v", err) + } + if err := os.WriteFile(keyFile, pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: der}), 0o600); err != nil { + t.Fatalf("identityharness: write TLS key: %v", err) + } + return certFile, keyFile +} + // faults holds one-shot failures a test forces on a stand-in endpoint. type faults struct { mu sync.Mutex diff --git a/internal/testutil/identityharness/idp.go b/internal/testutil/identityharness/idp.go index b6405fc930..d4a5a5cb89 100644 --- a/internal/testutil/identityharness/idp.go +++ b/internal/testutil/identityharness/idp.go @@ -81,6 +81,10 @@ func newIdP(t testing.TB, opts Options) *IdP { // Issuer is the IdP's issuer, which AS-1 access tokens carry as sub_id.iss. func (p *IdP) Issuer() string { return p.server.URL } +// OID is the stable object ID the IdP's ID tokens carry for subject in the +// oid claim, as Entra ID does. It differs from subject. +func OID(subject string) string { return "oid-" + subject } + // AddUser lets the IdP sign in subject. func (p *IdP) AddUser(subject string) { p.mu.Lock() @@ -229,6 +233,7 @@ func (p *IdP) token(w http.ResponseWriter, r *http.Request) { "iss": p.server.URL, "sub": code.subject, "aud": clientID, "iat": now.Unix(), "exp": now.Add(idpIDTokenLife).Unix(), "email": code.subject + "@example.com", + "oid": OID(code.subject), } if code.nonce != "" { claims["nonce"] = code.nonce diff --git a/user-docs/features/security-and-execution/caller-identity.md b/user-docs/features/security-and-execution/caller-identity.md index f69b5d1b9a..de70d2aac6 100644 --- a/user-docs/features/security-and-execution/caller-identity.md +++ b/user-docs/features/security-and-execution/caller-identity.md @@ -144,6 +144,8 @@ The server refuses to start when: list. - In `mecak8s`, `--enable-identity-broker` is set with `--oidc-issuer` but without `--oidc-owner-identity=sub_id`. +- In `mecak8s`, the hosted AS-1 flags conflict with the OIDC flags; see + [Host AS-1 in mecak8s](/operating/mecak8s/identity-and-client-access.md#host-as-1-in-mecak8s). Existing sessions, schedules, memory, and other owned data keep matching their owners only if, for each user, the authorization server's upstream IdP returns diff --git a/user-docs/operating/mecak8s/identity-and-client-access.md b/user-docs/operating/mecak8s/identity-and-client-access.md index 2bc4b58b4f..711642e815 100644 --- a/user-docs/operating/mecak8s/identity-and-client-access.md +++ b/user-docs/operating/mecak8s/identity-and-client-access.md @@ -181,15 +181,121 @@ oidc: `oidc.ownerIdentity` sets `--oidc-owner-identity` and accepts only `sub_id` or the default empty string. `oidc.acceptedClientIDs` sets -`--oidc-accepted-client-ids`; the chart rejects one without the other. The experimental identity broker requires this setting: -with `--enable-identity-broker` and OIDC enabled, `mecak8s` refuses to start -without it. When `oidc.clientID` is set, the chart requires it in +`--oidc-accepted-client-ids`; the chart rejects one without the other. The +experimental identity broker requires this setting: with +`--enable-identity-broker` and OIDC enabled, `mecak8s` refuses to start without +it, and the chart refuses `identityBroker.enabled` without it. When `oidc.clientID` is set, the chart requires it in `oidc.acceptedClientIDs`. Existing owners and principal settings keep matching only if AS-1's upstream IdP returns each user's same issuer and `sub`, which `mecak8s` cannot check at startup. For that precondition, the token checks, and the requirements on AS-1, see [Own resources by the `sub_id` identity](/features/security-and-execution/caller-identity.md#own-resources-by-the-sub_id-identity). +## Host AS-1 in mecak8s + +`mecak8s` can host AS-1, Mecatl's own sign-in server, in front of your company +IdP. `mecatui` users sign in at AS-1, AS-1 signs them in at the company IdP, +and `mecak8s` validates the access tokens AS-1 issues, keyed by the company +identity in their `sub_id` claim. + +Enabling AS-1 makes it the only issuer `mecak8s` accepts. Access tokens the +company IdP issues directly stop working, so every client must sign in through +AS-1. Studio does not support AS-1 yet and stops working until it does. + +These values turn it on: + +```yaml +replicaCount: 1 +extraArgs: [--no-shell] +oidc: + enabled: true + audience: https://mecatl.example.com/api + resource: https://mecatl.example.com + ownerIdentity: sub_id +identityBroker: + enabled: true + subjectClaim: oid + as1: + issuer: https://mecatl.example.com/as1 + tuiClientID: mecatui + signingKey: + secretName: mecak8s-as1 + key: signing-key.pem + upstream: + issuer: https://login.microsoftonline.com//v2.0 + clientID: + clientSecret: + secretName: mecak8s-as1 + key: upstream-client-secret +``` + +|Value|Flag|Meaning| +|-|-|-| +|`identityBroker.enabled`|`--enable-identity-broker`|Starts the identity broker. Requires `replicaCount: 1`, `oidc.ownerIdentity: sub_id` when OIDC is enabled, and no shell in the process: `--no-shell` in `extraArgs`, or `--shell=` with `execution.enabled` to keep the remote Shell. The chart refuses to render without one. The Deployment rolls out with `Recreate`. This value is the only way to start the broker: the chart refuses `--enable-identity-broker` in `extraArgs`.| +|`identityBroker.subjectClaim`|`--identity-broker-subject-claim`|The company IdP's ID-token claim that names each user. Required with `identityBroker.as1.issuer`, and allowed only with it.| +|`identityBroker.as1.issuer`|`--identity-broker-as1-issuer`|AS-1's HTTPS issuer URL. Setting it hosts AS-1 and requires `oidc.enabled`, `oidc.audience`, and `oidc.resource`.| +|`identityBroker.as1.tuiClientID`|`--identity-broker-as1-tui-client-id`|The client ID `mecatui` uses at AS-1.| +|`identityBroker.as1.signingKey`|`--identity-broker-as1-signing-key-file`|_Optional._ Secret key holding AS-1's PEM signing key (EC P-256 or RSA).| +|`identityBroker.as1.upstream.issuer`|`--identity-broker-as1-upstream-issuer`|The company IdP's HTTPS issuer URL.| +|`identityBroker.as1.upstream.clientID`|`--identity-broker-as1-upstream-client-id`|AS-1's client ID at the company IdP.| +|`identityBroker.as1.upstream.clientSecret`|`--identity-broker-as1-upstream-client-secret-file`|_Optional._ Secret key holding AS-1's client secret at the company IdP. Without it, AS-1 is a public client there.| + +Create the Secret before installing; the chart mounts each key read-only. Leave +`oidc.issuer`, `oidc.clientID`, and `oidc.acceptedClientIDs` empty: the chart +sets them to AS-1's issuer and `tuiClientID`. It refuses values that differ, +any `oidc.jwksURI` (AS-1's discovery document supplies its keys), +`oidc.allowPrivateHTTPSIssuer`, and identity broker flags in `extraArgs`. + +### Choose the issuer + +AS-1 serves its sign-in endpoints and discovery documents under the issuer +URL's path on the `mecak8s` HTTP listener. The issuer can be a dedicated +hostname, such as `https://signin.example.com`, or a path on the `mecak8s` +host, such as `https://mecatl.example.com/as1`. Route that hostname or path +prefix to the HTTP listener so browsers reach it. + +AS-1's sign-in endpoints are anonymous. Behind an ingress, `mecak8s` cannot +tell users apart, so its rate limits are global: 50 requests per second, with +bursts of 200, for the browser sign-in endpoints, and a separate budget of the +same size for `/oauth/token`, so a sign-in flood does not block token +refresh. One client can still use up the budget for everyone, so configure a +per-client rate limit for `/oauth/*` at your ingress. + +AS-1 also holds at most 10,000 sign-ins that have not been approved yet. Past +that, new sign-ins fail with a `server_error` until pending ones complete or +expire, within 15 minutes. + +Keep the issuer once users have signed in. It is in every token, in each user's +saved `mecatui` login, and in the redirect URI registered at the company IdP. + +### Register AS-1 at the company IdP + +Ask the IdP administrator to register one web application for AS-1 with the +redirect URI `/oauth/callback`. This registration replaces the separate +`mecatui` and Studio registrations: every client signs in through AS-1, so +per-application IdP policies can no longer tell those clients apart. AS-1 +requests the `offline_access` scope with `prompt=consent`. + +Set `subjectClaim` to a claim that is the same for a user across applications; +it has no default. For Microsoft Entra ID, use `oid`, because Entra ID issues a +different `sub` to each application. Use `sub` for an IdP whose `sub` does not +depend on the application. The claim names the user to MCP gateways; session +ownership still keys on the access token's `sub_id`. + +### Sign-in and restarts + +Users approve a consent page at AS-1 when they sign in. AS-1 keeps its users, +pending sign-ins, and refresh tokens in memory, so every pod restart or rollout +signs all users out of AS-1 and they run `mecatui login` again. With +`signingKey`, access tokens issued before a restart keep verifying until they +expire. Without it, AS-1 generates a key at startup and those tokens stop +verifying immediately. + +Turning on `oidc.ownerIdentity: sub_id` for a deployment with existing sessions leaves +those sessions with their previous owners; see +[Own resources by the `sub_id` identity](/features/security-and-execution/caller-identity.md#own-resources-by-the-sub_id-identity) +before switching. + ## Security boundaries |Boundary|Operator responsibility| @@ -199,6 +305,7 @@ the requirements on AS-1, see |**Ownerless data**|Caller APIs, retention, and scheduling exclude it. Preserve backups and use the [ownerless-record investigation](#investigate-ownerless-records); do not disable ownership on a tenant-facing service.| |**Signing-key revocation**|The default one-hour JWKS staleness bound permits cached signing keys during a short IdP outage. Tokens remain valid until their own expiry.| |**Rate limits and quotas**|`mecak8s` has no rate-limit flags. Enforce caller limits at the gateway or mesh.| +|**Identity broker and AS-1**|The identity broker and AS-1 run inside the `mecak8s` process and keep their state in its memory, so there is no separate broker listener or store for executor Pods to reach. AS-1's sign-in endpoints are public by design. Keep executor workloads on the `mecatl-execution` default-deny network policy, and do not give a workload profile egress to the `mecak8s` Service.| |**Store confidentiality**|Production Redis requires verified TLS. Client-certificate authentication is not supported.| |**Identity data**|The token's `name` claim, often an email address, is stored unredacted in session snapshots, schedule records, and durable event actor metadata. Configure retention and deletion accordingly.| |**Authentication visibility**|Mecatl emits no authentication metric and does not log authentication failures. The caller sees the 401 or 503 response.| diff --git a/user-docs/reference/server-cli.md b/user-docs/reference/server-cli.md index eb0662ebd5..c7bdf588fe 100644 --- a/user-docs/reference/server-cli.md +++ b/user-docs/reference/server-cli.md @@ -698,6 +698,34 @@ Usage: mecak8s [flags] authentication (default "0.0.0.0:8081") --http-shutdown-timeout value Maximum time for graceful HTTP and metrics shutdown (default 5s) + --identity-broker-as1-issuer string + HTTPS issuer URL of AS-1, the identity broker's sign-in authorization server, + hosted under the URL's path on the HTTP listener. Requires --enable-identity- + broker, --oidc-audience, --oidc-resource, and --oidc-owner-identity=sub_id; + sets --oidc-issuer, --oidc-client-id, and --oidc-accepted-client-ids when + unset, and refuses other values. AS-1 keeps sign-ins and refresh tokens in + memory. Empty does not host AS-1 + --identity-broker-as1-signing-key-file string + PEM private key (EC P-256 or RSA) AS-1 signs access tokens with. Empty + generates a key at startup, so access tokens stop verifying when mecak8s + restarts + --identity-broker-as1-tui-client-id string + OAuth client ID of mecatui at AS-1, a public client. Required with --identity- + broker-as1-issuer + --identity-broker-as1-upstream-client-id string + AS-1's client ID at the upstream identity provider, registered with the + redirect URI ISSUER/oauth/callback. Required with --identity-broker-as1-issuer + --identity-broker-as1-upstream-client-secret-file string + File holding AS-1's client secret at the upstream identity provider. Empty + makes AS-1 a public client there + --identity-broker-as1-upstream-issuer string + HTTPS issuer URL of the OIDC identity provider users sign in at through AS-1. + Required with --identity-broker-as1-issuer + --identity-broker-subject-claim string + Upstream identity provider ID-token claim that names the company identity, the + same for a user in every application: oid for Entra ID, whose sub differs per + application, or sub for a provider whose sub does not. Required with, and only + allowed with, --identity-broker-as1-issuer --import-claude-permissions Import compatible Claude Code permission settings --learning-store-url string