-
Notifications
You must be signed in to change notification settings - Fork 10
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
[#483] add support for mozilla django OIDC db config #490
Conversation
@SonnyBA is it an option to cherry pick the commits that provide the required setup-config structure instead, so that we can merge this to main? |
A new branch would be preferable @SonnyBA -- let's get this change in cleanly based off of |
b6ce629
to
7de377a
Compare
This PR is now branched from django-setup-configuration-upgrade and is ready to be re-reviewed. Edit: if there is feedback related to that origin branch, I'll have to do some rebasing (I'll squash these changes into the current single commit the branch has) |
docker/setup_configuration/data.yaml
Outdated
oidc_rp_client_id: client-id | ||
oidc_rp_client_secret: secret | ||
endpoint_config: | ||
oidc_op_authorization_endpoint: https://example.com/realms/test/protocol/openid-connect/auth | ||
oidc_op_token_endpoint: https://example.com/realms/test/protocol/openid-connect/token | ||
oidc_op_user_endpoint: https://example.com/realms/test/protocol/openid-connect/userinfo |
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.
The format for this changed in 0.21.1
oidc_rp_client_id: client-id | |
oidc_rp_client_secret: secret | |
endpoint_config: | |
oidc_op_authorization_endpoint: https://example.com/realms/test/protocol/openid-connect/auth | |
oidc_op_token_endpoint: https://example.com/realms/test/protocol/openid-connect/token | |
oidc_op_user_endpoint: https://example.com/realms/test/protocol/openid-connect/userinfo | |
items: | |
- identifier: admin-oidc | |
oidc_rp_client_id: client-id | |
oidc_rp_client_secret: secret | |
endpoint_config: | |
oidc_op_authorization_endpoint: https://example.com/realms/test/protocol/openid-connect/auth | |
oidc_op_token_endpoint: https://example.com/realms/test/protocol/openid-connect/token | |
oidc_op_user_endpoint: https://example.com/realms/test/protocol/openid-connect/userinfo | |
userinfo_claims_source: userinfo_endpoint |
endpoint_config: | ||
oidc_op_authorization_endpoint: https://example.com/realms/test/protocol/openid-connect/auth | ||
oidc_op_token_endpoint: https://example.com/realms/test/protocol/openid-connect/token | ||
oidc_op_user_endpoint: https://example.com/realms/test/protocol/openid-connect/userinfo |
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.
Is it just me or does this raise errors because userinfo_claims_source is missing?
Unable to satisfy prerequisites for step: Configuration for admin login via OpenID Connect:
CommandError: Prerequisites for configuration are not fulfilled: Configuration for admin login via OpenID Connect: Failed to load config model for Configuration for admin login via OpenID Connect. Further details:
1 validation error for ConfigSettingsSourceOidc_db_config_admin_auth
oidc_db_config_admin_auth.items.0.userinfo_claims_source
Input should be 'userinfo_endpoint' or 'id_token' [type=literal_error, input_value=UserInformationClaimsSources.userinfo_endpoint, input_type=UserInformationClaimsSources]
For further information visit https://errors.pydantic.dev/2.9/v/literal_error
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.
@stevenbal I also had the same error these days, and I think the problem is related to the right version of pydantic, i think should be pydantic>=2.10
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.
@danielmursa-dev The PR referenced in this issue fixes it through django-setup-configuration. It might be worthwhile to look at the PR and post your findings, a version bump of pydantic might be a good solution instead of fixing it in django-setup-configuration.
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 think I have a fix for this in the library, validating now and if it works I'll make a release of setup-configuration.
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.
This is a separate issue it turns out: the library is passing the actual attribute on the underlying TextChoices
rather than a literal string. Apparently, Pydantic 2.10 stringifies it somewhere along the lines. I filed an issue for this in maykinmedia/django-setup-configuration#31
Co-authored-by: Sidney Richards <swrichards@users.noreply.github.com>
3101ab1
to
8e316de
Compare
Fixes #483
Changes
Includes changes necessary to configure mozilla-django-oidc-db through django-setup-configuration.
Edit: This branch was based onfeature/467-objecttypes-setup-config
as that includes some changes to the setup configuration structure