diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-06-29 14:14:01 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-06-29 14:14:01 +0000 |
commit | a5baa12bfff6c41f6c9cf156edcf8e621f71848e (patch) | |
tree | 1a7f51da1300bca04a1bd070f12e66bc4955c832 | |
parent | bb51b8a098aa17b226d1e7941218512f8c835e08 (diff) | |
download | gitlab-ce-a5baa12bfff6c41f6c9cf156edcf8e621f71848e.tar.gz |
Add latest changes from gitlab-org/security/gitlab@15-1-stable-ee
41 files changed, 471 insertions, 108 deletions
diff --git a/app/assets/javascripts/error_tracking_settings/utils.js b/app/assets/javascripts/error_tracking_settings/utils.js index 7ef5f7bbd34..47a42dc3742 100644 --- a/app/assets/javascripts/error_tracking_settings/utils.js +++ b/app/assets/javascripts/error_tracking_settings/utils.js @@ -1,4 +1,4 @@ -export const projectKeys = ['name', 'organizationName', 'organizationSlug', 'slug']; +export const projectKeys = ['id', 'name', 'organizationName', 'organizationSlug', 'slug']; export const transformFrontendSettings = ({ apiHost, @@ -9,6 +9,7 @@ export const transformFrontendSettings = ({ }) => { const project = selectedProject ? { + sentry_project_id: selectedProject.id, slug: selectedProject.slug, name: selectedProject.name, organization_name: selectedProject.organizationName, diff --git a/app/assets/javascripts/projects/settings/access_dropdown.js b/app/assets/javascripts/projects/settings/access_dropdown.js index 7fb7a416dca..79dfa166b1a 100644 --- a/app/assets/javascripts/projects/settings/access_dropdown.js +++ b/app/assets/javascripts/projects/settings/access_dropdown.js @@ -537,7 +537,7 @@ export default class AccessDropdown { return ` <li> <a href="#" class="${isActiveClass}"> - <strong>${key.title}</strong> + <strong>${escape(key.title)}</strong> <p> ${sprintf( __('Owned by %{image_tag}'), diff --git a/app/controllers/projects/error_tracking_controller.rb b/app/controllers/projects/error_tracking_controller.rb index 06383d26133..d2e36ef5496 100644 --- a/app/controllers/projects/error_tracking_controller.rb +++ b/app/controllers/projects/error_tracking_controller.rb @@ -4,6 +4,7 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle respond_to :json before_action :authorize_read_sentry_issue! + before_action :authorize_update_sentry_issue!, only: %i[update] before_action :set_issue_id, only: :details before_action only: [:index] do diff --git a/app/controllers/projects/settings/operations_controller.rb b/app/controllers/projects/settings/operations_controller.rb index d4126cbd708..77d7f3570f3 100644 --- a/app/controllers/projects/settings/operations_controller.rb +++ b/app/controllers/projects/settings/operations_controller.rb @@ -144,7 +144,7 @@ module Projects :integrated, :api_host, :token, - project: [:slug, :name, :organization_slug, :organization_name] + project: [:slug, :name, :organization_slug, :organization_name, :sentry_project_id] ], grafana_integration_attributes: [:token, :grafana_url, :enabled], diff --git a/app/helpers/integrations_helper.rb b/app/helpers/integrations_helper.rb index 82d4ceee44e..8d5523464c7 100644 --- a/app/helpers/integrations_helper.rb +++ b/app/helpers/integrations_helper.rb @@ -160,27 +160,6 @@ module IntegrationsHelper !Gitlab.com? end - def jira_issue_breadcrumb_link(issue_reference) - link_to '', { class: 'gl-display-flex gl-align-items-center gl-white-space-nowrap' } do - icon = image_tag image_path('illustrations/logos/jira.svg'), width: 15, height: 15, class: 'gl-mr-2' - [icon, html_escape(issue_reference)].join.html_safe - end - end - - def zentao_issue_breadcrumb_link(issue) - link_to issue[:web_url], { target: '_blank', rel: 'noopener noreferrer', class: 'gl-display-flex gl-align-items-center gl-white-space-nowrap' } do - icon = image_tag image_path('logos/zentao.svg'), width: 15, height: 15, class: 'gl-mr-2' - [icon, html_escape(issue[:id])].join.html_safe - end - end - - def zentao_issues_show_data - { - issues_show_path: project_integrations_zentao_issue_path(@project, params[:id], format: :json), - issues_list_path: project_integrations_zentao_issues_path(@project) - } - end - extend self private diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 6112d05f37d..95e91a7ba27 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -300,6 +300,7 @@ module ProjectsHelper setting.organization_slug.blank? { + sentry_project_id: setting.sentry_project_id, name: setting.project_name, organization_name: setting.organization_name, organization_slug: setting.organization_slug, diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb index bed0eab5a58..1ac4cbac1da 100644 --- a/app/models/clusters/applications/runner.rb +++ b/app/models/clusters/applications/runner.rb @@ -3,7 +3,7 @@ module Clusters module Applications class Runner < ApplicationRecord - VERSION = '0.41.0' + VERSION = '0.42.1' self.table_name = 'clusters_applications_runners' diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb index 3ecfb895dac..30382a1c205 100644 --- a/app/models/error_tracking/project_error_tracking_setting.rb +++ b/app/models/error_tracking/project_error_tracking_setting.rb @@ -125,17 +125,22 @@ module ErrorTracking def issue_details(opts = {}) with_reactive_cache('issue_details', opts.stringify_keys) do |result| + ensure_issue_belongs_to_project!(result[:issue].project_id) result end end def issue_latest_event(opts = {}) with_reactive_cache('issue_latest_event', opts.stringify_keys) do |result| + ensure_issue_belongs_to_project!(result[:latest_event].project_id) result end end def update_issue(opts = {}) + issue_to_be_updated = sentry_client.issue_details(issue_id: opts[:issue_id]) + ensure_issue_belongs_to_project!(issue_to_be_updated.project_id) + handle_exceptions do { updated: sentry_client.update_issue(opts) } end @@ -177,6 +182,25 @@ module ErrorTracking private + def ensure_issue_belongs_to_project!(project_id_from_api) + raise 'The Sentry issue appers to be outside of the configured Sentry project' if Integer(project_id_from_api) != ensure_sentry_project_id! + end + + def ensure_sentry_project_id! + return sentry_project_id if sentry_project_id.present? + + raise("Couldn't find project: #{organization_name} / #{project_name} on Sentry") if sentry_project.nil? + + update!(sentry_project_id: sentry_project.id) + sentry_project_id + end + + def sentry_project + strong_memoize(:sentry_project) do + sentry_client.projects.find { |project| project.name == project_name && project.organization_name == organization_name } + end + end + def add_gitlab_issue_details(issue) issue.gitlab_commit = match_gitlab_commit(issue.first_release_version) issue.gitlab_commit_path = project_commit_path(project, issue.gitlab_commit) if issue.gitlab_commit diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 3bce26be756..6ddd83544bc 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -308,7 +308,6 @@ class ProjectPolicy < BasePolicy enable :read_deployment enable :read_merge_request enable :read_sentry_issue - enable :update_sentry_issue enable :read_prometheus enable :read_metrics_dashboard_annotation enable :metrics_dashboard @@ -423,6 +422,7 @@ class ProjectPolicy < BasePolicy enable :admin_feature_flags_user_lists enable :update_escalation_status enable :read_secure_files + enable :update_sentry_issue end rule { can?(:developer_access) & user_confirmed? }.policy do diff --git a/app/serializers/member_user_entity.rb b/app/serializers/member_user_entity.rb index b3d8efc9143..6a01c5bb297 100644 --- a/app/serializers/member_user_entity.rb +++ b/app/serializers/member_user_entity.rb @@ -16,7 +16,7 @@ class MemberUserEntity < UserEntity user.blocked? end - expose :two_factor_enabled do |user| + expose :two_factor_enabled, if: -> (user) { current_user_can_manage_members? || current_user?(user) } do |user| user.two_factor_enabled? end @@ -25,6 +25,18 @@ class MemberUserEntity < UserEntity user.status.emoji end end + + private + + def current_user_can_manage_members? + return false unless options[:source] + + Ability.allowed?(options[:current_user], :"admin_#{options[:source].to_ability_name}_member", options[:source]) + end + + def current_user?(user) + options[:current_user] == user + end end MemberUserEntity.prepend_mod_with('MemberUserEntity') diff --git a/app/services/projects/operations/update_service.rb b/app/services/projects/operations/update_service.rb index d01e96a1a2d..7e4e0d7378e 100644 --- a/app/services/projects/operations/update_service.rb +++ b/app/services/projects/operations/update_service.rb @@ -90,7 +90,8 @@ module Projects api_url: api_url, enabled: settings[:enabled], project_name: settings.dig(:project, :name), - organization_name: settings.dig(:project, :organization_name) + organization_name: settings.dig(:project, :organization_name), + sentry_project_id: settings.dig(:project, :sentry_project_id) } } params[:error_tracking_setting_attributes][:token] = settings[:token] unless /\A\*+\z/.match?(settings[:token]) # Don't update token if we receive masked value diff --git a/db/migrate/20220525084153_add_sentry_project_id_to_project_error_tracking_settings.rb b/db/migrate/20220525084153_add_sentry_project_id_to_project_error_tracking_settings.rb new file mode 100644 index 00000000000..248dd128bec --- /dev/null +++ b/db/migrate/20220525084153_add_sentry_project_id_to_project_error_tracking_settings.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddSentryProjectIdToProjectErrorTrackingSettings < Gitlab::Database::Migration[2.0] + def change + add_column :project_error_tracking_settings, :sentry_project_id, :bigint + end +end diff --git a/db/schema_migrations/20220525084153 b/db/schema_migrations/20220525084153 new file mode 100644 index 00000000000..dbf7eaa0c93 --- /dev/null +++ b/db/schema_migrations/20220525084153 @@ -0,0 +1 @@ +1f03beba0775e2a4eead512819592f590b02b70096cee250dfcdf426440cb5f5
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c58ff5d47ba..e92e77f0f60 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -19301,7 +19301,8 @@ CREATE TABLE project_error_tracking_settings ( encrypted_token_iv character varying, project_name character varying, organization_name character varying, - integrated boolean DEFAULT true NOT NULL + integrated boolean DEFAULT true NOT NULL, + sentry_project_id bigint ); CREATE TABLE project_export_jobs ( diff --git a/doc/operations/error_tracking.md b/doc/operations/error_tracking.md index 2a007eade99..08acf77b6c7 100644 --- a/doc/operations/error_tracking.md +++ b/doc/operations/error_tracking.md @@ -106,7 +106,7 @@ button and a link to the GitLab issue displays within the error detail section. ## Taking Action on errors -You can take action on Sentry Errors from within the GitLab UI. +You can take action on Sentry Errors from within the GitLab UI. Marking errors ignored or resolved require at least Developer role. ### Ignoring errors diff --git a/lib/api/helpers/label_helpers.rb b/lib/api/helpers/label_helpers.rb index 02613cbf9b9..8572cc89e71 100644 --- a/lib/api/helpers/label_helpers.rb +++ b/lib/api/helpers/label_helpers.rb @@ -82,8 +82,14 @@ module API params.delete(:label_id) params.delete(:name) - label = ::Labels::UpdateService.new(declared_params(include_missing: false)).execute(label) - render_validation_error!(label) unless label.valid? + update_params = declared_params(include_missing: false) + + if update_params.present? + authorize! :admin_label, label + + label = ::Labels::UpdateService.new(update_params).execute(label) + render_validation_error!(label) unless label.valid? + end if parent.is_a?(Project) && update_priority if priority.nil? @@ -97,10 +103,10 @@ module API end def delete_label(parent) - authorize! :admin_label, parent - label = find_label(parent, params_id_or_title, include_ancestor_groups: false) + authorize! :admin_label, label + destroy_conditionally!(label) end diff --git a/lib/bulk_imports/projects/graphql/get_project_query.rb b/lib/bulk_imports/projects/graphql/get_project_query.rb index b3d7f3f4683..76475893ac1 100644 --- a/lib/bulk_imports/projects/graphql/get_project_query.rb +++ b/lib/bulk_imports/projects/graphql/get_project_query.rb @@ -10,20 +10,8 @@ module BulkImports <<-'GRAPHQL' query($full_path: ID!) { project(fullPath: $full_path) { - description visibility - archived created_at: createdAt - shared_runners_enabled: sharedRunnersEnabled - container_registry_enabled: containerRegistryEnabled - only_allow_merge_if_pipeline_succeeds: onlyAllowMergeIfPipelineSucceeds - only_allow_merge_if_all_discussions_are_resolved: onlyAllowMergeIfAllDiscussionsAreResolved - request_access_enabled: requestAccessEnabled - printing_merge_request_link_enabled: printingMergeRequestLinkEnabled - remove_source_branch_after_merge: removeSourceBranchAfterMerge - autoclose_referenced_issues: autocloseReferencedIssues - suggestion_commit_message: suggestionCommitMessage - wiki_enabled: wikiEnabled } } GRAPHQL diff --git a/lib/bulk_imports/projects/transformers/project_attributes_transformer.rb b/lib/bulk_imports/projects/transformers/project_attributes_transformer.rb index 24c55d8dbb1..38730a7723b 100644 --- a/lib/bulk_imports/projects/transformers/project_attributes_transformer.rb +++ b/lib/bulk_imports/projects/transformers/project_attributes_transformer.rb @@ -7,16 +7,18 @@ module BulkImports PROJECT_IMPORT_TYPE = 'gitlab_project_migration' def transform(context, data) + project = {} entity = context.entity visibility = data.delete('visibility') - data['name'] = entity.destination_name - data['path'] = entity.destination_name.parameterize - data['import_type'] = PROJECT_IMPORT_TYPE - data['visibility_level'] = Gitlab::VisibilityLevel.string_options[visibility] if visibility.present? - data['namespace_id'] = Namespace.find_by_full_path(entity.destination_namespace)&.id if entity.destination_namespace.present? + project[:name] = entity.destination_name + project[:path] = entity.destination_name.parameterize + project[:created_at] = data['created_at'] + project[:import_type] = PROJECT_IMPORT_TYPE + project[:visibility_level] = Gitlab::VisibilityLevel.string_options[visibility] if visibility.present? + project[:namespace_id] = Namespace.find_by_full_path(entity.destination_namespace)&.id if entity.destination_namespace.present? - data.transform_keys!(&:to_sym) + project end end end diff --git a/lib/error_tracking/sentry_client/event.rb b/lib/error_tracking/sentry_client/event.rb index 5343eb7df57..1db31abeeb2 100644 --- a/lib/error_tracking/sentry_client/event.rb +++ b/lib/error_tracking/sentry_client/event.rb @@ -15,6 +15,7 @@ module ErrorTracking stack_trace = parse_stack_trace(event) Gitlab::ErrorTracking::ErrorEvent.new( + project_id: event['projectID'], issue_id: event['groupID'], date_received: event['dateReceived'], stack_trace_entries: stack_trace diff --git a/lib/gitlab/error_tracking/error_event.rb b/lib/gitlab/error_tracking/error_event.rb index d80289f6bc9..590fb82883b 100644 --- a/lib/gitlab/error_tracking/error_event.rb +++ b/lib/gitlab/error_tracking/error_event.rb @@ -7,7 +7,7 @@ module Gitlab class ErrorEvent include ActiveModel::Model - attr_accessor :issue_id, :date_received, :stack_trace_entries, :gitlab_project + attr_accessor :issue_id, :date_received, :stack_trace_entries, :gitlab_project, :project_id def self.declarative_policy_class 'ErrorTracking::BasePolicy' diff --git a/lib/gitlab/import_export/decompressed_archive_size_validator.rb b/lib/gitlab/import_export/decompressed_archive_size_validator.rb index 61b37256964..a185eb4df1c 100644 --- a/lib/gitlab/import_export/decompressed_archive_size_validator.rb +++ b/lib/gitlab/import_export/decompressed_archive_size_validator.rb @@ -8,6 +8,8 @@ module Gitlab DEFAULT_MAX_BYTES = 10.gigabytes.freeze TIMEOUT_LIMIT = 210.seconds + ServiceError = Class.new(StandardError) + def initialize(archive_path:, max_bytes: self.class.max_bytes) @archive_path = archive_path @max_bytes = max_bytes @@ -29,6 +31,8 @@ module Gitlab pgrp = nil valid_archive = true + validate_archive_path + Timeout.timeout(TIMEOUT_LIMIT) do stdin, stdout, stderr, wait_thr = Open3.popen3(command, pgroup: true) stdin.close @@ -78,15 +82,29 @@ module Gitlab false end + def validate_archive_path + Gitlab::Utils.check_path_traversal!(@archive_path) + + raise(ServiceError, 'Archive path is not a string') unless @archive_path.is_a?(String) + raise(ServiceError, 'Archive path is a symlink') if File.lstat(@archive_path).symlink? + raise(ServiceError, 'Archive path is not a file') unless File.file?(@archive_path) + end + def command "gzip -dc #{@archive_path} | wc -c" end def log_error(error) + archive_size = begin + File.size(@archive_path) + rescue StandardError + nil + end + Gitlab::Import::Logger.info( message: error, import_upload_archive_path: @archive_path, - import_upload_archive_size: File.size(@archive_path) + import_upload_archive_size: archive_size ) end end diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb index cf0e481495c..475e7d3229c 100644 --- a/spec/controllers/projects/error_tracking_controller_spec.rb +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -307,6 +307,18 @@ RSpec.describe Projects::ErrorTrackingController do end describe 'format json' do + context 'when user is a reporter' do + before do + project.add_reporter(user) + end + + it 'returns 404 error' do + update_issue + + expect(response).to have_gitlab_http_status(:not_found) + end + end + context 'when update result is successful' do before do allow(issue_update_service).to receive(:execute) diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 2d8454988d9..8070e17b7af 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -70,6 +70,7 @@ RSpec.describe 'Database schema' do oauth_applications: %w[owner_id], product_analytics_events_experimental: %w[event_id txn_id user_id], project_build_artifacts_size_refreshes: %w[last_job_artifact_id], + project_error_tracking_settings: %w[sentry_project_id], project_group_links: %w[group_id], project_statistics: %w[namespace_id], projects: %w[creator_id ci_id mirror_user_id], diff --git a/spec/factories/error_tracking/error.rb b/spec/factories/error_tracking/error.rb index bebdffb3614..62d16244122 100644 --- a/spec/factories/error_tracking/error.rb +++ b/spec/factories/error_tracking/error.rb @@ -13,7 +13,7 @@ FactoryBot.define do message { 'message' } culprit { 'culprit' } external_url { 'http://example.com/id' } - project_id { 'project1' } + project_id { '111111' } project_name { 'project name' } project_slug { 'project_name' } short_id { 'ID' } diff --git a/spec/factories/project_error_tracking_settings.rb b/spec/factories/project_error_tracking_settings.rb index ed743d8283c..a8ad1af6345 100644 --- a/spec/factories/project_error_tracking_settings.rb +++ b/spec/factories/project_error_tracking_settings.rb @@ -9,6 +9,7 @@ FactoryBot.define do project_name { 'Sentry Project' } organization_name { 'Sentry Org' } integrated { false } + sentry_project_id { 10 } trait :disabled do enabled { false } diff --git a/spec/fixtures/api/schemas/entities/member.json b/spec/fixtures/api/schemas/entities/member.json index dec98123e85..88f7d87b269 100644 --- a/spec/fixtures/api/schemas/entities/member.json +++ b/spec/fixtures/api/schemas/entities/member.json @@ -53,7 +53,7 @@ }, "user": { "allOf": [ - { "$ref": "member_user.json" } + { "$ref": "member_user_default.json" } ] }, "state": { "type": "integer" }, diff --git a/spec/fixtures/api/schemas/entities/member_user_default.json b/spec/fixtures/api/schemas/entities/member_user_default.json new file mode 100644 index 00000000000..e0b3dba5699 --- /dev/null +++ b/spec/fixtures/api/schemas/entities/member_user_default.json @@ -0,0 +1,35 @@ +{ + "type": "object", + "required": [ + "id", + "name", + "username", + "created_at", + "last_activity_on", + "avatar_url", + "web_url", + "blocked", + "show_status" + ], + "properties": { + "id": { "type": "integer" }, + "name": { "type": "string" }, + "username": { "type": "string" }, + "created_at": { "type": ["string"] }, + "avatar_url": { "type": ["string", "null"] }, + "web_url": { "type": "string" }, + "blocked": { "type": "boolean" }, + "two_factor_enabled": { "type": "boolean" }, + "availability": { "type": ["string", "null"] }, + "last_activity_on": { "type": ["string", "null"] }, + "status": { + "type": "object", + "required": ["emoji"], + "properties": { + "emoji": { "type": "string" } + }, + "additionalProperties": false + }, + "show_status": { "type": "boolean" } + } +} diff --git a/spec/fixtures/api/schemas/entities/member_user.json b/spec/fixtures/api/schemas/entities/member_user_for_admin_member.json index 0750e81e115..0750e81e115 100644 --- a/spec/fixtures/api/schemas/entities/member_user.json +++ b/spec/fixtures/api/schemas/entities/member_user_for_admin_member.json diff --git a/spec/frontend/projects/settings/access_dropdown_spec.js b/spec/frontend/projects/settings/access_dropdown_spec.js index 65b01172e7e..d51360a7597 100644 --- a/spec/frontend/projects/settings/access_dropdown_spec.js +++ b/spec/frontend/projects/settings/access_dropdown_spec.js @@ -159,4 +159,21 @@ describe('AccessDropdown', () => { expect(template).not.toContain(user.name); }); }); + + describe('deployKeyRowHtml', () => { + const deployKey = { + id: 1, + title: 'title <script>alert(document.domain)</script>', + fullname: 'fullname <script>alert(document.domain)</script>', + avatar_url: '', + username: '', + }; + + it('escapes deploy key title and fullname', () => { + const template = dropdown.deployKeyRowHtml(deployKey); + + expect(template).not.toContain(deployKey.title); + expect(template).not.toContain(deployKey.fullname); + }); + }); }); diff --git a/spec/helpers/integrations_helper_spec.rb b/spec/helpers/integrations_helper_spec.rb index dccbc110be6..95dfc51e8fd 100644 --- a/spec/helpers/integrations_helper_spec.rb +++ b/spec/helpers/integrations_helper_spec.rb @@ -150,19 +150,4 @@ RSpec.describe IntegrationsHelper do end end end - - describe '#jira_issue_breadcrumb_link' do - let(:issue_reference) { nil } - - subject { helper.jira_issue_breadcrumb_link(issue_reference) } - - context 'when issue_reference contains HTML' do - let(:issue_reference) { "<script>alert('XSS')</script>" } - - it 'escapes issue reference' do - is_expected.not_to include(issue_reference) - is_expected.to include(html_escape(issue_reference)) - end - end - end end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index e0c98bbc161..4502729866c 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -52,6 +52,7 @@ RSpec.describe ProjectsHelper do context 'api_url present' do let(:json) do { + sentry_project_id: error_tracking_setting.sentry_project_id, name: error_tracking_setting.project_name, organization_name: error_tracking_setting.organization_name, organization_slug: error_tracking_setting.organization_slug, diff --git a/spec/lib/bulk_imports/projects/pipelines/project_pipeline_spec.rb b/spec/lib/bulk_imports/projects/pipelines/project_pipeline_spec.rb index c53c0849931..567a0a4fcc3 100644 --- a/spec/lib/bulk_imports/projects/pipelines/project_pipeline_spec.rb +++ b/spec/lib/bulk_imports/projects/pipelines/project_pipeline_spec.rb @@ -25,18 +25,7 @@ RSpec.describe BulkImports::Projects::Pipelines::ProjectPipeline do let(:project_data) do { 'visibility' => 'private', - 'created_at' => 10.days.ago, - 'archived' => false, - 'shared_runners_enabled' => true, - 'container_registry_enabled' => true, - 'only_allow_merge_if_pipeline_succeeds' => true, - 'only_allow_merge_if_all_discussions_are_resolved' => true, - 'request_access_enabled' => true, - 'printing_merge_request_link_enabled' => true, - 'remove_source_branch_after_merge' => true, - 'autoclose_referenced_issues' => true, - 'suggestion_commit_message' => 'message', - 'wiki_enabled' => true + 'created_at' => '2016-08-12T09:41:03' } end @@ -58,17 +47,8 @@ RSpec.describe BulkImports::Projects::Pipelines::ProjectPipeline do expect(imported_project).not_to be_nil expect(imported_project.group).to eq(group) - expect(imported_project.suggestion_commit_message).to eq('message') - expect(imported_project.archived?).to eq(project_data['archived']) - expect(imported_project.shared_runners_enabled?).to eq(project_data['shared_runners_enabled']) - expect(imported_project.container_registry_enabled?).to eq(project_data['container_registry_enabled']) - expect(imported_project.only_allow_merge_if_pipeline_succeeds?).to eq(project_data['only_allow_merge_if_pipeline_succeeds']) - expect(imported_project.only_allow_merge_if_all_discussions_are_resolved?).to eq(project_data['only_allow_merge_if_all_discussions_are_resolved']) - expect(imported_project.request_access_enabled?).to eq(project_data['request_access_enabled']) - expect(imported_project.printing_merge_request_link_enabled?).to eq(project_data['printing_merge_request_link_enabled']) - expect(imported_project.remove_source_branch_after_merge?).to eq(project_data['remove_source_branch_after_merge']) - expect(imported_project.autoclose_referenced_issues?).to eq(project_data['autoclose_referenced_issues']) - expect(imported_project.wiki_enabled?).to eq(project_data['wiki_enabled']) + expect(imported_project.visibility).to eq(project_data['visibility']) + expect(imported_project.created_at).to eq(project_data['created_at']) end end diff --git a/spec/lib/bulk_imports/projects/transformers/project_attributes_transformer_spec.rb b/spec/lib/bulk_imports/projects/transformers/project_attributes_transformer_spec.rb index 822bb9a5605..a1d77b9732d 100644 --- a/spec/lib/bulk_imports/projects/transformers/project_attributes_transformer_spec.rb +++ b/spec/lib/bulk_imports/projects/transformers/project_attributes_transformer_spec.rb @@ -25,8 +25,8 @@ RSpec.describe BulkImports::Projects::Transformers::ProjectAttributesTransformer let(:data) do { - 'name' => 'source_name', - 'visibility' => 'private' + 'visibility' => 'private', + 'created_at' => '2016-11-18T09:29:42.634Z' } end @@ -76,8 +76,21 @@ RSpec.describe BulkImports::Projects::Transformers::ProjectAttributesTransformer end end - it 'converts all keys to symbols' do - expect(transformed_data.keys).to contain_exactly(:name, :path, :import_type, :visibility_level, :namespace_id) + context 'when data has extra keys' do + it 'returns a fixed number of keys' do + data = { + 'visibility' => 'private', + 'created_at' => '2016-11-18T09:29:42.634Z', + 'my_key' => 'my_key', + 'another_key' => 'another_key', + 'last_key' => 'last_key' + } + + transformed_data = described_class.new.transform(context, data) + + expect(transformed_data.keys) + .to contain_exactly(:created_at, :import_type, :name, :namespace_id, :path, :visibility_level) + end end end end diff --git a/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb b/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb index fe3b638d20f..dea584e5019 100644 --- a/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb +++ b/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb @@ -86,6 +86,65 @@ RSpec.describe Gitlab::ImportExport::DecompressedArchiveSizeValidator do include_examples 'logs raised exception and terminates validator process group' end end + + context 'archive path validation' do + let(:filesize) { nil } + + before do + expect(Gitlab::Import::Logger) + .to receive(:info) + .with( + import_upload_archive_path: filepath, + import_upload_archive_size: filesize, + message: error_message + ) + end + + context 'when archive path is traversed' do + let(:filepath) { '/foo/../bar' } + let(:error_message) { 'Invalid path' } + + it 'returns false' do + expect(subject.valid?).to eq(false) + end + end + + context 'when archive path is not a string' do + let(:filepath) { 123 } + let(:error_message) { 'Archive path is not a string' } + + it 'returns false' do + expect(subject.valid?).to eq(false) + end + end + + context 'which archive path is a symlink' do + let(:filepath) { File.join(Dir.tmpdir, 'symlink') } + let(:error_message) { 'Archive path is a symlink' } + + before do + FileUtils.ln_s(filepath, filepath, force: true) + end + + it 'returns false' do + expect(subject.valid?).to eq(false) + end + end + + context 'when archive path is not a file' do + let(:filepath) { Dir.mktmpdir } + let(:filesize) { File.size(filepath) } + let(:error_message) { 'Archive path is not a file' } + + after do + FileUtils.rm_rf(filepath) + end + + it 'returns false' do + expect(subject.valid?).to eq(false) + end + end + end end def create_compressed_file diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index 2939a40a84f..15b6b45eaba 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -10,7 +10,9 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do let(:sentry_client) { instance_double(ErrorTracking::SentryClient) } - subject(:setting) { build(:project_error_tracking_setting, project: project) } + let(:sentry_project_id) { 10 } + + subject(:setting) { build(:project_error_tracking_setting, project: project, sentry_project_id: sentry_project_id) } describe 'Associations' do it { is_expected.to belong_to(:project) } @@ -270,7 +272,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end describe '#issue_details' do - let(:issue) { build(:error_tracking_sentry_detailed_error) } + let(:issue) { build(:error_tracking_sentry_detailed_error, project_id: sentry_project_id) } let(:commit_id) { issue.first_release_version } let(:result) { subject.issue_details(opts) } let(:opts) { { issue_id: 1 } } @@ -317,12 +319,33 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end + describe '#issue_latest_event' do + let(:error_event) { build(:error_tracking_sentry_error_event, project_id: sentry_project_id) } + let(:result) { subject.issue_latest_event(opts) } + let(:opts) { { issue_id: 1 } } + + before do + stub_reactive_cache(subject, error_event, {}) + synchronous_reactive_cache(subject) + + allow(subject).to receive(:sentry_client).and_return(sentry_client) + allow(sentry_client).to receive(:issue_latest_event).with(opts).and_return(error_event) + end + + it 'returns the error event' do + expect(result[:latest_event].project_id).to eq(sentry_project_id) + end + end + describe '#update_issue' do let(:result) { subject.update_issue(**opts) } let(:opts) { { issue_id: 1, params: {} } } before do allow(subject).to receive(:sentry_client).and_return(sentry_client) + allow(sentry_client).to receive(:issue_details) + .with({ issue_id: 1 }) + .and_return(Gitlab::ErrorTracking::DetailedError.new(project_id: sentry_project_id)) end context 'when sentry response is successful' do @@ -344,6 +367,56 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do expect(result).to eq(error: 'Unexpected Error') end end + + context 'when sentry_project_id is not set' do + let(:sentry_projects) do + [ + Gitlab::ErrorTracking::Project.new( + id: 1111, + name: 'Some Project', + organization_name: 'Org' + ), + Gitlab::ErrorTracking::Project.new( + id: sentry_project_id, + name: setting.project_name, + organization_name: setting.organization_name + ) + ] + end + + context 'when sentry_project_id is not set' do + before do + setting.update!(sentry_project_id: nil) + + allow(sentry_client).to receive(:projects).and_return(sentry_projects) + allow(sentry_client).to receive(:update_issue).with(opts).and_return(true) + end + + it 'tries to backfill it from sentry API' do + expect(result).to eq(updated: true) + + expect(setting.reload.sentry_project_id).to eq(sentry_project_id) + end + + context 'when the project cannot be found on sentry' do + before do + sentry_projects.pop + end + + it 'raises error' do + expect { result }.to raise_error(/Couldn't find project/) + end + end + end + + context 'when mismatching sentry_project_id is detected' do + it 'raises error' do + setting.update!(sentry_project_id: sentry_project_id + 1) + + expect { result }.to raise_error(/The Sentry issue appers to be outside/) + end + end + end end describe 'slugs' do diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 7b3d1abadc1..59fe601ed43 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -2251,4 +2251,25 @@ RSpec.describe ProjectPolicy do it { is_expected.to be_disallowed(:register_project_runners) } end end + + describe 'update_sentry_issue' do + using RSpec::Parameterized::TableSyntax + + where(:role, :allowed) do + :owner | true + :maintainer | true + :developer | true + :reporter | false + :guest | false + end + + let(:project) { public_project } + let(:current_user) { public_send(role) } + + with_them do + it do + expect(subject.can?(:update_sentry_issue)).to be(allowed) + end + end + end end diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 48f2c45bd98..c1217292d5c 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -483,6 +483,29 @@ RSpec.describe API::Labels do let(:request) { api("/projects/#{project.id}/labels", user) } let(:params) { { name: valid_label_title_1 } } end + + context 'with group label' do + let_it_be(:group) { create(:group) } + let_it_be(:group_label) { create(:group_label, title: valid_group_label_title_1, group: group) } + + before do + project.update!(group: group) + end + + it 'returns 401 if user does not have access' do + delete api("/projects/#{project.id}/labels/#{group_label.id}", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'returns 204 if user has access' do + group.add_developer(user) + + delete api("/projects/#{project.id}/labels/#{group_label.id}", user) + + expect(response).to have_gitlab_http_status(:no_content) + end + end end describe 'PUT /projects/:id/labels' do @@ -537,6 +560,44 @@ RSpec.describe API::Labels do expect(response).to have_gitlab_http_status(:bad_request) end + + context 'with group label' do + let_it_be(:group) { create(:group) } + let_it_be(:group_label) { create(:group_label, title: valid_group_label_title_1, group: group) } + + before do + project.update!(group: group) + end + + it 'allows updating of group label priority' do + put api("/projects/#{project.id}/labels/#{group_label.id}", user), params: { priority: 5 } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['priority']).to eq(5) + end + + it 'returns 401 when updating other fields' do + put api("/projects/#{project.id}/labels/#{group_label.id}", user), params: { + priority: 5, + new_name: 'new label name' + } + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'returns 200 when user has access to the group label' do + group.add_developer(user) + + put api("/projects/#{project.id}/labels/#{group_label.id}", user), params: { + priority: 5, + new_name: 'new label name' + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['priority']).to eq(5) + expect(json_response['name']).to eq('new label name') + end + end end describe 'PUT /projects/:id/labels/promote' do diff --git a/spec/serializers/member_user_entity_spec.rb b/spec/serializers/member_user_entity_spec.rb index 0e6d4bcc3fb..85f29845d65 100644 --- a/spec/serializers/member_user_entity_spec.rb +++ b/spec/serializers/member_user_entity_spec.rb @@ -11,7 +11,7 @@ RSpec.describe MemberUserEntity do let(:entity_hash) { entity.as_json } it 'matches json schema' do - expect(entity.to_json).to match_schema('entities/member_user') + expect(entity.to_json).to match_schema('entities/member_user_default') end it 'correctly exposes `avatar_url`' do @@ -27,10 +27,8 @@ RSpec.describe MemberUserEntity do expect(entity_hash[:blocked]).to be(true) end - it 'correctly exposes `two_factor_enabled`' do - allow(user).to receive(:two_factor_enabled?).and_return(true) - - expect(entity_hash[:two_factor_enabled]).to be(true) + it 'does not expose `two_factor_enabled` by default' do + expect(entity_hash[:two_factor_enabled]).to be(nil) end it 'correctly exposes `status.emoji`' do @@ -44,4 +42,66 @@ RSpec.describe MemberUserEntity do it 'correctly exposes `last_activity_on`' do expect(entity_hash[:last_activity_on]).to be(user.last_activity_on) end + + context 'when options includes a source' do + let(:current_user) { create(:user) } + let(:options) { { current_user: current_user, source: source } } + let(:entity) { described_class.new(user, options) } + + shared_examples 'correctly exposes user two_factor_enabled' do + context 'when the current_user has a role lower than minimum manage member role' do + before do + source.add_user(current_user, Gitlab::Access::DEVELOPER) + end + + it 'does not expose user two_factor_enabled' do + expect(entity_hash[:two_factor_enabled]).to be(nil) + end + + it 'matches json schema' do + expect(entity.to_json).to match_schema('entities/member_user_default') + end + end + + context 'when the current user has a minimum manage member role or higher' do + before do + source.add_user(current_user, minimum_manage_member_role) + end + + it 'matches json schema' do + expect(entity.to_json).to match_schema('entities/member_user_for_admin_member') + end + + it 'exposes user two_factor_enabled' do + expect(entity_hash[:two_factor_enabled]).to be(false) + end + end + + context 'when the current user is self' do + let(:current_user) { user } + + it 'exposes user two_factor_enabled' do + expect(entity_hash[:two_factor_enabled]).to be(false) + end + + it 'matches json schema' do + expect(entity.to_json).to match_schema('entities/member_user_for_admin_member') + end + end + end + + context 'when the source is a group' do + let(:source) { create(:group) } + let(:minimum_manage_member_role) { Gitlab::Access::OWNER } + + it_behaves_like 'correctly exposes user two_factor_enabled' + end + + context 'when the source is a project' do + let(:source) { create(:project) } + let(:minimum_manage_member_role) { Gitlab::Access::MAINTAINER } + + it_behaves_like 'correctly exposes user two_factor_enabled' + end + end end diff --git a/spec/services/bulk_imports/file_decompression_service_spec.rb b/spec/services/bulk_imports/file_decompression_service_spec.rb index 1d6aa79a37f..77348428d60 100644 --- a/spec/services/bulk_imports/file_decompression_service_spec.rb +++ b/spec/services/bulk_imports/file_decompression_service_spec.rb @@ -80,7 +80,8 @@ RSpec.describe BulkImports::FileDecompressionService do subject { described_class.new(tmpdir: tmpdir, filename: 'symlink.gz') } it 'raises an error and removes the file' do - expect { subject.execute }.to raise_error(described_class::ServiceError, 'Invalid file') + expect { subject.execute } + .to raise_error(BulkImports::FileDecompressionService::ServiceError, 'File decompression error') expect(File.exist?(symlink)).to eq(false) end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 5c1544d8ebc..9f006603f29 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -303,7 +303,7 @@ RSpec.describe Issues::CreateService do context 'user is reporter or above' do before do - project.add_reporter(user) + project.add_developer(user) end it 'assigns the sentry error' do diff --git a/spec/support/shared_contexts/sentry_error_tracking_shared_context.rb b/spec/support/shared_contexts/sentry_error_tracking_shared_context.rb index e8ccb12e6b7..e7360de34f6 100644 --- a/spec/support/shared_contexts/sentry_error_tracking_shared_context.rb +++ b/spec/support/shared_contexts/sentry_error_tracking_shared_context.rb @@ -16,6 +16,6 @@ RSpec.shared_context 'sentry error tracking context' do before do allow(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting) - project.add_reporter(user) + project.add_developer(user) end end |