diff --git a/lib/api/filters.js b/lib/api/filters.js index 36d9b86c..6ddd9c49 100644 --- a/lib/api/filters.js +++ b/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(', ') ]; diff --git a/lib/tools.js b/lib/tools.js index 673be35c..3584c689 100644 --- a/lib/tools.js +++ b/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: [ diff --git a/test/api/filters-test.js b/test/api/filters-test.js index 1a1c0c73..8db1a18c 100644 --- a/test/api/filters-test.js +++ b/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; diff --git a/test/filtering-tools-test.js b/test/filtering-tools-test.js index 41cac304..1a18b5d9 100644 --- a/test/filtering-tools-test.js +++ b/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); + }); + }); + }); });