feat(auth): publish authenticated principal metadata - #1011
Conversation
3d26ded to
4ca6d53
Compare
Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
4ca6d53 to
c2fe248
Compare
praxis-bot
left a comment
There was a problem hiding this comment.
PR Review
Purpose: Publishes identity.user_id metadata after successful Basic Auth verification, enabling downstream filters to consume a trusted authenticated principal without reparsing credentials.
Assessment: Well-structured change with clear security intent. The fail-closed behavior on metadata capacity exhaustion and the bounded username length are sound design choices. Three improvements needed.
| Severity | Count | Summary |
|---|---|---|
| Medium | 3 | Missing Error trait on public type, non-idiomatic result discard, test coverage gap for KV-backed username rejection |
| const MAX_METADATA_ENTRIES: usize = 128; | ||
|
|
||
| /// Failure returned when a request metadata value cannot be stored. | ||
| #[derive(Clone, Copy, Debug, Eq, PartialEq)] |
There was a problem hiding this comment.
[Medium] MetadataError is publicly exported and used as the error position in Result<(), MetadataError>, but it doesn't implement Display or std::error::Error. Project conventions require thiserror for error types. Without these traits, callers can't use ? ergonomically or compose with standard error-handling crates.
#[derive(Clone, Copy, Debug, Eq, PartialEq, thiserror::Error)]
pub enum MetadataError {
#[error("metadata key is empty or exceeds the 64-byte limit")]
InvalidKey,
#[error("metadata value exceeds the 256-byte limit")]
ValueTooLong,
#[error("request metadata entry limit reached")]
Capacity,
}| /// memory growth. | ||
| pub fn set_metadata(&mut self, key: impl Into<String>, value: impl Into<String>) { | ||
| match self.try_set_metadata(key, value) { | ||
| Ok(()) | Err(_) => {}, |
There was a problem hiding this comment.
[Medium] Non-idiomatic way to discard a Result. The match with Ok(()) | Err(_) => {} is functionally correct but unusual -- both arms do the same thing, so the exhaustive matching doesn't convey intent better than the standard idiom:
let _ = self.try_set_metadata(key, value);| // The authenticated principal is trusted only after verification. | ||
| // Keep it bounded because filter metadata is request-scoped and can be | ||
| // consumed by downstream admission and routing filters. | ||
| if username.len() > 256 { |
There was a problem hiding this comment.
[Medium] This runtime 256-byte username check exists specifically for the KV-backed credential path (inline usernames are caught at config time by the TryFrom validation), but no test exercises a KV-backed credential with a >256-byte username being rejected at request time. The path is untested.
Add a test that:
- Creates a filter with
kv_storesource - Stores a credential with a 257-byte username key in the KV store
- Sends a request with matching 257-byte base64-encoded credentials
- Asserts the request is rejected with 401 and no
identity.user_idmetadata is published
Summary
This PR adds the common authenticated-principal handoff needed by downstream Praxis filters.
After Basic Auth credentials are successfully verified, the filter publishes the request-scoped metadata key:
The value contains the verified username. Downstream filters can use it for quota enforcement, authorization, auditing, and other identity-aware behavior without reparsing credentials.
Identity is published only after successful verification. Missing, malformed, unknown, or incorrect credentials publish no identity.
Metadata publication is now fallible. If the request metadata capacity is exhausted, authentication fails closed with a 401 response instead of returning Continue without the promised identity.
Usernames are bounded to 256 bytes. Inline credentials are rejected during configuration parsing when they exceed that limit; KV-backed identities retain runtime protection.
Why this belongs in Praxis core
Authentication and identity establishment are core request-pipeline responsibilities.
Downstream filters should consume a trusted identity established by the authentication layer rather than:
The reusable contract is:
Basic Auth is the first producer of this metadata. Future authentication mechanisms such as OIDC, API keys, mTLS, or external authentication can publish the same key after successful verification.
This is required by the Grid-aware token-rate-limit work, where Praxis AI must establish the authenticated principal before reserving quota.
Related work:
The AI PR is the provider-selection/load-balancing foundation. The distributed quota behavior is demonstrated separately by the experimental demo.
Implementation details
IDENTITY_USER_ID_METADATAconstant.No passwords, Authorization headers, credential-store values, or other credential material are placed in request metadata.
Performance considerations
This adds no network calls, filesystem access, Kubernetes access, or remote lookup to the request path.
The successful-authentication path performs:
The metadata operation is bounded and does not introduce a new lock, cache, background task, or external dependency.
When metadata capacity is exhausted, the request fails closed immediately. This prevents downstream filters from running without the identity they require.
No benchmark is included because this change does not add a new network or synchronization boundary. Performance benchmarking remains part of the broader AI token-rate-limit qualification.
Security considerations
The identity metadata is trusted only because it is written after credential verification.
Clients must not be allowed to set or override identity.user_id directly.
The metadata contains only the verified username. It does not contain:
Production telemetry should not use raw identity values as unbounded metric labels.
Validation
cargo test -p praxis-proxy-filter --features basic-auth-filter basic_auth --locked
cargo test -p praxis-proxy-filter context --locked
git diff --check
Coverage includes:
Breaking changes
None expected for existing valid configurations.
The new 256-byte username limit is an explicit validation constraint required to keep request metadata bounded. Inline configurations exceeding this limit now fail at startup. KV- backed usernames exceeding the limit are rejected at request time.
Downstream filters that consume identity.user_id should be placed after the authentication filter.