Skip to content

fix(appkit): surface invalid or expired user tokens as IDENTITY_EXPIRED - #714

Open
fjakobs wants to merge 2 commits into
mainfrom
fjakobs/la-2048-analytics-invalid-token-500
Open

fjakobs wants to merge 2 commits into
mainfrom
fjakobs/la-2048-analytics-invalid-token-500

Conversation

@fjakobs

@fjakobs fjakobs commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

When a forwarded user token is invalid, expired or revoked, AppKit is supposed to raise
IdentityExpiredError (HTTP 401, code IDENTITY_EXPIRED). That is the documented "reauthenticate
and retry" signal. Two bugs kept it from firing:

  1. Analytics returned 500 EXECUTION_ERROR. An on-behalf-of-user analytics query with a rejected
    token failed as a generic execution failure on every surface: the asUser(req).analytics.query()
    API, the ARROW_STREAM route and the JSON/SSE route.
  2. Databricks answers most rejected tokens with 403, not 401. Expired, revoked and unknown PATs
    get 403 {"error_code":403,"message":"Invalid access token."}. Expired OAuth tokens get a
    plain-text 403 Invalid Token. Only a 401 was recognized, so these surfaced as a generic
    403 Forbidden (or as the 500 from bug 1) and read like a permissions problem.

Root cause

  • SQLWarehouseConnector._rethrowStatementError wrapped the SDK ApiError in
    ExecutionError.statementFailed(message, code). The wrapper keeps no HTTP status and no cause,
    so normalizeIdentityError() could no longer see the 401 by the time it ran.
  • The analytics ARROW_STREAM error branch hard-coded res.status(500).
  • isUnauthorized() (used by normalizeIdentityError()) matched status 401 only.

Fix

  • connectors/sql-warehouse/client.ts: classify the raw SDK error with normalizeIdentityError()
    before wrapping. Inside a caller scope a rejected token becomes IdentityExpiredError. Outside one
    (service-principal execution) the behavior is unchanged. No upstream error is retained on the new
    error.
  • plugins/analytics/analytics.ts: the ARROW_STREAM error response now uses the AppKitError's
    own statusCode and forwards IDENTITY_EXPIRED as errorCode, matching the SSE error payload.
    As a side effect, other typed errors on this route also get their own status instead of a blanket
    500: ValidationError 400, ConnectionError 503. Execution and configuration errors stay 500.
  • errors/identity-expired.ts: isUnauthorized() also accepts a 403 whose message or HTTP body
    reports an invalid token
    (/\binvalid (access )?token\b/i). It reads the body because the SDK
    leaves message empty for the plain-text OAuth response. Permission-denied 403s and other failures
    keep their behavior.
  • Docs: execution-context.md and agents.md describe the 403 case.

SSE responses still use HTTP 200, because the stream headers are sent before the error. On that
transport the signal is errorCode: "IDENTITY_EXPIRED" in the error event, as before.

Tests

  • connectors/tests/sql-warehouse.test.ts:
    • a 401 from executeStatement → IdentityExpiredError under a caller, with fingerprint kept and
      no upstream text
    • the same while polling for the result
    • outside a caller the error stays ExecutionError (500)
  • plugins/analytics/tests/analytics.test.ts: the ARROW_STREAM route returns 401 with
    errorCode: "IDENTITY_EXPIRED".
  • errors/tests/http-status.test.ts, using the exact response bodies seen from the workspace:
    • invalid-token 403s are recognized: the JSON PAT body, the plain-text OAuth body (empty message),
      and the shared raw-request ApiError, both directly and as a cause
    • a PERMISSION_DENIED 403 and a 400 Invalid Token are not
  • context/tests/identity-lifecycle.test.ts: a 403 invalid-token error converts like a 401, and a
    permission-denied 403 is preserved.

Without the source changes, the new regression tests fail.

End-to-end verification

I ran against a real Databricks workspace using small AppKit apps in development mode
(server + analytics, and server + files with an on-behalf-of-user volume). Test tokens were
sent as x-forwarded-access-token:

Token Upstream Analytics asUser().query() Analytics ARROW_STREAM Analytics SSE errorCode asUser().run() Files route
unknown PAT (well-formed) 403 Invalid access token. 500 → 401 IDENTITY_EXPIRED 500 → 401 UNKNOWN → IDENTITY_EXPIRED 403 → 401 IDENTITY_EXPIRED 403 → 401
expired PAT 403 Invalid access token. 403 → 401 IDENTITY_EXPIRED 403 → 401
revoked PAT 403 Invalid access token. 401 IDENTITY_EXPIRED 401 IDENTITY_EXPIRED 403 → 401 IDENTITY_EXPIRED 403 → 401
expired OAuth token 403 Invalid Token (plain text) 401 IDENTITY_EXPIRED 401 IDENTITY_EXPIRED 403 → 401 IDENTITY_EXPIRED 403 → 401
unsupported credential 401 500 → 401 IDENTITY_EXPIRED 500 → 401 UNKNOWN → IDENTITY_EXPIRED 401 (unchanged) 401 (unchanged)
valid token 200 rows (unchanged) Arrow bytes (unchanged) result (unchanged) 200 (unchanged) 200/404 (unchanged)
OAuth JWT with a tampered signature 400 Invalid Token 400 (unchanged, intentionally) 400 (unchanged)

In each cell, a → b means I measured both before and after the change; a single value means I measured it after only. The
expired OAuth row covers the case where the SDK error has an empty message and only the response
body says the token is invalid. That is Databricks' response for an expired OAuth token, which is
the token type a deployed app forwards. I tested it with a workspace OAuth token rather than one
forwarded by a deployed app.

The token never appears in the server logs.

Checks: pnpm build, pnpm docs:build, pnpm check:fix and pnpm -r typecheck pass locally.
pnpm test passes locally (309 files, 5933 tests). I ran it with a raised per-test timeout: the
local machine was heavily loaded, and a few tests that boot a test HTTP server exceeded the default
5s. No assertion failed.

Not in this PR

  • Genie routes turn every upstream error into a generic SSE error event, so they never
    surface IDENTITY_EXPIRED.
  • Files plugin HTTP routes return the converted error as a bare 401 {"error":"Unauthorized"},
    without the IDENTITY_EXPIRED code.
  • A real permission-denied 403 (for example, missing CAN_USE on the warehouse) is deliberately
    not treated as identity expiry. From analytics it still becomes a 500 EXECUTION_ERROR, as before.
    I didn't reproduce this case.

This pull request and its description were written by Isaac.

…PIRED

An analytics query with an invalid or expired user token returned a
500 EXECUTION_ERROR. The SQL warehouse connector wrapped the SDK
ApiError in ExecutionError.statementFailed(), which drops the HTTP
status, so normalizeIdentityError() could no longer recognize the 401
and never produced IdentityExpiredError.

- Classify the raw SDK error with normalizeIdentityError() in
  _rethrowStatementError before wrapping. Inside a caller scope a 401
  now becomes IdentityExpiredError (401, IDENTITY_EXPIRED). The
  service-principal lane is unchanged.
- The ARROW_STREAM route returned a hard-coded 500. It now uses the
  AppKitError's own statusCode and forwards IDENTITY_EXPIRED as
  errorCode, matching the SSE error payload.

Databricks answers "Invalid access token" with a 403; the next commit
recognizes that as identity expiry. This change passes the raw error to
the shared classifier, so that fix applies here too.

Co-authored-by: Isaac <no-reply@databricks.com>
Signed-off-by: Fabian Jakobs <fabian.jakobs@databricks.com>
Databricks rejects expired, revoked, and malformed PATs with
403 {"error_code":403,"message":"Invalid access token."} and expired OAuth
tokens with a plain-text 403 "Invalid Token", not 401. isUnauthorized only
matched 401, so these never became IdentityExpiredError / IDENTITY_EXPIRED
and surfaced as a generic 403 Forbidden instead of a reauthenticate signal.

Also accept a 403 whose message or (sdk-core) HTTP body reports an invalid
token. Permission-denied 403s and other failures keep their behavior.

Co-authored-by: Isaac <no-reply@databricks.com>
Signed-off-by: Fabian Jakobs <fabian.jakobs@databricks.com>
@fjakobs
fjakobs requested a review from a team as a code owner November 6, 2026 15:26
@fjakobs
fjakobs requested a review from MarioCadenas November 6, 2026 15:26
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

📦 Bundle size report

Compared against bundle-size-baseline.json (main).

@databricks/appkit

npm tarball (packed): 1.2 MB (+1008 B) — gzipped download (dist + bin; excludes release-only docs/NOTICE).

dist raw gzip
JS (runtime) 1.3 MB (+1005 B) 460 KB (+354 B)
Type declarations 465 KB 169 KB (+1 B)
Source maps 2.5 MB (+2.0 KB) 860 KB (+759 B)
Other 11 KB 3.7 KB
Total 4.2 MB (+3.0 KB) 1.5 MB (+1.1 KB)
Per-entry composition (own code — deps external (as shipped))
Entry Initial (gz) Lazy (gz) Total (gz) node_modules (min) Own code (min)
. 106 KB (+125 B) 2.5 KB 109 KB (+125 B) external 345 KB (+322 B)
./beta 103 KB (+122 B) 485 B (+3 B) 104 KB (+125 B) external 310 KB (+269 B)
./testing 45 KB (+90 B) 32 KB (-1 B) 77 KB (+89 B) external 224 KB (+242 B)
./tsdown 520 B 0 B 520 B external 813 B
./type-generator 26 KB 0 B 26 KB external 74 KB

Chunks:

Entry Chunk Load Size (gz)
. index.js initial 102 KB
. utils.js initial 4.6 KB
. remote-tunnel-manager.js lazy 2.5 KB
./beta beta.js initial 83 KB
./beta modular.js initial 6.0 KB
./beta stream-manager.js initial 5.9 KB
./beta service-context.js initial 4.3 KB
./beta databricks.js initial 3.3 KB
./beta client.js initial 647 B
./beta index.js initial 20 B
./beta supervisor-api.js lazy 192 B
./beta databricks.js lazy 178 B
./beta index.js lazy 115 B
./testing manifest.js initial 32 KB
./testing index.js initial 10 KB
./testing wide-event-emitter.js initial 2.9 KB
./testing index.js lazy 28 KB
./testing remote-tunnel-manager.js lazy 2.5 KB
./testing utils.js lazy 1.8 KB
./tsdown index.js initial 520 B
./type-generator index.js initial 26 KB

@databricks/appkit-ui

npm tarball (packed): 397 KB — gzipped download (dist + bin; excludes release-only docs/NOTICE).

dist raw gzip
JS (runtime) 429 KB 147 KB
Type declarations 267 KB 98 KB
Source maps 853 KB 286 KB
CSS 16 KB 3.2 KB
Total 1.5 MB 534 KB
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
Entry Initial (gz) Lazy (gz) Total (gz) node_modules (min) Own code (min)
./js 5.3 KB 49 KB 55 KB 208 KB 14 KB
./js/beta 2.1 KB 0 B 2.1 KB 0 B 5.0 KB
./react 433 KB 49 KB 481 KB 1.3 MB 177 KB
./react/beta 4.9 KB 0 B 4.9 KB 0 B 12 KB

Chunks:

Entry Chunk Load Size (gz)
./js index.js initial 5.2 KB
./js chunk initial 120 B
./js apache-arrow lazy 49 KB
./js/beta beta.js initial 2.1 KB
./react index.js initial 431 KB
./react tslib initial 2.1 KB
./react apache-arrow lazy 49 KB
./react/beta beta.js initial 4.9 KB

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

🤖 AppKit PR bot

🔬 Run evals

Start an eval for this PR from the evals-monitor app: Go to Evals Monitor →

📦 Try this PR's app template

Scaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh auth login — and the Databricks CLI):

gh run download 42243086321 -R databricks/appkit -n appkit-template-0.90.0-pr.2be54de-fjakobs-la-2048-analytics-invalid-token-500-714 -D appkit-pr-714 \
  && unzip -o "appkit-pr-714/appkit-template-0.90.0-pr.2be54de-fjakobs-la-2048-analytics-invalid-token-500-714.zip" -d "appkit-pr-714" \
  && databricks apps init --template "appkit-pr-714"

The template pins @databricks/appkit and @databricks/appkit-ui to tarballs built from this branch, so the scaffolded app runs against this PR's code.

This branch has not been deployed

No deployments
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.

2 participants