From bcd75fc9055a8a999d807e53e1895c15ecd5ea1f Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 5 Mar 2024 11:09:45 -0500 Subject: [PATCH] PR(IDENTITY-FIX): Fix Panic On Private Doc Update This was bug that caused a panic when a request without an identity tried to update a document that was private (registered with an identity). The test that asserts the fix is in the next few commits. The test name is called: `TestACP_CreateWithIdentityAndUpdateWithoutIdentity_CanNotUpdate` --- client/collection.go | 2 ++ client/errors.go | 1 + db/collection_get.go | 5 +++++ db/collection_index.go | 4 +++- 4 files changed, 11 insertions(+), 1 deletion(-) diff --git a/client/collection.go b/client/collection.go index 9a1f437503..886d5c6b47 100644 --- a/client/collection.go +++ b/client/collection.go @@ -185,6 +185,8 @@ type Collection interface { // Get returns the document with the given DocID. // // Returns an ErrDocumentNotFound if a document matching the given DocID is not found. + // + // Returns an ErrDocumentNotAccessible if DocID is inaccessible. Get( ctx context.Context, identity immutable.Option[string], diff --git a/client/errors.go b/client/errors.go index 454495126e..6b987fd7e9 100644 --- a/client/errors.go +++ b/client/errors.go @@ -45,6 +45,7 @@ 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") + ErrDocumentNotAccessible = errors.New("document is not accessible") ErrPolicyAddFailureACPModuleNotFound = errors.New("failure adding policy because ACP module was not found") ErrInvalidACPPermToDeleteDocument = errors.New("invalid acp permission to delete the document") ErrInvalidACPPermToUpdateDocument = errors.New("invalid acp permission to update the document") diff --git a/db/collection_get.go b/db/collection_get.go index 46ca9c058d..15915f572c 100644 --- a/db/collection_get.go +++ b/db/collection_get.go @@ -48,6 +48,11 @@ func (c *collection) Get( if err != nil { return nil, err } + + if doc == nil { + return nil, client.ErrDocumentNotAccessible + } + return doc, c.commitImplicitTxn(ctx, txn) } diff --git a/db/collection_index.go b/db/collection_index.go index 386f970c92..dbeaedca8d 100644 --- a/db/collection_index.go +++ b/db/collection_index.go @@ -134,9 +134,11 @@ func (c *collection) updateIndexedDoc( if err != nil { return err } + // TODO-ACP: https://github.com/sourcenetwork/defradb/issues/2365 - ACP <> Indexing, possibly also check + // and handle the case of when oldDoc == nil (will be nil if inaccessible document). oldDoc, err := c.get( ctx, - acpIdentity.NoIdentity, // TODO-ACP: https://github.com/sourcenetwork/defradb/issues/2365 - ACP <> Indexing + acpIdentity.NoIdentity, txn, c.getPrimaryKeyFromDocID(doc.ID()), c.Definition().CollectIndexedFields(), false,