diff --git a/examples/medplum-demo-bots/src/hl7-bot.test.ts b/examples/medplum-demo-bots/src/hl7-bot.test.ts index 5d729751b3..2610f12d97 100644 --- a/examples/medplum-demo-bots/src/hl7-bot.test.ts +++ b/examples/medplum-demo-bots/src/hl7-bot.test.ts @@ -1,9 +1,9 @@ import { Hl7Message, indexSearchParameterBundle, indexStructureDefinitionBundle } from '@medplum/core'; +import { SEARCH_PARAMETER_BUNDLE_FILES, readJson } from '@medplum/definitions'; import { Bot, Bundle, Reference, SearchParameter } from '@medplum/fhirtypes'; import { MockClient } from '@medplum/mock'; import { expect, test } from 'vitest'; import { handler } from './hl7-bot'; -import { SEARCH_PARAMETER_BUNDLE_FILES, readJson } from '@medplum/definitions'; //To run these tests from the command line //npm t src/hl7-bot.test.ts diff --git a/examples/medplum-demo-bots/src/stripe-bots/stripe-create-invoice.test.ts b/examples/medplum-demo-bots/src/stripe-bots/stripe-create-invoice.test.ts index fee7c04306..2c01f695ea 100644 --- a/examples/medplum-demo-bots/src/stripe-bots/stripe-create-invoice.test.ts +++ b/examples/medplum-demo-bots/src/stripe-bots/stripe-create-invoice.test.ts @@ -1,3 +1,6 @@ +import { indexSearchParameterBundle, indexStructureDefinitionBundle } from '@medplum/core'; +import { SEARCH_PARAMETER_BUNDLE_FILES, readJson } from '@medplum/definitions'; +import { Bundle, SearchParameter } from '@medplum/fhirtypes'; import { MockClient } from '@medplum/mock'; import { expect, test } from 'vitest'; import { handler } from './stripe-create-invoice'; @@ -6,6 +9,11 @@ const medplum = new MockClient(); // npm t src/examples/stripe-bots/stripe-create-invoice.test.ts test('Create Invoice', async () => { + indexStructureDefinitionBundle(readJson('fhir/r4/profiles-types.json') as Bundle); + indexStructureDefinitionBundle(readJson('fhir/r4/profiles-resources.json') as Bundle); + for (const filename of SEARCH_PARAMETER_BUNDLE_FILES) { + indexSearchParameterBundle(readJson(filename) as Bundle); + } // This input is a Stripe Event object from https://stripe.com/docs/webhooks/stripe-events const input = { id: 'evt_1MqItaDlo6kh7lYQKFQhFJ2J', diff --git a/packages/app/src/resource/ProfilesPage.test.tsx b/packages/app/src/resource/ProfilesPage.test.tsx index f8272a4b4e..24d53eb707 100644 --- a/packages/app/src/resource/ProfilesPage.test.tsx +++ b/packages/app/src/resource/ProfilesPage.test.tsx @@ -16,7 +16,7 @@ describe('ProfilesPage', () => { beforeAll(async () => { const loadedProfileUrls: string[] = []; for (const profile of [fishPatientProfile, FishPatientResources.getFishSpeciesExtensionSD()]) { - const sd = await medplum.createResourceIfNoneExist(profile, `url:${profile.url}`); + const sd = await medplum.createResourceIfNoneExist(profile, `url=${profile.url}`); loadedProfileUrls.push(sd.url); loadDataType(sd, sd.url); } diff --git a/packages/core/src/access.test.ts b/packages/core/src/access.test.ts index 61fc3ba995..bae614b350 100644 --- a/packages/core/src/access.test.ts +++ b/packages/core/src/access.test.ts @@ -143,7 +143,27 @@ describe('Access', () => { }, ], }; - expect(matchesAccessPolicy(ap, { resourceType: 'Patient', meta: { compartment: [{ reference: '1' }] } }, true)); - expect(matchesAccessPolicy(ap, { resourceType: 'Patient', meta: { compartment: [{ reference: '2' }] } }, false)); + expect( + matchesAccessPolicy(ap, { resourceType: 'Patient', meta: { compartment: [{ reference: '1' }] } }, true) + ).toEqual(true); + expect( + matchesAccessPolicy(ap, { resourceType: 'Patient', meta: { compartment: [{ reference: '2' }] } }, false) + ).toEqual(false); + }); + + test('Invalid SearchParameter in criteria in AccessPolicy', () => { + const ap: AccessPolicy = { + resourceType: 'AccessPolicy', + resource: [ + { + resourceType: 'Patient', + criteria: 'Patient?invalid=invalid', + }, + ], + }; + + expect(() => + matchesAccessPolicy(ap, { resourceType: 'Patient', name: [{ given: ['John'], family: 'Doe' }] }, false) + ).toThrow(/Unknown search parameter: invalid/); }); }); diff --git a/packages/core/src/search/match.test.ts b/packages/core/src/search/match.test.ts index 536faf8fe9..cd7f3f4312 100644 --- a/packages/core/src/search/match.test.ts +++ b/packages/core/src/search/match.test.ts @@ -60,12 +60,12 @@ describe('Search matching', () => { }); test('Unknown filter', () => { - expect( + expect(() => matchesSearchRequest( { resourceType: 'Patient' }, { resourceType: 'Patient', filters: [{ code: 'unknown', operator: Operator.EQUALS, value: 'xyz' }] } ) - ).toBe(false); + ).toThrow('Unknown search parameter: unknown for resource type Patient'); }); test('Boolean filter', () => { diff --git a/packages/core/src/search/match.ts b/packages/core/src/search/match.ts index b1e9d67186..95580a8f44 100644 --- a/packages/core/src/search/match.ts +++ b/packages/core/src/search/match.ts @@ -34,7 +34,11 @@ export function matchesSearchRequest(resource: Resource, searchRequest: SearchRe function matchesSearchFilter(resource: Resource, searchRequest: SearchRequest, filter: Filter): boolean { const searchParam = globalSchema.types[searchRequest.resourceType]?.searchParams?.[filter.code]; if (!searchParam) { - return false; + throw new Error(`Unknown search parameter: ${filter.code} for resource type ${searchRequest.resourceType}`); + } + if (filter.operator === Operator.MISSING && searchParam) { + const values = evalFhirPath(searchParam.expression as string, resource); + return filter.value === 'true' ? !values.length : values.length > 0; } if (filter.operator === Operator.MISSING || filter.operator === Operator.PRESENT) { return matchesMissingOrPresent(resource, filter, searchParam); @@ -50,7 +54,7 @@ function matchesSearchFilter(resource: Resource, searchRequest: SearchRequest, f case 'date': return matchesDateFilter(resource, filter, searchParam); default: - // Unknown search parameter or search parameter type + // Unknown search parameter type // Default fail the check return false; } diff --git a/packages/fhir-router/src/fhirrouter.ts b/packages/fhir-router/src/fhirrouter.ts index 791c2ba45f..dfe6e5c89a 100644 --- a/packages/fhir-router/src/fhirrouter.ts +++ b/packages/fhir-router/src/fhirrouter.ts @@ -6,6 +6,7 @@ import { normalizeOperationOutcome, notFound, parseSearchRequest, + validateResourceType, } from '@medplum/core'; import { OperationOutcome, Resource, ResourceType } from '@medplum/fhirtypes'; import type { IncomingHttpHeaders } from 'node:http'; @@ -45,6 +46,7 @@ async function batch(req: FhirRequest, repo: FhirRepository, router: FhirRouter) // Search async function search(req: FhirRequest, repo: FhirRepository): Promise { const { resourceType } = req.params; + validateResourceType(resourceType); const query = req.query as Record; const bundle = await repo.search(parseSearchRequest(resourceType as ResourceType, query)); return [allOk, bundle]; diff --git a/packages/health-gorilla/src/utils.test.ts b/packages/health-gorilla/src/utils.test.ts index fa25f2ad06..dec75850b6 100644 --- a/packages/health-gorilla/src/utils.test.ts +++ b/packages/health-gorilla/src/utils.test.ts @@ -1,5 +1,6 @@ -import { ContentType, allOk, append } from '@medplum/core'; -import { RequestGroup } from '@medplum/fhirtypes'; +import { ContentType, allOk, append, indexSearchParameterBundle, indexStructureDefinitionBundle } from '@medplum/core'; +import { SEARCH_PARAMETER_BUNDLE_FILES, readJson } from '@medplum/definitions'; +import { Bundle, RequestGroup, SearchParameter } from '@medplum/fhirtypes'; import { MockClient } from '@medplum/mock'; import { HealthGorillaConfig, @@ -29,6 +30,14 @@ const testConfig: HealthGorillaConfig = { describe('Health Gorilla utils', () => { const OLD_ENV = process.env; + beforeAll(() => { + indexStructureDefinitionBundle(readJson('fhir/r4/profiles-types.json') as Bundle); + indexStructureDefinitionBundle(readJson('fhir/r4/profiles-resources.json') as Bundle); + for (const filename of SEARCH_PARAMETER_BUNDLE_FILES) { + indexSearchParameterBundle(readJson(filename) as Bundle); + } + }); + beforeEach(() => { jest.resetModules(); process.env = { ...OLD_ENV }; diff --git a/packages/mock/src/client.ts b/packages/mock/src/client.ts index c9418b9be0..39d2bbc622 100644 --- a/packages/mock/src/client.ts +++ b/packages/mock/src/client.ts @@ -693,11 +693,50 @@ export class MockFetchClient { const result = await this.router.handleRequest(request, this.repo); if (result.length === 1) { + await this.logStructureDefinitionError(request.params.resourceType); + await this.logSearchParameterError(request); return result[0]; } else { return result[1]; } } + + private async logStructureDefinitionError(resourceType: string): Promise { + const structureDefinitions = await this.repo.searchResources({ + resourceType: 'StructureDefinition', + }); + const isMatch = structureDefinitions.some((sd) => sd.id === resourceType); + if (!isMatch) { + console.error( + `Unknown resource type: ${resourceType}. Please check whether it is defined in structuredefinitions.json.` + ); + } + } + + private async logSearchParameterError(request: FhirRequest): Promise { + const { + method, + params: { resourceType }, + query, + } = request; + const codes = Object.keys(query); + if (method !== 'GET' || codes.length === 0) { + return; + } + const searchParameters = await this.repo.searchResources({ + resourceType: 'SearchParameter', + }); + codes.forEach((code) => { + const isMatch = searchParameters.some( + (searchParameter) => searchParameter.code === code && searchParameter.base.some((r) => r === resourceType) + ); + if (!isMatch) { + console.error( + `Unknown search parameter '${code}' for resource type '${resourceType}'. Please check whether it is defined in searchparameters.json.` + ); + } + }); + } } function base64Encode(str: string): string { diff --git a/packages/server/src/fhir/accesspolicy.test.ts b/packages/server/src/fhir/accesspolicy.test.ts index b76d74f640..cd55078257 100644 --- a/packages/server/src/fhir/accesspolicy.test.ts +++ b/packages/server/src/fhir/accesspolicy.test.ts @@ -2247,4 +2247,33 @@ describe('AccessPolicy', () => { }) ).rejects.toThrow(); })); + + test.only('readResource with invalid access policy', async () => + withTestContext(async () => { + const { repo, membership } = await createTestProject({ + withRepo: true, + withClient: true, + }); + + const accessPolicy = await systemRepo.createResource({ + resourceType: 'AccessPolicy', + resource: [ + { + resourceType: 'Patient', + criteria: `Patient?i=completed`, + readonly: true, + }, + ], + }); + + const patient = await repo.createResource({ resourceType: 'Patient' }); + await systemRepo.updateResource({ + ...membership, + accessPolicy: createReference(accessPolicy), + }); + + await expect( + repo.readResource('Patient', patient.id as string, { allowReadFrom: ['database'] }) + ).rejects.toThrow(); + })); }); diff --git a/packages/server/src/fhir/repo.test.ts b/packages/server/src/fhir/repo.test.ts index 4df3e150c9..3f60c67988 100644 --- a/packages/server/src/fhir/repo.test.ts +++ b/packages/server/src/fhir/repo.test.ts @@ -93,8 +93,8 @@ describe('FHIR Repo', () => { } }); - test('Read invalid resource with `checkCacheOnly` set', async () => { - await expect(systemRepo.readResource('Subscription', randomUUID(), { checkCacheOnly: true })).rejects.toThrow( + test('Read invalid resource with `allowReadFrom` only including `cache`', async () => { + await expect(systemRepo.readResource('Subscription', randomUUID(), { allowReadFrom: ['cache'] })).rejects.toThrow( new OperationOutcomeError(notFound) ); }); diff --git a/packages/server/src/fhir/repo.ts b/packages/server/src/fhir/repo.ts index fc86520860..be5918e8bd 100644 --- a/packages/server/src/fhir/repo.ts +++ b/packages/server/src/fhir/repo.ts @@ -172,7 +172,7 @@ export interface CacheEntry { } export type ReadResourceOptions = { - checkCacheOnly?: boolean; + allowReadFrom?: ('cache' | 'database')[]; }; /** @@ -254,21 +254,25 @@ export class Repository extends BaseRepository implements FhirRepository(resourceType, id); - if (cacheRecord) { - // This is an optimization to avoid a database query. - // However, it depends on all values in the cache having "meta.compartment" - // Old versions of Medplum did not populate "meta.compartment" - // So this optimization is blocked until we add a migration. - // if (!this.canReadCacheEntry(cacheRecord)) { - // throw new OperationOutcomeError(notFound); - // } - if (this.canReadCacheEntry(cacheRecord)) { - return cacheRecord.resource; + const allowReadFrom = options?.allowReadFrom ?? (['cache', 'database'] as const); + + if (allowReadFrom.includes('cache')) { + const cacheRecord = await getCacheEntry(resourceType, id); + if (cacheRecord) { + // This is an optimization to avoid a database query. + // However, it depends on all values in the cache having "meta.compartment" + // Old versions of Medplum did not populate "meta.compartment" + // So this optimization is blocked until we add a migration. + // if (!this.canReadCacheEntry(cacheRecord)) { + // throw new OperationOutcomeError(notFound); + // } + if (this.canReadCacheEntry(cacheRecord)) { + return cacheRecord.resource; + } } } - if (options?.checkCacheOnly) { + if (!allowReadFrom.includes('database')) { throw new OperationOutcomeError(notFound); } @@ -1156,6 +1160,7 @@ export class Repository extends BaseRepository implements FhirRepository { try { return await systemRepo.readResource('Subscription', subscriptionId, { - checkCacheOnly: channelType === 'websocket', + allowReadFrom: channelType === 'websocket' ? ['cache'] : ['cache', 'database'], }); } catch (err) { const outcome = normalizeOperationOutcome(err);