diff --git a/service/internal/access/v2/evaluate.go b/service/internal/access/v2/evaluate.go index f1a57fd296..e787fa62ec 100644 --- a/service/internal/access/v2/evaluate.go +++ b/service/internal/access/v2/evaluate.go @@ -55,11 +55,6 @@ func getResourceDecision( slog.Any("resource", resource.GetResource()), ) - if len(accessibleAttributeValues) == 0 { - l.WarnContext(ctx, "resource is not able to be entitled", slog.Any("resource", resource.GetResource())) - return failure, nil - } - switch resource.GetResource().(type) { case *authz.Resource_RegisteredResourceValueFqn: registeredResourceValueFQN = strings.ToLower(resource.GetRegisteredResourceValueFqn()) @@ -110,6 +105,12 @@ func getResourceDecision( return nil, fmt.Errorf("unsupported resource type: %w", ErrInvalidResource) } + // Cannot entitle any resource + if len(accessibleAttributeValues) == 0 { + l.WarnContext(ctx, "resource is not able to be entitled", slog.Any("resource", resource.GetResource())) + return failure, nil + } + return evaluateResourceAttributeValues(ctx, l, resourceAttributeValues, resourceID, registeredResourceValueFQN, action, entitlements, accessibleAttributeValues) } @@ -128,17 +129,34 @@ func evaluateResourceAttributeValues( // Group value FQNs by parent definition definitionFqnToValueFqns := make(map[string][]string) definitionsLookup := make(map[string]*policy.Attribute) + notFoundFQNs := make([]string, 0) for _, valueFQN := range resourceAttributeValues.GetFqns() { attributeAndValue, ok := accessibleAttributeValues[valueFQN] if !ok { - return nil, fmt.Errorf("%w: %s", ErrFQNNotFound, valueFQN) + notFoundFQNs = append(notFoundFQNs, valueFQN) + continue } definition := attributeAndValue.GetAttribute() definitionFqnToValueFqns[definition.GetFqn()] = append(definitionFqnToValueFqns[definition.GetFqn()], valueFQN) definitionsLookup[definition.GetFqn()] = definition } + // If ANY FQNs are missing, DENY the resource + if len(notFoundFQNs) > 0 { + l.WarnContext(ctx, "attribute value FQN(s) not found - denying access to resource", + slog.Any("not_found_fqns", notFoundFQNs), + slog.String("resource_id", resourceID)) + result := &ResourceDecision{ + Entitled: false, + ResourceID: resourceID, + } + if resourceName != "" { + result.ResourceName = resourceName + } + return result, nil + } + // Evaluate each definition by rule, resource attributes, action, and entitlements passed := true dataRuleResults := make([]DataRuleResult, 0) diff --git a/service/internal/access/v2/evaluate_test.go b/service/internal/access/v2/evaluate_test.go index 783edcbc8e..9e096afe1a 100644 --- a/service/internal/access/v2/evaluate_test.go +++ b/service/internal/access/v2/evaluate_test.go @@ -751,18 +751,32 @@ func (s *EvaluateTestSuite) TestEvaluateResourceAttributeValues() { expectError: false, }, { - name: "unknown attribute value FQN", + name: "partial FQNs not found - should DENY", resourceAttrs: &authz.Resource_AttributeValues{ Fqns: []string{ levelMidFQN, - "https://namespace.com/attr/department/value/unknown", // This FQN doesn't exist in accessibleAttributeValues + "https://namespace.com/attr/department/value/unknown", }, }, entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{ levelMidFQN: []*policy.Action{actionRead}, }, + // Should NOT error - but should DENY resource (ANY missing FQN = DENY) expectAccessible: false, - expectError: true, + expectError: false, + }, + { + name: "all FQNs not found - should DENY", + resourceAttrs: &authz.Resource_AttributeValues{ + Fqns: []string{ + createAttrValueFQN(baseNamespace, "significance", "critical"), + createAttrValueFQN(baseNamespace, "significance", "major"), + }, + }, + entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{}, + // Should NOT error - but should DENY resource (no FQNs exist) + expectAccessible: false, + expectError: false, }, } @@ -782,19 +796,30 @@ func (s *EvaluateTestSuite) TestEvaluateResourceAttributeValues() { if tc.expectError { s.Require().Error(err) - } else { - s.Require().NoError(err) - s.NotNil(resourceDecision) - s.Equal(tc.expectAccessible, resourceDecision.Entitled) - - // Check results array has the correct length based on grouping by definition - definitions := make(map[string]bool) - for _, fqn := range tc.resourceAttrs.GetFqns() { - if attrAndValue, ok := s.accessibleAttrValues[fqn]; ok { - definitions[attrAndValue.GetAttribute().GetFqn()] = true - } + return + } + + s.Require().NoError(err) + s.NotNil(resourceDecision) + s.Equal(tc.expectAccessible, resourceDecision.Entitled) + + // Check results array has the correct length based on grouping by definition + // If ANY FQN is missing, DataRuleResults should be empty (resource is denied without evaluation) + definitions := make(map[string]bool) + allFQNsExist := true + for _, fqn := range tc.resourceAttrs.GetFqns() { + if attrAndValue, ok := s.accessibleAttrValues[fqn]; ok { + definitions[attrAndValue.GetAttribute().GetFqn()] = true + } else { + allFQNsExist = false } + } + + if allFQNsExist { s.Len(resourceDecision.DataRuleResults, len(definitions)) + } else { + // Any missing FQN means DENY without evaluation + s.Empty(resourceDecision.DataRuleResults) } }) } @@ -887,7 +912,7 @@ func (s *EvaluateTestSuite) TestGetResourceDecision() { expectPass: false, }, { - name: "nonexistent registered resource value", + name: "nonexistent registered resource value - should DENY", resource: &authz.Resource{ Resource: &authz.Resource_RegisteredResourceValueFqn{ RegisteredResourceValueFqn: nonExistentRegResValueFQN, @@ -898,6 +923,73 @@ func (s *EvaluateTestSuite) TestGetResourceDecision() { expectError: false, expectPass: false, }, + { + name: "attribute value FQNs not found, namespace & definition exist - should DENY", + resource: &authz.Resource{ + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{ + createAttrValueFQN(baseNamespace, "department", "doesnotexist"), + }, + }, + }, + EphemeralId: "test-attr-missing-fqns", + }, + entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{}, + expectError: false, + expectPass: false, + }, + { + name: "attribute value FQNs not found, namespace exists - should DENY", + resource: &authz.Resource{ + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{ + createAttrValueFQN(baseNamespace, "unknown", "doesnotexist"), + }, + }, + }, + EphemeralId: "test-attr-missing-fqns", + }, + entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{}, + expectError: false, + expectPass: false, + }, + { + name: "attribute value FQNs not found, namespace does not exist - should DENY", + resource: &authz.Resource{ + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{ + "https://doesnot.exist/attr/severity/value/high", + }, + }, + }, + EphemeralId: "test-attr-missing-fqns", + }, + entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{}, + expectError: false, + expectPass: false, + }, + { + name: "attribute value FQNs partially exist - should DENY", + resource: &authz.Resource{ + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{ + levelMidFQN, + "https://doesnot.exist/attr/severity/value/high", + }, + }, + }, + EphemeralId: "test-attr-values-partially-exist", + }, + entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{ + levelMidFQN: []*policy.Action{actionRead}, + }, + expectError: false, + expectPass: false, + }, { name: "invalid nil resource", resource: nil, @@ -943,3 +1035,70 @@ func (s *EvaluateTestSuite) TestGetResourceDecision() { }) } } + +func (s *EvaluateTestSuite) Test_getResourceDecision_MultiResources_GranularDenials() { + nonExistentFQN := createAttrValueFQN(baseNamespace, "space", "cosmic") + + // Resource 1: Valid FQN, entity is entitled + resource1 := &authz.Resource{ + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{levelHighestFQN}, + }, + }, + EphemeralId: "valid-resource-1", + } + + // Resource 2: Non-existent FQN + resource2 := &authz.Resource{ + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{nonExistentFQN}, + }, + }, + EphemeralId: "invalid-resource-2", + } + + // Resource 3: Valid FQN, entity is entitled + resource3 := &authz.Resource{ + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{levelMidFQN}, + }, + }, + EphemeralId: "valid-resource-3", + } + + entitlements := subjectmappingbuiltin.AttributeValueFQNsToActions{ + levelHighestFQN: []*policy.Action{actionRead}, + levelMidFQN: []*policy.Action{actionRead}, + } + + testCases := []struct { + name string + resource *authz.Resource + expectedEntitled bool + }{ + {"valid resource 1", resource1, true}, + {"invalid resource 2 (missing FQN)", resource2, false}, + {"valid resource 3", resource3, true}, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + decision, err := getResourceDecision( + s.T().Context(), + s.logger, + s.accessibleAttrValues, + s.accessibleRegisteredResourceValues, + entitlements, + s.action, + tc.resource, + ) + + s.Require().NoError(err, "Should not error for resource: %s", tc.name) + s.Require().NotNil(decision) + s.Equal(tc.expectedEntitled, decision.Entitled, "Entitlement mismatch for: %s", tc.name) + }) + } +} diff --git a/service/internal/access/v2/pdp_test.go b/service/internal/access/v2/pdp_test.go index 1d5c0ea016..bdbf33ae3b 100644 --- a/service/internal/access/v2/pdp_test.go +++ b/service/internal/access/v2/pdp_test.go @@ -3148,50 +3148,6 @@ func (s *PDPTestSuite) Test_GetEntitlementsRegisteredResource() { }) } -// Helper functions for all tests - -// assertDecisionResult is a helper function to assert that a decision result for a given FQN matches the expected pass/fail state -func (s *PDPTestSuite) assertDecisionResult(decision *Decision, fqn string, shouldPass bool) { - resourceDecision := findResourceDecision(decision, fqn) - s.Require().NotNil(resourceDecision, "No result found for FQN: "+fqn) - s.Equal(shouldPass, resourceDecision.Entitled, "Unexpected result for FQN %s. Expected (%t), got (%t)", fqn, shouldPass, resourceDecision.Entitled) -} - -// assertAllDecisionResults tests all FQNs in a map of FQN to expected pass/fail state -func (s *PDPTestSuite) assertAllDecisionResults(decision *Decision, expectedResults map[string]bool) { - for fqn, shouldPass := range expectedResults { - s.assertDecisionResult(decision, fqn, shouldPass) - } - // Verify we didn't miss any results - s.Len(decision.Results, len(expectedResults), "Number of results doesn't match expected count") -} - -// createEntityWithProps creates an entity representation with the specified properties -func (s *PDPTestSuite) createEntityWithProps(entityID string, props map[string]interface{}) *entityresolutionV2.EntityRepresentation { - propsStruct := &structpb.Struct{ - Fields: make(map[string]*structpb.Value), - } - - for k, v := range props { - value, err := structpb.NewValue(v) - if err != nil { - panic(fmt.Sprintf("Failed to convert value %v to structpb.Value: %v", v, err)) - } - propsStruct.Fields[k] = value - } - - return &entityresolutionV2.EntityRepresentation{ - OriginalId: entityID, - AdditionalProps: []*structpb.Struct{ - { - Fields: map[string]*structpb.Value{ - "properties": structpb.NewStructValue(propsStruct), - }, - }, - }, - } -} - // createAttributeValueResource creates a resource with attribute values func createAttributeValueResource(ephemeralID string, attributeValueFQNs ...string) *authz.Resource { return &authz.Resource{ @@ -3304,3 +3260,336 @@ func findResourceDecision(decision *Decision, resourceID string) *ResourceDecisi } return nil } + +func (s *PDPTestSuite) Test_GetDecision_NonExistentAttributeFQN() { + f := s.fixtures + + // Create a PDP with only classification attribute + pdp, err := NewPolicyDecisionPoint( + s.T().Context(), + s.logger, + []*policy.Attribute{f.classificationAttr}, + []*policy.SubjectMapping{f.secretMapping, f.topSecretMapping}, + []*policy.RegisteredResource{}, + ) + s.Require().NoError(err) + s.Require().NotNil(pdp) + + s.Run("Single resource with non-existent FQN - should DENY", func() { + entity := s.createEntityWithProps("test-user", map[string]interface{}{ + "clearance": "ts", + }) + + nonExistentFQN := createAttrValueFQN(testBaseNamespace, "space", "cosmic") + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{nonExistentFQN}, + }, + }, + EphemeralId: "resource-with-missing-fqn", + }, + } + + decision, entitlements, err := pdp.GetDecision(s.T().Context(), entity, testActionRead, resources) + + // No error, but deny decision + s.Require().NoError(err) + s.Require().NotNil(decision) + s.False(decision.AllPermitted) + s.Len(decision.Results, 1) + + // Resource is denied + s.False(decision.Results[0].Entitled) + s.Equal("resource-with-missing-fqn", decision.Results[0].ResourceID) + s.Require().NotNil(entitlements) + }) + + s.Run("Multiple resources with mixed valid/invalid FQNs - fine-grained denial", func() { + entity := s.createEntityWithProps("test-user", map[string]interface{}{ + "clearance": "ts", + }) + + // Create resources: 2 valid, 1 with non-existent FQN + nonExistentFQNBadDefinition := createAttrValueFQN(testBaseNamespace, "severity", "high") + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{testClassSecretFQN}, // Valid - entity is entitled + }, + }, + EphemeralId: "valid-resource-1", + }, + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{nonExistentFQNBadDefinition}, // Invalid - doesn't exist + }, + }, + EphemeralId: "invalid-resource-2", + }, + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{testClassTopSecretFQN}, // Valid - entity is entitled + }, + }, + EphemeralId: "valid-resource-3", + }, + } + + decision, _, err := pdp.GetDecision(s.T().Context(), entity, testActionRead, resources) + + // Should NOT error + s.Require().NoError(err) + s.Require().NotNil(decision) + s.False(decision.AllPermitted) + s.Len(decision.Results, 3) + + // Verify individual resource decisions + expectedResults := map[string]bool{ + "valid-resource-1": true, // Entitled + "invalid-resource-2": false, // Denied due to non-existent FQN + "valid-resource-3": true, // Entitled + } + + for _, result := range decision.Results { + expected, exists := expectedResults[result.ResourceID] + s.Require().True(exists, "Unexpected resource ID: %s", result.ResourceID) + s.Equal(expected, result.Entitled, "Entitlement mismatch for resource: %s", result.ResourceID) + } + }) +} + +func (s *PDPTestSuite) Test_GetDecision_PartialFQNsInResource() { + f := s.fixtures + + // Create a PDP with classification attribute (hierarchical rule) + pdp, err := NewPolicyDecisionPoint( + s.T().Context(), + s.logger, + []*policy.Attribute{f.classificationAttr}, + []*policy.SubjectMapping{f.secretMapping, f.topSecretMapping, f.confidentialMapping}, + []*policy.RegisteredResource{}, + ) + s.Require().NoError(err) + s.Require().NotNil(pdp) + + s.Run("Resource with mix of valid and invalid FQNs - DENIED", func() { + entity := s.createEntityWithProps("test-user", map[string]interface{}{ + "clearance": "ts", + }) + + nonExistentValueFQNBadNamespace := createAttrValueFQN("does.notexist", "classification", "cosmic") + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{ + testClassSecretFQN, // Valid + nonExistentValueFQNBadNamespace, // Non-existent - causes DENY + testClassTopSecretFQN, // Valid + }, + }, + }, + EphemeralId: "mixed-fqn-resource", + }, + } + + decision, _, err := pdp.GetDecision(s.T().Context(), entity, testActionRead, resources) + + // No error, but deny decision + s.Require().NoError(err) + s.Require().NotNil(decision) + s.Len(decision.Results, 1) + + s.False(decision.AllPermitted) + s.False(decision.Results[0].Entitled) + }) +} + +func (s *PDPTestSuite) Test_GetDecisionRegisteredResource_NonExistentFQN() { + f := s.fixtures + + // Create a PDP with classification attribute + pdp, err := NewPolicyDecisionPoint( + s.T().Context(), + s.logger, + []*policy.Attribute{f.classificationAttr}, + []*policy.SubjectMapping{f.secretMapping}, + []*policy.RegisteredResource{f.classificationRegRes}, // Only classification registered resource + ) + s.Require().NoError(err) + s.Require().NotNil(pdp) + + s.Run("Registered resource with non-existent FQN - should DENY", func() { + entity := s.createEntityWithProps("test-user", map[string]interface{}{ + "clearance": "secret", + }) + + nonExistentRegResFQN := createRegisteredResourceValueFQN("special-system", "classified") + resources := []*authz.Resource{ + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: nonExistentRegResFQN, + }, + EphemeralId: "missing-reg-res", + }, + } + + decision, _, err := pdp.GetDecision(s.T().Context(), entity, testActionRead, resources) + + // No error, but deny decision + s.Require().NoError(err) + s.Require().NotNil(decision) + s.False(decision.AllPermitted) + s.Len(decision.Results, 1) + + // Resource is denied + s.False(decision.Results[0].Entitled) + s.Equal("missing-reg-res", decision.Results[0].ResourceID) + s.Equal(nonExistentRegResFQN, decision.Results[0].ResourceName) + }) + + s.Run("Mix of valid registered resource and non-existent FQN - fine-grained", func() { + entity := s.createEntityWithProps("test-user", map[string]interface{}{ + "clearance": "secret", + }) + + nonExistentRegResFQN := createRegisteredResourceValueFQN("secret-system", "classified") + resources := []*authz.Resource{ + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: testClassSecretRegResFQN, // Valid + }, + EphemeralId: "valid-reg-res", + }, + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: nonExistentRegResFQN, // Non-existent + }, + EphemeralId: "invalid-reg-res", + }, + } + + decision, _, err := pdp.GetDecision(s.T().Context(), entity, testActionRead, resources) + + // Should NOT error + s.Require().NoError(err) + s.Require().NotNil(decision) + s.False(decision.AllPermitted) + s.Len(decision.Results, 2) + + // Verify individual decisions + for _, result := range decision.Results { + switch result.ResourceID { + case "valid-reg-res": + s.True(result.Entitled) + case "invalid-reg-res": + s.False(result.Entitled) + default: + s.Failf("Unexpected resource ID: %s", result.ResourceID) + } + } + }) +} + +func (s *PDPTestSuite) Test_GetDecision_NoPolicies() { + // Empty PDP with no attributes, subject mappings, or registered resources + pdp, err := NewPolicyDecisionPoint( + s.T().Context(), + s.logger, + []*policy.Attribute{}, + []*policy.SubjectMapping{}, + []*policy.RegisteredResource{}, + ) + s.Require().NoError(err) + s.Require().NotNil(pdp) + + s.Run("No policy found - should DENY all resources", func() { + entity := s.createEntityWithProps("test-user", map[string]interface{}{ + "clearance": "ts", + }) + + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{testClassSecretFQN}, + }, + }, + EphemeralId: "resource-1", + }, + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{testClassTopSecretFQN}, + }, + }, + EphemeralId: "resource-2", + }, + } + + decision, entitlements, err := pdp.GetDecision(s.T().Context(), entity, testActionRead, resources) + + // No error, but deny decision + s.Require().NoError(err) + s.Require().NotNil(decision) + s.False(decision.AllPermitted) + s.Len(decision.Results, 2) + + for _, result := range decision.Results { + s.False(result.Entitled) + } + + // Empty entitlements without available policy + s.Require().NotNil(entitlements) + s.Empty(entitlements) + }) +} + +// Helper functions for all tests + +// assertDecisionResult is a helper function to assert that a decision result for a given FQN matches the expected pass/fail state +func (s *PDPTestSuite) assertDecisionResult(decision *Decision, fqn string, shouldPass bool) { + resourceDecision := findResourceDecision(decision, fqn) + s.Require().NotNil(resourceDecision, "No result found for FQN: "+fqn) + s.Equal(shouldPass, resourceDecision.Entitled, "Unexpected result for FQN %s. Expected (%t), got (%t)", fqn, shouldPass, resourceDecision.Entitled) +} + +// assertAllDecisionResults tests all FQNs in a map of FQN to expected pass/fail state +func (s *PDPTestSuite) assertAllDecisionResults(decision *Decision, expectedResults map[string]bool) { + for fqn, shouldPass := range expectedResults { + s.assertDecisionResult(decision, fqn, shouldPass) + } + // Verify we didn't miss any results + s.Len(decision.Results, len(expectedResults), "Number of results doesn't match expected count") +} + +// createEntityWithProps creates an entity representation with the specified properties +func (s *PDPTestSuite) createEntityWithProps(entityID string, props map[string]interface{}) *entityresolutionV2.EntityRepresentation { + propsStruct := &structpb.Struct{ + Fields: make(map[string]*structpb.Value), + } + + for k, v := range props { + value, err := structpb.NewValue(v) + if err != nil { + panic(fmt.Sprintf("Failed to convert value %v to structpb.Value: %v", v, err)) + } + propsStruct.Fields[k] = value + } + + return &entityresolutionV2.EntityRepresentation{ + OriginalId: entityID, + AdditionalProps: []*structpb.Struct{ + { + Fields: map[string]*structpb.Value{ + "properties": structpb.NewStructValue(propsStruct), + }, + }, + }, + } +}