Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a Vault-compatible authentication surface (AppRole + Kubernetes) and token self-service endpoints to support SPIRE integrations, backed by new database tables and shared client/server DTOs.
Changes:
- Added Vault-compatible HTTP endpoints for AppRole login + admin CRUD, Kubernetes login + admin CRUD, and token self-service (lookup/renew/revoke).
- Implemented
X-Vault-Tokenextraction middleware and added DB trait + backend implementations to persist/lookup/renew/revoke Vault tokens and roles. - Extended the shared
auth_clientDTO layer and updated the OpenAPI specification and changelog for the new API.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/server/endpoints/vault_token.rs | Adds Vault token self-service endpoints (lookup/renew/revoke). |
| server/src/server/endpoints/vault_k8s.rs | Adds Kubernetes JWT login + role CRUD endpoints and JWKS/JWT helpers. |
| server/src/server/endpoints/vault_approle.rs | Adds AppRole login + role/secret-id CRUD and shared token issuance helper. |
| server/src/server/endpoints/mod.rs | Exposes new Vault endpoint modules and handlers. |
| server/src/server/auth_verifier.rs | Registers Vault scopes and applies the new middleware stacks. |
| server/src/middleware/vault_token_extract.rs | Implements X-Vault-Token auth middleware and claims injection. |
| server/src/middleware/mod.rs | Re-exports the new Vault token middleware/types. |
| server/src/database/trait.rs | Adds Vault-related DB structs + trait methods. |
| server/src/database/mod.rs | Re-exports Vault DB types from the trait module. |
| server/src/database/impls/sqlite.rs | Adds Vault tables and implements Vault DB operations (SQLite). |
| server/src/database/impls/postgres.rs | Adds Vault tables and implements Vault DB operations (PostgreSQL). |
| server/src/database/impls/mysql.rs | Adds Vault tables and implements Vault DB operations (MySQL). |
| server/documentation/openapi.yaml | Documents new Vault endpoints and wire types in the OpenAPI contract. |
| server/Cargo.toml | Adds dependencies needed for token issuance / IDs (rand, uuid). |
| client/src/lib.rs | Re-exports new Vault DTOs from the client crate. |
| client/src/dto/vault.rs | Adds Vault-compatible wire DTOs (AppRole/K8s/token responses). |
| client/src/dto/mod.rs | Exposes the new vault DTO module. |
| CHANGELOG/spire-vault-compatible-api.md | Adds release notes for the new Vault-compatible API. |
| Cargo.toml | Adds workspace dependencies (rand, uuid). |
| Cargo.lock | Locks new transitive dependencies. |
Comments suppressed due to low confidence (1)
server/src/server/endpoints/vault_k8s.rs:80
- This bound-namespace check allocates a new
Stringfor"*"on every request. It can be allocation-free by checking withiter().any(...)instead.
if !bound_namespaces.contains(&"*".to_string()) && !bound_namespaces.contains(&ns) {
return Err(AuthError::Forbidden(format!(
"namespace '{ns}' not in bound_service_account_namespaces"
)));
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (3)
server/documentation/openapi.yaml:1769
/v1/auth/token/renew-selfreturns 403 on missing/invalid tokens (via VaultTokenExtract), but the OpenAPI spec documents a 401 response.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1783
/v1/auth/token/revoke-selfreturns 403 on missing/invalid tokens (via VaultTokenExtract), but the OpenAPI spec documents a 401 response.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:644
- The server implementation checks for exact namespace matches (or
"*"), but the OpenAPI description claims glob-pattern matching. The docs should match the actual behavior.
description: Allowed namespaces (glob patterns; `["*"]` for any).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (15)
server/src/server/endpoints/vault_k8s.rs:182
validate_k8s_jwtuses the JWT header'salgdirectly (Validation::new(alg)), which can allow algorithm-confusion cases (e.g. attacker suppliesHS256and the decoder treats the JWKS public key material as an HMAC secret). For Kubernetes SA tokens we should only accept asymmetric algorithms we expect (RS256/ES256) and reject everything else before attempting verification.
let header = decode_header(token).map_err(|e| format!("invalid JWT header: {e}"))?;
let alg = header.alg;
server/documentation/openapi.yaml:631
K8sRoleRequest.jwks_urlis documented with an/.well-known/openid-configurationexample, but the server expects a JWKS document with a top-levelkeysarray (seefetch_jwksparsing intoJwksResponse { keys: ... }). The OpenAPI example should point to the JWKS endpoint (e.g..../jwks.json) to avoid client misconfiguration.
jwks_url:
type: string
format: uri
description: URL of the Kubernetes API server JWKS endpoint.
example: https://kubernetes.default.svc/.well-known/openid-configuration
server/documentation/openapi.yaml:638
- The OpenAPI description says
bound_service_account_namessupports glob patterns, but the implementation only does exact string matching (plus the special"*"wildcard). The schema text should match the actual semantics to prevent operators from assuming patterns likespire-*will work.
default: ['*']
description: Allowed service account names (glob patterns; `["*"]` for any).
example: [spire-agent]
client/src/dto/vault.rs:113
- The doc comment says
bound_service_account_namespacessupports glob patterns, but the server-side check is exact-match only (with"*"as the only wildcard). Align this comment with the server implementation to avoid documenting unsupported behavior.
/// Allowed namespaces (glob patterns, `["*"]` for any).
#[serde(default)]
pub bound_service_account_namespaces: Vec<String>,
server/src/database/impls/sqlite.rs:161
vault_approle_roles.token_policiesis stored/parsed as a comma-separated string, but the table default is'[]'. If any row is ever inserted without explicitly settingtoken_policies, the runtime split-on-comma logic will treat"[]"as a real policy name. The default should be the empty string to match the storage format.
token_ttl_secs INTEGER NOT NULL DEFAULT 3600,
bind_secret_id INTEGER NOT NULL DEFAULT 1,
token_policies TEXT NOT NULL DEFAULT '[]'
)
server/src/database/impls/postgres.rs:142
vault_approle_roles.token_policiesis stored/parsed as a comma-separated string, but the table default is'[]'. If any row is inserted without explicitly settingtoken_policies, splitting will yield a bogus policy name"[]". Use an empty-string default to match the actual storage format.
secret_id_ttl_secs BIGINT NOT NULL DEFAULT 0,
token_ttl_secs BIGINT NOT NULL DEFAULT 3600,
bind_secret_id BOOLEAN NOT NULL DEFAULT TRUE,
token_policies TEXT NOT NULL DEFAULT '[]'
)
server/documentation/openapi.yaml:1752
- The token self-service endpoints return HTTP 403 on missing/invalid tokens (see
VaultTokenExtractmiddleware’sHttpResponse::Forbidden()), but the OpenAPI spec lists only a 401 response. Document 403 Forbidden here to match actual behavior and Vault client expectations.
ttl: 3540
creation_time: 1722070800
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1769
/v1/auth/token/renew-selfreturns HTTP 403 on missing/invalid tokens (middleware) and on non-renewable tokens (AuthError::Forbidden). The OpenAPI spec currently lists only 401; update it to include 403 Forbidden.
$ref: '#/components/schemas/VaultAuthResponse'
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1783
/v1/auth/token/revoke-selfreturns HTTP 403 on missing/invalid tokens viaVaultTokenExtract, but the OpenAPI spec lists only 401. Update the spec to include 403 Forbidden to match actual runtime behavior.
'204':
description: Token revoked.
'401':
$ref: '#/components/responses/Unauthorized'
server/src/server/endpoints/vault_token.rs:32
- When the token disappears/expires between the middleware lookup and this handler’s re-read, the code returns
AuthError::Session("token not found"), which maps to HTTP 401. Vault clients expect invalid/expired tokens to yield HTTP 403 ("permission denied"), and your middleware already returns 403 for missing/invalid tokens.
// Re-read from DB to get the freshest TTL
let token = database
.vault_lookup_token(&claims.token_hash)
.await?
.ok_or_else(|| AuthError::Session("token not found".to_string()))?;
server/documentation/openapi.yaml:645
- The OpenAPI description says
bound_service_account_namespacessupports glob patterns, but the implementation only does exact string matching (plus the special"*"wildcard). Update the schema description to avoid documenting unsupported matching behavior.
default: ['*']
description: Allowed namespaces (glob patterns; `["*"]` for any).
example: [spire]
client/src/dto/vault.rs:110
- The doc comment says
bound_service_account_namessupports glob patterns, but the server-side check is exact-match only (with"*"as the only wildcard). This comment should match the actual API behavior to avoid misleading downstream users ofauth_client.
This issue also appears on line 111 of the same file.
/// Allowed service-account names (glob patterns, `["*"]` for any).
#[serde(default)]
pub bound_service_account_names: Vec<String>,
server/documentation/adr/2026-07-26-vault-compatible-auth-api-for-spire.md:95
- The ADR schema snippet describes
vault_approle_roles.token_policiesas a JSON array, but the implementation stores policies as a comma-separated string (payload.token_policies.join(",")) and parses them via.split(','). The ADR should reflect the actual chosen storage format.
token_ttl_secs INTEGER NOT NULL,
bind_secret_id BOOLEAN NOT NULL,
token_policies TEXT NOT NULL -- JSON array
);
server/documentation/vault_auth_api.md:253
- This step says the server selects the JWKS key by
kid, but the current implementation iterates over all JWKS keys until one verifies (it doesn’t usekid). The doc should match the real behavior to avoid over-promising key-selection logic.
2. Selects the correct key by `kid` from the JWT header.
server/src/server/auth_verifier.rs:380
- This PR adds a new Vault-compatible
/v1/auth/*surface area, but there are no integration tests covering these routes (novault_references inserver/src/tests). Please add tests for AppRole login + secret-id consumption, K8s login (with a local JWKS server), and token lookup/renew/revoke, including expected 403/204 behaviors.
// ── Vault-compatible auth scopes ─────────────────────────────────────────
//
// Scope prefixes are kept intentionally specific to avoid Actix-web's
// FIFO matching swallowing requests before they reach the right scope.
//
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 33 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (10)
server/src/server/endpoints/vault_k8s.rs:181
validate_k8s_jwtuses the untrusted JWT header algorithm (Validation::new(header.alg)). If a token declares a symmetric alg (e.g. HS256) ornone, the JWK-derived public key bytes can be misused as an HMAC secret / bypass signature checks (alg confusion). Restrict accepted algorithms to the expected asymmetric ones (RS256/ES256) before decoding.
server/src/server/endpoints/vault_token.rs:32- If the token was revoked/expired between the middleware lookup and this handler, this currently returns
AuthError::Session(401). For Vault-token authentication failures, return 403 consistently (matchingVaultTokenExtractand the docs).
server/src/server/endpoints/vault_token.rs:83 - If the token is revoked/expired immediately after renewal, this currently returns
AuthError::Generic(HTTP 500). That’s a client-visible auth failure and should be a 403 (consistent withVaultTokenExtract).
server/src/server/endpoints/vault_token.rs:115 - Same as
lookup-self: missing vault token claims currently maps toAuthError::Session(401). For Vault token auth failures, return 403 consistently withVaultTokenExtract.
server/documentation/openapi.yaml:1752 - These Vault token self-service endpoints return
403 Forbiddenfor missing/invalid tokens (seeVaultTokenExtractandvault_auth_api.md), but the OpenAPI spec documents a401response here. Update the spec so clients don’t bake in the wrong error handling.
creation_time: 1722070800
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1769
- Same as
lookup-self: implementation returns 403 on auth failure, but OpenAPI documents 401 here. Align the contract with the actual status code.
schema:
$ref: '#/components/schemas/VaultAuthResponse'
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1783
- Same as the other Vault token endpoints: OpenAPI documents 401, but the middleware and server docs describe 403 for missing/invalid tokens. Align the spec.
'204':
description: Token revoked.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:631
jwks_urlis described/used as a JWKS endpoint, but the example points at/.well-known/openid-configuration(which is not a JWKS document). This example should use a JWKS URL (e.g..../jwks.json) to avoid confusing integrators.
type: string
format: uri
description: URL of the Kubernetes API server JWKS endpoint.
example: https://kubernetes.default.svc/.well-known/openid-configuration
server/src/server/endpoints/vault_token.rs:27
AuthError::Sessionmaps to HTTP 401 in this codebase, but Vault token self-service errors for missing/invalidX-Vault-Tokenare expected to be 403 (and theVaultTokenExtractmiddleware already returns 403). UseAuthError::Forbiddenhere so the handler can’t accidentally return 401 if middleware ordering changes.
This issue also appears in the following locations of the same file:
- line 29
- line 79
- line 112
server/src/server/endpoints/vault_approle.rs:166
- OpenAPI marks the
/v1/auth/approle/role/{name}/secret-idrequest body as optional (required: false), but the handler usesJson<AppRoleSecretIdRequest>, which rejects missing bodies with a 400. Accept an optional JSON body and defaultttl/num_usesto 0 when absent to match the documented contract.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 33 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (16)
server/src/database/impls/sqlite.rs:161
vault_approle_roles.token_policiesis stored/parsed as a comma-separated string (seevault_approle_create_rolejoining with,), but the SQLite schema default is'[]'. If a row is ever created without an explicit value (or migrated from an earlier schema), this will parse as a literal "[]" policy name.
role_id TEXT NOT NULL UNIQUE,
secret_id_ttl_secs INTEGER NOT NULL DEFAULT 0,
token_ttl_secs INTEGER NOT NULL DEFAULT 3600,
bind_secret_id INTEGER NOT NULL DEFAULT 1,
token_policies TEXT NOT NULL DEFAULT '[]'
)
server/src/database/impls/postgres.rs:142
vault_approle_roles.token_policiesis treated as comma-separated throughout the code, but the PostgreSQL schema default is'[]', which would later parse as a literal policy name[]. The default should be an empty string to match the actual storage format.
role_id TEXT NOT NULL UNIQUE,
secret_id_ttl_secs BIGINT NOT NULL DEFAULT 0,
token_ttl_secs BIGINT NOT NULL DEFAULT 3600,
bind_secret_id BOOLEAN NOT NULL DEFAULT TRUE,
token_policies TEXT NOT NULL DEFAULT '[]'
)
server/src/server/endpoints/vault_k8s.rs:75
- Same issue as
bound_sa_names:unwrap_or_default()hides malformed JSON inbound_sa_namespacesand turns it into an empty allow-list. This should surface as a server-side configuration error.
server/src/server/endpoints/vault_k8s.rs:224 - Same as above: clearing
required_spec_claimsin the RS256 fallback disables requiringexpand may accept non-expiring JWTs. Keepexprequired.
server/documentation/openapi.yaml:1752 - The Vault token self-service endpoints return
403 Forbiddenon missing/invalidX-Vault-Token(seeVaultTokenExtractmiddleware), not401 Unauthorized. The OpenAPI spec should match the actual behavior so clients can handle errors correctly.
creation_time: 1722070800
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1769
/v1/auth/token/renew-selfreturns403 Forbiddenon missing/invalidX-Vault-Token(middleware), not401. OpenAPI should reflect the real status code.
$ref: '#/components/schemas/VaultAuthResponse'
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1783
/v1/auth/token/revoke-selfreturns403 Forbiddenon missing/invalidX-Vault-Token(middleware), not401. Update the OpenAPI responses accordingly.
'204':
description: Token revoked.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:631
jwks_urlis described as a JWKS endpoint, but the example points to/.well-known/openid-configuration(which is not a JWKS document). This is likely to misconfigure clients/admins.
jwks_url:
type: string
format: uri
description: URL of the Kubernetes API server JWKS endpoint.
example: https://kubernetes.default.svc/.well-known/openid-configuration
server/documentation/openapi.yaml:638
- The implementation only supports exact matches plus the
"*"wildcard for bound service-account names/namespaces; it does not implement glob/pattern matching. The OpenAPI description should not claim "glob patterns" unless the server actually supports them.
default: ['*']
description: Allowed service account names (glob patterns; `["*"]` for any).
example: [spire-agent]
server/documentation/openapi.yaml:645
- Same as above for namespaces: the server enforces exact matches plus
"*"wildcard, not glob patterns. Update OpenAPI wording to match behavior.
default: ['*']
description: Allowed namespaces (glob patterns; `["*"]` for any).
example: [spire]
server/src/server/endpoints/vault_approle.rs:48
- New Vault-compatible auth endpoints introduce substantial new behavior (AppRole login, secret-id consumption, token issuance). There are existing integration tests for other HTTP APIs under
server/src/tests/, but no tests are added here to cover success/failure cases (invalid role_id/secret_id, secret-id reuse, TTL/renew, etc.).
server/src/server/endpoints/vault_k8s.rs:127 vault_k8s_create_rolewill storebound_sa_names/bound_sa_namespacesas[]when the fields are omitted (serde default = empty vec). That makes all logins fail by default, even though the schema/OpenAPI defaults indicate['*']. Also, a negativetoken_ttlwould be treated as non-expiring (expiry=0) when issuing tokens.
server/src/server/endpoints/vault_k8s.rs:66serde_json::from_str(&role.bound_sa_names).unwrap_or_default()silently turns invalid JSON in the stored role into an empty allow-list, which then denies all logins without surfacing the configuration error. Returning a server error makes misconfiguration diagnosable.
This issue also appears on line 74 of the same file.
client/src/dto/vault.rs:113
- The DTO docs claim
bound_service_account_names/bound_service_account_namespacesaccept "glob patterns", but the server currently enforces exact match plus"*"wildcard. This doc mismatch can lead users to configure patterns that will never match.
/// URL of the Kubernetes JWKS endpoint for verifying service-account JWTs.
pub jwks_url: String,
/// Allowed service-account names (glob patterns, `["*"]` for any).
#[serde(default)]
pub bound_service_account_names: Vec<String>,
/// Allowed namespaces (glob patterns, `["*"]` for any).
#[serde(default)]
pub bound_service_account_namespaces: Vec<String>,
server/documentation/vault_auth_api.md:257
- This flow description says the server "Selects the correct key by
kid", but the current implementation invalidate_k8s_jwtiterates over all keys and tries to validate until one works. Either update the docs or implementkid-based key selection.
1. Fetches the JWKS from the role's `jwks_url`.
2. Selects the correct key by `kid` from the JWT header.
3. Validates the JWT signature and expiry.
4. Checks `sub` (`system:serviceaccount:<namespace>:<name>`) against the role's allow-lists.
server/documentation/adr/2026-07-26-vault-compatible-auth-api-for-spire.md:96
- The ADR's schema block says
token_policiesis a JSON array, but the implementation stores policies as a comma-separated string (CSV) and splits on,in the handlers. The ADR should match the actual storage format to avoid future migration/interop mistakes.
CREATE TABLE vault_approle_roles (
name TEXT PRIMARY KEY,
role_id TEXT UNIQUE NOT NULL, -- stable UUID SPIRE stores as approle_id
secret_id_ttl_secs INTEGER NOT NULL, -- 0 = no expiry
token_ttl_secs INTEGER NOT NULL,
bind_secret_id BOOLEAN NOT NULL,
token_policies TEXT NOT NULL -- JSON array
);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 33 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (14)
server/documentation/openapi.yaml:1751
- The Vault token self-service endpoints return 403 on missing/invalid/expired tokens (see VaultTokenExtract middleware and vault_auth_api.md), but the OpenAPI spec documents a 401 here. This makes the published contract inconsistent with runtime behavior.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1769
/v1/auth/token/renew-selfis documented as returning 401, but the server responds with 403 when the Vault token is missing/invalid/expired (handled in VaultTokenExtract). OpenAPI should match the actual behavior.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1783
/v1/auth/token/revoke-selfis documented as returning 401, but the middleware returns 403 for missing/invalid/expired Vault tokens. This mismatch can break client codegen and troubleshooting.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:631
jwks_urlis described as a JWKS endpoint, but the example points to the OIDC discovery document (.../openid-configuration). The example should be a JWKS URL (e.g..../jwks.json) to match whatfetch_jwks()expects ({ "keys": [...] }).
example: https://kubernetes.default.svc/.well-known/openid-configuration
server/documentation/openapi.yaml:637
- The OpenAPI schema says
bound_service_account_namessupports glob patterns, but the implementation only supports exact matches plus a literal"*"wildcard. The description should reflect the actual matching behavior.
description: Allowed service account names (glob patterns; `["*"]` for any).
server/documentation/vault_auth_api.md:370
- The schema snippet documents
vault_approle_roles.token_policiesas "comma-separated", but it is persisted as a JSON array string across backends. This should be updated to avoid incorrect expectations when debugging.
token_ttl_secs INTEGER NOT NULL,
bind_secret_id BOOLEAN NOT NULL,
token_policies TEXT NOT NULL -- comma-separated
);
server/src/server/endpoints/vault_k8s.rs:133
- If
bound_service_account_names/bound_service_account_namespacesare omitted,serdedefaults them to empty arrays, and the login check will reject all service accounts/namespaces (because the only wildcard supported is a literal"*"). This contradicts the DB defaults (["*"]) and the OpenAPI defaults, and is likely an accidental deny-all configuration. Also, negativetoken_ttlcurrently becomes a non-expiring token (expiry=0) viaissue_vault_token().
server/src/server/endpoints/vault_approle.rs:109 - Negative TTL inputs are currently accepted for AppRole role creation; because
issue_vault_token()treatslease_duration_secs <= 0asexpiry = 0(never expires), a negativetoken_ttlcan unintentionally mint non-expiring Vault tokens. These TTL fields should be validated as>= 0(and the meaning of0should be explicitly documented).
server/src/server/endpoints/vault_approle.rs:50 - This PR introduces a substantial new Vault-compatible auth surface (AppRole login + admin CRUD, token issuance/renew/revoke). There are extensive integration tests under
server/src/tests/, but none cover these new endpoints yet. Adding end-to-end tests (SQLite backend) for: (1) AppRole role create → get role-id → secret-id generate → login consumes secret-id, (2) token lookup/renew/revoke behavior and status codes, and (3) Kubernetes role default bindings/wildcard behavior would reduce regression risk.
server/documentation/openapi.yaml:1823 - The server accepts both POST and PUT for
/v1/auth/approle/login(for Vault/SPIRE SDK compatibility), but the OpenAPI spec only documents POST. This violates the project rule of keeping endpoints and openapi.yaml in sync.
'403':
description: Invalid role_id or secret_id.
server/documentation/openapi.yaml:644
- The OpenAPI schema says
bound_service_account_namespacessupports glob patterns, but the server checks exact matches plus"*"only. Documenting globs here is misleading for operators.
description: Allowed namespaces (glob patterns; `["*"]` for any).
server/documentation/vault_auth_api.md:256
- This section says the server selects the correct key by
kid, butvalidate_k8s_jwt()currently iterates over all keys and does not usekidfor selection. The doc should match the actual validation behavior (or the code should be updated to usekid).
This issue also appears on line 367 of the same file.
1. Fetches the JWKS from the role's `jwks_url`.
2. Selects the correct key by `kid` from the JWT header.
3. Validates the JWT signature and expiry.
4. Checks `sub` (`system:serviceaccount:<namespace>:<name>`) against the role's allow-lists.
server/documentation/vault_auth_api.md:356
- The schema snippet documents
vault_tokens.policiesas "comma-separated", but all DB backends store it as a JSON array string (seeserde_json::to_string(&token.policies)in vault_issue_token andserde_json::from_stron lookup). This doc mismatch could mislead operators inspecting the DB.
token_hash BLOB PRIMARY KEY, -- SHA-256(raw "hvs.<base64>" token)
entity TEXT NOT NULL, -- AppRole role name or K8s service-account name
policies TEXT NOT NULL, -- comma-separated
expiry INTEGER NOT NULL, -- Unix timestamp; 0 = never
server/documentation/adr/2026-07-26-vault-compatible-auth-api-for-spire.md:83
- This ADR's schema snippet says
vault_tokens.policiesis comma-separated, but the implementation persists policies as a JSON array string in all DB backends. The ADR should match the actual schema/encoding to prevent confusion for future maintainers.
token_hash BLOB PRIMARY KEY, -- SHA-256(raw "hvs.<base64>" token)
entity TEXT NOT NULL, -- AppRole role name / K8s SA name
policies TEXT NOT NULL, -- comma-separated
expiry INTEGER NOT NULL, -- unix timestamp, 0 = never
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 33 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (16)
server/src/server/endpoints/vault_k8s.rs:252
- Same issue in the RS256 fallback loop: a single malformed/unexpected JWK causes an early return (due to
?), instead of being skipped. This can break login if the JWKS contains any key we don't understand.
server/src/server/endpoints/vault_k8s.rs:87 bound_sa_namespacesis parsed withunwrap_or_default(). If the stored JSON is invalid, the namespace allow-list becomes empty and login behavior changes silently. Prefer returning an operator-actionable error instead of defaulting.
server/src/server/endpoints/vault_token.rs:67vault_token_renew_selfreturns 401 when claims are missing, and 500 when the token row can't be re-read after renew. Both cases should be treated as a forbidden/invalid token (403), especially since revocation can race with renewal between these two DB calls.
server/src/server/endpoints/vault_token.rs:111vault_token_revoke_selfmaps missingVaultTokenClaimstoAuthError::Session(401). Since this endpoint is token-authenticated, missing/invalid token should be 403 to align withVaultTokenExtractand Vault client expectations.
server/documentation/openapi.yaml:631K8sRoleRequestin OpenAPI is out of sync with the implemented API/client DTO: it uses an OpenID configuration URL example (not a JWKS), and it omits theexpected_issuerandbound_audiencesfields thatvault_k8s_create_roleaccepts and persists.
jwks_url:
type: string
format: uri
description: URL of the Kubernetes API server JWKS endpoint.
example: https://kubernetes.default.svc/.well-known/openid-configuration
server/documentation/openapi.yaml:1752
- OpenAPI documents
401 Unauthorizedfor/v1/auth/token/lookup-self, but the server returns403 Forbiddenfor missing/invalid/revokedX-Vault-Token(seeVaultTokenExtract). This mismatch can break generated clients and contradicts Vault semantics.
ttl: 3540
creation_time: 1722070800
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1769
- OpenAPI documents
401 Unauthorizedfor/v1/auth/token/renew-self, but the implementation returns403 Forbiddenwhen the token is missing/expired/not-renewable. The contract should reflect the actual status code.
schema:
$ref: '#/components/schemas/VaultAuthResponse'
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1783
- OpenAPI documents
401 Unauthorizedfor/v1/auth/token/revoke-self, butVaultTokenExtractreturns403 ForbiddenwhenX-Vault-Tokenis missing/invalid. This should be reflected in the OpenAPI spec.
responses:
'204':
description: Token revoked.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/vault_auth_api.md:370
- The schema snippet documents
vault_approle_roles.token_policiesas "comma-separated", but the DB layer stores a JSON array string (viaserde_json::to_string(&role.token_policies)). This should be corrected for accuracy.
secret_id_ttl_secs INTEGER NOT NULL,
token_ttl_secs INTEGER NOT NULL,
bind_secret_id BOOLEAN NOT NULL,
token_policies TEXT NOT NULL -- comma-separated
);
server/src/server/endpoints/vault_k8s.rs:229
validate_k8s_jwtreturns an error as soon as it encounters a single malformed/unexpected JWK in the JWKS, because the JWK parse uses?inside the loop. That makes Kubernetes login fragile (one bad key DoS'es all logins) even though the code otherwise intends to skip non-matching keys.
This issue also appears on line 248 of the same file.
server/src/server/endpoints/vault_k8s.rs:62
bound_audiencesis parsed withunwrap_or_default(). If the stored JSON is corrupted or manually edited, this silently disables audience enforcement (treats it as an empty list), weakening auth without surfacing an operator-visible error.
server/src/server/endpoints/vault_k8s.rs:77bound_sa_namesis parsed withunwrap_or_default(). If the stored JSON is invalid, the allow-list becomes empty and the subsequent check denies all logins (or could behave unexpectedly if the check changes). It's safer to fail fast with a clear server-side error so operators can fix the role config.
This issue also appears on line 84 of the same file.
server/src/server/endpoints/vault_token.rs:33
- These token endpoints are behind
VaultTokenExtract(which returns 403 on missing/invalid token), but the handlers map missing claims / missing DB row toAuthError::Session(401). This can happen if the token is revoked between the middleware lookup and the handler’s DB re-read, and should return 403 to match Vault semantics and the middleware behavior.
This issue also appears on line 107 of the same file.
server/documentation/vault_auth_api.md:357
- The database schema snippet says
vault_tokens.policiesis "comma-separated", but the implementations store a JSON array string (they useserde_json::to_string(&token.policies)on insert). The doc should match the actual persisted format to avoid operator confusion.
This issue also appears on line 366 of the same file.
CREATE TABLE vault_tokens (
token_hash BLOB PRIMARY KEY, -- SHA-256(raw "hvs.<base64>" token)
entity TEXT NOT NULL, -- AppRole role name or K8s service-account name
policies TEXT NOT NULL, -- comma-separated
expiry INTEGER NOT NULL, -- Unix timestamp; 0 = never
server/documentation/vault_auth_api.md:388
vault_k8s_rolesin the schema snippet is missing theexpected_issuerandbound_audiencescolumns that are created by all DB backends and used byvault_k8s_login/vault_k8s_create_role. The doc should include these columns so operators can inspect/maintain the table correctly.
-- Kubernetes roles
CREATE TABLE vault_k8s_roles (
name TEXT PRIMARY KEY,
jwks_url TEXT NOT NULL,
bound_sa_names TEXT NOT NULL, -- JSON array
bound_sa_namespaces TEXT NOT NULL, -- JSON array
token_ttl_secs INTEGER NOT NULL
);
server/src/server/auth_verifier.rs:386
- New Vault auth/token scopes are registered here, but there are no corresponding integration tests under
server/src/tests/exercising AppRole login, Kubernetes login (JWT validation + bound SA checks), and token self-service (lookup/renew/revoke). Given the server already has a comprehensive HTTPS test harness, these flows should be covered to prevent regressions in auth semantics.
// ── Vault-compatible auth scopes ─────────────────────────────────────────
//
// Scope prefixes are kept intentionally specific to avoid Actix-web's
// FIFO matching swallowing requests before they reach the right scope.
//
// /v1/auth/approle/login — unauthenticated AppRole login (registered first, most specific)
// /v1/auth/kubernetes/login — unauthenticated K8s login
// /v1/auth/approle — AppRole admin CRUD (CookieAuthSameServer + AdminAuth)
// /v1/auth/kubernetes — K8s admin CRUD
// /v1/auth/token — token self-service (VaultTokenExtract middleware)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 33 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (17)
server/documentation/openapi.yaml:1751
- These endpoints return HTTP 403 when the
X-Vault-Tokenis missing/invalid (theAppTokenExtractmiddleware usesHttpResponse::Forbidden()), but the OpenAPI spec documents401 Unauthorizedhere. This makes the contract inaccurate for clients.
ttl: 3540
creation_time: 1722070800
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1769
/v1/auth/token/renew-selfreturns 403 for missing/invalidX-Vault-Token(middleware), but the OpenAPI spec lists only401 Unauthorized. The documented status code should match the implementation.
schema:
$ref: '#/components/schemas/AppAuthResponse'
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1783
/v1/auth/token/revoke-selfreturns 403 for missing/invalidX-Vault-Token(middleware), but the OpenAPI spec lists only401 Unauthorized. This is a contract mismatch.
'204':
description: Token revoked.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:632
jwks_urlis documented (and used in code) as a JWKS endpoint returning{ "keys": [...] }, but the example points to/.well-known/openid-configuration(OIDC discovery), which would failfetch_jwksparsing. The example should be a JWKS URL.
type: string
format: uri
description: URL of the Kubernetes API server JWKS endpoint.
example: https://kubernetes.default.svc/.well-known/openid-configuration
bound_service_account_names:
server/documentation/openapi.yaml:638
- The implementation only supports exact matches plus the special wildcard
"*"forbound_service_account_names(it usesVec::contains), but the schema description says “glob patterns”. This description should match the actual matching semantics.
type: array
items:
type: string
default: ['*']
description: Allowed service account names (glob patterns; `["*"]` for any).
example: [spire-agent]
client/src/dto/app_auth.rs:116
- The doc comment says namespaces support glob patterns, but the server code only supports exact matches plus
"*". Keeping this accurate helps avoid misconfiguration.
#[serde(default)]
pub bound_service_account_names: Vec<String>,
/// Allowed namespaces (glob patterns, `["*"]` for any).
#[serde(default)]
pub bound_service_account_namespaces: Vec<String>,
/// TTL for issued tokens in seconds.
#[serde(default = "default_token_ttl")]
pub token_ttl: i64,
server/documentation/app_auth_api.md:370
- The DB schema example says
approle_roles.token_policiesis “comma-separated”, but the implementation stores JSON (serde_json::to_string(&role.token_policies)). The doc should match the actual storage format.
secret_id_ttl_secs INTEGER NOT NULL,
token_ttl_secs INTEGER NOT NULL,
bind_secret_id BOOLEAN NOT NULL,
token_policies TEXT NOT NULL -- comma-separated
);
server/src/server/endpoints/auth_token.rs:79
auth_token_renew_selfmaps a missing token after renewal toAuthError::Generic, which becomes HTTP 500. A token that is revoked/expired concurrently should be treated as a client-side auth failure (403/401), not an internal error.
server/documentation/openapi.yaml:1791- The server accepts both POST and PUT for
/v1/auth/approle/login(see#[route("", method = "POST", method = "PUT")]inserver/src/server/endpoints/approle.rs), but the OpenAPI spec documents only POST. This can break client generation / validation for SPIRE which may use PUT.
/v1/auth/approle/login:
post:
tags: [AppRole]
summary: Login with AppRole credentials
description: |
server/documentation/openapi.yaml:644
- The implementation only supports exact matches plus the special wildcard
"*"forbound_service_account_namespaces(it usesVec::contains), but the schema description says “glob patterns”. This description should match the actual matching semantics.
bound_service_account_namespaces:
type: array
items:
type: string
default: ['*']
description: Allowed namespaces (glob patterns; `["*"]` for any).
example: [spire]
server/documentation/openapi.yaml:649
K8sRoleRequestin Rust includesexpected_issuerandbound_audiences, andk8s_create_rolepersists both, but the OpenAPI schema omits them. This makes the API contract incomplete.
token_ttl:
type: integer
default: 3600
description: Issued app token TTL in seconds.
client/src/dto/app_auth.rs:108
- The doc comments describe
bound_service_account_names/bound_service_account_namespacesas “glob patterns”, but the server-side check only supports exact matches plus"*"(wildcard). The docs should reflect the actual matching semantics so clients don’t assume globbing support.
This issue also appears on line 109 of the same file.
pub struct K8sRoleRequest {
/// URL of the Kubernetes JWKS endpoint for verifying service-account JWTs.
pub jwks_url: String,
/// Allowed service-account names (glob patterns, `["*"]` for any).
#[serde(default)]
pub bound_service_account_names: Vec<String>,
/// Allowed namespaces (glob patterns, `["*"]` for any).
#[serde(default)]
pub bound_service_account_namespaces: Vec<String>,
server/documentation/app_auth_api.md:355
- The DB schema example says
app_tokens.policiesis “comma-separated”, but the implementation stores JSON (serde_json::to_string(&token.policies)). This doc mismatch can confuse operators inspecting the DB.
This issue also appears on line 366 of the same file.
CREATE TABLE app_tokens (
token_hash BLOB PRIMARY KEY, -- SHA-256(raw "hvs.<base64>" token)
entity TEXT NOT NULL, -- AppRole role name or K8s service-account name
policies TEXT NOT NULL, -- comma-separated
expiry INTEGER NOT NULL, -- Unix timestamp; 0 = never
renewable BOOLEAN NOT NULL,
lease_duration_secs INTEGER NOT NULL,
created_at INTEGER NOT NULL
);
server/documentation/adr/2026-07-26-app-auth-api-for-spire.md:82
- The ADR’s schema sketch says
app_tokens.policiesis “comma-separated”, but the code stores JSON arrays in this column. This should be corrected so the ADR remains an accurate design reference.
CREATE TABLE app_tokens (
token_hash BLOB PRIMARY KEY, -- SHA-256(raw "hvs.<base64>" token)
entity TEXT NOT NULL, -- AppRole role name / K8s SA name
policies TEXT NOT NULL, -- comma-separated
expiry INTEGER NOT NULL, -- unix timestamp, 0 = never
server/src/database/impls/sqlite.rs:144
app_tokens.policiesis stored as JSON (seeserde_json::to_string(&token.policies)), but the table DDL setsDEFAULT ''which is not valid JSON. UsingDEFAULT '[]'keeps the schema consistent with how the column is parsed/stored.
CREATE TABLE IF NOT EXISTS app_tokens (
token_hash BLOB NOT NULL PRIMARY KEY,
entity TEXT NOT NULL,
policies TEXT NOT NULL DEFAULT '',
expiry INTEGER NOT NULL,
renewable INTEGER NOT NULL DEFAULT 0,
lease_duration_secs INTEGER NOT NULL DEFAULT 3600,
created_at INTEGER NOT NULL
server/src/database/impls/postgres.rs:122
app_tokens.policiesis stored as JSON (seeserde_json::to_string(&token.policies)), but the table DDL setsDEFAULT ''which is not valid JSON. UsingDEFAULT '[]'keeps the schema consistent with how the column is parsed/stored.
CREATE TABLE IF NOT EXISTS app_tokens (
token_hash BYTEA NOT NULL PRIMARY KEY,
entity TEXT NOT NULL,
policies TEXT NOT NULL DEFAULT '',
expiry BIGINT NOT NULL,
renewable BOOLEAN NOT NULL DEFAULT FALSE,
lease_duration_secs BIGINT NOT NULL DEFAULT 3600,
created_at BIGINT NOT NULL
server/src/server/auth_verifier.rs:385
- New
/v1/auth/*endpoints and theAppTokenExtractmiddleware introduce substantial new auth and persistence behavior, but there are no integration tests covering AppRole login, secret-id consumption semantics, K8s JWT validation, or token self-service. The repo already has extensive endpoint/integration tests underserver/src/tests/, so this is a noticeable gap for a security-sensitive feature.
// ── AppRole-compatible auth scopes ─────────────────────────────────────────
//
// Scope prefixes are kept intentionally specific to avoid Actix-web's
// FIFO matching swallowing requests before they reach the right scope.
//
// /v1/auth/approle/login — unauthenticated AppRole login (registered first, most specific)
// /v1/auth/kubernetes/login — unauthenticated K8s login
// /v1/auth/approle — AppRole admin CRUD (CookieAuthSameServer + AdminAuth)
// /v1/auth/kubernetes — K8s admin CRUD
// /v1/auth/token — token self-service (AppTokenExtract middleware)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 33 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (11)
server/documentation/openapi.yaml:1752
- The app-token endpoints currently document
401 Unauthorized, but the implementation (AppTokenExtract middleware) returns403 Forbiddenfor missing/invalid/expiredX-Vault-Token. The OpenAPI spec should match the actual status codes so client integrations don't mis-handle auth failures.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1769
/auth/token/renew-selfdocuments401 Unauthorized, but the server returns403 Forbiddenwhen the token is missing/invalid/not-renewable (via AppTokenExtract + renew_app_token mapping). Update the OpenAPI responses accordingly.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1783
/auth/token/revoke-selfdocuments401 Unauthorized, but the server returns403 ForbiddenwhenX-Vault-Tokenis missing/invalid (AppTokenExtract). Update the OpenAPI responses to match.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:632
K8sRoleRequest.jwks_urlis described as a JWKS endpoint, but the example points to/.well-known/openid-configuration(OIDC discovery), which will not return a{ keys: [...] }JWKS and would fail at runtime. Update the example to a JWKS JSON URL.
jwks_url:
type: string
format: uri
description: URL of the Kubernetes API server JWKS endpoint.
example: https://kubernetes.default.svc/.well-known/openid-configuration
bound_service_account_names:
server/documentation/openapi.yaml:645
- The OpenAPI descriptions claim
bound_service_account_names/bound_service_account_namespacesare "glob patterns", but the implementation only supports exact matches plus the special wildcard value"*". Either implement glob matching or adjust the documentation to avoid misleading operators.
bound_service_account_names:
type: array
items:
type: string
default: ['*']
description: Allowed service account names (glob patterns; `["*"]` for any).
example: [spire-agent]
bound_service_account_namespaces:
type: array
items:
type: string
default: ['*']
description: Allowed namespaces (glob patterns; `["*"]` for any).
example: [spire]
server/src/server/endpoints/kubernetes.rs:281
build_validationclearsvalidation.required_spec_claims, which makesexpno longer required even thoughvalidate_exp = true. This can allow acceptance of service-account JWTs without an expiry (depending on jsonwebtoken behavior), weakening replay resistance. Prefer requiring at leastexp(and typicallysub) for these tokens.
server/documentation/adr/2026-07-26-app-auth-api-for-spire.md:58- This ADR describes the new endpoints under a
/v1/auth/prefix, but the server routes and OpenAPI in this PR are registered under/auth/...(no/v1). The ADR should match the actual API surface to avoid deployment/configuration errors (e.g., SPIRE/KMS pointing at the wrong path).
### Decision 1 — New `/v1/auth/` scope, unauthenticated login + admin-gated CRUD
Three new endpoint modules under `server/src/server/endpoints/`:
| Module | Routes | Auth |
|--------|--------|------|
| `approle.rs` | `POST/PUT /v1/auth/approle/login` | none (credential is the body) |
| `approle.rs` | `POST /v1/auth/approle/role/{name}`, `GET .../role-id`, `POST .../secret-id`, `POST .../secret-id/destroy`, `DELETE /role/{name}`, `GET /role?list=true` | `CookieAuthSameServer` + `AdminAuth` |
| `kubernetes.rs` | `POST /v1/auth/kubernetes/login` | none (credential is the K8s SA JWT) |
| `kubernetes.rs` | `POST /v1/auth/kubernetes/role/{name}`, `DELETE /role/{name}` | `CookieAuthSameServer` + `AdminAuth` |
| `auth_token.rs` | `GET /v1/auth/token/lookup-self`, `POST /v1/auth/token/renew-self`, `POST /v1/auth/token/revoke-self` | `app_token_extract` middleware |
server/documentation/app_auth_api.md:370
- The DB schema snippet documents
app_tokens.policiesandapprole_roles.token_policiesas "comma-separated", but the implementations store JSON (theyserde_json::to_stringaVec<String>and parse JSON on reads). This mismatch will confuse operators inspecting DB state and future maintainers.
CREATE TABLE app_tokens (
token_hash BLOB PRIMARY KEY, -- SHA-256(raw "hvs.<base64>" token)
entity TEXT NOT NULL, -- AppRole role name or K8s service-account name
policies TEXT NOT NULL, -- comma-separated
expiry INTEGER NOT NULL, -- Unix timestamp; 0 = never
renewable BOOLEAN NOT NULL,
lease_duration_secs INTEGER NOT NULL,
created_at INTEGER NOT NULL
);
-- AppRole roles
CREATE TABLE approle_roles (
name TEXT PRIMARY KEY,
role_id TEXT UNIQUE NOT NULL,
secret_id_ttl_secs INTEGER NOT NULL,
token_ttl_secs INTEGER NOT NULL,
bind_secret_id BOOLEAN NOT NULL,
token_policies TEXT NOT NULL -- comma-separated
);
client/src/dto/app_auth.rs:33
AppRoleRoleRequest.bind_secret_idis documented as allowing login withrole_idonly when set tofalse, butAppRoleLoginRequestcurrently requiressecret_id: String(non-optional). As-is, role_id-only login cannot work (serde will reject bodies withoutsecret_id). Either makesecret_idoptional end-to-end or remove/adjust thebind_secret_idsemantics/documentation.
/// Request body for `POST /auth/approle/login`.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct AppRoleLoginRequest {
pub role_id: String,
pub secret_id: String,
}
/// Request body for `POST /auth/approle/role/{name}` (create/update).
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct AppRoleRoleRequest {
/// TTL for secret IDs in seconds (0 = no expiry).
#[serde(default)]
pub secret_id_ttl: i64,
/// TTL for issued tokens in seconds.
#[serde(default = "default_token_ttl")]
pub token_ttl: i64,
/// List of policies to attach to issued tokens.
#[serde(default)]
pub token_policies: Vec<String>,
/// Whether `secret_id` is required for login. Defaults to `true`; set to
/// `false` to allow login with `role_id` only (not recommended for production).
#[serde(default = "default_true")]
pub bind_secret_id: bool,
}
client/src/dto/app_auth.rs:113
K8sRoleRequestdocs say the bound service-account names/namespaces support "glob patterns", but the server implementation currently only supports exact matches plus the special wildcard value"*". Update these doc comments (or implement actual glob matching) so callers configure the role correctly.
pub struct K8sRoleRequest {
/// URL of the Kubernetes JWKS endpoint for verifying service-account JWTs.
pub jwks_url: String,
/// Allowed service-account names (glob patterns, `["*"]` for any).
#[serde(default)]
pub bound_service_account_names: Vec<String>,
/// Allowed namespaces (glob patterns, `["*"]` for any).
#[serde(default)]
pub bound_service_account_namespaces: Vec<String>,
server/documentation/adr/2026-07-26-app-auth-api-for-spire.md:87
- In the ADR's schema sketch,
app_tokens.policiesis documented as "comma-separated", but the code stores JSON (it writesserde_json::to_string(Vec<String>)and reads it back as JSON). Adjust the ADR so it matches the implemented storage format.
token_hash BLOB PRIMARY KEY, -- SHA-256(raw "hvs.<base64>" token)
entity TEXT NOT NULL, -- AppRole role name / K8s SA name
policies TEXT NOT NULL, -- comma-separated
expiry INTEGER NOT NULL, -- unix timestamp, 0 = never
renewable BOOLEAN NOT NULL,
lease_duration_secs INTEGER NOT NULL,
created_at INTEGER NOT NULL
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 37 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (18)
server/documentation/openapi.yaml:637
- The OpenAPI description says
bound_service_account_namessupports glob patterns, but the implementation only supports exact matches plus the special wildcard value"*". The docs should reflect the actual matching semantics to avoid unexpected auth failures.
description: Allowed service account names (glob patterns; `["*"]` for any).
server/documentation/openapi.yaml:631
K8sRoleRequest.jwks_urlexample points to/.well-known/openid-configuration, but the server expects a JWKS document with a top-levelkeysarray. Using the OpenID configuration URL here will fail at runtime.
example: https://kubernetes.default.svc/.well-known/openid-configuration
server/documentation/openapi.yaml:1751
- The token self-service endpoints return
403 Forbiddenon missing/invalid tokens (seeAppTokenExtractandapp_auth_api.md), but the OpenAPI spec documents401 Unauthorized. This mismatch will confuse clients and generated SDKs.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1769
/auth/token/renew-selfdocuments a401response, but the implementation responds with403 Forbiddenfor missing/invalid/non-renewable tokens (via middleware + handler mapping). The OpenAPI responses should match actual behavior.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/openapi.yaml:1783
/auth/token/revoke-selfdocuments a401response, but the middleware returns403 Forbiddenfor missing/invalid tokens. Update the OpenAPI responses to match runtime behavior.
'401':
$ref: '#/components/responses/Unauthorized'
server/documentation/adr/2026-07-26-app-auth-api-for-spire.md:58
- The routes table uses
/v1/auth/...paths, but the server registers scopes under/auth/...and the endpoint modules use/auth/...in their module docs. This inconsistency makes it hard to deploy the feature correctly (and may break SPIRE if it truly requires/v1).
| `approle.rs` | `POST/PUT /v1/auth/approle/login` | none (credential is the body) |
| `approle.rs` | `POST /v1/auth/approle/role/{name}`, `GET .../role-id`, `POST .../secret-id`, `POST .../secret-id/destroy`, `DELETE /role/{name}`, `GET /role?list=true` | `CookieAuthSameServer` + `AdminAuth` |
| `kubernetes.rs` | `POST /v1/auth/kubernetes/login` | none (credential is the K8s SA JWT) |
| `kubernetes.rs` | `POST /v1/auth/kubernetes/role/{name}`, `DELETE /role/{name}` | `CookieAuthSameServer` + `AdminAuth` |
| `auth_token.rs` | `GET /v1/auth/token/lookup-self`, `POST /v1/auth/token/renew-self`, `POST /v1/auth/token/revoke-self` | `app_token_extract` middleware |
server/documentation/adr/2026-07-26-app-auth-api-for-spire.md:66
- This paragraph still refers to the
/v1/auth/prefix, but the rest of the docs/OpenAPI and the Actix scopes are/auth/.... The wording should be updated so readers don’t configure the wrong paths.
**Distinct scope prefix**: the `/v1/auth/` scope was deliberately kept separate from
`/login`, `/realms`, etc. to avoid an Actix-web FIFO routing conflict between the new
unauthenticated login routes and existing authenticated scopes matching a similar path
client/src/dto/app_auth.rs:113
- This DTO doc comment says
bound_service_account_namespacessupports glob patterns, but the server currently only supports exact matches plus"*". The docs should match behavior.
/// Allowed namespaces (glob patterns, `["*"]` for any).
#[serde(default)]
pub bound_service_account_namespaces: Vec<String>,
server/documentation/openapi.yaml:649
K8sRoleRequestin the OpenAPI schema is missing theexpected_issuerandbound_audiencesfields, but the server handler (k8s_create_role) and client DTO (K8sRoleRequest) both support them. This makes the OpenAPI contract incomplete and will mislead clients/codegen.
token_ttl:
type: integer
default: 3600
description: Issued app token TTL in seconds.
server/documentation/openapi.yaml:644
- The OpenAPI description says
bound_service_account_namespacessupports glob patterns, but the implementation only supports exact matches plus the special wildcard value"*". Documenting this as globbing is misleading.
description: Allowed namespaces (glob patterns; `["*"]` for any).
server/documentation/openapi.yaml:1790
- The server accepts both
POSTandPUTfor/auth/approle/login(see#[route(... method = "POST", method = "PUT")]), but the OpenAPI spec documents onlypost. For Vault/SPIRE compatibility and accurate codegen, the spec should also include aputoperation with the same request/response schema.
/auth/approle/login:
post:
tags: [AppRole]
summary: Login with AppRole credentials
server/documentation/app_auth_api.md:255
- This doc says Kubernetes login selects the JWKS key by
kid, butvalidate_k8s_jwtcurrently tries each key in the JWKS and does not filter bykid. The step list should match the actual implementation.
1. Fetches the JWKS from the role's `jwks_url`.
2. Selects the correct key by `kid` from the JWT header.
3. Validates the JWT signature and expiry.
server/documentation/adr/2026-07-26-app-auth-api-for-spire.md:48
- This ADR describes the feature as being under a
/v1/auth/scope, but the implementation and OpenAPI are using/auth/...(no/v1). The ADR should be aligned with the actual route prefix (or the code should be changed to match the ADR).
This issue also appears in the following locations of the same file:
- line 54
- line 64
### Decision 1 — New `/v1/auth/` scope, unauthenticated login + admin-gated CRUD
CHANGELOG/spire-app-auth-api.md:27
- The changelog documents the new endpoints under
/v1/auth/..., but the server registers them under/auth/...and the OpenAPI spec uses/auth/.... The changelog should match the actual public routes.
- **Token endpoints** under `/v1/auth/token/`:
- `GET /lookup-self` — validate a token and return entity + policies;
- `POST /renew-self` — extend a renewable token's TTL;
- `POST /revoke-self` — immediately invalidate a token.
- **AppRole endpoints** under `/v1/auth/approle/`:
client/src/dto/app_auth.rs:110
- This DTO doc comment says
bound_service_account_namessupports glob patterns, but the server implementation only supports exact matches plus the special wildcard value"*". Keeping the client docs accurate helps prevent misconfiguration.
This issue also appears on line 111 of the same file.
/// Allowed service-account names (glob patterns, `["*"]` for any).
#[serde(default)]
pub bound_service_account_names: Vec<String>,
server/src/server/endpoints/mod.rs:82
lease_duration_secscan be negative (it comes from request bodies / DB values). With the current logic, a negative value producesexpiry = 0(non-expiring) but preserves a negativelease_duration_secs, which later yields a negativettlin lookup responses. Clamp the lease duration to>= 0before computing expiry and storing the record.
server/src/server/auth_verifier.rs:385- These scopes are registered under
/auth/..., but the ADR and changelog in this PR describe/v1/auth/...(and SPIRE/HashiCorp Vault clients typically call/v1/auth/...). Please reconcile the public route prefix across code + docs (either change the scopes to/v1/auth/...or update all docs/consumers to/auth/...).
server/src/server/auth_verifier.rs:391 - New unauthenticated login endpoints and token self-service endpoints are introduced here, but there are no integration tests covering: (1) AppRole login success/failure and secret_id consumption; (2) Kubernetes login JWT validation + bound SA/namespace/audience enforcement; (3) token lookup/renew/revoke behavior. The server crate already has an integration test harness, so adding tests would prevent regressions in this security-sensitive surface.
…corrections Security fixes: - kubernetes.rs: fail closed on corrupt bound_audiences/bound_sa_names/bound_sa_namespaces JSON in the DB (previously unwrap_or_default() silently disabled checks — CWE-754) - kubernetes.rs: require 'exp' claim in required_spec_claims so JWTs without an expiry are rejected; previously validate_exp=true was set but exp was not required (RFC 7519 §4.1.4) - kubernetes.rs: disable reqwest redirect following in fetch_jwks to prevent downgrade from https:// to plaintext HTTP via redirect; add error_for_status() for non-2xx responses - kubernetes.rs: normalize empty bound_sa_names/bound_sa_namespaces to ["*"] at create-role time to match the documented default (omitting = allow all) - kubernetes.rs: reject negative token_ttl values in k8s_create_role - approle.rs: reject negative secret_id_ttl/token_ttl values in approle_create_role - app_token_extract.rs: replace raw DB error string in HTTP 500 with generic message to avoid leaking internal details (CWE-209 information exposure) OpenAPI corrections: - token self-service (lookup-self, renew-self, revoke-self): change 401 to 403 to match actual middleware behaviour (AppTokenExtract returns HTTP 403, not 401) - renew-self: document that auth.client_token echoes the presented token per Vault spec §3.6 - K8sRoleRequest.jwks_url: fix example from OIDC discovery URL to JWKS document URL; add note that redirects are not followed - K8sRoleRequest.bound_service_account_names/namespaces: replace 'glob patterns' with accurate 'exact match or * wildcard' description Test fixes: - app_auth_tests: update assertions to expect client_token/id echoed per Vault spec - auth_token.rs: implement token echo in lookup-self and renew-self per Vault API spec Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 43 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (18)
server/documentation/openapi.yaml:1774
- The
/auth/token/lookup-selfexample showsdata.id: '', but the endpoint echoes back the token presented inX-Vault-Token. The example should reflect that to match actual behavior.
data:
id: ''
entity_id: spire-server
server/documentation/app_auth_api.md:337
- The text says
idis always empty, but the implementation returnsidas the raw token fromX-Vault-Token(to match Vault token lookup-self expectations). This sentence should be updated to avoid contradicting the server behavior.
`id` is always an empty string — the raw token is never echoed back.
server/documentation/app_auth_api.md:364
- The text says
client_tokenis always empty, but the implementation returns the presented token inauth.client_token(Vault renew-self behavior). This should be updated to match reality.
`client_token` is always empty — the raw token is never echoed back.
CHANGELOG/spire-app-auth-api.md:15
- This changelog path prefix (
/v1/auth/approle/) does not match the implemented routes (/auth/approle/...).
- **AppRole endpoints** under `/v1/auth/approle/`:
CHANGELOG/spire-app-auth-api.md:24
- This changelog path prefix (
/v1/auth/kubernetes/) does not match the implemented routes (/auth/kubernetes/...).
- **Kubernetes auth endpoints** under `/v1/auth/kubernetes/`:
server/src/server/endpoints/auth_token.rs:44
- Use the shared
APP_TOKEN_HEADERconstant here instead of a string literal to keep the header name consistent across middleware and endpoints.
server/src/server/endpoints/auth_token.rs:106 - Use the shared
APP_TOKEN_HEADERconstant here instead of a string literal to keep the header name consistent across middleware and endpoints.
server/documentation/app_auth_api.md:405 - The DB schema comment says
approle_roles.token_policiesis "comma-separated", but it is stored as a JSON array string in all backends (viaserde_json::to_string(&role.token_policies)).
token_policies TEXT NOT NULL -- comma-separated
server/documentation/openapi.yaml:723
AppTokenData.idis documented as always empty, but the server implementation returnsdata.idas the raw token from theX-Vault-Tokenheader (per Vault lookup-self behavior and the integration tests). This mismatch will mislead generated clients and consumers.
This issue also appears on line 1772 of the same file.
properties:
id:
type: string
description: Token ID — always empty string (never echoed for security).
entity_id:
server/documentation/app_auth_api.md:327
- This lookup-self response example sets
data.idto an empty string, but the server actually echoes back the presented token indata.id(and the integration tests assert that behavior).
This issue also appears on line 337 of the same file.
"id": "",
server/documentation/app_auth_api.md:355
- This renew-self response example sets
auth.client_tokento an empty string, but the server echoes back the presented token inauth.client_token(and tests assert this).
This issue also appears on line 364 of the same file.
"client_token": "",
CHANGELOG/spire-app-auth-api.md:11
- This changelog still documents the app-auth endpoints under
/v1/auth/..., but the implementation and OpenAPI use/auth/...(no/v1prefix). This will mislead users following the changelog.
This issue also appears in the following locations of the same file:
- line 15
- line 24
- **Token endpoints** under `/v1/auth/token/`:
server/src/server/endpoints/kubernetes.rs:150
- This comment says an explicit empty array is semantically different (deny-all), but with the current DTO type (
Vec<String>) the server cannot distinguish an omitted field from an explicitly empty array — both deserialize to an empty vec and are normalized to["*"]. The comment should be corrected to match the actual behavior.
server/src/server/endpoints/auth_token.rs:9 - This module hard-codes the
X-Vault-Tokenheader name in multiple places. Since the header name is already defined asAPP_TOKEN_HEADERin the app-token middleware, using the shared constant avoids drift if the header name ever changes.
This issue also appears in the following locations of the same file:
- line 41
- line 103
server/documentation/app_auth_api.md:391
- The DB schema comment says
app_tokens.policiesis "comma-separated", but the database implementations persist it as a JSON array string viaserde_json::to_string(&token.policies). The docs should match the actual storage format.
This issue also appears on line 405 of the same file.
policies TEXT NOT NULL, -- comma-separated
server/documentation/adr/2026-07-26-app-auth-api-for-spire.md:89
- The ADR's schema snippet says
app_tokens.policiesis "comma-separated", but the code persists policies as a JSON array string (see theissue_app_tokenimplementations that bindserde_json::to_string(&token.policies)). Update the ADR to avoid confusing operators inspecting the DB.
policies TEXT NOT NULL, -- comma-separated
server/src/server/endpoints/approle.rs:196
ttlis accepted as any integer; negative values currently fall back to the role default (same behavior as 0), which is surprising and inconsistent with the negative-TTL validation applied inapprole_create_role. It would be clearer to rejectttl < 0with400 BadRequest.
server/src/server/endpoints/approle.rs:207num_usescurrently treats any non-positive value (including negatives) as unlimited (-1). Given the API docs define0 = unlimited, accepting negative numbers as a synonym is unexpected input tolerance and makes it harder to detect misconfiguration. Consider rejectingnum_uses < 0with400 BadRequest.
HatemMn
left a comment
There was a problem hiding this comment.
One diagnosability gap in the new auth-scope registration: the unimplemented cert-auth method fails closed (safe) but silently, with a bare 404 instead of a structured error.
HatemMn
left a comment
There was a problem hiding this comment.
One consistency gap: Kubernetes login error messages distinguish role-not-found from JWT-validation-failure, unlike AppRole's uniform error — a low-value but easy-to-fix role-name enumeration oracle.
HatemMn
left a comment
There was a problem hiding this comment.
Two findings from the SPIRE-branch static review, re-posted after confirming they hadn't actually gone out in an earlier pass: an unimplemented cert-auth method that fails closed but silently (bare 404, no structured error), and a Kubernetes login error message that leaks role existence unlike AppRole's uniform error.
|
Note: my last review duplicates two findings (cert-auth 404, k8s role-existence leak) already posted earlier in this thread — my tooling gave me a stale read that made it look like they hadn't gone out. Apologies for the noise; the content is the same finding, not a new one. |
HatemMn
left a comment
There was a problem hiding this comment.
Round 2 review — live-confirmed Content-Type asymmetry between the K8s and AppRole login handlers.
- k8s_login: Content-Type-agnostic body parsing (Bytes + from_slice) to match
approle_login and SPIRE's Go client (ADR Decision 1).
- k8s_login: uniform generic error for all auth failures (role-not-found, JWT,
binding) to remove the role-name enumeration oracle (CWE-204); specifics logged.
- auth_verifier: structured default_service so unsupported auth methods (e.g.
cert_auth) fail closed with an {"errors":[...]} envelope, not a bare 404.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Update admin-ui pnpm dependency hashes for linux-x86_64 and linux-aarch64 after pnpm-lock.yaml drift - Flush React 19 scheduler setImmediate callbacks in vitest afterEach to prevent 'window is not defined' race on CI Linux environments
After clicking Renew, a toast notification 'Token renewed — new lease
7200s' appeared alongside the TTL display '7200s', causing
getByText('7200s') to resolve to 2 elements in Playwright strict mode.
Changed to getByText('7200s', { exact: true }).
Previously the server only used RUST_LOG env var via log_init(None). Now a [log] section in the TOML config controls the minimum log level, defaulting to "info". Target-qualified filters like "info,auth_verifier=debug" are also supported. Configs updated: - auth_verifier.toml: [log] level = "info" - auth_verifier.dev.toml: [log] level = "debug" - auth_verifier.spire.toml: [log] level = "info" (replaces old comment saying log level was not configurable)
…dcoded roles from default config - Rename auth_verifier.service → cosmian_auth_verifier.service for naming consistency with cosmian_kms.service - Update SyslogIdentifier to cosmian_auth_verifier so journalctl output is uniformly prefixed cosmian_* - Update cargo-deb unit-name to match new service file name - Comment out roles = [...] in auth_verifier.toml and auth_verifier.spire.toml: the default sample config should not prescribe domain-specific role names; operators define their own list matching their OPA policy Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…dow-is-not-defined in CI React 19 + scheduler@0.27 chains multiple setImmediate callbacks via performWorkUntilDeadline → schedulePerformWorkUntilDeadline. After the antd v5 → v6 upgrade a single flush round was insufficient; draining three rounds covers all known cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The no-await-in-loop rule is not enabled in this ESLint config so the disable directive was flagged as unused with --max-warnings 0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…service name The service was renamed from auth_verifier to cosmian_auth_verifier for naming consistency with cosmian_kms. Update all systemctl/journalctl/ systemd-analyze references in the packaging test workflow accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
No description provided.