Skip to content

Commit

Permalink
Tests and fixes for #2102
Browse files Browse the repository at this point in the history
  • Loading branch information
@simonw
simonw committed Aug 28, 2023
1 parent 6d57a8c commit 6fca0e8
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 1 deletion.
36 changes: 35 additions & 1 deletion datasette/default_permissions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,22 @@ def permission_allowed_actor_restrictions(datasette, actor, action, resource):
# Special case for view-instance: it's allowed if there are any view-database
# or view-table permissions defined
if action == "view-instance":
all_rules = _r.get("a") or []
if (
"view-database" in all_rules
or "vd" in all_rules
or "view-table" in all_rules
or "vt" in all_rules
):
return None
database_rules = _r.get("d") or {}
for rules in database_rules.values():
if "vd" in rules or "view-database" in rules:
if (
"vd" in rules
or "view-database" in rules
or "vt" in rules
or "view-table" in rules
):
return None
# Now check resources
resource_rules = _r.get("r") or {}
Expand All @@ -205,12 +218,33 @@ def permission_allowed_actor_restrictions(datasette, actor, action, resource):
# defined within that database
if action == "view-database":
database_name = resource
all_rules = _r.get("a") or []
if (
"view-database" in all_rules
or "vd" in all_rules
or "view-table" in all_rules
or "vt" in all_rules
):
return None
database_rules = (_r.get("d") or {}).get(database_name) or {}
if "vt" in database_rules or "view-table" in database_rules:
return None
resource_rules = _r.get("r") or {}
resources_in_database = resource_rules.get(database_name) or {}
for rules in resources_in_database.values():
if "vt" in rules or "view-table" in rules:
return None

if action == "view-table":
# Can view table if they have view-table in database or instance too
database_name = resource[0]
all_rules = _r.get("a") or []
if "view-table" in all_rules or "vt" in all_rules:
return None
database_rules = (_r.get("d") or {}).get(database_name) or {}
if "vt" in database_rules or "view-table" in database_rules:
return None

# Does this action have an abbreviation?
to_check = {action}
permission = datasette.permissions.get(action)
Expand Down
135 changes: 135 additions & 0 deletions tests/test_permissions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ async def perms_ds():
one = ds.add_memory_database("perms_ds_one")
two = ds.add_memory_database("perms_ds_two")
await one.execute_write("create table if not exists t1 (id integer primary key)")
await one.execute_write("insert or ignore into t1 (id) values (1)")
await one.execute_write("create view if not exists v1 as select * from t1")
await one.execute_write("create table if not exists t2 (id integer primary key)")
await two.execute_write("create table if not exists t1 (id integer primary key)")
return ds
Expand Down Expand Up @@ -1031,3 +1033,136 @@ async def test_view_table_token_can_access_table(perms_ds):
cookies = {"ds_actor": perms_ds.client.actor_cookie(actor)}
response = await perms_ds.client.get("/perms_ds_two/t1.json", cookies=cookies)
assert response.status_code == 200


@pytest.mark.asyncio
@pytest.mark.parametrize(
"restrictions,verb,path,body,expected_status",
(
# No restrictions
(None, "get", "/.json", None, 200),
(None, "get", "/perms_ds_one.json", None, 200),
(None, "get", "/perms_ds_one/t1.json", None, 200),
(None, "get", "/perms_ds_one/t1/1.json", None, 200),
(None, "get", "/perms_ds_one/v1.json", None, 200),
# Restricted to just view-instance
({"a": ["vi"]}, "get", "/.json", None, 200),
({"a": ["vi"]}, "get", "/perms_ds_one.json", None, 403),
({"a": ["vi"]}, "get", "/perms_ds_one/t1.json", None, 403),
({"a": ["vi"]}, "get", "/perms_ds_one/t1/1.json", None, 403),
({"a": ["vi"]}, "get", "/perms_ds_one/v1.json", None, 403),
# Restricted to just view-database
({"a": ["vd"]}, "get", "/.json", None, 200), # Can see instance too
({"a": ["vd"]}, "get", "/perms_ds_one.json", None, 200),
({"a": ["vd"]}, "get", "/perms_ds_one/t1.json", None, 403),
({"a": ["vd"]}, "get", "/perms_ds_one/t1/1.json", None, 403),
({"a": ["vd"]}, "get", "/perms_ds_one/v1.json", None, 403),
# Restricted to just view-table for specific database
(
{"d": {"perms_ds_one": ["vt"]}},
"get",
"/.json",
None,
200,
), # Can see instance
(
{"d": {"perms_ds_one": ["vt"]}},
"get",
"/perms_ds_one.json",
None,
200,
), # and this database
(
{"d": {"perms_ds_one": ["vt"]}},
"get",
"/perms_ds_two.json",
None,
403,
), # But not this one
(
# Can see the table
{"d": {"perms_ds_one": ["vt"]}},
"get",
"/perms_ds_one/t1.json",
None,
200,
),
(
# And the view
{"d": {"perms_ds_one": ["vt"]}},
"get",
"/perms_ds_one/v1.json",
None,
200,
),
# view-table access to a specific table
(
{"r": {"perms_ds_one": {"t1": ["vt"]}}},
"get",
"/.json",
None,
200,
),
(
{"r": {"perms_ds_one": {"t1": ["vt"]}}},
"get",
"/perms_ds_one.json",
None,
200,
),
(
{"r": {"perms_ds_one": {"t1": ["vt"]}}},
"get",
"/perms_ds_one/t1.json",
None,
200,
),
# But cannot see the other table
(
{"r": {"perms_ds_one": {"t1": ["vt"]}}},
"get",
"/perms_ds_one/t2.json",
None,
403,
),
# Or the view
(
{"r": {"perms_ds_one": {"t1": ["vt"]}}},
"get",
"/perms_ds_one/v1.json",
None,
403,
),
),
)
async def test_actor_restrictions(
perms_ds, restrictions, verb, path, body, expected_status
):
actor = {"id": "user"}
if restrictions:
actor["_r"] = restrictions
method = getattr(perms_ds.client, verb)
kwargs = {"cookies": {"ds_actor": perms_ds.client.actor_cookie(actor)}}
if body:
kwargs["json"] = body
perms_ds._permission_checks.clear()
response = await method(path, **kwargs)
assert response.status_code == expected_status, json.dumps(
{
"verb": verb,
"path": path,
"body": body,
"restrictions": restrictions,
"expected_status": expected_status,
"response_status": response.status_code,
"checks": [
{
"action": check["action"],
"resource": check["resource"],
"result": check["result"],
}
for check in perms_ds._permission_checks
],
},
indent=2,
)

0 comments on commit 6fca0e8

Please sign in to comment.