LDAP - Support user-level filters / mappings offering finer level of control than just Group mappings

We slacked about this briefly yesterday and I've seen you on several LDAP issues so just want to keep this one on the radar with the rest of them @dblessing . Thx :)

made the issue confidential

changed the description

made the issue visible to everyone

changed the description

added Platform label

changed milestone to %Next 3-6 months

changed the description

The description is not clear at all to me. Can't you already do this with https://docs.gitlab.com/ee/administration/auth/ldap.html#using-an-ldap-filter-to-limit-access-to-your-gitlab-server or https://docs.gitlab.com/ee/administration/auth/ldap-ee.html#supported-ldap-group-types-attributes ?

I think that filter applies to the entire LDAP directory, so by adding it would exclude users you may want in another group. For example, if you had jobTitle as an attribute, you may want drivers to be in one group, while clerk to be in another.

closed

reopened

changed the description

marked this issue as related to #3257

mentioned in issue #3257

@Haydn FYI there's now a meta issue for EEP Authentication & User Management, I believe this request is effectively what I proposed in there, so I've linked them together. Feel free to review the full meta issue and get feedback from any customers on the capabilities we are looking at bringing out over the coming releases.

https://gitlab.com/gitlab-org/gitlab-ee/issues/3257

We have several other cases that are similar, for example we want to create a group for each business unit for users with a specific security level.

Given the above, the other item in that list may be of interest that we're considering:

Inherit permissions from LDAP properties. LDAP group sync in GitLab EES allows for a mapping relationship between an LDAP group and a group on GitLab with an explicit level of permission specified in GitLab. By leveraging GitLap EEP Group filters, it is possible to define a permission mapping such that the permissions are managed via LDAP rather than GitLab

changed milestone to %10.1

added UX label

added backend frontend labels

added Deliverable label

Quick note that this would require quite some work compared to what group sync does now. Currently, we don't grab the individual group member's object from LDAP. We only grab the DN from the group's member list and look for a user in GitLab with that matching extern_uid. This would potentially add a lot of additional calls to LDAP to gather the needed information.

@dblessing Instead of grabbing the group object and reading its members list, would it be an option to search for user objects that are both members of that group and match a given filter, and only return their DNs? Or is that not how it works?

@DouweM Not all LDAP servers have two-way relationships. On some, if you look up a user you won't see any indication they're a member of a group. LDAP is a strange beast.

assigned to @pedroms

@pedroms I suggest having a conversation with @dblessing on this or getting your GitLab instance to work with LDAP https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/ldap.md

@dblessing do we see this as going into configuration at a server level, or when you link a group there are additional fields to provide filters or do something a bit fancier where you can have a UI that helps you pick out attributes etc...

@mydigitalself It's an extension of LDAP group links, right? At the server level, we already have a user filter.

@dblessing Ah, right. So writing query for "users that are members of a group" is not so trivial. Do you know what (common) LDAP servers would support that? I wonder if it would be acceptable to make this an AD/OpenLDAP-only feature, for example.

