Repository navigation
FIX: Connection String Param for Authentication - #368
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes the connection string parameter filtering logic for Azure AD token-based authentication. The change correctly identifies that Trusted_Connection (Windows integrated authentication) conflicts with token-based auth and should be removed, while Encrypt and TrustServerCertificate (connection security settings) do not conflict and should be retained in the connection string.
Key Changes:
- Updated
remove_sensitive_paramsfunction to removeTrusted_Connectioninstead of encryption-related parameters - Modified the test to verify the new filtering behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| mssql_python/auth.py | Updated remove_sensitive_params to exclude trusted_connection instead of encrypt and trustservercertificate from the authentication filter list |
| tests/test_008_auth.py | Updated test to include Trusted_Connection parameter and verify it's filtered out while encryption parameters are retained |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. 📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.hpp: 58.8%
mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.row.py: 66.2%
mssql_python.pybind.ddbc_bindings.cpp: 66.3%
mssql_python.helpers.py: 67.5%
mssql_python.pybind.connection.connection.cpp: 73.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.connection.py: 83.7%
mssql_python.cursor.py: 84.3%
mssql_python.logging.py: 85.3%🔗 Quick Links
|
Sumit Sarabhai (sumitmsft)
left a comment
Sumit Sarabhai (sumitmsft)
left a comment
There was a problem hiding this comment.
LGTM. Quick change.
Work Item / Issue Reference
Summary
This pull request updates the logic for filtering sensitive parameters in the authentication module. The main change is that the function now removes
Trusted_Connectioninstead ofEncryptandTrustServerCertificate, and the corresponding test has been updated to reflect this new behavior.Sensitive parameter filtering update:
mssql_python/auth.py, theremove_sensitive_paramsfunction now excludestrusted_connectioninstead ofencryptandtrustservercertificatewhen filtering parameters.Test updates for new filtering logic:
tests/test_008_auth.py, the test forremove_sensitive_paramshas been updated to expect thatEncryptandTrustServerCertificateare no longer removed, whileTrusted_Connectionis now excluded.