Skip to content

Commit

Permalink
Merge pull request from GHSA-7ch3-7pp7-7cpq
Browse files Browse the repository at this point in the history
* API explorer requires view-instance permission

* Check database/table permissions on /-/api page

* Release notes for 1.0a4

Refs #2119, #2133, #2138, #2140

Refs GHSA-7ch3-7pp7-7cpq
  • Loading branch information
simonw authored Aug 22, 2023
1 parent 943df09 commit 01e0558
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 7 deletions.
2 changes: 1 addition & 1 deletion datasette/templates/api_explorer.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

{% block content %}

<h1>API Explorer</h1>
<h1>API Explorer{% if private %} 🔒{% endif %}</h1>

<p>Use this tool to try out the
{% if datasette_version %}
Expand Down
2 changes: 1 addition & 1 deletion datasette/version.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
__version__ = "1.0a3"
__version__ = "1.0a4"
__version_info__ = tuple(__version__.split("."))
19 changes: 14 additions & 5 deletions datasette/views/special.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,7 @@ async def example_links(self, request):
if name == "_internal":
continue
database_visible, _ = await self.ds.check_visibility(
request.actor,
"view-database",
name,
request.actor, permissions=[("view-database", name), "view-instance"]
)
if not database_visible:
continue
Expand All @@ -365,8 +363,11 @@ async def example_links(self, request):
for table in table_names:
visible, _ = await self.ds.check_visibility(
request.actor,
"view-table",
(name, table),
permissions=[
("view-table", (name, table)),
("view-database", name),
"view-instance",
],
)
if not visible:
continue
Expand Down Expand Up @@ -463,6 +464,13 @@ async def example_links(self, request):
return databases

async def get(self, request):
visible, private = await self.ds.check_visibility(
request.actor,
permissions=["view-instance"],
)
if not visible:
raise Forbidden("You do not have permission to view this instance")

def api_path(link):
return "/-/api#{}".format(
urllib.parse.urlencode(
Expand All @@ -480,5 +488,6 @@ def api_path(link):
{
"example_links": await self.example_links(request),
"api_path": api_path,
"private": private,
},
)
16 changes: 16 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,22 @@
Changelog
=========

.. _v1_0_a4:

1.0a4 (2023-08-21)
------------------

This alpha fixes a security issue with the ``/-/api`` API explorer. On authenticated Datasette instances (instances protected using plugins such as `datasette-auth-passwords <https://datasette.io/plugins/datasette-auth-passwords>`__) the API explorer interface could reveal the names of databases and tables within the protected instance. The data stored in those tables was not revealed.

For more information and workarounds, read `the security advisory <https://github.com/simonw/datasette/security/advisories/GHSA-7ch3-7pp7-7cpq>`__. The issue has been present in every previous alpha version of Datasette 1.0: versions 1.0a0, 1.0a1, 1.0a2 and 1.0a3.

Also in this alpha:

- The new ``datasette plugins --requirements`` option outputs a list of currently installed plugins in Python ``requirements.txt`` format, useful for duplicating that installation elsewhere. (:issue:`2133`)
- :ref:`canned_queries_writable` can now define a ``on_success_message_sql`` field in their configuration, containing a SQL query that should be executed upon successful completion of the write operation in order to generate a message to be shown to the user. (:issue:`2138`)
- The automatically generated border color for a database is now shown in more places around the application. (:issue:`2119`)
- Every instance of example shell script code in the documentation should now include a working copy button, free from additional syntax. (:issue:`2140`)

.. _v1_0_a3:

1.0a3 (2023-08-09)
Expand Down
67 changes: 67 additions & 0 deletions tests/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ async def perms_ds():
(
"/",
"/fixtures",
"/-/api",
"/fixtures/compound_three_primary_keys",
"/fixtures/compound_three_primary_keys/a,a,a",
"/fixtures/two", # Query
Expand Down Expand Up @@ -951,3 +952,69 @@ def test_cli_create_token(options, expected):
)
assert 0 == result2.exit_code, result2.output
assert json.loads(result2.output) == {"actor": expected}


_visible_tables_re = re.compile(r">\/((\w+)\/(\w+))\.json<\/a> - Get rows for")


@pytest.mark.asyncio
@pytest.mark.parametrize(
"is_logged_in,metadata,expected_visible_tables",
(
# Unprotected instance logged out user sees everything:
(
False,
None,
["perms_ds_one/t1", "perms_ds_one/t2", "perms_ds_two/t1"],
),
# Fully protected instance logged out user sees nothing
(False, {"allow": {"id": "user"}}, None),
# User with visibility of just perms_ds_one sees both tables there
(
True,
{
"databases": {
"perms_ds_one": {"allow": {"id": "user"}},
"perms_ds_two": {"allow": False},
}
},
["perms_ds_one/t1", "perms_ds_one/t2"],
),
# User with visibility of only table perms_ds_one/t1 sees just that one
(
True,
{
"databases": {
"perms_ds_one": {
"allow": {"id": "user"},
"tables": {"t2": {"allow": False}},
},
"perms_ds_two": {"allow": False},
}
},
["perms_ds_one/t1"],
),
),
)
async def test_api_explorer_visibility(
perms_ds, is_logged_in, metadata, expected_visible_tables
):
try:
prev_metadata = perms_ds._metadata_local
perms_ds._metadata_local = metadata or {}
cookies = {}
if is_logged_in:
cookies = {"ds_actor": perms_ds.client.actor_cookie({"id": "user"})}
response = await perms_ds.client.get("/-/api", cookies=cookies)
if expected_visible_tables:
assert response.status_code == 200
# Search HTML for stuff matching:
# '>/perms_ds_one/t2.json</a> - Get rows for'
visible_tables = [
match[0] for match in _visible_tables_re.findall(response.text)
]
assert visible_tables == expected_visible_tables
else:
assert response.status_code == 403
finally:
perms_ds._metadata_local = prev_metadata

0 comments on commit 01e0558

Please sign in to comment.