Renamed resource_identifier to resource, refs #817

This commit is contained in:
Simon Willison 2020-06-08 11:59:11 -07:00
commit 799c5d5357
12 changed files with 40 additions and 47 deletions

View file

@ -464,16 +464,11 @@ class Datasette:
else: else:
return [] return []
async def permission_allowed( async def permission_allowed(self, actor, action, resource=None, default=False):
self, actor, action, resource_identifier=None, default=False
):
"Check permissions using the permissions_allowed plugin hook" "Check permissions using the permissions_allowed plugin hook"
result = None result = None
for check in pm.hook.permission_allowed( for check in pm.hook.permission_allowed(
datasette=self, datasette=self, actor=actor, action=action, resource=resource,
actor=actor,
action=action,
resource_identifier=resource_identifier,
): ):
if callable(check): if callable(check):
check = check() check = check()
@ -490,7 +485,7 @@ class Datasette:
"when": datetime.datetime.utcnow().isoformat(), "when": datetime.datetime.utcnow().isoformat(),
"actor": actor, "actor": actor,
"action": action, "action": action,
"resource_identifier": resource_identifier, "resource": resource,
"used_default": used_default, "used_default": used_default,
"result": result, "result": result,
} }

View file

@ -3,7 +3,7 @@ from datasette.utils import actor_matches_allow
@hookimpl @hookimpl
def permission_allowed(datasette, actor, action, resource_identifier): def permission_allowed(datasette, actor, action, resource):
if action == "permissions-debug": if action == "permissions-debug":
if actor and actor.get("id") == "root": if actor and actor.get("id") == "root":
return True return True
@ -12,12 +12,12 @@ def permission_allowed(datasette, actor, action, resource_identifier):
if allow is not None: if allow is not None:
return actor_matches_allow(actor, allow) return actor_matches_allow(actor, allow)
elif action == "view-database": elif action == "view-database":
database_allow = datasette.metadata("allow", database=resource_identifier) database_allow = datasette.metadata("allow", database=resource)
if database_allow is None: if database_allow is None:
return True return True
return actor_matches_allow(actor, database_allow) return actor_matches_allow(actor, database_allow)
elif action == "view-table": elif action == "view-table":
database, table = resource_identifier database, table = resource
tables = datasette.metadata("tables", database=database) or {} tables = datasette.metadata("tables", database=database) or {}
table_allow = (tables.get(table) or {}).get("allow") table_allow = (tables.get(table) or {}).get("allow")
if table_allow is None: if table_allow is None:
@ -25,7 +25,7 @@ def permission_allowed(datasette, actor, action, resource_identifier):
return actor_matches_allow(actor, table_allow) return actor_matches_allow(actor, table_allow)
elif action == "view-query": elif action == "view-query":
# Check if this query has a "allow" block in metadata # Check if this query has a "allow" block in metadata
database, query_name = resource_identifier database, query_name = resource
queries_metadata = datasette.metadata("queries", database=database) queries_metadata = datasette.metadata("queries", database=database)
assert query_name in queries_metadata assert query_name in queries_metadata
if isinstance(queries_metadata[query_name], str): if isinstance(queries_metadata[query_name], str):

View file

@ -66,5 +66,5 @@ def actor_from_request(datasette, request):
@hookspec @hookspec
def permission_allowed(datasette, actor, action, resource_identifier): def permission_allowed(datasette, actor, action, resource):
"Check if actor is allowed to perfom this action - return True, False or None" "Check if actor is allowed to perfom this action - return True, False or None"

View file

@ -46,8 +46,8 @@
{% endif %} {% endif %}
</h2> </h2>
<p><strong>Actor:</strong> {{ check.actor|tojson }}</p> <p><strong>Actor:</strong> {{ check.actor|tojson }}</p>
{% if check.resource_identifier %} {% if check.resource %}
<p><strong>Resource:</strong> {{ check.resource_identifier }}</p> <p><strong>Resource:</strong> {{ check.resource }}</p>
{% endif %} {% endif %}
</div> </div>
{% endfor %} {% endfor %}

View file

@ -876,14 +876,14 @@ def actor_matches_allow(actor, allow):
return False return False
async def check_visibility(datasette, actor, action, resource_identifier, default=True): async def check_visibility(datasette, actor, action, resource, default=True):
"Returns (visible, private) - visible = can you see it, private = can others see it too" "Returns (visible, private) - visible = can you see it, private = can others see it too"
visible = await datasette.permission_allowed( visible = await datasette.permission_allowed(
actor, action, resource_identifier=resource_identifier, default=default, actor, action, resource=resource, default=default,
) )
if not visible: if not visible:
return (False, False) return (False, False)
private = not await datasette.permission_allowed( private = not await datasette.permission_allowed(
None, action, resource_identifier=resource_identifier, default=default, None, action, resource=resource, default=default,
) )
return visible, private return visible, private

View file

@ -64,12 +64,9 @@ class BaseView(AsgiView):
response.body = b"" response.body = b""
return response return response
async def check_permission(self, request, action, resource_identifier=None): async def check_permission(self, request, action, resource=None):
ok = await self.ds.permission_allowed( ok = await self.ds.permission_allowed(
request.actor, request.actor, action, resource=resource, default=True,
action,
resource_identifier=resource_identifier,
default=True,
) )
if not ok: if not ok:
raise Forbidden(action) raise Forbidden(action)

View file

