From 3eea648134a8cb336eef559c9984f35e3d927ffb Mon Sep 17 00:00:00 2001 From: Pirate Praveen Date: Fri, 2 Jul 2021 01:05:55 +0530 Subject: [PATCH] New upstream version 13.12.6+ds1 --- CHANGELOG.md | 31 ++++ GITALY_SERVER_VERSION | 2 +- Gemfile | 6 +- Gemfile.lock | 110 ++++++------- VERSION | 2 +- .../behaviors/markdown/copy_as_gfm.js | 3 +- .../javascripts/lib/utils/url_utility.js | 24 +++ .../releases/components/app_edit_new.vue | 9 +- .../diff_viewer/viewers/not_diffable.vue | 2 +- .../admin/application_settings_controller.rb | 9 +- .../concerns/membership_actions.rb | 2 +- app/controllers/graphql_controller.rb | 29 +++- app/controllers/ide_controller.rb | 6 + app/graphql/mutations/echo.rb | 33 ++++ app/graphql/types/mutation_type.rb | 1 + app/models/audit_event.rb | 11 ++ app/models/integrations/bamboo.rb | 1 + .../project_services/drone_ci_service.rb | 6 +- .../project_services/external_wiki_service.rb | 2 +- .../project_services/issue_tracker_service.rb | 2 +- .../project_services/mock_ci_service.rb | 2 +- .../slack_mattermost/notifier.rb | 2 +- .../project_services/teamcity_service.rb | 5 +- .../project_services/unify_circuit_service.rb | 3 +- .../project_services/webex_teams_service.rb | 2 +- .../protected_branch/push_access_level.rb | 2 +- app/models/user.rb | 24 +++ app/policies/concerns/policy_actor.rb | 2 +- app/policies/global_policy.rb | 2 +- app/services/feature_flags/base_service.rb | 6 +- app/services/feature_flags/create_service.rb | 3 +- app/services/feature_flags/destroy_service.rb | 2 +- app/services/feature_flags/update_service.rb | 12 +- app/services/projects/fork_service.rb | 5 +- app/services/web_hook_service.rb | 3 +- app/views/import/_githubish_status.html.haml | 2 +- .../diffs/viewers/_not_diffable.html.haml | 2 +- .../shared/milestones/_issuable.html.haml | 2 +- doc/api/graphql/reference/index.md | 25 +++ lib/api/members.rb | 2 + lib/gitlab/auth.rb | 2 +- lib/gitlab/auth/user_access_denied_reason.rb | 4 + lib/gitlab/diff/file.rb | 11 +- lib/gitlab/diff/parser.rb | 2 +- lib/gitlab/git/diff.rb | 11 +- lib/gitlab/http.rb | 18 ++- lib/gitlab/lfs_token.rb | 2 +- locale/gitlab.pot | 9 +- spec/controllers/graphql_controller_spec.rb | 6 +- spec/deprecation_toolkit_env.rb | 6 +- spec/features/expand_collapse_diffs_spec.rb | 2 +- .../features/projects/diffs/diff_show_spec.rb | 10 ++ .../notes_on_personal_snippets_spec.rb | 12 -- spec/frontend/behaviors/copy_as_gfm_spec.js | 50 +++--- spec/frontend/lib/utils/url_utility_spec.js | 34 ++++ .../releases/components/app_edit_new_spec.js | 45 ++++-- .../merge_request_diff_spec.rb | 2 +- spec/lib/gitlab/diff/file_spec.rb | 58 +++++-- spec/lib/gitlab/diff/parser_spec.rb | 10 ++ spec/lib/gitlab/git_access_spec.rb | 24 ++- spec/lib/gitlab/http_spec.rb | 41 +++++ spec/models/audit_event_spec.rb | 12 +- .../push_access_level_spec.rb | 2 +- spec/models/user_spec.rb | 98 ++++++++++- spec/policies/global_policy_spec.rb | 32 ++++ .../merge_requests/set_assignees_spec.rb | 14 +- spec/requests/api/graphql_spec.rb | 152 +++++++++++++++++- .../api/import_bitbucket_server_spec.rb | 2 +- spec/requests/api/projects_spec.rb | 2 +- spec/requests/api/users_spec.rb | 2 +- spec/requests/git_http_spec.rb | 62 ++++--- spec/requests/ide_controller_spec.rb | 119 +++++++------- .../feature_flags/create_service_spec.rb | 12 +- .../feature_flags/destroy_service_spec.rb | 2 +- .../feature_flags/update_service_spec.rb | 28 ++-- spec/services/projects/fork_service_spec.rb | 21 ++- .../repositories/changelog_service_spec.rb | 2 +- spec/support/helpers/graphql_helpers.rb | 21 ++- 78 files changed, 1033 insertions(+), 308 deletions(-) create mode 100644 app/graphql/mutations/echo.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index cf7d07b984..3a24fe297a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,37 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 13.12.6 (2021-07-01) + +### Added (1 change) + +- [Added omniauth_user check when verifying user cap](gitlab-org/security/gitlab@a61062501630c35820301e9f79a036219d1e3074) ([merge request](gitlab-org/security/gitlab!1502)) **GitLab Enterprise Edition** + +### Security (14 changes) + +- [Bump rails gem version to 6.0.3.7](gitlab-org/security/gitlab@58d27ba819867baadf535e0d8d91d0cb818dc8b6) ([merge request](gitlab-org/security/gitlab!1515)) +- [Update rdoc to 6.3.1](gitlab-org/security/gitlab@ead11a6974576b0b1a974985493c75143e3bd575) ([merge request](gitlab-org/security/gitlab!1534)) +- [Add sanitizing for name field](gitlab-org/security/gitlab@2c5672eae4323c2682245485b327850e68e7e5b4) ([merge request](gitlab-org/security/gitlab!1490)) +- [Forbid GET requests with mutations](gitlab-org/security/gitlab@2b01d6dc310451fa3022f1865470ca004bbd4c33) ([merge request](gitlab-org/security/gitlab!1529)) +- [Copy feature visibility settings to a fork](gitlab-org/security/gitlab@5ee923ba64fb34fc38f831fc206a153d8f7eae91) ([merge request](gitlab-org/security/gitlab!1523)) +- [Avoid disclosing project in web IDE](gitlab-org/security/gitlab@759d1361e7f359d681c4f55ea2b6f7e1d0bb1e53) ([merge request](gitlab-org/security/gitlab!1512)) +- [Add new username validation](gitlab-org/security/gitlab@e79625541d04b0d6c94614f2afc6aaeb2ef40083) ([merge request](gitlab-org/security/gitlab!1495)) +- [Allow only same-origin URLs for Edit Release Cancel button](gitlab-org/security/gitlab@e5bda0a7e03978afee494616e2054b8650b61d3e) ([merge request](gitlab-org/security/gitlab!1486)) +- [Update Nokogiri to 1.11.4](gitlab-org/security/gitlab@d71973da1850df059b1ec1422d50bbccace21ff2) ([merge request](gitlab-org/security/gitlab!1479)) +- [Fix deploy key fallback issue in protected branch](gitlab-org/security/gitlab@0411bc45885e1122c06dbff084b48bf03d78c6a8) ([merge request](gitlab-org/security/gitlab!1478)) +- [Fix XSS on audit log for feature flag actions](gitlab-org/security/gitlab@22e2f903c821e54ce6d4b4b749a009d14abc4a13) ([merge request](gitlab-org/security/gitlab!1474)) +- [Sanitize input on pasteGFM](gitlab-org/security/gitlab@7dc511ebc2e77c3d22cd34ca87449f32120a5229) ([merge request](gitlab-org/security/gitlab!1453)) +- [Add total http read timeout](gitlab-org/security/gitlab@37c24c82d5dfa57fad03f265e7ba92f6ef250c30) ([merge request](gitlab-org/security/gitlab!1427)) +- [Fix merge request diff display issue with unsupported encoding](gitlab-org/security/gitlab@7d05892daa6aaf951b941628e2af41e17977b140) ([merge request](gitlab-org/security/gitlab!1424)) + +## 13.12.5 (2021-06-21) + +### Fixed (3 changes) + +- [Fix failing spec](gitlab-org/gitlab@7d1a9b0155195eb082f5b33ba1310deed742a7a4) ([merge request](gitlab-org/gitlab!64488)) +- [Advanced Search Settings page does not load if the ES url is unreachable](gitlab-org/gitlab@80b262f0e79f02a89724ed4e3988e686f53c959c) ([merge request](gitlab-org/gitlab!64488)) **GitLab Enterprise Edition** +- [Fix Password expired error on git fetch via SSH for LDAP user](gitlab-org/gitlab@19a7d7a6d3cd43f1c7559c729532ad3b9dafb75c) ([merge request](gitlab-org/gitlab!64488)) + ## 13.12.4 (2021-06-14) ### Fixed (3 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 535687224a..1c42729484 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -13.12.4 \ No newline at end of file +13.12.6 \ No newline at end of file diff --git a/Gemfile b/Gemfile index 3aee2845ea..b5629f4a2a 100644 --- a/Gemfile +++ b/Gemfile @@ -2,7 +2,7 @@ source 'https://rubygems.org' -gem 'rails', '~> 6.0.3.6' +gem 'rails', '~> 6.0.3.7' gem 'bootsnap', '~> 1.4.6' @@ -157,7 +157,7 @@ gem 'github-markup', '~> 1.7.0', require: 'github/markup' gem 'commonmarker', '~> 0.21' gem 'kramdown', '~> 2.3.1' gem 'RedCloth', '~> 4.3.2' -gem 'rdoc', '~> 6.1.2' +gem 'gitlab-rdoc', '~> 6.3.2', require: 'rdoc' # We need this fork until rdoc releases a new version. See https://gitlab.com/gitlab-org/gitlab/-/issues/334695 gem 'org-ruby', '~> 0.9.12' gem 'creole', '~> 0.5.0' gem 'wikicloth', '0.8.1' @@ -168,7 +168,7 @@ gem 'asciidoctor-kroki', '~> 0.4.0', require: false gem 'rouge', '~> 3.26.0' gem 'truncato', '~> 0.7.11' gem 'bootstrap_form', '~> 4.2.0' -gem 'nokogiri', '~> 1.11.1' +gem 'nokogiri', '~> 1.11.4' gem 'escape_utils', '~> 1.1' # Calendar rendering diff --git a/Gemfile.lock b/Gemfile.lock index 23876a9ecc..351e7ec94a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -12,59 +12,59 @@ GEM abstract_type (0.0.7) acme-client (2.0.6) faraday (>= 0.17, < 2.0.0) - actioncable (6.0.3.6) - actionpack (= 6.0.3.6) + actioncable (6.0.3.7) + actionpack (= 6.0.3.7) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (6.0.3.6) - actionpack (= 6.0.3.6) - activejob (= 6.0.3.6) - activerecord (= 6.0.3.6) - activestorage (= 6.0.3.6) - activesupport (= 6.0.3.6) + actionmailbox (6.0.3.7) + actionpack (= 6.0.3.7) + activejob (= 6.0.3.7) + activerecord (= 6.0.3.7) + activestorage (= 6.0.3.7) + activesupport (= 6.0.3.7) mail (>= 2.7.1) - actionmailer (6.0.3.6) - actionpack (= 6.0.3.6) - actionview (= 6.0.3.6) - activejob (= 6.0.3.6) + actionmailer (6.0.3.7) + actionpack (= 6.0.3.7) + actionview (= 6.0.3.7) + activejob (= 6.0.3.7) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (6.0.3.6) - actionview (= 6.0.3.6) - activesupport (= 6.0.3.6) + actionpack (6.0.3.7) + actionview (= 6.0.3.7) + activesupport (= 6.0.3.7) rack (~> 2.0, >= 2.0.8) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (6.0.3.6) - actionpack (= 6.0.3.6) - activerecord (= 6.0.3.6) - activestorage (= 6.0.3.6) - activesupport (= 6.0.3.6) + actiontext (6.0.3.7) + actionpack (= 6.0.3.7) + activerecord (= 6.0.3.7) + activestorage (= 6.0.3.7) + activesupport (= 6.0.3.7) nokogiri (>= 1.8.5) - actionview (6.0.3.6) - activesupport (= 6.0.3.6) + actionview (6.0.3.7) + activesupport (= 6.0.3.7) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - activejob (6.0.3.6) - activesupport (= 6.0.3.6) + activejob (6.0.3.7) + activesupport (= 6.0.3.7) globalid (>= 0.3.6) - activemodel (6.0.3.6) - activesupport (= 6.0.3.6) - activerecord (6.0.3.6) - activemodel (= 6.0.3.6) - activesupport (= 6.0.3.6) + activemodel (6.0.3.7) + activesupport (= 6.0.3.7) + activerecord (6.0.3.7) + activemodel (= 6.0.3.7) + activesupport (= 6.0.3.7) activerecord-explain-analyze (0.1.0) activerecord (>= 4) pg - activestorage (6.0.3.6) - actionpack (= 6.0.3.6) - activejob (= 6.0.3.6) - activerecord (= 6.0.3.6) + activestorage (6.0.3.7) + actionpack (= 6.0.3.7) + activejob (= 6.0.3.7) + activerecord (= 6.0.3.7) marcel (~> 1.0.0) - activesupport (6.0.3.6) + activesupport (6.0.3.7) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) @@ -483,6 +483,7 @@ GEM addressable (~> 2.7) omniauth (~> 1.9) openid_connect (~> 1.2) + gitlab-rdoc (6.3.2) gitlab-sidekiq-fetcher (0.5.6) sidekiq (~> 5) gitlab-styles (6.2.0) @@ -781,7 +782,7 @@ GEM netrc (0.11.0) nio4r (2.5.4) no_proxy_fix (0.1.2) - nokogiri (1.11.3) + nokogiri (1.11.4) mini_portile2 (~> 2.5.0) racc (~> 1.4) nokogumbo (2.0.2) @@ -962,20 +963,20 @@ GEM rack-test (1.1.0) rack (>= 1.0, < 3) rack-timeout (0.5.2) - rails (6.0.3.6) - actioncable (= 6.0.3.6) - actionmailbox (= 6.0.3.6) - actionmailer (= 6.0.3.6) - actionpack (= 6.0.3.6) - actiontext (= 6.0.3.6) - actionview (= 6.0.3.6) - activejob (= 6.0.3.6) - activemodel (= 6.0.3.6) - activerecord (= 6.0.3.6) - activestorage (= 6.0.3.6) - activesupport (= 6.0.3.6) + rails (6.0.3.7) + actioncable (= 6.0.3.7) + actionmailbox (= 6.0.3.7) + actionmailer (= 6.0.3.7) + actionpack (= 6.0.3.7) + actiontext (= 6.0.3.7) + actionview (= 6.0.3.7) + activejob (= 6.0.3.7) + activemodel (= 6.0.3.7) + activerecord (= 6.0.3.7) + activestorage (= 6.0.3.7) + activesupport (= 6.0.3.7) bundler (>= 1.3.0) - railties (= 6.0.3.6) + railties (= 6.0.3.7) sprockets-rails (>= 2.0.0) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) @@ -989,9 +990,9 @@ GEM rails-i18n (6.0.0) i18n (>= 0.7, < 2) railties (>= 6.0.0, < 7) - railties (6.0.3.6) - actionpack (= 6.0.3.6) - activesupport (= 6.0.3.6) + railties (6.0.3.7) + actionpack (= 6.0.3.7) + activesupport (= 6.0.3.7) method_source rake (>= 0.8.7) thor (>= 0.20.3, < 2.0) @@ -1008,7 +1009,6 @@ GEM msgpack (>= 0.4.3) optimist (>= 3.0.0) rchardet (1.8.0) - rdoc (6.1.2) re2 (1.2.0) recaptcha (4.13.1) json @@ -1485,6 +1485,7 @@ DEPENDENCIES gitlab-markup (~> 1.7.1) gitlab-net-dns (~> 0.9.1) gitlab-omniauth-openid-connect (~> 0.4.0) + gitlab-rdoc (~> 6.3.2) gitlab-sidekiq-fetcher (= 0.5.6) gitlab-styles (~> 6.2.0) gitlab_chronic_duration (~> 0.10.6.2) @@ -1544,7 +1545,7 @@ DEPENDENCIES net-ldap (~> 0.16.3) net-ntp net-ssh (~> 6.0) - nokogiri (~> 1.11.1) + nokogiri (~> 1.11.4) oauth2 (~> 1.4) octokit (~> 4.15) ohai (~> 16.10) @@ -1587,14 +1588,13 @@ DEPENDENCIES rack-oauth2 (~> 1.16.0) rack-proxy (~> 0.6.0) rack-timeout (~> 0.5.1) - rails (~> 6.0.3.6) + rails (~> 6.0.3.7) rails-controller-testing rails-i18n (~> 6.0) rainbow (~> 3.0) raindrops (~> 0.18) rblineprof (~> 0.3.6) rbtrace (~> 0.4) - rdoc (~> 6.1.2) re2 (~> 1.2.0) recaptcha (~> 4.11) redis (~> 4.0) diff --git a/VERSION b/VERSION index 535687224a..1c42729484 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -13.12.4 \ No newline at end of file +13.12.6 \ No newline at end of file diff --git a/app/assets/javascripts/behaviors/markdown/copy_as_gfm.js b/app/assets/javascripts/behaviors/markdown/copy_as_gfm.js index 9a8af79210..19ebab3648 100644 --- a/app/assets/javascripts/behaviors/markdown/copy_as_gfm.js +++ b/app/assets/javascripts/behaviors/markdown/copy_as_gfm.js @@ -1,4 +1,5 @@ import $ from 'jquery'; +import { sanitize } from '~/lib/dompurify'; import { getSelectedFragment, insertText } from '~/lib/utils/common_utils'; export class CopyAsGFM { @@ -69,7 +70,7 @@ export class CopyAsGFM { } else { // Due to the async copy call we are not able to produce gfm so we transform the cached HTML const div = document.createElement('div'); - div.innerHTML = gfmHtml; + div.innerHTML = sanitize(gfmHtml); CopyAsGFM.nodeToGFM(div) .then((transformedGfm) => { CopyAsGFM.insertPastedText(e.target, text, transformedGfm); diff --git a/app/assets/javascripts/lib/utils/url_utility.js b/app/assets/javascripts/lib/utils/url_utility.js index 5b3aa3cf9d..5ef39ded9c 100644 --- a/app/assets/javascripts/lib/utils/url_utility.js +++ b/app/assets/javascripts/lib/utils/url_utility.js @@ -535,3 +535,27 @@ export function getURLOrigin(url) { return null; } } + +/** + * Returns `true` if the given `url` resolves to the same origin the page is served + * from; otherwise, returns `false`. + * + * The `url` may be absolute or relative. + * + * @param {string} url The URL to check. + * @returns {boolean} + */ +export function isSameOriginUrl(url) { + if (typeof url !== 'string') { + return false; + } + + const { origin } = window.location; + + try { + return new URL(url, origin).origin === origin; + } catch { + // Invalid URLs cannot have the same origin + return false; + } +} diff --git a/app/assets/javascripts/releases/components/app_edit_new.vue b/app/assets/javascripts/releases/components/app_edit_new.vue index aecd0d6371..3774f97a06 100644 --- a/app/assets/javascripts/releases/components/app_edit_new.vue +++ b/app/assets/javascripts/releases/components/app_edit_new.vue @@ -2,6 +2,7 @@ import { GlButton, GlFormInput, GlFormGroup, GlSprintf } from '@gitlab/ui'; import { mapState, mapActions, mapGetters } from 'vuex'; import { getParameterByName } from '~/lib/utils/common_utils'; +import { isSameOriginUrl } from '~/lib/utils/url_utility'; import { __ } from '~/locale'; import MilestoneCombobox from '~/milestones/components/milestone_combobox.vue'; import { BACK_URL_PARAM } from '~/releases/constants'; @@ -65,7 +66,13 @@ export default { }, }, cancelPath() { - return getParameterByName(BACK_URL_PARAM) || this.releasesPagePath; + const backUrl = getParameterByName(BACK_URL_PARAM); + + if (isSameOriginUrl(backUrl)) { + return backUrl; + } + + return this.releasesPagePath; }, saveButtonLabel() { return this.isExistingRelease ? __('Save changes') : __('Create release'); diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/not_diffable.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/not_diffable.vue index d4d3038f06..5a6b1c1902 100644 --- a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/not_diffable.vue +++ b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/not_diffable.vue @@ -1,5 +1,5 @@ diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 80cb04ac49..2b6b64c8fd 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -208,7 +208,10 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController params[:application_setting][:import_sources]&.delete("") params[:application_setting][:restricted_visibility_levels]&.delete("") - params[:application_setting][:required_instance_ci_template] = nil if params[:application_setting][:required_instance_ci_template].blank? + + if params[:application_setting].key?(:required_instance_ci_template) + params[:application_setting][:required_instance_ci_template] = nil if params[:application_setting][:required_instance_ci_template].empty? + end remove_blank_params_for!(:elasticsearch_aws_secret_access_key, :eks_secret_access_key) @@ -217,9 +220,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController params.delete(:domain_denylist_raw) if params[:domain_denylist] params.delete(:domain_allowlist_raw) if params[:domain_allowlist] - params.require(:application_setting).permit( - visible_application_setting_attributes - ) + params[:application_setting].permit(visible_application_setting_attributes) end def recheck_user_consent? diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index 20861afbb8..a4b64a7848 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -108,7 +108,7 @@ module MembershipActions respond_to do |format| format.html do - redirect_path = member.request? ? member.source : [:dashboard, membershipable.class.to_s.tableize] + redirect_path = member.request? ? member.source : [:dashboard, membershipable.class.to_s.tableize.to_sym] redirect_to redirect_path, notice: notice end diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index 725d8b62c7..515fbd7b48 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -20,12 +20,16 @@ class GraphqlController < ApplicationController # around in GraphiQL. protect_from_forgery with: :null_session, only: :execute - before_action :authorize_access_api! + # must come first: current_user is set up here before_action(only: [:execute]) { authenticate_sessionless_user!(:api) } + + before_action :authorize_access_api! before_action :set_user_last_activity before_action :track_vs_code_usage before_action :disable_query_limiting + before_action :disallow_mutations_for_get + # Since we deactivate authentication from the main ApplicationController and # defer it to :authorize_access_api!, we need to override the bypass session # callback execution order here @@ -62,6 +66,25 @@ class GraphqlController < ApplicationController private + def disallow_mutations_for_get + return unless request.get? || request.head? + return unless any_mutating_query? + + raise ::Gitlab::Graphql::Errors::ArgumentError, "Mutations are forbidden in #{request.request_method} requests" + end + + def any_mutating_query? + if multiplex? + multiplex_queries.any? { |q| mutation?(q[:query], q[:operation_name]) } + else + mutation?(query) + end + end + + def mutation?(query_string, operation_name = params[:operationName]) + ::GraphQL::Query.new(GitlabSchema, query_string, operation_name: operation_name).mutation? + end + # Tests may mark some GraphQL queries as exempt from SQL query limits def disable_query_limiting return unless Gitlab::QueryLimiting.enabled_for_env? @@ -130,7 +153,9 @@ class GraphqlController < ApplicationController end def authorize_access_api! - access_denied!("API not accessible for user.") unless can?(current_user, :access_api) + return if can?(current_user, :access_api) + + render_error('API not accessible for user', status: :forbidden) end # Overridden from the ApplicationController to make the response look like diff --git a/app/controllers/ide_controller.rb b/app/controllers/ide_controller.rb index 4c7a91ee60..44beceb4f4 100644 --- a/app/controllers/ide_controller.rb +++ b/app/controllers/ide_controller.rb @@ -7,6 +7,8 @@ class IdeController < ApplicationController include StaticObjectExternalStorageCSP include Gitlab::Utils::StrongMemoize + before_action :authorize_read_project! + before_action do push_frontend_feature_flag(:build_service_proxy) push_frontend_feature_flag(:schema_linting) @@ -22,6 +24,10 @@ class IdeController < ApplicationController private + def authorize_read_project! + render_404 unless can?(current_user, :read_project, project) + end + def define_index_vars return unless project diff --git a/app/graphql/mutations/echo.rb b/app/graphql/mutations/echo.rb new file mode 100644 index 0000000000..61d39009ba --- /dev/null +++ b/app/graphql/mutations/echo.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Mutations + class Echo < BaseMutation + graphql_name 'EchoCreate' + description <<~DOC + A mutation that does not perform any changes. + + This is expected to be used for testing of endpoints, to verify + that a user has mutation access. + DOC + + argument :errors, + type: [::GraphQL::STRING_TYPE], + required: false, + description: 'Errors to return to the user.' + + argument :messages, + type: [::GraphQL::STRING_TYPE], + as: :echoes, + required: false, + description: 'Messages to return to the user.' + + field :echoes, + type: [::GraphQL::STRING_TYPE], + null: true, + description: 'Messages returned to the user.' + + def resolve(**args) + args + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 54a06ed534..aec3fdde8b 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -101,6 +101,7 @@ module Types mount_mutation Mutations::Ci::Job::Retry mount_mutation Mutations::Namespace::PackageSettings::Update mount_mutation Mutations::UserCallouts::Create + mount_mutation Mutations::Echo end end diff --git a/app/models/audit_event.rb b/app/models/audit_event.rb index aff7eef462..11036b76fc 100644 --- a/app/models/audit_event.rb +++ b/app/models/audit_event.rb @@ -32,6 +32,9 @@ class AuditEvent < ApplicationRecord scope :by_author_id, -> (author_id) { where(author_id: author_id) } after_initialize :initialize_details + + before_validation :sanitize_message + # Note: The intention is to remove this once refactoring of AuditEvent # has proceeded further. # @@ -83,6 +86,14 @@ class AuditEvent < ApplicationRecord private + def sanitize_message + message = details[:custom_message] + + return unless message + + self.details = details.merge(custom_message: Sanitize.clean(message)) + end + def default_author_value ::Gitlab::Audit::NullAuthor.for(author_id, (self[:author_name] || details[:author_name])) end diff --git a/app/models/integrations/bamboo.rb b/app/models/integrations/bamboo.rb index 82111c7322..5f43eae1e8 100644 --- a/app/models/integrations/bamboo.rb +++ b/app/models/integrations/bamboo.rb @@ -173,6 +173,7 @@ module Integrations query_params[:os_authType] = 'basic' params[:basic_auth] = basic_auth + params[:use_read_total_timeout] = true params end diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb index ab1ba768a8..581bd8077a 100644 --- a/app/models/project_services/drone_ci_service.rb +++ b/app/models/project_services/drone_ci_service.rb @@ -50,9 +50,11 @@ class DroneCiService < CiService end def calculate_reactive_cache(sha, ref) - response = Gitlab::HTTP.try_get(commit_status_path(sha, ref), + response = Gitlab::HTTP.try_get( + commit_status_path(sha, ref), verify: enable_ssl_verification, - extra_log_info: { project_id: project_id }) + extra_log_info: { project_id: project_id }, + use_read_total_timeout: true) status = if response && response.code == 200 && response['status'] diff --git a/app/models/project_services/external_wiki_service.rb b/app/models/project_services/external_wiki_service.rb index f49b008533..6798b8fb69 100644 --- a/app/models/project_services/external_wiki_service.rb +++ b/app/models/project_services/external_wiki_service.rb @@ -38,7 +38,7 @@ class ExternalWikiService < Integration end def execute(_data) - response = Gitlab::HTTP.get(properties['external_wiki_url'], verify: true) + response = Gitlab::HTTP.get(properties['external_wiki_url'], verify: true, use_read_total_timeout: true) response.body if response.code == 200 rescue StandardError nil diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index 099e3c336d..ba78b1a8dc 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -106,7 +106,7 @@ class IssueTrackerService < Integration result = false begin - response = Gitlab::HTTP.head(self.project_url, verify: true) + response = Gitlab::HTTP.head(self.project_url, verify: true, use_read_total_timeout: true) if response message = "#{self.type} received response #{response.code} when attempting to connect to #{self.project_url}" diff --git a/app/models/project_services/mock_ci_service.rb b/app/models/project_services/mock_ci_service.rb index bd6344c6e1..ebe6179d74 100644 --- a/app/models/project_services/mock_ci_service.rb +++ b/app/models/project_services/mock_ci_service.rb @@ -56,7 +56,7 @@ class MockCiService < CiService # # def commit_status(sha, ref) - response = Gitlab::HTTP.get(commit_status_path(sha), verify: false) + response = Gitlab::HTTP.get(commit_status_path(sha), verify: false, use_read_total_timeout: true) read_commit_status(response) rescue Errno::ECONNREFUSED :error diff --git a/app/models/project_services/slack_mattermost/notifier.rb b/app/models/project_services/slack_mattermost/notifier.rb index 1a78cea593..39ee571824 100644 --- a/app/models/project_services/slack_mattermost/notifier.rb +++ b/app/models/project_services/slack_mattermost/notifier.rb @@ -17,7 +17,7 @@ module SlackMattermost class HTTPClient def self.post(uri, params = {}) params.delete(:http_options) # these are internal to the client and we do not want them - Gitlab::HTTP.post(uri, body: params) + Gitlab::HTTP.post(uri, body: params, use_read_total_timeout: true) end end end diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb index 6fc24a4778..c3390dd9c2 100644 --- a/app/models/project_services/teamcity_service.rb +++ b/app/models/project_services/teamcity_service.rb @@ -169,7 +169,7 @@ class TeamcityService < CiService end def get_path(path) - Gitlab::HTTP.try_get(build_url(path), verify: false, basic_auth: basic_auth, extra_log_info: { project_id: project_id }) + Gitlab::HTTP.try_get(build_url(path), verify: false, basic_auth: basic_auth, extra_log_info: { project_id: project_id }, use_read_total_timeout: true) end def post_to_build_queue(data, branch) @@ -179,7 +179,8 @@ class TeamcityService < CiService ""\ '', headers: { 'Content-type' => 'application/xml' }, - basic_auth: basic_auth + basic_auth: basic_auth, + use_read_total_timeout: true ) end diff --git a/app/models/project_services/unify_circuit_service.rb b/app/models/project_services/unify_circuit_service.rb index 5f43388e1c..f4e93cf844 100644 --- a/app/models/project_services/unify_circuit_service.rb +++ b/app/models/project_services/unify_circuit_service.rb @@ -48,7 +48,8 @@ class UnifyCircuitService < ChatNotificationService response = Gitlab::HTTP.post(webhook, body: { subject: message.project_name, text: message.summary, - markdown: true + markdown: true, + use_read_total_timeout: true }.to_json) response if response.success? diff --git a/app/models/project_services/webex_teams_service.rb b/app/models/project_services/webex_teams_service.rb index 3d92d3bb85..37f8a1af6f 100644 --- a/app/models/project_services/webex_teams_service.rb +++ b/app/models/project_services/webex_teams_service.rb @@ -43,7 +43,7 @@ class WebexTeamsService < ChatNotificationService def notify(message, opts) header = { 'Content-Type' => 'application/json' } - response = Gitlab::HTTP.post(webhook, headers: header, body: { markdown: message.summary }.to_json) + response = Gitlab::HTTP.post(webhook, headers: header, body: { markdown: message.summary }.to_json, use_read_total_timeout: true) response if response.success? end diff --git a/app/models/protected_branch/push_access_level.rb b/app/models/protected_branch/push_access_level.rb index ea51dca8a4..5248834a2f 100644 --- a/app/models/protected_branch/push_access_level.rb +++ b/app/models/protected_branch/push_access_level.rb @@ -20,7 +20,7 @@ class ProtectedBranch::PushAccessLevel < ApplicationRecord def check_access(user) if user && deploy_key.present? - return true if user.can?(:read_project, project) && enabled_deploy_key_for_user?(deploy_key, user) + return user.can?(:read_project, project) && enabled_deploy_key_for_user?(deploy_key, user) end super diff --git a/app/models/user.rb b/app/models/user.rb index 0eb58baae1..5feb3485b8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -238,6 +238,7 @@ class User < ApplicationRecord validate :owns_commit_email, if: :commit_email_changed? validate :signup_domain_valid?, on: :create, if: ->(user) { !user.created_by_id } validate :check_email_restrictions, on: :create, if: ->(user) { !user.created_by_id } + validate :check_username_format, if: :username_changed? validates :theme_id, allow_nil: true, inclusion: { in: Gitlab::Themes.valid_ids, message: _("%{placeholder} is not a valid theme") % { placeholder: '%{value}' } } @@ -1255,12 +1256,23 @@ class User < ApplicationRecord end def sanitize_attrs + sanitize_links + sanitize_name + end + + def sanitize_links %i[skype linkedin twitter].each do |attr| value = self[attr] self[attr] = Sanitize.clean(value) if value.present? end end + def sanitize_name + return unless self.name + + self.name = self.name.gsub(%r{]*>}, '') + end + def set_notification_email if notification_email.blank? || all_emails.exclude?(notification_email) self.notification_email = email @@ -1873,6 +1885,12 @@ class User < ApplicationRecord !!(password_expires_at && password_expires_at < Time.current) end + def password_expired_if_applicable? + return false unless allow_password_authentication? + + password_expired? + end + def can_be_deactivated? active? && no_recent_activity? && !internal? end @@ -2066,6 +2084,12 @@ class User < ApplicationRecord end end + def check_username_format + return if username.blank? || Mime::EXTENSION_LOOKUP.keys.none? { |type| username.end_with?(type) } + + errors.add(:username, _('ending with MIME type format is not allowed.')) + end + def groups_with_developer_maintainer_project_access project_creation_levels = [::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS] diff --git a/app/policies/concerns/policy_actor.rb b/app/policies/concerns/policy_actor.rb index 08a26da667..790ab3eb71 100644 --- a/app/policies/concerns/policy_actor.rb +++ b/app/policies/concerns/policy_actor.rb @@ -81,7 +81,7 @@ module PolicyActor false end - def password_expired? + def password_expired_if_applicable? false end end diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb index 73757891cd..4e738abcc0 100644 --- a/app/policies/global_policy.rb +++ b/app/policies/global_policy.rb @@ -16,7 +16,7 @@ class GlobalPolicy < BasePolicy end condition(:password_expired, scope: :user) do - @user&.password_expired? + @user&.password_expired_if_applicable? end condition(:project_bot, scope: :user) { @user&.project_bot? } diff --git a/app/services/feature_flags/base_service.rb b/app/services/feature_flags/base_service.rb index f48f95e255..d041703803 100644 --- a/app/services/feature_flags/base_service.rb +++ b/app/services/feature_flags/base_service.rb @@ -49,9 +49,9 @@ module FeatureFlags end def created_scope_message(scope) - "Created rule #{scope.environment_scope} "\ - "and set it as #{scope.active ? "active" : "inactive"} "\ - "with strategies #{scope.strategies}." + "Created rule #{scope.environment_scope} "\ + "and set it as #{scope.active ? "active" : "inactive"} "\ + "with strategies #{scope.strategies}." end def feature_flag_by_name diff --git a/app/services/feature_flags/create_service.rb b/app/services/feature_flags/create_service.rb index de3a55d10f..5c87af561d 100644 --- a/app/services/feature_flags/create_service.rb +++ b/app/services/feature_flags/create_service.rb @@ -22,8 +22,7 @@ module FeatureFlags private def audit_message(feature_flag) - message_parts = ["Created feature flag #{feature_flag.name}", - "with description \"#{feature_flag.description}\"."] + message_parts = ["Created feature flag #{feature_flag.name} with description \"#{feature_flag.description}\"."] message_parts += feature_flag.scopes.map do |scope| created_scope_message(scope) diff --git a/app/services/feature_flags/destroy_service.rb b/app/services/feature_flags/destroy_service.rb index c77e3e03ec..b131a349fc 100644 --- a/app/services/feature_flags/destroy_service.rb +++ b/app/services/feature_flags/destroy_service.rb @@ -23,7 +23,7 @@ module FeatureFlags end def audit_message(feature_flag) - "Deleted feature flag #{feature_flag.name}." + "Deleted feature flag #{feature_flag.name}." end def can_destroy?(feature_flag) diff --git a/app/services/feature_flags/update_service.rb b/app/services/feature_flags/update_service.rb index d956d4b335..f5ab6f4005 100644 --- a/app/services/feature_flags/update_service.rb +++ b/app/services/feature_flags/update_service.rb @@ -45,14 +45,14 @@ module FeatureFlags return if changes.empty? - "Updated feature flag #{feature_flag.name}. " + changes.join(" ") + "Updated feature flag #{feature_flag.name}. " + changes.join(" ") end def changed_attributes_messages(feature_flag) feature_flag.changes.slice(*AUDITABLE_ATTRIBUTES).map do |attribute_name, changes| "Updated #{attribute_name} "\ - "from \"#{changes.first}\" to "\ - "\"#{changes.second}\"." + "from \"#{changes.first}\" to "\ + "\"#{changes.second}\"." end end @@ -69,17 +69,17 @@ module FeatureFlags end def deleted_scope_message(scope) - "Deleted rule #{scope.environment_scope}." + "Deleted rule #{scope.environment_scope}." end def updated_scope_message(scope) changes = scope.changes.slice(*AUDITABLE_SCOPE_ATTRIBUTES_HUMAN_NAMES.keys) return if changes.empty? - message = "Updated rule #{scope.environment_scope} " + message = "Updated rule #{scope.environment_scope} " message += changes.map do |attribute_name, change| name = AUDITABLE_SCOPE_ATTRIBUTES_HUMAN_NAMES[attribute_name] - "#{name} from #{change.first} to #{change.second}" + "#{name} from #{change.first} to #{change.second}" end.join(' ') message + '.' diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index fd9b64a4ee..3cee1b5975 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -34,8 +34,9 @@ module Projects new_project = CreateService.new(current_user, new_fork_params).execute return new_project unless new_project.persisted? - builds_access_level = @project.project_feature.builds_access_level - new_project.project_feature.update(builds_access_level: builds_access_level) + new_project.project_feature.update!( + @project.project_feature.slice(ProjectFeature::FEATURES.map { |f| "#{f}_access_level" }) + ) new_project end diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 654d935673..c2f12c133c 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -41,6 +41,7 @@ class WebHookService @hook_name = hook_name.to_s @request_options = { timeout: Gitlab.config.gitlab.webhook_timeout, + use_read_total_timeout: true, allow_local_requests: hook.allow_local_requests? } end @@ -67,7 +68,7 @@ class WebHookService { status: :success, http_status: response.code, - message: response.to_s + message: response.body } rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Net::OpenTimeout, Net::ReadTimeout, Gitlab::HTTP::BlockedUrlError, Gitlab::HTTP::RedirectionTooDeep, diff --git a/app/views/import/_githubish_status.html.haml b/app/views/import/_githubish_status.html.haml index 4cf08b1d2b..221529a048 100644 --- a/app/views/import/_githubish_status.html.haml +++ b/app/views/import/_githubish_status.html.haml @@ -1,5 +1,5 @@ - add_page_specific_style 'page_bundles/import' -- provider = local_assigns.fetch(:provider) +- provider = local_assigns.fetch(:provider).to_sym - extra_data = local_assigns.fetch(:extra_data, {}) - filterable = local_assigns.fetch(:filterable, true) - paginatable = local_assigns.fetch(:paginatable, false) diff --git a/app/views/projects/diffs/viewers/_not_diffable.html.haml b/app/views/projects/diffs/viewers/_not_diffable.html.haml index 7c55e272f5..63034331f6 100644 --- a/app/views/projects/diffs/viewers/_not_diffable.html.haml +++ b/app/views/projects/diffs/viewers/_not_diffable.html.haml @@ -1,2 +1,2 @@ .nothing-here-block - = _("This diff was suppressed by a .gitattributes entry.") + = _("File suppressed by a .gitattributes entry or the file's encoding is unsupported.") diff --git a/app/views/shared/milestones/_issuable.html.haml b/app/views/shared/milestones/_issuable.html.haml index a62ed00955..184904dd7a 100644 --- a/app/views/shared/milestones/_issuable.html.haml +++ b/app/views/shared/milestones/_issuable.html.haml @@ -3,7 +3,7 @@ - labels = issuable.labels - assignees = issuable.assignees - base_url_args = [project] -- issuable_type_args = base_url_args + [issuable.class.table_name] +- issuable_type_args = base_url_args + [issuable.class.table_name.to_sym] - issuable_url_args = base_url_args + [issuable] %li.issuable-row diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 7f496e1ade..817051519e 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -1959,6 +1959,31 @@ Input type: `DismissVulnerabilityInput` | `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | `vulnerability` | [`Vulnerability`](#vulnerability) | The vulnerability after dismissal. | +### `Mutation.echoCreate` + +A mutation that does not perform any changes. + +This is expected to be used for testing of endpoints, to verify +that a user has mutation access. + +Input type: `EchoCreateInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `errors` | [`[String!]`](#string) | Errors to return to the user. | +| `messages` | [`[String!]`](#string) | Messages to return to the user. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `echoes` | [`[String!]`](#string) | Messages returned to the user. | +| `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | + ### `Mutation.environmentsCanaryIngressUpdate` Input type: `EnvironmentsCanaryIngressUpdateInput` diff --git a/lib/api/members.rb b/lib/api/members.rb index a1a733ea7a..4898ad91d3 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -96,6 +96,8 @@ module API end # rubocop: disable CodeReuse/ActiveRecord post ":id/members" do + ::Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/333434') + source = find_source(source_type, params[:id]) authorize_admin_source!(source_type, source) diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 4489fc9f3b..7cae7e4ea8 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -382,7 +382,7 @@ module Gitlab end def can_user_login_with_non_expired_password?(user) - user.can?(:log_in) && !user.password_expired? + user.can?(:log_in) && !user.password_expired_if_applicable? end end end diff --git a/lib/gitlab/auth/user_access_denied_reason.rb b/lib/gitlab/auth/user_access_denied_reason.rb index 6639000dba..904759919a 100644 --- a/lib/gitlab/auth/user_access_denied_reason.rb +++ b/lib/gitlab/auth/user_access_denied_reason.rb @@ -23,6 +23,8 @@ module Gitlab "Your primary email address is not confirmed. "\ "Please check your inbox for the confirmation instructions. "\ "In case the link is expired, you can request a new confirmation email at #{Rails.application.routes.url_helpers.new_user_confirmation_url}" + when :blocked + "Your account has been blocked." when :password_expired "Your password expired. "\ "Please access GitLab from a web browser to update your password." @@ -44,6 +46,8 @@ module Gitlab :deactivated elsif !@user.confirmed? :unconfirmed + elsif @user.blocked? + :blocked elsif @user.password_expired? :password_expired else diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index dcd4bbdabf..35581952f4 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -250,7 +250,7 @@ module Gitlab end def diffable? - repository.attributes(file_path).fetch('diff') { true } + diffable_by_attribute? && !text_with_binary_notice? end def binary_in_repo? @@ -366,6 +366,15 @@ module Gitlab private + def diffable_by_attribute? + repository.attributes(file_path).fetch('diff') { true } + end + + # NOTE: Files with unsupported encodings (e.g. UTF-16) are treated as binary by git, but they are recognized as text files during encoding detection. These files have `Binary files a/filename and b/filename differ' as their raw diff content which cannot be used. We need to handle this special case and avoid displaying incorrect diff. + def text_with_binary_notice? + text? && has_binary_notice? + end + def fetch_blob(sha, path) return unless sha diff --git a/lib/gitlab/diff/parser.rb b/lib/gitlab/diff/parser.rb index 4a47e4b80b..adb711ca89 100644 --- a/lib/gitlab/diff/parser.rb +++ b/lib/gitlab/diff/parser.rb @@ -6,7 +6,7 @@ module Gitlab include Enumerable def parse(lines, diff_file: nil) - return [] if lines.blank? + return [] if lines.blank? || Git::Diff.has_binary_notice?(lines.first) @lines = lines line_obj_index = 0 diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb index 53df0b7b38..8325eadce2 100644 --- a/lib/gitlab/git/diff.rb +++ b/lib/gitlab/git/diff.rb @@ -33,6 +33,8 @@ module Gitlab SERIALIZE_KEYS = %i(diff new_path old_path a_mode b_mode new_file renamed_file deleted_file too_large).freeze + BINARY_NOTICE_PATTERN = %r(Binary files a\/(.*) and b\/(.*) differ).freeze + class << self def between(repo, head, base, options = {}, *paths) straight = options.delete(:straight) || false @@ -131,8 +133,13 @@ module Gitlab def patch_hard_limit_bytes Gitlab::CurrentSettings.diff_max_patch_bytes end - end + def has_binary_notice?(text) + return false unless text.present? + + text.start_with?(BINARY_NOTICE_PATTERN) + end + end def initialize(raw_diff, expanded: true) @expanded = expanded @@ -215,7 +222,7 @@ module Gitlab end def has_binary_notice? - @diff.start_with?('Binary') + self.class.has_binary_notice?(@diff) end private diff --git a/lib/gitlab/http.rb b/lib/gitlab/http.rb index be87dcc0ff..7e45cd216f 100644 --- a/lib/gitlab/http.rb +++ b/lib/gitlab/http.rb @@ -8,9 +8,10 @@ module Gitlab class HTTP BlockedUrlError = Class.new(StandardError) RedirectionTooDeep = Class.new(StandardError) + ReadTotalTimeout = Class.new(Net::ReadTimeout) HTTP_TIMEOUT_ERRORS = [ - Net::OpenTimeout, Net::ReadTimeout, Net::WriteTimeout + Net::OpenTimeout, Net::ReadTimeout, Net::WriteTimeout, Gitlab::HTTP::ReadTotalTimeout ].freeze HTTP_ERRORS = HTTP_TIMEOUT_ERRORS + [ SocketError, OpenSSL::SSL::SSLError, OpenSSL::OpenSSLError, @@ -23,6 +24,7 @@ module Gitlab read_timeout: 20, write_timeout: 30 }.freeze + DEFAULT_READ_TOTAL_TIMEOUT = 20.seconds include HTTParty # rubocop:disable Gitlab/HTTParty @@ -41,7 +43,19 @@ module Gitlab options end - httparty_perform_request(http_method, path, options_with_timeouts, &block) + unless options.has_key?(:use_read_total_timeout) + return httparty_perform_request(http_method, path, options_with_timeouts, &block) + end + + start_time = Gitlab::Metrics::System.monotonic_time + read_total_timeout = options.fetch(:timeout, DEFAULT_READ_TOTAL_TIMEOUT) + + httparty_perform_request(http_method, path, options_with_timeouts) do |fragment| + elapsed = Gitlab::Metrics::System.monotonic_time - start_time + raise ReadTotalTimeout, "Request timed out after #{elapsed} seconds" if elapsed > read_total_timeout + + block.call fragment if block + end rescue HTTParty::RedirectionTooDeep raise RedirectionTooDeep rescue *HTTP_ERRORS => e diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb index c7f2adb27d..2e8564b6e0 100644 --- a/lib/gitlab/lfs_token.rb +++ b/lib/gitlab/lfs_token.rb @@ -52,7 +52,7 @@ module Gitlab def valid_user? return true unless user? - !actor.blocked? && (!actor.allow_password_authentication? || !actor.password_expired?) + !actor.blocked? && !actor.password_expired_if_applicable? end def authentication_payload(repository_http_path) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a64a2cdb71..9f94ae98a6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -14015,6 +14015,9 @@ msgstr "" msgid "File renamed with no changes." msgstr "" +msgid "File suppressed by a .gitattributes entry or the file's encoding is unsupported." +msgstr "" + msgid "File synchronization concurrency limit" msgstr "" @@ -33097,9 +33100,6 @@ msgstr "" msgid "This diff is collapsed." msgstr "" -msgid "This diff was suppressed by a .gitattributes entry." -msgstr "" - msgid "This directory" msgstr "" @@ -38357,6 +38357,9 @@ msgstr "" msgid "encrypted: needs to be a :required, :optional or :migrating!" msgstr "" +msgid "ending with MIME type format is not allowed." +msgstr "" + msgid "entries cannot be larger than 255 characters" msgstr "" diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index f2d86b1b16..aed97a01a7 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -44,7 +44,7 @@ RSpec.describe GraphqlController do expect(response).to have_gitlab_http_status(:ok) end - it 'returns access denied template when user cannot access API' do + it 'returns forbidden when user cannot access API' do # User cannot access API in a couple of cases # * When user is internal(like ghost users) # * When user is blocked @@ -54,7 +54,9 @@ RSpec.describe GraphqlController do post :execute expect(response).to have_gitlab_http_status(:forbidden) - expect(response).to render_template('errors/access_denied') + expect(json_response).to include( + 'errors' => include(a_hash_including('message' => /API not accessible/)) + ) end it 'updates the users last_activity_on field' do diff --git a/spec/deprecation_toolkit_env.rb b/spec/deprecation_toolkit_env.rb index 57d32b5423..00d66ff3cd 100644 --- a/spec/deprecation_toolkit_env.rb +++ b/spec/deprecation_toolkit_env.rb @@ -55,9 +55,9 @@ module DeprecationToolkitEnv # one by one def self.allowed_kwarg_warning_paths %w[ - activerecord-6.0.3.6/lib/active_record/migration.rb - activesupport-6.0.3.6/lib/active_support/cache.rb - activerecord-6.0.3.6/lib/active_record/relation.rb + activerecord-6.0.3.7/lib/active_record/migration.rb + activesupport-6.0.3.7/lib/active_support/cache.rb + activerecord-6.0.3.7/lib/active_record/relation.rb asciidoctor-2.0.12/lib/asciidoctor/extensions.rb attr_encrypted-3.1.0/lib/attr_encrypted/adapters/active_record.rb ] diff --git a/spec/features/expand_collapse_diffs_spec.rb b/spec/features/expand_collapse_diffs_spec.rb index cbd1ae628d..add4af2bcd 100644 --- a/spec/features/expand_collapse_diffs_spec.rb +++ b/spec/features/expand_collapse_diffs_spec.rb @@ -253,7 +253,7 @@ RSpec.describe 'Expand and collapse diffs', :js do click_link('Expand all') # Wait for elements to appear to ensure full page reload - expect(page).to have_content('This diff was suppressed by a .gitattributes entry') + expect(page).to have_content("File suppressed by a .gitattributes entry or the file's encoding is unsupported.") expect(page).to have_content('This source diff could not be displayed because it is too large.') expect(page).to have_content('too_large_image.jpg') find('.note-textarea') diff --git a/spec/features/projects/diffs/diff_show_spec.rb b/spec/features/projects/diffs/diff_show_spec.rb index e47f36c4b7..56506ada3c 100644 --- a/spec/features/projects/diffs/diff_show_spec.rb +++ b/spec/features/projects/diffs/diff_show_spec.rb @@ -174,4 +174,14 @@ RSpec.describe 'Diff file viewer', :js, :with_clean_rails_cache do end end end + + context 'when the the encoding of the file is unsupported' do + before do + visit_commit('f05a98786e4274708e1fa118c7ad3a29d1d1b9a3') + end + + it 'shows it is not diffable' do + expect(page).to have_content("File suppressed by a .gitattributes entry or the file's encoding is unsupported.") + end + end end diff --git a/spec/features/snippets/notes_on_personal_snippets_spec.rb b/spec/features/snippets/notes_on_personal_snippets_spec.rb index 47dad9bd88..e03f71c535 100644 --- a/spec/features/snippets/notes_on_personal_snippets_spec.rb +++ b/spec/features/snippets/notes_on_personal_snippets_spec.rb @@ -65,18 +65,6 @@ RSpec.describe 'Comments on personal snippets', :js do expect(page).to have_content(user_name) end end - - context 'when the author name contains HTML' do - let(:user_name) { '

