Merge branch 'feature-120-multiple-ldap-groups' into 'master'

Add support for multiple LDAP groups per Gitlab group

concerns #120

See merge request !143

app/controllers/admin/groups_controller.rb

@@ -58,6 +58,6 @@ class Admin::GroupsController < Admin::ApplicationController
   end
 
   def group_params
-    params.require(:group).permit(:name, :description, :path, :avatar, :ldap_cn, :ldap_access)
+    params.require(:group).permit(:name, :description, :path, :avatar)
   end
 end

app/controllers/groups/ldap_group_links_controller.rb

+class Groups::LdapGroupLinksController < ApplicationController
+  before_action :group
+  before_action :authorize_admin_group!
+
+  def index
+  end
+
+  def create
+    ldap_group_link = @group.ldap_group_links.build(ldap_group_link_params)
+    if ldap_group_link.save
+      redirect_to :back, notice: 'New LDAP link saved'
+    else
+      redirect_to :back, alert: 'Could not create new LDAP link'
+    end
+  end
+
+  def destroy
+    @group.ldap_group_links.where(id: params[:id]).destroy_all
+    redirect_to :back, notice: 'LDAP link removed'
+  end
+
+  private
+
+  def group
+    @group ||= Group.find_by(path: params[:group_id])
+  end
+
+  def authorize_admin_group!
+    render_404 unless can?(current_user, :manage_group, group)
+  end
+
+  def ldap_group_link_params
+    params.require(:ldap_group_link).permit(:cn, :group_access)
+  end
+end
\ No newline at end of file

app/controllers/groups_controller.rb

@@ -163,6 +163,6 @@ class GroupsController < ApplicationController
   end
 
   def group_params
-    params.require(:group).permit(:name, :description, :path, :avatar, :ldap_access, :ldap_cn)
+    params.require(:group).permit(:name, :description, :path, :avatar)
   end
 end

app/models/group.rb

@@ -21,11 +21,7 @@ class Group < Namespace
   has_many :users, through: :users_groups
   has_many :project_group_links, dependent: :destroy
   has_many :shared_projects, through: :project_group_links, source: :project
-
-  validates :ldap_access,
-    inclusion: { in: UsersGroup.group_access_roles.values },
-    presence: true,
-    if: ->(group) { group.ldap_cn.present? }
+  has_many :ldap_group_links, foreign_key: 'group_id', dependent: :destroy
 
   validate :avatar_type, if: ->(user) { user.avatar_changed? }
   validates :avatar, file_size: { maximum: 100.kilobytes.to_i }
@@ -85,6 +81,15 @@ class Group < Namespace
     projects.public_only.any?
   end
 
+  # NOTE: Backwards compatibility with old ldap situation
+  def ldap_cn
+    ldap_group_links.first.try(:cn)
+  end
+
+  def ldap_access
+    ldap_group_links.first.try(:group_access)
+  end
+
   class << self
     def search(query)
       where("LOWER(namespaces.name) LIKE :query", query: "%#{query.downcase}%")

app/models/ldap_group_link.rb

+class LdapGroupLink < ActiveRecord::Base
+  include Gitlab::Access
+  belongs_to :group
+
+  validates :cn, :group_access, :group_id, presence: true
+  validates :cn, uniqueness: { scope: :group_id }
+  validates :group_access, inclusion: { in: UsersGroup.group_access_roles.values }
+
+  def access_field
+    group_access
+  end
+end

app/models/users_group.rb

@@ -27,6 +27,8 @@ class UsersGroup < ActiveRecord::Base
   scope :developers, -> { where(group_access: DEVELOPER) }
   scope :masters,  -> { where(group_access: MASTER) }
   scope :owners,  -> { where(group_access: OWNER) }
+  scope :with_ldap_dn, -> { references(:user).includes(:user).
+    where(users: { provider: 'ldap' }) }
 
   scope :with_group, ->(group) { where(group_id: group.id) }
   scope :with_user, ->(user) { where(user_id: user.id) }

