diff options
| author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-05-02 20:52:41 +0000 |
|---|---|---|
| committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-05-02 20:52:41 +0000 |
| commit | 056dd422ea9e03869c182a9200fcbe9166d01ad1 (patch) | |
| tree | cafe567cae3aa30b61d6d788640c08633c7f87d3 | |
| parent | 88da5554d9626377fe7868e956a47a0498e04eb5 (diff) | |
| parent | cfeee9d301b69a98a1bc11f41153cc783f0ed0cb (diff) | |
| download | gitlab-ce-056dd422ea9e03869c182a9200fcbe9166d01ad1.tar.gz | |
Merge remote-tracking branch 'dev/14-10-stable' into 14-10-stable
76 files changed, 1120 insertions, 375 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 39445e88fc3..5758dd763e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,25 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 14.10.1 (2022-04-29) + +### Security (14 changes) + +- [Add suffix to cache name to add isolation](gitlab-org/security/gitlab@9ff0233c191339f4dd042b7f55d1ffd66b3f9a2b) ([merge request](gitlab-org/security/gitlab!2426)) +- [Update Import/Export merge/push access levels & exclude ci config path](gitlab-org/security/gitlab@40f32316dad5bb0779907261215b3526ed8871fc) ([merge request](gitlab-org/security/gitlab!2404)) +- [Prevent maintainers from editing PipelineSchedule](gitlab-org/security/gitlab@2ce3805447b4b3b7336d46d1d21dcd9e173c40be) ([merge request](gitlab-org/security/gitlab!2421)) +- [Add validation to pypi file sha256 values](gitlab-org/security/gitlab@afc796f43df09a2e43f40beaffec942a80ad973d) ([merge request](gitlab-org/security/gitlab!2415)) +- [Conan Token uses PAT rather than ID in payload](gitlab-org/security/gitlab@2679b802ac4cd9bd36190bcca691177c5568a981) ([merge request](gitlab-org/security/gitlab!2412)) +- [[security] Fix markdown API disclosing issue titles of limited projects](gitlab-org/security/gitlab@66088697787bcd55a727602da4f7fdd51b997eb0) ([merge request](gitlab-org/security/gitlab!2407)) +- [Verify that mentioned user can read TODO's note](gitlab-org/security/gitlab@fd166c1b4cc01e2bbbecabbab706deb423fa17f6) ([merge request](gitlab-org/security/gitlab!2397)) +- [Invalidate markdown cache to clear up stored XSS](gitlab-org/security/gitlab@0a0aee802c8b7760ffb0213e67129863d1769313) ([merge request](gitlab-org/security/gitlab!2418)) +- [Allow rate limiting of deploy tokens](gitlab-org/security/gitlab@8de550917a4b86a3ca3e132465d7d2c8394c4493) ([merge request](gitlab-org/security/gitlab!2395)) +- [Disable wiki access with CI_JOB_TOKEN when improper access level](gitlab-org/security/gitlab@516dbcd83cb2bbda6b15e22f4fafdaed661f4eb1) ([merge request](gitlab-org/security/gitlab!2408)) +- [Sanitize error input to prevent HTML/CSS injection in messages](gitlab-org/security/gitlab@c3f62e0f2965fe871463ed7a8b6e438cd2e1f515) ([merge request](gitlab-org/security/gitlab!2379)) +- [Secure debug trace artifact download](gitlab-org/security/gitlab@d889fb31417a8b8c38f73341da7576e856a96c5b) ([merge request](gitlab-org/security/gitlab!2376)) +- [Use password type for all secret integration properties](gitlab-org/security/gitlab@c4e2f9c3e86d832c143086f05fad382f6a218c50) ([merge request](gitlab-org/security/gitlab!2409)) +- [Limit CI job group_name regexp](gitlab-org/security/gitlab@9e3fbfce686aac48402a097c16616ffffe27c32f) ([merge request](gitlab-org/security/gitlab!2382)) + ## 14.10.0 (2022-04-21) ### Added (141 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index ffb9fc0450a..76b91118e08 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -14.10.0
\ No newline at end of file +14.10.1
\ No newline at end of file @@ -1 +1 @@ -14.10.0
\ No newline at end of file +14.10.1
\ No newline at end of file diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 62233c8c3c9..028b7af02c9 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -32,6 +32,21 @@ class Projects::ApplicationController < ApplicationController ->(project) { !project.pending_delete? } end + def authorize_read_build_trace! + return if can?(current_user, :read_build_trace, build) + + if build.debug_mode? + access_denied!( + _('You must have developer or higher permissions in the associated project to view job logs when debug trace ' \ + "is enabled. To disable debug trace, set the 'CI_DEBUG_TRACE' variable to 'false' in your pipeline " \ + 'configuration or CI/CD settings. If you need to view this job log, a project maintainer must add you to ' \ + 'the project with developer permissions or higher.') + ) + else + access_denied!(_('The current user is not authorized to access the job log.')) + end + end + def build_canonical_path(project) params[:namespace_id] = project.namespace.to_param params[:project_id] = project.to_param diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index feed94708f6..997d321ac24 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -9,6 +9,7 @@ class Projects::ArtifactsController < Projects::ApplicationController layout 'project' before_action :authorize_read_build! + before_action :authorize_read_build_trace!, only: [:download] before_action :authorize_update_build!, only: [:keep] before_action :authorize_destroy_artifacts!, only: [:destroy] before_action :extract_ref_name_and_path @@ -164,4 +165,10 @@ class Projects::ArtifactsController < Projects::ApplicationController render_404 unless @entry.exists? end + + def authorize_read_build_trace! + return unless params[:file_type] == 'trace' + + super + end end diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 4189419c3ba..0f6cf97d69d 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -177,17 +177,7 @@ class Projects::JobsController < Projects::ApplicationController private - def authorize_read_build_trace! - return if can?(current_user, :read_build_trace, @build) - - msg = _( - "You must have developer or higher permissions in the associated project to view job logs when debug trace is enabled. To disable debug trace, set the 'CI_DEBUG_TRACE' variable to 'false' in your pipeline configuration or CI/CD settings. " \ - "If you need to view this job log, a project maintainer must add you to the project with developer permissions or higher." - ) - return access_denied!(msg) if @build.debug_mode? - - access_denied!(_('The current user is not authorized to access the job log.')) - end + attr_reader :build def authorize_update_build! return access_denied! unless can?(current_user, :update_build, @build) diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb index ac94cc001dd..f6171403667 100644 --- a/app/controllers/projects/pipeline_schedules_controller.rb +++ b/app/controllers/projects/pipeline_schedules_controller.rb @@ -7,7 +7,8 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController before_action :authorize_play_pipeline_schedule!, only: [:play] before_action :authorize_read_pipeline_schedule! before_action :authorize_create_pipeline_schedule!, only: [:new, :create] - before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create, :play] + before_action :authorize_update_pipeline_schedule!, only: [:edit, :update] + before_action :authorize_take_ownership_pipeline_schedule!, only: [:take_ownership] before_action :authorize_admin_pipeline_schedule!, only: [:destroy] feature_category :continuous_integration @@ -108,6 +109,10 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController return access_denied! unless can?(current_user, :update_pipeline_schedule, schedule) end + def authorize_take_ownership_pipeline_schedule! + return access_denied! unless can?(current_user, :take_ownership_pipeline_schedule, schedule) + end + def authorize_admin_pipeline_schedule! return access_denied! unless can?(current_user, :admin_pipeline_schedule, schedule) end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 16c9aa212d0..a8ad55fd5a4 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -911,7 +911,10 @@ module Ci end end - cache + type_suffix = pipeline.protected_ref? ? 'protected' : 'non_protected' + cache.map do |entry| + entry.merge(key: "#{entry[:key]}-#{type_suffix}") + end end def credentials diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 08fed353755..ac9d8c39bd2 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -229,7 +229,13 @@ class CommitStatus < Ci::ApplicationRecord end def group_name - name.to_s.sub(%r{([\b\s:]+((\[.*\])|(\d+[\s:\/\\]+\d+)))+\s*\z}, '').strip + # [\b\s:] -> whitespace or column + # (\[.*\])|(\d+[\s:\/\\]+\d+) -> variables/matrix or parallel-jobs numbers + # {1,3} -> number of times that matches the variables/matrix or parallel-jobs numbers + # we limit this to 3 because of possible abuse + regex = %r{([\b\s:]+((\[.*\])|(\d+[\s:\/\\]+\d+))){1,3}\s*\z} + + name.to_s.sub(regex, '').strip end def failed_but_allowed? diff --git a/app/models/integrations/asana.rb b/app/models/integrations/asana.rb index e7634f51ec4..d25bf8b1b1e 100644 --- a/app/models/integrations/asana.rb +++ b/app/models/integrations/asana.rb @@ -27,10 +27,12 @@ module Integrations def fields [ { - type: 'text', + type: 'password', name: 'api_key', title: 'API key', help: s_('AsanaService|User Personal Access Token. User must have access to the task. All comments are attributed to this user.'), + non_empty_password_title: s_('ProjectService|Enter new API key'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current API key.'), # Example Personal Access Token from Asana docs placeholder: '0/68a9e79b868c6789e79a124c30b0', required: true diff --git a/app/models/integrations/assembla.rb b/app/models/integrations/assembla.rb index 6a36045330a..ccd24c1fb2c 100644 --- a/app/models/integrations/assembla.rb +++ b/app/models/integrations/assembla.rb @@ -19,8 +19,19 @@ module Integrations def fields [ - { type: 'text', name: 'token', placeholder: '', required: true }, - { type: 'text', name: 'subdomain', placeholder: '' } + { + type: 'password', + name: 'token', + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), + placeholder: '', + required: true + }, + { + type: 'text', + name: 'subdomain', + placeholder: '' + } ] end diff --git a/app/models/integrations/bamboo.rb b/app/models/integrations/bamboo.rb index c614a9415ab..b384a94d713 100644 --- a/app/models/integrations/bamboo.rb +++ b/app/models/integrations/bamboo.rb @@ -54,10 +54,12 @@ module Integrations required: true }, { - type: 'text', + type: 'password', name: 'build_key', - placeholder: s_('KEY'), help: s_('BambooService|Bamboo build plan key.'), + non_empty_password_title: s_('BambooService|Enter new build key'), + non_empty_password_help: s_('BambooService|Leave blank to use your current build key.'), + placeholder: s_('KEY'), required: true }, { diff --git a/app/models/integrations/base_slash_commands.rb b/app/models/integrations/base_slash_commands.rb index 1d271e75a91..a0ac5474893 100644 --- a/app/models/integrations/base_slash_commands.rb +++ b/app/models/integrations/base_slash_commands.rb @@ -26,7 +26,13 @@ module Integrations def fields [ - { type: 'text', name: 'token', placeholder: 'XXxxXXxxXXxxXXxxXXxxXXxx' } + { + type: 'password', + name: 'token', + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), + placeholder: 'XXxxXXxxXXxxXXxxXXxxXXxx' + } ] end diff --git a/app/models/integrations/buildkite.rb b/app/models/integrations/buildkite.rb index b816f90ef52..3b802271a36 100644 --- a/app/models/integrations/buildkite.rb +++ b/app/models/integrations/buildkite.rb @@ -76,10 +76,12 @@ module Integrations def fields [ - { type: 'text', + { type: 'password', name: 'token', title: _('Token'), help: s_('ProjectService|The token you get after you create a Buildkite pipeline with a GitLab repository.'), + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), required: true }, { type: 'text', diff --git a/app/models/integrations/campfire.rb b/app/models/integrations/campfire.rb index 81e6c2411b8..7889cd8f9a9 100644 --- a/app/models/integrations/campfire.rb +++ b/app/models/integrations/campfire.rb @@ -25,11 +25,13 @@ module Integrations def fields [ { - type: 'text', + type: 'password', name: 'token', title: _('Campfire token'), - placeholder: '', help: s_('CampfireService|API authentication token from Campfire.'), + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), + placeholder: '', required: true }, { diff --git a/app/models/integrations/drone_ci.rb b/app/models/integrations/drone_ci.rb index 3c18e5d8732..73f78bec381 100644 --- a/app/models/integrations/drone_ci.rb +++ b/app/models/integrations/drone_ci.rb @@ -96,8 +96,21 @@ module Integrations def fields [ - { type: 'text', name: 'token', help: s_('ProjectService|Token for the Drone project.'), required: true }, - { type: 'text', name: 'drone_url', title: s_('ProjectService|Drone server URL'), placeholder: 'http://drone.example.com', required: true } + { + type: 'password', + name: 'token', + help: s_('ProjectService|Token for the Drone project.'), + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), + required: true + }, + { + type: 'text', + name: 'drone_url', + title: s_('ProjectService|Drone server URL'), + placeholder: 'http://drone.example.com', + required: true + } ] end diff --git a/app/models/integrations/flowdock.rb b/app/models/integrations/flowdock.rb index 476cdc35585..703d8013bab 100644 --- a/app/models/integrations/flowdock.rb +++ b/app/models/integrations/flowdock.rb @@ -24,7 +24,15 @@ module Integrations def fields [ - { type: 'text', name: 'token', placeholder: s_('FlowdockService|1b609b52537...'), required: true, help: 'Enter your Flowdock token.' } + { + type: 'password', + name: 'token', + help: s_('FlowdockService|Enter your Flowdock token.'), + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), + placeholder: '1b609b52537...', + required: true + } ] end diff --git a/app/models/integrations/harbor.rb b/app/models/integrations/harbor.rb index 4c76e418886..6b561575f30 100644 --- a/app/models/integrations/harbor.rb +++ b/app/models/integrations/harbor.rb @@ -64,11 +64,12 @@ module Integrations required: true }, { - type: 'text', + type: 'password', name: 'password', title: s_('HarborIntegration|Harbor password'), - non_empty_password_title: s_('HarborIntegration|Enter Harbor password'), - non_empty_password_help: s_('HarborIntegration|Password for your Harbor username.'), + help: s_('HarborIntegration|Password for your Harbor username.'), + non_empty_password_title: s_('HarborIntegration|Enter new Harbor password'), + non_empty_password_help: s_('HarborIntegration|Leave blank to use your current password.'), required: true } ] diff --git a/app/models/integrations/packagist.rb b/app/models/integrations/packagist.rb index f616bc5faf2..738319ce835 100644 --- a/app/models/integrations/packagist.rb +++ b/app/models/integrations/packagist.rb @@ -36,10 +36,12 @@ module Integrations required: true }, { - type: 'text', + type: 'password', name: 'token', title: _('Token'), help: s_('Enter your Packagist token.'), + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), placeholder: '', required: true }, diff --git a/app/models/integrations/pivotaltracker.rb b/app/models/integrations/pivotaltracker.rb index 5b9ac023b7e..931ccf46655 100644 --- a/app/models/integrations/pivotaltracker.rb +++ b/app/models/integrations/pivotaltracker.rb @@ -27,9 +27,11 @@ module Integrations def fields [ { - type: 'text', + type: 'password', name: 'token', help: s_('PivotalTrackerService|Pivotal Tracker API token. User must have access to the story. All comments are attributed to this user.'), + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), required: true }, { diff --git a/app/models/integrations/pushover.rb b/app/models/integrations/pushover.rb index db39a4c68bd..7fd5efa8765 100644 --- a/app/models/integrations/pushover.rb +++ b/app/models/integrations/pushover.rb @@ -22,18 +22,22 @@ module Integrations def fields [ { - type: 'text', + type: 'password', name: 'api_key', title: _('API key'), help: s_('PushoverService|Enter your application key.'), + non_empty_password_title: s_('ProjectService|Enter new API key'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current API key.'), placeholder: '', required: true }, { - type: 'text', + type: 'password', name: 'user_key', title: _('User key'), help: s_('PushoverService|Enter your user key.'), + non_empty_password_title: s_('PushoverService|Enter new user key'), + non_empty_password_help: s_('PushoverService|Leave blank to use your current user key.'), placeholder: '', required: true }, diff --git a/app/models/issue.rb b/app/models/issue.rb index c2b8b457049..484cceb9129 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -640,7 +640,8 @@ class Issue < ApplicationRecord # Returns `true` if this Issue is visible to everybody. def publicly_visible? - project.public? && !confidential? && !hidden? && !::Gitlab::ExternalAuthorization.enabled? + project.public? && project.feature_available?(:issues, nil) && + !confidential? && !hidden? && !::Gitlab::ExternalAuthorization.enabled? end def expire_etag_cache diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb index b49e04f481c..3d56c563ec8 100644 --- a/app/models/packages/package_file.rb +++ b/app/models/packages/package_file.rb @@ -35,6 +35,7 @@ class Packages::PackageFile < ApplicationRecord validates :file_name, presence: true validates :file_name, uniqueness: { scope: :package }, if: -> { !pending_destruction? && package&.pypi? } + validates :file_sha256, format: { with: Gitlab::Regex.sha256_regex }, if: -> { package&.pypi? }, allow_nil: true scope :recent, -> { order(id: :desc) } scope :limit_recent, ->(limit) { recent.limit(limit) } diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index 33783d31355..27692fe76f0 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -105,7 +105,7 @@ class ProjectFeature < ApplicationRecord # that the user has access to the feature. It's important to use this scope with others # that checks project authorizations first (e.g. `filter_by_feature_visibility`). # - # This method uses an optimised version of `with_feature_access_level` for + # This method uses an optimized version of `with_feature_access_level` for # logged in users to more efficiently get private projects with the given # feature. def self.with_feature_available_for_user(feature, user) diff --git a/app/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb index 2ef5ffd6a5a..3a674bfef92 100644 --- a/app/policies/ci/pipeline_schedule_policy.rb +++ b/app/policies/ci/pipeline_schedule_policy.rb @@ -15,11 +15,14 @@ module Ci rule { can?(:create_pipeline) }.enable :play_pipeline_schedule rule { can?(:admin_pipeline) | (can?(:update_build) & owner_of_schedule) }.policy do - enable :update_pipeline_schedule enable :admin_pipeline_schedule enable :read_pipeline_schedule_variables end + rule { admin | (owner_of_schedule & can?(:update_build)) }.policy do + enable :update_pipeline_schedule + end + rule { can?(:admin_pipeline_schedule) & ~owner_of_schedule }.policy do enable :take_ownership_pipeline_schedule end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 64309c7f786..14cf264cc51 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -369,8 +369,6 @@ class TodoService end def reject_users_without_access(users, parent, target) - target = target.noteable if target.is_a?(Note) - if target.respond_to?(:to_ability_name) select_users(users, :"read_#{target.to_ability_name}", target) else diff --git a/doc/ci/caching/index.md b/doc/ci/caching/index.md index c634491662d..777bbf6053f 100644 --- a/doc/ci/caching/index.md +++ b/doc/ci/caching/index.md @@ -31,6 +31,7 @@ can't link to files outside it. - Subsequent pipelines can use the cache. - Subsequent jobs in the same pipeline can use the cache, if the dependencies are identical. - Different projects cannot share the cache. +- Protected and non-protected branches do not share the cache. ### Artifacts @@ -446,6 +447,22 @@ is stored on the machine where GitLab Runner is installed. The location also dep If you use cache and artifacts to store the same path in your jobs, the cache might be overwritten because caches are restored before artifacts. +### Segregation of caches between protected and non-protected branches + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/330047) in GitLab 15.0. + +A suffix is added to the cache key, with the exception of the [fallback cache key](#use-a-fallback-cache-key). +This is done in order to prevent cache poisoning that might occur through manipulation of the cache in a non-protected +branch. Any subsequent protected-branch jobs would then potentially use a poisoned cache from the preceding job. + +As an example, assuming that `cache.key` is set to `$CI_COMMIT_REF_SLUG`, and that we have two branches `main` +and `feature`, then the following table represents the resulting cache keys: + +| Branch name | Cache key | +|-------------|-----------| +| `main` | `main-protected` | +| `feature` | `feature-non_protected` | + ### How archiving and extracting works This example shows two jobs in two consecutive stages: diff --git a/doc/ci/jobs/index.md b/doc/ci/jobs/index.md index b8129e1cf18..e589fd8b045 100644 --- a/doc/ci/jobs/index.md +++ b/doc/ci/jobs/index.md @@ -167,7 +167,7 @@ The jobs are ordered by comparing the numbers from left to right. You usually want the first number to be the index and the second number to be the total. [This regular expression](https://gitlab.com/gitlab-org/gitlab/-/blob/2f3dc314f42dbd79813e6251792853bc231e69dd/app/models/commit_status.rb#L99) -evaluates the job names: `([\b\s:]+((\[.*\])|(\d+[\s:\/\\]+\d+)))+\s*\z`. +evaluates the job names: `([\b\s:]+((\[.*\])|(\d+[\s:\/\\]+\d+))){1,3}\s*\z`. One or more `: [...]`, `X Y`, `X/Y`, or `X\Y` sequences are removed from the **end** of job names only. Matching substrings found at the beginning or in the middle of job names are not removed. diff --git a/doc/ci/pipelines/schedules.md b/doc/ci/pipelines/schedules.md index 8813f3e1d59..8ab80e3798a 100644 --- a/doc/ci/pipelines/schedules.md +++ b/doc/ci/pipelines/schedules.md @@ -39,6 +39,20 @@ To add a pipeline schedule: These variables are available only when the scheduled pipeline runs, and not in any other pipeline run. +## Edit a pipeline schedule + +> Introduced in GitLab 14.8, only a pipeline schedule owner can edit the schedule. + +The owner of a pipeline schedule can edit it: + +1. On the top bar, select **Menu > Projects** and find your project. +1. In the left sidebar, select **CI/CD > Schedules**. +1. Next to the schedule, select **Edit** (**{pencil}**) and fill in the form. + +The user must have the Developer role or above for the project. If the user is +not the owner of the schedule, they must first [take ownership](#take-ownership) +of the schedule. + ## Run manually To trigger a pipeline schedule manually, so that it runs immediately instead of diff --git a/lib/api/ci/pipeline_schedules.rb b/lib/api/ci/pipeline_schedules.rb index 8a9ba2cbe0f..6030fe86f00 100644 --- a/lib/api/ci/pipeline_schedules.rb +++ b/lib/api/ci/pipeline_schedules.rb @@ -93,7 +93,7 @@ module API requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' end post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do - authorize! :update_pipeline_schedule, pipeline_schedule + authorize! :take_ownership_pipeline_schedule, pipeline_schedule if pipeline_schedule.own!(current_user) present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails diff --git a/lib/api/helpers/packages/conan/api_helpers.rb b/lib/api/helpers/packages/conan/api_helpers.rb index e92547890e8..994d3c4c473 100644 --- a/lib/api/helpers/packages/conan/api_helpers.rb +++ b/lib/api/helpers/packages/conan/api_helpers.rb @@ -153,7 +153,7 @@ module API def token strong_memoize(:token) do token = nil - token = ::Gitlab::ConanToken.from_personal_access_token(access_token) if access_token + token = ::Gitlab::ConanToken.from_personal_access_token(find_personal_access_token.user_id, access_token_from_request) if find_personal_access_token token = ::Gitlab::ConanToken.from_deploy_token(deploy_token_from_request) if deploy_token_from_request token = ::Gitlab::ConanToken.from_job(find_job_from_token) if find_job_from_token token @@ -224,9 +224,27 @@ module API forbidden! end + # We override this method from auth_finders because we need to + # extract the token from the Conan JWT which is specific to the Conan API def find_personal_access_token - find_personal_access_token_from_conan_jwt || - find_personal_access_token_from_http_basic_auth + strong_memoize(:find_personal_access_token) do + PersonalAccessToken.find_by_token(access_token_from_request) + end + end + + def access_token_from_request + strong_memoize(:access_token_from_request) do + find_personal_access_token_from_conan_jwt || + find_password_from_basic_auth + end + end + + def find_password_from_basic_auth + return unless route_authentication_setting[:basic_auth_personal_access_token] + return unless has_basic_credentials?(current_request) + + _username, password = user_name_and_password(current_request) + password end def find_user_from_job_token @@ -256,7 +274,7 @@ module API return unless token - PersonalAccessToken.find_by_id_and_user_id(token.access_token_id, token.user_id) + token.access_token_id end def find_deploy_token_from_conan_jwt diff --git a/lib/api/pypi_packages.rb b/lib/api/pypi_packages.rb index 86f36d4fc00..d4f51beb2e5 100644 --- a/lib/api/pypi_packages.rb +++ b/lib/api/pypi_packages.rb @@ -174,7 +174,7 @@ module API requires :version, type: String optional :requires_python, type: String optional :md5_digest, type: String - optional :sha256_digest, type: String + optional :sha256_digest, type: String, regexp: Gitlab::Regex.sha256_regex end route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth diff --git a/lib/gitlab/auth/request_authenticator.rb b/lib/gitlab/auth/request_authenticator.rb index 0948663b4b3..0d376cdbd2e 100644 --- a/lib/gitlab/auth/request_authenticator.rb +++ b/lib/gitlab/auth/request_authenticator.rb @@ -13,6 +13,10 @@ module Gitlab @request = request end + def find_authenticated_requester(request_formats) + user(request_formats) || deploy_token_from_request + end + def user(request_formats) request_formats.each do |format| user = find_sessionless_user(format) @@ -84,7 +88,8 @@ module Gitlab def route_authentication_setting @route_authentication_setting ||= { job_token_allowed: api_request?, - basic_auth_personal_access_token: api_request? || git_request? + basic_auth_personal_access_token: api_request? || git_request?, + deploy_token_allowed: api_request? || git_request? } end diff --git a/lib/gitlab/ci/pipeline/chain/helpers.rb b/lib/gitlab/ci/pipeline/chain/helpers.rb index 09158bf8bfd..343a189f773 100644 --- a/lib/gitlab/ci/pipeline/chain/helpers.rb +++ b/lib/gitlab/ci/pipeline/chain/helpers.rb @@ -6,25 +6,28 @@ module Gitlab module Chain module Helpers def error(message, config_error: false, drop_reason: nil) + sanitized_message = ActionController::Base.helpers.sanitize(message, tags: []) + if config_error drop_reason = :config_error - pipeline.yaml_errors = message + pipeline.yaml_errors = sanitized_message end - pipeline.add_error_message(message) + pipeline.add_error_message(sanitized_message) drop_pipeline!(drop_reason) # TODO: consider not to rely on AR errors directly as they can be # polluted with other unrelated errors (e.g. state machine) # https://gitlab.com/gitlab-org/gitlab/-/issues/220823 - pipeline.errors.add(:base, message) + pipeline.errors.add(:base, sanitized_message) pipeline.errors.full_messages end def warning(message) - pipeline.add_warning_message(message) + sanitized_message = ActionController::Base.helpers.sanitize(message, tags: []) + pipeline.add_warning_message(sanitized_message) end private diff --git a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb index 1c1f7abb6f6..035167f1a74 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb @@ -23,7 +23,7 @@ module Gitlab end unless allowed_to_write_ref? - error("You do not have sufficient permission to run a pipeline on '#{command.ref}'. Please select a different branch or contact your administrator for assistance. <a href=https://docs.gitlab.com/ee/ci/pipelines/#pipeline-security-on-protected-branches>Learn more</a>".html_safe) + error("You do not have sufficient permission to run a pipeline on '#{command.ref}'. Please select a different branch or contact your administrator for assistance.") end end diff --git a/lib/gitlab/conan_token.rb b/lib/gitlab/conan_token.rb index d0560807f45..87a085461bc 100644 --- a/lib/gitlab/conan_token.rb +++ b/lib/gitlab/conan_token.rb @@ -13,8 +13,8 @@ module Gitlab attr_reader :access_token_id, :user_id class << self - def from_personal_access_token(access_token) - new(access_token_id: access_token.id, user_id: access_token.user_id) + def from_personal_access_token(user_id, token) + new(access_token_id: token, user_id: user_id) end def from_job(job) diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb index f8f61511265..fdd7e8a8c4a 100644 --- a/lib/gitlab/git_access_wiki.rb +++ b/lib/gitlab/git_access_wiki.rb @@ -31,7 +31,8 @@ module Gitlab def check_download_access! super - raise ForbiddenError, download_forbidden_message if deploy_token && !deploy_token.can?(:download_wiki_code, container) + raise ForbiddenError, download_forbidden_message if build_cannot_download? + raise ForbiddenError, download_forbidden_message if deploy_token_cannot_download? end override :check_change_access! @@ -52,6 +53,17 @@ module Gitlab def not_found_message error_message(:not_found) end + + private + + # when accessing via the CI_JOB_TOKEN + def build_cannot_download? + build_can_download_code? && !user_access.can_do_action?(download_ability) + end + + def deploy_token_cannot_download? + deploy_token && !deploy_token.can?(download_ability, container) + end end end diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index fc05cc1a79c..7dcf26ca89a 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -639,7 +639,6 @@ included_attributes: - :build_allow_git_fetch - :build_coverage_regex - :build_timeout - - :ci_config_path - :delete_error - :description - :disable_overriding_approvers_per_merge_request diff --git a/lib/gitlab/import_export/project/relation_factory.rb b/lib/gitlab/import_export/project/relation_factory.rb index c391f86b47b..8110720fb46 100644 --- a/lib/gitlab/import_export/project/relation_factory.rb +++ b/lib/gitlab/import_export/project/relation_factory.rb @@ -87,6 +87,8 @@ module Gitlab when *BUILD_MODELS then setup_build when :issues then setup_issue when :'Ci::PipelineSchedule' then setup_pipeline_schedule + when :'ProtectedBranch::MergeAccessLevel' then setup_protected_branch_access_level + when :'ProtectedBranch::PushAccessLevel' then setup_protected_branch_access_level end update_project_references @@ -152,6 +154,10 @@ module Gitlab @relation_hash['active'] = false end + def setup_protected_branch_access_level + @relation_hash['access_level'] = Gitlab::Access::MAINTAINER + end + def compute_relative_position return unless max_relative_position diff --git a/lib/gitlab/markdown_cache.rb b/lib/gitlab/markdown_cache.rb index d6371732624..283502d90c1 100644 --- a/lib/gitlab/markdown_cache.rb +++ b/lib/gitlab/markdown_cache.rb @@ -11,8 +11,8 @@ module Gitlab # this if the change to the renderer output is a new feature or a # minor bug fix. # See: https://gitlab.com/gitlab-org/gitlab/-/issues/330313 - CACHE_COMMONMARK_VERSION = 29 - CACHE_COMMONMARK_VERSION_START = 10 + CACHE_COMMONMARK_VERSION = 30 + CACHE_COMMONMARK_VERSION_START = 10 BaseError = Class.new(StandardError) UnsupportedClassError = Class.new(BaseError) diff --git a/lib/gitlab/metrics/subscribers/rack_attack.rb b/lib/gitlab/metrics/subscribers/rack_attack.rb index d86c0f83c6c..70dcc6fad90 100644 --- a/lib/gitlab/metrics/subscribers/rack_attack.rb +++ b/lib/gitlab/metrics/subscribers/rack_attack.rb @@ -73,12 +73,16 @@ module Gitlab matched: req.env['rack.attack.matched'] } - if THROTTLES_WITH_USER_INFORMATION.include? req.env['rack.attack.matched'].to_sym - user_id = req.env['rack.attack.match_discriminator'] - user = User.find_by(id: user_id) # rubocop:disable CodeReuse/ActiveRecord + discriminator = req.env['rack.attack.match_discriminator'].to_s + discriminator_id = discriminator.split(':').last - rack_attack_info[:user_id] = user_id + if discriminator.starts_with?('user:') + user = User.find_by(id: discriminator_id) # rubocop:disable CodeReuse/ActiveRecord + + rack_attack_info[:user_id] = discriminator_id.to_i rack_attack_info['meta.user'] = user.username unless user.nil? + elsif discriminator.starts_with?('deploy_token:') + rack_attack_info[:deploy_token_id] = discriminator_id.to_i end Gitlab::InstrumentationHelper.add_instrumentation_data(rack_attack_info) diff --git a/lib/gitlab/rack_attack.rb b/lib/gitlab/rack_attack.rb index 3f4c0fa45aa..b2664f87306 100644 --- a/lib/gitlab/rack_attack.rb +++ b/lib/gitlab/rack_attack.rb @@ -95,7 +95,7 @@ module Gitlab authenticated_options = Gitlab::Throttle.options(throttle, authenticated: true) throttle_or_track(rack_attack, "throttle_authenticated_#{throttle}", authenticated_options) do |req| if req.throttle?(throttle, authenticated: true) - req.throttled_user_id([:api]) + req.throttled_identifer([:api]) end end end @@ -117,7 +117,7 @@ module Gitlab throttle_or_track(rack_attack, 'throttle_authenticated_web', Gitlab::Throttle.authenticated_web_options) do |req| if req.throttle_authenticated_web? - req.throttled_user_id([:api, :rss, :ics]) + req.throttled_identifer([:api, :rss, :ics]) end end @@ -129,19 +129,19 @@ module Gitlab throttle_or_track(rack_attack, 'throttle_authenticated_protected_paths_api', Gitlab::Throttle.protected_paths_options) do |req| if req.throttle_authenticated_protected_paths_api? - req.throttled_user_id([:api]) + req.throttled_identifer([:api]) end end throttle_or_track(rack_attack, 'throttle_authenticated_protected_paths_web', Gitlab::Throttle.protected_paths_options) do |req| if req.throttle_authenticated_protected_paths_web? - req.throttled_user_id([:api, :rss, :ics]) + req.throttled_identifer([:api, :rss, :ics]) end end throttle_or_track(rack_attack, 'throttle_authenticated_git_lfs', Gitlab::Throttle.throttle_authenticated_git_lfs_options) do |req| if req.throttle_authenticated_git_lfs? - req.throttled_user_id([:api]) + req.throttled_identifer([:api]) end end diff --git a/lib/gitlab/rack_attack/request.rb b/lib/gitlab/rack_attack/request.rb index b24afd28dd7..08a5ddb6ad1 100644 --- a/lib/gitlab/rack_attack/request.rb +++ b/lib/gitlab/rack_attack/request.rb @@ -9,18 +9,22 @@ module Gitlab GROUP_PATH_REGEX = %r{^/api/v\d+/groups/[^/]+/?$}.freeze def unauthenticated? - !(authenticated_user_id([:api, :rss, :ics]) || authenticated_runner_id) + !(authenticated_identifier([:api, :rss, :ics]) || authenticated_runner_id) end - def throttled_user_id(request_formats) - user_id = authenticated_user_id(request_formats) + def throttled_identifer(request_formats) + identifier = authenticated_identifier(request_formats) + return unless identifier - if Gitlab::RackAttack.user_allowlist.include?(user_id) + identifier_type = identifier[:identifier_type] + identifier_id = identifier[:identifier_id] + + if identifier_type == :user && Gitlab::RackAttack.user_allowlist.include?(identifier_id) Gitlab::Instrumentation::Throttle.safelist = 'throttle_user_allowlist' return end - user_id + "#{identifier_type}:#{identifier_id}" end def authenticated_runner_id @@ -169,8 +173,18 @@ module Gitlab private - def authenticated_user_id(request_formats) - request_authenticator.user(request_formats)&.id + def authenticated_identifier(request_formats) + requester = request_authenticator.find_authenticated_requester(request_formats) + + return unless requester + + identifier_type = if requester.is_a?(DeployToken) + :deploy_token + else + :user + end + + { identifier_type: identifier_type, identifier_id: requester.id } end def request_authenticator diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index c9202c6c54c..205106afddb 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -237,6 +237,10 @@ module Gitlab generic_package_name_regex end + def sha256_regex + @sha256_regex ||= /\A[0-9a-f]{64}\z/i.freeze + end + private def conan_name_regex diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b7c3378db63..fa3357c723f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5614,6 +5614,12 @@ msgstr "" msgid "BambooService|Bamboo service root URL." msgstr "" +msgid "BambooService|Enter new build key" +msgstr "" + +msgid "BambooService|Leave blank to use your current build key." +msgstr "" + msgid "BambooService|Run CI/CD pipelines with Atlassian Bamboo." msgstr "" @@ -16101,7 +16107,7 @@ msgstr "" msgid "FloC|Federated Learning of Cohorts" msgstr "" -msgid "FlowdockService|1b609b52537..." +msgid "FlowdockService|Enter your Flowdock token." msgstr "" msgid "FlowdockService|Send event notifications from GitLab to Flowdock flows." @@ -18357,7 +18363,7 @@ msgstr "" msgid "HarborIntegration|Base URL of the Harbor instance." msgstr "" -msgid "HarborIntegration|Enter Harbor password" +msgid "HarborIntegration|Enter new Harbor password" msgstr "" msgid "HarborIntegration|Harbor URL" @@ -18372,6 +18378,9 @@ msgstr "" msgid "HarborIntegration|Harbor username" msgstr "" +msgid "HarborIntegration|Leave blank to use your current password." +msgstr "" + msgid "HarborIntegration|Password for your Harbor username." msgstr "" @@ -29398,6 +29407,9 @@ msgstr "" msgid "ProjectService|Leave blank to use your current API key" msgstr "" +msgid "ProjectService|Leave blank to use your current API key." +msgstr "" + msgid "ProjectService|Leave blank to use your current password" msgstr "" @@ -30751,6 +30763,9 @@ msgstr "" msgid "PushoverService|%{user_name} pushed new branch \"%{ref}\"." msgstr "" +msgid "PushoverService|Enter new user key" +msgstr "" + msgid "PushoverService|Enter your application key." msgstr "" @@ -30766,6 +30781,9 @@ msgstr "" msgid "PushoverService|Leave blank for all active devices." msgstr "" +msgid "PushoverService|Leave blank to use your current user key." +msgstr "" + msgid "PushoverService|Low priority" msgstr "" diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index d51880b282d..958fcd4360c 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -204,6 +204,44 @@ RSpec.describe Projects::ArtifactsController do end end end + + context 'when downloading a debug trace' do + let(:file_type) { 'trace' } + let(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) } + + before do + create(:ci_job_variable, key: 'CI_DEBUG_TRACE', value: 'true', job: job) + end + + context 'when the user does not have update_build permissions' do + let(:user) { create(:user) } + + before do + project.add_guest(user) + end + + render_views + + it 'denies the user access' do + download_artifact(file_type: file_type) + + expect(response).to have_gitlab_http_status(:forbidden) + expect(response.body).to include( + 'You must have developer or higher permissions in the associated project to view job logs when debug trace is enabled. ' \ + 'To disable debug trace, set the 'CI_DEBUG_TRACE' variable to 'false' in your pipeline configuration or CI/CD settings. ' \ + 'If you need to view this job log, a project maintainer must add you to the project with developer permissions or higher.' + ) + end + end + + context 'when the user has update_build permissions' do + it 'sends the trace' do + download_artifact(file_type: file_type) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end end describe 'GET browse' do diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index d86f38c1f0b..77acd5fe13c 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -13,10 +13,43 @@ RSpec.describe Projects::PipelineSchedulesController do project.add_developer(user) end + shared_examples 'access update schedule' do + describe 'security' do + it 'is allowed for admin when admin mode enabled', :enable_admin_mode do + expect { go }.to be_allowed_for(:admin) + end + + it 'is denied for admin when admin mode disabled' do + expect { go }.to be_denied_for(:admin) + end + + it { expect { go }.to be_denied_for(:owner).of(project) } + it { expect { go }.to be_denied_for(:maintainer).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + it { expect { go }.to be_denied_for(:visitor) } + + context 'when user is schedule owner' do + it { expect { go }.to be_allowed_for(:owner).of(project).own(pipeline_schedule) } + it { expect { go }.to be_allowed_for(:maintainer).of(project).own(pipeline_schedule) } + it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:reporter).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:guest).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:user).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:external).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:visitor).own(pipeline_schedule) } + end + end + end + describe 'GET #index' do render_views let(:scope) { nil } + let!(:inactive_pipeline_schedule) do create(:ci_pipeline_schedule, :inactive, project: project) end @@ -130,12 +163,15 @@ RSpec.describe Projects::PipelineSchedulesController do it 'is allowed for admin when admin mode enabled', :enable_admin_mode do expect { go }.to be_allowed_for(:admin) end + it 'is denied for admin when admin mode disabled' do expect { go }.to be_denied_for(:admin) end + it { expect { go }.to be_allowed_for(:owner).of(project) } it { expect { go }.to be_allowed_for(:maintainer).of(project) } it { expect { go }.to be_allowed_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } it { expect { go }.to be_denied_for(:guest).of(project) } it { expect { go }.to be_denied_for(:user) } @@ -284,20 +320,7 @@ RSpec.describe Projects::PipelineSchedulesController do describe 'security' do let(:schedule) { { description: 'updated_desc' } } - it 'is allowed for admin when admin mode enabled', :enable_admin_mode do - expect { go }.to be_allowed_for(:admin) - end - it 'is denied for admin when admin mode disabled' do - expect { go }.to be_denied_for(:admin) - end - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:maintainer).of(project) } - it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - it { expect { go }.to be_denied_for(:visitor) } + it_behaves_like 'access update schedule' context 'when a developer created a pipeline schedule' do let(:developer_1) { create(:user) } @@ -308,8 +331,10 @@ RSpec.describe Projects::PipelineSchedulesController do end it { expect { go }.to be_allowed_for(developer_1) } + + it { expect { go }.to be_denied_for(:owner).of(project) } + it { expect { go }.to be_denied_for(:maintainer).of(project) } it { expect { go }.to be_denied_for(:developer).of(project) } - it { expect { go }.to be_allowed_for(:maintainer).of(project) } end context 'when a maintainer created a pipeline schedule' do @@ -321,17 +346,21 @@ RSpec.describe Projects::PipelineSchedulesController do end it { expect { go }.to be_allowed_for(maintainer_1) } - it { expect { go }.to be_allowed_for(:maintainer).of(project) } + + it { expect { go }.to be_denied_for(:owner).of(project) } + it { expect { go }.to be_denied_for(:maintainer).of(project) } it { expect { go }.to be_denied_for(:developer).of(project) } end end def go - put :update, params: { namespace_id: project.namespace.to_param, - project_id: project, - id: pipeline_schedule, - schedule: schedule }, - as: :html + put :update, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: pipeline_schedule, + schedule: schedule + }, + as: :html end end @@ -341,6 +370,7 @@ RSpec.describe Projects::PipelineSchedulesController do before do project.add_maintainer(user) + pipeline_schedule.update!(owner: user) sign_in(user) end @@ -352,22 +382,7 @@ RSpec.describe Projects::PipelineSchedulesController do end end - describe 'security' do - it 'is allowed for admin when admin mode enabled', :enable_admin_mode do - expect { go }.to be_allowed_for(:admin) - end - it 'is denied for admin when admin mode disabled' do - expect { go }.to be_denied_for(:admin) - end - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:maintainer).of(project) } - it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - it { expect { go }.to be_denied_for(:visitor) } - end + it_behaves_like 'access update schedule' def go get :edit, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id } @@ -379,17 +394,30 @@ RSpec.describe Projects::PipelineSchedulesController do it 'is allowed for admin when admin mode enabled', :enable_admin_mode do expect { go }.to be_allowed_for(:admin) end + it 'is denied for admin when admin mode disabled' do expect { go }.to be_denied_for(:admin) end + it { expect { go }.to be_allowed_for(:owner).of(project) } it { expect { go }.to be_allowed_for(:maintainer).of(project) } - it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:developer).of(project) } it { expect { go }.to be_denied_for(:reporter).of(project) } it { expect { go }.to be_denied_for(:guest).of(project) } it { expect { go }.to be_denied_for(:user) } it { expect { go }.to be_denied_for(:external) } it { expect { go }.to be_denied_for(:visitor) } + + context 'when user is schedule owner' do + it { expect { go }.to be_denied_for(:owner).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:maintainer).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:developer).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:reporter).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:guest).of(project).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:user).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:external).own(pipeline_schedule) } + it { expect { go }.to be_denied_for(:visitor).own(pipeline_schedule) } + end end def go diff --git a/spec/features/projects/pipeline_schedules_spec.rb b/spec/features/projects/pipeline_schedules_spec.rb index 63867a7e900..7cb14feabd2 100644 --- a/spec/features/projects/pipeline_schedules_spec.rb +++ b/spec/features/projects/pipeline_schedules_spec.rb @@ -9,7 +9,77 @@ RSpec.describe 'Pipeline Schedules', :js do let(:scope) { nil } let!(:user) { create(:user) } - context 'logged in as maintainer' do + context 'logged in as the pipeline scheduler owner' do + before do + stub_feature_flags(bootstrap_confirmation_modals: false) + project.add_developer(user) + pipeline_schedule.update!(owner: user) + gitlab_sign_in(user) + end + + describe 'GET /projects/pipeline_schedules' do + before do + visit_pipelines_schedules + end + + it 'edits the pipeline' do + page.within('.pipeline-schedule-table-row') do + click_link 'Edit' + end + + expect(page).to have_content('Edit Pipeline Schedule') + end + end + + describe 'PATCH /projects/pipelines_schedules/:id/edit' do + before do + edit_pipeline_schedule + end + + it 'displays existing properties' do + description = find_field('schedule_description').value + expect(description).to eq('pipeline schedule') + expect(page).to have_button('master') + expect(page).to have_button('UTC') + end + + it 'edits the scheduled pipeline' do + fill_in 'schedule_description', with: 'my brand new description' + + save_pipeline_schedule + + expect(page).to have_content('my brand new description') + end + + context 'when ref is nil' do + before do + pipeline_schedule.update_attribute(:ref, nil) + edit_pipeline_schedule + end + + it 'shows the pipeline schedule with default ref' do + page.within('[data-testid="schedule-target-ref"]') do + expect(first('.gl-new-dropdown-button-text').text).to eq('master') + end + end + end + + context 'when ref is empty' do + before do + pipeline_schedule.update_attribute(:ref, '') + edit_pipeline_schedule + end + + it 'shows the pipeline schedule with default ref' do + page.within('[data-testid="schedule-target-ref"]') do + expect(first('.gl-new-dropdown-button-text').text).to eq('master') + end + end + end + end + end + + context 'logged in as a project maintainer' do before do stub_feature_flags(bootstrap_confirmation_modals: false) project.add_maintainer(user) @@ -46,14 +116,6 @@ RSpec.describe 'Pipeline Schedules', :js do end end - it 'edits the pipeline' do - page.within('.pipeline-schedule-table-row') do - click_link 'Edit' - end - - expect(page).to have_content('Edit Pipeline Schedule') - end - it 'deletes the pipeline' do accept_confirm { click_link 'Delete' } @@ -108,53 +170,6 @@ RSpec.describe 'Pipeline Schedules', :js do end end - describe 'PATCH /projects/pipelines_schedules/:id/edit' do - before do - edit_pipeline_schedule - end - - it 'displays existing properties' do - description = find_field('schedule_description').value - expect(description).to eq('pipeline schedule') - expect(page).to have_button('master') - expect(page).to have_button('UTC') - end - - it 'edits the scheduled pipeline' do - fill_in 'schedule_description', with: 'my brand new description' - - save_pipeline_schedule - - expect(page).to have_content('my brand new description') - end - - context 'when ref is nil' do - before do - pipeline_schedule.update_attribute(:ref, nil) - edit_pipeline_schedule - end - - it 'shows the pipeline schedule with default ref' do - page.within('[data-testid="schedule-target-ref"]') do - expect(first('.gl-new-dropdown-button-text').text).to eq('master') - end - end - end - - context 'when ref is empty' do - before do - pipeline_schedule.update_attribute(:ref, '') - edit_pipeline_schedule - end - - it 'shows the pipeline schedule with default ref' do - page.within('[data-testid="schedule-target-ref"]') do - expect(first('.gl-new-dropdown-button-text').text).to eq('master') - end - end - end - end - context 'when user creates a new pipeline schedule with variables' do before do visit_pipelines_schedules diff --git a/spec/fixtures/lib/gitlab/import_export/complex/project.json b/spec/fixtures/lib/gitlab/import_export/complex/project.json index 95f2ce45b46..ce0df4250d6 100644 --- a/spec/fixtures/lib/gitlab/import_export/complex/project.json +++ b/spec/fixtures/lib/gitlab/import_export/complex/project.json @@ -4,6 +4,7 @@ "creator_id": 123, "visibility_level": 10, "archived": false, + "ci_config_path": "config/path", "labels": [ { "id": 2, diff --git a/spec/fixtures/lib/gitlab/import_export/complex/tree/project.json b/spec/fixtures/lib/gitlab/import_export/complex/tree/project.json index 2c5045ce806..9e03e3e51a2 100644 --- a/spec/fixtures/lib/gitlab/import_export/complex/tree/project.json +++ b/spec/fixtures/lib/gitlab/import_export/complex/tree/project.json @@ -6,5 +6,6 @@ "archived": false, "deploy_keys": [], "hooks": [], - "shared_runners_enabled": true + "shared_runners_enabled": true, + "ci_config_path": "config/path" } diff --git a/spec/lib/gitlab/auth/request_authenticator_spec.rb b/spec/lib/gitlab/auth/request_authenticator_spec.rb index 2bc80edb98c..0ce5e6a7f5c 100644 --- a/spec/lib/gitlab/auth/request_authenticator_spec.rb +++ b/spec/lib/gitlab/auth/request_authenticator_spec.rb @@ -76,6 +76,38 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do end end + describe '#find_authenticated_requester' do + let_it_be(:api_user) { create(:user) } + let_it_be(:deploy_token_user) { create(:user) } + + it 'returns the deploy token if it exists' do + allow_next_instance_of(described_class) do |authenticator| + expect(authenticator).to receive(:deploy_token_from_request).and_return(deploy_token_user) + allow(authenticator).to receive(:user).and_return(nil) + end + + expect(subject.find_authenticated_requester([:api])).to eq deploy_token_user + end + + it 'returns the user id if it exists' do + allow_next_instance_of(described_class) do |authenticator| + allow(authenticator).to receive(:deploy_token_from_request).and_return(deploy_token_user) + expect(authenticator).to receive(:user).and_return(api_user) + end + + expect(subject.find_authenticated_requester([:api])).to eq api_user + end + + it 'rerturns nil if no match is found' do + allow_next_instance_of(described_class) do |authenticator| + expect(authenticator).to receive(:deploy_token_from_request).and_return(nil) + expect(authenticator).to receive(:user).and_return(nil) + end + + expect(subject.find_authenticated_requester([:api])).to eq nil + end + end + describe '#find_sessionless_user' do let_it_be(:dependency_proxy_user) { build(:user) } let_it_be(:access_token_user) { build(:user) } @@ -380,10 +412,10 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do describe '#route_authentication_setting' do using RSpec::Parameterized::TableSyntax - where(:script_name, :expected_job_token_allowed, :expected_basic_auth_personal_access_token) do - '/api/endpoint' | true | true - '/namespace/project.git' | false | true - '/web/endpoint' | false | false + where(:script_name, :expected_job_token_allowed, :expected_basic_auth_personal_access_token, :expected_deploy_token_allowed) do + '/api/endpoint' | true | true | true + '/namespace/project.git' | false | true | true + '/web/endpoint' | false | false | false end with_them do @@ -394,7 +426,8 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do it 'returns correct settings' do expect(subject.send(:route_authentication_setting)).to eql({ job_token_allowed: expected_job_token_allowed, - basic_auth_personal_access_token: expected_basic_auth_personal_access_token + basic_auth_personal_access_token: expected_basic_auth_personal_access_token, + deploy_token_allowed: expected_deploy_token_allowed }) end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb index bcea6462790..96ada90b4e1 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb @@ -22,6 +22,19 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Helpers do let(:command) { double(save_incompleted: true) } let(:message) { 'message' } + describe '.warning' do + context 'when the warning includes malicious HTML' do + let(:message) { '<div>gimme your password</div>' } + let(:sanitized_message) { 'gimme your password' } + + it 'sanitizes' do + subject.warning(message) + + expect(pipeline.warning_messages[0].content).to include(sanitized_message) + end + end + end + describe '.error' do shared_examples 'error function' do specify do @@ -36,6 +49,18 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Helpers do end end + context 'when the error includes malicious HTML' do + let(:message) { '<div>gimme your password</div>' } + let(:sanitized_message) { 'gimme your password' } + + it 'sanitizes the error and removes the HTML tags' do + subject.error(message, config_error: true, drop_reason: :config_error) + + expect(pipeline.yaml_errors).to eq(sanitized_message) + expect(pipeline.errors[:base]).to include(sanitized_message) + end + end + context 'when given a drop reason' do context 'when config error is true' do context 'sets the yaml error and overrides the drop reason' do diff --git a/spec/lib/gitlab/conan_token_spec.rb b/spec/lib/gitlab/conan_token_spec.rb index b6180f69044..c8bda0a5cf0 100644 --- a/spec/lib/gitlab/conan_token_spec.rb +++ b/spec/lib/gitlab/conan_token_spec.rb @@ -25,13 +25,17 @@ RSpec.describe Gitlab::ConanToken do end describe '.from_personal_access_token' do - it 'sets access token id and user id' do - access_token = double(id: 123, user_id: 456) + it 'sets access token and user id and does not use the token id' do + personal_access_token = double(id: 999, token: 123, user_id: 456) - token = described_class.from_personal_access_token(access_token) + token = described_class.from_personal_access_token( + personal_access_token.user_id, + personal_access_token.token + ) - expect(token.access_token_id).to eq(123) - expect(token.user_id).to eq(456) + expect(token.access_token_id).not_to eq(personal_access_token.id) + expect(token.access_token_id).to eq(personal_access_token.token) + expect(token.user_id).to eq(personal_access_token.user_id) end end diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb index 27175dc8c44..de3e674c3a7 100644 --- a/spec/lib/gitlab/git_access_wiki_spec.rb +++ b/spec/lib/gitlab/git_access_wiki_spec.rb @@ -4,8 +4,9 @@ require 'spec_helper' RSpec.describe Gitlab::GitAccessWiki do let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, :wiki_repo) } - let_it_be(:wiki) { create(:project_wiki, project: project) } + let_it_be_with_reload(:project) { create(:project, :wiki_repo) } + + let(:wiki) { create(:project_wiki, project: project) } let(:changes) { ['6f6d7e7ed 570e7b2ab refs/heads/master'] } let(:authentication_abilities) { %i[read_project download_code push_code] } @@ -17,6 +18,61 @@ RSpec.describe Gitlab::GitAccessWiki do redirected_path: redirected_path) end + RSpec.shared_examples 'wiki access by level' do + where(:project_visibility, :project_member?, :wiki_access_level, :wiki_repo?, :expected_behavior) do + [ + # Private project - is a project member + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::ENABLED, true, :no_error], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::PRIVATE, true, :no_error], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::DISABLED, true, :forbidden_wiki], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::ENABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::DISABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::PRIVATE, false, :not_found_wiki], + # Private project - is NOT a project member + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::ENABLED, true, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::PRIVATE, true, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::DISABLED, true, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::ENABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::DISABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::PRIVATE, false, :not_found_wiki], + # Public project - is a project member + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::ENABLED, true, :no_error], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::PRIVATE, true, :no_error], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::DISABLED, true, :forbidden_wiki], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::ENABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::DISABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::PRIVATE, false, :not_found_wiki], + # Public project - is NOT a project member + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::ENABLED, true, :no_error], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::PRIVATE, true, :forbidden_wiki], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::DISABLED, true, :forbidden_wiki], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::ENABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::DISABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::PRIVATE, false, :not_found_wiki] + ] + end + + with_them do + before do + project.update!(visibility_level: project_visibility) + project.add_developer(user) if project_member? + project.project_feature.update_attribute(:wiki_access_level, wiki_access_level) + allow(wiki.repository).to receive(:exists?).and_return(wiki_repo?) + end + + it 'provides access by level' do + case expected_behavior + when :no_error + expect { subject }.not_to raise_error + when :forbidden_wiki + expect { subject }.to raise_wiki_forbidden + when :not_found_wiki + expect { subject }.to raise_wiki_not_found + end + end + end + end + describe '#push_access_check' do subject { access.check('git-receive-pack', changes) } @@ -28,56 +84,26 @@ RSpec.describe Gitlab::GitAccessWiki do it { expect { subject }.not_to raise_error } context 'when in a read-only GitLab instance' do - let(:message) { "You can't push code to a read-only GitLab instance." } - before do allow(Gitlab::Database).to receive(:read_only?) { true } end - it_behaves_like 'forbidden git access' + it_behaves_like 'forbidden git access' do + let(:message) { "You can't push code to a read-only GitLab instance." } + end end end context 'the user cannot :create_wiki' do - it_behaves_like 'not-found git access' do - let(:message) { 'The wiki you were looking for could not be found.' } - end + it { expect { subject }.to raise_wiki_not_found } end end describe '#check_download_access!' do subject { access.check('git-upload-pack', Gitlab::GitAccess::ANY) } - context 'the user can :download_wiki_code' do - before do - project.add_developer(user) - end - - context 'when wiki feature is disabled' do - before do - project.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED) - end - - it_behaves_like 'forbidden git access' do - let(:message) { include('wiki') } - end - end - - context 'when the repository does not exist' do - before do - allow(wiki.repository).to receive(:exists?).and_return(false) - end - - it_behaves_like 'not-found git access' do - let(:message) { include('for this wiki') } - end - end - end - - context 'the user cannot :download_wiki_code' do - it_behaves_like 'not-found git access' do - let(:message) { include('wiki') } - end + context 'when actor is a user' do + it_behaves_like 'wiki access by level' end context 'when the actor is a deploy token' do @@ -99,10 +125,40 @@ RSpec.describe Gitlab::GitAccessWiki do context 'when the wiki is disabled' do let(:wiki_access_level) { ProjectFeature::DISABLED } - it_behaves_like 'forbidden git access' do - let(:message) { 'You are not allowed to download files from this wiki.' } - end + it { expect { subject }.to raise_wiki_forbidden } end end + + describe 'when actor is a user provided by build via CI_JOB_TOKEN' do + let(:protocol) { 'http' } + let(:authentication_abilities) { [:build_download_code] } + let(:auth_result_type) { :build } + + before do + project.project_feature.update_attribute(:wiki_access_level, wiki_access_level) + end + + subject { access.check('git-upload-pack', changes) } + + it_behaves_like 'wiki access by level' + end + end + + RSpec::Matchers.define :raise_wiki_not_found do + match do |actual| + expect { actual.call }.to raise_error(Gitlab::GitAccess::NotFoundError, include('wiki')) + end + def supports_block_expectations? + true + end + end + + RSpec::Matchers.define :raise_wiki_forbidden do + match do |actual| + expect { subject }.to raise_error(Gitlab::GitAccess::ForbiddenError, include('wiki')) + end + def supports_block_expectations? + true + end end end diff --git a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb index 240d86077c4..52b33e22089 100644 --- a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb @@ -417,4 +417,22 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory, :use_clean_rails_ expect(created_object.project).to equal(project) end end + + context 'merge request access level object' do + let(:relation_sym) { :'ProtectedBranch::MergeAccessLevel' } + let(:relation_hash) { { 'access_level' => 30, 'created_at' => '2022-03-29T09:53:13.457Z', 'updated_at' => '2022-03-29T09:54:13.457Z' } } + + it 'sets access level to maintainer' do + expect(created_object.access_level).to equal(Gitlab::Access::MAINTAINER) + end + end + + context 'push access level object' do + let(:relation_sym) { :'ProtectedBranch::PushAccessLevel' } + let(:relation_hash) { { 'access_level' => 30, 'created_at' => '2022-03-29T09:53:13.457Z', 'updated_at' => '2022-03-29T09:54:13.457Z' } } + + it 'sets access level to maintainer' do + expect(created_object.access_level).to equal(Gitlab::Access::MAINTAINER) + end + end end diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb index fdf8260c058..a3e891db658 100644 --- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb @@ -111,6 +111,10 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do end end + it 'does not import ci config path' do + expect(@project.ci_config_path).to be_nil + end + it 'creates a valid pipeline note' do expect(Ci::Pipeline.find_by_sha('sha-notes').notes).not_to be_empty end diff --git a/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb b/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb index 2d595632772..fda4b94bd78 100644 --- a/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb @@ -91,72 +91,110 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do end end - context 'when matched throttle requires user information' do - context 'when user not found' do - let(:event) do - ActiveSupport::Notifications::Event.new( - event_name, Time.current, Time.current + 2.seconds, '1', request: double( - :request, - ip: '1.2.3.4', - request_method: 'GET', - fullpath: '/api/v4/internal/authorized_keys', - env: { - 'rack.attack.match_type' => match_type, - 'rack.attack.matched' => 'throttle_authenticated_api', - 'rack.attack.match_discriminator' => 'not_exist_user_id' - } + context 'matching user or deploy token authenticated information' do + context 'when matching for user' do + context 'when user not found' do + let(:event) do + ActiveSupport::Notifications::Event.new( + event_name, Time.current, Time.current + 2.seconds, '1', request: double( + :request, + ip: '1.2.3.4', + request_method: 'GET', + fullpath: '/api/v4/internal/authorized_keys', + env: { + 'rack.attack.match_type' => match_type, + 'rack.attack.matched' => 'throttle_authenticated_api', + 'rack.attack.match_discriminator' => "user:#{non_existing_record_id}" + } + ) ) - ) + end + + it 'logs request information and user id' do + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Rack_Attack', + env: match_type, + remote_ip: '1.2.3.4', + request_method: 'GET', + path: '/api/v4/internal/authorized_keys', + matched: 'throttle_authenticated_api', + user_id: non_existing_record_id + ) + ) + subscriber.send(match_type, event) + end end - it 'logs request information and user id' do - expect(Gitlab::AuthLogger).to receive(:error).with( - include( - message: 'Rack_Attack', - env: match_type, - remote_ip: '1.2.3.4', - request_method: 'GET', - path: '/api/v4/internal/authorized_keys', - matched: 'throttle_authenticated_api', - user_id: 'not_exist_user_id' + context 'when user found' do + let(:user) { create(:user) } + let(:event) do + ActiveSupport::Notifications::Event.new( + event_name, Time.current, Time.current + 2.seconds, '1', request: double( + :request, + ip: '1.2.3.4', + request_method: 'GET', + fullpath: '/api/v4/internal/authorized_keys', + env: { + 'rack.attack.match_type' => match_type, + 'rack.attack.matched' => 'throttle_authenticated_api', + 'rack.attack.match_discriminator' => "user:#{user.id}" + } + ) ) - ) - subscriber.send(match_type, event) + end + + it 'logs request information and user meta' do + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Rack_Attack', + env: match_type, + remote_ip: '1.2.3.4', + request_method: 'GET', + path: '/api/v4/internal/authorized_keys', + matched: 'throttle_authenticated_api', + user_id: user.id, + 'meta.user' => user.username + ) + ) + subscriber.send(match_type, event) + end end end - context 'when user found' do - let(:user) { create(:user) } - let(:event) do - ActiveSupport::Notifications::Event.new( - event_name, Time.current, Time.current + 2.seconds, '1', request: double( - :request, - ip: '1.2.3.4', - request_method: 'GET', - fullpath: '/api/v4/internal/authorized_keys', - env: { - 'rack.attack.match_type' => match_type, - 'rack.attack.matched' => 'throttle_authenticated_api', - 'rack.attack.match_discriminator' => user.id - } + context 'when matching for deploy token' do + context 'when deploy token found' do + let(:deploy_token) { create(:deploy_token) } + let(:event) do + ActiveSupport::Notifications::Event.new( + event_name, Time.current, Time.current + 2.seconds, '1', request: double( + :request, + ip: '1.2.3.4', + request_method: 'GET', + fullpath: '/api/v4/internal/authorized_keys', + env: { + 'rack.attack.match_type' => match_type, + 'rack.attack.matched' => 'throttle_authenticated_api', + 'rack.attack.match_discriminator' => "deploy_token:#{deploy_token.id}" + } + ) ) - ) - end - - it 'logs request information and user meta' do - expect(Gitlab::AuthLogger).to receive(:error).with( - include( - message: 'Rack_Attack', - env: match_type, - remote_ip: '1.2.3.4', - request_method: 'GET', - path: '/api/v4/internal/authorized_keys', - matched: 'throttle_authenticated_api', - user_id: user.id, - 'meta.user' => user.username + end + + it 'logs request information and user meta' do + expect(Gitlab::AuthLogger).to receive(:error).with( + include( + message: 'Rack_Attack', + env: match_type, + remote_ip: '1.2.3.4', + request_method: 'GET', + path: '/api/v4/internal/authorized_keys', + matched: 'throttle_authenticated_api', + deploy_token_id: deploy_token.id + ) ) - ) - subscriber.send(match_type, event) + subscriber.send(match_type, event) + end end end end diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index f3e8c440fba..b4c1f3b689b 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -1005,4 +1005,19 @@ RSpec.describe Gitlab::Regex do it { is_expected.not_to match('.xt.est_') } it { is_expected.not_to match('0test1') } end + + describe '.sha256_regex' do + subject { described_class.sha256_regex } + + it { is_expected.to match('a' * 64) } + it { is_expected.to match('abcdefABCDEF1234567890abcdefABCDEF1234567890abcdefABCDEF12345678') } + it { is_expected.not_to match('a' * 63) } + it { is_expected.not_to match('a' * 65) } + it { is_expected.not_to match('a' * 63 + 'g') } + it { is_expected.not_to match('a' * 63 + '{') } + it { is_expected.not_to match('a' * 63 + '%') } + it { is_expected.not_to match('a' * 63 + '*') } + it { is_expected.not_to match('a' * 63 + '#') } + it { is_expected.not_to match('') } + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index fd87a388442..12e65974270 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1048,7 +1048,27 @@ RSpec.describe Ci::Build do allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(1) end - it { is_expected.to match([a_hash_including(key: "key-1"), a_hash_including(key: "key2-1")]) } + it { is_expected.to match([a_hash_including(key: 'key-1-non_protected'), a_hash_including(key: 'key2-1-non_protected')]) } + + context 'when pipeline is on a protected ref' do + before do + allow(build.pipeline).to receive(:protected_ref?).and_return(true) + end + + it do + is_expected.to all(a_hash_including(key: a_string_matching(/-protected$/))) + end + end + + context 'when pipeline is not on a protected ref' do + before do + allow(build.pipeline).to receive(:protected_ref?).and_return(false) + end + + it do + is_expected.to all(a_hash_including(key: a_string_matching(/-non_protected$/))) + end + end end context 'when project has jobs_cache_index' do @@ -1056,7 +1076,7 @@ RSpec.describe Ci::Build do allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(1) end - it { is_expected.to be_an(Array).and all(include(key: "key-1")) } + it { is_expected.to be_an(Array).and all(include(key: a_string_matching(/^key-1-(?>protected|non_protected)/))) } end context 'when project does not have jobs_cache_index' do @@ -1064,7 +1084,9 @@ RSpec.describe Ci::Build do allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(nil) end - it { is_expected.to eq(options[:cache]) } + it do + is_expected.to eq(options[:cache].map { |entry| entry.merge(key: "#{entry[:key]}-non_protected") }) + end end end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 155e0fbb0e9..d158a99ef9f 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -618,6 +618,7 @@ RSpec.describe CommitStatus do 'rspec:windows 10000 20000' | 'rspec:windows' 'rspec:windows 0 : / 1' | 'rspec:windows' 'rspec:windows 0 : / 1 name' | 'rspec:windows 0 : / 1 name' + 'rspec [inception: [something, other thing], value]' | 'rspec' '0 1 name ruby' | '0 1 name ruby' '0 :/ 1 name ruby' | '0 :/ 1 name ruby' 'rspec: [aws]' | 'rspec' diff --git a/spec/models/integrations/every_integration_spec.rb b/spec/models/integrations/every_integration_spec.rb new file mode 100644 index 00000000000..33e89b3dabc --- /dev/null +++ b/spec/models/integrations/every_integration_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Every integration' do + all_integration_names = Integration.available_integration_names + + all_integration_names.each do |integration_name| + describe integration_name do + let(:integration_class) { Integration.integration_name_to_model(integration_name) } + let(:integration) { integration_class.new } + + context 'secret fields', :aggregate_failures do + it "uses type: 'password' for all secret fields" do + integration.fields.each do |field| + next unless Integrations::Field::SECRET_NAME.match?(field[:name]) + + expect(field[:type]).to eq('password'), + "Field '#{field[:name]}' should use type 'password'" + end + end + + it 'defines non-empty titles and help texts for all secret fields' do + integration.fields.each do |field| + next unless field[:type] == 'password' + + expect(field[:non_empty_password_title]).to be_present, + "Field '#{field[:name]}' should define :non_empty_password_title" + expect(field[:non_empty_password_help]).to be_present, + "Field '#{field[:name]}' should define :non_empty_password_help" + end + end + end + end + end +end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index fe09dadd0db..bd75d95080f 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -742,14 +742,15 @@ RSpec.describe Issue do describe '#participants' do context 'using a public project' do - let_it_be(:issue) { create(:issue, project: reusable_project) } + let_it_be(:public_project) { create(:project, :public) } + let_it_be(:issue) { create(:issue, project: public_project) } let!(:note1) do - create(:note_on_issue, noteable: issue, project: reusable_project, note: 'a') + create(:note_on_issue, noteable: issue, project: public_project, note: 'a') end let!(:note2) do - create(:note_on_issue, noteable: issue, project: reusable_project, note: 'b') + create(:note_on_issue, noteable: issue, project: public_project, note: 'b') end it 'includes the issue author' do @@ -819,20 +820,35 @@ RSpec.describe Issue do context 'without a user' do let(:user) { nil } - before do - project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PUBLIC) - end + context 'with issue available as public' do + before do + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PUBLIC) + end + + it 'returns true when the issue is publicly visible' do + expect(issue).to receive(:publicly_visible?).and_return(true) + + is_expected.to eq(true) + end - it 'returns true when the issue is publicly visible' do - expect(issue).to receive(:publicly_visible?).and_return(true) + it 'returns false when the issue is not publicly visible' do + expect(issue).to receive(:publicly_visible?).and_return(false) - is_expected.to eq(true) + is_expected.to eq(false) + end end - it 'returns false when the issue is not publicly visible' do - expect(issue).to receive(:publicly_visible?).and_return(false) + context 'with issues available only to team members in a public project' do + let(:public_project) { create(:project, :public) } + let(:issue) { build(:issue, project: public_project) } - is_expected.to eq(false) + before do + public_project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE) + end + + it 'returns false' do + is_expected.to eq(false) + end end end diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index f6af8f6a951..82f5b44f38f 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -29,19 +29,48 @@ RSpec.describe Packages::PackageFile, type: :model do let(:package_file) { package.package_files.first } let(:status) { :default } + let(:file_name) { 'foo' } let(:file) { fixture_file_upload('spec/fixtures/dk.png') } + let(:params) { { file: file, file_name: file_name, status: status } } - subject { package.package_files.create!(file: file, file_name: package_file.file_name, status: status) } + subject { package.package_files.create!(params) } - it 'can not save a duplicated file' do - expect { subject }.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: File name has already been taken") + context 'file_name' do + let(:file_name) { package_file.file_name } + + it 'can not save a duplicated file' do + expect { subject }.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: File name has already been taken") + end + + context 'with a pending destruction package duplicated file' do + let(:status) { :pending_destruction } + + it 'can save it' do + expect { subject }.to change { package.package_files.count }.from(1).to(2) + end + end end - context 'with a pending destruction package duplicated file' do - let(:status) { :pending_destruction } + context 'file_sha256' do + where(:sha256_value, :expected_success) do + 'a' * 64 | true + nil | true + 'a' * 63 | false + 'a' * 65 | false + 'a' * 63 + '%' | false + '' | false + end + + with_them do + let(:params) { super().merge({ file_sha256: sha256_value }) } - it 'can save it' do - expect { subject }.to change { package.package_files.count }.from(1).to(2) + it 'does not allow invalid sha256 characters' do + if expected_success + expect { subject }.not_to raise_error + else + expect { subject }.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: File sha256 is invalid") + end + end end end end diff --git a/spec/policies/ci/pipeline_schedule_policy_spec.rb b/spec/policies/ci/pipeline_schedule_policy_spec.rb index 1e36f455f6f..f2c99e0de95 100644 --- a/spec/policies/ci/pipeline_schedule_policy_spec.rb +++ b/spec/policies/ci/pipeline_schedule_policy_spec.rb @@ -84,11 +84,14 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models do project.add_maintainer(user) end - it 'includes abilities to do all operations on pipeline schedule' do + it 'allows for playing and destroying a pipeline schedule' do expect(policy).to be_allowed :play_pipeline_schedule - expect(policy).to be_allowed :update_pipeline_schedule expect(policy).to be_allowed :admin_pipeline_schedule end + + it 'does not allow for updating of an existing schedule' do + expect(policy).not_to be_allowed :update_pipeline_schedule + end end describe 'rules for non-owner of schedule' do diff --git a/spec/requests/api/ci/pipeline_schedules_spec.rb b/spec/requests/api/ci/pipeline_schedules_spec.rb index 4c8a356469d..5fb94976c5f 100644 --- a/spec/requests/api/ci/pipeline_schedules_spec.rb +++ b/spec/requests/api/ci/pipeline_schedules_spec.rb @@ -291,10 +291,36 @@ RSpec.describe API::Ci::PipelineSchedules do end context 'authenticated user with invalid permissions' do - it 'does not update pipeline_schedule' do - put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user) + context 'as a project maintainer' do + before do + project.add_maintainer(user) + end - expect(response).to have_gitlab_http_status(:not_found) + it 'does not update pipeline_schedule' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'as a project owner' do + before do + project.add_owner(user) + end + + it 'does not update pipeline_schedule' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'with no special role' do + it 'does not update pipeline_schedule' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end end end @@ -312,16 +338,21 @@ RSpec.describe API::Ci::PipelineSchedules do create(:ci_pipeline_schedule, project: project, owner: developer) end - context 'authenticated user with valid permissions' do + let(:project_maintainer) do + create(:user).tap { |u| project.add_maintainer(u) } + end + + context 'as an authenticated user with valid permissions' do it 'updates owner' do - post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", developer) + expect { post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", project_maintainer) } + .to change { pipeline_schedule.reload.owner }.from(developer).to(project_maintainer) expect(response).to have_gitlab_http_status(:created) expect(response).to match_response_schema('pipeline_schedule') end end - context 'authenticated user with invalid permissions' do + context 'as an authenticated user with invalid permissions' do it 'does not update owner' do post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", user) @@ -329,13 +360,23 @@ RSpec.describe API::Ci::PipelineSchedules do end end - context 'unauthenticated user' do + context 'as an unauthenticated user' do it 'does not update owner' do post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership") expect(response).to have_gitlab_http_status(:unauthorized) end end + + context 'as the existing owner of the schedule' do + it 'rejects the request and leaves the schedule unchanged' do + expect do + post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", developer) + end.not_to change { pipeline_schedule.reload.owner } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end end describe 'DELETE /projects/:id/pipeline_schedules/:pipeline_schedule_id' do diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index 9e6bac41d59..a662c77e5a2 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -191,7 +191,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end let(:expected_cache) do - [{ 'key' => 'cache_key', + [{ 'key' => a_string_matching(/^cache_key-(?>protected|non_protected)$/), 'untracked' => false, 'paths' => ['vendor/*'], 'policy' => 'pull-push', @@ -225,7 +225,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do 'alias' => nil, 'command' => nil, 'ports' => [], 'variables' => [{ 'key' => 'MYSQL_ROOT_PASSWORD', 'value' => 'root123.' }] }]) expect(json_response['steps']).to eq(expected_steps) expect(json_response['artifacts']).to eq(expected_artifacts) - expect(json_response['cache']).to eq(expected_cache) + expect(json_response['cache']).to match(expected_cache) expect(json_response['variables']).to include(*expected_variables) expect(json_response['features']).to match(expected_features) end diff --git a/spec/requests/api/markdown_spec.rb b/spec/requests/api/markdown_spec.rb index 0488bce4663..47e1f007daa 100644 --- a/spec/requests/api/markdown_spec.rb +++ b/spec/requests/api/markdown_spec.rb @@ -156,6 +156,46 @@ RSpec.describe API::Markdown do end end end + + context 'with a public project and issues only for team members' do + let(:public_project) do + create(:project, :public).tap do |project| + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE) + end + end + + let(:issue) { create(:issue, project: public_project, title: 'Team only title') } + let(:text) { "#{issue.to_reference}" } + let(:params) { { text: text, gfm: true, project: public_project.full_path } } + + shared_examples 'user without proper access' do + it 'does not render the title' do + expect(response).to have_gitlab_http_status(:created) + expect(json_response["html"]).not_to include('Team only title') + end + end + + context 'when not logged in' do + let(:user) { } + + it_behaves_like 'user without proper access' + end + + context 'when logged in as user without access' do + let(:user) { create(:user) } + + it_behaves_like 'user without proper access' + end + + context 'when logged in as author' do + let(:user) { issue.author } + + it 'renders the title or link' do + expect(response).to have_gitlab_http_status(:created) + expect(json_response["html"]).to include('Team only title') + end + end + end end end end diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index 078db4f1509..8fa5f409298 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -136,7 +136,7 @@ RSpec.describe API::PypiPackages do let(:url) { "/projects/#{project.id}/packages/pypi" } let(:headers) { {} } let(:requires_python) { '>=3.7' } - let(:base_params) { { requires_python: requires_python, version: '1.0.0', name: 'sample-project', sha256_digest: '123' } } + let(:base_params) { { requires_python: requires_python, version: '1.0.0', name: 'sample-project', sha256_digest: '1' * 64 } } let(:params) { base_params.merge(content: temp_file(file_name)) } let(:send_rewritten_field) { true } let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: user } } @@ -221,6 +221,19 @@ RSpec.describe API::PypiPackages do it_behaves_like 'returning response status', :bad_request end + context 'with an invalid sha256' do + let(:token) { personal_access_token.token } + let(:user_headers) { basic_auth_header(user.username, token) } + let(:headers) { user_headers.merge(workhorse_headers) } + + before do + params[:sha256_digest] = 'a' * 63 + '%' + project.add_developer(user) + end + + it_behaves_like 'returning response status', :bad_request + end + it_behaves_like 'deploy token for package uploads' it_behaves_like 'job token for package uploads' diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index f2126e3cf9c..115f78a5600 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -93,28 +93,28 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the OAuth headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in basic auth' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, basic_auth_headers(user, token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, basic_auth_headers(other_user, other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with a read_api scope' do @@ -127,14 +127,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the OAuth headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end end @@ -155,14 +155,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, oauth_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, oauth_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with a read_api scope' do @@ -171,7 +171,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(read_token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, oauth_token_headers(other_user_read_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end @@ -184,7 +184,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [rss_url(user), params: nil] } let(:other_user_request_args) { [rss_url(other_user), params: nil] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end @@ -288,14 +288,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end @@ -444,14 +444,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'precedence over authenticated api throttle' do @@ -512,6 +512,16 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac end end end + + context 'authenticated via deploy token headers' do + let(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true, projects: [project]) } + let(:other_deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } + + let(:request_args) { [api(api_partial_url), { headers: deploy_token_headers(deploy_token) }] } + let(:other_user_request_args) { [api(api_partial_url), { headers: deploy_token_headers(other_deploy_token) }] } + + it_behaves_like 'rate-limited deploy-token-authenticated requests' + end end end @@ -558,7 +568,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac end end - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'getting a blob' do @@ -568,7 +578,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:path) { "/v2/#{blob.group.path}/dependency_proxy/containers/alpine/blobs/sha256:a0d0a0d46f8b52473982a3c466318f479767577551a53ffc9074c9fa7035982e" } let(:other_path) { "/v2/#{other_blob.group.path}/dependency_proxy/containers/alpine/blobs/sha256:a0d0a0d46f8b52473982a3c466318f479767577551a53ffc9074c9fa7035982e" } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end end @@ -598,7 +608,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [git_lfs_url, { headers: basic_auth_headers(user, token) }] } let(:other_user_request_args) { [git_lfs_url, { headers: basic_auth_headers(other_user, other_user_token) }] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'precedence over authenticated web throttle' do @@ -786,14 +796,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'precedence over authenticated api throttle' do @@ -993,14 +1003,14 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac let(:request_args) { [api(path, personal_access_token: token), {}] } let(:other_user_request_args) { [api(path, personal_access_token: other_user_token), {}] } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'with the token in the headers' do let(:request_args) { api_get_args_with_token_headers(path, personal_access_token_headers(token)) } let(:other_user_request_args) { api_get_args_with_token_headers(path, personal_access_token_headers(other_user_token)) } - it_behaves_like 'rate-limited token-authenticated requests' + it_behaves_like 'rate-limited user based token-authenticated requests' end context 'precedence over authenticated api throttle' do diff --git a/spec/services/ci/create_pipeline_service/cache_spec.rb b/spec/services/ci/create_pipeline_service/cache_spec.rb index ca85a8cce64..fe777bc50d9 100644 --- a/spec/services/ci/create_pipeline_service/cache_spec.rb +++ b/spec/services/ci/create_pipeline_service/cache_spec.rb @@ -33,7 +33,7 @@ RSpec.describe Ci::CreatePipelineService do it 'uses the provided key' do expected = { - key: 'a-key', + key: a_string_matching(/^a-key-(?>protected|non_protected)$/), paths: ['logs/', 'binaries/'], policy: 'pull-push', untracked: true, diff --git a/spec/services/packages/pypi/create_package_service_spec.rb b/spec/services/packages/pypi/create_package_service_spec.rb index f84a77f80f7..354ac92b99a 100644 --- a/spec/services/packages/pypi/create_package_service_spec.rb +++ b/spec/services/packages/pypi/create_package_service_spec.rb @@ -7,6 +7,9 @@ RSpec.describe Packages::Pypi::CreatePackageService, :aggregate_failures do let_it_be(:project) { create(:project) } let_it_be(:user) { create(:user) } + let(:sha256) { '1' * 64 } + let(:md5) { '567' } + let(:requires_python) { '>=2.7' } let(:params) do { @@ -14,8 +17,8 @@ RSpec.describe Packages::Pypi::CreatePackageService, :aggregate_failures do version: '1.0', content: temp_file('foo.tgz'), requires_python: requires_python, - sha256_digest: '123', - md5_digest: '567' + sha256_digest: sha256, + md5_digest: md5 } end @@ -34,8 +37,8 @@ RSpec.describe Packages::Pypi::CreatePackageService, :aggregate_failures do expect(created_package.pypi_metadatum.required_python).to eq '>=2.7' expect(created_package.package_files.size).to eq 1 expect(created_package.package_files.first.file_name).to eq 'foo.tgz' - expect(created_package.package_files.first.file_sha256).to eq '123' - expect(created_package.package_files.first.file_md5).to eq '567' + expect(created_package.package_files.first.file_sha256).to eq sha256 + expect(created_package.package_files.first.file_md5).to eq md5 end end @@ -74,8 +77,8 @@ RSpec.describe Packages::Pypi::CreatePackageService, :aggregate_failures do context 'with an existing file' do before do params[:content] = temp_file('foo.tgz') - params[:sha256_digest] = 'abc' - params[:md5_digest] = 'def' + params[:sha256_digest] = sha256 + params[:md5_digest] = md5 end it 'throws an error' do @@ -101,8 +104,8 @@ RSpec.describe Packages::Pypi::CreatePackageService, :aggregate_failures do expect(created_package.pypi_metadatum.required_python).to eq '>=2.7' expect(created_package.package_files.size).to eq 1 expect(created_package.package_files.first.file_name).to eq 'foo.tgz' - expect(created_package.package_files.first.file_sha256).to eq 'abc' - expect(created_package.package_files.first.file_md5).to eq 'def' + expect(created_package.package_files.first.file_sha256).to eq sha256 + expect(created_package.package_files.first.file_md5).to eq md5 end end end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 6e10d0281b7..e4582e19416 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -391,6 +391,7 @@ RSpec.describe TodoService do let!(:second_todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) } let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee]) } let(:note) { create(:note, project: project, noteable: issue, author: john_doe, note: mentions) } + let(:confidential_note) { create(:note, :confidential, project: project, noteable: issue, author: john_doe, note: mentions) } let(:addressed_note) { create(:note, project: project, noteable: issue, author: john_doe, note: directly_addressed) } let(:note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: mentions) } let(:addressed_note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: directly_addressed) } @@ -468,6 +469,17 @@ RSpec.describe TodoService do should_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue) end + it 'does not create todo if user can not read confidential note' do + service.new_note(confidential_note, john_doe) + + should_not_create_todo(user: non_member, target: issue, author: john_doe, action: Todo::MENTIONED, note: confidential_note) + should_not_create_todo(user: guest, target: issue, author: john_doe, action: Todo::MENTIONED, note: confidential_note) + should_create_todo(user: member, target: issue, author: john_doe, action: Todo::MENTIONED, note: confidential_note) + should_create_todo(user: author, target: issue, author: john_doe, action: Todo::MENTIONED, note: confidential_note) + should_create_todo(user: assignee, target: issue, author: john_doe, action: Todo::MENTIONED, note: confidential_note) + should_create_todo(user: john_doe, target: issue, author: john_doe, action: Todo::MENTIONED, note: confidential_note) + end + context 'commits' do let(:base_commit_todo_attrs) { { target_id: nil, target_type: 'Commit', author: john_doe } } diff --git a/spec/support/helpers/packages_manager_api_spec_helper.rb b/spec/support/helpers/packages_manager_api_spec_helper.rb index 34e92c0595c..1c9fce183e9 100644 --- a/spec/support/helpers/packages_manager_api_spec_helper.rb +++ b/spec/support/helpers/packages_manager_api_spec_helper.rb @@ -3,7 +3,7 @@ module PackagesManagerApiSpecHelpers def build_jwt(personal_access_token, secret: jwt_secret, user_id: nil) JSONWebToken::HMACToken.new(secret).tap do |jwt| - jwt['access_token'] = personal_access_token.id + jwt['access_token'] = personal_access_token.token jwt['user_id'] = user_id || personal_access_token.user_id end end diff --git a/spec/support/helpers/rack_attack_spec_helpers.rb b/spec/support/helpers/rack_attack_spec_helpers.rb index c82a87dc58e..6c06781df03 100644 --- a/spec/support/helpers/rack_attack_spec_helpers.rb +++ b/spec/support/helpers/rack_attack_spec_helpers.rb @@ -10,11 +10,11 @@ module RackAttackSpecHelpers end def private_token_headers(user) - { 'HTTP_PRIVATE_TOKEN' => user.private_token } + { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => user.private_token } end def personal_access_token_headers(personal_access_token) - { 'HTTP_PRIVATE_TOKEN' => personal_access_token.token } + { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => personal_access_token.token } end def oauth_token_headers(oauth_access_token) @@ -26,6 +26,10 @@ module RackAttackSpecHelpers { 'AUTHORIZATION' => "Basic #{encoded_login}" } end + def deploy_token_headers(deploy_token) + basic_auth_headers(deploy_token, deploy_token) + end + def expect_rejection(name = nil, &block) yield diff --git a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb index 82c34f0d6ad..135fa4cf5a4 100644 --- a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb @@ -62,15 +62,8 @@ RSpec.shared_examples 'conan authenticate endpoint' do end end - it 'responds with 401 Unauthorized when an invalid access token ID is provided' do - jwt = build_jwt(double(id: 12345), user_id: personal_access_token.user_id) - get api(url), headers: build_token_auth_header(jwt.encoded) - - expect(response).to have_gitlab_http_status(:unauthorized) - end - - it 'responds with 401 Unauthorized when invalid user is provided' do - jwt = build_jwt(personal_access_token, user_id: 12345) + it 'responds with 401 Unauthorized when an invalid access token is provided' do + jwt = build_jwt(double(token: 12345), user_id: user.id) get api(url), headers: build_token_auth_header(jwt.encoded) expect(response).to have_gitlab_http_status(:unauthorized) @@ -102,7 +95,7 @@ RSpec.shared_examples 'conan authenticate endpoint' do payload = JSONWebToken::HMACToken.decode( response.body, jwt_secret).first - expect(payload['access_token']).to eq(personal_access_token.id) + expect(payload['access_token']).to eq(personal_access_token.token) expect(payload['user_id']).to eq(personal_access_token.user_id) duration = payload['exp'] - payload['iat'] diff --git a/spec/support/shared_examples/requests/rack_attack_shared_examples.rb b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb index c6c6c44dce8..68cb91d7414 100644 --- a/spec/support/shared_examples/requests/rack_attack_shared_examples.rb +++ b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb @@ -8,7 +8,50 @@ # * requests_per_period # * period_in_seconds # * period -RSpec.shared_examples 'rate-limited token-authenticated requests' do +RSpec.shared_examples 'rate-limited user based token-authenticated requests' do + context 'when the throttle is enabled' do + before do + settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true + stub_application_setting(settings_to_set) + end + + it 'does not reject requests if the user is in the allowlist' do + stub_env('GITLAB_THROTTLE_USER_ALLOWLIST', user.id.to_s) + Gitlab::RackAttack.configure_user_allowlist + + expect(Gitlab::Instrumentation::Throttle).to receive(:safelist=).with('throttle_user_allowlist').at_least(:once) + + (requests_per_period + 1).times do + make_request(request_args) + expect(response).not_to have_gitlab_http_status(:too_many_requests) + end + + stub_env('GITLAB_THROTTLE_USER_ALLOWLIST', nil) + Gitlab::RackAttack.configure_user_allowlist + end + end + + include_examples 'rate-limited token requests' do + let(:log_data) do + { + user_id: user.id, + 'meta.user' => user.username + } + end + end +end + +RSpec.shared_examples 'rate-limited deploy-token-authenticated requests' do + include_examples 'rate-limited token requests' do + let(:log_data) do + { + deploy_token_id: deploy_token.id + } + end + end +end + +RSpec.shared_examples 'rate-limited token requests' do let(:throttle_types) do { "throttle_protected_paths" => "throttle_authenticated_protected_paths_api", @@ -51,18 +94,6 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do expect_rejection { make_request(request_args) } end - it 'does not reject requests if the user is in the allowlist' do - stub_env('GITLAB_THROTTLE_USER_ALLOWLIST', user.id.to_s) - Gitlab::RackAttack.configure_user_allowlist - - expect(Gitlab::Instrumentation::Throttle).to receive(:safelist=).with('throttle_user_allowlist').at_least(:once) - - (requests_per_period + 1).times do - make_request(request_args) - expect(response).not_to have_gitlab_http_status(:too_many_requests) - end - end - it 'allows requests after throttling and then waiting for the next period' do requests_per_period.times do make_request(request_args) @@ -81,7 +112,7 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do end end - it 'counts requests from different users separately, even from the same IP' do + it 'counts requests from different requesters separately, even from the same IP' do requests_per_period.times do make_request(request_args) expect(response).not_to have_gitlab_http_status(:too_many_requests) @@ -92,7 +123,7 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do expect(response).not_to have_gitlab_http_status(:too_many_requests) end - it 'counts all requests from the same user, even via different IPs' do + it 'counts all requests from the same requesters, even via different IPs' do requests_per_period.times do make_request(request_args) expect(response).not_to have_gitlab_http_status(:too_many_requests) @@ -122,10 +153,8 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do remote_ip: '127.0.0.1', request_method: request_method, path: request_args.first, - user_id: user.id, - 'meta.user' => user.username, matched: throttle_types[throttle_setting_prefix] - }) + }.merge(log_data)) expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once |