diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 98fdda3593..4eb17656ed 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,4 +1,4 @@ -image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.5.3-golang-1.11-git-2.18-chrome-71.0-node-10.x-yarn-1.12-postgresql-9.6-graphicsmagick-1.3.29" +image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.5.3-golang-1.11-git-2.18-chrome-73.0-node-10.x-yarn-1.12-postgresql-9.6-graphicsmagick-1.3.29" include: - local: /lib/gitlab/ci/templates/Code-Quality.gitlab-ci.yml @@ -655,7 +655,7 @@ gitlab:setup-mysql: # Frontend-related jobs gitlab:assets:compile: <<: *dedicated-no-docs-pull-cache-job - image: dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.5.3-git-2.18-chrome-71.0-node-8.x-yarn-1.12-graphicsmagick-1.3.29-docker-18.06.1 + image: dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.5.3-git-2.18-chrome-73.0-node-8.x-yarn-1.12-graphicsmagick-1.3.29-docker-18.06.1 dependencies: - setup-test-env services: diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a1c80afdc..1d73e94d73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,53 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 11.10.8 (2019-06-27) + +- No changes. +### Security (10 changes) + +- Fix Denial of Service for comments when rendering issues/MR comments. +- Gate MR head_pipeline behind read_pipeline ability. +- Fix DoS vulnerability in color validation regex. +- Expose merge requests count based on user access. +- Persist tmp snippet uploads at users. +- Add missing authorizations in GraphQL. +- Disable Rails SQL query cache when applying service templates. +- Prevent Billion Laughs attack. +- Correctly check permissions when creating snippet notes. +- Prevent the detection of merge request templates by unauthorized users. + +### Performance (1 change) + +- Add improvements to global search of issues and merge requests. !27817 + + +## 11.10.7 (2019-06-26) + +### Fixed (3 changes) + +- Remove a default git depth in Pipelines for merge requests. !28926 +- Fix label click scrolling to top. !29202 +- Fix scrolling to top on assignee change. !29500 + + +## 11.10.6 (2019-06-04) + +### Fixed (7 changes, 1 of them is from the community) + +- Allow a member to have an access level equal to parent group. !27913 +- Fix uploading of LFS tracked file through UI. !28052 +- Use 3-way merge for squashing commits. !28078 +- Use a path for the related merge requests endpoint. !28171 +- Fix project visibility level validation. !28305 (Peter Marko) +- Fix Rugged get_tree_entries recursive flag not working. !28494 +- Use source ref in pipeline webhook. !28772 + +### Other (1 change) + +- Fix input group height. + + ## 11.10.5 (2019-05-30) ### Security (12 changes, 1 of them is from the community) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index a95a46d9fa..7e3856fe87 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -1.34.1 +1.34.3 diff --git a/Gemfile b/Gemfile index 1c7ad5abcb..00f90bdf7e 100644 --- a/Gemfile +++ b/Gemfile @@ -419,7 +419,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 1.19.0', require: 'gitaly' +gem 'gitaly-proto', '~> 1.22.1', require: 'gitaly' gem 'grpc', '~> 1.15.0' diff --git a/Gemfile.lock b/Gemfile.lock index 3314a76994..d498172b02 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -281,7 +281,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (1.19.0) + gitaly-proto (1.22.1) grpc (~> 1.0) github-markup (1.7.0) gitlab-default_value_for (3.1.1) @@ -1020,7 +1020,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 1.19.0) + gitaly-proto (~> 1.22.1) github-markup (~> 1.7.0) gitlab-default_value_for (~> 3.1.1) gitlab-markup (~> 1.7.0) diff --git a/VERSION b/VERSION index 5413747c68..31265f292c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -11.10.5 +11.10.8 diff --git a/app/assets/javascripts/diffs/workers/tree_worker.js b/app/assets/javascripts/diffs/workers/tree_worker.js index 534d737c77..415c463fd1 100644 --- a/app/assets/javascripts/diffs/workers/tree_worker.js +++ b/app/assets/javascripts/diffs/workers/tree_worker.js @@ -4,6 +4,11 @@ import { generateTreeList } from '../store/utils'; // eslint-disable-next-line no-restricted-globals self.addEventListener('message', e => { const { data } = e; + + if (data === undefined) { + return; + } + const { treeEntries, tree } = generateTreeList(data); // eslint-disable-next-line no-restricted-globals diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index 1c6b18c0e0..7ac947c983 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -561,6 +561,11 @@ GitLabDropdown = (function() { !$target.data('isLink') ) { e.stopPropagation(); + + // This prevents automatic scrolling to the top + if ($target.closest('a').length) { + return false; + } } return true; diff --git a/app/assets/stylesheets/framework/forms.scss b/app/assets/stylesheets/framework/forms.scss index 1c23c14c2d..6671336658 100644 --- a/app/assets/stylesheets/framework/forms.scss +++ b/app/assets/stylesheets/framework/forms.scss @@ -275,3 +275,7 @@ label { max-width: $input-lg-width; width: 100%; } + +.input-group-text { + max-height: $input-height; +} diff --git a/app/assets/stylesheets/framework/variables_overrides.scss b/app/assets/stylesheets/framework/variables_overrides.scss index fb4d3f23cd..ea96381a09 100644 --- a/app/assets/stylesheets/framework/variables_overrides.scss +++ b/app/assets/stylesheets/framework/variables_overrides.scss @@ -7,6 +7,7 @@ $secondary: $gray-light; $input-disabled-bg: $gray-light; $input-border-color: $gray-200; $input-color: $gl-text-color; +$input-font-size: $gl-font-size; $font-family-sans-serif: $regular-font; $font-family-monospace: $monospace-font; $btn-line-height: 20px; diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index 0c99ff5341..37071a57bb 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -75,6 +75,8 @@ input[type='checkbox']:hover { } .search-input-wrap { + width: 100%; + .search-icon, .clear-icon { position: absolute; diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 6d6e0cc6c7..c51041513b 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -41,7 +41,7 @@ module IssuableCollections return if pagination_disabled? @issuables = @issuables.page(params[:page]) - @issuable_meta_data = issuable_meta_data(@issuables, collection_type) + @issuable_meta_data = issuable_meta_data(@issuables, collection_type, current_user) @total_pages = issuable_page_count end # rubocop:enable Gitlab/ModuleWithInstanceVariables diff --git a/app/controllers/concerns/issuable_collections_action.rb b/app/controllers/concerns/issuable_collections_action.rb index 18ed4027ea..4ad287c4a1 100644 --- a/app/controllers/concerns/issuable_collections_action.rb +++ b/app/controllers/concerns/issuable_collections_action.rb @@ -11,7 +11,7 @@ module IssuableCollectionsAction .non_archived .page(params[:page]) - @issuable_meta_data = issuable_meta_data(@issues, collection_type) + @issuable_meta_data = issuable_meta_data(@issues, collection_type, current_user) respond_to do |format| format.html @@ -22,7 +22,7 @@ module IssuableCollectionsAction def merge_requests @merge_requests = issuables_collection.page(params[:page]) - @issuable_meta_data = issuable_meta_data(@merge_requests, collection_type) + @issuable_meta_data = issuable_meta_data(@merge_requests, collection_type, current_user) end # rubocop:enable Gitlab/ModuleWithInstanceVariables diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index f96d182109..0098c4cdf4 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -203,17 +203,17 @@ module NotesActions # These params are also sent by the client but we need to set these based on # target_type and target_id because we're checking permissions based on that - create_params[:noteable_type] = params[:target_type].classify + create_params[:noteable_type] = noteable.class.name - case params[:target_type] - when 'commit' - create_params[:commit_id] = params[:target_id] - when 'merge_request' - create_params[:noteable_id] = params[:target_id] + case noteable + when Commit + create_params[:commit_id] = noteable.id + when MergeRequest + create_params[:noteable_id] = noteable.id # Notes on MergeRequest can have an extra `commit_id` context create_params[:commit_id] = params.dig(:note, :commit_id) else - create_params[:noteable_id] = params[:target_id] + create_params[:noteable_id] = noteable.id end end end diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 781eac7f08..9ffde6afdb 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -13,6 +13,11 @@ class Projects::ApplicationController < ApplicationController helper_method :repository, :can_collaborate_with_project?, :user_access + rescue_from Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError do |exception| + log_exception(exception) + render_404 + end + private def project diff --git a/app/controllers/projects/templates_controller.rb b/app/controllers/projects/templates_controller.rb index 7ceea4e5b9..f987033a26 100644 --- a/app/controllers/projects/templates_controller.rb +++ b/app/controllers/projects/templates_controller.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true class Projects::TemplatesController < Projects::ApplicationController - before_action :authenticate_user!, :get_template_class + before_action :authenticate_user! + before_action :authorize_can_read_issuable! + before_action :get_template_class def show template = @template_type.find(params[:key], project) @@ -13,9 +15,20 @@ class Projects::TemplatesController < Projects::ApplicationController private + # User must have: + # - `read_merge_request` to see merge request templates, or + # - `read_issue` to see issue templates + # + # Note params[:template_type] has a route constraint to limit it to + # `merge_request` or `issue` + def authorize_can_read_issuable! + action = [:read_, params[:template_type]].join + + authorize_action!(action) + end + def get_template_class template_types = { issue: Gitlab::Template::IssueTemplate, merge_request: Gitlab::Template::MergeRequestTemplate }.with_indifferent_access @template_type = template_types[params[:template_type]] - render json: [], status: :not_found unless @template_type end end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 62b97fc259..4ab23a0b93 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -297,7 +297,7 @@ class ProjectsController < Projects::ApplicationController elsif @project.feature_available?(:issues, current_user) @issues = issuables_collection.page(params[:page]) @collection_type = 'Issue' - @issuable_meta_data = issuable_meta_data(@issues, @collection_type) + @issuable_meta_data = issuable_meta_data(@issues, @collection_type, current_user) end render :show diff --git a/app/controllers/snippets/notes_controller.rb b/app/controllers/snippets/notes_controller.rb index eee14b0faf..612897f27e 100644 --- a/app/controllers/snippets/notes_controller.rb +++ b/app/controllers/snippets/notes_controller.rb @@ -5,8 +5,8 @@ class Snippets::NotesController < ApplicationController include ToggleAwardEmoji skip_before_action :authenticate_user!, only: [:index] - before_action :snippet - before_action :authorize_read_snippet!, only: [:show, :index, :create] + before_action :authorize_read_snippet!, only: [:show, :index] + before_action :authorize_create_note!, only: [:create] private @@ -33,4 +33,8 @@ class Snippets::NotesController < ApplicationController def authorize_read_snippet! return render_404 unless can?(current_user, :read_personal_snippet, snippet) end + + def authorize_create_note! + access_denied! unless can?(current_user, :create_note, noteable) + end end diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 8ea5450b4e..fad036b8df 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -137,7 +137,7 @@ class SnippetsController < ApplicationController def move_temporary_files params[:files].each do |file| - FileMover.new(file, @snippet).execute + FileMover.new(file, from_model: current_user, to_model: @snippet).execute end end end diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 568c6e2a85..1e73bcb0db 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -41,7 +41,11 @@ class UploadsController < ApplicationController when Note can?(current_user, :read_project, model.project) when User - true + # We validate the current user has enough (writing) + # access to itself when a secret is given. + # For instance, user avatars are readable by anyone, + # while temporary, user snippet uploads are not. + !secret? || can?(current_user, :update_user, model) when Appearance true else @@ -56,8 +60,13 @@ class UploadsController < ApplicationController def authorize_create_access! return unless model - # for now we support only personal snippets comments - authorized = can?(current_user, :comment_personal_snippet, model) + authorized = + case model + when User + can?(current_user, :update_user, model) + else + can?(current_user, :comment_personal_snippet, model) + end render_unauthorized unless authorized end @@ -74,6 +83,10 @@ class UploadsController < ApplicationController User === model || Appearance === model end + def secret? + params[:secret].present? + end + def upload_model_class MODEL_CLASSES[params[:model]] || raise(UnknownUploadModelError) end diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index fa9dda2ab3..21ae580fa0 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -29,6 +29,7 @@ # updated_after: datetime # updated_before: datetime # attempt_group_search_optimizations: boolean +# attempt_project_search_optimizations: boolean # class IssuableFinder prepend FinderWithCrossProjectAccess @@ -184,7 +185,6 @@ class IssuableFinder @project = project end - # rubocop: disable CodeReuse/ActiveRecord def projects return @projects if defined?(@projects) @@ -192,17 +192,25 @@ class IssuableFinder projects = if current_user && params[:authorized_only].presence && !current_user_related? - current_user.authorized_projects + current_user.authorized_projects(min_access_level) elsif group - finder_options = { include_subgroups: params[:include_subgroups], only_owned: true } - GroupProjectsFinder.new(group: group, current_user: current_user, options: finder_options).execute # rubocop: disable CodeReuse/Finder + find_group_projects else - ProjectsFinder.new(current_user: current_user).execute # rubocop: disable CodeReuse/Finder + Project.public_or_visible_to_user(current_user, min_access_level) end - @projects = projects.with_feature_available_for_user(klass, current_user).reorder(nil) + @projects = projects.with_feature_available_for_user(klass, current_user).reorder(nil) # rubocop: disable CodeReuse/ActiveRecord + end + + def find_group_projects + return Project.none unless group + + if params[:include_subgroups] + Project.where(namespace_id: group.self_and_descendants) # rubocop: disable CodeReuse/ActiveRecord + else + group.projects + end.public_or_visible_to_user(current_user, min_access_level) end - # rubocop: enable CodeReuse/ActiveRecord def search params[:search].presence @@ -572,4 +580,8 @@ class IssuableFinder scope = params[:scope] scope == 'created_by_me' || scope == 'authored' || scope == 'assigned_to_me' end + + def min_access_level + ProjectFeature.required_minimum_access_level(klass) + end end diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index cb44575d6f..c827059643 100644 --- a/app/finders/issues_finder.rb +++ b/app/finders/issues_finder.rb @@ -48,9 +48,9 @@ class IssuesFinder < IssuableFinder OR (issues.confidential = TRUE AND (issues.author_id = :user_id OR EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = :user_id AND issue_id = issues.id) - OR issues.project_id IN(:project_ids)))', + OR EXISTS (:authorizations)))', user_id: current_user.id, - project_ids: current_user.authorized_projects(CONFIDENTIAL_ACCESS_LEVEL).select(:id)) + authorizations: current_user.authorizations_for_projects(min_access_level: CONFIDENTIAL_ACCESS_LEVEL, related_project_column: "issues.project_id")) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 93d3c99184..23b731b1ae 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -62,7 +62,7 @@ class ProjectsFinder < UnionFinder collection = by_personal(collection) collection = by_starred(collection) collection = by_trending(collection) - collection = by_visibilty_level(collection) + collection = by_visibility_level(collection) collection = by_tags(collection) collection = by_search(collection) collection = by_archived(collection) @@ -71,12 +71,11 @@ class ProjectsFinder < UnionFinder collection end - # rubocop: disable CodeReuse/ActiveRecord def collection_with_user if owned_projects? current_user.owned_projects elsif min_access_level? - current_user.authorized_projects.where('project_authorizations.access_level >= ?', params[:min_access_level]) + current_user.authorized_projects(params[:min_access_level]) else if private_only? current_user.authorized_projects @@ -85,7 +84,6 @@ class ProjectsFinder < UnionFinder end end end - # rubocop: enable CodeReuse/ActiveRecord # Builds a collection for an anonymous user. def collection_without_user @@ -131,7 +129,7 @@ class ProjectsFinder < UnionFinder end # rubocop: disable CodeReuse/ActiveRecord - def by_visibilty_level(items) + def by_visibility_level(items) params[:visibility_level].present? ? items.where(visibility_level: params[:visibility_level]) : items end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/graphql/types/label_type.rb b/app/graphql/types/label_type.rb index ccd466edc1..41d035c1aa 100644 --- a/app/graphql/types/label_type.rb +++ b/app/graphql/types/label_type.rb @@ -4,6 +4,8 @@ module Types class LabelType < BaseObject graphql_name 'Label' + authorize :read_label + field :description, GraphQL::STRING_TYPE, null: true field :title, GraphQL::STRING_TYPE, null: false field :color, GraphQL::STRING_TYPE, null: false diff --git a/app/graphql/types/metadata_type.rb b/app/graphql/types/metadata_type.rb index 2d8bad0614..7d7813a765 100644 --- a/app/graphql/types/metadata_type.rb +++ b/app/graphql/types/metadata_type.rb @@ -4,6 +4,8 @@ module Types class MetadataType < ::Types::BaseObject graphql_name 'Metadata' + authorize :read_instance_metadata + field :version, GraphQL::STRING_TYPE, null: false field :revision, GraphQL::STRING_TYPE, null: false end diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index 0f655ab9d0..a7627e6926 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -12,10 +12,7 @@ module Types field :metadata, Types::MetadataType, null: true, resolver: Resolvers::MetadataResolver, - description: 'Metadata about GitLab' do |*args| - - authorize :read_instance_metadata - end + description: 'Metadata about GitLab' field :echo, GraphQL::STRING_TYPE, null: false, function: Functions::Echo.new end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 52c49498e9..c61c43c909 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -277,7 +277,7 @@ module IssuablesHelper initialTaskStatus: issuable.task_status } - data[:hasClosingMergeRequest] = issuable.merge_requests_count != 0 if issuable.is_a?(Issue) + data[:hasClosingMergeRequest] = issuable.merge_requests_count(current_user) != 0 if issuable.is_a?(Issue) if parent.is_a?(Group) data[:groupPath] = parent.path diff --git a/app/helpers/snippets_helper.rb b/app/helpers/snippets_helper.rb index ecb2b2d707..6ccc1fb2ed 100644 --- a/app/helpers/snippets_helper.rb +++ b/app/helpers/snippets_helper.rb @@ -1,6 +1,16 @@ # frozen_string_literal: true module SnippetsHelper + def snippets_upload_path(snippet, user) + return unless user + + if snippet&.persisted? + upload_path('personal_snippet', id: snippet.id) + else + upload_path('user', id: user.id) + end + end + def reliable_snippet_path(snippet, opts = nil) if snippet.project_id? project_snippet_path(snippet.project, snippet, opts) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 17f94b4bd9..2523d0070a 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -29,7 +29,11 @@ module Issuable # This object is used to gather issuable meta data for displaying # upvotes, downvotes, notes and closing merge requests count for issues and merge requests # lists avoiding n+1 queries and improving performance. - IssuableMeta = Struct.new(:upvotes, :downvotes, :user_notes_count, :merge_requests_count) + IssuableMeta = Struct.new(:upvotes, :downvotes, :user_notes_count, :mrs_count) do + def merge_requests_count(user = nil) + mrs_count + end + end included do cache_markdown_field :title, pipeline: :single_line diff --git a/app/models/issue.rb b/app/models/issue.rb index eb4c87e05d..44e7119239 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -270,8 +270,8 @@ class Issue < ApplicationRecord end # rubocop: enable CodeReuse/ServiceClass - def merge_requests_count - merge_requests_closing_issues.count + def merge_requests_count(user = nil) + ::MergeRequestsClosingIssues.count_for_issue(self.id, user) end private diff --git a/app/models/member.rb b/app/models/member.rb index 8a06bff51b..83b4f5b29c 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -446,10 +446,10 @@ class Member < ApplicationRecord end def higher_access_level_than_group - if highest_group_member && highest_group_member.access_level >= access_level + if highest_group_member && highest_group_member.access_level > access_level error_parameters = { access: highest_group_member.human_access, group_name: highest_group_member.group.name } - errors.add(:access_level, s_("should be higher than %{access} inherited membership from group %{group_name}") % error_parameters) + errors.add(:access_level, s_("should be greater than or equal to %{access} inherited membership from group %{group_name}") % error_parameters) end end end diff --git a/app/models/merge_requests_closing_issues.rb b/app/models/merge_requests_closing_issues.rb index 61af50841e..22cedf57b8 100644 --- a/app/models/merge_requests_closing_issues.rb +++ b/app/models/merge_requests_closing_issues.rb @@ -7,11 +7,38 @@ class MergeRequestsClosingIssues < ApplicationRecord validates :merge_request_id, uniqueness: { scope: :issue_id }, presence: true validates :issue_id, presence: true + scope :with_issues, ->(ids) { where(issue_id: ids) } + scope :with_merge_requests_enabled, -> do + joins(:merge_request) + .joins('INNER JOIN project_features ON merge_requests.target_project_id = project_features.project_id') + .where('project_features.merge_requests_access_level >= :access', access: ProjectFeature::ENABLED) + end + + scope :accessible_by, ->(user) do + joins(:merge_request) + .joins('INNER JOIN project_features ON merge_requests.target_project_id = project_features.project_id') + .where('project_features.merge_requests_access_level >= :access OR EXISTS(:authorizations)', + access: ProjectFeature::ENABLED, + authorizations: user.authorizations_for_projects(min_access_level: Gitlab::Access::REPORTER, related_project_column: "merge_requests.target_project_id") + ) + end + class << self - def count_for_collection(ids) - group(:issue_id) - .where(issue_id: ids) - .pluck('issue_id', 'COUNT(*) as count') + def count_for_collection(ids, current_user) + closing_merge_requests(ids, current_user).group(:issue_id).pluck('issue_id', 'COUNT(*) as count') + end + + def count_for_issue(id, current_user) + closing_merge_requests(id, current_user).count + end + + private + + def closing_merge_requests(ids, current_user) + return with_issues(ids) if current_user&.admin? + return with_issues(ids).with_merge_requests_enabled if current_user.blank? + + with_issues(ids).accessible_by(current_user) end end end diff --git a/app/models/project.rb b/app/models/project.rb index 873f93a3b0..d0b4c85c2a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -461,10 +461,12 @@ class Project < ApplicationRecord # Returns a collection of projects that is either public or visible to the # logged in user. - def self.public_or_visible_to_user(user = nil) + def self.public_or_visible_to_user(user = nil, min_access_level = nil) + min_access_level = nil if user&.admin? + if user where('EXISTS (?) OR projects.visibility_level IN (?)', - user.authorizations_for_projects, + user.authorizations_for_projects(min_access_level: min_access_level), Gitlab::VisibilityLevel.levels_for_user(user)) else public_to_user @@ -474,30 +476,32 @@ class Project < ApplicationRecord # project features may be "disabled", "internal", "enabled" or "public". If "internal", # they are only available to team members. This scope returns projects where # the feature is either public, enabled, or internal with permission for the user. + # Note: this scope doesn't enforce that the user has access to the projects, it just checks + # that the user has access to the feature. It's important to use this scope with others + # that checks project authorizations first. # # This method uses an optimised version of `with_feature_access_level` for # logged in users to more efficiently get private projects with the given # feature. def self.with_feature_available_for_user(feature, user) visible = [ProjectFeature::ENABLED, ProjectFeature::PUBLIC] - min_access_level = ProjectFeature.required_minimum_access_level(feature) if user&.admin? with_feature_enabled(feature) elsif user + min_access_level = ProjectFeature.required_minimum_access_level(feature) column = ProjectFeature.quoted_access_level_column(feature) with_project_feature - .where( - "(projects.visibility_level > :private AND (#{column} IS NULL OR #{column} >= (:public_visible) OR (#{column} = :private_visible AND EXISTS(:authorizations))))"\ - " OR (projects.visibility_level = :private AND (#{column} IS NULL OR #{column} >= :private_visible) AND EXISTS(:authorizations))", - { - private: Gitlab::VisibilityLevel::PRIVATE, - public_visible: ProjectFeature::ENABLED, - private_visible: ProjectFeature::PRIVATE, - authorizations: user.authorizations_for_projects(min_access_level: min_access_level) - }) + .where("#{column} IS NULL OR #{column} IN (:public_visible) OR (#{column} = :private_visible AND EXISTS (:authorizations))", + { + public_visible: visible, + private_visible: ProjectFeature::PRIVATE, + authorizations: user.authorizations_for_projects(min_access_level: min_access_level) + }) else + # This has to be added to include features whose value is nil in the db + visible << nil with_feature_access_level(feature, visible) end end diff --git a/app/models/user.rb b/app/models/user.rb index d3524bfd6a..541ec3098e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -761,11 +761,15 @@ class User < ApplicationRecord # Typically used in conjunction with projects table to get projects # a user has been given access to. + # The param `related_project_column` is the column to compare to the + # project_authorizations. By default is projects.id # # Example use: # `Project.where('EXISTS(?)', user.authorizations_for_projects)` - def authorizations_for_projects(min_access_level: nil) - authorizations = project_authorizations.select(1).where('project_authorizations.project_id = projects.id') + def authorizations_for_projects(min_access_level: nil, related_project_column: 'projects.id') + authorizations = project_authorizations + .select(1) + .where("project_authorizations.project_id = #{related_project_column}") return authorizations unless min_access_level.present? diff --git a/app/policies/repository_policy.rb b/app/policies/repository_policy.rb new file mode 100644 index 0000000000..3234074985 --- /dev/null +++ b/app/policies/repository_policy.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class RepositoryPolicy < BasePolicy + delegate { @subject.project } +end diff --git a/app/presenters/ci/build_runner_presenter.rb b/app/presenters/ci/build_runner_presenter.rb index ed3daf6585..6d46e0bf18 100644 --- a/app/presenters/ci/build_runner_presenter.rb +++ b/app/presenters/ci/build_runner_presenter.rb @@ -4,7 +4,6 @@ module Ci class BuildRunnerPresenter < SimpleDelegator include Gitlab::Utils::StrongMemoize - DEFAULT_GIT_DEPTH_MERGE_REQUEST = 10 RUNNER_REMOTE_TAG_PREFIX = 'refs/tags/'.freeze RUNNER_REMOTE_BRANCH_PREFIX = 'refs/remotes/origin/'.freeze @@ -28,7 +27,6 @@ module Ci def git_depth strong_memoize(:git_depth) do git_depth = variables&.find { |variable| variable[:key] == 'GIT_DEPTH' }&.dig(:value) - git_depth ||= DEFAULT_GIT_DEPTH_MERGE_REQUEST if merge_request_ref? git_depth.to_i end end @@ -39,12 +37,13 @@ module Ci if git_depth > 0 specs << refspec_for_branch(ref) if branch? || legacy_detached_merge_request_pipeline? specs << refspec_for_tag(ref) if tag? - specs << refspec_for_merge_request_ref if merge_request_ref? else specs << refspec_for_branch specs << refspec_for_tag end + specs << refspec_for_merge_request_ref if merge_request_ref? + specs end diff --git a/app/services/lfs/file_transformer.rb b/app/services/lfs/file_transformer.rb index 6ecf583cb6..5239fe1b6e 100644 --- a/app/services/lfs/file_transformer.rb +++ b/app/services/lfs/file_transformer.rb @@ -24,7 +24,7 @@ module Lfs def new_file(file_path, file_content, encoding: nil) if project.lfs_enabled? && lfs_file?(file_path) - file_content = Base64.decode64(file_content) if encoding == 'base64' + file_content = parse_file_content(file_content, encoding: encoding) lfs_pointer_file = Gitlab::Git::LfsPointerFile.new(file_content) lfs_object = create_lfs_object!(lfs_pointer_file, file_content) @@ -66,5 +66,12 @@ module Lfs def link_lfs_object!(lfs_object) project.lfs_objects << lfs_object end + + def parse_file_content(file_content, encoding: nil) + return file_content.read if file_content.respond_to?(:read) + return Base64.decode64(file_content) if encoding == 'base64' + + file_content + end end end diff --git a/app/services/projects/after_import_service.rb b/app/services/projects/after_import_service.rb index afb9048e87..bbdde4408d 100644 --- a/app/services/projects/after_import_service.rb +++ b/app/services/projects/after_import_service.rb @@ -9,7 +9,7 @@ module Projects end def execute - Projects::HousekeepingService.new(@project, :gc).execute do + Projects::HousekeepingService.new(@project).execute do repository.delete_all_refs_except(RESERVED_REF_PREFIXES) end rescue Projects::HousekeepingService::LeaseTaken => e diff --git a/app/services/projects/propagate_service_template.rb b/app/services/projects/propagate_service_template.rb index 633a263af7..9c753a7a91 100644 --- a/app/services/projects/propagate_service_template.rb +++ b/app/services/projects/propagate_service_template.rb @@ -24,7 +24,7 @@ module Projects def propagate_projects_with_template loop do - batch = project_ids_batch + batch = Project.uncached { project_ids_batch } bulk_create_from_template(batch) unless batch.empty? diff --git a/app/uploaders/file_mover.rb b/app/uploaders/file_mover.rb index 236b7ed2b3..12be1e2bb2 100644 --- a/app/uploaders/file_mover.rb +++ b/app/uploaders/file_mover.rb @@ -1,22 +1,29 @@ # frozen_string_literal: true class FileMover - attr_reader :secret, :file_name, :model, :update_field + include Gitlab::Utils::StrongMemoize - def initialize(file_path, model, update_field = :description) + attr_reader :secret, :file_name, :from_model, :to_model, :update_field + + def initialize(file_path, update_field = :description, from_model:, to_model:) @secret = File.split(File.dirname(file_path)).last @file_name = File.basename(file_path) - @model = model + @from_model = from_model + @to_model = to_model @update_field = update_field end def execute + temp_file_uploader.retrieve_from_store!(file_name) + return unless valid? + uploader.retrieve_from_store!(file_name) + move if update_markdown - uploader.record_upload + update_upload_model uploader.schedule_background_upload end end @@ -24,52 +31,77 @@ class FileMover private def valid? - Pathname.new(temp_file_path).realpath.to_path.start_with?( - (Pathname(temp_file_uploader.root) + temp_file_uploader.base_dir).to_path - ) + if temp_file_uploader.file_storage? + Pathname.new(temp_file_path).realpath.to_path.start_with?( + (Pathname(temp_file_uploader.root) + temp_file_uploader.base_dir).to_path + ) + else + temp_file_uploader.exists? + end end def move - FileUtils.mkdir_p(File.dirname(file_path)) - FileUtils.move(temp_file_path, file_path) + if temp_file_uploader.file_storage? + FileUtils.mkdir_p(File.dirname(file_path)) + FileUtils.move(temp_file_path, file_path) + else + uploader.copy_file(temp_file_uploader.file) + temp_file_uploader.upload.destroy! + end end def update_markdown - updated_text = model.read_attribute(update_field) - .gsub(temp_file_uploader.markdown_link, uploader.markdown_link) - model.update_attribute(update_field, updated_text) + updated_text = to_model.read_attribute(update_field) + .gsub(temp_file_uploader.markdown_link, uploader.markdown_link) + to_model.update_attribute(update_field, updated_text) rescue revert false end + def update_upload_model + return unless upload = temp_file_uploader.upload + return if upload.destroyed? + + upload.update!(model: to_model) + end + def temp_file_path - return @temp_file_path if @temp_file_path - - temp_file_uploader.retrieve_from_store!(file_name) - - @temp_file_path = temp_file_uploader.file.path + strong_memoize(:temp_file_path) do + temp_file_uploader.file.path + end end def file_path - return @file_path if @file_path - - uploader.retrieve_from_store!(file_name) - - @file_path = uploader.file.path + strong_memoize(:file_path) do + uploader.file.path + end end def uploader - @uploader ||= PersonalFileUploader.new(model, secret: secret) + @uploader ||= + begin + uploader = PersonalFileUploader.new(to_model, secret: secret) + + # Enforcing a REMOTE object storage given FileUploader#retrieve_from_store! won't do it + # (there's no upload at the target yet). + if uploader.class.object_store_enabled? + uploader.object_store = ::ObjectStorage::Store::REMOTE + end + + uploader + end end def temp_file_uploader - @temp_file_uploader ||= PersonalFileUploader.new(nil, secret: secret) + @temp_file_uploader ||= PersonalFileUploader.new(from_model, secret: secret) end def revert - Rails.logger.warn("Markdown not updated, file move reverted for #{model}") + Rails.logger.warn("Markdown not updated, file move reverted for #{to_model}") - FileUtils.move(file_path, temp_file_path) + if temp_file_uploader.file_storage? + FileUtils.move(file_path, temp_file_path) + end end end diff --git a/app/validators/color_validator.rb b/app/validators/color_validator.rb index 1932d042e8..974dfbbf39 100644 --- a/app/validators/color_validator.rb +++ b/app/validators/color_validator.rb @@ -12,7 +12,7 @@ # end # class ColorValidator < ActiveModel::EachValidator - PATTERN = /\A\#[0-9A-Fa-f]{3}{1,2}+\Z/.freeze + PATTERN = /\A\#(?:[0-9A-Fa-f]{3}){1,2}\Z/.freeze def validate_each(record, attribute, value) unless value =~ PATTERN diff --git a/app/views/layouts/snippets.html.haml b/app/views/layouts/snippets.html.haml index 5f986c81ff..841b2a5e79 100644 --- a/app/views/layouts/snippets.html.haml +++ b/app/views/layouts/snippets.html.haml @@ -1,9 +1,10 @@ - header_title _("Snippets"), snippets_path +- snippets_upload_path = snippets_upload_path(@snippet, current_user) - content_for :page_specific_javascripts do - - if @snippet && current_user + - if snippets_upload_path -# haml-lint:disable InlineJavaScript :javascript - window.uploads_path = "#{upload_path('personal_snippet', id: @snippet.id)}"; + window.uploads_path = "#{snippets_upload_path}"; = render template: "layouts/application" diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index f4d8aab8a8..48a4a59dcf 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -77,7 +77,7 @@ = edited_time_ago_with_tooltip(@issue, placement: 'bottom', html_class: 'issue-edited-ago js-issue-edited-ago') - #js-related-merge-requests{ data: { endpoint: expose_url(api_v4_projects_issues_related_merge_requests_path(id: @project.id, issue_iid: @issue.iid)), project_namespace: @project.namespace.path, project_path: @project.path } } + #js-related-merge-requests{ data: { endpoint: expose_path(api_v4_projects_issues_related_merge_requests_path(id: @project.id, issue_iid: @issue.iid)), project_namespace: @project.namespace.path, project_path: @project.path } } - if can?(current_user, :download_code, @project) #related-branches{ data: { url: related_branches_project_issue_path(@project, @issue) } } diff --git a/app/views/shared/_issuable_meta_data.html.haml b/app/views/shared/_issuable_meta_data.html.haml index 31a5370a5f..71b13a5d74 100644 --- a/app/views/shared/_issuable_meta_data.html.haml +++ b/app/views/shared/_issuable_meta_data.html.haml @@ -2,7 +2,7 @@ - issue_votes = @issuable_meta_data[issuable.id] - upvotes, downvotes = issue_votes.upvotes, issue_votes.downvotes - issuable_url = @collection_type == "Issue" ? issue_path(issuable, anchor: 'notes') : merge_request_path(issuable, anchor: 'notes') -- issuable_mr = @issuable_meta_data[issuable.id].merge_requests_count +- issuable_mr = @issuable_meta_data[issuable.id].merge_requests_count(current_user) - if issuable_mr > 0 %li.issuable-mr.d-none.d-sm-block.has-tooltip{ title: _('Related merge requests') } diff --git a/config/routes/project.rb b/config/routes/project.rb index 93d168fc59..bde482f0b6 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -41,7 +41,10 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do # # Templates # - get '/templates/:template_type/:key' => 'templates#show', as: :template, constraints: { key: %r{[^/]+} } + get '/templates/:template_type/:key' => 'templates#show', + as: :template, + defaults: { format: 'json' }, + constraints: { key: %r{[^/]+}, template_type: /issue|merge_request/, format: 'json' } resource :avatar, only: [:show, :destroy] resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do diff --git a/config/routes/uploads.rb b/config/routes/uploads.rb index b594f55f8a..920f8454ce 100644 --- a/config/routes/uploads.rb +++ b/config/routes/uploads.rb @@ -7,7 +7,7 @@ scope path: :uploads do # show uploads for models, snippets (notes) available for now get '-/system/:model/:id/:secret/:filename', to: 'uploads#show', - constraints: { model: /personal_snippet/, id: /\d+/, filename: %r{[^/]+} } + constraints: { model: /personal_snippet|user/, id: /\d+/, filename: %r{[^/]+} } # show temporary uploads get '-/system/temp/:secret/:filename', @@ -28,7 +28,7 @@ scope path: :uploads do # create uploads for models, snippets (notes) available for now post ':model', to: 'uploads#create', - constraints: { model: /personal_snippet/, id: /\d+/ }, + constraints: { model: /personal_snippet|user/, id: /\d+/ }, as: 'upload' end diff --git a/debian/changelog b/debian/changelog index 8545da73da..5c7fa93032 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,20 @@ +gitlab (11.10.8+dfsg-1) experimental; urgency=medium + + [ Pirate Praveen ] + * New upstream security release 11.10.8+dfsg (Fixes: CVE-2019-13001, + CVE-2019-13002, CVE-2019-13003, CVE-2019-13004, CVE-2019-13005, + CVE-2019-13006, CVE-2019-13007, CVE-2019-13009,CVE-2019-13010, + CVE-2019-13011, CVE-2019-13121) + * Refresh patches + * Use packaged versions of node-autosize, axios, brace-expansion, chart.js, + core-js, css-loader, d3-* sub modules, fuzzaldrin-plus, glob, jed, jquery, + jquery-ujs, jquery.waitforimages, js-cookie, jszip, jszip-utils, mousetrap, + popper.js, raven-js, bootstrap, three-orbit-control, three-stl-loader, + timeago.js, dateformat, webpack-stats-plugin and vue-resource + * Use packaged pikaday and update path in application.scss (Closes: #930529) + + -- Pirate Praveen Sun, 07 Jul 2019 13:14:52 +0530 + gitlab (11.10.5+dfsg-1+fto10+1) buster-fasttrack; urgency=medium * Rebuild for buster-fasttrack. diff --git a/debian/control b/debian/control index 0f80fc6403..27c354ec63 100644 --- a/debian/control +++ b/debian/control @@ -289,7 +289,7 @@ Depends: ${shlibs:Depends}, ${misc:Depends}, ruby-ed25519 (>= 1.2~), ruby-bcrypt-pbkdf (>= 1.0~), # Gitaly GRPC client - ruby-gitaly-proto (>= 1.19~), + ruby-gitaly-proto (>= 1.22.1~), ruby-grpc (>= 1.15~), ruby-google-protobuf (>= 3.6~), # @@ -303,51 +303,63 @@ Depends: ${shlibs:Depends}, ${misc:Depends}, ruby-grape-logging (>= 1.7~), # Vendored js files # Keeping this to ease backporting as it is in contrib anyway -# libjs-jquery-atwho, -# libjs-jquery-caret.js, libjs-pdf, # libjs-xterm, # libjs-jquery-nicescroll, -# libjs-clipboard, -# libjs-chartjs, # libjs-graphael, -# node-lie, -# node modules - all node packages are stuck in NEW -# using npm for all to ease backporting as it is in contrib anyway +# packaged node modules - all node packages are not packaged yet + node-autosize (>= 4.0~), + node-axios (>= 0.17.1~), + node-brace-expansion (>= 1.1.8~), + node-bootstrap, + node-chart.js (>= 2.7.2~), + node-core-js, + node-css-loader, + node-d3-array, + node-d3-axis, + node-d3-brush, + node-d3-ease, + node-d3-scale (>= 1.0.7~), + node-d3-selection (>= 1.2~), + node-d3-shape, + node-d3-time (>= 1.0.8~), + node-d3-time-format (>= 2.1.1~), + node-d3-transition (>= 1.1.1~), + node-dateformat, + node-fuzzaldrin-plus (>= 0.5~), + node-glob, + node-jed, + node-jquery (>= 3.2.1~), + node-jquery-ujs, + node-jquery.waitforimages, + node-js-cookie, + node-jszip, + node-jszip-utils, + node-mousetrap, + node-pikaday, + node-popper.js, + node-raven-js, + node-three-orbit-controls, + node-three-stl-loader, + node-timeago.js, + node-underscore, + node-vue-resource (>= 1.5.1~), + node-webpack-stats-plugin, +# using npm for remaining as it is in contrib # node-babel-core, # node-babel-eslint, # node-babel-loader, # node-babel-plugin-transform-define, # node-babel-preset-latest, # node-babel-preset-stage-2, -# node-bootstrap-sass, -# node-core-js, -# node-d3-array, -# node-d3-axis, -# node-d3-brush, -# node-d3-scale, -# node-d3-selection, -# node-d3-shape, -# node-d3-time, -# node-d3-time-format, # node-debug (>= 3.1.0~), # node-exports-loader, # node-file-loader, -# node-glob, # node-imports-loader, -# node-jed, -# node-jquery, -# node-js-cookie, -# node-jszip, -# node-jszip-utils, # node-katex, # node-marked, -# node-mousetrap, # node-raw-loader, -# node-stats-webpack-plugin, -# node-underscore, # node-url-loader, -# node-katex Recommends: certbot, gitaly (>= 1.34.1~) Conflicts: libruby2.3 diff --git a/debian/patches/0050-relax-stable-libs.patch b/debian/patches/0050-relax-stable-libs.patch index e1593a3694..fd7f84828e 100644 --- a/debian/patches/0050-relax-stable-libs.patch +++ b/debian/patches/0050-relax-stable-libs.patch @@ -12,7 +12,7 @@ gitlab Gemfile # Improves copy-on-write performance for MRI gem 'nakayoshi_fork', '~> 0.0.4' -@@ -9,7 +9,7 @@ gem 'nakayoshi_fork', '~> 0.0.4' +@@ -9,7 +9,7 @@ # Responders respond_to and respond_with gem 'responders', '~> 2.0' @@ -21,7 +21,7 @@ gitlab Gemfile # Default values for AR models gem 'gitlab-default_value_for', '~> 3.1.1', require: 'default_value_for' -@@ -28,57 +28,57 @@ gem 'devise', '~> 4.4' +@@ -28,57 +28,57 @@ gem 'doorkeeper', '~> 4.3' gem 'doorkeeper-openid_connect', '~> 1.5' gem 'omniauth', '~> 1.8' @@ -97,7 +97,7 @@ gitlab Gemfile # Disable strong_params so that Mash does not respond to :permitted? gem 'hashie-forbidden_attributes' -@@ -87,7 +87,7 @@ gem 'hashie-forbidden_attributes' +@@ -87,7 +87,7 @@ gem 'kaminari', '~> 1.0' # HAML @@ -106,7 +106,7 @@ gitlab Gemfile # Files attachments gem 'carrierwave', '~> 1.3' -@@ -97,7 +97,7 @@ gem 'mini_magick' +@@ -97,7 +97,7 @@ gem 'fog-aws', '~> 3.3' # Locked until fog-google resolves https://github.com/fog/fog-google/issues/421. # Also see config/initializers/fog_core_patch.rb. @@ -115,7 +115,7 @@ gitlab Gemfile gem 'fog-google', '~> 1.8' gem 'fog-local', '~> 0.6' gem 'fog-openstack', '~> 1.0' -@@ -111,38 +111,38 @@ gem 'google-api-client', '~> 0.23' +@@ -111,38 +111,38 @@ gem 'unf', '~> 0.1.4' # Seed data @@ -164,7 +164,7 @@ gitlab Gemfile gem 'unicorn-worker-killer', '~> 0.4.4' end -@@ -158,9 +158,9 @@ gem 'state_machines-activerecord', '~> 0.5.1' +@@ -158,9 +158,9 @@ gem 'acts-as-taggable-on', '~> 6.0' # Background jobs @@ -176,7 +176,7 @@ gitlab Gemfile gem 'gitlab-sidekiq-fetcher', '~> 0.4.0', require: 'sidekiq-reliable-fetch' # Cron Parser -@@ -176,14 +176,14 @@ gem 'rainbow', '~> 3.0' +@@ -176,14 +176,14 @@ gem 'ruby-progressbar' # GitLab settings @@ -194,7 +194,7 @@ gitlab Gemfile # Export Ruby Regex to Javascript gem 'js_regex', '~> 3.1' -@@ -192,7 +192,7 @@ gem 'js_regex', '~> 3.1' +@@ -192,7 +192,7 @@ gem 'device_detector' # Cache @@ -203,7 +203,7 @@ gitlab Gemfile # Redis gem 'redis', '~> 3.2' -@@ -202,7 +202,7 @@ gem 'connection_pool', '~> 2.0' +@@ -202,7 +202,7 @@ gem 'discordrb-webhooks-blackst0ne', '~> 3.3', require: false # HipChat integration @@ -212,7 +212,7 @@ gitlab Gemfile # JIRA integration gem 'jira-ruby', '~> 1.4' -@@ -211,7 +211,7 @@ gem 'jira-ruby', '~> 1.4' +@@ -211,7 +211,7 @@ gem 'flowdock', '~> 0.7' # Slack integration @@ -221,7 +221,7 @@ gitlab Gemfile # Hangouts Chat integration gem 'hangouts-chat', '~> 0.0.5' -@@ -223,11 +223,11 @@ gem 'asana', '~> 0.8.1' +@@ -223,11 +223,11 @@ gem 'ruby-fogbugz', '~> 0.2.1' # Kubernetes integration @@ -235,7 +235,7 @@ gitlab Gemfile # Sanitizes SVG input gem 'loofah', '~> 2.2' -@@ -236,10 +236,10 @@ gem 'loofah', '~> 2.2' +@@ -236,10 +236,10 @@ gem 'licensee', '~> 8.9' # Protect against bruteforcing @@ -248,7 +248,7 @@ gitlab Gemfile # Detect and convert string character encoding gem 'charlock_holmes', '~> 0.7.5' -@@ -257,39 +257,39 @@ gem 'chronic_duration', '~> 0.10.6' +@@ -257,39 +257,39 @@ gem 'webpack-rails', '~> 0.9.10' gem 'rack-proxy', '~> 0.6.0' @@ -300,7 +300,7 @@ gitlab Gemfile # Metrics group :metrics do -@@ -315,54 +315,54 @@ group :development do +@@ -315,54 +315,54 @@ gem 'rblineprof', '~> 0.3.6', platform: :mri, require: false # Better errors handler @@ -370,7 +370,7 @@ gitlab Gemfile gem 'license_finder', '~> 5.4', require: false gem 'knapsack', '~> 1.17' -@@ -371,18 +371,18 @@ group :development, :test do +@@ -371,18 +371,18 @@ gem 'stackprof', '~> 0.2.10', require: false @@ -395,7 +395,7 @@ gitlab Gemfile gem 'concurrent-ruby', '~> 1.1' gem 'test-prof', '~> 0.2.5' gem 'rspec_junit_formatter' -@@ -402,11 +402,11 @@ gem 'rbtrace', '~> 0.4', require: false +@@ -402,11 +402,11 @@ gem 'oauth2', '~> 1.4' # Health check @@ -410,13 +410,13 @@ gitlab Gemfile # SSH host key support gem 'net-ssh', '~> 5.0' -@@ -419,13 +419,13 @@ group :ed25519 do +@@ -419,13 +419,13 @@ end # Gitaly GRPC client --gem 'gitaly-proto', '~> 1.19.0', require: 'gitaly' -+gem 'gitaly-proto', '~> 1.19', require: 'gitaly' - +-gem 'gitaly-proto', '~> 1.22.1', require: 'gitaly' ++gem 'gitaly-proto', '~> 1.22', '>= 1.22.1', require: 'gitaly' + -gem 'grpc', '~> 1.15.0' +gem 'grpc', '~> 1.15' diff --git a/debian/patches/0680-rails-5_1.patch b/debian/patches/0680-rails-5_1.patch index 57441b1ccf..45600354d6 100644 --- a/debian/patches/0680-rails-5_1.patch +++ b/debian/patches/0680-rails-5_1.patch @@ -316,7 +316,7 @@ Model.new.attributes now also returns encrypted attributes. acts_as_ordered_taggable -@@ -1442,7 +1442,7 @@ +@@ -1446,7 +1446,7 @@ # update visibility_level of forks def update_forks_visibility_level @@ -1957,16 +1957,7 @@ Model.new.attributes now also returns encrypted attributes. end --- a/lib/gitlab/search_results.rb +++ b/lib/gitlab/search_results.rb -@@ -113,7 +113,7 @@ - issues.full_search(query) - end - -- issues.reorder('updated_at DESC') -+ issues.reorder('issues.updated_at DESC') - end - # rubocop: enable CodeReuse/ActiveRecord - -@@ -123,7 +123,7 @@ +@@ -107,7 +107,7 @@ milestones = filter_milestones_by_project(milestones) @@ -1975,15 +1966,6 @@ Model.new.attributes now also returns encrypted attributes. end # rubocop: enable CodeReuse/ActiveRecord -@@ -141,7 +141,7 @@ - merge_requests.full_search(query) - end - -- merge_requests.reorder('updated_at DESC') -+ merge_requests.reorder('merge_requests.updated_at DESC') - end - # rubocop: enable CodeReuse/ActiveRecord - --- a/spec/fixtures/api/schemas/entities/issue.json +++ b/spec/fixtures/api/schemas/entities/issue.json @@ -5,7 +5,7 @@ diff --git a/debian/patches/0730-install-graphql-tag.patch b/debian/patches/0730-install-graphql-tag.patch index 03067c7964..4a2352c34b 100644 --- a/debian/patches/0730-install-graphql-tag.patch +++ b/debian/patches/0730-install-graphql-tag.patch @@ -1,8 +1,8 @@ Description: Install graphql-tag via yarnpkg Author: Utkarsh Gupta ---- gitlab-11.10.4+dfsg.orig/package.json -+++ gitlab-11.10.4+dfsg/package.json +--- a/package.json ++++ b/package.json @@ -78,6 +78,7 @@ "fuzzaldrin-plus": "^0.5.0", "glob": "^7.1.2", diff --git a/debian/patches/0740-use-packaged-modules.patch b/debian/patches/0740-use-packaged-modules.patch new file mode 100644 index 0000000000..6d041448f3 --- /dev/null +++ b/debian/patches/0740-use-packaged-modules.patch @@ -0,0 +1,108 @@ +--- a/package.json ++++ b/package.json +@@ -39,32 +39,14 @@ + "apollo-client": "^2.5.1", + "apollo-upload-client": "^10.0.0", + "at.js": "^1.5.4", +- "autosize": "^4.0.0", +- "axios": "^0.17.1", + "babel-loader": "^8.0.5", +- "bootstrap": "4.3.1", +- "brace-expansion": "^1.1.8", + "cache-loader": "^2.0.1", +- "chart.js": "2.7.2", + "classlist-polyfill": "^1.2.0", + "clipboard": "^1.7.1", + "codesandbox-api": "^0.0.20", + "compression-webpack-plugin": "^2.0.0", +- "core-js": "^2.4.1", + "cropper": "^2.3.0", +- "css-loader": "^1.0.0", + "d3": "^4.13.0", +- "d3-array": "^1.2.1", +- "d3-axis": "^1.0.8", +- "d3-brush": "^1.0.4", +- "d3-ease": "^1.0.3", +- "d3-scale": "^1.0.7", +- "d3-selection": "^1.2.0", +- "d3-shape": "^1.2.0", +- "d3-time": "^1.0.8", +- "d3-time-format": "^2.1.1", +- "d3-transition": "^1.1.1", +- "dateformat": "^3.0.3", + "deckar01-task_list": "^2.2.0", + "diff": "^3.4.0", + "document-register-element": "1.13.1", +@@ -75,33 +57,20 @@ + "exports-loader": "^0.7.0", + "file-loader": "^3.0.1", + "formdata-polyfill": "^3.0.11", +- "fuzzaldrin-plus": "^0.5.0", +- "glob": "^7.1.2", + "graphql": "^14.0.2", + "graphql-tag": "^2.0.0", + "imports-loader": "^0.8.0", +- "jed": "^1.1.1", + "jest-transform-graphql": "^2.1.0", +- "jquery": "^3.2.1", +- "jquery-ujs": "1.2.2", + "jquery.caret": "^0.3.1", +- "jquery.waitforimages": "^2.2.0", +- "js-cookie": "^2.1.3", +- "jszip": "^3.1.3", +- "jszip-utils": "^0.0.2", + "katex": "^0.10.0", + "marked": "^0.3.12", + "mermaid": "^8.0.0-rc.8", + "monaco-editor": "^0.15.6", + "monaco-editor-webpack-plugin": "^1.7.0", +- "mousetrap": "^1.4.6", +- "pikaday": "^1.6.1", +- "popper.js": "^1.14.7", + "prismjs": "^1.6.0", + "prosemirror-markdown": "^1.3.0", + "prosemirror-model": "^1.6.4", + "raphael": "^2.2.7", +- "raven-js": "^3.22.1", + "raw-loader": "^1.0.0", + "sanitize-html": "^1.16.1", + "select2": "3.5.2-browserify", +@@ -114,19 +83,14 @@ + "stylelint-error-string-formatter": "1.0.2", + "svg4everybody": "2.1.9", + "three": "^0.84.0", +- "three-orbit-controls": "^82.1.0", +- "three-stl-loader": "^1.0.4", +- "timeago.js": "^3.0.2", + "tiptap": "^1.8.0", + "tiptap-commands": "^1.4.0", + "tiptap-extensions": "^1.8.0", +- "underscore": "^1.9.0", + "url-loader": "^1.1.2", + "visibilityjs": "^1.2.4", + "vue": "^2.6.10", + "vue-apollo": "^3.0.0-beta.28", + "vue-loader": "^15.7.0", +- "vue-resource": "^1.5.1", + "vue-router": "^3.0.2", + "vue-template-compiler": "^2.6.10", + "vue-virtual-scroll-list": "^1.3.1", +@@ -134,7 +98,6 @@ + "webpack": "^4.29.0", + "webpack-bundle-analyzer": "^3.0.3", + "webpack-cli": "^3.2.1", +- "webpack-stats-plugin": "^0.2.1", + "worker-loader": "^2.0.0", + "xterm": "^3.5.0" + }, +--- a/app/assets/stylesheets/application.scss ++++ b/app/assets/stylesheets/application.scss +@@ -12,7 +12,7 @@ + // If you need to add unique style that should affect only one page - use pages/ + // directory. + @import "../../../node_modules/at.js/dist/css/jquery.atwho"; +-@import "../../../node_modules/pikaday/scss/pikaday"; ++@import "/usr/lib/nodejs/pikaday/scss/pikaday"; + @import "../../../node_modules/dropzone/dist/basic"; + @import "../../../node_modules/select2/select2"; + diff --git a/debian/patches/bump-devise-to-4-6.patch b/debian/patches/bump-devise-to-4-6.patch index 6127e0a1e4..3e6926a97a 100644 --- a/debian/patches/bump-devise-to-4-6.patch +++ b/debian/patches/bump-devise-to-4-6.patch @@ -3,9 +3,9 @@ https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/28438 Author: Utkarsh Gupta --- ---- gitlab-11.10.4+dfsg.orig/Gemfile -+++ gitlab-11.10.4+dfsg/Gemfile -@@ -35,7 +35,7 @@ gem 'grape-path-helpers', '~> 1.0' +--- a/Gemfile ++++ b/Gemfile +@@ -35,7 +35,7 @@ gem 'faraday', '~> 0.12' # Authentication libraries @@ -14,9 +14,9 @@ Author: Utkarsh Gupta gem 'doorkeeper', '~> 4.3' gem 'doorkeeper-openid_connect', '~> 1.5' gem 'omniauth', '~> 1.8' ---- gitlab-11.10.4+dfsg.orig/Gemfile.lock -+++ gitlab-11.10.4+dfsg/Gemfile.lock -@@ -153,7 +153,7 @@ GEM +--- a/Gemfile.lock ++++ b/Gemfile.lock +@@ -153,7 +153,7 @@ descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) device_detector (1.0.0) @@ -25,7 +25,7 @@ Author: Utkarsh Gupta bcrypt (~> 3.0) orm_adapter (~> 0.1) railties (>= 4.1.0, < 6.0) -@@ -986,7 +986,7 @@ DEPENDENCIES +@@ -986,7 +986,7 @@ database_cleaner (~> 1.7.0) deckar01-task_list (= 2.2.0) device_detector @@ -34,9 +34,9 @@ Author: Utkarsh Gupta devise-two-factor (~> 3.0.0) diffy (~> 3.1.0) discordrb-webhooks-blackst0ne (~> 3.3) ---- gitlab-11.10.4+dfsg.orig/app/models/user.rb -+++ gitlab-11.10.4+dfsg/app/models/user.rb -@@ -1494,15 +1494,6 @@ class User < ApplicationRecord +--- a/app/models/user.rb ++++ b/app/models/user.rb +@@ -1498,15 +1498,6 @@ devise_mailer.__send__(notification, self, *args).deliver_later # rubocop:disable GitlabSecurity/PublicSend end @@ -52,8 +52,8 @@ Author: Utkarsh Gupta def ensure_user_rights_and_limits if external? self.can_create_group = false ---- gitlab-11.10.4+dfsg.orig/app/views/devise/confirmations/new.html.haml -+++ gitlab-11.10.4+dfsg/app/views/devise/confirmations/new.html.haml +--- a/app/views/devise/confirmations/new.html.haml ++++ b/app/views/devise/confirmations/new.html.haml @@ -3,7 +3,7 @@ .login-body = form_for(resource, as: resource_name, url: confirmation_path(resource_name), html: { method: :post, class: 'gl-show-field-errors' }) do |f| @@ -63,8 +63,8 @@ Author: Utkarsh Gupta .form-group = f.label :email = f.email_field :email, class: "form-control", required: true, title: 'Please provide a valid email address.' ---- gitlab-11.10.4+dfsg.orig/app/views/devise/passwords/edit.html.haml -+++ gitlab-11.10.4+dfsg/app/views/devise/passwords/edit.html.haml +--- a/app/views/devise/passwords/edit.html.haml ++++ b/app/views/devise/passwords/edit.html.haml @@ -3,7 +3,7 @@ .login-body = form_for(resource, as: resource_name, url: password_path(:user), html: { method: :put, class: 'gl-show-field-errors' }) do |f| @@ -74,8 +74,8 @@ Author: Utkarsh Gupta = f.hidden_field :reset_password_token .form-group = f.label 'New password', for: "user_password" ---- gitlab-11.10.4+dfsg.orig/app/views/devise/passwords/new.html.haml -+++ gitlab-11.10.4+dfsg/app/views/devise/passwords/new.html.haml +--- a/app/views/devise/passwords/new.html.haml ++++ b/app/views/devise/passwords/new.html.haml @@ -3,7 +3,7 @@ .login-body = form_for(resource, as: resource_name, url: password_path(resource_name), html: { method: :post, class: 'gl-show-field-errors' }) do |f| @@ -85,8 +85,8 @@ Author: Utkarsh Gupta .form-group = f.label :email = f.email_field :email, class: "form-control", required: true, value: params[:user_email], autofocus: true, title: 'Please provide a valid email address.' ---- gitlab-11.10.4+dfsg.orig/app/views/devise/registrations/edit.html.erb -+++ gitlab-11.10.4+dfsg/app/views/devise/registrations/edit.html.erb +--- a/app/views/devise/registrations/edit.html.erb ++++ b/app/views/devise/registrations/edit.html.erb @@ -1,7 +1,7 @@

