Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NullPointerException with fhir-smart when patient_id claim is missing in access token #4083

Closed
qing-xia-snkeos opened this issue Nov 17, 2022 · 6 comments
Assignees
Labels
bug Something isn't working P1 Priority 1 - Must Have

Comments

@qing-xia-snkeos
Copy link

qing-xia-snkeos commented Nov 17, 2022

Describe the bug
FHIR Server throws NullPointerException + 500 Internal Server Error for requests accessing resources in the allowed scopes with fhir-smart enabled.

Environment
Which version of LinuxForHealth FHIR Server?

  • 5.0.0
  • 5.1.0

To Reproduce
Steps to reproduce the behavior:

  1. After installation and confiugre AuthZ with the identity provider and verify the authorization flow works well.
  2. Import the fhir-smart (module version respective to the FHIR Server verison, downloaded from https://repo1.maven.org/maven2/org/linuxforhealth/fhir/fhir-smart) and pom.xml in ...\fhir-server\userlib directory. Content of the pom.xml:
    <?xml version="1.0" encoding="UTF-8"?>
    <project xmlns="http://maven.apache.org/POM/4.0.0"
        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    
        <modelVersion>4.0.0</modelVersion>
    
        <parent>
            <groupId>org.linuxforhealth.fhir</groupId>
            <artifactId>fhir-parent</artifactId>
            <version>5.1.0</version>
            <relativePath>../fhir-parent</relativePath>
        </parent>
    
        <artifactId>fhir-smart</artifactId>
    
        <packaging>jar</packaging>
    
        <properties>
        </properties>
    
        <dependencies>
    	    <dependency>
    	      <groupId>org.linuxforhealth.fhir</groupId>
    	      <artifactId>fhir-smart</artifactId>
    	      <version>5.1.0</version>
    	    </dependency>
        </dependencies>
    </project>
    
  3. Now test the scope validation of tokens. Create for the identity provider a client with the following allowed scope: "launch", "launch/patient", "openid", "profile", "fhirUser", "profile", "user/*.read", "user/Practitioner.read", "user/Practitioner.write".
  4. Get a token with following scopes: "fhirUser", "launch", "launch/patient", "user/Practitioner.read" and use this token to do GET request for all practitioners in Postman, the FHIR Server throws 500 Internal Server Error and NullPointerException, see screen shot

image

  1. See details in the following log

    [11/17/22, 8:30:44:505 UTC] 0000002c rity.authentication.filter.internal.AuthenticationFilterImpl I CWWKS4358I: The authentication filter filter configuration was successfully processed.
    [11/17/22, 8:30:44:893 UTC] 0000002c linuxforhealth.fhir.server.filter.rest.FHIRRestServletFilter I Received request: tenantId:[default] dsId:[default] user:[Postman] method:[GET] uri:[https://10.152.9.241/fhir-server/api/v4/Practitioner] originalUri:[https://10.152.9.241/fhir/Practitioner]
    [11/17/22, 8:30:45:247 UTC] 0000002c com.ibm.ws.recoverylog.spi.RecoveryDirectorImpl              I CWRLS0010I: Performing recovery processing for local WebSphere server (fhir-server).
    [11/17/22, 8:30:45:334 UTC] 0000002c com.ibm.ws.recoverylog.spi.RecoveryDirectorImpl              I CWRLS0012I: All persistent services have been directed to perform recovery processing for this WebSphere server (fhir-server).
    [11/17/22, 8:30:45:334 UTC] 00000046 com.ibm.tx.jta.impl.RecoveryManager                          I WTRN0135I: Transaction service recovering no transactions.
    [11/17/22, 8:30:45:391 UTC] 0000002c health.fhir.server.interceptor.FHIRPersistenceInterceptorMgr I Loaded the following persistence interceptors: [org.linuxforhealth.fhir.smart.AuthzPolicyEnforcementPersistenceInterceptor@58a3b55d]
    [11/17/22, 8:30:45:414 UTC] 0000002c com.ibm.ws.jca.cm.ConnectorService                           I J2CA8050I: An authentication alias should be used instead of defining a user name and password on dataSource[fhirDefaultDefault].
    [11/17/22, 8:30:46:289 UTC] 0000002c com.ibm.ws.rsadapter.impl.DatabaseHelper                     I DSRA8203I: Database product name : PostgreSQL
    [11/17/22, 8:30:46:292 UTC] 0000002c com.ibm.ws.rsadapter.impl.DatabaseHelper                     I DSRA8204I: Database product version : 13.4
    [11/17/22, 8:30:46:292 UTC] 0000002c com.ibm.ws.rsadapter.impl.DatabaseHelper                     I DSRA8205I: JDBC driver name  : PostgreSQL JDBC Driver
    [11/17/22, 8:30:46:292 UTC] 0000002c com.ibm.ws.rsadapter.impl.DatabaseHelper                     I DSRA8206I: JDBC driver version  : 42.5.0
    [11/17/22, 8:30:46:793 UTC] 0000002c nuxforhealth.fhir.persistence.jdbc.impl.CacheTransactionSync I Transaction failed - afterCompletion(status = STATUS_ROLLEDBACK)
    [11/17/22, 8:30:46:795 UTC] 0000002c org.linuxforhealth.fhir.server.resources.FHIRResource        E An unexpected exception occurred while processing the request
    java.lang.NullPointerException
        at java.base/java.util.HashSet.<init>(HashSet.java:119)
        at org.linuxforhealth.fhir.smart.AuthzPolicyEnforcementPersistenceInterceptor.getPatientIdFromToken(AuthzPolicyEnforcementPersistenceInterceptor.java:1068)
        at org.linuxforhealth.fhir.smart.AuthzPolicyEnforcementPersistenceInterceptor.afterSearch(AuthzPolicyEnforcementPersistenceInterceptor.java:657)
        at org.linuxforhealth.fhir.server.interceptor.FHIRPersistenceInterceptorMgr.fireAfterSearchEvent(FHIRPersistenceInterceptorMgr.java:169)
        at org.linuxforhealth.fhir.server.util.FHIRRestHelper.doSearch(FHIRRestHelper.java:1414)
        at org.linuxforhealth.fhir.server.util.FHIRRestHelper.doSearch(FHIRRestHelper.java:1355)
        at org.linuxforhealth.fhir.server.resources.Search.doSearch(Search.java:72)
        at org.linuxforhealth.fhir.server.resources.Search.searchGet(Search.java:49)
        at org.linuxforhealth.fhir.server.resources.Search$Proxy$_$$_WeldClientProxy.searchGet(Unknown Source)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:566)
        at com.ibm.ws.jaxrs20.cdi.component.JaxRsFactoryImplicitBeanCDICustomizer.serviceInvoke(JaxRsFactoryImplicitBeanCDICustomizer.java:350)
        at com.ibm.ws.jaxrs20.server.LibertyJaxRsServerFactoryBean.performInvocation(LibertyJaxRsServerFactoryBean.java:641)
        at com.ibm.ws.jaxrs20.server.LibertyJaxRsInvoker.performInvocation(LibertyJaxRsInvoker.java:160)
        at org.apache.cxf.service.invoker.AbstractInvoker.invoke(AbstractInvoker.java:101)
        at com.ibm.ws.jaxrs20.server.LibertyJaxRsInvoker.invoke(LibertyJaxRsInvoker.java:273)
        at org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:213)
        at com.ibm.ws.jaxrs20.server.LibertyJaxRsInvoker.invoke(LibertyJaxRsInvoker.java:444)
        at org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:112)
        at org.apache.cxf.interceptor.ServiceInvokerInterceptor$1.run(ServiceInvokerInterceptor.java:59)
        at org.apache.cxf.interceptor.ServiceInvokerInterceptor.handleMessage(ServiceInvokerInterceptor.java:96)
        at org.apache.cxf.phase.PhaseInterceptorChain.doIntercept(PhaseInterceptorChain.java:308)
        at org.apache.cxf.transport.ChainInitiationObserver.onMessage(ChainInitiationObserver.java:123)
        at org.apache.cxf.transport.http.AbstractHTTPDestination.invoke(AbstractHTTPDestination.java:277)
        at com.ibm.ws.jaxrs20.endpoint.AbstractJaxRsWebEndpoint.invoke(AbstractJaxRsWebEndpoint.java:137)
        at com.ibm.websphere.jaxrs.server.IBMRestServlet.handleRequest(IBMRestServlet.java:146)
        at com.ibm.websphere.jaxrs.server.IBMRestServlet.doGet(IBMRestServlet.java:112)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:686)
        at com.ibm.websphere.jaxrs.server.IBMRestServlet.service(IBMRestServlet.java:96)
        at com.ibm.ws.webcontainer.servlet.ServletWrapper.service(ServletWrapper.java:1258)
        at com.ibm.ws.webcontainer.servlet.ServletWrapper.handleRequest(ServletWrapper.java:746)
        at com.ibm.ws.webcontainer.servlet.ServletWrapper.handleRequest(ServletWrapper.java:443)
        at com.ibm.ws.webcontainer.filter.WebAppFilterChain.invokeTarget(WebAppFilterChain.java:193)
        at com.ibm.ws.webcontainer.filter.WebAppFilterChain.doFilter(WebAppFilterChain.java:98)
        at org.linuxforhealth.fhir.server.filter.rest.FHIRRestServletFilter.doFilter(FHIRRestServletFilter.java:177)
        at javax.servlet.http.HttpFilter.doFilter(HttpFilter.java:127)
        at com.ibm.ws.webcontainer.filter.FilterInstanceWrapper.doFilter(FilterInstanceWrapper.java:201)
        at com.ibm.ws.webcontainer.filter.WebAppFilterChain.doFilter(WebAppFilterChain.java:91)
        at com.ibm.ws.security.jaspi.JaspiServletFilter.doFilter(JaspiServletFilter.java:56)
        at com.ibm.ws.webcontainer.filter.FilterInstanceWrapper.doFilter(FilterInstanceWrapper.java:201)
        at com.ibm.ws.webcontainer.filter.WebAppFilterChain.doFilter(WebAppFilterChain.java:91)
        at com.ibm.ws.webcontainer.filter.WebAppFilterManager.doFilter(WebAppFilterManager.java:1002)
        at com.ibm.ws.webcontainer.filter.WebAppFilterManager.invokeFilters(WebAppFilterManager.java:1140)
        at com.ibm.ws.webcontainer.webapp.WebApp.handleRequest(WebApp.java:5058)
        at com.ibm.ws.webcontainer.osgi.DynamicVirtualHost$2.handleRequest(DynamicVirtualHost.java:316)
        at com.ibm.ws.webcontainer.WebContainer.handleRequest(WebContainer.java:1007)
        at com.ibm.ws.webcontainer.osgi.DynamicVirtualHost$2.run(DynamicVirtualHost.java:281)
        at com.ibm.ws.http.dispatcher.internal.channel.HttpDispatcherLink$TaskWrapper.run(HttpDispatcherLink.java:1239)
        at com.ibm.ws.http.dispatcher.internal.channel.HttpDispatcherLink.wrapHandlerAndExecute(HttpDispatcherLink.java:468)
        at com.ibm.ws.http.dispatcher.internal.channel.HttpDispatcherLink.ready(HttpDispatcherLink.java:427)
        at com.ibm.ws.http.channel.internal.inbound.HttpInboundLink.handleDiscrimination(HttpInboundLink.java:566)
        at com.ibm.ws.http.channel.internal.inbound.HttpInboundLink.handleNewRequest(HttpInboundLink.java:500)
        at com.ibm.ws.http.channel.internal.inbound.HttpInboundLink.processRequest(HttpInboundLink.java:360)
        at com.ibm.ws.http.channel.internal.inbound.HttpInboundLink.ready(HttpInboundLink.java:327)
        at com.ibm.ws.channel.ssl.internal.SSLConnectionLink.determineNextChannel(SSLConnectionLink.java:1100)
        at com.ibm.ws.channel.ssl.internal.SSLConnectionLink.readyInboundPostHandshake(SSLConnectionLink.java:757)
        at com.ibm.ws.channel.ssl.internal.SSLConnectionLink$MyHandshakeCompletedCallback.complete(SSLConnectionLink.java:427)
        at com.ibm.ws.channel.ssl.internal.SSLUtils.handleHandshake(SSLUtils.java:954)
        at com.ibm.ws.channel.ssl.internal.SSLHandshakeIOCallback.complete(SSLHandshakeIOCallback.java:85)
        at com.ibm.ws.tcpchannel.internal.WorkQueueManager.requestComplete(WorkQueueManager.java:514)
        at com.ibm.ws.tcpchannel.internal.WorkQueueManager.attemptIO(WorkQueueManager.java:584)
        at com.ibm.ws.tcpchannel.internal.WorkQueueManager.workerRun(WorkQueueManager.java:968)
        at com.ibm.ws.tcpchannel.internal.WorkQueueManager$Worker.run(WorkQueueManager.java:1057)
        at com.ibm.ws.threading.internal.ExecutorServiceImpl$RunnableWrapper.run(ExecutorServiceImpl.java:245)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
        at java.base/java.lang.Thread.run(Thread.java:829)
    
    [11/17/22, 8:30:46:843 UTC] 0000002c org.linuxforhealth.fhir.audit.AuditLogServiceFactory         I Successfully initialized audit log service: org.linuxforhealth.fhir.audit.impl.NopService
    [11/17/22, 8:30:46:899 UTC] 0000002c linuxforhealth.fhir.server.filter.rest.FHIRRestServletFilter I Completed request[2.0081932 secs]: tenantId:[default] dsId:[default] user:[Postman] method:[GET] uri:[https://10.152.9.241/fhir-server/api/v4/Practitioner] originalUri:[https://10.152.9.241/fhir/Practitioner] status:[500]
    

Expected behavior
It should be forbidden to read Patient (which is working as expected as the Postman shows below), but it should be possible to read Practitioner instead of the NullPointerException.
Postman looks like:
image

Additional context
Tested also the same process with FHIR Server version 4.11.1, where the scope validation works all as expected.

@qing-xia-snkeos qing-xia-snkeos added the bug Something isn't working label Nov 17, 2022
@lmsurpre
Copy link
Member

lmsurpre commented Nov 17, 2022

thank you for the detailed bug report @qing-xia-snkeos

what are you using for your auth server? can you confirm what your JWT access token looks like? does it have a patient_id claim?

If using the 'keycloak extensions for fhir' project, I would have expected the claims you mention to result in a token with a patient_id claim (because launch/patient scope was included).

if instead there is no patient_id claim, then i would expect the implementation to hit line 1063 from the following method
image

instead its getting to line 1068 for some reason.
IIRC we updated our JWT processing to match an update to the auth0.jwt library and I suspect thats the culprit here.

@lmsurpre lmsurpre changed the title NullPointerException when accessing resources in the allowed scopes with fhir-smart enabled NullPointerException with fhir-smart enabled when patient_id claim is missing in access token Nov 17, 2022
@lmsurpre lmsurpre added the P1 Priority 1 - Must Have label Nov 17, 2022
@qing-xia-snkeos
Copy link
Author

Thank you for the quick reply. You are absolutely right. The patient_id claim is not in our token.

I have the following questions:

  • Is this patient_id a standard compliant for Smart APP Launch?
  • What should I give for this patient_id in the token exactly?
    Thank you :)

lmsurpre added a commit that referenced this issue Nov 17, 2022
Signed-off-by: Lee Surprenant <lmsurpre@merative.com>
lmsurpre added a commit that referenced this issue Nov 17, 2022
Signed-off-by: Lee Surprenant <lmsurpre@merative.com>
@lmsurpre
Copy link
Member

lmsurpre commented Nov 17, 2022

Is this patient_id a standard compliant for Smart APP Launch?

The SMART specs do not concern themselves with the coordination between the auth server and the resource server.
For most EHRs, the auth service is tightly coupled to the rest of the platform.
However, for generic auth servers / fhir servers we need to fill this gap with our own implementation.

I believe we originally got the idea of using a patient_id claim in the access token from a MITRE-developed reference implementation.

Net:

  • is it dictated by the standard? no
  • is it compliant with the standard? yes

What should I give for this patient_id in the token exactly?

We actually support two formats:

  • a string claim
  • a list of strings (json list)

In either case the actual string value must be the Resource.id of the "in-context" patient (or patients in the case of a list).

What little documentation we have on this can be found at https://linuxforhealth.github.io/FHIR/guides/FHIRServerUsersGuide#533-smart-app-launch

lmsurpre added a commit that referenced this issue Nov 18, 2022
* issue #4083 - handle missing patient_id claim

Signed-off-by: Lee Surprenant <lmsurpre@merative.com>

* Update fhir-smart/src/test/java/org/linuxforhealth/fhir/smart/test/AuthzPolicyEnforcementTest.java

per review feedback

Signed-off-by: Lee Surprenant <lmsurpre@merative.com>

* Update fhir-smart/src/test/java/org/linuxforhealth/fhir/smart/test/AuthzPolicyEnforcementTest.java

per review

Signed-off-by: Lee Surprenant <lmsurpre@merative.com>

Signed-off-by: Lee Surprenant <lmsurpre@merative.com>
@lmsurpre lmsurpre self-assigned this Nov 18, 2022
@lmsurpre
Copy link
Member

We've merged the fix for the NullPointerException into main.
If you'd like to help us confirm it, please pull the latest (https://github.com/LinuxForHealth/FHIR/wiki/Setting-up-for-development may be helpful).

You'll still need to figure out how to create that patient_id claim in your access tokens for any patient/[resourceType].[permission] scopes, but hopefully with this change you can use user/[resourceType].[permission] scopes without it.

@lmsurpre lmsurpre changed the title NullPointerException with fhir-smart enabled when patient_id claim is missing in access token NullPointerException with fhir-smart when patient_id claim is missing in access token Nov 18, 2022
@qing-xia-snkeos
Copy link
Author

We've merged the fix for the NullPointerException into main. If you'd like to help us confirm it, please pull the latest (https://github.com/LinuxForHealth/FHIR/wiki/Setting-up-for-development may be helpful).

You'll still need to figure out how to create that patient_id claim in your access tokens for any patient/[resourceType].[permission] scopes, but hopefully with this change you can use user/[resourceType].[permission] scopes without it.

Thank you very much! I will find a moment next week to test this change :)

@PrasannaHegde1
Copy link
Collaborator

PrasannaHegde1 commented Dec 13, 2022

Generated a token with following scopes: "fhirUser", "launch", "launch/patient", "user/Practitioner.read" without the patient_id claim and used it to do GET request for all practitioners. The request is returning the list of Practitioners as expected.

image

Generated a token with following scopes: "fhirUser", "launch", "launch/patient", "user/Practitioner.read" with the patient_id claim and used it to do GET request for all practitioners. The request is returning the list of Practitioners as expected.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P1 Priority 1 - Must Have
Projects
None yet
Development

No branches or pull requests

3 participants