-
Notifications
You must be signed in to change notification settings - Fork 270
Add Two Factor Authentication for React Starter Kit #156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
I think this is better be its own starter kit |
# Conflicts: # routes/auth.php # tests/Feature/Auth/AuthenticationTest.php # tests/Feature/Auth/PasswordConfirmationTest.php
Waiting until we fix this bug in the Inertia |
Why is this using Fortify? I don't see how it makes sense to use part of that package when the rest of the authentication is already handled by open code. |
Its been merged! |
Hey It's needs to be tagged with new release in order to use it. I'm tracking it and will push changes once tagged. |
Don't bother they won't listen. Just wait for a maintainer to close this |
Thanks for the shout out @pushpak1300 😁 Excited to see this released 👍 |
We decided to use Fortify because it's battle tested and the likelihood of anyone changing the underlying code (not the views, which are open code) are very slim. We did not want to burden the user with maintaining this code, we instead opted for a |
Sorry you feel this way. This PR will be merged once it's ready. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a first pass here, I haven't run it locally, just looked at the code. I'll spin it up in the next round and we'll keep refining 👍
import { REGEXP_ONLY_DIGITS } from 'input-otp'; | ||
import { useMemo, useState } from 'react'; | ||
|
||
const OTP_MAX_LENGTH = 6; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now defined in two places, can we centralized it somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking lot about this while developing and now end up adding this in useTwoFactorAuth
hook. Which I think is the right place for it. what do you think ?
This MR adds Two-Factor Authentication (2FA) functionality to the React Starter Kit using Laravel Fortify's built-in support for 2FA
🔒 Backend Changes
We are now leveraging Fortify for handling two-factor authentication. Since Fortify provides additional security features out of the box, this MR also replaces our custom implementation of the password confirmation page with Fortify’s native confirm password functionality.
🖥️ Frontend/UI Updates
The UI has been updated to handle all the two-factor settings based on Fortify's configuration. The following Fortify features are enabled:
Demo
https://www.loom.com/share/6bf836eccae84778a511916a9d02bb59?sid=efb1fac2-7482-40ec-82e6-31f0b3fa77a8
Co-authored-by: Tony Lea [email protected]
Thanks to #101