From 6a5b78ac6945c0b0cd42293f11c94c2b3750fddc Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 7 Dec 2021 03:12:22 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .rubocop_todo/rails/save_bang.yml | 7 - CHANGELOG.md | 12 ++ Gemfile | 2 +- Gemfile.lock | 4 +- app/assets/javascripts/blob/openapi/index.js | 6 + .../components/issuables_list_app.vue | 2 +- .../components/issues_list_app.vue | 4 +- .../components/issuable_bulk_edit_sidebar.vue | 0 .../list}/components/issuable_item.vue | 0 .../list}/components/issuable_list_root.vue | 0 .../list}/components/issuable_tabs.vue | 0 .../issuable/list}/constants.js | 0 app/controllers/graphql_controller.rb | 16 +- .../projects/usage_quotas_controller.rb | 9 -- app/graphql/gitlab_schema.rb | 3 + app/graphql/types/user_interface.rb | 16 +- app/helpers/search_helper.rb | 24 ++- .../concerns/bulk_member_access_load.rb | 25 +-- app/models/concerns/diff_positionable_note.rb | 1 + ...er_max_access_level_in_groups_preloader.rb | 9 +- app/models/project.rb | 1 + app/models/project_team.rb | 6 +- app/models/todo.rb | 2 +- app/policies/issuable_policy.rb | 4 +- app/policies/project_policy.rb | 13 +- .../protected_branches/base_service.rb | 18 +++ .../protected_branches/create_service.rb | 2 +- .../protected_branches/update_service.rb | 2 +- app/validators/json_schema_validator.rb | 4 +- app/validators/json_schemas/position.json | 151 ++++++++++++++++++ app/views/layouts/_search.html.haml | 5 +- app/views/projects/show.html.haml | 2 +- .../projects/usage_quotas/index.html.haml | 2 +- config/application.rb | 1 + .../create_vulnerabilities_via_api.yml | 8 - .../auth/ldap/ldap-troubleshooting.md | 21 ++- doc/api/graphql/reference/index.md | 8 +- doc/ci/variables/predefined_variables.md | 2 +- doc/user/packages/maven_repository/index.md | 2 +- lib/api/entities/project.rb | 4 +- lib/api/entities/user_safe.rb | 12 +- lib/api/lint.rb | 12 +- lib/api/merge_request_approvals.rb | 2 - lib/api/merge_request_diffs.rb | 4 - lib/api/merge_requests.rb | 10 -- lib/api/todos.rb | 4 - lib/banzai/filter/front_matter_filter.rb | 2 +- lib/gitlab/current_settings.rb | 2 +- lib/gitlab/diff/lines_unfolder.rb | 1 + lib/gitlab/front_matter.rb | 10 +- lib/gitlab/git_access_wiki.rb | 7 + lib/gitlab/import_export/members_mapper.rb | 11 +- lib/gitlab/quick_actions/extractor.rb | 4 +- lib/gitlab/regex.rb | 2 +- lib/gitlab/slash_commands/deploy.rb | 12 +- lib/gitlab/wiki_pages/front_matter_parser.rb | 2 +- lib/sidebars/projects/menus/analytics_menu.rb | 2 +- locale/gitlab.pot | 3 + .../dashboard/todos_controller_spec.rb | 2 +- spec/controllers/graphql_controller_spec.rb | 38 +++++ spec/factories/diff_position.rb | 8 +- .../features/projects/blobs/blob_show_spec.rb | 51 +++++- spec/features/projects/members/list_spec.rb | 2 +- spec/features/projects_spec.rb | 18 +++ spec/features/protected_branches_spec.rb | 6 +- spec/frontend/diffs/store/utils_spec.js | 4 +- spec/frontend/issuable_show/mock_data.js | 2 +- .../components/issues_list_app_spec.js | 4 +- .../issuable_bulk_edit_sidebar_spec.js | 2 +- .../list}/components/issuable_item_spec.js | 2 +- .../components/issuable_list_root_spec.js | 6 +- .../list}/components/issuable_tabs_spec.js | 2 +- .../issuable/list}/mock_data.js | 0 spec/graphql/gitlab_schema_spec.rb | 34 ++++ spec/graphql/types/user_type_spec.rb | 80 ++++++++++ spec/helpers/search_helper_spec.rb | 16 +- spec/lib/api/entities/project_spec.rb | 22 +++ spec/lib/api/entities/user_spec.rb | 45 +++++- .../banzai/filter/front_matter_filter_spec.rb | 16 ++ spec/lib/gitlab/current_settings_spec.rb | 10 +- .../diff/formatters/text_formatter_spec.rb | 6 +- spec/lib/gitlab/diff/lines_unfolder_spec.rb | 10 ++ .../position_tracer/line_strategy_spec.rb | 24 ++- spec/lib/gitlab/git_access_wiki_spec.rb | 25 +++ .../group/relation_tree_restorer_spec.rb | 4 +- .../import_export/members_mapper_spec.rb | 60 +++++++ .../project/relation_factory_spec.rb | 2 +- .../project/relation_tree_restorer_spec.rb | 4 +- .../project/tree_restorer_spec.rb | 17 +- .../gitlab/quick_actions/extractor_spec.rb | 8 + spec/lib/gitlab/regex_spec.rb | 13 ++ spec/lib/gitlab/slash_commands/deploy_spec.rb | 59 +++++++ .../wiki_pages/front_matter_parser_spec.rb | 2 +- .../projects/menus/analytics_menu_spec.rb | 6 + .../concerns/after_commit_queue_spec.rb | 70 +++++++- spec/models/packages/package_spec.rb | 2 +- ...x_access_level_in_groups_preloader_spec.rb | 3 +- spec/policies/merge_request_policy_spec.rb | 35 +++- spec/requests/api/graphql/user_query_spec.rb | 14 ++ spec/requests/api/lint_spec.rb | 29 ++++ spec/requests/api/projects_spec.rb | 2 +- spec/requests/api/todos_spec.rb | 38 +++-- spec/requests/projects/usage_quotas_spec.rb | 10 -- .../protected_branches/create_service_spec.rb | 39 ++++- .../protected_branches/update_service_spec.rb | 39 ++++- .../helpers/features/members_helpers.rb | 4 + spec/support/helpers/graphql_helpers.rb | 5 + .../diff_positionable_note_shared_examples.rb | 33 ++++ .../api/diff_discussions_shared_examples.rb | 12 +- .../api/merge_requests_shared_examples.rb | 6 +- spec/validators/json_schema_validator_spec.rb | 12 ++ 111 files changed, 1241 insertions(+), 222 deletions(-) rename app/assets/javascripts/{issuable_list => vue_shared/issuable/list}/components/issuable_bulk_edit_sidebar.vue (100%) rename app/assets/javascripts/{issuable_list => vue_shared/issuable/list}/components/issuable_item.vue (100%) rename app/assets/javascripts/{issuable_list => vue_shared/issuable/list}/components/issuable_list_root.vue (100%) rename app/assets/javascripts/{issuable_list => vue_shared/issuable/list}/components/issuable_tabs.vue (100%) rename app/assets/javascripts/{issuable_list => vue_shared/issuable/list}/constants.js (100%) create mode 100644 app/validators/json_schemas/position.json delete mode 100644 config/feature_flags/development/create_vulnerabilities_via_api.yml rename spec/frontend/{issuable_list => vue_shared/issuable/list}/components/issuable_bulk_edit_sidebar_spec.js (95%) rename spec/frontend/{issuable_list => vue_shared/issuable/list}/components/issuable_item_spec.js (99%) rename spec/frontend/{issuable_list => vue_shared/issuable/list}/components/issuable_list_root_spec.js (98%) rename spec/frontend/{issuable_list => vue_shared/issuable/list}/components/issuable_tabs_spec.js (96%) rename spec/frontend/{issuable_list => vue_shared/issuable/list}/mock_data.js (100%) diff --git a/.rubocop_todo/rails/save_bang.yml b/.rubocop_todo/rails/save_bang.yml index 59dbe6b1dc5..54d62f79651 100644 --- a/.rubocop_todo/rails/save_bang.yml +++ b/.rubocop_todo/rails/save_bang.yml @@ -38,13 +38,6 @@ Rails/SaveBang: - ee/spec/models/visible_approvable_spec.rb - ee/spec/models/vulnerabilities/feedback_spec.rb - ee/spec/models/vulnerabilities/issue_link_spec.rb - - ee/spec/services/ee/boards/issues/create_service_spec.rb - - ee/spec/services/ee/boards/issues/list_service_spec.rb - - ee/spec/services/ee/boards/lists/list_service_spec.rb - - ee/spec/services/ee/issuable/clone/attributes_rewriter_spec.rb - - ee/spec/services/ee/issuable/common_system_notes_service_spec.rb - - ee/spec/services/ee/issues/update_service_spec.rb - - ee/spec/services/ee/merge_requests/refresh_service_spec.rb - ee/spec/services/ee/merge_requests/update_service_spec.rb - ee/spec/services/ee/notes/quick_actions_service_spec.rb - ee/spec/services/ee/notification_service_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 82dc7691ff9..9b39bd9a443 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) @@ -540,6 +544,10 @@ entry. - [Add pipeline artifacts and uploads sizes to project REST API](gitlab-org/gitlab@58d66f28faf42ae98ca11ff1ba0bdd9180e988ad) by @guillaume.chauvel ([merge request](gitlab-org/gitlab!72075)) - [Remove not used parameter from epics finder](gitlab-org/gitlab@49fce172b57b2f376a114726b1dd1900fe36a238) ([merge request](gitlab-org/gitlab!72285)) **GitLab Enterprise Edition** +## 14.4.4 (2021-12-03) + +No changes. + ## 14.4.3 (2021-12-01) ### Fixed (6 changes) @@ -974,6 +982,10 @@ entry. - [Cleanup bigint conversion for ci_builds](gitlab-org/gitlab@176992aa2b2e76b22637a07d5bafbd6541324a7d) ([merge request](gitlab-org/gitlab!70351)) - [Drop support for data-track-event](gitlab-org/gitlab@ac6027fbef6adf41643412a84945fda6f15c9666) ([merge request](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70234)) +## 14.3.6 (2021-12-03) + +No changes. + ## 14.3.5 (2021-11-26) ### Fixed (6 changes) diff --git a/Gemfile b/Gemfile index 38ffccf1778..b0b9e58534f 100644 --- a/Gemfile +++ b/Gemfile @@ -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 e417b8da225..7c85913ddd9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -566,7 +566,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) @@ -1499,7 +1499,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 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/assets/javascripts/issues_list/components/issuables_list_app.vue b/app/assets/javascripts/issues_list/components/issuables_list_app.vue index 62b52afdaca..73dba056e85 100644 --- a/app/assets/javascripts/issues_list/components/issuables_list_app.vue +++ b/app/assets/javascripts/issues_list/components/issuables_list_app.vue @@ -26,7 +26,7 @@ import { emptyStateHelper } from '../service_desk_helper'; import Issuable from './issuable.vue'; /** - * @deprecated Use app/assets/javascripts/issuable_list/components/issuable_list_root.vue instead + * @deprecated Use app/assets/javascripts/vue_shared/issuable/list/components/issuable_list_root.vue instead */ export default { LOADING_LIST_ITEMS_LENGTH, diff --git a/app/assets/javascripts/issues_list/components/issues_list_app.vue b/app/assets/javascripts/issues_list/components/issues_list_app.vue index ad810115ef0..3d9eb7bdcf1 100644 --- a/app/assets/javascripts/issues_list/components/issues_list_app.vue +++ b/app/assets/javascripts/issues_list/components/issues_list_app.vue @@ -18,8 +18,8 @@ import { convertToGraphQLId, getIdFromGraphQLId } from '~/graphql_shared/utils'; import { ITEM_TYPE } from '~/groups/constants'; import CsvImportExportButtons from '~/issuable/components/csv_import_export_buttons.vue'; import IssuableByEmail from '~/issuable/components/issuable_by_email.vue'; -import IssuableList from '~/issuable_list/components/issuable_list_root.vue'; -import { IssuableListTabs, IssuableStates } from '~/issuable_list/constants'; +import IssuableList from '~/vue_shared/issuable/list/components/issuable_list_root.vue'; +import { IssuableListTabs, IssuableStates } from '~/vue_shared/issuable/list/constants'; import { CREATED_DESC, i18n, diff --git a/app/assets/javascripts/issuable_list/components/issuable_bulk_edit_sidebar.vue b/app/assets/javascripts/vue_shared/issuable/list/components/issuable_bulk_edit_sidebar.vue similarity index 100% rename from app/assets/javascripts/issuable_list/components/issuable_bulk_edit_sidebar.vue rename to app/assets/javascripts/vue_shared/issuable/list/components/issuable_bulk_edit_sidebar.vue diff --git a/app/assets/javascripts/issuable_list/components/issuable_item.vue b/app/assets/javascripts/vue_shared/issuable/list/components/issuable_item.vue similarity index 100% rename from app/assets/javascripts/issuable_list/components/issuable_item.vue rename to app/assets/javascripts/vue_shared/issuable/list/components/issuable_item.vue diff --git a/app/assets/javascripts/issuable_list/components/issuable_list_root.vue b/app/assets/javascripts/vue_shared/issuable/list/components/issuable_list_root.vue similarity index 100% rename from app/assets/javascripts/issuable_list/components/issuable_list_root.vue rename to app/assets/javascripts/vue_shared/issuable/list/components/issuable_list_root.vue diff --git a/app/assets/javascripts/issuable_list/components/issuable_tabs.vue b/app/assets/javascripts/vue_shared/issuable/list/components/issuable_tabs.vue similarity index 100% rename from app/assets/javascripts/issuable_list/components/issuable_tabs.vue rename to app/assets/javascripts/vue_shared/issuable/list/components/issuable_tabs.vue diff --git a/app/assets/javascripts/issuable_list/constants.js b/app/assets/javascripts/vue_shared/issuable/list/constants.js similarity index 100% rename from app/assets/javascripts/issuable_list/constants.js rename to app/assets/javascripts/vue_shared/issuable/list/constants.js 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/controllers/projects/usage_quotas_controller.rb b/app/controllers/projects/usage_quotas_controller.rb index b319e427eaa..680874ffee4 100644 --- a/app/controllers/projects/usage_quotas_controller.rb +++ b/app/controllers/projects/usage_quotas_controller.rb @@ -9,14 +9,5 @@ class Projects::UsageQuotasController < Projects::ApplicationController def index @hide_search_settings = true - @storage_app_data = { - project_path: @project.full_path, - usage_quotas_help_page_path: help_page_path('user/usage_quotas'), - build_artifacts_help_page_path: help_page_path('ci/pipelines/job_artifacts', anchor: 'when-job-artifacts-are-deleted'), - packages_help_page_path: help_page_path('user/packages/package_registry/index.md', anchor: 'delete-a-package'), - repository_help_page_path: help_page_path('user/project/repository/reducing_the_repo_size_using_git'), - snippets_help_page_path: help_page_path('user/snippets', anchor: 'reduce-snippets-repository-size'), - wiki_help_page_path: help_page_path('administration/wikis/index.md', anchor: 'reduce-wiki-repository-size') - } end end 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 a5e9b0e0a1a..6d35534bb6a 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 d7c22bf8510..dc436570f52 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -70,7 +70,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 4757f50739b..d840ea01b89 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/app/views/projects/usage_quotas/index.html.haml b/app/views/projects/usage_quotas/index.html.haml index 6c7cccfb9b1..de1135cf928 100644 --- a/app/views/projects/usage_quotas/index.html.haml +++ b/app/views/projects/usage_quotas/index.html.haml @@ -16,4 +16,4 @@ = s_('UsageQuota|Storage') .tab-content .tab-pane#storage-quota-tab - #js-project-storage-count-app{ data: @storage_app_data } + #js-project-storage-count-app{ data: { project_path: @project.full_path } } 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/config/feature_flags/development/create_vulnerabilities_via_api.yml b/config/feature_flags/development/create_vulnerabilities_via_api.yml deleted file mode 100644 index 3f8af065dc2..00000000000 --- a/config/feature_flags/development/create_vulnerabilities_via_api.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: create_vulnerabilities_via_api -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68158 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338694 -milestone: '14.3' -type: development -group: group::threat insights -default_enabled: true diff --git a/doc/administration/auth/ldap/ldap-troubleshooting.md b/doc/administration/auth/ldap/ldap-troubleshooting.md index 197caee15ea..63e4490e332 100644 --- a/doc/administration/auth/ldap/ldap-troubleshooting.md +++ b/doc/administration/auth/ldap/ldap-troubleshooting.md @@ -106,7 +106,7 @@ here are some questions to ask yourself: - Does the user pass through the [configured `user_filter`](index.md#set-up-ldap-user-filter)? If one is not configured, this question can be ignored. If it is, then the user must also pass through this filter to be allowed to sign in. - - Refer to our docs on [debugging the `user_filter`](#debug-ldap-user-filter). + - Refer to our documentation on [debugging the `user_filter`](#debug-ldap-user-filter). If the above are both okay, the next place to look for the problem is the logs themselves while reproducing the issue. @@ -316,7 +316,7 @@ LDAP search error: No Such Object User Update (0.4ms) UPDATE "users" SET "state" = $1, "updated_at" = $2 WHERE "users"."id" = $3 [["state", "ldap_blocked"], ["updated_at", "2019-10-18 15:46:22.902177"], ["id", 20]] ``` -Once the user is found in LDAP, the rest of the output updates the GitLab +After the user is found in LDAP, the rest of the output updates the GitLab database with any changes. #### Query a user in LDAP @@ -337,8 +337,8 @@ Gitlab::Auth::Ldap::Person.find_by_uid('', adapter) #### Membership(s) not granted Sometimes you may think a particular user should be added to a GitLab group via -LDAP group sync, but for some reason it's not happening. There are several -things to check to debug the situation. +LDAP group sync, but for some reason it's not happening. You can check several +things to debug the situation. - Ensure LDAP configuration has a `group_base` specified. [This configuration](ldap_synchronization.md#group-sync) is required for group sync to work properly. @@ -421,7 +421,7 @@ Started syncing 'ldapmain' provider for 'my_group' group ``` The following entry shows an array of all user DNs GitLab sees in the LDAP server. -These are the users for a single LDAP group, not a GitLab group. If +These DNs are the users for a single LDAP group, not a GitLab group. If you have multiple LDAP groups linked to this GitLab group, you see multiple log entries like this - one for each LDAP group. If you don't see an LDAP user DN in this log entry, LDAP is not returning the user when we do the lookup. @@ -545,7 +545,7 @@ updates the stored DN to the new value so both values now match what's in LDAP. If the email has changed and the DN has not, GitLab finds the user with -the DN and update its own record of the user's email to match the one in LDAP. +the DN and updates its own record of the user's email to match the one in LDAP. However, if the primary email _and_ the DN change in LDAP, then GitLab has no way of identifying the correct LDAP record of the user and, as a @@ -563,7 +563,7 @@ email address are removed first. This is because emails have to be unique in Git Go to the [rails console](#rails-console) and then run: ```ruby -# Each entry will have to include the old username and the new email +# Each entry must include the old username and the new email emails = { 'ORIGINAL_USERNAME' => 'NEW_EMAIL_ADDRESS', ... @@ -686,7 +686,7 @@ For more information, see the [official `ldapsearch` documentation](https://linu ### Using **AdFind** (Windows) -You can use the [`AdFind`](https://social.technet.microsoft.com/wiki/contents/articles/7535.adfind-command-examples.aspx) utility (on Windows based systems) to test that your LDAP server is accessible and authentication is working correctly. This is a freeware utility built by [Joe Richards](http://www.joeware.net/freetools/tools/adfind/index.htm). +You can use the [`AdFind`](https://social.technet.microsoft.com/wiki/contents/articles/7535.adfind-command-examples.aspx) utility (on Windows based systems) to test that your LDAP server is accessible and authentication is working correctly. AdFind is a freeware utility built by [Joe Richards](http://www.joeware.net/freetools/tools/adfind/index.htm). **Return all objects** @@ -719,9 +719,8 @@ For instructions about how to use the rails console, refer to this #### Enable debug output -This provides debug output that is useful to see -what GitLab is doing and with what. This value is not persisted, and is only -enabled for this session in the rails console. +This provides debug output that shows what GitLab is doing and with what. +This value is not persisted, and is only enabled for this session in the Rails console. To enable debug output in the rails console, [enter the rails console](#rails-console) and run: diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index f8246de4518..261c7dd4fae 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -11725,7 +11725,7 @@ A user assigned to a merge request. | `id` | [`ID!`](#id) | ID of the user. | | `location` | [`String`](#string) | Location of the user. | | `mergeRequestInteraction` | [`UserMergeRequestInteraction`](#usermergerequestinteraction) | Details of this user's interactions with the merge request. | -| `name` | [`String!`](#string) | Human-readable name of the user. | +| `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. | | `namespace` | [`Namespace`](#namespace) | Personal namespace of the user. | | `projectMemberships` | [`ProjectMemberConnection`](#projectmemberconnection) | Project memberships of the user. (see [Connections](#connections)) | | `publicEmail` | [`String`](#string) | User's public email. | @@ -11977,7 +11977,7 @@ A user assigned to a merge request as a reviewer. | `id` | [`ID!`](#id) | ID of the user. | | `location` | [`String`](#string) | Location of the user. | | `mergeRequestInteraction` | [`UserMergeRequestInteraction`](#usermergerequestinteraction) | Details of this user's interactions with the merge request. | -| `name` | [`String!`](#string) | Human-readable name of the user. | +| `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. | | `namespace` | [`Namespace`](#namespace) | Personal namespace of the user. | | `projectMemberships` | [`ProjectMemberConnection`](#projectmemberconnection) | Project memberships of the user. (see [Connections](#connections)) | | `publicEmail` | [`String`](#string) | User's public email. | @@ -15047,7 +15047,7 @@ Core represention of a GitLab user. | `groupMemberships` | [`GroupMemberConnection`](#groupmemberconnection) | Group memberships of the user. (see [Connections](#connections)) | | `id` | [`ID!`](#id) | ID of the user. | | `location` | [`String`](#string) | Location of the user. | -| `name` | [`String!`](#string) | Human-readable name of the user. | +| `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. | | `namespace` | [`Namespace`](#namespace) | Personal namespace of the user. | | `projectMemberships` | [`ProjectMemberConnection`](#projectmemberconnection) | Project memberships of the user. (see [Connections](#connections)) | | `publicEmail` | [`String`](#string) | User's public email. | @@ -18146,7 +18146,7 @@ Implementations: | `groupMemberships` | [`GroupMemberConnection`](#groupmemberconnection) | Group memberships of the user. (see [Connections](#connections)) | | `id` | [`ID!`](#id) | ID of the user. | | `location` | [`String`](#string) | Location of the user. | -| `name` | [`String!`](#string) | Human-readable name of the user. | +| `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. | | `namespace` | [`Namespace`](#namespace) | Personal namespace of the user. | | `projectMemberships` | [`ProjectMemberConnection`](#projectmemberconnection) | Project memberships of the user. (see [Connections](#connections)) | | `publicEmail` | [`String`](#string) | User's public email. | diff --git a/doc/ci/variables/predefined_variables.md b/doc/ci/variables/predefined_variables.md index 45fa1994342..88a097127c0 100644 --- a/doc/ci/variables/predefined_variables.md +++ b/doc/ci/variables/predefined_variables.md @@ -14,7 +14,7 @@ Some variables are only available with more recent versions of [GitLab Runner](h You can [output the values of all variables available for a job](index.md#list-all-environment-variables) with a `script` command. -There are also [Kubernetes-specific deployment variables](../../user/project/clusters/deploy_to_cluster.md#deployment-variables). +There are also [Kubernetes-specific deployment variables (deprecated)](../../user/project/clusters/deploy_to_cluster.md#deployment-variables). | Variable | GitLab | Runner | Description | |------------------------------------------|--------|--------|-------------| 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 d7600f8a9b5..1b9299ed17e 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 299e3cabba3..bfd457a3092 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})[ \t]*(?\S*) # opening front matter marker (optional language specifier) - \s* - ^(?.*?) # front matter block content (not greedy) - \s* - ^(\k | \.{3}) # closing front matter marker + ^(?#{DELIM})[ \t]*(?\S*)\R # opening front matter marker (optional language specifier) + (?.*?) # front matter block content (not greedy) + ^(\k | \.{3}) # closing front matter marker \s* }mx.freeze end diff --git a/lib/gitlab/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 5c9006b1ae1..8e139ae0709 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -62,7 +62,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+(?\S+.*)\s+to+\s+(?\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 ed8b7d464fd..b9bcc3267d6 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 10a197100ed..4ff7fa1897d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -41938,6 +41938,9 @@ msgstr "" msgid "must be a valid IPv4 or IPv6 address" msgstr "" +msgid "must be a valid json schema" +msgstr "" + msgid "must be after start" msgstr "" diff --git a/spec/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: | +
+

