Skip to content

Login: Several small UX fixes for Google login #8022

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.

Sign up for GitHub

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

Merged
nheagy merged 7 commits into from
Oct 23, 2017
Merged

Login: Several small UX fixes for Google login #8022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
018699a
Fix spinner behaviour when logging in with Google
nheagy Oct 20, 2017
ed3dc24
Disconnect from Google accounts when error encountered to allow reatt…
nheagy Oct 20, 2017
4f60ebd
Improve error handling for Google login flow
nheagy Oct 20, 2017
5eff826
Cleanup spacing to silence Hound
nheagy Oct 20, 2017
40db9c7
Removed a ! that Xcode was certain I needed earlier.
nheagy Oct 20, 2017
0c0d788
Send correct responseDict to errorFor in WordPressComOAuthResponseSer…
nheagy Oct 22, 2017
38110b2
Cleanup minor issues in google login code
nheagy Oct 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ class LoginEmailViewController: LoginViewController, SigninKeyboardResponder {

func googleLoginTapped() {
awaitingGoogle = true
configureViewLoading(true)

GIDSignIn.sharedInstance().disconnect()

// Flag this as a social sign in.
Expand Down Expand Up @@ -329,8 +331,19 @@ class LoginEmailViewController: LoginViewController, SigninKeyboardResponder {

if awaitingGoogle {
awaitingGoogle = false
GIDSignIn.sharedInstance().disconnect()

let errorTitle: String
let errorDescription: String
if (error as NSError).code == WordPressComOAuthError.unknownUser.rawValue {
errorTitle = NSLocalizedString("Connected But…", comment: "Title shown when a user logs in with Google but no matching WordPress.com account is found")
errorDescription = NSLocalizedString("The Google account \"\(loginFields.username)\" doesn't match any account on WordPress.com", comment: "Description shown when a user logs in with Google but no matching WordPress.com account is found")
} else {
errorTitle = NSLocalizedString("Unable To Connect", comment: "Shown when a user logs in with Google but it subsequently fails to work as login to WordPress.com")
errorDescription = error.localizedDescription
}

let socialErrorVC = LoginSocialErrorViewController(title: NSLocalizedString("Unable To Connect", comment: "Shown when a user logs in with Google but it subsequently fails to work as login to WordPress.com"), description: error.localizedDescription)
let socialErrorVC = LoginSocialErrorViewController(title: errorTitle, description: errorDescription)
let socialErrorNav = LoginNavigationController(rootViewController: socialErrorVC)
socialErrorVC.delegate = self
present(socialErrorNav, animated: true) {}
Expand Down Expand Up @@ -458,6 +471,7 @@ extension LoginEmailViewController: GIDSignInDelegate {
let email = user.profile.email else {
// The Google SignIn for may have been canceled.
//TODO: Add analytis
configureViewLoading(false)
return
}

Expand All @@ -466,8 +480,6 @@ extension LoginEmailViewController: GIDSignInDelegate {
loginFields.username = email
loginFields.meta.socialServiceIDToken = token

configureViewLoading(true)

loginFacade.loginToWordPressDotCom(withGoogleIDToken: token)

//TODO: Add analytis
Expand Down
60 changes: 26 additions & 34 deletions WordPressKit/WordPressKit/WordPressComOAuthClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import CocoaLumberjack
case invalidOneTimePassword
case socialLoginExistingUserUnconnected
case invalidTwoStepCode
case unknownUser
}

/// `WordPressComOAuthClient` encapsulates the pattern of authenticating against WordPress.com OAuth2 service.
Expand Down Expand Up @@ -344,7 +345,8 @@ final class WordPressComOAuthResponseSerializer: AFJSONResponseSerializer {
"needs_2fa": WordPressComOAuthError.needsMultifactorCode,
"invalid_otp": WordPressComOAuthError.invalidOneTimePassword,
"user_exists": WordPressComOAuthError.socialLoginExistingUserUnconnected,
"invalid_two_step_code": WordPressComOAuthError.invalidTwoStepCode
"invalid_two_step_code": WordPressComOAuthError.invalidTwoStepCode,
"unknown_user": WordPressComOAuthError.unknownUser
]


Expand All @@ -365,44 +367,34 @@ final class WordPressComOAuthResponseSerializer: AFJSONResponseSerializer {
return responseObject
}

// Handle known 400 errors.
if httpResponse.statusCode == 400 {
// REST API Error format
if let responseDictionary = responseObject as? [String: AnyObject],
let errorCode = responseDictionary["error"] as? String,
let errorDescription = responseDictionary["error_description"] as? String {

error?.pointee = errorFor(errorCode: errorCode, errorDescription: errorDescription, responseObject: responseObject)
}

} else if httpResponse.statusCode == 409 {
// Social login user-exists error
if let responseDict = responseObject as? [String: AnyObject],
let data = responseDict["data"] as? [String: AnyObject],
let errors = data["errors"] as? NSArray,
let err = errors[0] as? [String: AnyObject],
let errorCode = err["code"] as? String,
let errorDescription = err["message"] as? String {

error?.pointee = errorFor(errorCode: errorCode, errorDescription: errorDescription, responseObject: responseObject)
}
} else if httpResponse.statusCode == 403 {
// Social 2FA token wasn't good
if let responseDict = responseObject as? [String: AnyObject],
let data = responseDict["data"] as? [String: AnyObject],
let newNonce = data["two_step_nonce"] as? String,
let errors = data["errors"] as? NSArray,
let err = errors[0] as? [String: AnyObject],
let errorCode = err["code"] as? String,
let errorDescription = err["message"] as? String {

error?.pointee = errorFor(errorCode: errorCode, errorDescription: errorDescription, responseObject: responseObject, newNonce: newNonce)
}
if [400, 409, 403].contains(httpResponse.statusCode),

@aerych aerych Oct 21, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So nice :)

let responseDictionary = responseObject as? [String: AnyObject] {
error?.pointee = parseError(from: responseDictionary)
}

return responseObject as AnyObject?
}

/// Create the NSError from the response dictionary
private func parseError(from responseDict: [String: AnyObject]) -> NSError {
var errorCode = ""
var errorDescription = ""
var newNonce: String?

// there's either a data object, or an error.
if let errorStr = responseDict["error"] as? String {
errorCode = errorStr
errorDescription = responseDict["error_description"] as? String ?? ""
} else if let data = responseDict["data"] as? [String: AnyObject],
let errors = data["errors"] as? NSArray,
let err = errors[0] as? [String: AnyObject] {
errorCode = err["code"] as? String ?? ""
errorDescription = err["message"] as? String ?? ""
newNonce = data["two_step_nonce"] as? String
}

return errorFor(errorCode: errorCode, errorDescription: errorDescription, responseObject: responseDict, newNonce: newNonce)
}

/// Creates an NSError from the supplied arguements. The response object is
/// added to the error's userInfo dictionary.
Expand Down