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

[simonw]

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]

It looks like this is the code at fault:

datasette/datasette/default_permissions.py

Lines 175 to 230 in 86335dc

	async def _resolve_config_permissions_blocks(datasette, actor, action, resource):
	# Check custom permissions: blocks
	config = datasette.config or {}
	root_block = (config.get("permissions", None) or {}).get(action)
	if root_block:
	root_result = actor_matches_allow(actor, root_block)
	if root_result is not None:
	return root_result
	# Now try database-specific blocks
	if not resource:
	return None
	if isinstance(resource, str):
	database = resource
	else:
	database = resource[0]
	database_block = (
	(config.get("databases", {}).get(database, {}).get("permissions", None)) or {}
	).get(action)
	if database_block:
	database_result = actor_matches_allow(actor, database_block)
	if database_result is not None:
	return database_result
	# Finally try table/query specific blocks
	if not isinstance(resource, tuple):
	return None
	database, table_or_query = resource
	table_block = (
	(
	config.get("databases", {})
	.get(database, {})
	.get("tables", {})
	.get(table_or_query, {})
	.get("permissions", None)
	)
	or {}
	).get(action)
	if table_block:
	table_result = actor_matches_allow(actor, table_block)
	if table_result is not None:
	return table_result
	# Finally the canned queries
	query_block = (
	(
	config.get("databases", {})
	.get(database, {})
	.get("queries", {})
	.get(table_or_query, {})
	.get("permissions", None)
	)
	or {}
	).get(action)
	if query_block:
	query_result = actor_matches_allow(actor, query_block)
	if query_result is not None:
	return query_result
	return None

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:

datasette/datasette/default_permissions.py

Lines 130 to 172 in 86335dc

	@hookimpl(tryfirst=True, specname="permission_allowed")
	def permission_allowed_default(datasette, actor, action, resource):
	async def inner():
	# id=root gets some special permissions:
	if action in (
	"permissions-debug",
	"debug-menu",
	"insert-row",
	"create-table",
	"alter-table",
	"drop-table",
	"delete-row",
	"update-row",
	):
	if actor and actor.get("id") == "root":
	return True
	# Resolve view permissions in allow blocks in configuration
	if action in (
	"view-instance",
	"view-database",
	"view-table",
	"view-query",
	"execute-sql",
	):
	result = await _resolve_config_view_permissions(
	datasette, actor, action, resource
	)
	if result is not None:
	return result
	# Resolve custom permissions: blocks in configuration
	result = await _resolve_config_permissions_blocks(
	datasette, actor, action, resource
	)
	if result is not None:
	return result
	# --setting default_allow_sql
	if action == "execute-sql" and not datasette.setting("default_allow_sql"):
	return False
	return inner

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