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

Adding security.shared_users_group option for default users group #1056

Merged
merged 2 commits into from
Feb 12, 2022

Conversation

costrouc
Copy link
Member

Closes #957

This adds an option for security.shared_users_group which is by
default False. If True all users created after this option is set True
will be added to the users group. If any users were created before
this option they must manually be added to this group.

Closes #957

This adds an option for `security.shared_users_group` which is by
default False. If True all users created after this option is set True
will be added to the `users` group. If any users were created before
this option they must manually be added to this group.
Copy link
Contributor

@danlester danlester left a comment

Choose a reason for hiding this comment

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

Code is fine I think, but not sure why the docs still refer to users and groups under security key in the YAML file (and also primary_group and secondary_groups). These have all been removed in favor of Keycloak managing them all in-cluster.

@danlester
Copy link
Contributor

Also, we should add shared_users_group: true when upgrading to 0.4.0.
I have updated this plus the docs.

BTW you did not transfer the code that controlled shared_users_group by flags to qhub init, but I think that's fine... it was overly complicated for a user to worry about. We should impose a sensible default (which may be better as shared_users_group: false but that's for a UX discussion...).

@danlester danlester merged commit bc0a33f into main Feb 12, 2022
@costrouc costrouc deleted the feat-default-users-group branch August 23, 2022 14:37
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.

2 participants