gristlabs / grist-core

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

Two-way references and forms don't work together #1280

Open fflorent opened 6 days ago

fflorent commented 6 days ago

Describe the current behavior

When we try to submit a form bound to a table which uses a two-way reference, we are answered an error. The devtools tells us it is a 403 error.

Steps to reproduce

You may download this Grist, import it and try to use the form: https://grist.incubateur.anct.gouv.fr/o/docs/api/docs/77NEs3HP29ttXX5hQuzhV8/download?template=false&nohistory=true

Otherwise, follow these steps:

  1. Create a new document
  2. Create a Towns table, with a name column and fill some data
  3. Create a Contacts table, with a name column (text) and a town column (reference to the Town table)
  4. Configure the town column and click on Add two-way reference and configure the two-way references with the Towns table
  5. Make a new form, publish it, try to submit data using this form

We this error message:

There was an error submitting your form. Please try again.

Using the devtools, we see this error in response to the request:

Blocked by table update access rules

Describe the expected behavior

We may either:

Where have you encountered this bug?

Instance information (when self-hosting only)

dsagal commented 3 days ago

I believe this issue translates to a low-level behavior of access rules, and a question whether it should be relaxed in general or if we need an exception specific to two-way references.

When you change a reference column C that's part of a two-way reference, that triggers another change to the reverse column R in the target table, where in fact multiple cells may get changed to stay in sync with the change in C. In general, forms rely on access rules to restrict an anonymous user's ability to only add a record. In particular, they don't allow anonymous users to change any cells in the target table.

There is a similar situation with formulas: a change to a cell that a user is allowed to edit may lead to calculated update to another cell that the user is not allowed to edit. We have an exception for that -- we call such actions "indirect" actions, and those bypass access rules.

So I see two ways to go about addressing this issue:

  1. Consider the updates to a reverse reference column to be "indirect" actions. There is a slightly wider class of such triggered updates (similar to formulas but not the same). To consider them all indirect, you'd wrap the two lines at https://github.com/gristlabs/grist-core/blob/3f49c60e89d9f6bc9f56d610561f8289fdd73d19/sandbox/grist/useractions.py#L526 into self.indirect_actions() (and two other similar lines elsewhere).

This would have a more general effect: even without forms, if you permit someone to edit C, but not to edit R (its reverse column), they would be allowed to change R indirectly by changing C. Currently, they would need permission to change both.

  1. We could keep the more general behavior strict (if R is a reverse column of C, then to be allowed to change either one, you need to give the user permission to edit both). Then we could address the forms issue by adding that second permission whenever we see that there is a two-way reference, and update it when two-way reference columns are added or removed.

I am leaning toward (1) as the more expected behavior generally. But any relaxing of access rules is a bit hard -- we need to think carefully of unexpected effects and unexpected use-cases that might rely on the current behavior. @paulfitz, what do you think?