🐛 fix issue with scoping for sheets

gcpde/sheets.py

@@ -4,11 +4,18 @@
 
 import gspread
 from google.auth.credentials import Credentials as GoogleCredentials
+from google.auth.credentials import Scoped
 from gspread import Spreadsheet, Worksheet
 from loguru import logger
 
 from gcpde.types import ListJsonType
 
+_SHEETS_SCOPES = [
+    "https://spreadsheets.google.com/feeds",
+    "https://www.googleapis.com/auth/spreadsheets",
+    "https://www.googleapis.com/auth/drive",
+]
+
 
 def _open_document(
     document_id: str,
@@ -19,11 +26,32 @@ def _open_document(
         raise ValueError(
             "You must provide either a json_key or credentials to connect to sheets."
         )
-    gc = (
-        gspread.authorize(credentials)
-        if credentials
-        else gspread.service_account_from_dict(json_key)  # type: ignore[arg-type]
-    )
+    if credentials is not None:
+        needs_scopes = isinstance(credentials, Scoped) and not (
+            credentials.scopes
+            and set(credentials.scopes)
+            & {
+                "https://www.googleapis.com/auth/spreadsheets",
+                "https://spreadsheets.google.com/feeds",
+            }
+        )
+        if needs_scopes:
+            assert isinstance(credentials, Scoped)
+            _creds: GoogleCredentials = credentials.with_scopes(  # type: ignore[no-untyped-call]
+                _SHEETS_SCOPES
+            )
+        elif not isinstance(credentials, Scoped):
+            logger.warning(
+                "Credentials do not support scoping. Ensure they were obtained with "
+                "the required Google Sheets scopes: "
+                "https://www.googleapis.com/auth/spreadsheets"
+            )
+            _creds = credentials
+        else:
+            _creds = credentials
+        gc = gspread.authorize(_creds)
+    else:
+        gc = gspread.service_account_from_dict(json_key)  # type: ignore[arg-type]
     return gc.open_by_key(document_id)
 
 
@@ -159,4 +187,3 @@ def read_sheets(
         )
         for sheet_name in sheet_names
     }
-

tests/unit/test_bq.py

@@ -512,8 +512,8 @@ def test_upsert_table_from_records_schema_mismatch(mock_delete_table):
     temp_table_mock = Mock()
     table_mock.schema = [{"name": "uuid", "type": "STRING", "mode": "NULLABLE"}]
     temp_table_mock.schema = [{"name": "id", "type": "INTEGER", "mode": "NULLABLE"}]
-    mock_client.get_table = (
-        lambda dataset, table: table_mock if table == "table" else temp_table_mock
+    mock_client.get_table = lambda dataset, table: (
+        table_mock if table == "table" else temp_table_mock
     )
 
     schema_json = [{"name": "uuid", "type": "STRING", "mode": "NULLABLE"}]

tests/unit/test_sheets.py

@@ -1,5 +1,8 @@
 from unittest import mock
 
+import pytest
+from google.auth.credentials import Scoped
+
 from gcpde import sheets
 
 MockWorksheet = mock.Mock("gspread.worksheet.Worksheet", autospec=True)
@@ -19,6 +22,51 @@ def test__open_sheet(mock_service_account_from_dict: mock.Mock):
     mock_service_account_from_dict.assert_called_once_with(MOCK_JSON_KEY)
 
 
+def test__open_document_raises_without_credentials():
+    with pytest.raises(ValueError, match="json_key or credentials"):
+        sheets._open_document(document_id="document_id")
+
+
+@mock.patch("gspread.authorize", autospec=True)
+def test__open_document_scoped_credentials_no_scopes(mock_authorize: mock.Mock):
+    """Scoped credentials with no scopes should have Sheets scopes added."""
+    mock_creds = mock.MagicMock(spec=Scoped)
+    mock_creds.scopes = None
+    scoped_creds = mock.MagicMock()
+    mock_creds.with_scopes.return_value = scoped_creds
+
+    sheets._open_document(document_id="document_id", credentials=mock_creds)
+
+    mock_creds.with_scopes.assert_called_once_with(sheets._SHEETS_SCOPES)
+    mock_authorize.assert_called_once_with(scoped_creds)
+
+
+@mock.patch("gspread.authorize", autospec=True)
+def test__open_document_scoped_credentials_with_sheets_scope(mock_authorize: mock.Mock):
+    """Scoped credentials that already have a Sheets scope should be used as-is."""
+    mock_creds = mock.MagicMock(spec=Scoped)
+    mock_creds.scopes = ["https://www.googleapis.com/auth/spreadsheets"]
+
+    sheets._open_document(document_id="document_id", credentials=mock_creds)
+
+    mock_creds.with_scopes.assert_not_called()
+    mock_authorize.assert_called_once_with(mock_creds)
+
+
+@mock.patch("gspread.authorize", autospec=True)
+def test__open_document_non_scoped_credentials_logs_warning(mock_authorize: mock.Mock):
+    """Non-Scoped credentials cannot be re-scoped; a warning should be emitted."""
+    from google.auth.credentials import Credentials as GoogleCredentials
+
+    mock_creds = mock.MagicMock(spec=GoogleCredentials)
+
+    with mock.patch("gcpde.sheets.logger") as mock_logger:
+        sheets._open_document(document_id="document_id", credentials=mock_creds)
+        mock_logger.warning.assert_called_once()
+
+    mock_authorize.assert_called_once_with(mock_creds)
+
+
 @mock.patch("gcpde.sheets._open_sheet", autospec=True)
 def test_replace_from_records(mock_open_sheet: mock.Mock):
     # arrange
@@ -42,6 +90,22 @@ def test_replace_from_records(mock_open_sheet: mock.Mock):
     assert kwargs["range_name"] == "A1"
 
 
+@mock.patch("gcpde.sheets._open_sheet", autospec=True)
+def test_read_sheet(mock_open_sheet: mock.Mock):
+    mock_worksheet = MockWorksheet()
+    mock_open_sheet.return_value = mock_worksheet
+    mock_worksheet.get_all_records.return_value = [{"col": "value"}, {"col": ""}]
+
+    result = sheets.read_sheet(
+        document_id="document_id",
+        sheet_name="sheet_name",
+        json_key=MOCK_JSON_KEY,
+    )
+
+    mock_open_sheet.assert_called_once()
+    assert result == [{"col": "value"}, {"col": None}]
+
+
 @mock.patch("gcpde.sheets._open_document", autospec=True)
 def test_read_sheets(mock_open_document: mock.Mock):
     # arrange