diff --git a/service/integration/actions_test.go b/service/integration/actions_test.go index f94d07f74d..36820c3caf 100644 --- a/service/integration/actions_test.go +++ b/service/integration/actions_test.go @@ -202,7 +202,7 @@ func (s *ActionsSuite) Test_ListActions_FiltersCustomActionsByNamespace_Succeeds func (s *ActionsSuite) Test_ListActions_WithNamespace_DoesNotLeakStandardActionsFromOtherNamespaces() { createdNamespace, err := s.db.PolicyClient.CreateNamespace(s.ctx, &namespaces.CreateNamespaceRequest{ - Name: "list-actions-standard-scope", + Name: fmt.Sprintf("list-actions-standard-scope-%d", time.Now().UnixNano()), }) s.Require().NoError(err) s.Require().NotNil(createdNamespace) diff --git a/service/integration/attribute_values_test.go b/service/integration/attribute_values_test.go index 644b3740b5..4710906e8e 100644 --- a/service/integration/attribute_values_test.go +++ b/service/integration/attribute_values_test.go @@ -9,6 +9,7 @@ import ( "github.com/opentdf/platform/protocol/go/common" "github.com/opentdf/platform/protocol/go/policy" + "github.com/opentdf/platform/protocol/go/policy/actions" "github.com/opentdf/platform/protocol/go/policy/attributes" "github.com/opentdf/platform/protocol/go/policy/kasregistry" "github.com/opentdf/platform/protocol/go/policy/namespaces" @@ -958,8 +959,8 @@ func (s *AttributeValuesSuite) Test_GetAttributeValue_With_Two_Obligations_Succe s.obligations = append(s.obligations, obl1) // Create first obligation value with two triggers - readAction := s.f.GetStandardAction("read") - updateAction := s.f.GetStandardAction("update") + readAction := s.getActionByNameInNamespace("read", ns.GetId()) + updateAction := s.getActionByNameInNamespace("update", ns.GetId()) obl1Val1, err := s.db.PolicyClient.CreateObligationValue(s.ctx, &obligations.CreateObligationValueRequest{ ObligationId: obl1.GetId(), @@ -1049,6 +1050,15 @@ func TestAttributeValuesSuite(t *testing.T) { suite.Run(t, new(AttributeValuesSuite)) } +func (s *AttributeValuesSuite) getActionByNameInNamespace(name string, namespaceID string) *policy.Action { + action, err := s.db.PolicyClient.GetAction(s.ctx, &actions.GetActionRequest{ + Identifier: &actions.GetActionRequest_Name{Name: name}, + NamespaceId: namespaceID, + }) + s.Require().NoError(err) + return action +} + func (s *AttributeValuesSuite) assertObligations(expected, actual []*policy.Obligation) { s.Require().Len(actual, len(expected), "number of obligations does not match") diff --git a/service/integration/obligation_triggers_test.go b/service/integration/obligation_triggers_test.go index 63703b6a69..aa37ba5849 100644 --- a/service/integration/obligation_triggers_test.go +++ b/service/integration/obligation_triggers_test.go @@ -2,6 +2,7 @@ package integration import ( "context" + "errors" "fmt" "log/slog" "testing" @@ -130,16 +131,26 @@ func (s *ObligationTriggersSuite) TearDownSuite() { func (s *ObligationTriggersSuite) TearDownTest() { for _, triggerID := range s.triggerIDsToClean { + if triggerID == "" { + continue + } _, err := s.db.PolicyClient.DeleteObligationTrigger(s.ctx, &obligations.RemoveObligationTriggerRequest{ Id: triggerID, }) - s.Require().NoError(err) + if err != nil && !errors.Is(err, db.ErrNotFound) { + s.Require().NoError(err) + } } for _, obligationValueID := range s.obligationValueIDsToClean { + if obligationValueID == "" { + continue + } _, err := s.db.PolicyClient.DeleteObligationValue(s.ctx, &obligations.DeleteObligationValueRequest{ Id: obligationValueID, }) - s.Require().NoError(err) + if err != nil && !errors.Is(err, db.ErrNotFound) { + s.Require().NoError(err) + } } s.triggerIDsToClean = nil s.obligationValueIDsToClean = nil @@ -317,7 +328,7 @@ func (s *ObligationTriggersSuite) Test_CreateObligationTrigger_AttributeValueNot }, }) s.Require().Error(err) - s.Require().ErrorIs(err, db.ErrNotNullViolation) + s.Require().ErrorIs(err, db.ErrNotFound) s.Nil(trigger) } @@ -334,7 +345,7 @@ func (s *ObligationTriggersSuite) Test_CreateObligationTrigger_ActionNotFound_Fa }, }) s.Require().Error(err) - s.Require().ErrorIs(err, db.ErrInvalidOblTriParam) + s.Require().ErrorIs(err, db.ErrNotFound) s.Nil(trigger) } @@ -376,7 +387,38 @@ func (s *ObligationTriggersSuite) Test_CreateObligationTrigger_AttributeValueDif }, }) s.Require().Error(err) - s.Require().ErrorIs(err, db.ErrInvalidOblTriParam) + s.Require().ErrorIs(err, db.ErrNamespaceMismatch) + s.Nil(trigger) +} + +func (s *ObligationTriggersSuite) Test_CreateObligationTrigger_ActionIDDifferentNamespace_Fails() { + differentNamespace, err := s.db.PolicyClient.CreateNamespace(s.ctx, &namespaces.CreateNamespaceRequest{ + Name: "different-action-ns", + }) + s.Require().NoError(err) + defer func() { + _, deleteErr := s.db.PolicyClient.UnsafeDeleteNamespace(s.ctx, differentNamespace, differentNamespace.GetFqn()) + s.Require().NoError(deleteErr) + }() + + differentAction, err := s.db.PolicyClient.CreateAction(s.ctx, &actions.CreateActionRequest{ + Name: "action-different-ns", + NamespaceId: differentNamespace.GetId(), + }) + s.Require().NoError(err) + + trigger, err := s.db.PolicyClient.CreateObligationTrigger(s.ctx, &obligations.AddObligationTriggerRequest{ + ObligationValue: &common.IdFqnIdentifier{Id: s.obligationValue.GetId()}, + AttributeValue: &common.IdFqnIdentifier{Id: s.attributeValue.GetId()}, + Action: &common.IdNameIdentifier{Id: differentAction.GetId()}, + Context: &policy.RequestContext{ + Pep: &policy.PolicyEnforcementPoint{ + ClientId: clientID, + }, + }, + }) + s.Require().Error(err) + s.Require().ErrorIs(err, db.ErrNamespaceMismatch) s.Nil(trigger) } @@ -725,15 +767,23 @@ func (s *ObligationTriggersSuite) appendObligationValuesToClean(createdTriggers } func (s *ObligationTriggersSuite) createDifferentNamespaceWithTrigger(namespaceName string) *DifferentNamespaceEntities { + uniqueNamespaceName := fmt.Sprintf("%s-%d", namespaceName, time.Now().UnixNano()) + // Create a different namespace differentNamespace, err := s.db.PolicyClient.CreateNamespace(s.ctx, &namespaces.CreateNamespaceRequest{ - Name: namespaceName, + Name: uniqueNamespaceName, + }) + s.Require().NoError(err) + + differentAction, err := s.db.PolicyClient.CreateAction(s.ctx, &actions.CreateActionRequest{ + Name: "different-action-" + uniqueNamespaceName, + NamespaceId: differentNamespace.GetId(), }) s.Require().NoError(err) // Create obligation in different namespace differentObligation, err := s.db.PolicyClient.CreateObligation(s.ctx, &obligations.CreateObligationRequest{ - Name: "different-obligation-" + namespaceName, + Name: "different-obligation-" + uniqueNamespaceName, NamespaceId: differentNamespace.GetId(), }) s.Require().NoError(err) @@ -741,13 +791,13 @@ func (s *ObligationTriggersSuite) createDifferentNamespaceWithTrigger(namespaceN // Create obligation value in different namespace differentObligationValue, err := s.db.PolicyClient.CreateObligationValue(s.ctx, &obligations.CreateObligationValueRequest{ ObligationId: differentObligation.GetId(), - Value: "different-obligation-value-" + namespaceName, + Value: "different-obligation-value-" + uniqueNamespaceName, }) s.Require().NoError(err) // Create attribute in different namespace differentAttribute, err := s.db.PolicyClient.CreateAttribute(s.ctx, &attributes.CreateAttributeRequest{ - Name: "different-attribute-" + namespaceName, + Name: "different-attribute-" + uniqueNamespaceName, NamespaceId: differentNamespace.GetId(), Rule: policy.AttributeRuleTypeEnum_ATTRIBUTE_RULE_TYPE_ENUM_ALL_OF, }) @@ -755,7 +805,7 @@ func (s *ObligationTriggersSuite) createDifferentNamespaceWithTrigger(namespaceN // Create attribute value in different namespace differentAttributeValue, err := s.db.PolicyClient.CreateAttributeValue(s.ctx, differentAttribute.GetId(), &attributes.CreateAttributeValueRequest{ - Value: "different-value-" + namespaceName, + Value: "different-value-" + uniqueNamespaceName, AttributeId: differentAttribute.GetId(), }) s.Require().NoError(err) @@ -764,7 +814,7 @@ func (s *ObligationTriggersSuite) createDifferentNamespaceWithTrigger(namespaceN differentTrigger, err := s.db.PolicyClient.CreateObligationTrigger(s.ctx, &obligations.AddObligationTriggerRequest{ ObligationValue: &common.IdFqnIdentifier{Id: differentObligationValue.GetId()}, AttributeValue: &common.IdFqnIdentifier{Id: differentAttributeValue.GetId()}, - Action: &common.IdNameIdentifier{Id: s.action.GetId()}, + Action: &common.IdNameIdentifier{Id: differentAction.GetId()}, Context: &policy.RequestContext{ Pep: &policy.PolicyEnforcementPoint{ ClientId: clientID, diff --git a/service/integration/obligations_test.go b/service/integration/obligations_test.go index f233a055c6..3576f31e7d 100644 --- a/service/integration/obligations_test.go +++ b/service/integration/obligations_test.go @@ -11,6 +11,7 @@ import ( "github.com/opentdf/platform/lib/identifier" "github.com/opentdf/platform/protocol/go/common" "github.com/opentdf/platform/protocol/go/policy" + "github.com/opentdf/platform/protocol/go/policy/actions" "github.com/opentdf/platform/protocol/go/policy/obligations" "github.com/opentdf/platform/service/internal/fixtures" "github.com/opentdf/platform/service/pkg/db" @@ -1527,10 +1528,15 @@ func (s *ObligationsSuite) Test_UpdateObligationValue_WithTriggers_Succeeds() { }) s.Require().NoError(err) s.NotNil(updatedOblValue) + readAction, err := s.db.PolicyClient.GetAction(s.ctx, &actions.GetActionRequest{ + Identifier: &actions.GetActionRequest_Name{Name: "read"}, + NamespaceId: triggerSetup.namespace.ID, + }) + s.Require().NoError(err) s.assertObligationValueBasics(updatedOblValue, oblValPrefix+"test-1-updated", triggerSetup.namespace.ID, triggerSetup.namespace.Name, httpsPrefix+triggerSetup.namespace.Name) s.assertTriggers(updatedOblValue, []*TriggerAssertion{ { - expectedAction: triggerSetup.action, + expectedAction: readAction, expectedObligation: triggerSetup.createdObl, expectedAttributeValue: triggerSetup.attributeValues[1], expectedAttributeValueFQN: "https://example.com/attr/attr1/value/value2", @@ -1766,7 +1772,11 @@ func (s *ObligationsSuite) assertObligationValueBasics(oblValue *policy.Obligati func (s *ObligationsSuite) setupTriggerTests() *TriggerSetup { namespaceID, _, namespace := s.getNamespaceData(nsExampleCom) createdObl := s.createObligation(namespaceID, oblName, nil) - triggerAction := s.f.GetStandardAction("read") + triggerAction, err := s.db.PolicyClient.CreateAction(s.ctx, &actions.CreateActionRequest{ + Name: fmt.Sprintf("trigger-action-%d", time.Now().UnixNano()), + NamespaceId: namespaceID, + }) + s.Require().NoError(err) triggerAttributeValue := s.f.GetAttributeValueKey("example.com/attr/attr1/value/value1") triggerAttributeValue2 := s.f.GetAttributeValueKey("example.com/attr/attr1/value/value2") @@ -1851,7 +1861,14 @@ func (s *ObligationsSuite) createObligationValueWithTriggers(obligationID string triggers = customTriggers } else { // Default triggers for backward compatibility - triggerAction := s.f.GetStandardAction("read") + obl, err := s.db.PolicyClient.GetObligation(s.ctx, &obligations.GetObligationRequest{Id: obligationID}) + s.Require().NoError(err) + + triggerAction, err := s.db.PolicyClient.CreateAction(s.ctx, &actions.CreateActionRequest{ + Name: fmt.Sprintf("trigger-action-%d", time.Now().UnixNano()), + NamespaceId: obl.GetNamespace().GetId(), + }) + s.Require().NoError(err) triggerAttributeValue := s.f.GetAttributeValueKey("example.com/attr/attr1/value/value1") triggerAttributeValue2 := s.f.GetAttributeValueKey("example.com/attr/attr1/value/value2") diff --git a/service/integration/registered_resources_test.go b/service/integration/registered_resources_test.go index 9d2479d3ff..c2aaa7d94a 100644 --- a/service/integration/registered_resources_test.go +++ b/service/integration/registered_resources_test.go @@ -820,21 +820,7 @@ func (s *RegisteredResourcesSuite) Test_CreateRegisteredResourceValue_WithInvali }, }, }, - err: db.ErrForeignKeyViolation, - }, - { - name: "Invalid Action Name", - actionAttrValues: []*registeredresources.ActionAttributeValue{ - { - ActionIdentifier: ®isteredresources.ActionAttributeValue_ActionName{ - ActionName: "invalid_action_name", - }, - AttributeValueIdentifier: ®isteredresources.ActionAttributeValue_AttributeValueFqn{ - AttributeValueFqn: "https://example.com/attr/attr1/value/value1", - }, - }, - }, - err: db.ErrNotFound, + err: db.ErrMissingValue, }, { name: "Invalid Attribute Value ID", @@ -848,7 +834,7 @@ func (s *RegisteredResourcesSuite) Test_CreateRegisteredResourceValue_WithInvali }, }, }, - err: db.ErrForeignKeyViolation, + err: db.ErrNotFound, }, { name: "Invalid Attribute Value FQN", @@ -1891,7 +1877,7 @@ func (s *RegisteredResourcesSuite) Test_SameNamespaceEnforcement_DifferentNamesp }, }) s.Require().Error(err) - s.Require().ErrorIs(err, db.ErrForeignKeyViolation) + s.Require().ErrorIs(err, db.ErrNamespaceMismatch) } func (s *RegisteredResourcesSuite) Test_SameNamespaceEnforcement_SameNamespace_Succeeds() { @@ -1939,6 +1925,69 @@ func (s *RegisteredResourcesSuite) Test_SameNamespaceEnforcement_SameNamespace_S s.Require().Len(resVal.GetActionAttributeValues(), 1) } +func (s *RegisteredResourcesSuite) Test_SameNamespaceEnforcement_ActionIDDifferentNamespace_Fails() { + comNsID := s.getNamespaceID("example.com") + netNsID := s.getNamespaceID("example.net") + + res, err := s.db.PolicyClient.CreateRegisteredResource(s.ctx, ®isteredresources.CreateRegisteredResourceRequest{ + NamespaceId: comNsID, + Name: fmt.Sprintf("test_rr_action_id_ns_mismatch_%d", time.Now().UnixNano()), + }) + s.Require().NoError(err) + s.NotNil(res) + + wrongAction, err := s.db.PolicyClient.CreateAction(s.ctx, &pbActions.CreateActionRequest{ + Name: fmt.Sprintf("rr_wrong_action_id_%d", time.Now().UnixNano()), + NamespaceId: netNsID, + }) + s.Require().NoError(err) + + _, err = s.db.PolicyClient.CreateRegisteredResourceValue(s.ctx, ®isteredresources.CreateRegisteredResourceValueRequest{ + ResourceId: res.GetId(), + Value: fmt.Sprintf("rr_val_action_id_mismatch_%d", time.Now().UnixNano()), + ActionAttributeValues: []*registeredresources.ActionAttributeValue{ + { + ActionIdentifier: ®isteredresources.ActionAttributeValue_ActionId{ActionId: wrongAction.GetId()}, + AttributeValueIdentifier: ®isteredresources.ActionAttributeValue_AttributeValueFqn{ + AttributeValueFqn: "https://example.com/attr/attr1/value/value1", + }, + }, + }, + }) + s.Require().Error(err) + s.Require().ErrorIs(err, db.ErrNamespaceMismatch) +} + +func (s *RegisteredResourcesSuite) Test_UnnamespacedResource_NamespacedActionID_Fails() { + res, err := s.db.PolicyClient.CreateRegisteredResource(s.ctx, ®isteredresources.CreateRegisteredResourceRequest{ + Name: fmt.Sprintf("test_rr_unnamespaced_%d", time.Now().UnixNano()), + }) + s.Require().NoError(err) + s.NotNil(res) + s.Nil(res.GetNamespace()) + + namespacedAction, err := s.db.PolicyClient.CreateAction(s.ctx, &pbActions.CreateActionRequest{ + Name: fmt.Sprintf("rr_namespaced_action_%d", time.Now().UnixNano()), + NamespaceId: s.getNamespaceID("example.com"), + }) + s.Require().NoError(err) + + _, err = s.db.PolicyClient.CreateRegisteredResourceValue(s.ctx, ®isteredresources.CreateRegisteredResourceValueRequest{ + ResourceId: res.GetId(), + Value: fmt.Sprintf("rr_val_unnamespaced_%d", time.Now().UnixNano()), + ActionAttributeValues: []*registeredresources.ActionAttributeValue{ + { + ActionIdentifier: ®isteredresources.ActionAttributeValue_ActionId{ActionId: namespacedAction.GetId()}, + AttributeValueIdentifier: ®isteredresources.ActionAttributeValue_AttributeValueFqn{ + AttributeValueFqn: "https://example.com/attr/attr1/value/value1", + }, + }, + }, + }) + s.Require().Error(err) + s.Require().ErrorIs(err, db.ErrNamespaceMismatch) +} + func (s *RegisteredResourcesSuite) Test_CreateRegisteredResourceValue_WithNamespacedCustomActionName_Succeeds() { nsID := s.getNamespaceID("example.com") diff --git a/service/policy/db/obligations.go b/service/policy/db/obligations.go index 2030fdf12c..b790027436 100644 --- a/service/policy/db/obligations.go +++ b/service/policy/db/obligations.go @@ -9,6 +9,7 @@ import ( "github.com/opentdf/platform/lib/identifier" "github.com/opentdf/platform/protocol/go/common" "github.com/opentdf/platform/protocol/go/policy" + "github.com/opentdf/platform/protocol/go/policy/attributes" "github.com/opentdf/platform/protocol/go/policy/obligations" "github.com/opentdf/platform/service/pkg/db" "google.golang.org/protobuf/types/known/timestamppb" @@ -664,11 +665,19 @@ func (c PolicyDBClient) CreateObligationTrigger(ctx context.Context, r *obligati if err != nil { return nil, fmt.Errorf("failed to get obligation value: %w", err) } + actionID, err := c.resolveObligationTriggerActionID(ctx, r.GetAction(), oblVal.GetObligation().GetNamespace().GetId()) + if err != nil { + return nil, err + } + + err = c.validateObligationNamespaceConsistency(ctx, oblVal.GetObligation().GetNamespace().GetId(), r.GetAttributeValue(), actionID) + if err != nil { + return nil, err + } params := createObligationTriggerParams{ ObligationValueID: pgtypeUUID(oblVal.GetId()), - ActionName: pgtypeText(r.GetAction().GetName()), - ActionID: pgtypeUUID(r.GetAction().GetId()), + ActionID: pgtypeUUID(actionID), AttributeValueID: pgtypeUUID(r.GetAttributeValue().GetId()), AttributeValueFqn: pgtypeText(r.GetAttributeValue().GetFqn()), ClientID: pgtypeText(r.GetContext().GetPep().GetClientId()), @@ -702,6 +711,85 @@ func (c PolicyDBClient) CreateObligationTrigger(ctx context.Context, r *obligati return trigger, nil } +func (c PolicyDBClient) resolveObligationTriggerActionID(ctx context.Context, action *common.IdNameIdentifier, obligationNamespaceID string) (string, error) { + actionID := action.GetId() + if actionID != "" { + return actionID, nil + } + + actionName := strings.ToLower(action.GetName()) + if actionName == "" { + // this shouldnt happen due to proto validation, but just in case + return "", errors.Join( + db.ErrMissingValue, + errors.New("action identifier must include either id or name"), + ) + } + + createdOrListedActions, err := c.queries.createOrListActionsByNameInNamespace(ctx, createOrListActionsByNameInNamespaceParams{ + ActionNames: []string{actionName}, + NamespaceID: obligationNamespaceID, + }) + if err != nil { + return "", db.WrapIfKnownInvalidQueryErr( + errors.Join(db.ErrMissingValue, fmt.Errorf("failed to create or list action names [%v]: %w", actionName, err)), + ) + } + if len(createdOrListedActions) == 0 { + return "", db.WrapIfKnownInvalidQueryErr( + errors.Join(db.ErrMissingValue, fmt.Errorf("failed to create or list action names [%v]", actionName)), + ) + } + + return createdOrListedActions[0].ID, nil +} + +// validateObligationNamespaceConsistency ensures that action and attribute value +// belongs to the same namespace as the obligation trigger being created. +func (c PolicyDBClient) validateObligationNamespaceConsistency( + ctx context.Context, + targetNsID string, + attributeValue *common.IdFqnIdentifier, + actionID string, +) error { + var attributeValueIdentifier any + if attributeValue.GetId() != "" { + attributeValueIdentifier = &attributes.GetAttributeValueRequest_ValueId{ValueId: attributeValue.GetId()} + } else { + attributeValueIdentifier = &attributes.GetAttributeValueRequest_Fqn{Fqn: attributeValue.GetFqn()} + } + + av, err := c.GetAttributeValue(ctx, attributeValueIdentifier) + if err != nil { + return db.WrapIfKnownInvalidQueryErr(err) + } + attr, err := c.GetAttribute(ctx, av.GetAttribute().GetId()) + if err != nil { + return db.WrapIfKnownInvalidQueryErr(err) + } + if attr.GetNamespace().GetId() != targetNsID { + return errors.Join(db.ErrNamespaceMismatch, + fmt.Errorf("attribute value namespace [%s] does not match the specified obligation trigger namespace [%s]", attr.GetNamespace().GetId(), targetNsID)) + } + + // All actions must be in the same namespace + actionRows, err := c.queries.getActionsByIDs(ctx, []string{actionID}) + if err != nil { + return db.WrapIfKnownInvalidQueryErr(err) + } + if len(actionRows) == 0 { + return errors.Join(db.ErrNotFound, fmt.Errorf("action [%s] was not found", actionID)) + } + a := actionRows[0] + actionNsID := UUIDToString(a.NamespaceID) + if actionNsID != targetNsID { + return errors.Join(db.ErrNamespaceMismatch, + fmt.Errorf("action [%s] namespace [%s] does not match the specified obligation namespace [%s]", a.ID, actionNsID, targetNsID)) + } + + return nil +} + func (c PolicyDBClient) DeleteObligationTrigger(ctx context.Context, r *obligations.RemoveObligationTriggerRequest) (*policy.ObligationTrigger, error) { id, err := c.queries.deleteObligationTrigger(ctx, r.GetId()) if err != nil { diff --git a/service/policy/db/obligations.sql.go b/service/policy/db/obligations.sql.go index f5f010538d..2e51dc545b 100644 --- a/service/policy/db/obligations.sql.go +++ b/service/policy/db/obligations.sql.go @@ -155,30 +155,9 @@ WITH ov_id AS ( WHERE $1::uuid IS NOT NULL AND ov.id = $1::uuid ), a_id AS ( - SELECT - CASE - WHEN $2::uuid IS NOT NULL THEN ( - SELECT a.id - FROM actions a - WHERE a.id = $2::uuid - ) - WHEN $3::text IS NOT NULL THEN COALESCE( - ( - SELECT a.id - FROM actions a - WHERE a.name = $3::text - -- TODO(namespaced-actions): Move action namespace resolution/enforcement to service level. - -- This query currently assumes same-namespace action lookup before legacy unscoped fallback. - AND a.namespace_id = (SELECT namespace_id FROM ov_id) - ), - ( - SELECT a.id - FROM actions a - WHERE a.name = $3::text - AND a.namespace_id IS NULL - ) - ) - END AS id + SELECT a.id + FROM actions a + WHERE ($2::uuid IS NOT NULL AND a.id = $2::uuid) ), av_id AS ( SELECT av.id @@ -186,9 +165,9 @@ av_id AS ( JOIN attribute_definitions ad ON av.attribute_definition_id = ad.id LEFT JOIN attribute_fqns fqns ON fqns.value_id = av.id WHERE - (($4::uuid IS NOT NULL AND av.id = $4::uuid) + (($3::uuid IS NOT NULL AND av.id = $3::uuid) OR - ($5::text IS NOT NULL AND fqns.fqn = $5::text)) + ($4::text IS NOT NULL AND fqns.fqn = $4::text)) AND ad.namespace_id = (SELECT namespace_id FROM ov_id) ), inserted AS ( @@ -197,8 +176,8 @@ inserted AS ( (SELECT id FROM ov_id), (SELECT id FROM a_id), (SELECT id FROM av_id), - $6, - $7::text + $5, + $6::text RETURNING id, obligation_value_id, action_id, attribute_value_id, metadata, created_at, updated_at, client_id ) SELECT @@ -258,7 +237,6 @@ LEFT JOIN attribute_fqns av_fqns ON av_fqns.value_id = av.id type createObligationTriggerParams struct { ObligationValueID pgtype.UUID `json:"obligation_value_id"` ActionID pgtype.UUID `json:"action_id"` - ActionName pgtype.Text `json:"action_name"` AttributeValueID pgtype.UUID `json:"attribute_value_id"` AttributeValueFqn pgtype.Text `json:"attribute_value_fqn"` Metadata []byte `json:"metadata"` @@ -282,30 +260,9 @@ type createObligationTriggerRow struct { // WHERE $1::uuid IS NOT NULL AND ov.id = $1::uuid // ), // a_id AS ( -// SELECT -// CASE -// WHEN $2::uuid IS NOT NULL THEN ( -// SELECT a.id -// FROM actions a -// WHERE a.id = $2::uuid -// ) -// WHEN $3::text IS NOT NULL THEN COALESCE( -// ( -// SELECT a.id -// FROM actions a -// WHERE a.name = $3::text -// -- TODO(namespaced-actions): Move action namespace resolution/enforcement to service level. -// -- This query currently assumes same-namespace action lookup before legacy unscoped fallback. -// AND a.namespace_id = (SELECT namespace_id FROM ov_id) -// ), -// ( -// SELECT a.id -// FROM actions a -// WHERE a.name = $3::text -// AND a.namespace_id IS NULL -// ) -// ) -// END AS id +// SELECT a.id +// FROM actions a +// WHERE ($2::uuid IS NOT NULL AND a.id = $2::uuid) // ), // av_id AS ( // SELECT av.id @@ -313,9 +270,9 @@ type createObligationTriggerRow struct { // JOIN attribute_definitions ad ON av.attribute_definition_id = ad.id // LEFT JOIN attribute_fqns fqns ON fqns.value_id = av.id // WHERE -// (($4::uuid IS NOT NULL AND av.id = $4::uuid) +// (($3::uuid IS NOT NULL AND av.id = $3::uuid) // OR -// ($5::text IS NOT NULL AND fqns.fqn = $5::text)) +// ($4::text IS NOT NULL AND fqns.fqn = $4::text)) // AND ad.namespace_id = (SELECT namespace_id FROM ov_id) // ), // inserted AS ( @@ -324,8 +281,8 @@ type createObligationTriggerRow struct { // (SELECT id FROM ov_id), // (SELECT id FROM a_id), // (SELECT id FROM av_id), -// $6, -// $7::text +// $5, +// $6::text // RETURNING id, obligation_value_id, action_id, attribute_value_id, metadata, created_at, updated_at, client_id // ) // SELECT @@ -384,7 +341,6 @@ func (q *Queries) createObligationTrigger(ctx context.Context, arg createObligat row := q.db.QueryRow(ctx, createObligationTrigger, arg.ObligationValueID, arg.ActionID, - arg.ActionName, arg.AttributeValueID, arg.AttributeValueFqn, arg.Metadata, diff --git a/service/policy/db/queries/obligations.sql b/service/policy/db/queries/obligations.sql index d756fa3b4f..5b4c259f2b 100644 --- a/service/policy/db/queries/obligations.sql +++ b/service/policy/db/queries/obligations.sql @@ -494,30 +494,9 @@ WITH ov_id AS ( WHERE sqlc.narg('obligation_value_id')::uuid IS NOT NULL AND ov.id = sqlc.narg('obligation_value_id')::uuid ), a_id AS ( - SELECT - CASE - WHEN sqlc.narg('action_id')::uuid IS NOT NULL THEN ( - SELECT a.id - FROM actions a - WHERE a.id = sqlc.narg('action_id')::uuid - ) - WHEN sqlc.narg('action_name')::text IS NOT NULL THEN COALESCE( - ( - SELECT a.id - FROM actions a - WHERE a.name = sqlc.narg('action_name')::text - -- TODO(namespaced-actions): Move action namespace resolution/enforcement to service level. - -- This query currently assumes same-namespace action lookup before legacy unscoped fallback. - AND a.namespace_id = (SELECT namespace_id FROM ov_id) - ), - ( - SELECT a.id - FROM actions a - WHERE a.name = sqlc.narg('action_name')::text - AND a.namespace_id IS NULL - ) - ) - END AS id + SELECT a.id + FROM actions a + WHERE (sqlc.narg('action_id')::uuid IS NOT NULL AND a.id = sqlc.narg('action_id')::uuid) ), -- Gets the attribute value, but also ensures that the attribute value belongs to the same namespace as the obligation, to which the obligation value belongs av_id AS ( diff --git a/service/policy/db/registered_resources.go b/service/policy/db/registered_resources.go index 7131b8d859..ec4faf5242 100644 --- a/service/policy/db/registered_resources.go +++ b/service/policy/db/registered_resources.go @@ -580,45 +580,14 @@ func (c PolicyDBClient) createRegisteredResourceActionAttributeValues(ctx contex resourceNamespaceID := UUIDToString(nsUUID) createActionAttributeValueParams := make([]createRegisteredResourceActionAttributeValuesParams, len(actionAttrValues)) - var actionID, attributeValueID string for i, aav := range actionAttrValues { - switch ident := aav.GetActionIdentifier().(type) { - case *registeredresources.ActionAttributeValue_ActionId: - actionID = ident.ActionId - case *registeredresources.ActionAttributeValue_ActionName: - actionParams := getActionParams{ - Name: pgtypeText(strings.ToLower(ident.ActionName)), - NamespaceID: nsUUID, - } - a, err := c.queries.getAction(ctx, actionParams) - err = db.WrapIfKnownInvalidQueryErr(err) - if err != nil && nsUUID.Valid && errors.Is(err, db.ErrNotFound) { - // Fall back to standard (non-namespaced) action lookup only on not-found - actionParams.NamespaceID = pgtype.UUID{} - a, err = c.queries.getAction(ctx, actionParams) - } - if err != nil { - err = db.WrapIfKnownInvalidQueryErr(err) - return err - } - actionID = a.ID - default: - return db.ErrSelectIdentifierInvalid - } - - switch ident := aav.GetAttributeValueIdentifier().(type) { - case *registeredresources.ActionAttributeValue_AttributeValueId: - attributeValueID = ident.AttributeValueId - case *registeredresources.ActionAttributeValue_AttributeValueFqn: - av, err := c.queries.getAttributeValue(ctx, getAttributeValueParams{ - Fqn: pgtypeText(strings.ToLower(ident.AttributeValueFqn)), - }) - if err != nil { - return db.WrapIfKnownInvalidQueryErr(err) - } - attributeValueID = av.ID - default: - return db.ErrSelectIdentifierInvalid + actionID, attributeValueID, err := c.resolveRegResAAV(ctx, aav, nsUUID) + if err != nil { + return err + } + err = c.validateRRAAVNamespaceConsistency(ctx, resourceNamespaceID, attributeValueID, actionID) + if err != nil { + return err } createActionAttributeValueParams[i] = createRegisteredResourceActionAttributeValuesParams{ @@ -656,3 +625,88 @@ func (c PolicyDBClient) createRegisteredResourceActionAttributeValues(ctx contex return nil } + +// resolveRegResAAV parses the action from a createRegisteredResourceActionAttributeValues input +// resolving actions by name within the given namespace and collecting existing action ID. +func (c PolicyDBClient) resolveRegResAAV(ctx context.Context, aav *registeredresources.ActionAttributeValue, parsedNamespaceID pgtype.UUID) (string, string, error) { + var actionID, attributeValueID string + switch ident := aav.GetActionIdentifier().(type) { + case *registeredresources.ActionAttributeValue_ActionId: + actionID = ident.ActionId + case *registeredresources.ActionAttributeValue_ActionName: + ids, err := c.resolveActionNameIDs(ctx, []string{strings.ToLower(ident.ActionName)}, parsedNamespaceID) + if err != nil { + return "", "", err + } + if len(ids) == 0 { + return "", "", db.ErrMissingValue + } + actionID = ids[0] + default: + return "", "", db.ErrSelectIdentifierInvalid + } + + switch ident := aav.GetAttributeValueIdentifier().(type) { + case *registeredresources.ActionAttributeValue_AttributeValueId: + attributeValueID = ident.AttributeValueId + case *registeredresources.ActionAttributeValue_AttributeValueFqn: + av, err := c.queries.getAttributeValue(ctx, getAttributeValueParams{ + Fqn: pgtypeText(strings.ToLower(ident.AttributeValueFqn)), + }) + if err != nil { + return "", "", db.WrapIfKnownInvalidQueryErr(err) + } + attributeValueID = av.ID + default: + return "", "", db.ErrSelectIdentifierInvalid + } + + return actionID, attributeValueID, nil +} + +// validateRRAAVNamespaceConsistency ensures that action +// belongs to the same namespace as the RR val being created. When the RR is namespaced, +// the attribute value must also be in that namespace. When unnamespaced, the attribute value +// may have any namespace, but actions must be unnamespaced. +func (c PolicyDBClient) validateRRAAVNamespaceConsistency( + ctx context.Context, + targetNsID string, + attributeValueID string, + actionID string, +) error { + // Attribute value namespace check only applies when the RR is namespaced + if targetNsID != "" { + av, err := c.GetAttributeValue(ctx, attributeValueID) + if err != nil { + return db.WrapIfKnownInvalidQueryErr(err) + } + attr, err := c.GetAttribute(ctx, av.GetAttribute().GetId()) + if err != nil { + return db.WrapIfKnownInvalidQueryErr(err) + } + if attr.GetNamespace().GetId() != targetNsID { + return errors.Join(db.ErrNamespaceMismatch, + fmt.Errorf("attribute value namespace [%s] does not match the specified registered resource namespace [%s]", attr.GetNamespace().GetId(), targetNsID)) + } + } + + // Action namespace must match the RR namespace (or be unnamespaced when RR is unnamespaced). + if actionID != "" { + actionRows, err := c.queries.getActionsByIDs(ctx, []string{actionID}) + if err != nil { + return db.WrapIfKnownInvalidQueryErr(err) + } + if len(actionRows) == 0 { + return errors.Join(db.ErrMissingValue, fmt.Errorf("action [%s] was not found", actionID)) + } + for _, a := range actionRows { + actionNsID := UUIDToString(a.NamespaceID) + if actionNsID != targetNsID { + return errors.Join(db.ErrNamespaceMismatch, + fmt.Errorf("action [%s] namespace [%s] does not match the specified registered resource namespace [%s]", a.ID, actionNsID, targetNsID)) + } + } + } + + return nil +}