Fake Content

' } - - it 'renders the name as plain text' do - visit snippet_path(snippet) - - content = find("#note_#{snippet_notes[0].id} .note-header-author-name").text - - expect(content).to eq user_name - end - end end context 'when submitting a note' do diff --git a/spec/frontend/behaviors/copy_as_gfm_spec.js b/spec/frontend/behaviors/copy_as_gfm_spec.js index acff990e84..557b609f5f 100644 --- a/spec/frontend/behaviors/copy_as_gfm_spec.js +++ b/spec/frontend/behaviors/copy_as_gfm_spec.js @@ -1,50 +1,54 @@ import initCopyAsGFM, { CopyAsGFM } from '~/behaviors/markdown/copy_as_gfm'; -import * as commonUtils from '~/lib/utils/common_utils'; describe('CopyAsGFM', () => { describe('CopyAsGFM.pasteGFM', () => { - function callPasteGFM() { + let target; + + beforeEach(() => { + target = document.createElement('input'); + target.value = 'This is code: '; + }); + + // When GFM code is copied, we put the regular plain text + // on the clipboard as `text/plain`, and the GFM as `text/x-gfm`. + // This emulates the behavior of `getData` with that data. + function callPasteGFM(data = { 'text/plain': 'code', 'text/x-gfm': '`code`' }) { const e = { originalEvent: { clipboardData: { getData(mimeType) { - // When GFM code is copied, we put the regular plain text - // on the clipboard as `text/plain`, and the GFM as `text/x-gfm`. - // This emulates the behavior of `getData` with that data. - if (mimeType === 'text/plain') { - return 'code'; - } - if (mimeType === 'text/x-gfm') { - return '`code`'; - } - return null; + return data[mimeType] || null; }, }, }, preventDefault() {}, + target, }; CopyAsGFM.pasteGFM(e); } it('wraps pasted code when not already in code tags', () => { - jest.spyOn(commonUtils, 'insertText').mockImplementation((el, textFunc) => { - const insertedText = textFunc('This is code: ', ''); - - expect(insertedText).toEqual('`code`'); - }); - callPasteGFM(); + + expect(target.value).toBe('This is code: `code`'); }); it('does not wrap pasted code when already in code tags', () => { - jest.spyOn(commonUtils, 'insertText').mockImplementation((el, textFunc) => { - const insertedText = textFunc('This is code: `', '`'); - - expect(insertedText).toEqual('code'); - }); + target.value = 'This is code: `'; callPasteGFM(); + + expect(target.value).toBe('This is code: `code'); + }); + + it('does not allow xss in x-gfm-html', () => { + const testEl = document.createElement('div'); + jest.spyOn(document, 'createElement').mockReturnValueOnce(testEl); + + callPasteGFM({ 'text/plain': 'code', 'text/x-gfm-html': 'code' }); + + expect(testEl.innerHTML).toBe('code'); }); }); diff --git a/spec/frontend/lib/utils/url_utility_spec.js b/spec/frontend/lib/utils/url_utility_spec.js index e12cd8b0e3..3b2852d08e 100644 --- a/spec/frontend/lib/utils/url_utility_spec.js +++ b/spec/frontend/lib/utils/url_utility_spec.js @@ -1,3 +1,4 @@ +import { TEST_HOST } from 'helpers/test_constants'; import * as urlUtils from '~/lib/utils/url_utility'; const shas = { @@ -921,4 +922,37 @@ describe('URL utility', () => { expect(urlUtils.encodeSaferUrl(input)).toBe(input); }); }); + + describe('isSameOriginUrl', () => { + // eslint-disable-next-line no-script-url + const javascriptUrl = 'javascript:alert(1)'; + + beforeEach(() => { + setWindowLocation({ origin: TEST_HOST }); + }); + + it.each` + url | expected + ${TEST_HOST} | ${true} + ${`${TEST_HOST}/a/path`} | ${true} + ${'//test.host/no-protocol'} | ${true} + ${'/a/root/relative/path'} | ${true} + ${'a/relative/path'} | ${true} + ${'#hash'} | ${true} + ${'?param=foo'} | ${true} + ${''} | ${true} + ${'../../../'} | ${true} + ${`${TEST_HOST}:8080/wrong-port`} | ${false} + ${'ws://test.host/wrong-protocol'} | ${false} + ${'http://phishing.test'} | ${false} + ${'//phishing.test'} | ${false} + ${'//invalid:url'} | ${false} + ${javascriptUrl} | ${false} + ${'data:,Hello%2C%20World%21'} | ${false} + ${null} | ${false} + ${undefined} | ${false} + `('returns $expected given $url', ({ url, expected }) => { + expect(urlUtils.isSameOriginUrl(url)).toBe(expected); + }); + }); }); diff --git a/spec/frontend/releases/components/app_edit_new_spec.js b/spec/frontend/releases/components/app_edit_new_spec.js index 65ed6d6166..748b48daca 100644 --- a/spec/frontend/releases/components/app_edit_new_spec.js +++ b/spec/frontend/releases/components/app_edit_new_spec.js @@ -4,6 +4,7 @@ import MockAdapter from 'axios-mock-adapter'; import { merge } from 'lodash'; import Vuex from 'vuex'; import { getJSONFixture } from 'helpers/fixtures'; +import { TEST_HOST } from 'helpers/test_constants'; import * as commonUtils from '~/lib/utils/common_utils'; import ReleaseEditNewApp from '~/releases/components/app_edit_new.vue'; import AssetLinksForm from '~/releases/components/asset_links_form.vue'; @@ -11,6 +12,7 @@ import { BACK_URL_PARAM } from '~/releases/constants'; const originalRelease = getJSONFixture('api/releases/release.json'); const originalMilestones = originalRelease.milestones; +const releasesPagePath = 'path/to/releases/page'; describe('Release edit/new component', () => { let wrapper; @@ -24,7 +26,7 @@ describe('Release edit/new component', () => { state = { release, markdownDocsPath: 'path/to/markdown/docs', - releasesPagePath: 'path/to/releases/page', + releasesPagePath, projectId: '8', groupId: '42', groupMilestonesAvailable: true, @@ -75,6 +77,8 @@ describe('Release edit/new component', () => { }; beforeEach(() => { + global.jsdom.reconfigure({ url: TEST_HOST }); + mock = new MockAdapter(axios); gon.api_version = 'v4'; @@ -146,22 +150,33 @@ describe('Release edit/new component', () => { }); }); - describe(`when the URL contains a "${BACK_URL_PARAM}" parameter`, () => { - const backUrl = 'https://example.gitlab.com/back/url'; + // eslint-disable-next-line no-script-url + const xssBackUrl = 'javascript:alert(1)'; + describe.each` + backUrl | expectedHref + ${`${TEST_HOST}/back/url`} | ${`${TEST_HOST}/back/url`} + ${`/back/url?page=2`} | ${`/back/url?page=2`} + ${`back/url?page=3`} | ${`back/url?page=3`} + ${'http://phishing.test/back/url'} | ${releasesPagePath} + ${'//phishing.test/back/url'} | ${releasesPagePath} + ${xssBackUrl} | ${releasesPagePath} + `( + `when the URL contains a "${BACK_URL_PARAM}=$backUrl" parameter`, + ({ backUrl, expectedHref }) => { + beforeEach(async () => { + global.jsdom.reconfigure({ + url: `${TEST_HOST}?${BACK_URL_PARAM}=${encodeURIComponent(backUrl)}`, + }); - beforeEach(async () => { - commonUtils.getParameterByName = jest - .fn() - .mockImplementation((paramToGet) => ({ [BACK_URL_PARAM]: backUrl }[paramToGet])); + await factory(); + }); - await factory(); - }); - - it('renders a "Cancel" button with an href pointing to the main Releases page', () => { - const cancelButton = wrapper.find('.js-cancel-button'); - expect(cancelButton.attributes().href).toBe(backUrl); - }); - }); + it(`renders a "Cancel" button with an href pointing to ${expectedHref}`, () => { + const cancelButton = wrapper.find('.js-cancel-button'); + expect(cancelButton.attributes().href).toBe(expectedHref); + }); + }, + ); describe('when creating a new release', () => { beforeEach(async () => { diff --git a/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb b/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb index 03a9b9bd21..d401c42fed 100644 --- a/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb +++ b/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb @@ -23,7 +23,7 @@ RSpec.describe Gitlab::Diff::FileCollection::MergeRequestDiff do it 'does not highlight files marked as undiffable in .gitattributes' do allow_next_instance_of(Gitlab::Diff::File) do |instance| - allow(instance).to receive(:diffable?).and_return(false) + allow(instance).to receive(:diffable_by_attribute?).and_return(false) end expect_next_instance_of(Gitlab::Diff::File) do |instance| diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index 78be89c449..1800d2d6b6 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -186,26 +186,46 @@ RSpec.describe Gitlab::Diff::File do end describe '#diffable?' do - let(:commit) { project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') } - let(:diffs) { commit.diffs } + context 'when attributes exist' do + let(:commit) { project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') } + let(:diffs) { commit.diffs } - before do - info_dir_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - File.join(project.repository.path_to_repo, 'info') + before do + info_dir_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(project.repository.path_to_repo, 'info') + end + + FileUtils.mkdir(info_dir_path) unless File.exist?(info_dir_path) + File.write(File.join(info_dir_path, 'attributes'), "*.md -diff\n") end - FileUtils.mkdir(info_dir_path) unless File.exist?(info_dir_path) - File.write(File.join(info_dir_path, 'attributes'), "*.md -diff\n") + it "returns true for files that do not have attributes" do + diff_file = diffs.diff_file_with_new_path('LICENSE') + expect(diff_file.diffable?).to be_truthy + end + + it "returns false for files that have been marked as not being diffable in attributes" do + diff_file = diffs.diff_file_with_new_path('README.md') + expect(diff_file.diffable?).to be_falsey + end end - it "returns true for files that do not have attributes" do - diff_file = diffs.diff_file_with_new_path('LICENSE') - expect(diff_file.diffable?).to be_truthy + context 'when the text has binary notice' do + let(:commit) { project.commit('f05a98786e4274708e1fa118c7ad3a29d1d1b9a3') } + let(:diff_file) { commit.diffs.diff_file_with_new_path('VERSION') } + + it "returns false" do + expect(diff_file.diffable?).to be_falsey + end end - it "returns false for files that have been marked as not being diffable in attributes" do - diff_file = diffs.diff_file_with_new_path('README.md') - expect(diff_file.diffable?).to be_falsey + context 'when the content is binary' do + let(:commit) { project.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } + let(:diff_file) { commit.diffs.diff_file_with_new_path('files/images/6049019_460s.jpg') } + + it "returns true" do + expect(diff_file.diffable?).to be_truthy + end end end @@ -729,6 +749,18 @@ RSpec.describe Gitlab::Diff::File do end end + context 'when the the encoding of the file is unsupported' do + let(:commit) { project.commit('f05a98786e4274708e1fa118c7ad3a29d1d1b9a3') } + let(:diff_file) { commit.diffs.diff_file_with_new_path('VERSION') } + + it 'returns a Not Diffable viewer' do + expect(diff_file.simple_viewer).to be_a(DiffViewer::NotDiffable) + end + + it { expect(diff_file.highlighted_diff_lines).to eq([]) } + it { expect(diff_file.parallel_diff_lines).to eq([]) } + end + describe '#diff_hunk' do context 'when first line is a match' do let(:raw_diff) do diff --git a/spec/lib/gitlab/diff/parser_spec.rb b/spec/lib/gitlab/diff/parser_spec.rb index 7448ae0b2e..c8069f82f0 100644 --- a/spec/lib/gitlab/diff/parser_spec.rb +++ b/spec/lib/gitlab/diff/parser_spec.rb @@ -146,6 +146,16 @@ eos it { expect(parser.parse(nil)).to eq([]) } end + context 'when it is a binary notice' do + let(:diff) do + <<~END + Binary files a/test and b/test differ + END + end + + it { expect(parser.parse(diff.each_line)).to eq([]) } + end + describe 'tolerates special diff markers in a content' do it "counts lines correctly" do diff = <<~END diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 3d6c04fd48..dce0c21006 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -440,6 +440,14 @@ RSpec.describe Gitlab::GitAccess do expect { pull_access_check }.to raise_forbidden("Your password expired. Please access GitLab from a web browser to update your password.") end + it 'allows ldap users with expired password to pull' do + project.add_maintainer(user) + user.update!(password_expires_at: 2.minutes.ago) + allow(user).to receive(:ldap_user?).and_return(true) + + expect { pull_access_check }.not_to raise_error + end + context 'when the project repository does not exist' do before do project.add_guest(user) @@ -977,12 +985,26 @@ RSpec.describe Gitlab::GitAccess do end it 'disallows users with expired password to push' do - project.add_maintainer(user) user.update!(password_expires_at: 2.minutes.ago) expect { push_access_check }.to raise_forbidden("Your password expired. Please access GitLab from a web browser to update your password.") end + it 'allows ldap users with expired password to push' do + user.update!(password_expires_at: 2.minutes.ago) + allow(user).to receive(:ldap_user?).and_return(true) + + expect { push_access_check }.not_to raise_error + end + + it 'disallows blocked ldap users with expired password to push' do + user.block + user.update!(password_expires_at: 2.minutes.ago) + allow(user).to receive(:ldap_user?).and_return(true) + + expect { push_access_check }.to raise_forbidden("Your account has been blocked.") + end + it 'cleans up the files' do expect(project.repository).to receive(:clean_stale_repository_files).and_call_original expect { push_access_check }.not_to raise_error diff --git a/spec/lib/gitlab/http_spec.rb b/spec/lib/gitlab/http_spec.rb index 308f7f4625..71e80de9f8 100644 --- a/spec/lib/gitlab/http_spec.rb +++ b/spec/lib/gitlab/http_spec.rb @@ -27,6 +27,47 @@ RSpec.describe Gitlab::HTTP do end end + context 'when reading the response is too slow' do + before do + stub_const("#{described_class}::DEFAULT_READ_TOTAL_TIMEOUT", 0.001.seconds) + + WebMock.stub_request(:post, /.*/).to_return do |request| + sleep 0.002.seconds + { body: 'I\m slow', status: 200 } + end + end + + let(:options) { {} } + + subject(:request_slow_responder) { described_class.post('http://example.org', **options) } + + specify do + expect { request_slow_responder }.not_to raise_error + end + + context 'with use_read_total_timeout option' do + let(:options) { { use_read_total_timeout: true } } + + it 'raises a timeout error' do + expect { request_slow_responder }.to raise_error(Gitlab::HTTP::ReadTotalTimeout, /Request timed out after ?([0-9]*[.])?[0-9]+ seconds/) + end + + context 'and timeout option' do + let(:options) { { use_read_total_timeout: true, timeout: 10.seconds } } + + it 'overrides the default timeout when timeout option is present' do + expect { request_slow_responder }.not_to raise_error + end + end + end + end + + it 'calls a block' do + WebMock.stub_request(:post, /.*/) + + expect { |b| described_class.post('http://example.org', &b) }.to yield_with_args + end + describe 'allow_local_requests_from_web_hooks_and_services is' do before do WebMock.stub_request(:get, /.*/).to_return(status: 200, body: 'Success') diff --git a/spec/models/audit_event_spec.rb b/spec/models/audit_event_spec.rb index 5c87c2e68d..bc603bc5ab 100644 --- a/spec/models/audit_event_spec.rb +++ b/spec/models/audit_event_spec.rb @@ -3,9 +3,6 @@ require 'spec_helper' RSpec.describe AuditEvent do - let_it_be(:audit_event) { create(:project_audit_event) } - subject { audit_event } - describe 'validations' do include_examples 'validates IP address' do let(:attribute) { :ip_address } @@ -13,6 +10,15 @@ RSpec.describe AuditEvent do end end + it 'sanitizes custom_message in the details hash' do + audit_event = create(:project_audit_event, details: { target_id: 678, custom_message: 'Arnold' }) + + expect(audit_event.details).to include( + target_id: 678, + custom_message: 'Arnold' + ) + end + describe '#as_json' do context 'ip_address' do subject { build(:group_audit_event, ip_address: '192.168.1.1').as_json } diff --git a/spec/models/protected_branch/push_access_level_spec.rb b/spec/models/protected_branch/push_access_level_spec.rb index 17a589f048..fa84cd660c 100644 --- a/spec/models/protected_branch/push_access_level_spec.rb +++ b/spec/models/protected_branch/push_access_level_spec.rb @@ -44,7 +44,7 @@ RSpec.describe ProtectedBranch::PushAccessLevel do let(:can_push) { true } before_all do - project.add_guest(user) + project.add_maintainer(user) end context 'when this push_access_level is tied to a deploy key' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index cb34917f07..bce3083ab9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -376,6 +376,19 @@ RSpec.describe User do expect(user.errors.full_messages).to eq(['Username has already been taken']) end end + + it 'validates format' do + Mime::EXTENSION_LOOKUP.keys.each do |type| + user = build(:user, username: "test.#{type}") + + expect(user).not_to be_valid + expect(user.errors.full_messages).to include('Username ending with MIME type format is not allowed.') + end + end + + it 'validates format on updated record' do + expect(create(:user).update(username: 'profile.html')).to be_falsey + end end it 'has a DB-level NOT NULL constraint on projects_limit' do @@ -2877,7 +2890,7 @@ RSpec.describe User do end describe '#sanitize_attrs' do - let(:user) { build(:user, name: 'test & user', skype: 'test&user') } + let(:user) { build(:user, name: 'test <& user', skype: 'test&user') } it 'encodes HTML entities in the Skype attribute' do expect { user.sanitize_attrs }.to change { user.skype }.to('test&user') @@ -2886,6 +2899,25 @@ RSpec.describe User do it 'does not encode HTML entities in the name attribute' do expect { user.sanitize_attrs }.not_to change { user.name } end + + it 'sanitizes attr from html tags' do + user = create(:user, name: 'Test', twitter: 'https://twitter.com') + + expect(user.name).to eq('Test') + expect(user.twitter).to eq('https://twitter.com') + end + + it 'sanitizes attr from js scripts' do + user = create(:user, name: '') + + expect(user.name).to eq("alert(\"Test\")") + end + + it 'sanitizes attr from iframe scripts' do + user = create(:user, name: 'User">