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

[User groups][Members] Implement 'User ID Overrides' section #548

Closed

Conversation

carma12
Copy link
Collaborator

@carma12 carma12 commented Sep 10, 2024

The 'User groups' > 'Members' > 'User ID Overrides' section needs to be implemented and its components adapted to reflect the same behavior as in the modern WebUI (e.g., its 'Add' modal is slightly different than in other sections).

How to reproduce (from current WebUI)

  1. Create a new ID View and some Active users (these will be associated later)
  2. Go to the new ID View settings page
  3. In the 'Overrides' tab section, add the newly-created users
  4. Create a new User group and go to its 'Members' > 'User ID Overrides' section
  5. Add any of the users associated to the ID View

Problem

It seems to be an issue that affects the addition and removal of User ID Overrides items. This is affecting both current and modern WebUI. Video that reproduces the issue.
Further investigation and possible report will be done on this.

@carma12 carma12 added the needs-review This PR is waiting on a review label Sep 10, 2024
@carma12 carma12 force-pushed the user-groups-members-useridoverrides-2 branch 2 times, most recently from b1325e8 to 440bc38 Compare September 10, 2024 15:10
@carma12 carma12 force-pushed the user-groups-members-useridoverrides-2 branch 2 times, most recently from fb01584 to ec1b682 Compare September 11, 2024 09:16
@carma12
Copy link
Collaborator Author

carma12 commented Sep 11, 2024

@mreynolds389 - I corrected the code based on your comments. The implementation of the tests are not possible at this point because it requires to add new ID Views and assign users from the 'Overrides' section, and that functionality is not implemented yet in the modern WebUI. So I will create an issue to track this and not forget about it. Feel free to keep reviewing, thanks!

@carma12
Copy link
Collaborator Author

carma12 commented Sep 11, 2024

New issue for the tests created: #549

@carma12 carma12 force-pushed the user-groups-members-useridoverrides-2 branch from ec1b682 to ef3c40e Compare September 11, 2024 15:28
@carma12 carma12 added the ack Pull Request approved, it can be merged label Sep 11, 2024
@carma12 carma12 force-pushed the user-groups-members-useridoverrides-2 branch 2 times, most recently from a0347f6 to ccfa777 Compare September 12, 2024 08:40
The 'User groups' > 'Members' > 'User ID
Overrides' section needs to be implemented
and its components adapted to reflect the
same behavior as in the modern WebUI
(e.g., its 'Add' modal is slightly different
than in other sections).

Signed-off-by: Carla Martinez <[email protected]>
@carma12 carma12 force-pushed the user-groups-members-useridoverrides-2 branch from ccfa777 to afe8f24 Compare September 12, 2024 14:02
@abbra
Copy link

abbra commented Sep 16, 2024

So, the core of the problem is that we only should be allowing ID overrides from 'Default Trust View' to be added to groups as 'ID overrides'. These are the only ones that matter because the groups in questions will be used for LDAP access controls. Access control in LDAP will be done against LDAP DN of the bound account which can only be mapped to the entry in "Default Trust View" ID View and not any other ID view.

So we'd need to fix following:

  • IPA server side needs to prevent adding ID overrides from any other ID view than 'Default Trust View'.
  • Web UI needs to not show any other ID view than 'Default Trust View' when allowing to choose an ID override to add to a group as a member

@carma12
Copy link
Collaborator Author

carma12 commented Sep 17, 2024

This PR won't be merged due to some existing errors in IPA (reported here and here). We will wait until those are fixed in order to continue with the implementation of this feature.

@carma12 carma12 closed this Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, it can be merged needs-review This PR is waiting on a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants