Skip to content

Commit dd7fd44

Browse files
committed
lower case resource attribute FQN at highest level in GetDecision flow
1 parent bcf4959 commit dd7fd44

File tree

4 files changed

+34
-21
lines changed

4 files changed

+34
-21
lines changed

service/internal/access/v2/evaluate.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,7 @@ func evaluateResourceAttributeValues(
7070
definitionFqnToValueFqns := make(map[string][]string)
7171
definitionsLookup := make(map[string]*policy.Attribute)
7272

73-
for idx, valueFQN := range resourceAttributeValues.GetFqns() {
74-
// lowercase the value FQN to ensure case-insensitive matching
75-
valueFQN = strings.ToLower(valueFQN)
76-
resourceAttributeValues.Fqns[idx] = valueFQN
77-
73+
for _, valueFQN := range resourceAttributeValues.GetFqns() {
7874
attributeAndValue, ok := accessibleAttributeValues[valueFQN]
7975
if !ok {
8076
return nil, fmt.Errorf("%w: %s", ErrFQNNotFound, valueFQN)

service/internal/access/v2/evaluate_test.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -681,21 +681,6 @@ func (s *EvaluateTestSuite) TestEvaluateResourceAttributeValues() {
681681
expectAccessible: true,
682682
expectError: false,
683683
},
684-
{
685-
name: "all rules passing - non lower-cased FQNs",
686-
resourceAttrs: &authz.Resource_AttributeValues{
687-
Fqns: []string{
688-
strings.ToUpper(levelMidFQN),
689-
strings.ToUpper(deptFinanceFQN),
690-
},
691-
},
692-
entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{
693-
levelMidFQN: []*policy.Action{actionRead},
694-
deptFinanceFQN: []*policy.Action{actionRead},
695-
},
696-
expectAccessible: true,
697-
expectError: false,
698-
},
699684
{
700685
name: "one rule failing",
701686
resourceAttrs: &authz.Resource_AttributeValues{

service/internal/access/v2/pdp.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,11 @@ func (p *PolicyDecisionPoint) GetDecision(
163163
return nil, fmt.Errorf("registered resource value FQN not supported: %w", ErrInvalidResource)
164164

165165
case *authz.Resource_AttributeValues_:
166-
for _, valueFQN := range resource.GetAttributeValues().GetFqns() {
166+
for idx, valueFQN := range resource.GetAttributeValues().GetFqns() {
167+
// lowercase each resource attribute value FQN for case consistent map key lookups
167168
valueFQN = strings.ToLower(valueFQN)
169+
resource.GetAttributeValues().Fqns[idx] = valueFQN
170+
168171
// If same value FQN more than once, skip
169172
if _, ok := decisionableAttributes[valueFQN]; ok {
170173
continue

service/internal/access/v2/pdp_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package access
22

33
import (
44
"fmt"
5+
"strings"
56
"testing"
67

78
"github.com/stretchr/testify/suite"
@@ -422,6 +423,34 @@ func (s *PDPTestSuite) Test_GetDecision_MultipleResources() {
422423
}
423424
})
424425

426+
s.Run("Multiple resources and entitled actions/attributes of varied casing - full access", func() {
427+
entity := s.createEntityWithProps("test-user-1", map[string]interface{}{
428+
"clearance": "ts",
429+
"department": "engineering",
430+
})
431+
secretFQN := strings.ToUpper(testClassSecretFQN)
432+
433+
resources := createResourcePerFqn(secretFQN, testDeptEngineeringFQN)
434+
435+
decision, err := pdp.GetDecision(s.T().Context(), entity, testActionRead, resources)
436+
437+
s.Require().NoError(err)
438+
s.Require().NotNil(decision)
439+
s.True(decision.Access)
440+
s.Len(decision.Results, 2)
441+
442+
expectedResults := map[string]bool{
443+
secretFQN: true,
444+
testDeptEngineeringFQN: true,
445+
}
446+
s.assertAllDecisionResults(decision, expectedResults)
447+
for _, result := range decision.Results {
448+
s.True(result.Passed, "All data rules should pass")
449+
s.Len(result.DataRuleResults, 1)
450+
s.Empty(result.DataRuleResults[0].EntitlementFailures)
451+
}
452+
})
453+
425454
s.Run("Multiple resources and unentitled attributes - full denial", func() {
426455
entity := s.createEntityWithProps("test-user-2", map[string]interface{}{
427456
"clearance": "confidential", // Not high enough for update on secret

0 commit comments

Comments
 (0)