Commit 39dc0a7
authored
feat(kernel): wire OAuth (M2M/U2M), TLS/mTLS, and Geometry on use_kernel path (#819)
* feat(kernel): wire OAuth (M2M/U2M), TLS/mTLS, and Geometry on use_kernel path
Consume the OAuth + TLS surface added to the Rust kernel's pyo3
binding so the use_kernel backend reaches feature parity with the
Thrift/SEA backends for these flows. Bumps KERNEL_REV to the kernel
commit that ships the new Session kwargs.
OAuth (kernel-owned lifecycle):
- auth_bridge.kernel_auth_kwargs now takes the raw connect() auth
options (the OAuth secret is consumed during auth_provider
construction and isn't recoverable from the built provider, so we
read it from the original kwargs):
- OAuth M2M: oauth_client_id + oauth_client_secret are forwarded to
the kernel's auth_type='oauth-m2m'; the kernel acquires and
refreshes tokens itself via workspace OIDC client-credentials.
- OAuth U2M: auth_type 'databricks-oauth' / 'azure-oauth' route to
the kernel's auth_type='oauth-u2m' (optional oauth_client_id /
oauth_redirect_port forwarded); the kernel runs the browser flow.
- A custom credentials_provider is rejected with a clear
NotSupportedError — it's an opaque token source with no extractable
raw creds, so the kernel can't own the lifecycle. Callers are
pointed at oauth_client_id / oauth_client_secret.
- oauth_client_secret is a new connect() kwarg, honored only on the
use_kernel path; Thrift/SEA are unaffected.
- session.py forwards the raw auth kwargs into KernelDatabricksClient.
TLS / mTLS:
- client._kernel_tls_kwargs translates the connector's SSLOptions into
the kernel's tls_* Session kwargs: tls_trusted_ca_file ->
tls_ca_cert (PEM bytes), client cert/key files -> tls_client_cert /
tls_client_key (mTLS), and inverts tls_verify -> tls_skip_verify /
tls_verify_hostname -> tls_skip_hostname_verify. A password-protected
client key is rejected (the kernel has no surface for it yet).
ssl_options is no longer accept-and-ignored on this path.
Geometry:
- type_mapping recovers GEOGRAPHY / GEOMETRY result columns from the
databricks.type_name metadata (they arrive as Arrow Utf8, like
VARIANT) so PEP-249 description reports the precise type instead of
collapsing to string.
Tests: 140 kernel unit tests pass (new M2M/U2M routing, custom-provider
rejection, SSLOptions->tls_* translation incl. mTLS + encrypted-key
rejection, and geospatial type recovery). black + mypy clean.
Note: KERNEL_REV points at the kernel's feat/pyo3-oauth-tls branch HEAD
until that PR merges; re-pin to the squash-merge SHA before this lands.
Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
* chore(kernel): re-pin KERNEL_REV to updated feat/pyo3-oauth-tls HEAD
The companion kernel PR (#107) was amended with review fixes (doc
backend-inversion fix, U2M degenerate-value validation), changing its
branch HEAD SHA. Re-pin so this PR's kernel-e2e builds against the
current kernel. Still the branch HEAD — re-pin to the squash-merge SHA
once #107 lands.
Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
* fix(kernel): address review — re-pin KERNEL_REV, secret hygiene, auth ambiguity, TLS superset
Addresses @gopalldb's review on #819.
P0:
- Re-pin KERNEL_REV to the squash-merge SHA of the now-merged kernel
PR (ec2288742cbac0cd9fab50da353e8405972eefe9 on kernel main),
replacing the orphaned branch-HEAD SHA.
P1:
- Scrub oauth_client_secret from the long-lived self._auth_options in
open_session's finally (even on the failure path). It outlives the
method, so a retained secret was exposed to vars()/pickle/debugger
far longer than needed; the kernel now owns it.
- tls_verify=False now also emits tls_skip_hostname_verify=True —
no-verify subsumes hostname verification, matching SSLOptions'
create_ssl_context (check_hostname=False when tls_verify is False).
- Reject ambiguous auth combos before resolving, instead of silently
picking M2M: (a) credentials_provider + oauth_client_secret, and
(b) a U2M auth_type (databricks-oauth/azure-oauth) + oauth_client_secret.
Both raise NotSupportedError naming the conflict, so the failure is at
session-open rather than a confusing first-call 401 against the wrong
principal.
- Secret-leak tests: oauth_client_secret is forwarded to the kernel but
scrubbed from _auth_options, and absent from repr(conn) / vars(conn);
scrub runs even when open_session raises.
P2:
- _read_pem_bytes rejects an empty/whitespace CA/cert file with a clear
ProgrammingError instead of passing empty PEM to the kernel.
- _normalize_scopes raises ProgrammingError on a non-str/list/tuple
oauth_scopes rather than silently dropping to default scopes.
- Added U2M oauth_scopes forwarding test.
150 kernel unit tests pass; black + mypy clean.
Note: KERNEL_REV now points at the merged kernel main SHA; no further
re-pin needed before merge.
Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
* fix(kernel): don't build connector OAuth provider on use_kernel path + add TLS e2e
Live mitmproxy TLS e2e (this commit) surfaced a real bug: on
use_kernel=True the connector still built its own auth provider in
Session.__init__ via get_python_sql_connector_auth_provider, BEFORE
use_kernel was consulted. For OAuth that constructor eagerly runs the
flow (DatabricksOAuthProvider calls _initial_get_token in __init__), so
passing oauth_client_id for kernel-managed M2M fired the U2M *browser*
flow at connect() time — opening a browser and using the M2M service
principal id as a U2M OAuth app client_id (which the account rejects).
Fix: on use_kernel=True, do NOT build the connector's provider. Hand
the kernel auth bridge a minimal AccessTokenAuthProvider when an
access_token is present, else None; OAuth M2M/U2M resolve purely from
the raw connect() kwargs the bridge already reads, and the kernel owns
the entire token lifecycle. Thrift/SEA backends are unchanged.
- session.py: branch the provider build on use_kernel; import
AccessTokenAuthProvider; update the kernel-branch comment.
- auth_bridge.py: kernel_auth_kwargs / _is_pat / _extract_bearer_token
accept Optional[AuthProvider] (None when no token on the kernel path);
clearer "no credentials" error; refreshed module docstring.
- tests/unit/test_session.py: regression guards — use_kernel must NOT
call get_python_sql_connector_auth_provider, forwards raw M2M creds
via auth_options, and uses a minimal AccessTokenAuthProvider for PAT.
TLS e2e (the connector-side counterpart to the kernel's v0_tls_e2e.rs):
- tests/e2e/test_kernel_tls.py: 3 mitmproxy-backed tests driving
sql.connect(use_kernel=True, _tls_*=...) — strict default rejects the
re-signed cert, _tls_trusted_ca_file trusts it, _tls_no_verify
bypasses. Skips cleanly without the kernel wheel / creds /
MITMPROXY_CA_CERT. Verified live against the dogfood workspace on the
OAuth M2M path (all 3 green) — this is what caught the bug above.
- kernel-e2e.yml: run test_kernel_tls.py in the existing (label/merge-
queue-gated) run-kernel-e2e job behind a mitmproxy service. The kernel
wheel is built before the proxy is in scope so cargo's JFrog fetch
isn't routed through it; HTTPS_PROXY is scoped to the TLS test step.
Path filter now also triggers on session.py and test_kernel_tls.py.
184 unit tests pass; black + mypy clean.
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
* test(kernel): make use_kernel auth-provider guards wheel-independent
The two TestKernelAuthProviderBypass tests went through
databricks.sql.connect(use_kernel=True), which triggers the lazy
`import databricks_sql_kernel` in the kernel client module. That import
fails in the unit-test job (no Rust wheel installed), so the tests
errored with ImportError before their patches applied — green locally
(wheel present) but red in CI.
Rewrite them to exercise Session.__init__'s provider-selection logic
directly with _create_backend stubbed, so the kernel client (and its
import) is never touched. Assert on session.auth_provider and that
get_python_sql_connector_auth_provider is not called. Verified passing
both with the wheel present and with `import databricks_sql_kernel`
forcibly blocked (reproducing the CI environment).
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
---------
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>1 parent 6723350 commit 39dc0a7
11 files changed
Lines changed: 977 additions & 68 deletions
File tree
- .github/workflows
- src/databricks/sql
- backend/kernel
- tests
- e2e
- unit
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
178 | 178 | | |
179 | 179 | | |
180 | 180 | | |
181 | | - | |
| 181 | + | |
182 | 182 | | |
183 | 183 | | |
184 | 184 | | |
| |||
319 | 319 | | |
320 | 320 | | |
321 | 321 | | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
322 | 374 | | |
323 | 375 | | |
324 | 376 | | |
| |||
341 | 393 | | |
342 | 394 | | |
343 | 395 | | |
344 | | - | |
| 396 | + | |
345 | 397 | | |
346 | 398 | | |
347 | 399 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
2 | | - | |
3 | | - | |
4 | | - | |
5 | | - | |
6 | | - | |
7 | | - | |
8 | | - | |
9 | | - | |
10 | | - | |
11 | | - | |
12 | | - | |
13 | | - | |
14 | | - | |
15 | | - | |
16 | | - | |
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
17 | 39 | | |
18 | 40 | | |
19 | 41 | | |
| |||
46 | 68 | | |
47 | 69 | | |
48 | 70 | | |
49 | | - | |
| 71 | + | |
50 | 72 | | |
51 | 73 | | |
52 | 74 | | |
| |||
65 | 87 | | |
66 | 88 | | |
67 | 89 | | |
68 | | - | |
| 90 | + | |
69 | 91 | | |
70 | 92 | | |
71 | 93 | | |
72 | 94 | | |
73 | 95 | | |
74 | 96 | | |
75 | 97 | | |
76 | | - | |
77 | | - | |
78 | | - | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
79 | 101 | | |
| 102 | + | |
| 103 | + | |
80 | 104 | | |
81 | 105 | | |
82 | 106 | | |
| |||
93 | 117 | | |
94 | 118 | | |
95 | 119 | | |
96 | | - | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
97 | 124 | | |
98 | 125 | | |
99 | | - | |
100 | | - | |
101 | | - | |
102 | | - | |
103 | | - | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
104 | 159 | | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
105 | 197 | | |
106 | 198 | | |
107 | 199 | | |
| |||
111 | 203 | | |
112 | 204 | | |
113 | 205 | | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
114 | 239 | | |
115 | | - | |
116 | | - | |
117 | | - | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
118 | 268 | | |
0 commit comments