diff --git a/CHANGELOG.md b/CHANGELOG.md index e520b162a2..ddb9a4fb0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,19 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 13.6.7 (2021-02-11) + +### Security (7 changes) + +- Cancel running and pending jobs when a project is deleted. !1220 +- Updates authorization for linting API. +- Prevent exposure of confidential issue titles in file browser. +- Check user access on API merge request read actions. +- Prevent Denial of Service Attack on gitlab-shell. +- Limit daily invitations to groups and projects. +- Prevent Server-side Request Forgery for Prometheus when secured by Google IAP. + + ## 13.6.6 (2021-02-01) ### Security (5 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 4c39112331..51067b9c1d 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -13.6.6 \ No newline at end of file +13.6.7 \ No newline at end of file diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index 0b704f2a43..4a60601836 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -13.13.0 +13.13.1 diff --git a/VERSION b/VERSION index 4c39112331..51067b9c1d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -13.6.6 \ No newline at end of file +13.6.7 \ No newline at end of file diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 8707d635e0..3778add83b 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -15,6 +15,7 @@ module Ci include ShaAttribute include FromUnion include UpdatedAtFilterable + include EachBatch PROJECT_ROUTE_AND_NAMESPACE_ROUTE = { project: [:project_feature, :route, { namespace: :route }] diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index ee9c2501bf..f9de9dec07 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -55,6 +55,7 @@ class CommitStatus < ApplicationRecord scope :for_ids, -> (ids) { where(id: ids) } scope :for_ref, -> (ref) { where(ref: ref) } scope :by_name, -> (name) { where(name: name) } + scope :in_pipelines, ->(pipelines) { where(pipeline: pipelines) } scope :for_project_paths, -> (paths) do where(project: Project.where_full_path_in(Array(paths))) diff --git a/app/models/member.rb b/app/models/member.rb index 687830f526..3000c13f4b 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -45,6 +45,19 @@ class Member < ApplicationRecord }, if: :project_bot? + scope :in_hierarchy, ->(source) do + groups = source.root_ancestor.self_and_descendants + group_members = Member.default_scoped.where(source: groups) + + projects = source.root_ancestor.all_projects + project_members = Member.default_scoped.where(source: projects) + + Member.default_scoped.from_union([ + group_members, + project_members + ]).merge(self) + end + # This scope encapsulates (most of) the conditions a row in the member table # must satisfy if it is a valid permission. Of particular note: # @@ -77,12 +90,18 @@ class Member < ApplicationRecord scope :invite, -> { where.not(invite_token: nil) } scope :non_invite, -> { where(invite_token: nil) } + scope :request, -> { where.not(requested_at: nil) } scope :non_request, -> { where(requested_at: nil) } scope :not_accepted_invitations, -> { invite.where(invite_accepted_at: nil) } scope :not_accepted_invitations_by_user, -> (user) { not_accepted_invitations.where(created_by: user) } scope :not_expired, -> (today = Date.current) { where(arel_table[:expires_at].gt(today).or(arel_table[:expires_at].eq(nil))) } + + scope :created_today, -> do + now = Date.current + where(created_at: now.beginning_of_day..now.end_of_day) + end scope :last_ten_days_excluding_today, -> (today = Date.current) { where(created_at: (today - 10).beginning_of_day..(today - 1).end_of_day) } scope :has_access, -> { active.where('access_level > 0') } diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index d0e62a1afb..ab04322783 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -183,7 +183,17 @@ class PrometheusService < MonitoringService manual_configuration? && google_iap_audience_client_id.present? && google_iap_service_account_json.present? end + def clean_google_iap_service_account + return unless google_iap_service_account_json + + google_iap_service_account_json + .then { |json| Gitlab::Json.parse(json) } + .except('token_credential_uri') + end + def iap_client - @iap_client ||= Google::Auth::Credentials.new(Gitlab::Json.parse(google_iap_service_account_json), target_audience: google_iap_audience_client_id).client + @iap_client ||= Google::Auth::Credentials + .new(clean_google_iap_service_account, target_audience: google_iap_audience_client_id) + .client end end diff --git a/app/services/ci/abort_project_pipelines_service.rb b/app/services/ci/abort_project_pipelines_service.rb new file mode 100644 index 0000000000..d4f228e37a --- /dev/null +++ b/app/services/ci/abort_project_pipelines_service.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Ci + class AbortProjectPipelinesService + # Danger: Cancels in bulk without callbacks + # Only for pipeline abandonment scenarios (current example: project delete) + def execute(project) + return unless Feature.enabled?(:abort_deleted_project_pipelines, default_enabled: true) + + pipelines = project.all_pipelines.cancelable + bulk_abort!(pipelines, status: :canceled) + + ServiceResponse.success(message: 'Pipelines canceled') + end + + private + + def bulk_abort!(pipelines, status:) + pipelines.each_batch do |pipeline_batch| + CommitStatus.in_pipelines(pipeline_batch).in_batches.update_all(status: status) # rubocop: disable Cop/InBatches + pipeline_batch.update_all(status: status) + end + end + end +end diff --git a/app/services/ci/cancel_user_pipelines_service.rb b/app/services/ci/cancel_user_pipelines_service.rb index 3a8b5e9108..3d3a8032e8 100644 --- a/app/services/ci/cancel_user_pipelines_service.rb +++ b/app/services/ci/cancel_user_pipelines_service.rb @@ -6,6 +6,7 @@ module Ci # This is a bug with CodeReuse/ActiveRecord cop # https://gitlab.com/gitlab-org/gitlab/issues/32332 def execute(user) + # TODO: fix N+1 queries https://gitlab.com/gitlab-org/gitlab/-/issues/300685 user.pipelines.cancelable.find_each(&:cancel_running) ServiceResponse.success(message: 'Pipeline canceled') diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index 088e6f031c..8dbf1ab1c9 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -2,12 +2,12 @@ module Members class CreateService < Members::BaseService + include Gitlab::Utils::StrongMemoize + DEFAULT_LIMIT = 100 def execute(source) - return error(s_('AddMember|No users specified.')) if params[:user_ids].blank? - - user_ids = params[:user_ids].split(',').uniq.flatten + return error(s_('AddMember|No users specified.')) if user_ids.blank? return error(s_("AddMember|Too many users specified (limit is %{user_limit})") % { user_limit: user_limit }) if user_limit && user_ids.size > user_limit @@ -45,6 +45,13 @@ module Members private + def user_ids + strong_memoize(:user_ids) do + ids = params[:user_ids] || '' + ids.split(',').uniq.flatten + end + end + def user_limit limit = params.fetch(:limit, DEFAULT_LIMIT) diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index bec7565753..c150162530 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -21,11 +21,14 @@ module Projects def execute return false unless can?(current_user, :remove_project, project) + project.update_attribute(:pending_delete, true) # Flush the cache for both repositories. This has to be done _before_ # removing the physical repositories as some expiration code depends on # Git data (e.g. a list of branch names). flush_caches(project) + ::Ci::AbortProjectPipelinesService.new.execute(project) + Projects::UnlinkForkService.new(project, current_user).execute attempt_destroy(project) diff --git a/config/feature_flags/development/abort_deleted_project_pipelines.yml b/config/feature_flags/development/abort_deleted_project_pipelines.yml new file mode 100644 index 0000000000..f09cc9dd86 --- /dev/null +++ b/config/feature_flags/development/abort_deleted_project_pipelines.yml @@ -0,0 +1,8 @@ +--- +name: abort_deleted_project_pipelines +introduced_by_url: https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/1220 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/301106 +milestone: '13.9' +type: development +group: group::continuous integration +default_enabled: true diff --git a/config/feature_flags/development/codequality_mr_diff.yml b/config/feature_flags/development/codequality_mr_diff.yml deleted file mode 100644 index ca6846b939..0000000000 --- a/config/feature_flags/development/codequality_mr_diff.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: codequality_mr_diff -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47938 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/284140 -milestone: '13.7' -type: development -group: group::testing -default_enabled: false diff --git a/db/migrate/20201007033527_add_daily_invites_to_plan_limits.rb b/db/migrate/20201007033527_add_daily_invites_to_plan_limits.rb new file mode 100644 index 0000000000..8f0079cd63 --- /dev/null +++ b/db/migrate/20201007033527_add_daily_invites_to_plan_limits.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddDailyInvitesToPlanLimits < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + add_column(:plan_limits, :daily_invites, :integer, default: 0, null: false) + end +end diff --git a/db/migrate/20201007033723_insert_daily_invites_plan_limits.rb b/db/migrate/20201007033723_insert_daily_invites_plan_limits.rb new file mode 100644 index 0000000000..dcdcbbb096 --- /dev/null +++ b/db/migrate/20201007033723_insert_daily_invites_plan_limits.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class InsertDailyInvitesPlanLimits < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + return unless Gitlab.com? + + create_or_update_plan_limit('daily_invites', 'free', 20) + create_or_update_plan_limit('daily_invites', 'bronze', 0) + create_or_update_plan_limit('daily_invites', 'silver', 0) + create_or_update_plan_limit('daily_invites', 'gold', 0) + end + + def down + return unless Gitlab.com? + + create_or_update_plan_limit('daily_invites', 'free', 0) + create_or_update_plan_limit('daily_invites', 'bronze', 0) + create_or_update_plan_limit('daily_invites', 'silver', 0) + create_or_update_plan_limit('daily_invites', 'gold', 0) + end +end diff --git a/db/schema_migrations/20201007033527 b/db/schema_migrations/20201007033527 new file mode 100644 index 0000000000..b2cedd5797 --- /dev/null +++ b/db/schema_migrations/20201007033527 @@ -0,0 +1 @@ +1200747265d5095a86250020786d6f1e9e50bc75328a71de497046807afa89d7 \ No newline at end of file diff --git a/db/schema_migrations/20201007033723 b/db/schema_migrations/20201007033723 new file mode 100644 index 0000000000..c874ae0475 --- /dev/null +++ b/db/schema_migrations/20201007033723 @@ -0,0 +1 @@ +febefead6f966960f6493d29add5f35fc4a1080b5118c5526502fa5fe1d29023 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 1ed6ea64ec..ae84f8aa79 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14732,7 +14732,8 @@ CREATE TABLE plan_limits ( golang_max_file_size bigint DEFAULT 104857600 NOT NULL, debian_max_file_size bigint DEFAULT '3221225472'::bigint NOT NULL, project_feature_flags integer DEFAULT 200 NOT NULL, - ci_max_artifact_size_api_fuzzing integer DEFAULT 0 NOT NULL + ci_max_artifact_size_api_fuzzing integer DEFAULT 0 NOT NULL, + daily_invites integer DEFAULT 0 NOT NULL ); CREATE SEQUENCE plan_limits_id_seq diff --git a/doc/administration/instance_limits.md b/doc/administration/instance_limits.md index ec56a59fdc..cfd9bccb24 100644 --- a/doc/administration/instance_limits.md +++ b/doc/administration/instance_limits.md @@ -96,6 +96,13 @@ Read more on the [Rack Attack initializer](../security/rack_attack.md) method of - **Default rate limit** - Disabled +### Member Invitations + +Limit the maximum daily member invitations allowed per group hierarchy. + +- GitLab.com: Free members may invite 20 members per day. +- Self-managed: Invites are not limited. + ## Gitaly concurrency limit Clone traffic can put a large strain on your Gitaly service. To prevent such workloads from overwhelming your Gitaly server, you can set concurrency limits in Gitaly’s configuration file. diff --git a/doc/user/project/integrations/prometheus.md b/doc/user/project/integrations/prometheus.md index 97be16f8dd..b99f9e0947 100644 --- a/doc/user/project/integrations/prometheus.md +++ b/doc/user/project/integrations/prometheus.md @@ -182,6 +182,8 @@ service account can be found at Google's documentation for Prometheus OAuth Client secured with Google IAP. 1. (Optional) In **Google IAP Service Account JSON**, provide the contents of the Service Account credentials file that is authorized to access the Prometheus resource. + The JSON key `token_credential_uri` is discarded to prevent + [Server-side Request Forgery (SSRF)](https://www.hackerone.com/blog-How-To-Server-Side-Request-Forgery-SSRF). 1. Click **Save changes**. ![Configure Prometheus Service](img/prometheus_manual_configuration_v13_2.png) diff --git a/lib/api/lint.rb b/lib/api/lint.rb index 58181adaa9..c65c52e305 100644 --- a/lib/api/lint.rb +++ b/lib/api/lint.rb @@ -11,6 +11,8 @@ module API optional :include_merged_yaml, type: Boolean, desc: 'Whether or not to include merged CI config yaml in the response' end post '/lint' do + unauthorized! unless Gitlab::CurrentSettings.signup_enabled? && current_user + result = Gitlab::Ci::YamlProcessor.new(params[:content], user: current_user).execute error = result.errors.first @@ -56,7 +58,7 @@ module API optional :dry_run, type: Boolean, default: false, desc: 'Run pipeline creation simulation, or only do static check.' end post ':id/ci/lint' do - authorize! :download_code, user_project + authorize! :create_pipeline, user_project result = Gitlab::Ci::Lint .new(project: user_project, current_user: current_user) diff --git a/lib/api/merge_request_approvals.rb b/lib/api/merge_request_approvals.rb index 27ef0b9c7c..0d49647641 100644 --- a/lib/api/merge_request_approvals.rb +++ b/lib/api/merge_request_approvals.rb @@ -26,6 +26,8 @@ module API # GET /projects/:id/merge_requests/:merge_request_iid/approvals desc 'List approvals for merge request' get 'approvals' do + not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) + merge_request = find_merge_request_with_access(params[:merge_request_iid]) present_approval(merge_request) diff --git a/lib/api/merge_request_diffs.rb b/lib/api/merge_request_diffs.rb index 0ffb38438e..97a6c7075b 100644 --- a/lib/api/merge_request_diffs.rb +++ b/lib/api/merge_request_diffs.rb @@ -23,6 +23,8 @@ module API use :pagination end get ":id/merge_requests/:merge_request_iid/versions" do + not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) + merge_request = find_merge_request_with_access(params[:merge_request_iid]) present paginate(merge_request.merge_request_diffs.order_id_desc), with: Entities::MergeRequestDiff @@ -39,6 +41,8 @@ module API end get ":id/merge_requests/:merge_request_iid/versions/:version_id" do + not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) + merge_request = find_merge_request_with_access(params[:merge_request_iid]) present merge_request.merge_request_diffs.find(params[:version_id]), with: Entities::MergeRequestDiffFull diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index d17e451093..ba90a249d1 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -243,6 +243,8 @@ module API success Entities::MergeRequest end get ':id/merge_requests/:merge_request_iid' do + not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) + merge_request = find_merge_request_with_access(params[:merge_request_iid]) present merge_request, @@ -259,7 +261,10 @@ module API success Entities::UserBasic end get ':id/merge_requests/:merge_request_iid/participants' do + not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) + merge_request = find_merge_request_with_access(params[:merge_request_iid]) + participants = ::Kaminari.paginate_array(merge_request.participants) present paginate(participants), with: Entities::UserBasic @@ -269,6 +274,8 @@ module API success Entities::Commit end get ':id/merge_requests/:merge_request_iid/commits' do + not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) + merge_request = find_merge_request_with_access(params[:merge_request_iid]) commits = @@ -350,6 +357,8 @@ module API success Entities::MergeRequestChanges end get ':id/merge_requests/:merge_request_iid/changes' do + not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) + merge_request = find_merge_request_with_access(params[:merge_request_iid]) present merge_request, @@ -365,6 +374,8 @@ module API get ':id/merge_requests/:merge_request_iid/pipelines' do pipelines = merge_request_pipelines_with_access + not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) + present paginate(pipelines), with: Entities::Ci::PipelineBasic end diff --git a/lib/api/todos.rb b/lib/api/todos.rb index 03850ba1c4..afc1525cbe 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -28,6 +28,11 @@ module API end post ":id/#{type}/:#{type_id_str}/todo" do issuable = instance_exec(params[type_id_str], &finder) + + unless can?(current_user, :read_merge_request, issuable.project) + not_found!(type.split("_").map(&:capitalize).join(" ")) + end + todo = TodoService.new.mark_todo(issuable, current_user).first if todo diff --git a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb index 8f1e690c08..4f90b7756e 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb @@ -10,6 +10,10 @@ module Gitlab include Chain::Helpers def perform! + if project.pending_delete? + return error('Project is deleted!') + end + unless project.builds_enabled? return error('Pipelines are disabled!') end diff --git a/lib/gitlab/tree_summary.rb b/lib/gitlab/tree_summary.rb index 9b67599668..bc7b8bd2b9 100644 --- a/lib/gitlab/tree_summary.rb +++ b/lib/gitlab/tree_summary.rb @@ -40,21 +40,17 @@ module Gitlab # - An Array of the unique ::Commit objects in the first value def summarize summary = contents - .map { |content| build_entry(content) } .tap { |summary| fill_last_commits!(summary) } [summary, commits] end def fetch_logs - cache_key = ['projects', project.id, 'logs', commit.id, path, offset] - Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRE_IN) do - logs, _ = summarize + logs, _ = summarize - new_offset = next_offset if more? + new_offset = next_offset if more? - [logs.as_json, new_offset] - end + [logs.as_json, new_offset] end # Does the tree contain more entries after the given offset + limit? @@ -71,7 +67,7 @@ module Gitlab private def contents - all_contents[offset, limit] + all_contents[offset, limit] || [] end def commits @@ -82,22 +78,17 @@ module Gitlab project.repository end + # Ensure the path is in "path/" format + def ensured_path + File.join(*[path, ""]) if path + end + def entry_path(entry) File.join(*[path, entry[:file_name]].compact).force_encoding(Encoding::ASCII_8BIT) end - def build_entry(entry) - { file_name: entry.name, type: entry.type } - end - def fill_last_commits!(entries) - # Ensure the path is in "path/" format - ensured_path = - if path - File.join(*[path, ""]) - end - - commits_hsh = repository.list_last_commits_for_tree(commit.id, ensured_path, offset: offset, limit: limit, literal_pathspec: true) + commits_hsh = fetch_last_cached_commits_list prerender_commit_full_titles!(commits_hsh.values) entries.each do |entry| @@ -112,6 +103,18 @@ module Gitlab end end + def fetch_last_cached_commits_list + cache_key = ['projects', project.id, 'last_commits_list', commit.id, ensured_path, offset, limit] + + commits = Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRE_IN) do + repository + .list_last_commits_for_tree(commit.id, ensured_path, offset: offset, limit: limit, literal_pathspec: true) + .transform_values!(&:to_hash) + end + + commits.transform_values! { |value| Commit.from_hash(value, project) } + end + def cache_commit(commit) return unless commit.present? @@ -123,12 +126,18 @@ module Gitlab end def all_contents - strong_memoize(:all_contents) do + strong_memoize(:all_contents) { cached_contents } + end + + def cached_contents + cache_key = ['projects', project.id, 'content', commit.id, path] + + Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRE_IN) do [ *tree.trees, *tree.blobs, *tree.submodules - ] + ].map { |entry| { file_name: entry.name, type: entry.type } } end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 68643cb4a6..c7ed47a279 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1762,6 +1762,9 @@ msgstr "" msgid "AddContextCommits|Add/remove" msgstr "" +msgid "AddMember|Invite limit of %{daily_invites} per day exceeded" +msgstr "" + msgid "AddMember|No users specified." msgstr "" diff --git a/spec/controllers/groups/group_links_controller_spec.rb b/spec/controllers/groups/group_links_controller_spec.rb index c411d9cfb6..a2f7161ca4 100644 --- a/spec/controllers/groups/group_links_controller_spec.rb +++ b/spec/controllers/groups/group_links_controller_spec.rb @@ -9,6 +9,10 @@ RSpec.describe Groups::GroupLinksController do let(:group_member) { create(:user) } let!(:project) { create(:project, group: shared_group) } + around do |example| + travel_to DateTime.new(2019, 4, 1) { example.run } + end + before do sign_in(user) diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 5425a437c8..4d78419e8e 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -9,6 +9,10 @@ RSpec.describe Groups::GroupMembersController do let(:group) { create(:group, :public) } let(:membership) { create(:group_member, group: group) } + around do |example| + travel_to DateTime.new(2019, 4, 1) { example.run } + end + describe 'GET index' do it 'renders index with 200 status code' do get :index, params: { group_id: group } diff --git a/spec/controllers/projects/group_links_controller_spec.rb b/spec/controllers/projects/group_links_controller_spec.rb index 3baadde46d..084a807e16 100644 --- a/spec/controllers/projects/group_links_controller_spec.rb +++ b/spec/controllers/projects/group_links_controller_spec.rb @@ -8,6 +8,10 @@ RSpec.describe Projects::GroupLinksController do let_it_be(:project) { create(:project, :private, group: group2) } let_it_be(:user) { create(:user) } + around do |example| + travel_to DateTime.new(2019, 4, 1) { example.run } + end + before do project.add_maintainer(user) sign_in(user) diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 74311fa89f..73b6d642be 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Projects::ProjectMembersController do let(:group) { create(:group, :public) } let(:project) { create(:project, :public) } + around do |example| + travel_to DateTime.new(2019, 4, 1) { example.run } + end + describe 'GET index' do it 'has the project_members address with a 200 status code' do get :index, params: { namespace_id: project.namespace, project_id: project } diff --git a/spec/controllers/projects/refs_controller_spec.rb b/spec/controllers/projects/refs_controller_spec.rb index d10351feb9..b625ce35d6 100644 --- a/spec/controllers/projects/refs_controller_spec.rb +++ b/spec/controllers/projects/refs_controller_spec.rb @@ -56,18 +56,6 @@ RSpec.describe Projects::RefsController do expect(response).to be_successful expect(json_response).to be_kind_of(Array) end - - it 'caches tree summary data', :use_clean_rails_memory_store_caching do - expect_next_instance_of(::Gitlab::TreeSummary) do |instance| - expect(instance).to receive_messages(summarize: ['logs'], next_offset: 50, more?: true) - end - - xhr_get(:json, offset: 25) - - cache_key = "projects/#{project.id}/logs/#{project.commit.id}/#{path}/25" - expect(Rails.cache.fetch(cache_key)).to eq(['logs', 50]) - expect(response.headers['More-Logs-Offset']).to eq("50") - end end end end diff --git a/spec/factories/project_members.rb b/spec/factories/project_members.rb index 0c2ffac411..1ce07f3602 100644 --- a/spec/factories/project_members.rb +++ b/spec/factories/project_members.rb @@ -15,7 +15,7 @@ FactoryBot.define do trait(:invited) do user_id { nil } invite_token { 'xxx' } - invite_email { 'email@email.com' } + sequence(:invite_email) { |n| "email#{n}@email.com" } end trait :blocked do diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb index ae3270cb9b..7aaeee32f4 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb @@ -74,6 +74,14 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do it 'does not break the chain' do expect(step.break?).to eq false end + + context 'when project is deleted' do + before do + project.update!(pending_delete: true) + end + + specify { expect(step.perform!).to contain_exactly('Project is deleted!') } + end end describe '#allowed_to_write_ref?' do diff --git a/spec/lib/gitlab/tree_summary_spec.rb b/spec/lib/gitlab/tree_summary_spec.rb index 303a4a8058..d2c5844b0f 100644 --- a/spec/lib/gitlab/tree_summary_spec.rb +++ b/spec/lib/gitlab/tree_summary_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe Gitlab::TreeSummary do + include RepoHelpers using RSpec::Parameterized::TableSyntax let(:project) { create(:project, :empty_repo) } @@ -44,6 +45,40 @@ RSpec.describe Gitlab::TreeSummary do expect(commits).to match_array(entries.map { |entry| entry[:commit] }) end end + + context 'when offset is over the limit' do + let(:offset) { 100 } + + it 'returns an empty array' do + expect(summarized).to eq([[], []]) + end + end + + context 'with caching', :use_clean_rails_memory_store_caching do + subject { Rails.cache.fetch(key) } + + before do + summarized + end + + context 'Repository tree cache' do + let(:key) { ['projects', project.id, 'content', commit.id, path] } + + it 'creates a cache for repository content' do + is_expected.to eq([{ file_name: 'a.txt', type: :blob }]) + end + end + + context 'Commits list cache' do + let(:offset) { 0 } + let(:limit) { 25 } + let(:key) { ['projects', project.id, 'last_commits_list', commit.id, path, offset, limit] } + + it 'creates a cache for commits list' do + is_expected.to eq('a.txt' => commit.to_hash) + end + end + end end describe '#summarize (entries)' do @@ -167,6 +202,46 @@ RSpec.describe Gitlab::TreeSummary do end end + describe 'References in commit messages' do + let_it_be(:project) { create(:project, :empty_repo) } + let_it_be(:issue) { create(:issue, project: project) } + let(:entries) { summary.summarize.first } + let(:entry) { entries.find { |entry| entry[:file_name] == 'issue.txt' } } + + before_all do + create_file_in_repo(project, 'master', 'master', 'issue.txt', '', commit_message: "Issue ##{issue.iid}") + end + + where(:project_visibility, :user_role, :issue_confidential, :expected_result) do + 'private' | :guest | false | true + 'private' | :guest | true | false + 'private' | :reporter | false | true + 'private' | :reporter | true | true + + 'internal' | :guest | false | true + 'internal' | :guest | true | false + 'internal' | :reporter | false | true + 'internal' | :reporter | true | true + + 'public' | :guest | false | true + 'public' | :guest | true | false + 'public' | :reporter | false | true + 'public' | :reporter | true | true + end + + with_them do + subject { entry[:commit_title_html].include?("title=\"#{issue.title}\"") } + + before do + project.add_role(user, user_role) + project.update!(visibility_level: Gitlab::VisibilityLevel.level_value(project_visibility)) + issue.update!(confidential: issue_confidential) + end + + it { is_expected.to eq(expected_result) } + end + end + describe '#more?' do let(:path) { 'tmp/more' } diff --git a/spec/migrations/insert_daily_invites_plan_limits_spec.rb b/spec/migrations/insert_daily_invites_plan_limits_spec.rb new file mode 100644 index 0000000000..3265efcb0c --- /dev/null +++ b/spec/migrations/insert_daily_invites_plan_limits_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20201007033723_insert_daily_invites_plan_limits.rb') + +RSpec.describe InsertDailyInvitesPlanLimits do + let(:plans) { table(:plans) } + let(:plan_limits) { table(:plan_limits) } + let!(:free_plan) { plans.create!(name: 'free') } + let!(:bronze_plan) { plans.create!(name: 'bronze') } + let!(:silver_plan) { plans.create!(name: 'silver') } + let!(:gold_plan) { plans.create!(name: 'gold') } + + context 'when on Gitlab.com' do + before do + expect(Gitlab).to receive(:com?).at_most(:twice).and_return(true) + end + + it 'correctly migrates up and down' do + reversible_migration do |migration| + migration.before -> { + expect(plan_limits.where.not(daily_invites: 0)).to be_empty + } + + # Expectations will run after the up migration. + migration.after -> { + expect(plan_limits.pluck(:plan_id, :daily_invites)).to contain_exactly( + [free_plan.id, 20], + [bronze_plan.id, 0], + [silver_plan.id, 0], + [gold_plan.id, 0] + ) + } + end + end + end + + context 'when on self hosted' do + before do + expect(Gitlab).to receive(:com?).at_most(:twice).and_return(false) + end + + it 'correctly migrates up and down' do + reversible_migration do |migration| + migration.before -> { + expect(plan_limits.pluck(:daily_invites)).to eq [] + } + + migration.after -> { + expect(plan_limits.pluck(:daily_invites)).to eq [] + } + end + end + end +end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 1ca370dc95..4c26cc0965 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -34,6 +34,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do it { is_expected.to have_many(:auto_canceled_jobs) } it { is_expected.to have_many(:sourced_pipelines) } it { is_expected.to have_many(:triggered_pipelines) } + it { is_expected.to have_many(:pipeline_artifacts) } it { is_expected.to have_one(:chat_data) } it { is_expected.to have_one(:source_pipeline) } @@ -41,14 +42,15 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do it { is_expected.to have_one(:source_job) } it { is_expected.to have_one(:pipeline_config) } - it { is_expected.to validate_presence_of(:sha) } - it { is_expected.to validate_presence_of(:status) } - it { is_expected.to respond_to :git_author_name } it { is_expected.to respond_to :git_author_email } it { is_expected.to respond_to :short_sha } it { is_expected.to delegate_method(:full_path).to(:project).with_prefix } - it { is_expected.to have_many(:pipeline_artifacts) } + + describe 'validations' do + it { is_expected.to validate_presence_of(:sha) } + it { is_expected.to validate_presence_of(:status) } + end describe 'associations' do it 'has a bidirectional relationship with projects' do diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 1a791820f1..b60af7abad 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -171,6 +171,43 @@ RSpec.describe Member do end end + describe '.in_hierarchy' do + let(:root_ancestor) { create(:group) } + let(:project) { create(:project, group: root_ancestor) } + let(:subgroup) { create(:group, parent: root_ancestor) } + let(:subgroup_project) { create(:project, group: subgroup) } + + let!(:root_ancestor_member) { create(:group_member, group: root_ancestor) } + let!(:project_member) { create(:project_member, project: project) } + let!(:subgroup_member) { create(:group_member, group: subgroup) } + let!(:subgroup_project_member) { create(:project_member, project: subgroup_project) } + + let(:hierarchy_members) do + [ + root_ancestor_member, + project_member, + subgroup_member, + subgroup_project_member + ] + end + + subject { Member.in_hierarchy(project) } + + it { is_expected.to contain_exactly(*hierarchy_members) } + + context 'with scope prefix' do + subject { Member.where.not(source: project).in_hierarchy(subgroup) } + + it { is_expected.to contain_exactly(root_ancestor_member, subgroup_member, subgroup_project_member) } + end + + context 'with scope suffix' do + subject { Member.in_hierarchy(project).where.not(source: project) } + + it { is_expected.to contain_exactly(root_ancestor_member, subgroup_member, subgroup_project_member) } + end + end + describe '.invite' do it { expect(described_class.invite).not_to include @maintainer } it { expect(described_class.invite).to include @invited_member } @@ -251,6 +288,21 @@ RSpec.describe Member do it { is_expected.to include(expiring_tomorrow, not_expiring) } end + describe '.created_today' do + let_it_be(:now) { Time.current } + let_it_be(:created_today) { create(:group_member, created_at: now.beginning_of_day) } + let_it_be(:created_yesterday) { create(:group_member, created_at: now - 1.day) } + + before do + travel_to now + end + + subject { described_class.created_today } + + it { is_expected.not_to include(created_yesterday) } + it { is_expected.to include(created_today) } + end + describe '.last_ten_days_excluding_today' do let_it_be(:now) { Time.current } let_it_be(:created_today) { create(:group_member, created_at: now.beginning_of_day) } diff --git a/spec/models/plan_limits_spec.rb b/spec/models/plan_limits_spec.rb index 67fb11f34e..4259c8b708 100644 --- a/spec/models/plan_limits_spec.rb +++ b/spec/models/plan_limits_spec.rb @@ -209,6 +209,7 @@ RSpec.describe PlanLimits do ci_pipeline_size ci_active_jobs storage_size_limit + daily_invites ] + disabled_max_artifact_size_columns end diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index 76fc5a826c..2813caae78 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -2,11 +2,13 @@ require 'spec_helper' +require 'googleauth' + RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowplow do include PrometheusHelpers include ReactiveCachingHelpers - let(:project) { create(:prometheus_project) } + let_it_be_with_reload(:project) { create(:prometheus_project) } let(:service) { project.prometheus_service } describe "Associations" do @@ -256,19 +258,66 @@ RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowpl context 'behind IAP' do let(:manual_configuration) { true } - before do - # dummy private key generated only for this test to pass openssl validation - service.google_iap_service_account_json = '{"type":"service_account","private_key":"-----BEGIN RSA PRIVATE KEY-----\nMIIBOAIBAAJAU85LgUY5o6j6j/07GMLCNUcWJOBA1buZnNgKELayA6mSsHrIv31J\nY8kS+9WzGPQninea7DcM4hHA7smMgQD1BwIDAQABAkAqKxMy6PL3tn7dFL43p0ex\nJyOtSmlVIiAZG1t1LXhE/uoLpYi5DnbYqGgu0oih+7nzLY/dXpNpXUmiRMOUEKmB\nAiEAoTi2rBXbrLSi2C+H7M/nTOjMQQDuZ8Wr4uWpKcjYJTMCIQCFEskL565oFl/7\nRRQVH+cARrAsAAoJSbrOBAvYZ0PI3QIgIEFwis10vgEF86rOzxppdIG/G+JL0IdD\n9IluZuXAGPECIGUo7qSaLr75o2VEEgwtAFH5aptIPFjrL5LFCKwtdB4RAiAYZgFV\nHCMmaooAw/eELuMoMWNYmujZ7VaAnOewGDW0uw==\n-----END RSA PRIVATE KEY-----\n"}' - service.google_iap_audience_client_id = "IAP_CLIENT_ID.apps.googleusercontent.com" + let(:google_iap_service_account) do + { + type: "service_account", + # dummy private key generated only for this test to pass openssl validation + private_key: <<~KEY + -----BEGIN RSA PRIVATE KEY----- + MIIBOAIBAAJAU85LgUY5o6j6j/07GMLCNUcWJOBA1buZnNgKELayA6mSsHrIv31J + Y8kS+9WzGPQninea7DcM4hHA7smMgQD1BwIDAQABAkAqKxMy6PL3tn7dFL43p0ex + JyOtSmlVIiAZG1t1LXhE/uoLpYi5DnbYqGgu0oih+7nzLY/dXpNpXUmiRMOUEKmB + AiEAoTi2rBXbrLSi2C+H7M/nTOjMQQDuZ8Wr4uWpKcjYJTMCIQCFEskL565oFl/7 + RRQVH+cARrAsAAoJSbrOBAvYZ0PI3QIgIEFwis10vgEF86rOzxppdIG/G+JL0IdD + 9IluZuXAGPECIGUo7qSaLr75o2VEEgwtAFH5aptIPFjrL5LFCKwtdB4RAiAYZgFV + HCMmaooAw/eELuMoMWNYmujZ7VaAnOewGDW0uw== + -----END RSA PRIVATE KEY----- + KEY + } + end - stub_request(:post, "https://oauth2.googleapis.com/token").to_return(status: 200, body: '{"id_token": "FOO"}', headers: { 'Content-Type': 'application/json; charset=UTF-8' }) + def stub_iap_request + service.google_iap_service_account_json = Gitlab::Json.generate(google_iap_service_account) + service.google_iap_audience_client_id = 'IAP_CLIENT_ID.apps.googleusercontent.com' + + stub_request(:post, 'https://oauth2.googleapis.com/token') + .to_return( + status: 200, + body: '{"id_token": "FOO"}', + headers: { 'Content-Type': 'application/json; charset=UTF-8' } + ) end it 'includes the authorization header' do + stub_iap_request + expect(service.prometheus_client).not_to be_nil expect(service.prometheus_client.send(:options)).to have_key(:headers) expect(service.prometheus_client.send(:options)[:headers]).to eq(authorization: "Bearer FOO") end + + context 'when passed with token_credential_uri', issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/284819' do + let(:malicious_host) { 'http://example.com' } + + where(:param_name) do + [ + :token_credential_uri, + :tokencredentialuri, + :Token_credential_uri, + :tokenCredentialUri + ] + end + + with_them do + it 'does not make any unexpected HTTP requests' do + google_iap_service_account[param_name] = malicious_host + stub_iap_request + stub_request(:any, malicious_host).to_raise('Making additional HTTP requests is forbidden!') + + expect(service.prometheus_client).not_to be_nil + end + end + end end end diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb index aecbcfb5b5..2316e702c3 100644 --- a/spec/requests/api/lint_spec.rb +++ b/spec/requests/api/lint_spec.rb @@ -4,91 +4,136 @@ require 'spec_helper' RSpec.describe API::Lint do describe 'POST /ci/lint' do - context 'with valid .gitlab-ci.yaml content' do - let(:yaml_content) do - File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) + context 'when signup settings are disabled' do + Gitlab::CurrentSettings.signup_enabled = false + + context 'when unauthenticated' do + it 'returns authentication error' do + post api('/ci/lint'), params: { content: 'content' } + + expect(response).to have_gitlab_http_status(:unauthorized) + end end - it 'passes validation without warnings or errors' do - post api('/ci/lint'), params: { content: yaml_content } + context 'when authenticated' do + it 'returns unauthorized error' do + post api('/ci/lint'), params: { content: 'content' } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Hash - expect(json_response['status']).to eq('valid') - expect(json_response['warnings']).to eq([]) - expect(json_response['errors']).to eq([]) - end - - it 'outputs expanded yaml content' do - post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true } - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to have_key('merged_yaml') + expect(response).to have_gitlab_http_status(:unauthorized) + end end end - context 'with valid .gitlab-ci.yaml with warnings' do - let(:yaml_content) { { job: { script: 'ls', rules: [{ when: 'always' }] } }.to_yaml } + context 'when signup settings are enabled' do + Gitlab::CurrentSettings.signup_enabled = true - it 'passes validation but returns warnings' do - post api('/ci/lint'), params: { content: yaml_content } + context 'when unauthenticated' do + it 'returns authentication error' do + post api('/ci/lint'), params: { content: 'content' } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['status']).to eq('valid') - expect(json_response['warnings']).not_to be_empty - expect(json_response['status']).to eq('valid') - expect(json_response['errors']).to eq([]) + expect(response).to have_gitlab_http_status(:unauthorized) + end end - end - context 'with an invalid .gitlab_ci.yml' do - context 'with invalid syntax' do - let(:yaml_content) { 'invalid content' } - - it 'responds with errors about invalid syntax' do - post api('/ci/lint'), params: { content: yaml_content } + context 'when authenticated' do + let_it_be(:api_user) { create(:user) } + it 'returns authentication success' do + post api('/ci/lint', api_user), params: { content: 'content' } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['status']).to eq('invalid') + end + end + end + + context 'when authenticated' do + let_it_be(:api_user) { create(:user) } + + context 'with valid .gitlab-ci.yaml content' do + let(:yaml_content) do + File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) + end + + it 'passes validation without warnings or errors' do + post api('/ci/lint', api_user), params: { content: yaml_content } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Hash + expect(json_response['status']).to eq('valid') expect(json_response['warnings']).to eq([]) - expect(json_response['errors']).to eq(['Invalid configuration format']) + expect(json_response['errors']).to eq([]) end it 'outputs expanded yaml content' do - post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true } + post api('/ci/lint', api_user), params: { content: yaml_content, include_merged_yaml: true } expect(response).to have_gitlab_http_status(:ok) expect(json_response).to have_key('merged_yaml') end end - context 'with invalid configuration' do - let(:yaml_content) { '{ image: "ruby:2.7", services: ["postgres"] }' } + context 'with valid .gitlab-ci.yaml with warnings' do + let(:yaml_content) { { job: { script: 'ls', rules: [{ when: 'always' }] } }.to_yaml } - it 'responds with errors about invalid configuration' do - post api('/ci/lint'), params: { content: yaml_content } + it 'passes validation but returns warnings' do + post api('/ci/lint', api_user), params: { content: yaml_content } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['status']).to eq('invalid') - expect(json_response['warnings']).to eq([]) - expect(json_response['errors']).to eq(['jobs config should contain at least one visible job']) - end - - it 'outputs expanded yaml content' do - post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true } - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to have_key('merged_yaml') + expect(json_response['status']).to eq('valid') + expect(json_response['warnings']).not_to be_empty + expect(json_response['status']).to eq('valid') + expect(json_response['errors']).to eq([]) end end - end - context 'without the content parameter' do - it 'responds with validation error about missing content' do - post api('/ci/lint') + context 'with an invalid .gitlab_ci.yml' do + context 'with invalid syntax' do + let(:yaml_content) { 'invalid content' } - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('content is missing') + it 'responds with errors about invalid syntax' do + post api('/ci/lint', api_user), params: { content: yaml_content } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['status']).to eq('invalid') + expect(json_response['warnings']).to eq([]) + expect(json_response['errors']).to eq(['Invalid configuration format']) + end + + it 'outputs expanded yaml content' do + post api('/ci/lint', api_user), params: { content: yaml_content, include_merged_yaml: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to have_key('merged_yaml') + end + end + + context 'with invalid configuration' do + let(:yaml_content) { '{ image: "ruby:2.7", services: ["postgres"] }' } + + it 'responds with errors about invalid configuration' do + post api('/ci/lint', api_user), params: { content: yaml_content } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['status']).to eq('invalid') + expect(json_response['warnings']).to eq([]) + expect(json_response['errors']).to eq(['jobs config should contain at least one visible job']) + end + + it 'outputs expanded yaml content' do + post api('/ci/lint', api_user), params: { content: yaml_content, include_merged_yaml: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to have_key('merged_yaml') + end + end + end + + context 'without the content parameter' do + it 'responds with validation error about missing content' do + post api('/ci/lint', api_user) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('content is missing') + end end end end @@ -364,6 +409,18 @@ RSpec.describe API::Lint do expect(response).to have_gitlab_http_status(:not_found) end + + context 'when project is public' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it 'returns authentication error' do + ci_lint + + expect(response).to have_gitlab_http_status(:forbidden) + end + end end context 'when authenticated as non-member' do @@ -387,13 +444,10 @@ RSpec.describe API::Lint do context 'when running as dry run' do let(:dry_run) { true } - it 'returns pipeline creation error' do + it 'returns authentication error' do ci_lint - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['merged_yaml']).to eq(nil) - expect(json_response['valid']).to eq(false) - expect(json_response['errors']).to eq(['Insufficient permissions to create a new pipeline']) + expect(response).to have_gitlab_http_status(:forbidden) end end @@ -410,7 +464,11 @@ RSpec.describe API::Lint do ) end - it_behaves_like 'valid project config' + it 'returns authentication error' do + ci_lint + + expect(response).to have_gitlab_http_status(:forbidden) + end end end end diff --git a/spec/requests/api/merge_request_approvals_spec.rb b/spec/requests/api/merge_request_approvals_spec.rb index fad5c3fb60..b18f3017e0 100644 --- a/spec/requests/api/merge_request_approvals_spec.rb +++ b/spec/requests/api/merge_request_approvals_spec.rb @@ -21,6 +21,12 @@ RSpec.describe API::MergeRequestApprovals do expect(response).to have_gitlab_http_status(:ok) end + + context 'when merge request author has only guest access' do + it_behaves_like 'rejects user from accessing merge request info' do + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvals" } + end + end end describe 'POST :id/merge_requests/:merge_request_iid/approve' do diff --git a/spec/requests/api/merge_request_diffs_spec.rb b/spec/requests/api/merge_request_diffs_spec.rb index 2e6cbe7bee..971fb5e991 100644 --- a/spec/requests/api/merge_request_diffs_spec.rb +++ b/spec/requests/api/merge_request_diffs_spec.rb @@ -35,6 +35,12 @@ RSpec.describe API::MergeRequestDiffs, 'MergeRequestDiffs' do get api("/projects/#{project.id}/merge_requests/0/versions", user) expect(response).to have_gitlab_http_status(:not_found) end + + context 'when merge request author has only guest access' do + it_behaves_like 'rejects user from accessing merge request info' do + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions" } + end + end end describe 'GET /projects/:id/merge_requests/:merge_request_iid/versions/:version_id' do @@ -63,5 +69,11 @@ RSpec.describe API::MergeRequestDiffs, 'MergeRequestDiffs' do get api("/projects/#{project.id}/merge_requests/#{non_existing_record_iid}/versions/#{merge_request_diff.id}", user) expect(response).to have_gitlab_http_status(:not_found) end + + context 'when merge request author has only guest access' do + it_behaves_like 'rejects user from accessing merge request info' do + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions/#{merge_request_diff.id}" } + end + end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index e7005bd3ec..b6c85e98bd 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1087,6 +1087,12 @@ RSpec.describe API::MergeRequests do end end + context 'when merge request author has only guest access' do + it_behaves_like 'rejects user from accessing merge request info' do + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}" } + end + end + context 'merge_request_metrics' do let(:pipeline) { create(:ci_empty_pipeline) } @@ -1263,6 +1269,12 @@ RSpec.describe API::MergeRequests do it_behaves_like 'issuable participants endpoint' do let(:entity) { create(:merge_request, :simple, milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, source_branch: 'markdown', title: "Test", created_at: base_time) } end + + context 'when merge request author has only guest access' do + it_behaves_like 'rejects user from accessing merge request info' do + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/participants" } + end + end end describe 'GET /projects/:id/merge_requests/:merge_request_iid/commits' do @@ -1288,6 +1300,12 @@ RSpec.describe API::MergeRequests do expect(response).to have_gitlab_http_status(:not_found) end + + context 'when merge request author has only guest access' do + it_behaves_like 'rejects user from accessing merge request info' do + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/commits" } + end + end end describe 'GET /projects/:id/merge_requests/:merge_request_iid/:context_commits' do @@ -1363,6 +1381,12 @@ RSpec.describe API::MergeRequests do expect(response).to have_gitlab_http_status(:not_found) end + context 'when merge request author has only guest access' do + it_behaves_like 'rejects user from accessing merge request info' do + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/changes" } + end + end + it_behaves_like 'find an existing merge request' it_behaves_like 'accesses diffs via raw_diffs' @@ -1452,6 +1476,12 @@ RSpec.describe API::MergeRequests do expect(response).to have_gitlab_http_status(:forbidden) end end + + context 'when merge request author has only guest access' do + it_behaves_like 'rejects user from accessing merge request info' do + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/pipelines" } + end + end end describe 'POST /projects/:id/merge_requests/:merge_request_iid/pipelines' do diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index bc315c5b3c..4a86453a09 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -329,6 +329,14 @@ RSpec.describe API::Todos do expect(response).to have_gitlab_http_status(:not_found) end end + + it 'returns an error if the issuable author does not have access' do + project_1.add_guest(issuable.author) + + post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.iid}/todo", issuable.author) + + expect(response).to have_gitlab_http_status(:not_found) + end end describe 'POST :id/issuable_type/:issueable_id/todo' do diff --git a/spec/services/ci/abort_project_pipelines_service_spec.rb b/spec/services/ci/abort_project_pipelines_service_spec.rb new file mode 100644 index 0000000000..9af909ac2a --- /dev/null +++ b/spec/services/ci/abort_project_pipelines_service_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::AbortProjectPipelinesService do + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, :running, project: project) } + let_it_be(:build) { create(:ci_build, :running, pipeline: pipeline) } + + describe '#execute' do + it 'cancels all running pipelines and related jobs' do + result = described_class.new.execute(project) + + expect(result).to be_success + expect(pipeline.reload).to be_canceled + expect(build.reload).to be_canceled + end + + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new { described_class.new.execute(project) }.count + + pipelines = create_list(:ci_pipeline, 5, :running, project: project) + create_list(:ci_build, 5, :running, pipeline: pipelines.first) + + expect { described_class.new.execute(project) }.not_to exceed_query_limit(control_count) + end + end + + context 'when feature disabled' do + before do + stub_feature_flags(abort_deleted_project_pipelines: false) + end + + it 'does not abort the pipeline' do + result = described_class.new.execute(project) + + expect(result).to be(nil) + expect(pipeline.reload).to be_running + expect(build.reload).to be_running + end + end +end diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index f0f09218b0..75d1c98923 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -69,6 +69,12 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do destroy_project(project, user, {}) end + it 'performs cancel for project ci pipelines' do + expect(::Ci::AbortProjectPipelinesService).to receive_message_chain(:new, :execute).with(project) + + destroy_project(project, user, {}) + end + context 'when project has remote mirrors' do let!(:project) do create(:project, :repository, namespace: user.namespace).tap do |project| diff --git a/spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb b/spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb new file mode 100644 index 0000000000..e6f9e5a434 --- /dev/null +++ b/spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'rejects user from accessing merge request info' do + let(:project) { create(:project, :private) } + let(:merge_request) do + create(:merge_request, + author: user, + source_project: project, + target_project: project + ) + end + + before do + project.add_guest(user) + end + + it 'returns a 404 error' do + get api(url, user) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Merge Request Not Found') + end +end