You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In review of #107, @David-Engel flagged the Authentication keyword-value handling in mssql-odbc/src/connection/connection_string_parser.rs:
I hate that we are perpetuating this mistake. 😢
No need to change it here since the PR is intentionally replicating msodbcsql behavior. Just sharing disappointment. Maybe we can fix it in the future...
The "mistake": msodbcsql only accepts ActiveDirectoryMSI as the Authentication value for managed-identity auth, and rejects the intuitive, self-documenting spelling ActiveDirectoryManagedIdentity — even though both map to the exact same auth method (dlgattr.h). Because #107 intentionally mirrors msodbcsql byte-for-byte, mssql-odbc inherits this:
Server=h;Authentication=ActiveDirectoryManagedIdentity -> ERROR (rejected)
Server=h;Authentication=ActiveDirectoryMSI -> OK
(See the authentication_managed_identity_is_rejected test, ~line 825, and referenced bug #46177.)
This is a poor experience: ActiveDirectoryManagedIdentity is the descriptive name users know from other Microsoft data drivers/SDKs, and having a brand-new driver reject it in favor of the cryptic ...MSI abbreviation perpetuates a long-standing wart with no back-compat reason to keep it.
Proposed solution
In mssql-odbc, accept ActiveDirectoryManagedIdentity as an alias for the managed-identity auth method, in addition to ActiveDirectoryMSI, so both resolve to the same method. Keep ActiveDirectoryMSI working for compatibility. Update the parser's recognized-keyword set, the authentication_managed_identity_is_rejected unit test (it should become "accepted"), and mssql-odbc/docs/connection_string_parser.md.
This is an intentional, documented divergence from msodbcsql in favor of correctness/UX — the same class of decision tracked in the broader parity-review follow-up (#114). Consider handling it there or as a focused change.
Affected crate
mssql-odbc (not in the dropdown; closest: Not sure / Multiple)
Rename to only accept ActiveDirectoryManagedIdentity and drop ActiveDirectoryMSI: rejected — would break users/scripts already relying on the ...MSI spelling. Accept both.
Problem statement
In review of #107, @David-Engel flagged the
Authenticationkeyword-value handling inmssql-odbc/src/connection/connection_string_parser.rs:The "mistake": msodbcsql only accepts
ActiveDirectoryMSIas theAuthenticationvalue for managed-identity auth, and rejects the intuitive, self-documenting spellingActiveDirectoryManagedIdentity— even though both map to the exact same auth method (dlgattr.h). Because #107 intentionally mirrors msodbcsql byte-for-byte,mssql-odbcinherits this:(See the
authentication_managed_identity_is_rejectedtest, ~line 825, and referenced bug #46177.)This is a poor experience:
ActiveDirectoryManagedIdentityis the descriptive name users know from other Microsoft data drivers/SDKs, and having a brand-new driver reject it in favor of the cryptic...MSIabbreviation perpetuates a long-standing wart with no back-compat reason to keep it.Proposed solution
In
mssql-odbc, acceptActiveDirectoryManagedIdentityas an alias for the managed-identity auth method, in addition toActiveDirectoryMSI, so both resolve to the same method. KeepActiveDirectoryMSIworking for compatibility. Update the parser's recognized-keyword set, theauthentication_managed_identity_is_rejectedunit test (it should become "accepted"), andmssql-odbc/docs/connection_string_parser.md.This is an intentional, documented divergence from msodbcsql in favor of correctness/UX — the same class of decision tracked in the broader parity-review follow-up (#114). Consider handling it there or as a focused change.
Affected crate
mssql-odbc (not in the dropdown; closest: Not sure / Multiple)
Alternatives considered
ActiveDirectoryManagedIdentity. Rejected — this is exactly the behavior David called a "mistake" we shouldn't perpetuate.ActiveDirectoryManagedIdentityand dropActiveDirectoryMSI: rejected — would break users/scripts already relying on the...MSIspelling. Accept both.Additional context
authentication_managed_identity_is_rejectedtest inconnection_string_parser.rs.