Skip to content

Commit

Permalink
Add support for blocklisting e-mail domains.
Browse files Browse the repository at this point in the history
E-mails in the domain blocklist are disallowed on the admin UI, public
subscription forms, API, and in the bulk importer.

- Add blocklist setting that takes a list of multi-line domains on the
  Settings -> Privacy UI.
- Refactor e-mail validation in subimporter to add blocklist checking
  centrally.
- Add Cypress testr testing domain blocklist behaviour on admin
  and non-admin views.

Closes #336.
  • Loading branch information
knadh committed Sep 25, 2021
1 parent 9f3eb7e commit 7aee36e
Show file tree
Hide file tree
Showing 28 changed files with 569 additions and 449 deletions.
30 changes: 16 additions & 14 deletions cmd/bounce.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@ import (
"io/ioutil"
"net/http"
"strconv"
"strings"
"time"

"github.com/knadh/listmonk/internal/subimporter"
"github.com/knadh/listmonk/models"
"github.com/labstack/echo"
"github.com/lib/pq"
Expand Down Expand Up @@ -164,12 +162,12 @@ func handleBounceWebhook(c echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.Ts("globals.messages.invalidData"))
}

if err := validateBounceFields(b, app); err != nil {
if bv, err := validateBounceFields(b, app); err != nil {
return err
} else {
b = bv
}

b.Email = strings.ToLower(b.Email)

if len(b.Meta) == 0 {
b.Meta = json.RawMessage("{}")
}
Expand Down Expand Up @@ -234,22 +232,26 @@ func handleBounceWebhook(c echo.Context) error {
return c.JSON(http.StatusOK, okResp{true})
}

func validateBounceFields(b models.Bounce, app *App) error {
func validateBounceFields(b models.Bounce, app *App) (models.Bounce, error) {
if b.Email == "" && b.SubscriberUUID == "" {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidData"))
return b, echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidData"))
}

if b.Type != models.BounceTypeHard && b.Type != models.BounceTypeSoft {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidData"))
if b.SubscriberUUID != "" && !reUUID.MatchString(b.SubscriberUUID) {
return b, echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidUUID"))
}

if b.Email != "" && !subimporter.IsEmail(b.Email) {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidEmail"))
if b.Email != "" {
em, err := app.importer.SanitizeEmail(b.Email)
if err != nil {
return b, echo.NewHTTPError(http.StatusBadRequest, err.Error())
}
b.Email = em
}

if b.SubscriberUUID != "" && !reUUID.MatchString(b.SubscriberUUID) {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidUUID"))
if b.Type != models.BounceTypeHard && b.Type != models.BounceTypeSoft {
return b, echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidData"))
}

return nil
return b, nil
}
3 changes: 1 addition & 2 deletions cmd/campaigns.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (

"github.com/gofrs/uuid"
"github.com/jmoiron/sqlx"
"github.com/knadh/listmonk/internal/subimporter"
"github.com/knadh/listmonk/models"
"github.com/labstack/echo"
"github.com/lib/pq"
Expand Down Expand Up @@ -687,7 +686,7 @@ func validateCampaignFields(c campaignReq, app *App) (campaignReq, error) {
if c.FromEmail == "" {
c.FromEmail = app.constants.FromEmail
} else if !regexFromAddress.Match([]byte(c.FromEmail)) {
if !subimporter.IsEmail(c.FromEmail) {
if _, err := app.importer.SanitizeEmail(c.FromEmail); err != nil {
return c, errors.New(app.i18n.T("campaigns.fieldInvalidFromEmail"))
}
}
Expand Down
5 changes: 4 additions & 1 deletion cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ type constants struct {
AllowExport bool `koanf:"allow_export"`
AllowWipe bool `koanf:"allow_wipe"`
Exportable map[string]bool `koanf:"-"`
DomainBlocklist map[string]bool `koanf:"-"`
} `koanf:"privacy"`
AdminUsername []byte `koanf:"admin_username"`
AdminPassword []byte `koanf:"admin_password"`
Expand Down Expand Up @@ -291,6 +292,7 @@ func initConstants() *constants {
c.Lang = ko.String("app.lang")
c.Privacy.Exportable = maps.StringSliceToLookupMap(ko.Strings("privacy.exportable"))
c.MediaProvider = ko.String("upload.provider")
c.Privacy.DomainBlocklist = maps.StringSliceToLookupMap(ko.Strings("privacy.domain_blocklist"))

// Static URLS.
// url.com/subscription/{campaign_uuid}/{subscriber_uuid}
Expand Down Expand Up @@ -366,14 +368,15 @@ func initCampaignManager(q *Queries, cs *constants, app *App) *manager.Manager {
func initImporter(q *Queries, db *sqlx.DB, app *App) *subimporter.Importer {
return subimporter.New(
subimporter.Options{
DomainBlocklist: app.constants.Privacy.DomainBlocklist,
UpsertStmt: q.UpsertSubscriber.Stmt,
BlocklistStmt: q.UpsertBlocklistSubscriber.Stmt,
UpdateListDateStmt: q.UpdateListsDate.Stmt,
NotifCB: func(subject string, data interface{}) error {
app.sendNotification(app.constants.NotifyEmails, subject, notifTplImport, data)
return nil
},
}, db.DB)
}, db.DB, app.i18n)
}

// initSMTPMessenger initializes the SMTP messenger.
Expand Down
6 changes: 4 additions & 2 deletions cmd/public.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,15 +318,17 @@ func handleSubscriptionForm(c echo.Context) error {
}

// If there's no name, use the name bit from the e-mail.
req.Email = strings.ToLower(req.Email)
req.Name = strings.TrimSpace(req.Name)
if req.Name == "" {
req.Name = strings.Split(req.Email, "@")[0]
}

// Validate fields.
if err := subimporter.ValidateFields(req.SubReq); err != nil {
if r, err := app.importer.ValidateFields(req.SubReq); err != nil {
return c.Render(http.StatusInternalServerError, tplMessage,
makeMsgTpl(app.i18n.T("public.errorTitle"), "", err.Error()))
} else {
req.SubReq = r
}

// Insert the subscriber into the DB.
Expand Down
11 changes: 11 additions & 0 deletions cmd/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type settings struct {
PrivacyAllowExport bool `json:"privacy.allow_export"`
PrivacyAllowWipe bool `json:"privacy.allow_wipe"`
PrivacyExportable []string `json:"privacy.exportable"`
DomainBlocklist []string `json:"privacy.domain_blocklist"`

UploadProvider string `json:"upload.provider"`
UploadFilesystemUploadPath string `json:"upload.filesystem.upload_path"`
Expand Down Expand Up @@ -246,6 +247,16 @@ func handleUpdateSettings(c echo.Context) error {
set.SendgridKey = cur.SendgridKey
}

// Domain blocklist.
doms := make([]string, 0)
for _, d := range set.DomainBlocklist {
d = strings.TrimSpace(strings.ToLower(d))
if d != "" {
doms = append(doms, d)
}
}
set.DomainBlocklist = doms

// Marshal settings.
b, err := json.Marshal(set)
if err != nil {
Expand Down
15 changes: 11 additions & 4 deletions cmd/subscribers.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,12 @@ func handleCreateSubscriber(c echo.Context) error {
if err := c.Bind(&req); err != nil {
return err
}
req.Email = strings.ToLower(strings.TrimSpace(req.Email))
if err := subimporter.ValidateFields(req); err != nil {

r, err := app.importer.ValidateFields(req)
if err != nil {
return echo.NewHTTPError(http.StatusBadRequest, err.Error())
} else {
req = r
}

// Insert the subscriber into the DB.
Expand Down Expand Up @@ -325,9 +328,13 @@ func handleUpdateSubscriber(c echo.Context) error {
if id < 1 {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidID"))
}
if req.Email != "" && !subimporter.IsEmail(req.Email) {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("subscribers.invalidEmail"))

if em, err := app.importer.SanitizeEmail(req.Email); err != nil {
return echo.NewHTTPError(http.StatusBadRequest, err.Error())
} else {
req.Email = em
}

if req.Name != "" && !strHasLen(req.Name, 1, stdInputMaxLen) {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("subscribers.invalidName"))
}
Expand Down
5 changes: 5 additions & 0 deletions frontend/cypress/fixtures/subs-domain-blocklist.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
email,name,attributes
[email protected],First0 Last0,"{""age"": 29, ""city"": ""Bangalore"", ""clientId"": ""DAXX79""}"
[email protected],First1 Last1,"{""age"": 43, ""city"": ""Bangalore"", ""clientId"": ""DAXX71""}"
[email protected],First2 Last2,"{""age"": 47, ""city"": ""Bangalore"", ""clientId"": ""DAXX70""}"
[email protected],First1 Last1,"{""age"": 43, ""city"": ""Bangalore"", ""clientId"": ""DAXX71""}"
114 changes: 109 additions & 5 deletions frontend/cypress/integration/subscribers.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const apiUrl = Cypress.env('apiUrl');

describe('Subscribers', () => {
it('Opens subscribers page', () => {
cy.resetDB();
Expand Down Expand Up @@ -43,6 +45,7 @@ describe('Subscribers', () => {
});

cy.get('[data-cy=btn-query-reset]').click();
cy.wait(1000);
cy.get('tbody td[data-label=Status]').its('length').should('eq', 2);
});

Expand All @@ -55,7 +58,7 @@ describe('Subscribers', () => {
{ radio: 'check-list-remove', lists: [0, 1], rows: { 1: [] } },
{ radio: 'check-list-add', lists: [0, 1], rows: { 0: ['unsubscribed', 'unsubscribed'], 1: ['unconfirmed', 'unconfirmed'] } },
{ radio: 'check-list-remove', lists: [0], rows: { 0: ['unsubscribed'] } },
{ radio: 'check-list-add', lists: [0], rows: { 0: ['unconfirmed', 'unsubscribed'] } },
{ radio: 'check-list-add', lists: [0], rows: { 0: ['unsubscribed', 'unconfirmed'] } },
];


Expand Down Expand Up @@ -109,7 +112,7 @@ describe('Subscribers', () => {

// Open the edit popup and edit the default lists.
cy.get('[data-cy=btn-edit]').each(($el, n) => {
const email = `email-${n}@email.com`;
const email = `email-${n}@EMAIL.com`;
const name = `name-${n}`;

// Open the edit modal.
Expand All @@ -136,7 +139,7 @@ describe('Subscribers', () => {
cy.wait(250);
cy.get('tbody tr').each(($el) => {
cy.wrap($el).find('td[data-id]').invoke('attr', 'data-id').then((id) => {
cy.wrap($el).find('td[data-label=E-mail]').contains(rows[id].email);
cy.wrap($el).find('td[data-label=E-mail]').contains(rows[id].email.toLowerCase());
cy.wrap($el).find('td[data-label=Name]').contains(rows[id].name);
cy.wrap($el).find('td[data-label=Status]').contains(rows[id].status, { matchCase: false });

Expand Down Expand Up @@ -171,7 +174,7 @@ describe('Subscribers', () => {
// Cycle through each status and each list ID combination and create subscribers.
const n = 0;
for (let n = 0; n < 6; n++) {
const email = `email-${n}@email.com`;
const email = `email-${n}@EMAIL.com`;
const name = `name-${n}`;
const status = statuses[(n + 1) % statuses.length];
const list = lists[(n + 1) % lists.length];
Expand All @@ -192,7 +195,7 @@ describe('Subscribers', () => {
// which is always the first row in the table.
cy.wait(250);
const tr = cy.get('tbody tr:nth-child(1)').then(($el) => {
cy.wrap($el).find('td[data-label=E-mail]').contains(email);
cy.wrap($el).find('td[data-label=E-mail]').contains(email.toLowerCase());
cy.wrap($el).find('td[data-label=Name]').contains(name);
cy.wrap($el).find('td[data-label=Status]').contains(status, { matchCase: false });
cy.wrap($el).find(`.tags .${status === 'enabled' ? 'unconfirmed' : 'unsubscribed'}`)
Expand All @@ -217,3 +220,104 @@ describe('Subscribers', () => {
});
});
});


describe('Domain blocklist', () => {
it('Opens settings page', () => {
cy.resetDB();
});

it('Add domains to blocklist', () => {
cy.loginAndVisit('/settings');
cy.get('.b-tabs nav a').eq(2).click();
cy.get('textarea[name="privacy.domain_blocklist"]').clear().type('ban.net\n\nBaN.OrG\n\nban.com\n\n');
cy.get('[data-cy=btn-save]').click();
});

it('Try subscribing via public page', () => {
cy.visit(`${apiUrl}/subscription/form`);
cy.get('input[name=email]').clear().type('[email protected]');
cy.get('button[type=submit]').click();
cy.get('h2').contains('Subscribe');

cy.visit(`${apiUrl}/subscription/form`);
cy.get('input[name=email]').clear().type('[email protected]');
cy.get('button[type=submit]').click();
cy.get('h2').contains('Error');
});


// Post to the admin API.
it('Try via admin API', () => {
cy.wait(1000);

// Add non-banned domain.
cy.request({
method: 'POST', url: `${apiUrl}/api/subscribers`, failOnStatusCode: true,
body: { email: '[email protected]', 'name': 'test', 'lists': [1], 'status': 'enabled' }
}).should((response) => {
expect(response.status).to.equal(200);
});

// Add banned domain.
cy.request({
method: 'POST', url: `${apiUrl}/api/subscribers`, failOnStatusCode: false,
body: { email: '[email protected]', 'name': 'test', 'lists': [1], 'status': 'enabled' }
}).should((response) => {
expect(response.status).to.equal(400);
});

// Modify an existinb subscriber to a banned domain.
cy.request({
method: 'PUT', url: `${apiUrl}/api/subscribers/1`, failOnStatusCode: false,
body: { email: '[email protected]', 'name': 'test', 'lists': [1], 'status': 'enabled' }
}).should((response) => {
expect(response.status).to.equal(400);
});
});

it('Try via import', () => {
cy.loginAndVisit('/subscribers/import');
cy.get('.list-selector input').click();
cy.get('.list-selector .autocomplete a').first().click();

cy.fixture('subs-domain-blocklist.csv').then((data) => {
cy.get('input[type="file"]').attachFile({
fileContent: data.toString(),
fileName: 'subs.csv',
mimeType: 'text/csv',
});
});

cy.get('button.is-primary').click();
cy.get('section.wrap .has-text-success');
// cy.get('button.is-primary').click();
cy.get('.log-view').should('contain', '[email protected]').and('contain', '[email protected]');
cy.wait(100);
});

it('Clear blocklist and try', () => {
cy.loginAndVisit('/settings');
cy.get('.b-tabs nav a').eq(2).click();
cy.get('textarea[name="privacy.domain_blocklist"]').clear();
cy.get('[data-cy=btn-save]').click();
cy.wait(1000);

// Add banned domain.
cy.request({
method: 'POST', url: `${apiUrl}/api/subscribers`, failOnStatusCode: true,
body: { email: '[email protected]', 'name': 'test', 'lists': [1], 'status': 'enabled' }
}).should((response) => {
expect(response.status).to.equal(200);
});

// Modify an existinb subscriber to a banned domain.
cy.request({
method: 'PUT', url: `${apiUrl}/api/subscribers/1`, failOnStatusCode: true,
body: { email: '[email protected]', 'name': 'test', 'lists': [1], 'status': 'enabled' }
}).should((response) => {
expect(response.status).to.equal(200);
});
});

})
2 changes: 1 addition & 1 deletion frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"@vue/cli-service": "~4.5.13",
"@vue/eslint-config-airbnb": "^5.3.0",
"babel-eslint": "^10.1.0",
"cypress": "^6.4.0",
"cypress": "8.4.1",
"cypress-file-upload": "^5.0.2",
"eslint": "^7.27.0",
"eslint-plugin-import": "^2.23.3",
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/views/Settings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ export default Vue.extend({
}
}
// Domain blocklist array from multi-line strings.
form['privacy.domain_blocklist'] = form['privacy.domain_blocklist'].split('\n').map((v) => v.trim().toLowerCase()).filter((v) => v !== '');
this.isLoading = true;
this.$api.updateSettings(form).then((data) => {
if (data.needsRestart) {
Expand Down Expand Up @@ -192,6 +195,9 @@ export default Vue.extend({
}
d['bounce.sendgrid_key'] = dummyPassword;
// Domain blocklist array to multi-line string.
d['privacy.domain_blocklist'] = d['privacy.domain_blocklist'].join('\n');
this.key += 1;
this.form = d;
this.formCopy = JSON.stringify(d);
Expand Down
Loading

0 comments on commit 7aee36e

Please sign in to comment.