cqframework/cqf-ruler

CDS Service Request fhirAuthorization broken in 0.5.1 and above

aeyates opened this issue · 2 comments

In attempting to upgrade from 0.5.0, I started receiving a 401 Unauthorized Exception when executing a plan definition. After debugging, I found that the fhirAuthorization clause specified by CDS Hooks (https://cds-hooks.hl7.org/ballots/2018May/specification/1.0/#fhir-resource-access) is no longer forming a proper request to the EHR. The hook request used to have the following format:

"fhirAuthorization": {
     "access_token": "blah",
     "token_type": "Bearer",
     "expires_in": 300,
     "scope": "patient/*.read",
     "subject": "cds-service4"
 }

Through a lot of trial and error, I eventually found I could modify the hook request like this and it would properly authenticate:

"fhirAuthorization": {
        "access_token": "Bearer blah",
        "token_type": "Authorization",
        "expires_in": 300,
        "scope": "patient/*.read",
        "subject": "cds-service4"
    }

This does not match the 1.0 or current draft specification for CDS Hooks. If CQF Ruler requires a different syntax, can you add documentation somewhere? This is unexpected.

I tracked down the original code in 0.5.0 that processed the Authorization header properly. It was in the (now removed) class EvaluationContext, and here's the relevant method:

public IGenericClient getHookFhirClient() {
        IGenericClient client = this.fhirContext.newRestfulGenericClient(this.hook.getRequest().getFhirServerUrl());
        if (this.hook.getRequest().getFhirAuthorization() != null
                && hook.getRequest().getFhirAuthorization().getTokenType().equals("Bearer")) {
            BearerTokenAuthInterceptor authInterceptor = new BearerTokenAuthInterceptor(
                    hook.getRequest().getFhirAuthorization().getAccessToken());
            client.registerInterceptor(authInterceptor);

            // TODO: account for the expires_in, scope and subject properties within
            // workflow
        }

        LoggingInterceptor loggingInterceptor = new LoggingInterceptor();
        loggingInterceptor.setLogRequestSummary(true);
        loggingInterceptor.setLogRequestHeaders(true);
        loggingInterceptor.setLogRequestBody(true);
        loggingInterceptor.setLogResponseSummary(true);
        loggingInterceptor.setLogResponseHeaders(true);
        loggingInterceptor.setLogResponseBody(true);
        client.registerInterceptor(loggingInterceptor);

        return client;
    }

This was replaced by the method resolveRemoteClient in the CQLEvaluationHelper. But that method no longer has the full hook information, only the EndpointInfo:

public IGenericClient resolveRemoteClient(EndpointInfo endpoint) {
		IGenericClient remoteClient = fhirContext.newRestfulGenericClient(endpoint.getAddress());
		endpoint.
		if (endpoint.getHeaders() != null) {
			AdditionalRequestHeadersInterceptor headerInterceptor = new AdditionalRequestHeadersInterceptor();
			for (HeaderInfo header : getHeaderNameValuePairs(endpoint.getHeaders())) {
				headerInterceptor.addHeaderValue(header.getName(), header.getValue());
			}
			remoteClient.registerInterceptor(headerInterceptor);
		}
		return remoteClient;
	}

One simple way I found to fix this is to add a check for the header name "Bearer" in this method and do the correct conversion. I also found it useful to reintroduce the LoggingInterceptor so the requests were transparent.

	public IGenericClient resolveRemoteClient(EndpointInfo endpoint) {
		IGenericClient remoteClient = fhirContext.newRestfulGenericClient(endpoint.getAddress());
		if (endpoint.getHeaders() != null) {
			AdditionalRequestHeadersInterceptor headerInterceptor = new AdditionalRequestHeadersInterceptor();
			for (HeaderInfo header : getHeaderNameValuePairs(endpoint.getHeaders())) {
				if (header.getName().equals("Bearer")) {
					Clients.registerBearerTokenAuth(remoteClient, header.getValue());
				} else {
					headerInterceptor.addHeaderValue(header.getName(), header.getValue());
				}
			}
			remoteClient.registerInterceptor(headerInterceptor);
		}
		LoggingInterceptor loggingInterceptor = new LoggingInterceptor();
		loggingInterceptor.setLogRequestSummary(true);
		loggingInterceptor.setLogRequestHeaders(true);
		loggingInterceptor.setLogRequestBody(true);
		loggingInterceptor.setLogResponseSummary(true);
		loggingInterceptor.setLogResponseHeaders(true);
		loggingInterceptor.setLogResponseBody(true);
		remoteClient.registerInterceptor(loggingInterceptor);
		return remoteClient;
	}

For anyone encountering this, I have not verified personally but a colleague has reported this issue was fixed in 0.13.0.