Swagger API documentation

+
+ 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 d0ef7aa3964..f2424a4acc3 100644 --- a/spec/features/projects/members/list_spec.rb +++ b/spec/features/projects/members/list_spec.rb @@ -148,7 +148,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/frontend/issuable_show/mock_data.js b/spec/frontend/issuable_show/mock_data.js index 986d32b4982..f5f3ed58655 100644 --- a/spec/frontend/issuable_show/mock_data.js +++ b/spec/frontend/issuable_show/mock_data.js @@ -1,4 +1,4 @@ -import { mockIssuable as issuable } from '../issuable_list/mock_data'; +import { mockIssuable as issuable } from 'jest/vue_shared/issuable/list/mock_data'; export const mockIssuable = { ...issuable, diff --git a/spec/frontend/issues_list/components/issues_list_app_spec.js b/spec/frontend/issues_list/components/issues_list_app_spec.js index 6489582bd0a..5b77c22f3a6 100644 --- a/spec/frontend/issues_list/components/issues_list_app_spec.js +++ b/spec/frontend/issues_list/components/issues_list_app_spec.js @@ -21,8 +21,8 @@ import createFlash, { FLASH_TYPES } from '~/flash'; import { convertToGraphQLId, getIdFromGraphQLId } from '~/graphql_shared/utils'; import CsvImportExportButtons from '~/issuable/components/csv_import_export_buttons.vue'; import IssuableByEmail from '~/issuable/components/issuable_by_email.vue'; -import IssuableList from '~/issuable_list/components/issuable_list_root.vue'; -import { IssuableListTabs, IssuableStates } from '~/issuable_list/constants'; +import IssuableList from '~/vue_shared/issuable/list/components/issuable_list_root.vue'; +import { IssuableListTabs, IssuableStates } from '~/vue_shared/issuable/list/constants'; import IssuesListApp from '~/issues_list/components/issues_list_app.vue'; import NewIssueDropdown from '~/issues_list/components/new_issue_dropdown.vue'; import { diff --git a/spec/frontend/issuable_list/components/issuable_bulk_edit_sidebar_spec.js b/spec/frontend/vue_shared/issuable/list/components/issuable_bulk_edit_sidebar_spec.js similarity index 95% rename from spec/frontend/issuable_list/components/issuable_bulk_edit_sidebar_spec.js rename to spec/frontend/vue_shared/issuable/list/components/issuable_bulk_edit_sidebar_spec.js index 52a238eac7c..0f33a3d1122 100644 --- a/spec/frontend/issuable_list/components/issuable_bulk_edit_sidebar_spec.js +++ b/spec/frontend/vue_shared/issuable/list/components/issuable_bulk_edit_sidebar_spec.js @@ -1,6 +1,6 @@ import { shallowMount } from '@vue/test-utils'; -import IssuableBulkEditSidebar from '~/issuable_list/components/issuable_bulk_edit_sidebar.vue'; +import IssuableBulkEditSidebar from '~/vue_shared/issuable/list/components/issuable_bulk_edit_sidebar.vue'; const createComponent = ({ expanded = true } = {}) => shallowMount(IssuableBulkEditSidebar, { diff --git a/spec/frontend/issuable_list/components/issuable_item_spec.js b/spec/frontend/vue_shared/issuable/list/components/issuable_item_spec.js similarity index 99% rename from spec/frontend/issuable_list/components/issuable_item_spec.js rename to spec/frontend/vue_shared/issuable/list/components/issuable_item_spec.js index a4d90613ca6..9cffd16a371 100644 --- a/spec/frontend/issuable_list/components/issuable_item_spec.js +++ b/spec/frontend/vue_shared/issuable/list/components/issuable_item_spec.js @@ -1,7 +1,7 @@ import { GlLink, GlLabel, GlIcon, GlFormCheckbox, GlSprintf } from '@gitlab/ui'; import { useFakeDate } from 'helpers/fake_date'; import { shallowMountExtended as shallowMount } from 'helpers/vue_test_utils_helper'; -import IssuableItem from '~/issuable_list/components/issuable_item.vue'; +import IssuableItem from '~/vue_shared/issuable/list/components/issuable_item.vue'; import IssuableAssignees from '~/vue_shared/components/issue/issue_assignees.vue'; import { mockIssuable, mockRegularLabel, mockScopedLabel } from '../mock_data'; diff --git a/spec/frontend/issuable_list/components/issuable_list_root_spec.js b/spec/frontend/vue_shared/issuable/list/components/issuable_list_root_spec.js similarity index 98% rename from spec/frontend/issuable_list/components/issuable_list_root_spec.js rename to spec/frontend/vue_shared/issuable/list/components/issuable_list_root_spec.js index 7dddd2c3405..e9bc4368f23 100644 --- a/spec/frontend/issuable_list/components/issuable_list_root_spec.js +++ b/spec/frontend/vue_shared/issuable/list/components/issuable_list_root_spec.js @@ -4,9 +4,9 @@ import VueDraggable from 'vuedraggable'; import { TEST_HOST } from 'helpers/test_constants'; -import IssuableItem from '~/issuable_list/components/issuable_item.vue'; -import IssuableListRoot from '~/issuable_list/components/issuable_list_root.vue'; -import IssuableTabs from '~/issuable_list/components/issuable_tabs.vue'; +import IssuableItem from '~/vue_shared/issuable/list/components/issuable_item.vue'; +import IssuableListRoot from '~/vue_shared/issuable/list/components/issuable_list_root.vue'; +import IssuableTabs from '~/vue_shared/issuable/list/components/issuable_tabs.vue'; import FilteredSearchBar from '~/vue_shared/components/filtered_search_bar/filtered_search_bar_root.vue'; import { mockIssuableListProps, mockIssuables } from '../mock_data'; diff --git a/spec/frontend/issuable_list/components/issuable_tabs_spec.js b/spec/frontend/vue_shared/issuable/list/components/issuable_tabs_spec.js similarity index 96% rename from spec/frontend/issuable_list/components/issuable_tabs_spec.js rename to spec/frontend/vue_shared/issuable/list/components/issuable_tabs_spec.js index cbf5765078a..8c22b67bdbe 100644 --- a/spec/frontend/issuable_list/components/issuable_tabs_spec.js +++ b/spec/frontend/vue_shared/issuable/list/components/issuable_tabs_spec.js @@ -1,7 +1,7 @@ import { GlTab, GlBadge } from '@gitlab/ui'; import { mount } from '@vue/test-utils'; -import IssuableTabs from '~/issuable_list/components/issuable_tabs.vue'; +import IssuableTabs from '~/vue_shared/issuable/list/components/issuable_tabs.vue'; import { mockIssuableListProps } from '../mock_data'; diff --git a/spec/frontend/issuable_list/mock_data.js b/spec/frontend/vue_shared/issuable/list/mock_data.js similarity index 100% rename from spec/frontend/issuable_list/mock_data.js rename to spec/frontend/vue_shared/issuable/list/mock_data.js 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 97aa36bfb74..83f85cc73d0 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -358,6 +358,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') } @@ -378,6 +390,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/concerns/after_commit_queue_spec.rb b/spec/models/concerns/after_commit_queue_spec.rb index 0a35ba34dce..8976ad58b7d 100644 --- a/spec/models/concerns/after_commit_queue_spec.rb +++ b/spec/models/concerns/after_commit_queue_spec.rb @@ -3,15 +3,71 @@ require 'spec_helper' RSpec.describe AfterCommitQueue do - it 'runs after transaction is committed' do - called = false - test_proc = proc { called = true } + describe '#run_after_commit' do + it 'runs after record is saved' do + called = false + test_proc = proc { called = true } - project = build(:project) - project.run_after_commit(&test_proc) + project = build(:project) + project.run_after_commit(&test_proc) - project.save! + expect(called).to be false - expect(called).to be true + # save! is run in its own transaction + project.save! + + expect(called).to be true + end + + it 'runs after transaction is committed' do + called = false + test_proc = proc { called = true } + + project = build(:project) + + Project.transaction do + project.run_after_commit(&test_proc) + + project.save! + + expect(called).to be false + end + + expect(called).to be true + end + end + + describe '#run_after_commit_or_now' do + it 'runs immediately if not within a transction' do + called = false + test_proc = proc { called = true } + + project = build(:project) + + project.run_after_commit_or_now(&test_proc) + + expect(called).to be true + end + + it 'runs after transaction has completed' do + called = false + test_proc = proc { called = true } + + project = build(:project) + + Project.transaction do + # Add this record to the current transaction so that after commit hooks + # are called + Project.connection.add_transaction_record(project) + + project.run_after_commit_or_now(&test_proc) + + project.save! + + expect(called).to be false + end + + expect(called).to be true + end end end 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 de0e89ee094..79dbbd20d83 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/requests/projects/usage_quotas_spec.rb b/spec/requests/projects/usage_quotas_spec.rb index 114e9bd9f1e..6e449a21804 100644 --- a/spec/requests/projects/usage_quotas_spec.rb +++ b/spec/requests/projects/usage_quotas_spec.rb @@ -23,20 +23,10 @@ RSpec.describe 'Project Usage Quotas' do describe 'GET /:namespace/:project/usage_quotas' do it 'renders usage quotas path' do - mock_storage_app_data = { - project_path: project.full_path, - usage_quotas_help_page_path: help_page_path('user/usage_quotas'), - build_artifacts_help_page_path: help_page_path('ci/pipelines/job_artifacts', anchor: 'when-job-artifacts-are-deleted'), - packages_help_page_path: help_page_path('user/packages/package_registry/index.md', anchor: 'delete-a-package'), - repository_help_page_path: help_page_path('user/project/repository/reducing_the_repo_size_using_git'), - snippets_help_page_path: help_page_path('user/snippets', anchor: 'reduce-snippets-repository-size'), - wiki_help_page_path: help_page_path('administration/wikis/index.md', anchor: 'reduce-wiki-repository-size') - } get project_usage_quotas_path(project) expect(response).to have_gitlab_http_status(:ok) expect(response.body).to include(project_usage_quotas_path(project)) - expect(assigns[:storage_app_data]).to eq(mock_storage_app_data) expect(response.body).to include("Usage of project resources across the #{project.name} project") 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) { 'master' } + + 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) { 'master' } + + 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 d25be4251b3..8b7d1c753d5 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 @@ -629,6 +630,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