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

migrate to keycloak as IDP #422

Merged
merged 11 commits into from
Dec 6, 2023
Merged

migrate to keycloak as IDP #422

merged 11 commits into from
Dec 6, 2023

Conversation

jeriox
Copy link
Contributor

@jeriox jeriox commented Nov 10, 2023

Migrates users to use the new sub from OIDC claims

closes #336

@frcroth
Copy link
Contributor

frcroth commented Nov 10, 2023

Can you provide test setup instructions?

@jeriox
Copy link
Contributor Author

jeriox commented Nov 10, 2023

My testing looked as follows:

  • log in with old OIDC provider with the configuration from main
  • checkout this branch, enter client_id and secret from auth.myhpi.de in .env
  • login with Keycloak and see how the sub of your user changes from firstname.lastname to your UUID

@dasGoogle
Copy link
Contributor

As we now have set up some basic group management in our Keycloak, I'd suggest we also integrate that into this PR.

@frcroth In case you want to test the setup yourself, please contact me or @lukasrad02 directly so we can give you some testing credentials.

@jeriox
Copy link
Contributor Author

jeriox commented Nov 14, 2023

@dasGoogle yep that was the plan, can you provide me with some infos on how the groups look in the OIDC claim or do I have to figure that out on my own? :D (would probably be good to write that down somewhere for the future anyways)

@dasGoogle
Copy link
Contributor

There will most likely (not Setup yet) a claim called "role", containing an array of arbitrary strings, each of which being a unique identifier for a group. @lukasrad02 and I expect that this mapping of ID to actual myHPI group would happen based on an attribute on each myHPI Group.

@jeriox jeriox force-pushed the keycloak-migration branch from 07924eb to a2a0ced Compare November 14, 2023 19:17
@jeriox
Copy link
Contributor Author

jeriox commented Nov 14, 2023

After another discussion with @dasGoogle we decided against a separate identifier for the groups and will instead use the group name directly (e.g. "student"). We could change our templates to capitalize the group names for the UI

@coveralls
Copy link

coveralls commented Dec 6, 2023

Pull Request Test Coverage Report for Build 7116695497

  • 24 of 28 (85.71%) changed or added relevant lines in 4 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.03%) to 74.263%

Changes Missing Coverage Covered Lines Changed/Added Lines %
myhpi/core/auth.py 13 15 86.67%
myhpi/core/models.py 5 7 71.43%
Files with Coverage Reduction New Missed Lines %
myhpi/core/auth.py 1 87.88%
Totals Coverage Status
Change from base Build 6850993127: -0.03%
Covered Lines: 1235
Relevant Lines: 1663

💛 - Coveralls

@jeriox jeriox merged commit ecfc008 into main Dec 6, 2023
8 checks passed
@jeriox jeriox deleted the keycloak-migration branch December 6, 2023 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Use select2 widget for author and moderator of minutes
5 participants