diff options
Diffstat (limited to 'app')
| -rw-r--r-- | app/assets/javascripts/error_tracking/components/stacktrace_entry.vue | 57 | ||||
| -rw-r--r-- | app/assets/javascripts/issuables_list/components/issuable.vue | 61 | ||||
| -rw-r--r-- | app/controllers/concerns/wiki_actions.rb | 2 | ||||
| -rw-r--r-- | app/controllers/groups/application_controller.rb | 12 | ||||
| -rw-r--r-- | app/controllers/groups/deploy_tokens_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/groups/settings/repository_controller.rb | 2 | ||||
| -rw-r--r-- | app/finders/events_finder.rb | 6 | ||||
| -rw-r--r-- | app/models/group.rb | 7 | ||||
| -rw-r--r-- | app/models/merge_request.rb | 2 | ||||
| -rw-r--r-- | app/policies/group_policy.rb | 4 | ||||
| -rw-r--r-- | app/services/snippets/repository_validation_service.rb | 72 | ||||
| -rw-r--r-- | app/validators/html_safety_validator.rb | 36 | ||||
| -rw-r--r-- | app/views/import/bitbucket_server/status.html.haml | 4 | ||||
| -rw-r--r-- | app/views/shared/notes/_note.html.haml | 2 |
14 files changed, 201 insertions, 68 deletions
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/concerns/wiki_actions.rb b/app/controllers/concerns/wiki_actions.rb index b4b4fd84c37..7eef12fadfe 100644 --- a/app/controllers/concerns/wiki_actions.rb +++ b/app/controllers/concerns/wiki_actions.rb @@ -58,7 +58,7 @@ module WikiActions render 'shared/wikis/show' elsif file_blob - send_blob(wiki.repository, file_blob, allow_caching: container.public?) + send_blob(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/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/finders/events_finder.rb b/app/finders/events_finder.rb index 52612f1f8aa..004fbc4cd22 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) @@ -103,6 +105,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 dd7624ab420..71f58a5fd1a 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 caf7b554427..a7e0907eb5f 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -518,7 +518,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 7523b8f7b1c..3e16f449831 100644 --- a/app/views/import/bitbucket_server/status.html.haml +++ b/app/views/import/bitbucket_server/status.html.haml @@ -57,7 +57,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 @@ -78,7 +78,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 |