manageiq icon indicating copy to clipboard operation
manageiq copied to clipboard

CP4AIOPS-3113 Introduce configurable delimiter for LDAP group names

Open kbrock opened this issue 1 year ago • 6 comments

Overview

Auth Server Interface Delimiter
Liberty oicd ,
saml ;
ipa sssd :
ldap sssd :

Goal

Allow users to have a : in group names.

Since the group can come in with 3 possible delimiters, we treat :, ,, and ; as possible delimiters.

This works fine if groups do not have one of those characters in it.

Liberty uses a , as a delimiter and allows : in the group name. If a liberty server sends groups value of 'group1,people:fun,people:sad', we treat this as groups 'group1', 'people', 'fun', 'people', and 'sad'.

If a configuration references group1, then it works fine, but configuration that references people:fun will not find the group and not work as expected.

Solution

  1. Allow apache to pass the delimiter used to ruby.
  2. Allow the admin to override this delimiter in advanced settings.

The various configurations use different delimiters, so it makes sense to allow each of those configurations to specify the delimiter that is being used.

If an oidc server uses a different delimiter, then allow the admin to configure the correct delimiter in settings.

See Also

blocked:

  • [ ] https://github.com/ManageIQ/manageiq-appliance_console/pull/265
  • [ ] https://github.com/ManageIQ/manageiq-appliance/pull/389
  • [ ] Release of appliance_console gem version 10.0.0

dependent:

  • [ ] https://github.com/ManageIQ/manageiq-pods/pull/1155

ref:

  • https://github.com/ManageIQ/manageiq/pull/32
  • update: https://github.com/ManageIQ/httpd_configmap_generator

Next steps

We could change the configuration for LookupUserGroup and mod_auth_mellon to just use a , as a delimiter, but that felt like too much change.

kbrock avatar Aug 09 '24 16:08 kbrock

update:

  • rebased
  • reindented hash rockets in specs (rubocop)

kbrock avatar Aug 21 '24 17:08 kbrock

Updated:

  • Rebased
  • No longer defaulting the delimiter settings.yml value.
  • Apache now sends a delimiter value for all configurations.
  • The settings takes precedence over the apache header.

kbrock avatar Aug 23 '24 02:08 kbrock

update:

  • Require appliance_console 9.2 (it can handle the 2 different saml configuration files now stored in appliance
  • group parameter now called REMOTE_USER_GROUP_DELIMITER
  • setting is now called group_delimiter (it was called delimiters, but it is just a single delimiter now)

kbrock avatar Sep 03 '24 21:09 kbrock

I noticed you have a test that checks priority of config over header, however I don't see a test that shows the header working correctly without config. There are a few other tests I think needed.

  • a test that shows a header working without config set
  • a test where the header is not set showing the fallback to the regex
  • update existing tests to have the header passed in even if there are no groups (because the env is blank in the .conf files)

Fryguy avatar Sep 04 '24 12:09 Fryguy

update:

  • removed duplicate custom delimiter test (, vs |)
  • changed spec wording so it was clear whether we were using setting, fallback, or header.
  • added spec to test setting override without header.
  • changed case of the header
  • fixed specs to call setting group_delimiter (was plural)

kbrock avatar Sep 04 '24 15:09 kbrock

Checked commits https://github.com/kbrock/manageiq/compare/e273f4c64df2abfa5e6da50b9d467ca56c6e7ba9~...c6766b81ec985bd55015805af84df24bea7df638 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint 3 files checked, 0 offenses detected Everything looks fine. :star:

miq-bot avatar Nov 18 '24 18:11 miq-bot

Backported to radjabov in commit 8859562fce4177f65287669bbc61c1cb22800d04.

commit 8859562fce4177f65287669bbc61c1cb22800d04
Author: Jason Frey <[email protected]>
Date:   Tue Nov 19 15:41:30 2024 -0500

    Merge pull request #23139 from kbrock/CP4AIOPS-3113
    
    CP4AIOPS-3113 Introduce configurable delimiter for LDAP group names
    
    (cherry picked from commit e88fdc670a6c34788becabe1c096680d04c549b9)

Fryguy avatar Nov 19 '24 20:11 Fryguy