From 58a549455e811346f29e0f5b880dc95eeddff392 Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Wed, 7 Feb 2024 02:13:51 -0500 Subject: [PATCH] PR(WRITE-PERM): Add ACP checks on writes/mutations --- client/errors.go | 2 ++ db/collection.go | 23 ++++++++++++++++++++++- db/collection_delete.go | 14 ++++++++++++++ db/collection_update.go | 6 +++--- db/permission/check.go | 3 ++- 5 files changed, 43 insertions(+), 5 deletions(-) diff --git a/client/errors.go b/client/errors.go index 60ccac9669..1da4882ca5 100644 --- a/client/errors.go +++ b/client/errors.go @@ -44,6 +44,8 @@ var ( ErrFieldNotObject = errors.New("trying to access field on a non object type") ErrValueTypeMismatch = errors.New("value does not match indicated type") ErrDocumentNotFound = errors.New("no document for the given ID exists") + ErrInvalidACPPermToDeleteDocument = errors.New("invalid acp permission to delete the document") + ErrInvalidACPPermToUpdateDocument = errors.New("invalid acp permission to update the document") ErrInvalidUpdateTarget = errors.New("the target document to update is of invalid type") ErrInvalidUpdater = errors.New("the updater of a document is of invalid type") ErrInvalidDeleteTarget = errors.New("the target document to delete is of invalid type") diff --git a/db/collection.go b/db/collection.go index 4b366b4af3..284d58bb65 100644 --- a/db/collection.go +++ b/db/collection.go @@ -23,6 +23,7 @@ import ( ipld "github.com/ipfs/go-ipld-format" "github.com/sourcenetwork/immutable" + "github.com/sourcenetwork/defradb/acp" "github.com/sourcenetwork/defradb/client" "github.com/sourcenetwork/defradb/client/request" "github.com/sourcenetwork/defradb/core" @@ -1013,7 +1014,20 @@ func (c *collection) Update(ctx context.Context, doc *client.Document) error { // Should probably be smart about the update due to the MerkleCRDT overhead, shouldn't // add to the bloat. func (c *collection) update(ctx context.Context, txn datastore.Txn, doc *client.Document) error { - _, err := c.save(ctx, txn, doc, false) + // Stop the update if the correct permissions aren't there. + canUpdate, err := c.checkDocPermissionedAccess( + ctx, + acp.WritePermission, + doc.ID().String(), + ) + if err != nil { + return err + } + if !canUpdate { + return client.ErrInvalidACPPermToDeleteDocument + } + + _, err = c.save(ctx, txn, doc, false) if err != nil { return err } @@ -1052,6 +1066,9 @@ func (c *collection) Save(ctx context.Context, doc *client.Document) error { return c.commitImplicitTxn(ctx, txn) } +// save saves the document state. save MUST not be called outside the `c.create` +// and `c.update` methods as we wrap the acp logic within those methods. Calling +// save elsewhere could cause the omission of acp checks. func (c *collection) save( ctx context.Context, txn datastore.Txn, @@ -1323,6 +1340,10 @@ func (c *collection) exists( return true, false, nil } +// saveCompositeToMerkleCRDT saves the composite to the merkle CRDT. +// saveCompositeToMerkleCRDT MUST not be called outside the `c.save` +// and `c.applyDelete` methods as we wrap the acp logic around those methods. +// Calling it elsewhere could cause the omission of acp checks. func (c *collection) saveCompositeToMerkleCRDT( ctx context.Context, txn datastore.Txn, diff --git a/db/collection_delete.go b/db/collection_delete.go index f91b8e38f2..831408fc14 100644 --- a/db/collection_delete.go +++ b/db/collection_delete.go @@ -14,6 +14,7 @@ import ( "context" "fmt" + "github.com/sourcenetwork/defradb/acp" "github.com/sourcenetwork/defradb/client" "github.com/sourcenetwork/defradb/client/request" "github.com/sourcenetwork/defradb/core" @@ -242,6 +243,19 @@ func (c *collection) applyDelete( return NewErrDocumentDeleted(primaryKey.DocID) } + // Stop deletion of document if the correct permissions aren't there. + canDelete, err := c.checkDocPermissionedAccess( + ctx, + acp.WritePermission, + primaryKey.DocID, + ) + if err != nil { + return err + } + if !canDelete { + return client.ErrInvalidACPPermToDeleteDocument + } + dsKey := primaryKey.ToDataStoreKey() headset := clock.NewHeadSet( diff --git a/db/collection_update.go b/db/collection_update.go index f4fc1eef6e..985b2eed6f 100644 --- a/db/collection_update.go +++ b/db/collection_update.go @@ -141,7 +141,7 @@ func (c *collection) updateWithDocID( return nil, err } - _, err = c.save(ctx, txn, doc, false) + err = c.update(ctx, txn, doc) if err != nil { return nil, err } @@ -189,7 +189,7 @@ func (c *collection) updateWithIDs( return nil, err } - _, err = c.save(ctx, txn, doc, false) + err = c.update(ctx, txn, doc) if err != nil { return nil, err } @@ -277,7 +277,7 @@ func (c *collection) updateWithFilter( } } - _, err = c.save(ctx, txn, doc, false) + err = c.update(ctx, txn, doc) if err != nil { return nil, err } diff --git a/db/permission/check.go b/db/permission/check.go index d7dd8f1096..6e61dd64d6 100644 --- a/db/permission/check.go +++ b/db/permission/check.go @@ -19,9 +19,10 @@ package permission import ( "context" + "github.com/sourcenetwork/immutable" + "github.com/sourcenetwork/defradb/acp" "github.com/sourcenetwork/defradb/client" - "github.com/sourcenetwork/immutable" ) // CheckDocPermissionedAccessOnCollection handles the check, which tells us if access to the target