diff options
47 files changed, 636 insertions, 175 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 1af05b60ddc..65f1a980d6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,30 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 13.0.8 (2020-07-01) + +### Security (18 changes) + +- Update xterm js dependency to latest stable 3.x version. +- Do not show activity for users with private profiles. +- Fix stored XSS in markdown renderer. +- Upgrade swagger-ui to solve XSS issues. +- Fix group deploy token API authorizations. +- Check access when sending TODOs related to merge requests. +- Change from hybrid to JSON cookies serializer. +- Prevent XSS in group name validations. +- Disable caching for wiki attachments. +- Disable Github Importer API by settings. +- Fix null byte error in upload path. +- Update permissions for time tracking endpoints. +- Add snippet repository validation after bundle import. +- Update Kaminari gem. +- Fix note author name rendering. +- Sanitize bitbucket repo urls to mitigate XSS. +- Stored XSS on the Error Tracking page. +- Fix security issue when rendering issuable. + + ## 13.0.7 (2020-06-25) ### Fixed (7 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 4e8f3240ec8..8e7aa5e40f2 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -13.0.7 +13.0.8 diff --git a/Gemfile.lock b/Gemfile.lock index ffff576e8b0..271ffc216ba 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -559,18 +559,18 @@ GEM json-schema (2.8.0) addressable (>= 2.4) jwt (2.1.0) - kaminari (1.0.1) + kaminari (1.2.1) activesupport (>= 4.1.0) - kaminari-actionview (= 1.0.1) - kaminari-activerecord (= 1.0.1) - kaminari-core (= 1.0.1) - kaminari-actionview (1.0.1) + kaminari-actionview (= 1.2.1) + kaminari-activerecord (= 1.2.1) + kaminari-core (= 1.2.1) + kaminari-actionview (1.2.1) actionview - kaminari-core (= 1.0.1) - kaminari-activerecord (1.0.1) + kaminari-core (= 1.2.1) + kaminari-activerecord (1.2.1) activerecord - kaminari-core (= 1.0.1) - kaminari-core (1.0.1) + kaminari-core (= 1.2.1) + kaminari-core (1.2.1) kgio (2.11.3) knapsack (1.17.0) rake @@ -1 +1 @@ -13.0.7 +13.0.8 diff --git a/app/assets/javascripts/error_tracking/components/stacktrace_entry.vue b/app/assets/javascripts/error_tracking/components/stacktrace_entry.vue index f7f2c450be1..d806c6934a3 100644 --- a/app/assets/javascripts/error_tracking/components/stacktrace_entry.vue +++ b/app/assets/javascripts/error_tracking/components/stacktrace_entry.vue @@ -1,7 +1,5 @@ <script> -import { escape } from 'lodash'; -import { GlTooltip } from '@gitlab/ui'; -import { __, sprintf } from '~/locale'; +import { GlTooltip, GlSprintf } from '@gitlab/ui'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import FileIcon from '~/vue_shared/components/file_icon.vue'; import Icon from '~/vue_shared/components/icon.vue'; @@ -11,6 +9,7 @@ export default { ClipboardButton, FileIcon, Icon, + GlSprintf, }, directives: { GlTooltip, @@ -57,36 +56,6 @@ export default { collapseIcon() { return this.isExpanded ? 'chevron-down' : 'chevron-right'; }, - errorFnText() { - return this.errorFn - ? sprintf( - __(`%{spanStart}in%{spanEnd} %{errorFn}`), - { - errorFn: `<strong>${escape(this.errorFn)}</strong>`, - spanStart: `<span class="text-tertiary">`, - spanEnd: `</span>`, - }, - false, - ) - : ''; - }, - errorPositionText() { - return this.errorLine - ? sprintf( - __(`%{spanStart}at line%{spanEnd} %{errorLine}%{errorColumn}`), - { - errorLine: `<strong>${this.errorLine}</strong>`, - errorColumn: this.errorColumn ? `:<strong>${this.errorColumn}</strong>` : ``, - spanStart: `<span class="text-tertiary">`, - spanEnd: `</span>`, - }, - false, - ) - : ''; - }, - errorInfo() { - return `${this.errorFnText} ${this.errorPositionText}`; - }, }, methods: { isHighlighted(lineNum) { @@ -132,7 +101,27 @@ export default { :text="filePath" css-class="btn-default btn-transparent btn-clipboard position-static" /> - <span v-html="errorInfo"></span> + + <gl-sprintf v-if="errorFn" :message="__('%{spanStart}in%{spanEnd} %{errorFn}')"> + <template #span="{content}"> + <span class="gl-text-gray-400">{{ content }} </span> + </template> + <template #errorFn> + <strong>{{ errorFn }} </strong> + </template> + </gl-sprintf> + + <gl-sprintf :message="__('%{spanStart}at line%{spanEnd} %{errorLine}%{errorColumn}')"> + <template #span="{content}"> + <span class="gl-text-gray-400">{{ content }} </span> + </template> + <template #errorLine> + <strong>{{ errorLine }}</strong> + </template> + <template #errorColumn> + <strong v-if="errorColumn">:{{ errorColumn }}</strong> + </template> + </gl-sprintf> </div> </div> diff --git a/app/assets/javascripts/issuables_list/components/issuable.vue b/app/assets/javascripts/issuables_list/components/issuable.vue index 2fd92e009eb..947c7518289 100644 --- a/app/assets/javascripts/issuables_list/components/issuable.vue +++ b/app/assets/javascripts/issuables_list/components/issuable.vue @@ -4,7 +4,7 @@ * any changes done to the haml need to be reflected here. */ import { escape, isNumber } from 'lodash'; -import { GlLink, GlTooltipDirective as GlTooltip } from '@gitlab/ui'; +import { GlLink, GlTooltipDirective as GlTooltip, GlSprintf } from '@gitlab/ui'; import { dateInWords, formatDate, @@ -20,10 +20,14 @@ import Icon from '~/vue_shared/components/icon.vue'; import IssueAssignees from '~/vue_shared/components/issue/issue_assignees.vue'; export default { + i18n: { + openedAgo: __('opened %{timeAgoString} by %{user}'), + }, components: { Icon, IssueAssignees, GlLink, + GlSprintf, }, directives: { GlTooltip, @@ -98,23 +102,21 @@ export default { } return __('Milestone'); }, - openedAgoByString() { - const { author, created_at } = this.issuable; + issuableAuthor() { + return this.issuable.author; + }, + issuableCreatedAt() { + return getTimeago().format(this.issuable.created_at); + }, + popoverDataAttrs() { + const { id, username, name, avatar_url } = this.issuableAuthor; - return sprintf( - __('opened %{timeAgoString} by %{user}'), - { - timeAgoString: escape(getTimeago().format(created_at)), - user: `<a href="${escape(author.web_url)}" - data-user-id=${escape(author.id)} - data-username=${escape(author.username)} - data-name=${escape(author.name)} - data-avatar-url="${escape(author.avatar_url)}"> - ${escape(author.name)} - </a>`, - }, - false, - ); + return { + 'data-user-id': id, + 'data-username': username, + 'data-name': name, + 'data-avatar-url': avatar_url, + }; }, referencePath() { return this.issuable.references.relative; @@ -160,7 +162,7 @@ export default { mounted() { // TODO: Refactor user popover to use its own component instead of // spawning event listeners on Vue-rendered elements. - initUserPopovers([this.$refs.openedAgoByContainer.querySelector('a')]); + initUserPopovers([this.$refs.openedAgoByContainer.$el]); }, methods: { labelStyle(label) { @@ -221,17 +223,30 @@ export default { ></i> <gl-link :href="issuable.web_url">{{ issuable.title }}</gl-link> </span> - <span v-if="issuable.has_tasks" class="ml-1 task-status d-none d-sm-inline-block">{{ - issuable.task_status - }}</span> + <span v-if="issuable.has_tasks" class="ml-1 task-status d-none d-sm-inline-block"> + {{ issuable.task_status }} + </span> </div> <div class="issuable-info"> <span class="js-ref-path">{{ referencePath }}</span> - <span class="d-none d-sm-inline-block mr-1"> + <span data-testid="openedByMessage" class="d-none d-sm-inline-block mr-1"> · - <span ref="openedAgoByContainer" v-html="openedAgoByString"></span> + <gl-sprintf :message="$options.i18n.openedAgo"> + <template #timeAgoString> + <span>{{ issuableCreatedAt }}</span> + </template> + <template #user> + <gl-link + ref="openedAgoByContainer" + v-bind="popoverDataAttrs" + :href="issuableAuthor.web_url" + > + {{ issuableAuthor.name }} + </gl-link> + </template> + </gl-sprintf> </span> <gl-link diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index 0760bdf1e01..84c8d7ada43 100644 --- a/app/controllers/groups/application_controller.rb +++ b/app/controllers/groups/application_controller.rb @@ -34,6 +34,18 @@ class Groups::ApplicationController < ApplicationController end end + def authorize_create_deploy_token! + unless can?(current_user, :create_deploy_token, group) + return render_404 + end + end + + def authorize_destroy_deploy_token! + unless can?(current_user, :destroy_deploy_token, group) + return render_404 + end + end + def authorize_admin_group_member! unless can?(current_user, :admin_group_member, group) return render_403 diff --git a/app/controllers/groups/deploy_tokens_controller.rb b/app/controllers/groups/deploy_tokens_controller.rb index 6bb075fd115..de951f2cb9f 100644 --- a/app/controllers/groups/deploy_tokens_controller.rb +++ b/app/controllers/groups/deploy_tokens_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Groups::DeployTokensController < Groups::ApplicationController - before_action :authorize_admin_group! + before_action :authorize_destroy_deploy_token! def revoke @token = @group.deploy_tokens.find(params[:id]) diff --git a/app/controllers/groups/settings/repository_controller.rb b/app/controllers/groups/settings/repository_controller.rb index 4af5e613296..e2fbdc39692 100644 --- a/app/controllers/groups/settings/repository_controller.rb +++ b/app/controllers/groups/settings/repository_controller.rb @@ -4,7 +4,7 @@ module Groups module Settings class RepositoryController < Groups::ApplicationController skip_cross_project_access_check :show - before_action :authorize_admin_group! + before_action :authorize_create_deploy_token! before_action :define_deploy_token_variables before_action do push_frontend_feature_flag(:ajax_new_deploy_token, @group) diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index 508b1f5bd0a..f6bde33e4cb 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -46,7 +46,7 @@ class Projects::WikisController < Projects::ApplicationController render 'show' elsif file_blob - send_blob(@project_wiki.repository, file_blob, allow_caching: @project.public?) + send_blob(@project_wiki.repository, file_blob) elsif show_create_form? # Assign a title to the WikiPage unless `id` is a randomly generated slug from #new title = params[:id] unless params[:random_title].present? diff --git a/app/finders/events_finder.rb b/app/finders/events_finder.rb index 9c56451fd44..8b4035ef9e9 100644 --- a/app/finders/events_finder.rb +++ b/app/finders/events_finder.rb @@ -33,6 +33,8 @@ class EventsFinder end def execute + return Event.none if cannot_access_private_profile? + events = get_events events = by_current_user_access(events) @@ -102,6 +104,10 @@ class EventsFinder end # rubocop: enable CodeReuse/ActiveRecord + def cannot_access_private_profile? + source.is_a?(User) && !Ability.allowed?(current_user, :read_user_profile, source) + end + def sort(events) return events unless params[:sort] diff --git a/app/models/group.rb b/app/models/group.rb index 5e835319b1e..dd8bdf988f0 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -73,9 +73,12 @@ class Group < Namespace validates :variables, variable_duplicates: true validates :two_factor_grace_period, presence: true, numericality: { greater_than_or_equal_to: 0 } + validates :name, - format: { with: Gitlab::Regex.group_name_regex, - message: Gitlab::Regex.group_name_regex_message }, if: :name_changed? + html_safety: true, + format: { with: Gitlab::Regex.group_name_regex, + message: Gitlab::Regex.group_name_regex_message }, + if: :name_changed? add_authentication_token_field :runners_token, encrypted: -> { Feature.enabled?(:groups_tokens_optional_encryption, default_enabled: true) ? :optional : :required } diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b4d0b729454..29f5626cd59 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -515,7 +515,7 @@ class MergeRequest < ApplicationRecord participants << merge_user end - participants + participants.select { |participant| Ability.allowed?(participant, :read_merge_request, self) } end def first_commit diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 136ac4cce63..b1b52d62b85 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -98,9 +98,7 @@ class GroupPolicy < BasePolicy enable :create_cluster enable :update_cluster enable :admin_cluster - enable :destroy_deploy_token enable :read_deploy_token - enable :create_deploy_token end rule { owner }.policy do @@ -112,6 +110,8 @@ class GroupPolicy < BasePolicy enable :set_note_created_at enable :set_emails_disabled enable :update_default_branch_protection + enable :create_deploy_token + enable :destroy_deploy_token end rule { can?(:read_nested_project_resources) }.policy do diff --git a/app/services/snippets/repository_validation_service.rb b/app/services/snippets/repository_validation_service.rb new file mode 100644 index 00000000000..c8197795383 --- /dev/null +++ b/app/services/snippets/repository_validation_service.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +module Snippets + class RepositoryValidationService + attr_reader :current_user, :snippet, :repository + + RepositoryValidationError = Class.new(StandardError) + + def initialize(user, snippet) + @current_user = user + @snippet = snippet + @repository = snippet.repository + end + + def execute + if snippet.nil? + return service_response_error('No snippet found.', 404) + end + + check_branch_count! + check_branch_name_default! + check_tag_count! + check_file_count! + check_size! + + ServiceResponse.success(message: 'Valid snippet repository.') + rescue RepositoryValidationError => e + ServiceResponse.error(message: "Error: #{e.message}", http_status: 400) + end + + private + + def check_branch_count! + return if repository.branch_count == 1 + + raise RepositoryValidationError, _('Repository has more than one branch.') + end + + def check_branch_name_default! + branches = repository.branch_names + + return if branches.first == Gitlab::Checks::SnippetCheck::DEFAULT_BRANCH + + raise RepositoryValidationError, _('Repository has an invalid default branch name.') + end + + def check_tag_count! + return if repository.tag_count == 0 + + raise RepositoryValidationError, _('Repository has tags.') + end + + def check_file_count! + file_count = repository.ls_files(nil).size + limit = Snippet.max_file_limit(current_user) + + if file_count > limit + raise RepositoryValidationError, _('Repository files count over the limit') + end + + if file_count == 0 + raise RepositoryValidationError, _('Repository must contain at least 1 file.') + end + end + + def check_size! + return unless snippet.repository_size_checker.above_size_limit? + + raise RepositoryValidationError, _('Repository size is above the limit.') + end + end +end diff --git a/app/validators/html_safety_validator.rb b/app/validators/html_safety_validator.rb new file mode 100644 index 00000000000..29e7d445697 --- /dev/null +++ b/app/validators/html_safety_validator.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +# HtmlSafetyValidator +# +# Validates that a value does not contain HTML +# or other unsafe content that could lead to XSS. +# Relies on Rails HTML Sanitizer: +# https://github.com/rails/rails-html-sanitizer +# +# Example: +# +# class Group < ActiveRecord::Base +# validates :name, presence: true, html_safety: true +# end +# +class HtmlSafetyValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + return if value.blank? || safe_value?(value) + + record.errors.add(attribute, self.class.error_message) + end + + def self.error_message + _("cannot contain HTML/XML tags, including any word between angle brackets (<,>).") + end + + private + + # The `FullSanitizer` encodes ampersands as the HTML entity name. + # This isn't particularly necessary for preventing XSS so the ampersand + # is pre-encoded to avoid it being flagged in the comparison. + def safe_value?(text) + pre_encoded_text = text.gsub('&', '&') + Rails::Html::FullSanitizer.new.sanitize(pre_encoded_text) == pre_encoded_text + end +end diff --git a/app/views/import/bitbucket_server/status.html.haml b/app/views/import/bitbucket_server/status.html.haml index 1aaf5883bf4..b4eda80201c 100644 --- a/app/views/import/bitbucket_server/status.html.haml +++ b/app/views/import/bitbucket_server/status.html.haml @@ -54,7 +54,7 @@ - @repos.each do |repo| %tr{ id: "repo_#{repo.project_key}___#{repo.slug}", data: { project: repo.project_key, repository: repo.slug } } %td - = link_to repo.browse_url, repo.browse_url, target: '_blank', rel: 'noopener noreferrer' + = sanitize(link_to(repo.browse_url, repo.browse_url, target: '_blank', rel: 'noopener noreferrer'), attributes: %w(href target rel)) %td.import-target %fieldset.row .input-group @@ -75,7 +75,7 @@ - @incompatible_repos.each do |repo| %tr{ id: "repo_#{repo.project_key}___#{repo.slug}" } %td - = link_to repo.browse_url, repo.browse_url, target: '_blank', rel: 'noopener noreferrer' + = sanitize(link_to(repo.browse_url, repo.browse_url, target: '_blank', rel: 'noopener noreferrer'), attributes: %w(href target rel)) %td.import-target %td.import-actions-job-status = label_tag 'Incompatible Project', nil, class: 'label badge-danger' diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index df09c4338a1..e6c8e13c5c1 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -32,7 +32,7 @@ .note-header-info %a{ href: user_path(note.author) } %span.note-header-author-name.bold - = sanitize(note.author.name) + = note.author.name = user_status(note.author) %span.note-headline-light = note.author.to_reference diff --git a/config/initializers/cookies_serializer.rb b/config/initializers/cookies_serializer.rb index fa1736dfea6..e9a71f32581 100644 --- a/config/initializers/cookies_serializer.rb +++ b/config/initializers/cookies_serializer.rb @@ -1,4 +1,5 @@ # Be sure to restart your server when you modify this file. Rails.application.config.action_dispatch.use_cookies_with_metadata = true -Rails.application.config.action_dispatch.cookies_serializer = :hybrid +Rails.application.config.action_dispatch.cookies_serializer = + Gitlab::Utils.to_boolean(ENV['USE_UNSAFE_HYBRID_COOKIES']) ? :hybrid : :json diff --git a/doc/api/deploy_tokens.md b/doc/api/deploy_tokens.md index 6e732a43da0..69dd93e113b 100644 --- a/doc/api/deploy_tokens.md +++ b/doc/api/deploy_tokens.md @@ -136,7 +136,8 @@ curl --request DELETE --header "PRIVATE-TOKEN: <your_access_token>" "https://git ## Group deploy tokens -These endpoints require group maintainer access or higher. +Group maintainers and owners can list group deploy +tokens. Only group owners can create and delete group deploy tokens. ### List group deploy tokens diff --git a/doc/user/permissions.md b/doc/user/permissions.md index e5893b291dc..ac17d564e6e 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -248,6 +248,8 @@ group. | Edit epic comments (posted by any user) **(ULTIMATE)** | | | | ✓ (2) | ✓ (2) | | Edit group | | | | | ✓ | | Manage group level CI/CD variables | | | | | ✓ | +| List group deploy tokens | | | | ✓ | ✓ | +| Create/Delete group deploy tokens | | | | | ✓ | | Manage group members | | | | | ✓ | | Remove group | | | | | ✓ | | Delete group epic **(ULTIMATE)** | | | | | ✓ | diff --git a/lib/api/import_github.rb b/lib/api/import_github.rb index 21d4928193e..f31cc15dc62 100644 --- a/lib/api/import_github.rb +++ b/lib/api/import_github.rb @@ -4,6 +4,10 @@ module API class ImportGithub < Grape::API rescue_from Octokit::Unauthorized, with: :provider_unauthorized + before do + forbidden! unless Gitlab::CurrentSettings.import_sources&.include?('github') + end + helpers do def client @client ||= Gitlab::LegacyGithubImport::Client.new(params[:personal_access_token], client_options) diff --git a/lib/api/time_tracking_endpoints.rb b/lib/api/time_tracking_endpoints.rb index 93fe06bec27..da234fb5277 100644 --- a/lib/api/time_tracking_endpoints.rb +++ b/lib/api/time_tracking_endpoints.rb @@ -14,8 +14,8 @@ module API "#{issuable_name}_iid".to_sym end - def update_issuable_key - "update_#{issuable_name}".to_sym + def admin_issuable_key + "admin_#{issuable_name}".to_sym end def read_issuable_key @@ -60,7 +60,7 @@ module API requires :duration, type: String, desc: 'The duration to be parsed' end post ":id/#{issuable_collection_name}/:#{issuable_key}/time_estimate" do - authorize! update_issuable_key, load_issuable + authorize! admin_issuable_key, load_issuable status :ok update_issuable(time_estimate: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration))) @@ -71,7 +71,7 @@ module API requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}" end post ":id/#{issuable_collection_name}/:#{issuable_key}/reset_time_estimate" do - authorize! update_issuable_key, load_issuable + authorize! admin_issuable_key, load_issuable status :ok update_issuable(time_estimate: 0) @@ -83,7 +83,7 @@ module API requires :duration, type: String, desc: 'The duration to be parsed' end post ":id/#{issuable_collection_name}/:#{issuable_key}/add_spent_time" do - authorize! update_issuable_key, load_issuable + authorize! admin_issuable_key, load_issuable update_issuable(spend_time: { duration: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration)), @@ -96,7 +96,7 @@ module API requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}" end post ":id/#{issuable_collection_name}/:#{issuable_key}/reset_spent_time" do - authorize! update_issuable_key, load_issuable + authorize! admin_issuable_key, load_issuable status :ok update_issuable(spend_time: { duration: :reset, user_id: current_user.id }) diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index 5962403d488..f142333d797 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -253,7 +253,7 @@ module Banzai object_parent_type = parent.is_a?(Group) ? :group : :project { - original: text, + original: escape_html_entities(text), link: link_content, link_reference: link_reference, object_parent_type => parent.id, diff --git a/lib/banzai/filter/base_relative_link_filter.rb b/lib/banzai/filter/base_relative_link_filter.rb index eca105ce9d9..fd526df4c48 100644 --- a/lib/banzai/filter/base_relative_link_filter.rb +++ b/lib/banzai/filter/base_relative_link_filter.rb @@ -38,7 +38,7 @@ module Banzai private def unescape_and_scrub_uri(uri) - Addressable::URI.unescape(uri).scrub + Addressable::URI.unescape(uri).scrub.delete("\0") end end end diff --git a/lib/gitlab/import_export/snippet_repo_restorer.rb b/lib/gitlab/import_export/snippet_repo_restorer.rb index b58ea14a3a8..7392d88f8f5 100644 --- a/lib/gitlab/import_export/snippet_repo_restorer.rb +++ b/lib/gitlab/import_export/snippet_repo_restorer.rb @@ -3,7 +3,9 @@ module Gitlab module ImportExport class SnippetRepoRestorer < RepoRestorer - attr_reader :snippet + attr_reader :snippet, :user + + SnippetRepositoryError = Class.new(StandardError) def initialize(snippet:, user:, shared:, path_to_bundle:) @snippet = snippet @@ -31,6 +33,16 @@ module Gitlab def create_repository_from_bundle repository.create_from_bundle(path_to_bundle) snippet.track_snippet_repository(repository.storage) + + response = Snippets::RepositoryValidationService.new(user, snippet).execute + + if response.error? + repository.remove + snippet.snippet_repository.delete + snippet.repository.expire_exists_cache + + raise SnippetRepositoryError, _("Invalid repository bundle for snippet with id %{snippet_id}") % { snippet_id: snippet.id } + end end def create_repository_from_db diff --git a/lib/gitlab/markdown_cache.rb b/lib/gitlab/markdown_cache.rb index 489fc6fddac..21797bf988d 100644 --- a/lib/gitlab/markdown_cache.rb +++ b/lib/gitlab/markdown_cache.rb @@ -3,7 +3,7 @@ module Gitlab module MarkdownCache # Increment this number every time the renderer changes its output - CACHE_COMMONMARK_VERSION = 21 + CACHE_COMMONMARK_VERSION = 23 CACHE_COMMONMARK_VERSION_START = 10 BaseError = Class.new(StandardError) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7d7701bd617..018ce49965a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -11864,6 +11864,9 @@ msgstr "" msgid "Invalid query" msgstr "" +msgid "Invalid repository bundle for snippet with id %{snippet_id}" +msgstr "" + msgid "Invalid repository path" msgstr "" @@ -18085,15 +18088,33 @@ msgstr "" msgid "Repository cleanup has started. You will receive an email once the cleanup operation is complete." msgstr "" +msgid "Repository files count over the limit" +msgstr "" + +msgid "Repository has an invalid default branch name." +msgstr "" + +msgid "Repository has more than one branch." +msgstr "" + msgid "Repository has no locks." msgstr "" +msgid "Repository has tags." +msgstr "" + msgid "Repository maintenance" msgstr "" msgid "Repository mirroring" msgstr "" +msgid "Repository must contain at least 1 file." +msgstr "" + +msgid "Repository size is above the limit." +msgstr "" + msgid "Repository static objects" msgstr "" @@ -25382,6 +25403,9 @@ msgstr "" msgid "cannot block others" msgstr "" +msgid "cannot contain HTML/XML tags, including any word between angle brackets (<,>)." +msgstr "" + msgid "cannot include leading slash or directory traversal." msgstr "" diff --git a/package.json b/package.json index 82976399171..bcac6ca271c 100644 --- a/package.json +++ b/package.json @@ -121,7 +121,7 @@ "stickyfilljs": "^2.1.0", "style-loader": "^1.1.3", "svg4everybody": "2.1.9", - "swagger-ui-dist": "^3.24.3", + "swagger-ui-dist": "^3.26.2", "three": "^0.84.0", "three-orbit-controls": "^82.1.0", "three-stl-loader": "^1.0.4", @@ -146,7 +146,7 @@ "webpack-cli": "^3.3.11", "webpack-stats-plugin": "^0.3.1", "worker-loader": "^2.0.0", - "xterm": "^3.5.0" + "xterm": "3.14.5" }, "devDependencies": { "@babel/plugin-transform-modules-commonjs": "^7.8.3", diff --git a/spec/controllers/projects/wikis_controller_spec.rb b/spec/controllers/projects/wikis_controller_spec.rb index b4bbf76ce18..dba7c6bc469 100644 --- a/spec/controllers/projects/wikis_controller_spec.rb +++ b/spec/controllers/projects/wikis_controller_spec.rb @@ -141,43 +141,19 @@ describe Projects::WikisController do context 'when page is a file' do include WikiHelpers - let(:id) { upload_file_to_wiki(project, user, file_name) } + where(:file_name) { ['dk.png', 'unsanitized.svg', 'git-cheat-sheet.pdf'] } - context 'when file is an image' do - let(:file_name) { 'dk.png' } + with_them do + let(:id) { upload_file_to_wiki(project, user, file_name) } - it 'delivers the image' do + it 'delivers the file with the correct headers' do subject expect(response.headers['Content-Disposition']).to match(/^inline/) - expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true" + expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq('true') + expect(response.cache_control[:public]).to be(false) + expect(response.cache_control[:extras]).to include('no-store') end - - context 'when file is a svg' do - let(:file_name) { 'unsanitized.svg' } - - it 'delivers the image' do - subject - - expect(response.headers['Content-Disposition']).to match(/^inline/) - expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true" - end - end - - it_behaves_like 'project cache control headers' - end - - context 'when file is a pdf' do - let(:file_name) { 'git-cheat-sheet.pdf' } - - it 'sets the content type to sets the content response headers' do - subject - - expect(response.headers['Content-Disposition']).to match(/^inline/) - expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true" - end - - it_behaves_like 'project cache control headers' 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 57264f97ddc..28954ab47de 100644 --- a/spec/features/snippets/notes_on_personal_snippets_spec.rb +++ b/spec/features/snippets/notes_on_personal_snippets_spec.rb @@ -5,15 +5,17 @@ require 'spec_helper' describe 'Comments on personal snippets', :js do include NoteInteractionHelpers - let!(:user) { create(:user) } - let!(:snippet) { create(:personal_snippet, :public) } + let_it_be(:snippet) { create(:personal_snippet, :public) } + let_it_be(:other_note) { create(:note_on_personal_snippet) } + + let(:user_name) { 'Test User' } + let!(:user) { create(:user, name: user_name) } let!(:snippet_notes) do [ create(:note_on_personal_snippet, noteable: snippet, author: user), create(:note_on_personal_snippet, noteable: snippet) ] end - let!(:other_note) { create(:note_on_personal_snippet) } before do stub_feature_flags(snippets_vue: false) @@ -56,6 +58,26 @@ describe 'Comments on personal snippets', :js do expect(page).to show_user_status(status) end end + + it 'shows the author name' do + visit snippet_path(snippet) + + within("#note_#{snippet_notes[0].id}") do + expect(page).to have_content(user_name) + end + end + + context 'when the author name contains HTML' do + let(:user_name) { '<h1><a href="https://bad.link/malicious.exe" class="evil">Fake Content<img class="fake-icon" src="image.png"></a></h1>' } + + 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/finders/events_finder_spec.rb b/spec/finders/events_finder_spec.rb index 443e9ab4bc4..224b4289f51 100644 --- a/spec/finders/events_finder_spec.rb +++ b/spec/finders/events_finder_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' describe EventsFinder do let_it_be(:user) { create(:user) } + let(:private_user) { create(:user, private_profile: true) } let(:other_user) { create(:user) } let(:project1) { create(:project, :private, creator_id: user.id, namespace: user.namespace) } @@ -57,6 +58,12 @@ describe EventsFinder do expect(events).to be_empty end + + it 'returns nothing when the target profile is private' do + events = described_class.new(source: private_user, current_user: other_user).execute + + expect(events).to be_empty + end end describe 'wiki events feature flag' do diff --git a/spec/frontend/error_tracking/components/stacktrace_entry_spec.js b/spec/frontend/error_tracking/components/stacktrace_entry_spec.js index 2a4e826b4ab..de746b8ac84 100644 --- a/spec/frontend/error_tracking/components/stacktrace_entry_spec.js +++ b/spec/frontend/error_tracking/components/stacktrace_entry_spec.js @@ -1,8 +1,10 @@ import { shallowMount } from '@vue/test-utils'; +import { GlSprintf } from '@gitlab/ui'; import StackTraceEntry from '~/error_tracking/components/stacktrace_entry.vue'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import FileIcon from '~/vue_shared/components/file_icon.vue'; import Icon from '~/vue_shared/components/icon.vue'; +import { trimText } from 'helpers/text_helper'; describe('Stacktrace Entry', () => { let wrapper; @@ -21,6 +23,9 @@ describe('Stacktrace Entry', () => { errorLine: 24, ...props, }, + stubs: { + GlSprintf, + }, }); } @@ -53,7 +58,7 @@ describe('Stacktrace Entry', () => { const extraInfo = { errorLine: 34, errorFn: 'errorFn', errorColumn: 77 }; mountComponent({ expanded: false, lines: [], ...extraInfo }); expect(wrapper.find(Icon).exists()).toBe(false); - expect(findFileHeaderContent()).toContain( + expect(trimText(findFileHeaderContent())).toContain( `in ${extraInfo.errorFn} at line ${extraInfo.errorLine}:${extraInfo.errorColumn}`, ); }); @@ -61,17 +66,17 @@ describe('Stacktrace Entry', () => { it('should render only lineNo:columnNO when there is no errorFn ', () => { const extraInfo = { errorLine: 34, errorFn: null, errorColumn: 77 }; mountComponent({ expanded: false, lines: [], ...extraInfo }); - expect(findFileHeaderContent()).not.toContain(`in ${extraInfo.errorFn}`); - expect(findFileHeaderContent()).toContain(`${extraInfo.errorLine}:${extraInfo.errorColumn}`); + const fileHeaderContent = trimText(findFileHeaderContent()); + expect(fileHeaderContent).not.toContain(`in ${extraInfo.errorFn}`); + expect(fileHeaderContent).toContain(`${extraInfo.errorLine}:${extraInfo.errorColumn}`); }); it('should render only lineNo when there is no errorColumn ', () => { const extraInfo = { errorLine: 34, errorFn: 'errorFn', errorColumn: null }; mountComponent({ expanded: false, lines: [], ...extraInfo }); - expect(findFileHeaderContent()).toContain( - `in ${extraInfo.errorFn} at line ${extraInfo.errorLine}`, - ); - expect(findFileHeaderContent()).not.toContain(`:${extraInfo.errorColumn}`); + const fileHeaderContent = trimText(findFileHeaderContent()); + expect(fileHeaderContent).toContain(`in ${extraInfo.errorFn} at line ${extraInfo.errorLine}`); + expect(fileHeaderContent).not.toContain(`:${extraInfo.errorColumn}`); }); }); }); diff --git a/spec/frontend/issuables_list/components/issuable_spec.js b/spec/frontend/issuables_list/components/issuable_spec.js index 980def06078..834d18246a5 100644 --- a/spec/frontend/issuables_list/components/issuable_spec.js +++ b/spec/frontend/issuables_list/components/issuable_spec.js @@ -1,5 +1,5 @@ import { shallowMount } from '@vue/test-utils'; -import { GlLink } from '@gitlab/ui'; +import { GlSprintf } from '@gitlab/ui'; import { TEST_HOST } from 'helpers/test_constants'; import { trimText } from 'helpers/text_helper'; import initUserPopovers from '~/user_popovers'; @@ -44,6 +44,10 @@ describe('Issuable component', () => { baseUrl: TEST_BASE_URL, ...props, }, + stubs: { + 'gl-sprintf': GlSprintf, + 'gl-link': '<a><slot></slot></a>', + }, }); }; @@ -66,12 +70,12 @@ describe('Issuable component', () => { const findConfidentialIcon = () => wrapper.find('.fa-eye-slash'); const findTaskStatus = () => wrapper.find('.task-status'); - const findOpenedAgoContainer = () => wrapper.find({ ref: 'openedAgoByContainer' }); + const findOpenedAgoContainer = () => wrapper.find('[data-testid="openedByMessage"]'); const findMilestone = () => wrapper.find('.js-milestone'); const findMilestoneTooltip = () => findMilestone().attributes('title'); const findDueDate = () => wrapper.find('.js-due-date'); const findLabelContainer = () => wrapper.find('.js-labels'); - const findLabelLinks = () => findLabelContainer().findAll(GlLink); + const findLabelLinks = () => findLabelContainer().findAll('a'); const findWeight = () => wrapper.find('.js-weight'); const findAssignees = () => wrapper.find(IssueAssignees); const findMergeRequestsCount = () => wrapper.find('.js-merge-requests'); @@ -86,7 +90,7 @@ describe('Issuable component', () => { factory(); - expect(initUserPopovers).toHaveBeenCalledWith([findOpenedAgoContainer().find('a').element]); + expect(initUserPopovers).toHaveBeenCalledWith([wrapper.vm.$refs.openedAgoByContainer.$el]); }); }); @@ -135,7 +139,7 @@ describe('Issuable component', () => { }); it('renders fuzzy opened date and author', () => { - expect(trimText(findOpenedAgoContainer().text())).toEqual( + expect(trimText(findOpenedAgoContainer().text())).toContain( `opened 1 month ago by ${TEST_USER_NAME}`, ); }); diff --git a/spec/initializers/cookies_serializer_spec.rb b/spec/initializers/cookies_serializer_spec.rb new file mode 100644 index 00000000000..6167039d3c2 --- /dev/null +++ b/spec/initializers/cookies_serializer_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Cookies serializer initializer' do + def load_initializer + load Rails.root.join('config/initializers/cookies_serializer.rb') + end + + subject { Rails.application.config.action_dispatch.cookies_serializer } + + it 'uses JSON serializer by default' do + load_initializer + + expect(subject).to eq(:json) + end + + it 'uses the unsafe hybrid serializer when the environment variables is set' do + stub_env('USE_UNSAFE_HYBRID_COOKIES', 'true') + + load_initializer + + expect(subject).to eq(:hybrid) + end +end diff --git a/spec/lib/banzai/filter/abstract_reference_filter_spec.rb b/spec/lib/banzai/filter/abstract_reference_filter_spec.rb index 798112d0f53..6890a70518b 100644 --- a/spec/lib/banzai/filter/abstract_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/abstract_reference_filter_spec.rb @@ -20,6 +20,18 @@ describe Banzai::Filter::AbstractReferenceFilter do end end + describe '#data_attributes_for' do + let_it_be(:issue) { create(:issue, project: project) } + + it 'is not an XSS vector' do + allow(described_class).to receive(:object_class).and_return(Issue) + + data_attributes = filter.data_attributes_for('xss <img onerror=alert(1) src=x>', project, issue, link_content: true) + + expect(data_attributes[:original]).to eq('xss &lt;img onerror=alert(1) src=x&gt;') + end + end + describe '#parent_per_reference' do it 'returns a Hash containing projects grouped per parent paths' do expect(filter).to receive(:references_per_parent) diff --git a/spec/lib/banzai/filter/upload_link_filter_spec.rb b/spec/lib/banzai/filter/upload_link_filter_spec.rb index c366f774895..8844ad78306 100644 --- a/spec/lib/banzai/filter/upload_link_filter_spec.rb +++ b/spec/lib/banzai/filter/upload_link_filter_spec.rb @@ -229,6 +229,7 @@ describe Banzai::Filter::UploadLinkFilter do 'invalid UTF-8 byte sequences' | '%FF' 'garbled path' | 'open(/var/tmp/):%20/location%0Afrom:%20/test' 'whitespace' | "d18213acd3732630991986120e167e3d/Landscape_8.jpg\nand more" + 'null byte' | "%00" end with_them do diff --git a/spec/lib/banzai/pipeline/full_pipeline_spec.rb b/spec/lib/banzai/pipeline/full_pipeline_spec.rb index 4fa39da3eb4..b4047e369fb 100644 --- a/spec/lib/banzai/pipeline/full_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/full_pipeline_spec.rb @@ -24,7 +24,7 @@ describe Banzai::Pipeline::FullPipeline do it 'escapes the data-original attribute on a reference' do markdown = %Q{[">bad things](#{issue.to_reference})} result = described_class.to_html(markdown, project: project) - expect(result).to include(%{data-original='\">bad things'}) + expect(result).to include(%{data-original='\"&gt;bad things'}) end end diff --git a/spec/lib/gitlab/import_export/snippet_repo_restorer_spec.rb b/spec/lib/gitlab/import_export/snippet_repo_restorer_spec.rb index 3ce950d6a64..030ec1b8d8e 100644 --- a/spec/lib/gitlab/import_export/snippet_repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/snippet_repo_restorer_spec.rb @@ -4,9 +4,9 @@ require 'spec_helper' describe Gitlab::ImportExport::SnippetRepoRestorer do let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, namespace: user.namespace) } - let(:snippet) { create(:project_snippet, project: project, author: user) } + let(:project) { create(:project, namespace: user.namespace) } + let(:snippet) { create(:project_snippet, project: project, author: user) } let(:shared) { project.import_export_shared } let(:exporter) { Gitlab::ImportExport::SnippetsRepoSaver.new(project: project, shared: shared, current_user: user) } let(:restorer) do @@ -49,33 +49,63 @@ describe Gitlab::ImportExport::SnippetRepoRestorer do it_behaves_like 'no bundle file present' end - context 'when the snippet bundle exists' do - let!(:snippet_with_repo) { create(:project_snippet, :repository, project: project) } + context 'when the snippet repository bundle exists' do + let!(:snippet_with_repo) { create(:project_snippet, :repository, project: project, author: user) } let(:bundle_path) { ::Gitlab::ImportExport.snippets_repo_bundle_path(shared.export_path) } let(:snippet_bundle_path) { File.join(bundle_path, "#{snippet_with_repo.hexdigest}.bundle") } let(:result) { exporter.save } + let(:repository) { snippet.repository } before do expect(exporter.save).to be_truthy end - it 'creates the repository from the bundle' do - expect(snippet.repository_exists?).to be_falsey - expect(snippet.snippet_repository).to be_nil - expect(snippet.repository).to receive(:create_from_bundle).and_call_original + context 'when it is valid' do + before do + allow(repository).to receive(:branch_count).and_return(1) + allow(repository).to receive(:tag_count).and_return(0) + allow(repository).to receive(:branch_names).and_return(['master']) + allow(repository).to receive(:ls_files).and_return(['foo']) + end - expect(restorer.restore).to be_truthy - expect(snippet.repository_exists?).to be_truthy - expect(snippet.snippet_repository).not_to be_nil - end + it 'creates the repository from the bundle' do + expect(snippet.repository_exists?).to be_falsey + expect(snippet.snippet_repository).to be_nil + expect(repository).to receive(:create_from_bundle).and_call_original + + expect(restorer.restore).to be_truthy + expect(snippet.repository_exists?).to be_truthy + expect(snippet.snippet_repository).not_to be_nil + end - it 'sets same shard in snippet repository as in the repository storage' do - expect(snippet).to receive(:repository_storage).and_return('picked') - expect(snippet.repository).to receive(:create_from_bundle) + it 'sets same shard in snippet repository as in the repository storage' do + expect(repository).to receive(:storage).and_return('picked') + expect(repository).to receive(:create_from_bundle) - restorer.restore + expect(restorer.restore).to be_truthy + expect(snippet.snippet_repository.shard_name).to eq 'picked' + end + end - expect(snippet.snippet_repository.shard_name).to eq 'picked' + context 'when it is invalid' do + it 'returns false and deletes the repository from disk and the database' do + gitlab_shell = Gitlab::Shell.new + shard_name = snippet.repository.shard + path = snippet.disk_path + '.git' + error_response = ServiceResponse.error(message: 'Foo', http_status: 400) + + allow_next_instance_of(Snippets::RepositoryValidationService) do |instance| + allow(instance).to receive(:execute).and_return(error_response) + end + + aggregate_failures do + expect(restorer.restore).to be false + expect(shared.errors.first).to match(/Invalid repository bundle/) + expect(snippet.repository_exists?).to eq false + expect(snippet.reload.snippet_repository).to be_nil + expect(gitlab_shell.repository_exists?(shard_name, path)).to eq false + end + end end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index fc4590f7b22..61893f78fd1 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -3566,7 +3566,7 @@ describe MergeRequest do describe '#merge_participants' do it 'contains author' do - expect(subject.merge_participants).to eq([subject.author]) + expect(subject.merge_participants).to contain_exactly(subject.author) end describe 'when merge_when_pipeline_succeeds? is true' do @@ -3580,8 +3580,20 @@ describe MergeRequest do author: user) end - it 'contains author only' do - expect(subject.merge_participants).to eq([subject.author]) + context 'author is not a project member' do + it 'is empty' do + expect(subject.merge_participants).to be_empty + end + end + + context 'author is a project member' do + before do + subject.project.team.add_reporter(user) + end + + it 'contains author only' do + expect(subject.merge_participants).to contain_exactly(subject.author) + end end end @@ -3594,8 +3606,24 @@ describe MergeRequest do merge_user: merge_user) end - it 'contains author and merge user' do - expect(subject.merge_participants).to eq([subject.author, merge_user]) + before do + subject.project.team.add_reporter(subject.author) + end + + context 'merge user is not a member' do + it 'contains author only' do + expect(subject.merge_participants).to contain_exactly(subject.author) + end + end + + context 'both author and merge users are project members' do + before do + subject.project.team.add_reporter(merge_user) + end + + it 'contains author and merge user' do + expect(subject.merge_participants).to contain_exactly(subject.author, merge_user) + end end end end diff --git a/spec/requests/api/deploy_tokens_spec.rb b/spec/requests/api/deploy_tokens_spec.rb index 499c334d491..2b86d59fbba 100644 --- a/spec/requests/api/deploy_tokens_spec.rb +++ b/spec/requests/api/deploy_tokens_spec.rb @@ -204,7 +204,7 @@ describe API::DeployTokens do end context 'deploy token creation' do - shared_examples 'creating a deploy token' do |entity, unauthenticated_response| + shared_examples 'creating a deploy token' do |entity, unauthenticated_response, authorized_role| let(:expires_time) { 1.year.from_now } let(:params) do { @@ -231,9 +231,9 @@ describe API::DeployTokens do it { is_expected.to have_gitlab_http_status(:forbidden) } end - context 'when authenticated as maintainer' do + context "when authenticated as #{authorized_role}" do before do - send(entity).add_maintainer(user) + send(entity).send("add_#{authorized_role}", user) end it 'creates the deploy token' do @@ -282,7 +282,7 @@ describe API::DeployTokens do response end - it_behaves_like 'creating a deploy token', :project, :not_found + it_behaves_like 'creating a deploy token', :project, :not_found, :maintainer end describe 'POST /groups/:id/deploy_tokens' do @@ -291,7 +291,17 @@ describe API::DeployTokens do response end - it_behaves_like 'creating a deploy token', :group, :forbidden + it_behaves_like 'creating a deploy token', :group, :forbidden, :owner + + context 'when authenticated as maintainer' do + before do + group.add_maintainer(user) + end + + let(:params) { { name: 'test', scopes: ['read_repository'] } } + + it { is_expected.to have_gitlab_http_status(:forbidden) } + end end end @@ -320,6 +330,14 @@ describe API::DeployTokens do group.add_maintainer(user) end + it { is_expected.to have_gitlab_http_status(:forbidden) } + end + + context 'when authenticated as owner' do + before do + group.add_owner(user) + end + it 'calls the deploy token destroy service' do expect(::Groups::DeployTokens::DestroyService).to receive(:new) .with(group, user, token_id: group_deploy_token.id) diff --git a/spec/requests/api/events_spec.rb b/spec/requests/api/events_spec.rb index decdcc66327..dd03a784c96 100644 --- a/spec/requests/api/events_spec.rb +++ b/spec/requests/api/events_spec.rb @@ -192,6 +192,19 @@ describe API::Events do end end + context 'when target users profile is private' do + it 'returns no events' do + user.update!(private_profile: true) + private_project.add_developer(non_member) + + get api("/users/#{user.username}/events", non_member) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to eq([]) + end + end + context 'when scope is passed' do context 'when unauthenticated' do it 'returns no user events' do diff --git a/spec/requests/api/import_github_spec.rb b/spec/requests/api/import_github_spec.rb index ecdb02beeb0..f33436df40e 100644 --- a/spec/requests/api/import_github_spec.rb +++ b/spec/requests/api/import_github_spec.rb @@ -26,6 +26,18 @@ describe API::ImportGithub do end end + it 'rejects requests when Github Importer is disabled' do + stub_application_setting(import_sources: nil) + + post api("/import/github", user), params: { + target_namespace: user.namespace_path, + personal_access_token: token, + repo_id: non_existing_record_id + } + + expect(response).to have_gitlab_http_status(:forbidden) + end + it 'returns 201 response when the project is imported successfully' do allow(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider) diff --git a/spec/services/snippets/repository_validation_service_spec.rb b/spec/services/snippets/repository_validation_service_spec.rb new file mode 100644 index 00000000000..1c139d8c223 --- /dev/null +++ b/spec/services/snippets/repository_validation_service_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Snippets::RepositoryValidationService do + describe '#execute' do + let_it_be(:user) { create(:user) } + let_it_be(:snippet) { create(:personal_snippet, :empty_repo, author: user) } + + let(:repository) { snippet.repository } + let(:service) { described_class.new(user, snippet) } + + subject { service.execute } + + before do + allow(repository).to receive(:branch_count).and_return(1) + allow(repository).to receive(:ls_files).and_return(['foo']) + allow(repository).to receive(:branch_names).and_return(['master']) + end + + it 'returns error when the repository has more than one branch' do + allow(repository).to receive(:branch_count).and_return(2) + + expect(subject).to be_error + expect(subject.message).to match /Repository has more than one branch/ + end + + it 'returns error when existing branch name is not the default one' do + allow(repository).to receive(:branch_names).and_return(['foo']) + + expect(subject).to be_error + expect(subject.message).to match /Repository has an invalid default branch name/ + end + + it 'returns error when the repository has tags' do + allow(repository).to receive(:tag_count).and_return(1) + + expect(subject).to be_error + expect(subject.message).to match /Repository has tags/ + end + + it 'returns error when the repository has more file than the limit' do + limit = Snippet.max_file_limit(user) + 1 + files = Array.new(limit) { FFaker::Filesystem.file_name } + allow(repository).to receive(:ls_files).and_return(files) + + expect(subject).to be_error + expect(subject.message).to match /Repository files count over the limit/ + end + + it 'returns error when the repository has no files' do + allow(repository).to receive(:ls_files).and_return([]) + + expect(subject).to be_error + expect(subject.message).to match /Repository must contain at least 1 file/ + end + + it 'returns error when the repository size is over the limit' do + expect_any_instance_of(Gitlab::RepositorySizeChecker).to receive(:above_size_limit?).and_return(true) + + expect(subject).to be_error + expect(subject.message).to match /Repository size is above the limit/ + end + + it 'returns success when no validation errors are raised' do + expect(subject).to be_success + end + end +end diff --git a/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb b/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb index 53183ac89f8..fb6d6603beb 100644 --- a/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb @@ -4,6 +4,16 @@ RSpec.shared_examples 'an unauthorized API user' do it { is_expected.to eq(403) } end +RSpec.shared_examples 'API user with insufficient permissions' do + context 'with non member that is the author' do + before do + issuable.update!(author: non_member) # an external author can't admin issuable + end + + it_behaves_like 'an unauthorized API user' + end +end + RSpec.shared_examples 'time tracking endpoints' do |issuable_name| let(:non_member) { create(:user) } @@ -14,6 +24,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name| subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", non_member), params: { duration: '1w' }) } it_behaves_like 'an unauthorized API user' + it_behaves_like 'API user with insufficient permissions' end it "sets the time estimate for #{issuable_name}" do @@ -53,6 +64,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name| subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_time_estimate", non_member)) } it_behaves_like 'an unauthorized API user' + it_behaves_like 'API user with insufficient permissions' end it "resets the time estimate for #{issuable_name}" do @@ -70,6 +82,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name| end it_behaves_like 'an unauthorized API user' + it_behaves_like 'API user with insufficient permissions' end it "add spent time for #{issuable_name}" do @@ -119,6 +132,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name| subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_spent_time", non_member)) } it_behaves_like 'an unauthorized API user' + it_behaves_like 'API user with insufficient permissions' end it "resets spent time for #{issuable_name}" do diff --git a/spec/validators/html_safety_validator_spec.rb b/spec/validators/html_safety_validator_spec.rb new file mode 100644 index 00000000000..4d9425235e3 --- /dev/null +++ b/spec/validators/html_safety_validator_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe HtmlSafetyValidator do + let(:validator) { described_class.new(attributes: [:name]) } + let(:group) { build(:group) } + + def validate(value) + validator.validate_each(group, :name, value) + end + + it 'adds an error when a script is included in the name' do + validate('My group <script>evil_script</script>') + + expect(group.errors[:name]).to eq([HtmlSafetyValidator.error_message]) + end + + it 'does not add an error when an ampersand is included in the name' do + validate('Group with 1 & 2') + + expect(group.errors).to be_empty + end +end diff --git a/yarn.lock b/yarn.lock index f6672fc96c8..9270bfe5c13 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10933,10 +10933,10 @@ svg4everybody@2.1.9: resolved "https://registry.yarnpkg.com/svg4everybody/-/svg4everybody-2.1.9.tgz#5bd9f6defc133859a044646d4743fabc28db7e2d" integrity sha1-W9n23vwTOFmgRGRtR0P6vCjbfi0= -swagger-ui-dist@^3.24.3: - version "3.24.3" - resolved "https://registry.yarnpkg.com/swagger-ui-dist/-/swagger-ui-dist-3.24.3.tgz#99754d11b0ddd314a1a50db850acb415e4b0a0c6" - integrity sha512-kB8qobP42Xazaym7sD9g5mZuRL4416VIIYZMqPEIskkzKqbPLQGEiHA3ga31bdzyzFLgr6Z797+6X1Am6zYpbg== +swagger-ui-dist@^3.26.2: + version "3.26.2" + resolved "https://registry.yarnpkg.com/swagger-ui-dist/-/swagger-ui-dist-3.26.2.tgz#22c700906c8911b1c9956da6c3fca371dba6219f" + integrity sha512-cpR3A9uEs95gGQSaIXgiTpnetIifTF1u2a0fWrnVl+HyLpCdHVgOy7FGlVD1iVkts7AE5GOiGjA7VyDNiRaNgw== symbol-observable@^1.0.2: version "1.2.0" @@ -12284,10 +12284,10 @@ xtend@^4.0.0, xtend@^4.0.1, xtend@~4.0.1: resolved "https://registry.yarnpkg.com/xtend/-/xtend-4.0.2.tgz#bb72779f5fa465186b1f438f674fa347fdb5db54" integrity sha512-LKYU1iAXJXUgAXn9URjiu+MWhyUXHsvfp7mcuYm9dSUKK0/CjtrUwFAxD82/mCWbtLsGjFIad0wIsod4zrTAEQ== -xterm@^3.5.0: - version "3.5.0" - resolved "https://registry.yarnpkg.com/xterm/-/xterm-3.5.0.tgz#ba3f464bc5730c9d259ebe62131862224db9ddcc" - integrity sha512-IpG3P3gkT0/xDPS0j3igpk92JYlUajaEHk3/EQSUeIRJmPiF2lyham3Xt/GD3o98uOrRluvowjNj0AFeYK+AXQ== +xterm@3.14.5: + version "3.14.5" + resolved "https://registry.yarnpkg.com/xterm/-/xterm-3.14.5.tgz#c9d14e48be6873aa46fb429f22f2165557fd2dea" + integrity sha512-DVmQ8jlEtL+WbBKUZuMxHMBgK/yeIZwkXB81bH+MGaKKnJGYwA+770hzhXPfwEIokK9On9YIFPRleVp/5G7z9g== y18n@^3.2.1: version "3.2.1" |