Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5b5b234
feat(authz): audit logs should properly handle obligations
jakedoublev Oct 17, 2025
2278200
Merge remote-tracking branch 'origin' into feat/DSPX-1735-audit
jakedoublev Oct 21, 2025
4ac327f
improve obligation decision result and audit logic
jakedoublev Oct 21, 2025
9875394
cleanup
jakedoublev Oct 21, 2025
7e2472b
Merge remote-tracking branch 'origin' into feat/DSPX-1735-audit
jakedoublev Oct 21, 2025
12d153d
isolated unit test that entitlements come back for audit
jakedoublev Oct 21, 2025
061b9fb
gemini suggestion unused param
jakedoublev Oct 21, 2025
c46dec2
improve tests
jakedoublev Oct 21, 2025
46d1c44
audit log should have empty fulfillable obligations and not nil when …
jakedoublev Oct 21, 2025
ea87577
improved logs and resource-level decisioning in response
jakedoublev Oct 21, 2025
3088d71
cleanup
jakedoublev Oct 21, 2025
dca8546
fix multiresource obligations decisioning
jakedoublev Oct 22, 2025
e814ddf
cleanup
jakedoublev Oct 22, 2025
e92f6f7
cleanup
jakedoublev Oct 22, 2025
203e7e2
Merge remote-tracking branch 'origin' into feat/DSPX-1735-audit
jakedoublev Oct 22, 2025
967dcb3
working
jakedoublev Oct 22, 2025
f5651d6
fix to handle no obligations pass cases
jakedoublev Oct 22, 2025
eacfa41
improve debug log
jakedoublev Oct 22, 2025
ee15d20
improve audit event metadata type
jakedoublev Oct 22, 2025
b4d6af1
var improvement
jakedoublev Oct 22, 2025
befb269
pointer to slice struct instead of copy
jakedoublev Oct 22, 2025
1d6f054
rename pdp decision field for clarity
jakedoublev Oct 22, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions service/authorization/v2/authorization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1374,7 +1374,7 @@ func Test_RollupSingleResourceDecision(t *testing.T) {
permitted: true,
decisions: []*access.Decision{
{
Access: true,
AllPermitted: true,
Results: []access.ResourceDecision{
{
ResourceID: "resource-123",
Expand All @@ -1395,7 +1395,7 @@ func Test_RollupSingleResourceDecision(t *testing.T) {
permitted: true,
decisions: []*access.Decision{
{
Access: true,
AllPermitted: true,
Results: []access.ResourceDecision{
{
ResourceID: "resource-123",
Expand All @@ -1422,7 +1422,7 @@ func Test_RollupSingleResourceDecision(t *testing.T) {
permitted: false,
decisions: []*access.Decision{
{
Access: true, // Verify permitted takes precedence
AllPermitted: true, // Verify permitted takes precedence
Results: []access.ResourceDecision{
{
ResourceID: "resource-123",
Expand All @@ -1443,7 +1443,7 @@ func Test_RollupSingleResourceDecision(t *testing.T) {
permitted: false,
decisions: []*access.Decision{
{
Access: true, // Verify permitted takes precedence
AllPermitted: true, // Verify permitted takes precedence
Results: []access.ResourceDecision{
{
ResourceID: "resource-123",
Expand Down Expand Up @@ -1473,8 +1473,8 @@ func Test_RollupSingleResourceDecision(t *testing.T) {
permitted: true,
decisions: []*access.Decision{
{
Access: true,
Results: []access.ResourceDecision{},
AllPermitted: true,
Results: []access.ResourceDecision{},
},
},
expectedResult: nil,
Expand Down Expand Up @@ -1509,7 +1509,7 @@ func Test_RollupMultiResourceDecisions(t *testing.T) {
name: "should return multiple permit decisions",
decisions: []*access.Decision{
{
Access: true,
AllPermitted: true,
Results: []access.ResourceDecision{
{
Passed: true,
Expand All @@ -1518,7 +1518,7 @@ func Test_RollupMultiResourceDecisions(t *testing.T) {
},
},
{
Access: true,
AllPermitted: true,
Results: []access.ResourceDecision{
{
Passed: true,
Expand All @@ -1542,7 +1542,7 @@ func Test_RollupMultiResourceDecisions(t *testing.T) {
name: "should return mix of permit and deny decisions",
decisions: []*access.Decision{
{
Access: true,
AllPermitted: true,
Results: []access.ResourceDecision{
{
Passed: true,
Expand All @@ -1551,7 +1551,7 @@ func Test_RollupMultiResourceDecisions(t *testing.T) {
},
},
{
Access: false,
AllPermitted: false,
Results: []access.ResourceDecision{
{
Passed: false,
Expand All @@ -1575,7 +1575,7 @@ func Test_RollupMultiResourceDecisions(t *testing.T) {
name: "should rely on results and default to false decisions",
decisions: []*access.Decision{
{
Access: true,
AllPermitted: true,
Results: []access.ResourceDecision{
{
Passed: true,
Expand All @@ -1588,7 +1588,7 @@ func Test_RollupMultiResourceDecisions(t *testing.T) {
},
},
{
Access: false,
AllPermitted: false,
Results: []access.ResourceDecision{
{
Passed: false,
Expand Down Expand Up @@ -1616,7 +1616,7 @@ func Test_RollupMultiResourceDecisions(t *testing.T) {
name: "should ignore global access and care about resource decisions predominantly",
decisions: []*access.Decision{
{
Access: false,
AllPermitted: false,
Results: []access.ResourceDecision{
{
Passed: false,
Expand All @@ -1629,7 +1629,7 @@ func Test_RollupMultiResourceDecisions(t *testing.T) {
},
},
{
Access: false,
AllPermitted: false,
Results: []access.ResourceDecision{
{
Passed: true,
Expand Down Expand Up @@ -1657,7 +1657,7 @@ func Test_RollupMultiResourceDecisions(t *testing.T) {
name: "should return obligations whenever found on a resource",
decisions: []*access.Decision{
{
Access: true,
AllPermitted: true,
Results: []access.ResourceDecision{
{
Passed: true,
Expand All @@ -1678,7 +1678,7 @@ func Test_RollupMultiResourceDecisions(t *testing.T) {
},
},
{
Access: false,
AllPermitted: false,
Results: []access.ResourceDecision{
{
Passed: false,
Expand Down Expand Up @@ -1728,8 +1728,8 @@ func Test_RollupMultiResourceDecisions(t *testing.T) {
name: "should return error when decision has no results",
decisions: []*access.Decision{
{
Access: true,
Results: []access.ResourceDecision{},
AllPermitted: true,
Results: []access.ResourceDecision{},
},
},
expectedError: ErrDecisionMustHaveResults,
Expand Down Expand Up @@ -1794,8 +1794,8 @@ func Test_RollupMultiResourceDecisions_WithNilChecks(t *testing.T) {
t.Run("nil Results field", func(t *testing.T) {
decisions := []*access.Decision{
{
Access: true,
Results: nil,
AllPermitted: true,
Results: nil,
},
}
_, err := rollupMultiResourceDecisions(decisions)
Expand All @@ -1822,8 +1822,8 @@ func Test_RollupSingleResourceDecision_WithNilChecks(t *testing.T) {
t.Run("nil Results field", func(t *testing.T) {
decisions := []*access.Decision{
{
Access: true,
Results: nil,
AllPermitted: true,
Results: nil,
},
}
_, err := rollupSingleResourceDecision(true, decisions)
Expand Down
4 changes: 2 additions & 2 deletions service/internal/access/v2/evaluate.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func getResourceDecision(
// indicates a failure before attribute definition rule evaluation
if len(resourceAttributeValues.GetFqns()) == 0 {
failure := &ResourceDecision{
Passed: false,
Entitled: false,
ResourceID: resourceID,
ResourceName: registeredResourceValueFQN,
}
Expand Down Expand Up @@ -151,7 +151,7 @@ func evaluateResourceAttributeValues(

// Return results in the appropriate structure
result := &ResourceDecision{
Passed: passed,
Entitled: passed,
ResourceID: resourceID,
DataRuleResults: dataRuleResults,
}
Expand Down
4 changes: 2 additions & 2 deletions service/internal/access/v2/evaluate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ func (s *EvaluateTestSuite) TestEvaluateResourceAttributeValues() {
} else {
s.Require().NoError(err)
s.NotNil(resourceDecision)
s.Equal(tc.expectAccessible, resourceDecision.Passed)
s.Equal(tc.expectAccessible, resourceDecision.Entitled)

// Check results array has the correct length based on grouping by definition
definitions := make(map[string]bool)
Expand Down Expand Up @@ -937,7 +937,7 @@ func (s *EvaluateTestSuite) TestGetResourceDecision() {
} else {
s.Require().NoError(err)
s.NotNil(decision)
s.Equal(tc.expectPass, decision.Passed, "Decision pass status didn't match")
s.Equal(tc.expectPass, decision.Entitled, "Decision entitlement status didn't match")
s.Equal(tc.resource.GetEphemeralId(), decision.ResourceID, "Resource ID didn't match")
}
})
Expand Down
109 changes: 81 additions & 28 deletions service/internal/access/v2/just_in_time_pdp.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"github.com/opentdf/platform/service/internal/access/v2/obligations"
"github.com/opentdf/platform/service/logger"
"github.com/opentdf/platform/service/logger/audit"
)

var (
Expand Down Expand Up @@ -141,7 +142,7 @@ func (p *JustInTimePDP) GetDecision(
)

// Because there are three possible types of entities, check obligations first to more easily handle decisioning logic
allTriggeredObligationsCanBeFulfilled, requiredObligationsPerResource, err := p.obligationsPDP.GetAllTriggeredObligationsAreFulfilled(
obligationDecision, err := p.obligationsPDP.GetAllTriggeredObligationsAreFulfilled(
ctx,
resources,
action,
Expand All @@ -165,26 +166,22 @@ func (p *JustInTimePDP) GetDecision(
case *authzV2.EntityIdentifier_RegisteredResourceValueFqn:
regResValueFQN := strings.ToLower(entityIdentifier.GetRegisteredResourceValueFqn())
// Registered resources do not have entity representations, so only one decision is made
decision, err := p.pdp.GetDecisionRegisteredResource(ctx, regResValueFQN, action, resources)
decision, entitlements, err := p.pdp.GetDecisionRegisteredResource(ctx, regResValueFQN, action, resources)
if err != nil {
return nil, false, fmt.Errorf("failed to get decision for registered resource value FQN [%s]: %w", regResValueFQN, err)
}
if decision == nil {
return nil, false, fmt.Errorf("decision is nil for registered resource value FQN [%s]", regResValueFQN)
}

// If not entitled, obligations are not considered
if !decision.Access {
return []*Decision{decision}, decision.Access, nil
}

// Access should only be granted if entitled AND obligations fulfilled
decision.Access = allTriggeredObligationsCanBeFulfilled
for idx, required := range requiredObligationsPerResource {
decision.Results[idx].RequiredObligationValueFQNs = required
}
// Update resource decisions with obligations and set final access decision
hasRequiredObligations := len(obligationDecision.RequiredObligationValueFQNs) > 0
entitledWithAnyObligationsSatisfied := decision.AllPermitted && (!hasRequiredObligations || obligationDecision.AllObligationsSatisfied)
decision.AllPermitted = entitledWithAnyObligationsSatisfied
decision = setResourceDecisionsWithObligations(decision, obligationDecision)

return []*Decision{decision}, decision.Access, nil
p.auditDecision(ctx, regResValueFQN, action, decision, entitlements, fulfillableObligationValueFQNs, obligationDecision)
return []*Decision{decision}, decision.AllPermitted, nil

default:
return nil, false, ErrInvalidEntityType
Expand All @@ -195,38 +192,69 @@ func (p *JustInTimePDP) GetDecision(

// Make initial entitlement decisions
entityDecisions := make([]*Decision, len(entityRepresentations))
entityEntitlements := make([]map[string][]*policy.Action, len(entityRepresentations))
allPermitted := true
for idx, entityRep := range entityRepresentations {
d, err := p.pdp.GetDecision(ctx, entityRep, action, resources)
d, entitlements, err := p.pdp.GetDecision(ctx, entityRep, action, resources)
if err != nil {
return nil, false, fmt.Errorf("failed to get decision for entityRepresentation with original id [%s]: %w", entityRep.GetOriginalId(), err)
}
if d == nil {
return nil, false, fmt.Errorf("decision is nil: %w", err)
}
if !d.Access {
// If any entity lacks access to any resource, update overall decision denial
if !d.AllPermitted {
allPermitted = false
}
entityDecisions[idx] = d
entityEntitlements[idx] = entitlements
}

// If even one entity was denied access, obligations are not considered or returned
if !allPermitted {
return entityDecisions, allPermitted, nil
}
// Update resource decisions with obligations and set final access decision
hasRequiredObligations := len(obligationDecision.RequiredObligationValueFQNs) > 0
allEntitledWithAnyObligationsSatisfied := allPermitted && (!hasRequiredObligations || obligationDecision.AllObligationsSatisfied)
allPermitted = allEntitledWithAnyObligationsSatisfied

// Access should only be granted if entitled AND obligations fulfilled
allPermitted = allTriggeredObligationsCanBeFulfilled
// Obligations are not entity-specific at this time so will be the same across every entity
for _, decision := range entityDecisions {
for idx, required := range requiredObligationsPerResource {
decision.Results[idx].RequiredObligationValueFQNs = required
}
// Propagate obligations within policy on each resource decision object
for entityIdx, decision := range entityDecisions {
// TODO: figure out this multi-entity response?
// entitledWithAnyObligationsSatisfied := decision.AllPermitted && (!hasRequiredObligations || obligationDecision.AllObligationsSatisfied)
// decision.AllPermitted = entitledWithAnyObligationsSatisfied
decision = setResourceDecisionsWithObligations(decision, obligationDecision)
decision.AllPermitted = allPermitted
entityRepID := entityRepresentations[entityIdx].GetOriginalId()
p.auditDecision(ctx, entityRepID, action, decision, entityEntitlements[entityIdx], fulfillableObligationValueFQNs, obligationDecision)
}

return entityDecisions, allPermitted, nil
}

// setResourceDecisionsWithObligations updates all resource decisions with obligation
// information and sets each resource passed state
func setResourceDecisionsWithObligations(
decision *Decision,
obligationDecision obligations.ObligationPolicyDecision,
) *Decision {
hasRequiredObligations := len(obligationDecision.RequiredObligationValueFQNs) > 0

for idx := range decision.Results {
resourceDecision := &decision.Results[idx]

if hasRequiredObligations {
// Update with specific obligation data from the obligations PDP
perResource := obligationDecision.RequiredObligationValueFQNsPerResource[idx]
resourceDecision.ObligationsSatisfied = perResource.ObligationsSatisfied
resourceDecision.RequiredObligationValueFQNs = perResource.RequiredObligationValueFQNs
} else {
// No required obligations means all obligations are satisfied
resourceDecision.ObligationsSatisfied = true
}

resourceDecision.Passed = resourceDecision.Entitled && resourceDecision.ObligationsSatisfied
}
return decision
}

// GetEntitlements retrieves the entitlements for the provided entity chain.
// It resolves the entity chain to get the entity representations and then calls the embedded PDP to get the entitlements.
func (p *JustInTimePDP) GetEntitlements(
Expand Down Expand Up @@ -287,8 +315,6 @@ func (p *JustInTimePDP) GetEntitlements(
func (p *JustInTimePDP) getMatchedSubjectMappings(
ctx context.Context,
entityRepresentations []*entityresolutionV2.EntityRepresentation,
// updated with the results, attrValue FQN to attribute and value with subject mappings
// entitleableAttributes map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue,
) ([]*policy.SubjectMapping, error) {
// Break the entity down the entities into their properties/selectors and retrieve only those subject mappings
subjectProperties := make([]*policy.SubjectProperty, 0)
Expand Down Expand Up @@ -400,3 +426,30 @@ func (p *JustInTimePDP) resolveEntitiesFromRequestToken(

return p.resolveEntitiesFromToken(ctx, token, skipEnvironmentEntities)
}

// auditDecision logs a GetDecisionV2 audit event with obligation information
func (p *JustInTimePDP) auditDecision(
ctx context.Context,
entityID string,
action *policy.Action,
decision *Decision,
entitlements map[string][]*policy.Action,
fulfillableObligationValueFQNs []string,
obligationDecision obligations.ObligationPolicyDecision,
) {
// Determine audit decision result
auditDecision := audit.GetDecisionResultDeny
if decision.AllPermitted {
auditDecision = audit.GetDecisionResultPermit
}

p.logger.Audit.GetDecisionV2(ctx, audit.GetDecisionV2EventParams{
EntityID: entityID,
ActionName: action.GetName(),
Decision: auditDecision,
Entitlements: entitlements,
FulfillableObligationValueFQNs: fulfillableObligationValueFQNs,
ObligationsSatisfied: obligationDecision.AllObligationsSatisfied,
ResourceDecisions: decision.Results,
})
}
Loading
Loading