diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index a3811dfb2dd..c7c5e3545e6 100644 --- a/.rubocop_todo/rspec/feature_category.yml +++ b/.rubocop_todo/rspec/feature_category.yml @@ -3980,7 +3980,6 @@ RSpec/FeatureCategory: - 'spec/lib/gitlab/usage_data_counters/merge_request_widget_extension_counter_spec.rb' - 'spec/lib/gitlab/usage_data_counters/note_counter_spec.rb' - 'spec/lib/gitlab/usage_data_counters/package_event_counter_spec.rb' - - 'spec/lib/gitlab/usage_data_counters/productivity_analytics_counter_spec.rb' - 'spec/lib/gitlab/usage_data_counters/quick_action_activity_unique_counter_spec.rb' - 'spec/lib/gitlab/usage_data_counters/redis_counter_spec.rb' - 'spec/lib/gitlab/usage_data_counters/search_counter_spec.rb' diff --git a/.rubocop_todo/style/class_and_module_children.yml b/.rubocop_todo/style/class_and_module_children.yml index e6a3076b089..c389a3966d2 100644 --- a/.rubocop_todo/style/class_and_module_children.yml +++ b/.rubocop_todo/style/class_and_module_children.yml @@ -510,7 +510,6 @@ Style/ClassAndModuleChildren: - 'lib/gitlab/usage_data_counters/ci_template_unique_counter.rb' - 'lib/gitlab/usage_data_counters/designs_counter.rb' - 'lib/gitlab/usage_data_counters/note_counter.rb' - - 'lib/gitlab/usage_data_counters/productivity_analytics_counter.rb' - 'lib/gitlab/usage_data_counters/snippet_counter.rb' - 'lib/gitlab/usage_data_counters/source_code_counter.rb' - 'lib/gitlab/usage_data_counters/wiki_page_counter.rb' diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ae50ea0307..c082652e7cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 16.11.1 (2024-04-24) + +### Security (5 changes) + +- [Validation for encoded formatting characters](gitlab-org/security/gitlab@fc42e4b96ae1ac3cd766569d62d025cbf23ef16c) ([merge request](gitlab-org/security/gitlab!3979)) +- [Forbid untrusted sign-ins to GitLab with Bitbucket and fix related uid](gitlab-org/security/gitlab@ef083c319e67072029787cd5c6a588562984a58c) ([merge request](gitlab-org/security/gitlab!3983)) +- [Ensure PAT scope is validated everywhere for GraphQL/ActionCable](gitlab-org/security/gitlab@1847435210161d95b9c5fcd079380e7f2892195f) ([merge request](gitlab-org/security/gitlab!3975)) +- [Protect against ReDoS in FileFinder with wildcard filters](gitlab-org/security/gitlab@dc16f3baa640ca8d5b223782ef3d58369423a1dd) ([merge request](gitlab-org/security/gitlab!3969)) +- [fix: Validate security report version against schema during parsing](gitlab-org/security/gitlab@55e58d49051aa42938ec1d159b5e7eb3c47d2eb1) ([merge request](gitlab-org/security/gitlab!3967)) + ## 16.11.0 (2024-04-17) ### Added (121 changes) @@ -605,6 +615,20 @@ entry. - [Finalize the backfill migration for onboarding status step url](gitlab-org/gitlab@f986c1b1cf00968ff106136893bfe68d47895c69) ([merge request](gitlab-org/gitlab!147278)) - [Remove ClusterRepositoryCache migration helper class](gitlab-org/gitlab@f71a7a94ce8d70d9d378ebc225b802b58f0ae006) ([merge request](gitlab-org/gitlab!147244)) +## 16.10.4 (2024-04-24) + +### Fixed (1 change) + +- [Update vulnerability_reads scanner in the ingestion pipeline](gitlab-org/security/gitlab@14b8876233e5dd29149426fd88bab0fc4f014d46) **GitLab Enterprise Edition** + +### Security (5 changes) + +- [Validation for encoded formatting characters](gitlab-org/security/gitlab@4cd13c705ce1a94152fb2fd6fcaa77e90e6441e5) ([merge request](gitlab-org/security/gitlab!3950)) +- [Forbid untrusted sign-ins to GitLab with Bitbucket and fix related uid](gitlab-org/security/gitlab@5d3c3a599cc5560dea2236474309537536428cdc) ([merge request](gitlab-org/security/gitlab!3984)) +- [Ensure PAT scope is validated everywhere for GraphQL/ActionCable](gitlab-org/security/gitlab@079dfee8cff9da9075eec7c03ce002e87eeebfff) ([merge request](gitlab-org/security/gitlab!3976)) +- [Protect against ReDoS in FileFinder with wildcard filters](gitlab-org/security/gitlab@0e7e54050f1c4829b1d55aac85bd4e9cd96f1580) ([merge request](gitlab-org/security/gitlab!3960)) +- [fix: Validate security report version against schema during parsing](gitlab-org/security/gitlab@217040b1062caad501d60af387c47cff758788a1) ([merge request](gitlab-org/security/gitlab!3956)) + ## 16.10.3 (2024-04-12) No changes. @@ -1319,6 +1343,16 @@ No changes. - [Add sharding keys for system_access](gitlab-org/gitlab@62c2fd4788e62e46f1469e2f18d178840e8e3df2) ([merge request](gitlab-org/gitlab!142501)) - [Add sharding keys for purchase](gitlab-org/gitlab@9c3843da74714c72483c17489d5d3d68ceffd2c8) ([merge request](gitlab-org/gitlab!142505)) +## 16.9.6 (2024-04-24) + +### Security (5 changes) + +- [Validation for encoded formatting characters](gitlab-org/security/gitlab@de8dc151e5ef3f07cf50839e50645df6ec12f5a5) ([merge request](gitlab-org/security/gitlab!3951)) +- [Forbid untrusted sign-ins to GitLab with Bitbucket and fix related uid](gitlab-org/security/gitlab@94496a91c17a0f73202cd5c55abc93395825c68c) ([merge request](gitlab-org/security/gitlab!3985)) +- [Ensure PAT scope is validated everywhere for GraphQL/ActionCable](gitlab-org/security/gitlab@0dccf32b71614584e05a8590b21a902220e8c701) ([merge request](gitlab-org/security/gitlab!3977)) +- [Protect against ReDoS in FileFinder with wildcard filters](gitlab-org/security/gitlab@60a7418ec10f7c6f4ef9bcc75b2fec71255ddcc3) ([merge request](gitlab-org/security/gitlab!3961)) +- [fix: Validate security report version against schema during parsing](gitlab-org/security/gitlab@ce709ff78fd8f18024383085d6ac0bf43fa2efbb) ([merge request](gitlab-org/security/gitlab!3957)) + ## 16.9.5 (2024-04-12) No changes. diff --git a/Gemfile b/Gemfile index f6ecd52da52..13ff99f7b68 100644 --- a/Gemfile +++ b/Gemfile @@ -358,7 +358,6 @@ gem 'gitlab-license', '~> 2.4', feature_category: :shared gem 'rack-attack', '~> 6.7.0' # rubocop:todo Gemfile/MissingFeatureCategory # Sentry integration -gem 'sentry-raven', '~> 3.1', feature_category: :error_tracking gem 'sentry-ruby', '~> 5.10.0', feature_category: :error_tracking gem 'sentry-rails', '~> 5.10.0', feature_category: :error_tracking gem 'sentry-sidekiq', '~> 5.10.0', feature_category: :error_tracking @@ -411,7 +410,6 @@ group :opentelemetry do gem 'opentelemetry-instrumentation-action_view', feature_category: :tooling gem 'opentelemetry-instrumentation-aws_sdk', feature_category: :tooling gem 'opentelemetry-instrumentation-http', feature_category: :tooling - gem 'opentelemetry-instrumentation-active_model_serializers', feature_category: :tooling gem 'opentelemetry-instrumentation-concurrent_ruby', feature_category: :tooling gem 'opentelemetry-instrumentation-ethon', feature_category: :tooling gem 'opentelemetry-instrumentation-excon', feature_category: :tooling @@ -592,7 +590,7 @@ gem 'ssh_data', '~> 1.3' # rubocop:todo Gemfile/MissingFeatureCategory gem 'spamcheck', '~> 1.3.0' # rubocop:todo Gemfile/MissingFeatureCategory # Gitaly GRPC protocol definitions -gem 'gitaly', '~> 16.11.0.pre.rc1', feature_category: :gitaly +gem 'gitaly', '~> 17.0.0.pre.rc2', feature_category: :gitaly # KAS GRPC protocol definitions gem 'kas-grpc', '~> 0.4.0', feature_category: :deployment_management diff --git a/Gemfile.checksum b/Gemfile.checksum index 476c0f8f31e..3dbd7f5e4d9 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -206,7 +206,7 @@ {"name":"gettext","version":"3.4.9","platform":"ruby","checksum":"292864fe6a15c224cee4125a4a72fab426fdbb280e4cff3cfe44935f549b009a"}, {"name":"gettext_i18n_rails","version":"1.12.0","platform":"ruby","checksum":"6ac4817731a9e2ce47e1e83381ac34f9142263bc2911aaaafb2526d2f1afc1be"}, {"name":"git","version":"1.18.0","platform":"ruby","checksum":"c9b80462e4565cd3d7a9ba8440c41d2c52244b17b0dad0bfddb46de70630c465"}, -{"name":"gitaly","version":"16.11.0.pre.rc1","platform":"ruby","checksum":"24334f5f3fd5b6c3d278eea9fe2b6732dd08e87a4146cd4374615506b1a6e7ae"}, +{"name":"gitaly","version":"17.0.0.pre.rc2","platform":"ruby","checksum":"2fa5998d3cbc37ba66bef428fb7150ab3f8b4105a4bd29ea48d965fae98ead43"}, {"name":"gitlab","version":"4.19.0","platform":"ruby","checksum":"3f645e3e195dbc24f0834fbf83e8ccfb2056d8e9712b01a640aad418a6949679"}, {"name":"gitlab-chronic","version":"0.10.5","platform":"ruby","checksum":"f80f18dc699b708870a80685243331290bc10cfeedb6b99c92219722f729c875"}, {"name":"gitlab-dangerfiles","version":"4.7.0","platform":"ruby","checksum":"2576876a8dcb7290853fc3aef8048001cfe593b87318dd0016959d42e0e145ca"}, @@ -453,7 +453,6 @@ {"name":"opentelemetry-instrumentation-action_pack","version":"0.9.0","platform":"ruby","checksum":"c5df8472afc9cdbfc1425d9af7816b9cfc1a1a69b86621f1fc624974bd9acb9a"}, {"name":"opentelemetry-instrumentation-action_view","version":"0.7.0","platform":"ruby","checksum":"bc7c714be0b4bb76843085c29ecc9465e65cb7fe6722e34c71629e44f8c3cb75"}, {"name":"opentelemetry-instrumentation-active_job","version":"0.7.1","platform":"ruby","checksum":"da24806c9d92fe580db42638f6c763fe1324ff90aa147d45d4247f8052c68089"}, -{"name":"opentelemetry-instrumentation-active_model_serializers","version":"0.20.1","platform":"ruby","checksum":"8c47f859fc925c4c078d37f5a13c55f4ba9751f880aa64d0c9568f3f59a3efaa"}, {"name":"opentelemetry-instrumentation-active_record","version":"0.7.0","platform":"ruby","checksum":"327ca53ebb74187b463ab05c1d89508552e9cd9122db0843ad1f27930ee91797"}, {"name":"opentelemetry-instrumentation-active_support","version":"0.5.1","platform":"ruby","checksum":"03898327e8284410b8935a3d3b980bda56e2063eb5a7d30acf75487dd6934a66"}, {"name":"opentelemetry-instrumentation-aws_sdk","version":"0.5.1","platform":"ruby","checksum":"496a8d13c59ff4d08dcd69b16db97c013398173295058593aa0c2f3ef3090cce"}, @@ -623,7 +622,6 @@ {"name":"selenium-webdriver","version":"4.19.0","platform":"ruby","checksum":"4c8bd1d6016a456154b4ba71a3bb4d532a0ae185a38acf9cec0acbd38b4e5066"}, {"name":"semver_dialects","version":"2.0.2","platform":"ruby","checksum":"60059c9f416f931b5212d862fad2879d6b9affb8e0b9afb0d91b793639c116fe"}, {"name":"sentry-rails","version":"5.10.0","platform":"ruby","checksum":"99aa2fac136c26942eb1897c65de65dac88ad43ac5eb183ff20711287a137ebd"}, -{"name":"sentry-raven","version":"3.1.2","platform":"ruby","checksum":"103d3b122958810d34898ce2e705bcf549ddb9d855a70ce9a3970ee2484f364a"}, {"name":"sentry-ruby","version":"5.10.0","platform":"ruby","checksum":"115c24c0aee1309210f3a2988fb118e2bec1f11609feeda90e694388b1183619"}, {"name":"sentry-sidekiq","version":"5.10.0","platform":"ruby","checksum":"cc81018d0733fb1be3fb5641c9e0b61030bbeaa1d0b23ca64797d70def7aea1a"}, {"name":"sexp_processor","version":"4.17.1","platform":"ruby","checksum":"91110946720307f30bf1d549e90d9a529fef40d1fc471c069c8cca7667015da0"}, diff --git a/Gemfile.lock b/Gemfile.lock index da5d5dcac70..d6740b03c1d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -680,7 +680,7 @@ GEM git (1.18.0) addressable (~> 2.8) rchardet (~> 1.8) - gitaly (16.11.0.pre.rc1) + gitaly (17.0.0.pre.rc2) grpc (~> 1.0) gitlab (4.19.0) httparty (~> 0.20) @@ -1253,9 +1253,6 @@ GEM opentelemetry-instrumentation-active_job (0.7.1) opentelemetry-api (~> 1.0) opentelemetry-instrumentation-base (~> 0.22.1) - opentelemetry-instrumentation-active_model_serializers (0.20.1) - opentelemetry-api (~> 1.0) - opentelemetry-instrumentation-base (~> 0.22.1) opentelemetry-instrumentation-active_record (0.7.0) opentelemetry-api (~> 1.0) opentelemetry-instrumentation-base (~> 0.22.1) @@ -1669,8 +1666,6 @@ GEM sentry-rails (5.10.0) railties (>= 5.0) sentry-ruby (~> 5.10.0) - sentry-raven (3.1.2) - faraday (>= 1.0) sentry-ruby (5.10.0) concurrent-ruby (~> 1.0, >= 1.0.2) sentry-sidekiq (5.10.0) @@ -2008,7 +2003,7 @@ DEPENDENCIES gdk-toogle (~> 0.9) gettext (~> 3.4, >= 3.4.9) gettext_i18n_rails (~> 1.12.0) - gitaly (~> 16.11.0.pre.rc1) + gitaly (~> 17.0.0.pre.rc2) gitlab-backup-cli! gitlab-chronic (~> 0.10.5) gitlab-dangerfiles (~> 4.7.0) @@ -2142,7 +2137,6 @@ DEPENDENCIES opentelemetry-instrumentation-action_pack opentelemetry-instrumentation-action_view opentelemetry-instrumentation-active_job - opentelemetry-instrumentation-active_model_serializers opentelemetry-instrumentation-active_record opentelemetry-instrumentation-active_support opentelemetry-instrumentation-aws_sdk @@ -2224,7 +2218,6 @@ DEPENDENCIES selenium-webdriver (~> 4.19) semver_dialects (~> 2.0, >= 2.0.2) sentry-rails (~> 5.10.0) - sentry-raven (~> 3.1) sentry-ruby (~> 5.10.0) sentry-sidekiq (~> 5.10.0) shoulda-matchers (~> 5.1.0) diff --git a/app/assets/javascripts/ci/catalog/components/details/ci_resource_header.vue b/app/assets/javascripts/ci/catalog/components/details/ci_resource_header.vue index a027363a0c2..3001bb2e1ae 100644 --- a/app/assets/javascripts/ci/catalog/components/details/ci_resource_header.vue +++ b/app/assets/javascripts/ci/catalog/components/details/ci_resource_header.vue @@ -13,6 +13,7 @@ import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { cleanLeadingSeparator } from '~/lib/utils/url_utility'; import { formatDate } from '~/lib/utils/datetime_utility'; import AbuseCategorySelector from '~/abuse_reports/components/abuse_category_selector.vue'; +import Markdown from '~/vue_shared/components/markdown/non_gfm_markdown.vue'; import CiVerificationBadge from '../shared/ci_verification_badge.vue'; import CiResourceAbout from './ci_resource_about.vue'; import CiResourceHeaderSkeletonLoader from './ci_resource_header_skeleton_loader.vue'; @@ -35,6 +36,7 @@ export default { GlDisclosureDropdown, GlDisclosureDropdownItem, GlLink, + Markdown, }, directives: { GlTooltip: GlTooltipDirective, @@ -198,9 +200,7 @@ export default { v-if="isLoadingSharedData" class="gl-animate-skeleton-loader gl-h-4 gl-rounded-base gl-my-3 gl-max-w-20!" > -
- {{ resource.description }} -
+{{ query.repository_ref }}
diff --git a/app/assets/javascripts/sentry/init_sentry.js b/app/assets/javascripts/sentry/init_sentry.js
index 722741b50e4..8c8d8b655de 100644
--- a/app/assets/javascripts/sentry/init_sentry.js
+++ b/app/assets/javascripts/sentry/init_sentry.js
@@ -1,3 +1,4 @@
+/* eslint-disable no-restricted-imports */
import {
BrowserClient,
getCurrentHub,
@@ -9,7 +10,7 @@ import {
// exports
captureException,
SDK_VERSION,
-} from 'sentrybrowser';
+} from '@sentry/browser';
const initSentry = () => {
if (!gon?.sentry_dsn) {
diff --git a/app/assets/javascripts/sentry/legacy_constants.js b/app/assets/javascripts/sentry/legacy_constants.js
deleted file mode 100644
index d04011dab2f..00000000000
--- a/app/assets/javascripts/sentry/legacy_constants.js
+++ /dev/null
@@ -1,46 +0,0 @@
-import { __ } from '~/locale';
-
-// https://docs.sentry.io/platforms/javascript/configuration/filtering/#decluttering-sentry
-export const IGNORE_ERRORS = [
- // Random plugins/extensions
- 'top.GLOBALS',
- // See: http://blog.errorception.com/2012/03/tale-of-unfindable-js-error. html
- 'originalCreateNotification',
- 'canvas.contentDocument',
- 'MyApp_RemoveAllHighlights',
- 'http://tt.epicplay.com',
- __("Can't find variable: ZiteReader"),
- __('jigsaw is not defined'),
- __('ComboSearch is not defined'),
- 'http://loading.retry.widdit.com/',
- 'atomicFindClose',
- // Facebook borked
- 'fb_xd_fragment',
- // ISP "optimizing" proxy - `Cache-Control: no-transform` seems to
- // reduce this. (thanks @acdha)
- 'bmi_SafeAddOnload',
- 'EBCallBackMessageReceived',
- // See http://toolbar.conduit.com/Developer/HtmlAndGadget/Methods/JSInjection.aspx
- 'conduitPage',
- // Exclude errors from polling when navigating away from a page
- 'TypeError: Failed to fetch',
-];
-
-export const DENY_URLS = [
- // Facebook flakiness
- /graph\.facebook\.com/i,
- // Facebook blocked
- /connect\.facebook\.net\/en_US\/all\.js/i,
- // Woopra flakiness
- /eatdifferent\.com\.woopra-ns\.com/i,
- /static\.woopra\.com\/js\/woopra\.js/i,
- // Chrome extensions
- /extensions\//i,
- /^chrome:\/\//i,
- // Other plugins
- /127\.0\.0\.1:4001\/isrunning/i, // Cacaoweb
- /webappstoolbarba\.texthelp\.com\//i,
- /metrics\.itunes\.apple\.com\.edgesuite\.net\//i,
-];
-
-export const SAMPLE_RATE = 0.95;
diff --git a/app/assets/javascripts/sentry/legacy_index.js b/app/assets/javascripts/sentry/legacy_index.js
deleted file mode 100644
index 688f8eb0a44..00000000000
--- a/app/assets/javascripts/sentry/legacy_index.js
+++ /dev/null
@@ -1,34 +0,0 @@
-import '../webpack';
-
-import * as Sentry5 from 'sentrybrowser5';
-import LegacySentryConfig from './legacy_sentry_config';
-
-const index = function index() {
- // Configuration for legacy versions of Sentry SDK (v5)
- LegacySentryConfig.init({
- dsn: gon.sentry_dsn,
- currentUserId: gon.current_user_id,
- whitelistUrls:
- process.env.NODE_ENV === 'production'
- ? [gon.gitlab_url]
- : [gon.gitlab_url, 'webpack-internal://'],
- environment: gon.sentry_environment,
- release: gon.revision,
- tags: {
- revision: gon.revision,
- feature_category: gon.feature_category,
- },
- });
-};
-
-index();
-
-// The _Sentry object is globally exported so it can be used by
-// ./sentry_browser_wrapper.js
-// This hack allows us to load a single version of `~/sentry/sentry_browser_wrapper`
-// in the browser, see app/views/layouts/_head.html.haml to find how it is imported.
-
-// eslint-disable-next-line no-underscore-dangle
-window._Sentry = Sentry5;
-
-export default index;
diff --git a/app/assets/javascripts/sentry/legacy_sentry_config.js b/app/assets/javascripts/sentry/legacy_sentry_config.js
deleted file mode 100644
index 137c31f5526..00000000000
--- a/app/assets/javascripts/sentry/legacy_sentry_config.js
+++ /dev/null
@@ -1,64 +0,0 @@
-import * as Sentry5 from 'sentrybrowser5';
-import $ from 'jquery';
-import { __ } from '~/locale';
-import { IGNORE_ERRORS, DENY_URLS, SAMPLE_RATE } from './legacy_constants';
-
-const SentryConfig = {
- IGNORE_ERRORS,
- DENYLIST_URLS: DENY_URLS,
- SAMPLE_RATE,
- init(options = {}) {
- this.options = options;
-
- this.configure();
- this.bindSentryErrors();
- if (this.options.currentUserId) this.setUser();
- },
-
- configure() {
- const { dsn, release, tags, whitelistUrls, environment } = this.options;
-
- Sentry5.init({
- dsn,
- release,
- whitelistUrls,
- environment,
- ignoreErrors: this.IGNORE_ERRORS, // TODO: Remove in favor of https://gitlab.com/gitlab-org/gitlab/issues/35144
- blacklistUrls: this.DENYLIST_URLS,
- sampleRate: SAMPLE_RATE,
- });
-
- Sentry5.setTags(tags);
- },
-
- setUser() {
- Sentry5.setUser({
- id: this.options.currentUserId,
- });
- },
-
- bindSentryErrors() {
- $(document).on('ajaxError.sentry', this.handleSentryErrors);
- },
-
- handleSentryErrors(event, req, config, err) {
- const error = err || req.statusText;
- const { responseText = __('Unknown response text') } = req;
- const { type, url, data } = config;
- const { status } = req;
-
- Sentry5.captureMessage(error, {
- extra: {
- type,
- url,
- data,
- status,
- response: responseText,
- error,
- event,
- },
- });
- },
-};
-
-export default SentryConfig;
diff --git a/app/assets/javascripts/single_file_diff.js b/app/assets/javascripts/single_file_diff.js
index 1e01da795e8..a3b10f3a77b 100644
--- a/app/assets/javascripts/single_file_diff.js
+++ b/app/assets/javascripts/single_file_diff.js
@@ -1,6 +1,8 @@
import $ from 'jquery';
+import { GlButton } from '@gitlab/ui';
import { createAlert } from '~/alert';
import { loadingIconForLegacyJS } from '~/loading_icon_for_legacy_js';
+import { renderVueComponentForLegacyJS } from '~/render_vue_component_for_legacy_js';
import { spriteIcon } from '~/lib/utils/common_utils';
import FilesCommentButton from './files_comment_button';
import initImageDiffHelper from './image_diff/helpers/init_image_diff';
@@ -14,8 +16,15 @@ const ERROR_HTML = `bar
static: |-bar
wysiwyg: |-Foo bar
Bar foo
wysiwyg: |-Foo bar
@@ -575,9 +575,9 @@ static: |- - - - +of dashes"/>
static: |-`
of dashes"/>
wysiwyg: |-Baz
wysiwyg: |-Foo
@@ -878,7 +878,7 @@
Foo
baz
wysiwyg: |-Foo
@@ -1090,13 +1090,13 @@foo
foo
bar
bar
@@ -2345,7 +2345,7 @@
static: |-
bar
@@ -2360,7 +2360,7 @@ static: |-- bar
+ bar wysiwyg: |-[foo]: /url@@ -2510,7 +2510,7 @@ static: |-aaa
- aaa
+ aaa wysiwyg: |-aaa
aaa
@@ -2784,7 +2784,7 @@ static: |-@@ -2801,7 +2801,7 @@ static: |-- Foo
+ Foobar baz
@@ -2818,7 +2818,7 @@ static: |-- Foo
+ Foobar baz
@@ -2852,7 +2852,7 @@ static: |-- Foo
+ Foobar baz
@@ -4142,11 +4142,11 @@- Foo
+ Foobar baz
wysiwyg: |- @@ -7572,7 +7572,7 @@
- Foo
+ Foo- Bar
+ Bar bazfoo\
static: |-- foo\
+ foo\ wysiwyg: |-foo\
06_13_00__inlines__hard_line_breaks__015: @@ -7580,7 +7580,7 @@foo
static: |-- foo
+ foo wysiwyg: |-foo
06_14_00__inlines__soft_line_breaks__001: @@ -7753,7 +7753,7 @@text
- title: YAML front matter
+ title: YAML front matter wysiwyg: |-text
@@ -7765,7 +7765,7 @@ static: |-
- title: YAML front matter
+ title: YAML front matter wysiwyg: |-
title: YAML front matter
@@ -7786,9 +7786,9 @@ Heading 1- Heading 1
+ Heading 1- Heading 2
+ Heading 2 wysiwyg: |-Table of contentsHeading 1
@@ -7810,9 +7810,9 @@ Heading 1- Heading 1
+ Heading 1- Heading 2
+ Heading 2 wysiwyg: |-Table of contentsHeading 1
@@ -7842,7 +7842,7 @@ static: |-- Heading 1
+ Heading 1 wysiwyg: |-Table of contentsHeading 1
@@ -8292,17 +8292,17 @@ TODO: Write canonical HTML for this example static: |-- Heading 1
+ Heading 1- Heading 2
+ Heading 2- Heading 3
+ Heading 3- Heading 4
+ Heading 4- Heading 5
+ Heading 5- Heading 6
+ Heading 6 wysiwyg: |-Heading 1
Heading 2
@@ -8517,7 +8517,7 @@- content after table
+ content after table wysiwyg: |-
header
header
codecell with bold
strikecell with italic
content after table
@@ -8536,16 +8536,16 @@- Lorem
+ LoremWell, that's just like... your opinion.. man.
- Ipsum
+ Ipsum- Dolar
+ Dolar- Sit amit
+ Sit amit- I don't know
+ I don't know wysiwyg: |-Table of contentsLorem
diff --git a/glfm_specification/output_example_snapshots/snapshot_spec.html b/glfm_specification/output_example_snapshots/snapshot_spec.html index e8c3551ea20..b176e0ebf8c 100644 --- a/glfm_specification/output_example_snapshots/snapshot_spec.html +++ b/glfm_specification/output_example_snapshots/snapshot_spec.html @@ -238,148 +238,11 @@GitLab Flavored Markdown Internal Extensions
Version alpha--
+- -Preliminaries -
-- -Blocks and inlines -
-- -Leaf blocks -
-- -Container blocks -
-- -Inlines -
-- -GitLab Official Specification Markdown -
-- -GitLab Internal Extension Markdown
--
-- Audio
-- Video
-- Markdown Preview API Request Overrides
-- -Migrated golden master examples
--
-- attachment_image_for_group
-- attachment_image_for_project
-- attachment_image_for_project_wiki
-- attachment_link_for_group
-- attachment_link_for_project
-- attachment_link_for_project_wiki
-- attachment_link_for_group_wiki
-- audio
-- audio_and_video_in_lists
-- blockquote
-- bold
-- bullet_list_style_1
-- bullet_list_style_2
-- bullet_list_style_3
-- code_block_javascript
-- code_block_plaintext
-- code_block_unknown
-- color_chips
-- description_list
-- details
-- diagram_kroki_nomnoml
-- diagram_plantuml
-- diagram_plantuml_unicode
-- div
-- emoji
-- emphasis
-- figure
-- footnotes
-- frontmatter_json
-- frontmatter_toml
-- frontmatter_yaml
-- hard_break
-- headings
-- horizontal_rule
-- html_marks
-- image
-- inline_code
-- inline_diff
-- label
-- link
-- math
-- ordered_list
-- ordered_list_with_start_order
-- ordered_task_list
-- ordered_task_list_with_order
-- reference_for_project_wiki
-- strike
-- table
-- table_of_contents
-- task_list
-- video
-- word_break
-- Image Attributes
-- Footnotes
-- -GFM undocumented extensions and more robust test -
--Preliminaries
+Preliminaries-Characters and lines
+Characters and linesAny sequence of [characters] is a valid CommonMark document.
A character is a Unicode code point. Although some @@ -422,7 +285,7 @@ is
!,Pc,Pd,Pe,Pf,Pi,Po, orPs.-Tabs
+TabsTabs in lines are not expanded to [spaces]. However, in contexts where whitespace helps to define block structure, tabs behave as if they were replaced by spaces with a tab stop @@ -607,11 +470,11 @@ code block starting with two spaces.
For security reasons, the Unicode character U+0000 must be replaced
with the REPLACEMENT CHARACTER (U+FFFD).
We can think of a document as a sequence of blocks---structural elements like paragraphs, block quotations, lists, headings, rules, and code blocks. Some blocks (like @@ -619,7 +482,7 @@ block quotes and list items) contain other blocks; others (like headings and paragraphs) contain inline content---text, links, emphasized text, images, code spans, and so on.
Indicators of block structure always take precedence over indicators of inline structure. So, for example, the following is a list with two items, not a list with one item containing a code span:
@@ -647,17 +510,17 @@ step. Note that the first step requires processing lines in sequence, but the second can be parallelized, since the inline parsing of one block element does not affect the inline parsing of any other.We can divide blocks into two types: container blocks, which can contain other blocks, and leaf blocks, which cannot.
This section describes the different kinds of leaf block that make up a Markdown document.
A line consisting of 0-3 spaces of indentation, followed by a sequence
of three or more matching -, _, or * characters, each followed
optionally by any number of spaces or tabs, forms a
@@ -942,7 +805,7 @@ interpretations of a line, the thematic break takes precedence:
An ATX heading
consists of a string of characters, parsed as inline content, between an
opening sequence of 1--6 unescaped # characters and an optional
@@ -1219,7 +1082,7 @@ lines, and they can interrupt paragraphs:
A setext heading consists of one or more lines of text, each containing at least one [non-whitespace character], with no more than 3 spaces indentation, followed by @@ -1720,7 +1583,7 @@ underline], such as
An indented code block is composed of one or more [indented chunks] separated by blank lines. An indented chunk is a sequence of non-blank lines, @@ -1954,7 +1817,7 @@ are not included in it:
A code fence is a sequence
of at least three consecutive backtick characters (`) or
tildes (~). (Tildes and backticks cannot be mixed.)
@@ -2471,7 +2334,7 @@ of language- followed by the langua
An HTML block is a group of lines that is treated as raw HTML (and will not be escaped in HTML output).
There are seven kinds of [HTML block], which can be defined by their
@@ -3388,7 +3251,7 @@ deleted. The exception is inside <pre
[above][HTML blocks], raw HTML blocks starting with <pre>
can contain blank lines.
A link reference definition
consists of a [link label], indented up to three spaces, followed
by a colon (:), optional [whitespace] (including up to one
@@ -3846,7 +3709,7 @@ no visible content:
A sequence of non-blank lines that cannot be interpreted as other kinds of blocks forms a paragraph. The contents of the paragraph are the result of parsing the @@ -3980,7 +3843,7 @@ break]:
[Blank lines] between block-level elements are ignored, except for the role they play in determining whether a [list] is [tight] or [loose].
@@ -4006,7 +3869,7 @@ is [tight] or [loose].GFM enables the table extension, where an additional leaf block type is
available.
A table is an arrangement of data with rows and columns, consisting of a @@ -4243,7 +4106,7 @@ cells are inserted. If there are greater, the excess is ignored:
A container block is a block that has other blocks as its contents. There are two basic kinds of container blocks: [block quotes] and [list items]. @@ -4261,7 +4124,7 @@ to define the syntax, although it does not give a recipe for A parsing strategy.)
A block quote marker
consists of 0-3 spaces of initial indent, plus (a) the character > together
with a following space, or (b) a single character > not followed by a space.
>:
A list marker is a [bullet list marker] or an [ordered list marker].
A bullet list marker @@ -5857,7 +5720,7 @@ in the list item.
John Gruber's Markdown spec says the following about list items:
GFM enables the tasklist extension, where an additional processing step is
performed on [list items].
A task list item is a [list item][list items] where the first block in it @@ -6091,7 +5954,7 @@ the final rendered document.
A list is a sequence of one or more list items [of the same type]. The list items may be separated by any number of blank lines.
@@ -6801,7 +6664,7 @@ two block elements in the list item:Inlines are parsed sequentially from the beginning of the character stream to the end (left to right, in left-to-right languages). Thus, for example, in
@@ -6819,7 +6682,7 @@ Thus, for example, inhi is parsed as code, leaving the backtick at the end as a literal
backtick.
Any ASCII punctuation character may be backslash-escaped:
Valid HTML entity references and numeric character references can be used in place of the corresponding Unicode character, with the following exceptions:
@@ -7267,7 +7130,7 @@ documents.A backtick string
is a string of one or more backtick characters (`) that is neither
preceded nor followed by a backtick.
John Gruber's original Markdown syntax description says:
@@ -9384,7 +9247,7 @@ delimiters:-Strikethrough (extension)
+Strikethrough (extension)GFM enables the
strikethroughextension, where an additional emphasis type is available.Strikethrough text is any text wrapped in two tildes (
@@ -9417,7 +9280,7 @@ parsing to cease:~).-Links
+LinksA link contains [link text] (the visible text), a [link destination] (the URI that is the link destination), and optionally a [link title]. There are two basic kinds of links in Markdown. In [inline links] the @@ -10762,7 +10625,7 @@ is followed by a link label (even though
-Images +ImagesSyntax for images is like the syntax for links, with one difference. Instead of [link text], we have an image description. The rules for this are the @@ -11065,7 +10928,7 @@ backslash-escape the opening
[:<-Autolinks
+AutolinksAutolinks are absolute URIs and email addresses inside
@@ -11317,7 +11180,7 @@ spec:<and>. They are parsed as links, with the URL or email address as the link label.-Autolinks (extension)
+Autolinks (extension)GFM enables the
autolinkextension, where autolinks will be recognised in a greater number of conditions.[Autolink]s can also be constructed without requiring the use of
<and to>@@ -11523,7 +11386,7 @@ the address:-Raw HTML
+Raw HTMLText between
<and>that looks like an HTML tag is parsed as a raw HTML tag and will be rendered in HTML without escaping. Tag and attribute names are not limited to current HTML tags, @@ -11846,7 +11709,7 @@ attributes:-Disallowed Raw HTML (extension)
+Disallowed Raw HTML (extension)GFM enables the
tagfilterextension, where the following HTML tags will be filtered when rendering HTML output:@@ -11885,7 +11748,7 @@ usually undesireable in the context of other rendered Markdown content.
-Hard line breaks
+Hard line breaksA line break (not in a code span or HTML tag) that is preceded by two or more spaces and does not occur at the end of a block is parsed as a hard line break (rendered @@ -12087,7 +11950,7 @@ other block element:
-Soft line breaks
+Soft line breaksA regular line break (not in a code span or HTML tag) that is not preceded by two or more spaces or a backslash is parsed as a softbreak. (A softbreak may be rendered in HTML either as a @@ -12126,7 +11989,7 @@ line break or as a space.
A renderer may also provide an option to render soft line breaks as hard line breaks.
-Textual content
+Textual contentAny characters not given an interpretation by the above rules will be parsed as plain textual content.
@@ -12164,14 +12027,14 @@ be parsed as plain textual content.-GitLab Official Specification Markdown
+GitLab Official Specification MarkdownNote: This specification is a work in progress. Only some of the official GLFM extensions are defined. We will continue to add any additional ones found in the user-facing documentation for GitLab Flavored Markdown.
There is currently only this single top-level heading, but the examples may be split into multiple top-level headings in the future.
-Task list items
+Task list itemsSee Task lists in the GitLab Flavored Markdown documentation.
Task list items (checkboxes) are defined as a GitHub Flavored Markdown extension in a section above. @@ -12265,7 +12128,7 @@ loose text; it has strikethrough applied with CSS.
-Front matter
+Front matterSee Front matter in the GitLab Flavored Markdown documentation.
Front matter is metadata included at the beginning of a Markdown document, preceding the content. @@ -12362,7 +12225,7 @@ This data can be used by static site generators like Jekyll, Hugo, and many othe
-Table of contents
+Table of contentsSee table of contents in the GitLab Flavored Markdown documentation.
@@ -12459,9 +12322,9 @@ line.-GitLab Internal Extension Markdown
+GitLab Internal Extension Markdown-Audio
+AudioSee audio in the GitLab Flavored Markdown documentation.
GLFM renders image elements as an audio player as long as the resource’s file extension is @@ -12493,7 +12356,7 @@ Audio ignore the alternative text part of an image declaration.
-Video
+VideoSee videos in the GitLab Flavored Markdown documentation.
GLFM renders image elements as a video player as long as the resource’s file extension is @@ -12525,7 +12388,7 @@ Videos ignore the alternative text part of an image declaration.
-Markdown Preview API Request Overrides
+Markdown Preview API Request OverridesThis section contains examples of all controllers which use
@@ -12606,9 +12469,9 @@ also requires an EE license enabling thePreviewMarkdownmodule and use differentmarkdown_context_params. They exercise the variouspreview_markdownendpoints viaglfm_example_metadata.yml.-Migrated golden master examples +Migrated golden master examples-attachment_image_for_group
+attachment_image_for_group@@ -12621,7 +12484,7 @@ also requires an EE license enabling the-attachment_image_for_project +attachment_image_for_project@@ -12634,7 +12497,7 @@ also requires an EE license enabling the-attachment_image_for_project_wiki +attachment_image_for_project_wiki@@ -12647,7 +12510,7 @@ also requires an EE license enabling the-attachment_link_for_group +attachment_link_for_group@@ -12660,7 +12523,7 @@ also requires an EE license enabling the-attachment_link_for_project +attachment_link_for_project@@ -12673,7 +12536,7 @@ also requires an EE license enabling the-attachment_link_for_project_wiki +attachment_link_for_project_wiki@@ -12686,7 +12549,7 @@ also requires an EE license enabling the-attachment_link_for_group_wiki +attachment_link_for_group_wiki@@ -12699,7 +12562,7 @@ also requires an EE license enabling the-audio +audio@@ -12712,7 +12575,7 @@ also requires an EE license enabling the-audio_and_video_in_lists +audio_and_video_in_lists@@ -12733,7 +12596,7 @@ also requires an EE license enabling the-blockquote +blockquote@@ -12748,7 +12611,7 @@ also requires an EE license enabling the-bold +bold@@ -12761,7 +12624,7 @@ also requires an EE license enabling the-bullet_list_style_1 +bullet_list_style_1@@ -12776,7 +12639,7 @@ also requires an EE license enabling the-bullet_list_style_2 +bullet_list_style_2@@ -12791,7 +12654,7 @@ also requires an EE license enabling the-bullet_list_style_3 +bullet_list_style_3@@ -12806,7 +12669,7 @@ also requires an EE license enabling the-code_block_javascript +code_block_javascript@@ -12821,7 +12684,7 @@ also requires an EE license enabling the-code_block_plaintext +code_block_plaintext@@ -12836,7 +12699,7 @@ also requires an EE license enabling the-code_block_unknown +code_block_unknown@@ -12851,7 +12714,7 @@ also requires an EE license enabling the-color_chips +color_chips@@ -12872,7 +12735,7 @@ also requires an EE license enabling the-description_list +description_list@@ -12900,7 +12763,7 @@ also requires an EE license enabling the-details +details@@ -12919,7 +12782,7 @@ also requires an EE license enabling the-diagram_kroki_nomnoml +diagram_kroki_nomnoml@@ -12942,7 +12805,7 @@ also requires an EE license enabling the-diagram_plantuml +diagram_plantuml@@ -12961,7 +12824,7 @@ also requires an EE license enabling the-diagram_plantuml_unicode +diagram_plantuml_unicode@@ -12976,7 +12839,7 @@ also requires an EE license enabling the-div +div@@ -12994,7 +12857,7 @@ also requires an EE license enabling the-emoji +emoji@@ -13007,7 +12870,7 @@ also requires an EE license enabling the-emphasis +emphasis@@ -13020,7 +12883,7 @@ also requires an EE license enabling the-figure +figure@@ -13048,7 +12911,7 @@ also requires an EE license enabling the-footnotes +footnotes@@ -13067,7 +12930,7 @@ also requires an EE license enabling the-frontmatter_json +frontmatter_json@@ -13084,7 +12947,7 @@ also requires an EE license enabling the-frontmatter_toml +frontmatter_toml@@ -13099,7 +12962,7 @@ also requires an EE license enabling the-frontmatter_yaml +frontmatter_yaml@@ -13114,7 +12977,7 @@ also requires an EE license enabling the-hard_break +hard_break@@ -13128,7 +12991,7 @@ also requires an EE license enabling the-headings +headings@@ -13151,7 +13014,7 @@ also requires an EE license enabling the-horizontal_rule +horizontal_rule@@ -13164,7 +13027,7 @@ also requires an EE license enabling the-html_marks +html_marks@@ -13191,7 +13054,7 @@ also requires an EE license enabling the-image +image@@ -13204,7 +13067,7 @@ also requires an EE license enabling the-inline_code +inline_code@@ -13217,7 +13080,7 @@ also requires an EE license enabling the-inline_diff +inline_diff@@ -13231,7 +13094,7 @@ also requires an EE license enabling the-label +label@@ -13244,7 +13107,7 @@ also requires an EE license enabling the-link +link@@ -13257,7 +13120,7 @@ also requires an EE license enabling the-math +math@@ -13276,7 +13139,7 @@ also requires an EE license enabling the-ordered_list +ordered_list@@ -13291,7 +13154,7 @@ also requires an EE license enabling the-ordered_list_with_start_order +ordered_list_with_start_order@@ -13306,7 +13169,7 @@ also requires an EE license enabling the-ordered_task_list +ordered_task_list@@ -13324,7 +13187,7 @@ also requires an EE license enabling the-ordered_task_list_with_order +ordered_task_list_with_order@@ -13339,7 +13202,7 @@ also requires an EE license enabling the-reference_for_project_wiki +reference_for_project_wiki@@ -13352,7 +13215,7 @@ also requires an EE license enabling the-strike +strike@@ -13365,7 +13228,7 @@ also requires an EE license enabling the-table +table@@ -13383,7 +13246,7 @@ also requires an EE license enabling the-table_of_contents +table_of_contents@@ -13408,7 +13271,7 @@ also requires an EE license enabling the-task_list +task_list@@ -13426,7 +13289,7 @@ also requires an EE license enabling the-video +video@@ -13439,7 +13302,7 @@ also requires an EE license enabling the-word_break +word_break@@ -13452,7 +13315,7 @@ also requires an EE license enabling the-Image Attributes +Image AttributesSee Change the image dimensions in the GitLab Flavored Markdown documentation.
@@ -13520,7 +13383,7 @@ where it makes sense.-Footnotes
+FootnotesSee the footnotes section of the user-facing documentation for GitLab Flavored Markdown.
@@ -13555,12 +13418,12 @@ where it makes sense.-GFM undocumented extensions and more robust test
+GFM undocumented extensions and more robust testThis section contains tests borrowed from https://github.com/github/cmark-gfm/blob/master/test/extensions.txt. It includes items not found in the official GFM specification, such as footnotes and additional tests for tables, task lists, etc.
-Footnotes
+Footnotes@@ -13625,7 +13488,7 @@ task lists, etc.-When a footnote is used multiple times, we insert multiple backrefs.
+When a footnote is used multiple times, we insert multiple backrefs.@@ -13650,7 +13513,7 @@ task lists, etc.-Footnote reference labels are href escaped
+Footnote reference labels are href escaped@@ -13672,7 +13535,7 @@ task lists, etc.-Interop
+InteropAutolink and strikethrough.
@@ -13716,7 +13579,7 @@ task lists, etc.-Task lists
+Task listsdiff --git a/glfm_specification/output_spec/spec.html b/glfm_specification/output_spec/spec.html index 07af9a162d5..78e1e572138 100644 --- a/glfm_specification/output_spec/spec.html +++ b/glfm_specification/output_spec/spec.html @@ -253,7 +253,7 @@ version: alpha ...-Introduction
+IntroductionGitLab Flavored Markdown (GLFM) extends the CommonMark specification and is considered a strict superset of CommonMark. It also incorporates the extensions defined by the GitHub Flavored Markdown specification.
This specification will define the various official extensions that comprise GLFM. These extensions are GitLab independent - they do not require a GitLab server for parsing or interaction. The intent is to provide a specification that can be implemented in standard markdown editors. This includes many of the features listed in user-facing documentation for GitLab Flavored Markdown.
The CommonMark and GitHub specifications will not be duplicated here.
@@ -269,14 +269,14 @@ for a complete list of all examples, which are a superset of examples from:-GitLab Official Specification Markdown
+GitLab Official Specification MarkdownNote: This specification is a work in progress. Only some of the official GLFM extensions are defined. We will continue to add any additional ones found in the user-facing documentation for GitLab Flavored Markdown.
There is currently only this single top-level heading, but the examples may be split into multiple top-level headings in the future.
-Task list items
+Task list itemsSee Task lists in the GitLab Flavored Markdown documentation.
Task list items (checkboxes) are defined as a GitHub Flavored Markdown extension in a section above. @@ -370,7 +370,7 @@ loose text; it has strikethrough applied with CSS.
-Front matter
+Front matterSee Front matter in the GitLab Flavored Markdown documentation.
Front matter is metadata included at the beginning of a Markdown document, preceding the content. @@ -467,7 +467,7 @@ This data can be used by static site generators like Jekyll, Hugo, and many othe
-Table of contents
+Table of contentsSee table of contents in the GitLab Flavored Markdown documentation.
diff --git a/lib/banzai/filter/markdown_engines/glfm_markdown.rb b/lib/banzai/filter/markdown_engines/glfm_markdown.rb index cd0dfa0ab06..6743e3a9037 100644 --- a/lib/banzai/filter/markdown_engines/glfm_markdown.rb +++ b/lib/banzai/filter/markdown_engines/glfm_markdown.rb @@ -15,6 +15,7 @@ module Banzai full_info_string: true, github_pre_lang: true, hardbreaks: false, + header_ids: Banzai::Renderer::USER_CONTENT_ID_PREFIX, math_code: true, math_dollars: true, multiline_block_quotes: true, @@ -35,7 +36,16 @@ module Banzai private def render_options - sourcepos_disabled? ? OPTIONS.merge(sourcepos: false) : OPTIONS + return OPTIONS unless sourcepos_disabled? || headers_disabled? + + OPTIONS.merge( + sourcepos: !sourcepos_disabled?, + header_ids: headers_disabled? ? nil : OPTIONS[:header_ids] + ) + end + + def headers_disabled? + context[:no_header_anchors] || Feature.disabled?(:native_header_anchors) end end end diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb index da976cf9ddc..577d02cc559 100644 --- a/lib/banzai/filter/sanitization_filter.rb +++ b/lib/banzai/filter/sanitization_filter.rb @@ -25,11 +25,16 @@ module Banzai # Remove any `style` properties not required for table alignment allowlist[:transformers].push(self.class.remove_unsafe_table_style) - # Allow `id` in a and li elements for footnotes - # and remove any `id` properties not matching for footnotes + # Allow `id` in `a` and `li` elements for footnotes + # and `a` elements for header anchors. + # Remove any `id` properties not matching allowlist[:attributes]['a'].push('id') allowlist[:attributes]['li'] = %w[id] - allowlist[:transformers].push(self.class.remove_non_footnote_ids) + allowlist[:transformers].push(self.class.remove_id_attributes) + + # Remove any `class` property not required for `a` + allowlist[:attributes]['a'].push('class') + allowlist[:transformers].push(self.class.remove_unsafe_link_class) # Allow section elements with data-footnotes attribute allowlist[:elements].push('section') @@ -55,18 +60,38 @@ module Banzai end end - def remove_non_footnote_ids + def remove_unsafe_link_class + lambda do |env| + node = env[:node] + + return unless node.name == 'a' + return unless node.has_attribute?('class') + + node.remove_attribute('class') if remove_link_class?(node) + end + end + + def remove_link_class?(node) + return if node['class'] == 'anchor' + + true + end + + def remove_id_attributes lambda do |env| node = env[:node] return unless node.name == 'a' || node.name == 'li' return unless node.has_attribute?('id') + # footnote ids should not be removed + return if node.name == 'li' && node['id'].start_with?(Banzai::Filter::FootnoteFilter::FOOTNOTE_ID_PREFIX) return if node.name == 'a' && node['id'].start_with?(Banzai::Filter::FootnoteFilter::FOOTNOTE_LINK_ID_PREFIX) - return if node.name == 'li' && - node['id'].start_with?(Banzai::Filter::FootnoteFilter::FOOTNOTE_ID_PREFIX) + # links with generated header anchors should not be removed + return if node.name == 'a' && node['class'] == 'anchor' && + node['id'].start_with?(Banzai::Renderer::USER_CONTENT_ID_PREFIX) node.remove_attribute('id') end diff --git a/lib/banzai/filter/table_of_contents_filter.rb b/lib/banzai/filter/table_of_contents_filter.rb index 3e6505c82c4..357b880b74f 100644 --- a/lib/banzai/filter/table_of_contents_filter.rb +++ b/lib/banzai/filter/table_of_contents_filter.rb @@ -2,6 +2,11 @@ require 'cgi/util' +# TODO: This is now a legacy filter, and is only used with the Ruby parser. +# The current markdown parser now properly handles adding anchors to headers. +# The Ruby parser is now only for benchmarking purposes. +# issue: https://gitlab.com/gitlab-org/gitlab/-/issues/454601 + # Generated HTML is transformed back to GFM by app/assets/javascripts/behaviors/markdown/nodes/table_of_contents.js module Banzai module Filter @@ -25,6 +30,7 @@ module Banzai XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS).freeze def call + return doc if MarkdownFilter.glfm_markdown?(context) && Feature.enabled?(:native_header_anchors) return doc if context[:no_header_anchors] result[:toc] = +"" diff --git a/lib/banzai/filter/table_of_contents_tag_filter.rb b/lib/banzai/filter/table_of_contents_tag_filter.rb index 4e80b543e2d..b7b6fded06a 100644 --- a/lib/banzai/filter/table_of_contents_tag_filter.rb +++ b/lib/banzai/filter/table_of_contents_tag_filter.rb @@ -14,6 +14,9 @@ module Banzai class TableOfContentsTagFilter < HTML::Pipeline::Filter TEXT_QUERY = %q(descendant-or-self::text()[ancestor::p and contains(translate(., 'TOC', 'toc'), 'toc')]) + HEADER_CSS = 'h1, h2, h3, h4, h5, h6' + HEADER_XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(HEADER_CSS).freeze + def call return doc if context[:no_header_anchors] @@ -43,6 +46,8 @@ module Banzai # Replace an entire `[TOC]` node with the result generated by # TableOfContentsFilter def process_toc_tag(node) + build_toc if Feature.enabled?(:native_header_anchors) + # we still need to go one step up to also replace the surrounding node.parent.replace(result[:toc].presence || '') end @@ -56,6 +61,85 @@ module Banzai def toc_tag?(node) node.parent.text.casecmp?('[toc]') end + + def build_toc + return if result[:toc] + + result[:toc] = +"" + + header_root = current_header = HeaderNode.new + + doc.xpath(HEADER_XPATH).each do |node| + header_anchor = node.children.first + next unless header_anchor + next unless header_anchor.name == 'a' + next unless header_anchor[:class] == 'anchor' + + # remove leading anchor `#` so we can add it back later + href = header_anchor[:href].slice(1..) + current_header = HeaderNode.new(node: node, href: href, previous_header: current_header) + end + + push_toc(header_root.children, root: true) + end + + def push_toc(children, root: false) + return if children.empty? + + klass = ' class="section-nav"' if root + + result[:toc] << "" + children.each { |child| push_anchor(child) } + result[:toc] << '
' + end + + def push_anchor(header_node) + result[:toc] << %(- #{header_node.text}) + push_toc(header_node.children) + result[:toc] << '
' + end + + class HeaderNode + attr_reader :node, :href, :parent, :children + + def initialize(node: nil, href: nil, previous_header: nil) + @node = node + @href = CGI.escape(href) if href + @children = [] + + @parent = find_parent(previous_header) + @parent.children.push(self) if @parent + end + + def level + return 0 unless node + + @level ||= node.name[1].to_i + end + + def text + return '' unless node + + @text ||= CGI.escapeHTML(node.text) + end + + private + + def find_parent(previous_header) + return unless previous_header + + if level == previous_header.level + parent = previous_header.parent + elsif level > previous_header.level + parent = previous_header + else + parent = previous_header + parent = parent.parent while parent.level >= level + end + + parent + end + end end end end diff --git a/lib/banzai/pipeline/description_pipeline.rb b/lib/banzai/pipeline/description_pipeline.rb index 8236c7092d2..e7632c2c4a3 100644 --- a/lib/banzai/pipeline/description_pipeline.rb +++ b/lib/banzai/pipeline/description_pipeline.rb @@ -9,8 +9,8 @@ module Banzai def self.transform_context(context) super(context).merge( - # SanitizationFilter - allowlist: ALLOWLIST + allowlist: ALLOWLIST, # SanitizationFilter + no_header_anchors: true # header elements not allowed ) end end diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb index 4dfa8ee0a41..5afcc80c8ce 100644 --- a/lib/gitlab/auth/o_auth/user.rb +++ b/lib/gitlab/auth/o_auth/user.rb @@ -11,16 +11,31 @@ module Gitlab class User class << self # rubocop: disable CodeReuse/ActiveRecord - def find_by_uid_and_provider(uid, provider) + + # `auth_hash` argument should be removed by https://gitlab.com/gitlab-org/gitlab/-/issues/456424 + def find_by_uid_and_provider(uid, provider, auth_hash = nil) identity = ::Identity.with_extern_uid(provider, uid).take - identity && identity.user + # This fallback should be removed by https://gitlab.com/gitlab-org/gitlab/-/issues/456424 + if provider == 'bitbucket' && !auth_hash.nil? + identity ||= ::Identity.with_extern_uid(provider, auth_hash.username).take + + if identity && !identity.trusted_extern_uid? && identity.user && identity.user.email == auth_hash.email + identity.update(extern_uid: uid, trusted_extern_uid: true) + end + end + + return unless identity + raise IdentityWithUntrustedExternUidError unless identity.trusted_extern_uid? + + identity.user end # rubocop: enable CodeReuse/ActiveRecord end SignupDisabledError = Class.new(StandardError) SigninDisabledForProviderError = Class.new(StandardError) + IdentityWithUntrustedExternUidError = Class.new(StandardError) attr_reader :auth_hash @@ -212,7 +227,7 @@ module Gitlab end def find_by_uid_and_provider - self.class.find_by_uid_and_provider(auth_hash.uid, auth_hash.provider) + self.class.find_by_uid_and_provider(auth_hash.uid, auth_hash.provider, auth_hash) end def build_new_user(skip_confirmation: true) diff --git a/lib/gitlab/background_migration/backfill_workspace_variables_project_id.rb b/lib/gitlab/background_migration/backfill_workspace_variables_project_id.rb new file mode 100644 index 00000000000..63e2cbf1d33 --- /dev/null +++ b/lib/gitlab/background_migration/backfill_workspace_variables_project_id.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # rubocop: disable Migration/BackgroundMigrationBaseClass -- BackfillDesiredShardingKeyJob inherits from BatchedMigrationJob. + class BackfillWorkspaceVariablesProjectId < BackfillDesiredShardingKeyJob + operation_name :backfill_workspace_variables_project_id + feature_category :remote_development + end + # rubocop: enable Migration/BackgroundMigrationBaseClass + end +end diff --git a/lib/gitlab/ci/parsers/security/validators/schema_validator.rb b/lib/gitlab/ci/parsers/security/validators/schema_validator.rb index c6e065f29b8..196c907038d 100644 --- a/lib/gitlab/ci/parsers/security/validators/schema_validator.rb +++ b/lib/gitlab/ci/parsers/security/validators/schema_validator.rb @@ -32,6 +32,10 @@ module Gitlab CURRENT_VERSIONS = SUPPORTED_VERSIONS.to_h { |k, v| [k, v - DEPRECATED_VERSIONS[k]] } + # Matches schema-defined pattern + # https://gitlab.com/gitlab-org/security-products/security-report-schemas/-/blob/e3d280d7f0862ca66a1555ea8b24016a004bb914/src/security-report-format.json#L151 + SCHEMA_VERSION_REGEX = /^[0-9]+\.[0-9]+\.[0-9]+$/ + class Schema def root_path File.join(__dir__, 'schemas') @@ -97,7 +101,7 @@ module Gitlab @deprecation_warnings = [] populate_schema_version_errors - populate_validation_errors + populate_validation_errors if report_version_matches_schema? populate_deprecation_warnings end @@ -147,6 +151,10 @@ module Gitlab end end + def report_version_matches_schema? + report_version && report_version.match(SCHEMA_VERSION_REGEX) + end + def find_latest_patch_version ::Security::ReportSchemaVersionMatcher.new( report_declared_version: report_version, diff --git a/lib/gitlab/content_security_policy/config_loader.rb b/lib/gitlab/content_security_policy/config_loader.rb index b22652e8e9a..d63f2c7c388 100644 --- a/lib/gitlab/content_security_policy/config_loader.rb +++ b/lib/gitlab/content_security_policy/config_loader.rb @@ -132,7 +132,6 @@ module Gitlab end def allow_sentry(directives) - allow_legacy_sentry(directives) if legacy_sentry_configured? return unless sentry_client_side_dsn_enabled? sentry_uri = URI(Gitlab::CurrentSettings.sentry_clientside_dsn) @@ -140,18 +139,6 @@ module Gitlab append_to_directive(directives, 'connect_src', "#{sentry_uri.scheme}://#{sentry_uri.host}") end - def allow_legacy_sentry(directives) - # Support for Sentry setup via configuration files will be removed in 16.0 - # in favor of Gitlab::CurrentSettings. - sentry_uri = URI(Gitlab.config.sentry.clientside_dsn) - - append_to_directive(directives, 'connect_src', "#{sentry_uri.scheme}://#{sentry_uri.host}") - end - - def legacy_sentry_configured? - Gitlab.config.sentry&.enabled && Gitlab.config.sentry&.clientside_dsn - end - def sentry_client_side_dsn_enabled? Gitlab::CurrentSettings.try(:sentry_enabled) && Gitlab::CurrentSettings.try(:sentry_clientside_dsn) end diff --git a/lib/gitlab/error_tracking.rb b/lib/gitlab/error_tracking.rb index 6bf30e152f8..de6be9d1617 100644 --- a/lib/gitlab/error_tracking.rb +++ b/lib/gitlab/error_tracking.rb @@ -27,34 +27,12 @@ module Gitlab ].freeze class << self - def configure(&block) - configure_raven(&block) - configure_sentry(&block) - end - - def configure_raven - Raven.configure do |config| + def configure + Sentry.init do |config| config.dsn = sentry_dsn config.release = Gitlab.revision - config.current_environment = Gitlab.config.sentry.environment - - # Sanitize fields based on those sanitized from Rails. - config.sanitize_fields = Rails.application.config.filter_parameters.map(&:to_s) - - # Sanitize authentication headers - config.sanitize_http_headers = %w[Authorization Private-Token] - config.before_send = method(:before_send_raven) - - yield config if block_given? - end - end - - def configure_sentry - Sentry.init do |config| - config.dsn = new_sentry_dsn - config.release = Gitlab.revision - config.environment = new_sentry_environment - config.before_send = method(:before_send_sentry) + config.environment = sentry_environment + config.before_send = method(:before_send) config.background_worker_threads = 0 config.send_default_pii = true config.send_modules = false @@ -134,18 +112,6 @@ module Gitlab private - def before_send_raven(event, hint) - return unless Feature.enabled?(:enable_old_sentry_integration) - - before_send(event, hint) - end - - def before_send_sentry(event, hint) - return unless Feature.enabled?(:enable_new_sentry_integration) - - before_send(event, hint) - end - def before_send(event, hint) # Don't report Sidekiq retry errors to Sentry return if hint[:exception].is_a?(Gitlab::SidekiqMiddleware::RetryError) @@ -170,7 +136,6 @@ module Gitlab def default_trackers [].tap do |destinations| - destinations << Raven if Raven.configuration.server # There is a possibility that this method is called before Sentry is # configured. Since Sentry 4.0, some methods of Sentry are forwarded to # to `nil`, hence we have to check the client as well. @@ -179,27 +144,58 @@ module Gitlab end end + # Some configuration attributes like `dsn`, and `environment` + # can be configured both via `ENV` and `Application Settings`. + # The reason being is while GitLab.com uses application_settings + # in Geo installations, we can't override values in the primary database. + # Setting this value in application_settings would propagate the value + # to all Geo nodes, which doesn't solve that particular problem. def sentry_dsn - return unless sentry_configurable? - return unless Gitlab.config.sentry.enabled - - Gitlab.config.sentry.dsn + env_sentry_dsn || database_sentry_dsn end - def new_sentry_dsn + def sentry_environment + env_sentry_environment || database_sentry_environment + end + + def database_sentry_dsn return unless sentry_configurable? - return unless Gitlab::CurrentSettings.respond_to?(:sentry_enabled?) - return unless Gitlab::CurrentSettings.sentry_enabled? + return unless database_sentry_enabled? Gitlab::CurrentSettings.sentry_dsn end - def new_sentry_environment + def env_sentry_dsn + return unless sentry_configurable? + return unless env_sentry_enabled? + + Gitlab.config.sentry.dsn + end + + def env_sentry_environment + return unless sentry_configurable? + return unless env_sentry_enabled? + + Gitlab.config.sentry.environment + end + + def database_sentry_environment + return unless sentry_configurable? + return unless database_sentry_enabled? return unless Gitlab::CurrentSettings.respond_to?(:sentry_environment) Gitlab::CurrentSettings.sentry_environment end + def database_sentry_enabled? + Gitlab::CurrentSettings.respond_to?(:sentry_enabled?) && + Gitlab::CurrentSettings.sentry_enabled? + end + + def env_sentry_enabled? + Gitlab.config.sentry.enabled + end + def sentry_configurable? Rails.env.production? || Rails.env.development? end diff --git a/lib/gitlab/error_tracking/log_formatter.rb b/lib/gitlab/error_tracking/log_formatter.rb index d004c4e20bb..98b64ff335f 100644 --- a/lib/gitlab/error_tracking/log_formatter.rb +++ b/lib/gitlab/error_tracking/log_formatter.rb @@ -3,7 +3,7 @@ module Gitlab module ErrorTracking class LogFormatter - # Note: all the accesses to Raven's contexts here are to keep the + # Note: all the accesses to Sentry's contexts here are to keep the # backward-compatibility to Sentry's built-in integrations. In future, # they can be removed. def generate_log(exception, context_payload) @@ -20,21 +20,24 @@ module Gitlab private def append_user_to_log!(payload, context_payload) - user_context = Raven.context.user.merge(context_payload[:user]) + user_context = context_payload[:user] + user_context = Sentry.get_current_scope.user.merge(user_context) if Sentry.initialized? user_context.each do |key, value| payload["user.#{key}"] = value end end def append_tags_to_log!(payload, context_payload) - tags_context = Raven.context.tags.merge(context_payload[:tags]) + tags_context = context_payload[:tags] + tags_context = Sentry.get_current_scope.tags.merge(tags_context) if Sentry.initialized? tags_context.each do |key, value| payload["tags.#{key}"] = value end end def append_extra_to_log!(payload, context_payload) - extra = Raven.context.extra.merge(context_payload[:extra]) + extra = context_payload[:extra] + extra = Sentry.get_current_scope.extra.merge(extra) if Sentry.initialized? extra = extra.except(:server) # The extra value for sidekiq is a hash whose keys are strings. diff --git a/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb b/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb index 4b6c69a8b33..daf570536d6 100644 --- a/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb +++ b/lib/gitlab/error_tracking/processor/concerns/processes_exceptions.rb @@ -8,11 +8,7 @@ module Gitlab private def extract_exceptions_from(event) - exceptions = if event.is_a?(Raven::Event) - event.instance_variable_get(:@interfaces)[:exception]&.values - else - event&.exception&.instance_variable_get(:@values) - end + exceptions = event&.exception&.instance_variable_get(:@values) Array.wrap(exceptions) end @@ -27,7 +23,7 @@ module Gitlab def valid_exception?(exception) case exception - when Raven::SingleExceptionInterface, Sentry::SingleExceptionInterface + when Sentry::SingleExceptionInterface exception&.value.present? else false diff --git a/lib/gitlab/error_tracking/processor/sanitizer_processor.rb b/lib/gitlab/error_tracking/processor/sanitizer_processor.rb index e6114f8e206..577b0f2697d 100644 --- a/lib/gitlab/error_tracking/processor/sanitizer_processor.rb +++ b/lib/gitlab/error_tracking/processor/sanitizer_processor.rb @@ -15,9 +15,6 @@ module Gitlab # For more information, please visit: # https://docs.sentry.io/platforms/ruby/guides/rails/configuration/filtering/#using-beforesend def self.call(event) - # Raven::Event instances don't need this processing. - return event unless event.is_a?(Sentry::Event) - if event.request.present? event.request.cookies = {} event.request.data = {} diff --git a/lib/gitlab/file_finder.rb b/lib/gitlab/file_finder.rb index 8a894901ca1..0ca6b30f907 100644 --- a/lib/gitlab/file_finder.rb +++ b/lib/gitlab/file_finder.rb @@ -15,9 +15,17 @@ module Gitlab def find(query, content_match_cutoff: nil) query = Gitlab::Search::Query.new(query, encode_binary: true) do - filter :filename, matcher: ->(filter, blob) { blob.binary_path =~ /#{filter[:regex_value]}$/i } - filter :path, matcher: ->(filter, blob) { blob.binary_path =~ /#{filter[:regex_value]}/i } - filter :extension, matcher: ->(filter, blob) { blob.binary_path =~ /\.#{filter[:regex_value]}$/i } + filter :filename, matcher: ->(filter, blob) do + ::Gitlab::UntrustedRegexp.new("(?i)#{filter[:regex_value]}$").match?(blob.binary_path) + end + + filter :path, matcher: ->(filter, blob) do + ::Gitlab::UntrustedRegexp.new("(?i)#{filter[:regex_value]}").match?(blob.binary_path) + end + + filter :extension, matcher: ->(filter, blob) do + ::Gitlab::UntrustedRegexp.new("(?i)\\.#{filter[:regex_value]}$").match?(blob.binary_path) + end end content_match_cutoff = nil if query.filters.any? diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 7ed047811fe..204dafa228d 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1012,12 +1012,12 @@ module Gitlab user, branch_name:, message:, actions:, author_email: nil, author_name: nil, start_branch_name: nil, start_sha: nil, start_repository: nil, - force: false) + force: false, sign: true) wrapped_gitaly_errors do gitaly_operation_client.user_commit_files(user, branch_name, message, actions, author_email, author_name, - start_branch_name, start_repository, force, start_sha) + start_branch_name, start_repository, force, start_sha, sign) end end # rubocop:enable Metrics/ParameterLists diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index 014881535e2..bc9512b1b5c 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -479,11 +479,11 @@ module Gitlab # rubocop:disable Metrics/ParameterLists def user_commit_files( user, branch_name, commit_message, actions, author_email, author_name, - start_branch_name, start_repository, force = false, start_sha = nil) + start_branch_name, start_repository, force = false, start_sha = nil, sign = true) req_enum = Enumerator.new do |y| header = user_commit_files_request_header(user, branch_name, commit_message, actions, author_email, author_name, - start_branch_name, start_repository, force, start_sha) + start_branch_name, start_repository, force, start_sha, sign) y.yield Gitaly::UserCommitFilesRequest.new(header: header) @@ -575,7 +575,7 @@ module Gitlab # rubocop:disable Metrics/ParameterLists def user_commit_files_request_header( user, branch_name, commit_message, actions, author_email, author_name, - start_branch_name, start_repository, force, start_sha) + start_branch_name, start_repository, force, start_sha, sign) Gitaly::UserCommitFilesRequestHeader.new( repository: @gitaly_repo, @@ -588,6 +588,7 @@ module Gitlab start_repository: start_repository&.gitaly_repository, force: force, start_sha: encode_binary(start_sha), + sign: sign, timestamp: Google::Protobuf::Timestamp.new(seconds: Time.now.utc.to_i) ) end diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index a3dd12ebabc..de4dde89f0a 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -20,14 +20,10 @@ module Gitlab add_browsersdk_tracking - if Gitlab.config.sentry.enabled - gon.sentry_dsn = Gitlab.config.sentry.clientside_dsn - gon.sentry_environment = Gitlab.config.sentry.environment - end - - # Support for Sentry setup via configuration files will be removed in 17.0 - # in favor of Gitlab::CurrentSettings. - if Feature.enabled?(:enable_new_sentry_integration) && Gitlab::CurrentSettings.sentry_enabled + # Sentry configurations for the browser client are done + # via `Gitlab::CurrentSettings` from the Admin panel: + # `/admin/application_settings/metrics_and_profiling` + if Gitlab::CurrentSettings.sentry_enabled gon.sentry_dsn = Gitlab::CurrentSettings.sentry_clientside_dsn gon.sentry_environment = Gitlab::CurrentSettings.sentry_environment gon.sentry_clientside_traces_sample_rate = Gitlab::CurrentSettings.sentry_clientside_traces_sample_rate diff --git a/lib/gitlab/graphql/authorize/authorize_resource.rb b/lib/gitlab/graphql/authorize/authorize_resource.rb index e3548b97ebf..2c9b10cc33b 100644 --- a/lib/gitlab/graphql/authorize/authorize_resource.rb +++ b/lib/gitlab/graphql/authorize/authorize_resource.rb @@ -64,7 +64,7 @@ module Gitlab def authorized_resource?(object) raise ConfigurationError, "#{self.class.name} has no authorizations" if self.class.authorization.none? - self.class.authorization.ok?(object, current_user) + self.class.authorization.ok?(object, current_user, scope_validator: context[:scope_validator]) end def raise_resource_not_available_error!(...) diff --git a/lib/gitlab/graphql/authorize/object_authorization.rb b/lib/gitlab/graphql/authorize/object_authorization.rb index f13acc9ea27..20f24a6823e 100644 --- a/lib/gitlab/graphql/authorize/object_authorization.rb +++ b/lib/gitlab/graphql/authorize/object_authorization.rb @@ -19,7 +19,7 @@ module Gitlab abilities.present? end - def ok?(object, current_user, scope_validator: nil) + def ok?(object, current_user, scope_validator:) scopes_ok?(scope_validator) && abilities_ok?(object, current_user) end diff --git a/lib/gitlab/rack_attack.rb b/lib/gitlab/rack_attack.rb index 2182f5b56e4..113c6dec6c1 100644 --- a/lib/gitlab/rack_attack.rb +++ b/lib/gitlab/rack_attack.rb @@ -126,6 +126,16 @@ module Gitlab 'throttle_authenticated_git_lfs' => ThrottleDefinition.new( Gitlab::Throttle.throttle_authenticated_git_lfs_options, ->(req) { req.throttled_identifer([:api]) if req.throttle_authenticated_git_lfs? } + ), + **throttle_definitions_unauthenticated_git_http + } + end + + def self.throttle_definitions_unauthenticated_git_http + { + 'throttle_unauthenticated_git_http' => ThrottleDefinition.new( + Gitlab::Throttle.throttle_unauthenticated_git_http_options, + ->(req) { req.ip if req.throttle_unauthenticated_git_http? } ) } end diff --git a/lib/gitlab/rack_attack/request.rb b/lib/gitlab/rack_attack/request.rb index e45782b8be0..63c0215c65a 100644 --- a/lib/gitlab/rack_attack/request.rb +++ b/lib/gitlab/rack_attack/request.rb @@ -100,6 +100,7 @@ module Gitlab def throttle_unauthenticated_web? (web_request? || frontend_request?) && !should_be_skipped? && + !git_path? && # TODO: Column will be renamed in https://gitlab.com/gitlab-org/gitlab/-/issues/340031 Gitlab::Throttle.settings.throttle_unauthenticated_enabled && unauthenticated? @@ -175,6 +176,12 @@ module Gitlab Gitlab::Throttle.settings.throttle_authenticated_packages_api_enabled end + def throttle_unauthenticated_git_http? + git_path? && + Gitlab::Throttle.settings.throttle_unauthenticated_git_http_enabled && + unauthenticated? + end + def throttle_authenticated_git_lfs? git_lfs_path? && Gitlab::Throttle.settings.throttle_authenticated_git_lfs_enabled @@ -242,6 +249,10 @@ module Gitlab matches?(::Gitlab::Regex::Packages::API_PATH_REGEX) end + def git_path? + matches?(::Gitlab::PathRegex.repository_git_route_regex) + end + def git_lfs_path? matches?(::Gitlab::PathRegex.repository_git_lfs_route_regex) end diff --git a/lib/gitlab/throttle.rb b/lib/gitlab/throttle.rb index 384953533b5..1aef19f1e6a 100644 --- a/lib/gitlab/throttle.rb +++ b/lib/gitlab/throttle.rb @@ -71,6 +71,13 @@ module Gitlab { limit: limit_proc, period: period_proc } end + def self.throttle_unauthenticated_git_http_options + limit_proc = proc { |req| settings.throttle_unauthenticated_git_http_requests_per_period } + period_proc = proc { |req| settings.throttle_unauthenticated_git_http_period_in_seconds.seconds } + + { limit: limit_proc, period: period_proc } + end + def self.throttle_authenticated_git_lfs_options limit_proc = proc { |req| settings.throttle_authenticated_git_lfs_requests_per_period } period_proc = proc { |req| settings.throttle_authenticated_git_lfs_period_in_seconds.seconds } diff --git a/lib/gitlab/usage_data_counters.rb b/lib/gitlab/usage_data_counters.rb index 19c6e04583e..3472d63a40a 100644 --- a/lib/gitlab/usage_data_counters.rb +++ b/lib/gitlab/usage_data_counters.rb @@ -13,7 +13,6 @@ module Gitlab WebIdeCounter, WikiPageCounter, SnippetCounter, - ProductivityAnalyticsCounter, SourceCodeCounter, MergeRequestWidgetExtensionCounter ].freeze diff --git a/lib/gitlab/usage_data_counters/productivity_analytics_counter.rb b/lib/gitlab/usage_data_counters/productivity_analytics_counter.rb deleted file mode 100644 index 3b92fa01fbe..00000000000 --- a/lib/gitlab/usage_data_counters/productivity_analytics_counter.rb +++ /dev/null @@ -1,8 +0,0 @@ -# frozen_string_literal: true - -module Gitlab::UsageDataCounters - class ProductivityAnalyticsCounter < BaseCounter - KNOWN_EVENTS = %w[views].freeze - PREFIX = 'productivity_analytics' - end -end diff --git a/lib/gitlab/usage_data_counters/total_counter_redis_key_overrides.yml b/lib/gitlab/usage_data_counters/total_counter_redis_key_overrides.yml index 2619f49a86d..a99631a0963 100644 --- a/lib/gitlab/usage_data_counters/total_counter_redis_key_overrides.yml +++ b/lib/gitlab/usage_data_counters/total_counter_redis_key_overrides.yml @@ -12,6 +12,7 @@ '{event_counters}_usage_data_download_payload_clicked': USAGE_SERVICE_USAGE_DATA_DOWNLOAD_PAYLOAD_CLICK '{event_counters}_value_streams_dashboard_viewed': USAGE_VALUE_STREAMS_DASHBOARD_VIEWS '{event_counters}_view_cycle_analytics': USAGE_CYCLE_ANALYTICS_VIEWS +'{event_counters}_view_productivity_analytics': USAGE_PRODUCTIVITY_ANALYTICS_VIEWS '{event_counters}_web_ide_viewed': WEB_IDE_VIEWS_COUNT '{event_counters}_status_page_incident_published': USAGE_STATUS_PAGE_INCIDENT_PUBLISHES '{event_counters}_status_page_incident_unpublished': USAGE_STATUS_PAGE_INCIDENT_UNPUBLISHES diff --git a/lib/omni_auth/strategies/bitbucket.rb b/lib/omni_auth/strategies/bitbucket.rb index d64f3dd987d..dff3652f3ca 100644 --- a/lib/omni_auth/strategies/bitbucket.rb +++ b/lib/omni_auth/strategies/bitbucket.rb @@ -14,12 +14,13 @@ module OmniAuth } uid do - raw_info['username'] + raw_info['uuid'] end info do { name: raw_info['display_name'], + username: raw_info['username'], avatar: raw_info['links']['avatar']['href'], email: primary_email } @@ -36,7 +37,7 @@ module OmniAuth def emails email_response = access_token.get('api/2.0/user/emails').parsed - @emails ||= email_response && email_response['values'] || nil + @emails ||= email_response && email_response['values'] || [] end def callback_url diff --git a/lib/tasks/gitlab/ldap.rake b/lib/tasks/gitlab/ldap.rake index 4da22e686ef..78a67d82096 100644 --- a/lib/tasks/gitlab/ldap.rake +++ b/lib/tasks/gitlab/ldap.rake @@ -5,9 +5,9 @@ namespace :gitlab do desc 'GitLab | LDAP | Rename provider' task :rename_provider, [:old_provider, :new_provider] => :gitlab_environment do |_, args| old_provider = args[:old_provider] || - prompt('What is the old provider? Ex. \'ldapmain\': '.color(:blue)) + prompt(Rainbow('What is the old provider? Ex. \'ldapmain\': ').blue) new_provider = args[:new_provider] || - prompt('What is the new provider ID? Ex. \'ldapcustom\': '.color(:blue)) + prompt(Rainbow('What is the new provider ID? Ex. \'ldapcustom\': ').blue) puts '' # Add some separation in the output identities = Identity.where(provider: old_provider) @@ -31,10 +31,10 @@ namespace :gitlab do updated_count = identities.update_all(provider: new_provider) if updated_count == identity_count - puts 'User identities were successfully updated'.color(:green) + puts Rainbow('User identities were successfully updated').green else plural_updated_count = ActionController::Base.helpers.pluralize(updated_count, 'user') - puts 'Some user identities could not be updated'.color(:red) + puts Rainbow('Some user identities could not be updated').red puts "Successfully updated #{plural_updated_count} out of #{plural_id_count} total" end end diff --git a/lib/tasks/gitlab/seed.rake b/lib/tasks/gitlab/seed.rake index 41830065044..9cc6b906aba 100644 --- a/lib/tasks/gitlab/seed.rake +++ b/lib/tasks/gitlab/seed.rake @@ -16,7 +16,7 @@ namespace :gitlab do error_message += " Did you mean '#{potential_projects.first.full_path}'?" end - puts error_message.color(:red) + puts Rainbow(error_message).red exit 1 end @@ -57,7 +57,7 @@ namespace :gitlab do error_message += " Did you mean '#{potential_groups.first.full_path}'?" end - puts error_message.color(:red) + puts Rainbow(error_message).red exit 1 end diff --git a/lib/tasks/gitlab/setup.rake b/lib/tasks/gitlab/setup.rake index 38c5902702c..2e24e38eb43 100644 --- a/lib/tasks/gitlab/setup.rake +++ b/lib/tasks/gitlab/setup.rake @@ -12,7 +12,7 @@ namespace :gitlab do Gitlab::GitalyClient::ServerService.new(name).info end rescue GRPC::Unavailable => ex - puts "Failed to connect to Gitaly...".color(:red) + puts Rainbow("Failed to connect to Gitaly...").red puts "Error: #{ex}" exit 1 end @@ -36,7 +36,7 @@ namespace :gitlab do Rake::Task["gitlab:db:lock_writes"].invoke Rake::Task["db:seed_fu"].invoke rescue Gitlab::TaskAbortedByUserError - puts "Quitting...".color(:red) + puts Rainbow("Quitting...").red exit 1 end end diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake index abeb5bbdf29..5eb3ff8c28b 100644 --- a/lib/tasks/gitlab/shell.rake +++ b/lib/tasks/gitlab/shell.rake @@ -69,12 +69,12 @@ namespace :gitlab do Key.auth.find_in_batches(batch_size: 1000) do |keys| unless authorized_keys.batch_add_keys(keys) - puts "Failed to add keys...".color(:red) + puts Rainbow("Failed to add keys...").red exit 1 end end rescue Gitlab::TaskAbortedByUserError - puts "Quitting...".color(:red) + puts Rainbow("Quitting...").red exit 1 end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fdb5631089d..b72e5ddce8f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3742,9 +3742,6 @@ msgstr "" msgid "AdminSettings|Git abuse rate limit" msgstr "" -msgid "AdminSettings|GitLab uses the %{bold_start}Rails%{bold_end} and %{bold_start}Browser JavaScript%{bold_end} Sentry SDKs to send events to Sentry. For changes to Rails integration settings to take effect, enable the %{code_start}enable_new_sentry_integration%{code_end} feature flag and restart GitLab." -msgstr "" - msgid "AdminSettings|GitLab uses the %{bold_start}Rails%{bold_end} and %{bold_start}Browser JavaScript%{bold_end} Sentry SDKs to send events to Sentry. For changes to Rails integration settings to take effect, restart GitLab." msgstr "" @@ -10002,9 +9999,6 @@ msgstr "" msgid "Can't find HEAD commit for this branch" msgstr "" -msgid "Can't find variable: ZiteReader" -msgstr "" - msgid "Can't scan the code?" msgstr "" @@ -12687,9 +12681,6 @@ msgstr "" msgid "Colorize messages" msgstr "" -msgid "ComboSearch is not defined" -msgstr "" - msgid "Comma-separated list of branches to be automatically inspected. Leave blank to include all branches." msgstr "" @@ -13696,6 +13687,9 @@ msgstr "" msgid "Configure specific limits for Files API requests that supersede the general user and IP rate limits." msgstr "" +msgid "Configure specific limits for Git HTTP requests." +msgstr "" + msgid "Configure specific limits for Git LFS requests that supersede the general user and IP rate limits." msgstr "" @@ -19716,6 +19710,9 @@ msgstr "" msgid "Enable unauthenticated API request rate limit" msgstr "" +msgid "Enable unauthenticated Git HTTP request rate limit" +msgstr "" + msgid "Enable unauthenticated web request rate limit" msgstr "" @@ -23088,6 +23085,9 @@ msgstr "" msgid "Git" msgstr "" +msgid "Git HTTP rate limits" +msgstr "" + msgid "Git LFS is not enabled on this GitLab server, contact your admin." msgstr "" @@ -23664,7 +23664,7 @@ msgstr "" msgid "GlobalSearch|%{linkStart}Exact code search (powered by Zoekt)%{linkEnd} is disabled since %{ref_elem} is not the default branch. %{docs_link}" msgstr "" -msgid "GlobalSearch|%{linkStart}Exact code search (powered by Zoekt)%{linkEnd} is enabled" +msgid "GlobalSearch|%{linkStart}Exact code search (powered by Zoekt)%{linkEnd} is enabled." msgstr "" msgid "GlobalSearch|Aggregations load error." @@ -23838,9 +23838,6 @@ msgstr "" msgid "GlobalSearch|Showing top %{maxItems}" msgstr "" -msgid "GlobalSearch|Syntax options" -msgstr "" - msgid "GlobalSearch|The search term must be at least 3 characters long." msgstr "" @@ -23874,6 +23871,9 @@ msgstr "" msgid "GlobalSearch|Users" msgstr "" +msgid "GlobalSearch|View syntax options." +msgstr "" + msgid "GlobalSearch|What are you searching for?" msgstr "" @@ -25683,6 +25683,9 @@ msgstr "" msgid "Helps reduce request volume for protected paths." msgstr "" +msgid "Helps reduce unauthenticated Git HTTP request volume for git paths." +msgstr "" + msgid "Hi %{recipient_name}," msgstr "" @@ -31403,6 +31406,9 @@ msgstr "" msgid "Maximum unauthenticated API requests per rate limit period per IP" msgstr "" +msgid "Maximum unauthenticated Git HTTP requests per period per IP" +msgstr "" + msgid "Maximum unauthenticated web requests per rate limit period per IP" msgstr "" @@ -34941,6 +34947,9 @@ msgstr "" msgid "Object does not exist on the server or you don't have permissions to access it" msgstr "" +msgid "ObservabilityLogs|Attribute" +msgstr "" + msgid "ObservabilityLogs|Attributes" msgstr "" @@ -34962,6 +34971,9 @@ msgstr "" msgid "ObservabilityLogs|Fatal" msgstr "" +msgid "ObservabilityLogs|Fingerprint" +msgstr "" + msgid "ObservabilityLogs|Info" msgstr "" @@ -34980,21 +34992,45 @@ msgstr "" msgid "ObservabilityLogs|No logs to display." msgstr "" +msgid "ObservabilityLogs|Resource Attribute" +msgstr "" + msgid "ObservabilityLogs|Resource attributes" msgstr "" +msgid "ObservabilityLogs|Search logs..." +msgstr "" + msgid "ObservabilityLogs|Service" msgstr "" +msgid "ObservabilityLogs|Severity" +msgstr "" + msgid "ObservabilityLogs|Showing %{count} logs" msgstr "" +msgid "ObservabilityLogs|Span ID" +msgstr "" + msgid "ObservabilityLogs|Trace" msgstr "" +msgid "ObservabilityLogs|Trace Flags" +msgstr "" + +msgid "ObservabilityLogs|Trace ID" +msgstr "" + msgid "ObservabilityLogs|Warning" msgstr "" +msgid "ObservabilityLogs|name" +msgstr "" + +msgid "ObservabilityLogs|value" +msgstr "" + msgid "ObservabilityMetrics|+%{count} more" msgstr "" @@ -39582,7 +39618,7 @@ msgstr "" msgid "Profiles|The ideal image size is 192 x 192 pixels." msgstr "" -msgid "Profiles|The maximum file size allowed is 200KB." +msgid "Profiles|The maximum file size allowed is 200 KiB." msgstr "" msgid "Profiles|This email will be displayed on your public profile." @@ -47013,6 +47049,9 @@ msgstr "" msgid "SecurityReports|Confirm dismissal" msgstr "" +msgid "SecurityReports|Container registry vulnerabilities" +msgstr "" + msgid "SecurityReports|Create Jira issue" msgstr "" @@ -48664,6 +48703,9 @@ msgstr "" msgid "Signing in using %{label} has been disabled" msgstr "" +msgid "Signing in using your %{label} account has been disabled for security reasons. Please sign in to your GitLab account using another authentication method and reconnect to your %{label} account." +msgstr "" + msgid "Signing in using your %{label} account without a pre-existing GitLab account is not allowed." msgstr "" @@ -54817,6 +54859,12 @@ msgstr "" msgid "Unauthenticated API rate limit period in seconds" msgstr "" +msgid "Unauthenticated Git HTTP rate limit period in seconds" +msgstr "" + +msgid "Unauthenticated Git HTTP request rate limit" +msgstr "" + msgid "Unauthenticated requests" msgstr "" @@ -54916,9 +54964,6 @@ msgstr "" msgid "Unknown format" msgstr "" -msgid "Unknown response text" -msgstr "" - msgid "Unknown user" msgstr "" @@ -61146,9 +61191,6 @@ msgstr "" msgid "it is too large" msgstr "" -msgid "jigsaw is not defined" -msgstr "" - msgid "key result" msgstr "" diff --git a/package.json b/package.json index 20612d2306d..296a5864990 100644 --- a/package.json +++ b/package.json @@ -67,6 +67,7 @@ "@mattiasbuelens/web-streams-adapter": "^0.1.0", "@rails/actioncable": "7.0.8-1", "@rails/ujs": "7.0.8-1", + "@sentry/browser": "7.102.0", "@snowplow/browser-plugin-client-hints": "^3.9.0", "@snowplow/browser-plugin-form-tracking": "^3.9.0", "@snowplow/browser-plugin-ga-cookies": "^3.9.0", @@ -195,8 +196,6 @@ "sass": "^1.69.7", "scrollparent": "^2.0.1", "semver": "^7.3.4", - "sentrybrowser": "npm:@sentry/browser@7.102.0", - "sentrybrowser5": "npm:@sentry/browser@5.30.0", "sortablejs": "^1.10.2", "string-hash": "1.1.3", "style-loader": "^2.0.0", diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/nuget/nuget_group_level_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/nuget/nuget_group_level_spec.rb index b97e16ebcf6..bde39fde877 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/nuget/nuget_group_level_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/nuget/nuget_group_level_spec.rb @@ -1,7 +1,11 @@ # frozen_string_literal: true module QA - RSpec.describe 'Package', :object_storage, product_group: :package_registry do + RSpec.describe 'Package', :object_storage, product_group: :package_registry, quarantine: { + issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/455304', + only: { condition: -> { ENV['QA_RUN_TYPE']&.match?('gdk-qa-blocking') } }, + type: :investigating + } do describe 'NuGet group level endpoint', :external_api_calls do using RSpec::Parameterized::TableSyntax include Runtime::Fixtures diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/nuget/nuget_project_level_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/nuget/nuget_project_level_spec.rb index 15147c1a393..1c93a2bb1c5 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/nuget/nuget_project_level_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/nuget/nuget_project_level_spec.rb @@ -1,7 +1,11 @@ # frozen_string_literal: true module QA - RSpec.describe 'Package', :object_storage, product_group: :package_registry do + RSpec.describe 'Package', :object_storage, product_group: :package_registry, quarantine: { + issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/455027', + only: { condition: -> { ENV['QA_RUN_TYPE']&.match?('gdk-qa-blocking') } }, + type: :investigating + } do describe 'NuGet project level endpoint', :external_api_calls do include Support::Helpers::MaskToken diff --git a/spec/channels/graphql_channel_spec.rb b/spec/channels/graphql_channel_spec.rb new file mode 100644 index 00000000000..e02c58f30f9 --- /dev/null +++ b/spec/channels/graphql_channel_spec.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GraphqlChannel, feature_category: :api do + let_it_be(:merge_request) { create(:merge_request) } + let_it_be(:user) { create(:user, developer_of: merge_request.project) } + let_it_be(:read_api_token) { create(:personal_access_token, scopes: ['read_api'], user: user) } + let_it_be(:read_user_token) { create(:personal_access_token, scopes: ['read_user'], user: user) } + let_it_be(:read_api_and_read_user_token) do + create(:personal_access_token, scopes: %w[read_user read_api], user: user) + end + + describe '#subscribed' do + let(:query) do + <<~GRAPHQL + subscription mergeRequestReviewersUpdated($issuableId: IssuableID!) { + mergeRequestReviewersUpdated(issuableId: $issuableId) { + ... on MergeRequest { id title } + } + } + GRAPHQL + end + + let(:subscribe_params) do + { + query: query, + variables: { issuableId: merge_request.to_global_id } + } + end + + before do + stub_action_cable_connection current_user: user + end + + it 'subscribes to the given graphql subscription' do + subscribe(subscribe_params) + + expect(subscription).to be_confirmed + expect(subscription.streams).to include(/graphql-event::mergeRequestReviewersUpdated:issuableId/) + end + + context 'with a personal access token' do + before do + stub_action_cable_connection current_user: user, access_token: access_token + end + + context 'with an api scoped personal access token' do + let(:access_token) { read_api_token } + + it 'subscribes to the given graphql subscription' do + subscribe(subscribe_params) + + expect(subscription).to be_confirmed + expect(subscription.streams).to include(/graphql-event::mergeRequestReviewersUpdated:issuableId/) + end + end + + context 'with a read_user personal access token' do + let(:access_token) { read_user_token } + + it 'does not subscribe to the given graphql subscription' do + subscribe(subscribe_params) + + expect(subscription).not_to be_confirmed + end + end + + context 'with a read_api and read_user personal access token' do + let(:access_token) { read_api_and_read_user_token } + + it 'subscribes to the given graphql subscription' do + subscribe(subscribe_params) + + expect(subscription).to be_confirmed + expect(subscription.streams).to include(/graphql-event::mergeRequestReviewersUpdated:issuableId/) + end + end + end + end +end diff --git a/spec/channels/noteable/notes_channel_spec.rb b/spec/channels/noteable/notes_channel_spec.rb index 911e70630fa..ed8c67bb64b 100644 --- a/spec/channels/noteable/notes_channel_spec.rb +++ b/spec/channels/noteable/notes_channel_spec.rb @@ -4,7 +4,15 @@ require 'spec_helper' RSpec.describe Noteable::NotesChannel, feature_category: :team_planning do let_it_be(:project) { create(:project, :repository, :private) } - let_it_be(:developer) { create(:user, developer_of: project) } + let_it_be(:user) { create(:user, developer_of: project) } + + let_it_be(:read_api_token) { create(:personal_access_token, scopes: ['read_api'], user: user) } + let_it_be(:read_user_token) { create(:personal_access_token, scopes: ['read_user'], user: user) } + let_it_be(:read_api_and_read_user_token) do + create(:personal_access_token, scopes: %w[read_user read_api], user: user) + end + + let_it_be(:noteable) { create(:issue, project: project) } describe '#subscribed' do let(:subscribe_params) do @@ -16,7 +24,7 @@ RSpec.describe Noteable::NotesChannel, feature_category: :team_planning do end before do - stub_action_cable_connection current_user: developer + stub_action_cable_connection current_user: user end it 'rejects the subscription when noteable params are missing' do @@ -26,8 +34,6 @@ RSpec.describe Noteable::NotesChannel, feature_category: :team_planning do end context 'on an issue' do - let_it_be(:noteable) { create(:issue, project: project) } - it_behaves_like 'handle subscription based on user access' end @@ -37,4 +43,50 @@ RSpec.describe Noteable::NotesChannel, feature_category: :team_planning do it_behaves_like 'handle subscription based on user access' end end + + context 'with a personal access token' do + let(:subscribe_params) do + { + project_id: noteable.project_id, + noteable_type: noteable.class.underscore, + noteable_id: noteable.id + } + end + + before do + stub_action_cable_connection current_user: user, access_token: access_token + end + + context 'with an api scoped personal access token' do + let(:access_token) { read_api_token } + + it 'subscribes to the given graphql subscription' do + subscribe(subscribe_params) + + expect(subscription).to be_confirmed + expect(subscription).to have_stream_for(noteable) + end + end + + context 'with a read_user personal access token' do + let(:access_token) { read_user_token } + + it 'does not subscribe to the given graphql subscription' do + subscribe(subscribe_params) + + expect(subscription).not_to be_confirmed + end + end + + context 'with a read_api and read_user personal access token' do + let(:access_token) { read_api_and_read_user_token } + + it 'subscribes to the given graphql subscription' do + subscribe(subscribe_params) + + expect(subscription).to be_confirmed + expect(subscription).to have_stream_for(noteable) + end + end + end end diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index 8070ae554b3..ac4c725e132 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -94,10 +94,11 @@ RSpec.describe OmniauthCallbacksController, type: :controller, feature_category: describe 'omniauth' do let(:user) { create(:omniauth_user, extern_uid: extern_uid, provider: provider) } + let(:omniauth_email) { user.email } let(:additional_info) { {} } before do - @original_env_config_omniauth_auth = mock_auth_hash(provider.to_s, extern_uid, user.email, additional_info: additional_info) + @original_env_config_omniauth_auth = mock_auth_hash(provider.to_s, extern_uid, omniauth_email, additional_info: additional_info) stub_omniauth_provider(provider, context: request) end @@ -580,6 +581,23 @@ RSpec.describe OmniauthCallbacksController, type: :controller, feature_category: end end end + + context "when user's identity with untrusted extern_uid" do + let(:provider) { 'bitbucket' } + let(:extern_uid) { 'untrusted-uid' } + let(:omniauth_email) { 'bitbucketuser@example.com' } + + before do + user.identities.with_extern_uid(provider, extern_uid).update!(trusted_extern_uid: false) + end + + it 'shows warning when attempting login' do + post provider + + expect(response).to redirect_to new_user_session_path + expect(flash[:alert]).to eq('Signing in using your Bitbucket account has been disabled for security reasons. Please sign in to your GitLab account using another authentication method and reconnect to your Bitbucket account.') + end + end end describe '#openid_connect' do diff --git a/spec/features/admin/users/admin_sees_unconfirmed_user_spec.rb b/spec/features/admin/users/admin_sees_unconfirmed_user_spec.rb index 7b45e5b5cde..3554b3d40d2 100644 --- a/spec/features/admin/users/admin_sees_unconfirmed_user_spec.rb +++ b/spec/features/admin/users/admin_sees_unconfirmed_user_spec.rb @@ -14,8 +14,7 @@ RSpec.describe 'Admin sees unconfirmed user', feature_category: :user_management end context 'when user has an unconfirmed email', :js do - # Email address contains HTML to ensure email address is displayed in an HTML safe way. - let_it_be(:unconfirmed_email) { "#{generate(:email)}testing
" } + let_it_be(:unconfirmed_email) { generate(:email) } let_it_be(:unconfirmed_user) { create(:user, :unconfirmed, unconfirmed_email: unconfirmed_email) } where(:path_helper) do diff --git a/spec/features/sentry_js_spec.rb b/spec/features/sentry_js_spec.rb index 583dcba6220..672dc3010aa 100644 --- a/spec/features/sentry_js_spec.rb +++ b/spec/features/sentry_js_spec.rb @@ -3,57 +3,22 @@ require 'spec_helper' RSpec.describe 'Sentry', feature_category: :error_tracking do - context 'when enable_new_sentry_integration is disabled' do - before do - stub_feature_flags(enable_new_sentry_integration: false) - end + it 'does not load sentry if sentry settings are disabled' do + allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(false) - it 'does not load sentry if sentry is disabled' do - allow(Gitlab.config.sentry).to receive(:enabled).and_return(false) + visit new_user_session_path - visit new_user_session_path - - expect(has_requested_legacy_sentry).to eq(false) - end - - it 'loads legacy sentry if sentry config is enabled', :js do - allow(Gitlab.config.sentry).to receive(:enabled).and_return(true) - - visit new_user_session_path - - expect(has_requested_legacy_sentry).to eq(true) - expect(evaluate_script('window._Sentry.SDK_VERSION')).to match(%r{^5\.}) - end + expect(has_requested_sentry).to eq(false) end - context 'when enable_new_sentry_integration is enabled' do - before do - stub_feature_flags(enable_new_sentry_integration: true) - end + it 'loads sentry if sentry settings are enabled', :js do + allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(true) + allow(Gitlab::CurrentSettings).to receive(:sentry_clientside_dsn).and_return('https://mockdsn@example.com/1') - it 'does not load sentry if sentry settings are disabled' do - allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(false) + visit new_user_session_path - visit new_user_session_path - - expect(has_requested_sentry).to eq(false) - end - - it 'loads sentry if sentry settings are enabled', :js do - allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(true) - allow(Gitlab::CurrentSettings).to receive(:sentry_clientside_dsn).and_return('https://mockdsn@example.com/1') - - visit new_user_session_path - - expect(has_requested_sentry).to eq(true) - expect(evaluate_script('window._Sentry.SDK_VERSION')).to match(%r{^7\.}) - end - end - - def has_requested_legacy_sentry - page.all('script', visible: false).one? do |elm| - elm[:src] =~ %r{/legacy_sentry.*\.chunk\.js\z} - end + expect(has_requested_sentry).to eq(true) + expect(evaluate_script('window._Sentry.SDK_VERSION')).to match(%r{^7\.}) end def has_requested_sentry diff --git a/spec/frontend/ci/catalog/components/list/ci_resources_list_item_spec.js b/spec/frontend/ci/catalog/components/list/ci_resources_list_item_spec.js index 9c53fab6ec1..fbc1a8ec27d 100644 --- a/spec/frontend/ci/catalog/components/list/ci_resources_list_item_spec.js +++ b/spec/frontend/ci/catalog/components/list/ci_resources_list_item_spec.js @@ -7,6 +7,7 @@ import { createRouter } from '~/ci/catalog/router/index'; import CiResourcesListItem from '~/ci/catalog/components/list/ci_resources_list_item.vue'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import CiVerificationBadge from '~/ci/catalog/components/shared/ci_verification_badge.vue'; +import Markdown from '~/vue_shared/components/markdown/non_gfm_markdown.vue'; import { catalogSinglePageResponse } from '../../mock'; Vue.use(VueRouter); @@ -46,11 +47,11 @@ describe('CiResourcesListItem', () => { const findBadge = () => wrapper.findComponent(GlBadge); const findComponentNames = () => wrapper.findByTestId('ci-resource-component-names'); const findResourceName = () => wrapper.findByTestId('ci-resource-link'); - const findResourceDescription = () => wrapper.findByText(defaultProps.resource.description); const findUserLink = () => wrapper.findByTestId('user-link'); const findVerificationBadge = () => wrapper.findComponent(CiVerificationBadge); const findTimeAgoMessage = () => wrapper.findComponent(GlSprintf); const findFavorites = () => wrapper.findByTestId('stats-favorites'); + const findMarkdown = () => wrapper.findComponent(Markdown); beforeEach(() => { routerPush = jest.spyOn(router, 'push').mockImplementation(() => {}); @@ -82,7 +83,9 @@ describe('CiResourcesListItem', () => { }); it('renders the resource description', () => { - expect(findResourceDescription().exists()).toBe(true); + const markdown = findMarkdown(); + expect(markdown.exists()).toBe(true); + expect(markdown.props().markdown).toBe(defaultProps.resource.description); }); }); diff --git a/spec/frontend/observability/client_spec.js b/spec/frontend/observability/client_spec.js index 46726d491fa..cee501c8d7e 100644 --- a/spec/frontend/observability/client_spec.js +++ b/spec/frontend/observability/client_spec.js @@ -1173,16 +1173,72 @@ describe('buildClient', () => { }); }); + describe('attributes filters', () => { + it('converts filter to proper query params', async () => { + await client.fetchLogs({ + filters: { + attributes: { + service: [ + { operator: '=', value: 'serviceName' }, + { operator: '!=', value: 'serviceName2' }, + ], + severityName: [ + { operator: '=', value: 'info' }, + { operator: '!=', value: 'warning' }, + ], + traceId: [{ operator: '=', value: 'traceId' }], + spanId: [{ operator: '=', value: 'spanId' }], + fingerprint: [{ operator: '=', value: 'fingerprint' }], + traceFlags: [ + { operator: '=', value: '1' }, + { operator: '!=', value: '2' }, + ], + attribute: [{ operator: '=', value: 'attr=bar' }], + resourceAttribute: [{ operator: '=', value: 'res=foo' }], + search: [{ value: 'some-search' }], + }, + }, + }); + expect(getQueryParam()).toEqual( + `service_name=serviceName¬[service_name]=serviceName2` + + `&severity_name=info¬[severity_name]=warning` + + `&trace_id=traceId` + + `&span_id=spanId` + + `&fingerprint=fingerprint` + + `&trace_flags=1¬[trace_flags]=2` + + `&log_attr_name=attr&log_attr_value=bar` + + `&res_attr_name=res&res_attr_value=foo` + + `&body=some-search`, + ); + }); + + it('ignores unsupported operators', async () => { + await client.fetchLogs({ + filters: { + attributes: { + traceId: [{ operator: '!=', value: 'traceId2' }], + spanId: [{ operator: '!=', value: 'spanId2' }], + fingerprint: [{ operator: '!=', value: 'fingerprint2' }], + attribute: [{ operator: '!=', value: 'bar' }], + resourceAttribute: [{ operator: '!=', value: 'resourceAttribute2' }], + unsupported: [{ value: 'something', operator: '=' }], + }, + }, + }); + expect(getQueryParam()).toEqual(''); + }); + }); + it('ignores empty filter', async () => { await client.fetchLogs({ - filters: { dateRange: {} }, + filters: { attributes: {}, dateRange: {} }, }); expect(getQueryParam()).toBe(''); }); it('ignores undefined filter', async () => { await client.fetchLogs({ - filters: { dateRange: undefined }, + filters: { dateRange: undefined, attributes: undefined }, }); expect(getQueryParam()).toBe(''); }); diff --git a/spec/frontend/sentry/init_sentry_spec.js b/spec/frontend/sentry/init_sentry_spec.js index 118a48cc1de..0ed27196226 100644 --- a/spec/frontend/sentry/init_sentry_spec.js +++ b/spec/frontend/sentry/init_sentry_spec.js @@ -1,3 +1,4 @@ +/* eslint-disable no-restricted-imports */ import { BrowserClient, defaultStackParser, @@ -8,8 +9,8 @@ import { // exports captureException, SDK_VERSION, -} from 'sentrybrowser'; -import * as Sentry from 'sentrybrowser'; +} from '@sentry/browser'; +import * as Sentry from '@sentry/browser'; import { initSentry } from '~/sentry/init_sentry'; @@ -23,14 +24,14 @@ const mockFeatureCategory = 'my_feature_category'; const mockPage = 'index:page'; const mockSentryClientsideTracesSampleRate = 0.1; -jest.mock('sentrybrowser', () => { +jest.mock('@sentry/browser', () => { return { - ...jest.createMockFromModule('sentrybrowser'), + ...jest.createMockFromModule('@sentry/browser'), // unmock actual configuration options - defaultStackParser: jest.requireActual('sentrybrowser').defaultStackParser, - makeFetchTransport: jest.requireActual('sentrybrowser').makeFetchTransport, - defaultIntegrations: jest.requireActual('sentrybrowser').defaultIntegrations, + defaultStackParser: jest.requireActual('@sentry/browser').defaultStackParser, + makeFetchTransport: jest.requireActual('@sentry/browser').makeFetchTransport, + defaultIntegrations: jest.requireActual('@sentry/browser').defaultIntegrations, }; }); diff --git a/spec/frontend/sentry/legacy_index_spec.js b/spec/frontend/sentry/legacy_index_spec.js deleted file mode 100644 index fad1760ffc5..00000000000 --- a/spec/frontend/sentry/legacy_index_spec.js +++ /dev/null @@ -1,51 +0,0 @@ -import index from '~/sentry/legacy_index'; - -import LegacySentryConfig from '~/sentry/legacy_sentry_config'; - -describe('Sentry init', () => { - const dsn = 'https://123@sentry.gitlab.test/123'; - const environment = 'test'; - const currentUserId = '1'; - const gitlabUrl = 'gitlabUrl'; - const revision = 'revision'; - const featureCategory = 'my_feature_category'; - - beforeEach(() => { - window.gon = { - sentry_dsn: dsn, - sentry_environment: environment, - current_user_id: currentUserId, - gitlab_url: gitlabUrl, - revision, - feature_category: featureCategory, - }; - - jest.spyOn(LegacySentryConfig, 'init').mockImplementation(); - }); - - it('exports legacy version of Sentry in the global object', () => { - // eslint-disable-next-line no-underscore-dangle - expect(window._Sentry.SDK_VERSION).toMatch(/^5\./); - }); - - describe('when called', () => { - beforeEach(() => { - index(); - }); - - it('configures legacy sentry', () => { - expect(LegacySentryConfig.init).toHaveBeenCalledTimes(1); - expect(LegacySentryConfig.init).toHaveBeenCalledWith({ - dsn, - currentUserId, - whitelistUrls: [gitlabUrl, 'webpack-internal://'], - environment, - release: revision, - tags: { - revision, - feature_category: featureCategory, - }, - }); - }); - }); -}); diff --git a/spec/frontend/sentry/legacy_sentry_config_spec.js b/spec/frontend/sentry/legacy_sentry_config_spec.js deleted file mode 100644 index 94256784ca2..00000000000 --- a/spec/frontend/sentry/legacy_sentry_config_spec.js +++ /dev/null @@ -1,215 +0,0 @@ -import * as Sentry5 from 'sentrybrowser5'; -import LegacySentryConfig from '~/sentry/legacy_sentry_config'; - -describe('LegacySentryConfig', () => { - describe('IGNORE_ERRORS', () => { - it('should be an array of strings', () => { - const areStrings = LegacySentryConfig.IGNORE_ERRORS.every( - (error) => typeof error === 'string', - ); - - expect(areStrings).toBe(true); - }); - }); - - describe('DENYLIST_URLS', () => { - it('should be an array of regexps', () => { - const areRegExps = LegacySentryConfig.DENYLIST_URLS.every((url) => url instanceof RegExp); - - expect(areRegExps).toBe(true); - }); - }); - - describe('SAMPLE_RATE', () => { - it('should be a finite number', () => { - expect(typeof LegacySentryConfig.SAMPLE_RATE).toEqual('number'); - }); - }); - - describe('init', () => { - const options = { - currentUserId: 1, - }; - - beforeEach(() => { - jest.spyOn(LegacySentryConfig, 'configure'); - jest.spyOn(LegacySentryConfig, 'bindSentryErrors'); - jest.spyOn(LegacySentryConfig, 'setUser'); - - LegacySentryConfig.init(options); - }); - - it('should set the options property', () => { - expect(LegacySentryConfig.options).toEqual(options); - }); - - it('should call the configure method', () => { - expect(LegacySentryConfig.configure).toHaveBeenCalled(); - }); - - it('should call the error bindings method', () => { - expect(LegacySentryConfig.bindSentryErrors).toHaveBeenCalled(); - }); - - it('should call setUser', () => { - expect(LegacySentryConfig.setUser).toHaveBeenCalled(); - }); - - it('should not call setUser if there is no current user ID', () => { - LegacySentryConfig.setUser.mockClear(); - options.currentUserId = undefined; - - LegacySentryConfig.init(options); - - expect(LegacySentryConfig.setUser).not.toHaveBeenCalled(); - }); - }); - - describe('configure', () => { - const sentryConfig = {}; - const options = { - dsn: 'https://123@sentry.gitlab.test/123', - whitelistUrls: ['//gitlabUrl', 'webpack-internal://'], - environment: 'test', - release: 'revision', - tags: { - revision: 'revision', - feature_category: 'my_feature_category', - }, - }; - - beforeEach(() => { - jest.spyOn(Sentry5, 'init').mockImplementation(); - jest.spyOn(Sentry5, 'setTags').mockImplementation(); - - sentryConfig.options = options; - sentryConfig.IGNORE_ERRORS = 'ignore_errors'; - sentryConfig.DENYLIST_URLS = 'blacklist_urls'; - - LegacySentryConfig.configure.call(sentryConfig); - }); - - it('should call Sentry5.init', () => { - expect(Sentry5.init).toHaveBeenCalledWith({ - dsn: options.dsn, - release: options.release, - sampleRate: 0.95, - whitelistUrls: options.whitelistUrls, - environment: 'test', - ignoreErrors: sentryConfig.IGNORE_ERRORS, - blacklistUrls: sentryConfig.DENYLIST_URLS, - }); - }); - - it('should call Sentry5.setTags', () => { - expect(Sentry5.setTags).toHaveBeenCalledWith(options.tags); - }); - - it('should set environment from options', () => { - sentryConfig.options.environment = 'development'; - - LegacySentryConfig.configure.call(sentryConfig); - - expect(Sentry5.init).toHaveBeenCalledWith({ - dsn: options.dsn, - release: options.release, - sampleRate: 0.95, - whitelistUrls: options.whitelistUrls, - environment: 'development', - ignoreErrors: sentryConfig.IGNORE_ERRORS, - blacklistUrls: sentryConfig.DENYLIST_URLS, - }); - }); - }); - - describe('setUser', () => { - let sentryConfig; - - beforeEach(() => { - sentryConfig = { options: { currentUserId: 1 } }; - jest.spyOn(Sentry5, 'setUser'); - - LegacySentryConfig.setUser.call(sentryConfig); - }); - - it('should call .setUser', () => { - expect(Sentry5.setUser).toHaveBeenCalledWith({ - id: sentryConfig.options.currentUserId, - }); - }); - }); - - describe('handleSentryErrors', () => { - let event; - let req; - let config; - let err; - - beforeEach(() => { - event = {}; - req = { status: 'status', responseText: 'Unknown response text', statusText: 'statusText' }; - config = { type: 'type', url: 'url', data: 'data' }; - err = {}; - - jest.spyOn(Sentry5, 'captureMessage'); - - LegacySentryConfig.handleSentryErrors(event, req, config, err); - }); - - it('should call Sentry5.captureMessage', () => { - expect(Sentry5.captureMessage).toHaveBeenCalledWith(err, { - extra: { - type: config.type, - url: config.url, - data: config.data, - status: req.status, - response: req.responseText, - error: err, - event, - }, - }); - }); - - describe('if no err is provided', () => { - beforeEach(() => { - LegacySentryConfig.handleSentryErrors(event, req, config); - }); - - it('should use req.statusText as the error value', () => { - expect(Sentry5.captureMessage).toHaveBeenCalledWith(req.statusText, { - extra: { - type: config.type, - url: config.url, - data: config.data, - status: req.status, - response: req.responseText, - error: req.statusText, - event, - }, - }); - }); - }); - - describe('if no req.responseText is provided', () => { - beforeEach(() => { - req.responseText = undefined; - - LegacySentryConfig.handleSentryErrors(event, req, config, err); - }); - - it('should use `Unknown response text` as the response', () => { - expect(Sentry5.captureMessage).toHaveBeenCalledWith(err, { - extra: { - type: config.type, - url: config.url, - data: config.data, - status: req.status, - response: 'Unknown response text', - error: err, - event, - }, - }); - }); - }); - }); -}); diff --git a/spec/graphql/resolvers/base_resolver_spec.rb b/spec/graphql/resolvers/base_resolver_spec.rb index 16b6212a833..c46e750f91d 100644 --- a/spec/graphql/resolvers/base_resolver_spec.rb +++ b/spec/graphql/resolvers/base_resolver_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Resolvers::BaseResolver, feature_category: :api do include GraphqlHelpers + let_it_be(:user) { create(:user) } let(:resolver) do Class.new(described_class) do @@ -247,8 +248,6 @@ RSpec.describe Resolvers::BaseResolver, feature_category: :api do end describe '#object' do - let_it_be(:user) { create(:user) } - it 'returns object' do expect_next_instance_of(resolver) do |r| expect(r).to receive(:process).with(user) @@ -275,4 +274,18 @@ RSpec.describe Resolvers::BaseResolver, feature_category: :api do expect(instance.offset_pagination(User.none)).to be_a(::Gitlab::Graphql::Pagination::OffsetPaginatedRelation) end end + + describe '#authorized?' do + let(:object) { :object } + let(:scope_validator) { instance_double(::Gitlab::Auth::ScopeValidator) } + let(:context) { { current_user: user, scope_validator: scope_validator } } + + it 'delegates to authorization' do + expect(resolver.authorization).to be_kind_of(::Gitlab::Graphql::Authorize::ObjectAuthorization) + expect(resolver.authorization).to receive(:ok?) + .with(object, user, scope_validator: scope_validator) + + resolver.authorized?(object, context) + end + end end diff --git a/spec/graphql/types/base_enum_spec.rb b/spec/graphql/types/base_enum_spec.rb index db8fb877390..48ef38cbb63 100644 --- a/spec/graphql/types/base_enum_spec.rb +++ b/spec/graphql/types/base_enum_spec.rb @@ -139,4 +139,19 @@ RSpec.describe Types::BaseEnum, feature_category: :api do enum.values['TEST_VALUE'] end end + + describe '#authorized?' do + let(:object) { :object } + let(:scope_validator) { instance_double(::Gitlab::Auth::ScopeValidator) } + let(:user) { double } + let(:context) { { current_user: user, scope_validator: scope_validator } } + + it 'delegates to authorization' do + expect(described_class.authorization).to be_kind_of(::Gitlab::Graphql::Authorize::ObjectAuthorization) + expect(described_class.authorization).to receive(:ok?) + .with(object, user, scope_validator: scope_validator) + + described_class.authorized?(object, context) + end + end end diff --git a/spec/graphql/types/base_field_spec.rb b/spec/graphql/types/base_field_spec.rb index b52d5514368..6141259cd65 100644 --- a/spec/graphql/types/base_field_spec.rb +++ b/spec/graphql/types/base_field_spec.rb @@ -235,4 +235,21 @@ RSpec.describe Types::BaseField, feature_category: :api do described_class.new(**base_args.merge(args)) end end + + describe '#field_authorized?' do + let(:object) { :object } + let(:scope_validator) { instance_double(::Gitlab::Auth::ScopeValidator) } + let(:user) { :user } + let(:context) { { current_user: user, scope_validator: scope_validator } } + + it 'delegates to authorization' do + expect_next_instance_of(::Gitlab::Graphql::Authorize::ObjectAuthorization) do |authorization| + expect(authorization).to receive(:ok?) + .with(object, user, scope_validator: scope_validator) + end + + field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true) + field.authorized?(object, nil, context) + end + end end diff --git a/spec/graphql/types/base_object_spec.rb b/spec/graphql/types/base_object_spec.rb index af0639e84d3..2bf9f8e3f4d 100644 --- a/spec/graphql/types/base_object_spec.rb +++ b/spec/graphql/types/base_object_spec.rb @@ -12,15 +12,18 @@ RSpec.describe Types::BaseObject, feature_category: :api do true end - def ok?(object, _current_user) + def ok?(object, _current_user, scope_validator: nil) return false if object == { id: 100 } return false if object.try(:deactivated?) + raise "Missing scope_validator" unless scope_validator true end end end + let(:scope_validator) { instance_double(::Gitlab::Auth::ScopeValidator, valid_for?: true) } + let_it_be(:test_schema) do auth = custom_auth.new(nil) @@ -172,6 +175,7 @@ RSpec.describe Types::BaseObject, feature_category: :api do let(:data) do { + scope_validator: scope_validator, x: { title: 'Hey', ys: [{ id: 1 }, { id: 100 }, { id: 2 }] @@ -216,6 +220,7 @@ RSpec.describe Types::BaseObject, feature_category: :api do n = 7 data = { + scope_validator: scope_validator, x: { ys: (95..105).to_a.map { |id| { id: id } } } @@ -262,7 +267,10 @@ RSpec.describe Types::BaseObject, feature_category: :api do active_users = create_list(:user, 3, state: :active) inactive = create(:user, state: :deactivated) - data = { user_ids: [inactive, *active_users].map(&:id) } + data = { + scope_validator: scope_validator, + user_ids: [inactive, *active_users].map(&:id) + } doc = GraphQL.parse(<<~GQL) query { @@ -281,6 +289,7 @@ RSpec.describe Types::BaseObject, feature_category: :api do it 'filters polymorphic connections' do data = { current_user: :the_user, + scope_validator: scope_validator, x: { values: [{ value: 1 }, { value: 2 }, { value: 3 }, { value: 4 }] } @@ -324,6 +333,7 @@ RSpec.describe Types::BaseObject, feature_category: :api do it 'filters interface connections' do data = { current_user: :the_user, + scope_validator: scope_validator, x: { values: [{ value: 1 }, { value: 2 }, { value: 3 }, { value: 4 }] } @@ -368,6 +378,7 @@ RSpec.describe Types::BaseObject, feature_category: :api do it 'redacts polymorphic objects' do data = { current_user: :the_user, + scope_validator: scope_validator, x: { values: [{ value: 1 }] } @@ -408,7 +419,10 @@ RSpec.describe Types::BaseObject, feature_category: :api do inactive = create_list(:user, n - 1, state: :deactivated) active_users = create_list(:user, 2, state: :active) - data = { user_ids: [*inactive, *active_users].map(&:id) } + data = { + scope_validator: scope_validator, + user_ids: [*inactive, *active_users].map(&:id) + } doc = GraphQL.parse(<<~GQL) query { diff --git a/spec/lib/banzai/filter/markdown_engines/glfm_markdown_spec.rb b/spec/lib/banzai/filter/markdown_engines/glfm_markdown_spec.rb index da58b824a06..dfd7b8f0cce 100644 --- a/spec/lib/banzai/filter/markdown_engines/glfm_markdown_spec.rb +++ b/spec/lib/banzai/filter/markdown_engines/glfm_markdown_spec.rb @@ -5,13 +5,43 @@ require 'spec_helper' RSpec.describe Banzai::Filter::MarkdownEngines::GlfmMarkdown, feature_category: :team_planning do it 'defaults to generating sourcepos' do engine = described_class.new({}) + expected = <<~TEXT +
hi
+ TEXT - expect(engine.render('# hi')).to eq %(hi
\n) + expect(engine.render('# hi')).to eq expected end it 'turns off sourcepos' do engine = described_class.new({ no_sourcepos: true }) + expected = <<~TEXT +hi
+ TEXT - expect(engine.render('# hi')).to eq %(hi
\n) + expect(engine.render('# hi')).to eq expected + end + + it 'turns off header anchors' do + engine = described_class.new({ no_header_anchors: true, no_sourcepos: true }) + expected = <<~TEXT +hi
+ TEXT + + expect(engine.render('# hi')).to eq expected + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(native_header_anchors: false) + end + + it 'turns off header anchors' do + engine = described_class.new({ no_sourcepos: true }) + expected = <<~TEXT +hi
+ TEXT + + expect(engine.render('# hi')).to eq expected + end end end diff --git a/spec/lib/banzai/filter/sanitization_filter_spec.rb b/spec/lib/banzai/filter/sanitization_filter_spec.rb index fc3a50c5ce2..e40b65fce7b 100644 --- a/spec/lib/banzai/filter/sanitization_filter_spec.rb +++ b/spec/lib/banzai/filter/sanitization_filter_spec.rb @@ -192,5 +192,35 @@ RSpec.describe Banzai::Filter::SanitizationFilter, feature_category: :team_plann end end end + + describe 'link anchors' do + it 'allows id property on anchor links' do + exp = %q() + act = filter(exp) + + expect(act.to_html).to eq exp + end + + it 'removes id property for non-anchor links' do + exp = %q() + act = filter(%q()) + + expect(act.to_html).to eq exp + end + + it 'removes id property for non-user-content links' do + exp = %q() + act = filter(%q()) + + expect(act.to_html).to eq exp + end + + it 'removes class property for non-anchor links' do + exp = %q() + act = filter(%q()) + + expect(act.to_html).to eq exp + end + end end end diff --git a/spec/lib/banzai/filter/table_of_contents_filter_spec.rb b/spec/lib/banzai/filter/table_of_contents_filter_spec.rb index 31e6313215e..bd55cce8d3f 100644 --- a/spec/lib/banzai/filter/table_of_contents_filter_spec.rb +++ b/spec/lib/banzai/filter/table_of_contents_filter_spec.rb @@ -2,6 +2,10 @@ require 'spec_helper' +# TODO: This is now a legacy filter, and is only used with the Ruby parser. +# The current markdown parser now properly handles adding anchors to headers. +# The Ruby parser is now only for benchmarking purposes. +# issue: https://gitlab.com/gitlab-org/gitlab/-/issues/454601 RSpec.describe Banzai::Filter::TableOfContentsFilter, feature_category: :team_planning do include FilterSpecHelper @@ -9,20 +13,28 @@ RSpec.describe Banzai::Filter::TableOfContentsFilter, feature_category: :team_pl "#{text} \n" end + before do + stub_feature_flags(native_header_anchors: false) + end + + # TODO: enable when feature flag is removed + # let_it_be(:context) { { markdown_engine: Banzai::Filter::MarkdownFilter::CMARK_ENGINE } } + let_it_be(:context) { {} } + it 'does nothing when :no_header_anchors is truthy' do exp = act = header(1, 'Header') - expect(filter(act, no_header_anchors: 1).to_html).to eq exp + expect(filter(act, context.merge({ no_header_anchors: 1 })).to_html).to eq exp end it 'does nothing with empty headers' do exp = act = header(1, nil) - expect(filter(act).to_html).to eq exp + expect(filter(act, context).to_html).to eq exp end 1.upto(6) do |i| it "processes h#{i} elements" do html = header(i, "Header #{i}") - doc = filter(html) + doc = filter(html, context) expect(doc.css("h#{i} a").first.attr('id')).to eq "user-content-header-#{i}" end @@ -30,57 +42,57 @@ RSpec.describe Banzai::Filter::TableOfContentsFilter, feature_category: :team_pl describe 'anchor tag' do it 'has an `anchor` class' do - doc = filter(header(1, 'Header')) + doc = filter(header(1, 'Header'), context) expect(doc.css('h1 a').first.attr('class')).to eq 'anchor' end it 'has a namespaced id' do - doc = filter(header(1, 'Header')) + doc = filter(header(1, 'Header'), context) expect(doc.css('h1 a').first.attr('id')).to eq 'user-content-header' end it 'links to the non-namespaced id' do - doc = filter(header(1, 'Header')) + doc = filter(header(1, 'Header'), context) expect(doc.css('h1 a').first.attr('href')).to eq '#header' end describe 'generated IDs' do it 'translates spaces to dashes' do - doc = filter(header(1, 'This header has spaces in it')) + doc = filter(header(1, 'This header has spaces in it'), context) expect(doc.css('h1 a').first.attr('href')).to eq '#this-header-has-spaces-in-it' end it 'squeezes multiple spaces and dashes' do - doc = filter(header(1, 'This---header is poorly-formatted')) + doc = filter(header(1, 'This---header is poorly-formatted'), context) expect(doc.css('h1 a').first.attr('href')).to eq '#this-header-is-poorly-formatted' end it 'removes punctuation' do - doc = filter(header(1, "This, header! is, filled. with @ punctuation?")) + doc = filter(header(1, "This, header! is, filled. with @ punctuation?"), context) expect(doc.css('h1 a').first.attr('href')).to eq '#this-header-is-filled-with-punctuation' end it 'removes any leading or trailing spaces' do - doc = filter(header(1, " \r\n\tTitle with spaces\r\n\t ")) + doc = filter(header(1, " \r\n\tTitle with spaces\r\n\t "), context) expect(doc.css('h1 a').first.attr('href')).to eq '#title-with-spaces' end it 'appends a unique number to duplicates' do - doc = filter(header(1, 'One') + header(2, 'One')) + doc = filter(header(1, 'One') + header(2, 'One'), context) expect(doc.css('h1 a').first.attr('href')).to eq '#one' expect(doc.css('h2 a').first.attr('href')).to eq '#one-1' end it 'prepends a prefix to digits-only ids' do - doc = filter(header(1, "123") + header(2, "1.0")) + doc = filter(header(1, "123") + header(2, "1.0"), context) expect(doc.css('h1 a').first.attr('href')).to eq '#anchor-123' expect(doc.css('h2 a').first.attr('href')).to eq '#anchor-10' end it 'supports Unicode' do - doc = filter(header(1, '한글')) + doc = filter(header(1, '한글'), context) expect(doc.css('h1 a').first.attr('id')).to eq 'user-content-한글' # check that we encode the href to avoid issues with the # ExternalLinkFilter (see https://gitlab.com/gitlab-org/gitlab/issues/26210) @@ -88,7 +100,7 @@ RSpec.describe Banzai::Filter::TableOfContentsFilter, feature_category: :team_pl end it 'limits header href length with 255 characters' do - doc = filter(header(1, 'a' * 500)) + doc = filter(header(1, 'a' * 500), context) expect(doc.css('h1 a').first.attr('href')).to eq "##{'a' * 255}" end @@ -97,7 +109,7 @@ RSpec.describe Banzai::Filter::TableOfContentsFilter, feature_category: :team_pl describe 'result' do def result(html) - HTML::Pipeline.new([described_class]).call(html) + HTML::Pipeline.new([described_class], context).call(html) end let(:results) { result(header(1, 'Header 1') + header(2, 'Header 2')) } diff --git a/spec/lib/banzai/filter/table_of_contents_tag_filter_spec.rb b/spec/lib/banzai/filter/table_of_contents_tag_filter_spec.rb index 322225b38a9..fca44101657 100644 --- a/spec/lib/banzai/filter/table_of_contents_tag_filter_spec.rb +++ b/spec/lib/banzai/filter/table_of_contents_tag_filter_spec.rb @@ -44,4 +44,86 @@ RSpec.describe Banzai::Filter::TableOfContentsTagFilter, feature_category: :team end end end + + describe 'structure of a toc' do + def header(level, text) + "#{'#' * level} #{text}\n" + end + + def result(html) + HTML::Pipeline.new([Banzai::Filter::MarkdownFilter, described_class]).call(html) + end + + let(:results) { result("[toc]\n\n#{header(1, 'Header 1')}#{header(2, 'Header 2')}") } + let(:doc) { results[:output] } + + it 'is contained within a `ul` element' do + expect(doc.children.first.name).to eq 'ul' + expect(doc.children.first.attr('class')).to eq 'section-nav' + end + + it 'contains an `li` element for each header' do + expect(doc.css('li').length).to eq 2 + + links = doc.css('li a') + + expect(links.first.attr('href')).to eq '#header-1' + expect(links.first.text).to eq 'Header 1' + expect(links.last.attr('href')).to eq '#header-2' + expect(links.last.text).to eq 'Header 2' + end + + context 'table of contents nesting' do + let(:results) do + result( + <<~MARKDOWN + [toc] + + #{header(1, 'Header 1')} + #{header(2, 'Header 1-1')} + #{header(3, 'Header 1-1-1')} + #{header(2, 'Header 1-2')} + #{header(1, 'Header 2')} + #{header(2, 'Header 2-1')} + #{header(2, 'Header 2-1b')} + MARKDOWN + ) + end + + it 'keeps list levels regarding header levels' do + items = doc.css('li') + + # Header 1 + expect(items[0].ancestors).to satisfy_none { |node| node.name == 'li' } + + # Header 1-1 + expect(items[1].ancestors).to include(items[0]) + + # Header 1-1-1 + expect(items[2].ancestors).to include(items[0], items[1]) + + # Header 1-2 + expect(items[3].ancestors).to include(items[0]) + expect(items[3].ancestors).not_to include(items[1]) + + # Header 2 + expect(items[4].ancestors).to satisfy_none { |node| node.name == 'li' } + + # Header 2-1 + expect(items[5].ancestors).to include(items[4]) + + # Header 2-1b + expect(items[6].ancestors).to include(items[4]) + end + end + + context 'header text contains escaped content' do + let(:content) { '<img src="x" onerror="alert(42)">' } + let(:results) { result(header(1, content)) } + + it 'outputs escaped content' do + expect(doc.inner_html).to include(content) + end + end + end end diff --git a/spec/lib/gitlab/auth/o_auth/user_spec.rb b/spec/lib/gitlab/auth/o_auth/user_spec.rb index d02c9d16373..c1629f5a435 100644 --- a/spec/lib/gitlab/auth/o_auth/user_spec.rb +++ b/spec/lib/gitlab/auth/o_auth/user_spec.rb @@ -31,19 +31,130 @@ RSpec.describe Gitlab::Auth::OAuth::User, feature_category: :system_access do let(:ldap_user_2) { Gitlab::Auth::Ldap::Person.new(Net::LDAP::Entry.new, 'ldapmain') } describe '.find_by_uid_and_provider' do - let(:dn) { 'CN=John Åström, CN=Users, DC=Example, DC=com' } + let(:provider) { 'provider' } + let(:uid) { 'uid' } + let(:user) { create(:user) } + let!(:identity) { create(:identity, provider: provider, extern_uid: uid, user: user) } - it 'retrieves the correct user' do - special_info = { - name: 'John Åström', - email: 'john@example.com', - nickname: 'jastrom' - } - special_hash = OmniAuth::AuthHash.new(uid: dn, provider: 'ldapmain', info: special_info) - special_chars_user = described_class.new(special_hash) - user = special_chars_user.save + context 'when user exists for given uid and provider' do + it 'returns the user for given uid and provider' do + expect(described_class.find_by_uid_and_provider(uid, provider)).to eq user + end - expect(described_class.find_by_uid_and_provider(dn, 'ldapmain')).to eq user + context "when user's identity with untrusted extern_uid" do + before do + identity.update!(trusted_extern_uid: false) + end + + it 'raises Gitlab::Auth::OAuth::User::IdentityWithUntrustedExternUidError' do + expect { described_class.find_by_uid_and_provider(uid, provider) } + .to raise_error(Gitlab::Auth::OAuth::User::IdentityWithUntrustedExternUidError) + end + end + end + + context 'when user does not exist for given uid and provider' do + it 'returns nil' do + expect(described_class.find_by_uid_and_provider('unknown-uid', provider)).to eq nil + end + end + + context 'when identity exists for given uid and provider but is not tied to a user' do + before do + identity.update!(user: nil) + end + + it 'returns nil' do + expect(described_class.find_by_uid_and_provider(uid, provider)).to eq nil + end + end + + # This context should be removed by https://gitlab.com/gitlab-org/gitlab/-/issues/456424 + context 'for fallback that allows sign-ins to GitLab with Bitbucket identity with untrusted extern_uid when email matches' do + let(:provider) { 'bitbucket' } + let(:bitbucket_username) { user.username } + let(:bitbucket_email) { user.email } + let!(:identity) { create(:identity, provider: provider, extern_uid: user.username, user: user, trusted_extern_uid: false) } + + let(:auth_hash) do + OmniAuth::AuthHash.new(uid: uid, provider: provider, username: bitbucket_username, email: bitbucket_email) + end + + context 'when Bitbucket and GitLab accounts have the same email' do + it "updates the identity's extern_uid; updates the identity's trusted_extern_uid to true; returns the user" do + expect(identity.reload.extern_uid).not_to eq(uid) + expect(identity.reload.extern_uid).to eq(user.username) + expect(identity.reload.trusted_extern_uid).to eq(false) + + expect(described_class.find_by_uid_and_provider(uid, provider, auth_hash)).to eq user + + expect(identity.reload.extern_uid).to eq(uid) + expect(identity.reload.trusted_extern_uid).to eq(true) + end + end + + context 'when auth_hash argument is not provided' do + it 'returns nil' do + expect(described_class.find_by_uid_and_provider(uid, provider)).to eq nil + end + end + + context 'when Bitbucket and GitLab accounts have different emails' do + let(:bitbucket_email) { 'bitbucketuser@example.com' } + + it 'raises Gitlab::Auth::OAuth::User::IdentityWithUntrustedExternUidError' do + expect { described_class.find_by_uid_and_provider(uid, provider, auth_hash) } + .to raise_error(Gitlab::Auth::OAuth::User::IdentityWithUntrustedExternUidError) + end + end + + context 'when there is no Bitbucket identity with untrusted extern_uid' do + let(:bitbucket_username) { 'bitbucketuser' } + + it 'returns nil' do + expect(described_class.find_by_uid_and_provider(uid, provider, auth_hash)).to eq nil + end + end + + context 'when identity exists for given untrusted uid and provider but is not tied to a user' do + before do + identity.update!(user: nil) + end + + it 'raises Gitlab::Auth::OAuth::User::IdentityWithUntrustedExternUidError' do + expect { described_class.find_by_uid_and_provider(uid, provider, auth_hash) } + .to raise_error(Gitlab::Auth::OAuth::User::IdentityWithUntrustedExternUidError) + end + end + + context 'when bitbucket identity with trusted_extern_uid' do + let!(:identity) { create(:identity, provider: provider, extern_uid: uid, user: user) } + + it "does not update the identity and returns the user" do + identity_updated_at = identity.reload.updated_at + + expect(described_class.find_by_uid_and_provider(uid, provider, auth_hash)).to eq user + + expect(identity.reload.updated_at).to eq(identity_updated_at) + end + end + end + + context 'for LDAP' do + let(:dn) { 'CN=John Åström, CN=Users, DC=Example, DC=com' } + + it 'retrieves the correct user' do + special_info = { + name: 'John Åström', + email: 'john@example.com', + nickname: 'jastrom' + } + special_hash = OmniAuth::AuthHash.new(uid: dn, provider: 'ldapmain', info: special_info) + special_chars_user = described_class.new(special_hash) + user = special_chars_user.save + + expect(described_class.find_by_uid_and_provider(dn, 'ldapmain')).to eq user + end end end diff --git a/spec/lib/gitlab/background_migration/backfill_workspace_variables_project_id_spec.rb b/spec/lib/gitlab/background_migration/backfill_workspace_variables_project_id_spec.rb new file mode 100644 index 00000000000..3a1b3262fb7 --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_workspace_variables_project_id_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillWorkspaceVariablesProjectId, + feature_category: :remote_development, + schema: 20240419035356 do + include_examples 'desired sharding key backfill job' do + let(:batch_table) { :workspace_variables } + let(:backfill_column) { :project_id } + let(:backfill_via_table) { :workspaces } + let(:backfill_via_column) { :project_id } + let(:backfill_via_foreign_key) { :workspace_id } + end +end diff --git a/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb b/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb index 2064a592246..fcb5721382d 100644 --- a/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb +++ b/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb @@ -226,6 +226,30 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator, featu end end + context 'when given a malformed schema version' do + let(:security_report_failure) { 'using_unsupported_schema_version' } + + [ + '../../../../../../../../../spec/fixtures/security_reports/master/gl-secret-detection-report.json', + './fixtures/gl-secret-detection.json', + '%2e%2e%2f1.2.3' + ].each do |version| + context version do + let(:report_version) { version } + + it { is_expected.to be_falsey } + + it_behaves_like 'logs related information' + + it 'ensures version is not passed to schemer' do + expect(JSONSchemer).not_to receive(:schema) + + subject + end + end + end + end + context 'when not given a schema version' do let(:report_version) { nil } @@ -290,14 +314,13 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator, featu context 'and the report does not pass schema validation' do let(:report_data) do - valid_data['version'] = "V2.7.0" + valid_data['version'] = "2.7.0" valid_data.delete('vulnerabilities') valid_data end let(:expected_errors) do [ - "property '/version' does not match pattern: ^[0-9]+\\.[0-9]+\\.[0-9]+$", "root is missing required keys: vulnerabilities" ] end @@ -356,7 +379,6 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator, featu let(:expected_errors) do [ - "root is missing required keys: version", expected_missing_version_message ] end @@ -398,7 +420,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator, featu context 'when given a deprecated schema version' do let(:deprecations_hash) do { - dast: %w[V2.7.0] + dast: %w[2.7.0] } end @@ -426,7 +448,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator, featu context 'and the report does not pass schema validation' do let(:report_data) do - valid_data['version'] = "V2.7.0" + valid_data['version'] = "2.7.0" valid_data.delete('vulnerabilities') valid_data end diff --git a/spec/lib/gitlab/content_security_policy/config_loader_spec.rb b/spec/lib/gitlab/content_security_policy/config_loader_spec.rb index a795957f81e..9abea9f1281 100644 --- a/spec/lib/gitlab/content_security_policy/config_loader_spec.rb +++ b/spec/lib/gitlab/content_security_policy/config_loader_spec.rb @@ -346,28 +346,14 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader, feature_category: :s end context 'when sentry is configured' do - let(:legacy_dsn) { 'dummy://abc@legacy-sentry.example.com/1' } let(:dsn) { 'dummy://def@sentry.example.com/2' } before do stub_config_setting(host: 'gitlab.example.com') end - context 'when legacy sentry is configured' do - before do - allow(Gitlab.config.sentry).to receive(:enabled).and_return(true) - allow(Gitlab.config.sentry).to receive(:clientside_dsn).and_return(legacy_dsn) - allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(false) - end - - it 'adds legacy sentry path to CSP' do - expect(connect_src).to eq("'self' ws://gitlab.example.com dummy://legacy-sentry.example.com") - end - end - context 'when sentry is configured' do before do - allow(Gitlab.config.sentry).to receive(:enabled).and_return(false) allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(true) allow(Gitlab::CurrentSettings).to receive(:sentry_clientside_dsn).and_return(dsn) end @@ -379,8 +365,6 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader, feature_category: :s context 'when sentry settings are from older schemas and sentry setting are missing' do before do - allow(Gitlab.config.sentry).to receive(:enabled).and_return(false) - allow(Gitlab::CurrentSettings).to receive(:respond_to?).with(:sentry_enabled).and_return(false) allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_raise(NoMethodError) @@ -392,31 +376,6 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader, feature_category: :s expect(connect_src).to eq("'self' ws://gitlab.example.com") end end - - context 'when legacy sentry and sentry are both configured' do - let(:connect_src_expectation) do - # rubocop:disable Lint/PercentStringArray - %w[ - 'self' - ws://gitlab.example.com - dummy://legacy-sentry.example.com - dummy://sentry.example.com - ].join(' ') - # rubocop:enable Lint/PercentStringArray - end - - before do - allow(Gitlab.config.sentry).to receive(:enabled).and_return(true) - allow(Gitlab.config.sentry).to receive(:clientside_dsn).and_return(legacy_dsn) - - allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(true) - allow(Gitlab::CurrentSettings).to receive(:sentry_clientside_dsn).and_return(dsn) - end - - it 'adds both sentry paths to CSP' do - expect(connect_src).to eq(connect_src_expectation) - end - end end describe 'Customer portal frames' do diff --git a/spec/lib/gitlab/error_tracking/log_formatter_spec.rb b/spec/lib/gitlab/error_tracking/log_formatter_spec.rb index 15d201401f4..7773e33a5ff 100644 --- a/spec/lib/gitlab/error_tracking/log_formatter_spec.rb +++ b/spec/lib/gitlab/error_tracking/log_formatter_spec.rb @@ -27,9 +27,9 @@ RSpec.describe Gitlab::ErrorTracking::LogFormatter do end before do - Raven.context.user[:user_flag] = 'flag' - Raven.context.tags[:shard] = 'catchall' - Raven.context.extra[:some_info] = 'info' + Sentry.get_current_scope.user[:user_flag] = 'flag' + Sentry.get_current_scope.tags[:shard] = 'catchall' + Sentry.get_current_scope.extra[:some_info] = 'info' allow(exception).to receive(:backtrace).and_return( [ @@ -40,7 +40,7 @@ RSpec.describe Gitlab::ErrorTracking::LogFormatter do end after do - ::Raven::Context.clear! + Sentry.get_current_scope.clear end it 'appends error-related log fields and filters sensitive Sidekiq arguments' do diff --git a/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb index c9b632b50e1..cf0e45eb833 100644 --- a/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb @@ -4,18 +4,29 @@ require 'spec_helper' RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do describe '.call' do - let(:required_options) do + let(:event) { Sentry::Event.new(configuration: Sentry.configuration) } + let(:result_hash) { described_class.call(event).to_hash } + let(:payload) do { - configuration: Raven.configuration, - context: Raven.context, - breadcrumbs: Raven.breadcrumbs + user: { ip_address: '127.0.0.1' }, + tags: { priority: 'high' }, + extra: { sidekiq: { class: 'SomeWorker', args: ['[FILTERED]', 1, 2] } } } end - let(:event) { Raven::Event.new(required_options.merge(payload)) } - let(:result_hash) { described_class.call(event).to_hash } + around do |example| + Sentry.with_scope do |scope| + scope.set_user(payload[:user]) + scope.set_tags(payload[:tags]) + scope.set_extras(payload[:extra]) + + example.run + end + end before do + Sentry.get_current_scope.apply_to_event(event) + allow_next_instance_of(Gitlab::ErrorTracking::ContextPayloadGenerator) do |generator| allow(generator).to receive(:generate).and_return( user: { username: 'root' }, @@ -25,14 +36,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do end end - let(:payload) do - { - user: { ip_address: '127.0.0.1' }, - tags: { priority: 'high' }, - extra: { sidekiq: { class: 'SomeWorker', args: ['[FILTERED]', 1, 2] } } - } - end - it 'merges the context payload into event payload', :aggregate_failures do expect(result_hash[:user]).to include(ip_address: '127.0.0.1', username: 'root') diff --git a/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb index 3399c6dd9f4..2c886738f09 100644 --- a/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb @@ -4,19 +4,6 @@ require 'spec_helper' RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor, :sentry, feature_category: :integrations do describe '.call' do - let(:raven_required_options) do - { - configuration: Raven.configuration, - context: Raven.context, - breadcrumbs: Raven.breadcrumbs - } - end - - let(:raven_event) do - Raven::Event - .from_exception(exception, raven_required_options.merge(data)) - end - let(:sentry_event) do Sentry.get_current_client.event_from_exception(exception) end @@ -52,12 +39,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor, :sentry, fe it { expect(result_hash).to include(data) } end - context 'with Raven event' do - let(:event) { raven_event } - - it_behaves_like 'leaves data unchanged' - end - context 'with Sentry event' do let(:event) { sentry_event } @@ -97,12 +78,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor, :sentry, fe end end - context 'with Raven event' do - let(:event) { raven_event } - - it_behaves_like 'processes the exception' - end - context 'with Sentry event' do let(:event) { sentry_event } @@ -160,12 +135,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor, :sentry, fe end end - context 'with Raven event' do - let(:event) { raven_event } - - it_behaves_like 'processes the exception' - end - context 'with Sentry event' do let(:event) { sentry_event } diff --git a/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb index 74b6df24178..8cf93287c2c 100644 --- a/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/sanitize_error_message_processor_spec.rb @@ -20,20 +20,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SanitizeErrorMessageProcessor, end end - context 'with Raven event' do - let(:raven_required_options) do - { - configuration: Raven.configuration, - context: Raven.context, - breadcrumbs: Raven.breadcrumbs - } - end - - let(:event) { Raven::Event.from_exception(exception, raven_required_options) } - - it_behaves_like 'processes the exception' - end - context 'with Sentry event' do let(:event) { Sentry.get_current_client.event_from_exception(exception) } diff --git a/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb index bc4526758c0..4dd780addf0 100644 --- a/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb @@ -97,18 +97,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor, :sentry do describe '.call' do let(:exception) { StandardError.new('Test exception') } - let(:raven_required_options) do - { - configuration: Raven.configuration, - context: Raven.context, - breadcrumbs: Raven.breadcrumbs - } - end - - let(:raven_event) do - Raven::Event.new(raven_required_options.merge(wrapped_value)) - end - let(:sentry_event) do Sentry.get_current_client.event_from_exception(exception) end @@ -158,12 +146,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor, :sentry do end context 'when processing via the default error handler' do - context 'with Raven events' do - let(:event) { raven_event } - - include_examples 'Sidekiq arguments', args_in_job_hash: true - end - context 'with Sentry events' do let(:event) { sentry_event } @@ -172,12 +154,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor, :sentry do end context 'when processing via Gitlab::ErrorTracking' do - context 'with Raven events' do - let(:event) { raven_event } - - include_examples 'Sidekiq arguments', args_in_job_hash: false - end - context 'with Sentry events' do let(:event) { sentry_event } @@ -208,12 +184,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor, :sentry do end end - context 'with Raven events' do - let(:event) { raven_event } - - it_behaves_like 'handles jobstr fields' - end - context 'with Sentry events' do let(:event) { sentry_event } @@ -232,12 +202,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor, :sentry do end end - context 'with Raven events' do - let(:event) { raven_event } - - it_behaves_like 'does nothing' - end - context 'with Sentry events' do let(:event) { sentry_event } @@ -255,12 +219,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor, :sentry do end end - context 'with Raven events' do - let(:event) { raven_event } - - it_behaves_like 'does nothing' - end - context 'with Sentry events' do let(:event) { sentry_event } diff --git a/spec/lib/gitlab/error_tracking_spec.rb b/spec/lib/gitlab/error_tracking_spec.rb index 7b2c5ca27cb..5c7b3e72fbe 100644 --- a/spec/lib/gitlab/error_tracking_spec.rb +++ b/spec/lib/gitlab/error_tracking_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true require 'spec_helper' - -require 'raven/transports/dummy' require 'sentry/transport/dummy_transport' RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do @@ -44,18 +42,11 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do } end - let(:raven_event) do - event = Raven.client.transport.events.last[1] - Gitlab::Json.parse(event) - end - let(:sentry_event) do Sentry.get_current_client.transport.events.last end before do - stub_feature_flags(enable_old_sentry_integration: true) - stub_feature_flags(enable_new_sentry_integration: true) stub_sentry_settings allow(described_class).to receive(:sentry_configurable?).and_return(true) @@ -86,7 +77,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do end it 'raises the exception' do - expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) expect do @@ -106,7 +96,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do end it 'includes additional tags' do - expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) expect do @@ -126,7 +115,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do end it 'logs the exception with all attributes passed' do - expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) described_class.track_and_raise_for_dev_exception( @@ -150,7 +138,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do describe '.track_and_raise_exception' do it 'always raises the exception' do - expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) expect do @@ -195,7 +182,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do end it 'only logs and raises the exception' do - expect(Raven).not_to receive(:capture_exception) expect(Sentry).not_to receive(:capture_exception) expect(Gitlab::ErrorTracking::Logger).to receive(:error).with(logger_payload) @@ -233,17 +219,13 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do end before do - allow(Raven).to receive(:capture_exception).and_call_original allow(Sentry).to receive(:capture_exception).and_call_original allow(Gitlab::ErrorTracking::Logger).to receive(:error) end - it 'calls Raven.capture_exception' do + it 'calls Sentry.capture_exception' do track_exception - expect(Raven) - .to have_received(:capture_exception).with(exception, sentry_payload) - expect(Sentry) .to have_received(:capture_exception).with(exception, sentry_payload) end @@ -296,10 +278,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do it 'includes the extra data from the exception in the tracking information' do track_exception - expect(Raven).to have_received(:capture_exception).with( - exception, a_hash_including(extra: a_hash_including(extra_info)) - ) - expect(Sentry).to have_received(:capture_exception).with( exception, a_hash_including(extra: a_hash_including(extra_info)) ) @@ -316,10 +294,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do it 'just includes the other extra info' do track_exception - expect(Raven).to have_received(:capture_exception).with( - exception, a_hash_including(extra: a_hash_including(extra)) - ) - expect(Sentry).to have_received(:capture_exception).with( exception, a_hash_including(extra: a_hash_including(extra)) ) @@ -331,40 +305,10 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do subject(:track_exception) { described_class.track_exception(exception, extra) } before do - allow(Raven).to receive(:capture_exception).and_call_original allow(Sentry).to receive(:capture_exception).and_call_original allow(Gitlab::ErrorTracking::Logger).to receive(:error) end - # This is a workaround for restoring Raven's user context below. - # Raven.user_context(&block) does not restore the user context correctly. - around do |example| - previous_user_context = Raven.context.user.dup - example.run - ensure - Raven.context.user = previous_user_context - end - - context 'with custom GitLab context when using Raven.capture_exception directly' do - subject(:track_exception) { Raven.capture_exception(exception) } - - it 'merges a default set of tags into the existing tags' do - allow(Raven.context).to receive(:tags).and_return(foo: 'bar') - - track_exception - - expect(raven_event['tags']).to include('correlation_id', 'feature_category', 'foo', 'locale', 'program') - end - - it 'merges the current user information into the existing user information' do - Raven.user_context(id: -1) - - track_exception - - expect(raven_event['user']).to eq('id' => -1, 'username' => user.username) - end - end - context 'with custom GitLab context when using Sentry.capture_exception directly' do subject(:track_exception) { Sentry.capture_exception(exception) } @@ -418,7 +362,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do track_exception expected_data = [1, { 'id' => 2, 'name' => 'hello' }, 'some-value', 'another-value'] - expect(raven_event.dig('extra', 'sidekiq', 'args')).to eq(expected_data) expect(sentry_event.extra[:sidekiq]['args']).to eq(expected_data) end end @@ -429,7 +372,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do it 'filters sensitive arguments before sending and logging' do track_exception - expect(raven_event.dig('extra', 'sidekiq', 'args')).to eq(['[FILTERED]', 1, 2]) expect(sentry_event.extra[:sidekiq]['args']).to eq(['[FILTERED]', 1, 2]) expect(Gitlab::ErrorTracking::Logger).to have_received(:error).with( hash_including( @@ -450,8 +392,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do it 'sets the GRPC debug error string in the Sentry event and adds a custom fingerprint' do track_exception - expect(raven_event.dig('extra', 'grpc_debug_error_string')).to eq('{"hello":1}') - expect(raven_event['fingerprint']).to eq(['GRPC::DeadlineExceeded', '4:unknown cause.']) expect(sentry_event.extra[:grpc_debug_error_string]).to eq('{"hello":1}') expect(sentry_event.fingerprint).to eq(['GRPC::DeadlineExceeded', '4:unknown cause.']) end @@ -463,8 +403,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do it 'does not do any processing on the event' do track_exception - expect(raven_event['extra']).not_to include('grpc_debug_error_string') - expect(raven_event['fingerprint']).to eq(['GRPC::DeadlineExceeded', '4:unknown cause']) expect(sentry_event.extra).not_to include(:grpc_debug_error_string) expect(sentry_event.fingerprint).to eq(['GRPC::DeadlineExceeded', '4:unknown cause']) end @@ -485,7 +423,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do track_exception - expect(Raven.client.transport.events).to eq([]) expect(Sentry.get_current_client.transport.events).to eq([]) end end @@ -494,7 +431,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do context 'when processing invalid URI exceptions' do let(:invalid_uri) { 'http://foo:bar' } - let(:raven_exception_values) { raven_event['exception']['values'] } let(:sentry_exception_values) { sentry_event.exception.to_hash[:values] } context 'when the error is a URI::InvalidURIError' do @@ -507,12 +443,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do it 'filters the URI from the error message' do track_exception - expect(raven_exception_values).to include( - hash_including( - 'type' => 'URI::InvalidURIError', - 'value' => 'bad URI(is not URI?): [FILTERED]' - ) - ) expect(sentry_exception_values).to include( hash_including( type: 'URI::InvalidURIError', @@ -532,12 +462,6 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do it 'filters the URI from the error message' do track_exception - expect(raven_exception_values).to include( - hash_including( - 'type' => 'Addressable::URI::InvalidURIError', - 'value' => 'Invalid port number: [FILTERED]' - ) - ) expect(sentry_exception_values).to include( hash_including( type: 'Addressable::URI::InvalidURIError', @@ -573,7 +497,7 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do context 'when ENABLE_SENTRY_PERFORMANCE_MONITORING env is disabled' do before do stub_env('ENABLE_SENTRY_PERFORMANCE_MONITORING', false) - described_class.configure_sentry # Force re-initialization to reset traces_sample_rate setting + described_class.configure # Force re-initialization to reset traces_sample_rate setting end it 'does not set traces_sample_rate' do @@ -584,7 +508,7 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do context 'when ENABLE_SENTRY_PERFORMANCE_MONITORING env is enabled' do before do stub_env('ENABLE_SENTRY_PERFORMANCE_MONITORING', true) - described_class.configure_sentry # Force re-initialization to reset traces_sample_rate setting + described_class.configure # Force re-initialization to reset traces_sample_rate setting end it 'sets traces_sample_rate' do @@ -592,4 +516,37 @@ RSpec.describe Gitlab::ErrorTracking, feature_category: :shared do end end end + + describe '.configure' do + using RSpec::Parameterized::TableSyntax + + let(:envdsn) { 'dummy://b44a0828b72421a6d8e99efd68d44fa8@example.com/41' } + let(:appdsn) { 'dummy://b44a0828b72421a6d8e99efd68d44fa8@example.com/42' } + + where(:env_enabled, :env_dsn, :env_env, :app_enabled, :app_dsn, :app_env, :dsn, :environment) do + true | ref(:envdsn) | 'eprd' | true | ref(:appdsn) | 'aprd' | ref(:envdsn) | 'eprd' + false | ref(:envdsn) | 'eprd' | true | ref(:appdsn) | 'aprd' | ref(:appdsn) | 'aprd' + true | ref(:envdsn) | 'eprd' | false | ref(:appdsn) | 'aprd' | ref(:envdsn) | 'eprd' + false | ref(:envdsn) | 'eprd' | false | ref(:appdsn) | 'aprd' | '' | '' + end + with_them do + before do + allow(Gitlab.config.sentry).to receive(:enabled) { env_enabled } + allow(Gitlab::CurrentSettings).to receive(:sentry_enabled?) { app_enabled } + + allow(Gitlab.config.sentry).to receive(:dsn) { env_dsn } + allow(Gitlab::CurrentSettings).to receive(:sentry_dsn) { app_dsn } + + allow(Gitlab.config.sentry).to receive(:environment).and_return('eprd') + allow(Gitlab::CurrentSettings).to receive(:sentry_environment).and_return('aprd') + + described_class.configure + end + + it 'selects the correct settings' do + expect(Sentry.configuration.dsn.to_s).to eq(dsn) + expect(Sentry.configuration.environment).to eq(environment) + end + end + end end diff --git a/spec/lib/gitlab/file_finder_spec.rb b/spec/lib/gitlab/file_finder_spec.rb index 8afaec3c381..ac4dde8a634 100644 --- a/spec/lib/gitlab/file_finder_spec.rb +++ b/spec/lib/gitlab/file_finder_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Gitlab::FileFinder, feature_category: :global_search do describe '#find' do let_it_be(:project) { create(:project, :public, :repository) } - subject { described_class.new(project, project.default_branch) } + subject(:file_finder) { described_class.new(project, project.default_branch) } it_behaves_like 'file finder' do let(:expected_file_by_path) { 'files/images/wm.svg' } @@ -14,40 +14,46 @@ RSpec.describe Gitlab::FileFinder, feature_category: :global_search do end context 'with inclusive filters' do - it 'filters by filename' do - results = subject.find('files filename:wm.svg') + it 'filters by filename and ignores case', :aggregate_failures do + results = file_finder.find('files filename:wm.svg') + expect(results.count).to eq(1) + results = file_finder.find('files filename:Wm.svg') expect(results.count).to eq(1) end - it 'filters by path' do - results = subject.find('white path:images') + it 'filters by path and ignores case', :aggregate_failures do + results = file_finder.find('white path:images') + expect(results.count).to eq(2) + results = file_finder.find('white path:imAGes') expect(results.count).to eq(2) end - it 'filters by extension' do - results = subject.find('files extension:md') + it 'filters by extension and ignores case', :aggregate_failures do + results = file_finder.find('files extension:md') + expect(results.count).to eq(4) + results = file_finder.find('files extension:MD') expect(results.count).to eq(4) end end context 'with exclusive filters' do it 'filters by filename' do - results = subject.find('files -filename:wm.svg') + results = file_finder.find('files -filename:wm.svg') expect(results.count).to eq(26) end it 'filters by path' do - results = subject.find('white -path:images') + results = file_finder.find('white -path:images') expect(results.count).to eq(5) end it 'filters by extension' do - results = subject.find('files -extension:md') + results = file_finder.find('files -extension:md') expect(results.count).to eq(23) end @@ -64,7 +70,34 @@ RSpec.describe Gitlab::FileFinder, feature_category: :global_search do it 'does not cause N+1 query' do expect(Gitlab::GitalyClient).to receive(:call).at_most(10).times.and_call_original - subject.find(': filename:wm.svg') + file_finder.find(': filename:wm.svg') + end + + context 'for protection against ReDOS' do + # filter is run once for each result returned, searching for `PROCESS.md` returns 2 results + it 'utilizes ::Gitlab::UntrustedRegexp for filename filter' do + query = "PROCESS.md filename:P#{'*' * 50}m" + expected_regex_value = "(?i)p#{'.*?' * 50}m$" + expect(::Gitlab::UntrustedRegexp).to receive(:new).with(expected_regex_value).twice.and_call_original + + file_finder.find(query) + end + + it 'utilizes ::Gitlab::UntrustedRegexp for path filter' do + query = "PROCESS.md path:P#{'*' * 10}m" + expected_regex_value = "(?i)p#{'.*?' * 10}m" + + expect(::Gitlab::UntrustedRegexp).to receive(:new).with(expected_regex_value).twice.and_call_original + file_finder.find(query) + end + + it 'utilizes ::Gitlab::UntrustedRegexp for extension filter' do + query = "PROCESS.md extension:#{'*' * 10}md" + expected_regex_value = "(?i)\\.#{'.*?' * 10}md$" + + expect(::Gitlab::UntrustedRegexp).to receive(:new).with(expected_regex_value).twice.and_call_original + file_finder.find(query) + end end end end diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index e462f5a8366..1ac4efcf3d2 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -1023,10 +1023,32 @@ RSpec.describe Gitlab::GitalyClient::OperationService, feature_category: :source end describe '#user_commit_files' do + let(:force) { false } + let(:start_sha) { nil } + let(:sign) { true } + subject do client.user_commit_files( - gitaly_user, 'my-branch', 'Commit files message', [], 'janedoe@example.com', 'Jane Doe', - 'master', repository) + user, 'my-branch', 'Commit files message', [], 'janedoe@example.com', 'Jane Doe', + 'master', repository, force, start_sha, sign) + end + + context 'when UserCommitFiles RPC is called' do + let(:force) { true } + let(:start_sha) { project.commit.id } + let(:sign) { false } + + it 'successfully builds the header' do + expect_any_instance_of(Gitaly::OperationService::Stub).to receive(:user_commit_files) do |_, req_enum| + header = req_enum.first.header + + expect(header.force).to eq(force) + expect(header.start_sha).to eq(start_sha) + expect(header.sign).to eq(sign) + end.and_return(Gitaly::UserCommitFilesResponse.new) + + subject + end end context 'with unstructured errors' do diff --git a/spec/lib/gitlab/gon_helper_spec.rb b/spec/lib/gitlab/gon_helper_spec.rb index e95623a01e1..4fc9a525792 100644 --- a/spec/lib/gitlab/gon_helper_spec.rb +++ b/spec/lib/gitlab/gon_helper_spec.rb @@ -60,19 +60,6 @@ RSpec.describe Gitlab::GonHelper, feature_category: :shared do let(:environment) { 'staging' } let(:sentry_clientside_traces_sample_rate) { 0.5 } - context 'with legacy sentry configuration' do - before do - stub_config(sentry: { enabled: true, clientside_dsn: clientside_dsn, environment: environment }) - end - - it 'sets sentry dsn and environment from config' do - expect(gon).to receive(:sentry_dsn=).with(clientside_dsn) - expect(gon).to receive(:sentry_environment=).with(environment) - - helper.add_gon_variables - end - end - context 'with sentry settings' do before do stub_application_setting(sentry_enabled: true) @@ -88,21 +75,6 @@ RSpec.describe Gitlab::GonHelper, feature_category: :shared do helper.add_gon_variables end - - context 'when enable_new_sentry_integration is disabled' do - before do - stub_feature_flags(enable_new_sentry_integration: false) - end - - it 'does not set sentry dsn and environment from config' do - expect(gon).not_to receive(:sentry_dsn=).with(clientside_dsn) - expect(gon).not_to receive(:sentry_environment=).with(environment) - expect(gon).not_to receive(:sentry_clientside_traces_sample_rate=) - .with(sentry_clientside_traces_sample_rate) - - helper.add_gon_variables - end - end end end end diff --git a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb index 1cd93d7b364..be94c2a958f 100644 --- a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb +++ b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb @@ -7,13 +7,14 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do Class.new do include Gitlab::Graphql::Authorize::AuthorizeResource - attr_reader :user, :found_object + attr_reader :user, :found_object, :scope_validator authorize :read_the_thing - def initialize(user, found_object) + def initialize(user, found_object, scope_validator) @user = user @found_object = found_object + @scope_validator = scope_validator end def find_object @@ -25,7 +26,7 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do end def context - { current_user: user } + { current_user: user, scope_validator: scope_validator } end def self.authorization @@ -35,9 +36,10 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do end let(:user) { build(:user) } + let(:scope_validator) { instance_double(::Gitlab::Auth::ScopeValidator, valid_for?: true) } let(:project) { build(:project) } - subject(:loading_resource) { fake_class.new(user, project) } + subject(:loading_resource) { fake_class.new(user, project, scope_validator) } before do # don't allow anything by default @@ -139,4 +141,16 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do expect(child).to be_authorizes_object end end + + describe '#authorized_resource?' do + let(:object) { :object } + + it 'delegates to authorization' do + expect(fake_class.authorization).to be_kind_of(::Gitlab::Graphql::Authorize::ObjectAuthorization) + expect(fake_class.authorization).to receive(:ok?) + .with(object, user, scope_validator: scope_validator) + + fake_class.new(user, :found_object, scope_validator).authorized_resource?(object) + end + end end diff --git a/spec/lib/gitlab/graphql/authorize/object_authorization_spec.rb b/spec/lib/gitlab/graphql/authorize/object_authorization_spec.rb index 274cc83a6be..54b3e260524 100644 --- a/spec/lib/gitlab/graphql/authorize/object_authorization_spec.rb +++ b/spec/lib/gitlab/graphql/authorize/object_authorization_spec.rb @@ -4,9 +4,10 @@ require 'spec_helper' RSpec.describe ::Gitlab::Graphql::Authorize::ObjectAuthorization do describe '#ok?' do - subject { described_class.new(%i[go_fast go_slow]) } + subject(:authorization) { described_class.new(%i[go_fast go_slow]) } let(:user) { double(:User, id: 10001) } + let(:scope_validator) { instance_double(::Gitlab::Auth::ScopeValidator, valid_for?: true) } let(:policy) do Class.new(::DeclarativePolicy::Base) do @@ -31,25 +32,25 @@ RSpec.describe ::Gitlab::Graphql::Authorize::ObjectAuthorization do context 'when there are no abilities' do subject { described_class.new([]) } - it { is_expected.to be_ok(double, double) } + it { is_expected.to be_ok(double, double, scope_validator: scope_validator) } end context 'when no ability should be allowed' do let(:object) { Foo.new(0, 0) } - it { is_expected.not_to be_ok(object, user) } + it { is_expected.not_to be_ok(object, user, scope_validator: scope_validator) } end context 'when go_fast should be allowed' do let(:object) { Foo.new(100, 0) } - it { is_expected.not_to be_ok(object, user) } + it { is_expected.not_to be_ok(object, user, scope_validator: scope_validator) } end context 'when go_fast and go_slow should be allowed' do let(:object) { Foo.new(100, 100) } - it { is_expected.to be_ok(object, user) } + it { is_expected.to be_ok(object, user, scope_validator: scope_validator) } end context 'when the object delegates to another subject' do @@ -57,8 +58,19 @@ RSpec.describe ::Gitlab::Graphql::Authorize::ObjectAuthorization do double(:Proxy, declarative_policy_subject: foo) end - it { is_expected.to be_ok(proxy(Foo.new(100, 100)), user) } - it { is_expected.not_to be_ok(proxy(Foo.new(0, 100)), user) } + it { is_expected.to be_ok(proxy(Foo.new(100, 100)), user, scope_validator: scope_validator) } + it { is_expected.not_to be_ok(proxy(Foo.new(0, 100)), user, scope_validator: scope_validator) } + end + + context 'when scope is not valid for scope validator' do + let(:object) { Foo.new(100, 100) } + + it 'returns false' do + expect(scope_validator).to receive(:valid_for?).with([:api, :read_api]) + .and_return(false) + + expect(authorization).not_to be_ok(object, user, scope_validator: scope_validator) + end end end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 14bfe1c8d3a..3b5db907428 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -256,6 +256,7 @@ merge_requests: - user_agent_detail - scan_result_policy_violations - applicable_post_merge_approval_rules +- requested_changes external_pull_requests: - project merge_request_diff: diff --git a/spec/lib/gitlab/rack_attack/request_spec.rb b/spec/lib/gitlab/rack_attack/request_spec.rb index 92c9acb83cf..a076cb557a0 100644 --- a/spec/lib/gitlab/rack_attack/request_spec.rb +++ b/spec/lib/gitlab/rack_attack/request_spec.rb @@ -248,6 +248,38 @@ RSpec.describe Gitlab::RackAttack::Request, feature_category: :rate_limiting do end end + describe '#throttle_unauthenticated_git_http?' do + let_it_be(:project) { create(:project) } + + let(:git_clone_project_path_get_info_refs) { "/#{project.full_path}.git/info/refs?service=git-upload-pack" } + let(:git_clone_path_post_git_upload_pack) { "/#{project.full_path}.git/git-upload-pack" } + + subject { request.throttle_unauthenticated_git_http? } + + where(:path, :request_unauthenticated?, :application_setting_throttle_unauthenticated_git_http_enabled, :expected) do + ref(:git_clone_project_path_get_info_refs) | true | true | true + ref(:git_clone_project_path_get_info_refs) | false | true | false + ref(:git_clone_project_path_get_info_refs) | true | false | false + ref(:git_clone_project_path_get_info_refs) | false | false | false + + ref(:git_clone_path_post_git_upload_pack) | true | true | true + ref(:git_clone_path_post_git_upload_pack) | false | false | false + + '/users/sign_in' | true | true | false + '/users/sign_in' | false | false | false + end + + with_them do + before do + stub_application_setting(throttle_unauthenticated_git_http_enabled: application_setting_throttle_unauthenticated_git_http_enabled) + + allow(request).to receive(:unauthenticated?).and_return(request_unauthenticated?) + end + + it { is_expected.to eq expected } + end + end + describe '#protected_path?' do subject { request.protected_path? } diff --git a/spec/lib/gitlab/usage_data_counters/productivity_analytics_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/productivity_analytics_counter_spec.rb deleted file mode 100644 index 34b2cfcf1de..00000000000 --- a/spec/lib/gitlab/usage_data_counters/productivity_analytics_counter_spec.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::UsageDataCounters::ProductivityAnalyticsCounter do - it_behaves_like 'a redis usage counter', 'ProductivityAnalytics', :views - - it_behaves_like 'a redis usage counter with totals', :productivity_analytics, views: 3 -end diff --git a/spec/lib/omni_auth/strategies/bitbucket_spec.rb b/spec/lib/omni_auth/strategies/bitbucket_spec.rb index d85ce71d60a..86bfe99d5c1 100644 --- a/spec/lib/omni_auth/strategies/bitbucket_spec.rb +++ b/spec/lib/omni_auth/strategies/bitbucket_spec.rb @@ -1,28 +1,83 @@ # frozen_string_literal: true require 'spec_helper' +require 'securerandom' RSpec.describe OmniAuth::Strategies::Bitbucket do - subject { described_class.new({}) } + subject(:strategy) { described_class.new({}) } + + let(:raw_info) do + { + display_name: 'display_name', + username: 'username', + uuid: "{#{SecureRandom.uuid}}", + links: { + avatar: { + href: 'avatar-href' + } + } + }.deep_stringify_keys + end + + let(:primary_email) { 'primaryemail@example.com' } + + let(:emails) do + [ + { + email: primary_email, + is_primary: true, + is_confirmed: true + }.stringify_keys, + { + email: 'secondaryemail@example.com', + is_primary: false, + is_confirmed: true + }.stringify_keys + ] + end + + before do + allow(strategy).to receive(:raw_info).and_return(raw_info) + allow(strategy).to receive(:emails).and_return(emails) + end + + describe 'uid' do + it 'returns Bitbucket user uuid' do + expect(strategy.uid).to eq(raw_info['uuid']) + end + end + + describe 'info' do + it 'returns Bitbucket user info' do + expect(strategy.info).to eq( + { + name: raw_info['display_name'], + username: raw_info['username'], + avatar: raw_info['links']['avatar']['href'], + email: primary_email + } + ) + end + end describe '#callback_url' do let(:base_url) { 'https://example.com' } context 'when script name is not present' do it 'has the correct default callback path' do - allow(subject).to receive(:full_host) { base_url } - allow(subject).to receive(:script_name).and_return('') - allow(subject).to receive(:query_string).and_return('') - expect(subject.callback_url).to eq("#{base_url}/users/auth/bitbucket/callback") + allow(strategy).to receive(:full_host) { base_url } + allow(strategy).to receive(:script_name).and_return('') + allow(strategy).to receive(:query_string).and_return('') + expect(strategy.callback_url).to eq("#{base_url}/users/auth/bitbucket/callback") end end context 'when script name is present' do it 'sets the callback path with script_name' do - allow(subject).to receive(:full_host) { base_url } - allow(subject).to receive(:script_name).and_return('/v1') - allow(subject).to receive(:query_string).and_return('') - expect(subject.callback_url).to eq("#{base_url}/v1/users/auth/bitbucket/callback") + allow(strategy).to receive(:full_host) { base_url } + allow(strategy).to receive(:script_name).and_return('/v1') + allow(strategy).to receive(:query_string).and_return('') + expect(strategy.callback_url).to eq("#{base_url}/v1/users/auth/bitbucket/callback") end end end diff --git a/spec/migrations/20240419035360_queue_backfill_workspace_variables_project_id_spec.rb b/spec/migrations/20240419035360_queue_backfill_workspace_variables_project_id_spec.rb new file mode 100644 index 00000000000..2c2a6a8884a --- /dev/null +++ b/spec/migrations/20240419035360_queue_backfill_workspace_variables_project_id_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe QueueBackfillWorkspaceVariablesProjectId, feature_category: :remote_development do + let!(:batched_migration) { described_class::MIGRATION } + + it 'schedules a new batched migration' do + reversible_migration do |migration| + migration.before -> { + expect(batched_migration).not_to have_scheduled_batched_migration + } + + migration.after -> { + expect(batched_migration).to have_scheduled_batched_migration( + table_name: :workspace_variables, + column_name: :id, + interval: described_class::DELAY_INTERVAL, + batch_size: described_class::BATCH_SIZE, + sub_batch_size: described_class::SUB_BATCH_SIZE, + gitlab_schema: :gitlab_main_cell, + job_arguments: [ + :project_id, + :workspaces, + :project_id, + :workspace_id + ] + ) + } + end + end +end diff --git a/spec/migrations/20240419140530_set_trusted_extern_uid_to_false_for_existing_bitbucket_identities_spec.rb b/spec/migrations/20240419140530_set_trusted_extern_uid_to_false_for_existing_bitbucket_identities_spec.rb new file mode 100644 index 00000000000..ba098d6e9b9 --- /dev/null +++ b/spec/migrations/20240419140530_set_trusted_extern_uid_to_false_for_existing_bitbucket_identities_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe SetTrustedExternUidToFalseForExistingBitbucketIdentities, feature_category: :system_access do + let!(:google_oauth2_identity) do + table(:identities).create!(id: 1, provider: 'google_oauth2') + end + + let!(:github_identity) do + table(:identities).create!(id: 2, provider: 'github') + end + + let!(:salesforce_identity) do + table(:identities).create!(id: 3, provider: 'salesforce') + end + + let!(:bitbucket_identity1) do + table(:identities).create!(id: 4, provider: 'bitbucket') + end + + let!(:group_saml_identity) do + table(:identities).create!(id: 5, provider: 'group_saml') + end + + let!(:bitbucket_identity2) do + table(:identities).create!(id: 6, provider: 'bitbucket') + end + + let!(:bitbucket_identity3) do + table(:identities).create!(id: 7, provider: 'bitbucket') + end + + describe '#up' do + it 'sets trusted_extern_uid to false for existing bitbucket identities', :aggregate_failures do + expect(google_oauth2_identity.reload.trusted_extern_uid).to eq(true) + expect(github_identity.reload.trusted_extern_uid).to eq(true) + expect(salesforce_identity.reload.trusted_extern_uid).to eq(true) + expect(bitbucket_identity1.reload.trusted_extern_uid).to eq(true) + expect(group_saml_identity.reload.trusted_extern_uid).to eq(true) + expect(bitbucket_identity2.reload.trusted_extern_uid).to eq(true) + expect(bitbucket_identity3.reload.trusted_extern_uid).to eq(true) + + migrate! + + expect(google_oauth2_identity.reload.trusted_extern_uid).to eq(true) + expect(github_identity.reload.trusted_extern_uid).to eq(true) + expect(salesforce_identity.reload.trusted_extern_uid).to eq(true) + expect(bitbucket_identity1.reload.trusted_extern_uid).to eq(false) + expect(group_saml_identity.reload.trusted_extern_uid).to eq(true) + expect(bitbucket_identity2.reload.trusted_extern_uid).to eq(false) + expect(bitbucket_identity3.reload.trusted_extern_uid).to eq(false) + end + end +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index ff956d99014..4d9dfa72c5a 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -6425,22 +6425,6 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end end - describe '#has_changes_requested?' do - let_it_be(:merge_request) { create(:merge_request, reviewers: [create(:user)]) } - - context 'reviews have requested changed' do - before_all do - merge_request.merge_request_reviewers.first.update!(state: :requested_changes) - end - - it { expect(merge_request.has_changes_requested?).to be_truthy } - end - - context 'reviews have not requested changed' do - it { expect(merge_request.has_changes_requested?).to be_falsey } - end - end - describe '#batch_update_reviewer_state' do let_it_be(:merge_request) { create(:merge_request, reviewers: create_list(:user, 2)) } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index f035c133d74..b85663e2ef2 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -4177,4 +4177,28 @@ RSpec.describe Repository, feature_category: :source_code_management do it { expect { file_attributes }.to raise_error(ArgumentError) } end end + + describe '#commit_files' do + let(:project) { create(:project, :empty_repo) } + + it 'calls UserCommitFiles RPC' do + expect_next_instance_of(Gitlab::GitalyClient::OperationService) do |client| + expect(client).to receive(:user_commit_files).with( + user, 'extra-branch', 'commit message', [], + 'author email', 'author name', nil, nil, true, nil, false + ) + end + + repository.commit_files( + user, + branch_name: 'extra-branch', + message: 'commit message', + author_name: 'author name', + author_email: 'author email', + actions: [], + force: true, + sign: false + ) + end + end end diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 7304437bc42..28ddea7cdd9 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'spec_helper' -require 'raven/transports/dummy' require_relative '../../../config/initializers/sentry' RSpec.describe API::Helpers, :enable_admin_mode, feature_category: :system_access do diff --git a/spec/requests/api/merge_request_approvals_spec.rb b/spec/requests/api/merge_request_approvals_spec.rb index 08cb5f677ac..c003736b936 100644 --- a/spec/requests/api/merge_request_approvals_spec.rb +++ b/spec/requests/api/merge_request_approvals_spec.rb @@ -100,7 +100,7 @@ RSpec.describe API::MergeRequestApprovals, feature_category: :source_code_manage MergeRequests::UpdateReviewerStateService, project: project, current_user: unapprover ) do |service| - expect(service).to receive(:execute).with(merge_request, :unapproved) + expect(service).to receive(:execute).with(merge_request, 'unapproved') end post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unapprove", unapprover) diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index 28cb295d3ed..56473dbc7d7 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_caching, feature_category: :system_access do include RackAttackSpecHelpers + include WorkhorseHelpers include SessionHelpers let(:settings) { Gitlab::CurrentSettings.current_application_settings } @@ -27,6 +28,8 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac throttle_unauthenticated_packages_api_period_in_seconds: 1, throttle_authenticated_packages_api_requests_per_period: 100, throttle_authenticated_packages_api_period_in_seconds: 1, + throttle_unauthenticated_git_http_requests_per_period: 100, + throttle_unauthenticated_git_http_period_in_seconds: 1, throttle_authenticated_git_lfs_requests_per_period: 100, throttle_authenticated_git_lfs_period_in_seconds: 1, throttle_unauthenticated_files_api_requests_per_period: 100, @@ -697,6 +700,45 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac end end + describe 'unauthenticated git http requests' do + let_it_be(:project) { create(:project, :repository, :public) } + + let(:git_http_clone_path) { "/#{project.full_path}.git/info/refs?service=git-upload-pack" } + + it_behaves_like 'rate-limited unauthenticated requests' do + let(:throttle_name) { 'throttle_unauthenticated_git_http' } + let(:throttle_setting_prefix) { 'throttle_unauthenticated_git_http' } + let(:url_that_does_not_require_authentication) { "/#{project.full_path}.git/info/refs?service=git-upload-pack" } + let(:url_that_is_not_matched) { '/users/sign_in' } + let(:headers) { WorkhorseHelpers.workhorse_internal_api_request_header } + end + + context 'when authenticated' do + let_it_be(:user) { create(:user) } + let_it_be(:token) { create(:personal_access_token, user: user) } + + let(:headers) { WorkhorseHelpers.workhorse_internal_api_request_header.merge(basic_auth_headers(user, token)) } + + def do_request + get git_http_clone_path, headers: headers + end + + before do + settings_to_set[:throttle_unauthenticated_git_http_requests_per_period] = requests_per_period + settings_to_set[:throttle_unauthenticated_git_http_period_in_seconds] = period_in_seconds + settings_to_set[:throttle_unauthenticated_git_http_enabled] = true + stub_application_setting(settings_to_set) + end + + it 'rejects requests over the rate limit' do + (requests_per_period + 1).times do + do_request + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end + describe 'authenticated git lfs requests', :api do let_it_be(:project) { create(:project, :internal) } let_it_be(:user) { create(:user) } diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index 7683ec4ca40..bdef2944876 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -2774,7 +2774,7 @@ RSpec.describe QuickActions::InterpretService, feature_category: :team_planning MergeRequests::UpdateReviewerStateService, project: project, current_user: current_user ) do |service| - expect(service).to receive(:execute).with(merge_request, :unapproved) + expect(service).to receive(:execute).with(merge_request, 'unapproved') end service.execute(content, merge_request) diff --git a/spec/support/helpers/stub_action_cable_connection.rb b/spec/support/helpers/stub_action_cable_connection.rb index b4e9c2ae48c..ede8cc1c981 100644 --- a/spec/support/helpers/stub_action_cable_connection.rb +++ b/spec/support/helpers/stub_action_cable_connection.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module StubActionCableConnection - def stub_action_cable_connection(current_user: nil, request: ActionDispatch::TestRequest.create) + def stub_action_cable_connection(current_user: nil, access_token: nil, request: ActionDispatch::TestRequest.create) + request.headers['Authorization'] = "Bearer #{access_token.token}" if access_token stub_connection(current_user: current_user, request: request) end end diff --git a/spec/support/helpers/stub_configuration.rb b/spec/support/helpers/stub_configuration.rb index ea334fa8fac..bc769ce0796 100644 --- a/spec/support/helpers/stub_configuration.rb +++ b/spec/support/helpers/stub_configuration.rb @@ -109,15 +109,12 @@ module StubConfiguration end def stub_sentry_settings(enabled: true) - allow(Gitlab.config.sentry).to receive(:enabled) { enabled } allow(Gitlab::CurrentSettings).to receive(:sentry_enabled?) { enabled } dsn = 'dummy://b44a0828b72421a6d8e99efd68d44fa8@example.com/42' - allow(Gitlab.config.sentry).to receive(:dsn) { dsn } allow(Gitlab::CurrentSettings).to receive(:sentry_dsn) { dsn } clientside_dsn = 'dummy://b44a0828b72421a6d8e99efd68d44fa8@example.com/43' - allow(Gitlab.config.sentry).to receive(:clientside_dsn) { clientside_dsn } allow(Gitlab::CurrentSettings) .to receive(:sentry_clientside_dsn) { clientside_dsn } end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index b0c30aea4ec..a3ec99d4870 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -6625,7 +6625,6 @@ - './spec/lib/gitlab/usage_data_counters/merge_request_widget_extension_counter_spec.rb' - './spec/lib/gitlab/usage_data_counters/note_counter_spec.rb' - './spec/lib/gitlab/usage_data_counters/package_event_counter_spec.rb' -- './spec/lib/gitlab/usage_data_counters/productivity_analytics_counter_spec.rb' - './spec/lib/gitlab/usage_data_counters/quick_action_activity_unique_counter_spec.rb' - './spec/lib/gitlab/usage_data_counters/redis_counter_spec.rb' - './spec/lib/gitlab/usage_data_counters/search_counter_spec.rb' diff --git a/spec/support/shared_examples/lint_factories_shared_examples.rb b/spec/support/shared_examples/lint_factories_shared_examples.rb index 8714f43169c..31266379d4b 100644 --- a/spec/support/shared_examples/lint_factories_shared_examples.rb +++ b/spec/support/shared_examples/lint_factories_shared_examples.rb @@ -76,6 +76,7 @@ module Support [:ci_job_artifact, :gzip], [:ci_job_artifact, :correct_checksum], [:dependency_proxy_blob, :remote_store], + [:discussion_note_on_personal_snippet, Any], [:environment, :non_playable], [:issue_customer_relations_contact, :for_contact], [:issue_customer_relations_contact, :for_issue], diff --git a/spec/support/shared_examples/models/email_format_shared_examples.rb b/spec/support/shared_examples/models/email_format_shared_examples.rb index 77ded168637..2a06ce441cd 100644 --- a/spec/support/shared_examples/models/email_format_shared_examples.rb +++ b/spec/support/shared_examples/models/email_format_shared_examples.rb @@ -14,7 +14,6 @@ RSpec.shared_examples 'an object with email-formatted attributes' do |*attribute info+test@example.com o'reilly@example.com mailto:test@example.com - lol!'+=?><#$%^&*()@gmail.com ].each do |valid_email| context "with a value of '#{valid_email}'" do let(:email_value) { valid_email } @@ -30,6 +29,21 @@ RSpec.shared_examples 'an object with email-formatted attributes' do |*attribute %w[ foobar test@test@example.com + lol!'+=?><#$%^&*()@gmail.com + test?invalidcharacter@example.com + test!invalidcharacter@example.com + test#invalidcharacter@example.com + test$invalidcharacter@example.com + test%invalidcharacter@example.com + test&invalidcharacter@example.com + test*invalidcharacter@example.com + test/invalidcharacter@example.com + test=invalidcharacter@example.com + test^invalidcharacter@example.com + testinvalidcharacter@example.com + =?iso-8859-1?q?testencodedformat=40new.example.com=3e=20?=testencodedformat@example.com + =?iso-8859-1?q?testencodedformat=40new.example.com?=testencodedformat@example.com ].each do |invalid_email| context "with a value of '#{invalid_email}'" do let(:email_value) { invalid_email } diff --git a/spec/support/shared_examples/requests/rack_attack_shared_examples.rb b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb index 89ae165f3fa..d15c88244f8 100644 --- a/spec/support/shared_examples/requests/rack_attack_shared_examples.rb +++ b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb @@ -459,7 +459,10 @@ end # * requests_per_period # * period_in_seconds # * period +# * headers RSpec.shared_examples 'rate-limited unauthenticated requests' do + let(:headers) { {} } + before do # Set low limits settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period @@ -467,6 +470,10 @@ RSpec.shared_examples 'rate-limited unauthenticated requests' do travel_back end + def do_request + get url_that_does_not_require_authentication, headers: headers + end + context 'when the throttle is enabled' do before do settings_to_set[:"#{throttle_setting_prefix}_enabled"] = true @@ -476,12 +483,12 @@ RSpec.shared_examples 'rate-limited unauthenticated requests' do it 'rejects requests over the rate limit' do # At first, allow requests under the rate limit. requests_per_period.times do - get url_that_does_not_require_authentication + do_request expect(response).to have_gitlab_http_status(:ok) end # the last straw - expect_rejection { get url_that_does_not_require_authentication } + expect_rejection { do_request } end context 'with custom response text' do @@ -492,37 +499,37 @@ RSpec.shared_examples 'rate-limited unauthenticated requests' do it 'rejects requests over the rate limit' do # At first, allow requests under the rate limit. requests_per_period.times do - get url_that_does_not_require_authentication + do_request expect(response).to have_gitlab_http_status(:ok) end # the last straw - expect_rejection { get url_that_does_not_require_authentication } + expect_rejection { do_request } expect(response.body).to eq("Custom response\n") end end it 'allows requests after throttling and then waiting for the next period' do requests_per_period.times do - get url_that_does_not_require_authentication + do_request expect(response).to have_gitlab_http_status(:ok) end - expect_rejection { get url_that_does_not_require_authentication } + expect_rejection { do_request } travel_to(period.from_now) do requests_per_period.times do - get url_that_does_not_require_authentication + do_request expect(response).to have_gitlab_http_status(:ok) end - expect_rejection { get url_that_does_not_require_authentication } + expect_rejection { do_request } end end it 'counts requests from different IPs separately' do requests_per_period.times do - get url_that_does_not_require_authentication + do_request expect(response).to have_gitlab_http_status(:ok) end @@ -531,7 +538,7 @@ RSpec.shared_examples 'rate-limited unauthenticated requests' do end # would be over limit for the same IP - get url_that_does_not_require_authentication + do_request expect(response).to have_gitlab_http_status(:ok) end @@ -603,7 +610,7 @@ RSpec.shared_examples 'rate-limited unauthenticated requests' do it 'logs RackAttack info into structured logs' do requests_per_period.times do - get url_that_does_not_require_authentication + do_request expect(response).to have_gitlab_http_status(:ok) end @@ -619,12 +626,12 @@ RSpec.shared_examples 'rate-limited unauthenticated requests' do expect(Gitlab::AuthLogger).to receive(:error).with(arguments) - get url_that_does_not_require_authentication + do_request end it_behaves_like 'tracking when dry-run mode is set' do def do_request - get url_that_does_not_require_authentication + get url_that_does_not_require_authentication, headers: headers end end end @@ -637,7 +644,7 @@ RSpec.shared_examples 'rate-limited unauthenticated requests' do it 'allows requests over the rate limit' do (1 + requests_per_period).times do - get url_that_does_not_require_authentication + do_request expect(response).to have_gitlab_http_status(:ok) end end diff --git a/spec/validators/devise_email_validator_spec.rb b/spec/validators/devise_email_validator_spec.rb index 64d11d4d963..d25ab1a55a9 100644 --- a/spec/validators/devise_email_validator_spec.rb +++ b/spec/validators/devise_email_validator_spec.rb @@ -5,14 +5,14 @@ require 'spec_helper' RSpec.describe DeviseEmailValidator do let!(:user) { build(:user, public_email: 'test@example.com') } - subject { validator.validate(user) } + subject(:validate) { validator.validate(user) } describe 'validations' do context 'by default' do let(:validator) { described_class.new(attributes: [:public_email]) } it 'allows when email is valid' do - subject + validate expect(user.errors).to be_empty end @@ -20,7 +20,7 @@ RSpec.describe DeviseEmailValidator do it 'returns error when email is invalid' do user.public_email = 'invalid' - subject + validate expect(user.errors).to be_present expect(user.errors.added?(:public_email)).to be true @@ -29,7 +29,7 @@ RSpec.describe DeviseEmailValidator do it 'returns error when email is nil' do user.public_email = nil - subject + validate expect(user.errors).to be_present end @@ -37,11 +37,40 @@ RSpec.describe DeviseEmailValidator do it 'returns error when email is blank' do user.public_email = '' - subject + validate expect(user.errors).to be_present expect(user.errors.added?(:public_email)).to be true end + + context 'for email with invalid characters' do + %w[ + lol!'+=?><#$%^&*()@gmail.com + test?invalidcharacter@example.com + test!invalidcharacter@example.com + test#invalidcharacter@example.com + test$invalidcharacter@example.com + test%invalidcharacter@example.com + test&invalidcharacter@example.com + test*invalidcharacter@example.com + test/invalidcharacter@example.com + test=invalidcharacter@example.com + test^invalidcharacter@example.com + test invalidcharacter@example.com + =?iso-8859-1?q?testencodedformat=40new.example.com=3e=20?=testencodedformat@example.com + =?iso-8859-1?q?testencodedformat=40new.example.com?=testencodedformat@example.com + ].each do |invalid_email| + it "returns error as invalid email for '#{invalid_email}'" do + user.public_email = invalid_email + + validate + + expect(user.errors).to be_present + expect(user.errors.added?(:public_email)).to be true + end + end + end end end @@ -51,13 +80,13 @@ RSpec.describe DeviseEmailValidator do it 'allows when value match' do user.public_email = '1' - subject + validate expect(user.errors).to be_empty end it 'returns error when value does not match' do - subject + validate expect(user.errors).to be_present end @@ -75,7 +104,7 @@ RSpec.describe DeviseEmailValidator do it 'allows when email is nil' do user.public_email = nil - subject + validate expect(user.errors).to be_empty end @@ -87,9 +116,22 @@ RSpec.describe DeviseEmailValidator do it 'allows when email is blank' do user.public_email = '' - subject + validate expect(user.errors).to be_empty end end + + context 'when attribute is already marked invalid' do + let(:validator) { described_class.new(attributes: [:email]) } + + it 'does not add duplicate error' do + user.email = 'Invalid as per Devise::Models::Validatable' + user.validate + + validate + + expect(user.errors[:email].size).to eq 1 + end + end end diff --git a/spec/views/admin/application_settings/network.html.haml_spec.rb b/spec/views/admin/application_settings/network.html.haml_spec.rb index 193ee8a32d5..defaa97f82a 100644 --- a/spec/views/admin/application_settings/network.html.haml_spec.rb +++ b/spec/views/admin/application_settings/network.html.haml_spec.rb @@ -11,6 +11,16 @@ RSpec.describe 'admin/application_settings/network.html.haml', feature_category: allow(view).to receive(:current_user) { admin } end + context 'for Git HTTP rate limit' do + it 'renders the `git_http_rate_limit_unauthenticated` field' do + render + + expect(rendered).to have_field('application_setting_throttle_unauthenticated_git_http_enabled') + expect(rendered).to have_field('application_setting_throttle_unauthenticated_git_http_requests_per_period') + expect(rendered).to have_field('application_setting_throttle_unauthenticated_git_http_period_in_seconds') + end + end + context 'for Projects API rate limit' do it 'renders the `projects_api_rate_limit_unauthenticated` field' do render diff --git a/yarn.lock b/yarn.lock index 2b3ceef232d..da3c1e227e5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2075,16 +2075,18 @@ "@sentry/types" "7.102.0" "@sentry/utils" "7.102.0" -"@sentry/core@5.30.0": - version "5.30.0" - resolved "https://registry.yarnpkg.com/@sentry/core/-/core-5.30.0.tgz#6b203664f69e75106ee8b5a2fe1d717379b331f3" - integrity sha512-TmfrII8w1PQZSZgPpUESqjB+jC6MvZJZdLtE/0hZ+SrnKhW3x5WlYLvTXZpcWePYBku7rl2wn1RZu6uT0qCTeg== +"@sentry/browser@7.102.0": + version "7.102.0" + resolved "https://registry.yarnpkg.com/@sentry/browser/-/browser-7.102.0.tgz#335f51d01aabf8c4d2abc871855f9c2d19f8f70d" + integrity sha512-hIggcMnojIbWhbmlRfkykHmy6n7pjug0AHfF19HRUQxAx9KJfMH5YdWvohov0Hb9fS+jdvqgE+/4AWbEeXQrHw== dependencies: - "@sentry/hub" "5.30.0" - "@sentry/minimal" "5.30.0" - "@sentry/types" "5.30.0" - "@sentry/utils" "5.30.0" - tslib "^1.9.3" + "@sentry-internal/feedback" "7.102.0" + "@sentry-internal/replay-canvas" "7.102.0" + "@sentry-internal/tracing" "7.102.0" + "@sentry/core" "7.102.0" + "@sentry/replay" "7.102.0" + "@sentry/types" "7.102.0" + "@sentry/utils" "7.102.0" "@sentry/core@7.102.0": version "7.102.0" @@ -2094,24 +2096,6 @@ "@sentry/types" "7.102.0" "@sentry/utils" "7.102.0" -"@sentry/hub@5.30.0": - version "5.30.0" - resolved "https://registry.yarnpkg.com/@sentry/hub/-/hub-5.30.0.tgz#2453be9b9cb903404366e198bd30c7ca74cdc100" - integrity sha512-2tYrGnzb1gKz2EkMDQcfLrDTvmGcQPuWxLnJKXJvYTQDGLlEvi2tWz1VIHjunmOvJrB5aIQLhm+dcMRwFZDCqQ== - dependencies: - "@sentry/types" "5.30.0" - "@sentry/utils" "5.30.0" - tslib "^1.9.3" - -"@sentry/minimal@5.30.0": - version "5.30.0" - resolved "https://registry.yarnpkg.com/@sentry/minimal/-/minimal-5.30.0.tgz#ce3d3a6a273428e0084adcb800bc12e72d34637b" - integrity sha512-BwWb/owZKtkDX+Sc4zCSTNcvZUq7YcH3uAVlmh/gtR9rmUvbzAA3ewLuB3myi4wWRAMEtny6+J/FN/x+2wn9Xw== - dependencies: - "@sentry/hub" "5.30.0" - "@sentry/types" "5.30.0" - tslib "^1.9.3" - "@sentry/replay@7.102.0": version "7.102.0" resolved "https://registry.yarnpkg.com/@sentry/replay/-/replay-7.102.0.tgz#209b7adb68e89772824218ecab498d3a6fbc2c42" @@ -2122,24 +2106,11 @@ "@sentry/types" "7.102.0" "@sentry/utils" "7.102.0" -"@sentry/types@5.30.0": - version "5.30.0" - resolved "https://registry.yarnpkg.com/@sentry/types/-/types-5.30.0.tgz#19709bbe12a1a0115bc790b8942917da5636f402" - integrity sha512-R8xOqlSTZ+htqrfteCWU5Nk0CDN5ApUTvrlvBuiH1DyP6czDZ4ktbZB0hAgBlVcK0U+qpD3ag3Tqqpa5Q67rPw== - "@sentry/types@7.102.0": version "7.102.0" resolved "https://registry.yarnpkg.com/@sentry/types/-/types-7.102.0.tgz#b31e9faa54036053ab82c09c3c855035a4889c59" integrity sha512-FPfFBP0x3LkPARw1/6cWySLq1djIo8ao3Qo2KNBeE9CHdq8bsS1a8zzjJLuWG4Ww+wieLP8/lY3WTgrCz4jowg== -"@sentry/utils@5.30.0": - version "5.30.0" - resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-5.30.0.tgz#9a5bd7ccff85ccfe7856d493bffa64cabc41e980" - integrity sha512-zaYmoH0NWWtvnJjC9/CBseXMtKHm/tm40sz3YfJRxeQjyzRqNQPgivpd9R/oDJCYj999mzdW382p/qi2ypjLww== - dependencies: - "@sentry/types" "5.30.0" - tslib "^1.9.3" - "@sentry/utils@7.102.0": version "7.102.0" resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-7.102.0.tgz#66325f2567986cc3fd12fbdb980fb8ada170342b" @@ -12560,29 +12531,6 @@ send@0.17.2: range-parser "~1.2.1" statuses "~1.5.0" -"sentrybrowser5@npm:@sentry/browser@5.30.0": - version "5.30.0" - resolved "https://registry.yarnpkg.com/@sentry/browser/-/browser-5.30.0.tgz#c28f49d551db3172080caef9f18791a7fd39e3b3" - integrity sha512-rOb58ZNVJWh1VuMuBG1mL9r54nZqKeaIlwSlvzJfc89vyfd7n6tQ1UXMN383QBz/MS5H5z44Hy5eE+7pCrYAfw== - dependencies: - "@sentry/core" "5.30.0" - "@sentry/types" "5.30.0" - "@sentry/utils" "5.30.0" - tslib "^1.9.3" - -"sentrybrowser@npm:@sentry/browser@7.102.0": - version "7.102.0" - resolved "https://registry.yarnpkg.com/@sentry/browser/-/browser-7.102.0.tgz#335f51d01aabf8c4d2abc871855f9c2d19f8f70d" - integrity sha512-hIggcMnojIbWhbmlRfkykHmy6n7pjug0AHfF19HRUQxAx9KJfMH5YdWvohov0Hb9fS+jdvqgE+/4AWbEeXQrHw== - dependencies: - "@sentry-internal/feedback" "7.102.0" - "@sentry-internal/replay-canvas" "7.102.0" - "@sentry-internal/tracing" "7.102.0" - "@sentry/core" "7.102.0" - "@sentry/replay" "7.102.0" - "@sentry/types" "7.102.0" - "@sentry/utils" "7.102.0" - serialize-javascript@^2.1.2: version "2.1.2" resolved "https://registry.yarnpkg.com/serialize-javascript/-/serialize-javascript-2.1.2.tgz#ecec53b0e0317bdc95ef76ab7074b7384785fa61" @@ -13768,7 +13716,7 @@ tslib@2.3.0, tslib@~2.3.0: resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.3.0.tgz#803b8cdab3e12ba581a4ca41c8839bbb0dacb09e" integrity sha512-N82ooyxVNm6h1riLCoyS9e3fuJ3AMG2zIZs2Gd1ATcSFjSA23Q0fzjjZeh0jbJvWVDZ0cJT8yaNNaaXHzueNjg== -tslib@^1.8.1, tslib@^1.9.0, tslib@^1.9.3: +tslib@^1.8.1, tslib@^1.9.0: version "1.14.1" resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.14.1.tgz#cf2d38bdc34a134bcaf1091c41f6619e2f672d00" integrity sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg==