diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-26 18:08:03 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-26 18:08:03 +0000 |
| commit | dc003cd08b4cb72fecbb03aa978ea0c53c03aeb4 (patch) | |
| tree | 5e77ce228c33619201ac6706b9789d4a2eed2a3b | |
| parent | e80e0dd64fbb04f60394cb1bb08e17dbcb22b8ce (diff) | |
| download | gitlab-ce-dc003cd08b4cb72fecbb03aa978ea0c53c03aeb4.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
94 files changed, 1802 insertions, 684 deletions
diff --git a/CHANGELOG-EE.md b/CHANGELOG-EE.md index e461983526c..2d1ef56eb69 100644 --- a/CHANGELOG-EE.md +++ b/CHANGELOG-EE.md @@ -1,5 +1,12 @@ Please view this file on the master branch, on stable branches it's out of date. +## 12.9.1 (2020-03-26) + +### Security (1 change) + +- Add NPM package versions SemVer validation. + + ## 12.9.0 (2020-03-22) ### Removed (1 change) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf9105114c0..f5d55efc24a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,32 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 12.9.1 (2020-03-26) + +### Security (16 changes) + +- Add permission check for pipeline status of MR. +- Ignore empty remote_id params from Workhorse accelerated uploads. +- External user can not create personal snippet through API. +- Prevent malicious entry for group name. +- Restrict mirroring changes to admins only when mirroring is disabled. +- Reject all container registry requests from blocked users. +- Deny localhost requests on fogbugz importer. +- Redact notes in moved confidential issues. +- Fix UploadRewriter Path Traversal vulnerability. +- Block hotlinking to repository archives. +- Restrict access to project pipeline metrics reports. +- vulnerability_feedback records should be restricted to a dev role and above. +- Exclude Carrierwave remote URL methods from import. +- Update Nokogiri to fix CVE-2020-7595. +- Prevent updating trigger by other maintainers. +- Fix XSS vulnerability in `admin/email` "Recipient Group" dropdown. + +### Fixed (1 change) + +- Fix updating the authorized_keys file. !27798 + + ## 12.9.0 (2020-03-22) ### Security (1 change) diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index ab7fcc3c4d6..3d6a12c1619 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -8.27.0 +8.28.0 diff --git a/Gemfile.lock b/Gemfile.lock index d050f628ab2..277eabf180d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -652,7 +652,7 @@ GEM netrc (0.11.0) nio4r (2.5.2) no_proxy_fix (0.1.2) - nokogiri (1.10.7) + nokogiri (1.10.8) mini_portile2 (~> 2.4.0) nokogumbo (1.5.0) nokogiri diff --git a/app/assets/javascripts/clusters/components/applications.vue b/app/assets/javascripts/clusters/components/applications.vue index f4d37c8c5f3..219825b1c01 100644 --- a/app/assets/javascripts/clusters/components/applications.vue +++ b/app/assets/javascripts/clusters/components/applications.vue @@ -107,8 +107,12 @@ export default { isProjectCluster() { return this.type === CLUSTER_TYPE.PROJECT; }, + managedAppsLocalTillerEnabled() { + return Boolean(gon.features?.managedAppsLocalTiller); + }, helmInstalled() { return ( + this.managedAppsLocalTillerEnabled || this.applications.helm.status === APPLICATION_STATUS.INSTALLED || this.applications.helm.status === APPLICATION_STATUS.UPDATED ); @@ -270,6 +274,7 @@ Crossplane runs inside your Kubernetes cluster and supports secure connectivity <div class="cluster-application-list prepend-top-10"> <application-row + v-if="!managedAppsLocalTillerEnabled" id="helm" :logo-url="helmLogo" :title="applications.helm.title" diff --git a/app/assets/javascripts/frequent_items/utils.js b/app/assets/javascripts/frequent_items/utils.js index a992480c22b..d4b7ffdcbe1 100644 --- a/app/assets/javascripts/frequent_items/utils.js +++ b/app/assets/javascripts/frequent_items/utils.js @@ -45,8 +45,19 @@ export const updateExistingFrequentItem = (frequentItem, item) => { }; }; -export const sanitizeItem = item => ({ - ...item, - name: sanitize(item.name.toString(), { allowedTags: [] }), - namespace: sanitize(item.namespace.toString(), { allowedTags: [] }), -}); +export const sanitizeItem = item => { + // Only sanitize if the key exists on the item + const maybeSanitize = key => { + if (!Object.prototype.hasOwnProperty.call(item, key)) { + return {}; + } + + return { [key]: sanitize(item[key].toString(), { allowedTags: [] }) }; + }; + + return { + ...item, + ...maybeSanitize('name'), + ...maybeSanitize('namespace'), + }; +}; diff --git a/app/assets/javascripts/pages/projects/snippets/show/index.js b/app/assets/javascripts/pages/projects/snippets/show/index.js index e49d46ea97b..f955a41e18a 100644 --- a/app/assets/javascripts/pages/projects/snippets/show/index.js +++ b/app/assets/javascripts/pages/projects/snippets/show/index.js @@ -1,19 +1 @@ -import initNotes from '~/init_notes'; -import ZenMode from '~/zen_mode'; -import LineHighlighter from '~/line_highlighter'; -import BlobViewer from '~/blob/viewer'; -import snippetEmbed from '~/snippet/snippet_embed'; -import { SnippetShowInit } from '~/snippets'; - -document.addEventListener('DOMContentLoaded', () => { - if (!gon.features.snippetsVue) { - new LineHighlighter(); // eslint-disable-line no-new - new BlobViewer(); // eslint-disable-line no-new - initNotes(); - new ZenMode(); // eslint-disable-line no-new - snippetEmbed(); - } else { - SnippetShowInit(); - initNotes(); - } -}); +import '~/snippet/snippet_show'; diff --git a/app/assets/javascripts/pages/snippets/show/index.js b/app/assets/javascripts/pages/snippets/show/index.js index 9a463b4762b..f955a41e18a 100644 --- a/app/assets/javascripts/pages/snippets/show/index.js +++ b/app/assets/javascripts/pages/snippets/show/index.js @@ -1,19 +1 @@ -import LineHighlighter from '~/line_highlighter'; -import BlobViewer from '~/blob/viewer'; -import ZenMode from '~/zen_mode'; -import initNotes from '~/init_notes'; -import snippetEmbed from '~/snippet/snippet_embed'; -import { SnippetShowInit } from '~/snippets'; - -document.addEventListener('DOMContentLoaded', () => { - if (!gon.features.snippetsVue) { - new LineHighlighter(); // eslint-disable-line no-new - new BlobViewer(); // eslint-disable-line no-new - initNotes(); - new ZenMode(); // eslint-disable-line no-new - snippetEmbed(); - } else { - SnippetShowInit(); - initNotes(); - } -}); +import '~/snippet/snippet_show'; diff --git a/app/assets/javascripts/snippet/snippet_show.js b/app/assets/javascripts/snippet/snippet_show.js new file mode 100644 index 00000000000..9a463b4762b --- /dev/null +++ b/app/assets/javascripts/snippet/snippet_show.js @@ -0,0 +1,19 @@ +import LineHighlighter from '~/line_highlighter'; +import BlobViewer from '~/blob/viewer'; +import ZenMode from '~/zen_mode'; +import initNotes from '~/init_notes'; +import snippetEmbed from '~/snippet/snippet_embed'; +import { SnippetShowInit } from '~/snippets'; + +document.addEventListener('DOMContentLoaded', () => { + if (!gon.features.snippetsVue) { + new LineHighlighter(); // eslint-disable-line no-new + new BlobViewer(); // eslint-disable-line no-new + initNotes(); + new ZenMode(); // eslint-disable-line no-new + snippetEmbed(); + } else { + SnippetShowInit(); + initNotes(); + } +}); diff --git a/app/controllers/clusters/base_controller.rb b/app/controllers/clusters/base_controller.rb index 188805c6106..8c13cc67be2 100644 --- a/app/controllers/clusters/base_controller.rb +++ b/app/controllers/clusters/base_controller.rb @@ -6,6 +6,10 @@ class Clusters::BaseController < ApplicationController skip_before_action :authenticate_user! before_action :authorize_read_cluster! + before_action do + push_frontend_feature_flag(:managed_apps_local_tiller) + end + helper_method :clusterable private diff --git a/app/controllers/concerns/hotlink_interceptor.rb b/app/controllers/concerns/hotlink_interceptor.rb new file mode 100644 index 00000000000..712a10eab98 --- /dev/null +++ b/app/controllers/concerns/hotlink_interceptor.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module HotlinkInterceptor + extend ActiveSupport::Concern + + def intercept_hotlinking! + return render_406 if Gitlab::HotlinkingDetector.intercept_hotlinking?(request) + end + + private + + def render_406 + head :not_acceptable + end +end diff --git a/app/controllers/import/fogbugz_controller.rb b/app/controllers/import/fogbugz_controller.rb index 28ead8d44da..4fb6efde7ff 100644 --- a/app/controllers/import/fogbugz_controller.rb +++ b/app/controllers/import/fogbugz_controller.rb @@ -3,6 +3,7 @@ class Import::FogbugzController < Import::BaseController before_action :verify_fogbugz_import_enabled before_action :user_map, only: [:new_user_map, :create_user_map] + before_action :verify_blocked_uri, only: :callback rescue_from Fogbugz::AuthenticationException, with: :fogbugz_unauthorized @@ -106,4 +107,21 @@ class Import::FogbugzController < Import::BaseController def verify_fogbugz_import_enabled render_404 unless fogbugz_import_enabled? end + + def verify_blocked_uri + Gitlab::UrlBlocker.validate!( + params[:uri], + { + allow_localhost: allow_local_requests?, + allow_local_network: allow_local_requests?, + schemes: %w(http https) + } + ) + rescue Gitlab::UrlBlocker::BlockedUrlError => e + redirect_to new_import_fogbugz_url, alert: _('Specified URL cannot be used: "%{reason}"') % { reason: e.message } + end + + def allow_local_requests? + Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services? + end end diff --git a/app/controllers/projects/mirrors_controller.rb b/app/controllers/projects/mirrors_controller.rb index dd1ea151de7..936f89e58e7 100644 --- a/app/controllers/projects/mirrors_controller.rb +++ b/app/controllers/projects/mirrors_controller.rb @@ -67,7 +67,7 @@ class Projects::MirrorsController < Projects::ApplicationController end def check_mirror_available! - Gitlab::CurrentSettings.current_application_settings.mirror_available || current_user&.admin? + render_404 unless can?(current_user, :admin_remote_mirror, project) end def mirror_params_attributes diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb index 1cb9e1d2c9b..303326bbe23 100644 --- a/app/controllers/projects/repositories_controller.rb +++ b/app/controllers/projects/repositories_controller.rb @@ -4,12 +4,14 @@ class Projects::RepositoriesController < Projects::ApplicationController include ExtractsPath include StaticObjectExternalStorage include Gitlab::RateLimitHelpers + include HotlinkInterceptor prepend_before_action(only: [:archive]) { authenticate_sessionless_user!(:archive) } # Authorize before_action :require_non_empty_project, except: :create before_action :archive_rate_limit!, only: :archive + before_action :intercept_hotlinking!, only: :archive before_action :assign_archive_vars, only: :archive before_action :assign_append_sha, only: :archive before_action :authorize_download_code! diff --git a/app/models/group.rb b/app/models/group.rb index da69f7cc11e..5e6e3032251 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -70,6 +70,9 @@ class Group < Namespace validates :variables, variable_duplicates: true validates :two_factor_grace_period, presence: true, numericality: { greater_than_or_equal_to: 0 } + validates :name, + format: { with: Gitlab::Regex.group_name_regex, + message: Gitlab::Regex.group_name_regex_message } add_authentication_token_field :runners_token, encrypted: -> { Feature.enabled?(:groups_tokens_optional_encryption, default_enabled: true) ? :optional : :required } diff --git a/app/models/issue.rb b/app/models/issue.rb index 0f00a78c728..f414be51065 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -326,10 +326,8 @@ class Issue < ApplicationRecord true elsif project.owner == user true - elsif confidential? - author == user || - assignees.include?(user) || - project.team.member?(user, Gitlab::Access::REPORTER) + elsif confidential? && !assignee_or_author?(user) + project.team.member?(user, Gitlab::Access::REPORTER) else project.public? || project.internal? && !user.external? || diff --git a/app/serializers/merge_request_poll_widget_entity.rb b/app/serializers/merge_request_poll_widget_entity.rb index a45026ea016..18e8ec0e7d1 100644 --- a/app/serializers/merge_request_poll_widget_entity.rb +++ b/app/serializers/merge_request_poll_widget_entity.rb @@ -53,7 +53,7 @@ class MergeRequestPollWidgetEntity < Grape::Entity # CI related expose :has_ci?, as: :has_ci - expose :ci_status do |merge_request| + expose :ci_status, if: -> (mr, _) { presenter(mr).can_read_pipeline? } do |merge_request| presenter(merge_request).ci_status end diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index 0e83932ff26..7db1b29ca9a 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -318,7 +318,7 @@ module ObjectStorage def cache!(new_file = sanitized_file) # We intercept ::UploadedFile which might be stored on remote storage # We use that for "accelerated" uploads, where we store result on remote storage - if new_file.is_a?(::UploadedFile) && new_file.remote_id + if new_file.is_a?(::UploadedFile) && new_file.remote_id.present? return cache_remote_file!(new_file.remote_id, new_file.original_filename) end diff --git a/app/views/projects/mirrors/_mirror_repos.html.haml b/app/views/projects/mirrors/_mirror_repos.html.haml index 80d2d2afada..4004c4f4b07 100644 --- a/app/views/projects/mirrors/_mirror_repos.html.haml +++ b/app/views/projects/mirrors/_mirror_repos.html.haml @@ -1,7 +1,9 @@ - expanded = expanded_by_default? - protocols = Gitlab::UrlSanitizer::ALLOWED_SCHEMES.join('|') +- mirror_settings_enabled = can?(current_user, :admin_remote_mirror, @project) +- mirror_settings_class = "#{'expanded' if expanded} #{'js-mirror-settings' if mirror_settings_enabled}".strip -%section.settings.project-mirror-settings.js-mirror-settings.no-animate#js-push-remote-settings{ class: ('expanded' if expanded), data: { qa_selector: 'mirroring_repositories_settings_section' } } +%section.settings.project-mirror-settings.no-animate#js-push-remote-settings{ class: mirror_settings_class, data: { qa_selector: 'mirroring_repositories_settings_section' } } .settings-header %h4= _('Mirroring repositories') %button.btn.js-settings-toggle @@ -11,26 +13,32 @@ = link_to _('Read more'), help_page_path('workflow/repository_mirroring'), target: '_blank' .settings-content - = form_for @project, url: project_mirror_path(@project), html: { class: 'gl-show-field-errors js-mirror-form', autocomplete: 'new-password', data: mirrors_form_data_attributes } do |f| - .panel.panel-default - .panel-body - %div= form_errors(@project) + - if mirror_settings_enabled + = form_for @project, url: project_mirror_path(@project), html: { class: 'gl-show-field-errors js-mirror-form', autocomplete: 'new-password', data: mirrors_form_data_attributes } do |f| + .panel.panel-default + .panel-body + %div= form_errors(@project) - .form-group.has-feedback - = label_tag :url, _('Git repository URL'), class: 'label-light' - = text_field_tag :url, nil, class: 'form-control js-mirror-url js-repo-url qa-mirror-repository-url-input', placeholder: _('Input your repository URL'), required: true, pattern: "(#{protocols}):\/\/.+", autocomplete: 'new-password' + .form-group.has-feedback + = label_tag :url, _('Git repository URL'), class: 'label-light' + = text_field_tag :url, nil, class: 'form-control js-mirror-url js-repo-url qa-mirror-repository-url-input', placeholder: _('Input your repository URL'), required: true, pattern: "(#{protocols}):\/\/.+", autocomplete: 'new-password' - = render 'projects/mirrors/instructions' + = render 'projects/mirrors/instructions' - = render 'projects/mirrors/mirror_repos_form', f: f + = render 'projects/mirrors/mirror_repos_form', f: f - .form-check.append-bottom-10 - = check_box_tag :only_protected_branches, '1', false, class: 'js-mirror-protected form-check-input' - = label_tag :only_protected_branches, _('Only mirror protected branches'), class: 'form-check-label' - = link_to icon('question-circle'), help_page_path('user/project/protected_branches'), target: '_blank' + .form-check.append-bottom-10 + = check_box_tag :only_protected_branches, '1', false, class: 'js-mirror-protected form-check-input' + = label_tag :only_protected_branches, _('Only mirror protected branches'), class: 'form-check-label' + = link_to icon('question-circle'), help_page_path('user/project/protected_branches'), target: '_blank' - .panel-footer - = f.submit _('Mirror repository'), class: 'btn btn-success js-mirror-submit qa-mirror-repository-button', name: :update_remote_mirror + .panel-footer + = f.submit _('Mirror repository'), class: 'btn btn-success js-mirror-submit qa-mirror-repository-button', name: :update_remote_mirror + - else + .gl-alert.gl-alert-info{ role: 'alert' } + = sprite_icon('information-o', size: 16, css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title') + .gl-alert-body + = _('Mirror settings are only available to GitLab administrators.') .panel.panel-default .table-responsive @@ -61,8 +69,10 @@ - if mirror.last_error.present? .badge.mirror-error-badge{ data: { toggle: 'tooltip', html: 'true', qa_selector: 'mirror_error_badge' }, title: html_escape(mirror.last_error.try(:strip)) }= _('Error') %td - .btn-group.mirror-actions-group.pull-right{ role: 'group' } - - if mirror.ssh_key_auth? - = clipboard_button(text: mirror.ssh_public_key, class: 'btn btn-default', title: _('Copy SSH public key'), qa_selector: 'copy_public_key_button') - = render 'shared/remote_mirror_update_button', remote_mirror: mirror + - if mirror_settings_enabled + .btn-group.mirror-actions-group.pull-right{ role: 'group' } + - if mirror.ssh_key_auth? + = clipboard_button(text: mirror.ssh_public_key, class: 'btn btn-default', title: _('Copy SSH public key'), qa_selector: 'copy_public_key_button') + = render 'shared/remote_mirror_update_button', remote_mirror: mirror %button.js-delete-mirror.qa-delete-mirror.rspec-delete-mirror.btn.btn-danger{ type: 'button', data: { mirror_id: mirror.id, toggle: 'tooltip', container: 'body' }, title: _('Remove') }= icon('trash-o') + diff --git a/changelogs/unreleased/212178-fix-authorized-keys-worker.yml b/changelogs/unreleased/212178-fix-authorized-keys-worker.yml deleted file mode 100644 index a95f2e0e71a..00000000000 --- a/changelogs/unreleased/212178-fix-authorized-keys-worker.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix updating the authorized_keys file -merge_request: 27798 -author: -type: fixed diff --git a/changelogs/unreleased/fix-deploy-token-optional-attributes.yml b/changelogs/unreleased/fix-deploy-token-optional-attributes.yml new file mode 100644 index 00000000000..a57f8eae541 --- /dev/null +++ b/changelogs/unreleased/fix-deploy-token-optional-attributes.yml @@ -0,0 +1,5 @@ +--- +title: Fix optional params for deploy token API +merge_request: 27961 +author: Nejc Habjan +type: fixed diff --git a/config/initializers/fill_shards.rb b/config/initializers/fill_shards.rb index cad662e12f3..40a9a271953 100644 --- a/config/initializers/fill_shards.rb +++ b/config/initializers/fill_shards.rb @@ -1,5 +1,8 @@ # The `table_exists?` check is needed because during our migration rollback testing, # `Shard.connected?` could be cached and return true even though the table doesn't exist -if Shard.connected? && Shard.table_exists? && !Gitlab::Database.read_only? - Shard.populate! -end +return unless Shard.connected? +return unless Shard.table_exists? +return unless Shard.connection.index_exists?(:shards, :name, unique: true) +return if Gitlab::Database.read_only? + +Shard.populate! diff --git a/doc/user/packages/container_registry/img/container_registry_repositories_v12_10.png b/doc/user/packages/container_registry/img/container_registry_repositories_v12_10.png Binary files differnew file mode 100644 index 00000000000..9e113be0a26 --- /dev/null +++ b/doc/user/packages/container_registry/img/container_registry_repositories_v12_10.png diff --git a/doc/user/packages/container_registry/img/container_registry_tags_v12_10.png b/doc/user/packages/container_registry/img/container_registry_tags_v12_10.png Binary files differnew file mode 100644 index 00000000000..40abc7eda9b --- /dev/null +++ b/doc/user/packages/container_registry/img/container_registry_tags_v12_10.png diff --git a/doc/user/packages/container_registry/index.md b/doc/user/packages/container_registry/index.md index a3eb1d5232a..c511dbb5d01 100644 --- a/doc/user/packages/container_registry/index.md +++ b/doc/user/packages/container_registry/index.md @@ -18,6 +18,8 @@ have its own space to store its Docker images. You can read more about Docker Registry at <https://docs.docker.com/registry/introduction/>. + + ## Enable the Container Registry for your project CAUTION: **Warning:** @@ -329,10 +331,124 @@ If you forget to set the service alias, the `docker:19.03.1` image won't find th error during connect: Get http://docker:2376/v1.39/info: dial tcp: lookup docker on 192.168.0.1:53: no such host ``` +## Delete images + +You can delete images from your Container Registry in multiple ways. + +CAUTION: **Warning:** +Deleting images is a destructive action and can't be undone. To restore +a deleted image, you must rebuild and re-upload it. + +NOTE: **Note:** +Administrators should review how to +[garbage collect](../../../administration/packages/container_registry.md#container-registry-garbage-collection) +the deleted images. + +### Delete images from within GitLab + +To delete images from within GitLab: + +1. Navigate to your project's or group's **{package}** **Packages > Container Registry**. +1. From the **Container Registry** page, you can select what you want to delete, + by either: + + - Deleting the entire repository, and all the tags it contains, by clicking + the red **{remove}** **Trash** icon. + - Navigating to the repository, and deleting tags individually or in bulk + by clicking the red **{remove}** **Trash** icon next to the tag you want + to delete. + +1. In the dialog box, click **Remove tag**. + +  + +### Delete images using the API + +If you want to automate the process of deleting images, GitLab provides an API. For more +information, see the following endpoints: + +- [Delete a Registry repository](../../../api/container_registry.md#delete-registry-repository) +- [Delete an individual Registry repository tag](../../../api/container_registry.md#delete-a-registry-repository-tag) +- [Delete Registry repository tags in bulk](../../../api/container_registry.md#delete-registry-repository-tags-in-bulk) + +### Delete images using GitLab CI/CD + +CAUTION: **Warning:** +GitLab CI/CD doesn't provide a built-in way to remove your images, but this example +uses a third-party tool called [reg](https://github.com/genuinetools/reg) +that talks to the GitLab Registry API. You are responsible for your own actions. +For assistance with this tool, see +[the issue queue for reg](https://github.com/genuinetools/reg/issues). + +The following example defines two stages: `build`, and `clean`. The +`build_image` job builds the Docker image for the branch, and the +`delete_image` job deletes it. The `reg` executable is downloaded and used to +remove the image matching the `$CI_PROJECT_PATH:$CI_COMMIT_REF_SLUG` +[environment variable](../../../ci/variables/predefined_variables.md). + +To use this example, change the `IMAGE_TAG` variable to match your needs: + +```yaml +stages: + - build + - clean + +build_image: + image: docker:19.03.1 + stage: build + services: + - docker:19.03.1-dind + variables: + IMAGE_TAG: $CI_REGISTRY_IMAGE:$CI_COMMIT_REF_SLUG + script: + - docker login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD $CI_REGISTRY + - docker build -t $IMAGE_TAG . + - docker push $IMAGE_TAG + only: + - branches + except: + - master + +delete_image: + image: docker:19.03.1 + stage: clean + services: + - docker:19.03.1-dind + variables: + IMAGE_TAG: $CI_PROJECT_PATH:$CI_COMMIT_REF_SLUG + REG_SHA256: ade837fc5224acd8c34732bf54a94f579b47851cc6a7fd5899a98386b782e228 + REG_VERSION: 0.16.1 + before_script: + - apk add --no-cache curl + - curl --fail --show-error --location "https://github.com/genuinetools/reg/releases/download/v$REG_VERSION/reg-linux-amd64" --output /usr/local/bin/reg + - echo "$REG_SHA256 /usr/local/bin/reg" | sha256sum -c - + - chmod a+x /usr/local/bin/reg + script: + - /usr/local/bin/reg rm -d --auth-url $CI_REGISTRY -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD $IMAGE_TAG + only: + - branches + except: + - master +``` + +TIP: **Tip:** +You can download the latest `reg` release from +[the releases page](https://github.com/genuinetools/reg/releases), then update +the code example by changing the `REG_SHA256` and `REG_VERSION` variables +defined in the `delete_image` job. + +### Delete images using an expiration policy + +You can create a per-project [expiration policy](#expiration-policy) to ensure +older tags and images are regularly removed from the Container Registry. + ## Expiration policy > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/15398) in GitLab 12.8. +NOTE: **Note:** +Expiration policies are only available for projects created in GitLab 12.8 and later. + It is possible to create a per-project expiration policy, so that you can make sure that older tags and images are regularly removed from the Container Registry. @@ -348,6 +464,20 @@ then goes through a process of excluding tags from it until only the ones to be 1. Excludes from the list the tags older than the `older_than` value (Expiration interval). 1. Finally, the remaining tags in the list are deleted from the Container Registry. +### Managing project expiration policy through the UI + +To manage project expiration policy, navigate to **{settings}** **Settings > CI/CD > Container Registry tag expiration policy**. + + + +The UI allows you to configure the following: + +- **Expiration policy:** enable or disable the expiration policy. +- **Expiration interval:** how long tags are exempt from being deleted. +- **Expiration schedule:** how often the cron job checking the tags should run. +- **Expiration latest:** how many tags to _always_ keep for each image. +- **Docker tags with names matching this regex pattern will expire:** the regex used to determine what tags should be expired. To qualify all tags for expiration, use the default value of `.*`. + ### Managing project expiration policy through the API You can set, update, and disable the expiration policies using the GitLab API. @@ -368,20 +498,6 @@ Examples: See the API documentation for further details: [Edit project](../../../api/projects.md#edit-project). -### Managing project expiration policy through the UI - -To manage project expiration policy, navigate to **Settings > CI/CD > Container Registry tag expiration policy**. - - - -The UI allows you to configure the following: - -- **Expiration policy:** enable or disable the expiration policy. -- **Expiration interval:** how long tags are exempt from being deleted. -- **Expiration schedule:** how often the cron job checking the tags should run. -- **Expiration latest:** how many tags to _always_ keep for each image. -- **Docker tags with names matching this regex pattern will expire:** the regex used to determine what tags should be expired. To qualify all tags for expiration, use the default value of `.*`. - ## Limitations Moving or renaming existing Container Registry repositories is not supported diff --git a/lib/api/deploy_tokens.rb b/lib/api/deploy_tokens.rb index 2b1c485785b..a637bfcb180 100644 --- a/lib/api/deploy_tokens.rb +++ b/lib/api/deploy_tokens.rb @@ -53,10 +53,10 @@ module API params do requires :name, type: String, desc: "New deploy token's name" - requires :expires_at, type: DateTime, desc: 'Expiration date for the deploy token. Does not expire if no value is provided.' - requires :username, type: String, desc: 'Username for deploy token. Default is `gitlab+deploy-token-{n}`' requires :scopes, type: Array[String], values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s), desc: 'Indicates the deploy token scopes. Must be at least one of "read_repository" or "read_registry".' + optional :expires_at, type: DateTime, desc: 'Expiration date for the deploy token. Does not expire if no value is provided.' + optional :username, type: String, desc: 'Username for deploy token. Default is `gitlab+deploy-token-{n}`' end desc 'Create a project deploy token' do detail 'This feature was introduced in GitLab 12.9' @@ -114,10 +114,10 @@ module API params do requires :name, type: String, desc: 'The name of the deploy token' - requires :expires_at, type: DateTime, desc: 'Expiration date for the deploy token. Does not expire if no value is provided.' - requires :username, type: String, desc: 'Username for deploy token. Default is `gitlab+deploy-token-{n}`' requires :scopes, type: Array[String], values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s), desc: 'Indicates the deploy token scopes. Must be at least one of "read_repository" or "read_registry".' + optional :expires_at, type: DateTime, desc: 'Expiration date for the deploy token. Does not expire if no value is provided.' + optional :username, type: String, desc: 'Username for deploy token. Default is `gitlab+deploy-token-{n}`' end desc 'Create a group deploy token' do detail 'This feature was introduced in GitLab 12.9' diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 47784dc771e..ff61cceb4c9 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -367,6 +367,10 @@ module API render_api_error!('405 Method Not Allowed', 405) end + def not_acceptable! + render_api_error!('406 Not Acceptable', 406) + end + def service_unavailable! render_api_error!('503 Service Unavailable', 503) end diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 62f5b67af1e..0b2df85f61f 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -95,6 +95,8 @@ module API render_api_error!({ error: ::Gitlab::RateLimitHelpers::ARCHIVE_RATE_LIMIT_REACHED_MESSAGE }, 429) end + not_acceptable! if Gitlab::HotlinkingDetector.intercept_hotlinking?(request) + send_git_archive user_project.repository, ref: params[:sha], format: params[:format], append_sha: true rescue not_found!('File') diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index b5df036c5ca..0aaab9a812f 100644 --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -74,6 +74,8 @@ module API desc: 'The visibility of the snippet' end post do + authorize! :create_snippet + attrs = declared_params(include_missing: false).merge(request: request, api: true) service_response = ::Snippets::CreateService.new(nil, current_user, attrs).execute snippet = service_response.payload[:snippet] diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb index 76af29b2977..e1829403941 100644 --- a/lib/api/triggers.rb +++ b/lib/api/triggers.rb @@ -109,6 +109,8 @@ module API trigger = user_project.triggers.find(params.delete(:trigger_id)) break not_found!('Trigger') unless trigger + authorize! :admin_trigger, trigger + if trigger.update(declared_params(include_missing: false)) present trigger, with: Entities::Trigger, current_user: current_user else diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index c16c2ce96de..7f7bdda953f 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -171,6 +171,8 @@ module Gitlab if valid_oauth_token?(token) user = User.find_by(id: token.resource_owner_id) + return unless user.can?(:log_in) + Gitlab::Auth::Result.new(user, nil, :oauth, full_authentication_abilities) end end @@ -182,7 +184,7 @@ module Gitlab token = PersonalAccessTokensFinder.new(state: 'active').find_by_token(password) - if token && valid_scoped_token?(token, all_available_scopes) + if token && valid_scoped_token?(token, all_available_scopes) && token.user.can?(:log_in) Gitlab::Auth::Result.new(token.user, nil, :personal_access_token, abilities_for_scopes(token.scopes)) end end @@ -260,6 +262,8 @@ module Gitlab return unless build.project.builds_enabled? if build.user + return unless build.user.can?(:log_in) + # If user is assigned to build, use restricted credentials of user Gitlab::Auth::Result.new(build.user, build.project, :build, build_authentication_abilities) else diff --git a/lib/gitlab/gfm/uploads_rewriter.rb b/lib/gitlab/gfm/uploads_rewriter.rb index 6b52d6e88e5..23af0a9bb18 100644 --- a/lib/gitlab/gfm/uploads_rewriter.rb +++ b/lib/gitlab/gfm/uploads_rewriter.rb @@ -22,6 +22,8 @@ module Gitlab return @text unless needs_rewrite? @text.gsub(@pattern) do |markdown| + Gitlab::Utils.check_path_traversal!($~[:file]) + file = find_file(@source_project, $~[:secret], $~[:file]) break markdown unless file.try(:exists?) diff --git a/lib/gitlab/hotlinking_detector.rb b/lib/gitlab/hotlinking_detector.rb new file mode 100644 index 00000000000..44901297870 --- /dev/null +++ b/lib/gitlab/hotlinking_detector.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module Gitlab + class HotlinkingDetector + IMAGE_FORMATS = %w(image/jpeg image/apng image/png image/webp image/svg+xml image/*).freeze + MEDIA_FORMATS = %w(video/webm video/ogg video/* application/ogg audio/webm audio/ogg audio/wav audio/*).freeze + CSS_FORMATS = %w(text/css).freeze + INVALID_FORMATS = (IMAGE_FORMATS + MEDIA_FORMATS + CSS_FORMATS).freeze + INVALID_FETCH_MODES = %w(cors no-cors websocket).freeze + + class << self + def intercept_hotlinking?(request) + request_accepts = parse_request_accepts(request) + + return false unless Feature.enabled?(:repository_archive_hotlinking_interception, default_enabled: true) + + # Block attempts to embed as JS + return true if sec_fetch_invalid?(request) + + # If no Accept header was set, skip the rest + return false if request_accepts.empty? + + # Workaround for IE8 weirdness + return false if IMAGE_FORMATS.include?(request_accepts.first) && request_accepts.include?("application/x-ms-application") + + # Block all other media requests if the first format is a media type + return true if INVALID_FORMATS.include?(request_accepts.first) + + false + end + + private + + def sec_fetch_invalid?(request) + fetch_mode = request.headers["Sec-Fetch-Mode"] + + return if fetch_mode.blank? + return true if INVALID_FETCH_MODES.include?(fetch_mode) + end + + def parse_request_accepts(request) + # Rails will already have parsed the Accept header + return request.accepts if request.respond_to?(:accepts) + + # Grape doesn't parse it, so we can use the Rails system for this + return Mime::Type.parse(request.headers["Accept"]) if request.respond_to?(:headers) && request.headers["Accept"].present? + + [] + end + end + end +end diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb index 3bfc059dcd3..018cb36fc58 100644 --- a/lib/gitlab/import_export/attribute_cleaner.rb +++ b/lib/gitlab/import_export/attribute_cleaner.rb @@ -11,7 +11,14 @@ module Gitlab 'discussion_id', 'custom_attributes' ].freeze - PROHIBITED_REFERENCES = Regexp.union(/\Acached_markdown_version\Z/, /_id\Z/, /_ids\Z/, /_html\Z/, /attributes/).freeze + PROHIBITED_REFERENCES = Regexp.union( + /\Acached_markdown_version\Z/, + /_id\Z/, + /_ids\Z/, + /_html\Z/, + /attributes/, + /\Aremote_\w+_(url|urls|request_header)\Z/ # carrierwave automatically creates these attribute methods for uploads + ).freeze def self.clean(*args) new(*args).clean diff --git a/lib/gitlab/import_export/group/tree_restorer.rb b/lib/gitlab/import_export/group/tree_restorer.rb index 247e39a68b9..f6ebd83bfaa 100644 --- a/lib/gitlab/import_export/group/tree_restorer.rb +++ b/lib/gitlab/import_export/group/tree_restorer.rb @@ -4,12 +4,13 @@ module Gitlab module ImportExport module Group class TreeRestorer + include Gitlab::Utils::StrongMemoize + attr_reader :user attr_reader :shared attr_reader :group def initialize(user:, shared:, group:, group_hash:) - @path = File.join(shared.export_path, 'group.json') @user = user @shared = shared @group = group @@ -17,17 +18,14 @@ module Gitlab end def restore - @relation_reader ||= - if @group_hash.present? - ImportExport::JSON::LegacyReader::User.new(@group_hash, reader.group_relation_names) - else - ImportExport::JSON::LegacyReader::File.new(@path, reader.group_relation_names) - end + @group_attributes = relation_reader.consume_attributes(nil) + @group_members = relation_reader.consume_relation(nil, 'members') + + # We need to remove `name` and `path` as we did consume it in previous pass + @group_attributes.delete('name') + @group_attributes.delete('path') - @group_members = @relation_reader.consume_relation('members') - @children = @relation_reader.consume_attribute('children') - @relation_reader.consume_attribute('name') - @relation_reader.consume_attribute('path') + @children = @group_attributes.delete('children') if members_mapper.map && restorer.restore @children&.each do |group_hash| @@ -53,16 +51,32 @@ module Gitlab private + def relation_reader + strong_memoize(:relation_reader) do + if @group_hash.present? + ImportExport::JSON::LegacyReader::Hash.new( + @group_hash, + relation_names: reader.group_relation_names) + else + ImportExport::JSON::LegacyReader::File.new( + File.join(shared.export_path, 'group.json'), + relation_names: reader.group_relation_names) + end + end + end + def restorer @relation_tree_restorer ||= RelationTreeRestorer.new( - user: @user, - shared: @shared, - importable: @group, - relation_reader: @relation_reader, - members_mapper: members_mapper, - object_builder: object_builder, - relation_factory: relation_factory, - reader: reader + user: @user, + shared: @shared, + relation_reader: relation_reader, + members_mapper: members_mapper, + object_builder: object_builder, + relation_factory: relation_factory, + reader: reader, + importable: @group, + importable_attributes: @group_attributes, + importable_path: nil ) end @@ -88,7 +102,11 @@ module Gitlab end def members_mapper - @members_mapper ||= Gitlab::ImportExport::MembersMapper.new(exported_members: @group_members, user: @user, importable: @group) + @members_mapper ||= Gitlab::ImportExport::MembersMapper.new( + exported_members: @group_members, + user: @user, + importable: @group + ) end def relation_factory diff --git a/lib/gitlab/import_export/json/legacy_reader.rb b/lib/gitlab/import_export/json/legacy_reader.rb index 477e41ae3eb..57579fe9def 100644 --- a/lib/gitlab/import_export/json/legacy_reader.rb +++ b/lib/gitlab/import_export/json/legacy_reader.rb @@ -5,19 +5,25 @@ module Gitlab module JSON class LegacyReader class File < LegacyReader - def initialize(path, relation_names) + include Gitlab::Utils::StrongMemoize + + def initialize(path, relation_names:, allowed_path: nil) @path = path - super(relation_names) + super( + relation_names: relation_names, + allowed_path: allowed_path) end - def valid? + def exist? ::File.exist?(@path) end - private + protected def tree_hash - @tree_hash ||= read_hash + strong_memoize(:tree_hash) do + read_hash + end end def read_hash @@ -28,13 +34,15 @@ module Gitlab end end - class User < LegacyReader - def initialize(tree_hash, relation_names) + class Hash < LegacyReader + def initialize(tree_hash, relation_names:, allowed_path: nil) @tree_hash = tree_hash - super(relation_names) + super( + relation_names: relation_names, + allowed_path: allowed_path) end - def valid? + def exist? @tree_hash.present? end @@ -43,11 +51,16 @@ module Gitlab attr_reader :tree_hash end - def initialize(relation_names) + def initialize(relation_names:, allowed_path:) @relation_names = relation_names.map(&:to_s) + + # This is legacy reader, to be used in transition + # period before `.ndjson`, + # we strong validate what is being readed + @allowed_path = allowed_path end - def valid? + def exist? raise NotImplementedError end @@ -55,15 +68,22 @@ module Gitlab true end - def root_attributes(excluded_attributes = []) - attributes.except(*excluded_attributes.map(&:to_s)) + def consume_attributes(importable_path) + unless importable_path == @allowed_path + raise ArgumentError, "Invalid #{importable_path} passed to `consume_attributes`. Use #{@allowed_path} instead." + end + + attributes end - def consume_relation(key) + def consume_relation(importable_path, key) + unless importable_path == @allowed_path + raise ArgumentError, "Invalid #{importable_name} passed to `consume_relation`. Use #{@allowed_path} instead." + end + value = relations.delete(key) return value unless block_given? - return if value.nil? if value.is_a?(Array) @@ -75,17 +95,13 @@ module Gitlab end end - def consume_attribute(key) - attributes.delete(key) - end - def sort_ci_pipelines_by_id relations['ci_pipelines']&.sort_by! { |hash| hash['id'] } end private - attr_reader :relation_names + attr_reader :relation_names, :allowed_path def tree_hash raise NotImplementedError diff --git a/lib/gitlab/import_export/json/legacy_writer.rb b/lib/gitlab/import_export/json/legacy_writer.rb index c935e360a65..7be21410d26 100644 --- a/lib/gitlab/import_export/json/legacy_writer.rb +++ b/lib/gitlab/import_export/json/legacy_writer.rb @@ -8,11 +8,15 @@ module Gitlab attr_reader :path - def initialize(path) + def initialize(path, allowed_path:) @path = path - @last_array = nil @keys = Set.new + # This is legacy writer, to be used in transition + # period before `.ndjson`, + # we strong validate what is being written + @allowed_path = allowed_path + mkdir_p(File.dirname(@path)) file.write('{}') end @@ -22,12 +26,44 @@ module Gitlab @file = nil end - def set(hash) + def write_attributes(exportable_path, hash) + unless exportable_path == @allowed_path + raise ArgumentError, "Invalid #{exportable_path}" + end + hash.each do |key, value| write(key, value) end end + def write_relation(exportable_path, key, value) + unless exportable_path == @allowed_path + raise ArgumentError, "Invalid #{exportable_path}" + end + + write(key, value) + end + + def write_relation_array(exportable_path, key, items) + unless exportable_path == @allowed_path + raise ArgumentError, "Invalid #{exportable_path}" + end + + write(key, []) + + # rewind by two bytes, to overwrite ']}' + file.pos = file.size - 2 + + items.each_with_index do |item, idx| + file.write(',') if idx > 0 + file.write(item.to_json) + end + + file.write(']}') + end + + private + def write(key, value) raise ArgumentError, "key '#{key}' already written" if @keys.include?(key) @@ -41,29 +77,8 @@ module Gitlab file.write('}') @keys.add(key) - @last_array = nil - @last_array_count = nil - end - - def append(key, value) - unless @last_array == key - write(key, []) - - @last_array = key - @last_array_count = 0 - end - - # rewind by two bytes, to overwrite ']}' - file.pos = file.size - 2 - - file.write(',') if @last_array_count > 0 - file.write(value.to_json) - file.write(']}') - @last_array_count += 1 end - private - def file @file ||= File.open(@path, "wb") end diff --git a/lib/gitlab/import_export/json/streaming_serializer.rb b/lib/gitlab/import_export/json/streaming_serializer.rb index d053bf16166..04ca598ff5d 100644 --- a/lib/gitlab/import_export/json/streaming_serializer.rb +++ b/lib/gitlab/import_export/json/streaming_serializer.rb @@ -14,8 +14,9 @@ module Gitlab end end - def initialize(exportable, relations_schema, json_writer) + def initialize(exportable, relations_schema, json_writer, exportable_path:) @exportable = exportable + @exportable_path = exportable_path @relations_schema = relations_schema @json_writer = json_writer end @@ -35,7 +36,7 @@ module Gitlab def serialize_root attributes = exportable.as_json( relations_schema.merge(include: nil, preloads: nil)) - json_writer.set(attributes) + json_writer.write_attributes(@exportable_path, attributes) end def serialize_relation(definition) @@ -47,16 +48,28 @@ module Gitlab record = exportable.public_send(key) # rubocop: disable GitlabSecurity/PublicSend if record.is_a?(ActiveRecord::Relation) serialize_many_relations(key, record, options) + elsif record.respond_to?(:each) # this is to support `project_members` that return an Array + serialize_many_each(key, record, options) else serialize_single_relation(key, record, options) end end def serialize_many_relations(key, records, options) - key_preloads = preloads&.dig(key) - records = records.preload(key_preloads) if key_preloads + enumerator = Enumerator.new do |items| + key_preloads = preloads&.dig(key) + records = records.preload(key_preloads) if key_preloads - records.find_each(batch_size: BATCH_SIZE) do |record| + records.find_each(batch_size: BATCH_SIZE) do |record| + items << Raw.new(record.to_json(options)) + end + end + + json_writer.write_relation_array(@exportable_path, key, enumerator) + end + + def serialize_many_each(key, records, options) + records.each do |record| json = Raw.new(record.to_json(options)) json_writer.append(key, json) @@ -66,7 +79,7 @@ module Gitlab def serialize_single_relation(key, record, options) json = Raw.new(record.to_json(options)) - json_writer.write(key, json) + json_writer.write_relation(@exportable_path, key, json) end def includes diff --git a/lib/gitlab/import_export/project/tree_restorer.rb b/lib/gitlab/import_export/project/tree_restorer.rb index f8d25e14c02..99e57d9decd 100644 --- a/lib/gitlab/import_export/project/tree_restorer.rb +++ b/lib/gitlab/import_export/project/tree_restorer.rb @@ -4,6 +4,8 @@ module Gitlab module ImportExport module Project class TreeRestorer + include Gitlab::Utils::StrongMemoize + attr_reader :user attr_reader :shared attr_reader :project @@ -15,9 +17,8 @@ module Gitlab end def restore - @relation_reader = ImportExport::JSON::LegacyReader::File.new(File.join(shared.export_path, 'project.json'), reader.project_relation_names) - - @project_members = @relation_reader.consume_relation('project_members') + @project_attributes = relation_reader.consume_attributes(importable_path) + @project_members = relation_reader.consume_relation(importable_path, 'project_members') if relation_tree_restorer.restore import_failure_service.with_retry(action: 'set_latest_merge_request_diff_ids!') do @@ -35,16 +36,28 @@ module Gitlab private + def relation_reader + strong_memoize(:relation_reader) do + ImportExport::JSON::LegacyReader::File.new( + File.join(shared.export_path, 'project.json'), + relation_names: reader.project_relation_names, + allowed_path: importable_path + ) + end + end + def relation_tree_restorer @relation_tree_restorer ||= RelationTreeRestorer.new( user: @user, shared: @shared, - importable: @project, - relation_reader: @relation_reader, + relation_reader: relation_reader, object_builder: object_builder, members_mapper: members_mapper, relation_factory: relation_factory, - reader: reader + reader: reader, + importable: @project, + importable_attributes: @project_attributes, + importable_path: importable_path ) end @@ -69,6 +82,10 @@ module Gitlab def import_failure_service @import_failure_service ||= ImportFailureService.new(@project) end + + def importable_path + "project" + end end end end diff --git a/lib/gitlab/import_export/project/tree_saver.rb b/lib/gitlab/import_export/project/tree_saver.rb index 01000c9d6d9..988776fe600 100644 --- a/lib/gitlab/import_export/project/tree_saver.rb +++ b/lib/gitlab/import_export/project/tree_saver.rb @@ -15,10 +15,17 @@ module Gitlab end def save - json_writer = ImportExport::JSON::LegacyWriter.new(@full_path) - - serializer = ImportExport::JSON::StreamingSerializer.new(exportable, reader.project_tree, json_writer) - serializer.execute + json_writer = ImportExport::JSON::LegacyWriter.new( + @full_path, + allowed_path: "project" + ) + + ImportExport::JSON::StreamingSerializer.new( + exportable, + reader.project_tree, + json_writer, + exportable_path: "project" + ).execute true rescue => e diff --git a/lib/gitlab/import_export/relation_tree_restorer.rb b/lib/gitlab/import_export/relation_tree_restorer.rb index 1157e18c7f9..7f25222ba41 100644 --- a/lib/gitlab/import_export/relation_tree_restorer.rb +++ b/lib/gitlab/import_export/relation_tree_restorer.rb @@ -11,7 +11,15 @@ module Gitlab attr_reader :importable attr_reader :relation_reader - def initialize(user:, shared:, importable:, relation_reader:, members_mapper:, object_builder:, relation_factory:, reader:) + def initialize( # rubocop:disable Metrics/ParameterLists + user:, shared:, relation_reader:, + members_mapper:, object_builder:, + relation_factory:, + reader:, + importable:, + importable_attributes:, + importable_path: + ) @user = user @shared = shared @importable = importable @@ -20,6 +28,8 @@ module Gitlab @object_builder = object_builder @relation_factory = relation_factory @reader = reader + @importable_attributes = importable_attributes + @importable_path = importable_path end def restore @@ -57,7 +67,7 @@ module Gitlab end def process_relation!(relation_key, relation_definition) - @relation_reader.consume_relation(relation_key) do |data_hash, relation_index| + @relation_reader.consume_relation(@importable_path, relation_key) do |data_hash, relation_index| process_relation_item!(relation_key, relation_definition, relation_index, data_hash) end end @@ -94,7 +104,7 @@ module Gitlab end def update_params! - params = @relation_reader.root_attributes(relations.keys) + params = @importable_attributes.except(*relations.keys.map(&:to_s)) params = params.merge(present_override_params) # Cleaning all imported and overridden params diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 38281fb1c91..66503621851 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -16,6 +16,14 @@ module Gitlab "It must start with letter, digit, emoji or '_'." end + def group_name_regex + project_name_regex + end + + def group_name_regex_message + project_name_regex_message + end + ## # Docker Distribution Registry repository / tag name rules # diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 80f16fd3e1d..6de9caf097a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -12860,6 +12860,9 @@ msgstr "" msgid "Mirror repository" msgstr "" +msgid "Mirror settings are only available to GitLab administrators." +msgstr "" + msgid "Mirror user" msgstr "" @@ -18862,6 +18865,9 @@ msgstr "" msgid "Specified URL cannot be used." msgstr "" +msgid "Specified URL cannot be used: \"%{reason}\"" +msgstr "" + msgid "Specify an e-mail address regex pattern to identify default internal users." msgstr "" diff --git a/qa/qa/specs/features/browser_ui/1_manage/project/dashboard_images_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/project/dashboard_images_spec.rb index 6a5bc6173e0..1f1adf9afb4 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/project/dashboard_images_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/project/dashboard_images_spec.rb @@ -3,7 +3,7 @@ require 'nokogiri' module QA - context 'Manage' do + context 'Manage', quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/212145', type: :stale } do describe 'Check for broken images', :requires_admin do before(:context) do admin = QA::Resource::User.new.tap do |user| diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 11c70d3aeca..22427f581d4 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -258,6 +258,18 @@ describe GroupsController do end end end + + context "malicious group name" do + subject { post :create, params: { group: { name: "<script>alert('Mayday!');</script>", path: "invalid_group_url" } } } + + before do + sign_in(user) + end + + it { expect { subject }.not_to change { Group.count } } + + it { expect(subject).to render_template(:new) } + end end describe 'GET #index' do @@ -836,6 +848,16 @@ describe GroupsController do put :update, params: { id: group.to_param, group: { name: 'world' } } end.to change { group.reload.name } end + + context "malicious group name" do + subject { put :update, params: { id: group.to_param, group: { name: "<script>alert('Attack!');</script>" } } } + + it { is_expected.to render_template(:edit) } + + it 'does not update name' do + expect { subject }.not_to change { group.reload.name } + end + end end describe 'DELETE #destroy' do diff --git a/spec/controllers/import/fogbugz_controller_spec.rb b/spec/controllers/import/fogbugz_controller_spec.rb index 9a647b8caae..c833fbfaea5 100644 --- a/spec/controllers/import/fogbugz_controller_spec.rb +++ b/spec/controllers/import/fogbugz_controller_spec.rb @@ -25,6 +25,35 @@ describe Import::FogbugzController do expect(session[:fogbugz_uri]).to eq(uri) expect(response).to redirect_to(new_user_map_import_fogbugz_path) end + + context 'verify url' do + shared_examples 'denies local request' do |reason| + it 'does not allow requests' do + post :callback, params: { uri: uri, email: 'test@example.com', password: 'mypassword' } + + expect(response).to redirect_to(new_import_fogbugz_url) + expect(flash[:alert]).to eq("Specified URL cannot be used: \"#{reason}\"") + end + end + + context 'when host is localhost' do + let(:uri) { 'https://localhost:3000' } + + include_examples 'denies local request', 'Requests to localhost are not allowed' + end + + context 'when host is on local network' do + let(:uri) { 'http://192.168.0.1/' } + + include_examples 'denies local request', 'Requests to the local network are not allowed' + end + + context 'when host is ftp protocol' do + let(:uri) { 'ftp://testing' } + + include_examples 'denies local request', 'Only allowed schemes are http, https' + end + end end describe 'POST #create_user_map' do diff --git a/spec/controllers/projects/mirrors_controller_spec.rb b/spec/controllers/projects/mirrors_controller_spec.rb index 4362febda5c..3579e4aa2cf 100644 --- a/spec/controllers/projects/mirrors_controller_spec.rb +++ b/spec/controllers/projects/mirrors_controller_spec.rb @@ -5,6 +5,72 @@ require 'spec_helper' describe Projects::MirrorsController do include ReactiveCachingHelpers + shared_examples 'only admin is allowed when mirroring is disabled' do + let(:subject_action) { raise 'subject_action is required' } + let(:user) { project.owner } + let(:project_settings_path) { project_settings_repository_path(project, anchor: 'js-push-remote-settings') } + + context 'when project mirroring is enabled' do + it 'allows requests from a maintainer' do + sign_in(user) + + subject_action + expect(response).to redirect_to(project_settings_path) + end + + it 'allows requests from an admin user' do + user.update!(admin: true) + sign_in(user) + + subject_action + expect(response).to redirect_to(project_settings_path) + end + end + + context 'when project mirroring is disabled' do + before do + stub_application_setting(mirror_available: false) + end + + it 'disallows requests from a maintainer' do + sign_in(user) + + subject_action + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'allows requests from an admin user' do + user.update!(admin: true) + sign_in(user) + + subject_action + expect(response).to redirect_to(project_settings_path) + end + end + end + + describe 'Access control' do + let(:project) { create(:project, :repository) } + + describe '#update' do + include_examples 'only admin is allowed when mirroring is disabled' do + let(:subject_action) do + do_put(project, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => 'http://foo.com' } }) + end + end + end + + describe '#update_now' do + include_examples 'only admin is allowed when mirroring is disabled' do + let(:options) { { namespace_id: project.namespace, project_id: project } } + + let(:subject_action) do + get :update_now, params: options.merge(sync_remote: true) + end + end + end + end + describe 'setting up a remote mirror' do let_it_be(:project) { create(:project, :repository) } diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index 2d39f0afaee..42032b4cad0 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -28,6 +28,12 @@ describe Projects::RepositoriesController do sign_in(user) end + it_behaves_like "hotlink interceptor" do + let(:http_request) do + get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master" }, format: "zip" + end + end + it "uses Gitlab::Workhorse" do get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master" }, format: "zip" diff --git a/spec/features/clusters/installing_applications_shared_examples.rb b/spec/features/clusters/installing_applications_shared_examples.rb index 8710e05e5cc..5b565c0a304 100644 --- a/spec/features/clusters/installing_applications_shared_examples.rb +++ b/spec/features/clusters/installing_applications_shared_examples.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true -shared_examples "installing applications on a cluster" do +shared_examples "installing applications for a cluster" do |managed_apps_local_tiller| before do + stub_feature_flags(managed_apps_local_tiller: managed_apps_local_tiller) + visit cluster_path end @@ -26,48 +28,61 @@ shared_examples "installing applications on a cluster" do it 'user can install applications' do wait_for_requests - page.within('.js-cluster-application-row-helm') do - expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to be_nil + application_row = + if managed_apps_local_tiller + '.js-cluster-application-row-ingress' + else + '.js-cluster-application-row-helm' + end + + page.within(application_row) do + expect(page).not_to have_css('.js-cluster-application-install-button[disabled]') expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Install') end end - context 'when user installs Helm' do - before do - allow(ClusterInstallAppWorker).to receive(:perform_async) - wait_for_requests + if managed_apps_local_tiller + it 'does not show the Helm application' do + expect(page).not_to have_selector(:css, '.js-cluster-application-row-helm') + end + else + context 'when user installs Helm' do + before do + allow(ClusterInstallAppWorker).to receive(:perform_async) + wait_for_requests - page.within('.js-cluster-application-row-helm') do - page.find(:css, '.js-cluster-application-install-button').click + page.within('.js-cluster-application-row-helm') do + page.find(:css, '.js-cluster-application-install-button').click + end + + wait_for_requests end - wait_for_requests - end + it 'shows the status transition' do + page.within('.js-cluster-application-row-helm') do + # FE sends request and gets the response, then the buttons is "Installing" + expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installing') - it 'shows the status transition' do - page.within('.js-cluster-application-row-helm') do - # FE sends request and gets the response, then the buttons is "Installing" - expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installing') + Clusters::Cluster.last.application_helm.make_installing! - Clusters::Cluster.last.application_helm.make_installing! + # FE starts polling and update the buttons to "Installing" + expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installing') - # FE starts polling and update the buttons to "Installing" - expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Installing') + Clusters::Cluster.last.application_helm.make_installed! - Clusters::Cluster.last.application_helm.make_installed! + expect(page).not_to have_css('button', exact_text: 'Install', visible: :all) + expect(page).not_to have_css('button', exact_text: 'Installing', visible: :all) + expect(page).to have_css('.js-cluster-application-uninstall-button:not([disabled])', exact_text: 'Uninstall') + end - expect(page).not_to have_css('button', exact_text: 'Install', visible: :all) - expect(page).not_to have_css('button', exact_text: 'Installing', visible: :all) - expect(page).to have_css('.js-cluster-application-uninstall-button:not([disabled])', exact_text: 'Uninstall') + expect(page).to have_content('Helm Tiller was successfully installed on your Kubernetes cluster') end - - expect(page).to have_content('Helm Tiller was successfully installed on your Kubernetes cluster') end end context 'when user installs Knative' do before do - create(:clusters_applications_helm, :installed, cluster: cluster) + create(:clusters_applications_helm, :installed, cluster: cluster) unless managed_apps_local_tiller end context 'on an abac cluster' do @@ -153,7 +168,7 @@ shared_examples "installing applications on a cluster" do allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) - create(:clusters_applications_helm, :installed, cluster: cluster) + create(:clusters_applications_helm, :installed, cluster: cluster) unless managed_apps_local_tiller page.within('.js-cluster-application-row-cert_manager') do click_button 'Install' @@ -189,7 +204,7 @@ shared_examples "installing applications on a cluster" do before do allow(ClusterInstallAppWorker).to receive(:perform_async) - create(:clusters_applications_helm, :installed, cluster: cluster) + create(:clusters_applications_helm, :installed, cluster: cluster) unless managed_apps_local_tiller page.within('.js-cluster-application-row-elastic_stack') do click_button 'Install' @@ -221,7 +236,7 @@ shared_examples "installing applications on a cluster" do allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) - create(:clusters_applications_helm, :installed, cluster: cluster) + create(:clusters_applications_helm, :installed, cluster: cluster) unless managed_apps_local_tiller page.within('.js-cluster-application-row-ingress') do expect(page).to have_css('.js-cluster-application-install-button:not([disabled])') @@ -263,3 +278,8 @@ shared_examples "installing applications on a cluster" do end end end + +shared_examples "installing applications on a cluster" do + it_behaves_like "installing applications for a cluster", false + it_behaves_like "installing applications for a cluster", true +end diff --git a/spec/features/projects/settings/repository_settings_spec.rb b/spec/features/projects/settings/repository_settings_spec.rb index 9030cd6a648..9b956dee089 100644 --- a/spec/features/projects/settings/repository_settings_spec.rb +++ b/spec/features/projects/settings/repository_settings_spec.rb @@ -26,11 +26,7 @@ describe 'Projects > Settings > Repository settings' do let(:role) { :maintainer } context 'remote mirror settings' do - let(:user2) { create(:user) } - before do - project.add_maintainer(user2) - visit project_settings_repository_path(project) end @@ -90,6 +86,18 @@ describe 'Projects > Settings > Repository settings' do expect(page).to have_selector('[title="Copy SSH public key"]') end + context 'when project mirroring is disabled' do + before do + stub_application_setting(mirror_available: false) + visit project_settings_repository_path(project) + end + + it 'hides remote mirror settings' do + expect(page.find('.project-mirror-settings')).not_to have_selector('form') + expect(page).to have_content('Mirror settings are only available to GitLab administrators.') + end + end + def select_direction(direction = 'push') direction_select = find('#mirror_direction') @@ -154,4 +162,31 @@ describe 'Projects > Settings > Repository settings' do expect(mirror).not_to have_selector('.rspec-update-now-button') end end + + context 'for admin' do + shared_examples_for 'shows mirror settings' do + it 'shows mirror settings' do + expect(page.find('.project-mirror-settings')).to have_selector('form') + expect(page).not_to have_content('Changing mirroring setting is disabled for non-admin users.') + end + end + + before do + stub_application_setting(mirror_available: mirror_available) + user.update!(admin: true) + visit project_settings_repository_path(project) + end + + context 'when project mirroring is enabled' do + let(:mirror_available) { true } + + include_examples 'shows mirror settings' + end + + context 'when project mirroring is disabled' do + let(:mirror_available) { false } + + include_examples 'shows mirror settings' + end + end end diff --git a/spec/frontend/clusters/components/applications_spec.js b/spec/frontend/clusters/components/applications_spec.js index 3e25c825fe8..db5d5c84820 100644 --- a/spec/frontend/clusters/components/applications_spec.js +++ b/spec/frontend/clusters/components/applications_spec.js @@ -15,6 +15,9 @@ describe('Applications', () => { beforeEach(() => { Applications = Vue.extend(applications); + + gon.features = gon.features || {}; + gon.features.managedAppsLocalTiller = false; }); afterEach(() => { @@ -156,6 +159,22 @@ describe('Applications', () => { }); }); + describe('Helm application', () => { + describe('when managedAppsLocalTiller enabled', () => { + beforeEach(() => { + gon.features.managedAppsLocalTiller = true; + }); + + it('does not render a row for Helm Tiller', () => { + vm = mountComponent(Applications, { + applications: APPLICATIONS_MOCK_STATE, + }); + + expect(vm.$el.querySelector('.js-cluster-application-row-helm')).toBeNull(); + }); + }); + }); + describe('Ingress application', () => { describe('with nested component', () => { const propsData = { diff --git a/spec/frontend/fixtures/autocomplete_sources.rb b/spec/frontend/fixtures/autocomplete_sources.rb index 382eff02b0f..812364c8b06 100644 --- a/spec/frontend/fixtures/autocomplete_sources.rb +++ b/spec/frontend/fixtures/autocomplete_sources.rb @@ -5,10 +5,10 @@ require 'spec_helper' describe Projects::AutocompleteSourcesController, '(JavaScript fixtures)', type: :controller do include JavaScriptFixturesHelpers - set(:admin) { create(:admin) } - set(:group) { create(:group, name: 'frontend-fixtures') } - set(:project) { create(:project, namespace: group, path: 'autocomplete-sources-project') } - set(:issue) { create(:issue, project: project) } + let_it_be(:admin) { create(:admin) } + let_it_be(:group) { create(:group, name: 'frontend-fixtures') } + let_it_be(:project) { create(:project, namespace: group, path: 'autocomplete-sources-project') } + let_it_be(:issue) { create(:issue, project: project) } before(:all) do clean_frontend_fixtures('autocomplete_sources/') diff --git a/spec/frontend/fixtures/commit.rb b/spec/frontend/fixtures/commit.rb index 2c4bf6fbd3d..c9a5aa9a67c 100644 --- a/spec/frontend/fixtures/commit.rb +++ b/spec/frontend/fixtures/commit.rb @@ -5,9 +5,9 @@ require 'spec_helper' describe Projects::CommitController, '(JavaScript fixtures)', type: :controller do include JavaScriptFixturesHelpers - set(:project) { create(:project, :repository) } - set(:user) { create(:user) } - let(:commit) { project.commit("master") } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + let(:commit) { project.commit("master") } render_views diff --git a/spec/javascripts/frequent_items/utils_spec.js b/spec/javascripts/frequent_items/utils_spec.js index fd5bd002428..2939b46bc31 100644 --- a/spec/javascripts/frequent_items/utils_spec.js +++ b/spec/javascripts/frequent_items/utils_spec.js @@ -108,5 +108,23 @@ describe('Frequent Items utils spec', () => { expect(sanitizeItem(input)).toEqual({ name: 'test', namespace: 'test', id: 1 }); }); + + it("skips `name` key if it doesn't exist on the item", () => { + const input = { + namespace: '<br>test', + id: 1, + }; + + expect(sanitizeItem(input)).toEqual({ namespace: 'test', id: 1 }); + }); + + it("skips `namespace` key if it doesn't exist on the item", () => { + const input = { + name: '<br><b>test</b>', + id: 1, + }; + + expect(sanitizeItem(input)).toEqual({ name: 'test', id: 1 }); + }); }); }); diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb index 5a672de13d7..de7a70db1ac 100644 --- a/spec/lib/banzai/filter/label_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb @@ -523,7 +523,12 @@ describe Banzai::Filter::LabelReferenceFilter do end context 'when group name has HTML entities' do - let(:another_group) { create(:group, name: '<img src=x onerror=alert(1)>', path: 'another_group') } + let(:another_group) { create(:group, name: 'random', path: 'another_group') } + + before do + another_group.name = "<img src=x onerror=alert(1)>" + another_group.save!(validate: false) + end it 'escapes the HTML entities' do expect(result.text) diff --git a/spec/lib/banzai/filter/reference_redactor_filter_spec.rb b/spec/lib/banzai/filter/reference_redactor_filter_spec.rb index 9739afd3d57..a68581b3000 100644 --- a/spec/lib/banzai/filter/reference_redactor_filter_spec.rb +++ b/spec/lib/banzai/filter/reference_redactor_filter_spec.rb @@ -20,8 +20,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do it 'skips when the skip_redaction flag is set' do user = create(:user) project = create(:project) - link = reference_link(project: project.id, reference_type: 'test') + doc = filter(link, current_user: user, skip_redaction: true) expect(doc.css('a').length).to eq 1 @@ -51,8 +51,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do user = create(:user) project = create(:project) project.add_maintainer(user) - link = reference_link(project: project.id, reference_type: 'test') + doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 @@ -69,8 +69,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do it 'removes unpermitted references' do user = create(:user) project = create(:project) - link = reference_link(project: project.id, reference_type: 'test') + doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 0 @@ -90,8 +90,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do non_member = create(:user) project = create(:project, :public) issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') + doc = filter(link, current_user: non_member) expect(doc.css('a').length).to eq 0 @@ -124,8 +124,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do assignee = create(:user) project = create(:project, :public) issue = create(:issue, :confidential, project: project, assignees: [assignee]) - link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') + doc = filter(link, current_user: assignee) expect(doc.css('a').length).to eq 1 @@ -136,8 +136,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do project = create(:project, :public) project.add_developer(member) issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') + doc = filter(link, current_user: member) expect(doc.css('a').length).to eq 1 @@ -147,20 +147,62 @@ describe Banzai::Filter::ReferenceRedactorFilter do admin = create(:admin) project = create(:project, :public) issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') + doc = filter(link, current_user: admin) expect(doc.css('a').length).to eq 1 end + + context "when a confidential issue is moved from a public project to a private one" do + let(:public_project) { create(:project, :public) } + let(:private_project) { create(:project, :private) } + + it 'removes references for author' do + author = create(:user) + issue = create(:issue, :confidential, project: public_project, author: author) + issue.update!(project: private_project) # move issue to private project + link = reference_link(project: private_project.id, issue: issue.id, reference_type: 'issue') + + doc = filter(link, current_user: author) + + expect(doc.css('a').length).to eq 0 + end + + it 'removes references for assignee' do + assignee = create(:user) + issue = create(:issue, :confidential, project: public_project, assignees: [assignee]) + issue.update!(project: private_project) # move issue to private project + link = reference_link(project: private_project.id, issue: issue.id, reference_type: 'issue') + + doc = filter(link, current_user: assignee) + + expect(doc.css('a').length).to eq 0 + end + + it 'allows references for project members' do + member = create(:user) + project = create(:project, :public) + project_2 = create(:project, :private) + project.add_developer(member) + project_2.add_developer(member) + issue = create(:issue, :confidential, project: project) + issue.update!(project: project_2) # move issue to private project + link = reference_link(project: project_2.id, issue: issue.id, reference_type: 'issue') + + doc = filter(link, current_user: member) + + expect(doc.css('a').length).to eq 1 + end + end end it 'allows references for non confidential issues' do user = create(:user) project = create(:project, :public) issue = create(:issue, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') + doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 @@ -172,8 +214,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do it 'removes unpermitted Group references' do user = create(:user) group = create(:group, :private) - link = reference_link(group: group.id, reference_type: 'user') + doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 0 @@ -183,8 +225,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do user = create(:user) group = create(:group, :private) group.add_developer(user) - link = reference_link(group: group.id, reference_type: 'user') + doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 @@ -200,8 +242,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do context 'with data-user' do it 'allows any User reference' do user = create(:user) - link = reference_link(user: user.id, reference_type: 'user') + doc = filter(link) expect(doc.css('a').length).to eq 1 diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index e0c1f830165..528019bb9ff 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -164,6 +164,12 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do expect(subject).to eq(Gitlab::Auth::Result.new(build.user, build.project, :build, described_class.build_authentication_abilities)) end + + it 'fails with blocked user token' do + build.update(user: create(:user, :blocked)) + + expect(subject).to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil)) + end end (HasStatus::AVAILABLE_STATUSES - ['running']).each do |build_status| @@ -259,6 +265,15 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip') end + + context 'blocked user' do + let(:user) { create(:user, :blocked) } + + it 'fails' do + expect(gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip')) + .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil)) + end + end end context 'while using personal access tokens as passwords' do @@ -307,9 +322,35 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it 'fails if password is nil' do expect_results_with_abilities(nil, nil, false) end + + context 'when user is blocked' do + let(:user) { create(:user, :blocked) } + let(:personal_access_token) { create(:personal_access_token, scopes: ['read_registry'], user: user) } + + before do + stub_container_registry_config(enabled: true) + end + + it 'fails if user is blocked' do + expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')) + .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil)) + end + end end context 'while using regular user and password' do + it 'fails for a blocked user' do + user = create( + :user, + :blocked, + username: 'normal_user', + password: 'my-secret' + ) + + expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')) + .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil)) + end + it 'goes through lfs authentication' do user = create( :user, diff --git a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb index 5a930d44dcb..ebd7c7af265 100644 --- a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb +++ b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb @@ -68,6 +68,16 @@ describe Gitlab::Gfm::UploadsRewriter do expect(moved_text.scan(/\A\[.*?\]/).count).to eq(1) end + context 'path traversal in file name' do + let(:text) do + "" + end + + it 'throw an error' do + expect { rewriter.rewrite(new_project) }.to raise_error(an_instance_of(StandardError).and having_attributes(message: "Invalid path")) + end + end + context "file are stored locally" do include_examples "files are accessible" end diff --git a/spec/lib/gitlab/hotlinking_detector_spec.rb b/spec/lib/gitlab/hotlinking_detector_spec.rb new file mode 100644 index 00000000000..536d744c197 --- /dev/null +++ b/spec/lib/gitlab/hotlinking_detector_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Gitlab::HotlinkingDetector do + describe ".intercept_hotlinking?" do + using RSpec::Parameterized::TableSyntax + + subject { described_class.intercept_hotlinking?(request) } + + let(:request) { double("request", headers: headers) } + let(:headers) { {} } + + context "hotlinked as media" do + where(:return_value, :accept_header) do + # These are default formats in modern browsers, and IE + false | "*/*" + false | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" + false | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8" + false | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" + false | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8" + false | "image/jpeg, application/x-ms-application, image/gif, application/xaml+xml, image/pjpeg, application/x-ms-xbap, application/x-shockwave-flash, application/msword, */*" + false | "text/html, application/xhtml+xml, image/jxr, */*" + false | "text/html, application/xml;q=0.9, application/xhtml+xml, image/png, image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1" + + # These are image request formats + true | "image/webp,*/*" + true | "image/png,image/*;q=0.8,*/*;q=0.5" + true | "image/webp,image/apng,image/*,*/*;q=0.8" + true | "image/png,image/svg+xml,image/*;q=0.8, */*;q=0.5" + + # Video request formats + true | "video/webm,video/ogg,video/*;q=0.9,application/ogg;q=0.7,audio/*;q=0.6,*/*;q=0.5" + + # Audio request formats + true | "audio/webm,audio/ogg,audio/wav,audio/*;q=0.9,application/ogg;q=0.7,video/*;q=0.6,*/*;q=0.5" + + # CSS request formats + true | "text/css,*/*;q=0.1" + true | "text/css" + true | "text/css,*/*;q=0.1" + end + + with_them do + let(:headers) do + { "Accept" => accept_header } + end + + it { is_expected.to be(return_value) } + end + end + + context "hotlinked as a script" do + where(:return_value, :fetch_mode) do + # Standard navigation fetch modes + false | "navigate" + false | "nested-navigate" + false | "same-origin" + + # Fetch modes when linking as JS + true | "cors" + true | "no-cors" + true | "websocket" + end + + with_them do + let(:headers) do + { "Sec-Fetch-Mode" => fetch_mode } + end + + it { is_expected.to be(return_value) } + end + end + end +end diff --git a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb index 12857f97f7c..65e99c0c3b8 100644 --- a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb +++ b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb @@ -32,6 +32,9 @@ describe Gitlab::ImportExport::AttributeCleaner do 'issue_ids' => [1, 2, 3], 'merge_request_ids' => [1, 2, 3], 'note_ids' => [1, 2, 3], + 'remote_attachment_url' => 'http://something.dodgy', + 'remote_attachment_request_header' => 'bad value', + 'remote_attachment_urls' => %w(http://something.dodgy http://something.okay), 'attributes' => { 'issue_ids' => [1, 2, 3], 'merge_request_ids' => [1, 2, 3], diff --git a/spec/lib/gitlab/import_export/json/legacy_reader/file_spec.rb b/spec/lib/gitlab/import_export/json/legacy_reader/file_spec.rb new file mode 100644 index 00000000000..1021ce3cd50 --- /dev/null +++ b/spec/lib/gitlab/import_export/json/legacy_reader/file_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative 'shared_example.rb' + +describe Gitlab::ImportExport::JSON::LegacyReader::File do + it_behaves_like 'import/export json legacy reader' do + let(:valid_path) { 'spec/fixtures/lib/gitlab/import_export/light/project.json' } + let(:data) { valid_path } + let(:json_data) { JSON.parse(File.read(valid_path)) } + end + + describe '#exist?' do + let(:legacy_reader) do + described_class.new(path, relation_names: []) + end + + subject { legacy_reader.exist? } + + context 'given valid path' do + let(:path) { 'spec/fixtures/lib/gitlab/import_export/light/project.json' } + + it { is_expected.to be true } + end + + context 'given invalid path' do + let(:path) { 'spec/non-existing-folder/do-not-create-this-file.json' } + + it { is_expected.to be false } + end + end +end diff --git a/spec/lib/gitlab/import_export/json/legacy_reader/hash_spec.rb b/spec/lib/gitlab/import_export/json/legacy_reader/hash_spec.rb new file mode 100644 index 00000000000..8c4dfd2f356 --- /dev/null +++ b/spec/lib/gitlab/import_export/json/legacy_reader/hash_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative 'shared_example.rb' + +describe Gitlab::ImportExport::JSON::LegacyReader::Hash do + it_behaves_like 'import/export json legacy reader' do + let(:path) { 'spec/fixtures/lib/gitlab/import_export/light/project.json' } + + # the hash is modified by the `LegacyReader` + # we need to deep-dup it + let(:json_data) { JSON.parse(File.read(path)) } + let(:data) { JSON.parse(File.read(path)) } + end + + describe '#exist?' do + let(:legacy_reader) do + described_class.new(tree_hash, relation_names: []) + end + + subject { legacy_reader.exist? } + + context 'tree_hash is nil' do + let(:tree_hash) { nil } + + it { is_expected.to be_falsey } + end + + context 'tree_hash presents' do + let(:tree_hash) { { "issues": [] } } + + it { is_expected.to be_truthy } + end + end +end diff --git a/spec/lib/gitlab/import_export/json/legacy_reader/shared_example.rb b/spec/lib/gitlab/import_export/json/legacy_reader/shared_example.rb new file mode 100644 index 00000000000..297a5946703 --- /dev/null +++ b/spec/lib/gitlab/import_export/json/legacy_reader/shared_example.rb @@ -0,0 +1,110 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'import/export json legacy reader' do + let(:relation_names) { [] } + + let(:legacy_reader) do + described_class.new( + data, + relation_names: relation_names, + allowed_path: "project") + end + + describe '#consume_attributes' do + context 'when valid path is passed' do + subject { legacy_reader.consume_attributes("project") } + + context 'no excluded attributes' do + let(:excluded_attributes) { [] } + let(:relation_names) { [] } + + it 'returns the whole tree from parsed JSON' do + expect(subject).to eq(json_data) + end + end + + context 'some attributes are excluded' do + let(:relation_names) { %w[milestones labels] } + + it 'returns hash without excluded attributes and relations' do + expect(subject).not_to include('milestones', 'labels') + end + end + end + + context 'when invalid path is passed' do + it 'raises an exception' do + expect { legacy_reader.consume_attributes("invalid-path") } + .to raise_error(ArgumentError) + end + end + end + + describe '#consume_relation' do + context 'when valid path is passed' do + let(:key) { 'description' } + + context 'block not given' do + it 'returns value of the key' do + expect(legacy_reader).to receive(:relations).and_return({ key => 'test value' }) + expect(legacy_reader.consume_relation("project", key)).to eq('test value') + end + end + + context 'key has been consumed' do + before do + legacy_reader.consume_relation("project", key) + end + + it 'does not yield' do + expect do |blk| + legacy_reader.consume_relation("project", key, &blk) + end.not_to yield_control + end + end + + context 'value is nil' do + before do + expect(legacy_reader).to receive(:relations).and_return({ key => nil }) + end + + it 'does not yield' do + expect do |blk| + legacy_reader.consume_relation("project", key, &blk) + end.not_to yield_control + end + end + + context 'value is not array' do + before do + expect(legacy_reader).to receive(:relations).and_return({ key => 'value' }) + end + + it 'yield the value with index 0' do + expect do |blk| + legacy_reader.consume_relation("project", key, &blk) + end.to yield_with_args('value', 0) + end + end + + context 'value is an array' do + before do + expect(legacy_reader).to receive(:relations).and_return({ key => %w[item1 item2 item3] }) + end + + it 'yield each array element with index' do + expect do |blk| + legacy_reader.consume_relation("project", key, &blk) + end.to yield_successive_args(['item1', 0], ['item2', 1], ['item3', 2]) + end + end + end + + context 'when invalid path is passed' do + it 'raises an exception' do + expect { legacy_reader.consume_relation("invalid") } + .to raise_error(ArgumentError) + end + end + end +end diff --git a/spec/lib/gitlab/import_export/json/legacy_reader_spec.rb b/spec/lib/gitlab/import_export/json/legacy_reader_spec.rb deleted file mode 100644 index 0009a5f81de..00000000000 --- a/spec/lib/gitlab/import_export/json/legacy_reader_spec.rb +++ /dev/null @@ -1,149 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::ImportExport::JSON::LegacyReader::User do - let(:relation_names) { [] } - let(:legacy_reader) { described_class.new(tree_hash, relation_names) } - - describe '#valid?' do - subject { legacy_reader.valid? } - - context 'tree_hash not present' do - let(:tree_hash) { nil } - - it { is_expected.to be false } - end - - context 'tree_hash presents' do - let(:tree_hash) { { "issues": [] } } - - it { is_expected.to be true } - end - end -end - -describe Gitlab::ImportExport::JSON::LegacyReader::File do - let(:fixture) { 'spec/fixtures/lib/gitlab/import_export/light/project.json' } - let(:project_tree) { JSON.parse(File.read(fixture)) } - let(:relation_names) { [] } - let(:legacy_reader) { described_class.new(path, relation_names) } - - describe '#valid?' do - subject { legacy_reader.valid? } - - context 'given valid path' do - let(:path) { fixture } - - it { is_expected.to be true } - end - - context 'given invalid path' do - let(:path) { 'spec/non-existing-folder/do-not-create-this-file.json' } - - it { is_expected.to be false } - end - end - - describe '#root_attributes' do - let(:path) { fixture } - - subject { legacy_reader.root_attributes(excluded_attributes) } - - context 'No excluded attributes' do - let(:excluded_attributes) { [] } - let(:relation_names) { [] } - - it 'returns the whole tree from parsed JSON' do - expect(subject).to eq(project_tree) - end - end - - context 'Some attributes are excluded' do - let(:excluded_attributes) { %w[milestones labels issues services snippets] } - let(:relation_names) { %w[import_type archived] } - - it 'returns hash without excluded attributes and relations' do - expect(subject).not_to include('milestones', 'labels', 'issues', 'services', 'snippets', 'import_type', 'archived') - end - end - end - - describe '#consume_relation' do - let(:path) { fixture } - let(:key) { 'description' } - - context 'block not given' do - it 'returns value of the key' do - expect(legacy_reader).to receive(:relations).and_return({ key => 'test value' }) - expect(legacy_reader.consume_relation(key)).to eq('test value') - end - end - - context 'key has been consumed' do - before do - legacy_reader.consume_relation(key) - end - - it 'does not yield' do - expect do |blk| - legacy_reader.consume_relation(key, &blk) - end.not_to yield_control - end - end - - context 'value is nil' do - before do - expect(legacy_reader).to receive(:relations).and_return({ key => nil }) - end - - it 'does not yield' do - expect do |blk| - legacy_reader.consume_relation(key, &blk) - end.not_to yield_control - end - end - - context 'value is not array' do - before do - expect(legacy_reader).to receive(:relations).and_return({ key => 'value' }) - end - - it 'yield the value with index 0' do - expect do |blk| - legacy_reader.consume_relation(key, &blk) - end.to yield_with_args('value', 0) - end - end - - context 'value is an array' do - before do - expect(legacy_reader).to receive(:relations).and_return({ key => %w[item1 item2 item3] }) - end - - it 'yield each array element with index' do - expect do |blk| - legacy_reader.consume_relation(key, &blk) - end.to yield_successive_args(['item1', 0], ['item2', 1], ['item3', 2]) - end - end - end - - describe '#tree_hash' do - let(:path) { fixture } - - subject { legacy_reader.send(:tree_hash) } - - it 'parses the JSON into the expected tree' do - expect(subject).to eq(project_tree) - end - - context 'invalid JSON' do - let(:path) { 'spec/fixtures/lib/gitlab/import_export/invalid_json/project.json' } - - it 'raise Exception' do - expect { subject }.to raise_exception(Gitlab::ImportExport::Error, 'Incorrect JSON format') - end - end - end -end diff --git a/spec/lib/gitlab/import_export/json/legacy_writer_spec.rb b/spec/lib/gitlab/import_export/json/legacy_writer_spec.rb index b4cdfee3b22..1f39b26e46a 100644 --- a/spec/lib/gitlab/import_export/json/legacy_writer_spec.rb +++ b/spec/lib/gitlab/import_export/json/legacy_writer_spec.rb @@ -5,20 +5,37 @@ require 'spec_helper' describe Gitlab::ImportExport::JSON::LegacyWriter do let(:path) { "#{Dir.tmpdir}/legacy_writer_spec/test.json" } - subject { described_class.new(path) } + subject do + described_class.new(path, allowed_path: "project") + end after do FileUtils.rm_rf(path) end - describe "#write" do + describe "#write_attributes" do + it "writes correct json" do + expected_hash = { "key" => "value_1", "key_1" => "value_2" } + subject.write_attributes("project", expected_hash) + + expect(subject_json).to eq(expected_hash) + end + + context 'when invalid path is used' do + it 'raises an exception' do + expect { subject.write_attributes("invalid", { "key" => "value" }) } + .to raise_error(ArgumentError) + end + end + end + + describe "#write_relation" do context "when key is already written" do it "raises exception" do - key = "key" - value = "value" - subject.write(key, value) + subject.write_relation("project", "key", "old value") - expect { subject.write(key, "new value") }.to raise_exception("key '#{key}' already written") + expect { subject.write_relation("project", "key", "new value") } + .to raise_exception("key 'key' already written") end end @@ -27,53 +44,58 @@ describe Gitlab::ImportExport::JSON::LegacyWriter do it "writes correct json" do expected_hash = { "key" => "value_1", "key_1" => "value_2" } expected_hash.each do |key, value| - subject.write(key, value) + subject.write_relation("project", key, value) end - subject.close - expect(saved_json(path)).to eq(expected_hash) + expect(subject_json).to eq(expected_hash) end end end + + context 'when invalid path is used' do + it 'raises an exception' do + expect { subject.write_relation("invalid", "key", "value") } + .to raise_error(ArgumentError) + end + end end - describe "#append" do - context "when key is already written" do - it "appends values under a given key" do - key = "key" - values = %w(value_1 value_2) - expected_hash = { key => values } - values.each do |value| - subject.append(key, value) - end - subject.close + describe "#write_relation_array" do + context 'when array is used' do + it 'writes correct json' do + subject.write_relation_array("project", "key", ["value"]) - expect(saved_json(path)).to eq(expected_hash) + expect(subject_json).to eq({ "key" => ["value"] }) end end - context "when key is not already written" do - it "writes correct json" do - expected_hash = { "key" => ["value"] } - subject.append("key", "value") - subject.close + context 'when enumerable is used' do + it 'writes correct json' do + values = %w(value1 value2) + + enumerator = Enumerator.new do |items| + values.each { |value| items << value } + end + + subject.write_relation_array("project", "key", enumerator) - expect(saved_json(path)).to eq(expected_hash) + expect(subject_json).to eq({ "key" => values }) end end - end - describe "#set" do - it "writes correct json" do - expected_hash = { "key" => "value_1", "key_1" => "value_2" } - subject.set(expected_hash) - subject.close + context "when key is already written" do + it "raises an exception" do + subject.write_relation_array("project", "key", %w(old_value)) - expect(saved_json(path)).to eq(expected_hash) + expect { subject.write_relation_array("project", "key", %w(new_value)) } + .to raise_error(ArgumentError) + end end end - def saved_json(filename) - ::JSON.parse(IO.read(filename)) + def subject_json + subject.close + + ::JSON.parse(IO.read(subject.path)) end end diff --git a/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb index e36144b1a30..52e1efa70e0 100644 --- a/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb @@ -14,23 +14,24 @@ describe Gitlab::ImportExport::RelationTreeRestorer do let(:user) { create(:user) } let(:shared) { Gitlab::ImportExport::Shared.new(importable) } - let(:members_mapper) { Gitlab::ImportExport::MembersMapper.new(exported_members: {}, user: user, importable: importable) } + let(:attributes) { {} } - let(:importable_hash) do - json = IO.read(path) - ActiveSupport::JSON.decode(json) + let(:members_mapper) do + Gitlab::ImportExport::MembersMapper.new(exported_members: {}, user: user, importable: importable) end let(:relation_tree_restorer) do described_class.new( - user: user, - shared: shared, - relation_reader: relation_reader, - importable: importable, - object_builder: object_builder, - members_mapper: members_mapper, - relation_factory: relation_factory, - reader: reader + user: user, + shared: shared, + relation_reader: relation_reader, + object_builder: object_builder, + members_mapper: members_mapper, + relation_factory: relation_factory, + reader: reader, + importable: importable, + importable_path: nil, + importable_attributes: attributes ) end @@ -100,7 +101,14 @@ describe Gitlab::ImportExport::RelationTreeRestorer do let(:reader) { Gitlab::ImportExport::Reader.new(shared: shared) } context 'using legacy reader' do - let(:relation_reader) { Gitlab::ImportExport::JSON::LegacyReader::File.new(path, reader.project_relation_names) } + let(:relation_reader) do + Gitlab::ImportExport::JSON::LegacyReader::File.new( + path, + relation_names: reader.project_relation_names + ) + end + + let(:attributes) { relation_reader.consume_attributes(nil) } it_behaves_like 'import project successfully' @@ -119,7 +127,7 @@ describe Gitlab::ImportExport::RelationTreeRestorer do let(:importable) { create(:group, parent: group) } let(:object_builder) { Gitlab::ImportExport::Group::ObjectBuilder } let(:relation_factory) { Gitlab::ImportExport::Group::RelationFactory } - let(:relation_reader) { Gitlab::ImportExport::JSON::LegacyReader::File.new(path, reader.group_relation_names) } + let(:relation_reader) { Gitlab::ImportExport::JSON::LegacyReader::File.new(path, relation_names: reader.group_relation_names) } let(:reader) do Gitlab::ImportExport::Reader.new( shared: shared, diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index c580b46cf8d..f1b5393a2d8 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -3,9 +3,7 @@ require 'spec_helper' describe Gitlab::Regex do - describe '.project_name_regex' do - subject { described_class.project_name_regex } - + shared_examples_for 'project/group name regex' do it { is_expected.to match('gitlab-ce') } it { is_expected.to match('GitLab CE') } it { is_expected.to match('100 lines') } @@ -15,6 +13,34 @@ describe Gitlab::Regex do it { is_expected.not_to match('?gitlab') } end + shared_examples_for 'project/group name error message' do + it { is_expected.to eq("can contain only letters, digits, emojis, '_', '.', dash, space. It must start with letter, digit, emoji or '_'.") } + end + + describe '.project_name_regex' do + subject { described_class.project_name_regex } + + it_behaves_like 'project/group name regex' + end + + describe '.group_name_regex' do + subject { described_class.group_name_regex } + + it_behaves_like 'project/group name regex' + end + + describe '.project_name_regex_message' do + subject { described_class.project_name_regex_message } + + it_behaves_like 'project/group name error message' + end + + describe '.group_name_regex_message' do + subject { described_class.group_name_regex_message } + + it_behaves_like 'project/group name error message' + end + describe '.environment_name_regex' do subject { described_class.environment_name_regex } diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index f49abb24c44..c574cc91a59 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -13,9 +13,9 @@ describe Notify do let(:current_user_sanitized) { 'www_example_com' } - let_it_be(:user) { create(:user) } - let_it_be(:current_user) { create(:user, email: "current@email.com", name: 'www.example.com') } - let_it_be(:assignee) { create(:user, email: 'assignee@example.com', name: 'John Doe') } + let_it_be(:user, reload: true) { create(:user) } + let_it_be(:current_user, reload: true) { create(:user, email: "current@email.com", name: 'www.example.com') } + let_it_be(:assignee, reload: true) { create(:user, email: 'assignee@example.com', name: 'John Doe') } let_it_be(:merge_request) do create(:merge_request, source_project: project, diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 576ac4393ed..576ac880fca 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -48,6 +48,9 @@ describe Group do describe 'validations' do it { is_expected.to validate_presence_of :name } + it { is_expected.to allow_value('group test_4').for(:name) } + it { is_expected.not_to allow_value('test/../foo').for(:name) } + it { is_expected.not_to allow_value('<script>alert("Attack!")</script>').for(:name) } it { is_expected.to validate_presence_of :path } it { is_expected.not_to validate_presence_of :owner } it { is_expected.to validate_presence_of :two_factor_grace_period } diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 6f12d72c723..9797e0a0472 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -529,222 +529,258 @@ describe Issue do end describe '#visible_to_user?' do + let(:project) { build(:project) } + let(:issue) { build(:issue, project: project) } + let(:user) { create(:user) } + + subject { issue.visible_to_user?(user) } + + context 'with a project' do + it 'returns false when feature is disabled' do + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) + + is_expected.to eq(false) + end + + it 'returns false when restricted for members' do + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE) + + is_expected.to eq(false) + end + end + context 'without a user' do - let(:issue) { build(:issue) } + let(:user) { nil } it 'returns true when the issue is publicly visible' do expect(issue).to receive(:publicly_visible?).and_return(true) - expect(issue.visible_to_user?).to eq(true) + is_expected.to eq(true) end it 'returns false when the issue is not publicly visible' do expect(issue).to receive(:publicly_visible?).and_return(false) - expect(issue.visible_to_user?).to eq(false) + is_expected.to eq(false) end end context 'with a user' do - let(:user) { create(:user) } - let(:issue) { build(:issue) } - - it 'returns true when the issue is readable' do - expect(issue).to receive(:readable_by?).with(user).and_return(true) - - expect(issue.visible_to_user?(user)).to eq(true) + shared_examples 'issue readable by user' do + it { is_expected.to eq(true) } end - it 'returns false when the issue is not readable' do - expect(issue).to receive(:readable_by?).with(user).and_return(false) - - expect(issue.visible_to_user?(user)).to eq(false) + shared_examples 'issue not readable by user' do + it { is_expected.to eq(false) } end - it 'returns false when feature is disabled' do - expect(issue).not_to receive(:readable_by?) - - issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) + shared_examples 'confidential issue readable by user' do + specify do + issue.confidential = true - expect(issue.visible_to_user?(user)).to eq(false) + is_expected.to eq(true) + end end - it 'returns false when restricted for members' do - expect(issue).not_to receive(:readable_by?) - - issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE) + shared_examples 'confidential issue not readable by user' do + specify do + issue.confidential = true - expect(issue.visible_to_user?(user)).to eq(false) + is_expected.to eq(false) + end end - end - - describe 'with a regular user that is not a team member' do - let(:user) { create(:user) } - context 'using a public project' do - let(:project) { create(:project, :public) } + context 'with an admin user' do + let(:user) { build(:admin) } - it 'returns true for a regular issue' do - issue = build(:issue, project: project) + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' + end - expect(issue.visible_to_user?(user)).to eq(true) + context 'with an owner' do + before do + project.add_maintainer(user) end - it 'returns false for a confidential issue' do - issue = build(:issue, project: project, confidential: true) + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' + end - expect(issue.visible_to_user?(user)).to eq(false) + context 'with a reporter user' do + before do + project.add_reporter(user) end + + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' end - context 'using an internal project' do - let(:project) { create(:project, :internal) } + context 'with a guest user' do + before do + project.add_guest(user) + end - context 'using an internal user' do - it 'returns true for a regular issue' do - issue = build(:issue, project: project) + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue not readable by user' - expect(issue.visible_to_user?(user)).to eq(true) + context 'when user is an assignee' do + before do + issue.update!(assignees: [user]) end - it 'returns false for a confidential issue' do - issue = build(:issue, :confidential, project: project) - - expect(issue.visible_to_user?(user)).to eq(false) - end + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' end - context 'using an external user' do + context 'when user is the author' do before do - allow(user).to receive(:external?).and_return(true) - end - - it 'returns false for a regular issue' do - issue = build(:issue, project: project) - - expect(issue.visible_to_user?(user)).to eq(false) + issue.update!(author: user) end - it 'returns false for a confidential issue' do - issue = build(:issue, :confidential, project: project) - - expect(issue.visible_to_user?(user)).to eq(false) - end + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' end end - context 'using a private project' do - let(:project) { create(:project, :private) } - - it 'returns false for a regular issue' do - issue = build(:issue, project: project) + context 'with a user that is not a member' do + context 'using a public project' do + let(:project) { build(:project, :public) } - expect(issue.visible_to_user?(user)).to eq(false) + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue not readable by user' end - it 'returns false for a confidential issue' do - issue = build(:issue, :confidential, project: project) + context 'using an internal project' do + let(:project) { build(:project, :internal) } - expect(issue.visible_to_user?(user)).to eq(false) - end + context 'using an internal user' do + before do + allow(user).to receive(:external?).and_return(false) + end - context 'when the user is the project owner' do - before do - project.add_maintainer(user) + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue not readable by user' end - it 'returns true for a regular issue' do - issue = build(:issue, project: project) + context 'using an external user' do + before do + allow(user).to receive(:external?).and_return(true) + end - expect(issue.visible_to_user?(user)).to eq(true) + it_behaves_like 'issue not readable by user' + it_behaves_like 'confidential issue not readable by user' end + end - it 'returns true for a confidential issue' do - issue = build(:issue, :confidential, project: project) - - expect(issue.visible_to_user?(user)).to eq(true) + context 'using an external user' do + before do + allow(user).to receive(:external?).and_return(true) end + + it_behaves_like 'issue not readable by user' + it_behaves_like 'confidential issue not readable by user' end end - end - - context 'with a regular user that is a team member' do - let(:user) { create(:user) } - let(:project) { create(:project, :public) } - context 'using a public project' do + context 'with an external authentication service' do before do - project.add_developer(user) + enable_external_authorization_service_check end - it 'returns true for a regular issue' do - issue = build(:issue, project: project) + it 'is `false` when an external authorization service is enabled' do + issue = build(:issue, project: build(:project, :public)) - expect(issue.visible_to_user?(user)).to eq(true) + expect(issue).not_to be_visible_to_user end - it 'returns true for a confidential issue' do - issue = build(:issue, :confidential, project: project) - - expect(issue.visible_to_user?(user)).to eq(true) - end - end - - context 'using an internal project' do - let(:project) { create(:project, :internal) } + it 'checks the external service to determine if an issue is readable by a user' do + project = build(:project, :public, + external_authorization_classification_label: 'a-label') + issue = build(:issue, project: project) + user = build(:user) - before do - project.add_developer(user) + expect(::Gitlab::ExternalAuthorization).to receive(:access_allowed?).with(user, 'a-label') { false } + expect(issue.visible_to_user?(user)).to be_falsy end - it 'returns true for a regular issue' do + it 'does not check the external service if a user does not have access to the project' do + project = build(:project, :private, + external_authorization_classification_label: 'a-label') issue = build(:issue, project: project) + user = build(:user) - expect(issue.visible_to_user?(user)).to eq(true) + expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?) + expect(issue.visible_to_user?(user)).to be_falsy end - it 'returns true for a confidential issue' do - issue = build(:issue, :confidential, project: project) + it 'does not check the external webservice for admins' do + issue = build(:issue) + user = build(:admin) - expect(issue.visible_to_user?(user)).to eq(true) + expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?) + + issue.visible_to_user?(user) end end - context 'using a private project' do - let(:project) { create(:project, :private) } + context 'when issue is moved to a private project' do + let(:private_project) { build(:project, :private)} before do - project.add_developer(user) + issue.update(project: private_project) # move issue to private project end - it 'returns true for a regular issue' do - issue = build(:issue, project: project) + shared_examples 'issue visible if user has guest access' do + context 'when user is not a member' do + it_behaves_like 'issue not readable by user' + it_behaves_like 'confidential issue not readable by user' + end - expect(issue.visible_to_user?(user)).to eq(true) + context 'when user is a guest' do + before do + private_project.add_guest(user) + end + + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' + end end - it 'returns true for a confidential issue' do - issue = build(:issue, :confidential, project: project) + context 'when user is the author of the original issue' do + before do + issue.update!(author: user) + end - expect(issue.visible_to_user?(user)).to eq(true) + it_behaves_like 'issue visible if user has guest access' end - end - end - context 'with an admin user' do - let(:project) { create(:project) } - let(:user) { create(:admin) } + context 'when user is an assignee in the original issue' do + before do + issue.update!(assignees: [user]) + end - it 'returns true for a regular issue' do - issue = build(:issue, project: project) + it_behaves_like 'issue visible if user has guest access' + end - expect(issue.visible_to_user?(user)).to eq(true) - end + context 'when user is not the author or an assignee in original issue' do + context 'when user is a guest' do + before do + private_project.add_guest(user) + end - it 'returns true for a confidential issue' do - issue = build(:issue, :confidential, project: project) + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue not readable by user' + end - expect(issue.visible_to_user?(user)).to eq(true) + context 'when user is a reporter' do + before do + private_project.add_reporter(user) + end + + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' + end + end end end end @@ -868,49 +904,6 @@ describe Issue do subject { create(:issue, updated_at: 1.hour.ago) } end - context 'when an external authentication service' do - before do - enable_external_authorization_service_check - end - - describe '#visible_to_user?' do - it 'is `false` when an external authorization service is enabled' do - issue = build(:issue, project: build(:project, :public)) - - expect(issue).not_to be_visible_to_user - end - - it 'checks the external service to determine if an issue is readable by a user' do - project = build(:project, :public, - external_authorization_classification_label: 'a-label') - issue = build(:issue, project: project) - user = build(:user) - - expect(::Gitlab::ExternalAuthorization).to receive(:access_allowed?).with(user, 'a-label') { false } - expect(issue.visible_to_user?(user)).to be_falsy - end - - it 'does not check the external service if a user does not have access to the project' do - project = build(:project, :private, - external_authorization_classification_label: 'a-label') - issue = build(:issue, project: project) - user = build(:user) - - expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?) - expect(issue.visible_to_user?(user)).to be_falsy - end - - it 'does not check the external webservice for admins' do - issue = build(:issue) - user = build(:admin) - - expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?) - - issue.visible_to_user?(user) - end - end - end - describe "#labels_hook_attrs" do let(:label) { create(:label) } let(:issue) { create(:labeled_issue, labels: [label]) } diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb index 89fcf3c10df..242a002bc23 100644 --- a/spec/policies/issue_policy_spec.rb +++ b/spec/policies/issue_policy_spec.rb @@ -103,12 +103,24 @@ describe IssuePolicy do expect(permissions(author, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end + it 'does not allow issue author to read or update confidential issue moved to an private project' do + confidential_issue.project = build(:project, :private) + + expect(permissions(author, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue) + end + it 'allows issue assignees to read and update their confidential issues' do expect(permissions(assignee, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) expect(permissions(assignee, confidential_issue)).to be_disallowed(:admin_issue) expect(permissions(assignee, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end + + it 'does not allow issue assignees to read or update confidential issue moved to an private project' do + confidential_issue.project = build(:project, :private) + + expect(permissions(assignee, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue) + end end end diff --git a/spec/requests/api/deploy_tokens_spec.rb b/spec/requests/api/deploy_tokens_spec.rb index fa20635056f..a885e80fd55 100644 --- a/spec/requests/api/deploy_tokens_spec.rb +++ b/spec/requests/api/deploy_tokens_spec.rb @@ -234,6 +234,25 @@ describe API::DeployTokens do expect(response).to match_response_schema('public_api/v4/deploy_token') end + context 'with no optional params given' do + let(:params) do + { + name: 'Foo', + scopes: [ + 'read_repository' + ] + } + end + + it 'creates the deploy token with default values' do + subject + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['username']).to match(/gitlab\+deploy-token-\d+/) + expect(json_response['expires_at']).to eq(nil) + end + end + context 'with an invalid scope' do before do params[:scopes] = %w[read_repository all_access] diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index ea60f783b48..30c1f99569b 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -642,6 +642,20 @@ describe API::Groups do expect(json_response['default_branch_protection']).to eq(::Gitlab::Access::MAINTAINER_PROJECT_ACCESS) end + context 'malicious group name' do + subject { put api("/groups/#{group1.id}", user1), params: { name: "<SCRIPT>alert('DOUBLE-ATTACK!')</SCRIPT>" } } + + it 'returns bad request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'does not update group name' do + expect { subject }.not_to change { group1.reload.name } + end + end + it 'returns 404 for a non existing group' do put api('/groups/1328', user1), params: { name: new_group_name } @@ -1083,6 +1097,20 @@ describe API::Groups do expect(json_response["parent_id"]).to eq(parent.id) end + context 'malicious group name' do + subject { post api("/groups", user3), params: group_params } + + let(:group_params) { attributes_for_group_api name: "<SCRIPT>alert('ATTACKED!')</SCRIPT>", path: "unique-url" } + + it 'returns bad request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it { expect { subject }.not_to change { Group.count } } + end + it "does not create group, duplicate" do post api("/groups", user3), params: { name: 'Duplicate Test', path: group2.path } diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 426e15faaa6..77501c3a136 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -3,15 +3,14 @@ require 'spec_helper' describe API::Internal::Base do - set(:user) { create(:user) } + let_it_be(:user, reload: true) { create(:user) } + let_it_be(:project, reload: true) { create(:project, :repository, :wiki_repo) } + let_it_be(:personal_snippet) { create(:personal_snippet, :repository, author: user) } + let_it_be(:project_snippet) { create(:project_snippet, :repository, author: user, project: project) } let(:key) { create(:key, user: user) } - set(:project) { create(:project, :repository, :wiki_repo) } let(:secret_token) { Gitlab::Shell.secret_token } let(:gl_repository) { "project-#{project.id}" } let(:reference_counter) { double('ReferenceCounter') } - - let_it_be(:personal_snippet) { create(:personal_snippet, :repository, author: user) } - let_it_be(:project_snippet) { create(:project_snippet, :repository, author: user, project: project) } let(:snippet_changes) { "#{TestEnv::BRANCH_SHA['snippet/single-file']} #{TestEnv::BRANCH_SHA['snippet/edit-file']} refs/heads/snippet/edit-file" } describe "GET /internal/check" do diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 8e2aed76913..1af5d553bf0 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -164,6 +164,30 @@ describe API::ProjectSnippets do end end + context 'with an external user' do + let(:user) { create(:user, :external) } + + context 'that belongs to the project' do + before do + project.add_developer(user) + end + + it 'creates a new snippet' do + post api("/projects/#{project.id}/snippets/", user), params: params + + expect(response).to have_gitlab_http_status(:created) + end + end + + context 'that does not belong to the project' do + it 'does not create a new snippet' do + post api("/projects/#{project.id}/snippets/", user), params: params + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + context 'with a regular user' do let(:user) { create(:user) } diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 97dc3899d3f..b503c923037 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -275,6 +275,18 @@ describe API::Repositories do expect(response).to have_gitlab_http_status(:too_many_requests) end + + context "when hotlinking detection is enabled" do + before do + Feature.enable(:repository_archive_hotlinking_interception) + end + + it_behaves_like "hotlink interceptor" do + let(:http_request) do + get api(route, current_user), headers: headers + end + end + end end context 'when unauthenticated', 'and project is public' do diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 865b0534cb0..caa9d9251d8 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -266,6 +266,16 @@ describe API::Snippets do it_behaves_like 'snippet creation' + context 'with an external user' do + let(:user) { create(:user, :external) } + + it 'does not create a new snippet' do + post api("/snippets/", user), params: params + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + it 'returns 400 for missing parameters' do params.delete(:title) diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index bcc1c6bc4d4..19b01cb7913 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -238,24 +238,44 @@ describe API::Triggers do end describe 'PUT /projects/:id/triggers/:trigger_id' do - context 'authenticated user with valid permissions' do - let(:new_description) { 'new description' } + context 'user is maintainer of the project' do + context 'the trigger belongs to user' do + let(:new_description) { 'new description' } - it 'updates description' do - put api("/projects/#{project.id}/triggers/#{trigger.id}", user), - params: { description: new_description } + it 'updates description' do + put api("/projects/#{project.id}/triggers/#{trigger.id}", user), + params: { description: new_description } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to include('description' => new_description) - expect(trigger.reload.description).to eq(new_description) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include('description' => new_description) + expect(trigger.reload.description).to eq(new_description) + end + end + + context 'the trigger does not belong to user' do + it 'does not update trigger' do + put api("/projects/#{project.id}/triggers/#{trigger2.id}", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end end end - context 'authenticated user with invalid permissions' do - it 'does not update trigger' do - put api("/projects/#{project.id}/triggers/#{trigger.id}", user2) + context 'user is developer of the project' do + context 'the trigger belongs to user' do + it 'does not update trigger' do + put api("/projects/#{project.id}/triggers/#{trigger2.id}", user2) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'the trigger does not belong to user' do + it 'does not update trigger' do + put api("/projects/#{project.id}/triggers/#{trigger.id}", user2) + + expect(response).to have_gitlab_http_status(:forbidden) + end end end diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index 754ab3e6a45..73dc9d8c63e 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -25,6 +25,17 @@ describe JwtController do end context 'when using authenticated request' do + shared_examples 'rejecting a blocked user' do + context 'with blocked user' do + let(:user) { create(:user, :blocked) } + + it 'rejects the request as unauthorized' do + expect(response).to have_gitlab_http_status(:unauthorized) + expect(response.body).to include('HTTP Basic: Access denied') + end + end + end + context 'using CI token' do let(:build) { create(:ci_build, :running) } let(:project) { build.project } @@ -61,6 +72,8 @@ describe JwtController do expect(response).to have_gitlab_http_status(:ok) expect(service_class).to have_received(:new).with(nil, user, ActionController::Parameters.new(parameters).permit!) end + + it_behaves_like 'rejecting a blocked user' end end @@ -72,6 +85,8 @@ describe JwtController do it { expect(service_class).to have_received(:new).with(nil, user, ActionController::Parameters.new(parameters).permit!) } + it_behaves_like 'rejecting a blocked user' + context 'when passing a flat array of scopes' do # We use this trick to make rails to generate a query_string: # scope=scope1&scope=scope2 diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index c71b803a7ab..f3fa5e36fec 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -6,9 +6,9 @@ describe 'Git LFS API and storage' do include ProjectForksHelper include WorkhorseHelpers - set(:project) { create(:project, :repository) } - set(:other_project) { create(:project, :repository) } - set(:user) { create(:user) } + let_it_be(:project, reload: true) { create(:project, :repository) } + let_it_be(:other_project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } let!(:lfs_object) { create(:lfs_object, :with_file) } let(:headers) do diff --git a/spec/serializers/merge_request_poll_widget_entity_spec.rb b/spec/serializers/merge_request_poll_widget_entity_spec.rb index 0593dd527cc..29d35fdc811 100644 --- a/spec/serializers/merge_request_poll_widget_entity_spec.rb +++ b/spec/serializers/merge_request_poll_widget_entity_spec.rb @@ -138,7 +138,7 @@ describe MergeRequestPollWidgetEntity do end describe 'pipeline' do - let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) } + let!(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) } before do allow_any_instance_of(MergeRequestPresenter).to receive(:can?).and_call_original @@ -158,6 +158,10 @@ describe MergeRequestPollWidgetEntity do expect(subject[:pipeline]).to eq(pipeline_payload) end + + it 'returns ci_status' do + expect(subject[:ci_status]).to eq('pending') + end end context 'when is not up to date' do @@ -171,10 +175,15 @@ describe MergeRequestPollWidgetEntity do context 'when user does not have access to pipelines' do let(:result) { false } + let(:req) { double('request', current_user: user, project: project) } it 'does not have pipeline' do expect(subject[:pipeline]).to eq(nil) end + + it 'does not return ci_status' do + expect(subject[:ci_status]).to eq(nil) + end end end end diff --git a/spec/services/metrics/dashboard/update_dashboard_service_spec.rb b/spec/services/metrics/dashboard/update_dashboard_service_spec.rb index 6ba4b4035e4..72d36959bce 100644 --- a/spec/services/metrics/dashboard/update_dashboard_service_spec.rb +++ b/spec/services/metrics/dashboard/update_dashboard_service_spec.rb @@ -5,9 +5,9 @@ require 'spec_helper' describe Metrics::Dashboard::UpdateDashboardService, :use_clean_rails_memory_store_caching do include MetricsDashboardHelpers - set(:user) { create(:user) } - set(:project) { create(:project, :repository) } - set(:environment) { create(:environment, project: project) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:environment) { create(:environment, project: project) } describe '#execute' do subject(:service_call) { described_class.new(project, user, params).execute } diff --git a/spec/support/helpers/prometheus_helpers.rb b/spec/support/helpers/prometheus_helpers.rb index 0fdc4de1b36..fdce00e7dec 100644 --- a/spec/support/helpers/prometheus_helpers.rb +++ b/spec/support/helpers/prometheus_helpers.rb @@ -68,6 +68,21 @@ module PrometheusHelpers }) end + def stub_prometheus_query_error(url, error_message = 'error', body: {}, headers: {}) + response = { + status: 'error', + errorType: 'bad_data', + error: error_message + }.merge(body) + + WebMock.stub_request(:get, url) + .to_return({ + status: 400, + headers: { 'Content-Type' => 'application/json' }.merge(headers), + body: response.to_json + }) + end + def stub_prometheus_request_with_exception(url, exception_type) WebMock.stub_request(:get, url).to_raise(exception_type) end diff --git a/spec/support/helpers/workhorse_helpers.rb b/spec/support/helpers/workhorse_helpers.rb index 27d5083728d..de232da3c8c 100644 --- a/spec/support/helpers/workhorse_helpers.rb +++ b/spec/support/helpers/workhorse_helpers.rb @@ -76,7 +76,7 @@ module WorkhorseHelpers "#{key}.size" => file.size }.tap do |params| params["#{key}.path"] = file.path if file.path - params["#{key}.remote_id"] = file.remote_id if file.respond_to?(:remote_id) && file.remote_id + params["#{key}.remote_id"] = file.remote_id if file.respond_to?(:remote_id) && file.remote_id.present? end end diff --git a/spec/support/shared_contexts/finders/issues_finder_shared_contexts.rb b/spec/support/shared_contexts/finders/issues_finder_shared_contexts.rb index 6b950a354cf..fc62ae5a13f 100644 --- a/spec/support/shared_contexts/finders/issues_finder_shared_contexts.rb +++ b/spec/support/shared_contexts/finders/issues_finder_shared_contexts.rb @@ -1,23 +1,23 @@ # frozen_string_literal: true RSpec.shared_context 'IssuesFinder context' do - set(:user) { create(:user) } - set(:user2) { create(:user) } - set(:group) { create(:group) } - set(:subgroup) { create(:group, parent: group) } - set(:project1) { create(:project, group: group) } - set(:project2) { create(:project) } - set(:project3) { create(:project, group: subgroup) } - set(:milestone) { create(:milestone, project: project1) } - set(:label) { create(:label, project: project2) } - set(:label2) { create(:label, project: project2) } - set(:issue1) { create(:issue, author: user, assignees: [user], project: project1, milestone: milestone, title: 'gitlab', created_at: 1.week.ago, updated_at: 1.week.ago) } - set(:issue2) { create(:issue, author: user, assignees: [user], project: project2, description: 'gitlab', created_at: 1.week.from_now, updated_at: 1.week.from_now) } - set(:issue3) { create(:issue, author: user2, assignees: [user2], project: project2, title: 'tanuki', description: 'tanuki', created_at: 2.weeks.from_now, updated_at: 2.weeks.from_now) } - set(:issue4) { create(:issue, project: project3) } - set(:award_emoji1) { create(:award_emoji, name: 'thumbsup', user: user, awardable: issue1) } - set(:award_emoji2) { create(:award_emoji, name: 'thumbsup', user: user2, awardable: issue2) } - set(:award_emoji3) { create(:award_emoji, name: 'thumbsdown', user: user, awardable: issue3) } + let_it_be(:user) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:project1, reload: true) { create(:project, group: group) } + let_it_be(:project2, reload: true) { create(:project) } + let_it_be(:project3, reload: true) { create(:project, group: subgroup) } + let_it_be(:milestone) { create(:milestone, project: project1) } + let_it_be(:label) { create(:label, project: project2) } + let_it_be(:label2) { create(:label, project: project2) } + let_it_be(:issue1, reload: true) { create(:issue, author: user, assignees: [user], project: project1, milestone: milestone, title: 'gitlab', created_at: 1.week.ago, updated_at: 1.week.ago) } + let_it_be(:issue2, reload: true) { create(:issue, author: user, assignees: [user], project: project2, description: 'gitlab', created_at: 1.week.from_now, updated_at: 1.week.from_now) } + let_it_be(:issue3, reload: true) { create(:issue, author: user2, assignees: [user2], project: project2, title: 'tanuki', description: 'tanuki', created_at: 2.weeks.from_now, updated_at: 2.weeks.from_now) } + let_it_be(:issue4, reload: true) { create(:issue, project: project3) } + let_it_be(:award_emoji1) { create(:award_emoji, name: 'thumbsup', user: user, awardable: issue1) } + let_it_be(:award_emoji2) { create(:award_emoji, name: 'thumbsup', user: user2, awardable: issue2) } + let_it_be(:award_emoji3) { create(:award_emoji, name: 'thumbsdown', user: user, awardable: issue3) } end RSpec.shared_context 'IssuesFinder#execute context' do diff --git a/spec/support/shared_contexts/finders/merge_requests_finder_shared_contexts.rb b/spec/support/shared_contexts/finders/merge_requests_finder_shared_contexts.rb index 82190fb7793..617701abf27 100644 --- a/spec/support/shared_contexts/finders/merge_requests_finder_shared_contexts.rb +++ b/spec/support/shared_contexts/finders/merge_requests_finder_shared_contexts.rb @@ -13,15 +13,14 @@ RSpec.shared_context 'MergeRequestsFinder multiple projects with merge requests end end - set(:user) { create(:user) } - set(:user2) { create(:user) } - - set(:group) { create(:group) } - set(:subgroup) { create(:group, parent: group) } - set(:project1) do + let_it_be(:user) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:project1, reload: true) do allow_gitaly_n_plus_1 { create(:project, :public, group: group) } end - # We cannot use `set` here otherwise we get: + # We cannot use `let_it_be` here otherwise we get: # Failure/Error: allow(RepositoryForkWorker).to receive(:perform_async).and_return(true) # The use of doubles or partial doubles from rspec-mocks outside of the per-test lifecycle is not supported. let(:project2) do @@ -36,13 +35,13 @@ RSpec.shared_context 'MergeRequestsFinder multiple projects with merge requests end end end - set(:project4) do + let_it_be(:project4, reload: true) do allow_gitaly_n_plus_1 { create(:project, :repository, group: subgroup) } end - set(:project5) do + let_it_be(:project5, reload: true) do allow_gitaly_n_plus_1 { create(:project, group: subgroup) } end - set(:project6) do + let_it_be(:project6, reload: true) do allow_gitaly_n_plus_1 { create(:project, group: subgroup) } end diff --git a/spec/support/shared_contexts/finders/users_finder_shared_contexts.rb b/spec/support/shared_contexts/finders/users_finder_shared_contexts.rb index a2fa3d7beac..fc8f9d2f407 100644 --- a/spec/support/shared_contexts/finders/users_finder_shared_contexts.rb +++ b/spec/support/shared_contexts/finders/users_finder_shared_contexts.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true RSpec.shared_context 'UsersFinder#execute filter by project context' do - set(:normal_user) { create(:user, username: 'johndoe') } - set(:blocked_user) { create(:user, :blocked, username: 'notsorandom') } - set(:external_user) { create(:user, :external) } - set(:omniauth_user) { create(:omniauth_user, provider: 'twitter', extern_uid: '123456') } + let_it_be(:normal_user) { create(:user, username: 'johndoe') } + let_it_be(:blocked_user) { create(:user, :blocked, username: 'notsorandom') } + let_it_be(:external_user) { create(:user, :external) } + let_it_be(:omniauth_user) { create(:omniauth_user, provider: 'twitter', extern_uid: '123456') } end diff --git a/spec/support/shared_contexts/mailers/notify_shared_context.rb b/spec/support/shared_contexts/mailers/notify_shared_context.rb index d5b44f8df2c..de8c0d5d2b4 100644 --- a/spec/support/shared_contexts/mailers/notify_shared_context.rb +++ b/spec/support/shared_contexts/mailers/notify_shared_context.rb @@ -1,11 +1,10 @@ # frozen_string_literal: true RSpec.shared_context 'gitlab email notification' do - set(:group) { create(:group) } - set(:subgroup) { create(:group, parent: group) } - set(:project) { create(:project, :repository, name: 'a-known-name', group: group) } - set(:recipient) { create(:user, email: 'recipient@example.com') } - + let_it_be(:group, reload: true) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:project, reload: true) { create(:project, :repository, name: 'a-known-name', group: group) } + let_it_be(:recipient, reload: true) { create(:user, email: 'recipient@example.com') } let(:gitlab_sender_display_name) { Gitlab.config.gitlab.email_display_name } let(:gitlab_sender) { Gitlab.config.gitlab.email_from } let(:gitlab_sender_reply_to) { Gitlab.config.gitlab.email_reply_to } diff --git a/spec/support/shared_contexts/policies/project_policy_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_shared_context.rb index 29a64e9b559..055164ec38e 100644 --- a/spec/support/shared_contexts/policies/project_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/project_policy_shared_context.rb @@ -1,12 +1,12 @@ # frozen_string_literal: true RSpec.shared_context 'ProjectPolicy context' do - set(:guest) { create(:user) } - set(:reporter) { create(:user) } - set(:developer) { create(:user) } - set(:maintainer) { create(:user) } - set(:owner) { create(:user) } - set(:admin) { create(:admin) } + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:developer) { create(:user) } + let_it_be(:maintainer) { create(:user) } + let_it_be(:owner) { create(:user) } + let_it_be(:admin) { create(:admin) } let(:project) { create(:project, :public, namespace: owner.namespace) } let(:base_guest_permissions) do diff --git a/spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb b/spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb new file mode 100644 index 00000000000..93a394387a3 --- /dev/null +++ b/spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +RSpec.shared_examples "hotlink interceptor" do + let(:http_request) { nil } + let(:headers) { nil } + + describe "DDOS prevention" do + using RSpec::Parameterized::TableSyntax + + context "hotlinked as media" do + where(:response_status, :accept_header) do + # These are default formats in modern browsers, and IE + :ok | "*/*" + :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" + :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8" + :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" + :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8" + :ok | "image/jpeg, application/x-ms-application, image/gif, application/xaml+xml, image/pjpeg, application/x-ms-xbap, application/x-shockwave-flash, application/msword, */*" + :ok | "text/html, application/xhtml+xml, image/jxr, */*" + :ok | "text/html, application/xml;q=0.9, application/xhtml+xml, image/png, image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1" + + # These are image request formats + :not_acceptable | "image/webp,*/*" + :not_acceptable | "image/png,image/*;q=0.8,*/*;q=0.5" + :not_acceptable | "image/webp,image/apng,image/*,*/*;q=0.8" + :not_acceptable | "image/png,image/svg+xml,image/*;q=0.8, */*;q=0.5" + + # Video request formats + :not_acceptable | "video/webm,video/ogg,video/*;q=0.9,application/ogg;q=0.7,audio/*;q=0.6,*/*;q=0.5" + + # Audio request formats + :not_acceptable | "audio/webm,audio/ogg,audio/wav,audio/*;q=0.9,application/ogg;q=0.7,video/*;q=0.6,*/*;q=0.5" + + # CSS request formats + :not_acceptable | "text/css,*/*;q=0.1" + :not_acceptable | "text/css" + :not_acceptable | "text/css,*/*;q=0.1" + end + + with_them do + let(:headers) do + { "Accept" => accept_header } + end + + before do + request.headers.merge!(headers) if request.present? + end + + it "renders the response" do + http_request + + expect(response).to have_gitlab_http_status(response_status) + end + end + end + + context "hotlinked as a script" do + where(:response_status, :fetch_mode) do + # Standard navigation fetch modes + :ok | "navigate" + :ok | "nested-navigate" + :ok | "same-origin" + + # Fetch modes when linking as JS + :not_acceptable | "cors" + :not_acceptable | "no-cors" + :not_acceptable | "websocket" + end + + with_them do + let(:headers) do + { "Sec-Fetch-Mode" => fetch_mode } + end + + before do + request.headers.merge!(headers) if request.present? + end + + it "renders the response" do + http_request + + expect(response).to have_gitlab_http_status(response_status) + end + end + end + end +end diff --git a/spec/support/shared_examples/finders/snippet_visibility_shared_examples.rb b/spec/support/shared_examples/finders/snippet_visibility_shared_examples.rb index 98ab141ab26..5bd2da03f3f 100644 --- a/spec/support/shared_examples/finders/snippet_visibility_shared_examples.rb +++ b/spec/support/shared_examples/finders/snippet_visibility_shared_examples.rb @@ -8,12 +8,12 @@ RSpec.shared_examples 'snippet visibility' do DatabaseCleaner.clean_with(:truncation) end - set(:author) { create(:user) } - set(:member) { create(:user) } - set(:external) { create(:user, :external) } - set(:non_member) { create(:user) } + let_it_be(:author) { create(:user) } + let_it_be(:member) { create(:user) } + let_it_be(:external) { create(:user, :external) } + let_it_be(:non_member) { create(:user) } - set(:project) do + let_it_be(:project, reload: true) do create(:project).tap do |project| project.add_developer(author) project.add_developer(member) diff --git a/spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb b/spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb index 4db52795cd4..b03da4471bc 100644 --- a/spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb +++ b/spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb @@ -224,7 +224,7 @@ RSpec.shared_examples 'issuable quick actions' do end context 'when user can update issuable' do - set(:developer) { create(:user) } + let_it_be(:developer) { create(:user) } let(:note_author) { developer } before do @@ -251,7 +251,7 @@ RSpec.shared_examples 'issuable quick actions' do end context 'when user cannot update issuable' do - set(:non_member) { create(:user) } + let_it_be(:non_member) { create(:user) } let(:note_author) { non_member } it 'applies commands that user can execute' do diff --git a/spec/support/shared_examples/requests/api/boards_shared_examples.rb b/spec/support/shared_examples/requests/api/boards_shared_examples.rb index 2bc79a2ef4d..20b0f4f0dd2 100644 --- a/spec/support/shared_examples/requests/api/boards_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/boards_shared_examples.rb @@ -165,7 +165,7 @@ RSpec.shared_examples 'group and project boards' do |route_definition, ee = fals end context "when the user is parent owner" do - set(:owner) { create(:user) } + let_it_be(:owner, reload: true) { create(:user) } before do if board_parent.try(:namespace) diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index 06116bd4737..e844b0ad799 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -714,6 +714,19 @@ describe ObjectStorage do end end + context 'when empty remote_id is specified' do + let(:uploaded_file) do + UploadedFile.new(temp_file.path, remote_id: '') + end + + it 'uses local storage' do + subject + + expect(uploader).to be_file_storage + expect(uploader.object_store).to eq(described_class::Store::LOCAL) + end + end + context 'when valid file is specified' do let(:uploaded_file) do UploadedFile.new(temp_file.path, filename: "my_file.txt", remote_id: "test/123123") |
