Skip to content

feat(sqlserver): Part 3 — SQL login auth and TLS connection args - #13

Open
axellpadilla wants to merge 1 commit into
part-2-sqlserver-db-configfrom
part-3-sqlserver-auth
Open

feat(sqlserver): Part 3 — SQL login auth and TLS connection args#13
axellpadilla wants to merge 1 commit into
part-2-sqlserver-db-configfrom
part-3-sqlserver-auth

Conversation

@axellpadilla

Copy link
Copy Markdown
Collaborator

Part 3 of the SQL Server Fusion adapter series. Part of dbt-labs#15714.

Stacked on #12 (Part 2). Base is part-2-sqlserver-db-config, so this diff shows only Part 3's one file. Merge #11#12 first; this retargets automatically as each lands.

SQLServerAuth reached the driver with Entra flows only, and built a URI carrying just host, port and database. A native SQL Server login — the default in every dbt-sqlserver v1 profile — had no path through it, and encrypt / trust_cert / login_timeout were parsed into SqlServerDbConfig in Part 2 and then dropped.

SqlLogin

A new top-level SQLServerAuthIR variant rather than a refinement of an existing one: a server-local login is a different authentication contract from a federated Entra token, not a subtype of it (this crate's AGENTS.md, invariant 5 — horizontal growth). It sets user id and password with no fedauth, reading UID/PWD exactly as ActiveDirectoryPassword does. authentication: sql selects it, case-insensitively, alongside the existing serviceprincipal normalization.

Query pairs rather than URI userinfo, which is how v1's ADBC backend writes it. Both work with go-mssqldb, and the pair form matches the three variants already in the file — and append_pair percent-encodes, which closes the user/password encoding gap v1 handles by hand.

Connection args

encrypt, TrustServerCertificate and connection timeout now reach the URI. Values and defaults are ported from v1's build_adbc_connection_uri (sqlserver_backend.py), which builds the same query string against the same driver, per dbt-msft/dbt-sqlserver#783: encrypt=true, trust_cert=false, login_timeout omitted when it is not positive.

Both flags accept a YAML boolean or its string spelling, following the secure handling in clickhouse/mod.rs. An unrecognized value falls back to the default rather than erroring; in both cases the default is the safe direction (encrypted, certificate verified).

Live measurement

Against SQL Server 2022 in the project's own container, through the same go-mssqldb ADBC driver v1.6.0 dbt installs, driving it with the URI shape this PR produces:

result
authentication: sql connects as the login; ! in the password survives the encoding
connection timeout=30 accepted
encrypt=true, TrustServerCertificate=false TLS Handshake failed: … doesn't contain any IP SANs

That last row is the point of the parameters: without them an on-prem instance with a self-signed certificate cannot be reached, and there is no way to say so in the profile.

The same session also re-measured what dbt-msft/dbt-sqlserver#795 found, and one thing it didn't:

QUOTED_IDENTIFIER = 1    ANSI_NULLS = 1    CONCAT_NULL_YIELDS_NULL = 1
@@OPTIONS & 16384 = 0    -- XACT_ABORT OFF

Not in this PR

SET QUOTED_IDENTIFIER ON connection-init SQL (the third item on #3). Two findings move it out of scope here:

  1. It is already on — measured above, matching Option to specify csv file from seeds config dbt-labs/dbt-core#795's conclusion that the equivalent v1 issue item "needs no code change". The setting v1 does issue on connect is SET XACT_ABORT ON (sqlserver_connections.py _apply_session_settings, Evaluate SET XACT_ABORT ON for SQL Server DML refresh transactions dbt-msft/dbt-sqlserver#718), and that one is measurably off by default. Several v1 macros are written assuming it is on — create.sql, table_dml_refresh.sql, indexes.sql all say so in comments.
  2. There is nowhere to put it yet. dbt-core has no per-connection init hook: AdbcEngine::new_connection_with_config goes straight to connection::Builder::default().build(). The one precedent, apply_duckdb_init_sql, runs once per database on a throwaway connection — which cannot carry a SQL Server session setting, since every connection is its own session.

So a dbt-auth/src/sqlserver/init.rs landed here would be dead code with no call site. It belongs with the dbt-adapter work in Parts 4–7, alongside the hook that executes it, and it should probably be about XACT_ABORT rather than QUOTED_IDENTIFIER. Filed as a follow-up rather than stubbed.

Named instances. host: myserver\SQLEXPRESS fails at URI parse — invalid domain character — because the url crate rejects a backslash in the host. v1 handles this by omitting the port and letting the SQL Server Browser resolve it. Percent-encoding to %5C parses on our side, but whether go-mssqldb decodes it back is not something this checkout can verify, and shipping an unverified host rewrite is worse than the loud error we have today. Left as a TODO at the parse site.

DEFAULT_AUTH is unchanged. v1 defaults authentication to sql; this crate defaults it to ActiveDirectoryServicePrincipal. Changing it would also change Fabric, which maps to the same backend (AdapterType::Fabric => Backend::SQLServer) and shares this module — and auth_for_backend receives only a Backend, so the two cannot be told apart here. Until that is resolved, a ported v1 profile that omits authentication gets a "client_id is required" error rather than a SQL login. Loud, but not what a migrating user expects.

Tests

Ten new tests in sqlserver::tests: SQL login and its case-insensitive spelling, missing-credential rejection, password encoding, TLS defaults, TLS flags from YAML booleans and from strings, and login_timeout applied and omitted. cargo nextest run -p dbt-auth is 287 passed / 0 failed; cargo fmt --check and cargo clippy --all-targets are clean.

crates/dbt-auth/AGENTS.md asks for a specific report on changes to this crate:

  • Borrowed fields made owned: none. SqlLogin holds &'a str; the only allocation is login_timeout.to_string() at the URI boundary.
  • get_str / get_string behavior: no existing call changed. New reads use require_str for UID/PWD (matching ActiveDirectoryPassword) and get_string for encrypt / trust_cert / login_timeout, so native YAML and string forms both work.
  • Accepted input shapes: widened only. authentication: sql previously errored; no previously-valid value now fails.
  • Enum structure: one new top-level variant, classified above.
  • Normalization: no existing value is coerced differently.

It also points at live smoke tests in crates/dbt-auth-tests, which does not exist in this checkout — the live measurements above are the substitute.

Closes #3

`SQLServerAuth` reached the driver with Entra flows only, and built a URI
carrying just host, port and database. A native SQL Server login -- the
default in every dbt-sqlserver v1 profile -- had no path through it, and
`encrypt` / `trust_cert` / `login_timeout` were parsed into the profile
struct and then dropped.

`SqlLogin` is a new top-level `SQLServerAuthIR` variant rather than a
refinement of an existing one: a server-local login is a different
authentication contract from a federated Entra token, not a subtype of it.
It sets `user id` and `password` with no `fedauth`, reading `UID` and `PWD`
the same way `ActiveDirectoryPassword` does. `authentication: sql` selects
it, case-insensitively, alongside the existing `serviceprincipal`
normalization.

`apply_connection_args` now emits `encrypt`, `TrustServerCertificate` and
`connection timeout`. Values and defaults come from v1's
`build_adbc_connection_uri`, which builds the same query string against the
same driver: `encrypt=true`, `trust_cert=false`, and `login_timeout`
omitted when it is not positive. Both flags accept a YAML boolean or its
string spelling, and an unrecognized value falls back to the default rather
than erroring -- in both cases the default is the safe direction.

Measured against a live SQL Server 2022 through the same go-mssqldb ADBC
driver v1.6.0 that dbt installs, using the URI shape this produces:

- the SQL login connects, and a `!` in the password survives the query-pair
  encoding
- `connection timeout=30` is accepted
- `encrypt=true` with `TrustServerCertificate=false` fails the handshake
  against a self-signed certificate, which is what these parameters exist
  to let an on-prem profile opt out of

Ten unit tests cover the new paths. `DEFAULT_AUTH` is unchanged: it is
shared with Fabric, which maps to the same backend.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
axellpadilla added a commit to dbt-sqlserver-next/dbt-sqlserver-v2-roadmap that referenced this pull request Aug 2, 2026
…pter

Part 3 (dbt-sqlserver-next/dbt-core#13) landed SQL login auth and the
`encrypt` / `TrustServerCertificate` / `connection timeout` params. Two of
§5.4's remaining items turned out to be stated wrongly, both checkable:

`src/init.rs` cannot live in `dbt-auth`. dbt-core has no per-connection
init hook — `AdbcEngine::new_connection_with_config` goes straight to
`connection::Builder::default().build()` — and the one precedent,
`apply_duckdb_init_sql`, runs once per database on a throwaway connection,
which cannot carry a SQL Server session setting. The file would have no
call site. It belongs in dbt-adapter next to the hook, so §5.4 and `05` #1
both now say so.

And it should not be about `QUOTED_IDENTIFIER`. Measured through the same
go-mssqldb ADBC driver v1.6.0 dbt installs, against `make server`:
QUOTED_IDENTIFIER, ANSI_NULLS and CONCAT_NULL_YIELDS_NULL are all on, and
`@@options & 16384 = 0` — XACT_ABORT is off. That reproduces #795's
finding and identifies the setting that actually differs from v1, whose
macro bodies say in comments that they rely on it.

Two smaller corrections: the plan told the implementer to write SQL auth
into the URI userinfo, which would have skipped the percent-encoding the
existing query-pair form gets for free; and it did not mention that
`DEFAULT_AUTH` can't be changed to v1's `sql` default without changing
Fabric, which shares the module through `Backend::SQLServer`.

Named instances are recorded as still broken rather than fixed: the `url`
crate rejects a backslash in a host, and nothing here can confirm
go-mssqldb decodes `%5C` back.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant