-
Notifications
You must be signed in to change notification settings - Fork 3
Support multiple authenticators with path matching #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d96d7ac
1e32b0c
06979c6
50f0e31
15178ad
b426d9d
79eeca0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,3 +23,4 @@ vendor | |
| e2e_* | ||
|
|
||
| .buildxcache/ | ||
| .claude/* | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,8 +48,8 @@ type tenantHandlers map[string]http.Handler | |
| type ProviderManager struct { | ||
| mtx sync.RWMutex | ||
| patternHandlers map[string]tenantHandlers | ||
| middlewares map[string]Middleware | ||
| gRPCInterceptors map[string]grpc.StreamServerInterceptor | ||
| middlewares map[string][]Middleware | ||
| gRPCInterceptors map[string][]grpc.StreamServerInterceptor | ||
| logger log.Logger | ||
| registrationRetryCount *prometheus.CounterVec | ||
| } | ||
|
|
@@ -59,8 +59,8 @@ func NewProviderManager(l log.Logger, registrationRetryCount *prometheus.Counter | |
| return &ProviderManager{ | ||
| registrationRetryCount: registrationRetryCount, | ||
| patternHandlers: make(map[string]tenantHandlers), | ||
| middlewares: make(map[string]Middleware), | ||
| gRPCInterceptors: make(map[string]grpc.StreamServerInterceptor), | ||
| middlewares: make(map[string][]Middleware), | ||
| gRPCInterceptors: make(map[string][]grpc.StreamServerInterceptor), | ||
| logger: l, | ||
| } | ||
| } | ||
|
|
@@ -88,8 +88,8 @@ func (pm *ProviderManager) InitializeProvider(config map[string]interface{}, | |
| } | ||
|
|
||
| pm.mtx.Lock() | ||
| pm.middlewares[tenant] = provider.Middleware() | ||
| pm.gRPCInterceptors[tenant] = provider.GRPCMiddleware() | ||
| pm.middlewares[tenant] = append(pm.middlewares[tenant], provider.Middleware()) | ||
| pm.gRPCInterceptors[tenant] = append(pm.gRPCInterceptors[tenant], provider.GRPCMiddleware()) | ||
| pattern, handler := provider.Handler() | ||
| if pattern != "" && handler != nil { | ||
| if pm.patternHandlers[pattern] == nil { | ||
|
|
@@ -109,19 +109,45 @@ func (pm *ProviderManager) InitializeProvider(config map[string]interface{}, | |
| // Middleware returns an authentication middleware for a tenant. | ||
| func (pm *ProviderManager) Middlewares(tenant string) (Middleware, bool) { | ||
| pm.mtx.RLock() | ||
| mw, ok := pm.middlewares[tenant] | ||
| mws, ok := pm.middlewares[tenant] | ||
| pm.mtx.RUnlock() | ||
|
|
||
| return mw, ok | ||
| if !ok || len(mws) == 0 { | ||
| return nil, false | ||
| } | ||
|
|
||
| // If only one middleware, return it directly | ||
| if len(mws) == 1 { | ||
| return mws[0], true | ||
| } | ||
|
|
||
| // Chain all middlewares together - each will check its own PathPatterns | ||
| // and either authenticate or pass through to the next middleware | ||
| return func(next http.Handler) http.Handler { | ||
| handler := next | ||
| for i := len(mws) - 1; i >= 0; i-- { | ||
| handler = mws[i](handler) | ||
| } | ||
| return handler | ||
| }, true | ||
| } | ||
|
|
||
| // GRPCMiddlewares returns an authentication interceptor for a tenant. | ||
| func (pm *ProviderManager) GRPCMiddlewares(tenant string) (grpc.StreamServerInterceptor, bool) { | ||
| pm.mtx.RLock() | ||
| mw, ok := pm.gRPCInterceptors[tenant] | ||
| interceptors, ok := pm.gRPCInterceptors[tenant] | ||
| pm.mtx.RUnlock() | ||
|
|
||
| return mw, ok | ||
| if !ok || len(interceptors) == 0 { | ||
| return nil, false | ||
| } | ||
|
|
||
| // If only one interceptor, return it directly | ||
| if len(interceptors) == 1 { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this conditional needed? We
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it was just for consistency since this doesnt really work here right now and to keep the logic the same, but happy to adapt |
||
| return interceptors[0], true | ||
| } | ||
|
|
||
| return interceptors[0], true | ||
| } | ||
|
|
||
| // PatternHandler return an http.HandlerFunc for a corresponding pattern. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import ( | |
| "context" | ||
| "fmt" | ||
| "net/http" | ||
| "regexp" | ||
| "strings" | ||
|
|
||
| "github.com/go-chi/chi/v5" | ||
|
|
@@ -31,6 +32,8 @@ const ( | |
| tenantKey contextKey = "tenant" | ||
| // tenantIDKey is the key that holds the tenant ID in a request context. | ||
| tenantIDKey contextKey = "tenantID" | ||
| // authenticatedKey is the key that indicates a request has been successfully authenticated. | ||
| authenticatedKey contextKey = "authenticated" | ||
| ) | ||
|
|
||
| // WithTenant finds the tenant from the URL parameters and adds it to the request context. | ||
|
|
@@ -131,6 +134,18 @@ func GetAccessToken(ctx context.Context) (string, bool) { | |
| return token, ok | ||
| } | ||
|
|
||
| // SetAuthenticated marks the request as successfully authenticated. | ||
| func SetAuthenticated(ctx context.Context) context.Context { | ||
| return context.WithValue(ctx, authenticatedKey, true) | ||
| } | ||
|
|
||
| // IsAuthenticated checks if the request has been successfully authenticated. | ||
| func IsAuthenticated(ctx context.Context) bool { | ||
| value := ctx.Value(authenticatedKey) | ||
| authenticated, ok := value.(bool) | ||
| return ok && authenticated | ||
| } | ||
|
|
||
| // Middleware is a convenience type for functions that wrap http.Handlers. | ||
| type Middleware func(http.Handler) http.Handler | ||
|
|
||
|
|
@@ -161,34 +176,35 @@ func WithTenantMiddlewares(mwFns ...MiddlewareFunc) Middleware { | |
| } | ||
| } | ||
|
|
||
| // EnforceAccessTokenPresentOnSignalWrite enforces that the Authorization header is present in the incoming request | ||
| // for the given list of tenants. Otherwise, it returns an error. | ||
| // It protects the Prometheus remote write and Loki push endpoints. The tracing endpoint is not protected because | ||
| // it goes through the gRPC middleware stack, which behaves differently from the HTTP one. | ||
| func EnforceAccessTokenPresentOnSignalWrite(oidcTenants map[string]struct{}) func(http.Handler) http.Handler { | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i really dont understand why this was here. it was enforcing oidc on all write requests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose it ignores it further down, but good refactor. Your implementation is clearer. |
||
| // EnforceAuthentication is a final middleware that ensures at least one authenticator | ||
| // has successfully validated the request. This prevents security gaps where requests | ||
| // might bypass all authentication checks. | ||
| func EnforceAuthentication() Middleware { | ||
| return func(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| tenant := chi.URLParam(r, "tenant") | ||
|
|
||
| // If there's no tenant, we're not interested in blocking this request. | ||
| if tenant == "" { | ||
| next.ServeHTTP(w, r) | ||
| if !IsAuthenticated(r.Context()) { | ||
| httperr.PrometheusAPIError(w, "request not authenticated by any provider", http.StatusUnauthorized) | ||
| return | ||
| } | ||
|
|
||
| // And we aren't interested in blocking requests from tenants not using OIDC. | ||
| if _, found := oidcTenants[tenant]; !found { | ||
| next.ServeHTTP(w, r) | ||
| return | ||
| } | ||
|
|
||
| rawToken := r.Header.Get("Authorization") | ||
| if rawToken == "" { | ||
| httperr.PrometheusAPIError(w, "couldn't find the authorization header", http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
||
| next.ServeHTTP(w, r) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Path pattern matching operators. | ||
| const ( | ||
| OperatorMatches = "=~" // Pattern must match | ||
| OperatorNotMatches = "!~" // Pattern must NOT match | ||
| ) | ||
|
|
||
| // PathPattern represents a path pattern with an operator for matching. | ||
| type PathPattern struct { | ||
| Operator string `json:"operator,omitempty"` | ||
| Pattern string `json:"pattern"` | ||
| } | ||
|
|
||
| // PathMatcher represents a compiled path pattern with operator. | ||
| type PathMatcher struct { | ||
| Operator string | ||
| Regex *regexp.Regexp | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ import ( | |
| "regexp" | ||
|
|
||
| "github.com/go-kit/log" | ||
| "github.com/go-kit/log/level" | ||
| grpc_middleware_auth "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/auth" | ||
| "github.com/mitchellh/mapstructure" | ||
| "github.com/prometheus/client_golang/prometheus" | ||
|
|
@@ -29,11 +30,11 @@ func init() { | |
| } | ||
|
|
||
| type mTLSConfig struct { | ||
| RawCA []byte `json:"ca"` | ||
| CAPath string `json:"caPath"` | ||
| PathPatterns []string `json:"pathPatterns"` | ||
| RawCA []byte `json:"ca"` | ||
| CAPath string `json:"caPath"` | ||
| Paths []PathPattern `json:"paths,omitempty"` | ||
| CAs []*x509.Certificate | ||
| pathMatchers []*regexp.Regexp | ||
| pathMatchers []PathMatcher | ||
| } | ||
|
|
||
| type MTLSAuthenticator struct { | ||
|
|
@@ -86,13 +87,25 @@ func newMTLSAuthenticator(c map[string]interface{}, tenant string, registrationR | |
| config.CAs = cas | ||
| } | ||
|
|
||
| // Compile path patterns | ||
| for _, pattern := range config.PathPatterns { | ||
| matcher, err := regexp.Compile(pattern) | ||
| for _, pathPattern := range config.Paths { | ||
| operator := pathPattern.Operator | ||
| if operator == "" { | ||
| operator = OperatorMatches | ||
| } | ||
|
Comment on lines
+92
to
+94
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't got to that part of the PR, but we should document that the default behaviour flag docs is |
||
|
|
||
| if operator != OperatorMatches && operator != OperatorNotMatches { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if having both isn't redundant. I.e. if a user specifies
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was my first iteration, but because go does not support negative lookaheads and only RE2 it is going to make configuration extremely complicate to avoid overlpas if one so wishes. For example, take a look at oidc:
clientID: observatorium-api
clientSecret: ZXhhbXBsZS1hcHAtc2VjcmV0
issuerURL: http://dex.proxy.svc.cluster.local:5556/dex
redirectURL: http://localhost:8080/oidc/auth-tenant/callback
usernameClaim: email
paths:
- operator: "!~"
pattern: ".*(loki/api/v1/push|api/v1/receive).*"
mTLS:
caPath: /etc/certs/ca.crt
paths:
- operator: "=~"
pattern: ".*(api/v1/receive).*"
- operator: "=~"
pattern: ".*(loki/api/v1/push).*"and imagine the alternative where the operator isnt available. I would have preferred the original too for simplicity but can confidently say having iterated through the test environment to try to achieve the outcome above, this is much better and leads to much simpler runtime config for end user. |
||
| return nil, fmt.Errorf("invalid mTLS path operator %q, must be %q or %q", operator, OperatorMatches, OperatorNotMatches) | ||
| } | ||
|
|
||
| matcher, err := regexp.Compile(pathPattern.Pattern) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to compile mTLS path pattern %q: %v", pattern, err) | ||
| return nil, fmt.Errorf("failed to compile mTLS path pattern %q: %v", pathPattern.Pattern, err) | ||
| } | ||
| config.pathMatchers = append(config.pathMatchers, matcher) | ||
|
|
||
| config.pathMatchers = append(config.pathMatchers, PathMatcher{ | ||
| Operator: operator, | ||
| Regex: matcher, | ||
| }) | ||
| } | ||
|
|
||
| return MTLSAuthenticator{ | ||
|
|
@@ -105,24 +118,37 @@ func newMTLSAuthenticator(c map[string]interface{}, tenant string, registrationR | |
| func (a MTLSAuthenticator) Middleware() Middleware { | ||
| return func(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| level.Debug(a.logger).Log("msg", "mTLS middleware processing", "path", r.URL.Path, "tenant", a.tenant, "numPatterns", len(a.config.pathMatchers)) | ||
|
|
||
| // Check if mTLS is required for this path | ||
| if len(a.config.pathMatchers) > 0 { | ||
| pathMatches := false | ||
| shouldEnforceMTLS := false | ||
|
|
||
| for _, matcher := range a.config.pathMatchers { | ||
| if matcher.MatchString(r.URL.Path) { | ||
| pathMatches = true | ||
| regexMatches := matcher.Regex.MatchString(r.URL.Path) | ||
| level.Debug(a.logger).Log("msg", "mTLS path pattern check", "path", r.URL.Path, "operator", matcher.Operator, "pattern", matcher.Regex.String(), "matches", regexMatches) | ||
|
|
||
| if matcher.Operator == OperatorMatches && regexMatches { | ||
| level.Debug(a.logger).Log("msg", "mTLS positive match - enforcing", "path", r.URL.Path) | ||
| shouldEnforceMTLS = true | ||
| break | ||
| } else if matcher.Operator == OperatorNotMatches && !regexMatches { | ||
| // Negative match - enforce mTLS (path does NOT match pattern) | ||
| level.Debug(a.logger).Log("msg", "mTLS negative match - enforcing", "path", r.URL.Path) | ||
| shouldEnforceMTLS = true | ||
| break | ||
| } | ||
| } | ||
|
|
||
| // If path doesn't match, skip mTLS enforcement | ||
| if !pathMatches { | ||
| level.Debug(a.logger).Log("msg", "mTLS enforcement decision", "path", r.URL.Path, "shouldEnforceMTLS", shouldEnforceMTLS) | ||
| if !shouldEnforceMTLS { | ||
| level.Debug(a.logger).Log("msg", "mTLS skipping enforcement", "path", r.URL.Path) | ||
| next.ServeHTTP(w, r) | ||
| return | ||
| } | ||
| } | ||
|
|
||
| // Path matches or no paths configured, enforce mTLS | ||
| level.Debug(a.logger).Log("msg", "mTLS enforcing authentication", "path", r.URL.Path, "tenant", a.tenant) | ||
| if r.TLS == nil { | ||
| httperr.PrometheusAPIError(w, "mTLS required but no TLS connection", http.StatusBadRequest) | ||
| return | ||
|
|
@@ -174,10 +200,11 @@ func (a MTLSAuthenticator) Middleware() Middleware { | |
| return | ||
| } | ||
| ctx := context.WithValue(r.Context(), subjectKey, sub) | ||
|
|
||
| // Add organizational units as groups. | ||
| ctx = context.WithValue(ctx, groupsKey, r.TLS.PeerCertificates[0].Subject.OrganizationalUnit) | ||
|
|
||
| // Mark request as successfully authenticated | ||
| ctx = SetAuthenticated(ctx) | ||
| next.ServeHTTP(w, r.WithContext(ctx)) | ||
| }) | ||
| } | ||
|
|
@@ -192,4 +219,3 @@ func (a MTLSAuthenticator) GRPCMiddleware() grpc.StreamServerInterceptor { | |
| func (a MTLSAuthenticator) Handler() (string, http.Handler) { | ||
| return "", nil | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😗👌🏽