app/services/ldap_group_reset_service.rb

 class LdapGroupResetService
   def execute(group, current_user)
-    group.members.includes(:user).each do |member|
-      user = member.user
+    # Only for ldap connected users
+    # reset last_credential_check_at to force LDAP::Access::update_permissions
+    # set Gitlab::Access::Guest to later on upgrade the access of a user
 
-      if user.ldap_user? && user != current_user
-        member.group_access = group.ldap_access
-        member.save
-      end
+    # trigger the lowest access possible for all LDAP connected users
+    a = group.members.with_ldap_dn.map do |member|
+      # don't unauthorize the current user
+      next if current_user == member.user
+      member.update_attribute :group_access, Gitlab::Access::GUEST
     end
+
+    group.users.ldap.update_all last_credential_check_at: nil
   end
 end

app/views/admin/groups/_form.html.haml

@@ -52,30 +52,11 @@
             %li It will change web url for access group and group projects.
             %li It will change the git path to repositories under this group.
 
-      %fieldset
-        %legend LDAP group settings
-        %div.form-holder
-          .form-group.clearfix
-            = f.label :ldap_cn, class: 'control-label' do
-              LDAP Group cn
-            .col-sm-10
-              = f.hidden_field :ldap_cn, placeholder: "Ex. QA group", class: "xxlarge ajax-ldap-groups-select input-mn-300"
-              .help-block
-                Synchronize #{@group.name}'s members with this LDAP group.
-                %br
-                If you select an LDAP group you do not belong to you will lose ownership of #{@group.name}.
-
-          .form-group.clearfix
-            = f.label :ldap_access, class: 'control-label' do
-              LDAP Access
-            .col-sm-10
-              = f.select :ldap_access, options_for_select(UsersGroup.group_access_roles, @group.ldap_access)
-              .help-block
-                Default, minimum permission level for LDAP group members of #{@group.name}.
-                %br
-                You can manage permission levels for individual group members in the Members tab.
-
     .form-actions
       = f.submit 'Save changes', class: "btn btn-primary"
       = link_to  'Cancel', admin_group_path(@group), class: "btn btn-cancel"
 
+- if @group.persisted?
+  %h3.page-title Linked LDAP groups
+  = render 'ldap_group_links/form', group: @group
+  = render 'ldap_group_links/ldap_group_links', group: @group

app/views/admin/groups/show.html.haml

@@ -31,16 +31,16 @@
           %strong
             = @group.created_at.stamp("March 1, 1999")
 
-        - if @group.ldap_cn.present?
-          %li
-            %span.light LDAP group cn:
-            %strong
-              = @group.ldap_cn
-
+    .panel.panel-default
+      .panel-heading Linked LDAP groups
+      %ul.well-list
+        - if @group.ldap_group_links.any?
+          - @group.ldap_group_links.each do |ldap_group_link|
             %li
-            %span.light LDAP access level:
-            %strong
-              = @group.human_ldap_access
+              cn:
+              %strong= ldap_group_link.cn
+              as
+              %strong= ldap_group_link.human_access
 
     .panel.panel-default
       .panel-heading

app/views/groups/_settings_nav.html.haml

@@ -7,4 +7,8 @@
     = link_to projects_group_path(@group) do
       %i.icon-folder-close
       Projects
+  = nav_link(controller: :ldap_group_links) do
+    = link_to group_ldap_group_links_path(@group) do
+      %i.icon-exchange
+      LDAP Groups
 

app/views/groups/edit.html.haml

@@ -41,29 +41,6 @@
                 %hr
                 = link_to 'Remove avatar', group_avatar_path(@group.to_param), data: { confirm: "Group avatar will be removed. Are you sure?"}, method: :delete, class: "btn btn-remove btn-small remove-avatar"
 
