Skip to content

Commit 19a681d

Browse files
Thun78A-makarim
authored andcommitted
Replaced admin-view, with per-user ownership model
Signed-off-by: A-makarim <syedmakarim0.2@gmail.com>
1 parent 1827b1a commit 19a681d

11 files changed

Lines changed: 850 additions & 266 deletions

File tree

ai_platform_engineering/autonomous_agents/src/autonomous_agents/routes/tasks.py

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,17 @@ def _assert_task_access(task: TaskDefinition, caller_email: str | None, is_admin
5151
"""Raise 403 if caller does not own the task and is not an admin.
5252
5353
Tasks without an owner_id (created before this feature) are treated as
54-
admin-only to prevent accidental cross-user exposure.
54+
admin-only to prevent accidental cross-user exposure. This orphaned-task
55+
branch is the **only** path that produces a 403 for an admin-eligible
56+
caller: once `is_admin` is true above, every other ownership check is
57+
short-circuited. Backfilling `owner_id` for pre-feature tasks is the
58+
out-of-band remediation; we deliberately do not auto-assign here so the
59+
audit story stays clean (admin acted, not "system silently re-owned").
60+
61+
Audit signal for cross-user admin actions is NOT emitted here — it is
62+
emitted at the verb call sites (`update_task` / `delete_task` /
63+
`trigger_task_manually`) so the log line carries the action verb and
64+
the task's human-readable name without re-fetching from the store.
5565
"""
5666
if is_admin:
5767
return
@@ -243,6 +253,15 @@ async def update_task(task_id: str, task: TaskDefinition, request: Request) -> d
243253
if existing is not None:
244254
caller_email, is_admin = _get_caller(request)
245255
_assert_task_access(existing, caller_email, is_admin)
256+
# Admin acting on someone else's task -- emit an audit log line at
257+
# the verb call site (per plan section 4.4) so log scanners see the
258+
# action verb and the human-readable task name without joining
259+
# against the store.
260+
if is_admin and existing.owner_id and existing.owner_id != caller_email:
261+
logger.info(
262+
"Admin %s acted on task %s (%r) owned by %s (action=%s)",
263+
caller_email, task_id, existing.name, existing.owner_id, "update",
264+
)
246265
# Preserve owner_id from the original task — callers cannot reassign ownership.
247266
task = task.model_copy(update={"owner_id": existing.owner_id})
248267

@@ -310,6 +329,11 @@ async def delete_task(task_id: str, request: Request) -> None:
310329
raise HTTPException(status_code=404, detail=f"Task '{task_id}' not found")
311330
caller_email, is_admin = _get_caller(request)
312331
_assert_task_access(task, caller_email, is_admin)
332+
if is_admin and task.owner_id and task.owner_id != caller_email:
333+
logger.info(
334+
"Admin %s acted on task %s (%r) owned by %s (action=%s)",
335+
caller_email, task_id, task.name, task.owner_id, "delete",
336+
)
313337
try:
314338
await store.delete(task_id)
315339
except TaskNotFoundError as exc:
@@ -340,6 +364,11 @@ async def trigger_task_manually(task_id: str, request: Request) -> dict:
340364
raise HTTPException(status_code=404, detail=f"Task '{task_id}' not found")
341365
caller_email, is_admin = _get_caller(request)
342366
_assert_task_access(task, caller_email, is_admin)
367+
if is_admin and task.owner_id and task.owner_id != caller_email:
368+
logger.info(
369+
"Admin %s acted on task %s (%r) owned by %s (action=%s)",
370+
caller_email, task_id, task.name, task.owner_id, "trigger",
371+
)
343372

344373
# Fire-and-forget -- the run is recorded in the store as it
345374
# progresses so the UI can poll /tasks/{id}/runs to see the result.

ai_platform_engineering/autonomous_agents/tests/test_tasks_route.py

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -762,3 +762,185 @@ def test_owner_can_delete_own_task(self, client: TestClient):
762762
client.post("/api/v1/tasks", json=_cron_task("t1"), headers=_user_headers("alice@example.com"))
763763
resp = client.delete("/api/v1/tasks/t1", headers=_user_headers("alice@example.com"))
764764
assert resp.status_code == 204
765+
766+
def test_non_admin_cannot_update_another_users_task(self, client: TestClient):
767+
"""PUT /tasks/{id} returns 403 when task belongs to a different user."""
768+
client.post(
769+
"/api/v1/tasks",
770+
json=_cron_task("t1"),
771+
headers=_user_headers("alice@example.com"),
772+
)
773+
updated_payload = _cron_task("t1")
774+
updated_payload["name"] = "renamed by bob"
775+
resp = client.put(
776+
"/api/v1/tasks/t1",
777+
json=updated_payload,
778+
headers=_user_headers("bob@example.com"),
779+
)
780+
assert resp.status_code == 403
781+
782+
def test_non_admin_cannot_trigger_another_users_task(self, client: TestClient):
783+
"""POST /tasks/{id}/run returns 403 when task belongs to a different user."""
784+
client.post(
785+
"/api/v1/tasks",
786+
json=_cron_task("t1"),
787+
headers=_user_headers("alice@example.com"),
788+
)
789+
resp = client.post(
790+
"/api/v1/tasks/t1/run",
791+
headers=_user_headers("bob@example.com"),
792+
)
793+
assert resp.status_code == 403
794+
795+
def test_admin_can_update_another_users_task(self, client: TestClient):
796+
"""An admin can PUT another user's task."""
797+
client.post(
798+
"/api/v1/tasks",
799+
json=_cron_task("t1"),
800+
headers=_user_headers("bob@example.com"),
801+
)
802+
updated_payload = _cron_task("t1")
803+
updated_payload["name"] = "renamed by admin"
804+
resp = client.put(
805+
"/api/v1/tasks/t1",
806+
json=updated_payload,
807+
headers=_admin_headers(),
808+
)
809+
assert resp.status_code == 200
810+
# Ownership is preserved across an admin-initiated update.
811+
listed = client.get("/api/v1/tasks", headers=_admin_headers()).json()
812+
owners = {t["id"]: t["owner_id"] for t in listed}
813+
assert owners["t1"] == "bob@example.com"
814+
815+
def test_admin_can_delete_another_users_task(self, client: TestClient):
816+
"""An admin can DELETE another user's task."""
817+
client.post(
818+
"/api/v1/tasks",
819+
json=_cron_task("t1"),
820+
headers=_user_headers("bob@example.com"),
821+
)
822+
resp = client.delete("/api/v1/tasks/t1", headers=_admin_headers())
823+
assert resp.status_code == 204
824+
825+
826+
class TestAdminAuditLogging:
827+
"""Section 4.4: admin actions on someone else's task emit a log line."""
828+
829+
def test_admin_update_emits_audit_log(
830+
self, client: TestClient, caplog: pytest.LogCaptureFixture
831+
):
832+
"""Admin PUT on another user's task emits a log line with both emails + action."""
833+
client.post(
834+
"/api/v1/tasks",
835+
json=_cron_task("t1"),
836+
headers=_user_headers("bob@example.com"),
837+
)
838+
updated_payload = _cron_task("t1")
839+
updated_payload["name"] = "renamed by admin"
840+
caplog.clear()
841+
with caplog.at_level("INFO", logger="autonomous_agents"):
842+
resp = client.put(
843+
"/api/v1/tasks/t1",
844+
json=updated_payload,
845+
headers={
846+
"X-Authenticated-User-Email": "admin@example.com",
847+
"X-Authenticated-User-Is-Admin": "true",
848+
},
849+
)
850+
assert resp.status_code == 200
851+
admin_log_lines = [
852+
r.getMessage()
853+
for r in caplog.records
854+
if "Admin " in r.getMessage() and "acted on task" in r.getMessage()
855+
]
856+
assert admin_log_lines, "expected an admin audit log line on PUT"
857+
msg = admin_log_lines[0]
858+
assert "admin@example.com" in msg
859+
assert "bob@example.com" in msg
860+
assert "t1" in msg
861+
assert "update" in msg
862+
863+
def test_admin_delete_emits_audit_log(
864+
self, client: TestClient, caplog: pytest.LogCaptureFixture
865+
):
866+
"""Admin DELETE on another user's task emits a log line with both emails + action."""
867+
client.post(
868+
"/api/v1/tasks",
869+
json=_cron_task("t1"),
870+
headers=_user_headers("bob@example.com"),
871+
)
872+
caplog.clear()
873+
with caplog.at_level("INFO", logger="autonomous_agents"):
874+
resp = client.delete(
875+
"/api/v1/tasks/t1",
876+
headers={
877+
"X-Authenticated-User-Email": "admin@example.com",
878+
"X-Authenticated-User-Is-Admin": "true",
879+
},
880+
)
881+
assert resp.status_code == 204
882+
admin_log_lines = [
883+
r.getMessage()
884+
for r in caplog.records
885+
if "Admin " in r.getMessage() and "acted on task" in r.getMessage()
886+
]
887+
assert admin_log_lines, "expected an admin audit log line on DELETE"
888+
msg = admin_log_lines[0]
889+
assert "admin@example.com" in msg
890+
assert "bob@example.com" in msg
891+
assert "delete" in msg
892+
893+
def test_admin_trigger_emits_audit_log(
894+
self, client: TestClient, caplog: pytest.LogCaptureFixture
895+
):
896+
"""Admin manual trigger on another user's task emits a log line."""
897+
client.post(
898+
"/api/v1/tasks",
899+
json=_cron_task("t1"),
900+
headers=_user_headers("bob@example.com"),
901+
)
902+
caplog.clear()
903+
with caplog.at_level("INFO", logger="autonomous_agents"):
904+
resp = client.post(
905+
"/api/v1/tasks/t1/run",
906+
headers={
907+
"X-Authenticated-User-Email": "admin@example.com",
908+
"X-Authenticated-User-Is-Admin": "true",
909+
},
910+
)
911+
assert resp.status_code == 200
912+
admin_log_lines = [
913+
r.getMessage()
914+
for r in caplog.records
915+
if "Admin " in r.getMessage() and "acted on task" in r.getMessage()
916+
]
917+
assert admin_log_lines, "expected an admin audit log line on manual trigger"
918+
msg = admin_log_lines[0]
919+
assert "admin@example.com" in msg
920+
assert "bob@example.com" in msg
921+
assert "trigger" in msg
922+
923+
def test_owner_action_does_not_emit_admin_audit_log(
924+
self, client: TestClient, caplog: pytest.LogCaptureFixture
925+
):
926+
"""Owner acting on their own task must not emit the cross-user admin audit log line."""
927+
client.post(
928+
"/api/v1/tasks",
929+
json=_cron_task("t1"),
930+
headers=_user_headers("bob@example.com"),
931+
)
932+
caplog.clear()
933+
with caplog.at_level("INFO", logger="autonomous_agents"):
934+
resp = client.delete(
935+
"/api/v1/tasks/t1",
936+
headers=_user_headers("bob@example.com"),
937+
)
938+
assert resp.status_code == 204
939+
admin_log_lines = [
940+
r.getMessage()
941+
for r in caplog.records
942+
if "Admin " in r.getMessage() and "acted on task" in r.getMessage()
943+
]
944+
assert admin_log_lines == [], (
945+
"owner deleting their own task should not emit the admin audit line"
946+
)

0 commit comments

Comments
 (0)