diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 8ddcf9c209..baaeb37506 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -19,6 +19,9 @@ .if-default-branch-refs: &if-default-branch-refs if: '$CI_COMMIT_REF_NAME == $CI_DEFAULT_BRANCH' +.if-stable-branch-refs: &if-stable-branch-refs + if: '$CI_COMMIT_REF_NAME =~ /^[\d-]+-stable(-ee)?$/' + .if-default-branch-push: &if-default-branch-push if: '$CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH && $CI_PIPELINE_SOURCE == "push"' @@ -40,6 +43,9 @@ .if-automated-merge-request: &if-automated-merge-request if: '$CI_MERGE_REQUEST_SOURCE_BRANCH_NAME == "release-tools/update-gitaly" || $CI_MERGE_REQUEST_TARGET_BRANCH_NAME =~ /stable-ee$/' +.if-merge-request-targeting-stable-branch: &if-merge-request-targeting-stable-branch + if: '$CI_MERGE_REQUEST_IID && $CI_MERGE_REQUEST_TARGET_BRANCH_NAME =~ /^[\d-]+-stable(-ee)?$/' + .if-merge-request-labels-as-if-foss: &if-merge-request-labels-as-if-foss if: '$CI_MERGE_REQUEST_LABELS =~ /pipeline:run-as-if-foss/' @@ -518,6 +524,12 @@ when: never - <<: *if-jh when: never + - <<: *if-security-merge-request + when: never + - <<: *if-merge-request-targeting-stable-branch + when: never + - <<: *if-stable-branch-refs + when: never - <<: *if-merge-request-labels-as-if-jh - <<: *if-merge-request-labels-run-all-rspec - changes: *code-backstage-qa-patterns @@ -550,7 +562,11 @@ - <<: *if-jh when: never - <<: *if-security-merge-request - changes: *code-backstage-patterns + when: never + - <<: *if-merge-request-targeting-stable-branch + when: never + - <<: *if-stable-branch-refs + when: never - <<: *if-merge-request-labels-as-if-jh - <<: *if-merge-request-labels-run-all-rspec - <<: *if-merge-request @@ -1163,6 +1179,24 @@ - <<: *if-merge-request-labels-as-if-foss changes: *code-backstage-patterns +.rails:rules:as-if-jh-rspec: + rules: + - <<: *if-not-ee + when: never + - <<: *if-jh + when: never + - <<: *if-security-merge-request + when: never + - <<: *if-merge-request-targeting-stable-branch + when: never + - <<: *if-stable-branch-refs + when: never + - <<: *if-merge-request-labels-as-if-jh + allow_failure: true + - <<: *if-merge-request + changes: *ci-patterns + allow_failure: true + .rails:rules:ee-and-foss-db-library-code: rules: - changes: *db-library-patterns @@ -1638,6 +1672,12 @@ when: never - <<: *if-jh when: never + - <<: *if-security-merge-request + when: never + - <<: *if-merge-request-targeting-stable-branch + when: never + - <<: *if-stable-branch-refs + when: never - <<: *if-merge-request-labels-as-if-jh - <<: *if-merge-request-labels-run-all-rspec - changes: *code-backstage-qa-patterns diff --git a/.gitlab/issue_templates/Geo Replicate a new Git repository type.md b/.gitlab/issue_templates/Geo Replicate a new Git repository type.md index 0d82294579..ee4cb27053 100644 --- a/.gitlab/issue_templates/Geo Replicate a new Git repository type.md +++ b/.gitlab/issue_templates/Geo Replicate a new Git repository type.md @@ -365,6 +365,10 @@ That's all of the required database changes. ::Gitlab::GitAccessCoolWidget end + def self.no_repo_message + git_access_class.error_message(:no_repo) + end + # The feature flag follows the format `geo_#{replicable_name}_replication`, # so here it would be `geo_cool_widget_replication` def self.replication_enabled_by_default? @@ -403,6 +407,9 @@ That's all of the required database changes. ``` - [ ] Make sure a Geo secondary site can request and download Cool Widgets on the Geo primary site. You may need to make some changes to `Gitlab::GitAccessCoolWidget`. For example, see [this change for Group-level Wikis](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/54914/diffs?commit_id=0f2b36f66697b4addbc69bd377ee2818f648dd33). + +- [ ] Make sure a Geo secondary site can replicate Cool Widgets where repository does not exist on the Geo primary site. The only way to know about this is to parse the error text. You may need to make some changes to `Gitlab::CoolWidgetReplicator.no_repo_message` to return the proper error message. For example, see [this change for Group-level Wikis](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74133). + - [ ] Generate the feature flag definition file by running the feature flag command and following the command prompts: ```shell diff --git a/CHANGELOG.md b/CHANGELOG.md index dad5599a28..a55a7771a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 14.4.4 (2021-12-03) + +No changes. + +## 14.4.3 (2021-12-01) + +### Fixed (6 changes) + +- [Check validation only if new record of license](gitlab-org/gitlab@5e0834a921dad1b1e07119de629ea44eb0ad5733) ([merge request](gitlab-org/gitlab!75421)) **GitLab Enterprise Edition** +- [Fix for hexadecimal branch deletion](gitlab-org/gitlab@fc3c2f211d5a2f190032c4d0109e2bcb31050b4d) ([merge request](gitlab-org/gitlab!75421)) +- [Geo - Fix no repo error message for group-level wikis](gitlab-org/gitlab@bdf3a712a4bfe245dfa7e7a90c24f2fdb482e309) ([merge request](gitlab-org/gitlab!75421)) **GitLab Enterprise Edition** +- [Prevent Git operations from checking replication lag on non-Geo-secondary sites](gitlab-org/gitlab@c158c01027f61aadd1c72f0817731d368d0d58cc) ([merge request](gitlab-org/gitlab!75421)) **GitLab Enterprise Edition** +- [Allow SSO callbacks through maintenance mode](gitlab-org/gitlab@1acae9807b1808ac360a4be098a50c547c9540b9) by @dzaporozhets ([merge request](gitlab-org/gitlab!75421)) **GitLab Enterprise Edition** +- [Fix 2FA setup for LDAP users](gitlab-org/gitlab@9b9a7230aed3ffeef3e8f608dd1a569397c71684) ([merge request](gitlab-org/gitlab!75421)) + ## 14.4.2 (2021-11-08) ### Fixed (3 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index afc1b70fc5..38d3ef5d70 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -14.4.2 \ No newline at end of file +14.4.4 \ No newline at end of file diff --git a/Gemfile b/Gemfile index 1e6648df48..10eca89721 100644 --- a/Gemfile +++ b/Gemfile @@ -96,7 +96,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 a6b7f598ec..8f1cd39dca 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -560,7 +560,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) @@ -1484,7 +1484,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/VERSION b/VERSION index afc1b70fc5..38d3ef5d70 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -14.4.2 \ No newline at end of file +14.4.4 \ No newline at end of file diff --git a/app/assets/javascripts/behaviors/markdown/render_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_mermaid.js index 3f878949f9..d78c456ed5 100644 --- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js @@ -75,7 +75,7 @@ export function initMermaid(mermaid) { function importMermaidModule() { return import(/* webpackChunkName: 'mermaid' */ 'mermaid') - .then((mermaid) => { + .then(({ default: mermaid }) => { mermaidModule = initMermaid(mermaid); }) .catch((err) => { diff --git a/app/assets/javascripts/blob/openapi/index.js b/app/assets/javascripts/blob/openapi/index.js index cb251274b1..b19cc19cb8 100644 --- a/app/assets/javascripts/blob/openapi/index.js +++ b/app/assets/javascripts/blob/openapi/index.js @@ -1,5 +1,6 @@ import { SwaggerUIBundle } from 'swagger-ui-dist'; import createFlash from '~/flash'; +import { removeParams, updateHistory } from '~/lib/utils/url_utility'; import { __ } from '~/locale'; export default () => { @@ -7,9 +8,14 @@ export default () => { Promise.all([import(/* webpackChunkName: 'openapi' */ 'swagger-ui-dist/swagger-ui.css')]) .then(() => { + // Temporary fix to prevent an XSS attack due to "useUnsafeMarkdown" + // Once we upgrade Swagger to "4.0.0", we can safely remove this as it will be deprecated + // Follow-up issue: https://gitlab.com/gitlab-org/gitlab/-/issues/339696 + updateHistory({ url: removeParams(['useUnsafeMarkdown']), replace: true }); SwaggerUIBundle({ url: el.dataset.endpoint, dom_id: '#js-openapi-viewer', + useUnsafeMarkdown: false, }); }) .catch((error) => { diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index 0722a712b5..4e38133318 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/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index e0b5d6be15..1968fa81b2 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -147,7 +147,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController end def current_password_required? - !current_user.password_automatically_set? + !current_user.password_automatically_set? && current_user.allow_password_authentication_for_web? end def build_qr_code diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb index 38ba1611c4..d4c9269c68 100644 --- a/app/graphql/gitlab_schema.rb +++ b/app/graphql/gitlab_schema.rb @@ -26,6 +26,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 8c67275eb7..7cc201b6df 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 cb28025c90..aedb7df9e4 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 e252ca3662..927d6ccb28 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 cea3c7d119..b13ca4bf06 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 14f1d27157..8ef8b9763a 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 @@ -5,8 +5,6 @@ module Preloaders # stores the values in requests store. # Will only be able to preload max access level for groups where the user is a direct member class UserMaxAccessLevelInGroupsPreloader - include BulkMemberAccessLoad - def initialize(groups, user) @groups = groups @user = user @@ -19,8 +17,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 end diff --git a/app/models/project.rb b/app/models/project.rb index 2ceba10e86..6a5cf00aba 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -37,6 +37,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 774d81156b..b40c4366c2 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) @@ -169,7 +167,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) @@ -178,7 +176,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 94a9960384..a7a34b2caa 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -67,7 +67,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 39ce26526e..ed5a0f24ed 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 87573c9ad1..dc70d79312 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -90,6 +90,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) @@ -497,6 +502,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)) @@ -678,12 +687,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 f48e02ab4b..df801311aa 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 dada449989..ea494dd442 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 1e70f2d979..40e9a286af 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 68f03e8a6a..4896c2ea2e 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 0000000000..d2c83be763 --- /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 2d186dfbd9..0350dc82e4 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 e515f1e732..1cbb061784 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -1,5 +1,4 @@ - current_route_path = request.fullpath.match(%r{-/tree/[^/]+/(.+$)}).to_a[1] -- add_page_startup_graphql_call('repository/path_last_commit', { projectPath: @project.full_path, ref: current_ref, path: current_route_path || "" }) - @content_class = "limit-container-width" unless fluid_layout - @skip_current_level_breadcrumb = true - add_page_specific_style 'page_bundles/project' @@ -14,6 +13,7 @@ = render "home_panel" - if can?(current_user, :download_code, @project) && @project.repository_languages.present? + - add_page_startup_graphql_call('repository/path_last_commit', { projectPath: @project.full_path, ref: current_ref, path: current_route_path || "" }) = repository_languages_bar(@project.repository_languages) = render "archived_notice", project: @project diff --git a/config/application.rb b/config/application.rb index dba9550a3d..8bf365e5cd 100644 --- a/config/application.rb +++ b/config/application.rb @@ -376,6 +376,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 # This is needed for gitlab-shell ENV['GITLAB_PATH_OUTSIDE_HOOK'] = ENV['PATH'] diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index d33550b82d..86ddb45b6b 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -114,3 +114,5 @@ Sidekiq.configure_client do |config| config.client_middleware(&Gitlab::SidekiqMiddleware.client_configurator) end + +Sidekiq::Cron::Poller.prepend Gitlab::Patch::SidekiqCronPoller diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index e0f18f931f..413a0e53ef 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -11466,7 +11466,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. | @@ -11712,7 +11712,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. | @@ -14660,7 +14660,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. | @@ -17705,7 +17705,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/user/packages/maven_repository/index.md b/doc/user/packages/maven_repository/index.md index 1757104735..c1a46a548f 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 41320d184f..0f19557e6a 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 feb01767fd..6006a07602 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 fa871b4bc5..d7d1f52c02 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 @@ -11,7 +21,7 @@ module API optional :include_merged_yaml, type: Boolean, desc: 'Whether or not to include merged CI config yaml 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 83150bb51c..2e840e8e6e 100644 --- a/lib/api/merge_request_approvals.rb +++ b/lib/api/merge_request_approvals.rb @@ -25,9 +25,7 @@ module API # Examples: # GET /projects/:id/merge_requests/:merge_request_iid/approvals desc 'List approvals for merge request' - get 'approvals' do - not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project) - + get 'approvals', urgency: :low do 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 470f78a7dc..8fa7138af4 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 21c1b7969a..96d1a69c03 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 e0e5ca615a..9b3cb85ee3 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 d47900b816..705400a549 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/checks/branch_check.rb b/lib/gitlab/checks/branch_check.rb index cfff6e919d..237a6bbb0f 100644 --- a/lib/gitlab/checks/branch_check.rb +++ b/lib/gitlab/checks/branch_check.rb @@ -40,6 +40,7 @@ module Gitlab private def prohibited_branch_checks + return if deletion? return unless Feature.enabled?(:prohibit_hexadecimal_branch_names, project, default_enabled: true) if branch_name =~ /\A\h{40}\z/ diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index bfe3f06a56..ab55a592de 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 6def3a074a..04ed585723 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 7612bd36ac..5c5c74ca1a 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 0963eb6b72..f8f6151126 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 ce886cb873..dd7ec361dd 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/patch/sidekiq_cron_poller.rb b/lib/gitlab/patch/sidekiq_cron_poller.rb new file mode 100644 index 0000000000..56ca24c68f --- /dev/null +++ b/lib/gitlab/patch/sidekiq_cron_poller.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Gitlab + module Patch + module SidekiqCronPoller + def enqueue + Rails.application.reloader.wrap do + ::Gitlab::WithRequestStore.with_request_store do + super + ensure + ::Gitlab::Database::LoadBalancing.release_hosts + end + end + end + end + end +end diff --git a/lib/gitlab/quick_actions/extractor.rb b/lib/gitlab/quick_actions/extractor.rb index 1294e47514..2e4817e6b1 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 8b2f786a91..904fc744c6 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -57,7 +57,7 @@ module Gitlab end def maven_version_regex - @maven_version_regex ||= /\A(\.?[\w\+-]+\.?)+\z/.freeze + @maven_version_regex ||= /\A(?!.*\.\.)[\w+.-]+\z/.freeze end def maven_app_group_regex diff --git a/lib/gitlab/sidekiq_enq.rb b/lib/gitlab/sidekiq_enq.rb index d8a01ac8ef..8095eba970 100644 --- a/lib/gitlab/sidekiq_enq.rb +++ b/lib/gitlab/sidekiq_enq.rb @@ -11,6 +11,16 @@ module Gitlab class SidekiqEnq def enqueue_jobs(now = Time.now.to_f.to_s, sorted_sets = Sidekiq::Scheduled::SETS) + Rails.application.reloader.wrap do + ::Gitlab::WithRequestStore.with_request_store do + find_jobs_and_enqueue(now, sorted_sets) + end + ensure + ::Gitlab::Database::LoadBalancing.release_hosts + end + end + + def find_jobs_and_enqueue(now, sorted_sets) # A job's "score" in Redis is the time at which it should be processed. # Just check Redis for the set of jobs with a timestamp before now. Sidekiq.redis do |conn| diff --git a/lib/gitlab/slash_commands/deploy.rb b/lib/gitlab/slash_commands/deploy.rb index 157d924f99..9fcefd99f8 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 45dc6cf7fd..0ceec39782 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 b13b25d1cf..2a89dc6621 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 56e7413c53..bb182d96bc 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -40994,6 +40994,9 @@ msgstr "" msgid "must be a valid IPv4 or IPv6 address" msgstr "" +msgid "must be a valid json schema" +msgstr "" + msgid "must be after start" msgstr "" diff --git a/package.json b/package.json index b8ad3a5e84..9a7b586558 100644 --- a/package.json +++ b/package.json @@ -148,7 +148,7 @@ "lowlight": "^1.20.0", "marked": "^0.3.12", "mathjax": "3", - "mermaid": "^8.13.2", + "mermaid": "^8.13.4", "minimatch": "^3.0.4", "monaco-editor": "^0.25.2", "monaco-editor-webpack-plugin": "^4.0.0", diff --git a/spec/controllers/dashboard/todos_controller_spec.rb b/spec/controllers/dashboard/todos_controller_spec.rb index f0aa351bee..c1ce32abb6 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) 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 6e7bcfdaa0..f9b15c9a48 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/controllers/profiles/two_factor_auths_controller_spec.rb b/spec/controllers/profiles/two_factor_auths_controller_spec.rb index e57bd5be93..48331d74dc 100644 --- a/spec/controllers/profiles/two_factor_auths_controller_spec.rb +++ b/spec/controllers/profiles/two_factor_auths_controller_spec.rb @@ -62,6 +62,32 @@ RSpec.describe Profiles::TwoFactorAuthsController do expect(flash[:alert]).to be_nil end end + + context 'when password authentication is disabled' do + before do + stub_application_setting(password_authentication_enabled_for_web: false) + end + + it 'does not require the current password', :aggregate_failures do + go + + expect(response).not_to redirect_to(redirect_path) + expect(flash[:alert]).to be_nil + end + end + + context 'when the user is an LDAP user' do + before do + allow(user).to receive(:ldap_user?).and_return(true) + end + + it 'does not require the current password', :aggregate_failures do + go + + expect(response).not_to redirect_to(redirect_path) + expect(flash[:alert]).to be_nil + end + end end describe 'GET show' do diff --git a/spec/factories/diff_position.rb b/spec/factories/diff_position.rb index 41f9a7b574..bd248452de 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 8281e82959..9d05c985af 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 2559814660..308098c72a 100644 --- a/spec/features/projects/members/list_spec.rb +++ b/spec/features/projects/members/list_spec.rb @@ -147,7 +147,7 @@ RSpec.describe 'Project members list', :js do it 'does not show form used to change roles and "Expiration date" or the remove user button', :aggregate_failures do visit_members_page - page.within find_member_row(project_bot) do + page.within find_username_row(project_bot) do expect(page).not_to have_button('Maintainer') expect(page).to have_field('Expiration date', disabled: true) expect(page).not_to have_button('Remove member') diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index 59ad7d31ea..7ca1788769 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 6fbed21acd..15ec11c256 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 73de0a6d38..55c0141552 100644 --- a/spec/frontend/diffs/store/utils_spec.js +++ b/spec/frontend/diffs/store/utils_spec.js @@ -138,7 +138,7 @@ describe('DiffsStoreUtils', () => { old_line: 1, }, linePosition: LINE_POSITION_LEFT, - lineRange: { start_line_code: 'abc_1_1', end_line_code: 'abc_2_2' }, + lineRange: { start: { line_code: 'abc_1_1' }, end: { line_code: 'abc_2_2' } }, }; const position = JSON.stringify({ @@ -608,7 +608,7 @@ describe('DiffsStoreUtils', () => { // When multi line comments are fully implemented `line_code` will be // included in all requests. Until then we need to ensure the logic does // not change when it is included only in the "comparison" argument. - const lineRange = { start_line_code: 'abc_1_1', end_line_code: 'abc_1_2' }; + const lineRange = { start: { line_code: 'abc_1_1' }, end: { line_code: 'abc_1_2' } }; it('returns true when the discussion is up to date', () => { expect( diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index 3fa0dc9512..02c686af68 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 0bad8c95ba..4e3f442dc7 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 9e87065887..17dcbab09b 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 8d1c3aa878..6b542278fa 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 9c9a157d68..14dc60e1a5 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 cef6a2ddcc..1562c38829 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/checks/branch_check_spec.rb b/spec/lib/gitlab/checks/branch_check_spec.rb index 3086cb1bd3..f503759f3f 100644 --- a/spec/lib/gitlab/checks/branch_check_spec.rb +++ b/spec/lib/gitlab/checks/branch_check_spec.rb @@ -32,6 +32,15 @@ RSpec.describe Gitlab::Checks::BranchCheck do expect { subject.validate! }.not_to raise_error end + context "deleting a hexadecimal branch" do + let(:newrev) { "0000000000000000000000000000000000000000" } + let(:ref) { "refs/heads/267208abfe40e546f5e847444276f7d43a39503e" } + + it "doesn't prohibit the deletion of a hexadecimal branch name" do + expect { subject.validate! }.not_to raise_error + end + end + context "the feature flag is disabled" do it "doesn't prohibit a 40-character hexadecimal branch name" do stub_feature_flags(prohibit_hexadecimal_branch_names: false) diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb index a5ab1047a4..46c33d7b7b 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 41877a16eb..b6bdc5ff49 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 8385cba353..f0e710be2e 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 bdeaabec1f..752ef7f6b5 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 diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb index 5ada8a6ef4..27175dc8c4 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/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb index 847d6b5d1e..8b9ca90a28 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 49df231392..80ba50976a 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/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb index f512f49764..79cf20fbac 100644 --- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb @@ -675,6 +675,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 } }) @@ -716,13 +717,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 @@ -751,13 +758,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 @@ -786,13 +794,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 @@ -871,7 +880,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/import_export/relation_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb index 5e4075c2b5..41d4c1f0b9 100644 --- a/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb @@ -105,7 +105,7 @@ RSpec.describe Gitlab::ImportExport::RelationTreeRestorer do it_behaves_like 'import project successfully' context '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 @@ -122,7 +122,7 @@ RSpec.describe Gitlab::ImportExport::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 @@ -155,7 +155,7 @@ RSpec.describe Gitlab::ImportExport::RelationTreeRestorer do context 'when restoring a group' do let_it_be(:group) { create(:group) } - let_it_be(:importable) { create(:group, parent: group) } + let_it_be(:importable) { create(:group, parent: group).tap { |g| g.add_owner(user) } } let(:path) { 'spec/fixtures/lib/gitlab/import_export/group_exports/no_children/group.json' } let(:importable_name) { nil } diff --git a/spec/lib/gitlab/quick_actions/extractor_spec.rb b/spec/lib/gitlab/quick_actions/extractor_spec.rb index 61fffe3fb6..c040a70e40 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 9514654204..05f1c88a6a 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -344,6 +344,18 @@ RSpec.describe Gitlab::Regex do describe '.maven_version_regex' do subject { described_class.maven_version_regex } + it 'has no ReDoS issues with long strings' do + Timeout.timeout(5) do + expect(subject).to match("aaaaaaaa.aaaaaaaaa+aa-111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111") + end + end + + it 'has no ReDos issues with long strings ending with an exclamation mark' do + Timeout.timeout(5) do + expect(subject).not_to match('a' * 50000 + '!') + end + end + it { is_expected.to match('0')} it { is_expected.to match('1') } it { is_expected.to match('03') } @@ -364,6 +376,7 @@ RSpec.describe Gitlab::Regex do it { is_expected.to match('703220b4e2cea9592caeb9f3013f6b1e5335c293') } it { is_expected.to match('RELEASE') } it { is_expected.not_to match('..1.2.3') } + it { is_expected.not_to match('1.2.3..beta') } it { is_expected.not_to match(' 1.2.3') } it { is_expected.not_to match("1.2.3 \r\t") } it { is_expected.not_to match("\r\t 1.2.3") } diff --git a/spec/lib/gitlab/slash_commands/deploy_spec.rb b/spec/lib/gitlab/slash_commands/deploy_spec.rb index 36f47c711b..71fca1e1fc 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 c78103f33f..3152dc2ad2 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 9d5f029fff..6f2ca719bc 100644 --- a/spec/lib/sidebars/projects/menus/analytics_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/analytics_menu_spec.rb @@ -102,6 +102,12 @@ RSpec.describe Sidebars::Projects::Menus::AnalyticsMenu do specify { is_expected.to be_nil } end + describe 'when a user does not have access to repository graphs' do + let(:current_user) { guest } + + specify { is_expected.to be_nil } + end + describe 'when the user does not have access' do let(:current_user) { nil } diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 2573c01d68..996fe0757e 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 8144e1ad23..1ad744db76 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,13 +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 - queries = ActiveRecord::QueryRecorder.new do - groups.each { |group| user.can?(:read_group, group) } - end - - max_queries = queries.log.grep(max_query_regex) - - expect(max_queries.count).to eq(expected_query_count) + expect { groups.each { |group| user.can?(:read_group, group) } } + .to make_queries_matching(max_query_regex, expected_query_count) end end diff --git a/spec/policies/merge_request_policy_spec.rb b/spec/policies/merge_request_policy_spec.rb index b94df4d437..e05de25f18 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 59b805bb25..1cba3674d2 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 d7f22b9d61..8c701414be 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 dd6afa869e..1c79093a83 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -224,7 +224,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 d31f571e63..791db11780 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -372,30 +372,36 @@ RSpec.describe API::Todos do expect(response).to have_gitlab_http_status(:not_found) end end - - it 'returns an error if the issuable author does not have access' do - project_1.add_guest(issuable.author) - - post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.iid}/todo", issuable.author) - - expect(response).to have_gitlab_http_status(:not_found) - end end describe 'POST :id/issuable_type/:issueable_id/todo' do context 'for an issue' do - it_behaves_like 'an issuable', 'issues' do - let_it_be(:issuable) do - create(:issue, :confidential, author: author_1, project: project_1) - end + let_it_be(:issuable) do + create(:issue, :confidential, project: project_1) + end + + it_behaves_like 'an issuable', 'issues' + + it 'returns an error if the issue author does not have access' do + post api("/projects/#{project_1.id}/issues/#{issuable.iid}/todo", issuable.author) + + expect(response).to have_gitlab_http_status(:not_found) end end context 'for a merge request' do - it_behaves_like 'an issuable', 'merge_requests' do - let_it_be(:issuable) do - create(:merge_request, :simple, source_project: project_1) - end + let_it_be(:issuable) do + create(:merge_request, :simple, source_project: project_1) + end + + it_behaves_like 'an issuable', 'merge_requests' + + it 'returns an error if the merge request author does not have access' do + project_1.add_guest(issuable.author) + + post api("/projects/#{project_1.id}/merge_requests/#{issuable.iid}/todo", issuable.author) + + expect(response).to have_gitlab_http_status(:forbidden) end end end diff --git a/spec/services/ci/job_artifacts/create_service_spec.rb b/spec/services/ci/job_artifacts/create_service_spec.rb index e6d9f20809..c4657d138f 100644 --- a/spec/services/ci/job_artifacts/create_service_spec.rb +++ b/spec/services/ci/job_artifacts/create_service_spec.rb @@ -24,6 +24,8 @@ RSpec.describe Ci::JobArtifacts::CreateService do def file_to_upload(path, params = {}) upload = Tempfile.new('upload') FileUtils.copy(path, upload.path) + # This is a workaround for https://github.com/docker/for-linux/issues/1015 + FileUtils.touch(upload.path) UploadedFile.new(upload.path, **params) end diff --git a/spec/services/protected_branches/create_service_spec.rb b/spec/services/protected_branches/create_service_spec.rb index 45462831a3..756c775be9 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 88e58ad590..b5cf1a54af 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/spec_helper.rb b/spec/spec_helper.rb index c866459869..f3dce375ae 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -465,3 +465,14 @@ Rugged::Settings['search_path_global'] = Rails.root.join('tmp/tests').to_s # Initialize FactoryDefault to use create_default helper TestProf::FactoryDefault.init + +module TouchRackUploadedFile + def initialize_from_file_path(path) + super + + # This is a no-op workaround for https://github.com/docker/for-linux/issues/1015 + File.utime @tempfile.atime, @tempfile.mtime, @tempfile.path # rubocop:disable Gitlab/ModuleWithInstanceVariables + end +end + +Rack::Test::UploadedFile.prepend(TouchRackUploadedFile) diff --git a/spec/support/helpers/features/members_helpers.rb b/spec/support/helpers/features/members_helpers.rb index 2e86e014a1..bdadcb8af4 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 6f17d3cb49..065dea7fd5 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 @@ -625,6 +626,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 759b22f794..eafa589a1d 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 518c5b8dc2..7f2c445e93 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 e6f9e5a434..28813a23fe 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 83eb0e2f3d..01caf4ab0b 100644 --- a/spec/validators/json_schema_validator_spec.rb +++ b/spec/validators/json_schema_validator_spec.rb @@ -46,5 +46,17 @@ RSpec.describe JsonSchemaValidator do expect { subject }.to raise_error(described_class::FilenameError) end end + + describe 'hash_conversion option' do + context 'when hash_conversion is enabled' do + let(:validator) { described_class.new(attributes: [:data], filename: "build_report_result_data", hash_conversion: true) } + + it 'returns no errors' do + subject + + expect(build_report_result.errors).to be_empty + end + end + end end end diff --git a/yarn.lock b/yarn.lock index 6746a1d71f..846c439740 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4683,12 +4683,7 @@ domhandler@^4.0.0, domhandler@^4.2.0: dependencies: domelementtype "^2.2.0" -dompurify@2.3.1: - version "2.3.1" - resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-2.3.1.tgz#a47059ca21fd1212d3c8f71fdea6943b8bfbdf6a" - integrity sha512-xGWt+NHAQS+4tpgbOAI08yxW0Pr256Gu/FNE2frZVTbgrBUn8M7tz7/ktS/LZ2MHeGqz6topj0/xY+y8R5FBFw== - -dompurify@^2.3.3: +dompurify@2.3.3, dompurify@^2.3.3: version "2.3.3" resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-2.3.3.tgz#c1af3eb88be47324432964d8abc75cf4b98d634c" integrity sha512-dqnqRkPMAjOZE0FogZ+ceJNM2dZ3V/yNOuFB7+39qpO93hHhfRpHw3heYQC7DPK9FqbQTfBKUJhiSfz4MvXYwg== @@ -8114,16 +8109,16 @@ merge2@^1.3.0: resolved "https://registry.yarnpkg.com/merge2/-/merge2-1.4.1.tgz#4368892f885e907455a6fd7dc55c0c9d404990ae" integrity sha512-8q7VEgMJW4J8tcfVPy8g09NcQwZdbwFEqhe/WZkoIzjn/3TGDwtOCYtXGxA3O8tPzpczCCDgv+P2P5y00ZJOOg== -mermaid@^8.13.2: - version "8.13.2" - resolved "https://registry.yarnpkg.com/mermaid/-/mermaid-8.13.2.tgz#9f8abc66ba1c53b132fdaa0d4a80f4717b7b7655" - integrity sha512-qTFI7MfC2d+x0Hft5gx063EH9tZg36lERG8o7Zq0Ag+MnO8CgVaMZEU6oA8gzTtTn9upMdy4UlYSLVmavu27cQ== +mermaid@^8.13.4: + version "8.13.4" + resolved "https://registry.yarnpkg.com/mermaid/-/mermaid-8.13.4.tgz#924cb85f39380285e0a99f245c66cfa61014a2e1" + integrity sha512-zdWtsXabVy1PEAE25Jkm4zbTDlQe8rqNlTMq2B3j+D+NxDskJEY5OsgalarvNLsw+b5xFa1a8D1xcm/PijrDow== dependencies: "@braintree/sanitize-url" "^3.1.0" d3 "^7.0.0" dagre "^0.8.5" dagre-d3 "^0.6.4" - dompurify "2.3.1" + dompurify "2.3.3" graphlib "^2.1.8" khroma "^1.4.1" moment-mini "^2.24.0"