diff --git a/gcpde/sheets.py b/gcpde/sheets.py index 7422817..aa8940e 100644 --- a/gcpde/sheets.py +++ b/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 } - diff --git a/tests/unit/test_bq.py b/tests/unit/test_bq.py index 1fb1052..579fa99 100644 --- a/tests/unit/test_bq.py +++ b/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"}] diff --git a/tests/unit/test_sheets.py b/tests/unit/test_sheets.py index 6a1c632..3f625bb 100644 --- a/tests/unit/test_sheets.py +++ b/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