Skip to content

Commit 488a201

Browse files
fix(security): scope calendar export by user_id to close cross-user IDOR (#123)
export_to_google selected each assignment by id alone, so an authenticated caller could pass another user's assignment UUIDs to read and decrypt their private notes, push them into the caller's Google Calendar, and stamp google_event_id onto the victim's row (data corruption). require_self only validated the body's user_id, not the ids. Scope both the select and the write-back update by user_id (matching the sync/ update/delete siblings): a non-owned id now returns no row and is skipped. Test exercises the exact cross-user path — attacker authenticated as self, targeting the victim's assignment id — and asserts nothing is decrypted, exported, or stamped. Fails on pre-fix code (exported_count==1).
1 parent a27b688 commit 488a201

2 files changed

Lines changed: 108 additions & 2 deletions

File tree

backend/routes/calendar.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,9 +376,15 @@ def export_to_google(body: ExportBody, request: FastAPIRequest):
376376
exported = 0
377377
skipped = 0
378378
for aid in body.assignment_ids:
379+
# #123: scope by user_id, not just id. Without this an authenticated
380+
# caller could pass another user's assignment UUIDs to read+decrypt
381+
# their private notes, push them into the caller's calendar, and stamp
382+
# google_event_id onto the victim's row. Every sibling endpoint
383+
# (update/delete/sync) already scopes by user_id; a non-owned id now
384+
# returns no row and is skipped.
379385
rows = table("assignments").select(
380386
"id,title,due_date,notes,google_event_id,courses!left(course_code,course_name)",
381-
filters={"id": f"eq.{aid}"},
387+
filters={"id": f"eq.{aid}", "user_id": f"eq.{body.user_id}"},
382388
)
383389
if not rows:
384390
continue
@@ -400,9 +406,11 @@ def export_to_google(body: ExportBody, request: FastAPIRequest):
400406
"end": {"date": a["due_date"]},
401407
}
402408
created = service.events().insert(calendarId="primary", body=event).execute()
409+
# Scope the write-back by user_id too (defense in depth): never stamp
410+
# google_event_id onto a row the caller doesn't own.
403411
table("assignments").update(
404412
{"google_event_id": created["id"]},
405-
filters={"id": f"eq.{aid}"},
413+
filters={"id": f"eq.{aid}", "user_id": f"eq.{body.user_id}"},
406414
)
407415
exported += 1
408416

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
"""
2+
Regression test for #123: calendar.export_to_google cross-user IDOR.
3+
4+
Pre-fix, export_to_google selected each assignment by `id` alone, so an
5+
authenticated user could pass ANOTHER user's assignment UUIDs to read+decrypt
6+
their private notes, push them into the caller's Google Calendar, and stamp
7+
google_event_id onto the victim's row. The fix scopes the select (and the
8+
write-back) by user_id, so a non-owned id returns no row and is skipped.
9+
10+
The cross-user test makes the DB mock behave like a real row-scoped store: the
11+
victim's row is only returned when the query is NOT scoped to the caller
12+
(pre-fix) or is scoped to the victim. The fix queries scoped to the caller, so
13+
the victim row is never returned, never decrypted, never pushed.
14+
"""
15+
from unittest.mock import MagicMock, patch
16+
17+
from fastapi.testclient import TestClient
18+
19+
from main import app
20+
21+
client = TestClient(app)
22+
23+
ATTACKER = "user_attacker"
24+
VICTIM = "user_victim"
25+
VICTIM_ASSIGNMENT_ID = "assignment_owned_by_victim"
26+
27+
VICTIM_ROW = {
28+
"id": VICTIM_ASSIGNMENT_ID,
29+
"title": "Victim private assignment",
30+
"due_date": "2026-03-01",
31+
"notes": "ENC_secret_victim_notes",
32+
"google_event_id": None,
33+
"courses": {"course_code": "BIO101", "course_name": "Biology"},
34+
}
35+
36+
37+
def _row_scoped_select(*_args, **kwargs):
38+
"""Simulate a row-scoped table: return the victim row only for an unscoped
39+
query (pre-fix) or one scoped to the victim — never for one scoped to the
40+
attacker."""
41+
filters = kwargs.get("filters", {})
42+
uid = filters.get("user_id")
43+
if uid is None or uid == f"eq.{VICTIM}":
44+
return [VICTIM_ROW]
45+
return []
46+
47+
48+
class TestCalendarExportIDOR:
49+
def test_cannot_export_another_users_assignment(self):
50+
with patch("routes.calendar._require_google_creds", return_value=MagicMock()), \
51+
patch("routes.calendar.build") as build, \
52+
patch("routes.calendar.decrypt_if_present") as decrypt, \
53+
patch("routes.calendar.table") as t:
54+
service = MagicMock()
55+
service.events.return_value.insert.return_value.execute.return_value = {"id": "evt_new"}
56+
build.return_value = service
57+
t.return_value.select.side_effect = _row_scoped_select
58+
59+
# Attacker authenticates as themselves but targets the victim's id.
60+
r = client.post(
61+
"/api/calendar/export",
62+
json={"user_id": ATTACKER, "assignment_ids": [VICTIM_ASSIGNMENT_ID]},
63+
)
64+
65+
assert r.status_code == 200
66+
# Nothing exported: the user_id-scoped query found no row → skipped.
67+
assert r.json()["exported_count"] == 0
68+
# The victim's notes were never decrypted and never pushed to a calendar.
69+
decrypt.assert_not_called()
70+
service.events.return_value.insert.assert_not_called()
71+
# And the victim's row was never stamped (no data corruption).
72+
t.return_value.update.assert_not_called()
73+
74+
def test_owner_can_export_their_own_assignment(self):
75+
"""Control: scoping by user_id must not break the legitimate path."""
76+
own_row = {**VICTIM_ROW, "id": "my_assignment"}
77+
78+
def _select(*_args, **kwargs):
79+
uid = kwargs.get("filters", {}).get("user_id")
80+
return [own_row] if uid == f"eq.{VICTIM}" else []
81+
82+
with patch("routes.calendar._require_google_creds", return_value=MagicMock()), \
83+
patch("routes.calendar.build") as build, \
84+
patch("routes.calendar.decrypt_if_present", return_value="secret"), \
85+
patch("routes.calendar.table") as t:
86+
service = MagicMock()
87+
service.events.return_value.insert.return_value.execute.return_value = {"id": "evt_new"}
88+
build.return_value = service
89+
t.return_value.select.side_effect = _select
90+
91+
r = client.post(
92+
"/api/calendar/export",
93+
json={"user_id": VICTIM, "assignment_ids": ["my_assignment"]},
94+
)
95+
96+
assert r.status_code == 200
97+
assert r.json()["exported_count"] == 1
98+
service.events.return_value.insert.assert_called_once()

0 commit comments

Comments
 (0)