audit,portal: render bodies as utf-8 strings; propagate identity through Try-It and replay#9
Merged
Merged
Conversation
…ugh Try-It and replay dispatch Two defects surfaced in the live portal: 1. audit_payloads.request_body/response_body are []byte, which Go's default JSON encoder emits as base64. The audit detail panel showed raw base64 instead of the captured request/response JSON. Add a custom MarshalJSON on Payload that emits the body as a utf-8 string when valid; falls back to base64 with a sibling _encoding=base64 flag for non-utf-8 bytes so binary payloads still round-trip. 2. Try-It (and audit replay) dispatch through the local mux via replayTarget.ServeHTTP. The dispatched request carries no Authorization/X-API-Key on the wire (Try-It deliberately strips operator-supplied Authorization/Cookie), so the inbound auth middleware would 401 a request the portal had already accepted. Fix by translating the portal-resolved auth.Identity into an inbound.Identity on the dispatched ctx, and short-circuiting httpmw.Identity when one is already set — except when a real credential is on the wire, so "type X-API-Key into Try-It to test as a different principal" still resolves through the chain. Replay also has to drop credential headers (Authorization, X-API-Key, Cookie) and the api_key query param before re-emission: those values are persisted as the "[redacted]" sentinel and would otherwise look like wire credentials, defeating the bypass and 401'ing through chain validation of the redaction string. Tests: - Payload MarshalJSON: utf-8, binary, empty cases. - httpmw.Identity: pre-set bypass; wire-cred override of pre-set. - portal_audit_replay: end-to-end through real httpmw.Identity with a chain that rejects "[redacted]" — would 401 if the redacted-header filter regressed. Known limitation (deferred): - hasInboundCredential and redactedReplayHeaders hardcode the default api-key header/query names (X-API-Key, api_key). Deployments that customize APIKeysConfig.HeaderName / QueryParamName will silently lose Try-It "test as someone else" semantics under the custom name, and replay will leak the captured redaction sentinel for the custom header. The live api-test-server.plexara.io deployment runs the defaults so this is not a regression for the immediate rollout; TODOs left in code at the two call sites. make verify clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Payload.MarshalJSONemits utf-8 strings (with arequest_body_encoding/response_body_encoding=base64flag fallback for non-utf-8 bytes so binary payloads still round-trip).httpmw.Identityshort-circuits when one is already set, except when a real credential is on the wire — so typingX-API-Key: <other-key>into the Try-It headers field to "test as a different principal" still resolves through the chain.Authorization,X-API-Key,Cookie) and theapi_keyquery param before re-emission, since those values are persisted as the[redacted]sentinel and would otherwise look like wire credentials.Why the user ran into this
Both defects show up immediately on the live
api-test-server.plexara.iodeployment of v1.1.0 (audit panel showed base64 strings, Try-It returned 401 while the user was logged in via the portal session). The deployment manifest itself is correct; the defects are upstream in v1.1.0. Cutting a v1.1.1 tag here unblocks the cluster.Known limitation (deferred with TODO)
hasInboundCredentialandredactedReplayHeadershardcode the default api-key header/query names (X-API-Key,api_key). Deployments that customizeAPIKeysConfig.HeaderName/QueryParamNamewill silently lose Try-It "test as someone else" semantics under the custom name, and replay will leak the captured redaction sentinel for the custom header. The live deployment runs the defaults so this isn't a regression for the immediate rollout. Follow-up would either thread the configured names throughIdentity()or add aHasCredential(r)method toinbound.Chainthat delegates to each authenticator.Test plan
make verifyclean (fmt, vet, test, golangci-lint, gosec, semgrep, coverage gate, SPA-bundle in-sync check)Payload.MarshalJSON(utf-8 / binary / empty);httpmw.Identitypre-set bypass + wire-cred overrideTestAuditReplay_RedactedCredentialsThroughRealIdentityMWmounts the realhttpmw.Identityin front of the replay target with a chain that would reject[redacted], asserts replay reaches the inner handler — would 401 if the redacted-header filter regressedmake dev: audit detail shows decoded JSON, Try-It on/v1/whoamireturns 200 with{"subject":"devkey","auth_type":"apikey","key_name":"devkey"}