Merge branch 'group-sync-update-perms' into 'master'

Override LDAP members permissions

This gives owner/master users the ability to override LDAP group synced members permissions. 

LDAP users are given a label to indicate they are a LDAP user. Next to the LDAP user is an edit button which allows the owner/master to change the permissions (with a pre-warning). The owner/master user can also revert this with a new option in the dropdown.

![Screen_Shot_2016-12-09_at_15.33.47](/uploads/ea32bfbf10870abfcac32d533c65c274/Screen_Shot_2016-12-09_at_15.33.47.png)

![Screen_Shot_2016-12-09_at_15.33.50](/uploads/b15ffae6cd939f232698203b1d184877/Screen_Shot_2016-12-09_at_15.33.50.png)

![Screen_Shot_2016-12-09_at_15.33.56](/uploads/d432aa40af75233a7b8d8b0e25e6a8ad/Screen_Shot_2016-12-09_at_15.33.56.png)

Closes #343

See merge request !822

app/assets/javascripts/members.js.es6

 /* eslint-disable class-methods-use-this */
+/* eslint-disable no-new */
+/* global Flash */
 (() => {
   window.gl = window.gl || {};
 
@@ -9,6 +11,8 @@
     }
 
     addListeners() {
+      $('.js-ldap-permissions').off('click').on('click', this.showLDAPPermissionsWarning.bind(this));
+      $('.js-ldap-override').off('click').on('click', this.toggleMemberAccessToggle.bind(this));
       $('.project_member, .group_member').off('ajax:success').on('ajax:success', this.removeRow);
       $('.js-member-update-control').off('change').on('change', this.formSubmit.bind(this));
       $('.js-edit-member-form').off('ajax:success').on('ajax:success', this.formSuccess.bind(this));
@@ -22,6 +26,10 @@
         $btn.glDropdown({
           selectable: true,
           isSelectable(selected, $el) {
+            if ($el.data('revert')) {
+              return false;
+            }
+
             return !$el.hasClass('is-active');
           },
           fieldName: $btn.data('field-name'),
@@ -29,10 +37,26 @@
             return $el.data('id');
           },
           toggleLabel(selected, $el) {
+            if ($el.data('revert')) {
+              return $btn.text();
+            }
+
             return $el.text();
           },
           clicked: (selected, $link) => {
-            this.formSubmit(null, $link);
+            if (!$link.data('revert')) {
+              this.formSubmit(null, $link);
+            } else {
+              const { $memberListItem, $toggle, $dateInput } = this.getMemberListItems($link);
+
+              $toggle.disable();
+              $dateInput.disable();
+
+              this.overrideLdap($memberListItem, $link.data('endpoint'), false).fail(() => {
+                $toggle.enable();
+                $dateInput.enable();
+              });
+            }
           },
         });
       });
@@ -66,6 +90,14 @@
       $dateInput.enable();
     }
 
+    showLDAPPermissionsWarning(e) {
+      const $btn = $(e.currentTarget);
+      const { $memberListItem } = this.getMemberListItems($btn);
+      const $ldapPermissionsElement = $memberListItem.next();
+
+      $ldapPermissionsElement.toggle();
+    }
+
     getMemberListItems($el) {
       const $memberListItem = $el.is('.member') ? $el : $(`#${$el.data('el-id')}`);
 
@@ -75,6 +107,42 @@
         $dateInput: $memberListItem.find('.js-access-expiration-date'),
       };
     }
+
+    toggleMemberAccessToggle(e) {
+      const $btn = $(e.currentTarget);
+      const { $memberListItem, $toggle, $dateInput } = this.getMemberListItems($btn);
+
+      $btn.disable();
+
+      this.overrideLdap($memberListItem, $btn.data('endpoint'), true).then(() => {
+        this.showLDAPPermissionsWarning(e);
+
+        $toggle.enable();
+        $dateInput.enable();
+      }).fail((xhr) => {
+        $btn.enable();
+
+        if (xhr.status === 403) {
+          new Flash('You do not have the correct permissions to override the settings from the LDAP group sync.', 'alert');
+        } else {
+          new Flash('An error occured whilst saving LDAP override status. Please try again.', 'alert');
+        }
+      });
+    }
+
+    overrideLdap($memberListitem, endpoint, override) {
+      return $.ajax({
+        url: endpoint,
+        type: 'PATCH',
+        data: {
+          group_member: {
+            override,
+          },
+        },
+      }).then(() => {
+        $memberListitem.toggleClass('is-overriden', override);
+      });
+    }
   }
 
   gl.Members = Members;

