refactor: use better ignore paths in context middleware#743
refactor: use better ignore paths in context middleware#743steveiliop56 merged 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughThe context middleware now uses prefix-based path matching to skip processing for multiple endpoints (health checks, OAuth, JWKS) instead of exact-match logic on two OIDC paths. The zerolog middleware updates its skip list to use the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #743 +/- ##
==========================================
- Coverage 18.89% 18.85% -0.04%
==========================================
Files 50 50
Lines 3853 4906 +1053
==========================================
+ Hits 728 925 +197
- Misses 3053 3909 +856
Partials 72 72 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/middleware/context_middleware.go (1)
242-245: Prefer exact-match for static routes and prefix-match only for dynamic ones.
HasPrefixhere can unintentionally ignore similarly named future routes. Consider splitting exact vs prefix rules.♻️ Suggested direction
-var ( - contextSkipPathsPrefix = []string{ +var ( + contextSkipPathsExact = map[string]struct{}{ "GET /api/context/app", "GET /api/healthz", "HEAD /api/healthz", "GET /api/oauth/url", "GET /api/oauth/callback", - "GET /api/oidc/clients", - "POST /api/oidc/token", - "GET /api/oidc/userinfo", - "GET /resources", + "POST /api/oidc/token", + "GET /api/oidc/userinfo", "POST /api/user/login", "GET /.well-known/openid-configuration", "GET /.well-known/jwks.json", + "GET /resources", } + contextSkipPathsPrefix = []string{ + "GET /api/oidc/clients/", + "GET /resources/", + } ) @@ func (m *ContextMiddleware) isIgnorePath(path string) bool { + if _, ok := contextSkipPathsExact[path]; ok { + return true + } for _, prefix := range contextSkipPathsPrefix { if strings.HasPrefix(path, prefix) { return true } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/middleware/context_middleware.go` around lines 242 - 245, The isIgnorePath behavior on ContextMiddleware currently uses strings.HasPrefix against contextSkipPathsPrefix which can accidentally match future static routes; change it to use two rule sets: one for exact matches (e.g., contextSkipPathsExact) checked with path == rule, and one for prefix/dynamic matches (e.g., contextSkipPathsPrefix) checked with strings.HasPrefix; update the ContextMiddleware.isIgnorePath implementation to first check exact matches, then prefix matches, and adjust any references to the old contextSkipPathsPrefix variable names accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/middleware/context_middleware.go`:
- Around line 242-245: The isIgnorePath behavior on ContextMiddleware currently
uses strings.HasPrefix against contextSkipPathsPrefix which can accidentally
match future static routes; change it to use two rule sets: one for exact
matches (e.g., contextSkipPathsExact) checked with path == rule, and one for
prefix/dynamic matches (e.g., contextSkipPathsPrefix) checked with
strings.HasPrefix; update the ContextMiddleware.isIgnorePath implementation to
first check exact matches, then prefix matches, and adjust any references to the
old contextSkipPathsPrefix variable names accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9f88ebe0-703c-4543-a16e-13d7231a678f
📒 Files selected for processing (2)
internal/middleware/context_middleware.gointernal/middleware/zerolog_middleware.go
Summary by CodeRabbit