diff --git a/.rubocop_todo/rspec/instance_variable.yml b/.rubocop_todo/rspec/instance_variable.yml index 2c8b8e15eed..193eae8e754 100644 --- a/.rubocop_todo/rspec/instance_variable.yml +++ b/.rubocop_todo/rspec/instance_variable.yml @@ -102,7 +102,6 @@ RSpec/InstanceVariable: - 'spec/rack_servers/puma_spec.rb' - 'spec/requests/api/admin/plan_limits_spec.rb' - 'spec/requests/api/users_spec.rb' - - 'spec/requests/git_http_spec.rb' - 'spec/requests/openid_connect_spec.rb' - 'spec/requests/projects/issues/discussions_spec.rb' - 'spec/services/ci/process_sync_events_service_spec.rb' diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e43218bfc0..bfea1d298e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,27 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 17.6.2 (2024-12-10) + +### Fixed (2 changes) + +- [Add guard clause to Wiki#find_page when title is nil](https://gitlab.com/gitlab-org/security/gitlab/-/commit/1be99d9925c659f168dccb4b2cfb3510ac74e7ed) +- [Fix 401 errors when installing the GitLab for Jira app](https://gitlab.com/gitlab-org/security/gitlab/-/commit/8e15de4128733083fe3bf640751aecf95d5471a7) + +### Security (11 changes) + +- [Add timeout around Parslet in template parser](https://gitlab.com/gitlab-org/security/gitlab/-/commit/74de080527cf262ecec44e97c78705953cfa1cdc) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4653)) +- [Add authorization check to protectableBranches field](https://gitlab.com/gitlab-org/security/gitlab/-/commit/16152cf39642bd4dc9ed023d52493c9522ef87f2) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4652)) +- [Check harbor name & digest for path traversal](https://gitlab.com/gitlab-org/security/gitlab/-/commit/734520792bc637580fd79ce2d368268501382d76) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4629)) +- [Ignore titles for GFM links in rich text editor](https://gitlab.com/gitlab-org/security/gitlab/-/commit/769b309ded5f3fca7f550ef9972750cd60298b73) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4649)) +- [Restrict user and group creation when same pages unique domain exist](https://gitlab.com/gitlab-org/security/gitlab/-/commit/09997ce510251b8f58343464143e40f1f5ed00c2) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4618)) +- [DoS by repeatedly sending unauthenticated requests for diff-files of a commit or merge request](https://gitlab.com/gitlab-org/security/gitlab/-/commit/c0045078225c4b64fa1dd2582c246df5b7b4a96a) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4639)) +- [Add query to filter_parameters](https://gitlab.com/gitlab-org/security/gitlab/-/commit/32485a34d6f3ee22fdbe20d0a41cd6b10f0cd511) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4625)) +- [Added invalid redirect fragment check](https://gitlab.com/gitlab-org/security/gitlab/-/commit/5c69fef592ceab17eaeda04fd78e120116229b03) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4609)) +- [Make confidential threads unresolvable via new issue](https://gitlab.com/gitlab-org/security/gitlab/-/commit/1396d48051a02153a9bd064d39d2d5c09233f3c6) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4633)) +- [Do not set session cookie for /v2 endpoints in the response](https://gitlab.com/gitlab-org/security/gitlab/-/commit/3305b0fafe245a02fa01a5b882e8ad5b565f8736) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4630)) +- [HTML injection in vulnerability details, leads to XSS on self hosted servers](https://gitlab.com/gitlab-org/security/gitlab/-/commit/4284532cd6ae8f0166806a81628887f82756ceef) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4619)) + ## 17.6.1 (2024-11-26) ### Security (6 changes) @@ -995,6 +1016,26 @@ entry. - [Quarantine a flaky test](https://gitlab.com/gitlab-org/gitlab/-/commit/7427f68ca476bd1294900155a2a93b470ef888a6) ([merge request](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165742)) - [Quarantine a flaky test](https://gitlab.com/gitlab-org/gitlab/-/commit/81ccade46593d99c938fd8ab03c2e299f6f62377) ([merge request](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/164711)) +## 17.5.4 (2024-12-10) + +### Fixed (1 change) + +- [Fix 401 errors when installing the GitLab for Jira app](https://gitlab.com/gitlab-org/security/gitlab/-/commit/5499b8941f6d0dec42bbd7469ca806890edae35e) + +### Security (11 changes) + +- [Add timeout around Parslet in template parser](https://gitlab.com/gitlab-org/security/gitlab/-/commit/b9ce9e051da449add787b16f7cf2d08f8eb11115) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4654)) +- [Add authorization check to protectableBranches field](https://gitlab.com/gitlab-org/security/gitlab/-/commit/3f870e741e15034bca056fba125a0badbbe264bf) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4595)) +- [Check harbor name & digest for path traversal](https://gitlab.com/gitlab-org/security/gitlab/-/commit/2257cdf16e6ddbfdfddbbecd694e30589581be4e) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4628)) +- [Ignore titles for GFM links in rich text editor](https://gitlab.com/gitlab-org/security/gitlab/-/commit/2215af32dfa6074844e4b39a5ce12dc8b2590d09) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4650)) +- [Restrict user and group creation when same pages unique domain exist](https://gitlab.com/gitlab-org/security/gitlab/-/commit/c7c6fbba10470644b4d532b3ba1aa00240bde391) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4576)) +- [DoS by repeatedly sending unauthenticated requests for diff-files of a commit or merge request](https://gitlab.com/gitlab-org/security/gitlab/-/commit/8f0c1b73b4e2584aba7866653828b15283d10a90) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4638)) +- [Add query to filter_parameters](https://gitlab.com/gitlab-org/security/gitlab/-/commit/707d7792996ebe8e4c8da2a587810e3339432352) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4626)) +- [Added invalid redirect fragment check](https://gitlab.com/gitlab-org/security/gitlab/-/commit/e2760b5a3425f50c3444ff264d4e3381f11894ea) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4605)) +- [Make confidential threads unresolvable via new issue](https://gitlab.com/gitlab-org/security/gitlab/-/commit/a7ff5a159f7d699eec9e9844e5ab0727219ecb91) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4634)) +- [Do not set session cookie for /v2 endpoints in the response](https://gitlab.com/gitlab-org/security/gitlab/-/commit/542c5b0dbc4744dab0d89bc42b34bfe16e760e54) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4631)) +- [HTML injection in vulnerability details, leads to XSS on self hosted servers](https://gitlab.com/gitlab-org/security/gitlab/-/commit/f7e572e94c2360b93fe6e04a65b9874975382693) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4553)) + ## 17.5.3 (2024-11-26) ### Fixed (1 change) @@ -1764,6 +1805,27 @@ entry. - [Adjust signup page items for more clarity](https://gitlab.com/gitlab-org/gitlab/-/commit/e272c8a4c7b243758454d6f15363d0c13ca05c04) ([merge request](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165202)) **GitLab Enterprise Edition** - [Removes Unused CSS class](https://gitlab.com/gitlab-org/gitlab/-/commit/4e17154650ee4afc8b1ae4238d27efb908855a19) by @NIKU-SINGH ([merge request](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/164637)) +## 17.4.6 (2024-12-10) + +### Fixed (2 changes) + +- [Add param filtering to avoid error while saving project settings](https://gitlab.com/gitlab-org/security/gitlab/-/commit/4787ee4000679f645aa1eaa1f1d07bfc34c461cd) ([merge request](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/173428)) **GitLab Enterprise Edition** +- [Fix 401 errors when installing the GitLab for Jira app](https://gitlab.com/gitlab-org/security/gitlab/-/commit/601e8e20637690102b5118d638e290f68f79fb43) + +### Security (11 changes) + +- [Add timeout around Parslet in template parser](https://gitlab.com/gitlab-org/security/gitlab/-/commit/f974f850463f267b5a636f28c99cac61c4ef6259) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4655)) +- [Add authorization check to protectableBranches field](https://gitlab.com/gitlab-org/security/gitlab/-/commit/e6a47ce0dbdc4da3e8838451194203709c56fc5d) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4596)) +- [Check harbor name & digest for path traversal](https://gitlab.com/gitlab-org/security/gitlab/-/commit/cb40c0144b6bf27b49a7745d61fcf37dbe84e8d2) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4642)) +- [Ignore titles for GFM links in rich text editor](https://gitlab.com/gitlab-org/security/gitlab/-/commit/551e6018a99c91918f0f9a2f177ee237ae897246) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4651)) +- [Restrict user and group creation when same pages unique domain exist](https://gitlab.com/gitlab-org/security/gitlab/-/commit/495025a35f59b39fcfb6a49077a067c246f9fe06) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4577)) +- [DoS by repeatedly sending unauthenticated requests for diff-files of a commit or merge request](https://gitlab.com/gitlab-org/security/gitlab/-/commit/01fa899f15e792ce2c54dae3d3db85cb00a49789) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4637)) +- [Add query to filter_parameters](https://gitlab.com/gitlab-org/security/gitlab/-/commit/322db9627a33a74d73e48ef05d87269191328346) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4627)) +- [Added invalid redirect fragment check](https://gitlab.com/gitlab-org/security/gitlab/-/commit/f690a49166c32965403070699436d8328768cd69) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4606)) +- [Make confidential threads unresolvable via new issue](https://gitlab.com/gitlab-org/security/gitlab/-/commit/b055634ab615a20599b0403570b5a8b27b812ec2) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4635)) +- [Do not set session cookie for /v2 endpoints in the response](https://gitlab.com/gitlab-org/security/gitlab/-/commit/d6dd0f12d146021074a4a36412b6e3cae9782001) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4632)) +- [HTML injection in vulnerability details, leads to XSS on self hosted servers](https://gitlab.com/gitlab-org/security/gitlab/-/commit/7a6bd953a1f70b58b2fd48d58431fadb9e8249f8) ([merge request](https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4516)) + ## 17.4.5 (2024-11-26) ### Security (6 changes) diff --git a/GITLAB_KAS_VERSION b/GITLAB_KAS_VERSION index 9dd109fd77e..44911902130 100644 --- a/GITLAB_KAS_VERSION +++ b/GITLAB_KAS_VERSION @@ -1 +1 @@ -d3839f7e054c608c98399cff2b4c07aa18f102ed +351ea2b5585c837394a616f08fa16bdbb28808aa diff --git a/app/assets/javascripts/content_editor/components/bubble_menus/media_bubble_menu.vue b/app/assets/javascripts/content_editor/components/bubble_menus/media_bubble_menu.vue index ab28b55b854..7cda4e3e405 100644 --- a/app/assets/javascripts/content_editor/components/bubble_menus/media_bubble_menu.vue +++ b/app/assets/javascripts/content_editor/components/bubble_menus/media_bubble_menu.vue @@ -35,11 +35,6 @@ export default { [Image.name]: __('Edit image description'), [Video.name]: __('Edit video description'), }, - replaceLabels: { - [Audio.name]: __('Replace audio'), - [Image.name]: __('Replace image'), - [Video.name]: __('Replace video'), - }, deleteLabels: { [Audio.name]: __('Delete audio'), [DrawioDiagram.name]: __('Delete diagram'), @@ -85,9 +80,6 @@ export default { editLabel() { return this.$options.i18n.editLabels[this.mediaType]; }, - replaceLabel() { - return this.$options.i18n.replaceLabels[this.mediaType]; - }, deleteLabel() { return this.$options.i18n.deleteLabels[this.mediaType]; }, @@ -178,10 +170,6 @@ export default { this.uploadProgress = 0; }, - replaceMedia() { - this.$refs.fileSelector.click(); - }, - editDiagram() { this.tiptapEditor.chain().focus().createOrEditDiagram().run(); }, @@ -275,18 +263,6 @@ export default { icon="diagram" @click="editDiagram" /> - diff --git a/app/assets/javascripts/content_editor/extensions/link.js b/app/assets/javascripts/content_editor/extensions/link.js index 166db64eaa5..5b13e359012 100644 --- a/app/assets/javascripts/content_editor/extensions/link.js +++ b/app/assets/javascripts/content_editor/extensions/link.js @@ -50,7 +50,8 @@ export default Link.extend({ }, title: { title: null, - parseHTML: (element) => element.getAttribute('title'), + parseHTML: (element) => + element.classList.contains('gfm') ? null : element.getAttribute('title'), }, // only for gollum links (wikis) isGollumLink: { diff --git a/app/assets/javascripts/lib/apollo/sentry_breadcrumb_link.js b/app/assets/javascripts/lib/apollo/sentry_breadcrumb_link.js new file mode 100644 index 00000000000..cdbb1ce2834 --- /dev/null +++ b/app/assets/javascripts/lib/apollo/sentry_breadcrumb_link.js @@ -0,0 +1,33 @@ +import { ApolloLink } from '@apollo/client/core'; +import * as Sentry from '~/sentry/sentry_browser_wrapper'; + +const REQUEST = 'graphql.request'; +const RESPONSE = 'graphql.response'; + +const addGraphqlBreadcrumb = (category, data = {}) => { + Sentry.addBreadcrumb({ + level: 'info', + category, + data, + }); +}; + +/** + * An ApolloLink that sets a Sentry breadcrumb to make the GraphQL operation name + * visible in a Sentry event report. + * + * @see https://develop.sentry.dev/sdk/data-model/event-payloads/breadcrumbs/ + * @see https://www.apollographql.com/docs/react/api/link/introduction + */ +export const sentryBreadcrumbLink = new ApolloLink((operation, forward) => { + addGraphqlBreadcrumb(REQUEST, { operationName: operation.operationName }); + + return forward(operation).map((response) => { + addGraphqlBreadcrumb(RESPONSE, { + correlationId: response.correlationId, + operationName: operation.operationName, + }); + + return response; + }); +}); diff --git a/app/assets/javascripts/lib/graphql.js b/app/assets/javascripts/lib/graphql.js index d3e2d418579..53f02d06160 100644 --- a/app/assets/javascripts/lib/graphql.js +++ b/app/assets/javascripts/lib/graphql.js @@ -13,6 +13,7 @@ import { getInstrumentationLink } from './apollo/instrumentation_link'; import { getSuppressNetworkErrorsDuringNavigationLink } from './apollo/suppress_network_errors_during_navigation_link'; import { getPersistLink } from './apollo/persist_link'; import { persistenceMapper } from './apollo/persistence_mapper'; +import { sentryBreadcrumbLink } from './apollo/sentry_breadcrumb_link'; import { correlationIdLink } from './apollo/correlation_id_link'; export const fetchPolicies = { @@ -253,6 +254,7 @@ function createApolloClient(resolvers = {}, config = {}) { [ getSuppressNetworkErrorsDuringNavigationLink(), getInstrumentationLink(), + sentryBreadcrumbLink, correlationIdLink, requestCounterLink, performanceBarLink, diff --git a/app/assets/javascripts/ml/model_registry/apps/show_ml_model_version.vue b/app/assets/javascripts/ml/model_registry/apps/show_ml_model_version.vue index 932e77f0cce..3e8a3baafee 100644 --- a/app/assets/javascripts/ml/model_registry/apps/show_ml_model_version.vue +++ b/app/assets/javascripts/ml/model_registry/apps/show_ml_model_version.vue @@ -4,7 +4,7 @@ import { GlAvatar, GlTab, GlTabs, GlBadge, GlButton, GlSprintf, GlIcon, GlLink } import TitleArea from '~/vue_shared/components/registry/title_area.vue'; import * as Sentry from '~/sentry/sentry_browser_wrapper'; import { createAlert, VARIANT_DANGER } from '~/alert'; -import { s__, sprintf } from '~/locale'; +import { __, s__, sprintf } from '~/locale'; import { setUrlFragment, visitUrlWithAlerts } from '~/lib/utils/url_utility'; import getModelVersionQuery from '~/ml/model_registry/graphql/queries/get_model_version.query.graphql'; import deleteModelVersionMutation from '~/ml/model_registry/graphql/mutations/delete_model_version.mutation.graphql'; @@ -232,7 +232,7 @@ export default { }, }, i18n: { - editModelVersionButtonLabel: s__('MlModelRegistry|Edit model version'), + editModelVersionButtonLabel: __('Edit'), authorTitle: s__('MlModelRegistry|Publisher'), tabs: { modelVersionCard: s__('MlModelRegistry|Version card'), @@ -280,7 +280,7 @@ export default { {{ $options.i18n.editModelVersionButtonLabel }} diff --git a/app/assets/javascripts/sentry/init_sentry.js b/app/assets/javascripts/sentry/init_sentry.js index 580fa8451c3..0c9b67404b4 100644 --- a/app/assets/javascripts/sentry/init_sentry.js +++ b/app/assets/javascripts/sentry/init_sentry.js @@ -5,6 +5,7 @@ import { // exports captureException, + addBreadcrumb, SDK_VERSION, } from '@sentry/browser'; @@ -71,6 +72,7 @@ const initSentry = () => { // eslint-disable-next-line no-underscore-dangle window._Sentry = { captureException, + addBreadcrumb, SDK_VERSION, // used to verify compatibility with the Sentry instance }; }; diff --git a/app/assets/javascripts/sentry/sentry_browser_wrapper.js b/app/assets/javascripts/sentry/sentry_browser_wrapper.js index 99f5adf8e89..1d83de0ad88 100644 --- a/app/assets/javascripts/sentry/sentry_browser_wrapper.js +++ b/app/assets/javascripts/sentry/sentry_browser_wrapper.js @@ -21,3 +21,17 @@ export const captureException = (...args) => { Sentry?.captureException(...args); }; + +/** @type {import('@sentry/core').addBreadcrumb} */ +export const addBreadcrumb = (...args) => { + // eslint-disable-next-line no-underscore-dangle + const Sentry = window._Sentry; + + // When Sentry is not configured during development, show console error + if (process.env.NODE_ENV === 'development' && !Sentry) { + console.debug('[Sentry stub]', 'addBreadcrumb(...) called with:', { ...args }); + return; + } + + Sentry?.addBreadcrumb(...args); +}; diff --git a/app/controllers/groups/dependency_proxy/application_controller.rb b/app/controllers/groups/dependency_proxy/application_controller.rb index de5bdb6d832..8ccc37d6bf3 100644 --- a/app/controllers/groups/dependency_proxy/application_controller.rb +++ b/app/controllers/groups/dependency_proxy/application_controller.rb @@ -14,6 +14,7 @@ module Groups skip_before_action :authenticate_user!, raise: false prepend_before_action :authenticate_user_from_jwt_token! + before_action :skip_session def authenticate_user_from_jwt_token! authenticate_with_http_token do |token, _| @@ -55,6 +56,10 @@ module Groups def set_auth_result(actor, type) @authentication_result = Gitlab::Auth::Result.new(actor, nil, type, []) end + + def skip_session + request.session_options[:skip] = true + end end end end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index f962fbc3229..8c16c8339d1 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -15,6 +15,13 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ACTIVE_SINCE_KEY = 'active_since' + # Following https://www.rfc-editor.org/rfc/rfc3986.txt + # to check for the present of reserved characters + # in redirect_fragment + INVALID_FRAGMENT_EXP = %r{[;/?:@&=+$,]+} + + InvalidFragmentError = Class.new(StandardError) + after_action :verify_known_sign_in protect_from_forgery except: [:failure] + AuthHelper.saml_providers, with: :exception, prepend: true @@ -102,6 +109,14 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController private + def verify_redirect_fragment(fragment) + if URI.decode_uri_component(fragment).match(INVALID_FRAGMENT_EXP) + raise InvalidFragmentError + else + fragment + end + end + def track_event(user, provider, status) log_audit_event(user, with: provider) update_login_counter_metric(provider, status) @@ -172,6 +187,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController else sign_in_user_flow(auth_module::User) end + rescue InvalidFragmentError + fail_login_with_message("Invalid state") end def link_identity(identity_linker) @@ -369,7 +386,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController key = stored_location_key_for(:user) location = session[key] if uri = parse_uri(location) - uri.fragment = redirect_fragment + uri.fragment = verify_redirect_fragment(redirect_fragment) store_location_for(:user, uri.to_s) end end diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index a7fe92fced0..0f921522df2 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -24,6 +24,7 @@ class Projects::CommitController < Projects::ApplicationController before_action do push_frontend_feature_flag(:ci_graphql_pipeline_mini_graph, @project) end + before_action :rate_limit_for_expanded_diff_files, only: :diff_files BRANCH_SEARCH_LIMIT = 1000 COMMIT_DIFFS_PER_PAGE = 20 @@ -57,7 +58,7 @@ class Projects::CommitController < Projects::ApplicationController format.html do render template: 'projects/commit/diff_files', layout: false, - locals: { diffs: @diffs, environment: @environment } + locals: { diffs: @diffs.with_highlights_preloaded, environment: @environment } end end end @@ -290,6 +291,12 @@ class Projects::CommitController < Projects::ApplicationController view: diff_view ) end + + def rate_limit_for_expanded_diff_files + return unless diffs_expanded? + + check_rate_limit!(:expanded_diff_files, scope: current_user || request.ip) + end end Projects::CommitController.prepend_mod_with('Projects::CommitController') diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index a3bc0953d30..b3af555665d 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -742,7 +742,8 @@ module Types description: 'List of unprotected branches, ignoring any wildcard branch rules', null: true, calls_gitaly: true, - experiment: { milestone: '16.9' } + experiment: { milestone: '16.9' }, + authorize: :read_code field :project_plan_limits, Types::ProjectPlanLimitsType, resolver: Resolvers::Projects::PlanLimitsResolver, diff --git a/app/models/group.rb b/app/models/group.rb index 50d37fbd6f6..5cc5e0ab6be 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -171,6 +171,8 @@ class Group < Namespace validates :group_feature, presence: true + validate :top_level_group_name_not_assigned_to_pages_unique_domain, if: :path_changed? + add_authentication_token_field :runners_token, encrypted: :required, format_with_prefix: :runners_token_prefix, @@ -1157,6 +1159,15 @@ class Group < Namespace def runners_token_prefix RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX end + + def top_level_group_name_not_assigned_to_pages_unique_domain + return unless parent_id.nil? + + return unless ProjectSetting.unique_domain_exists?(path) + + # We cannot disclose the Pages unique domain, hence returning generic error message + errors.add(:path, _('has already been taken')) + end end Group.prepend_mod_with('Group') diff --git a/app/models/namespace.rb b/app/models/namespace.rb index b1040f8975a..6b29ee5bac6 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -356,7 +356,7 @@ class Namespace < ApplicationRecord def clean_path(path, limited_to: Namespace.all) slug = Gitlab::Slug::Path.new(path).generate path = Namespaces::RandomizedSuffixPath.new(slug) - Gitlab::Utils::Uniquify.new.string(path) { |s| limited_to.find_by_path_or_name(s) } + Gitlab::Utils::Uniquify.new.string(path) { |s| limited_to.find_by_path_or_name(s) || ProjectSetting.unique_domain_exists?(s) } end def clean_name(value) @@ -759,6 +759,14 @@ class Namespace < ApplicationRecord { organization_id: organization_id } end + def pipeline_variables_default_role + return namespace_settings.pipeline_variables_default_role if namespace_settings.present? + + # We could have old namespaces that don't have an associated `namespace_settings` record. + # To avoid returning `nil` we return the database-level default. + NamespaceSetting.column_defaults['pipeline_variables_default_role'] + end + private def require_organization? diff --git a/app/models/namespace_setting.rb b/app/models/namespace_setting.rb index d02469a5eae..4a3dbd619ad 100644 --- a/app/models/namespace_setting.rb +++ b/app/models/namespace_setting.rb @@ -6,6 +6,8 @@ class NamespaceSetting < ApplicationRecord include ChronicDurationAttribute ignore_column :token_expiry_notify_inherited, remove_with: '17.9', remove_after: '2025-01-11' + enum pipeline_variables_default_role: ProjectCiCdSetting::PIPELINE_VARIABLES_OVERRIDE_ROLES, _prefix: true + ignore_column :third_party_ai_features_enabled, remove_with: '16.11', remove_after: '2024-04-18' ignore_column :code_suggestions, remove_with: '17.8', remove_after: '2024-05-16' @@ -29,6 +31,8 @@ class NamespaceSetting < ApplicationRecord sanitizes! :default_branch_name + before_validation :set_pipeline_variables_default_role, on: :create + before_validation :normalize_default_branch_name chronic_duration_attr :runner_token_expiration_interval_human_readable, :runner_token_expiration_interval @@ -68,6 +72,15 @@ class NamespaceSetting < ApplicationRecord namespace.root_ancestor.prevent_sharing_groups_outside_hierarchy end + def pipeline_variables_default_role + # We consider only the root namespace setting to cascade the default value to all projects. + # By ignoring settings from sub-groups we don't need to deal with complexities from + # hierarchical settings. + return namespace.root_ancestor.pipeline_variables_default_role unless namespace.root? + + super + end + def emails_enabled? return emails_enabled unless namespace.has_parent? @@ -101,6 +114,18 @@ class NamespaceSetting < ApplicationRecord private + def set_pipeline_variables_default_role + # After FF `change_namespace_default_role_for_pipeline_variables` rollout - we have to remove both FF and pipeline_variables_default_role = NO_ONE_ALLOWED_ROLE + # As any self-managed and Dedicated instance should opt-in by changing their namespace settings explicitly. + # NO_ONE_ALLOWED will be set as the default value for namespace_settings through a database migration. + + # WARNING: Removing this FF could cause breaking changes for self-hosted and dedicated instances. + + return if Feature.disabled?(:change_namespace_default_role_for_pipeline_variables, namespace) + + self.pipeline_variables_default_role = ProjectCiCdSetting::NO_ONE_ALLOWED_ROLE + end + def all_ancestors_have_emails_enabled? self.class.where(namespace_id: namespace.self_and_ancestors, emails_enabled: false).none? end diff --git a/app/models/project_ci_cd_setting.rb b/app/models/project_ci_cd_setting.rb index 27f017c47fc..52e374f8b9a 100644 --- a/app/models/project_ci_cd_setting.rb +++ b/app/models/project_ci_cd_setting.rb @@ -11,15 +11,17 @@ class ProjectCiCdSetting < ApplicationRecord MAINTAINER_ROLE = 3 OWNER_ROLE = 4 + PIPELINE_VARIABLES_OVERRIDE_ROLES = + { no_one_allowed: NO_ONE_ALLOWED_ROLE, + developer: DEVELOPER_ROLE, + maintainer: MAINTAINER_ROLE, + owner: OWNER_ROLE }.freeze + ALLOWED_SUB_CLAIM_COMPONENTS = %w[project_path ref_type ref].freeze - enum pipeline_variables_minimum_override_role: { - no_one_allowed: NO_ONE_ALLOWED_ROLE, - developer: DEVELOPER_ROLE, - maintainer: MAINTAINER_ROLE, - owner: OWNER_ROLE - }, _prefix: true + enum pipeline_variables_minimum_override_role: PIPELINE_VARIABLES_OVERRIDE_ROLES, _prefix: true + before_validation :set_pipeline_variables_secure_defaults, on: :create before_create :set_default_git_depth validates :id_token_sub_claim_components, length: { @@ -79,6 +81,11 @@ class ProjectCiCdSetting < ApplicationRecord private + def set_pipeline_variables_secure_defaults + self.restrict_user_defined_variables = true + self.pipeline_variables_minimum_override_role = project.root_namespace.pipeline_variables_default_role + end + def role_map_pipeline_variables_minimum_override_role { DEVELOPER_ROLE => Gitlab::Access::DEVELOPER, diff --git a/app/models/user.rb b/app/models/user.rb index 76fa1c7403b..783f34204c7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -328,6 +328,7 @@ class User < ApplicationRecord presence: true, exclusion: { in: Gitlab::PathRegex::TOP_LEVEL_ROUTES, message: N_('%{value} is a reserved name') } validates :username, uniqueness: true, unless: :namespace + validate :username_not_assigned_to_pages_unique_domain, if: :username_changed? validates :name, presence: true, length: { maximum: 255 } validates :first_name, length: { maximum: 127 } validates :last_name, length: { maximum: 127 } @@ -2882,6 +2883,13 @@ class User < ApplicationRecord # method overridden in EE def audit_unlock_access(author: self); end + + def username_not_assigned_to_pages_unique_domain + if ProjectSetting.unique_domain_exists?(username) + # We cannot disclose the Pages unique domain, hence returning generic error message + errors.add(:username, _('has already been taken')) + end + end end User.prepend_mod_with('User') diff --git a/app/serializers/integrations/harbor_serializers/artifact_entity.rb b/app/serializers/integrations/harbor_serializers/artifact_entity.rb index 010380561eb..2731a34e429 100644 --- a/app/serializers/integrations/harbor_serializers/artifact_entity.rb +++ b/app/serializers/integrations/harbor_serializers/artifact_entity.rb @@ -10,7 +10,7 @@ module Integrations end expose :digest do |item| - strip_tags(item['digest']) + validate_path(item['digest']).then { |digest| strip_tags(digest) } end expose :size do |item| @@ -24,6 +24,19 @@ module Integrations expose :tags do |item| item['tags'].map { |tag| strip_tags(tag['name']) } end + + private + + def validate_path(path) + ::Gitlab::PathTraversal.check_path_traversal!(path) + rescue ::Gitlab::PathTraversal::PathTraversalAttackError => e + ::Gitlab::ErrorTracking.track_exception( + e, + message: "Path traversal attack detected #{path}", + class: self.class.name + ) + '' + end end end end diff --git a/app/serializers/integrations/harbor_serializers/repository_entity.rb b/app/serializers/integrations/harbor_serializers/repository_entity.rb index a6366ebfb36..609f42e85a6 100644 --- a/app/serializers/integrations/harbor_serializers/repository_entity.rb +++ b/app/serializers/integrations/harbor_serializers/repository_entity.rb @@ -10,7 +10,7 @@ module Integrations end expose :name do |item| - strip_tags(item['name']) + validate_path(item['name']).then { |name| strip_tags(name) } end expose :artifact_count do |item| diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 479401a8bab..2cb2af447d5 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -8,9 +8,9 @@ module Ci LOG_MAX_PIPELINE_SIZE = 2_000 LOG_MAX_CREATION_THRESHOLD = 20.seconds SEQUENCE = [Gitlab::Ci::Pipeline::Chain::Build, - Gitlab::Ci::Pipeline::Chain::Build::Associations, Gitlab::Ci::Pipeline::Chain::Validate::Abilities, Gitlab::Ci::Pipeline::Chain::Validate::Repository, + Gitlab::Ci::Pipeline::Chain::Build::Associations, Gitlab::Ci::Pipeline::Chain::Limit::RateLimit, Gitlab::Ci::Pipeline::Chain::Validate::SecurityOrchestrationPolicy, Gitlab::Ci::Pipeline::Chain::AssignPartition, diff --git a/app/services/concerns/issues/resolve_discussions.rb b/app/services/concerns/issues/resolve_discussions.rb index 5e87f610e4e..c09cad893b0 100644 --- a/app/services/concerns/issues/resolve_discussions.rb +++ b/app/services/concerns/issues/resolve_discussions.rb @@ -37,7 +37,7 @@ module Issues else merge_request_to_resolve_discussions_of .discussions_to_be_resolved - end + end.reject(&:confidential?) end end end diff --git a/config/application.rb b/config/application.rb index 3c972fcb099..6f9d3baaf7a 100644 --- a/config/application.rb +++ b/config/application.rb @@ -220,6 +220,7 @@ module Gitlab /key$/, /^body$/, /^description$/, + /^query$/, /^note$/, /^text$/, /^title$/, diff --git a/config/environments/development.rb b/config/environments/development.rb index ff19b05a7d5..ebd991bc8ba 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -117,7 +117,7 @@ Rails.application.configure do end config.middleware.insert_before( - ActionDispatch::Cookies, Gitlab::Middleware::StripCookies, paths: [%r{^/assets/}, %r{^/v2$}, %r{^/v2/}] + ActionDispatch::Cookies, Gitlab::Middleware::StripCookies, paths: [%r{^/assets/}] ) config.log_level = Gitlab::Utils.to_rails_log_level(ENV["GITLAB_LOG_LEVEL"], :debug) diff --git a/config/environments/production.rb b/config/environments/production.rb index 68bff1b41c4..8efc793223b 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require 'gitlab/middleware/strip_cookies' - Rails.application.configure do # Settings specified here will take precedence over those in config/application.rb @@ -77,8 +75,4 @@ Rails.application.configure do config.action_mailer.raise_delivery_errors = true config.eager_load = true - - config.middleware.insert_before( - ActionDispatch::Cookies, Gitlab::Middleware::StripCookies, paths: [%r{^/v2$}, %r{^/v2/}] - ) end diff --git a/config/environments/test.rb b/config/environments/test.rb index d233601b687..3c27f9f3499 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -14,9 +14,7 @@ Rails.application.configure do config.middleware.insert_before(ActionDispatch::Static, Gitlab::Testing::RobotsBlockerMiddleware) config.middleware.insert_before(ActionDispatch::Static, Gitlab::Testing::RequestInspectorMiddleware) config.middleware.insert_before(ActionDispatch::Static, Gitlab::Testing::ClearProcessMemoryCacheMiddleware) - config.middleware.insert_before( - ActionDispatch::Cookies, Gitlab::Middleware::StripCookies, paths: [%r{^/assets/}, %r{^/v2$}, %r{^/v2/}] - ) + config.middleware.insert_before(ActionDispatch::Cookies, Gitlab::Middleware::StripCookies, paths: [%r{^/assets/}]) Gitlab::Testing::ActionCableBlocker.install diff --git a/config/feature_flags/development/change_namespace_default_role_for_pipeline_variables.yml b/config/feature_flags/development/change_namespace_default_role_for_pipeline_variables.yml new file mode 100644 index 00000000000..6d985fa5e5a --- /dev/null +++ b/config/feature_flags/development/change_namespace_default_role_for_pipeline_variables.yml @@ -0,0 +1,8 @@ +--- +name: change_namespace_default_role_for_pipeline_variables +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/171424 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/502238 +milestone: '17.7' +type: development +group: group::pipeline security +default_enabled: false diff --git a/config/metrics/schema/product_groups.json b/config/metrics/schema/product_groups.json index 61a7db17b26..7ee8a1454a8 100644 --- a/config/metrics/schema/product_groups.json +++ b/config/metrics/schema/product_groups.json @@ -60,6 +60,7 @@ "runner", "scalability", "secret_detection", + "security_infrastructure", "security_insights", "security_policies", "source_code", diff --git a/db/docs/geo_events.yml b/db/docs/geo_events.yml index 5ed50493fc9..ef4b7288f73 100644 --- a/db/docs/geo_events.yml +++ b/db/docs/geo_events.yml @@ -9,6 +9,5 @@ description: Geo events implemented generically, used by the SSF where all objec introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23447 milestone: '12.8' gitlab_schema: gitlab_main_cell -sharding_key_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/479383 table_size: small exempt_from_sharding: true diff --git a/db/migrate/20241112055215_add_pipeline_variables_default_role_to_namespace_settings.rb b/db/migrate/20241112055215_add_pipeline_variables_default_role_to_namespace_settings.rb new file mode 100644 index 00000000000..94e84ca81e5 --- /dev/null +++ b/db/migrate/20241112055215_add_pipeline_variables_default_role_to_namespace_settings.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class AddPipelineVariablesDefaultRoleToNamespaceSettings < Gitlab::Database::Migration[2.2] + milestone '17.7' + + DEVELOPER_ROLE = 2 + + def change + add_column :namespace_settings, :pipeline_variables_default_role, + :integer, default: DEVELOPER_ROLE, null: false, limit: 2 + end +end diff --git a/db/post_migrate/20241127102904_validate_namespaces_organization_id_not_null_constraint.rb b/db/post_migrate/20241127102904_validate_namespaces_organization_id_not_null_constraint.rb new file mode 100644 index 00000000000..52a5ed2e1fc --- /dev/null +++ b/db/post_migrate/20241127102904_validate_namespaces_organization_id_not_null_constraint.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class ValidateNamespacesOrganizationIdNotNullConstraint < Gitlab::Database::Migration[2.2] + milestone '17.7' + + def up + validate_not_null_constraint :namespaces, :organization_id + end + + def down + # no-op + end +end diff --git a/db/schema_migrations/20241112055215 b/db/schema_migrations/20241112055215 new file mode 100644 index 00000000000..6007f0af1eb --- /dev/null +++ b/db/schema_migrations/20241112055215 @@ -0,0 +1 @@ +3a45e94fffa5b618bff82b0768429dfb68412f709f21cc380ac39058ba4adc6b \ No newline at end of file diff --git a/db/schema_migrations/20241127102904 b/db/schema_migrations/20241127102904 new file mode 100644 index 00000000000..ec56a9186e3 --- /dev/null +++ b/db/schema_migrations/20241127102904 @@ -0,0 +1 @@ +bfb69bc3eb91b9b301f546eb18726512c3dee89f9d025974b4181a14d4e10b98 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 1a972b59edd..a442d68c6e3 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -201,7 +201,8 @@ CREATE TABLE namespaces ( shared_runners_enabled boolean DEFAULT true NOT NULL, allow_descendants_override_disabled_shared_runners boolean DEFAULT false NOT NULL, traversal_ids bigint[] DEFAULT '{}'::bigint[] NOT NULL, - organization_id bigint DEFAULT 1 + organization_id bigint DEFAULT 1, + CONSTRAINT check_2eae3bdf93 CHECK ((organization_id IS NOT NULL)) ); CREATE FUNCTION find_namespaces_by_id(namespaces_id bigint) RETURNS namespaces @@ -15589,6 +15590,7 @@ CREATE TABLE namespace_settings ( token_expiry_notify_inherited boolean DEFAULT true NOT NULL, resource_access_token_notify_inherited boolean, lock_resource_access_token_notify_inherited boolean DEFAULT false NOT NULL, + pipeline_variables_default_role smallint DEFAULT 2 NOT NULL, CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255)), CONSTRAINT namespace_settings_unique_project_download_limit_alertlist_size CHECK ((cardinality(unique_project_download_limit_alertlist) <= 100)), CONSTRAINT namespace_settings_unique_project_download_limit_allowlist_size CHECK ((cardinality(unique_project_download_limit_allowlist) <= 100)) @@ -25397,9 +25399,6 @@ ALTER TABLE workspaces ALTER TABLE security_scans ADD CONSTRAINT check_2d56d882f6 CHECK ((project_id IS NOT NULL)) NOT VALID; -ALTER TABLE namespaces - ADD CONSTRAINT check_2eae3bdf93 CHECK ((organization_id IS NOT NULL)) NOT VALID; - ALTER TABLE vulnerability_scanners ADD CONSTRAINT check_37608c9db5 CHECK ((char_length(vendor) <= 255)) NOT VALID; diff --git a/doc/api/api_resources.md b/doc/api/api_resources.md index 7be0e7cd941..5901520edba 100644 --- a/doc/api/api_resources.md +++ b/doc/api/api_resources.md @@ -32,7 +32,8 @@ The following API resources are available in the project context: | [Agents](cluster_agents.md) | `/projects/:id/cluster_agents` | | [Branches](branches.md) | `/projects/:id/repository/branches/`, `/projects/:id/repository/merged_branches` | | [Commits](commits.md) | `/projects/:id/repository/commits`, `/projects/:id/statuses` | -| [Container Registry](container_registry.md) | `/projects/:id/registry/repositories` | +| [Container registry](container_registry.md) | `/projects/:id/registry/repositories` | +| [Container repository protection rules](container_repository_protection_rules.md) | `/projects/:id/registry/protection/repository/rules` | | [Custom attributes](custom_attributes.md) | `/projects/:id/custom_attributes` (also available for groups and users) | | [Composer distributions](packages/composer.md) | `/projects/:id/packages/composer` (also available for groups) | | [Conan distributions](packages/conan.md) | `/projects/:id/packages/conan` (also available standalone) | diff --git a/doc/api/repositories.md b/doc/api/repositories.md index 0148971c1d7..dcf28db696c 100644 --- a/doc/api/repositories.md +++ b/doc/api/repositories.md @@ -330,6 +330,10 @@ of commits, GitLab generates a changelog for all commits that use a particular a new Markdown-formatted section to a changelog file in the Git repository of the project. The output format can be customized. +For performance and security reasons, parsing the changelog configuration is limited to `2` seconds. +This limitation helps prevent potential DoS attacks from malformed changelog templates. +If the request times out, consider reducing the size of your `changelog_config.yml` file. + For user-facing documentation, see [Changelogs](../user/project/changelogs.md). ```plaintext diff --git a/doc/ci/variables/index.md b/doc/ci/variables/index.md index 92b0fab9747..6a077d21419 100644 --- a/doc/ci/variables/index.md +++ b/doc/ci/variables/index.md @@ -838,11 +838,8 @@ use this setting for control over the environment the pipeline runs in. #### Set a minimum role for pipeline variables -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/440338) in GitLab 17.1 - -When [pipeline variables are restricted](#restrict-pipeline-variables), you can also -set a specific minimum [role](../../user/permissions.md#roles) that can run pipelines -with pipeline variables. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/440338) in GitLab 17.1. +> - For GitLab.com, setting defaults [updated for all new projects in new namespaces](https://gitlab.com/gitlab-org/gitlab/-/issues/502382) to `enabled` for `restrict_user_defined_variables` and `no_one_allowed` for `ci_pipeline_variables_minimum_override_role` in GitLab 17.7. Prerequisites: @@ -853,10 +850,11 @@ To change the setting, use [the projects API](../../api/projects.md#edit-a-proje to set `ci_pipeline_variables_minimum_override_role` to one of: - `no_one_allowed`: No pipelines can run with pipeline variables. + Default for new projects in new namespaces on GitLab.com. - `owner`: Only users with the Owner role can run pipelines with pipeline variables. You must have the Owner role for the project to change the setting to this value. - `maintainer`: Only users with at least the Maintainer role can run pipelines with pipeline variables. - Default when not specified. + Default when not specified on self-managed and Dedicated. - `developer`: Only users with at least the Developer role can run pipelines with pipeline variables. ## Exporting variables diff --git a/doc/security/rate_limits.md b/doc/security/rate_limits.md index ce379ded4aa..bd7a8791685 100644 --- a/doc/security/rate_limits.md +++ b/doc/security/rate_limits.md @@ -214,6 +214,20 @@ There is a rate limit for triggering project imports from FogBugz. The **rate limit** is 1 triggered import per minute per user. +### Commit diff files + +This is a rate limit for expanded commit diff files (`/[group]/[project]/-/commit/[:sha]/diff_files?expanded=1`), +which is enforced to prevent from abusing this endpoint. + +The **rate limit** is 6 requests per minute per user (authenticated) or per IP address (unauthenticated). + +### Changelog generation + +There is a rate limit per user per project on the `:id/repository/changelog` endpoint. This is to mitigate attempts to misuse the endpoint. +The rate limit is shared between GET and POST actions. + +The **rate limit** is 5 calls per minute per user per project. + ## Troubleshooting ### Rack Attack is denylisting the load balancer diff --git a/doc/user/application_security/policies/pipeline_execution_policies.md b/doc/user/application_security/policies/pipeline_execution_policies.md index a2eb2103304..c703ae78170 100644 --- a/doc/user/application_security/policies/pipeline_execution_policies.md +++ b/doc/user/application_security/policies/pipeline_execution_policies.md @@ -46,6 +46,7 @@ the following sections and tables provide an alternative. | `pipeline_config_strategy` | `string` | false | Can either be `inject_ci` or `override_project_ci`. See [Pipeline strategies](#pipeline-strategies) for more information. | | `policy_scope` | `object` of [`policy_scope`](index.md#scope) | false | Scopes the policy based on projects, groups, or compliance framework labels you specify. | | `suffix` | `string` | false | Can either be `on_conflict` (default), or `never`. Defines the behavior for handling job naming conflicts. `on_conflict` applies a unique suffix to the job names for jobs that would break the uniqueness. `never` causes the pipeline to fail if the job names across the project and all applicable policies are not unique. | +| `skip_ci` | `object` of [`skip_ci`](pipeline_execution_policies.md#skip_ci-type) | false | Defines whether users can apply the `skip-ci` directive. By default, the use of `skip-ci` is ignored and as a result, pipelines with pipeline execution policies cannot be skipped. | Note the following: @@ -164,6 +165,19 @@ Prerequisites: Enabling this setting grants the user who triggered the pipeline access to read the CI/CD configuration file enforced by the pipeline execution policy. This setting does not grant the user access to any other parts of the project where the configuration file is stored. +### `skip_ci` type + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/173480) in GitLab 17.7. + +Use the `skip_ci` keyword to specify whether users are allowed to apply the `skip-ci` directive to skip the pipelines. +When the keyword is not specified, the `skip-ci` directive is ignored, preventing all users +from bypassing the pipeline execution policies. + +| Field | Type | Possible values | Description | +|-------------------------|----------|--------------------------|-------------| +| `allowed` | `boolean` | `true`, `false` | Flag to allow (`true`) or prevent (`false`) the use of the `skip-ci` directive for pipelines with enforced pipeline execution policies. | +| `allowlist` | `object` | `users` | Specify users who are always allowed to use `skip-ci` directive, regardless of the `allowed` flag. Use `users:` followed by an array of objects with `id` keys representing user IDs. | + ### Policy scope schema To customize policy enforcement, you can define a policy's scope to either include, or exclude, diff --git a/doc/user/project/changelogs.md b/doc/user/project/changelogs.md index a726fa29f3e..19b1e2f471d 100644 --- a/doc/user/project/changelogs.md +++ b/doc/user/project/changelogs.md @@ -111,9 +111,11 @@ for definitions and usage. ## Customize the changelog output -To customize the changelog output, edit the changelog configuration file, and commit these changes to your project's Git repository. -The default location for this configuration is `.gitlab/changelog_config.yml`. The file supports -these variables: +To customize the changelog output, edit the changelog configuration file, and commit these changes to your project's Git repository. The default location for this configuration is `.gitlab/changelog_config.yml`. + +For performance and security reasons, parsing the changelog configuration is limited to `2` seconds. +If parsing the configuration results in timeout errors, consider reducing the size of the configuration. +The file supports these variables: - `date_format`: The date format, in `strftime` format, used in the title of the newly added changelog data. - `template`: A custom template to use when generating the changelog data. diff --git a/lib/api/api.rb b/lib/api/api.rb index 57d9d490e67..190f1009479 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -358,6 +358,7 @@ module API mount ::API::UserRunners mount ::API::VirtualRegistries::Packages::Maven::Registries mount ::API::VirtualRegistries::Packages::Maven::Upstreams + mount ::API::VirtualRegistries::Packages::Maven::CachedResponses mount ::API::VirtualRegistries::Packages::Maven::Endpoints mount ::API::WebCommits mount ::API::Wikis diff --git a/lib/api/concerns/virtual_registries/packages/maven/cached_response_endpoints.rb b/lib/api/concerns/virtual_registries/packages/maven/cached_response_endpoints.rb deleted file mode 100644 index 237c5450afe..00000000000 --- a/lib/api/concerns/virtual_registries/packages/maven/cached_response_endpoints.rb +++ /dev/null @@ -1,90 +0,0 @@ -# frozen_string_literal: true - -module API - module Concerns - module VirtualRegistries - module Packages - module Maven - module CachedResponseEndpoints - extend ActiveSupport::Concern - - included do - include ::API::PaginationParams - - helpers do - def cached_responses - upstream.cached_responses.default.search_by_relative_path(params[:search]) - end - - def cached_response - upstream.cached_responses.default.find_by_relative_path!(declared_params[:cached_response_id]) - end - end - - desc 'List maven virtual registry upstream cached responses' do - detail 'This feature was introduced in GitLab 17.4. \ - This feature is currently in an experimental state. \ - This feature is behind the `virtual_registry_maven` feature flag.' - success Entities::VirtualRegistries::Packages::Maven::CachedResponse - failure [ - { code: 400, message: 'Bad Request' }, - { code: 401, message: 'Unauthorized' }, - { code: 403, message: 'Forbidden' }, - { code: 404, message: 'Not found' } - ] - tags %w[maven_virtual_registries] - is_array true - hidden true - end - params do - optional :search, type: String, desc: 'Search query', documentation: { example: 'foo/bar/mypkg' } - use :pagination - end - get do - authorize! :read_virtual_registry, registry - - # TODO: refactor this when we support multiple upstreams. - # https://gitlab.com/gitlab-org/gitlab/-/issues/480461 - not_found! if upstream&.id != params[:upstream_id] - - present paginate(cached_responses), with: Entities::VirtualRegistries::Packages::Maven::CachedResponse - end - - desc 'Delete a maven virtual registry upstream cached response' do - detail 'This feature was introduced in GitLab 17.4. \ - This feature is currently in an experimental state. \ - This feature is behind the `virtual_registry_maven` feature flag.' - success code: 204 - failure [ - { code: 400, message: 'Bad Request' }, - { code: 401, message: 'Unauthorized' }, - { code: 403, message: 'Forbidden' }, - { code: 404, message: 'Not found' } - ] - tags %w[maven_virtual_registries] - hidden true - end - params do - requires :cached_response_id, type: String, coerce_with: Base64.method(:urlsafe_decode64), - desc: 'The base64 encoded relative path of the cached response', - documentation: { example: 'Zm9vL2Jhci9teXBrZy5wb20=' } - end - - delete '*cached_response_id' do - authorize! :destroy_virtual_registry, registry - - # TODO: refactor this when we support multiple upstreams. - # https://gitlab.com/gitlab-org/gitlab/-/issues/480461 - not_found! if upstream&.id != params[:upstream_id] - - destroy_conditionally!(cached_response) do |cached_response| - render_validation_error!(cached_response) unless cached_response.update(upstream: nil) - end - end - end - end - end - end - end - end -end diff --git a/lib/api/entities/virtual_registries/packages/maven/cached_response.rb b/lib/api/entities/virtual_registries/packages/maven/cached_response.rb index e322e408fdb..736b7eca7ec 100644 --- a/lib/api/entities/virtual_registries/packages/maven/cached_response.rb +++ b/lib/api/entities/virtual_registries/packages/maven/cached_response.rb @@ -6,8 +6,8 @@ module API module Packages module Maven class CachedResponse < Grape::Entity - expose :cached_response_id do |cached_response, _options| - Base64.urlsafe_encode64(cached_response.relative_path) + expose :id do |cached_response, _options| + Base64.urlsafe_encode64("#{cached_response.upstream_id} #{cached_response.relative_path}") end expose :group_id, diff --git a/lib/api/namespaces.rb b/lib/api/namespaces.rb index 0ef9c23f915..0a4ed9d3c1b 100644 --- a/lib/api/namespaces.rb +++ b/lib/api/namespaces.rb @@ -93,7 +93,7 @@ module API namespace_path = params[:id] existing_namespaces_within_the_parent = Namespace.without_project_namespaces.by_parent(params[:parent_id]) - exists = existing_namespaces_within_the_parent.filter_by_path(namespace_path).exists? + exists = existing_namespaces_within_the_parent.filter_by_path(namespace_path).exists? || ProjectSetting.unique_domain_exists?(namespace_path) suggestions = exists ? [Namespace.clean_path(namespace_path, limited_to: existing_namespaces_within_the_parent)] : [] present :exists, exists diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 70651b018e4..39ac79705ac 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -288,6 +288,10 @@ module API end route_setting :authentication, job_token_allowed: true get ':id/repository/changelog' do + check_rate_limit!(:project_repositories_changelog, scope: [current_user, user_project]) do + render_api_error!({ error: 'This changelog has been requested too many times. Try again later.' }, 429) + end + service = ::Repositories::ChangelogService.new( user_project, current_user, @@ -329,6 +333,10 @@ module API documentation: { example: 'Initial commit' } end post ':id/repository/changelog' do + check_rate_limit!(:project_repositories_changelog, scope: [current_user, user_project]) do + render_api_error!({ error: 'This changelog has been requested too many times. Try again later.' }, 429) + end + branch = params[:branch] || user_project.default_branch_or_main access = Gitlab::UserAccess.new(current_user, container: user_project) diff --git a/lib/api/virtual_registries/packages/maven/cached_responses.rb b/lib/api/virtual_registries/packages/maven/cached_responses.rb new file mode 100644 index 00000000000..f6215a74e2a --- /dev/null +++ b/lib/api/virtual_registries/packages/maven/cached_responses.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +module API + module VirtualRegistries + module Packages + module Maven + class CachedResponses < ::API::Base + include ::API::Helpers::Authentication + include ::API::PaginationParams + + feature_category :virtual_registry + urgency :low + + authenticate_with do |accept| + accept.token_types(:personal_access_token).sent_through(:http_private_token_header) + accept.token_types(:deploy_token).sent_through(:http_deploy_token_header) + accept.token_types(:job_token).sent_through(:http_job_token_header) + end + + helpers do + include ::Gitlab::Utils::StrongMemoize + + def require_dependency_proxy_enabled! + not_found! unless ::Gitlab.config.dependency_proxy.enabled + end + + def upstream + ::VirtualRegistries::Packages::Maven::Upstream.find(params[:id]) + end + strong_memoize_attr :upstream + + def cached_responses + upstream.cached_responses.default.search_by_relative_path(params[:search]) + end + + def cached_response + ::VirtualRegistries::Packages::Maven::CachedResponse + .default + .find_by_upstream_id_and_relative_path!(*declared_params[:id].split) + end + strong_memoize_attr :cached_response + end + + after_validation do + not_found! unless Feature.enabled?(:virtual_registry_maven, current_user) + + require_dependency_proxy_enabled! + + authenticate! + end + + namespace 'virtual_registries/packages/maven' do + namespace :upstreams do + route_param :id, type: Integer, desc: 'The ID of the maven virtual registry upstream' do + namespace :cached_responses do + desc 'List maven virtual registry upstream cached responses' do + detail 'This feature was introduced in GitLab 17.4. \ + This feature is currently in an experimental state. \ + This feature is behind the `virtual_registry_maven` feature flag.' + success ::API::Entities::VirtualRegistries::Packages::Maven::CachedResponse + failure [ + { code: 400, message: 'Bad Request' }, + { code: 401, message: 'Unauthorized' }, + { code: 403, message: 'Forbidden' }, + { code: 404, message: 'Not found' } + ] + tags %w[maven_virtual_registries] + is_array true + hidden true + end + + params do + optional :search, type: String, desc: 'Search query', documentation: { example: 'foo/bar/mypkg' } + use :pagination + end + get do + authorize! :read_virtual_registry, upstream + + present paginate(cached_responses), + with: ::API::Entities::VirtualRegistries::Packages::Maven::CachedResponse + end + end + end + end + + namespace :cached_responses do + desc 'Delete a maven virtual registry upstream cached response' do + detail 'This feature was introduced in GitLab 17.4. \ + This feature is currently in an experimental state. \ + This feature is behind the `virtual_registry_maven` feature flag.' + success code: 204 + failure [ + { code: 400, message: 'Bad Request' }, + { code: 401, message: 'Unauthorized' }, + { code: 403, message: 'Forbidden' }, + { code: 404, message: 'Not found' } + ] + tags %w[maven_virtual_registries] + hidden true + end + params do + requires :id, type: String, coerce_with: Base64.method(:urlsafe_decode64), + desc: 'The base64 encoded upstream id and relative path of the cached response', + documentation: { example: 'Zm9vL2Jhci9teXBrZy5wb20=' } + end + + delete '*id' do + authorize! :destroy_virtual_registry, cached_response.upstream + + destroy_conditionally!(cached_response) do |cached_response| + render_validation_error!(cached_response) unless cached_response.update(upstream: nil) + end + end + end + end + end + end + end + end +end diff --git a/lib/api/virtual_registries/packages/maven/endpoints.rb b/lib/api/virtual_registries/packages/maven/endpoints.rb index 39dd8cf6aec..c874aa5607f 100644 --- a/lib/api/virtual_registries/packages/maven/endpoints.rb +++ b/lib/api/virtual_registries/packages/maven/endpoints.rb @@ -6,6 +6,7 @@ module API module Maven class Endpoints < ::API::Base include ::API::Helpers::Authentication + include ::API::Concerns::VirtualRegistries::Packages::Endpoint feature_category :virtual_registry urgency :low @@ -39,6 +40,13 @@ module API end strong_memoize_attr :registry + def download_file_extra_response_headers(action_params:) + { + SHA1_CHECKSUM_HEADER => action_params[:file_sha1], + MD5_CHECKSUM_HEADER => action_params[:file_md5] + } + end + params :id_and_path do requires :id, type: Integer, @@ -59,108 +67,83 @@ module API authenticate! end - namespace 'virtual_registries/packages/maven' do - namespace :registries do - route_param :id, type: Integer, desc: 'The ID of the maven virtual registry' do - namespace :upstreams do - route_param :upstream_id, type: Integer, desc: 'The ID of the maven virtual registry upstream' do - namespace :cached_responses do - include ::API::Concerns::VirtualRegistries::Packages::Maven::CachedResponseEndpoints - end - end - end - end + namespace 'virtual_registries/packages/maven/:id/*path' do + desc 'Download endpoint of the Maven virtual registry.' do + detail 'This feature was introduced in GitLab 17.3. \ + This feature is currently in experiment state. \ + This feature is behind the `virtual_registry_maven` feature flag.' + success [ + { code: 200 } + ] + failure [ + { code: 400, message: 'Bad request' }, + { code: 401, message: 'Unauthorized' }, + { code: 403, message: 'Forbidden' }, + { code: 404, message: 'Not Found' } + ] + tags %w[maven_virtual_registries] + hidden true + end + params do + use :id_and_path + end + get format: false do + service_response = ::VirtualRegistries::Packages::Maven::HandleFileRequestService.new( + registry: registry, + current_user: current_user, + params: { path: declared_params[:path] } + ).execute + + send_error_response_from!(service_response: service_response) if service_response.error? + send_successful_response_from(service_response: service_response) end - namespace ':id/*path' do - include ::API::Concerns::VirtualRegistries::Packages::Endpoint - - helpers do - def download_file_extra_response_headers(action_params:) - { - SHA1_CHECKSUM_HEADER => action_params[:file_sha1], - MD5_CHECKSUM_HEADER => action_params[:file_md5] - } - end - end - - desc 'Download endpoint of the Maven virtual registry.' do - detail 'This feature was introduced in GitLab 17.3. \ + desc 'Workhorse upload endpoint of the Maven virtual registry. Only workhorse can access it.' do + detail 'This feature was introduced in GitLab 17.4. \ This feature is currently in experiment state. \ This feature is behind the `virtual_registry_maven` feature flag.' - success [ - { code: 200 } - ] - failure [ - { code: 400, message: 'Bad request' }, - { code: 401, message: 'Unauthorized' }, - { code: 403, message: 'Forbidden' }, - { code: 404, message: 'Not Found' } - ] - tags %w[maven_virtual_registries] - hidden true - end - params do - use :id_and_path - end - get format: false do - service_response = ::VirtualRegistries::Packages::Maven::HandleFileRequestService.new( - registry: registry, - current_user: current_user, - params: { path: declared_params[:path] } - ).execute + success [ + { code: 200 } + ] + failure [ + { code: 400, message: 'Bad request' }, + { code: 401, message: 'Unauthorized' }, + { code: 403, message: 'Forbidden' }, + { code: 404, message: 'Not Found' } + ] + tags %w[maven_virtual_registries] + hidden true + end + params do + use :id_and_path + requires :file, + type: ::API::Validations::Types::WorkhorseFile, + desc: 'The file being uploaded', + documentation: { type: 'file' } + end + post 'upload' do + require_gitlab_workhorse! + authorize!(:read_virtual_registry, registry) - send_error_response_from!(service_response: service_response) if service_response.error? - send_successful_response_from(service_response: service_response) - end + etag, content_type, upstream_gid = request.headers.fetch_values( + 'Etag', + ::Gitlab::Workhorse::SEND_DEPENDENCY_CONTENT_TYPE_HEADER, + UPSTREAM_GID_HEADER + ) { nil } - desc 'Workhorse upload endpoint of the Maven virtual registry. Only workhorse can access it.' do - detail 'This feature was introduced in GitLab 17.4. \ - This feature is currently in experiment state. \ - This feature is behind the `virtual_registry_maven` feature flag.' - success [ - { code: 200 } - ] - failure [ - { code: 400, message: 'Bad request' }, - { code: 401, message: 'Unauthorized' }, - { code: 403, message: 'Forbidden' }, - { code: 404, message: 'Not Found' } - ] - tags %w[maven_virtual_registries] - hidden true - end - params do - use :id_and_path - requires :file, - type: ::API::Validations::Types::WorkhorseFile, - desc: 'The file being uploaded', - documentation: { type: 'file' } - end - post 'upload' do - require_gitlab_workhorse! - authorize!(:read_virtual_registry, registry) + # TODO: revisit this part when multiple upstreams are supported + # https://gitlab.com/gitlab-org/gitlab/-/issues/480461 + # coherence check + not_found!('Upstream') unless upstream == GlobalID::Locator.locate(upstream_gid) - etag, content_type, upstream_gid = request.headers.fetch_values( - 'Etag', - ::Gitlab::Workhorse::SEND_DEPENDENCY_CONTENT_TYPE_HEADER, - UPSTREAM_GID_HEADER - ) { nil } + service_response = ::VirtualRegistries::Packages::Maven::CachedResponses::CreateOrUpdateService.new( + upstream: upstream, + current_user: current_user, + params: declared_params.merge(etag: etag, content_type: content_type) + ).execute - # TODO: revisit this part when multiple upstreams are supported - # https://gitlab.com/gitlab-org/gitlab/-/issues/480461 - # coherence check - not_found!('Upstream') unless upstream == GlobalID::Locator.locate(upstream_gid) - - service_response = ::VirtualRegistries::Packages::Maven::CachedResponses::CreateOrUpdateService.new( - upstream: upstream, - current_user: current_user, - params: declared_params.merge(etag: etag, content_type: content_type) - ).execute - - send_error_response_from!(service_response: service_response) if service_response.error? - ok_empty_response - end + send_error_response_from!(service_response: service_response) if service_response.error? + ok_empty_response end end end diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb index 454fed679ec..c345ffa8351 100644 --- a/lib/gitlab/application_rate_limiter.rb +++ b/lib/gitlab/application_rate_limiter.rb @@ -23,6 +23,7 @@ module Gitlab project_export: { threshold: -> { application_settings.project_export_limit }, interval: 1.minute }, project_download_export: { threshold: -> { application_settings.project_download_export_limit }, interval: 1.minute }, project_repositories_archive: { threshold: 5, interval: 1.minute }, + project_repositories_changelog: { threshold: 5, interval: 1.minute }, project_generate_new_export: { threshold: -> { application_settings.project_export_limit }, interval: 1.minute }, project_import: { threshold: -> { application_settings.project_import_limit }, interval: 1.minute }, play_pipeline_schedule: { threshold: 1, interval: 1.minute }, @@ -92,7 +93,8 @@ module Gitlab }, downstream_pipeline_trigger: { threshold: -> { application_settings.downstream_pipeline_trigger_limit_per_project_user_sha }, interval: 1.minute - } + }, + expanded_diff_files: { threshold: 6, interval: 1.minute } }.freeze end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 26527b95a09..38a4a38fb13 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -228,9 +228,12 @@ module Gitlab def oauth_access_token_check(password) if password.present? - token = Doorkeeper::AccessToken.by_token(password) + token = OauthAccessToken.by_token(password) if valid_oauth_token?(token) + identity = ::Gitlab::Auth::Identity.link_from_oauth_token(token) + return if identity && !identity.valid? + user = User.id_in(token.resource_owner_id).first return unless user && user.can_log_in_with_non_expired_password? diff --git a/lib/gitlab/diff/file_collection/commit.rb b/lib/gitlab/diff/file_collection/commit.rb index 0f8f408d326..6f5c1a385e4 100644 --- a/lib/gitlab/diff/file_collection/commit.rb +++ b/lib/gitlab/diff/file_collection/commit.rb @@ -4,6 +4,9 @@ module Gitlab module Diff module FileCollection class Commit < Base + # The maximum time allowed to highlight all the files in a commit (in seconds). + DEFAULT_LIMIT_HIGHLIGHT_COLLECTION = 10 + def initialize(commit, diff_options:) super(commit, project: commit.project, @@ -11,6 +14,29 @@ module Gitlab diff_refs: commit.diff_refs) end + # We need to preload the diffs highlighting to track every diff file + # and the time that they take to format. If the highlight rich collection + # limit is reached, then we render the rest of diff files + # as plain text to avoid saturating the resources. + def with_highlights_preloaded + @with_highlights_preloaded ||= begin + start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC, :second) + + diff_files.each do |diff_file| + current_time = Process.clock_gettime(Process::CLOCK_MONOTONIC, :second) + use_plain_highlight = current_time - start_time >= DEFAULT_LIMIT_HIGHLIGHT_COLLECTION + + diff_file.highlighted_diff_lines = Gitlab::Diff::Highlight.new( + diff_file, + repository: diff_file.repository, + plain: use_plain_highlight + ).highlight + end + + self + end + end + def cache_key ['commit', @diffable.id] end diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb index 39da9c4e7c8..4f491dbed0f 100644 --- a/lib/gitlab/diff/highlight.rb +++ b/lib/gitlab/diff/highlight.rb @@ -9,9 +9,10 @@ module Gitlab delegate :old_path, :new_path, :old_sha, :new_sha, to: :diff_file, prefix: :diff - def initialize(diff_lines, repository: nil) + def initialize(diff_lines, repository: nil, plain: false) @repository = repository @project = repository&.project + @plain = plain if diff_lines.is_a?(Gitlab::Diff::File) @diff_file = diff_lines @@ -42,6 +43,8 @@ module Gitlab private + attr_reader :plain + def populate_marker_ranges pair_selector = Gitlab::Diff::PairSelector.new(@raw_lines) @@ -77,7 +80,7 @@ module Gitlab def highlight_line(diff_line) return unless diff_file && diff_file.diff_refs - return diff_line_highlighting(diff_line, plain: true) if blobs_too_large? + return diff_line_highlighting(diff_line, plain: true) if blobs_too_large? || plain if Feature.enabled?(:diff_line_syntax_highlighting, project) diff_line_highlighting(diff_line) diff --git a/lib/gitlab/legacy_github_import/user_formatter.rb b/lib/gitlab/legacy_github_import/user_formatter.rb index 6bc70b13b6f..2483dba277b 100644 --- a/lib/gitlab/legacy_github_import/user_formatter.rb +++ b/lib/gitlab/legacy_github_import/user_formatter.rb @@ -25,12 +25,12 @@ module Gitlab end def gitlab_id - project.import_data.user_mapping_enabled? ? gitlab_user&.id : find_by_email + user_mapping_enabled? ? gitlab_user&.id : find_by_email end strong_memoize_attr :gitlab_id def source_user - return if !project.import_data.user_mapping_enabled? || ghost_user? + return if !user_mapping_enabled? || ghost_user? source_user_mapper.find_or_create_source_user( source_name: gitea_user[:full_name].presence || gitea_user[:login], @@ -78,6 +78,11 @@ module Gitlab source_user.mapped_user end strong_memoize_attr :gitlab_user + + def user_mapping_enabled? + project.import_data.reset.user_mapping_enabled? + end + strong_memoize_attr :user_mapping_enabled? end end end diff --git a/lib/gitlab/pages.rb b/lib/gitlab/pages.rb index c8e3e92f5d6..92796f029b5 100644 --- a/lib/gitlab/pages.rb +++ b/lib/gitlab/pages.rb @@ -56,7 +56,9 @@ module Gitlab def generate_unique_domain(project) 10.times do pages_unique_domain = Gitlab::Pages::RandomDomain.generate(project_path: project.path) - return pages_unique_domain unless ProjectSetting.unique_domain_exists?(pages_unique_domain) + return pages_unique_domain unless + ProjectSetting.unique_domain_exists?(pages_unique_domain) || + Namespace.top_level.by_path(pages_unique_domain).present? end raise UniqueDomainGenerationFailure diff --git a/lib/gitlab/template_parser/parser.rb b/lib/gitlab/template_parser/parser.rb index f3aca909609..545086f7d7c 100644 --- a/lib/gitlab/template_parser/parser.rb +++ b/lib/gitlab/template_parser/parser.rb @@ -164,12 +164,14 @@ module Gitlab end def parse_and_transform(input) - AST::Transformer.new.apply(parse(input)) + Timeout.timeout(2.seconds) { AST::Transformer.new.apply(parse(input)) } rescue Parslet::ParseFailed => ex # We raise a custom error so it's easier to catch different parser # related errors. In addition, this ensures the caller of this method # doesn't depend on a Parslet specific error class. raise Error, "Failed to parse the template: #{ex.message}" + rescue Timeout::Error + raise Error, 'Template parser timeout. Consider reducing the size of the template' end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4ff48bbf5f9..6cc5ed9ebc4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4931,9 +4931,6 @@ msgstr "" msgid "AdminUsers|user cap" msgstr "" -msgid "Administrator" -msgstr "" - msgid "Administrator Account Setup" msgstr "" @@ -7458,9 +7455,6 @@ msgstr "" msgid "Are you sure you want to %{action} %{name}?" msgstr "" -msgid "Are you sure you want to %{action} this directory?" -msgstr "" - msgid "Are you sure you want to approve %{user}?" msgstr "" @@ -46181,18 +46175,9 @@ msgstr "" msgid "Replace all labels" msgstr "" -msgid "Replace audio" -msgstr "" - msgid "Replace file" msgstr "" -msgid "Replace image" -msgstr "" - -msgid "Replace video" -msgstr "" - msgid "Replaced all labels with %{label_references} %{label_text}." msgstr "" @@ -64232,6 +64217,9 @@ msgstr "" msgid "You do not have permission to leave this %{namespaceType}." msgstr "" +msgid "You do not have permission to lock this" +msgstr "" + msgid "You do not have permission to run a pipeline on this branch." msgstr "" diff --git a/qa/qa/resource/project.rb b/qa/qa/resource/project.rb index b5d0e949dcb..1911a61f0ef 100644 --- a/qa/qa/resource/project.rb +++ b/qa/qa/resource/project.rb @@ -297,6 +297,18 @@ module QA end end + def change_pipeline_variables_minimum_override_role(new_role) + response = put(request_url(api_put_path), ci_pipeline_variables_minimum_override_role: new_role) + + return if response.code == HTTP_STATUS_OK + + raise( + ResourceUpdateFailedError, + "Failed to update pipeline_variables_minimum_override_role to '#{new_role}'. " \ + "Response (#{response.code}): `#{response}`." + ) + end + def change_repository_storage(new_storage) response = put(request_url(api_put_path), repository_storage: new_storage) diff --git a/qa/qa/specs/features/browser_ui/4_verify/ci_variable/custom_variable_spec.rb b/qa/qa/specs/features/browser_ui/4_verify/ci_variable/custom_variable_spec.rb index 6451aa7c611..b8f0b93e49b 100644 --- a/qa/qa/specs/features/browser_ui/4_verify/ci_variable/custom_variable_spec.rb +++ b/qa/qa/specs/features/browser_ui/4_verify/ci_variable/custom_variable_spec.rb @@ -31,6 +31,8 @@ module QA before do Flow::Login.sign_in + project.change_pipeline_variables_minimum_override_role('developer') + project.visit! Page::Project::Menu.perform(&:go_to_pipelines) Page::Project::Pipeline::Index.perform(&:click_run_pipeline_button) diff --git a/qa/qa/specs/features/browser_ui/4_verify/pipeline/pass_dotenv_variables_to_downstream_via_bridge_spec.rb b/qa/qa/specs/features/browser_ui/4_verify/pipeline/pass_dotenv_variables_to_downstream_via_bridge_spec.rb index b687e6ab205..9d55dd5456e 100644 --- a/qa/qa/specs/features/browser_ui/4_verify/pipeline/pass_dotenv_variables_to_downstream_via_bridge_spec.rb +++ b/qa/qa/specs/features/browser_ui/4_verify/pipeline/pass_dotenv_variables_to_downstream_via_bridge_spec.rb @@ -14,6 +14,9 @@ module QA Flow::Login.sign_in add_ci_file(downstream_project, downstream_ci_file) add_ci_file(upstream_project, upstream_ci_file) + + upstream_project.change_pipeline_variables_minimum_override_role('developer') + downstream_project.change_pipeline_variables_minimum_override_role('developer') upstream_project.visit! Flow::Pipeline.visit_latest_pipeline(status: 'Passed') end diff --git a/qa/qa/specs/features/shared_contexts/variable_inheritance_shared_context.rb b/qa/qa/specs/features/shared_contexts/variable_inheritance_shared_context.rb index be28c9d69d8..c83efd7051c 100644 --- a/qa/qa/specs/features/shared_contexts/variable_inheritance_shared_context.rb +++ b/qa/qa/specs/features/shared_contexts/variable_inheritance_shared_context.rb @@ -31,6 +31,9 @@ module QA before do Flow::Login.sign_in + upstream_project.change_pipeline_variables_minimum_override_role('developer') + downstream1_project.change_pipeline_variables_minimum_override_role('developer') + downstream2_project.change_pipeline_variables_minimum_override_role('developer') end after do diff --git a/scripts/frontend/quarantined_vue3_specs.txt b/scripts/frontend/quarantined_vue3_specs.txt index 0b8268acd69..14c0ef9ed39 100644 --- a/scripts/frontend/quarantined_vue3_specs.txt +++ b/scripts/frontend/quarantined_vue3_specs.txt @@ -111,7 +111,6 @@ ee/spec/frontend/vue_shared/components/projects_list/projects_list_item_spec.js ee/spec/frontend/vue_shared/security_reports/components/security_training_promo_spec.js ee/spec/frontend/vulnerabilities/generic_report/types/list_graphql_spec.js ee/spec/frontend/vulnerabilities/related_issues_spec.js -ee/spec/frontend/vulnerabilities/vulnerability_file_contents_spec.js spec/frontend/__helpers__/vue_test_utils_helper_spec.js spec/frontend/access_tokens/index_spec.js spec/frontend/add_context_commits_modal/components/add_context_commits_modal_spec.js diff --git a/spec/config/application_spec.rb b/spec/config/application_spec.rb index 3658ff68c74..be136c11b5c 100644 --- a/spec/config/application_spec.rb +++ b/spec/config/application_spec.rb @@ -27,6 +27,7 @@ RSpec.describe Gitlab::Application, feature_category: :scalability do # rubocop: '/?safe[note]=secret&target_type=1' | { 'safe' => { 'note' => filtered }, 'target_type' => '1' } '/?safe[selectedText]=secret' | { 'safe' => { 'selectedText' => filtered } } '/?selectedText=secret' | { 'selectedText' => filtered } + '/?query=secret' | { 'query' => filtered } end with_them do diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index 9e0ed9ce621..028dffcb2e2 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -314,6 +314,46 @@ RSpec.describe OmniauthCallbacksController, type: :controller, feature_category: it_behaves_like 'omniauth sign in that remembers user with two factor enabled' end + + context 'when redirect fragment contains special characters' do + before do + request.env['omniauth.params'] = { 'redirect_fragment' => 'confirm-merge_request_diff_id-context' } + end + + it 'redirects with fragment' do + post provider, session: { user_return_to: '/fake/url' } + + expect(response).to redirect_to('/fake/url#confirm-merge_request_diff_id-context') + end + end + + context 'when stored redirect fragment is malicious' do + let(:malicious_redirect_fragment) { '#code=test_code&' } + + before do + request.env['omniauth.params'] = { 'redirect_fragment' => malicious_redirect_fragment } + end + + it 'fails login and redirects to login path' do + post provider, session: { user_return_to: '/fake/url#replaceme' } + + expect(response.redirect?).to be true + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to match(/Invalid state/) + end + + context 'when fragment has encoded content' do + let_it_be(:malicious_redirect_fragment, reload: true) { '#code%3Dtest_code&L90' } + + it 'fails login and redirects to login path' do + post provider, session: { user_return_to: '/fake/url#replaceme' } + + expect(response.redirect?).to be true + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to match(/Invalid state/) + end + end + end end context 'with strategies' do @@ -720,7 +760,7 @@ RSpec.describe OmniauthCallbacksController, type: :controller, feature_category: post :saml, params: { SAMLResponse: mock_saml_response } - expect(flash[:alert]).to eq('Signing in using your SAML account without a pre-existing account in localhost is not allowed. Create an account in localhost first, and then connect it to your SAML account.') + expect(flash[:alert]).to eq("Signing in using your SAML account without a pre-existing account in #{Gitlab.config.gitlab.host} is not allowed. Create an account in #{Gitlab.config.gitlab.host} first, and then connect it to your SAML account.") expect(response).to redirect_to(new_user_registration_path) end end @@ -735,7 +775,7 @@ RSpec.describe OmniauthCallbacksController, type: :controller, feature_category: post :saml, params: { SAMLResponse: mock_saml_response } - expect(flash[:alert]).to eq('Signing in using your SAML account without a pre-existing account in localhost is not allowed.') + expect(flash[:alert]).to eq("Signing in using your SAML account without a pre-existing account in #{Gitlab.config.gitlab.host} is not allowed.") expect(response).to redirect_to(new_user_session_path) end end diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index bcf40e24dae..d94fea1d675 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -459,7 +459,7 @@ RSpec.describe Projects::CommitController, feature_category: :source_code_manage { namespace_id: project.namespace, project_id: project, - id: commit.id, + id: master_pickable_sha, format: format } end @@ -471,6 +471,25 @@ RSpec.describe Projects::CommitController, feature_category: :source_code_manage expect(assigns(:environment)).to be_nil end + it 'preloads highlights' do + allow(Process).to receive(:clock_gettime).and_call_original + allow(Process).to receive(:clock_gettime).with(Process::CLOCK_MONOTONIC, :second).and_return(0, 4, 8, 12, 16, 20) + + diff_highlight = instance_double(Gitlab::Diff::Highlight, highlight: []) + allow(Gitlab::Diff::Highlight).to receive(:new).and_return(diff_highlight) + + send_request + + assigns(:diffs).diff_files.each do |diff_file| + expect(diff_file.instance_variable_get(:@highlighted_diff_lines)).not_to be_nil + end + + expect(Gitlab::Diff::Highlight) + .to have_received(:new).with(anything, hash_including(plain: false)).twice.times + expect(Gitlab::Diff::Highlight) + .to have_received(:new).with(anything, hash_including(plain: true)).exactly(4).times + end + context 'when format is not html' do let(:format) { :json } diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index b6db1ab8097..e9b256ad501 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -16,6 +16,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu let_it_be(:guest) { create(:user) } before_all do + project.update!(ci_pipeline_variables_minimum_override_role: :developer) project.add_owner(owner) project.add_maintainer(maintainer) project.add_developer(developer) diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index 91e25293122..3c6933bab2c 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -10,6 +10,10 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu let_it_be_with_reload(:project) { create(:project, :public, :repository, developers: user) } let_it_be_with_reload(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) } + before do + project.update!(ci_pipeline_variables_minimum_override_role: :developer) + end + shared_examples 'access update schedule' do describe 'security' do it 'is allowed for admin when admin mode enabled', :enable_admin_mode do @@ -178,7 +182,8 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu context 'when the user is not allowed to create a pipeline schedule with variables' do before do - project.update!(restrict_user_defined_variables: true) + project.update!(restrict_user_defined_variables: true, + ci_pipeline_variables_minimum_override_role: :maintainer) end it 'does not create a new schedule' do @@ -272,7 +277,8 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu context 'when the user is not allowed to update pipeline schedule variables' do before do - project.update!(restrict_user_defined_variables: true) + project.update!(restrict_user_defined_variables: true, + ci_pipeline_variables_minimum_override_role: :maintainer) end it 'does not update the schedule' do diff --git a/spec/features/projects/jobs/user_triggers_manual_job_with_variables_spec.rb b/spec/features/projects/jobs/user_triggers_manual_job_with_variables_spec.rb index 3beea5bf269..8a265f7daa3 100644 --- a/spec/features/projects/jobs/user_triggers_manual_job_with_variables_spec.rb +++ b/spec/features/projects/jobs/user_triggers_manual_job_with_variables_spec.rb @@ -10,6 +10,7 @@ RSpec.describe 'User triggers manual job with variables', :js, feature_category: let!(:build) { create(:ci_build, :manual, pipeline: pipeline) } before do + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) project.add_maintainer(user) project.enable_ci diff --git a/spec/features/projects/pipeline_schedules_spec.rb b/spec/features/projects/pipeline_schedules_spec.rb index c95c05b3700..0dbe8021d67 100644 --- a/spec/features/projects/pipeline_schedules_spec.rb +++ b/spec/features/projects/pipeline_schedules_spec.rb @@ -12,6 +12,10 @@ RSpec.describe 'Pipeline Schedules', :js, feature_category: :continuous_integrat let!(:user) { create(:user) } let!(:maintainer) { create(:user) } + before do + project.update!(ci_pipeline_variables_minimum_override_role: :developer) + end + context 'logged in as the pipeline schedule owner' do before do project.add_developer(user) diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb index 70a47806ad2..4ece24e14e5 100644 --- a/spec/features/projects/pipelines/pipelines_spec.rb +++ b/spec/features/projects/pipelines/pipelines_spec.rb @@ -789,6 +789,10 @@ RSpec.describe 'Pipelines', :js, feature_category: :continuous_integration do end context 'when variables are specified' do + before do + project.update!(ci_pipeline_variables_minimum_override_role: :developer) + end + it 'creates a new pipeline with variables' do within_testid('ci-variable-row-container') do find_by_testid('pipeline-form-ci-variable-key-field').set('key_name') diff --git a/spec/frontend/content_editor/components/bubble_menus/media_bubble_menu_spec.js b/spec/frontend/content_editor/components/bubble_menus/media_bubble_menu_spec.js index 50d9bbec014..fe722ad6ece 100644 --- a/spec/frontend/content_editor/components/bubble_menus/media_bubble_menu_spec.js +++ b/spec/frontend/content_editor/components/bubble_menus/media_bubble_menu_spec.js @@ -9,12 +9,7 @@ import Audio from '~/content_editor/extensions/audio'; import DrawioDiagram from '~/content_editor/extensions/drawio_diagram'; import Image from '~/content_editor/extensions/image'; import Video from '~/content_editor/extensions/video'; -import { - createTestEditor, - emitEditorEvent, - mockChainedCommands, - createTransactionWithMeta, -} from '../../test_utils'; +import { createTestEditor, emitEditorEvent, createTransactionWithMeta } from '../../test_utils'; import { PROJECT_WIKI_ATTACHMENT_IMAGE_HTML, PROJECT_WIKI_ATTACHMENT_AUDIO_HTML, @@ -83,14 +78,6 @@ describe.each` return showMenu(); }; - const selectFile = async (file) => { - const input = wrapper.findComponent({ ref: 'fileSelector' }); - - // override the property definition because `input.files` isn't directly modifyable - Object.defineProperty(input.element, 'files', { value: [file], writable: true }); - await input.trigger('change'); - }; - const expectLinkButtonsToExist = (exist = true) => { expect(wrapper.findComponent(GlLink).exists()).toBe(exist); expect(wrapper.findByTestId('edit-media').exists()).toBe(exist); @@ -177,44 +164,6 @@ describe.each` }); }); - describe(`replace ${mediaType} button`, () => { - beforeEach(buildWrapperAndDisplayMenu); - - if (mediaType !== 'drawioDiagram') { - it('uploads and replaces the selected image when file input changes', async () => { - const commands = mockChainedCommands(tiptapEditor, [ - 'focus', - 'deleteSelection', - 'uploadAttachment', - 'run', - ]); - const file = new File(['foo'], 'foo.png', { type: 'image/png' }); - - await wrapper.findByTestId('replace-media').vm.$emit('click'); - await selectFile(file); - - expect(commands.focus).toHaveBeenCalled(); - expect(commands.deleteSelection).toHaveBeenCalled(); - expect(commands.uploadAttachment).toHaveBeenCalledWith({ file }); - expect(commands.run).toHaveBeenCalled(); - }); - } else { - // draw.io diagrams are replaced using the edit diagram button - it('invokes editDiagram command', async () => { - const commands = mockChainedCommands(tiptapEditor, [ - 'focus', - 'createOrEditDiagram', - 'run', - ]); - await wrapper.findByTestId('edit-diagram').vm.$emit('click'); - - expect(commands.focus).toHaveBeenCalled(); - expect(commands.createOrEditDiagram).toHaveBeenCalled(); - expect(commands.run).toHaveBeenCalled(); - }); - } - }); - describe('edit button', () => { let mediaSrcInput; let mediaAltInput; diff --git a/spec/frontend/content_editor/extensions/link_spec.js b/spec/frontend/content_editor/extensions/link_spec.js index 4a7a83f1eb2..3b7b2706250 100644 --- a/spec/frontend/content_editor/extensions/link_spec.js +++ b/spec/frontend/content_editor/extensions/link_spec.js @@ -2,6 +2,9 @@ import { builders } from 'prosemirror-test-builder'; import Link from '~/content_editor/extensions/link'; import { createTestEditor, triggerMarkInputRule } from '../test_utils'; +const GFM_LINK_HTML = + '

test

'; + describe('content_editor/extensions/link', () => { let tiptapEditor; let doc; @@ -13,10 +16,6 @@ describe('content_editor/extensions/link', () => { ({ doc, paragraph: p, link } = builders(tiptapEditor.schema)); }); - afterEach(() => { - tiptapEditor.destroy(); - }); - it.each` input | insertedNode ${'[gitlab](https://gitlab.com)'} | ${() => p(link({ href: 'https://gitlab.com' }, 'gitlab'))} @@ -32,4 +31,15 @@ describe('content_editor/extensions/link', () => { expect(tiptapEditor.getJSON()).toEqual(expectedDoc.toJSON()); }); + + describe('when parsing HTML', () => { + it('ignores titles for links with "gfm" class in it', () => { + const expectedDoc = doc( + p(link({ href: 'https://gitlab.com/gitlab-org/gitlab-test/-/issues/1' }, 'test')), + ); + tiptapEditor.commands.setContent(GFM_LINK_HTML); + + expect(tiptapEditor.getJSON()).toEqual(expectedDoc.toJSON()); + }); + }); }); diff --git a/spec/frontend/lib/apollo/sentry_breadcrumb_link_spec.js b/spec/frontend/lib/apollo/sentry_breadcrumb_link_spec.js new file mode 100644 index 00000000000..5982385b7e7 --- /dev/null +++ b/spec/frontend/lib/apollo/sentry_breadcrumb_link_spec.js @@ -0,0 +1,52 @@ +import { gql, execute, ApolloLink, Observable } from '@apollo/client/core'; +import * as Sentry from '~/sentry/sentry_browser_wrapper'; + +import { sentryBreadcrumbLink } from '~/lib/apollo/sentry_breadcrumb_link'; + +jest.mock('~/sentry/sentry_browser_wrapper'); + +const executeQuery = async (query, correlationId) => { + const terminatingLink = new ApolloLink(() => + Observable.of({ data: { things: 1 }, correlationId }), + ); + const mockLink = sentryBreadcrumbLink.concat(terminatingLink); + + await new Promise((resolve) => { + execute(mockLink, { query }).subscribe(resolve); + }); +}; + +describe('sentryBreadcrumbLink', () => { + describe('with a named query', () => { + const QUERY = gql` + query getThings { + things + } + `; + + beforeEach(async () => { + await executeQuery(QUERY, 'my-correlation-id'); + }); + + it('addBreadcrumb is called', () => { + expect(Sentry.addBreadcrumb).toHaveBeenCalledTimes(2); + + expect(Sentry.addBreadcrumb).toHaveBeenNthCalledWith(1, { + level: 'info', + category: 'graphql.request', + data: { + operationName: 'getThings', + }, + }); + + expect(Sentry.addBreadcrumb).toHaveBeenNthCalledWith(2, { + level: 'info', + category: 'graphql.response', + data: { + operationName: 'getThings', + correlationId: 'my-correlation-id', + }, + }); + }); + }); +}); diff --git a/spec/frontend/ml/model_registry/apps/show_ml_model_version_spec.js b/spec/frontend/ml/model_registry/apps/show_ml_model_version_spec.js index cd910518dda..2f606e10616 100644 --- a/spec/frontend/ml/model_registry/apps/show_ml_model_version_spec.js +++ b/spec/frontend/ml/model_registry/apps/show_ml_model_version_spec.js @@ -141,9 +141,10 @@ describe('ml/model_registry/apps/show_model_version.vue', () => { it('displays model version edit button', () => { expect(findModelVersionEditButton().props()).toMatchObject({ - variant: 'confirm', - category: 'primary', + variant: 'default', + category: 'secondary', }); + expect(findModelVersionEditButton().text()).toBe('Edit'); }); describe('when user has no permission to write model registry', () => { diff --git a/spec/frontend/sentry/init_sentry_spec.js b/spec/frontend/sentry/init_sentry_spec.js index c51e2de27a2..e86e2d0b3ce 100644 --- a/spec/frontend/sentry/init_sentry_spec.js +++ b/spec/frontend/sentry/init_sentry_spec.js @@ -1,5 +1,5 @@ /* eslint-disable no-restricted-imports */ -import { captureException, SDK_VERSION } from '@sentry/browser'; +import { captureException, addBreadcrumb, SDK_VERSION } from '@sentry/browser'; import * as Sentry from '@sentry/browser'; import { initSentry } from '~/sentry/init_sentry'; @@ -111,6 +111,7 @@ describe('SentryConfig', () => { // eslint-disable-next-line no-underscore-dangle expect(window._Sentry).toEqual({ captureException, + addBreadcrumb, SDK_VERSION, }); }); diff --git a/spec/frontend/sentry/sentry_browser_wrapper_spec.js b/spec/frontend/sentry/sentry_browser_wrapper_spec.js index 60c441fe83c..052afa10d92 100644 --- a/spec/frontend/sentry/sentry_browser_wrapper_spec.js +++ b/spec/frontend/sentry/sentry_browser_wrapper_spec.js @@ -3,6 +3,7 @@ import * as Sentry from '~/sentry/sentry_browser_wrapper'; const mockError = new Error('error!'); +const mockBreadcrumb = { category: 'mockCategory' }; describe('SentryBrowserWrapper', () => { beforeAll(() => { @@ -15,10 +16,12 @@ describe('SentryBrowserWrapper', () => { beforeEach(() => { jest.spyOn(console, 'error').mockImplementation(); + jest.spyOn(console, 'debug').mockImplementation(); }); afterEach(() => { console.error.mockRestore(); + console.debug.mockRestore(); // eslint-disable-next-line no-underscore-dangle delete window._Sentry; @@ -35,17 +38,31 @@ describe('SentryBrowserWrapper', () => { { 0: mockError }, ); }); + + it('addBreadcrumb will report to console instead', () => { + Sentry.addBreadcrumb(mockBreadcrumb); + + expect(console.debug).toHaveBeenCalledTimes(1); + expect(console.debug).toHaveBeenCalledWith( + '[Sentry stub]', + 'addBreadcrumb(...) called with:', + { 0: mockBreadcrumb }, + ); + }); }); describe('when _Sentry is defined', () => { let mockCaptureException; + let mockAddBreadcrumb; beforeEach(() => { mockCaptureException = jest.fn(); + mockAddBreadcrumb = jest.fn(); // eslint-disable-next-line no-underscore-dangle window._Sentry = { captureException: mockCaptureException, + addBreadcrumb: mockAddBreadcrumb, }; }); @@ -54,5 +71,11 @@ describe('SentryBrowserWrapper', () => { expect(mockCaptureException).toHaveBeenCalledWith(mockError); }); + + it('addBreadcrumb is called', () => { + Sentry.addBreadcrumb(mockBreadcrumb); + + expect(mockAddBreadcrumb).toHaveBeenCalledWith(mockBreadcrumb); + }); }); }); diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index b84c776ca52..464e2e8796b 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -1242,6 +1242,26 @@ RSpec.describe GitlabSchema.types['Project'], feature_category: :groups_and_proj expect(protectable_branches).to match_array(branch_names) end end + + context 'without read_code permissions' do + before do + project.add_guest(current_user) + end + + it 'returns an empty array' do + expect(protectable_branches).to be_nil + end + end + + context 'with read_code permissions' do + before do + project.add_member(current_user, Gitlab::Access::REPORTER) + end + + it 'returns all the branch names' do + expect(protectable_branches).to match_array(branch_names) + end + end end end diff --git a/spec/lib/api/entities/virtual_registries/packages/maven/cached_response_spec.rb b/spec/lib/api/entities/virtual_registries/packages/maven/cached_response_spec.rb index 4fbb1c7831d..286bbcce6bc 100644 --- a/spec/lib/api/entities/virtual_registries/packages/maven/cached_response_spec.rb +++ b/spec/lib/api/entities/virtual_registries/packages/maven/cached_response_spec.rb @@ -8,7 +8,7 @@ RSpec.describe API::Entities::VirtualRegistries::Packages::Maven::CachedResponse subject { described_class.new(cached_response).as_json } it 'has the expected attributes' do - is_expected.to include(:cached_response_id, :group_id, :upstream_id, :upstream_checked_at, :created_at, :updated_at, + is_expected.to include(:id, :group_id, :upstream_id, :upstream_checked_at, :created_at, :updated_at, :file, :file_md5, :file_sha1, :size, :relative_path, :upstream_etag, :content_type) end end diff --git a/spec/lib/gitlab/auth/auth_finders_spec.rb b/spec/lib/gitlab/auth/auth_finders_spec.rb index 91a609371e2..db2f4830e75 100644 --- a/spec/lib/gitlab/auth/auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/auth_finders_spec.rb @@ -130,11 +130,17 @@ RSpec.describe Gitlab::Auth::AuthFinders, feature_category: :system_access do end context 'with oauth token' do - let(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) } - let(:doorkeeper_access_token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'api', organization_id: organization.id) } + let_it_be(:oauth_application) { create(:oauth_application, owner: user) } + let(:oauth_access_token) do + create(:oauth_access_token, + application_id: oauth_application.id, + resource_owner_id: user.id, + scopes: 'api', + organization_id: organization.id) + end before do - set_bearer_token(doorkeeper_access_token.plaintext_token) + set_bearer_token(oauth_access_token.plaintext_token) end it { is_expected.to eq user } @@ -708,18 +714,11 @@ RSpec.describe Gitlab::Auth::AuthFinders, feature_category: :system_access do end describe '#find_oauth_access_token' do + let_it_be(:oauth_application) { create(:oauth_application, owner: user) } let(:scopes) { 'api' } - - let(:application) do - Doorkeeper::Application.create!( - name: 'MyApp', - redirect_uri: 'https://app.com', - owner: user) - end - - let(:doorkeeper_access_token) do - Doorkeeper::AccessToken.create!( - application_id: application.id, + let(:oauth_access_token) do + create(:oauth_access_token, + application_id: oauth_application.id, resource_owner_id: user.id, scopes: scopes, organization_id: organization.id) @@ -727,19 +726,19 @@ RSpec.describe Gitlab::Auth::AuthFinders, feature_category: :system_access do context 'passed as header' do before do - set_bearer_token(doorkeeper_access_token.plaintext_token) + set_bearer_token(oauth_access_token.plaintext_token) end it 'returns token if valid oauth_access_token' do - expect(find_oauth_access_token.token).to eq doorkeeper_access_token.token + expect(find_oauth_access_token.token).to eq oauth_access_token.token end end context 'passed as param' do it 'returns user if valid oauth_access_token' do - set_param(:access_token, doorkeeper_access_token.plaintext_token) + set_param(:access_token, oauth_access_token.plaintext_token) - expect(find_oauth_access_token.token).to eq doorkeeper_access_token.token + expect(find_oauth_access_token.token).to eq oauth_access_token.token end end @@ -765,7 +764,7 @@ RSpec.describe Gitlab::Auth::AuthFinders, feature_category: :system_access do user.username == 'gitlab-duo' end - set_bearer_token(doorkeeper_access_token.plaintext_token) + set_bearer_token(oauth_access_token.plaintext_token) end context 'when scoped user is specified' do @@ -773,7 +772,7 @@ RSpec.describe Gitlab::Auth::AuthFinders, feature_category: :system_access do context 'when linking composite identitiy succeeds' do it 'returns the oauth token' do - expect(find_oauth_access_token.token).to eq(doorkeeper_access_token.token) + expect(find_oauth_access_token.token).to eq(oauth_access_token.token) end end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 75871fc5b83..333bdc22870 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -487,16 +487,28 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching, feature_cate end end - context 'while using OAuth tokens as passwords' do + describe 'using OAuth tokens as passwords' do let_it_be(:organization) { create(:organization) } + let(:user) { create(:user, organizations: [organization]) } let(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) } + let(:scopes) { 'api' } + + let(:token) do + Doorkeeper::AccessToken.create!( + application_id: application.id, + resource_owner_id: user.id, + scopes: scopes, + organization_id: organization.id).plaintext_token + end + + def authenticate(username:, password:) + gl_auth.find_for_git_client(username, password, project: nil, request: request) + end shared_examples 'an oauth failure' do it 'fails' do - access_token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'api', organization_id: organization.id) - - expect(gl_auth.find_for_git_client("oauth2", access_token.token, project: nil, request: request)) + expect(authenticate(username: "oauth2", password: token)) .to have_attributes(auth_failure) end end @@ -522,27 +534,28 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching, feature_cate with_them do it 'authenticates with correct abilities' do - access_token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: scopes, organization_id: organization.id) - - expect(gl_auth.find_for_git_client("oauth2", access_token.token, project: nil, request: request)) + expect(authenticate(username: 'oauth2', password: token)) .to have_attributes(actor: user, project: nil, type: :oauth, authentication_abilities: abilities) end it 'authenticates with correct abilities without special username' do - access_token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: scopes, organization_id: organization.id) + expect(authenticate(username: user.username, password: token)) + .to have_attributes(actor: user, project: nil, type: :oauth, authentication_abilities: abilities) + end - expect(gl_auth.find_for_git_client(user.username, access_token.token, project: nil, request: request)) + it 'tracks any composite identity' do + expect(::Gitlab::Auth::Identity).to receive(:link_from_oauth_token).and_call_original + + expect(authenticate(username: "oauth2", password: token)) .to have_attributes(actor: user, project: nil, type: :oauth, authentication_abilities: abilities) end end end it 'does not try password auth before oauth' do - access_token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'api', organization_id: organization.id) - expect(gl_auth).not_to receive(:find_with_user_password) - gl_auth.find_for_git_client("oauth2", access_token.token, project: nil, request: request) + authenticate(username: "oauth2", password: token) end context 'blocked user' do diff --git a/spec/lib/gitlab/background_migration/backfill_ci_runners_partitioned_table_spec.rb b/spec/lib/gitlab/background_migration/backfill_ci_runners_partitioned_table_spec.rb index 1223e7efb40..2600eeb4777 100644 --- a/spec/lib/gitlab/background_migration/backfill_ci_runners_partitioned_table_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_ci_runners_partitioned_table_spec.rb @@ -3,9 +3,7 @@ require 'spec_helper' RSpec.describe Gitlab::BackgroundMigration::BackfillCiRunnersPartitionedTable, - feature_category: :runner, - schema: 20241023144448, - migration: :gitlab_ci do + feature_category: :runner, migration: :gitlab_ci do let(:connection) { Ci::ApplicationRecord.connection } describe '#perform' do @@ -27,24 +25,19 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillCiRunnersPartitionedTable, end before do - # Don't sync records to partitioned table connection.execute <<~SQL - DROP TRIGGER table_sync_trigger_61879721b5 ON ci_runners; - SQL + BEGIN; + ALTER TABLE ci_runners DISABLE TRIGGER ALL; -- Don't sync records to partitioned table - runners.create!(runner_type: 1) - runners.create!(runner_type: 2, sharding_key_id: 89) - runners.create!(runner_type: 2, sharding_key_id: nil) - runners.create!(runner_type: 3, sharding_key_id: 10) - runners.create!(runner_type: 3, sharding_key_id: nil) - runners.create!(runner_type: 3, sharding_key_id: 100) + INSERT INTO ci_runners(runner_type) VALUES (1); + INSERT INTO ci_runners(runner_type, sharding_key_id) VALUES (2, 89); + INSERT INTO ci_runners(runner_type, sharding_key_id) VALUES (2, NULL); + INSERT INTO ci_runners(runner_type, sharding_key_id) VALUES (3, 10); + INSERT INTO ci_runners(runner_type, sharding_key_id) VALUES (3, NULL); + INSERT INTO ci_runners(runner_type, sharding_key_id) VALUES (3, 100); - ensure - connection.execute <<~SQL - CREATE TRIGGER table_sync_trigger_61879721b5 - AFTER INSERT OR DELETE OR UPDATE ON ci_runners - FOR EACH ROW - EXECUTE FUNCTION table_sync_function_686d6c7993 (); + ALTER TABLE ci_runners ENABLE TRIGGER ALL; + COMMIT; SQL end diff --git a/spec/lib/gitlab/ci/pipeline/chain/build/associations_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/build/associations_spec.rb index 81d0bea5747..16315796a93 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/build/associations_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/build/associations_spec.rb @@ -33,6 +33,10 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build::Associations, feature_categor let(:step) { described_class.new(pipeline, command) } + before do + project.update!(ci_pipeline_variables_minimum_override_role: :developer) + end + shared_examples 'breaks the chain' do it 'returns true' do step.perform! @@ -85,7 +89,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build::Associations, feature_categor context 'when project setting restrict_user_defined_variables is enabled' do before do - project.update!(restrict_user_defined_variables: true) + project.update!(restrict_user_defined_variables: true, ci_pipeline_variables_minimum_override_role: :maintainer) end context 'when user is developer' do diff --git a/spec/lib/gitlab/diff/highlight_spec.rb b/spec/lib/gitlab/diff/highlight_spec.rb index e9e65f64887..6bf2fab60e3 100644 --- a/spec/lib/gitlab/diff/highlight_spec.rb +++ b/spec/lib/gitlab/diff/highlight_spec.rb @@ -140,6 +140,17 @@ RSpec.describe Gitlab::Diff::Highlight, feature_category: :source_code_managemen expect(subject[2].rich_text).to be_html_safe end end + + context 'when blob highlight is plain' do + let(:subject) { described_class.new(diff_file, repository: project.repository, plain: true).highlight } + + it 'blobs are highlighted as plain text without loading all data' do + expect(diff_file.blob).not_to receive(:load_all_data!) + + expect(subject[2].rich_text).to eq(%{ def popen(cmd, path=nil)\n}) + expect(subject[2].rich_text).to be_html_safe + end + end end it_behaves_like 'diff highlighter' diff --git a/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb index c70661c80af..63dc5429314 100644 --- a/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::CommentFormatter, :clean_gitlab_redis_shared_state, feature_category: :importers do + include Import::GiteaHelper + let_it_be(:project) do create(:project, :with_import_url, :import_user_mapping_enabled, import_type: ::Import::SOURCE_GITEA) end @@ -191,7 +193,7 @@ RSpec.describe Gitlab::LegacyGithubImport::CommentFormatter, :clean_gitlab_redis let(:raw) { base.merge(user: octocat) } before do - allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?).and_return(false) + stub_user_mapping_chain(project, false) end context 'when author is a GitLab user' do @@ -277,7 +279,7 @@ RSpec.describe Gitlab::LegacyGithubImport::CommentFormatter, :clean_gitlab_redis context 'when user contribution mapping is disabled' do before do - allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?).and_return(false) + stub_user_mapping_chain(project, false) end it 'does not push any placeholder references' do diff --git a/spec/lib/gitlab/legacy_github_import/importer_spec.rb b/spec/lib/gitlab/legacy_github_import/importer_spec.rb index 40644b4d409..0bbda83d4e8 100644 --- a/spec/lib/gitlab/legacy_github_import/importer_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/importer_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::Importer, :clean_gitlab_redis_shared_state, feature_category: :importers do + include Import::GiteaHelper + subject(:importer) { described_class.new(project) } let_it_be(:api_root) { 'https://try.gitea.io/api/v1' } @@ -254,7 +256,7 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer, :clean_gitlab_redis_shared_ context 'when user contribution mapping is disabled' do before do - allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?).and_return(false) + stub_user_mapping_chain(project, false) end it 'does not enqueue the worker to load placeholder references' do diff --git a/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb index 1a795087312..cc61e6c07c7 100644 --- a/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter, :clean_gitlab_redis_shared_state, feature_category: :importers do + include Import::GiteaHelper + let_it_be(:project) do create( :project, @@ -148,7 +150,7 @@ RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter, :clean_gitlab_redis_s let(:raw_data) { base_data.merge(assignee: octocat) } before do - allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?).and_return(false) + stub_user_mapping_chain(project, false) end it 'returns nil as assignee_id when is not a GitLab user' do @@ -216,7 +218,7 @@ RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter, :clean_gitlab_redis_s let(:raw_data) { base_data.merge(user: octocat) } before do - allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?).and_return(false) + stub_user_mapping_chain(project, false) end it 'returns project creator_id as author_id when is not a GitLab user' do @@ -407,7 +409,7 @@ RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter, :clean_gitlab_redis_s context 'when user contribution mapping is disabled' do before do - allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?).and_return(false) + stub_user_mapping_chain(project, false) end it 'does not push any placeholder references' do diff --git a/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb index 78233dfce82..cdb98378803 100644 --- a/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter, :clean_gitlab_redis_shared_state, feature_category: :importers do + include Import::GiteaHelper + let_it_be(:project) do create( :project, @@ -192,7 +194,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter, :clean_gitlab_r let(:raw_data) { base_data.merge(assignee: octocat) } before do - allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?).and_return(false) + stub_user_mapping_chain(project, false) end it 'returns nil as assignee_id when is not a GitLab user' do @@ -239,7 +241,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter, :clean_gitlab_r let(:raw_data) { base_data.merge(user: octocat) } before do - allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?).and_return(false) + stub_user_mapping_chain(project, false) end it 'returns project creator_id as author_id when is not a GitLab user' do @@ -546,7 +548,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter, :clean_gitlab_r context 'when user contribution mapping is disabled' do before do - allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?).and_return(false) + stub_user_mapping_chain(project, false) end it 'does not push any placeholder references' do diff --git a/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb index 4b35925636b..7f650cf8116 100644 --- a/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::UserFormatter, feature_category: :importers do + include Import::GiteaHelper + let_it_be(:project) { create(:project, :import_user_mapping_enabled, import_type: 'gitea') } let_it_be(:source_user_mapper) do @@ -72,7 +74,7 @@ RSpec.describe Gitlab::LegacyGithubImport::UserFormatter, feature_category: :imp context 'when user contribution mapping is disabled' do before do allow(client).to receive(:user).and_return(gitea_user) - allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?).and_return(false) + stub_user_mapping_chain(project, false) end it 'returns GitLab user id when user confirmed primary email matches Gitea email' do @@ -118,7 +120,7 @@ RSpec.describe Gitlab::LegacyGithubImport::UserFormatter, feature_category: :imp context 'and improved user mapping is disabled' do before do allow(client).to receive(:user).and_return(ghost_user) - allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?).and_return(false) + stub_user_mapping_chain(project, false) end it 'returns nil' do @@ -193,7 +195,7 @@ RSpec.describe Gitlab::LegacyGithubImport::UserFormatter, feature_category: :imp context 'when user contribution mapping is disabled' do before do allow(client).to receive(:user).and_return(gitea_user) - allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?).and_return(false) + stub_user_mapping_chain(project, false) end it 'returns nil' do diff --git a/spec/lib/gitlab/middleware/strip_cookies_spec.rb b/spec/lib/gitlab/middleware/strip_cookies_spec.rb index f6cfd91a15b..531a757b70c 100644 --- a/spec/lib/gitlab/middleware/strip_cookies_spec.rb +++ b/spec/lib/gitlab/middleware/strip_cookies_spec.rb @@ -21,7 +21,7 @@ RSpec.describe Gitlab::Middleware::StripCookies, feature_category: :shared do let(:app) { mock_app.new } subject do - described_class.new(app, paths: [%r{^/assets/}, %r{^/v2$}, %r{^/v2/}]) + described_class.new(app, paths: [%r{^/assets/}]) end describe '#call' do @@ -36,10 +36,6 @@ RSpec.describe Gitlab::Middleware::StripCookies, feature_category: :shared do "/assets/test.css" | false "/something/assets/test.css" | true "/merge_requests/1" | true - "/v2" | false - "/v2/" | false - "/v2/something" | false - "/v2something" | true end with_them do diff --git a/spec/lib/gitlab/pages/virtual_host_finder_spec.rb b/spec/lib/gitlab/pages/virtual_host_finder_spec.rb index b6bd1740acf..2d2aa5360b6 100644 --- a/spec/lib/gitlab/pages/virtual_host_finder_spec.rb +++ b/spec/lib/gitlab/pages/virtual_host_finder_spec.rb @@ -123,9 +123,10 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do context 'when a project path conflicts with a unique domain' do it 'prioritizes the unique domain project' do - group = create(:group, path: 'unique-domain') + group = build(:group, path: 'unique-domain') + .tap { |g| g.save!(validate: false) } other_project = build(:project, path: 'unique-domain.example.com', group: group) - .tap { |project| project.save!(validate: false) } + .tap { |project| project.save!(validate: false) } create(:pages_deployment, project: project) create(:pages_deployment, project: other_project) diff --git a/spec/lib/gitlab/pages_spec.rb b/spec/lib/gitlab/pages_spec.rb index 1ed804f9cac..cfc28a84d8b 100644 --- a/spec/lib/gitlab/pages_spec.rb +++ b/spec/lib/gitlab/pages_spec.rb @@ -141,6 +141,24 @@ RSpec.describe Gitlab::Pages, feature_category: :pages do end end + RSpec.shared_examples 'generates a different unique domain' do |entity| + let!(:existing_entity) { create(entity, path: 'existing-path') } + + context "when #{entity} path is already in use" do + it 'assigns a different unique domain to pages' do + allow(Gitlab::Pages::RandomDomain).to receive(:generate).and_return('existing-path', 'new-unique-domain') + + described_class.add_unique_domain_to(project) + + expect(project.project_setting.pages_unique_domain_enabled).to eq(true) + expect(project.project_setting.pages_unique_domain).to eq('new-unique-domain') + end + end + end + + it_behaves_like 'generates a different unique domain', :group + it_behaves_like 'generates a different unique domain', :namespace + context 'when generated 10 unique domains are already in use' do it 'raises an error' do allow(Gitlab::Pages::RandomDomain).to receive(:generate).and_return('existing-domain') diff --git a/spec/lib/gitlab/template_parser/parser_spec.rb b/spec/lib/gitlab/template_parser/parser_spec.rb index 22247cbb693..f5aa8781d1a 100644 --- a/spec/lib/gitlab/template_parser/parser_spec.rb +++ b/spec/lib/gitlab/template_parser/parser_spec.rb @@ -74,5 +74,19 @@ RSpec.describe Gitlab::TemplateParser::Parser do expect { parser.parse_and_transform('{% each') } .to raise_error(Gitlab::TemplateParser::Error) end + + context 'when Parslet times out' do + before do + allow_next_instance_of(Gitlab::TemplateParser::AST::Transformer) do |instance| + allow(instance).to receive(:apply).and_raise(Timeout::Error) + end + end + + it 'raises a custom timeout error' do + expect { parser.parse_and_transform('foo') } + .to raise_error(Gitlab::TemplateParser::Error, + 'Template parser timeout. Consider reducing the size of the template') + end + end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 6d07498135a..e63b07b2f08 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -324,6 +324,16 @@ RSpec.describe Group, feature_category: :groups_and_projects do expect(group).not_to be_valid end + + it 'rejects paths already assigned to any pages unique domain' do + # Simulate the existing domain being in use + create(:project_setting, pages_unique_domain: 'existing-domain') + + group = build(:group, path: 'existing-domain') + + expect(group).not_to be_valid + expect(group.errors.full_messages.to_sentence).to eq('Group URL has already been taken') + end end describe '#notification_settings' do @@ -2829,6 +2839,18 @@ RSpec.describe Group, feature_category: :groups_and_projects do group.update!(name: 'new name') end end + + context 'when the path is changed to existing pages unique domain' do + let(:new_path) { 'existing-domain' } + + it 'rejects path' do + # Simulate the existing domain being in use + create(:project_setting, pages_unique_domain: 'existing-domain') + + expect(group.update(path: new_path)).to be_falsey + expect(group.errors.full_messages.to_sentence).to eq('Group URL has already been taken') + end + end end end diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index 95d955f75a3..fe5efa77123 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -451,4 +451,59 @@ RSpec.describe NamespaceSetting, feature_category: :groups_and_projects, type: : it { is_expected.not_to allow_value({ foo: 'bar' }).for(:default_branch_protection_defaults) } end end + + describe 'pipeline_variables_default_role' do + subject { group.namespace_settings.pipeline_variables_default_role } + + context 'validations' do + let(:namespace_settings) { build(:namespace_settings) } + + context 'when pipeline_variables_default_role is valid' do + it 'does not add an error' do + valid_roles = ProjectCiCdSetting::PIPELINE_VARIABLES_OVERRIDE_ROLES.keys.map(&:to_s) + + valid_roles.each do |role| + namespace_settings.pipeline_variables_default_role = role + expect(namespace_settings).to be_valid + end + end + end + end + + context 'when an invalid role is assigned to pipeline_variables_default_role' do + it 'raises an ArgumentError' do + expect do + namespace_settings.pipeline_variables_default_role = 'invalid_role' + end.to raise_error(ArgumentError, "'invalid_role' is not a valid pipeline_variables_default_role") + end + end + + context 'when namespace is root' do + let(:group) { create(:group) } + let(:variables_default_role) { group.namespace_settings.pipeline_variables_default_role } + + it { expect(variables_default_role).to eq('no_one_allowed') } + + context 'when feature flag `change_namespace_default_role_for_pipeline_variables` is disabled' do + before do + stub_feature_flags(change_namespace_default_role_for_pipeline_variables: false) + end + + it { expect(variables_default_role).to eq('developer') } + end + end + + context 'when namespace is not root' do + let(:root_group) { create(:group) } + + before do + group.parent = root_group + + root_settings = group.parent.namespace_settings + root_settings.pipeline_variables_default_role = 'maintainer' + end + + it { is_expected.to eq('maintainer') } + end + end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index d345397a03f..5f9b8ffe1b3 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -1522,12 +1522,14 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do it "cleans the path and makes sure it's available", time_travel_to: '2023-04-20 00:07 -0700' do create :user, username: "johngitlab-etc" create :namespace, path: "JohnGitLab-etc1" + create :project_setting, pages_unique_domain: 'existing-domain' [nil, 1, 2, 3].each do |count| create :namespace, path: "pickle#{count}" end expect(described_class.clean_path("-john+gitlab-ETC%.git@gmail.com")).to eq("johngitlab-ETC2") expect(described_class.clean_path("--%+--valid_*&%name=.git.%.atom.atom.@email.com")).to eq("valid_name") + expect(described_class.clean_path("existing-domain")).to eq("existing-domain1") # when we have more than MAX_TRIES count of a path use a more randomized suffix expect(described_class.clean_path("pickle@gmail.com")).to eq("pickle4") diff --git a/spec/models/project_ci_cd_setting_spec.rb b/spec/models/project_ci_cd_setting_spec.rb index 73b740bc3cd..e2a6186a613 100644 --- a/spec/models/project_ci_cd_setting_spec.rb +++ b/spec/models/project_ci_cd_setting_spec.rb @@ -6,6 +6,10 @@ RSpec.describe ProjectCiCdSetting, feature_category: :continuous_integration do using RSpec::Parameterized::TableSyntax describe 'validations' do + let(:project) { build(:project) } + + subject { described_class.new(project: project) } + it 'validates default_git_depth is between 0 and 1000 or nil' do expect(subject).to validate_numericality_of(:default_git_depth) .only_integer @@ -44,8 +48,54 @@ RSpec.describe ProjectCiCdSetting, feature_category: :continuous_integration do end describe '#pipeline_variables_minimum_override_role' do - it 'is maintainer by default' do - expect(described_class.new.pipeline_variables_minimum_override_role).to eq('maintainer') + shared_examples 'enables restrict_user_defined_variables' do + it 'enables restrict_user_defined_variables' do + expect(project.restrict_user_defined_variables).to be_truthy + end + end + + shared_examples 'sets the default ci_pipeline_variables_minimum_override_role' do |expected_role| + it "sets ci_pipeline_variables_minimum_override_role to #{expected_role}" do + expect(project.ci_pipeline_variables_minimum_override_role).to eq(expected_role) + end + end + + context 'when a namespace is defined' do + let_it_be(:project) { create(:project, :with_namespace_settings) } + + it_behaves_like 'sets the default ci_pipeline_variables_minimum_override_role', 'no_one_allowed' + + it_behaves_like 'enables restrict_user_defined_variables' + end + + context 'when a namespace is not defined' do + let_it_be(:project) { create(:project) } + + it_behaves_like 'sets the default ci_pipeline_variables_minimum_override_role', 'developer' + + it_behaves_like 'enables restrict_user_defined_variables' + end + + context 'when feature flag `change_namespace_default_role_for_pipeline_variables` is disabled' do + before do + stub_feature_flags(change_namespace_default_role_for_pipeline_variables: false) + end + + context 'and a namespace is defined' do + let(:project) { create(:project, :with_namespace_settings) } + + it_behaves_like 'sets the default ci_pipeline_variables_minimum_override_role', 'developer' + + it_behaves_like 'enables restrict_user_defined_variables' + end + + context 'and a namespace is not defined' do + let(:project) { create(:project) } + + it_behaves_like 'sets the default ci_pipeline_variables_minimum_override_role', 'developer' + + it_behaves_like 'enables restrict_user_defined_variables' + end end end @@ -79,15 +129,13 @@ RSpec.describe ProjectCiCdSetting, feature_category: :continuous_integration do describe '#default_git_depth' do let(:default_value) { described_class::DEFAULT_GIT_DEPTH } + let_it_be(:project) { create(:project) } it 'sets default value for new records' do - project = create(:project) - expect(project.ci_cd_settings.default_git_depth).to eq(default_value) end it 'does not set default value if present' do - project = build(:project) project.build_ci_cd_settings(default_git_depth: 0) project.save! diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d99a2218d84..0565061d0cf 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -695,6 +695,18 @@ RSpec.describe User, feature_category: :user_profile do end end + context 'when the username is assigned to another project pages unique domain' do + let(:username) { 'existing-domain' } + + it 'is invalid' do + # Simulate the existing domain being in use + create(:project_setting, pages_unique_domain: 'existing-domain') + + expect(user).not_to be_valid + expect(user.errors.full_messages).to eq(['Username has already been taken']) + end + end + Mime::EXTENSION_LOOKUP.keys.each do |type| context 'with extension format' do let(:username) { "test.#{type}" } @@ -6450,6 +6462,14 @@ RSpec.describe User, feature_category: :user_profile do expect(user.errors[:base]).to include('A user, alias, or group already exists with that username.') end end + + it 'when the username is assigned to another project pages unique domain' do + # Simulate the existing domain being in use + create(:project_setting, pages_unique_domain: 'existing-domain') + + expect(user.update(username: 'existing-domain')).to be_falsey + expect(user.errors.full_messages).to eq(['Username has already been taken']) + end end context 'when the username is not changed' do diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index e0bbf1e957a..4bffcadc0af 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -1087,7 +1087,7 @@ RSpec.describe ProjectPolicy, feature_category: :system_access do context 'when project restricts use of user defined variables' do before do - project.update!(restrict_user_defined_variables: true) + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) end it { is_expected.not_to be_allowed(:set_pipeline_variables) } @@ -1107,7 +1107,7 @@ RSpec.describe ProjectPolicy, feature_category: :system_access do context 'when project restricts use of user defined variables' do before do - project.update!(restrict_user_defined_variables: true) + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) end it { is_expected.to be_allowed(:set_pipeline_variables) } diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index 6388ef049d7..96be6bb3b1b 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -1041,6 +1041,7 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do let(:params) { {} } before do + project.update!(ci_pipeline_variables_minimum_override_role: :developer) post api("/projects/#{project.id}/jobs/#{job.id}/play", api_user), params: params end diff --git a/spec/requests/api/ci/pipeline_schedules_spec.rb b/spec/requests/api/ci/pipeline_schedules_spec.rb index 38b23a20ba1..7d6e679e1f5 100644 --- a/spec/requests/api/ci/pipeline_schedules_spec.rb +++ b/spec/requests/api/ci/pipeline_schedules_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integration do let_it_be(:developer) { create(:user) } let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, :repository, public_builds: false) } + let_it_be_with_reload(:project) { create(:project, :repository, public_builds: false) } before do project.add_developer(developer) @@ -687,7 +687,7 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra context 'when project restricts use of user defined variables' do before do - project.update!(restrict_user_defined_variables: true) + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) end context 'as developer' do @@ -743,6 +743,10 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra end context 'when key has validation error' do + before do + project.update!(ci_pipeline_variables_minimum_override_role: :developer) + end + it 'does not create pipeline_schedule_variable' do post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables", developer), params: params.merge('key' => '!?!?') @@ -815,7 +819,7 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra context 'when project restricts use of user defined variables' do before do - project.update!(restrict_user_defined_variables: true) + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) end context 'as developer' do @@ -928,7 +932,7 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra context 'when project restricts use of user defined variables' do before do - project.update!(restrict_user_defined_variables: true) + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) end context 'as developer' do diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index cd6eaeaa4ef..1952ac91c74 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -775,6 +775,10 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'variables given' do let(:variables) { [{ 'variable_type' => 'file', 'key' => 'UPLOAD_TO_S3', 'value' => 'true' }] } + before do + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) + end + it 'creates and returns a new pipeline using the given variables', :aggregate_failures do expect do post api("/projects/#{project.id}/pipeline", user), params: { ref: project.default_branch, variables: variables } @@ -794,6 +798,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do before do config = YAML.dump(test: { script: 'test', only: { variables: ['$STAGING'] } }) stub_ci_pipeline_yaml_file(config) + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) end it 'creates and returns a new pipeline using the given variables', :aggregate_failures do diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index 94dc73e00ad..3798f0a4e8d 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -2,23 +2,26 @@ require 'spec_helper' -RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_visibility do +RSpec.describe API::Ci::Runners, :aggregate_failures, factory_default: :keep, feature_category: :fleet_visibility do using RSpec::Parameterized::TableSyntax - let_it_be(:admin) { create(:user, :admin) } - let_it_be(:user) { create(:user) } - let_it_be(:user2) { create(:user) } + let_it_be(:organization) { create_default(:organization) } + let_it_be(:admin) { create(:user, :admin, last_activity_on: Time.current) } + let_it_be(:users) { create_list(:user, 2) } let_it_be(:group_guest) { create(:user, guest_of: group) } let_it_be(:group_reporter) { create(:user, reporter_of: group) } let_it_be(:group_developer) { create(:user, developer_of: group) } let_it_be(:group_maintainer) { create(:user, maintainer_of: group) } - let_it_be(:project) { create(:project, creator_id: user.id, maintainers: user, reporters: user2) } - let_it_be(:project2) { create(:project, creator_id: user.id, maintainers: user, organization: project.organization) } - - let_it_be(:group) { create(:group, owners: user) } + let_it_be(:group) { create(:group, owners: users.first) } let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:project) do + create(:project, creator_id: users.first.id, maintainers: users.first, reporters: users.second) + end + + let_it_be(:project2) { create(:project, creator_id: users.first.id, maintainers: users.first) } + let_it_be(:shared_runner, reload: true) { create(:ci_runner, :instance, :with_runner_manager, description: 'Shared runner') } let_it_be(:project_runner, reload: true) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } let_it_be(:two_projects_runner) { create(:ci_runner, :project, description: 'Two projects runner', projects: [project, project2]) } @@ -31,7 +34,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v shared_context 'access token setup' do let(:current_user) { nil } - let(:pat_user) { user } + let(:pat_user) { users.first } let(:pat) { create(:personal_access_token, user: pat_user, scopes: [scope]) } let(:extra_query_parts) { { private_token: pat.token } } end @@ -66,7 +69,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v subject(:perform_request) { get api(path, current_user) } context 'authorized user' do - let(:current_user) { user } + let(:current_user) { users.first } it 'returns response status and headers' do perform_request @@ -219,7 +222,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v subject(:perform_request) { get api(path, current_user) } context 'authorized user' do - let(:current_user) { user } + let(:current_user) { users.first } context 'when runner has managers' do let(:runner) { shared_runner } @@ -496,7 +499,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'without admin privileges' do - let(:current_user) { user } + let(:current_user) { users.first } it 'does not return runners list' do perform_request @@ -597,7 +600,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context "runner project's administrative user" do - let(:current_user) { user } + let(:current_user) { users.first } context 'when runner is not shared' do let(:runner) { project_runner } @@ -624,7 +627,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'authorized user' do - let(:current_user) { user } + let(:current_user) { users.first } it_behaves_like 'an endpoint returning expected results' @@ -646,7 +649,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'other authorized user' do - let(:current_user) { user2 } + let(:current_user) { users.second } let(:runner) { project_runner } it "does not return project runner's details" do @@ -898,7 +901,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'authorized user' do - let(:current_user) { user } + let(:current_user) { users.first } let(:params) { { description: 'test' } } context 'when runner is shared' do @@ -945,7 +948,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'when user does not have access to runner' do - let(:current_user) { user2 } + let(:current_user) { users.second } it 'does not update runner' do perform_request @@ -1059,7 +1062,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'authorized user' do - let(:current_user) { user } + let(:current_user) { users.first } context 'when runner is shared' do let(:runner) { shared_runner } @@ -1081,7 +1084,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v let(:runner) { project_runner } context 'when user does not have access to runner' do - let(:current_user) { user2 } + let(:current_user) { users.second } it 'does not delete runner without access to it' do perform_request @@ -1179,7 +1182,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'when user has owner access' do - let(:current_user) { user } + let(:current_user) { users.first } it 'deletes runner' do expect do @@ -1217,7 +1220,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v let(:runner) { group_runner_b } context 'when user has owner access' do - let(:current_user) { user } + let(:current_user) { users.first } it 'deletes group runner' do expect do @@ -1329,7 +1332,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v let(:runner) { project_runner } context 'when user does not have access to runner' do - let(:current_user) { user2 } + let(:current_user) { users.second } it 'does not reset runner' do expect do @@ -1341,7 +1344,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v context 'with request authorized with access token' do include_context 'access token setup' do - let(:pat_user) { user2 } + let(:pat_user) { users.second } end it_behaves_like 'when scope is forbidden', forbidden_scopes: %i[manage_runner create_runner read_api] @@ -1349,7 +1352,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'when user has access to runner' do - let(:current_user) { user } + let(:current_user) { users.first } it_behaves_like 'a runner accepting authentication token reset' @@ -1419,7 +1422,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'when user has owner access' do - let(:current_user) { user } + let(:current_user) { users.first } it_behaves_like 'a runner accepting authentication token reset' @@ -1476,15 +1479,17 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v let_it_be(:jobs) do project_runner_manager1 = create(:ci_runner_machine, runner: project_runner, system_xid: 'id1') project_runner_manager2 = create(:ci_runner_machine, runner: two_projects_runner, system_xid: 'id1') + pipeline_args = { pipeline: create(:ci_pipeline, project: project) } + pipeline2_args = { pipeline: create(:ci_pipeline, project: project2) } [ - create(:ci_build), - create(:ci_build, :running, runner_manager: shared_runner_manager1, project: project), - create(:ci_build, :failed, runner_manager: shared_runner_manager1, project: project), - create(:ci_build, :running, runner_manager: project_runner_manager1, project: project), - create(:ci_build, :failed, runner_manager: project_runner_manager1, project: project), - create(:ci_build, :running, runner_manager: project_runner_manager2, project: project), - create(:ci_build, :running, runner_manager: project_runner_manager2, project: project2) + create(:ci_build, pipeline: create(:ci_pipeline)), + create(:ci_build, :running, runner_manager: shared_runner_manager1, **pipeline_args), + create(:ci_build, :failed, runner_manager: shared_runner_manager1, **pipeline_args), + create(:ci_build, :running, runner_manager: project_runner_manager1, **pipeline_args), + create(:ci_build, :failed, runner_manager: project_runner_manager1, **pipeline_args), + create(:ci_build, :running, runner_manager: project_runner_manager2, **pipeline_args), + create(:ci_build, :running, runner_manager: project_runner_manager2, **pipeline2_args) ] end @@ -1586,11 +1591,11 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v context 'when user does not have authorization to see all jobs' do let(:runner) { two_projects_runner } - let(:current_user) { user2 } + let(:current_user) { users.second } before_all do - project.add_guest(user2) - project2.add_maintainer(user2) + project.add_guest(users.second) + project2.add_maintainer(users.second) end it 'shows only jobs it has permission to see' do @@ -1603,7 +1608,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v context 'with request authorized with access token' do include_context 'access token setup' do - let(:pat_user) { user2 } + let(:pat_user) { users.second } end it_behaves_like 'when scope is forbidden', forbidden_scopes: %i[create_runner] @@ -1706,12 +1711,13 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v it 'avoids N+1 DB queries', :use_sql_query_cache do get api(path, current_user) + pipeline = create(:ci_pipeline, project: project2, sha: 'ddd0f15ae83993f5cb66a927a28673882e99100b') control = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api(path, current_user) end - create(:ci_build, :failed, runner: shared_runner, project: project) + create(:ci_build, :failed, runner: shared_runner, project: project2, pipeline: pipeline) expect do get api(path, current_user) @@ -1755,7 +1761,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context "runner project's administrative user" do - let(:current_user) { user } + let(:current_user) { users.first } context 'when runner exists' do context 'when runner is shared' do @@ -1821,7 +1827,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'other authorized user' do - let(:current_user) { user2 } + let(:current_user) { users.second } let(:runner) { shared_runner } it 'does not return jobs' do @@ -1845,7 +1851,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v context 'with system_id param' do let(:extra_query_parts) { { system_id: 'id1' } } - let(:current_user) { user } + let(:current_user) { users.first } context 'with project runner' do let(:runner) { project_runner } @@ -1880,7 +1886,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v shared_examples_for 'unauthorized access to runners list' do context 'authorized user without maintainer privileges' do - let(:current_user) { user2 } + let(:current_user) { users.second } it "does not return group's runners" do perform_request @@ -1917,7 +1923,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'authorized user with maintainer privileges' do - let(:current_user) { user } + let(:current_user) { users.first } it 'returns all runners' do perform_request @@ -1938,10 +1944,10 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers - expect(json_response).to match_array [ + expect(json_response).to contain_exactly( a_hash_including('description' => 'Project runner'), a_hash_including('description' => 'Two projects runner') - ] + ) end context 'and scope is unknown' do @@ -1961,10 +1967,10 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v it 'filters runners by type' do perform_request - expect(json_response).to match_array [ + expect(json_response).to contain_exactly( a_hash_including('description' => 'Project runner'), a_hash_including('description' => 'Two projects runner') - ] + ) end context 'and type is invalid' do @@ -2063,7 +2069,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v subject(:perform_request) { get api(path, current_user) } context 'authorized user with maintainer privileges' do - let(:current_user) { user } + let(:current_user) { users.first } it 'returns all runners' do perform_request @@ -2208,7 +2214,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v context 'authorized user' do let_it_be(:project_runner2) { create(:ci_runner, :project, projects: [project2]) } - let(:current_user) { user } + let(:current_user) { users.first } let(:runner) { project_runner2 } it 'enables project runner' do @@ -2307,7 +2313,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'when user does not have permissions' do - let(:current_user) { user2 } + let(:current_user) { users.second } let(:runner) { project_runner } it 'does not enable runner' do @@ -2319,11 +2325,10 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'user is not admin and does not have access to project runner' do - let_it_be(:project3) { create(:project) } - let_it_be(:new_project_runner) { create(:ci_runner, :project, projects: [project3]) } + let_it_be(:new_project_runner) { create(:ci_runner, :project, projects: [project]) } let(:runner) { new_project_runner } - let(:current_user) { user } + let(:current_user) { create(:user, guest_of: project) } it 'does not enable runner' do perform_request @@ -2351,7 +2356,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v subject(:perform_request) { delete api(path, current_user) } context 'authorized user' do - let(:current_user) { user } + let(:current_user) { users.first } context 'when runner have more than one associated projects' do let(:runner) { two_projects_runner } @@ -2395,7 +2400,7 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end context 'authorized user without permissions' do - let(:current_user) { user2 } + let(:current_user) { users.second } let(:runner) { project_runner } it "does not disable project's runner" do diff --git a/spec/requests/api/ci/triggers_spec.rb b/spec/requests/api/ci/triggers_spec.rb index 0fece5dea3e..4d887ef2add 100644 --- a/spec/requests/api/ci/triggers_spec.rb +++ b/spec/requests/api/ci/triggers_spec.rb @@ -15,6 +15,10 @@ RSpec.describe API::Ci::Triggers, feature_category: :pipeline_composition do let_it_be(:trigger2) { create(:ci_trigger, project: project, token: trigger_token_2, owner: user2) } let_it_be(:trigger_request) { create(:ci_trigger_request, trigger: trigger, created_at: '2015-01-01 12:13:14') } + before do + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) + end + describe 'POST /projects/:project_id/trigger/pipeline' do let(:options) do { diff --git a/spec/requests/api/graphql/mutations/ci/job/play_spec.rb b/spec/requests/api/graphql/mutations/ci/job/play_spec.rb index 2caaf40807d..329ae8d4f87 100644 --- a/spec/requests/api/graphql/mutations/ci/job/play_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/job/play_spec.rb @@ -6,7 +6,7 @@ RSpec.describe 'JobPlay', feature_category: :continuous_integration do include GraphqlHelpers let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, maintainers: user) } + let_it_be_with_reload(:project) { create(:project, maintainers: user) } let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } let(:variables) do @@ -33,6 +33,10 @@ RSpec.describe 'JobPlay', feature_category: :continuous_integration do let(:mutation_response) { graphql_mutation_response(:job_play) } + before do + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) + end + shared_examples 'playing a job' do it 'returns an error if the user is not allowed to play the job' do post_graphql_mutation(mutation, current_user: create(:user)) diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/create_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/create_spec.rb index 579e726641f..a1e63d9ccde 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/create_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/create_spec.rb @@ -6,7 +6,7 @@ RSpec.describe 'PipelineSchedulecreate', feature_category: :continuous_integrati include GraphqlHelpers let_it_be(:current_user) { create(:user) } - let_it_be(:project) { create(:project, :public, :repository) } + let_it_be_with_reload(:project) { create(:project, :public, :repository) } let(:mutation) do variables = { @@ -61,6 +61,7 @@ RSpec.describe 'PipelineSchedulecreate', feature_category: :continuous_integrati context 'when authorized' do before_all do + project.update!(ci_pipeline_variables_minimum_override_role: :developer) project.add_developer(current_user) end diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/update_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/update_spec.rb index a537d4cd145..8e8e75eeaf6 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/update_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/update_spec.rb @@ -57,6 +57,7 @@ RSpec.describe 'PipelineScheduleUpdate', feature_category: :continuous_integrati context 'when authorized' do before_all do + project.update!(ci_pipeline_variables_minimum_override_role: :developer) project.add_developer(current_user) end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index ba1ac291bcf..6e1d76d16d3 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -1282,6 +1282,25 @@ RSpec.describe API::Groups, :with_current_organization, feature_category: :group expect(json_response['description']).to eq('it works') end end + + context 'update path with existing pages unique domain' do + before do + stub_pages_setting(enabled: true) + + create( + :project_setting, + project: project1, + pages_unique_domain_enabled: true, + pages_unique_domain: 'existing-domain') + end + + it "returns 400 bad request error" do + put api("/groups/#{group1.id}", user1), params: { path: 'existing-domain' } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq({ "path" => ["has already been taken"] }) + end + end end context 'when authenticated as the admin' do @@ -2947,6 +2966,27 @@ RSpec.describe API::Groups, :with_current_organization, feature_category: :group expect(response).to have_gitlab_http_status(:bad_request) end + + context 'with existing pages unique domain' do + before do + stub_pages_setting(enabled: true) + + create( + :project_setting, + project: project1, + pages_unique_domain_enabled: true, + pages_unique_domain: 'existing-domain') + end + + it "returns 400 bad request error if path is already used by pages unique domain" do + expect do + post api("/groups", user3), params: { name: 'test', path: 'existing-domain' } + end.not_to change { Group.count } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq("Failed to save group {:path=>[\"has already been taken\"]}") + end + end end end diff --git a/spec/requests/api/namespaces_spec.rb b/spec/requests/api/namespaces_spec.rb index 2320b3be0c1..5833ff2cf23 100644 --- a/spec/requests/api/namespaces_spec.rb +++ b/spec/requests/api/namespaces_spec.rb @@ -367,6 +367,17 @@ RSpec.describe API::Namespaces, :aggregate_failures, feature_category: :groups_a expect(response.body).to eq(expected_json) end + it 'returns JSON indicating the namespace exists and a suggestion when same pages unique domain exists' do + # Simulate the existing domain being in use + create(:project_setting, pages_unique_domain: 'existing-domain') + + get api("#{path}/existing-domain/exists", user) + + expected_json = { exists: true, suggests: ["existing-domain1"] }.to_json + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(expected_json) + end + it 'ignores paths of groups present in other hierarchies when making suggestions' do (1..2).to_a.each do |suffix| create(:group, name: "mygroup#{suffix}", path: "mygroup#{suffix}", parent: namespace2) diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 24e72995716..26f31aa2a31 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -939,6 +939,19 @@ RSpec.describe API::Repositories, feature_category: :source_code_management do expect(response).to have_gitlab_http_status(:ok) end + it 'rate limits user when thresholds hit' do + allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true) + + get( + api("/projects/#{project.id}/repository/changelog", user), + params: { + version: '1.0.0' + } + ) + + expect(response).to have_gitlab_http_status(:too_many_requests) + end + context 'when previous tag version does not exist' do it_behaves_like '422 response' do let(:request) { get api("/projects/#{project.id}/repository/changelog", user), params: { version: 'v0.0.0' } } @@ -1100,5 +1113,18 @@ RSpec.describe API::Repositories, feature_category: :source_code_management do expect(response).to have_gitlab_http_status(:ok) end + + it 'rate limits user when thresholds hit' do + allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true) + + post( + api("/projects/#{project.id}/repository/changelog", user), + params: { + version: '1.0.0' + } + ) + + expect(response).to have_gitlab_http_status(:too_many_requests) + end end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 7035db5d245..226702b0724 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1560,6 +1560,34 @@ RSpec.describe API::Users, :with_current_organization, :aggregate_failures, feat end end + context 'with existing pages unique domain' do + let_it_be(:project) { create(:project) } + + before do + stub_pages_setting(enabled: true) + + create( + :project_setting, + project: project, + pages_unique_domain_enabled: true, + pages_unique_domain: 'unique-domain') + end + + it 'returns 400 bad request error if same username is already used by pages unique domain' do + expect do + post api(path, admin, admin_mode: true), + params: { + name: 'foo', + email: 'foo@example.com', + password: User.random_password, + username: 'unique-domain' + } + end.to change { User.count }.by(0) + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq({ "username" => ["has already been taken"] }) + end + end + context 'when user with a primary email exists' do context 'when the primary email is confirmed' do let!(:confirmed_user) { create(:user, email: 'foo@example.com') } @@ -1848,6 +1876,26 @@ RSpec.describe API::Users, :with_current_organization, :aggregate_failures, feat expect(user.reload.username).to eq(user.username) end + context 'with existing pages unique domain' do + let_it_be(:project) { create(:project) } + + before do + stub_pages_setting(enabled: true) + + create( + :project_setting, + project: project, + pages_unique_domain_enabled: true, + pages_unique_domain: 'unique-domain') + end + + it 'returns 400 bad request error if same username is already used by pages unique domain' do + put api(path, admin, admin_mode: true), params: { username: 'unique-domain' } + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq({ "username" => ["has already been taken"] }) + end + end + it "updates user's existing identity" do put api("/users/#{ldap_user.id}", admin, admin_mode: true), params: { provider: 'ldapmain', extern_uid: '654321' } diff --git a/spec/requests/api/virtual_registries/packages/maven/endpoints_cached_responses_spec.rb b/spec/requests/api/virtual_registries/packages/maven/cached_responses_spec.rb similarity index 76% rename from spec/requests/api/virtual_registries/packages/maven/endpoints_cached_responses_spec.rb rename to spec/requests/api/virtual_registries/packages/maven/cached_responses_spec.rb index 28ea94ea7df..fdc5a963b99 100644 --- a/spec/requests/api/virtual_registries/packages/maven/endpoints_cached_responses_spec.rb +++ b/spec/requests/api/virtual_registries/packages/maven/cached_responses_spec.rb @@ -2,15 +2,13 @@ require 'spec_helper' -RSpec.describe API::VirtualRegistries::Packages::Maven::Endpoints, :aggregate_failures, feature_category: :virtual_registry do +RSpec.describe API::VirtualRegistries::Packages::Maven::CachedResponses, :aggregate_failures, feature_category: :virtual_registry do using RSpec::Parameterized::TableSyntax include_context 'for maven virtual registry api setup' - describe 'GET /api/v4/virtual_registries/packages/maven/registries/:id/upstreams/:upstream_id/cached_responses' do + describe 'GET /api/v4/virtual_registries/packages/maven/upstreams/:id/cached_responses' do let(:upstream_id) { upstream.id } - let(:url) do - "/virtual_registries/packages/maven/registries/#{registry.id}/upstreams/#{upstream_id}/cached_responses" - end + let(:url) { "/virtual_registries/packages/maven/upstreams/#{upstream_id}/cached_responses" } let_it_be(:processing_cached_response) do create( @@ -32,8 +30,8 @@ RSpec.describe API::VirtualRegistries::Packages::Maven::Endpoints, :aggregate_fa expect(Gitlab::Json.parse(response.body)).to contain_exactly( cached_response .as_json - .merge('cached_response_id' => Base64.urlsafe_encode64(cached_response.relative_path)) - .except('id', 'object_storage_key', 'file_store', 'status', 'file_final_path') + .merge('id' => Base64.urlsafe_encode64("#{upstream.id} #{cached_response.relative_path}")) + .except('object_storage_key', 'file_store', 'status', 'file_final_path') ) end end @@ -77,22 +75,12 @@ RSpec.describe API::VirtualRegistries::Packages::Maven::Endpoints, :aggregate_fa context 'for authentication' do where(:token, :sent_as, :status) do :personal_access_token | :header | :ok - :personal_access_token | :basic_auth | :ok :deploy_token | :header | :ok - :deploy_token | :basic_auth | :ok :job_token | :header | :ok - :job_token | :basic_auth | :ok end with_them do - let(:headers) do - case sent_as - when :header - token_header(token) - when :basic_auth - token_basic_auth(token) - end - end + let(:headers) { token_header(token) } it_behaves_like 'returning response status', params[:status] end @@ -123,13 +111,9 @@ RSpec.describe API::VirtualRegistries::Packages::Maven::Endpoints, :aggregate_fa end end - describe 'DELETE /api/v4/virtual_registries/packages/maven/registries/:id/upstreams/' \ - ':upstream_id/cached_responses/:cached_response_id' do - let(:cached_response_id) { Base64.urlsafe_encode64(cached_response.relative_path) } - let(:url) do - "/virtual_registries/packages/maven/registries/#{registry.id}/upstreams/#{upstream.id}/" \ - "cached_responses/#{cached_response_id}" - end + describe 'DELETE /api/v4/virtual_registries/packages/maven/cached_responses/:id' do + let(:id) { Base64.urlsafe_encode64("#{upstream.id} #{cached_response.relative_path}") } + let(:url) { "/virtual_registries/packages/maven/cached_responses/#{id}" } let_it_be(:processing_cached_response) do create( @@ -185,22 +169,12 @@ RSpec.describe API::VirtualRegistries::Packages::Maven::Endpoints, :aggregate_fa where(:token, :sent_as, :status) do :personal_access_token | :header | :no_content - :personal_access_token | :basic_auth | :no_content :deploy_token | :header | :forbidden - :deploy_token | :basic_auth | :forbidden :job_token | :header | :no_content - :job_token | :basic_auth | :no_content end with_them do - let(:headers) do - case sent_as - when :header - token_header(token) - when :basic_auth - token_basic_auth(token) - end - end + let(:headers) { token_header(token) } if params[:status] == :no_content it_behaves_like 'successful response' diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index b9a3bed87d9..ce9b6707851 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -639,40 +639,42 @@ RSpec.describe 'Git HTTP requests', feature_category: :source_code_management do context "when an oauth token is provided" do let_it_be(:organization) { create(:organization) } - context "when oauth token has ai_workflows scope" do - let(:path) { "#{project.full_path}.git" } - let(:env) { { user: 'oauth2', password: @token.token } } + let!(:token) do + Doorkeeper::AccessToken.create!( + application_id: application.id, + resource_owner_id: user.id, + scopes: scopes, + organization_id: organization.id) + .plaintext_token + end - before do - application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) - @token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "ai_workflows", organization_id: organization.id) - end + let(:application) do + Doorkeeper::Application.create!( + name: "MyApp", + redirect_uri: "https://app.com", + owner: user) + end + + let(:scopes) { 'api' } + let(:path) { "#{project.full_path}.git" } + let(:env) { { user: 'oauth2', password: token } } + + context "when oauth token has ai_workflows scope" do + let(:scopes) { 'ai_workflows' } it_behaves_like 'pulls are allowed' it_behaves_like 'pushes are allowed' end context "when oauth token has api scope" do - let(:path) { "#{project.full_path}.git" } - let(:env) { { user: 'oauth2', password: @token.token } } - - before do - application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) - @token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "api", organization_id: organization.id) - end + let(:scopes) { 'api' } it_behaves_like 'pulls are allowed' it_behaves_like 'pushes are allowed' end context "when oauth token has write_repository scope" do - let(:path) { "#{project.full_path}.git" } - let(:env) { { user: 'oauth2', password: @token.token } } - - before do - application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) - @token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "write_repository", organization_id: organization.id) - end + let(:scopes) { 'write_repository' } it_behaves_like 'pulls are allowed' it_behaves_like 'pushes are allowed' @@ -1316,13 +1318,24 @@ RSpec.describe 'Git HTTP requests', feature_category: :source_code_management do end context "when an oauth token is provided" do - before do - application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) - @token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "api", organization_id: project.organization_id) + let!(:token) do + Doorkeeper::AccessToken.create!( + application_id: application.id, + resource_owner_id: user.id, + scopes: 'api', + organization_id: project.organization_id) + .plaintext_token + end + + let(:application) do + Doorkeeper::Application.create!( + name: "MyApp", + redirect_uri: "https://app.com", + owner: user) end let(:path) { "#{project.full_path}.git" } - let(:env) { { user: 'oauth2', password: @token.token } } + let(:env) { { user: 'oauth2', password: token } } it_behaves_like 'pulls are allowed' it_behaves_like 'pushes are allowed' diff --git a/spec/requests/projects/commit_controller_spec.rb b/spec/requests/projects/commit_controller_spec.rb index 5da1513ec98..a42fa7a3781 100644 --- a/spec/requests/projects/commit_controller_spec.rb +++ b/spec/requests/projects/commit_controller_spec.rb @@ -3,11 +3,12 @@ require 'spec_helper' RSpec.describe Projects::CommitController, feature_category: :source_code_management do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { project.owner } + describe '#rapid_diffs' do let_it_be(:sha) { "913c66a37b4a45b9769037c55c2d238bd0942d2e" } - let_it_be(:project) { create(:project, :repository) } let_it_be(:commit) { project.commit_by(oid: sha) } - let_it_be(:user) { project.owner } let_it_be(:diff_view) { :inline } let(:params) do @@ -92,4 +93,66 @@ RSpec.describe Projects::CommitController, feature_category: :source_code_manage end end end + + describe 'GET #diff_files' do + let(:master_pickable_sha) { '7d3b0f7cff5f37573aea97cebfd5692ea1689924' } + let(:format) { :html } + let(:params) do + { + namespace_id: project.namespace, + project_id: project, + id: master_pickable_sha, + format: format + } + end + + before_all do + project.add_maintainer(user) + end + + context 'without expanded parameter' do + before do + sign_in(user) + end + + it 'does not rate limit the endpoint' do + get diff_files_namespace_project_commit_url(params) + + expect(::Gitlab::ApplicationRateLimiter) + .not_to receive(:throttled?).with(:expanded_diff_files, scope: user) + end + end + + context 'with expanded parameter' do + before do + params[:expanded] = 1 + end + + context 'with signed in user' do + it_behaves_like 'rate limited endpoint', rate_limit_key: :expanded_diff_files do + let_it_be(:current_user) { user } + + before do + sign_in current_user + end + + def request + get diff_files_namespace_project_commit_url(params), params: { scope: user } + end + end + end + + context 'without a signed in user' do + it_behaves_like 'rate limited endpoint', rate_limit_key: :expanded_diff_files do + let_it_be(:project) { create(:project, :public, :repository) } + let(:request_ip) { '1.2.3.4' } + + def request + get diff_files_namespace_project_commit_url(params), + params: { scope: request_ip }, env: { REMOTE_ADDR: request_ip } + end + end + end + end + end end diff --git a/spec/serializers/integrations/harbor_serializers/artifact_entity_spec.rb b/spec/serializers/integrations/harbor_serializers/artifact_entity_spec.rb index 20a24a9ff04..97a7d509c2c 100644 --- a/spec/serializers/integrations/harbor_serializers/artifact_entity_spec.rb +++ b/spec/serializers/integrations/harbor_serializers/artifact_entity_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::HarborSerializers::ArtifactEntity do +RSpec.describe Integrations::HarborSerializers::ArtifactEntity, feature_category: :container_registry do let_it_be(:harbor_integration) { create(:harbor_integration) } let(:artifact) do @@ -48,4 +48,20 @@ RSpec.describe Integrations::HarborSerializers::ArtifactEntity do tags: %w[2 1] }) end + + context 'with data that may contain path traversal attacks' do + before do + artifact['digest'] = './../../../../../etc/hosts' + end + + it 'logs an error and forbids the path traversal values' do + expect(::Gitlab::ErrorTracking).to receive(:track_exception).with( + an_instance_of(::Gitlab::PathTraversal::PathTraversalAttackError), + message: /Path traversal attack detected/, + class: described_class.name + ) + + expect(subject[:digest]).to eq('') + end + end end diff --git a/spec/serializers/integrations/harbor_serializers/repository_entity_spec.rb b/spec/serializers/integrations/harbor_serializers/repository_entity_spec.rb index 29708bd0416..64e71a80b0c 100644 --- a/spec/serializers/integrations/harbor_serializers/repository_entity_spec.rb +++ b/spec/serializers/integrations/harbor_serializers/repository_entity_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::HarborSerializers::RepositoryEntity do +RSpec.describe Integrations::HarborSerializers::RepositoryEntity, feature_category: :container_registry do let_it_be(:harbor_integration) { create(:harbor_integration) } let(:repo) do @@ -37,14 +37,17 @@ RSpec.describe Integrations::HarborSerializers::RepositoryEntity do context 'with data that may contain path traversal attacks' do before do repo["project_id"] = './../../../../../etc/hosts' + repo['name'] = './../../../../../etc/hosts' end - it 'returns empty location' do + it 'logs an error and forbids the path traversal values' do + expect(Gitlab::AppLogger).to receive(:error).with(/Path traversal attack detected/).twice + expect(subject).to include({ artifact_count: 1, creation_time: "2022-03-13T09:36:43.240Z".to_datetime, harbor_id: 1, - name: "jihuprivate/busybox", + name: '', harbor_project_id: './../../../../../etc/hosts', pull_count: 0, update_time: "2022-03-13T09:36:43.240Z".to_datetime, diff --git a/spec/services/ci/create_downstream_pipeline_service_spec.rb b/spec/services/ci/create_downstream_pipeline_service_spec.rb index 5a2bf4f7e97..16c96726910 100644 --- a/spec/services/ci/create_downstream_pipeline_service_spec.rb +++ b/spec/services/ci/create_downstream_pipeline_service_spec.rb @@ -431,7 +431,8 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute', feature_category before do bridge.yaml_variables = [{ key: 'BRIDGE', value: '$PIPELINE_VARIABLE-var', public: true }] - upstream_pipeline.project.update!(restrict_user_defined_variables: true) + upstream_pipeline.project.update!(restrict_user_defined_variables: true, + ci_pipeline_variables_minimum_override_role: :maintainer) end it 'creates a new pipeline allowing variables to be passed downstream' do @@ -626,6 +627,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute', feature_category context 'when bridge job has YAML variables defined' do before do bridge.yaml_variables = [{ key: 'BRIDGE', value: 'var', public: true }] + downstream_project.update!(ci_pipeline_variables_minimum_override_role: :developer) end it 'passes bridge variables to downstream pipeline' do @@ -648,6 +650,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute', feature_category context 'when using YAML variables interpolation' do before do bridge.yaml_variables = [{ key: 'BRIDGE', value: '$PIPELINE_VARIABLE-var', public: true }] + downstream_project.update!(ci_pipeline_variables_minimum_override_role: :developer) end it 'makes it possible to pass pipeline variable downstream' do @@ -658,7 +661,8 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute', feature_category context 'when downstream project does not allow user-defined variables for multi-project pipelines' do before do - downstream_project.update!(restrict_user_defined_variables: true) + downstream_project.update!(restrict_user_defined_variables: true, + ci_pipeline_variables_minimum_override_role: :maintainer) end it 'does not create a new pipeline' do @@ -818,6 +822,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute', feature_category context 'when downstream pipeline has workflow rule' do before do stub_ci_pipeline_yaml_file(config) + downstream_project.update!(ci_pipeline_variables_minimum_override_role: :developer) end let(:config) do diff --git a/spec/services/ci/create_pipeline_service/include_spec.rb b/spec/services/ci/create_pipeline_service/include_spec.rb index a79924c14f1..fe9926b8d8a 100644 --- a/spec/services/ci/create_pipeline_service/include_spec.rb +++ b/spec/services/ci/create_pipeline_service/include_spec.rb @@ -31,6 +31,10 @@ RSpec.describe Ci::CreatePipelineService, feature_category: :pipeline_compositio end end + before do + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) + end + shared_examples 'not including the file' do it 'does not include the job in the file' do expect(pipeline).to be_created_successfully diff --git a/spec/services/ci/create_pipeline_service/partitioning_spec.rb b/spec/services/ci/create_pipeline_service/partitioning_spec.rb index 1d63b77a006..dfb33631399 100644 --- a/spec/services/ci/create_pipeline_service/partitioning_spec.rb +++ b/spec/services/ci/create_pipeline_service/partitioning_spec.rb @@ -42,6 +42,7 @@ RSpec.describe Ci::CreatePipelineService, :aggregate_failures, before do stub_ci_pipeline_yaml_file(config) allow(Ci::Pipeline).to receive(:current_partition_value) { current_partition_id } + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) end it 'assigns partition_id to pipeline' do diff --git a/spec/services/ci/create_pipeline_service/rules_spec.rb b/spec/services/ci/create_pipeline_service/rules_spec.rb index 8a046be1843..4a4eb50a7e7 100644 --- a/spec/services/ci/create_pipeline_service/rules_spec.rb +++ b/spec/services/ci/create_pipeline_service/rules_spec.rb @@ -16,6 +16,10 @@ RSpec.describe Ci::CreatePipelineService, feature_category: :pipeline_compositio let(:base_initialization_params) { { ref: ref, before: '00000000', after: project.commit(ref).sha, variables_attributes: nil } } let(:initialization_params) { base_initialization_params } + before do + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) + end + context 'job:rules' do let(:regular_job) { find_job('regular-job') } let(:rules_job) { find_job('rules-job') } diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 6060560367f..f90861e7792 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -12,6 +12,7 @@ RSpec.describe Ci::CreatePipelineService, :clean_gitlab_redis_cache, feature_cat let(:ref_name) { 'refs/heads/master' } before do + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) stub_ci_pipeline_to_return_yaml_file end diff --git a/spec/services/ci/pipeline_schedules/create_service_spec.rb b/spec/services/ci/pipeline_schedules/create_service_spec.rb index 60d802906b7..44f5078fbaf 100644 --- a/spec/services/ci/pipeline_schedules/create_service_spec.rb +++ b/spec/services/ci/pipeline_schedules/create_service_spec.rb @@ -12,6 +12,7 @@ RSpec.describe Ci::PipelineSchedules::CreateService, feature_category: :continuo describe "execute" do before_all do + project.update!(ci_pipeline_variables_minimum_override_role: :developer) repository.add_branch(project.creator, 'patch-x', 'master') repository.add_branch(project.creator, 'ambiguous', 'master') repository.add_branch(project.creator, '1/nested/branch-name', 'master') diff --git a/spec/services/ci/pipeline_schedules/update_service_spec.rb b/spec/services/ci/pipeline_schedules/update_service_spec.rb index ef7cca55be5..a8520e67a2a 100644 --- a/spec/services/ci/pipeline_schedules/update_service_spec.rb +++ b/spec/services/ci/pipeline_schedules/update_service_spec.rb @@ -17,6 +17,7 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo let_it_be_with_reload(:repository) { project.repository } before_all do + project.update!(ci_pipeline_variables_minimum_override_role: :developer) project.add_maintainer(user) project.add_owner(project_owner) project.add_reporter(reporter) diff --git a/spec/services/ci/pipeline_schedules/variables_create_service_spec.rb b/spec/services/ci/pipeline_schedules/variables_create_service_spec.rb index 6ea50dbbd76..007d271c13e 100644 --- a/spec/services/ci/pipeline_schedules/variables_create_service_spec.rb +++ b/spec/services/ci/pipeline_schedules/variables_create_service_spec.rb @@ -14,6 +14,10 @@ RSpec.describe Ci::PipelineSchedules::VariablesCreateService, feature_category: subject(:service) { described_class.new(pipeline_schedule, user, params) } + before do + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) + end + describe 'execute' do context 'when user does not have permission' do subject(:service) { described_class.new(pipeline_schedule, reporter, {}) } diff --git a/spec/services/ci/pipeline_schedules/variables_update_service_spec.rb b/spec/services/ci/pipeline_schedules/variables_update_service_spec.rb index fe8064c4ae0..614acad0dd1 100644 --- a/spec/services/ci/pipeline_schedules/variables_update_service_spec.rb +++ b/spec/services/ci/pipeline_schedules/variables_update_service_spec.rb @@ -15,6 +15,10 @@ RSpec.describe Ci::PipelineSchedules::VariablesUpdateService, feature_category: subject(:service) { described_class.new(pipeline_schedule_variable, user, params) } + before do + project.update!(ci_pipeline_variables_minimum_override_role: :maintainer) + end + describe 'execute' do context 'when user does not have permission' do subject(:service) { described_class.new(pipeline_schedule_variable, reporter, {}) } diff --git a/spec/services/ci/pipeline_trigger_service_spec.rb b/spec/services/ci/pipeline_trigger_service_spec.rb index df1be2f5299..68f0ca5efee 100644 --- a/spec/services/ci/pipeline_trigger_service_spec.rb +++ b/spec/services/ci/pipeline_trigger_service_spec.rb @@ -9,6 +9,7 @@ RSpec.describe Ci::PipelineTriggerService, feature_category: :continuous_integra before do stub_ci_pipeline_to_return_yaml_file + project.update!(ci_pipeline_variables_minimum_override_role: :developer) end describe '#execute' do diff --git a/spec/services/ci/play_build_service_spec.rb b/spec/services/ci/play_build_service_spec.rb index ee43d273424..8e436da7626 100644 --- a/spec/services/ci/play_build_service_spec.rb +++ b/spec/services/ci/play_build_service_spec.rb @@ -12,6 +12,10 @@ RSpec.describe Ci::PlayBuildService, '#execute', feature_category: :continuous_i described_class.new(project, user) end + before do + project.update!(ci_pipeline_variables_minimum_override_role: :developer) + end + context 'when project does not have repository yet' do let(:project) { create(:project) } @@ -96,7 +100,8 @@ RSpec.describe Ci::PlayBuildService, '#execute', feature_category: :continuous_i context 'when user defined variables are restricted' do before do - project.update!(restrict_user_defined_variables: true) + project.update!(restrict_user_defined_variables: true, + ci_pipeline_variables_minimum_override_role: :maintainer) end context 'when user is maintainer' do diff --git a/spec/services/issues/resolve_discussions_spec.rb b/spec/services/issues/resolve_discussions_spec.rb index ea4ad0440ec..61615e13eeb 100644 --- a/spec/services/issues/resolve_discussions_spec.rb +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -102,6 +102,49 @@ RSpec.describe Issues::ResolveDiscussions, feature_category: :team_planning do expect(discussion_ids).to contain_exactly(discussion.id) end + it "contains only non-confidential discussions" do + _second_discussion = Discussion.new([ + create( + :discussion_note_on_merge_request, + :confidential, + noteable: merge_request, + project: merge_request.target_project + ) + ]) + service = DummyService.new( + container: project, + current_user: user, + params: { merge_request_to_resolve_discussions_of: merge_request.iid } + ) + # We need to compare discussion id's because the Discussion-objects are rebuilt + # which causes the object-id's not to be different. + discussion_ids = service.discussions_to_resolve.map(&:id) + + expect(discussion_ids).to contain_exactly(discussion.id) + end + + it "is empty when the discussion is confidential" do + second_discussion = Discussion.new([ + create( + :discussion_note_on_merge_request, + :confidential, + noteable: merge_request, + project: merge_request.target_project + ) + ]) + + service = DummyService.new( + container: project, + current_user: user, + params: { + discussion_to_resolve: second_discussion.id, + merge_request_to_resolve_discussions_of: merge_request.iid + } + ) + + expect(service.discussions_to_resolve).to be_empty + end + it "is empty when a discussion and another merge request are passed" do service = DummyService.new( container: project, diff --git a/spec/support/helpers/import/gitea_helper.rb b/spec/support/helpers/import/gitea_helper.rb new file mode 100644 index 00000000000..7528ed83e69 --- /dev/null +++ b/spec/support/helpers/import/gitea_helper.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# Helper methods for Gitea user mapping specs +module Import + module GiteaHelper + # This method allows a project to receive two message chains to check the + # user mapping state of a project. + def stub_user_mapping_chain(project, user_mapping_enabled) + allow(project).to receive_message_chain(:import_data, :reset, :user_mapping_enabled?) + .and_return(user_mapping_enabled) + + allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?) + .and_return(user_mapping_enabled) + end + end +end diff --git a/spec/support/shared_examples/ci/pipeline_schedules_create_or_update_shared_examples.rb b/spec/support/shared_examples/ci/pipeline_schedules_create_or_update_shared_examples.rb index fb9e04c05df..fd1b73a613f 100644 --- a/spec/support/shared_examples/ci/pipeline_schedules_create_or_update_shared_examples.rb +++ b/spec/support/shared_examples/ci/pipeline_schedules_create_or_update_shared_examples.rb @@ -72,7 +72,7 @@ RSpec.shared_examples 'pipeline schedules checking variables permission' do context 'when restrict_user_defined_variables is true' do before_all do - project.update!(restrict_user_defined_variables: true) + project.update!(restrict_user_defined_variables: true, ci_pipeline_variables_minimum_override_role: :maintainer) end it_behaves_like 'success response with variables'