LDAP group sync never completes when subgroup gives lower permission to a user

Summary

Consider the following scenario:

  • group parent_group with subgroup child_group
  • parent_group gives a high level of permission (e.g. Maintainer) to ldap_group_1
  • child_group gives a lower level of permission (e.g. Guest) to ldap_group_2

If a user is member of both ldap_group_1 and ldap_group_2, then LDAP sync will never complete for child_group. In the child group members page, the group is forever in state "syncing..."

Steps to reproduce

  1. create a group parent_group, grant Maintainer permission to ldap_group_1
  2. create a subgroup child_group inside parent_group, grant Guest permission to ldap_group_2
  3. add a user to both ldap_group_1 and ldap_group_2
  4. make sure LDAP sync completes for parent_group
  5. from the child_group members page, click Sync now

Example Project

N/A

What is the current bug behavior?

LDAP sync never completes for child group. The sync state will be forever Syncing...

What is the expected correct behavior?

LDAP sync completes and user member of both LDAP group gets the highest permission (i.e. Maintainer)

Relevant logs and/or screenshots

In the rails console we can see that there are errors in this case because of inherited membership: (applying logic from https://gitlab.com/gitlab-org/gitlab-ee/blob/master/ee/lib/ee/gitlab/auth/ldap/sync/group.rb)

> g=Group.find_by_full_path("parent_group/child_group")
> g.start_ldap_sync
> EE::Gitlab::Auth::LDAP::Sync::Proxy.open(::Gitlab::Auth::LDAP::Config.providers.first) {|proxy|  EE::Gitlab::Auth::LDAP::Sync::Group.new(g, proxy).update_permissions}
> g.finish_ldap_sync
=> false # THIS IS THE ISSUE, SYNC CANNOT FINISH!
> g.members.first.errors
=> #<ActiveModel::Errors:0x00007f1344524b90 @base=#<GroupMember id: 1042736, access_level: 10, source_id: 16566, source_type: "Namespace", user_id: 6630, notification_level: 3, type: "GroupMember", created_at: "2019-03-01 10:00:43", updated_at: "2019-03-01 10:00:43", created_by_id: nil, invite_email: nil, invite_token: nil, invite_accepted_at: nil, requested_at: nil, expires_at: nil, ldap: true, override: false>, @messages={:access_level=>["should be higher than Guest inherited membership from group XXXXX"]}, @details={:access_level=>[{:error=>"should be higher than Guest inherited membership from group XXXX"}]}>

Output of checks

(If you are reporting a bug on GitLab.com, write: This bug happens on GitLab.com)

Results of GitLab environment info

Expand for output related to GitLab environment info 
System information
System:
Proxy:          no
Current User:   git
Using RVM:      no
Ruby Version:   2.5.3p105
Gem Version:    2.7.6
Bundler Version:1.16.6
Rake Version:   12.3.2
Redis Version:  3.2.12
Git Version:    2.18.1
Sidekiq Version:5.2.3
Go Version:     unknown

GitLab information
Version:        11.6.9-ee
Revision:       d6fc7c7
Directory:      /opt/gitlab/embedded/service/gitlab-rails
DB Adapter:     postgresql
DB Version:     9.6.2
URL:            https://gitlab.xxxxx
HTTP Clone URL: https://gitlab.xxxxx/some-group/some-project.git
SSH Clone URL:  ssh://git@gitlab.xxxxx:7999/some-group/some-project.git
Elasticsearch:  no
Geo:            no
Using LDAP:     yes
Using Omniauth: yes
Omniauth Providers: saml, kerberos_spnego


GitLab Shell
Version:        8.4.3
Repository storage paths:
xxxxx
Hooks:          /opt/gitlab/embedded/service/gitlab-shell/hooks
Git:            /opt/gitlab/embedded/bin/git

Results of GitLab application Check

Possible fixes

Possibly the LDAP sync should check for existing inherited membership for the same user?

Edited by Alex Lossent