Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Commit

Permalink
feat!: update system export to only allow system scope by default (#60)
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: User scope will not be allowed for system export by default. To allow user scope for system export, please update `src/config.ts` in the deployment package to pass in parameter `isUserScopeAllowedForSystemExport`.
  • Loading branch information
@Bingjiling
Bingjiling authored Sep 14, 2021
1 parent 82b6ff3 commit 45b7960
Show file tree
Hide file tree
Showing 4 changed files with 390 additions and 52 deletions.
216 changes: 199 additions & 17 deletions src/smartHandler.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -677,24 +677,206 @@ describe('verifyAccessToken; System level export requests', () => {

const authZConfig = baseAuthZConfig();

const authZHandler: SMARTHandler = new SMARTHandler(authZConfig, apiUrl, '4.0.1');
test.each(arrayScopesCases)('CASE: %p', (_firstArg, request, decodedAccessToken, isValid) => {
jest.spyOn(smartAuthorizationHelper, 'verifyJwtToken').mockImplementation(() =>
Promise.resolve(decodedAccessToken),
);
const { decode } = jwt as jest.Mocked<typeof import('jsonwebtoken')>;
decode.mockReturnValue(<{ [key: string]: any }>decodedAccessToken);
if (!isValid) {
return expect(authZHandler.verifyAccessToken(<VerifyAccessTokenRequest>request)).rejects.toThrowError(
UnauthorizedError,
const authZHandler: SMARTHandler = new SMARTHandler(authZConfig, apiUrl, '4.0.1', undefined, undefined, true);
test.each(arrayScopesCases)(
'isUserScopeAllowedForSystemExport = true, CASE: %p',
(_firstArg, request, decodedAccessToken, isValid) => {
jest.spyOn(smartAuthorizationHelper, 'verifyJwtToken').mockImplementation(() =>
Promise.resolve(decodedAccessToken),
);
}
const expectedUserIdentity = getExpectedUserIdentity(decodedAccessToken);
expectedUserIdentity.scp = decodedAccessToken.scp;
return expect(authZHandler.verifyAccessToken(<VerifyAccessTokenRequest>request)).resolves.toMatchObject(
expectedUserIdentity,
);
});
const { decode } = jwt as jest.Mocked<typeof import('jsonwebtoken')>;
decode.mockReturnValue(<{ [key: string]: any }>decodedAccessToken);
if (!isValid) {
return expect(authZHandler.verifyAccessToken(<VerifyAccessTokenRequest>request)).rejects.toThrowError(
UnauthorizedError,
);
}
const expectedUserIdentity = getExpectedUserIdentity(decodedAccessToken);
expectedUserIdentity.scp = decodedAccessToken.scp;
return expect(authZHandler.verifyAccessToken(<VerifyAccessTokenRequest>request)).resolves.toMatchObject(
expectedUserIdentity,
);
},
);

const arrayScopesCasesNoUserScope: (string | boolean | VerifyAccessTokenRequest | any)[][] = [
[
'Read and Write Access: initiate-export',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'initiate-export' },
},
{ ...baseAccessNoScopes, scp: ['user/*.*', 'patient/*.write'], ...practitionerFhirUser },
false,
],
[
'Read Access: initiate-export',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'initiate-export' },
},
{ ...baseAccessNoScopes, scp: ['system/*.read', 'patient/*.*'], ...practitionerFhirUser },
true,
],
[
'Read and Write Access: get-status-export',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'get-status-export' },
},
{ ...baseAccessNoScopes, scp: ['user/*.*'], ...practitionerFhirUser },
false,
],
[
'Read and Write Access: cancel-export',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'cancel-export' },
},
{ ...baseAccessNoScopes, scp: ['system/*.*'], ...practitionerFhirUser },
true,
],
[
'External practitioner: initiate-export; fail',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'initiate-export' },
},
{ ...baseAccessNoScopes, scp: ['user/*.*', 'patient/*.write'], fhirUser: externalPractitionerIdentity },
false,
],
[
'No export read access: cancel-export',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'cancel-export' },
},
{ ...baseAccessNoScopes, scp: ['user/*.write'], ...practitionerFhirUser },
false,
],
[
'No export read access; Patient only: cancel-export',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'cancel-export' },
},
{ ...baseAccessNoScopes, scp: ['patient/*.*'], ...patientContext },
false,
],
[
'System all scope: initiate-export',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'initiate-export' },
},
{ ...baseAccessNoScopes, scp: ['system/*.*'] },
true,
],
[
'System read scope: initiate-export',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'initiate-export' },
},
{ ...baseAccessNoScopes, scp: ['system/*.read'] },
true,
],
[
'System read scope: cancel-export',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'cancel-export' },
},
{ ...baseAccessNoScopes, scp: ['system/*.read'] },
true,
],
[
'System read scope: get-status-export',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'get-status-export' },
},
{ ...baseAccessNoScopes, scp: ['system/*.read'] },
true,
],
[
'System write scope: get-status-export',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'get-status-export' },
},
{ ...baseAccessNoScopes, scp: ['system/*.write'] },
false,
],
[
'System read scope, group export: initiate-export',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'get-status-export' },
},
{ ...baseAccessNoScopes, scp: ['system/*.read'], fhirUser: externalPractitionerIdentity },
true,
],
[
'Internal patient read scope, group export: get-status-export',
{
accessToken: 'fake',
operation: 'read',
resourceType: '',
bulkDataAuth: { exportType: 'system', operation: 'get-status-export' },
},
{ ...baseAccessNoScopes, scp: ['user/*.read'], ...patientContext },
false,
],
];

