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

CP4AIOPS-3113 Pass authentication group delimiter to rails #389

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Aug 16, 2024

Each authentication system is configurated a little differently. open idc uses a , as the group delimiter

This sets the configuration in apache so it can easily be passed to the individual containers.

Just setting the value in settings.yml is a viable alternative

see also ManageIQ/manageiq#23139

@kbrock kbrock requested a review from jrafanie as a code owner August 16, 2024 21:29
@kbrock kbrock changed the title Pass authentication group delimiter to rails CP4AIOPS-3113 Pass authentication group delimiter to rails Aug 20, 2024
@Fryguy Fryguy self-assigned this Aug 21, 2024
@@ -10,4 +10,5 @@ RequestHeader set X_REMOTE_USER_FIRSTNAME %{OIDC_CLAIM_GIVEN_NAME}e
RequestHeader set X_REMOTE_USER_LASTNAME %{OIDC_CLAIM_FAMILY_NAME}e env=OIDC_CLAIM_FAMILY_NAME
RequestHeader set X_REMOTE_USER_FULLNAME %{OIDC_CLAIM_NAME}e env=OIDC_CLAIM_NAME
RequestHeader set X_REMOTE_USER_GROUPS %{OIDC_CLAIM_GROUPS}e env=OIDC_CLAIM_GROUPS
RequestHeader set X_REMOTE_USER_DELIMITER , env=OIDC_CLAIM_GROUPS
Copy link
Member

@Fryguy Fryguy Sep 3, 2024

Choose a reason for hiding this comment

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

Not sure if this is the right place for this, but X_REMOTE_USER_DELIMITER feels like a confusing name, because it sounds like this delimeter is for the users themselves, and not the user's groups? I'm thinking X_REMOTE_USER_GROUPS_DELIMITER and it happens to match name-wise with X_REMOTE_USER_GROUPS showing they are related.

@Fryguy
Copy link
Member

Fryguy commented Sep 3, 2024

Seems the remote-user-saml.conf changes from https://github.com/ManageIQ/manageiq-appliance_console/pull/265/files are missing here? Additionally, I think the mellon change from here is needed: https://github.com/ManageIQ/manageiq-pods/pull/1155/files

Each authentication system is configurated a little differently.
open idc uses a , as the group delimiter

This sets the configuration in apache so it can easily
be passed to the individual containers.

Just setting the value in settings.yml is a viable alternative
MellonMergeEnvVars On brings back all groups for REMOTE_USER_GROUPS
By default, the delimiter is ;, but this can be overridden

This is a no-op, but it highlights that a semicolon is used to join groups.
Liberty (via oidc) is using a comma, but that is not displayed here.
LookupUserGroups specifies a ":"
For multiple groups, LookupUser sets the delimiter to :
rails will then use that to split up the group parameter

Since saml (mod_auth_melon) uses a ;, we need to create a separate file
This will also need to change in rails
@miq-bot
Copy link
Member

miq-bot commented Sep 3, 2024

Checked commits kbrock/manageiq-appliance@7f78b09~...7814228 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. 🏆

@kbrock
Copy link
Member Author

kbrock commented Sep 3, 2024

update:

  • pushed missing change (hadn't pushed :( )

update:

  • header now called X_REMOTE_USER_GROUP_DELIMITER (note GROUP is singular)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants