From 4bf983b58829a2d2789fd2fa10f1776b0fdecba3 Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Mon, 8 Sep 2025 13:57:22 -0700 Subject: [PATCH 01/17] feat(authz): add obligation policy decision point --- .../access/v2/obligations/obligations_pdp.go | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 service/internal/access/v2/obligations/obligations_pdp.go diff --git a/service/internal/access/v2/obligations/obligations_pdp.go b/service/internal/access/v2/obligations/obligations_pdp.go new file mode 100644 index 0000000000..82b61956fe --- /dev/null +++ b/service/internal/access/v2/obligations/obligations_pdp.go @@ -0,0 +1,38 @@ +package obligations + +import ( + "context" + + "github.com/opentdf/platform/protocol/go/policy" + attrs "github.com/opentdf/platform/protocol/go/policy/attributes" + "github.com/opentdf/platform/service/logger" +) + +type ObligationsPolicyDecisionPoint struct { + logger *logger.Logger + attributesByValueFQN map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue + registeredResourceValuesByFQN map[string]*policy.RegisteredResourceValue + obligationValuesByFQN map[string]*policy.ObligationValue +} + +func NewObligationsPolicyDecisionPoint( + ctx context.Context, + l *logger.Logger, + attributesByValueFQN map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue, + registeredResourceValuesByFQN map[string]*policy.RegisteredResourceValue, + allObligations []*policy.Obligation, +) (*ObligationsPolicyDecisionPoint, error) { + pdp := &ObligationsPolicyDecisionPoint{ + logger: l, + attributesByValueFQN: attributesByValueFQN, + registeredResourceValuesByFQN: registeredResourceValuesByFQN, + } + + for _, definition := range allObligations { + for _, value := range definition.GetValues() { + pdp.obligationValuesByFQN[value.GetValue()] = value + } + } + + return pdp, nil +} From 3dfdd0ed4f05f71491709524616fe20c1baec1d5 Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Tue, 9 Sep 2025 14:03:27 -0700 Subject: [PATCH 02/17] wip: get required obligations --- .../access/v2/obligations/obligations_pdp.go | 179 +++++++++++++++++- 1 file changed, 177 insertions(+), 2 deletions(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp.go b/service/internal/access/v2/obligations/obligations_pdp.go index 82b61956fe..e52a348aee 100644 --- a/service/internal/access/v2/obligations/obligations_pdp.go +++ b/service/internal/access/v2/obligations/obligations_pdp.go @@ -2,17 +2,37 @@ package obligations import ( "context" + "fmt" + "log/slog" + "strconv" + authz "github.com/opentdf/platform/protocol/go/authorization/v2" "github.com/opentdf/platform/protocol/go/policy" attrs "github.com/opentdf/platform/protocol/go/policy/attributes" "github.com/opentdf/platform/service/logger" ) +// A graph of action names to attribute value FQNs to lists of obligation value FQNs +// i.e. read : https://example.org/attr/attr1/value/val1 : [https://example.org/obl/some_obligation/value/some_value] +type obligationValuesByActionOnAnAttributeValue map[string]map[string][]string + type ObligationsPolicyDecisionPoint struct { logger *logger.Logger attributesByValueFQN map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue registeredResourceValuesByFQN map[string]*policy.RegisteredResourceValue obligationValuesByFQN map[string]*policy.ObligationValue + // When resolving triggered obligations, there are multiple trigger paths: + // 1. actions on attributes + // 2. actions on attributes within the request context of a specific PEP, driven by PEP idP clientID + // + // Both are able to be pre-computed from policy into a graph data structure so an actual PDP + // trigger check can traverse in fastest possible time complexity. + // + // read : attrValFQN : []string{obl1} + simpleTriggerActionsToAttributes obligationValuesByActionOnAnAttributeValue + // pep-client : read : attrValFQN : []string{obl2} + // other-pep-client : read : attrValFQN : []string{obl2,obl3} + clientIDScopedTriggerActionsToAttributes map[string]obligationValuesByActionOnAnAttributeValue } func NewObligationsPolicyDecisionPoint( @@ -26,13 +46,168 @@ func NewObligationsPolicyDecisionPoint( logger: l, attributesByValueFQN: attributesByValueFQN, registeredResourceValuesByFQN: registeredResourceValuesByFQN, + obligationValuesByFQN: make(map[string]*policy.ObligationValue), } + simpleTriggered := make(obligationValuesByActionOnAnAttributeValue) + clientScopedTriggered := make(map[string]obligationValuesByActionOnAnAttributeValue) + for _, definition := range allObligations { - for _, value := range definition.GetValues() { - pdp.obligationValuesByFQN[value.GetValue()] = value + for _, obligationValue := range definition.GetValues() { + pdp.obligationValuesByFQN[obligationValue.GetFqn()] = obligationValue + + for _, trigger := range obligationValue.GetTriggers() { + attrValFqn := trigger.GetAttributeValue().GetFqn() + actionName := trigger.GetAction().GetName() + optionalPepClientID := trigger.GetContext().GetPep().GetClientId() + + // If the request context PEP client ID was provided to scope an obligation value to a PEP, populate that lookup graph + if optionalPepClientID != "" { + if _, ok := clientScopedTriggered[optionalPepClientID]; !ok { + clientScopedTriggered[optionalPepClientID] = make(obligationValuesByActionOnAnAttributeValue) + } + if _, ok := clientScopedTriggered[optionalPepClientID][actionName]; !ok { + clientScopedTriggered[optionalPepClientID][actionName] = make(map[string][]string) + } + clientScopedTriggered[optionalPepClientID][actionName][attrValFqn] = append(clientScopedTriggered[optionalPepClientID][actionName][attrValFqn], obligationValue.GetFqn()) + } else { + // Otherwise, populate unscoped lookup graph + if _, ok := simpleTriggered[actionName]; !ok { + simpleTriggered[actionName] = make(map[string][]string) + } + simpleTriggered[actionName][attrValFqn] = append(simpleTriggered[actionName][attrValFqn], obligationValue.GetFqn()) + } + } } } + // Store lookup resolution graphs in state for the duration of the PDP + pdp.clientIDScopedTriggerActionsToAttributes = clientScopedTriggered + pdp.simpleTriggerActionsToAttributes = simpleTriggered + + pdp.logger.DebugContext( + ctx, + "created obligations policy decision point", + slog.Int("obligation_values_count", len(pdp.obligationValuesByFQN)), + ) + return pdp, nil } + +// GetRequiredObligations takes in an action and multiple resources subject to decisioning. +// +// It drills into the resources to find all triggered obligations on each combination of: +// 1. action +// 2. attribute value +// 3. decision request context (at present, strictly any scoped PEP clientID) +// +// In response, it returns the obligations required per each input resource index and the entire list of deduplicated required obligations +func (p *ObligationsPolicyDecisionPoint) GetRequiredObligations( + ctx context.Context, + action *policy.Action, + resources []*authz.Resource, + decisionRequestContext *mockDecisionRequestContext, +) ([][]string, []string, error) { + // Required obligations per resource of a given index + requiredOblValueFQNsPerResource := make([][]string, len(resources)) + // Set of required obligations across all resources + allRequiredOblValueFQNs := make([]string, 0) + allOblValFQNsSeen := make(map[string]struct{}) + + pepClientID := decisionRequestContext.PEP.clientID + + l := p.logger. + With("action", action.GetName()). + With("pep_client_id", pepClientID). + With("resources_count", strconv.Itoa(len(resources))) + + // Short-circuit if the requested action and optional scoping clientID are not found within any obligation triggers + attrValueFQNsToObligations, triggersOnActionExist := p.simpleTriggerActionsToAttributes[action.GetName()] + clientScoped, triggersOnClientIDExist := p.clientIDScopedTriggerActionsToAttributes[pepClientID] + if triggersOnClientIDExist { + _, triggersOnClientIDExist = clientScoped[action.GetName()] + } + if !triggersOnActionExist && !triggersOnClientIDExist { + l.DebugContext(ctx, "no triggered obligations found for action") + return nil, nil, nil + } + + // Traverse trigger lookup graphs to resolve required obligations + for i, resource := range resources { + // For each type of resource, drill down within to collect the attribute value FQNs relevant to this action + attrValueFQNs := []string{} + switch resource.GetResource().(type) { + case *authz.Resource_RegisteredResourceValueFqn: + regResValFQN := resource.GetRegisteredResourceValueFqn() + regResValue, ok := p.registeredResourceValuesByFQN[regResValFQN] + if !ok { + return nil, nil, fmt.Errorf("unknown registered resource value: %s", regResValFQN) + } + + // Check the action-attribute-values associated with a Registered Resource Value for a match to the request action + for _, aav := range regResValue.GetActionAttributeValues() { + actionName := aav.GetAction().GetName() + attrValFQN := aav.GetAttributeValue().GetFqn() + if actionName != action.GetName() { + continue + } + attrValueFQNs = append(attrValueFQNs, attrValFQN) + } + + case *authz.Resource_AttributeValues_: + attrValueFQNs = append(attrValueFQNs, resource.GetAttributeValues().GetFqns()...) + + default: + return nil, nil, fmt.Errorf("unsupported resource type: %T", resource) + } + + // With list of attribute values for the resource, traverse each lookup graph to resolve the Set of required obligations + seenThisResource := make(map[string]struct{}) + resourceRequiredOblValueFQNsSet := make([]string, 0) + for _, attrValFQN := range attrValueFQNs { + if triggered, ok := attrValueFQNsToObligations[attrValFQN]; ok { + for _, oblValFQN := range triggered { + if _, ok := seenThisResource[oblValFQN]; ok { + continue + } + // Update set of obligations triggered for this specific resource + seenThisResource[oblValFQN] = struct{}{} + resourceRequiredOblValueFQNsSet = append(resourceRequiredOblValueFQNsSet, oblValFQN) + + // Update global set tracking those triggered across all resources + if _, seen := allOblValFQNsSeen[oblValFQN]; !seen { + allOblValFQNsSeen[oblValFQN] = struct{}{} + allRequiredOblValueFQNs = append(allRequiredOblValueFQNs, oblValFQN) + } + } + } + + if triggered, ok := p.clientIDScopedTriggerActionsToAttributes[pepClientID][action.GetName()][attrValFQN]; ok { + for _, oblValFQN := range triggered { + if _, ok := seenThisResource[oblValFQN]; ok { + continue + } + // Update set of obligations triggered for this specific resource + seenThisResource[oblValFQN] = struct{}{} + resourceRequiredOblValueFQNsSet = append(resourceRequiredOblValueFQNsSet, oblValFQN) + + // Update global set tracking those triggered across all resources + if _, seen := allOblValFQNsSeen[oblValFQN]; !seen { + allOblValFQNsSeen[oblValFQN] = struct{}{} + allRequiredOblValueFQNs = append(allRequiredOblValueFQNs, oblValFQN) + } + } + } + } + requiredOblValueFQNsPerResource[i] = resourceRequiredOblValueFQNsSet + } + + l.DebugContext( + ctx, + "found required obligations", + slog.Any("required_obl_values_per_resource", requiredOblValueFQNsPerResource), + slog.Any("required_obligations_across_all_resources", allRequiredOblValueFQNs), + ) + + return requiredOblValueFQNsPerResource, allRequiredOblValueFQNs, nil +} From fcfaa1331951a702963cf064068a2dece3bda21a Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Tue, 9 Sep 2025 14:25:09 -0700 Subject: [PATCH 03/17] mock to allow compilation --- service/internal/access/v2/obligations/obligations_pdp.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp.go b/service/internal/access/v2/obligations/obligations_pdp.go index e52a348aee..0ad38544a5 100644 --- a/service/internal/access/v2/obligations/obligations_pdp.go +++ b/service/internal/access/v2/obligations/obligations_pdp.go @@ -59,7 +59,8 @@ func NewObligationsPolicyDecisionPoint( for _, trigger := range obligationValue.GetTriggers() { attrValFqn := trigger.GetAttributeValue().GetFqn() actionName := trigger.GetAction().GetName() - optionalPepClientID := trigger.GetContext().GetPep().GetClientId() + optionalPepClientID := "mock-client-id" + // optionalPepClientID := trigger.GetContext().GetPep().GetClientId() // If the request context PEP client ID was provided to scope an obligation value to a PEP, populate that lookup graph if optionalPepClientID != "" { @@ -71,7 +72,7 @@ func NewObligationsPolicyDecisionPoint( } clientScopedTriggered[optionalPepClientID][actionName][attrValFqn] = append(clientScopedTriggered[optionalPepClientID][actionName][attrValFqn], obligationValue.GetFqn()) } else { - // Otherwise, populate unscoped lookup graph + // Otherwise, populate unscoped lookup graph with just actions and attributes alone if _, ok := simpleTriggered[actionName]; !ok { simpleTriggered[actionName] = make(map[string][]string) } @@ -211,3 +212,5 @@ func (p *ObligationsPolicyDecisionPoint) GetRequiredObligations( return requiredOblValueFQNsPerResource, allRequiredOblValueFQNs, nil } + +// TODO: pdp.GetObligationsFulfilled? From e56eb6dab73ffa8eed68f676cf503f71d54ad860 Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Tue, 9 Sep 2025 14:34:18 -0700 Subject: [PATCH 04/17] scaffold test suite --- .../v2/obligations/obligations_pdp_test.go | 212 ++++++++++++++++++ 1 file changed, 212 insertions(+) create mode 100644 service/internal/access/v2/obligations/obligations_pdp_test.go diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go new file mode 100644 index 0000000000..b76c7c431d --- /dev/null +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -0,0 +1,212 @@ +package obligations + +import ( + "context" + "testing" + + "github.com/stretchr/testify/suite" + + authz "github.com/opentdf/platform/protocol/go/authorization/v2" + "github.com/opentdf/platform/protocol/go/policy" + attrs "github.com/opentdf/platform/protocol/go/policy/attributes" + "github.com/opentdf/platform/service/logger" +) + +const ( + mockAttrValFQN1 = "https://example.org/attr/attr1/value/val1" + mockAttrValFQN2 = "https://example.org/attr/attr2/value/val2" + mockObligationFQN1 = "https://example.org/obl/some_obligation/value/some_value" + mockObligationFQN2 = "https://example.org/obl/another_obligation/value/another_value" + mockClientID = "mock-client-id" +) + +var mockAction = &policy.Action{Name: "read"} + +type ObligationsPDPSuite struct { + suite.Suite + pdp *ObligationsPolicyDecisionPoint +} + +func (s *ObligationsPDPSuite) SetupSuite() { + // Mock attributes + attributesByValueFQN := map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue{ + mockAttrValFQN1: { + Attribute: &policy.Attribute{Name: "attr1"}, + Value: &policy.Value{Fqn: mockAttrValFQN1}, + }, + mockAttrValFQN2: { + Attribute: &policy.Attribute{Name: "attr2"}, + Value: &policy.Value{Fqn: mockAttrValFQN2}, + }, + } + + // Mock obligations + allObligations := []*policy.Obligation{ + { + Values: []*policy.ObligationValue{ + { + Fqn: mockObligationFQN1, + Triggers: []*policy.ObligationTrigger{ + { + Action: mockAction, + AttributeValue: &policy.Value{Fqn: mockAttrValFQN1}, + }, + }, + }, + { + Fqn: mockObligationFQN2, + Triggers: []*policy.ObligationTrigger{ + { + Action: mockAction, + AttributeValue: &policy.Value{Fqn: mockAttrValFQN2}, + // Context: &policy.ObligationTrigger_Context{ + // Pep: &policy.ObligationTrigger_Context_PEP{ + // ClientId: mockClientID, + // }, + // }, + }, + }, + }, + }, + }, + } + + // Create a new PDP instance + log, err := logger.NewLogger(logger.Config{Output: "stdout", Level: "info"}) + s.Require().NoError(err) + s.pdp, err = NewObligationsPolicyDecisionPoint( + context.Background(), + log, + attributesByValueFQN, + nil, + allObligations, + ) + s.Require().NoError(err) +} + +func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{"https://example.org/attr/other/value/val"}, + }, + }, + }, + } + decisionRequestContext := &mockDecisionRequestContext{} + + perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) + + s.NoError(err) + s.Equal([][]string{{}}, perResource) + s.Empty(all) +} + +func (s *ObligationsPDPSuite) Test_SimpleObligationTriggered() { + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN1}, + }, + }, + }, + } + decisionRequestContext := &mockDecisionRequestContext{} + + perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) + + s.NoError(err) + s.Equal([][]string{{mockObligationFQN1}}, perResource) + s.Equal([]string{mockObligationFQN1}, all) +} + +func (s *ObligationsPDPSuite) Test_ClientScopedObligationTriggered() { + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN2}, + }, + }, + }, + } + decisionRequestContext := &mockDecisionRequestContext{ + PEP: struct{ clientID string }{clientID: mockClientID}, + } + + perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) + + s.NoError(err) + s.Equal([][]string{{mockObligationFQN2}}, perResource) + s.Equal([]string{mockObligationFQN2}, all) +} + +func (s *ObligationsPDPSuite) Test_ClientScopedObligationNotTriggered() { + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN2}, + }, + }, + }, + } + decisionRequestContext := &mockDecisionRequestContext{ + PEP: struct{ clientID string }{clientID: "different-client-id"}, + } + + perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) + + s.NoError(err) + s.Equal([][]string{{}}, perResource) + s.Empty(all) +} + +func (s *ObligationsPDPSuite) Test_MixedObligationsTriggered() { + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN1}, + }, + }, + }, + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN2}, + }, + }, + }, + } + decisionRequestContext := &mockDecisionRequestContext{ + PEP: struct{ clientID string }{clientID: mockClientID}, + } + + perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) + + s.NoError(err) + s.Equal([][]string{{mockObligationFQN1}, {mockObligationFQN2}}, perResource) + s.ElementsMatch([]string{mockObligationFQN1, mockObligationFQN2}, all) +} + +func (s *ObligationsPDPSuite) Test_UnsupportedResourceType() { + resources := []*authz.Resource{ + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: "unsupported", + }, + }, + } + decisionRequestContext := &mockDecisionRequestContext{} + + _, _, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) + + s.Error(err) +} + +func Test_ObligationsPDPSuite(t *testing.T) { + suite.Run(t, new(ObligationsPDPSuite)) +} From a451615f859f73a562fdbc47e623cba78e54d2c0 Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Tue, 9 Sep 2025 14:57:23 -0700 Subject: [PATCH 05/17] wip --- .../v2/obligations/obligations_pdp_test.go | 82 +++++++++++++++---- 1 file changed, 66 insertions(+), 16 deletions(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go index b76c7c431d..17b54aeac1 100644 --- a/service/internal/access/v2/obligations/obligations_pdp_test.go +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -27,6 +27,10 @@ type ObligationsPDPSuite struct { pdp *ObligationsPolicyDecisionPoint } +func Test_ObligationsPDPSuite(t *testing.T) { + suite.Run(t, new(ObligationsPDPSuite)) +} + func (s *ObligationsPDPSuite) SetupSuite() { // Mock attributes attributesByValueFQN := map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue{ @@ -59,11 +63,11 @@ func (s *ObligationsPDPSuite) SetupSuite() { { Action: mockAction, AttributeValue: &policy.Value{Fqn: mockAttrValFQN2}, - // Context: &policy.ObligationTrigger_Context{ - // Pep: &policy.ObligationTrigger_Context_PEP{ - // ClientId: mockClientID, - // }, - // }, + Context: &policy.ObligationTrigger_Context{ + Pep: &policy.ObligationTrigger_Context_PEP{ + ClientId: mockClientID, + }, + }, }, }, }, @@ -72,11 +76,10 @@ func (s *ObligationsPDPSuite) SetupSuite() { } // Create a new PDP instance - log, err := logger.NewLogger(logger.Config{Output: "stdout", Level: "info"}) - s.Require().NoError(err) + var err error s.pdp, err = NewObligationsPolicyDecisionPoint( - context.Background(), - log, + s.T().Context(), + logger.CreateTestLogger(), attributesByValueFQN, nil, allObligations, @@ -84,12 +87,13 @@ func (s *ObligationsPDPSuite) SetupSuite() { s.Require().NoError(err) } -func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { +func (s *ObligationsPDPSuite) Test_NoObligationsTriggered_AttributeValue() { + nonObligatedAttributeValueFQN := "https://example.org/attr/other/value/val" resources := []*authz.Resource{ { Resource: &authz.Resource_AttributeValues_{ AttributeValues: &authz.Resource_AttributeValues{ - Fqns: []string{"https://example.org/attr/other/value/val"}, + Fqns: []string{nonObligatedAttributeValueFQN}, }, }, }, @@ -99,7 +103,57 @@ func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) s.NoError(err) - s.Equal([][]string{{}}, perResource) + s.Len(perResource, len(resources)) + for _, r := range perResource { + s.Empty(r) + } + s.Empty(all) +} + +func (s *ObligationsPDPSuite) Test_NoObligationsTriggered_Action() { + nonObligatedAction := &policy.Action{Name: "random-name"} + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN1}, + }, + }, + }, + } + decisionRequestContext := &mockDecisionRequestContext{} + + perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), nonObligatedAction, resources, decisionRequestContext) + + s.NoError(err) + s.Len(perResource, len(resources)) + for _, r := range perResource { + s.Empty(r) + } + s.Empty(all) +} +func (s *ObligationsPDPSuite) Test_NoObligationsTriggered_RequestContext() { + unknownClientID := "unknown-client-id" + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN1}, + }, + }, + }, + } + decisionRequestContext := &mockDecisionRequestContext{ + PEP: struct{ clientID string }{clientID: unknownClientID}, + } + + perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) + + s.NoError(err) + s.Len(perResource, len(resources)) + for _, r := range perResource { + s.Empty(r) + } s.Empty(all) } @@ -206,7 +260,3 @@ func (s *ObligationsPDPSuite) Test_UnsupportedResourceType() { s.Error(err) } - -func Test_ObligationsPDPSuite(t *testing.T) { - suite.Run(t, new(ObligationsPDPSuite)) -} From dd1a1c3106f60a4fd3b429577b839e6e39ef2ffc Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Tue, 9 Sep 2025 15:11:48 -0700 Subject: [PATCH 06/17] improve logs, responses, and tests --- .../access/v2/obligations/obligations_pdp.go | 16 +- .../v2/obligations/obligations_pdp_test.go | 165 ++++++++---------- 2 files changed, 85 insertions(+), 96 deletions(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp.go b/service/internal/access/v2/obligations/obligations_pdp.go index 0ad38544a5..50c8f17a89 100644 --- a/service/internal/access/v2/obligations/obligations_pdp.go +++ b/service/internal/access/v2/obligations/obligations_pdp.go @@ -112,25 +112,29 @@ func (p *ObligationsPolicyDecisionPoint) GetRequiredObligations( // Required obligations per resource of a given index requiredOblValueFQNsPerResource := make([][]string, len(resources)) // Set of required obligations across all resources - allRequiredOblValueFQNs := make([]string, 0) + var allRequiredOblValueFQNs []string allOblValFQNsSeen := make(map[string]struct{}) pepClientID := decisionRequestContext.PEP.clientID + actionName := action.GetName() l := p.logger. - With("action", action.GetName()). + With("action", actionName). With("pep_client_id", pepClientID). With("resources_count", strconv.Itoa(len(resources))) // Short-circuit if the requested action and optional scoping clientID are not found within any obligation triggers - attrValueFQNsToObligations, triggersOnActionExist := p.simpleTriggerActionsToAttributes[action.GetName()] + attrValueFQNsToObligations, triggersOnActionExist := p.simpleTriggerActionsToAttributes[actionName] clientScoped, triggersOnClientIDExist := p.clientIDScopedTriggerActionsToAttributes[pepClientID] if triggersOnClientIDExist { - _, triggersOnClientIDExist = clientScoped[action.GetName()] + _, triggersOnClientIDExist = clientScoped[actionName] } if !triggersOnActionExist && !triggersOnClientIDExist { - l.DebugContext(ctx, "no triggered obligations found for action") - return nil, nil, nil + l.DebugContext(ctx, "no triggered obligations found for action", + slog.Any("simple", p.simpleTriggerActionsToAttributes), + slog.Any("client_scoped", p.clientIDScopedTriggerActionsToAttributes), + ) + return requiredOblValueFQNsPerResource, nil, nil } // Traverse trigger lookup graphs to resolve required obligations diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go index 17b54aeac1..222ba5934a 100644 --- a/service/internal/access/v2/obligations/obligations_pdp_test.go +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -57,20 +57,20 @@ func (s *ObligationsPDPSuite) SetupSuite() { }, }, }, - { - Fqn: mockObligationFQN2, - Triggers: []*policy.ObligationTrigger{ - { - Action: mockAction, - AttributeValue: &policy.Value{Fqn: mockAttrValFQN2}, - Context: &policy.ObligationTrigger_Context{ - Pep: &policy.ObligationTrigger_Context_PEP{ - ClientId: mockClientID, - }, - }, - }, - }, - }, + // { + // Fqn: mockObligationFQN2, + // Triggers: []*policy.ObligationTrigger{ + // { + // Action: mockAction, + // AttributeValue: &policy.Value{Fqn: mockAttrValFQN2}, + // Context: &policy.ObligationTrigger_Context{ + // Pep: &policy.ObligationTrigger_Context_PEP{ + // ClientId: mockClientID, + // }, + // }, + // }, + // }, + // }, }, }, } @@ -87,74 +87,79 @@ func (s *ObligationsPDPSuite) SetupSuite() { s.Require().NoError(err) } -func (s *ObligationsPDPSuite) Test_NoObligationsTriggered_AttributeValue() { - nonObligatedAttributeValueFQN := "https://example.org/attr/other/value/val" - resources := []*authz.Resource{ +func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { + type args struct { + action *policy.Action + resources []*authz.Resource + decisionRequestContext *mockDecisionRequestContext + } + tests := []struct { + name string + args args + }{ { - Resource: &authz.Resource_AttributeValues_{ - AttributeValues: &authz.Resource_AttributeValues{ - Fqns: []string{nonObligatedAttributeValueFQN}, + name: "no obligation triggered by attribute value", + args: args{ + action: mockAction, + resources: []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{"https://example.org/attr/other/value/val"}, + }, + }, + }, }, + decisionRequestContext: &mockDecisionRequestContext{}, }, }, - } - decisionRequestContext := &mockDecisionRequestContext{} - - perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) - - s.NoError(err) - s.Len(perResource, len(resources)) - for _, r := range perResource { - s.Empty(r) - } - s.Empty(all) -} - -func (s *ObligationsPDPSuite) Test_NoObligationsTriggered_Action() { - nonObligatedAction := &policy.Action{Name: "random-name"} - resources := []*authz.Resource{ { - Resource: &authz.Resource_AttributeValues_{ - AttributeValues: &authz.Resource_AttributeValues{ - Fqns: []string{mockAttrValFQN1}, + name: "no obligation triggered by action", + args: args{ + action: &policy.Action{Name: "random-name"}, + resources: []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN1}, + }, + }, + }, }, + decisionRequestContext: &mockDecisionRequestContext{}, }, }, - } - decisionRequestContext := &mockDecisionRequestContext{} - - perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), nonObligatedAction, resources, decisionRequestContext) - - s.NoError(err) - s.Len(perResource, len(resources)) - for _, r := range perResource { - s.Empty(r) - } - s.Empty(all) -} -func (s *ObligationsPDPSuite) Test_NoObligationsTriggered_RequestContext() { - unknownClientID := "unknown-client-id" - resources := []*authz.Resource{ { - Resource: &authz.Resource_AttributeValues_{ - AttributeValues: &authz.Resource_AttributeValues{ - Fqns: []string{mockAttrValFQN1}, + name: "no obligation triggered by request context", + args: args{ + action: mockAction, + resources: []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN1}, + }, + }, + }, + }, + decisionRequestContext: &mockDecisionRequestContext{ + PEP: struct{ clientID string }{clientID: "unknown-client-id"}, }, }, }, } - decisionRequestContext := &mockDecisionRequestContext{ - PEP: struct{ clientID string }{clientID: unknownClientID}, - } - - perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) + for _, tt := range tests { + s.T().Run(tt.name, func(t *testing.T) { + perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), tt.args.action, tt.args.resources, tt.args.decisionRequestContext) - s.NoError(err) - s.Len(perResource, len(resources)) - for _, r := range perResource { - s.Empty(r) + s.NoError(err) + s.Len(perResource, len(tt.args.resources)) + for _, r := range perResource { + s.Empty(r) + } + s.Empty(all) + }) } - s.Empty(all) } func (s *ObligationsPDPSuite) Test_SimpleObligationTriggered() { @@ -197,27 +202,6 @@ func (s *ObligationsPDPSuite) Test_ClientScopedObligationTriggered() { s.Equal([]string{mockObligationFQN2}, all) } -func (s *ObligationsPDPSuite) Test_ClientScopedObligationNotTriggered() { - resources := []*authz.Resource{ - { - Resource: &authz.Resource_AttributeValues_{ - AttributeValues: &authz.Resource_AttributeValues{ - Fqns: []string{mockAttrValFQN2}, - }, - }, - }, - } - decisionRequestContext := &mockDecisionRequestContext{ - PEP: struct{ clientID string }{clientID: "different-client-id"}, - } - - perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) - - s.NoError(err) - s.Equal([][]string{{}}, perResource) - s.Empty(all) -} - func (s *ObligationsPDPSuite) Test_MixedObligationsTriggered() { resources := []*authz.Resource{ { @@ -246,11 +230,12 @@ func (s *ObligationsPDPSuite) Test_MixedObligationsTriggered() { s.ElementsMatch([]string{mockObligationFQN1, mockObligationFQN2}, all) } -func (s *ObligationsPDPSuite) Test_UnsupportedResourceType() { +// This fails because we're currently hardcoding the mock pep client ID in new PDP +func (s *ObligationsPDPSuite) Test_UnknownRegisteredResourceValue() { resources := []*authz.Resource{ { Resource: &authz.Resource_RegisteredResourceValueFqn{ - RegisteredResourceValueFqn: "unsupported", + RegisteredResourceValueFqn: "not_found", }, }, } From 8bf761eb09cd8d2a4f68a956555aa8b61152d806 Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Thu, 11 Sep 2025 10:04:56 -0700 Subject: [PATCH 07/17] use real protos instead of mock types --- .../access/v2/obligations/obligations_pdp.go | 36 +++++++----- .../v2/obligations/obligations_pdp_test.go | 58 +++++++++++-------- 2 files changed, 53 insertions(+), 41 deletions(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp.go b/service/internal/access/v2/obligations/obligations_pdp.go index 50c8f17a89..efde7a39eb 100644 --- a/service/internal/access/v2/obligations/obligations_pdp.go +++ b/service/internal/access/v2/obligations/obligations_pdp.go @@ -59,25 +59,29 @@ func NewObligationsPolicyDecisionPoint( for _, trigger := range obligationValue.GetTriggers() { attrValFqn := trigger.GetAttributeValue().GetFqn() actionName := trigger.GetAction().GetName() - optionalPepClientID := "mock-client-id" - // optionalPepClientID := trigger.GetContext().GetPep().GetClientId() - - // If the request context PEP client ID was provided to scope an obligation value to a PEP, populate that lookup graph - if optionalPepClientID != "" { - if _, ok := clientScopedTriggered[optionalPepClientID]; !ok { - clientScopedTriggered[optionalPepClientID] = make(obligationValuesByActionOnAnAttributeValue) - } - if _, ok := clientScopedTriggered[optionalPepClientID][actionName]; !ok { - clientScopedTriggered[optionalPepClientID][actionName] = make(map[string][]string) - } - clientScopedTriggered[optionalPepClientID][actionName][attrValFqn] = append(clientScopedTriggered[optionalPepClientID][actionName][attrValFqn], obligationValue.GetFqn()) - } else { - // Otherwise, populate unscoped lookup graph with just actions and attributes alone + // Populate unscoped lookup graph with just actions and attributes alone + if len(trigger.GetContext()) == 0 { if _, ok := simpleTriggered[actionName]; !ok { simpleTriggered[actionName] = make(map[string][]string) } simpleTriggered[actionName][attrValFqn] = append(simpleTriggered[actionName][attrValFqn], obligationValue.GetFqn()) } + + // If request contexts were provided, PEP client ID was required to scope an obligation value to a PEP, so populate that lookup graph + for _, optionalRequestContext := range trigger.GetContext() { + requiredPEPClientID := optionalRequestContext.GetPep().GetClientId() + + if requiredPEPClientID == "" { + return nil, fmt.Errorf("trigger request context is optional but must contain PEP client ID") + } + if _, ok := clientScopedTriggered[requiredPEPClientID]; !ok { + clientScopedTriggered[requiredPEPClientID] = make(obligationValuesByActionOnAnAttributeValue) + } + if _, ok := clientScopedTriggered[requiredPEPClientID][actionName]; !ok { + clientScopedTriggered[requiredPEPClientID][actionName] = make(map[string][]string) + } + clientScopedTriggered[requiredPEPClientID][actionName][attrValFqn] = append(clientScopedTriggered[requiredPEPClientID][actionName][attrValFqn], obligationValue.GetFqn()) + } } } } @@ -107,7 +111,7 @@ func (p *ObligationsPolicyDecisionPoint) GetRequiredObligations( ctx context.Context, action *policy.Action, resources []*authz.Resource, - decisionRequestContext *mockDecisionRequestContext, + decisionRequestContext *policy.RequestContext, ) ([][]string, []string, error) { // Required obligations per resource of a given index requiredOblValueFQNsPerResource := make([][]string, len(resources)) @@ -115,7 +119,7 @@ func (p *ObligationsPolicyDecisionPoint) GetRequiredObligations( var allRequiredOblValueFQNs []string allOblValFQNsSeen := make(map[string]struct{}) - pepClientID := decisionRequestContext.PEP.clientID + pepClientID := decisionRequestContext.GetPep().GetClientId() actionName := action.GetName() l := p.logger. diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go index 222ba5934a..c6c593722b 100644 --- a/service/internal/access/v2/obligations/obligations_pdp_test.go +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -57,20 +57,22 @@ func (s *ObligationsPDPSuite) SetupSuite() { }, }, }, - // { - // Fqn: mockObligationFQN2, - // Triggers: []*policy.ObligationTrigger{ - // { - // Action: mockAction, - // AttributeValue: &policy.Value{Fqn: mockAttrValFQN2}, - // Context: &policy.ObligationTrigger_Context{ - // Pep: &policy.ObligationTrigger_Context_PEP{ - // ClientId: mockClientID, - // }, - // }, - // }, - // }, - // }, + { + Fqn: mockObligationFQN2, + Triggers: []*policy.ObligationTrigger{ + { + Action: mockAction, + AttributeValue: &policy.Value{Fqn: mockAttrValFQN2}, + Context: []*policy.RequestContext{ + { + Pep: &policy.PolicyEnforcementPoint{ + ClientId: mockClientID, + }, + }, + }, + }, + }, + }, }, }, } @@ -91,7 +93,7 @@ func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { type args struct { action *policy.Action resources []*authz.Resource - decisionRequestContext *mockDecisionRequestContext + decisionRequestContext *policy.RequestContext } tests := []struct { name string @@ -110,7 +112,7 @@ func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { }, }, }, - decisionRequestContext: &mockDecisionRequestContext{}, + decisionRequestContext: &policy.RequestContext{}, }, }, { @@ -126,7 +128,7 @@ func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { }, }, }, - decisionRequestContext: &mockDecisionRequestContext{}, + decisionRequestContext: &policy.RequestContext{}, }, }, { @@ -142,8 +144,10 @@ func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { }, }, }, - decisionRequestContext: &mockDecisionRequestContext{ - PEP: struct{ clientID string }{clientID: "unknown-client-id"}, + decisionRequestContext: &policy.RequestContext{ + Pep: &policy.PolicyEnforcementPoint{ + ClientId: "unknown-client-id", + }, }, }, }, @@ -172,7 +176,7 @@ func (s *ObligationsPDPSuite) Test_SimpleObligationTriggered() { }, }, } - decisionRequestContext := &mockDecisionRequestContext{} + decisionRequestContext := &policy.RequestContext{} perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) @@ -191,8 +195,10 @@ func (s *ObligationsPDPSuite) Test_ClientScopedObligationTriggered() { }, }, } - decisionRequestContext := &mockDecisionRequestContext{ - PEP: struct{ clientID string }{clientID: mockClientID}, + decisionRequestContext := &policy.RequestContext{ + Pep: &policy.PolicyEnforcementPoint{ + ClientId: mockClientID, + }, } perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) @@ -219,8 +225,10 @@ func (s *ObligationsPDPSuite) Test_MixedObligationsTriggered() { }, }, } - decisionRequestContext := &mockDecisionRequestContext{ - PEP: struct{ clientID string }{clientID: mockClientID}, + decisionRequestContext := &policy.RequestContext{ + Pep: &policy.PolicyEnforcementPoint{ + ClientId: mockClientID, + }, } perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) @@ -239,7 +247,7 @@ func (s *ObligationsPDPSuite) Test_UnknownRegisteredResourceValue() { }, }, } - decisionRequestContext := &mockDecisionRequestContext{} + decisionRequestContext := &policy.RequestContext{} _, _, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) From 5e9915b43a3021906d81344dbe8aa441b018148e Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Mon, 15 Sep 2025 15:16:12 -0700 Subject: [PATCH 08/17] test and lint fixes --- .../access/v2/obligations/obligations_pdp.go | 1 + .../v2/obligations/obligations_pdp_test.go | 80 +++++++++---------- 2 files changed, 39 insertions(+), 42 deletions(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp.go b/service/internal/access/v2/obligations/obligations_pdp.go index efde7a39eb..f86634e7f2 100644 --- a/service/internal/access/v2/obligations/obligations_pdp.go +++ b/service/internal/access/v2/obligations/obligations_pdp.go @@ -16,6 +16,7 @@ import ( // i.e. read : https://example.org/attr/attr1/value/val1 : [https://example.org/obl/some_obligation/value/some_value] type obligationValuesByActionOnAnAttributeValue map[string]map[string][]string +//nolint:revive // There are a growing number of PDP types, so keep the naming verbose type ObligationsPolicyDecisionPoint struct { logger *logger.Logger attributesByValueFQN map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go index c6c593722b..452f28d077 100644 --- a/service/internal/access/v2/obligations/obligations_pdp_test.go +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -1,7 +1,6 @@ package obligations import ( - "context" "testing" "github.com/stretchr/testify/suite" @@ -15,6 +14,7 @@ import ( const ( mockAttrValFQN1 = "https://example.org/attr/attr1/value/val1" mockAttrValFQN2 = "https://example.org/attr/attr2/value/val2" + mockAttrValFQN3 = "https://example.org/attr/attr2/value/val3" mockObligationFQN1 = "https://example.org/obl/some_obligation/value/some_value" mockObligationFQN2 = "https://example.org/obl/another_obligation/value/another_value" mockClientID = "mock-client-id" @@ -22,6 +22,8 @@ const ( var mockAction = &policy.Action{Name: "read"} +// TODO: registered resources + type ObligationsPDPSuite struct { suite.Suite pdp *ObligationsPolicyDecisionPoint @@ -42,12 +44,17 @@ func (s *ObligationsPDPSuite) SetupSuite() { Attribute: &policy.Attribute{Name: "attr2"}, Value: &policy.Value{Fqn: mockAttrValFQN2}, }, + mockAttrValFQN3: { + Attribute: &policy.Attribute{Name: "attr2"}, + Value: &policy.Value{Fqn: mockAttrValFQN3}, + }, } // Mock obligations allObligations := []*policy.Obligation{ { Values: []*policy.ObligationValue{ + // No client PEP scope { Fqn: mockObligationFQN1, Triggers: []*policy.ObligationTrigger{ @@ -57,6 +64,7 @@ func (s *ObligationsPDPSuite) SetupSuite() { }, }, }, + // Scoped to the mockClientID PEP { Fqn: mockObligationFQN2, Triggers: []*policy.ObligationTrigger{ @@ -100,25 +108,24 @@ func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { args args }{ { - name: "no obligation triggered by attribute value", + name: "no obligation triggered by known but unobligated attribute value", args: args{ action: mockAction, resources: []*authz.Resource{ { Resource: &authz.Resource_AttributeValues_{ AttributeValues: &authz.Resource_AttributeValues{ - Fqns: []string{"https://example.org/attr/other/value/val"}, + Fqns: []string{mockAttrValFQN3}, }, }, }, }, - decisionRequestContext: &policy.RequestContext{}, }, }, { - name: "no obligation triggered by action", + name: "no obligation triggered by unobligated action", args: args{ - action: &policy.Action{Name: "random-name"}, + action: &policy.Action{Name: "random-action-name"}, resources: []*authz.Resource{ { Resource: &authz.Resource_AttributeValues_{ @@ -128,36 +135,15 @@ func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { }, }, }, - decisionRequestContext: &policy.RequestContext{}, - }, - }, - { - name: "no obligation triggered by request context", - args: args{ - action: mockAction, - resources: []*authz.Resource{ - { - Resource: &authz.Resource_AttributeValues_{ - AttributeValues: &authz.Resource_AttributeValues{ - Fqns: []string{mockAttrValFQN1}, - }, - }, - }, - }, - decisionRequestContext: &policy.RequestContext{ - Pep: &policy.PolicyEnforcementPoint{ - ClientId: "unknown-client-id", - }, - }, }, }, } for _, tt := range tests { s.T().Run(tt.name, func(t *testing.T) { - perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), tt.args.action, tt.args.resources, tt.args.decisionRequestContext) - + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), tt.args.action, tt.args.resources, tt.args.decisionRequestContext) s.NoError(err) s.Len(perResource, len(tt.args.resources)) + for _, r := range perResource { s.Empty(r) } @@ -166,7 +152,7 @@ func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { } } -func (s *ObligationsPDPSuite) Test_SimpleObligationTriggered() { +func (s *ObligationsPDPSuite) Test_SimpleObligation_NoRequestContextPEP_Triggered() { resources := []*authz.Resource{ { Resource: &authz.Resource_AttributeValues_{ @@ -178,14 +164,14 @@ func (s *ObligationsPDPSuite) Test_SimpleObligationTriggered() { } decisionRequestContext := &policy.RequestContext{} - perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), mockAction, resources, decisionRequestContext) s.NoError(err) s.Equal([][]string{{mockObligationFQN1}}, perResource) s.Equal([]string{mockObligationFQN1}, all) } -func (s *ObligationsPDPSuite) Test_ClientScopedObligationTriggered() { +func (s *ObligationsPDPSuite) Test_ClientScopedObligation_Triggered() { resources := []*authz.Resource{ { Resource: &authz.Resource_AttributeValues_{ @@ -201,14 +187,14 @@ func (s *ObligationsPDPSuite) Test_ClientScopedObligationTriggered() { }, } - perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), mockAction, resources, decisionRequestContext) s.NoError(err) s.Equal([][]string{{mockObligationFQN2}}, perResource) s.Equal([]string{mockObligationFQN2}, all) } -func (s *ObligationsPDPSuite) Test_MixedObligationsTriggered() { +func (s *ObligationsPDPSuite) Test_MixedObligations_Triggered() { resources := []*authz.Resource{ { Resource: &authz.Resource_AttributeValues_{ @@ -224,6 +210,13 @@ func (s *ObligationsPDPSuite) Test_MixedObligationsTriggered() { }, }, }, + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN1, mockAttrValFQN2}, + }, + }, + }, } decisionRequestContext := &policy.RequestContext{ Pep: &policy.PolicyEnforcementPoint{ @@ -231,25 +224,28 @@ func (s *ObligationsPDPSuite) Test_MixedObligationsTriggered() { }, } - perResource, all, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) - + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), mockAction, resources, decisionRequestContext) s.NoError(err) - s.Equal([][]string{{mockObligationFQN1}, {mockObligationFQN2}}, perResource) + // Obligations in order of resources: unscoped, scoped, both + s.Equal([][]string{{mockObligationFQN1}, {mockObligationFQN2}, {mockObligationFQN1, mockObligationFQN2}}, perResource) + // Deduplicated obligations s.ElementsMatch([]string{mockObligationFQN1, mockObligationFQN2}, all) } -// This fails because we're currently hardcoding the mock pep client ID in new PDP -func (s *ObligationsPDPSuite) Test_UnknownRegisteredResourceValue() { +func (s *ObligationsPDPSuite) Test_UnknownRegisteredResourceValue_Fails() { + badRegResValFQN := "https://reg_res/not_found_reg_res" resources := []*authz.Resource{ { Resource: &authz.Resource_RegisteredResourceValueFqn{ - RegisteredResourceValueFqn: "not_found", + RegisteredResourceValueFqn: badRegResValFQN, }, }, } decisionRequestContext := &policy.RequestContext{} - _, _, err := s.pdp.GetRequiredObligations(context.Background(), mockAction, resources, decisionRequestContext) - + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), mockAction, resources, decisionRequestContext) s.Error(err) + s.Empty(perResource) + s.Empty(all) + s.Contains(err.Error(), badRegResValFQN) } From 86144f735d99eb42b32d230f87d2c90001348e84 Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Mon, 15 Sep 2025 15:17:06 -0700 Subject: [PATCH 09/17] lint --- service/internal/access/v2/obligations/obligations_pdp_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go index 452f28d077..88c60e1694 100644 --- a/service/internal/access/v2/obligations/obligations_pdp_test.go +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -140,7 +140,7 @@ func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { } for _, tt := range tests { s.T().Run(tt.name, func(t *testing.T) { - perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), tt.args.action, tt.args.resources, tt.args.decisionRequestContext) + perResource, all, err := s.pdp.GetRequiredObligations(t.Context(), tt.args.action, tt.args.resources, tt.args.decisionRequestContext) s.NoError(err) s.Len(perResource, len(tt.args.resources)) From 528e1adcbea1e8465698ba5812299d5e8202fe6c Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Mon, 15 Sep 2025 15:25:37 -0700 Subject: [PATCH 10/17] more lint fixes and shadow var fixes --- service/internal/access/v2/evaluate_test.go | 4 ++-- .../access/v2/obligations/obligations_pdp.go | 21 +++++++++++-------- .../v2/obligations/obligations_pdp_test.go | 10 ++++----- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/service/internal/access/v2/evaluate_test.go b/service/internal/access/v2/evaluate_test.go index 791502db23..468f225aa9 100644 --- a/service/internal/access/v2/evaluate_test.go +++ b/service/internal/access/v2/evaluate_test.go @@ -701,7 +701,7 @@ func (s *EvaluateTestSuite) TestEvaluateDefinition() { result, err := evaluateDefinition(s.T().Context(), s.logger, tc.entitlements, s.action, tc.resourceValues, tc.definition) if tc.expectError { - s.Error(err) + s.Require().Error(err) } else { s.Require().NoError(err) s.NotNil(result) @@ -933,7 +933,7 @@ func (s *EvaluateTestSuite) TestGetResourceDecision() { ) if tc.expectError { - s.Error(err) + s.Require().Error(err) } else { s.Require().NoError(err) s.NotNil(decision) diff --git a/service/internal/access/v2/obligations/obligations_pdp.go b/service/internal/access/v2/obligations/obligations_pdp.go index f86634e7f2..1f8543bae7 100644 --- a/service/internal/access/v2/obligations/obligations_pdp.go +++ b/service/internal/access/v2/obligations/obligations_pdp.go @@ -2,6 +2,7 @@ package obligations import ( "context" + "errors" "fmt" "log/slog" "strconv" @@ -12,6 +13,8 @@ import ( "github.com/opentdf/platform/service/logger" ) +// TODO: named errors + // A graph of action names to attribute value FQNs to lists of obligation value FQNs // i.e. read : https://example.org/attr/attr1/value/val1 : [https://example.org/obl/some_obligation/value/some_value] type obligationValuesByActionOnAnAttributeValue map[string]map[string][]string @@ -73,7 +76,7 @@ func NewObligationsPolicyDecisionPoint( requiredPEPClientID := optionalRequestContext.GetPep().GetClientId() if requiredPEPClientID == "" { - return nil, fmt.Errorf("trigger request context is optional but must contain PEP client ID") + return nil, errors.New("trigger request context is optional but must contain PEP client ID") } if _, ok := clientScopedTriggered[requiredPEPClientID]; !ok { clientScopedTriggered[requiredPEPClientID] = make(obligationValuesByActionOnAnAttributeValue) @@ -156,9 +159,9 @@ func (p *ObligationsPolicyDecisionPoint) GetRequiredObligations( // Check the action-attribute-values associated with a Registered Resource Value for a match to the request action for _, aav := range regResValue.GetActionAttributeValues() { - actionName := aav.GetAction().GetName() + aavActionName := aav.GetAction().GetName() attrValFQN := aav.GetAttributeValue().GetFqn() - if actionName != action.GetName() { + if aavActionName != actionName { continue } attrValueFQNs = append(attrValueFQNs, attrValFQN) @@ -175,9 +178,9 @@ func (p *ObligationsPolicyDecisionPoint) GetRequiredObligations( seenThisResource := make(map[string]struct{}) resourceRequiredOblValueFQNsSet := make([]string, 0) for _, attrValFQN := range attrValueFQNs { - if triggered, ok := attrValueFQNsToObligations[attrValFQN]; ok { - for _, oblValFQN := range triggered { - if _, ok := seenThisResource[oblValFQN]; ok { + if triggeredObligations, someTriggered := attrValueFQNsToObligations[attrValFQN]; someTriggered { + for _, oblValFQN := range triggeredObligations { + if _, seen := seenThisResource[oblValFQN]; seen { continue } // Update set of obligations triggered for this specific resource @@ -192,9 +195,9 @@ func (p *ObligationsPolicyDecisionPoint) GetRequiredObligations( } } - if triggered, ok := p.clientIDScopedTriggerActionsToAttributes[pepClientID][action.GetName()][attrValFQN]; ok { - for _, oblValFQN := range triggered { - if _, ok := seenThisResource[oblValFQN]; ok { + if triggeredObligations, someTriggered := p.clientIDScopedTriggerActionsToAttributes[pepClientID][actionName][attrValFQN]; someTriggered { + for _, oblValFQN := range triggeredObligations { + if _, seen := seenThisResource[oblValFQN]; seen { continue } // Update set of obligations triggered for this specific resource diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go index 88c60e1694..61e11d7018 100644 --- a/service/internal/access/v2/obligations/obligations_pdp_test.go +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -141,7 +141,7 @@ func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { for _, tt := range tests { s.T().Run(tt.name, func(t *testing.T) { perResource, all, err := s.pdp.GetRequiredObligations(t.Context(), tt.args.action, tt.args.resources, tt.args.decisionRequestContext) - s.NoError(err) + s.Require().NoError(err) s.Len(perResource, len(tt.args.resources)) for _, r := range perResource { @@ -166,7 +166,7 @@ func (s *ObligationsPDPSuite) Test_SimpleObligation_NoRequestContextPEP_Triggere perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), mockAction, resources, decisionRequestContext) - s.NoError(err) + s.Require().NoError(err) s.Equal([][]string{{mockObligationFQN1}}, perResource) s.Equal([]string{mockObligationFQN1}, all) } @@ -189,7 +189,7 @@ func (s *ObligationsPDPSuite) Test_ClientScopedObligation_Triggered() { perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), mockAction, resources, decisionRequestContext) - s.NoError(err) + s.Require().NoError(err) s.Equal([][]string{{mockObligationFQN2}}, perResource) s.Equal([]string{mockObligationFQN2}, all) } @@ -225,7 +225,7 @@ func (s *ObligationsPDPSuite) Test_MixedObligations_Triggered() { } perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), mockAction, resources, decisionRequestContext) - s.NoError(err) + s.Require().NoError(err) // Obligations in order of resources: unscoped, scoped, both s.Equal([][]string{{mockObligationFQN1}, {mockObligationFQN2}, {mockObligationFQN1, mockObligationFQN2}}, perResource) // Deduplicated obligations @@ -244,7 +244,7 @@ func (s *ObligationsPDPSuite) Test_UnknownRegisteredResourceValue_Fails() { decisionRequestContext := &policy.RequestContext{} perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), mockAction, resources, decisionRequestContext) - s.Error(err) + s.Require().Error(err) s.Empty(perResource) s.Empty(all) s.Contains(err.Error(), badRegResValFQN) From b605ce6e6059e4d976e46585d2563383dcafc4df Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Tue, 16 Sep 2025 12:16:33 -0700 Subject: [PATCH 11/17] new obligations PDP tests --- .../v2/obligations/obligations_pdp_test.go | 166 ++++++++++++++++-- 1 file changed, 153 insertions(+), 13 deletions(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go index 61e11d7018..21c2f8eecc 100644 --- a/service/internal/access/v2/obligations/obligations_pdp_test.go +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -20,7 +20,10 @@ const ( mockClientID = "mock-client-id" ) -var mockAction = &policy.Action{Name: "read"} +var ( + actionNameRead = "read" + actionRead = &policy.Action{Name: actionNameRead} +) // TODO: registered resources @@ -59,7 +62,7 @@ func (s *ObligationsPDPSuite) SetupSuite() { Fqn: mockObligationFQN1, Triggers: []*policy.ObligationTrigger{ { - Action: mockAction, + Action: actionRead, AttributeValue: &policy.Value{Fqn: mockAttrValFQN1}, }, }, @@ -69,7 +72,7 @@ func (s *ObligationsPDPSuite) SetupSuite() { Fqn: mockObligationFQN2, Triggers: []*policy.ObligationTrigger{ { - Action: mockAction, + Action: actionRead, AttributeValue: &policy.Value{Fqn: mockAttrValFQN2}, Context: []*policy.RequestContext{ { @@ -97,7 +100,109 @@ func (s *ObligationsPDPSuite) SetupSuite() { s.Require().NoError(err) } -func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { +func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_Success() { + attributesByValueFQN := s.createAttributesByValueFQN(mockAttrValFQN1, "attr1") + var noClientID string + allObligations := []*policy.Obligation{s.createObligation(mockObligationFQN1, mockAttrValFQN1, noClientID, actionRead)} + + pdp, err := NewObligationsPolicyDecisionPoint( + s.T().Context(), + logger.CreateTestLogger(), + attributesByValueFQN, + nil, + allObligations, + ) + + s.Require().NoError(err) + s.NotNil(pdp) + s.NotNil(pdp.logger) + s.Equal(attributesByValueFQN, pdp.attributesByValueFQN) + s.Empty(pdp.registeredResourceValuesByFQN) + s.Len(pdp.obligationValuesByFQN, 1) + s.Contains(pdp.obligationValuesByFQN, mockObligationFQN1) + s.NotNil(pdp.simpleTriggerActionsToAttributes) + s.NotNil(pdp.clientIDScopedTriggerActionsToAttributes) +} + +func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_WithClientScoped() { + attributesByValueFQN := s.createAttributesByValueFQN(mockAttrValFQN2, "attr2") + allObligations := []*policy.Obligation{s.createObligation(mockObligationFQN2, mockAttrValFQN2, mockClientID, actionRead)} + + pdp, err := NewObligationsPolicyDecisionPoint( + s.T().Context(), + logger.CreateTestLogger(), + attributesByValueFQN, + nil, + allObligations, + ) + + s.Require().NoError(err) + s.NotNil(pdp) + s.Len(pdp.obligationValuesByFQN, 1) + s.Contains(pdp.obligationValuesByFQN, mockObligationFQN2) + s.Contains(pdp.clientIDScopedTriggerActionsToAttributes, mockClientID) + s.Contains(pdp.clientIDScopedTriggerActionsToAttributes[mockClientID], actionNameRead) + s.Contains(pdp.clientIDScopedTriggerActionsToAttributes[mockClientID][actionNameRead], mockAttrValFQN2) +} + +func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_EmptyClientID_Fails() { + attributesByValueFQN := s.createAttributesByValueFQN(mockAttrValFQN1, "attr1") + + // Create obligation with empty client ID using special case + allObligations := []*policy.Obligation{ + { + Values: []*policy.ObligationValue{ + { + Fqn: mockObligationFQN1, + Triggers: []*policy.ObligationTrigger{ + { + Action: actionRead, + AttributeValue: &policy.Value{Fqn: mockAttrValFQN1}, + Context: []*policy.RequestContext{ + { + Pep: &policy.PolicyEnforcementPoint{ + ClientId: "", + }, + }, + }, + }, + }, + }, + }, + }, + } + + pdp, err := NewObligationsPolicyDecisionPoint( + s.T().Context(), + logger.CreateTestLogger(), + attributesByValueFQN, + nil, + allObligations, + ) + + s.Require().Error(err) + s.Nil(pdp) +} + +func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_EmptyObligations() { + attributesByValueFQN := s.createAttributesByValueFQN(mockAttrValFQN1, "attr1") + + pdp, err := NewObligationsPolicyDecisionPoint( + s.T().Context(), + logger.CreateTestLogger(), + attributesByValueFQN, + nil, + []*policy.Obligation{}, + ) + + s.Require().NoError(err) + s.NotNil(pdp) + s.Empty(pdp.obligationValuesByFQN) + s.Empty(pdp.simpleTriggerActionsToAttributes) + s.Empty(pdp.clientIDScopedTriggerActionsToAttributes) +} + +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_NoObligationsTriggered() { type args struct { action *policy.Action resources []*authz.Resource @@ -110,7 +215,7 @@ func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { { name: "no obligation triggered by known but unobligated attribute value", args: args{ - action: mockAction, + action: actionRead, resources: []*authz.Resource{ { Resource: &authz.Resource_AttributeValues_{ @@ -152,7 +257,7 @@ func (s *ObligationsPDPSuite) Test_NoObligationsTriggered() { } } -func (s *ObligationsPDPSuite) Test_SimpleObligation_NoRequestContextPEP_Triggered() { +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_SimpleObligation_NoRequestContextPEP_Triggered() { resources := []*authz.Resource{ { Resource: &authz.Resource_AttributeValues_{ @@ -164,14 +269,14 @@ func (s *ObligationsPDPSuite) Test_SimpleObligation_NoRequestContextPEP_Triggere } decisionRequestContext := &policy.RequestContext{} - perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), mockAction, resources, decisionRequestContext) + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) s.Require().NoError(err) s.Equal([][]string{{mockObligationFQN1}}, perResource) s.Equal([]string{mockObligationFQN1}, all) } -func (s *ObligationsPDPSuite) Test_ClientScopedObligation_Triggered() { +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_ClientScopedObligation_Triggered() { resources := []*authz.Resource{ { Resource: &authz.Resource_AttributeValues_{ @@ -187,14 +292,14 @@ func (s *ObligationsPDPSuite) Test_ClientScopedObligation_Triggered() { }, } - perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), mockAction, resources, decisionRequestContext) + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) s.Require().NoError(err) s.Equal([][]string{{mockObligationFQN2}}, perResource) s.Equal([]string{mockObligationFQN2}, all) } -func (s *ObligationsPDPSuite) Test_MixedObligations_Triggered() { +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_MixedObligations_Triggered() { resources := []*authz.Resource{ { Resource: &authz.Resource_AttributeValues_{ @@ -224,7 +329,7 @@ func (s *ObligationsPDPSuite) Test_MixedObligations_Triggered() { }, } - perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), mockAction, resources, decisionRequestContext) + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) s.Require().NoError(err) // Obligations in order of resources: unscoped, scoped, both s.Equal([][]string{{mockObligationFQN1}, {mockObligationFQN2}, {mockObligationFQN1, mockObligationFQN2}}, perResource) @@ -232,7 +337,7 @@ func (s *ObligationsPDPSuite) Test_MixedObligations_Triggered() { s.ElementsMatch([]string{mockObligationFQN1, mockObligationFQN2}, all) } -func (s *ObligationsPDPSuite) Test_UnknownRegisteredResourceValue_Fails() { +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_UnknownRegisteredResourceValue_Fails() { badRegResValFQN := "https://reg_res/not_found_reg_res" resources := []*authz.Resource{ { @@ -243,9 +348,44 @@ func (s *ObligationsPDPSuite) Test_UnknownRegisteredResourceValue_Fails() { } decisionRequestContext := &policy.RequestContext{} - perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), mockAction, resources, decisionRequestContext) + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) s.Require().Error(err) s.Empty(perResource) s.Empty(all) s.Contains(err.Error(), badRegResValFQN) } + +func (s *ObligationsPDPSuite) createAttributesByValueFQN(attrValFQN, attrName string) map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue { + return map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue{ + attrValFQN: { + Attribute: &policy.Attribute{Name: attrName}, + Value: &policy.Value{Fqn: attrValFQN}, + }, + } +} + +func (s *ObligationsPDPSuite) createObligation(oblFQN, attrValFQN, clientID string, action *policy.Action) *policy.Obligation { + trigger := &policy.ObligationTrigger{ + Action: action, + AttributeValue: &policy.Value{Fqn: attrValFQN}, + } + + if clientID != "" { + trigger.Context = []*policy.RequestContext{ + { + Pep: &policy.PolicyEnforcementPoint{ + ClientId: clientID, + }, + }, + } + } + + return &policy.Obligation{ + Values: []*policy.ObligationValue{ + { + Fqn: oblFQN, + Triggers: []*policy.ObligationTrigger{trigger}, + }, + }, + } +} From ebe3c82343fbc56f7748e765d3440d4343aca03e Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Tue, 16 Sep 2025 12:26:00 -0700 Subject: [PATCH 12/17] add another action --- .../v2/obligations/obligations_pdp_test.go | 97 ++++++++++++++++++- 1 file changed, 93 insertions(+), 4 deletions(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go index 21c2f8eecc..ed6944f9bf 100644 --- a/service/internal/access/v2/obligations/obligations_pdp_test.go +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -17,12 +17,15 @@ const ( mockAttrValFQN3 = "https://example.org/attr/attr2/value/val3" mockObligationFQN1 = "https://example.org/obl/some_obligation/value/some_value" mockObligationFQN2 = "https://example.org/obl/another_obligation/value/another_value" + mockObligationFQN3 = "https://example.org/obl/create_obligation/value/create_value" mockClientID = "mock-client-id" + actionNameRead = "read" + actionNameCreate = "create" ) var ( - actionNameRead = "read" - actionRead = &policy.Action{Name: actionNameRead} + actionRead = &policy.Action{Name: actionNameRead} + actionCreate = &policy.Action{Name: actionNameCreate} ) // TODO: registered resources @@ -57,7 +60,7 @@ func (s *ObligationsPDPSuite) SetupSuite() { allObligations := []*policy.Obligation{ { Values: []*policy.ObligationValue{ - // No client PEP scope + // No client PEP scope - triggered by 'read' action { Fqn: mockObligationFQN1, Triggers: []*policy.ObligationTrigger{ @@ -67,7 +70,7 @@ func (s *ObligationsPDPSuite) SetupSuite() { }, }, }, - // Scoped to the mockClientID PEP + // Scoped to the mockClientID PEP - triggered by 'read' action { Fqn: mockObligationFQN2, Triggers: []*policy.ObligationTrigger{ @@ -84,6 +87,16 @@ func (s *ObligationsPDPSuite) SetupSuite() { }, }, }, + // No client PEP scope - triggered by 'create' action + { + Fqn: mockObligationFQN3, + Triggers: []*policy.ObligationTrigger{ + { + Action: actionCreate, + AttributeValue: &policy.Value{Fqn: mockAttrValFQN1}, + }, + }, + }, }, }, } @@ -355,6 +368,82 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_UnknownRegisteredResou s.Contains(err.Error(), badRegResValFQN) } +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_CreateAction_SimpleObligation_Triggered() { + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN1}, + }, + }, + }, + } + decisionRequestContext := &policy.RequestContext{} + + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionCreate, resources, decisionRequestContext) + + s.Require().NoError(err) + s.Equal([][]string{{mockObligationFQN3}}, perResource) + s.Equal([]string{mockObligationFQN3}, all) +} + +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_CreateAction_NoObligationsTriggered() { + // Test that 'create' action doesn't trigger 'read' obligations + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN2}, + }, + }, + }, + } + decisionRequestContext := &policy.RequestContext{ + Pep: &policy.PolicyEnforcementPoint{ + ClientId: mockClientID, + }, + } + + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionCreate, resources, decisionRequestContext) + + s.Require().NoError(err) + // No create obligations exist for mockAttrValFQN2, so nothing should be triggered + s.Len(perResource, len(resources)) + for _, r := range perResource { + s.Empty(r) + } + s.Empty(all) +} + +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_ReadVsCreateAction_DifferentObligationsTriggered() { + // Test the same resource with both actions to verify action-specific filtering + resources := []*authz.Resource{ + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN1}, + }, + }, + }, + } + decisionRequestContext := &policy.RequestContext{} + + // Test with 'read' action - should trigger read obligation + perResourceRead, allRead, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) + s.Require().NoError(err) + s.Equal([][]string{{mockObligationFQN1}}, perResourceRead) + s.Equal([]string{mockObligationFQN1}, allRead) + + // Test with 'create' action - should trigger create obligation + perResourceCreate, allCreate, err := s.pdp.GetRequiredObligations(s.T().Context(), actionCreate, resources, decisionRequestContext) + s.Require().NoError(err) + s.Equal([][]string{{mockObligationFQN3}}, perResourceCreate) + s.Equal([]string{mockObligationFQN3}, allCreate) + + // Verify the obligations are different + s.NotEqual(allRead, allCreate) +} + func (s *ObligationsPDPSuite) createAttributesByValueFQN(attrValFQN, attrName string) map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue { return map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue{ attrValFQN: { From 6ed8f03e137e372a70015dc732c30d827e95bc5a Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Tue, 16 Sep 2025 13:08:46 -0700 Subject: [PATCH 13/17] registered resource tests as well --- .../v2/obligations/obligations_pdp_test.go | 307 +++++++++++++++++- 1 file changed, 300 insertions(+), 7 deletions(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go index ed6944f9bf..916edc70f5 100644 --- a/service/internal/access/v2/obligations/obligations_pdp_test.go +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -12,20 +12,30 @@ import ( ) const ( - mockAttrValFQN1 = "https://example.org/attr/attr1/value/val1" - mockAttrValFQN2 = "https://example.org/attr/attr2/value/val2" - mockAttrValFQN3 = "https://example.org/attr/attr2/value/val3" + mockAttrValFQN1 = "https://example.org/attr/attr1/value/val1" + mockAttrValFQN2 = "https://example.org/attr/attr2/value/val2" + mockAttrValFQN3 = "https://example.org/attr/attr2/value/val3" + mockObligationFQN1 = "https://example.org/obl/some_obligation/value/some_value" mockObligationFQN2 = "https://example.org/obl/another_obligation/value/another_value" mockObligationFQN3 = "https://example.org/obl/create_obligation/value/create_value" - mockClientID = "mock-client-id" - actionNameRead = "read" - actionNameCreate = "create" + mockObligationFQN4 = "https://example.org/obl/custom_obligation/value/custom_value" + + mockRegResValFQN1 = "https://example.org/reg_res/resource1/value/val1" + mockRegResValFQN2 = "https://example.org/reg_res/resource2/value/val2" + mockRegResValFQN3 = "https://example.org/reg_res/resource2/value/val3" + + mockClientID = "mock-client-id" + + actionNameRead = "read" + actionNameCreate = "create" + actionNameCustom = "custom_action" ) var ( actionRead = &policy.Action{Name: actionNameRead} actionCreate = &policy.Action{Name: actionNameCreate} + actionCustom = &policy.Action{Name: actionNameCustom} ) // TODO: registered resources @@ -56,6 +66,41 @@ func (s *ObligationsPDPSuite) SetupSuite() { }, } + // Mock registered resources + registeredResourceValuesByFQN := map[string]*policy.RegisteredResourceValue{ + mockRegResValFQN1: { + Value: "val1", + ActionAttributeValues: []*policy.RegisteredResourceValue_ActionAttributeValue{ + { + Action: actionRead, + AttributeValue: &policy.Value{Fqn: mockAttrValFQN1}, + }, + { + Action: actionCreate, + AttributeValue: &policy.Value{Fqn: mockAttrValFQN1}, + }, + }, + }, + mockRegResValFQN2: { + Value: "val2", + ActionAttributeValues: []*policy.RegisteredResourceValue_ActionAttributeValue{ + { + Action: actionRead, + AttributeValue: &policy.Value{Fqn: mockAttrValFQN2}, + }, + }, + }, + mockRegResValFQN3: { + Value: "val3", + ActionAttributeValues: []*policy.RegisteredResourceValue_ActionAttributeValue{ + { + Action: actionCustom, + AttributeValue: &policy.Value{Fqn: mockAttrValFQN3}, + }, + }, + }, + } + // Mock obligations allObligations := []*policy.Obligation{ { @@ -97,6 +142,16 @@ func (s *ObligationsPDPSuite) SetupSuite() { }, }, }, + // No client PEP scope - triggered by 'custom' action + { + Fqn: mockObligationFQN4, + Triggers: []*policy.ObligationTrigger{ + { + Action: actionCustom, + AttributeValue: &policy.Value{Fqn: mockAttrValFQN3}, + }, + }, + }, }, }, } @@ -107,7 +162,7 @@ func (s *ObligationsPDPSuite) SetupSuite() { s.T().Context(), logger.CreateTestLogger(), attributesByValueFQN, - nil, + registeredResourceValuesByFQN, allObligations, ) s.Require().NoError(err) @@ -444,6 +499,244 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_ReadVsCreateAction_Dif s.NotEqual(allRead, allCreate) } +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_RegisteredResource_ReadAction_Triggered() { + resources := []*authz.Resource{ + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: mockRegResValFQN1, + }, + }, + } + decisionRequestContext := &policy.RequestContext{} + + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) + + s.Require().NoError(err) + s.Require().Len(perResource, 1, "should have obligations for exactly one resource") + s.Require().Len(perResource[0], 1, "should have exactly one obligation for the resource") + s.Equal(mockObligationFQN1, perResource[0][0]) + s.Require().Len(all, 1, "should have exactly one obligation total") + s.Contains(all, mockObligationFQN1) +} + +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_RegisteredResource_CreateAction_Triggered() { + resources := []*authz.Resource{ + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: mockRegResValFQN1, + }, + }, + } + decisionRequestContext := &policy.RequestContext{} + + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionCreate, resources, decisionRequestContext) + + s.Require().NoError(err) + s.Require().Len(perResource, 1, "should have obligations for exactly one resource") + s.Require().Len(perResource[0], 1, "should have exactly one obligation for the resource") + s.Equal(mockObligationFQN3, perResource[0][0]) + s.Require().Len(all, 1, "should have exactly one obligation total") + s.Contains(all, mockObligationFQN3) +} + +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_RegisteredResource_NoCreateAction_NoObligationsTriggered() { + // Use mockRegResValFQN2 which only has read action, not create + resources := []*authz.Resource{ + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: mockRegResValFQN2, + }, + }, + } + decisionRequestContext := &policy.RequestContext{ + Pep: &policy.PolicyEnforcementPoint{ + ClientId: mockClientID, + }, + } + + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionCreate, resources, decisionRequestContext) + + s.Require().NoError(err) + s.Len(perResource, len(resources)) + for _, r := range perResource { + s.Empty(r, "no obligations should be triggered for create action on read-only registered resource") + } + s.Empty(all) +} + +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_RegisteredResource_ClientScoped_Triggered() { + // Use mockRegResValFQN2 which maps to mockAttrValFQN2 (has client-scoped read obligation) + resources := []*authz.Resource{ + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: mockRegResValFQN2, + }, + }, + } + decisionRequestContext := &policy.RequestContext{ + Pep: &policy.PolicyEnforcementPoint{ + ClientId: mockClientID, + }, + } + + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) + + s.Require().NoError(err) + s.Require().Len(perResource, 1, "should have obligations for exactly one resource") + s.Require().Len(perResource[0], 1, "should have exactly one obligation for the resource") + s.Equal(mockObligationFQN2, perResource[0][0]) + s.Require().Len(all, 1, "should have exactly one obligation total") + s.Contains(all, mockObligationFQN2) +} + +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_MixedResources_RegisteredAndDirect_Triggered() { + // Mix registered resource and direct attribute values + resources := []*authz.Resource{ + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: mockRegResValFQN1, + }, + }, + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN2}, + }, + }, + }, + } + decisionRequestContext := &policy.RequestContext{ + Pep: &policy.PolicyEnforcementPoint{ + ClientId: mockClientID, + }, + } + + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) + + s.Require().NoError(err) + s.Require().Len(perResource, 2, "should have obligations for exactly two resources") + + // First resource (registered resource mapping to mockAttrValFQN1) -> mockObligationFQN1 + s.Require().Len(perResource[0], 1, "first resource should have exactly one obligation") + s.Equal(mockObligationFQN1, perResource[0][0]) + + // Second resource (direct attribute mockAttrValFQN2 with client scoping) -> mockObligationFQN2 + s.Require().Len(perResource[1], 1, "second resource should have exactly one obligation") + s.Equal(mockObligationFQN2, perResource[1][0]) + + // Should have both obligations in total + s.Require().Len(all, 2, "should have exactly two obligations total") + s.ElementsMatch([]string{mockObligationFQN1, mockObligationFQN2}, all) +} + +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_RegisteredResource_CustomAction_Triggered() { + // Use mockRegResValFQN3 which has custom action and should trigger mockObligationFQN4 + resources := []*authz.Resource{ + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: mockRegResValFQN3, + }, + }, + } + decisionRequestContext := &policy.RequestContext{} + + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionCustom, resources, decisionRequestContext) + + s.Require().NoError(err) + s.Require().Len(perResource, 1, "should have obligations for exactly one resource") + s.Require().Len(perResource[0], 1, "should have exactly one obligation for the resource") + s.Equal(mockObligationFQN4, perResource[0][0]) + s.Require().Len(all, 1, "should have exactly one obligation total") + s.Contains(all, mockObligationFQN4) +} + +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_RegisteredResource_CustomAction_WrongAction_NoObligationsTriggered() { + // Use mockRegResValFQN3 (has custom action) but call with read action - should trigger nothing + resources := []*authz.Resource{ + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: mockRegResValFQN3, + }, + }, + } + decisionRequestContext := &policy.RequestContext{} + + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) + + s.Require().NoError(err) + s.Require().Len(perResource, 1, "should have results for exactly one resource") + s.Empty(perResource[0], "no obligations should be triggered for read action on resource that only has custom action mapping") + s.Empty(all, "no obligations should be triggered globally") +} + +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_CustomAction_RegisteredResource_Triggered() { + // Test custom action with registered resource + resources := []*authz.Resource{ + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: mockRegResValFQN3, + }, + }, + } + decisionRequestContext := &policy.RequestContext{} + + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionCustom, resources, decisionRequestContext) + + s.Require().NoError(err) + s.Require().Len(perResource, 1, "should have obligations for exactly one resource") + s.Require().Len(perResource[0], 1, "should have exactly one obligation for the resource") + s.Equal(mockObligationFQN4, perResource[0][0]) + s.Require().Len(all, 1, "should have exactly one obligation total") + s.Contains(all, mockObligationFQN4) +} + +func (s *ObligationsPDPSuite) Test_GetRequiredObligations_CustomAction_MixedResources_Triggered() { + // Test custom action with mixed resource types + resources := []*authz.Resource{ + // Direct attribute that triggers custom obligation + { + Resource: &authz.Resource_AttributeValues_{ + AttributeValues: &authz.Resource_AttributeValues{ + Fqns: []string{mockAttrValFQN3}, + }, + }, + }, + // Registered resource that also triggers custom obligation + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: mockRegResValFQN3, + }, + }, + // Registered resource that doesn't trigger custom obligation (only has read action) + { + Resource: &authz.Resource_RegisteredResourceValueFqn{ + RegisteredResourceValueFqn: mockRegResValFQN2, + }, + }, + } + decisionRequestContext := &policy.RequestContext{} + + perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionCustom, resources, decisionRequestContext) + + s.Require().NoError(err) + s.Require().Len(perResource, 3, "should have results for exactly three resources") + + // First resource (direct attribute) should trigger custom obligation + s.Require().Len(perResource[0], 1, "first resource should have exactly one obligation") + s.Equal(mockObligationFQN4, perResource[0][0]) + + // Second resource (registered resource with custom action) should trigger custom obligation + s.Require().Len(perResource[1], 1, "second resource should have exactly one obligation") + s.Equal(mockObligationFQN4, perResource[1][0]) + + // Third resource (registered resource without custom action) should trigger no obligations + s.Empty(perResource[2], "third resource should have no obligations for custom action") + + // Should have exactly one unique obligation in total (deduplicated) + s.Require().Len(all, 1, "should have exactly one unique obligation total") + s.Contains(all, mockObligationFQN4) +} + func (s *ObligationsPDPSuite) createAttributesByValueFQN(attrValFQN, attrName string) map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue { return map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue{ attrValFQN: { From 5122e416b25ea5c4d48dd1f76d3efcc6c457b16b Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Tue, 16 Sep 2025 13:39:57 -0700 Subject: [PATCH 14/17] more tests --- .../v2/obligations/obligations_pdp_test.go | 72 ++++++++++++++----- 1 file changed, 55 insertions(+), 17 deletions(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go index 916edc70f5..10b62a2597 100644 --- a/service/internal/access/v2/obligations/obligations_pdp_test.go +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -36,9 +36,9 @@ var ( actionRead = &policy.Action{Name: actionNameRead} actionCreate = &policy.Action{Name: actionNameCreate} actionCustom = &policy.Action{Name: actionNameCustom} -) -// TODO: registered resources + emptyDecisionRequestContext = &policy.RequestContext{} +) type ObligationsPDPSuite struct { suite.Suite @@ -171,13 +171,14 @@ func (s *ObligationsPDPSuite) SetupSuite() { func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_Success() { attributesByValueFQN := s.createAttributesByValueFQN(mockAttrValFQN1, "attr1") var noClientID string + var noRegisteredResources map[string]*policy.RegisteredResourceValue allObligations := []*policy.Obligation{s.createObligation(mockObligationFQN1, mockAttrValFQN1, noClientID, actionRead)} pdp, err := NewObligationsPolicyDecisionPoint( s.T().Context(), logger.CreateTestLogger(), attributesByValueFQN, - nil, + noRegisteredResources, allObligations, ) @@ -195,12 +196,13 @@ func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_Success() { func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_WithClientScoped() { attributesByValueFQN := s.createAttributesByValueFQN(mockAttrValFQN2, "attr2") allObligations := []*policy.Obligation{s.createObligation(mockObligationFQN2, mockAttrValFQN2, mockClientID, actionRead)} + var noRegisteredResources map[string]*policy.RegisteredResourceValue pdp, err := NewObligationsPolicyDecisionPoint( s.T().Context(), logger.CreateTestLogger(), attributesByValueFQN, - nil, + noRegisteredResources, allObligations, ) @@ -215,6 +217,7 @@ func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_WithClientS func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_EmptyClientID_Fails() { attributesByValueFQN := s.createAttributesByValueFQN(mockAttrValFQN1, "attr1") + var noRegisteredResources map[string]*policy.RegisteredResourceValue // Create obligation with empty client ID using special case allObligations := []*policy.Obligation{ @@ -244,7 +247,7 @@ func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_EmptyClient s.T().Context(), logger.CreateTestLogger(), attributesByValueFQN, - nil, + noRegisteredResources, allObligations, ) @@ -254,12 +257,13 @@ func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_EmptyClient func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_EmptyObligations() { attributesByValueFQN := s.createAttributesByValueFQN(mockAttrValFQN1, "attr1") + var noRegisteredResources map[string]*policy.RegisteredResourceValue pdp, err := NewObligationsPolicyDecisionPoint( s.T().Context(), logger.CreateTestLogger(), attributesByValueFQN, - nil, + noRegisteredResources, []*policy.Obligation{}, ) @@ -335,7 +339,7 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_SimpleObligation_NoReq }, }, } - decisionRequestContext := &policy.RequestContext{} + decisionRequestContext := emptyDecisionRequestContext perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) @@ -360,11 +364,20 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_ClientScopedObligation }, } + // Found when client provided and matching perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) - s.Require().NoError(err) s.Equal([][]string{{mockObligationFQN2}}, perResource) s.Equal([]string{mockObligationFQN2}, all) + + // Not found when client not provided + decisionRequestContext.Pep.ClientId = "" + perResource, all, err = s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) + s.Require().NoError(err) + for _, r := range perResource { + s.Empty(r) + } + s.Empty(all) } func (s *ObligationsPDPSuite) Test_GetRequiredObligations_MixedObligations_Triggered() { @@ -414,7 +427,7 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_UnknownRegisteredResou }, }, } - decisionRequestContext := &policy.RequestContext{} + decisionRequestContext := emptyDecisionRequestContext perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) s.Require().Error(err) @@ -433,7 +446,7 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_CreateAction_SimpleObl }, }, } - decisionRequestContext := &policy.RequestContext{} + decisionRequestContext := emptyDecisionRequestContext perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionCreate, resources, decisionRequestContext) @@ -481,7 +494,7 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_ReadVsCreateAction_Dif }, }, } - decisionRequestContext := &policy.RequestContext{} + decisionRequestContext := emptyDecisionRequestContext // Test with 'read' action - should trigger read obligation perResourceRead, allRead, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) @@ -507,7 +520,7 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_RegisteredResource_Rea }, }, } - decisionRequestContext := &policy.RequestContext{} + decisionRequestContext := emptyDecisionRequestContext perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) @@ -527,7 +540,7 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_RegisteredResource_Cre }, }, } - decisionRequestContext := &policy.RequestContext{} + decisionRequestContext := emptyDecisionRequestContext perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionCreate, resources, decisionRequestContext) @@ -587,6 +600,16 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_RegisteredResource_Cli s.Equal(mockObligationFQN2, perResource[0][0]) s.Require().Len(all, 1, "should have exactly one obligation total") s.Contains(all, mockObligationFQN2) + + // Nothing should be triggered if no client + decisionRequestContext.Pep.ClientId = "" + perResource, all, err = s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) + s.Require().NoError(err) + s.Len(perResource, len(resources)) + for _, r := range perResource { + s.Empty(r, "no obligations should be triggered for create action on read-only registered resource") + } + s.Empty(all) } func (s *ObligationsPDPSuite) Test_GetRequiredObligations_MixedResources_RegisteredAndDirect_Triggered() { @@ -638,7 +661,7 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_RegisteredResource_Cus }, }, } - decisionRequestContext := &policy.RequestContext{} + decisionRequestContext := emptyDecisionRequestContext perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionCustom, resources, decisionRequestContext) @@ -648,6 +671,21 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_RegisteredResource_Cus s.Equal(mockObligationFQN4, perResource[0][0]) s.Require().Len(all, 1, "should have exactly one obligation total") s.Contains(all, mockObligationFQN4) + + // Same result even if a client is provided + decisionRequestContext = &policy.RequestContext{ + Pep: &policy.PolicyEnforcementPoint{ + ClientId: mockClientID, + }, + } + perResource, all, err = s.pdp.GetRequiredObligations(s.T().Context(), actionCustom, resources, decisionRequestContext) + + s.Require().NoError(err) + s.Require().Len(perResource, 1, "should have obligations for exactly one resource") + s.Require().Len(perResource[0], 1, "should have exactly one obligation for the resource") + s.Equal(mockObligationFQN4, perResource[0][0]) + s.Require().Len(all, 1, "should have exactly one obligation total") + s.Contains(all, mockObligationFQN4) } func (s *ObligationsPDPSuite) Test_GetRequiredObligations_RegisteredResource_CustomAction_WrongAction_NoObligationsTriggered() { @@ -659,7 +697,7 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_RegisteredResource_Cus }, }, } - decisionRequestContext := &policy.RequestContext{} + decisionRequestContext := emptyDecisionRequestContext perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) @@ -678,7 +716,7 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_CustomAction_Registere }, }, } - decisionRequestContext := &policy.RequestContext{} + decisionRequestContext := emptyDecisionRequestContext perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionCustom, resources, decisionRequestContext) @@ -714,7 +752,7 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_CustomAction_MixedReso }, }, } - decisionRequestContext := &policy.RequestContext{} + decisionRequestContext := emptyDecisionRequestContext perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionCustom, resources, decisionRequestContext) From f7116b414ba7701ada189aa675ea140fe639139b Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Tue, 16 Sep 2025 13:52:39 -0700 Subject: [PATCH 15/17] named errors --- .../access/v2/obligations/obligations_pdp.go | 12 ++++++++---- .../access/v2/obligations/obligations_pdp_test.go | 5 ++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp.go b/service/internal/access/v2/obligations/obligations_pdp.go index 1f8543bae7..c7b712afb7 100644 --- a/service/internal/access/v2/obligations/obligations_pdp.go +++ b/service/internal/access/v2/obligations/obligations_pdp.go @@ -13,7 +13,11 @@ import ( "github.com/opentdf/platform/service/logger" ) -// TODO: named errors +var ( + ErrEmptyPEPClientID = errors.New("trigger request context is optional but must contain PEP client ID") + ErrUnknownRegisteredResourceValue = errors.New("unknown registered resource value") + ErrUnsupportedResourceType = errors.New("unsupported resource type") +) // A graph of action names to attribute value FQNs to lists of obligation value FQNs // i.e. read : https://example.org/attr/attr1/value/val1 : [https://example.org/obl/some_obligation/value/some_value] @@ -76,7 +80,7 @@ func NewObligationsPolicyDecisionPoint( requiredPEPClientID := optionalRequestContext.GetPep().GetClientId() if requiredPEPClientID == "" { - return nil, errors.New("trigger request context is optional but must contain PEP client ID") + return nil, ErrEmptyPEPClientID } if _, ok := clientScopedTriggered[requiredPEPClientID]; !ok { clientScopedTriggered[requiredPEPClientID] = make(obligationValuesByActionOnAnAttributeValue) @@ -154,7 +158,7 @@ func (p *ObligationsPolicyDecisionPoint) GetRequiredObligations( regResValFQN := resource.GetRegisteredResourceValueFqn() regResValue, ok := p.registeredResourceValuesByFQN[regResValFQN] if !ok { - return nil, nil, fmt.Errorf("unknown registered resource value: %s", regResValFQN) + return nil, nil, fmt.Errorf("%w: %s", ErrUnknownRegisteredResourceValue, regResValFQN) } // Check the action-attribute-values associated with a Registered Resource Value for a match to the request action @@ -171,7 +175,7 @@ func (p *ObligationsPolicyDecisionPoint) GetRequiredObligations( attrValueFQNs = append(attrValueFQNs, resource.GetAttributeValues().GetFqns()...) default: - return nil, nil, fmt.Errorf("unsupported resource type: %T", resource) + return nil, nil, fmt.Errorf("%w: %T", ErrUnsupportedResourceType, resource) } // With list of attribute values for the resource, traverse each lookup graph to resolve the Set of required obligations diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go index 10b62a2597..f376f9bedd 100644 --- a/service/internal/access/v2/obligations/obligations_pdp_test.go +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -1,6 +1,7 @@ package obligations import ( + "errors" "testing" "github.com/stretchr/testify/suite" @@ -252,6 +253,7 @@ func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_EmptyClient ) s.Require().Error(err) + s.True(errors.Is(err, ErrEmptyPEPClientID), "error should be ErrEmptyPEPClientID") s.Nil(pdp) } @@ -431,9 +433,10 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_UnknownRegisteredResou perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) s.Require().Error(err) + s.True(errors.Is(err, ErrUnknownRegisteredResourceValue)) + s.Contains(err.Error(), badRegResValFQN, "error should contain the FQN that was not found") s.Empty(perResource) s.Empty(all) - s.Contains(err.Error(), badRegResValFQN) } func (s *ObligationsPDPSuite) Test_GetRequiredObligations_CreateAction_SimpleObligation_Triggered() { From 4ffa490da757d16c961509106ca2c4a7032488b9 Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Tue, 16 Sep 2025 14:03:39 -0700 Subject: [PATCH 16/17] lint fixes --- service/internal/access/v2/obligations/obligations_pdp.go | 4 ++-- .../internal/access/v2/obligations/obligations_pdp_test.go | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp.go b/service/internal/access/v2/obligations/obligations_pdp.go index c7b712afb7..b0c8c6aa73 100644 --- a/service/internal/access/v2/obligations/obligations_pdp.go +++ b/service/internal/access/v2/obligations/obligations_pdp.go @@ -14,9 +14,9 @@ import ( ) var ( - ErrEmptyPEPClientID = errors.New("trigger request context is optional but must contain PEP client ID") + ErrEmptyPEPClientID = errors.New("trigger request context is optional but must contain PEP client ID") ErrUnknownRegisteredResourceValue = errors.New("unknown registered resource value") - ErrUnsupportedResourceType = errors.New("unsupported resource type") + ErrUnsupportedResourceType = errors.New("unsupported resource type") ) // A graph of action names to attribute value FQNs to lists of obligation value FQNs diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go index f376f9bedd..f62dcd01e8 100644 --- a/service/internal/access/v2/obligations/obligations_pdp_test.go +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -1,7 +1,6 @@ package obligations import ( - "errors" "testing" "github.com/stretchr/testify/suite" @@ -253,7 +252,7 @@ func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_EmptyClient ) s.Require().Error(err) - s.True(errors.Is(err, ErrEmptyPEPClientID), "error should be ErrEmptyPEPClientID") + s.ErrorIs(err, ErrEmptyPEPClientID) s.Nil(pdp) } @@ -433,7 +432,7 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_UnknownRegisteredResou perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) s.Require().Error(err) - s.True(errors.Is(err, ErrUnknownRegisteredResourceValue)) + s.ErrorIs(err, ErrUnknownRegisteredResourceValue) s.Contains(err.Error(), badRegResValFQN, "error should contain the FQN that was not found") s.Empty(perResource) s.Empty(all) From 2ee9f0891af419a325de273038cf4cb13250eb6f Mon Sep 17 00:00:00 2001 From: jakedoublev Date: Tue, 16 Sep 2025 14:07:19 -0700 Subject: [PATCH 17/17] lint fixes --- .../internal/access/v2/obligations/obligations_pdp_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/internal/access/v2/obligations/obligations_pdp_test.go b/service/internal/access/v2/obligations/obligations_pdp_test.go index f62dcd01e8..18dd4b032d 100644 --- a/service/internal/access/v2/obligations/obligations_pdp_test.go +++ b/service/internal/access/v2/obligations/obligations_pdp_test.go @@ -252,7 +252,7 @@ func (s *ObligationsPDPSuite) Test_NewObligationsPolicyDecisionPoint_EmptyClient ) s.Require().Error(err) - s.ErrorIs(err, ErrEmptyPEPClientID) + s.Require().ErrorIs(err, ErrEmptyPEPClientID) s.Nil(pdp) } @@ -432,7 +432,7 @@ func (s *ObligationsPDPSuite) Test_GetRequiredObligations_UnknownRegisteredResou perResource, all, err := s.pdp.GetRequiredObligations(s.T().Context(), actionRead, resources, decisionRequestContext) s.Require().Error(err) - s.ErrorIs(err, ErrUnknownRegisteredResourceValue) + s.Require().ErrorIs(err, ErrUnknownRegisteredResourceValue) s.Contains(err.Error(), badRegResValFQN, "error should contain the FQN that was not found") s.Empty(perResource) s.Empty(all)