Conversation
|
Also, ignore the broken tests—I need to get around to fixing them after the rate-limiting addition broke them (because firing off a bunch of POST requests in a short timeframe trips the ratelimiter, and that's exactly what the integration testing does) |
Should hopefully fix pre-commit issues
| suppressEmail: { | ||
| type: Boolean, | ||
| required: false, | ||
| }, |
There was a problem hiding this comment.
This isn't necessary for email verification requests, since there isn't really a "confirmation" of the email being verified like there is for password resets. We have this option in the password reset model because there's two scenarios where we do a password reset: 1) explicitly creating a new account and 2) a user-requested password reset. In scenario 2 we send a confirmation email after the change happens (see here), but not in scenario 1.
There was a problem hiding this comment.
Wait, why was this marked as resolved? This hasn't been fixed.
| +sauce-layout('Your request has been submitted!')(hideNavbar, includeBootstrapScripts) | ||
| h1.mb-4 We've sent you an email, follow the instructions to complete setting up your SCCS account | ||
| p. | ||
| To prevent abuse, SCCS staff manually approve all account creation |
There was a problem hiding this comment.
This isn't true anymore with these new changes!
(But also, don't worry about this, I'll write some copy here. This is more for my own note)
| @@ -280,6 +280,7 @@ export const isEmailAvailable = async (email: string): Promise<boolean> => { | |||
| const [inDatabase, inPending] = await Promise.all([ | |||
There was a problem hiding this comment.
I realize this code needs slightly more tweaking - needs to capture the results in three different variables
| const [inDatabase, inPending] = await Promise.all([ | |
| const [inDatabase, inPending, inNeedsVerify] = await Promise.all([ |
| VerifyEmailRequestModel.exists({ 'data.email': email }), | ||
| ]); | ||
|
|
||
| if (inDatabase || inPending) { |
There was a problem hiding this comment.
| if (inDatabase || inPending) { | |
| if (inDatabase || inPending || inNeedsVerify) { |
| suppressEmail: { | ||
| type: Boolean, | ||
| required: false, | ||
| }, |
There was a problem hiding this comment.
Wait, why was this marked as resolved? This hasn't been fixed.
views/createAccount.pug
Outdated
| input(type='hidden', name='id', value=id) | ||
| input(type='hidden', name='key', value=key) |
There was a problem hiding this comment.
The createAccount page itself (the one that doesn't contain Cygnet data) doesn't need id and key hidden form parameters. Those will go in the other page that requires you to actually click a reset link.
src/routes/account.ts
Outdated
| // router.get( | ||
| // '/init', | ||
| // catchErrors(async (req, res, next) => { | ||
| // return res.render('initAccount',); | ||
| // }), | ||
| // ); | ||
|
|
||
| // router.post( | ||
| // '/init', | ||
| // catchErrors(async (req, res, next) => { | ||
| // const { error, value } = jf.validateAsClass(req.query, controller.ResetCredentials); | ||
|
|
||
| // if (error) { | ||
| // throw new HttpException(400, { message: `Invalid request: ${error.message}` }); | ||
| // } | ||
|
|
||
| // // I have no idea why joiful throws a fit here but this is the necessary workaround | ||
| // const verifyEmail = await controller.verifyEmail( | ||
| // value as unknown as controller.ResetCredentials, | ||
| // ); | ||
|
|
||
| // return res.render('initAccountSuccess', { | ||
| // id: value.id, | ||
| // key: value.key, | ||
| // email: verifyEmail.email, | ||
| // }); | ||
| // }), | ||
| // ); | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Not sure why the commented block here but of course let me know if you need any help w/this
makinbacon21
force-pushed
the
master
branch
2 times, most recently
from
d59592f to
e024f21
Compare
makinbacon21
force-pushed
the
master
branch
2 times, most recently
from
2b8ae93 to
2dc9f80
Compare
makinbacon21
force-pushed
the
master
branch
2 times, most recently
from
d1ea2bb to
85dd740
Compare
3 participants
Resolves #37.