diff options
35 files changed, 868 insertions, 120 deletions
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 85a991a1ec9..6922ec9c5a5 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/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/integrations/asana.rb b/app/models/integrations/asana.rb index 054f0606dd2..e7634f51ec4 100644 --- a/app/models/integrations/asana.rb +++ b/app/models/integrations/asana.rb @@ -59,12 +59,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 @@ -103,5 +100,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/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/policies/project_policy.rb b/app/policies/project_policy.rb index 09085bef9f0..2ffafb79134 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -240,7 +240,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 @@ -497,7 +496,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)) @@ -600,13 +599,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 @@ -664,6 +664,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/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/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/error_tracking.rb b/lib/gitlab/error_tracking.rb index 259b430a73c..d71f9b5e7cf 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..4b6c69a8b33 --- /dev/null +++ b/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Gitlab + module ErrorTracking + module Processor + module Concerns + module ProcessesExceptions + private + + def extract_exceptions_from(event) + exceptions = if event.is_a?(Raven::Event) + event.instance_variable_get(:@interfaces)[:exception]&.values + else + event&.exception&.instance_variable_get(:@values) + end + + Array.wrap(exceptions) + end + + def set_exception_message(exception, message) + if exception.respond_to?(:value=) + exception.value = message + else + exception.instance_variable_set(:@value, message) + end + end + + def valid_exception?(exception) + case exception + when Raven::SingleExceptionInterface, Sentry::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 045a18f4110..ab0df39e512 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 @@ -19,9 +21,6 @@ module Gitlab def process_first_exception_value(event) # Better in new version, will be event.exception.values exceptions = extract_exceptions_from(event) - - return unless exceptions.is_a?(Array) - exception = exceptions.first return unless valid_exception?(exception) @@ -39,11 +38,7 @@ module Gitlab exceptions.each do |exception| next unless valid_exception?(exception) - if exception.respond_to?(:value=) - exception.value = message - else - exception.instance_variable_set(:@value, message) - end + set_exception_message(exception, message) end end @@ -59,16 +54,6 @@ module Gitlab fingerprint[1] = message if message end - private - - def extract_exceptions_from(event) - if event.is_a?(Raven::Event) - event.instance_variable_get(:@interfaces)[:exception]&.values - else - event.exception&.instance_variable_get(:@values) - end - end - def custom_grpc_fingerprint?(fingerprint) fingerprint.is_a?(Array) && fingerprint.length == 2 && fingerprint[0].start_with?('GRPC::') end @@ -82,15 +67,6 @@ module Gitlab [match[1], match[2]] end - - def valid_exception?(exception) - case exception - when Raven::SingleExceptionInterface, Sentry::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..1d6547256c7 --- /dev/null +++ b/lib/gitlab/error_tracking/processor/sanitize_error_message_processor.rb @@ -0,0 +1,27 @@ +# 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) + + message = Gitlab::Sanitizers::ExceptionMessage.clean(exception.type, exception.value) + + set_exception_message(exception, message) + 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/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/locale/gitlab.pot b/locale/gitlab.pot index e8075deb16c..87cdb4e3838 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -13120,6 +13120,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/spec/controllers/projects/merge_requests/creations_controller_spec.rb b/spec/controllers/projects/merge_requests/creations_controller_spec.rb index 3c650988b4f..a061a14c7b1 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 7bc86d7c583..686effd799e 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/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/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/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/reference_redactor_spec.rb b/spec/lib/banzai/reference_redactor_spec.rb index 45e14032a98..344b8988296 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/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..5ec73233e77 --- /dev/null +++ b/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb @@ -0,0 +1,53 @@ +# 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(:result_hash) { described_class.call(event).to_hash } + + shared_examples 'processes the exception' do + 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 + end + + context 'with Raven event' do + let(:raven_required_options) do + { + configuration: Raven.configuration, + context: Raven.context, + breadcrumbs: Raven.breadcrumbs + } + end + + let(:event) { Raven::Event.from_exception(exception, raven_required_options) } + + it_behaves_like 'processes the exception' + end + + context 'with Sentry event' do + let(:event) { Sentry.get_current_client.event_from_exception(exception) } + + it_behaves_like 'processes the exception' + end + + context 'with invalid event' do + let(:event) { instance_double('Sentry::Event', to_hash: { invalid: true }) } + + it 'does nothing' do + extracted_exception = instance_double('Sentry::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 936954fc1b6..1ade3a51c55 100644 --- a/spec/lib/gitlab/error_tracking_spec.rb +++ b/spec/lib/gitlab/error_tracking_spec.rb @@ -368,5 +368,61 @@ RSpec.describe Gitlab::ErrorTracking do end end end + + context 'when processing invalid URI exceptions' do + let(:invalid_uri) { 'http://foo:bar' } + let(:raven_exception_values) { raven_event['exception']['values'] } + let(:sentry_exception_values) { sentry_event.exception.to_hash[: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(raven_exception_values).to include( + hash_including( + 'type' => 'URI::InvalidURIError', + 'value' => 'bad URI(is not URI?): [FILTERED]' + ) + ) + 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(raven_exception_values).to include( + hash_including( + 'type' => 'Addressable::URI::InvalidURIError', + 'value' => 'Invalid port number: [FILTERED]' + ) + ) + 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/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/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/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/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 0da37fc5378..fb1c5874335 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 { 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 '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 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/support/shared_contexts/policies/project_policy_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_shared_context.rb index 3641edc845a..a78953e8199 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_planning_hierarchy 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 } |