From e12f099f39ef8fb81f9b91612f8b35aefba7347c Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 3 Dec 2021 10:05:41 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@14-5-stable-ee --- app/models/concerns/bulk_member_access_load.rb | 25 ++-- app/models/concerns/diff_positionable_note.rb | 1 + .../user_max_access_level_in_groups_preloader.rb | 9 +- app/models/project.rb | 1 + app/models/project_team.rb | 6 +- app/validators/json_schema_validator.rb | 4 +- app/validators/json_schemas/position.json | 151 +++++++++++++++++++++ lib/api/lint.rb | 12 +- lib/banzai/filter/front_matter_filter.rb | 2 +- lib/gitlab/current_settings.rb | 2 +- lib/gitlab/diff/lines_unfolder.rb | 1 + lib/gitlab/front_matter.rb | 10 +- lib/gitlab/wiki_pages/front_matter_parser.rb | 2 +- locale/gitlab.pot | 3 + spec/factories/diff_position.rb | 8 +- spec/frontend/diffs/store/utils_spec.js | 4 +- spec/lib/banzai/filter/front_matter_filter_spec.rb | 16 +++ spec/lib/gitlab/current_settings_spec.rb | 10 +- .../gitlab/diff/formatters/text_formatter_spec.rb | 6 +- spec/lib/gitlab/diff/lines_unfolder_spec.rb | 10 ++ .../diff/position_tracer/line_strategy_spec.rb | 24 +++- .../gitlab/wiki_pages/front_matter_parser_spec.rb | 2 +- ...er_max_access_level_in_groups_preloader_spec.rb | 3 +- spec/requests/api/lint_spec.rb | 29 ++++ .../diff_positionable_note_shared_examples.rb | 33 +++++ .../api/diff_discussions_shared_examples.rb | 12 +- spec/validators/json_schema_validator_spec.rb | 12 ++ 27 files changed, 346 insertions(+), 52 deletions(-) create mode 100644 app/validators/json_schemas/position.json diff --git a/app/models/concerns/bulk_member_access_load.rb b/app/models/concerns/bulk_member_access_load.rb index e252ca36629..927d6ccb28f 100644 --- a/app/models/concerns/bulk_member_access_load.rb +++ b/app/models/concerns/bulk_member_access_load.rb @@ -9,11 +9,15 @@ module BulkMemberAccessLoad # Determine the maximum access level for a group of resources in bulk. # # Returns a Hash mapping resource ID -> maximum access level. - def max_member_access_for_resource_ids(resource_klass, resource_ids, memoization_index = self.id, &block) + def max_member_access_for_resource_ids(resource_klass, resource_ids, &block) raise 'Block is mandatory' unless block_given? + memoization_index = self.id + memoization_class = self.class + resource_ids = resource_ids.uniq - access = load_access_hash(resource_klass, memoization_index) + memo_id = "#{memoization_class}:#{memoization_index}" + access = load_access_hash(resource_klass, memo_id) # Look up only the IDs we need resource_ids -= access.keys @@ -33,8 +37,8 @@ module BulkMemberAccessLoad access end - def merge_value_to_request_store(resource_klass, resource_id, memoization_index, value) - max_member_access_for_resource_ids(resource_klass, [resource_id], memoization_index) do + def merge_value_to_request_store(resource_klass, resource_id, value) + max_member_access_for_resource_ids(resource_klass, [resource_id]) do { resource_id => value } end end @@ -45,16 +49,13 @@ module BulkMemberAccessLoad "max_member_access_for_#{klass.name.underscore.pluralize}:#{memoization_index}" end - def load_access_hash(resource_klass, memoization_index) - key = max_member_access_for_resource_key(resource_klass, memoization_index) + def load_access_hash(resource_klass, memo_id) + return {} unless Gitlab::SafeRequestStore.active? - access = {} - if Gitlab::SafeRequestStore.active? - Gitlab::SafeRequestStore[key] ||= {} - access = Gitlab::SafeRequestStore[key] - end + key = max_member_access_for_resource_key(resource_klass, memo_id) + Gitlab::SafeRequestStore[key] ||= {} - access + Gitlab::SafeRequestStore[key] end end end diff --git a/app/models/concerns/diff_positionable_note.rb b/app/models/concerns/diff_positionable_note.rb index cea3c7d119c..b13ca4bf06e 100644 --- a/app/models/concerns/diff_positionable_note.rb +++ b/app/models/concerns/diff_positionable_note.rb @@ -12,6 +12,7 @@ module DiffPositionableNote serialize :change_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize validate :diff_refs_match_commit, if: :for_commit? + validates :position, json_schema: { filename: "position", hash_conversion: true } end %i(original_position position change_position).each do |meth| diff --git a/app/models/preloaders/user_max_access_level_in_groups_preloader.rb b/app/models/preloaders/user_max_access_level_in_groups_preloader.rb index bdd76d39ec1..2cd54b975f3 100644 --- a/app/models/preloaders/user_max_access_level_in_groups_preloader.rb +++ b/app/models/preloaders/user_max_access_level_in_groups_preloader.rb @@ -4,8 +4,6 @@ module Preloaders # This class preloads the max access level (role) for the user within the given groups and # stores the values in requests store. class UserMaxAccessLevelInGroupsPreloader - include BulkMemberAccessLoad - def initialize(groups, user) @groups = groups @user = user @@ -27,8 +25,9 @@ module Preloaders .group(:source_id) .maximum(:access_level) - group_memberships.each do |group_id, max_access_level| - merge_value_to_request_store(User, @user.id, group_id, max_access_level) + @groups.each do |group| + access_level = group_memberships[group.id] + group.merge_value_to_request_store(User, @user.id, access_level) if access_level.present? end end @@ -41,7 +40,7 @@ module Preloaders @groups.each do |group| max_access_level = max_access_levels[group.id] || Gitlab::Access::NO_ACCESS - merge_value_to_request_store(User, @user.id, group.id, max_access_level) + group.merge_value_to_request_store(User, @user.id, max_access_level) end end diff --git a/app/models/project.rb b/app/models/project.rb index 2288850553c..45999da7839 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -36,6 +36,7 @@ class Project < ApplicationRecord include Repositories::CanHousekeepRepository include EachBatch include GitlabRoutingHelper + include BulkMemberAccessLoad extend Gitlab::Cache::RequestCache extend Gitlab::Utils::Override diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 94904e9792f..8061554006d 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class ProjectTeam - include BulkMemberAccessLoad - attr_accessor :project def initialize(project) @@ -171,7 +169,7 @@ class ProjectTeam # # Returns a Hash mapping user ID -> maximum access level. def max_member_access_for_user_ids(user_ids) - max_member_access_for_resource_ids(User, user_ids, project.id) do |user_ids| + project.max_member_access_for_resource_ids(User, user_ids) do |user_ids| project.project_authorizations .where(user: user_ids) .group(:user_id) @@ -180,7 +178,7 @@ class ProjectTeam end def write_member_access_for_user_id(user_id, project_access_level) - merge_value_to_request_store(User, user_id, project.id, project_access_level) + project.merge_value_to_request_store(User, user_id, project_access_level) end def max_member_access(user_id) diff --git a/app/validators/json_schema_validator.rb b/app/validators/json_schema_validator.rb index 68f03e8a6a3..4896c2ea2ef 100644 --- a/app/validators/json_schema_validator.rb +++ b/app/validators/json_schema_validator.rb @@ -24,8 +24,10 @@ class JsonSchemaValidator < ActiveModel::EachValidator end def validate_each(record, attribute, value) + value = value.to_h.stringify_keys if options[:hash_conversion] == true + unless valid_schema?(value) - record.errors.add(attribute, "must be a valid json schema") + record.errors.add(attribute, _("must be a valid json schema")) end end diff --git a/app/validators/json_schemas/position.json b/app/validators/json_schemas/position.json new file mode 100644 index 00000000000..d2c83be7639 --- /dev/null +++ b/app/validators/json_schemas/position.json @@ -0,0 +1,151 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Gitlab::Diff::Position", + "type": "object", + "additionalProperties": false, + "properties": { + "base_sha": { + "oneOf": [ + { "type": "null" }, + { "type": "string", "maxLength": 40 } + ] + }, + "start_sha": { + "oneOf": [ + { "type": "null" }, + { "type": "string", "maxLength": 40 } + ] + }, + "head_sha": { + "oneOf": [ + { "type": "null" }, + { "type": "string", "maxLength": 40 } + ] + }, + "file_identifier_hash": { + "oneOf": [ + { "type": "null" }, + { "type": "string", "maxLength": 40 } + ] + }, + "old_path": { + "oneOf": [ + { "type": "null" }, + { "type": "string", "maxLength": 1000 } + ] + }, + "new_path": { + "oneOf": [ + { "type": "null" }, + { "type": "string", "maxLength": 1000 } + ] + }, + "position_type": { + "oneOf": [ + { "type": "null" }, + { "type": "string", "maxLength": 10 } + ] + }, + "old_line": { + "oneOf": [ + { "type": "null" }, + { "type": "integer" } + ] + }, + "new_line": { + "oneOf": [ + { "type": "null" }, + { "type": "integer" } + ] + }, + "line_range": { + "oneOf": [ + { "type": "null" }, + { + "type": "object", + "additionalProperties": false, + "properties": { + "start": { + "type": "object", + "additionalProperties": false, + "properties": { + "line_code": { "type": "string", "maxLength": 100 }, + "type": { + "oneOf": [ + { "type": "null" }, + { "type": "string", "maxLength": 100 } + ] + }, + "old_line": { + "oneOf": [ + { "type": "null" }, + { "type": "integer" } + ] + }, + "new_line": { + "oneOf": [ + { "type": "null" }, + { "type": "integer" } + ] + } + } + }, + "end": { + "type": "object", + "additionalProperties": false, + "properties": { + "line_code": { "type": "string", "maxLength": 100 }, + "type": { + "oneOf": [ + { "type": "null" }, + { "type": "string", "maxLength": 100 } + ] + }, + "old_line": { + "oneOf": [ + { "type": "null" }, + { "type": "integer" } + ] + }, + "new_line": { + "oneOf": [ + { "type": "null" }, + { "type": "integer" } + ] + } + } + } + } + } + ] + }, + "width": { + "oneOf": [ + { "type": "null" }, + { "type": "integer" }, + { "type": "string", "maxLength": 10 } + ] + }, + "height": { + "oneOf": [ + { "type": "null" }, + { "type": "integer" }, + { "type": "string", "maxLength": 10 } + ] + }, + "x": { + "oneOf": [ + { "type": "null" }, + { "type": "integer" }, + { "type": "string", "maxLength": 10 } + ] + }, + "y": { + "oneOf": [ + { "type": "null" }, + { "type": "integer" }, + { "type": "string", "maxLength": 10 } + ] + } + } +} diff --git a/lib/api/lint.rb b/lib/api/lint.rb index f1e19e9c3c5..3655cb56564 100644 --- a/lib/api/lint.rb +++ b/lib/api/lint.rb @@ -4,6 +4,16 @@ module API class Lint < ::API::Base feature_category :pipeline_authoring + helpers do + def can_lint_ci? + signup_unrestricted = Gitlab::CurrentSettings.signup_enabled? && !Gitlab::CurrentSettings.signup_limited? + internal_user = current_user.present? && !current_user.external? + is_developer = current_user.present? && current_user.projects.any? { |p| p.team.member?(current_user, Gitlab::Access::DEVELOPER) } + + signup_unrestricted || internal_user || is_developer + end + end + namespace :ci do desc 'Validation of .gitlab-ci.yml content' params do @@ -12,7 +22,7 @@ module API optional :include_jobs, type: Boolean, desc: 'Whether or not to include CI jobs in the response' end post '/lint' do - unauthorized! if (Gitlab::CurrentSettings.signup_disabled? || Gitlab::CurrentSettings.signup_limited?) && current_user.nil? + unauthorized! unless can_lint_ci? result = Gitlab::Ci::Lint.new(project: nil, current_user: current_user) .validate(params[:content], dry_run: false) diff --git a/lib/banzai/filter/front_matter_filter.rb b/lib/banzai/filter/front_matter_filter.rb index d47900b816a..705400a5497 100644 --- a/lib/banzai/filter/front_matter_filter.rb +++ b/lib/banzai/filter/front_matter_filter.rb @@ -9,7 +9,7 @@ module Banzai html.sub(Gitlab::FrontMatter::PATTERN) do |_match| lang = $~[:lang].presence || lang_mapping[$~[:delim]] - ["```#{lang}:frontmatter", $~[:front_matter], "```", "\n"].join("\n") + ["```#{lang}:frontmatter", $~[:front_matter].strip!, "```", "\n"].join("\n") end end end diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index b9034cff447..2d2d8c41236 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -8,7 +8,7 @@ module Gitlab end def signup_limited? - domain_allowlist.present? || email_restrictions_enabled? || require_admin_approval_after_user_signup? + domain_allowlist.present? || email_restrictions_enabled? || require_admin_approval_after_user_signup? || user_default_external? end def current_application_settings diff --git a/lib/gitlab/diff/lines_unfolder.rb b/lib/gitlab/diff/lines_unfolder.rb index 6def3a074a3..04ed5857233 100644 --- a/lib/gitlab/diff/lines_unfolder.rb +++ b/lib/gitlab/diff/lines_unfolder.rb @@ -57,6 +57,7 @@ module Gitlab next false unless @position.unfoldable? next false if @diff_file.new_file? || @diff_file.deleted_file? next false unless @position.old_line + next false unless @position.old_line.is_a?(Integer) # Invalid position (MR import scenario) next false if @position.old_line > @blob.lines.size next false if @diff_file.diff_lines.empty? diff --git a/lib/gitlab/front_matter.rb b/lib/gitlab/front_matter.rb index 7612bd36aca..5c5c74ca1a0 100644 --- a/lib/gitlab/front_matter.rb +++ b/lib/gitlab/front_matter.rb @@ -11,13 +11,11 @@ module Gitlab DELIM = Regexp.union(DELIM_LANG.keys) PATTERN = %r{ - \A(?:[^\r\n]*coding:[^\r\n]*)? # optional encoding line + \A(?:[^\r\n]*coding:[^\r\n]*\R)? # optional encoding line \s* - ^(?#{DELIM})[ \t]*(?\S*) # opening front matter marker (optional language specifier) - \s* - ^(?.*?) # front matter block content (not greedy) - \s* - ^(\k | \.{3}) # closing front matter marker + ^(?#{DELIM})[ \t]*(?\S*)\R # opening front matter marker (optional language specifier) + (?.*?) # front matter block content (not greedy) + ^(\k | \.{3}) # closing front matter marker \s* }mx.freeze end diff --git a/lib/gitlab/wiki_pages/front_matter_parser.rb b/lib/gitlab/wiki_pages/front_matter_parser.rb index 45dc6cf7fd1..0ceec39782c 100644 --- a/lib/gitlab/wiki_pages/front_matter_parser.rb +++ b/lib/gitlab/wiki_pages/front_matter_parser.rb @@ -54,7 +54,7 @@ module Gitlab def initialize(delim = nil, lang = '', text = nil) @lang = lang.downcase.presence || Gitlab::FrontMatter::DELIM_LANG[delim] - @text = text + @text = text&.strip! end def data diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f87833e829d..1e290fa6c62 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -41577,6 +41577,9 @@ msgstr "" msgid "must be a valid IPv4 or IPv6 address" msgstr "" +msgid "must be a valid json schema" +msgstr "" + msgid "must be after start" msgstr "" diff --git a/spec/factories/diff_position.rb b/spec/factories/diff_position.rb index 41f9a7b574e..bd248452de8 100644 --- a/spec/factories/diff_position.rb +++ b/spec/factories/diff_position.rb @@ -43,8 +43,12 @@ FactoryBot.define do trait :multi_line do line_range do { - start_line_code: Gitlab::Git.diff_line_code(file, 10, 10), - end_line_code: Gitlab::Git.diff_line_code(file, 12, 13) + start: { + line_code: Gitlab::Git.diff_line_code(file, 10, 10) + }, + end: { + line_code: Gitlab::Git.diff_line_code(file, 12, 13) + } } end end diff --git a/spec/frontend/diffs/store/utils_spec.js b/spec/frontend/diffs/store/utils_spec.js index 73de0a6d381..55c0141552d 100644 --- a/spec/frontend/diffs/store/utils_spec.js +++ b/spec/frontend/diffs/store/utils_spec.js @@ -138,7 +138,7 @@ describe('DiffsStoreUtils', () => { old_line: 1, }, linePosition: LINE_POSITION_LEFT, - lineRange: { start_line_code: 'abc_1_1', end_line_code: 'abc_2_2' }, + lineRange: { start: { line_code: 'abc_1_1' }, end: { line_code: 'abc_2_2' } }, }; const position = JSON.stringify({ @@ -608,7 +608,7 @@ describe('DiffsStoreUtils', () => { // When multi line comments are fully implemented `line_code` will be // included in all requests. Until then we need to ensure the logic does // not change when it is included only in the "comparison" argument. - const lineRange = { start_line_code: 'abc_1_1', end_line_code: 'abc_1_2' }; + const lineRange = { start: { line_code: 'abc_1_1' }, end: { line_code: 'abc_1_2' } }; it('returns true when the discussion is up to date', () => { expect( diff --git a/spec/lib/banzai/filter/front_matter_filter_spec.rb b/spec/lib/banzai/filter/front_matter_filter_spec.rb index cef6a2ddcce..1562c388296 100644 --- a/spec/lib/banzai/filter/front_matter_filter_spec.rb +++ b/spec/lib/banzai/filter/front_matter_filter_spec.rb @@ -139,4 +139,20 @@ RSpec.describe Banzai::Filter::FrontMatterFilter do end end end + + it 'fails fast for strings with many spaces' do + content = "coding:" + " " * 50_000 + ";" + + expect do + Timeout.timeout(3.seconds) { filter(content) } + end.not_to raise_error + end + + it 'fails fast for strings with many newlines' do + content = "coding:\n" + ";;;" + "\n" * 10_000 + "x" + + expect do + Timeout.timeout(3.seconds) { filter(content) } + end.not_to raise_error + end end diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb index a5ab1047a40..46c33d7b7b2 100644 --- a/spec/lib/gitlab/current_settings_spec.rb +++ b/spec/lib/gitlab/current_settings_spec.rb @@ -51,9 +51,17 @@ RSpec.describe Gitlab::CurrentSettings do it { is_expected.to be_truthy } end + context 'when new users are set to external' do + before do + create(:application_setting, user_default_external: true) + end + + it { is_expected.to be_truthy } + end + context 'when there are no restrictions' do before do - create(:application_setting, domain_allowlist: [], email_restrictions_enabled: false, require_admin_approval_after_user_signup: false) + create(:application_setting, domain_allowlist: [], email_restrictions_enabled: false, require_admin_approval_after_user_signup: false, user_default_external: false) end it { is_expected.to be_falsey } diff --git a/spec/lib/gitlab/diff/formatters/text_formatter_spec.rb b/spec/lib/gitlab/diff/formatters/text_formatter_spec.rb index 41877a16ebf..b6bdc5ff493 100644 --- a/spec/lib/gitlab/diff/formatters/text_formatter_spec.rb +++ b/spec/lib/gitlab/diff/formatters/text_formatter_spec.rb @@ -47,14 +47,14 @@ RSpec.describe Gitlab::Diff::Formatters::TextFormatter do describe "#==" do it "is false when the line_range changes" do - formatter_1 = described_class.new(base.merge(line_range: { start_line_code: "foo", end_line_code: "bar" })) - formatter_2 = described_class.new(base.merge(line_range: { start_line_code: "foo", end_line_code: "baz" })) + formatter_1 = described_class.new(base.merge(line_range: { "start": { "line_code" => "foo" }, "end": { "line_code" => "bar" } })) + formatter_2 = described_class.new(base.merge(line_range: { "start": { "line_code" => "foo" }, "end": { "line_code" => "baz" } })) expect(formatter_1).not_to eq(formatter_2) end it "is true when the line_range doesn't change" do - attrs = base.merge({ line_range: { start_line_code: "foo", end_line_code: "baz" } }) + attrs = base.merge({ line_range: { start: { line_code: "foo" }, end: { line_code: "baz" } } }) formatter_1 = described_class.new(attrs) formatter_2 = described_class.new(attrs) diff --git a/spec/lib/gitlab/diff/lines_unfolder_spec.rb b/spec/lib/gitlab/diff/lines_unfolder_spec.rb index 8385cba3532..f0e710be2e4 100644 --- a/spec/lib/gitlab/diff/lines_unfolder_spec.rb +++ b/spec/lib/gitlab/diff/lines_unfolder_spec.rb @@ -215,6 +215,16 @@ RSpec.describe Gitlab::Diff::LinesUnfolder do build(:text_diff_position, old_line: 43, new_line: 40) end + context 'old_line is an invalid number' do + let(:position) do + build(:text_diff_position, old_line: "foo", new_line: 40) + end + + it 'fails gracefully' do + expect(subject.unfolded_diff_lines).to be_nil + end + end + context 'blob lines' do let(:expected_blob_lines) do [[40, 40, " \"config-opts\": [ \"--disable-introspection\" ],"], diff --git a/spec/lib/gitlab/diff/position_tracer/line_strategy_spec.rb b/spec/lib/gitlab/diff/position_tracer/line_strategy_spec.rb index b646cf38178..c46f476899e 100644 --- a/spec/lib/gitlab/diff/position_tracer/line_strategy_spec.rb +++ b/spec/lib/gitlab/diff/position_tracer/line_strategy_spec.rb @@ -295,8 +295,12 @@ RSpec.describe Gitlab::Diff::PositionTracer::LineStrategy, :clean_gitlab_redis_c new_path: file_name, new_line: 2, line_range: { - "start_line_code" => 1, - "end_line_code" => 2 + "start" => { + "line_code" => 1 + }, + "end" => { + "line_code" => 2 + } } ) end @@ -575,8 +579,12 @@ RSpec.describe Gitlab::Diff::PositionTracer::LineStrategy, :clean_gitlab_redis_c new_path: file_name, new_line: 2, line_range: { - "start_line_code" => 1, - "end_line_code" => 2 + "start" => { + "line_code" => 1 + }, + "end" => { + "line_code" => 2 + } } ) end @@ -588,8 +596,12 @@ RSpec.describe Gitlab::Diff::PositionTracer::LineStrategy, :clean_gitlab_redis_c old_line: nil, new_line: 2, line_range: { - "start_line_code" => 1, - "end_line_code" => 2 + "start" => { + "line_code" => 1 + }, + "end" => { + "line_code" => 2 + } } ) end diff --git a/spec/lib/gitlab/wiki_pages/front_matter_parser_spec.rb b/spec/lib/gitlab/wiki_pages/front_matter_parser_spec.rb index c78103f33f4..3152dc2ad2f 100644 --- a/spec/lib/gitlab/wiki_pages/front_matter_parser_spec.rb +++ b/spec/lib/gitlab/wiki_pages/front_matter_parser_spec.rb @@ -118,7 +118,7 @@ RSpec.describe Gitlab::WikiPages::FrontMatterParser do MD end - it { is_expected.to have_attributes(reason: :not_mapping) } + it { is_expected.to have_attributes(reason: :no_match) } end context 'there is a string in the YAML block' do diff --git a/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb index 5fc7bfb1f62..2060e6cd44a 100644 --- a/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb +++ b/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb @@ -13,7 +13,8 @@ RSpec.describe Preloaders::UserMaxAccessLevelInGroupsPreloader do shared_examples 'executes N max member permission queries to the DB' do it 'executes the specified max membership queries' do - expect { groups.each { |group| user.can?(:read_group, group) } }.to make_queries_matching(max_query_regex, expected_query_count) + expect { groups.each { |group| user.can?(:read_group, group) } } + .to make_queries_matching(max_query_regex, expected_query_count) end it 'caches the correct access_level for each group' do diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb index ac30da99afe..0e83b964121 100644 --- a/spec/requests/api/lint_spec.rb +++ b/spec/requests/api/lint_spec.rb @@ -26,6 +26,35 @@ RSpec.describe API::Lint do expect(response).to have_gitlab_http_status(:ok) end end + + context 'when authenticated as external user' do + let(:project) { create(:project) } + let(:api_user) { create(:user, :external) } + + context 'when reporter in a project' do + before do + project.add_reporter(api_user) + end + + it 'returns authorization failure' do + post api('/ci/lint', api_user), params: { content: 'content' } + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when developer in a project' do + before do + project.add_developer(api_user) + end + + it 'returns authorization success' do + post api('/ci/lint', api_user), params: { content: 'content' } + + expect(response).to have_gitlab_http_status(:ok) + end + end + end end context 'when signup is enabled and not limited' do diff --git a/spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb b/spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb index 759b22f794e..eafa589a1d3 100644 --- a/spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb +++ b/spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb @@ -71,5 +71,38 @@ RSpec.shared_examples 'a valid diff positionable note' do |factory_on_commit| end end end + + describe 'schema validation' do + where(:position_attrs) do + [ + { old_path: SecureRandom.alphanumeric(1001) }, + { new_path: SecureRandom.alphanumeric(1001) }, + { old_line: "foo" }, # this should be an integer + { new_line: "foo" }, # this should be an integer + { line_range: { "foo": "bar" } }, + { line_range: { "line_code": SecureRandom.alphanumeric(101) } }, + { line_range: { "type": SecureRandom.alphanumeric(101) } }, + { line_range: { "old_line": "foo" } }, + { line_range: { "new_line": "foo" } } + ] + end + + with_them do + let(:position) do + Gitlab::Diff::Position.new( + { + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + line_range: nil, + diff_refs: diff_refs + }.merge(position_attrs) + ) + end + + it { is_expected.to be_invalid } + end + end end end diff --git a/spec/support/shared_examples/requests/api/diff_discussions_shared_examples.rb b/spec/support/shared_examples/requests/api/diff_discussions_shared_examples.rb index 518c5b8dc28..7f2c445e93d 100644 --- a/spec/support/shared_examples/requests/api/diff_discussions_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/diff_discussions_shared_examples.rb @@ -29,10 +29,14 @@ RSpec.shared_examples 'diff discussions API' do |parent_type, noteable_type, id_ describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do it "creates a new diff note" do line_range = { - "start_line_code" => Gitlab::Git.diff_line_code(diff_note.position.file_path, 1, 1), - "end_line_code" => Gitlab::Git.diff_line_code(diff_note.position.file_path, 2, 2), - "start_line_type" => diff_note.position.type, - "end_line_type" => diff_note.position.type + "start" => { + "line_code" => Gitlab::Git.diff_line_code(diff_note.position.file_path, 1, 1), + "type" => diff_note.position.type + }, + "end" => { + "line_code" => Gitlab::Git.diff_line_code(diff_note.position.file_path, 2, 2), + "type" => diff_note.position.type + } } position = diff_note.position.to_h.merge({ line_range: line_range }) diff --git a/spec/validators/json_schema_validator_spec.rb b/spec/validators/json_schema_validator_spec.rb index 83eb0e2f3dd..01caf4ab0bd 100644 --- a/spec/validators/json_schema_validator_spec.rb +++ b/spec/validators/json_schema_validator_spec.rb @@ -46,5 +46,17 @@ RSpec.describe JsonSchemaValidator do expect { subject }.to raise_error(described_class::FilenameError) end end + + describe 'hash_conversion option' do + context 'when hash_conversion is enabled' do + let(:validator) { described_class.new(attributes: [:data], filename: "build_report_result_data", hash_conversion: true) } + + it 'returns no errors' do + subject + + expect(build_report_result.errors).to be_empty + end + end + end end end -- cgit v1.2.3