feat: check for proper spreadsheet service_account access before archiving, plus email notification

app/tests/shared/utils/test_sheets.py

@@ -0,0 +1,171 @@
+from unittest.mock import mock_open, patch
+
+from app.shared.utils.sheets import (
+    check_sheet_write_access,
+    get_service_account_json_path,
+    get_sheet_access_error,
+)
+
+
+class TestGetServiceAccountJsonPath:
+    def test_returns_none_for_empty_path(self):
+        assert get_service_account_json_path("") is None
+        assert get_service_account_json_path(None) is None
+
+    def test_returns_path_from_orchestrator_yaml(self):
+        # The test orchestration file has a service_account key
+        get_service_account_json_path.cache_clear()
+        result = get_service_account_json_path(
+            "app/tests/orchestration.test.yaml"
+        )
+        assert result == "app/tests/fake_service_account.json"
+
+    def test_returns_none_for_missing_file(self):
+        get_service_account_json_path.cache_clear()
+        assert get_service_account_json_path("nonexistent/path.yaml") is None
+
+    def test_returns_none_for_invalid_yaml(self):
+        get_service_account_json_path.cache_clear()
+        with patch(
+            "builtins.open", mock_open(read_data="!!! invalid yaml {{{")
+        ):
+            result = get_service_account_json_path("some/path.yaml")
+            # yaml.safe_load may return a string for some invalid inputs
+            # The function should not crash
+            assert result is None or isinstance(result, str)
+
+    def test_returns_none_when_no_service_account_key(self):
+        get_service_account_json_path.cache_clear()
+        yaml_content = "steps:\n  feeders:\n    - cli_feeder\n"
+        with patch("builtins.open", mock_open(read_data=yaml_content)):
+            assert get_service_account_json_path("some/path.yaml") is None
+
+    def test_finds_nested_service_account_key(self):
+        get_service_account_json_path.cache_clear()
+        yaml_content = (
+            "configurations:\n"
+            "  gsheet_feeder_db:\n"
+            "    service_account: secrets/nested_sa.json\n"
+        )
+        with patch("builtins.open", mock_open(read_data=yaml_content)):
+            result = get_service_account_json_path("some/path.yaml")
+            assert result == "secrets/nested_sa.json"
+
+
+class TestCheckSheetWriteAccess:
+    @patch("app.shared.utils.sheets.http_requests.get")
+    @patch(
+        "google.oauth2.service_account.Credentials.from_service_account_file"
+    )
+    def test_returns_true_when_can_edit(self, m_creds, m_get):
+        m_creds.return_value.token = "fake-token"
+        m_get.return_value.status_code = 200
+        m_get.return_value.json.return_value = {
+            "capabilities": {"canEdit": True}
+        }
+
+        result = check_sheet_write_access("sa.json", "sheet123")
+        assert result is True
+        m_get.assert_called_once()
+
+    @patch("app.shared.utils.sheets.http_requests.get")
+    @patch(
+        "google.oauth2.service_account.Credentials.from_service_account_file"
+    )
+    def test_returns_false_when_cannot_edit(self, m_creds, m_get):
+        m_creds.return_value.token = "fake-token"
+        m_get.return_value.status_code = 200
+        m_get.return_value.json.return_value = {
+            "capabilities": {"canEdit": False}
+        }
+
+        result = check_sheet_write_access("sa.json", "sheet123")
+        assert result is False
+
+    @patch("app.shared.utils.sheets.http_requests.get")
+    @patch(
+        "google.oauth2.service_account.Credentials.from_service_account_file"
+    )
+    def test_returns_false_on_404(self, m_creds, m_get):
+        m_creds.return_value.token = "fake-token"
+        m_get.return_value.status_code = 404
+
+        result = check_sheet_write_access("sa.json", "sheet123")
+        assert result is False
+
+    @patch("app.shared.utils.sheets.http_requests.get")
+    @patch(
+        "google.oauth2.service_account.Credentials.from_service_account_file"
+    )
+    def test_returns_false_on_403(self, m_creds, m_get):
+        m_creds.return_value.token = "fake-token"
+        m_get.return_value.status_code = 403
+
+        result = check_sheet_write_access("sa.json", "sheet123")
+        assert result is False
+
+    @patch("app.shared.utils.sheets.http_requests.get")
+    @patch(
+        "google.oauth2.service_account.Credentials.from_service_account_file"
+    )
+    def test_returns_none_on_unexpected_status(self, m_creds, m_get):
+        m_creds.return_value.token = "fake-token"
+        m_get.return_value.status_code = 500
+        m_get.return_value.text = "Internal Server Error"
+
+        result = check_sheet_write_access("sa.json", "sheet123")
+        assert result is None
+
+    def test_returns_none_when_file_not_found(self):
+        result = check_sheet_write_access("nonexistent/sa.json", "sheet123")
+        assert result is None
+
+    @patch(
+        "google.oauth2.service_account.Credentials.from_service_account_file",
+        side_effect=Exception("auth failed"),
+    )
+    def test_returns_none_on_auth_error(self, m_creds):
+        result = check_sheet_write_access("sa.json", "sheet123")
+        assert result is None
+
+
+class TestGetSheetAccessError:
+    @patch("app.shared.utils.sheets.check_sheet_write_access")
+    @patch("app.shared.utils.sheets.get_service_account_json_path")
+    def test_returns_none_when_access_ok(self, m_get_path, m_check):
+        m_get_path.return_value = "sa.json"
+        m_check.return_value = True
+
+        result = get_sheet_access_error("orch.yaml", "sa@test.com", "sheet1")
+        assert result is None
+
+    @patch("app.shared.utils.sheets.check_sheet_write_access")
+    @patch("app.shared.utils.sheets.get_service_account_json_path")
+    def test_returns_error_when_no_access(self, m_get_path, m_check):
+        m_get_path.return_value = "sa.json"
+        m_check.return_value = False
+
+        result = get_sheet_access_error("orch.yaml", "sa@test.com", "sheet1")
+        assert result is not None
+        assert "sa@test.com" in result
+        assert "Editor" in result
+
+    @patch("app.shared.utils.sheets.check_sheet_write_access")
+    @patch("app.shared.utils.sheets.get_service_account_json_path")
+    def test_returns_none_when_indeterminate(self, m_get_path, m_check):
+        m_get_path.return_value = "sa.json"
+        m_check.return_value = None
+
+        result = get_sheet_access_error("orch.yaml", "sa@test.com", "sheet1")
+        assert result is None
+
+    def test_returns_none_when_no_orchestrator_path(self):
+        assert get_sheet_access_error(None, "sa@test.com", "sheet1") is None
+        assert get_sheet_access_error("", "sa@test.com", "sheet1") is None
+
+    @patch("app.shared.utils.sheets.get_service_account_json_path")
+    def test_returns_none_when_no_sa_json_path(self, m_get_path):
+        m_get_path.return_value = None
+
+        result = get_sheet_access_error("orch.yaml", "sa@test.com", "sheet1")
+        assert result is None

app/tests/web/routers/test_sheet.py

@@ -344,3 +344,133 @@ class TestArchiveUserSheetEndpoint:
         assert r.json() == {
             "detail": "User cannot manually trigger sheet archiving in this group."
         }
+
+
+class TestSheetAccessPermissionCheck:
+    """Tests for the Google Sheet write access permission check."""
+
+    ERROR_MSG = (
+        "The Google Sheet has not been shared with the Auto Archiver "
+        "service account (sa@test.iam.gserviceaccount.com). Please "
+        "share the sheet with this email address and give it Editor "
+        "permissions."
+    )
+
+    @patch(
+        "app.web.routers.sheet.get_sheet_access_error",
+        return_value=ERROR_MSG,
+    )
+    def test_create_sheet_no_write_access(
+        self, m_access, app_with_auth, db_session
+    ):
+        """Sheet creation is blocked when the SA has no write access."""
+        client = TestClient(app_with_auth)
+        data = {
+            "id": "no-access-sheet",
+            "name": "Test Sheet",
+            "group_id": "spaceship",
+            "frequency": "daily",
+        }
+        r = client.post("/sheet/create", json=data)
+        assert r.status_code == HTTPStatus.FORBIDDEN
+        assert "service account" in r.json()["detail"]
+        m_access.assert_called_once()
+
+    @patch(
+        "app.web.routers.sheet.get_sheet_access_error",
+        return_value=None,
+    )
+    def test_create_sheet_access_indeterminate_proceeds(
+        self, m_access, app_with_auth, db_session
+    ):
+        """Sheet creation proceeds when access check is indeterminate."""
+        client = TestClient(app_with_auth)
+        data = {
+            "id": "maybe-access-sheet",
+            "name": "Test Sheet",
+            "group_id": "spaceship",
+            "frequency": "daily",
+        }
+        r = client.post("/sheet/create", json=data)
+        assert r.status_code == HTTPStatus.CREATED
+        m_access.assert_called_once()
+
+    @patch(
+        "app.web.routers.sheet.get_sheet_access_error",
+        return_value=ERROR_MSG,
+    )
+    def test_archive_sheet_no_write_access(
+        self, m_access, app_with_auth, db_session
+    ):
+        """Manual trigger is blocked when the SA has no write access."""
+        db_session.add(
+            models.Sheet(
+                id="123-sheet-id",
+                name="Test Sheet 1",
+                author_id="rick@example.com",
+                group_id="spaceship",
+                frequency="hourly",
+            )
+        )
+        db_session.commit()
+        client = TestClient(app_with_auth)
+        r = client.post("/sheet/123-sheet-id/archive")
+        assert r.status_code == HTTPStatus.FORBIDDEN
+        assert "service account" in r.json()["detail"]
+        assert "Editor" in r.json()["detail"]
+        m_access.assert_called_once()
+
+    @patch("app.web.routers.sheet.celery", return_value=MagicMock())
+    @patch(
+        "app.web.routers.sheet.get_sheet_access_error",
+        return_value=None,
+    )
+    def test_archive_sheet_access_ok_proceeds(
+        self, m_access, m_celery, app_with_auth, db_session
+    ):
+        """Manual trigger proceeds when access check passes."""
+        db_session.add(
+            models.Sheet(
+                id="123-sheet-id",
+                name="Test Sheet 1",
+                author_id="rick@example.com",
+                group_id="spaceship",
+                frequency="hourly",
+            )
+        )
+        db_session.commit()
+
+        m_signature = MagicMock()
+        m_signature.apply_async.return_value = TaskResult(
+            id="task-123", status=STATUS_PENDING, result=""
+        )
+        m_celery.signature.return_value = m_signature
+
+        client = TestClient(app_with_auth)
+        r = client.post("/sheet/123-sheet-id/archive")
+        assert r.status_code == HTTPStatus.CREATED
+        m_access.assert_called_once()
+        m_celery.signature.assert_called_once()
+
+    @patch(
+        "app.web.routers.sheet.get_sheet_access_error",
+        return_value=ERROR_MSG,
+    )
+    def test_token_archive_sheet_no_write_access(
+        self, m_access, app_with_token, db_session
+    ):
+        """API token trigger is also blocked when SA has no write access."""
+        db_session.add(
+            models.Sheet(
+                id="token-sheet-id",
+                name="Token Sheet",
+                author_id="rick@example.com",
+                group_id="spaceship",
+                frequency="hourly",
+            )
+        )
+        db_session.commit()
+        client = TestClient(app_with_token)
+        r = client.post("/sheet/token-sheet-id/archive")
+        assert r.status_code == HTTPStatus.FORBIDDEN
+        assert "service account" in r.json()["detail"]