Commit 84804e8c authored by Stan Hu's avatar Stan Hu

Merge branch '339888-group-saml-scheduled-pipelines-cannot-find-valid-sso-session' into 'master'

Do not check SSO session for Git operations originating from CI/CD jobs

See merge request gitlab-org/gitlab!76909
parents 454afb79 fc84cab8
......@@ -110,6 +110,7 @@ The certificate [fingerprint algorithm](../../../integration/saml.md#notes-on-co
> - [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/292811) in GitLab 13.8, with an updated timeout experience.
> - [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/211962) in GitLab 13.8 with allowing group owners to not go through SSO.
> - [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/9152) in GitLab 13.11 with enforcing open SSO session to use Git if this setting is switched on.
> - [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/339888) in GitLab 14.7 to not enforce SSO checks for Git activity originating from CI/CD jobs.
With this option enabled, users (except users with the Owner role) must access GitLab using your group GitLab single sign-on URL to access group resources. Users added manually as members can't access group resources.
......@@ -127,6 +128,7 @@ SSO has the following effects when enabled:
even if the project is forked.
- For Git activity over SSH and HTTPS, users must have at least one active session signed-in through SSO before they can push to or
pull from a GitLab repository.
- Git activity originating from CI/CD jobs do not have the SSO check enforced.
- Credentials that are not tied to regular users (for example, access tokens and deploy keys) do not have the SSO check enforced.
- Users must be signed-in through SSO before they can pull images using the [Dependency Proxy](../../packages/dependency_proxy/index.md).
<!-- Add bullet for API activity when https://gitlab.com/gitlab-org/gitlab/-/issues/9152 is complete -->
......
......@@ -119,6 +119,7 @@ module EE
def check_sso_session!
return true unless user && container
return if request_from_ci_build?
return unless ::Gitlab::Auth::GroupSaml::SessionEnforcer.new(user, containing_group).access_restricted?
......
......@@ -14,6 +14,7 @@ RSpec.describe Gitlab::GitAccess do
let(:repository) { project.repository }
let(:repository_path) { "#{project.full_path}.git" }
let(:protocol) { 'web' }
let(:auth_result_type) { nil }
let(:authentication_abilities) { %i[read_project download_code push_code] }
let(:redirected_path) { nil }
......@@ -963,29 +964,54 @@ RSpec.describe Gitlab::GitAccess do
context 'user without a sso session' do
let(:access_restricted?) { true }
before do
expect(Gitlab::Auth::GroupSaml::SessionEnforcer).to receive(:new).with(user, group).twice.and_return(double(access_restricted?: access_restricted?))
end
context 'when the request is made directly by the user' do
before do
expect(Gitlab::Auth::GroupSaml::SessionEnforcer).to receive(:new).with(user, group).twice.and_return(double(access_restricted?: access_restricted?))
end
it 'does not allow pull or push changes with proper url in the message' do
aggregate_failures do
address = "http://localhost/groups/#{group.name}/-/saml/sso?token=#{group.saml_discovery_token}"
it 'does not allow pull or push changes with proper url in the message' do
aggregate_failures do
address = "http://localhost/groups/#{group.name}/-/saml/sso?token=#{group.saml_discovery_token}"
expect { pull_changes }.to raise_error(Gitlab::GitAccess::ForbiddenError, /#{Regexp.quote(address)}/)
expect { push_changes }.to raise_error(Gitlab::GitAccess::ForbiddenError, /#{Regexp.quote(address)}/)
end
end
context 'with a subgroup' do
let_it_be(:root_group) { create(:group) }
let_it_be(:group) { create(:group, parent: root_group) }
expect { pull_changes }.to raise_error(Gitlab::GitAccess::ForbiddenError, /#{Regexp.quote(address)}/)
expect { push_changes }.to raise_error(Gitlab::GitAccess::ForbiddenError, /#{Regexp.quote(address)}/)
it 'does not allow pull or push changes with proper url in the message' do
aggregate_failures do
address = "http://localhost/groups/#{root_group.name}/-/saml/sso?token=#{root_group.saml_discovery_token}"
expect { pull_changes }.to raise_error(Gitlab::GitAccess::ForbiddenError, /#{Regexp.quote(address)}/)
expect { push_changes }.to raise_error(Gitlab::GitAccess::ForbiddenError, /#{Regexp.quote(address)}/)
end
end
end
end
context 'with a subgroup' do
let_it_be(:root_group) { create(:group) }
let_it_be(:group) { create(:group, parent: root_group) }
context 'when the request is made from CI builds' do
let(:protocol) { 'http' }
let(:auth_result_type) { :build }
it 'does not allow pull or push changes with proper url in the message' do
it 'allows pull and push changes' do
aggregate_failures do
address = "http://localhost/groups/#{root_group.name}/-/saml/sso?token=#{root_group.saml_discovery_token}"
expect { pull_changes }.not_to raise_error
expect { push_changes }.not_to raise_error
end
end
expect { pull_changes }.to raise_error(Gitlab::GitAccess::ForbiddenError, /#{Regexp.quote(address)}/)
expect { push_changes }.to raise_error(Gitlab::GitAccess::ForbiddenError, /#{Regexp.quote(address)}/)
context 'when legacy CI credentials are used' do
let(:auth_result_type) { :ci }
it 'allows pull and push changes' do
aggregate_failures do
expect { pull_changes }.not_to raise_error
expect { push_changes }.not_to raise_error
end
end
end
end
......@@ -1052,7 +1078,8 @@ RSpec.describe Gitlab::GitAccess do
protocol,
authentication_abilities: authentication_abilities,
repository_path: repository_path,
redirected_path: redirected_path
redirected_path: redirected_path,
auth_result_type: auth_result_type
)
end
......
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