fix: correctly handle fine-grained authorization for the /series endpoint (#810)

Author: JoaoBraveCoding

api/logs/v1/labels_enforcer.go

@@ -22,6 +22,7 @@ type AuthzResponseData struct {
 const (
 	logicalOr  = "or"
 	queryParam = "query"
+	matchParam = "match"
 )
 
 type matchersContextKey struct{}
@@ -70,80 +71,64 @@ func WithEnforceAuthorizationLabels() func(http.Handler) http.Handler {
 func enforceValues(mInfo AuthzResponseData, u *url.URL) (values string, err error) {
 	switch {
 	case strings.HasSuffix(u.Path, "/values"):
-		return enforceValuesOnLabelValues(mInfo, u.Query())
+		return enforceValuesOnLogQL(mInfo, u.Query(), queryParam, false)
+	case strings.HasSuffix(u.Path, "/series"):
+		return enforceValuesOnLogQL(mInfo, u.Query(), matchParam, false)
 	default:
-		return enforceValuesOnQuery(mInfo, u.Query())
+		return enforceValuesOnLogQL(mInfo, u.Query(), queryParam, true)
 	}
 }
 
-func enforceValuesOnLabelValues(mInfo AuthzResponseData, v url.Values) (values string, err error) {
+func enforceValuesOnLogQL(mInfo AuthzResponseData, v url.Values, paramName string, queryEndpoint bool) (values string, err error) {
 	lm, err := initAuthzMatchers(mInfo.Matchers)
 	if err != nil {
 		return "", err
 	}
 
-	var expr logqlv2.Expr
-
-	if q := v.Get(queryParam); q != "" {
-		expr, err = logqlv2.ParseExpr(q)
-		if err != nil {
-			return "", fmt.Errorf("failed parsing LogQL expression: %w", err)
+	paramValue := v.Get(paramName)
+	if paramValue == "" {
+		// For query endpoints, we don't always to enforce the authorization
+		// label matchers, so weskip it if the query is empty.
+		if queryEndpoint {
+			return v.Encode(), nil
 		}
 
-		if le, ok := expr.(*logqlv2.LogQueryExpr); ok {
-			matchers := combineLabelMatchers(le.Matchers(), lm)
-			le.SetMatchers(matchers)
-		}
-	} else {
-		expr = &logqlv2.StreamMatcherExpr{}
-		expr.(*logqlv2.StreamMatcherExpr).SetMatchers(lm)
-	}
-
-	v.Set(queryParam, expr.String())
-
-	return v.Encode(), nil
-}
-
-func enforceValuesOnQuery(mInfo AuthzResponseData, v url.Values) (values string, err error) {
-	if v.Get(queryParam) == "" {
+		// For the other endpoints we want to enforce the authZ label matchers
+		expr := &logqlv2.StreamMatcherExpr{}
+		expr.SetMatchers(lm)
+		v.Set(paramName, expr.String())
 		return v.Encode(), nil
 	}
 
-	lm, err := initAuthzMatchers(mInfo.Matchers)
-	if err != nil {
-		return "", err
-	}
-
-	expr, err := logqlv2.ParseExpr(v.Get(queryParam))
+	expr, err := logqlv2.ParseExpr(paramValue)
 	if err != nil {
 		return "", fmt.Errorf("failed parsing LogQL expression: %w", err)
 	}
 
-	switch mInfo.MatcherOp {
-	case logicalOr:
+	// Logical "OR" only applies to query expressions, not for filter params
+	if mInfo.MatcherOp == logicalOr && paramValue == queryParam {
 		// Logical "OR" to combine multiple matchers needs to be done via LogQueryExpr > LogPipelineExpr
 		expr.Walk(func(expr interface{}) {
-			switch le := expr.(type) {
-			case *logqlv2.LogQueryExpr:
+			if le, ok := expr.(*logqlv2.LogQueryExpr); ok {
 				le.AppendPipelineMatchers(mInfo.Matchers, logicalOr)
-			default:
-				// Do nothing
-			}
-		})
-	default:
-		expr.Walk(func(expr interface{}) {
-			switch le := expr.(type) {
-			case *logqlv2.StreamMatcherExpr:
-				matchers := combineLabelMatchers(le.Matchers(), lm)
-				le.SetMatchers(matchers)
-			default:
-				// Do nothing
 			}
 		})
+		v.Set(paramName, expr.String())
+		return v.Encode(), nil
 	}
 
-	v.Set(queryParam, expr.String())
+	expr.Walk(func(expr interface{}) {
+		switch le := expr.(type) {
+		case *logqlv2.LogQueryExpr:
+			matchers := combineLabelMatchers(le.Matchers(), lm)
+			le.SetMatchers(matchers)
+		case *logqlv2.StreamMatcherExpr:
+			matchers := combineLabelMatchers(le.Matchers(), lm)
+			le.SetMatchers(matchers)
+		}
+	})
 
+	v.Set(paramName, expr.String())
 	return v.Encode(), nil
 }
 

authorization/http.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"strings"
 
 	"github.com/go-kit/log"
 	"github.com/go-kit/log/level"
@@ -83,7 +84,19 @@ func WithLogsStreamSelectorsExtractor(logger log.Logger, selectorNames []string)
 				return
 			}
 
-			selectorsInfo, err := extractLogStreamSelectors(selectorNameMap, r.URL.Query())
+			var (
+				selectorsInfo *SelectorsInfo
+				err           error
+			)
+
+			switch {
+			case strings.HasSuffix(r.URL.Path, "/rules"):
+				selectorsInfo = extractLogRulesSelectors(selectorNameMap, r.URL.Query())
+			case strings.HasSuffix(r.URL.Path, "/series"):
+				selectorsInfo, err = extractLogStreamSelectors(selectorNameMap, r.URL.Query(), "match")
+			default:
+				selectorsInfo, err = extractLogStreamSelectors(selectorNameMap, r.URL.Query(), "query")
+			}
 			if err != nil {
 				// Don't error out, just warn about error and continue with empty selectorsInfo
 				level.Warn(logger).Log("msg", err)

authorization/query.go

@@ -9,20 +9,12 @@ import (
 	"github.com/prometheus/prometheus/model/labels"
 )
 
-func extractLogStreamSelectors(selectorNames map[string]bool, values url.Values) (*SelectorsInfo, error) {
-	query := values.Get("query")
-	if query == "" {
-		// If query is empty we will assume it's a possibly a rules request
-		selectors := parseLogRulesSelectors(selectorNames, values)
+func extractLogStreamSelectors(selectorNames map[string]bool, values url.Values, param string) (*SelectorsInfo, error) {
+	value := values.Get(param)
 
-		return &SelectorsInfo{
-			Selectors: selectors,
-		}, nil
-	}
-
-	selectors, hasWildcard, err := parseLogStreamSelectors(selectorNames, query)
+	selectors, hasWildcard, err := parseLogStreamSelectors(selectorNames, value)
 	if err != nil {
-		return nil, fmt.Errorf("error extracting selectors from query %#q: %w", query, err)
+		return nil, fmt.Errorf("error extracting selectors from %s %#q: %w", param, value, err)
 	}
 
 	return &SelectorsInfo{

authorization/rules.go

@@ -4,7 +4,13 @@ import (
 	"net/url"
 )
 
-func parseLogRulesSelectors(selectorNames map[string]bool, queryParameter url.Values) map[string][]string {
+func extractLogRulesSelectors(selectorNames map[string]bool, values url.Values) *SelectorsInfo {
+	return &SelectorsInfo{
+		Selectors: parseLogRulesSelectors(selectorNames, values),
+	}
+}
+
+func parseLogRulesSelectors(selectorNames map[string]bool, values url.Values) map[string][]string {
 	selectors := make(map[string][]string)
 	appendSelector := func(selector, value string) {
 		values, ok := selectors[selector]
@@ -17,7 +23,7 @@ func parseLogRulesSelectors(selectorNames map[string]bool, queryParameter url.Va
 	}
 
 	for selector := range selectorNames {
-		values := queryParameter[selector]
+		values := values[selector]
 		for _, value := range values {
 			appendSelector(selector, value)
 		}