Close the socket when authentication verification fails #262
Conversation
| @@ -65,7 +66,8 @@ internals.schema = Joi.object({ | |||
| ]), | |||
| index: Joi.boolean(), | |||
| timeout: Joi.number().integer().min(1).allow(false), | |||
| maxConnectionsPerUser: Joi.number().integer().min(1).allow(false).when('index', { is: true, otherwise: Joi.valid(false) }) | |||
| maxConnectionsPerUser: Joi.number().integer().min(1).allow(false).when('index', { is: true, otherwise: Joi.valid(false) }), | |||
| verify: Joi.number().integer() | |||
There was a problem hiding this comment.
@hueniverse you mentioned you'd like to see a min here? Aside from Joi trickery to achieve that (ref a key from another object) - is there really a need for that?
There was a problem hiding this comment.
The point of making sure this is not less than the heartbeat rate is to set expectations. If you want to verify the connection faster than the heartbeat check, it will still only happen as fast as messages are passing through or the heartbeat check.
Also, verify is not very descriptive. I prefer minAuthVerifyIntervalMsec.
| return true; | ||
| } | ||
|
|
||
| if (Date.now() - this.auth._verified < this._listener._settings.auth.verify) { |
There was a problem hiding this comment.
I'm not adding a way to disable this completely - should I? By e.g. setting verify: false?
There was a problem hiding this comment.
Sounds reasonable. false works for me.
| @@ -263,6 +264,11 @@ internals.Socket.prototype._lifecycle = function (request) { | |||
| throw Boom.badRequest('Message missing id'); | |||
| } | |||
|
|
|||
| if (!this._verifyAuth()) { | |||
There was a problem hiding this comment.
I only added this for received messages, because of the heartbeat. If the server receives no heartbeat - it will disconnect anyways. There is a chance of someone configuring a very long heartbeat timeout and keeping on pushing messages, but is that an issue?
|
This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions. |
Closes #260.