Skip to content

Commit e730547

Browse files
opentdf-automation[bot]opentdf-automation[bot]
authored
fix(authz): handle individual resource edge cases in decisions (#2835)
### Proposed Changes Individual resources should be denied for these cases, instead of an overall error response * attribute value FQN not found * registered resource value FQN not found * registered resource missing action attribute values altogether * registered resource missing action attribute values that relate to request action (i.e. `read -> https://example.com/attr/attr1/value/value1` but request was for `delete` action) ### 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 (cherry picked from commit fad4437)
1 parent ebe6469 commit e730547

File tree

9 files changed

+87
-98
lines changed

9 files changed

+87
-98
lines changed

‎service/internal/access/v2/evaluate.go‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import (
1717

1818
var (
1919
ErrInvalidResource = errors.New("access: invalid resource")
20-
ErrFQNNotFound = errors.New("access: attribute value FQN not found")
20+
ErrFQNNotFound = errors.New("access: FQN not found")
2121
ErrDefinitionNotFound = errors.New("access: definition not found for FQN")
2222
ErrFailedEvaluation = errors.New("access: failed to evaluate definition")
2323
ErrMissingRequiredSpecifiedRule = errors.New("access: AttributeDefinition rule cannot be unspecified")
@@ -35,7 +35,17 @@ func getResourceDecision(
3535
action *policy.Action,
3636
resource *authz.Resource,
3737
) (*ResourceDecision, error) {
38-
if err := validateGetResourceDecision(accessibleAttributeValues, entitlements, action, resource); err != nil {
38+
var (
39+
resourceID = resource.GetEphemeralId()
40+
registeredResourceValueFQN string
41+
resourceAttributeValues *authz.Resource_AttributeValues
42+
failure = &ResourceDecision{
43+
Entitled: false,
44+
ResourceID: resourceID,
45+
ResourceName: resourceID,
46+
}
47+
)
48+
if err := validateGetResourceDecision(entitlements, action, resource); err != nil {
3949
return nil, err
4050
}
4151

@@ -45,23 +55,28 @@ func getResourceDecision(
4555
slog.Any("resource", resource.GetResource()),
4656
)
4757

48-
var (
49-
resourceID = resource.GetEphemeralId()
50-
registeredResourceValueFQN string
51-
resourceAttributeValues *authz.Resource_AttributeValues
52-
)
58+
if len(accessibleAttributeValues) == 0 {
59+
l.WarnContext(ctx, "resource is not able to be entitled", slog.Any("resource", resource.GetResource()))
60+
return failure, nil
61+
}
5362

5463
switch resource.GetResource().(type) {
5564
case *authz.Resource_RegisteredResourceValueFqn:
5665
registeredResourceValueFQN = strings.ToLower(resource.GetRegisteredResourceValueFqn())
66+
l = l.With("registered_resource_value_fqn", registeredResourceValueFQN)
67+
failure.ResourceName = registeredResourceValueFQN
68+
5769
regResValue, found := accessibleRegisteredResourceValues[registeredResourceValueFQN]
5870
if !found {
59-
return nil, fmt.Errorf("%w: %s", ErrFQNNotFound, registeredResourceValueFQN)
71+
l.WarnContext(
72+
ctx,
73+
"registered resource value not found - denying access",
74+
)
75+
return failure, nil
6076
}
6177
l.DebugContext(
6278
ctx,
6379
"registered_resource_value",
64-
slog.String("registered_resource_value_fqn", registeredResourceValueFQN),
6580
slog.Any("action_attribute_values", regResValue.GetActionAttributeValues()),
6681
)
6782

@@ -84,11 +99,7 @@ func getResourceDecision(
8499
// if no relevant attributes from action-attribute-values with the requested action,
85100
// indicates a failure before attribute definition rule evaluation
86101
if len(resourceAttributeValues.GetFqns()) == 0 {
87-
failure := &ResourceDecision{
88-
Entitled: false,
89-
ResourceID: resourceID,
90-
ResourceName: registeredResourceValueFQN,
91-
}
102+
l.WarnContext(ctx, "registered resource value missing action-attribute-values for requested action")
92103
return failure, nil
93104
}
94105

‎service/internal/access/v2/evaluate_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -895,7 +895,7 @@ func (s *EvaluateTestSuite) TestGetResourceDecision() {
895895
EphemeralId: "test-reg-res-id-5",
896896
},
897897
entitlements: subjectmappingbuiltin.AttributeValueFQNsToActions{},
898-
expectError: true,
898+
expectError: false,
899899
expectPass: false,
900900
},
901901
{

‎service/internal/access/v2/helpers.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ func getResourceDecisionableAttributes(
200200
var (
201201
decisionableAttributes = make(map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue)
202202
attrValueFQNs = make([]string, 0)
203+
notFoundFQNs = make([]string, 0)
203204
)
204205

205206
// Parse attribute value FQNs from various resource types
@@ -214,7 +215,7 @@ func getResourceDecisionableAttributes(
214215
regResValueFQN := strings.ToLower(resource.GetRegisteredResourceValueFqn())
215216
regResValue, found := accessibleRegisteredResourceValues[regResValueFQN]
216217
if !found {
217-
return nil, fmt.Errorf("resource registered resource value FQN not found in memory [%s]: %w", regResValueFQN, ErrInvalidResource)
218+
notFoundFQNs = append(notFoundFQNs, regResValueFQN)
218219
}
219220

220221
for _, aav := range regResValue.GetActionAttributeValues() {
@@ -245,7 +246,8 @@ func getResourceDecisionableAttributes(
245246

246247
attributeAndValue, ok := entitleableAttributesByValueFQN[attrValueFQN]
247248
if !ok {
248-
return nil, fmt.Errorf("resource attribute value FQN not found in memory [%s]: %w", attrValueFQN, ErrInvalidResource)
249+
notFoundFQNs = append(notFoundFQNs, attrValueFQN)
250+
continue
249251
}
250252

251253
decisionableAttributes[attrValueFQN] = attributeAndValue
@@ -255,5 +257,9 @@ func getResourceDecisionableAttributes(
255257
}
256258
}
257259

260+
if len(notFoundFQNs) > 0 {
261+
return decisionableAttributes, fmt.Errorf("resource FQNs not found in memory %v: %w", notFoundFQNs, ErrFQNNotFound)
262+
}
263+
258264
return decisionableAttributes, nil
259265
}

‎service/internal/access/v2/helpers_test.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"google.golang.org/protobuf/proto"
1414
)
1515

16-
// Updated assertions to include better validation of the retrieved definition
1716
func TestGetDefinition(t *testing.T) {
1817
validFQN := "https://example.org/attr/classification/value/public"
1918
invalidFQN := "invalid-fqn"

‎service/internal/access/v2/obligations/obligations_pdp.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,8 @@ import (
1414
)
1515

1616
var (
17-
ErrEmptyPEPClientID = errors.New("trigger request context is optional but must contain PEP client ID")
18-
ErrUnknownRegisteredResourceValue = errors.New("unknown registered resource value")
19-
ErrUnsupportedResourceType = errors.New("unsupported resource type")
17+
ErrEmptyPEPClientID = errors.New("trigger request context is optional but must contain PEP client ID")
18+
ErrUnsupportedResourceType = errors.New("unsupported resource type")
2019
)
2120

2221
// A graph of action names to attribute value FQNs to lists of obligation value FQNs
@@ -263,8 +262,14 @@ func (p *ObligationsPolicyDecisionPoint) getTriggeredObligations(
263262
case *authz.Resource_RegisteredResourceValueFqn:
264263
regResValFQN := strings.ToLower(resource.GetRegisteredResourceValueFqn())
265264
regResValue, ok := p.registeredResourceValuesByFQN[regResValFQN]
265+
// If not found, cannot trigger obligations
266266
if !ok {
267-
return nil, nil, fmt.Errorf("%w: %s", ErrUnknownRegisteredResourceValue, regResValFQN)
267+
p.logger.WarnContext(
268+
ctx,
269+
"registered resource value not found - skipping",
270+
slog.String("registered_resource_value_fqn", regResValFQN),
271+
)
272+
continue
268273
}
269274

270275
// Check the action-attribute-values associated with a Registered Resource Value for a match to the request action

‎service/internal/access/v2/obligations/obligations_pdp_test.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -430,10 +430,9 @@ func (s *ObligationsPDPSuite) Test_getTriggeredObligations_UnknownRegisteredReso
430430
decisionRequestContext := emptyDecisionRequestContext
431431

432432
perResource, all, err := s.pdp.getTriggeredObligations(s.T().Context(), actionRead, resources, decisionRequestContext)
433-
s.Require().Error(err)
434-
s.Require().ErrorIs(err, ErrUnknownRegisteredResourceValue)
435-
s.Contains(err.Error(), badRegResValFQN, "error should contain the FQN that was not found")
436-
s.Empty(perResource)
433+
s.Require().NoError(err, "none triggered if FQN not found")
434+
s.Len(perResource, 1)
435+
s.Empty(perResource[0])
437436
s.Empty(all)
438437
}
439438

‎service/internal/access/v2/pdp.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,11 @@ func (p *PolicyDecisionPoint) GetDecision(
186186
// Filter all attributes down to only those that relevant to the entitlement decisioning of these specific resources
187187
decisionableAttributes, err := getResourceDecisionableAttributes(ctx, l, p.allRegisteredResourceValuesByFQN, p.allEntitleableAttributesByValueFQN /* action, */, resources)
188188
if err != nil {
189-
return nil, nil, fmt.Errorf("error getting decisionable attributes: %w", err)
189+
if !errors.Is(err, ErrFQNNotFound) {
190+
return nil, nil, fmt.Errorf("error getting decisionable attributes: %w", err)
191+
}
192+
// Not an error: deny access to individual resources, not the entire request
193+
l.WarnContext(ctx, "encountered unknown FQN on resource", slog.Any("error", err))
190194
}
191195
l.DebugContext(ctx, "filtered to only entitlements relevant to decisioning", slog.Int("decisionable_attribute_values_count", len(decisionableAttributes)))
192196

‎service/internal/access/v2/validators.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
authzV2 "github.com/opentdf/platform/protocol/go/authorization/v2"
1010
entityresolutionV2 "github.com/opentdf/platform/protocol/go/entityresolution/v2"
1111
"github.com/opentdf/platform/protocol/go/policy"
12-
attrs "github.com/opentdf/platform/protocol/go/policy/attributes"
1312
"github.com/opentdf/platform/service/internal/subjectmappingbuiltin"
1413
)
1514

@@ -170,12 +169,10 @@ func validateEntityRepresentations(entityRepresentations []*entityresolutionV2.E
170169

171170
// validateOneResourceDecision validates the parameters for an access decision on a resource
172171
//
173-
// - accessibleAttributeValues: must not be nil
174172
// - entitlements: must not be nil
175173
// - action: must not be nil
176174
// - resource: must not be nil
177175
func validateGetResourceDecision(
178-
accessibleAttributeValues map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue,
179176
entitlements subjectmappingbuiltin.AttributeValueFQNsToActions,
180177
action *policy.Action,
181178
resource *authzV2.Resource,
@@ -189,8 +186,5 @@ func validateGetResourceDecision(
189186
if resource.GetResource() == nil {
190187
return fmt.Errorf("resource is nil: %w", ErrInvalidResource)
191188
}
192-
if len(accessibleAttributeValues) == 0 {
193-
return fmt.Errorf("accessible attribute values are empty: %w", ErrMissingRequiredPolicy)
194-
}
195189
return nil
196190
}

‎service/internal/access/v2/validators_test.go‎

Lines changed: 36 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
authzV2 "github.com/opentdf/platform/protocol/go/authorization/v2"
88
entityresolutionV2 "github.com/opentdf/platform/protocol/go/entityresolution/v2"
99
"github.com/opentdf/platform/protocol/go/policy"
10-
attrs "github.com/opentdf/platform/protocol/go/policy/attributes"
1110
"github.com/opentdf/platform/service/policy/actions"
1211
"github.com/stretchr/testify/require"
1312
"google.golang.org/protobuf/types/known/structpb"
@@ -403,11 +402,6 @@ func TestValidateEntityRepresentations(t *testing.T) {
403402
}
404403

405404
func TestValidateGetResourceDecision(t *testing.T) {
406-
// non-nil policy map
407-
validDecisionableAttributes := map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue{
408-
"https://example.org/attr/classification/value/public": {},
409-
}
410-
411405
// non-nil entitlements mapmap
412406
validEntitledFQNsToActions := map[string][]*policy.Action{
413407
"https://example.org/attr/name/value/public": {},
@@ -428,82 +422,59 @@ func TestValidateGetResourceDecision(t *testing.T) {
428422
}
429423

430424
tests := []struct {
431-
name string
432-
accessibleAttributeValues map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue
433-
entitlements map[string][]*policy.Action
434-
action *policy.Action
435-
resource *authzV2.Resource
436-
wantErr error
425+
name string
426+
entitlements map[string][]*policy.Action
427+
action *policy.Action
428+
resource *authzV2.Resource
429+
wantErr error
437430
}{
438431
{
439-
name: "Valid inputs",
440-
accessibleAttributeValues: validDecisionableAttributes,
441-
entitlements: validEntitledFQNsToActions,
442-
action: validAction,
443-
resource: validResource,
444-
wantErr: nil,
445-
},
446-
{
447-
name: "Nil accessible attribute values",
448-
accessibleAttributeValues: nil,
449-
entitlements: validEntitledFQNsToActions,
450-
action: validAction,
451-
resource: validResource,
452-
wantErr: ErrMissingRequiredPolicy,
453-
},
454-
{
455-
name: "Nil entitlements",
456-
accessibleAttributeValues: validDecisionableAttributes,
457-
entitlements: nil,
458-
action: validAction,
459-
resource: validResource,
460-
wantErr: ErrInvalidEntitledFQNsToActions,
432+
name: "Valid inputs",
433+
entitlements: validEntitledFQNsToActions,
434+
action: validAction,
435+
resource: validResource,
436+
wantErr: nil,
461437
},
462438
{
463-
name: "Nil action",
464-
accessibleAttributeValues: validDecisionableAttributes,
465-
entitlements: validEntitledFQNsToActions,
466-
action: nil,
467-
resource: validResource,
468-
wantErr: ErrInvalidAction,
439+
name: "Nil entitlements",
440+
entitlements: nil,
441+
action: validAction,
442+
resource: validResource,
443+
wantErr: ErrInvalidEntitledFQNsToActions,
469444
},
470445
{
471-
name: "Nil resource",
472-
accessibleAttributeValues: validDecisionableAttributes,
473-
entitlements: validEntitledFQNsToActions,
474-
action: validAction,
475-
resource: nil,
476-
wantErr: ErrInvalidResource,
446+
name: "Nil action",
447+
entitlements: validEntitledFQNsToActions,
448+
action: nil,
449+
resource: validResource,
450+
wantErr: ErrInvalidAction,
477451
},
478452
{
479-
name: "Empty accessible attribute values",
480-
accessibleAttributeValues: map[string]*attrs.GetAttributeValuesByFqnsResponse_AttributeAndValue{},
481-
entitlements: validEntitledFQNsToActions,
482-
action: validAction,
483-
resource: validResource,
484-
wantErr: ErrMissingRequiredPolicy,
453+
name: "Nil resource",
454+
entitlements: validEntitledFQNsToActions,
455+
action: validAction,
456+
resource: nil,
457+
wantErr: ErrInvalidResource,
485458
},
486459
{
487-
name: "Empty action",
488-
accessibleAttributeValues: validDecisionableAttributes,
489-
entitlements: validEntitledFQNsToActions,
490-
action: &policy.Action{},
491-
resource: validResource,
492-
wantErr: ErrInvalidAction,
460+
name: "Empty action",
461+
entitlements: validEntitledFQNsToActions,
462+
action: &policy.Action{},
463+
resource: validResource,
464+
wantErr: ErrInvalidAction,
493465
},
494466
{
495-
name: "Empty resource",
496-
accessibleAttributeValues: validDecisionableAttributes,
497-
entitlements: validEntitledFQNsToActions,
498-
action: validAction,
499-
resource: &authzV2.Resource{},
500-
wantErr: ErrInvalidResource,
467+
name: "Empty resource",
468+
entitlements: validEntitledFQNsToActions,
469+
action: validAction,
470+
resource: &authzV2.Resource{},
471+
wantErr: ErrInvalidResource,
501472
},
502473
}
503474

504475
for _, tt := range tests {
505476
t.Run(tt.name, func(t *testing.T) {
506-
err := validateGetResourceDecision(tt.accessibleAttributeValues, tt.entitlements, tt.action, tt.resource)
477+
err := validateGetResourceDecision(tt.entitlements, tt.action, tt.resource)
507478
if tt.wantErr != nil {
508479
require.ErrorIs(t, err, tt.wantErr)
509480
} else {

0 commit comments

Comments
 (0)