diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb
index 6f7a65d1117e1ce89722b3d1807fba54f5d51b0f..d68057fb5813b68b8b068b686ef97b86873f4af2 100644
--- a/ee/app/models/ee/namespace.rb
+++ b/ee/app/models/ee/namespace.rb
@@ -33,7 +33,7 @@ module Namespace
foreign_key: :namespace_id,
inverse_of: :namespace
has_one :upcoming_reconciliation, inverse_of: :namespace, class_name: "GitlabSubscriptions::UpcomingReconciliation"
- has_one :system_access_microsoft_application, class_name: 'SystemAccess::MicrosoftApplication'
+ has_one :system_access_microsoft_application, class_name: '::SystemAccess::MicrosoftApplication'
has_many :automation_rules, class_name: '::Automation::Rule'
has_many :ci_minutes_additional_packs, class_name: "Ci::Minutes::AdditionalPack"
diff --git a/ee/app/models/system_access/microsoft_application.rb b/ee/app/models/system_access/microsoft_application.rb
index c7c2c3ab1303a881aa37688abb5d50b6608785c1..aba88c9efca8d3605c17c70120c4e7fe28fae060 100644
--- a/ee/app/models/system_access/microsoft_application.rb
+++ b/ee/app/models/system_access/microsoft_application.rb
@@ -4,7 +4,7 @@ module SystemAccess
class MicrosoftApplication < ApplicationRecord
belongs_to :namespace
has_one :system_access_microsoft_graph_access_token,
- class_name: 'SystemAccess::MicrosoftGraphAccessToken',
+ class_name: '::SystemAccess::MicrosoftGraphAccessToken',
inverse_of: :system_access_microsoft_application,
foreign_key: :system_access_microsoft_application_id
diff --git a/ee/config/feature_flags/development/microsoft_azure_group_sync.yml b/ee/config/feature_flags/development/microsoft_azure_group_sync.yml
new file mode 100644
index 0000000000000000000000000000000000000000..9247f59cd8a660b88c82832b6b56537e474a2a74
--- /dev/null
+++ b/ee/config/feature_flags/development/microsoft_azure_group_sync.yml
@@ -0,0 +1,8 @@
+---
+name: microsoft_azure_group_sync
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/125923
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/417614
+milestone: '16.2'
+type: development
+group: group::authentication and authorization
+default_enabled: false
diff --git a/ee/lib/gitlab/auth/group_saml/auth_hash.rb b/ee/lib/gitlab/auth/group_saml/auth_hash.rb
index 07952d0b1a1c8f04ec3512aa7216c12babf1ae8e..c57a7558f48051a06742822248eba324a49dddf6 100644
--- a/ee/lib/gitlab/auth/group_saml/auth_hash.rb
+++ b/ee/lib/gitlab/auth/group_saml/auth_hash.rb
@@ -12,6 +12,10 @@ def groups
Array.wrap(get_raw('groups') || get_raw('Groups'))
end
+ def azure_group_overage_claim?
+ get_raw('http://schemas.microsoft.com/claims/groups.link').present?
+ end
+
# Access user attributes by hash.
#
# auth_hash.user_attributes['can_create_group']
diff --git a/ee/lib/gitlab/auth/group_saml/membership_updater.rb b/ee/lib/gitlab/auth/group_saml/membership_updater.rb
index a48438bffbf6a87ed726b73a10c87fb8e7544d14..d06ed4eb17c03651c6796115ad7f4c47b38a3450 100644
--- a/ee/lib/gitlab/auth/group_saml/membership_updater.rb
+++ b/ee/lib/gitlab/auth/group_saml/membership_updater.rb
@@ -18,6 +18,7 @@ def initialize(user, saml_provider, auth_hash)
def execute
add_default_membership
+
enqueue_group_sync if sync_groups?
end
@@ -34,13 +35,29 @@ def add_default_membership
end
def enqueue_group_sync
- GroupSamlGroupSyncWorker.perform_async(user.id, group.id, group_link_ids)
+ return enqueue_microsoft_group_sync if microsoft_overage_sync?
+
+ enqueue_saml_group_sync if saml_group_sync?
end
def sync_groups?
- return false unless user && group.saml_group_sync_available?
+ user && any_group_links_in_hierarchy?
+ end
- any_group_links_in_hierarchy?
+ def saml_group_sync?
+ group.saml_group_sync_available?
+ end
+
+ def microsoft_overage_sync?
+ auth_hash.azure_group_overage_claim? && microsoft_group_sync_available?
+ end
+
+ def enqueue_saml_group_sync
+ GroupSamlGroupSyncWorker.perform_async(user.id, group.id, group_link_ids)
+ end
+
+ def enqueue_microsoft_group_sync
+ ::SystemAccess::GroupSamlMicrosoftGroupSyncWorker.perform_async(user.id, group.id)
end
# rubocop:disable CodeReuse/ActiveRecord
@@ -79,6 +96,12 @@ def log_audit_event(member:)
action: :create
).for_member(member).security_event
end
+
+ def microsoft_group_sync_available?
+ ::Feature.enabled?(:microsoft_azure_group_sync, group) &&
+ group.saml_group_sync_available? && group.licensed_feature_available?(:microsoft_group_sync) &&
+ group.system_access_microsoft_application.present? && group.system_access_microsoft_application.enabled?
+ end
end
end
end
diff --git a/ee/spec/features/groups/saml_providers_spec.rb b/ee/spec/features/groups/saml_providers_spec.rb
index 2e8f725d427914c7003b5316833b71d19cc9aea2..6bfe9f616da5ec21cf5516555521b2db45607975 100644
--- a/ee/spec/features/groups/saml_providers_spec.rb
+++ b/ee/spec/features/groups/saml_providers_spec.rb
@@ -23,7 +23,9 @@
end
def submit
- click_button('Save changes')
+ page.within('.saml_provider') do
+ click_button('Save changes')
+ end
end
def test_sso
diff --git a/ee/spec/lib/gitlab/auth/group_saml/auth_hash_spec.rb b/ee/spec/lib/gitlab/auth/group_saml/auth_hash_spec.rb
index e08838df651c3bbd675406d4e27e2b90d1184869..2a0e1a7725ed89d765dff41103c2ec160a8ebc23 100644
--- a/ee/spec/lib/gitlab/auth/group_saml/auth_hash_spec.rb
+++ b/ee/spec/lib/gitlab/auth/group_saml/auth_hash_spec.rb
@@ -3,7 +3,6 @@
require 'spec_helper'
RSpec.describe Gitlab::Auth::GroupSaml::AuthHash do
- let(:raw_info_attr) { { group_attribute => %w(Developers Owners) } }
let(:omniauth_auth_hash) do
OmniAuth::AuthHash.new(extra: { raw_info: OneLogin::RubySaml::Attributes.new(raw_info_attr) })
end
@@ -11,6 +10,8 @@
subject(:saml_auth_hash) { described_class.new(omniauth_auth_hash) }
describe '#groups' do
+ let(:raw_info_attr) { { group_attribute => %w(Developers Owners) } }
+
context 'with a lowercase groups attribute' do
let(:group_attribute) { 'groups' }
@@ -36,6 +37,32 @@
end
end
+ describe '#azure_group_overage_claim?' do
+ context 'when the claim is not present' do
+ let(:raw_info_attr) { {} }
+
+ it 'is false' do
+ expect(saml_auth_hash.azure_group_overage_claim?).to eq(false)
+ end
+ end
+
+ context 'when the claim is present' do
+ # The value of the claim is irrelevant, but it's still included
+ # in the test response to keep tests as real-world as possible.
+ # https://learn.microsoft.com/en-us/security/zero-trust/develop/configure-tokens-group-claims-app-roles#group-overages
+ let(:raw_info_attr) do
+ {
+ 'http://schemas.microsoft.com/claims/groups.link' =>
+ ['https://graph.windows.net/8c750e43/users/e631c82c/getMemberObjects']
+ }
+ end
+
+ it 'is true' do
+ expect(saml_auth_hash.azure_group_overage_claim?).to eq(true)
+ end
+ end
+ end
+
describe 'allowed user attributes methods' do
context 'when the attributes are presented as an array' do
let(:raw_info_attr) { { 'can_create_group' => %w(true), 'projects_limit' => %w(20) } }
diff --git a/ee/spec/lib/gitlab/auth/group_saml/membership_updater_spec.rb b/ee/spec/lib/gitlab/auth/group_saml/membership_updater_spec.rb
index 1f91bbc0b58fa5847c1580b573943c73d027dda3..6d67d7b5c847ba45ed13969a31ab3bbb0c15bbc3 100644
--- a/ee/spec/lib/gitlab/auth/group_saml/membership_updater_spec.rb
+++ b/ee/spec/lib/gitlab/auth/group_saml/membership_updater_spec.rb
@@ -2,134 +2,273 @@
require 'spec_helper'
-RSpec.describe Gitlab::Auth::GroupSaml::MembershipUpdater do
+RSpec.describe Gitlab::Auth::GroupSaml::MembershipUpdater, feature_category: :system_access do
let(:user) { create(:user) }
let(:saml_provider) { create(:saml_provider, default_membership_role: Gitlab::Access::DEVELOPER) }
let(:group) { saml_provider.group }
- let(:auth_hash) do
- Gitlab::Auth::GroupSaml::AuthHash.new(
- OmniAuth::AuthHash.new(extra: {
- raw_info: OneLogin::RubySaml::Attributes.new('groups' => %w(Developers Owners))
- })
- )
- end
subject(:update_membership) { described_class.new(user, saml_provider, auth_hash).execute }
- it 'adds the user to the group' do
- subject
+ shared_examples 'not enqueueing Microsoft Group Sync worker' do
+ it 'does not enqueue Microsoft Group Sync worker' do
+ expect(::SystemAccess::GroupSamlMicrosoftGroupSyncWorker).not_to receive(:perform_async)
- expect(group.users).to include(user)
+ update_membership
+ end
end
- it 'adds the member with the specified `default_membership_role`' do
- expect(group).to receive(:add_member).with(user, Gitlab::Access::DEVELOPER).and_call_original
-
- update_membership
+ shared_examples 'not enqueueing Group SAML Group Sync worker' do
+ it 'does not enqueue Microsoft Group Sync worker' do
+ expect(GroupSamlGroupSyncWorker).not_to receive(:perform_async)
- created_member = group.members.find_by(user: user)
- expect(created_member.access_level).to eq(Gitlab::Access::DEVELOPER)
+ update_membership
+ end
end
- it "doesn't duplicate group membership" do
- group.add_guest(user)
+ context 'for default behavior' do
+ let_it_be(:auth_hash) { {} }
- subject
+ it 'adds the user to the group' do
+ subject
- expect(group.members.count).to eq 1
- end
+ expect(group.users).to include(user)
+ end
- it "doesn't overwrite existing membership level" do
- group.add_maintainer(user)
+ it 'adds the member with the specified `default_membership_role`' do
+ expect(group).to receive(:add_member).with(user, Gitlab::Access::DEVELOPER).and_call_original
- subject
+ update_membership
- expect(group.members.pluck(:access_level)).to eq([Gitlab::Access::MAINTAINER])
- end
+ created_member = group.members.find_by(user: user)
+ expect(created_member.access_level).to eq(Gitlab::Access::DEVELOPER)
+ end
+
+ it "doesn't duplicate group membership" do
+ group.add_guest(user)
- it "logs an audit event" do
- expect do
subject
- end.to change { AuditEvent.by_entity('Group', group).count }.by(1)
- expect(AuditEvent.last.details).to include(add: 'user_access', target_details: user.name, as: 'Developer')
- end
+ expect(group.members.count).to eq 1
+ end
- it 'does not enqueue group sync' do
- expect(GroupSamlGroupSyncWorker).not_to receive(:perform_async)
+ it "doesn't overwrite existing membership level" do
+ group.add_maintainer(user)
- update_membership
- end
+ subject
- context 'when SAML group links exist' do
- def stub_saml_group_sync_available(enabled)
- allow(group).to receive(:saml_group_sync_available?).and_return(enabled)
+ expect(group.members.pluck(:access_level)).to eq([Gitlab::Access::MAINTAINER])
end
- let!(:group_link) { create(:saml_group_link, saml_group_name: 'Owners', group: group) }
- let!(:subgroup_link) { create(:saml_group_link, saml_group_name: 'Developers', group: create(:group, parent: group)) }
+ it "logs an audit event" do
+ expect do
+ subject
+ end.to change { AuditEvent.by_entity('Group', group).count }.by(1)
- context 'when group sync is not available' do
- before do
- stub_saml_group_sync_available(false)
- end
+ expect(AuditEvent.last.details).to include(add: 'user_access', target_details: user.name, as: 'Developer')
+ end
- it 'does not enqueue group sync' do
- expect(GroupSamlGroupSyncWorker).not_to receive(:perform_async)
+ it_behaves_like 'not enqueueing Group SAML Group Sync worker'
+ it_behaves_like 'not enqueueing Microsoft Group Sync worker'
+ end
- update_membership
- end
- end
+ context 'when SAML group links exist' do
+ let!(:group_link) { create(:saml_group_link, saml_group_name: 'Owners', group: group) }
+ let!(:subgroup_link) { create(:saml_group_link, saml_group_name: 'Developers', group: create(:group, parent: group)) }
- context 'when group sync is available' do
- before do
- stub_saml_group_sync_available(true)
+ context 'when the auth hash contains groups' do
+ let_it_be(:auth_hash) do
+ Gitlab::Auth::GroupSaml::AuthHash.new(
+ OmniAuth::AuthHash.new(extra: {
+ raw_info: OneLogin::RubySaml::Attributes.new('groups' => %w(Developers Owners))
+ })
+ )
end
- it 'enqueues group sync' do
- expect(GroupSamlGroupSyncWorker).to receive(:perform_async).with(user.id, group.id, match_array([group_link.id, subgroup_link.id]))
+ context 'when group sync is not available' do
+ before do
+ stub_saml_group_sync_available(false)
+ end
- update_membership
+ it_behaves_like 'not enqueueing Group SAML Group Sync worker'
end
- context 'with a group link outside the top-level group' do
+ context 'when group sync is available' do
before do
- create(:saml_group_link, saml_group_name: 'Developers', group: create(:group))
+ stub_saml_group_sync_available(true)
end
- it 'enqueues group sync without the outside group' do
- expect(GroupSamlGroupSyncWorker).to receive(:perform_async).with(user.id, group.id, match_array([group_link.id, subgroup_link.id]))
+ it 'enqueues group sync' do
+ expect(GroupSamlGroupSyncWorker)
+ .to receive(:perform_async).with(user.id, group.id, match_array([group_link.id, subgroup_link.id]))
update_membership
end
- end
- context 'when auth hash contains no groups' do
- let!(:auth_hash) do
- Gitlab::Auth::GroupSaml::AuthHash.new(
- OmniAuth::AuthHash.new(extra: { raw_info: OneLogin::RubySaml::Attributes.new })
- )
+ context 'with a group link outside the top-level group' do
+ before do
+ create(:saml_group_link, saml_group_name: 'Developers', group: create(:group))
+ end
+
+ it 'enqueues group sync without the outside group' do
+ expect(GroupSamlGroupSyncWorker)
+ .to receive(:perform_async).with(user.id, group.id, match_array([group_link.id, subgroup_link.id]))
+
+ update_membership
+ end
end
- it 'enqueues group sync' do
- expect(GroupSamlGroupSyncWorker).to receive(:perform_async).with(user.id, group.id, [])
+ context 'when auth hash contains no groups' do
+ let!(:auth_hash) do
+ Gitlab::Auth::GroupSaml::AuthHash.new(
+ OmniAuth::AuthHash.new(extra: { raw_info: OneLogin::RubySaml::Attributes.new })
+ )
+ end
- update_membership
+ it 'enqueues group sync' do
+ expect(GroupSamlGroupSyncWorker).to receive(:perform_async).with(user.id, group.id, [])
+
+ update_membership
+ end
+ end
+
+ context 'when auth hash groups do not match group links' do
+ before do
+ group_link.update!(saml_group_name: 'Web Developers')
+ subgroup_link.destroy!
+ end
+
+ it 'enqueues group sync' do
+ expect(GroupSamlGroupSyncWorker).to receive(:perform_async).with(user.id, group.id, [])
+
+ update_membership
+ end
end
end
+ end
+
+ context 'when the auth hash contains a Microsoft group claim' do
+ let_it_be(:auth_hash) do
+ Gitlab::Auth::GroupSaml::AuthHash.new(
+ OmniAuth::AuthHash.new(extra: {
+ raw_info: OneLogin::RubySaml::Attributes.new({
+ 'http://schemas.microsoft.com/claims/groups.link' =>
+ ['https://graph.windows.net/8c750e43/users/e631c82c/getMemberObjects']
+ })
+ })
+ )
+ end
+
+ context 'when Microsoft Group Sync is not licensed' do
+ let!(:application) { create(:system_access_microsoft_application, enabled: true, namespace: group) }
- context 'when auth hash groups do not match group links' do
before do
- group_link.update!(saml_group_name: 'Web Developers')
- subgroup_link.destroy!
+ stub_feature_flags(microsoft_azure_group_sync: true)
+ stub_saml_group_sync_available(true)
end
- it 'enqueues group sync' do
- expect(GroupSamlGroupSyncWorker).to receive(:perform_async).with(user.id, group.id, [])
+ it_behaves_like 'not enqueueing Microsoft Group Sync worker'
+ end
- update_membership
+ context 'when Microsoft Group Sync is licensed' do
+ before do
+ stub_licensed_features(microsoft_group_sync: true)
+ end
+
+ it_behaves_like 'not enqueueing Microsoft Group Sync worker'
+
+ context 'when SAML Group Sync is not available' do
+ before do
+ stub_saml_group_sync_available(false)
+ end
+
+ it_behaves_like 'not enqueueing Microsoft Group Sync worker'
+
+ context 'when a Microsoft Application is present and enabled' do
+ let!(:application) { create(:system_access_microsoft_application, enabled: true, namespace: group) }
+
+ it_behaves_like 'not enqueueing Microsoft Group Sync worker'
+ end
+ end
+
+ context 'when Group SAML Group Sync is enabled' do
+ before do
+ stub_saml_group_sync_available(true)
+ end
+
+ it_behaves_like 'not enqueueing Microsoft Group Sync worker'
+
+ context 'when a Microsoft Application is present' do
+ let!(:application) { create(:system_access_microsoft_application, namespace: group) }
+
+ context 'when the Microsoft Application is not enabled' do
+ before do
+ application.update!(enabled: false)
+ end
+
+ it_behaves_like 'not enqueueing Microsoft Group Sync worker'
+ end
+
+ context 'when the Microsoft application is enabled' do
+ before do
+ application.update!(enabled: true)
+ end
+
+ it 'enqueues Microsoft Group Sync worker' do
+ expect(::SystemAccess::GroupSamlMicrosoftGroupSyncWorker)
+ .to receive(:perform_async).with(user.id, group.id)
+
+ update_membership
+ end
+
+ it_behaves_like 'not enqueueing Group SAML Group Sync worker'
+
+ context 'when microsoft_azure_group_sync feature flag is not enabled' do
+ before do
+ stub_feature_flags(microsoft_azure_group_sync: false)
+ end
+
+ it_behaves_like 'not enqueueing Microsoft Group Sync worker'
+ end
+ end
+ end
end
end
end
+
+ # Microsoft should never send both, but it's important we're only running
+ # one sync. This test serves to ensure we have that safeguard in place.
+ context 'when the auth hash contains both groups and a group claim' do
+ let_it_be(:auth_hash) do
+ Gitlab::Auth::GroupSaml::AuthHash.new(
+ OmniAuth::AuthHash.new(extra: {
+ raw_info: OneLogin::RubySaml::Attributes.new({
+ 'groups' => %w(Developers Owners),
+ 'http://schemas.microsoft.com/claims/groups.link' =>
+ ['https://graph.windows.net/8c750e43/users/e631c82c/getMemberObjects']
+ })
+ })
+ )
+ end
+
+ let!(:application) { create(:system_access_microsoft_application, enabled: true, namespace: group) }
+
+ before do
+ stub_licensed_features(microsoft_group_sync: true)
+ stub_saml_group_sync_available(true)
+ end
+
+ it 'enqueues Microsoft Group Sync worker' do
+ expect(::SystemAccess::GroupSamlMicrosoftGroupSyncWorker)
+ .to receive(:perform_async).with(user.id, group.id)
+
+ update_membership
+ end
+
+ it_behaves_like 'not enqueueing Group SAML Group Sync worker'
+ end
+ end
+
+ def stub_saml_group_sync_available(enabled)
+ allow(group).to receive(:saml_group_sync_available?).and_return(enabled)
end
end