const authZHandlerNoUserScope: SMARTHandler = new SMARTHandler(authZConfig, apiUrl, '4.0.1');
test.each(arrayScopesCasesNoUserScope)(
'isUserScopeAllowedForSystemExport = false, CASE: %p',
(_firstArg, request, decodedAccessToken, isValid) => {
jest.spyOn(smartAuthorizationHelper, 'verifyJwtToken').mockImplementation(() =>
Promise.resolve(decodedAccessToken),
);
const { decode } = jwt as jest.Mocked<typeof import('jsonwebtoken')>;
decode.mockReturnValue(<{ [key: string]: any }>decodedAccessToken);
if (!isValid) {
return expect(
authZHandlerNoUserScope.verifyAccessToken(<VerifyAccessTokenRequest>request),
).rejects.toThrowError(UnauthorizedError);
}
const expectedUserIdentity = getExpectedUserIdentity(decodedAccessToken);
expectedUserIdentity.scp = decodedAccessToken.scp;
return expect(
authZHandlerNoUserScope.verifyAccessToken(<VerifyAccessTokenRequest>request),
).resolves.toMatchObject(expectedUserIdentity);
},
);

test.each([['user'], ['system']])('CASE: %p scope; bulk data request; no sub set in JWT', baseScope => {
const decodedAccessToken = { ...baseAccessNoScopes, scp: [`${baseScope}/*.read`], sub: '' };
Expand Down
13 changes: 12 additions & 1 deletion src/smartHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ export class SMARTHandler implements Authorization {

private readonly fhirVersion: FhirVersion;

private readonly isUserScopeAllowedForSystemExport: boolean;

private readonly jwksClient?: JwksClient;

/**
Expand All @@ -78,6 +80,7 @@ export class SMARTHandler implements Authorization {
fhirVersion: FhirVersion,
adminAccessTypes = ['Practitioner'],
bulkDataAccessTypes = ['Practitioner'],
isUserScopeAllowedForSystemExport = false,
) {
if (config.version !== this.version) {
throw Error('Authorization configuration version does not match handler version');
Expand All @@ -87,6 +90,7 @@ export class SMARTHandler implements Authorization {
this.fhirVersion = fhirVersion;
this.adminAccessTypes = adminAccessTypes;
this.bulkDataAccessTypes = bulkDataAccessTypes;
this.isUserScopeAllowedForSystemExport = isUserScopeAllowedForSystemExport;
if (this.config.jwksEndpoint && !this.config.tokenIntrospection) {
this.jwksClient = getJwksClient(this.config.jwksEndpoint);
}
Expand Down Expand Up @@ -124,6 +128,7 @@ export class SMARTHandler implements Authorization {
scopes,
this.config.scopeRule,
request.operation,
this.isUserScopeAllowedForSystemExport,
request.resourceType,
request.bulkDataAuth,
patientContextClaim,
Expand Down Expand Up @@ -249,7 +254,13 @@ export class SMARTHandler implements Authorization {
request.requests.forEach((req: BatchReadWriteRequest) => {
if (
!usableScopes.some((scope: string) =>
isScopeSufficient(scope, this.config.scopeRule, req.operation, req.resourceType),
isScopeSufficient(
scope,
this.config.scopeRule,
req.operation,
this.isUserScopeAllowedForSystemExport,
req.resourceType,
),
)
) {
logger.error('User supplied scopes are insufficient', {
Expand Down
Loading

0 comments on commit 45b7960

Please sign in to comment.