Added middleware for subscribe API #1102
Conversation
|
Nit:- can you please connect issue ticket in description |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
middlewares/validators/challenges.js
Outdated
| @@ -17,6 +17,22 @@ const createChallenge = async (req, res, next) => { | |||
| } | |||
| }; | |||
|
|
|||
| const subscribeToChallenge = async (req, res, next) => { | |||
| const schema = joi.object().strict().keys({ | |||
| userId: joi.string().guid({ version: 'uuidv4' }).required(), | |||
There was a problem hiding this comment.
Can you please explain why you used version: 'uuidv4' ?
because I am not seeing it is used anywhere in backend repo validator file
cc: @heyrandhir @RitikJaiswal75
There was a problem hiding this comment.
We actually dont need this here. @arpit01923 Could you point me to the file where we are using uuid in userid?
middlewares/validators/challenges.js
Outdated
| @@ -17,6 +17,22 @@ const createChallenge = async (req, res, next) => { | |||
| } | |||
| }; | |||
|
|
|||
| const subscribeToChallenge = async (req, res, next) => { | |||
| const schema = joi.object().strict().keys({ | |||
| userId: joi.string().guid({ version: 'uuidv4' }).required(), | |||
There was a problem hiding this comment.
We actually dont need this here. @arpit01923 Could you point me to the file where we are using uuid in userid?
middlewares/validators/challenges.js
Outdated
| const subscribeToChallenge = async (req, res, next) => { | ||
| const schema = joi.object().strict().keys({ | ||
| userId: joi.string().guid({ version: 'uuidv4' }).required(), | ||
| challengeId: joi.string().guid({ version: 'uuidv4' }).required(), |
There was a problem hiding this comment.
why not just keep the id as string? to compare that id is unique we need some some other userIds, but since here a single user id is being passed we are ok considering the user id as string.
There was a problem hiding this comment.
Ok got it @RitikJaiswal75, removed uuid from validator
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| router.get("/", authenticate, challenges.fetchChallenges); | ||
| router.post("/", authenticate, createChallenge, challenges.createChallenge); | ||
| router.post("/subscribe", authenticate, challenges.subscribeToChallenge); | ||
| router.post("/subscribe", authenticate, subscribeToChallenge, challenges.subscribeToChallenge); |
Check failure
Code scanning / CodeQL
Missing rate limiting High
middlewares/validators/challenges.js
Outdated
| next(); | ||
| } catch (error) { | ||
| logger.error(`Error validating subscribeToChallenge payload : ${error}`); | ||
| res.boom.badRequest(error.details[0].message); |
There was a problem hiding this comment.
Hey @arpit01923, sending the exact error message to the end user might be harmful and can expose critical information on the backend Making us prone to external threats.
We already have a response saved in constants file which can be used here to send back to the end user.
test/unit/models/challenges.test.js
Outdated
There was a problem hiding this comment.
Please write a test for the middleware in tests/unit/middleware.
Added middleware for /challenge/subscribe API. So that -
Issue - #1017