app/assets/stylesheets/framework/lists.scss

@@ -161,6 +161,12 @@ ul.content-list {
         margin-top: 3px;
         margin-bottom: 4px;
 
+        &.btn-ldap-override {
+          @media (min-width: $screen-sm-min) {
+            margin-bottom: 0;
+          }
+        }
+
         &:last-child {
           margin-right: 0;
         }

app/assets/stylesheets/framework/variables.scss

@@ -197,6 +197,7 @@ $divergence-graph-bar-bg: #ccc;
 $divergence-graph-separator-bg: #ccc;
 $issue-box-upcoming-bg: #8f8f8f;
 $pages-group-name-color: #4c4e54;
+$ldap-members-override-bg: #fff1e0;
 
 /*
 * Common component specific colors

app/assets/stylesheets/pages/members.scss

@@ -4,6 +4,12 @@
 }
 
 .member {
+  &.is-overriden {
+    .btn-ldap-override {
+      display: none!important;
+    }
+  }
+
   .list-item-name {
     @media (min-width: $screen-sm-min) {
       float: left;
@@ -100,3 +106,47 @@
   border: 0;
   outline: 0;
 }
+
+.members-ldap {
+  -webkit-align-self: center;
+  align-self: center;
+  height: 100%;
+  margin-right: 10px;
+  margin-left: -49px;
+}
+
+.alert-member-ldap {
+  background-color: $ldap-members-override-bg;
+
+  @media (min-width: $screen-sm-min) {
+    line-height: 40px;
+  }
+
+  > p {
+    float: left;
+    margin-bottom: 10px;
+    color: $orange-normal;
+
+    @media (min-width: $screen-sm-min) {
+      padding-left: 55px;
+      margin-bottom: 0;
+    }
+  }
+
+  .controls {
+    width: 100%;
+
+    @media (min-width: $screen-sm-min) {
+      width: auto;
+    }
+  }
+}
+
+.btn-ldap-override {
+  width: 100%;
+
+  @media (min-width: $screen-sm-min) {
+    margin-left: 10px;
+    width: auto;
+  }
+}

app/controllers/ee/groups/group_members_controller.rb

+module EE
+  module Groups
+    module GroupMembersController
+      def override
+        @group_member = @group.group_members.find(params[:id])
+
+        return render_403 unless can?(current_user, :override_group_member, @group_member)
+
+        if @group_member.update_attributes(override_params)
+          log_audit_event(@group_member, action: :override)
+
+          respond_to do |format|
+            format.js { head :ok }
+          end
+        end
+      end
+
+      protected
+
+      def authorize_update_group_member!
+        unless can?(current_user, :admin_group_member, group) || can?(current_user, :override_group_member, group)
+          render_403
+        end
+      end
+
+      def override_params
+        params.require(:group_member).permit(:override)
+      end
+    end
+  end
+end

app/controllers/groups/group_members_controller.rb

 class Groups::GroupMembersController < Groups::ApplicationController
+  prepend EE::Groups::GroupMembersController
+
   include MembershipActions
 
   # Authorize
-  before_action :authorize_admin_group_member!, except: [:index, :leave, :request_access]
+  before_action :authorize_admin_group_member!, except: [:index, :leave, :request_access, :update, :override]
+  before_action :authorize_update_group_member!, only: [:update, :override]
 
   def index
     @project = @group.projects.find(params[:project_id]) if params[:project_id]

app/policies/group_member_policy.rb

@@ -15,5 +15,16 @@ class GroupMemberPolicy < BasePolicy
     elsif @user == target_user
       can! :destroy_group_member
     end
+
+    additional_rules!
+  end
+
+  def additional_rules!
+    can_override = Ability.allowed?(@user, :override_group_member, @subject.group)
+
+    if can_override
+      can! :override_group_member if @subject.ldap?
+      can! :update_group_member unless @subject.ldap? && !@subject.override?
+    end
   end
 end

app/policies/group_policy.rb

@@ -34,8 +34,7 @@ class GroupPolicy < BasePolicy
       can! :request_access
     end
 
-    # EE-only
-    cannot! :admin_group_member if @subject.ldap_synced?
+    additional_rules!(master)
   end
 
   def can_read_group?
@@ -46,4 +45,11 @@ class GroupPolicy < BasePolicy
 
     GroupProjectsFinder.new(@subject).execute(@user).any?
   end
+
+  def additional_rules!(master)
+    if @subject.ldap_synced?
+      cannot! :admin_group_member
+      can! :override_group_member if master
+    end
+  end
 end

app/views/groups/group_members/_ldap_sync.html.haml

 - if current_user && @group.ldap_synced?
   .bs-callout.bs-callout-info
-    The members of this group are managed using LDAP and cannot be added, changed or removed here.
+    The members of this group are managed using LDAP and cannot be added or removed here.
     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|

app/views/shared/members/_member.html.haml

@@ -4,7 +4,10 @@
 - source = member.source
 - can_admin_member = can?(current_user, action_member_permission(:update, member), member)
 
-%li.member{ class: dom_class(member), id: dom_id(member) }
+-# EE-only
+- can_override_member = can?(current_user, action_member_permission(:override, member), member)
+
+%li.member{ class: [dom_class(member), ("is-overriden" if member.override)], id: dom_id(member) }
   %span.list-item-name
     - if user
       = image_tag avatar_icon(user, 40), class: "avatar s40", alt: ''
@@ -45,6 +48,7 @@
         = time_ago_with_tooltip(member.created_at)
   - if show_roles
     .controls.member-controls
+      = render 'shared/members/ee/ldap_tag', can_override: can_override_member, visible: false
       - if show_controls && (member.respond_to?(:group) && @group) || (member.respond_to?(:project) && @project)
         - if user != current_user
           = form_for member, remote: true, html: { class: 'form-horizontal js-edit-member-form' } do |f|
@@ -65,6 +69,7 @@
                         = link_to role, "javascript:void(0)",
                           class: ("is-active" if member.access_level == role_id),
                           data: { id: role_id, el_id: dom_id(member) }
+                    = render 'shared/members/ee/revert_ldap_group_sync_option', group: @group, member: member, can_override: can_override_member
             .prepend-left-5.clearable-input.member-form-control
               = f.text_field :expires_at, class: 'form-control js-access-expiration-date js-member-update-control', placeholder: 'Expiration date', id: "member_expires_at_#{member.id}", disabled: !can_admin_member, data: { el_id: dom_id(member) }
               %i.clear-icon.js-clear-input
@@ -98,5 +103,8 @@
               %span.visible-xs-block
                 Delete
               = icon('trash', class: 'hidden-xs')
+        = render 'shared/members/ee/override_member_buttons', group: @group, member: member, user: user, action: :edit, can_override: can_override_member
       - else
         %span.member-access-text= member.human_access
+
+= render 'shared/members/ee/override_member_buttons', group: @group, member: member, user: user, action: :confirm, can_override: can_override_member

app/views/shared/members/ee/_ldap_tag.html.haml

+- can_override = local_assigns.fetch(:can_override, false)
+- visible = local_assigns.fetch(:visible, false)
+
+- if can_override
+  - if visible
+    %span.label.label-info.pull-right.visible-xs-block
+      LDAP
+  - else
+    %span.label.label-info.members-ldap.hidden-xs
+      LDAP

app/views/shared/members/ee/_override_member_buttons.html.haml

+- group  = local_assigns.fetch(:group)
+- member = local_assigns.fetch(:member)
+- user   = local_assigns.fetch(:user)
+- action = local_assigns.fetch(:action, :edit).to_s.inquiry
+- can_override = local_assigns.fetch(:can_override, false)
+
+- if can_override
+  - if action.edit?
+    %button.btn.btn-default.btn-ldap-override.js-ldap-permissions{ type: 'button',
+      'aria-label' => 'Edit permissions',
+      data: { name: user.name, el_id: dom_id(member) } }
+      %span.visible-xs-block.visible-sm-block
+        Edit permissions
+      = icon('pencil', class: 'hidden-xs hidden-sm')
+  - else
+    %li.alert.alert-member-ldap{ style: 'display: none;' }
+      %p
+        = user.name
+        is currently an LDAP user. Editing their permissions will override the settings from the LDAP group sync.
+      .controls
+        %button.btn.btn-warning.btn-loading.js-ldap-override{ type: 'button',
+          'aria-label' => 'Change LDAP member permissions',
+          data: { el_id: dom_id(member), endpoint: override_group_group_member_path(group, member) } }
+          = icon('spinner spin')
+          Change permissions
+        %button.btn.btn-default.js-ldap-permissions{ type: 'button',
+          'aria-label' => 'Close permissions override',
+          data: { el_id: dom_id(member) } }
+          = icon('times')

app/views/shared/members/ee/_revert_ldap_group_sync_option.html.haml

+- group  = local_assigns.fetch(:group)
+- member = local_assigns.fetch(:member)
+- can_override = local_assigns.fetch(:can_override, false)
+
+- if can_override
+  %li.divider
+  %li
+    = link_to 'Revert to LDAP group sync settings', 'javascript:void(0)',
+      data: { revert: 'true', endpoint: override_group_group_member_path(group, member), el_id: dom_id(member) }

changelogs/unreleased-ee/group-sync-update-perms.yml

+---
+title: Allow master/owner to change permission levels when LDAP group sync is enabled
+merge_request: 822
+author:

config/routes/group.rb

@@ -18,6 +18,10 @@ scope(path: 'groups/*group_id',
   resources :group_members, only: [:index, :create, :update, :destroy], concerns: :access_requestable do
     post :resend_invite, on: :member
     delete :leave, on: :collection
+
+    ## EE-specific
+    patch :override, on: :member
+    ## EE-specific
   end
 
   resource :avatar, only: [:destroy]

spec/features/groups/members/override_ldap_memberships_spec.rb

+require 'spec_helper'
+
+feature 'Groups > Members > Master/Owner can override LDAP access levels', feature: true do
+  include WaitForAjax
+
+  let(:johndoe)  { create(:user, name: 'John Doe') }
+  let(:maryjane) { create(:user, name: 'Mary Jane') }
+  let(:owner)    { create(:user) }
+  let(:group)    { create(:group_with_ldap_group_link, :public) }
+
+  let!(:owner_member)   { create(:group_member, :owner, group: group, user: owner) }
+  let!(:ldap_member)    { create(:group_member, :guest, group: group, user: johndoe, ldap: true) }
+  let!(:regular_member) { create(:group_member, :guest, group: group, user: maryjane, ldap: false) }
+
+  background do
+    # We need to actually activate the LDAP config otherwise `Group#ldap_synced?` will always be false!
+    allow(Gitlab.config.ldap).to receive_messages(enabled: true)
+
+    login_as(owner)
+  end
+
+  scenario 'owner can override LDAP access level', js: true do
+    ldap_override_message = 'John Doe is currently an LDAP user. Editing their permissions will override the settings from the LDAP group sync.'
+
+    visit group_group_members_path(group)
+
+    within "#group_member_#{ldap_member.id}" do
+      expect(page).to have_content 'LDAP'
+      expect(page).to have_button 'Guest', disabled: true
+      expect(page).to have_button 'Edit permissions'
+
+      click_button 'Edit permissions'
+    end
+
+    expect(page).to have_content ldap_override_message
+
+    click_button 'Change permissions'
+
+    expect(page).not_to have_content ldap_override_message
+    expect(page).not_to have_button 'Change permissions'
+
+    within "#group_member_#{ldap_member.id}" do
+      expect(page).not_to have_button 'Edit permissions'
+      expect(page).to have_button 'Guest', disabled: false
+
+      click_button 'Guest'
+
+      within '.dropdown-menu' do
+        click_link 'Revert to LDAP group sync settings'
+      end
+
+      wait_for_ajax
+
+      expect(page).to have_button 'Guest', disabled: true
+      expect(page).to have_button 'Edit permissions'
+    end
+
+    within "#group_member_#{regular_member.id}" do
+      expect(page).not_to have_content 'LDAP'
+      expect(page).not_to have_button 'Edit permissions'
+      expect(page).to have_button 'Guest', disabled: false
+
+      click_button 'Guest'
+
+      within '.dropdown-menu' do
+        expect(page).not_to have_content 'Revert to LDAP group sync settings'
+      end
+    end
+  end
+end