summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>2021-02-11 14:12:40 +0000
committerGitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>2021-02-11 14:12:40 +0000
commit045b8e4bc47841fcb1af56687a61f48899ce3e0d (patch)
tree2e828365b8c7efe281d7fc25aa89f01a694140e1
parentefdc7889a59a7e5a52f8bacb578de2d40beb5871 (diff)
parent9fb9cbf50c31dc9d99fe3d6fd04782fbfc4216c0 (diff)
downloadgitlab-ce-045b8e4bc47841fcb1af56687a61f48899ce3e0d.tar.gz
Merge remote-tracking branch 'dev/13-8-stable' into 13-8-stable
-rw-r--r--CHANGELOG.md15
-rw-r--r--GITALY_SERVER_VERSION2
-rw-r--r--GITLAB_SHELL_VERSION2
-rw-r--r--VERSION2
-rw-r--r--app/controllers/projects/pipelines_controller.rb1
-rw-r--r--app/models/ci/pipeline.rb1
-rw-r--r--app/models/commit_status.rb1
-rw-r--r--app/models/member.rb19
-rw-r--r--app/models/project_services/prometheus_service.rb12
-rw-r--r--app/policies/project_policy.rb5
-rw-r--r--app/services/ci/abort_project_pipelines_service.rb25
-rw-r--r--app/services/ci/cancel_user_pipelines_service.rb1
-rw-r--r--app/services/members/create_service.rb13
-rw-r--r--app/services/projects/destroy_service.rb3
-rw-r--r--config/feature_flags/development/abort_deleted_project_pipelines.yml8
-rw-r--r--db/migrate/20201007033527_add_daily_invites_to_plan_limits.rb9
-rw-r--r--db/migrate/20201007033723_insert_daily_invites_plan_limits.rb25
-rw-r--r--db/schema_migrations/202010070335271
-rw-r--r--db/schema_migrations/202010070337231
-rw-r--r--db/structure.sql3
-rw-r--r--doc/administration/instance_limits.md7
-rw-r--r--doc/user/project/integrations/prometheus.md2
-rw-r--r--lib/api/lint.rb4
-rw-r--r--lib/api/merge_request_approvals.rb2
-rw-r--r--lib/api/merge_request_diffs.rb4
-rw-r--r--lib/api/merge_requests.rb11
-rw-r--r--lib/api/todos.rb5
-rw-r--r--lib/gitlab/auth/otp/strategies/forti_token_cloud.rb3
-rw-r--r--lib/gitlab/ci/pipeline/chain/validate/abilities.rb4
-rw-r--r--lib/gitlab/tree_summary.rb51
-rw-r--r--locale/gitlab.pot3
-rw-r--r--spec/controllers/projects/refs_controller_spec.rb12
-rw-r--r--spec/factories/projects.rb7
-rw-r--r--spec/lib/gitlab/auth/otp/strategies/forti_token_cloud_spec.rb21
-rw-r--r--spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb8
-rw-r--r--spec/lib/gitlab/tree_summary_spec.rb75
-rw-r--r--spec/migrations/insert_daily_invites_plan_limits_spec.rb55
-rw-r--r--spec/models/ci/pipeline_spec.rb10
-rw-r--r--spec/models/member_spec.rb52
-rw-r--r--spec/models/plan_limits_spec.rb1
-rw-r--r--spec/models/project_services/prometheus_service_spec.rb61
-rw-r--r--spec/policies/project_policy_spec.rb72
-rw-r--r--spec/requests/api/lint_spec.rb168
-rw-r--r--spec/requests/api/merge_request_approvals_spec.rb6
-rw-r--r--spec/requests/api/merge_request_diffs_spec.rb12
-rw-r--r--spec/requests/api/merge_requests_spec.rb30
-rw-r--r--spec/requests/api/projects_spec.rb1
-rw-r--r--spec/requests/api/todos_spec.rb8
-rw-r--r--spec/services/ci/abort_project_pipelines_service_spec.rb42
-rw-r--r--spec/services/projects/destroy_service_spec.rb6
-rw-r--r--spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb23
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
diff --git a/VERSION b/VERSION
index 32d894f3e94..355a70a7731 100644
--- a/VERSION
+++ b/VERSION
@@ -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