Add self-managed Microsoft Group Sync worker

Adds a self-managed Microsoft Azure Group Sync Worker and
combines reusable methods in a base class. This does not yet
make sync available to self-managed.

config/sidekiq_queues.yml

@@ -611,6 +611,8 @@
   - 1
 - - system_access_group_saml_microsoft_group_sync
   - 1
+- - system_access_saml_microsoft_group_sync
+  - 1
 - - system_hook_push
   - 1
 - - tasks_to_be_done_create

ee/app/workers/all_queues.yml

@@ -1956,6 +1956,15 @@
   :weight: 1
   :idempotent: true
   :tags: []
+- :name: system_access_saml_microsoft_group_sync
+  :worker_name: SystemAccess::SamlMicrosoftGroupSyncWorker
+  :feature_category: :system_access
+  :has_external_dependencies: false
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: vulnerabilities_mark_dropped_as_resolved
   :worker_name: Vulnerabilities::MarkDroppedAsResolvedWorker
   :feature_category: :static_application_security_testing

ee/app/workers/auth/saml_group_sync_worker.rb

@@ -5,7 +5,7 @@
 # When a user signs in with SAML this worker will
 # be triggered to manage that user's group membership.
 module Auth
-  class SamlGroupSyncWorker
+  class SamlGroupSyncWorker < ::SystemAccess::BaseGlobalGroupSyncWorker
     include ApplicationWorker
     include Gitlab::Utils::StrongMemoize
 
@@ -34,73 +34,10 @@ def sync_enabled?
       Gitlab::Auth::Saml::Config.new(@provider).group_sync_enabled?
     end
 
-    def groups_to_sync?
-      group_links.any? || group_ids_by_root_ancestor_id.any?
-    end
-
-    def sync_groups
-      group_ids_to_manage = group_ids_by_root_ancestor_id.dup
-
-      # Sync groups user for which user should be a member
-      group_links_by_root_ancestor.each do |root_ancestor, group_links|
-        Groups::SyncService.new(
-          root_ancestor, user,
-          group_links: group_links, manage_group_ids: group_ids_to_manage.delete(root_ancestor.id)
-        ).execute
-      end
-
-      return if group_ids_to_manage.empty?
-
-      root_ancestors = preload_groups(group_ids_to_manage.keys)
-
-      # Sync groups with links for which user should not be a member
-      group_ids_to_manage.each do |root_ancestor_id, group_ids|
-        Groups::SyncService.new(
-          root_ancestors[root_ancestor_id], user, group_links: [], manage_group_ids: group_ids
-        ).execute
-      end
-    end
-
     def group_links
       strong_memoize(:group_links) do
         SamlGroupLink.id_in(group_link_ids).preload_group
       end
     end
-
-    def group_ids_by_root_ancestor_id
-      strong_memoize(:group_ids_by_root_ancestor_id) do
-        grouped = {}
-        groups = Group.with_saml_group_links.select(:id, 'traversal_ids[1] as root_id')
-
-        groups.each do |group|
-          grouped[group.root_id] ||= []
-
-          grouped[group.root_id].push(group.id)
-        end
-
-        grouped
-      end
-    end
-
-    def group_links_by_root_ancestor
-      strong_memoize(:group_links_by_root_ancestor) do
-        grouped = {}
-        groups = group_links.map(&:group)
-        Preloaders::GroupRootAncestorPreloader.new(groups, [:route]).execute
-
-        group_links.each do |link|
-          root_ancestor = link.group.root_ancestor
-          grouped[root_ancestor] ||= []
-
-          grouped[root_ancestor].push(link)
-        end
-
-        grouped
-      end
-    end
-
-    def preload_groups(group_ids)
-      Group.by_id(group_ids).index_by(&:id)
-    end
   end
 end

ee/app/workers/system_access/base_global_group_sync_worker.rb

