MM-30090 Open managed resource links in a new tab (mattermost#7024)

* MM-30090 Open managed resource links in a new tab * MM-30090 Add ManagedResourcePaths setting * Update test case to reflect new case * Fix types * Revert unnecessary type changes * Switch mattermost-redux back to master
seongwon-kang · Nov 16, 2020 · 6413890 · 6413890
1 parent 31a05b1
commit 6413890
Show file tree

Hide file tree

Showing 9 changed files with 102 additions and 22 deletions.
diff --git a/components/admin_console/admin_definition.jsx b/components/admin_console/admin_definition.jsx
@@ -703,6 +703,16 @@ const AdminDefinition = {
                         help_text_default: 'When true, any outgoing HTTPS requests will accept unverified, self-signed certificates. For example, outgoing webhooks to a server with a self-signed TLS certificate, using any domain, will be allowed. Note that this makes these connections susceptible to man-in-the-middle attacks.',
                         isDisabled: it.not(it.userHasWritePermissionOnResource('environment')),
                     },
+                    {
+                        type: Constants.SettingsTypes.TYPE_TEXT,
+                        key: 'ServiceSettings.ManagedResourcePaths',
+                        label: t('admin.service.managedResourcePaths'),
+                        label_default: 'Managed Resource Paths:',
+                        help_text: t('admin.service.managedResourcePathsDescription'),
+                        help_text_default: 'A comma-separated list of paths on the Mattermost server that are managed by another service. See [here](!https://docs.mattermost.com/install/desktop-managed-resources.html) for more information.',
+                        help_text_markdown: true,
+                        isDisabled: it.not(it.userHasWritePermissionOnResource('environment')),
+                    },
                     {
                         type: Constants.SettingsTypes.TYPE_BUTTON,
                         action: reloadConfig,

diff --git a/components/markdown/index.ts b/components/markdown/index.ts
@@ -6,7 +6,7 @@ import {createSelector} from 'reselect';
 
 import {Preferences} from 'mattermost-redux/constants';
 import {getChannelsNameMapInCurrentTeam} from 'mattermost-redux/selectors/entities/channels';
-import {getAutolinkedUrlSchemes, getConfig} from 'mattermost-redux/selectors/entities/general';
+import {getAutolinkedUrlSchemes, getConfig, getManagedResourcePaths} from 'mattermost-redux/selectors/entities/general';
 import {getBool} from 'mattermost-redux/selectors/entities/preferences';
 import {getCurrentTeam} from 'mattermost-redux/selectors/entities/teams';
 import {getAllUserMentionKeys} from 'mattermost-redux/selectors/entities/search';
@@ -48,6 +48,7 @@ function makeMapStateToProps() {
             autolinkedUrlSchemes: getAutolinkedUrlSchemes(state),
             channelNamesMap: getChannelNamesMap(state, ownProps),
             enableFormatting: getBool(state, Preferences.CATEGORY_ADVANCED_SETTINGS, 'formatting', true),
+            managedResourcePaths: getManagedResourcePaths(state),
             mentionKeys: ownProps.mentionKeys || getAllUserMentionKeys(state),
             siteURL: getSiteURL(),
             team: getCurrentTeam(state),

diff --git a/components/markdown/markdown.tsx b/components/markdown/markdown.tsx
@@ -34,6 +34,11 @@ type Props = {
      */
     isRHS?: boolean;
 
+    /*
+     * An array of paths on the server that are managed by another server
+     */
+    managedResourcePaths?: string[];
+
     /*
      * An array of words that can be used to mention a user
      */
@@ -47,7 +52,7 @@ type Props = {
     /*
      * Any additional text formatting options to be used
      */
-    options: TextFormattingOptions;
+    options: Partial<TextFormattingOptions>;
 
     /*
      * The root Site URL for the page
@@ -125,6 +130,7 @@ export default class Markdown extends React.PureComponent<Props> {
             proxyImages: this.props.hasImageProxy && this.props.proxyImages,
             team: this.props.team,
             minimumHashtagLength: this.props.minimumHashtagLength,
+            managedResourcePaths: this.props.managedResourcePaths,
         }, this.props.options);
 
         const htmlFormattedText = formatText(this.props.message, options, this.props.emojiMap);

diff --git a/i18n/en.json b/i18n/en.json
@@ -1677,6 +1677,8 @@
   "admin.service.listenAddress": "Listen Address:",
   "admin.service.listenDescription": "The address and port to which to bind and listen. Specifying \":8065\" will bind to all network interfaces. Specifying \"127.0.0.1:8065\" will only bind to the network interface having that IP address. If you choose a port of a lower level (called \"system ports\" or \"well-known ports\", in the range of 0-1023), you must have permissions to bind to that port. On Linux you can use: \"sudo setcap cap_net_bind_service=+ep ./bin/mattermost\" to allow Mattermost to bind to well-known ports.",
   "admin.service.listenExample": "E.g.: \":8065\"",
+  "admin.service.managedResourcePaths": "Managed Resource Paths:",
+  "admin.service.managedResourcePathsDescription": "A comma-separated list of paths on the Mattermost server that are managed by another service. See [here](!https://docs.mattermost.com/install/desktop-managed-resources.html) for more information.",
   "admin.service.mfaDesc": "When true, users with AD/LDAP or email login can add multi-factor authentication to their account using Google Authenticator.",
   "admin.service.mfaTitle": "Enable Multi-factor Authentication:",
   "admin.service.minimumHashtagLengthDescription": "Minimum number of characters in a hashtag. This must be greater than or equal to 2. MySQL databases must be configured to support searching strings shorter than three characters, [see documentation](!https://dev.mysql.com/doc/refman/8.0/en/fulltext-fine-tuning.html).",

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -37,7 +37,7 @@
     "localforage-observable": "2.0.1",
     "mark.js": "8.11.1",
     "marked": "github:mattermost/marked#87769262aa02e1784570f61f4f962050e07cc335",
-    "mattermost-redux": "github:mattermost/mattermost-redux#aba4df6b300ad4aaf8e0adb734ff549bf236d110",
+    "mattermost-redux": "github:mattermost/mattermost-redux#5aa938486c49d76f55953dca7890fb6f4ad601b7",
     "moment-timezone": "0.5.31",
     "p-queue": "6.6.1",
     "pdfjs-dist": "2.1.266",

diff --git a/utils/markdown/index.test.js b/utils/markdown/index.test.js
@@ -169,4 +169,48 @@ this is long text this is long text this is long text this is long text this is
         const plugin = format('[plugin](/reiciendis-0/plugins/example))', {siteURL: 'http://localhost'});
         expect(plugin).toContain('<a class="theme markdown__link" href="/reiciendis-0/plugins/example" rel="noreferrer" data-link="/reiciendis-0/plugins/example">plugin</a>');
     });
+
+    describe('should correctly open links in the current tab based on whether they are handled by the web app', () => {
+        for (const testCase of [
+            {name: 'regular link', link: 'https://example.com', handled: false},
+            {name: 'www link', link: 'www.example.com', handled: false},
+
+            {name: 'link to a channel', link: 'http://localhost/team/channels/foo', handled: true},
+            {name: 'link to a DM', link: 'http://localhost/team/messages/@bar', handled: true},
+            {name: 'link to the system console', link: 'http://localhost/admin_console', handled: true},
+            {name: 'permalink', link: 'http://localhost/reiciendis-0/pl/b3hrs3brjjn7fk4kge3xmeuffc', handled: true},
+            {name: 'link to a specific system console page', link: 'http://localhost/admin_console/plugins/plugin_com.github.matterpoll.matterpoll', handled: true},
+
+            {name: 'relative link', link: '/', handled: true},
+            {name: 'relative link to a channel', link: '/reiciendis-0/channels/b3hrs3brjjn7fk4kge3xmeuffc', handled: true},
+            {name: 'relative link to a DM', link: '/reiciendis-0/messages/b3hrs3brjjn7fk4kge3xmeuffc', handled: true},
+            {name: 'relative permalink', link: '/reiciendis-0/pl/b3hrs3brjjn7fk4kge3xmeuffc', handled: true},
+            {name: 'relative link to the system console', link: '/admin_console', handled: true},
+            {name: 'relative link to a specific system console page', link: '/admin_console/plugins/plugin_com.github.matterpoll.matterpoll', handled: true},
+
+            {name: 'link to a plugin-handled path', link: 'http://localhost/plugins/example', handled: false},
+            {name: 'link to a file attachment public link', link: 'http://localhost/files/o6eujqkmjfd138ykpzmsmc131y/public?h=j5nPX8JlgUeNVMOB3dLXwyG_jlxlSw4nSgZmegXfpHw', handled: false},
+
+            {name: 'relative link to a plugin-handled path', link: '/plugins/example', handled: false},
+            {name: 'relative link to a file attachment public link', link: '/files/o6eujqkmjfd138ykpzmsmc131y/public?h=j5nPX8JlgUeNVMOB3dLXwyG_jlxlSw4nSgZmegXfpHw', handled: false},
+
+            {name: 'link to a managed resource', link: 'http://localhost/trusted/jitsi', options: {managedResourcePaths: ['trusted']}, handled: false},
+            {name: 'relative link to a managed resource', link: '/trusted/jitsi', options: {managedResourcePaths: ['trusted']}, handled: false},
+            {name: 'link that is not to a managed resource', link: 'http://localhost/trusted/jitsi', options: {managedResourcePaths: ['jitsi']}, handled: true},
+        ]) {
+            test(testCase.name, () => {
+                const options = {
+                    siteURL: 'http://localhost',
+                    ...testCase.options,
+                };
+                const output = format(`[link](${testCase.link})`, options);
+
+                if (testCase.handled) {
+                    expect(output).not.toContain('target="_blank"');
+                } else {
+                    expect(output).toContain('rel="noreferrer" target="_blank"');
+                }
+            });
+        }
+    });
 });
diff --git a/utils/markdown/renderer.tsx b/utils/markdown/renderer.tsx
@@ -14,7 +14,7 @@ export default class Renderer extends marked.Renderer {
     private emojiMap: EmojiMap;
     public constructor(
         options: MarkedOptions,
-        formattingOptions = {},
+        formattingOptions: TextFormatting.TextFormattingOptions,
         emojiMap = new EmojiMap(new Map()),
     ) {
         super(options);
@@ -207,29 +207,38 @@ export default class Renderer extends marked.Renderer {
 
         output += `" href="${outHref}" rel="noreferrer"`;
 
-        const pluginURL = `${this.formattingOptions.siteURL}/plugins`;
-        const fileURL = `${this.formattingOptions.siteURL}/files`;
+        const isInternalLink = outHref.startsWith(this.formattingOptions.siteURL || '') || outHref.startsWith('/');
 
-        // Any link that begins with siteURL should be opened inside the app, except when rooted
-        // at /plugins, which is logically "outside the app" despite being hosted by a plugin,
-        // or /files, which should be launched "outside the app".
-        let internalLink = outHref.startsWith(this.formattingOptions.siteURL || '') && !outHref.startsWith(pluginURL) && !outHref.startsWith(fileURL);
+        let openInNewTab;
+        if (isInternalLink) {
+            const path = outHref.startsWith('/') ? outHref : outHref.substring(this.formattingOptions.siteURL?.length || 0);
 
-        // special case for team invite links, channel links, and permalinks that are inside the app
-        const pattern = new RegExp(
-            '^(' +
-            TextFormatting.escapeRegex(this.formattingOptions.siteURL) +
-            ')?\\/(?:signup_user_complete|admin_console|[^\\/]+\\/(?:pl|channels|messages|plugins))\\/',
-        );
-        internalLink = internalLink || pattern.test(outHref);
+            // Paths managed by plugins and public file links aren't handled by the web app
+            const unhandledPaths = [
+                'plugins',
+                'files',
+            ];
+
+            // Paths managed by another service shouldn't be handled by the web app either
+            if (this.formattingOptions.managedResourcePaths) {
+                for (const managedPath of this.formattingOptions.managedResourcePaths) {
+                    unhandledPaths.push(TextFormatting.escapeRegex(managedPath));
+                }
+            }
 
-        if (internalLink && this.formattingOptions.siteURL) {
+            openInNewTab = unhandledPaths.some((unhandledPath) => new RegExp('^/' + unhandledPath + '\\b').test(path));
+        } else {
+            // All links outside of Mattermost should be opened in a new tab
+            openInNewTab = true;
+        }
+
+        if (openInNewTab || !this.formattingOptions.siteURL) {
+            output += ' target="_blank"';
+        } else {
             output += ` data-link="${outHref.replace(
                 this.formattingOptions.siteURL,
                 '',
             )}"`;
-        } else {
-            output += ' target="_blank"';
         }
 
         if (title) {

diff --git a/utils/text_formatting.tsx b/utils/text_formatting.tsx
@@ -156,6 +156,14 @@ interface TextFormattingOptionsBase {
    */
     autolinkedUrlSchemes: string[];
 
+    /*
+     * An array of paths on the server that are managed by another server. Any path provided will be treated as an
+     * external link that will not by handled by react-router.
+     *
+     * Defaults to an empty array.
+     */
+    managedResourcePaths: string[];
+
     /**
    * A custom renderer object to use in the formatWithRenderer function.
    *