ZMS-28: filters, return full http forwarding target

lib/api/filters.js

@@ -3,7 +3,6 @@
 const log = require('npmlog');
 const Joi = require('joi');
 const ObjectId = require('mongodb').ObjectId;
-const urllib = require('url');
 const tools = require('../tools');
 const roles = require('../roles');
 const { nextPageCursorSchema, previousPageCursorSchema, sessSchema, sessIPSchema, booleanSchema, metaDataSchema } = require('../schemas');
@@ -173,7 +172,7 @@ module.exports = (db, server, userHandler, settingsHandler) => {
                 previousCursor: listingWrapper.previousCursor,
                 nextCursor: listingWrapper.nextCursor,
                 results: (listingWrapper.listing.results || []).map(filterData => {
-                    let descriptions = getFilterStrings(filterData, mailboxes);
+                    let descriptions = getFilterStrings(filterData, mailboxes, { preserveTargetUrls: true });
 
                     let values = {
                         id: filterData._id.toString(),
@@ -381,7 +380,7 @@ module.exports = (db, server, userHandler, settingsHandler) => {
                 },
 
                 results: filters.map(filterData => {
-                    let descriptions = getFilterStrings(filterData, mailboxes);
+                    let descriptions = getFilterStrings(filterData, mailboxes, { preserveTargetUrls: true });
 
                     const values = {
                         id: filterData._id.toString(),
@@ -1283,7 +1282,9 @@ module.exports = (db, server, userHandler, settingsHandler) => {
     );
 };
 
-function getFilterStrings(filter, mailboxes) {
+function getFilterStrings(filter, mailboxes, options) {
+    options = options || {};
+
     let query = Object.keys(filter.query.headers || {}).map(key => [key, '(' + filter.query.headers[key] + ')']);
 
     if (filter.query.ha && filter.query.ha > 0) {
@@ -1347,15 +1348,16 @@ function getFilterStrings(filter, mailboxes) {
                             'forward to',
                             filter.action[key]
                                 .map(target => {
-                                    switch (target.type) {
-                                        case 'http': {
-                                            let parsed = urllib.parse(target.value);
-                                            return parsed.hostname || parsed.host;
-                                        }
-
-                                        default:
+                                    if (target.type === 'http' && !options.preserveTargetUrls) {
+                                        try {
+                                            let parsed = new URL(target.value);
+                                            return parsed.hostname || parsed.host || target.value;
+                                        } catch (err) {
                                             return target.value;
+                                        }
                                     }
+
+                                    return target.value;
                                 })
                                 .join(', ')
                         ];

lib/tools.js

@@ -10,7 +10,6 @@ const fs = require('fs');
 const he = require('he');
 const pathlib = require('path');
 const crypto = require('crypto');
-const urllib = require('url');
 const net = require('net');
 const ipaddr = require('ipaddr.js');
 const ObjectId = require('mongodb').ObjectId;
@@ -385,22 +384,29 @@ function escapeRegexStr(string) {
 }
 
 function getRelayData(url) {
-    let urlparts = urllib.parse(url);
+    let urlparts;
+    try {
+        urlparts = new URL(url);
+    } catch {
+        urlparts = {};
+    }
+    let hostname = urlparts.hostname || '';
+    if (/^\[[^\]]+\]$/.test(hostname)) {
+        hostname = hostname.slice(1, -1);
+    }
     let targetMx = {
-        host: urlparts.hostname,
+        host: hostname,
         port: urlparts.port || 25,
-        auth: urlparts.auth
-            ? [urlparts.auth].map(auth => {
-                  let parts = auth.split(':');
-                  return {
-                      user: decodeURIComponent(parts[0] || ''),
-                      pass: decodeURIComponent(parts[1] || '')
-                  };
-              })[0]
-            : false,
+        auth:
+            urlparts.username || urlparts.password
+                ? {
+                      user: decodeURIComponent(urlparts.username || ''),
+                      pass: decodeURIComponent(urlparts.password || '')
+                  }
+                : false,
         secure: urlparts.protocol === 'smtps:',
-        A: [].concat(net.isIPv4(urlparts.hostname) ? urlparts.hostname : []),
-        AAAA: [].concat(net.isIPv6(urlparts.hostname) ? urlparts.hostname : [])
+        A: [].concat(net.isIPv4(hostname) ? hostname : []),
+        AAAA: [].concat(net.isIPv6(hostname) ? hostname : [])
     };
     let data = {
         mx: [

test/api/filters-test.js

@@ -207,6 +207,61 @@ describe('API Filters', function () {
         expect(responseGet.body.action.mailbox).to.be.equal(inbox);
     });
 
+    it('should preserve full HTTP target URL when returning filter listings', async () => {
+        const targetUrl = 'https://example.com/inbound/email?token=abc123';
+        const createResponse = await server
+            .post(`/users/${user}/filters`)
+            .send({
+                name: 'http target filter',
+                query: {
+                    from: 'http-target'
+                },
+                action: {
+                    targets: [targetUrl]
+                }
+            })
+            .expect(200);
+        expect(createResponse.body.success).to.be.true;
+
+        const filter = createResponse.body.id;
+
+        const authResponse = await server
+            .post('/authenticate')
+            .send({
+                username: 'filteruser',
+                password: 'secretvalue',
+                token: true
+            })
+            .expect(200);
+        expect(authResponse.body.success).to.be.true;
+
+        const userListResponse = await server.get(`/users/${user}/filters`).expect(200);
+        expect(userListResponse.body.success).to.be.true;
+
+        const userListFilter = userListResponse.body.results.find(entry => entry.id === filter);
+        expect(userListFilter).to.exist;
+        expect(userListFilter.action).to.deep.equal([['forward to', targetUrl]]);
+        expect(userListFilter.originalAction.targets).to.deep.equal([targetUrl]);
+
+        const ownFiltersResponse = await server.get(`/filters?accessToken=${authResponse.body.token}`).expect(200);
+        expect(ownFiltersResponse.body.success).to.be.true;
+
+        const ownFiltersEntry = ownFiltersResponse.body.results.find(entry => entry.id === filter);
+        expect(ownFiltersEntry).to.exist;
+        expect(ownFiltersEntry.action).to.deep.equal([['forward to', targetUrl]]);
+        expect(ownFiltersEntry.originalAction.targets).to.deep.equal([targetUrl]);
+        expect(ownFiltersEntry.targets).to.deep.equal([targetUrl]);
+
+        const allFiltersResponse = await server.get(`/filters`).expect(200);
+        expect(allFiltersResponse.body.success).to.be.true;
+
+        const allFiltersEntry = allFiltersResponse.body.results.find(entry => entry.id === filter);
+        expect(allFiltersEntry).to.exist;
+        expect(allFiltersEntry.action).to.deep.equal([['forward to', targetUrl]]);
+        expect(allFiltersEntry.originalAction.targets).to.deep.equal([targetUrl]);
+        expect(allFiltersEntry.targets).to.deep.equal([targetUrl]);
+    });
+
     describe('Filter spam action', function () {
         let spamFilter;
 

test/filtering-tools-test.js

@@ -2,7 +2,7 @@
 'use strict';
 
 const { expect } = require('chai');
-const { extractQuotedPhrases, parseFilterQueryText, filterQueryTermMatches } = require('../lib/tools');
+const { extractQuotedPhrases, parseFilterQueryText, filterQueryTermMatches, getRelayData } = require('../lib/tools');
 
 describe('Email Filtering helper functions', () => {
     describe('extractQuotedPhrases', () => {
@@ -371,4 +371,95 @@ describe('Email Filtering helper functions', () => {
             expect(parsed.exactPhrases).to.deep.equal(['first phrase', 'second phrase']);
         });
     });
+
+    describe('getRelayData', () => {
+        [
+            {
+                title: 'should return expected structure for a hostname relay without auth',
+                input: 'smtp://mx.example.com',
+                expected: {
+                    mx: [
+                        {
+                            priority: 0,
+                            mx: true,
+                            exchange: 'mx.example.com',
+                            A: [],
+                            AAAA: []
+                        }
+                    ],
+                    mxPort: 25,
+                    mxAuth: false,
+                    mxSecure: false,
+                    url: 'smtp://mx.example.com'
+                }
+            },
+            {
+                title: 'should return expected structure for a hostname relay with auth and port',
+                input: 'smtp://user:pass@mx.example.com:2525',
+                expected: {
+                    mx: [
+                        {
+                            priority: 0,
+                            mx: true,
+                            exchange: 'mx.example.com',
+                            A: [],
+                            AAAA: []
+                        }
+                    ],
+                    mxPort: '2525',
+                    mxAuth: {
+                        user: 'user',
+                        pass: 'pass'
+                    },
+                    mxSecure: false,
+                    url: 'smtp://user:pass@mx.example.com:2525'
+                }
+            },
+            {
+                title: 'should return expected structure for an IPv4 relay',
+                input: 'smtp://192.0.2.15:25',
+                expected: {
+                    mx: [
+                        {
+                            priority: 0,
+                            mx: true,
+                            exchange: '192.0.2.15',
+                            A: ['192.0.2.15'],
+                            AAAA: []
+                        }
+                    ],
+                    mxPort: '25',
+                    mxAuth: false,
+                    mxSecure: false,
+                    url: 'smtp://192.0.2.15:25'
+                }
+            },
+            {
+                title: 'should return expected structure for an IPv6 relay',
+                input: 'smtps://user:p%40ss@[2001:db8::1]:465',
+                expected: {
+                    mx: [
+                        {
+                            priority: 0,
+                            mx: true,
+                            exchange: '2001:db8::1',
+                            A: [],
+                            AAAA: ['2001:db8::1']
+                        }
+                    ],
+                    mxPort: '465',
+                    mxAuth: {
+                        user: 'user',
+                        pass: 'p@ss'
+                    },
+                    mxSecure: true,
+                    url: 'smtps://user:p%40ss@[2001:db8::1]:465'
+                }
+            }
+        ].forEach(testCase => {
+            it(testCase.title, () => {
+                expect(getRelayData(testCase.input)).to.deep.equal(testCase.expected);
+            });
+        });
+    });
 });