#23187419173 - fx:fixed Two Factor Authentication for Sellers #43
Conversation
soleil00
requested review from
dniwemugisha,
teerenzo,
niyobertin,
Jovz19200,
Heisjabo and
yvanddniyo
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #43 +/- ##
==========================================
+ Coverage 78.30% 78.35% +0.04%
==========================================
Files 56 55 -1
Lines 899 901 +2
Branches 124 128 +4
==========================================
+ Hits 704 706 +2
+ Misses 195 192 -3
- Partials 0 3 +3 ☔ View full report in Codecov by Sentry. |
src/controllers/userControllers.ts
Outdated
| const link = | ||
| process.env.NODE_ENV !== "production" | ||
| ? `http://localhost:${env.port}/api/v1/users/2fa-verify/${token}` | ||
| : `https://eagles-ec-be-development.onrender.com/api/v1/users/2fa-verify/${token}`; |
There was a problem hiding this comment.
don't hardcode this things, create .env for url, this this will be called from front-end
| userRoutes.put("/passwordupdate", isLoggedIn, validateSchema(passwordUpdateSchema), updatePassword); | ||
| userRoutes.post("/2fa-verify", isTokenFound, tokenVerification); | ||
| userRoutes.get("/2fa-verify/:token",tokenVerification); |
There was a problem hiding this comment.
are you verifying from get method?
There was a problem hiding this comment.
oooh I missed this
There was a problem hiding this comment.
again for that get its verification link that user click and u can't use post using link since it will be opened directly in browser
perhaps when we put fronted url its possibl
There was a problem hiding this comment.
this will be handled from front-end tha means link will be front-end link, but leave it for now
|
|
||
| export const sendEmailService = async (user: IUser, subject: string, template: any, token: number) => { | ||
| export const sendEmailService = async (user: IUser, subject: string, template: any) => { |
There was a problem hiding this comment.
do you think this function can be re-used , why that user can you put email instead whole user object
There was a problem hiding this comment.
yes its reusable i removed token since token doesn't have naything todo with that sendEmailService, token or any other related info has be passed as argument in emailTemplate not in service
There was a problem hiding this comment.
also pass email to send to not whole user
| @@ -86,6 +86,7 @@ | |||
| "express": "^4.19.2", | |||
| "express-session": "^1.18.0", | |||
| "husky": "^9.0.11", | |||
| "ioredis": "^5.4.1", | |||
There was a problem hiding this comment.
i was about to use it but realy i'm not using it currently, since token don't last long(10min) i decided not to blacklist verification token s
soleil00
force-pushed
the
fx-2fa-#187419173
branch
from
671f0ae to
eecc41f
Compare
What does this PR do?
This PR sets up Two Factor Authentication for Sellers
Description of Task to be completed?
-GIVEN there is a request to log in a user to the website
WHEN 2FA is enabled on the particular user
THEN Further request for a 2FA token should be requested and once the token is received and validated, the user is allowed to log in.
How should this be manually tested?
/api/v1/users/2fa-verifypivotal tracker id ----> #23187419173
Screenshots