-          %fieldset
-            %legend LDAP settings
-            .form-group.clearfix
-              = f.label :ldap_cn, class: 'control-label' do
-                LDAP Group cn
-              .col-sm-10
-                = f.hidden_field :ldap_cn, placeholder: "Ex. QA group", class: "xxlarge ajax-ldap-groups-select input-mn-300"
-                .help-block
-                  Synchronize #{@group.name}'s members with this LDAP group.
-                  %br
-                  If you select an LDAP group you do not belong to you will lose ownership of #{@group.name}.
-
-            .form-group.clearfix
-              = f.label :ldap_access, class: 'control-label' do
-                LDAP Access
-              .col-sm-10
-                = f.select :ldap_access, options_for_select(UsersGroup.group_access_roles, @group.ldap_access)
-                .help-block
-                  Default, minimum permission level for LDAP group members of #{@group.name}.
-                  %br
-                  You can manage permission levels for individual group members in the Members tab.
-
-
           .form-actions
             = f.submit 'Save group', class: "btn btn-save"
 

app/views/groups/ldap_group_links/index.html.haml

+.row
+  .col-md-2= render 'groups/settings_nav'
+  .col-md-10
+    %h3.page-title Linked LDAP groups
+    = render 'ldap_group_links/form', group: @group
+    = render 'ldap_group_links/ldap_group_links', group: @group
+

app/views/groups/members.html.haml

@@ -17,7 +17,7 @@
 
   - if current_user && current_user.can?(:manage_group, @group)
     .pull-right
-      - if ldap_enabled? && @group.ldap_cn.present?
+      - if ldap_enabled? && @group.ldap_group_links.any?
         = link_to reset_access_group_ldap_path(@group), class: 'btn btn-grouped', data: { confirm: "Reset the access level of all other LDAP group team members to '#{@group.human_ldap_access}'?" }, method: :put do
           Reset access to #{@group.human_ldap_access}
 
@@ -28,13 +28,18 @@
     .js-toggle-content.hide.new-group-member-holder
       = render "new_group_member"
 
-- if ldap_enabled? && @group.ldap_cn.present?
+- if ldap_enabled? && @group.ldap_group_links.any?
   .bs-callout.bs-callout-info
-    The members of this group are synced with the LDAP group with cn
-    %code #{@group.ldap_cn}
-    \. They are given
-    %code #{@group.human_ldap_access}
-    access. Because LDAP permissions in GitLab get updated one user at a time and because GitLab caches LDAP check results, changes on your LDAP server or in this group's LDAP sync settings may take up to #{Gitlab.config.ldap['sync_time']}s to show in the list below.
+    The members of this group are sync with LDAP.
+    Because LDAP permissions in GitLab get updated one user at a time and because GitLab caches LDAP check results, changes on your LDAP server or in this group's LDAP sync settings may take up to #{Gitlab.config.ldap['sync_time']}s to show in the list below.
+    %ul
+      - @group.ldap_group_links.each do |ldap_group_link|
+        %li
+          People in cn
+          %code= ldap_group_link.cn
+          are given
+          %code= ldap_group_link.human_access
+          access.
 
 .panel.panel-default.prepend-top-20
   .panel-heading

app/views/ldap_group_links/_form.html.haml

+%section.ldap-group-links
+  = form_for [group, LdapGroupLink.new] do |f|
+    %fieldset
+      %legend
+      %div.form-holder
+        .form-group.clearfix
+          = f.label :cn, class: 'control-label' do
+            LDAP Group cn
+          .col-sm-10
+            = f.hidden_field :cn, placeholder: "Ex. QA group", class: "xxlarge ajax-ldap-groups-select input-mn-300"
+            .help-block
+              Synchronize #{group.name}'s members with this LDAP group.
+              %br
+              If you select an LDAP group you do not belong to you will lose ownership of #{group.name}.
+
+        .form-group.clearfix
+          = f.label :group_access, class: 'control-label' do
+            LDAP Access
+          .col-sm-10
+            = f.select :group_access, options_for_select(UsersGroup.group_access_roles)
+            .help-block
+              Default, minimum permission level for LDAP group members of #{group.name}.
+              %br
+              You can manage permission levels for individual group members in the Members tab.
+
+      .form-actions
+        = f.submit 'Add synchronization', class: "btn btn-create"
\ No newline at end of file

