diff --git a/service/integration/obligation_triggers_test.go b/service/integration/obligation_triggers_test.go index 504010f455..785b1ec599 100644 --- a/service/integration/obligation_triggers_test.go +++ b/service/integration/obligation_triggers_test.go @@ -27,6 +27,7 @@ const ( obligationName = "test-obligation" obligationValue = "test-obligation-value" clientID = "test-client-id" + secondClientID = "test-client-id-2" ) type ObligationTriggersSuite struct { @@ -171,6 +172,54 @@ func (s *ObligationTriggersSuite) Test_CreateObligationTrigger_WithIDs_Success() s.Require().Equal("test", trigger.GetMetadata().GetLabels()["source"]) } +func (s *ObligationTriggersSuite) Test_CreateObligationTrigger_SameTupleDifferentClients_Success() { + req := &obligations.AddObligationTriggerRequest{ + ObligationValue: &common.IdFqnIdentifier{Id: s.obligationValue.GetId()}, + AttributeValue: &common.IdFqnIdentifier{Id: s.attributeValue.GetId()}, + Action: &common.IdNameIdentifier{Id: s.action.GetId()}, + Context: &policy.RequestContext{ + Pep: &policy.PolicyEnforcementPoint{ + ClientId: clientID, + }, + }, + } + + firstTrigger, err := s.db.PolicyClient.CreateObligationTrigger(s.ctx, req) + s.Require().NoError(err) + s.triggerIDsToClean = append(s.triggerIDsToClean, firstTrigger.GetId()) + s.validateTriggerWithDefaults(firstTrigger, true) + + req.Context.Pep.ClientId = secondClientID + secondTrigger, err := s.db.PolicyClient.CreateObligationTrigger(s.ctx, req) + s.Require().NoError(err) + s.triggerIDsToClean = append(s.triggerIDsToClean, secondTrigger.GetId()) + s.Require().NotEqual(firstTrigger.GetId(), secondTrigger.GetId()) + s.Require().Len(secondTrigger.GetContext(), 1) + s.Require().Equal(secondClientID, secondTrigger.GetContext()[0].GetPep().GetClientId()) +} + +func (s *ObligationTriggersSuite) Test_CreateObligationTrigger_SameTupleSameClient_Fails() { + req := &obligations.AddObligationTriggerRequest{ + ObligationValue: &common.IdFqnIdentifier{Id: s.obligationValue.GetId()}, + AttributeValue: &common.IdFqnIdentifier{Id: s.attributeValue.GetId()}, + Action: &common.IdNameIdentifier{Id: s.action.GetId()}, + Context: &policy.RequestContext{ + Pep: &policy.PolicyEnforcementPoint{ + ClientId: clientID, + }, + }, + } + + firstTrigger, err := s.db.PolicyClient.CreateObligationTrigger(s.ctx, req) + s.Require().NoError(err) + s.triggerIDsToClean = append(s.triggerIDsToClean, firstTrigger.GetId()) + + duplicateTrigger, err := s.db.PolicyClient.CreateObligationTrigger(s.ctx, req) + s.Require().Error(err) + s.Require().ErrorIs(err, db.ErrUniqueConstraintViolation) + s.Nil(duplicateTrigger) +} + func (s *ObligationTriggersSuite) Test_CreateObligationTrigger_NoCtx_Success() { trigger, err := s.db.PolicyClient.CreateObligationTrigger(s.ctx, &obligations.AddObligationTriggerRequest{ ObligationValue: &common.IdFqnIdentifier{Id: s.obligationValue.GetId()}, diff --git a/service/policy/db/migrations/20260306000000_make_obligation_trigger_uniqueness_client_aware.md b/service/policy/db/migrations/20260306000000_make_obligation_trigger_uniqueness_client_aware.md new file mode 100644 index 0000000000..ff768c0a11 --- /dev/null +++ b/service/policy/db/migrations/20260306000000_make_obligation_trigger_uniqueness_client_aware.md @@ -0,0 +1,31 @@ +# Make Obligation Trigger Uniqueness Client-Aware + +This migration updates uniqueness semantics for `obligation_triggers` after the +introduction of optional `client_id` scoping. + +## Why + +The previous unique constraint only considered: + +- `obligation_value_id` +- `action_id` +- `attribute_value_id` + +That prevented creating multiple triggers for different PEP clients when all +other fields were the same. + +## Changes + +1. Drop the existing table-level unique constraint on: + - `(obligation_value_id, action_id, attribute_value_id)` + - Includes handling historical truncated constraint names in Postgres. +2. Add partial unique index for unscoped triggers: + - `(obligation_value_id, action_id, attribute_value_id)` where `client_id IS NULL` +3. Add partial unique index for client-scoped triggers: + - `(obligation_value_id, action_id, attribute_value_id, client_id)` where `client_id IS NOT NULL` + +## Resulting Behavior + +- Allows one unscoped trigger per obligation/action/attribute tuple. +- Allows one scoped trigger per unique `client_id` for the same tuple. +- Prevents duplicate scoped triggers for the same `client_id`. diff --git a/service/policy/db/migrations/20260306000000_make_obligation_trigger_uniqueness_client_aware.sql b/service/policy/db/migrations/20260306000000_make_obligation_trigger_uniqueness_client_aware.sql new file mode 100644 index 0000000000..0f95bc22e3 --- /dev/null +++ b/service/policy/db/migrations/20260306000000_make_obligation_trigger_uniqueness_client_aware.sql @@ -0,0 +1,30 @@ +-- +goose Up +-- +goose StatementBegin +-- Make trigger uniqueness aware of optional client_id scoping. +ALTER TABLE IF EXISTS obligation_triggers +DROP CONSTRAINT IF EXISTS obligation_triggers_obligation_value_id_action_id_attribute_value_id_key; + +ALTER TABLE IF EXISTS obligation_triggers +DROP CONSTRAINT IF EXISTS obligation_triggers_obligation_value_id_action_id_attribute_key; + +ALTER TABLE IF EXISTS obligation_triggers +DROP CONSTRAINT IF EXISTS obligation_triggers_obligation_value_id_action_id_attribute_val; + +CREATE UNIQUE INDEX IF NOT EXISTS obligation_triggers_unscoped_unique_idx +ON obligation_triggers (obligation_value_id, action_id, attribute_value_id) +WHERE client_id IS NULL; + +CREATE UNIQUE INDEX IF NOT EXISTS obligation_triggers_scoped_unique_idx +ON obligation_triggers (obligation_value_id, action_id, attribute_value_id, client_id) +WHERE client_id IS NOT NULL; +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +DROP INDEX IF EXISTS obligation_triggers_scoped_unique_idx; +DROP INDEX IF EXISTS obligation_triggers_unscoped_unique_idx; + +ALTER TABLE IF EXISTS obligation_triggers +ADD CONSTRAINT obligation_triggers_obligation_value_id_action_id_attribute_key +UNIQUE (obligation_value_id, action_id, attribute_value_id); +-- +goose StatementEnd diff --git a/service/policy/db/schema_erd.md b/service/policy/db/schema_erd.md index bb35cbd18e..78b82f73f9 100644 --- a/service/policy/db/schema_erd.md +++ b/service/policy/db/schema_erd.md @@ -36,7 +36,7 @@ erDiagram attribute_definitions { boolean active "Active/Inactive state" - boolean allow_traversal + boolean allow_traversal "Whether or not to allow platform to return the definition key when encrypting, if the value specified is missing." timestamp_with_time_zone created_at uuid id PK "Primary key for the table" jsonb metadata "Metadata for the attribute definition (see protos for structure)" @@ -55,11 +55,6 @@ erDiagram uuid value_id FK,UK "Foreign key to the attribute value" } - attribute_namespace_certificates { - uuid certificate_id PK,FK "Foreign key to the certificate" - uuid namespace_id PK,FK "Foreign key to the namespace" - } - attribute_namespace_key_access_grants { uuid key_access_server_id PK,FK "Foreign key to the KAS registration" uuid namespace_id PK,FK "Foreign key to the namespace of the KAS grant" @@ -104,14 +99,6 @@ erDiagram uuid key_access_server_key_id FK } - certificates { - timestamp_with_time_zone created_at "Timestamp when the certificate was created" - uuid id PK "Unique identifier for the certificate" - jsonb metadata "Optional metadata for the certificate" - text pem "PEM format - Base64-encoded DER certificate (not PEM; no headers/footers)" - timestamp_with_time_zone updated_at "Timestamp when the certificate was last updated" - } - goose_db_version { integer id PK boolean is_applied @@ -166,13 +153,13 @@ erDiagram } obligation_triggers { - uuid action_id FK,UK - uuid attribute_value_id FK,UK + uuid action_id FK + uuid attribute_value_id FK text client_id "Holds the client_id associated with this trigger." timestamp_with_time_zone created_at uuid id PK jsonb metadata - uuid obligation_value_id FK,UK + uuid obligation_value_id FK timestamp_with_time_zone updated_at } @@ -290,8 +277,6 @@ erDiagram attribute_values }o--|| attribute_definitions : "attribute_definition_id" attribute_fqns }o--|| attribute_namespaces : "namespace_id" attribute_fqns }o--|| attribute_values : "value_id" - attribute_namespace_certificates }o--|| attribute_namespaces : "namespace_id" - attribute_namespace_certificates }o--|| certificates : "certificate_id" attribute_namespace_key_access_grants }o--|| attribute_namespaces : "namespace_id" attribute_namespace_key_access_grants }o--|| key_access_servers : "key_access_server_id" attribute_namespace_public_key_map }o--|| attribute_namespaces : "namespace_id"