diff --git a/datasette/__init__.py b/datasette/__init__.py index ea10c13d..64fb4ff7 100644 --- a/datasette/__init__.py +++ b/datasette/__init__.py @@ -1,3 +1,4 @@ +from datasette.permissions import Permission from datasette.version import __version_info__, __version__ # noqa from datasette.utils.asgi import Forbidden, NotFound, Request, Response # noqa from datasette.utils import actor_matches_allow # noqa diff --git a/datasette/app.py b/datasette/app.py index 282c0984..52b70c3e 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -194,6 +194,8 @@ DEFAULT_SETTINGS = {option.name: option.default for option in SETTINGS} FAVICON_PATH = app_root / "datasette" / "static" / "favicon.png" +DEFAULT_NOT_SET = object() + async def favicon(request, send): await asgi_send_file( @@ -264,6 +266,7 @@ class Datasette: self.inspect_data = inspect_data self.immutables = set(immutables or []) self.databases = collections.OrderedDict() + self.permissions = {} # .invoke_startup() will populate this try: self._refresh_schemas_lock = asyncio.Lock() except RuntimeError as rex: @@ -430,6 +433,24 @@ class Datasette: # This must be called for Datasette to be in a usable state if self._startup_invoked: return + # Register permissions, but watch out for duplicate name/abbr + names = {} + abbrs = {} + for hook in pm.hook.register_permissions(datasette=self): + if hook: + for p in hook: + if p.name in names and p != names[p.name]: + raise StartupError( + "Duplicate permission name: {}".format(p.name) + ) + if p.abbr and p.abbr in abbrs and p != abbrs[p.abbr]: + raise StartupError( + "Duplicate permission abbr: {}".format(p.abbr) + ) + names[p.name] = p + if p.abbr: + abbrs[p.abbr] = p + self.permissions[p.name] = p for hook in pm.hook.prepare_jinja2_environment( env=self.jinja_env, datasette=self ): @@ -668,9 +689,7 @@ class Datasette: if request: actor = request.actor # Top-level link - if await self.permission_allowed( - actor=actor, action="view-instance", default=True - ): + if await self.permission_allowed(actor=actor, action="view-instance"): crumbs.append({"href": self.urls.instance(), "label": "home"}) # Database link if database: @@ -678,7 +697,6 @@ class Datasette: actor=actor, action="view-database", resource=database, - default=True, ): crumbs.append( { @@ -693,7 +711,6 @@ class Datasette: actor=actor, action="view-table", resource=(database, table), - default=True, ): crumbs.append( { @@ -703,9 +720,14 @@ class Datasette: ) return crumbs - async def permission_allowed(self, actor, action, resource=None, default=False): + async def permission_allowed( + self, actor, action, resource=None, default=DEFAULT_NOT_SET + ): """Check permissions using the permissions_allowed plugin hook""" result = None + # Use default from registered permission, if available + if default is DEFAULT_NOT_SET and action in self.permissions: + default = self.permissions[action].default for check in pm.hook.permission_allowed( datasette=self, actor=actor, diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index ab2f6312..27e6d61b 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -1,4 +1,4 @@ -from datasette import hookimpl +from datasette import hookimpl, Permission from datasette.utils import actor_matches_allow import click import itsdangerous @@ -6,9 +6,44 @@ import json import time +@hookimpl +def register_permissions(): + return ( + # name, abbr, description, takes_database, takes_resource, default + Permission( + "view-instance", "vi", "View Datasette instance", False, False, True + ), + Permission("view-database", "vd", "View database", True, False, True), + Permission( + "view-database-download", "vdd", "Download database file", True, False, True + ), + Permission("view-table", "vt", "View table", True, True, True), + Permission("view-query", "vq", "View named query results", True, True, True), + Permission( + "execute-sql", "es", "Execute read-only SQL queries", True, False, True + ), + Permission( + "permissions-debug", + "pd", + "Access permission debug tool", + False, + False, + False, + ), + Permission("debug-menu", "dm", "View debug menu items", False, False, False), + # Write API permissions + Permission("insert-row", "ir", "Insert rows", True, True, False), + Permission("delete-row", "dr", "Delete rows", True, True, False), + Permission("update-row", "ur", "Update rows", True, True, False), + Permission("create-table", "ct", "Create tables", True, False, False), + Permission("drop-table", "dt", "Drop tables", True, True, False), + ) + + @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", @@ -20,45 +55,72 @@ def permission_allowed_default(datasette, actor, action, resource): ): 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) - elif action == "view-database": - if resource == "_internal" and (actor is None or actor.get("id") != "root"): - return False - database_allow = datasette.metadata("allow", database=resource) - if database_allow is None: - return None - return actor_matches_allow(actor, database_allow) - elif action == "view-table": - database, table = resource - tables = datasette.metadata("tables", database=database) or {} - table_allow = (tables.get(table) or {}).get("allow") - if table_allow is None: - return None - 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 - query = await datasette.get_canned_query(database, query_name, actor) - assert query is not None - allow = query.get("allow") - if allow is None: - return None - return actor_matches_allow(actor, allow) - elif action == "execute-sql": - # Use allow_sql block from database block, or from top-level - database_allow_sql = datasette.metadata("allow_sql", database=resource) - if database_allow_sql is None: - database_allow_sql = datasette.metadata("allow_sql") - if database_allow_sql is None: - return None - return actor_matches_allow(actor, database_allow_sql) + + # Resolve metadata view permissions + if action in ( + "view-instance", + "view-database", + "view-table", + "view-query", + "execute-sql", + ): + result = await _resolve_metadata_view_permissions( + datasette, actor, action, resource + ) + if result is not None: + return result + + # Check custom permissions: blocks + return await _resolve_metadata_permissions_blocks( + datasette, actor, action, resource + ) return inner +async def _resolve_metadata_permissions_blocks(datasette, actor, action, resource): + # Check custom permissions: blocks - not yet implemented + return None + + +async def _resolve_metadata_view_permissions(datasette, actor, action, resource): + if action == "view-instance": + allow = datasette.metadata("allow") + if allow is not None: + return actor_matches_allow(actor, allow) + elif action == "view-database": + if resource == "_internal" and (actor is None or actor.get("id") != "root"): + return False + database_allow = datasette.metadata("allow", database=resource) + if database_allow is None: + return None + return actor_matches_allow(actor, database_allow) + elif action == "view-table": + database, table = resource + tables = datasette.metadata("tables", database=database) or {} + table_allow = (tables.get(table) or {}).get("allow") + if table_allow is None: + return None + 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 + query = await datasette.get_canned_query(database, query_name, actor) + assert query is not None + allow = query.get("allow") + if allow is None: + return None + return actor_matches_allow(actor, allow) + elif action == "execute-sql": + # Use allow_sql block from database block, or from top-level + database_allow_sql = datasette.metadata("allow_sql", database=resource) + if database_allow_sql is None: + database_allow_sql = datasette.metadata("allow_sql") + if database_allow_sql is None: + return None + return actor_matches_allow(actor, database_allow_sql) + + @hookimpl(specname="permission_allowed") def permission_allowed_actor_restrictions(actor, action, resource): if actor is None: diff --git a/datasette/hookspecs.py b/datasette/hookspecs.py index 34e19664..bcd798d0 100644 --- a/datasette/hookspecs.py +++ b/datasette/hookspecs.py @@ -74,6 +74,11 @@ def register_facet_classes(): """Register Facet subclasses""" +@hookspec +def register_permissions(datasette): + """Register permissions: returns a list of datasette.permission.Permission named tuples""" + + @hookspec def register_routes(datasette): """Register URL routes: return a list of (regex, view_function) pairs""" diff --git a/datasette/permissions.py b/datasette/permissions.py index 91c9e774..1cd3474d 100644 --- a/datasette/permissions.py +++ b/datasette/permissions.py @@ -1,19 +1,6 @@ import collections Permission = collections.namedtuple( - "Permission", ("name", "abbr", "takes_database", "takes_table", "default") -) - -PERMISSIONS = ( - Permission("view-instance", "vi", False, False, True), - Permission("view-database", "vd", True, False, True), - Permission("view-database-download", "vdd", True, False, True), - Permission("view-table", "vt", True, True, True), - Permission("view-query", "vq", True, True, True), - Permission("insert-row", "ir", True, True, False), - Permission("delete-row", "dr", True, True, False), - Permission("drop-table", "dt", True, True, False), - Permission("execute-sql", "es", True, False, True), - Permission("permissions-debug", "pd", False, False, False), - Permission("debug-menu", "dm", False, False, False), + "Permission", + ("name", "abbr", "description", "takes_database", "takes_resource", "default"), ) diff --git a/datasette/views/database.py b/datasette/views/database.py index 8467aa5b..2872bebc 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -138,7 +138,7 @@ class DatabaseView(DataView): attached_databases = [d.name for d in await db.attached_databases()] allow_execute_sql = await self.ds.permission_allowed( - request.actor, "execute-sql", database, default=True + request.actor, "execute-sql", database ) return ( { @@ -375,7 +375,7 @@ class QueryView(DataView): columns = [] allow_execute_sql = await self.ds.permission_allowed( - request.actor, "execute-sql", database, default=True + request.actor, "execute-sql", database ) async def extra_template(): diff --git a/datasette/views/index.py b/datasette/views/index.py index 1f366a49..df411c4a 100644 --- a/datasette/views/index.py +++ b/datasette/views/index.py @@ -142,7 +142,7 @@ class IndexView(BaseView): "metadata": self.ds.metadata(), "datasette_version": __version__, "private": not await self.ds.permission_allowed( - None, "view-instance", default=True + None, "view-instance" ), }, ) diff --git a/datasette/views/special.py b/datasette/views/special.py index 1b4a9d3c..bae94ebc 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -1,8 +1,6 @@ import json -from datasette.permissions import PERMISSIONS from datasette.utils.asgi import Response, Forbidden from datasette.utils import actor_matches_allow, add_cors_headers -from datasette.permissions import PERMISSIONS from .base import BaseView import secrets import time @@ -108,7 +106,7 @@ class PermissionsDebugView(BaseView): # list() avoids error if check is performed during template render: { "permission_checks": list(reversed(self.ds._permission_checks)), - "permissions": PERMISSIONS, + "permissions": list(self.ds.permissions.values()), }, ) diff --git a/datasette/views/table.py b/datasette/views/table.py index 9e8b5254..3fd4b9aa 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -864,7 +864,7 @@ class TableView(DataView): "next_url": next_url, "private": private, "allow_execute_sql": await self.ds.permission_allowed( - request.actor, "execute-sql", database_name, default=True + request.actor, "execute-sql", database_name ), }, extra_template, diff --git a/docs/authentication.rst b/docs/authentication.rst index 5881143a..3dfd9f61 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -185,8 +185,14 @@ The ``/-/allow-debug`` tool lets you try out different ``"action"`` blocks agai .. _authentication_permissions_metadata: -Configuring permissions in metadata.json -======================================== +Access permissions in metadata +============================== + +There are two ways to configure permissions using ``metadata.json`` (or ``metadata.yaml``). + +For simple visibility permissions you can use ``"allow"`` blocks in the root, database, table and query sections. + +For other permissions you can use a ``"permissions"`` block, described :ref:`in the next section `. You can limit who is allowed to view different parts of your Datasette instance using ``"allow"`` keys in your :ref:`metadata` configuration. @@ -201,8 +207,8 @@ If a user cannot access a specific database, they will not be able to access tab .. _authentication_permissions_instance: -Controlling access to an instance ---------------------------------- +Access to an instance +--------------------- Here's how to restrict access to your entire Datasette instance to just the ``"id": "root"`` user: @@ -228,8 +234,8 @@ One reason to do this is if you are using a Datasette plugin - such as `datasett .. _authentication_permissions_database: -Controlling access to specific databases ----------------------------------------- +Access to specific databases +---------------------------- To limit access to a specific ``private.db`` database to just authenticated users, use the ``"allow"`` block like this: @@ -247,8 +253,8 @@ To limit access to a specific ``private.db`` database to just authenticated user .. _authentication_permissions_table: -Controlling access to specific tables and views ------------------------------------------------ +Access to specific tables and views +----------------------------------- To limit access to the ``users`` table in your ``bakery.db`` database: @@ -277,8 +283,8 @@ This works for SQL views as well - you can list their names in the ``"tables"`` .. _authentication_permissions_query: -Controlling access to specific canned queries ---------------------------------------------- +Access to specific canned queries +--------------------------------- :ref:`canned_queries` allow you to configure named SQL queries in your ``metadata.json`` that can be executed by users. These queries can be set up to both read and write to the database, so controlling who can execute them can be important. @@ -333,6 +339,63 @@ To limit this ability for just one specific database, use this: } } +.. _authentication_permissions_other: + +Other permissions in metadata +============================= + +For all other permissions, you can use one or more ``"permissions"`` blocks in your metadata. + +To grant access to the :ref:`permissions debug tool ` to all signed in users you can grant ``permissions-debug`` to any actor with an ``id`` matching the wildcard ``*`` by adding this a the root of your metadata: + +.. code-block:: json + + { + "permissions": { + "debug-menu": { + "id": "*" + } + } + } + +To grant ``create-table`` to the user with ``id`` of ``editor`` for the ``docs`` database: + +.. code-block:: json + + { + "databases": { + "docs": { + "permissions": { + "create-table": { + "id": "editor" + } + } + } + } + } + +And for ``insert-row`` against the ``reports`` table in that ``docs`` database: + +.. code-block:: json + + { + "databases": { + "docs": { + "tables": { + "reports": { + "permissions": { + "insert-row": { + "id": "editor" + } + } + } + } + } + } + } + +The :ref:`PermissionsDebugView` can be useful for helping test permissions that you have configured in this way. + .. _CreateTokenView: API Tokens @@ -423,10 +486,12 @@ The currently authenticated actor is made available to plugins as ``request.acto The permissions debug tool ========================== -The debug tool at ``/-/permissions`` is only available to the :ref:`authenticated root user ` (or any actor granted the ``permissions-debug`` action according to a plugin). +The debug tool at ``/-/permissions`` is only available to the :ref:`authenticated root user ` (or any actor granted the ``permissions-debug`` action). It shows the thirty most recent permission checks that have been carried out by the Datasette instance. +It also provides an interface for running hypothetical permission checks against a hypothetical actor. This is a useful way of confirming that your configured permissions work in the way you expect. + This is designed to help administrators and plugin authors understand exactly how permission checks are being carried out, in order to effectively configure Datasette's permission system. .. _authentication_ds_actor: diff --git a/docs/internals.rst b/docs/internals.rst index 8b5a2b6e..fe495264 100644 --- a/docs/internals.rst +++ b/docs/internals.rst @@ -273,6 +273,15 @@ The dictionary keys are the name of the database that is used in the URL - e.g. All databases are listed, irrespective of user permissions. This means that the ``_internal`` database will always be listed here. +.. _datasette_permissions: + +.permissions +------------ + +Property exposing a dictionary of permissions that have been registered using the :ref:`plugin_register_permissions` plugin hook. + +The dictionary keys are the permission names - e.g. ``view-instance`` - and the values are ``Permission()`` named tuples describing the permission. Here is a :ref:`description of that tuple `. + .. _datasette_plugin_config: .plugin_config(plugin_name, database=None, table=None) @@ -315,8 +324,8 @@ Renders a `Jinja template `__ usin .. _datasette_permission_allowed: -await .permission_allowed(actor, action, resource=None, default=False) ----------------------------------------------------------------------- +await .permission_allowed(actor, action, resource=None, default=...) +-------------------------------------------------------------------- ``actor`` - dictionary The authenticated actor. This is usually ``request.actor``. @@ -327,8 +336,10 @@ await .permission_allowed(actor, action, resource=None, default=False) ``resource`` - string or tuple, optional The resource, e.g. the name of the database, or a tuple of two strings containing the name of the database and the name of the table. Only some permissions apply to a resource. -``default`` - optional, True or False - Should this permission check be default allow or default deny. +``default`` - optional: True, False or None + What value should be returned by default if nothing provides an opinion on this permission check. + Set to ``True`` for default allow or ``False`` for default deny. + If not specified the ``default`` from the ``Permission()`` tuple that was registered using :ref:`plugin_register_permissions` will be used. Check if the given actor has :ref:`permission ` to perform the given action on the given resource. diff --git a/docs/plugin_hooks.rst b/docs/plugin_hooks.rst index 956887db..f41ca876 100644 --- a/docs/plugin_hooks.rst +++ b/docs/plugin_hooks.rst @@ -760,6 +760,53 @@ The plugin hook can then be used to register the new facet class like this: def register_facet_classes(): return [SpecialFacet] +.. _plugin_register_permissions: + +register_permissions(datasette) +-------------------------------- + +If your plugin needs to register additional permissions unique to that plugin - ``upload-csvs`` for example - you can return a list of those permissions from this hook. + +.. code-block:: python + + from datasette import hookimpl, Permission + + + @hookimpl + def register_permissions(datasette): + return [ + Permission( + name="upload-csvs", + abbr=None, + description="Upload CSV files", + takes_database=True, + takes_resource=False, + default=False, + ) + ] + +The fields of the ``Permission`` named tuple are as follows: + +``name`` + The name of the permission, e.g. ``upload-csvs``. This should be unique across all plugins that the user might have installed, so choose carefully. + +``abbr`` + An abbreviation of the permission, e.g. ``uc``. This is optional - you can set it to ``None`` if you do not want to pick an abbreviation. Since this needs to be unique across all installed plugins it's best not to specify an abbreviation at all. If an abbreviation is provided it will be used when creating restricted signed API tokens. + +``description`` + A human-readable description of what the permission lets you do. Should make sense as the second part of a sentence that starts "A user with this permission can ...". + +``takes_database`` + ``True`` if this permission can be granted on a per-database basis, ``False`` if it is only valid at the overall Datasette instance level. + +``takes_resource`` + ``True`` if this permission can be granted on a per-resource basis. A resource is a database table, SQL view or :ref:`canned query `. + +``default`` + The default value for this permission if it is not explicitly granted to a user. ``True`` means the permission is granted by default, ``False`` means it is not. + + This should only be ``True`` if you want anonymous users to be able to take this action. + .. _plugin_asgi_wrapper: asgi_wrapper(datasette) diff --git a/docs/plugins.rst b/docs/plugins.rst index 71eaa935..fa97628c 100644 --- a/docs/plugins.rst +++ b/docs/plugins.rst @@ -154,6 +154,7 @@ If you run ``datasette plugins --all`` it will include default plugins that ship "actor_from_request", "permission_allowed", "register_commands", + "register_permissions", "skip_csrf" ] }, diff --git a/tests/conftest.py b/tests/conftest.py index f4638a14..cd735e12 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -90,6 +90,13 @@ def check_permission_actions_are_documented(): def before(hook_name, hook_impls, kwargs): if hook_name == "permission_allowed": + datasette = kwargs["datasette"] + assert kwargs["action"] in datasette.permissions, ( + "'{}' has not been registered with register_permissions()".format( + kwargs["action"] + ) + + " (or maybe a test forgot to do await ds.invoke_startup())" + ) action = kwargs.get("action").replace("-", "_") assert ( action in documented_permission_actions diff --git a/tests/fixtures.py b/tests/fixtures.py index ba5f065e..a6700239 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -48,6 +48,7 @@ EXPECTED_PLUGINS = [ "prepare_jinja2_environment", "register_facet_classes", "register_magic_parameters", + "register_permissions", "register_routes", "render_cell", "skip_csrf", diff --git a/tests/plugins/my_plugin.py b/tests/plugins/my_plugin.py index 91d2888d..dafcd1cb 100644 --- a/tests/plugins/my_plugin.py +++ b/tests/plugins/my_plugin.py @@ -1,5 +1,5 @@ import asyncio -from datasette import hookimpl +from datasette import hookimpl, Permission from datasette.facets import Facet from datasette import tracer from datasette.utils import path_with_added_args @@ -406,3 +406,31 @@ def database_actions(datasette, database, actor, request): @hookimpl def skip_csrf(scope): return scope["path"] == "/skip-csrf" + + +@hookimpl +def register_permissions(datasette): + extras = datasette.plugin_config("datasette-register-permissions") or {} + permissions = [ + Permission( + name="new-permission", + abbr="np", + description="New permission", + takes_database=True, + takes_resource=False, + default=False, + ) + ] + if extras: + permissions.extend( + Permission( + name=p["name"], + abbr=p["abbr"], + description=p["description"], + takes_database=p["takes_database"], + takes_resource=p["takes_resource"], + default=p["default"], + ) + for p in extras["permissions"] + ) + return permissions diff --git a/tests/test_filters.py b/tests/test_filters.py index 2ff57489..7e3692f8 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -126,6 +126,7 @@ async def test_through_filters_from_request(app_client): @pytest.mark.asyncio async def test_where_filters_from_request(app_client): + await app_client.ds.invoke_startup() request = Request.fake("/?_where=pk+>+3") filter_args = await ( where_filters( diff --git a/tests/test_internals_datasette.py b/tests/test_internals_datasette.py index a61aac2d..97fdc35d 100644 --- a/tests/test_internals_datasette.py +++ b/tests/test_internals_datasette.py @@ -116,6 +116,7 @@ async def test_datasette_ensure_permissions_check_visibility( actor, metadata, permissions, should_allow, expected_private ): ds = Datasette([], memory=True, metadata=metadata) + await ds.invoke_startup() if not should_allow: with pytest.raises(Forbidden): await ds.ensure_permissions(actor, permissions) diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 4eb18cee..50237ea0 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -1,3 +1,4 @@ +import collections from datasette.app import Datasette from .fixtures import app_client, assert_permissions_checked, make_app_client from bs4 import BeautifulSoup as Soup @@ -640,3 +641,49 @@ async def test_actor_restricted_permissions( "result": expected_result, } assert response.json() == expected + + +PermMetadataTestCase = collections.namedtuple( + "PermMetadataTestCase", + "metadata,actor,action,resource,default,expected_result", +) + + +@pytest.mark.asyncio +@pytest.mark.xfail(reason="Not implemented yet") +@pytest.mark.parametrize( + "metadata,actor,action,resource,default,expected_result", + ( + # Simple view-instance default=True example + PermMetadataTestCase( + metadata={}, + actor=None, + action="view-instance", + resource=None, + default=True, + expected_result=True, + ), + # debug-menu on root + PermMetadataTestCase( + metadata={"permissions": {"debug-menu": {"id": "user"}}}, + actor={"id": "user"}, + action="debug-menu", + resource=None, + default=False, + expected_result=True, + ), + ), +) +async def test_permissions_in_metadata( + perms_ds, metadata, actor, action, resource, default, expected_result +): + previous_metadata = perms_ds.metadata() + updated_metadata = copy.deepcopy(previous_metadata) + updated_metadata.update(metadata) + try: + result = await perms_ds.permission_allowed( + actor, action, resource, default=default + ) + assert result == expected_result + finally: + perms_ds._metadata_local = previous_metadata diff --git a/tests/test_plugins.py b/tests/test_plugins.py index de3fde8e..8312b1f3 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -4,15 +4,16 @@ from .fixtures import ( make_app_client, TABLES, TEMP_PLUGIN_SECRET_FILE, + PLUGINS_DIR, TestClient as _TestClient, ) # noqa from click.testing import CliRunner from datasette.app import Datasette -from datasette import cli, hookimpl +from datasette import cli, hookimpl, Permission from datasette.filters import FilterArguments from datasette.plugins import get_plugins, DEFAULT_PLUGINS, pm from datasette.utils.sqlite import sqlite3 -from datasette.utils import CustomRow +from datasette.utils import CustomRow, StartupError from jinja2.environment import Template import base64 import importlib @@ -635,14 +636,32 @@ def test_existing_scope_actor_respected(app_client): ("this_is_denied", False), ("this_is_allowed_async", True), ("this_is_denied_async", False), - ("no_match", None), ], ) -async def test_hook_permission_allowed(app_client, action, expected): - actual = await app_client.ds.permission_allowed( - {"id": "actor"}, action, default=None - ) - assert expected == actual +async def test_hook_permission_allowed(action, expected): + class TestPlugin: + __name__ = "TestPlugin" + + @hookimpl + def register_permissions(self): + return [ + Permission(name, None, None, False, False, False) + for name in ( + "this_is_allowed", + "this_is_denied", + "this_is_allowed_async", + "this_is_denied_async", + ) + ] + + pm.register(TestPlugin(), name="undo_register_extras") + try: + ds = Datasette(plugins_dir=PLUGINS_DIR) + await ds.invoke_startup() + actual = await ds.permission_allowed({"id": "actor"}, action) + assert expected == actual + finally: + pm.unregister(name="undo_register_extras") def test_actor_json(app_client): @@ -1023,3 +1042,125 @@ def test_hook_filters_from_request(app_client): json_response = app_client.get("/fixtures/facetable.json?_nothing=1") assert json_response.json["rows"] == [] pm.unregister(name="ReturnNothingPlugin") + + +@pytest.mark.asyncio +@pytest.mark.parametrize("extra_metadata", (False, True)) +async def test_hook_register_permissions(extra_metadata): + ds = Datasette( + metadata={ + "plugins": { + "datasette-register-permissions": { + "permissions": [ + { + "name": "extra-from-metadata", + "abbr": "efm", + "description": "Extra from metadata", + "takes_database": False, + "takes_resource": False, + "default": True, + } + ] + } + } + } + if extra_metadata + else None, + plugins_dir=PLUGINS_DIR, + ) + await ds.invoke_startup() + assert ds.permissions["new-permission"] == Permission( + name="new-permission", + abbr="np", + description="New permission", + takes_database=True, + takes_resource=False, + default=False, + ) + if extra_metadata: + assert ds.permissions["extra-from-metadata"] == Permission( + name="extra-from-metadata", + abbr="efm", + description="Extra from metadata", + takes_database=False, + takes_resource=False, + default=True, + ) + else: + assert "extra-from-metadata" not in ds.permissions + + +@pytest.mark.asyncio +@pytest.mark.parametrize("duplicate", ("name", "abbr")) +async def test_hook_register_permissions_no_duplicates(duplicate): + name1, name2 = "name1", "name2" + abbr1, abbr2 = "abbr1", "abbr2" + if duplicate == "name": + name2 = "name1" + if duplicate == "abbr": + abbr2 = "abbr1" + ds = Datasette( + metadata={ + "plugins": { + "datasette-register-permissions": { + "permissions": [ + { + "name": name1, + "abbr": abbr1, + "description": None, + "takes_database": False, + "takes_resource": False, + "default": True, + }, + { + "name": name2, + "abbr": abbr2, + "description": None, + "takes_database": False, + "takes_resource": False, + "default": True, + }, + ] + } + } + }, + plugins_dir=PLUGINS_DIR, + ) + # This should error: + with pytest.raises(StartupError) as ex: + await ds.invoke_startup() + assert "Duplicate permission {}".format(duplicate) in str(ex.value) + + +@pytest.mark.asyncio +async def test_hook_register_permissions_allows_identical_duplicates(): + ds = Datasette( + metadata={ + "plugins": { + "datasette-register-permissions": { + "permissions": [ + { + "name": "name1", + "abbr": "abbr1", + "description": None, + "takes_database": False, + "takes_resource": False, + "default": True, + }, + { + "name": "name1", + "abbr": "abbr1", + "description": None, + "takes_database": False, + "takes_resource": False, + "default": True, + }, + ] + } + } + }, + plugins_dir=PLUGINS_DIR, + ) + await ds.invoke_startup() + # Check that ds.permissions has only one of each + assert len([p for p in ds.permissions.values() if p.abbr == "abbr1"]) == 1