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

Added user "realm roles" to user list #159

Merged
merged 6 commits into from
Mar 13, 2023
Merged

Added user "realm roles" to user list #159

merged 6 commits into from
Mar 13, 2023

Conversation

joshburt
Copy link
Contributor

@joshburt joshburt commented Mar 6, 2023

What?

  • Added realm roles a user is assigned to when using the user list command.

Example Usage

List as a table

Screenshot 2023-03-06 at 4 12 14 PM

List as json

Screenshot 2023-03-06 at 4 12 53 PM

Why?

  • We've had a client request for additional user level control. Specifically the ability to expose and operate on a user's roles.

How?

  • I was able to find the related KeyCloak API documentation. We can request all realm roles, then all users mapped to those roles. With this detail we can merge the role assignments with the user data we already have.

How was this tested?

  • Unit tests added
  • System tests added

Possible Discussion Points

  • KeyCloak User<->Role Mapping Thoughts
  • Column name. It's currently realm_roles. It looks like internally there might also be other role types as well as groups. If/when we expose these I didn't want to have overlap. Please let me know if you have any suggestions on naming.

Notes

  • I have a simple dynamic network ae5 mock which is usable for the integration tests I wrote. I am however also running into issues getting this to work as expected when run through the GitHub Action. I've pulled this out for now and will introduce it later once I have the issues ironed out.
  • There is opportunity to split the test suites up (unit/integration/system) but did not want to introduce a large amount of change in this PR. I'll do this separately.

@joshburt joshburt marked this pull request as ready for review March 7, 2023 00:24
@joshburt
Copy link
Contributor Author

joshburt commented Mar 7, 2023

After some discussions this morning around how we expose KeyCloak, I was able to access the other endpoints mentioned in option 2 for the PR. I'm going to continue down the route of Option 2 (less network calls) and update the PR shortly with the enhancements.

@jlstevens
Copy link
Collaborator

Looks good to me so far!

@joshburt
Copy link
Contributor Author

Checking in to see if anyone had any feedback on this? I'd like to get updates in and a build out. :)

@mcg1969
Copy link
Collaborator

mcg1969 commented Mar 12, 2023

I don't have a strong desire to monitor you closely here. If what you're adding is strictly additive or minor, and others are giving it a thumbs up, I say go for it.

@joshburt
Copy link
Contributor Author

Sounds good! Thank you!

@joshburt joshburt merged commit 2d8b23a into master Mar 13, 2023
@joshburt joshburt deleted the jb_CS-120_1 branch March 13, 2023 17:11
@joshburt joshburt restored the jb_CS-120_1 branch March 13, 2023 17:52
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