Add OIDC JWT authentication and group-based authorization#32
Open
hanstrompert wants to merge 13 commits into
Open
Add OIDC JWT authentication and group-based authorization#32hanstrompert wants to merge 13 commits into
hanstrompert wants to merge 13 commits into
Conversation
- Add PyJWT[crypto] dependency for RS256 JWT validation - Create dds_proxy/auth.py with OIDCProvider class (JWKS key retrieval via PyJWKClient, userinfo endpoint with TTL cache) - Add get_authenticated_user FastAPI dependency applied to all data routes via include_router(dependencies=...) - Token validation is opportunistic: JWTs are validated when present but requests without a Bearer token pass through, enabling a single deployment behind both mTLS and OIDC ingresses - Add 9 OIDC settings to config.py (issuer, audience, JWKS URI, userinfo URI, group claim, required groups, cache TTLs) - Support comma-separated and JSON array formats for OIDC_REQUIRED_GROUPS via field validator - Separate vanilla httpx.AsyncClient for OIDC calls (not the mTLS DDS client), with auto-discovery from .well-known endpoint - Add structured logging at INFO (authn/authz decisions), WARNING (rejected tokens), and DEBUG (claims, userinfo, JWKS) - Add 29 tests covering token extraction, signature validation, claim validation, group authorization, passthrough, config parsing - Update CLAUDE.md, README.md, and dds_proxy.env Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Document that OIDC_REQUIRED_GROUPS must be `[]` (not empty string) when no groups are required — pydantic-settings JSON-parses list env vars before field validators run, so `""` causes a SettingsError - Update README.md default from _(empty)_ to `[]` with explanation - Add caveat to CLAUDE.md Key Design Decisions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Log each request header name and value at DEBUG level in the auth dependency to diagnose header forwarding issues Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The nginx ingress controller has a known issue where it clears the Authorization header from auth subrequest responses. Custom X-* headers are forwarded correctly. - Check Authorization: Bearer first, fall back to X-Auth-Request-Access-Token when absent (set by oauth2-proxy with --pass-access-token) - Add info logging for which token source is used - Parametrize signature validation test for both header paths - Document the nginx issue and fallback behavior in README Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove per-header debug loop added for diagnosing nginx header forwarding; targeted debug logs for token claims, userinfo, and group checks remain Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- check_groups now returns the matched groups list - Log email and matched_groups at INFO level on successful group authorization for easier auditing - Update check_groups tests to verify returned matched groups Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace oidc_enabled with auth_enabled and add mtls_header setting - Rewrite auth flow: auth off = passthrough, auth on = OIDC or mTLS must succeed, otherwise 401 - OIDC is active when oidc_issuer is set; mTLS is active when mtls_header is set and the header is present in the request - Add mTLS test class with 5 tests covering header present, absent, JWT priority, and fallback scenarios - Update existing tests for new auth_enabled setting - Update README, CLAUDE.md, and dds_proxy.env template Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add test for invalid JWT not rescued by mTLS header (security path) - Add tests for empty and whitespace mTLS header values - Add test for auth enabled with neither method configured - Add test for mTLS success without X-Client-DN header - Consolidate mTLS-only tests into parametrized test - Extract _mtls_only_client and _dual_auth_client helpers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Bound userinfo cache to 1024 entries to prevent unbounded memory growth - Consolidate 4 JWT exception handlers into single handler with message lookup table - Replace two-pass cache eviction with single-pass dict comprehension - Read X-Auth-Request-Access-Token header once, reuse for group lookup - Extract _mock_dds_http_client() helper to deduplicate test setup Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Pydantic-settings JSON-parses list env vars, so bare strings can cause startup errors. Use JSON array to match documented guidance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Mermaid flowchart showing mTLS and OIDC ingress paths, auth services, header flows, and dds-proxy auth decision logic - Defense-in-depth measures and header flow summary tables - Link from README Authentication section Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Inline Mermaid diagram, defense-in-depth table, and header flow summary directly into the Authentication section of README - Remove standalone artwork/dual-ingress-auth.md (artwork/ is for images, not documentation) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Reorder except blocks so PyJWKClientError is caught before PyJWTError (it's a subclass), returning generic "Token validation failed" instead of leaking internal error details - Sanitize userinfo fetch error response to not expose internal error messages to clients - Wrap json.loads in OIDC_REQUIRED_GROUPS validator with proper error handling for malformed JSON input - Validate OIDC discovery returns both jwks_uri and userinfo_endpoint, failing fast at startup if incomplete - Add tests for JWKS failure, sanitized error messages, and malformed JSON config - Document error responses table in README authentication section - Clarify access token requirement for group-based authorization - Use "nsi-auth" consistently instead of "auth service" Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
OIDC_ENABLED,OIDC_ISSUER,OIDC_AUDIENCE, etc.) — auth is disabled by default for backward compatibilityTest plan
OIDC_ENABLED=false— verify all endpoints work without tokens (backward compat)OIDC_ENABLED=truebehind OIDC ingress — verify JWT validation and/docsaccessOIDC_ENABLED=truebehind mTLS ingress — verify passthrough without JWT/healthreturns 200 without a token in all configurationsOIDC_REQUIRED_GROUPSset🤖 Generated with Claude Code