-
Notifications
You must be signed in to change notification settings - Fork 495
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
9228 - add OIDC development setup for OIDC login feature testing #9234
Conversation
…ealm initialization
…client config changes to make it suitable for Dataverse
I gave this a size of 3. As long as a developer has Docker installed, it should be pretty easy to spin up Keycloak, add the config, and to a quick test. The docs @GPortas wrote are excellent. |
Prioritization note: |
Corrected. This is part of: "NIH OTA: 1.7.1" and so is part of the NIH Backlog items. |
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.
I tested this and it works amazingly well. Thank you, @GPortas!! I've wanted something like this for over three years, when OIDC support was added in PR #6433 by @poikilotherm and I never had an easy way to test it myself.
It's inconvenient to add screenshots and details in the review box but I'll add another comment in a minute. I did make a couple tiny doc tweaks.
Approved. Thanks again! 🎉 🚀
This pull request is a dream come true. Just works! I did make a couple tiny tweaks to the docs (formatting stuff and a security warning not to run this config it prod since the client secret is in GitHub). The Bash option was listed first so I tried it first. Worked great. Here's the output: $ cd conf/keycloak I got a working Keycloak instance at http://localhost:8090 which looked like this: Then I loaded up the auth provider: $ curl -X POST -H 'Content-type: application/json' --upload-file oidc-keycloak-auth-provider.json http://localhost:8080/api/admin/authenticationProviders Here are screenshots of the login process: One thing to note above is that no username was prepopulated (given name, family name, and email were). @poikilotherm already wrote this bug up here: This PR means that issue will be MUCH easier for a developer to work on! 🎉 During the auth meeting today I said I was curious what gets stored as a Here's the full output of a dump of this keycloak user:
I also tested the rm-keycloak.sh script and the docker compose option. It all worked great. Thanks again, @GPortas! |
Very nice step-by-step guide with screenshots and good information about Thank you @pdurbin! |
Updated information.
|
I said I would pick this up during the auth meeting last week as we discussed the Dataverse - SPA Authentication V1 doc. It seemed absolutely ready to merge. High value (extremely useful for ongoing auth work). Zero risk. So I went ahead and moved it to QA. Next time, I'm happy to simply merge it if that's better. I was certainly tempted! 😄 |
What this PR does / why we need it:
As stated in #9228, developers currently don't have a way to test the login with OIDC feature of Dataverse.
This PR includes a dockerized Keycloak setup with OIDC support for development purposes, as well as its associated documentation.
TODOs.
Which issue(s) this PR closes:
Closes #9228
Special notes for your reviewer:
Not yet.
Suggestions on how to test this:
Follow the next steps:
/conf/keycloak/
) Run Keycloak docker container: You can use docker-compose file or run-keycloak.sh script./conf/keycloak/
) Execute the following API call which enables the Keycloak OIDC client as an authentication provider for Dataverse:curl -X POST -H 'Content-type: application/json' --upload-file oidc-keycloak-auth-provider.json http://localhost:8080/api/admin/authenticationProviders
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No.
Is there a release notes update needed for this change?:
Not sure.
Additional documentation:
Not yet.