From 0c17ee642c95c65f5a383d65e7c16ccfe7152e6a Mon Sep 17 00:00:00 2001
From: David Pisek <dpisek@gitlab.com>
Date: Tue, 22 Oct 2024 09:42:41 +0000
Subject: [PATCH] Add HTML-rendering to the alert util

In some cases, error messages that are provided
by the backend contain HTML tags, for example
links to relevant documentation.

This change adds an option to the alert-js
utility, to allow the provided message to be
rendered as HTML, so we can show those error
messages correctly.
---
 app/assets/javascripts/alert.js               | 15 ++++++
 .../vulnerability_list_graphql.vue            | 50 +++----------------
 .../vulnerability_list_graphql_spec.js        | 15 +++---
 spec/frontend/alert_spec.js                   | 34 +++++++++++++
 4 files changed, 62 insertions(+), 52 deletions(-)

diff --git a/app/assets/javascripts/alert.js b/app/assets/javascripts/alert.js
index fd060c918100d..1af4315487e9c 100644
--- a/app/assets/javascripts/alert.js
+++ b/app/assets/javascripts/alert.js
@@ -3,6 +3,7 @@ import isEmpty from 'lodash/isEmpty';
 import { GlAlert, GlLink, GlSprintf } from '@gitlab/ui';
 import * as Sentry from '~/sentry/sentry_browser_wrapper';
 import { __ } from '~/locale';
+import { sanitize } from '~/lib/dompurify';
 
 export const VARIANT_SUCCESS = 'success';
 export const VARIANT_WARNING = 'warning';
