Commit 96150fbc authored by Bogdan Denkovych's avatar Bogdan Denkovych

Allow creating a group access token for a group with SSO enforcement

For each project access token created, a project bot user is created
and added to the project

GitLab docs explain how to create a group access token manually.
https://docs.gitlab.com/ee/user/project/settings/project_access_tokens.html#group-access-token-workaround

If a project is under the group that has SSO enforcement enabled, it
won't allow to add a user not linked to the SAML account as a member of
this project. It didn't allow to create project access token for a
project under SSO enforcement since project bot users are not supposed to be
linked SAML account.
It was fixed in https://gitlab.com/gitlab-org/gitlab/-/commit/f9df9d2f97f091a4633ee8497e81921e4d6ebfff

I [noticed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74240/diffs#307d381390eea67e67da59bea05d1f657b9b8e5d_22_25)
that we have the same issue on creating group access token.
https://app.slack.com/client/T02592416/CLM1D8QR0

This change makes it possible to create group access token for a group
that has SSO enforcement enabled.

Changelog: fixed
EE: true
parent 57ac6715
...@@ -8,7 +8,7 @@ module EE ...@@ -8,7 +8,7 @@ module EE
prepended do prepended do
include UsageStatistics include UsageStatistics
validate :sso_enforcement, if: :group validate :sso_enforcement, if: -> { group && user }
validate :group_domain_limitations, if: :group_has_domain_limitations? validate :group_domain_limitations, if: :group_has_domain_limitations?
scope :by_group_ids, ->(group_ids) { where(source_id: group_ids) } scope :by_group_ids, ->(group_ids) { where(source_id: group_ids) }
......
...@@ -7,7 +7,7 @@ module EE ...@@ -7,7 +7,7 @@ module EE
prepended do prepended do
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
validate :sso_enforcement, if: :group, unless: :project_bot validate :sso_enforcement, if: -> { group && user }
validate :gma_enforcement, if: :group, unless: :project_bot validate :gma_enforcement, if: :group, unless: :project_bot
validate :group_domain_limitations, if: -> { group && group_has_domain_limitations? }, on: :create validate :group_domain_limitations, if: -> { group && group_has_domain_limitations? }, on: :create
......
...@@ -9,7 +9,8 @@ module Gitlab ...@@ -9,7 +9,8 @@ module Gitlab
end end
def can_add_user?(user) def can_add_user?(user)
return true unless root_group&.saml_provider&.enforced_sso? return true unless root_group.saml_provider&.enforced_sso?
return true if user.project_bot?
GroupSamlIdentityFinder.new(user: user).find_linked(group: root_group) GroupSamlIdentityFinder.new(user: user).find_linked(group: root_group)
end end
...@@ -17,7 +18,7 @@ module Gitlab ...@@ -17,7 +18,7 @@ module Gitlab
private private
def root_group def root_group
@root_group ||= @group&.root_ancestor @root_group ||= @group.root_ancestor
end end
end end
end end
......
...@@ -10,13 +10,19 @@ RSpec.describe Gitlab::Auth::GroupSaml::MembershipEnforcer do ...@@ -10,13 +10,19 @@ RSpec.describe Gitlab::Auth::GroupSaml::MembershipEnforcer do
allow_any_instance_of(SamlProvider).to receive(:enforced_sso?).and_return(true) allow_any_instance_of(SamlProvider).to receive(:enforced_sso?).and_return(true)
end end
it 'allows adding the group member' do it 'allows adding a user linked to the SAML account as member' do
expect(described_class.new(group).can_add_user?(user)).to be_truthy expect(described_class.new(group).can_add_user?(user)).to be_truthy
end end
it 'does not add the group member' do it 'does not allow adding a user not linked to the SAML account as member' do
non_saml_user = create(:user) non_saml_user = create(:user)
expect(described_class.new(group).can_add_user?(non_saml_user)).to be_falsey expect(described_class.new(group).can_add_user?(non_saml_user)).to be_falsey
end end
it 'allows adding a project bot as member' do
project_bot = create(:user, :project_bot)
expect(described_class.new(group).can_add_user?(project_bot)).to be_truthy
end
end end
...@@ -42,36 +42,6 @@ RSpec.describe ProjectMember do ...@@ -42,36 +42,6 @@ RSpec.describe ProjectMember do
end end
end end
context "for SSO enforced groups" do
let(:group) { create(:group, :private) }
let!(:saml_provider) { create(:saml_provider, group: group, enforced_sso: true) }
let(:identity) { create(:group_saml_identity, saml_provider: saml_provider) }
before do
stub_licensed_features(group_saml: true)
end
it 'allows adding a user linked to the SAML account as project member' do
sso_user = identity.user
member = entity.add_developer(sso_user)
expect(member).to be_valid
end
it 'does not allow adding a user not linked to the SAML account as a project member' do
member = entity.add_developer(create(:user))
expect(member).not_to be_valid
expect(member.errors.messages[:user]).to include('is not linked to a SAML account')
end
it 'allows adding a project bot' do
member = entity.add_developer(create(:user, :project_bot))
expect(member).to be_valid
end
end
context 'enforced group managed account disabled' do context 'enforced group managed account disabled' do
it 'allows adding any user as project member' do it 'allows adding any user as project member' do
member = entity.add_developer(create(:user)) member = entity.add_developer(create(:user))
...@@ -79,14 +49,6 @@ RSpec.describe ProjectMember do ...@@ -79,14 +49,6 @@ RSpec.describe ProjectMember do
expect(member).to be_valid expect(member).to be_valid
end end
end end
context 'enforced SSO disabled' do
it 'allows adding any user as project member' do
member = entity.add_developer(create(:user))
expect(member).to be_valid
end
end
end end
describe '#group_domain_validations' do describe '#group_domain_validations' do
......
...@@ -13,19 +13,25 @@ RSpec.shared_examples 'member validations' do ...@@ -13,19 +13,25 @@ RSpec.shared_examples 'member validations' do
allow_any_instance_of(SamlProvider).to receive(:enforced_sso?).and_return(true) allow_any_instance_of(SamlProvider).to receive(:enforced_sso?).and_return(true)
end end
it 'allows adding the group member' do it 'allows adding a user linked to the SAML account as member' do
member = entity.add_user(user, Member::DEVELOPER) member = entity.add_user(user, Member::DEVELOPER)
expect(member).to be_valid expect(member).to be_valid
end end
it 'does not add the group member' do it 'does not allow adding a user not linked to the SAML account as member' do
member = entity.add_user(create(:user), Member::DEVELOPER) member = entity.add_user(create(:user), Member::DEVELOPER)
expect(member).not_to be_valid expect(member).not_to be_valid
expect(member.errors.messages[:user]).to eq(['is not linked to a SAML account']) expect(member.errors.messages[:user]).to eq(['is not linked to a SAML account'])
end end
it 'allows adding a project bot as member' do
member = entity.add_user(create(:user, :project_bot), Member::DEVELOPER)
expect(member).to be_valid
end
context 'subgroups' do context 'subgroups' do
let!(:subgroup) { create(:group, parent: group) } let!(:subgroup) { create(:group, parent: group) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment