gristlabs / grist-core

Grist is the evolution of spreadsheets.
https://www.getgrist.com/
Apache License 2.0
7.11k stars 318 forks source link

Columns protected by access rules can be deleted by any user. #1272

Open S7venLights opened 1 day ago

S7venLights commented 1 day ago

Current behavior

I have a crowd sourced table and one of the columns needs to be read only, which I managed to set-up fine. The issue is, while public contributors can't delete the contents of the cells in that column, they can still delete the whole column and there doesn't appear to be a way to prevent that.

I need the protected column to be safe from deletion as it contains moderator verified links. Additionally, if a public user deletes the column, they can't undo that action. Here is the spreadsheet I'm working in.

Steps to reproduce

  1. Setup access rules for a column as follows: Screenshot_20241016_134325

  2. Using the following public access link: Screenshot_20241016_142644 Open the link in a separate browser and click the following button to delete a protected column Screenshot_20241016_142857

  3. after doing so, the document undo button will not work

Describe the expected behavior

I expected that denying edit permission would naturally also deny deletion.

However this option can be added. In the access rules there is only a RU (Read | Update) option for a column. If there was also a Delete permission that would be intuitive.

Also I have an idea to make column access rules UI&UX much easier. For logged in Owners, there could be toggles for these permissions right in the column settings sidebar: Screenshot_20241016_143832

Where have you encountered this bug?

dsagal commented 1 day ago

A first step recommended when locking down parts of a document is to deny most users the permission to change structure: https://support.getgrist.com/access-rules/#lock-down-structure. This is not the default for compatibility with documents without access rules. Locking down structure will prevent actions like deleting columns.

That said, I agree that denying "Update" permission on a table or column should probably deny the ability to delete that table/column as well. At least I don't see any argument against that.

As for a simplified toggle, the issue that always comes up with simplified UI ideas is that they don't include the ability to specify who should have that permission. For example, important documents often match users with some in-document data using user attributes, and then control permissions using some data associated with the document (organization role, department, etc)

S7venLights commented 9 hours ago

Locking down structure will prevent actions like deleting columns.

I haven't allowed structure editing (See screenshot below)

The Problem

The problem with this structure permission deny approach (For my use case) is that I do want people to be able to create and delete tables and columns, to work with the structure... But then I want to lock down columns or tables that I don't want to be delete-able anymore.

My Goal

I want the crowd (people with public access to the grist doc link, that don't need to sign-up) to be able to make new tables add and remove columns they make by mistake, etc. But then owners/editors/moderators (people with accounts) to be able to restrict columns or whole tables form being deleted once they're ready, while still allowing public users to update the content of cells.

Deny update should deny delete by default

Naturally yes, you would expect that denying update permission would automatically deny delete. Can't see any reason for it not to. Agree there.

Possible Solutions

More granular permissions will probably help cover a wider set of use cases. For example, I imagine if one had separate permissions for R, U & D per column then I could deny public users from deleting columns or tables, but they would still be able to update data in the cells of that column or create new columns.

Let me know if there is actually a way to do this. Because I did some more testing and I don't see a way. This is what I found:

More Delete issues

I created a test table using the owner account then locked it with the following permissions: Screenshot_20241017_115411 But then from a public user browser I was still able to remove the entire table and delete the raw data... Surely that isn't right? I have re-created the 'Test' table with some locked columns and a Test2 table with permissions where all columns are set to read only. Then you can open it and test for for yourself.

I also found, A public user with read only permission is also able to edit column descriptions and this probably shouldn't be allowed.

Structure permission Questions

Also as you can see there "Allow editors to edit structure" is not enabled and it hasn't been enabled before. But 'user.Access in [EDITOR, OWNER]' is allowed to edit structure and I can't change that permission. :thinking: is that right?

I'm happy to set a time/meeting to figure this all out if need be.

As for a simplified toggle... specify who should have that permission

I imagine being logged in as the owner or an editor with permissions, them having that permission toggle be visible in column settings only to them. Then for moderators to be able to specify users access in that UI, you could have a searchable drop down list of existing users, user groups whom you would grant a set permissions to. Also have 'Everyone' as an option.drop down list.

S7venLights commented 8 hours ago

I've run into another issue now, where sometimes when I save access rules, I get logged out and also between sessions even though cookies/browser data aren't set to delete on browser exit. This is on Brave browser with adblocking and a VPN, but fingerprint blocking turned off. Should I open a separate issue?