Skip to content

Commit 35da5e3

Browse files
authored
fix(authz): obligations should be logged to audit but not returned when not entitled (#2847)
### Proposed Changes * Fixes a regression introduced by #2824 where obligations were returned when triggered when the requester was not entitled ### Checklist - [ ] I have added or updated unit tests - [ ] I have added or updated integration tests (if appropriate) - [ ] I have added or updated documentation ### Testing Instructions
1 parent 23f76c0 commit 35da5e3

File tree

2 files changed

+334
-19
lines changed

2 files changed

+334
-19
lines changed

service/internal/access/v2/just_in_time_pdp.go

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -178,10 +178,10 @@ func (p *JustInTimePDP) GetDecision(
178178
hasRequiredObligations := len(obligationDecision.RequiredObligationValueFQNs) > 0
179179
entitledWithAnyObligationsSatisfied := decision.AllPermitted && (!hasRequiredObligations || obligationDecision.AllObligationsSatisfied)
180180
decision.AllPermitted = entitledWithAnyObligationsSatisfied
181-
decision = setResourceDecisionsWithObligations(decision, obligationDecision)
181+
decisionWithObligationsWhenEntitled, auditResourceDecisions := getResourceDecisionsWithObligations(decision, obligationDecision)
182182

183-
p.auditDecision(ctx, regResValueFQN, action, decision, entitlements, fulfillableObligationValueFQNs, obligationDecision)
184-
return []*Decision{decision}, decision.AllPermitted, nil
183+
p.auditDecision(ctx, regResValueFQN, action, entitledWithAnyObligationsSatisfied, entitlements, fulfillableObligationValueFQNs, obligationDecision, auditResourceDecisions)
184+
return []*Decision{decisionWithObligationsWhenEntitled}, entitledWithAnyObligationsSatisfied, nil
185185

186186
default:
187187
return nil, false, ErrInvalidEntityType
@@ -220,39 +220,55 @@ func (p *JustInTimePDP) GetDecision(
220220
// TODO: figure out this multi-entity response?
221221
// entitledWithAnyObligationsSatisfied := decision.AllPermitted && (!hasRequiredObligations || obligationDecision.AllObligationsSatisfied)
222222
// decision.AllPermitted = entitledWithAnyObligationsSatisfied
223-
decision = setResourceDecisionsWithObligations(decision, obligationDecision)
223+
var auditResourceDecisions []ResourceDecision
224+
decision, auditResourceDecisions = getResourceDecisionsWithObligations(decision, obligationDecision)
224225
decision.AllPermitted = allPermitted
225226
entityRepID := entityRepresentations[entityIdx].GetOriginalId()
226-
p.auditDecision(ctx, entityRepID, action, decision, entityEntitlements[entityIdx], fulfillableObligationValueFQNs, obligationDecision)
227+
p.auditDecision(ctx, entityRepID, action, allPermitted, entityEntitlements[entityIdx], fulfillableObligationValueFQNs, obligationDecision, auditResourceDecisions)
227228
}
228229

229230
return entityDecisions, allPermitted, nil
230231
}
231232

232-
// setResourceDecisionsWithObligations updates all resource decisions with obligation
233-
// information and sets each resource passed state
234-
func setResourceDecisionsWithObligations(
233+
// getResourceDecisionsWithObligations updates the Decision Results with obligation info when
234+
// entitled, then sets each Resource Decision's passed state. Obligations are always populated on
235+
// the Resource Decisions returned separately for audit logs, but kept distinct to avoid leaking
236+
// obligations to those not entitled.
237+
func getResourceDecisionsWithObligations(
235238
decision *Decision,
236239
obligationDecision obligations.ObligationPolicyDecision,
237-
) *Decision {
240+
) (*Decision, []ResourceDecision) {
238241
hasRequiredObligations := len(obligationDecision.RequiredObligationValueFQNs) > 0
239242

243+
// Create audit snapshot with full obligation context for all resources, even if not entitled
244+
auditResourceDecisions := make([]ResourceDecision, len(decision.Results))
245+
240246
for idx := range decision.Results {
241247
resourceDecision := &decision.Results[idx]
242248

249+
// Default all obligations satisfied when none are required
250+
resourceDecision.ObligationsSatisfied = true
251+
var obligationFQNs []string
252+
243253
if hasRequiredObligations {
244-
// Update with specific obligation data from the obligations PDP
245254
perResource := obligationDecision.RequiredObligationValueFQNsPerResource[idx]
246255
resourceDecision.ObligationsSatisfied = perResource.ObligationsSatisfied
247-
resourceDecision.RequiredObligationValueFQNs = perResource.RequiredObligationValueFQNs
248-
} else {
249-
// No required obligations means all obligations are satisfied
250-
resourceDecision.ObligationsSatisfied = true
256+
obligationFQNs = perResource.RequiredObligationValueFQNs
257+
258+
// Only set obligations in response if entitled
259+
if resourceDecision.Entitled {
260+
resourceDecision.RequiredObligationValueFQNs = obligationFQNs
261+
}
251262
}
252263

253264
resourceDecision.Passed = resourceDecision.Entitled && resourceDecision.ObligationsSatisfied
265+
266+
// For audit, copy but always include list of required obligations even if not entitled
267+
auditResourceDecisions[idx] = *resourceDecision
268+
auditResourceDecisions[idx].RequiredObligationValueFQNs = obligationFQNs
254269
}
255-
return decision
270+
271+
return decision, auditResourceDecisions
256272
}
257273

258274
// GetEntitlements retrieves the entitlements for the provided entity chain.
@@ -427,19 +443,22 @@ func (p *JustInTimePDP) resolveEntitiesFromRequestToken(
427443
return p.resolveEntitiesFromToken(ctx, token, skipEnvironmentEntities)
428444
}
429445

430-
// auditDecision logs a GetDecisionV2 audit event with obligation information
446+
// auditDecision logs a GetDecisionV2 audit event with obligation information.
447+
// The auditResourceDecisions parameter should contain the full obligation context including
448+
// for non-entitled resources, which is intentionally excluded from the actual response.
431449
func (p *JustInTimePDP) auditDecision(
432450
ctx context.Context,
433451
entityID string,
434452
action *policy.Action,
435-
decision *Decision,
453+
allPermitted bool,
436454
entitlements map[string][]*policy.Action,
437455
fulfillableObligationValueFQNs []string,
438456
obligationDecision obligations.ObligationPolicyDecision,
457+
auditResourceDecisions []ResourceDecision,
439458
) {
440459
// Determine audit decision result
441460
auditDecision := audit.GetDecisionResultDeny
442-
if decision.AllPermitted {
461+
if allPermitted {
443462
auditDecision = audit.GetDecisionResultPermit
444463
}
445464

@@ -450,6 +469,6 @@ func (p *JustInTimePDP) auditDecision(
450469
Entitlements: entitlements,
451470
FulfillableObligationValueFQNs: fulfillableObligationValueFQNs,
452471
ObligationsSatisfied: obligationDecision.AllObligationsSatisfied,
453-
ResourceDecisions: decision.Results,
472+
ResourceDecisions: auditResourceDecisions,
454473
})
455474
}

0 commit comments

Comments
 (0)