Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify activities for relevant storage operations #183

Open
elf-pavlik opened this issue Jul 21, 2023 · 10 comments
Open

Specify activities for relevant storage operations #183

elf-pavlik opened this issue Jul 21, 2023 · 10 comments

Comments

@elf-pavlik
Copy link
Member

elf-pavlik commented Jul 21, 2023

Based on the table in solid/web-access-control-spec#85 (comment)

Request Operation Access Modes Level Activity
GET /C/R Read acl:Read Resource * as:Read
HEAD /C/R Read acl:Read Resource * as:Read
OPTIONS /C/R Read acl:Read Resource * as:Read
POST /C/ Create + Update acl:Append [1] Resource, Content ? as:Add
PUT /C/R + If-None-Match Create, Read acl:Write, acl:Read [2] Resource ? as:Create
PATCH /C/R + If-None-Match + INSERT Create, Read acl:Write, acl:Read [2] Resource, Content ? as:Update
POST /C/R Update acl:Append (or acl:Write) Content ? as:Add
PUT /C/R Create?, Update acl:Write [2][3] Resource ? as:Create
PATCH /C/R + INSERT Create?, Update acl:Write [2][3] Resource?, Content as:Update
PATCH /C/R + DELETE + INSERT Create?, Read, Update acl:Write, acl:Read [2][3] Resource?, Content [4] as:Update
PATCH /C/R + DELETE Update, Read acl:Write, acl:Read Content as:Update
DELETE /C/ Delete acl:Write, acl:Read Resource as:Remove
DELETE /C/R Delete acl:Write [5] Resource as:Delete

Also keeping in mind as a reference: https://communitysolidserver.github.io/CommunitySolidServer/6.x/usage/notifications/#notification-types

  • All Read can be ignored
  • Update results in as:Update (except updates to ldp:contains addressed by Create and Remove)
  • Create only as:Add on the parent, since the newly created resource couldn't have been a topic of an existing channel
  • Delete
    • as:Delete when the deleted resource was the topic
    • as:Remove when the parent container was the topic

@joachimvh in what case CSS sends as:Create notifications?

@CxRes
Copy link
Member

CxRes commented Jul 21, 2023

As discussed in the meeting between me and @csarven, we would add an implementation guidance for as:Read, stating that origin servers need to ensure that they aggregate read (GET) events from an intermediary and broadcast them back to all intermediaries. Mechanisms for doing so would be outside the scope of the specification.

We need this because GET requests can be handled by HTTP intermediaries, whereas all write requests are always sent back to the origin server.

@CxRes
Copy link
Member

CxRes commented Jul 21, 2023

Just be careful that as:Create and as:Add are distinct. @elf-pavlik Can you please add a fifth column, say "Activity", to the table that includes AS vocab mapping?

@elf-pavlik
Copy link
Member Author

If you want to support as:Read I think there would need to be a way to opt-in for it since many (most?) use cases don't want to get notifications as:Read. There is also the question of who would be authorized to receive as:Read notifications. This is a privacy issue and probably only the storage owner would have access to such notifications.

@CxRes
Copy link
Member

CxRes commented Jul 21, 2023

The context of this was audit functionality, so you are right this would be available to storage owner (or auditors who are given access to notifications but not contents/diffs). It is just that we do not want to shut off that possibility, even if we do not specify it right now. I think read will be allowed using a filter (just that it is "unfiltering").

@joachimvh
Copy link
Contributor

@joachimvh in what case CSS sends as:Create notifications?

Create notifications get sent when a POST/PUT/PATCH request writes data to a resource that didn't exist yet. So the same situations where the required permissions would be different, as you require acl:Append permissions on a container when trying to create a new resource.

Besides that, Update gets sent when an existing resource is modified through PUT/PATCH, so not when it gets created initially, and Delete when it gets removed.

In the case of Create and Delete, Add and Remove events get sent out for the parent container.

@CxRes
Copy link
Member

CxRes commented Jul 24, 2023

@joachimvh Can you point us to where this code resides in CSS?

@joachimvh
Copy link
Contributor

@joachimvh Can you point us to where this code resides in CSS?

The DataAccessorBasedStore, which is the class that handles the "core" of data interactions as described in the Solid Protocol specification, is also the class that causes the initial trigger to happen for specific notifications, after which the MonitoringStore emits events that follow the path described here.

For example, this line causes the Remove event when deleting a resource from a container, and this line adds the metadata for the Delete event.

@elf-pavlik
Copy link
Member Author

Create notifications get sent when a POST/PUT/PATCH request writes data to a resource that didn't exist yet. So the same situations where the required permissions would be different, as you require acl:Append permissions on a container when trying to create a new resource.

@joachimvh does CSS allow creating notification channels for topic which doesn't exist yet? #184

If it does not I think an Add notification would be sufficient if the parent container is the topic of the notification channel. Currently Solid doesn't support any other of adding a resource than creating a new one. Even if it eventually did, I don't know if a notification channel with a container as the topic has to communicate the (again currently non-existent) difference between already adding existing and newly created resource.
The only situation where the Create notification would be required, seems to me if the notification channel is created (if we allow/support it) with at topic which doesn't exist yet.

@joachimvh
Copy link
Contributor

@joachimvh does CSS allow creating notification channels for topic which doesn't exist yet?

CSS does no existence checks on a resource when trying to subscribe, so yes. The only check is if you have read permissions on the resource, so if the acl:default is set correctly on the parent container this is possible. Another possible scenario is when you subscribe to an existing resource, it gets deleted, and then it gets created again. The subscription will persist through those states so that last action will cause a Create event to be sent out.

The Add notifications are sent to clients who subscribed to the container, the Create notifications to the client that subscribed to the resource itself. So which one you get depends on what you subscribed to.

Note that CSS only sends Add when a resource is added to the container, not when one of its children is updated.

@elf-pavlik
Copy link
Member Author

Thank you @joachimvh

I see at least two other issues relevant to what we discussed here:

You are very welcomed to provide feedback there as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants