diff options
| author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2021-12-06 22:53:58 +0000 |
|---|---|---|
| committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2021-12-06 22:53:58 +0000 |
| commit | 83ca7f60f94ee2d3f1a966fa9757849da5de4035 (patch) | |
| tree | 5cca41bee4989af1291c17ab8d7aa3479e20aff0 | |
| parent | 9d9ee598bc514eaee681b40cdff4d12a3a8f412a (diff) | |
| parent | 76ceea558aae5013c4e16b3b2f97363608765c21 (diff) | |
| download | gitlab-ce-83ca7f60f94ee2d3f1a966fa9757849da5de4035.tar.gz | |
Merge remote-tracking branch 'dev/14-5-stable' into 14-5-stable
94 files changed, 1156 insertions, 171 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index d204a9aeaeb..fce415ec963 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 14.5.2 (2021-12-03) + +No changes. + ## 14.5.1 (2021-12-01) ### Fixed (4 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index afd36b76968..e9d8f88edb6 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -14.5.1
\ No newline at end of file +14.5.2
\ No newline at end of file @@ -97,7 +97,7 @@ gem 'grape-entity', '~> 0.10.0' gem 'rack-cors', '~> 1.0.6', require: 'rack/cors' # GraphQL API -gem 'graphql', '~> 1.11.8' +gem 'graphql', '~> 1.11.10' # NOTE: graphiql-rails v1.5+ doesn't work: https://gitlab.com/gitlab-org/gitlab/issues/31771 # TODO: remove app/views/graphiql/rails/editors/show.html.erb when https://github.com/rmosolgo/graphiql-rails/pull/71 is released: # https://gitlab.com/gitlab-org/gitlab/issues/31747 diff --git a/Gemfile.lock b/Gemfile.lock index 270ea4532f9..b54874e9d80 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -558,7 +558,7 @@ GEM faraday (>= 1.0) faraday_middleware graphql-client - graphql (1.11.8) + graphql (1.11.10) graphql-client (0.16.0) activesupport (>= 3.0) graphql (~> 1.8) @@ -1490,7 +1490,7 @@ DEPENDENCIES grape_logging (~> 1.7) graphiql-rails (~> 1.4.10) graphlient (~> 0.4.0) - graphql (~> 1.11.8) + graphql (~> 1.11.10) graphql-docs (~> 1.6.0) grpc (~> 1.30.2) gssapi @@ -1 +1 @@ -14.5.1
\ No newline at end of file +14.5.2
\ No newline at end of file diff --git a/app/assets/javascripts/behaviors/markdown/render_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_mermaid.js index 3f878949f9b..d78c456ed5b 100644 --- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js @@ -75,7 +75,7 @@ export function initMermaid(mermaid) { function importMermaidModule() { return import(/* webpackChunkName: 'mermaid' */ 'mermaid') - .then((mermaid) => { + .then(({ default: mermaid }) => { mermaidModule = initMermaid(mermaid); }) .catch((err) => { diff --git a/app/assets/javascripts/blob/openapi/index.js b/app/assets/javascripts/blob/openapi/index.js index cb251274b18..b19cc19cb8c 100644 --- a/app/assets/javascripts/blob/openapi/index.js +++ b/app/assets/javascripts/blob/openapi/index.js @@ -1,5 +1,6 @@ import { SwaggerUIBundle } from 'swagger-ui-dist'; import createFlash from '~/flash'; +import { removeParams, updateHistory } from '~/lib/utils/url_utility'; import { __ } from '~/locale'; export default () => { @@ -7,9 +8,14 @@ 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, }); }) .catch((error) => { diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index fde0f133e53..899fa614949 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -9,6 +9,9 @@ class GraphqlController < ApplicationController # Header can be passed by tests to disable SQL query limits. DISABLE_SQL_QUERY_LIMIT_HEADER = 'HTTP_X_GITLAB_DISABLE_SQL_QUERY_LIMIT' + # Max size of the query text in characters + MAX_QUERY_SIZE = 10_000 + # If a user is using their session to access GraphQL, we need to have session # storage, since the admin-mode check is session wide. # We can't enable this for anonymous users because that would cause users using @@ -29,6 +32,7 @@ class GraphqlController < ApplicationController before_action :set_user_last_activity before_action :track_vs_code_usage before_action :disable_query_limiting + before_action :limit_query_size before_action :disallow_mutations_for_get @@ -81,6 +85,16 @@ class GraphqlController < ApplicationController raise ::Gitlab::Graphql::Errors::ArgumentError, "Mutations are forbidden in #{request.request_method} requests" end + def limit_query_size + total_size = if multiplex? + params[:_json].sum { _1[:query].size } + else + query.size + end + + raise ::Gitlab::Graphql::Errors::ArgumentError, "Query too large" if total_size > MAX_QUERY_SIZE + end + def any_mutating_query? if multiplex? multiplex_queries.any? { |q| mutation?(q[:query], q[:operation_name]) } @@ -126,7 +140,7 @@ class GraphqlController < ApplicationController end def query - params[:query] + params.fetch(:query, '') end def multiplex_queries diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb index e15a185a743..9b23aa60eab 100644 --- a/app/graphql/gitlab_schema.rb +++ b/app/graphql/gitlab_schema.rb @@ -32,6 +32,9 @@ class GitlabSchema < GraphQL::Schema default_max_page_size 100 + validate_max_errors 5 + validate_timeout 0.2.seconds + lazy_resolve ::Gitlab::Graphql::Lazy, :force class << self diff --git a/app/graphql/types/user_interface.rb b/app/graphql/types/user_interface.rb index 8c67275eb73..7cc201b6df4 100644 --- a/app/graphql/types/user_interface.rb +++ b/app/graphql/types/user_interface.rb @@ -29,7 +29,10 @@ module Types field :name, type: GraphQL::Types::String, null: false, - description: 'Human-readable name of the user.' + resolver_method: :redacted_name, + description: 'Human-readable name of the user. ' \ + 'Will return `****` if the user is a project bot and the requester does not have permission to read resource access tokens.' + field :state, type: Types::UserStateEnum, null: false, @@ -121,5 +124,16 @@ module Types ::Types::UserType end end + + def redacted_name + return object.name unless object.project_bot? + + return object.name if context[:current_user]&.can?(:read_resource_access_tokens, object.projects.first) + + # If the requester does not have permission to read the project bot name, + # the API returns an arbitrary string. UI changes will be addressed in a follow up issue: + # https://gitlab.com/gitlab-org/gitlab/-/issues/346058 + '****' + end end end diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index cb28025c900..aedb7df9e4f 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -201,18 +201,30 @@ module SearchHelper if @project && @project.repository.root_ref ref = @ref || @project.repository.root_ref - result = [ - { category: "In this project", label: _("Files"), url: project_tree_path(@project, ref) }, - { category: "In this project", label: _("Commits"), url: project_commits_path(@project, ref) }, - { category: "In this project", label: _("Network"), url: project_network_path(@project, ref) }, - { category: "In this project", label: _("Graph"), url: project_graph_path(@project, ref) }, + result = [] + + if can?(current_user, :download_code, @project) + result.concat([ + { category: "In this project", label: _("Files"), url: project_tree_path(@project, ref) }, + { category: "In this project", label: _("Commits"), url: project_commits_path(@project, ref) } + ]) + end + + if can?(current_user, :read_repository_graphs, @project) + result.concat([ + { category: "In this project", label: _("Network"), url: project_network_path(@project, ref) }, + { category: "In this project", label: _("Graph"), url: project_graph_path(@project, ref) } + ]) + end + + result.concat([ { category: "In this project", label: _("Issues"), url: project_issues_path(@project) }, { category: "In this project", label: _("Merge requests"), url: project_merge_requests_path(@project) }, { category: "In this project", label: _("Milestones"), url: project_milestones_path(@project) }, { category: "In this project", label: _("Snippets"), url: project_snippets_path(@project) }, { category: "In this project", label: _("Members"), url: project_project_members_path(@project) }, { category: "In this project", label: _("Wiki"), url: project_wikis_path(@project) } - ] + ]) if can?(current_user, :read_feature_flag, @project) result << { category: "In this project", label: _("Feature Flags"), url: project_feature_flags_path(@project) } 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/models/todo.rb b/app/models/todo.rb index 742b8fd2a9d..cfcb2201b80 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -69,7 +69,7 @@ class Todo < ApplicationRecord scope :for_type, -> (type) { where(target_type: type) } scope :for_target, -> (id) { where(target_id: id) } scope :for_commit, -> (id) { where(commit_id: id) } - scope :with_entity_associations, -> { preload(:target, :author, :note, group: :route, project: [:route, { namespace: :route }]) } + scope :with_entity_associations, -> { preload(:target, :author, :note, group: :route, project: [:route, { namespace: [:route, :owner] }]) } scope :joins_issue_and_assignees, -> { left_joins(issue: :assignees) } enum resolved_by_action: { system_done: 0, api_all_done: 1, api_done: 2, mark_all_done: 3, mark_done: 4 }, _prefix: :resolved_by diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb index 39ce26526e6..ed5a0f24ed0 100644 --- a/app/policies/issuable_policy.rb +++ b/app/policies/issuable_policy.rb @@ -17,7 +17,9 @@ class IssuablePolicy < BasePolicy enable :read_issue enable :update_issue enable :reopen_issue - enable :read_merge_request + end + + rule { can?(:read_merge_request) & assignee_or_author }.policy do enable :update_merge_request enable :reopen_merge_request end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index d81db357162..b3aa49a00ae 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -93,6 +93,11 @@ class ProjectPolicy < BasePolicy user.is_a?(DeployToken) && user.has_access_to?(project) && user.write_package_registry end + desc "Deploy token with read access" + condition(:download_code_deploy_token) do + user.is_a?(DeployToken) && user.has_access_to?(project) + end + desc "If user is authenticated via CI job token then the target project should be in scope" condition(:project_allowed_for_job_token) do !@user&.from_ci_job_token? || @user.ci_job_token_scope.includes?(project) @@ -506,6 +511,10 @@ class ProjectPolicy < BasePolicy prevent(:download_wiki_code) end + rule { download_code_deploy_token }.policy do + enable :download_wiki_code + end + rule { builds_disabled | repository_disabled }.policy do prevent(*create_read_update_admin_destroy(:build)) prevent(*create_read_update_admin_destroy(:pipeline_schedule)) @@ -687,12 +696,14 @@ class ProjectPolicy < BasePolicy rule { project_bot }.enable :project_bot_access + rule { can?(:read_all_resources) }.enable :read_resource_access_tokens + rule { can?(:admin_project) & resource_access_token_feature_available }.policy do enable :read_resource_access_tokens enable :destroy_resource_access_tokens end - rule { can?(:read_resource_access_tokens) & resource_access_token_creation_allowed }.policy do + rule { can?(:admin_project) & resource_access_token_feature_available & resource_access_token_creation_allowed }.policy do enable :create_resource_access_tokens end diff --git a/app/services/protected_branches/base_service.rb b/app/services/protected_branches/base_service.rb index f48e02ab4b5..df801311aaf 100644 --- a/app/services/protected_branches/base_service.rb +++ b/app/services/protected_branches/base_service.rb @@ -13,5 +13,23 @@ module ProtectedBranches def after_execute(*) # overridden in EE::ProtectedBranches module end + + def filtered_params + return unless params + + params[:name] = sanitize_branch_name(params[:name]) if params[:name].present? + params + end + + private + + def sanitize_branch_name(name) + name = CGI.unescapeHTML(name) + name = Sanitize.fragment(name) + + # Sanitize.fragment escapes HTML chars, so unescape again to allow names + # like `feature->master` + CGI.unescapeHTML(name) + end end end diff --git a/app/services/protected_branches/create_service.rb b/app/services/protected_branches/create_service.rb index dada449989a..ea494dd4426 100644 --- a/app/services/protected_branches/create_service.rb +++ b/app/services/protected_branches/create_service.rb @@ -21,7 +21,7 @@ module ProtectedBranches end def protected_branch - @protected_branch ||= project.protected_branches.new(params) + @protected_branch ||= project.protected_branches.new(filtered_params) end end end diff --git a/app/services/protected_branches/update_service.rb b/app/services/protected_branches/update_service.rb index 1e70f2d9793..40e9a286af9 100644 --- a/app/services/protected_branches/update_service.rb +++ b/app/services/protected_branches/update_service.rb @@ -8,7 +8,7 @@ module ProtectedBranches old_merge_access_levels = protected_branch.merge_access_levels.map(&:clone) old_push_access_levels = protected_branch.push_access_levels.map(&:clone) - if protected_branch.update(params) + if protected_branch.update(filtered_params) after_execute(protected_branch: protected_branch, old_merge_access_levels: old_merge_access_levels, old_push_access_levels: old_push_access_levels) end 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/app/views/layouts/_search.html.haml b/app/views/layouts/_search.html.haml index 2d186dfbd91..0350dc82e46 100644 --- a/app/views/layouts/_search.html.haml +++ b/app/views/layouts/_search.html.haml @@ -29,8 +29,9 @@ = hidden_field_tag :scope, search_context.scope = hidden_field_tag :search_code, search_context.code_search? + - ref = search_context.ref if can?(current_user, :download_code, search_context.project) = hidden_field_tag :snippets, search_context.for_snippets? - = hidden_field_tag :repository_ref, search_context.ref + = hidden_field_tag :repository_ref, ref = hidden_field_tag :nav_source, 'navbar' -# workaround for non-JS feature specs, see spec/support/helpers/search_helpers.rb @@ -38,4 +39,4 @@ %noscript= button_tag 'Search' .search-autocomplete-opts.hide{ :'data-autocomplete-path' => search_autocomplete_path, :'data-autocomplete-project-id' => search_context.project.try(:id), - :'data-autocomplete-project-ref' => search_context.ref } + :'data-autocomplete-project-ref' => ref } diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index e515f1e7320..1cbb061784e 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -1,5 +1,4 @@ - current_route_path = request.fullpath.match(%r{-/tree/[^/]+/(.+$)}).to_a[1] -- add_page_startup_graphql_call('repository/path_last_commit', { projectPath: @project.full_path, ref: current_ref, path: current_route_path || "" }) - @content_class = "limit-container-width" unless fluid_layout - @skip_current_level_breadcrumb = true - add_page_specific_style 'page_bundles/project' @@ -14,6 +13,7 @@ = render "home_panel" - if can?(current_user, :download_code, @project) && @project.repository_languages.present? + - add_page_startup_graphql_call('repository/path_last_commit', { projectPath: @project.full_path, ref: current_ref, path: current_route_path || "" }) = repository_languages_bar(@project.repository_languages) = render "archived_notice", project: @project diff --git a/config/application.rb b/config/application.rb index dde1eae30e7..f64e5c998eb 100644 --- a/config/application.rb +++ b/config/application.rb @@ -411,6 +411,7 @@ module Gitlab config.cache_store = :redis_cache_store, Gitlab::Redis::Cache.active_support_config config.active_job.queue_adapter = :sidekiq + config.active_job.logger = nil config.action_mailer.deliver_later_queue_name = :mailers # This is needed for gitlab-shell diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 34af9736056..905b6d418a9 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -11651,7 +11651,7 @@ A user assigned to a merge request. | <a id="mergerequestassigneeid"></a>`id` | [`ID!`](#id) | ID of the user. | | <a id="mergerequestassigneelocation"></a>`location` | [`String`](#string) | Location of the user. | | <a id="mergerequestassigneemergerequestinteraction"></a>`mergeRequestInteraction` | [`UserMergeRequestInteraction`](#usermergerequestinteraction) | Details of this user's interactions with the merge request. | -| <a id="mergerequestassigneename"></a>`name` | [`String!`](#string) | Human-readable name of the user. | +| <a id="mergerequestassigneename"></a>`name` | [`String!`](#string) | Human-readable name of the user. Will return `****` if the user is a project bot and the requester does not have permission to read resource access tokens. | | <a id="mergerequestassigneenamespace"></a>`namespace` | [`Namespace`](#namespace) | Personal namespace of the user. | | <a id="mergerequestassigneeprojectmemberships"></a>`projectMemberships` | [`ProjectMemberConnection`](#projectmemberconnection) | Project memberships of the user. (see [Connections](#connections)) | | <a id="mergerequestassigneepublicemail"></a>`publicEmail` | [`String`](#string) | User's public email. | @@ -11903,7 +11903,7 @@ A user assigned to a merge request as a reviewer. | <a id="mergerequestreviewerid"></a>`id` | [`ID!`](#id) | ID of the user. | | <a id="mergerequestreviewerlocation"></a>`location` | [`String`](#string) | Location of the user. | | <a id="mergerequestreviewermergerequestinteraction"></a>`mergeRequestInteraction` | [`UserMergeRequestInteraction`](#usermergerequestinteraction) | Details of this user's interactions with the merge request. | -| <a id="mergerequestreviewername"></a>`name` | [`String!`](#string) | Human-readable name of the user. | +| <a id="mergerequestreviewername"></a>`name` | [`String!`](#string) | Human-readable name of the user. Will return `****` if the user is a project bot and the requester does not have permission to read resource access tokens. | | <a id="mergerequestreviewernamespace"></a>`namespace` | [`Namespace`](#namespace) | Personal namespace of the user. | | <a id="mergerequestreviewerprojectmemberships"></a>`projectMemberships` | [`ProjectMemberConnection`](#projectmemberconnection) | Project memberships of the user. (see [Connections](#connections)) | | <a id="mergerequestreviewerpublicemail"></a>`publicEmail` | [`String`](#string) | User's public email. | @@ -14970,7 +14970,7 @@ Core represention of a GitLab user. | <a id="usercoregroupmemberships"></a>`groupMemberships` | [`GroupMemberConnection`](#groupmemberconnection) | Group memberships of the user. (see [Connections](#connections)) | | <a id="usercoreid"></a>`id` | [`ID!`](#id) | ID of the user. | | <a id="usercorelocation"></a>`location` | [`String`](#string) | Location of the user. | -| <a id="usercorename"></a>`name` | [`String!`](#string) | Human-readable name of the user. | +| <a id="usercorename"></a>`name` | [`String!`](#string) | Human-readable name of the user. Will return `****` if the user is a project bot and the requester does not have permission to read resource access tokens. | | <a id="usercorenamespace"></a>`namespace` | [`Namespace`](#namespace) | Personal namespace of the user. | | <a id="usercoreprojectmemberships"></a>`projectMemberships` | [`ProjectMemberConnection`](#projectmemberconnection) | Project memberships of the user. (see [Connections](#connections)) | | <a id="usercorepublicemail"></a>`publicEmail` | [`String`](#string) | User's public email. | @@ -18058,7 +18058,7 @@ Implementations: | <a id="usergroupmemberships"></a>`groupMemberships` | [`GroupMemberConnection`](#groupmemberconnection) | Group memberships of the user. (see [Connections](#connections)) | | <a id="userid"></a>`id` | [`ID!`](#id) | ID of the user. | | <a id="userlocation"></a>`location` | [`String`](#string) | Location of the user. | -| <a id="username"></a>`name` | [`String!`](#string) | Human-readable name of the user. | +| <a id="username"></a>`name` | [`String!`](#string) | Human-readable name of the user. Will return `****` if the user is a project bot and the requester does not have permission to read resource access tokens. | | <a id="usernamespace"></a>`namespace` | [`Namespace`](#namespace) | Personal namespace of the user. | | <a id="userprojectmemberships"></a>`projectMemberships` | [`ProjectMemberConnection`](#projectmemberconnection) | Project memberships of the user. (see [Connections](#connections)) | | <a id="userpublicemail"></a>`publicEmail` | [`String`](#string) | User's public email. | diff --git a/doc/user/packages/maven_repository/index.md b/doc/user/packages/maven_repository/index.md index 1ca4bb2759d..6646f18e6fe 100644 --- a/doc/user/packages/maven_repository/index.md +++ b/doc/user/packages/maven_repository/index.md @@ -806,7 +806,7 @@ When the pipeline is successful, the package is created. The version string is validated by using the following regex. ```ruby -\A(\.?[\w\+-]+\.?)+\z +\A(?!.*\.\.)[\w+.-]+\z ``` You can play around with the regex and try your version strings on [this regular expression editor](https://rubular.com/r/rrLQqUXjfKEoL6). diff --git a/lib/api/entities/project.rb b/lib/api/entities/project.rb index e3f1e90b80f..662ca59852e 100644 --- a/lib/api/entities/project.rb +++ b/lib/api/entities/project.rb @@ -55,7 +55,9 @@ module API expose(:snippets_enabled) { |project, options| project.feature_available?(:snippets, options[:current_user]) } expose(:container_registry_enabled) { |project, options| project.feature_available?(:container_registry, options[:current_user]) } expose :service_desk_enabled - expose :service_desk_address + expose :service_desk_address, if: -> (project, options) do + Ability.allowed?(options[:current_user], :admin_issue, project) + end expose(:can_create_merge_request_in) do |project, options| Ability.allowed?(options[:current_user], :create_merge_request_in, project) diff --git a/lib/api/entities/user_safe.rb b/lib/api/entities/user_safe.rb index feb01767fd6..6006a076020 100644 --- a/lib/api/entities/user_safe.rb +++ b/lib/api/entities/user_safe.rb @@ -3,7 +3,17 @@ module API module Entities class UserSafe < Grape::Entity - expose :id, :name, :username + expose :id, :username + expose :name do |user| + next user.name unless user.project_bot? + + next user.name if options[:current_user]&.can?(:read_resource_access_tokens, user.projects.first) + + # If the requester does not have permission to read the project bot name, + # the API returns an arbitrary string. UI changes will be addressed in a follow up issue: + # https://gitlab.com/gitlab-org/gitlab/-/issues/346058 + '****' + end end end end 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/api/merge_request_approvals.rb b/lib/api/merge_request_approvals.rb index dd49624c74f..71ca8331ed6 100644 --- a/lib/api/merge_request_approvals.rb +++ b/lib/api/merge_request_approvals.rb @@ -26,8 +26,6 @@ module API # GET /projects/:id/merge_requests/:merge_request_iid/approvals desc 'List approvals for merge request' get 'approvals', urgency: :low do - not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) - merge_request = find_merge_request_with_access(params[:merge_request_iid]) present_approval(merge_request) diff --git a/lib/api/merge_request_diffs.rb b/lib/api/merge_request_diffs.rb index 470f78a7dc2..8fa7138af42 100644 --- a/lib/api/merge_request_diffs.rb +++ b/lib/api/merge_request_diffs.rb @@ -23,8 +23,6 @@ module API use :pagination end get ":id/merge_requests/:merge_request_iid/versions" do - not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) - merge_request = find_merge_request_with_access(params[:merge_request_iid]) present paginate(merge_request.merge_request_diffs.order_id_desc), with: Entities::MergeRequestDiff @@ -41,8 +39,6 @@ module API end get ":id/merge_requests/:merge_request_iid/versions/:version_id" do - not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) - merge_request = find_merge_request_with_access(params[:merge_request_iid]) present_cached merge_request.merge_request_diffs.find(params[:version_id]), with: Entities::MergeRequestDiffFull, cache_context: nil diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 21c1b7969aa..96d1a69c03a 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -264,8 +264,6 @@ module API success Entities::MergeRequest end get ':id/merge_requests/:merge_request_iid', feature_category: :code_review do - not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) - merge_request = find_merge_request_with_access(params[:merge_request_iid]) present merge_request, @@ -282,8 +280,6 @@ module API success Entities::UserBasic end get ':id/merge_requests/:merge_request_iid/participants', feature_category: :code_review do - not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) - merge_request = find_merge_request_with_access(params[:merge_request_iid]) participants = ::Kaminari.paginate_array(merge_request.participants) @@ -295,8 +291,6 @@ module API success Entities::Commit end get ':id/merge_requests/:merge_request_iid/commits', feature_category: :code_review do - not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) - merge_request = find_merge_request_with_access(params[:merge_request_iid]) commits = @@ -378,8 +372,6 @@ module API success Entities::MergeRequestChanges end get ':id/merge_requests/:merge_request_iid/changes', feature_category: :code_review do - not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) - merge_request = find_merge_request_with_access(params[:merge_request_iid]) present merge_request, @@ -395,8 +387,6 @@ module API get ':id/merge_requests/:merge_request_iid/pipelines', feature_category: :continuous_integration do pipelines = merge_request_pipelines_with_access - not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) - present paginate(pipelines), with: Entities::Ci::PipelineBasic end diff --git a/lib/api/todos.rb b/lib/api/todos.rb index 57a6ee0bebb..1bc3e25a46c 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -29,10 +29,6 @@ module API post ":id/#{type}/:#{type_id_str}/todo" do issuable = instance_exec(params[type_id_str], &finder) - unless can?(current_user, :read_merge_request, issuable.project) - not_found!(type.split("_").map(&:capitalize).join(" ")) - end - todo = TodoService.new.mark_todo(issuable, current_user).first if todo 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>#{DELIM})[ \t]*(?<lang>\S*) # opening front matter marker (optional language specifier) - \s* - ^(?<front_matter>.*?) # front matter block content (not greedy) - \s* - ^(\k<delim> | \.{3}) # closing front matter marker + ^(?<delim>#{DELIM})[ \t]*(?<lang>\S*)\R # opening front matter marker (optional language specifier) + (?<front_matter>.*?) # front matter block content (not greedy) + ^(\k<delim> | \.{3}) # closing front matter marker \s* }mx.freeze end diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb index 0963eb6b72a..f8f61511265 100644 --- a/lib/gitlab/git_access_wiki.rb +++ b/lib/gitlab/git_access_wiki.rb @@ -27,6 +27,13 @@ module Gitlab :create_wiki end + override :check_download_access! + def check_download_access! + super + + raise ForbiddenError, download_forbidden_message if deploy_token && !deploy_token.can?(:download_wiki_code, container) + end + override :check_change_access! def check_change_access! raise ForbiddenError, write_to_wiki_message unless user_can_push? diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb index ce886cb8738..dd7ec361dd8 100644 --- a/lib/gitlab/import_export/members_mapper.rb +++ b/lib/gitlab/import_export/members_mapper.rb @@ -52,11 +52,20 @@ module Gitlab @importable.members.destroy_all # rubocop: disable Cop/DestroyAll - relation_class.create!(user: @user, access_level: highest_access_level, source_id: @importable.id, importing: true) + relation_class.create!(user: @user, access_level: importer_access_level, source_id: @importable.id, importing: true) rescue StandardError => e raise e, "Error adding importer user to #{@importable.class} members. #{e.message}" end + def importer_access_level + if @importable.parent.is_a?(::Group) && !@user.admin? + lvl = @importable.parent.max_member_access_for_user(@user, only_concrete_membership: true) + [lvl, highest_access_level].min + else + highest_access_level + end + end + def user_already_member? member = @importable.members&.first diff --git a/lib/gitlab/quick_actions/extractor.rb b/lib/gitlab/quick_actions/extractor.rb index 1294e475145..2e4817e6b17 100644 --- a/lib/gitlab/quick_actions/extractor.rb +++ b/lib/gitlab/quick_actions/extractor.rb @@ -29,9 +29,7 @@ module Gitlab # Anything, including `/cmd arg` which are ignored by this filter # ` - `\n* - .+? - \n*` + `.+?` ) }mix.freeze diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 8b2f786a91a..904fc744c6b 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -57,7 +57,7 @@ module Gitlab end def maven_version_regex - @maven_version_regex ||= /\A(\.?[\w\+-]+\.?)+\z/.freeze + @maven_version_regex ||= /\A(?!.*\.\.)[\w+.-]+\z/.freeze end def maven_app_group_regex diff --git a/lib/gitlab/slash_commands/deploy.rb b/lib/gitlab/slash_commands/deploy.rb index 157d924f99f..9fcefd99f81 100644 --- a/lib/gitlab/slash_commands/deploy.rb +++ b/lib/gitlab/slash_commands/deploy.rb @@ -3,8 +3,18 @@ module Gitlab module SlashCommands class Deploy < BaseCommand + DEPLOY_REGEX = /\Adeploy\s/.freeze + def self.match(text) - /\Adeploy\s+(?<from>\S+.*)\s+to+\s+(?<to>\S+.*)\z/.match(text) + return unless text&.match?(DEPLOY_REGEX) + + from, _, to = text.sub(DEPLOY_REGEX, '').rpartition(/\sto+\s/) + return if from.blank? || to.blank? + + { + from: from.strip, + to: to.strip + } end def self.help_message 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/lib/sidebars/projects/menus/analytics_menu.rb b/lib/sidebars/projects/menus/analytics_menu.rb index b13b25d1cfe..2a89dc66219 100644 --- a/lib/sidebars/projects/menus/analytics_menu.rb +++ b/lib/sidebars/projects/menus/analytics_menu.rb @@ -60,7 +60,7 @@ module Sidebars end def repository_analytics_menu_item - if context.project.empty_repo? + if context.project.empty_repo? || !can?(context.current_user, :read_repository_graphs, context.project) return ::Sidebars::NilMenuItem.new(item_id: :repository_analytics) end 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/package.json b/package.json index 943a176ce75..5016d75f74c 100644 --- a/package.json +++ b/package.json @@ -148,7 +148,7 @@ "lowlight": "^1.20.0", "marked": "^0.3.12", "mathjax": "3", - "mermaid": "^8.13.2", + "mermaid": "^8.13.4", "minimatch": "^3.0.4", "monaco-editor": "^0.25.2", "monaco-editor-webpack-plugin": "^4.0.0", diff --git a/spec/controllers/dashboard/todos_controller_spec.rb b/spec/controllers/dashboard/todos_controller_spec.rb index cf528b414c0..abada97fb10 100644 --- a/spec/controllers/dashboard/todos_controller_spec.rb +++ b/spec/controllers/dashboard/todos_controller_spec.rb @@ -65,7 +65,7 @@ RSpec.describe Dashboard::TodosController do project_2 = create(:project, namespace: user.namespace) project_2.add_developer(user) merge_request_2 = create(:merge_request, source_project: project_2) - create(:todo, project: project, author: author, user: user, target: merge_request_2) + create(:todo, project: project_2, author: author, user: user, target: merge_request_2) expect { get :index }.not_to exceed_query_limit(control) expect(response).to have_gitlab_http_status(:ok) diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index 6e7bcfdaa08..f9b15c9a48e 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -52,6 +52,44 @@ RSpec.describe GraphqlController do expect(response).to have_gitlab_http_status(:ok) end + it 'executes a simple query with no errors' do + post :execute, params: { query: '{ __typename }' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'data' => { '__typename' => 'Query' } }) + end + + it 'executes a simple multiplexed query with no errors' do + multiplex = [{ query: '{ __typename }' }] * 2 + + post :execute, params: { _json: multiplex } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq([ + { 'data' => { '__typename' => 'Query' } }, + { 'data' => { '__typename' => 'Query' } } + ]) + end + + it 'sets a limit on the total query size' do + graphql_query = "{#{(['__typename'] * 1000).join(' ')}}" + + post :execute, params: { query: graphql_query } + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response).to eq({ 'errors' => [{ 'message' => 'Query too large' }] }) + end + + it 'sets a limit on the total query size for multiplex queries' do + graphql_query = "{#{(['__typename'] * 200).join(' ')}}" + multiplex = [{ query: graphql_query }] * 5 + + post :execute, params: { _json: multiplex } + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response).to eq({ 'errors' => [{ 'message' => 'Query too large' }] }) + end + it 'returns forbidden when user cannot access API' do # User cannot access API in a couple of cases # * When user is internal(like ghost users) 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/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 8281e82959b..9d05c985af1 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -7,8 +7,8 @@ RSpec.describe 'File blob', :js do let(:project) { create(:project, :public, :repository) } - def visit_blob(path, anchor: nil, ref: 'master') - visit project_blob_path(project, File.join(ref, path), anchor: anchor) + def visit_blob(path, anchor: nil, ref: 'master', **additional_args) + visit project_blob_path(project, File.join(ref, path), anchor: anchor, **additional_args) wait_for_requests end @@ -1501,6 +1501,53 @@ RSpec.describe 'File blob', :js do end end end + + context 'openapi.yml' do + before do + file_name = 'openapi.yml' + + create_file(file_name, ' + swagger: \'2.0\' + info: + title: Classic API Resource Documentation + description: | + <div class="foo-bar" style="background-color: red;" data-foo-bar="baz"> + <h1>Swagger API documentation</h1> + </div> + version: production + basePath: /JSSResource/ + produces: + - application/xml + - application/json + consumes: + - application/xml + - application/json + security: + - basicAuth: [] + paths: + /accounts: + get: + responses: + \'200\': + description: No response was specified + tags: + - accounts + operationId: findAccounts + summary: Finds all accounts + ') + visit_blob(file_name, useUnsafeMarkdown: '1') + click_button('Display rendered file') + + wait_for_requests + end + + it 'removes `style`, `class`, and `data-*`` attributes from HTML' do + expect(page).to have_css('h1', text: 'Swagger API documentation') + expect(page).not_to have_css('.foo-bar') + expect(page).not_to have_css('[style="background-color: red;"]') + expect(page).not_to have_css('[data-foo-bar="baz"]') + end + end end end diff --git a/spec/features/projects/members/list_spec.rb b/spec/features/projects/members/list_spec.rb index 25598146604..308098c72a1 100644 --- a/spec/features/projects/members/list_spec.rb +++ b/spec/features/projects/members/list_spec.rb @@ -147,7 +147,7 @@ RSpec.describe 'Project members list', :js do it 'does not show form used to change roles and "Expiration date" or the remove user button', :aggregate_failures do visit_members_page - page.within find_member_row(project_bot) do + page.within find_username_row(project_bot) do expect(page).not_to have_button('Maintainer') expect(page).to have_field('Expiration date', disabled: true) expect(page).not_to have_button('Remove member') diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index c4619b5498e..26deca9c8f1 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -383,6 +383,24 @@ RSpec.describe 'Project' do { form: '.rspec-merge-request-settings', input: '#project_printing_merge_request_link_enabled' }] end + describe 'view for a user without an access to a repo' do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + + it 'does not contain default branch information in its content' do + default_branch = 'merge-commit-analyze-side-branch' + + project.add_guest(user) + project.change_head(default_branch) + + sign_in(user) + visit project_path(project) + + lines_with_default_branch = page.html.lines.select { |line| line.include?(default_branch) } + expect(lines_with_default_branch).to eq([]) + end + end + def remove_with_confirm(button_text, confirm_with, confirm_button_text = 'Confirm') click_button button_text fill_in 'confirm_name_input', with: confirm_with diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index 6fbed21acdb..15ec11c256f 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -118,12 +118,12 @@ RSpec.describe 'Protected Branches', :js do it "allows creating explicit protected branches" do visit project_protected_branches_path(project) set_defaults - set_protected_branch_name('some-branch') + set_protected_branch_name('some->branch') click_on "Protect" - within(".protected-branches-list") { expect(page).to have_content('some-branch') } + within(".protected-branches-list") { expect(page).to have_content('some->branch') } expect(ProtectedBranch.count).to eq(1) - expect(ProtectedBranch.last.name).to eq('some-branch') + expect(ProtectedBranch.last.name).to eq('some->branch') end it "displays the last commit on the matching branch if it exists" do 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/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index 3fa0dc95126..02c686af688 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -35,6 +35,10 @@ RSpec.describe GitlabSchema do expect(connection).to eq(Gitlab::Graphql::Pagination::ExternallyPaginatedArrayConnection) end + it 'sets an appropriate validation timeout' do + expect(described_class.validate_timeout).to be <= 0.2.seconds + end + describe '.execute' do describe 'setting query `max_complexity` and `max_depth`' do subject(:result) { described_class.execute('query', **kwargs).query } @@ -195,6 +199,36 @@ RSpec.describe GitlabSchema do end end + describe 'validate_max_errors' do + it 'reports at most 5 errors' do + query = <<~GQL + query { + currentUser { + x: id + x: bot + x: username + x: state + x: name + + x: id + x: bot + x: username + x: state + x: name + + badField + veryBadField + alsoNotAGoodField + } + } + GQL + + result = described_class.execute(query) + + expect(result.to_h['errors'].count).to eq 5 + end + end + describe '.parse_gid' do let_it_be(:global_id) { 'gid://gitlab/TestOne/2147483647' } diff --git a/spec/graphql/types/user_type_spec.rb b/spec/graphql/types/user_type_spec.rb index 0bad8c95ba2..4e3f442dc71 100644 --- a/spec/graphql/types/user_type_spec.rb +++ b/spec/graphql/types/user_type_spec.rb @@ -44,6 +44,86 @@ RSpec.describe GitlabSchema.types['User'] do expect(described_class).to have_graphql_fields(*expected_fields) end + describe 'name field' do + let_it_be(:admin) { create(:user, :admin)} + let_it_be(:user) { create(:user) } + let_it_be(:requested_user) { create(:user, name: 'John Smith') } + let_it_be(:requested_project_bot) { create(:user, :project_bot, name: 'Project bot') } + let_it_be(:project) { create(:project, :public) } + + before do + project.add_maintainer(requested_project_bot) + end + + let(:username) { requested_user.username } + + let(:query) do + %( + query { + user(username: "#{username}") { + name + } + } + ) + end + + subject { GitlabSchema.execute(query, context: { current_user: current_user }).as_json.dig('data', 'user', 'name') } + + context 'user requests' do + let(:current_user) { user } + + context 'a user' do + it 'returns name' do + expect(subject).to eq('John Smith') + end + end + + context 'a project bot' do + let(:username) { requested_project_bot.username } + + context 'when requester is nil' do + let(:current_user) { nil } + + it 'returns `****`' do + expect(subject).to eq('****') + end + end + + it 'returns `****` for a regular user' do + expect(subject).to eq('****') + end + + context 'when requester is a project maintainer' do + before do + project.add_maintainer(user) + end + + it 'returns name' do + expect(subject).to eq('Project bot') + end + end + end + end + + context 'admin requests', :enable_admin_mode do + let(:current_user) { admin } + + context 'a user' do + it 'returns name' do + expect(subject).to eq('John Smith') + end + end + + context 'a project bot' do + let(:username) { requested_project_bot.username } + + it 'returns name' do + expect(subject).to eq('Project bot') + end + end + end + end + describe 'snippets field' do subject { described_class.fields['snippets'] } diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb index 9e870658870..17dcbab09bb 100644 --- a/spec/helpers/search_helper_spec.rb +++ b/spec/helpers/search_helper_spec.rb @@ -174,12 +174,26 @@ RSpec.describe SearchHelper do context "with a current project" do before do @project = create(:project, :repository) + + allow(self).to receive(:can?).and_return(true) allow(self).to receive(:can?).with(user, :read_feature_flag, @project).and_return(false) end - it "includes project-specific sections", :aggregate_failures do + it 'returns repository related labels based on users abilities', :aggregate_failures do expect(search_autocomplete_opts("Files").size).to eq(1) expect(search_autocomplete_opts("Commits").size).to eq(1) + expect(search_autocomplete_opts("Network").size).to eq(1) + expect(search_autocomplete_opts("Graph").size).to eq(1) + + allow(self).to receive(:can?).with(user, :download_code, @project).and_return(false) + + expect(search_autocomplete_opts("Files").size).to eq(0) + expect(search_autocomplete_opts("Commits").size).to eq(0) + + allow(self).to receive(:can?).with(user, :read_repository_graphs, @project).and_return(false) + + expect(search_autocomplete_opts("Network").size).to eq(0) + expect(search_autocomplete_opts("Graph").size).to eq(0) end context 'when user does not have access to project' do diff --git a/spec/lib/api/entities/project_spec.rb b/spec/lib/api/entities/project_spec.rb index 8d1c3aa878d..6b542278fa6 100644 --- a/spec/lib/api/entities/project_spec.rb +++ b/spec/lib/api/entities/project_spec.rb @@ -13,6 +13,28 @@ RSpec.describe ::API::Entities::Project do subject(:json) { entity.as_json } + describe '.service_desk_address' do + before do + allow(project).to receive(:service_desk_enabled?).and_return(true) + end + + context 'when a user can admin issues' do + before do + project.add_reporter(current_user) + end + + it 'is present' do + expect(json[:service_desk_address]).to be_present + end + end + + context 'when a user can not admin project' do + it 'is empty' do + expect(json[:service_desk_address]).to be_nil + end + end + end + describe '.shared_with_groups' do let(:group) { create(:group, :private) } diff --git a/spec/lib/api/entities/user_spec.rb b/spec/lib/api/entities/user_spec.rb index 9c9a157d68a..14dc60e1a5f 100644 --- a/spec/lib/api/entities/user_spec.rb +++ b/spec/lib/api/entities/user_spec.rb @@ -12,7 +12,7 @@ RSpec.describe API::Entities::User do subject { entity.as_json } it 'exposes correct attributes' do - expect(subject).to include(:bio, :location, :public_email, :skype, :linkedin, :twitter, :website_url, :organization, :job_title, :work_information, :pronouns) + expect(subject).to include(:name, :bio, :location, :public_email, :skype, :linkedin, :twitter, :website_url, :organization, :job_title, :work_information, :pronouns) end it 'exposes created_at if the current user can read the user profile' do @@ -31,12 +31,51 @@ RSpec.describe API::Entities::User do expect(subject[:bot]).to be_falsey end - context 'with bot user' do - let(:user) { create(:user, :security_bot) } + context 'with project bot user' do + let(:project) { create(:project) } + let(:user) { create(:user, :project_bot, name: 'secret') } + + before do + project.add_maintainer(user) + end it 'exposes user as a bot' do expect(subject[:bot]).to eq(true) end + + context 'when the requester is not an admin' do + it 'does not expose project bot user name' do + expect(subject[:name]).to eq('****') + end + end + + context 'when the requester is nil' do + let(:current_user) { nil } + + it 'does not expose project bot user name' do + expect(subject[:name]).to eq('****') + end + end + + context 'when the requester is a project maintainer' do + let(:current_user) { create(:user) } + + before do + project.add_maintainer(current_user) + end + + it 'exposes project bot user name' do + expect(subject[:name]).to eq('secret') + end + end + + context 'when the requester is an admin' do + let(:current_user) { create(:user, :admin) } + + it 'exposes project bot user name', :enable_admin_mode do + expect(subject[:name]).to eq('secret') + end + end end it 'exposes local_time' do 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/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb index 5ada8a6ef40..27175dc8c44 100644 --- a/spec/lib/gitlab/git_access_wiki_spec.rb +++ b/spec/lib/gitlab/git_access_wiki_spec.rb @@ -79,5 +79,30 @@ RSpec.describe Gitlab::GitAccessWiki do let(:message) { include('wiki') } end end + + context 'when the actor is a deploy token' do + let_it_be(:actor) { create(:deploy_token, projects: [project]) } + let_it_be(:user) { actor } + + before do + project.project_feature.update_attribute(:wiki_access_level, wiki_access_level) + end + + subject { access.check('git-upload-pack', changes) } + + context 'when the wiki is enabled' do + let(:wiki_access_level) { ProjectFeature::ENABLED } + + it { expect { subject }.not_to raise_error } + end + + context 'when the wiki is disabled' do + let(:wiki_access_level) { ProjectFeature::DISABLED } + + it_behaves_like 'forbidden git access' do + let(:message) { 'You are not allowed to download files from this wiki.' } + end + end + end end end diff --git a/spec/lib/gitlab/import_export/group/relation_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/group/relation_tree_restorer_spec.rb index 473dbf5ecc5..ce6607f6a26 100644 --- a/spec/lib/gitlab/import_export/group/relation_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/group/relation_tree_restorer_spec.rb @@ -10,8 +10,8 @@ require 'spec_helper' RSpec.describe Gitlab::ImportExport::Group::RelationTreeRestorer do - let_it_be(:group) { create(:group) } - let_it_be(:importable) { create(:group, parent: group) } + let(:group) { create(:group).tap { |g| g.add_owner(user) } } + let(:importable) { create(:group, parent: group) } include_context 'relation tree restorer shared context' do let(:importable_name) { nil } diff --git a/spec/lib/gitlab/import_export/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb index 847d6b5d1ed..8b9ca90a280 100644 --- a/spec/lib/gitlab/import_export/members_mapper_spec.rb +++ b/spec/lib/gitlab/import_export/members_mapper_spec.rb @@ -267,6 +267,66 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do end end + context 'when importer is not an admin' do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:members_mapper) do + described_class.new( + exported_members: [], user: user, importable: importable) + end + + shared_examples_for 'it fetches the access level from parent group' do + before do + group.add_users([user], group_access_level) + end + + it "and resolves it correctly" do + members_mapper.map + expect(member_class.find_by_user_id(user.id).access_level).to eq(resolved_access_level) + end + end + + context 'and the imported project is part of a group' do + let(:importable) { create(:project, namespace: group) } + let(:member_class) { ProjectMember } + + it_behaves_like 'it fetches the access level from parent group' do + let(:group_access_level) { GroupMember::DEVELOPER } + let(:resolved_access_level) { ProjectMember::DEVELOPER } + end + + it_behaves_like 'it fetches the access level from parent group' do + let(:group_access_level) { GroupMember::MAINTAINER } + let(:resolved_access_level) { ProjectMember::MAINTAINER } + end + + it_behaves_like 'it fetches the access level from parent group' do + let(:group_access_level) { GroupMember::OWNER } + let(:resolved_access_level) { ProjectMember::MAINTAINER } + end + end + + context 'and the imported group is part of another group' do + let(:importable) { create(:group, parent: group) } + let(:member_class) { GroupMember } + + it_behaves_like 'it fetches the access level from parent group' do + let(:group_access_level) { GroupMember::DEVELOPER } + let(:resolved_access_level) { GroupMember::DEVELOPER } + end + + it_behaves_like 'it fetches the access level from parent group' do + let(:group_access_level) { GroupMember::MAINTAINER } + let(:resolved_access_level) { GroupMember::MAINTAINER } + end + + it_behaves_like 'it fetches the access level from parent group' do + let(:group_access_level) { GroupMember::OWNER } + let(:resolved_access_level) { GroupMember::OWNER } + end + end + end + context 'when importable is Group' do include_examples 'imports exported members' do let(:source_type) { 'Namespace' } diff --git a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb index 49df2313924..80ba50976af 100644 --- a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Gitlab::ImportExport::Project::RelationFactory, :use_clean_rails_memory_store_caching do - let(:group) { create(:group) } + let(:group) { create(:group).tap { |g| g.add_maintainer(importer_user) } } let(:project) { create(:project, :repository, group: group) } let(:members_mapper) { double('members_mapper').as_null_object } let(:admin) { create(:admin) } diff --git a/spec/lib/gitlab/import_export/project/relation_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/relation_tree_restorer_spec.rb index 5ebace263ba..577f1e46db6 100644 --- a/spec/lib/gitlab/import_export/project/relation_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/relation_tree_restorer_spec.rb @@ -101,7 +101,7 @@ RSpec.describe Gitlab::ImportExport::Project::RelationTreeRestorer do it_behaves_like 'import project successfully' context 'with logging of relations creation' do - let_it_be(:group) { create(:group) } + let_it_be(:group) { create(:group).tap { |g| g.add_maintainer(user) } } let_it_be(:importable) do create(:project, :builds_enabled, :issues_disabled, name: 'project', path: 'project', group: group) end @@ -118,7 +118,7 @@ RSpec.describe Gitlab::ImportExport::Project::RelationTreeRestorer do context 'when inside a group' do let_it_be(:group) do - create(:group, :disabled_and_unoverridable) + create(:group, :disabled_and_unoverridable).tap { |g| g.add_maintainer(user) } end before do diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb index cd3d29f1a51..6bb6be07749 100644 --- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb @@ -674,6 +674,7 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do # Project needs to be in a group for visibility level comparison # to happen group = create(:group) + group.add_maintainer(user) project.group = group project.create_import_data(data: { override_params: { visibility_level: Gitlab::VisibilityLevel::INTERNAL.to_s } }) @@ -715,13 +716,19 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do end context 'with a project that has a group' do + let(:group) do + create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE).tap do |g| + g.add_maintainer(user) + end + end + let!(:project) do create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project', - group: create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE)) + group: group) end before do @@ -750,13 +757,14 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do end context 'with existing group models' do + let(:group) { create(:group).tap { |g| g.add_maintainer(user) } } let!(:project) do create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project', - group: create(:group)) + group: group) end before do @@ -785,13 +793,14 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do end context 'with clashing milestones on IID' do + let(:group) { create(:group).tap { |g| g.add_maintainer(user) } } let!(:project) do create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project', - group: create(:group)) + group: group) end before do @@ -870,7 +879,7 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do context 'with group visibility' do before do group = create(:group, visibility_level: group_visibility) - + group.add_users([user], GroupMember::MAINTAINER) project.update(group: group) end diff --git a/spec/lib/gitlab/quick_actions/extractor_spec.rb b/spec/lib/gitlab/quick_actions/extractor_spec.rb index 61fffe3fb6b..c040a70e403 100644 --- a/spec/lib/gitlab/quick_actions/extractor_spec.rb +++ b/spec/lib/gitlab/quick_actions/extractor_spec.rb @@ -352,6 +352,14 @@ RSpec.describe Gitlab::QuickActions::Extractor do expect(commands).to eq(expected_commands) expect(msg).to eq expected_msg end + + it 'fails fast for strings with many newlines' do + msg = '`' + "\n" * 100_000 + + expect do + Timeout.timeout(3.seconds) { extractor.extract_commands(msg) } + end.not_to raise_error + end end describe '#redact_commands' do diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index 9514654204b..05f1c88a6ab 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -344,6 +344,18 @@ RSpec.describe Gitlab::Regex do describe '.maven_version_regex' do subject { described_class.maven_version_regex } + it 'has no ReDoS issues with long strings' do + Timeout.timeout(5) do + expect(subject).to match("aaaaaaaa.aaaaaaaaa+aa-111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111") + end + end + + it 'has no ReDos issues with long strings ending with an exclamation mark' do + Timeout.timeout(5) do + expect(subject).not_to match('a' * 50000 + '!') + end + end + it { is_expected.to match('0')} it { is_expected.to match('1') } it { is_expected.to match('03') } @@ -364,6 +376,7 @@ RSpec.describe Gitlab::Regex do it { is_expected.to match('703220b4e2cea9592caeb9f3013f6b1e5335c293') } it { is_expected.to match('RELEASE') } it { is_expected.not_to match('..1.2.3') } + it { is_expected.not_to match('1.2.3..beta') } it { is_expected.not_to match(' 1.2.3') } it { is_expected.not_to match("1.2.3 \r\t") } it { is_expected.not_to match("\r\t 1.2.3") } diff --git a/spec/lib/gitlab/slash_commands/deploy_spec.rb b/spec/lib/gitlab/slash_commands/deploy_spec.rb index 36f47c711bc..71fca1e1fc8 100644 --- a/spec/lib/gitlab/slash_commands/deploy_spec.rb +++ b/spec/lib/gitlab/slash_commands/deploy_spec.rb @@ -109,6 +109,21 @@ RSpec.describe Gitlab::SlashCommands::Deploy do end end end + + context 'with extra spaces in the deploy command' do + let(:regex_match) { described_class.match('deploy staging to production ') } + + before do + create(:ci_build, :manual, pipeline: pipeline, name: 'production', environment: 'production') + create(:ci_build, :manual, pipeline: pipeline, name: 'not prod', environment: 'not prod') + end + + it 'deploys to production' do + expect(subject[:text]) + .to start_with('Deployment started from staging to production') + expect(subject[:response_type]).to be(:in_channel) + end + end end end @@ -119,5 +134,49 @@ RSpec.describe Gitlab::SlashCommands::Deploy do expect(match[:from]).to eq('staging') expect(match[:to]).to eq('production') end + + it 'matches the environment with spaces in it' do + match = described_class.match('deploy staging env to production env') + + expect(match[:from]).to eq('staging env') + expect(match[:to]).to eq('production env') + end + + it 'matches the environment name with surrounding spaces' do + match = described_class.match('deploy staging to production ') + + # The extra spaces are stripped later in the code + expect(match[:from]).to eq('staging') + expect(match[:to]).to eq('production') + end + + it 'returns nil for text that is not a deploy command' do + match = described_class.match('foo bar') + + expect(match).to be_nil + end + + it 'returns nil for a partial command' do + match = described_class.match('deploy staging to ') + + expect(match).to be_nil + end + + context 'with ReDoS attempts' do + def duration_for(&block) + start = Time.zone.now + yield if block_given? + Time.zone.now - start + end + + it 'has smaller than linear execution time growth with a malformed "to"' do + Timeout.timeout(3.seconds) do + sample1 = duration_for { described_class.match("deploy abc t" + "o" * 1000 + "X") } + sample2 = duration_for { described_class.match("deploy abc t" + "o" * 4000 + "X") } + + expect((sample2 / sample1) < 4).to be_truthy + end + end + end end 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/lib/sidebars/projects/menus/analytics_menu_spec.rb b/spec/lib/sidebars/projects/menus/analytics_menu_spec.rb index 9d5f029fff5..6f2ca719bc9 100644 --- a/spec/lib/sidebars/projects/menus/analytics_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/analytics_menu_spec.rb @@ -102,6 +102,12 @@ RSpec.describe Sidebars::Projects::Menus::AnalyticsMenu do specify { is_expected.to be_nil } end + describe 'when a user does not have access to repository graphs' do + let(:current_user) { guest } + + specify { is_expected.to be_nil } + end + describe 'when the user does not have access' do let(:current_user) { nil } diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 6ee5219819c..44ba6e0e2fd 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -289,7 +289,6 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to allow_value('1.1-beta-2').for(:version) } it { is_expected.to allow_value('1.2-SNAPSHOT').for(:version) } it { is_expected.to allow_value('12.1.2-2-1').for(:version) } - it { is_expected.to allow_value('1.2.3..beta').for(:version) } it { is_expected.to allow_value('1.2.3-beta').for(:version) } it { is_expected.to allow_value('10.2.3-beta').for(:version) } it { is_expected.to allow_value('2.0.0.v200706041905-7C78EK9E_EkMNfNOd2d8qq').for(:version) } @@ -297,6 +296,7 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to allow_value('703220b4e2cea9592caeb9f3013f6b1e5335c293').for(:version) } it { is_expected.to allow_value('RELEASE').for(:version) } it { is_expected.not_to allow_value('..1.2.3').for(:version) } + it { is_expected.not_to allow_value('1.2.3..beta').for(:version) } it { is_expected.not_to allow_value(' 1.2.3').for(:version) } it { is_expected.not_to allow_value("1.2.3 \r\t").for(:version) } it { is_expected.not_to allow_value("\r\t 1.2.3").for(:version) } 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/policies/merge_request_policy_spec.rb b/spec/policies/merge_request_policy_spec.rb index b94df4d4374..e05de25f182 100644 --- a/spec/policies/merge_request_policy_spec.rb +++ b/spec/policies/merge_request_policy_spec.rb @@ -5,10 +5,11 @@ require 'spec_helper' RSpec.describe MergeRequestPolicy do include ExternalAuthorizationServiceHelpers - let(:guest) { create(:user) } - let(:author) { create(:user) } - let(:developer) { create(:user) } - let(:non_team_member) { create(:user) } + let_it_be(:guest) { create(:user) } + let_it_be(:author) { create(:user) } + let_it_be(:developer) { create(:user) } + let_it_be(:non_team_member) { create(:user) } + let(:project) { create(:project, :public) } def permissions(user, merge_request) @@ -50,15 +51,31 @@ RSpec.describe MergeRequestPolicy do end context 'when merge request is public' do - context 'and user is anonymous' do - let(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: author) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: author) } + context 'and user is anonymous' do subject { permissions(nil, merge_request) } it do is_expected.to be_disallowed(:create_todo, :update_subscription) end end + + describe 'the author, who became a guest' do + subject { permissions(author, merge_request) } + + it do + is_expected.to be_allowed(:update_merge_request) + end + + it do + is_expected.to be_allowed(:reopen_merge_request) + end + + it do + is_expected.to be_allowed(:approve_merge_request) + end + end end context 'when merge requests have been disabled' do @@ -107,6 +124,12 @@ RSpec.describe MergeRequestPolicy do it_behaves_like 'a denied user' end + describe 'the author' do + subject { author } + + it_behaves_like 'a denied user' + end + describe 'a developer' do subject { developer } diff --git a/spec/requests/api/graphql/user_query_spec.rb b/spec/requests/api/graphql/user_query_spec.rb index 59b805bb25b..1cba3674d25 100644 --- a/spec/requests/api/graphql/user_query_spec.rb +++ b/spec/requests/api/graphql/user_query_spec.rb @@ -488,5 +488,19 @@ RSpec.describe 'getting user information' do end end end + + context 'the user is project bot' do + let(:user) { create(:user, :project_bot) } + + before do + post_graphql(query, current_user: current_user) + end + + context 'we only request basic fields' do + let(:user_fields) { %i[id name username state web_url avatar_url] } + + it_behaves_like 'a working graphql query' + end + end end end 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/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 4f84e6f2562..cc546cbcda1 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -225,7 +225,7 @@ RSpec.describe API::Projects do create(:project, :public, group: create(:group)) end - it_behaves_like 'projects response without N + 1 queries', 0 do + it_behaves_like 'projects response without N + 1 queries', 1 do let(:current_user) { user } let(:additional_project) { create(:project, :public, group: create(:group)) } end diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index c9deb84ff98..c6b4f50afae 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -378,30 +378,36 @@ RSpec.describe API::Todos do expect(response).to have_gitlab_http_status(:not_found) end end - - it 'returns an error if the issuable author does not have access' do - project_1.add_guest(issuable.author) - - post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.iid}/todo", issuable.author) - - expect(response).to have_gitlab_http_status(:not_found) - end end describe 'POST :id/issuable_type/:issueable_id/todo' do context 'for an issue' do - it_behaves_like 'an issuable', 'issues' do - let_it_be(:issuable) do - create(:issue, :confidential, author: author_1, project: project_1) - end + let_it_be(:issuable) do + create(:issue, :confidential, project: project_1) + end + + it_behaves_like 'an issuable', 'issues' + + it 'returns an error if the issue author does not have access' do + post api("/projects/#{project_1.id}/issues/#{issuable.iid}/todo", issuable.author) + + expect(response).to have_gitlab_http_status(:not_found) end end context 'for a merge request' do - it_behaves_like 'an issuable', 'merge_requests' do - let_it_be(:issuable) do - create(:merge_request, :simple, source_project: project_1) - end + let_it_be(:issuable) do + create(:merge_request, :simple, source_project: project_1) + end + + it_behaves_like 'an issuable', 'merge_requests' + + it 'returns an error if the merge request author does not have access' do + project_1.add_guest(issuable.author) + + post api("/projects/#{project_1.id}/merge_requests/#{issuable.iid}/todo", issuable.author) + + expect(response).to have_gitlab_http_status(:forbidden) end end end diff --git a/spec/services/protected_branches/create_service_spec.rb b/spec/services/protected_branches/create_service_spec.rb index 45462831a31..756c775be9b 100644 --- a/spec/services/protected_branches/create_service_spec.rb +++ b/spec/services/protected_branches/create_service_spec.rb @@ -7,13 +7,15 @@ RSpec.describe ProtectedBranches::CreateService do let(:user) { project.owner } let(:params) do { - name: 'master', + name: name, merge_access_levels_attributes: [{ access_level: Gitlab::Access::MAINTAINER }], push_access_levels_attributes: [{ access_level: Gitlab::Access::MAINTAINER }] } end describe '#execute' do + let(:name) { 'master' } + subject(:service) { described_class.new(project, user, params) } it 'creates a new protected branch' do @@ -22,6 +24,41 @@ RSpec.describe ProtectedBranches::CreateService do expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) end + context 'when name has escaped HTML' do + let(:name) { 'feature->test' } + + it 'creates the new protected branch matching the unescaped version' do + expect { service.execute }.to change(ProtectedBranch, :count).by(1) + expect(project.protected_branches.last.name).to eq('feature->test') + end + + context 'and name contains HTML tags' do + let(:name) { '<b>master</b>' } + + it 'creates the new protected branch with sanitized name' do + expect { service.execute }.to change(ProtectedBranch, :count).by(1) + expect(project.protected_branches.last.name).to eq('master') + end + + context 'and contains unsafe HTML' do + let(:name) { '<script>alert('foo');</script>' } + + it 'does not create the new protected branch' do + expect { service.execute }.not_to change(ProtectedBranch, :count) + end + end + end + + context 'when name contains unescaped HTML tags' do + let(:name) { '<b>master</b>' } + + it 'creates the new protected branch with sanitized name' do + expect { service.execute }.to change(ProtectedBranch, :count).by(1) + expect(project.protected_branches.last.name).to eq('master') + end + end + end + context 'when user does not have permission' do let(:user) { create(:user) } diff --git a/spec/services/protected_branches/update_service_spec.rb b/spec/services/protected_branches/update_service_spec.rb index 88e58ad5907..b5cf1a54aff 100644 --- a/spec/services/protected_branches/update_service_spec.rb +++ b/spec/services/protected_branches/update_service_spec.rb @@ -6,17 +6,50 @@ RSpec.describe ProtectedBranches::UpdateService do let(:protected_branch) { create(:protected_branch) } let(:project) { protected_branch.project } let(:user) { project.owner } - let(:params) { { name: 'new protected branch name' } } + let(:params) { { name: new_name } } describe '#execute' do + let(:new_name) { 'new protected branch name' } + let(:result) { service.execute(protected_branch) } + subject(:service) { described_class.new(project, user, params) } it 'updates a protected branch' do - result = service.execute(protected_branch) - expect(result.reload.name).to eq(params[:name]) end + context 'when name has escaped HTML' do + let(:new_name) { 'feature->test' } + + it 'updates protected branch name with unescaped HTML' do + expect(result.reload.name).to eq('feature->test') + end + + context 'and name contains HTML tags' do + let(:new_name) { '<b>master</b>' } + + it 'updates protected branch name with sanitized name' do + expect(result.reload.name).to eq('master') + end + + context 'and contains unsafe HTML' do + let(:new_name) { '<script>alert('foo');</script>' } + + it 'does not update the protected branch' do + expect(result.reload.name).to eq(protected_branch.name) + end + end + end + end + + context 'when name contains unescaped HTML tags' do + let(:new_name) { '<b>master</b>' } + + it 'updates protected branch name with sanitized name' do + expect(result.reload.name).to eq('master') + end + end + context 'without admin_project permissions' do let(:user) { create(:user) } diff --git a/spec/support/helpers/features/members_helpers.rb b/spec/support/helpers/features/members_helpers.rb index 2e86e014a1b..bdadcb8af43 100644 --- a/spec/support/helpers/features/members_helpers.rb +++ b/spec/support/helpers/features/members_helpers.rb @@ -37,6 +37,10 @@ module Spec find_row(user.name) end + def find_username_row(user) + find_row(user.username) + end + def find_invited_member_row(email) find_row(email) end diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index ee4621deb2d..1f0c9b658dc 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -374,6 +374,7 @@ module GraphqlHelpers allow_unlimited_graphql_depth if max_depth > 1 allow_high_graphql_recursion allow_high_graphql_transaction_threshold + allow_high_graphql_query_size type = class_name.respond_to?(:kind) ? class_name : GitlabSchema.types[class_name.to_s] raise "#{class_name} is not a known type in the GitlabSchema" unless type @@ -624,6 +625,10 @@ module GraphqlHelpers stub_const("Gitlab::QueryLimiting::Transaction::THRESHOLD", 1000) end + def allow_high_graphql_query_size + stub_const('GraphqlController::MAX_QUERY_SIZE', 10_000_000) + end + def node_array(data, extract_attribute = nil) data.map do |item| extract_attribute ? item['node'][extract_attribute] : item['node'] 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/support/shared_examples/requests/api/merge_requests_shared_examples.rb b/spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb index e6f9e5a434c..28813a23fed 100644 --- a/spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb @@ -14,10 +14,10 @@ RSpec.shared_examples 'rejects user from accessing merge request info' do project.add_guest(user) end - it 'returns a 404 error' do + it 'returns a 403 error' do get api(url, user) - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 Merge Request Not Found') + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('403 Forbidden') end end 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 diff --git a/yarn.lock b/yarn.lock index b7f16a7282f..4e493812ead 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4918,12 +4918,7 @@ domhandler@^4.0.0, domhandler@^4.2.0: dependencies: domelementtype "^2.2.0" -dompurify@2.3.1: - version "2.3.1" - resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-2.3.1.tgz#a47059ca21fd1212d3c8f71fdea6943b8bfbdf6a" - integrity sha512-xGWt+NHAQS+4tpgbOAI08yxW0Pr256Gu/FNE2frZVTbgrBUn8M7tz7/ktS/LZ2MHeGqz6topj0/xY+y8R5FBFw== - -dompurify@^2.3.3: +dompurify@2.3.3, dompurify@^2.3.3: version "2.3.3" resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-2.3.3.tgz#c1af3eb88be47324432964d8abc75cf4b98d634c" integrity sha512-dqnqRkPMAjOZE0FogZ+ceJNM2dZ3V/yNOuFB7+39qpO93hHhfRpHw3heYQC7DPK9FqbQTfBKUJhiSfz4MvXYwg== @@ -8445,16 +8440,16 @@ merge2@^1.3.0: resolved "https://registry.yarnpkg.com/merge2/-/merge2-1.4.1.tgz#4368892f885e907455a6fd7dc55c0c9d404990ae" integrity sha512-8q7VEgMJW4J8tcfVPy8g09NcQwZdbwFEqhe/WZkoIzjn/3TGDwtOCYtXGxA3O8tPzpczCCDgv+P2P5y00ZJOOg== -mermaid@^8.13.2: - version "8.13.2" - resolved "https://registry.yarnpkg.com/mermaid/-/mermaid-8.13.2.tgz#9f8abc66ba1c53b132fdaa0d4a80f4717b7b7655" - integrity sha512-qTFI7MfC2d+x0Hft5gx063EH9tZg36lERG8o7Zq0Ag+MnO8CgVaMZEU6oA8gzTtTn9upMdy4UlYSLVmavu27cQ== +mermaid@^8.13.4: + version "8.13.4" + resolved "https://registry.yarnpkg.com/mermaid/-/mermaid-8.13.4.tgz#924cb85f39380285e0a99f245c66cfa61014a2e1" + integrity sha512-zdWtsXabVy1PEAE25Jkm4zbTDlQe8rqNlTMq2B3j+D+NxDskJEY5OsgalarvNLsw+b5xFa1a8D1xcm/PijrDow== dependencies: "@braintree/sanitize-url" "^3.1.0" d3 "^7.0.0" dagre "^0.8.5" dagre-d3 "^0.6.4" - dompurify "2.3.1" + dompurify "2.3.3" graphlib "^2.1.8" khroma "^1.4.1" moment-mini "^2.24.0" |