diff options
author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2021-02-11 14:12:40 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2021-02-11 14:12:40 +0000 |
commit | 045b8e4bc47841fcb1af56687a61f48899ce3e0d (patch) | |
tree | 2e828365b8c7efe281d7fc25aa89f01a694140e1 | |
parent | efdc7889a59a7e5a52f8bacb578de2d40beb5871 (diff) | |
parent | 9fb9cbf50c31dc9d99fe3d6fd04782fbfc4216c0 (diff) | |
download | gitlab-ce-045b8e4bc47841fcb1af56687a61f48899ce3e0d.tar.gz |
Merge remote-tracking branch 'dev/13-8-stable' into 13-8-stable
51 files changed, 801 insertions, 114 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index cf92f4b7651..46776b926c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 13.8.4 (2021-02-11) + +### Security (9 changes) + +- Cancel running and pending jobs when a project is deleted. !1220 +- Prevent Denial of Service Attack on gitlab-shell. +- Prevent exposure of confidential issue titles in file browser. +- Updates authorization for linting API. +- Check user access on API merge request read actions. +- Limit daily invitations to groups and projects. +- Enforce the analytics enabled project setting for project-level analytics features. +- Perform SSL verification for FortiTokenCloud Integration. +- Prevent Server-side Request Forgery for Prometheus when secured by Google IAP. + + ## 13.8.3 (2021-02-05) ### Fixed (2 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 32d894f3e94..355a70a7731 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -13.8.3
\ No newline at end of file +13.8.4
\ No newline at end of file diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index 04f6473099c..548118b6e78 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -13.15.0 +13.15.1 @@ -1 +1 @@ -13.8.3
\ No newline at end of file +13.8.4
\ No newline at end of file diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index e44c00e501e..ae8b3d9b51d 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -9,6 +9,7 @@ class Projects::PipelinesController < Projects::ApplicationController before_action :set_pipeline_path, only: [:show] before_action :authorize_read_pipeline! before_action :authorize_read_build!, only: [:index] + before_action :authorize_read_analytics!, only: [:charts] before_action :authorize_create_pipeline!, only: [:new, :create, :config_variables] before_action :authorize_update_pipeline!, only: [:retry, :cancel] before_action do diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 4a579892e3f..88c7002b1b6 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -16,6 +16,7 @@ module Ci include ShaAttribute include FromUnion include UpdatedAtFilterable + include EachBatch MAX_OPEN_MERGE_REQUESTS_REFS = 4 diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index a399ffc32de..c2aecc524d4 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 2e79b50d6b7..62fe757683f 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -47,6 +47,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: # @@ -79,12 +92,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 d0e62a1afba..ab043227832 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/policies/project_policy.rb b/app/policies/project_policy.rb index 03cb53f55be..6135523a2f8 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -218,6 +218,7 @@ class ProjectPolicy < BasePolicy enable :read_pages_content enable :read_release enable :read_analytics + enable :read_insights end # These abilities are not allowed to admins that are not members of the project, @@ -447,6 +448,9 @@ class ProjectPolicy < BasePolicy rule { analytics_disabled }.policy do prevent(:read_analytics) + prevent(:read_insights) + prevent(:read_cycle_analytics) + prevent(:read_repository_graphs) end rule { wiki_disabled }.policy do @@ -520,6 +524,7 @@ class ProjectPolicy < BasePolicy enable :read_cycle_analytics enable :read_pages_content enable :read_analytics + enable :read_insights # NOTE: may be overridden by IssuePolicy enable :read_issue 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 00000000000..0b2fa9ed3c0 --- /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: :yaml) + + 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 3a8b5e91088..3d3a8032e8e 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 5fcf2d711b0..cffccda1a44 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 @@ -47,6 +47,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 bec75657530..c1501625300 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 00000000000..f09cc9dd86b --- /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/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 00000000000..8f0079cd639 --- /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 00000000000..dcdcbbb0964 --- /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 00000000000..b2cedd57973 --- /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 00000000000..c874ae0475b --- /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 de4218ed405..c1f13a2fca4 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -15254,7 +15254,8 @@ CREATE TABLE plan_limits ( project_feature_flags integer DEFAULT 200 NOT NULL, ci_max_artifact_size_api_fuzzing integer DEFAULT 0 NOT NULL, ci_pipeline_deployments integer DEFAULT 500 NOT NULL, - pull_mirror_interval_seconds integer DEFAULT 300 NOT NULL + pull_mirror_interval_seconds integer DEFAULT 300 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 0eede5f3ca5..a3452a1a605 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 959c4cc623b..1507cea348a 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 f1f34622187..2d30754a36d 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 status 200 @@ -55,7 +57,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 00f42703731..0cdfd8f94b4 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 0ffb38438eb..97a6c7075b3 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 ab0e9b95e4a..142ecd0dc1e 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -246,6 +246,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, @@ -262,7 +264,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 @@ -272,6 +277,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 = @@ -353,6 +360,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, @@ -368,6 +377,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 03850ba1c4e..afc1525cbe2 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/auth/otp/strategies/forti_token_cloud.rb b/lib/gitlab/auth/otp/strategies/forti_token_cloud.rb index d7506eca242..079d631e22a 100644 --- a/lib/gitlab/auth/otp/strategies/forti_token_cloud.rb +++ b/lib/gitlab/auth/otp/strategies/forti_token_cloud.rb @@ -61,8 +61,7 @@ module Gitlab headers: { 'Content-Type': 'application/json' }.merge(headers), - body: body, - verify: false # FTC API Docs specifically mentions to turn off SSL Verification while making requests. + body: body ) end end diff --git a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb index e68d9020a21..55c125e03d5 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 9b67599668a..bc7b8bd2b94 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 - def entry_path(entry) - File.join(*[path, entry[:file_name]].compact).force_encoding(Encoding::ASCII_8BIT) + # Ensure the path is in "path/" format + def ensured_path + File.join(*[path, ""]) if path end - def build_entry(entry) - { file_name: entry.name, type: entry.type } + def entry_path(entry) + File.join(*[path, entry[:file_name]].compact).force_encoding(Encoding::ASCII_8BIT) 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 e47bbec804f..0b1b80d59ff 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1807,6 +1807,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/projects/refs_controller_spec.rb b/spec/controllers/projects/refs_controller_spec.rb index d10351feb9e..b625ce35d61 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/projects.rb b/spec/factories/projects.rb index f5e496080c4..54a5dea49bb 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -28,6 +28,7 @@ FactoryBot.define do forking_access_level { ProjectFeature::ENABLED } merge_requests_access_level { ProjectFeature::ENABLED } repository_access_level { ProjectFeature::ENABLED } + analytics_access_level { ProjectFeature::ENABLED } pages_access_level do visibility_level == Gitlab::VisibilityLevel::PUBLIC ? ProjectFeature::ENABLED : ProjectFeature::PRIVATE end @@ -63,7 +64,8 @@ FactoryBot.define do repository_access_level: evaluator.repository_access_level, pages_access_level: evaluator.pages_access_level, metrics_dashboard_access_level: evaluator.metrics_dashboard_access_level, - operations_access_level: evaluator.operations_access_level + operations_access_level: evaluator.operations_access_level, + analytics_access_level: evaluator.analytics_access_level } project.build_project_feature(hash) @@ -335,6 +337,9 @@ FactoryBot.define do trait(:operations_enabled) { operations_access_level { ProjectFeature::ENABLED } } trait(:operations_disabled) { operations_access_level { ProjectFeature::DISABLED } } trait(:operations_private) { operations_access_level { ProjectFeature::PRIVATE } } + trait(:analytics_enabled) { analytics_access_level { ProjectFeature::ENABLED } } + trait(:analytics_disabled) { analytics_access_level { ProjectFeature::DISABLED } } + trait(:analytics_private) { analytics_access_level { ProjectFeature::PRIVATE } } trait :auto_devops do association :auto_devops, factory: :project_auto_devops diff --git a/spec/lib/gitlab/auth/otp/strategies/forti_token_cloud_spec.rb b/spec/lib/gitlab/auth/otp/strategies/forti_token_cloud_spec.rb index 1580fc82279..368cf98dfec 100644 --- a/spec/lib/gitlab/auth/otp/strategies/forti_token_cloud_spec.rb +++ b/spec/lib/gitlab/auth/otp/strategies/forti_token_cloud_spec.rb @@ -13,6 +13,8 @@ RSpec.describe Gitlab::Auth::Otp::Strategies::FortiTokenCloud do let(:otp_verification_url) { url + '/auth' } let(:access_token) { 'an_access_token' } let(:access_token_create_response_body) { '' } + let(:access_token_request_body) { { client_id: client_id, client_secret: client_secret } } + let(:headers) { { 'Content-Type': 'application/json' } } subject(:validate) { described_class.new(user).validate(otp_code) } @@ -27,11 +29,8 @@ RSpec.describe Gitlab::Auth::Otp::Strategies::FortiTokenCloud do client_secret: client_secret ) - access_token_request_body = { client_id: client_id, - client_secret: client_secret } - stub_request(:post, access_token_create_url) - .with(body: JSON(access_token_request_body), headers: { 'Content-Type' => 'application/json' }) + .with(body: JSON(access_token_request_body), headers: headers) .to_return( status: access_token_create_response_status, body: Gitlab::Json.generate(access_token_create_response_body), @@ -81,6 +80,20 @@ RSpec.describe Gitlab::Auth::Otp::Strategies::FortiTokenCloud do end end + context 'SSL Verification' do + let(:access_token_create_response_status) { 400 } + + context 'with `Gitlab::HTTP`' do + it 'does not use a `verify` argument,'\ + 'thereby always performing SSL verification while making API calls' do + expect(Gitlab::HTTP).to receive(:post) + .with(access_token_create_url, body: JSON(access_token_request_body), headers: headers).and_call_original + + validate + end + end + end + def stub_forti_token_cloud_config(forti_token_cloud_settings) allow(::Gitlab.config.forti_token_cloud).to(receive_messages(forti_token_cloud_settings)) end 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 ae3270cb9b2..7aaeee32f49 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 303a4a80581..d2c5844b0fa 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 00000000000..3265efcb0ce --- /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 f5e824bb066..140527e4414 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 1a791820f1b..b60af7abade 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 67fb11f34e0..4259c8b708b 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 8215fb5c336..ea63406e615 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 + + 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' }) + 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/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index e6650549f7f..8bd4a463f87 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -992,6 +992,78 @@ RSpec.describe ProjectPolicy do it { is_expected.to be_allowed(:read_analytics) } end + context 'with various analytics features' do + let_it_be(:project_with_analytics_disabled) { create(:project, :analytics_disabled) } + let_it_be(:project_with_analytics_private) { create(:project, :analytics_private) } + let_it_be(:project_with_analytics_enabled) { create(:project, :analytics_enabled) } + + before do + project_with_analytics_disabled.add_developer(developer) + project_with_analytics_private.add_developer(developer) + project_with_analytics_enabled.add_developer(developer) + end + + context 'when analytics is enabled for the project' do + let(:project) { project_with_analytics_disabled } + + context 'for guest user' do + let(:current_user) { guest } + + it { is_expected.to be_disallowed(:read_cycle_analytics) } + it { is_expected.to be_disallowed(:read_insights) } + it { is_expected.to be_disallowed(:read_repository_graphs) } + end + + context 'for developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:read_cycle_analytics) } + it { is_expected.to be_disallowed(:read_insights) } + it { is_expected.to be_disallowed(:read_repository_graphs) } + end + end + + context 'when analytics is private for the project' do + let(:project) { project_with_analytics_private } + + context 'for guest user' do + let(:current_user) { guest } + + it { is_expected.to be_disallowed(:read_cycle_analytics) } + it { is_expected.to be_disallowed(:read_insights) } + it { is_expected.to be_disallowed(:read_repository_graphs) } + end + + context 'for developer' do + let(:current_user) { developer } + + it { is_expected.to be_allowed(:read_cycle_analytics) } + it { is_expected.to be_allowed(:read_insights) } + it { is_expected.to be_allowed(:read_repository_graphs) } + end + end + + context 'when analytics is enabled for the project' do + let(:project) { project_with_analytics_private } + + context 'for guest user' do + let(:current_user) { guest } + + it { is_expected.to be_disallowed(:read_cycle_analytics) } + it { is_expected.to be_disallowed(:read_insights) } + it { is_expected.to be_disallowed(:read_repository_graphs) } + end + + context 'for developer' do + let(:current_user) { developer } + + it { is_expected.to be_allowed(:read_cycle_analytics) } + it { is_expected.to be_allowed(:read_insights) } + it { is_expected.to be_allowed(:read_repository_graphs) } + end + end + end + context 'project member' do let(:project) { private_project } diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb index 2653653c896..2316e702c3e 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')) - end + context 'when signup settings are disabled' do + Gitlab::CurrentSettings.signup_enabled = false - it 'passes validation without warnings or errors' 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).to be_an Hash - expect(json_response['status']).to eq('valid') - expect(json_response['warnings']).to eq([]) - expect(json_response['errors']).to eq([]) + expect(response).to have_gitlab_http_status(:unauthorized) + end end - it 'outputs expanded yaml content' do - post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true } + 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 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 + + context 'when authenticated' do + let_it_be(:api_user) { create(:user) } + it 'returns authentication success' do + post api('/ci/lint', api_user), params: { content: 'content' } + + expect(response).to have_gitlab_http_status(:ok) + end end end - context 'with an invalid .gitlab_ci.yml' do - context 'with invalid syntax' do - let(:yaml_content) { 'invalid content' } + context 'when authenticated' do + let_it_be(:api_user) { create(:user) } - it 'responds with errors about invalid syntax' do - post api('/ci/lint'), params: { content: yaml_content } + 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['status']).to eq('invalid') + 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"], invalid }' } + 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 invalid config should implement a script: or a trigger: keyword', 'jobs config should contain at least one visible job']) + 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 - it 'outputs expanded yaml content' do - post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true } + 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(:ok) - expect(json_response).to have_key('merged_yaml') + 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 - end - context 'without the content parameter' do - it 'responds with validation error about missing content' do - post api('/ci/lint') + 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') + 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 fad5c3fb60e..b18f3017e03 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 2e6cbe7bee7..971fb5e991c 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 3a3eae73932..a04867658e8 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1226,6 +1226,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) } @@ -1402,6 +1408,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 @@ -1427,6 +1439,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 @@ -1502,6 +1520,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' @@ -1591,6 +1615,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/projects_spec.rb b/spec/requests/api/projects_spec.rb index e3eb3649a1a..8a4a7880ab4 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -886,6 +886,7 @@ RSpec.describe API::Projects do merge_method: 'ff' }).tap do |attrs| attrs[:operations_access_level] = 'disabled' + attrs[:analytics_access_level] = 'disabled' end post api('/projects', user), params: project diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index eaffa49fc9d..00de1ef5964 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -331,6 +331,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 00000000000..9af909ac2ab --- /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 f0f09218b06..75d1c98923a 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 00000000000..e6f9e5a434c --- /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 |