Having view-table permission but NOT view-database should still grant access to /db/table

[simonw]

Stumbled into this while working on datasette-permissions-sql. I had granted table permissions, but the permission check wasn't even executed because the user failed the previous view-database check.

[simonw]

Relevant code:

datasette/datasette/views/table.py

Lines 267 to 272 in ce49580

	if not is_view and not table_exists:
	raise NotFound("Table not found: {}".format(table))
	await self.check_permission(request, "view-instance")
	await self.check_permission(request, "view-database", database)
	await self.check_permission(request, "view-table", (database, table))

[simonw]

May the fix here is to implement a .check_permissions() method which passes when the first permission passes?

await self.check_permissions(request, [
    ("view-table", (database, table)),
    ("view-database", database),
    "view-instance",
])

[simonw]

How would I document this? Probably in another section on https://datasette.readthedocs.io/en/latest/authentication.html#permissions

But I'd also need to add documentation to the individual views stating what permissions are checked and in what order. I could do that on this page: https://datasette.readthedocs.io/en/latest/pages.html

[simonw]

So for the following:

await self.check_permissions(request, [
    ("view-table", (database, table)),
    ("view-database", database),
    "view-instance",
])

The logic is: if the first test returns True, you get access. If it returns False you are denied. If it says None then move on to the next check in the list and repeat.

[simonw]

I think the new .check_permissions() should be a documented utility that is available to plugins.
Maybe a method on datasette?

[simonw]

I already have this method on Datasette:

async def permission_allowed(self, actor, action, resource=None, default=False):

What would be a good method name that complements that and indicates "check a list of permissions in order"? Should it even run against the request or should you have to hand it request.actor?

[simonw]

I could rename permission_allowed() to check_permission() and have a complementary check_permissions() method.

This is a breaking change but we're pre-1.0 so I think that's OK. I could even set up a temporary permission_allowed() alias which prints a deprecation warning to the console, then remove that at 1.0.

[simonw]

permission_allowed is already the name of the pugin hook. It's actually a bit confusing that it's also the name of a method on datasette..

[simonw]

Hah... but check_permissionis a method onBaseView`. Here are the various permission methods at the moment:

datasette/datasette/default_permissions.py

Lines 5 to 14 in 6c26345

	@hookimpl(tryfirst=True)
	def permission_allowed(datasette, actor, action, resource):
	async def inner():
	if action == "permissions-debug":
	if actor and actor.get("id") == "root":
	return True
	elif action == "view-instance":
	allow = datasette.metadata("allow")
	if allow is not None:
	return actor_matches_allow(actor, allow)

And on BaseView:

datasette/datasette/views/base.py

Lines 65 to 70 in a8a5f81

	async def check_permission(self, request, action, resource=None):
	ok = await self.ds.permission_allowed(
	request.actor, action, resource=resource, default=True,
	)
	if not ok:
	raise Forbidden(action)

[simonw]

I'm going to put the new check_permissions() method on BaseView as well. If I want that method to be available to plugins I can do so by turning that BaseView class into a documented API that plugins are encouraged to use themselves.

[simonw]

Tests needed for this:

• If a user has view table but NOT view database / view instance, can they view the table page?
• If a user has view canned query but NOT view database / view instance, can they view the canned query page?
• If a user has view database but NOT view instance, can they view the database page?

[simonw]

I don't think this needs any additional documentation - the new behaviour matches how the permissions are documented here: https://datasette.readthedocs.io/en/0.44/authentication.html#built-in-permissions