Edit <%= resource_name.to_s.humanize %>

@@ -96,8 +96,8 @@ Author: Utkarsh Gupta
<%= f.label :email %>
<%= f.email_field :email %>
---- gitlab-11.10.4+dfsg.orig/app/views/devise/shared/_signup_box.html.haml -+++ gitlab-11.10.4+dfsg/app/views/devise/shared/_signup_box.html.haml +--- a/app/views/devise/shared/_signup_box.html.haml ++++ b/app/views/devise/shared/_signup_box.html.haml @@ -2,7 +2,7 @@ .login-body = form_for(resource, as: "new_#{resource_name}", url: registration_path(resource_name), html: { class: "new_new_user gl-show-field-errors", "aria-live" => "assertive" }) do |f| @@ -107,8 +107,8 @@ Author: Utkarsh Gupta .name.form-group = f.label :name, 'Full name', class: 'label-bold' = f.text_field :name, class: "form-control top qa-new-user-name js-block-emoji", required: true, title: _("This field is required.") ---- gitlab-11.10.4+dfsg.orig/app/views/devise/unlocks/new.html.haml -+++ gitlab-11.10.4+dfsg/app/views/devise/unlocks/new.html.haml +--- a/app/views/devise/unlocks/new.html.haml ++++ b/app/views/devise/unlocks/new.html.haml @@ -3,7 +3,7 @@ .login-body = form_for(resource, as: resource_name, url: unlock_path(resource_name), html: { method: :post, class: 'gl-show-field-errors' }) do |f| @@ -118,9 +118,9 @@ Author: Utkarsh Gupta .form-group.append-bottom-20 = f.label :email = f.email_field :email, class: 'form-control', autofocus: 'autofocus', autocapitalize: 'off', autocorrect: 'off', title: 'Please provide a valid email address.' ---- gitlab-11.10.4+dfsg.orig/config/initializers/8_devise.rb -+++ gitlab-11.10.4+dfsg/config/initializers/8_devise.rb -@@ -100,6 +100,11 @@ Devise.setup do |config| +--- a/config/initializers/8_devise.rb ++++ b/config/initializers/8_devise.rb +@@ -100,6 +100,11 @@ # secure: true in order to force SSL only cookies. # config.cookie_options = {} diff --git a/debian/patches/series b/debian/patches/series index f2f02f5e95..f73636dc45 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -32,4 +32,5 @@ 0700-add-salsa-link-to-help.patch 0710-use-yarnpkg.patch 0730-install-graphql-tag.patch +0740-use-packaged-modules.patch bump-devise-to-4-6.patch diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 079ee7f5cc..b6884f79de 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -493,9 +493,9 @@ module API expose :state, :created_at, :updated_at # Avoids an N+1 query when metadata is included - def issuable_metadata(subject, options, method) + def issuable_metadata(subject, options, method, args = nil) cached_subject = options.dig(:issuable_metadata, subject.id) - (cached_subject || subject).public_send(method) # rubocop: disable GitlabSecurity/PublicSend + (cached_subject || subject).public_send(method, *args) # rubocop: disable GitlabSecurity/PublicSend end end @@ -554,7 +554,7 @@ module API end expose(:user_notes_count) { |issue, options| issuable_metadata(issue, options, :user_notes_count) } - expose(:merge_requests_count) { |issue, options| issuable_metadata(issue, options, :merge_requests_count) } + expose(:merge_requests_count) { |issue, options| issuable_metadata(issue, options, :merge_requests_count, options[:current_user]) } expose(:upvotes) { |issue, options| issuable_metadata(issue, options, :upvotes) } expose(:downvotes) { |issue, options| issuable_metadata(issue, options, :downvotes) } expose :due_date @@ -731,7 +731,9 @@ module API merge_request.metrics&.pipeline end - expose :head_pipeline, using: 'API::Entities::Pipeline' + expose :head_pipeline, using: 'API::Entities::Pipeline', if: -> (_, options) do + Ability.allowed?(options[:current_user], :read_pipeline, options[:project]) + end expose :diff_refs, using: Entities::DiffRefs diff --git a/lib/api/helpers/related_resources_helpers.rb b/lib/api/helpers/related_resources_helpers.rb index 793ae11b41..9cdde25fe4 100644 --- a/lib/api/helpers/related_resources_helpers.rb +++ b/lib/api/helpers/related_resources_helpers.rb @@ -13,6 +13,10 @@ module API available?(:merge_requests, project, options[:current_user]) end + def expose_path(path) + Gitlab::Utils.append_path(Gitlab.config.gitlab.relative_url_root, path) + end + def expose_url(path) url_options = Gitlab::Application.routes.default_url_options protocol, host, port, script_name = url_options.values_at(:protocol, :host, :port, :script_name) diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 999a9cb5a8..54ee62e3e8 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -93,7 +93,7 @@ module API options = { with: Entities::IssueBasic, current_user: current_user, - issuable_metadata: issuable_meta_data(issues, 'Issue') + issuable_metadata: issuable_meta_data(issues, 'Issue', current_user) } present issues, options @@ -120,7 +120,7 @@ module API options = { with: Entities::IssueBasic, current_user: current_user, - issuable_metadata: issuable_meta_data(issues, 'Issue') + issuable_metadata: issuable_meta_data(issues, 'Issue', current_user) } present issues, options @@ -150,7 +150,7 @@ module API with: Entities::IssueBasic, current_user: current_user, project: user_project, - issuable_metadata: issuable_meta_data(issues, 'Issue') + issuable_metadata: issuable_meta_data(issues, 'Issue', current_user) } present issues, options diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index f164e1c1ee..460fb2ed6b 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -71,7 +71,7 @@ module API if params[:view] == 'simple' options[:with] = Entities::MergeRequestSimple else - options[:issuable_metadata] = issuable_meta_data(merge_requests, 'MergeRequest') + options[:issuable_metadata] = issuable_meta_data(merge_requests, 'MergeRequest', current_user) end options diff --git a/lib/api/todos.rb b/lib/api/todos.rb index d2196f0517..f332a554c4 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -65,7 +65,7 @@ module API next unless collection targets = collection.map(&:target) - options[type] = { issuable_metadata: issuable_meta_data(targets, type) } + options[type] = { issuable_metadata: issuable_meta_data(targets, type, current_user) } end end end diff --git a/lib/banzai/filter/relative_link_filter.rb b/lib/banzai/filter/relative_link_filter.rb index 2745905c5f..f68cd9b941 100644 --- a/lib/banzai/filter/relative_link_filter.rb +++ b/lib/banzai/filter/relative_link_filter.rb @@ -100,7 +100,7 @@ module Banzai end def relative_file_path(uri) - path = Addressable::URI.unescape(uri.path) + path = Addressable::URI.unescape(uri.path).delete("\0") request_path = Addressable::URI.unescape(context[:requested_path]) nested_path = build_relative_path(path, request_path) file_exists?(nested_path) ? nested_path : path diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 15643fa03a..456e8a4f50 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -15,6 +15,9 @@ module Gitlab @global = Entry::Global.new(@config) @global.compose! + rescue Gitlab::Config::Loader::Yaml::DataTooLargeError => e + Gitlab::Sentry.track_exception(e, extra: { user: user.inspect, project: project.inspect }) + raise Config::ConfigError, e.message rescue Gitlab::Config::Loader::FormatError, Extendable::ExtensionError, External::Processor::IncludeError => e diff --git a/lib/gitlab/config/loader/yaml.rb b/lib/gitlab/config/loader/yaml.rb index 8159f8b802..4cedab1545 100644 --- a/lib/gitlab/config/loader/yaml.rb +++ b/lib/gitlab/config/loader/yaml.rb @@ -4,6 +4,13 @@ module Gitlab module Config module Loader class Yaml + DataTooLargeError = Class.new(Loader::FormatError) + + include Gitlab::Utils::StrongMemoize + + MAX_YAML_SIZE = 1.megabyte + MAX_YAML_DEPTH = 100 + def initialize(config) @config = YAML.safe_load(config, [Symbol], [], true) rescue Psych::Exception => e @@ -11,16 +18,35 @@ module Gitlab end def valid? - @config.is_a?(Hash) + hash? && !too_big? end def load! - unless valid? - raise Loader::FormatError, 'Invalid configuration format' - end + raise DataTooLargeError, 'The parsed YAML is too big' if too_big? + raise Loader::FormatError, 'Invalid configuration format' unless hash? @config.deep_symbolize_keys end + + private + + def hash? + @config.is_a?(Hash) + end + + def too_big? + return false unless Feature.enabled?(:ci_yaml_limit_size, default_enabled: true) + + !deep_size.valid? + end + + def deep_size + strong_memoize(:deep_size) do + Gitlab::Utils::DeepSize.new(@config, + max_size: MAX_YAML_SIZE, + max_depth: MAX_YAML_DEPTH) + end + end end end end diff --git a/lib/gitlab/data_builder/pipeline.rb b/lib/gitlab/data_builder/pipeline.rb index fa06fb935f..e1e813849b 100644 --- a/lib/gitlab/data_builder/pipeline.rb +++ b/lib/gitlab/data_builder/pipeline.rb @@ -19,7 +19,7 @@ module Gitlab def hook_attrs(pipeline) { id: pipeline.id, - ref: pipeline.ref, + ref: pipeline.source_ref, tag: pipeline.tag, sha: pipeline.sha, before_sha: pipeline.before_sha, diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 7f5eb1188f..cc61bb7fa0 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -905,6 +905,12 @@ module Gitlab end end + def remove_foreign_key_if_exists(*args) + if foreign_key_exists?(*args) + remove_foreign_key(*args) + end + end + def remove_foreign_key_without_error(*args) remove_foreign_key(*args) rescue ArgumentError diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index be9e926728..97b2929a34 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -303,6 +303,11 @@ module Gitlab (size.to_f / 1024).round(2) end + # Return git object directory size in bytes + def object_directory_size + gitaly_repository_client.get_object_directory_size.to_f * 1024 + end + # Build an array of commits. # # Usage. diff --git a/lib/gitlab/git/rugged_impl/tree.rb b/lib/gitlab/git/rugged_impl/tree.rb index bb13d114d4..9c37bb0196 100644 --- a/lib/gitlab/git/rugged_impl/tree.rb +++ b/lib/gitlab/git/rugged_impl/tree.rb @@ -43,6 +43,8 @@ module Gitlab ordered_entries.concat(tree_entries_from_rugged(repository, sha, entry.path, true)) end end + + ordered_entries end def rugged_populate_flat_path(repository, sha, path, entries) diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index a08bfd0e25..16820a29e8 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -47,6 +47,13 @@ module Gitlab response.size end + def get_object_directory_size + request = Gitaly::GetObjectDirectorySizeRequest.new(repository: @gitaly_repo) + response = GitalyClient.call(@storage, :repository_service, :get_object_directory_size, request, timeout: GitalyClient.medium_timeout) + + response.size + end + def apply_gitattributes(revision) request = Gitaly::ApplyGitattributesRequest.new(repository: @gitaly_repo, revision: encode_binary(revision)) GitalyClient.call(@storage, :repository_service, :apply_gitattributes, request, timeout: GitalyClient.fast_timeout) diff --git a/lib/gitlab/graphql/authorize/authorize_field_service.rb b/lib/gitlab/graphql/authorize/authorize_field_service.rb index 03d6aabb0e..de9f193c57 100644 --- a/lib/gitlab/graphql/authorize/authorize_field_service.rb +++ b/lib/gitlab/graphql/authorize/authorize_field_service.rb @@ -39,6 +39,8 @@ module Gitlab type = node_type_for_basic_connection(type) end + type = type.unwrap if type.kind.non_null? + Array.wrap(type.metadata[:authorize]) end diff --git a/lib/gitlab/group_search_results.rb b/lib/gitlab/group_search_results.rb index 7255293b19..334642f252 100644 --- a/lib/gitlab/group_search_results.rb +++ b/lib/gitlab/group_search_results.rb @@ -2,6 +2,8 @@ module Gitlab class GroupSearchResults < SearchResults + attr_reader :group + def initialize(current_user, limit_projects, group, query, default_project_filter: false, per_page: 20) super(current_user, limit_projects, query, default_project_filter: default_project_filter, per_page: per_page) @@ -26,5 +28,9 @@ module Gitlab .where(id: groups.select('members.user_id')) end # rubocop:enable CodeReuse/ActiveRecord + + def issuable_params + super.merge(group_id: group.id) + end end end diff --git a/lib/gitlab/issuable_metadata.rb b/lib/gitlab/issuable_metadata.rb index 351d15605e..be73bcd550 100644 --- a/lib/gitlab/issuable_metadata.rb +++ b/lib/gitlab/issuable_metadata.rb @@ -2,7 +2,7 @@ module Gitlab module IssuableMetadata - def issuable_meta_data(issuable_collection, collection_type) + def issuable_meta_data(issuable_collection, collection_type, user = nil) # ActiveRecord uses Object#extend for null relations. if !(issuable_collection.singleton_class < ActiveRecord::NullRelation) && issuable_collection.respond_to?(:limit_value) && @@ -23,7 +23,7 @@ module Gitlab issuable_votes_count = ::AwardEmoji.votes_for_collection(issuable_ids, collection_type) issuable_merge_requests_count = if collection_type == 'Issue' - ::MergeRequestsClosingIssues.count_for_collection(issuable_ids) + ::MergeRequestsClosingIssues.count_for_collection(issuable_ids, user) else [] end diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index 9dc162b7b4..0f3b97e231 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -151,5 +151,9 @@ module Gitlab def repository_wiki_ref @repository_wiki_ref ||= repository_ref || project.wiki.default_branch end + + def issuable_params + super.merge(project_id: project.id) + end end end diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb index e52c828687..7c1e6b1baf 100644 --- a/lib/gitlab/search_results.rb +++ b/lib/gitlab/search_results.rb @@ -2,6 +2,8 @@ module Gitlab class SearchResults + COUNT_LIMIT = 1001 + attr_reader :current_user, :query, :per_page # Limit search results by passed projects @@ -25,29 +27,26 @@ module Gitlab def objects(scope, page = nil, without_count = true) collection = case scope when 'projects' - projects.page(page).per(per_page) + projects when 'issues' - issues.page(page).per(per_page) + issues when 'merge_requests' - merge_requests.page(page).per(per_page) + merge_requests when 'milestones' - milestones.page(page).per(per_page) + milestones when 'users' - users.page(page).per(per_page) + users else - Kaminari.paginate_array([]).page(page).per(per_page) - end + Kaminari.paginate_array([]) + end.page(page).per(per_page) without_count ? collection.without_count : collection end - # rubocop: disable CodeReuse/ActiveRecord def limited_projects_count - @limited_projects_count ||= projects.limit(count_limit).count + @limited_projects_count ||= limited_count(projects) end - # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord def limited_issues_count return @limited_issues_count if @limited_issues_count @@ -56,35 +55,28 @@ module Gitlab # and confidential issues user has access to, is too complex. # It's faster to try to fetch all public issues first, then only # if necessary try to fetch all issues. - sum = issues(public_only: true).limit(count_limit).count - @limited_issues_count = sum < count_limit ? issues.limit(count_limit).count : sum + sum = limited_count(issues(public_only: true)) + @limited_issues_count = sum < count_limit ? limited_count(issues) : sum end - # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord def limited_merge_requests_count - @limited_merge_requests_count ||= merge_requests.limit(count_limit).count + @limited_merge_requests_count ||= limited_count(merge_requests) end - # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord def limited_milestones_count - @limited_milestones_count ||= milestones.limit(count_limit).count + @limited_milestones_count ||= limited_count(milestones) end - # rubocop: enable CodeReuse/ActiveRecord - # rubocop:disable CodeReuse/ActiveRecord def limited_users_count - @limited_users_count ||= users.limit(count_limit).count + @limited_users_count ||= limited_count(users) end - # rubocop:enable CodeReuse/ActiveRecord def single_commit_result? false end def count_limit - 1001 + COUNT_LIMIT end def users @@ -99,23 +91,15 @@ module Gitlab limit_projects.search(query) end - # rubocop: disable CodeReuse/ActiveRecord def issues(finder_params = {}) - issues = IssuesFinder.new(current_user, finder_params).execute + issues = IssuesFinder.new(current_user, issuable_params.merge(finder_params)).execute + unless default_project_filter - issues = issues.where(project_id: project_ids_relation) + issues = issues.where(project_id: project_ids_relation) # rubocop: disable CodeReuse/ActiveRecord end - issues = - if query =~ /#(\d+)\z/ - issues.where(iid: $1) - else - issues.full_search(query) - end - - issues.reorder('updated_at DESC') + issues end - # rubocop: enable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord def milestones @@ -127,23 +111,15 @@ module Gitlab end # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord def merge_requests - merge_requests = MergeRequestsFinder.new(current_user).execute + merge_requests = MergeRequestsFinder.new(current_user, issuable_params).execute + unless default_project_filter merge_requests = merge_requests.in_projects(project_ids_relation) end - merge_requests = - if query =~ /[#!](\d+)\z/ - merge_requests.where(iid: $1) - else - merge_requests.full_search(query) - end - - merge_requests.reorder('updated_at DESC') + merge_requests end - # rubocop: enable CodeReuse/ActiveRecord def default_scope 'projects' @@ -174,5 +150,23 @@ module Gitlab limit_projects.select(:id).reorder(nil) end # rubocop: enable CodeReuse/ActiveRecord + + def issuable_params + {}.tap do |params| + params[:sort] = 'updated_desc' + + if query =~ /#(\d+)\z/ + params[:iids] = $1 + else + params[:search] = query + end + end + end + + # rubocop: disable CodeReuse/ActiveRecord + def limited_count(relation) + relation.reorder(nil).limit(count_limit).size + end + # rubocop: enable CodeReuse/ActiveRecord end end diff --git a/lib/gitlab/utils/deep_size.rb b/lib/gitlab/utils/deep_size.rb new file mode 100644 index 0000000000..562cf09e24 --- /dev/null +++ b/lib/gitlab/utils/deep_size.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'objspace' + +module Gitlab + module Utils + class DeepSize + Error = Class.new(StandardError) + TooMuchDataError = Class.new(Error) + + DEFAULT_MAX_SIZE = 1.megabyte + DEFAULT_MAX_DEPTH = 100 + + def initialize(root, max_size: DEFAULT_MAX_SIZE, max_depth: DEFAULT_MAX_DEPTH) + @root = root + @max_size = max_size + @max_depth = max_depth + @size = 0 + @depth = 0 + + evaluate + end + + def valid? + !too_big? && !too_deep? + end + + private + + def evaluate + add_object(@root) + rescue Error + # NOOP + end + + def too_big? + @size > @max_size + end + + def too_deep? + @depth > @max_depth + end + + def add_object(object) + @size += ObjectSpace.memsize_of(object) + raise TooMuchDataError if @size > @max_size + + add_array(object) if object.is_a?(Array) + add_hash(object) if object.is_a?(Hash) + end + + def add_array(object) + with_nesting do + object.each do |n| + add_object(n) + end + end + end + + def add_hash(object) + with_nesting do + object.each do |key, value| + add_object(key) + add_object(value) + end + end + end + + def with_nesting + @depth += 1 + raise TooMuchDataError if too_deep? + + yield + + @depth -= 1 + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index bbe8b1bde3..45607c8956 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10281,7 +10281,7 @@ msgstr[1] "" msgid "score" msgstr "" -msgid "should be higher than %{access} inherited membership from group %{group_name}" +msgid "should be greater than or equal to %{access} inherited membership from group %{group_name}" msgstr "" msgid "show less" diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index deecb7fefe..a335c08e51 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -250,7 +250,7 @@ describe Projects::NotesController do before do service_params = ActionController::Parameters.new({ note: 'some note', - noteable_id: merge_request.id.to_s, + noteable_id: merge_request.id, noteable_type: 'MergeRequest', commit_id: nil, merge_request_diff_head_sha: 'sha' diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb index 01e5366962..a7352b252e 100644 --- a/spec/controllers/projects/templates_controller_spec.rb +++ b/spec/controllers/projects/templates_controller_spec.rb @@ -1,49 +1,101 @@ require 'spec_helper' describe Projects::TemplatesController do - let(:project) { create(:project, :repository) } + let(:project) { create(:project, :repository, :private) } let(:user) { create(:user) } - let(:user2) { create(:user) } - let(:file_path_1) { '.gitlab/issue_templates/bug.md' } + let(:file_path_1) { '.gitlab/issue_templates/issue_template.md' } + let(:file_path_2) { '.gitlab/merge_request_templates/merge_request_template.md' } let(:body) { JSON.parse(response.body) } - - before do - project.add_developer(user) - sign_in(user) - end - - before do - project.add_user(user, Gitlab::Access::MAINTAINER) - project.repository.create_file(user, file_path_1, 'something valid', - message: 'test 3', branch_name: 'master') - end + let!(:file_1) { project.repository.create_file(user, file_path_1, 'issue content', message: 'message', branch_name: 'master') } + let!(:file_2) { project.repository.create_file(user, file_path_2, 'merge request content', message: 'message', branch_name: 'master') } describe '#show' do - it 'renders template name and content as json' do - get(:show, params: { namespace_id: project.namespace.to_param, template_type: "issue", key: "bug", project_id: project }, format: :json) + shared_examples 'renders issue templates as json' do + it do + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json) - expect(response.status).to eq(200) - expect(body["name"]).to eq("bug") - expect(body["content"]).to eq("something valid") + expect(response.status).to eq(200) + expect(body['name']).to eq('issue_template') + expect(body['content']).to eq('issue content') + end end - it 'renders 404 when unauthorized' do - sign_in(user2) - get(:show, params: { namespace_id: project.namespace.to_param, template_type: "issue", key: "bug", project_id: project }, format: :json) + shared_examples 'renders merge request templates as json' do + it do + get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json) - expect(response.status).to eq(404) + expect(response.status).to eq(200) + expect(body['name']).to eq('merge_request_template') + expect(body['content']).to eq('merge request content') + end end - it 'renders 404 when template type is not found' do - sign_in(user) - get(:show, params: { namespace_id: project.namespace.to_param, template_type: "dont_exist", key: "bug", project_id: project }, format: :json) + shared_examples 'renders 404 when requesting an issue template' do + it do + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json) - expect(response.status).to eq(404) + expect(response.status).to eq(404) + end end - it 'renders 404 without errors' do - sign_in(user) - expect { get(:show, params: { namespace_id: project.namespace.to_param, template_type: "dont_exist", key: "bug", project_id: project }, format: :json) }.not_to raise_error + shared_examples 'renders 404 when requesting a merge request template' do + it do + get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json) + + expect(response.status).to eq(404) + end + end + + shared_examples 'renders 404 when params are invalid' do + it 'does not route when the template type is invalid' do + expect do + get(:show, params: { namespace_id: project.namespace, template_type: 'invalid_type', key: 'issue_template', project_id: project }, format: :json) + end.to raise_error(ActionController::UrlGenerationError) + end + + it 'renders 404 when the format type is invalid' do + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :html) + + expect(response.status).to eq(404) + end + + it 'renders 404 when the key is unknown' do + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'unknown_template', project_id: project }, format: :json) + + expect(response.status).to eq(404) + end + end + + context 'when the user is not a member of the project' do + before do + sign_in(user) + end + + include_examples 'renders 404 when requesting an issue template' + include_examples 'renders 404 when requesting a merge request template' + include_examples 'renders 404 when params are invalid' + end + + context 'when user is a member of the project' do + before do + project.add_developer(user) + sign_in(user) + end + + include_examples 'renders issue templates as json' + include_examples 'renders merge request templates as json' + include_examples 'renders 404 when params are invalid' + end + + context 'when user is a guest of the project' do + before do + project.add_guest(user) + sign_in(user) + end + + include_examples 'renders issue templates as json' + include_examples 'renders 404 when requesting a merge request template' + include_examples 'renders 404 when params are invalid' end end end diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb index 6efbd1f6c7..0e2c93003b 100644 --- a/spec/controllers/snippets/notes_controller_spec.rb +++ b/spec/controllers/snippets/notes_controller_spec.rb @@ -117,6 +117,119 @@ describe Snippets::NotesController do end end + describe 'POST create' do + context 'when a snippet is public' do + let(:request_params) do + { + note: attributes_for(:note_on_personal_snippet, noteable: public_snippet), + snippet_id: public_snippet.id + } + end + + before do + sign_in user + end + + it 'returns status 302' do + post :create, params: request_params + + expect(response).to have_gitlab_http_status(302) + end + + it 'creates the note' do + expect { post :create, params: request_params }.to change { Note.count }.by(1) + end + end + + context 'when a snippet is internal' do + let(:request_params) do + { + note: attributes_for(:note_on_personal_snippet, noteable: internal_snippet), + snippet_id: internal_snippet.id + } + end + + before do + sign_in user + end + + it 'returns status 302' do + post :create, params: request_params + + expect(response).to have_gitlab_http_status(302) + end + + it 'creates the note' do + expect { post :create, params: request_params }.to change { Note.count }.by(1) + end + end + + context 'when a snippet is private' do + let(:request_params) do + { + note: attributes_for(:note_on_personal_snippet, noteable: private_snippet), + snippet_id: private_snippet.id + } + end + + before do + sign_in user + end + + context 'when user is not the author' do + before do + sign_in(user) + end + + it 'returns status 404' do + post :create, params: request_params + + expect(response).to have_gitlab_http_status(404) + end + + it 'does not create the note' do + expect { post :create, params: request_params }.not_to change { Note.count } + end + + context 'when user sends a snippet_id for a public snippet' do + let(:request_params) do + { + note: attributes_for(:note_on_personal_snippet, noteable: private_snippet), + snippet_id: public_snippet.id + } + end + + it 'returns status 302' do + post :create, params: request_params + + expect(response).to have_gitlab_http_status(302) + end + + it 'creates the note on the public snippet' do + expect { post :create, params: request_params }.to change { Note.count }.by(1) + expect(Note.last.noteable).to eq public_snippet + end + end + end + + context 'when user is the author' do + before do + sign_in(private_snippet.author) + end + + it 'returns status 302' do + post :create, params: request_params + + expect(response).to have_gitlab_http_status(302) + end + + it 'creates the note' do + expect { post :create, params: request_params }.to change { Note.count }.by(1) + end + end + end + end + describe 'DELETE destroy' do let(:request_params) do { diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 77a94f26d8..da2810df71 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -207,8 +207,8 @@ describe SnippetsController do context 'when the snippet description contains a file' do include FileMoverHelpers - let(:picture_file) { '/-/system/temp/secret56/picture.jpg' } - let(:text_file) { '/-/system/temp/secret78/text.txt' } + let(:picture_file) { "/-/system/user/#{user.id}/secret56/picture.jpg" } + let(:text_file) { "/-/system/user/#{user.id}/secret78/text.txt" } let(:description) do "Description with picture: ![picture](/uploads#{picture_file}) and "\ "text: [text.txt](/uploads#{text_file})" diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index e52a5fe42f..dd8d901871 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -22,121 +22,160 @@ describe UploadsController do let!(:user) { create(:user, avatar: fixture_file_upload("spec/fixtures/dk.png", "image/png")) } describe 'POST create' do - let(:model) { 'personal_snippet' } - let(:snippet) { create(:personal_snippet, :public) } let(:jpg) { fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg') } let(:txt) { fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain') } - context 'when a user does not have permissions to upload a file' do - it "returns 401 when the user is not logged in" do - post :create, params: { model: model, id: snippet.id }, format: :json + context 'snippet uploads' do + let(:model) { 'personal_snippet' } + let(:snippet) { create(:personal_snippet, :public) } + + context 'when a user does not have permissions to upload a file' do + it "returns 401 when the user is not logged in" do + post :create, params: { model: model, id: snippet.id }, format: :json + + expect(response).to have_gitlab_http_status(401) + end + + it "returns 404 when user can't comment on a snippet" do + private_snippet = create(:personal_snippet, :private) + + sign_in(user) + post :create, params: { model: model, id: private_snippet.id }, format: :json + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when a user is logged in' do + before do + sign_in(user) + end + + it "returns an error without file" do + post :create, params: { model: model, id: snippet.id }, format: :json + + expect(response).to have_gitlab_http_status(422) + end + + it "returns an error with invalid model" do + expect { post :create, params: { model: 'invalid', id: snippet.id }, format: :json } + .to raise_error(ActionController::UrlGenerationError) + end + + it "returns 404 status when object not found" do + post :create, params: { model: model, id: 9999 }, format: :json + + expect(response).to have_gitlab_http_status(404) + end + + context 'with valid image' do + before do + post :create, params: { model: 'personal_snippet', id: snippet.id, file: jpg }, format: :json + end + + it 'returns a content with original filename, new link, and correct type.' do + expect(response.body).to match '\"alt\":\"rails_sample\"' + expect(response.body).to match "\"url\":\"/uploads" + end + + it 'creates a corresponding Upload record' do + upload = Upload.last + + aggregate_failures do + expect(upload).to exist + expect(upload.model).to eq snippet + end + end + end + + context 'with valid non-image file' do + before do + post :create, params: { model: 'personal_snippet', id: snippet.id, file: txt }, format: :json + end + + it 'returns a content with original filename, new link, and correct type.' do + expect(response.body).to match '\"alt\":\"doc_sample.txt\"' + expect(response.body).to match "\"url\":\"/uploads" + end + + it 'creates a corresponding Upload record' do + upload = Upload.last + + aggregate_failures do + expect(upload).to exist + expect(upload.model).to eq snippet + end + end + end + end + end + + context 'user uploads' do + let(:model) { 'user' } + + it 'returns 401 when the user has no access' do + post :create, params: { model: 'user', id: user.id }, format: :json expect(response).to have_gitlab_http_status(401) end - it "returns 404 when user can't comment on a snippet" do - private_snippet = create(:personal_snippet, :private) - - sign_in(user) - post :create, params: { model: model, id: private_snippet.id }, format: :json - - expect(response).to have_gitlab_http_status(404) - end - end - - context 'when a user is logged in' do - before do - sign_in(user) - end - - it "returns an error without file" do - post :create, params: { model: model, id: snippet.id }, format: :json - - expect(response).to have_gitlab_http_status(422) - end - - it "returns an error with invalid model" do - expect { post :create, params: { model: 'invalid', id: snippet.id }, format: :json } - .to raise_error(ActionController::UrlGenerationError) - end - - it "returns 404 status when object not found" do - post :create, params: { model: model, id: 9999 }, format: :json - - expect(response).to have_gitlab_http_status(404) - end - - context 'with valid image' do + context 'when user is logged in' do before do - post :create, params: { model: 'personal_snippet', id: snippet.id, file: jpg }, format: :json + sign_in(user) end - it 'returns a content with original filename, new link, and correct type.' do - expect(response.body).to match '\"alt\":\"rails_sample\"' - expect(response.body).to match "\"url\":\"/uploads" - end - - it 'creates a corresponding Upload record' do - upload = Upload.last - - aggregate_failures do - expect(upload).to exist - expect(upload.model).to eq snippet - end - end - end - - context 'with valid non-image file' do - before do - post :create, params: { model: 'personal_snippet', id: snippet.id, file: txt }, format: :json - end - - it 'returns a content with original filename, new link, and correct type.' do - expect(response.body).to match '\"alt\":\"doc_sample.txt\"' - expect(response.body).to match "\"url\":\"/uploads" - end - - it 'creates a corresponding Upload record' do - upload = Upload.last - - aggregate_failures do - expect(upload).to exist - expect(upload.model).to eq snippet - end - end - end - - context 'temporal with valid image' do subject do - post :create, params: { model: 'personal_snippet', file: jpg }, format: :json + post :create, params: { model: model, id: user.id, file: jpg }, format: :json end it 'returns a content with original filename, new link, and correct type.' do subject expect(response.body).to match '\"alt\":\"rails_sample\"' - expect(response.body).to match "\"url\":\"/uploads/-/system/temp" + expect(response.body).to match "\"url\":\"/uploads/-/system/user/#{user.id}/" end - it 'does not create an Upload record' do - expect { subject }.not_to change { Upload.count } - end - end + it 'creates a corresponding Upload record' do + expect { subject }.to change { Upload.count } - context 'temporal with valid non-image file' do - subject do - post :create, params: { model: 'personal_snippet', file: txt }, format: :json + upload = Upload.last + + aggregate_failures do + expect(upload).to exist + expect(upload.model).to eq user + end end - it 'returns a content with original filename, new link, and correct type.' do - subject + context 'with valid non-image file' do + subject do + post :create, params: { model: model, id: user.id, file: txt }, format: :json + end - expect(response.body).to match '\"alt\":\"doc_sample.txt\"' - expect(response.body).to match "\"url\":\"/uploads/-/system/temp" + it 'returns a content with original filename, new link, and correct type.' do + subject + + expect(response.body).to match '\"alt\":\"doc_sample.txt\"' + expect(response.body).to match "\"url\":\"/uploads/-/system/user/#{user.id}/" + end + + it 'creates a corresponding Upload record' do + expect { subject }.to change { Upload.count } + + upload = Upload.last + + aggregate_failures do + expect(upload).to exist + expect(upload.model).to eq user + end + end end - it 'does not create an Upload record' do - expect { subject }.not_to change { Upload.count } + it 'returns 404 when given user is not the logged in one' do + another_user = create(:user) + + post :create, params: { model: model, id: another_user.id, file: txt }, format: :json + + expect(response).to have_gitlab_http_status(404) end end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index ab185ab397..743ec32288 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -260,6 +260,7 @@ FactoryBot.define do trait(:merge_requests_enabled) { merge_requests_access_level ProjectFeature::ENABLED } trait(:merge_requests_disabled) { merge_requests_access_level ProjectFeature::DISABLED } trait(:merge_requests_private) { merge_requests_access_level ProjectFeature::PRIVATE } + trait(:merge_requests_public) { merge_requests_access_level ProjectFeature::PUBLIC } trait(:repository_enabled) { repository_access_level ProjectFeature::ENABLED } trait(:repository_disabled) { repository_access_level ProjectFeature::DISABLED } trait(:repository_private) { repository_access_level ProjectFeature::PRIVATE } diff --git a/spec/features/snippets/user_creates_snippet_spec.rb b/spec/features/snippets/user_creates_snippet_spec.rb index 1c97d5ec5b..93d77d5b5c 100644 --- a/spec/features/snippets/user_creates_snippet_spec.rb +++ b/spec/features/snippets/user_creates_snippet_spec.rb @@ -41,7 +41,7 @@ describe 'User creates snippet', :js do expect(page).to have_content('My Snippet') link = find('a.no-attachment-icon img[alt="banana_sample"]')['src'] - expect(link).to match(%r{/uploads/-/system/temp/\h{32}/banana_sample\.gif\z}) + expect(link).to match(%r{/uploads/-/system/user/#{user.id}/\h{32}/banana_sample\.gif\z}) reqs = inspect_requests { visit(link) } expect(reqs.first.status_code).to eq(200) diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 4133987a07..f66395c48b 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -669,9 +669,7 @@ describe IssuesFinder do end it 'filters by confidentiality' do - expect(Issue).to receive(:where).with(a_string_matching('confidential'), anything) - - subject + expect(subject.to_sql).to match("issues.confidential") end end @@ -688,9 +686,7 @@ describe IssuesFinder do end it 'filters by confidentiality' do - expect(Issue).to receive(:where).with(a_string_matching('confidential'), anything) - - subject + expect(subject.to_sql).to match("issues.confidential") end end diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 9d4b9af3ec..a2417f8df1 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -31,7 +31,7 @@ describe MergeRequestsFinder do end context 'filtering by group' do - it 'includes all merge requests when user has access exceluding merge requests from projects the user does not have access to' do + it 'includes all merge requests when user has access excluding merge requests from projects the user does not have access to' do private_project = allow_gitaly_n_plus_1 { create(:project, :private, group: group) } private_project.add_guest(user) create(:merge_request, :simple, author: user, source_project: private_project, target_project: private_project) diff --git a/spec/graphql/types/label_type_spec.rb b/spec/graphql/types/label_type_spec.rb new file mode 100644 index 0000000000..da2b0b578d --- /dev/null +++ b/spec/graphql/types/label_type_spec.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe GitlabSchema.types['Label'] do + it { is_expected.to require_graphql_authorizations(:read_label) } +end diff --git a/spec/graphql/types/metadata_type_spec.rb b/spec/graphql/types/metadata_type_spec.rb index 55205bf5b6..5236380e47 100644 --- a/spec/graphql/types/metadata_type_spec.rb +++ b/spec/graphql/types/metadata_type_spec.rb @@ -2,4 +2,5 @@ require 'spec_helper' describe GitlabSchema.types['Metadata'] do it { expect(described_class.graphql_name).to eq('Metadata') } + it { is_expected.to require_graphql_authorizations(:read_instance_metadata) } end diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb index 69e3ea8a4a..89d215fe06 100644 --- a/spec/graphql/types/query_type_spec.rb +++ b/spec/graphql/types/query_type_spec.rb @@ -24,9 +24,5 @@ describe GitlabSchema.types['Query'] do is_expected.to have_graphql_type(Types::MetadataType) is_expected.to have_graphql_resolver(Resolvers::MetadataResolver) end - - it 'authorizes with read_instance_metadata' do - is_expected.to require_graphql_authorizations(:read_instance_metadata) - end end end diff --git a/spec/lib/api/helpers/related_resources_helpers_spec.rb b/spec/lib/api/helpers/related_resources_helpers_spec.rb index 66af7f8153..99fe8795d9 100644 --- a/spec/lib/api/helpers/related_resources_helpers_spec.rb +++ b/spec/lib/api/helpers/related_resources_helpers_spec.rb @@ -5,6 +5,40 @@ describe API::Helpers::RelatedResourcesHelpers do Class.new.include(described_class).new end + describe '#expose_path' do + let(:path) { '/api/v4/awesome_endpoint' } + + context 'empty relative URL root' do + before do + stub_config_setting(relative_url_root: '') + end + + it 'returns the existing path' do + expect(helpers.expose_path(path)).to eq(path) + end + end + + context 'slash relative URL root' do + before do + stub_config_setting(relative_url_root: '/') + end + + it 'returns the existing path' do + expect(helpers.expose_path(path)).to eq(path) + end + end + + context 'with relative URL root' do + before do + stub_config_setting(relative_url_root: '/gitlab/root') + end + + it 'returns the existing path' do + expect(helpers.expose_path(path)).to eq("/gitlab/root" + path) + end + end + end + describe '#expose_url' do let(:path) { '/api/v4/awesome_endpoint' } subject(:url) { helpers.expose_url(path) } diff --git a/spec/lib/banzai/filter/relative_link_filter_spec.rb b/spec/lib/banzai/filter/relative_link_filter_spec.rb index dad0a5535c..a6e8a2fc8f 100644 --- a/spec/lib/banzai/filter/relative_link_filter_spec.rb +++ b/spec/lib/banzai/filter/relative_link_filter_spec.rb @@ -83,6 +83,11 @@ describe Banzai::Filter::RelativeLinkFilter do expect { filter(act) }.not_to raise_error end + it 'does not explode with an escaped null byte' do + act = link("/%00") + expect { filter(act) }.not_to raise_error + end + it 'does not raise an exception with a space in the path' do act = link("/uploads/d18213acd3732630991986120e167e3d/Landscape_8.jpg \nBut here's some more unexpected text :smile:)") expect { filter(act) }.not_to raise_error diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 64396a746e..d7c657383d 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -90,6 +90,27 @@ describe Gitlab::Ci::Config do end end + context 'when yml is too big' do + let(:yml) do + <<~YAML + --- &1 + - hi + - *1 + YAML + end + + describe '.new' do + it 'raises error' do + expect(Gitlab::Sentry).to receive(:track_exception) + + expect { config }.to raise_error( + described_class::ConfigError, + /The parsed YAML is too big/ + ) + end + end + end + context 'when config logic is incorrect' do let(:yml) { 'before_script: "ls"' } diff --git a/spec/lib/gitlab/config/loader/yaml_spec.rb b/spec/lib/gitlab/config/loader/yaml_spec.rb index 44c9a3896a..0affeb01f6 100644 --- a/spec/lib/gitlab/config/loader/yaml_spec.rb +++ b/spec/lib/gitlab/config/loader/yaml_spec.rb @@ -8,7 +8,7 @@ describe Gitlab::Config::Loader::Yaml do describe '#valid?' do it 'returns true' do - expect(loader.valid?).to be true + expect(loader).to be_valid end end @@ -24,7 +24,7 @@ describe Gitlab::Config::Loader::Yaml do describe '#valid?' do it 'returns false' do - expect(loader.valid?).to be false + expect(loader).not_to be_valid end end @@ -43,7 +43,10 @@ describe Gitlab::Config::Loader::Yaml do describe '#initialize' do it 'raises FormatError' do - expect { loader }.to raise_error(Gitlab::Config::Loader::FormatError, 'Unknown alias: bad_alias') + expect { loader }.to raise_error( + Gitlab::Config::Loader::FormatError, + 'Unknown alias: bad_alias' + ) end end end @@ -53,7 +56,68 @@ describe Gitlab::Config::Loader::Yaml do describe '#valid?' do it 'returns false' do - expect(loader.valid?).to be false + expect(loader).not_to be_valid + end + end + end + + # Prevent Billion Laughs attack: https://gitlab.com/gitlab-org/gitlab-ce/issues/56018 + context 'when yaml size is too large' do + let(:yml) do + <<~YAML + a: &a ["lol","lol","lol","lol","lol","lol","lol","lol","lol"] + b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a] + c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b] + d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c] + e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d] + f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e] + g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f] + h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g] + i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h] + YAML + end + + describe '#valid?' do + it 'returns false' do + expect(loader).not_to be_valid + end + + it 'returns true if "ci_yaml_limit_size" feature flag is disabled' do + stub_feature_flags(ci_yaml_limit_size: false) + + expect(loader).to be_valid + end + end + + describe '#load!' do + it 'raises FormatError' do + expect { loader.load! }.to raise_error( + Gitlab::Config::Loader::FormatError, + 'The parsed YAML is too big' + ) + end + end + end + + # Prevent Billion Laughs attack: https://gitlab.com/gitlab-org/gitlab-ce/issues/56018 + context 'when yaml has cyclic data structure' do + let(:yml) do + <<~YAML + --- &1 + - hi + - *1 + YAML + end + + describe '#valid?' do + it 'returns false' do + expect(loader.valid?).to be(false) + end + end + + describe '#load!' do + it 'raises FormatError' do + expect { loader.load! }.to raise_error(Gitlab::Config::Loader::FormatError, 'The parsed YAML is too big') end end end diff --git a/spec/lib/gitlab/data_builder/pipeline_spec.rb b/spec/lib/gitlab/data_builder/pipeline_spec.rb index 9ef987a082..1f36fd5c6e 100644 --- a/spec/lib/gitlab/data_builder/pipeline_spec.rb +++ b/spec/lib/gitlab/data_builder/pipeline_spec.rb @@ -50,5 +50,14 @@ describe Gitlab::DataBuilder::Pipeline do it { expect(attributes[:variables]).to be_a(Array) } it { expect(attributes[:variables]).to contain_exactly({ key: 'TRIGGER_KEY_1', value: 'TRIGGER_VALUE_1' }) } end + + context 'when pipeline is a detached merge request pipeline' do + let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) } + let(:pipeline) { merge_request.all_pipelines.first } + + it 'returns a source ref' do + expect(attributes[:ref]).to eq(merge_request.source_branch) + end + end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 088f8acf55..34ba819b12 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -215,6 +215,18 @@ describe Gitlab::Git::Repository, :seed_helper do it { is_expected.to be < 2 } end + describe '#object_directory_size' do + before do + allow(repository.gitaly_repository_client) + .to receive(:get_object_directory_size) + .and_return(2) + end + + subject { repository.object_directory_size } + + it { is_expected.to eq 2048 } + end + describe '#empty?' do it { expect(repository).not_to be_empty } end diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index 7ad3cde97f..7e169cfe27 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -19,7 +19,9 @@ describe Gitlab::Git::Tree, :seed_helper do it 'returns a list of tree objects' do entries = described_class.where(repository, SeedRepo::Commit::ID, 'files', true) - expect(entries.count).to be >= 5 + expect(entries.map(&:path)).to include('files/html', + 'files/markdown/ruby-style-guide.md') + expect(entries.count).to be >= 10 expect(entries).to all(be_a(Gitlab::Git::Tree)) end diff --git a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb index 46ca234038..544f572920 100644 --- a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb @@ -73,6 +73,17 @@ describe Gitlab::GitalyClient::RepositoryService do end end + describe '#get_object_directory_size' do + it 'sends a get_object_directory_size message' do + expect_any_instance_of(Gitaly::RepositoryService::Stub) + .to receive(:get_object_directory_size) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return(size: 0) + + client.get_object_directory_size + end + end + describe '#apply_gitattributes' do let(:revision) { 'master' } diff --git a/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb index 95a4eb296f..91be2b5dcb 100644 --- a/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb +++ b/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb @@ -7,35 +7,39 @@ require 'spec_helper' describe Gitlab::Graphql::Authorize::AuthorizeFieldService do def type(type_authorizations = []) Class.new(Types::BaseObject) do - graphql_name "TestType" + graphql_name 'TestType' authorize type_authorizations end end - def type_with_field(field_type, field_authorizations = [], resolved_value = "Resolved value") + def type_with_field(field_type, field_authorizations = [], resolved_value = 'Resolved value', **options) Class.new(Types::BaseObject) do - graphql_name "TestTypeWithField" - field :test_field, field_type, null: true, authorize: field_authorizations, resolve: -> (_, _, _) { resolved_value} + graphql_name 'TestTypeWithField' + options.reverse_merge!(null: true) + field :test_field, field_type, + authorize: field_authorizations, + resolve: -> (_, _, _) { resolved_value }, + **options end end let(:current_user) { double(:current_user) } subject(:service) { described_class.new(field) } - describe "#authorized_resolve" do - let(:presented_object) { double("presented object") } - let(:presented_type) { double("parent type", object: presented_object) } + describe '#authorized_resolve' do + let(:presented_object) { double('presented object') } + let(:presented_type) { double('parent type', object: presented_object) } subject(:resolved) { service.authorized_resolve.call(presented_type, {}, { current_user: current_user }) } - context "scalar types" do - shared_examples "checking permissions on the presented object" do - it "checks the abilities on the object being presented and returns the value" do + context 'scalar types' do + shared_examples 'checking permissions on the presented object' do + it 'checks the abilities on the object being presented and returns the value' do expected_permissions.each do |permission| spy_ability_check_for(permission, presented_object, passed: true) end - expect(resolved).to eq("Resolved value") + expect(resolved).to eq('Resolved value') end it "returns nil if the value wasn't authorized" do @@ -45,47 +49,57 @@ describe Gitlab::Graphql::Authorize::AuthorizeFieldService do end end - context "when the field is a scalar type" do - let(:field) { type_with_field(GraphQL::STRING_TYPE, :read_field).fields["testField"].to_graphql } + context 'when the field is a built-in scalar type' do + let(:field) { type_with_field(GraphQL::STRING_TYPE, :read_field).fields['testField'].to_graphql } let(:expected_permissions) { [:read_field] } - it_behaves_like "checking permissions on the presented object" + it_behaves_like 'checking permissions on the presented object' end - context "when the field is a list of scalar types" do - let(:field) { type_with_field([GraphQL::STRING_TYPE], :read_field).fields["testField"].to_graphql } + context 'when the field is a list of scalar types' do + let(:field) { type_with_field([GraphQL::STRING_TYPE], :read_field).fields['testField'].to_graphql } let(:expected_permissions) { [:read_field] } - it_behaves_like "checking permissions on the presented object" + it_behaves_like 'checking permissions on the presented object' end end - context "when the field is a specific type" do + context 'when the field is a specific type' do let(:custom_type) { type(:read_type) } - let(:object_in_field) { double("presented in field") } - let(:field) { type_with_field(custom_type, :read_field, object_in_field).fields["testField"].to_graphql } + let(:object_in_field) { double('presented in field') } + let(:field) { type_with_field(custom_type, :read_field, object_in_field).fields['testField'].to_graphql } - it "checks both field & type permissions" do + it 'checks both field & type permissions' do spy_ability_check_for(:read_field, object_in_field, passed: true) spy_ability_check_for(:read_type, object_in_field, passed: true) expect(resolved).to eq(object_in_field) end - it "returns nil if viewing was not allowed" do + it 'returns nil if viewing was not allowed' do spy_ability_check_for(:read_field, object_in_field, passed: false) spy_ability_check_for(:read_type, object_in_field, passed: true) expect(resolved).to be_nil end - context "when the field is a list" do - let(:object_1) { double("presented in field 1") } - let(:object_2) { double("presented in field 2") } - let(:presented_types) { [double(object: object_1), double(object: object_2)] } - let(:field) { type_with_field([custom_type], :read_field, presented_types).fields["testField"].to_graphql } + context 'when the field is not nullable' do + let(:field) { type_with_field(custom_type, [], object_in_field, null: false).fields['testField'].to_graphql } - it "checks all permissions" do + it 'returns nil when viewing is not allowed' do + spy_ability_check_for(:read_type, object_in_field, passed: false) + + expect(resolved).to be_nil + end + end + + context 'when the field is a list' do + let(:object_1) { double('presented in field 1') } + let(:object_2) { double('presented in field 2') } + let(:presented_types) { [double(object: object_1), double(object: object_2)] } + let(:field) { type_with_field([custom_type], :read_field, presented_types).fields['testField'].to_graphql } + + it 'checks all permissions' do allow(Ability).to receive(:allowed?) { true } spy_ability_check_for(:read_field, object_1, passed: true) @@ -96,7 +110,7 @@ describe Gitlab::Graphql::Authorize::AuthorizeFieldService do expect(resolved).to eq(presented_types) end - it "filters out objects that the user cannot see" do + it 'filters out objects that the user cannot see' do allow(Ability).to receive(:allowed?) { true } spy_ability_check_for(:read_type, object_1, passed: false) diff --git a/spec/lib/gitlab/issuable_metadata_spec.rb b/spec/lib/gitlab/issuable_metadata_spec.rb index 6ec8616323..a0a6353bb5 100644 --- a/spec/lib/gitlab/issuable_metadata_spec.rb +++ b/spec/lib/gitlab/issuable_metadata_spec.rb @@ -7,11 +7,11 @@ describe Gitlab::IssuableMetadata do subject { Class.new { include Gitlab::IssuableMetadata }.new } it 'returns an empty Hash if an empty collection is provided' do - expect(subject.issuable_meta_data(Issue.none, 'Issue')).to eq({}) + expect(subject.issuable_meta_data(Issue.none, 'Issue', user)).to eq({}) end it 'raises an error when given a collection with no limit' do - expect { subject.issuable_meta_data(Issue.all, 'Issue') }.to raise_error(/must have a limit/) + expect { subject.issuable_meta_data(Issue.all, 'Issue', user) }.to raise_error(/must have a limit/) end context 'issues' do @@ -23,7 +23,7 @@ describe Gitlab::IssuableMetadata do let!(:closing_issues) { create(:merge_requests_closing_issues, issue: issue, merge_request: merge_request) } it 'aggregates stats on issues' do - data = subject.issuable_meta_data(Issue.all.limit(10), 'Issue') + data = subject.issuable_meta_data(Issue.all.limit(10), 'Issue', user) expect(data.count).to eq(2) expect(data[issue.id].upvotes).to eq(1) @@ -46,7 +46,7 @@ describe Gitlab::IssuableMetadata do let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") } it 'aggregates stats on merge requests' do - data = subject.issuable_meta_data(MergeRequest.all.limit(10), 'MergeRequest') + data = subject.issuable_meta_data(MergeRequest.all.limit(10), 'MergeRequest', user) expect(data.count).to eq(2) expect(data[merge_request.id].upvotes).to eq(1) diff --git a/spec/lib/gitlab/utils/deep_size_spec.rb b/spec/lib/gitlab/utils/deep_size_spec.rb new file mode 100644 index 0000000000..1e619a1598 --- /dev/null +++ b/spec/lib/gitlab/utils/deep_size_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' + +describe Gitlab::Utils::DeepSize do + let(:data) do + { + a: [1, 2, 3], + b: { + c: [4, 5], + d: [ + { e: [[6], [7]] } + ] + } + } + end + + let(:max_size) { 1.kilobyte } + let(:max_depth) { 10 } + let(:deep_size) { described_class.new(data, max_size: max_size, max_depth: max_depth) } + + describe '#evaluate' do + context 'when data within size and depth limits' do + it 'returns true' do + expect(deep_size).to be_valid + end + end + + context 'when data not within size limit' do + let(:max_size) { 200.bytes } + + it 'returns false' do + expect(deep_size).not_to be_valid + end + end + + context 'when data not within depth limit' do + let(:max_depth) { 2 } + + it 'returns false' do + expect(deep_size).not_to be_valid + end + end + end +end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index c68c3ce2ab..782a84f922 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -70,6 +70,16 @@ describe Member do expect(child_member).not_to be_valid end + # Membership in a subgroup confers certain access rights, such as being + # able to merge or push code to protected branches. + it "is valid with an equal level" do + child_member.access_level = GroupMember::DEVELOPER + + child_member.validate + + expect(child_member).to be_valid + end + it "is valid with a higher level" do child_member.access_level = GroupMember::MAINTAINER diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 07de913516..7013616314 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3188,61 +3188,105 @@ describe Project do end describe '.with_feature_available_for_user' do - let!(:user) { create(:user) } - let!(:feature) { MergeRequest } - let!(:project) { create(:project, :public, :merge_requests_enabled) } + let(:user) { create(:user) } + let(:feature) { MergeRequest } subject { described_class.with_feature_available_for_user(feature, user) } - context 'when user has access to project' do - subject { described_class.with_feature_available_for_user(feature, user) } + shared_examples 'feature disabled' do + let(:project) { create(:project, :public, :merge_requests_disabled) } + it 'does not return projects with the project feature disabled' do + is_expected.not_to include(project) + end + end + + shared_examples 'feature public' do + let(:project) { create(:project, :public, :merge_requests_public) } + + it 'returns projects with the project feature public' do + is_expected.to include(project) + end + end + + shared_examples 'feature enabled' do + let(:project) { create(:project, :public, :merge_requests_enabled) } + + it 'returns projects with the project feature enabled' do + is_expected.to include(project) + end + end + + shared_examples 'feature access level is nil' do + let(:project) { create(:project, :public) } + + it 'returns projects with the project feature access level nil' do + project.project_feature.update(merge_requests_access_level: nil) + + is_expected.to include(project) + end + end + + context 'with user' do before do project.add_guest(user) end - context 'when public project' do - context 'when feature is public' do - it 'returns project' do - is_expected.to include(project) - end - end + it_behaves_like 'feature disabled' + it_behaves_like 'feature public' + it_behaves_like 'feature enabled' + it_behaves_like 'feature access level is nil' - context 'when feature is private' do - let!(:project) { create(:project, :public, :merge_requests_private) } + context 'when feature is private' do + let(:project) { create(:project, :public, :merge_requests_private) } - it 'returns project when user has access to the feature' do - project.add_maintainer(user) - - is_expected.to include(project) - end - - it 'does not return project when user does not have the minimum access level required' do + context 'when user does not has access to the feature' do + it 'does not return projects with the project feature private' do is_expected.not_to include(project) end end - end - context 'when private project' do - let!(:project) { create(:project) } + context 'when user has access to the feature' do + it 'returns projects with the project feature private' do + project.add_reporter(user) - it 'returns project when user has access to the feature' do - project.add_maintainer(user) - - is_expected.to include(project) - end - - it 'does not return project when user does not have the minimum access level required' do - is_expected.not_to include(project) + is_expected.to include(project) + end end end end - context 'when user does not have access to project' do - let!(:project) { create(:project) } + context 'user is an admin' do + let(:user) { create(:user, :admin) } - it 'does not return project when user cant access project' do - is_expected.not_to include(project) + it_behaves_like 'feature disabled' + it_behaves_like 'feature public' + it_behaves_like 'feature enabled' + it_behaves_like 'feature access level is nil' + + context 'when feature is private' do + let(:project) { create(:project, :public, :merge_requests_private) } + + it 'returns projects with the project feature private' do + is_expected.to include(project) + end + end + end + + context 'without user' do + let(:user) { nil } + + it_behaves_like 'feature disabled' + it_behaves_like 'feature public' + it_behaves_like 'feature enabled' + it_behaves_like 'feature access level is nil' + + context 'when feature is private' do + let(:project) { create(:project, :public, :merge_requests_private) } + + it 'does not return projects with the project feature private' do + is_expected.not_to include(project) + end end end end diff --git a/spec/presenters/ci/build_runner_presenter_spec.rb b/spec/presenters/ci/build_runner_presenter_spec.rb index ad6cb012d0..3430111ca9 100644 --- a/spec/presenters/ci/build_runner_presenter_spec.rb +++ b/spec/presenters/ci/build_runner_presenter_spec.rb @@ -136,24 +136,6 @@ describe Ci::BuildRunnerPresenter do is_expected.to eq(1) end end - - context 'when pipeline is detached merge request pipeline' do - let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) } - let(:pipeline) { merge_request.all_pipelines.first } - let(:build) { create(:ci_build, ref: pipeline.ref, pipeline: pipeline) } - - it 'returns the default git depth for pipelines for merge requests' do - is_expected.to eq(described_class::DEFAULT_GIT_DEPTH_MERGE_REQUEST) - end - - context 'when pipeline is legacy detached merge request pipeline' do - let(:merge_request) { create(:merge_request, :with_legacy_detached_merge_request_pipeline) } - - it 'behaves as branch pipeline' do - is_expected.to eq(0) - end - end - end end describe '#refspecs' do @@ -191,7 +173,9 @@ describe Ci::BuildRunnerPresenter do it 'returns the correct refspecs' do is_expected - .to contain_exactly('+refs/merge-requests/1/head:refs/merge-requests/1/head') + .to contain_exactly('+refs/heads/*:refs/remotes/origin/*', + '+refs/tags/*:refs/tags/*', + '+refs/merge-requests/1/head:refs/merge-requests/1/head') end context 'when pipeline is legacy detached merge request pipeline' do diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 79edbb301f..48869cab4d 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -236,7 +236,7 @@ describe API::Members do params: { user_id: stranger.id, access_level: Member::REPORTER } expect(response).to have_gitlab_http_status(400) - expect(json_response['message']['access_level']).to eq(["should be higher than Developer inherited membership from group #{parent.name}"]) + expect(json_response['message']['access_level']).to eq(["should be greater than or equal to Developer inherited membership from group #{parent.name}"]) end it 'creates the member if group level is lower', :nested_groups do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index e148f6033a..7c3b223fbb 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -830,6 +830,31 @@ describe API::MergeRequests do end end + context 'head_pipeline' do + before do + merge_request.update(head_pipeline: create(:ci_pipeline)) + merge_request.project.project_feature.update(builds_access_level: 10) + end + + context 'when user can read the pipeline' do + it 'exposes pipeline information' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) + + expect(json_response).to include('head_pipeline') + end + end + + context 'when user can not read the pipeline' do + let(:guest) { create(:user) } + + it 'does not expose pipeline information' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", guest) + + expect(json_response).not_to include('head_pipeline') + end + end + end + it 'returns the commits behind the target branch when include_diverged_commits_count is present' do allow_any_instance_of(merge_request.class).to receive(:diverged_commits_count).and_return(1) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 352ea448c0..1b6261aad2 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -504,8 +504,9 @@ describe API::Projects do project4.add_reporter(user2) end - it 'returns an array of groups the user has at least developer access' do + it 'returns an array of projects the user has at least developer access' do get api('/projects', user2), params: { min_access_level: 30 } + expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index a0d01fc826..e4c21bc307 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -661,4 +661,24 @@ describe 'project routing' do end end end + + describe Projects::TemplatesController, 'routing' do + describe '#show' do + def show_with_template_type(template_type) + "/gitlab/gitlabhq/templates/#{template_type}/template_name" + end + + it 'routes when :template_type is `merge_request`' do + expect(get(show_with_template_type('merge_request'))).to route_to('projects/templates#show', namespace_id: 'gitlab', project_id: 'gitlabhq', template_type: 'merge_request', key: 'template_name', format: 'json') + end + + it 'routes when :template_type is `issue`' do + expect(get(show_with_template_type('issue'))).to route_to('projects/templates#show', namespace_id: 'gitlab', project_id: 'gitlabhq', template_type: 'issue', key: 'template_name', format: 'json') + end + + it 'routes to application#route_not_found when :template_type is unknown' do + expect(get(show_with_template_type('invalid'))).to route_to('application#route_not_found', unmatched_route: 'gitlab/gitlabhq/templates/invalid/template_name') + end + end + end end diff --git a/spec/routing/uploads_routing_spec.rb b/spec/routing/uploads_routing_spec.rb new file mode 100644 index 0000000000..42e8477408 --- /dev/null +++ b/spec/routing/uploads_routing_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Uploads', 'routing' do + it 'allows creating uploads for personal snippets' do + expect(post('/uploads/personal_snippet?id=1')).to route_to( + controller: 'uploads', + action: 'create', + model: 'personal_snippet', + id: '1' + ) + end + + it 'allows creating uploads for users' do + expect(post('/uploads/user?id=1')).to route_to( + controller: 'uploads', + action: 'create', + model: 'user', + id: '1' + ) + end + + it 'does not allow creating uploads for other models' do + unroutable_models = UploadsController::MODEL_CLASSES.keys.compact - %w(personal_snippet user) + + unroutable_models.each do |model| + expect(post("/uploads/#{model}?id=1")).not_to be_routable + end + end +end diff --git a/spec/services/lfs/file_transformer_spec.rb b/spec/services/lfs/file_transformer_spec.rb index e8938338cb..9b1a64ee05 100644 --- a/spec/services/lfs/file_transformer_spec.rb +++ b/spec/services/lfs/file_transformer_spec.rb @@ -62,6 +62,25 @@ describe Lfs::FileTransformer do expect(result.encoding).to eq('text') end + context 'when an actual file is passed' do + let(:file) { Tempfile.new(file_path) } + + before do + file.write(file_content) + file.rewind + end + + after do + file.unlink + end + + it "creates an LfsObject with the file's content" do + subject.new_file(file_path, file) + + expect(LfsObject.last.file.read).to eq file_content + end + end + context "when doesn't use LFS" do let(:file_path) { 'other.filetype' } diff --git a/spec/services/projects/after_import_service_spec.rb b/spec/services/projects/after_import_service_spec.rb index 765b4ffae8..4dd6c6dab8 100644 --- a/spec/services/projects/after_import_service_spec.rb +++ b/spec/services/projects/after_import_service_spec.rb @@ -13,7 +13,7 @@ describe Projects::AfterImportService do describe '#execute' do before do allow(Projects::HousekeepingService) - .to receive(:new).with(project, :gc).and_return(housekeeping_service) + .to receive(:new).with(project).and_return(housekeeping_service) allow(housekeeping_service) .to receive(:execute).and_yield diff --git a/spec/services/projects/propagate_service_template_spec.rb b/spec/services/projects/propagate_service_template_spec.rb index f4c59735c4..e015374f3a 100644 --- a/spec/services/projects/propagate_service_template_spec.rb +++ b/spec/services/projects/propagate_service_template_spec.rb @@ -70,7 +70,7 @@ describe Projects::PropagateServiceTemplate do expect(project.pushover_service.properties).to eq(service_template.properties) end - describe 'bulk update' do + describe 'bulk update', :use_sql_query_cache do let(:project_total) { 5 } before do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 60db3e1bc4..74501ed880 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -213,6 +213,12 @@ RSpec.configure do |config| ActionController::Base.cache_store = caching_store end + config.around(:each, :use_sql_query_cache) do |example| + ActiveRecord::Base.cache do + example.run + end + end + # The :each scope runs "inside" the example, so this hook ensures the DB is in the # correct state before any examples' before hooks are called. This prevents a # problem where `ScheduleIssuesClosedAtTypeChange` (or any migration that depends diff --git a/spec/support/shared_examples/models/member_shared_examples.rb b/spec/support/shared_examples/models/member_shared_examples.rb index 7737649685..e5375bc828 100644 --- a/spec/support/shared_examples/models/member_shared_examples.rb +++ b/spec/support/shared_examples/models/member_shared_examples.rb @@ -41,7 +41,7 @@ shared_examples_for 'inherited access level as a member of entity' do member.update(access_level: Gitlab::Access::REPORTER) - expect(member.errors.full_messages).to eq(["Access level should be higher than Developer inherited membership from group #{parent_entity.name}"]) + expect(member.errors.full_messages).to eq(["Access level should be greater than or equal to Developer inherited membership from group #{parent_entity.name}"]) end it 'allows changing the level from a non existing member' do diff --git a/spec/support/shared_examples/requests/api/issues/merge_requests_count_shared_examples.rb b/spec/support/shared_examples/requests/api/issues/merge_requests_count_shared_examples.rb new file mode 100644 index 0000000000..5f4e178f2e --- /dev/null +++ b/spec/support/shared_examples/requests/api/issues/merge_requests_count_shared_examples.rb @@ -0,0 +1,37 @@ +def get_issue + json_response.is_a?(Array) ? json_response.detect {|issue| issue['id'] == target_issue.id} : json_response +end + +shared_examples 'accessible merge requests count' do + it 'returns anonymous accessible merge requests count' do + get api(api_url), params: { scope: 'all' } + + issue = get_issue + expect(issue).not_to be_nil + expect(issue['merge_requests_count']).to eq(1) + end + + it 'returns guest accessible merge requests count' do + get api(api_url, guest), params: { scope: 'all' } + + issue = get_issue + expect(issue).not_to be_nil + expect(issue['merge_requests_count']).to eq(1) + end + + it 'returns reporter accessible merge requests count' do + get api(api_url, user), params: { scope: 'all' } + + issue = get_issue + expect(issue).not_to be_nil + expect(issue['merge_requests_count']).to eq(2) + end + + it 'returns admin accessible merge requests count' do + get api(api_url, admin), params: { scope: 'all' } + + issue = get_issue + expect(issue).not_to be_nil + expect(issue['merge_requests_count']).to eq(2) + end +end diff --git a/spec/uploaders/file_mover_spec.rb b/spec/uploaders/file_mover_spec.rb index e474a714b1..322c37b605 100644 --- a/spec/uploaders/file_mover_spec.rb +++ b/spec/uploaders/file_mover_spec.rb @@ -3,79 +3,140 @@ require 'spec_helper' describe FileMover do include FileMoverHelpers + let(:user) { create(:user) } let(:filename) { 'banana_sample.gif' } - let(:temp_file_path) { File.join('uploads/-/system/temp', 'secret55', filename) } + let(:secret) { 'secret55' } + let(:temp_file_path) { File.join("uploads/-/system/user/#{user.id}", secret, filename) } let(:temp_description) do "test ![banana_sample](/#{temp_file_path}) "\ "same ![banana_sample](/#{temp_file_path}) " end - let(:file_path) { File.join('uploads/-/system/personal_snippet', snippet.id.to_s, 'secret55', filename) } + let(:file_path) { File.join('uploads/-/system/personal_snippet', snippet.id.to_s, secret, filename) } let(:snippet) { create(:personal_snippet, description: temp_description) } - subject { described_class.new(temp_file_path, snippet).execute } + let(:tmp_uploader) do + PersonalFileUploader.new(user, secret: secret) + end + + let(:file) { fixture_file_upload('spec/fixtures/banana_sample.gif') } + subject { described_class.new(temp_file_path, from_model: user, to_model: snippet).execute } describe '#execute' do + let(:tmp_upload) { tmp_uploader.upload } + before do - expect(FileUtils).to receive(:mkdir_p).with(a_string_including(File.dirname(file_path))) - expect(FileUtils).to receive(:move).with(a_string_including(temp_file_path), a_string_including(file_path)) - allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:exists?).and_return(true) - allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:size).and_return(10) - - stub_file_mover(temp_file_path) + tmp_uploader.store!(file) end - context 'when move and field update successful' do - it 'updates the description correctly' do - subject - - expect(snippet.reload.description) - .to eq( - "test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\ - "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) " - ) - end - - it 'creates a new update record' do - expect { subject }.to change { Upload.count }.by(1) - end - - it 'schedules a background migration' do - expect_any_instance_of(PersonalFileUploader).to receive(:schedule_background_upload).once - - subject - end - end - - context 'when update_markdown fails' do + context 'local storage' do before do - expect(FileUtils).to receive(:move).with(a_string_including(file_path), a_string_including(temp_file_path)) + allow(FileUtils).to receive(:mkdir_p).with(a_string_including(File.dirname(file_path))) + allow(FileUtils).to receive(:move).with(a_string_including(temp_file_path), a_string_including(file_path)) + allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:exists?).and_return(true) + allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:size).and_return(10) + + stub_file_mover(temp_file_path) end - subject { described_class.new(file_path, snippet, :non_existing_field).execute } + context 'when move and field update successful' do + it 'updates the description correctly' do + subject - it 'does not update the description' do - subject + expect(snippet.reload.description) + .to eq("test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\ + "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) ") + end - expect(snippet.reload.description) - .to eq( - "test ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif) "\ - "same ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif) " - ) + it 'updates existing upload record' do + expect { subject } + .to change { tmp_upload.reload.attributes.values_at('model_id', 'model_type') } + .from([user.id, 'User']).to([snippet.id, 'Snippet']) + end + + it 'schedules a background migration' do + expect_any_instance_of(PersonalFileUploader).to receive(:schedule_background_upload).once + + subject + end end - it 'does not create a new update record' do - expect { subject }.not_to change { Upload.count } + context 'when update_markdown fails' do + before do + expect(FileUtils).to receive(:move).with(a_string_including(file_path), a_string_including(temp_file_path)) + end + + subject { described_class.new(file_path, :non_existing_field, from_model: user, to_model: snippet).execute } + + it 'does not update the description' do + subject + + expect(snippet.reload.description) + .to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) "\ + "same ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) ") + end + + it 'does not change the upload record' do + expect { subject } + .not_to change { tmp_upload.reload.attributes.values_at('model_id', 'model_type') } + end + end + end + + context 'when tmp uploader is not local storage' do + before do + allow(PersonalFileUploader).to receive(:object_store_enabled?) { true } + tmp_uploader.object_store = ObjectStorage::Store::REMOTE + allow_any_instance_of(PersonalFileUploader).to receive(:file_storage?) { false } + end + + after do + FileUtils.rm_f(File.join('personal_snippet', snippet.id.to_s, secret, filename)) + end + + context 'when move and field update successful' do + it 'updates the description correctly' do + subject + + expect(snippet.reload.description) + .to eq("test ![banana_sample](/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\ + "same ![banana_sample](/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) ") + end + + it 'creates new target upload record an delete the old upload' do + expect { subject } + .to change { Upload.last.attributes.values_at('model_id', 'model_type') } + .from([user.id, 'User']).to([snippet.id, 'Snippet']) + + expect(Upload.count).to eq(1) + end + end + + context 'when update_markdown fails' do + subject { described_class.new(file_path, :non_existing_field, from_model: user, to_model: snippet).execute } + + it 'does not update the description' do + subject + + expect(snippet.reload.description) + .to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) "\ + "same ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) ") + end + + it 'does not change the upload record' do + expect { subject } + .to change { Upload.last.attributes.values_at('model_id', 'model_type') }.from([user.id, 'User']) + end end end end context 'security' do context 'when relative path is involved' do - let(:temp_file_path) { File.join('uploads/-/system/temp', '..', 'another_subdir_of_temp') } + let(:temp_file_path) { File.join("uploads/-/system/user/#{user.id}", '..', 'another_subdir_of_temp') } it 'does not trigger move if path is outside designated directory' do - stub_file_mover('uploads/-/system/another_subdir_of_temp') + stub_file_mover("uploads/-/system/user/#{user.id}/another_subdir_of_temp") expect(FileUtils).not_to receive(:move) subject diff --git a/spec/validators/color_validator_spec.rb b/spec/validators/color_validator_spec.rb new file mode 100644 index 0000000000..e5a38ac937 --- /dev/null +++ b/spec/validators/color_validator_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ColorValidator do + using RSpec::Parameterized::TableSyntax + + subject do + Class.new do + include ActiveModel::Model + include ActiveModel::Validations + attr_accessor :color + validates :color, color: true + end.new + end + + where(:color, :is_valid) do + '#000abc' | true + '#aaa' | true + '#BBB' | true + '#cCc' | true + '#ffff' | false + '#000111222' | false + 'invalid' | false + '000' | false + end + + with_them do + it 'only accepts valid colors' do + subject.color = color + + expect(subject.valid?).to eq(is_valid) + end + end + + it 'fails fast for long invalid string' do + subject.color = '#' + ('0' * 50_000) + 'xxx' + + expect do + Timeout.timeout(5.seconds) { subject.valid? } + end.not_to raise_error + end +end