feat(auth): negotiate DICOMweb authorization per server at runtime

[fedorov]

dmv-branch:

Context

With an oidc block configured, Slim attached the OIDC access token as an
Authorization header to every DICOMweb request, regardless of whether the
server wanted one. applyAuthorization fanned the token out to every store
unconditionally.

Two consequences:

1. The token is disclosed to servers that have no business seeing it —
   including any URL typed into the server-selection dialog, which previously
   copied Authorization from the default client onto the new endpoint and then
   re-applied a fresh token on top.
2. Authorization is not a CORS-safelisted request header, so every request
   is preceded by an OPTIONS preflight. Public DICOMweb endpoints that do not
   answer preflights correctly then fail with an opaque CORS error, even though
   plain GETs would have succeeded.

This surfaced while testing WG-26 connectathon endpoints, where most servers are
open and the app is pointed at new ones at runtime.

Changes & Results

Requests now start anonymous and escalate only when a server actually asks.

Step	Behavior
1. First request	Safelisted headers only — no preflight, no token. An open server answers 200 and never sees the credential.
2. On 401/403	Escalate. A server listed in servers is credentialed silently (the operator vouched for it by putting it there). A server introduced at runtime — selection dialog or ?gcp= — prompts the user first.
3. Answer	Remembered per origin in localStorage, so each server is negotiated once per browser, not once per session.

The consent prompt exists because escalation is triggered by the server.
Without it, any endpoint could obtain a live cloud credential simply by replying
401 to an anonymous request — a realistic risk for a URL pasted into the
selector.

Because this is negotiated at runtime, adding or swapping endpoints needs no
redeployment. That matters for the two paths that never appear in the config
file at all: the server-selection dialog and the ?gcp= parameter.

Verified against the servers that matter:

• GCP Healthcare returns 401 with access-control-allow-origin set and
  www-authenticate: Bearer listed in access-control-expose-headers, so the
  challenge is readable from JS and escalation works.
• An open DICOMweb proxy returns 200 anonymously and never reaches step 2.

Override

servers[].sendAuthorization bypasses the negotiation:

• false — never send the token, even if the server returns 401.
• true — send from the first request, skipping the anonymous attempt.

true is needed for a server that answers 200 with fewer results rather than
401 when unauthenticated. Runtime detection cannot distinguish that from an
open server, and Slim would otherwise silently under-report studies. This is the
one case the runtime approach cannot handle.

Files

• src/utils/authPolicy.ts (new) — per-origin decisions in localStorage,
  guarded against private-browsing failures.
• src/DicomWebManager.ts — tri-state auth mode per store; callStore performs
  the challenge-and-retry; per-origin in-flight deduplication so N parallel 401s
  produce one prompt rather than N; error-interceptor suppression so an expected
  401 does not fire a spurious sign-in redirect.
• src/App.tsx — policy implementation and consent modal; the selection dialog
  no longer forwards the token to typed URLs.
• src/AppConfig.d.ts — sendAuthorization documented as an override.
• docs/CONFIGURATION.md — new "How the access token is sent to servers" section.

The retry is capped at one attempt and fires only after a 401/403, so nothing was
stored or returned on the first attempt — safe even for STOW-RS.

Testing

$ pnpm test          # 110 passed, 12 suites
$ pnpm run typecheck # no new errors (pre-existing @types/lodash gaps remain)
$ pnpm run lint

13 new unit tests in src/__tests__/DicomWebManager.test.ts cover: withholding
until challenged, escalation on 401 and on 403, refusal leaving the store
anonymous, sendAuthorization: false never escalating, one prompt for
concurrent challenges, 404 not being treated as a challenge, and pre-authorized
origins being credentialed from the first request.

Manual verification:

1. REACT_APP_CONFIG=gcp pnpm start — sign in, confirm the worklist loads. The
   first request 401s and escalates silently; subsequent loads carry the token
   from the start.
2. Enable enableServerSelection, paste an open DICOMweb URL — confirm in
   DevTools that no OPTIONS preflight is sent and no Authorization header
   appears.
