From 71f8349984bc27c0f2eb0741a644e82f70df1d63 Mon Sep 17 00:00:00 2001 From: Bharath Gajjala <120367134+bgajjala8@users.noreply.github.com> Date: Mon, 9 Feb 2026 13:45:15 -0500 Subject: [PATCH] Syntax Normalization --- internal/auth/oidc/service_callback.go | 47 +++++++ internal/auth/oidc/service_callback_test.go | 143 ++++++++++++++++++++ 2 files changed, 190 insertions(+) diff --git a/internal/auth/oidc/service_callback.go b/internal/auth/oidc/service_callback.go index 4921b6a0d3..b7ab6f996d 100644 --- a/internal/auth/oidc/service_callback.go +++ b/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) +} diff --git a/internal/auth/oidc/service_callback_test.go b/internal/auth/oidc/service_callback_test.go index 38a645214f..aa1801311f 100644 --- a/internal/auth/oidc/service_callback_test.go +++ b/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) + }) + } +}