London | 26-SDC-Mar | Ebrahim Beiati-Asl | Sprint 3 | Sprint3 middleware#76
London | 26-SDC-Mar | Ebrahim Beiati-Asl | Sprint 3 | Sprint3 middleware#76ebrahimbeiati wants to merge 23 commits into
Conversation
ebrahimbeiati
added
Needs Review
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Did you intent to commit the chat app files here?
Good work on this, I have a few comments though
| export default function usernameMiddleware(req, res, next) { | ||
| const username = req.headers["x-username"]; | ||
|
|
||
| req.username = username ?? null; |
There was a problem hiding this comment.
is it ok to continue with "null" as a username?
There was a problem hiding this comment.
Yes, returning null here is intentional. It allows the rest of the app to distinguish between:
a user who did provide an x-username header
and a user who did not authenticate
The main route checks this value and outputs either an authenticated or unauthenticated message, so null is a safe and explicit fallback.
| } else { | ||
| subjectMessage = `You have ${count} subjects: ${list}.`; | ||
| } | ||
| res.send(`${authMessage} ${subjectMessage}`); |
There was a problem hiding this comment.
What should the program output as a subject message if the authentication failed?
There was a problem hiding this comment.
If authentication fails, I still show the subject message on purpose so the user gets feedback about the subjects they sent.
LonMcGregor
added
Reviewed
ebrahimbeiati
added
Needs Review
|
Sorry for the delayed response. Thanks for your clarifications. In practice in the real world if authentication didn't pass you probably wouldn't return anything, but as long as you are aware of the impacts of authentication, this is fine. Good that you have thought about how you are using null here as part of the auth step! |
LonMcGregor
added
Complete
2 participants
Learners, PR Template
Self checklist
Sprint 3 middleware