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.
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.
"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
@aearnfjord If the users have an LDAP identity matching the LDAP group link then they will be removed. Otherwise, they will remain. However, new manual users cannot be added while a group link is present.
I 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.
This would work, but then you wouldn't be modifying the permissions of
the top-level group, but setting different permissions on a sub-group,
which is different from the issue reported here.
We would also be interested in the feature, also having a mixed authentication environment and agree sub-groups are not the same as adding individuals from more than one authentication scheme to the same group.
Just to make sure, I think we should only allow adding members to LDAP linked groups for owners. Other LDAP management is limited to owners and this allows admins to restrict this capability in cases where they want LDAP to be the source of truth.
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.
@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.
@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
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".
Jeremy Watson (ex-GitLab)changed title from Allow to manually add members to a LDAP-synced group to Allow group Owners to manually add members to a LDAP-synced group
changed title from Allow to manually add members to a LDAP-synced group to Allow group Owners to manually add members to a LDAP-synced group
Jeremy Watson (ex-GitLab)changed title from Allow group Owners to manually add members to a LDAP-synced group to Allow group Owners to manually add members when LDAP lock is enabled
changed title from Allow group Owners to manually add members to a LDAP-synced group to Allow group Owners to manually add members when LDAP lock is enabled
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.
Jeremy Watson @jeremy changed milestone to %12.0 5 months ago
Jeremy Watson @jeremy changed milestone to %12.1 4 months ago
Jeremy Watson @jeremy changed milestone to %12.2 4 months ago
Jeremy Watson @jeremy changed milestone to %12.3 2 months ago
Jeremy Watson @jeremy changed milestone to %12.4 1 month ago
Jeremy Watson @jeremy changed milestone to %12.5 19 hours ago
really?
Your support is telling us for half a year that this feature will be implemented in "the next version".
"The next version" is probably a stretchy term.
@sarcila: It looks like this has missed a few releases and the bot is just bumping the milestone every month. Do you know if this is still something we're working on? Should we pull this back into the backlog, or are you comfortable keeping this in %13.4?
@sarcila I'm going to move this into %Next 1-3 releases so give you a chance to focus on your other work. We can pull it back into a release as you have more availability.
@hsutor Hi, Hannah! It looks like you are now the PM for the ~"group::authentication and authorization" group that this issue belongs in. The related !36710 (closed) MR has recent activity from interested customers carries a milestone of 14.9 but this issue has a milestone of Backlog. Are these milestones accurate? (It looks like the bot has been bumping the milestone for that MR along.)
The first MR I linked to has behavior behind the :ldap_settings_unlock_groups_by_owners feature flag. The !29682 (closed) MR (currently listed as WIP though it was being worked on by someone who is no longer a GitLab team member) would remove the :ldap_settings_unlock_groups_by_owners feature flag, making the behavior behind the feature flag the default. Lastly, there is some conversation about this in gitlab-com/www-gitlab-com!49384 (closed).
I have been working with a customer who is interested in this functionality. GitLab team members with access to Zendesk can learn more in the ticket.
Hi @bcarranza ! Thanks for the ping. Yes, the bot has been bumping this along and there is no work currently scheduled on it. I've moved both this issue and the MR to the backlog. This is actually a timely discussion, this issue was recently created and is related:
@dblessing Do you mind ELI5 the difference between the issue linked above and this one? I am confused because I thought, as it stands today, you can manually add members when LDAP lock is enabled...
from the issue above:
we should not allow group maintainers/owners to manually add users with LDAP identities as members of the group. It's still desirable to allow non-LDAP users as members.
@hsutor It's really hard to keep all of this straight. I am not sure we should allow group owners to add non-LDAP members when the lock is enabled. I think this is exactly the type of thing the lock intends to protect against.
We might need to have a higher level brainstorm about what user management we want to allow in which cases when we have things like LDAP or SAML involved. #354511 makes sense regardless because it's just shoring up what is already the end result. But the rest of it has some implications - some customers want X but not Y. So do we allow it, or have a setting to allow/disallow? Then I worry about getting too many different configurations.
@hsutor@lpeng1991 (From slack), Since this work maybe scheduled by JiHu,
We will allow owners to only add non-LDAP members when this setting is enabled, correct?
We'd be making a conscious choice to offer essentially a bypass to LDAP as SSoT with this setting where users like contractors may need to be added (which is a fair feature request). Since this directly affects the security model of larger organizations, it will default to off and have appropriate messaging on impact of enabling is on user's end + logging around when users are added outside of SSO?
We'd prioritize #354511 to maintain the coherence of both features?
The flag :allow_group_owners_to_manage_ldap should enable group owners to manually add members even with active ldap syncs right?
For us this works for groups and subgroups, but it does not work for sub-sub-groups, the "Invite Members" button does not appear for deeper nested groups, only one level.
Group (works)
Subgroup (works)
Subsubgroup (doesn't work)
Edit: I just checked and the problem was, that even though I am an admin, I wasn't the Owner of the subsubgroup.
I think admins should always have the same permissions as Owners.
Due to the discussion (linked above), we have decided not to implement this feature. I'm going to mark this with wontfix so people can see it in the future.
Hey @hsutor! There's a lot of discussion on both this issue and the MR you linked. Can you edit your comment to provide a quick summary of the why for this decision? That will help folks who encounter this issue in the future quickly understand, without needing to sift through other comment threads. That should also make a reopen or a duplicate issue less likely.
For whatever reason, the first time I clicked your link, it didn't send me to the comment anchor. I understand better now, but it might still be worth quoting that comment here? :)
This goes against our convention over configuration principle and would complicate the understanding of LDAP sync for our customers and may potentially open their instances up to security issues (due to the misunderstanding of how LDAP sync is supposed to work with these additional configuration options). SAML group sync also works the same way and having a disparity between the two would be confusing.
workaround: Using group sharing. Having dedicated groups for LDAP sync, admin can create a new group and invite these dedicated groups and other members. Using the example in the description, this would mean having a team group consist of ldap-sync/group-a group and user4.
@hsutor I was looking at code to help with a support case. I think before we remove this capability we need to investigate further. Based on what I'm seeing, this feature flag also currently prevents inviting a group to an LDAP synced group. Based on what you've said I don't think that's the intention. We might need to tweak our permissions to make things work as we intend.
I think before we remove this capability we need to investigate further.
Remove which capability, exactly? This one is labeled as a wontfix so we don't intend to add this functionality. You mention a feature flag, what is it for?
This issue is specifically to add this feature as requested by multiple gitlab customers. The earlier comment about going against convention over configuration is contradictory, because the configuration of manually adding members to projects is already allowed and used, making it glaring that the feature is missing for groups. I would say this actually violates convention as it is unexpected behavior to the user. Please remove the wontfix label from this.