From b0b0df8d75a3d8234f84dd3f8231f9fa631fa4d7 Mon Sep 17 00:00:00 2001 From: rsmayda Date: Thu, 9 Sep 2021 17:37:16 -0400 Subject: [PATCH 1/2] fix: scope checking for getting status on an export --- src/smartScopeHelper.test.ts | 110 ++++++++++++++++++++++------------- src/smartScopeHelper.ts | 51 +++++++++------- 2 files changed, 100 insertions(+), 61 deletions(-) diff --git a/src/smartScopeHelper.test.ts b/src/smartScopeHelper.test.ts index 8486d99..1eb0f59 100644 --- a/src/smartScopeHelper.test.ts +++ b/src/smartScopeHelper.test.ts @@ -2,7 +2,7 @@ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * SPDX-License-Identifier: Apache-2.0 */ -import { BulkDataAuth } from 'fhir-works-on-aws-interface'; +import { BulkDataAuth, ExportType } from 'fhir-works-on-aws-interface'; import { ScopeRule, ScopeType } from './smartConfig'; import { isScopeSufficient, @@ -27,7 +27,9 @@ const emptyScopeRule = (): ScopeRule => ({ }, }); const isScopeSufficientCases: ScopeType[][] = [['user'], ['patient'], ['system']]; -describe.each(isScopeSufficientCases)('%s: isScopeSufficient', (scopeType: ScopeType) => { +const exportTypes: ExportType[][] = [['group'], ['system']]; +const exportOperations: ('cancel-export' | 'get-status-export')[][] = [['cancel-export'], ['get-status-export']]; +describe.each(isScopeSufficientCases)('ScopeType: %s: isScopeSufficient', (scopeType: ScopeType) => { test('scope is sufficient to read Observation: Scope with resourceType "Observation" should be able to read "Observation" resources', () => { const clonedScopeRule = emptyScopeRule(); clonedScopeRule[scopeType].read = ['read']; @@ -62,43 +64,6 @@ describe.each(isScopeSufficientCases)('%s: isScopeSufficient', (scopeType: Scope ); }); - test('scope is sufficient for system bulk data access with "user" || "system" scopeType but not "patient" scopeType', () => { - const clonedScopeRule = emptyScopeRule(); - clonedScopeRule[scopeType].read = ['read']; - const bulkDataAuth: BulkDataAuth = { operation: 'initiate-export', exportType: 'system' }; - - // Only scopeType of user has bulkDataAccess - expect(isScopeSufficient(`${scopeType}/*.read`, clonedScopeRule, 'read', undefined, bulkDataAuth)).toEqual( - scopeType !== 'patient', - ); - }); - - test('scope is NOT sufficient for system bulk data access: Scope needs to have resourceType "*"', () => { - const clonedScopeRule = emptyScopeRule(); - clonedScopeRule[scopeType].read = ['read']; - - const bulkDataAuth: BulkDataAuth = { operation: 'initiate-export', exportType: 'system' }; - expect( - isScopeSufficient(`${scopeType}/Observation.read`, clonedScopeRule, 'read', undefined, bulkDataAuth), - ).toEqual(false); - }); - - test('scope is sufficient for group export with "system" scopeType, not "user" of "patient" scopeType', () => { - const clonedScopeRule = emptyScopeRule(); - clonedScopeRule[scopeType].read = ['read']; - const bulkDataAuth: BulkDataAuth = { operation: 'initiate-export', exportType: 'group' }; - - // Only scopeType of system has bulkDataAccess - expect(isScopeSufficient(`${scopeType}/*.read`, clonedScopeRule, 'read', undefined, bulkDataAuth)).toEqual( - scopeType === 'system', - ); - - // Group export result is filtered on allowed resourceType, scope not having resourceType "*" should be passed - expect( - isScopeSufficient(`${scopeType}/Observation.read`, clonedScopeRule, 'read', undefined, bulkDataAuth), - ).toEqual(scopeType === 'system'); - }); - test('scope is sufficient to do a search-system', () => { const clonedScopeRule = emptyScopeRule(); clonedScopeRule[scopeType].read = ['search-system']; @@ -124,6 +89,73 @@ describe.each(isScopeSufficientCases)('%s: isScopeSufficient', (scopeType: Scope expect(isScopeSufficient(`fake`, clonedScopeRule, 'read')).toEqual(false); }); + + describe('BulkDataAuth', () => { + test('scope is sufficient for `system` initiate-export with "user" || "system" scopeType but not "patient" scopeType', () => { + const clonedScopeRule = emptyScopeRule(); + clonedScopeRule[scopeType].read = ['read']; + const bulkDataAuth: BulkDataAuth = { operation: 'initiate-export', exportType: 'system' }; + + // Only scopeType of user has bulkDataAccess + expect(isScopeSufficient(`${scopeType}/*.read`, clonedScopeRule, 'read', undefined, bulkDataAuth)).toEqual( + scopeType !== 'patient', + ); + }); + + test('scope is NOT sufficient for `system` initiate-export: Scope needs to have resourceType "*"', () => { + const clonedScopeRule = emptyScopeRule(); + clonedScopeRule[scopeType].read = ['read']; + + const bulkDataAuth: BulkDataAuth = { operation: 'initiate-export', exportType: 'system' }; + expect( + isScopeSufficient(`${scopeType}/Observation.read`, clonedScopeRule, 'read', undefined, bulkDataAuth), + ).toEqual(false); + }); + + test('scope is sufficient for `group` initiate-export with "system" scopeType, not "user" of "patient" scopeType', () => { + const clonedScopeRule = emptyScopeRule(); + clonedScopeRule[scopeType].read = ['read']; + const bulkDataAuth: BulkDataAuth = { operation: 'initiate-export', exportType: 'group' }; + + // Only scopeType of system has bulkDataAccess + expect(isScopeSufficient(`${scopeType}/*.read`, clonedScopeRule, 'read', undefined, bulkDataAuth)).toEqual( + scopeType === 'system', + ); + + // Group export result is filtered on allowed resourceType, scope not having resourceType "*" should be passed + expect( + isScopeSufficient(`${scopeType}/Observation.read`, clonedScopeRule, 'read', undefined, bulkDataAuth), + ).toEqual(scopeType === 'system'); + }); + describe.each(exportTypes)('export type: %s', (exportType: ExportType) => { + describe.each(exportOperations)( + 'export operation: %s', + (operation: 'cancel-export' | 'get-status-export') => { + test('scope is sufficient for non "patient" scopeType', () => { + const clonedScopeRule = emptyScopeRule(); + clonedScopeRule[scopeType].read = ['read']; + const bulkDataAuth: BulkDataAuth = { operation, exportType }; + + // Only scopeType of system has bulkDataAccess + expect( + isScopeSufficient(`${scopeType}/*.read`, clonedScopeRule, 'read', undefined, bulkDataAuth), + ).toEqual(scopeType !== 'patient'); + + // Group export result is filtered on allowed resourceType, scope not having resourceType "*" should be passed + expect( + isScopeSufficient( + `${scopeType}/Observation.read`, + clonedScopeRule, + 'read', + undefined, + bulkDataAuth, + ), + ).toEqual(scopeType !== 'patient'); + }); + }, + ); + }); + }); }); describe('getScopes', () => { diff --git a/src/smartScopeHelper.ts b/src/smartScopeHelper.ts index 3d11e7b..9a2a6a0 100644 --- a/src/smartScopeHelper.ts +++ b/src/smartScopeHelper.ts @@ -84,30 +84,37 @@ function isSmartScopeSufficientForBulkDataAccess( smartScope: ClinicalSmartScope, scopeRule: ScopeRule, ) { - let bulkDataRequestHasCorrectScope = false; - if (bulkDataAuth.exportType === 'system') { - bulkDataRequestHasCorrectScope = - ['system', 'user'].includes(smartScope.scopeType) && - smartScope.resourceType === '*' && - ['*', 'read'].includes(smartScope.accessType) && - getValidOperationsForScopeTypeAndAccessType( - smartScope.scopeType, - smartScope.accessType, - scopeRule, - ).includes('read'); - } else if (bulkDataAuth.exportType === 'group') { - bulkDataRequestHasCorrectScope = - ['system'].includes(smartScope.scopeType) && - ['*', 'read'].includes(smartScope.accessType) && - getValidOperationsForScopeTypeAndAccessType( - smartScope.scopeType, - smartScope.accessType, - scopeRule, - ).includes('read'); + if (bulkDataAuth.operation === 'initiate-export') { + let bulkDataRequestHasCorrectScope = false; + if (bulkDataAuth.exportType === 'system') { + bulkDataRequestHasCorrectScope = + ['system', 'user'].includes(smartScope.scopeType) && + smartScope.resourceType === '*' && + ['*', 'read'].includes(smartScope.accessType) && + getValidOperationsForScopeTypeAndAccessType( + smartScope.scopeType, + smartScope.accessType, + scopeRule, + ).includes('read'); + } else if (bulkDataAuth.exportType === 'group') { + bulkDataRequestHasCorrectScope = + ['system'].includes(smartScope.scopeType) && + ['*', 'read'].includes(smartScope.accessType) && + getValidOperationsForScopeTypeAndAccessType( + smartScope.scopeType, + smartScope.accessType, + scopeRule, + ).includes('read'); + } + return bulkDataRequestHasCorrectScope; } return ( - ['initiate-export', 'get-status-export', 'cancel-export'].includes(bulkDataAuth.operation) && - bulkDataRequestHasCorrectScope + ['get-status-export', 'cancel-export'].includes(bulkDataAuth.operation) && + ['system', 'user'].includes(smartScope.scopeType) && + ['*', 'read'].includes(smartScope.accessType) && + getValidOperationsForScopeTypeAndAccessType(smartScope.scopeType, smartScope.accessType, scopeRule).includes( + 'read', + ) ); } From 280b8d99128d509260d5fbae14c707c490adb515 Mon Sep 17 00:00:00 2001 From: rsmayda Date: Thu, 9 Sep 2021 17:47:23 -0400 Subject: [PATCH 2/2] chore: clean up code --- src/smartScopeHelper.test.ts | 2 -- src/smartScopeHelper.ts | 29 ++++++++--------------------- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/src/smartScopeHelper.test.ts b/src/smartScopeHelper.test.ts index 1eb0f59..f06b40f 100644 --- a/src/smartScopeHelper.test.ts +++ b/src/smartScopeHelper.test.ts @@ -136,12 +136,10 @@ describe.each(isScopeSufficientCases)('ScopeType: %s: isScopeSufficient', (scope clonedScopeRule[scopeType].read = ['read']; const bulkDataAuth: BulkDataAuth = { operation, exportType }; - // Only scopeType of system has bulkDataAccess expect( isScopeSufficient(`${scopeType}/*.read`, clonedScopeRule, 'read', undefined, bulkDataAuth), ).toEqual(scopeType !== 'patient'); - // Group export result is filtered on allowed resourceType, scope not having resourceType "*" should be passed expect( isScopeSufficient( `${scopeType}/Observation.read`, diff --git a/src/smartScopeHelper.ts b/src/smartScopeHelper.ts index 9a2a6a0..5a59df7 100644 --- a/src/smartScopeHelper.ts +++ b/src/smartScopeHelper.ts @@ -84,37 +84,24 @@ function isSmartScopeSufficientForBulkDataAccess( smartScope: ClinicalSmartScope, scopeRule: ScopeRule, ) { + const { scopeType, accessType, resourceType } = smartScope; + const hasReadPermissions = getValidOperationsForScopeTypeAndAccessType(scopeType, accessType, scopeRule).includes( + 'read', + ); if (bulkDataAuth.operation === 'initiate-export') { let bulkDataRequestHasCorrectScope = false; if (bulkDataAuth.exportType === 'system') { bulkDataRequestHasCorrectScope = - ['system', 'user'].includes(smartScope.scopeType) && - smartScope.resourceType === '*' && - ['*', 'read'].includes(smartScope.accessType) && - getValidOperationsForScopeTypeAndAccessType( - smartScope.scopeType, - smartScope.accessType, - scopeRule, - ).includes('read'); + ['system', 'user'].includes(scopeType) && resourceType === '*' && hasReadPermissions; } else if (bulkDataAuth.exportType === 'group') { - bulkDataRequestHasCorrectScope = - ['system'].includes(smartScope.scopeType) && - ['*', 'read'].includes(smartScope.accessType) && - getValidOperationsForScopeTypeAndAccessType( - smartScope.scopeType, - smartScope.accessType, - scopeRule, - ).includes('read'); + bulkDataRequestHasCorrectScope = ['system'].includes(scopeType) && hasReadPermissions; } return bulkDataRequestHasCorrectScope; } return ( ['get-status-export', 'cancel-export'].includes(bulkDataAuth.operation) && - ['system', 'user'].includes(smartScope.scopeType) && - ['*', 'read'].includes(smartScope.accessType) && - getValidOperationsForScopeTypeAndAccessType(smartScope.scopeType, smartScope.accessType, scopeRule).includes( - 'read', - ) + ['system', 'user'].includes(scopeType) && + hasReadPermissions ); }