feat: add otp functions #265
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
esaminu
force-pushed
the
add-otp-cloud-functions
branch
from
0a4c757
to
0480c38
Compare
packages/functions/src/index.ts
Outdated
| const otp = crypto.randomInt(100000, 999999).toString(); | ||
|
|
||
| const otpDoc = admin.firestore().collection('otps').doc(email); | ||
| await otpDoc.set({ |
There was a problem hiding this comment.
As an enhancement, could we add a check to see if there is an existing non-expired OTP for the same email before creating a new one? This would help prevent flooding the otps collection with multiple OTP requests and ensure that old OTPs are not left without cleanup.
There was a problem hiding this comment.
There might be some issue with the email receiver that caused it to not receive the previous otp causing the user to click resend. I don't think it's good practice to re use the same code in these cases. Only one OTP code is stored at a time and every time the user clicks "Resend" we overwrite that
| @@ -0,0 +1,5 @@ | |||
| { | |||
There was a problem hiding this comment.
Should we add a cron job to clean up old OTPs?
There was a problem hiding this comment.
Old otps that are left in our db will happen when someone requests codes to multiple emails and does not verify any of them. We create one record per email (we override the record for multiple requests to the same email) and we delete the record once the user is verified and gets a token. We can add a daily cron job to handle expired otps older than a month or so IMO but they serve as a log for people trying to login and unable to get access, we can inspect the db and verify that we sent them a code - we did something similar for wallet
There was a problem hiding this comment.
Yeah, I was thinking about the expired ones. But it makes sense to leave it there for logging purpose.
|
Great improvement!!! @esaminu |
This PR adds otp cloud functions to be called instead of using firebase email links