diff --git a/datasette/app.py b/datasette/app.py index 7b246d12..a3037e3a 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1233,7 +1233,22 @@ class Datasette: resources = [] for row in result.rows: resource = self.resource_for_action(action, parent=row[0], child=row[1]) - reason = row[2] + reason_json = row[2] + + # Parse JSON array of reasons and filter out nulls + try: + import json + + reasons_array = ( + json.loads(reason_json) if isinstance(reason_json, str) else [] + ) + reasons_filtered = [r for r in reasons_array if r is not None] + # Store as list for multiple reasons, or keep empty list + reason = reasons_filtered + except (json.JSONDecodeError, TypeError): + # Fallback for backward compatibility + reason = [reason_json] if reason_json else [] + resources.append(AllowedResource(resource=resource, reason=reason)) return resources diff --git a/datasette/templates/debug_allowed.html b/datasette/templates/debug_allowed.html index f63fdc63..c3688e26 100644 --- a/datasette/templates/debug_allowed.html +++ b/datasette/templates/debug_allowed.html @@ -177,7 +177,12 @@ function displayResults(data) { html += `${escapeHtml(item.parent || '—')}`; html += `${escapeHtml(item.child || '—')}`; if (hasDebugPermission) { - html += `${escapeHtml(item.reason || '—')}`; + // Display reason as JSON array + let reasonHtml = '—'; + if (item.reason && Array.isArray(item.reason)) { + reasonHtml = `${escapeHtml(JSON.stringify(item.reason))}`; + } + html += `${reasonHtml}`; } html += ''; } diff --git a/datasette/templates/debug_rules.html b/datasette/templates/debug_rules.html index 3873bebe..a89303d2 100644 --- a/datasette/templates/debug_rules.html +++ b/datasette/templates/debug_rules.html @@ -154,7 +154,6 @@ function displayResults(data) { html += 'Parent'; html += 'Child'; html += 'Reason'; - html += 'Source Plugin'; html += ''; html += ''; @@ -170,7 +169,6 @@ function displayResults(data) { html += `${escapeHtml(item.parent || '—')}`; html += `${escapeHtml(item.child || '—')}`; html += `${escapeHtml(item.reason || '—')}`; - html += `${escapeHtml(item.source_plugin || '—')}`; html += ''; } diff --git a/datasette/utils/actions_sql.py b/datasette/utils/actions_sql.py index 6807a728..a167bd87 100644 --- a/datasette/utils/actions_sql.py +++ b/datasette/utils/actions_sql.py @@ -261,8 +261,8 @@ async def _build_single_action_sql( " SELECT b.parent, b.child,", " MAX(CASE WHEN ar.allow = 0 THEN 1 ELSE 0 END) AS any_deny,", " MAX(CASE WHEN ar.allow = 1 THEN 1 ELSE 0 END) AS any_allow,", - " MAX(CASE WHEN ar.allow = 0 THEN ar.reason ELSE NULL END) AS deny_reason,", - " MAX(CASE WHEN ar.allow = 1 THEN ar.reason ELSE NULL END) AS allow_reason", + " json_group_array(CASE WHEN ar.allow = 0 THEN ar.reason END) AS deny_reasons,", + " json_group_array(CASE WHEN ar.allow = 1 THEN ar.reason END) AS allow_reasons", " FROM base b", " LEFT JOIN all_rules ar ON ar.parent = b.parent AND ar.child = b.child", " GROUP BY b.parent, b.child", @@ -271,8 +271,8 @@ async def _build_single_action_sql( " SELECT b.parent, b.child,", " MAX(CASE WHEN ar.allow = 0 THEN 1 ELSE 0 END) AS any_deny,", " MAX(CASE WHEN ar.allow = 1 THEN 1 ELSE 0 END) AS any_allow,", - " MAX(CASE WHEN ar.allow = 0 THEN ar.reason ELSE NULL END) AS deny_reason,", - " MAX(CASE WHEN ar.allow = 1 THEN ar.reason ELSE NULL END) AS allow_reason", + " json_group_array(CASE WHEN ar.allow = 0 THEN ar.reason END) AS deny_reasons,", + " json_group_array(CASE WHEN ar.allow = 1 THEN ar.reason END) AS allow_reasons", " FROM base b", " LEFT JOIN all_rules ar ON ar.parent = b.parent AND ar.child IS NULL", " GROUP BY b.parent, b.child", @@ -281,8 +281,8 @@ async def _build_single_action_sql( " SELECT b.parent, b.child,", " MAX(CASE WHEN ar.allow = 0 THEN 1 ELSE 0 END) AS any_deny,", " MAX(CASE WHEN ar.allow = 1 THEN 1 ELSE 0 END) AS any_allow,", - " MAX(CASE WHEN ar.allow = 0 THEN ar.reason ELSE NULL END) AS deny_reason,", - " MAX(CASE WHEN ar.allow = 1 THEN ar.reason ELSE NULL END) AS allow_reason", + " json_group_array(CASE WHEN ar.allow = 0 THEN ar.reason END) AS deny_reasons,", + " json_group_array(CASE WHEN ar.allow = 1 THEN ar.reason END) AS allow_reasons", " FROM base b", " LEFT JOIN all_rules ar ON ar.parent IS NULL AND ar.child IS NULL", " GROUP BY b.parent, b.child", @@ -363,13 +363,13 @@ async def _build_single_action_sql( " ELSE 0", " END AS is_allowed,", " CASE", - " WHEN cl.any_deny = 1 THEN cl.deny_reason", - " WHEN cl.any_allow = 1 THEN cl.allow_reason", - " WHEN pl.any_deny = 1 THEN pl.deny_reason", - " WHEN pl.any_allow = 1 THEN pl.allow_reason", - " WHEN gl.any_deny = 1 THEN gl.deny_reason", - " WHEN gl.any_allow = 1 THEN gl.allow_reason", - " ELSE 'default deny'", + " WHEN cl.any_deny = 1 THEN cl.deny_reasons", + " WHEN cl.any_allow = 1 THEN cl.allow_reasons", + " WHEN pl.any_deny = 1 THEN pl.deny_reasons", + " WHEN pl.any_allow = 1 THEN pl.allow_reasons", + " WHEN gl.any_deny = 1 THEN gl.deny_reasons", + " WHEN gl.any_allow = 1 THEN gl.allow_reasons", + " ELSE '[]'", " END AS reason", ] ) diff --git a/datasette/views/special.py b/datasette/views/special.py index 2e658d05..cf7bb1cb 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -327,7 +327,7 @@ class AllowedResourcesView(BaseView): "resource": resource_path, } - # Add reason if we have it + # Add reason if we have it (it's already a list from allowed_resources_with_reasons) if reason is not None: row["reason"] = reason diff --git a/tests/test_actions_sql.py b/tests/test_actions_sql.py index 08f36f23..63f89bf5 100644 --- a/tests/test_actions_sql.py +++ b/tests/test_actions_sql.py @@ -163,10 +163,11 @@ async def test_allowed_resources_with_reasons(test_ds): # Check we can access both resource and reason for item in allowed: assert isinstance(item.resource, TableResource) - assert isinstance(item.reason, str) + assert isinstance(item.reason, list) if item.resource.parent == "analytics": - # Should mention parent-level reason - assert "analyst access" in item.reason.lower() + # Should mention parent-level reason in at least one of the reasons + reasons_text = " ".join(item.reason).lower() + assert "analyst access" in reasons_text finally: pm.unregister(plugin, name="test_plugin")