summaryrefslogtreecommitdiff
path: root/app
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2020-06-29 19:21:38 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2020-06-29 19:21:38 +0000
commit11e9b7b58837da351f08c18e6f0f4faba4d7d301 (patch)
treed9b28159a53c3814c8a2e6b33a5f01557b757439 /app
parent2b0b97e746e327c6168505df7740e667b690a27f (diff)
downloadgitlab-ce-11e9b7b58837da351f08c18e6f0f4faba4d7d301.tar.gz
Add latest changes from gitlab-org/security/gitlab@13-1-stable-ee
Diffstat (limited to 'app')
-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/models/group.rb7
-rw-r--r--app/policies/group_policy.rb4
-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
10 files changed, 121 insertions, 66 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 }}&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/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/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/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 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