app/views/ldap_group_links/_ldap_group_link.html.haml

+%li
+  = ldap_group_link.cn
+  %small.light== as #{ldap_group_link.human_access}
+  .pull-right
+    = link_to group_ldap_group_link_path(group, ldap_group_link), method: :delete, class: 'btn btn-danger btn-small' do
+      = fa_icon('unlink', text: 'unlink')

app/views/ldap_group_links/_ldap_group_links.html.haml

+.panel.panel-default
+  .panel-heading
+    %h4.panel-title
+      Linked LDAP groups
+      == (#{group.ldap_group_links.count})
+
+  - if group.ldap_group_links.any?
+    %ul.well-list
+      = render collection: group.ldap_group_links, partial: 'ldap_group_links/ldap_group_link', locals: { group: group }
+  - else
+    %p No linked LDAP groups

config/routes.rb

@@ -178,8 +178,8 @@ Gitlab::Application.routes.draw do
     end
 
     resources :users_groups, only: [:create, :update, :destroy]
-
     scope module: :groups do
+      resources :ldap_group_links, only: [:index, :create, :destroy]
       resource :avatar, only: [:destroy]
       resources :milestones
     end

db/migrate/20140813090117_add_ldap_groups_table.rb

+class AddLdapGroupsTable < ActiveRecord::Migration
+  def up
+    create_table :ldap_groups do |t|
+      t.string :cn, null: false
+      t.integer :group_access, null: false
+      t.references :group, null: false
+
+      t.timestamps
+    end
+  end
+
+  def down
+    drop_table :ldap_groups
+  end
+end

db/migrate/20140813133925_rename_ldap_group_to_ldap_group_link.rb

+class RenameLdapGroupToLdapGroupLink < ActiveRecord::Migration
+  def up
+    rename_table :ldap_groups, :ldap_group_links
+
+    # NOTE: we use the old_ methods because the new methods are overloaded
+    # for backwards compatibility
+
+    Group.where.not(ldap_cn: nil).each do |group|
+      # Make sure we use the database column, not the model methods
+      ldap_cn = group.read_attribute(:ldap_cn)
+      ldap_access = group.read_attribute(:ldap_access)
+
+      group.ldap_group_links.where(cn: ldap_cn).first_or_create do |ldap_group_link|
+        ldap_group_link.group_access = ldap_access
+      end
+    end
+  end
+
+  def down
+    rename_table :ldap_group_links, :ldap_groups
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20140811155127) do
+ActiveRecord::Schema.define(version: 20140813133925) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -149,6 +149,14 @@ ActiveRecord::Schema.define(version: 20140811155127) do
 
   add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree
 
+  create_table "ldap_group_links", force: true do |t|
+    t.string   "cn",           null: false
+    t.integer  "group_access", null: false
+    t.integer  "group_id",     null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
+  end
+
   create_table "merge_request_diffs", force: true do |t|
     t.string   "state"
     t.text     "st_commits"

features/group.feature

@@ -55,6 +55,11 @@ Feature: Groups
     Then I should not see group "Owned" avatar
     And I should not see the "Remove avatar" button
 
+  Scenario: Add new LDAP synchronization
+    When I visit Group "Owned" LDAP settings page
+    And I add a new LDAP synchronization
+    Then I see a new LDAP synchronization listed
+
   # Leave
 
   @javascript

features/steps/group/group.rb

@@ -285,4 +285,18 @@ class Groups < Spinach::FeatureSteps
             author: current_user,
             milestone: milestone2_project3
   end
+
+  step 'I add a new LDAP synchronization' do
+    within('form#new_ldap_group_link') do
+      find('#ldap_group_link_cn', visible: false).set('my-group-cn')
+      # fill_in('LDAP Group cn', with: 'my-group-cn', visible: false)
+      select 'Developer', from: "ldap_group_link_group_access"
+      click_button 'Add synchronization'
+    end
+  end
+
+  step 'I see a new LDAP synchronization listed' do
+    expect(page).not_to have_content('No synchronizations yet')
+    expect(page).to have_content('my-group-cn as Developer')
+  end
 end

features/steps/shared/paths.rb

@@ -42,6 +42,10 @@ module SharedPaths
     visit edit_group_path(Group.find_by(name:"Owned"))
   end
 
+  step 'I visit group "Owned" LDAP settings page' do
+    visit group_ldap_group_links_path(Group.find_by(name:"Owned"))
+  end
+
   step 'I visit group "Guest" page' do
     visit group_path(Group.find_by(name:"Guest"))
   end

lib/api/entities.rb

@@ -60,6 +60,11 @@ module API
 
     class Group < Grape::Entity
       expose :id, :name, :path, :owner_id, :ldap_cn, :ldap_access
+      expose :ldap_group_links, if: ->(group, _) { group.ldap_group_links.any? } do |group, _|
+        group.ldap_group_links.map do |group_link|
+          group_link.slice(:cn, :group_access)
+        end
+      end
     end
 
     class GroupDetail < Group

lib/api/groups.rb

@@ -44,11 +44,19 @@ module API
         authenticated_as_admin!
         required_attributes! [:name, :path]
 
-        attrs = attributes_for_keys [:name, :path, :ldap_cn, :ldap_access]
-        @group = Group.new(attrs)
+        group_attrs = attributes_for_keys [:name, :path]
+        @group = Group.new(group_attrs)
         @group.owner = current_user
 
         if @group.save
+          # NOTE: add backwards compatibility for single ldap link
+          ldap_attrs  = attributes_for_keys [:ldap_cn, :ldap_access]
+          if ldap_attrs.present?
+            @group.ldap_group_links.create({
+              cn: ldap_attrs[:ldap_cn],
+              group_access: ldap_attrs[:ldap_access]
+            })
+          end
           present @group, with: Entities::Group
         else
           not_found!

lib/gitlab/ldap/access.rb

@@ -37,10 +37,11 @@ module Gitlab
         false
       end
 
-      def update_permissions(user)
-        # Get LDAP user entry
-        ldap_user = Gitlab::LDAP::Person.find_by_dn(user.extern_uid)
+      def get_ldap_user(user)
+        @ldap_user ||= Gitlab::LDAP::Person.find_by_dn(user.extern_uid)
+      end
 
+      def update_permissions(user)
         if Gitlab.config.ldap['sync_ssh_keys']
           update_ssh_keys(user)
         end
@@ -49,23 +50,8 @@ module Gitlab
         # if instance does not use group_base setting
         return true unless Gitlab.config.ldap['group_base'].present?
 
-        # Get all GitLab groups with activated LDAP
-        groups = ::Group.where('ldap_cn IS NOT NULL')
-
-        # Get LDAP groups based on cn from GitLab groups
-        ldap_groups = groups.pluck(:ldap_cn).map { |cn| Gitlab::LDAP::Group.find_by_cn(cn, adapter) }
-        ldap_groups = ldap_groups.compact.uniq
+        update_ldap_group_links(user)
 
-        # Iterate over ldap groups and check user membership
-        ldap_groups.each do |ldap_group|
-          if ldap_group.has_member?(ldap_user)
-            # If user present in LDAP group -> add him to GitLab groups
-            add_user_to_groups(user.id, ldap_group.cn)
-          else
-            # If not - remove him from GitLab groups
-            remove_user_from_groups(user.id, ldap_group.cn)
-          end
-        end
         if Gitlab.config.ldap['admin_group'].present?
           update_admin_status(user)
         end
@@ -74,7 +60,7 @@ module Gitlab
       # Update user ssh keys if they changed in LDAP
       def update_ssh_keys(user)
         # Get LDAP user entry
-        ldap_user = Gitlab::LDAP::Person.find_by_dn(user.extern_uid)
+        ldap_user = get_ldap_user(user)
 
         user.keys.ldap.where.not(key: ldap_user.ssh_keys).each do |deleted_key|
           Rails.logger.info "#{self.class.name}: removing LDAP SSH key #{deleted_key.key} from #{user.name} (#{user.id})"