LDAP sync performance is already not great at large scale (https://gitlab.com/gitlab-org/gitlab-ee/issues/3347), and implementing this using extra LDAP queries is definitely not going to make it better.

assigned to @iamphill

assigned to @jameslopez

@dblessing @DouweM I've been thinking of something like this:

Basically, give the option to either sync 1:1 groups as we have at the moment, or provide a filter that could potentially span several groups, include/exclude certain groups of users, etc...

So instead of modifying the current group sync, create a new one and let the user choose between the two. I guess users that don't really have a 1:1 mapping they won't ever use that, and won't have that other worker running.

We could improve performance by limiting the number of updates (While I don't have LDAP coding experience, I used it in the past as part of 2 large orgs and updates always took 24 hours on both anyway), and perhaps implementing different strategies depending on the LDAP server.

mentioned in merge request !2885 (merged)

@jameslopez a few suggestions and questions

Why have you made the two options mutually exclusive? Why could you not select a group, and then filter out by a particular attribute or object class? This makes more sense with question two...
If we returned all groups in the Group Selector, and made it custom field such that each group had a checkbox associated with it, then you could effectively choose multiple groups to sync from or choose no groups, in which case we would look across all users.
Related to the above, is there a way we can help people build a query using some UI primitives so there was a more visual way of building up your query - or we build the basic query and still give people the ability to edit it manually.
Extended from above, could we provide a Preview ability? This way you can see the data that is actually being returned to validate your logic.

cc @DouweM @pedroms

Thanks @mydigitalself, I appreciate the feedback!

1 Why have you made the two options mutually exclusive? Why could you not select a group, and then filter out by a particular attribute or object class? This makes more sense with question two...

In the original description, I see we may want to filter by multiple groups. So in your example if we select a group, then you can't filter across multiple groups. Or for instance, users who belong to 30 different groups that have one attribute in common (managers), you will then have to select the groups one by one if you were to use a multiselect. Plus the current logic is tightly coupled with a 1:1 relationship between GitLab/LDAP groups, I wouldn't like to run both types of queries at the same time, as it will affect performance.

2 If we returned all groups in the Group Selector, and made it custom field such that each group had a checkbox associated with it, then you could effectively choose multiple groups to sync from or choose no groups, in which case we would look across all users.

I didn't think this as an option as we're already having performance problems, this will effectively multiply the queries. I thought that it would be more performant to add the filter, which can be as restrictive as possible. That way we:

Avoid drilling down, which would have effectively multiplied queries to the LDAP server.
UI/Frontend/Backend would be much easier
By restricting the query to just groups, you can't provide other types of filters, including objectClass, attributes, etc... (This is mentioned in the issue description as a requirement)

3 Related to the above, is there a way we can help people build a query using some UI primitives so there was a more visual way of building up your query - or we build the basic query and still give people the ability to edit it manually.

Perhaps listing the attributes available on a drop-down box, then do the same with conditionals, etc... By selecting them you could build the query. Although I don't know if this adds more complexity. The minimum we could do is provide some help (as we do with markdown) to see the list of filters available, examples, etc...

4 Extended from above, could we provide a Preview ability? This way you can see the data that is actually being returned to validate your logic.

What would you show in the preview? I don't think it would be useful to see a scrollable list of 10K users. I'm thinking perhaps we could provide a count of affected users... Doing that is a bit more performant on LDAP (so we don't have to query for the actual objects) and it's also a way to verify the information is correct.

Basically, I don't think that performance-wise we could provide the end user with instant feedback, as LDAP may take a while to respond and (I might be wrong) but LDAP servers may be on a different network or have some network related lag. I do think those are valid concerns, though.

In the original description, I see we may want to filter by multiple groups.

@jameslopez I couldn't find any mention to multiple groups, where did you find that?

My perception of the requirements is that we need to add the ability to filter users of a specific linked LDAP group. So @jameslopez's suggestion makes sense to me if the “LDAP Group cn” field is always active. The user would choose the group they want to filter and then input the filter in the “LDAP filter” field.

@mydigitalself I like the idea of having a visual way to build the query, but I'm not sure it's worth the effort as queries can be very custom, like the one in the description: (&(objectcategory=person)(objectclass=user)(extensionAttribute20=FTE)(|(extensionAttribute501=developer)(extensionAttribute501=technical)).

I think the preview ability is definitely more important and useful than the visual query, as it would allow users to validate and make sure they giving access to the right people. For this, if performance is not a big issue, we could show the total count of affected users, a small list of the first 10 results, and a button to show all results (if the user wants to search a particular user).

However, to keep this issue as small and focused as possible, I suggest we move the visual query and the preview feature to separate issues.

Thanks for the input @pedroms - I agree with all your comments.

@jameslopez I couldn't find any mention to multiple groups, where did you find that?

I got that from: These users span across all business units of the company, there is no single group that includes all of these users

And if you read the following I think it makes sense that these filters have to be across multiple groups:

We need the user filter option as we need to have a GitLab group that includes all Full Time Employees (FTE) who are developers and active employees.  These users span across all business units of the company, there is no single group that includes all of these users.  Without this enhancement I will have to create a script to manually create a group for this either in GitLab or an Ldap group that can be mapped to the GitLab Group.
In this case the base would be: dc=customer_name,dc=com
The user filter may be something like: 
(&(objectcategory=person)(objectclass=user)(extensionAttribute20=FTE)(|(extensionAttribute501=developer)(extensionAttribute501=technical))
In this case extensionAttribute20 defines Full Time Employee, and extensionAttribute501 defines they have the role of developer or technical.  This example would work with GitLab access.

Also, the filter doesn't include a group CN.

@jameslopez

These users span across all business units of the company, there is no single group that includes all of these users

This is a single customer comment and any LDAP topology could be crafted differently. This is why I think being able to include none or many groups in the query would facilitate more variations of topology.

@mydigitalself yep, I think we are really on the same page:

This is why I think being able to include none or many groups in the query would facilitate more variations of topology.

This is compatible with the filter option I was referring to, you can include/exclude groups directly within the filter, it's just part of it. What we could provide is a separate base, though.

I've been testing some of this locally, but haven't had great success (note that my LDAP knowledge is unexistent :D).

When using a filter, the results are normally limited to 1000 (or 500 in our local setup). This can be changed at server level, but I assume it's always limited. The thing is that this is normally fine for groups, as you can specify a range (and we do that in the code already) so we can loop through in batches.

It's not so easy for a global search/filter. So we may either have to always specify groups (very inefficient when users belong to multiple groups), or use something like another LDAP gem that supports this, or it could also be that I haven't done enough research yet :)

Small update on this - It looks like we can use paged results if the LDAP server supports it. for the case of RFC 2696 (which is supported by more server types than VLV) this is the case for:

AD (has a default max page size of 1000)
eDirectory
OpenDS / OpenDJ
OpenLDAP
389 Directory Server
IBM Tivoli Directory Server
Oracle Internet Directory

Some that may not support it are Apache and Sun DS.

Unfortunately, I couldn't get this to work locally (yet). The search keeps returning a max of 500 entities. I managed to debug the gem and find out that the OpenLDAP server is returning (silently) a code 4 which means sizeLimitExceeded according to https://tools.ietf.org/html/rfc4511#appendix-A

The problem could be this issue: https://github.com/ruby-ldap/ruby-net-ldap/issues/36

And this feature request could be great for us as well: https://github.com/ruby-ldap/ruby-net-ldap/issues/12

I'll keep investigating, though.

This is why I think being able to include none or many groups in the query would facilitate more variations of topology.

This is compatible with the filter option I was referring to, you can include/exclude groups directly within the filter, it's just part of it.

@jameslopez I agree, it makes sense to support group filters within the query, if the user is comfortable writing LDAP queries. On the other hand, I think it's safe to assume that the most common use case for the “Linked LDAP groups” feature is directly mapping LDAP groups with GitLab groups, which should have a selection method that's easier than a query for most users. To support variations of topology, if it's possible, I think we should aim for: a list of LDAP groups for direct user selection and a query field. The user may simply choose one or more LDAP groups, or simply enter a query, or a combination of both. If they enter a query without indicating a group (either through the list or as a query filter), the query would be scoped to the entire LDAP server. But then again, I'm not sure this is feasible performance-wise. WDYT?

The user may simply choose one or more LDAP groups, or simply enter a query, or a combination of both. If they enter a query without indicating a group (either through the list or as a query filter), the query would be scoped to the entire LDAP server

@pedroms I'm fine with this. This means the only filter would be a group, and then in order to filter by anything else we have to enter a query. I'll be happy with this (although redundant) as long as the rest of the filtering is done via the filter query and not drilling down through more filters.

I think we need to have a chat about this. Right now I would like to keep the current group-group mapping as it is, the reason being that it's more efficient than anything we could build with the filters - so that's why I said we could separate both. This could be transparent to the user, though.

added In dev label

Not in need of any FE work , so removing @iamphill + frontend

removed frontend label

unassigned @iamphill

Happy to have a chat with @jameslopez and @pedroms. From a code perspective, whether the user chooses a group in the autocomplete field or uses the filter, it all gets compiled down into a filter on the backend anyway. Maybe it would be appropriate to hide the filter in an 'Advanced' tab so we still lead users to the more user-friendly approach most of the time but allow power users to enter the raw filter.

From the call with @jameslopez and @mydigitalself (added to the description):

Keep the “LDAP group cn” field as it is today
Add a “LDAP filter” text field that is validated for the correct syntax
- @jameslopez: to be correct, should this field be labeled “LDAP filter” or “LDAP user filter”?
Add an exclusivity behavior where the user either uses the “LDAP group cn” field or the “LDAP filter” field (I'll be looking into this)
For synchronizations in the “Linked LDAP groups” list that use a LDAP filter, show the full query

@mydigitalself we might want to think about renaming the “LDAP groups” feature to “LDAP sync” (or similar), as it won't be limited to just groups anymore (filters allow you to sync across a server).

changed the description

@jameslopez: to be correct, should this field be labeled “LDAP filter” or “LDAP user filter”?

Either way... It's really a user filter, so perhaps we can call it that :)

changed the description

LDAP group cn	LDAP user filter

Add a “Synchronize with” field with radio buttons that toggle the “LDAP group cn” and “LDAP user filter” fields
Add a “LDAP user filter” text field:
- Placeholder: “e.g. (&(objectCategory=person)(objectClass=developer))”
- Description: “This query must use valid [LDAP Search Filter Syntax]. Synchronize [GROUP]'s members with this LDAP user filter. If you do not belong to this LDAP user filter you will lose ownership of [GROUP].”
- The “LDAP Search Filter Syntax” link should point to our own documentation about the syntax (I didn't find any, so it would need to be created) or to an external documentation. In my searches I've found these: Microsoft, LDAP Explorer, CentOS.
Make the help text of the “LDAP group cn” and “LDAP user filter” fields all in one line
If an active synchronization is:
- A group, prefix it with “Group: ”
- A user filter, prefix it with “User filter: ” and append the full query. Very long queries should be truncated with ellipsis according to the full width of the panel.
As this feature won't be limited to just groups anymore (filters allow you to sync across a server):
- Rename the navigation item from “LDAP Group” to “LDAP”
- Rename the breadcrumb item from “LDAP Groups” to “LDAP Settings”
- Rename the page title from “Linked LDAP groups” to “LDAP synchronizations”
- Rename the list panel from “Linked LDAP groups” “Active synchronizations”
- (Not in the images) Rename the red “unlink” button to “Remove”

@mydigitalself any objection to the renaming points above?

@tauriedavis if you're able, can you please follow this issue as I'll be off (you can assign yourself)? If not, let me know and I'll mention another UX Designer.

assigned to @tauriedavis

Sounds good! Ill keep an eye on it :)

changed the description

@mydigitalself if no objections to @pedroms points above, I think we can move this to UX ready. Ping me if you need anything else here!

added UX ready and removed UX labels

'Synchronize with' doesn't seem like the right label. I don't know of something better but this term is confusing to me.

@dblessing Just getting caught up with this issue, but would just "Sync" make more sense? Or "Sync as" ?

@tauriedavis Not really. It's more of a method to use for sync - simple or advanced. 'Link by', 'Method'? I'm not 100% convinced those are great but it's closer.

Also, the title of the page isn't 'LDAP Synchronizations' but 'LDAP Group Links'.

@dblessing Pedroms mentioned the page wouldn't just be for groups anymore because you can sync across a server.

As this feature won't be limited to just groups anymore (filters allow you to sync across a server):

Rename the navigation item from “LDAP Group” to “LDAP”

Rename the breadcrumb item from “LDAP Groups” to “LDAP Settings”

Rename the page title from “Linked LDAP groups” to “LDAP synchronizations”

Rename the list panel from “Linked LDAP groups” “Active synchronizations”

(Not in the images) Rename the red “unlink” button to “Remove”

What about "Sync method" as the label?

added UX and removed UX ready labels

@tauriedavis @pedroms what do you mean "sync across servers"?

@tauriedavis "Sync method" sounds good to me.

Either way, I think the new names make sense. Thanks for explaining.

changed the description

I updated the description with the new label name.

@pedroms did you happen to push your mockups to the design repo?

@dblessing it's more “sync across a server” than “across servers”. The user filter option will allow users to enter queries that search and match LDAP users in the entire LDAP server, and that query may not have any group filter in it.

@tauriedavis thanks for following up on such short notice and updating the description those are browser screenshots I took from my GDK (I tweaked things manually in DevTools)

@pedroms maybe it's just me or some OCD from myself but should it be Sync Method instead of Sync method?

The rest of the labels have a capital letter on the second word ¯\＿(ツ)＿/¯

They should be sentence case, as per our copy guidelines. I'll update the mockup! Thanks!

changed the description

Mockups updated - moving to UX ready

added UX ready and removed UX labels

added In review and removed In dev labels

closed via merge request !2885 (merged)

LDAP - Support user-level filters / mappings offering finer level of control than just Group mappings

Description

Use cases

Proposal

Design

Feature checklist

Designs

Child items ...

Activity

Admin message

Admin message

LDAP - Support user-level filters / mappings offering finer level of control than just Group mappings

Description

Use cases

Proposal

Design

Feature checklist

Relates to

Activity