diff --git a/app/assets/javascripts/security_configuration/components/app.vue b/app/assets/javascripts/security_configuration/components/app.vue index ff0e0ed0ab2..866148596e3 100644 --- a/app/assets/javascripts/security_configuration/components/app.vue +++ b/app/assets/javascripts/security_configuration/components/app.vue @@ -4,9 +4,10 @@ import { __, s__ } from '~/locale'; import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue'; import UserCalloutDismisser from '~/vue_shared/components/user_callout_dismisser.vue'; import SectionLayout from '~/vue_shared/security_configuration/components/section_layout.vue'; +import currentLicenseQuery from '~/security_configuration/graphql/current_license.query.graphql'; import AutoDevOpsAlert from './auto_dev_ops_alert.vue'; import AutoDevOpsEnabledAlert from './auto_dev_ops_enabled_alert.vue'; -import { AUTO_DEVOPS_ENABLED_ALERT_DISMISSED_STORAGE_KEY } from './constants'; +import { AUTO_DEVOPS_ENABLED_ALERT_DISMISSED_STORAGE_KEY, LICENSE_ULTIMATE } from './constants'; import FeatureCard from './feature_card.vue'; import TrainingProviderList from './training_provider_list.vue'; import UpgradeBanner from './upgrade_banner.vue'; @@ -50,6 +51,14 @@ export default { TrainingProviderList, }, inject: ['projectFullPath', 'vulnerabilityTrainingDocsPath'], + apollo: { + currentLicensePlan: { + query: currentLicenseQuery, + update({ currentLicense }) { + return currentLicense?.plan; + }, + }, + }, props: { augmentedSecurityFeatures: { type: Array, @@ -89,6 +98,7 @@ export default { return { autoDevopsEnabledAlertDismissedProjects: [], errorMessage: '', + currentLicensePlan: '', }; }, computed: { @@ -109,6 +119,9 @@ export default { !this.autoDevopsEnabledAlertDismissedProjects.includes(this.projectFullPath) ); }, + shouldShowVulnerabilityManagementTab() { + return this.currentLicensePlan === LICENSE_ULTIMATE; + }, }, methods: { dismissAutoDevopsEnabledAlert() { @@ -250,6 +263,7 @@ export default { ul.nav > li:not(.d-none) { margin: 0 2px; } diff --git a/app/assets/stylesheets/startup/startup-general.scss b/app/assets/stylesheets/startup/startup-general.scss index 989e63d32c4..999f1772118 100644 --- a/app/assets/stylesheets/startup/startup-general.scss +++ b/app/assets/stylesheets/startup/startup-general.scss @@ -792,6 +792,9 @@ input { margin: 4px 2px 4px -12px; border-radius: 4px; } +.navbar-gitlab .header-content .title .canary-badge { + margin-left: -8px; +} .navbar-gitlab .header-content .navbar-collapse > ul.nav > li:not(.d-none) { margin: 0 2px; } diff --git a/app/controllers/projects/service_ping_controller.rb b/app/controllers/projects/service_ping_controller.rb index 368da8d1ef2..d8e3990a244 100644 --- a/app/controllers/projects/service_ping_controller.rb +++ b/app/controllers/projects/service_ping_controller.rb @@ -17,6 +17,7 @@ class Projects::ServicePingController < Projects::ApplicationController return render_404 unless Gitlab::CurrentSettings.web_ide_clientside_preview_enabled? Gitlab::UsageDataCounters::WebIdeCounter.increment_previews_success_count + Gitlab::UsageDataCounters::EditorUniqueCounter.track_live_preview_edit_action(author: current_user) head(200) end diff --git a/app/models/concerns/integrations/loggable.rb b/app/models/concerns/integrations/loggable.rb new file mode 100644 index 00000000000..6d9c04b49cd --- /dev/null +++ b/app/models/concerns/integrations/loggable.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Integrations + module Loggable + def log_info(message, params = {}) + message = build_message(message, params) + + logger.info(message) + end + + def log_error(message, params = {}) + message = build_message(message, params) + + logger.error(message) + end + + def build_message(message, params = {}) + { + integration_class: self.class.name, + project_id: project&.id, + project_path: project&.full_path, + message: message + }.merge(params) + end + + def logger + Gitlab::IntegrationsLogger + end + end +end diff --git a/app/models/concerns/project_services_loggable.rb b/app/models/concerns/project_services_loggable.rb deleted file mode 100644 index e5385435138..00000000000 --- a/app/models/concerns/project_services_loggable.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -module ProjectServicesLoggable - def log_info(message, params = {}) - message = build_message(message, params) - - logger.info(message) - end - - def log_error(message, params = {}) - message = build_message(message, params) - - logger.error(message) - end - - def build_message(message, params = {}) - { - service_class: self.class.name, - project_id: project&.id, - project_path: project&.full_path, - message: message - }.merge(params) - end - - def logger - Gitlab::ProjectServiceLogger - end -end diff --git a/app/models/integration.rb b/app/models/integration.rb index 37447890c27..13ef37e0157 100644 --- a/app/models/integration.rb +++ b/app/models/integration.rb @@ -5,7 +5,7 @@ class Integration < ApplicationRecord include Sortable include Importable - include ProjectServicesLoggable + include Integrations::Loggable include Integrations::HasDataFields include Integrations::ResetSecretFields include FromUnion diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb index a667c843bc6..a341d1ef661 100644 --- a/app/policies/issue_policy.rb +++ b/app/policies/issue_policy.rb @@ -12,8 +12,11 @@ class IssuePolicy < IssuablePolicy @user && IssueCollection.new([@subject]).visible_to(@user).any? end - desc "User can read contacts belonging to the issue group" - condition(:can_read_crm_contacts, scope: :subject) { @user.can?(:read_crm_contact, @subject.project.root_ancestor) } + desc "Project belongs to a group, crm is enabled and user can read contacts in the root group" + condition(:can_read_crm_contacts, scope: :subject) do + subject.project.group&.crm_enabled? && + @user.can?(:read_crm_contact, @subject.project.root_ancestor) + end desc "Issue is confidential" condition(:confidential, scope: :subject) { @subject.confidential? } @@ -81,6 +84,10 @@ class IssuePolicy < IssuablePolicy enable :set_confidentiality end + rule { can_read_crm_contacts }.policy do + enable :read_crm_contacts + end + rule { can?(:set_issue_metadata) & can_read_crm_contacts }.policy do enable :set_issue_crm_contacts end diff --git a/app/serializers/issue_sidebar_basic_entity.rb b/app/serializers/issue_sidebar_basic_entity.rb index 7222b5df425..2450c6a4d85 100644 --- a/app/serializers/issue_sidebar_basic_entity.rb +++ b/app/serializers/issue_sidebar_basic_entity.rb @@ -12,7 +12,7 @@ class IssueSidebarBasicEntity < IssuableSidebarBasicEntity end expose :show_crm_contacts do |issuable| - current_user&.can?(:read_crm_contact, issuable.project.root_ancestor) && + current_user&.can?(:read_crm_contacts, issuable) && CustomerRelations::Contact.exists_for_group?(issuable.project.root_ancestor) end end diff --git a/app/services/concerns/group_linkable.rb b/app/services/concerns/group_linkable.rb new file mode 100644 index 00000000000..3e2e9cfd5eb --- /dev/null +++ b/app/services/concerns/group_linkable.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module GroupLinkable + extend ActiveSupport::Concern + + def execute + return error('Not Found', 404) unless valid_to_create? + + build_link + + if link.save + after_successful_save + success(link: link) + else + error(link.errors.full_messages.to_sentence, 409) + end + end + + private + + attr_reader :shared_with_group, :link + + def sharing_allowed? + sharing_outside_hierarchy_allowed? || within_hierarchy? + end + + def sharing_outside_hierarchy_allowed? + !root_ancestor.namespace_settings.prevent_sharing_groups_outside_hierarchy + end + + def within_hierarchy? + root_ancestor.self_and_descendants_ids.include?(shared_with_group.id) + end + + def after_successful_save + setup_authorizations + end +end diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb index 63f3f73905a..269637805ad 100644 --- a/app/services/git/base_hooks_service.rb +++ b/app/services/git/base_hooks_service.rb @@ -172,7 +172,7 @@ module Git else # This service runs in Sidekiq, so this shouldn't ever be # called, but this is included just in case. - Gitlab::ProjectServiceLogger + Gitlab::IntegrationsLogger end end end diff --git a/app/services/groups/group_links/create_service.rb b/app/services/groups/group_links/create_service.rb index 8c3ba0a63f2..56ddf3ec0b4 100644 --- a/app/services/groups/group_links/create_service.rb +++ b/app/services/groups/group_links/create_service.rb @@ -3,50 +3,35 @@ module Groups module GroupLinks class CreateService < Groups::BaseService - def initialize(shared_group, shared_with_group, user, params) - @shared_group = shared_group - super(shared_with_group, user, params) - end + include GroupLinkable - def execute - unless shared_with_group && shared_group && - can?(current_user, :admin_group_member, shared_group) && - can?(current_user, :read_group, shared_with_group) && - sharing_allowed? - return error('Not Found', 404) - end + def initialize(group, shared_with_group, user, params) + @shared_with_group = shared_with_group - link = GroupGroupLink.new( - shared_group: shared_group, - shared_with_group: shared_with_group, - group_access: params[:shared_group_access], - expires_at: params[:expires_at] - ) - - if link.save - shared_with_group.refresh_members_authorized_projects(blocking: false, direct_members_only: true) - success(link: link) - else - error(link.errors.full_messages.to_sentence, 409) - end + super(group, user, params) end private - attr_reader :shared_group + delegate :root_ancestor, to: :group - alias_method :shared_with_group, :group - - def sharing_allowed? - sharing_outside_hierarchy_allowed? || within_hierarchy? + def valid_to_create? + can?(current_user, :admin_group_member, group) && + can?(current_user, :read_group, shared_with_group) && + sharing_allowed? end - def sharing_outside_hierarchy_allowed? - !shared_group.root_ancestor.namespace_settings.prevent_sharing_groups_outside_hierarchy + def build_link + @link = GroupGroupLink.new( + shared_group: group, + shared_with_group: shared_with_group, + group_access: params[:shared_group_access], + expires_at: params[:expires_at] + ) end - def within_hierarchy? - shared_group.root_ancestor.self_and_descendants_ids.include?(shared_with_group.id) + def setup_authorizations + shared_with_group.refresh_members_authorized_projects(blocking: false, direct_members_only: true) end end end diff --git a/app/services/jira/requests/base.rb b/app/services/jira/requests/base.rb index 3e15d47e8af..f73bba55bb9 100644 --- a/app/services/jira/requests/base.rb +++ b/app/services/jira/requests/base.rb @@ -3,7 +3,7 @@ module Jira module Requests class Base - include ProjectServicesLoggable + include Integrations::Loggable JIRA_API_VERSION = 2 # Limit the size of the JSON error message we will attempt to parse, as the JSON is external input. diff --git a/app/services/jira_connect/sync_service.rb b/app/services/jira_connect/sync_service.rb index bddc7cbe5a0..92255711399 100644 --- a/app/services/jira_connect/sync_service.rb +++ b/app/services/jira_connect/sync_service.rb @@ -39,7 +39,7 @@ module JiraConnect end def logger - Gitlab::ProjectServiceLogger + Gitlab::IntegrationsLogger end end end diff --git a/app/services/projects/group_links/create_service.rb b/app/services/projects/group_links/create_service.rb index a0232779c97..72036aaff35 100644 --- a/app/services/projects/group_links/create_service.rb +++ b/app/services/projects/group_links/create_service.rb @@ -3,26 +3,31 @@ module Projects module GroupLinks class CreateService < BaseService - def execute(group) - return error('Not Found', 404) unless group && can?(current_user, :read_namespace, group) + include GroupLinkable - link = project.project_group_links.new( - group: group, - group_access: params[:link_group_access], - expires_at: params[:expires_at] - ) + def initialize(project, shared_with_group, user, params) + @shared_with_group = shared_with_group - if link.save - setup_authorizations(group) - success(link: link) - else - error(link.errors.full_messages.to_sentence, 409) - end + super(project, user, params) end private - def setup_authorizations(group) + delegate :root_ancestor, to: :project + + def valid_to_create? + can?(current_user, :read_namespace, shared_with_group) && sharing_allowed? + end + + def build_link + @link = project.project_group_links.new( + group: shared_with_group, + group_access: params[:link_group_access], + expires_at: params[:expires_at] + ) + end + + def setup_authorizations AuthorizedProjectUpdate::ProjectRecalculateWorker.perform_async(project.id) # AuthorizedProjectsWorker uses an exclusive lease per user but @@ -30,7 +35,7 @@ module Projects # compare the inconsistency rates of both approaches, we still run # AuthorizedProjectsWorker but with some delay and lower urgency as a # safety net. - group.refresh_members_authorized_projects( + shared_with_group.refresh_members_authorized_projects( blocking: false, priority: UserProjectAccessChangedService::LOW_PRIORITY ) diff --git a/config/feature_flags/development/ci_expand_environment_name_and_url.yml b/config/feature_flags/development/ci_expand_environment_name_and_url.yml index 58626cdfc46..beb7065cf53 100644 --- a/config/feature_flags/development/ci_expand_environment_name_and_url.yml +++ b/config/feature_flags/development/ci_expand_environment_name_and_url.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/358831 milestone: '14.10' type: development group: group::pipeline authoring -default_enabled: false +default_enabled: true diff --git a/config/metrics/counts_28d/20220428154012_live_preview.yml b/config/metrics/counts_28d/20220428154012_live_preview.yml new file mode 100644 index 00000000000..8d2954cb825 --- /dev/null +++ b/config/metrics/counts_28d/20220428154012_live_preview.yml @@ -0,0 +1,26 @@ +--- +data_category: optional +key_path: usage_activity_by_stage_monthly.create.action_monthly_active_users_live_preview_edit +description: Count of monthly unique users that successfully connect to Live Preview +product_section: dev +product_stage: create +product_group: group::editor +product_category: web_ide +value_type: number +status: active +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/85420 +time_frame: 28d +data_source: redis_hll +instrumentation_class: RedisHLLMetric +options: + events: + - g_edit_by_live_preview +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate +performance_indicator_type: [] +milestone: "15.0" diff --git a/data/removals/15_0/15-0-removal-testcoveragesetting.yml b/data/removals/15_0/15-0-removal-testcoveragesetting.yml new file mode 100644 index 00000000000..b929eaa96dd --- /dev/null +++ b/data/removals/15_0/15-0-removal-testcoveragesetting.yml @@ -0,0 +1,14 @@ +- name: "Test coverage project CI/CD setting" # The headline announcing the removal. i.e. "`CI_PROJECT_CONFIG_PATH` removed in Gitlab 14.0" + announcement_milestone: "14.8" # The milestone when this feature was deprecated. + announcement_date: "2022-03-22" # The date of the milestone release when this feature was deprecated. This should almost always be the 22nd of a month (YYYY-MM-DD), unless you did an out of band blog post. + removal_milestone: "15.0" # The milestone when this feature is being removed. + removal_date: "2022-05-22" # This should almost always be the 22nd of a month (YYYY-MM-DD), the date of the milestone release when this feature will be removed. + breaking_change: true # Change to true if this removal is a breaking change. + reporter: exampleuser # GitLab username of the person reporting the removal + body: | # Do not modify this line, instead modify the lines below. + To specify a test coverage pattern, beginning in GitLab 15.0 the + [project setting for test coverage parsing](https://docs.gitlab.com/ee/ci/pipelines/settings.html#add-test-coverage-results-to-a-merge-request-deprecated) + has been removed. + + To set test coverage parsing, use the project’s `.gitlab-ci.yml` file by providing a regular expression with the + [`coverage` keyword](https://docs.gitlab.com/ee/ci/yaml/index.html#coverage). diff --git a/db/migrate/20220420173247_add_group_inheritance_type_to_pe_authorizable.rb b/db/migrate/20220420173247_add_group_inheritance_type_to_pe_authorizable.rb new file mode 100644 index 00000000000..f1ddf48304e --- /dev/null +++ b/db/migrate/20220420173247_add_group_inheritance_type_to_pe_authorizable.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class AddGroupInheritanceTypeToPeAuthorizable < Gitlab::Database::Migration[2.0] + def change + add_column :protected_environment_deploy_access_levels, + :group_inheritance_type, + :smallint, + default: 0, limit: 2, null: false + add_column :protected_environment_approval_rules, + :group_inheritance_type, + :smallint, + default: 0, limit: 2, null: false + end +end diff --git a/db/schema_migrations/20220420173247 b/db/schema_migrations/20220420173247 new file mode 100644 index 00000000000..9ab6dac867e --- /dev/null +++ b/db/schema_migrations/20220420173247 @@ -0,0 +1 @@ +a4113363674f268a3beaef22e29b2aba4e5ba7566bc47dc5676ddc8f8733d331 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index ea6c398df46..a84db424277 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -19786,6 +19786,7 @@ CREATE TABLE protected_environment_approval_rules ( updated_at timestamp with time zone NOT NULL, access_level smallint, required_approvals smallint NOT NULL, + group_inheritance_type smallint DEFAULT 0 NOT NULL, CONSTRAINT chk_rails_bed75249bc CHECK ((((access_level IS NOT NULL) AND (group_id IS NULL) AND (user_id IS NULL)) OR ((user_id IS NOT NULL) AND (access_level IS NULL) AND (group_id IS NULL)) OR ((group_id IS NOT NULL) AND (user_id IS NULL) AND (access_level IS NULL)))), CONSTRAINT chk_rails_cfa90ae3b5 CHECK ((required_approvals > 0)) ); @@ -19806,7 +19807,8 @@ CREATE TABLE protected_environment_deploy_access_levels ( access_level integer DEFAULT 40, protected_environment_id integer NOT NULL, user_id integer, - group_id integer + group_id integer, + group_inheritance_type smallint DEFAULT 0 NOT NULL ); CREATE SEQUENCE protected_environment_deploy_access_levels_id_seq diff --git a/doc/update/removals.md b/doc/update/removals.md index 98d4718049f..a536d938eda 100644 --- a/doc/update/removals.md +++ b/doc/update/removals.md @@ -104,6 +104,21 @@ A feature flag was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/11 In GitLab 15.0, we will remove the feature flag, and you must always authenticate when you use the Dependency Proxy. +### Test coverage project CI/CD setting + +WARNING: +This feature was changed or removed in 15.0 +as a [breaking change](https://docs.gitlab.com/ee/development/contributing/#breaking-changes). +Before updating GitLab, review the details carefully to determine if you need to make any +changes to your code, settings, or workflow. + +To specify a test coverage pattern, beginning in GitLab 15.0 the +[project setting for test coverage parsing](https://docs.gitlab.com/ee/ci/pipelines/settings.html#add-test-coverage-results-to-a-merge-request-deprecated) +has been removed. + +To set test coverage parsing, use the project’s `.gitlab-ci.yml` file by providing a regular expression with the +[`coverage` keyword](https://docs.gitlab.com/ee/ci/yaml/index.html#coverage). + ### Update to the Container Registry group-level API WARNING: diff --git a/doc/user/infrastructure/iac/terraform_state.md b/doc/user/infrastructure/iac/terraform_state.md index 60f97f522cf..7277a9c714a 100644 --- a/doc/user/infrastructure/iac/terraform_state.md +++ b/doc/user/infrastructure/iac/terraform_state.md @@ -199,8 +199,8 @@ and the CI YAML file: dependencies: - plan when: manual - only: - - master + rules: + - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH ``` 1. Push your project to GitLab, which triggers a CI job pipeline. This pipeline diff --git a/lib/api/groups.rb b/lib/api/groups.rb index d6f679d0255..60bb51bf48f 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -418,7 +418,6 @@ module API optional :expires_at, type: Date, desc: 'Share expiration date' end post ":id/share", feature_category: :subgroups do - shared_group = find_group!(params[:id]) shared_with_group = find_group!(params[:group_id]) group_link_create_params = { @@ -426,11 +425,11 @@ module API expires_at: params[:expires_at] } - result = ::Groups::GroupLinks::CreateService.new(shared_group, shared_with_group, current_user, group_link_create_params).execute - shared_group.preload_shared_group_links + result = ::Groups::GroupLinks::CreateService.new(user_group, shared_with_group, current_user, group_link_create_params).execute + user_group.preload_shared_group_links if result[:status] == :success - present shared_group, with: Entities::GroupDetail, current_user: current_user + present user_group, with: Entities::GroupDetail, current_user: current_user else render_api_error!(result[:message], result[:http_status]) end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 0bb062a6f66..28d6bfb1858 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -575,14 +575,14 @@ module API end post ":id/share", feature_category: :authentication_and_authorization do authorize! :admin_project, user_project - group = Group.find_by_id(params[:group_id]) + shared_with_group = Group.find_by_id(params[:group_id]) unless user_project.allowed_to_share_with_group? break render_api_error!("The project sharing with group is disabled", 400) end - result = ::Projects::GroupLinks::CreateService.new(user_project, current_user, declared_params(include_missing: false)) - .execute(group) + result = ::Projects::GroupLinks::CreateService + .new(user_project, shared_with_group, current_user, declared_params(include_missing: false)).execute if result[:status] == :success present result[:link], with: Entities::ProjectGroupLink diff --git a/lib/bulk_imports/projects/stage.rb b/lib/bulk_imports/projects/stage.rb index 289db225c5d..229df9c410d 100644 --- a/lib/bulk_imports/projects/stage.rb +++ b/lib/bulk_imports/projects/stage.rb @@ -63,10 +63,6 @@ module BulkImports pipeline: BulkImports::Projects::Pipelines::ProtectedBranchesPipeline, stage: 4 }, - ci_pipelines: { - pipeline: BulkImports::Projects::Pipelines::CiPipelinesPipeline, - stage: 4 - }, project_feature: { pipeline: BulkImports::Projects::Pipelines::ProjectFeaturePipeline, stage: 4 @@ -83,6 +79,10 @@ module BulkImports pipeline: BulkImports::Projects::Pipelines::ReleasesPipeline, stage: 4 }, + ci_pipelines: { + pipeline: BulkImports::Projects::Pipelines::CiPipelinesPipeline, + stage: 5 + }, wiki: { pipeline: BulkImports::Common::Pipelines::WikiPipeline, stage: 5 diff --git a/lib/gitlab/project_service_logger.rb b/lib/gitlab/integrations_logger.rb similarity index 70% rename from lib/gitlab/project_service_logger.rb rename to lib/gitlab/integrations_logger.rb index 9b0357d3161..c62a5f6d321 100644 --- a/lib/gitlab/project_service_logger.rb +++ b/lib/gitlab/integrations_logger.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Gitlab - class ProjectServiceLogger < Gitlab::JsonLogger + class IntegrationsLogger < Gitlab::JsonLogger def self.file_name_noext 'integrations_json' end diff --git a/lib/gitlab/usage_data_counters/editor_unique_counter.rb b/lib/gitlab/usage_data_counters/editor_unique_counter.rb index bc0126cd893..f97ebdccecf 100644 --- a/lib/gitlab/usage_data_counters/editor_unique_counter.rb +++ b/lib/gitlab/usage_data_counters/editor_unique_counter.rb @@ -8,6 +8,7 @@ module Gitlab EDIT_BY_WEB_IDE = 'g_edit_by_web_ide' EDIT_BY_SSE = 'g_edit_by_sse' EDIT_CATEGORY = 'ide_edit' + EDIT_BY_LIVE_PREVIEW = 'g_edit_by_live_preview' class << self def track_web_ide_edit_action(author:, time: Time.zone.now) @@ -47,6 +48,10 @@ module Gitlab count_unique(EDIT_BY_SSE, date_from, date_to) end + def track_live_preview_edit_action(author:, time: Time.zone.now) + track_unique_action(EDIT_BY_LIVE_PREVIEW, author, time) + end + private def track_unique_action(action, author, time) diff --git a/lib/gitlab/usage_data_counters/known_events/common.yml b/lib/gitlab/usage_data_counters/known_events/common.yml index 0d89a5181ec..448ed4c66e1 100644 --- a/lib/gitlab/usage_data_counters/known_events/common.yml +++ b/lib/gitlab/usage_data_counters/known_events/common.yml @@ -40,6 +40,11 @@ redis_slot: edit expiry: 29 aggregation: daily +- name: g_edit_by_live_preview + category: ide_edit + redis_slot: edit + expiry: 29 + aggregation: daily - name: i_search_total category: search redis_slot: search diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2260741f390..9532437d5a1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -11725,25 +11725,25 @@ msgstr "" msgid "DastSiteValidation|Revoke validation" msgstr "" -msgid "DastSiteValidation|Step 1 - Choose site validation method" +msgid "DastSiteValidation|Step 1 - Choose site validation method." msgstr "" -msgid "DastSiteValidation|Step 2 - Add following HTTP header to your site" +msgid "DastSiteValidation|Step 2 - Add the following HTTP header to your site." msgstr "" -msgid "DastSiteValidation|Step 2 - Add following meta tag to your site" +msgid "DastSiteValidation|Step 2 - Add the following meta tag to your site." msgstr "" -msgid "DastSiteValidation|Step 2 - Add following text to the target site" +msgid "DastSiteValidation|Step 2 - Download the following text file, then upload it to the target site." msgstr "" -msgid "DastSiteValidation|Step 3 - Confirm header location and validate" +msgid "DastSiteValidation|Step 3 - Confirm header location." msgstr "" -msgid "DastSiteValidation|Step 3 - Confirm meta tag location and validate" +msgid "DastSiteValidation|Step 3 - Confirm meta tag location." msgstr "" -msgid "DastSiteValidation|Step 3 - Confirm text file location and validate" +msgid "DastSiteValidation|Step 3 - Confirm text file location." msgstr "" msgid "DastSiteValidation|Text file validation" @@ -11760,13 +11760,13 @@ msgid_plural "DastSiteValidation|This will affect %d other profiles targeting th msgstr[0] "" msgstr[1] "" -msgid "DastSiteValidation|To run an active scan, validate your target site. All site profiles that share the same base URL share the same validation status." +msgid "DastSiteValidation|To run an active scan, validate your site. Site profile validation reduces the risk of running an active scan against the wrong website. All site profiles that share the same base URL share the same validation status." msgstr "" msgid "DastSiteValidation|Validate" msgstr "" -msgid "DastSiteValidation|Validate target site" +msgid "DastSiteValidation|Validate site" msgstr "" msgid "DastSiteValidation|Validated" diff --git a/spec/controllers/groups/shared_projects_controller_spec.rb b/spec/controllers/groups/shared_projects_controller_spec.rb index 528d5c073b7..0c5a3b9df08 100644 --- a/spec/controllers/groups/shared_projects_controller_spec.rb +++ b/spec/controllers/groups/shared_projects_controller_spec.rb @@ -12,9 +12,10 @@ RSpec.describe Groups::SharedProjectsController do Projects::GroupLinks::CreateService.new( project, + group, user, link_group_access: Gitlab::Access::DEVELOPER - ).execute(group) + ).execute end let!(:group) { create(:group) } diff --git a/spec/controllers/projects/service_ping_controller_spec.rb b/spec/controllers/projects/service_ping_controller_spec.rb index 13b34290962..fa92efee079 100644 --- a/spec/controllers/projects/service_ping_controller_spec.rb +++ b/spec/controllers/projects/service_ping_controller_spec.rb @@ -79,6 +79,18 @@ RSpec.describe Projects::ServicePingController do it_behaves_like 'counter is not increased' it_behaves_like 'counter is increased', 'WEB_IDE_PREVIEWS_SUCCESS_COUNT' + + context 'when the user has access to the project' do + let(:user) { project.owner } + + it 'increases the live preview view counter' do + expect(Gitlab::UsageDataCounters::EditorUniqueCounter).to receive(:track_live_preview_edit_action).with(author: user) + + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end end context 'when web ide clientside preview is not enabled' do diff --git a/spec/frontend/security_configuration/components/app_spec.js b/spec/frontend/security_configuration/components/app_spec.js index f18225f1246..a983ec43557 100644 --- a/spec/frontend/security_configuration/components/app_spec.js +++ b/spec/frontend/security_configuration/components/app_spec.js @@ -1,6 +1,8 @@ +import Vue, { nextTick } from 'vue'; +import VueApollo from 'vue-apollo'; import { GlTab, GlTabs, GlLink } from '@gitlab/ui'; import { mount } from '@vue/test-utils'; -import { nextTick } from 'vue'; + import { useLocalStorageSpy } from 'helpers/local_storage_helper'; import { makeMockUserCalloutDismisser } from 'helpers/mock_user_callout_dismisser'; import stubChildren from 'helpers/stub_children'; @@ -18,15 +20,22 @@ import { LICENSE_COMPLIANCE_DESCRIPTION, LICENSE_COMPLIANCE_HELP_PATH, AUTO_DEVOPS_ENABLED_ALERT_DISMISSED_STORAGE_KEY, + LICENSE_ULTIMATE, + LICENSE_PREMIUM, + LICENSE_FREE, } from '~/security_configuration/components/constants'; import FeatureCard from '~/security_configuration/components/feature_card.vue'; import TrainingProviderList from '~/security_configuration/components/training_provider_list.vue'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import currentLicenseQuery from '~/security_configuration/graphql/current_license.query.graphql'; +import waitForPromises from 'helpers/wait_for_promises'; import UpgradeBanner from '~/security_configuration/components/upgrade_banner.vue'; import { REPORT_TYPE_LICENSE_COMPLIANCE, REPORT_TYPE_SAST, } from '~/vue_shared/security_reports/constants'; +import { getCurrentLicensePlanResponse } from '../mock_data'; const upgradePath = '/upgrade'; const autoDevopsHelpPagePath = '/autoDevopsHelpPagePath'; @@ -36,14 +45,24 @@ const projectFullPath = 'namespace/project'; const vulnerabilityTrainingDocsPath = 'user/application_security/vulnerabilities/index'; useLocalStorageSpy(); +Vue.use(VueApollo); describe('App component', () => { let wrapper; let userCalloutDismissSpy; + let mockApollo; - const createComponent = ({ shouldShowCallout = true, ...propsData }) => { + const createComponent = ({ + shouldShowCallout = true, + license = LICENSE_ULTIMATE, + ...propsData + }) => { userCalloutDismissSpy = jest.fn(); + mockApollo = createMockApollo([ + [currentLicenseQuery, jest.fn().mockResolvedValue(getCurrentLicensePlanResponse(license))], + ]); + wrapper = extendedWrapper( mount(SecurityConfigurationApp, { propsData, @@ -54,6 +73,7 @@ describe('App component', () => { projectFullPath, vulnerabilityTrainingDocsPath, }, + apolloProvider: mockApollo, stubs: { ...stubChildren(SecurityConfigurationApp), GlLink: false, @@ -128,14 +148,16 @@ describe('App component', () => { afterEach(() => { wrapper.destroy(); + mockApollo = null; }); describe('basic structure', () => { - beforeEach(() => { + beforeEach(async () => { createComponent({ augmentedSecurityFeatures: securityFeaturesMock, augmentedComplianceFeatures: complianceFeaturesMock, }); + await waitForPromises(); }); it('renders main-heading with correct text', () => { @@ -438,11 +460,12 @@ describe('App component', () => { }); describe('Vulnerability management', () => { - beforeEach(() => { + beforeEach(async () => { createComponent({ augmentedSecurityFeatures: securityFeaturesMock, augmentedComplianceFeatures: complianceFeaturesMock, }); + await waitForPromises(); }); it('renders TrainingProviderList component', () => { @@ -459,5 +482,21 @@ describe('App component', () => { expect(trainingLink.text()).toBe('Learn more about vulnerability training'); expect(trainingLink.attributes('href')).toBe(vulnerabilityTrainingDocsPath); }); + + it.each` + license | display + ${LICENSE_ULTIMATE} | ${true} + ${LICENSE_PREMIUM} | ${false} + ${LICENSE_FREE} | ${false} + ${null} | ${false} + `('displays $display for license $license', async ({ license, display }) => { + createComponent({ + license, + augmentedSecurityFeatures: securityFeaturesMock, + augmentedComplianceFeatures: complianceFeaturesMock, + }); + await waitForPromises(); + expect(findVulnerabilityManagementTab().exists()).toBe(display); + }); }); }); diff --git a/spec/frontend/security_configuration/mock_data.js b/spec/frontend/security_configuration/mock_data.js index 18a480bf082..94a36472a1d 100644 --- a/spec/frontend/security_configuration/mock_data.js +++ b/spec/frontend/security_configuration/mock_data.js @@ -111,3 +111,12 @@ export const tempProviderLogos = { svg: `${[testProviderName[1]]}`, }, }; + +export const getCurrentLicensePlanResponse = (plan) => ({ + data: { + currentLicense: { + id: 'gid://gitlab/License/1', + plan, + }, + }, +}); diff --git a/spec/lib/bulk_imports/projects/stage_spec.rb b/spec/lib/bulk_imports/projects/stage_spec.rb index 3cc9877df7d..e81d9cc5fb4 100644 --- a/spec/lib/bulk_imports/projects/stage_spec.rb +++ b/spec/lib/bulk_imports/projects/stage_spec.rb @@ -20,11 +20,11 @@ RSpec.describe BulkImports::Projects::Stage do [4, BulkImports::Projects::Pipelines::MergeRequestsPipeline], [4, BulkImports::Projects::Pipelines::ExternalPullRequestsPipeline], [4, BulkImports::Projects::Pipelines::ProtectedBranchesPipeline], - [4, BulkImports::Projects::Pipelines::CiPipelinesPipeline], [4, BulkImports::Projects::Pipelines::ProjectFeaturePipeline], [4, BulkImports::Projects::Pipelines::ContainerExpirationPolicyPipeline], [4, BulkImports::Projects::Pipelines::ServiceDeskSettingPipeline], [4, BulkImports::Projects::Pipelines::ReleasesPipeline], + [5, BulkImports::Projects::Pipelines::CiPipelinesPipeline], [5, BulkImports::Common::Pipelines::WikiPipeline], [5, BulkImports::Common::Pipelines::UploadsPipeline], [5, BulkImports::Common::Pipelines::LfsObjectsPipeline], diff --git a/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb index 5f66387c82b..9aecb8f8b25 100644 --- a/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb @@ -80,10 +80,13 @@ RSpec.describe Gitlab::UsageDataCounters::EditorUniqueCounter, :clean_gitlab_red it 'can return the count of actions per user deduplicated' do described_class.track_web_ide_edit_action(author: user1) + described_class.track_live_preview_edit_action(author: user1) described_class.track_snippet_editor_edit_action(author: user1) described_class.track_sfe_edit_action(author: user1) described_class.track_web_ide_edit_action(author: user2, time: time - 2.days) described_class.track_web_ide_edit_action(author: user3, time: time - 3.days) + described_class.track_live_preview_edit_action(author: user2, time: time - 2.days) + described_class.track_live_preview_edit_action(author: user3, time: time - 3.days) described_class.track_snippet_editor_edit_action(author: user3, time: time - 3.days) described_class.track_sfe_edit_action(author: user3, time: time - 3.days) diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 0f596d3908d..a2b914c42f2 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -848,7 +848,7 @@ RSpec.describe Integration do let(:test_message) { "test message" } let(:arguments) do { - service_class: integration.class.name, + integration_class: integration.class.name, project_path: project.full_path, project_id: project.id, message: test_message, @@ -857,13 +857,13 @@ RSpec.describe Integration do end it 'logs info messages using json logger' do - expect(Gitlab::JsonLogger).to receive(:info).with(arguments) + expect(Gitlab::IntegrationsLogger).to receive(:info).with(arguments) integration.log_info(test_message, additional_argument: 'some argument') end it 'logs error messages using json logger' do - expect(Gitlab::JsonLogger).to receive(:error).with(arguments) + expect(Gitlab::IntegrationsLogger).to receive(:error).with(arguments) integration.log_error(test_message, additional_argument: 'some argument') end @@ -872,7 +872,7 @@ RSpec.describe Integration do let(:project) { nil } let(:arguments) do { - service_class: integration.class.name, + integration_class: integration.class.name, project_path: nil, project_id: nil, message: test_message, @@ -881,7 +881,7 @@ RSpec.describe Integration do end it 'logs info messages using json logger' do - expect(Gitlab::JsonLogger).to receive(:info).with(arguments) + expect(Gitlab::IntegrationsLogger).to receive(:info).with(arguments) integration.log_info(test_message, additional_argument: 'some argument') end diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb index 1fe9e430011..557bda985af 100644 --- a/spec/policies/issue_policy_spec.rb +++ b/spec/policies/issue_policy_spec.rb @@ -397,7 +397,7 @@ RSpec.describe IssuePolicy do end end - describe 'set_issue_crm_contacts' do + describe 'crm permissions' do let(:user) { create(:user) } let(:subgroup) { create(:group, :crm_enabled, parent: create(:group, :crm_enabled)) } let(:project) { create(:project, group: subgroup) } @@ -408,6 +408,7 @@ RSpec.describe IssuePolicy do it 'is disallowed' do project.add_reporter(user) + expect(policies).to be_disallowed(:read_crm_contacts) expect(policies).to be_disallowed(:set_issue_crm_contacts) end end @@ -416,6 +417,7 @@ RSpec.describe IssuePolicy do it 'is allowed' do subgroup.add_reporter(user) + expect(policies).to be_disallowed(:read_crm_contacts) expect(policies).to be_disallowed(:set_issue_crm_contacts) end end @@ -424,8 +426,31 @@ RSpec.describe IssuePolicy do it 'is allowed' do subgroup.parent.add_reporter(user) + expect(policies).to be_allowed(:read_crm_contacts) expect(policies).to be_allowed(:set_issue_crm_contacts) end end + + context 'when crm disabled on subgroup' do + let(:subgroup) { create(:group, parent: create(:group, :crm_enabled)) } + + it 'is disallowed' do + subgroup.parent.add_reporter(user) + + expect(policies).to be_disallowed(:read_crm_contacts) + expect(policies).to be_disallowed(:set_issue_crm_contacts) + end + end + + context 'when peronsal namespace' do + let(:project) { create(:project) } + + it 'is disallowed' do + project.add_reporter(user) + + expect(policies).to be_disallowed(:read_crm_contacts) + expect(policies).to be_disallowed(:set_issue_crm_contacts) + end + end end end diff --git a/spec/serializers/issue_sidebar_basic_entity_spec.rb b/spec/serializers/issue_sidebar_basic_entity_spec.rb index 716c97f72af..564ffb1aea9 100644 --- a/spec/serializers/issue_sidebar_basic_entity_spec.rb +++ b/spec/serializers/issue_sidebar_basic_entity_spec.rb @@ -94,5 +94,37 @@ RSpec.describe IssueSidebarBasicEntity do expect(entity[:show_crm_contacts]).to be(expected) end end + + context 'in subgroup' do + let(:subgroup_project) { create(:project, :repository, group: subgroup) } + let(:subgroup_issue) { create(:issue, project: subgroup_project) } + let(:serializer) { IssueSerializer.new(current_user: user, project: subgroup_project) } + + subject(:entity) { serializer.represent(subgroup_issue, serializer: 'sidebar') } + + before do + subgroup_project.root_ancestor.add_reporter(user) + end + + context 'with crm enabled' do + let(:subgroup) { create(:group, :crm_enabled, parent: group) } + + it 'is true' do + allow(CustomerRelations::Contact).to receive(:exists_for_group?).with(group).and_return(true) + + expect(entity[:show_crm_contacts]).to be_truthy + end + end + + context 'with crm disabled' do + let(:subgroup) { create(:group, parent: group) } + + it 'is false' do + allow(CustomerRelations::Contact).to receive(:exists_for_group?).with(group).and_return(true) + + expect(entity[:show_crm_contacts]).to be_falsy + end + end + end end end diff --git a/spec/services/groups/group_links/create_service_spec.rb b/spec/services/groups/group_links/create_service_spec.rb index 03dac14be54..bfbaedbd06f 100644 --- a/spec/services/groups/group_links/create_service_spec.rb +++ b/spec/services/groups/group_links/create_service_spec.rb @@ -3,23 +3,13 @@ require 'spec_helper' RSpec.describe Groups::GroupLinks::CreateService, '#execute' do - let(:parent_group_user) { create(:user) } - let(:group_user) { create(:user) } - let(:child_group_user) { create(:user) } - let(:prevent_sharing) { false } + let_it_be(:shared_with_group_parent) { create(:group, :private) } + let_it_be(:shared_with_group) { create(:group, :private, parent: shared_with_group_parent) } + let_it_be(:shared_with_group_child) { create(:group, :private, parent: shared_with_group) } let_it_be(:group_parent) { create(:group, :private) } - let_it_be(:group) { create(:group, :private, parent: group_parent) } - let_it_be(:group_child) { create(:group, :private, parent: group) } - let(:ns_for_parent) { create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: prevent_sharing) } - let(:shared_group_parent) { create(:group, :private, namespace_settings: ns_for_parent) } - let(:shared_group) { create(:group, :private, parent: shared_group_parent) } - let(:shared_group_child) { create(:group, :private, parent: shared_group) } - - let(:project_parent) { create(:project, group: shared_group_parent) } - let(:project) { create(:project, group: shared_group) } - let(:project_child) { create(:project, group: shared_group_child) } + let(:group) { create(:group, :private, parent: group_parent) } let(:opts) do { @@ -28,127 +18,161 @@ RSpec.describe Groups::GroupLinks::CreateService, '#execute' do } end - let(:user) { group_user } + subject { described_class.new(group, shared_with_group, user, opts) } - subject { described_class.new(shared_group, group, user, opts) } + shared_examples_for 'not shareable' do + it 'does not share and returns an error' do + expect do + result = subject.execute - before do - group.add_guest(group_user) - shared_group.add_owner(group_user) + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(404) + end.not_to change { group.shared_with_group_links.count } + end end - it 'adds group to another group' do - expect { subject.execute }.to change { group.shared_group_links.count }.from(0).to(1) + shared_examples_for 'shareable' do + it 'adds group to another group' do + expect do + result = subject.execute + + expect(result[:status]).to eq(:success) + end.to change { group.shared_with_group_links.count }.from(0).to(1) + end end - it 'returns false if shared group is blank' do - expect { described_class.new(nil, group, user, opts) }.not_to change { group.shared_group_links.count } + context 'when user has proper membership to share a group' do + let_it_be(:group_user) { create(:user) } + + let(:user) { group_user } + + before do + shared_with_group.add_guest(group_user) + group.add_owner(group_user) + end + + it_behaves_like 'shareable' + + context 'when sharing outside the hierarchy is disabled' do + let_it_be(:group_parent) do + create(:group, + namespace_settings: create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: true)) + end + + it_behaves_like 'not shareable' + + context 'when group is inside hierarchy' do + let(:shared_with_group) { create(:group, :private, parent: group_parent) } + + it_behaves_like 'shareable' + end + end + + context 'project authorizations based on group hierarchies' do + let_it_be(:child_group_user) { create(:user) } + let_it_be(:parent_group_user) { create(:user) } + + before do + shared_with_group_parent.add_owner(parent_group_user) + shared_with_group.add_owner(group_user) + shared_with_group_child.add_owner(child_group_user) + end + + context 'project authorizations refresh' do + it 'is executed only for the direct members of the group' do + expect(UserProjectAccessChangedService).to receive(:new).with(contain_exactly(group_user.id)) + .and_call_original + + subject.execute + end + end + + context 'project authorizations' do + let(:group_child) { create(:group, :private, parent: group) } + let(:project_parent) { create(:project, group: group_parent) } + let(:project) { create(:project, group: group) } + let(:project_child) { create(:project, group: group_child) } + + context 'group user' do + let(:user) { group_user } + + it 'create proper authorizations' do + subject.execute + + expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey + expect(Ability.allowed?(user, :read_project, project)).to be_truthy + expect(Ability.allowed?(user, :read_project, project_child)).to be_truthy + end + end + + context 'parent group user' do + let(:user) { parent_group_user } + + it 'create proper authorizations' do + subject.execute + + expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey + expect(Ability.allowed?(user, :read_project, project)).to be_falsey + expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey + end + end + + context 'child group user' do + let(:user) { child_group_user } + + it 'create proper authorizations' do + subject.execute + + expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey + expect(Ability.allowed?(user, :read_project, project)).to be_falsey + expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey + end + end + end + end end context 'user does not have access to group' do let(:user) { create(:user) } before do - shared_group.add_owner(user) + group.add_owner(user) end - it 'returns error' do - result = subject.execute - - expect(result[:status]).to eq(:error) - expect(result[:http_status]).to eq(404) - end + it_behaves_like 'not shareable' end context 'user does not have admin access to shared group' do let(:user) { create(:user) } before do - group.add_guest(user) - shared_group.add_developer(user) + shared_with_group.add_guest(user) + group.add_developer(user) end - it 'returns error' do - result = subject.execute + it_behaves_like 'not shareable' + end - expect(result[:status]).to eq(:error) - expect(result[:http_status]).to eq(404) + context 'when group is blank' do + let(:group_user) { create(:user) } + let(:user) { group_user } + let(:group) { nil } + + it 'does not share and returns an error' do + expect do + result = subject.execute + + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(404) + end.not_to change { shared_with_group.shared_group_links.count } end end - context 'project authorizations based on group hierarchies' do - before do - group_parent.add_owner(parent_group_user) - group.add_owner(group_user) - group_child.add_owner(child_group_user) - end + context 'when shared_with_group is blank' do + let(:group_user) { create(:user) } + let(:user) { group_user } + let(:shared_with_group) { nil } - context 'project authorizations refresh' do - it 'is executed only for the direct members of the group' do - expect(UserProjectAccessChangedService).to receive(:new).with(contain_exactly(group_user.id)).and_call_original - - subject.execute - end - end - - context 'project authorizations' do - context 'group user' do - let(:user) { group_user } - - it 'create proper authorizations' do - subject.execute - - expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey - expect(Ability.allowed?(user, :read_project, project)).to be_truthy - expect(Ability.allowed?(user, :read_project, project_child)).to be_truthy - end - end - - context 'parent group user' do - let(:user) { parent_group_user } - - it 'create proper authorizations' do - subject.execute - - expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey - expect(Ability.allowed?(user, :read_project, project)).to be_falsey - expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey - end - end - - context 'child group user' do - let(:user) { child_group_user } - - it 'create proper authorizations' do - subject.execute - - expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey - expect(Ability.allowed?(user, :read_project, project)).to be_falsey - expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey - end - end - end - end - - context 'sharing outside the hierarchy is disabled' do - let(:prevent_sharing) { true } - - it 'prevents sharing with a group outside the hierarchy' do - result = subject.execute - - expect(group.reload.shared_group_links.count).to eq(0) - expect(result[:status]).to eq(:error) - expect(result[:http_status]).to eq(404) - end - - it 'allows sharing with a group within the hierarchy' do - sibling_group = create(:group, :private, parent: shared_group_parent) - sibling_group.add_guest(group_user) - - result = described_class.new(shared_group, sibling_group, user, opts).execute - - expect(sibling_group.reload.shared_group_links.count).to eq(1) - expect(result[:status]).to eq(:success) - end + it_behaves_like 'not shareable' end end diff --git a/spec/services/issues/set_crm_contacts_service_spec.rb b/spec/services/issues/set_crm_contacts_service_spec.rb index b0befb9f77c..5613cc49cc5 100644 --- a/spec/services/issues/set_crm_contacts_service_spec.rb +++ b/spec/services/issues/set_crm_contacts_service_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Issues::SetCrmContactsService do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group, :crm_enabled) } - let_it_be(:project) { create(:project, group: create(:group, parent: group)) } + let_it_be(:project) { create(:project, group: create(:group, :crm_enabled, parent: group)) } let_it_be(:contacts) { create_list(:contact, 4, group: group) } let_it_be(:issue, reload: true) { create(:issue, project: project) } let_it_be(:issue_contact_1) do @@ -58,6 +58,20 @@ RSpec.describe Issues::SetCrmContactsService do group.add_reporter(user) end + context 'but the crm setting is disabled' do + let(:params) { { replace_ids: [contacts[1].id, contacts[2].id] } } + let(:subgroup_with_crm_disabled) { create(:group, parent: group) } + let(:project_with_crm_disabled) { create(:project, group: subgroup_with_crm_disabled) } + let(:issue_with_crm_disabled) { create(:issue, project: project_with_crm_disabled) } + + it 'returns expected error response' do + response = described_class.new(project: project_with_crm_disabled, current_user: user, params: params).execute(issue_with_crm_disabled) + + expect(response).to be_error + expect(response.message).to eq('You have insufficient permissions to set customer relations contacts for this issue') + end + end + context 'when the contact does not exist' do let(:params) { { replace_ids: [non_existing_record_id] } } diff --git a/spec/services/jira_connect/sync_service_spec.rb b/spec/services/jira_connect/sync_service_spec.rb index c20aecaaef0..75d93a678f1 100644 --- a/spec/services/jira_connect/sync_service_spec.rb +++ b/spec/services/jira_connect/sync_service_spec.rb @@ -24,7 +24,7 @@ RSpec.describe JiraConnect::SyncService do end def expect_log(type, message) - expect(Gitlab::ProjectServiceLogger) + expect(Gitlab::IntegrationsLogger) .to receive(type).with( message: 'response from jira dev_info api', integration: 'JiraConnect', diff --git a/spec/services/projects/group_links/create_service_spec.rb b/spec/services/projects/group_links/create_service_spec.rb index 4ea5f2b3a53..65d3085a850 100644 --- a/spec/services/projects/group_links/create_service_spec.rb +++ b/spec/services/projects/group_links/create_service_spec.rb @@ -5,65 +5,104 @@ require 'spec_helper' RSpec.describe Projects::GroupLinks::CreateService, '#execute' do let_it_be(:user) { create :user } let_it_be(:group) { create :group } - let_it_be(:project) { create :project } + let_it_be(:project) { create(:project, namespace: create(:namespace, :with_namespace_settings)) } - let(:group_access) { Gitlab::Access::DEVELOPER } let(:opts) do { - link_group_access: group_access, + link_group_access: Gitlab::Access::DEVELOPER, expires_at: nil } end - subject { described_class.new(project, user, opts) } + subject { described_class.new(project, group, user, opts) } - before do - group.add_developer(user) + shared_examples_for 'not shareable' do + it 'does not share and returns an error' do + expect do + result = subject.execute + + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(404) + end.not_to change { project.project_group_links.count } + end end - it 'adds group to project' do - expect { subject.execute(group) }.to change { project.project_group_links.count }.from(0).to(1) + shared_examples_for 'shareable' do + it 'adds group to project' do + expect do + result = subject.execute + + expect(result[:status]).to eq(:success) + end.to change { project.project_group_links.count }.from(0).to(1) + end end - it 'updates authorization', :sidekiq_inline do - expect { subject.execute(group) }.to( - change { Ability.allowed?(user, :read_project, project) } - .from(false).to(true)) - end - - it 'returns false if group is blank' do - expect { subject.execute(nil) }.not_to change { project.project_group_links.count } - end - - it 'returns error if user is not allowed to share with a group' do - expect { subject.execute(create(:group)) }.not_to change { project.project_group_links.count } - end - - context 'with specialized project_authorization workers' do - let_it_be(:other_user) { create(:user) } - + context 'when user has proper membership to share a group' do before do - group.add_developer(other_user) + group.add_guest(user) end - it 'schedules authorization update for users with access to group' do - expect(AuthorizedProjectsWorker).not_to( - receive(:bulk_perform_async) - ) - expect(AuthorizedProjectUpdate::ProjectRecalculateWorker).to( - receive(:perform_async) - .with(project.id) - .and_call_original - ) - expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to( - receive(:bulk_perform_in) - .with(1.hour, - array_including([user.id], [other_user.id]), - batch_delay: 30.seconds, batch_size: 100) - .and_call_original - ) + it_behaves_like 'shareable' - subject.execute(group) + it 'updates authorization', :sidekiq_inline do + expect { subject.execute }.to( + change { Ability.allowed?(user, :read_project, project) } + .from(false).to(true)) end + + context 'with specialized project_authorization workers' do + let_it_be(:other_user) { create(:user) } + + before do + group.add_developer(other_user) + end + + it 'schedules authorization update for users with access to group' do + expect(AuthorizedProjectsWorker).not_to( + receive(:bulk_perform_async) + ) + expect(AuthorizedProjectUpdate::ProjectRecalculateWorker).to( + receive(:perform_async) + .with(project.id) + .and_call_original + ) + expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to( + receive(:bulk_perform_in) + .with(1.hour, + array_including([user.id], [other_user.id]), + batch_delay: 30.seconds, batch_size: 100) + .and_call_original + ) + + subject.execute + end + end + + context 'when sharing outside the hierarchy is disabled' do + let_it_be(:shared_group_parent) do + create(:group, + namespace_settings: create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: true)) + end + + let_it_be(:project, reload: true) { create(:project, group: shared_group_parent) } + + it_behaves_like 'not shareable' + + context 'when group is inside hierarchy' do + let(:group) { create(:group, :private, parent: shared_group_parent) } + + it_behaves_like 'shareable' + end + end + end + + context 'when user does not have permissions for the group' do + it_behaves_like 'not shareable' + end + + context 'when group is blank' do + let(:group) { nil } + + it_behaves_like 'not shareable' end end diff --git a/spec/support/shared_examples/services/jira/requests/base_shared_examples.rb b/spec/support/shared_examples/services/jira/requests/base_shared_examples.rb index c4f6273b46c..ce611601f19 100644 --- a/spec/support/shared_examples/services/jira/requests/base_shared_examples.rb +++ b/spec/support/shared_examples/services/jira/requests/base_shared_examples.rb @@ -66,11 +66,11 @@ RSpec.shared_examples 'a service that handles Jira API errors' do it 'logs the error' do stub_client_and_raise(Timeout::Error, 'foo') - expect(Gitlab::ProjectServiceLogger).to receive(:error).with( + expect(Gitlab::IntegrationsLogger).to receive(:error).with( hash_including( client_url: be_present, message: 'Error sending message', - service_class: described_class.name, + integration_class: described_class.name, error: hash_including( exception_class: Timeout::Error.name, exception_message: 'foo',