diff options
56 files changed, 631 insertions, 132 deletions
diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 5e183177a63..c9c83937434 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -569,11 +569,13 @@ - <<: *if-merge-request-labels-run-review-app - <<: *if-auto-deploy-branches - changes: *code-qa-patterns + - changes: *workhorse-patterns .frontend:rules:compile-test-assets: rules: - - changes: *code-backstage-qa-patterns - <<: *if-merge-request-labels-run-all-rspec + - changes: *code-backstage-qa-patterns + - changes: *workhorse-patterns .frontend:rules:compile-test-assets-as-if-foss: rules: @@ -583,6 +585,7 @@ - <<: *if-merge-request-labels-run-all-rspec - changes: *code-backstage-qa-patterns - changes: *startup-css-patterns + - changes: *workhorse-patterns .frontend:rules:compile-test-assets-as-if-jh: rules: @@ -604,6 +607,7 @@ allow_failure: true - changes: *startup-css-patterns allow_failure: true + - changes: *workhorse-patterns .frontend:rules:default-frontend-jobs: rules: @@ -1335,8 +1339,9 @@ .rails:rules:detect-tests: rules: - - changes: *code-backstage-qa-patterns - <<: *if-merge-request-labels-run-all-rspec + - changes: *code-backstage-qa-patterns + - changes: *workhorse-patterns .rails:rules:detect-previous-failed-tests: rules: @@ -1803,6 +1808,8 @@ allow_failure: true - changes: *startup-css-patterns allow_failure: true + - changes: *workhorse-patterns + allow_failure: true ####################### # Test metadata rules # @@ -1810,7 +1817,7 @@ .test-metadata:rules:retrieve-tests-metadata: rules: - changes: *code-backstage-patterns - when: on_success + - changes: *workhorse-patterns - <<: *if-merge-request-labels-run-all-rspec .test-metadata:rules:update-tests-metadata: @@ -1827,8 +1834,7 @@ ################### .workhorse:rules:workhorse: rules: - - <<: *if-default-refs - changes: *workhorse-patterns + - changes: *workhorse-patterns ################### # yaml-lint rules # diff --git a/CHANGELOG.md b/CHANGELOG.md index 19948bb0118..1c2b44662f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,27 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 14.7.2 (2022-02-08) + +### Added (1 change) + +- [Allow self-hosted instances to render same-origin Iframe](gitlab-org/gitlab@eb7c78363cdfc670286967872d8458fc5f6d82e8) ([merge request](gitlab-org/gitlab!79966)) + +### Fixed (4 changes) + +- [Geo: Fix reverify object stored files](gitlab-org/gitlab@603700dcca3b8f25a3b80b44b11a73df549c0cb3) ([merge request](gitlab-org/gitlab!79966)) **GitLab Enterprise Edition** +- [Geo: Fix verification failures of remote stored files](gitlab-org/gitlab@2eb8ac7e88dcd40f0e8266966655962e4d6e3171) ([merge request](gitlab-org/gitlab!79966)) **GitLab Enterprise Edition** +- [GitLab Version - CE Admin Dashboard [RUN ALL RSPEC] [RUN AS-IF-FOSS]](gitlab-org/gitlab@f2253ce2d729fa202a26b54f3ca870b932ea1855) ([merge request](gitlab-org/gitlab!79966)) +- [Fix cluster integration HTTP adapter](gitlab-org/gitlab@c05027ef4d7ec35fc16e8e16dc6e5af201f665c3) ([merge request](gitlab-org/gitlab!79966)) + +### Changed (1 change) + +- [Update to ruby-magic v0.5.4](gitlab-org/gitlab@ced6ef1001730dc2851f58f7db3229d1c585b9d3) ([merge request](gitlab-org/gitlab!79966)) + +### Removed (1 change) + +- [Disable sandboxed_mermaid feature flag by default](gitlab-org/gitlab@70c40d43169bd48d360ed7a6a03c33c05d5e3738) ([merge request](gitlab-org/gitlab!79966)) + ## 14.7.1 (2022-02-03) ### Security diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index efbed93a512..ea67bb9e62b 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -269b04ae5248eea05fe2d6ca02e33fdd3f6cee76 +32b9777c9f3f217324d95d6e25b6ed1ddee13f68 diff --git a/app/assets/javascripts/repository/components/blob_viewers/index.js b/app/assets/javascripts/repository/components/blob_viewers/index.js index daa11802cc5..e942f59e7d8 100644 --- a/app/assets/javascripts/repository/components/blob_viewers/index.js +++ b/app/assets/javascripts/repository/components/blob_viewers/index.js @@ -3,7 +3,7 @@ const viewers = { image: () => import('./image_viewer.vue'), video: () => import('./video_viewer.vue'), empty: () => import('./empty_viewer.vue'), - text: () => import('~/vue_shared/components/source_viewer.vue'), + text: () => import('~/vue_shared/components/source_viewer/source_viewer.vue'), pdf: () => import('./pdf_viewer.vue'), lfs: () => import('./lfs_viewer.vue'), }; diff --git a/app/assets/javascripts/repository/queries/blob_info.query.graphql b/app/assets/javascripts/repository/queries/blob_info.query.graphql index aeba84d481d..91b5ff9ccf1 100644 --- a/app/assets/javascripts/repository/queries/blob_info.query.graphql +++ b/app/assets/javascripts/repository/queries/blob_info.query.graphql @@ -20,6 +20,7 @@ query getBlobInfo($projectPath: ID!, $filePath: String!, $ref: String!) { rawSize rawTextBlob fileType + language path editBlobPath ideEditPath diff --git a/app/assets/javascripts/vue_shared/components/source_viewer/constants.js b/app/assets/javascripts/vue_shared/components/source_viewer/constants.js new file mode 100644 index 00000000000..9efe0147c37 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/source_viewer/constants.js @@ -0,0 +1,111 @@ +// Language map from Rouge::Lexer to highlight.js +// Rouge::Lexer - We use it on the BE to determine the language of a source file (https://github.com/rouge-ruby/rouge/blob/master/docs/Languages.md). +// Highlight.js - We use it on the FE to highlight the syntax of a source file (https://github.com/highlightjs/highlight.js/tree/main/src/languages). +export const ROUGE_TO_HLJS_LANGUAGE_MAP = { + bsl: '1c', + actionscript: 'actionscript', + ada: 'ada', + apache: 'apache', + applescript: 'applescript', + armasm: 'armasm', + awk: 'awk', + c: 'c', + ceylon: 'ceylon', + clean: 'clean', + clojure: 'clojure', + cmake: 'cmake', + coffeescript: 'coffeescript', + coq: 'coq', + cpp: 'cpp', + crystal: 'crystal', + csharp: 'csharp', + css: 'css', + d: 'd', + dart: 'dart', + pascal: 'delphi', + diff: 'diff', + jinja: 'django', + docker: 'dockerfile', + batchfile: 'dos', + elixir: 'elixir', + elm: 'elm', + erb: 'erb', + erlang: 'erlang', + fortran: 'fortran', + fsharp: 'fsharp', + gherkin: 'gherkin', + glsl: 'glsl', + go: 'go', + gradle: 'gradle', + groovy: 'groovy', + haml: 'haml', + handlebars: 'handlebars', + haskell: 'haskell', + haxe: 'haxe', + http: 'http', + hylang: 'hy', + ini: 'ini', + isbl: 'isbl', + java: 'java', + javascript: 'javascript', + json: 'json', + julia: 'julia', + kotlin: 'kotlin', + lasso: 'lasso', + tex: 'latex', + common_lisp: 'lisp', + livescript: 'livescript', + llvm: 'llvm', + hlsl: 'lsl', + lua: 'lua', + make: 'makefile', + markdown: 'markdown', + mathematica: 'mathematica', + matlab: 'matlab', + moonscript: 'moonscript', + nginx: 'nginx', + nim: 'nim', + nix: 'nix', + objective_c: 'objectivec', + ocaml: 'ocaml', + perl: 'perl', + php: 'php', + plaintext: 'plaintext', + pony: 'pony', + powershell: 'powershell', + prolog: 'prolog', + properties: 'properties', + protobuf: 'protobuf', + puppet: 'puppet', + python: 'python', + q: 'q', + qml: 'qml', + r: 'r', + reasonml: 'reasonml', + ruby: 'ruby', + rust: 'rust', + sas: 'sas', + scala: 'scala', + scheme: 'scheme', + scss: 'scss', + shell: 'shell', + smalltalk: 'smalltalk', + sml: 'sml', + sqf: 'sqf', + sql: 'sql', + stan: 'stan', + stata: 'stata', + swift: 'swift', + tap: 'tap', + tcl: 'tcl', + twig: 'twig', + typescript: 'typescript', + vala: 'vala', + vb: 'vbnet', + verilog: 'verilog', + vhdl: 'vhdl', + viml: 'vim', + xml: 'xml', + xquery: 'xquery', + yaml: 'yaml', +}; diff --git a/app/assets/javascripts/vue_shared/components/source_viewer.vue b/app/assets/javascripts/vue_shared/components/source_viewer/source_viewer.vue index f6580a716ee..5aae1812de3 100644 --- a/app/assets/javascripts/vue_shared/components/source_viewer.vue +++ b/app/assets/javascripts/vue_shared/components/source_viewer/source_viewer.vue @@ -1,14 +1,16 @@ <script> -import { GlSafeHtmlDirective } from '@gitlab/ui'; +import { GlSafeHtmlDirective, GlLoadingIcon } from '@gitlab/ui'; import LineNumbers from '~/vue_shared/components/line_numbers.vue'; import { sanitize } from '~/lib/dompurify'; +import { ROUGE_TO_HLJS_LANGUAGE_MAP } from './constants'; +import { wrapLines } from './utils'; const LINE_SELECT_CLASS_NAME = 'hll'; -const PLAIN_TEXT_LANGUAGE = 'plaintext'; export default { components: { LineNumbers, + GlLoadingIcon, }, directives: { SafeHtml: GlSafeHtmlDirective, @@ -18,17 +20,12 @@ export default { type: Object, required: true, }, - autoDetect: { - type: Boolean, - required: false, - default: true, // We'll eventually disable autoDetect and pass the language explicitly to reduce the footprint (https://gitlab.com/gitlab-org/gitlab/-/issues/348145) - }, }, data() { return { languageDefinition: null, content: this.blob.rawTextBlob, - language: this.blob.language || PLAIN_TEXT_LANGUAGE, + language: ROUGE_TO_HLJS_LANGUAGE_MAP[this.blob.language], hljs: null, }; }, @@ -40,14 +37,14 @@ export default { let highlightedContent; if (this.hljs) { - if (this.autoDetect) { + if (!this.language) { highlightedContent = this.hljs.highlightAuto(this.content).value; } else if (this.languageDefinition) { highlightedContent = this.hljs.highlight(this.content, { language: this.language }).value; } } - return this.wrapLines(highlightedContent); + return wrapLines(highlightedContent); }, }, watch: { @@ -61,14 +58,14 @@ export default { async mounted() { this.hljs = await this.loadHighlightJS(); - if (!this.autoDetect) { + if (this.language) { this.languageDefinition = await this.loadLanguage(); } }, methods: { loadHighlightJS() { - // With auto-detect enabled we load all common languages else we load only the core (smallest footprint) - return this.autoDetect ? import('highlight.js/lib/common') : import('highlight.js/lib/core'); + // If no language can be mapped to highlight.js we load all common languages else we load only the core (smallest footprint) + return !this.language ? import('highlight.js/lib/common') : import('highlight.js/lib/core'); }, async loadLanguage() { let languageDefinition; @@ -82,15 +79,6 @@ export default { return languageDefinition; }, - wrapLines(content) { - return ( - content && - content - .split('\n') - .map((line, i) => `<span id="LC${i + 1}" class="line">${line}</span>`) - .join('\r\n') - ); - }, selectLine() { const hash = sanitize(this.$route.hash); const lineToSelect = hash && this.$el.querySelector(hash); @@ -113,7 +101,9 @@ export default { }; </script> <template> + <gl-loading-icon v-if="!highlightedContent" size="sm" class="gl-my-5" /> <div + v-else class="file-content code js-syntax-highlight blob-content gl-display-flex" :class="$options.userColorScheme" data-type="simple" diff --git a/app/assets/javascripts/vue_shared/components/source_viewer/utils.js b/app/assets/javascripts/vue_shared/components/source_viewer/utils.js new file mode 100644 index 00000000000..e64e564bf61 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/source_viewer/utils.js @@ -0,0 +1,26 @@ +export const wrapLines = (content) => { + return ( + content && + content + .split('\n') + .map((line, i) => { + let formattedLine; + const idAttribute = `id="LC${i + 1}"`; + + if (line.includes('<span class="hljs') && !line.includes('</span>')) { + /** + * In some cases highlight.js will wrap multiple lines in a span, in these cases we want to append the line number to the existing span + * + * example (before): <span class="hljs-code">```bash + * example (after): <span id="LC67" class="hljs-code">```bash + */ + formattedLine = line.replace(/(?=class="hljs)/, `${idAttribute} `); + } else { + formattedLine = `<span ${idAttribute} class="line">${line}</span>`; + } + + return formattedLine; + }) + .join('\n') + ); +}; diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 9cf435af726..2f3db25f53b 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -720,7 +720,7 @@ $calendar-activity-colors: ( #7fa8c9, #527ba0, #254e77, -); +) !default; /* * Commit Page diff --git a/app/assets/stylesheets/themes/_dark.scss b/app/assets/stylesheets/themes/_dark.scss index 1265d27d5f6..b8c9e913beb 100644 --- a/app/assets/stylesheets/themes/_dark.scss +++ b/app/assets/stylesheets/themes/_dark.scss @@ -259,3 +259,11 @@ $line-removed-dark: $red-200; $well-expand-item: $gray-200; $well-inner-border: $gray-200; + +$calendar-activity-colors: ( + #303030, + #333861, + #4a5593, + #6172c5, + #788ff7 +); diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index f90cc85ba67..9ba1b7ecd7b 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -423,7 +423,9 @@ module ApplicationSettingsHelper :sidekiq_job_limiter_compression_threshold_bytes, :sidekiq_job_limiter_limit_bytes, :suggest_pipeline_enabled, - :user_email_lookup_limit + :user_email_lookup_limit, + :users_get_by_id_limit, + :users_get_by_id_limit_allowlist_raw ].tap do |settings| settings << :deactivate_dormant_users unless Gitlab.com? end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 3c9f7c4dd7f..02fbf0f855e 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -563,6 +563,12 @@ class ApplicationSetting < ApplicationRecord presence: true, length: { maximum: 255 }, if: :sentry_enabled? + validates :users_get_by_id_limit, + numericality: { only_integer: true, greater_than_or_equal_to: 0 } + validates :users_get_by_id_limit_allowlist, + length: { maximum: 100, message: N_('is too long (maximum is 100 entries)') }, + allow_nil: false + attr_encrypted :asset_proxy_secret_key, mode: :per_attribute_iv, key: Settings.attr_encrypted_db_key_base_truncated, diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index 4bdeceb7c25..415f0b35f3a 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -231,7 +231,9 @@ module ApplicationSettingImplementation rate_limiting_response_text: nil, whats_new_variant: 0, user_deactivation_emails_enabled: true, - user_email_lookup_limit: 60 + user_email_lookup_limit: 60, + users_get_by_id_limit: 300, + users_get_by_id_limit_allowlist: [] } end @@ -334,6 +336,14 @@ module ApplicationSettingImplementation self.notes_create_limit_allowlist = strings_to_array(values).map(&:downcase) end + def users_get_by_id_limit_allowlist_raw + array_to_string(self.users_get_by_id_limit_allowlist) + end + + def users_get_by_id_limit_allowlist_raw=(values) + self.users_get_by_id_limit_allowlist = strings_to_array(values).map(&:downcase) + end + def asset_proxy_whitelist=(values) values = strings_to_array(values) if values.is_a?(String) diff --git a/app/models/instance_configuration.rb b/app/models/instance_configuration.rb index 77e0653d3dc..2016024b2f4 100644 --- a/app/models/instance_configuration.rb +++ b/app/models/instance_configuration.rb @@ -118,7 +118,12 @@ class InstanceConfiguration group_export_download: application_setting_limit_per_minute(:group_download_export_limit), group_import: application_setting_limit_per_minute(:group_import_limit), raw_blob: application_setting_limit_per_minute(:raw_blob_request_limit), - user_email_lookup: application_setting_limit_per_minute(:user_email_lookup_limit) + user_email_lookup: application_setting_limit_per_minute(:user_email_lookup_limit), + users_get_by_id: { + enabled: application_settings[:users_get_by_id_limit] > 0, + requests_per_period: application_settings[:users_get_by_id_limit], + period_in_seconds: 10.minutes + } } end diff --git a/app/presenters/projects/import_export/project_export_presenter.rb b/app/presenters/projects/import_export/project_export_presenter.rb index 7b2ffb6d755..53c547cde9e 100644 --- a/app/presenters/projects/import_export/project_export_presenter.rb +++ b/app/presenters/projects/import_export/project_export_presenter.rb @@ -3,11 +3,14 @@ module Projects module ImportExport class ProjectExportPresenter < Gitlab::View::Presenter::Delegated + # NOTE: This is needed because this presenter is serialized to JSON, + # and we need to make sure that `#as_json` is called in this class so + # it will use the overriden attributes below. Otherwise the call is + # delegated to the model and will use the original methods. include ActiveModel::Serializers::JSON presents ::Project, as: :project - # TODO: Remove `ActiveModel::Serializers::JSON` inclusion as it's duplicate delegator_override_with ActiveModel::Serializers::JSON delegator_override_with ActiveModel::Naming delegator_override :include_root_in_json, :include_root_in_json? diff --git a/app/views/admin/application_settings/_note_limits.html.haml b/app/views/admin/application_settings/_note_limits.html.haml index eb6122f244a..38a5d6a1010 100644 --- a/app/views/admin/application_settings/_note_limits.html.haml +++ b/app/views/admin/application_settings/_note_limits.html.haml @@ -9,7 +9,7 @@ = f.label :notes_create_limit_allowlist, _('Users to exclude from the rate limit'), class: 'label-bold' = f.text_area :notes_create_limit_allowlist_raw, placeholder: 'username1, username2', class: 'form-control gl-form-input', rows: 5 .form-text.text-muted - = _('Comma-separated list of users allowed to exceed the rate limit.') + = _('List of users allowed to exceed the rate limit.') = f.submit _('Save changes'), class: "gl-button btn btn-confirm", data: { qa_selector: 'save_changes_button' } diff --git a/app/views/admin/application_settings/_users_api_limits.html.haml b/app/views/admin/application_settings/_users_api_limits.html.haml new file mode 100644 index 00000000000..e9b657f8942 --- /dev/null +++ b/app/views/admin/application_settings/_users_api_limits.html.haml @@ -0,0 +1,14 @@ += form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-users-api-limits-settings'), html: { class: 'fieldset-form' } do |f| + = form_errors(@application_setting) + + %fieldset + .form-group + = f.label :users_get_by_id_limit, _('Maximum requests per 10 minutes per user'), class: 'label-bold' + = f.number_field :users_get_by_id_limit, class: 'form-control gl-form-input' + .form-group + = f.label :users_get_by_id_limit_allowlist_raw, _('Users to exclude from the rate limit'), class: 'label-bold' + = f.text_area :users_get_by_id_limit_allowlist_raw, placeholder: 'username1, username2', class: 'form-control gl-form-input', rows: 5 + .form-text.text-muted + = _('List of users allowed to exceed the rate limit.') + + = f.submit _('Save changes'), class: "gl-button btn btn-confirm", data: { qa_selector: 'save_changes_button' } diff --git a/app/views/admin/application_settings/network.html.haml b/app/views/admin/application_settings/network.html.haml index 242d0c364f4..90183b028f0 100644 --- a/app/views/admin/application_settings/network.html.haml +++ b/app/views/admin/application_settings/network.html.haml @@ -122,6 +122,18 @@ .settings-content = render 'note_limits' +%section.settings.as-users-api-limits.no-animate#js-users-api-limits-settings{ class: ('expanded' if expanded_by_default?) } + .settings-header + %h4 + = _('Users API rate limit') + %button.btn.gl-button.btn-default.js-settings-toggle{ type: 'button' } + = expanded_by_default? ? _('Collapse') : _('Expand') + %p + = _('Set the per-user rate limit for getting a user by ID via the API.') + = link_to _('Learn more.'), help_page_path('user/admin_area/settings/rate_limit_on_users_api.md'), target: '_blank', rel: 'noopener noreferrer' + .settings-content + = render 'users_api_limits' + %section.settings.as-import-export-limits.no-animate#js-import-export-limits-settings{ class: ('expanded' if expanded_by_default?) } .settings-header %h4 diff --git a/app/views/shared/members/_access_request_links.html.haml b/app/views/shared/members/_access_request_links.html.haml index 8600db25e65..7af946377be 100644 --- a/app/views/shared/members/_access_request_links.html.haml +++ b/app/views/shared/members/_access_request_links.html.haml @@ -4,7 +4,8 @@ - link_text = source.is_a?(Group) ? _('Leave group') : _('Leave project') = link_to link_text, polymorphic_path([:leave, source, :members]), method: :delete, - data: { confirm: leave_confirmation_message(source), qa_selector: 'leave_group_link' }, + aria: { label: link_text }, + data: { confirm: leave_confirmation_message(source), confirm_btn_variant: 'danger', qa_selector: 'leave_group_link' }, class: 'js-leave-link' - elsif requester = source.requesters.find_by(user_id: current_user.id) # rubocop: disable CodeReuse/ActiveRecord = link_to _('Withdraw Access Request'), polymorphic_path([:leave, source, :members]), diff --git a/app/workers/merge_requests/update_head_pipeline_worker.rb b/app/workers/merge_requests/update_head_pipeline_worker.rb index c8dc9d1f7c8..acebf5fc767 100644 --- a/app/workers/merge_requests/update_head_pipeline_worker.rb +++ b/app/workers/merge_requests/update_head_pipeline_worker.rb @@ -2,7 +2,6 @@ module MergeRequests class UpdateHeadPipelineWorker - include ApplicationWorker include Gitlab::EventStore::Subscriber feature_category :code_review diff --git a/config/feature_flags/development/enforce_security_report_validation.yml b/config/feature_flags/development/enforce_security_report_validation.yml new file mode 100644 index 00000000000..ada5863b4d7 --- /dev/null +++ b/config/feature_flags/development/enforce_security_report_validation.yml @@ -0,0 +1,8 @@ +--- +name: enforce_security_report_validation +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79798 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351000 +milestone: '14.8' +type: development +group: group::threat insights +default_enabled: false diff --git a/db/migrate/20220207083129_add_users_get_by_id_limit_to_application_setting.rb b/db/migrate/20220207083129_add_users_get_by_id_limit_to_application_setting.rb new file mode 100644 index 00000000000..cdf2aced213 --- /dev/null +++ b/db/migrate/20220207083129_add_users_get_by_id_limit_to_application_setting.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddUsersGetByIdLimitToApplicationSetting < Gitlab::Database::Migration[1.0] + enable_lock_retries! + + def up + add_column :application_settings, :users_get_by_id_limit, :integer, null: false, default: 300 + add_column :application_settings, :users_get_by_id_limit_allowlist, :text, array: true, limit: 255, null: false, default: [] + end + + def down + remove_column :application_settings, :users_get_by_id_limit + remove_column :application_settings, :users_get_by_id_limit_allowlist + end +end diff --git a/db/schema_migrations/20220207083129 b/db/schema_migrations/20220207083129 new file mode 100644 index 00000000000..25dca4578f4 --- /dev/null +++ b/db/schema_migrations/20220207083129 @@ -0,0 +1 @@ +01cc0139097235991fa2caf8b780ccd1c3ce580647197418424ade83ce9be77e
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 9c95424bae7..06f86a90041 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10620,6 +10620,8 @@ CREATE TABLE application_settings ( project_runner_token_expiration_interval integer, ecdsa_sk_key_restriction integer DEFAULT 0 NOT NULL, ed25519_sk_key_restriction integer DEFAULT 0 NOT NULL, + users_get_by_id_limit integer DEFAULT 300 NOT NULL, + users_get_by_id_limit_allowlist text[] DEFAULT '{}'::text[] NOT NULL, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), diff --git a/doc/development/event_store.md b/doc/development/event_store.md index 0eb59724d3f..c6d553f41cd 100644 --- a/doc/development/event_store.md +++ b/doc/development/event_store.md @@ -232,7 +232,6 @@ the event safely via the `handle_event` method. For example: ```ruby module MergeRequests class UpdateHeadPipelineWorker - include ApplicationWorker include Gitlab::EventStore::Subscriber def handle_event(event) diff --git a/doc/development/fe_guide/development_process.md b/doc/development/fe_guide/development_process.md index cd82a7dadc3..9921b851344 100644 --- a/doc/development/fe_guide/development_process.md +++ b/doc/development/fe_guide/development_process.md @@ -64,6 +64,30 @@ Please use your best judgment when to use it and please contribute new points th - [ ] Follow up on issues that came out of the review. Create issues for discovered edge cases that should be covered in future iterations. ``` +### Code deletion checklist + +When your merge request deletes code, it's important to also delete all +related code that is no longer used. +When deleting Haml and Vue code, check whether it contains the following types of +code that is unused: + +- CSS. + + For example, we've deleted a Vue component that contained the `.mr-card` class, which is now unused. + The `.mr-card` CSS rule set should then be deleted from `merge_requests.scss`. + +- Ruby variables. + + Deleting unused Ruby variables is important so we don't continue instantiating them with + potentially expensive code. + + For example, we've deleted a Haml template that used the `@total_count` Ruby variable. + The `@total_count` variable was no longer used in the remaining templates for the page. + The instantiation of `@total_count` in `issues_controller.rb` should then be deleted so that we + don't make unnecessary database calls to calculate the count of issues. + +- Ruby methods. + ### Merge Request Review With the purpose of being [respectful of others' time](https://about.gitlab.com/handbook/values/#be-respectful-of-others-time) please follow these guidelines when asking for a review: diff --git a/doc/user/admin_area/settings/index.md b/doc/user/admin_area/settings/index.md index 67546ff0f54..a581fd4aebc 100644 --- a/doc/user/admin_area/settings/index.md +++ b/doc/user/admin_area/settings/index.md @@ -137,6 +137,7 @@ The **Network** settings contain: - [Incident Management Limits](../../../operations/incident_management/index.md) - Limit the number of inbound alerts that can be sent to a project. - [Notes creation limit](rate_limit_on_notes_creation.md) - Set a rate limit on the note creation requests. +- [Get single user limit](rate_limit_on_users_api.md) - Set a rate limit on users API endpoint to get a user by ID. ### Preferences diff --git a/doc/user/admin_area/settings/rate_limit_on_users_api.md b/doc/user/admin_area/settings/rate_limit_on_users_api.md new file mode 100644 index 00000000000..d9f473cb722 --- /dev/null +++ b/doc/user/admin_area/settings/rate_limit_on_users_api.md @@ -0,0 +1,33 @@ +--- +type: reference +stage: Manage +group: Authentication & Authorization +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Rate limits on Users API **(FREE SELF)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78364) in GitLab 14.8. + +You can configure the per-user rate limit for requests to the users API endpoint to get a user by ID. + +To change getting a single user rate limit: + +1. On the top bar, select **Menu > Admin**. +1. On the left sidebar, select **Settings > Network**. +1. Expand **Users API rate limit**. +1. In the **Maximum requests per 10 minutes** text box, enter the new value. +1. Optional. In the **Users to exclude from the rate limit** box, list users allowed to exceed the limit. +1. Select **Save changes**. + +This limit is: + +- Applied independently per user. +- Not applied per IP address. + +The default value is `300`. + +Requests over the rate limit are logged into the `auth.log` file. + +For example, if you set a limit of 300, requests to the `GET /users/:id` API endpoint +exceeding a rate of 300 per 10 minutes are blocked. Access to the endpoint is allowed after ten minutes have elapsed. diff --git a/doc/user/workspace/img/1.1-Instance_overview.png b/doc/user/workspace/img/1.1-Instance_overview.png Binary files differdeleted file mode 100644 index a3c642cda3f..00000000000 --- a/doc/user/workspace/img/1.1-Instance_overview.png +++ /dev/null diff --git a/doc/user/workspace/img/1.2-Groups_overview.png b/doc/user/workspace/img/1.2-Groups_overview.png Binary files differdeleted file mode 100644 index 7771e5f4c3c..00000000000 --- a/doc/user/workspace/img/1.2-Groups_overview.png +++ /dev/null diff --git a/doc/user/workspace/img/1.3-Admin.png b/doc/user/workspace/img/1.3-Admin.png Binary files differdeleted file mode 100644 index 5f531e4ef5e..00000000000 --- a/doc/user/workspace/img/1.3-Admin.png +++ /dev/null diff --git a/doc/user/workspace/img/Admin_Settings.png b/doc/user/workspace/img/Admin_Settings.png Binary files differdeleted file mode 100644 index b0d13f43ccb..00000000000 --- a/doc/user/workspace/img/Admin_Settings.png +++ /dev/null diff --git a/doc/user/workspace/img/hardware_settings.png b/doc/user/workspace/img/hardware_settings.png Binary files differdeleted file mode 100644 index 919ff46f8c8..00000000000 --- a/doc/user/workspace/img/hardware_settings.png +++ /dev/null diff --git a/doc/user/workspace/index.md b/doc/user/workspace/index.md index cf35f082880..0befcbe25d2 100644 --- a/doc/user/workspace/index.md +++ b/doc/user/workspace/index.md @@ -14,6 +14,9 @@ As with all projects, the items mentioned on this page are subject to change or The development, release, and timing of any products, features, or functionality remain at the sole discretion of GitLab Inc. +NOTE: +Workspace is currently in development. + Workspace will be above the [top-level namespaces](../group/index.md#namespaces) for you to manage everything you do as a GitLab administrator, including: @@ -27,33 +30,12 @@ Our goal is to reach feature parity between SaaS and self-managed installations, - Hardware Controls. For functionality that does not apply to groups, Hardware Controls are only applicable to self-managed installations. There is one Hardware Controls section per installation. -NOTE: -Workspace is currently in development. - -## Demo: New hierarchy concept for groups and projects for epics - -The following demo introduces the new hierarchy concept for groups and projects for epics. - -<div class="video-fallback"> - See the video: <a href="https://www.youtube.com/embed/fE74lsG_8yM">Consolidating groups and projects update (2021-08-23)</a>. -</div> -<figure class="video-container"> - <iframe src="https://www.youtube.com/embed/fE74lsG_8yM" frameborder="0" allowfullscreen="true"> </iframe> -</figure> - -## Concept previews - -The following provide a preview to the Workspace concept. - - - - - - - - +To learn about the current state of workspace development, +see [epic 4257](https://gitlab.com/groups/gitlab-org/-/epics/4257). - +<i class="fa fa-youtube-play youtube" aria-hidden="true"></i> +For a video introduction to the new hierarchy concept for groups and projects for epics, see +[Consolidating groups and projects update (August 2021)](https://www.youtube.com/watch?v=fE74lsG_8yM). ## Related topics diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 508ccdb4b33..90868037938 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -177,6 +177,7 @@ module API optional :floc_enabled, type: Grape::API::Boolean, desc: 'Enable FloC (Federated Learning of Cohorts)' optional :user_deactivation_emails_enabled, type: Boolean, desc: 'Send emails to users upon account deactivation' optional :suggest_pipeline_enabled, type: Boolean, desc: 'Enable pipeline suggestion banner' + optional :users_get_by_id_limit, type: Integer, desc: "Maximum number of calls to the /users/:id API per 10 minutes per user. Set to 0 for unlimited requests." ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type| optional :"#{type}_key_restriction", diff --git a/lib/api/users.rb b/lib/api/users.rb index eeb5244466a..d540978931e 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -143,7 +143,12 @@ module API forbidden!('Not authorized!') unless current_user if Feature.enabled?(:rate_limit_user_by_id_endpoint, type: :development) - check_rate_limit! :users_get_by_id, scope: current_user unless current_user.admin? + unless current_user.admin? + check_rate_limit!(:users_get_by_id, + scope: current_user, + users_allowlist: Gitlab::CurrentSettings.current_application_settings.users_get_by_id_limit_allowlist + ) + end end user = User.find_by(id: params[:id]) diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb index eba1980f832..d2a31938e89 100644 --- a/lib/gitlab/application_rate_limiter.rb +++ b/lib/gitlab/application_rate_limiter.rb @@ -14,7 +14,7 @@ module Gitlab # Threshold value can be either an Integer or a Proc # in order to not evaluate it's value every time this method is called # and only do that when it's needed. - def rate_limits + def rate_limits # rubocop:disable Metrics/AbcSize { issues_create: { threshold: -> { application_settings.issues_create_limit }, interval: 1.minute }, notes_create: { threshold: -> { application_settings.notes_create_limit }, interval: 1.minute }, @@ -32,7 +32,7 @@ module Gitlab group_testing_hook: { threshold: 5, interval: 1.minute }, profile_add_new_email: { threshold: 5, interval: 1.minute }, web_hook_calls: { interval: 1.minute }, - users_get_by_id: { threshold: 10, interval: 1.minute }, + users_get_by_id: { threshold: -> { application_settings.users_get_by_id_limit }, interval: 10.minutes }, username_exists: { threshold: 20, interval: 1.minute }, user_sign_up: { threshold: 20, interval: 1.minute }, profile_resend_email_confirmation: { threshold: 5, interval: 1.minute }, diff --git a/lib/gitlab/ci/parsers/security/common.rb b/lib/gitlab/ci/parsers/security/common.rb index 0c969daf7fd..9aec615d012 100644 --- a/lib/gitlab/ci/parsers/security/common.rb +++ b/lib/gitlab/ci/parsers/security/common.rb @@ -42,11 +42,22 @@ module Gitlab attr_reader :json_data, :report, :validate def valid? - return true if !validate || schema_validator.valid? + if Feature.enabled?(:enforce_security_report_validation) + if !validate || schema_validator.valid? + report.schema_validation_status = :valid_schema + true + else + report.schema_validation_status = :invalid_schema + schema_validator.errors.each { |error| report.add_error('Schema', error) } + false + end + else + return true if !validate || schema_validator.valid? - schema_validator.errors.each { |error| report.add_error('Schema', error) } + schema_validator.errors.each { |error| report.add_error('Schema', error) } - false + false + end end def schema_validator diff --git a/lib/gitlab/ci/parsers/security/validators/schemas/dependency-scanning-report-format.json b/lib/gitlab/ci/parsers/security/validators/schemas/dependency-scanning-report-format.json new file mode 120000 index 00000000000..11e0a6846fb --- /dev/null +++ b/lib/gitlab/ci/parsers/security/validators/schemas/dependency-scanning-report-format.json @@ -0,0 +1 @@ +14.0.0/dependency-scanning-report-format.json
\ No newline at end of file diff --git a/lib/gitlab/ci/reports/security/report.rb b/lib/gitlab/ci/reports/security/report.rb index 3e4a44a2e70..fbf8c81ac36 100644 --- a/lib/gitlab/ci/reports/security/report.rb +++ b/lib/gitlab/ci/reports/security/report.rb @@ -6,7 +6,7 @@ module Gitlab module Security class Report attr_reader :created_at, :type, :pipeline, :findings, :scanners, :identifiers - attr_accessor :scan, :scanned_resources, :errors, :analyzer, :version + attr_accessor :scan, :scanned_resources, :errors, :analyzer, :version, :schema_validation_status delegate :project_id, to: :pipeline diff --git a/lib/gitlab/event_store/subscriber.rb b/lib/gitlab/event_store/subscriber.rb index cf326d1f9e4..9f569059736 100644 --- a/lib/gitlab/event_store/subscriber.rb +++ b/lib/gitlab/event_store/subscriber.rb @@ -7,7 +7,6 @@ # # @example: # class SomeEventSubscriber -# include ApplicationWorker # include Gitlab::EventStore::Subscriber # # def handle_event(event) @@ -18,6 +17,14 @@ module Gitlab module EventStore module Subscriber + extend ActiveSupport::Concern + + included do + include ApplicationWorker + + loggable_arguments 0, 1 + end + def perform(event_type, data) raise InvalidEvent, event_type unless self.class.const_defined?(event_type) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4b64f227b9f..8ac9ee868f0 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8751,9 +8751,6 @@ msgstr "" msgid "Comma-separated list of email addresses." msgstr "" -msgid "Comma-separated list of users allowed to exceed the rate limit." -msgstr "" - msgid "Comma-separated, e.g. '1.1.1.1, 2.2.2.0/24'" msgstr "" @@ -21734,6 +21731,9 @@ msgstr "" msgid "List of all merge commits" msgstr "" +msgid "List of users allowed to exceed the rate limit." +msgstr "" + msgid "List options" msgstr "" @@ -22361,6 +22361,9 @@ msgstr "" msgid "Maximum push size (MB)" msgstr "" +msgid "Maximum requests per 10 minutes per user" +msgstr "" + msgid "Maximum requests per minute" msgstr "" @@ -33140,6 +33143,9 @@ msgstr "" msgid "Set the milestone to %{milestone_reference}." msgstr "" +msgid "Set the per-user rate limit for getting a user by ID via the API." +msgstr "" + msgid "Set the per-user rate limit for notes created by web or API requests." msgstr "" @@ -39585,6 +39591,9 @@ msgstr "" msgid "Users" msgstr "" +msgid "Users API rate limit" +msgstr "" + msgid "Users can launch a development environment from a GitLab browser tab when the %{linkStart}Gitpod%{linkEnd} integration is enabled." msgstr "" @@ -39910,6 +39919,9 @@ msgstr "" msgid "View group labels" msgstr "" +msgid "View group pipeline usage quota" +msgstr "" + msgid "View incident details at" msgstr "" diff --git a/qa/qa/page/component/blob_content.rb b/qa/qa/page/component/blob_content.rb index 16a258ff0bc..ce743b24dda 100644 --- a/qa/qa/page/component/blob_content.rb +++ b/qa/qa/page/component/blob_content.rb @@ -22,7 +22,7 @@ module QA element :copy_contents_button end - base.view 'app/assets/javascripts/vue_shared/components/source_viewer.vue' do + base.view 'app/assets/javascripts/vue_shared/components/source_viewer/source_viewer.vue' do element :blob_viewer_file_content end diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index aa396d10074..8dad031899e 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -631,6 +631,20 @@ RSpec.describe 'Admin updates settings' do expect(current_settings.issues_create_limit).to eq(0) end + it 'changes Users API rate limits settings' do + visit network_admin_application_settings_path + + page.within('.as-users-api-limits') do + fill_in 'Maximum requests per 10 minutes per user', with: 0 + fill_in 'Users to exclude from the rate limit', with: 'someone, someone_else' + click_button 'Save changes' + end + + expect(page).to have_content "Application settings saved successfully" + expect(current_settings.users_get_by_id_limit).to eq(0) + expect(current_settings.users_get_by_id_limit_allowlist).to eq(%w[someone someone_else]) + end + shared_examples 'regular throttle rate limit settings' do it 'changes rate limit settings' do visit network_admin_application_settings_path diff --git a/spec/frontend/repository/components/blob_content_viewer_spec.js b/spec/frontend/repository/components/blob_content_viewer_spec.js index cbccdcece5d..8699ee3df26 100644 --- a/spec/frontend/repository/components/blob_content_viewer_spec.js +++ b/spec/frontend/repository/components/blob_content_viewer_spec.js @@ -15,7 +15,7 @@ import ForkSuggestion from '~/repository/components/fork_suggestion.vue'; import { loadViewer } from '~/repository/components/blob_viewers'; import DownloadViewer from '~/repository/components/blob_viewers/download_viewer.vue'; import EmptyViewer from '~/repository/components/blob_viewers/empty_viewer.vue'; -import SourceViewer from '~/vue_shared/components/source_viewer.vue'; +import SourceViewer from '~/vue_shared/components/source_viewer/source_viewer.vue'; import blobInfoQuery from '~/repository/queries/blob_info.query.graphql'; import { redirectTo } from '~/lib/utils/url_utility'; import { isLoggedIn } from '~/lib/utils/common_utils'; diff --git a/spec/frontend/repository/mock_data.js b/spec/frontend/repository/mock_data.js index 2df89965fbe..52c68c7925c 100644 --- a/spec/frontend/repository/mock_data.js +++ b/spec/frontend/repository/mock_data.js @@ -5,6 +5,7 @@ export const simpleViewerMock = { rawSize: 123, rawTextBlob: 'raw content', fileType: 'text', + language: 'javascript', path: 'some_file.js', webPath: 'some_file.js', editBlobPath: 'some_file.js/edit', diff --git a/spec/frontend/vue_shared/components/source_viewer_spec.js b/spec/frontend/vue_shared/components/source_viewer/source_viewer_spec.js index a0d07ddc245..2010bac7060 100644 --- a/spec/frontend/vue_shared/components/source_viewer_spec.js +++ b/spec/frontend/vue_shared/components/source_viewer/source_viewer_spec.js @@ -1,8 +1,10 @@ import hljs from 'highlight.js/lib/core'; +import { GlLoadingIcon } from '@gitlab/ui'; import Vue, { nextTick } from 'vue'; import VueRouter from 'vue-router'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; -import SourceViewer from '~/vue_shared/components/source_viewer.vue'; +import SourceViewer from '~/vue_shared/components/source_viewer/source_viewer.vue'; +import { ROUGE_TO_HLJS_LANGUAGE_MAP } from '~/vue_shared/components/source_viewer/constants'; import LineNumbers from '~/vue_shared/components/line_numbers.vue'; import waitForPromises from 'helpers/wait_for_promises'; @@ -12,43 +14,50 @@ const router = new VueRouter(); describe('Source Viewer component', () => { let wrapper; - const language = 'javascript'; + const language = 'docker'; + const mappedLanguage = ROUGE_TO_HLJS_LANGUAGE_MAP[language]; const content = `// Some source code`; const DEFAULT_BLOB_DATA = { language, rawTextBlob: content }; const highlightedContent = `<span data-testid='test-highlighted' id='LC1'>${content}</span><span id='LC2'></span>`; - hljs.highlight.mockImplementation(() => ({ value: highlightedContent })); - hljs.highlightAuto.mockImplementation(() => ({ value: highlightedContent })); - - const createComponent = async (props = { autoDetect: false }) => { + const createComponent = async (blob = {}) => { wrapper = shallowMountExtended(SourceViewer, { router, - propsData: { blob: { ...DEFAULT_BLOB_DATA }, ...props }, + propsData: { blob: { ...DEFAULT_BLOB_DATA, ...blob } }, }); await waitForPromises(); }; + const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); const findLineNumbers = () => wrapper.findComponent(LineNumbers); const findHighlightedContent = () => wrapper.findByTestId('test-highlighted'); const findFirstLine = () => wrapper.find('#LC1'); - beforeEach(() => createComponent()); + beforeEach(() => { + hljs.highlight.mockImplementation(() => ({ value: highlightedContent })); + hljs.highlightAuto.mockImplementation(() => ({ value: highlightedContent })); + + return createComponent(); + }); afterEach(() => wrapper.destroy()); describe('highlight.js', () => { it('registers the language definition', async () => { - const languageDefinition = await import(`highlight.js/lib/languages/${language}`); + const languageDefinition = await import(`highlight.js/lib/languages/${mappedLanguage}`); - expect(hljs.registerLanguage).toHaveBeenCalledWith(language, languageDefinition.default); + expect(hljs.registerLanguage).toHaveBeenCalledWith( + mappedLanguage, + languageDefinition.default, + ); }); it('highlights the content', () => { - expect(hljs.highlight).toHaveBeenCalledWith(content, { language }); + expect(hljs.highlight).toHaveBeenCalledWith(content, { language: mappedLanguage }); }); - describe('auto-detect enabled', () => { - beforeEach(() => createComponent({ autoDetect: true })); + describe('auto-detects if a language cannot be loaded', () => { + beforeEach(() => createComponent({ language: 'some_unknown_language' })); it('highlights the content with auto-detection', () => { expect(hljs.highlightAuto).toHaveBeenCalledWith(content); @@ -57,6 +66,13 @@ describe('Source Viewer component', () => { }); describe('rendering', () => { + it('renders a loading icon if no highlighted content is available yet', async () => { + hljs.highlight.mockImplementation(() => ({ value: null })); + await createComponent(); + + expect(findLoadingIcon().exists()).toBe(true); + }); + it('renders Line Numbers', () => { expect(findLineNumbers().props('lines')).toBe(1); }); diff --git a/spec/frontend/vue_shared/components/source_viewer/utils_spec.js b/spec/frontend/vue_shared/components/source_viewer/utils_spec.js new file mode 100644 index 00000000000..937c3b26c67 --- /dev/null +++ b/spec/frontend/vue_shared/components/source_viewer/utils_spec.js @@ -0,0 +1,13 @@ +import { wrapLines } from '~/vue_shared/components/source_viewer/utils'; + +describe('Wrap lines', () => { + it.each` + input | output + ${'line 1'} | ${'<span id="LC1" class="line">line 1</span>'} + ${'line 1\nline 2'} | ${`<span id="LC1" class="line">line 1</span>\n<span id="LC2" class="line">line 2</span>`} + ${'<span class="hljs-code">line 1\nline 2</span>'} | ${`<span id="LC1" class="hljs-code">line 1\n<span id="LC2" class="line">line 2</span></span>`} + ${'<span class="hljs-code">```bash'} | ${'<span id="LC1" class="hljs-code">```bash'} + `('returns lines wrapped in spans containing line numbers', ({ input, output }) => { + expect(wrapLines(input)).toBe(output); + }); +}); diff --git a/spec/helpers/application_settings_helper_spec.rb b/spec/helpers/application_settings_helper_spec.rb index e722f301522..169b1c75995 100644 --- a/spec/helpers/application_settings_helper_spec.rb +++ b/spec/helpers/application_settings_helper_spec.rb @@ -46,6 +46,15 @@ RSpec.describe ApplicationSettingsHelper do expect(helper.visible_attributes).to include(:deactivate_dormant_users) end + it 'contains rate limit parameters' do + expect(helper.visible_attributes).to include(*%i( + issues_create_limit notes_create_limit project_export_limit + project_download_export_limit project_export_limit project_import_limit + raw_blob_request_limit group_export_limit group_download_export_limit + group_import_limit users_get_by_id_limit user_email_lookup_limit + )) + end + context 'when GitLab.com' do before do allow(Gitlab).to receive(:com?).and_return(true) diff --git a/spec/lib/gitlab/ci/parsers/security/common_spec.rb b/spec/lib/gitlab/ci/parsers/security/common_spec.rb index c49673f5a4a..7eec78ff186 100644 --- a/spec/lib/gitlab/ci/parsers/security/common_spec.rb +++ b/spec/lib/gitlab/ci/parsers/security/common_spec.rb @@ -40,60 +40,142 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do allow(validator_class).to receive(:new).and_call_original end - context 'when the validate flag is set as `false`' do - let(:validate) { false } + context 'when enforce_security_report_validation is enabled' do + before do + stub_feature_flags(enforce_security_report_validation: true) + end - it 'does not run the validation logic' do - parse_report + context 'when the validate flag is set as `true`' do + let(:validate) { true } - expect(validator_class).not_to have_received(:new) - end - end + it 'instantiates the validator with correct params' do + parse_report - context 'when the validate flag is set as `true`' do - let(:validate) { true } - let(:valid?) { false } + expect(validator_class).to have_received(:new).with(report.type, {}) + end - before do - allow_next_instance_of(validator_class) do |instance| - allow(instance).to receive(:valid?).and_return(valid?) - allow(instance).to receive(:errors).and_return(['foo']) + context 'when the report data is valid according to the schema' do + let(:valid?) { true } + + before do + allow_next_instance_of(validator_class) do |instance| + allow(instance).to receive(:valid?).and_return(valid?) + allow(instance).to receive(:errors).and_return([]) + end + + allow(parser).to receive_messages(create_scanner: true, create_scan: true) + end + + it 'does not add errors to the report' do + expect { parse_report }.not_to change { report.errors }.from([]) + end + + it 'adds the schema validation status to the report' do + parse_report + + expect(report.schema_validation_status).to eq(:valid_schema) + end + + it 'keeps the execution flow as normal' do + parse_report + + expect(parser).to have_received(:create_scanner) + expect(parser).to have_received(:create_scan) + end end - allow(parser).to receive_messages(create_scanner: true, create_scan: true) - end + context 'when the report data is not valid according to the schema' do + let(:valid?) { false } - it 'instantiates the validator with correct params' do - parse_report + before do + allow_next_instance_of(validator_class) do |instance| + allow(instance).to receive(:valid?).and_return(valid?) + allow(instance).to receive(:errors).and_return(['foo']) + end - expect(validator_class).to have_received(:new).with(report.type, {}) - end + allow(parser).to receive_messages(create_scanner: true, create_scan: true) + end + + it 'adds errors to the report' do + expect { parse_report }.to change { report.errors }.from([]).to([{ message: 'foo', type: 'Schema' }]) + end + + it 'adds the schema validation status to the report' do + parse_report - context 'when the report data is not valid according to the schema' do - it 'adds errors to the report' do - expect { parse_report }.to change { report.errors }.from([]).to([{ message: 'foo', type: 'Schema' }]) + expect(report.schema_validation_status).to eq(:invalid_schema) + end + + it 'does not try to create report entities' do + parse_report + + expect(parser).not_to have_received(:create_scanner) + expect(parser).not_to have_received(:create_scan) + end end + end + end + + context 'when enforce_security_report_validation is disabled' do + before do + stub_feature_flags(enforce_security_report_validation: false) + end + + context 'when the validate flag is set as `false`' do + let(:validate) { false } - it 'does not try to create report entities' do + it 'does not run the validation logic' do parse_report - expect(parser).not_to have_received(:create_scanner) - expect(parser).not_to have_received(:create_scan) + expect(validator_class).not_to have_received(:new) end end - context 'when the report data is valid according to the schema' do - let(:valid?) { true } + context 'when the validate flag is set as `true`' do + let(:validate) { true } + let(:valid?) { false } - it 'does not add errors to the report' do - expect { parse_report }.not_to change { report.errors }.from([]) + before do + allow_next_instance_of(validator_class) do |instance| + allow(instance).to receive(:valid?).and_return(valid?) + allow(instance).to receive(:errors).and_return(['foo']) + end + + allow(parser).to receive_messages(create_scanner: true, create_scan: true) end - it 'keeps the execution flow as normal' do + it 'instantiates the validator with correct params' do parse_report - expect(parser).to have_received(:create_scanner) - expect(parser).to have_received(:create_scan) + expect(validator_class).to have_received(:new).with(report.type, {}) + end + + context 'when the report data is not valid according to the schema' do + it 'adds errors to the report' do + expect { parse_report }.to change { report.errors }.from([]).to([{ message: 'foo', type: 'Schema' }]) + end + + it 'does not try to create report entities' do + parse_report + + expect(parser).not_to have_received(:create_scanner) + expect(parser).not_to have_received(:create_scan) + end + end + + context 'when the report data is valid according to the schema' do + let(:valid?) { true } + + it 'does not add errors to the report' do + expect { parse_report }.not_to change { report.errors }.from([]) + end + + it 'keeps the execution flow as normal' do + parse_report + + expect(parser).to have_received(:create_scanner) + expect(parser).to have_received(:create_scan) + end end end end diff --git a/spec/lib/gitlab/event_store/store_spec.rb b/spec/lib/gitlab/event_store/store_spec.rb index 284018582a7..94e8f0ff2ff 100644 --- a/spec/lib/gitlab/event_store/store_spec.rb +++ b/spec/lib/gitlab/event_store/store_spec.rb @@ -10,7 +10,6 @@ RSpec.describe Gitlab::EventStore::Store do let(:worker) do stub_const('EventSubscriber', Class.new).tap do |klass| klass.class_eval do - include ApplicationWorker include Gitlab::EventStore::Subscriber def handle_event(event) @@ -23,7 +22,6 @@ RSpec.describe Gitlab::EventStore::Store do let(:another_worker) do stub_const('AnotherEventSubscriber', Class.new).tap do |klass| klass.class_eval do - include ApplicationWorker include Gitlab::EventStore::Subscriber end end @@ -32,7 +30,6 @@ RSpec.describe Gitlab::EventStore::Store do let(:unrelated_worker) do stub_const('UnrelatedEventSubscriber', Class.new).tap do |klass| klass.class_eval do - include ApplicationWorker include Gitlab::EventStore::Subscriber end end @@ -253,6 +250,10 @@ RSpec.describe Gitlab::EventStore::Store do subject { worker_instance.perform(event_name, data) } + it 'is a Sidekiq worker' do + expect(worker_instance).to be_a(ApplicationWorker) + end + it 'handles the event' do expect(worker_instance).to receive(:handle_event).with(instance_of(event.class)) diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 0ece212d692..a962703d460 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -141,7 +141,7 @@ RSpec.describe ApplicationSetting do it { is_expected.not_to allow_value('default' => 101).for(:repository_storages_weighted).with_message("value for 'default' must be between 0 and 100") } it { is_expected.not_to allow_value('default' => 100, shouldntexist: 50).for(:repository_storages_weighted).with_message("can't include: shouldntexist") } - %i[notes_create_limit user_email_lookup_limit].each do |setting| + %i[notes_create_limit user_email_lookup_limit users_get_by_id_limit].each do |setting| it { is_expected.to allow_value(400).for(setting) } it { is_expected.not_to allow_value('two').for(setting) } it { is_expected.not_to allow_value(nil).for(setting) } @@ -158,6 +158,11 @@ RSpec.describe ApplicationSetting do it { is_expected.not_to allow_value(nil).for(:notes_create_limit_allowlist) } it { is_expected.to allow_value([]).for(:notes_create_limit_allowlist) } + it { is_expected.to allow_value(many_usernames(100)).for(:users_get_by_id_limit_allowlist) } + it { is_expected.not_to allow_value(many_usernames(101)).for(:users_get_by_id_limit_allowlist) } + it { is_expected.not_to allow_value(nil).for(:users_get_by_id_limit_allowlist) } + it { is_expected.to allow_value([]).for(:users_get_by_id_limit_allowlist) } + it { is_expected.to allow_value('all_tiers').for(:whats_new_variant) } it { is_expected.to allow_value('current_tier').for(:whats_new_variant) } it { is_expected.to allow_value('disabled').for(:whats_new_variant) } diff --git a/spec/models/instance_configuration_spec.rb b/spec/models/instance_configuration_spec.rb index a47bc6a5b6d..6b0d8d7ca4a 100644 --- a/spec/models/instance_configuration_spec.rb +++ b/spec/models/instance_configuration_spec.rb @@ -206,7 +206,8 @@ RSpec.describe InstanceConfiguration do group_download_export_limit: 1019, group_import_limit: 1020, raw_blob_request_limit: 1021, - user_email_lookup_limit: 1022 + user_email_lookup_limit: 1022, + users_get_by_id_limit: 1023 ) end @@ -230,6 +231,7 @@ RSpec.describe InstanceConfiguration do expect(rate_limits[:group_import]).to eq({ enabled: true, requests_per_period: 1020, period_in_seconds: 60 }) expect(rate_limits[:raw_blob]).to eq({ enabled: true, requests_per_period: 1021, period_in_seconds: 60 }) expect(rate_limits[:user_email_lookup]).to eq({ enabled: true, requests_per_period: 1022, period_in_seconds: 60 }) + expect(rate_limits[:users_get_by_id]).to eq({ enabled: true, requests_per_period: 1023, period_in_seconds: 600 }) end end end diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index ea7fee1c6f0..4e85bdebdb0 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -141,7 +141,8 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do personal_access_token_prefix: "GL-", user_deactivation_emails_enabled: false, admin_mode: true, - suggest_pipeline_enabled: false + suggest_pipeline_enabled: false, + users_get_by_id_limit: 456 } expect(response).to have_gitlab_http_status(:ok) @@ -196,6 +197,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['admin_mode']).to be(true) expect(json_response['user_deactivation_emails_enabled']).to be(false) expect(json_response['suggest_pipeline_enabled']).to be(false) + expect(json_response['users_get_by_id_limit']).to eq(456) end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 98875d7e8d2..985e07bf174 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -499,7 +499,8 @@ RSpec.describe API::Users do let_it_be(:user2, reload: true) { create(:user, username: 'another_user') } before do - allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:users_get_by_id, scope: user).and_return(false) + allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?) + .with(:users_get_by_id, scope: user, users_allowlist: []).and_return(false) end it "returns a user by id" do @@ -600,7 +601,7 @@ RSpec.describe API::Users do context 'when the rate limit is not exceeded' do it 'returns a success status' do expect(Gitlab::ApplicationRateLimiter) - .to receive(:throttled?).with(:users_get_by_id, scope: user) + .to receive(:throttled?).with(:users_get_by_id, scope: user, users_allowlist: []) .and_return(false) get api("/users/#{user.id}", user) @@ -613,7 +614,7 @@ RSpec.describe API::Users do context 'when feature flag is enabled' do it 'returns "too many requests" status' do expect(Gitlab::ApplicationRateLimiter) - .to receive(:throttled?).with(:users_get_by_id, scope: user) + .to receive(:throttled?).with(:users_get_by_id, scope: user, users_allowlist: []) .and_return(true) get api("/users/#{user.id}", user) @@ -629,6 +630,24 @@ RSpec.describe API::Users do expect(response).to have_gitlab_http_status(:ok) end + + it 'allows users whose username is in the allowlist' do + allowlist = [user.username] + current_settings = Gitlab::CurrentSettings.current_application_settings + + # Necessary to ensure the same object is returned on each call + allow(Gitlab::CurrentSettings).to receive(:current_application_settings).and_return current_settings + + allow(current_settings).to receive(:users_get_by_id_limit_allowlist).and_return(allowlist) + + expect(Gitlab::ApplicationRateLimiter) + .to receive(:throttled?).with(:users_get_by_id, scope: user, users_allowlist: allowlist) + .and_call_original + + get api("/users/#{user.id}", user) + + expect(response).to have_gitlab_http_status(:ok) + end end context 'when feature flag is disabled' do diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb index 5c9d2c5e680..e20d59fb0ef 100644 --- a/spec/services/application_settings/update_service_spec.rb +++ b/spec/services/application_settings/update_service_spec.rb @@ -475,6 +475,24 @@ RSpec.describe ApplicationSettings::UpdateService do end end + context 'when users_get_by_id_limit and users_get_by_id_limit_allowlist_raw are passed' do + let(:params) do + { + users_get_by_id_limit: 456, + users_get_by_id_limit_allowlist_raw: 'someone, someone_else' + } + end + + it 'updates users_get_by_id_limit and users_get_by_id_limit_allowlist value' do + subject.execute + + application_settings.reload + + expect(application_settings.users_get_by_id_limit).to eq(456) + expect(application_settings.users_get_by_id_limit_allowlist).to eq(%w[someone someone_else]) + end + end + context 'when require_admin_approval_after_user_signup changes' do context 'when it goes from enabled to disabled' do let(:params) { { require_admin_approval_after_user_signup: false } } |