+# frozen_string_literal: true
+
+module SystemAccess
+  class BaseGlobalGroupSyncWorker # rubocop:disable Scalability/IdempotentWorker
+    include ::Gitlab::Utils::StrongMemoize
+
+    private
+
+    def groups_to_sync?
+      group_links.any? || group_ids_by_root_ancestor_id.any?
+    end
+
+    def sync_groups
+      group_ids_to_manage = group_ids_by_root_ancestor_id.dup
+
+      # Sync groups user for which user should be a member
+      group_links_by_root_ancestor.each do |root_ancestor, group_links|
+        Groups::SyncService.new(
+          root_ancestor, user,
+          group_links: group_links, manage_group_ids: group_ids_to_manage.delete(root_ancestor.id)
+        ).execute
+      end
+
+      return if group_ids_to_manage.empty?
+
+      root_ancestors = preload_groups(group_ids_to_manage.keys)
+
+      # Sync groups with links for which user should not be a member
+      group_ids_to_manage.each do |root_ancestor_id, group_ids|
+        Groups::SyncService.new(
+          root_ancestors[root_ancestor_id], user, group_links: [], manage_group_ids: group_ids
+        ).execute
+      end
+    end
+
+    def group_ids_by_root_ancestor_id
+      grouped = {}
+      groups = Group.with_saml_group_links.select(:id, 'traversal_ids[1] as root_id')
+
+      groups.each do |group|
+        grouped[group.root_id] ||= []
+
+        grouped[group.root_id].push(group.id)
+      end
+
+      grouped
+    end
+    strong_memoize_attr :group_ids_by_root_ancestor_id
+
+    def group_links_by_root_ancestor
+      grouped = {}
+      groups = group_links.map(&:group)
+      Preloaders::GroupRootAncestorPreloader.new(groups, [:route]).execute
+
+      group_links.each do |link|
+        root_ancestor = link.group.root_ancestor
+        grouped[root_ancestor] ||= []
+
+        grouped[root_ancestor].push(link)
+      end
+
+      grouped
+    end
+    strong_memoize_attr :group_links_by_root_ancestor
+
+    def preload_groups(group_ids)
+      Group.by_id(group_ids).index_by(&:id)
+    end
+  end
+end

ee/app/workers/system_access/saml_microsoft_group_sync_worker.rb

+# frozen_string_literal: true
+
+module SystemAccess
+  class SamlMicrosoftGroupSyncWorker < BaseGlobalGroupSyncWorker
+    include ::ApplicationWorker
+    include ::Gitlab::Utils::StrongMemoize
+
+    feature_category :system_access
+    idempotent!
+    urgency :low
+    data_consistency :always
+
+    def perform(user_id, provider = 'saml')
+      self.user = User.find_by_id(user_id)
+      self.provider = provider
+
+      return unless user && group_sync_enabled?
+      return unless microsoft_groups.any? && group_links.any?
+
+      sync_groups
+    end
+
+    private
+
+    attr_accessor :provider, :user
+
+    def group_sync_enabled?
+      ::Gitlab::Auth::Saml::Config.new(@provider).microsoft_group_sync_enabled? &&
+        application&.enabled? &&
+        microsoft_user_object_id.present?
+    end
+
+    def application
+      SystemAccess::MicrosoftApplication.instance_application
+    end
+    strong_memoize_attr :application
+
+    def client
+      ::Microsoft::GraphClient.new(application)
+    end
+    strong_memoize_attr :client
+
+    def microsoft_groups
+      client.user_group_membership_object_ids(microsoft_user_object_id)
+    end
+    strong_memoize_attr :microsoft_groups
+
+    def microsoft_user_object_id
+      identity = user.identities.with_provider(provider)&.first
+      identity&.extern_uid
+    end
+    strong_memoize_attr :microsoft_user_object_id
+
+    def group_links
+      SamlGroupLink
+        .by_saml_group_name(microsoft_groups)
+        .preload_group
+    end
+  end
+end

ee/spec/workers/system_access/saml_microsoft_group_sync_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe SystemAccess::SamlMicrosoftGroupSyncWorker, :aggregate_failures, feature_category: :system_access do
+  describe '#perform' do
+    let(:worker) { described_class.new }
+
+    let_it_be(:user_no_identity) { create(:user) }
+    let_it_be(:user_with_identity) { create(:user) }
+    let_it_be(:identity) { create(:identity, user: user_with_identity, provider: 'saml') }
+
+    let_it_be_with_refind(:application) do
+      create(:system_access_microsoft_application, namespace: nil)
+    end
+
+    let_it_be(:group1) { create(:group) }
+    let_it_be(:subgroup1) { create(:group, parent: group1) }
+    let_it_be(:group2) { create(:group) }
+    let_it_be(:group3) { create(:group) }
+
+    let_it_be(:org_users) { 'Org Users' }
+    let_it_be(:dept_users) { 'Dept Users' }
+    let_it_be(:all_groups) { [org_users, dept_users] }
+    let_it_be(:no_groups) { [] }
+
+    let_it_be(:group_link1) { create(:saml_group_link, group: group1, saml_group_name: org_users) }
+    let_it_be(:subgroup1_link) { create(:saml_group_link, group: subgroup1, saml_group_name: dept_users) }
+
+    let_it_be(:group_link2) { create(:saml_group_link, group: group2, saml_group_name: org_users) }
+
+    using RSpec::Parameterized::TableSyntax
+
+    # Test every combination of preconditions to ensure sync does not execute unless all are met.
+    where(:sync_enabled, :app_enabled, :user, :microsoft_groups, :expect_sync_service_called_times) do
+      false | false | ref(:user_no_identity) | ref(:no_groups)    | 0
+      false | true  | ref(:user_no_identity) | ref(:no_groups)    | 0
+      true  | false | ref(:user_no_identity) | ref(:no_groups)    | 0
+      true  | true  | ref(:user_no_identity) | ref(:no_groups)    | 0
+
+      false | false | ref(:user_no_identity) | ref(:all_groups)   | 0
+      true  | false | ref(:user_no_identity) | ref(:all_groups)   | 0
+      false | true  | ref(:user_no_identity) | ref(:all_groups)   | 0
+      true  | true  | ref(:user_no_identity) | ref(:all_groups)   | 0
+
+      false | false | ref(:user_with_identity) | ref(:no_groups)  | 0
+      false | true  | ref(:user_with_identity) | ref(:no_groups)  | 0
+      true  | false | ref(:user_with_identity) | ref(:no_groups)  | 0
+      true  | true  | ref(:user_with_identity) | ref(:no_groups)  | 0
+
+      false | false | ref(:user_with_identity) | ref(:all_groups) | 0
+      true  | false | ref(:user_with_identity) | ref(:all_groups) | 0
+      false | true  | ref(:user_with_identity) | ref(:all_groups) | 0
+
+      # This is the only scenario that satisfies all preconditions.
+      # Sync is called once per top-level group. Since we have 3 group links in 2
+      # top-level group hierarchies, we expect sync service to be called twice.
+      true  | true  | ref(:user_with_identity) | ref(:all_groups) | 2
+    end
+
+    with_them do
+      before do
+        stub_sync_enabled(sync_enabled)
+        application.update!(enabled: app_enabled)
+        stub_microsoft_groups(microsoft_groups)
+      end
+
+      it 'calls the sync service the appropriate number of times' do
+        expect(Groups::SyncService).to receive(:new).exactly(expect_sync_service_called_times).times.and_call_original
+
+        worker.perform(user.id)
+      end
+    end
+
+    # More in-depth verification of how sync service is called and the outcomes of sync.
+    context 'when all preconditions are met and sync executes' do
+      before do
+        stub_sync_enabled(true)
+        application.update!(enabled: true)
+      end
+
+      context 'when group links exist in hierarchies which the user should not be a member of' do
+        before do
+          stub_microsoft_groups([dept_users]) # dept_users group link is in group1 / subgroup1 hierarchy
+        end
+
+        # User should be a member of group1 and subgroup1, but not group2. Still, sync should be
+        # executed for group2 to ensure user is removed if they were previously a member.
+        # Sync service is not called for group3 because no group links exist.
+        it 'calls the service for all top-level groups with any groups links in the hierarchy' do
+          expect(Groups::SyncService).to receive(:new).with(
+            group1, user_with_identity, group_links: [subgroup1_link], manage_group_ids: [group1.id, subgroup1.id]
+          ).and_call_original
+
+          expect(Groups::SyncService).to receive(:new).with(
+            group2, user_with_identity, group_links: [], manage_group_ids: [group2.id]
+          ).and_call_original
+
+          expect(Groups::SyncService).not_to receive(:new).with(group3, any_args)
+
+          worker.perform(user_with_identity.id)
+        end
+      end
+
+      context 'with a group in the hierarchy that has no group links' do
+        let(:subgroup_without_links) { create(:group, parent: group2) }
+
+        before do
+          stub_microsoft_groups([dept_users])
+        end
+
+        it 'is not included in manage_group_ids' do
+          expect(Groups::SyncService).to receive(:new).with(
+            group1, user_with_identity, group_links: [subgroup1_link], manage_group_ids: [group1.id, subgroup1.id]
+          ).and_call_original
+
+          expect(Groups::SyncService).to receive(:new).with(
+            group2, user_with_identity, group_links: [], manage_group_ids: [group2.id]
+          ).and_call_original
+
+          worker.perform(user_with_identity.id)
+        end
+      end
+    end
+
+    def stub_sync_enabled(enabled)
+      allow_next_instance_of(::Gitlab::Auth::Saml::Config) do |instance|
+        allow(instance).to receive(:microsoft_group_sync_enabled?).and_return(enabled)
+      end
+    end
+
+    def stub_microsoft_groups(groups)
+      allow_next_instance_of(::Microsoft::GraphClient) do |instance|
+        allow(instance).to receive_messages(user_group_membership_object_ids: groups)
+      end
+    end
+  end
+end