Extract out check_permissions() from `BaseView

[simonw]

I'm going to refactor this stuff out and document it so it can be easily used by plugins:

datasette/datasette/views/base.py

Lines 69 to 103 in 4a4164b

	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)
	async def check_permissions(self, request, permissions):
	"""permissions is a list of (action, resource) tuples or 'action' strings"""
	for permission in permissions:
	if isinstance(permission, str):
	action = permission
	resource = None
	elif isinstance(permission, (tuple, list)) and len(permission) == 2:
	action, resource = permission
	else:
	assert (
	False
	), "permission should be string or tuple of two items: {}".format(
	repr(permission)
	)
	ok = await self.ds.permission_allowed(
	request.actor,
	action,
	resource=resource,
	default=None,
	)
	if ok is not None:
	if ok:
	return
	else:
	raise Forbidden(action)

Originally posted by @simonw in #1660 (comment)

[simonw]

A slightly odd thing about these methods is that they either fail silently or they raise a Forbidden exception.

Maybe they should instead return True or False and the calling code could decide if it wants to raise the exception? That would make them more usable and a little less surprising.

[simonw]

Though at that point check_permission is such a light wrapper around self.ds.permission_allowed() that there's little point in it existing at all.

So maybe check_permisions() becomes ds.permissions_allowed().

permission_allowed() v.s. permissions_allowed() is a bit of a subtle naming difference, but I think it works.

[simonw]

The other difference though is that ds.permission_allowed(...) works against an actor, while check_permission() works against a request (though just to access request.actor).

[simonw]

One benefit of the current design of check_permissions that raises an exception is that the exception includes information on WHICH of the permission checks failed. Returning just True or False loses that information.

I could return an object which evaluates to False but also carries extra information? Bit weird, I've never seen anything like that in other Python code.

[simonw]

Idea: ds.permission_allowed() continues to just return True or False.

A new ds.ensure_permissions(...) method is added which raises a Forbidden exception if a check fails (hence the different name)`.

[simonw]

Also calling that function permissions_allowed() is confusing because there is a plugin hook with a similar name already: https://docs.datasette.io/en/stable/plugin_hooks.html#permission-allowed-datasette-actor-action-resource

[simonw]

Updated documentation: https://github.com/simonw/datasette/blob/e627510b760198ccedba9e5af47a771e847785c9/docs/internals.rst#await-ensure_permissionsactor-permissions

This method allows multiple permissions to be checked at onced. It raises a datasette.Forbidden exception if any of the checks are denied before one of them is explicitly granted.

This is useful when you need to check multiple permissions at once. For example, an actor should be able to view a table if either one of the following checks returns True or not a single one of them returns False:

That's pretty hard to understand! I'm going to open a separate issue to reconsider if this is a useful enough abstraction given how confusing it is.