Skip to content
This repository has been archived by the owner on Jan 23, 2024. It is now read-only.

Conformance statement results are hard to debug #5

Open
jmandel opened this issue May 31, 2018 · 7 comments
Open

Conformance statement results are hard to debug #5

jmandel opened this issue May 31, 2018 · 7 comments

Comments

@jmandel
Copy link
Contributor

jmandel commented May 31, 2018

When I test https://portal.demo.syncfor.science/api/fhir I get

image
https://portal.demo.syncfor.science/api/fhir/metadata includes

      "security": {
        "extension": [
          {
            "extension": [
              {
                "url": "authorize", 
                "valueUri": "https://portal.demo.syncfor.science/oauth/authorize"
              }, 
              {
                "url": "manage", 
                "valueUri": "https://portal.demo.syncfor.science/apps"
              }, 
              {
                "url": "token", 
                "valueUri": "https://portal.demo.syncfor.science/oauth/token"
              }, 
              {
                "url": "register", 
                "valueUri": "https://portal.demo.syncfor.science/oauth/register"
              }
            ], 
            "url": "http://fhir-registry.smarthealthit.org/StructureDefinition/oauth-uris"
          }
        ]
      }

... which I think should be valid. Clicking on the "Details: No authorize URI provided in conformance statement." error line turns the screen grey but does not convey additional information about what went wrong.

@arscan
Copy link
Contributor

arscan commented May 31, 2018

Thanks for the report @jmandel. Is the 'gray screen' issue on the public hosted instance of this on SITE or a copy you are running locally? If it is on the public instance, this was fixed in onc-healthit/legacy-inferno-deployment#4, but hasn't yet been deployed.

I believe the test is failing because the conformance statement does not explicitly declare SMART-on-FHIR as a supported security service in Conformance.rest.security.service. Documentation in smart app launch seems vague about whether or not it is required, but the example included that field.

We ended up making it required, since it is a reasonable assumption from a client dev perspective (and an assumption we made when making our client). What are your thoughts on this?

At the very least, this error message should be more explicit (missing security service, not missing oauth uri).

@jmandel
Copy link
Contributor Author

jmandel commented May 31, 2018

Thanks @arscan! I think ideally we'd make this required, but our docs don't say this today, and I'd rather not break compatibility over this. We should review what existing Argonaut vendors do here, for example. If there's consistent support, we can bring this up as we're finalizing ballot reconciliation of the SMART spec through HL7.

@jmandel
Copy link
Contributor Author

jmandel commented May 31, 2018

(Oh, and this is on the public site -- thanks.)

@arscan
Copy link
Contributor

arscan commented May 31, 2018

I would strongly prefer it to be required... after all, it caused a "bug" in our client (that misleading error message). I assume others will make the same mistake, especially if it is in the reference example. And even if it is removed from the reference example, it will still exist in the restful-security-service value set.

I believe that most argo vendors already use it, though I can't say with certainty at the moment. Even if they don't, this seems like it should be an easy change (famous last words).

The only other time I recall seeing this missing is in the Blue Button 2.0 API. But they aren't argo, and it doesn't claim SMART-on-FHIR conformance in their documentation either (though it does quack like a SoF enabled API).

@jmandel
Copy link
Contributor Author

jmandel commented May 31, 2018

Thanks for this. The BB 2.0 API aims to follow the SMART on FHIR guide, so it's worth including this in the roundup/assessment.

In any case, I too would prefer if it was required.

@johnrsnyder
Copy link
Contributor

This should be decided when finalizing the ballot reconciliation of the SMART spec through HL7. Meanwhile, let's adjust the Inferno test tool to return a message that says "Missing security service in conformance statement" when the URI is present but it does not explicitly declare SMART-on-FHIR as a supported security service in Conformance.rest.security.service.

@aviars
Copy link
Contributor

aviars commented Jun 7, 2018

I wanted to point out that https://portal.demo.syncfor.science/api/fhir/metadata is now including this information.
...

"service": [
      {
        "coding": [
          {
            "code": "SMART-on-FHIR", 
            "system": "http://hl7.org/fhir/restful-security-service"
          }
        ], 
        "text": "OAuth2 using SMART-on-FHIR profile (see http://docs.smarthealthit.org)"
      }
    ]
   ....

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants