From ab579c5063408697ce364557b2eb39247c18f849 Mon Sep 17 00:00:00 2001 From: msramalho <19508417+msramalho@users.noreply.github.com> Date: Mon, 2 Mar 2026 16:05:26 +0000 Subject: [PATCH] adds admin access to /sheet/archive endpoint by sheet_id --- app/tests/web/db/test_crud.py | 18 ++++++ app/tests/web/routers/test_sheet.py | 94 ++++++++++++++++++++++++++--- app/web/db/crud.py | 4 ++ app/web/routers/sheet.py | 58 ++++++++++++------ 4 files changed, 146 insertions(+), 28 deletions(-) diff --git a/app/tests/web/db/test_crud.py b/app/tests/web/db/test_crud.py index c3ea851..073099f 100644 --- a/app/tests/web/db/test_crud.py +++ b/app/tests/web/db/test_crud.py @@ -626,6 +626,24 @@ def test_create_sheet(db_session): ) +def test_get_sheet_by_id(test_data, db_session): + # nonexistent sheet + assert crud.get_sheet_by_id(db_session, "nonexistent") is None + + # find sheets regardless of owner + sheet = crud.get_sheet_by_id(db_session, "sheet-0") + assert sheet is not None + assert sheet.author_id == "rick@example.com" + + sheet = crud.get_sheet_by_id(db_session, "sheet-1") + assert sheet is not None + assert sheet.author_id == "morty@example.com" + + sheet = crud.get_sheet_by_id(db_session, "sheet-2") + assert sheet is not None + assert sheet.author_id == "jerry@example.com" + + def test_get_user_sheet(test_data, db_session): assert crud.get_user_sheet(db_session, "", "sheet-0") is None assert ( diff --git a/app/tests/web/routers/test_sheet.py b/app/tests/web/routers/test_sheet.py index 41c6fd6..2885598 100644 --- a/app/tests/web/routers/test_sheet.py +++ b/app/tests/web/routers/test_sheet.py @@ -191,7 +191,7 @@ class TestArchiveUserSheetEndpoint: models.Sheet( id="123-sheet-id", name="Test Sheet 1", - author_id="morty@example.com", + author_id="rick@example.com", group_id="spaceship", frequency="hourly", ) @@ -210,8 +210,82 @@ class TestArchiveUserSheetEndpoint: m_celery.signature.assert_called_once() m_signature.apply_async.assert_called_once() - def test_token_auth(self, client_with_token, test_no_auth): - test_no_auth(client_with_token.post, "/sheet/123-sheet-id/archive") + def test_token_auth(self, client_with_token): + # API token with nonexistent sheet returns 404 + r = client_with_token.post("/sheet/123-sheet-id/archive") + assert r.status_code == HTTPStatus.NOT_FOUND + assert r.json() == {"detail": "Sheet not found."} + + @patch("app.web.routers.sheet.celery", return_value=MagicMock()) + def test_token_auth_triggers_any_sheet( + self, m_celery, client_with_token, db_session + ): + # Add a sheet owned by someone else + db_session.add( + models.Sheet( + id="rick-sheet-id", + name="Rick's Sheet", + author_id="rick@example.com", + group_id="spaceship", + frequency="hourly", + ) + ) + db_session.commit() + + m_signature = MagicMock() + m_signature.apply_async.return_value = TaskResult( + id="token-taskid", status=STATUS_PENDING, result="" + ) + m_celery.signature.return_value = m_signature + + r = client_with_token.post("/sheet/rick-sheet-id/archive") + assert r.status_code == HTTPStatus.CREATED + assert r.json() == {"id": "token-taskid"} + m_celery.signature.assert_called_once() + # Verify it was queued as high priority + m_signature.apply_async.assert_called_once_with( + priority=0, queue="high_priority" + ) + + @patch("app.web.routers.sheet.celery", return_value=MagicMock()) + def test_token_auth_uses_sheet_owner_as_author( + self, m_celery, client_with_token, db_session + ): + db_session.add( + models.Sheet( + id="jerry-sheet-id", + name="Jerry's Sheet", + author_id="jerry@example.com", + group_id="the-jerrys-club", + frequency="daily", + ) + ) + db_session.commit() + + m_signature = MagicMock() + m_signature.apply_async.return_value = TaskResult( + id="token-taskid-2", status=STATUS_PENDING, result="" + ) + m_celery.signature.return_value = m_signature + + r = client_with_token.post("/sheet/jerry-sheet-id/archive") + assert r.status_code == HTTPStatus.CREATED + # Verify the sheet task uses the original sheet owner as author + call_args = m_celery.signature.call_args + import json + + submitted = json.loads( + call_args[1]["args"][0] + if "args" in call_args[1] + else call_args[0][1][0] + ) + assert submitted["author_id"] == "jerry@example.com" + assert submitted["sheet_id"] == "jerry-sheet-id" + + def test_token_auth_sheet_not_found(self, client_with_token): + r = client_with_token.post("/sheet/nonexistent-sheet/archive") + assert r.status_code == HTTPStatus.NOT_FOUND + assert r.json() == {"detail": "Sheet not found."} def test_missing_data(self, client_with_auth): r = client_with_auth.post("/sheet/123-sheet-id/archive") @@ -219,11 +293,12 @@ class TestArchiveUserSheetEndpoint: assert r.json() == {"detail": "No access to this sheet."} def test_no_access(self, client_with_auth, db_session): + # Sheet owned by morty, but auth user is rick — rick can't see morty's sheet db_session.add( models.Sheet( id="123-sheet-id", name="Test Sheet 1", - author_id="rick@example.com", + author_id="morty@example.com", group_id="spaceship", frequency="hourly", ) @@ -234,12 +309,13 @@ class TestArchiveUserSheetEndpoint: assert r.json() == {"detail": "No access to this sheet."} def test_user_not_in_group(self, client_with_auth, db_session): + # Rick owns the sheet but is not in this group db_session.add( models.Sheet( id="123-sheet-id", name="Test Sheet 1", - author_id="morty@example.com", - group_id="interdimensional", + author_id="rick@example.com", + group_id="the-jerrys-club", frequency="hourly", ) ) @@ -251,12 +327,14 @@ class TestArchiveUserSheetEndpoint: } def test_user_cannot_manually_trigger(self, client_with_auth, db_session): + # Rick is in 'animated-characters' (via domain) but that group + # does not have manually_trigger_sheet permission db_session.add( models.Sheet( id="123-sheet-id", name="Test Sheet 1", - author_id="morty@example.com", - group_id="default", + author_id="rick@example.com", + group_id="animated-characters", frequency="hourly", ) ) diff --git a/app/web/db/crud.py b/app/web/db/crud.py index e17393e..9c771dc 100644 --- a/app/web/db/crud.py +++ b/app/web/db/crud.py @@ -370,6 +370,10 @@ def create_sheet( return db_sheet +def get_sheet_by_id(db: Session, sheet_id: str) -> models.Sheet: + return db.query(models.Sheet).filter(models.Sheet.id == sheet_id).first() + + def get_user_sheet(db: Session, email: str, sheet_id: str) -> models.Sheet: return ( db.query(models.Sheet) diff --git a/app/web/routers/sheet.py b/app/web/routers/sheet.py index 2a06bf3..9bac061 100644 --- a/app/web/routers/sheet.py +++ b/app/web/routers/sheet.py @@ -13,9 +13,11 @@ from app.shared.schemas import ( SubmitSheet, ) from app.shared.task_messaging import get_celery +from app.web.config import ALLOW_ANY_EMAIL from app.web.db import crud from app.web.db.user_state import UserState -from app.web.security import get_user_state +from app.web.security import get_token_or_user_auth, get_user_state +from app.web.utils.misc import convert_priority_to_queue_dict router = APIRouter(prefix="/sheet", tags=["Google Spreadsheet operations"]) @@ -93,38 +95,54 @@ def delete_sheet( @router.post( "/{sheet_id}/archive", status_code=HTTPStatus.CREATED, - summary="Trigger an archiving task for a GSheet you own.", + summary="Trigger an archiving task for a GSheet you own, or any sheet with the API token.", response_description="task_id for the archiving task.", ) def archive_user_sheet( sheet_id: str, - user: UserState = Depends(get_user_state), + email: str = Depends(get_token_or_user_auth), db: Session = Depends(get_db_dependency), ) -> JSONResponse: - sheet = crud.get_user_sheet(db, user.email, sheet_id=sheet_id) - if not sheet: - raise HTTPException( - status_code=HTTPStatus.FORBIDDEN, detail="No access to this sheet." - ) + is_api_token = email == ALLOW_ANY_EMAIL - if not user.in_group(sheet.group_id): - raise HTTPException( - status_code=HTTPStatus.FORBIDDEN, - detail="User does not have access to this group.", - ) + if is_api_token: + # API token can trigger any sheet + sheet = crud.get_sheet_by_id(db, sheet_id) + if not sheet: + raise HTTPException( + status_code=HTTPStatus.NOT_FOUND, detail="Sheet not found." + ) + group_queue = convert_priority_to_queue_dict("high") + author_id = sheet.author_id + else: + user = UserState(db, email) + sheet = crud.get_user_sheet(db, user.email, sheet_id=sheet_id) + if not sheet: + raise HTTPException( + status_code=HTTPStatus.FORBIDDEN, + detail="No access to this sheet.", + ) - if not user.can_manually_trigger(sheet.group_id): - raise HTTPException( - status_code=HTTPStatus.TOO_MANY_REQUESTS, - detail="User cannot manually trigger sheet archiving in this group.", - ) + if not user.in_group(sheet.group_id): + raise HTTPException( + status_code=HTTPStatus.FORBIDDEN, + detail="User does not have access to this group.", + ) + + if not user.can_manually_trigger(sheet.group_id): + raise HTTPException( + status_code=HTTPStatus.TOO_MANY_REQUESTS, + detail="User cannot manually trigger sheet archiving in this group.", + ) + + group_queue = user.priority_group(sheet.group_id) + author_id = user.email - group_queue = user.priority_group(sheet.group_id) task = celery.signature( "create_sheet_task", args=[ SubmitSheet( - sheet_id=sheet_id, author_id=user.email, group_id=sheet.group_id + sheet_id=sheet_id, author_id=author_id, group_id=sheet.group_id ).model_dump_json() ], ).apply_async(**group_queue)