-
Notifications
You must be signed in to change notification settings - Fork 6
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
FHIR: Add support for SMART & UDAP authentication #575
Conversation
Refactor base test to enable usage of various install bits from other, non-subclassed tests,
…perties. Enhance parameter validation to take these into account. Added `ApplicationManaged` to values for authenticationMechanism. This indicates that the user expects (generally) to call the useAccessToken() entry point. It is intended to be used with SMART & UDAP-based server authorization.
This is used for various authentication settings.
Seen in passing...
Reopened -- was prematurely submitted... |
@@ -233,8 +233,8 @@ However, if importing many projects, use `../gradlew importAssemblies -PcamelAss | |||
where `<profileName>` is the name of a Vantiq CLI profile that will connect to your publishing namespace. Other | |||
gradle properties that are used are based on that profile name, and are as follows. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting typos that hid important information
@@ -132,16 +132,38 @@ outline those defined below. These are all found in the package `com.vantiq.fhir | |||
* `id` -- (String, Required) The id of the resource instance to be read | |||
* `versionId` -- (String) The version id to be read. If missing, this operates the same as a `read` procedure. | |||
* `modifiers` -- (Modifiers) Any modifiers to the interaction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor reorganization to divide things into categories.
exception("com.vantiq.fhir.basicauth.required", | ||
"A username and password are required for Basic authentication: {0}", [theConfig]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tweak auth info validation with new items.
Added AHA for the ability to check some of these more dynamic things via rules or conditionals for configuration parameters. By the time we check here it's really too late, but it's the only chance we get.
@@ -0,0 +1,17 @@ | |||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FHIR servers define some well-known places to pick up configuration information. Provide built-in means to fetch these.
if (modifiers.headers) { | ||
headers = modifiers.headers | ||
} | ||
var headers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add ability to send our application managed access token if present.
@@ -163,18 +163,36 @@ public static void performSetup(Map<String, ?> assemblyConfig) { | |||
((JsonElement) targetAssembly.get("name")).getAsString(), | |||
((JsonElement) targetAssembly.get("name")).getAsString(), | |||
FHIR_ASSY_NAME); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor a bit to allow us to reuse part of the set up in another test.
import java.util.Map; | ||
import java.util.concurrent.TimeUnit; | ||
|
||
@Slf4j |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes use of an apache test framework that sets up an http server for each test. Using that, we can make our calls & return the headers supplied, which we can then validate.
void doOperations(String tType, String tValue, Map<String, ?> assyConfig) { | ||
doOperations(tType, tValue, assyConfig, false); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make a series of so-called FHIR calls that span the different underlying call mechanisms we use. Our code doesn't interpret the FHIR Json coming back, so just passing back the map of the headers is fine here. And we use those to check that we send what was expected.
params.put("type", "Patient"); | ||
resp = v.execute("com.vantiq.fhir.fhirService.read", params); | ||
assertTrue("Could not read resource: " + resp.getErrors(), resp.isSuccess()); | ||
//noinspection unchecked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The call above just returns whatever object is at that address. the other calls are a bit different.
"resource", ourHeroMap, | ||
"modifiers", modifiers)); | ||
assertTrue("Could not create our hero: " + resp.getErrors(), resp.isSuccess()); | ||
//noinspection unchecked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The call above returns an Object that contains the status code, body, and any response headers. So we need to get the body part of that.
The remaining FHIR calls in this method operate the same way.
No rush no this -- I can merge it next week some time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
saw a couple typos
fhirAssembly/README.md
Outdated
or `previous` links), but this procedure is agnostic with respect to the purpose of the _link_. | ||
* `link` -- (String, Required) The link to be returned | ||
|
||
In addition to the _interaction_-related procedures above, there are a number of procedures that can be used to | ||
fetch about the FHIR server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something missing?
"used to fetch WHAT about the FHIR server"
Map<String, ?> modifiers = Map.of("headers", | ||
Map.of("If-None-Exist", "name=" + "Man")); | ||
|
||
// Now, create our here again, but conditional on it's not being already there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: here -> hero
Fixes #569
...and it's minions:
Fixes #570
Fixes #571
Fixes #572
Fixes #573
Fixes #574
Adds support, albeit indirect, for SMART & UDAP authentication. Both of these are layered atop the OAuth authorization flow grant type/flow that is intended to allow for direct user interaction to authorized the connection. As such, it is improper for the server to attempt to bypass this interaction. I looked at some sort of call back, but that was more confusing than it seemed worth.
So, what we provide is a mechanism by which our caller can tell use to user a certain access token (and, optionally, a token type). This is known as an application managed authentication mechanism. We expect this to be used for handling SMART and UDAP situations, but our callers can, indeed, use this whenever they wish.
Enhanced assembly configuration appropriately. While, fixed a couple of inadvertent omissions -- the ability to supply an access token & realm for Basic authentication.
Added documentation & tests for these changes as well.
While doing some unrelated work, found formatting bugs in the Camel Assemblies README that hid some important information. Fixed those whilst nearby.