diff options
| author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-03-31 13:46:52 +0000 |
|---|---|---|
| committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-03-31 13:46:52 +0000 |
| commit | ad82eede77ba61c886b3f9b12551070bb80ad0a3 (patch) | |
| tree | d7a062fdd1da63da017ad306eac0d65436372ec5 | |
| parent | 9499073eb7e284ef1a867c6636a29bddb912fa17 (diff) | |
| parent | 10669ffeb75d4f5f010c186fa5f4bd0de8b8574e (diff) | |
| download | gitlab-ce-14-7-stable.tar.gz | |
Merge remote-tracking branch 'dev/14-7-stable' into 14-7-stable14-7-stable
94 files changed, 1374 insertions, 282 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index cfae24aa2bb..1f26dca1730 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,32 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 14.7.7 (2022-03-31) + +### Security (21 changes) + +- [Update to commonmarker 0.23.4](gitlab-org/security/gitlab@eb4b231173c86901f93b5b7781716b1f7706dad1) ([merge request](gitlab-org/security/gitlab!2283)) +- [Revert merge request approval groups behavior](gitlab-org/security/gitlab@08e3ecced649f6ad241db6de7050b1502f7bef21) ([merge request](gitlab-org/security/gitlab!2333)) +- [Disallow login if password matches a fixed list](gitlab-org/security/gitlab@02a69ab32da1ac67d855de3ee388d0bd2bb6586e) ([merge request](gitlab-org/security/gitlab!2359)) +- [Update devise-two-factor to 4.0.2](gitlab-org/security/gitlab@c9fde96c7780f5b883cd1ac63d7ac3d5f4d78dc6) ([merge request](gitlab-org/security/gitlab!2351)) +- [Limit the number of tags associated with a CI runner](gitlab-org/security/gitlab@00124d5f8ba0d7437d1f6f19b029754bf481185b) ([merge request](gitlab-org/security/gitlab!2305)) +- [GitLab Pages Security Updates for 14.9](gitlab-org/security/gitlab@d335917e233658fa9d4452053469c3582ef38368) ([merge request](gitlab-org/security/gitlab!2325)) +- [Upgrade swagger-ui dependency](gitlab-org/security/gitlab@7a8ce32f70fd0338817705651ee0dbe0a277d5f1) ([merge request](gitlab-org/security/gitlab!2338)) +- [Modify release link format check to avoid regex if string is too long](gitlab-org/security/gitlab@e18dc2be245bca7e192c8536d1ba7de2ad798c43) ([merge request](gitlab-org/security/gitlab!2244)) +- [Masks variables in error messages](gitlab-org/security/gitlab@1706c5cf9b939a6ab0682db7b8945feb851a3f8b) ([merge request](gitlab-org/security/gitlab!2292)) +- [Escape user provided string to prevent XSS](gitlab-org/security/gitlab@c57edf9ab52810d455e41d71bad4e4d12c098cad) ([merge request](gitlab-org/security/gitlab!2315)) +- [Monkey patch of RDoc to prevent Ruby segfault](gitlab-org/security/gitlab@f9e5597d1864d03bf1f0103787becbc84886968d) ([merge request](gitlab-org/security/gitlab!2233)) +- [Project import maps members' created_by_id users based on source user ID](gitlab-org/security/gitlab@3ea1e477e0596f15e040f42b59fa86953d057128) ([merge request](gitlab-org/security/gitlab!2239)) +- [Redact InvalidURIError error messages](gitlab-org/security/gitlab@a42ede835e32f44b68c1affe78a7ee48332bb30a) ([merge request](gitlab-org/security/gitlab!2297)) +- [Fix access for approval rules API](gitlab-org/security/gitlab@b8c3997763d1e041dc2b82e464a99a5b2f15a798) ([merge request](gitlab-org/security/gitlab!2324)) +- [Fix kroki exploit](gitlab-org/security/gitlab@ad123e33510103af4fb00378ef1fc8dae4cacb21) ([merge request](gitlab-org/security/gitlab!2278)) +- [Fix blind SSRF when looking up SSH host keys for mirroring](gitlab-org/security/gitlab@0209f44cb4876f0a9ef13d4c8875a95a0cda1e2f) ([merge request](gitlab-org/security/gitlab!2311)) +- [Escape original content in reference redactor](gitlab-org/security/gitlab@f63861d8fe7b2b8d161162063e7995782cbfada8) ([merge request](gitlab-org/security/gitlab!2319)) +- [Security fix for CI/CD analytics visibility](gitlab-org/security/gitlab@fea6a4ff80862f9dba493405d03d82cf129e8854) ([merge request](gitlab-org/security/gitlab!2274)) +- [Latest commit exposed through fork of a private project](gitlab-org/security/gitlab@b573cea38cdce020e5f25fb9de60e0e506c87a9b) ([merge request](gitlab-org/security/gitlab!2272)) +- [Fix Asana integration restricted branch filter](gitlab-org/security/gitlab@56e2d9ae3de4f587d2c8a5aa111c2922553d6b7b) ([merge request](gitlab-org/security/gitlab!2214)) +- [Revert "JH need more complex passwords"](gitlab-org/security/gitlab@2419522b02700ce98e0c4d6e7bfd4d28b6464506) ([merge request](gitlab-org/security/gitlab!2354)) + ## 14.7.6 (2022-03-24) ### Added (1 change) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index cb5c184a7e6..d3938fb3d67 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -14.7.6
\ No newline at end of file +14.7.7
\ No newline at end of file diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index ba0a719118c..c73f500ec8c 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -1.51.0 +1.51.1 @@ -65,7 +65,7 @@ gem 'akismet', '~> 3.0' gem 'invisible_captcha', '~> 1.1.0' # Two-factor authentication -gem 'devise-two-factor', '~> 4.0.0' +gem 'devise-two-factor', '~> 4.0.2' gem 'rqrcode-rails3', '~> 0.1.7' gem 'attr_encrypted', '~> 3.1.0' gem 'u2f', '~> 0.2.1' @@ -153,7 +153,7 @@ gem 'html-pipeline', '~> 2.13.2' gem 'deckar01-task_list', '2.3.1' gem 'gitlab-markup', '~> 1.8.0' gem 'github-markup', '~> 1.7.0', require: 'github/markup' -gem 'commonmarker', '~> 0.23.2' +gem 'commonmarker', '~> 0.23.4' gem 'kramdown', '~> 2.3.1' gem 'RedCloth', '~> 4.3.2' gem 'rdoc', '~> 6.3.2' diff --git a/Gemfile.lock b/Gemfile.lock index 4fd0bf053c4..13667507339 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -200,7 +200,7 @@ GEM open4 (~> 1.3) coderay (1.1.3) colored2 (3.1.2) - commonmarker (0.23.2) + commonmarker (0.23.4) concurrent-ruby (1.1.9) connection_pool (2.2.2) contracts (0.11.0) @@ -265,11 +265,11 @@ GEM railties (>= 4.1.0) responders warden (~> 1.2.3) - devise-two-factor (4.0.0) - activesupport (< 6.2) + devise-two-factor (4.0.2) + activesupport (< 7.1) attr_encrypted (>= 1.3, < 4, != 2) devise (~> 4.0) - railties (< 6.2) + railties (< 7.1) rotp (~> 6.0) diff-lcs (1.4.4) diff_match_patch (0.1.0) @@ -1421,7 +1421,7 @@ DEPENDENCIES capybara-screenshot (~> 1.0.22) carrierwave (~> 1.3) charlock_holmes (~> 0.7.7) - commonmarker (~> 0.23.2) + commonmarker (~> 0.23.4) concurrent-ruby (~> 1.1) connection_pool (~> 2.0) countries (~> 3.0) @@ -1435,7 +1435,7 @@ DEPENDENCIES derailed_benchmarks device_detector devise (~> 4.7.2) - devise-two-factor (~> 4.0.0) + devise-two-factor (~> 4.0.2) diff_match_patch (~> 0.1.0) diffy (~> 3.3) discordrb-webhooks (~> 3.4) @@ -1 +1 @@ -14.7.6
\ No newline at end of file +14.7.7
\ No newline at end of file diff --git a/app/assets/javascripts/behaviors/components/diagram_performance_warning.vue b/app/assets/javascripts/behaviors/components/diagram_performance_warning.vue new file mode 100644 index 00000000000..31b2682b546 --- /dev/null +++ b/app/assets/javascripts/behaviors/components/diagram_performance_warning.vue @@ -0,0 +1,25 @@ +<script> +import { GlAlert } from '@gitlab/ui'; +import { __ } from '~/locale'; + +export default { + i18n: { + bodyText: __('Warning: Displaying this diagram might cause performance issues on this page.'), + buttonText: __('Display'), + }, + components: { + GlAlert, + }, +}; +</script> + +<template> + <gl-alert + :primary-button-text="$options.i18n.buttonText" + variant="warning" + @dismiss="$emit('closeAlert')" + @primaryAction="$emit('showImage')" + > + {{ $options.i18n.bodyText }} + </gl-alert> +</template> diff --git a/app/assets/javascripts/behaviors/markdown/constants.js b/app/assets/javascripts/behaviors/markdown/constants.js index b4545d6c6c6..13f8d9ef0cf 100644 --- a/app/assets/javascripts/behaviors/markdown/constants.js +++ b/app/assets/javascripts/behaviors/markdown/constants.js @@ -1,3 +1,19 @@ // https://prosemirror.net/docs/ref/#model.ParseRule.priority export const DEFAULT_PARSE_RULE_PRIORITY = 50; export const HIGHER_PARSE_RULE_PRIORITY = 1 + DEFAULT_PARSE_RULE_PRIORITY; + +export const unrestrictedPages = [ + // Group wiki + 'groups:wikis:show', + 'groups:wikis:edit', + 'groups:wikis:create', + + // Project wiki + 'projects:wikis:show', + 'projects:wikis:edit', + 'projects:wikis:create', + + // Project files + 'projects:show', + 'projects:blob:show', +]; diff --git a/app/assets/javascripts/behaviors/markdown/render_gfm.js b/app/assets/javascripts/behaviors/markdown/render_gfm.js index c4e09efe263..6236e3bdefc 100644 --- a/app/assets/javascripts/behaviors/markdown/render_gfm.js +++ b/app/assets/javascripts/behaviors/markdown/render_gfm.js @@ -2,6 +2,7 @@ import $ from 'jquery'; import syntaxHighlight from '~/syntax_highlight'; import initUserPopovers from '../../user_popovers'; import highlightCurrentUser from './highlight_current_user'; +import { renderKroki } from './render_kroki'; import renderMath from './render_math'; import renderMermaid from './render_mermaid'; import renderSandboxedMermaid from './render_sandboxed_mermaid'; @@ -13,6 +14,7 @@ import renderMetrics from './render_metrics'; // $.fn.renderGFM = function renderGFM() { syntaxHighlight(this.find('.js-syntax-highlight').get()); + renderKroki(this.find('.js-render-kroki[hidden]').get()); renderMath(this.find('.js-render-math')); if (gon.features?.sandboxedMermaid) { renderSandboxedMermaid(this.find('.js-render-mermaid')); diff --git a/app/assets/javascripts/behaviors/markdown/render_kroki.js b/app/assets/javascripts/behaviors/markdown/render_kroki.js new file mode 100644 index 00000000000..7843df0cd8e --- /dev/null +++ b/app/assets/javascripts/behaviors/markdown/render_kroki.js @@ -0,0 +1,63 @@ +import Vue from 'vue'; +import DiagramPerformanceWarning from '../components/diagram_performance_warning.vue'; +import { unrestrictedPages } from './constants'; + +/** + * Create alert element. + * + * @param {Element} krokiImage Kroki `img` element + * @return {Element} Alert element + */ +function createAlert(krokiImage) { + const app = new Vue({ + el: document.createElement('div'), + name: 'DiagramPerformanceWarningRoot', + render(createElement) { + return createElement(DiagramPerformanceWarning, { + on: { + closeAlert() { + app.$destroy(); + app.$el.remove(); + }, + showImage() { + krokiImage.removeAttribute('hidden'); + app.$destroy(); + app.$el.remove(); + }, + }, + }); + }, + }); + + return app.$el; +} + +/** + * Add warning alert to hidden Kroki images, + * or show Kroki image if on an unrestricted page. + * + * Kroki images are given a hidden attribute by the + * backend when the original markdown source is large. + * + * @param {Array<Element>} krokiImages Array of hidden Kroki `img` elements + */ +export function renderKroki(krokiImages) { + const pageName = document.querySelector('body').dataset.page; + const isUnrestrictedPage = unrestrictedPages.includes(pageName); + + krokiImages.forEach((krokiImage) => { + if (isUnrestrictedPage) { + krokiImage.removeAttribute('hidden'); + return; + } + + const parent = krokiImage.closest('.js-markdown-code'); + + // A single Kroki image is processed multiple times for some reason, + // so this condition ensures we only create one alert per Kroki image + if (!parent.hasAttribute('data-kroki-processed')) { + parent.setAttribute('data-kroki-processed', 'true'); + parent.after(createAlert(krokiImage)); + } + }); +} diff --git a/app/assets/javascripts/behaviors/markdown/render_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_mermaid.js index d78c456ed5b..f9cf3af98bb 100644 --- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js @@ -3,6 +3,7 @@ import { once, countBy } from 'lodash'; import createFlash from '~/flash'; import { darkModeEnabled } from '~/lib/utils/color_utils'; import { __, sprintf } from '~/locale'; +import { unrestrictedPages } from './constants'; // Renders diagrams and flowcharts from text using Mermaid in any element with the // `js-render-mermaid` class. @@ -30,24 +31,6 @@ let renderedMermaidBlocks = 0; let mermaidModule = {}; -// Whitelist pages where we won't impose any restrictions -// on mermaid rendering -const WHITELISTED_PAGES = [ - // Group wiki - 'groups:wikis:show', - 'groups:wikis:edit', - 'groups:wikis:create', - - // Project wiki - 'projects:wikis:show', - 'projects:wikis:edit', - 'projects:wikis:create', - - // Project files - 'projects:show', - 'projects:blob:show', -]; - export function initMermaid(mermaid) { let theme = 'neutral'; @@ -163,7 +146,7 @@ function renderMermaids($els) { * up the entire thread and causing a DoS. */ if ( - !WHITELISTED_PAGES.includes(pageName) && + !unrestrictedPages.includes(pageName) && ((source && source.length > MAX_CHAR_LIMIT) || renderedChars > MAX_CHAR_LIMIT || renderedMermaidBlocks >= MAX_MERMAID_BLOCK_LIMIT || diff --git a/app/assets/javascripts/behaviors/markdown/render_sandboxed_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_sandboxed_mermaid.js index 1d54a1b0c04..399e4916696 100644 --- a/app/assets/javascripts/behaviors/markdown/render_sandboxed_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_sandboxed_mermaid.js @@ -9,6 +9,7 @@ import { } from '~/lib/utils/url_utility'; import { darkModeEnabled } from '~/lib/utils/color_utils'; import { setAttributes } from '~/lib/utils/dom_utils'; +import { unrestrictedPages } from './constants'; // Renders diagrams and flowcharts from text using Mermaid in any element with the // `js-render-mermaid` class. @@ -36,23 +37,6 @@ const BUFFER_IFRAME_HEIGHT = 10; const elsProcessingMap = new WeakMap(); let renderedMermaidBlocks = 0; -// Pages without any restrictions on mermaid rendering -const PAGES_WITHOUT_RESTRICTIONS = [ - // Group wiki - 'groups:wikis:show', - 'groups:wikis:edit', - 'groups:wikis:create', - - // Project wiki - 'projects:wikis:show', - 'projects:wikis:edit', - 'projects:wikis:create', - - // Project files - 'projects:show', - 'projects:blob:show', -]; - function shouldLazyLoadMermaidBlock(source) { /** * If source contains `&`, which means that it might @@ -149,7 +133,7 @@ function renderMermaids($els) { * up the entire thread and causing a DoS. */ if ( - !PAGES_WITHOUT_RESTRICTIONS.includes(pageName) && + !unrestrictedPages.includes(pageName) && ((source && source.length > MAX_CHAR_LIMIT) || renderedChars > MAX_CHAR_LIMIT || renderedMermaidBlocks >= MAX_MERMAID_BLOCK_LIMIT || diff --git a/app/assets/javascripts/blob/openapi/index.js b/app/assets/javascripts/blob/openapi/index.js index b19cc19cb8c..a04da98ff77 100644 --- a/app/assets/javascripts/blob/openapi/index.js +++ b/app/assets/javascripts/blob/openapi/index.js @@ -1,6 +1,5 @@ import { SwaggerUIBundle } from 'swagger-ui-dist'; import createFlash from '~/flash'; -import { removeParams, updateHistory } from '~/lib/utils/url_utility'; import { __ } from '~/locale'; export default () => { @@ -8,14 +7,10 @@ export default () => { Promise.all([import(/* webpackChunkName: 'openapi' */ 'swagger-ui-dist/swagger-ui.css')]) .then(() => { - // Temporary fix to prevent an XSS attack due to "useUnsafeMarkdown" - // Once we upgrade Swagger to "4.0.0", we can safely remove this as it will be deprecated - // Follow-up issue: https://gitlab.com/gitlab-org/gitlab/-/issues/339696 - updateHistory({ url: removeParams(['useUnsafeMarkdown']), replace: true }); SwaggerUIBundle({ url: el.dataset.endpoint, dom_id: '#js-openapi-viewer', - useUnsafeMarkdown: false, + deepLinking: true, }); }) .catch((error) => { diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index 88337242fcd..93e2298ca98 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -81,7 +81,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap def branch_to @target_project = selected_target_project - if @target_project && params[:ref].present? + if @target_project && params[:ref].present? && Ability.allowed?(current_user, :create_merge_request_in, @target_project) @ref = params[:ref] @commit = @target_project.commit(Gitlab::Git::BRANCH_REF_PREFIX + @ref) end diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 809c245d2b9..49d8f2d7c4d 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -65,6 +65,8 @@ module Ci FORM_EDITABLE = %i[description tag_list active run_untagged locked access_level maximum_timeout_human_readable].freeze MINUTES_COST_FACTOR_FIELDS = %i[public_projects_minutes_cost_factor private_projects_minutes_cost_factor].freeze + TAG_LIST_MAX_LENGTH = 50 + has_many :builds has_many :runner_projects, inverse_of: :runner, autosave: true, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :projects, through: :runner_projects @@ -508,6 +510,11 @@ module Ci errors.add(:tags_list, 'can not be empty when runner is not allowed to pick untagged jobs') end + + if tag_list_changed? && tag_list.count > TAG_LIST_MAX_LENGTH + errors.add(:tags_list, + "Too many tags specified. Please limit the number of tags to #{TAG_LIST_MAX_LENGTH}") + end end # TODO: remove this method once feature flag ci_runners_short_circuit_assignable_for diff --git a/app/models/integrations/asana.rb b/app/models/integrations/asana.rb index 7949563a1dc..acce2d98798 100644 --- a/app/models/integrations/asana.rb +++ b/app/models/integrations/asana.rb @@ -61,12 +61,9 @@ module Integrations def execute(data) return unless supported_events.include?(data[:object_kind]) - # check the branch restriction is poplulated and branch is not included branch = Gitlab::Git.ref_name(data[:ref]) - branch_restriction = restrict_to_branch.to_s - if branch_restriction.present? && branch_restriction.index(branch).nil? - return - end + + return unless branch_allowed?(branch) user = data[:user_name] project_name = project.full_name @@ -105,5 +102,13 @@ module Integrations end end end + + private + + def branch_allowed?(branch_name) + return true if restrict_to_branch.blank? + + restrict_to_branch.to_s.gsub(/\s+/, '').split(',').include?(branch_name) + end end end diff --git a/app/models/releases/link.rb b/app/models/releases/link.rb index acc56d3980a..347adbdf96a 100644 --- a/app/models/releases/link.rb +++ b/app/models/releases/link.rb @@ -9,10 +9,20 @@ module Releases # See https://gitlab.com/gitlab-org/gitlab/-/issues/218753 # Regex modified to prevent catastrophic backtracking FILEPATH_REGEX = %r{\A\/[^\/](?!.*\/\/.*)[\-\.\w\/]+[\da-zA-Z]+\z}.freeze + FILEPATH_MAX_LENGTH = 128 validates :url, presence: true, addressable_url: { schemes: %w(http https ftp) }, uniqueness: { scope: :release } validates :name, presence: true, uniqueness: { scope: :release } - validates :filepath, uniqueness: { scope: :release }, format: { with: FILEPATH_REGEX }, allow_blank: true, length: { maximum: 128 } + validates :filepath, uniqueness: { scope: :release }, allow_blank: true + validate :filepath_format_valid? + + # we use a custom validator here to prevent running the regex if the string is too long + # see https://gitlab.com/gitlab-org/gitlab/-/issues/273771 + def filepath_format_valid? + return if filepath.nil? # valid use case + return errors.add(:filepath, "is too long (maximum is #{FILEPATH_MAX_LENGTH} characters)") if filepath.length > FILEPATH_MAX_LENGTH + return errors.add(:filepath, 'is in an invalid format') unless FILEPATH_REGEX.match? filepath + end scope :sorted, -> { order(created_at: :desc) } diff --git a/app/models/ssh_host_key.rb b/app/models/ssh_host_key.rb index bb928118edf..ac7ba9530dd 100644 --- a/app/models/ssh_host_key.rb +++ b/app/models/ssh_host_key.rb @@ -46,11 +46,11 @@ class SshHostKey .select(&:valid?) end - attr_reader :project, :url, :compare_host_keys + attr_reader :project, :url, :ip, :compare_host_keys def initialize(project:, url:, compare_host_keys: nil) @project = project - @url = normalize_url(url) + @url, @ip = normalize_url(url) @compare_host_keys = compare_host_keys end @@ -90,9 +90,11 @@ class SshHostKey end def calculate_reactive_cache + input = [ip, url.hostname].compact.join(' ') + known_hosts, errors, status = Open3.popen3({}, *%W[ssh-keyscan -T 5 -p #{url.port} -f-]) do |stdin, stdout, stderr, wait_thr| - stdin.puts(url.host) + stdin.puts(input) stdin.close [ @@ -127,11 +129,31 @@ class SshHostKey end def normalize_url(url) - full_url = ::Addressable::URI.parse(url) - raise ArgumentError, "Invalid URL" unless full_url&.scheme == 'ssh' + url, real_hostname = Gitlab::UrlBlocker.validate!( + url, + schemes: %w[ssh], + allow_localhost: allow_local_requests?, + allow_local_network: allow_local_requests?, + dns_rebind_protection: Gitlab::CurrentSettings.dns_rebinding_protection_enabled? + ) + + # When DNS rebinding protection is required, the hostname is replaced by the + # resolved IP. However, `url` is used in `id`, so we can't change it. Track + # the resolved IP separately instead. + if real_hostname + ip = url.hostname + url.hostname = real_hostname + end + + # Ensure ssh://foo and ssh://foo:22 share the same cache + url.port = url.inferred_port - Addressable::URI.parse("ssh://#{full_url.host}:#{full_url.inferred_port}") - rescue Addressable::URI::InvalidURIError + [url, ip] + rescue Gitlab::UrlBlocker::BlockedUrlError raise ArgumentError, "Invalid URL" end + + def allow_local_requests? + Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services? + end end diff --git a/app/models/user.rb b/app/models/user.rb index 1d452fc2e50..66ac1df6c27 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -895,6 +895,23 @@ class User < ApplicationRecord reset_password_sent_at.present? && reset_password_sent_at >= 1.minute.ago end + # See https://gitlab.com/gitlab-org/security/gitlab/-/issues/638 + DISALLOWED_PASSWORDS = %w[123qweQWE!@#000000000].freeze + + # Overwrites valid_password? from Devise::Models::DatabaseAuthenticatable + # In constant-time, check both that the password isn't on a denylist AND + # that the password is the user's password + def valid_password?(password) + password_allowed = true + DISALLOWED_PASSWORDS.each do |disallowed_password| + password_allowed = false if Devise.secure_compare(password, disallowed_password) + end + + original_result = super + + password_allowed && original_result + end + def remember_me! super if ::Gitlab::Database.read_write? end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 55f43cd9f7b..3f17729be05 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -234,7 +234,6 @@ class ProjectPolicy < BasePolicy rule { can?(:guest_access) }.policy do enable :read_project - enable :create_merge_request_in enable :read_issue_board enable :read_issue_board_list enable :read_wiki @@ -487,7 +486,7 @@ class ProjectPolicy < BasePolicy prevent(*create_read_update_admin_destroy(:issue_board_list)) end - rule { merge_requests_disabled | repository_disabled }.policy do + rule { merge_requests_disabled | repository_disabled | ~can?(:download_code) }.policy do prevent :create_merge_request_in prevent :create_merge_request_from prevent(*create_read_update_admin_destroy(:merge_request)) @@ -589,13 +588,14 @@ class ProjectPolicy < BasePolicy enable :read_cycle_analytics enable :read_pages_content enable :read_analytics - enable :read_ci_cd_analytics enable :read_insights # NOTE: may be overridden by IssuePolicy enable :read_issue end + rule { can?(:public_access) & public_builds }.enable :read_ci_cd_analytics + rule { public_builds }.policy do enable :read_build end @@ -653,6 +653,10 @@ class ProjectPolicy < BasePolicy enable :read_security_configuration end + rule { can?(:guest_access) & can?(:read_commit_status) }.policy do + enable :create_merge_request_in + end + # Design abilities could also be prevented in the issue policy. rule { design_management_disabled }.policy do prevent :read_design diff --git a/config/initializers/rdoc_segfault_patch.rb b/config/initializers/rdoc_segfault_patch.rb new file mode 100644 index 00000000000..2494d7ef421 --- /dev/null +++ b/config/initializers/rdoc_segfault_patch.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +# Monkey patch of RDoc to prevent Ruby segfault due to +# stack buffer overflow Ruby bug - +# https://bugs.ruby-lang.org/issues/16376 +# +# Safe to remove once GitLab upgrades to Ruby 3.0 +# or once the fix is backported to 2.7.x and +# GitLab upgrades. +# https://gitlab.com/gitlab-org/gitlab/-/issues/351179 +class RDoc::Markup::ToHtml + def parseable?(_) + false + end +end + +class RDoc::Markup::Verbatim + def ruby? + false + end +end diff --git a/db/fixtures/development/18_abuse_reports.rb b/db/fixtures/development/18_abuse_reports.rb index b06beca35e9..88d2f784852 100644 --- a/db/fixtures/development/18_abuse_reports.rb +++ b/db/fixtures/development/18_abuse_reports.rb @@ -11,7 +11,7 @@ module Db name: FFaker::Name.name, email: FFaker::Internet.email, confirmed_at: DateTime.now, - password: Gitlab::Password.test_default + password: '12345678' ) ::AbuseReport.create(reporter: ::User.take, user: reported_user, message: 'User sends spam') diff --git a/lib/banzai/filter/kroki_filter.rb b/lib/banzai/filter/kroki_filter.rb index 3803302c324..9aa2afce5a8 100644 --- a/lib/banzai/filter/kroki_filter.rb +++ b/lib/banzai/filter/kroki_filter.rb @@ -6,8 +6,10 @@ require "asciidoctor/extensions/asciidoctor_kroki/extension" module Banzai module Filter # HTML that replaces all diagrams supported by Kroki with the corresponding img tags. - # + # If the source content is large then the hidden attribute is added to the img tag. class KrokiFilter < HTML::Pipeline::Filter + MAX_CHARACTER_LIMIT = 2000 + def call return doc unless settings.kroki_enabled @@ -21,7 +23,12 @@ module Banzai diagram_format = "svg" doc.xpath(xpath).each do |node| diagram_type = node.parent['lang'] - img_tag = Nokogiri::HTML::DocumentFragment.parse(%(<img src="#{create_image_src(diagram_type, diagram_format, node.content)}"/>)) + diagram_src = node.content + image_src = create_image_src(diagram_type, diagram_format, diagram_src) + lazy_load = diagram_src.length > MAX_CHARACTER_LIMIT + other_attrs = lazy_load ? "hidden" : "" + + img_tag = Nokogiri::HTML::DocumentFragment.parse(%(<img class="js-render-kroki" src="#{image_src}" #{other_attrs} />)) node.parent.replace(img_tag) end diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index 07f82c98666..bcd9f39d1dc 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/lib/banzai/filter/syntax_highlight_filter.rb @@ -56,7 +56,7 @@ module Banzai retry end - sourcepos_attr = sourcepos ? "data-sourcepos=\"#{sourcepos}\"" : '' + sourcepos_attr = sourcepos ? "data-sourcepos=\"#{escape_once(sourcepos)}\"" : '' highlighted = %(<div class="gl-relative markdown-code-block js-markdown-code"><pre #{sourcepos_attr} class="#{css_classes}" lang="#{language}" diff --git a/lib/banzai/reference_redactor.rb b/lib/banzai/reference_redactor.rb index 81e4fd45966..c19f992078a 100644 --- a/lib/banzai/reference_redactor.rb +++ b/lib/banzai/reference_redactor.rb @@ -65,16 +65,15 @@ module Banzai # def redacted_node_content(node) original_content = node.attr('data-original') - link_reference = node.attr('data-link-reference') + original_content = CGI.escape_html(original_content) if original_content # Build the raw <a> tag just with a link as href and content if # it's originally a link pattern. We shouldn't return a plain text href. original_link = - if link_reference == 'true' + if node.attr('data-link-reference') == 'true' href = node.attr('href') - content = original_content - %(<a href="#{href}">#{content}</a>) + %(<a href="#{href}">#{original_content}</a>) end # The reference should be replaced by the original link's content, diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb index 9f142727ebb..feb5fea4c85 100644 --- a/lib/gitlab/auth/o_auth/user.rb +++ b/lib/gitlab/auth/o_auth/user.rb @@ -230,8 +230,8 @@ module Gitlab name: name.strip.presence || valid_username, username: valid_username, email: email, - password: Gitlab::Password.test_default(21), - password_confirmation: Gitlab::Password.test_default(21), + password: auth_hash.password, + password_confirmation: auth_hash.password, password_automatically_set: true } end diff --git a/lib/gitlab/ci/config/external/context.rb b/lib/gitlab/ci/config/external/context.rb index 308414af47d..512cfdde474 100644 --- a/lib/gitlab/ci/config/external/context.rb +++ b/lib/gitlab/ci/config/external/context.rb @@ -70,6 +70,16 @@ module Gitlab } end + def mask_variables_from(location) + variables.reduce(location.dup) do |loc, variable| + if variable[:masked] + Gitlab::Ci::MaskSecret.mask!(loc, variable[:value]) + else + loc + end + end + end + protected attr_writer :expandset, :execution_deadline, :logger diff --git a/lib/gitlab/ci/config/external/file/artifact.rb b/lib/gitlab/ci/config/external/file/artifact.rb index e6ff33d6f79..4f79e64ca9a 100644 --- a/lib/gitlab/ci/config/external/file/artifact.rb +++ b/lib/gitlab/ci/config/external/file/artifact.rb @@ -37,7 +37,7 @@ module Gitlab def validate_content! return unless ensure_preconditions_satisfied! - errors.push("File `#{location}` is empty!") unless content.present? + errors.push("File `#{masked_location}` is empty!") unless content.present? end def ensure_preconditions_satisfied! diff --git a/lib/gitlab/ci/config/external/file/base.rb b/lib/gitlab/ci/config/external/file/base.rb index 7d3fddd850d..a660dd339d8 100644 --- a/lib/gitlab/ci/config/external/file/base.rb +++ b/lib/gitlab/ci/config/external/file/base.rb @@ -79,21 +79,21 @@ module Gitlab def validate_location! if invalid_location_type? - errors.push("Included file `#{location}` needs to be a string") + errors.push("Included file `#{masked_location}` needs to be a string") elsif invalid_extension? - errors.push("Included file `#{location}` does not have YAML extension!") + errors.push("Included file `#{masked_location}` does not have YAML extension!") end end def validate_content! if content.blank? - errors.push("Included file `#{location}` is empty or does not exist!") + errors.push("Included file `#{masked_location}` is empty or does not exist!") end end def validate_hash! if to_hash.blank? - errors.push("Included file `#{location}` does not have valid YAML syntax!") + errors.push("Included file `#{masked_location}` does not have valid YAML syntax!") end end @@ -104,6 +104,12 @@ module Gitlab def expand_context_attrs {} end + + def masked_location + strong_memoize(:masked_location) do + context.mask_variables_from(location) + end + end end end end diff --git a/lib/gitlab/ci/config/external/file/local.rb b/lib/gitlab/ci/config/external/file/local.rb index fdb3e1b00f9..12dbaadb654 100644 --- a/lib/gitlab/ci/config/external/file/local.rb +++ b/lib/gitlab/ci/config/external/file/local.rb @@ -23,11 +23,11 @@ module Gitlab def validate_content! if context.project&.repository.nil? - errors.push("Local file `#{location}` does not have project!") + errors.push("Local file `#{masked_location}` does not have project!") elsif content.nil? - errors.push("Local file `#{location}` does not exist!") + errors.push("Local file `#{masked_location}` does not exist!") elsif content.blank? - errors.push("Local file `#{location}` is empty!") + errors.push("Local file `#{masked_location}` is empty!") end end diff --git a/lib/gitlab/ci/config/external/file/project.rb b/lib/gitlab/ci/config/external/file/project.rb index 114d493381c..27e097ba980 100644 --- a/lib/gitlab/ci/config/external/file/project.rb +++ b/lib/gitlab/ci/config/external/file/project.rb @@ -35,9 +35,9 @@ module Gitlab elsif sha.nil? errors.push("Project `#{project_name}` reference `#{ref_name}` does not exist!") elsif content.nil? - errors.push("Project `#{project_name}` file `#{location}` does not exist!") + errors.push("Project `#{project_name}` file `#{masked_location}` does not exist!") elsif content.blank? - errors.push("Project `#{project_name}` file `#{location}` is empty!") + errors.push("Project `#{project_name}` file `#{masked_location}` is empty!") end end diff --git a/lib/gitlab/ci/config/external/file/remote.rb b/lib/gitlab/ci/config/external/file/remote.rb index 4bd8e250d7a..8335a9ef625 100644 --- a/lib/gitlab/ci/config/external/file/remote.rb +++ b/lib/gitlab/ci/config/external/file/remote.rb @@ -24,7 +24,7 @@ module Gitlab super unless ::Gitlab::UrlSanitizer.valid?(location) - errors.push("Remote file `#{location}` does not have a valid address!") + errors.push("Remote file `#{masked_location}` does not have a valid address!") end end @@ -32,17 +32,17 @@ module Gitlab begin response = Gitlab::HTTP.get(location) rescue SocketError - errors.push("Remote file `#{location}` could not be fetched because of a socket error!") + errors.push("Remote file `#{masked_location}` could not be fetched because of a socket error!") rescue Timeout::Error - errors.push("Remote file `#{location}` could not be fetched because of a timeout error!") + errors.push("Remote file `#{masked_location}` could not be fetched because of a timeout error!") rescue Gitlab::HTTP::Error - errors.push("Remote file `#{location}` could not be fetched because of HTTP error!") + errors.push("Remote file `#{masked_location}` could not be fetched because of HTTP error!") rescue Gitlab::HTTP::BlockedUrlError => e errors.push("Remote file could not be fetched because #{e}!") end if response&.code.to_i >= 400 - errors.push("Remote file `#{location}` could not be fetched because of HTTP code `#{response.code}` error!") + errors.push("Remote file `#{masked_location}` could not be fetched because of HTTP code `#{response.code}` error!") end response.body if errors.none? diff --git a/lib/gitlab/ci/config/external/file/template.rb b/lib/gitlab/ci/config/external/file/template.rb index 47441fa3818..c3d120dfdce 100644 --- a/lib/gitlab/ci/config/external/file/template.rb +++ b/lib/gitlab/ci/config/external/file/template.rb @@ -26,7 +26,7 @@ module Gitlab super unless template_name_valid? - errors.push("Template file `#{location}` is not a valid location!") + errors.push("Template file `#{masked_location}` is not a valid location!") end end diff --git a/lib/gitlab/ci/config/external/mapper.rb b/lib/gitlab/ci/config/external/mapper.rb index a5bf066c81f..e52f0f16666 100644 --- a/lib/gitlab/ci/config/external/mapper.rb +++ b/lib/gitlab/ci/config/external/mapper.rb @@ -146,7 +146,7 @@ module Gitlab file_class.new(location, context) end.select(&:matching?) - raise AmbigiousSpecificationError, "Include `#{location.to_json}` needs to match exactly one accessor!" unless matching.one? + raise AmbigiousSpecificationError, "Include `#{masked_location(location.to_json)}` needs to match exactly one accessor!" unless matching.one? matching.first end @@ -181,6 +181,10 @@ module Gitlab def expand(data) ExpandVariables.expand(data, -> { context.variables_hash }) end + + def masked_location(location) + context.mask_variables_from(location) + end end end end diff --git a/lib/gitlab/error_tracking.rb b/lib/gitlab/error_tracking.rb index 6a637306225..fd0c407543d 100644 --- a/lib/gitlab/error_tracking.rb +++ b/lib/gitlab/error_tracking.rb @@ -19,7 +19,8 @@ module Gitlab PROCESSORS = [ ::Gitlab::ErrorTracking::Processor::SidekiqProcessor, ::Gitlab::ErrorTracking::Processor::GrpcErrorProcessor, - ::Gitlab::ErrorTracking::Processor::ContextPayloadProcessor + ::Gitlab::ErrorTracking::Processor::ContextPayloadProcessor, + ::Gitlab::ErrorTracking::Processor::SanitizeErrorMessageProcessor ].freeze class << self diff --git a/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb b/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb new file mode 100644 index 00000000000..c2345395f5f --- /dev/null +++ b/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Gitlab + module ErrorTracking + module Processor + module Concerns + module ProcessesExceptions + private + + def extract_exceptions_from(event) + exceptions = event.instance_variable_get(:@interfaces)[:exception]&.values + + Array.wrap(exceptions) + end + + def valid_exception?(exception) + case exception + when Raven::SingleExceptionInterface + exception&.value.present? + else + false + end + end + end + end + end + end +end diff --git a/lib/gitlab/error_tracking/processor/grpc_error_processor.rb b/lib/gitlab/error_tracking/processor/grpc_error_processor.rb index e835deeea2c..3064d18bb93 100644 --- a/lib/gitlab/error_tracking/processor/grpc_error_processor.rb +++ b/lib/gitlab/error_tracking/processor/grpc_error_processor.rb @@ -4,6 +4,8 @@ module Gitlab module ErrorTracking module Processor module GrpcErrorProcessor + extend Gitlab::ErrorTracking::Processor::Concerns::ProcessesExceptions + DEBUG_ERROR_STRING_REGEX = RE2('(.*) debug_error_string:(.*)') class << self @@ -18,10 +20,7 @@ module Gitlab # only the first one since that's what is used for grouping. def process_first_exception_value(event) # Better in new version, will be event.exception.values - exceptions = event.instance_variable_get(:@interfaces)[:exception]&.values - - return unless exceptions.is_a?(Array) - + exceptions = extract_exceptions_from(event) exception = exceptions.first return unless valid_exception?(exception) @@ -68,15 +67,6 @@ module Gitlab [match[1], match[2]] end - - def valid_exception?(exception) - case exception - when Raven::SingleExceptionInterface - exception&.value - else - false - end - end end end end diff --git a/lib/gitlab/error_tracking/processor/sanitize_error_message_processor.rb b/lib/gitlab/error_tracking/processor/sanitize_error_message_processor.rb new file mode 100644 index 00000000000..d53269aade1 --- /dev/null +++ b/lib/gitlab/error_tracking/processor/sanitize_error_message_processor.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Gitlab + module ErrorTracking + module Processor + module SanitizeErrorMessageProcessor + extend Gitlab::ErrorTracking::Processor::Concerns::ProcessesExceptions + + class << self + def call(event) + exceptions = extract_exceptions_from(event) + + exceptions.each do |exception| + next unless valid_exception?(exception) + + exception.value = Gitlab::Sanitizers::ExceptionMessage.clean(exception.type, exception.value) + end + + event + end + end + end + end + end +end diff --git a/lib/gitlab/exception_log_formatter.rb b/lib/gitlab/exception_log_formatter.rb index 315574fed31..ce802b562f0 100644 --- a/lib/gitlab/exception_log_formatter.rb +++ b/lib/gitlab/exception_log_formatter.rb @@ -10,7 +10,7 @@ module Gitlab # Use periods to flatten the fields. payload.merge!( 'exception.class' => exception.class.name, - 'exception.message' => exception.message + 'exception.message' => sanitize_message(exception) ) if exception.backtrace @@ -38,6 +38,10 @@ module Gitlab rescue PgQuery::ParseError sql end + + def sanitize_message(exception) + Gitlab::Sanitizers::ExceptionMessage.clean(exception.class.name, exception.message) + end end end end diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb index dd7ec361dd8..d3b1bb6a57d 100644 --- a/lib/gitlab/import_export/members_mapper.rb +++ b/lib/gitlab/import_export/members_mapper.rb @@ -19,9 +19,8 @@ module Gitlab @exported_members.inject(missing_keys_tracking_hash) do |hash, member| if member['user'] old_user_id = member['user']['id'] - old_user_email = member.dig('user', 'public_email') || member.dig('user', 'email') - existing_user = User.find_by(find_user_query(old_user_email)) if old_user_email - hash[old_user_id] = existing_user.id if existing_user && add_team_member(member, existing_user) + existing_user_id = existing_users_email_map[get_email(member)] + hash[old_user_id] = existing_user_id if existing_user_id && add_team_member(member, existing_user_id) else add_team_member(member) end @@ -72,11 +71,45 @@ module Gitlab member&.user == @user && member.access_level >= highest_access_level end - def add_team_member(member, existing_user = nil) - return true if existing_user && @importable.members.exists?(user_id: existing_user.id) + # Returns {email => user_id} hash where user_id is an ID at current instance + def existing_users_email_map + @existing_users_email_map ||= begin + emails = @exported_members.map { |member| get_email(member) } + + User.by_user_email(emails).pluck(:email, :id).to_h + end + end + + # Returns {user_id => email} hash where user_id is an ID at source "old" instance + def exported_members_email_map + @exported_members_email_map ||= begin + result = {} + @exported_members.each do |member| + email = get_email(member) + + next unless email + + result[member.dig('user', 'id')] = email + end + + result + end + end + + def get_email(member_data) + return unless member_data['user'] + + member_data.dig('user', 'public_email') || member_data.dig('user', 'email') + end + + def add_team_member(member, existing_user_id = nil) + return true if existing_user_id && @importable.members.exists?(user_id: existing_user_id) - member['user'] = existing_user member_hash = member_hash(member) + if existing_user_id + member_hash.delete('user') + member_hash['user_id'] = existing_user_id + end member = relation_class.create(member_hash) @@ -92,11 +125,19 @@ module Gitlab end def member_hash(member) - parsed_hash(member).merge( + result = parsed_hash(member).merge( 'source_id' => @importable.id, 'importing' => true, 'access_level' => [member['access_level'], highest_access_level].min ).except('user_id') + + if result['created_by_id'] + created_by_email = exported_members_email_map[result['created_by_id']] + + result['created_by_id'] = existing_users_email_map[created_by_email] + end + + result end def parsed_hash(member) @@ -104,14 +145,6 @@ module Gitlab relation_class: relation_class) end - def find_user_query(email) - user_arel[:email].eq(email) - end - - def user_arel - @user_arel ||= User.arel_table - end - def relation_class case @importable when ::Project @@ -143,7 +176,7 @@ module Gitlab def base_log_params(member_hash) { - user_id: member_hash['user']&.id, + user_id: member_hash['user_id'], access_level: member_hash['access_level'], importable_type: @importable.class.to_s, importable_id: @importable.id, diff --git a/lib/gitlab/password.rb b/lib/gitlab/password.rb deleted file mode 100644 index 00aef8754d6..00000000000 --- a/lib/gitlab/password.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -# This module is used to return fake strong password for tests - -module Gitlab - module Password - DEFAULT_LENGTH = 12 - TEST_DEFAULT = "123qweQWE!@#" + "0" * (User.password_length.max - DEFAULT_LENGTH) - def self.test_default(length = 12) - password_length = [[User.password_length.min, length].max, User.password_length.max].min - TEST_DEFAULT[...password_length] - end - end -end diff --git a/lib/gitlab/sanitizers/exception_message.rb b/lib/gitlab/sanitizers/exception_message.rb new file mode 100644 index 00000000000..11c91093d88 --- /dev/null +++ b/lib/gitlab/sanitizers/exception_message.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Gitlab + module Sanitizers + module ExceptionMessage + FILTERED_STRING = '[FILTERED]' + EXCEPTION_NAMES = %w(URI::InvalidURIError Addressable::URI::InvalidURIError).freeze + MESSAGE_REGEX = %r{(\A[^:]+:\s).*\Z}.freeze + + class << self + def clean(exception_name, message) + return message unless exception_name.in?(EXCEPTION_NAMES) + + message.sub(MESSAGE_REGEX, '\1' + FILTERED_STRING) + end + end + end + end +end diff --git a/lib/tasks/gitlab/seed/group_seed.rake b/lib/tasks/gitlab/seed/group_seed.rake index 491cf782985..a9a350fb6c3 100644 --- a/lib/tasks/gitlab/seed/group_seed.rake +++ b/lib/tasks/gitlab/seed/group_seed.rake @@ -125,7 +125,7 @@ class GroupSeeder name: FFaker::Name.name, email: FFaker::Internet.email, confirmed_at: DateTime.now, - password: Gitlab::Password.test_default + password: Devise.friendly_token ) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 489fe7e91ee..53519354864 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -12625,6 +12625,9 @@ msgstr "" msgid "Dismissed on pipeline %{pipelineLink} at %{projectLink}" msgstr "" +msgid "Display" +msgstr "" + msgid "Display alerts from all configured monitoring tools." msgstr "" diff --git a/package.json b/package.json index d1630b6a64d..4d91a533ffe 100644 --- a/package.json +++ b/package.json @@ -177,7 +177,7 @@ "sql.js": "^0.4.0", "string-hash": "1.1.3", "style-loader": "^2.0.0", - "swagger-ui-dist": "^3.52.3", + "swagger-ui-dist": "4.8.0", "three": "^0.84.0", "three-orbit-controls": "^82.1.0", "three-stl-loader": "^1.0.4", diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index c52223d4758..c46a12680a2 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -612,8 +612,8 @@ RSpec.describe Admin::UsersController do end context 'when the new password does not match the password confirmation' do - let(:password) { Gitlab::Password.test_default } - let(:password_confirmation) { "not" + Gitlab::Password.test_default } + let(:password) { 'some_password' } + let(:password_confirmation) { 'not_same_as_password' } it 'shows the edit page again' do update_password(user, password, password_confirmation) diff --git a/spec/controllers/projects/merge_requests/creations_controller_spec.rb b/spec/controllers/projects/merge_requests/creations_controller_spec.rb index df2023b7356..328cbcd0c83 100644 --- a/spec/controllers/projects/merge_requests/creations_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/creations_controller_spec.rb @@ -186,6 +186,7 @@ RSpec.describe Projects::MergeRequests::CreationsController do it 'fetches the commit if a user has access' do expect(Ability).to receive(:allowed?).with(user, :read_project, project) { true } + expect(Ability).to receive(:allowed?).with(user, :create_merge_request_in, project) { true }.at_least(:once) get :branch_to, params: { @@ -199,8 +200,25 @@ RSpec.describe Projects::MergeRequests::CreationsController do expect(response).to have_gitlab_http_status(:ok) end + it 'does not load the commit when the user cannot create_merge_request_in' do + expect(Ability).to receive(:allowed?).with(user, :read_project, project) { true } + expect(Ability).to receive(:allowed?).with(user, :create_merge_request_in, project) { false }.at_least(:once) + + get :branch_to, + params: { + namespace_id: fork_project.namespace, + project_id: fork_project, + target_project_id: project.id, + ref: 'master' + } + + expect(assigns(:commit)).to be_nil + expect(response).to have_gitlab_http_status(:ok) + end + it 'does not load the commit when the user cannot read the project' do expect(Ability).to receive(:allowed?).with(user, :read_project, project) { false } + expect(Ability).to receive(:allowed?).with(user, :create_merge_request_in, project) { true }.at_least(:once) get :branch_to, params: { diff --git a/spec/controllers/projects/mirrors_controller_spec.rb b/spec/controllers/projects/mirrors_controller_spec.rb index 7c5d14d3a22..77306c86683 100644 --- a/spec/controllers/projects/mirrors_controller_spec.rb +++ b/spec/controllers/projects/mirrors_controller_spec.rb @@ -177,6 +177,7 @@ RSpec.describe Projects::MirrorsController do INVALID git@example.com:foo/bar.git ssh://git@example.com:foo/bar.git + ssh://127.0.0.1/foo/bar.git ].each do |url| it "returns an error with a 400 response for URL #{url.inspect}" do do_get(project, url) diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index d5fe32ac094..71e824dad35 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -499,7 +499,7 @@ RSpec.describe RegistrationsController do end it 'succeeds if password is confirmed' do - post :destroy, params: { password: Gitlab::Password.test_default } + post :destroy, params: { password: '12345678' } expect_success end @@ -540,7 +540,7 @@ RSpec.describe RegistrationsController do end it 'fails' do - delete :destroy, params: { password: Gitlab::Password.test_default } + delete :destroy, params: { password: '12345678' } expect_failure(s_('Profiles|You must transfer ownership or delete groups you are an owner of before you can delete your account')) end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 5f325717ec5..c23203b31f3 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -5,7 +5,7 @@ FactoryBot.define do email { generate(:email) } name { generate(:name) } username { generate(:username) } - password { Gitlab::Password.test_default } + password { "12345678" } role { 'software_developer' } confirmed_at { Time.now } confirmation_token { nil } @@ -23,6 +23,10 @@ FactoryBot.define do after(:build) { |user, _| user.block! } end + trait :disallowed_password do + password { User::DISALLOWED_PASSWORDS.first } + end + trait :blocked_pending_approval do after(:build) { |user, _| user.block_pending_approval! } end diff --git a/spec/features/markdown/kroki_spec.rb b/spec/features/markdown/kroki_spec.rb new file mode 100644 index 00000000000..f02f5d44244 --- /dev/null +++ b/spec/features/markdown/kroki_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'kroki rendering', :js do + let_it_be(:project) { create(:project, :public) } + + before do + stub_application_setting(kroki_enabled: true, kroki_url: 'http://localhost:8000') + end + + it 'shows kroki image' do + plain_text = 'This text length is ignored. ' * 300 + + description = <<~KROKI + #{plain_text} + ```plantuml + A -> A: T + ``` + KROKI + + issue = create(:issue, project: project, description: description) + + visit project_issue_path(project, issue) + + within('.description') do + expect(page).to have_css('img') + expect(page).not_to have_text 'Warning: Displaying this diagram might cause performance issues on this page.' + end + end + + it 'hides kroki image and shows warning alert when kroki source size is large' do + plantuml_text = 'A -> A: T ' * 300 + + description = <<~KROKI + ```plantuml + #{plantuml_text} + ``` + KROKI + + issue = create(:issue, project: project, description: description) + + visit project_issue_path(project, issue) + + within('.description') do + expect(page).not_to have_css('img') + expect(page).to have_text 'Warning: Displaying this diagram might cause performance issues on this page.' + + click_button 'Display' + + expect(page).to have_css('img') + expect(page).not_to have_text 'Warning: Displaying this diagram might cause performance issues on this page.' + end + end +end diff --git a/spec/features/password_reset_spec.rb b/spec/features/password_reset_spec.rb index 322ccc6a0c0..31b2b2d15aa 100644 --- a/spec/features/password_reset_spec.rb +++ b/spec/features/password_reset_spec.rb @@ -44,8 +44,8 @@ RSpec.describe 'Password reset' do visit(edit_user_password_path(reset_password_token: token)) - fill_in 'New password', with: "new" + Gitlab::Password.test_default - fill_in 'Confirm new password', with: "new" + Gitlab::Password.test_default + fill_in 'New password', with: 'hello1234' + fill_in 'Confirm new password', with: 'hello1234' click_button 'Change your password' diff --git a/spec/features/profile_spec.rb b/spec/features/profile_spec.rb index 34eb07d78f1..36657406303 100644 --- a/spec/features/profile_spec.rb +++ b/spec/features/profile_spec.rb @@ -29,7 +29,7 @@ RSpec.describe 'Profile account page', :js do it 'deletes user', :js, :sidekiq_might_not_need_inline do click_button 'Delete account' - fill_in 'password', with: Gitlab::Password.test_default + fill_in 'password', with: '12345678' page.within '.modal' do click_button 'Delete account' diff --git a/spec/features/profiles/password_spec.rb b/spec/features/profiles/password_spec.rb index 25fe43617fd..7059697354d 100644 --- a/spec/features/profiles/password_spec.rb +++ b/spec/features/profiles/password_spec.rb @@ -39,7 +39,7 @@ RSpec.describe 'Profile > Password' do describe 'User puts the same passwords in the field and in the confirmation' do it 'shows a success message' do - fill_passwords(Gitlab::Password.test_default, Gitlab::Password.test_default) + fill_passwords('mypassword', 'mypassword') page.within('.flash-notice') do expect(page).to have_content('Password was successfully updated. Please sign in again.') @@ -79,7 +79,7 @@ RSpec.describe 'Profile > Password' do end context 'Change password' do - let(:new_password) { "new" + Gitlab::Password.test_default } + let(:new_password) { '22233344' } before do sign_in(user) @@ -170,8 +170,8 @@ RSpec.describe 'Profile > Password' do expect(current_path).to eq new_profile_password_path fill_in :user_password, with: user.password - fill_in :user_new_password, with: Gitlab::Password.test_default - fill_in :user_password_confirmation, with: Gitlab::Password.test_default + fill_in :user_new_password, with: '12345678' + fill_in :user_password_confirmation, with: '12345678' click_button 'Set new password' expect(current_path).to eq new_user_session_path diff --git a/spec/features/users/anonymous_sessions_spec.rb b/spec/features/users/anonymous_sessions_spec.rb index f9b23626397..6b21412ae3d 100644 --- a/spec/features/users/anonymous_sessions_spec.rb +++ b/spec/features/users/anonymous_sessions_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'Session TTLs', :clean_gitlab_redis_shared_state do visit new_user_session_path # The session key only gets created after a post fill_in 'user_login', with: 'non-existant@gitlab.org' - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' expect(page).to have_content('Invalid login or password') diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 2780549eea1..195026ca102 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -49,15 +49,15 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do expect(current_path).to eq edit_user_password_path expect(page).to have_content('Please create a password for your new account.') - fill_in 'user_password', with: Gitlab::Password.test_default - fill_in 'user_password_confirmation', with: Gitlab::Password.test_default + fill_in 'user_password', with: 'password' + fill_in 'user_password_confirmation', with: 'password' click_button 'Change your password' expect(current_path).to eq new_user_session_path expect(page).to have_content(I18n.t('devise.passwords.updated_not_active')) fill_in 'user_login', with: user.username - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: 'password' click_button 'Sign in' expect_single_session_with_authenticated_ttl @@ -150,6 +150,27 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do end end + describe 'with a disallowed password' do + let(:user) { create(:user, :disallowed_password) } + + before do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + .and increment(:user_password_invalid_counter) + end + + it 'disallows login' do + gitlab_sign_in(user, password: user.password) + + expect(page).to have_content('Invalid login or password.') + end + + it 'does not update Devise trackable attributes' do + expect { gitlab_sign_in(user, password: user.password) } + .not_to change { User.ghost.reload.sign_in_count } + end + end + describe 'with the ghost user' do it 'disallows login' do expect(authentication_metrics) @@ -210,7 +231,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do end it 'does not allow sign-in if the user password is updated before entering a one-time code' do - user.update!(password: "new" + Gitlab::Password.test_default) + user.update!(password: 'new_password') enter_code(user.current_otp) @@ -447,7 +468,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do visit new_user_session_path fill_in 'user_login', with: user.email - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' expect(current_path).to eq(new_profile_password_path) @@ -456,7 +477,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do end context 'with invalid username and password' do - let(:user) { create(:user, password: "not" + Gitlab::Password.test_default) } + let(:user) { create(:user, password: 'not-the-default') } it 'blocks invalid login' do expect(authentication_metrics) @@ -767,7 +788,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do visit new_user_session_path fill_in 'user_login', with: user.email - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' @@ -788,7 +809,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do visit new_user_session_path fill_in 'user_login', with: user.email - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' @@ -809,7 +830,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do visit new_user_session_path fill_in 'user_login', with: user.email - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' @@ -844,7 +865,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do visit new_user_session_path fill_in 'user_login', with: user.email - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' fill_in 'user_otp_attempt', with: user.reload.current_otp @@ -870,7 +891,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do visit new_user_session_path fill_in 'user_login', with: user.email - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' click_button 'Sign in' expect_to_be_on_terms_page @@ -878,7 +899,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do expect(current_path).to eq(new_profile_password_path) - fill_in 'user_password', with: Gitlab::Password.test_default + fill_in 'user_password', with: '12345678' fill_in 'user_new_password', with: 'new password' fill_in 'user_password_confirmation', with: 'new password' click_button 'Set new password' diff --git a/spec/frontend/behaviors/components/diagram_performance_warning_spec.js b/spec/frontend/behaviors/components/diagram_performance_warning_spec.js new file mode 100644 index 00000000000..c58c2bc55a9 --- /dev/null +++ b/spec/frontend/behaviors/components/diagram_performance_warning_spec.js @@ -0,0 +1,40 @@ +import { GlAlert } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import DiagramPerformanceWarning from '~/behaviors/components/diagram_performance_warning.vue'; + +describe('DiagramPerformanceWarning component', () => { + let wrapper; + + const findAlert = () => wrapper.findComponent(GlAlert); + + beforeEach(() => { + wrapper = shallowMount(DiagramPerformanceWarning); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('renders warning alert with button', () => { + expect(findAlert().props()).toMatchObject({ + primaryButtonText: DiagramPerformanceWarning.i18n.buttonText, + variant: 'warning', + }); + }); + + it('renders warning message', () => { + expect(findAlert().text()).toBe(DiagramPerformanceWarning.i18n.bodyText); + }); + + it('emits event when closing alert', () => { + findAlert().vm.$emit('dismiss'); + + expect(wrapper.emitted('closeAlert')).toEqual([[]]); + }); + + it('emits event when accepting alert', () => { + findAlert().vm.$emit('primaryAction'); + + expect(wrapper.emitted('showImage')).toEqual([[]]); + }); +}); diff --git a/spec/graphql/resolvers/project_pipeline_statistics_resolver_spec.rb b/spec/graphql/resolvers/project_pipeline_statistics_resolver_spec.rb index ccc861baae5..66a5f0277fb 100644 --- a/spec/graphql/resolvers/project_pipeline_statistics_resolver_spec.rb +++ b/spec/graphql/resolvers/project_pipeline_statistics_resolver_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Resolvers::ProjectPipelineStatisticsResolver do let(:current_user) { reporter } - before_all do + before do project.add_guest(guest) project.add_reporter(reporter) end @@ -20,13 +20,8 @@ RSpec.describe Resolvers::ProjectPipelineStatisticsResolver do expect(described_class).to have_nullable_graphql_type(::Types::Ci::AnalyticsType) end - def resolve_statistics(project, args) - ctx = { current_user: current_user } - resolve(described_class, obj: project, args: args, ctx: ctx) - end - - describe '#resolve' do - it 'returns the pipelines statistics for a given project' do + shared_examples 'returns the pipelines statistics for a given project' do + it do result = resolve_statistics(project, {}) expect(result.keys).to contain_exactly( :week_pipelines_labels, @@ -42,14 +37,67 @@ RSpec.describe Resolvers::ProjectPipelineStatisticsResolver do :pipeline_times_values ) end + end + + shared_examples 'it returns nils' do + it do + result = resolve_statistics(project, {}) + + expect(result).to be_nil + end + end + + def resolve_statistics(project, args) + ctx = { current_user: current_user } + resolve(described_class, obj: project, args: args, ctx: ctx) + end + + describe '#resolve' do + it_behaves_like 'returns the pipelines statistics for a given project' context 'when the user does not have access to the CI/CD analytics data' do let(:current_user) { guest } - it 'returns nil' do - result = resolve_statistics(project, {}) + it_behaves_like 'it returns nils' + end + + context 'when the project is public' do + let_it_be(:project) { create(:project, :public) } + + context 'public pipelines are disabled' do + before do + project.update!(public_builds: false) + end + + context 'user is not a member' do + let(:current_user) { create(:user) } + + it_behaves_like 'it returns nils' + end + + context 'user is a guest' do + let(:current_user) { guest } + + it_behaves_like 'it returns nils' + end + + context 'user is a reporter or above' do + let(:current_user) { reporter } + + it_behaves_like 'returns the pipelines statistics for a given project' + end + end + + context 'public pipelines are enabled' do + before do + project.update!(public_builds: true) + end + + context 'user is not a member' do + let(:current_user) { create(:user) } - expect(result).to be_nil + it_behaves_like 'returns the pipelines statistics for a given project' + end end end end diff --git a/spec/initializers/rdoc_segfault_patch_spec.rb b/spec/initializers/rdoc_segfault_patch_spec.rb new file mode 100644 index 00000000000..f9630295052 --- /dev/null +++ b/spec/initializers/rdoc_segfault_patch_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +RSpec.describe 'RDoc segfault patch fix' do + describe 'RDoc::Markup::ToHtml' do + describe '#parseable?' do + it 'returns false' do + to_html = RDoc::Markup::ToHtml.new( nil) + + expect(to_html.parseable?('"def foo; end"')).to eq(false) + end + end + end + + describe 'RDoc::Markup::Verbatim' do + describe 'ruby?' do + it 'returns false' do + verbatim = RDoc::Markup::Verbatim.new('def foo; end') + verbatim.format = :ruby + + expect(verbatim.ruby?).to eq(false) + end + end + end +end diff --git a/spec/lib/banzai/filter/kroki_filter_spec.rb b/spec/lib/banzai/filter/kroki_filter_spec.rb index 57caba1d4d7..c9594ac702d 100644 --- a/spec/lib/banzai/filter/kroki_filter_spec.rb +++ b/spec/lib/banzai/filter/kroki_filter_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Banzai::Filter::KrokiFilter do stub_application_setting(kroki_enabled: true, kroki_url: "http://localhost:8000") doc = filter("<pre lang='nomnoml'><code>[Pirate|eyeCount: Int|raid();pillage()|\n [beard]--[parrot]\n [beard]-:>[foul mouth]\n]</code></pre>") - expect(doc.to_s).to eq '<img src="http://localhost:8000/nomnoml/svg/eNqLDsgsSixJrUmtTHXOL80rsVLwzCupKUrMTNHQtC7IzMlJTE_V0KzhUlCITkpNLEqJ1dWNLkgsKsoviUUSs7KLTssvzVHIzS8tyYjligUAMhEd0g==">' + expect(doc.to_s).to eq '<img class="js-render-kroki" src="http://localhost:8000/nomnoml/svg/eNqLDsgsSixJrUmtTHXOL80rsVLwzCupKUrMTNHQtC7IzMlJTE_V0KzhUlCITkpNLEqJ1dWNLkgsKsoviUUSs7KLTssvzVHIzS8tyYjligUAMhEd0g==">' end it 'replaces nomnoml pre tag with img tag if both kroki and plantuml are enabled' do @@ -19,7 +19,7 @@ RSpec.describe Banzai::Filter::KrokiFilter do plantuml_url: "http://localhost:8080") doc = filter("<pre lang='nomnoml'><code>[Pirate|eyeCount: Int|raid();pillage()|\n [beard]--[parrot]\n [beard]-:>[foul mouth]\n]</code></pre>") - expect(doc.to_s).to eq '<img src="http://localhost:8000/nomnoml/svg/eNqLDsgsSixJrUmtTHXOL80rsVLwzCupKUrMTNHQtC7IzMlJTE_V0KzhUlCITkpNLEqJ1dWNLkgsKsoviUUSs7KLTssvzVHIzS8tyYjligUAMhEd0g==">' + expect(doc.to_s).to eq '<img class="js-render-kroki" src="http://localhost:8000/nomnoml/svg/eNqLDsgsSixJrUmtTHXOL80rsVLwzCupKUrMTNHQtC7IzMlJTE_V0KzhUlCITkpNLEqJ1dWNLkgsKsoviUUSs7KLTssvzVHIzS8tyYjligUAMhEd0g==">' end it 'does not replace nomnoml pre tag with img tag if kroki is disabled' do @@ -38,4 +38,12 @@ RSpec.describe Banzai::Filter::KrokiFilter do expect(doc.to_s).to eq '<pre lang="plantuml"><code>Bob->Alice : hello</code></pre>' end + + it 'adds hidden attribute when content size is large' do + stub_application_setting(kroki_enabled: true, kroki_url: "http://localhost:8000") + text = '[Pirate|eyeCount: Int|raid();pillage()|\n [beard]--[parrot]\n [beard]-:>[foul mouth]\n]' * 25 + doc = filter("<pre lang='nomnoml'><code>#{text}</code></pre>") + + expect(doc.to_s).to eq '<img class="js-render-kroki" src="http://localhost:8000/nomnoml/svg/eNqLDsgsSixJrUmtTHXOL80rsVLwzCupKUrMTNHQtC7IzMlJTE_V0KyJyVNQiE5KTSxKidXVjS5ILCrKL4lFFrSyi07LL81RyM0vLckAysRGjxo8avCowaMGjxo8avCowaMGU8lgAE7mIdc=" hidden>' + end end diff --git a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb index aee4bd93207..16c958ec10b 100644 --- a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb +++ b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb @@ -132,6 +132,12 @@ RSpec.describe Banzai::Filter::SyntaxHighlightFilter do expect(result.to_html.delete("\n")).to eq('<div class="gl-relative markdown-code-block js-markdown-code"><pre data-sourcepos="1:1-3:3" class="code highlight js-syntax-highlight language-plaintext" lang="plaintext" v-pre="true"><code><span id="LC1" class="line" lang="plaintext">This is a test</span></code></pre><copy-code></copy-code></div>') end + + it "escape sourcepos metadata to prevent XSS" do + result = filter('<pre data-sourcepos=""%22 href="x"></pre><base href=http://unsafe-website.com/><pre x=""><code></code></pre>') + + expect(result.to_html.delete("\n")).to eq('<div class="gl-relative markdown-code-block js-markdown-code"><pre data-sourcepos=\'"%22 href="x"></pre><base href=http://unsafe-website.com/><pre x="\' class="code highlight js-syntax-highlight language-plaintext" lang="plaintext" v-pre="true"><code></code></pre><copy-code></copy-code></div>') + end end context "when Rouge lexing fails" do diff --git a/spec/lib/banzai/reference_redactor_spec.rb b/spec/lib/banzai/reference_redactor_spec.rb index 78cceedd0e5..59410fbce71 100644 --- a/spec/lib/banzai/reference_redactor_spec.rb +++ b/spec/lib/banzai/reference_redactor_spec.rb @@ -35,7 +35,7 @@ RSpec.describe Banzai::ReferenceRedactor do end context 'when data-original attribute provided' do - let(:original_content) { '<code>foo</code>' } + let(:original_content) { '<script>alert(1);</script>' } it 'replaces redacted reference with original content' do doc = Nokogiri::HTML.fragment("<a class='gfm' href='https://www.gitlab.com' data-reference-type='issue' data-original='#{original_content}'>bar</a>") diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 611c70d73a1..32e647688ff 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -87,7 +87,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do end context 'when IP is already banned' do - subject { gl_auth.find_for_git_client('username', Gitlab::Password.test_default, project: nil, ip: 'ip') } + subject { gl_auth.find_for_git_client('username', 'password', project: nil, ip: 'ip') } before do expect_next_instance_of(Gitlab::Auth::IpRateLimiter) do |rate_limiter| @@ -204,16 +204,16 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do end it 'recognizes master passwords' do - user = create(:user, password: Gitlab::Password.test_default) + user = create(:user, password: 'password') - expect(gl_auth.find_for_git_client(user.username, Gitlab::Password.test_default, project: nil, ip: 'ip')).to have_attributes(actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities) + expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: 'ip')).to have_attributes(actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities) end include_examples 'user login operation with unique ip limit' do - let(:user) { create(:user, password: Gitlab::Password.test_default) } + let(:user) { create(:user, password: 'password') } def operation - expect(gl_auth.find_for_git_client(user.username, Gitlab::Password.test_default, project: nil, ip: 'ip')).to have_attributes(actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities) + expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: 'ip')).to have_attributes(actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities) end end @@ -477,7 +477,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do :user, :blocked, username: 'normal_user', - password: Gitlab::Password.test_default + password: 'my-secret' ) expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')) @@ -486,7 +486,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do context 'when 2fa is enabled globally' do let_it_be(:user) do - create(:user, username: 'normal_user', password: Gitlab::Password.test_default, otp_grace_period_started_at: 1.day.ago) + create(:user, username: 'normal_user', password: 'my-secret', otp_grace_period_started_at: 1.day.ago) end before do @@ -510,7 +510,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do context 'when 2fa is enabled personally' do let(:user) do - create(:user, :two_factor, username: 'normal_user', password: Gitlab::Password.test_default, otp_grace_period_started_at: 1.day.ago) + create(:user, :two_factor, username: 'normal_user', password: 'my-secret', otp_grace_period_started_at: 1.day.ago) end it 'fails' do @@ -523,7 +523,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do user = create( :user, username: 'normal_user', - password: Gitlab::Password.test_default + password: 'my-secret' ) expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')) @@ -534,7 +534,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do user = create( :user, username: 'oauth2', - password: Gitlab::Password.test_default + password: 'my-secret' ) expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')) @@ -609,7 +609,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do context 'when deploy token and user have the same username' do let(:username) { 'normal_user' } - let(:user) { create(:user, username: username, password: Gitlab::Password.test_default) } + let(:user) { create(:user, username: username, password: 'my-secret') } let(:deploy_token) { create(:deploy_token, username: username, read_registry: false, projects: [project]) } it 'succeeds for the token' do @@ -622,7 +622,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it 'succeeds for the user' do auth_success = { actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities } - expect(gl_auth.find_for_git_client(username, Gitlab::Password.test_default, project: project, ip: 'ip')) + expect(gl_auth.find_for_git_client(username, 'my-secret', project: project, ip: 'ip')) .to have_attributes(auth_success) end end @@ -816,7 +816,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do end let(:username) { 'John' } # username isn't lowercase, test this - let(:password) { Gitlab::Password.test_default } + let(:password) { 'my-secret' } it "finds user by valid login/password" do expect(gl_auth.find_with_user_password(username, password)).to eql user @@ -941,13 +941,13 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it "does not find user by using ldap as fallback to for authentication" do expect(Gitlab::Auth::Ldap::Authentication).to receive(:login).and_return(nil) - expect(gl_auth.find_with_user_password('ldap_user', Gitlab::Password.test_default)).to be_nil + expect(gl_auth.find_with_user_password('ldap_user', 'password')).to be_nil end it "find new user by using ldap as fallback to for authentication" do expect(Gitlab::Auth::Ldap::Authentication).to receive(:login).and_return(user) - expect(gl_auth.find_with_user_password('ldap_user', Gitlab::Password.test_default)).to eq(user) + expect(gl_auth.find_with_user_password('ldap_user', 'password')).to eq(user) end end diff --git a/spec/lib/gitlab/ci/config/external/file/artifact_spec.rb b/spec/lib/gitlab/ci/config/external/file/artifact_spec.rb index 8dd92c5b5fd..b59fc95a8cc 100644 --- a/spec/lib/gitlab/ci/config/external/file/artifact_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/artifact_spec.rb @@ -127,6 +127,12 @@ RSpec.describe Gitlab::Ci::Config::External::File::Artifact do let!(:metadata) { create(:ci_job_artifact, :metadata, job: generator_job) } context 'when file is empty' do + let(:params) { { artifact: 'secret_stuff/generated.yml', job: 'generator' } } + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret_stuff', 'masked' => true }]) } + let(:context) do + Gitlab::Ci::Config::External::Context.new(parent_pipeline: parent_pipeline, variables: variables) + end + before do allow_next_instance_of(Gitlab::Ci::ArtifactFileReader) do |reader| allow(reader).to receive(:read).and_return('') @@ -134,7 +140,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Artifact do end let(:expected_error) do - 'File `generated.yml` is empty!' + 'File `xxxxxxxxxxxx/generated.yml` is empty!' end it_behaves_like 'is invalid' diff --git a/spec/lib/gitlab/ci/config/external/file/base_spec.rb b/spec/lib/gitlab/ci/config/external/file/base_spec.rb index 445edb253fd..536f48ecba6 100644 --- a/spec/lib/gitlab/ci/config/external/file/base_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/base_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Config::External::File::Base do - let(:context_params) { { sha: 'HEAD' } } + let(:variables) { } + let(:context_params) { { sha: 'HEAD', variables: variables } } let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:test_class) do @@ -76,7 +77,8 @@ RSpec.describe Gitlab::Ci::Config::External::File::Base do end context 'when there are YAML syntax errors' do - let(:location) { 'some/file/config.yml' } + let(:location) { 'some/file/secret_file_name.yml' } + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret_file_name', 'masked' => true }]) } before do allow_any_instance_of(test_class) @@ -85,7 +87,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Base do it 'is not a valid file' do expect(subject).not_to be_valid - expect(subject.error_message).to match /does not have valid YAML syntax/ + expect(subject.error_message).to eq('Included file `some/file/xxxxxxxxxxxxxxxx.yml` does not have valid YAML syntax!') end end end diff --git a/spec/lib/gitlab/ci/config/external/file/local_spec.rb b/spec/lib/gitlab/ci/config/external/file/local_spec.rb index 3d1fc32a62d..61eb52a615b 100644 --- a/spec/lib/gitlab/ci/config/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Local do let_it_be(:user) { create(:user) } let(:sha) { '12345' } + let(:variables) { project.predefined_variables.to_runner_variables } let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:params) { { local: location } } let(:local_file) { described_class.new(params, context) } @@ -18,7 +19,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Local do sha: sha, user: user, parent_pipeline: parent_pipeline, - variables: project.predefined_variables.to_runner_variables + variables: variables } end @@ -66,7 +67,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Local do end end - context 'when is not a valid local path' do + context 'when it is not a valid local path' do let(:location) { '/lib/gitlab/ci/templates/non-existent-file.yml' } it 'returns false' do @@ -74,13 +75,23 @@ RSpec.describe Gitlab::Ci::Config::External::File::Local do end end - context 'when is not a yaml file' do + context 'when it is not a yaml file' do let(:location) { '/config/application.rb' } it 'returns false' do expect(local_file.valid?).to be_falsy end end + + context 'when it is an empty file' do + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret', 'masked' => true }]) } + let(:location) { '/lib/gitlab/ci/templates/secret/existent-file.yml' } + + it 'returns false and adds an error message about an empty file' do + allow_any_instance_of(described_class).to receive(:fetch_local_content).and_return("") + expect(local_file.errors).to include("Local file `/lib/gitlab/ci/templates/xxxxxx/existent-file.yml` is empty!") + end + end end describe '#content' do @@ -116,10 +127,11 @@ RSpec.describe Gitlab::Ci::Config::External::File::Local do end describe '#error_message' do - let(:location) { '/lib/gitlab/ci/templates/non-existent-file.yml' } + let(:location) { '/lib/gitlab/ci/templates/secret_file.yml' } + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret_file', 'masked' => true }]) } it 'returns an error message' do - expect(local_file.error_message).to eq("Local file `#{location}` does not exist!") + expect(local_file.error_message).to eq("Local file `/lib/gitlab/ci/templates/xxxxxxxxxxx.yml` does not exist!") end end diff --git a/spec/lib/gitlab/ci/config/external/file/project_spec.rb b/spec/lib/gitlab/ci/config/external/file/project_spec.rb index c53914c5772..74720c0a3ca 100644 --- a/spec/lib/gitlab/ci/config/external/file/project_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/project_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Project do let(:parent_pipeline) { double(:parent_pipeline) } let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:project_file) { described_class.new(params, context) } + let(:variables) { project.predefined_variables.to_runner_variables } let(:context_params) do { @@ -18,7 +19,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Project do sha: '12345', user: context_user, parent_pipeline: parent_pipeline, - variables: project.predefined_variables.to_runner_variables + variables: variables } end @@ -108,18 +109,19 @@ RSpec.describe Gitlab::Ci::Config::External::File::Project do context 'when an empty file is used' do let(:params) do - { project: project.full_path, file: '/file.yml' } + { project: project.full_path, file: '/secret_file.yml' } end + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret_file', 'masked' => true }]) } let(:root_ref_sha) { project.repository.root_ref_sha } before do - stub_project_blob(root_ref_sha, '/file.yml') { '' } + stub_project_blob(root_ref_sha, '/secret_file.yml') { '' } end it 'returns false' do expect(project_file).not_to be_valid - expect(project_file.error_message).to include("Project `#{project.full_path}` file `/file.yml` is empty!") + expect(project_file.error_message).to include("Project `#{project.full_path}` file `/xxxxxxxxxxx.yml` is empty!") end end @@ -135,13 +137,15 @@ RSpec.describe Gitlab::Ci::Config::External::File::Project do end context 'when non-existing file is requested' do + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret-invalid-file', 'masked' => true }]) } + let(:params) do - { project: project.full_path, file: '/invalid-file.yml' } + { project: project.full_path, file: '/secret-invalid-file.yml' } end it 'returns false' do expect(project_file).not_to be_valid - expect(project_file.error_message).to include("Project `#{project.full_path}` file `/invalid-file.yml` does not exist!") + expect(project_file.error_message).to include("Project `#{project.full_path}` file `/xxxxxxxxxxxxxxxxxxx.yml` does not exist!") end end diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb index ab60ac215ba..2613bfbfdcf 100644 --- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -5,11 +5,12 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Config::External::File::Remote do include StubRequests - let(:context_params) { { sha: '12345' } } + let(:variables) {Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret_file', 'masked' => true }]) } + let(:context_params) { { sha: '12345', variables: variables } } let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:params) { { remote: location } } let(:remote_file) { described_class.new(params, context) } - let(:location) { 'https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.gitlab-ci-1.yml' } + let(:location) { 'https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.secret_file.yml' } let(:remote_file_content) do <<~HEREDOC before_script: @@ -144,10 +145,10 @@ RSpec.describe Gitlab::Ci::Config::External::File::Remote do subject { remote_file.error_message } context 'when remote file location is not valid' do - let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.gitlab-ci-1.yml' } + let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-foss/blob/1234/?secret_file.yml' } it 'returns an error message describing invalid address' do - expect(subject).to match /does not have a valid address!/ + expect(subject).to eq('Remote file `not-valid://gitlab.com/gitlab-org/gitlab-foss/blob/1234/?xxxxxxxxxxx.yml` does not have a valid address!') end end @@ -157,7 +158,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Remote do end it 'returns error message about a timeout' do - expect(subject).to match /could not be fetched because of a timeout error!/ + expect(subject).to eq('Remote file `https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.xxxxxxxxxxx.yml` could not be fetched because of a timeout error!') end end @@ -167,7 +168,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Remote do end it 'returns error message about a HTTP error' do - expect(subject).to match /could not be fetched because of HTTP error!/ + expect(subject).to eq('Remote file `https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.xxxxxxxxxxx.yml` could not be fetched because of HTTP error!') end end @@ -177,7 +178,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Remote do end it 'returns error message about a timeout' do - expect(subject).to match /could not be fetched because of HTTP code `404` error!/ + expect(subject).to eq('Remote file `https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.xxxxxxxxxxx.yml` could not be fetched because of HTTP code `404` error!') end end diff --git a/spec/lib/gitlab/ci/config/external/file/template_spec.rb b/spec/lib/gitlab/ci/config/external/file/template_spec.rb index 75b22c1516c..66a06de3d28 100644 --- a/spec/lib/gitlab/ci/config/external/file/template_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/template_spec.rb @@ -54,11 +54,13 @@ RSpec.describe Gitlab::Ci::Config::External::File::Template do end context 'with invalid template name' do - let(:template) { 'Template.yml' } + let(:template) { 'SecretTemplate.yml' } + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'SecretTemplate', 'masked' => true }]) } + let(:context_params) { { project: project, sha: '12345', user: user, variables: variables } } it 'returns false' do expect(template_file).not_to be_valid - expect(template_file.error_message).to include('Template file `Template.yml` is not a valid location!') + expect(template_file.error_message).to include('`xxxxxxxxxxxxxx.yml` is not a valid location!') end end diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb index cebe8984741..83e4d058492 100644 --- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -11,7 +11,8 @@ RSpec.describe Gitlab::Ci::Config::External::Mapper do let(:local_file) { '/lib/gitlab/ci/templates/non-existent-file.yml' } let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-foss/blob/1234/.gitlab-ci-1.yml' } let(:template_file) { 'Auto-DevOps.gitlab-ci.yml' } - let(:context_params) { { project: project, sha: '123456', user: user, variables: project.predefined_variables } } + let(:variables) { project.predefined_variables } + let(:context_params) { { project: project, sha: '123456', user: user, variables: variables } } let(:context) { Gitlab::Ci::Config::External::Context.new(**context_params) } let(:file_content) do @@ -92,13 +93,16 @@ RSpec.describe Gitlab::Ci::Config::External::Mapper do end context 'when the key is a hash of file and remote' do + let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'GITLAB_TOKEN', 'value' => 'secret-file', 'masked' => true }]) } + let(:local_file) { 'secret-file.yml' } + let(:remote_url) { 'https://gitlab.com/secret-file.yml' } let(:values) do { include: { 'local' => local_file, 'remote' => remote_url }, image: 'ruby:2.7' } end it 'returns ambigious specification error' do - expect { subject }.to raise_error(described_class::AmbigiousSpecificationError) + expect { subject }.to raise_error(described_class::AmbigiousSpecificationError, 'Include `{"local":"xxxxxxxxxxx.yml","remote":"https://gitlab.com/xxxxxxxxxxx.yml"}` needs to match exactly one accessor!') end end diff --git a/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb new file mode 100644 index 00000000000..b4173617a14 --- /dev/null +++ b/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::ErrorTracking::Processor::SanitizeErrorMessageProcessor, :sentry do + describe '.call' do + let(:exception) { StandardError.new('raw error') } + let(:event) { Raven::Event.from_exception(exception, raven_required_options) } + let(:result_hash) { described_class.call(event).to_hash } + let(:raven_required_options) do + { + configuration: Raven.configuration, + context: Raven.context, + breadcrumbs: Raven.breadcrumbs + } + end + + it 'cleans the exception message' do + expect(Gitlab::Sanitizers::ExceptionMessage).to receive(:clean).with('StandardError', 'raw error').and_return('cleaned') + + expect(result_hash[:exception][:values].first).to include( + type: 'StandardError', + value: 'cleaned' + ) + end + + context 'when event is invalid' do + let(:event) { instance_double('Raven::Event', to_hash: { invalid: true }) } + + it 'does nothing' do + extracted_exception = instance_double('Raven::SingleExceptionInterface', value: nil) + allow(described_class).to receive(:extract_exceptions_from).and_return([extracted_exception]) + + expect(Gitlab::Sanitizers::ExceptionMessage).not_to receive(:clean) + expect(result_hash).to eq(invalid: true) + end + end + end +end diff --git a/spec/lib/gitlab/error_tracking_spec.rb b/spec/lib/gitlab/error_tracking_spec.rb index a5d44963f4b..284b773ef24 100644 --- a/spec/lib/gitlab/error_tracking_spec.rb +++ b/spec/lib/gitlab/error_tracking_spec.rb @@ -301,5 +301,48 @@ RSpec.describe Gitlab::ErrorTracking do end end end + + context 'when processing invalid URI exceptions' do + let(:invalid_uri) { 'http://foo:bar' } + let(:sentry_exception_values) { sentry_event['exception']['values'] } + + context 'when the error is a URI::InvalidURIError' do + let(:exception) do + URI.parse(invalid_uri) + rescue URI::InvalidURIError => error + error + end + + it 'filters the URI from the error message' do + track_exception + + expect(sentry_exception_values).to include( + hash_including( + 'type' => 'URI::InvalidURIError', + 'value' => 'bad URI(is not URI?): [FILTERED]' + ) + ) + end + end + + context 'when the error is a Addressable::URI::InvalidURIError' do + let(:exception) do + Addressable::URI.parse(invalid_uri) + rescue Addressable::URI::InvalidURIError => error + error + end + + it 'filters the URI from the error message' do + track_exception + + expect(sentry_exception_values).to include( + hash_including( + 'type' => 'Addressable::URI::InvalidURIError', + 'value' => 'Invalid port number: [FILTERED]' + ) + ) + end + end + end end end diff --git a/spec/lib/gitlab/exception_log_formatter_spec.rb b/spec/lib/gitlab/exception_log_formatter_spec.rb index beeeeb2b64c..7dda56f0bf5 100644 --- a/spec/lib/gitlab/exception_log_formatter_spec.rb +++ b/spec/lib/gitlab/exception_log_formatter_spec.rb @@ -22,6 +22,14 @@ RSpec.describe Gitlab::ExceptionLogFormatter do expect(payload['exception.sql']).to be_nil end + it 'cleans the exception message' do + expect(Gitlab::Sanitizers::ExceptionMessage).to receive(:clean).with('RuntimeError', 'bad request').and_return('cleaned') + + described_class.format!(exception, payload) + + expect(payload['exception.message']).to eq('cleaned') + end + context 'when exception is ActiveRecord::StatementInvalid' do let(:exception) { ActiveRecord::StatementInvalid.new(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = 1 AND "users"."foo" = $1') } diff --git a/spec/lib/gitlab/import_export/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb index 8b9ca90a280..1085cde767d 100644 --- a/spec/lib/gitlab/import_export/members_mapper_spec.rb +++ b/spec/lib/gitlab/import_export/members_mapper_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do "notification_level" => 3, "created_at" => "2016-03-11T10:21:44.822Z", "updated_at" => "2016-03-11T10:21:44.822Z", - "created_by_id" => nil, + "created_by_id" => 1, "invite_email" => nil, "invite_token" => nil, "invite_accepted_at" => nil, @@ -38,10 +38,24 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do "notification_level" => 3, "created_at" => "2016-03-11T10:21:44.822Z", "updated_at" => "2016-03-11T10:21:44.822Z", - "created_by_id" => 1, + "created_by_id" => 2, "invite_email" => 'invite@test.com', "invite_token" => 'token', "invite_accepted_at" => nil + }, + { + "id" => 3, + "access_level" => 40, + "source_id" => 14, + "source_type" => source_type, + "user_id" => nil, + "notification_level" => 3, + "created_at" => "2016-03-11T10:21:44.822Z", + "updated_at" => "2016-03-11T10:21:44.822Z", + "created_by_id" => nil, + "invite_email" => 'invite2@test.com', + "invite_token" => 'token', + "invite_accepted_at" => nil }] end @@ -68,12 +82,37 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do expect(member_class.find_by_invite_email('invite@test.com')).not_to be_nil end - it 'removes old user_id from member_hash to avoid conflict with user key' do + it 'maps created_by_id to user on new instance' do + expect(member_class) + .to receive(:create) + .once + .with(hash_including('user_id' => user2.id, 'created_by_id' => nil)) + .and_call_original + expect(member_class) + .to receive(:create) + .once + .with(hash_including('invite_email' => 'invite@test.com', 'created_by_id' => nil)) + .and_call_original + expect(member_class) + .to receive(:create) + .once + .with(hash_including('invite_email' => 'invite2@test.com', 'created_by_id' => nil)) + .and_call_original + + members_mapper.map + end + + it 'replaced user_id with user_id from new instance' do + expect(member_class) + .to receive(:create) + .once + .with(hash_including('user_id' => user2.id)) + .and_call_original expect(member_class) .to receive(:create) - .twice - .with(hash_excluding('user_id')) - .and_call_original + .twice + .with(hash_excluding('user_id')) + .and_call_original members_mapper.map end @@ -99,7 +138,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do end expect(logger).to receive(:info).with(hash_including(expected_log_params.call(user2.id))).once - expect(logger).to receive(:info).with(hash_including(expected_log_params.call(nil))).once + expect(logger).to receive(:info).with(hash_including(expected_log_params.call(nil))).twice members_mapper.map end diff --git a/spec/lib/gitlab/sanitizers/exception_message_spec.rb b/spec/lib/gitlab/sanitizers/exception_message_spec.rb new file mode 100644 index 00000000000..8b54b353235 --- /dev/null +++ b/spec/lib/gitlab/sanitizers/exception_message_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rspec-parameterized' + +RSpec.describe Gitlab::Sanitizers::ExceptionMessage do + describe '.clean' do + let(:exception_name) { exception.class.name } + let(:exception_message) { exception.message } + + subject { described_class.clean(exception_name, exception_message) } + + context 'when error is a URI::InvalidURIError' do + let(:exception) do + URI.parse('http://foo:bar') + rescue URI::InvalidURIError => error + error + end + + it { is_expected.to eq('bad URI(is not URI?): [FILTERED]') } + end + + context 'when error is an Addressable::URI::InvalidURIError' do + using RSpec::Parameterized::TableSyntax + + let(:exception) do + Addressable::URI.parse(uri) + rescue Addressable::URI::InvalidURIError => error + error + end + + where(:uri, :result) do + 'http://foo:bar' | 'Invalid port number: [FILTERED]' + 'http://foo:%eb' | 'Invalid encoding in port' + 'ht%0atp://foo' | 'Invalid scheme format: [FILTERED]' + 'http:' | 'Absolute URI missing hierarchical segment: [FILTERED]' + '::http' | 'Cannot assemble URI string with ambiguous path: [FILTERED]' + 'http://foo bar' | 'Invalid character in host: [FILTERED]' + end + + with_them do + it { is_expected.to eq(result) } + end + end + + context 'with any other exception' do + let(:exception) { StandardError.new('Error message: http://foo@bar:baz@ex:ample.com') } + + it 'is not invoked and does nothing' do + is_expected.to eq('Error message: http://foo@bar:baz@ex:ample.com') + end + end + end +end diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb index af77989dbbc..365ca892bb1 100644 --- a/spec/mailers/emails/profile_spec.rb +++ b/spec/mailers/emails/profile_spec.rb @@ -49,7 +49,7 @@ RSpec.describe Emails::Profile do describe 'for users that signed up, the email' do let(:example_site_path) { root_path } - let(:new_user) { create(:user, email: new_user_address, password: Gitlab::Password.test_default) } + let(:new_user) { create(:user, email: new_user_address, password: "securePassword") } subject { Notify.new_user_email(new_user.id) } diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 6830a8daa3b..79f97c606a0 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -29,15 +29,25 @@ RSpec.describe Ci::Runner do context 'when runner is not allowed to pick untagged jobs' do context 'when runner does not have tags' do + let(:runner) { build(:ci_runner, tag_list: [], run_untagged: false) } + + it 'is not valid' do + expect(runner).to be_invalid + end + end + + context 'when runner has too many tags' do + let(:runner) { build(:ci_runner, tag_list: (1..::Ci::Runner::TAG_LIST_MAX_LENGTH + 1).map { |i| "tag#{i}" }, run_untagged: false) } + it 'is not valid' do - runner = build(:ci_runner, tag_list: [], run_untagged: false) expect(runner).to be_invalid end end context 'when runner has tags' do + let(:runner) { build(:ci_runner, tag_list: ['tag'], run_untagged: false) } + it 'is valid' do - runner = build(:ci_runner, tag_list: ['tag'], run_untagged: false) expect(runner).to be_valid end end diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 89bfb742f5d..17cb5da977a 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -37,7 +37,7 @@ RSpec.describe SystemHook do let(:project) { create(:project, namespace: user.namespace) } let(:group) { create(:group) } let(:params) do - { name: 'John Doe', username: 'jduser', email: 'jg@example.com', password: Gitlab::Password.test_default } + { name: 'John Doe', username: 'jduser', email: 'jg@example.com', password: 'mydummypass' } end before do diff --git a/spec/models/integrations/asana_spec.rb b/spec/models/integrations/asana_spec.rb index b6602964182..43e876a4f47 100644 --- a/spec/models/integrations/asana_spec.rb +++ b/spec/models/integrations/asana_spec.rb @@ -20,11 +20,13 @@ RSpec.describe Integrations::Asana do let(:gid) { "123456789ABCD" } let(:asana_task) { double(::Asana::Resources::Task) } let(:asana_integration) { described_class.new } + let(:ref) { 'main' } + let(:restrict_to_branch) { nil } let(:data) do { object_kind: 'push', - ref: 'master', + ref: ref, user_name: user.name, commits: [ { @@ -40,16 +42,44 @@ RSpec.describe Integrations::Asana do project: project, project_id: project.id, api_key: 'verySecret', - restrict_to_branch: 'master' + restrict_to_branch: restrict_to_branch ) end subject(:execute_integration) { asana_integration.execute(data) } + context 'with restrict_to_branch' do + let(:restrict_to_branch) { 'feature-branch, main' } + let(:message) { 'fix #456789' } + + context 'when ref is in scope of restriced branches' do + let(:ref) { 'main' } + + it 'calls the Asana integration' do + expect(asana_task).to receive(:add_comment) + expect(asana_task).to receive(:update).with(completed: true) + expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '456789').once.and_return(asana_task) + + execute_integration + end + end + + context 'when ref is not in scope of restricted branches' do + let(:ref) { 'mai' } + + it 'does not call the Asana integration' do + expect(asana_task).not_to receive(:add_comment) + expect(::Asana::Resources::Task).not_to receive(:find_by_id) + + execute_integration + end + end + end + context 'when creating a story' do let(:message) { "Message from commit. related to ##{gid}" } let(:expected_message) do - "#{user.name} pushed to branch master of #{project.full_name} ( https://gitlab.com/ ): #{message}" + "#{user.name} pushed to branch main of #{project.full_name} ( https://gitlab.com/ ): #{message}" end it 'calls Asana integration to create a story' do diff --git a/spec/models/releases/link_spec.rb b/spec/models/releases/link_spec.rb index 4dc1e53d59e..74ef38f482b 100644 --- a/spec/models/releases/link_spec.rb +++ b/spec/models/releases/link_spec.rb @@ -113,6 +113,17 @@ RSpec.describe Releases::Link do end end + describe 'when filepath is greater than max length' do + let!(:invalid_link) { build(:release_link, filepath: 'x' * (Releases::Link::FILEPATH_MAX_LENGTH + 1), release: release) } + + it 'will not execute regex' do + invalid_link.filepath_format_valid? + + expect(invalid_link.errors[:filepath].size).to eq(1) + expect(invalid_link.errors[:filepath].first).to start_with("is too long") + end + end + describe 'FILEPATH_REGEX with table' do using RSpec::Parameterized::TableSyntax diff --git a/spec/models/ssh_host_key_spec.rb b/spec/models/ssh_host_key_spec.rb index 4d729d5585f..4b756846598 100644 --- a/spec/models/ssh_host_key_spec.rb +++ b/spec/models/ssh_host_key_spec.rb @@ -4,7 +4,9 @@ require 'spec_helper' RSpec.describe SshHostKey do using RSpec::Parameterized::TableSyntax + include ReactiveCachingHelpers + include StubRequests let(:key1) do 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC3UpyF2iLqy1d63M6k3jH1vuEnq/NWtE+o' \ @@ -35,6 +37,7 @@ RSpec.describe SshHostKey do let(:extra) { known_hosts + "foo\nbar\n" } let(:reversed) { known_hosts.lines.reverse.join } + let(:url) { 'ssh://example.com:2222' } let(:compare_host_keys) { nil } def stub_ssh_keyscan(args, status: true, stdout: "", stderr: "") @@ -50,7 +53,7 @@ RSpec.describe SshHostKey do let(:project) { build(:project) } - subject(:ssh_host_key) { described_class.new(project: project, url: 'ssh://example.com:2222', compare_host_keys: compare_host_keys) } + subject(:ssh_host_key) { described_class.new(project: project, url: url, compare_host_keys: compare_host_keys) } describe '.primary_key' do it 'returns a symbol' do @@ -191,5 +194,45 @@ RSpec.describe SshHostKey do is_expected.to eq(error: 'Failed to detect SSH host keys') end end + + context 'DNS rebinding protection enabled' do + before do + stub_application_setting(dns_rebinding_protection_enabled: true) + end + + it 'sends an address as well as hostname to ssh-keyscan' do + stub_dns(url, ip_address: '1.2.3.4') + + stdin = stub_ssh_keyscan(%w[-T 5 -p 2222 -f-]) + + cache + + expect(stdin.string).to eq("1.2.3.4 example.com\n") + end + end + end + + describe 'URL validation' do + let(:url) { 'ssh://127.0.0.1' } + + context 'when local requests are not allowed' do + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) + end + + it 'forbids scanning localhost' do + expect { ssh_host_key }.to raise_error(/Invalid URL/) + end + end + + context 'when local requests are allowed' do + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) + end + + it 'permits scanning localhost' do + expect(ssh_host_key.url.to_s).to eq('ssh://127.0.0.1:22') + end + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c2535fd3698..cd8be088c6c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1696,9 +1696,9 @@ RSpec.describe User do describe '#generate_password' do it 'does not generate password by default' do - user = create(:user, password: Gitlab::Password.test_default) + user = create(:user, password: 'abcdefghe') - expect(user.password).to eq(Gitlab::Password.test_default) + expect(user.password).to eq('abcdefghe') end end @@ -5792,6 +5792,36 @@ RSpec.describe User do end end + describe '#valid_password?' do + subject { user.valid_password?(password) } + + context 'user with password not in disallowed list' do + let(:user) { create(:user) } + let(:password) { user.password } + + it { is_expected.to be_truthy } + + context 'using a wrong password' do + let(:password) { 'WRONG PASSWORD' } + + it { is_expected.to be_falsey } + end + end + + context 'user with disallowed password' do + let(:user) { create(:user, :disallowed_password) } + let(:password) { user.password } + + it { is_expected.to be_falsey } + + context 'using a wrong password' do + let(:password) { 'WRONG PASSWORD' } + + it { is_expected.to be_falsey } + end + end + end + describe '#password_expired?' do let(:user) { build(:user, password_expires_at: password_expires_at) } diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 38e4e18c894..9ab69455ea2 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -81,25 +81,62 @@ RSpec.describe ProjectPolicy do context 'merge requests feature' do let(:current_user) { owner } + let(:mr_permissions) do + [:create_merge_request_from, :read_merge_request, :update_merge_request, + :admin_merge_request, :create_merge_request_in] + end it 'disallows all permissions when the feature is disabled' do project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED) - mr_permissions = [:create_merge_request_from, :read_merge_request, - :update_merge_request, :admin_merge_request, - :create_merge_request_in] - expect_disallowed(*mr_permissions) end + + context 'for a guest in a private project' do + let(:current_user) { guest } + let(:project) { private_project } + + it 'disallows the guest from all merge request permissions' do + expect_disallowed(*mr_permissions) + end + end end - context 'for a guest in a private project' do - let(:current_user) { guest } - let(:project) { private_project } + context 'creating_merge_request_in' do + context 'when project is public' do + let(:project) { public_project } + + context 'when the current_user is guest' do + let(:current_user) { guest } - it 'disallows the guest from reading the merge request and merge request iid' do - expect_disallowed(:read_merge_request) - expect_disallowed(:read_merge_request_iid) + it { is_expected.to be_allowed(:create_merge_request_in) } + end + end + + context 'when project is internal' do + let(:project) { internal_project } + + context 'when the current_user is guest' do + let(:current_user) { guest } + + it { is_expected.to be_allowed(:create_merge_request_in) } + end + end + + context 'when project is private' do + let(:project) { private_project } + + context 'when the current_user is guest' do + let(:current_user) { guest } + + it { is_expected.not_to be_allowed(:create_merge_request_in) } + end + + context 'when the current_user is reporter or above' do + let(:current_user) { reporter } + + it { is_expected.to be_allowed(:create_merge_request_in) } + end end end @@ -1316,6 +1353,110 @@ RSpec.describe ProjectPolicy do end end + describe 'read_ci_cd_analytics' do + context 'public project' do + let(:project) { create(:project, :public, :analytics_enabled) } + let(:current_user) { create(:user) } + + context 'when public pipelines are disabled for the project' do + before do + project.update!(public_builds: false) + end + + context 'project member' do + %w(guest reporter developer maintainer).each do |role| + context role do + before do + project.add_user(current_user, role.to_sym) + end + + if role == 'guest' + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + else + it { is_expected.to be_allowed(:read_ci_cd_analytics) } + end + end + end + end + + context 'non member' do + let(:current_user) { non_member } + + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end + + context 'anonymous' do + let(:current_user) { anonymous } + + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end + end + + context 'when public pipelines are enabled for the project' do + before do + project.update!(public_builds: true) + end + + context 'project member' do + %w(guest reporter developer maintainer).each do |role| + context role do + before do + project.add_user(current_user, role.to_sym) + end + + it { is_expected.to be_allowed(:read_ci_cd_analytics) } + end + end + end + + context 'non member' do + let(:current_user) { non_member } + + it { is_expected.to be_allowed(:read_ci_cd_analytics) } + end + + context 'anonymous' do + let(:current_user) { anonymous } + + it { is_expected.to be_allowed(:read_ci_cd_analytics) } + end + end + end + + context 'private project' do + let(:project) { create(:project, :private, :analytics_enabled) } + let(:current_user) { create(:user) } + + context 'project member' do + %w(guest reporter developer maintainer).each do |role| + context role do + before do + project.add_user(current_user, role.to_sym) + end + + if role == 'guest' + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + else + it { is_expected.to be_allowed(:read_ci_cd_analytics) } + end + end + end + end + + context 'non member' do + let(:current_user) { non_member } + + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end + + context 'anonymous' do + let(:current_user) { anonymous } + + it { is_expected.to be_disallowed(:read_ci_cd_analytics) } + end + end + end + it_behaves_like 'Self-managed Core resource access tokens' describe 'operations feature' do diff --git a/spec/requests/api/ci/runner/runners_post_spec.rb b/spec/requests/api/ci/runner/runners_post_spec.rb index 530b601add9..9174918b737 100644 --- a/spec/requests/api/ci/runner/runners_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_post_spec.rb @@ -116,6 +116,54 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(response).to have_gitlab_http_status(:created) expect(::Ci::Runner.last.ip_address).to eq('123.111.123.111') end + + context 'when tags parameter is provided' do + def request + post api('/runners'), params: { + token: registration_token, + tag_list: tag_list + } + end + + context 'with number of tags above limit' do + let(:tag_list) { (1..::Ci::Runner::TAG_LIST_MAX_LENGTH + 1).map { |i| "tag#{i}" } } + + it 'uses tag_list value in registration and returns error' do + expect_next_instance_of(::Ci::RegisterRunnerService) do |service| + expected_params = { tag_list: tag_list }.stringify_keys + + expect(service).to receive(:execute) + .once + .with(registration_token, a_hash_including(expected_params)) + .and_call_original + end + + request + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response.dig('message', 'tags_list')).to contain_exactly("Too many tags specified. Please limit the number of tags to #{::Ci::Runner::TAG_LIST_MAX_LENGTH}") + end + end + + context 'with number of tags below limit' do + let(:tag_list) { (1..20).map { |i| "tag#{i}" } } + + it 'uses tag_list value in registration and successfully creates runner' do + expect_next_instance_of(::Ci::RegisterRunnerService) do |service| + expected_params = { tag_list: tag_list }.stringify_keys + + expect(service).to receive(:execute) + .once + .with(registration_token, a_hash_including(expected_params)) + .and_call_original + end + + request + + expect(response).to have_gitlab_http_status(:created) + end + end + end end end end diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index 305c0bd9df0..e00a1becf17 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -366,6 +366,19 @@ RSpec.describe API::Ci::Runners do expect(shared_runner.reload.tag_list).to include('ruby2.1', 'pgsql', 'mysql') end + it 'unrelated runner attribute on an existing runner with too many tags' do + # This test ensures that it is possible to update any attribute on a runner that currently fails the + # validation that ensures that there aren't too many tags associated with a runner + existing_invalid_shared_runner = build(:ci_runner, :instance, tag_list: (1..::Ci::Runner::TAG_LIST_MAX_LENGTH + 1).map { |i| "tag#{i}" } ) + existing_invalid_shared_runner.save!(validate: false) + + active = existing_invalid_shared_runner.active + update_runner(existing_invalid_shared_runner.id, admin, active: !active) + + expect(response).to have_gitlab_http_status(:ok) + expect(existing_invalid_shared_runner.reload.active).to eq(!active) + end + it 'runner untagged flag' do # Ensure tag list is non-empty before setting untagged to false. update_runner(shared_runner.id, admin, tag_list: ['ruby2.1', 'pgsql', 'mysql']) diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 98875d7e8d2..0fb0150ecc9 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1027,7 +1027,7 @@ RSpec.describe API::Users do post api('/users', admin), params: { email: 'invalid email', - password: Gitlab::Password.test_default, + password: 'password', name: 'test' } expect(response).to have_gitlab_http_status(:bad_request) @@ -1093,7 +1093,7 @@ RSpec.describe API::Users do post api('/users', admin), params: { email: 'test@example.com', - password: Gitlab::Password.test_default, + password: 'password', username: 'test', name: 'foo' } @@ -1105,7 +1105,7 @@ RSpec.describe API::Users do params: { name: 'foo', email: 'test@example.com', - password: Gitlab::Password.test_default, + password: 'password', username: 'foo' } end.to change { User.count }.by(0) @@ -1119,7 +1119,7 @@ RSpec.describe API::Users do params: { name: 'foo', email: 'foo@example.com', - password: Gitlab::Password.test_default, + password: 'password', username: 'test' } end.to change { User.count }.by(0) @@ -1133,7 +1133,7 @@ RSpec.describe API::Users do params: { name: 'foo', email: 'foo@example.com', - password: Gitlab::Password.test_default, + password: 'password', username: 'TEST' } end.to change { User.count }.by(0) @@ -1478,8 +1478,8 @@ RSpec.describe API::Users do context "with existing user" do before do - post api("/users", admin), params: { email: 'test@example.com', password: Gitlab::Password.test_default, username: 'test', name: 'test' } - post api("/users", admin), params: { email: 'foo@bar.com', password: Gitlab::Password.test_default, username: 'john', name: 'john' } + post api("/users", admin), params: { email: 'test@example.com', password: 'password', username: 'test', name: 'test' } + post api("/users", admin), params: { email: 'foo@bar.com', password: 'password', username: 'john', name: 'john' } @user = User.all.last end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 623cf24b9cb..d2528600477 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -319,7 +319,7 @@ RSpec.describe 'Git HTTP requests' do context 'when user is using credentials with special characters' do context 'with password with special characters' do before do - user.update!(password: Gitlab::Password.test_default) + user.update!(password: 'RKszEwéC5kFnû∆f243fycGu§Gh9ftDj!U') end it 'allows clones' do @@ -1670,7 +1670,7 @@ RSpec.describe 'Git HTTP requests' do context 'when user is using credentials with special characters' do context 'with password with special characters' do before do - user.update!(password: Gitlab::Password.test_default) + user.update!(password: 'RKszEwéC5kFnû∆f243fycGu§Gh9ftDj!U') end it 'allows clones' do diff --git a/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb b/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb index e62a94b6df8..a920b90b97d 100644 --- a/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb +++ b/spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb @@ -53,6 +53,24 @@ RSpec.describe Ci::CreatePipelineService do end context 'when failed to create the pipeline' do + context 'when errors are raised and masked variables are involved' do + let_it_be(:variable) { create(:ci_variable, project: project, key: 'GL_TOKEN', value: 'test_value', masked: true) } + + let(:config) do + <<~YAML + include: + - local: $GL_TOKEN/gitlab-ci.txt + YAML + end + + it 'contains errors and masks variables' do + error_message = "Included file `xxxxxxxxxx/gitlab-ci.txt` does not have YAML extension!" + expect(pipeline.yaml_errors).to eq(error_message) + expect(pipeline.error_messages.map(&:content)).to contain_exactly(error_message) + expect(pipeline.errors.full_messages).to contain_exactly(error_message) + end + end + context 'when warnings are raised' do let(:config) do <<~YAML diff --git a/spec/services/users/create_service_spec.rb b/spec/services/users/create_service_spec.rb index ab9da82e91c..74340bac055 100644 --- a/spec/services/users/create_service_spec.rb +++ b/spec/services/users/create_service_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Users::CreateService do context 'when required parameters are provided' do let(:params) do - { name: 'John Doe', username: 'jduser', email: email, password: Gitlab::Password.test_default } + { name: 'John Doe', username: 'jduser', email: email, password: 'mydummypass' } end it 'returns a persisted user' do @@ -82,13 +82,13 @@ RSpec.describe Users::CreateService do context 'when force_random_password parameter is true' do let(:params) do - { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: Gitlab::Password.test_default, force_random_password: true } + { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass', force_random_password: true } end it 'generates random password' do user = service.execute - expect(user.password).not_to eq Gitlab::Password.test_default + expect(user.password).not_to eq 'mydummypass' expect(user.password).to be_present end end @@ -99,7 +99,7 @@ RSpec.describe Users::CreateService do name: 'John Doe', username: 'jduser', email: 'jd@example.com', - password: Gitlab::Password.test_default, + password: 'mydummypass', password_automatically_set: true } end @@ -121,7 +121,7 @@ RSpec.describe Users::CreateService do context 'when skip_confirmation parameter is true' do let(:params) do - { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: Gitlab::Password.test_default, skip_confirmation: true } + { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass', skip_confirmation: true } end it 'confirms the user' do @@ -131,7 +131,7 @@ RSpec.describe Users::CreateService do context 'when reset_password parameter is true' do let(:params) do - { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: Gitlab::Password.test_default, reset_password: true } + { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass', reset_password: true } end it 'resets password even if a password parameter is given' do @@ -152,7 +152,7 @@ RSpec.describe Users::CreateService do context 'with nil user' do let(:params) do - { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: Gitlab::Password.test_default, skip_confirmation: true } + { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass', skip_confirmation: true } end let(:service) { described_class.new(nil, params) } diff --git a/spec/support/helpers/login_helpers.rb b/spec/support/helpers/login_helpers.rb index 4e0e8dd96ee..55650391695 100644 --- a/spec/support/helpers/login_helpers.rb +++ b/spec/support/helpers/login_helpers.rb @@ -91,11 +91,12 @@ module LoginHelpers # user - User instance to login with # remember - Whether or not to check "Remember me" (default: false) # two_factor_auth - If two-factor authentication is enabled (default: false) - def gitlab_sign_in_with(user, remember: false, two_factor_auth: false) + # password - password to attempt to login with + def gitlab_sign_in_with(user, remember: false, two_factor_auth: false, password: nil) visit new_user_session_path fill_in "user_login", with: user.email - fill_in "user_password", with: Gitlab::Password.test_default + fill_in "user_password", with: (password || "12345678") check 'user_remember_me' if remember click_button "Sign in" diff --git a/spec/support/shared_contexts/policies/project_policy_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_shared_context.rb index c39252cef13..9504a3c4b17 100644 --- a/spec/support/shared_contexts/policies/project_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/project_policy_shared_context.rb @@ -15,7 +15,7 @@ RSpec.shared_context 'ProjectPolicy context' do let(:base_guest_permissions) do %i[ - award_emoji create_issue create_merge_request_in create_note + award_emoji create_issue create_note create_project read_issue_board read_issue read_issue_iid read_issue_link read_label read_issue_board_list read_milestone read_note read_project read_project_for_iids read_project_member read_release read_snippet @@ -26,12 +26,12 @@ RSpec.shared_context 'ProjectPolicy context' do let(:base_reporter_permissions) do %i[ admin_issue admin_issue_link admin_label admin_issue_board_list - create_snippet create_incident daily_statistics download_code + create_snippet create_incident daily_statistics create_merge_request_in download_code download_wiki_code fork_project metrics_dashboard read_build read_commit_status read_confidential_issues read_container_image read_deployment read_environment read_merge_request read_metrics_dashboard_annotation read_pipeline read_prometheus - read_sentry_issue update_issue + read_sentry_issue update_issue create_merge_request_in ] end @@ -66,7 +66,7 @@ RSpec.shared_context 'ProjectPolicy context' do let(:public_permissions) do %i[ - build_download_code build_read_container_image download_code + build_download_code build_read_container_image create_merge_request_in download_code download_wiki_code fork_project read_commit_status read_container_image read_pipeline read_release ] diff --git a/spec/support/shared_examples/policies/project_policy_shared_examples.rb b/spec/support/shared_examples/policies/project_policy_shared_examples.rb index a4243db6bc9..63e4d458ad4 100644 --- a/spec/support/shared_examples/policies/project_policy_shared_examples.rb +++ b/spec/support/shared_examples/policies/project_policy_shared_examples.rb @@ -107,6 +107,19 @@ RSpec.shared_examples 'deploy token does not get confused with user' do end RSpec.shared_examples 'project policies as guest' do + context 'abilities for public projects' do + let(:project) { public_project } + let(:current_user) { guest } + + it do + expect_allowed(*guest_permissions) + expect_allowed(*public_permissions) + expect_disallowed(*developer_permissions) + expect_disallowed(*maintainer_permissions) + expect_disallowed(*owner_permissions) + end + end + context 'abilities for non-public projects' do let(:project) { private_project } let(:current_user) { guest } diff --git a/spec/tasks/gitlab/password_rake_spec.rb b/spec/tasks/gitlab/password_rake_spec.rb index ec18d713351..65bba836024 100644 --- a/spec/tasks/gitlab/password_rake_spec.rb +++ b/spec/tasks/gitlab/password_rake_spec.rb @@ -3,7 +3,7 @@ require 'rake_helper' RSpec.describe 'gitlab:password rake tasks', :silence_stdout do - let_it_be(:user_1) { create(:user, username: 'foobar', password: Gitlab::Password.test_default) } + let_it_be(:user_1) { create(:user, username: 'foobar', password: 'initial_password') } def stub_username(username) allow(Gitlab::TaskHelpers).to receive(:prompt).with('Enter username: ').and_return(username) @@ -19,14 +19,14 @@ RSpec.describe 'gitlab:password rake tasks', :silence_stdout do Rake.application.rake_require 'tasks/gitlab/password' stub_username('foobar') - stub_password(Gitlab::Password.test_default) + stub_password('secretpassword') end describe ':reset' do context 'when all inputs are correct' do it 'updates the password properly' do run_rake_task('gitlab:password:reset', user_1.username) - expect(user_1.reload.valid_password?(Gitlab::Password.test_default)).to eq(true) + expect(user_1.reload.valid_password?('secretpassword')).to eq(true) end end @@ -55,7 +55,7 @@ RSpec.describe 'gitlab:password rake tasks', :silence_stdout do context 'when passwords do not match' do before do - stub_password(Gitlab::Password.test_default, "different" + Gitlab::Password.test_default) + stub_password('randompassword', 'differentpassword') end it 'aborts with an error' do diff --git a/yarn.lock b/yarn.lock index 51cc427e10f..5d7b3ecec43 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11398,10 +11398,10 @@ svg-tags@^1.0.0: resolved "https://registry.yarnpkg.com/svg-tags/-/svg-tags-1.0.0.tgz#58f71cee3bd519b59d4b2a843b6c7de64ac04764" integrity sha1-WPcc7jvVGbWdSyqEO2x95krAR2Q= -swagger-ui-dist@^3.52.3: - version "3.52.3" - resolved "https://registry.yarnpkg.com/swagger-ui-dist/-/swagger-ui-dist-3.52.3.tgz#a09b5cdccac69e3f5f1cbd258654a110119a7f0e" - integrity sha512-7QSY4milmYx5O8dbzU5tTftiaoZt+4JGxahTTBiLAnbTvhTyzum9rsjDIJjC+xeT8Tt1KfB38UuQQjmrh2THDQ== +swagger-ui-dist@4.8.0: + version "4.8.0" + resolved "https://registry.yarnpkg.com/swagger-ui-dist/-/swagger-ui-dist-4.8.0.tgz#5f39a038a02ffbd5defb8e1921a9ac1620d779ae" + integrity sha512-jdcO4XcbwkAtrwvHp90Usjx3d4JZMjaiS02CxBFfuSxr6G8DBXPcK471+N6BcBkwZK7VTgpUBFAyyarsAvKYFQ== symbol-observable@^1.0.2, symbol-observable@^1.0.4: version "1.2.0" |