Inject OBO access token in apps run-local#5795
Conversation
f7263cb to
1c6609e
Compare
Approval status: pending
|
theof-db
left a comment
There was a problem hiding this comment.
Were you able to test this e2e locally?
yes! I was able to run it against my app locally and see the token forwarded to my app. I wrote a note about this in the how it works section |
MarioCadenas
left a comment
There was a problem hiding this comment.
Blocking: PAT / basic-auth users get a 502 on every request — run-local becomes unusable for them
setupProxy unconditionally registers the token header from w.Config.GetTokenSource(), but that only returns a working source for OAuth-based auth. For PAT (token = dapi… / DATABRICKS_TOKEN) and basic auth, GetTokenSource() returns an errorTokenSource because the credentials provider isn't an OAuthCredentialsProvider (SDK config.Config.GetTokenSource; PAT uses credentials.FromCredentials). So the per-request tokenSource.Token(ctx) errors on every request, applyInjectedHeaders propagates it, and handleHTTP returns 502 and never forwards to the app.
This is a regression — PAT users could use run-local before this PR (static headers only). Reproduced end-to-end against this branch: a request that should return 200 instead returns
502 Bad Gateway
Error resolving injected header: OAuth Token not supported for current auth type pat
Suggested fix: when the auth type has no OAuth token, either (a) skip the token injection and still forward the request, or (b) fail fast at setupProxy with one actionable error (e.g. "run-local OBO token injection requires OAuth auth; you're using PAT") — rather than a silent per-request 502 with an opaque gateway error.
Also worth a test that drives a request through setupProxy with a non-OAuth config to lock this in — the current TestSetupProxyPortInUse uses a PAT config but never issues a request, so it misses this.
| defer middlewareConn.Close() | ||
|
|
||
| // Inject headers on the upgrade request too, matching the HTTP path. | ||
| if err := p.applyInjectedHeaders(r); err != nil { |
There was a problem hiding this comment.
WebSocket 502 is written after Hijack(), so the client never sees it. applyInjectedHeaders(r) runs after hj.Hijack() + defer middlewareConn.Close(); once the connection is hijacked, http.Error(w, …) no longer reaches the client (w is detached), so on a token-resolution failure the client just gets a closed connection with no 502. Move this block to before hj.Hijack() so the error branch's http.Error still lands. (Not a leak — r.Write is correctly skipped.)
There was a problem hiding this comment.
Fixed — moved applyInjectedHeaders (and its http.Error 502 branch) to before hj.Hijack(), so on a resolution failure the error still reaches the client instead of a bare closed connection. Added TestProxyInjectHeaderFuncErrorWebSocket to lock in that the 502 lands on the upgrade path.
The deployed Apps platform fronts each slot with an OAuth2 proxy that injects the caller's on-behalf-of (OBO) token as X-Forwarded-Access-Token. `databricks apps run-local` starts a local proxy that injects the identity headers (X-Forwarded-Email, etc.) but not the token, so OBO code paths can't be exercised locally — the app sees the header as missing and every OBO route fails. Mint a token from the CLI's own credentials (the same profile already used for CurrentUser.Me) and inject it as X-Forwarded-Access-Token. The value is resolved per request via a new appproxy InjectHeaderFunc so a browser session outliving a single token still gets a refreshed one; the SDK token source returns a cached token without blocking in the steady state. A resolution failure fails the request with 502 rather than forwarding it without the header. Co-authored-by: Isaac
- run-local: probe the token source once; on non-OAuth auth (PAT/basic) log a warning and forward without X-Forwarded-Access-Token instead of 502ing every request. Restores pre-OBO behavior for those users. - appproxy: move header injection before Hijack on the WebSocket path so a resolution error's 502 still reaches the client. - tests: drive a request through setupProxy with a PAT config (no 502, header omitted); assert the WebSocket 502-before-Hijack path. Co-authored-by: Isaac
1c6609e to
d8b6636
Compare
|
@MarioCadenas good catch, and thanks for reproducing it end-to-end — you're right that this was a regression for PAT/basic-auth users. The root cause is that a PAT can't be turned into an OAuth token here. So I switched to a best-effort approach: probe the token source once at Also added the test you asked for: |
|
An authorized user can trigger integration tests manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
What did you change, and why?
databricks apps run-localstarts a local proxy that fronts the app andinjects the identity headers the deployed Apps OAuth2 proxy would
(
X-Forwarded-Email,X-Forwarded-User, ...) — but notX-Forwarded-Access-Token. That token is the on-behalf-of (OBO) credentialthe deployed platform forwards so an app can call Databricks APIs as the
calling user. Locally it is simply absent, so every OBO code path sees the
header as missing and fails, and OBO-gated surfaces can't be exercised in a
browser without deploying.
This injects
X-Forwarded-Access-Tokentoo, minted from the CLI's owncredentials (the same profile already used for
CurrentUser.Me), matchingthe deployed proxy.
libs/appproxy: addInjectHeaderFunc(key, fn)for a header whose value isresolved per request. Used for the token because it expires: the SDK token
source returns a cached token without blocking in the steady state and
refreshes on demand, so a browser session that outlives a single token keeps
working. A resolution error fails the request with 502 rather than
forwarding it without the header. Both the HTTP and WebSocket paths now go
through a shared
applyInjectedHeaders(on the WebSocket path the headersare applied before the connection is hijacked, so a resolution error's 502
still reaches the client).
cmd/apps/run_local.go: inject the token fromw.Config.GetTokenSource(),mirroring the existing idiom in
cmd/apps/logs.go. Only OAuth-based auth canmint a token this way; for PAT/basic auth
GetTokenSource()has nothing toexchange (a PAT is not a JWT assertion). Rather than 502 every request, we
probe the source once at setup and, when it can't mint a token, log a warning
and forward without the header — restoring the pre-OBO behavior for those
users. OBO paths still can't be exercised without OAuth, and the warning says
so and points at
databricks auth login.libs/apps/runlocal: export the header name as a constant.How do you know it works?
Unit tests cover the new
InjectHeaderFunc(per-request resolution, the502-on-error path for HTTP and WebSocket, and the PAT fallback that omits the
header instead of 502ing). Manually verified end-to-end against a Lakehouse
App: a request straight to the app returns 401 (OBO fails, as before), while
the same request through the run-local proxy returns 200 with OBO-backed pages
rendering as the calling user.