Skip to content

Commit

Permalink
Renamed resource_identifier to resource, refs #817
Browse files Browse the repository at this point in the history
  • Loading branch information
simonw committed Jun 8, 2020
1 parent c9f1ec6 commit 799c5d5
Show file tree
Hide file tree
Showing 12 changed files with 40 additions and 47 deletions.
11 changes: 3 additions & 8 deletions datasette/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,16 +464,11 @@ def _show_messages(self, request):
else:
return []

async def permission_allowed(
self, actor, action, resource_identifier=None, default=False
):
async def permission_allowed(self, actor, action, resource=None, default=False):
"Check permissions using the permissions_allowed plugin hook"
result = None
for check in pm.hook.permission_allowed(
datasette=self,
actor=actor,
action=action,
resource_identifier=resource_identifier,
datasette=self, actor=actor, action=action, resource=resource,
):
if callable(check):
check = check()
Expand All @@ -490,7 +485,7 @@ async def permission_allowed(
"when": datetime.datetime.utcnow().isoformat(),
"actor": actor,
"action": action,
"resource_identifier": resource_identifier,
"resource": resource,
"used_default": used_default,
"result": result,
}
Expand Down
8 changes: 4 additions & 4 deletions datasette/default_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@


@hookimpl
def permission_allowed(datasette, actor, action, resource_identifier):
def permission_allowed(datasette, actor, action, resource):
if action == "permissions-debug":
if actor and actor.get("id") == "root":
return True
Expand All @@ -12,20 +12,20 @@ def permission_allowed(datasette, actor, action, resource_identifier):
if allow is not None:
return actor_matches_allow(actor, allow)
elif action == "view-database":
database_allow = datasette.metadata("allow", database=resource_identifier)
database_allow = datasette.metadata("allow", database=resource)
if database_allow is None:
return True
return actor_matches_allow(actor, database_allow)
elif action == "view-table":
database, table = resource_identifier
database, table = resource
tables = datasette.metadata("tables", database=database) or {}
table_allow = (tables.get(table) or {}).get("allow")
if table_allow is None:
return True
return actor_matches_allow(actor, table_allow)
elif action == "view-query":
# 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)
assert query_name in queries_metadata
if isinstance(queries_metadata[query_name], str):
Expand Down
2 changes: 1 addition & 1 deletion datasette/hookspecs.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,5 @@ def actor_from_request(datasette, request):


@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"
4 changes: 2 additions & 2 deletions datasette/templates/permissions_debug.html
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ <h2>
{% endif %}
</h2>
<p><strong>Actor:</strong> {{ check.actor|tojson }}</p>
{% if check.resource_identifier %}
<p><strong>Resource:</strong> {{ check.resource_identifier }}</p>
{% if check.resource %}
<p><strong>Resource:</strong> {{ check.resource }}</p>
{% endif %}
</div>
{% endfor %}
Expand Down
6 changes: 3 additions & 3 deletions datasette/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -876,14 +876,14 @@ def actor_matches_allow(actor, allow):
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"
visible = await datasette.permission_allowed(
actor, action, resource_identifier=resource_identifier, default=default,
actor, action, resource=resource, default=default,
)
if not visible:
return (False, False)
private = not await datasette.permission_allowed(
None, action, resource_identifier=resource_identifier, default=default,
None, action, resource=resource, default=default,
)
return visible, private
7 changes: 2 additions & 5 deletions datasette/views/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,9 @@ async def head(self, *args, **kwargs):
response.body = b""
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(
request.actor,
action,
resource_identifier=resource_identifier,
default=True,
request.actor, action, resource=resource, default=True,
)
if not ok:
raise Forbidden(action)
Expand Down
2 changes: 1 addition & 1 deletion datasette/views/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ async def data(self, request, database, hash, default_labels=False, _size=None):
"views": views,
"queries": canned_queries,
"private": not await self.ds.permission_allowed(
None, "view-database", "database", database
None, "view-database", database
),
},
{
Expand Down
12 changes: 6 additions & 6 deletions docs/authentication.rst
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ This is designed to help administrators and plugin authors understand exactly ho
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:

Expand All @@ -176,7 +176,7 @@ view-database

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

.. _permissions_view_database_download:
Expand All @@ -186,7 +186,7 @@ view-database-download

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

.. _permissions_view_table:
Expand All @@ -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

``resource_identifier`` - tuple: (string, string)
``resource`` - tuple: (string, string)
The name of the database, then the name of the table

.. _permissions_view_query:
Expand All @@ -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

``resource_identifier`` - string
``resource`` - string
The name of the canned query

.. _permissions_execute_sql:
Expand All @@ -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

``resource_identifier`` - string
``resource`` - string
The name of the database

.. _permissions_permissions_debug:
Expand Down
10 changes: 6 additions & 4 deletions docs/internals.rst
Original file line number Diff line number Diff line change
Expand Up @@ -121,22 +121,24 @@ Renders a `Jinja template <https://jinja.palletsprojects.com/en/2.11.x/>`__ usin

.. _datasette_permission_allowed:

await .permission_allowed(actor, action, resource_identifier=None, default=False)
---------------------------------------------------------------------------------
await .permission_allowed(actor, action, resource=None, default=False)
----------------------------------------------------------------------

``actor`` - dictionary
The authenticated actor. This is usually ``request.actor``.

``action`` - string
The name of the action that is being permission checked.

``resource_identifier`` - string, optional
The resource identifier, e.g. the name of the table.
``resource`` - string, optional
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.

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:

.get_database(name)
Expand Down
6 changes: 4 additions & 2 deletions docs/plugins.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,7 @@ Instead of returning a dictionary, this function can return an awaitable functio
.. _plugin_permission_allowed:

permission_allowed(datasette, actor, action, resource_identifier)
permission_allowed(datasette, actor, action, resource)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

``datasette`` - :ref:`internals_datasette`
Expand All @@ -1017,7 +1017,9 @@ permission_allowed(datasette, actor, action, resource_identifier)
``action`` - string
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.

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.
4 changes: 2 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ def before(hook_name, hook_impls, kwargs):
action = kwargs.get("action").replace("-", "_")
assert (
action in documented_permission_actions
), "Undocumented permission action: {}, resource_identifier: {}".format(
action, kwargs["resource_identifier"]
), "Undocumented permission action: {}, resource: {}".format(
action, kwargs["resource"]
)

pm.add_hookcall_monitoring(
Expand Down
15 changes: 6 additions & 9 deletions tests/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -857,21 +857,18 @@ def generate_sortable_rows(num):


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:
if isinstance(action, str):
resource_identifier = None
resource = None
else:
action, resource_identifier = action
action, resource = action
assert [
pc
for pc in datasette._permission_checks
if pc["action"] == action
and pc["resource_identifier"] == resource_identifier
], """Missing expected permission check: action={}, resource_identifier={}
if pc["action"] == action and pc["resource"] == resource
], """Missing expected permission check: action={}, resource={}
Permission checks seen: {}
""".format(
action,
resource_identifier,
json.dumps(list(datasette._permission_checks), indent=4),
action, resource, json.dumps(list(datasette._permission_checks), indent=4),
)

0 comments on commit 799c5d5

Please sign in to comment.