diff --git a/docs/approval-engine-auth-contract.md b/docs/approval-engine-auth-contract.md index ead4d1c..a78b962 100644 --- a/docs/approval-engine-auth-contract.md +++ b/docs/approval-engine-auth-contract.md @@ -19,13 +19,23 @@ exact deployment-owned callback, `audience: approval-engine`, consume or other approval grants to that client. No callback is invented here. The ID token is for the login client; present the access token to approval-engine. -These fragments are not live registrations. Deployment requires custody-managed -values for the named environment references, the exact human callback, and a -rollout of this version. Never log the token or secret. Verify the resulting +These fragments are not live registrations. The two service registrations require +custody-managed values for the named environment references and a reviewed +rollout of this version, including the upstream issuer precondition. Platform's +CCR-2026-0017/0018 use OpenBao field `CLIENT_SECRET`; their approval remains open. +The separate human registration needs its actual UI-owned callback. A bearer-only +approval resource server has no such callback; its absence does not prevent +service-client issuance or service startup, and service credentials cannot be +counted as human approval evidence. Never log the token or secret. Verify the resulting access token against the deployed issuer's `/jwks`, checking issuer, audience, expiry, subject, principal type, tenant, roles, scope and assurance. Verify that operator consume and human consume requests are rejected. Local tests verify signatures against the JWKS handler; they do not constitute live issuance proof. +Negative verification requires the token endpoint's typed refusal: HTTP 400, +`invalid_profile_usage`, feature `scope` for excess scope; HTTP 401 with feature +`Authorization` for a predecessor secret. A timeout, 5xx, malformed response, +invalid signature or JWKS failure is not proof of denial. + KeyCape owns issuance and client grants/disablement. OpenBao and the deployment operator own credential custody; approval-engine enforces its resource policy. diff --git a/docs/approval-engine-provisioning-request.yaml b/docs/approval-engine-provisioning-request.yaml index 8337f76..e5b58ff 100644 --- a/docs/approval-engine-provisioning-request.yaml +++ b/docs/approval-engine-provisioning-request.yaml @@ -15,7 +15,7 @@ requests: scopes: [approval:read, approval:consume] lifetime: 15m proposed_openbao_path: platform/workloads/secrets-engine/approval-client - field: client_secret + field: CLIENT_SECRET proposed_kubernetes_secret: sso/keycape-secrets-engine-approval-client kubernetes_key: client-secret keycape_environment: KEYCAPE_SECRETS_ENGINE_APPROVAL_CLIENT_SECRET @@ -27,7 +27,7 @@ requests: scopes: [approval:create, approval:read, approval:approve, approval:revoke, approval:supersede, approval:observe, approval:emit] lifetime: 15m proposed_openbao_path: platform/workloads/approval-engine/operator-client - field: client_secret + field: CLIENT_SECRET proposed_kubernetes_secret: sso/keycape-approval-engine-operator-client kubernetes_key: client-secret keycape_environment: KEYCAPE_APPROVAL_ENGINE_OPERATOR_CLIENT_SECRET @@ -35,6 +35,8 @@ requests: consumer: approval-engine-operator human_registration: status: awaiting-exact-callback + blocks_service_client_rollout: false + owner: unassigned-approver-ui scopes: [openid, approval:approve] mfa_required: true client_type: public @@ -65,4 +67,13 @@ verification: blockers: - Admit exact custody paths, field delivery, consumer identities and lifecycle authority. - Resolve attended first-provision authority through the custody owner. - - Supply exact human client ID and callback URI. + - Verify the actual upstream ID-token issuer before production image rollout. +custody_return: + owner_record: railiance-platform/workplans/RPF-WP-0035-credential-lane-implementation.md#Admit-KeyCape-approval-engine-client-custody-and-delivery + requests: [CCR-2026-0017, CCR-2026-0018] + status: proposed-awaiting-owner-approval + field_correction: CLIENT_SECRET + scope: KeyCape verifier-side copies only; client-side retrieval is not admitted. +human_registration_gate: + - The approver UI owner must supply its real client ID and exact callback. + - This gate blocks human approval entry; it does not block the two independent client_credentials registrations or service startup. diff --git a/src/internal/authclient/client.go b/src/internal/authclient/client.go index 17ebefa..0d39586 100644 --- a/src/internal/authclient/client.go +++ b/src/internal/authclient/client.go @@ -43,6 +43,19 @@ func New(issuer string) (*Client, error) { return &Client{Issuer: issuer, HTTP: &http.Client{Timeout: 30 * time.Second, CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }}}, nil } +// providerRejection preserves only the typed refusal needed by verification. +// Error() never includes response text, credentials, endpoint or claim values. +type providerRejection struct { + status int + endpoint string + code string + feature string +} + +func (e *providerRejection) Error() string { + return fmt.Sprintf("provider rejected request (HTTP %d)", e.status) +} + func (c *Client) request(ctx context.Context, method, endpoint string, form url.Values, id, secret string, out any) error { req, err := http.NewRequestWithContext(ctx, method, endpoint, strings.NewReader(form.Encode())) if err != nil { @@ -60,7 +73,18 @@ func (c *Client) request(ctx context.Context, method, endpoint string, form url. } defer response.Body.Close() if response.StatusCode != http.StatusOK { - return fmt.Errorf("provider rejected request (HTTP %d)", response.StatusCode) + e := &providerRejection{status: response.StatusCode, endpoint: endpoint} + body, readErr := io.ReadAll(io.LimitReader(response.Body, 4097)) + if readErr == nil && len(body) <= 4096 { + var refusal struct { + Code string `json:"error"` + Feature string `json:"feature"` + } + if json.Unmarshal(body, &refusal) == nil { + e.code, e.feature = refusal.Code, refusal.Feature + } + } + return e } body, err := io.ReadAll(io.LimitReader(response.Body, 1024*1024+1)) if err != nil || len(body) > 1024*1024 { diff --git a/src/internal/authclient/verify.go b/src/internal/authclient/verify.go index d912efd..66b0ea4 100644 --- a/src/internal/authclient/verify.go +++ b/src/internal/authclient/verify.go @@ -6,6 +6,7 @@ import ( "flag" "fmt" "io" + "net/http" "net/url" "os" "sort" @@ -124,7 +125,7 @@ func runVerify(ctx context.Context, c *Client, o verifyOptions, stderr io.Writer for _, s := range strings.Fields(o.DenyScope) { excess := url.Values{"grant_type": {"client_credentials"}, "scope": {s}} _, err := c.Exchange(ctx, d, excess, o.ClientID, o.Secret, o.Audience, "") - report.record("excess scope refused: "+s, mustFail(err)) + report.record("excess scope refused: "+s, mustReject(err, d.Token, http.StatusBadRequest, "scope")) } // Predecessor rejection, for a rotation. Only meaningful after step 3 of the @@ -136,7 +137,7 @@ func runVerify(ctx context.Context, c *Client, o verifyOptions, stderr io.Writer report.record("predecessor secret refused", errors.New("predecessor and current secret are identical; no rotation occurred")) } else { _, err := c.Exchange(ctx, d, form, o.ClientID, o.Previous, o.Audience, "") - report.record("predecessor secret refused", mustFail(err)) + report.record("predecessor secret refused", mustReject(err, d.Token, http.StatusUnauthorized, "Authorization")) } } return report.result() @@ -223,9 +224,14 @@ func noExcessScope(claims map[string]any, requested string) error { return nil } -func mustFail(err error) error { +func mustReject(err error, endpoint string, status int, feature string) error { if err == nil { return errors.New("the issuer accepted a request it must have refused") } - return nil + var rejection *providerRejection + if errors.As(err, &rejection) && rejection.endpoint == endpoint && + rejection.status == status && rejection.code == "invalid_profile_usage" && rejection.feature == feature { + return nil + } + return errors.New("the expected token-endpoint refusal was not proved; transport, server, or token-validation failures are not denial evidence") } diff --git a/src/internal/authclient/verify_test.go b/src/internal/authclient/verify_test.go index b7eda28..a2a824f 100644 --- a/src/internal/authclient/verify_test.go +++ b/src/internal/authclient/verify_test.go @@ -3,6 +3,10 @@ package authclient import ( "bytes" "context" + "errors" + "io" + "net/http" + "net/url" "strings" "testing" @@ -160,3 +164,62 @@ func TestHasAllRolesNamesWhatIsMissing(t *testing.T) { } var _ = domain.Client{} + +type verifyTransport func(*http.Request) (*http.Response, error) + +func (f verifyTransport) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +func TestVerifyClientDoesNotConfuseFailuresWithRefusals(t *testing.T) { + for _, target := range []string{"scope", "predecessor"} { + for _, failure := range []string{"transport", "server", "invalid_token", "unrelated_refusal", "malformed_refusal"} { + t.Run(target+"/"+failure, func(t *testing.T) { + c, _, _ := provider(t) + original := c.HTTP.Transport + o := verifyOpts() + if target == "predecessor" { + o.PreviousNamed, o.Previous = true, "old-secret" + } + c.HTTP.Transport = verifyTransport(func(r *http.Request) (*http.Response, error) { + if r.Method == http.MethodPost && r.URL.Path == "/token" { + body, _ := io.ReadAll(r.Body) + r.Body = io.NopCloser(bytes.NewReader(body)) + form, _ := url.ParseQuery(string(body)) + _, password, _ := r.BasicAuth() + password, _ = url.QueryUnescape(password) + negative := target == "scope" && form.Get("scope") != o.Scope || target == "predecessor" && password == o.Previous + if negative { + status, payload := 503, `{"error":"unavailable"}` + switch failure { + case "transport": + return nil, errors.New("transport failed") + case "invalid_token": + status, payload = 200, `{"token_type":"Bearer","access_token":"invalid","expires_in":900}` + case "unrelated_refusal": + status, payload = 400, `{"error":"invalid_profile_usage","feature":"client_id","description":"must not disclose this"}` + case "malformed_refusal": + status, payload = 400, `not-json-sensitive-body` + } + return &http.Response{StatusCode: status, Header: http.Header{}, Body: io.NopCloser(strings.NewReader(payload)), Request: r}, nil + } + } + return original.RoundTrip(r) + }) + var out bytes.Buffer + if err := runVerify(context.Background(), c, o, &out); err == nil { + t.Fatal("a failed negative check was accepted as issuer refusal") + } + if !strings.Contains(out.String(), "PASS exchange and JWKS signature for granted scopes") { + t.Fatalf("test did not reach the negative checks: %s", out.String()) + } + if !strings.Contains(out.String(), "expected token-endpoint refusal was not proved") { + t.Fatalf("wrong failure: %s", out.String()) + } + for _, value := range []string{o.Secret, o.Previous, "must not disclose this", "not-json-sensitive-body", "eyJ"} { + if value != "" && strings.Contains(out.String(), value) { + t.Fatal("verification disclosed input or provider payload") + } + } + }) + } + } +} diff --git a/workplans/KEY-WP-0013-approval-engine-resource-audience.md b/workplans/KEY-WP-0013-approval-engine-resource-audience.md index fc11d83..8b267df 100644 --- a/workplans/KEY-WP-0013-approval-engine-resource-audience.md +++ b/workplans/KEY-WP-0013-approval-engine-resource-audience.md @@ -41,10 +41,12 @@ priority: high state_hub_task_id: "607897c5-bad9-55e5-86df-7802f592d6e8" ``` -Needs deployment-owned custody for both new secret references and an exact human -callback registration. Deploy the implementation and registrations together, +Needs deployment-owned custody for both new service secret references and the +upstream issuer precondition. Deploy the implementation and service registrations together, then prove live JWKS verification and denied excess scopes without logging values. Local signature proof is not live rollout evidence. See docs/approval-engine-auth-contract.md. +The separate human UI callback gate is retained in T05; a bearer-only resource +server does not own a redirect and its absence does not block these service clients. 2026-09-05 follow-up: read-only deployment metadata shows the current image is @@ -168,3 +170,42 @@ Evidence: `TestApprovalClientIssuesExactPlatformTenantAndRejectsAliases` (exact client's tenant against the real fixture so a reintroduced alias fails the build. Full `go test ./...` and `go vet ./...` pass. Choice only — live provisioning and the other admission gates remain with KEY-WP-0013-T02. + +## Assign and register the human approver browser client + +```task +id: KEY-WP-0013-T05 +status: wait +priority: high +assignee: the-custodian +blocking_reason: "An actual approver UI owner, client ID and deployed callback are not yet supplied. approval-engine is a bearer-only resource server." +``` + +Own the residual human registration separately from service-client T02. Reuse +an existing accepted UI if available; do not invent a callback on approval-engine. +Require public S256 PKCE, exact redirect, MFA, approval:approve without consume, +and a real human access-token proof. No service credential substitutes for this. +HFACT-WP-0001-T03 consumes the acceptance where a human approval is required. + +## Make negative rollout evidence discriminate actual issuer refusal + +```task +id: KEY-WP-0013-T06 +status: progress +priority: high +assignee: the-custodian +``` + +Critical-path review found `mustFail` accepted any Exchange error: timeout, +HTTP 5xx, invalid token or JWKS failure could falsely prove excess-scope denial +or predecessor rejection after a successful positive exchange. Preserve a bounded +typed provider refusal, require the exact token endpoint/status/error/feature, +and prove the checker rejects unrelated failures without exposing provider bodies. +Publish the verified image candidate and return its immutable digest to T02. + +Platform's 2026-09-08 return is RPF-WP-0035-T05 and proposed CCR-2026-0017/0018. +The custody field is `CLIENT_SECRET`; Kubernetes key `client-secret` and both env +names remain as requested. These records cover verifier-side delivery only. +Owner approval, client-side retrieval and the actual upstream ID-token issuer +proof remain distinct gates. Public discovery currently advertises +`https://auth.coulomb.social`; that alone is not the signed-token observation.