CIF-1084 - Implement the password reset page #405
Conversation
herzog31
requested review from
dplaton,
cjelger,
mhaack,
LSantha,
laurentiumagureanu,
hbacila and
alkim91
Codecov Report
@@ Coverage Diff @@
## issue/CIF-1084 #405 +/- ##
====================================================
- Coverage 85.86% 85.40% -0.46%
+ Complexity 929 919 -10
====================================================
Files 186 188 +2
Lines 4781 4811 +30
Branches 670 674 +4
====================================================
+ Hits 4105 4109 +4
- Misses 538 557 +19
- Partials 138 145 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| const [resetPassword] = useMutation(MUTATION_RESET_PASSWORD); | ||
|
|
||
| const handleFormSubmit = useCallback( | ||
| async ({ email, token, password }) => { |
There was a problem hiding this comment.
I think you should revalidate the email address here, and not just rely on the client-side validation which can easily be skipped, it's green-belt security ;-)
I'm also wondering if we can also use a CSRF token to make sure that nobody can just DDoS the form or do some kind of brute-force attack against Magento's reset password feature.
There was a problem hiding this comment.
I think you should revalidate the email address here, and not just rely on the client-side validation which can easily be skipped, it's green-belt security ;-)
Validating the email again in JS is again client-side validation and can easily be skipped. No point in doing that twice ;)
I'm also wondering if we can also use a CSRF token to make sure that nobody can just DDoS the form or do some kind of brute-force attack against Magento's reset password feature.
Good point. Currently we don't do this at all (login for example). I think this should be investigated in a separate issue.
There was a problem hiding this comment.
Validating the email again in JS is again client-side validation and can easily be skipped. No point in doing that twice ;)
Good point, my brain was still in client-server mode ;-)
For CSRF I agree we could do this in a separate issue.
* CIF-1084 - Add password reset request email mutation * CIF-1565 - Implement password reset mutation and components * CIF-1565 - Update unit tests * CIF-1084 - Implement the password reset page (#405) * CIF-1084 - Add reset password form component * CIF-1084 - Add ResetPassword component * CIF-1084 - Add unit tests
Description
This PR depends on #400.
ResetPasswordReact component to display the reset password form.resetpasswordcomponent to display the placeholder for the React component.Please also see adobe/aem-cif-guides-venia#60.
How Has This Been Tested?
Types of changes
Checklist: