My Account - Forgot Password #2619

revanth0212 · 2020-08-10T19:52:36Z

Description

Implemented forgot password UX and workflow for the new my account feature.

For this PR I had to add a new route /customer/account/createPassword which provides the UX to reset the user's password.

When the user requests a password reset, the backend sends an email with a button to take the user to a new tab. The link that the backend includes in the email, uses the MAGENTO_BACKEND_URL as the domain. Since all the backends don't have the code to handle the new route /customer/account/createPassword, @dpatil-magento helped me deploy the new code on this server: https://staging-vdt2zeq-c5v7sxvquxwl4.eu-4.magentosite.cloud/. If someone wants to test this PR locally, please use the above backend URL instead.

Also if you wanna test this PR on a deployed instance please use #2621. @dpatil-magento made sure the backend for that PR is set to https://staging-vdt2zeq-c5v7sxvquxwl4.eu-4.magentosite.cloud/ which has the code to handle the new route.

Another way to test this is to paste the query string part from the email link next to the deployed app URL, for instance: https://pr-2619.pwa-venia.com/customer/account/createPassword/ + ?email=gooseton%40gmail.com&token=*******.

Sorry if this is confusing.

Note: To be merged only after #2590

Related Issue

Closes PWA-77

Acceptance

Users should be able to reset their password.

Verification Stakeholders

@soumya-ashok
@schensley
@jimbo

Specification

This PR adds a new route customer/account/createPassword which takes 2 query params ?email=EMAIL_ID&token=RESET_TOKEN.

Verification Steps

Go to the deployed app and open the sign-in dropdown. Click on the forgot password link.
Enter the email id and submit.
Check for the reset email. Click the reset button in the email.
Reset UI should open up in the new tab.
Enter new password and click submit.

Screenshots / Screen Captures (if appropriate)

Demo link: https://jira.corp.magento.com/secure/attachment/367170/Forgot%20password%20demo.mp4

Checklist

I have added tests to cover my changes, if necessary.

I have updated the documentation accordingly, if necessary.

Co-authored-by: Stephen <[email protected]>

PWAStudioBot · 2020-08-10T20:05:27Z

	Messages
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.
📖	DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖	Associated JIRA tickets: PWA-77.

Generated by 🚫 dangerJS against a7c62e6

devops-pwa-codebuild · 2020-08-10T20:14:31Z

Performance Test Results

The following fails have been reported by WebpageTest. These numbers indicates a possible performance issue with the PR which requires further manual testing to validate.

https://pr-2619.pwa-venia.com : LH Performance Expected 0.85 Actual 0.33, LH Accessibility Expected 1 Actual 0.97, LH Best Practices Expected 1 Actual 0.93, WPT Cache Expected 90 Actual 38
https://pr-2619.pwa-venia.com/venia-tops.html : LH Performance Expected 0.75 Actual 0.35, LH Best Practices Expected 1 Actual 0.93
https://pr-2619.pwa-venia.com/valeria-two-layer-tank.html : LH Performance Expected 0.8 Actual 0.41, LH Accessibility Expected 0.9 Actual 0.89, LH Best Practices Expected 1 Actual 0.93, WPT Cache Expected 65 Actual 50

revanth0212 · 2020-08-10T22:28:40Z

packages/venia-ui/lib/targets/venia-ui-intercept.js

+        },
+        {
+            name: 'Reset Password',
+            pattern: '/customer/account/createPassword',


The route can change. It is the route configured in the forgot password email template in the admin panel.

I don't think we have a setting for changing it without modifying the email template manually. Should we have this in the app config?

TBH I'm not sure how much we care about the preconfigured routes. I would also probably use hyphen instead of camelcase ie /customer/account/create-password.

It is the default route. If we happen to change this and someone forgets to change the template, the whole workflow fails. That is no reason to not change since someone configuring this should know for sure. I prefer your idea as well, hyphen case is correct in this case.

I believe until such time that there is a M2 module for Magento Core that configures the front-end to be in line with PWA Studio expectations with a click of a button the default route should work.

If there was preference for something else maybe an upward rule could be added to redirect from the default to the PWA Studio expected?

Hey @fooman, I don't think UPWARD will be bothered in this case because the link from the email will directly hit the backend server. If we have runtime UPWARD rules, we can perform such kind of mapping. @zetlen any thoughts?

Link from the email would look something like this:

https://staging-vdt2zeq-c5v7sxvquxwl4.eu-4.magentosite.cloud/customer/account/createPassword/?email=EMAIL_ID&token=TOKEN

@revanth0212 I do believe you might have stumbled on a bigger discussion here...

This link:
https://staging-vdt2zeq-c5v7sxvquxwl4.eu-4.magentosite.cloud/customer/account/createPassword/?email=EMAIL_ID&token=TOKEN
combined with running the upward php connector would result in this

https://github.com/magento/magento2-upward-connector/blob/develop/Plugin/Magento/Framework/App/AreaList.php#L50

making it an upward response (unless the route gets whitelisted, I am not sure that there is currently an option to whitelist a changeable url).

The other option I consider likely is that the back-end will be configured to send out an email like
https://venia.magento.com/customer/account/createPassword/?email=EMAIL_ID&token=TOKEN
(the base url can be changed for the complete site, whereas the /customer/account/createPassword part is in the individual email template) with a separate back-end url. In this scenario upward-js would handle it.

I am not sure if the admin panel has an option to store/change the main site URL / public URL for instance https://venia.magento.com/. If so, we can check to use that.

This is how the URL is generated in the email template:

<a href="{{var this.getUrl($store,'customer/account/createPassword/',[_query:[token:$customer.rp_token,email:$customer.email],_nosid:1])}}" target="_blank">{{trans "Set a New Password"}}</a>

Ill have to look into the implementation of the this.getUrl function to know what variables it is considering.

@revanth0212 it is all configurable under Stores > Configuration > Web.

I guess the point I was trying to make is that upward is part of it one way or the other (upward-php or upward-js).

So if you wanted to have a route my-account/create-password then an upward rule could redirect from customer/account/createPassword/ to my-account/create-password to cover people using the default template.

…_forgot_password

supernova-at · 2020-08-24T17:11:28Z

QA

Verification Steps

Note: verification steps were performed on #2621 PR deployment.

Go to the deployed app and open the sign-in dropdown. Click on the forgot password link.
Enter the email id and submit.
Check for the reset email. Click the reset button in the email.
Reset UI should open up in the new tab.
Enter new password and click submit.

Other

Cannot Reset the Customer's Password

I entered an email that does not have an account and got the following error:

This seems like a reasonable error message, my only concern is that we may be providing valuable information to a potential attacker. The standard message "If there is an account associated with EMAIL you will receive an email with a link to change your password" covers this error scenario and it might be better to show that message even when we get this error in the errors of the GraphQL response. That said, I know these are just strings and we don't want to hardcode english comparisons, so 🤷‍♂️

I should also note that I am assuming that the reason we got that error was because there was no account associated with that email address. Should probably check with the GraphQL team to make sure.

Stay on Reset Password Page

After resetting my password and signing in via the site header, I was never navigated away from the "Reset Password" page:

I'm not quite sure what I'd want to happen instead, but it was a strange UX, especially after I signed in successfully. cc @schensley

revanth0212 · 2020-08-24T21:32:01Z

This seems like a reasonable error message, my only concern is that we may be providing valuable information to a potential attacker. The standard message "If there is an account associated with EMAIL you will receive an email with a link to change your password" covers this error scenario and it might be better to show that message even when we get this error in the errors of the GraphQL response.

If I remember correctly, the GQL team is working on updating their mutations to provide more reasonable/understandable error messages. It should be just a matter of time before they come up with a better error message for such use cases. That said, should we wait for them or render our own message? If we do it ourselves, it will be a standard message for any sort of an error from the server.

Also, even if we do that, it might still be subject to security because the attacker can simply check the network tab for the error reported from the server. What do you guys think?

schensley · 2020-08-24T22:05:01Z

@supernova-at

After resetting my password and signing in via the site header, I was never navigated away from the "Reset Password" page:

This isn't uncommon for reset pages, at least from my experience. I don't think we can predict where the user would like to be. Maybe they will sign in immediately, but do they intend to go to checkout, perform some Account management, or enable a personalized shopping experience? Arbitrarily navigating them to some page (i.e. Home page) could feel imposing or interpreted as presenting some workflow they think they must complete. Keeping them on a page that displays a Success message allows them to make that decision themselves.

supernova-at · 2020-08-25T15:59:22Z

Also, even if we do that, it might still be subject to security because the attacker can simply check the network tab for the error reported from the server. What do you guys think?

Good point. I'm good keeping this as-is then, with the hopes that GQL error messages improve.

supernova-at · 2020-08-25T16:00:01Z

Are there security concerns with having the customer email and a reset token in the URL?

revanth0212 · 2020-08-25T16:43:59Z

Are there security concerns with having the customer email and a reset token in the URL?

It is usual practice to include email and a token in the reset link. We can get away with not having the email in the link by adding another field on the page where the user can enter their email. But, token needs to be present in the link.

sirugh · 2020-08-25T19:23:01Z

It is usual practice to include email and a token in the reset link. We can get away with not having the email in the link by adding another field on the page where the user can enter their email. But, token needs to be present in the link.

Is the link and params configurable in the back end? What is the default setting?

revanth0212 · 2020-08-25T19:54:11Z

Is the link and params configurable in the back end? What is the default setting?

Yeah, link and params are configurable. We are using the default link which is customer/account/createPassword. The email template by default only has the logic to include the token in the email, but due to the UX requirements, we had to add the email id as well.

tjwiebell · 2020-08-25T20:00:53Z

... but due to the UX requirements, we had to add the email id as well.

This is also a technical requirement, the GraphQL mutation requires token, email, and new password.

revanth0212 · 2020-08-25T20:49:14Z

... but due to the UX requirements, we had to add the email id as well.

This is also a technical requirement, the GraphQL mutation requires token, email, and new password.

Yes, you are right but UX wanted the user to click the link, enter new password and submit. Hence we had to change the email template to include email id as well.

sirugh · 2020-08-25T20:52:58Z

The email template by default only has the logic to include the token in the email, but due to the UX requirements, we had to add the email id as well.

Are you saying that you customized your back end to include email in the reset link that is sent to the user? Was the UX requirement not to have an email input field?

I would be cautious about customizing the default link - it would be preferable to leave it as is so pwa studio just works out of the box.

If we absolutely must stick to this UX then can you create a follow up ticket to have @jcalcaben update the "getting started" steps and whatever documentation is necessary?

My instinct is to have an email entry field and a new password field and to not customize the link.

abso-bmarquis · 2020-11-17T19:11:52Z

Hi @supernova-at,

I believe this code was removed accidentally when merging this PR: #2657
I don't see the feature on any of the current tags.

Please advise.
Regards,
Ben

dpatil-magento · 2020-11-17T20:18:27Z

HI @abso-bmarquis,

Latest PR related to forgot password refactor is #2726.

Thanks,
Dev

abso-bmarquis · 2020-11-17T21:13:37Z

Thanks!

revanth0212 and others added 23 commits July 29, 2020 16:03

Added singin trigger content.

94915e9

Tests update.

d9ace43

Removed forgot password and create account comps.

b53c886

Reverting unnecessary changes.

73ad9be

Converted VIEWS to const export instead of prop.

3c93713

Updated account menu tests.

5c7a95e

Added useAccountTrigger tests.

8220961

Added formErrors support.

d834672

Minor UX changes.

4db9dd6

Merge branch 'develop' into revanth/my_account_singin_trigger

10d3a6a

Implemented forgot password component.

51df066

Updated forgot password buttons css.

a9b92e9

Wired in forgot password mutation.

000f91a

Added reser password route.

86459f8

Update packages/peregrine/lib/talons/Header/useAccountTrigger.js

5954897

Co-authored-by: Stephen <[email protected]>

Added reset password UI.

2238c11

Added new password submit logic.

08c32f3

Minor CSS changes.

3e5b1e8

Loading state CSS update.

168cac6

Prop names change.

59fab55

Removing form errors. Tobe handled in 2588.

a8b6e00

Moving account menu logic to account menu trigger.

4e21c97

Minor.

9c6e0e9

PWAStudioBot added pkg:peregrine pkg:venia-ui labels Aug 10, 2020

revanth0212 commented Aug 10, 2020

View reviewed changes

m2-community-project bot assigned revanth0212 Aug 10, 2020

revanth0212 added 5 commits August 21, 2020 14:12

Added forgot password docs and minor modifications.

8bbe818

Added forgot password component and talon tests.

c515b14

Added new js docs.

8144d21

Added user context docs.

9c98d80

Minor.

d3572ae

sirugh previously approved these changes Aug 24, 2020

View reviewed changes

revanth0212 added 2 commits August 24, 2020 10:49

Merge remote-tracking branch 'origin/develop' into revanth/my_account…

9c18215

…_forgot_password

Tests update.

c3a4206

revanth0212 dismissed sirugh’s stale review via c3a4206 August 24, 2020 16:06

sirugh approved these changes Aug 24, 2020

View reviewed changes

Merge branch 'develop' into revanth/my_account_forgot_password

a7c62e6

supernova-at merged commit b942aff into develop Aug 25, 2020

supernova-at deleted the revanth/my_account_forgot_password branch August 25, 2020 21:18

m2-community-project bot removed the Progress: review label Aug 25, 2020

abso-bmarquis mentioned this pull request Nov 17, 2020

My Account - Create an Account component. #2657

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

My Account - Forgot Password #2619

My Account - Forgot Password #2619

revanth0212 commented Aug 10, 2020 •

edited

Loading

PWAStudioBot commented Aug 10, 2020 •

edited

Loading

devops-pwa-codebuild commented Aug 10, 2020 •

edited

Loading

revanth0212 Aug 10, 2020

sirugh Aug 12, 2020

revanth0212 Aug 12, 2020

fooman Aug 12, 2020

revanth0212 Aug 12, 2020

fooman Aug 12, 2020 •

edited

Loading

revanth0212 Aug 12, 2020

fooman Aug 12, 2020 •

edited

Loading

supernova-at commented Aug 24, 2020 •

edited

Loading

revanth0212 commented Aug 24, 2020 •

edited

Loading

schensley commented Aug 24, 2020

supernova-at commented Aug 25, 2020

supernova-at commented Aug 25, 2020

revanth0212 commented Aug 25, 2020

sirugh commented Aug 25, 2020

revanth0212 commented Aug 25, 2020

tjwiebell commented Aug 25, 2020 •

edited

Loading

revanth0212 commented Aug 25, 2020

sirugh commented Aug 25, 2020

abso-bmarquis commented Nov 17, 2020

dpatil-magento commented Nov 17, 2020

abso-bmarquis commented Nov 17, 2020

My Account - Forgot Password #2619

My Account - Forgot Password #2619

Conversation

revanth0212 commented Aug 10, 2020 • edited Loading

Description

Related Issue

Acceptance

Verification Stakeholders

Specification

Verification Steps

Screenshots / Screen Captures (if appropriate)

Checklist

PWAStudioBot commented Aug 10, 2020 • edited Loading

devops-pwa-codebuild commented Aug 10, 2020 • edited Loading

revanth0212 Aug 10, 2020

Choose a reason for hiding this comment

sirugh Aug 12, 2020

Choose a reason for hiding this comment

revanth0212 Aug 12, 2020

Choose a reason for hiding this comment

fooman Aug 12, 2020

Choose a reason for hiding this comment

revanth0212 Aug 12, 2020

Choose a reason for hiding this comment

fooman Aug 12, 2020 • edited Loading

Choose a reason for hiding this comment

revanth0212 Aug 12, 2020

Choose a reason for hiding this comment

fooman Aug 12, 2020 • edited Loading

Choose a reason for hiding this comment

supernova-at commented Aug 24, 2020 • edited Loading

QA

Verification Steps

Other

Cannot Reset the Customer's Password

Stay on Reset Password Page

revanth0212 commented Aug 24, 2020 • edited Loading

schensley commented Aug 24, 2020

supernova-at commented Aug 25, 2020

supernova-at commented Aug 25, 2020

revanth0212 commented Aug 25, 2020

sirugh commented Aug 25, 2020

revanth0212 commented Aug 25, 2020

tjwiebell commented Aug 25, 2020 • edited Loading

revanth0212 commented Aug 25, 2020

sirugh commented Aug 25, 2020

abso-bmarquis commented Nov 17, 2020

dpatil-magento commented Nov 17, 2020

abso-bmarquis commented Nov 17, 2020

revanth0212 commented Aug 10, 2020 •

edited

Loading

PWAStudioBot commented Aug 10, 2020 •

edited

Loading

devops-pwa-codebuild commented Aug 10, 2020 •

edited

Loading

fooman Aug 12, 2020 •

edited

Loading

fooman Aug 12, 2020 •

edited

Loading

supernova-at commented Aug 24, 2020 •

edited

Loading

revanth0212 commented Aug 24, 2020 •

edited

Loading

tjwiebell commented Aug 25, 2020 •

edited

Loading