Adding validations for body of request.

[Aryaman1706]

Feature Request

• Adding validation for body/query params of express:Request.

Advantages

• Surety of client-side data we are working with.
• Prevention from various types of attacks including cross-site scripting, command injection, etc

Implementation

• I would like to work on this issue. Can you please assign this issue to me?
• There are a number of npm libraries to implement this Joi, Yup, express-validatior, validatorjs etc.

Additional Context

• OWASP - NodeJs Security
• OWASP - Input Validation Guidelines

[anandbaburajan]

Hey @Aryaman1706! Thanks for opening this issue! We had thought about validation but didn't think much on the security aspect. Can you check out my comments at RocketMeet/RocketMeet-client#159 too and let us know what you think here? Also, do you find any vulnerabilities in our API already? Thanks again!

[Aryaman1706]

Hey @AnandBaburajan ! I read that discussion and I somewhat agree that mongoose validators could do the job, but don't you think that in case of an invalid request body, we should not allow the data to reach the saving stage as mongoose validators are pre-save hooks? I think we should keep validation of request as a separate step, this way we could save on some unnecessary operations being performed and would be able to deliver intuitive error messages to users. Also if you use something like yup, we could also reuse the validation schema on react side as well with formik for validating it there and then.

I have not tested the API thoroughly, I am gonna do it tonight and would get back if I found some issues. Meanwhile here are some I found:-

• Here let's say we pass in a number instead of an array then we would get an error vote.choices.every is not a function. Though this is will fall in catch block but the message is not clear for an average user.
• Here if we do not provide the field of encryptedEmailID, we would get an error like can not make Buffer from undefined (inside decrypt function) and this statement is not inside trycatch block so this might break the API.

Thanks a lot for addressing the raised issue so soon and I would love to discuss the issue further.

[anandbaburajan]

don't you think that in case of an invalid request body, we should not allow the data to reach the saving stage as mongoose validators are pre-save hooks? I think we should keep validation of request as a separate step, this way we could save on some unnecessary operations being performed and would be able to deliver intuitive error messages to users.

I agree, validattion would be good. But since the only user of this API would be the RM-client, do you suggest handling the necessary validation only at the client provided the server handles all the edge cases like the ones you mentioned?

• Here if we do not provide the field of encryptedEmailID, we would get an error like can not make Buffer from undefined (inside decrypt function) and this statement is not inside trycatch block so this might break the API.

Good catch, thanks! You're welcome to fix it.

Thanks a lot for addressing the raised issue so soon and I would love to discuss the issue further.

Thanks for your thoughts! :D please let me know what you think!

[Aryaman1706]

I agree, validation would be good. But since the only user of this API would be the RM-client, do you suggest handling the necessary validation only at the client provided the server handles all the edge cases like the ones you mentioned?

• I agree that since only one consuming the API is RM-client. I am already concerned about miscellaneous users if anyone gets the API endpoints, they could exploit some parts.
• Also, I am of the opinion that a self-contained API would go a long way as it could be integrated easily with a discord/slack bot or a chrome extension. We could take this project to the moon. by making more utilities for RocketMeet like bots or extensions.

Good catch, thanks! You're welcome to fix it

Sure, I am on it. Would soon raise a PR to fix the same.

Thanks a lot.

[anandbaburajan]

a self-contained API would go a long way as it could be integrated easily with a discord/slack bot or a chrome extension

Good point, makes sense, thanks! You're welcome to go ahead with validation. :D

[anandbaburajan]

Update: our API endpoints are now inside pages/api.