Commit 3ad30f1f authored by Scott Stern's avatar Scott Stern Committed by GitLab Release Tools Bot

Make sure user info is sanitized when rendered

parent d7c0fba9
......@@ -6,7 +6,7 @@
// TODO: need to move this component to graphql - https://gitlab.com/gitlab-org/gitlab/-/issues/221246
import { escape, isNumber } from 'lodash';
import { GlLink, GlTooltipDirective as GlTooltip, GlLabel } from '@gitlab/ui';
import { GlLink, GlTooltipDirective as GlTooltip, GlSprintf, GlLabel } from '@gitlab/ui';
import {
dateInWords,
formatDate,
......@@ -24,12 +24,15 @@ import { isScopedLabel } from '~/lib/utils/common_utils';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
export default {
isScopedLabel,
i18n: {
openedAgo: __('opened %{timeAgoString} by %{user}'),
},
components: {
Icon,
IssueAssignees,
GlLink,
GlLabel,
GlSprintf,
},
directives: {
GlTooltip,
......@@ -105,23 +108,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;
......@@ -167,7 +168,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: {
issuableLink(params) {
......@@ -233,9 +234,22 @@ export default {
<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
......
---
title: Fix security issue when rendering issuable
merge_request:
author:
type: security
import { shallowMount } from '@vue/test-utils';
import { GlLabel } from '@gitlab/ui';
import { GlSprintf, GlLabel } from '@gitlab/ui';
import { TEST_HOST } from 'helpers/test_constants';
import { trimText } from 'helpers/text_helper';
import initUserPopovers from '~/user_popovers';
......@@ -50,6 +50,10 @@ describe('Issuable component', () => {
scopedLabels,
},
},
stubs: {
'gl-sprintf': GlSprintf,
'gl-link': '<a><slot></slot></a>',
},
});
};
......@@ -73,7 +77,7 @@ 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');
......@@ -94,7 +98,7 @@ describe('Issuable component', () => {
factory();
expect(initUserPopovers).toHaveBeenCalledWith([findOpenedAgoContainer().find('a').element]);
expect(initUserPopovers).toHaveBeenCalledWith([wrapper.vm.$refs.openedAgoByContainer.$el]);
});
});
......@@ -191,7 +195,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}`,
);
});
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment