Closed shahzadlone closed 2 weeks ago
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
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?
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.
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.
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).
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
.
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).
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)
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).
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.
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.
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.
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:
Then the policy above (assume
XYZ
is resultingpolicyID
) is linked in a schema that is loaded:Now if the
owner
(index1
) makes a relationship givingwrite
access to thesecond
actor (index2
) in our testing frame work like syntax:The identity
2
still can not mutate due to lack of read permission.Some existing tests that document this:
TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_OtherActorCantUpdate
TestACP_OwnerGivesUpdateWriteAccessToAnotherActorWithoutExplicitReadPerm_OtherActorCantDelete