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

40 : Add keycloak authentication in jpa server starter #1

Merged
merged 7 commits into from
Jun 28, 2021

Conversation

rehammuzzamil
Copy link

@rehammuzzamil rehammuzzamil commented Jun 11, 2021

opensrp/fhircore#40

To build:

  1. Update keycloak details from the stage Keycloak server in application.yml
  2. Add http://localhost:8080/* as a redirect URL on the stage Keycloak server for this realm and client
  3. Build hapi-fhir-opensrp-security-config package from the hapi-fhir repo (Reference PR)
  4. Build and run jpa-server-starter project using Pboot profile. Deploy on Tomcat server.
  5. Visit http://localhost:8080/fhir/rest/Patient/1 on your browser. This should currently redirect to the Keycloak login page. The valid response is returned with a status code of 200 when the correct login credentials are entered.
  6. Visit http://localhost:8080/fhir/rest/Patient/1 on Postman with the correct Oauth token to fetch the response.

Please note : Authentication in test-page-overlay (web UI) is not a part of this PR.

pom.xml Outdated
<dependency>
<groupId>ca.uhn.hapi.fhir</groupId>
<artifactId>hapi-fhir-opensrp-security-config</artifactId>
<version>5.4.0-PRE5-SNAPSHOT</version>

Choose a reason for hiding this comment

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

The version for this should be start from 0.0.1

Copy link
Author

Choose a reason for hiding this comment

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

Done, here is the new repo.

Comment on lines -3 to +9
url: 'jdbc:h2:file:./target/database/h2'
url: 'jdbc:postgresql://localhost:5432/hapi_fhir'
#url: jdbc:h2:mem:test_mem
username: sa
password: null
driverClassName: org.h2.Driver
username: postgres
password: root
driverClassName: org.postgresql.Driver

Choose a reason for hiding this comment

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

The secrets in this should be moved to a private repository or secrets file. @dubdabasoduba will liase with SRE and decided how to move forward with this

Copy link
Member

Choose a reason for hiding this comment

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

@manutarus @bennsimon Do you have an idea of how to use secret files to provide these creds?

src/main/resources/application.yaml Show resolved Hide resolved
@@ -52,7 +55,7 @@ public ServletRegistrationBean hapiServletRegistration() {
JpaRestfulServer jpaRestfulServer = new JpaRestfulServer();
beanFactory.autowireBean(jpaRestfulServer);
servletRegistrationBean.setServlet(jpaRestfulServer);
servletRegistrationBean.addUrlMappings("/fhir/*");
servletRegistrationBean.addUrlMappings("/fhir/rest/*");
Copy link

@ekigamba ekigamba Jun 15, 2021

Choose a reason for hiding this comment

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

I am not sure why this change was made. Could you help me understand why this change is needed?

Copy link
Author

Choose a reason for hiding this comment

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

@ekigamba this change was made to make the test page overlay work.
As context for web UI and API both were same starting with /fhir therefore we have added /fhir/rest to distinguish the context for the APIs. With this approach, we have disabled the web security on /fhir.

But test page overlay still does not support Authentication and is broken when we try to hit API from there. We may think to remove it as it was before and fix the test page overlay in some other issue.

Let me re-test it after removing the /rest from the context.

cc: @maimoonak @dubdabasoduba

Choose a reason for hiding this comment

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

Cool

Copy link
Author

Choose a reason for hiding this comment

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

Updated back to the original one. @ekigamba
cc : @maimoonak

@ekigamba
Copy link

I have tested this locally with the stage Keycloak instance and it works as described on the PR description 👍 . The overlay page is not working as described

@rehammuzzamil rehammuzzamil requested a review from ekigamba June 15, 2021 11:49
Comment on lines -3 to +9
url: 'jdbc:h2:file:./target/database/h2'
url: 'jdbc:postgresql://localhost:5432/hapi_fhir'
#url: jdbc:h2:mem:test_mem
username: sa
password: null
driverClassName: org.h2.Driver
username: postgres
password: root
driverClassName: org.postgresql.Driver
Copy link
Member

Choose a reason for hiding this comment

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

@manutarus @bennsimon Do you have an idea of how to use secret files to provide these creds?

src/main/resources/application.yaml Show resolved Hide resolved
@dubdabasoduba dubdabasoduba self-requested a review June 28, 2021 11:51
@rehammuzzamil
Copy link
Author

As discussed, we can merge this PR for now. Discussion for encrypting secrets will be done afterward.

@rehammuzzamil rehammuzzamil merged commit 5e39957 into master Jun 28, 2021
rehammuzzamil pushed a commit that referenced this pull request Nov 28, 2023
Fixing some typos in README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants