Skip to content
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

Login: Replace endpoint that fetches site info when using a site address to login #12940

Merged
merged 13 commits into from
Sep 17, 2020

Conversation

renanferrari
Copy link
Member

@renanferrari renanferrari commented Sep 11, 2020

Fixes #12832

When logging in using a site address, we used to make a call to /sites/$site to fetch more info about the site and to determine if we should treat it as a WordPress.com site or a self-hosted one.

That endpoint did not work well with private sites, which would cause issues for users that had custom domains and 2FA enabled (you can read more about it in the linked issue or in this internal ref: pbArwn-16v-p2).

This PR replaces the usage of that endpoint with /connect/site-info, which allows us to better handle those situations. This endpoint was already being implemented in the LoginFlow library, but was only used by the Woo app. I tried to reuse some of that code, while also making sure to keep the existing behavior regarding how to treat different site types (WordPress.com, Jetpack and self-hosted).

To test

You can test the following scenarios by creating sites with jurassic.ninja or by using a pre-configured test site.

Main Scenario

Private WordPress.com site with custom domain and 2FA enabled

  1. Clear app data.
  2. On the Prologue screen, if the Smart Lock dialog appears, dismiss it.
  3. Tap Enter your site address.
  4. On the Site Address screen, enter a site address for a private WordPress.com site with a custom domain and 2FA enabled.
  5. Tap Continue.
  6. Notice the Username/Password screen.
  7. Enter the username and password associated with the site.
  8. Tap Continue.
  9. Notice that the 2FA screen was displayed with no errors.
  10. Enter the two-factor authentication code.
  11. Tap Continue.
  12. Complete the login flow normally.

Secondary Scenarios

WordPress.com, Self-Hosted or Jetpack site

  1. Clear app data.
  2. On the Prologue screen, if the Smart Lock dialog appears, dismiss it.
  3. Tap Enter your site address.
  4. On the Site Address screen, enter either a WordPress.com, a Self-Hosted or a Jetpack site address.
  5. Tap Continue.
  6. Notice the Username/Password screen.
  7. Enter the username and password associated with the site.
  8. Tap Continue.
  9. Complete the login flow normally.

Note: While working on this PR, it was noted that Atomic and Jetpack sites are being treated as Self-Hosted sites as opposed to WP.com sites, which is the expected behavior. This will be addressed in a follow-up PR.

HTTP Auth

  1. Clear app data.
  2. On the Prologue screen, if the Smart Lock dialog appears, dismiss it.
  3. Tap Enter your site address.
  4. On the Site Address screen, enter http://do.wpmt.co/httpauth/.
  5. Tap Continue.
  6. Notice the Authorization Required dialog asking for the username/password pair for HTTP Basic Auth.

Self-Signed SSL

  1. Clear app data.
  2. On the Prologue screen, if the Smart Lock dialog appears, dismiss it.
  3. Tap Enter your site address.
  4. On the Site Address screen, enter https://do2.wpmt.co/vanilla/.
  5. Tap Continue.
  6. Notice the Invalid SSL Certificate dialog asking for a confirmation of the self-signed SSL certificate.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • 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.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Sep 11, 2020

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

@peril-wordpress-mobile
Copy link

Messages
📖

This PR contains changes in the subtree libs/login/. It is your responsibility to ensure these changes are merged back into wordpress-mobile/WordPress-Login-Flow-Android. Follow these handy steps!
WARNING: Make sure your git version is 2.19.x or lower - there is currently a bug in later versions that will corrupt the subtree history!

  1. cd WordPress-Android
  2. git checkout issue/12832-private-custom-domain-2fa-wpcom-site
  3. git subtree push --prefix=libs/login/ https://github.com/wordpress-mobile/WordPress-Login-Flow-Android.git merge/WordPress-Android/12940
  4. Browse to https://github.com/wordpress-mobile/WordPress-Login-Flow-Android/pull/new/merge/WordPress-Android/12940 and open a new PR.

Generated by 🚫 dangerJS

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Sep 11, 2020

You can test the changes on this Pull Request by downloading the APK here.

@renanferrari renanferrari added this to the 15.8 milestone Sep 15, 2020
@renanferrari renanferrari marked this pull request as ready for review September 15, 2020 14:50
@renanferrari renanferrari force-pushed the issue/12832-private-custom-domain-2fa-wpcom-site branch from d069324 to 2debdef Compare September 15, 2020 17:37
@khaykov khaykov self-assigned this Sep 16, 2020
Copy link
Member

@khaykov khaykov left a comment

Choose a reason for hiding this comment

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

Test scenarios work as expected! I'm not super familiar with the login codebase, but nothing particularly stands out to me 👍

@renanferrari renanferrari merged commit 12a5008 into develop Sep 17, 2020
@renanferrari renanferrari deleted the issue/12832-private-custom-domain-2fa-wpcom-site branch September 17, 2020 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants