-
Notifications
You must be signed in to change notification settings - Fork 3
Closed
Description
🔍 Duplicate Code Pattern: Authorization Header Parsing
Part of duplicate code analysis: #375
Summary
The logic for extracting session IDs from Authorization headers is duplicated across 3 locations in the server package. This creates maintenance burden as auth-related changes must be applied consistently across all locations.
Duplication Details
Pattern: Authorization Header Session Extraction
- Severity: High
- Occurrences: 3 instances
- Locations:
internal/server/routed.go(lines 106-118)internal/server/transport.go(lines 104-114)internal/server/sdk_logging.go(lines 165-170 - helper functionextractSessionID)
Code Sample (repeated 2x inline):
// Extract session ID from Authorization header
// Per spec 7.1: When API key is configured, Authorization contains plain API key
// When API key is not configured, supports Bearer token for backward compatibility
authHeader := r.Header.Get("Authorization")
var sessionID string
if strings.HasPrefix(authHeader, "Bearer ") {
// Bearer token format (for backward compatibility when no API key)
sessionID = strings.TrimPrefix(authHeader, "Bearer ")
sessionID = strings.TrimSpace(sessionID)
} else if authHeader != "" {
// Plain format (per spec 7.1 - API key is session ID)
sessionID = authHeader
}
// Reject requests without Authorization header
if sessionID == "" {
// ... error handling (slightly different per location)
return nil
}Additional Instance: sdk_logging.go already has extracted function extractSessionID() but it's not reused by the other locations.
Impact Analysis
- Maintainability: Any change to auth spec (e.g., supporting new token formats) requires updating 3 locations
- Bug Risk: Inconsistent implementations can lead to auth bypasses or different behavior between routed/unified modes
- Code Bloat: ~20 lines duplicated across 2 locations + 1 helper function not being reused
- Recent Change Impact: PR Added frontend logging and testing #372 added new logging around session handling, making this duplication more visible
Refactoring Recommendations
1. Extract to Shared Auth Utility (Recommended)
- Create or enhance
internal/auth/session.gowith:// ExtractSessionID extracts session ID from Authorization header per spec 7.1 func ExtractSessionID(r *http.Request) (string, error)
- Location:
internal/auth/session.go(new file) - Estimated effort: 1-2 hours
- Benefits:
- Single source of truth for session extraction
- Easier to test auth logic in isolation
- Consistent error handling across all endpoints
- Aligns with existing
internal/auth/header.gopackage structure
2. Reuse Existing Helper
- Move
extractSessionID()fromsdk_logging.gotoauthpackage - Update
routed.goandtransport.goto use the helper - Estimated effort: 30 minutes
- Benefits:
- Quick win - function already exists
- Maintains backward compatibility
3. Add Validation Tests
- Create comprehensive tests for session extraction in
internal/auth/ - Test cases: Bearer tokens, plain API keys, empty headers, malformed headers
- Estimated effort: 1 hour
- Benefits: Ensures refactoring doesn't break auth
Implementation Checklist
- Review current auth implementation in
internal/auth/header.go - Create
internal/auth/session.gowithExtractSessionID()function - Add comprehensive unit tests for session extraction
- Update
routed.goto use shared function (remove inline logic) - Update
transport.goto use shared function (remove inline logic) - Update
sdk_logging.goto use shared function (remove duplicate helper) - Run full test suite to verify no functionality broken
- Update AGENTS.md if auth patterns change
Parent Issue
See parent analysis report: #375
Related to #375
AI generated by Duplicate Code Detector
Reactions are currently unavailable