3. Paste a URL for a server that requires auth — confirm the consent prompt
   appears once, and that declining leaves requests anonymous.

Checklist

PR

• PR title is descriptive and follows semantic-release style
• Linked related issues / DMV PRs when applicable

Code

• Code follows project style (pnpm run lint, pnpm run fmt)
• Non-obvious logic is documented (JSDoc where appropriate)

Docs

• README / CONFIGURATION / CONTRIBUTING updated when behavior changes

Tested environment

• OS: macOS 15
• Node version:
• Browser:

🤖 Generated with Claude Code

[deepsource-io]

DeepSource Code Review

We reviewed changes in ff3080c...98f8600 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade

Security   Reliability   Complexity   Hygiene

Code Review Summary

Analyzer	Status	Updated (UTC)	Details
JavaScript		Aug 19, 2026 4:53p.m.	Review ↗

---

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

[github-actions]

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version	^0.48.24

To link a DMV branch for testing (requires an open PR in DMV):

• Add dmv-branch: <branch-name> to the PR description, OR
• Use the same branch name in both repos (automatic matching)

[github-actions]

Visit the preview URL for this PR (updated for commit 98f8600):

https://idc-external-006--pr427-oidc-selector-9d3osx74.web.app

_{(expires Wed, 26 Aug 2026 16:56:10 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 88aacecd98ba54d2f9c8d201a9444e43d1ad8307}

[Copilot]

Pull request overview

Implements runtime, per-origin negotiation of whether to send the user’s OIDC access token on DICOMweb requests, defaulting to anonymous requests and escalating only after a 401/403 challenge (with persisted per-origin decisions).

Changes:

• Added authPolicy utilities to persist per-origin grant/deny decisions in localStorage.
• Updated DicomWebManager to support tri-state auth mode per store (always/never/auto) and to perform challenge-and-retry with per-origin in-flight deduplication.
• Integrated an app-level authorization policy + consent modal, and stopped forwarding Authorization to user-typed server URLs by default; updated docs and types accordingly.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/utils/authPolicy.ts	Adds origin parsing and localStorage-backed persistence for per-origin auth disclosure decisions.
src/DicomWebManager.ts	Introduces per-store auth modes, challenge detection, consent-driven escalation, and header filtering.
src/AppConfig.d.ts	Documents servers[].sendAuthorization override semantics (always/never vs runtime negotiation).
src/App.tsx	Implements policy wiring + consent modal; prevents forwarding tokens to typed URLs; applies policy to managers.
src/tests/DicomWebManager.test.ts	Adds unit tests for escalation behavior, refusal behavior, overrides, and concurrency deduplication.
docs/CONFIGURATION.md	Documents the new runtime negotiation flow and sendAuthorization override in configuration docs.

Suppressed comments (1)

src/DicomWebManager.ts:476

• After a grant, callStore sets the Authorization header only on the challenged store (store.client.headers.Authorization = ...). If multiple stores share the same origin (or other stores are already pre-authorized), they’ll remain anonymous until they individually 401 or until some later updateHeaders() call, causing extra round-trips/retries. Consider reusing updateHeaders({ Authorization }) here so all eligible stores pick up the token immediately (while still respecting per-store filtering).

      store.authGranted = true
      this.currentAuthorization = authorization
      store.client.headers.Authorization = authorization
      return await call(store.client)

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[fedorov]

Confirmed and fixed in dbcac10.

The rewrite is serverSettings.url = pathUrl in _createClientMapping (src/App.tsx:118), which mutates the config object in place, so the snapshot taken beforehand held the pre-rewrite origin. On a /projects/ route the effective origin comes from gcpBaseUrl, so any deployment whose configured URL had a different origin would classify its own server as runtime/untrusted and prompt for consent — for a server the operator had listed all along.

Took the approach you described: configuredServers now holds references (not copies) taken before addGcpSecondaryAnnotationServer, and the origins are read from them after _createClientMapping has run. Since the objects are shared, reading .url afterwards picks up the rewrite, and the ?gcp= server is still excluded because it is appended after the snapshot.

[fedorov]

Good catch — confirmed and fixed in dbcac10.

The path was real: handleDICOMwebError (src/App.tsx:203) calls ensureAuthorized() on any 401, which renews the session and can escalate to an interactive redirect to the IdP. So clicking "Don't send" would bounce the user through a sign-in that could not have helped — the request failed because the credential was withheld, not because it was stale.

Worse, the original guard suppressed the interceptor only while a store was still an escalation candidate. Once authRefused was set, every subsequent 401 from that server fell through to the general handler, so the redirect would fire repeatedly for the rest of the session.

Both are addressed by splitting the predicate:

• isAnonymousChallenge — any 401/403 against a store we deliberately queried without credentials (auto not yet granted, already refused, or sendAuthorization: false). The error interceptor now suppresses all of these, so none reach the expired-token recovery.
• isAuthChallenge — the subset that can still be escalated from.

Withheld-credential failures get their own NotificationMiddleware message naming the origin, reported once per store so a refused server does not spam on every subsequent request. A 401 against a store that did send its token is untouched and still drives re-authentication as before.

Four regression tests added, including the sendAuthorization: false variant of the same path and an assertion that consent is requested only once no matter how many requests follow a refusal.

[github-actions]

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version	^0.48.24

To link a DMV branch for testing (requires an open PR in DMV):

• Add dmv-branch: <branch-name> to the PR description, OR
• Use the same branch name in both repos (automatic matching)

[Copilot]

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (7)

src/tests/DicomWebManager.test.ts:627

• As above, this onError assertion is vacuous because the replacement stub never calls the real client's errorInterceptor. Please make the stub execute the interceptor for rejected requests so the sendAuthorization: false suppression path is actually covered.

    expect(onError).not.toHaveBeenCalled()

src/DicomWebManager.ts:427

• This predicate uses the store's current grant state rather than the state of the failed request. If two calls start anonymously and one challenge grants access before a slower 401/403 reaches the interceptor, authGranted is already true: that anonymous failure is routed through expired-token recovery, and callStore also skips its authenticated retry. Capture whether each call actually dispatched with authorization and classify its failure from that request-local value; the interceptor and retry path need to share that classification.

      status !== undefined &&
      AUTH_CHALLENGE_STATUSES.has(status) &&
      store.authMode !== 'always' &&
      !store.authGranted
    )

src/App.tsx:390

• The in-flight deduplication is local to each DicomWebManager, but _createClientMapping creates distinct managers per storage class containing the same runtime ?gcp= origin. Concurrent challenges from two managers can both reach this prompt before either writes localStorage, producing multiple dialogs (and potentially racing opposite persisted answers). Deduplicate requestAuthorization in this shared application policy by origin so all managers share one pending decision.

      if (remembered !== 'granted' && !this.configuredOrigins.has(origin)) {
        const approved = await App.confirmAuthorizationDisclosure(origin)
        writeAuthorizationDecision(origin, approved ? 'granted' : 'denied')

src/tests/DicomWebManager.test.ts:578

• This assertion does not exercise the configured errorInterceptor: stubManagerClients replaces the real DICOMweb client with a plain stub, so rejecting searchForStudies never invokes the interceptor or onError. The test would still pass if anonymous 401s incorrectly triggered reauthentication. Preserve or explicitly invoke the captured interceptor in the test double before asserting this behavior.

This issue also appears on line 627 of the same file.

    expect(onError).not.toHaveBeenCalled()

src/App.tsx:478

• README.md:161 still states that custom server selections re-apply the current authorization when OIDC is enabled, which is now the opposite of this behavior. Update that runtime-selection documentation so users are not told that typed endpoints receive the token immediately.

     * Carry over non-credential headers only. The token is deliberately not
     * forwarded here: this URL was typed by the user and has not been vetted by
     * anyone. If the server actually needs credentials it will answer 401, and
     * the authorization policy will ask before anything is disclosed.

docs/CONFIGURATION.md:78

• Omitting Authorization does not guarantee a CORS-simple request or no preflight. For example, this project adds a non-safelisted Content-Security-Policy header when upgradeInsecureRequests is enabled, and STOW uses a non-safelisted content type. Rephrase this as avoiding the preflight caused by Authorization when the remaining request is otherwise CORS-safelisted.

1. **Try anonymously.** The first request carries only safelisted headers, so
   the browser sends no `OPTIONS` preflight. An open server answers `200` and
   the exchange ends here — it never sees your token.

src/utils/authPolicy.ts:66

• The new persistent authorization boundary has no direct tests, although comparable utilities under src/utils are unit-tested. Add coverage for per-origin grant/denial round trips, malformed stored data, and localStorage read/write exceptions; regressions here can either disclose a token unexpectedly or repeatedly prompt users.

export const readAuthorizationDecision = (
  origin: string,
): AuthorizationDecision | undefined => {
  const decision = readPolicy()[origin]
  return decision === 'granted' || decision === 'denied' ? decision : undefined

[igoroctaviano]

Code Review

The implementation correctly matches the PR description and is well-designed. The anonymous-first approach properly solves both the token disclosure and CORS preflight issues. Tests are comprehensive and the security model is sound.

Potential Hardening Opportunities

A few areas that could be improved in this PR or a follow-up:

1. HTTPS enforcement/warning

Currently no check if credentials are about to be sent over plain HTTP:

// In confirmAuthorizationDisclosure or requestAuthorization
if (new URL(origin).protocol !== 'https:') {
  // Warn or block - sending tokens over HTTP is risky
}

2. Consent revocation UI

clearAuthorizationDecisions() exists but there's no UI to use it. Users can't revoke consent without manually clearing localStorage. Consider adding a settings option.

3. Session vs persistent storage

Decisions persist in localStorage indefinitely (survives browser restarts). If a user grants consent on a shared computer, that consent remains. Consider:

• sessionStorage instead (per-session only)
• Expiry timestamps on decisions
• Or document this as intentional behavior

4. More informative consent modal

The modal could be more specific about what access the token provides (e.g., naming the identity provider).

5. Audit logging

For debugging/security review, log authorization decisions with timestamps.

[fedorov]

Thanks @igoroctaviano — three of these are in as of 27ea6a1, and I'd like to push back mildly on one of the framings for the other two.

1. HTTPS enforcement — done, as a hard refusal

Agreed this was a real gap. I went further than a warning: escalation to a non-HTTPS origin is now refused outright, with no prompt offered. A bearer token in cleartext is readable by anything on the path, and a consent dialog doesn't change that — offering the choice mostly launders the risk onto the user.

Loopback hosts (localhost, *.localhost, 127.0.0.1, ::1) are exempt, matching the browser's own secure-context definition, so local development is unaffected.

Worth noting the blast radius is narrower than it first looks: a Slim instance served over HTTPS can't reach an http:// endpoint at all, because mixed content blocks it. The case this actually protects is Slim served over plain HTTP on an intranet, which is exactly where someone is most likely to have an http:// DICOMweb server sitting next to it.

An operator who needs it anyway can still force the token with sendAuthorization: true, which bypasses negotiation entirely — explicit operator intent, recorded in the config file, rather than a click-through.

3. Storage lifetime — grants now expire, denials don't

Good catch on the shared-computer case. The precise risk is worth stating: consent isn't itself a credential, so a leftover decision grants nothing on its own. But the next user signs in as themselves, and the stale approval then silently applies to their token. So the exposure is real, just indirect.

Grants now carry a 30-day expiry. Denials are kept indefinitely, and the asymmetry is deliberate — forgetting a grant costs one extra prompt, whereas forgetting a denial silently widens what the token is disclosed to. Fail-closed is the safe direction to be wrong in.

I considered sessionStorage and decided against it. It only affects typed-in servers (configured origins never prompt), so the cost lands entirely on the connectathon-style workflow of returning to the same handful of endpoints day after day — and it would make the prompt frequent enough to train people to click through it, which is worse than the thing it fixes.

4. Consent modal — done

Fair point that "your access token" doesn't tell anyone what's at stake; the answer differs a lot between a hospital SSO and a personal Google account. The modal now names the issuer from oidc.authority and states plainly that whoever holds the token can act as the user against that provider for as long as it's valid.

5. Audit logging — already there, now more specific

Both paths already logged. I've made the lines say why a decision was reached, which is the part that was actually missing:

approved disclosure of access token to https://example.org (user decision)
approved disclosure of access token to https://example.org (origin present in the deployed configuration)
refusing to send access token to http://example.org over an insecure connection; ...

I left out explicit timestamps — devtools timestamps console output already, and the project's logger config gates verbosity — but say the word if you want these routed through NotificationMiddleware or the configured logger instead of console, which would make them capturable.

2. Consent revocation UI — deferring, and I think that's the right call

This is the one I'd rather not do here. Slim has no existing "site permissions" surface, so doing it properly means deciding where it lives (header menu? settings modal?), how the list of remembered origins is presented, and whether individual origins can be revoked or only the whole set. That's a UI design conversation, not a line of code, and folding it into an auth-negotiation PR would bury it.

I've documented the manual path in docs/CONFIGURATION.md (clear the slim_authorization_policy key) and clearAuthorizationDecisions() is exported and ready for whatever UI we land on. Happy to open a follow-up issue with the above as the starting point — say the word and I'll file it.

---

Full suite: 129 passing across 13 suites. New src/__tests__/authPolicy.test.ts covers grant expiry, denial persistence, the loopback exemptions, plain-HTTP rejection, and corrupted-storage handling.

[github-actions]

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version	^0.48.24

To link a DMV branch for testing (requires an open PR in DMV):

• Add dmv-branch: <branch-name> to the PR description, OR
• Use the same branch name in both repos (automatic matching)

[fedorov]

Picking up the suppressed comment on callStore — fixed in 94da073.

The observation was correct: after a grant, the header was written directly onto the challenged store's client, so any other store the policy already permitted stayed anonymous until it was individually refused. Applying through updateHeaders instead re-evaluates every store against the policy, which is the right choice because the grant is recorded per origin, not per store — so a sibling on the same origin is already permitted the moment the first one is approved.

Per-store filtering is unchanged, and the two boundaries that matter are now covered by tests:

• a store on an origin that was never approved stays anonymous;
• a store configured sendAuthorization: false stays anonymous even when it shares an origin with the store that was just granted, since an explicit operator override outranks a runtime grant.

While fixing it I found the same gap one level up, which the comment didn't reach: each storage class gets its own DicomWebManager, so a grant discovered by one manager left the others to rediscover it independently. That cost no extra consent prompt — the decision is persisted, so requestAuthorization returns immediately — but it did cost a wasted 401 per manager. The policy now pushes the token across all managers once an origin is approved, and each re-applies its own filtering, so this cannot widen disclosure beyond what was actually granted.

Three tests added; full suite is 132 passing across 13 suites.

[github-actions]

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version	^0.48.24

To link a DMV branch for testing (requires an open PR in DMV):

• Add dmv-branch: <branch-name> to the PR description, OR
• Use the same branch name in both repos (automatic matching)

[github-actions]

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version	^0.48.24

To link a DMV branch for testing (requires an open PR in DMV):

• Add dmv-branch: <branch-name> to the PR description, OR
• Use the same branch name in both repos (automatic matching)

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

📦 Firebase Preview - Using Published DMV

This preview is using the published dicom-microscopy-viewer from package.json:

Version	^0.48.24

To link a DMV branch for testing (requires an open PR in DMV):

• Add dmv-branch: <branch-name> to the PR description, OR
• Use the same branch name in both repos (automatic matching)

[fedorov]

@igoroctaviano I think it is ready for you - if looks good, please merge!