Commit 9091eaf7 authored by Arturo Herrero's avatar Arturo Herrero

Merge branch 'lm-security-increase-authorization-for-lint-endpoint' into 'master'

Updates authorization for lint [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!62004
parents 71302e5f f8524e7f
---
name: security_ci_lint_authorization
introduced_by_url: https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/1279
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/326708
milestone: '14.0'
type: development
group: group::pipeline authoring
default_enabled: false
...@@ -13,7 +13,16 @@ info: To determine the technical writer assigned to the Stage/Group associated w ...@@ -13,7 +13,16 @@ info: To determine the technical writer assigned to the Stage/Group associated w
Checks if CI/CD YAML configuration is valid. This endpoint validates basic CI/CD Checks if CI/CD YAML configuration is valid. This endpoint validates basic CI/CD
configuration syntax. It doesn't have any namespace specific context. configuration syntax. It doesn't have any namespace specific context.
Access to this endpoint requires authentication. Access to this endpoint does not require authentication when the instance
[allows new sign ups](../user/admin_area/settings/sign_up_restrictions.md#disable-new-sign-ups)
and:
- Does not have an [allowlist or denylist](../user/admin_area/settings/sign_up_restrictions.md#allow-or-deny-sign-ups-using-specific-email-domains).
- Does not [require administrator approval for new sign ups](../user/admin_area/settings/sign_up_restrictions.md#require-administrator-approval-for-new-sign-ups).
- Does not have additional [sign up
restrictions](../user/admin_area/settings/sign_up_restrictions.html#sign-up-restrictions).
Otherwise, authentication is required.
```plaintext ```plaintext
POST /ci/lint POST /ci/lint
......
...@@ -11,7 +11,11 @@ module API ...@@ -11,7 +11,11 @@ module API
optional :include_merged_yaml, type: Boolean, desc: 'Whether or not to include merged CI config yaml in the response' optional :include_merged_yaml, type: Boolean, desc: 'Whether or not to include merged CI config yaml in the response'
end end
post '/lint' do post '/lint' do
unauthorized! if Gitlab::CurrentSettings.signup_disabled? && current_user.nil? if Feature.enabled?(:security_ci_lint_authorization)
unauthorized! if (Gitlab::CurrentSettings.signup_disabled? || Gitlab::CurrentSettings.signup_limited?) && current_user.nil?
else
unauthorized! if Gitlab::CurrentSettings.signup_disabled? && current_user.nil?
end
result = Gitlab::Ci::YamlProcessor.new(params[:content], user: current_user).execute result = Gitlab::Ci::YamlProcessor.new(params[:content], user: current_user).execute
......
...@@ -7,6 +7,10 @@ module Gitlab ...@@ -7,6 +7,10 @@ module Gitlab
!signup_enabled? !signup_enabled?
end end
def signup_limited?
domain_allowlist.present? || email_restrictions_enabled? || require_admin_approval_after_user_signup?
end
def current_application_settings def current_application_settings
Gitlab::SafeRequestStore.fetch(:current_application_settings) { ensure_application_settings! } Gitlab::SafeRequestStore.fetch(:current_application_settings) { ensure_application_settings! }
end end
......
...@@ -24,6 +24,42 @@ RSpec.describe Gitlab::CurrentSettings do ...@@ -24,6 +24,42 @@ RSpec.describe Gitlab::CurrentSettings do
end end
end end
describe '.signup_limited?' do
subject { described_class.signup_limited? }
context 'when there are allowed domains' do
before do
create(:application_setting, domain_allowlist: ['www.gitlab.com'])
end
it { is_expected.to be_truthy }
end
context 'when there are email restrictions' do
before do
create(:application_setting, email_restrictions_enabled: true)
end
it { is_expected.to be_truthy }
end
context 'when the admin has to approve signups' do
before do
create(:application_setting, require_admin_approval_after_user_signup: true)
end
it { is_expected.to be_truthy }
end
context 'when there are no restrictions' do
before do
create(:application_setting, domain_allowlist: [], email_restrictions_enabled: false, require_admin_approval_after_user_signup: false)
end
it { is_expected.to be_falsey }
end
end
describe '.signup_disabled?' do describe '.signup_disabled?' do
subject { described_class.signup_disabled? } subject { described_class.signup_disabled? }
......
...@@ -27,9 +27,10 @@ RSpec.describe API::Lint do ...@@ -27,9 +27,10 @@ RSpec.describe API::Lint do
end end
end end
context 'when signup settings are enabled' do context 'when signup is enabled and not limited' do
before do before do
Gitlab::CurrentSettings.signup_enabled = true Gitlab::CurrentSettings.signup_enabled = true
stub_application_setting(domain_allowlist: [], email_restrictions_enabled: false, require_admin_approval_after_user_signup: false)
end end
context 'when unauthenticated' do context 'when unauthenticated' do
...@@ -50,6 +51,31 @@ RSpec.describe API::Lint do ...@@ -50,6 +51,31 @@ RSpec.describe API::Lint do
end end
end end
context 'when limited signup is enabled' do
before do
stub_application_setting(domain_allowlist: ['www.gitlab.com'])
Gitlab::CurrentSettings.signup_enabled = true
end
context 'when unauthenticated' do
it 'returns unauthorized' do
post api('/ci/lint'), params: { content: 'content' }
expect(response).to have_gitlab_http_status(:unauthorized)
end
end
context 'when authenticated' do
let_it_be(:api_user) { create(:user) }
it 'returns authentication success' do
post api('/ci/lint', api_user), params: { content: 'content' }
expect(response).to have_gitlab_http_status(:ok)
end
end
end
context 'when authenticated' do context 'when authenticated' do
let_it_be(:api_user) { create(:user) } let_it_be(:api_user) { create(:user) }
......
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