Allow group Owners to manually add members when LDAP lock is enabled
Problem
In https://gitlab.com/gitlab-org/gitlab-ee/issues/4354 we are proposing an admin-level setting to lock membership changes to LDAP sync. Under that setting, only instance administrators are allowed to manually add members to group outside of LDAP.
We should optionally extend this privilege to allow group Owners to manually add members to their group:
- Some instances may want to add users to groups/projects, but not add them to LDAP. This is common for contractors/customers.
- Manual member management will be burdensome for administrators if they're the only user type who can handle this. Frequently, admins are a very small group of privileged users.
Proposal
Allow group Owners to manually add to group membership when "allow group Owners to manage LDAP-related settings" is enabled.
- What happens when this is unchecked? Members of groups (and their subgroups/projects) are not manually editable by Owners.
- What happens when this is checked? A configuration option is exposed in group LDAP settings that reads "Lock membership of this group to LDAP". This should default to true.
When this lock is enabled, we should remove all existing users added to projects/subgroups outside of LDAP. We should inform the group owner enabling the lock that this will take place.
Availability & Testing
This feature appears to be low risk in terms of availability.
When "Lock membership of this group to LDAP" is checked, ensure that LDAP users are retained and only non-LDAP ones are removed.
Besides unit and feature tests, we should extend the admin_ldap_sync_spec end-to-end test to cover this functionality.
This page may contain information related to upcoming products, features and functionality. It is important to note that the information presented is for informational purposes only, so please do not rely on the information for purchasing or planning purposes. Just like with all projects, the items mentioned on the page are subject to change or delay, and the development, release, and timing of any products, features, or functionality remain at the sole discretion of GitLab Inc.
Designs
Relates to
- #38639015.103
Activity
mentioned in issue #1702
assigned to @regisF
assigned to @rdavila
Here is an example for this feature.
Business Area: Point of Sale
- Manager: Chris
- AD Group: Chris_group
Members of Chris_group
- Chris (manager)
- Bob (Junior Dev)
- Doug (Junior Dev)
- Walter (Business Analyst)
- Tom (Dev Lead)
Eduardo transferred from Chris’ group. He is no longer in the Chris_group, but he still does some support work from time to time.
A group called POS is created in GitLab. It has been linked to the Chris_group LDAP group for developer role. It would be ideal to individually add Tom, the dev lead, as Master role. Currently, we are artificially having to create an AD group and assigning it to the master role.
It would also be good to have Eduardo manually added to the developer role so he can do work from time to time.
Desired role mapping:
Developer Role
- Chris_group
- Eduardo
Master Role
- Tom (Dev Lead)
Edited by Matthew Beasley"it's not possible to add members manually without them being removed on the next LDAP-sync for the group."
This is demonstrably not the case. I have LDAP sync enabled and in the past users were added manually, they're still there. This is on GitLab Enterprise Edition 10.4.5-ee a3ce9cd0
Edited by Ævar Arnfjörð BjarmasonI just wanted to add my 2 cents on this issue. My scenario is essentially the same as described above by @matt.beasley, except it's not hypothetical. I have a need to grant access to a handful of users to projects within an LDAP-synced group. I can do it per-project as a workaround, but I'm a bit lazy :)
I understand it's old, and not yet assigned, but there is at least some interest out there.
We would also be interested in this feature since we use a mixed authentication scheme with corporate users signing in via LDAP, while some external users are just GitLab-local users accounts. Currently, it is not possible to add them to an LDAP-synced group, which is really annoying.
This design decision does also not make so much sense for me, as members for subgroups or projects within an LDAP-synced group can already be added manually without any problems. So it seems that GitLab already supports such mixed permissions to a certain extent.
mentioned in issue #7219 (closed)
Customer interest --> https://gitlab.zendesk.com/agent/tickets/104091 (internal use)
added Manage [DEPRECATED] label
This issue is causing some problems for users at this customer: https://gitlab.my.salesforce.com/0016100001HGPYw
added customer+ label and removed 1 deleted label
changed milestone to %11.11
added backend label
If you add this feature then you will cause problems for large enterprise customers who rely on AD and have security audits.
- all of our repos have membership managed by AD groups
- all changes to AD groups are managed by a process that gets tracked for security audits
- we have many repos that are audited for PCI COMPLIANCE so this kind of tracking is a LEGAL requirement.
- if we are allow members to be added manually without AD then we will fail our audits
If you do add this feature anyway then for the enterprise customers you will need to make it configurable from the Sys Admin level :
- we would need to be able to turn it off globally and have it immediately globally effective.
- It would NOT be an inherited permission that could persist after a change (ie not the way "Can Create Groups" is)
- it should NOT be the DEFAULT, it should be an override only available to Sys Admin.
In summary, we're not a fan of this, but if you do it anyway, please do it in a way that protects your enterprise customers. In our way of thinking, if you want to manually add members then don't apply LDAP groups to members and just leave the whole group manual.
@stephen.m.carlson If we implement this ability for 'Owner' only then I think we retain basically the current level of integrity. For example, although 'Owner' cannot currently add users manually they can:
- Override the role given by LDAP.
- Remove the LDAP Group Links thereby reverting the group to manual group membership.
Given those facts I think this is a minor change so long as we keep this at 'Owner' level. Additionally, we have a global setting "Allow group owners to manage LDAP-related settings". We should also make sure we honor that setting for changes here.
Would this address your concerns?
mentioned in issue #4354 (closed)
-
@dblessing - we have SYS Admins that apply the AD groups at the GROUP level when the group is created. So, each role (including Owners) is assigned at the GROUP level via an AD group. It seems to me that your proposal would let anyone in the OWNER role override the AD groups without going thru the process to add members via AD group only. This seems it would go around and break the audit trail which is MANDATORY because of PCI COMPLIANCE. Maybe I am misunderstanding your comments, but so far ... No it does not address our concerns. At this point, I dont see it as a "minor change"... quite the opposite.... but maybe I am misunderstanding your explanation.
Consider a large enterprise with many hundreds or even thousands of groups, and thousands of users. Can you imagine the problems we would have if you give this power to all OWNERS and you expect each of them to understand the repercussions of what they are doing ? Are you only looking at this change from a convenience perspective and not a SECURITY perspective?
I know we have some interest in #4354 (closed) which also relates to LDAP usage.
The only way I can see this feature working for us is if our SYS ADMINs can completely disable the feature. Pls see my bullet points in previous post. Generally speaking we are looking for more security not less. There is a lot at stake here so please dont consider the SECURITY of our groups and repos to be a "minor" issue.
Im going to try to pull in some more comments our GitLab SysAdmin and one of our Enterprise Architects. We can see what they have to say:
- @chad.stevens - can you please weigh in on this from the Sys Admin perspective ?
- @matt.beasley - can you please weigh in on this from the Enterprise Architecture perspective ?
@jeremy - please note these concerns and watch this thread. I hope you guys are thinking this through thoroughly.
Edited by Stephen M Carlson @stephen.m.carlson I'll try to explain another way as I think there's still confusion.
We have the global setting 'Allow group owners to manage LDAP-related settings'. If you uncheck this option then group owners should not be able to change LDAP-related settings. In the same manner maybe it makes sense to make this proposed feature abide by that setting. That is, if 'Allow group owners to manage LDAP-related settings' is unchecked, group owners cannot add group members manually.
We might want to change the text of that setting slightly as it would be a bit outside of what you might expect. But I think the end goal is reasonable - essentially, LDAP is the source of truth for membership for this group, and do not let owners modify any settings that would affect that.
Of course, this is for @jeremy and product to ultimately decide.
-
@dblessing - maybe it would be OK, ... ONLY IF as you said "LDAP is the source of truth for membership for this group, and do not let owners modify any settings that would affect that" . that needs to be definitive/absolute AND it needs to be THE RULE by DEFAULT not by exception. We don't need our Security team telling us that our GitLab instances are not secure or can't be secured and we can't put our PCI and Security audits at risk.
Im waiting for @chad.stevens and @matt.beasley to weigh in. And I am assuming that @jeremy is aware of the importance of this issue and issues like it when it comes to GitLab's enterprise customers.
I don't think the current setting does enough as it is still possible to override the LDAP access even when 'Allow group owners to manage LDAP-related settings' is not selected. Even with the setting 'Prevent adding new members to project membership within this group' selected, you can circumvent the LDAP restriction by adding other groups. I think it would be best if there was a default option to make access be controlled entirely through LDAP. If some admins do not want LDAP to be the only access control then, there should be something that they can remove. I really don't think this option should be given to the group owners and should be managed by admins. Our security and audit teams have a keen interest in making sure that our source code is secure and only those who have gone through proper channels be granted access. Allowing users to be added manually increases our risk of unauthorized access to the source code.
-
@chad.stevens This issue is for situations when defined access control in an LDAP group sync situation does not work as documented -- where the user ends up with the highest access level defined via any source. That is clearly broken here, and it confuses people. What you are looking for is related, but different, and can be found in issue https://gitlab.com/gitlab-org/gitlab-ee/issues/4354
Edited by Reb added [deprecated] Accepting merge requests label
I wrote a brief overview of how we should consider handling this in https://gitlab.com/gitlab-org/gitlab-ee/issues/4354#note_153540178.
For this issue, I think we need a setting to optionally extend manual member management to group Owners. I agree that it makes sense to simply add this behavior to "allow group Owners to manage LDAP-related settings".
added Deliverable priority2 labels
added estimationneeded label
added direction label
-
Customer requesting this feature --> https://gitlab.zendesk.com/agent/tickets/117713 (internal use only)
From the customer:
The problem is that we have a set of users that should be synced via the LDAP Group Sync but we also require some manually added members that are only added e.g. for a short period of time. Due to your LDAP/AD structure it is not possible for us to add these users to the corresponding AD Group for this purpose. The access should be granted on application/GitLab side.
I noticed that I'm able to add users manually to specific projects in a "LDAP synced group". But this is not useful as a workaround on a larger number of projects.
changed milestone to %12.0
removed groupauthentication and authorization [DEPRECATED] label
added estimationcompleted label and removed estimationneeded label
mentioned in issue #9683
changed milestone to %12.1
changed milestone to %12.2
mentioned in merge request !14007 (merged)
