An actor granted a write permission still can't write unless also given read permission

[shahzadlone]

An actor granted a write permission still can't write unless also given read permission

Example Policy where reader can strictly only read and writer can strictly only write:

name: Test Policy

description: A Policy

actor:
  name: actor

resources:
  users:
    permissions:
      read:
        expr: owner + reader

      write:
        expr: owner + writer

    relations:
      owner:
        types:
          - actor

      reader:
        types:
          - actor

      writer:
        types:
          - actor

Then the policy above (assume XYZ is resulting policyID) is linked in a schema that is loaded:

type Users @policy(id: XYZ, resource: "users") {
	name: String
	age: Int
}

Now if the owner (index 1) makes a relationship giving write access to the second actor (index 2) in our testing frame work like syntax:

testUtils.AddDocActorRelationship{
	DocID: 0,
	RequestorIdentity: 1,
	TargetIdentity: 2,
	Relation: "writer",
}

The identity 2 still can not mutate due to lack of read permission.

testUtils.UpdateDoc{
	Identity: immutable.Some(2), // This identity can still not update.
	DocID: 0,
	Doc: `
		{
			"name": "Shahzad Lone"
		}
	`,
	ExpectedError: "document not found or not authorized to access",
}

Some existing tests that document this:

• TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_OtherActorCantUpdate
• TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_OtherActorCantDelete

[AndrewSisley]

Be careful with this one, is a chance that the test harness is reading after every write and as a result some failures may be test artifacts

[shahzadlone]

Be careful with this one, is a chance that the test harness is reading after every write and as a result some failures may be test artifacts

What failures are you talking about? What does reading after writes have to do with this issue?

[AndrewSisley]

What failures are you talking about? What does reading after writes have to do with this issue?

The failures you are seeing in the tests you mentioned in the description, such as TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_OtherActorCantUpdate.

At the moment I'm pretty sure that even if the prod code was working and handling writes without read permission, the test harness will still log a failure as it will try and read the document just written to.

[shahzadlone]

The failures you are seeing in the tests you mentioned in the description, such as TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_OtherActorCantUpdate.

How are they failures? That is an expected behavior.

At the moment I'm pretty sure that even if the prod code was working and handling writes without read permission, the test
harness will still log a failure as it will try and read the document just written to.

I think you might be confused as to what this issue is, I don't understand the relevance of the situation you are talking about.

This issue marks the current behavior that will be introduced when the relationship sharing with an other actor PR is merged because unlike before (as owner always had read and write permission) now the new relation might not have a read permission but might have a write permission.

When acp was initially was being implemented the team did decide to assume read access when write access is there, so now we can add that "manipulation" on defra side.

[AndrewSisley]

The issue title An actor granted a write permission still can't write unless also given read permission, and description body suggest that the current behaviour is unwanted and that you want something to change? The tests mentioned document the unwanted behaviour, and will hopefully need to change when the behaviour is changed.

When acp was initially was being implemented the team did decide to assume read access when write access is there, so now we can add that "manipulation" on defra side.

Are you saying Defra will overwrite the policy given by the user to include the read permission when only write is given? The title does not reflect that, and I would have concerns about the impact on replicator nodes that should not have read permission (that do have write).

[shahzadlone]

The issue title An actor granted a write permission still can't write unless also given read permission, and description body suggest that the current behaviour is unwanted and that you want something to change? The tests mentioned document the unwanted behaviour, and will hopefully need to change when the behaviour is changed.

Yes

Are you saying Defra will overwrite the policy given by the user to include the read permission when only write is given? The title does not reflect that, and I would have concerns about the impact on replicator nodes that should not have read permission (that do have write).

Defra won't overwrite the policy but instead let users who have access to write also have access to read.

[AndrewSisley]

Defra won't overwrite the policy but instead let users who have access to write also have access to read.

Without informing/affecting sourcehub? Have you discussed that with @jsimnz and/or @Lodek? That could be seen as a node ignoring ACP to some extent, and would that work with doc encryption (if ACP/Orbis is managing keys)?

How would a user configure a node where they want read to be prevented, but allow writes? Such as a replicator node, or data-source node (e.g. an edge device such as a wind turbine).

[shahzadlone]

Without informing/affecting sourcehub? Have you discussed that with @jsimnz and/or @Lodek? That could be seen as a node ignoring ACP to some extent, and would that work with doc encryption (if ACP/Orbis is managing keys)?

That depends on the implementation:

1. One implementation can be to automatically make a read relationship when a write relationship is added, however this can be tedious to always maintain the correct state specially when a user removes a relationship to ensure the extra read permission is also removed (however a user might have already added an explicit read relationship manually in which case we would not want to remove that relationship).

2. Another approach can be to ignore sourcehub completely and in the check call if requesting a read to see if this identity has write permission and then just the request work.

I raised this point already in a standup with the whole team in March (https://discord.com/channels/427944769851752448/1216831384966922321/1217923732589510779) and the consensus without any pushback was to assume read when write is there. I am happy to bring this up again if there is a pushback or change in consensus, specially as we now have doc encryption and p2p source-hub acp.

How would a user configure a node where they want read to be prevented, but allow writes? Such as a replicator node, or data-source node (e.g. an edge device such as a wind turbine).

A user will have acp on a collection and they want to be able to write without reading? Keep in mind when we talk about write we are only talking about update and delete, both of those AFAIK require the ability to read the data (i.e. ensure existence, before the write op)

[AndrewSisley]

A user will have acp on a collection and they want to be able to write without reading? Keep in mind when we talk about write we are only talking about update and delete, both of those AFAIK require the ability to read the data (i.e. ensure existence, before the write op)

This depends on the CRDT types in play (LWWR does not need to read) when writing via client paths (such as GQL). Delete never requires read. Read is not required at all when updating the DAG directly via stuff like P2P, where the node receiving docs might not have read permission at all (although depending on how you think about it, that might not be classed as a write).

[shahzadlone]

This depends on the CRDT types in play (LWWR does not need to read) when writing via client paths (such as GQL). Delete never requires read.

I could have sworn that before applying delete there was a check for primary key existing.

Read is not required at all when updating the DAG directly via stuff like P2P, where the node receiving docs might not have read permission at all (although depending on how you think about it, that might not be classed as a write).

As you mentioned, I am not sure that is considered a write permission specific to Document Access Control.

[AndrewSisley]

I could have sworn that before applying delete there was a check for primary key existing

DocIDs are public I think, so when I talk about delete I assume someone has a docID and is trying to delete it without being able to read it. I can't think of anything relation-wise that would require a read, only thing comes to mind is if a secondary is not-nil, but we dont have not-nil support for relations yet.

[shahzadlone]

DocIDs are public I think, so when I talk about delete I assume someone has a docID and is trying to delete it without being able to read it. I can't think of anything relation-wise that would require a read, only thing comes to mind is if a secondary is not-nil, but we dont have not-nil support for relations yet.

Not all DocIDs are public, only DocIDs of public documents are public. Currently as ACP stands, requesting a list of docIDs in a collection will only show the requesting identity the DocIDs that it can access + the docIDs that are public.

Discussed in standup and the consensus is still to assume read permission is granted when write permission is granted. Small discussion on the implementation, likely to do the write permission check on the defradb level and not bother making a read relation on sourcehub.