@@ -97,7 +83,7 @@ module Gitlab
       # Update user email if it changed in LDAP
       def update_email(user)
         uid = user.extern_uid
-        ldap_user = Gitlab::LDAP::Person.find_by_dn(uid, adapter)
+        ldap_user = get_ldap_user(user)
         gitlab_user = ::User.where(provider: 'ldap', extern_uid: uid).last
 
         if gitlab_user && ldap_user && ldap_user.email
@@ -113,29 +99,6 @@ 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|
-          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|
-          group.users_groups.where(user_id: user_id).destroy_all
-        end
-      end
-
       def update_admin_status(user)
         admin_group = Gitlab::LDAP::Group.find_by_cn(Gitlab.config.ldap['admin_group'], adapter)
         if admin_group.has_member?(Gitlab::LDAP::Person.find_by_dn(user.extern_uid, adapter))
@@ -150,6 +113,49 @@ module Gitlab
           end
         end
       end
+
+      def ldap_groups
+        @ldap_groups ||= ::LdapGroupLink.distinct(:cn).pluck(:cn).map do |cn|
+          Gitlab::LDAP::Group.find_by_cn(cn, adapter)
+        end.compact
+      end
+
+      # returns a collection of cn strings to which the user has access
+      def cns_with_access(ldap_user)
+        @ldap_groups_with_access ||= ldap_groups.select do |ldap_group|
+          ldap_group.has_member?(ldap_user)
+        end.map(&:cn)
+      end
+
+      def gitlab_groups_with_ldap_link
+        ::Group.includes(:ldap_group_links).references(:ldap_group_links).
+          where.not(ldap_group_links: { id: nil })
+      end
+
+      # Loop throug all ldap conneted groups, and update the users link with it
+      def update_ldap_group_links(user)
+        gitlab_groups_with_ldap_link.each do |group|
+          active_group_links = group.ldap_group_links.where(cn: cns_with_access(get_ldap_user(user)))
+
+          if active_group_links.any?
+            group.add_users([user.id], fetch_group_access(group, user, active_group_links))
+          else
+            group.users.delete(user)
+          end
+        end
+      end
+
+      # Get the group_access for a give user.
+      # Always respect the current level, never downgrade it.
+      def fetch_group_access(group, user, active_group_links)
+        current_access_level = group.users_groups.where(user_id: user).maximum(:group_access)
+        max_group_access_level = active_group_links.maximum(:group_access)
+
+        # TODO: Test if nil value of current_access_level in handled properly
+        [current_access_level, max_group_access_level].compact.max
+      end
     end
   end
 end
+
+

spec/lib/gitlab/ldap/ldap_access_spec.rb → spec/lib/gitlab/ldap/access_spec.rb

@@ -3,34 +3,6 @@ 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
 
   describe :update_user_email do
     let(:user_ldap) { create(:user, provider: 'ldap', extern_uid: "66048")}
@@ -208,4 +180,134 @@ objectclass: posixGroup
       expect(gitlab_admin.admin?).to be false
     end
   end
