diff options
author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-05-02 21:12:53 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-05-02 21:12:53 +0000 |
commit | 2bdcc43491635132fdab4e4eadb7e62b731140ea (patch) | |
tree | b61b0ceb52da5d5ea36c13370fb87f09c7db6ffe /spec | |
parent | 7bc34aad19503153d8d9b849006e59b43a1eda5b (diff) | |
parent | 1390b6e51192c50ebf55378fc183cbd4ddf94ab0 (diff) | |
download | gitlab-ce-2bdcc43491635132fdab4e4eadb7e62b731140ea.tar.gz |
Merge remote-tracking branch 'dev/14-8-stable' into 14-8-stable14-8-stable
Diffstat (limited to 'spec')
31 files changed, 822 insertions, 291 deletions
diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index f410c16b30b..433114f3e64 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -204,6 +204,44 @@ RSpec.describe Projects::ArtifactsController do end end end + + context 'when downloading a debug trace' do + let(:file_type) { 'trace' } + let(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) } + + before do + create(:ci_job_variable, key: 'CI_DEBUG_TRACE', value: 'true', job: job) + end + + context 'when the user does not have update_build permissions' do + let(:user) { create(:user) } + + before do + project.add_guest(user) + end + + render_views + + it 'denies the user access' do + download_artifact(file_type: file_type) + + expect(response).to have_gitlab_http_status(:forbidden) + expect(response.body).to include( + 'You must have developer or higher permissions in the associated project to view job logs when debug trace is enabled. ' \ + 'To disable debug trace, set the 'CI_DEBUG_TRACE' variable to 'false' in your pipeline configuration or CI/CD settings. ' \ + 'If you need to view this job log, a project maintainer must add you to the project with developer permissions or higher.' + ) + end + end + + context 'when the user has update_build permissions' do + it 'sends the trace' do + download_artifact(file_type: file_type) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end end describe 'GET browse' do diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index d86f38c1f0b..77acd5fe13c 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -13,10 +13,43 @@ RSpec.describe Projects::PipelineSchedulesController do project.add_developer(user) end + shared_examples 'access update schedule' do + describe 'security' do + it 'is allowed for admin when admin mode enabled', :enable_admin_mode do + expect { go }.to be_allowed_for(:admin) + end + + it 'is denied for admin when admin mode disabled' do + expect { go }.to be_denied_for(:admin) + end + + it { expect { go }.to be_denied_for(:owner).of(project) } + it { expect { go }.to be_denied_for(:maintainer).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + it { expect { go }.to be_denied_for(:visitor) } + + context 'when user is schedule owner' do + it { expect { go }.to be_allowed_for(:owner).of(project).own(pipeline_schedule) } + it { expect { go }.to be_allowed_for(:maintainer).of(project).own(pipeline_schedule) } + it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:reporter).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:guest).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:user).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:external).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:visitor).own(pipeline_schedule) } + end + end + end + describe 'GET #index' do render_views let(:scope) { nil } + let!(:inactive_pipeline_schedule) do create(:ci_pipeline_schedule, :inactive, project: project) end @@ -130,12 +163,15 @@ RSpec.describe Projects::PipelineSchedulesController do it 'is allowed for admin when admin mode enabled', :enable_admin_mode do expect { go }.to be_allowed_for(:admin) end + it 'is denied for admin when admin mode disabled' do expect { go }.to be_denied_for(:admin) end + it { expect { go }.to be_allowed_for(:owner).of(project) } it { expect { go }.to be_allowed_for(:maintainer).of(project) } it { expect { go }.to be_allowed_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } it { expect { go }.to be_denied_for(:guest).of(project) } it { expect { go }.to be_denied_for(:user) } @@ -284,20 +320,7 @@ RSpec.describe Projects::PipelineSchedulesController do describe 'security' do let(:schedule) { { description: 'updated_desc' } } - it 'is allowed for admin when admin mode enabled', :enable_admin_mode do - expect { go }.to be_allowed_for(:admin) - end - it 'is denied for admin when admin mode disabled' do - expect { go }.to be_denied_for(:admin) - end - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:maintainer).of(project) } - it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - it { expect { go }.to be_denied_for(:visitor) } + it_behaves_like 'access update schedule' context 'when a developer created a pipeline schedule' do let(:developer_1) { create(:user) } @@ -308,8 +331,10 @@ RSpec.describe Projects::PipelineSchedulesController do end it { expect { go }.to be_allowed_for(developer_1) } + + it { expect { go }.to be_denied_for(:owner).of(project) } + it { expect { go }.to be_denied_for(:maintainer).of(project) } it { expect { go }.to be_denied_for(:developer).of(project) } - it { expect { go }.to be_allowed_for(:maintainer).of(project) } end context 'when a maintainer created a pipeline schedule' do @@ -321,17 +346,21 @@ RSpec.describe Projects::PipelineSchedulesController do end it { expect { go }.to be_allowed_for(maintainer_1) } - it { expect { go }.to be_allowed_for(:maintainer).of(project) } + + it { expect { go }.to be_denied_for(:owner).of(project) } + it { expect { go }.to be_denied_for(:maintainer).of(project) } it { expect { go }.to be_denied_for(:developer).of(project) } end end def go - put :update, params: { namespace_id: project.namespace.to_param, - project_id: project, - id: pipeline_schedule, - schedule: schedule }, - as: :html + put :update, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: pipeline_schedule, + schedule: schedule + }, + as: :html end end @@ -341,6 +370,7 @@ RSpec.describe Projects::PipelineSchedulesController do before do project.add_maintainer(user) + pipeline_schedule.update!(owner: user) sign_in(user) end @@ -352,22 +382,7 @@ RSpec.describe Projects::PipelineSchedulesController do end end - describe 'security' do - it 'is allowed for admin when admin mode enabled', :enable_admin_mode do - expect { go }.to be_allowed_for(:admin) - end - it 'is denied for admin when admin mode disabled' do - expect { go }.to be_denied_for(:admin) - end - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:maintainer).of(project) } - it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - it { expect { go }.to be_denied_for(:visitor) } - end + it_behaves_like 'access update schedule' def go get :edit, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id } @@ -379,17 +394,30 @@ RSpec.describe Projects::PipelineSchedulesController do it 'is allowed for admin when admin mode enabled', :enable_admin_mode do expect { go }.to be_allowed_for(:admin) end + it 'is denied for admin when admin mode disabled' do expect { go }.to be_denied_for(:admin) end + it { expect { go }.to be_allowed_for(:owner).of(project) } it { expect { go }.to be_allowed_for(:maintainer).of(project) } - it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:developer).of(project) } it { expect { go }.to be_denied_for(:reporter).of(project) } it { expect { go }.to be_denied_for(:guest).of(project) } it { expect { go }.to be_denied_for(:user) } it { expect { go }.to be_denied_for(:external) } it { expect { go }.to be_denied_for(:visitor) } + + context 'when user is schedule owner' do + it { expect { go }.to be_denied_for(:owner).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:maintainer).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:developer).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:reporter).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:guest).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:user).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:external).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:visitor).own(pipeline_schedule) } + end end def go diff --git a/spec/features/projects/pipeline_schedules_spec.rb b/spec/features/projects/pipeline_schedules_spec.rb index aae5ab58b5d..29f190028bd 100644 --- a/spec/features/projects/pipeline_schedules_spec.rb +++ b/spec/features/projects/pipeline_schedules_spec.rb @@ -9,7 +9,77 @@ RSpec.describe 'Pipeline Schedules', :js do let(:scope) { nil } let!(:user) { create(:user) } - context 'logged in as maintainer' do + context 'logged in as the pipeline scheduler owner' do + before do + stub_feature_flags(bootstrap_confirmation_modals: false) + project.add_developer(user) + pipeline_schedule.update!(owner: user) + gitlab_sign_in(user) + end + + describe 'GET /projects/pipeline_schedules' do + before do + visit_pipelines_schedules + end + + it 'edits the pipeline' do + page.within('.pipeline-schedule-table-row') do + click_link 'Edit' + end + + expect(page).to have_content('Edit Pipeline Schedule') + end + end + + describe 'PATCH /projects/pipelines_schedules/:id/edit' do + before do + edit_pipeline_schedule + end + + it 'displays existing properties' do + description = find_field('schedule_description').value + expect(description).to eq('pipeline schedule') + expect(page).to have_button('master') + expect(page).to have_button('UTC') + end + + it 'edits the scheduled pipeline' do + fill_in 'schedule_description', with: 'my brand new description' + + save_pipeline_schedule + + expect(page).to have_content('my brand new description') + end + + context 'when ref is nil' do + before do + pipeline_schedule.update_attribute(:ref, nil) + edit_pipeline_schedule + end + + it 'shows the pipeline schedule with default ref' do + page.within('.js-target-branch-dropdown') do + expect(first('.dropdown-toggle-text').text).to eq('master') + end + end + end + + context 'when ref is empty' do + before do + pipeline_schedule.update_attribute(:ref, '') + edit_pipeline_schedule + end + + it 'shows the pipeline schedule with default ref' do + page.within('.js-target-branch-dropdown') do + expect(first('.dropdown-toggle-text').text).to eq('master') + end + end + end + end + end + + context 'logged in as a project maintainer' do before do stub_feature_flags(bootstrap_confirmation_modals: false) project.add_maintainer(user) @@ -46,14 +116,6 @@ RSpec.describe 'Pipeline Schedules', :js do end end - it 'edits the pipeline' do - page.within('.pipeline-schedule-table-row') do - click_link 'Edit' - end - - expect(page).to have_content('Edit Pipeline Schedule') - end - it 'deletes the pipeline' do accept_confirm { click_link 'Delete' } @@ -108,53 +170,6 @@ RSpec.describe 'Pipeline Schedules', :js do end end - describe 'PATCH /projects/pipelines_schedules/:id/edit' do - before do - edit_pipeline_schedule - end - - it 'displays existing properties' do - description = find_field('schedule_description').value - expect(description).to eq('pipeline schedule') - expect(page).to have_button('master') - expect(page).to have_button('UTC') - end - - it 'edits the scheduled pipeline' do - fill_in 'schedule_description', with: 'my brand new description' - - save_pipeline_schedule - - expect(page).to have_content('my brand new description') - end - - context 'when ref is nil' do - before do - pipeline_schedule.update_attribute(:ref, nil) - edit_pipeline_schedule - end - - it 'shows the pipeline schedule with default ref' do - page.within('.js-target-branch-dropdown') do - expect(first('.dropdown-toggle-text').text).to eq('master') - end - end - end - - context 'when ref is empty' do - before do - pipeline_schedule.update_attribute(:ref, '') - edit_pipeline_schedule - end - - it 'shows the pipeline schedule with default ref' do - page.within('.js-target-branch-dropdown') do - expect(first('.dropdown-toggle-text').text).to eq('master') - end - end - end - end - context 'when user creates a new pipeline schedule with variables' do before do visit_pipelines_schedules diff --git a/spec/fixtures/lib/gitlab/import_export/complex/project.json b/spec/fixtures/lib/gitlab/import_export/complex/project.json index 95f2ce45b46..ce0df4250d6 100644 --- a/spec/fixtures/lib/gitlab/import_export/complex/project.json +++ b/spec/fixtures/lib/gitlab/import_export/complex/project.json @@ -4,6 +4,7 @@ "creator_id": 123, "visibility_level": 10, "archived": false, + "ci_config_path": "config/path", "labels": [ { "id": 2, diff --git a/spec/fixtures/lib/gitlab/import_export/complex/tree/project.json b/spec/fixtures/lib/gitlab/import_export/complex/tree/project.json index 2c5045ce806..9e03e3e51a2 100644 --- a/spec/fixtures/lib/gitlab/import_export/complex/tree/project.json +++ b/spec/fixtures/lib/gitlab/import_export/complex/tree/project.json @@ -6,5 +6,6 @@ "archived": false, "deploy_keys": [], "hooks": [], - "shared_runners_enabled": true + "shared_runners_enabled": true, + "ci_config_path": "config/path" } diff --git a/spec/lib/gitlab/auth/request_authenticator_spec.rb b/spec/lib/gitlab/auth/request_authenticator_spec.rb index 5e9d07a8bf7..fa20fd22886 100644 --- a/spec/lib/gitlab/auth/request_authenticator_spec.rb +++ b/spec/lib/gitlab/auth/request_authenticator_spec.rb @@ -44,6 +44,38 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do end end + describe '#find_authenticated_requester' do + let_it_be(:api_user) { create(:user) } + let_it_be(:deploy_token_user) { create(:user) } + + it 'returns the deploy token if it exists' do + allow_next_instance_of(described_class) do |authenticator| + expect(authenticator).to receive(:deploy_token_from_request).and_return(deploy_token_user) + allow(authenticator).to receive(:user).and_return(nil) + end + + expect(subject.find_authenticated_requester([:api])).to eq deploy_token_user + end + + it 'returns the user id if it exists' do + allow_next_instance_of(described_class) do |authenticator| + allow(authenticator).to receive(:deploy_token_from_request).and_return(deploy_token_user) + expect(authenticator).to receive(:user).and_return(api_user) + end + + expect(subject.find_authenticated_requester([:api])).to eq api_user + end + + it 'rerturns nil if no match is found' do + allow_next_instance_of(described_class) do |authenticator| + expect(authenticator).to receive(:deploy_token_from_request).and_return(nil) + expect(authenticator).to receive(:user).and_return(nil) + end + + expect(subject.find_authenticated_requester([:api])).to eq nil + end + end + describe '#find_sessionless_user' do let_it_be(:dependency_proxy_user) { build(:user) } let_it_be(:access_token_user) { build(:user) } @@ -348,10 +380,10 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do describe '#route_authentication_setting' do using RSpec::Parameterized::TableSyntax - where(:script_name, :expected_job_token_allowed, :expected_basic_auth_personal_access_token) do - '/api/endpoint' | true | true - '/namespace/project.git' | false | true - '/web/endpoint' | false | false + where(:script_name, :expected_job_token_allowed, :expected_basic_auth_personal_access_token, :expected_deploy_token_allowed) do + '/api/endpoint' | true | true | true + '/namespace/project.git' | false | true | true + '/web/endpoint' | false | false | false end with_them do @@ -362,7 +394,8 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do it 'returns correct settings' do expect(subject.send(:route_authentication_setting)).to eql({ job_token_allowed: expected_job_token_allowed, - basic_auth_personal_access_token: expected_basic_auth_personal_access_token + basic_auth_personal_access_token: expected_basic_auth_personal_access_token, + deploy_token_allowed: expected_deploy_token_allowed }) end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb index bcea6462790..96ada90b4e1 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb @@ -22,6 +22,19 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Helpers do let(:command) { double(save_incompleted: true) } let(:message) { 'message' } + describe '.warning' do + context 'when the warning includes malicious HTML' do + let(:message) { '<div>gimme your password</div>' } + let(:sanitized_message) { 'gimme your password' } + + it 'sanitizes' do + subject.warning(message) + + expect(pipeline.warning_messages[0].content).to include(sanitized_message) + end + end + end + describe '.error' do shared_examples 'error function' do specify do @@ -36,6 +49,18 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Helpers do end end + context 'when the error includes malicious HTML' do + let(:message) { '<div>gimme your password</div>' } + let(:sanitized_message) { 'gimme your password' } + + it 'sanitizes the error and removes the HTML tags' do + subject.error(message, config_error: true, drop_reason: :config_error) + + expect(pipeline.yaml_errors).to eq(sanitized_message) + expect(pipeline.errors[:base]).to include(sanitized_message) + end + end + context 'when given a drop reason' do context 'when config error is true' do context 'sets the yaml error and overrides the drop reason' do diff --git a/spec/lib/gitlab/conan_token_spec.rb b/spec/lib/gitlab/conan_token_spec.rb index b6180f69044..c8bda0a5cf0 100644 --- a/spec/lib/gitlab/conan_token_spec.rb +++ b/spec/lib/gitlab/conan_token_spec.rb @@ -25,13 +25,17 @@ RSpec.describe Gitlab::ConanToken do end describe '.from_personal_access_token' do - it 'sets access token id and user id' do - access_token = double(id: 123, user_id: 456) + it 'sets access token and user id and does not use the token id' do + personal_access_token = double(id: 999, token: 123, user_id: 456) - token = described_class.from_personal_access_token(access_token) + token = described_class.from_personal_access_token( + personal_access_token.user_id, + personal_access_token.token + ) - expect(token.access_token_id).to eq(123) - expect(token.user_id).to eq(456) + expect(token.access_token_id).not_to eq(personal_access_token.id) + expect(token.access_token_id).to eq(personal_access_token.token) + expect(token.user_id).to eq(personal_access_token.user_id) end end diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb index 27175dc8c44..de3e674c3a7 100644 --- a/spec/lib/gitlab/git_access_wiki_spec.rb +++ b/spec/lib/gitlab/git_access_wiki_spec.rb @@ -4,8 +4,9 @@ require 'spec_helper' RSpec.describe Gitlab::GitAccessWiki do let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, :wiki_repo) } - let_it_be(:wiki) { create(:project_wiki, project: project) } + let_it_be_with_reload(:project) { create(:project, :wiki_repo) } + + let(:wiki) { create(:project_wiki, project: project) } let(:changes) { ['6f6d7e7ed 570e7b2ab refs/heads/master'] } let(:authentication_abilities) { %i[read_project download_code push_code] } @@ -17,6 +18,61 @@ RSpec.describe Gitlab::GitAccessWiki do redirected_path: redirected_path) end + RSpec.shared_examples 'wiki access by level' do + where(:project_visibility, :project_member?, :wiki_access_level, :wiki_repo?, :expected_behavior) do + [ + # Private project - is a project member + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::ENABLED, true, :no_error], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::PRIVATE, true, :no_error], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::DISABLED, true, :forbidden_wiki], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::ENABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::DISABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::PRIVATE, false, :not_found_wiki], + # Private project - is NOT a project member + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::ENABLED, true, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::PRIVATE, true, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::DISABLED, true, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::ENABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::DISABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::PRIVATE, false, :not_found_wiki], + # Public project - is a project member + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::ENABLED, true, :no_error], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::PRIVATE, true, :no_error], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::DISABLED, true, :forbidden_wiki], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::ENABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::DISABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::PRIVATE, false, :not_found_wiki], + # Public project - is NOT a project member + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::ENABLED, true, :no_error], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::PRIVATE, true, :forbidden_wiki], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::DISABLED, true, :forbidden_wiki], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::ENABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::DISABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::PRIVATE, false, :not_found_wiki] + ] + end + + with_them do + before do + project.update!(visibility_level: project_visibility) + project.add_developer(user) if project_member? + project.project_feature.update_attribute(:wiki_access_level, wiki_access_level) + allow(wiki.repository).to receive(:exists?).and_return(wiki_repo?) + end + + it 'provides access by level' do + case expected_behavior + when :no_error + expect { subject }.not_to raise_error + when :forbidden_wiki + expect { subject }.to raise_wiki_forbidden + when :not_found_wiki + expect { subject }.to raise_wiki_not_found + end + end + end + end + describe '#push_access_check' do subject { access.check('git-receive-pack', changes) } @@ -28,56 +84,26 @@ RSpec.describe Gitlab::GitAccessWiki do it { expect { subject }.not_to raise_error } context 'when in a read-only GitLab instance' do - let(:message) { "You can't push code to a read-only GitLab instance." } - before do allow(Gitlab::Database).to receive(:read_only?) { true } end - it_behaves_like 'forbidden git access' + it_behaves_like 'forbidden git access' do + let(:message) { "You can't push code to a read-only GitLab instance." } + end end end context 'the user cannot :create_wiki' do - it_behaves_like 'not-found git access' do - let(:message) { 'The wiki you were looking for could not be found.' } - end + it { expect { subject }.to raise_wiki_not_found } end end describe '#check_download_access!' do subject { access.check('git-upload-pack', Gitlab::GitAccess::ANY) } - context 'the user can :download_wiki_code' do - before do - project.add_developer(user) - end - - context 'when wiki feature is disabled' do - before do - project.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED) - end - - it_behaves_like 'forbidden git access' do - let(:message) { include('wiki') } - end - end - - context 'when the repository does not exist' do - before do - allow(wiki.repository).to receive(:exists?).and_return(false) - end - - it_behaves_like 'not-found git access' do - let(:message) { include('for this wiki') } - end - end - end - - context 'the user cannot :download_wiki_code' do - it_behaves_like 'not-found git access' do - let(:message) { include('wiki') } - end + context 'when actor is a user' do + it_behaves_like 'wiki access by level' end context 'when the actor is a deploy token' do @@ -99,10 +125,40 @@ RSpec.describe Gitlab::GitAccessWiki do context 'when the wiki is disabled' do let(:wiki_access_level) { ProjectFeature::DISABLED } - it_behaves_like 'forbidden git access' do - let(:message) { 'You are not allowed to download files from this wiki.' } - end + it { expect { subject }.to raise_wiki_forbidden } end end + + describe 'when actor is a user provided by build via CI_JOB_TOKEN' do + let(:protocol) { 'http' } + let(:authentication_abilities) { [:build_download_code] } + let(:auth_result_type) { :build } + + before do + project.project_feature.update_attribute(:wiki_access_level, wiki_access_level) + end + + subject { access.check('git-upload-pack', changes) } + + it_behaves_like 'wiki access by level' + end + end + + RSpec::Matchers.define :raise_wiki_not_found do + match do |actual| + expect { actual.call }.to raise_error(Gitlab::GitAccess::NotFoundError, include('wiki')) + end + def supports_block_expectations? + true + end + end + + RSpec::Matchers.define :raise_wiki_forbidden do + match do |actual| + expect { subject }.to raise_error(Gitlab::GitAccess::ForbiddenError, include('wiki')) + end + def supports_block_expectations? + true + end end end diff --git a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb index ffbbf9326ec..8477300c318 100644 --- a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb @@ -401,4 +401,22 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory, :use_clean_rails_ expect(created_object.value).to be_nil end end + + context 'merge request access level object' do + let(:relation_sym) { :'ProtectedBranch::MergeAccessLevel' } + let(:relation_hash) { { 'access_level' => 30, 'created_at' => '2022-03-29T09:53:13.457Z', 'updated_at' => '2022-03-29T09:54:13.457Z' } } + + it 'sets access level to maintainer' do + expect(created_object.access_level).to equal(Gitlab::Access::MAINTAINER) + end + end + + context 'push access level object' do + let(:relation_sym) { :'ProtectedBranch::PushAccessLevel' } + let(:relation_hash) { { 'access_level' => 30, 'created_at' => '2022-03-29T09:53:13.457Z', 'updated_at' => '2022-03-29T09:54:13.457Z' } } + + it 'sets access level to maintainer' do + expect(created_object.access_level).to equal(Gitlab::Access::MAINTAINER) + end + end end diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb index 8884722254d..33291f5eebf 100644 --- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb @@ -111,6 +111,10 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do end end + it 'does not import ci config path' do + expect(@project.ci_config_path).to be_nil + end + it 'creates a valid pipeline note' do expect(Ci::Pipeline.find_by_sha('sha-notes').notes).not_to be_empty end diff --git a/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb b/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb index 2d595632772..fda4b94bd78 100644 --- a/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb @@ -91,72 +91,110 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do end end - context 'when matched throttle requires user information' do - context 'when user not found' do - let(:event) do - ActiveSupport::Notifications::Event.new( - event_name, Time.current, Time.current + 2.seconds, '1', request: double( - :request, - ip: '1.2.3.4', - request_method: 'GET', - fullpath: '/api/v4/internal/authorized_keys', - env: { - 'rack.attack.match_type' => match_type, - 'rack.attack.matched' => 'throttle_authenticated_api', - 'rack.attack.match_discriminator' => 'not_exist_user_id' - } + context 'matching user or deploy token authenticated information' do + context 'when matching for user' do + context 'when user not found' do + let(:event) do + ActiveSupport::Notifications::Event.new( + event_name, Time.current, Time.current + 2.seconds, '1', request: double( + :request, + ip: '1.2.3.4', + request_method: 'GET', + fullpath: '/api/v4/internal/authorized_keys', + env: { + 'rack.attack.match_type' => match_type, + 'rack.attack.matched' => 'throttle_authenticated_api', + 'rack.attack.match_discriminator' => "user:#{non_existing_record_id}" + } + ) ) - ) + end + + it 'logs request information and user id' do + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Rack_Attack', + env: match_type, + remote_ip: '1.2.3.4', + request_method: 'GET', + path: '/api/v4/internal/authorized_keys', + matched: 'throttle_authenticated_api', + user_id: non_existing_record_id + ) + ) + subscriber.send(match_type, event) + end end - it 'logs request information and user id' do - expect(Gitlab::AuthLogger).to receive(:error).with( - include( - message: 'Rack_Attack', - env: match_type, - remote_ip: '1.2.3.4', - request_method: 'GET', - path: '/api/v4/internal/authorized_keys', - matched: 'throttle_authenticated_api', - user_id: 'not_exist_user_id' + context 'when user found' do + let(:user) { create(:user) } + let(:event) do + ActiveSupport::Notifications::Event.new( + event_name, Time.current, Time.current + 2.seconds, '1', request: double( + :request, + ip: '1.2.3.4', + request_method: 'GET', + fullpath: '/api/v4/internal/authorized_keys', + env: { + 'rack.attack.match_type' => match_type, + 'rack.attack.matched' => 'throttle_authenticated_api', + 'rack.attack.match_discriminator' => "user:#{user.id}" + } + ) ) - ) - subscriber.send(match_type, event) + end + + it 'logs request information and user meta' do + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Rack_Attack', + env: match_type, + remote_ip: '1.2.3.4', + request_method: 'GET', + path: '/api/v4/internal/authorized_keys', + matched: 'throttle_authenticated_api', + user_id: user.id, + 'meta.user' => user.username + ) + ) + subscriber.send(match_type, event) + end end end - context 'when user found' do - let(:user) { create(:user) } - let(:event) do - ActiveSupport::Notifications::Event.new( - event_name, Time.current, Time.current + 2.seconds, '1', request: double( - :request, - ip: '1.2.3.4', - request_method: 'GET', - fullpath: '/api/v4/internal/authorized_keys', - env: { - 'rack.attack.match_type' => match_type, - 'rack.attack.matched' => 'throttle_authenticated_api', - 'rack.attack.match_discriminator' => user.id - } + context 'when matching for deploy token' do + context 'when deploy token found' do + let(:deploy_token) { create(:deploy_token) } + let(:event) do + ActiveSupport::Notifications::Event.new( + event_name, Time.current, Time.current + 2.seconds, '1', request: double( + :request, + ip: '1.2.3.4', + request_method: 'GET', + fullpath: '/api/v4/internal/authorized_keys', + env: { + 'rack.attack.match_type' => match_type, + 'rack.attack.matched' => 'throttle_authenticated_api', + 'rack.attack.match_discriminator' => "deploy_token:#{deploy_token.id}" + } + ) ) - ) - end - - it 'logs request information and user meta' do - expect(Gitlab::AuthLogger).to receive(:error).with( - include( - message: 'Rack_Attack', - env: match_type, - remote_ip: '1.2.3.4', - request_method: 'GET', - path: '/api/v4/internal/authorized_keys', - matched: 'throttle_authenticated_api', - user_id: user.id, - 'meta.user' => user.username + end + + it 'logs request information and user meta' do + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Rack_Attack', + env: match_type, + remote_ip: '1.2.3.4', + request_method: 'GET', + path: '/api/v4/internal/authorized_keys', + matched: 'throttle_authenticated_api', + deploy_token_id: deploy_token.id + ) ) - ) - subscriber.send(match_type, event) + subscriber.send(match_type, event) + end end end end diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index 54a0b282e99..5d5f6ebb817 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -990,4 +990,19 @@ RSpec.describe Gitlab::Regex do it { is_expected.not_to match('../../../../../1.2.3') } it { is_expected.not_to match('%2e%2e%2f1.2.3') } end + + describe '.sha256_regex' do + subject { described_class.sha256_regex } + + it { is_expected.to match('a' * 64) } + it { is_expected.to match('abcdefABCDEF1234567890abcdefABCDEF1234567890abcdefABCDEF12345678') } + it { is_expected.not_to match('a' * 63) } + it { is_expected.not_to match('a' * 65) } + it { is_expected.not_to match('a' * 63 + 'g') } + it { is_expected.not_to match('a' * 63 + '{') } + it { is_expected.not_to match('a' * 63 + '%') } + it { is_expected.not_to match('a' * 63 + '*') } + it { is_expected.not_to match('a' * 63 + '#') } + it { is_expected.not_to match('') } + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 90298f0e973..88d2516d690 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1048,7 +1048,27 @@ RSpec.describe Ci::Build do allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(1) end - it { is_expected.to match([a_hash_including(key: "key-1"), a_hash_including(key: "key2-1")]) } + it { is_expected.to match([a_hash_including(key: 'key-1-non_protected'), a_hash_including(key: 'key2-1-non_protected')]) } + + context 'when pipeline is on a protected ref' do + before do + allow(build.pipeline).to receive(:protected_ref?).and_return(true) + end + + it do + is_expected.to all(a_hash_including(key: a_string_matching(/-protected$/))) + end + end + + context 'when pipeline is not on a protected ref' do + before do + allow(build.pipeline).to receive(:protected_ref?).and_return(false) + end + + it do + is_expected.to all(a_hash_including(key: a_string_matching(/-non_protected$/))) + end + end end context 'when project has jobs_cache_index' do @@ -1056,7 +1076,7 @@ RSpec.describe Ci::Build do allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(1) end - it { is_expected.to be_an(Array).and all(include(key: "key-1")) } + it { is_expected.to be_an(Array).and all(include(key: a_string_matching(/^key-1-(?>protected|non_protected)/))) } end context 'when project does not have jobs_cache_index' do @@ -1064,7 +1084,9 @@ RSpec.describe Ci::Build do allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(nil) end - it { is_expected.to eq(options[:cache]) } + it do + is_expected.to eq(options[:cache].map { |entry| entry.merge(key: "#{entry[:key]}-non_protected") }) + end end end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 86ee159b97e..7e35ede64d1 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -618,6 +618,7 @@ RSpec.describe CommitStatus do 'rspec:windows 10000 20000' | 'rspec:windows' 'rspec:windows 0 : / 1' | 'rspec:windows' 'rspec:windows 0 : / 1 name' | 'rspec:windows 0 : / 1 name' + 'rspec [inception: [something, other thing], value]' | 'rspec' '0 1 name ruby' | '0 1 name ruby' '0 :/ 1 name ruby' | '0 :/ 1 name ruby' 'rspec: [aws]' | 'rspec' diff --git a/spec/models/integrations/every_integration_spec.rb b/spec/models/integrations/every_integration_spec.rb new file mode 100644 index 00000000000..1b9c6539f44 --- /dev/null +++ b/spec/models/integrations/every_integration_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Every integration' do + all_integration_names = Integration.available_integration_names + + all_integration_names.each do |integration_name| + describe integration_name do + let(:integration_class) { Integration.integration_name_to_model(integration_name) } + let(:integration) { integration_class.new } + let(:secret_name_pattern) { %r/token|key|password|passphrase|secret/.freeze } + + context 'secret fields', :aggregate_failures do + it "uses type: 'password' for all secret fields" do + integration.fields.each do |field| + next unless secret_name_pattern.match?(field[:name]) + + expect(field[:type]).to eq('password'), + "Field '#{field[:name]}' should use type 'password'" + end + end + + it 'defines non-empty titles and help texts for all secret fields' do + integration.fields.each do |field| + next unless field[:type] == 'password' + + expect(field[:non_empty_password_title]).to be_present, + "Field '#{field[:name]}' should define :non_empty_password_title" + expect(field[:non_empty_password_help]).to be_present, + "Field '#{field[:name]}' should define :non_empty_password_help" + end + end + end + end + end +end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 5af42cc67ea..1b43e1e572e 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -724,14 +724,15 @@ RSpec.describe Issue do describe '#participants' do context 'using a public project' do - let_it_be(:issue) { create(:issue, project: reusable_project) } + let_it_be(:public_project) { create(:project, :public) } + let_it_be(:issue) { create(:issue, project: public_project) } let!(:note1) do - create(:note_on_issue, noteable: issue, project: reusable_project, note: 'a') + create(:note_on_issue, noteable: issue, project: public_project, note: 'a') end let!(:note2) do - create(:note_on_issue, noteable: issue, project: reusable_project, note: 'b') + create(:note_on_issue, noteable: issue, project: public_project, note: 'b') end it 'includes the issue author' do @@ -801,20 +802,35 @@ RSpec.describe Issue do context 'without a user' do let(:user) { nil } - before do - project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PUBLIC) - end + context 'with issue available as public' do + before do + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PUBLIC) + end + + it 'returns true when the issue is publicly visible' do + expect(issue).to receive(:publicly_visible?).and_return(true) + + is_expected.to eq(true) + end - it 'returns true when the issue is publicly visible' do - expect(issue).to receive(:publicly_visible?).and_return(true) + it 'returns false when the issue is not publicly visible' do + expect(issue).to receive(:publicly_visible?).and_return(false) - is_expected.to eq(true) + is_expected.to eq(false) + end end - it 'returns false when the issue is not publicly visible' do - expect(issue).to receive(:publicly_visible?).and_return(false) + context 'with issues available only to team members in a public project' do + let(:public_project) { create(:project, :public) } + let(:issue) { build(:issue, project: public_project) } - is_expected.to eq(false) + before do + public_project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE) + end + + it 'returns false' do + is_expected.to be_falsey + end end end diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index fd453d8e5a9..0fe5c772261 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -23,6 +23,41 @@ RSpec.describe Packages::PackageFile, type: :model do describe 'validations' do it { is_expected.to validate_presence_of(:package) } + + context 'with pypi package' do + let_it_be(:package) { create(:pypi_package) } + + let(:package_file) { package.package_files.first } + let(:status) { :default } + let(:file_name) { 'foo' } + let(:file) { fixture_file_upload('spec/fixtures/dk.png') } + let(:params) { { file: file, file_name: file_name, status: status } } + + subject { package.package_files.create!(params) } + + context 'file_sha256' do + where(:sha256_value, :expected_success) do + 'a' * 64 | true + nil | true + 'a' * 63 | false + 'a' * 65 | false + 'a' * 63 + '%' | false + '' | false + end + + with_them do + let(:params) { super().merge({ file_sha256: sha256_value }) } + + it 'does not allow invalid sha256 characters' do + if expected_success + expect { subject }.not_to raise_error + else + expect { subject }.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: File sha256 is invalid") + end + end + end + end + end end context 'with package filenames' do diff --git a/spec/policies/ci/pipeline_schedule_policy_spec.rb b/spec/policies/ci/pipeline_schedule_policy_spec.rb index 1e36f455f6f..f2c99e0de95 100644 --- a/spec/policies/ci/pipeline_schedule_policy_spec.rb +++ b/spec/policies/ci/pipeline_schedule_policy_spec.rb @@ -84,11 +84,14 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models do project.add_maintainer(user) end - it 'includes abilities to do all operations on pipeline schedule' do + it 'allows for playing and destroying a pipeline schedule' do expect(policy).to be_allowed :play_pipeline_schedule - expect(policy).to be_allowed :update_pipeline_schedule expect(policy).to be_allowed :admin_pipeline_schedule end + + it 'does not allow for updating of an existing schedule' do + expect(policy).not_to be_allowed :update_pipeline_schedule + end end describe 'rules for non-owner of schedule' do diff --git a/spec/requests/api/ci/pipeline_schedules_spec.rb b/spec/requests/api/ci/pipeline_schedules_spec.rb index 4c8a356469d..0eeb89b4d8c 100644 --- a/spec/requests/api/ci/pipeline_schedules_spec.rb +++ b/spec/requests/api/ci/pipeline_schedules_spec.rb @@ -291,10 +291,32 @@ RSpec.describe API::Ci::PipelineSchedules do end context 'authenticated user with invalid permissions' do - it 'does not update pipeline_schedule' do - put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user) + context 'as a project maintainer' do + before do + project.add_maintainer(user) + end - expect(response).to have_gitlab_http_status(:not_found) + it 'does not update pipeline_schedule' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'as a project owner' do + it 'does not update pipeline_schedule' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", project.owner) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'with no special role' do + it 'does not update pipeline_schedule' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end end end @@ -312,16 +334,21 @@ RSpec.describe API::Ci::PipelineSchedules do create(:ci_pipeline_schedule, project: project, owner: developer) end - context 'authenticated user with valid permissions' do + let(:project_maintainer) do + create(:user).tap { |u| project.add_maintainer(u) } + end + + context 'as an authenticated user with valid permissions' do it 'updates owner' do - post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", developer) + expect { post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", project_maintainer) } + .to change { pipeline_schedule.reload.owner }.from(developer).to(project_maintainer) expect(response).to have_gitlab_http_status(:created) expect(response).to match_response_schema('pipeline_schedule') end end - context 'authenticated user with invalid permissions' do + context 'as an authenticated user with invalid permissions' do it 'does not update owner' do post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", user) @@ -329,13 +356,23 @@ RSpec.describe API::Ci::PipelineSchedules do end end - context 'unauthenticated user' do + context 'as an unauthenticated user' do it 'does not update owner' do post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership") expect(response).to have_gitlab_http_status(:unauthorized) end end + + context 'as the existing owner of the schedule' do + it 'rejects the request and leaves the schedule unchanged' do + expect do + post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", developer) + end.not_to change { pipeline_schedule.reload.owner } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end end describe 'DELETE /projects/:id/pipeline_schedules/:pipeline_schedule_id' do diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index 68f7581bf06..249def77b7a 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -191,7 +191,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end let(:expected_cache) do - [{ 'key' => 'cache_key', + [{ 'key' => a_string_matching(/^cache_key-(?>protected|non_protected)$/), 'untracked' => false, 'paths' => ['vendor/*'], 'policy' => 'pull-push', @@ -225,7 +225,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do 'alias' => nil, 'command' => nil, 'ports' => [], 'variables' => [{ 'key' => 'MYSQL_ROOT_PASSWORD', 'value' => 'root123.' }] }]) expect(json_response['steps']).to eq(expected_steps) expect(json_response['artifacts']).to eq(expected_artifacts) - expect(json_response['cache']).to eq(expected_cache) + expect(json_response['cache']).to match(expected_cache) expect(json_response['variables']).to include(*expected_variables) expect(json_response['features']).to match(expected_features) end diff --git a/spec/requests/api/markdown_spec.rb b/spec/requests/api/markdown_spec.rb index 0488bce4663..47e1f007daa 100644 --- a/spec/requests/api/markdown_spec.rb +++ b/spec/requests/api/markdown_spec.rb @@ -156,6 +156,46 @@ RSpec.describe API::Markdown do end end end + + context 'with a public project and issues only for team members' do + let(:public_project) do + create(:project, :public).tap do |project| + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE) + end + end + + let(:issue) { create(:issue, project: public_project, title: 'Team only title') } + let(:text) { "#{issue.to_reference}" } + let(:params) { { text: text, gfm: true, project: public_project.full_path } } + + shared_examples 'user without proper access' do + it 'does not render the title' do + expect(response).to have_gitlab_http_status(:created) + expect(json_response["html"]).not_to include('Team only title') + end + end + + context 'when not logged in' do + let(:user) { } + + it_behaves_like 'user without proper access' + end + + context 'when logged in as user without access' do + let(:user) { create(:user) } + + it_behaves_like 'user without proper access' + end + + context 'when logged in as author' do + let(:user) { issue.author } + + it 'renders the title or link' do + expect(response).to have_gitlab_http_status(:created) + expect(json_response["html"]).to include('Team only title') + end + end + end end end end diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index fcd2d56e655..883cdd2a4f1 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -136,7 +136,7 @@ RSpec.describe API::PypiPackages do let(:url) { "/projects/#{project.id}/packages/pypi" } let(:headers) { {} } let(:requires_python) { '>=3.7' } - let(:base_params) { { requires_python: requires_python, version: '1.0.0', name: 'sample-project', sha256_digest: '123' } } + let(:base_params) { { requires_python: requires_python, version: '1.0.0', name: 'sample-project', sha256_digest: '1' * 64 } } let(:params) { base_params.merge(content: temp_file(file_name)) } let(:send_rewritten_field) { true } let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: user } } @@ -213,6 +213,19 @@ RSpec.describe API::PypiPackages do it_behaves_like 'returning response status', :bad_request end + context 'with an invalid sha256' do + let(:token) { personal_access_token.token } + let(:user_headers) { basic_auth_header(user.username, token) } + let(:headers) { user_headers.merge(workhorse_headers) } + + before do + params[:sha256_digest] = 'a' * 63 + '%' + project.add_developer(user) + end + + it_behaves_like 'returning response status', :bad_request + end + it_behaves_like 'deploy token for package uploads' it_behaves_like 'job token for package uploads' diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index f2126e3cf9c..115f78a5600 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -93,28 +93,28 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the OAuth headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in basic auth' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, basic_auth_headers(user, token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, basic_auth_headers(other_user, other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with a read_api scope' do @@ -127,14 +127,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the OAuth headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end end @@ -155,14 +155,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, oauth_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, oauth_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with a read_api scope' do @@ -171,7 +171,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(read_token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_read_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end @@ -184,7 +184,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [rss_url(user), params: nil] } let(:other_user_request_args) { [rss_url(other_user), params: nil] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end @@ -288,14 +288,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end @@ -444,14 +444,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'precedence over authenticated api throttle' do @@ -512,6 +512,16 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac end end end + + context 'authenticated via deploy token headers' do + let(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true, projects: [project]) } + let(:other_deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } + + let(:request_args) { [api(api_partial_url), { headers: deploy_token_headers(deploy_token) }] } + let(:other_user_request_args) { [api(api_partial_url), { headers: deploy_token_headers(other_deploy_token) }] } + + it_behaves_like 'rate-limited deploy-token-authenticated requests' + end end end @@ -558,7 +568,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac end end - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'getting a blob' do @@ -568,7 +578,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:path) { "/v2/#{blob.group.path}/dependency_proxy/containers/alpine/blobs/sha256:a0d0a0d46f8b52473982a3c466318f479767577551a53ffc9074c9fa7035982e" } let(:other_path) { "/v2/#{other_blob.group.path}/dependency_proxy/containers/alpine/blobs/sha256:a0d0a0d46f8b52473982a3c466318f479767577551a53ffc9074c9fa7035982e" } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end @@ -598,7 +608,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [git_lfs_url, { headers: basic_auth_headers(user, token) }] } let(:other_user_request_args) { [git_lfs_url, { headers: basic_auth_headers(other_user, other_user_token) }] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'precedence over authenticated web throttle' do @@ -786,14 +796,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'precedence over authenticated api throttle' do @@ -993,14 +1003,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(path, personal_access_token: token), {}] } let(:other_user_request_args) { [api(path, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(path, personal_access_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(path, personal_access_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'precedence over authenticated api throttle' do diff --git a/spec/services/ci/create_pipeline_service/cache_spec.rb b/spec/services/ci/create_pipeline_service/cache_spec.rb index ca85a8cce64..fe777bc50d9 100644 --- a/spec/services/ci/create_pipeline_service/cache_spec.rb +++ b/spec/services/ci/create_pipeline_service/cache_spec.rb @@ -33,7 +33,7 @@ RSpec.describe Ci::CreatePipelineService do it 'uses the provided key' do expected = { - key: 'a-key', + key: a_string_matching(/^a-key-(?>protected|non_protected)$/), paths: ['logs/', 'binaries/'], policy: 'pull-push', untracked: true, diff --git a/spec/services/packages/pypi/create_package_service_spec.rb b/spec/services/packages/pypi/create_package_service_spec.rb index 3d0c10724d4..187c7ea680c 100644 --- a/spec/services/packages/pypi/create_package_service_spec.rb +++ b/spec/services/packages/pypi/create_package_service_spec.rb @@ -7,6 +7,9 @@ RSpec.describe Packages::Pypi::CreatePackageService do let_it_be(:project) { create(:project) } let_it_be(:user) { create(:user) } + let(:sha256) { '1' * 64 } + let(:md5) { '567' } + let(:requires_python) { '>=2.7' } let(:params) do { @@ -14,8 +17,8 @@ RSpec.describe Packages::Pypi::CreatePackageService do version: '1.0', content: temp_file('foo.tgz'), requires_python: requires_python, - sha256_digest: '123', - md5_digest: '567' + sha256_digest: sha256, + md5_digest: md5 } end @@ -34,8 +37,8 @@ RSpec.describe Packages::Pypi::CreatePackageService do expect(created_package.pypi_metadatum.required_python).to eq '>=2.7' expect(created_package.package_files.size).to eq 1 expect(created_package.package_files.first.file_name).to eq 'foo.tgz' - expect(created_package.package_files.first.file_sha256).to eq '123' - expect(created_package.package_files.first.file_md5).to eq '567' + expect(created_package.package_files.first.file_sha256).to eq sha256 + expect(created_package.package_files.first.file_md5).to eq md5 end end @@ -62,8 +65,8 @@ RSpec.describe Packages::Pypi::CreatePackageService do context 'with an existing file' do before do params[:content] = temp_file('foo.tgz') - params[:sha256_digest] = 'abc' - params[:md5_digest] = 'def' + params[:sha256_digest] = sha256 + params[:md5_digest] = md5 end it 'throws an error' do @@ -89,8 +92,8 @@ RSpec.describe Packages::Pypi::CreatePackageService do expect(created_package.pypi_metadatum.required_python).to eq '>=2.7' expect(created_package.package_files.size).to eq 1 expect(created_package.package_files.first.file_name).to eq 'foo.tgz' - expect(created_package.package_files.first.file_sha256).to eq 'abc' - expect(created_package.package_files.first.file_md5).to eq 'def' + expect(created_package.package_files.first.file_sha256).to eq sha256 + expect(created_package.package_files.first.file_md5).to eq md5 end end end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 7103cb0b66a..6c25b230855 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -391,6 +391,7 @@ RSpec.describe TodoService do let!(:second_todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) } let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee]) } let(:note) { create(:note, project: project, noteable: issue, author: john_doe, note: mentions) } + let(:confidential_note) { create(:note, :confidential, project: project, noteable: issue, author: john_doe, note: mentions) } let(:addressed_note) { create(:note, project: project, noteable: issue, author: john_doe, note: directly_addressed) } let(:note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: mentions) } let(:addressed_note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: directly_addressed) } @@ -468,6 +469,17 @@ RSpec.describe TodoService do should_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue) end + it 'does not create todo if user can not read confidential note' do + service.new_note(confidential_note, john_doe) + + should_not_create_todo(user: non_member, target: issue, author: john_doe, action: Todo::MENTIONED, note: confidential_note) + should_not_create_todo(user: guest, target: issue, author: john_doe, action: Todo::MENTIONED, note: confidential_note) + should_create_todo(user: member, target: issue, author: john_doe, action: Todo::MENTIONED, note: confidential_note) + should_create_todo(user: author, target: issue, author: john_doe, action: Todo::MENTIONED, note: confidential_note) + should_create_todo(user: assignee, target: issue, author: john_doe, action: Todo::MENTIONED, note: confidential_note) + should_create_todo(user: john_doe, target: issue, author: john_doe, action: Todo::MENTIONED, note: confidential_note) + end + context 'commits' do let(:base_commit_todo_attrs) { { target_id: nil, target_type: 'Commit', author: john_doe } } diff --git a/spec/support/helpers/packages_manager_api_spec_helper.rb b/spec/support/helpers/packages_manager_api_spec_helper.rb index 34e92c0595c..1c9fce183e9 100644 --- a/spec/support/helpers/packages_manager_api_spec_helper.rb +++ b/spec/support/helpers/packages_manager_api_spec_helper.rb @@ -3,7 +3,7 @@ module PackagesManagerApiSpecHelpers def build_jwt(personal_access_token, secret: jwt_secret, user_id: nil) JSONWebToken::HMACToken.new(secret).tap do |jwt| - jwt['access_token'] = personal_access_token.id + jwt['access_token'] = personal_access_token.token jwt['user_id'] = user_id || personal_access_token.user_id end end diff --git a/spec/support/helpers/rack_attack_spec_helpers.rb b/spec/support/helpers/rack_attack_spec_helpers.rb index c82a87dc58e..6c06781df03 100644 --- a/spec/support/helpers/rack_attack_spec_helpers.rb +++ b/spec/support/helpers/rack_attack_spec_helpers.rb @@ -10,11 +10,11 @@ module RackAttackSpecHelpers end def private_token_headers(user) - { 'HTTP_PRIVATE_TOKEN' => user.private_token } + { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => user.private_token } end def personal_access_token_headers(personal_access_token) - { 'HTTP_PRIVATE_TOKEN' => personal_access_token.token } + { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => personal_access_token.token } end def oauth_token_headers(oauth_access_token) @@ -26,6 +26,10 @@ module RackAttackSpecHelpers { 'AUTHORIZATION' => "Basic #{encoded_login}" } end + def deploy_token_headers(deploy_token) + basic_auth_headers(deploy_token, deploy_token) + end + def expect_rejection(name = nil, &block) yield diff --git a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb index b30c4186f0d..891e444df9e 100644 --- a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb @@ -62,15 +62,8 @@ RSpec.shared_examples 'conan authenticate endpoint' do end end - it 'responds with 401 Unauthorized when an invalid access token ID is provided' do - jwt = build_jwt(double(id: 12345), user_id: personal_access_token.user_id) - get api(url), headers: build_token_auth_header(jwt.encoded) - - expect(response).to have_gitlab_http_status(:unauthorized) - end - - it 'responds with 401 Unauthorized when invalid user is provided' do - jwt = build_jwt(personal_access_token, user_id: 12345) + it 'responds with 401 Unauthorized when an invalid access token is provided' do + jwt = build_jwt(double(token: 12345), user_id: user.id) get api(url), headers: build_token_auth_header(jwt.encoded) expect(response).to have_gitlab_http_status(:unauthorized) @@ -102,7 +95,7 @@ RSpec.shared_examples 'conan authenticate endpoint' do payload = JSONWebToken::HMACToken.decode( response.body, jwt_secret).first - expect(payload['access_token']).to eq(personal_access_token.id) + expect(payload['access_token']).to eq(personal_access_token.token) expect(payload['user_id']).to eq(personal_access_token.user_id) duration = payload['exp'] - payload['iat'] diff --git a/spec/support/shared_examples/requests/rack_attack_shared_examples.rb b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb index c6c6c44dce8..68cb91d7414 100644 --- a/spec/support/shared_examples/requests/rack_attack_shared_examples.rb +++ b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb @@ -8,7 +8,50 @@ # * requests_per_period # * period_in_seconds # * period -RSpec.shared_examples 'rate-limited token-authenticated requests' do +RSpec.shared_examples 'rate-limited user based token-authenticated requests' do + context 'when the throttle is enabled' do + before do + settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true + stub_application_setting(settings_to_set) + end + + it 'does not reject requests if the user is in the allowlist' do + stub_env('GITLAB_THROTTLE_USER_ALLOWLIST', user.id.to_s) + Gitlab::RackAttack.configure_user_allowlist + + expect(Gitlab::Instrumentation::Throttle).to receive(:safelist=).with('throttle_user_allowlist').at_least(:once) + + (requests_per_period + 1).times do + make_request(request_args) + expect(response).not_to have_gitlab_http_status(:too_many_requests) + end + + stub_env('GITLAB_THROTTLE_USER_ALLOWLIST', nil) + Gitlab::RackAttack.configure_user_allowlist + end + end + + include_examples 'rate-limited token requests' do + let(:log_data) do + { + user_id: user.id, + 'meta.user' => user.username + } + end + end +end + +RSpec.shared_examples 'rate-limited deploy-token-authenticated requests' do + include_examples 'rate-limited token requests' do + let(:log_data) do + { + deploy_token_id: deploy_token.id + } + end + end +end + +RSpec.shared_examples 'rate-limited token requests' do let(:throttle_types) do { "throttle_protected_paths" => "throttle_authenticated_protected_paths_api", @@ -51,18 +94,6 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do expect_rejection { make_request(request_args) } end - it 'does not reject requests if the user is in the allowlist' do - stub_env('GITLAB_THROTTLE_USER_ALLOWLIST', user.id.to_s) - Gitlab::RackAttack.configure_user_allowlist - - expect(Gitlab::Instrumentation::Throttle).to receive(:safelist=).with('throttle_user_allowlist').at_least(:once) - - (requests_per_period + 1).times do - make_request(request_args) - expect(response).not_to have_gitlab_http_status(:too_many_requests) - end - end - it 'allows requests after throttling and then waiting for the next period' do requests_per_period.times do make_request(request_args) @@ -81,7 +112,7 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do end end - it 'counts requests from different users separately, even from the same IP' do + it 'counts requests from different requesters separately, even from the same IP' do requests_per_period.times do make_request(request_args) expect(response).not_to have_gitlab_http_status(:too_many_requests) @@ -92,7 +123,7 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do expect(response).not_to have_gitlab_http_status(:too_many_requests) end - it 'counts all requests from the same user, even via different IPs' do + it 'counts all requests from the same requesters, even via different IPs' do requests_per_period.times do make_request(request_args) expect(response).not_to have_gitlab_http_status(:too_many_requests) @@ -122,10 +153,8 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do remote_ip: '127.0.0.1', request_method: request_method, path: request_args.first, - user_id: user.id, - 'meta.user' => user.username, matched: throttle_types[throttle_setting_prefix] - }) + }.merge(log_data)) expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once |