Support path based matching for authenticators #41
Conversation
moadz
left a comment
moadz
left a comment
There was a problem hiding this comment.
LGTM :) Thanks for this. Excited to ttest.
| for _, pattern := range config.PathPatterns { | ||
| matcher, err := regexp.Compile(pattern) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to compile mTLS path pattern %q: %v", pattern, err) |
There was a problem hiding this comment.
nit: %w for error wrapping, should give detailed regex error to user when it fails to compile (i think) i'm rusty on regex stdlib
| pathMatches := false | ||
| for _, matcher := range a.config.pathMatchers { | ||
| if matcher.MatchString(r.URL.Path) { | ||
| pathMatches = true | ||
| break | ||
| } | ||
| } | ||
|
|
||
| // If path doesn't match, skip mTLS enforcement | ||
| if !pathMatches { | ||
| next.ServeHTTP(w, r) | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
nit/readability: extract into helper (only used once so feel free to ignore 🤷🏽 )
pathMatches(path string)|
|
||
| // Path matches or no paths configured, enforce mTLS | ||
| if r.TLS == nil { | ||
| httperr.PrometheusAPIError(w, "mTLS required but no TLS connection", http.StatusBadRequest) |
There was a problem hiding this comment.
Is this not used outside of prom? Why do we use PrometehusAPIError
There was a problem hiding this comment.
just following the pattern for the rest of the project :)
| } | ||
| } | ||
|
|
||
| // Path matches or no paths configured, enforce mTLS |
| { | ||
| name: "overlapping_patterns", | ||
| yamlConfig: ` | ||
| tenants: |
There was a problem hiding this comment.
Do we need a test case for mTLS with no paths defined? Or is this tested elsewhere?
There was a problem hiding this comment.
e2e tests already exist i think. i just wanted to isolate this clearly for now
| CAs []*x509.Certificate | ||
| RawCA []byte `json:"ca"` | ||
| CAPath string `json:"caPath"` | ||
| PathPatterns []string `json:"pathPatterns"` |
There was a problem hiding this comment.
for later, we should document that this is case sensitive.
2 participants
No description provided.