Generic error code is returned for all authorization (#321)

* Fix error returning discovery document * Add status code for .unexpectedAuthCodeResponse * Refactoring of APIError * Argument renaming underlyingError * Minor fix * Handle error and error_description parameters for authorization response * Handle error and error_description parameters for token response * Support CustomNSError * Fix error returning * Minor fix * Add tests for errors * Minor updates for example project. Update documentation * Fix comments
okta · Dec 2, 2021 · 7a738fc · 7a738fc
1 parent ed43488
commit 7a738fc
Show file tree

Hide file tree

Showing 14 changed files with 362 additions and 112 deletions.
diff --git a/Example/AuthViewController.swift b/Example/AuthViewController.swift
@@ -46,8 +46,9 @@ final class AuthViewController: UIViewController {
         showProgress()
         oktaAppAuth?.authenticate(withSessionToken: token) { authStateManager, error in
             self.hideProgress()
+
             if let error = error {
-                self.presentError(error)
+                self.showMessage(error)
                 return
             }
 
@@ -56,8 +57,12 @@ final class AuthViewController: UIViewController {
         }
     }
 
-    func presentError(_ error: Error) {
-        self.showMessage("Error: \(error.localizedDescription)")
+    func showMessage(_ error: Error) {
+        if let oidcError = error as? OktaOidcError {
+            messageView.text = oidcError.displayMessage
+        } else {
+            messageView.text = error.localizedDescription
+        }
     }
 
     func showMessage(_ message: String) {

diff --git a/Example/ViewController.swift b/Example/ViewController.swift
@@ -65,7 +65,7 @@ final class ViewController: UIViewController {
         super.viewWillAppear(animated)
 
         guard oktaAppAuth != nil else {
-            self.updateUI(updateText: "SDK is not configured!")
+            self.showMessage("SDK is not configured!")
             return
         }
 
@@ -103,14 +103,14 @@ final class ViewController: UIViewController {
     @IBAction func userInfoButton(_ sender: Any) {
         authStateManager?.getUser() { response, error in
             if let error = error {
-                self.updateUI(updateText: "Error: \(error)")
+                self.showMessage(error)
                 return
             }
 
             if response != nil {
                 var userInfoText = ""
                 response?.forEach { userInfoText += ("\($0): \($1) \n") }
-                self.updateUI(updateText: userInfoText)
+                self.showMessage(userInfoText)
             }
         }
     }
@@ -121,11 +121,11 @@ final class ViewController: UIViewController {
 
         authStateManager?.introspect(token: accessToken, callback: { payload, error in
             guard let isValid = payload?["active"] as? Bool else {
-                self.updateUI(updateText: "Error: \(error?.localizedDescription ?? "Unknown")")
+                self.showMessage("Error: \(error?.localizedDescription ?? "Unknown")")
                 return
             }
 
-            self.updateUI(updateText: "Is the AccessToken valid? - \(isValid)")
+            self.showMessage("Is the AccessToken valid? - \(isValid)")
         })
     }
 
@@ -134,16 +134,16 @@ final class ViewController: UIViewController {
         guard let accessToken = authStateManager?.accessToken else { return }
 
         authStateManager?.revoke(accessToken) { _, error in
-            if error != nil { self.updateUI(updateText: "Error: \(error!)") }
-            self.updateUI(updateText: "AccessToken was revoked")
+            if error != nil { self.showMessage("Error: \(error!)") }
+            self.showMessage("AccessToken was revoked")
         }
     }
 
     func signInWithBrowser() {
         oktaAppAuth?.signInWithBrowser(from: self) { authStateManager, error in
             if let error = error {
                 self.authStateManager = nil
-                self.updateUI(updateText: "Error: \(error)")
+                self.showMessage("Error: \(error.localizedDescription)")
                 return
             }
 
@@ -158,9 +158,9 @@ final class ViewController: UIViewController {
         oktaAppAuth?.signOut(authStateManager: authStateManager, from: self, progressHandler: { currentOption in
             switch currentOption {
             case .revokeAccessToken, .revokeRefreshToken, .removeTokensFromStorage, .revokeTokensOptions:
-                self.updateUI(updateText: "Revoking tokens...")
+                self.showMessage("Revoking tokens...")
             case .signOutFromOkta:
-                self.updateUI(updateText: "Signing out from Okta...")
+                self.showMessage("Signing out from Okta...")
             default:
                 break
             }
@@ -169,13 +169,21 @@ final class ViewController: UIViewController {
                 self.authStateManager = nil
                 self.buildTokenTextView()
             } else {
-                self.updateUI(updateText: "Error: failed to logout")
+                self.showMessage("Error: failed to logout")
             }
         })
     }
-
-    func updateUI(updateText: String) {
-        tokenView.text = updateText
+
+    func showMessage(_ message: String) {
+        tokenView.text = message
+    }
+
+    func showMessage(_ error: Error) {
+        if let oidcError = error as? OktaOidcError {
+            tokenView.text = oidcError.displayMessage
+        } else {
+            tokenView.text = error.localizedDescription
+        }
     }
 
     func buildTokenTextView() {
@@ -197,7 +205,7 @@ final class ViewController: UIViewController {
             tokenString += "\nRefresh Token: \(refreshToken)\n"
         }
 
-        self.updateUI(updateText: tokenString)
+        self.showMessage(tokenString)
     }
 }
 
@@ -215,3 +223,30 @@ extension ViewController: OktaNetworkRequestCustomizationDelegate {
         }
     }
 }
+
+extension OktaOidcError {
+    var displayMessage: String {
+        switch self {
+        case let .api(message, _):
+            switch (self as NSError).code {
+            case NSURLErrorNotConnectedToInternet,
+                NSURLErrorNetworkConnectionLost,
+                NSURLErrorCannotLoadFromNetwork,
+                NSURLErrorCancelled:
+                return "No Internet Connection"
+            case NSURLErrorTimedOut:
+                return "Connection timed out"
+            default:
+                break
+            }
+
+            return "API Error occurred: \(message)"
+        case let .authorization(error, _):
+            return "Authorization error: \(error)"
+        case let .unexpectedAuthCodeResponse(statusCode):
+            return "Authorization failed due to incorrect status code: \(statusCode)"
+        default:
+            return localizedDescription
+        }
+    }
+}
diff --git a/README.md b/README.md
@@ -38,6 +38,7 @@ You can learn more on the [Okta + iOS](https://developer.okta.com/code/ios/) pag
 - [Development](#development)
   - [Running Tests](#running-tests)
 - [Modify network requests](#modify-network-requests)
+- [Migration](#migration)
 - [Known issues](#known-issues)
 - [Contributing](#contributing)
 
@@ -507,6 +508,20 @@ extension SomeNSObject: OktaNetworkRequestCustomizationDelegate {
 
 ***Note:*** It is highly recommended to copy all of the existing parameters from the original URLRequest object to modified request without any changes. Altering of this data could lead network request to fail. If `customizableURLRequest(_:)` method returns `nil` default request will be used.
 
+## Migration
+
+### Migrating from 3.10.x to 3.11.x
+
+The SDK `okta-oidc-ios` has a major changes in error handling. Consider these guidelines to update your code.
+
+- `APIError` is renamed as `api`.
+- `api` error has the additional parameter `underlyingError`, it's an optional and indicates the origin of the error.
+- Introduced a new error `authorization(error:description:)`.
+- `authorization` error appears when authorization server fails due to errors during authorization.
+- `unexpectedAuthCodeResponse(statusCode:)` has an error code parameter.
+- `OktaOidcError` conforms to `CustomNSError` protocol. It means you can convert the error to `NSError` and get `code`, `userInfo`, `domain`, `underlyingErrors`.
+- `OktaOidcError` conforms to `Equatable` protocol. The errors can be compared for equality using the operator `==` or inequality using the operator `!=`.
+
 ## Known issues
 
 ### iOS shows permission dialog(`{App} Wants to Use {Auth Domain} to Sign In`) for Okta Sign Out flows

diff --git a/Sources/OktaOidc/Common/Internal/OIDAuthState+Okta.swift b/Sources/OktaOidc/Common/Internal/OIDAuthState+Okta.swift
@@ -17,29 +17,37 @@ import OktaOidc_AppAuth
 // Okta Extension of OIDAuthState
 extension OKTAuthState {
 
-    static func getState(withAuthRequest authRequest: OKTAuthorizationRequest, delegate: OktaNetworkRequestCustomizationDelegate? = nil, callback: @escaping (OKTAuthState?, OktaOidcError?) -> Void ) {
+    static func getState(withAuthRequest authRequest: OKTAuthorizationRequest, delegate: OktaNetworkRequestCustomizationDelegate? = nil, callback finalize: @escaping (OKTAuthState?, OktaOidcError?) -> Void ) {
 
-        let finalize: ((OKTAuthState?, OktaOidcError?) -> Void) = { state, error in
-            callback(state, error)
-        }
-
         // Make authCode request
-        OKTAuthorizationService.perform(authRequest: authRequest, delegate: delegate, callback: { authResponse, error in
+        OKTAuthorizationService.perform(authRequest: authRequest, delegate: delegate) { authResponse, error in
             guard let authResponse = authResponse else {
-                finalize(nil, OktaOidcError.APIError("Authorization Error: \(error?.localizedDescription ?? "No authentication response.")"))
+                finalize(nil, .api(message: "Authorization Error: \(error?.localizedDescription ?? "No authentication response.")", underlyingError: error))
                 return
             }
-
+
+            if let oauthError = authResponse.additionalParameters?[OKTOAuthErrorFieldError] as? String {
+                let oauthErrorDescription = authResponse.additionalParameters?[OKTOAuthErrorFieldErrorDescription] as? String
+                finalize(nil, .authorization(error: oauthError, description: oauthErrorDescription))
+                return
+            }
+
             guard authResponse.authorizationCode != nil,
                   let tokenRequest = authResponse.tokenExchangeRequest() else {
-                    finalize(nil, OktaOidcError.unableToGetAuthCode)
+                    finalize(nil, .unableToGetAuthCode)
                     return
             }
 
             // Make token request
-            OKTAuthorizationService.perform(tokenRequest, originalAuthorizationResponse: authResponse, delegate: delegate, callback: { tokenResponse, error in
+            OKTAuthorizationService.perform(tokenRequest, originalAuthorizationResponse: authResponse, delegate: delegate) { tokenResponse, error in
                 guard let tokenResponse = tokenResponse else {
-                    finalize(nil, OktaOidcError.APIError("Authorization Error: \(error?.localizedDescription ?? "No token response.")"))
+                    finalize(nil, OktaOidcError.api(message: "Authorization Error: \(error?.localizedDescription ?? "No token response.")", underlyingError: error))
+                    return
+                }
+
+                if let oauthError = tokenResponse.additionalParameters?[OKTOAuthErrorFieldError] as? String {
+                    let oauthErrorDescription = tokenResponse.additionalParameters?[OKTOAuthErrorFieldErrorDescription] as? String
+                    finalize(nil, .authorization(error: oauthError, description: oauthErrorDescription))
                     return
                 }
 
@@ -48,7 +56,7 @@ extension OKTAuthState {
                                              registrationResponse: nil,
                                              delegate: delegate)
                 finalize(authState, nil)
-            })
-        })
+            }
+        }
     }
 }
diff --git a/Sources/OktaOidc/Common/Internal/OIDAuthorizationService+Okta.swift b/Sources/OktaOidc/Common/Internal/OIDAuthorizationService+Okta.swift
@@ -35,33 +35,43 @@ extension OKTAuthorizationService {
 
             delegate?.didReceive(response)
             guard let response = response as? HTTPURLResponse else {
-                callback(nil, error)
+                callback(nil, OktaOidcError.api(message: "Authentication Error: No response", underlyingError: error))
                 return
             }
 
-            guard response.statusCode == 302,
-                  let locationHeader = response.allHeaderFields["Location"] as? String,
-                  let urlComonents = URLComponents(string: locationHeader),
-                  let queryItems = urlComonents.queryItems else {
-                    callback(nil, OktaOidcError.unexpectedAuthCodeResponse)
-                    return
+            guard response.statusCode == 302 else {
+                callback(nil, OktaOidcError.unexpectedAuthCodeResponse(statusCode: response.statusCode))
+                return
             }
+
+            guard let locationHeader = response.allHeaderFields["Location"] as? String,
+                  let urlComponents = URLComponents(string: locationHeader),
+                  let queryItems = urlComponents.queryItems else {
+                      callback(nil, OktaOidcError.noLocationHeader)
+                      return
+                  }
 
-            var parameters = [String: NSString]()
-            queryItems.forEach({ item in
+            var parameters: [String: NSString] = [:]
+            queryItems.forEach { item in
                 parameters[item.name] = item.value as NSString?
-            })
-
-            if let allHeaderFields = response.allHeaderFields as? [String: String],
-               let url = response.url {
-                let httpCookies = HTTPCookie.cookies(withResponseHeaderFields: allHeaderFields, for: url)
-                for cookie in httpCookies {
-                    HTTPCookieStorage.shared.setCookie(cookie)
-                }
             }
-
+            
             let authResponse = OKTAuthorizationResponse(request: authRequest, parameters: parameters)
+
+            setCookie(from: response)
+
             callback(authResponse, error)
         }.resume()
     }
+
+    private static func setCookie(from response: HTTPURLResponse) {
+        guard let allHeaderFields = response.allHeaderFields as? [String: String],
+              let url = response.url else {
+                  return
+              }
+
+        HTTPCookie.cookies(withResponseHeaderFields: allHeaderFields, for: url).forEach {
+            HTTPCookieStorage.shared.setCookie($0)
+        }
+    }
 }
diff --git a/Sources/OktaOidc/Common/Internal/OktaOidcRestApi.swift b/Sources/OktaOidc/Common/Internal/OktaOidcRestApi.swift
@@ -28,14 +28,14 @@ class OktaOidcRestApi: OktaOidcHttpApiProtocol {
                   let httpResponse = response as? HTTPURLResponse else {
                 let errorMessage = error?.localizedDescription ?? "No response data"
                 DispatchQueue.main.async {
-                    onError(OktaOidcError.APIError(errorMessage))
+                    onError(OktaOidcError.api(message: errorMessage, underlyingError: error))
                 }
                 return
             }
 
             guard 200 ..< 300 ~= httpResponse.statusCode else {
                 DispatchQueue.main.async {
-                    onError(OktaOidcError.APIError(HTTPURLResponse.localizedString(forStatusCode: httpResponse.statusCode)))
+                    onError(OktaOidcError.api(message: HTTPURLResponse.localizedString(forStatusCode: httpResponse.statusCode), underlyingError: nil))
                 }
                 return
             }

diff --git a/Sources/OktaOidc/Common/Internal/Tasks/OktaOidcBrowserTask.swift b/Sources/OktaOidc/Common/Internal/Tasks/OktaOidcBrowserTask.swift
@@ -40,7 +40,7 @@ class OktaOidcBrowserTask: OktaOidcTask {
                                                   responseType: OKTResponseTypeCode,
                                                   additionalParameters: self.config.additionalParams)
             guard let externalUserAgent = self.externalUserAgent() else {
-                callback(nil, OktaOidcError.APIError("Authorization Error: \(error?.localizedDescription ?? "No external User Agent.")"))
+                callback(nil, OktaOidcError.api(message: "Authorization Error: \(error?.localizedDescription ?? "No external User Agent.")", underlyingError: nil))
                 return
             }
 
@@ -49,17 +49,22 @@ class OktaOidcBrowserTask: OktaOidcTask {
                                                                    delegate: delegate) { authorizationResponse, error in
                 defer { self.userAgentSession = nil }
 
-                guard let authResponse = authorizationResponse else {
-                    guard let error = error else {
-                        return callback(nil, OktaOidcError.APIError("Authorization Error"))
-                    }
-                    if (error as NSError).code == OKTErrorCode.userCanceledAuthorizationFlow.rawValue {
-                        return callback(nil, OktaOidcError.userCancelledAuthorizationFlow)
-                    } else {
-                        return callback(nil, OktaOidcError.APIError("Authorization Error: \(error.localizedDescription)"))
-                    }
+                if let authResponse = authorizationResponse {
+                    callback(authResponse, nil)
+                    return
                 }
-                callback(authResponse, nil)
+
+                guard let error = error else {
+                    callback(nil, OktaOidcError.api(message: "Authorization Error: No authorization response", underlyingError: nil))
+                    return
+                }
+
+                if (error as NSError).code == OKTErrorCode.userCanceledAuthorizationFlow.rawValue {
+                    callback(nil, OktaOidcError.userCancelledAuthorizationFlow)
+                    return
+                }
+
+                return callback(nil, OktaOidcError.api(message: "Authorization Error: \(error.localizedDescription)", underlyingError: error))
             }
             self.userAgentSession = userAgentSession
         }
@@ -83,7 +88,7 @@ class OktaOidcBrowserTask: OktaOidcTask {
                                                postLogoutRedirectURL: successRedirectURL,
                                                additionalParameters: self.config.additionalParams)
             guard let externalUserAgent = self.externalUserAgent() else {
-                callback(nil, OktaOidcError.APIError("Authorization Error: \(error?.localizedDescription ?? "No external User Agent.")"))
+                callback(nil, OktaOidcError.api(message: "Authorization Error: \(error?.localizedDescription ?? "No external User Agent.")", underlyingError: nil))
                 return
             }
 
@@ -92,7 +97,7 @@ class OktaOidcBrowserTask: OktaOidcTask {
 
                 var error: OktaOidcError?
                 if let responseError = responseError {
-                    error = OktaOidcError.APIError("Sign Out Error: \(responseError.localizedDescription)")
+                    error = OktaOidcError.api(message: "Sign Out Error: \(responseError.localizedDescription)", underlyingError: nil)
                 }
 
                 callback((), error)