+
+  describe 'ldap_groups' do
+    let(:ldap_group_1) do
+      Net::LDAP::Entry.from_single_ldif_string(
+%Q{dn: cn=#{Gitlab.config.ldap['admin_group']},ou=groups,dc=bar,dc=com
+cn: #{Gitlab.config.ldap['admin_group']}
+description: GitLab group 1
+gidnumber: 42
+memberuid: user1
+memberuid: user2
+objectclass: top
+objectclass: posixGroup
+})
+    end
+
+    it "returns an interator of LDAP Groups" do
+      ::LdapGroupLink.create cn: 'example', group_access: Gitlab::Access::DEVELOPER, group_id: 42
+      Gitlab::LDAP::Adapter.any_instance.stub(:group) { Gitlab::LDAP::Group.new(ldap_group_1) }
+
+      expect(access.ldap_groups.first).to be_a Gitlab::LDAP::Group
+    end
+
+    it "only returns found ldap groups" do
+      ::LdapGroupLink.create cn: 'example', group_access: Gitlab::Access::DEVELOPER, group_id: 42
+      Gitlab::LDAP::Group.stub(find_by_cn: nil) # group not found
+
+      expect(access.ldap_groups).to be_empty
+    end
+  end
+
+  describe :cns_with_access do
+    let(:ldap_group_response_1) do
+      Net::LDAP::Entry.from_single_ldif_string(
+%Q{dn: cn=group1,ou=groups,dc=bar,dc=com
+cn: group1
+description: GitLab group 1
+gidnumber: 21
+memberuid: #{ldap_user.uid}
+memberuid: user2
+objectclass: top
+objectclass: posixGroup
+})
+    end
+    let(:ldap_group_response_2) do
+      Net::LDAP::Entry.from_single_ldif_string(
+%Q{dn: cn=group2,ou=groups,dc=bar,dc=com
+cn: group2
+description: GitLab group 2
+gidnumber: 42
+memberuid: user3
+memberuid: user4
+objectclass: top
+objectclass: posixGroup
+})
+    end
+    let(:ldap_groups) do
+      [
+        Gitlab::LDAP::Group.new(ldap_group_response_1),
+        Gitlab::LDAP::Group.new(ldap_group_response_2)
+      ]
+    end
+    let(:ldap_user) { Gitlab::LDAP::Person.new(Net::LDAP::Entry.new) }
+
+    before { ldap_user.stub(:uid) { 'user42' } }
+
+    it "only returns ldap cns to which the user has access" do
+      access.stub(ldap_groups: ldap_groups)
+      expect(access.cns_with_access(ldap_user)).to eql ['group1']
+    end
+  end
+
+  describe :update_ldap_group_links do
+    let(:cns_with_access) { %w(ldap-group1 ldap-group2) }
+    let(:gitlab_group_1) { create :group }
+    let(:gitlab_group_2) { create :group }
+
+    before do
+      access.stub(:get_ldap_user)
+      access.stub(cns_with_access: cns_with_access)
+    end
+
+    context "non existing access for group-1, allowed via ldap-group1 as MASTER" do
+      before do
+        gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::MASTER
+      end
+
+      it "gives the user master access for group 1" do
+        access.update_ldap_group_links(user)
+        expect( gitlab_group_1.has_master?(user) ).to be_true
+      end
+    end
+
+    context "existing access as guest for group-1, allowed via ldap-group1 as DEVELOPER" do
+      before do
+        gitlab_group_1.users_groups.guests.create(user_id: user.id)
+        gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::MASTER
+      end
+
+      it "upgrades the users access to master for group 1" do
+        expect { access.update_ldap_group_links(user) }.to \
+          change{ gitlab_group_1.has_master?(user) }.from(false).to(true)
+      end
+    end
+
+    context "existing access as MASTER for group-1, allowed via ldap-group1 as DEVELOPER" do
+      before do
+        gitlab_group_1.users_groups.masters.create(user_id: user.id)
+        gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::DEVELOPER
+      end
+
+      it "keeps the users master access for group 1" do
+        expect { access.update_ldap_group_links(user) }.not_to \
+          change{ gitlab_group_1.has_master?(user) }
+      end
+    end
+
+    context "existing access as master for group-1, not allowed" do
+      before do
+        gitlab_group_1.users_groups.masters.create(user_id: user.id)
+        gitlab_group_1.ldap_group_links.create cn: 'ldap-group1', group_access: Gitlab::Access::MASTER
+        access.stub(cns_with_access: ['ldap-group2'])
+      end
+
+      it "removes user from gitlab_group_1" do
+        expect { access.update_ldap_group_links(user) }.to \
+          change{ gitlab_group_1.members.where(user_id: user).any? }.from(true).to(false)
+      end
+    end
+  end
 end
+

spec/requests/api/groups_spec.rb

@@ -6,12 +6,13 @@ describe API::API, api: true  do
   let(:user1) { create(:user) }
   let(:user2) { create(:user) }
   let(:admin) { create(:admin) }
