mirror of
https://github.com/simonw/datasette.git
synced 2026-05-27 20:36:17 +02:00
Removed some no-longer-necessary code, simplified
view-query is back in the default allow actions now. We have other mechanisms that work for controlling visibility, and the fact that queries default to running with the permissions of the actor makes this safe.
This commit is contained in:
parent
4a1a4d7807
commit
1cd162e9da
3 changed files with 51 additions and 52 deletions
|
|
@ -21,37 +21,12 @@ DEFAULT_ALLOW_ACTIONS = frozenset(
|
|||
"view-database",
|
||||
"view-database-download",
|
||||
"view-table",
|
||||
"view-query",
|
||||
"execute-sql",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _configured_query_restriction_selects(datasette: "Datasette") -> tuple[list[str], dict]:
|
||||
selects = []
|
||||
params = {}
|
||||
for index, (database_name, db_config) in enumerate(
|
||||
((datasette.config or {}).get("databases") or {}).items()
|
||||
):
|
||||
for query_name, query_config in (db_config.get("queries") or {}).items():
|
||||
if isinstance(query_config, dict) and query_config.get("is_private"):
|
||||
continue
|
||||
parent_param = f"query_config_parent_{index}_{len(selects)}"
|
||||
child_param = f"query_config_child_{index}_{len(selects)}"
|
||||
selects.append(
|
||||
f"""
|
||||
SELECT :{parent_param} AS parent, :{child_param} AS child
|
||||
WHERE NOT EXISTS (
|
||||
SELECT 1 FROM queries
|
||||
WHERE database_name = :{parent_param}
|
||||
AND name = :{child_param}
|
||||
)
|
||||
"""
|
||||
)
|
||||
params[parent_param] = database_name
|
||||
params[child_param] = query_name
|
||||
return selects, params
|
||||
|
||||
|
||||
@hookimpl(specname="permission_resources_sql")
|
||||
async def default_allow_sql_check(
|
||||
datasette: "Datasette",
|
||||
|
|
@ -121,16 +96,6 @@ async def default_query_permissions_sql(
|
|||
|
||||
params = {"query_owner_id": actor_id}
|
||||
rule_sqls = []
|
||||
if not datasette.default_deny:
|
||||
rule_sqls.append(
|
||||
"""
|
||||
SELECT database_name AS parent, name AS child, 1 AS allow,
|
||||
'non-private query' AS reason
|
||||
FROM queries
|
||||
WHERE is_private = 0
|
||||
"""
|
||||
)
|
||||
|
||||
if actor_id is not None:
|
||||
rule_sqls.append(
|
||||
"""
|
||||
|
|
@ -141,23 +106,13 @@ async def default_query_permissions_sql(
|
|||
"""
|
||||
)
|
||||
|
||||
config_restriction_selects, config_restriction_params = (
|
||||
_configured_query_restriction_selects(datasette)
|
||||
)
|
||||
|
||||
restriction_sqls = [
|
||||
"""
|
||||
return PermissionSQL(
|
||||
sql="\nUNION ALL\n".join(rule_sqls) if rule_sqls else None,
|
||||
restriction_sql="""
|
||||
SELECT database_name AS parent, name AS child
|
||||
FROM queries
|
||||
WHERE is_private = 0
|
||||
OR owner_id = :query_owner_id
|
||||
"""
|
||||
]
|
||||
restriction_sqls.extend(config_restriction_selects)
|
||||
params.update(config_restriction_params)
|
||||
|
||||
return PermissionSQL(
|
||||
sql="\nUNION ALL\n".join(rule_sqls) if rule_sqls else None,
|
||||
restriction_sql="\nUNION ALL\n".join(restriction_sqls),
|
||||
""",
|
||||
params=params,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -937,16 +937,20 @@ async def test_permissions_in_config(
|
|||
updated_config = copy.deepcopy(previous_config)
|
||||
updated_config.update(config)
|
||||
perms_ds.config = updated_config
|
||||
await perms_ds.apply_queries_config()
|
||||
try:
|
||||
# Convert old-style resource to Resource object
|
||||
from datasette.resources import DatabaseResource, TableResource
|
||||
from datasette.resources import DatabaseResource, QueryResource, TableResource
|
||||
|
||||
resource_obj = None
|
||||
if resource:
|
||||
if isinstance(resource, str):
|
||||
resource_obj = DatabaseResource(database=resource)
|
||||
elif isinstance(resource, tuple) and len(resource) == 2:
|
||||
resource_obj = TableResource(database=resource[0], table=resource[1])
|
||||
if action == "view-query":
|
||||
resource_obj = QueryResource(database=resource[0], query=resource[1])
|
||||
else:
|
||||
resource_obj = TableResource(database=resource[0], table=resource[1])
|
||||
|
||||
result = await perms_ds.allowed(
|
||||
action=action, resource=resource_obj, actor=actor
|
||||
|
|
@ -956,6 +960,7 @@ async def test_permissions_in_config(
|
|||
assert result == expected_result
|
||||
finally:
|
||||
perms_ds.config = previous_config
|
||||
await perms_ds.apply_queries_config()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -248,6 +248,45 @@ async def test_default_deny_blocks_view_query_even_for_trusted_query():
|
|||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_view_query_default_allow_still_respects_private_restriction():
|
||||
ds = Datasette(memory=True)
|
||||
ds.add_memory_database("default_view_query_permissions", name="data")
|
||||
await ds.invoke_startup()
|
||||
await ds.add_query(
|
||||
"data",
|
||||
"private_report",
|
||||
"select 1",
|
||||
is_private=True,
|
||||
source="user",
|
||||
owner_id="alice",
|
||||
)
|
||||
await ds.add_query(
|
||||
"data",
|
||||
"shared_report",
|
||||
"select 2",
|
||||
is_private=False,
|
||||
source="user",
|
||||
owner_id="alice",
|
||||
)
|
||||
|
||||
assert await ds.allowed(
|
||||
action="view-query",
|
||||
resource=QueryResource("data", "shared_report"),
|
||||
actor=None,
|
||||
)
|
||||
assert await ds.allowed(
|
||||
action="view-query",
|
||||
resource=QueryResource("data", "private_report"),
|
||||
actor={"id": "alice"},
|
||||
)
|
||||
assert not await ds.allowed(
|
||||
action="view-query",
|
||||
resource=QueryResource("data", "private_report"),
|
||||
actor={"id": "bob"},
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_private_query_restriction_blocks_broad_view_query_permission():
|
||||
ds = Datasette(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue