Merge 71f8349984 into b99601aec4

internal/auth/oidc/service_callback.go

@@ -15,6 +15,7 @@ import (
 	"github.com/hashicorp/boundary/internal/event"
 	"github.com/hashicorp/cap/oidc"
 	"github.com/hashicorp/go-bexpr"
+	"github.com/hashicorp/go-bexpr/grammar"
 	"github.com/mitchellh/pointerstructure"
 	"google.golang.org/protobuf/proto"
 )
@@ -203,8 +204,17 @@ func Callback(
 			"token":    idTkClaims,
 			"userinfo": userInfoClaims,
 		}
+
 		// Iterate through and check claims against filters
 		for _, mg := range mgs {
+
+			ast, err := grammar.Parse("", []byte(mg.Filter))
+			if err != nil {
+				return "", errors.Wrap(ctx, err, op)
+			}
+
+			//normalize the "in" claims
+			normalizeInSyntaxClaims(ast.(grammar.Expression), mg.Filter, evalData)
 			eval, err := bexpr.CreateEvaluator(mg.Filter)
 			if err != nil {
 				// We check all filters on ingress so this should never happen,
@@ -267,3 +277,40 @@ func Callback(
 	// tada!  we can return a final redirect URL for the successful authentication.
 	return reqState.FinalRedirectUrl, nil
 }
+
+// normalizeInSyntaxClaims traverses AST and normalizes string values to []string
+// for any MatchIn/MatchNotIn that uses "in"/"not in" syntax (not "contains")
+func normalizeInSyntaxClaims(expr grammar.Expression, filter string, data map[string]any) {
+	switch e := expr.(type) {
+	case *grammar.MatchExpression:
+		if e.Operator == grammar.MatchIn || e.Operator == grammar.MatchNotIn {
+			searchPath := "/" + strings.Join(e.Selector.Path, "/")
+
+			// Because the AST does not retain original syntax, we need to check the original
+			//  filter string to see if "in"/"not in" syntax was used.
+			// This is to `normalize` only those cases, and not affect "contains" syntax.
+			if usesInSyntax(filter, searchPath, e.Value.Raw) {
+				val, err := pointerstructure.Get(data, searchPath)
+				if err == nil {
+					if str, ok := val.(string); ok {
+						pointerstructure.Set(data, searchPath, []string{str})
+					}
+				}
+			}
+		}
+	case *grammar.BinaryExpression:
+		normalizeInSyntaxClaims(e.Left, filter, data)
+		normalizeInSyntaxClaims(e.Right, filter, data)
+	case *grammar.UnaryExpression:
+		normalizeInSyntaxClaims(e.Operand, filter, data)
+	}
+}
+
+// usesInSyntax checks if the original filter used "in" or "not in" syntax for this path/value
+// We rebuild the patterns to search for in the filter string.
+func usesInSyntax(filter string, selector string, value string) bool {
+	// Looking for both "in" and "not in" exact pattern
+	inPattern := fmt.Sprintf(`"%s" in "%s"`, value, selector)
+	notInPattern := fmt.Sprintf(`"%s" not in "%s"`, value, selector)
+	return strings.Contains(filter, inPattern) || strings.Contains(filter, notInPattern)
+}

internal/auth/oidc/service_callback_test.go

@@ -29,6 +29,7 @@ import (
 	"github.com/hashicorp/boundary/internal/oplog"
 	"github.com/hashicorp/cap/oidc"
 	"github.com/hashicorp/eventlogger/formatter_filters/cloudevents"
+	"github.com/hashicorp/go-bexpr/grammar"
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-secure-stdlib/strutil"
 	"github.com/stretchr/testify/assert"
@@ -874,3 +875,145 @@ func Test_ManagedGroupFiltering(t *testing.T) {
 		})
 	}
 }
+
+func Test_normalizeInSyntaxClaims(t *testing.T) {
+	tests := []struct {
+		name         string
+		filter       string
+		evalData     map[string]any
+		expectedData map[string]any
+	}{
+		{
+			name:   "IN normalizes string to array",
+			filter: `"operator" in "/userinfo/roles"`,
+			evalData: map[string]any{
+				"userinfo": map[string]any{
+					"roles": "operator",
+				},
+			},
+			expectedData: map[string]any{
+				"userinfo": map[string]any{
+					"roles": []string{"operator"},
+				},
+			},
+		},
+		{
+			name:   "CONTAINS does not normalize",
+			filter: `"/userinfo/email" contains "@example.com"`,
+			evalData: map[string]any{
+				"userinfo": map[string]any{
+					"email": "test@example.com",
+				},
+			},
+			expectedData: map[string]any{
+				"userinfo": map[string]any{
+					"email": "test@example.com",
+				},
+			},
+		},
+		{
+			name:   "NOT IN normalizes string to array",
+			filter: `"admin" not in "/userinfo/roles"`,
+			evalData: map[string]any{
+				"userinfo": map[string]any{
+					"roles": "user",
+				},
+			},
+			expectedData: map[string]any{
+				"userinfo": map[string]any{
+					"roles": []string{"user"},
+				},
+			},
+		},
+		{
+			name:   "Token nested path - IN normalizes",
+			filter: `"infosec" in "/token/custom/department"`,
+			evalData: map[string]any{
+				"token": map[string]any{
+					"custom": map[string]any{
+						"department": "infosec",
+					},
+				},
+			},
+			expectedData: map[string]any{
+				"token": map[string]any{
+					"custom": map[string]any{
+						"department": []string{"infosec"},
+					},
+				},
+			},
+		},
+		{
+			name:   "OR with multiple departments - both paths normalized",
+			filter: `"infosec" in "/token/custom/department" or "ops" in "/token/custom/department"`,
+			evalData: map[string]any{
+				"token": map[string]any{
+					"custom": map[string]any{
+						"department": "infosec",
+					},
+				},
+			},
+			expectedData: map[string]any{
+				"token": map[string]any{
+					"custom": map[string]any{
+						"department": []string{"infosec"},
+					},
+				},
+			},
+		},
+		{
+			name:   "NOT wrapping IN - UnaryExpression",
+			filter: `not "banned" in "/userinfo/roles"`,
+			evalData: map[string]any{
+				"userinfo": map[string]any{
+					"roles": "user",
+				},
+			},
+			expectedData: map[string]any{
+				"userinfo": map[string]any{
+					"roles": []string{"user"},
+				},
+			},
+		},
+		{
+			name:   "IN AND CONTAINS - only IN normalized",
+			filter: `"operator" in "/userinfo/roles" and "/userinfo/email" contains "@example.com"`,
+			evalData: map[string]any{
+				"userinfo": map[string]any{
+					"roles": "operator",
+					"email": "testuser@example.com",
+				},
+			},
+			expectedData: map[string]any{
+				"userinfo": map[string]any{
+					"roles": []string{"operator"},
+					"email": "testuser@example.com",
+				},
+			},
+		},
+		{
+			name:   "IN OR CONTAINS same path - IN causes normalization",
+			filter: `"admin" in "/userinfo/roles" or "/userinfo/roles" contains "admin"`,
+			evalData: map[string]any{
+				"userinfo": map[string]any{
+					"roles": "operator",
+				},
+			},
+			expectedData: map[string]any{
+				"userinfo": map[string]any{
+					"roles": []string{"operator"},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert := assert.New(t)
+
+			ast, err := grammar.Parse("", []byte(tt.filter))
+			require.NoError(t, err)
+			normalizeInSyntaxClaims(ast.(grammar.Expression), tt.filter, tt.evalData)
+			assert.Equal(tt.expectedData, tt.evalData)
+		})
+	}
+}