-  let!(:group1) { create(:group, ldap_cn: "ldap-group", ldap_access: Gitlab::Access::MASTER ) }
+  let!(:group1) { create(:group) }
   let!(:group2) { create(:group) }
 
   before do
     group1.add_owner(user1)
     group2.add_owner(user2)
+    group1.ldap_group_links.create cn: 'ldap-group', group_access: Gitlab::Access::MASTER
   end
 
   describe "GET /groups" do
@@ -31,6 +32,10 @@ describe API::API, api: true  do
         json_response.first['name'].should == group1.name
         json_response.first['ldap_cn'].should == group1.ldap_cn
         json_response.first['ldap_access'].should == group1.ldap_access
+
+        ldap_group_link = json_response.first['ldap_group_links'].first
+        ldap_group_link['cn'].should == group1.ldap_cn
+        ldap_group_link['group_access'].should == group1.ldap_access
       end
     end
 
@@ -105,6 +110,13 @@ describe API::API, api: true  do
         post api("/groups", admin), { name: 'test' }
         response.status.should == 400
       end
+
+      it "creates an ldap_group_link if ldap_cn and ldap_access are supplied" do
+        group_attributes = attributes_for(:group, ldap_cn: 'ldap-group', ldap_access: Gitlab::Access::DEVELOPER)
+        expect {
+          post api("/groups", admin), group_attributes
+        }.to change{ LdapGroupLink.count }.by(1)
+      end
     end
   end
 

spec/services/ldap_group_reset_service_spec.rb

 require 'spec_helper'
 
 describe LdapGroupResetService do
-  let(:group) { create(:group, ldap_cn: 'developers', ldap_access: Gitlab::Access::DEVELOPER) }
+  # TODO: refactor to multi-ldap setup
+  let(:group) { create(:group) }
   let(:user) { create(:user) }
-  let(:ldap_user) { create(:user, extern_uid: 'john', provider: 'ldap') }
-  let(:ldap_user_2) { create(:user, extern_uid: 'mike', provider: 'ldap') }
+  let(:ldap_user) { create(:user, extern_uid: 'john', provider: 'ldap', last_credential_check_at: Time.now) }
+  let(:ldap_user_2) { create(:user, extern_uid: 'mike', provider: 'ldap', last_credential_check_at: Time.now) }
 
   before do
     group.add_owner(user)
     group.add_owner(ldap_user)
     group.add_user(ldap_user_2, Gitlab::Access::REPORTER)
+    group.ldap_group_links.create cn: 'developers', group_access: Gitlab::Access::DEVELOPER
   end
 
   describe '#execute' do
@@ -17,16 +19,20 @@ describe LdapGroupResetService do
       before { LdapGroupResetService.new.execute(group, ldap_user) }
 
       it { member_access(ldap_user).should == Gitlab::Access::OWNER }
-      it { member_access(ldap_user_2).should == Gitlab::Access::DEVELOPER }
+      it { member_access(ldap_user_2).should == Gitlab::Access::GUEST }
       it { member_access(user).should == Gitlab::Access::OWNER }
+      it { expect(ldap_user.reload.last_credential_check_at).to be_nil }
+      it { expect(ldap_user_2.reload.last_credential_check_at).to be_nil }
     end
 
     context 'initiated by regular user' do
       before { LdapGroupResetService.new.execute(group, user) }
 
-      it { member_access(ldap_user).should == Gitlab::Access::DEVELOPER }
-      it { member_access(ldap_user_2).should == Gitlab::Access::DEVELOPER }
+      it { member_access(ldap_user).should == Gitlab::Access::GUEST }
+      it { member_access(ldap_user_2).should == Gitlab::Access::GUEST }
       it { member_access(user).should == Gitlab::Access::OWNER }
+      it { expect(ldap_user.reload.last_credential_check_at).to be_nil }
+      it { expect(ldap_user_2.reload.last_credential_check_at).to be_nil }
     end
   end