diff options
110 files changed, 773 insertions, 455 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index bbac90ce56c..ff544fd4272 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -933db61fb6cd67139730d83ff2170624986d30e5 +42c93313900baabb0621ef2fb95feae8050ca418 diff --git a/app/assets/javascripts/groups/index.js b/app/assets/javascripts/groups/index.js index f6711bde7d0..f4981c020cd 100644 --- a/app/assets/javascripts/groups/index.js +++ b/app/assets/javascripts/groups/index.js @@ -2,11 +2,11 @@ import { GlToast } from '@gitlab/ui'; import Vue from 'vue'; import { parseBoolean } from '~/lib/utils/common_utils'; import UserCallout from '~/user_callout'; +import GroupItemComponent from 'jh_else_ce/groups/components/group_item.vue'; import Translate from '../vue_shared/translate'; import GroupsApp from './components/app.vue'; import GroupFolderComponent from './components/group_folder.vue'; -import GroupItemComponent from './components/group_item.vue'; import { GROUPS_LIST_HOLDER_CLASS, CONTENT_LIST_CLASS } from './constants'; import GroupFilterableList from './groups_filterable_list'; import GroupsService from './service/groups_service'; diff --git a/app/assets/javascripts/groups/init_overview_tabs.js b/app/assets/javascripts/groups/init_overview_tabs.js index 4064520d1ca..ced5d76d8b9 100644 --- a/app/assets/javascripts/groups/init_overview_tabs.js +++ b/app/assets/javascripts/groups/init_overview_tabs.js @@ -2,8 +2,8 @@ import Vue from 'vue'; import VueRouter from 'vue-router'; import { GlToast } from '@gitlab/ui'; import { parseBoolean } from '~/lib/utils/common_utils'; +import GroupItem from 'jh_else_ce/groups/components/group_item.vue'; import GroupFolder from './components/group_folder.vue'; -import GroupItem from './components/group_item.vue'; import { ACTIVE_TAB_SUBGROUPS_AND_PROJECTS, ACTIVE_TAB_SHARED, diff --git a/app/assets/javascripts/search/index.js b/app/assets/javascripts/search/index.js index f5684cebbf9..1e4b1e36514 100644 --- a/app/assets/javascripts/search/index.js +++ b/app/assets/javascripts/search/index.js @@ -15,7 +15,7 @@ export const initSearchApp = () => { const store = createStore({ query, navigation, - useSidebarNavigation: gon.use_new_navigation, + useNewNavigation: gon.use_new_navigation, }); initTopbar(store); diff --git a/app/assets/javascripts/search/sidebar/components/app.vue b/app/assets/javascripts/search/sidebar/components/app.vue index 7046a57058d..317145d4cd1 100644 --- a/app/assets/javascripts/search/sidebar/components/app.vue +++ b/app/assets/javascripts/search/sidebar/components/app.vue @@ -1,7 +1,7 @@ <script> import { mapState, mapGetters } from 'vuex'; -import ScopeLegacyNavigation from '~/search/sidebar/components/scope_legacy_navigation.vue'; -import ScopeSidebarNavigation from '~/search/sidebar/components/scope_sidebar_navigation.vue'; +import ScopeNavigation from '~/search/sidebar/components/scope_navigation.vue'; +import ScopeNewNavigation from '~/search/sidebar/components/scope_new_navigation.vue'; import SidebarPortal from '~/super_sidebar/components/sidebar_portal.vue'; import { SCOPE_ISSUES, SCOPE_MERGE_REQUESTS, SCOPE_BLOB } from '../constants'; import ResultsFilters from './results_filters.vue'; @@ -11,14 +11,13 @@ export default { name: 'GlobalSearchSidebar', components: { ResultsFilters, - ScopeLegacyNavigation, - ScopeSidebarNavigation, + ScopeNavigation, + ScopeNewNavigation, LanguageFilter, SidebarPortal, }, computed: { - // useSidebarNavigation refers to whether the new left sidebar navigation is enabled - ...mapState(['useSidebarNavigation']), + ...mapState(['urlQuery', 'useNewNavigation']), ...mapGetters(['currentScope']), showIssueAndMergeFilters() { return this.currentScope === SCOPE_ISSUES || this.currentScope === SCOPE_MERGE_REQUESTS; @@ -26,9 +25,7 @@ export default { showBlobFilter() { return this.currentScope === SCOPE_BLOB; }, - showScopeNavigation() { - // showLegacyNavigation refers to whether the scope navigation should be shown - // while the legacy navigation is being used and there are no search results the scope navigation has to be hidden + showOldNavigation() { return Boolean(this.currentScope); }, }, @@ -36,18 +33,18 @@ export default { </script> <template> - <section v-if="useSidebarNavigation"> + <section v-if="useNewNavigation"> <sidebar-portal> - <scope-sidebar-navigation /> + <scope-new-navigation /> <results-filters v-if="showIssueAndMergeFilters" /> <language-filter v-if="showBlobFilter" /> </sidebar-portal> </section> <section - v-else-if="showScopeNavigation" + v-else class="search-sidebar gl-display-flex gl-flex-direction-column gl-md-mr-5 gl-mb-6 gl-mt-5" > - <scope-legacy-navigation /> + <scope-navigation /> <results-filters v-if="showIssueAndMergeFilters" /> <language-filter v-if="showBlobFilter" /> </section> diff --git a/app/assets/javascripts/search/sidebar/components/scope_legacy_navigation.vue b/app/assets/javascripts/search/sidebar/components/scope_navigation.vue index e682369d60b..fc41baee831 100644 --- a/app/assets/javascripts/search/sidebar/components/scope_legacy_navigation.vue +++ b/app/assets/javascripts/search/sidebar/components/scope_navigation.vue @@ -8,7 +8,7 @@ import { NAV_LINK_DEFAULT_CLASSES, NAV_LINK_COUNT_DEFAULT_CLASSES } from '../con import { slugifyWithUnderscore } from '../../../lib/utils/text_utility'; export default { - name: 'ScopeLegacyNavigation', + name: 'ScopeNavigation', i18n: { countOverLimitLabel: s__('GlobalSearch|Result count is over limit.'), }, diff --git a/app/assets/javascripts/search/sidebar/components/scope_sidebar_navigation.vue b/app/assets/javascripts/search/sidebar/components/scope_new_navigation.vue index 3707e152e47..86b7cc577a6 100644 --- a/app/assets/javascripts/search/sidebar/components/scope_sidebar_navigation.vue +++ b/app/assets/javascripts/search/sidebar/components/scope_new_navigation.vue @@ -6,7 +6,7 @@ import NavItem from '~/super_sidebar/components/nav_item.vue'; import { NAV_LINK_DEFAULT_CLASSES, NAV_LINK_COUNT_DEFAULT_CLASSES } from '../constants'; export default { - name: 'ScopeSidebarNavigation', + name: 'ScopeNewNavigation', i18n: { countOverLimitLabel: s__('GlobalSearch|Result count is over limit.'), }, diff --git a/app/assets/javascripts/search/store/index.js b/app/assets/javascripts/search/store/index.js index 2478518c157..634f8f7a7fa 100644 --- a/app/assets/javascripts/search/store/index.js +++ b/app/assets/javascripts/search/store/index.js @@ -7,11 +7,11 @@ import createState from './state'; Vue.use(Vuex); -export const getStoreConfig = (storeInitValues) => ({ +export const getStoreConfig = ({ query, navigation, useNewNavigation }) => ({ actions, getters, mutations, - state: createState(storeInitValues), + state: createState({ query, navigation, useNewNavigation }), }); const createStore = (config) => new Vuex.Store(getStoreConfig(config)); diff --git a/app/assets/javascripts/search/store/state.js b/app/assets/javascripts/search/store/state.js index c897d4108a8..a62b6728819 100644 --- a/app/assets/javascripts/search/store/state.js +++ b/app/assets/javascripts/search/store/state.js @@ -1,7 +1,7 @@ import { cloneDeep } from 'lodash'; import { GROUPS_LOCAL_STORAGE_KEY, PROJECTS_LOCAL_STORAGE_KEY } from './constants'; -const createState = ({ query, navigation, useSidebarNavigation }) => ({ +const createState = ({ query, navigation, useNewNavigation }) => ({ urlQuery: cloneDeep(query), query, groups: [], @@ -14,7 +14,7 @@ const createState = ({ query, navigation, useSidebarNavigation }) => ({ }, sidebarDirty: false, navigation, - useSidebarNavigation, + useNewNavigation, aggregations: { error: false, fetching: false, diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb index 0d64a685065..d747d397e20 100644 --- a/app/controllers/concerns/uploads_actions.rb +++ b/app/controllers/concerns/uploads_actions.rb @@ -11,7 +11,7 @@ module UploadsActions prepend_before_action :set_request_format_from_path_extension rescue_from FileUploader::InvalidSecret, with: :render_404 - rescue_from ::Gitlab::Utils::PathTraversalAttackError do + rescue_from ::Gitlab::PathTraversal::PathTraversalAttackError do head :bad_request end end @@ -37,7 +37,7 @@ module UploadsActions # - or redirect to its URL # def show - Gitlab::Utils.check_path_traversal!(params[:filename]) + Gitlab::PathTraversal.check_path_traversal!(params[:filename]) return render_404 unless uploader&.exists? diff --git a/app/controllers/groups/dependency_proxy_for_containers_controller.rb b/app/controllers/groups/dependency_proxy_for_containers_controller.rb index 1b1aed0ec2e..1fc631f299b 100644 --- a/app/controllers/groups/dependency_proxy_for_containers_controller.rb +++ b/app/controllers/groups/dependency_proxy_for_containers_controller.rb @@ -121,7 +121,7 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy end def manifest_file_name - @manifest_file_name ||= Gitlab::Utils.check_path_traversal!("#{image}:#{tag}.json") + @manifest_file_name ||= Gitlab::PathTraversal.check_path_traversal!("#{image}:#{tag}.json") end def group diff --git a/app/controllers/projects/releases_controller.rb b/app/controllers/projects/releases_controller.rb index 7c569df7267..6a6a47bc33d 100644 --- a/app/controllers/projects/releases_controller.rb +++ b/app/controllers/projects/releases_controller.rb @@ -74,6 +74,6 @@ class Projects::ReleasesController < Projects::ApplicationController end def validate_suffix_path - Gitlab::Utils.check_path_traversal!(params[:suffix_path]) if params[:suffix_path] + Gitlab::PathTraversal.check_path_traversal!(params[:suffix_path]) if params[:suffix_path] end end diff --git a/app/controllers/repositories/lfs_api_controller.rb b/app/controllers/repositories/lfs_api_controller.rb index d52ae723eee..32119ddf89e 100644 --- a/app/controllers/repositories/lfs_api_controller.rb +++ b/app/controllers/repositories/lfs_api_controller.rb @@ -6,6 +6,10 @@ module Repositories include Gitlab::Utils::StrongMemoize LFS_TRANSFER_CONTENT_TYPE = 'application/octet-stream' + # Downloading directly with presigned URLs via batch requests + # require longer expire time. + # The 1h should be enough to download 100 objects. + LFS_DIRECT_BATCH_EXPIRE_IN = 3600.seconds skip_before_action :lfs_check_access!, only: [:deprecated] before_action :lfs_check_batch_operation!, only: [:batch] @@ -22,7 +26,11 @@ module Repositories end if download_request? - render json: { objects: download_objects! }, content_type: LfsRequest::CONTENT_TYPE + if Feature.enabled?(:lfs_batch_direct_downloads, project) + render json: { objects: download_objects! }, content_type: LfsRequest::CONTENT_TYPE + else + render json: { objects: legacy_download_objects! }, content_type: LfsRequest::CONTENT_TYPE + end elsif upload_request? render json: { objects: upload_objects! }, content_type: LfsRequest::CONTENT_TYPE else @@ -52,11 +60,34 @@ module Repositories end def download_objects! + existing_oids = project.lfs_objects + .for_oids(objects_oids) + .index_by(&:oid) + + objects.each do |object| + if lfs_object = existing_oids[object[:oid]] + object[:actions] = download_actions(object, lfs_object) + + if Guest.can?(:download_code, project) + object[:authenticated] = true + end + else + object[:error] = { + code: 404, + message: _("Object does not exist on the server or you don't have permissions to access it") + } + end + end + + objects + end + + def legacy_download_objects! existing_oids = project.lfs_objects_oids(oids: objects_oids) objects.each do |object| if existing_oids.include?(object[:oid]) - object[:actions] = download_actions(object) + object[:actions] = proxy_download_actions(object) if Guest.can?(:download_code, project) object[:authenticated] = true @@ -85,7 +116,26 @@ module Repositories objects end - def download_actions(object) + def download_actions(object, lfs_object) + if lfs_object.file.file_storage? || lfs_object.file.class.proxy_download_enabled? + proxy_download_actions(object) + else + direct_download_actions(lfs_object) + end + end + + def direct_download_actions(lfs_object) + { + download: { + href: lfs_object.file.url( + content_type: "application/octet-stream", + expire_at: LFS_DIRECT_BATCH_EXPIRE_IN.since + ) + } + } + end + + def proxy_download_actions(object) { download: { href: "#{project.http_url_to_repo}/gitlab-lfs/objects/#{object[:oid]}", diff --git a/app/finders/uploader_finder.rb b/app/finders/uploader_finder.rb index 0d1de0d56fd..e4a0e831720 100644 --- a/app/finders/uploader_finder.rb +++ b/app/finders/uploader_finder.rb @@ -16,12 +16,12 @@ class UploaderFinder retrieve_file_state! uploader - rescue ::Gitlab::Utils::PathTraversalAttackError + rescue ::Gitlab::PathTraversal::PathTraversalAttackError nil # no-op if for incorrect files end def prevent_path_traversal_attack! - Gitlab::Utils.check_path_traversal!(@file_path) + Gitlab::PathTraversal.check_path_traversal!(@file_path) end def retrieve_file_state! diff --git a/app/helpers/ssh_keys_helper.rb b/app/helpers/ssh_keys_helper.rb index 13d6851f3cd..6100093b7c2 100644 --- a/app/helpers/ssh_keys_helper.rb +++ b/app/helpers/ssh_keys_helper.rb @@ -52,6 +52,6 @@ module SshKeysHelper quoted_allowed_algorithms = allowed_algorithms.map { |name| "'#{name}'" } - Gitlab::Utils.to_exclusive_sentence(quoted_allowed_algorithms) + Gitlab::Sentence.to_exclusive_sentence(quoted_allowed_algorithms) end end diff --git a/app/models/concerns/has_user_type.rb b/app/models/concerns/has_user_type.rb index 468ea26c51a..9d4b8328e8d 100644 --- a/app/models/concerns/has_user_type.rb +++ b/app/models/concerns/has_user_type.rb @@ -4,7 +4,6 @@ module HasUserType extend ActiveSupport::Concern USER_TYPES = { - human_deprecated: nil, human: 0, support_bot: 1, alert_bot: 2, @@ -39,25 +38,20 @@ module HasUserType # `service_account` allows instance/namespaces to configure a user for external integrations/automations # `service_user` is an internal, `gitlab-com`-specific user type for integrations like suggested reviewers - NON_INTERNAL_USER_TYPES = %w[human human_deprecated project_bot service_user service_account].freeze + NON_INTERNAL_USER_TYPES = %w[human project_bot service_user service_account].freeze INTERNAL_USER_TYPES = (USER_TYPES.keys - NON_INTERNAL_USER_TYPES).freeze included do enum user_type: USER_TYPES - scope :humans, -> { where(user_type: :human).or(where(user_type: :human_deprecated)) } - # Override default scope to include temporary human type. See https://gitlab.com/gitlab-org/gitlab/-/issues/386474 - scope :human, -> { humans } scope :bots, -> { where(user_type: BOT_USER_TYPES) } - scope :without_bots, -> { humans.or(where(user_type: USER_TYPES.keys - BOT_USER_TYPES)) } - scope :non_internal, -> { humans.or(where(user_type: NON_INTERNAL_USER_TYPES)) } - scope :without_ghosts, -> { humans.or(where(user_type: USER_TYPES.keys - ['ghost'])) } - scope :without_project_bot, -> { humans.or(where(user_type: USER_TYPES.keys - ['project_bot'])) } - scope :human_or_service_user, -> { humans.or(where(user_type: :service_user)) } + scope :without_bots, -> { where(user_type: USER_TYPES.keys - BOT_USER_TYPES) } + scope :non_internal, -> { where(user_type: NON_INTERNAL_USER_TYPES) } + scope :without_ghosts, -> { where(user_type: USER_TYPES.keys - ['ghost']) } + scope :without_project_bot, -> { where(user_type: USER_TYPES.keys - ['project_bot']) } + scope :human_or_service_user, -> { where(user_type: %i[human service_user]) } - def human? - super || human_deprecated? || user_type.nil? - end + validates :user_type, presence: true end def bot? diff --git a/app/models/concerns/sanitizable.rb b/app/models/concerns/sanitizable.rb index 653d7a4875d..d05ce389ebf 100644 --- a/app/models/concerns/sanitizable.rb +++ b/app/models/concerns/sanitizable.rb @@ -48,11 +48,11 @@ module Sanitizable # This method raises an exception on failure so perform this # last if multiple errors should be returned. - Gitlab::Utils.check_path_traversal!(input.to_s) + Gitlab::PathTraversal.check_path_traversal!(input.to_s) rescue Gitlab::Utils::DoubleEncodingError record.errors.add(attr, 'cannot contain escaped components') - rescue Gitlab::Utils::PathTraversalAttackError + rescue Gitlab::PathTraversal::PathTraversalAttackError record.errors.add(attr, "cannot contain a path traversal component") end end diff --git a/app/models/diff_viewer/base.rb b/app/models/diff_viewer/base.rb index 05552e83700..31118791075 100644 --- a/app/models/diff_viewer/base.rb +++ b/app/models/diff_viewer/base.rb @@ -88,7 +88,7 @@ module DiffViewer { viewer: switcher_title, reason: render_error_reason, - options: Gitlab::Utils.to_exclusive_sentence(render_error_options) + options: Gitlab::Sentence.to_exclusive_sentence(render_error_options) } end diff --git a/app/models/key.rb b/app/models/key.rb index 2ea71bfcd6d..fdc54c9f56e 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -182,7 +182,7 @@ class Key < ApplicationRecord def forbidden_key_type_message allowed_types = Gitlab::CurrentSettings.allowed_key_types.map(&:upcase) - "type is forbidden. Must be #{Gitlab::Utils.to_exclusive_sentence(allowed_types)}" + "type is forbidden. Must be #{Gitlab::Sentence.to_exclusive_sentence(allowed_types)}" end def expiration diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index d36857fc94a..33930836c48 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -592,8 +592,8 @@ class MergeRequestDiff < ApplicationRecord end def remove_cached_external_diff - Gitlab::Utils.check_path_traversal!(external_diff_cache_dir) - Gitlab::Utils.check_allowed_absolute_path!(external_diff_cache_dir, [Dir.tmpdir]) + Gitlab::PathTraversal.check_path_traversal!(external_diff_cache_dir) + Gitlab::PathTraversal.check_allowed_absolute_path!(external_diff_cache_dir, [Dir.tmpdir]) return unless Dir.exist?(external_diff_cache_dir) diff --git a/app/serializers/integrations/harbor_serializers/repository_entity.rb b/app/serializers/integrations/harbor_serializers/repository_entity.rb index f03465fe8e2..a6366ebfb36 100644 --- a/app/serializers/integrations/harbor_serializers/repository_entity.rb +++ b/app/serializers/integrations/harbor_serializers/repository_entity.rb @@ -47,8 +47,8 @@ module Integrations private def validate_path(path) - Gitlab::Utils.check_path_traversal!(path) - rescue ::Gitlab::Utils::PathTraversalAttackError + Gitlab::PathTraversal.check_path_traversal!(path) + rescue ::Gitlab::PathTraversal::PathTraversalAttackError Gitlab::AppLogger.error("Path traversal attack detected #{path}") '' end diff --git a/app/services/bulk_imports/archive_extraction_service.rb b/app/services/bulk_imports/archive_extraction_service.rb index fec8fd0e1f5..4485b19035b 100644 --- a/app/services/bulk_imports/archive_extraction_service.rb +++ b/app/services/bulk_imports/archive_extraction_service.rb @@ -41,11 +41,11 @@ module BulkImports attr_reader :tmpdir, :filename, :filepath def validate_filepath - Gitlab::Utils.check_path_traversal!(filepath) + Gitlab::PathTraversal.check_path_traversal!(filepath) end def validate_tmpdir - Gitlab::Utils.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir]) + Gitlab::PathTraversal.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir]) end def validate_symlink diff --git a/app/services/bulk_imports/file_decompression_service.rb b/app/services/bulk_imports/file_decompression_service.rb index 41616fc1c75..94573f6bb13 100644 --- a/app/services/bulk_imports/file_decompression_service.rb +++ b/app/services/bulk_imports/file_decompression_service.rb @@ -41,11 +41,11 @@ module BulkImports attr_reader :tmpdir, :filename, :filepath, :decompressed_filename, :decompressed_filepath def validate_filepath - Gitlab::Utils.check_path_traversal!(filepath) + Gitlab::PathTraversal.check_path_traversal!(filepath) end def validate_tmpdir - Gitlab::Utils.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir]) + Gitlab::PathTraversal.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir]) end def validate_decompressed_file_size diff --git a/app/services/bulk_imports/file_download_service.rb b/app/services/bulk_imports/file_download_service.rb index ee499c782b4..ef7e0ae8258 100644 --- a/app/services/bulk_imports/file_download_service.rb +++ b/app/services/bulk_imports/file_download_service.rb @@ -99,7 +99,7 @@ module BulkImports end def validate_tmpdir - Gitlab::Utils.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir]) + Gitlab::PathTraversal.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir]) end def filepath diff --git a/app/services/projects/readme_renderer_service.rb b/app/services/projects/readme_renderer_service.rb index 6871976aded..8fd33a717c5 100644 --- a/app/services/projects/readme_renderer_service.rb +++ b/app/services/projects/readme_renderer_service.rb @@ -17,9 +17,9 @@ module Projects end def sanitized_filename(template_name) - path = Gitlab::Utils.check_path_traversal!("#{template_name}.md.tt") + path = Gitlab::PathTraversal.check_path_traversal!("#{template_name}.md.tt") path = TEMPLATE_PATH.join(path).to_s - Gitlab::Utils.check_allowed_absolute_path!(path, [TEMPLATE_PATH.to_s]) + Gitlab::PathTraversal.check_allowed_absolute_path!(path, [TEMPLATE_PATH.to_s]) path end diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb index 2eb34288bd7..06bf742a22d 100644 --- a/app/uploaders/gitlab_uploader.rb +++ b/app/uploaders/gitlab_uploader.rb @@ -189,10 +189,10 @@ class GitlabUploader < CarrierWave::Uploader::Base # # @param [CarrierWave::SanitizedFile] # @return [Nil] - # @raise [Gitlab::Utils::PathTraversalAttackError] + # @raise [Gitlab::PathTraversal::PathTraversalAttackError] def protect_from_path_traversal!(file) PROTECTED_METHODS.each do |method| - Gitlab::Utils.check_path_traversal!(self.send(method)) # rubocop: disable GitlabSecurity/PublicSend + Gitlab::PathTraversal.check_path_traversal!(self.send(method)) # rubocop: disable GitlabSecurity/PublicSend rescue ObjectNotReadyError # Do nothing. This test was attempted before the file was ready for that method diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index 70f7625ffbd..672433ec534 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -426,7 +426,7 @@ module ObjectStorage end def retrieve_from_store!(identifier) - Gitlab::Utils.check_path_traversal!(identifier) + Gitlab::PathTraversal.check_path_traversal!(identifier) # We need to force assign the value of @filename so that we will still # get the original_filename in cases wherein the file points to a random generated diff --git a/app/validators/key_restriction_validator.rb b/app/validators/key_restriction_validator.rb index 0094d6156a3..3eb7aa4b652 100644 --- a/app/validators/key_restriction_validator.rb +++ b/app/validators/key_restriction_validator.rb @@ -31,7 +31,7 @@ class KeyRestrictionValidator < ActiveModel::EachValidator sizes << "allowed" if valid_restriction?(ALLOWED) sizes += self.class.supported_sizes(options[:type]) - Gitlab::Utils.to_exclusive_sentence(sizes) + Gitlab::Sentence.to_exclusive_sentence(sizes) end def valid_restriction?(value) diff --git a/app/views/devise/confirmations/new.html.haml b/app/views/devise/confirmations/new.html.haml index 5af247703f6..98c6e601fc9 100644 --- a/app/views/devise/confirmations/new.html.haml +++ b/app/views/devise/confirmations/new.html.haml @@ -15,7 +15,7 @@ = recaptcha_tags nonce: content_security_policy_nonce .gl-mt-5 - = render Pajamas::ButtonComponent.new(block: true, type: :submit, variant: :confirm) do + = render Pajamas::ButtonComponent.new(type: :submit, variant: :confirm, block: true, button_options: { class: 'gl-mb-0' }) do = _("Resend") .clearfix.prepend-top-20 diff --git a/app/views/devise/passwords/edit.html.haml b/app/views/devise/passwords/edit.html.haml index 498fb08969c..80aed344db0 100644 --- a/app/views/devise/passwords/edit.html.haml +++ b/app/views/devise/passwords/edit.html.haml @@ -1,7 +1,7 @@ = render 'devise/shared/tab_single', tab_title: _('Change your password') .login-box .login-body - = form_for(resource, as: resource_name, url: password_path(:user), html: { method: :put, class: 'gl-show-field-errors gl-pt-5' }) do |f| + = gitlab_ui_form_for(resource, as: resource_name, url: password_path(:user), html: { method: :put, class: 'gl-show-field-errors gl-pt-5' }) do |f| .devise-errors.gl-px-5 = render "devise/shared/error_messages", resource: resource = f.hidden_field :reset_password_token @@ -13,7 +13,8 @@ = f.label _('Confirm new password'), for: "user_password_confirmation" = f.password_field :password_confirmation, autocomplete: 'new-password', class: "form-control gl-form-input bottom", title: _('This field is required.'), data: { qa_selector: 'password_confirmation_field' }, required: true .clearfix.gl-px-5.gl-pb-5 - = f.submit _("Change your password"), class: "gl-button btn btn-confirm", data: { qa_selector: 'change_password_button' } + = render Pajamas::ButtonComponent.new(type: :submit, variant: :confirm, block: true, button_options: { class: 'gl-mb-0', data: { qa_selector: 'change_password_button' } }) do + = _('Change your password') .clearfix.prepend-top-20 %p diff --git a/app/views/devise/passwords/new.html.haml b/app/views/devise/passwords/new.html.haml index 93ac252823a..23b2f1d14d4 100644 --- a/app/views/devise/passwords/new.html.haml +++ b/app/views/devise/passwords/new.html.haml @@ -1,6 +1,6 @@ .login-box .login-body - = form_for(resource, as: resource_name, url: password_path(resource_name), html: { method: :post, class: 'gl-show-field-errors' }) do |f| + = gitlab_ui_form_for(resource, as: resource_name, url: password_path(resource_name), html: { method: :post, class: 'gl-show-field-errors' }) do |f| .devise-errors = render "devise/shared/error_messages", resource: resource .form-group.gl-px-5.gl-pt-5 @@ -17,7 +17,8 @@ = recaptcha_tags nonce: content_security_policy_nonce .gl-p-5 - = f.submit _("Reset password"), class: "gl-button btn-confirm btn" + = render Pajamas::ButtonComponent.new(type: :submit, variant: :confirm, block: true, button_options: { class: 'gl-mb-0' }) do + = _('Reset password') .clearfix.prepend-top-20 = render 'devise/shared/sign_in_link' diff --git a/app/views/devise/sessions/_new_ldap.html.haml b/app/views/devise/sessions/_new_ldap.html.haml index 89f17a62998..0e4c70e2e69 100644 --- a/app/views/devise/sessions/_new_ldap.html.haml +++ b/app/views/devise/sessions/_new_ldap.html.haml @@ -1,19 +1,18 @@ - server = local_assigns.fetch(:server) +- provider = server['provider_name'] - render_remember_me = remember_me_enabled? && local_assigns.fetch(:render_remember_me, true) - submit_message = local_assigns.fetch(:submit_message, _('Sign in')) -= form_tag(omniauth_callback_path(:user, server['provider_name']), id: 'new_ldap_user', class: "gl-show-field-errors") do - .form-group.gl-px-5.gl-pt-5 - = label_tag :username, "#{server['label']} Username" - = text_field_tag :username, nil, { class: "form-control gl-form-input top", title: _("This field is required."), autofocus: "autofocus", data: { qa_selector: 'username_field' }, required: true } - .form-group.gl-px-5 - = label_tag :password - = password_field_tag :password, nil, { class: 'form-control gl-form-input js-password', data: { id: 'password', name: 'password', qa_selector: 'password_field' } } += gitlab_ui_form_for(provider, url: omniauth_callback_path(:user, provider), html: { class: 'gl-p-5 gl-show-field-errors', aria: { live: 'assertive' }, data: { testid: 'new_ldap_user' }}) do |f| + .form-group + = f.label :username, _('Username') + = f.text_field :username, name: :username, autocomplete: :username, class: 'form-control gl-form-input top', title: _('This field is required.'), autofocus: 'autofocus', data: { qa_selector: 'username_field' }, required: true + .form-group + = f.label :password, _('Password') + = f.password_field :password, name: :password, autocomplete: :current_password, class: 'form-control gl-form-input js-password', data: { id: "#{provider}-password", name: 'password', qa_selector: 'password_field' } + - if render_remember_me - .gl-px-5 - = render Pajamas::CheckboxTagComponent.new(name: 'remember_me') do |c| - - c.with_label do - = _('Remember me') + = f.gitlab_ui_checkbox_component :remember_me, _('Remember me'), checkbox_options: { name: :remember_me, autocomplete: 'off' } - .submit-container.gl-px-5.gl-pb-5 - = submit_tag submit_message, class: "gl-button btn btn-confirm", data: { qa_selector: 'sign_in_button' } + = render Pajamas::ButtonComponent.new(type: :submit, variant: :confirm, block: true, button_options: { data: { qa_selector: 'sign_in_button' } }) do + = submit_message diff --git a/app/views/projects/blob/_render_error.html.haml b/app/views/projects/blob/_render_error.html.haml index 1ff68cd2d11..d5dd24a6995 100644 --- a/app/views/projects/blob/_render_error.html.haml +++ b/app/views/projects/blob/_render_error.html.haml @@ -3,5 +3,5 @@ The #{viewer.switcher_title} could not be displayed because #{blob_render_error_reason(viewer)}. You can - = Gitlab::Utils.to_exclusive_sentence(blob_render_error_options(viewer)).html_safe + = Gitlab::Sentence.to_exclusive_sentence(blob_render_error_options(viewer)).html_safe instead. diff --git a/app/views/projects/blob/viewers/_contributing.html.haml b/app/views/projects/blob/viewers/_contributing.html.haml index eac8c17b7ff..efc12a31603 100644 --- a/app/views/projects/blob/viewers/_contributing.html.haml +++ b/app/views/projects/blob/viewers/_contributing.html.haml @@ -4,6 +4,6 @@ - options = contribution_options(viewer.project) - if options.any? = succeed '.' do - = Gitlab::Utils.to_exclusive_sentence(options).html_safe + = Gitlab::Sentence.to_exclusive_sentence(options).html_safe - else = _("contribute to this project.") diff --git a/config/feature_flags/development/lfs_batch_direct_downloads.yml b/config/feature_flags/development/lfs_batch_direct_downloads.yml new file mode 100644 index 00000000000..71634fc1d63 --- /dev/null +++ b/config/feature_flags/development/lfs_batch_direct_downloads.yml @@ -0,0 +1,8 @@ +--- +name: lfs_batch_direct_downloads +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/122221 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/413684 +milestone: '16.1' +type: development +group: group::tenant scale +default_enabled: false diff --git a/db/fixtures/development/24_forks.rb b/db/fixtures/development/24_forks.rb index f92b7972de5..d6730ae983a 100644 --- a/db/fixtures/development/24_forks.rb +++ b/db/fixtures/development/24_forks.rb @@ -2,7 +2,7 @@ require './spec/support/sidekiq_middleware' Sidekiq::Testing.inline! do Gitlab::Seeder.quiet do - User.humans.not_mass_generated.sample(10).each do |user| + User.human.not_mass_generated.sample(10).each do |user| source_project = Project.not_mass_generated.public_only.sample ## diff --git a/db/migrate/20230529182720_recreate_billable_index.rb b/db/migrate/20230529182720_recreate_billable_index.rb new file mode 100644 index 00000000000..5e56dd7005a --- /dev/null +++ b/db/migrate/20230529182720_recreate_billable_index.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class RecreateBillableIndex < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = "index_users_for_active_billable_users" + + def up + remove_concurrent_index_by_name :users, INDEX_NAME + + add_concurrent_index :users, :id, name: INDEX_NAME, + where: "state = 'active' AND (user_type IN (0, 6, 4, 13)) AND (user_type IN (0, 4, 5))" + end + + def down + remove_concurrent_index_by_name :users, INDEX_NAME + + add_concurrent_index :users, :id, name: INDEX_NAME, + where: "state = 'active' AND (user_type IS NULL OR user_type IN (6, 4, 13)) " \ + "AND (user_type IS NULL OR user_type IN (4, 5))" + end +end diff --git a/db/migrate/20230529184716_recreated_activity_index.rb b/db/migrate/20230529184716_recreated_activity_index.rb new file mode 100644 index 00000000000..2b949d39de1 --- /dev/null +++ b/db/migrate/20230529184716_recreated_activity_index.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class RecreatedActivityIndex < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = 'index_users_on_id_and_last_activity_on_for_active_human_service' + + def up + remove_concurrent_index_by_name :users, INDEX_NAME + + add_concurrent_index :users, [:id, :last_activity_on], + name: INDEX_NAME, + where: "state = 'active' AND user_type IN (0, 4)" + end + + def down + remove_concurrent_index_by_name :users, INDEX_NAME + + add_concurrent_index :users, [:id, :last_activity_on], + name: INDEX_NAME, + where: "state = 'active' AND ((user_type IS NULL) OR (user_type = 4))" + end +end diff --git a/db/post_migrate/20230524093249_add_async_index_to_vsa_issues.rb b/db/post_migrate/20230524093249_add_async_index_to_vsa_issues.rb new file mode 100644 index 00000000000..b11dcae4b84 --- /dev/null +++ b/db/post_migrate/20230524093249_add_async_index_to_vsa_issues.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +class AddAsyncIndexToVsaIssues < Gitlab::Database::Migration[2.1] + include Gitlab::Database::PartitioningMigrationHelpers + + TABLE_NAME = :analytics_cycle_analytics_issue_stage_events + COLUMN_NAMES = %I[stage_event_hash_id group_id end_event_timestamp issue_id] + INDEX_NAME = 'index_issue_stage_events_for_consistency_check' + + disable_ddl_transaction! + + def up + # The table is hash partitioned + each_partition(TABLE_NAME) do |partition, partition_index_name| + prepare_async_index( + partition.identifier, + COLUMN_NAMES, + name: partition_index_name + ) + end + end + + def down + each_partition(TABLE_NAME) do |partition, partition_index_name| + unprepare_async_index_by_name(partition.identifier, partition_index_name) + end + end + + private + + def each_partition(table_name) + partitioned_table = find_partitioned_table(table_name) + partitioned_table.postgres_partitions.order(:name).each do |partition| + partition_index_name = generated_index_name(partition.identifier, INDEX_NAME) + + yield partition, partition_index_name + end + end +end diff --git a/db/post_migrate/20230524093355_add_async_index_to_vsa_mrs.rb b/db/post_migrate/20230524093355_add_async_index_to_vsa_mrs.rb new file mode 100644 index 00000000000..4104493cb36 --- /dev/null +++ b/db/post_migrate/20230524093355_add_async_index_to_vsa_mrs.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +class AddAsyncIndexToVsaMrs < Gitlab::Database::Migration[2.1] + include Gitlab::Database::PartitioningMigrationHelpers + + TABLE_NAME = :analytics_cycle_analytics_merge_request_stage_events + COLUMN_NAMES = %I[stage_event_hash_id group_id end_event_timestamp merge_request_id] + INDEX_NAME = 'index_mr_stage_events_for_consistency_check' + + disable_ddl_transaction! + + def up + # The table is hash partitioned + each_partition(TABLE_NAME) do |partition, partition_index_name| + prepare_async_index( + partition.identifier, + COLUMN_NAMES, + name: partition_index_name + ) + end + end + + def down + each_partition(TABLE_NAME) do |partition, partition_index_name| + unprepare_async_index_by_name(partition.identifier, partition_index_name) + end + end + + private + + def each_partition(table_name) + partitioned_table = find_partitioned_table(table_name) + partitioned_table.postgres_partitions.order(:name).each do |partition| + partition_index_name = generated_index_name(partition.identifier, INDEX_NAME) + + yield partition, partition_index_name + end + end +end diff --git a/db/post_migrate/20230529183648_remove_temporary_billable_index.rb b/db/post_migrate/20230529183648_remove_temporary_billable_index.rb new file mode 100644 index 00000000000..88940aad968 --- /dev/null +++ b/db/post_migrate/20230529183648_remove_temporary_billable_index.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class RemoveTemporaryBillableIndex < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = 'migrate_index_users_for_active_billable_users' + def up + remove_concurrent_index_by_name :users, INDEX_NAME + end + + def down + add_concurrent_index :users, :id, + name: INDEX_NAME, + where: "((state)::text = 'active'::text) " \ + "AND (user_type IS NULL OR user_type = 0 OR user_type = ANY (ARRAY[0, 6, 4, 13])) " \ + "AND (user_type IS NULL OR user_type = 0 OR user_type = ANY (ARRAY[0, 4, 5]))" + end +end diff --git a/db/post_migrate/20230529185110_cleanup_temporary_activity_index.rb b/db/post_migrate/20230529185110_cleanup_temporary_activity_index.rb new file mode 100644 index 00000000000..d3e75aa7975 --- /dev/null +++ b/db/post_migrate/20230529185110_cleanup_temporary_activity_index.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class CleanupTemporaryActivityIndex < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = 'i_users_on_last_activity_for_active_human_service_migration' + + def up + remove_concurrent_index_by_name :users, INDEX_NAME + end + + def down + add_concurrent_index :users, [:id, :last_activity_on], + name: INDEX_NAME, + where: "state = 'active' AND ((user_type IS NULL) OR (user_type = 0) OR (user_type = 4))" + end +end diff --git a/db/post_migrate/20230530100400_change_user_type_null.rb b/db/post_migrate/20230530100400_change_user_type_null.rb new file mode 100644 index 00000000000..a74e8c3a774 --- /dev/null +++ b/db/post_migrate/20230530100400_change_user_type_null.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class ChangeUserTypeNull < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + def up + add_not_null_constraint :users, :user_type, validate: false + end + + def down + remove_not_null_constraint :users, :user_type + end +end diff --git a/db/schema_migrations/20230524093249 b/db/schema_migrations/20230524093249 new file mode 100644 index 00000000000..1421e0aad4e --- /dev/null +++ b/db/schema_migrations/20230524093249 @@ -0,0 +1 @@ +82219f2b9352956f8e9d116d003236603442b9eae70406d814dcc2199ca512f7
\ No newline at end of file diff --git a/db/schema_migrations/20230524093355 b/db/schema_migrations/20230524093355 new file mode 100644 index 00000000000..399d33043da --- /dev/null +++ b/db/schema_migrations/20230524093355 @@ -0,0 +1 @@ +962b8a385dc47c892fc3a55542ec5dcb9426e60084418549dcbca438bc804bdd
\ No newline at end of file diff --git a/db/schema_migrations/20230529182720 b/db/schema_migrations/20230529182720 new file mode 100644 index 00000000000..a232e2ea178 --- /dev/null +++ b/db/schema_migrations/20230529182720 @@ -0,0 +1 @@ +8d56850b0b02aa7237c5a31c7c98f0db96e20a4d4fb0a0646482bc75bcadf615
\ No newline at end of file diff --git a/db/schema_migrations/20230529183648 b/db/schema_migrations/20230529183648 new file mode 100644 index 00000000000..c4d08703216 --- /dev/null +++ b/db/schema_migrations/20230529183648 @@ -0,0 +1 @@ +32991a8e920d92c6f1bff2754d2af44ed816602aa5a29012b5884ee3a017bdee
\ No newline at end of file diff --git a/db/schema_migrations/20230529184716 b/db/schema_migrations/20230529184716 new file mode 100644 index 00000000000..da1f596ea77 --- /dev/null +++ b/db/schema_migrations/20230529184716 @@ -0,0 +1 @@ +b3f7215cdec2da13bc1ca4f656ec7f7a356155e6e656baac388023a1231fe19d
\ No newline at end of file diff --git a/db/schema_migrations/20230529185110 b/db/schema_migrations/20230529185110 new file mode 100644 index 00000000000..eab98ff6549 --- /dev/null +++ b/db/schema_migrations/20230529185110 @@ -0,0 +1 @@ +6a3f8302ec88005555e91b6192476b49a1d032ce2c7f414cf9a908df16f93b3f
\ No newline at end of file diff --git a/db/schema_migrations/20230530100400 b/db/schema_migrations/20230530100400 new file mode 100644 index 00000000000..c27ba5036fb --- /dev/null +++ b/db/schema_migrations/20230530100400 @@ -0,0 +1 @@ +b25dec0fa0c5fb952ba29561d9bbc9364e8459f8a381f776b6a2bd99100be239
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 6a382cc533e..eb7d753fecb 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -26734,6 +26734,9 @@ ALTER TABLE ONLY chat_names ALTER TABLE ONLY chat_teams ADD CONSTRAINT chat_teams_pkey PRIMARY KEY (id); +ALTER TABLE users + ADD CONSTRAINT check_0dd5948e38 CHECK ((user_type IS NOT NULL)) NOT VALID; + ALTER TABLE vulnerability_scanners ADD CONSTRAINT check_37608c9db5 CHECK ((char_length(vendor) <= 255)) NOT VALID; @@ -29461,8 +29464,6 @@ CREATE UNIQUE INDEX i_pm_package_versions_on_package_id_and_version ON pm_packag CREATE UNIQUE INDEX i_pm_packages_purl_type_and_name ON pm_packages USING btree (purl_type, name); -CREATE INDEX i_users_on_last_activity_for_active_human_service_migration ON users USING btree (id, last_activity_on) WHERE (((state)::text = 'active'::text) AND ((user_type IS NULL) OR (user_type = 0) OR (user_type = 4))); - CREATE INDEX idx_analytics_devops_adoption_segments_on_namespace_id ON analytics_devops_adoption_segments USING btree (namespace_id); CREATE INDEX idx_analytics_devops_adoption_snapshots_finalized ON analytics_devops_adoption_snapshots USING btree (namespace_id, end_time) WHERE (recorded_at >= end_time); @@ -32781,7 +32782,7 @@ CREATE INDEX index_user_statuses_on_user_id ON user_statuses USING btree (user_i CREATE UNIQUE INDEX index_user_synced_attributes_metadata_on_user_id ON user_synced_attributes_metadata USING btree (user_id); -CREATE INDEX index_users_for_active_billable_users ON users USING btree (id) WHERE (((state)::text = 'active'::text) AND ((user_type IS NULL) OR (user_type = ANY (ARRAY[6, 4, 13]))) AND ((user_type IS NULL) OR (user_type = ANY (ARRAY[4, 5])))); +CREATE INDEX index_users_for_active_billable_users ON users USING btree (id) WHERE (((state)::text = 'active'::text) AND (user_type = ANY (ARRAY[0, 6, 4, 13])) AND (user_type = ANY (ARRAY[0, 4, 5]))); CREATE INDEX index_users_on_accepted_term_id ON users USING btree (accepted_term_id); @@ -32799,7 +32800,7 @@ CREATE INDEX index_users_on_feed_token ON users USING btree (feed_token); CREATE INDEX index_users_on_group_view ON users USING btree (group_view); -CREATE INDEX index_users_on_id_and_last_activity_on_for_active_human_service ON users USING btree (id, last_activity_on) WHERE (((state)::text = 'active'::text) AND ((user_type IS NULL) OR (user_type = 4))); +CREATE INDEX index_users_on_id_and_last_activity_on_for_active_human_service ON users USING btree (id, last_activity_on) WHERE (((state)::text = 'active'::text) AND (user_type = ANY (ARRAY[0, 4]))); CREATE INDEX index_users_on_incoming_email_token ON users USING btree (incoming_email_token); @@ -33121,8 +33122,6 @@ CREATE UNIQUE INDEX merge_request_user_mentions_on_mr_id_and_note_id_index ON me CREATE UNIQUE INDEX merge_request_user_mentions_on_mr_id_index ON merge_request_user_mentions USING btree (merge_request_id) WHERE (note_id IS NULL); -CREATE INDEX migrate_index_users_for_active_billable_users ON users USING btree (id) WHERE (((state)::text = 'active'::text) AND ((user_type IS NULL) OR (user_type = 0) OR (user_type = ANY (ARRAY[0, 6, 4, 13]))) AND ((user_type IS NULL) OR (user_type = 0) OR (user_type = ANY (ARRAY[0, 4, 5])))); - CREATE INDEX note_mentions_temp_index ON notes USING btree (id, noteable_type) WHERE (note ~~ '%@%'::text); CREATE UNIQUE INDEX one_canonical_wiki_page_slug_per_metadata ON wiki_page_slugs USING btree (wiki_page_meta_id) WHERE (canonical = true); diff --git a/doc/development/internal_api/index.md b/doc/development/internal_api/index.md index c1c0177609b..4db9739d746 100644 --- a/doc/development/internal_api/index.md +++ b/doc/development/internal_api/index.md @@ -1254,7 +1254,7 @@ Example request: ```shell curl --verbose --request PATCH "https://gitlab.example.com/api/scim/v2/groups/test_group/Users/f0b1d561c-21ff-4092-beab-8154b17f82f2" \ - --data '{ "Operations": [{"op":"Add","path":"name.formatted","value":"New Name"}] }' \ + --data '{ "Operations": [{"op":"Update","path":"name.formatted","value":"New Name"}] }' \ --header "Authorization: Bearer <your_scim_token>" --header "Content-Type: application/scim+json" ``` @@ -1468,7 +1468,7 @@ Example request: ```shell curl --verbose --request PATCH "https://gitlab.example.com/api/scim/v2/application/Users/f0b1d561c-21ff-4092-beab-8154b17f82f2" \ - --data '{ "Operations": [{"op":"Add","path":"name.formatted","value":"New Name"}] }' \ + --data '{ "Operations": [{"op":"Update","path":"active","value":"false"}] }' \ --header "Authorization: Bearer <your_scim_token>" --header "Content-Type: application/scim+json" ``` diff --git a/doc/development/secure_coding_guidelines.md b/doc/development/secure_coding_guidelines.md index e8fda066ca3..c5e7a58af0d 100644 --- a/doc/development/secure_coding_guidelines.md +++ b/doc/development/secure_coding_guidelines.md @@ -485,7 +485,7 @@ In order to prevent Path Traversal vulnerabilities, user-controlled filenames or #### GitLab specific validations -The methods `Gitlab::Utils.check_path_traversal!()` and `Gitlab::Utils.check_allowed_absolute_path!()` +The methods `Gitlab::PathTraversal.check_path_traversal!()` and `Gitlab::PathTraversal.check_allowed_absolute_path!()` can be used to validate user-supplied paths and prevent vulnerabilities. `check_path_traversal!()` will detect their Path Traversal payloads and accepts URL-encoded paths. `check_allowed_absolute_path!()` will check if a path is absolute and whether it is inside the allowed path list. By default, absolute @@ -495,7 +495,7 @@ parameter when using `check_allowed_absolute_path!()`. To use a combination of both checks, follow the example below: ```ruby -Gitlab::Utils.check_allowed_absolute_path_and_path_traversal!(path, path_allowlist) +Gitlab::PathTraversal.check_allowed_absolute_path_and_path_traversal!(path, path_allowlist) ``` In the REST API, we have the [`FilePath`](https://gitlab.com/gitlab-org/security/gitlab/-/blob/master/lib/api/validations/validators/file_path.rb) diff --git a/doc/tutorials/build_application.md b/doc/tutorials/build_application.md index cbeeb7a614f..f704a546612 100644 --- a/doc/tutorials/build_application.md +++ b/doc/tutorials/build_application.md @@ -22,6 +22,14 @@ Use CI/CD pipelines to automatically build, test, and deploy your code. | <i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Understand CI/CD rules](https://www.youtube.com/watch?v=QjQc-zeL16Q) (8m 56s) | Learn more about how to use CI/CD rules. | | | [Use Auto DevOps to deploy an application](../topics/autodevops/cloud_deployments/auto_devops_with_gke.md) | Deploy an application to Google Kubernetes Engine (GKE). | | +## Configure GitLab Runner + +Set up runners to run jobs in a pipeline. + +| Topic | Description | Good for beginners | +|-------|-------------|--------------------| +| [Configure GitLab Runner to use the Google Kubernetes Engine](configure_gitlab_runner_to_use_gke/index.md) | Learn how to configure GitLab Runner to use the GKE to run jobs. | | + ## Publish a static website Use GitLab Pages to publish a static website directly from your project. diff --git a/lib/api/ci/secure_files.rb b/lib/api/ci/secure_files.rb index 41faaf80c82..c08889b87a2 100644 --- a/lib/api/ci/secure_files.rb +++ b/lib/api/ci/secure_files.rb @@ -81,7 +81,7 @@ module API route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true post ':id/secure_files' do secure_file = user_project.secure_files.new( - name: Gitlab::Utils.check_path_traversal!(params[:name]) + name: Gitlab::PathTraversal.check_path_traversal!(params[:name]) ) secure_file.file = params[:file] diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 98f6733a6f2..df080c8e666 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -20,6 +20,10 @@ module API API_RESPONSE_STATUS_CODE = 'gitlab.api.response_status_code' INTEGER_ID_REGEX = /^-?\d+$/.freeze + def logger + API.logger + end + def declared_params(options = {}) options = { include_parent_namespaces: false }.merge(options) declared(params, options).to_h.symbolize_keys diff --git a/lib/api/validations/validators/file_path.rb b/lib/api/validations/validators/file_path.rb index 268ddc29d4e..2440d8890e2 100644 --- a/lib/api/validations/validators/file_path.rb +++ b/lib/api/validations/validators/file_path.rb @@ -8,7 +8,7 @@ module API options = @option.is_a?(Hash) ? @option : {} path_allowlist = options.fetch(:allowlist, []) path = params[attr_name] - Gitlab::Utils.check_allowed_absolute_path_and_path_traversal!(path, path_allowlist) + Gitlab::PathTraversal.check_allowed_absolute_path_and_path_traversal!(path, path_allowlist) rescue StandardError raise Grape::Exceptions::Validation.new( params: [@scope.full_name(attr_name)], diff --git a/lib/api/validations/validators/integer_or_custom_value.rb b/lib/api/validations/validators/integer_or_custom_value.rb index d2352495948..ad7848583fc 100644 --- a/lib/api/validations/validators/integer_or_custom_value.rb +++ b/lib/api/validations/validators/integer_or_custom_value.rb @@ -15,7 +15,7 @@ module API return if value.is_a?(Integer) return if @custom_values.map(&:downcase).include?(value.to_s.downcase) - valid_options = Gitlab::Utils.to_exclusive_sentence(['an integer'] + @custom_values) + valid_options = Gitlab::Sentence.to_exclusive_sentence(['an integer'] + @custom_values) raise Grape::Exceptions::Validation.new( params: [@scope.full_name(attr_name)], message: "should be #{valid_options}, however got #{value}" diff --git a/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb b/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb index 2e6a29f4738..68bd64dc2ff 100644 --- a/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb +++ b/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb @@ -18,8 +18,8 @@ module BulkImports # rubocop: disable CodeReuse/ActiveRecord def load(_context, file_path) - Gitlab::Utils.check_path_traversal!(file_path) - Gitlab::Utils.check_allowed_absolute_path!(file_path, [Dir.tmpdir]) + Gitlab::PathTraversal.check_path_traversal!(file_path) + Gitlab::PathTraversal.check_allowed_absolute_path!(file_path, [Dir.tmpdir]) return if tar_filepath?(file_path) return if lfs_json_filepath?(file_path) diff --git a/lib/bulk_imports/common/pipelines/uploads_pipeline.rb b/lib/bulk_imports/common/pipelines/uploads_pipeline.rb index a1b338aeb9f..06132791ea6 100644 --- a/lib/bulk_imports/common/pipelines/uploads_pipeline.rb +++ b/lib/bulk_imports/common/pipelines/uploads_pipeline.rb @@ -22,7 +22,7 @@ module BulkImports def load(context, file_path) # Validate that the path is OK to load - Gitlab::Utils.check_allowed_absolute_path_and_path_traversal!(file_path, [Dir.tmpdir]) + Gitlab::PathTraversal.check_allowed_absolute_path_and_path_traversal!(file_path, [Dir.tmpdir]) return if File.directory?(file_path) return if File.lstat(file_path).symlink? diff --git a/lib/bulk_imports/file_downloads/validations.rb b/lib/bulk_imports/file_downloads/validations.rb index ae94267a6e8..b852a50c888 100644 --- a/lib/bulk_imports/file_downloads/validations.rb +++ b/lib/bulk_imports/file_downloads/validations.rb @@ -22,7 +22,7 @@ module BulkImports private def validate_filepath - Gitlab::Utils.check_path_traversal!(filepath) + Gitlab::PathTraversal.check_path_traversal!(filepath) end def validate_content_type diff --git a/lib/bulk_imports/projects/pipelines/design_bundle_pipeline.rb b/lib/bulk_imports/projects/pipelines/design_bundle_pipeline.rb index 2d5231b0541..373cd2bd75a 100644 --- a/lib/bulk_imports/projects/pipelines/design_bundle_pipeline.rb +++ b/lib/bulk_imports/projects/pipelines/design_bundle_pipeline.rb @@ -20,8 +20,8 @@ module BulkImports end def load(_context, bundle_path) - Gitlab::Utils.check_path_traversal!(bundle_path) - Gitlab::Utils.check_allowed_absolute_path!(bundle_path, [Dir.tmpdir]) + Gitlab::PathTraversal.check_path_traversal!(bundle_path) + Gitlab::PathTraversal.check_allowed_absolute_path!(bundle_path, [Dir.tmpdir]) return unless portable.lfs_enabled? return unless File.exist?(bundle_path) diff --git a/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline.rb b/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline.rb index 9a3c582642f..f19d8931f4a 100644 --- a/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline.rb +++ b/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline.rb @@ -21,8 +21,8 @@ module BulkImports end def load(_context, bundle_path) - Gitlab::Utils.check_path_traversal!(bundle_path) - Gitlab::Utils.check_allowed_absolute_path!(bundle_path, [Dir.tmpdir]) + Gitlab::PathTraversal.check_path_traversal!(bundle_path) + Gitlab::PathTraversal.check_allowed_absolute_path!(bundle_path, [Dir.tmpdir]) return unless File.exist?(bundle_path) return if File.directory?(bundle_path) diff --git a/lib/gitlab/import_export/decompressed_archive_size_validator.rb b/lib/gitlab/import_export/decompressed_archive_size_validator.rb index 564008e7a73..104c9e6c456 100644 --- a/lib/gitlab/import_export/decompressed_archive_size_validator.rb +++ b/lib/gitlab/import_export/decompressed_archive_size_validator.rb @@ -85,7 +85,7 @@ module Gitlab end def validate_archive_path - Gitlab::Utils.check_path_traversal!(@archive_path) + Gitlab::PathTraversal.check_path_traversal!(@archive_path) raise(ServiceError, 'Archive path is a symlink') if File.lstat(@archive_path).symlink? raise(ServiceError, 'Archive path is not a file') unless File.file?(@archive_path) diff --git a/lib/gitlab/import_export/project/exported_relations_merger.rb b/lib/gitlab/import_export/project/exported_relations_merger.rb index dda3d00d608..b5eba768e56 100644 --- a/lib/gitlab/import_export/project/exported_relations_merger.rb +++ b/lib/gitlab/import_export/project/exported_relations_merger.rb @@ -20,8 +20,8 @@ module Gitlab tar_gz_full_path = File.join(dirpath, filename) decompress_path = File.join(dirpath, relation) - Gitlab::Utils.check_path_traversal!(tar_gz_full_path) - Gitlab::Utils.check_path_traversal!(decompress_path) + Gitlab::PathTraversal.check_path_traversal!(tar_gz_full_path) + Gitlab::PathTraversal.check_path_traversal!(decompress_path) # Download tar.gz download_or_copy_upload( diff --git a/lib/gitlab/import_export/recursive_merge_folders.rb b/lib/gitlab/import_export/recursive_merge_folders.rb index 982358699bd..827385d4daf 100644 --- a/lib/gitlab/import_export/recursive_merge_folders.rb +++ b/lib/gitlab/import_export/recursive_merge_folders.rb @@ -45,9 +45,9 @@ module Gitlab DEFAULT_DIR_MODE = 0o700 def self.merge(source_path, target_path) - Gitlab::Utils.check_path_traversal!(source_path) - Gitlab::Utils.check_path_traversal!(target_path) - Gitlab::Utils.check_allowed_absolute_path!(source_path, [Dir.tmpdir]) + Gitlab::PathTraversal.check_path_traversal!(source_path) + Gitlab::PathTraversal.check_path_traversal!(target_path) + Gitlab::PathTraversal.check_allowed_absolute_path!(source_path, [Dir.tmpdir]) recursive_merge(source_path, target_path) end diff --git a/lib/gitlab/path_traversal.rb b/lib/gitlab/path_traversal.rb new file mode 100644 index 00000000000..7b8afa79d3b --- /dev/null +++ b/lib/gitlab/path_traversal.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module Gitlab + module PathTraversal + extend self + PathTraversalAttackError = Class.new(StandardError) + + private_class_method def logger + @logger ||= Gitlab::AppLogger + end + + # Ensure that the relative path will not traverse outside the base directory + # We url decode the path to avoid passing invalid paths forward in url encoded format. + # Also see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24223#note_284122580 + # It also checks for ALT_SEPARATOR aka '\' (forward slash) + def check_path_traversal!(path) + return unless path + + path = path.to_s if path.is_a?(Gitlab::HashedPath) + raise PathTraversalAttackError, 'Invalid path' unless path.is_a?(String) + + path = ::Gitlab::Utils.decode_path(path) + path_regex = %r{(\A(\.{1,2})\z|\A\.\.[/\\]|[/\\]\.\.\z|[/\\]\.\.[/\\]|\n)} + + if path.match?(path_regex) + logger.warn(message: "Potential path traversal attempt detected", path: path.to_s) + raise PathTraversalAttackError, 'Invalid path' + end + + path + end + + def check_allowed_absolute_path!(path, allowlist) + return unless Pathname.new(path).absolute? + return if ::Gitlab::Utils.allowlisted?(path, allowlist) + + raise StandardError, "path #{path} is not allowed" + end + + def check_allowed_absolute_path_and_path_traversal!(path, path_allowlist) + traversal_path = check_path_traversal!(path) + raise StandardError, "path is not a string!" unless traversal_path.is_a?(String) + + check_allowed_absolute_path!(traversal_path, path_allowlist) + end + end +end diff --git a/lib/gitlab/sentence.rb b/lib/gitlab/sentence.rb new file mode 100644 index 00000000000..963459e31a3 --- /dev/null +++ b/lib/gitlab/sentence.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Gitlab + module Sentence + extend self + # Wraps ActiveSupport's Array#to_sentence to convert the given array to a + # comma-separated sentence joined with localized 'or' Strings instead of 'and'. + def to_exclusive_sentence(array) + array.to_sentence(two_words_connector: _(' or '), last_word_connector: _(', or ')) + end + end +end diff --git a/lib/gitlab/template/finders/global_template_finder.rb b/lib/gitlab/template/finders/global_template_finder.rb index 6d2677175e6..a6ab36e6cd2 100644 --- a/lib/gitlab/template/finders/global_template_finder.rb +++ b/lib/gitlab/template/finders/global_template_finder.rb @@ -25,7 +25,7 @@ module Gitlab # The key is untrusted input, so ensure we can't be directed outside # of base_dir - Gitlab::Utils.check_path_traversal!(file_name) + Gitlab::PathTraversal.check_path_traversal!(file_name) directory = select_directory(file_name) directory ? File.join(category_directory(directory), file_name) : nil diff --git a/lib/gitlab/template/finders/repo_template_finder.rb b/lib/gitlab/template/finders/repo_template_finder.rb index 9f0ba97bcdf..8343750e04a 100644 --- a/lib/gitlab/template/finders/repo_template_finder.rb +++ b/lib/gitlab/template/finders/repo_template_finder.rb @@ -29,7 +29,7 @@ module Gitlab # The key is untrusted input, so ensure we can't be directed outside # of base_dir inside the repository - Gitlab::Utils.check_path_traversal!(file_name) + Gitlab::PathTraversal.check_path_traversal!(file_name) directory = select_directory(file_name) raise FileNotFoundError if directory.nil? diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb index b92e7dbb725..dc0112c14d6 100644 --- a/lib/gitlab/utils.rb +++ b/lib/gitlab/utils.rb @@ -3,34 +3,8 @@ module Gitlab module Utils extend self - PathTraversalAttackError ||= Class.new(StandardError) DoubleEncodingError ||= Class.new(StandardError) - private_class_method def logger - @logger ||= Gitlab::AppLogger - end - - # Ensure that the relative path will not traverse outside the base directory - # We url decode the path to avoid passing invalid paths forward in url encoded format. - # Also see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24223#note_284122580 - # It also checks for ALT_SEPARATOR aka '\' (forward slash) - def check_path_traversal!(path) - return unless path - - path = path.to_s if path.is_a?(Gitlab::HashedPath) - raise PathTraversalAttackError, 'Invalid path' unless path.is_a?(String) - - path = decode_path(path) - path_regex = %r{(\A(\.{1,2})\z|\A\.\.[/\\]|[/\\]\.\.\z|[/\\]\.\.[/\\]|\n)} - - if path.match?(path_regex) - logger.warn(message: "Potential path traversal attempt detected", path: "#{path}") - raise PathTraversalAttackError, 'Invalid path' - end - - path - end - def allowlisted?(absolute_path, allowlist) path = absolute_path.downcase @@ -39,20 +13,6 @@ module Gitlab end end - def check_allowed_absolute_path!(path, allowlist) - return unless Pathname.new(path).absolute? - return if allowlisted?(path, allowlist) - - raise StandardError, "path #{path} is not allowed" - end - - def check_allowed_absolute_path_and_path_traversal!(path, path_allowlist) - traversal_path = check_path_traversal!(path) - raise StandardError, "path is not a string!" unless traversal_path.is_a?(String) - - check_allowed_absolute_path!(traversal_path, path_allowlist) - end - def decode_path(encoded_path) decoded = CGI.unescape(encoded_path) if decoded != CGI.unescape(decoded) @@ -103,12 +63,6 @@ module Gitlab .gsub(/(\A-+|-+\z)/, '') end - # Wraps ActiveSupport's Array#to_sentence to convert the given array to a - # comma-separated sentence joined with localized 'or' Strings instead of 'and'. - def to_exclusive_sentence(array) - array.to_sentence(two_words_connector: _(' or '), last_word_connector: _(', or ')) - end - # Converts newlines into HTML line break elements def nlbr(str) ActionView::Base.full_sanitizer.sanitize(+str, tags: []).gsub(/\r?\n/, '<br>').html_safe diff --git a/qa/qa/page/main/login.rb b/qa/qa/page/main/login.rb index f18e227424b..245f0a189cc 100644 --- a/qa/qa/page/main/login.rb +++ b/qa/qa/page/main/login.rb @@ -244,9 +244,7 @@ module QA Support::WaitForRequests.wait_for_requests - wait_until(sleep_interval: 5, message: '502 - GitLab is taking too much time to respond') do - has_no_text?('GitLab is taking too much time to respond') - end + wait_for_gitlab_to_respond # For debugging invalid login attempts has_notice?('Invalid login or password') @@ -255,10 +253,20 @@ module QA terms.accept_terms if terms.visible? end + Flow::UserOnboarding.onboard_user + + wait_for_gitlab_to_respond + Page::Main::Menu.perform(&:enable_new_navigation) if Runtime::Env.super_sidebar_enabled? Page::Main::Menu.validate_elements_present! unless skip_page_validation end + def wait_for_gitlab_to_respond + wait_until(sleep_interval: 5, message: '502 - GitLab is taking too much time to respond') do + has_no_text?('GitLab is taking too much time to respond') + end + end + def fill_in_credential(user) fill_element :login_field, user.username fill_element :password_field, user.password diff --git a/qa/qa/page/search/results.rb b/qa/qa/page/search/results.rb index a04fd9092d1..769f8accfca 100644 --- a/qa/qa/page/search/results.rb +++ b/qa/qa/page/search/results.rb @@ -4,7 +4,7 @@ module QA module Page module Search class Results < QA::Page::Base - view 'app/assets/javascripts/search/sidebar/components/scope_legacy_navigation.vue' do + view 'app/assets/javascripts/search/sidebar/components/scope_navigation.vue' do element :code_tab, ':data-qa-selector="qaSelectorValue(item)"' # rubocop:disable QA/ElementWithPattern element :projects_tab, ':data-qa-selector="qaSelectorValue(item)"' # rubocop:disable QA/ElementWithPattern end diff --git a/qa/qa/specs/features/browser_ui/1_manage/login/login_via_oauth_and_oidc_with_gitlab_as_idp_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/login/login_via_oauth_and_oidc_with_gitlab_as_idp_spec.rb index 0ef26d71abc..b2aa3166b9d 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/login/login_via_oauth_and_oidc_with_gitlab_as_idp_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/login/login_via_oauth_and_oidc_with_gitlab_as_idp_spec.rb @@ -63,8 +63,6 @@ module QA Flow::Login.sign_in(as: user, skip_page_validation: true) - Flow::UserOnboarding.onboard_user - expect(page.driver.current_url).to include(consumer_host) Page::Dashboard::Welcome.perform do |welcome| diff --git a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb index a59c90a3cf2..89a75fb53f2 100644 --- a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb +++ b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb @@ -28,7 +28,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do let(:image) { '../path_traversal' } it 'raises an error' do - expect { subject }.to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path') + expect { subject }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path') end end @@ -36,7 +36,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do let(:tag) { 'latest%2f..%2f..%2fpath_traversal' } it 'raises an error' do - expect { subject }.to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path') + expect { subject }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path') end end end diff --git a/spec/controllers/projects/releases_controller_spec.rb b/spec/controllers/projects/releases_controller_spec.rb index 17bf9308834..35ac7ed0aa4 100644 --- a/spec/controllers/projects/releases_controller_spec.rb +++ b/spec/controllers/projects/releases_controller_spec.rb @@ -317,7 +317,7 @@ RSpec.describe Projects::ReleasesController do it 'raises attack error' do expect do subject - end.to raise_error(Gitlab::Utils::PathTraversalAttackError) + end.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError) end end diff --git a/spec/frontend/groups/components/app_spec.js b/spec/frontend/groups/components/app_spec.js index 7b42e50fee5..0c265ec6cae 100644 --- a/spec/frontend/groups/components/app_spec.js +++ b/spec/frontend/groups/components/app_spec.js @@ -6,7 +6,7 @@ import waitForPromises from 'helpers/wait_for_promises'; import { createAlert } from '~/alert'; import appComponent from '~/groups/components/app.vue'; import groupFolderComponent from '~/groups/components/group_folder.vue'; -import groupItemComponent from '~/groups/components/group_item.vue'; +import groupItemComponent from 'jh_else_ce/groups/components/group_item.vue'; import eventHub from '~/groups/event_hub'; import GroupsService from '~/groups/service/groups_service'; import GroupsStore from '~/groups/store/groups_store'; diff --git a/spec/frontend/groups/components/group_folder_spec.js b/spec/frontend/groups/components/group_folder_spec.js index da31fb02f69..b274c01a43b 100644 --- a/spec/frontend/groups/components/group_folder_spec.js +++ b/spec/frontend/groups/components/group_folder_spec.js @@ -1,7 +1,7 @@ import { shallowMount } from '@vue/test-utils'; import Vue from 'vue'; import GroupFolder from '~/groups/components/group_folder.vue'; -import GroupItem from '~/groups/components/group_item.vue'; +import GroupItem from 'jh_else_ce/groups/components/group_item.vue'; import { MAX_CHILDREN_COUNT } from '~/groups/constants'; import { mockGroups, mockParentGroupItem } from '../mock_data'; diff --git a/spec/frontend/groups/components/group_item_spec.js b/spec/frontend/groups/components/group_item_spec.js index 663dd341a58..94460de9dd6 100644 --- a/spec/frontend/groups/components/group_item_spec.js +++ b/spec/frontend/groups/components/group_item_spec.js @@ -1,7 +1,7 @@ import { GlPopover } from '@gitlab/ui'; import waitForPromises from 'helpers/wait_for_promises'; import GroupFolder from '~/groups/components/group_folder.vue'; -import GroupItem from '~/groups/components/group_item.vue'; +import GroupItem from 'jh_else_ce/groups/components/group_item.vue'; import ItemActions from '~/groups/components/item_actions.vue'; import eventHub from '~/groups/event_hub'; import { getGroupItemMicrodata } from '~/groups/store/utils'; diff --git a/spec/frontend/groups/components/groups_spec.js b/spec/frontend/groups/components/groups_spec.js index c04eaa501ba..3cdbd3e38be 100644 --- a/spec/frontend/groups/components/groups_spec.js +++ b/spec/frontend/groups/components/groups_spec.js @@ -3,7 +3,7 @@ import { GlEmptyState } from '@gitlab/ui'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import GroupFolderComponent from '~/groups/components/group_folder.vue'; -import GroupItemComponent from '~/groups/components/group_item.vue'; +import GroupItemComponent from 'jh_else_ce/groups/components/group_item.vue'; import PaginationLinks from '~/vue_shared/components/pagination_links.vue'; import GroupsComponent from '~/groups/components/groups.vue'; import eventHub from '~/groups/event_hub'; diff --git a/spec/frontend/pages/admin/jobs/components/table/admin_job_table_app_spec.js b/spec/frontend/pages/admin/jobs/components/table/admin_job_table_app_spec.js index dad7308ac0a..71ebf64f43c 100644 --- a/spec/frontend/pages/admin/jobs/components/table/admin_job_table_app_spec.js +++ b/spec/frontend/pages/admin/jobs/components/table/admin_job_table_app_spec.js @@ -120,34 +120,28 @@ describe('Job table app', () => { }); it('should refetch jobs query on fetchJobsByStatus event', async () => { - jest.spyOn(wrapper.vm.$apollo.queries.jobs, 'refetch').mockImplementation(jest.fn()); - - expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(0); + expect(successHandler).toHaveBeenCalledTimes(1); await findTabs().vm.$emit('fetchJobsByStatus'); - expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(1); + expect(successHandler).toHaveBeenCalledTimes(2); }); it('avoids refetch jobs query when scope has not changed', async () => { - jest.spyOn(wrapper.vm.$apollo.queries.jobs, 'refetch').mockImplementation(jest.fn()); - - expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(0); + expect(successHandler).toHaveBeenCalledTimes(1); await findTabs().vm.$emit('fetchJobsByStatus', null); - expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(0); + expect(successHandler).toHaveBeenCalledTimes(1); }); it('should refetch jobs count query when the amount jobs and count do not match', async () => { - jest.spyOn(wrapper.vm.$apollo.queries.jobsCount, 'refetch').mockImplementation(jest.fn()); - - expect(wrapper.vm.$apollo.queries.jobsCount.refetch).toHaveBeenCalledTimes(0); + expect(countSuccessHandler).toHaveBeenCalledTimes(1); // after applying filter a new count is fetched findFilteredSearch().vm.$emit('filterJobsBySearch', [mockFailedSearchToken]); - expect(wrapper.vm.$apollo.queries.jobsCount.refetch).toHaveBeenCalledTimes(1); + expect(successHandler).toHaveBeenCalledTimes(2); // tab is switched to `finished`, no count await findTabs().vm.$emit('fetchJobsByStatus', ['FAILED', 'SUCCESS', 'CANCELED']); @@ -155,7 +149,7 @@ describe('Job table app', () => { // tab is switched back to `all`, the old filter count has to be overwritten with new count await findTabs().vm.$emit('fetchJobsByStatus', null); - expect(wrapper.vm.$apollo.queries.jobsCount.refetch).toHaveBeenCalledTimes(2); + expect(successHandler).toHaveBeenCalledTimes(4); }); describe('when infinite scrolling is triggered', () => { @@ -313,25 +307,21 @@ describe('Job table app', () => { it('refetches jobs query when filtering', async () => { createComponent(); - jest.spyOn(wrapper.vm.$apollo.queries.jobs, 'refetch').mockImplementation(jest.fn()); - - expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(0); + expect(successHandler).toHaveBeenCalledTimes(1); await findFilteredSearch().vm.$emit('filterJobsBySearch', [mockFailedSearchToken]); - expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(1); + expect(successHandler).toHaveBeenCalledTimes(2); }); it('refetches jobs count query when filtering', async () => { createComponent(); - jest.spyOn(wrapper.vm.$apollo.queries.jobsCount, 'refetch').mockImplementation(jest.fn()); - - expect(wrapper.vm.$apollo.queries.jobsCount.refetch).toHaveBeenCalledTimes(0); + expect(countSuccessHandler).toHaveBeenCalledTimes(1); await findFilteredSearch().vm.$emit('filterJobsBySearch', [mockFailedSearchToken]); - expect(wrapper.vm.$apollo.queries.jobsCount.refetch).toHaveBeenCalledTimes(1); + expect(countSuccessHandler).toHaveBeenCalledTimes(2); }); it('shows raw text warning when user inputs raw text', async () => { @@ -342,14 +332,14 @@ describe('Job table app', () => { createComponent(); - jest.spyOn(wrapper.vm.$apollo.queries.jobs, 'refetch').mockImplementation(jest.fn()); - jest.spyOn(wrapper.vm.$apollo.queries.jobsCount, 'refetch').mockImplementation(jest.fn()); + expect(successHandler).toHaveBeenCalledTimes(1); + expect(countSuccessHandler).toHaveBeenCalledTimes(1); await findFilteredSearch().vm.$emit('filterJobsBySearch', ['raw text']); expect(createAlert).toHaveBeenCalledWith(expectedWarning); - expect(wrapper.vm.$apollo.queries.jobs.refetch).toHaveBeenCalledTimes(0); - expect(wrapper.vm.$apollo.queries.jobsCount.refetch).toHaveBeenCalledTimes(0); + expect(successHandler).toHaveBeenCalledTimes(1); + expect(countSuccessHandler).toHaveBeenCalledTimes(1); }); it('updates URL query string when filtering jobs by status', async () => { diff --git a/spec/frontend/search/sidebar/components/app_spec.js b/spec/frontend/search/sidebar/components/app_spec.js index 4bdb034d4bb..963b73aeae5 100644 --- a/spec/frontend/search/sidebar/components/app_spec.js +++ b/spec/frontend/search/sidebar/components/app_spec.js @@ -4,8 +4,7 @@ import Vuex from 'vuex'; import { MOCK_QUERY } from 'jest/search/mock_data'; import GlobalSearchSidebar from '~/search/sidebar/components/app.vue'; import ResultsFilters from '~/search/sidebar/components/results_filters.vue'; -import ScopeLegacyNavigation from '~/search/sidebar/components/scope_legacy_navigation.vue'; -import ScopeSidebarNavigation from '~/search/sidebar/components/scope_sidebar_navigation.vue'; +import ScopeNavigation from '~/search/sidebar/components/scope_navigation.vue'; import LanguageFilter from '~/search/sidebar/components/language_filter/index.vue'; Vue.use(Vuex); @@ -13,16 +12,22 @@ Vue.use(Vuex); describe('GlobalSearchSidebar', () => { let wrapper; + const actionSpies = { + applyQuery: jest.fn(), + resetQuery: jest.fn(), + }; + const getterSpies = { currentScope: jest.fn(() => 'issues'), }; - const createComponent = (initialState = {}, featureFlags = {}) => { + const createComponent = (initialState, featureFlags) => { const store = new Vuex.Store({ state: { urlQuery: MOCK_QUERY, ...initialState, }, + actions: actionSpies, getters: getterSpies, }); @@ -38,14 +43,13 @@ describe('GlobalSearchSidebar', () => { const findSidebarSection = () => wrapper.find('section'); const findFilters = () => wrapper.findComponent(ResultsFilters); - const findScopeLegacyNavigation = () => wrapper.findComponent(ScopeLegacyNavigation); - const findScopeSidebarNavigation = () => wrapper.findComponent(ScopeSidebarNavigation); + const findSidebarNavigation = () => wrapper.findComponent(ScopeNavigation); const findLanguageAggregation = () => wrapper.findComponent(LanguageFilter); describe('renders properly', () => { describe('always', () => { beforeEach(() => { - createComponent(); + createComponent({}); }); it(`shows section`, () => { expect(findSidebarSection().exists()).toBe(true); @@ -73,24 +77,12 @@ describe('GlobalSearchSidebar', () => { }); }); - describe.each` - currentScope | sidebarNavShown | legacyNavShown - ${'issues'} | ${false} | ${true} - ${''} | ${false} | ${false} - ${'issues'} | ${true} | ${false} - ${''} | ${true} | ${false} - `('renders navigation', ({ currentScope, sidebarNavShown, legacyNavShown }) => { + describe('renders navigation', () => { beforeEach(() => { - getterSpies.currentScope = jest.fn(() => currentScope); - createComponent({ useSidebarNavigation: sidebarNavShown }); + createComponent({}); }); - - it(`${!legacyNavShown ? 'hides' : 'shows'} the legacy navigation`, () => { - expect(findScopeLegacyNavigation().exists()).toBe(legacyNavShown); - }); - - it(`${!sidebarNavShown ? 'hides' : 'shows'} the sidebar navigation`, () => { - expect(findScopeSidebarNavigation().exists()).toBe(sidebarNavShown); + it('shows the vertical navigation', () => { + expect(findSidebarNavigation().exists()).toBe(true); }); }); }); diff --git a/spec/frontend/search/sidebar/components/scope_legacy_navigation_spec.js b/spec/frontend/search/sidebar/components/scope_navigation_spec.js index 6a94da31a1b..e8737384f27 100644 --- a/spec/frontend/search/sidebar/components/scope_legacy_navigation_spec.js +++ b/spec/frontend/search/sidebar/components/scope_navigation_spec.js @@ -3,11 +3,11 @@ import { shallowMount } from '@vue/test-utils'; import Vue from 'vue'; import Vuex from 'vuex'; import { MOCK_QUERY, MOCK_NAVIGATION } from 'jest/search/mock_data'; -import ScopeLegacyNavigation from '~/search/sidebar/components/scope_legacy_navigation.vue'; +import ScopeNavigation from '~/search/sidebar/components/scope_navigation.vue'; Vue.use(Vuex); -describe('ScopeLegacyNavigation', () => { +describe('ScopeNavigation', () => { let wrapper; const actionSpies = { @@ -29,7 +29,7 @@ describe('ScopeLegacyNavigation', () => { getters: getterSpies, }); - wrapper = shallowMount(ScopeLegacyNavigation, { + wrapper = shallowMount(ScopeNavigation, { store, }); }; diff --git a/spec/frontend/search/sidebar/components/scope_sidebar_navigation_spec.js b/spec/frontend/search/sidebar/components/scope_new_navigation_spec.js index f31a7c8fa5d..5207665f883 100644 --- a/spec/frontend/search/sidebar/components/scope_sidebar_navigation_spec.js +++ b/spec/frontend/search/sidebar/components/scope_new_navigation_spec.js @@ -1,13 +1,13 @@ import { mount } from '@vue/test-utils'; import Vue, { nextTick } from 'vue'; import Vuex from 'vuex'; -import ScopeSidebarNavigation from '~/search/sidebar/components/scope_sidebar_navigation.vue'; +import ScopeNewNavigation from '~/search/sidebar/components/scope_new_navigation.vue'; import NavItem from '~/super_sidebar/components/nav_item.vue'; import { MOCK_QUERY, MOCK_NAVIGATION, MOCK_NAVIGATION_ITEMS } from '../../mock_data'; Vue.use(Vuex); -describe('ScopeSidebarNavigation', () => { +describe('ScopeNewNavigation', () => { let wrapper; const actionSpies = { @@ -30,7 +30,7 @@ describe('ScopeSidebarNavigation', () => { getters: getterSpies, }); - wrapper = mount(ScopeSidebarNavigation, { + wrapper = mount(ScopeNewNavigation, { store, stubs: { NavItem, diff --git a/spec/helpers/ssh_keys_helper_spec.rb b/spec/helpers/ssh_keys_helper_spec.rb index 522331090e4..dbb141e41cc 100644 --- a/spec/helpers/ssh_keys_helper_spec.rb +++ b/spec/helpers/ssh_keys_helper_spec.rb @@ -11,7 +11,7 @@ RSpec.describe SshKeysHelper do quoted_allowed_algorithms = allowed_algorithms.map { |name| "'#{name}'" } - expected_string = Gitlab::Utils.to_exclusive_sentence(quoted_allowed_algorithms) + expected_string = Gitlab::Sentence.to_exclusive_sentence(quoted_allowed_algorithms) expect(ssh_key_allowed_algorithms).to eq(expected_string) end diff --git a/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb b/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb index 5220b9d37e5..297ac0ca0ba 100644 --- a/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb +++ b/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb @@ -105,7 +105,7 @@ RSpec.describe BulkImports::Common::Pipelines::LfsObjectsPipeline do context 'when file path is being traversed' do it 'raises an error' do - expect { pipeline.load(context, File.join(tmpdir, '..')) }.to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path') + expect { pipeline.load(context, File.join(tmpdir, '..')) }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path') end end diff --git a/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb b/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb index d6622785e65..bc6d36452b4 100644 --- a/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb +++ b/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb @@ -128,7 +128,7 @@ RSpec.describe BulkImports::Common::Pipelines::UploadsPipeline, feature_category context 'when path traverses' do it 'does not upload the file' do path_traversal = "#{uploads_dir_path}/avatar/../../../../etc/passwd" - expect { pipeline.load(context, path_traversal) }.to not_change { portable.uploads.count }.and raise_error(Gitlab::Utils::PathTraversalAttackError) + expect { pipeline.load(context, path_traversal) }.to not_change { portable.uploads.count }.and raise_error(Gitlab::PathTraversal::PathTraversalAttackError) end end diff --git a/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb b/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb index 6a509ca7f14..5b7309b09f5 100644 --- a/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb +++ b/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb @@ -146,7 +146,7 @@ RSpec.describe BulkImports::Projects::Pipelines::DesignBundlePipeline do context 'when path is being traversed' do it 'raises an error' do expect { pipeline.load(context, File.join(tmpdir, '..')) } - .to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path') + .to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path') end end end diff --git a/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb b/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb index b8c21feb05d..07fafc19026 100644 --- a/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb +++ b/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb @@ -144,7 +144,7 @@ RSpec.describe BulkImports::Projects::Pipelines::RepositoryBundlePipeline do context 'when path is being traversed' do it 'raises an error' do expect { pipeline.load(context, File.join(tmpdir, '..')) } - .to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path') + .to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path') end end end diff --git a/spec/lib/gitlab/github_import/attachments_downloader_spec.rb b/spec/lib/gitlab/github_import/attachments_downloader_spec.rb index dc9f939a19b..f7781226ecf 100644 --- a/spec/lib/gitlab/github_import/attachments_downloader_spec.rb +++ b/spec/lib/gitlab/github_import/attachments_downloader_spec.rb @@ -44,7 +44,7 @@ RSpec.describe Gitlab::GithubImport::AttachmentsDownloader do it 'raises expected exception' do expect { downloader.perform }.to raise_exception( - Gitlab::Utils::PathTraversalAttackError, + Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path' ) end diff --git a/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb b/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb index 6e5be0b2829..cb8ac088493 100644 --- a/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb +++ b/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb @@ -31,7 +31,7 @@ RSpec.describe Gitlab::ImportExport::RecursiveMergeFolders do Dir.mktmpdir do |tmpdir| expect do described_class.merge("#{tmpdir}/../", tmpdir) - end.to raise_error(Gitlab::Utils::PathTraversalAttackError) + end.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError) end end @@ -47,7 +47,7 @@ RSpec.describe Gitlab::ImportExport::RecursiveMergeFolders do Dir.mktmpdir do |tmpdir| expect do described_class.merge(tmpdir, "#{tmpdir}/../") - end.to raise_error(Gitlab::Utils::PathTraversalAttackError) + end.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError) end end end diff --git a/spec/lib/gitlab/path_traversal_spec.rb b/spec/lib/gitlab/path_traversal_spec.rb new file mode 100644 index 00000000000..8ae1330c203 --- /dev/null +++ b/spec/lib/gitlab/path_traversal_spec.rb @@ -0,0 +1,173 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::PathTraversal, feature_category: :shared do + using RSpec::Parameterized::TableSyntax + + delegate :check_path_traversal!, :check_allowed_absolute_path!, + :check_allowed_absolute_path_and_path_traversal!, to: :described_class + + describe '.check_path_traversal!' do + it 'detects path traversal in string without any separators' do + expect { check_path_traversal!('.') }.to raise_error(/Invalid path/) + expect { check_path_traversal!('..') }.to raise_error(/Invalid path/) + end + + it 'detects path traversal at the start of the string' do + expect { check_path_traversal!('../foo') }.to raise_error(/Invalid path/) + expect { check_path_traversal!('..\\foo') }.to raise_error(/Invalid path/) + end + + it 'detects path traversal at the start of the string, even to just the subdirectory' do + expect { check_path_traversal!('../') }.to raise_error(/Invalid path/) + expect { check_path_traversal!('..\\') }.to raise_error(/Invalid path/) + expect { check_path_traversal!('/../') }.to raise_error(/Invalid path/) + expect { check_path_traversal!('\\..\\') }.to raise_error(/Invalid path/) + end + + it 'detects path traversal in the middle of the string' do + expect { check_path_traversal!('foo/../../bar') }.to raise_error(/Invalid path/) + expect { check_path_traversal!('foo\\..\\..\\bar') }.to raise_error(/Invalid path/) + expect { check_path_traversal!('foo/..\\bar') }.to raise_error(/Invalid path/) + expect { check_path_traversal!('foo\\../bar') }.to raise_error(/Invalid path/) + expect { check_path_traversal!('foo/..\\..\\..\\..\\../bar') }.to raise_error(/Invalid path/) + end + + it 'detects path traversal at the end of the string when slash-terminates' do + expect { check_path_traversal!('foo/../') }.to raise_error(/Invalid path/) + expect { check_path_traversal!('foo\\..\\') }.to raise_error(/Invalid path/) + end + + it 'detects path traversal at the end of the string' do + expect { check_path_traversal!('foo/..') }.to raise_error(/Invalid path/) + expect { check_path_traversal!('foo\\..') }.to raise_error(/Invalid path/) + end + + it 'does nothing for a safe string' do + expect(check_path_traversal!('./foo')).to eq('./foo') + expect(check_path_traversal!('.test/foo')).to eq('.test/foo') + expect(check_path_traversal!('..test/foo')).to eq('..test/foo') + expect(check_path_traversal!('dir/..foo.rb')).to eq('dir/..foo.rb') + expect(check_path_traversal!('dir/.foo.rb')).to eq('dir/.foo.rb') + end + + it 'logs potential path traversal attempts' do + expect(Gitlab::AppLogger).to receive(:warn) + .with(message: "Potential path traversal attempt detected", path: "..") + expect { check_path_traversal!('..') }.to raise_error(/Invalid path/) + end + + it 'logs does nothing for a safe string' do + expect(Gitlab::AppLogger).not_to receive(:warn) + .with(message: "Potential path traversal attempt detected", path: "dir/.foo.rb") + expect(check_path_traversal!('dir/.foo.rb')).to eq('dir/.foo.rb') + end + + it 'does nothing for nil' do + expect(check_path_traversal!(nil)).to be_nil + end + + it 'does nothing for safe HashedPath' do + expect(check_path_traversal!(Gitlab::HashedPath.new('tmp', root_hash: 1))) + .to eq '6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b/tmp' + end + + it 'raises for unsafe HashedPath' do + expect { check_path_traversal!(Gitlab::HashedPath.new('tmp', '..', 'etc', 'passwd', root_hash: 1)) } + .to raise_error(/Invalid path/) + end + + it 'raises for other non-strings' do + expect { check_path_traversal!(%w[/tmp /tmp/../etc/passwd]) }.to raise_error(/Invalid path/) + end + end + + describe '.check_allowed_absolute_path!' do + let(:allowed_paths) { ['/home/foo'] } + + it 'raises an exception if an absolute path is not allowed' do + expect { check_allowed_absolute_path!('/etc/passwd', allowed_paths) }.to raise_error(StandardError) + end + + it 'does nothing for an allowed absolute path' do + expect(check_allowed_absolute_path!('/home/foo', allowed_paths)).to be_nil + end + end + + describe '.check_allowed_absolute_path_and_path_traversal!' do + let(:allowed_paths) { %w[/home/foo ./foo .test/foo ..test/foo dir/..foo.rb dir/.foo.rb] } + + it 'detects path traversal in string without any separators' do + expect { check_allowed_absolute_path_and_path_traversal!('.', allowed_paths) } + .to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('..', allowed_paths) } + .to raise_error(/Invalid path/) + end + + it 'detects path traversal at the start of the string' do + expect { check_allowed_absolute_path_and_path_traversal!('../foo', allowed_paths) } + .to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('..\\foo', allowed_paths) } + .to raise_error(/Invalid path/) + end + + it 'detects path traversal at the start of the string, even to just the subdirectory' do + expect { check_allowed_absolute_path_and_path_traversal!('../', allowed_paths) } + .to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('..\\', allowed_paths) } + .to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('/../', allowed_paths) } + .to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('\\..\\', allowed_paths) } + .to raise_error(/Invalid path/) + end + + it 'detects path traversal in the middle of the string' do + expect { check_allowed_absolute_path_and_path_traversal!('foo/../../bar', allowed_paths) } + .to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('foo\\..\\..\\bar', allowed_paths) } + .to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('foo/..\\bar', allowed_paths) } + .to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('foo\\../bar', allowed_paths) } + .to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('foo/..\\..\\..\\..\\../bar', allowed_paths) } + .to raise_error(/Invalid path/) + end + + it 'detects path traversal at the end of the string when slash-terminates' do + expect { check_allowed_absolute_path_and_path_traversal!('foo/../', allowed_paths) } + .to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('foo\\..\\', allowed_paths) } + .to raise_error(/Invalid path/) + end + + it 'detects path traversal at the end of the string' do + expect { check_allowed_absolute_path_and_path_traversal!('foo/..', allowed_paths) } + .to raise_error(/Invalid path/) + expect { check_allowed_absolute_path_and_path_traversal!('foo\\..', allowed_paths) } + .to raise_error(/Invalid path/) + end + + it 'does not return errors for a safe string' do + expect(check_allowed_absolute_path_and_path_traversal!('./foo', allowed_paths)).to be_nil + expect(check_allowed_absolute_path_and_path_traversal!('.test/foo', allowed_paths)).to be_nil + expect(check_allowed_absolute_path_and_path_traversal!('..test/foo', allowed_paths)).to be_nil + expect(check_allowed_absolute_path_and_path_traversal!('dir/..foo.rb', allowed_paths)).to be_nil + expect(check_allowed_absolute_path_and_path_traversal!('dir/.foo.rb', allowed_paths)).to be_nil + end + + it 'raises error for a non-string' do + expect { check_allowed_absolute_path_and_path_traversal!(nil, allowed_paths) }.to raise_error(StandardError) + end + + it 'raises an exception if an absolute path is not allowed' do + expect { check_allowed_absolute_path!('/etc/passwd', allowed_paths) }.to raise_error(StandardError) + end + + it 'does nothing for an allowed absolute path' do + expect(check_allowed_absolute_path!('/home/foo', allowed_paths)).to be_nil + end + end +end diff --git a/spec/lib/gitlab/sentence_spec.rb b/spec/lib/gitlab/sentence_spec.rb new file mode 100644 index 00000000000..b37925abbc6 --- /dev/null +++ b/spec/lib/gitlab/sentence_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Sentence, feature_category: :shared do + delegate :to_exclusive_sentence, to: :described_class + + describe '.to_exclusive_sentence' do + it 'calls #to_sentence on the array' do + array = double + + expect(array).to receive(:to_sentence) + + to_exclusive_sentence(array) + end + + it 'joins arrays with two elements correctly' do + array = %w[foo bar] + + expect(to_exclusive_sentence(array)).to eq('foo or bar') + end + + it 'joins arrays with more than two elements correctly' do + array = %w[foo bar baz] + + expect(to_exclusive_sentence(array)).to eq('foo, bar, or baz') + end + + it 'localizes the connector words' do + array = %w[foo bar baz] + + expect(described_class).to receive(:_).with(' or ').and_return(' <1> ') + expect(described_class).to receive(:_).with(', or ').and_return(', <2> ') + expect(to_exclusive_sentence(array)).to eq('foo, bar, <2> baz') + end + end +end diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index 102d608072b..7b9504366ec 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -6,139 +6,9 @@ RSpec.describe Gitlab::Utils do using RSpec::Parameterized::TableSyntax delegate :to_boolean, :boolean_to_yes_no, :slugify, :which, - :ensure_array_from_string, :to_exclusive_sentence, :bytes_to_megabytes, - :append_path, :remove_leading_slashes, :check_path_traversal!, :allowlisted?, :check_allowed_absolute_path!, - :decode_path, :ms_to_round_sec, :check_allowed_absolute_path_and_path_traversal!, to: :described_class - - describe '.check_path_traversal!' do - it 'detects path traversal in string without any separators' do - expect { check_path_traversal!('.') }.to raise_error(/Invalid path/) - expect { check_path_traversal!('..') }.to raise_error(/Invalid path/) - end - - it 'detects path traversal at the start of the string' do - expect { check_path_traversal!('../foo') }.to raise_error(/Invalid path/) - expect { check_path_traversal!('..\\foo') }.to raise_error(/Invalid path/) - end - - it 'detects path traversal at the start of the string, even to just the subdirectory' do - expect { check_path_traversal!('../') }.to raise_error(/Invalid path/) - expect { check_path_traversal!('..\\') }.to raise_error(/Invalid path/) - expect { check_path_traversal!('/../') }.to raise_error(/Invalid path/) - expect { check_path_traversal!('\\..\\') }.to raise_error(/Invalid path/) - end - - it 'detects path traversal in the middle of the string' do - expect { check_path_traversal!('foo/../../bar') }.to raise_error(/Invalid path/) - expect { check_path_traversal!('foo\\..\\..\\bar') }.to raise_error(/Invalid path/) - expect { check_path_traversal!('foo/..\\bar') }.to raise_error(/Invalid path/) - expect { check_path_traversal!('foo\\../bar') }.to raise_error(/Invalid path/) - expect { check_path_traversal!('foo/..\\..\\..\\..\\../bar') }.to raise_error(/Invalid path/) - end - - it 'detects path traversal at the end of the string when slash-terminates' do - expect { check_path_traversal!('foo/../') }.to raise_error(/Invalid path/) - expect { check_path_traversal!('foo\\..\\') }.to raise_error(/Invalid path/) - end - - it 'detects path traversal at the end of the string' do - expect { check_path_traversal!('foo/..') }.to raise_error(/Invalid path/) - expect { check_path_traversal!('foo\\..') }.to raise_error(/Invalid path/) - end - - it 'does nothing for a safe string' do - expect(check_path_traversal!('./foo')).to eq('./foo') - expect(check_path_traversal!('.test/foo')).to eq('.test/foo') - expect(check_path_traversal!('..test/foo')).to eq('..test/foo') - expect(check_path_traversal!('dir/..foo.rb')).to eq('dir/..foo.rb') - expect(check_path_traversal!('dir/.foo.rb')).to eq('dir/.foo.rb') - end - - it 'logs potential path traversal attempts' do - expect(Gitlab::AppLogger).to receive(:warn).with(message: "Potential path traversal attempt detected", path: "..") - expect { check_path_traversal!('..') }.to raise_error(/Invalid path/) - end - - it 'logs does nothing for a safe string' do - expect(Gitlab::AppLogger).not_to receive(:warn).with(message: "Potential path traversal attempt detected", path: "dir/.foo.rb") - expect(check_path_traversal!('dir/.foo.rb')).to eq('dir/.foo.rb') - end - - it 'does nothing for nil' do - expect(check_path_traversal!(nil)).to be_nil - end - - it 'does nothing for safe HashedPath' do - expect(check_path_traversal!(Gitlab::HashedPath.new('tmp', root_hash: 1))).to eq '6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b/tmp' - end - - it 'raises for unsafe HashedPath' do - expect { check_path_traversal!(Gitlab::HashedPath.new('tmp', '..', 'etc', 'passwd', root_hash: 1)) }.to raise_error(/Invalid path/) - end - - it 'raises for other non-strings' do - expect { check_path_traversal!(%w[/tmp /tmp/../etc/passwd]) }.to raise_error(/Invalid path/) - end - end - - describe '.check_allowed_absolute_path_and_path_traversal!' do - let(:allowed_paths) { %w[/home/foo ./foo .test/foo ..test/foo dir/..foo.rb dir/.foo.rb] } - - it 'detects path traversal in string without any separators' do - expect { check_allowed_absolute_path_and_path_traversal!('.', allowed_paths) }.to raise_error(/Invalid path/) - expect { check_allowed_absolute_path_and_path_traversal!('..', allowed_paths) }.to raise_error(/Invalid path/) - end - - it 'detects path traversal at the start of the string' do - expect { check_allowed_absolute_path_and_path_traversal!('../foo', allowed_paths) }.to raise_error(/Invalid path/) - expect { check_allowed_absolute_path_and_path_traversal!('..\\foo', allowed_paths) }.to raise_error(/Invalid path/) - end - - it 'detects path traversal at the start of the string, even to just the subdirectory' do - expect { check_allowed_absolute_path_and_path_traversal!('../', allowed_paths) }.to raise_error(/Invalid path/) - expect { check_allowed_absolute_path_and_path_traversal!('..\\', allowed_paths) }.to raise_error(/Invalid path/) - expect { check_allowed_absolute_path_and_path_traversal!('/../', allowed_paths) }.to raise_error(/Invalid path/) - expect { check_allowed_absolute_path_and_path_traversal!('\\..\\', allowed_paths) }.to raise_error(/Invalid path/) - end - - it 'detects path traversal in the middle of the string' do - expect { check_allowed_absolute_path_and_path_traversal!('foo/../../bar', allowed_paths) }.to raise_error(/Invalid path/) - expect { check_allowed_absolute_path_and_path_traversal!('foo\\..\\..\\bar', allowed_paths) }.to raise_error(/Invalid path/) - expect { check_allowed_absolute_path_and_path_traversal!('foo/..\\bar', allowed_paths) }.to raise_error(/Invalid path/) - expect { check_allowed_absolute_path_and_path_traversal!('foo\\../bar', allowed_paths) }.to raise_error(/Invalid path/) - expect { check_allowed_absolute_path_and_path_traversal!('foo/..\\..\\..\\..\\../bar', allowed_paths) }.to raise_error(/Invalid path/) - end - - it 'detects path traversal at the end of the string when slash-terminates' do - expect { check_allowed_absolute_path_and_path_traversal!('foo/../', allowed_paths) }.to raise_error(/Invalid path/) - expect { check_allowed_absolute_path_and_path_traversal!('foo\\..\\', allowed_paths) }.to raise_error(/Invalid path/) - end - - it 'detects path traversal at the end of the string' do - expect { check_allowed_absolute_path_and_path_traversal!('foo/..', allowed_paths) }.to raise_error(/Invalid path/) - expect { check_allowed_absolute_path_and_path_traversal!('foo\\..', allowed_paths) }.to raise_error(/Invalid path/) - end - - it 'does not return errors for a safe string' do - expect(check_allowed_absolute_path_and_path_traversal!('./foo', allowed_paths)).to be_nil - expect(check_allowed_absolute_path_and_path_traversal!('.test/foo', allowed_paths)).to be_nil - expect(check_allowed_absolute_path_and_path_traversal!('..test/foo', allowed_paths)).to be_nil - expect(check_allowed_absolute_path_and_path_traversal!('dir/..foo.rb', allowed_paths)).to be_nil - expect(check_allowed_absolute_path_and_path_traversal!('dir/.foo.rb', allowed_paths)).to be_nil - end - - it 'raises error for a non-string' do - expect { check_allowed_absolute_path_and_path_traversal!(nil, allowed_paths) }.to raise_error(StandardError) - end - - it 'raises an exception if an absolute path is not allowed' do - expect { check_allowed_absolute_path!('/etc/passwd', allowed_paths) }.to raise_error(StandardError) - end - - it 'does nothing for an allowed absolute path' do - expect(check_allowed_absolute_path!('/home/foo', allowed_paths)).to be_nil - end - end + :ensure_array_from_string, :bytes_to_megabytes, + :append_path, :remove_leading_slashes, :allowlisted?, + :decode_path, :ms_to_round_sec, to: :described_class describe '.allowlisted?' do let(:allowed_paths) { ['/home/foo', '/foo/bar', '/etc/passwd'] } @@ -152,18 +22,6 @@ RSpec.describe Gitlab::Utils do end end - describe '.check_allowed_absolute_path!' do - let(:allowed_paths) { ['/home/foo'] } - - it 'raises an exception if an absolute path is not allowed' do - expect { check_allowed_absolute_path!('/etc/passwd', allowed_paths) }.to raise_error(StandardError) - end - - it 'does nothing for an allowed absolute path' do - expect(check_allowed_absolute_path!('/home/foo', allowed_paths)).to be_nil - end - end - describe '.decode_path' do it 'returns path unencoded for singled-encoded paths' do expect(decode_path('%2Fhome%2Fbar%3Fasd%3Dqwe')).to eq('/home/bar?asd=qwe') @@ -212,36 +70,6 @@ RSpec.describe Gitlab::Utils do end end - describe '.to_exclusive_sentence' do - it 'calls #to_sentence on the array' do - array = double - - expect(array).to receive(:to_sentence) - - to_exclusive_sentence(array) - end - - it 'joins arrays with two elements correctly' do - array = %w(foo bar) - - expect(to_exclusive_sentence(array)).to eq('foo or bar') - end - - it 'joins arrays with more than two elements correctly' do - array = %w(foo bar baz) - - expect(to_exclusive_sentence(array)).to eq('foo, bar, or baz') - end - - it 'localizes the connector words' do - array = %w(foo bar baz) - - expect(described_class).to receive(:_).with(' or ').and_return(' <1> ') - expect(described_class).to receive(:_).with(', or ').and_return(', <2> ') - expect(to_exclusive_sentence(array)).to eq('foo, bar, <2> baz') - end - end - describe '.nlbr' do it 'replaces new lines with <br>' do expect(described_class.nlbr("<b>hello</b>\n<i>world</i>")).to eq("hello<br>world") diff --git a/spec/lib/sidebars/projects/menus/merge_requests_menu_spec.rb b/spec/lib/sidebars/projects/menus/merge_requests_menu_spec.rb index 08f35b6acd0..ecc1f6c09f1 100644 --- a/spec/lib/sidebars/projects/menus/merge_requests_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/merge_requests_menu_spec.rb @@ -73,11 +73,14 @@ RSpec.describe Sidebars::Projects::Menus::MergeRequestsMenu, feature_category: : end describe 'formatting' do - it 'returns truncated digits for count value over 1000' do - create_list(:merge_request, 1001, :unique_branches, source_project: project, author: user, state: :opened) - create(:merge_request, source_project: project, state: :merged) + context 'when the count value is over 1000' do + before do + allow(context).to receive(:project).and_return(instance_double(Project, open_merge_requests_count: 1001)) + end - expect(subject.pill_count).to eq('1k') + it 'returns truncated digits' do + expect(subject.pill_count).to eq('1k') + end end end end diff --git a/spec/models/concerns/has_user_type_spec.rb b/spec/models/concerns/has_user_type_spec.rb index b5abd114f9a..f9bf576d75b 100644 --- a/spec/models/concerns/has_user_type_spec.rb +++ b/spec/models/concerns/has_user_type_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe User, feature_category: :system_access do specify 'types consistency checks', :aggregate_failures do expect(described_class::USER_TYPES.keys) - .to match_array(%w[human human_deprecated ghost alert_bot project_bot support_bot service_user security_bot + .to match_array(%w[human ghost alert_bot project_bot support_bot service_user security_bot visual_review_bot migration_bot automation_bot security_policy_bot admin_bot suggested_reviewers_bot service_account llm_bot]) expect(described_class::USER_TYPES).to include(*described_class::BOT_USER_TYPES) @@ -13,6 +13,12 @@ RSpec.describe User, feature_category: :system_access do expect(described_class::USER_TYPES).to include(*described_class::INTERNAL_USER_TYPES) end + describe 'validations' do + it 'validates type presence' do + expect(User.new).to validate_presence_of(:user_type) + end + end + describe 'scopes & predicates' do User::USER_TYPES.keys.each do |type| # rubocop:disable RSpec/UselessDynamicDefinition let_it_be(type) { create(:user, username: type, user_type: type) } @@ -21,18 +27,6 @@ RSpec.describe User, feature_category: :system_access do let(:non_internal) { User::NON_INTERNAL_USER_TYPES.map { |type| public_send(type) } } let(:everyone) { User::USER_TYPES.keys.map { |type| public_send(type) } } - describe '.humans' do - it 'includes humans only' do - expect(described_class.humans).to match_array([human, human_deprecated]) - end - end - - describe '.human' do - it 'includes humans only' do - expect(described_class.human).to match_array([human, human_deprecated]) - end - end - describe '.bots' do it 'includes all bots' do expect(described_class.bots).to match_array(bots) @@ -73,15 +67,6 @@ RSpec.describe User, feature_category: :system_access do end end - describe '#human?' do - it 'is true for humans only' do - expect(human).to be_human - expect(human_deprecated).to be_human - expect(alert_bot).not_to be_human - expect(User.new).to be_human - end - end - describe '#internal?' do it 'is true for all internal user types and false for others' do expect(everyone - non_internal).to all(be_internal) diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 1ecc4356672..e2c87b0d85c 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -1274,7 +1274,7 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do it 'raises' do allow(diff).to receive(:external_diff_cache_dir).and_return(File.join(cache_dir, '..')) - expect { diff.remove_cached_external_diff }.to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path') + expect { diff.remove_cached_external_diff }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path') end end diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index e5f03d16dda..b07296a0df2 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -323,6 +323,62 @@ RSpec.describe 'Git LFS API and storage', feature_category: :source_code_managem it_behaves_like 'process authorization header', renew_authorization: renew_authorization end + + context 'when downloading an LFS object that is stored on object storage' do + before do + stub_lfs_object_storage + lfs_object.file.migrate!(LfsObjectUploader::Store::REMOTE) + end + + context 'when lfs.object_store.proxy_download=true' do + before do + stub_lfs_object_storage(proxy_download: true) + end + + it_behaves_like 'LFS http 200 response' + + it 'does return proxied address URL' do + expect(json_response['objects'].first).to include(sample_object) + expect(json_response['objects'].first['actions']['download']['href']).to eq(objects_url(project, sample_oid)) + end + end + + context 'when "lfs.object_store.proxy_download" is "false"' do + before do + stub_lfs_object_storage(proxy_download: false) + end + + it_behaves_like 'LFS http 200 response' + + it 'does return direct object storage URL' do + expect(json_response['objects'].first).to include(sample_object) + expect(json_response['objects'].first['actions']['download']['href']).to start_with("https://lfs-objects.s3.amazonaws.com/") + expect(json_response['objects'].first['actions']['download']['href']).to include("X-Amz-Expires=3600&") + end + + context 'when feature flag "lfs_batch_direct_downloads" is "false"' do + before do + stub_feature_flags(lfs_batch_direct_downloads: false) + end + + it_behaves_like 'LFS http 200 response' + + it 'does return proxied address URL' do + expect(json_response['objects'].first).to include(sample_object) + expect(json_response['objects'].first['actions']['download']['href']).to eq(objects_url(project, sample_oid)) + end + end + end + end + + context 'when sending objects=[]' do + let(:body) { download_body([]) } + + it_behaves_like 'LFS http expected response code and message' do + let(:response_code) { 404 } + let(:message) { 'Not found.' } + end + end end context 'when user is authenticated' do diff --git a/spec/requests/projects/work_items_spec.rb b/spec/requests/projects/work_items_spec.rb index c02f76d2c65..ee9a0ff0a4e 100644 --- a/spec/requests/projects/work_items_spec.rb +++ b/spec/requests/projects/work_items_spec.rb @@ -27,8 +27,8 @@ RSpec.describe 'Work Items', feature_category: :team_planning do shared_examples 'safely handles uploaded files' do it 'ensures the upload is handled safely', :aggregate_failures do - allow(Gitlab::Utils).to receive(:check_path_traversal!).and_call_original - expect(Gitlab::Utils).to receive(:check_path_traversal!).with(filename).at_least(:once) + allow(Gitlab::PathTraversal).to receive(:check_path_traversal!).and_call_original + expect(Gitlab::PathTraversal).to receive(:check_path_traversal!).with(filename).at_least(:once) expect(FileUploader).not_to receive(:cache) subject diff --git a/spec/services/bulk_imports/archive_extraction_service_spec.rb b/spec/services/bulk_imports/archive_extraction_service_spec.rb index 40f8d8718ae..5593218c259 100644 --- a/spec/services/bulk_imports/archive_extraction_service_spec.rb +++ b/spec/services/bulk_imports/archive_extraction_service_spec.rb @@ -53,7 +53,7 @@ RSpec.describe BulkImports::ArchiveExtractionService, feature_category: :importe context 'when filepath is being traversed' do it 'raises an error' do expect { described_class.new(tmpdir: File.join(Dir.mktmpdir, 'test', '..'), filename: 'name').execute } - .to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path') + .to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path') end end end diff --git a/spec/services/bulk_imports/file_decompression_service_spec.rb b/spec/services/bulk_imports/file_decompression_service_spec.rb index 9b8320aeac5..9d80ab3cd8f 100644 --- a/spec/services/bulk_imports/file_decompression_service_spec.rb +++ b/spec/services/bulk_imports/file_decompression_service_spec.rb @@ -66,7 +66,7 @@ RSpec.describe BulkImports::FileDecompressionService, feature_category: :importe subject { described_class.new(tmpdir: File.join(Dir.mktmpdir, 'test', '..'), filename: 'filename') } it 'raises an error' do - expect { subject.execute }.to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path') + expect { subject.execute }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path') end end diff --git a/spec/services/bulk_imports/file_download_service_spec.rb b/spec/services/bulk_imports/file_download_service_spec.rb index 7c64d6efc65..4aa9cf3a73d 100644 --- a/spec/services/bulk_imports/file_download_service_spec.rb +++ b/spec/services/bulk_imports/file_download_service_spec.rb @@ -281,7 +281,7 @@ RSpec.describe BulkImports::FileDownloadService, feature_category: :importers do it 'raises an error' do expect { subject.execute }.to raise_error( - Gitlab::Utils::PathTraversalAttackError, + Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path' ) end diff --git a/spec/services/projects/readme_renderer_service_spec.rb b/spec/services/projects/readme_renderer_service_spec.rb index 4d599a26c22..cced9e52227 100644 --- a/spec/services/projects/readme_renderer_service_spec.rb +++ b/spec/services/projects/readme_renderer_service_spec.rb @@ -52,7 +52,7 @@ RSpec.describe Projects::ReadmeRendererService, '#execute', feature_category: :g context 'with path traversal in mind' do where(:template_name, :exception, :expected_path) do - '../path/traversal/bad' | [Gitlab::Utils::PathTraversalAttackError, 'Invalid path'] | nil + '../path/traversal/bad' | [Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path'] | nil '/bad/template' | [StandardError, 'path /bad/template.md.tt is not allowed'] | nil 'good/template' | nil | 'good/template.md.tt' end diff --git a/spec/support/helpers/search_helpers.rb b/spec/support/helpers/search_helpers.rb index 30ffe3d6228..75853371c0f 100644 --- a/spec/support/helpers/search_helpers.rb +++ b/spec/support/helpers/search_helpers.rb @@ -35,8 +35,6 @@ module SearchHelpers end def has_search_scope?(scope) - return false unless page.has_selector?('[data-testid="search-filter"]') - page.within '[data-testid="search-filter"]' do has_link?(scope) end diff --git a/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb b/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb index 7126d3ace96..a7e5892d439 100644 --- a/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb +++ b/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb @@ -63,8 +63,8 @@ RSpec.shared_examples "builds correct paths" do |**patterns| end it "throws an exception" do - expect { subject.cache!(fixture_file_upload(fixture)) }.to raise_error(Gitlab::Utils::PathTraversalAttackError) - expect { subject.store!(fixture_file_upload(fixture)) }.to raise_error(Gitlab::Utils::PathTraversalAttackError) + expect { subject.cache!(fixture_file_upload(fixture)) }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError) + expect { subject.store!(fixture_file_upload(fixture)) }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError) end end end diff --git a/spec/views/admin/sessions/new.html.haml_spec.rb b/spec/views/admin/sessions/new.html.haml_spec.rb index c1f4cafce0c..18a673b1c31 100644 --- a/spec/views/admin/sessions/new.html.haml_spec.rb +++ b/spec/views/admin/sessions/new.html.haml_spec.rb @@ -62,7 +62,7 @@ RSpec.describe 'admin/sessions/new.html.haml' do expect(rendered).to have_selector('[data-testid="ldap-tab"]') expect(rendered).to have_css('.login-box#ldapmain') - expect(rendered).to have_field('LDAP Username') + expect(rendered).to have_field(_('Username')) expect(rendered).not_to have_content('No authentication methods configured') end @@ -72,7 +72,7 @@ RSpec.describe 'admin/sessions/new.html.haml' do render expect(rendered).not_to have_selector('[data-testid="ldap-tab"]') - expect(rendered).not_to have_field('LDAP Username') + expect(rendered).not_to have_field(_('Username')) expect(rendered).to have_content('No authentication methods configured') end diff --git a/spec/views/devise/sessions/new.html.haml_spec.rb b/spec/views/devise/sessions/new.html.haml_spec.rb index 8de2eab36e9..70ca0bb2195 100644 --- a/spec/views/devise/sessions/new.html.haml_spec.rb +++ b/spec/views/devise/sessions/new.html.haml_spec.rb @@ -56,7 +56,7 @@ RSpec.describe 'devise/sessions/new' do expect(rendered).to have_selector('.new-session-tabs') expect(rendered).to have_selector('[data-testid="ldap-tab"]') - expect(rendered).to have_field('LDAP Username') + expect(rendered).to have_field(_('Username')) end it 'is not shown when LDAP sign in is disabled' do @@ -66,7 +66,7 @@ RSpec.describe 'devise/sessions/new' do expect(rendered).to have_content('No authentication methods configured') expect(rendered).not_to have_selector('[data-testid="ldap-tab"]') - expect(rendered).not_to have_field('LDAP Username') + expect(rendered).not_to have_field(_('Username')) end end diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index 8a0e05e6fb6..07a51e92c98 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -408,6 +408,7 @@ RSpec.describe 'Every Sidekiq worker', feature_category: :shared do 'PostReceive' => 3, 'ProcessCommitWorker' => 3, 'ProductAnalytics::InitializeAnalyticsWorker' => 3, + 'ProductAnalytics::InitializeSnowplowProductAnalyticsWorker' => 1, 'ProjectCacheWorker' => 3, 'ProjectDestroyWorker' => 3, 'ProjectExportWorker' => false, diff --git a/spec/workers/update_highest_role_worker_spec.rb b/spec/workers/update_highest_role_worker_spec.rb index 3e4a2f6be36..d0a7a1e3a40 100644 --- a/spec/workers/update_highest_role_worker_spec.rb +++ b/spec/workers/update_highest_role_worker_spec.rb @@ -18,7 +18,7 @@ RSpec.describe UpdateHighestRoleWorker, :clean_gitlab_redis_shared_state, featur let(:active_attributes) do { state: 'active', - user_type: nil + user_type: :human } end diff --git a/spec/workers/users/deactivate_dormant_users_worker_spec.rb b/spec/workers/users/deactivate_dormant_users_worker_spec.rb index fdcbb624562..39d282a6e18 100644 --- a/spec/workers/users/deactivate_dormant_users_worker_spec.rb +++ b/spec/workers/users/deactivate_dormant_users_worker_spec.rb @@ -35,7 +35,6 @@ RSpec.describe Users::DeactivateDormantUsersWorker, feature_category: :seat_cost where(:user_type, :expected_state) do :human | 'deactivated' - :human_deprecated | 'deactivated' :support_bot | 'active' :alert_bot | 'active' :visual_review_bot | 'active' @@ -58,13 +57,11 @@ RSpec.describe Users::DeactivateDormantUsersWorker, feature_category: :seat_cost it 'does not deactivate non-active users' do human_user = create(:user, user_type: :human, state: :blocked, last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date) - human_user2 = create(:user, user_type: :human_deprecated, state: :blocked, last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date) service_user = create(:user, user_type: :service_user, state: :blocked, last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date) worker.perform expect(human_user.reload.state).to eq('blocked') - expect(human_user2.reload.state).to eq('blocked') expect(service_user.reload.state).to eq('blocked') end |