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

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"
@@ -342,39 +343,86 @@ func (a *sourceHubBridge) CheckDocAccess(
 	resourceName string,
 	docID string,
 ) (bool, error) {
-	isValid, err := a.client.VerifyAccessRequest(
+	// We grant "read" access even if the identity does not explicitly have the "read" permission,
+	// as long as they have any of the permissions that imply read access.
+	if permission == ReadPermission {
+		var canRead bool = false
+		var withPermission string
+		var err error
+
+		for _, permissionThatImpliesRead := range permissionsThatImplyRead {
+			canRead, err = a.client.VerifyAccessRequest(
+				ctx,
+				permissionThatImpliesRead,
+				actorID,
+				policyID,
+				resourceName,
+				docID,
+			)
+
+			if err != nil {
+				return false, NewErrFailedToVerifyDocAccessWithACP(
+					err,
+					"Local",
+					permissionThatImpliesRead.String(),
+					policyID,
+					actorID,
+					resourceName,
+					docID,
+				)
+			}
+
+			if canRead {
+				withPermission = permissionThatImpliesRead.String()
+				break
+			}
+		}
+
+		log.InfoContext(
+			ctx,
+			"Document readable="+strconv.FormatBool(canRead),
+			corelog.Any("Permission", withPermission),
+			corelog.Any("PolicyID", policyID),
+			corelog.Any("Resource", resourceName),
+			corelog.Any("ActorID", actorID),
+			corelog.Any("DocID", docID),
+		)
+
+		return canRead, nil
+	}
+
+	hasAccess, err := a.client.VerifyAccessRequest(
 		ctx,
 		permission,
 		actorID,
 		policyID,
 		resourceName,
 		docID,
 	)
-	if err != nil {
-		return false, NewErrFailedToVerifyDocAccessWithACP(err, "Local", policyID, actorID, resourceName, docID)
-	}
 
-	if isValid {
-		log.InfoContext(
-			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),
+	if err != nil {
+		return false, NewErrFailedToVerifyDocAccessWithACP(
+			err,
+			"Local",
+			permission.String(),
+			policyID,
+			actorID,
+			resourceName,
+			docID,
 		)
-		return false, nil
 	}
+
+	log.InfoContext(
+		ctx,
+		"Document accessible="+strconv.FormatBool(hasAccess),
+		corelog.Any("Permission", permission),
+		corelog.Any("PolicyID", policyID),
+		corelog.Any("Resource", resourceName),
+		corelog.Any("ActorID", actorID),
+		corelog.Any("DocID", docID),
+	)
+
+	return hasAccess, nil
 }
 
 func (a *sourceHubBridge) AddDocActorRelationship(

tests/integration/acp/relationship/doc_actor/add/with_only_write_gql_test.go

@@ -19,12 +19,12 @@ import (
 	testUtils "github.com/sourcenetwork/defradb/tests/integration"
 )
 
-func TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_GQL_OtherActorCantUpdate(t *testing.T) {
+func TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_GQL_OtherActorCanUpdate(t *testing.T) {
 	expectedPolicyID := "0a243b1e61f990bccde41db7e81a915ffa1507c1403ae19727ce764d3b08846b"
 
 	test := testUtils.TestCase{
 
-		Description: "Test acp, owner gives write(update) access to another actor, without explicit read permission",
+		Description: "Test acp, owner gives write(update) access without explicit read permission, can still update",
 
 		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
 			// GQL mutation will return no error when wrong identity is used so test that separately.
@@ -161,7 +161,7 @@ func TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_GQ
 			testUtils.UpdateDoc{
 				CollectionID: 0,
 
-				Identity: immutable.Some(2), // This identity can still not update.
+				Identity: immutable.Some(2), // This identity can now update.
 
 				DocID: 0,
 
@@ -170,12 +170,10 @@ func TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_GQ
 						"name": "Shahzad Lone"
 					}
 				`,
-
-				SkipLocalUpdateEvent: true,
 			},
 
 			testUtils.Request{
-				Identity: immutable.Some(2), // This identity can still not read.
+				Identity: immutable.Some(2), // This identity can now also read.
 
 				Request: `
 					query {
@@ -188,7 +186,13 @@ func TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_GQ
 				`,
 
 				Results: map[string]any{
-					"Users": []map[string]any{},
+					"Users": []map[string]any{
+						{
+							"_docID": "bae-9d443d0c-52f6-568b-8f74-e8ff0825697b",
+							"name":   "Shahzad Lone", // Note: updated name
+							"age":    int64(28),
+						},
+					},
 				},
 			},
 		},

tests/integration/acp/relationship/doc_actor/add/with_only_write_test.go

@@ -19,12 +19,12 @@ import (
 	testUtils "github.com/sourcenetwork/defradb/tests/integration"
 )
 
-func TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_OtherActorCantUpdate(t *testing.T) {
+func TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_OtherActorCanUpdate(t *testing.T) {
 	expectedPolicyID := "0a243b1e61f990bccde41db7e81a915ffa1507c1403ae19727ce764d3b08846b"
 
 	test := testUtils.TestCase{
 
-		Description: "Test acp, owner gives write(update) access to another actor, without explicit read permission",
+		Description: "Test acp, owner gives write(update) access without explicit read permission, can still update",
 
 		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
 			testUtils.CollectionNamedMutationType,
@@ -161,7 +161,7 @@ func TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_Ot
 			testUtils.UpdateDoc{
 				CollectionID: 0,
 
-				Identity: immutable.Some(2), // This identity can still not update.
+				Identity: immutable.Some(2), // This identity can now update.
 
 				DocID: 0,
 
@@ -170,12 +170,10 @@ func TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_Ot
 						"name": "Shahzad Lone"
 					}
 				`,
-
-				ExpectedError: "document not found or not authorized to access",
 			},
 
 			testUtils.Request{
-				Identity: immutable.Some(2), // This identity can still not read.
+				Identity: immutable.Some(2), // This identity can now also read.
 
 				Request: `
 					query {
@@ -188,7 +186,13 @@ func TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_Ot
 				`,
 
 				Results: map[string]any{
-					"Users": []map[string]any{},
+					"Users": []map[string]any{
+						{
+							"_docID": "bae-9d443d0c-52f6-568b-8f74-e8ff0825697b",
+							"name":   "Shahzad Lone", // Note: updated name
+							"age":    int64(28),
+						},
+					},
 				},
 			},
 		},
@@ -197,12 +201,12 @@ func TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_Ot
 	testUtils.ExecuteTestCase(t, test)
 }
 
-func TestACP_OwnerGivesDeleteWriteAccessToAnotherActorWithoutExplicitReadPerm_OtherActorCantDelete(t *testing.T) {
+func TestACP_OwnerGivesDeleteWriteAccessToAnotherActorWithoutExplicitReadPerm_OtherActorCanDelete(t *testing.T) {
 	expectedPolicyID := "0a243b1e61f990bccde41db7e81a915ffa1507c1403ae19727ce764d3b08846b"
 
 	test := testUtils.TestCase{
 
-		Description: "Test acp, owner gives write(delete) access to another actor, without explicit read permission",
+		Description: "Test acp, owner gives write(delete) access without explicit read permission, can still delete",
 
 		Actions: []any{
 			testUtils.AddPolicy{
@@ -326,7 +330,7 @@ func TestACP_OwnerGivesDeleteWriteAccessToAnotherActorWithoutExplicitReadPerm_Ot
 			},
 
 			testUtils.Request{
-				Identity: immutable.Some(2), // This identity can still not read.
+				Identity: immutable.Some(2), // This identity can now read.
 
 				Request: `
 					query {
@@ -339,18 +343,40 @@ func TestACP_OwnerGivesDeleteWriteAccessToAnotherActorWithoutExplicitReadPerm_Ot
 				`,
 
 				Results: map[string]any{
-					"Users": []map[string]any{},
+					"Users": []map[string]any{
+						{
+							"_docID": "bae-9d443d0c-52f6-568b-8f74-e8ff0825697b",
+							"name":   "Shahzad",
+							"age":    int64(28),
+						},
+					},
 				},
 			},
 
 			testUtils.DeleteDoc{
 				CollectionID: 0,
 
-				Identity: immutable.Some(2), // This identity can still not delete.
+				Identity: immutable.Some(2), // This identity can now delete.
 
 				DocID: 0,
+			},
 
-				ExpectedError: "document not found or not authorized to access",
+			testUtils.Request{
+				Identity: immutable.Some(2), // Check if actually deleted.
+
+				Request: `
+					query {
+						Users {
+							_docID
+							name
+							age
+						}
+					}
+				`,
+
+				Results: map[string]any{
+					"Users": []map[string]any{},
+				},
 			},
 		},
 	}