simonw / datasette

An open source multi-tool for exploring and publishing data
https://datasette.io
Apache License 2.0
9.44k stars 676 forks source link

Permissions defined in datasette.yml do not correctly obey the veto rule #2292

Open simonw opened 7 months ago

simonw commented 7 months ago

https://docs.datasette.io/en/1.0a12/authentication.html#other-permissions-in-datasette-yaml describes how you can nest permissions in datasette.yml something like this:

databases:
  docs:
    permissions:
      update-row:
        id: editor
    tables:
      news:
        update-low:
          false

One would expect the above to allow editor to update row in any table in docs except for news - since that would fit the veto rule described in https://docs.datasette.io/en/1.0a12/authentication.html#how-permissions-are-resolved

But that's not actually what happens - in the above case (or a case like it) - as far as I can tell the database-level rule is obeyed and the table-level one is ignored.

I was testing this with:

datasette content.db --secret 1  \
  -s plugins.datasette-unsafe-actor-debug.enabled 1 \
  -s databases.content.tables.news.permissions.update-row false \
  -s databases.content.permissions.update-row true \
  --reload
simonw commented 7 months ago

It looks like this is the code at fault: https://github.com/simonw/datasette/blob/86335dc722d31dbf44c6d4bbffd7c5d2d11b1290/datasette/default_permissions.py#L175-L230

Also of note: since that's called by this parent function a related issue is that root gets a True for every check, even if there should have been a veto in place. Root is excluded from the veto rule (as it applies to configured permissions, not as it applies to permissions from other plugins) thanks to this:

https://github.com/simonw/datasette/blob/86335dc722d31dbf44c6d4bbffd7c5d2d11b1290/datasette/default_permissions.py#L130-L172

I don't like that. It should either be fixed or at least be documented.