Skip to content

Commit

Permalink
PR(WRITE-PERM): Add ACP checks on writes/mutations
Browse files Browse the repository at this point in the history
  • Loading branch information
shahzadlone committed Feb 8, 2024
1 parent 667ff0d commit 5625931
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 5 deletions.
2 changes: 2 additions & 0 deletions client/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
23 changes: 22 additions & 1 deletion db/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -945,7 +946,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
}
Expand Down Expand Up @@ -984,6 +998,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,
Expand Down Expand Up @@ -1271,6 +1288,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,
Expand Down
14 changes: 14 additions & 0 deletions db/collection_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions db/collection_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 2 additions & 1 deletion db/permission/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 5625931

Please sign in to comment.