Skip to content

Commit

Permalink
fix: >= 400 errors should 421 on SMTP, ensured that response property…
Browse files Browse the repository at this point in the history
… is set to NO to render login errors properly (instead of TEMPFAIL), updated tests for new quota feature
  • Loading branch information
titanism committed Aug 30, 2024
1 parent 4256656 commit d35a4b4
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 3 deletions.
2 changes: 1 addition & 1 deletion helpers/get-error-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function getErrorCode(err) {
if (err.isBoom === true && typeof err?.output?.statusCode === 'number') {
if ([403, 404].includes(err.output.statusCode)) return 550;

if (err.output.statusCode > 400 && err.output.statusCode < 500) return 421;
if (err.output.statusCode >= 400 && err.output.statusCode < 500) return 421;
}

return 550;
Expand Down
10 changes: 8 additions & 2 deletions helpers/on-auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -679,16 +679,22 @@ async function onAuth(auth, session, fn) {
} catch (err) {
//
// NOTE: if err.response === 'NO' then WildDuck POP3 will return error message too
// similarly if `error.response` is set then IMAP will return that instead of TEMPFAIL
//
// NOTE: we should actually share error message if it was not a code bug
// (otherwise it won't be intuitive to users if they're late on payment)
// (and we now do this via "ALERT" `imapResponse` code set in refineAndLogError
//
// <https://github.com/nodemailer/smtp-server/blob/a570d0164e4b4ef463eeedd80cadb37d5280e9da/lib/sasl.js#L189-L222>
// <https://github.com/nodemailer/wildduck/issues/726>
fn(
refineAndLogError(err, session, this.server instanceof IMAPServer, this)
const error = refineAndLogError(
err,
session,
this.server instanceof IMAPServer,
this
);
error.response = 'NO';
fn(error);
}
}

Expand Down
5 changes: 5 additions & 0 deletions test/api/v1.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ test('creates alias with global catch-all', async (t) => {
t.deepEqual(
_.sortBy(Object.keys(res.body)),
_.sortBy([
'max_quota',
'created_at',
'error_code_if_disabled',
'has_imap',
Expand Down Expand Up @@ -229,6 +230,7 @@ test('creates alias with global catch-all', async (t) => {
t.deepEqual(
_.sortBy(Object.keys(res.body.domain)),
_.sortBy([
'max_quota_per_alias',
'allowlist',
'denylist',
'invites',
Expand Down Expand Up @@ -300,6 +302,7 @@ test('creates alias with global catch-all', async (t) => {
'retention',
'name',
'is_enabled',
'max_quota',
'error_code_if_disabled',
'has_recipient_verification',
'recipients',
Expand Down Expand Up @@ -1280,10 +1283,12 @@ test('create domain without catchall', async (t) => {
t.is(res.body.length, 1);
t.true(res.body[0].name === 'testdomain1.com');
t.true(res.body[0].plan === 'enhanced_protection');

// filter for properties for exposed values
t.deepEqual(
_.sortBy(Object.keys(res.body[0])),
_.sortBy([
'max_quota_per_alias',
'allowlist',
'denylist',
'created_at',
Expand Down

0 comments on commit d35a4b4

Please sign in to comment.