Skip to content

Commit 3cdd1b2

Browse files
authored
feat(policy): subject condition sets prune service/db (#1688)
Resolves #1178
1 parent ce289a4 commit 3cdd1b2

File tree

7 files changed

+330
-188
lines changed

7 files changed

+330
-188
lines changed

protocol/go/policy/objects.pb.go

Lines changed: 187 additions & 186 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

service/integration/subject_mappings_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ func (s *SubjectMappingsSuite) SetupSuite() {
2626
c := *Config
2727
c.DB.Schema = "test_opentdf_subject_mappings"
2828
s.db = fixtures.NewDBInterface(c)
29+
s.ctx = context.Background()
2930
s.f = fixtures.NewFixture(s.db)
3031
s.ctx = context.Background()
3132
s.f.Provision()
@@ -637,6 +638,64 @@ func (s *SubjectMappingsSuite) TestDeleteSubjectConditionSet_WithNonExistentId_F
637638
s.Require().ErrorIs(err, db.ErrNotFound)
638639
}
639640

641+
func (s *SubjectMappingsSuite) TestDeleteAllUnmappedSubjectConditionSets() {
642+
// create two new subject condition sets, create a subject mapping with one of them, then verify only the unmapped is deleted
643+
newSCS := &subjectmapping.SubjectConditionSetCreate{
644+
SubjectSets: []*policy.SubjectSet{
645+
{
646+
ConditionGroups: []*policy.ConditionGroup{
647+
{
648+
Conditions: []*policy.Condition{
649+
{
650+
SubjectExternalSelectorValue: ".some_selector",
651+
},
652+
},
653+
},
654+
},
655+
},
656+
},
657+
}
658+
659+
unmapped, err := s.db.PolicyClient.CreateSubjectConditionSet(s.ctx, newSCS)
660+
s.Require().NoError(err)
661+
s.NotNil(unmapped)
662+
663+
mapped, err := s.db.PolicyClient.CreateSubjectConditionSet(s.ctx, newSCS)
664+
s.Require().NoError(err)
665+
s.NotNil(mapped)
666+
667+
sm, err := s.db.PolicyClient.CreateSubjectMapping(s.ctx, &subjectmapping.CreateSubjectMappingRequest{
668+
AttributeValueId: s.f.GetAttributeValueKey("example.net/attr/attr1/value/value2").ID,
669+
Actions: []*policy.Action{fixtureActions[Decrypt]},
670+
ExistingSubjectConditionSetId: mapped.GetId(),
671+
})
672+
s.Require().NoError(err)
673+
s.NotNil(sm)
674+
675+
deleted, err := s.db.PolicyClient.DeleteAllUnmappedSubjectConditionSets(s.ctx)
676+
s.Require().NoError(err)
677+
s.NotEmpty(deleted)
678+
unmappedDeleted := true
679+
mappedDeleted := false
680+
for _, scs := range deleted {
681+
deletedID := scs.GetId()
682+
if deletedID == unmapped.GetId() {
683+
unmappedDeleted = true
684+
}
685+
if deletedID == mapped.GetId() {
686+
mappedDeleted = true
687+
}
688+
}
689+
s.True(unmappedDeleted)
690+
s.False(mappedDeleted)
691+
692+
// cannot get after pruning
693+
got, err := s.db.PolicyClient.GetSubjectConditionSet(s.ctx, unmapped.GetId())
694+
s.Nil(got)
695+
s.Require().Error(err)
696+
s.ErrorIs(err, db.ErrNotFound)
697+
}
698+
640699
func (s *SubjectMappingsSuite) TestUpdateSubjectConditionSet_NewSubjectSets() {
641700
// create a new one, update nothing but the subject sets, and verify the solo update
642701
newConditionSet := &subjectmapping.SubjectConditionSetCreate{

service/policy/db/query.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,11 @@ WHERE id = $1;
735735
-- name: DeleteSubjectConditionSet :execrows
736736
DELETE FROM subject_condition_set WHERE id = $1;
737737

738+
-- name: DeleteAllUnmappedSubjectConditionSets :many
739+
DELETE FROM subject_condition_set
740+
WHERE id NOT IN (SELECT DISTINCT sm.subject_condition_set_id FROM subject_mappings sm)
741+
RETURNING id;
742+
738743
----------------------------------------------------------------
739744
-- SUBJECT MAPPINGS
740745
----------------------------------------------------------------

service/policy/db/query.sql.go

Lines changed: 31 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

service/policy/db/subject_mappings.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,25 @@ func (c PolicyDBClient) DeleteSubjectConditionSet(ctx context.Context, id string
222222
}, nil
223223
}
224224

225+
// Deletes/prunes all subject condition sets not referenced within a subject mapping
226+
func (c PolicyDBClient) DeleteAllUnmappedSubjectConditionSets(ctx context.Context) ([]*policy.SubjectConditionSet, error) {
227+
deletedIDs, err := c.Queries.DeleteAllUnmappedSubjectConditionSets(ctx)
228+
if err != nil {
229+
return nil, db.WrapIfKnownInvalidQueryErr(err)
230+
}
231+
if len(deletedIDs) == 0 {
232+
return nil, db.ErrNotFound
233+
}
234+
235+
setList := make([]*policy.SubjectConditionSet, len(deletedIDs))
236+
for i, id := range deletedIDs {
237+
setList[i] = &policy.SubjectConditionSet{
238+
Id: id,
239+
}
240+
}
241+
return setList, nil
242+
}
243+
225244
/*
226245
Subject Mappings
227246
*/

service/policy/objects.proto

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ message SubjectMapping {
154154
*/
155155
message Condition {
156156
// a selector for a field value on a flattened Entity Representation (such as from idP/LDAP)
157-
string subject_external_selector_value = 1;
157+
string subject_external_selector_value = 1 [ (buf.validate.field).required = true ];
158158

159159
// the evaluation operator of relation
160160
SubjectMappingOperatorEnum operator = 2 [
@@ -163,7 +163,7 @@ message Condition {
163163
];
164164

165165
// list of comparison values for the result of applying the subject_external_selector_value on a flattened Entity Representation (Subject), evaluated by the operator
166-
repeated string subject_external_values = 3;
166+
repeated string subject_external_values = 3 [(buf.validate.field).repeated.min_items = 1];
167167
}
168168

169169
// A collection of Conditions evaluated by the boolean_operator provided

service/policy/subjectmapping/subject_mapping.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,3 +293,30 @@ func (s SubjectMappingService) DeleteSubjectConditionSet(ctx context.Context,
293293
}
294294
return rsp, nil
295295
}
296+
297+
func (s SubjectMappingService) DeleteAllUnmappedSubjectConditionSets(ctx context.Context,
298+
_ *sm.DeleteAllUnmappedSubjectConditionSetsRequest,
299+
) (*sm.DeleteAllUnmappedSubjectConditionSetsResponse, error) {
300+
rsp := &sm.DeleteAllUnmappedSubjectConditionSetsResponse{}
301+
s.logger.Debug("deleting all unmapped subject condition sets")
302+
303+
auditParams := audit.PolicyEventParams{
304+
ActionType: audit.ActionTypeDelete,
305+
ObjectType: audit.ObjectTypeConditionSet,
306+
}
307+
308+
deleted, err := s.dbClient.DeleteAllUnmappedSubjectConditionSets(ctx)
309+
if err != nil {
310+
s.logger.Audit.PolicyCRUDFailure(ctx, auditParams)
311+
return nil, db.StatusifyError(err, db.ErrTextDeletionFailed)
312+
}
313+
314+
// Log each pruned subject condition set to audit
315+
for _, scs := range deleted {
316+
auditParams.ObjectID = scs.GetId()
317+
s.logger.Audit.PolicyCRUDSuccess(ctx, auditParams)
318+
}
319+
320+
rsp.SubjectConditionSets = deleted
321+
return rsp, nil
322+
}

0 commit comments

Comments
 (0)