Adambean / gitlab-ce-ldap-sync

Synchronise users and groups (including group members) from an LDAP instance with Gitlab CE (and EE in free tier) self-hosted instance(s).
Apache License 2.0
59 stars 23 forks source link

Remove users from groups #2

Closed jsiegers closed 5 years ago

jsiegers commented 5 years ago

Hi,

Thanks for this awesome tool! I'm using the FreeIPA directory servers and got it working. The only thing I can't do is remove users from a group. I've created an LDAP group (team1) and placed users in them. During the sync the group is then created and the users are added as members. All fine. When I now remove a user from the LDAP group and then do a sync I get the following error:

[root@fipa01 gitlab-ce-ldap-sync]# php bin/console ldap:sync -vv
LDAP users and groups sync script for Gitlab-CE

[notice] Loading configuration.
[notice] Loaded configuration.
[notice] Validating configuration.
[notice] Validated configuration.
[notice] Retrieving directory users and groups.
[notice] Establishing LDAP connection.
[notice] LDAP connection established.
[notice] 4 directory user(s) found.
[info] User "gitlab" in ignore list.
[info] Found directory user "jsiegers" [uid=jsiegers,cn=users,cn=accounts,dc=postlab,dc=cloud].
[info] Found directory user "hdjong" [uid=hdjong,cn=users,cn=accounts,dc=postlab,dc=cloud].
[notice] 2 directory user(s) recognised.
[notice] 9 directory group(s) found.
[info] Found directory group "team1".
[info] Found directory group "team1" member "jsiegers".
[warning] Group #1 / member #2: User not found "gitlab".
[notice] 1 directory group "team1" member(s) recognised.
[info] Group "externalusers" in ignore list.
[info] Group "gitlabadmins" in ignore list.
[info] Found directory group "jsiegers".
[warning] Group #4: Missing attribute "memberuid". (Could also mean this group has no members.)
[info] Found directory group "gitlab".
[warning] Group #5: Missing attribute "memberuid". (Could also mean this group has no members.)
[info] Group "postlab-cloud" in ignore list.
[info] Group "editors" in ignore list.
[info] Group "admins" in ignore list.
[notice] 3 directory group(s) recognised.
[notice] LDAP connection closed.
[notice] Retrieved directory users and groups.
[notice] Deploying users and groups to Gitlab instances.
[notice] Establishing Gitlab connection.
[notice] Finding all existing Gitlab users...
[info] Found Gitlab user #38 "jsiegers".
[info] Found Gitlab user #37 "hdjong".
[info] Found Gitlab user #4 "ghost".
[info] Gitlab built-in root user will be ignored.
[notice] 3 Gitlab user(s) found.
[notice] Creating directory users of which don't exist in Gitlab...
[notice] 0 Gitlab user(s) created.
[notice] Disabling Gitlab users of which don't exist in directory...
[info] User "ghost" in ignore list.
[notice] 0 Gitlab user(s) disabled.
[notice] Updating users of which were already in both Gitlab and the directory...
[info] User "ghost" in ignore list.
[info] Updating Gitlab user #37 "hdjong".
[info] Updating Gitlab user #38 "jsiegers".
[notice] 2 Gitlab user(s) updated.
[notice] Finding all existing Gitlab groups...
[info] Found Gitlab group #4 "team1" [team1].
[notice] 1 Gitlab group(s) found.
[notice] Creating directory groups of which don't exist in Gitlab...
[warning] Not creating Gitlab group "gitlab" [gitlab]: No members in directory group, or config gitlab->options->createEmptyGroups is disabled.
[warning] Not creating Gitlab group "jsiegers" [jsiegers]: No members in directory group, or config gitlab->options->createEmptyGroups is disabled.
[notice] 0 Gitlab group(s) created.
[notice] Deleting Gitlab groups of which don't exist in directory...
[notice] 0 Gitlab group(s) deleted.
[notice] Updating groups of which were already in both Gitlab and the directory...
[info] Updating Gitlab group #4 "team1" [team1].
[notice] 1 Gitlab group(s) updated.
[notice] Synchronising Gitlab group members with directory group members...
[notice] Synchronising 1 member(s) for group #4 "team1" [team1]...
[notice] Finding existing group members...
[info] Gitlab built-in root user will be ignored.
[info] Found Gitlab group member #38 "jsiegers".
**[error] Group member #3: User not found.**
[notice] 1 Gitlab group "team1" [team1] member(s) found.
[notice] Adding missing group members...
[notice] 0 Gitlab group "team1" [team1] member(s) added.
[notice] Deleting extra group members...
[notice] 0 Gitlab group "team1" [team1] member(s) deleted.
[notice] Gitlab connection closed.
[notice] Deployed users and groups to Gitlab instances.
[root@fipa01 gitlab-ce-ldap-sync]#

I'm expecting that user is removed from the gitlab group as well during the Deleting extra group members. But that doesn't seem the case. Am I missing something or is a bug?

Cheers in advance!

Adambean commented 5 years ago

I get that too. I don't think the user (on Gitlab) is being removed, the error looks like a non-existent group-member relationship.

This is quite a simple check:

if (!isset($membersOfThisGroup[$gitlabUserId]) || $membersOfThisGroup[$gitlabUserId] != $gitlabUserName) {
    $this->logger->error(sprintf("Group member #%d: User not found.", $n));
    continue;
}

I suspect this issue is happening because of how the array $membersOfThisGroup is being built:

$membersOfThisGroup = [];
foreach ($usersToSyncMembership as $gitlabUserId => $gitlabUserName) {
    if (!$this->in_array_i($gitlabUserName, $ldapGroupsSafe[$gitlabGroupName])) {
        continue;
    }

    $membersOfThisGroup[$gitlabUserId] = $gitlabUserName;
}
asort($membersOfThisGroup);
$this->logger->notice(sprintf("Synchronising %d member(s) for group #%d \"%s\" [%s]...", ($membersOfThisGroupNum = count($membersOfThisGroup)), $gitlabGroupId, $gitlabGroupName, $gitlabGroupPath));

This is a concatenation of 3 arrays earlier:

$usersToSyncMembership  = ($usersSync["found"] + $usersSync["new"] + $usersSync["update"]);
Adambean commented 5 years ago

Actually thinking about it the problem is probably right here:

if (!$this->in_array_i($gitlabUserName, $ldapGroupsSafe[$gitlabGroupName])) {
    continue;
}

At this point the user you've taken out of the group is no longer in the $ldapGroupsSafe[$gitlabGroupName] array, so that user would not be added to the $membersOfThisGroup array for comparison later on.

If so this could be resolved by commenting out lines 1351-1353, then if it looks successful after a --dryrun, delete them entirely. I will try this later this evening, but you're welcome to do so sooner and let me know if it worked.

Adambean commented 5 years ago

Disregard that. ^ The comparison on lines 1409-1411 was what needed removing. I've just tried this now and it appears to have removed extra group memberships correctly.

Adambean commented 5 years ago

Release 0.0.3 contains this fix amongst half a dozen others.

jsiegers commented 5 years ago

Wow that was really fast! Nice work!

Adambean commented 5 years ago

It just so happened that I didn't have much on last night. 🙂 Let me know if this fix didn't work for you.

jsiegers commented 5 years ago

Just tested it and works now! Cheers!