PR(MAIN): Assume read if only has write permission.

acp/acp.go

@@ -89,10 +89,11 @@ type ACP interface {
 	// Otherwise if check failed then an error is returned (and the boolean result should not be used).
 	//
 	// Note(s):
-	// - permission here is a valid DPI permission we are checking for ("read" or "write").
+	// - permissions here is a valid DPI permission we are checking for ("read" or "write").
+	// - returns true if has access to ANY of the given permissions.
 	CheckDocAccess(
 		ctx context.Context,
-		permission DPIPermission,
+		permissions []DPIPermission,
 		actorID string,
 		policyID string,
 		resourceName string,

acp/acp_local_test.go

@@ -485,7 +485,7 @@ func Test_LocalACP_InMemory_CheckDocAccess_TrueIfHaveAccessFalseIfNotErrorOtherw
 	// Invalid empty arguments such that we can't check doc access.
 	hasAccess, errCheckDocAccess := localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity1.DID,
 		validPolicyID,
 		"",
@@ -498,7 +498,7 @@ func Test_LocalACP_InMemory_CheckDocAccess_TrueIfHaveAccessFalseIfNotErrorOtherw
 	// Check document accesss for a document that does not exist.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity1.DID,
 		validPolicyID,
 		"users",
@@ -520,7 +520,7 @@ func Test_LocalACP_InMemory_CheckDocAccess_TrueIfHaveAccessFalseIfNotErrorOtherw
 	// Now check using correct identity if it has access.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity1.DID,
 		validPolicyID,
 		"users",
@@ -532,7 +532,7 @@ func Test_LocalACP_InMemory_CheckDocAccess_TrueIfHaveAccessFalseIfNotErrorOtherw
 	// Now check using wrong identity, it should not have access.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity2.DID,
 		validPolicyID,
 		"users",
@@ -571,7 +571,7 @@ func Test_LocalACP_PersistentMemory_CheckDocAccess_TrueIfHaveAccessFalseIfNotErr
 	// Invalid empty arguments such that we can't check doc access.
 	hasAccess, errCheckDocAccess := localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity1.DID,
 		validPolicyID,
 		"",
@@ -584,7 +584,7 @@ func Test_LocalACP_PersistentMemory_CheckDocAccess_TrueIfHaveAccessFalseIfNotErr
 	// Check document accesss for a document that does not exist.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity1.DID,
 		validPolicyID,
 		"users",
@@ -606,7 +606,7 @@ func Test_LocalACP_PersistentMemory_CheckDocAccess_TrueIfHaveAccessFalseIfNotErr
 	// Now check using correct identity if it has access.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity1.DID,
 		validPolicyID,
 		"users",
@@ -618,7 +618,7 @@ func Test_LocalACP_PersistentMemory_CheckDocAccess_TrueIfHaveAccessFalseIfNotErr
 	// Now check using wrong identity, it should not have access.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity2.DID,
 		validPolicyID,
 		"users",
@@ -638,7 +638,7 @@ func Test_LocalACP_PersistentMemory_CheckDocAccess_TrueIfHaveAccessFalseIfNotErr
 	// Now check again after the restart using correct identity if it still has access.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity1.DID,
 		validPolicyID,
 		"users",
@@ -650,7 +650,7 @@ func Test_LocalACP_PersistentMemory_CheckDocAccess_TrueIfHaveAccessFalseIfNotErr
 	// Now check again after restart using wrong identity, it should continue to not have access.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity2.DID,
 		validPolicyID,
 		"users",
@@ -696,7 +696,7 @@ func Test_LocalACP_InMemory_AddDocActorRelationship_FalseIfExistsBeforeTrueIfNoO
 	// Other identity does not have access yet.
 	hasAccess, errCheckDocAccess := localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity2.DID,
 		validPolicyID,
 		"users",
@@ -734,7 +734,7 @@ func Test_LocalACP_InMemory_AddDocActorRelationship_FalseIfExistsBeforeTrueIfNoO
 	// Now the other identity has access.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity2.DID,
 		validPolicyID,
 		"users",
@@ -783,7 +783,7 @@ func Test_LocalACP_PersistentMemory_AddDocActorRelationship_FalseIfExistsBeforeT
 	// Other identity does not have access yet.
 	hasAccess, errCheckDocAccess := localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity2.DID,
 		validPolicyID,
 		"users",
@@ -821,7 +821,7 @@ func Test_LocalACP_PersistentMemory_AddDocActorRelationship_FalseIfExistsBeforeT
 	// Now the other identity has access.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity2.DID,
 		validPolicyID,
 		"users",
@@ -841,7 +841,7 @@ func Test_LocalACP_PersistentMemory_AddDocActorRelationship_FalseIfExistsBeforeT
 	// Now check again after the restart that the second identity still has access.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity2.DID,
 		validPolicyID,
 		"users",
@@ -900,7 +900,7 @@ func Test_LocalACP_InMemory_DeleteDocActorRelationship_TrueIfFoundAndDeletedFals
 	// Now the other identity has access.
 	hasAccess, errCheckDocAccess := localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity2.DID,
 		validPolicyID,
 		"users",
@@ -938,7 +938,7 @@ func Test_LocalACP_InMemory_DeleteDocActorRelationship_TrueIfFoundAndDeletedFals
 	// Other identity now has no access again.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity2.DID,
 		validPolicyID,
 		"users",
@@ -1000,7 +1000,7 @@ func Test_LocalACP_PersistentMemory_DeleteDocActorRelationship_TrueIfFoundAndDel
 	// Now the other identity has access.
 	hasAccess, errCheckDocAccess := localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity2.DID,
 		validPolicyID,
 		"users",
@@ -1038,7 +1038,7 @@ func Test_LocalACP_PersistentMemory_DeleteDocActorRelationship_TrueIfFoundAndDel
 	// Other identity now has no access again.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity2.DID,
 		validPolicyID,
 		"users",
@@ -1058,7 +1058,7 @@ func Test_LocalACP_PersistentMemory_DeleteDocActorRelationship_TrueIfFoundAndDel
 	// Now check again after the restart that the second identity still has no access.
 	hasAccess, errCheckDocAccess = localACP.CheckDocAccess(
 		ctx,
-		ReadPermission,
+		[]DPIPermission{ReadPermission},
 		identity2.DID,
 		validPolicyID,
 		"users",

acp/dpi.go

@@ -22,6 +22,14 @@ const (
 	WritePermission
 )
 
+// PermissionsThatImplyRead is a list of any permissions that if we have, we assume that the user can read.
+// This is because for DefraDB's purposes if an identity has access to the write permission, then they don't
+// need to explicitly have read permission inorder to read, we can just imply that they have read access.
+var PermissionsThatImplyRead = []DPIPermission{
+	ReadPermission,
+	WritePermission,
+}
+
 // List of all valid DPI permissions, the order of permissions in this list must match
 // the above defined ordering such that iota matches the index position within the list.
 var dpiRequiredPermissions = []string{

acp/errors.go

@@ -129,6 +129,7 @@ func NewErrFailedToCheckIfDocIsRegisteredWithACP(
 func NewErrFailedToVerifyDocAccessWithACP(
 	inner error,
 	Type string,
+	permission string,
 	policyID string,
 	actorID string,
 	resourceName string,
@@ -138,6 +139,7 @@ func NewErrFailedToVerifyDocAccessWithACP(
 		errFailedToVerifyDocAccessWithACP,
 		inner,
 		errors.NewKV("Type", Type),
+		errors.NewKV("Permission", permission),
 		errors.NewKV("PolicyID", policyID),
 		errors.NewKV("ActorID", actorID),
 		errors.NewKV("ResourceName", resourceName),

acp/source_hub_client.go

@@ -12,6 +12,7 @@ package acp
 
 import (
 	"context"
+	"strconv"
 
 	protoTypes "github.com/cosmos/gogoproto/types"
 	"github.com/sourcenetwork/corelog"
@@ -336,45 +337,52 @@ func (a *sourceHubBridge) IsDocRegistered(
 
 func (a *sourceHubBridge) CheckDocAccess(
 	ctx context.Context,
-	permission DPIPermission,
+	permissions []DPIPermission,
 	actorID string,
 	policyID string,
 	resourceName string,
 	docID string,
 ) (bool, error) {
-	isValid, err := a.client.VerifyAccessRequest(
-		ctx,
-		permission,
-		actorID,
-		policyID,
-		resourceName,
-		docID,
-	)
-	if err != nil {
-		return false, NewErrFailedToVerifyDocAccessWithACP(err, "Local", policyID, actorID, resourceName, docID)
-	}
+	var hasAccess bool = false
+	var err error
 
-	if isValid {
-		log.InfoContext(
+	for _, permission := range permissions {
+		hasAccess, err = a.client.VerifyAccessRequest(
 			ctx,
-			"Document accessible",
-			corelog.Any("PolicyID", policyID),
-			corelog.Any("ActorID", actorID),
-			corelog.Any("Resource", resourceName),
-			corelog.Any("DocID", docID),
-		)
-		return true, nil
-	} else {
-		log.InfoContext(
-			ctx,
-			"Document inaccessible",
-			corelog.Any("PolicyID", policyID),
-			corelog.Any("ActorID", actorID),
-			corelog.Any("Resource", resourceName),
-			corelog.Any("DocID", docID),
+			permission,
+			actorID,
+			policyID,
+			resourceName,
+			docID,
 		)
-		return false, nil
+
+		if err != nil {
+			return false, NewErrFailedToVerifyDocAccessWithACP(
+				err,
+				"Local",
+				permission.String(),
+				policyID,
+				actorID,
+				resourceName,
+				docID,
+			)
+		}
+
+		if hasAccess {
+			break
+		}
 	}
+
+	log.InfoContext(
+		ctx,
+		"Document accessible="+strconv.FormatBool(hasAccess),
+		corelog.Any("PolicyID", policyID),
+		corelog.Any("Resource", resourceName),
+		corelog.Any("ActorID", actorID),
+		corelog.Any("DocID", docID),
+	)
+
+	return hasAccess, nil
 }
 
 func (a *sourceHubBridge) AddDocActorRelationship(

internal/db/collection.go

@@ -327,7 +327,7 @@ func (c *collection) getAllDocIDsChan(
 
 			canRead, err := c.checkAccessOfDocWithACP(
 				ctx,
-				acp.ReadPermission,
+				acp.PermissionsThatImplyRead,
 				docID.String(),
 			)
 
@@ -524,7 +524,7 @@ func (c *collection) update(
 	// Stop the update if the correct permissions aren't there.
 	canUpdate, err := c.checkAccessOfDocWithACP(
 		ctx,
-		acp.WritePermission,
+		[]acp.DPIPermission{acp.WritePermission},
 		doc.ID().String(),
 	)
 	if err != nil {
@@ -890,7 +890,7 @@ func (c *collection) exists(
 ) (exists bool, isDeleted bool, err error) {
 	canRead, err := c.checkAccessOfDocWithACP(
 		ctx,
-		acp.ReadPermission,
+		acp.PermissionsThatImplyRead,
 		primaryKey.DocID,
 	)
 	if err != nil {

internal/db/collection_acp.go

@@ -48,7 +48,7 @@ func (c *collection) registerDocWithACP(
 
 func (c *collection) checkAccessOfDocWithACP(
 	ctx context.Context,
-	dpiPermission acp.DPIPermission,
+	dpiPermissions []acp.DPIPermission,
 	docID string,
 ) (bool, error) {
 	// If acp is not available, then we have unrestricted access.
@@ -61,7 +61,7 @@ func (c *collection) checkAccessOfDocWithACP(
 		identity,
 		c.db.acp.Value(),
 		c,
-		dpiPermission,
+		dpiPermissions,
 		docID,
 	)
 }

internal/db/collection_delete.go

@@ -126,7 +126,7 @@ func (c *collection) applyDelete(
 	// Stop deletion of document if the correct permissions aren't there.
 	canDelete, err := c.checkAccessOfDocWithACP(
 		ctx,
-		acp.WritePermission,
+		[]acp.DPIPermission{acp.WritePermission},
 		primaryKey.DocID,
 	)
 

internal/db/fetcher/fetcher.go

@@ -633,7 +633,7 @@ func (df *DocumentFetcher) fetchNext(ctx context.Context) (EncodedDocument, Exec
 					df.identity,
 					df.acp.Value(),
 					df.col,
-					acp.ReadPermission,
+					acp.PermissionsThatImplyRead,
 					df.kv.Key.DocID,
 				)
 

internal/db/permission/check.go

@@ -38,7 +38,7 @@ func CheckAccessOfDocOnCollectionWithACP(
 	identity immutable.Option[acpIdentity.Identity],
 	acpSystem acp.ACP,
 	collection client.Collection,
-	permission acp.DPIPermission,
+	permissions []acp.DPIPermission,
 	docID string,
 ) (bool, error) {
 	// Even if acp exists, but there is no policy on the collection (unpermissioned collection)
@@ -77,7 +77,7 @@ func CheckAccessOfDocOnCollectionWithACP(
 	// Now actually check using the signature if this identity has access or not.
 	hasAccess, err := acpSystem.CheckDocAccess(
 		ctx,
-		permission,
+		permissions,
 		identity.Value().DID,
 		policyID,
 		resourceName,