From 8245c667b552b90e1f64cdadffea9b2649e769ab Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Thu, 1 Sep 2016 16:12:29 -0500 Subject: [PATCH] Add Sync now to group members page --- CHANGELOG-EE | 1 + app/controllers/groups/ldaps_controller.rb | 7 +++-- app/models/ee/group.rb | 7 ++++- .../groups/group_members/_ldap_sync.html.haml | 14 ++++++++++ .../group_members/_sync_button.html.haml | 15 ++++++++++ .../groups/group_members/index.html.haml | 20 +------------ app/workers/ldap_group_sync_worker.rb | 20 ++++++++++--- config/routes.rb | 2 +- spec/lib/ee/gitlab/ldap/sync/group_spec.rb | 18 ++++++++++++ spec/workers/ldap_group_sync_worker_spec.rb | 28 +++++++++++++++++++ 10 files changed, 104 insertions(+), 28 deletions(-) create mode 100644 app/views/groups/group_members/_ldap_sync.html.haml create mode 100644 app/views/groups/group_members/_sync_button.html.haml create mode 100644 spec/workers/ldap_group_sync_worker_spec.rb diff --git a/CHANGELOG-EE b/CHANGELOG-EE index e494253fa54c41..8daf475013fce0 100644 --- a/CHANGELOG-EE +++ b/CHANGELOG-EE @@ -1,5 +1,6 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.12.0 (Unreleased) + - Add 'Sync now' to group members page !704 v 8.11.5 - API: Restore backward-compatibility for POST /projects/:id/members when membership is locked diff --git a/app/controllers/groups/ldaps_controller.rb b/app/controllers/groups/ldaps_controller.rb index 4354ce3a36987c..b0bd2048d3568f 100644 --- a/app/controllers/groups/ldaps_controller.rb +++ b/app/controllers/groups/ldaps_controller.rb @@ -2,9 +2,10 @@ class Groups::LdapsController < Groups::ApplicationController before_action :group before_action :authorize_admin_group! - def reset_access - LdapGroupResetService.new.execute(group, current_user) + def sync + @group.pending_ldap_sync + LdapGroupSyncWorker.perform_async(@group.id) - redirect_to group_group_members_path(@group), notice: 'Access reset complete' + redirect_to group_group_members_path(@group), notice: 'The group sync has been scheduled' end end diff --git a/app/models/ee/group.rb b/app/models/ee/group.rb index 1ff674e9221aba..f29ed1c692bb3d 100644 --- a/app/models/ee/group.rb +++ b/app/models/ee/group.rb @@ -10,10 +10,15 @@ module Group state_machine :ldap_sync_status, namespace: :ldap_sync, initial: :ready do state :ready state :started + state :pending state :failed + event :pending do + transition [:ready, :failed] => :pending + end + event :start do - transition [:ready, :failed] => :started + transition [:ready, :pending, :failed] => :started end event :finish do diff --git a/app/views/groups/group_members/_ldap_sync.html.haml b/app/views/groups/group_members/_ldap_sync.html.haml new file mode 100644 index 00000000000000..48c7318329b599 --- /dev/null +++ b/app/views/groups/group_members/_ldap_sync.html.haml @@ -0,0 +1,14 @@ +- 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. + 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. + - if can?(current_user, :admin_group, @group) + = render 'sync_button' diff --git a/app/views/groups/group_members/_sync_button.html.haml b/app/views/groups/group_members/_sync_button.html.haml new file mode 100644 index 00000000000000..ed6e1c6c616723 --- /dev/null +++ b/app/views/groups/group_members/_sync_button.html.haml @@ -0,0 +1,15 @@ +- if @group.ldap_sync_started? + %span.btn.disabled + = icon("refresh spin") + Syncing… +- elsif @group.ldap_sync_pending? + %span.btn.disabled + = icon("refresh spin") + Pending sync… +- else + = link_to sync_group_ldap_path(@group), method: :put, class: 'btn' do + = icon("refresh") + Sync now +- if @group.ldap_sync_ready? && @group.ldap_sync_last_successful_update_at + %p.inline.prepend-left-10 + Successfully synced #{time_ago_with_tooltip(@group.ldap_sync_last_successful_update_at)}. diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml index 18daf119d9c01b..7c5ea5e148e179 100644 --- a/app/views/groups/group_members/index.html.haml +++ b/app/views/groups/group_members/index.html.haml @@ -13,23 +13,7 @@ = render 'shared/members/requests', membership_source: @group, requesters: @requesters - - 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. - 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. - - if can?(current_user, :admin_group_member, @group) - = form_tag(reset_access_group_ldap_path(@group), method: :put, class: 'inline') do - = button_to 'Clear LDAP permission cache', '#', class: "btn btn-remove js-confirm-danger", - data: { "confirm-danger-message" => clear_ldap_permission_cache_message, - 'warning-message' => 'If you made manual permission tweaks for some group members they will be lost.' } + = render 'ldap_sync' .panel.panel-default .panel-heading @@ -51,5 +35,3 @@ event.preventDefault(); Turbolinks.visit(this.action + '?' + $(this).serialize()); }); - -= render 'shared/confirm_modal', phrase: 'reset' diff --git a/app/workers/ldap_group_sync_worker.rb b/app/workers/ldap_group_sync_worker.rb index 433ca54abf91dc..10777bc9093532 100644 --- a/app/workers/ldap_group_sync_worker.rb +++ b/app/workers/ldap_group_sync_worker.rb @@ -3,9 +3,21 @@ class LdapGroupSyncWorker sidekiq_options retry: false - def perform - logger.info 'Started LDAP group sync' - EE::Gitlab::LDAP::Sync::Groups.execute - logger.info 'Finished LDAP group sync' + def perform(group_id = nil) + if group_id + group = Group.find_by(id: group_id) + unless group + logger.warn "Could not find group #{group_id} for LDAP group sync" + return + end + + logger.info "Started LDAP group sync for group #{group.name} (#{group.id})" + EE::Gitlab::LDAP::Sync::Group.execute_all_providers(group) + logger.info "Finished LDAP group sync for group #{group.name} (#{group.id})" + else + logger.info 'Started LDAP group sync' + EE::Gitlab::LDAP::Sync::Groups.execute + logger.info 'Finished LDAP group sync' + end end end diff --git a/config/routes.rb b/config/routes.rb index b23800d8d39817..070c27b70408ae 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -463,7 +463,7 @@ resource :analytics, only: [:show] resource :ldap, only: [] do member do - put :reset_access + put :sync end end diff --git a/spec/lib/ee/gitlab/ldap/sync/group_spec.rb b/spec/lib/ee/gitlab/ldap/sync/group_spec.rb index a7393555ec020d..f615c8f1260f73 100644 --- a/spec/lib/ee/gitlab/ldap/sync/group_spec.rb +++ b/spec/lib/ee/gitlab/ldap/sync/group_spec.rb @@ -92,6 +92,24 @@ def execute include_examples :group_state_machine end + describe '.ldap_sync_ready?' do + let(:ldap_group1) { nil } + + it 'returns false when ldap sync started' do + group = create(:group) + group.start_ldap_sync + + expect(described_class.ldap_sync_ready?(group)).to be_falsey + end + + it 'returns true when ldap sync pending' do + group = create(:group) + group.pending_ldap_sync + + expect(described_class.ldap_sync_ready?(group)).to be_truthy + end + end + describe '#update_permissions' do before { group.start_ldap_sync } after { group.finish_ldap_sync } diff --git a/spec/workers/ldap_group_sync_worker_spec.rb b/spec/workers/ldap_group_sync_worker_spec.rb new file mode 100644 index 00000000000000..4ec5f0e6afc1d2 --- /dev/null +++ b/spec/workers/ldap_group_sync_worker_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe LdapGroupSyncWorker do + describe '#perform' do + it 'syncs all groups when group_id is nil' do + expect(EE::Gitlab::LDAP::Sync::Groups).to receive(:execute) + + described_class.new.perform + end + + it 'syncs a single group when group_id is present' do + group = create(:group) + + expect(EE::Gitlab::LDAP::Sync::Group) + .to receive(:execute_all_providers).with(group) + + described_class.new.perform(group.id) + end + + it 'logs an error when group cannot be found' do + expect(EE::Gitlab::LDAP::Sync::Group).not_to receive(:execute_all_providers) + expect(Sidekiq.logger) + .to receive(:warn).with('Could not find group 9999 for LDAP group sync') + + described_class.new.perform(9999) + end + end +end -- GitLab