Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
49 changes: 49 additions & 0 deletions service/integration/obligation_triggers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const (
obligationName = "test-obligation"
obligationValue = "test-obligation-value"
clientID = "test-client-id"
secondClientID = "test-client-id-2"
)

type ObligationTriggersSuite struct {
Expand Down Expand Up @@ -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()},
Expand Down
Original file line number Diff line number Diff line change
@@ -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`.
Original file line number Diff line number Diff line change
@@ -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
23 changes: 4 additions & 19 deletions service/policy/db/schema_erd.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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"
Expand Down
Loading