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

Implementing WordPress.com Authentication #69

Merged
merged 26 commits into from
May 29, 2018

Conversation

jleandroperez
Copy link
Collaborator

@jleandroperez jleandroperez commented May 24, 2018

Details:

This PR is the first stab at implementing WordPress.com authentication:

  • The Authenticator will be displayed each time the app is launched
  • Account metadata / credentials won't be persisted. For now! (just keeping the PR's as small as possible).
  • Prologue + Epilogue to be implemented in another PR, as well.

Testing:

We'd just need to run smoke tests over (all of the possible flows), and make sure the app doesn't crash. This is should cover all of the "Sign into WordPress.com" scenarios:

  1. Log into a WordPress.com account (Email + Password)
  2. Log into a WordPress.com account (Email + Password + 2FA)
  3. Log into a WordPress.com account (Email + Password + SMS Token)
  4. Log into a WordPress.com account (Magic Link)
  5. Log into a WordPress.com account (With Google!)
  6. Log into a WordPress.com account (1Password support)

@mindgraffiti the PR itself looks HUGE (but i promise, it's mostly stubs!!!).

@jleandroperez jleandroperez added status: do not merge Dependent on another PR, ready for review but not ready for merge. [Status] Not Ready for Review labels May 24, 2018
@jleandroperez jleandroperez self-assigned this May 24, 2018
return true
}

func application(_ app: UIApplication, open url: URL, options: [UIApplicationOpenURLOptionsKey : Any] = [:]) -> Bool {

Choose a reason for hiding this comment

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

Colon Violation: Colons should be next to the identifier when specifying a type and next to the key in dictionary literals. (colon)

@jleandroperez jleandroperez added feature: login Related to any part of the log in or sign in flow, or authentication. and removed status: do not merge Dependent on another PR, ready for review but not ready for merge. [Status] Not Ready for Review labels May 25, 2018
@astralbodies astralbodies mentioned this pull request May 25, 2018
7 tasks
@mindgraffiti
Copy link
Contributor

mindgraffiti commented May 25, 2018

Tested: (things not marked with a checkmark failed testing)

  • Log into a WordPress.com account (Email + Password)
  • Log into a WordPress.com account (Username + Password + 2FA)
  • Log into a WordPress.com account (Username + Password + SMS Token)
  • Log into a WordPress.com account (Magic Link) - fails to dismiss "Your magic link is on its way!" or do something else after tapping Magic Link in email. (I can go back but not forwards)
  • Log into a WordPress.com account (With Google!)
  • Log into a WordPress.com account (1Password support)

@mindgraffiti
Copy link
Contributor

Magic link console error output 2018-05-25 16:03:10.390140-0500 WooCommerce[39278:3081435] Received XPC error Connection invalid for message type 3

Copy link
Contributor

@mindgraffiti mindgraffiti left a comment

Choose a reason for hiding this comment

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

Found some potential bugs.

@jleandroperez
Copy link
Collaborator Author

jleandroperez commented May 28, 2018

Issue description updated: it should have never said Username (but instead Email, since username isn't really supported in this iteration!).

Let's regroup on tuesday + debug the rest, thanks Thuy!

@mindgraffiti
Copy link
Contributor

Removed (username + password) as a failed test, since it's not applicable

@mindgraffiti
Copy link
Contributor

Bug fix verified. Merge whenever you're ready 🎉

@jleandroperez
Copy link
Collaborator Author

Thank you Thuy!!

@jleandroperez jleandroperez merged commit 10fcb49 into develop May 29, 2018
@jleandroperez jleandroperez deleted the issue/22-authentication branch May 29, 2018 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: login Related to any part of the log in or sign in flow, or authentication.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants