PR(FETCH): Check signatured & non-signatured done

acp/acp.go

@@ -60,11 +60,18 @@ type ACPModule interface {
 	// - docID here is the object identifier.
 	RegisterDocCreation(ctx context.Context, creator, policyID, resource, docID string) error
 
+	// IsDocRegistered returns true if the document was registered with ACP. Otherwise returns false or an error.
+	//
+	// Note:
+	// - resource here is the resource object name (likely collection name).
+	// - docID here is the object identifier we want to see was registered.
+	IsDocRegistered(ctx context.Context, policyID, resource, docID string) (bool, error)
+
 	// CheckDocAccess returns true if request has access to the document, otherwise returns false or an error.
 	//
 	// Note:
 	// - permission here is the type of permission we are checking for ("read" or "write").
 	// - resource here is the resource object name (likely collection name).
 	// - docID here is the object identifier.
-	CheckDocAccess(ctx context.Context, actorID, permission, policyID, resource, docID string) (bool, error)
+	CheckDocAccess(ctx context.Context, permission, actorID, policyID, resource, docID string) (bool, error)
 }

acp/acp_local.go

@@ -225,10 +225,40 @@ func (l *ACPLocalEmbedded) RegisterDocCreation(
 	return ErrObjectDidNotRegister
 }
 
+func (l *ACPLocalEmbedded) IsDocRegistered(
+	ctx context.Context,
+	policyID string,
+	resource string,
+	docID string,
+) (bool, error) {
+	queryObjectOwner := types.QueryObjectOwnerRequest{
+		PolicyId: policyID,
+		Object:   types.NewObject(resource, docID),
+	}
+
+	queryObjectOwnerResponse, err := l.localModule.GetQueryService().ObjectOwner(
+		l.localModule.GetCtx(),
+		&queryObjectOwner,
+	)
+	if err != nil {
+		log.FatalE(
+			ctx,
+			"failed to check if doc is registered alread with local acp module",
+			err,
+			logging.NewKV("PolicyID", policyID),
+			logging.NewKV("Resource", resource),
+			logging.NewKV("DocID", docID),
+		)
+		return false, err
+	}
+
+	return queryObjectOwnerResponse.IsRegistered, nil
+}
+
 func (l *ACPLocalEmbedded) CheckDocAccess(
 	ctx context.Context,
-	actorID string,
 	permission string,
+	actorID string,
 	policyID string,
 	resource string,
 	docID string,

db/collection.go

@@ -953,7 +953,9 @@ func (c *collection) getDocIDAndPrimaryKeyFromDoc(
 // - (!SignatureRequest, PermissionedCollection, !ModuleExists)  => Normal/Public - Don't Register with ACP
 // - (!SignatureRequest, !PermissionedCollection, !ModuleExists) => Normal/Public - Don't Register with ACP
 func (c *collection) tryRegisterDocWithACP(ctx context.Context, doc *client.Document) error {
+	// Check if acp module exists.
 	if c.db.ACPModule().HasValue() {
+		// Check if collection has policy.
 		if policyID, resourceName, hasPolicy := client.IsPermissioned(c); hasPolicy {
 			return c.db.ACPModule().Value().RegisterDocCreation(
 				ctx,

db/fetcher/fetcher.go

@@ -18,6 +18,8 @@ import (
 	"github.com/bits-and-blooms/bitset"
 	dsq "github.com/ipfs/go-datastore/query"
 
+	"github.com/sourcenetwork/immutable"
+
 	"github.com/sourcenetwork/defradb/acp"
 	"github.com/sourcenetwork/defradb/client"
 	"github.com/sourcenetwork/defradb/core"
@@ -26,7 +28,6 @@ import (
 	"github.com/sourcenetwork/defradb/db/base"
 	"github.com/sourcenetwork/defradb/planner/mapper"
 	"github.com/sourcenetwork/defradb/request/graphql/parser"
-	"github.com/sourcenetwork/immutable"
 )
 
 // ExecInfo contains statistics about the fetcher execution.
@@ -94,10 +95,10 @@ type DocumentFetcher struct {
 	order        []dsq.Order
 	curSpanIndex int
 
-	filter           *mapper.Filter
-	passedPermission bool // have valid permission to access
-	ranFilter        bool // did we run the filter
-	passedFilter     bool // did we pass the filter
+	filter                *mapper.Filter
+	passedPermissionCheck bool // have valid permission to access
+	ranFilter             bool // did we run the filter
+	passedFilter          bool // did we pass the filter
 
 	filterFields map[uint32]client.FieldDescription
 	selectFields map[uint32]client.FieldDescription
@@ -484,7 +485,7 @@ func (df *DocumentFetcher) processKV(kv *keyValue) error {
 			}
 		}
 		df.doc.id = []byte(kv.Key.DocID)
-		df.passedPermission = false
+		df.passedPermissionCheck = false
 		df.passedFilter = false
 		df.ranFilter = false
 
@@ -575,6 +576,72 @@ func (df *DocumentFetcher) FetchNext(ctx context.Context) (EncodedDocument, Exec
 	return encdoc, resultExecInfo, err
 }
 
+// runDocumentPermissionCheck handles the checking (while fetching) of the document with acp module,
+// according to our access logic based on weather (1) the request is permissioned,
+// (2) the collection is permissioned (has a policy), (3) acp module exists.
+//
+// Note: we only need to make a call to the acp module if (2) and (3) are true, where if (1) is true
+// we then check passes only if the document has proper access, otherwise if (1) is false then
+// the check passes only if the document is public (is not registered with acp module at all).
+//
+// Moreover 8 states, upon checking access:
+// (SignatureRequest, PermissionedCollection, ModuleExists)    => Must pass ACP check, unless public (not registered)
+// (SignatureRequest, PermissionedCollection, !ModuleExists)   => No check needed
+// (SignatureRequest, !PermissionedCollection, ModuleExists)   => No check needed
+// (SignatureRequest, !PermissionedCollection, !ModuleExists)  => No check needed
+// (!SignatureRequest, PermissionedCollection, ModuleExists)   => Only public (No access if registered with ACP)
+// (!SignatureRequest, !PermissionedCollection, ModuleExists)  => No check needed
+// (!SignatureRequest, PermissionedCollection, !ModuleExists)  => No check needed
+// (!SignatureRequest, !PermissionedCollection, !ModuleExists) => No check needed
+func (df *DocumentFetcher) runDocumentPermissionCheck(ctx context.Context) error {
+	// TODO-ACP: Add an additional check to handle without signature identity case once signatures are implemented
+	hasSignature := true
+	// Check if acp module exists.
+	if df.acp.HasValue() {
+		// Check if collection has policy.
+		if policyID, resourceName, hasPolicy := client.IsPermissioned(df.col); hasPolicy {
+			if hasSignature {
+				// TODO-ACP: Add an additional check to handle without signature identity case:
+				hasAccess, err := df.acp.Value().CheckDocAccess(
+					ctx,
+					"read", // TODO-ACP: Replace with permission
+					"cosmos1zzg43wdrhmmk89z3pmejwete2kkd4a3vn7w969", // TODO-ACP: Replace with signature identity
+					policyID,
+					resourceName,
+					df.kv.Key.DocID,
+				)
+				if err != nil {
+					df.passedPermissionCheck = false
+					return err
+				}
+				df.passedPermissionCheck = hasAccess
+				return nil
+			} else {
+				// If does not have signature, we need to make sure we don't operate on registered documents.
+				// In this case actor only has access to the public (unregistered) documents.
+				isRegistered, err := df.acp.Value().IsDocRegistered(
+					ctx,
+					policyID,
+					resourceName,
+					df.kv.Key.DocID,
+				)
+				if err != nil {
+					df.passedPermissionCheck = false
+					return err
+				}
+
+				// Check passes if document is NOT registered. If it is registered then the
+				// document must not be accessed.
+				df.passedPermissionCheck = !isRegistered
+				return nil
+			}
+		}
+	}
+
+	df.passedPermissionCheck = true
+	return nil
+}
+
 func (df *DocumentFetcher) fetchNext(ctx context.Context) (EncodedDocument, ExecInfo, error) {
 	if df.kvEnd {
 		return nil, ExecInfo{}, nil
@@ -583,9 +650,6 @@ func (df *DocumentFetcher) fetchNext(ctx context.Context) (EncodedDocument, Exec
 	if df.kv == nil {
 		return nil, ExecInfo{}, client.NewErrUninitializeProperty("DocumentFetcher", "kv")
 	}
-	// save the DocID of the current kv pair so we can track when we cross the doc pair boundries
-	// keyparts := df.kv.Key.List()
-	// key := keyparts[len(keyparts)-2]
 
 	prevExecInfo := df.execInfo
 	defer func() { df.execInfo.Add(prevExecInfo) }()
@@ -594,8 +658,7 @@ func (df *DocumentFetcher) fetchNext(ctx context.Context) (EncodedDocument, Exec
 	// we'll know when were done when either
 	// A) Reach the end of the iterator
 	for {
-		err := df.processKV(df.kv)
-		if err != nil {
+		if err := df.processKV(df.kv); err != nil {
 			return nil, ExecInfo{}, err
 		}
 
@@ -616,16 +679,29 @@ func (df *DocumentFetcher) fetchNext(ctx context.Context) (EncodedDocument, Exec
 			}
 		}
 
+		// Check if can access document with current permissions/signature.
+		if !df.passedPermissionCheck {
+			if err := df.runDocumentPermissionCheck(ctx); err != nil {
+				return nil, ExecInfo{}, err
+			}
+		}
+
 		// if we don't pass the filter (ran and pass)
 		// theres no point in collecting other select fields
 		// so we seek to the next doc
-		spansDone, docDone, err := df.nextKey(ctx, !df.passedFilter && df.ranFilter)
+		spansDone, docDone, err := df.nextKey(ctx, !df.passedPermissionCheck || !df.passedFilter && df.ranFilter)
+
 		if err != nil {
 			return nil, ExecInfo{}, err
 		}
 
-		if docDone {
-			df.execInfo.DocsFetched++
+		if !docDone {
+			continue
+		}
+
+		df.execInfo.DocsFetched++
+
+		if df.passedPermissionCheck {
 			if df.filter != nil {
 				// if we passed, return
 				if df.passedFilter {
@@ -646,21 +722,11 @@ func (df *DocumentFetcher) fetchNext(ctx context.Context) (EncodedDocument, Exec
 			} else {
 				return df.doc, df.execInfo, nil
 			}
+		}
 
-			if !spansDone {
-				continue
-			}
-
+		if spansDone {
 			return nil, df.execInfo, nil
 		}
-
-		// // crossed document kv boundary?
-		// // if so, return document
-		// newkeyparts := df.kv.Key.List()
-		// newKey := newkeyparts[len(newkeyparts)-2]
-		// if newKey != key {
-		// 	return df.doc, nil
-		// }
 	}
 }
 

db/fetcher/indexer.go

@@ -13,13 +13,14 @@ package fetcher
 import (
 	"context"
 
+	"github.com/sourcenetwork/immutable"
+
 	"github.com/sourcenetwork/defradb/acp"
 	"github.com/sourcenetwork/defradb/client"
 	"github.com/sourcenetwork/defradb/core"
 	"github.com/sourcenetwork/defradb/datastore"
 	"github.com/sourcenetwork/defradb/db/base"
 	"github.com/sourcenetwork/defradb/planner/mapper"
-	"github.com/sourcenetwork/immutable"
 )
 
 // IndexFetcher is a fetcher that fetches documents by index.

db/fetcher/versioned.go

@@ -19,6 +19,8 @@ import (
 	ds "github.com/ipfs/go-datastore"
 	format "github.com/ipfs/go-ipld-format"
 
+	"github.com/sourcenetwork/immutable"
+
 	"github.com/sourcenetwork/defradb/acp"
 	"github.com/sourcenetwork/defradb/client"
 	"github.com/sourcenetwork/defradb/core"
@@ -28,7 +30,6 @@ import (
 	"github.com/sourcenetwork/defradb/errors"
 	merklecrdt "github.com/sourcenetwork/defradb/merkle/crdt"
 	"github.com/sourcenetwork/defradb/planner/mapper"
-	"github.com/sourcenetwork/immutable"
 )
 
 var (

lens/fetcher.go

@@ -16,14 +16,15 @@ import (
 
 	"github.com/fxamacker/cbor/v2"
 
+	"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"
 	"github.com/sourcenetwork/defradb/datastore"
 	"github.com/sourcenetwork/defradb/db/fetcher"
 	"github.com/sourcenetwork/defradb/planner/mapper"
-	"github.com/sourcenetwork/immutable"
 )
 
 // todo: The code in here can be significantly simplified with:

tests/integration/acp/simple_test.go

@@ -93,3 +93,75 @@ func TestACP_CreateAndRead(t *testing.T) {
 
 	testUtils.ExecuteTestCase(t, test)
 }
+
+//func TestACP_CreateAndRead_WithoutSignature(t *testing.T) {
+//	test := testUtils.TestCase{
+//		Description: "Simple acp create and read",
+//		Actions: []any{
+//			testUtils.AddPolicy{
+//				Policy: `
+//                    description: a test policy which marks a collection in a database as a resource
+//
+//                    resources:
+//                      users:
+//                        permissions:
+//                          read:
+//                            expr: owner + reader
+//                          write:
+//                            expr: owner
+//
+//                        relations:
+//                          owner:
+//                            types:
+//                              - actor
+//                          reader:
+//                            types:
+//                              - actor
+//                          admin:
+//                            manages:
+//                              - reader
+//                            types:
+//                              - actor
+//
+//                    actor:
+//                      name: actor
+//                `,
+//				IsYAML:           true,
+//				Creator:          "cosmos1zzg43wdrhmmk89z3pmejwete2kkd4a3vn7w969",
+//				ExpectedPolicyID: "53980e762616fcffbe76307995895e862f87ef3f21d509325d1dc772a770b001",
+//				ExpectedError:    "",
+//			},
+//
+//			testUtils.SchemaUpdate{
+//				Schema: `
+//					type Users @policy(id: "53980e762616fcffbe76307995895e862f87ef3f21d509325d1dc772a770b001", resource: "users") {
+//						name: String
+//						age: Int
+//					}
+//				`,
+//			},
+//
+//			testUtils.CreateDoc{
+//				Doc: `{
+//					"name": "John",
+//					"age": 27
+//				}`,
+//			},
+//
+//			testUtils.Request{
+//				Request: `
+//					query {
+//						Users {
+//							_docID
+//							name
+//							age
+//						}
+//					}
+//				`,
+//				Results: []map[string]any{},
+//			},
+//		},
+//	}
+//
+//	testUtils.ExecuteTestCase(t, test)
+//}