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.

Sign up for GitHub

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

Jump to bottom

Introduce ".well-known/smart-configuration", change liberty config layout, and update user's guide #939

Merged
merged 7 commits into from
Apr 22, 2020
Merged

Introduce ".well-known/smart-configuration", change liberty config layout, and update user's guide #939

merged 7 commits into from
Apr 22, 2020

Conversation

lmsurpre
Copy link
Member

@lmsurpre lmsurpre commented Apr 17, 2020 •
edited
Loading

  1. Added lots of details to the OAuth 2.0 section of the user's guide

  2. Introduced a new JAX-RS resource for the
    [base]/.well-known/smart-configuration endpoint which is expected by
    SMART apps. Left TODOs to implement the various smart-on-fhir capabilities.

  3. Replaced fhirKeystore.jks with a newly-generated fhirKeyStore.p12; the
    new private key in this keystore can be used for encrypting JWT tokens.

  4. Converted clientKeystore.jks to .p12 and updated all client and
    server truststores.

  5. Removed the commented-out config in our default server.xml and
    introduced oidcProvider.xml and oauthResourceServer.xml at
    fhir-server/libertyConfig/configDropins/disabled to prep for enabling
    OpenId Connection / OAuth supported (perhaps behind some flag).

  6. Moved batchDS.xml to configDropins/defaults, added the rest of the
    bulkdata config to it, and renamed to bulkdata.xml

  7. Removed the fhir-install docker build from the default maven build...now you need to explicitly ask for it via either a docker build or using the following maven command:

mvn dockerfile:build -f fhir-install
  1. issue Improve the docker image #923 - split keystore config into separate config file
    This tells the openliberty docker.ci process not to bother with creating
    a separate keystore, thereby removing the conflicting values warning:
  Property password has conflicting values:
      Secure value is set in
    file:/opt/ol/wlp/usr/servers/fhir-server/configDropins/defaults/keystore.xml.
      Secure value is set in
    file:/opt/ol/wlp/usr/servers/fhir-server/server.xml.
  1. Move Liberty OAuth tables to the FHIR_OAUTH schema and gen by default
    Previously, a user needed to create these tables themselves if they
    wanted to use liberty as an OAuth 2.0 provider with a databaseStore
    (e.g. to dynamically manage OAuth 2.0 clients).
    Now these tables will be created/updated as part of the
    fhir-persistence-schema create-schema/update-schema actions, or as part
    of derby bootstrapping.

Signed-off-by: Lee Surprenant lmsurpre@us.ibm.com

prb112
prb112 reviewed Apr 17, 2020
View reviewed changes
docs/src/pages/guides/FHIRServerUsersGuide.md Outdated
CREATE INDEX OAUTH20CACHE_EXPIRES ON OAUTHDBSCHEMA.OAUTH20CACHE (EXPIRES ASC);
```

3. Attempting to register a client (based on https://www.ibm.com/support/knowledgecenter/SSEQTP_liberty/com.ibm.websphere.wlp.doc/ae/twlp_client_registration.html). The endpoint is determined by the id value of the oauthProvider element from the previous step.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
3. Attempting to register a client (based on https://www.ibm.com/support/knowledgecenter/SSEQTP_liberty/com.ibm.websphere.wlp.doc/ae/twlp_client_registration.html). The endpoint is determined by the id value of the oauthProvider element from the previous step.
3. Attempting to register a client (based on the [Client Registration Article](https://www.ibm.com/support/knowledgecenter/SSEQTP_liberty/com.ibm.websphere.wlp.doc/ae/twlp_client_registration.html)). The endpoint is determined by the id value of the oauthProvider element from the previous step.

prb112
prb112 reviewed Apr 17, 2020
View reviewed changes
docs/src/pages/guides/FHIRServerUsersGuide.md Outdated
/>

<authFilter id="filter">
<requestUrl urlPattern="/fhir-server" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<requestUrl urlPattern="/fhir-server" />
<requestUrl urlPattern="/fhir-server" />

should we make a note about bulkimport not being client usable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should figure out how to secure that endpoint via OAuth too...not totally sure how to handle the different groups yet.

prb112
prb112 approved these changes Apr 17, 2020
View reviewed changes
Copy link
Contributor

@prb112 prb112 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@prb112
Copy link
Contributor

prb112 commented Apr 17, 2020

I marked LGTM (however the comments above are optional and hope to improve the contents)

@lmsurpre lmsurpre changed the title WIP: Introduce ".well-known/smart-configuration" and update user's guide Introduce ".well-known/smart-configuration", change liberty config layout, and update user's guide Apr 20, 2020
@lmsurpre lmsurpre force-pushed the lee-master branch 3 times, most recently from e674bde to 98daf11 Compare April 21, 2020 22:25
lmsurpre and others added 5 commits April 21, 2020 22:10
@lmsurpre
Introduce ".well-known/smart-configuration" and update user's guide
f153c64 
1. Added lots of details to the OAuth 2.0 section of the user's guide

2. Introduces a new JAX-RS resource for the
`[base]/.well-known/smart-configuration` endpoint which is expected by
SMART apps.  But currently it claims we support a lot more than we
actually do...

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre @prb112
Apply suggestions from code review
232e4c5 
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>

Co-Authored-By: Paul Bastide <pbastide@us.ibm.com>
@lmsurpre
Update keystores and introduce oauth configDropins
50172cc 
1. Replace fhirKeystore.jks with a newly-generated fhirKeyStore.p12; the
new private key in this keystore can be used for encrypting JWT tokens.

2. Update clientKeystore.jks to .p12 and update both the client and
server truststores.

3. Removed the commented-out config in our default server.xml and
introduced `oauthProvider.xml` and `oauthResourceServer.xml` at
fhir-server/libertyConfig/configDropins/disabled to prep for enabling
OpenId Connection / OAuth supported (perhaps behind some flag).

4. Moved batchDS.xml to configDropins/defaults, added the rest of the
bulkdata config to it, and renamed to bulkdata.xml

5. Update the user's guide

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre
issue #923 - split keystore config into separate config file
794aafe 
This tells the openliberty docker.ci process not to bother with creating
a separate keystore, thereby removing the conflicting values warning:
```
  Property password has conflicting values:
    Secure value is set in
file:/opt/ol/wlp/usr/servers/fhir-server/configDropins/defaults/keystore.xml.
    Secure value is set in
file:/opt/ol/wlp/usr/servers/fhir-server/server.xml.
```

I also added a version tag (20.0.0.3) to our Dockerfile to make our
builds more consistent...this should be kept in-sync with our
liberty-runtime dependency version in `fhir-parent/pom.xml`.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre
Walk back the claims in WellKnown smart-configuration
916927a 
I also moved the keystore config back into server.xml to fix warnings
generated by tools that aren't dropinConfig-aware.

Finally, I removed the dockerfile-maven-plugin from the default build.
Now you must invoke the docker build yourself after doing the main build.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre
Move Liberty OAuth tables to the FHIR_OAUTH schema and gen by default
567dc9b 
Previously, a user needed to create these tables themselves if they
wanted to use liberty as an OAuth 2.0 provider with a databaseStore
(e.g. to dynamically manage OAuth 2.0 clients).

Now these tables will be created/updated as part of the
fhir-persistence-schema create-schema/update-schema actions, or as part
of derby bootstrapping.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre
Update fhir-server/liberty-config/server.xml
15470b4 
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
albertwang-ibm
albertwang-ibm approved these changes Apr 22, 2020
View reviewed changes
Copy link
Contributor

@albertwang-ibm albertwang-ibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lmsurpre lmsurpre merged commit 4ebdeba into master Apr 22, 2020
@lmsurpre lmsurpre deleted the lee-master branch April 22, 2020 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prb112 prb112 prb112 approved these changes

@albertwang-ibm albertwang-ibm albertwang-ibm approved these changes

@JohnTimm JohnTimm Awaiting requested review from JohnTimm

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lmsurpre @prb112 @albertwang-ibm