Skip to content

Commit

Permalink
Cascading permissions for .db download, closes #1058
Browse files Browse the repository at this point in the history
  • Loading branch information
simonw committed Oct 28, 2020
1 parent c3aba4a commit 7d9fedc
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 5 deletions.
11 changes: 8 additions & 3 deletions datasette/views/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,14 @@ class DatabaseDownload(DataView):
name = "database_download"

async def view_get(self, request, database, hash, correct_hash_present, **kwargs):
await self.check_permission(request, "view-instance")
await self.check_permission(request, "view-database", database)
await self.check_permission(request, "view-database-download", database)
await self.check_permissions(
request,
[
("view-database-download", database),
("view-database", database),
"view-instance",
],
)
if database not in self.ds.databases:
raise DatasetteError("Invalid database", status=404)
db = self.ds.databases[database]
Expand Down
2 changes: 2 additions & 0 deletions tests/plugins/my_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ def permission_allowed(actor, action):
return True
elif action == "this_is_denied":
return False
elif action == "view-database-download":
return (actor and actor.get("can_download")) or None


@hookimpl
Expand Down
12 changes: 10 additions & 2 deletions tests/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ def test_view_instance(path, view_instance_client):

@pytest.fixture(scope="session")
def cascade_app_client():
with make_app_client() as client:
with make_app_client(is_immutable=True) as client:
yield client


Expand Down Expand Up @@ -439,6 +439,11 @@ def cascade_app_client():
("/fixtures", [], 403),
("/fixtures", ["instance"], 403),
("/fixtures", ["database"], 200),
# Downloading the fixtures.db file
("/fixtures.db", [], 403),
("/fixtures.db", ["instance"], 403),
("/fixtures.db", ["database"], 200),
("/fixtures.db", ["download"], 200),
],
)
def test_permissions_cascade(cascade_app_client, path, permissions, expected_status):
Expand All @@ -447,6 +452,9 @@ def test_permissions_cascade(cascade_app_client, path, permissions, expected_sta
deny = {}
previous_metadata = cascade_app_client.ds._metadata
updated_metadata = copy.deepcopy(previous_metadata)
actor = {"id": "test"}
if "download" in permissions:
actor["can_download"] = 1
try:
# Set up the different allow blocks
updated_metadata["allow"] = allow if "instance" in permissions else deny
Expand All @@ -462,7 +470,7 @@ def test_permissions_cascade(cascade_app_client, path, permissions, expected_sta
cascade_app_client.ds._metadata = updated_metadata
response = cascade_app_client.get(
path,
cookies={"ds_actor": cascade_app_client.actor_cookie({"id": "test"})},
cookies={"ds_actor": cascade_app_client.actor_cookie(actor)},
)
assert expected_status == response.status
finally:
Expand Down

0 comments on commit 7d9fedc

Please sign in to comment.