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

Feature/354 openid auth #1007

Merged
merged 14 commits into from
Feb 21, 2025
Merged

Feature/354 openid auth #1007

merged 14 commits into from
Feb 21, 2025

Conversation

MikeNeilson
Copy link
Contributor

@MikeNeilson MikeNeilson commented Jan 10, 2025

Adds support for initial user creation from a trusted JWT.

NOTES:

  • Requires "execute on cwms_upass" for the web_user role.
  • Still some work to be done on mapping claims, some will be ignored but the test keycloak just doesn't have several configured at the moment.

Uses the "principle_name" (note: spelled incorrectly in database) field to store the combination of JWT issuer and subject claim . The subject is a UUID and so the combination of issue+subject is always unique. This was done after discovering that the one of the test infrastructure OIDC providers doesn't include the CAC EDIPI anywhere and it could not be used for user lookup.

Using a full identity provider principal is better anyways, so may as well start here.

Additionally updates the docker-compose to require no external setup. This should make testing and development far easier for newcomers to the project.

@MikeNeilson
Copy link
Contributor Author

Still thinking of ways to automate testing this without going overboard.

@MikeNeilson MikeNeilson marked this pull request as ready for review February 5, 2025 15:23
@MikeNeilson MikeNeilson marked this pull request as draft February 5, 2025 15:25
* Add extension to start keycloak instance.
* Configure OpenID parameters for test CDA instance.
* Add additional user to keycloak that intentionally doesn't have privileges
* Verify keycloak user with priveleges works.
@MikeNeilson MikeNeilson marked this pull request as ready for review February 6, 2025 22:36
@MikeNeilson MikeNeilson added the approved-W192HQ23F0232-task4 Only valid if set by MikeNeilson, DanielO, CharlesG label Feb 6, 2025
@rma-rripken
Copy link
Collaborator

I'm not in a position to test this on my own. The changes look ok but there are several things here that I don't entirely understand. Not in a "I'm confused about it" sense but in the sense that I don't know any of the details so I don't have an opinion.

rma-rripken
rma-rripken previously approved these changes Feb 7, 2025
Copy link
Collaborator

@rma-rripken rma-rripken left a comment

Choose a reason for hiding this comment

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

The random getProperty call still needs to be fixed - right?

@MikeNeilson MikeNeilson merged commit 15ee22f into develop Feb 21, 2025
3 checks passed
@MikeNeilson MikeNeilson deleted the feature/354-openid-auth branch February 21, 2025 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved-W192HQ23F0232-task4 Only valid if set by MikeNeilson, DanielO, CharlesG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants