Skip to content

Commit af17725

Browse files
committed
fix(shl): derive the access document per manifest fetch, not once at mint
The completeness fix in 9c9b19b could not reach a single link already in circulation, and this is why: encryptSHLFile runs at mint, the ciphertext is stored on the session, and the manifest endpoint replays it verbatim (`embedded: entry.jwe`). Whatever the document claimed on the day it was created, it claims forever. A link minted before that fix still tells its recipient Complete summary — the patient shared their full health record over a view where the proxy denies all but Patient, ImagingStudy and metadata. Nothing required the snapshot: the session already holds studyInstanceUID, shareScope, patientId, sessionToken, expiresAt and the SHL key, so the manifest can state what is true when asked. It now rebuilds and re-encrypts the document on each fetch, falling back to the stored blob only if encryption fails — a stale claim still beats handing the recipient a link that will not open. Mint and manifest share one buildSmartApiAccess so a fresh link and a later fetch cannot disagree. `expires_in` is now counted from the request rather than from mint, where it had been describing a lifetime that started days ago. Existing links, including the one that prompted this, are corrected as soon as the backend deploys. No re-share needed.
1 parent 3ea80e8 commit af17725

2 files changed

Lines changed: 120 additions & 14 deletions

File tree

backend/src/routes/api/shl.ts

Lines changed: 58 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,37 @@ function isJsonContentType(contentType: string | null): boolean {
9292
return !!contentType && /json/i.test(contentType)
9393
}
9494

95+
/**
96+
* The smart-api-access document the recipient decrypts (SHL spec §3.2).
97+
*
98+
* Derived from the session on every manifest fetch rather than frozen at mint,
99+
* because what it asserts — how long the token is good for, whether the share is
100+
* complete — are properties of the share as it stands now. Freezing them meant a
101+
* link kept telling recipients it carried the full record after the rule deciding
102+
* that was corrected, since the claim sat inside ciphertext minted days earlier.
103+
*/
104+
export function buildSmartApiAccess(session: {
105+
sessionToken: string
106+
patientId: string
107+
expiresAt: number
108+
shareScope?: ShareScope
109+
studyInstanceUID?: string
110+
}): string {
111+
return JSON.stringify({
112+
access_token: session.sessionToken,
113+
token_type: 'Bearer',
114+
expires_in: Math.max(0, Math.floor((session.expiresAt - Date.now()) / 1000)),
115+
scope: 'patient/*.read',
116+
patient: session.patientId,
117+
// aud points to our FHIR proxy — the viewer never talks to the real FHIR server.
118+
aud: `${config.baseUrl}/api/shl/fhir`,
119+
complete: isCompleteShare({
120+
selectiveScope: session.shareScope,
121+
studyInstanceUID: session.studyInstanceUID,
122+
}),
123+
})
124+
}
125+
95126
/** Build the pure SelectiveScope from a session's persisted shareScope (all-empty when absent). */
96127
function sessionSelectiveScope(session: { shareScope?: ShareScope }): SelectiveScope {
97128
return {
@@ -438,19 +469,15 @@ export const shlRoutes = new Elysia({ prefix: '/shl', tags: ['shl'] })
438469
? { excludedTypes, excludedIds, excludedObservationCategories }
439470
: undefined
440471

441-
// Build the SMART API Access token response (per SHL spec)
442-
// aud points to our FHIR proxy — the viewer never talks to the real FHIR server.
443-
// `complete` is a non-standard hint for our own viewer: false when the patient
444-
// de-selected records, so the recipient can be told the summary is partial
445-
// (qualitative only — no counts leak). Only the key holder can read it (JWE).
446-
const smartApiAccess = JSON.stringify({
447-
access_token: sessionToken,
448-
token_type: 'Bearer',
449-
expires_in: ttlSeconds,
450-
scope: 'patient/*.read',
451-
patient: patientId,
452-
aud: `${config.baseUrl}/api/shl/fhir`,
453-
complete: isCompleteShare({ selectiveScope: shareScope, studyInstanceUID: body.studyInstanceUID }),
472+
// The same document the manifest re-derives on every fetch — built once here
473+
// so a freshly minted link and a later fetch cannot disagree. `complete` is a
474+
// non-standard hint for our own viewer, readable only by the key holder (JWE).
475+
const smartApiAccess = buildSmartApiAccess({
476+
sessionToken,
477+
patientId,
478+
expiresAt,
479+
shareScope,
480+
studyInstanceUID: body.studyInstanceUID,
454481
})
455482

456483
// Generate SHL using kill-the-clipboard
@@ -614,12 +641,29 @@ export const shlRoutes = new Elysia({ prefix: '/shl', tags: ['shl'] })
614641
recipient: body.recipient,
615642
})
616643

644+
// Re-derive the access document from the session so it describes the share as
645+
// it is now. Falls back to the stored blob only if encryption fails, since a
646+
// stale claim still beats handing the recipient an unopenable link.
647+
let embedded = entry.jwe
648+
try {
649+
embedded = await encryptSHLFile({
650+
content: buildSmartApiAccess(entry),
651+
key: entry.shl.key,
652+
contentType: SMART_API_ACCESS,
653+
})
654+
} catch (error) {
655+
logger.auth.error('Could not rebuild the SHL access document — serving the one stored at mint', {
656+
shlId: params.id,
657+
error: error instanceof Error ? error.message : String(error),
658+
})
659+
}
660+
617661
// Return spec-compliant SHL manifest
618662
// The JWE compact string goes directly in `embedded` (not wrapped in custom JSON)
619663
return {
620664
files: [{
621665
contentType: SMART_API_ACCESS as string,
622-
embedded: entry.jwe,
666+
embedded,
623667
}],
624668
}
625669
}, {
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// SPDX-FileCopyrightText: Max Health Inc.
2+
// SPDX-License-Identifier: AGPL-3.0-or-later OR LicenseRef-Commercial
3+
4+
/**
5+
* The smart-api-access document a recipient decrypts.
6+
*
7+
* It used to be built once at mint and stored as ciphertext, which the manifest
8+
* replayed forever. So when `complete` was corrected to count study scoping, every
9+
* link already in circulation kept telling recipients it carried the full record —
10+
* over a view where the proxy answers all but three queries with 403.
11+
*
12+
* Deriving it from the session on each fetch is what makes a fix reach links that
13+
* already exist. These tests pin that the document follows the session.
14+
*/
15+
import { describe, expect, it } from 'bun:test'
16+
import { buildSmartApiAccess } from '../src/routes/api/shl'
17+
18+
const base = {
19+
sessionToken: 'opaque-session-token',
20+
patientId: 'max-nussbaumer',
21+
expiresAt: Date.now() + 3600_000,
22+
}
23+
24+
const parse = (json: string) => JSON.parse(json) as Record<string, unknown>
25+
26+
describe('buildSmartApiAccess', () => {
27+
it('reports a whole-patient share as complete', () => {
28+
expect(parse(buildSmartApiAccess(base)).complete).toBe(true)
29+
})
30+
31+
it('reports a study-scoped share as incomplete', () => {
32+
const doc = parse(buildSmartApiAccess({ ...base, studyInstanceUID: '1.2.840.113619.2.55.3' }))
33+
expect(doc.complete).toBe(false)
34+
})
35+
36+
it('reports a de-selected share as incomplete', () => {
37+
const doc = parse(buildSmartApiAccess({
38+
...base,
39+
shareScope: { excludedTypes: ['Condition'], excludedIds: [], excludedObservationCategories: [] },
40+
}))
41+
expect(doc.complete).toBe(false)
42+
})
43+
44+
it('carries the session token and patient the viewer needs', () => {
45+
const doc = parse(buildSmartApiAccess(base))
46+
expect(doc.access_token).toBe('opaque-session-token')
47+
expect(doc.patient).toBe('max-nussbaumer')
48+
expect(doc.token_type).toBe('Bearer')
49+
expect(doc.scope).toBe('patient/*.read')
50+
})
51+
52+
/** Frozen at mint, expires_in counted down from the wrong instant on every later fetch. */
53+
it('counts expires_in from now, not from mint', () => {
54+
const expiresIn = parse(buildSmartApiAccess({ ...base, expiresAt: Date.now() + 600_000 })).expires_in
55+
expect(expiresIn).toBeGreaterThan(590)
56+
expect(expiresIn).toBeLessThanOrEqual(600)
57+
})
58+
59+
it('never reports a negative lifetime for an expired share', () => {
60+
expect(parse(buildSmartApiAccess({ ...base, expiresAt: Date.now() - 60_000 })).expires_in).toBe(0)
61+
})
62+
})

0 commit comments

Comments
 (0)