Skip to content

Commit

Permalink
upsert needs both insert-row and update-row, refs #1878
Browse files Browse the repository at this point in the history
Also made a few tweaks to how _r works in tokens and actors,
refs #1855 - I needed that mechanism for the tests.
  • Loading branch information
simonw committed Dec 6, 2022
1 parent 6c3c8f7 commit 6da17d5
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 13 deletions.
4 changes: 3 additions & 1 deletion datasette/default_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def permission_allowed_actor_restrictions(actor, action, resource):
if action_initials in database_allowed:
return None
# Or the current table? That's any time the resource is (database, table)
if not isinstance(resource, str) and len(resource) == 2:
if resource is not None and not isinstance(resource, str) and len(resource) == 2:
database, table = resource
table_allowed = _r.get("t", {}).get(database, {}).get(table)
# TODO: What should this do for canned queries?
Expand Down Expand Up @@ -138,6 +138,8 @@ def actor_from_request(datasette, request):
# Expired
return None
actor = {"id": decoded["a"], "token": "dstok"}
if "_r" in decoded:
actor["_r"] = decoded["_r"]
if duration:
actor["token_expires"] = created + duration
return actor
Expand Down
28 changes: 22 additions & 6 deletions datasette/views/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -1174,11 +1174,27 @@ async def post(self, request, upsert=False):
db = self.ds.get_database(database_name)
if not await db.table_exists(table_name):
return _error(["Table not found: {}".format(table_name)], 404)
# Must have insert-row permission
if not await self.ds.permission_allowed(
request.actor, "insert-row", resource=(database_name, table_name)
):
return _error(["Permission denied"], 403)

if upsert:
# Must have insert-row AND upsert-row permissions
if not (
await self.ds.permission_allowed(
request.actor, "insert-row", database_name, table_name
)
and await self.ds.permission_allowed(
request.actor, "update-row", database_name, table_name
)
):
return _error(
["Permission denied: need both insert-row and update-row"], 403
)
else:
# Must have insert-row permission
if not await self.ds.permission_allowed(
request.actor, "insert-row", resource=(database_name, table_name)
):
return _error(["Permission denied"], 403)

if not db.is_mutable:
return _error(["Database is immutable"], 403)

Expand Down Expand Up @@ -1227,7 +1243,7 @@ def insert_or_upsert_rows(conn):
result = {"ok": True}
if should_return:
result["rows"] = rows
return Response.json(result, status=201)
return Response.json(result, status=200 if upsert else 201)


class TableUpsertView(TableInsertView):
Expand Down
36 changes: 30 additions & 6 deletions tests/test_api_write.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@ def ds_write(tmp_path_factory):
db.close()


def write_token(ds, actor_id="root"):
return "dstok_{}".format(
ds.sign(
{"a": actor_id, "token": "dstok", "t": int(time.time())}, namespace="token"
)
)
def write_token(ds, actor_id="root", permissions=None):
to_sign = {"a": actor_id, "token": "dstok", "t": int(time.time())}
if permissions:
to_sign["_r"] = {"a": permissions}
return "dstok_{}".format(ds.sign(to_sign, namespace="token"))


@pytest.mark.asyncio
Expand Down Expand Up @@ -244,12 +243,31 @@ async def test_insert_rows(ds_write, return_rows):
400,
["Upsert does not support ignore or replace"],
),
# Upsert permissions
(
"/data/docs/-/upsert",
{"rows": [{"id": 1, "title": "Disallowed"}]},
"insert-but-not-update",
403,
["Permission denied: need both insert-row and update-row"],
),
(
"/data/docs/-/upsert",
{"rows": [{"id": 1, "title": "Disallowed"}]},
"update-but-not-insert",
403,
["Permission denied: need both insert-row and update-row"],
),
),
)
async def test_insert_or_upsert_row_errors(
ds_write, path, input, special_case, expected_status, expected_errors
):
token = write_token(ds_write)
if special_case == "insert-but-not-update":
token = write_token(ds_write, permissions=["ir", "vi"])
if special_case == "update-but-not-insert":
token = write_token(ds_write, permissions=["ur", "vi"])
if special_case == "duplicate_id":
await ds_write.get_database("data").execute_write(
"insert into docs (id) values (1)"
Expand All @@ -265,6 +283,12 @@ async def test_insert_or_upsert_row_errors(
else "application/json",
},
)

actor_response = (
await ds_write.client.get("/-/actor.json", headers=kwargs["headers"])
).json()
print(actor_response)

if special_case == "invalid_json":
del kwargs["json"]
kwargs["content"] = "{bad json"
Expand Down

0 comments on commit 6da17d5

Please sign in to comment.