T33: Create sign up method in backend #19
Conversation
…tion in the database and hash + salt the password first
…e creating a new user.
…isAuth property on session object in the create user handler
…e session object.
…ill create. It will check if the session objects isAuth property is set to true to access a certain route.
| next(); | ||
| } else { | ||
| res.json({ message: 'Unauthorized access' }); | ||
| res.redirect('/api/auth/sign-in'); |
There was a problem hiding this comment.
have you tested this redirect? this should be a frontend URL not a backend API endpoint
| const THREE_DAYS = 1000 * 60 * 60 * 24 * 3; | ||
| const TWO_MINUTES = 1000 * 60 * 2; |
There was a problem hiding this comment.
nit: slightly more useful names like THREE_DAYS_IN_MS help make it clear what the units are
| store: new PrismaSessionStore(new PrismaClient(), { | ||
| checkPeriod: TWO_MINUTES, | ||
| dbRecordIdIsSessionId: true, | ||
| dbRecordIdFunction: undefined, |
There was a problem hiding this comment.
is it needed that you supply this as undefined? can we just omit?
| import prisma from '../utils/prismaClient'; | ||
| import { SessionData } from '../utils/types'; | ||
|
|
||
| export const createNewUser = async (req: Request, res: Response) => { |
There was a problem hiding this comment.
generally it's good to redraw the boundaries of route handlers and controller functions such as these.
for example, idk if you're planning on adding testing, but it is a lot more annoying to test a function defined like this since you have to mock entire Request and Response arguments in order to unit test this function. generally, you'd have a light wrapper as a controller function that would only serve to pull the right arguments out of the request object and inject them into this controller fn, and then you'd update the return type of this fn in order to return the newly created User object or a boolean, depending on your exact logic
export const createNewUser = async (firstName, lastName, email, rawPassword) {...}
| req: Request, | ||
| res: Response |
There was a problem hiding this comment.
similar here, this fn is meant to do something very simple, there's no need to throw the entire request and response objects into here, when simply accepting an email and returning a boolean are enough.
| import { NextFunction, Request, Response } from 'express'; | ||
|
|
||
| export const getHashedPlusSaltedPassword = (password: string) => { | ||
| return bcrypt.hash(password, 5); |
There was a problem hiding this comment.
nit: make the salt rounds variable a constant, it's not clear what that value is otherwise
| }; | ||
|
|
||
| // middleware to be used to check if the user is authenticated or not on protected routes (to be used for future routes) | ||
| export const isAuth = (req: Request, res: Response, next: NextFunction) => { |
There was a problem hiding this comment.
given that your session field is isAuth i'd rename this fn to be more informative. since this is a middleware, you could do something like checkAuthenticatedSession, which is a clear enough name that you can remove the comment above
| return false; | ||
| }; | ||
|
|
||
| // sign in |
| export const createNewUser = async (req: Request, res: Response) => { | ||
| const emailExists: boolean = await doesEmailExistInDatabase(req, res); | ||
|
|
||
| if (!emailExists) { |
There was a problem hiding this comment.
what happens if the email exists? doesn't look like the response ever ends if it does
| password: await getHashedPlusSaltedPassword(req.body.password), | ||
| }, | ||
| }); | ||
| req.session.isAuth = true; |
There was a problem hiding this comment.
not sure if you've tested this but the express docs seem to imply that you should use the regenerate and save methods. other docs suggest that you don't have to explicitly call save though, so I'm unsure what is currently best practice
Session.save(callback)
Save the session back to the store, replacing the contents on the store with the contents in memory (though a store may do something else–consult the store’s documentation for exact behavior).
This method is automatically called at the end of the HTTP response if the session data has been altered (though this behavior can be altered with various options in the middleware constructor). Because of this, typically this method does not need to be called.
There are some cases where it is useful to call this method, for example, redirects, long-lived requests or in WebSockets.
Description:
Happy Case:
Email Already Exists Case:
