nodemailer / smtp-server

Create custom SMTP servers on the fly
Other
862 stars 146 forks source link

authOptional and maxAllowedUnauthenticatedCommands #161

Open clizSec opened 3 years ago

clizSec commented 3 years ago

When using AUTH as optional using authOptional: true flag, the default of maxAllowedUnauthenticatedCommands cannot be changed, as it expects the value to be a number. maxAllowedUnauthenticatedCommands: false cannot be set on maxAllowedUnauthenticatedCommands as it will take the default, which is 10.

// Max allowed unauthenticated commands
this._maxAllowedUnauthenticatedCommands = this._server.options.maxAllowedUnauthenticatedCommands || 10;

cannot be false

I fixed on my end, by using maxAllowedUnauthenticatedCommands: Infinity, but the better way of it being handled, is by adding sanity to the condition below, to check for the authOptional: true flag which was set at the initinal options.


Before

// block users that try to fiddle around without logging in
if (!this.session.user && this._isSupported('AUTH') && commandName !== 'AUTH' && this._maxAllowedUnauthenticatedCommands !== false) {
    this._unauthenticatedCommands++;
    if (this._unauthenticatedCommands >= this._maxAllowedUnauthenticatedCommands) {
        return this.send(421, 'Error: too many unauthenticated commands');
    }
}

After

// block users that try to fiddle around without logging in
if (!this.session.user && this._isSupported('AUTH') && !this._server.options.authOptional && commandName !== 'AUTH' && this._maxAllowedUnauthenticatedCommands !== false) {
    this._unauthenticatedCommands++;
    if (this._unauthenticatedCommands >= this._maxAllowedUnauthenticatedCommands) {
        return this.send(421, 'Error: too many unauthenticated commands');
    }
}

Added !this._server.options.authOptional


What do you think?

andris9 commented 3 years ago

Seems reasonable, can you make a PR?