From 018699a555019d365c4c6580eef0ed86cfd9db75 Mon Sep 17 00:00:00 2001 From: Nate Heagy Date: Fri, 20 Oct 2017 10:53:19 -0600 Subject: [PATCH 1/7] Fix spinner behaviour when logging in with Google --- .../Classes/ViewRelated/NUX/LoginEmailViewController.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift b/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift index aaf35781a9b4..8ec4d9c50017 100644 --- a/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift +++ b/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift @@ -158,6 +158,8 @@ class LoginEmailViewController: LoginViewController, SigninKeyboardResponder { func googleLoginTapped() { awaitingGoogle = true + configureViewLoading(true) + GIDSignIn.sharedInstance().disconnect() // Flag this as a social sign in. @@ -329,6 +331,7 @@ class LoginEmailViewController: LoginViewController, SigninKeyboardResponder { if awaitingGoogle { awaitingGoogle = false + configureViewLoading(false) 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 socialErrorNav = LoginNavigationController(rootViewController: socialErrorVC) @@ -458,6 +461,7 @@ extension LoginEmailViewController: GIDSignInDelegate { let email = user.profile.email else { // The Google SignIn for may have been canceled. //TODO: Add analytis + configureViewLoading(false) return } @@ -466,8 +470,6 @@ extension LoginEmailViewController: GIDSignInDelegate { loginFields.username = email loginFields.meta.socialServiceIDToken = token - configureViewLoading(true) - loginFacade.loginToWordPressDotCom(withGoogleIDToken: token) //TODO: Add analytis From ed3dc24fac0b4b68aad1c42e4528cf3a643f1f6f Mon Sep 17 00:00:00 2001 From: Nate Heagy Date: Fri, 20 Oct 2017 10:53:45 -0600 Subject: [PATCH 2/7] Disconnect from Google accounts when error encountered to allow reattempt --- WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift b/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift index 8ec4d9c50017..49474a4e00ba 100644 --- a/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift +++ b/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift @@ -332,6 +332,7 @@ class LoginEmailViewController: LoginViewController, SigninKeyboardResponder { if awaitingGoogle { awaitingGoogle = false configureViewLoading(false) + GIDSignIn.sharedInstance().disconnect() 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 socialErrorNav = LoginNavigationController(rootViewController: socialErrorVC) From 4f60ebd1ab30efedb9e6563adc63c7e99d1e6220 Mon Sep 17 00:00:00 2001 From: Nate Heagy Date: Fri, 20 Oct 2017 14:08:58 -0600 Subject: [PATCH 3/7] Improve error handling for Google login flow --- .../NUX/LoginEmailViewController.swift | 13 +++- .../WordPressComOAuthClient.swift | 60 ++++++++----------- 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift b/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift index 49474a4e00ba..8ba9ebf6fd2d 100644 --- a/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift +++ b/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift @@ -331,10 +331,19 @@ class LoginEmailViewController: LoginViewController, SigninKeyboardResponder { if awaitingGoogle { awaitingGoogle = false - configureViewLoading(false) GIDSignIn.sharedInstance().disconnect() - 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 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:"D 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: errorTitle, description: errorDescription) let socialErrorNav = LoginNavigationController(rootViewController: socialErrorVC) socialErrorVC.delegate = self present(socialErrorNav, animated: true) {} diff --git a/WordPressKit/WordPressKit/WordPressComOAuthClient.swift b/WordPressKit/WordPressKit/WordPressComOAuthClient.swift index 131146806023..d287a391c7ed 100644 --- a/WordPressKit/WordPressKit/WordPressComOAuthClient.swift +++ b/WordPressKit/WordPressKit/WordPressComOAuthClient.swift @@ -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. @@ -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 ] @@ -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), + 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: String = "" + var errorDescription: String = "" + var newNonce: String? = nil + + // 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: responseObject, newNonce: newNonce) + } /// Creates an NSError from the supplied arguements. The response object is /// added to the error's userInfo dictionary. From 5eff8267fa856c666acc4cdec4d668cd03e94a2b Mon Sep 17 00:00:00 2001 From: Nate Heagy Date: Fri, 20 Oct 2017 15:03:51 -0600 Subject: [PATCH 4/7] Cleanup spacing to silence Hound --- .../Classes/ViewRelated/NUX/LoginEmailViewController.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift b/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift index 8ba9ebf6fd2d..983e17c4598c 100644 --- a/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift +++ b/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift @@ -336,8 +336,8 @@ class LoginEmailViewController: LoginViewController, SigninKeyboardResponder { 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:"D shown when a user logs in with Google but no matching WordPress.com account is found") + 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: "D 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 From 40db9c739990e1dea461a3f1f5e30611567408cb Mon Sep 17 00:00:00 2001 From: Nate Heagy Date: Fri, 20 Oct 2017 15:04:53 -0600 Subject: [PATCH 5/7] Removed a ! that Xcode was certain I needed earlier. --- .../Classes/ViewRelated/NUX/LoginEmailViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift b/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift index 983e17c4598c..45d76730fb76 100644 --- a/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift +++ b/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift @@ -335,7 +335,7 @@ class LoginEmailViewController: LoginViewController, SigninKeyboardResponder { let errorTitle: String let errorDescription: String - if (error as! NSError).code == WordPressComOAuthError.unknownUser.rawValue { + 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: "D shown when a user logs in with Google but no matching WordPress.com account is found") } else { From 0c0d7885a35712239d4193deb32669b360606d37 Mon Sep 17 00:00:00 2001 From: Nate Heagy Date: Sat, 21 Oct 2017 22:18:54 -0600 Subject: [PATCH 6/7] Send correct responseDict to errorFor in WordPressComOAuthResponseSerializer --- WordPressKit/WordPressKit/WordPressComOAuthClient.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPressKit/WordPressKit/WordPressComOAuthClient.swift b/WordPressKit/WordPressKit/WordPressComOAuthClient.swift index d287a391c7ed..54e925c73bd5 100644 --- a/WordPressKit/WordPressKit/WordPressComOAuthClient.swift +++ b/WordPressKit/WordPressKit/WordPressComOAuthClient.swift @@ -393,7 +393,7 @@ final class WordPressComOAuthResponseSerializer: AFJSONResponseSerializer { newNonce = data["two_step_nonce"] as? String } - return errorFor(errorCode: errorCode, errorDescription: errorDescription, responseObject: responseObject, newNonce: newNonce) + return errorFor(errorCode: errorCode, errorDescription: errorDescription, responseObject: responseDict, newNonce: newNonce) } /// Creates an NSError from the supplied arguements. The response object is From 38110b28d1f56c4937fb217e5f7c49fc05e57b4d Mon Sep 17 00:00:00 2001 From: Nate Heagy Date: Sat, 21 Oct 2017 22:36:03 -0600 Subject: [PATCH 7/7] Cleanup minor issues in google login code --- .../Classes/ViewRelated/NUX/LoginEmailViewController.swift | 2 +- WordPressKit/WordPressKit/WordPressComOAuthClient.swift | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift b/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift index 45d76730fb76..9b78e387b5ad 100644 --- a/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift +++ b/WordPress/Classes/ViewRelated/NUX/LoginEmailViewController.swift @@ -337,7 +337,7 @@ class LoginEmailViewController: LoginViewController, SigninKeyboardResponder { 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: "D 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 diff --git a/WordPressKit/WordPressKit/WordPressComOAuthClient.swift b/WordPressKit/WordPressKit/WordPressComOAuthClient.swift index 54e925c73bd5..04cb67ae3e91 100644 --- a/WordPressKit/WordPressKit/WordPressComOAuthClient.swift +++ b/WordPressKit/WordPressKit/WordPressComOAuthClient.swift @@ -377,9 +377,9 @@ final class WordPressComOAuthResponseSerializer: AFJSONResponseSerializer { /// Create the NSError from the response dictionary private func parseError(from responseDict: [String: AnyObject]) -> NSError { - var errorCode: String = "" - var errorDescription: String = "" - var newNonce: String? = nil + var errorCode = "" + var errorDescription = "" + var newNonce: String? // there's either a data object, or an error. if let errorStr = responseDict["error"] as? String {