Skip to content

Commit 443cedb

Browse files
alkalescentalkalescent
authored
feat(authz): RR GetDecision improvements (#2479)
### Proposed Changes * ### Checklist - [ ] I have added or updated unit tests - [ ] I have added or updated integration tests (if appropriate) - [ ] I have added or updated documentation ### Testing Instructions
1 parent b3093cc commit 443cedb

File tree

7 files changed

+241
-30
lines changed

7 files changed

+241
-30
lines changed

‎lib/identifier/policyidentifier.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"errors"
55
"fmt"
66
"regexp"
7+
"strings"
78
)
89

910
var (
@@ -49,7 +50,7 @@ func Parse[T FullyQualified](identifier string) (T, error) {
4950
parsed any
5051
err error
5152
)
52-
53+
identifier = strings.ToLower(identifier)
5354
// Use type assertion to determine the concrete type and call the appropriate parser
5455
switch any(result).(type) {
5556
case *FullyQualifiedAttribute:

‎service/internal/access/v2/evaluate.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,6 @@ func getResourceDecision(
6262
Fqns: make([]string, 0),
6363
}
6464
for _, aav := range regResValue.GetActionAttributeValues() {
65-
// TODO: DSPX-1295 - revisit this logic -- reg res' are different from attr values since they can be both entity and resource
66-
// and are tied to actions and attribute values
67-
//
68-
// if aav.GetAction().GetName() != action.GetName() {
69-
// continue
70-
// }
71-
7265
aavAttrValueFQN := aav.GetAttributeValue().GetFqn()
7366
if !slices.Contains(resourceAttributeValues.GetFqns(), aavAttrValueFQN) {
7467
resourceAttributeValues.Fqns = append(resourceAttributeValues.Fqns, aavAttrValueFQN)

‎service/internal/access/v2/evaluate_test.go‎

Lines changed: 143 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ var (
3939
projectAvengersFQN = createAttrValueFQN(baseNamespace, "project", "avengers")
4040
projectXmenFQN = createAttrValueFQN(baseNamespace, "project", "xmen")
4141
projectFantasicFourFQN = createAttrValueFQN(baseNamespace, "project", "fantasticfour")
42+
43+
// Registered resource values
44+
netRegResValFQN = createRegisteredResourceValueFQN("network", "external")
45+
platRegResValFQN = createRegisteredResourceValueFQN("platform", "internal")
4246
)
4347

4448
var (
@@ -155,8 +159,53 @@ func (s *EvaluateTestSuite) SetupTest() {
155159
}
156160

157161
// Setup accessible registered resource values map
158-
// TODO: DSPX-1295
159-
s.accessibleRegisteredResourceValues = map[string]*policy.RegisteredResourceValue{}
162+
// Create the registered resource values with action attribute values
163+
s.accessibleRegisteredResourceValues = map[string]*policy.RegisteredResourceValue{
164+
netRegResValFQN: {
165+
Id: "network-registered-res-id",
166+
Value: "external",
167+
ActionAttributeValues: []*policy.RegisteredResourceValue_ActionAttributeValue{
168+
{
169+
Id: "network-action-attr-val-1",
170+
Action: actionRead,
171+
AttributeValue: &policy.Value{
172+
Fqn: levelHighestFQN,
173+
Value: "highest",
174+
},
175+
},
176+
{
177+
Id: "network-action-attr-val-2",
178+
Action: actionCreate,
179+
AttributeValue: &policy.Value{
180+
Fqn: levelMidFQN,
181+
Value: "mid",
182+
},
183+
},
184+
},
185+
},
186+
platRegResValFQN: {
187+
Id: "platform-registered-res-id",
188+
Value: "internal",
189+
ActionAttributeValues: []*policy.RegisteredResourceValue_ActionAttributeValue{
190+
{
191+
Id: "platform-action-attr-val-1",
192+
Action: actionRead,
193+
AttributeValue: &policy.Value{
194+
Fqn: projectAvengersFQN,
195+
Value: "avengers",
196+
},
197+
},
198+
{
199+
Id: "platform-action-attr-val-2",
200+
Action: actionRead,
201+
AttributeValue: &policy.Value{
202+
Fqn: projectJusticeLeagueFQN,
203+
Value: "justiceleague",
204+
},
205+
},
206+
},
207+
},
208+
}
160209
}
161210

162211
func TestEvaluateSuite(t *testing.T) {
@@ -751,11 +800,14 @@ func (s *EvaluateTestSuite) TestEvaluateResourceAttributeValues() {
751800

752801
// Test cases for getResourceDecision
753802
func (s *EvaluateTestSuite) TestGetResourceDecision() {
803+
nonExistentRegResValueFQN := createRegisteredResourceValueFQN("nonexistent", "value")
804+
754805
tests := []struct {
755806
name string
756807
resource *authz.Resource
757808
entitlements subjectmappingbuiltin.AttributeValueFQNsToActions
758809
expectError bool
810+
expectPass bool
759811
}{
760812
{
761813
name: "attribute values resource",
@@ -765,18 +817,105 @@ func (s *EvaluateTestSuite) TestGetResourceDecision() {
765817
Fqns: []string{levelMidFQN},
766818
},
767819
},
820+
EphemeralId: "test-attr-values-id-1",
768821
},
769822
entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{
770823
levelMidFQN: []*policy.Action{actionRead},
771824
},
772825
expectError: false,
826+
expectPass: true,
827+
},
828+
{
829+
name: "registered resource value with all entitlements",
830+
resource: &authz.Resource{
831+
Resource: &authz.Resource_RegisteredResourceValueFqn{
832+
RegisteredResourceValueFqn: netRegResValFQN,
833+
},
834+
EphemeralId: "test-reg-res-id-1",
835+
},
836+
entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{
837+
levelHighestFQN: []*policy.Action{actionRead},
838+
},
839+
expectError: false,
840+
expectPass: true,
841+
},
842+
{
843+
name: "registered resource value with project values",
844+
resource: &authz.Resource{
845+
Resource: &authz.Resource_RegisteredResourceValueFqn{
846+
RegisteredResourceValueFqn: platRegResValFQN,
847+
},
848+
EphemeralId: "test-reg-res-id-2",
849+
},
850+
entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{
851+
projectAvengersFQN: []*policy.Action{actionRead},
852+
projectJusticeLeagueFQN: []*policy.Action{actionRead},
853+
},
854+
expectError: false,
855+
expectPass: true,
856+
},
857+
{
858+
name: "registered resource value with missing entitlements",
859+
resource: &authz.Resource{
860+
Resource: &authz.Resource_RegisteredResourceValueFqn{
861+
RegisteredResourceValueFqn: platRegResValFQN,
862+
},
863+
EphemeralId: "test-reg-res-id-3",
864+
},
865+
entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{
866+
// Missing projectJusticeLeagueFQN
867+
projectAvengersFQN: []*policy.Action{actionRead},
868+
},
869+
expectError: false,
870+
expectPass: false, // Missing entitlement for projectJusticeLeagueFQN
871+
},
872+
{
873+
name: "registered resource value with wrong action",
874+
resource: &authz.Resource{
875+
Resource: &authz.Resource_RegisteredResourceValueFqn{
876+
RegisteredResourceValueFqn: netRegResValFQN,
877+
},
878+
EphemeralId: "test-reg-res-id-4",
879+
},
880+
entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{
881+
// Wrong action
882+
levelHighestFQN: []*policy.Action{actionCreate},
883+
},
884+
expectError: false,
885+
expectPass: false,
886+
},
887+
{
888+
name: "nonexistent registered resource value",
889+
resource: &authz.Resource{
890+
Resource: &authz.Resource_RegisteredResourceValueFqn{
891+
RegisteredResourceValueFqn: nonExistentRegResValueFQN,
892+
},
893+
EphemeralId: "test-reg-res-id-5",
894+
},
895+
entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{},
896+
expectError: true,
897+
expectPass: false,
773898
},
774899
{
775900
name: "invalid nil resource",
776901
resource: nil,
777902
entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{},
778903
expectError: true,
779904
},
905+
{
906+
name: "case insensitive registered resource value FQN",
907+
resource: &authz.Resource{
908+
Resource: &authz.Resource_RegisteredResourceValueFqn{
909+
RegisteredResourceValueFqn: strings.ToUpper(netRegResValFQN), // Test case insensitivity
910+
},
911+
EphemeralId: "test-reg-res-id-6",
912+
},
913+
entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{
914+
levelHighestFQN: []*policy.Action{actionRead},
915+
},
916+
expectError: false,
917+
expectPass: true,
918+
},
780919
}
781920

782921
for _, tc := range tests {
@@ -796,6 +935,8 @@ func (s *EvaluateTestSuite) TestGetResourceDecision() {
796935
} else {
797936
s.Require().NoError(err)
798937
s.NotNil(decision)
938+
s.Equal(tc.expectPass, decision.Passed, "Decision pass status didn't match")
939+
s.Equal(tc.resource.GetEphemeralId(), decision.ResourceID, "Resource ID didn't match")
799940
}
800941
})
801942
}

‎service/internal/access/v2/helpers.go‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -218,14 +218,6 @@ func getResourceDecisionableAttributes(
218218
}
219219

220220
for _, aav := range regResValue.GetActionAttributeValues() {
221-
// TODO: DSPX-1295 - revisit this logic bc it is causing failures for attributes with missing actions
222-
// slog.Info("processing action attribute value", slog.Any("aav", aav))
223-
// aavAction := aav.GetAction()
224-
// if aavAction.GetName() != action.GetName() {
225-
// logger.DebugContext(ctx, "skipping action not matching Decision Request action", slog.String("action", aavAction.GetName()))
226-
// continue
227-
// }
228-
229221
attrValueFQNs = append(attrValueFQNs, aav.GetAttributeValue().GetFqn())
230222
}
231223

‎service/internal/access/v2/pdp_test.go‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ var (
8787
testPlatformOnPremFQN = createAttrValueFQN(testSecondaryNamespace, "platform", "onprem")
8888
testPlatformHybridFQN = createAttrValueFQN(testSecondaryNamespace, "platform", "hybrid")
8989

90-
// Registered resource value FQNs (TODO: DSPX-1295 - remove)
90+
// Registered resource value FQNs (TODO: remove)
9191
testNetworkPrivateFQN = createRegisteredResourceValueFQN("network", "private")
9292
testNetworkPublicFQN = createRegisteredResourceValueFQN("network", "public")
9393
// testNetworkConfidentialFQN = createRegisteredResourceValueFQN("network", "confidential")
@@ -97,7 +97,6 @@ var (
9797
// registered resource value FQNs using identifier package
9898
var (
9999
// Classification values
100-
// testClassTopSecretRegResFQN = createRegisteredResourceValueFQN("classification", "topsecret")
101100
testClassSecretRegResFQN = createRegisteredResourceValueFQN("classification", "secret")
102101
testClassConfidentialRegResFQN = createRegisteredResourceValueFQN("classification", "confidential")
103102
// testClassPublicRegResFQN = createRegisteredResourceValueFQN("classification", "public")
@@ -124,7 +123,7 @@ var (
124123
)
125124

126125
// Registered resource value FQNs using identifier package
127-
// TODO: DSPX-1295 - remove these and use the other ones above
126+
// TODO: remove these and use the other ones above
128127
var (
129128
regResValNoActionAttrValFQN string
130129
regResValSingleActionAttrValFQN string
@@ -175,12 +174,12 @@ type PDPTestSuite struct {
175174
// Test registered resources
176175
classificationRegRes *policy.RegisteredResource
177176
deptRegRes *policy.RegisteredResource
178-
networkRegRes *policy.RegisteredResource // TODO: DSPX-1295 - remove this and use the others that match test attributes
177+
networkRegRes *policy.RegisteredResource // TODO: remove this and use the others that match test attributes
179178
countryRegRes *policy.RegisteredResource
180179
projectRegRes *policy.RegisteredResource
181180
platformRegRes *policy.RegisteredResource
182181

183-
// Test registered resources (TODO: DSPX-1295 - remove these and use the ones above)
182+
// Test registered resources (TODO: remove these and use the ones above)
184183
regRes *policy.RegisteredResource
185184
regResValNoActionAttrVal *policy.RegisteredResourceValue
186185
regResValSingleActionAttrVal *policy.RegisteredResourceValue
@@ -698,7 +697,7 @@ func (s *PDPTestSuite) SetupTest() {
698697
},
699698
}
700699

701-
// Initialize test registered resources (TODO: DSPX-1295: replace with above real use cases)
700+
// Initialize test registered resources (TODO: replace with above real use cases)
702701
regResValNoActionAttrVal := &policy.RegisteredResourceValue{
703702
Value: "no-action-attr-val",
704703
ActionAttributeValues: []*policy.RegisteredResourceValue_ActionAttributeValue{},
@@ -2259,6 +2258,18 @@ func (s *PDPTestSuite) Test_GetDecisionRegisteredResource_MultipleResources() {
22592258
})
22602259
}
22612260

2261+
func (s *PDPTestSuite) Test_GetDecisionRegisteredResource_PartialActionEntitlement() {
2262+
s.T().Skip("TODO")
2263+
}
2264+
2265+
func (s *PDPTestSuite) Test_GetDecisionRegisteredResource_CombinedAttributeRules_SingleResource() {
2266+
s.T().Skip("TODO")
2267+
}
2268+
2269+
func (s *PDPTestSuite) Test_GetDecisionRegisteredResource_AcrossNamespaces() {
2270+
s.T().Skip("TODO")
2271+
}
2272+
22622273
// TestGetEntitlements tests the functionality of retrieving entitlements for entities
22632274
func (s *PDPTestSuite) Test_GetEntitlements() {
22642275
f := s.fixtures
@@ -2875,10 +2886,7 @@ func createResourcePerFqn(attributeValueFQNs ...string) []*authz.Resource {
28752886
for i, fqn := range attributeValueFQNs {
28762887
// Use the FQN itself as the resource ID instead of a generic "ephemeral-id-X"
28772888
resourceID := fqn
2878-
2879-
// TODO: DSPX-1295 - identifier lib does not do case-insensitive parsing, so we need to ensure FQNs are lowercased
2880-
// should maybe be fixed in the identifier library?
2881-
if _, err := identifier.Parse[*identifier.FullyQualifiedRegisteredResourceValue](strings.ToLower(fqn)); err == nil {
2889+
if _, err := identifier.Parse[*identifier.FullyQualifiedRegisteredResourceValue](fqn); err == nil {
28822890
// FQN is a registered resource value
28832891
resources[i] = createRegisteredResource(resourceID, fqn)
28842892
} else {

‎service/internal/access/v2/validators.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,6 @@ func validateGetDecision(entityRepresentation *entityresolutionV2.EntityRepresen
4646
// - registeredResourceValueFQN: must be a valid registered resource value FQN
4747
// - action: must not be nil
4848
// - resources: must not be nil and must contain at least one resource
49-
//
50-
// TODO: DSPX-1295 - add unit tests to detect regressions
5149
func validateGetDecisionRegisteredResource(registeredResourceValueFQN string, action *policy.Action, resources []*authzV2.Resource) error {
5250
if _, err := identifier.Parse[*identifier.FullyQualifiedRegisteredResourceValue](registeredResourceValueFQN); err != nil {
5351
return err

0 commit comments

Comments
 (0)