summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>2020-07-01 17:04:05 +0000
committerGitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>2020-07-01 17:04:05 +0000
commitf1f06dfe2c91d253d8d1e76e01dfc89c52dd4387 (patch)
tree075dc90f61e7a42a30a6026278824f2a99291443
parentbcfbac449a751edb6174233b97b8ce1b73e6f7a2 (diff)
parentfef9786ee5723436fcb3ea53c4cd1b90db8227f6 (diff)
downloadgitlab-ce-f1f06dfe2c91d253d8d1e76e01dfc89c52dd4387.tar.gz
Merge remote-tracking branch 'dev/13-0-stable' into 13-0-stable
-rw-r--r--CHANGELOG.md24
-rw-r--r--GITALY_SERVER_VERSION2
-rw-r--r--Gemfile.lock18
-rw-r--r--VERSION2
-rw-r--r--app/assets/javascripts/error_tracking/components/stacktrace_entry.vue57
-rw-r--r--app/assets/javascripts/issuables_list/components/issuable.vue61
-rw-r--r--app/controllers/groups/application_controller.rb12
-rw-r--r--app/controllers/groups/deploy_tokens_controller.rb2
-rw-r--r--app/controllers/groups/settings/repository_controller.rb2
-rw-r--r--app/controllers/projects/wikis_controller.rb2
-rw-r--r--app/finders/events_finder.rb6
-rw-r--r--app/models/group.rb7
-rw-r--r--app/models/merge_request.rb2
-rw-r--r--app/policies/group_policy.rb4
-rw-r--r--app/services/snippets/repository_validation_service.rb72
-rw-r--r--app/validators/html_safety_validator.rb36
-rw-r--r--app/views/import/bitbucket_server/status.html.haml4
-rw-r--r--app/views/shared/notes/_note.html.haml2
-rw-r--r--config/initializers/cookies_serializer.rb3
-rw-r--r--doc/api/deploy_tokens.md3
-rw-r--r--doc/user/permissions.md2
-rw-r--r--lib/api/import_github.rb4
-rw-r--r--lib/api/time_tracking_endpoints.rb12
-rw-r--r--lib/banzai/filter/abstract_reference_filter.rb2
-rw-r--r--lib/banzai/filter/base_relative_link_filter.rb2
-rw-r--r--lib/gitlab/import_export/snippet_repo_restorer.rb14
-rw-r--r--lib/gitlab/markdown_cache.rb2
-rw-r--r--locale/gitlab.pot24
-rw-r--r--package.json4
-rw-r--r--spec/controllers/projects/wikis_controller_spec.rb38
-rw-r--r--spec/features/snippets/notes_on_personal_snippets_spec.rb28
-rw-r--r--spec/finders/events_finder_spec.rb7
-rw-r--r--spec/frontend/error_tracking/components/stacktrace_entry_spec.js19
-rw-r--r--spec/frontend/issuables_list/components/issuable_spec.js14
-rw-r--r--spec/initializers/cookies_serializer_spec.rb25
-rw-r--r--spec/lib/banzai/filter/abstract_reference_filter_spec.rb12
-rw-r--r--spec/lib/banzai/filter/upload_link_filter_spec.rb1
-rw-r--r--spec/lib/banzai/pipeline/full_pipeline_spec.rb2
-rw-r--r--spec/lib/gitlab/import_export/snippet_repo_restorer_spec.rb64
-rw-r--r--spec/models/merge_request_spec.rb38
-rw-r--r--spec/requests/api/deploy_tokens_spec.rb28
-rw-r--r--spec/requests/api/events_spec.rb13
-rw-r--r--spec/requests/api/import_github_spec.rb12
-rw-r--r--spec/services/snippets/repository_validation_service_spec.rb69
-rw-r--r--spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb14
-rw-r--r--spec/validators/html_safety_validator_spec.rb24
-rw-r--r--yarn.lock16
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
diff --git a/VERSION b/VERSION
index 4e8f3240ec8..8e7aa5e40f2 100644
--- a/VERSION
+++ b/VERSION
@@ -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 }}&nbsp;</span>
+ </template>
+ <template #errorFn>
+ <strong>{{ errorFn }}&nbsp;</strong>
+ </template>
+ </gl-sprintf>
+
+ <gl-sprintf :message="__('%{spanStart}at line%{spanEnd} %{errorLine}%{errorColumn}')">
+ <template #span="{content}">
+ <span class="gl-text-gray-400">{{ content }}&nbsp;</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">
&middot;
- <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('&', '&amp;')
+ 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 &lt;img onerror=alert(1) src=x&gt;', project, issue, link_content: true)
+
+ expect(data_attributes[:original]).to eq('xss &amp;lt;img onerror=alert(1) src=x&amp;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='\"&gt;bad things'})
+ expect(result).to include(%{data-original='\"&amp;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"