@ -88,7 +88,7 @@ class DatabaseView(DataView):
"views": views, "views": views,
"queries": canned_queries, "queries": canned_queries,
"private": not await self.ds.permission_allowed( "private": not await self.ds.permission_allowed(
None, "view-database", "database", database None, "view-database", database
), ),
}, },
{ {

View file

@ -159,7 +159,7 @@ This is designed to help administrators and plugin authors understand exactly ho
Permissions Permissions
=========== ===========
This section lists all of the permission checks that are carried out by Datasette core, along with the ``resource_identifier`` if it was passed. This section lists all of the permission checks that are carried out by Datasette core, along with the ``resource`` if it was passed.
.. _permissions_view_instance: .. _permissions_view_instance:
@ -176,7 +176,7 @@ view-database
Actor is allowed to view a database page, e.g. https://latest.datasette.io/fixtures Actor is allowed to view a database page, e.g. https://latest.datasette.io/fixtures
``resource_identifier`` - string ``resource`` - string
The name of the database The name of the database
.. _permissions_view_database_download: .. _permissions_view_database_download:
@ -186,7 +186,7 @@ view-database-download
Actor is allowed to download a database, e.g. https://latest.datasette.io/fixtures.db Actor is allowed to download a database, e.g. https://latest.datasette.io/fixtures.db
``resource_identifier`` - string ``resource`` - string
The name of the database The name of the database
.. _permissions_view_table: .. _permissions_view_table:
@ -196,7 +196,7 @@ view-table
Actor is allowed to view a table (or view) page, e.g. https://latest.datasette.io/fixtures/complex_foreign_keys Actor is allowed to view a table (or view) page, e.g. https://latest.datasette.io/fixtures/complex_foreign_keys
``resource_identifier`` - tuple: (string, string) ``resource`` - tuple: (string, string)
The name of the database, then the name of the table The name of the database, then the name of the table
.. _permissions_view_query: .. _permissions_view_query:
@ -206,7 +206,7 @@ view-query
Actor is allowed to view a :ref:`canned query <canned_queries>` page, e.g. https://latest.datasette.io/fixtures/pragma_cache_size Actor is allowed to view a :ref:`canned query <canned_queries>` page, e.g. https://latest.datasette.io/fixtures/pragma_cache_size
``resource_identifier`` - string ``resource`` - string
The name of the canned query The name of the canned query
.. _permissions_execute_sql: .. _permissions_execute_sql:
@ -216,7 +216,7 @@ execute-sql
Actor is allowed to run arbitrary SQL queries against a specific database, e.g. https://latest.datasette.io/fixtures?sql=select+100 Actor is allowed to run arbitrary SQL queries against a specific database, e.g. https://latest.datasette.io/fixtures?sql=select+100
``resource_identifier`` - string ``resource`` - string
The name of the database The name of the database
.. _permissions_permissions_debug: .. _permissions_permissions_debug:

View file

@ -121,8 +121,8 @@ Renders a `Jinja template <https://jinja.palletsprojects.com/en/2.11.x/>`__ usin
.. _datasette_permission_allowed: .. _datasette_permission_allowed:
await .permission_allowed(actor, action, resource_identifier=None, default=False) await .permission_allowed(actor, action, resource=None, default=False)
--------------------------------------------------------------------------------- ----------------------------------------------------------------------
``actor`` - dictionary ``actor`` - dictionary
The authenticated actor. This is usually ``request.actor``. The authenticated actor. This is usually ``request.actor``.
@ -130,13 +130,15 @@ await .permission_allowed(actor, action, resource_identifier=None, default=False
``action`` - string ``action`` - string
The name of the action that is being permission checked. The name of the action that is being permission checked.
``resource_identifier`` - string, optional ``resource`` - string, optional
The resource identifier, e.g. the name of the table. The resource, e.g. the name of the table. Only some permissions apply to a resource.
Check if the given actor has permission to perform the given action on the given resource. This uses plugins that implement the :ref:`plugin_permission_allowed` plugin hook to decide if the action is allowed or not. Check if the given actor has permission to perform the given action on the given resource. This uses plugins that implement the :ref:`plugin_permission_allowed` plugin hook to decide if the action is allowed or not.
If none of the plugins express an opinion, the return value will be the ``default`` argument. This is deny, but you can pass ``default=True`` to default allow instead. If none of the plugins express an opinion, the return value will be the ``default`` argument. This is deny, but you can pass ``default=True`` to default allow instead.
See :ref:`permissions` for a full list of permissions included in Datasette core.
.. _datasette_get_database: .. _datasette_get_database:
.get_database(name) .get_database(name)

View file

@ -1005,7 +1005,7 @@ Instead of returning a dictionary, this function can return an awaitable functio
.. _plugin_permission_allowed: .. _plugin_permission_allowed:
permission_allowed(datasette, actor, action, resource_identifier) permission_allowed(datasette, actor, action, resource)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
``datasette`` - :ref:`internals_datasette` ``datasette`` - :ref:`internals_datasette`
@ -1017,7 +1017,9 @@ permission_allowed(datasette, actor, action, resource_identifier)
``action`` - string ``action`` - string
The action to be performed, e.g. ``"edit-table"``. The action to be performed, e.g. ``"edit-table"``.
``resource_identifier`` - string ``resource`` - string or None
An identifier for the individual resource, e.g. the name of the table. An identifier for the individual resource, e.g. the name of the table.
Called to check that an actor has permission to perform an action on a resource. Can return ``True`` if the action is allowed, ``False`` if the action is not allowed or ``None`` if the plugin does not have an opinion one way or the other. Called to check that an actor has permission to perform an action on a resource. Can return ``True`` if the action is allowed, ``False`` if the action is not allowed or ``None`` if the plugin does not have an opinion one way or the other.
See :ref:`permissions` for a full list of permissions included in Datasette core.

View file

@ -70,8 +70,8 @@ def check_permission_actions_are_documented():
action = kwargs.get("action").replace("-", "_") action = kwargs.get("action").replace("-", "_")
assert ( assert (
action in documented_permission_actions action in documented_permission_actions
), "Undocumented permission action: {}, resource_identifier: {}".format( ), "Undocumented permission action: {}, resource: {}".format(
action, kwargs["resource_identifier"] action, kwargs["resource"]
) )
pm.add_hookcall_monitoring( pm.add_hookcall_monitoring(

View file

@ -857,21 +857,18 @@ if __name__ == "__main__":
def assert_permissions_checked(datasette, actions): def assert_permissions_checked(datasette, actions):
# actions is a list of "action" or (action, resource_identifier) tuples # actions is a list of "action" or (action, resource) tuples
for action in actions: for action in actions:
if isinstance(action, str): if isinstance(action, str):
resource_identifier = None resource = None
else: else:
action, resource_identifier = action action, resource = action
assert [ assert [
pc pc
for pc in datasette._permission_checks for pc in datasette._permission_checks
if pc["action"] == action if pc["action"] == action and pc["resource"] == resource
and pc["resource_identifier"] == resource_identifier ], """Missing expected permission check: action={}, resource={}
], """Missing expected permission check: action={}, resource_identifier={}
Permission checks seen: {} Permission checks seen: {}
""".format( """.format(
action, action, resource, json.dumps(list(datasette._permission_checks), indent=4),
resource_identifier,
json.dumps(list(datasette._permission_checks), indent=4),
) )