Skip to content

Commit d51c5d4

Browse files
authored
FIX: Strip stale auth fields from pycore_context after token acquisition in bulkcopy (#488)
### Work Item / Issue Reference <!-- IMPORTANT: Please follow the PR template guidelines below. For mssql-python maintainers: Insert your ADO Work Item ID below For external contributors: Insert Github Issue number below Only one reference is required - either GitHub issue OR ADO Work Item. --> <!-- mssql-python maintainers: ADO Work Item --> > AB#43402 > [AB#43408](https://sqlclientdrivers.visualstudio.com/c6d89619-62de-46a0-8b46-70b92a84d85e/_workitems/edit/43408) <!-- External contributors: GitHub Issue --> > GitHub Issue: #<ISSUE_NUMBER> ### Summary <!-- Insert your summary of changes below. Minimum 10 characters required. --> ## Problem `cursor.bulkcopy()` acquires a fresh Azure AD token and sets `pycore_context["access_token"]`, but leaves the original `authentication`, `user_name`, and `password` keys from the parsed connection string. py-core's validator rejects `access_token` combined with those fields (ODBC parity). Affects: `ActiveDirectoryDefault`, `ActiveDirectoryInteractive`, `ActiveDirectoryDeviceCode`. ## Fix Pop `authentication`, `user_name`, and `password` from `pycore_context` after setting `access_token` — the token is the sole credential for the py-core connection. ## Companion PR mssql-rs PR replaces a panic with a proper error for the case where no token factory is registered (ADIntegrated, ADPassword). <!-- ### PR Title Guide > For feature requests FEAT: (short-description) > For non-feature requests like test case updates, config updates , dependency updates etc CHORE: (short-description) > For Fix requests FIX: (short-description) > For doc update requests DOC: (short-description) > For Formatting, indentation, or styling update STYLE: (short-description) > For Refactor, without any feature changes REFACTOR: (short-description) > For release related changes, without any feature changes RELEASE: #<RELEASE_VERSION> (short-description) ### Contribution Guidelines External contributors: - Create a GitHub issue first: https://github.com/microsoft/mssql-python/issues/new - Link the GitHub issue in the "GitHub Issue" section above - Follow the PR title format and provide a meaningful summary mssql-python maintainers: - Create an ADO Work Item following internal processes - Link the ADO Work Item in the "ADO Work Item" section above - Follow the PR title format and provide a meaningful summary -->
1 parent b786900 commit d51c5d4

2 files changed

Lines changed: 114 additions & 0 deletions

File tree

mssql_python/cursor.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2828,6 +2828,10 @@ def bulkcopy(
28282828
f"for auth_type '{self.connection._auth_type}': {e}"
28292829
) from e
28302830
pycore_context["access_token"] = raw_token
2831+
# Token replaces credential fields — py-core's validator rejects
2832+
# access_token combined with authentication/user_name/password.
2833+
for key in ("authentication", "user_name", "password"):
2834+
pycore_context.pop(key, None)
28312835
logger.debug(
28322836
"Bulk copy: acquired fresh Azure AD token for auth_type=%s",
28332837
self.connection._auth_type,
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT License.
3+
4+
"""Tests for bulkcopy auth field cleanup in cursor.py.
5+
6+
When cursor.bulkcopy() acquires an Azure AD token, it must strip stale
7+
authentication/user_name/password keys from the pycore_context dict before
8+
passing it to mssql_py_core. The Rust validator rejects access_token
9+
combined with those fields (ODBC parity).
10+
"""
11+
12+
import secrets
13+
from unittest.mock import MagicMock, patch
14+
15+
SAMPLE_TOKEN = secrets.token_hex(44)
16+
17+
18+
def _make_cursor(connection_str, auth_type):
19+
"""Build a mock Cursor with just enough wiring for bulkcopy's auth path."""
20+
from mssql_python.cursor import Cursor
21+
22+
mock_conn = MagicMock()
23+
mock_conn.connection_str = connection_str
24+
mock_conn._auth_type = auth_type
25+
mock_conn._is_connected = True
26+
27+
cursor = Cursor.__new__(Cursor)
28+
cursor._connection = mock_conn
29+
cursor.closed = False
30+
cursor.hstmt = None
31+
return cursor
32+
33+
34+
class TestBulkcopyAuthCleanup:
35+
"""Verify cursor.bulkcopy strips stale auth fields after token acquisition."""
36+
37+
@patch("mssql_python.cursor.logger")
38+
def test_token_replaces_auth_fields(self, mock_logger):
39+
"""access_token present ⇒ authentication, user_name, password removed."""
40+
mock_logger.is_debug_enabled = False
41+
42+
cursor = _make_cursor(
43+
"Server=tcp:test.database.windows.net;Database=testdb;"
44+
"Authentication=ActiveDirectoryDefault;UID=user@test.com;PWD=secret",
45+
"activedirectorydefault",
46+
)
47+
48+
captured_context = {}
49+
50+
mock_pycore_cursor = MagicMock()
51+
mock_pycore_cursor.bulkcopy.return_value = {
52+
"rows_copied": 1,
53+
"batch_count": 1,
54+
"elapsed_time": 0.1,
55+
}
56+
mock_pycore_conn = MagicMock()
57+
mock_pycore_conn.cursor.return_value = mock_pycore_cursor
58+
59+
def capture_context(ctx, **kwargs):
60+
captured_context.update(ctx)
61+
return mock_pycore_conn
62+
63+
mock_pycore_module = MagicMock()
64+
mock_pycore_module.PyCoreConnection = capture_context
65+
66+
with (
67+
patch.dict("sys.modules", {"mssql_py_core": mock_pycore_module}),
68+
patch("mssql_python.auth.AADAuth.get_raw_token", return_value=SAMPLE_TOKEN),
69+
):
70+
cursor.bulkcopy("dbo.test_table", [(1, "row")], timeout=10)
71+
72+
assert captured_context.get("access_token") == SAMPLE_TOKEN
73+
assert "authentication" not in captured_context
74+
assert "user_name" not in captured_context
75+
assert "password" not in captured_context
76+
77+
@patch("mssql_python.cursor.logger")
78+
def test_no_auth_type_leaves_fields_intact(self, mock_logger):
79+
"""No _auth_type ⇒ credentials pass through unchanged (SQL auth path)."""
80+
mock_logger.is_debug_enabled = False
81+
82+
cursor = _make_cursor(
83+
"Server=tcp:test.database.windows.net;Database=testdb;" "UID=sa;PWD=password123",
84+
None, # no AD auth
85+
)
86+
87+
captured_context = {}
88+
89+
mock_pycore_cursor = MagicMock()
90+
mock_pycore_cursor.bulkcopy.return_value = {
91+
"rows_copied": 1,
92+
"batch_count": 1,
93+
"elapsed_time": 0.1,
94+
}
95+
mock_pycore_conn = MagicMock()
96+
mock_pycore_conn.cursor.return_value = mock_pycore_cursor
97+
98+
def capture_context(ctx, **kwargs):
99+
captured_context.update(ctx)
100+
return mock_pycore_conn
101+
102+
mock_pycore_module = MagicMock()
103+
mock_pycore_module.PyCoreConnection = capture_context
104+
105+
with patch.dict("sys.modules", {"mssql_py_core": mock_pycore_module}):
106+
cursor.bulkcopy("dbo.test_table", [(1, "row")], timeout=10)
107+
108+
assert "access_token" not in captured_context
109+
assert captured_context.get("user_name") == "sa"
110+
assert captured_context.get("password") == "password123"

0 commit comments

Comments
 (0)