Add optional OIDC auth #102
Open
Conversation
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
- Use cryptographically secure random key for session store instead of client secret - Add proper session cookie security flags (HttpOnly, Secure, SameSite) - Implement token expiry validation to prevent using expired tokens - Add comprehensive error handling throughout OIDC flow - Fix path handling and URL normalization for redirects - Add configuration validation to ensure all OIDC params are provided together - Include "groups" scope for proper group claim support - Re-add accidentally deleted Azure function tests - Add comprehensive test coverage for OIDC functionality Co-Authored-By: Claude <noreply@anthropic.com>
hskiba
force-pushed
the
codex/add-oidc-support-with-group-filtering
branch
from
370c4c3 to
ec6250f
Compare
- Display detailed error messages from OIDC provider in callback - Parse and show OAuth error and error_description parameters - Provide clearer error messages for all authentication failures - Help users understand configuration issues (like missing scopes) Co-Authored-By: Claude <noreply@anthropic.com>
- Conditionally add "groups" scope only if --oidc-groups is specified - Prevents Azure AD errors when groups scope isn't configured - Allows OIDC to work without group filtering when not needed Co-Authored-By: Claude <noreply@anthropic.com>
- Add --oidc-scopes flag to allow custom scope configuration - Default to openid, profile, email (plus groups if --oidc-groups is set) - Allow complete override of scopes when custom scopes are provided - Document the new flag and configuration option Co-Authored-By: Claude <noreply@anthropic.com>
- Add detailed error logging for session operations - Fix session path configuration to ensure proper trailing slash - Log session configuration at startup for debugging - Provide more informative error messages to users Co-Authored-By: Claude <noreply@anthropic.com>
- Store only essential claims (subject, groups, expiry, authenticated flag) instead of full ID token - Reduces session size from 7784 bytes to minimal data needed for authorization - Update AuthMiddleware to use stored claims instead of re-verifying ID token - Maintains same security level while avoiding cookie size limits Co-Authored-By: Claude <noreply@anthropic.com>
- Create UserAwareLogger that includes user subject in request logs - Add user_subject to request context in AuthMiddleware for authenticated requests - Replace default negroni logger with custom user-aware logger - Add comprehensive test coverage for user-aware logging functionality - Logs now show format: "GET /path user=user123 -> 200 OK in 1ms" Co-Authored-By: Claude <noreply@anthropic.com>
- Add clearSessionAndRedirectToLogin helper to handle session errors gracefully - Clear corrupted session cookies instead of showing technical errors - Redirect users to login page when session is invalid - Log technical details server-side while providing clean UX - Fixes "securecookie: the value is not valid" user experience - Fix all golangci-lint issues (constants, context keys, unused params, formatting) Co-Authored-By: Claude <noreply@anthropic.com>
- Document linting workflow using golangci-lint run --fix - OIDC implementation security principles and patterns - Session management best practices - Error handling patterns for user-friendly UX - Logging patterns including user-aware logging - Development workflow and testing requirements - Architecture notes and common code patterns Co-Authored-By: Claude <noreply@anthropic.com>
- Convert AuthMiddleware to negroni-compatible middleware - Reorder middleware: AuthMiddleware now runs before UserAwareLogger - This ensures user context is available when logging authenticated requests - Update tests to work with new AuthMiddleware struct - Now authenticated requests will show: "GET /path user=username -> 200 OK" Co-Authored-By: Claude <noreply@anthropic.com>
- Add --oidc-log-claims flag to specify which claims to include in logs - Default to ["sub"] for backward compatibility - Parse all ID token claims and store configurable subset for logging - Update AuthMiddleware to pass claim map to logging context - Update UserAwareLogger to format multiple claims as key=value pairs - Add comprehensive tests for single and multiple claim scenarios - Update documentation with examples Example log output: - Single: "GET /path user=sub=user123 -> 200 OK" - Multiple: "GET /path user=sub=user123,email=user@example.com,name=John Doe -> 200 OK" Co-Authored-By: Claude <noreply@anthropic.com>
- Convert map[string]interface{} to map[string]string for gob compatibility
- Use fmt.Sprintf to convert claim values to strings before storing in session
- Update AuthMiddleware and UserAwareLogger to handle string maps
- Add comprehensive unit test for gob serialization/deserialization
- Resolves "gob: type not registered for interface: map[string]interface {}" error
- Maintains same logging functionality with session-safe storage
Co-Authored-By: Claude <noreply@anthropic.com>
- Run npx prettier --write on README.md and CLAUDE.md - Add prettier requirement to CLAUDE.md development workflow - Ensure consistent markdown formatting Co-Authored-By: Claude <noreply@anthropic.com>
- Store claims as separate session keys (user_claim_sub, user_claim_email, etc.) instead of map - Reconstruct claims map in AuthMiddleware when needed for logging context - Avoids "gob: type not registered for interface: map[string]string" error - Update comprehensive test to verify session serialization and claim reconstruction - Document session storage best practices in CLAUDE.md This completely resolves session serialization issues with OIDC claims. Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
This PR adds optional OIDC authentication support to River Guide with group-based access control.
Features
--oidc-groupsflagSecurity Improvements (latest commit)
Configuration
All four OIDC parameters must be provided to enable authentication:
--oidc-issuer: OIDC issuer URL--oidc-client-id: OIDC client ID--oidc-client-secret: OIDC client secret--oidc-redirect-url: OIDC redirect URL--oidc-groups: (optional) comma-separated list of allowed groupsTesting
go test ./...)go fmt