Merge branch 'improve/ldap_respect_group_access' of /home/git/repositories/gitlab/gitlab-ee

lib/gitlab/ldap/access.rb

@@ -40,13 +40,22 @@ module Gitlab
         end
       end
 
+      # Add user to GitLab group
+      # In case user already exists: update his access level
+      # only if existing permissions are lower than ldap one.
       def add_user_to_groups(user_id, group_cn)
         groups = ::Group.where(ldap_cn: group_cn)
         groups.each do |group|
-          group.add_users([user_id], group.ldap_access) if group.ldap_access.present?
+          next unless group.ldap_access.present?
+
+          group_access = group.users_groups.find_by_user_id(user_id)
+          next if group_access && group_access.group_access >= group.ldap_access
+
+          group.add_users([user_id], group.ldap_access)
         end
       end
 
+      # Remove user from GitLab group
       def remove_user_from_groups(user_id, group_cn)
         groups = ::Group.where(ldap_cn: group_cn)
         groups.each do |group|

spec/lib/gitlab/ldap/ldap_access_spec.rb

+require 'spec_helper'
+
+describe Gitlab::LDAP::Access do
+  let(:access) { Gitlab::LDAP::Access.new }
+  let(:user) { create(:user) }
+  let(:group) { create(:group, ldap_cn: 'oss', ldap_access: Gitlab::Access::DEVELOPER) }
+
+  before do
+    group
+  end
+
+  describe :add_user_to_groups do
+    it "should add user to group" do
+      access.add_user_to_groups(user.id, "oss")
+      member = group.members.first
+      member.user.should == user
+      member.group_access.should == Gitlab::Access::DEVELOPER
+    end
+
+    it "should respect higher permissions" do
+      group.add_owner(user)
+      access.add_user_to_groups(user.id, "oss")
+      group.owners.should include(user)
+    end
+
+    it "should update lower permissions" do
+      group.add_user(user, Gitlab::Access::REPORTER)
+      access.add_user_to_groups(user.id, "oss")
+      member = group.members.first
+      member.user.should == user
+      member.group_access.should == Gitlab::Access::DEVELOPER
+    end
+  end
+end