Skip to content

Conversation

@frosty
Copy link
Contributor

@frosty frosty commented Jan 6, 2022

Resolves #17696. This PR makes some improvements to the handling of password manager entries that contain usernames instead of email addresses.

Please see also the associated WordPress Authenticator PR wordpress-mobile/WordPressAuthenticator-iOS#630

While we require an email address via the UI in the app (iirc it's because it's typically easier for a user to remember correctly), it's always been technically possible to login using a username. You can do this if your password manager contains an username instead of an email address.

#17696 picked up a couple of odd edge cases if you navigate back during the process:

  1. While the email field would still show the email address, under the hood the loginFields.username property would have been updated to the username of the account resulting in a mismatch between the UI and the submitted data.
  2. As a result of 1, when tapping Continue in the above situation an error would be displayed because the username was being validated instead of the email address contained in the text field onscreen.
  3. The error message displayed in 2 was unrelated, and was actually the error to be shown when requesting a magic link.

I've added a couple of tweaks to improve each of these points:

  1. When the Get Started screen appears, we now ensure that the loginFields.username field matches whatever is currently in the email field. This essentially 'resets' any changes made under the hood on the password screen, so the user can have confidence that the UI is correct.
    2 and 3. The situation in 2 should actually now no longer happen because the email address is correctly validated. However, I've provided a separate error message if we're just validating the form vs validating to request a magic link. I have been unable to find a use case where this happens, as the continue button is disabled if the email isn't valid, but I've included it for completeness:

To test

  • Build and run on a device that has 1Password installed, or passwords saved in iCloud Keychain
  • Ensure that one of those accounts has a username saved instead of an email address
  • Log out if you are already logged in the app
  • Tap Log in or sign up with WordPress.com
  • Enter a valid email address for a WordPress.com account and tap Continue
  • On the password screen, tap the key icon above the keyboard and autofill the details of an account that has a username saved instead of an email address. You should see the email address at the top of the screen update to show the username instead. Do not tap Continue.
  • Tap Back to return to the Get Started screen. Ensure that the email address you entered originally is still present and the Continue button is still enabled.
  • Tap Continue. You should see the Password screen, and it should be displaying the email address you entered previously. Ensure you can continue through the rest of the login process and log in successfully.

Finally, ensure that you can log in normally by typing your details and also by completing an autofill successfully from the beginning.

Regression Notes

  1. Potential unintended areas of impact

The changes here are very minimal, but of course they touch the login flow which is one of the most critical flows of the app.

  1. What I did to test those areas of impact (or what existing automated tests I relied on)

I ensured that I could still log in as before, and ran the UI tests.

  1. What automated tests I added (or what prevented me from doing so)

None

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@frosty frosty added this to the 19.0 milestone Jan 6, 2022
@frosty frosty requested a review from ScoutHarris January 6, 2022 13:41
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jan 6, 2022

You can trigger an installable build for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jan 6, 2022

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

Copy link
Contributor

@ScoutHarris ScoutHarris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @frosty . Thanks for the detailed testing steps! Tried various scenarios, and was able to log in successfully.

:shipit:

@frosty frosty enabled auto-merge January 7, 2022 23:35
@frosty frosty merged commit 8984d25 into develop Jan 8, 2022
@frosty frosty deleted the fix/auth-username-autofill branch January 8, 2022 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update error message when a username / password manager is used to log in

3 participants