diff options
| author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-08-30 15:09:23 +0000 |
|---|---|---|
| committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-08-30 15:09:23 +0000 |
| commit | 33d3043f0f3b825f98c7ff2c794208a79bcafdb3 (patch) | |
| tree | 2fc8f97f12f9e3049ed3daad5700ae438b7eac9b | |
| parent | 99e4792893862d913d0bc9168da7d85775445590 (diff) | |
| parent | f1452cd5cf4c3e2dd6697bc25636b49c1aadecd1 (diff) | |
| download | gitlab-ce-15-1-stable.tar.gz | |
Merge remote-tracking branch 'dev/15-1-stable' into 15-1-stable15-1-stable
76 files changed, 1198 insertions, 322 deletions
diff --git a/.rubocop_todo/gitlab/namespaced_class.yml b/.rubocop_todo/gitlab/namespaced_class.yml index 23ff86ba289..b89b8559921 100644 --- a/.rubocop_todo/gitlab/namespaced_class.yml +++ b/.rubocop_todo/gitlab/namespaced_class.yml @@ -726,6 +726,7 @@ Gitlab/NamespacedClass: - 'app/validators/top_level_group_validator.rb' - 'app/validators/untrusted_regexp_validator.rb' - 'app/validators/x509_certificate_credentials_validator.rb' + - 'app/validators/bytesize_validator.rb' - 'app/workers/admin_email_worker.rb' - 'app/workers/approve_blocked_pending_approval_users_worker.rb' - 'app/workers/archive_trace_worker.rb' diff --git a/CHANGELOG.md b/CHANGELOG.md index 94cc84a7526..42ceaa5ebd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,28 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 15.1.6 (2022-08-30) + +### Security (17 changes) + +- [No overriding methods for Sawyer class](gitlab-org/security/gitlab@720a17d03791c298d193b2d49d322a5f259bb6f2) ([merge request](gitlab-org/security/gitlab!2756)) +- [Bump yajl-ruby gem version](gitlab-org/security/gitlab@acb8bee73354ddbd7a7a52e3d09c870d1cd99e27) ([merge request](gitlab-org/security/gitlab!2690)) +- [Prevent long loops when generating suggested branch name](gitlab-org/security/gitlab@e331ecf658de25901def2ea4a368104b82a0109c) ([merge request](gitlab-org/security/gitlab!2745)) +- [IDOR in Zentao integration issue show page](gitlab-org/security/gitlab@0a238baf6a1d4aa0bc834448aefaf756d594a7be) ([merge request](gitlab-org/security/gitlab!2742)) +- [Patch VULNDB-255039 (potential Rack cache poisoning)](gitlab-org/security/gitlab@1f5ecd95b3631c8352ff57cf4bee23d26aa51ecc) ([merge request](gitlab-org/security/gitlab!2695)) +- [HTML escape the label background color](gitlab-org/security/gitlab@470b75a53ea4383ea30de5a482d39b322f87dfa2) ([merge request](gitlab-org/security/gitlab!2721)) +- [Sandbox jupyter notebook HTML output](gitlab-org/security/gitlab@72089898a60de7f17c19a2fa9d4f1330d3052b52) ([merge request](gitlab-org/security/gitlab!2713)) +- [Fix unauthorized GFM references in Incident Timeline](gitlab-org/security/gitlab@c62408682ed47bc2e5f93585a5b4e92e8cfebf9f) ([merge request](gitlab-org/security/gitlab!2709)) +- [Optimize handling repositories with huge trees](gitlab-org/security/gitlab@396f20e019a9888d1645e9345a82fdf21153bf76) ([merge request](gitlab-org/security/gitlab!2667)) +- [Parse commit trailers without using regexp](gitlab-org/security/gitlab@b377a1ecbb37c5359b2c2a0ecfbd911654664700) ([merge request](gitlab-org/security/gitlab!2701)) +- [Check for pathological markdown input](gitlab-org/security/gitlab@e3a1376ec70d8d60f11a380cce6e0b3c35f68646) ([merge request](gitlab-org/security/gitlab!2731)) +- [Replaced smooshpack to fix the vulnerability in LivePreview](gitlab-org/security/gitlab@d520ffd2a5a75d33ac98c39cd2f2fe623b0e1115) ([merge request](gitlab-org/security/gitlab!2664)) +- [Update package auth for group IP allowlist](gitlab-org/security/gitlab@12bb8656bdaa9a7502c0a1b77c12fefb72677ba1) ([merge request](gitlab-org/security/gitlab!2685)) +- [Don't show pipeline status](gitlab-org/security/gitlab@7fb43f899f2342704bda81643f8375a126efc2ae) ([merge request](gitlab-org/security/gitlab!2679)) +- [Sanitize img attributes in Banzai::Filter::ImageLinkFilter](gitlab-org/security/gitlab@594fa5874fb7cc6b6588bbf8aff2f04b8acbbfd0) ([merge request](gitlab-org/security/gitlab!2677)) +- [Validate description length for snippets](gitlab-org/security/gitlab@94ae3d05741bc69b9307e5f58f0d61bf2566c21b) ([merge request](gitlab-org/security/gitlab!2704)) +- [Prevent brute force vuln for Git over HTTP(S) requests](gitlab-org/security/gitlab@7b76542e197ea72289c881c312b3a519c8b28e63) ([merge request](gitlab-org/security/gitlab!2718)) + ## 15.1.5 (2022-08-22) ### Security (2 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 086830eb8db..9bc1af644c7 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -15.1.5
\ No newline at end of file +15.1.6
\ No newline at end of file @@ -537,7 +537,7 @@ gem 'json', '~> 2.5.1' gem 'json_schemer', '~> 0.2.18' gem 'oj', '~> 3.10.6' gem 'multi_json', '~> 1.14.1' -gem 'yajl-ruby', '~> 1.4.1', require: 'yajl' +gem 'yajl-ruby', '~> 1.4.3', require: 'yajl' gem 'webauthn', '~> 2.3' diff --git a/Gemfile.lock b/Gemfile.lock index dab51a0803c..78902daa204 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1439,7 +1439,7 @@ GEM xml-simple (1.1.5) xpath (3.2.0) nokogiri (~> 1.8) - yajl-ruby (1.4.1) + yajl-ruby (1.4.3) yard (0.9.26) zeitwerk (2.6.0) @@ -1735,7 +1735,7 @@ DEPENDENCIES webmock (~> 3.9.1) webrick (~> 1.6.1) wikicloth (= 0.8.1) - yajl-ruby (~> 1.4.1) + yajl-ruby (~> 1.4.3) BUNDLED WITH 2.3.15 @@ -1 +1 @@ -15.1.5
\ No newline at end of file +15.1.6
\ No newline at end of file diff --git a/app/assets/javascripts/ide/components/preview/clientside.vue b/app/assets/javascripts/ide/components/preview/clientside.vue index b1f6f2c87b9..70b881b6ff6 100644 --- a/app/assets/javascripts/ide/components/preview/clientside.vue +++ b/app/assets/javascripts/ide/components/preview/clientside.vue @@ -2,7 +2,7 @@ import { GlLoadingIcon } from '@gitlab/ui'; import { listen } from 'codesandbox-api'; import { isEmpty, debounce } from 'lodash'; -import { Manager } from 'smooshpack'; +import { SandpackClient } from '@codesandbox/sandpack-client'; import { mapActions, mapGetters, mapState } from 'vuex'; import { packageJsonPath, @@ -21,7 +21,7 @@ export default { }, data() { return { - manager: {}, + client: {}, loading: false, sandpackReady: false, }; @@ -94,11 +94,11 @@ export default { this.sandpackReady = false; eventHub.$off('ide.files.change', this.onFilesChangeCallback); - if (!isEmpty(this.manager)) { - this.manager.listener(); + if (!isEmpty(this.client)) { + this.client.cleanup(); } - this.manager = {}; + this.client = {}; if (this.listener) { this.listener(); @@ -120,7 +120,7 @@ export default { return this.loadFileContent(this.mainEntry) .then(() => this.$nextTick()) .then(() => { - this.initManager(); + this.initClient(); this.listener = listen((e) => { switch (e.type) { @@ -136,15 +136,15 @@ export default { update() { if (!this.sandpackReady) return; - if (isEmpty(this.manager)) { + if (isEmpty(this.client)) { this.initPreview(); return; } - this.manager.updatePreview(this.sandboxOpts); + this.client.updatePreview(this.sandboxOpts); }, - initManager() { + initClient() { const { codesandboxBundlerUrl: bundlerURL } = this; const settings = { @@ -155,7 +155,7 @@ export default { ...(bundlerURL ? { bundlerURL } : {}), }; - this.manager = new Manager('#ide-preview', this.sandboxOpts, settings); + this.client = new SandpackClient('#ide-preview', this.sandboxOpts, settings); }, }, }; @@ -164,7 +164,7 @@ export default { <template> <div class="preview h-100 w-100 d-flex flex-column gl-bg-white"> <template v-if="showPreview"> - <navigator :manager="manager" /> + <navigator :client="client" /> <div id="ide-preview"></div> </template> <div diff --git a/app/assets/javascripts/ide/components/preview/navigator.vue b/app/assets/javascripts/ide/components/preview/navigator.vue index 96f9a85c23f..852de16d508 100644 --- a/app/assets/javascripts/ide/components/preview/navigator.vue +++ b/app/assets/javascripts/ide/components/preview/navigator.vue @@ -8,7 +8,7 @@ export default { GlLoadingIcon, }, props: { - manager: { + client: { type: Object, required: true, }, @@ -51,7 +51,7 @@ export default { onUrlChange(e) { const lastPath = this.path; - this.path = e.url.replace(this.manager.bundlerURL, '') || '/'; + this.path = e.url.replace(this.client.bundlerURL, '') || '/'; if (lastPath !== this.path) { this.currentBrowsingIndex = @@ -79,7 +79,7 @@ export default { }, visitPath(path) { // eslint-disable-next-line vue/no-mutating-props - this.manager.iframe.src = `${this.manager.bundlerURL}${path}`; + this.client.iframe.src = `${this.client.bundlerURL}${path}`; }, }, }; diff --git a/app/assets/javascripts/notebook/cells/output/html.vue b/app/assets/javascripts/notebook/cells/output/html.vue index 2d1d8845e41..fdcea300388 100644 --- a/app/assets/javascripts/notebook/cells/output/html.vue +++ b/app/assets/javascripts/notebook/cells/output/html.vue @@ -40,6 +40,13 @@ export default { <template> <div class="output"> <prompt type="Out" :count="count" :show-output="showOutput" /> - <div v-safe-html:[$options.safeHtmlConfig]="rawCode" class="gl-overflow-auto"></div> + <iframe + sandbox + :srcdoc="rawCode" + frameborder="0" + scrolling="no" + width="100%" + class="gl-overflow-auto" + ></iframe> </div> </template> diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb index 8eebf9fbf6b..9e0ca28a5ea 100644 --- a/app/controllers/jwt_controller.rb +++ b/app/controllers/jwt_controller.rb @@ -36,31 +36,26 @@ class JwtController < ApplicationController @authentication_result = Gitlab::Auth.find_for_git_client(login, password, project: nil, ip: request.ip) if @authentication_result.failed? - render_unauthorized + render_access_denied end end rescue Gitlab::Auth::MissingPersonalAccessTokenError - render_missing_personal_access_token + render_access_denied end - def render_missing_personal_access_token - render json: { - errors: [ - { code: 'UNAUTHORIZED', - message: _('HTTP Basic: Access denied\n' \ - 'You must use a personal access token with \'api\' scope for Git over HTTP.\n' \ - 'You can generate one at %{profile_personal_access_tokens_url}') % { profile_personal_access_tokens_url: profile_personal_access_tokens_url } } - ] - }, status: :unauthorized - end - - def render_unauthorized - render json: { - errors: [ - { code: 'UNAUTHORIZED', - message: 'HTTP Basic: Access denied' } - ] - }, status: :unauthorized + def render_access_denied + help_page = help_page_url( + 'user/profile/account/two_factor_authentication', + anchor: 'troubleshooting' + ) + + render( + json: { errors: [{ + code: 'UNAUTHORIZED', + message: format(_("HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See %{help_page_url}"), help_page_url: help_page) + }] }, + status: :unauthorized + ) end def auth_params diff --git a/app/controllers/repositories/git_http_client_controller.rb b/app/controllers/repositories/git_http_client_controller.rb index 24520a187e3..fbf5d82a45b 100644 --- a/app/controllers/repositories/git_http_client_controller.rb +++ b/app/controllers/repositories/git_http_client_controller.rb @@ -67,9 +67,21 @@ module Repositories end send_challenges - render plain: "HTTP Basic: Access denied\n", status: :unauthorized + render_access_denied rescue Gitlab::Auth::MissingPersonalAccessTokenError - render_missing_personal_access_token + render_access_denied + end + + def render_access_denied + help_page = help_page_url( + 'topics/git/troubleshooting_git', + anchor: 'error-on-git-fetch-http-basic-access-denied' + ) + + render( + plain: format(_("HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See %{help_page_url}"), help_page_url: help_page), + status: :unauthorized + ) end def basic_auth_provided? @@ -103,13 +115,6 @@ module Repositories @container, @project, @repo_type, @redirected_path = Gitlab::RepoPath.parse(repository_path) end - def render_missing_personal_access_token - render plain: "HTTP Basic: Access denied\n" \ - "You must use a personal access token with 'read_repository' or 'write_repository' scope for Git over HTTP.\n" \ - "You can generate one at #{profile_personal_access_tokens_url}", - status: :unauthorized - end - def repository strong_memoize(:repository) do repo_type.repository_for(container) diff --git a/app/graphql/types/incident_management/timeline_event_type.rb b/app/graphql/types/incident_management/timeline_event_type.rb index a6d3f57404b..690facc8732 100644 --- a/app/graphql/types/incident_management/timeline_event_type.rb +++ b/app/graphql/types/incident_management/timeline_event_type.rb @@ -33,11 +33,6 @@ module Types null: true, description: 'Text note of the timeline event.' - field :note_html, - GraphQL::Types::String, - null: true, - description: 'HTML note of the timeline event.' - field :promoted_from_note, Types::Notes::NoteType, null: true, @@ -67,6 +62,8 @@ module Types Types::TimeType, null: false, description: 'Timestamp when the event updated.' + + markdown_field :note_html, null: true, description: 'HTML note of the timeline event.' end end end diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 3c3179f6fbe..2e90842d09f 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -166,7 +166,7 @@ module CommitsHelper ref, { merge_request: merge_request&.cache_key, - pipeline_status: commit.status_for(ref)&.cache_key, + pipeline_status: commit.detailed_status_for(ref)&.cache_key, xhr: request.xhr?, controller: controller.controller_path, path: @path # referred to in #link_to_browse_code diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index 877785c9eaf..54d0b45b794 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -247,7 +247,7 @@ module LabelsHelper class="#{css_class}" data-container="body" data-html="true" - #{"style=\"background-color: #{bg_color}\"" if bg_color} + #{"style=\"background-color: #{h bg_color}\"" if bg_color} >#{ERB::Util.html_escape_once(name)}#{suffix}</span> HTML end diff --git a/app/models/integrations/zentao.rb b/app/models/integrations/zentao.rb index 53194089296..459756c865b 100644 --- a/app/models/integrations/zentao.rb +++ b/app/models/integrations/zentao.rb @@ -69,6 +69,10 @@ module Integrations } end + def client_url + api_url.presence || url + end + def self.to_param name.demodulize.downcase end diff --git a/app/models/issue.rb b/app/models/issue.rb index 47aa2b24feb..1eefb74260a 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -400,7 +400,13 @@ class Issue < ApplicationRecord return to_branch_name unless project.repository.branch_exists?(to_branch_name) start_counting_from = 2 - Uniquify.new(start_counting_from).string(-> (counter) { "#{to_branch_name}-#{counter}" }) do |suggested_branch_name| + + branch_name_generator = -> (counter) do + suffix = counter > 5 ? SecureRandom.hex(8) : counter + "#{to_branch_name}-#{suffix}" + end + + Uniquify.new(start_counting_from).string(branch_name_generator) do |suggested_branch_name| project.repository.branch_exists?(suggested_branch_name) end end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 2001577c88b..d7eb1860831 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -21,6 +21,8 @@ class Snippet < ApplicationRecord MAX_FILE_COUNT = 10 + DESCRIPTION_LENGTH_MAX = 1.megabyte + cache_markdown_field :title, pipeline: :single_line cache_markdown_field :description cache_markdown_field :content @@ -56,19 +58,10 @@ class Snippet < ApplicationRecord validates :title, presence: true, length: { maximum: 255 } validates :file_name, length: { maximum: 255 } + validates :description, bytesize: { maximum: -> { DESCRIPTION_LENGTH_MAX } }, if: :description_changed? validates :content, presence: true - validates :content, - length: { - maximum: ->(_) { Gitlab::CurrentSettings.snippet_size_limit }, - message: -> (_, data) do - current_value = ActiveSupport::NumberHelper.number_to_human_size(data[:value].size) - max_size = ActiveSupport::NumberHelper.number_to_human_size(Gitlab::CurrentSettings.snippet_size_limit) - - _("is too long (%{current_value}). The maximum size is %{max_size}.") % { current_value: current_value, max_size: max_size } - end - }, - if: :content_changed? + validates :content, bytesize: { maximum: -> { Gitlab::CurrentSettings.snippet_size_limit } }, if: :content_changed? after_create :create_statistics diff --git a/app/presenters/commit_presenter.rb b/app/presenters/commit_presenter.rb index 7df45ca03bb..2cb88179845 100644 --- a/app/presenters/commit_presenter.rb +++ b/app/presenters/commit_presenter.rb @@ -5,12 +5,20 @@ class CommitPresenter < Gitlab::View::Presenter::Delegated presents ::Commit, as: :commit - def status_for(ref) + def detailed_status_for(ref) + return unless can?(current_user, :read_pipeline, commit.latest_pipeline(ref)) return unless can?(current_user, :read_commit_status, commit.project) commit.latest_pipeline(ref)&.detailed_status(current_user) end + def status_for(ref = nil) + return unless can?(current_user, :read_pipeline, commit.latest_pipeline(ref)) + return unless can?(current_user, :read_commit_status, commit.project) + + commit.status(ref) + end + def any_pipelines? return false unless can?(current_user, :read_pipeline, commit.project) diff --git a/app/validators/bytesize_validator.rb b/app/validators/bytesize_validator.rb new file mode 100644 index 00000000000..adbdd81d5c4 --- /dev/null +++ b/app/validators/bytesize_validator.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +# BytesizeValidator +# +# Custom validator for verifying that bytesize of a field doesn't exceed the specified limit. +# It is different from Rails length validator because it takes .bytesize into account instead of .size/.length +# +# Example: +# +# class Snippet < ActiveRecord::Base +# validates :content, bytesize: { maximum: -> { Gitlab::CurrentSettings.snippet_size_limit } } +# end +# +# Configuration options: +# * <tt>maximum</tt> - Proc that evaluates the bytesize limit that cannot be exceeded +class BytesizeValidator < ActiveModel::EachValidator + def validate_each(record, attr, value) + size = value.to_s.bytesize + max_size = options[:maximum].call + + return if size <= max_size + + error_message = format(_('is too long (%{size}). The maximum size is %{max_size}.'), { + size: ActiveSupport::NumberHelper.number_to_human_size(size), + max_size: ActiveSupport::NumberHelper.number_to_human_size(max_size) + }) + + record.errors.add(attr, error_message) + end +end diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 4442f62b221..3232580b999 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -14,7 +14,7 @@ - project = local_assigns.fetch(:project) { merge_request&.project } - ref = local_assigns.fetch(:ref) { merge_request&.source_branch } - commit = commit.present(current_user: current_user) -- commit_status = commit.status_for(ref) +- commit_status = commit.detailed_status_for(ref) - collapsible = local_assigns.fetch(:collapsible, true) - link_data_attrs = local_assigns.fetch(:link_data_attrs, {}) - link = commit_path(project, commit, merge_request: merge_request) diff --git a/config/initializers/rack_VULNDB-255039_patch.rb b/config/initializers/rack_VULNDB-255039_patch.rb new file mode 100644 index 00000000000..b613ed9bdb1 --- /dev/null +++ b/config/initializers/rack_VULNDB-255039_patch.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +if Gem.loaded_specs['rack'].version >= Gem::Version.new("3.0.0") + raise <<~ERR + This patch is unnecessary in Rack versions 3.0.0 or newer. + Please remove this file and the associated spec. + + See https://github.com/rack/rack/blob/main/CHANGELOG.md#security (issue #1733) + ERR +end + +# Patches a cache poisoning attack vector in Rack by not allowing semicolons +# to delimit query parameters. +# See https://github.com/rack/rack/issues/1732. +# +# Solution is taken from the same issue. +# +# The actual patch is due for release in Rack 3.0.0. +module Rack + class Request + Helpers.module_eval do + # rubocop: disable Naming/MethodName + def GET + if get_header(RACK_REQUEST_QUERY_STRING) == query_string + get_header(RACK_REQUEST_QUERY_HASH) + else + query_hash = parse_query(query_string, '&') # only allow ampersand here + set_header(RACK_REQUEST_QUERY_STRING, query_string) + set_header(RACK_REQUEST_QUERY_HASH, query_hash) + end + end + # rubocop: enable Naming/MethodName + end + end +end diff --git a/config/initializers/sawyer_patch.rb b/config/initializers/sawyer_patch.rb new file mode 100644 index 00000000000..08d249645cc --- /dev/null +++ b/config/initializers/sawyer_patch.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true +# +# This patch updates SawyerResource class to not allow Ruby methods to be overridden and accessed. +# Any attempt to access a Ruby method will result in an exception. +module SawyerClassPatch + def attr_accessor(*attrs) + attrs.each do |attribute| + class_eval do + # rubocop:disable Gitlab/ModuleWithInstanceVariables + if method_defined?(attribute) || method_defined?("#{attribute}=") || method_defined?("#{attribute}?") + define_method attribute do + raise Sawyer::Error, + "Sawyer method \"#{attribute}\" overlaps Ruby method. Convert to a hash to access the attribute." + end + + define_method "#{attribute}=" do |value| + raise Sawyer::Error, + "Sawyer method \"#{attribute}\" overlaps Ruby method. Convert to a hash to access the attribute." + end + + define_method "#{attribute}?" do + raise Sawyer::Error, + "Sawyer method \"#{attribute}\" overlaps Ruby method. Convert to a hash to access the attribute." + end + else + define_method attribute do + @attrs[attribute.to_sym] + end + + define_method "#{attribute}=" do |value| + @attrs[attribute.to_sym] = value + end + + define_method "#{attribute}?" do + !!@attrs[attribute.to_sym] + end + end + end + # rubocop:enable Gitlab/ModuleWithInstanceVariables + end + end +end + +Sawyer::Resource.singleton_class.prepend(SawyerClassPatch) diff --git a/doc/topics/git/troubleshooting_git.md b/doc/topics/git/troubleshooting_git.md index 13962ad0376..1fe2dcc4d92 100644 --- a/doc/topics/git/troubleshooting_git.md +++ b/doc/topics/git/troubleshooting_git.md @@ -267,3 +267,8 @@ To resolve this issue, you can update the password expiration by either: ``` The bug was reported [in this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/332455). + +## Error on Git fetch: "HTTP Basic: Access Denied" + +If you receive an `HTTP Basic: Access denied` error when using Git over HTTP(S), +refer to the [two-factor authentication troubleshooting guide](../../user/profile/account/two_factor_authentication.md#troubleshooting). diff --git a/doc/user/packages/dependency_proxy/index.md b/doc/user/packages/dependency_proxy/index.md index af54d928bec..006dd618ce5 100644 --- a/doc/user/packages/dependency_proxy/index.md +++ b/doc/user/packages/dependency_proxy/index.md @@ -299,6 +299,10 @@ hub_docker_quota_check: ## Troubleshooting +## Authentication error: "HTTP Basic: Access Denied" + +If you receive an `HTTP Basic: Access denied` error when authenticating against the Dependency Proxy, refer to the [two-factor authentication troubleshooting guide](../../profile/account/two_factor_authentication.md#troubleshooting). + ### Dependency Proxy Connection Failure If a service alias is not set the `docker:19.03.12` image is unable to find the diff --git a/doc/user/packages/pypi_repository/index.md b/doc/user/packages/pypi_repository/index.md index eee6d55a3ce..17008016acc 100644 --- a/doc/user/packages/pypi_repository/index.md +++ b/doc/user/packages/pypi_repository/index.md @@ -336,6 +336,11 @@ when a PyPI package is not found in the Package Registry, the request is forward Administrators can disable this behavior in the [Continuous Integration settings](../../admin_area/settings/continuous_integration.md). +WARNING: +When you use the `--index-url` option, do not specify the port if it is a default +port, such as `80` for a URL starting with `http` or `443` for a URL starting +with `https`. + ### Install from the project level To install the latest version of a package, use the following command: diff --git a/doc/user/profile/account/two_factor_authentication.md b/doc/user/profile/account/two_factor_authentication.md index 2dbeaae2267..26c86010f24 100644 --- a/doc/user/profile/account/two_factor_authentication.md +++ b/doc/user/profile/account/two_factor_authentication.md @@ -431,6 +431,39 @@ a GitLab global administrator disable 2FA for your account: ## Troubleshooting +### Error: "HTTP Basic: Access denied. The provided password or token ..." + +When making a request, you can receive the following error: + +```plaintext +HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal +access token instead of a password. +``` + +This error occurs in the following scenarios: + +- You have 2FA enabled and have attempted to authenticate with a username and + password. For 2FA-enabled users, a [personal access token](../personal_access_tokens.md) (PAT) + must be used instead of a password. To authenticate: + - Git requests over HTTP(S), a PAT with `read_repository` or `write_repository` scope is required. + - [GitLab Container Registry](../../packages/container_registry/index.md#authenticate-with-the-container-registry) requests, a PAT + with `read_registry` or `write_registry` scope is required. + - [Dependency Proxy](../../packages/dependency_proxy/index.md#authenticate-with-the-dependency-proxy) requests, a PAT with + `read_registry` and `write_registry` scopes is required. +- You do not have 2FA enabled and have sent an incorrect username or password + with your request. +- You do not have 2FA enabled but an administrator has enabled the + [enforce 2FA for all users](../../../security/two_factor_authentication.md#enforce-2fa-for-all-users) setting. +- You do not have 2FA enabled, but an administrator has disabled the + [password authentication enabled for Git over HTTP(S)](../../admin_area/settings/sign_in_restrictions.md#password-authentication-enabled) + setting. If LDAP is: + - Configured, an [LDAP password](../../../administration/auth/ldap/index.md) + or a [personal access token](../personal_access_tokens.md) + must be used to authenticate Git requests over HTTP(S). + - Not configured, you must use a [personal access token](../personal_access_tokens.md). + +### Error: "invalid pin code" + If you receive an `invalid pin code` error, this can indicate that there is a time sync issue between the authentication application and the GitLab instance itself. To avoid the time sync issue, enable time synchronization in the device that generates the codes. For example: diff --git a/lib/api/commits.rb b/lib/api/commits.rb index dedda82091f..71594025688 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -142,7 +142,7 @@ module API Gitlab::UsageDataCounters::EditorUniqueCounter.track_web_ide_edit_action(author: current_user) end - present commit_detail, with: Entities::CommitDetail, stats: params[:stats] + present commit_detail, with: Entities::CommitDetail, include_stats: params[:stats], current_user: current_user else render_api_error!(result[:message], 400) end @@ -161,7 +161,7 @@ module API not_found! 'Commit' unless commit - present commit, with: Entities::CommitDetail, stats: params[:stats], current_user: current_user + present commit, with: Entities::CommitDetail, include_stats: params[:stats], current_user: current_user end desc 'Get the diff for a specific commit of a project' do diff --git a/lib/api/entities/commit.rb b/lib/api/entities/commit.rb index fd23c23b980..6cd180cd584 100644 --- a/lib/api/entities/commit.rb +++ b/lib/api/entities/commit.rb @@ -12,7 +12,9 @@ module API expose :trailers expose :web_url do |commit, _options| - Gitlab::UrlBuilder.build(commit) + c = commit + c = c.__subject__ if c.is_a?(Gitlab::View::Presenter::Base) + Gitlab::UrlBuilder.build(c) end end end diff --git a/lib/api/entities/commit_detail.rb b/lib/api/entities/commit_detail.rb index 61238102e9d..cc529639359 100644 --- a/lib/api/entities/commit_detail.rb +++ b/lib/api/entities/commit_detail.rb @@ -3,8 +3,10 @@ module API module Entities class CommitDetail < Commit - expose :stats, using: Entities::CommitStats, if: :stats - expose :status + include ::API::Helpers::Presentable + + expose :stats, using: Entities::CommitStats, if: :include_stats + expose :status_for, as: :status expose :project_id expose :last_pipeline do |commit, options| diff --git a/lib/api/helpers/packages/basic_auth_helpers.rb b/lib/api/helpers/packages/basic_auth_helpers.rb index 6c381d85cd8..ebedb3b7563 100644 --- a/lib/api/helpers/packages/basic_auth_helpers.rb +++ b/lib/api/helpers/packages/basic_auth_helpers.rb @@ -14,28 +14,12 @@ module API include Constants include Gitlab::Utils::StrongMemoize - def unauthorized_user_project - @unauthorized_user_project ||= find_project(params[:id]) - end - - def unauthorized_user_project! - unauthorized_user_project || not_found! - end - - def unauthorized_user_group - @unauthorized_user_group ||= find_group(params[:id]) - end - - def unauthorized_user_group! - unauthorized_user_group || not_found! - end - def authorized_user_project @authorized_user_project ||= authorized_project_find! end def authorized_project_find! - project = unauthorized_user_project + project = find_project(params[:id]) unless project && can?(current_user, :read_project, project) return unauthorized_or! { not_found! } diff --git a/lib/api/pypi_packages.rb b/lib/api/pypi_packages.rb index 5bf3c3b8aac..b6ed4103d2b 100644 --- a/lib/api/pypi_packages.rb +++ b/lib/api/pypi_packages.rb @@ -84,6 +84,16 @@ module API body content end + + def ensure_group! + find_group(params[:id]) || not_found! + find_authorized_group! + end + + def ensure_project! + find_project(params[:id]) || not_found! + authorized_user_project + end end params do @@ -91,7 +101,7 @@ module API end resource :groups, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do after_validation do - unauthorized_user_group! + ensure_group! end namespace ':id/-/packages/pypi' do @@ -101,7 +111,8 @@ module API route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth get 'files/:sha256/*file_identifier' do - group = unauthorized_user_group! + group = find_authorized_group! + authorize_read_package!(group) filename = "#{params[:file_identifier]}.#{params[:format]}" package = Packages::Pypi::PackageFinder.new(current_user, group, { filename: filename, sha256: params[:sha256] }).execute @@ -146,7 +157,7 @@ module API resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do before do - unauthorized_user_project! + ensure_project! end namespace ':id/packages/pypi' do @@ -160,7 +171,8 @@ module API route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth get 'files/:sha256/*file_identifier' do - project = unauthorized_user_project! + project = authorized_user_project + authorize_read_package!(project) filename = "#{params[:file_identifier]}.#{params[:format]}" package = Packages::Pypi::PackageFinder.new(current_user, project, { filename: filename, sha256: params[:sha256] }).execute diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 2e21f591667..023c993d1d6 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -183,7 +183,7 @@ module API compare = CompareService.new(user_project, params[:to]).execute(target_project, params[:from], straight: params[:straight]) if compare - present compare, with: Entities::Compare + present compare, with: Entities::Compare, current_user: current_user else not_found!("Ref") end diff --git a/lib/api/search.rb b/lib/api/search.rb index fd4d46cf77d..d8789d8839e 100644 --- a/lib/api/search.rb +++ b/lib/api/search.rb @@ -102,7 +102,7 @@ module API get do verify_search_scope!(resource: nil) - present search, with: entity + present search, with: entity, current_user: current_user end end @@ -124,7 +124,7 @@ module API get ':id/(-/)search' do verify_search_scope!(resource: user_group) - present search(group_id: user_group.id), with: entity + present search(group_id: user_group.id), with: entity, current_user: current_user end end @@ -145,7 +145,7 @@ module API use :pagination end get ':id/(-/)search' do - present search({ project_id: user_project.id, repository_ref: params[:ref] }), with: entity + present search({ project_id: user_project.id, repository_ref: params[:ref] }), with: entity, current_user: current_user end end end diff --git a/lib/api/submodules.rb b/lib/api/submodules.rb index 5c71a18c6d0..2b51ab91c40 100644 --- a/lib/api/submodules.rb +++ b/lib/api/submodules.rb @@ -39,7 +39,7 @@ module API if result[:status] == :success commit_detail = user_project.repository.commit(result[:result]) - present commit_detail, with: Entities::CommitDetail + present commit_detail, with: Entities::CommitDetail, current_user: current_user else render_api_error!(result[:message], result[:http_status] || 400) end diff --git a/lib/banzai/filter/commit_trailers_filter.rb b/lib/banzai/filter/commit_trailers_filter.rb index a615abc1989..817bea42757 100644 --- a/lib/banzai/filter/commit_trailers_filter.rb +++ b/lib/banzai/filter/commit_trailers_filter.rb @@ -17,21 +17,10 @@ module Banzai include ActionView::Helpers::TagHelper include AvatarsHelper - TRAILER_REGEXP = /(?<label>[[:alpha:]-]+-by:)/i.freeze - AUTHOR_REGEXP = /(?<author_name>.+)/.freeze - # Devise.email_regexp wouldn't work here since its designed to match - # against strings that only contains email addresses; the \A and \z - # around the expression will only match if the string being matched - # contains just the email nothing else. - MAIL_REGEXP = /<(?<author_email>[^@\s]+@[^@\s]+)>/.freeze - FILTER_REGEXP = /(?<trailer>^\s*#{TRAILER_REGEXP}\s*#{AUTHOR_REGEXP}\s+#{MAIL_REGEXP}$)/mi.freeze - def call doc.xpath('descendant-or-self::text()').each do |node| content = node.to_html - next unless content.match(FILTER_REGEXP) - html = trailer_filter(content) next if html == content @@ -52,11 +41,24 @@ module Banzai # Returns a String with all trailer lines replaced with links to GitLab # users and mailto links to non GitLab users. All links have `data-trailer` # and `data-user` attributes attached. + # + # The code intentionally avoids using Regex for security and performance + # reasons: https://gitlab.com/gitlab-org/gitlab/-/issues/363734 def trailer_filter(text) - text.gsub(FILTER_REGEXP) do |author_match| - label = $~[:label] - "#{label} #{parse_user($~[:author_name], $~[:author_email], label)}" - end + text.lines.map! do |line| + trailer, rest = line.split(':', 2) + + next line unless trailer.downcase.end_with?('-by') && rest.present? + + chunks = rest.split + author_email = chunks.pop.delete_prefix('<').delete_suffix('>') + next line unless Devise.email_regexp.match(author_email) + + author_name = chunks.join(' ').strip + trailer = "#{trailer.strip}:" + + "#{trailer} #{link_to_user_or_email(author_name, author_email, trailer)}\n" + end.join end # Find a GitLab user using the supplied email and generate @@ -67,7 +69,7 @@ module Banzai # trailer - String trailer used in the commit message # # Returns a String with a link to the user. - def parse_user(name, email, trailer) + def link_to_user_or_email(name, email, trailer) link_to_user User.find_by_any_email(email), name: name, email: email, diff --git a/lib/banzai/filter/image_link_filter.rb b/lib/banzai/filter/image_link_filter.rb index 60881b5f511..262c0b5340d 100644 --- a/lib/banzai/filter/image_link_filter.rb +++ b/lib/banzai/filter/image_link_filter.rb @@ -34,17 +34,20 @@ module Banzai img.remove_attribute('data-diagram-src') end - link.children = if link_replaces_image - img['alt'] || img['data-src'] || img['src'] - else - img.clone - end + link.children = link_replaces_image ? link_children(img) : img.clone img.replace(link) end doc end + + private + + def link_children(img) + [img['alt'], img['data-src'], img['src']] + .map { |f| Sanitize.fragment(f).presence }.compact.first || '' + end end end end diff --git a/lib/banzai/filter/pathological_markdown_filter.rb b/lib/banzai/filter/pathological_markdown_filter.rb new file mode 100644 index 00000000000..0f94150c7a1 --- /dev/null +++ b/lib/banzai/filter/pathological_markdown_filter.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Banzai + module Filter + class PathologicalMarkdownFilter < HTML::Pipeline::TextFilter + # It's not necessary for this to be precise - we just need to detect + # when there are a non-trivial number of unclosed image links. + # So we don't really care about code blocks, etc. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/370428 + REGEX = /!\[(?:[^\]])+?!\[/.freeze + DETECTION_MAX = 10 + + def call + count = 0 + + @text.scan(REGEX) do |_match| + count += 1 + break if count > DETECTION_MAX + end + + return @text if count <= DETECTION_MAX + + "_Unable to render markdown - too many unclosed markdown image links detected._" + end + end + end +end diff --git a/lib/banzai/pipeline/incident_management/timeline_event_pipeline.rb b/lib/banzai/pipeline/incident_management/timeline_event_pipeline.rb index 01ee3f5d9e8..eef2b2674dd 100644 --- a/lib/banzai/pipeline/incident_management/timeline_event_pipeline.rb +++ b/lib/banzai/pipeline/incident_management/timeline_event_pipeline.rb @@ -11,9 +11,9 @@ module Banzai def self.filters @filters ||= FilterArray[ *super, + Filter::SanitizationFilter, *Banzai::Pipeline::GfmPipeline.reference_filters, Filter::EmojiFilter, - Filter::SanitizationFilter, Filter::ExternalLinkFilter, Filter::ImageLinkFilter ] diff --git a/lib/banzai/pipeline/plain_markdown_pipeline.rb b/lib/banzai/pipeline/plain_markdown_pipeline.rb index 1da0f72996b..fb6f6e9077d 100644 --- a/lib/banzai/pipeline/plain_markdown_pipeline.rb +++ b/lib/banzai/pipeline/plain_markdown_pipeline.rb @@ -5,6 +5,7 @@ module Banzai class PlainMarkdownPipeline < BasePipeline def self.filters FilterArray[ + Filter::PathologicalMarkdownFilter, Filter::MarkdownPreEscapeFilter, Filter::MarkdownFilter, Filter::MarkdownPostEscapeFilter diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 9fb34f74c82..529d4354a1b 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -5,6 +5,8 @@ module Gitlab class CommitService include Gitlab::EncodingHelper + TREE_ENTRIES_DEFAULT_LIMIT = 1000 + def initialize(repository) @gitaly_repo = repository.gitaly_repository @repository = repository @@ -112,6 +114,9 @@ module Gitlab end def tree_entries(repository, revision, path, recursive, pagination_params) + pagination_params ||= {} + pagination_params[:limit] ||= TREE_ENTRIES_DEFAULT_LIMIT + request = Gitaly::GetTreeEntriesRequest.new( repository: @gitaly_repo, revision: encode_binary(revision), diff --git a/lib/gitlab/markdown_cache.rb b/lib/gitlab/markdown_cache.rb index 09ba95666de..f426f70800c 100644 --- a/lib/gitlab/markdown_cache.rb +++ b/lib/gitlab/markdown_cache.rb @@ -11,7 +11,7 @@ module Gitlab # this if the change to the renderer output is a new feature or a # minor bug fix. # See: https://gitlab.com/gitlab-org/gitlab/-/issues/330313 - CACHE_COMMONMARK_VERSION = 31 + CACHE_COMMONMARK_VERSION = 32 CACHE_COMMONMARK_VERSION_START = 10 BaseError = Class.new(StandardError) diff --git a/lib/gitlab/set_cache.rb b/lib/gitlab/set_cache.rb index feb2c3c1d7d..896e7e3f65e 100644 --- a/lib/gitlab/set_cache.rb +++ b/lib/gitlab/set_cache.rb @@ -68,6 +68,10 @@ module Gitlab with { |redis| redis.ttl(cache_key(key)) } end + def count(key) + with { |redis| redis.scard(cache_key(key)) } + end + private def with(&blk) diff --git a/lib/gitlab/zentao/client.rb b/lib/gitlab/zentao/client.rb index 4da4631eecf..0de4ca3a09b 100644 --- a/lib/gitlab/zentao/client.rb +++ b/lib/gitlab/zentao/client.rb @@ -5,6 +5,10 @@ module Gitlab class Client Error = Class.new(StandardError) ConfigError = Class.new(Error) + RequestError = Class.new(Error) + + CACHE_MAX_SET_SIZE = 5_000 + CACHE_TTL = 1.month.freeze attr_reader :integration @@ -29,11 +33,21 @@ module Gitlab end def fetch_issues(params = {}) - get("products/#{zentao_product_xid}/issues", params) + get("products/#{zentao_product_xid}/issues", params).tap do |response| + mark_issues_as_seen_in_product(response['issues']) + end end def fetch_issue(issue_id) - raise Gitlab::Zentao::Client::Error, 'invalid issue id' unless issue_id_pattern.match(issue_id) + raise Error, 'invalid issue id' unless issue_id_pattern.match(issue_id) + + # Only return issues that are associated with the product configured in + # the integration. Due to a lack of available data in the ZenTao APIs, we + # can only determine if an issue belongs to a product if the issue was + # previously returned in the `#fetch_issues` call. + # + # See https://gitlab.com/gitlab-org/gitlab/-/issues/360372#note_1016963713 + raise RequestError unless issue_seen_in_product?(issue_id) get("issues/#{issue_id}") end @@ -48,17 +62,15 @@ module Gitlab options = { headers: headers, query: params } response = Gitlab::HTTP.get(url(path), options) - raise Gitlab::Zentao::Client::Error, 'request error' unless response.success? + raise RequestError unless response.success? Gitlab::Json.parse(response.body) rescue JSON::ParserError - raise Gitlab::Zentao::Client::Error, 'invalid response format' + raise Error, 'invalid response format' end def url(path) - host = integration.api_url.presence || integration.url - - URI.parse(Gitlab::Utils.append_path(host, "api.php/v1/#{path}")) + URI.parse(Gitlab::Utils.append_path(integration.client_url, "api.php/v1/#{path}")) end def headers @@ -71,6 +83,30 @@ module Gitlab def zentao_product_xid integration.zentao_product_xid end + + def issue_ids_cache_key + @issue_ids_cache_key ||= [ + :zentao_product_issues, + OpenSSL::Digest::SHA256.hexdigest(integration.client_url), + zentao_product_xid + ].join(':') + end + + def issue_ids_cache + @issue_ids_cache ||= ::Gitlab::SetCache.new(expires_in: CACHE_TTL) + end + + def mark_issues_as_seen_in_product(issues) + return unless issues && issue_ids_cache.count(issue_ids_cache_key) < CACHE_MAX_SET_SIZE + + ids = issues.map { _1['id'] } + + issue_ids_cache.write(issue_ids_cache_key, ids) + end + + def issue_seen_in_product?(id) + issue_ids_cache.include?(issue_ids_cache_key, id) + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 566ec3633fc..ab945f06438 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -18624,7 +18624,7 @@ msgstr "" msgid "HTTP Archive (HAR)" msgstr "" -msgid "HTTP Basic: Access denied\\nYou must use a personal access token with 'api' scope for Git over HTTP.\\nYou can generate one at %{profile_personal_access_tokens_url}" +msgid "HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See %{help_page_url}" msgstr "" msgid "Harbor Registry" @@ -45398,6 +45398,9 @@ msgstr "" msgid "is too long (%{current_value}). The maximum size is %{max_size}." msgstr "" +msgid "is too long (%{size}). The maximum size is %{max_size}." +msgstr "" + msgid "is too long (maximum is %{count} characters)" msgstr "" diff --git a/package.json b/package.json index f1589d5d37f..35ff46a45e1 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,7 @@ "@apollo/client": "^3.5.10", "@babel/core": "^7.18.5", "@babel/preset-env": "^7.18.2", + "@codesandbox/sandpack-client": "^1.2.2", "@gitlab/at.js": "1.5.7", "@gitlab/favicon-overlay": "2.0.0", "@gitlab/svgs": "2.21.0", @@ -166,7 +167,6 @@ "remark-rehype": "^10.1.0", "scrollparent": "^2.0.1", "select2": "3.5.2-browserify", - "smooshpack": "^0.0.62", "sortablejs": "^1.10.2", "string-hash": "1.1.3", "style-loader": "^2.0.0", diff --git a/qa/qa/fixtures/package_managers/pypi/pypi_upload_install_package.yaml.erb b/qa/qa/fixtures/package_managers/pypi/pypi_upload_install_package.yaml.erb index 3ea71152801..87c01fb5477 100644 --- a/qa/qa/fixtures/package_managers/pypi/pypi_upload_install_package.yaml.erb +++ b/qa/qa/fixtures/package_managers/pypi/pypi_upload_install_package.yaml.erb @@ -16,4 +16,4 @@ install: script: - "pip install <%= package.name %> --no-deps --index-url <%= uri.scheme %>://<%= personal_access_token %>:<%= personal_access_token %>@<%= gitlab_host_with_port %>/api/v4/projects/${CI_PROJECT_ID}/packages/pypi/simple --trusted-host <%= gitlab_host_with_port %>" tags: - - runner-for-<%= project.name %>
\ No newline at end of file + - runner-for-<%= project.name %> diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/pypi_repository_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/pypi_repository_spec.rb index 4614eced300..22d76d684e5 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/pypi_repository_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/pypi_repository_spec.rb @@ -30,9 +30,16 @@ module QA end let(:uri) { URI.parse(Runtime::Scenario.gitlab_address) } - let(:gitlab_address_with_port) { "#{uri.scheme}://#{uri.host}:#{uri.port}" } - let(:gitlab_host_with_port) { "#{uri.host}:#{uri.port}" } let(:personal_access_token) { use_ci_variable(name: 'PERSONAL_ACCESS_TOKEN', value: Runtime::Env.personal_access_token, project: project) } + let(:gitlab_address_with_port) { "#{uri.scheme}://#{uri.host}:#{uri.port}" } + let(:gitlab_host_with_port) do + # Don't specify port if it is a standard one + if uri.port == 80 || uri.port == 443 + uri.host + else + "#{uri.host}:#{uri.port}" + end + end before do Flow::Login.sign_in diff --git a/spec/frontend/ide/components/preview/clientside_spec.js b/spec/frontend/ide/components/preview/clientside_spec.js index 426fbd5c04c..8e8cf4ab929 100644 --- a/spec/frontend/ide/components/preview/clientside_spec.js +++ b/spec/frontend/ide/components/preview/clientside_spec.js @@ -2,15 +2,15 @@ import { GlLoadingIcon } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import Vue, { nextTick } from 'vue'; import { dispatch } from 'codesandbox-api'; -import smooshpack from 'smooshpack'; +import { SandpackClient } from '@codesandbox/sandpack-client'; import Vuex from 'vuex'; import waitForPromises from 'helpers/wait_for_promises'; import Clientside from '~/ide/components/preview/clientside.vue'; import { PING_USAGE_PREVIEW_KEY, PING_USAGE_PREVIEW_SUCCESS_KEY } from '~/ide/constants'; import eventHub from '~/ide/eventhub'; -jest.mock('smooshpack', () => ({ - Manager: jest.fn(), +jest.mock('@codesandbox/sandpack-client', () => ({ + SandpackClient: jest.fn(), })); Vue.use(Vuex); @@ -78,8 +78,8 @@ describe('IDE clientside preview', () => { // eslint-disable-next-line no-restricted-syntax wrapper.setData({ sandpackReady: true, - manager: { - listener: jest.fn(), + client: { + cleanup: jest.fn(), updatePreview: jest.fn(), }, }); @@ -90,9 +90,9 @@ describe('IDE clientside preview', () => { }); describe('without main entry', () => { - it('creates sandpack manager', () => { + it('creates sandpack client', () => { createComponent(); - expect(smooshpack.Manager).not.toHaveBeenCalled(); + expect(SandpackClient).not.toHaveBeenCalled(); }); }); describe('with main entry', () => { @@ -102,8 +102,8 @@ describe('IDE clientside preview', () => { return waitForPromises(); }); - it('creates sandpack manager', () => { - expect(smooshpack.Manager).toHaveBeenCalledWith( + it('creates sandpack client', () => { + expect(SandpackClient).toHaveBeenCalledWith( '#ide-preview', expectedSandpackOptions(), expectedSandpackSettings(), @@ -141,8 +141,8 @@ describe('IDE clientside preview', () => { return waitForPromises(); }); - it('creates sandpack manager with bundlerURL', () => { - expect(smooshpack.Manager).toHaveBeenCalledWith('#ide-preview', expectedSandpackOptions(), { + it('creates sandpack client with bundlerURL', () => { + expect(SandpackClient).toHaveBeenCalledWith('#ide-preview', expectedSandpackOptions(), { ...expectedSandpackSettings(), bundlerURL: TEST_BUNDLER_URL, }); @@ -156,8 +156,8 @@ describe('IDE clientside preview', () => { return waitForPromises(); }); - it('creates sandpack manager', () => { - expect(smooshpack.Manager).toHaveBeenCalledWith( + it('creates sandpack client', () => { + expect(SandpackClient).toHaveBeenCalledWith( '#ide-preview', { files: {}, @@ -332,7 +332,7 @@ describe('IDE clientside preview', () => { }); describe('update', () => { - it('initializes manager if manager is empty', () => { + it('initializes client if client is empty', () => { createComponent({ getters: { packageJson: dummyPackageJson } }); // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details // eslint-disable-next-line no-restricted-syntax @@ -340,7 +340,7 @@ describe('IDE clientside preview', () => { wrapper.vm.update(); return waitForPromises().then(() => { - expect(smooshpack.Manager).toHaveBeenCalled(); + expect(SandpackClient).toHaveBeenCalled(); }); }); @@ -349,7 +349,7 @@ describe('IDE clientside preview', () => { wrapper.vm.update(); - expect(wrapper.vm.manager.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts); + expect(wrapper.vm.client.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts); }); }); @@ -361,7 +361,7 @@ describe('IDE clientside preview', () => { }); it('calls updatePreview', () => { - expect(wrapper.vm.manager.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts); + expect(wrapper.vm.client.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts); }); }); }); @@ -405,7 +405,7 @@ describe('IDE clientside preview', () => { beforeEach(() => { createInitializedComponent(); - spy = wrapper.vm.manager.updatePreview; + spy = wrapper.vm.client.updatePreview; wrapper.destroy(); }); diff --git a/spec/frontend/ide/components/preview/navigator_spec.js b/spec/frontend/ide/components/preview/navigator_spec.js index a199f4704f7..bccfb75835f 100644 --- a/spec/frontend/ide/components/preview/navigator_spec.js +++ b/spec/frontend/ide/components/preview/navigator_spec.js @@ -11,7 +11,7 @@ jest.mock('codesandbox-api', () => ({ describe('IDE clientside preview navigator', () => { let wrapper; - let manager; + let client; let listenHandler; const findBackButton = () => wrapper.findAll('button').at(0); @@ -20,9 +20,9 @@ describe('IDE clientside preview navigator', () => { beforeEach(() => { listen.mockClear(); - manager = { bundlerURL: TEST_HOST, iframe: { src: '' } }; + client = { bundlerURL: TEST_HOST, iframe: { src: '' } }; - wrapper = shallowMount(ClientsideNavigator, { propsData: { manager } }); + wrapper = shallowMount(ClientsideNavigator, { propsData: { client } }); [[listenHandler]] = listen.mock.calls; }); @@ -31,7 +31,7 @@ describe('IDE clientside preview navigator', () => { }); it('renders readonly URL bar', async () => { - listenHandler({ type: 'urlchange', url: manager.bundlerURL }); + listenHandler({ type: 'urlchange', url: client.bundlerURL }); await nextTick(); expect(wrapper.find('input[readonly]').element.value).toBe('/'); }); @@ -89,13 +89,13 @@ describe('IDE clientside preview navigator', () => { expect(findBackButton().attributes('disabled')).toBe('disabled'); }); - it('updates manager iframe src', async () => { + it('updates client iframe src', async () => { listenHandler({ type: 'urlchange', url: `${TEST_HOST}/url1` }); listenHandler({ type: 'urlchange', url: `${TEST_HOST}/url2` }); await nextTick(); findBackButton().trigger('click'); - expect(manager.iframe.src).toBe(`${TEST_HOST}/url1`); + expect(client.iframe.src).toBe(`${TEST_HOST}/url1`); }); }); @@ -133,13 +133,13 @@ describe('IDE clientside preview navigator', () => { expect(findForwardButton().attributes('disabled')).toBe('disabled'); }); - it('updates manager iframe src', async () => { + it('updates client iframe src', async () => { listenHandler({ type: 'urlchange', url: `${TEST_HOST}/url1` }); listenHandler({ type: 'urlchange', url: `${TEST_HOST}/url2` }); await nextTick(); findBackButton().trigger('click'); - expect(manager.iframe.src).toBe(`${TEST_HOST}/url1`); + expect(client.iframe.src).toBe(`${TEST_HOST}/url1`); }); }); @@ -152,10 +152,10 @@ describe('IDE clientside preview navigator', () => { }); it('calls refresh with current path', () => { - manager.iframe.src = 'something-other'; + client.iframe.src = 'something-other'; findRefreshButton().trigger('click'); - expect(manager.iframe.src).toBe(url); + expect(client.iframe.src).toBe(url); }); }); }); diff --git a/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js b/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js index 70c7f56b62f..296d01ddd99 100644 --- a/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js +++ b/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js @@ -38,7 +38,7 @@ export default [ '</tr>\n', '</table>', ].join(''), - output: '<table>', + output: '<table data-myattr="XSS">', }, ], // Note: style is sanitized out @@ -98,7 +98,7 @@ export default [ '</svg>', ].join(), output: - '<svg xmlns="http://www.w3.org/2000/svg" width="388.84pt" version="1.0" id="svg2" height="115.02pt">', + '<svg height="115.02pt" id="svg2" version="1.0" width="388.84pt" xmlns="http://www.w3.org/2000/svg">', }, ], ]; diff --git a/spec/frontend/notebook/cells/output/index_spec.js b/spec/frontend/notebook/cells/output/index_spec.js index 8e04e4c146c..5c36bcaef2a 100644 --- a/spec/frontend/notebook/cells/output/index_spec.js +++ b/spec/frontend/notebook/cells/output/index_spec.js @@ -52,16 +52,16 @@ describe('Output component', () => { const htmlType = json.cells[4]; createComponent(htmlType.outputs[0]); - expect(vm.$el.querySelector('p')).not.toBeNull(); - expect(vm.$el.querySelectorAll('p')).toHaveLength(1); - expect(vm.$el.textContent.trim()).toContain('test'); + const iframe = vm.$el.querySelector('iframe'); + expect(iframe).not.toBeNull(); + expect(iframe.srcdoc).toBe('<p>test</p>'); }); it('renders multiple raw HTML outputs', () => { const htmlType = json.cells[4]; createComponent([htmlType.outputs[0], htmlType.outputs[0]]); - expect(vm.$el.querySelectorAll('p')).toHaveLength(2); + expect(vm.$el.querySelectorAll('iframe')).toHaveLength(2); }); }); @@ -90,7 +90,10 @@ describe('Output component', () => { }); it('renders as an svg', () => { - expect(vm.$el.querySelector('svg')).not.toBeNull(); + const iframe = vm.$el.querySelector('iframe'); + + expect(iframe).not.toBeNull(); + expect(iframe.srcdoc).toBe('<svg></svg>'); }); }); diff --git a/spec/helpers/commits_helper_spec.rb b/spec/helpers/commits_helper_spec.rb index 961e7688202..ab183a3ed19 100644 --- a/spec/helpers/commits_helper_spec.rb +++ b/spec/helpers/commits_helper_spec.rb @@ -312,7 +312,7 @@ RSpec.describe CommitsHelper do let(:current_path) { "test" } before do - expect(commit).to receive(:status_for).with(ref).and_return(commit_status) + expect(commit).to receive(:detailed_status_for).with(ref).and_return(commit_status) assign(:path, current_path) end diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index 5efa88a2a7d..90366d7772c 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -112,6 +112,14 @@ RSpec.describe LabelsHelper do end end + describe 'render_label_text' do + it 'html escapes the bg_color correctly' do + xss_payload = '"><img src=x onerror=prompt(1)>' + label_text = render_label_text('xss', bg_color: xss_payload) + expect(label_text).to include(html_escape(xss_payload)) + end + end + describe 'text_color_for_bg' do it 'uses light text on dark backgrounds' do expect(text_color_for_bg('#222E2E')).to be_color('#FFFFFF') diff --git a/spec/initializers/rack_VULNDB-255039_patch_spec.rb b/spec/initializers/rack_VULNDB-255039_patch_spec.rb new file mode 100644 index 00000000000..754ff2f10e0 --- /dev/null +++ b/spec/initializers/rack_VULNDB-255039_patch_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Rack VULNDB-255039' do + context 'when handling query params in GET requests' do + it 'does not treat semicolons as query delimiters' do + env = ::Rack::MockRequest.env_for('http://gitlab.com?a=b;c=1') + + query_hash = ::Rack::Request.new(env).GET + + # Prior to this patch, this was splitting around the semicolon, which + # would return {"a"=>"b", "c"=>"1"} + expect(query_hash).to eq({ "a" => "b;c=1" }) + end + end +end diff --git a/spec/initializers/sawyer_patch_spec.rb b/spec/initializers/sawyer_patch_spec.rb new file mode 100644 index 00000000000..dc922654d7d --- /dev/null +++ b/spec/initializers/sawyer_patch_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true +require 'fast_spec_helper' +require 'sawyer' + +require_relative '../../config/initializers/sawyer_patch' + +RSpec.describe 'sawyer_patch' do + it 'raises error when acessing a method that overlaps a Ruby method' do + sawyer_resource = Sawyer::Resource.new( + Sawyer::Agent.new(''), + { + to_s: 'Overriding method', + user: { to_s: 'Overriding method', name: 'User name' } + } + ) + + error_message = 'Sawyer method "to_s" overlaps Ruby method. Convert to a hash to access the attribute.' + expect { sawyer_resource.to_s }.to raise_error(Sawyer::Error, error_message) + expect { sawyer_resource.to_s? }.to raise_error(Sawyer::Error, error_message) + expect { sawyer_resource.to_s = 'new value' }.to raise_error(Sawyer::Error, error_message) + expect { sawyer_resource.user.to_s }.to raise_error(Sawyer::Error, error_message) + expect(sawyer_resource.user.name).to eq('User name') + end + + it 'raises error when acessing a boolean method that overlaps a Ruby method' do + sawyer_resource = Sawyer::Resource.new( + Sawyer::Agent.new(''), + { + nil?: 'value' + } + ) + + expect { sawyer_resource.nil? }.to raise_error(Sawyer::Error) + end + + it 'raises error when acessing a method that expects an argument' do + sawyer_resource = Sawyer::Resource.new( + Sawyer::Agent.new(''), + { + 'user': 'value', + 'user=': 'value', + '==': 'value', + '!=': 'value', + '+': 'value' + } + ) + + expect(sawyer_resource.user).to eq('value') + expect { sawyer_resource.user = 'New user' }.to raise_error(ArgumentError) + expect { sawyer_resource == true }.to raise_error(ArgumentError) + expect { sawyer_resource != true }.to raise_error(ArgumentError) + expect { sawyer_resource + 1 }.to raise_error(ArgumentError) + end + + it 'does not raise error if is not an overlapping method' do + sawyer_resource = Sawyer::Resource.new( + Sawyer::Agent.new(''), + { + count_total: 1, + user: { name: 'User name' } + } + ) + + expect(sawyer_resource.count_total).to eq(1) + expect(sawyer_resource.count_total?).to eq(true) + expect(sawyer_resource.count_total + 1).to eq(2) + expect(sawyer_resource.user.name).to eq('User name') + end +end diff --git a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb index f7cb6b92b48..44738e5e771 100644 --- a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb @@ -18,10 +18,20 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter do context 'detects' do let(:email) { FFaker::Internet.email } - it 'trailers in the form of *-by and replace users with links' do - doc = filter(commit_message_html) + context 'trailers in the form of *-by' do + where(:commit_trailer) do + ["#{FFaker::Lorem.word}-by:", "#{FFaker::Lorem.word}-BY:", "#{FFaker::Lorem.word}-By:"] + end - expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) + with_them do + let(:trailer) { commit_trailer } + + it 'replaces users with links' do + doc = filter(commit_message_html) + + expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) + end + end end it 'trailers prefixed with whitespaces' do @@ -121,7 +131,14 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter do context "ignores" do it 'commit messages without trailers' do - exp = message = commit_html(FFaker::Lorem.sentence) + exp = message = commit_html(Array.new(5) { FFaker::Lorem.sentence }.join("\n")) + doc = filter(message) + + expect(doc.to_html).to match Regexp.escape(exp) + end + + it 'trailers without emails' do + exp = message = commit_html(Array.new(5) { 'Merged-By:' }.join("\n")) doc = filter(message) expect(doc.to_html).to match Regexp.escape(exp) diff --git a/spec/lib/banzai/filter/image_link_filter_spec.rb b/spec/lib/banzai/filter/image_link_filter_spec.rb index 6326d894b08..78d68697ac7 100644 --- a/spec/lib/banzai/filter/image_link_filter_spec.rb +++ b/spec/lib/banzai/filter/image_link_filter_spec.rb @@ -92,5 +92,50 @@ RSpec.describe Banzai::Filter::ImageLinkFilter do expect(doc.at_css('a')['class']).to match(%r{with-attachment-icon}) end + + context 'when link attributes contain malicious code' do + let(:malicious_code) do + # rubocop:disable Layout/LineLength + %q(<a class='fixed-top fixed-bottom' data-create-path=/malicious-url><style> .tab-content>.tab-pane{display: block !important}</style>) + # rubocop:enable Layout/LineLength + end + + context 'when image alt contains malicious code' do + it 'ignores image alt and uses image path as the link text', :aggregate_failures do + doc = filter(image(path, alt: malicious_code), context) + + expect(doc.to_html).to match(%r{^<a[^>]*>#{path}</a>$}) + expect(doc.at_css('a')['href']).to eq(path) + end + end + + context 'when image src contains malicious code' do + it 'ignores image src and does not use it as the link text' do + doc = filter(image(malicious_code), context) + + expect(doc.to_html).to match(%r{^<a[^>]*></a>$}) + end + + it 'keeps image src unchanged, malicious code does not execute as part of url' do + doc = filter(image(malicious_code), context) + + expect(doc.at_css('a')['href']).to eq(malicious_code) + end + end + + context 'when image data-src contains malicious code' do + it 'ignores data-src and uses image path as the link text', :aggregate_failures do + doc = filter(image(path, data_src: malicious_code), context) + + expect(doc.to_html).to match(%r{^<a[^>]*>#{path}</a>$}) + end + + it 'uses image data-src, malicious code does not execute as part of url' do + doc = filter(image(path, data_src: malicious_code), context) + + expect(doc.at_css('a')['href']).to eq(malicious_code) + end + end + end end end diff --git a/spec/lib/banzai/filter/pathological_markdown_filter_spec.rb b/spec/lib/banzai/filter/pathological_markdown_filter_spec.rb new file mode 100644 index 00000000000..e0a07d1ea77 --- /dev/null +++ b/spec/lib/banzai/filter/pathological_markdown_filter_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Banzai::Filter::PathologicalMarkdownFilter do + include FilterSpecHelper + + let_it_be(:short_text) { '![a' * 5 } + let_it_be(:long_text) { ([short_text] * 10).join(' ') } + let_it_be(:with_images_text) { " #{'and\n' * 200} " } + + it 'detects a significat number of unclosed image links' do + msg = <<~TEXT + _Unable to render markdown - too many unclosed markdown image links detected._ + TEXT + + expect(filter(long_text)).to eq(msg.strip) + end + + it 'does nothing when there are only a few unclosed image links' do + expect(filter(short_text)).to eq(short_text) + end + + it 'does nothing when there are only a few unclosed image links and images' do + expect(filter(with_images_text)).to eq(with_images_text) + end +end diff --git a/spec/lib/banzai/pipeline/full_pipeline_spec.rb b/spec/lib/banzai/pipeline/full_pipeline_spec.rb index 376edfb99fc..c07f99dc9fc 100644 --- a/spec/lib/banzai/pipeline/full_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/full_pipeline_spec.rb @@ -167,4 +167,16 @@ RSpec.describe Banzai::Pipeline::FullPipeline do expect(output).to include('<em>@test_</em>') end end + + describe 'unclosed image links' do + it 'detects a significat number of unclosed image links' do + markdown = '![a ' * 30 + msg = <<~TEXT + Unable to render markdown - too many unclosed markdown image links detected. + TEXT + output = described_class.to_html(markdown, project: nil) + + expect(output).to include(msg.strip) + end + end end diff --git a/spec/lib/banzai/pipeline/incident_management/timeline_event_pipeline_spec.rb b/spec/lib/banzai/pipeline/incident_management/timeline_event_pipeline_spec.rb index 09d2919c6c4..76f2dd4b659 100644 --- a/spec/lib/banzai/pipeline/incident_management/timeline_event_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/incident_management/timeline_event_pipeline_spec.rb @@ -10,9 +10,9 @@ RSpec.describe Banzai::Pipeline::IncidentManagement::TimelineEventPipeline do expect(described_class.filters).to eq( [ *Banzai::Pipeline::PlainMarkdownPipeline.filters, + Banzai::Filter::SanitizationFilter, *Banzai::Pipeline::GfmPipeline.reference_filters, Banzai::Filter::EmojiFilter, - Banzai::Filter::SanitizationFilter, Banzai::Filter::ExternalLinkFilter, Banzai::Filter::ImageLinkFilter ] @@ -62,7 +62,32 @@ RSpec.describe Banzai::Pipeline::IncidentManagement::TimelineEventPipeline do context 'when markdown contains emojis' do let(:markdown) { ':+1:π' } - it { is_expected.to eq('<p>ππ</p>') } + it 'renders emojis wrapped in <gl-emoji> tag' do + # rubocop:disable Layout/LineLength + is_expected.to eq( + %q(<p><gl-emoji title="thumbs up sign" data-name="thumbsup" data-unicode-version="6.0">π</gl-emoji><gl-emoji title="thumbs up sign" data-name="thumbsup" data-unicode-version="6.0">π</gl-emoji></p>) + ) + # rubocop:enable Layout/LineLength + end + end + + context 'when markdown contains labels' do + let(:label) { create(:label, project: project, title: 'backend') } + let(:markdown) { %Q(~"#{label.name}" ~unknown) } + + it 'replaces existing label to a link' do + # rubocop:disable Layout/LineLength + is_expected.to match( + %r(<p>.+<a href=\"[\w/]+-/issues\?label_name=#{label.name}\".+style=\"background-color: #\d{6}\".*>#{label.name}</span></a></span> ~unknown</p>) + ) + # rubocop:enable Layout/LineLength + end + end + + context 'when markdown contains table' do + let(:markdown) { '<table><tr><th>table head</th><tr><tr><td>table content</td></tr></table>'} + + it { is_expected.to eq('table headtable content') } end context 'when markdown contains a reference to an issue' do diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 3a34d39c722..eb7bbbf1646 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -155,10 +155,11 @@ RSpec.describe Gitlab::GitalyClient::CommitService do let(:recursive) { false } let(:pagination_params) { nil } - it 'sends a get_tree_entries message' do + it 'sends a get_tree_entries message with default limit' do + expected_pagination_params = Gitaly::PaginationParameter.new(limit: Gitlab::GitalyClient::CommitService::TREE_ENTRIES_DEFAULT_LIMIT) expect_any_instance_of(Gitaly::CommitService::Stub) .to receive(:get_tree_entries) - .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .with(gitaly_request_with_params({ pagination_params: expected_pagination_params }), kind_of(Hash)) .and_return([]) is_expected.to eq([[], nil]) @@ -188,9 +189,10 @@ RSpec.describe Gitlab::GitalyClient::CommitService do pagination_cursor: pagination_cursor ) + expected_pagination_params = Gitaly::PaginationParameter.new(limit: 3) expect_any_instance_of(Gitaly::CommitService::Stub) .to receive(:get_tree_entries) - .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .with(gitaly_request_with_params({ pagination_params: expected_pagination_params }), kind_of(Hash)) .and_return([response]) is_expected.to eq([[], pagination_cursor]) diff --git a/spec/lib/gitlab/reactive_cache_set_cache_spec.rb b/spec/lib/gitlab/reactive_cache_set_cache_spec.rb index f405b2ad86e..207ac1c0eaa 100644 --- a/spec/lib/gitlab/reactive_cache_set_cache_spec.rb +++ b/spec/lib/gitlab/reactive_cache_set_cache_spec.rb @@ -72,4 +72,18 @@ RSpec.describe Gitlab::ReactiveCacheSetCache, :clean_gitlab_redis_cache do it { is_expected.to be(true) } end end + + describe 'count' do + subject { cache.count(cache_prefix) } + + it { is_expected.to be(0) } + + context 'item added' do + before do + cache.write(cache_prefix, 'test_item') + end + + it { is_expected.to be(1) } + end + end end diff --git a/spec/lib/gitlab/zentao/client_spec.rb b/spec/lib/gitlab/zentao/client_spec.rb index 135f13e6265..b17ad867f0d 100644 --- a/spec/lib/gitlab/zentao/client_spec.rb +++ b/spec/lib/gitlab/zentao/client_spec.rb @@ -2,17 +2,21 @@ require 'spec_helper' -RSpec.describe Gitlab::Zentao::Client do - subject(:integration) { described_class.new(zentao_integration) } +RSpec.describe Gitlab::Zentao::Client, :clean_gitlab_redis_cache do + subject(:client) { described_class.new(zentao_integration) } let(:zentao_integration) { create(:zentao_integration) } def mock_get_products_url - integration.send(:url, "products/#{zentao_integration.zentao_product_xid}") + client.send(:url, "products/#{zentao_integration.zentao_product_xid}") + end + + def mock_fetch_issues_url + client.send(:url, "products/#{zentao_integration.zentao_product_xid}/issues") end def mock_fetch_issue_url(issue_id) - integration.send(:url, "issues/#{issue_id}") + client.send(:url, "issues/#{issue_id}") end let(:mock_headers) do @@ -29,13 +33,13 @@ RSpec.describe Gitlab::Zentao::Client do let(:zentao_integration) { nil } it 'raises ConfigError' do - expect { integration }.to raise_error(described_class::ConfigError) + expect { client }.to raise_error(described_class::ConfigError) end end context 'integration is provided' do it 'is initialized successfully' do - expect { integration }.not_to raise_error + expect { client }.not_to raise_error end end end @@ -50,7 +54,7 @@ RSpec.describe Gitlab::Zentao::Client do end it 'fetches the product' do - expect(integration.fetch_product(zentao_integration.zentao_product_xid)).to eq mock_response + expect(client.fetch_product(zentao_integration.zentao_product_xid)).to eq mock_response end end @@ -62,8 +66,8 @@ RSpec.describe Gitlab::Zentao::Client do it 'fetches the empty product' do expect do - integration.fetch_product(zentao_integration.zentao_product_xid) - end.to raise_error(Gitlab::Zentao::Client::Error, 'request error') + client.fetch_product(zentao_integration.zentao_product_xid) + end.to raise_error(Gitlab::Zentao::Client::RequestError) end end @@ -75,7 +79,7 @@ RSpec.describe Gitlab::Zentao::Client do it 'fetches the empty product' do expect do - integration.fetch_product(zentao_integration.zentao_product_xid) + client.fetch_product(zentao_integration.zentao_product_xid) end.to raise_error(Gitlab::Zentao::Client::Error, 'invalid response format') end end @@ -89,7 +93,7 @@ RSpec.describe Gitlab::Zentao::Client do end it 'responds with success' do - expect(integration.ping[:success]).to eq true + expect(client.ping[:success]).to eq true end end @@ -100,7 +104,69 @@ RSpec.describe Gitlab::Zentao::Client do end it 'responds with unsuccess' do - expect(integration.ping[:success]).to eq false + expect(client.ping[:success]).to eq false + end + end + end + + describe '#fetch_issues' do + let(:mock_response) { { 'issues' => [{ 'id' => 'story-1' }, { 'id' => 'bug-11' }] } } + + before do + WebMock.stub_request(:get, mock_fetch_issues_url) + .with(mock_headers).to_return(status: 200, body: mock_response.to_json) + end + + it 'returns the response' do + expect(client.fetch_issues).to eq(mock_response) + end + + describe 'marking the issues as seen in the product' do + let(:cache) { ::Gitlab::SetCache.new } + let(:cache_key) do + [ + :zentao_product_issues, + OpenSSL::Digest::SHA256.hexdigest(zentao_integration.client_url), + zentao_integration.zentao_product_xid + ].join(':') + end + + it 'adds issue ids to the cache' do + expect { client.fetch_issues }.to change { cache.read(cache_key) } + .from(be_empty) + .to match_array(%w[bug-11 story-1]) + end + + it 'does not add issue ids to the cache if max set size has been reached' do + cache.write(cache_key, %w[foo bar]) + stub_const("#{described_class}::CACHE_MAX_SET_SIZE", 1) + + client.fetch_issues + + expect(cache.read(cache_key)).to match_array(%w[foo bar]) + end + + it 'does not duplicate issue ids in the cache' do + client.fetch_issues + client.fetch_issues + + expect(cache.read(cache_key)).to match_array(%w[bug-11 story-1]) + end + + it 'touches the cache ttl every time issues are fetched' do + fresh_ttl = 1.month.to_i + + freeze_time do + client.fetch_issues + + expect(cache.ttl(cache_key)).to eq(fresh_ttl) + end + + travel_to(1.minute.from_now) do + client.fetch_issues + + expect(cache.ttl(cache_key)).to eq(fresh_ttl) + end end end end @@ -109,9 +175,9 @@ RSpec.describe Gitlab::Zentao::Client do context 'with invalid id' do let(:invalid_ids) { ['story', 'story-', '-', '123', ''] } - it 'returns empty object' do + it 'raises Error' do invalid_ids.each do |id| - expect { integration.fetch_issue(id) } + expect { client.fetch_issue(id) } .to raise_error(Gitlab::Zentao::Client::Error, 'invalid issue id') end end @@ -120,12 +186,31 @@ RSpec.describe Gitlab::Zentao::Client do context 'with valid id' do let(:valid_ids) { %w[story-1 bug-23] } - it 'fetches current issue' do - valid_ids.each do |id| - WebMock.stub_request(:get, mock_fetch_issue_url(id)) - .with(mock_headers).to_return(status: 200, body: { issue: { id: id } }.to_json) + context 'when issue has been seen on the index' do + before do + issues_body = { issues: valid_ids.map { { id: _1 } } }.to_json + + WebMock.stub_request(:get, mock_fetch_issues_url) + .with(mock_headers).to_return(status: 200, body: issues_body) + + client.fetch_issues + end + + it 'fetches the issue' do + valid_ids.each do |id| + WebMock.stub_request(:get, mock_fetch_issue_url(id)) + .with(mock_headers).to_return(status: 200, body: { issue: { id: id } }.to_json) + + expect(client.fetch_issue(id).dig('issue', 'id')).to eq id + end + end + end - expect(integration.fetch_issue(id).dig('issue', 'id')).to eq id + context 'when issue has not been seen on the index' do + it 'raises RequestError' do + valid_ids.each do |id| + expect { client.fetch_issue(id) }.to raise_error(Gitlab::Zentao::Client::RequestError) + end end end end @@ -135,7 +220,7 @@ RSpec.describe Gitlab::Zentao::Client do context 'api url' do shared_examples 'joins api_url correctly' do it 'verify url' do - expect(integration.send(:url, "products/1").to_s) + expect(client.send(:url, "products/1").to_s) .to eq("https://jihudemo.zentao.net/zentao/api.php/v1/products/1") end end @@ -157,7 +242,7 @@ RSpec.describe Gitlab::Zentao::Client do let(:zentao_integration) { create(:zentao_integration, url: 'https://jihudemo.zentao.net') } it 'joins url correctly' do - expect(integration.send(:url, "products/1").to_s) + expect(client.send(:url, "products/1").to_s) .to eq("https://jihudemo.zentao.net/api.php/v1/products/1") end end diff --git a/spec/models/incident_management/timeline_event_spec.rb b/spec/models/incident_management/timeline_event_spec.rb index 17150fc9266..9f4011fe6a7 100644 --- a/spec/models/incident_management/timeline_event_spec.rb +++ b/spec/models/incident_management/timeline_event_spec.rb @@ -47,11 +47,20 @@ RSpec.describe IncidentManagement::TimelineEvent do describe '#cache_markdown_field' do let(:note) { 'note **bold** _italic_ `code`  :+1:π' } + + let(:expected_image_html) do + '<a class="with-attachment-icon" href="/path/img.png" target="_blank" rel="noopener noreferrer">image</a>' + end + + # rubocop:disable Layout/LineLength + let(:expected_emoji_html) do + '<gl-emoji title="thumbs up sign" data-name="thumbsup" data-unicode-version="6.0">π</gl-emoji><gl-emoji title="thumbs up sign" data-name="thumbsup" data-unicode-version="6.0">π</gl-emoji>' + end + let(:expected_note_html) do - # rubocop:disable Layout/LineLength - '<p>note <strong>bold</strong> <em>italic</em> <code>code</code> <a class="with-attachment-icon" href="/path/img.png" target="_blank" rel="noopener noreferrer">image</a> ππ</p>' - # rubocop:enable Layout/LineLength + %Q(<p>note <strong>bold</strong> <em>italic</em> <code>code</code> #{expected_image_html} #{expected_emoji_html}</p>) end + # rubocop:enable Layout/LineLength before do allow(Banzai::Renderer).to receive(:cacheless_render_field).and_call_original diff --git a/spec/models/integrations/zentao_spec.rb b/spec/models/integrations/zentao_spec.rb index 4ef977ba3d2..1a32453819d 100644 --- a/spec/models/integrations/zentao_spec.rb +++ b/spec/models/integrations/zentao_spec.rb @@ -81,4 +81,24 @@ RSpec.describe Integrations::Zentao do expect(zentao_integration.help).not_to be_empty end end + + describe '#client_url' do + subject(:integration) { build(:zentao_integration, api_url: api_url, url: 'url').client_url } + + context 'when api_url is set' do + let(:api_url) { 'api_url' } + + it 'returns the api_url' do + is_expected.to eq(api_url) + end + end + + context 'when api_url is not set' do + let(:api_url) { '' } + + it 'returns the url' do + is_expected.to eq('url') + end + end + end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index d45a23a7ef8..eaa95de54b9 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -640,14 +640,22 @@ RSpec.describe Issue do end describe '#to_branch_name exists ending with -index' do - before do + it 'returns #to_branch_name ending with max index + 1' do allow(repository).to receive(:branch_exists?).and_return(true) allow(repository).to receive(:branch_exists?).with("#{subject.to_branch_name}-3").and_return(false) - end - it 'returns #to_branch_name ending with max index + 1' do expect(subject.suggested_branch_name).to eq("#{subject.to_branch_name}-3") end + + context 'when branch name still exists after 5 attempts' do + it 'returns #to_branch_name ending with random characters' do + allow(repository).to receive(:branch_exists?).with(subject.to_branch_name).and_return(true) + allow(repository).to receive(:branch_exists?).with(/#{subject.to_branch_name}-\d/).and_return(true) + allow(repository).to receive(:branch_exists?).with(/#{subject.to_branch_name}-\h{8}/).and_return(false) + + expect(subject.suggested_branch_name).to match(/#{subject.to_branch_name}-\h{8}/) + end + end end end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index a54edc8510e..da94441d621 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -91,6 +91,45 @@ RSpec.describe Snippet do end end end + + context 'description validations' do + let_it_be(:invalid_description) { 'a' * (described_class::DESCRIPTION_LENGTH_MAX * 2) } + + context 'with existing snippets' do + let(:snippet) { create(:personal_snippet, description: 'This is a valid content at the time of creation') } + + it 'does not raise a validation error if the description is not changed' do + snippet.title = 'new title' + + expect(snippet).to be_valid + end + + it 'raises and error if the description is changed and the size is bigger than limit' do + expect(snippet).to be_valid + + snippet.description = invalid_description + + expect(snippet).not_to be_valid + end + end + + context 'with new snippets' do + it 'is valid when description is smaller than the limit' do + snippet = build(:personal_snippet, description: 'Valid Desc') + + expect(snippet).to be_valid + end + + it 'raises error when description is bigger than setting limit' do + snippet = build(:personal_snippet, description: invalid_description) + + aggregate_failures do + expect(snippet).not_to be_valid + expect(snippet.errors.messages_for(:description)).to include("is too long (2 MB). The maximum size is 1 MB.") + end + end + end + end end describe 'callbacks' do diff --git a/spec/presenters/commit_presenter_spec.rb b/spec/presenters/commit_presenter_spec.rb index b221c9ca8f7..df3ee69621b 100644 --- a/spec/presenters/commit_presenter_spec.rb +++ b/spec/presenters/commit_presenter_spec.rb @@ -12,29 +12,51 @@ RSpec.describe CommitPresenter do it { expect(presenter.web_path).to eq("/#{project.full_path}/-/commit/#{commit.sha}") } end - describe '#status_for' do - subject { presenter.status_for('ref') } + describe '#detailed_status_for' do + using RSpec::Parameterized::TableSyntax + + let(:pipeline) { create(:ci_pipeline, :success, project: project, sha: commit.sha, ref: 'ref') } - context 'when user can read_commit_status' do + subject { presenter.detailed_status_for('ref')&.text } + + where(:read_commit_status, :read_pipeline, :expected_result) do + true | true | 'passed' + true | false | nil + false | true | nil + false | false | nil + end + + with_them do before do - allow(presenter).to receive(:can?).with(user, :read_commit_status, project).and_return(true) + allow(presenter).to receive(:can?).with(user, :read_commit_status, project).and_return(read_commit_status) + allow(presenter).to receive(:can?).with(user, :read_pipeline, pipeline).and_return(read_pipeline) end - it 'returns commit status for ref' do - pipeline = double - status = double + it { is_expected.to eq expected_result } + end + end - expect(commit).to receive(:latest_pipeline).with('ref').and_return(pipeline) - expect(pipeline).to receive(:detailed_status).with(user).and_return(status) + describe '#status_for' do + using RSpec::Parameterized::TableSyntax - expect(subject).to eq(status) - end + let(:pipeline) { create(:ci_pipeline, :success, project: project, sha: commit.sha) } + + subject { presenter.status_for } + + where(:read_commit_status, :read_pipeline, :expected_result) do + true | true | 'success' + true | false | nil + false | true | nil + false | false | nil end - context 'when user can not read_commit_status' do - it 'is nil' do - is_expected.to eq(nil) + with_them do + before do + allow(presenter).to receive(:can?).with(user, :read_commit_status, project).and_return(read_commit_status) + allow(presenter).to receive(:can?).with(user, :read_pipeline, pipeline).and_return(read_pipeline) end + + it { is_expected.to eq expected_result } end end diff --git a/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb b/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb index 31fef75f679..bcbb1f11d43 100644 --- a/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb +++ b/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb @@ -6,11 +6,16 @@ RSpec.describe 'getting incident timeline events' do include GraphqlHelpers let_it_be(:project) { create(:project) } + let_it_be(:private_project) { create(:project, :private) } + let_it_be(:issue) { create(:issue, project: private_project) } let_it_be(:current_user) { create(:user) } let_it_be(:updated_by_user) { create(:user) } let_it_be(:incident) { create(:incident, project: project) } let_it_be(:another_incident) { create(:incident, project: project) } let_it_be(:promoted_from_note) { create(:note, project: project, noteable: incident) } + let_it_be(:issue_url) { project_issue_url(private_project, issue) } + let_it_be(:issue_ref) { "#{private_project.full_path}##{issue.iid}" } + let_it_be(:issue_link) { %Q(<a href="#{issue_url}">#{issue_url}</a>) } let_it_be(:timeline_event) do create( @@ -18,7 +23,8 @@ RSpec.describe 'getting incident timeline events' do incident: incident, project: project, updated_by_user: updated_by_user, - promoted_from_note: promoted_from_note + promoted_from_note: promoted_from_note, + note: "Referencing #{issue.to_reference(full: true)} - Full URL #{issue_url}" ) end @@ -89,7 +95,7 @@ RSpec.describe 'getting incident timeline events' do 'title' => incident.title }, 'note' => timeline_event.note, - 'noteHtml' => timeline_event.note_html, + 'noteHtml' => "<p>Referencing #{issue_ref} - Full URL #{issue_link}</p>", 'promotedFromNote' => { 'id' => promoted_from_note.to_global_id.to_s, 'body' => promoted_from_note.note diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index 4d2a69cd85b..7d8213c48f7 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -752,6 +752,96 @@ RSpec.describe API::Search do it_behaves_like 'pagination', scope: :commits, search: 'merge' it_behaves_like 'ping counters', scope: :commits + + describe 'pipeline visibility' do + shared_examples 'pipeline information visible' do + it 'contains status and last_pipeline' do + request + + expect(json_response[0]['status']).to eq 'success' + expect(json_response[0]['last_pipeline']).not_to be_nil + end + end + + shared_examples 'pipeline information not visible' do + it 'does not contain status and last_pipeline' do + request + + expect(json_response[0]['status']).to be_nil + expect(json_response[0]['last_pipeline']).to be_nil + end + end + + let(:request) { get api(endpoint, user), params: { scope: 'commits', search: repo_project.commit.sha } } + + before do + create(:ci_pipeline, :success, project: repo_project, sha: repo_project.commit.sha) + end + + context 'with non public pipeline' do + let_it_be(:repo_project) do + create(:project, :public, :repository, public_builds: false, group: group) + end + + context 'user is project member with reporter role or above' do + before do + repo_project.add_reporter(user) + end + + it_behaves_like 'pipeline information visible' + end + + context 'user is project member with guest role' do + before do + repo_project.add_guest(user) + end + + it_behaves_like 'pipeline information not visible' + end + + context 'user is not project member' do + let_it_be(:user) { create(:user) } + + it_behaves_like 'pipeline information not visible' + end + end + + context 'with public pipeline' do + let_it_be(:repo_project) do + create(:project, :public, :repository, public_builds: true, group: group) + end + + context 'user is project member with reporter role or above' do + before do + repo_project.add_reporter(user) + end + + it_behaves_like 'pipeline information visible' + end + + context 'user is project member with guest role' do + before do + repo_project.add_guest(user) + end + + it_behaves_like 'pipeline information visible' + end + + context 'user is not project member' do + let_it_be(:user) { create(:user) } + + it_behaves_like 'pipeline information visible' + + context 'when CI/CD is set to only project members' do + before do + repo_project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + end + + it_behaves_like 'pipeline information not visible' + end + end + end + end end context 'for commits scope with project path as id' do diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 05b16119a0e..21d92adebce 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -643,17 +643,17 @@ RSpec.describe 'Git HTTP requests' do end context 'when username and password are provided' do - it 'rejects pulls with personal access token error message' do + it 'rejects pulls with generic error message' do download(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end - it 'rejects the push attempt with personal access token error message' do + it 'rejects the push attempt with generic error message' do upload(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end end @@ -750,17 +750,17 @@ RSpec.describe 'Git HTTP requests' do allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { false } end - it 'rejects pulls with personal access token error message' do + it 'rejects pulls with generic error message' do download(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end - it 'rejects pushes with personal access token error message' do + it 'rejects pushes with generic error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end @@ -771,10 +771,10 @@ RSpec.describe 'Git HTTP requests' do .to receive(:login).and_return(nil) end - it 'does not display the personal access token error message' do + it 'displays the generic error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).not_to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end end @@ -1300,17 +1300,18 @@ RSpec.describe 'Git HTTP requests' do end context 'when username and password are provided' do - it 'rejects pulls with personal access token error message' do + it 'rejects pulls with generic error message' do download(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end - it 'rejects the push attempt with personal access token error message' do + it 'rejects the push attempt with generic error message' do upload(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end end @@ -1381,17 +1382,17 @@ RSpec.describe 'Git HTTP requests' do allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { false } end - it 'rejects pulls with personal access token error message' do + it 'rejects pulls with generic error message' do download(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end - it 'rejects pushes with personal access token error message' do + it 'rejects pushes with generic error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end @@ -1402,10 +1403,10 @@ RSpec.describe 'Git HTTP requests' do .to receive(:login).and_return(nil) end - it 'does not display the personal access token error message' do + it 'returns a generic error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).not_to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end end diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index 70097234762..843d8f22d75 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -22,6 +22,37 @@ RSpec.describe JwtController do end end + shared_examples 'a token that expires today' do + let(:pat) { create(:personal_access_token, user: user, scopes: ['api'], expires_at: Date.today ) } + let(:headers) { { authorization: credentials('personal_access_token', pat.token) } } + + it 'fails authentication' do + expect(::Gitlab::AuthLogger).to receive(:warn).with( + hash_including(message: 'JWT authentication failed', + http_user: 'personal_access_token')).and_call_original + + get '/jwt/auth', params: parameters, headers: headers + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + shared_examples "with invalid credentials" do + it "returns a generic error message" do + subject + + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response).to eq( + { + "errors" => [{ + "code" => "UNAUTHORIZED", + "message" => "HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/user/profile/account/two_factor_authentication#troubleshooting" + }] + } + ) + end + end + context 'authenticating against container registry' do context 'existing service' do subject! { get '/jwt/auth', params: parameters } @@ -40,10 +71,7 @@ RSpec.describe JwtController 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 + it_behaves_like 'with invalid credentials' end end @@ -142,10 +170,7 @@ RSpec.describe JwtController do let(:user) { create(:user, :two_factor) } context 'without personal token' do - it 'rejects the authorization attempt' do - expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP') - end + it_behaves_like 'with invalid credentials' end context 'with personal token' do @@ -169,14 +194,10 @@ RSpec.describe JwtController do context 'using invalid login' do let(:headers) { { authorization: credentials('invalid', 'password') } } + let(:subject) { get '/jwt/auth', params: parameters, headers: headers } context 'when internal auth is enabled' do - it 'rejects the authorization attempt' do - get '/jwt/auth', params: parameters, headers: headers - - expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).not_to include('You must use a personal access token with \'api\' scope for Git over HTTP') - end + it_behaves_like 'with invalid credentials' end context 'when internal auth is disabled' do @@ -184,12 +205,7 @@ RSpec.describe JwtController do stub_application_setting(password_authentication_enabled_for_git: false) end - it 'rejects the authorization attempt with personal access token message' do - get '/jwt/auth', params: parameters, headers: headers - - expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP') - end + it_behaves_like 'with invalid credentials' end end end diff --git a/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb index 795545e4ad1..a97b89ed3a4 100644 --- a/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb @@ -159,6 +159,17 @@ RSpec.shared_examples 'PyPI package download' do |user_type, status, add_member end end +RSpec.shared_examples 'rejected package download' do |user_type, status, add_member = true| + context "for user type #{user_type}" do + before do + project.send("add_#{user_type}", user) if add_member && user_type != :anonymous + group.send("add_#{user_type}", user) if add_member && user_type != :anonymous + end + + it_behaves_like 'returning response status', status + end +end + RSpec.shared_examples 'process PyPI api request' do |user_type, status, add_member = true| context "for user type #{user_type}" do before do @@ -319,25 +330,25 @@ RSpec.shared_examples 'pypi file download endpoint' do using RSpec::Parameterized::TableSyntax context 'with valid project' do - where(:visibility_level, :user_role, :member, :user_token) do - :public | :developer | true | true - :public | :guest | true | true - :public | :developer | true | false - :public | :guest | true | false - :public | :developer | false | true - :public | :guest | false | true - :public | :developer | false | false - :public | :guest | false | false - :public | :anonymous | false | true - :private | :developer | true | true - :private | :guest | true | true - :private | :developer | true | false - :private | :guest | true | false - :private | :developer | false | true - :private | :guest | false | true - :private | :developer | false | false - :private | :guest | false | false - :private | :anonymous | false | true + where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do + :public | :developer | true | true | 'PyPI package download' | :success + :public | :guest | true | true | 'PyPI package download' | :success + :public | :developer | true | false | 'PyPI package download' | :success + :public | :guest | true | false | 'PyPI package download' | :success + :public | :developer | false | true | 'PyPI package download' | :success + :public | :guest | false | true | 'PyPI package download' | :success + :public | :developer | false | false | 'PyPI package download' | :success + :public | :guest | false | false | 'PyPI package download' | :success + :public | :anonymous | false | true | 'PyPI package download' | :success + :private | :developer | true | true | 'PyPI package download' | :success + :private | :guest | true | true | 'rejected package download' | :forbidden + :private | :developer | true | false | 'rejected package download' | :unauthorized + :private | :guest | true | false | 'rejected package download' | :unauthorized + :private | :developer | false | true | 'rejected package download' | :not_found + :private | :guest | false | true | 'rejected package download' | :not_found + :private | :developer | false | false | 'rejected package download' | :unauthorized + :private | :guest | false | false | 'rejected package download' | :unauthorized + :private | :anonymous | false | true | 'rejected package download' | :unauthorized end with_them do @@ -349,7 +360,7 @@ RSpec.shared_examples 'pypi file download endpoint' do group.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value(visibility_level.to_s)) end - it_behaves_like 'PyPI package download', params[:user_role], :success, params[:member] + it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] end end diff --git a/spec/validators/bytesize_validator_spec.rb b/spec/validators/bytesize_validator_spec.rb new file mode 100644 index 00000000000..1914ccedd87 --- /dev/null +++ b/spec/validators/bytesize_validator_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BytesizeValidator do + let(:model) do + Class.new do + include ActiveModel::Model + include ActiveModel::Validations + + attr_accessor :content + alias_method :content_before_type_cast, :content + + validates :content, bytesize: { maximum: -> { 7 } } + end.new + end + + using RSpec::Parameterized::TableSyntax + + where(:content, :validity, :errors) do + 'short' | true | {} + 'very long' | false | { content: ['is too long (9 Bytes). The maximum size is 7 Bytes.'] } + 'shortπ' | false | { content: ['is too long (9 Bytes). The maximum size is 7 Bytes.'] } + 'shortβ' | false | { content: ['is too long (8 Bytes). The maximum size is 7 Bytes.'] } + end + + with_them do + before do + model.content = content + model.validate + end + + it { expect(model.valid?).to eq(validity) } + it { expect(model.errors.messages).to eq(errors) } + end +end diff --git a/spec/views/projects/commits/_commit.html.haml_spec.rb b/spec/views/projects/commits/_commit.html.haml_spec.rb index da93871e0e4..8d7bee8f7f9 100644 --- a/spec/views/projects/commits/_commit.html.haml_spec.rb +++ b/spec/views/projects/commits/_commit.html.haml_spec.rb @@ -47,13 +47,12 @@ RSpec.describe 'projects/commits/_commit.html.haml' do context 'with ci status' do let(:ref) { 'master' } - let(:user) { create(:user) } + + let_it_be(:user) { create(:user) } before do allow(view).to receive(:current_user).and_return(user) - project.add_developer(user) - create( :ci_empty_pipeline, ref: 'master', @@ -80,18 +79,32 @@ RSpec.describe 'projects/commits/_commit.html.haml' do end context 'when pipelines are enabled' do - before do - allow(project).to receive(:builds_enabled?).and_return(true) + context 'when user has access' do + before do + project.add_developer(user) + end + + it 'displays a ci status icon' do + render partial: template, formats: :html, locals: { + project: project, + ref: ref, + commit: commit + } + + expect(rendered).to have_css('.ci-status-link') + end end - it 'does display a ci status icon when pipelines are enabled' do - render partial: template, formats: :html, locals: { - project: project, - ref: ref, - commit: commit - } + context 'when user does not have access' do + it 'does not display a ci status icon' do + render partial: template, formats: :html, locals: { + project: project, + ref: ref, + commit: commit + } - expect(rendered).to have_css('.ci-status-link') + expect(rendered).not_to have_css('.ci-status-link') + end end end end diff --git a/yarn.lock b/yarn.lock index 7bbe41b71e7..95f728433cd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -976,6 +976,14 @@ exec-sh "^0.3.2" minimist "^1.2.0" +"@codesandbox/sandpack-client@^1.2.2": + version "1.2.2" + resolved "https://registry.yarnpkg.com/@codesandbox/sandpack-client/-/sandpack-client-1.2.2.tgz#e0b79c52dcbc0b622f93527dc9ff3b163467e14a" + integrity sha512-sTPQVS7mzpEm2ttpHFFSqkGd1A1tBZn7UTZwIjBNCXKHywrt9o7MyrdhUuS03J7MyXN+HSJ55Vz+OGD1Wv4ejQ== + dependencies: + codesandbox-import-utils "^1.2.3" + lodash.isequal "^4.5.0" + "@discoveryjs/json-ext@^0.5.0": version "0.5.6" resolved "https://registry.yarnpkg.com/@discoveryjs/json-ext/-/json-ext-0.5.6.tgz#d5e0706cf8c6acd8c6032f8d54070af261bbbb2f" @@ -3071,9 +3079,9 @@ binary-extensions@^2.0.0: integrity sha512-Phlt0plgpIIBOGTT/ehfFnbNlfsDEiqmzE2KRXoX1bLIlir4X/MR+zSyBEkL05ffWgnRSf/DXv+WrUAVr93/ow== binaryextensions@2: - version "2.1.1" - resolved "https://registry.yarnpkg.com/binaryextensions/-/binaryextensions-2.1.1.tgz#3209a51ca4a4ad541a3b8d3d6a6d5b83a2485935" - integrity sha512-XBaoWE9RW8pPdPQNibZsW2zh8TW6gcarXp1FZPwT8Uop8ScSNldJEWf2k9l3HeTqdrEwsOsFcq74RiJECW34yA== + version "2.3.0" + resolved "https://registry.yarnpkg.com/binaryextensions/-/binaryextensions-2.3.0.tgz#1d269cbf7e6243ea886aa41453c3651ccbe13c22" + integrity sha512-nAihlQsYGyc5Bwq6+EsubvANYGExeJKHDO3RjnvwU042fawQTQfM3Kxn7IHUXQOz4bzfwsGYYHGSvXyW4zOGLg== bluebird@^3.1.1, bluebird@^3.5.5, bluebird@~3.5.0: version "3.5.5" @@ -3662,18 +3670,18 @@ codesandbox-api@0.0.23: resolved "https://registry.yarnpkg.com/codesandbox-api/-/codesandbox-api-0.0.23.tgz#bf650a21b5f3c2369e03f0c19d10b4e2ba255b4f" integrity sha512-fFGBkIghDkQILh7iHYlpZU5sfWncCDb92FQSFE4rR3VBcTfUsD5VZgpQi+JjZQuwWIdfl4cOhcIFrUYwshUezA== -codesandbox-import-util-types@^1.2.11: - version "1.2.11" - resolved "https://registry.yarnpkg.com/codesandbox-import-util-types/-/codesandbox-import-util-types-1.2.11.tgz#68e812f21d6b309e9a52eec5cf027c3e63b4c703" - integrity sha512-n1PC/OQ0tcD9o6N5TStBB/A7tKOggUjuhnNxUU5GnVol8vmKMMLvmC6tK+8iDovQb2X2+xoDCBnl5BBgZ5OcIQ== +codesandbox-import-util-types@^1.3.7: + version "1.3.7" + resolved "https://registry.yarnpkg.com/codesandbox-import-util-types/-/codesandbox-import-util-types-1.3.7.tgz#7a6097e248a75424d13b06b74368cd76bd2b3e10" + integrity sha512-8oP3emA0jyEuVOM2FBTpo/AF4C9vxHn14saVWZf2CQ/QhMtonBlNPE98ElrHkW+PFNXiO7Ad52Qr73b03n8qlA== codesandbox-import-utils@^1.2.3: - version "1.2.11" - resolved "https://registry.yarnpkg.com/codesandbox-import-utils/-/codesandbox-import-utils-1.2.11.tgz#b88423a4a7c785175c784c84e87f5950820280e1" - integrity sha512-KPuf7tR/SMPSRfqjWbTrYvIaW6Yt9Ajt/1FB64RsOv4BLjBNo6CwLCCPoRHYcrAKSafpWkghTZ2Bffyz7EX7AA== + version "1.3.8" + resolved "https://registry.yarnpkg.com/codesandbox-import-utils/-/codesandbox-import-utils-1.3.8.tgz#5576786439c5f37ebd3fee5751e06027a1edef84" + integrity sha512-S12zO49QEkldoYLGh5KbkHRLOacg5BCNTue2vlyZXSpuK3oQdArwC/G1hCLKryV460bW3Ecn5xdkpfkUcFeOwQ== dependencies: - codesandbox-import-util-types "^1.2.11" - istextorbinary "^2.2.1" + codesandbox-import-util-types "^1.3.7" + istextorbinary "2.2.1" lz-string "^1.4.4" collect-v8-coverage@^1.0.0: @@ -7389,7 +7397,7 @@ istanbul-reports@^3.0.0, istanbul-reports@^3.0.2: html-escaper "^2.0.0" istanbul-lib-report "^3.0.0" -istextorbinary@^2.2.1: +istextorbinary@2.2.1: version "2.2.1" resolved "https://registry.yarnpkg.com/istextorbinary/-/istextorbinary-2.2.1.tgz#a5231a08ef6dd22b268d0895084cf8d58b5bec53" integrity sha512-TS+hoFl8Z5FAFMK38nhBkdLt44CclNRgDHWeMgsV8ko3nDlr/9UI2Sf839sW7enijf8oKsZYXRvM8g0it9Zmcw== @@ -8434,7 +8442,7 @@ lru-cache@^6.0.0: lz-string@^1.4.4: version "1.4.4" resolved "https://registry.yarnpkg.com/lz-string/-/lz-string-1.4.4.tgz#c0d8eaf36059f705796e1e344811cf4c498d3a26" - integrity sha1-wNjq82BZ9wV5bh40SBHPTEmNOiY= + integrity sha512-0ckx7ZHRPqb0oUm8zNr+90mtf9DQB60H1wMCjBtfi62Kl3a7JbHob6gA2bC+xRvZoOL+1hzUK8jeuEIQE8svEQ== make-dir@^2.0.0: version "2.1.0" @@ -11437,15 +11445,6 @@ slice-ansi@^4.0.0: astral-regex "^2.0.0" is-fullwidth-code-point "^3.0.0" -smooshpack@^0.0.62: - version "0.0.62" - resolved "https://registry.yarnpkg.com/smooshpack/-/smooshpack-0.0.62.tgz#cb31b9f808f73de3146b050f84d044eb353b5503" - integrity sha512-lFuJV2f504/U78sifWy0V2FyoE/8mTgOXM4DL918ncNxAxbtu236XSCLAH3SQwXZWn0JdmRnWs/XU4+sIUVVmQ== - dependencies: - codesandbox-api "0.0.23" - codesandbox-import-utils "^1.2.3" - lodash.isequal "^4.5.0" - snapdragon-node@^2.0.1: version "2.1.1" resolved "https://registry.yarnpkg.com/snapdragon-node/-/snapdragon-node-2.1.1.tgz#6c175f86ff14bdb0724563e8f3c1b021a286853b" @@ -12062,9 +12061,9 @@ text-table@^0.2.0: integrity sha1-f17oI66AUgfACvLfSoTsP8+lcLQ= textextensions@2: - version "2.2.0" - resolved "https://registry.yarnpkg.com/textextensions/-/textextensions-2.2.0.tgz#38ac676151285b658654581987a0ce1a4490d286" - integrity sha512-j5EMxnryTvKxwH2Cq+Pb43tsf6sdEgw6Pdwxk83mPaq0ToeFJt6WE4J3s5BqY7vmjlLgkgXvhtXUxo80FyBhCA== + version "2.6.0" + resolved "https://registry.yarnpkg.com/textextensions/-/textextensions-2.6.0.tgz#d7e4ab13fe54e32e08873be40d51b74229b00fc4" + integrity sha512-49WtAWS+tcsy93dRt6P0P3AMD2m5PvXRhuEA0kaXos5ZLlujtYmpmFsB+QvWUSxE1ZsstmYXfQ7L40+EcQgpAQ== three-orbit-controls@^82.1.0: version "82.1.0" |