@@ -76,6 +77,7 @@ export const createAlert = ({
   error = null,
   messageLinks = null,
   dismissible = true,
+  renderMessageHTML = false,
 }) => {
   if (captureError && error) Sentry.captureException(error);
 
@@ -90,6 +92,19 @@ export const createAlert = ({
   }
 
   const createMessageNodes = (h) => {
+    if (renderMessageHTML) {
+      return [
+        h('div', {
+          domProps: {
+            innerHTML: sanitize(message, {
+              ALLOWED_TAGS: ['a'],
+              ALLOWED_ATTR: ['href', 'rel', 'target'],
+            }),
+          },
+        }),
+      ];
+    }
+
     if (isEmpty(messageLinks)) {
       return message;
     }
diff --git a/ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql.vue b/ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql.vue
index 0e8ea646abbf4..1531c2cd1b1fd 100644
--- a/ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql.vue
+++ b/ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql.vue
@@ -22,43 +22,6 @@ const GRAPHQL_DATA_PATH = {
   [DASHBOARD_TYPES.PIPELINE]: 'project.pipeline.securityReportFindings',
 };
 
-// @NOTE: This is a temporary solution to extract links from the error message.
-// It was specifically created to handle error messages returned by the GraphQL API when
-// there are failures with the Jira integration (app/services/jira/requests/base.rb) and we
-// need to render links to relevant docs/settings pages.
-// There is a follow-up to improve the `createAlert`, which will make this obsolete:
-// https://gitlab.com/gitlab-org/gitlab/-/issues/488393
-export const getErrorAlertFormatFromHtml = (htmlErrorMessage = '') => {
-  // Initialize variables
-  let linkCounter = 0;
-  let messageLinks = null;
-
-  // Regular expression to match <a> tags and extract both the href and the text content
-  const linkRegex = /<a\s+[^>]*href="([^"]*)".*?>(.*?)<\/a>/g;
-
-  // Replace all <a> tags with placeholders and extract the links
-  const message = htmlErrorMessage.replace(linkRegex, (match, href, text) => {
-    if (messageLinks === null) {
-      messageLinks = {};
-    }
-
-    // Generate dynamic placeholders for each link
-    const placeholderStart = `%{link${linkCounter}Start}`;
-    const placeholderEnd = `%{link${linkCounter}End}`;
-
-    // Add the extracted href to the messageLinks object
-    messageLinks[`link${linkCounter}`] = href;
-
-    // Increment the link counter
-    linkCounter += 1;
-
-    // Return the placeholders wrapped around the link text
-    return `${placeholderStart}${text}${placeholderEnd}`;
-  });
-
-  return { message, messageLinks };
-};
-
 export default {
   components: {
     VulnerabilityList,
@@ -131,14 +94,13 @@ export default {
         return vulnerabilities.nodes;
       },
       error({ message }) {
-        createAlert(
-          getErrorAlertFormatFromHtml(
-            message ||
-              s__(
-                'SecurityReports|Error fetching the vulnerability list. Please check your network connection and try again.',
-              ),
-          ),
+        const fallbackMessage = s__(
+          'SecurityReports|Error fetching the vulnerability list. Please check your network connection and try again.',
         );
+        createAlert({
+          message: message || fallbackMessage,
+          renderMessageHTML: true,
+        });
       },
       skip() {
         return !this.filters;
diff --git a/ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql_spec.js b/ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql_spec.js
index c844095044fbd..ada5097480886 100644
--- a/ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql_spec.js
+++ b/ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql_spec.js
@@ -140,22 +140,21 @@ describe('Vulnerability list GraphQL component', () => {
     });
 
     it.each`
-      errorMessage                                                  | expectedAlertMessage                                                                            | expectedAlertmessageLinks
-      ${'Something went wrong'}                                     | ${'Something went wrong'}                                                                       | ${null}
-      ${'Something went wrong: <a href="http://docs.com">docs</a>'} | ${'Something went wrong: %{link0Start}docs%{link0End}'}                                         | ${{ link0: 'http://docs.com' }}
-      ${'Something went wrong: <a href="http://docs.com">docs</a>'} | ${'Something went wrong: %{link0Start}docs%{link0End}'}                                         | ${{ link0: 'http://docs.com' }}
-      ${undefined}                                                  | ${'Error fetching the vulnerability list. Please check your network connection and try again.'} | ${null}
-      ${''}                                                         | ${'Error fetching the vulnerability list. Please check your network connection and try again.'} | ${null}
+      errorMessage                                                                                | expectedAlertMessage
+      ${'Something went wrong'}                                                                   | ${'Something went wrong'}
+      ${'Something went wrong. <a href="http://documentation.com/describe-error">Learn more</a>'} | ${'Something went wrong. <a href="http://documentation.com/describe-error">Learn more</a>'}
+      ${undefined}                                                                                | ${'Error fetching the vulnerability list. Please check your network connection and try again.'}
+      ${''}                                                                                       | ${'Error fetching the vulnerability list. Please check your network connection and try again.'}
     `(
       'shows an error message if the query fails',
-      async ({ errorMessage, expectedAlertMessage, expectedAlertmessageLinks }) => {
+      async ({ errorMessage, expectedAlertMessage }) => {
         const vulnerabilitiesHandler = jest.fn().mockRejectedValue(new Error(errorMessage));
         createWrapper({ vulnerabilitiesHandler });
         await waitForPromises();
 
         expect(createAlert).toHaveBeenCalledWith({
           message: expectedAlertMessage,
-          messageLinks: expectedAlertmessageLinks,
+          renderMessageHTML: true,
         });
       },
     );
diff --git a/spec/frontend/alert_spec.js b/spec/frontend/alert_spec.js
index 6a0d5843d9e0c..c8749f6639508 100644
--- a/spec/frontend/alert_spec.js
+++ b/spec/frontend/alert_spec.js
@@ -1,7 +1,11 @@
 import * as Sentry from '~/sentry/sentry_browser_wrapper';
 import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures';
+import { sanitize } from '~/lib/dompurify';
 import { createAlert, VARIANT_WARNING } from '~/alert';
 
+jest.mock('~/lib/dompurify', () => ({
+  sanitize: jest.fn((val) => val),
+}));
 jest.mock('~/sentry/sentry_browser_wrapper');
 
 describe('Flash', () => {
@@ -359,6 +363,36 @@ describe('Flash', () => {
           expect(messageLinks).toHaveLength(0);
         });
       });
+
+      describe('when rendered as HTML', () => {
+        const findMessageHTML = () => document.querySelector('.gl-alert-body div').innerHTML;
+        const message =
+          'error: <a href="https://documentation.com/further-information">learn more</a>';
+
+        it('renders the given message as HTML', () => {
+          alert = createAlert({
+            message,
+            renderMessageHTML: true,
+          });
+
+          expect(findMessageHTML()).toBe(message);
+        });
+
+        it('sanitizes the given message', () => {
+          expect(sanitize).not.toHaveBeenCalled();
+
+          createAlert({
+            message,
+            renderMessageHTML: true,
+          });
+
+          expect(sanitize).toHaveBeenCalledTimes(1);
+          expect(sanitize).toHaveBeenCalledWith(message, {
+            ALLOWED_TAGS: ['a'],
+            ALLOWED_ATTR: ['href', 'rel', 'target'],
+          });
+        });
+      });
     });
   });
 });
-- 
GitLab