Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions internal/auth/oidc/service_callback.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"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"
)
Expand Down Expand Up @@ -203,8 +204,17 @@
"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,
Expand Down Expand Up @@ -267,3 +277,40 @@
// 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})

Check failure on line 296 in internal/auth/oidc/service_callback.go

View workflow job for this annotation

GitHub Actions / Run Linter

Error return value of `pointerstructure.Set` is not checked (errcheck)

Check failure on line 296 in internal/auth/oidc/service_callback.go

View workflow job for this annotation

GitHub Actions / Run Linter

Error return value of `pointerstructure.Set` is not checked (errcheck)

Check failure on line 296 in internal/auth/oidc/service_callback.go

View workflow job for this annotation

GitHub Actions / Run Linter

Error return value of `pointerstructure.Set` is not checked (errcheck)

Check failure on line 296 in internal/auth/oidc/service_callback.go

View workflow job for this annotation

GitHub Actions / Run Linter

Error return value of `pointerstructure.Set` is not checked (errcheck)
}
}
}
}
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)
}
143 changes: 143 additions & 0 deletions internal/auth/oidc/service_callback_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
})
}
}
Loading