diff --git a/lib/identifier/policyidentifier.go b/lib/identifier/policyidentifier.go index 29211c7e66..9f16dbcb58 100644 --- a/lib/identifier/policyidentifier.go +++ b/lib/identifier/policyidentifier.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "regexp" + "strings" ) var ( @@ -49,7 +50,7 @@ func Parse[T FullyQualified](identifier string) (T, error) { parsed any err error ) - + identifier = strings.ToLower(identifier) // Use type assertion to determine the concrete type and call the appropriate parser switch any(result).(type) { case *FullyQualifiedAttribute: diff --git a/service/internal/access/v2/evaluate.go b/service/internal/access/v2/evaluate.go index 3e9d9c1df9..05ac19c453 100644 --- a/service/internal/access/v2/evaluate.go +++ b/service/internal/access/v2/evaluate.go @@ -62,13 +62,6 @@ func getResourceDecision( Fqns: make([]string, 0), } for _, aav := range regResValue.GetActionAttributeValues() { - // TODO: DSPX-1295 - revisit this logic -- reg res' are different from attr values since they can be both entity and resource - // and are tied to actions and attribute values - // - // if aav.GetAction().GetName() != action.GetName() { - // continue - // } - aavAttrValueFQN := aav.GetAttributeValue().GetFqn() if !slices.Contains(resourceAttributeValues.GetFqns(), aavAttrValueFQN) { resourceAttributeValues.Fqns = append(resourceAttributeValues.Fqns, aavAttrValueFQN) diff --git a/service/internal/access/v2/evaluate_test.go b/service/internal/access/v2/evaluate_test.go index e0174b0d1d..eac22f1f52 100644 --- a/service/internal/access/v2/evaluate_test.go +++ b/service/internal/access/v2/evaluate_test.go @@ -39,6 +39,10 @@ var ( projectAvengersFQN = createAttrValueFQN(baseNamespace, "project", "avengers") projectXmenFQN = createAttrValueFQN(baseNamespace, "project", "xmen") projectFantasicFourFQN = createAttrValueFQN(baseNamespace, "project", "fantasticfour") + + // Registered resource values + netRegResValFQN = createRegisteredResourceValueFQN("network", "external") + platRegResValFQN = createRegisteredResourceValueFQN("platform", "internal") ) var ( @@ -155,8 +159,53 @@ func (s *EvaluateTestSuite) SetupTest() { } // Setup accessible registered resource values map - // TODO: DSPX-1295 - s.accessibleRegisteredResourceValues = map[string]*policy.RegisteredResourceValue{} + // Create the registered resource values with action attribute values + s.accessibleRegisteredResourceValues = map[string]*policy.RegisteredResourceValue{ + netRegResValFQN: { + Id: "network-registered-res-id", + Value: "external", + ActionAttributeValues: []*policy.RegisteredResourceValue_ActionAttributeValue{ + { + Id: "network-action-attr-val-1", + Action: actionRead, + AttributeValue: &policy.Value{ + Fqn: levelHighestFQN, + Value: "highest", + }, + }, + { + Id: "network-action-attr-val-2", + Action: actionCreate, + AttributeValue: &policy.Value{ + Fqn: levelMidFQN, + Value: "mid", + }, + }, + }, + }, + platRegResValFQN: { + Id: "platform-registered-res-id", + Value: "internal", + ActionAttributeValues: []*policy.RegisteredResourceValue_ActionAttributeValue{ + { + Id: "platform-action-attr-val-1", + Action: actionRead, + AttributeValue: &policy.Value{ + Fqn: projectAvengersFQN, + Value: "avengers", + }, + }, + { + Id: "platform-action-attr-val-2", + Action: actionRead, + AttributeValue: &policy.Value{ + Fqn: projectJusticeLeagueFQN, + Value: "justiceleague", + }, + }, + }, + }, + } } func TestEvaluateSuite(t *testing.T) { @@ -751,11 +800,14 @@ func (s *EvaluateTestSuite) TestEvaluateResourceAttributeValues() { // Test cases for getResourceDecision func (s *EvaluateTestSuite) TestGetResourceDecision() { + nonExistentRegResValueFQN := createRegisteredResourceValueFQN("nonexistent", "value") + tests := []struct { name string resource *authz.Resource entitlements subjectmappingbuiltin.AttributeValueFQNsToActions expectError bool + expectPass bool }{ { name: "attribute values resource", @@ -765,11 +817,84 @@ func (s *EvaluateTestSuite) TestGetResourceDecision() { Fqns: []string{levelMidFQN}, }, }, + EphemeralId: "test-attr-values-id-1", }, entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{ levelMidFQN: []*policy.Action{actionRead}, }, expectError: false, + expectPass: true, + }, + { + name: "registered resource value with all entitlements", + resource: &authz.Resource{ + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: netRegResValFQN, + }, + EphemeralId: "test-reg-res-id-1", + }, + entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{ + levelHighestFQN: []*policy.Action{actionRead}, + }, + expectError: false, + expectPass: true, + }, + { + name: "registered resource value with project values", + resource: &authz.Resource{ + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: platRegResValFQN, + }, + EphemeralId: "test-reg-res-id-2", + }, + entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{ + projectAvengersFQN: []*policy.Action{actionRead}, + projectJusticeLeagueFQN: []*policy.Action{actionRead}, + }, + expectError: false, + expectPass: true, + }, + { + name: "registered resource value with missing entitlements", + resource: &authz.Resource{ + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: platRegResValFQN, + }, + EphemeralId: "test-reg-res-id-3", + }, + entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{ + // Missing projectJusticeLeagueFQN + projectAvengersFQN: []*policy.Action{actionRead}, + }, + expectError: false, + expectPass: false, // Missing entitlement for projectJusticeLeagueFQN + }, + { + name: "registered resource value with wrong action", + resource: &authz.Resource{ + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: netRegResValFQN, + }, + EphemeralId: "test-reg-res-id-4", + }, + entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{ + // Wrong action + levelHighestFQN: []*policy.Action{actionCreate}, + }, + expectError: false, + expectPass: false, + }, + { + name: "nonexistent registered resource value", + resource: &authz.Resource{ + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: nonExistentRegResValueFQN, + }, + EphemeralId: "test-reg-res-id-5", + }, + entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{}, + expectError: true, + expectPass: false, }, { name: "invalid nil resource", @@ -777,6 +902,20 @@ func (s *EvaluateTestSuite) TestGetResourceDecision() { entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{}, expectError: true, }, + { + name: "case insensitive registered resource value FQN", + resource: &authz.Resource{ + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: strings.ToUpper(netRegResValFQN), // Test case insensitivity + }, + EphemeralId: "test-reg-res-id-6", + }, + entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{ + levelHighestFQN: []*policy.Action{actionRead}, + }, + expectError: false, + expectPass: true, + }, } for _, tc := range tests { @@ -796,6 +935,8 @@ func (s *EvaluateTestSuite) TestGetResourceDecision() { } else { s.Require().NoError(err) s.NotNil(decision) + s.Equal(tc.expectPass, decision.Passed, "Decision pass status didn't match") + s.Equal(tc.resource.GetEphemeralId(), decision.ResourceID, "Resource ID didn't match") } }) } diff --git a/service/internal/access/v2/helpers.go b/service/internal/access/v2/helpers.go index 69342cadd7..574ff98181 100644 --- a/service/internal/access/v2/helpers.go +++ b/service/internal/access/v2/helpers.go @@ -218,14 +218,6 @@ func getResourceDecisionableAttributes( } for _, aav := range regResValue.GetActionAttributeValues() { - // TODO: DSPX-1295 - revisit this logic bc it is causing failures for attributes with missing actions - // slog.Info("processing action attribute value", slog.Any("aav", aav)) - // aavAction := aav.GetAction() - // if aavAction.GetName() != action.GetName() { - // logger.DebugContext(ctx, "skipping action not matching Decision Request action", slog.String("action", aavAction.GetName())) - // continue - // } - attrValueFQNs = append(attrValueFQNs, aav.GetAttributeValue().GetFqn()) } diff --git a/service/internal/access/v2/pdp_test.go b/service/internal/access/v2/pdp_test.go index 0224e9b1ae..d76c670780 100644 --- a/service/internal/access/v2/pdp_test.go +++ b/service/internal/access/v2/pdp_test.go @@ -87,7 +87,7 @@ var ( testPlatformOnPremFQN = createAttrValueFQN(testSecondaryNamespace, "platform", "onprem") testPlatformHybridFQN = createAttrValueFQN(testSecondaryNamespace, "platform", "hybrid") - // Registered resource value FQNs (TODO: DSPX-1295 - remove) + // Registered resource value FQNs (TODO: remove) testNetworkPrivateFQN = createRegisteredResourceValueFQN("network", "private") testNetworkPublicFQN = createRegisteredResourceValueFQN("network", "public") // testNetworkConfidentialFQN = createRegisteredResourceValueFQN("network", "confidential") @@ -97,7 +97,6 @@ var ( // registered resource value FQNs using identifier package var ( // Classification values - // testClassTopSecretRegResFQN = createRegisteredResourceValueFQN("classification", "topsecret") testClassSecretRegResFQN = createRegisteredResourceValueFQN("classification", "secret") testClassConfidentialRegResFQN = createRegisteredResourceValueFQN("classification", "confidential") // testClassPublicRegResFQN = createRegisteredResourceValueFQN("classification", "public") @@ -124,7 +123,7 @@ var ( ) // Registered resource value FQNs using identifier package -// TODO: DSPX-1295 - remove these and use the other ones above +// TODO: remove these and use the other ones above var ( regResValNoActionAttrValFQN string regResValSingleActionAttrValFQN string @@ -175,12 +174,12 @@ type PDPTestSuite struct { // Test registered resources classificationRegRes *policy.RegisteredResource deptRegRes *policy.RegisteredResource - networkRegRes *policy.RegisteredResource // TODO: DSPX-1295 - remove this and use the others that match test attributes + networkRegRes *policy.RegisteredResource // TODO: remove this and use the others that match test attributes countryRegRes *policy.RegisteredResource projectRegRes *policy.RegisteredResource platformRegRes *policy.RegisteredResource - // Test registered resources (TODO: DSPX-1295 - remove these and use the ones above) + // Test registered resources (TODO: remove these and use the ones above) regRes *policy.RegisteredResource regResValNoActionAttrVal *policy.RegisteredResourceValue regResValSingleActionAttrVal *policy.RegisteredResourceValue @@ -698,7 +697,7 @@ func (s *PDPTestSuite) SetupTest() { }, } - // Initialize test registered resources (TODO: DSPX-1295: replace with above real use cases) + // Initialize test registered resources (TODO: replace with above real use cases) regResValNoActionAttrVal := &policy.RegisteredResourceValue{ Value: "no-action-attr-val", ActionAttributeValues: []*policy.RegisteredResourceValue_ActionAttributeValue{}, @@ -2259,6 +2258,18 @@ func (s *PDPTestSuite) Test_GetDecisionRegisteredResource_MultipleResources() { }) } +func (s *PDPTestSuite) Test_GetDecisionRegisteredResource_PartialActionEntitlement() { + s.T().Skip("TODO") +} + +func (s *PDPTestSuite) Test_GetDecisionRegisteredResource_CombinedAttributeRules_SingleResource() { + s.T().Skip("TODO") +} + +func (s *PDPTestSuite) Test_GetDecisionRegisteredResource_AcrossNamespaces() { + s.T().Skip("TODO") +} + // TestGetEntitlements tests the functionality of retrieving entitlements for entities func (s *PDPTestSuite) Test_GetEntitlements() { f := s.fixtures @@ -2875,10 +2886,7 @@ func createResourcePerFqn(attributeValueFQNs ...string) []*authz.Resource { for i, fqn := range attributeValueFQNs { // Use the FQN itself as the resource ID instead of a generic "ephemeral-id-X" resourceID := fqn - - // TODO: DSPX-1295 - identifier lib does not do case-insensitive parsing, so we need to ensure FQNs are lowercased - // should maybe be fixed in the identifier library? - if _, err := identifier.Parse[*identifier.FullyQualifiedRegisteredResourceValue](strings.ToLower(fqn)); err == nil { + if _, err := identifier.Parse[*identifier.FullyQualifiedRegisteredResourceValue](fqn); err == nil { // FQN is a registered resource value resources[i] = createRegisteredResource(resourceID, fqn) } else { diff --git a/service/internal/access/v2/validators.go b/service/internal/access/v2/validators.go index bd4148daa6..e49c9d6509 100644 --- a/service/internal/access/v2/validators.go +++ b/service/internal/access/v2/validators.go @@ -46,8 +46,6 @@ func validateGetDecision(entityRepresentation *entityresolutionV2.EntityRepresen // - registeredResourceValueFQN: must be a valid registered resource value FQN // - action: must not be nil // - resources: must not be nil and must contain at least one resource -// -// TODO: DSPX-1295 - add unit tests to detect regressions func validateGetDecisionRegisteredResource(registeredResourceValueFQN string, action *policy.Action, resources []*authzV2.Resource) error { if _, err := identifier.Parse[*identifier.FullyQualifiedRegisteredResourceValue](registeredResourceValueFQN); err != nil { return err diff --git a/service/internal/access/v2/validators_test.go b/service/internal/access/v2/validators_test.go index 0d9f57d226..19f002b6bb 100644 --- a/service/internal/access/v2/validators_test.go +++ b/service/internal/access/v2/validators_test.go @@ -3,6 +3,7 @@ package access import ( "testing" + "github.com/opentdf/platform/lib/identifier" authzV2 "github.com/opentdf/platform/protocol/go/authorization/v2" entityresolutionV2 "github.com/opentdf/platform/protocol/go/entityresolution/v2" "github.com/opentdf/platform/protocol/go/policy" @@ -511,3 +512,80 @@ func TestValidateGetResourceDecision(t *testing.T) { }) } } + +func TestValidateGetDecisionRegisteredResource(t *testing.T) { + validRegisteredResourceValueFQN := "https://reg_res/resource1/value/value1" + + validAction := &policy.Action{ + Name: "read", + } + + emptyNameAction := &policy.Action{ + Name: "", + } + + validResources := []*authzV2.Resource{ + { + Resource: &authzV2.Resource_AttributeValues_{ + AttributeValues: &authzV2.Resource_AttributeValues{ + Fqns: []string{"https://example.org/attr/classification/value/public"}, + }, + }, + }, + } + + tests := []struct { + name string + registeredResourceValueFQN string + action *policy.Action + resources []*authzV2.Resource + wantErr error + }{ + { + name: "Valid inputs", + registeredResourceValueFQN: validRegisteredResourceValueFQN, + action: validAction, + resources: validResources, + wantErr: nil, + }, + { + name: "Invalid registered resource value FQN", + registeredResourceValueFQN: "invalid-fqn", + action: validAction, + resources: validResources, + wantErr: identifier.ErrInvalidFQNFormat, + }, + { + name: "Empty action name", + registeredResourceValueFQN: validRegisteredResourceValueFQN, + action: emptyNameAction, + resources: validResources, + wantErr: ErrInvalidAction, + }, + { + name: "Empty resources", + registeredResourceValueFQN: validRegisteredResourceValueFQN, + action: validAction, + resources: []*authzV2.Resource{}, + wantErr: ErrInvalidResource, + }, + { + name: "Nil resource in list", + registeredResourceValueFQN: validRegisteredResourceValueFQN, + action: validAction, + resources: []*authzV2.Resource{nil}, + wantErr: ErrInvalidResource, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateGetDecisionRegisteredResource(tt.registeredResourceValueFQN, tt.action, tt.resources) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + }) + } +}