From 8c5e6fa7161dcc1c345858ff40367732090a265e Mon Sep 17 00:00:00 2001 From: Benjamin King Date: Thu, 22 Aug 2024 23:10:59 -0500 Subject: [PATCH] For table and query permissions blocks, a boolean value (true or false) will immediately return that value, overriding any other permission checks. Fixes #2402 --- datasette/default_permissions.py | 105 +++++++++++++++++++++++-------- docs/authentication.rst | 56 +++++++++++++++++ tests/test_permissions.py | 68 ++++++++++++++++++++ 3 files changed, 202 insertions(+), 27 deletions(-) diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index 757b3a460b..002e9694a2 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -173,56 +173,107 @@ async def inner(): async def _resolve_config_permissions_blocks(datasette, actor, action, resource): - # Check custom permissions: blocks + """ + Resolve custom permissions blocks defined in the Datasette configuration. + + This function checks for permission blocks at different levels of the configuration: + root, database, table, and query. It returns the result of the first matching + permission block found, or None if no matching block is found. + + Args: + datasette (Datasette): The Datasette instance. + actor (dict): The actor (user) requesting the action. + action (str): The action being requested (e.g., "view-table", "execute-sql"). + resource (str or tuple): The resource the action is being performed on. + Can be a string (database name) or a tuple (database name, table/query name). + + Returns: + bool or None: + - True if the action is explicitly allowed + - False if the action is explicitly denied + - None if no matching permission block is found + + Note: + This function checks permission blocks in the following order: + 1. Root-level block for the action + 2. Database-specific block for the action + 3. Table-specific block for the action (if applicable) + 4. Query-specific block for the action (if applicable) + + For table and query blocks, a boolean value (True or False) will immediately + return that value, overriding any other permission checks. + """ config = datasette.config or {} root_block = (config.get("permissions", None) or {}).get(action) if root_block: root_result = actor_matches_allow(actor, root_block) if root_result is not None: return root_result - # Now try database-specific blocks + if not resource: return None + + table_or_query = None if isinstance(resource, str): database = resource + elif isinstance(resource, tuple): + database, table_or_query = resource else: - database = resource[0] + return None + database_block = ( (config.get("databases", {}).get(database, {}).get("permissions", None)) or {} ).get(action) + + table_block = None + query_block = None + if table_or_query: + table_block = ( + ( + config.get("databases", {}) + .get(database, {}) + .get("tables", {}) + .get(table_or_query, {}) + .get("permissions", None) + ) + or {} + ).get(action) + + query_block = ( + ( + config.get("databases", {}) + .get(database, {}) + .get("queries", {}) + .get(table_or_query, {}) + .get("permissions", None) + ) + or {} + ).get(action) + + # For table and query blocks, a boolean value (True or False) will immediately + # return that value, overriding any other permission checks. + # + # For example, `insert-row: true` permission at the table/query level should + # over-ride `insert-row` permission check at the database level. + # Github Issue #2402 + if isinstance(table_block, bool): + return table_block + elif isinstance(query_block, bool): + return query_block + + # First try database-specific permissions blocks if database_block: database_result = actor_matches_allow(actor, database_block) if database_result is not None: return database_result - # Finally try table/query specific blocks - if not isinstance(resource, tuple): - return None - database, table_or_query = resource - table_block = ( - ( - config.get("databases", {}) - .get(database, {}) - .get("tables", {}) - .get(table_or_query, {}) - .get("permissions", None) - ) - or {} - ).get(action) + + # Then try table/query specific permissions blocks if table_block: table_result = actor_matches_allow(actor, table_block) if table_result is not None: return table_result + # Finally the canned queries - query_block = ( - ( - config.get("databases", {}) - .get(database, {}) - .get("queries", {}) - .get(table_or_query, {}) - .get("permissions", None) - ) - or {} - ).get(action) if query_block: query_result = actor_matches_allow(actor, query_block) if query_result is not None: diff --git a/docs/authentication.rst b/docs/authentication.rst index 5452b9c37e..1dfd1d087e 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -880,6 +880,62 @@ And for ``insert-row`` against the ``reports`` table in that ``docs`` database: } .. [[[end]]] +For table and query-level permissions blocks, a boolean value (``true`` or ``false``) will immediately return that value, overriding any other permission checks. Anyone can insert a row into ``my-table``: + +.. [[[cog + config_example(cog, """ + databases: + my-db: + permissions: + insert-row: + id: root + tables: + my-table: + permissions: + insert-row: true + """) +.. ]]] + +.. tab:: datasette.yaml + + .. code-block:: yaml + + + databases: + my-db: + permissions: + insert-row: + id: root + tables: + my-table: + permissions: + insert-row: true + + +.. tab:: datasette.json + + .. code-block:: json + + { + "databases": { + "my-db": { + "permissions": { + "insert-row": { + "id": "root" + } + }, + "tables": { + "my-table": { + "permissions": { + "insert-row": true + } + } + } + } + } + } +.. [[[end]]] + The :ref:`permissions debug tool ` can be useful for helping test permissions that you have configured in this way. .. _CreateTokenView: diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 0e9cd15b0a..8902c58fc4 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -810,6 +810,74 @@ async def test_actor_restricted_permissions( resource=("perms_ds_one", "t1"), expected_result=True, ), + # insert-row: true permission at the table level should over-ride insert-row at the database level + # With no actor => True + # Github Issue #2402 + PermConfigTestCase( + config={ + "databases": { + "perms_ds_one": { + "permissions": {"insert-row": {"id": "user"}}, + "tables": {"t1": {"permissions": {"insert-row": True}}}, + } + } + }, + actor=None, + action="insert-row", + resource=("perms_ds_one", "t1"), + expected_result=True, + ), + # insert-row: true permission at the table level should over-ride insert-row at the database level + # With different actor then set in the database-level permissions => True + # Github Issue #2402 + PermConfigTestCase( + config={ + "databases": { + "perms_ds_one": { + "permissions": {"insert-row": {"id": "user"}}, + "tables": {"t1": {"permissions": {"insert-row": True}}}, + } + } + }, + actor={"id": "user2"}, + action="insert-row", + resource=("perms_ds_one", "t1"), + expected_result=True, + ), + # insert-row: false permission at the table level should over-ride insert-row at the database level #2402 + # With actor set in the database-level permissions => False + # Github Issue #2402 + PermConfigTestCase( + config={ + "databases": { + "perms_ds_one": { + "permissions": {"insert-row": {"id": "user"}}, + "tables": {"t1": {"permissions": {"insert-row": False}}}, + } + } + }, + actor={"id": "user"}, + action="insert-row", + resource=("perms_ds_one", "t1"), + expected_result=False, + ), + # insert-row: false permission at the table level should over-ride insert-row at the database level #2402 + # With no actor => False + # Github Issue #2402 + PermConfigTestCase( + config={ + "databases": { + "perms_ds_one": { + "permissions": {"insert-row": {"id": "user"}}, + "tables": {"t1": {"permissions": {"insert-row": False}}}, + } + } + }, + actor=None, + action="insert-row", + resource=("perms_ds_one", "t1"), + expected_result=False, + ), # view-query on canned query, wrong actor PermConfigTestCase( config={