diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-04-29 08:21:56 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-04-29 08:21:56 +0000 |
commit | a0fde4af8e107fbd9acb498e4bc96eb18a0f9d29 (patch) | |
tree | 2d62a8c788327a49594455dd3fe1071c9e76ba92 | |
parent | 9bd3bea4c606a44c3c83b1818a030d58b0a3a330 (diff) | |
download | gitlab-ce-a0fde4af8e107fbd9acb498e4bc96eb18a0f9d29.tar.gz |
Add latest changes from gitlab-org/security/gitlab@14-8-stable-ee
32 files changed, 443 insertions, 156 deletions
diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb index ac94cc001dd..f6171403667 100644 --- a/app/controllers/projects/pipeline_schedules_controller.rb +++ b/app/controllers/projects/pipeline_schedules_controller.rb @@ -7,7 +7,8 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController before_action :authorize_play_pipeline_schedule!, only: [:play] before_action :authorize_read_pipeline_schedule! before_action :authorize_create_pipeline_schedule!, only: [:new, :create] - before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create, :play] + before_action :authorize_update_pipeline_schedule!, only: [:edit, :update] + before_action :authorize_take_ownership_pipeline_schedule!, only: [:take_ownership] before_action :authorize_admin_pipeline_schedule!, only: [:destroy] feature_category :continuous_integration @@ -108,6 +109,10 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController return access_denied! unless can?(current_user, :update_pipeline_schedule, schedule) end + def authorize_take_ownership_pipeline_schedule! + return access_denied! unless can?(current_user, :take_ownership_pipeline_schedule, schedule) + end + def authorize_admin_pipeline_schedule! return access_denied! unless can?(current_user, :admin_pipeline_schedule, schedule) end diff --git a/app/models/issue.rb b/app/models/issue.rb index 68ea6cb3abc..6a1f26a68b3 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -630,7 +630,8 @@ class Issue < ApplicationRecord # Returns `true` if this Issue is visible to everybody. def publicly_visible? - project.public? && !confidential? && !hidden? && !::Gitlab::ExternalAuthorization.enabled? + project.public? && project.feature_available?(:issues, nil) && + !confidential? && !hidden? && !::Gitlab::ExternalAuthorization.enabled? end def expire_etag_cache diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb index fc7c348dfdb..bd250caca72 100644 --- a/app/models/packages/package_file.rb +++ b/app/models/packages/package_file.rb @@ -35,6 +35,7 @@ class Packages::PackageFile < ApplicationRecord validates :file_name, presence: true validates :file_name, uniqueness: { scope: :package }, if: -> { package&.pypi? } + validates :file_sha256, format: { with: Gitlab::Regex.sha256_regex }, if: -> { package&.pypi? }, allow_nil: true scope :recent, -> { order(id: :desc) } scope :limit_recent, ->(limit) { recent.limit(limit) } diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index 0d3e50837ab..30bca435a25 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -104,7 +104,7 @@ class ProjectFeature < ApplicationRecord # that the user has access to the feature. It's important to use this scope with others # that checks project authorizations first (e.g. `filter_by_feature_visibility`). # - # This method uses an optimised version of `with_feature_access_level` for + # This method uses an optimized version of `with_feature_access_level` for # logged in users to more efficiently get private projects with the given # feature. def self.with_feature_available_for_user(feature, user) diff --git a/app/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb index 2ef5ffd6a5a..3a674bfef92 100644 --- a/app/policies/ci/pipeline_schedule_policy.rb +++ b/app/policies/ci/pipeline_schedule_policy.rb @@ -15,11 +15,14 @@ module Ci rule { can?(:create_pipeline) }.enable :play_pipeline_schedule rule { can?(:admin_pipeline) | (can?(:update_build) & owner_of_schedule) }.policy do - enable :update_pipeline_schedule enable :admin_pipeline_schedule enable :read_pipeline_schedule_variables end + rule { admin | (owner_of_schedule & can?(:update_build)) }.policy do + enable :update_pipeline_schedule + end + rule { can?(:admin_pipeline_schedule) & ~owner_of_schedule }.policy do enable :take_ownership_pipeline_schedule end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 091f441831a..6c6ef6a958e 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -352,8 +352,6 @@ class TodoService end def reject_users_without_access(users, parent, target) - target = target.noteable if target.is_a?(Note) - if target.respond_to?(:to_ability_name) select_users(users, :"read_#{target.to_ability_name}", target) else diff --git a/doc/ci/pipelines/schedules.md b/doc/ci/pipelines/schedules.md index fe9db3306cd..0840184ca03 100644 --- a/doc/ci/pipelines/schedules.md +++ b/doc/ci/pipelines/schedules.md @@ -53,6 +53,20 @@ is installed on. ![Schedules list](img/pipeline_schedules_list.png) +## Edit a pipeline schedule + +> Introduced in GitLab 14.8, only a pipeline schedule owner can edit the schedule. + +The owner of a pipeline schedule can edit it: + +1. On the top bar, select **Menu > Projects** and find your project. +1. In the left sidebar, select **CI/CD > Schedules**. +1. Next to the schedule, select **Edit** (**{pencil}**) and fill in the form. + +The user must have the Developer role or above for the project. If the user is +not the owner of the schedule, they must first take ownership. +of the schedule. + ### Using variables You can pass any number of arbitrary variables. They are available in diff --git a/lib/api/ci/pipeline_schedules.rb b/lib/api/ci/pipeline_schedules.rb index 8a9ba2cbe0f..6030fe86f00 100644 --- a/lib/api/ci/pipeline_schedules.rb +++ b/lib/api/ci/pipeline_schedules.rb @@ -93,7 +93,7 @@ module API requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' end post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do - authorize! :update_pipeline_schedule, pipeline_schedule + authorize! :take_ownership_pipeline_schedule, pipeline_schedule if pipeline_schedule.own!(current_user) present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails diff --git a/lib/api/helpers/packages/conan/api_helpers.rb b/lib/api/helpers/packages/conan/api_helpers.rb index e92547890e8..994d3c4c473 100644 --- a/lib/api/helpers/packages/conan/api_helpers.rb +++ b/lib/api/helpers/packages/conan/api_helpers.rb @@ -153,7 +153,7 @@ module API def token strong_memoize(:token) do token = nil - token = ::Gitlab::ConanToken.from_personal_access_token(access_token) if access_token + token = ::Gitlab::ConanToken.from_personal_access_token(find_personal_access_token.user_id, access_token_from_request) if find_personal_access_token token = ::Gitlab::ConanToken.from_deploy_token(deploy_token_from_request) if deploy_token_from_request token = ::Gitlab::ConanToken.from_job(find_job_from_token) if find_job_from_token token @@ -224,9 +224,27 @@ module API forbidden! end + # We override this method from auth_finders because we need to + # extract the token from the Conan JWT which is specific to the Conan API def find_personal_access_token - find_personal_access_token_from_conan_jwt || - find_personal_access_token_from_http_basic_auth + strong_memoize(:find_personal_access_token) do + PersonalAccessToken.find_by_token(access_token_from_request) + end + end + + def access_token_from_request + strong_memoize(:access_token_from_request) do + find_personal_access_token_from_conan_jwt || + find_password_from_basic_auth + end + end + + def find_password_from_basic_auth + return unless route_authentication_setting[:basic_auth_personal_access_token] + return unless has_basic_credentials?(current_request) + + _username, password = user_name_and_password(current_request) + password end def find_user_from_job_token @@ -256,7 +274,7 @@ module API return unless token - PersonalAccessToken.find_by_id_and_user_id(token.access_token_id, token.user_id) + token.access_token_id end def find_deploy_token_from_conan_jwt diff --git a/lib/api/pypi_packages.rb b/lib/api/pypi_packages.rb index 706c0702fce..6f308b2aca0 100644 --- a/lib/api/pypi_packages.rb +++ b/lib/api/pypi_packages.rb @@ -174,7 +174,7 @@ module API requires :name, type: String requires :version, type: String optional :md5_digest, type: String - optional :sha256_digest, type: String + optional :sha256_digest, type: String, regexp: Gitlab::Regex.sha256_regex end route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth diff --git a/lib/gitlab/conan_token.rb b/lib/gitlab/conan_token.rb index d0560807f45..87a085461bc 100644 --- a/lib/gitlab/conan_token.rb +++ b/lib/gitlab/conan_token.rb @@ -13,8 +13,8 @@ module Gitlab attr_reader :access_token_id, :user_id class << self - def from_personal_access_token(access_token) - new(access_token_id: access_token.id, user_id: access_token.user_id) + def from_personal_access_token(user_id, token) + new(access_token_id: token, user_id: user_id) end def from_job(job) diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index 059f6bd42e3..6f64bf741a5 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -638,7 +638,6 @@ included_attributes: - :build_allow_git_fetch - :build_coverage_regex - :build_timeout - - :ci_config_path - :delete_error - :description - :disable_overriding_approvers_per_merge_request diff --git a/lib/gitlab/import_export/project/relation_factory.rb b/lib/gitlab/import_export/project/relation_factory.rb index c391f86b47b..8110720fb46 100644 --- a/lib/gitlab/import_export/project/relation_factory.rb +++ b/lib/gitlab/import_export/project/relation_factory.rb @@ -87,6 +87,8 @@ module Gitlab when *BUILD_MODELS then setup_build when :issues then setup_issue when :'Ci::PipelineSchedule' then setup_pipeline_schedule + when :'ProtectedBranch::MergeAccessLevel' then setup_protected_branch_access_level + when :'ProtectedBranch::PushAccessLevel' then setup_protected_branch_access_level end update_project_references @@ -152,6 +154,10 @@ module Gitlab @relation_hash['active'] = false end + def setup_protected_branch_access_level + @relation_hash['access_level'] = Gitlab::Access::MAINTAINER + end + def compute_relative_position return unless max_relative_position diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index a6491d23bf5..7b3590e8707 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -237,6 +237,10 @@ module Gitlab generic_package_name_regex end + def sha256_regex + @sha256_regex ||= /\A[0-9a-f]{64}\z/i.freeze + end + private def conan_name_regex 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/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/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/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/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/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/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/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'] |