Checking an authentication decision in a JSON response

Question

I've been working on this login screen logic.

It checks for the parameters in the JSON response to decide which action to follow. It works but it's kinda ugly and verbose. Any suggestions to refactor to make it look more elegant and simple?

 Alamofire.request(apiLink, method: .post, parameters: parameters, encoding: URLEncoding.default, headers: nil)
 .responseJSON { response in
 guard response.result.error == nil else {
 print("error calling POST in /###/###/###")
 print("Error: \(response.result.error)")
 return
 }
 if let JSON = response.result.value {
 print(response.result)
 print("JSON: \(JSON)")
 if let dictionary = response.result.value as? [String: Any] {
 if let status = dictionary["status"] as? String {
 if status == "ok" {
 self.performSegue(withIdentifier: "Home", sender: nil)
 } else {
 if let message = dictionary["message"] as? String {
 if message == "invalid_username" {
 let alertMessage = UIAlertController(title: "Invalid User", message: "Please check out the user name", preferredStyle: .alert)
 alertMessage.addAction(UIAlertAction(title: "Ok", style: .default, handler: nil))
 self.present(alertMessage, animated: true, completion: nil)
 } else {
 if let message = dictionary["message"] as? String {
 if message == "incorrect_password" {
 let alertMessage = UIAlertController(title: "Invalid Password", message: "Please check out the password", preferredStyle: .alert)
 alertMessage.addAction(UIAlertAction(title: "Ok", style: .default, handler: nil))
 self.present(alertMessage, animated: true, completion: nil)
 }
 }
 }
 }
 }
 }
 }
 }
 }
 }

• oopexpert: You should be careful with giving hints about reasons, why a login could not be processed. Knowing that the password is wrong but the the username is right is a valueable information for an attacker.

Answer 1

Guard Statements

My proposed changes takes advantage of Swift's guard statements. In this case I find them useful not only for breaking up the nested if statements, but also for clarifying the "requirements" or key conditions and increasing the readability for others and yourself.

For example:

guard let dictionary = json as? [String: Any] else {
 return
}

Can be interpreted as:

"This dictionary must be valid or else we cannot continue"

The alternative to this is using a traditional if statement. However, this can lead to additional nesting and potentially also requires you to handle the alternative conditions, or in other words the else and else if conditions.

if let dictionary as? [String: Any] else {
 // I need another if statement to get something from the dictionary, but what if that key is not in the dictionary?
}
// Do I need an else condition? What if the dictionary is not valid?

Repetitive Code

The use of the guard statements also had the effect of eliminating some repetitive code used for setting up the UIAlertController. Repetitive code can lead to additional maintenance overhead and also bugs where you make a change in one place but forgot to replicate it in the duplicate code.

Unused Constant

I also made use of the json constant you declared. It seems that after it was declared, you did not use it in the block that follows.

Additional note

My proposed changes also incorporates oopexpert's advice. From a security standpoint, it would be better to not be specific as to why a login attempt has failed.

Proposed Refactoring

Alamofire.request(apiLink, method: .post, parameters: parameters, encoding: URLEncoding.default, headers: nil).responseJSON {
 response in
 guard response.result.error == nil else {
 print("error calling POST in /###/###/###")
 print("Error: \(response.result.error)")
 return
 }
 if let json = response.result.value {
 print(response.result)
 print("JSON: \(json)")
 guard let dictionary = json as? [String: Any] else {
 return
 }
 guard let status = dictionary["status"] as? String, let message = dictionary["message"] as? String else {
 return
 }
 guard status == "ok" else {
 if message == "invalid_login" {
 let alertMessage = UIAlertController(title: "Unable to login", message: "Please try again", preferredStyle: .alert)
 alertMessage.addAction(UIAlertAction(title: "Ok", style: .default, handler: nil))
 self.present(alertMessage, animated: true, completion: nil)
 }
 return
 }
 self.performSegue(withIdentifier: "Home", sender: nil)
 }
}

Answer 2

You could do something like this to eliminate one if statement :

if let status = dictionary["status"] as? String where status == "ok" {
 self.performSegue(withIdentifier: "Home", sender: nil)
}

Also SwiftyJSON might be your friend!