Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds functionality to configure allowed groups for OIDC authentication via an environment variable. The implementation reads from the keycloak_allowed_groups environment variable and sets the allowed groups on the OIDC authentication plugin during setup.
Key changes:
- Added
_set_allowed_groups()function to parse and set allowed groups from environment variable - Integrated the function into the
set_oidc_settings()workflow - Added comprehensive test coverage for the new functionality
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/pas/plugins/kimug/utils.py | Implemented _set_allowed_groups() function and integrated it into OIDC setup |
| tests/utils/test_utils.py | Added test coverage for _set_allowed_groups() function |
| CHANGES.md | Documented the new feature in the changelog |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/pas/plugins/kimug/utils.py
Outdated
| # varenv set by puppet is a string representation of a list, e.g. "[group1, group2]" | ||
| # we need to convert it to a tuple | ||
| if varenv_allowed_groups is not None: | ||
| if varenv_allowed_groups.startswith("[") and varenv_allowed_groups.endswith( |
There was a problem hiding this comment.
maybe it should be better to use python ast lib, something like ast.literal_eval(varenv_allowed_groups) ?
There was a problem hiding this comment.
Done (see latest commit). I also had to quote the items, because the environment variable value generated by puppet is not a valid list representation.
It's [group1, group2, group3] , instead of ["group1", "group2", "group3"].
|
|
||
| utils._set_allowed_groups(oidc) | ||
|
|
||
| assert oidc.allowed_groups == ("group1", "group2", "group3") |
There was a problem hiding this comment.
And adding a test with a string of an empty list ?
WEB-4331