diff options
24 files changed, 405 insertions, 159 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 c2b8b457049..484cceb9129 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -640,7 +640,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 b49e04f481c..3d56c563ec8 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: -> { !pending_destruction? && 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 33783d31355..27692fe76f0 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -105,7 +105,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/doc/ci/pipelines/schedules.md b/doc/ci/pipelines/schedules.md index 8813f3e1d59..8ab80e3798a 100644 --- a/doc/ci/pipelines/schedules.md +++ b/doc/ci/pipelines/schedules.md @@ -39,6 +39,20 @@ To add a pipeline schedule: These variables are available only when the scheduled pipeline runs, and not in any other pipeline run. +## 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](#take-ownership) +of the schedule. + ## Run manually To trigger a pipeline schedule manually, so that it runs immediately instead of 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 86f36d4fc00..d4f51beb2e5 100644 --- a/lib/api/pypi_packages.rb +++ b/lib/api/pypi_packages.rb @@ -174,7 +174,7 @@ module API requires :version, type: String optional :requires_python, 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/regex.rb b/lib/gitlab/regex.rb index c9202c6c54c..205106afddb 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 63867a7e900..7cb14feabd2 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('[data-testid="schedule-target-ref"]') do + expect(first('.gl-new-dropdown-button-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('[data-testid="schedule-target-ref"]') do + expect(first('.gl-new-dropdown-button-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('[data-testid="schedule-target-ref"]') do - expect(first('.gl-new-dropdown-button-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('[data-testid="schedule-target-ref"]') do - expect(first('.gl-new-dropdown-button-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/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/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index f3e8c440fba..b4c1f3b689b 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -1005,4 +1005,19 @@ RSpec.describe Gitlab::Regex do it { is_expected.not_to match('.xt.est_') } it { is_expected.not_to match('0test1') } 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 fe09dadd0db..bd75d95080f 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -742,14 +742,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 @@ -819,20 +820,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 eq(false) + end end end diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index f6af8f6a951..82f5b44f38f 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -29,19 +29,48 @@ RSpec.describe Packages::PackageFile, type: :model do 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!(file: file, file_name: package_file.file_name, status: status) } + subject { package.package_files.create!(params) } - it 'can not save a duplicated file' do - expect { subject }.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: File name has already been taken") + context 'file_name' do + let(:file_name) { package_file.file_name } + + it 'can not save a duplicated file' do + expect { subject }.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: File name has already been taken") + end + + context 'with a pending destruction package duplicated file' do + let(:status) { :pending_destruction } + + it 'can save it' do + expect { subject }.to change { package.package_files.count }.from(1).to(2) + end + end end - context 'with a pending destruction package duplicated file' do - let(:status) { :pending_destruction } + 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 'can save it' do - expect { subject }.to change { package.package_files.count }.from(1).to(2) + 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 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..5fb94976c5f 100644 --- a/spec/requests/api/ci/pipeline_schedules_spec.rb +++ b/spec/requests/api/ci/pipeline_schedules_spec.rb @@ -291,10 +291,36 @@ 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 + before do + project.add_owner(user) + end + + 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 '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 +338,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 +360,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 078db4f1509..8fa5f409298 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 } } @@ -221,6 +221,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 f84a77f80f7..354ac92b99a 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, :aggregate_failures 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, :aggregate_failures 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, :aggregate_failures 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 @@ -74,8 +77,8 @@ RSpec.describe Packages::Pypi::CreatePackageService, :aggregate_failures 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 @@ -101,8 +104,8 @@ RSpec.describe Packages::Pypi::CreatePackageService, :aggregate_failures 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/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 82c34f0d6ad..135fa4cf5a4 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'] |