diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-07-28 21:09:03 +0300 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-07-28 21:09:03 +0300 |
| commit | b420660ef1369fec4d09b7bf5e961d81980974e5 (patch) | |
| tree | f096fdd1bd3b4898e1b2ca80957ce68c200c09f0 | |
| parent | b8026fd558e7ec154c626208a33c1485aec8f4ea (diff) | |
Add latest changes from gitlab-org/gitlab@master
113 files changed, 1928 insertions, 371 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 56913441b25..d3bc7625bf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,29 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 15.2.1 (2022-07-28) + +### Security (18 changes) + +- [Security datadog integration leaking](gitlab-org/security/gitlab@49ec4f1a982ba1798461fad8a0f053b21c8ce8bf) ([merge request](gitlab-org/security/gitlab!2643)) +- [Prevent users who cannot admin a public project from viewing deploy keys](gitlab-org/security/gitlab@1ff5d27ad0574fd5304114ddcc2f0e312d6bd29c) ([merge request](gitlab-org/security/gitlab!2640)) +- [Add additional condition to accept invitation](gitlab-org/security/gitlab@90ad2f07ff08c1da02600af0c2cfe3fdd20a6856) ([merge request](gitlab-org/security/gitlab!2656)) +- [Update GITLAB_PAGES_VERSION](gitlab-org/security/gitlab@bf54d6fa66c4981d75410591e8370c721f2f68e5) ([merge request](gitlab-org/security/gitlab!2615)) +- [Add html_escape to build_details_entity](gitlab-org/security/gitlab@9cfafde666f0f33fb110d585652ea0db4afee340) ([merge request](gitlab-org/security/gitlab!2636)) +- [Check permissions when filtering by contact or organization](gitlab-org/security/gitlab@bf32322d55bf148901b45aa4ae3a7daecdd4ed24) ([merge request](gitlab-org/security/gitlab!2644)) +- [Use author to run subscribed pipeline](gitlab-org/security/gitlab@36addfe325af0780cff649ad43a9cd18d22367e3) ([merge request](gitlab-org/security/gitlab!2616)) +- [Remove prohibited branches after project import](gitlab-org/security/gitlab@96f8f0a30b8bce1c51c3e39808baf74ba6504b33) ([merge request](gitlab-org/security/gitlab!2590)) +- [Remove feature flag `ci_yaml_limit_size`](gitlab-org/security/gitlab@fe4b00b9ce8db49b12a7c59c9a8bd2260cbd8f53) ([merge request](gitlab-org/security/gitlab!2602)) +- [Maintainer can change the visibility of Project and Group](gitlab-org/security/gitlab@91d953642a41305c2a8907ac252af370a837c5ab) ([merge request](gitlab-org/security/gitlab!2619)) +- [Do not link unverified secondary emails with any users](gitlab-org/security/gitlab@84e5ba9eb2c7bbc97d6527333bb8142cbe481304) ([merge request](gitlab-org/security/gitlab!2651)) +- [Forbid exchanging access token for ROP flow to users required 2FA setup](gitlab-org/security/gitlab@979f5c2c2b4421e8a8c002a4fffb59f4df80967b) ([merge request](gitlab-org/security/gitlab!2622)) +- [Remove todos from confidential notes when user loses access](gitlab-org/security/gitlab@fa1d6002710610f6d59f6cdb3548fdde700121f2) ([merge request](gitlab-org/security/gitlab!2632)) +- [Remove group_bot_user and group_access_token after group delete](gitlab-org/security/gitlab@5b27afb5b25e102799df73d314035e059a116b91) ([merge request](gitlab-org/security/gitlab!2633)) +- [Protect Grafana and Sentry integrations](gitlab-org/security/gitlab@73fb74cd4fd96178c0ed89a9b3286e145e6c44fc) ([merge request](gitlab-org/security/gitlab!2639)) +- [Protect integration secrets](gitlab-org/security/gitlab@66f9732bead5e561c868c3e258431235fa189d62) ([merge request](gitlab-org/security/gitlab!2638)) +- [Fix IDOR in Jira issue show action](gitlab-org/security/gitlab@7a65af5f948724784054d126ab1749c3595f31c6) ([merge request](gitlab-org/security/gitlab!2647)) +- [Limit proxied requests to Grafana API](gitlab-org/security/gitlab@cf9a43d06a3daaac1dcb53805d5fbcda45e96c70) ([merge request](gitlab-org/security/gitlab!2606)) + ## 15.2.0 (2022-07-21) ### Added (171 changes) @@ -680,6 +703,29 @@ entry. - [Update GitLab Runner Helm Chart to 0.42.0](gitlab-org/gitlab@cc89200f498fe216864914c79b5b0d1d578edab3) ([merge request](gitlab-org/gitlab!90605)) - [Address database documentation Vale warningss](gitlab-org/gitlab@e5f9a089766bace046d3bbd760a2979865a4bbc0) by @cgives ([merge request](gitlab-org/gitlab!90093)) +## 15.1.4 (2022-07-28) + +### Security (18 changes) + +- [Security datadog integration leaking](gitlab-org/security/gitlab@1aadbf61796ff95b4716fb8ef335c5a4dbdf8c6a) ([merge request](gitlab-org/security/gitlab!2594)) +- [Prevent users who cannot admin a public project from viewing deploy keys](gitlab-org/security/gitlab@a69bec0e58f4ee0df0774dcde1ce1817d41daa7c) ([merge request](gitlab-org/security/gitlab!2641)) +- [Add additional condition to accept invitation](gitlab-org/security/gitlab@b3480d20512d61b761da2a8772ec2bfe32182169) ([merge request](gitlab-org/security/gitlab!2655)) +- [Update GITLAB_PAGES_VERSION](gitlab-org/security/gitlab@6161c0445e4908cdc152bee4be8b9d223df9f1d0) ([merge request](gitlab-org/security/gitlab!2584)) +- [Add html_escape to build_details_entity](gitlab-org/security/gitlab@c0a82385320e144b55e400eff96f39aa56af33ef) ([merge request](gitlab-org/security/gitlab!2610)) +- [Check permissions when filtering by contact or organization](gitlab-org/security/gitlab@5b8f2da656ec7839614d4a49f35778ed00b31b2b) ([merge request](gitlab-org/security/gitlab!2645)) +- [Use author to run subscribed pipeline](gitlab-org/security/gitlab@982917161408cfe4c3191f352bb1d390dfa3c9fd) ([merge request](gitlab-org/security/gitlab!2558)) +- [Remove prohibited branches after project import](gitlab-org/security/gitlab@48864bf7539176eb385f5ce792c8e369a2b79fe6) ([merge request](gitlab-org/security/gitlab!2589)) +- [Remove feature flag `ci_yaml_limit_size`](gitlab-org/security/gitlab@b532b82ef2a0ef5b045932a56ffc06281df7a1d8) ([merge request](gitlab-org/security/gitlab!2630)) +- [Maintainer can change the visibility of Project and Group](gitlab-org/security/gitlab@9556769a3a2fe2c030eb55e87b1465638d315358) ([merge request](gitlab-org/security/gitlab!2618)) +- [Do not link unverified secondary emails with any users](gitlab-org/security/gitlab@331b1d6b590b01c3d45332a6c8e09def249c627e) ([merge request](gitlab-org/security/gitlab!2627)) +- [Forbid exchanging access token for ROP flow to users required 2FA setup](gitlab-org/security/gitlab@15bc29c4ab878365356e59c0c8d2e0f361b71e70) ([merge request](gitlab-org/security/gitlab!2621)) +- [Remove todos from confidential notes when user loses access](gitlab-org/security/gitlab@9672296987190cc4b08b1f32f3c45086880b06bc) ([merge request](gitlab-org/security/gitlab!2608)) +- [Remove group_bot_user and group_access_token after group delete](gitlab-org/security/gitlab@9b1a5e7101e1addee8c3429f4937980020c5f6b3) ([merge request](gitlab-org/security/gitlab!2634)) +- [Protect integration secrets](gitlab-org/security/gitlab@4920ac9b10e237cca5d279cbd3cf60daa829a71b) ([merge request](gitlab-org/security/gitlab!2585)) +- [Protect Grafana and Sentry integrations](gitlab-org/security/gitlab@a5d33edb1c557e9bbd0a1a79c81e851c1d740e37) ([merge request](gitlab-org/security/gitlab!2576)) +- [Fix IDOR in Jira issue show action](gitlab-org/security/gitlab@8b62e0c09c33c44f508266d7b8db4892105124ac) ([merge request](gitlab-org/security/gitlab!2648)) +- [Limit proxied requests to Grafana API](gitlab-org/security/gitlab@4580e61bc22b04eda13e293c5868a26a1a6ee571) ([merge request](gitlab-org/security/gitlab!2597)) + ## 15.1.3 (2022-07-19) ### Added (1 change) @@ -1689,6 +1735,29 @@ entry. - [Fix JH skipped subscription portal spec](gitlab-org/gitlab@0e7e7cb4a62d004989c47fafe6fe1f9ffd90da44) by @chaomao ([merge request](gitlab-org/gitlab!87213)) - [Add not null constraint to requirements.issue_id validate:false](gitlab-org/gitlab@5ccac890b13c53c5761ccb8e5cb7ca202e0656c3) ([merge request](gitlab-org/gitlab!86590)) +## 15.0.5 (2022-07-28) + +### Security (18 changes) + +- [Security datadog integration leaking](gitlab-org/security/gitlab@827505e96860979709654210525007901abd56f0) ([merge request](gitlab-org/security/gitlab!2595)) +- [Prevent users who cannot admin a public project from viewing deploy keys](gitlab-org/security/gitlab@2b06195d21bbd6b566221c49e871cd8da1f744b6) ([merge request](gitlab-org/security/gitlab!2642)) +- [Add additional condition to accept invitation](gitlab-org/security/gitlab@8aa70bcc8a3eb1bd2f96c06e9b0e5b6eb0ac561d) ([merge request](gitlab-org/security/gitlab!2654)) +- [Update GITLAB_PAGES_VERSION](gitlab-org/security/gitlab@6b81c541a60da6c389788d21a216dab4d7e40304) ([merge request](gitlab-org/security/gitlab!2583)) +- [Add html_escape to build_details_entity](gitlab-org/security/gitlab@efe16cfa36ff42981891994d5b5e63dcdd2f1daa) ([merge request](gitlab-org/security/gitlab!2611)) +- [Check permissions when filtering by contact or organization](gitlab-org/security/gitlab@3d210e31d933cfff0c9fae9dd2f1ed97058bfea6) ([merge request](gitlab-org/security/gitlab!2646)) +- [Use author to run subscribed pipeline](gitlab-org/security/gitlab@c1ca513e600d1a963e23ab63c261c982780593b5) ([merge request](gitlab-org/security/gitlab!2559)) +- [Remove prohibited branches after project import](gitlab-org/security/gitlab@8ceb9492555471041f962d54e190314aac86207c) ([merge request](gitlab-org/security/gitlab!2588)) +- [Remove feature flag `ci_yaml_limit_size`](gitlab-org/security/gitlab@df0d379902f0e03a6f1506276402246cfe2e922f) ([merge request](gitlab-org/security/gitlab!2631)) +- [Maintainer can change the visibility of Project and Group](gitlab-org/security/gitlab@1e575f57368d7374dc33e151e81a2f5dfe3fa21e) ([merge request](gitlab-org/security/gitlab!2617)) +- [Do not link unverified secondary emails with any users](gitlab-org/security/gitlab@e15f72a77f67e946258576d1b3006e9471d5ec9a) ([merge request](gitlab-org/security/gitlab!2626)) +- [Forbid exchanging access token for ROP flow to users required 2FA setup](gitlab-org/security/gitlab@7250fb15bf59401acd0c88c89d27423578b24f71) ([merge request](gitlab-org/security/gitlab!2620)) +- [Remove todos from confidential notes when user loses access](gitlab-org/security/gitlab@f91cc66eaa83c9a0744d8f25ee56197b5805035e) ([merge request](gitlab-org/security/gitlab!2609)) +- [Remove group_bot_user and group_access_token after group delete](gitlab-org/security/gitlab@995de100fcba61c0efa11c1a2a98377c90f68f0c) ([merge request](gitlab-org/security/gitlab!2635)) +- [Protect integration secrets](gitlab-org/security/gitlab@e475ad82568f5d9da62d8a89c0ee3393a97685bf) ([merge request](gitlab-org/security/gitlab!2586)) +- [Protect Grafana and Sentry integrations](gitlab-org/security/gitlab@0c3fa9f84e2459e63ec58d6253752649489d6283) ([merge request](gitlab-org/security/gitlab!2577)) +- [Fix IDOR in Jira issue show action](gitlab-org/security/gitlab@cf6ad0474584f701e3f14262284ab681f6a06a42) ([merge request](gitlab-org/security/gitlab!2649)) +- [Limit proxied requests to Grafana API](gitlab-org/security/gitlab@28225721c1bce851d81b6e462ec6965570c7320a) ([merge request](gitlab-org/security/gitlab!2598)) + ## 15.0.4 (2022-06-30) ### Security (17 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index c5e5b0e121f..7cc7ed09682 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -4da75e5814680fe0d657bb734099527c74b76905 +cef54e67714844049ec017b38cd6be2fd3db74b8 diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index 91951fd8ad7..30e298c7494 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -1.61.0 +1.61.1 diff --git a/app/assets/javascripts/pipelines/components/pipeline_tabs.vue b/app/assets/javascripts/pipelines/components/pipeline_tabs.vue index e1745969649..df59962569e 100644 --- a/app/assets/javascripts/pipelines/components/pipeline_tabs.vue +++ b/app/assets/javascripts/pipelines/components/pipeline_tabs.vue @@ -34,7 +34,13 @@ export default { PipelineGraphWrapper, TestReports, }, - inject: ['defaultTabValue', 'failedJobsCount', 'failedJobsSummary', 'totalJobCount'], + inject: [ + 'defaultTabValue', + 'failedJobsCount', + 'failedJobsSummary', + 'totalJobCount', + 'testsCount', + ], computed: { showFailedJobsTab() { return this.failedJobsCount > 0; @@ -81,11 +87,11 @@ export default { </template> <failed-jobs-app :failed-jobs-summary="failedJobsSummary" /> </gl-tab> - <gl-tab - :title="$options.i18n.tabs.testsTitle" - :active="isActive($options.tabNames.tests)" - data-testid="tests-tab" - > + <gl-tab :active="isActive($options.tabNames.tests)" data-testid="tests-tab" lazy> + <template #title> + <span class="gl-mr-2">{{ $options.i18n.tabs.testsTitle }}</span> + <gl-badge size="sm" data-testid="tests-counter">{{ testsCount }}</gl-badge> + </template> <test-reports /> </gl-tab> <slot></slot> diff --git a/app/assets/javascripts/pipelines/components/test_reports/test_reports.vue b/app/assets/javascripts/pipelines/components/test_reports/test_reports.vue index 13e942e55d3..e5666f7a658 100644 --- a/app/assets/javascripts/pipelines/components/test_reports/test_reports.vue +++ b/app/assets/javascripts/pipelines/components/test_reports/test_reports.vue @@ -1,6 +1,7 @@ <script> import { GlLoadingIcon } from '@gitlab/ui'; import { mapActions, mapGetters, mapState } from 'vuex'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import createTestReportsStore from '../../stores/test_reports'; import EmptyState from './empty_state.vue'; import TestSuiteTable from './test_suite_table.vue'; @@ -16,6 +17,7 @@ export default { TestSummary, TestSummaryTable, }, + mixins: [glFeatureFlagMixin()], inject: ['blobPath', 'summaryEndpoint', 'suiteEndpoint'], computed: { ...mapState('testReports', ['isLoading', 'selectedSuiteIndex', 'testReports']), @@ -29,14 +31,16 @@ export default { }, }, created() { - this.$store.registerModule( - 'testReports', - createTestReportsStore({ - blobPath: this.blobPath, - summaryEndpoint: this.summaryEndpoint, - suiteEndpoint: this.suiteEndpoint, - }), - ); + if (!this.glFeatures.pipelineTabsVue) { + this.$store.registerModule( + 'testReports', + createTestReportsStore({ + blobPath: this.blobPath, + summaryEndpoint: this.summaryEndpoint, + suiteEndpoint: this.suiteEndpoint, + }), + ); + } this.fetchSummary(); }, diff --git a/app/assets/javascripts/pipelines/pipeline_tabs.js b/app/assets/javascripts/pipelines/pipeline_tabs.js index 8f2fc9b189e..7051d356089 100644 --- a/app/assets/javascripts/pipelines/pipeline_tabs.js +++ b/app/assets/javascripts/pipelines/pipeline_tabs.js @@ -5,6 +5,7 @@ import PipelineTabs from 'ee_else_ce/pipelines/components/pipeline_tabs.vue'; import { removeParams, updateHistory } from '~/lib/utils/url_utility'; import { TAB_QUERY_PARAM } from '~/pipelines/constants'; import { parseBoolean } from '~/lib/utils/common_utils'; +import createTestReportsStore from './stores/test_reports'; import { getPipelineDefaultTab, reportToSentry } from './utils'; Vue.use(VueApollo); @@ -33,6 +34,13 @@ export const createAppOptions = (selector, apolloProvider) => { licenseManagementSettingsPath, licensesApiPath, canManageLicenses, + summaryEndpoint, + suiteEndpoint, + blobPath, + hasTestReport, + emptyStateImagePath, + artifactsExpiredImagePath, + testsCount, } = dataset; const defaultTabValue = getPipelineDefaultTab(window.location.href); @@ -43,7 +51,15 @@ export const createAppOptions = (selector, apolloProvider) => { PipelineTabs, }, apolloProvider, - store: new Vuex.Store(), + store: new Vuex.Store({ + modules: { + testReports: createTestReportsStore({ + blobPath, + summaryEndpoint, + suiteEndpoint, + }), + }, + }), provide: { canGenerateCodequalityReports: parseBoolean(canGenerateCodequalityReports), codequalityReportDownloadPath, @@ -62,6 +78,13 @@ export const createAppOptions = (selector, apolloProvider) => { licenseManagementSettingsPath, licensesApiPath, canManageLicenses: parseBoolean(canManageLicenses), + summaryEndpoint, + suiteEndpoint, + blobPath, + hasTestReport, + emptyStateImagePath, + artifactsExpiredImagePath, + testsCount, }, errorCaptured(err, _vm, info) { reportToSentry('pipeline_tabs', `error: ${err}, info: ${info}`); diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index 32d1ddf920e..6d1ffc1f2e8 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -5,6 +5,7 @@ class AutocompleteController < ApplicationController skip_before_action :authenticate_user!, only: [:users, :award_emojis, :merge_request_target_branches] before_action :check_search_rate_limit!, only: [:users, :projects] + before_action :authorize_admin_project, only: :deploy_keys_with_owners feature_category :users, [:users, :user] feature_category :projects, [:projects] @@ -69,6 +70,10 @@ class AutocompleteController < ApplicationController private + def authorize_admin_project + render_403 unless Ability.allowed?(current_user, :admin_project, project) + end + def project @project ||= Autocomplete::ProjectFinder .new(current_user, params) diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 8ecf0c158e0..47b2a460e6f 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -483,10 +483,14 @@ class IssuableFinder end def by_crm_contact(items) + return items unless can_filter_by_crm_contact? + Issuables::CrmContactFilter.new(params: original_params).filter(items) end def by_crm_organization(items) + return items unless can_filter_by_crm_organization? + Issuables::CrmOrganizationFilter.new(params: original_params).filter(items) end @@ -499,4 +503,20 @@ class IssuableFinder def feature_flag_scope params.group || params.project end + + def can_filter_by_crm_contact? + current_user&.can?(:read_crm_contact, root_group) + end + + def can_filter_by_crm_organization? + current_user&.can?(:read_crm_organization, root_group) + end + + def root_group + strong_memoize(:root_group) do + base_group = params.group || params.project&.group + + base_group&.root_ancestor + end + end end diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb index b399f0490ee..c0e063a34d5 100644 --- a/app/graphql/gitlab_schema.rb +++ b/app/graphql/gitlab_schema.rb @@ -53,6 +53,7 @@ class GitlabSchema < GraphQL::Schema def get_type(type_name, context = GraphQL::Query::NullContext) type_name = Gitlab::GlobalId::Deprecations.apply_to_graphql_name(type_name) + type_name = Gitlab::Graphql::TypeNameDeprecations.apply_to_graphql_name(type_name) super(type_name, context) end @@ -163,6 +164,7 @@ class GitlabSchema < GraphQL::Schema def get_type(type_name) type_name = Gitlab::GlobalId::Deprecations.apply_to_graphql_name(type_name) + type_name = Gitlab::Graphql::TypeNameDeprecations.apply_to_graphql_name(type_name) super(type_name) end diff --git a/app/graphql/types/global_id_type.rb b/app/graphql/types/global_id_type.rb index 145a5a22460..a71c2fb0e6c 100644 --- a/app/graphql/types/global_id_type.rb +++ b/app/graphql/types/global_id_type.rb @@ -50,7 +50,7 @@ module Types #{ if deprecation = Gitlab::GlobalId::Deprecations.deprecation_by(model_name) 'The older format `"' + - ::Gitlab::GlobalId.build(model_name: deprecation.old_model_name, id: 1).to_s + + ::Gitlab::GlobalId.build(model_name: deprecation.old_name, id: 1).to_s + '"` was deprecated in ' + deprecation.milestone + '.' end} diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 56574062a11..823a0b28a79 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -18,11 +18,31 @@ module MergeRequestsHelper classes.join(' ') end - def merge_path_description(merge_request, separator) + def merge_path_description(merge_request, with_arrow: false) if merge_request.for_fork? - "Project:Branches: #{@merge_request.source_project_path}:#{@merge_request.source_branch} #{separator} #{@merge_request.target_project.full_path}:#{@merge_request.target_branch}" + msg = if with_arrow + _("Project:Branches: %{source_project_path}:%{source_branch} → %{target_project_path}:%{target_branch}") + else + _("Project:Branches: %{source_project_path}:%{source_branch} to %{target_project_path}:%{target_branch}") + end + + msg % { + source_project_path: merge_request.source_project_path, + source_branch: merge_request.source_branch, + target_project_path: merge_request.target_project.full_path, + target_branch: merge_request.target_branch + } else - "Branches: #{@merge_request.source_branch} #{separator} #{@merge_request.target_branch}" + msg = if with_arrow + _("Branches: %{source_branch} → %{target_branch}") + else + _("Branches: %{source_branch} to %{target_branch}") + end + + msg % { + source_branch: merge_request.source_branch, + target_branch: merge_request.target_branch + } end end diff --git a/app/helpers/projects/pipeline_helper.rb b/app/helpers/projects/pipeline_helper.rb index 3b3fe13e58a..5f2a9f7bf21 100644 --- a/app/helpers/projects/pipeline_helper.rb +++ b/app/helpers/projects/pipeline_helper.rb @@ -14,7 +14,14 @@ module Projects metrics_path: namespace_project_ci_prometheus_metrics_histograms_path(namespace_id: project.namespace, project_id: project, format: :json), pipeline_iid: pipeline.iid, pipeline_project_path: project.full_path, - total_job_count: pipeline.total_size + total_job_count: pipeline.total_size, + summary_endpoint: summary_project_pipeline_tests_path(project, pipeline, format: :json), + suite_endpoint: project_pipeline_test_path(project, pipeline, suite_name: 'suite', format: :json), + blob_path: project_blob_path(project, pipeline.sha), + has_test_report: pipeline.has_test_reports?, + empty_state_image_path: image_path('illustrations/empty-state/empty-test-cases-lg.svg'), + artifacts_expired_image_path: image_path('illustrations/pipeline.svg'), + tests_count: pipeline.test_report_summary.total[:count] } end end diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb index 30382a1c205..4953f24755c 100644 --- a/app/models/error_tracking/project_error_tracking_setting.rb +++ b/app/models/error_tracking/project_error_tracking_setting.rb @@ -44,6 +44,8 @@ module ErrorTracking key: Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm' + before_validation :reset_token + after_save :clear_reactive_cache! # When a user enables the integrated error tracking @@ -182,6 +184,12 @@ module ErrorTracking private + def reset_token + if api_url_changed? && !encrypted_token_changed? + self.token = nil + end + end + 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 diff --git a/app/models/grafana_integration.rb b/app/models/grafana_integration.rb index 00213732fee..0358e37c58b 100644 --- a/app/models/grafana_integration.rb +++ b/app/models/grafana_integration.rb @@ -18,6 +18,8 @@ class GrafanaIntegration < ApplicationRecord validates :enabled, inclusion: { in: [true, false] } + before_validation :reset_token + scope :enabled, -> { where(enabled: true) } def client @@ -36,6 +38,12 @@ class GrafanaIntegration < ApplicationRecord private + def reset_token + if grafana_url_changed? && !encrypted_token_changed? + self.token = nil + end + end + def token decrypt(:token, encrypted_token) end diff --git a/app/models/hooks/web_hook_log.rb b/app/models/hooks/web_hook_log.rb index 2f03b3591cf..24e5f193a32 100644 --- a/app/models/hooks/web_hook_log.rb +++ b/app/models/hooks/web_hook_log.rb @@ -22,6 +22,7 @@ class WebHookLog < ApplicationRecord validates :web_hook, presence: true before_save :obfuscate_basic_auth + before_save :redact_author_email def self.recent where('created_at >= ?', 2.days.ago.beginning_of_day) @@ -52,4 +53,10 @@ class WebHookLog < ApplicationRecord def obfuscate_basic_auth self.url = safe_url end + + def redact_author_email + return unless self.request_data.dig('commit', 'author', 'email').present? + + self.request_data['commit']['author']['email'] = _('[REDACTED]') + end end diff --git a/app/models/integrations/campfire.rb b/app/models/integrations/campfire.rb index bf1358ac0f6..3f7fa1c51b2 100644 --- a/app/models/integrations/campfire.rb +++ b/app/models/integrations/campfire.rb @@ -2,7 +2,15 @@ module Integrations class Campfire < Integration + SUBDOMAIN_REGEXP = %r{\A[a-z](?:[a-z0-9-]*[a-z0-9])?\z}i.freeze + validates :token, presence: true, if: :activated? + validates :room, + allow_blank: true, + numericality: { only_integer: true, greater_than: 0 } + validates :subdomain, + allow_blank: true, + format: { with: SUBDOMAIN_REGEXP }, length: { in: 1..63 } field :token, type: 'password', @@ -16,6 +24,7 @@ module Integrations field :subdomain, title: -> { _('Campfire subdomain (optional)') }, placeholder: '', + exposes_secrets: true, help: -> do ERB::Util.html_escape( s_('CampfireService|The %{code_open}.campfirenow.com%{code_close} subdomain.') diff --git a/app/models/integrations/drone_ci.rb b/app/models/integrations/drone_ci.rb index b1f72b7144e..de69afeba6a 100644 --- a/app/models/integrations/drone_ci.rb +++ b/app/models/integrations/drone_ci.rb @@ -13,6 +13,7 @@ module Integrations field :drone_url, title: -> { s_('ProjectService|Drone server URL') }, placeholder: 'http://drone.example.com', + exposes_secrets: true, required: true field :token, diff --git a/app/models/integrations/jira.rb b/app/models/integrations/jira.rb index c9c9b9d59d6..566bbc456f8 100644 --- a/app/models/integrations/jira.rb +++ b/app/models/integrations/jira.rb @@ -223,7 +223,9 @@ module Integrations # support any events. end - def find_issue(issue_key, rendered_fields: false, transitions: false) + def find_issue(issue_key, rendered_fields: false, transitions: false, restrict_project_key: false) + return if restrict_project_key && parse_project_from_issue_key(issue_key) != project_key + expands = [] expands << 'renderedFields' if rendered_fields expands << 'transitions' if transitions @@ -321,6 +323,10 @@ module Integrations private + def parse_project_from_issue_key(issue_key) + issue_key.gsub(Gitlab::Regex.jira_issue_key_project_key_extraction_regex, '') + end + def branch_name(commit) commit.first_ref_by_oid(project.repository) end diff --git a/app/models/integrations/packagist.rb b/app/models/integrations/packagist.rb index 05ee919892d..502e509f7b9 100644 --- a/app/models/integrations/packagist.rb +++ b/app/models/integrations/packagist.rb @@ -6,14 +6,14 @@ module Integrations extend Gitlab::Utils::Override field :username, - title: -> { _('Username') }, + title: -> { s_('Username') }, help: -> { s_('Enter your Packagist username.') }, placeholder: '', required: true field :token, type: 'password', - title: -> { _('Token') }, + title: -> { s_('Token') }, help: -> { s_('Enter your Packagist token.') }, non_empty_password_title: -> { s_('ProjectService|Enter new token') }, non_empty_password_help: -> { s_('ProjectService|Leave blank to use your current token.') }, @@ -21,9 +21,11 @@ module Integrations required: true field :server, - title: -> { _('Server (optional)') }, + title: -> { s_('Server (optional)') }, help: -> { s_('Enter your Packagist server. Defaults to https://packagist.org.') }, - placeholder: 'https://packagist.org' + placeholder: 'https://packagist.org', + exposes_secrets: true, + required: false validates :username, presence: true, if: :activated? validates :token, presence: true, if: :activated? diff --git a/app/models/integrations/zentao.rb b/app/models/integrations/zentao.rb index 11db469f7ee..53194089296 100644 --- a/app/models/integrations/zentao.rb +++ b/app/models/integrations/zentao.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Integrations - class Zentao < Integration + class Zentao < BaseIssueTracker include Gitlab::Routing self.field_storage = :data_fields @@ -10,11 +10,13 @@ module Integrations title: -> { s_('ZentaoIntegration|ZenTao Web URL') }, placeholder: 'https://www.zentao.net', help: -> { s_('ZentaoIntegration|Base URL of the ZenTao instance.') }, + exposes_secrets: true, required: true field :api_url, title: -> { s_('ZentaoIntegration|ZenTao API URL (optional)') }, - help: -> { s_('ZentaoIntegration|If different from Web URL.') } + help: -> { s_('ZentaoIntegration|If different from Web URL.') }, + exposes_secrets: true field :api_token, type: 'password', @@ -40,6 +42,17 @@ module Integrations zentao_tracker_data || self.build_zentao_tracker_data end + alias_method :project_url, :url + + def set_default_data + return unless issues_tracker.present? + + return if url + + data_fields.url ||= issues_tracker['url'] + data_fields.api_url ||= issues_tracker['api_url'] + end + def title 'ZenTao' end diff --git a/app/models/project.rb b/app/models/project.rb index 130ab41baa6..127e47695c5 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2126,6 +2126,7 @@ class Project < ApplicationRecord end def after_import + repository.remove_prohibited_branches repository.expire_content_cache wiki.repository.expire_content_cache diff --git a/app/models/repository.rb b/app/models/repository.rb index 0da71d87457..9039bdf1a20 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1168,6 +1168,16 @@ class Repository @cache ||= Gitlab::RepositoryCache.new(self) end + def remove_prohibited_branches + return unless exists? + + prohibited_branches = raw_repository.branch_names.select { |name| name.match(/\A\h{40}\z/) } + + return if prohibited_branches.blank? + + prohibited_branches.each { |name| raw_repository.delete_branch(name) } + end + private # TODO Genericize finder, later split this on finders by Ref or Oid diff --git a/app/models/snippet.rb b/app/models/snippet.rb index c813c5cb5b8..47b23bbd28a 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -71,8 +71,6 @@ class Snippet < ApplicationRecord }, if: :content_changed? - validates :visibility_level, inclusion: { in: Gitlab::VisibilityLevel.values } - after_create :create_statistics # Scopes diff --git a/app/models/todo.rb b/app/models/todo.rb index cff7a93f72f..c698783d750 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -74,6 +74,7 @@ class Todo < ApplicationRecord scope :for_commit, -> (id) { where(commit_id: id) } scope :with_entity_associations, -> { preload(:target, :author, :note, group: :route, project: [:route, { namespace: [:route, :owner] }]) } scope :joins_issue_and_assignees, -> { left_joins(issue: :assignees) } + scope :for_internal_notes, -> { joins(:note).where(note: { confidential: true }) } enum resolved_by_action: { system_done: 0, api_all_done: 1, api_done: 2, mark_all_done: 3, mark_done: 4 }, _prefix: :resolved_by diff --git a/app/models/u2f_registration.rb b/app/models/u2f_registration.rb index 7c01aa7a420..b4b24ea1486 100644 --- a/app/models/u2f_registration.rb +++ b/app/models/u2f_registration.rb @@ -6,21 +6,7 @@ class U2fRegistration < ApplicationRecord belongs_to :user after_create :create_webauthn_registration - after_update :update_webauthn_registration, if: :counter_changed? - - def create_webauthn_registration - converter = Gitlab::Auth::U2fWebauthnConverter.new(self) - WebauthnRegistration.create!(converter.convert) - rescue StandardError => ex - Gitlab::ErrorTracking.track_exception(ex, u2f_registration_id: self.id) - end - - def update_webauthn_registration - # When we update the sign count of this registration - # we need to update the sign count of the corresponding webauthn registration - # as well if it exists already - WebauthnRegistration.find_by_credential_xid(webauthn_credential_xid)&.update_attribute(:counter, counter) - end + after_update :update_webauthn_registration, if: :saved_change_to_counter? def self.register(user, app_id, params, challenges) u2f = U2F::U2F.new(app_id) @@ -60,10 +46,22 @@ class U2fRegistration < ApplicationRecord private + def create_webauthn_registration + converter = Gitlab::Auth::U2fWebauthnConverter.new(self) + WebauthnRegistration.create!(converter.convert) + rescue StandardError => ex + Gitlab::ErrorTracking.track_exception(ex, u2f_registration_id: self.id) + end + + def update_webauthn_registration + # When we update the sign count of this registration + # we need to update the sign count of the corresponding webauthn registration + # as well if it exists already + WebauthnRegistration.find_by_credential_xid(webauthn_credential_xid) + &.update_attribute(:counter, counter) + end + def webauthn_credential_xid - # To find the corresponding webauthn registration, we use that - # the key handle of the u2f reg corresponds to the credential xid of the webauthn reg - # (with some base64 back and forth) Base64.strict_encode64(Base64.urlsafe_decode64(key_handle)) end end diff --git a/app/models/user.rb b/app/models/user.rb index ca27e0aea6b..1bbad2491d9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -605,23 +605,24 @@ class User < ApplicationRecord end end - # Find a User by their primary email or any associated secondary email + # Find a User by their primary email or any associated confirmed secondary email def find_by_any_email(email, confirmed: false) return unless email by_any_email(email, confirmed: confirmed).take end - # Returns a relation containing all the users for the given email addresses + # Returns a relation containing all found users by their primary email + # or any associated confirmed secondary email # # @param emails [String, Array<String>] email addresses to check - # @param confirmed [Boolean] Only return users where the email is confirmed + # @param confirmed [Boolean] Only return users where the primary email is confirmed def by_any_email(emails, confirmed: false) from_users = by_user_email(emails) from_users = from_users.confirmed if confirmed - from_emails = by_emails(emails) - from_emails = from_emails.confirmed.merge(Email.confirmed) if confirmed + from_emails = by_emails(emails).merge(Email.confirmed) + from_emails = from_emails.confirmed if confirmed items = [from_users, from_emails] @@ -752,6 +753,7 @@ class User < ApplicationRecord matched_by_email_user_id = email_table .project(email_table[:user_id]) .where(email_table[:email].eq(email_address)) + .where(email_table[:confirmed_at].not_eq(nil)) .take(1) # at most 1 record as there is a unique constraint where( @@ -1502,7 +1504,7 @@ class User < ApplicationRecord all_emails = [] all_emails << email unless temp_oauth_email? all_emails << private_commit_email if include_private_email - all_emails.concat(emails.map(&:email)) + all_emails.concat(emails.filter_map { |email| email.email if email.confirmed? }) all_emails.uniq end diff --git a/app/policies/ci/runner_policy.rb b/app/policies/ci/runner_policy.rb index 6dfe9cc496b..8a99f4d1a3e 100644 --- a/app/policies/ci/runner_policy.rb +++ b/app/policies/ci/runner_policy.rb @@ -31,3 +31,5 @@ module Ci rule { ~admin & locked }.prevent :assign_runner end end + +Ci::RunnerPolicy.prepend_mod_with('Ci::RunnerPolicy') diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index 5f72259f34a..dc7b5e95361 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -151,7 +151,7 @@ class BuildDetailsEntity < Ci::JobEntity # We do not return the invalid_dependencies for all scenarios see https://gitlab.com/gitlab-org/gitlab/-/issues/287772#note_914406387 punctuation = invalid_dependencies.empty? ? '.' : ': ' _("This job could not start because it could not retrieve the needed artifacts%{punctuation}%{invalid_dependencies}") % - { invalid_dependencies: invalid_dependencies, punctuation: punctuation } + { invalid_dependencies: html_escape(invalid_dependencies), punctuation: punctuation } end def help_message(docs_url) diff --git a/app/services/concerns/update_visibility_level.rb b/app/services/concerns/update_visibility_level.rb index 4cd14a2fb53..debcff0295c 100644 --- a/app/services/concerns/update_visibility_level.rb +++ b/app/services/concerns/update_visibility_level.rb @@ -5,7 +5,7 @@ module UpdateVisibilityLevel def valid_visibility_level_change?(target, new_visibility) return true unless new_visibility - new_visibility_level = Gitlab::VisibilityLevel.level_value(new_visibility) + new_visibility_level = Gitlab::VisibilityLevel.level_value(new_visibility, fallback_value: nil) if new_visibility_level != target.visibility_level_value unless can?(current_user, :change_visibility_level, target) && diff --git a/app/services/grafana/proxy_service.rb b/app/services/grafana/proxy_service.rb index ac4c3cc091c..37272c85638 100644 --- a/app/services/grafana/proxy_service.rb +++ b/app/services/grafana/proxy_service.rb @@ -15,6 +15,10 @@ module Grafana self.reactive_cache_work_type = :external_dependency self.reactive_cache_worker_finder = ->(_id, *args) { from_cache(*args) } + SUPPORTED_DATASOURCE_PATTERN = %r{\A\d+\z}.freeze + + SUPPORTED_PROXY_PATH = Gitlab::Metrics::Dashboard::Stages::GrafanaFormatter::PROXY_PATH + attr_accessor :project, :datasource_id, :proxy_path, :query_params # @param project_id [Integer] Project id for which grafana is configured. @@ -38,6 +42,7 @@ module Grafana end def execute + return cannot_proxy_response unless can_proxy? return cannot_proxy_response unless client with_reactive_cache(*cache_key) { |result| result } @@ -69,6 +74,11 @@ module Grafana private + def can_proxy? + SUPPORTED_PROXY_PATH == proxy_path && + SUPPORTED_DATASOURCE_PATTERN.match?(datasource_id) + end + def client project.grafana_integration&.client end diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index c88c139a22e..bcf3110ca21 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -35,6 +35,8 @@ module Groups user_ids_for_project_authorizations_refresh = obtain_user_ids_for_project_authorizations_refresh + destroy_group_bots + group.destroy if user_ids_for_project_authorizations_refresh.present? @@ -76,6 +78,19 @@ module Groups group.users_ids_of_direct_members end + + # rubocop:disable CodeReuse/ActiveRecord + def destroy_group_bots + bot_ids = group.members_and_requesters.joins(:user).merge(User.project_bot).pluck(:user_id) + current_user_id = current_user.id + + group.run_after_commit do + bot_ids.each do |user_id| + DeleteUserWorker.perform_async(current_user_id, user_id, skip_authorization: true) + end + end + end + # rubocop:enable CodeReuse/ActiveRecord end end diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb index 1fe397d24e7..5b04d2fd3af 100644 --- a/app/services/todos/destroy/entity_leave_service.rb +++ b/app/services/todos/destroy/entity_leave_service.rb @@ -41,11 +41,20 @@ module Todos end def remove_confidential_resource_todos + # Deletes todos for confidential issues Todo .for_target(confidential_issues.select(:id)) .for_type(Issue.name) .for_user(user) .delete_all + + # Deletes todos for internal notes on unauthorized projects + Todo + .for_type(Issue.name) + .for_internal_notes + .for_project(non_authorized_reporter_projects) # Only Reporter+ can read internal notes + .for_user(user) + .delete_all end def remove_project_todos diff --git a/app/views/notify/approved_merge_request_email.text.haml b/app/views/notify/approved_merge_request_email.text.haml index 476da7f9af7..ab79a96c4ed 100644 --- a/app/views/notify/approved_merge_request_email.text.haml +++ b/app/views/notify/approved_merge_request_email.text.haml @@ -2,7 +2,7 @@ Merge request #{@merge_request.to_reference} was approved by #{sanitize_name(@ap Merge request URL: #{project_merge_request_url(@merge_request.target_project, @merge_request)} -= merge_path_description(@merge_request, 'to') += merge_path_description(@merge_request) Author: #{sanitize_name(@merge_request.author_name)} = assignees_label(@merge_request) diff --git a/app/views/notify/closed_merge_request_email.text.haml b/app/views/notify/closed_merge_request_email.text.haml index 942e771261a..c6e38f5fc3d 100644 --- a/app/views/notify/closed_merge_request_email.text.haml +++ b/app/views/notify/closed_merge_request_email.text.haml @@ -2,7 +2,7 @@ Merge request #{@merge_request.to_reference} was closed by #{sanitize_name(@upda Merge request URL: #{project_merge_request_url(@merge_request.target_project, @merge_request)} -= merge_path_description(@merge_request, 'to') += merge_path_description(@merge_request) Author: #{sanitize_name(@merge_request.author_name)} = assignees_label(@merge_request) diff --git a/app/views/notify/merge_request_status_email.text.haml b/app/views/notify/merge_request_status_email.text.haml index f3845b2b910..61c9b130da8 100644 --- a/app/views/notify/merge_request_status_email.text.haml +++ b/app/views/notify/merge_request_status_email.text.haml @@ -2,7 +2,7 @@ = sprintf(s_('Notify|Merge request URL: %{merge_request_url}'), { merge_request_url: project_merge_request_url(@merge_request.target_project, @merge_request) }) -= merge_path_description(@merge_request, 'to') += merge_path_description(@merge_request) = sprintf(s_('Notify|Author: %{author_name}'), { author_name: sanitize_name(@merge_request.author_name) }) = assignees_label(@merge_request) diff --git a/app/views/notify/merge_request_unmergeable_email.html.haml b/app/views/notify/merge_request_unmergeable_email.html.haml index f0a5e5d4367..38c9710f385 100644 --- a/app/views/notify/merge_request_unmergeable_email.html.haml +++ b/app/views/notify/merge_request_unmergeable_email.html.haml @@ -1,7 +1,7 @@ %p = sprintf(s_('Notify|Merge request %{merge_request} can no longer be merged due to conflict.'), { merge_request: merge_request_reference_link(@merge_request) }).html_safe %p - = merge_path_description(@merge_request, 'to') + = merge_path_description(@merge_request) %p = sprintf(s_('Author: %{author_name}'), { author_name: sanitize_name(@merge_request.author_name) }) %p diff --git a/app/views/notify/merge_request_unmergeable_email.text.haml b/app/views/notify/merge_request_unmergeable_email.text.haml index 22d56e73ca8..211e4e379df 100644 --- a/app/views/notify/merge_request_unmergeable_email.text.haml +++ b/app/views/notify/merge_request_unmergeable_email.text.haml @@ -2,7 +2,7 @@ = sprintf(s_('Notify|Merge request URL: %{merge_request_url}'), { merge_request_url: project_merge_request_url(@merge_request.target_project, @merge_request) }) -= merge_path_description(@merge_request, 'to') += merge_path_description(@merge_request) = sprintf(s_('Author: %{author_name}'), { author_name: sanitize_name(@merge_request.author_name) }) = assignees_label(@merge_request) diff --git a/app/views/notify/merge_when_pipeline_succeeds_email.text.haml b/app/views/notify/merge_when_pipeline_succeeds_email.text.haml index 568ca995e04..dbf742a5cbc 100644 --- a/app/views/notify/merge_when_pipeline_succeeds_email.text.haml +++ b/app/views/notify/merge_when_pipeline_succeeds_email.text.haml @@ -2,7 +2,7 @@ Merge request #{@merge_request.to_reference} was scheduled to merge after pipeli Merge request url: #{project_merge_request_url(@merge_request.target_project, @merge_request)} -= merge_path_description(@merge_request, 'to') += merge_path_description(@merge_request) Author: #{sanitize_name(@merge_request.author_name)} = assignees_label(@merge_request) diff --git a/app/views/notify/merged_merge_request_email.html.haml b/app/views/notify/merged_merge_request_email.html.haml index e2e4d6d937f..bf50ad0a9ad 100644 --- a/app/views/notify/merged_merge_request_email.html.haml +++ b/app/views/notify/merged_merge_request_email.html.haml @@ -2,7 +2,7 @@ = sprintf(s_('Notify|Merge request %{merge_request} was merged'), { merge_request: merge_request_reference_link(@merge_request) }).html_safe %p - = merge_path_description(@merge_request, 'to') + = merge_path_description(@merge_request) %div = sprintf(s_('Notify|Author: %{author_name}'), { author_name: sanitize_name(@merge_request.author_name) }) diff --git a/app/views/notify/merged_merge_request_email.text.haml b/app/views/notify/merged_merge_request_email.text.haml index 9b9eb566903..b80e4606f35 100644 --- a/app/views/notify/merged_merge_request_email.text.haml +++ b/app/views/notify/merged_merge_request_email.text.haml @@ -2,7 +2,7 @@ = sprintf(s_('Notify|Merge request URL: %{merge_request_url}'), { merge_request_url: project_merge_request_url(@merge_request.target_project, @merge_request) }) -= merge_path_description(@merge_request, 'to') += merge_path_description(@merge_request) = sprintf(s_('Notify|Author: %{author_name}'), { author_name: sanitize_name(@merge_request.author_name) }) diff --git a/app/views/notify/new_mention_in_merge_request_email.text.erb b/app/views/notify/new_mention_in_merge_request_email.text.erb index 9ba86f17ef6..9a45aaf1148 100644 --- a/app/views/notify/new_mention_in_merge_request_email.text.erb +++ b/app/views/notify/new_mention_in_merge_request_email.text.erb @@ -2,7 +2,7 @@ You have been mentioned in merge request <%= @merge_request.to_reference %> <%= url_for(project_merge_request_url(@merge_request.target_project, @merge_request)) %> -<%= merge_path_description(@merge_request, 'to') %> +<%= merge_path_description(@merge_request) %> Author: <%= sanitize_name(@merge_request.author_name) %> <%= assignees_label(@merge_request) %> <%= reviewers_label(@merge_request) %> diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index 1542d5bba85..3c60235e6d2 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -3,7 +3,7 @@ mr_link: merge_request_reference_link(@merge_request) } .branch - = merge_path_description(@merge_request, 'to') + = merge_path_description(@merge_request) .author Author: #{@merge_request.author_name} .assignee diff --git a/app/views/notify/new_merge_request_email.text.erb b/app/views/notify/new_merge_request_email.text.erb index 09e8ca36225..f2be0b71592 100644 --- a/app/views/notify/new_merge_request_email.text.erb +++ b/app/views/notify/new_merge_request_email.text.erb @@ -2,7 +2,7 @@ mr_link: url_for(project_merge_request_url(@merge_request.target_project, @merge_request)) } %> -<%= merge_path_description(@merge_request, 'to') %> +<%= merge_path_description(@merge_request) %> <%= "#{_('Author')}: #{sanitize_name(@merge_request.author_name)}" %> <%= assignees_label(@merge_request) if @merge_request.assignees.any? %> <%= reviewers_label(@merge_request) if @merge_request.reviewers.any? %> diff --git a/app/views/notify/unapproved_merge_request_email.text.haml b/app/views/notify/unapproved_merge_request_email.text.haml index 4e34b883906..52c65e6f5c6 100644 --- a/app/views/notify/unapproved_merge_request_email.text.haml +++ b/app/views/notify/unapproved_merge_request_email.text.haml @@ -2,7 +2,7 @@ Merge request #{@merge_request.to_reference} was unapproved by #{@unapproved_by. Merge request URL: #{project_merge_request_url(@merge_request.target_project, @merge_request)} -= merge_path_description(@merge_request, 'to') += merge_path_description(@merge_request) Author: #{sanitize_name(@merge_request.author_name)} = assignees_label(@merge_request) diff --git a/config/feature_flags/development/ci_yaml_limit_size.yml b/config/feature_flags/development/ci_yaml_limit_size.yml deleted file mode 100644 index 222dc409c45..00000000000 --- a/config/feature_flags/development/ci_yaml_limit_size.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: ci_yaml_limit_size -introduced_by_url: https://dev.gitlab.org/gitlab/gitlabhq/-/merge_requests/3126 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/29875 -milestone: '12.0' -type: development -group: group::pipeline authoring -default_enabled: true diff --git a/config/gitlab_loose_foreign_keys.yml b/config/gitlab_loose_foreign_keys.yml index 8a9f3f0da43..c5c2d0a61b9 100644 --- a/config/gitlab_loose_foreign_keys.yml +++ b/config/gitlab_loose_foreign_keys.yml @@ -144,6 +144,9 @@ ci_subscriptions_projects: - table: projects column: upstream_project_id on_delete: async_delete + - table: users + column: author_id + on_delete: async_delete ci_triggers: - table: users column: owner_id diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 6ad8b02bfea..761904009bb 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -24,7 +24,11 @@ Doorkeeper.configure do resource_owner_from_credentials do |routes| user = Gitlab::Auth.find_with_user_password(params[:username], params[:password], increment_failed_attempts: true) - user unless user.try(:two_factor_enabled?) + + next unless user + next if user.two_factor_enabled? || Gitlab::Auth::TwoFactorAuthVerifier.new(user).two_factor_authentication_enforced? + + user end # If you want to restrict access to the web interface for adding oauth authorized applications, you need to declare the block below. diff --git a/db/migrate/20220329130330_add_author_to_ci_subscriptions_projects.rb b/db/migrate/20220329130330_add_author_to_ci_subscriptions_projects.rb new file mode 100644 index 00000000000..b1d0ac64d42 --- /dev/null +++ b/db/migrate/20220329130330_add_author_to_ci_subscriptions_projects.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddAuthorToCiSubscriptionsProjects < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + INDEX_NAME = 'index_ci_subscriptions_projects_author_id' + + def up + unless column_exists?(:ci_subscriptions_projects, :author_id) + add_column :ci_subscriptions_projects, :author_id, :bigint + end + + add_concurrent_index :ci_subscriptions_projects, :author_id, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :ci_subscriptions_projects, INDEX_NAME + remove_column :ci_subscriptions_projects, :author_id + end +end diff --git a/db/post_migrate/20220712175029_add_index_with_target_type_to_todos.rb b/db/post_migrate/20220712175029_add_index_with_target_type_to_todos.rb new file mode 100644 index 00000000000..077e8ed4bbe --- /dev/null +++ b/db/post_migrate/20220712175029_add_index_with_target_type_to_todos.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddIndexWithTargetTypeToTodos < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + INDEX_FOR_PROJECTS_NAME = 'index_requirements_project_id_user_id_id_and_target_type' + INDEX_FOR_TARGET_TYPE_NAME = 'index_requirements_user_id_and_target_type' + + def up + add_concurrent_index :todos, [:project_id, :user_id, :id, :target_type], name: INDEX_FOR_PROJECTS_NAME + add_concurrent_index :todos, [:user_id, :target_type], name: INDEX_FOR_TARGET_TYPE_NAME + end + + def down + remove_concurrent_index_by_name :todos, INDEX_FOR_PROJECTS_NAME + remove_concurrent_index_by_name :todos, INDEX_FOR_TARGET_TYPE_NAME + end +end diff --git a/db/post_migrate/20220712181304_remove_deprecated_indexes_from_todos.rb b/db/post_migrate/20220712181304_remove_deprecated_indexes_from_todos.rb new file mode 100644 index 00000000000..4f99caa10a4 --- /dev/null +++ b/db/post_migrate/20220712181304_remove_deprecated_indexes_from_todos.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class RemoveDeprecatedIndexesFromTodos < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + PROJECTS_INDEX = 'index_todos_on_project_id_and_user_id_and_id' + USERS_INDEX = 'index_todos_on_user_id' + + # These indexes are deprecated in favor of two new ones + # added in a previous migration: + # + # * index_requirements_project_id_user_id_target_type_and_id + # * index_requirements_user_id_and_target_type + def up + remove_concurrent_index_by_name :todos, PROJECTS_INDEX + remove_concurrent_index_by_name :todos, USERS_INDEX + end + + def down + add_concurrent_index :todos, [:project_id, :user_id, :id], name: PROJECTS_INDEX + add_concurrent_index :todos, :user_id, name: USERS_INDEX + end +end diff --git a/db/schema_migrations/20220329130330 b/db/schema_migrations/20220329130330 new file mode 100644 index 00000000000..367d43a89a2 --- /dev/null +++ b/db/schema_migrations/20220329130330 @@ -0,0 +1 @@ +8726707f9f4bb8d256886c592b6a11ba8487de24f5340c837800f5ce0c32df9d
\ No newline at end of file diff --git a/db/schema_migrations/20220712175029 b/db/schema_migrations/20220712175029 new file mode 100644 index 00000000000..bb7fdca340f --- /dev/null +++ b/db/schema_migrations/20220712175029 @@ -0,0 +1 @@ +f6638435457f57f5c566e107de4f4557a1d87b5dd27acc9e5345999197d18e6e
\ No newline at end of file diff --git a/db/schema_migrations/20220712181304 b/db/schema_migrations/20220712181304 new file mode 100644 index 00000000000..ff111fe7c28 --- /dev/null +++ b/db/schema_migrations/20220712181304 @@ -0,0 +1 @@ +ff9ad44a43be82867da8e0f51e68a2284065cab6b2eb7cf6496108dce1cdd657
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 10c453d0d0c..5d349c4450b 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13296,7 +13296,8 @@ ALTER SEQUENCE ci_stages_id_seq OWNED BY ci_stages.id; CREATE TABLE ci_subscriptions_projects ( id bigint NOT NULL, downstream_project_id bigint NOT NULL, - upstream_project_id bigint NOT NULL + upstream_project_id bigint NOT NULL, + author_id bigint ); CREATE SEQUENCE ci_subscriptions_projects_id_seq @@ -27831,6 +27832,8 @@ CREATE INDEX index_ci_stages_on_pipeline_id_and_position ON ci_stages USING btre CREATE INDEX index_ci_stages_on_project_id ON ci_stages USING btree (project_id); +CREATE INDEX index_ci_subscriptions_projects_author_id ON ci_subscriptions_projects USING btree (author_id); + CREATE INDEX index_ci_subscriptions_projects_on_upstream_project_id ON ci_subscriptions_projects USING btree (upstream_project_id); CREATE UNIQUE INDEX index_ci_subscriptions_projects_unique_subscription ON ci_subscriptions_projects USING btree (downstream_project_id, upstream_project_id); @@ -29579,6 +29582,10 @@ CREATE INDEX index_requirements_on_title_trigram ON requirements USING gin (titl CREATE INDEX index_requirements_on_updated_at ON requirements USING btree (updated_at); +CREATE INDEX index_requirements_project_id_user_id_id_and_target_type ON todos USING btree (project_id, user_id, id, target_type); + +CREATE INDEX index_requirements_user_id_and_target_type ON todos USING btree (user_id, target_type); + CREATE INDEX index_resource_iteration_events_on_issue_id ON resource_iteration_events USING btree (issue_id); CREATE INDEX index_resource_iteration_events_on_iteration_id ON resource_iteration_events USING btree (iteration_id); @@ -29897,12 +29904,8 @@ CREATE INDEX index_todos_on_note_id ON todos USING btree (note_id); CREATE INDEX index_todos_on_project_id_and_id ON todos USING btree (project_id, id); -CREATE INDEX index_todos_on_project_id_and_user_id_and_id ON todos USING btree (project_id, user_id, id); - CREATE INDEX index_todos_on_target_type_and_target_id ON todos USING btree (target_type, target_id); -CREATE INDEX index_todos_on_user_id ON todos USING btree (user_id); - CREATE INDEX index_todos_on_user_id_and_id_done ON todos USING btree (user_id, id) WHERE ((state)::text = 'done'::text); CREATE INDEX index_todos_on_user_id_and_id_pending ON todos USING btree (user_id, id) WHERE ((state)::text = 'pending'::text); diff --git a/doc/administration/instance_limits.md b/doc/administration/instance_limits.md index 2007f5edb3b..5158e2bbdd9 100644 --- a/doc/administration/instance_limits.md +++ b/doc/administration/instance_limits.md @@ -747,12 +747,6 @@ You can change these limits in the [GitLab Rails console](operations/rails_conso ApplicationSetting.update!(max_yaml_depth: 125) ``` -To disable this limitation entirely, disable the feature flag in the console: - -```ruby -Feature.disable(:ci_yaml_limit_size) -``` - ### Limit dotenv variables > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/321552) in GitLab 14.5. diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index a584df50ce4..eecb8679390 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -716,7 +716,7 @@ To allow clients to continue to interact with the mutation unchanged, edit the ` ```ruby DEPRECATIONS = [ - Deprecation.new(old_model_name: 'PrometheusService', new_model_name: 'Integrations::Prometheus', milestone: '14.0') + Gitlab::Graphql::DeprecationsBase::NameDeprecation.new(old_name: 'PrometheusService', new_name: 'Integrations::Prometheus', milestone: '14.0') ].freeze ``` diff --git a/lib/gitlab/config/loader/yaml.rb b/lib/gitlab/config/loader/yaml.rb index 0559c85647d..7b87b5b8f97 100644 --- a/lib/gitlab/config/loader/yaml.rb +++ b/lib/gitlab/config/loader/yaml.rb @@ -41,8 +41,6 @@ module Gitlab end def too_big? - return false unless Feature.enabled?(:ci_yaml_limit_size) - !deep_size.valid? end diff --git a/lib/gitlab/global_id/deprecations.rb b/lib/gitlab/global_id/deprecations.rb index 2753e2b8372..b68aac8f862 100644 --- a/lib/gitlab/global_id/deprecations.rb +++ b/lib/gitlab/global_id/deprecations.rb @@ -3,46 +3,27 @@ module Gitlab module GlobalId module Deprecations - Deprecation = Struct.new(:old_model_name, :new_model_name, :milestone, keyword_init: true) - # Contains the deprecations in place. # Example: # # DEPRECATIONS = [ - # Deprecation.new(old_model_name: 'PrometheusService', new_model_name: 'Integrations::Prometheus', milestone: '14.1') + # Gitlab::Graphql::DeprecationsBase::NameDeprecation.new(old_name: 'PrometheusService', new_name: 'Integrations::Prometheus', milestone: '14.1') # ].freeze DEPRECATIONS = [ # This works around an accidentally released argument named as `"EEIterationID"` in 7000489db. - Deprecation.new(old_model_name: 'EEIteration', new_model_name: 'Iteration', milestone: '13.3'), - Deprecation.new(old_model_name: 'PrometheusService', new_model_name: 'Integrations::Prometheus', milestone: '14.1') + Gitlab::Graphql::DeprecationsBase::NameDeprecation.new( + old_name: 'EEIteration', new_name: 'Iteration', milestone: '13.3' + ), + Gitlab::Graphql::DeprecationsBase::NameDeprecation.new( + old_name: 'PrometheusService', new_name: 'Integrations::Prometheus', milestone: '14.1' + ) ].freeze - # Maps of the DEPRECATIONS Hash for quick access. - OLD_NAME_MAP = DEPRECATIONS.index_by(&:old_model_name).freeze - NEW_NAME_MAP = DEPRECATIONS.index_by(&:new_model_name).freeze - OLD_GRAPHQL_NAME_MAP = DEPRECATIONS.index_by do |d| - Types::GlobalIDType.model_name_to_graphql_name(d.old_model_name) - end.freeze - - def self.deprecated?(old_model_name) - OLD_NAME_MAP.key?(old_model_name) - end - - def self.deprecation_for(old_model_name) - OLD_NAME_MAP[old_model_name] + def self.map_graphql_name(model_name) + Types::GlobalIDType.model_name_to_graphql_name(model_name) end - def self.deprecation_by(new_model_name) - NEW_NAME_MAP[new_model_name] - end - - # Returns the new `graphql_name` (Type#graphql_name) of a deprecated GID, - # or the `graphql_name` argument given if no deprecation applies. - def self.apply_to_graphql_name(graphql_name) - return graphql_name unless deprecation = OLD_GRAPHQL_NAME_MAP[graphql_name] - - Types::GlobalIDType.model_name_to_graphql_name(deprecation.new_model_name) - end + include Gitlab::Graphql::DeprecationsBase end end end diff --git a/lib/gitlab/graphql/deprecations_base.rb b/lib/gitlab/graphql/deprecations_base.rb new file mode 100644 index 00000000000..2ee14620907 --- /dev/null +++ b/lib/gitlab/graphql/deprecations_base.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + # include DeprecationsBase at the end of the target module + module DeprecationsBase + NameDeprecation = Struct.new(:old_name, :new_name, :milestone, keyword_init: true) + + def self.included(klass) + klass.extend(ClassMethods) + + klass.const_set('OLD_GRAPHQL_NAME_MAP', klass::DEPRECATIONS.index_by do |d| + klass.map_graphql_name(d.old_name) + end.freeze) + klass.const_set('OLD_NAME_MAP', klass::DEPRECATIONS.index_by(&:old_name).freeze) + klass.const_set('NEW_NAME_MAP', klass::DEPRECATIONS.index_by(&:new_name).freeze) + end + + module ClassMethods + def deprecated?(old_name) + self::OLD_NAME_MAP.key?(old_name) + end + + def deprecation_for(old_name) + self::OLD_NAME_MAP[old_name] + end + + def deprecation_by(new_name) + self::NEW_NAME_MAP[new_name] + end + + # Returns the new `graphql_name` (Type#graphql_name) of a deprecated GID, + # or the `graphql_name` argument given if no deprecation applies. + def apply_to_graphql_name(graphql_name) + return graphql_name unless deprecation = self::OLD_GRAPHQL_NAME_MAP[graphql_name] + + self.map_graphql_name(deprecation.new_name) + end + + private + + def map_graphql_name(name) + raise NotImplementedError, "Implement `#{__method__}` in #{self.name}" + end + end + end + end +end diff --git a/lib/gitlab/graphql/type_name_deprecations.rb b/lib/gitlab/graphql/type_name_deprecations.rb new file mode 100644 index 00000000000..4c75a0e7bd3 --- /dev/null +++ b/lib/gitlab/graphql/type_name_deprecations.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module TypeNameDeprecations + # Contains the deprecations in place. + # Example: + # + # DEPRECATIONS = [ + # Gitlab::Graphql::DeprecationsBase::NameDeprecation.new( + # old_name: 'CiRunnerUpgradeStatusType', new_name: 'CiRunnerUpgradeStatus', milestone: '15.3' + # ) + # ].freeze + DEPRECATIONS = [].freeze + + def self.map_graphql_name(name) + name + end + + include Gitlab::Graphql::DeprecationsBase + end + end +end diff --git a/lib/gitlab/patch/global_id.rb b/lib/gitlab/patch/global_id.rb index 145a7bfe842..9b093186aec 100644 --- a/lib/gitlab/patch/global_id.rb +++ b/lib/gitlab/patch/global_id.rb @@ -9,7 +9,7 @@ module Gitlab super if deprecation = Gitlab::GlobalId::Deprecations.deprecation_for(model_name) - @new_model_name = deprecation.new_model_name + @new_model_name = deprecation.new_name end end diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 0534f890152..551750f9798 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -418,6 +418,10 @@ module Gitlab @jira_issue_key_regex ||= /[A-Z][A-Z_0-9]+-\d+/ end + def jira_issue_key_project_key_extraction_regex + @jira_issue_key_project_key_extraction_regex ||= /-\d+/ + end + def jira_transition_id_regex @jira_transition_id_regex ||= /\d+/ end diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb index d378e558b8a..049e3befe64 100644 --- a/lib/gitlab/visibility_level.rb +++ b/lib/gitlab/visibility_level.rb @@ -18,6 +18,8 @@ module Gitlab scope :public_to_user, -> (user = nil) do where(visibility_level: VisibilityLevel.levels_for_user(user)) end + + alias_method :visibility_level=, :visibility= end PRIVATE = 0 unless const_defined?(:PRIVATE) @@ -108,10 +110,10 @@ module Gitlab options.key(level.to_i) || s_('VisibilityLevel|Unknown') end - def level_value(level) + def level_value(level, fallback_value: PRIVATE) return level.to_i if level.to_i.to_s == level.to_s && string_options.key(level.to_i) - string_options[level] || PRIVATE + string_options[level] || fallback_value end def string_level(level) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0f3b5a747e3..193a8fffb77 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6657,6 +6657,12 @@ msgstr "" msgid "Branches" msgstr "" +msgid "Branches: %{source_branch} to %{target_branch}" +msgstr "" + +msgid "Branches: %{source_branch} → %{target_branch}" +msgstr "" + msgid "Branches|Active" msgstr "" @@ -30286,6 +30292,12 @@ msgstr "" msgid "Project: %{name}" msgstr "" +msgid "Project:Branches: %{source_project_path}:%{source_branch} to %{target_project_path}:%{target_branch}" +msgstr "" + +msgid "Project:Branches: %{source_project_path}:%{source_branch} → %{target_project_path}:%{target_branch}" +msgstr "" + msgid "ProjectActivityRSS|Subscribe" msgstr "" diff --git a/scripts/changed-feature-flags b/scripts/changed-feature-flags index 86214e86788..ded6156bfa8 100755 --- a/scripts/changed-feature-flags +++ b/scripts/changed-feature-flags @@ -10,8 +10,8 @@ require_relative 'api/default_options' # Each desired feature flag state is specified as 'feature-flag=state'. This allows us to run package-and-qa with the # feature flag set to the desired state. # -# For example, if the specified files included `config/feature_flags/development/ci_yaml_limit_size.yml` and the desired -# state as specified by the second argument was enabled, the value returned would be `ci_yaml_limit_size=enabled` +# For example, if the specified files included `config/feature_flags/development/ci_awesome_feature.yml` and the desired +# state as specified by the second argument was enabled, the value returned would be `ci_awesome_feature=enabled` class GetFeatureFlagsFromFiles def initialize(options) diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index e874df62cd7..70e58124d21 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -378,63 +378,74 @@ RSpec.describe AutocompleteController do end context 'GET deploy_keys_with_owners' do - let!(:deploy_key) { create(:deploy_key, user: user) } - let!(:deploy_keys_project) { create(:deploy_keys_project, :write_access, project: project, deploy_key: deploy_key) } + let_it_be(:public_project) { create(:project, :public) } + let_it_be(:user) { create(:user) } + let_it_be(:deploy_key) { create(:deploy_key, user: user) } + let_it_be(:deploy_keys_project) do + create(:deploy_keys_project, :write_access, project: public_project, deploy_key: deploy_key) + end context 'unauthorized user' do it 'returns a not found response' do - get(:deploy_keys_with_owners, params: { project_id: project.id }) + get(:deploy_keys_with_owners, params: { project_id: public_project.id }) expect(response).to have_gitlab_http_status(:redirect) end end - context 'when the user who can read the project is logged in' do + context 'when the user is logged in' do before do sign_in(user) end - context 'and they cannot read the project' do + context 'with a non-existing project' do it 'returns a not found response' do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(false) - - get(:deploy_keys_with_owners, params: { project_id: project.id }) + get(:deploy_keys_with_owners, params: { project_id: 9999 }) expect(response).to have_gitlab_http_status(:not_found) end end - it 'renders the deploy key in a json payload, with its owner' do - get(:deploy_keys_with_owners, params: { project_id: project.id }) + context 'with an existing project' do + context 'when user cannot admin project' do + it 'returns a forbidden response' do + get(:deploy_keys_with_owners, params: { project_id: public_project.id }) - expect(json_response.count).to eq(1) - expect(json_response.first['title']).to eq(deploy_key.title) - expect(json_response.first['owner']['id']).to eq(deploy_key.user.id) - expect(json_response.first['deploy_keys_projects']).to be_nil - end + expect(response).to have_gitlab_http_status(:forbidden) + end + end - context 'with an unknown project' do - it 'returns a not found response' do - get(:deploy_keys_with_owners, params: { project_id: 9999 }) + context 'when user can admin project' do + before do + public_project.add_maintainer(user) + end - expect(response).to have_gitlab_http_status(:not_found) - end - end + context 'and user can read owner of key' do + it 'renders the deploy keys in a json payload, with owner' do + get(:deploy_keys_with_owners, params: { project_id: public_project.id }) - context 'and the user cannot read the owner of the key' do - before do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?).with(user, :read_user, deploy_key.user).and_return(false) - end + expect(json_response.count).to eq(1) + expect(json_response.first['title']).to eq(deploy_key.title) + expect(json_response.first['owner']['id']).to eq(deploy_key.user.id) + expect(json_response.first['deploy_keys_projects']).to be_nil + end + end + + context 'and user cannot read owner of key' do + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :read_user, deploy_key.user).and_return(false) + end - it 'returns a payload without owner' do - get(:deploy_keys_with_owners, params: { project_id: project.id }) + it 'returns a payload without owner' do + get(:deploy_keys_with_owners, params: { project_id: public_project.id }) - expect(json_response.count).to eq(1) - expect(json_response.first['title']).to eq(deploy_key.title) - expect(json_response.first['owner']).to be_nil - expect(json_response.first['deploy_keys_projects']).to be_nil + expect(json_response.count).to eq(1) + expect(json_response.first['title']).to eq(deploy_key.title) + expect(json_response.first['owner']).to be_nil + expect(json_response.first['deploy_keys_projects']).to be_nil + end + end end end end diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index c5e693e3489..b3b7753df61 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -143,14 +143,28 @@ RSpec.describe InvitesController do context 'when user exists with the invited email as secondary email' do before do - secondary_email = create(:email, user: user, email: 'foo@example.com') member.update!(invite_email: secondary_email.email) end - it 'is redirected to a new session with invite email param' do - request + context 'when secondary email is confirmed' do + let(:secondary_email) { create(:email, :confirmed, user: user, email: 'foo@example.com') } - expect(response).to redirect_to(new_user_session_path(invite_email: member.invite_email)) + it 'is redirected to a new session with invite email param' do + request + + expect(response).to redirect_to(new_user_session_path(invite_email: member.invite_email)) + end + end + + context 'when secondary email is unconfirmed' do + let(:secondary_email) { create(:email, user: user, email: 'foo@example.com') } + + it 'is redirected to a new registration with invite email param and flash message', :aggregate_failures do + request + + expect(response).to redirect_to(new_user_registration_path(invite_email: member.invite_email)) + expect(flash[:notice]).to eq 'To accept this invitation, create an account or sign in.' + end end end diff --git a/spec/features/projects/pipelines/legacy_pipeline_spec.rb b/spec/features/projects/pipelines/legacy_pipeline_spec.rb index db6feecba03..14f60dfe061 100644 --- a/spec/features/projects/pipelines/legacy_pipeline_spec.rb +++ b/spec/features/projects/pipelines/legacy_pipeline_spec.rb @@ -385,6 +385,37 @@ RSpec.describe 'Pipeline', :js do end end + describe 'test tabs' do + let(:pipeline) { create(:ci_pipeline, :with_test_reports, :with_report_results, project: project) } + + before do + stub_feature_flags(pipeline_tabs_vue: false) + visit_pipeline + wait_for_requests + end + + context 'with test reports' do + it 'shows badge counter in Tests tab' do + expect(page.find('.js-test-report-badge-counter').text).to eq(pipeline.test_report_summary.total[:count].to_s) + end + + it 'calls summary.json endpoint', :js do + find('.js-tests-tab-link').click + + expect(page).to have_content('Jobs') + expect(page).to have_selector('[data-testid="tests-detail"]', visible: :all) + end + end + + context 'without test reports' do + let(:pipeline) { create(:ci_pipeline, project: project) } + + it 'shows zero' do + expect(page.find('.js-test-report-badge-counter', visible: :all).text).to eq("0") + end + end + end + context 'retrying jobs' do before do visit_pipeline diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index e54fda8edda..cfdd851cb80 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -515,18 +515,17 @@ RSpec.describe 'Pipeline', :js do let(:pipeline) { create(:ci_pipeline, :with_test_reports, :with_report_results, project: project) } before do - stub_feature_flags(pipeline_tabs_vue: false) visit_pipeline wait_for_requests end context 'with test reports' do it 'shows badge counter in Tests tab' do - expect(page.find('.js-test-report-badge-counter').text).to eq(pipeline.test_report_summary.total[:count].to_s) + expect(page.find('[data-testid="tests-counter"]').text).to eq(pipeline.test_report_summary.total[:count].to_s) end it 'calls summary.json endpoint', :js do - find('.js-tests-tab-link').click + find('.gl-tab-nav-item', text: 'Tests').click expect(page).to have_content('Jobs') expect(page).to have_selector('[data-testid="tests-detail"]', visible: :all) @@ -537,7 +536,7 @@ RSpec.describe 'Pipeline', :js do let(:pipeline) { create(:ci_pipeline, project: project) } it 'shows zero' do - expect(page.find('.js-test-report-badge-counter', visible: :all).text).to eq("0") + expect(page.find('[data-testid="tests-counter"]', visible: :all).text).to eq("0") end end end diff --git a/spec/features/signed_commits_spec.rb b/spec/features/signed_commits_spec.rb index 610a80eb12c..dbf35567803 100644 --- a/spec/features/signed_commits_spec.rb +++ b/spec/features/signed_commits_spec.rb @@ -61,7 +61,7 @@ RSpec.describe 'GPG signed commits' do let(:user_2) do create(:user, email: GpgHelpers::User2.emails.first, username: 'bette.cartwright', name: 'Bette Cartwright').tap do |user| # secondary, unverified email - create :email, user: user, email: GpgHelpers::User2.emails.last + create :email, user: user, email: 'mail@koffeinfrei.org' end end @@ -83,10 +83,11 @@ RSpec.describe 'GPG signed commits' do end end - it 'unverified signature: user email does not match the committer email, but is the same user' do + it 'unverified signature: gpg key email does not match the committer_email but is the same user when the committer_email belongs to the user as a confirmed secondary email' do user_2_key + user_2.emails.find_by(email: 'mail@koffeinfrei.org').confirm - visit project_commit_path(project, GpgHelpers::DIFFERING_EMAIL_SHA) + visit project_commit_path(project, GpgHelpers::SIGNED_COMMIT_SHA) wait_for_all_requests page.find('.gpg-status-box', text: 'Unverified').click @@ -99,7 +100,7 @@ RSpec.describe 'GPG signed commits' do end end - it 'unverified signature: user email does not match the committer email' do + it 'unverified signature: gpg key email does not match the committer_email when the committer_email belongs to the user as a unconfirmed secondary email' do user_2_key visit project_commit_path(project, GpgHelpers::SIGNED_COMMIT_SHA) diff --git a/spec/frontend/pipelines/components/pipeline_tabs_spec.js b/spec/frontend/pipelines/components/pipeline_tabs_spec.js index e0210307823..3680d9d62c7 100644 --- a/spec/frontend/pipelines/components/pipeline_tabs_spec.js +++ b/spec/frontend/pipelines/components/pipeline_tabs_spec.js @@ -24,12 +24,14 @@ describe('The Pipeline Tabs', () => { const findFailedJobsBadge = () => wrapper.findByTestId('failed-builds-counter'); const findJobsBadge = () => wrapper.findByTestId('builds-counter'); + const findTestsBadge = () => wrapper.findByTestId('tests-counter'); const defaultProvide = { defaultTabValue: '', failedJobsCount: 1, failedJobsSummary: [], totalJobCount: 10, + testsCount: 123, }; const createComponent = (provide = {}) => { @@ -41,7 +43,6 @@ describe('The Pipeline Tabs', () => { }, stubs: { GlTab, - TestReports: { template: '<div id="tests" />' }, }, }), ); @@ -82,6 +83,7 @@ describe('The Pipeline Tabs', () => { tabName | badgeComponent | badgeText ${'Jobs'} | ${findJobsBadge} | ${String(defaultProvide.totalJobCount)} ${'Failed Jobs'} | ${findFailedJobsBadge} | ${String(defaultProvide.failedJobsCount)} + ${'Tests'} | ${findTestsBadge} | ${String(defaultProvide.testsCount)} `('shows badge for $tabName with the correct text', ({ badgeComponent, badgeText }) => { createComponent(); diff --git a/spec/graphql/resolvers/issues_resolver_spec.rb b/spec/graphql/resolvers/issues_resolver_spec.rb index a5b5a8e4f72..89e45810033 100644 --- a/spec/graphql/resolvers/issues_resolver_spec.rb +++ b/spec/graphql/resolvers/issues_resolver_spec.rb @@ -30,6 +30,9 @@ RSpec.describe Resolvers::IssuesResolver do before_all do project.add_developer(current_user) project.add_reporter(reporter) + + create(:crm_settings, group: group, enabled: true) + create(:label_link, label: label1, target: issue1) create(:label_link, label: label1, target: issue2) create(:label_link, label: label2, target: issue2) @@ -399,6 +402,8 @@ RSpec.describe Resolvers::IssuesResolver do let_it_be(:crm_issue3) { create(:issue, project: project) } before_all do + group.add_developer(current_user) + create(:issue_customer_relations_contact, issue: crm_issue1, contact: contact1) create(:issue_customer_relations_contact, issue: crm_issue2, contact: contact2) create(:issue_customer_relations_contact, issue: crm_issue3, contact: contact3) @@ -631,13 +636,13 @@ RSpec.describe Resolvers::IssuesResolver do end it 'finds a specific issue with iid', :request_store do - result = batch_sync(max_queries: 7) { resolve_issues(iid: issue1.iid).to_a } + result = batch_sync(max_queries: 8) { resolve_issues(iid: issue1.iid).to_a } expect(result).to contain_exactly(issue1) end it 'batches queries that only include IIDs', :request_store do - result = batch_sync(max_queries: 7) do + result = batch_sync(max_queries: 8) do [issue1, issue2] .map { |issue| resolve_issues(iid: issue.iid.to_s) } .flat_map(&:to_a) @@ -647,7 +652,7 @@ RSpec.describe Resolvers::IssuesResolver do end it 'finds a specific issue with iids', :request_store do - result = batch_sync(max_queries: 7) do + result = batch_sync(max_queries: 8) do resolve_issues(iids: [issue1.iid]).to_a end diff --git a/spec/graphql/types/global_id_type_spec.rb b/spec/graphql/types/global_id_type_spec.rb index a57db9234f1..fa0b34113bc 100644 --- a/spec/graphql/types/global_id_type_spec.rb +++ b/spec/graphql/types/global_id_type_spec.rb @@ -114,7 +114,11 @@ RSpec.describe Types::GlobalIDType do end before do - deprecation = Gitlab::GlobalId::Deprecations::Deprecation.new(old_model_name: 'OldIssue', new_model_name: 'Issue', milestone: '10.0') + deprecation = Gitlab::GlobalId::Deprecations::NameDeprecation.new( + old_name: 'OldIssue', + new_name: 'Issue', + milestone: '10.0' + ) stub_global_id_deprecations(deprecation) end diff --git a/spec/helpers/merge_requests_helper_spec.rb b/spec/helpers/merge_requests_helper_spec.rb index 97ad55d9df9..6a45b640ead 100644 --- a/spec/helpers/merge_requests_helper_spec.rb +++ b/spec/helpers/merge_requests_helper_spec.rb @@ -27,6 +27,38 @@ RSpec.describe MergeRequestsHelper do end end + describe '#merge_path_description' do + let(:project) { create(:project) } + let(:forked_project) { fork_project(project) } + let(:merge_request_forked) { create(:merge_request, source_project: forked_project, target_project: project) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + + where(:case_name, :mr, :with_arrow, :result) do + [ + ['forked with arrow', ref(:merge_request_forked), true, lazy do + "Project:Branches: #{ + mr.source_project_path}:#{mr.source_branch} → #{ + mr.target_project.full_path}:#{mr.target_branch}" + end], + ['forked default', ref(:merge_request_forked), false, lazy do + "Project:Branches: #{ + mr.source_project_path}:#{mr.source_branch} to #{ + mr.target_project.full_path}:#{mr.target_branch}" + end], + ['with arrow', ref(:merge_request), true, lazy { "Branches: #{mr.source_branch} → #{mr.target_branch}" }], + ['default', ref(:merge_request), false, lazy { "Branches: #{mr.source_branch} to #{mr.target_branch}" }] + ] + end + + with_them do + subject { merge_path_description(mr, with_arrow: with_arrow) } + + it { + is_expected.to eq(result) + } + end + end + describe '#tab_link_for' do let(:merge_request) { create(:merge_request, :simple) } let(:options) { {} } diff --git a/spec/helpers/projects/pipeline_helper_spec.rb b/spec/helpers/projects/pipeline_helper_spec.rb index 2b2dad286c7..8ce4e9f5293 100644 --- a/spec/helpers/projects/pipeline_helper_spec.rb +++ b/spec/helpers/projects/pipeline_helper_spec.rb @@ -27,7 +27,13 @@ RSpec.describe Projects::PipelineHelper do metrics_path: namespace_project_ci_prometheus_metrics_histograms_path(namespace_id: project.namespace, project_id: project, format: :json), pipeline_iid: pipeline.iid, pipeline_project_path: project.full_path, - total_job_count: pipeline.total_size + total_job_count: pipeline.total_size, + summary_endpoint: summary_project_pipeline_tests_path(project, pipeline, format: :json), + suite_endpoint: project_pipeline_test_path(project, pipeline, suite_name: 'suite', format: :json), + blob_path: project_blob_path(project, pipeline.sha), + has_test_report: pipeline.has_reports?(Ci::JobArtifact.test_reports), + empty_state_image_path: match_asset_path('illustrations/empty-state/empty-test-cases-lg.svg'), + artifacts_expired_image_path: match_asset_path('illustrations/pipeline.svg') }) end end diff --git a/spec/initializers/global_id_spec.rb b/spec/initializers/global_id_spec.rb index 4deb1833999..edca4533b3a 100644 --- a/spec/initializers/global_id_spec.rb +++ b/spec/initializers/global_id_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'global_id' do it 'patches GlobalID to find aliased models when a deprecation exists' do allow(Gitlab::GlobalId::Deprecations).to receive(:deprecation_for).and_call_original - allow(Gitlab::GlobalId::Deprecations).to receive(:deprecation_for).with('Issue').and_return(double(new_model_name: 'Project')) + allow(Gitlab::GlobalId::Deprecations).to receive(:deprecation_for).with('Issue').and_return(double(new_name: 'Project')) project = create(:project) gid_string = Gitlab::GlobalId.build(model_name: Issue.name, id: project.id).to_s diff --git a/spec/lib/gitlab/config/loader/yaml_spec.rb b/spec/lib/gitlab/config/loader/yaml_spec.rb index 66ea931a42c..c7f84cd583c 100644 --- a/spec/lib/gitlab/config/loader/yaml_spec.rb +++ b/spec/lib/gitlab/config/loader/yaml_spec.rb @@ -120,12 +120,6 @@ RSpec.describe Gitlab::Config::Loader::Yaml do it 'returns false' do expect(loader).not_to be_valid end - - it 'returns true if "ci_yaml_limit_size" feature flag is disabled' do - stub_feature_flags(ci_yaml_limit_size: false) - - expect(loader).to be_valid - end end describe '#load!' do diff --git a/spec/lib/gitlab/global_id/deprecations_spec.rb b/spec/lib/gitlab/global_id/deprecations_spec.rb index 22a4766c0a0..2c58c7ff15d 100644 --- a/spec/lib/gitlab/global_id/deprecations_spec.rb +++ b/spec/lib/gitlab/global_id/deprecations_spec.rb @@ -1,12 +1,28 @@ # frozen_string_literal: true -require 'spec_helper' +require 'fast_spec_helper' +require 'test_prof/recipes/rspec/let_it_be' +require 'graphql' +require_relative '../../../../app/graphql/types/base_scalar' +require_relative '../../../../app/graphql/types/global_id_type' +require_relative '../../../support/helpers/global_id_deprecation_helpers' + +TestProf::BeforeAll.adapter = Class.new do + def begin_transaction; end + + def rollback_transaction; end +end.new RSpec.describe Gitlab::GlobalId::Deprecations do include GlobalIDDeprecationHelpers - let_it_be(:deprecation_1) { described_class::Deprecation.new(old_model_name: 'Foo::Model', new_model_name: 'Bar', milestone: '9.0') } - let_it_be(:deprecation_2) { described_class::Deprecation.new(old_model_name: 'Baz', new_model_name: 'Qux::Model', milestone: '10.0') } + let_it_be(:deprecation_1) do + described_class::NameDeprecation.new(old_name: 'Foo::Model', new_name: 'Bar', milestone: '9.0') + end + + let_it_be(:deprecation_2) do + described_class::NameDeprecation.new(old_name: 'Baz', new_name: 'Qux::Model', milestone: '10.0') + end before do stub_global_id_deprecations(deprecation_1, deprecation_2) diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb index 919335bc9fa..819a5633a78 100644 --- a/spec/lib/gitlab/gpg/commit_spec.rb +++ b/spec/lib/gitlab/gpg/commit_spec.rb @@ -206,12 +206,12 @@ RSpec.describe Gitlab::Gpg::Commit do it_behaves_like 'returns the cached signature on second call' end - context 'user email does not match the committer email, but is the same user' do + context 'gpg key email does not match the committer_email but is the same user when the committer_email belongs to the user as a confirmed secondary email' do let(:committer_email) { GpgHelpers::User2.emails.first } let(:user) do create(:user, email: GpgHelpers::User1.emails.first).tap do |user| - create :email, user: user, email: GpgHelpers::User2.emails.first + create :email, :confirmed, user: user, email: committer_email end end @@ -230,6 +230,30 @@ RSpec.describe Gitlab::Gpg::Commit do it_behaves_like 'returns the cached signature on second call' end + context 'gpg key email does not match the committer_email when the committer_email belongs to the user as a unconfirmed secondary email' do + let(:committer_email) { GpgHelpers::User2.emails.first } + + let(:user) do + create(:user, email: GpgHelpers::User1.emails.first).tap do |user| + create :email, user: user, email: committer_email + end + end + + it 'returns an invalid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'other_user' + ) + end + + it_behaves_like 'returns the cached signature on second call' + end + context 'user email does not match the committer email' do let(:committer_email) { GpgHelpers::User2.emails.first } let(:user_email) { GpgHelpers::User1.emails.first } diff --git a/spec/lib/gitlab/graphql/type_name_deprecations_spec.rb b/spec/lib/gitlab/graphql/type_name_deprecations_spec.rb new file mode 100644 index 00000000000..9fa2d59c093 --- /dev/null +++ b/spec/lib/gitlab/graphql/type_name_deprecations_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'test_prof/recipes/rspec/let_it_be' +require_relative '../../../support/helpers/type_name_deprecation_helpers' + +TestProf::BeforeAll.adapter = Class.new do + def begin_transaction; end + + def rollback_transaction; end +end.new + +RSpec.describe Gitlab::Graphql::TypeNameDeprecations do + include TypeNameDeprecationHelpers + + let_it_be(:deprecation_1) do + described_class::NameDeprecation.new(old_name: 'Foo::Model', new_name: 'Bar', milestone: '9.0') + end + + let_it_be(:deprecation_2) do + described_class::NameDeprecation.new(old_name: 'Baz', new_name: 'Qux::Model', milestone: '10.0') + end + + before do + stub_type_name_deprecations(deprecation_1, deprecation_2) + end + + describe '.deprecated?' do + it 'returns a boolean to signal if model name has a deprecation', :aggregate_failures do + expect(described_class.deprecated?('Foo::Model')).to eq(true) + expect(described_class.deprecated?('Qux::Model')).to eq(false) + end + end + + describe '.deprecation_for' do + it 'returns the deprecation for the model if it exists', :aggregate_failures do + expect(described_class.deprecation_for('Foo::Model')).to eq(deprecation_1) + expect(described_class.deprecation_for('Qux::Model')).to be_nil + end + end + + describe '.deprecation_by' do + it 'returns the deprecation by the model if it exists', :aggregate_failures do + expect(described_class.deprecation_by('Foo::Model')).to be_nil + expect(described_class.deprecation_by('Qux::Model')).to eq(deprecation_2) + end + end + + describe '.apply_to_graphql_name' do + it 'returns the corresponding graphql_name of the GID for the new model', :aggregate_failures do + expect(described_class.apply_to_graphql_name('Foo::Model')).to eq('Bar') + expect(described_class.apply_to_graphql_name('Baz')).to eq('Qux::Model') + end + + it 'returns the same value if there is no deprecation' do + expect(described_class.apply_to_graphql_name('Project')).to eq('Project') + end + end +end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 4471ea4670a..a324fd70424 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -578,7 +578,6 @@ project: - remove_source_branch_after_merge - deleting_user - upstream_projects -- downstream_projects - upstream_project_subscriptions - downstream_project_subscriptions - service_desk_setting diff --git a/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb index 34659030020..ab3ffddc042 100644 --- a/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb @@ -20,18 +20,31 @@ RSpec.describe Gitlab::LegacyGithubImport::UserFormatter do expect(user.gitlab_id).to eq gl_user.id end - it 'returns GitLab user id when user primary email matches GitHub email' do + it 'returns GitLab user id when user confirmed primary email matches GitHub email' do gl_user = create(:user, email: octocat.email) expect(user.gitlab_id).to eq gl_user.id end - it 'returns GitLab user id when any of user linked emails matches GitHub email' do + it 'returns GitLab user id when user unconfirmed primary email matches GitHub email' do + gl_user = create(:user, :unconfirmed, email: octocat.email) + + expect(user.gitlab_id).to eq gl_user.id + end + + it 'returns GitLab user id when user confirmed secondary email matches GitHub email' do gl_user = create(:user, email: 'johndoe@example.com') - create(:email, user: gl_user, email: octocat.email) + create(:email, :confirmed, user: gl_user, email: octocat.email) expect(user.gitlab_id).to eq gl_user.id end + + it 'returns nil when user unconfirmed secondary email matches GitHub email' do + gl_user = create(:user, email: 'johndoe@example.com') + create(:email, user: gl_user, email: octocat.email) + + expect(user.gitlab_id).to be_nil + end end it 'returns nil when GitHub user is not a GitLab user' do diff --git a/spec/lib/gitlab/visibility_level_spec.rb b/spec/lib/gitlab/visibility_level_spec.rb index 0d34d22cbbe..9555c0afba2 100644 --- a/spec/lib/gitlab/visibility_level_spec.rb +++ b/spec/lib/gitlab/visibility_level_spec.rb @@ -4,20 +4,52 @@ require 'spec_helper' RSpec.describe Gitlab::VisibilityLevel do describe '.level_value' do - it 'converts "public" to integer value' do - expect(described_class.level_value('public')).to eq(Gitlab::VisibilityLevel::PUBLIC) + where(:string_value, :integer_value) do + [ + ['private', described_class::PRIVATE], + ['internal', described_class::INTERNAL], + ['public', described_class::PUBLIC] + ] end - it 'converts string integer to integer value' do - expect(described_class.level_value('20')).to eq(20) + with_them do + it "converts '#{params[:string_value]}' to integer value #{params[:integer_value]}" do + expect(described_class.level_value(string_value)).to eq(integer_value) + end + + it "converts string integer '#{params[:integer_value]}' to integer value #{params[:integer_value]}" do + expect(described_class.level_value(integer_value.to_s)).to eq(integer_value) + end + + it 'defaults to PRIVATE when string integer value is not valid' do + expect(described_class.level_value(integer_value.to_s + 'r')).to eq(described_class::PRIVATE) + expect(described_class.level_value(integer_value.to_s + ' ')).to eq(described_class::PRIVATE) + expect(described_class.level_value('r' + integer_value.to_s)).to eq(described_class::PRIVATE) + end + + it 'defaults to PRIVATE when string value is not valid' do + expect(described_class.level_value(string_value.capitalize)).to eq(described_class::PRIVATE) + expect(described_class.level_value(string_value + ' ')).to eq(described_class::PRIVATE) + expect(described_class.level_value(string_value + 'r')).to eq(described_class::PRIVATE) + end end it 'defaults to PRIVATE when string value is not valid' do - expect(described_class.level_value('invalid')).to eq(Gitlab::VisibilityLevel::PRIVATE) + expect(described_class.level_value('invalid')).to eq(described_class::PRIVATE) end it 'defaults to PRIVATE when integer value is not valid' do - expect(described_class.level_value(100)).to eq(Gitlab::VisibilityLevel::PRIVATE) + expect(described_class.level_value(100)).to eq(described_class::PRIVATE) + end + + context 'when `fallback_value` is set to `nil`' do + it 'returns `nil` when string value is not valid' do + expect(described_class.level_value('invalid', fallback_value: nil)).to be_nil + end + + it 'returns `nil` when integer value is not valid' do + expect(described_class.level_value(100, fallback_value: nil)).to be_nil + end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 187be557064..08d770a1beb 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -226,27 +226,45 @@ RSpec.describe Commit do end describe '#committer' do - context 'with a confirmed e-mail' do - it 'returns the user' do - user = create(:user, email: commit.committer_email) + context "when committer_email is the user's primary email" do + context 'when the user email is confirmed' do + let!(:user) { create(:user, email: commit.committer_email) } - expect(commit.committer).to eq(user) + it 'returns the user' do + expect(commit.committer).to eq(user) + expect(commit.committer(confirmed: false)).to eq(user) + end end - end - context 'with an unconfirmed e-mail' do - let(:user) { create(:user) } + context 'when the user email is unconfirmed' do + let!(:user) { create(:user, :unconfirmed, email: commit.committer_email) } - before do - create(:email, user: user, email: commit.committer_email) + it 'returns the user according to confirmed argument' do + expect(commit.committer).to be_nil + expect(commit.committer(confirmed: false)).to eq(user) + end end + end - it 'returns no user' do - expect(commit.committer).to be_nil + context "when committer_email is the user's secondary email" do + let!(:user) { create(:user) } + + context 'when the user email is confirmed' do + let!(:email) { create(:email, :confirmed, user: user, email: commit.committer_email) } + + it 'returns the user' do + expect(commit.committer).to eq(user) + expect(commit.committer(confirmed: false)).to eq(user) + end end - it 'returns the user' do - expect(commit.committer(confirmed: false)).to eq(user) + context 'when the user email is unconfirmed' do + let!(:email) { create(:email, user: user, email: commit.committer_email) } + + it 'does not return the user' do + expect(commit.committer).to be_nil + expect(commit.committer(confirmed: false)).to be_nil + end end end end 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 15b6b45eaba..0685144dea6 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -121,6 +121,38 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end end + + describe 'before_validation :reset_token' do + context 'when a token was previously set' do + subject { create(:project_error_tracking_setting, project: project) } + + it 'resets token if url changed' do + subject.api_url = 'http://sentry.com/api/0/projects/org-slug/proj-slug/' + + expect(subject).not_to be_valid + expect(subject.token).to be_nil + end + + it "does not reset token if new url is set together with the same token" do + subject.api_url = 'http://sentrytest.com/api/0/projects/org-slug/proj-slug/' + current_token = subject.token + subject.token = current_token + + expect(subject).to be_valid + expect(subject.token).to eq(current_token) + expect(subject.api_url).to eq('http://sentrytest.com/api/0/projects/org-slug/proj-slug/') + end + + it 'does not reset token if new url is set together with a new token' do + subject.api_url = 'http://sentrytest.com/api/0/projects/org-slug/proj-slug/' + subject.token = 'token' + + expect(subject).to be_valid + expect(subject.token).to eq('token') + expect(subject.api_url).to eq('http://sentrytest.com/api/0/projects/org-slug/proj-slug/') + end + end + end end describe '.extract_sentry_external_url' do diff --git a/spec/models/grafana_integration_spec.rb b/spec/models/grafana_integration_spec.rb index bb822187e0c..73ec2856c05 100644 --- a/spec/models/grafana_integration_spec.rb +++ b/spec/models/grafana_integration_spec.rb @@ -86,4 +86,38 @@ RSpec.describe GrafanaIntegration do end end end + + describe 'Callbacks' do + describe 'before_validation :reset_token' do + context 'when a token was previously set' do + subject(:grafana_integration) { create(:grafana_integration) } + + it 'resets token if url changed' do + grafana_integration.grafana_url = 'http://gitlab1.com' + + expect(grafana_integration).not_to be_valid + expect(grafana_integration.send(:token)).to be_nil + end + + it "does not reset token if new url is set together with the same token" do + grafana_integration.grafana_url = 'http://gitlab_edited.com' + current_token = grafana_integration.send(:token) + grafana_integration.token = current_token + + expect(grafana_integration).to be_valid + expect(grafana_integration.send(:token)).to eq(current_token) + expect(grafana_integration.grafana_url).to eq('http://gitlab_edited.com') + end + + it 'does not reset token if new url is set together with a new token' do + grafana_integration.grafana_url = 'http://gitlab_edited.com' + grafana_integration.token = 'token' + + expect(grafana_integration).to be_valid + expect(grafana_integration.send(:token)).to eq('token') + expect(grafana_integration.grafana_url).to eq('http://gitlab_edited.com') + end + end + end + end end diff --git a/spec/models/hooks/web_hook_log_spec.rb b/spec/models/hooks/web_hook_log_spec.rb index e1fea3318f6..8ff8a1c3865 100644 --- a/spec/models/hooks/web_hook_log_spec.rb +++ b/spec/models/hooks/web_hook_log_spec.rb @@ -30,15 +30,12 @@ RSpec.describe WebHookLog do end describe '#save' do - let(:web_hook_log) { build(:web_hook_log, url: url) } - let(:url) { 'http://example.com' } - - subject { web_hook_log.save! } + context 'with basic auth credentials' do + let(:web_hook_log) { build(:web_hook_log, url: 'http://test:123@example.com') } - it { is_expected.to eq(true) } + subject { web_hook_log.save! } - context 'with basic auth credentials' do - let(:url) { 'http://test:123@example.com'} + it { is_expected.to eq(true) } it 'obfuscates the basic auth credentials' do subject @@ -46,6 +43,30 @@ RSpec.describe WebHookLog do expect(web_hook_log.url).to eq('http://*****:*****@example.com') end end + + context 'with author email' do + let(:author) { create(:user) } + let(:web_hook_log) { create(:web_hook_log, request_data: data) } + let(:data) do + { + commit: { + author: { + name: author.name, + email: author.email + } + } + }.deep_stringify_keys + end + + it "redacts author's email" do + expect(web_hook_log.request_data['commit']).to match a_hash_including( + 'author' => { + 'name' => author.name, + 'email' => _('[REDACTED]') + } + ) + end + end end describe '.delete_batch_for' do diff --git a/spec/models/integrations/campfire_spec.rb b/spec/models/integrations/campfire_spec.rb index 405a9ff4b3f..a6bcd22b6f6 100644 --- a/spec/models/integrations/campfire_spec.rb +++ b/spec/models/integrations/campfire_spec.rb @@ -5,7 +5,17 @@ require 'spec_helper' RSpec.describe Integrations::Campfire do include StubRequests + it_behaves_like Integrations::ResetSecretFields do + let(:integration) { described_class.new } + end + describe 'Validations' do + it { is_expected.to validate_numericality_of(:room).is_greater_than(0).only_integer } + it { is_expected.to validate_length_of(:subdomain).is_at_least(1).is_at_most(63).allow_blank } + it { is_expected.to allow_value("foo").for(:subdomain) } + it { is_expected.not_to allow_value("foo.bar").for(:subdomain) } + it { is_expected.not_to allow_value("foo.bar/#").for(:subdomain) } + context 'when integration is active' do before do subject.active = true diff --git a/spec/models/integrations/drone_ci_spec.rb b/spec/models/integrations/drone_ci_spec.rb index 78d55c49e7b..5ae4af1a665 100644 --- a/spec/models/integrations/drone_ci_spec.rb +++ b/spec/models/integrations/drone_ci_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do subject(:integration) { described_class.new } + it_behaves_like Integrations::ResetSecretFields do + let(:integration) { subject } + end + describe 'validations' do context 'active' do before do diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 2a994540bd3..01c08a0948f 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -12,6 +12,7 @@ RSpec.describe Integrations::Jira do let(:api_url) { 'http://api-jira.example.com' } let(:username) { 'jira-username' } let(:password) { 'jira-password' } + let(:project_key) { nil } let(:transition_id) { 'test27' } let(:server_info_results) { { 'deploymentType' => 'Cloud' } } let(:jira_integration) do @@ -19,7 +20,8 @@ RSpec.describe Integrations::Jira do project: project, url: url, username: username, - password: password + password: password, + project_key: project_key ) end @@ -533,6 +535,22 @@ RSpec.describe Integrations::Jira do expect(WebMock).to have_requested(:get, issue_url) end end + + context 'with restricted restrict_project_key option' do + subject(:find_issue) { jira_integration.find_issue(issue_key, restrict_project_key: true) } + + it { is_expected.to eq(nil) } + + context 'and project_key matches' do + let(:project_key) { 'JIRA' } + + it 'calls the Jira API to get the issue' do + find_issue + + expect(WebMock).to have_requested(:get, issue_url) + end + end + end end describe '#close_issue' do diff --git a/spec/models/integrations/packagist_spec.rb b/spec/models/integrations/packagist_spec.rb index dce96890522..d1976e73e2e 100644 --- a/spec/models/integrations/packagist_spec.rb +++ b/spec/models/integrations/packagist_spec.rb @@ -29,6 +29,10 @@ RSpec.describe Integrations::Packagist do let(:hook_url) { "#{packagist_server}/api/update-package?username=#{packagist_username}&apiToken=#{packagist_token}" } end + it_behaves_like Integrations::ResetSecretFields do + let(:integration) { described_class.new(packagist_params) } + end + describe '#execute' do let(:user) { create(:user) } let(:project) { create(:project, :repository) } diff --git a/spec/models/integrations/zentao_spec.rb b/spec/models/integrations/zentao_spec.rb index 2b0532c7930..4ef977ba3d2 100644 --- a/spec/models/integrations/zentao_spec.rb +++ b/spec/models/integrations/zentao_spec.rb @@ -9,6 +9,31 @@ RSpec.describe Integrations::Zentao do let(:zentao_product_xid) { '3' } let(:zentao_integration) { create(:zentao_integration) } + it_behaves_like Integrations::ResetSecretFields do + let(:integration) { zentao_integration } + end + + describe 'set_default_data' do + let(:project) { create(:project, :repository) } + + context 'when gitlab.yml was initialized' do + it 'is prepopulated with the settings' do + settings = { + 'zentao' => { + 'url' => 'http://zentao.sample/projects/project_a', + 'api_url' => 'http://zentao.sample/api' + } + } + allow(Gitlab.config).to receive(:issues_tracker).and_return(settings) + + integration = project.create_zentao_integration(active: true) + + expect(integration.url).to eq('http://zentao.sample/projects/project_a') + expect(integration.api_url).to eq('http://zentao.sample/api') + end + end + end + describe '#create' do let(:project) { create(:project, :repository) } let(:params) do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2b69d830f43..cbaf3d0b3d5 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -5632,6 +5632,7 @@ RSpec.describe Project, factory_default: :keep do let(:import_state) { create(:import_state, project: project) } it 'runs the correct hooks' do + expect(project.repository).to receive(:remove_prohibited_branches) expect(project.repository).to receive(:expire_content_cache) expect(project.wiki.repository).to receive(:expire_content_cache) expect(import_state).to receive(:finish) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 11323c40d28..b3fbe75a526 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -3364,4 +3364,62 @@ RSpec.describe Repository do end end end + + describe '#remove_prohibited_branches' do + let(:branch_name) { '37fd3601be4c25497a39fa2e6a206e09e759d597' } + + before do + allow(repository.raw_repository).to receive(:branch_names).and_return([branch_name]) + end + + context 'when prohibited branch exists' do + it 'deletes prohibited branch' do + expect(repository.raw_repository).to receive(:delete_branch).with(branch_name) + + repository.remove_prohibited_branches + end + end + + shared_examples 'does not delete branch' do + it 'returns without removing the branch' do + expect(repository.raw_repository).not_to receive(:delete_branch) + + repository.remove_prohibited_branches + end + end + + context 'when branch name is 40-characters long but not hexadecimal' do + let(:branch_name) { '37fd3601be4c25497a39fa2e6a206e09e759d59s' } + + include_examples 'does not delete branch' + end + + context 'when branch name is hexadecimal' do + context 'when branch name is less than 40-characters long' do + let(:branch_name) { '37fd3601be4c25497a39fa2e6a206e09e759d' } + + include_examples 'does not delete branch' + end + + context 'when branch name is more than 40-characters long' do + let(:branch_name) { '37fd3601be4c25497a39fa2e6a206e09e759dfdfd' } + + include_examples 'does not delete branch' + end + end + + context 'when prohibited branch does not exist' do + let(:branch_name) { 'main' } + + include_examples 'does not delete branch' + end + + context 'when raw repository does not exist' do + before do + allow(repository).to receive(:exists?).and_return(false) + end + + include_examples 'does not delete branch' + end + end end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 70afafce132..a54edc8510e 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -36,8 +36,6 @@ RSpec.describe Snippet do it { is_expected.to validate_presence_of(:content) } - it { is_expected.to validate_inclusion_of(:visibility_level).in_array(Gitlab::VisibilityLevel.values) } - it do allow(Gitlab::CurrentSettings).to receive(:snippet_size_limit).and_return(1) diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 7df22078c6d..18b0cb36cc6 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -495,4 +495,14 @@ RSpec.describe Todo do it { is_expected.to contain_exactly(user1.id, user2.id) } end + + describe '.for_internal_notes' do + it 'returns todos created from internal notes' do + internal_note = create(:note, confidential: true ) + todo = create(:todo, note: internal_note) + create(:todo) + + expect(described_class.for_internal_notes).to contain_exactly(todo) + end + end end diff --git a/spec/models/u2f_registration_spec.rb b/spec/models/u2f_registration_spec.rb index 6bb9ccfcf35..027d26d9657 100644 --- a/spec/models/u2f_registration_spec.rb +++ b/spec/models/u2f_registration_spec.rb @@ -6,23 +6,22 @@ RSpec.describe U2fRegistration do let_it_be(:user) { create(:user) } let(:u2f_registration_name) { 'u2f_device' } - let(:u2f_registration) do device = U2F::FakeU2F.new(FFaker::BaconIpsum.characters(5)) - create(:u2f_registration, name: u2f_registration_name, - user: user, - certificate: Base64.strict_encode64(device.cert_raw), - key_handle: U2F.urlsafe_encode64(device.key_handle_raw), - public_key: Base64.strict_encode64(device.origin_public_key_raw)) + create( + :u2f_registration, name: u2f_registration_name, + user: user, + certificate: Base64.strict_encode64(device.cert_raw), + key_handle: U2F.urlsafe_encode64(device.key_handle_raw), + public_key: Base64.strict_encode64(device.origin_public_key_raw) + ) end describe 'callbacks' do - describe '#create_webauthn_registration' do + describe 'after create' do shared_examples_for 'creates webauthn registration' do it 'creates webauthn registration' do - created_record = u2f_registration - - webauthn_registration = WebauthnRegistration.where(u2f_registration_id: created_record.id) + webauthn_registration = WebauthnRegistration.where(u2f_registration_id: u2f_registration.id) expect(webauthn_registration).to exist end end @@ -55,5 +54,29 @@ RSpec.describe U2fRegistration do u2f_registration end end + + describe 'after update' do + context 'when counter is updated' do + it 'updates the webauthn registration counter to be the same value' do + new_counter = u2f_registration.counter + 1 + webauthn_registration = WebauthnRegistration.find_by(u2f_registration_id: u2f_registration.id) + + u2f_registration.update!(counter: new_counter) + + expect(u2f_registration.reload.counter).to eq(new_counter) + expect(webauthn_registration.reload.counter).to eq(new_counter) + end + end + + context 'when sign count of registration is not updated' do + it 'does not update the counter' do + webauthn_registration = WebauthnRegistration.find_by(u2f_registration_id: u2f_registration.id) + + expect do + u2f_registration.update!(name: 'a new name') + end.not_to change { webauthn_registration.counter } + end + end + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6e5406a6f65..f62183a213a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2650,6 +2650,14 @@ RSpec.describe User do expect(described_class.find_by_any_email(private_email, confirmed: true)).to eq(user) end + it 'finds user through private commit email when user is unconfirmed' do + user = create(:user, :unconfirmed) + private_email = user.private_commit_email + + expect(described_class.find_by_any_email(private_email)).to eq(user) + expect(described_class.find_by_any_email(private_email, confirmed: true)).to eq(user) + end + it 'finds by primary email' do user = create(:user, email: 'foo@example.com') @@ -2657,6 +2665,13 @@ RSpec.describe User do expect(described_class.find_by_any_email(user.email, confirmed: true)).to eq user end + it 'finds by primary email when user is unconfirmed according to confirmed argument' do + user = create(:user, :unconfirmed, email: 'foo@example.com') + + expect(described_class.find_by_any_email(user.email)).to eq user + expect(described_class.find_by_any_email(user.email, confirmed: true)).to be_nil + end + it 'finds by uppercased email' do user = create(:user, email: 'foo@example.com') @@ -2665,35 +2680,47 @@ RSpec.describe User do end context 'finds by secondary email' do - let(:user) { email.user } + context 'when primary email is confirmed' do + let(:user) { email.user } - context 'primary email confirmed' do - context 'secondary email confirmed' do + context 'when secondary email is confirmed' do let!(:email) { create(:email, :confirmed, email: 'foo@example.com') } - it 'finds user respecting the confirmed flag' do + it 'finds user' do expect(described_class.find_by_any_email(email.email)).to eq user expect(described_class.find_by_any_email(email.email, confirmed: true)).to eq user end end - context 'secondary email not confirmed' do + context 'when secondary email is unconfirmed' do let!(:email) { create(:email, email: 'foo@example.com') } - it 'finds user respecting the confirmed flag' do - expect(described_class.find_by_any_email(email.email)).to eq user + it 'does not find user' do + expect(described_class.find_by_any_email(email.email)).to be_nil expect(described_class.find_by_any_email(email.email, confirmed: true)).to be_nil end end end - context 'primary email not confirmed' do + context 'when primary email is unconfirmed' do let(:user) { create(:user, :unconfirmed) } - let!(:email) { create(:email, :confirmed, user: user, email: 'foo@example.com') } - it 'finds user respecting the confirmed flag' do - expect(described_class.find_by_any_email(email.email)).to eq user - expect(described_class.find_by_any_email(email.email, confirmed: true)).to be_nil + context 'when secondary email is confirmed' do + let!(:email) { create(:email, :confirmed, user: user, email: 'foo@example.com') } + + it 'finds user according to confirmed argument' do + expect(described_class.find_by_any_email(email.email)).to eq user + expect(described_class.find_by_any_email(email.email, confirmed: true)).to be_nil + end + end + + context 'when secondary email is unconfirmed' do + let!(:email) { create(:email, user: user, email: 'foo@example.com') } + + it 'does not find user' do + expect(described_class.find_by_any_email(email.email)).to be_nil + expect(described_class.find_by_any_email(email.email, confirmed: true)).to be_nil + end end end end @@ -2701,13 +2728,6 @@ RSpec.describe User do it 'returns nil when nothing found' do expect(described_class.find_by_any_email('')).to be_nil end - - it 'returns nil when user is not confirmed' do - user = create(:user, :unconfirmed, email: 'foo@example.com') - - expect(described_class.find_by_any_email(user.email, confirmed: false)).to eq(user) - expect(described_class.find_by_any_email(user.email, confirmed: true)).to be_nil - end end describe '.by_any_email' do @@ -2716,32 +2736,99 @@ RSpec.describe User do .to be_a_kind_of(ActiveRecord::Relation) end - it 'returns a relation of users' do + it 'returns empty relation of users when nothing found' do + expect(described_class.by_any_email('')).to be_empty + end + + it 'returns a relation of users for confirmed primary emails' do user = create(:user) - expect(described_class.by_any_email(user.email)).to eq([user]) + expect(described_class.by_any_email(user.email)).to match_array([user]) + expect(described_class.by_any_email(user.email, confirmed: true)).to match_array([user]) end - it 'returns a relation of users for confirmed users' do - user = create(:user) + it 'returns a relation of users for unconfirmed primary emails according to confirmed argument' do + user = create(:user, :unconfirmed) - expect(described_class.by_any_email(user.email, confirmed: true)).to eq([user]) + expect(described_class.by_any_email(user.email)).to match_array([user]) + expect(described_class.by_any_email(user.email, confirmed: true)).to be_empty end - it 'finds user through a private commit email' do + it 'finds users through private commit emails' do user = create(:user) private_email = user.private_commit_email - expect(described_class.by_any_email(private_email)).to eq([user]) - expect(described_class.by_any_email(private_email, confirmed: true)).to eq([user]) + expect(described_class.by_any_email(private_email)).to match_array([user]) + expect(described_class.by_any_email(private_email, confirmed: true)).to match_array([user]) + end + + it 'finds unconfirmed users through private commit emails' do + user = create(:user, :unconfirmed) + private_email = user.private_commit_email + + expect(described_class.by_any_email(private_email)).to match_array([user]) + expect(described_class.by_any_email(private_email, confirmed: true)).to match_array([user]) end it 'finds user through a private commit email in an array' do user = create(:user) private_email = user.private_commit_email - expect(described_class.by_any_email([private_email])).to eq([user]) - expect(described_class.by_any_email([private_email], confirmed: true)).to eq([user]) + expect(described_class.by_any_email([private_email])).to match_array([user]) + expect(described_class.by_any_email([private_email], confirmed: true)).to match_array([user]) + end + + it 'finds by uppercased email' do + user = create(:user, email: 'foo@example.com') + + expect(described_class.by_any_email(user.email.upcase)).to match_array([user]) + expect(described_class.by_any_email(user.email.upcase, confirmed: true)).to match_array([user]) + end + + context 'finds by secondary email' do + context 'when primary email is confirmed' do + let(:user) { email.user } + + context 'when secondary email is confirmed' do + let!(:email) { create(:email, :confirmed, email: 'foo@example.com') } + + it 'finds user' do + expect(described_class.by_any_email(email.email)).to match_array([user]) + expect(described_class.by_any_email(email.email, confirmed: true)).to match_array([user]) + end + end + + context 'when secondary email is unconfirmed' do + let!(:email) { create(:email, email: 'foo@example.com') } + + it 'does not find user' do + expect(described_class.by_any_email(email.email)).to be_empty + expect(described_class.by_any_email(email.email, confirmed: true)).to be_empty + end + end + end + + context 'when primary email is unconfirmed' do + let(:user) { create(:user, :unconfirmed) } + + context 'when secondary email is confirmed' do + let!(:email) { create(:email, :confirmed, user: user, email: 'foo@example.com') } + + it 'finds user according to confirmed argument' do + expect(described_class.by_any_email(email.email)).to match_array([user]) + expect(described_class.by_any_email(email.email, confirmed: true)).to be_empty + end + end + + context 'when secondary email is unconfirmed' do + let!(:email) { create(:email, user: user, email: 'foo@example.com') } + + it 'does not find user' do + expect(described_class.by_any_email(email.email)).to be_empty + expect(described_class.by_any_email(email.email, confirmed: true)).to be_empty + end + end + end end end @@ -2756,7 +2843,10 @@ RSpec.describe User do let_it_be(:user2) { create(:user, name: 'user name', username: 'username', email: 'someemail@example.com') } let_it_be(:user3) { create(:user, name: 'us', username: 'se', email: 'foo@example.com') } - let_it_be(:email) { create(:email, user: user, email: 'alias@example.com') } + let_it_be(:unconfirmed_user) { create(:user, :unconfirmed, name: 'not verified', username: 'notverified') } + + let_it_be(:unconfirmed_secondary_email) { create(:email, user: user, email: 'alias@example.com') } + let_it_be(:confirmed_secondary_email) { create(:email, :confirmed, user: user, email: 'alias2@example.com') } describe 'name user and email relative ordering' do let_it_be(:named_alexander) { create(:user, name: 'Alexander Person', username: 'abcd', email: 'abcd@example.com') } @@ -2814,16 +2904,26 @@ RSpec.describe User do it 'does not return users with a matching private email' do expect(described_class.search(user.email)).to be_empty - expect(described_class.search(email.email)).to be_empty + + expect(described_class.search(unconfirmed_secondary_email.email)).to be_empty + expect(described_class.search(confirmed_secondary_email.email)).to be_empty end context 'with private emails search' do - it 'returns users with matching private email' do + it 'returns users with matching private primary email' do expect(described_class.search(user.email, with_private_emails: true)).to match_array([user]) end - it 'returns users with matching private secondary email' do - expect(described_class.search(email.email, with_private_emails: true)).to match_array([user]) + it 'returns users with matching private unconfirmed primary email' do + expect(described_class.search(unconfirmed_user.email, with_private_emails: true)).to match_array([unconfirmed_user]) + end + + it 'returns users with matching private confirmed secondary email' do + expect(described_class.search(confirmed_secondary_email.email, with_private_emails: true)).to match_array([user]) + end + + it 'does not return users with matching private unconfirmed secondary email' do + expect(described_class.search(unconfirmed_secondary_email.email, with_private_emails: true)).to be_empty end end end @@ -3083,47 +3183,108 @@ RSpec.describe User do describe '#accept_pending_invitations!' do let(:user) { create(:user, email: 'user@email.com') } + + let(:confirmed_secondary_email) { create(:email, :confirmed, email: 'confirmedsecondary@example.com', user: user) } + let(:unconfirmed_secondary_email) { create(:email, email: 'unconfirmedsecondary@example.com', user: user) } + let!(:project_member_invite) { create(:project_member, :invited, invite_email: user.email) } let!(:group_member_invite) { create(:group_member, :invited, invite_email: user.email) } + let!(:external_project_member_invite) { create(:project_member, :invited, invite_email: 'external@email.com') } let!(:external_group_member_invite) { create(:group_member, :invited, invite_email: 'external@email.com') } + let!(:project_member_invite_via_confirmed_secondary_email) { create(:project_member, :invited, invite_email: confirmed_secondary_email.email) } + let!(:group_member_invite_via_confirmed_secondary_email) { create(:group_member, :invited, invite_email: confirmed_secondary_email.email) } + + let!(:project_member_invite_via_unconfirmed_secondary_email) { create(:project_member, :invited, invite_email: unconfirmed_secondary_email.email) } + let!(:group_member_invite_via_unconfirmed_secondary_email) { create(:group_member, :invited, invite_email: unconfirmed_secondary_email.email) } + it 'accepts all the user members pending invitations and returns the accepted_members' do accepted_members = user.accept_pending_invitations! - expect(accepted_members).to match_array([project_member_invite, group_member_invite]) + expect(accepted_members).to match_array( + [ + project_member_invite, + group_member_invite, + project_member_invite_via_confirmed_secondary_email, + group_member_invite_via_confirmed_secondary_email + ] + ) + expect(group_member_invite.reload).not_to be_invite expect(project_member_invite.reload).not_to be_invite + expect(external_project_member_invite.reload).to be_invite expect(external_group_member_invite.reload).to be_invite + + expect(project_member_invite_via_confirmed_secondary_email.reload).not_to be_invite + expect(group_member_invite_via_confirmed_secondary_email.reload).not_to be_invite + + expect(project_member_invite_via_unconfirmed_secondary_email.reload).to be_invite + expect(group_member_invite_via_unconfirmed_secondary_email.reload).to be_invite end end describe '#all_emails' do let(:user) { create(:user) } - let!(:email_confirmed) { create :email, user: user, confirmed_at: Time.current } - let!(:email_unconfirmed) { create :email, user: user } + let!(:unconfirmed_secondary_email) { create(:email, user: user) } + let!(:confirmed_secondary_email) { create(:email, :confirmed, user: user) } + + it 'returns all emails' do + expect(user.all_emails).to contain_exactly( + user.email, + user.private_commit_email, + confirmed_secondary_email.email + ) + end + + context 'when the primary email is confirmed' do + it 'includes the primary email' do + expect(user.all_emails).to include(user.email) + end + end + + context 'when the primary email is unconfirmed' do + let!(:user) { create(:user, :unconfirmed) } + + it 'includes the primary email' do + expect(user.all_emails).to include(user.email) + end + end + + context 'when the primary email is temp email for oauth' do + let!(:user) { create(:omniauth_user, :unconfirmed, email: 'temp-email-for-oauth-user@gitlab.localhost') } + + it 'does not include the primary email' do + expect(user.all_emails).not_to include(user.email) + end + end context 'when `include_private_email` is true' do - it 'returns all emails' do - expect(user.reload.all_emails).to contain_exactly( - user.email, - user.private_commit_email, - email_unconfirmed.email, - email_confirmed.email - ) + it 'includes the private commit email' do + expect(user.all_emails).to include(user.private_commit_email) end end context 'when `include_private_email` is false' do it 'does not include the private commit email' do - expect(user.reload.all_emails(include_private_email: false)).to contain_exactly( - user.email, - email_unconfirmed.email, - email_confirmed.email + expect(user.all_emails(include_private_email: false)).not_to include( + user.private_commit_email ) end end + + context 'when the secondary email is confirmed' do + it 'includes the secondary email' do + expect(user.all_emails).to include(confirmed_secondary_email.email) + end + end + + context 'when the secondary email is unconfirmed' do + it 'does not include the secondary email' do + expect(user.all_emails).not_to include(unconfirmed_secondary_email.email) + end + end end describe '#verified_emails' do diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb index 53154aef21e..cb351635081 100644 --- a/spec/requests/api/invitations_spec.rb +++ b/spec/requests/api/invitations_spec.rb @@ -7,6 +7,7 @@ RSpec.describe API::Invitations do let_it_be(:developer) { create(:user) } let_it_be(:access_requester) { create(:user) } let_it_be(:stranger) { create(:user) } + let_it_be(:unconfirmed_stranger) { create(:user, :unconfirmed) } let(:email) { 'email1@example.com' } let(:email2) { 'email2@example.com' } @@ -92,6 +93,46 @@ RSpec.describe API::Invitations do end.to change { source.members.invite.count }.by(1) end + it 'adds a new member by confirmed primary email' do + expect do + post invitations_url(source, maintainer), + params: { email: stranger.email, access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.non_invite.count }.by(1) + end + + it 'adds a new member by unconfirmed primary email' do + expect do + post invitations_url(source, maintainer), + params: { email: unconfirmed_stranger.email, access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.non_invite.count }.by(1) + end + + it 'adds a new member by confirmed secondary email' do + secondary_email = create(:email, :confirmed, email: 'secondary@example.com', user: stranger) + + expect do + post invitations_url(source, maintainer), + params: { email: secondary_email.email, access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.non_invite.count }.by(1) + end + + it 'adds a new member as an invite for unconfirmed secondary email' do + secondary_email = create(:email, email: 'secondary@example.com', user: stranger) + + expect do + post invitations_url(source, maintainer), + params: { email: secondary_email.email, access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.invite.count }.by(1).and change { source.members.non_invite.count }.by(0) + end + it 'adds a new member by user_id' do expect do post invitations_url(source, maintainer), diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb index edadfbc3d0c..f07dcfcccd6 100644 --- a/spec/requests/api/oauth_tokens_spec.rb +++ b/spec/requests/api/oauth_tokens_spec.rb @@ -25,6 +25,40 @@ RSpec.describe 'OAuth tokens' do end end + context 'when 2FA enforced' do + let_it_be(:user) { create(:user, otp_grace_period_started_at: 1.day.ago) } + + before do + stub_application_setting(require_two_factor_authentication: true) + end + + context 'when grace period expired' do + before do + stub_application_setting(two_factor_grace_period: 0) + end + + it 'does not create an access token' do + request_oauth_token(user, client_basic_auth_header(client)) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('invalid_grant') + end + end + + context 'when grace period is not expired' do + before do + stub_application_setting(two_factor_grace_period: 72) + end + + it 'creates an access token' do + request_oauth_token(user, client_basic_auth_header(client)) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['access_token']).not_to be_nil + end + end + end + context 'when user does not have 2FA enabled' do context 'when no client credentials provided' do it 'creates an access token' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 1c8b74558ae..26238a87209 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1307,6 +1307,81 @@ RSpec.describe API::Users do end end + context 'when user with a primary email exists' do + context 'when the primary email is confirmed' do + let!(:confirmed_user) { create(:user, email: 'foo@example.com') } + + it 'returns 409 conflict error' do + expect do + post api('/users', admin), + params: { + name: 'foo', + email: confirmed_user.email, + password: 'password', + username: 'TEST' + } + end.to change { User.count }.by(0) + expect(response).to have_gitlab_http_status(:conflict) + expect(json_response['message']).to eq('Email has already been taken') + end + end + + context 'when the primary email is unconfirmed' do + let!(:unconfirmed_user) { create(:user, :unconfirmed, email: 'foo@example.com') } + + it 'returns 409 conflict error' do + expect do + post api('/users', admin), + params: { + name: 'foo', + email: unconfirmed_user.email, + password: 'password', + username: 'TEST' + } + end.to change { User.count }.by(0) + expect(response).to have_gitlab_http_status(:conflict) + expect(json_response['message']).to eq('Email has already been taken') + end + end + end + + context 'when user with a secondary email exists' do + context 'when the secondary email is confirmed' do + let!(:email) { create(:email, :confirmed, email: 'foo@example.com') } + + it 'returns 409 conflict error' do + expect do + post api('/users', admin), + params: { + name: 'foo', + email: email.email, + password: 'password', + username: 'TEST' + } + end.to change { User.count }.by(0) + expect(response).to have_gitlab_http_status(:conflict) + expect(json_response['message']).to eq('Email has already been taken') + end + end + + context 'when the secondary email is unconfirmed' do + let!(:email) { create(:email, email: 'foo@example.com') } + + it 'does not create user' do + expect do + post api('/users', admin), + params: { + name: 'foo', + email: email.email, + password: 'password', + username: 'TEST' + } + end.to change { User.count }.by(0) + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + context "scopes" do let(:user) { admin } let(:path) { '/users' } @@ -1663,6 +1738,54 @@ RSpec.describe API::Users do expect(@user.reload.username).to eq(@user.username) end end + + context 'when user with a primary email exists' do + context 'when the primary email is confirmed' do + let!(:confirmed_user) { create(:user, email: 'foo@example.com') } + + it 'returns 409 conflict error' do + put api("/users/#{user.id}", admin), params: { email: confirmed_user.email } + + expect(response).to have_gitlab_http_status(:conflict) + expect(user.reload.email).not_to eq(confirmed_user.email) + end + end + + context 'when the primary email is unconfirmed' do + let!(:unconfirmed_user) { create(:user, :unconfirmed, email: 'foo@example.com') } + + it 'returns 409 conflict error' do + put api("/users/#{user.id}", admin), params: { email: unconfirmed_user.email } + + expect(response).to have_gitlab_http_status(:conflict) + expect(user.reload.email).not_to eq(unconfirmed_user.email) + end + end + end + + context 'when user with a secondary email exists' do + context 'when the secondary email is confirmed' do + let!(:email) { create(:email, :confirmed, email: 'foo@example.com') } + + it 'returns 409 conflict error' do + put api("/users/#{user.id}", admin), params: { email: email.email } + + expect(response).to have_gitlab_http_status(:conflict) + expect(user.reload.email).not_to eq(email.email) + end + end + + context 'when the secondary email is unconfirmed' do + let!(:email) { create(:email, email: 'foo@example.com') } + + it 'does not update email' do + put api("/users/#{user.id}", admin), params: { email: email.email } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(user.reload.email).not_to eq(email.email) + end + end + end end describe "PUT /user/:id/credit_card_validation" do @@ -2227,6 +2350,50 @@ RSpec.describe API::Users do expect(json_response['confirmed_at']).not_to be_nil end + + context 'when user with a primary email exists' do + context 'when the primary email is confirmed' do + let!(:confirmed_user) { create(:user, email: 'foo@example.com') } + + it 'returns 400 error' do + post api("/users/#{user.id}/emails", admin), params: { email: confirmed_user.email } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when the primary email is unconfirmed' do + let!(:unconfirmed_user) { create(:user, :unconfirmed, email: 'foo@example.com') } + + it 'returns 400 error' do + post api("/users/#{user.id}/emails", admin), params: { email: unconfirmed_user.email } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + + context 'when user with a secondary email exists' do + context 'when the secondary email is confirmed' do + let!(:email) { create(:email, :confirmed, email: 'foo@example.com') } + + it 'returns 400 error' do + post api("/users/#{user.id}/emails", admin), params: { email: email.email } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when the secondary email is unconfirmed' do + let!(:email) { create(:email, email: 'foo@example.com') } + + it 'returns 400 error' do + post api("/users/#{user.id}/emails", admin), params: { email: email.email } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end end describe 'GET /user/:id/emails' do diff --git a/spec/serializers/build_details_entity_spec.rb b/spec/serializers/build_details_entity_spec.rb index dd8238456aa..916798c669c 100644 --- a/spec/serializers/build_details_entity_spec.rb +++ b/spec/serializers/build_details_entity_spec.rb @@ -170,6 +170,24 @@ RSpec.describe BuildDetailsEntity do expect(message).to include('could not retrieve the needed artifacts.') end end + + context 'when dependency contains invalid dependency names' do + invalid_name = 'XSS<a href=# data-disable-with="<img src=x onerror=alert(document.domain)>">' + let!(:test1) { create(:ci_build, :success, :expired, pipeline: pipeline, name: invalid_name, stage_idx: 0) } + let!(:build) { create(:ci_build, :pending, pipeline: pipeline, stage_idx: 1, options: { dependencies: [invalid_name] }) } + + before do + build.pipeline.unlocked! + build.drop!(:missing_dependency_failure) + end + + it { is_expected.to include(failure_reason: 'missing_dependency_failure') } + + it 'escapes the invalid dependency names' do + escaped_name = html_escape(invalid_name) + expect(message).to include(escaped_name) + end + end end context 'when a build has environment with latest deployment' do diff --git a/spec/services/grafana/proxy_service_spec.rb b/spec/services/grafana/proxy_service_spec.rb index 7ddc31d45d9..99120de3593 100644 --- a/spec/services/grafana/proxy_service_spec.rb +++ b/spec/services/grafana/proxy_service_spec.rb @@ -50,12 +50,8 @@ RSpec.describe Grafana::ProxyService do describe '#execute' do subject(:result) { service.execute } - context 'when grafana integration is not configured' do - before do - allow(project).to receive(:grafana_integration).and_return(nil) - end - - it 'returns error' do + shared_examples 'missing proxy support' do + it 'returns API not supported error' do expect(result).to eq( status: :error, message: 'Proxy support for this API is not available currently' @@ -63,6 +59,40 @@ RSpec.describe Grafana::ProxyService do end end + context 'with unsupported proxy path' do + where(:proxy_path) do + %w[ + /api/vl/query_range + api/vl/query_range/ + api/vl/labels + api/v2/query_range + ../../../org/users + ] + end + + with_them do + include_examples 'missing proxy support' + end + end + + context 'with unsupported datasource_id' do + where(:datasource_id) do + ['', '-1', '1str', 'str1', '../../1', '1/../..', "1\n1"] + end + + with_them do + include_examples 'missing proxy support' + end + end + + context 'when grafana integration is not configured' do + before do + allow(project).to receive(:grafana_integration).and_return(nil) + end + + include_examples 'missing proxy support' + end + context 'with caching', :use_clean_rails_memory_store_caching do context 'when value not present in cache' do it 'returns nil' do diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index 57a151efda6..f43f64fdf89 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -35,6 +35,20 @@ RSpec.describe Groups::DestroyService do it { expect(NotificationSetting.unscoped.all).not_to include(notification_setting) } end + context 'bot tokens', :sidekiq_might_not_need_inline do + it 'removes group bot', :aggregate_failures do + bot = create(:user, :project_bot) + group.add_developer(bot) + token = create(:personal_access_token, user: bot) + + destroy_group(group, user, async) + + expect(PersonalAccessToken.find_by(id: token.id)).to be_nil + expect(User.find_by(id: bot.id)).to be_nil + expect(User.find_by(id: user.id)).not_to be_nil + end + end + context 'mattermost team', :sidekiq_might_not_need_inline do let!(:chat_team) { create(:chat_team, namespace: group) } diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb index c0e1691fe26..856dd4a2567 100644 --- a/spec/services/groups/update_service_spec.rb +++ b/spec/services/groups/update_service_spec.rb @@ -242,6 +242,69 @@ RSpec.describe Groups::UpdateService do end end + context 'when user is not group owner' do + context 'when group is private' do + before do + private_group.add_maintainer(user) + end + + it 'does not update the group to public' do + result = described_class.new(private_group, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC).execute + + expect(result).to eq(false) + expect(private_group.errors.count).to eq(1) + expect(private_group).to be_private + end + + it 'does not update the group to public with tricky value' do + result = described_class.new(private_group, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC.to_s + 'r').execute + + expect(result).to eq(false) + expect(private_group.errors.count).to eq(1) + expect(private_group).to be_private + end + end + + context 'when group is public' do + before do + public_group.add_maintainer(user) + end + + it 'does not update the group to private' do + result = described_class.new(public_group, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE).execute + + expect(result).to eq(false) + expect(public_group.errors.count).to eq(1) + expect(public_group).to be_public + end + + it 'does not update the group to private with invalid string value' do + result = described_class.new(public_group, user, visibility_level: 'invalid').execute + + expect(result).to eq(false) + expect(public_group.errors.count).to eq(1) + expect(public_group).to be_public + end + + it 'does not update the group to private with valid string value' do + result = described_class.new(public_group, user, visibility_level: 'private').execute + + expect(result).to eq(false) + expect(public_group.errors.count).to eq(1) + expect(public_group).to be_public + end + + # See https://gitlab.com/gitlab-org/gitlab/-/issues/359910 + it 'does not update the group to private because of Active Record typecasting' do + result = described_class.new(public_group, user, visibility_level: 'public').execute + + expect(result).to eq(true) + expect(public_group.errors.count).to eq(0) + expect(public_group).to be_public + end + end + end + context 'when updating #emails_disabled' do let(:service) { described_class.new(internal_group, user, emails_disabled: true) } diff --git a/spec/services/members/invite_service_spec.rb b/spec/services/members/invite_service_spec.rb index 7a1512970b4..d25c8996931 100644 --- a/spec/services/members/invite_service_spec.rb +++ b/spec/services/members/invite_service_spec.rb @@ -30,8 +30,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_ end end - context 'when email belongs to an existing user as a secondary email' do - let(:secondary_email) { create(:email, email: 'secondary@example.com', user: project_user) } + context 'when email belongs to an existing user as a confirmed secondary email' do + let(:secondary_email) { create(:email, :confirmed, email: 'secondary@example.com', user: project_user) } let(:params) { { email: secondary_email.email } } it 'adds an existing user to members', :aggregate_failures do @@ -42,6 +42,18 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_ end end + context 'when email belongs to an existing user as an unconfirmed secondary email' do + let(:unconfirmed_secondary_email) { create(:email, email: 'secondary@example.com', user: project_user) } + let(:params) { { email: unconfirmed_secondary_email.email } } + + it 'does not link the email with any user and successfully creates a member as an invite for that email' do + expect_to_create_members(count: 1) + expect(result[:status]).to eq(:success) + expect(project.users).not_to include project_user + expect(project.members.last).to be_invite + end + end + context 'when invites are passed as array' do context 'with emails' do let(:params) { { email: %w[email@example.org email2@example.org] } } @@ -291,6 +303,19 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_ end end + context 'with unconfirmed primary email' do + let_it_be(:unconfirmed_user) { create(:user, :unconfirmed) } + + let(:params) { { email: unconfirmed_user.email } } + + it 'adds an existing user to members' do + expect_to_create_members(count: 1) + expect(result[:status]).to eq(:success) + expect(project.users).to include unconfirmed_user + expect(project.members.last).not_to be_invite + end + end + context 'with user_id' do let(:params) { { user_id: project_user.id } } @@ -376,8 +401,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_ expect(existing_member.reset.access_level).to eq ProjectMember::MAINTAINER end - context 'when email belongs to an existing user as a secondary email' do - let(:secondary_email) { create(:email, email: 'secondary@example.com', user: existing_member.user) } + context 'when email belongs to an existing user as a confirmed secondary email' do + let(:secondary_email) { create(:email, :confirmed, email: 'secondary@example.com', user: existing_member.user) } let(:params) { { email: "#{secondary_email.email}" } } it 'allows re-invite to an already invited email' do diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb index bee91c358ce..95f2176dbc0 100644 --- a/spec/services/projects/operations/update_service_spec.rb +++ b/spec/services/projects/operations/update_service_spec.rb @@ -306,6 +306,11 @@ RSpec.describe Projects::Operations::UpdateService do let(:params) do { error_tracking_setting_attributes: { + api_host: 'https://sentrytest.gitlab.com/', + project: { + slug: 'sentry-project', + organization_slug: 'sentry-org' + }, enabled: false, token: '*' * 8 } @@ -313,7 +318,7 @@ RSpec.describe Projects::Operations::UpdateService do end before do - create(:project_error_tracking_setting, project: project, token: 'token') + create(:project_error_tracking_setting, project: project, token: 'token', api_url: 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/') end it 'does not update token' do diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index f019434a4fe..ca838be0fa8 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -120,6 +120,65 @@ RSpec.describe Projects::UpdateService do end end + context 'when user is not project owner' do + let_it_be(:maintainer) { create(:user) } + + before do + project.add_maintainer(maintainer) + end + + context 'when project is private' do + it 'does not update the project to public' do + result = update_project(project, maintainer, visibility_level: Gitlab::VisibilityLevel::PUBLIC) + + expect(result).to eq({ status: :error, message: 'New visibility level not allowed!' }) + expect(project).to be_private + end + + it 'does not update the project to public with tricky value' do + result = update_project(project, maintainer, visibility_level: Gitlab::VisibilityLevel::PUBLIC.to_s + 'r') + + expect(result).to eq({ status: :error, message: 'New visibility level not allowed!' }) + expect(project).to be_private + end + end + + context 'when project is public' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it 'does not update the project to private' do + result = update_project(project, maintainer, visibility_level: Gitlab::VisibilityLevel::PRIVATE) + + expect(result).to eq({ status: :error, message: 'New visibility level not allowed!' }) + expect(project).to be_public + end + + it 'does not update the project to private with invalid string value' do + result = update_project(project, maintainer, visibility_level: 'invalid') + + expect(result).to eq({ status: :error, message: 'New visibility level not allowed!' }) + expect(project).to be_public + end + + it 'does not update the project to private with valid string value' do + result = update_project(project, maintainer, visibility_level: 'private') + + expect(result).to eq({ status: :error, message: 'New visibility level not allowed!' }) + expect(project).to be_public + end + + # See https://gitlab.com/gitlab-org/gitlab/-/issues/359910 + it 'does not update the project to private because of Active Record typecasting' do + result = update_project(project, maintainer, visibility_level: 'public') + + expect(result).to eq({ status: :success }) + expect(project).to be_public + end + end + end + context 'when updating shared runners' do context 'can enable shared runners' do let(:group) { create(:group, shared_runners_enabled: true) } diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb index 03fa2482bbf..225e7933d79 100644 --- a/spec/services/todos/destroy/entity_leave_service_spec.rb +++ b/spec/services/todos/destroy/entity_leave_service_spec.rb @@ -3,21 +3,24 @@ require 'spec_helper' RSpec.describe Todos::Destroy::EntityLeaveService do - let_it_be(:user, reload: true) { create(:user) } - let_it_be(:user2, reload: true) { create(:user) } - - let(:group) { create(:group, :private) } - let(:project) { create(:project, :private, group: group) } - let(:issue) { create(:issue, project: project) } - let(:issue_c) { create(:issue, project: project, confidential: true) } - let!(:todo_group_user) { create(:todo, user: user, group: group) } - let!(:todo_group_user2) { create(:todo, user: user2, group: group) } - - let(:mr) { create(:merge_request, source_project: project) } - let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) } - let!(:todo_issue_user) { create(:todo, user: user, target: issue, project: project) } - let!(:todo_issue_c_user) { create(:todo, user: user, target: issue_c, project: project) } + let_it_be(:user, reload: true) { create(:user) } + let_it_be(:user2, reload: true) { create(:user) } + let_it_be_with_refind(:group) { create(:group, :private) } + let_it_be(:project) { create(:project, :private, group: group) } + + let(:issue) { create(:issue, project: project) } + let(:issue_c) { create(:issue, project: project, confidential: true) } + let!(:todo_group_user) { create(:todo, user: user, group: group) } + let!(:todo_group_user2) { create(:todo, user: user2, group: group) } + let(:mr) { create(:merge_request, source_project: project) } + let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) } + let!(:todo_issue_user) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_c_user) { create(:todo, user: user, target: issue_c, project: project) } let!(:todo_issue_c_user2) { create(:todo, user: user2, target: issue_c, project: project) } + let(:internal_note) { create(:note, noteable: issue, project: project, confidential: true ) } + let!(:todo_for_internal_note) do + create(:todo, user: user, target: issue, project: project, note: internal_note) + end shared_examples 'using different access permissions' do before do @@ -34,20 +37,28 @@ RSpec.describe Todos::Destroy::EntityLeaveService do it { does_not_remove_any_todos } end - shared_examples 'removes only confidential issues todos' do - it { removes_only_confidential_issues_todos } + shared_examples 'removes confidential issues and internal notes todos' do + it { removes_confidential_issues_and_internal_notes_todos } + end + + shared_examples 'removes only internal notes todos' do + it { removes_only_internal_notes_todos } end def does_not_remove_any_todos expect { subject }.not_to change { Todo.count } end - def removes_only_confidential_issues_todos - expect { subject }.to change { Todo.count }.from(6).to(5) + def removes_only_internal_notes_todos + expect { subject }.to change { Todo.count }.from(7).to(6) + end + + def removes_confidential_issues_and_internal_notes_todos + expect { subject }.to change { Todo.count }.from(7).to(5) end - def removes_confidential_issues_and_merge_request_todos - expect { subject }.to change { Todo.count }.from(6).to(4) + def removes_confidential_issues_and_internal_notes_and_merge_request_todos + expect { subject }.to change { Todo.count }.from(7).to(4) expect(user.todos).to match_array([todo_issue_user, todo_group_user]) end @@ -70,7 +81,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'when project is private' do context 'when user is not a member of the project' do it 'removes project todos for the provided user' do - expect { subject }.to change { Todo.count }.from(6).to(3) + expect { subject }.to change { Todo.count }.from(7).to(3) expect(user.todos).to match_array([todo_group_user]) expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2]) @@ -81,11 +92,11 @@ RSpec.describe Todos::Destroy::EntityLeaveService do where(:group_access, :project_access, :method_name) do [ [nil, :reporter, :does_not_remove_any_todos], - [nil, :guest, :removes_confidential_issues_and_merge_request_todos], + [nil, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], [:reporter, nil, :does_not_remove_any_todos], - [:guest, nil, :removes_confidential_issues_and_merge_request_todos], + [:guest, nil, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], [:guest, :reporter, :does_not_remove_any_todos], - [:guest, :guest, :removes_confidential_issues_and_merge_request_todos] + [:guest, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos] ] end @@ -97,11 +108,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do # a private project in an internal/public group is valid context 'when project is private in an internal/public group' do - let(:group) { create(:group, :internal) } + let_it_be(:group) { create(:group, :internal) } + let_it_be(:project) { create(:project, :private, group: group) } context 'when user is not a member of the project' do it 'removes project todos for the provided user' do - expect { subject }.to change { Todo.count }.from(6).to(3) + expect { subject }.to change { Todo.count }.from(7).to(3) expect(user.todos).to match_array([todo_group_user]) expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2]) @@ -112,11 +124,11 @@ RSpec.describe Todos::Destroy::EntityLeaveService do where(:group_access, :project_access, :method_name) do [ [nil, :reporter, :does_not_remove_any_todos], - [nil, :guest, :removes_confidential_issues_and_merge_request_todos], + [nil, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], [:reporter, nil, :does_not_remove_any_todos], - [:guest, nil, :removes_confidential_issues_and_merge_request_todos], + [:guest, nil, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], [:guest, :reporter, :does_not_remove_any_todos], - [:guest, :guest, :removes_confidential_issues_and_merge_request_todos] + [:guest, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos] ] end @@ -142,7 +154,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'confidential issues' do context 'when a user is not an author of confidential issue' do - it_behaves_like 'removes only confidential issues todos' + it_behaves_like 'removes confidential issues and internal notes todos' end context 'when a user is an author of confidential issue' do @@ -150,7 +162,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do issue_c.update!(author: user) end - it_behaves_like 'does not remove any todos' + it_behaves_like 'removes only internal notes todos' end context 'when a user is an assignee of confidential issue' do @@ -158,18 +170,18 @@ RSpec.describe Todos::Destroy::EntityLeaveService do issue_c.assignees << user end - it_behaves_like 'does not remove any todos' + it_behaves_like 'removes only internal notes todos' end context 'access permissions' do where(:group_access, :project_access, :method_name) do [ [nil, :reporter, :does_not_remove_any_todos], - [nil, :guest, :removes_only_confidential_issues_todos], + [nil, :guest, :removes_confidential_issues_and_internal_notes_todos], [:reporter, nil, :does_not_remove_any_todos], - [:guest, nil, :removes_only_confidential_issues_todos], + [:guest, nil, :removes_confidential_issues_and_internal_notes_todos], [:guest, :reporter, :does_not_remove_any_todos], - [:guest, :guest, :removes_only_confidential_issues_todos] + [:guest, :guest, :removes_confidential_issues_and_internal_notes_todos] ] end @@ -186,7 +198,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end it 'removes only users issue todos' do - expect { subject }.to change { Todo.count }.from(6).to(5) + expect { subject }.to change { Todo.count }.from(7).to(5) end end end @@ -199,7 +211,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'when group is private' do context 'when a user leaves a group' do it 'removes group and subproject todos for the user' do - expect { subject }.to change { Todo.count }.from(6).to(2) + expect { subject }.to change { Todo.count }.from(7).to(2) expect(user.todos).to be_empty expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2]) @@ -210,11 +222,11 @@ RSpec.describe Todos::Destroy::EntityLeaveService do where(:group_access, :project_access, :method_name) do [ [nil, :reporter, :does_not_remove_any_todos], - [nil, :guest, :removes_confidential_issues_and_merge_request_todos], + [nil, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], [:reporter, nil, :does_not_remove_any_todos], - [:guest, nil, :removes_confidential_issues_and_merge_request_todos], + [:guest, nil, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], [:guest, :reporter, :does_not_remove_any_todos], - [:guest, :guest, :removes_confidential_issues_and_merge_request_todos] + [:guest, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos] ] end @@ -224,12 +236,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end context 'with nested groups' do - let(:parent_group) { create(:group, :public) } - let(:parent_subgroup) { create(:group)} - let(:subgroup) { create(:group, :private, parent: group) } - let(:subgroup2) { create(:group, :private, parent: group) } - let(:subproject) { create(:project, group: subgroup) } - let(:subproject2) { create(:project, group: subgroup2) } + let_it_be_with_refind(:parent_group) { create(:group, :public) } + let_it_be_with_refind(:parent_subgroup) { create(:group) } + let_it_be(:subgroup) { create(:group, :private, parent: group) } + let_it_be(:subgroup2) { create(:group, :private, parent: group) } + let_it_be(:subproject) { create(:project, group: subgroup) } + let_it_be(:subproject2) { create(:project, group: subgroup2) } let!(:todo_subproject_user) { create(:todo, user: user, project: subproject) } let!(:todo_subproject2_user) { create(:todo, user: user, project: subproject2) } @@ -238,6 +250,10 @@ RSpec.describe Todos::Destroy::EntityLeaveService do let!(:todo_subproject_user2) { create(:todo, user: user2, project: subproject) } let!(:todo_subpgroup_user2) { create(:todo, user: user2, group: subgroup) } let!(:todo_parent_group_user) { create(:todo, user: user, group: parent_group) } + let(:subproject_internal_note) { create(:note, noteable: issue, project: project, confidential: true ) } + let!(:todo_for_internal_subproject_note) do + create(:todo, user: user, target: issue, project: project, note: subproject_internal_note) + end before do group.update!(parent: parent_group) @@ -245,7 +261,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'when the user is not a member of any groups/projects' do it 'removes todos for the user including subprojects todos' do - expect { subject }.to change { Todo.count }.from(13).to(5) + expect { subject }.to change { Todo.count }.from(15).to(5) expect(user.todos).to eq([todo_parent_group_user]) expect(user2.todos) @@ -269,7 +285,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end it 'does not remove group and subproject todos' do - expect { subject }.to change { Todo.count }.from(13).to(8) + expect { subject }.to change { Todo.count }.from(15).to(8) expect(user.todos) .to match_array( @@ -288,7 +304,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end it 'does not remove subproject and group todos' do - expect { subject }.to change { Todo.count }.from(13).to(8) + expect { subject }.to change { Todo.count }.from(15).to(8) expect(user.todos) .to match_array( @@ -319,13 +335,13 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'access permissions' do where(:group_access, :project_access, :method_name) do [ - [nil, nil, :removes_only_confidential_issues_todos], + [nil, nil, :removes_confidential_issues_and_internal_notes_todos], [nil, :reporter, :does_not_remove_any_todos], - [nil, :guest, :removes_only_confidential_issues_todos], + [nil, :guest, :removes_confidential_issues_and_internal_notes_todos], [:reporter, nil, :does_not_remove_any_todos], - [:guest, nil, :removes_only_confidential_issues_todos], + [:guest, nil, :removes_confidential_issues_and_internal_notes_todos], [:guest, :reporter, :does_not_remove_any_todos], - [:guest, :guest, :removes_only_confidential_issues_todos] + [:guest, :guest, :removes_confidential_issues_and_internal_notes_todos] ] end diff --git a/spec/support/helpers/global_id_deprecation_helpers.rb b/spec/support/helpers/global_id_deprecation_helpers.rb index 37ba1420fb3..5c6862ca84a 100644 --- a/spec/support/helpers/global_id_deprecation_helpers.rb +++ b/spec/support/helpers/global_id_deprecation_helpers.rb @@ -2,9 +2,11 @@ module GlobalIDDeprecationHelpers def stub_global_id_deprecations(*deprecations) - old_name_map = deprecations.index_by(&:old_model_name) - new_name_map = deprecations.index_by(&:new_model_name) - old_graphql_name_map = deprecations.index_by { |d| Types::GlobalIDType.model_name_to_graphql_name(d.old_model_name) } + old_name_map = deprecations.index_by(&:old_name) + new_name_map = deprecations.index_by(&:new_name) + old_graphql_name_map = deprecations.index_by do |d| + Gitlab::GlobalId::Deprecations.map_graphql_name(d.old_name) + end stub_const('Gitlab::GlobalId::Deprecations::OLD_NAME_MAP', old_name_map) stub_const('Gitlab::GlobalId::Deprecations::NEW_NAME_MAP', new_name_map) diff --git a/spec/support/helpers/type_name_deprecation_helpers.rb b/spec/support/helpers/type_name_deprecation_helpers.rb new file mode 100644 index 00000000000..591737ab532 --- /dev/null +++ b/spec/support/helpers/type_name_deprecation_helpers.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module TypeNameDeprecationHelpers + def stub_type_name_deprecations(*deprecations) + old_name_map = deprecations.index_by(&:old_name) + new_name_map = deprecations.index_by(&:new_name) + old_graphql_name_map = deprecations.index_by do |d| + Gitlab::Graphql::TypeNameDeprecations.map_graphql_name(d.old_name) + end + + stub_const('Gitlab::Graphql::TypeNameDeprecations::OLD_NAME_MAP', old_name_map) + stub_const('Gitlab::Graphql::TypeNameDeprecations::NEW_NAME_MAP', new_name_map) + stub_const('Gitlab::Graphql::TypeNameDeprecations::OLD_GRAPHQL_NAME_MAP', old_graphql_name_map) + end +end diff --git a/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb index 3d3b8c2207d..255c4e6f882 100644 --- a/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb +++ b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb @@ -66,6 +66,8 @@ Integration.available_integration_names.each do |integration| hash.merge!(k => 'foo@bar.com') elsif (integration == 'slack' || integration == 'mattermost') && k == :labels_to_be_notified_behavior hash.merge!(k => "match_any") + elsif integration == 'campfire' && k = :room + hash.merge!(k => '1234') else hash.merge!(k => "someword") end diff --git a/spec/support/shared_examples/finders/issues_finder_shared_examples.rb b/spec/support/shared_examples/finders/issues_finder_shared_examples.rb index 9d8f37a3e64..049ead9fb89 100644 --- a/spec/support/shared_examples/finders/issues_finder_shared_examples.rb +++ b/spec/support/shared_examples/finders/issues_finder_shared_examples.rb @@ -914,42 +914,65 @@ RSpec.shared_examples 'issues or work items finder' do |factory, execute_context end end - context 'filtering by crm contact' do - let_it_be(:contact1) { create(:contact, group: group) } - let_it_be(:contact2) { create(:contact, group: group) } + context 'crm filtering' do + let_it_be(:root_group) { create(:group) } + let_it_be(:group) { create(:group, parent: root_group) } + let_it_be(:project_crm) { create(:project, :public, group: group) } + let_it_be(:organization) { create(:organization, group: root_group) } + let_it_be(:contact1) { create(:contact, group: root_group, organization: organization) } + let_it_be(:contact2) { create(:contact, group: root_group, organization: organization) } - let_it_be(:contact1_item1) { create(factory, project: project1) } - let_it_be(:contact1_item2) { create(factory, project: project1) } - let_it_be(:contact2_item1) { create(factory, project: project1) } + let_it_be(:contact1_item1) { create(factory, project: project_crm) } + let_it_be(:contact1_item2) { create(factory, project: project_crm) } + let_it_be(:contact2_item1) { create(factory, project: project_crm) } + let_it_be(:item_no_contact) { create(factory, project: project_crm) } - let(:params) { { crm_contact_id: contact1.id } } + let_it_be(:all_project_issues) do + [contact1_item1, contact1_item2, contact2_item1, item_no_contact] + end + + before do + create(:crm_settings, group: root_group, enabled: true) - it 'returns for that contact' do create(:issue_customer_relations_contact, issue: contact1_item1, contact: contact1) create(:issue_customer_relations_contact, issue: contact1_item2, contact: contact1) create(:issue_customer_relations_contact, issue: contact2_item1, contact: contact2) - - expect(items).to contain_exactly(contact1_item1, contact1_item2) end - end - context 'filtering by crm organization' do - let_it_be(:organization) { create(:organization, group: group) } - let_it_be(:contact1) { create(:contact, group: group, organization: organization) } - let_it_be(:contact2) { create(:contact, group: group, organization: organization) } + context 'filtering by crm contact' do + let(:params) { { project_id: project_crm.id, crm_contact_id: contact1.id } } - let_it_be(:contact1_item1) { create(factory, project: project1) } - let_it_be(:contact1_item2) { create(factory, project: project1) } - let_it_be(:contact2_item1) { create(factory, project: project1) } + context 'when the user can read crm contacts' do + it 'returns for that contact' do + root_group.add_reporter(user) - let(:params) { { crm_organization_id: organization.id } } + expect(items).to contain_exactly(contact1_item1, contact1_item2) + end + end - it 'returns for that contact' do - create(:issue_customer_relations_contact, issue: contact1_item1, contact: contact1) - create(:issue_customer_relations_contact, issue: contact1_item2, contact: contact1) - create(:issue_customer_relations_contact, issue: contact2_item1, contact: contact2) + context 'when the user can not read crm contacts' do + it 'does not filter by contact' do + expect(items).to match_array(all_project_issues) + end + end + end + + context 'filtering by crm organization' do + let(:params) { { project_id: project_crm.id, crm_organization_id: organization.id } } + + context 'when the user can read crm organization' do + it 'returns for that organization' do + root_group.add_reporter(user) - expect(items).to contain_exactly(contact1_item1, contact1_item2, contact2_item1) + expect(items).to contain_exactly(contact1_item1, contact1_item2, contact2_item1) + end + end + + context 'when the user can not read crm organization' do + it 'does not filter by organization' do + expect(items).to match_array(all_project_issues) + end + end end end |