-
Notifications
You must be signed in to change notification settings - Fork 16
Use URLSession in WordPressOAuthClient #660
Changes from all commits
911013c
0734426
204a387
1ad9261
a59263c
5c98260
72451fa
a83f361
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,17 @@ public struct AuthenticationFailure: LocalizedError { | |
| public var newNonce: String? | ||
| public var originalErrorJSON: [String: AnyObject] | ||
|
|
||
| init?(response: HTTPURLResponse, body: Data) { | ||
| guard [400, 409, 403].contains(response.statusCode), | ||
| let responseObject = try? JSONSerialization.jsonObject(with: body, options: .allowFragments), | ||
| let responseDictionary = responseObject as? [String: AnyObject] | ||
| else { | ||
| return nil | ||
| } | ||
|
|
||
| self.init(apiJSONResponse: responseDictionary) | ||
| } | ||
|
|
||
| init(apiJSONResponse responseDict: [String: AnyObject]) { | ||
| originalErrorJSON = responseDict | ||
|
|
||
|
|
@@ -78,15 +89,14 @@ public final class WordPressComOAuthClient: NSObject { | |
| @objc public static let WordPressComOAuthDefaultApiBaseUrl = "https://public-api.wordpress.com" | ||
|
|
||
| enum WordPressComURL: String { | ||
| case oAuthBase = "/oauth2/token" | ||
| case webauthnChallenge = "wp-login.php?action=webauthn-challenge-endpoint" | ||
| case webauthnAuthentication = "wp-login.php?action=webauthn-authentication-endpoint" | ||
| case socialLogin = "/wp-login.php?action=social-login-endpoint&version=1.0" | ||
| case socialLogin2FA = "/wp-login.php?action=two-step-authentication-endpoint&version=1.0" | ||
| case socialLoginNewSMS2FA = "/wp-login.php?action=send-sms-code-endpoint" | ||
|
|
||
| func url(base: String) -> URL { | ||
| return URL(string: self.rawValue, relativeTo: URL(string: base))! | ||
| func url(base: URL) -> URL { | ||
| return URL(string: self.rawValue, relativeTo: base)! | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -95,11 +105,12 @@ public final class WordPressComOAuthClient: NSObject { | |
| private let clientID: String | ||
| private let secret: String | ||
|
|
||
| private let wordPressComBaseUrl: String | ||
| private let wordPressComApiBaseUrl: String | ||
| private let wordPressComBaseUrl: URL | ||
| private let wordPressComApiBaseUrl: URL | ||
|
|
||
| private let oauth2SessionManager: SessionManager = { | ||
| return WordPressComOAuthClient.sessionManager() | ||
| // Question: Is it necessary to use these many URLSession instances? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think of making this a
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or... have you discovered an answer since writing that comment with the extra work you've done on this library?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't use But the pattern sure raises a question, at least to me. Because it's pretty rare to see code that uses different URLSession instances to send requests to endpoints that are in the same area (in this case, authentication). But considering the "endpoints" here are all |
||
| private let oauth2Session: URLSession = { | ||
| WordPressComOAuthClient.urlSession() | ||
| }() | ||
|
|
||
| private let webauthnSessionManager: SessionManager = { | ||
|
|
@@ -126,6 +137,12 @@ public final class WordPressComOAuthClient: NSObject { | |
| return sessionManager | ||
| } | ||
|
|
||
| private class func urlSession() -> URLSession { | ||
| let configuration = URLSessionConfiguration.ephemeral | ||
| configuration.httpAdditionalHeaders = ["Accept": "application/json"] | ||
| return URLSession(configuration: configuration) | ||
| } | ||
|
|
||
| /// Creates a WordPresComOAuthClient initialized with the clientID and secrets provided | ||
| /// | ||
| @objc public class func client(clientID: String, secret: String) -> WordPressComOAuthClient { | ||
|
|
@@ -160,8 +177,13 @@ public final class WordPressComOAuthClient: NSObject { | |
| wordPressComApiBaseUrl: String = WordPressComOAuthClient.WordPressComOAuthDefaultApiBaseUrl) { | ||
| self.clientID = clientID | ||
| self.secret = secret | ||
| self.wordPressComBaseUrl = wordPressComBaseUrl | ||
| self.wordPressComApiBaseUrl = wordPressComApiBaseUrl | ||
| self.wordPressComBaseUrl = URL(string: wordPressComBaseUrl)! | ||
| self.wordPressComApiBaseUrl = URL(string: wordPressComApiBaseUrl)! | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we plan on more breaking changes, we could also slot one in to require those |
||
| } | ||
|
|
||
| public enum AuthenticationResult { | ||
| case authenticated(token: String) | ||
| case needsMultiFactor(userID: Int, nonceInfo: SocialLogin2FANonceInfo) | ||
| } | ||
|
Comment on lines
+184
to
187
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice |
||
|
|
||
| /// Authenticates on WordPress.com using the OAuth endpoints. | ||
|
|
@@ -170,64 +192,86 @@ public final class WordPressComOAuthClient: NSObject { | |
| /// - username: the account's username. | ||
| /// - password: the account's password. | ||
| /// - multifactorCode: Multifactor Authentication One-Time-Password. If not needed, can be nil | ||
| /// - needsMultifactor: @escaping (_ userID: Int, _ nonceInfo: SocialLogin2FANonceInfo) -> Void, | ||
| /// - success: block to be called if authentication was successful. The OAuth2 token is passed as a parameter. | ||
| /// - failure: block to be called if authentication failed. The error object is passed as a parameter. | ||
| /// | ||
| public func authenticate( | ||
| username: String, | ||
| password: String, | ||
| multifactorCode: String?, | ||
| needsMultifactor: @escaping ((_ userID: Int, _ nonceInfo: SocialLogin2FANonceInfo) -> Void), | ||
| success: @escaping (_ authToken: String?) -> Void, | ||
| failure: @escaping (_ error: WordPressComOAuthError) -> Void | ||
| ) { | ||
| var parameters: [String: AnyObject] = [ | ||
| "username": username as AnyObject, | ||
| "password": password as AnyObject, | ||
| "grant_type": "password" as AnyObject, | ||
| "client_id": clientID as AnyObject, | ||
| "client_secret": secret as AnyObject, | ||
| "wpcom_supports_2fa": true as AnyObject, | ||
| "with_auth_types": true as AnyObject | ||
| multifactorCode: String? | ||
| ) async -> WordPressAPIResult<AuthenticationResult, AuthenticationFailure> { | ||
| var form = [ | ||
| "username": username, | ||
| "password": password, | ||
| "grant_type": "password", | ||
| "client_id": clientID, | ||
| "client_secret": secret, | ||
| "wpcom_supports_2fa": "true", | ||
| "with_auth_types": "true" | ||
| ] | ||
|
|
||
| if let multifactorCode = multifactorCode, !multifactorCode.isEmpty { | ||
| parameters["wpcom_otp"] = multifactorCode as AnyObject? | ||
| if let multifactorCode, !multifactorCode.isEmpty { | ||
| form["wpcom_otp"] = multifactorCode | ||
| } | ||
|
|
||
| oauth2SessionManager.request(WordPressComURL.oAuthBase.url(base: wordPressComApiBaseUrl), method: .post, parameters: parameters) | ||
| .validate() | ||
| .responseJSON(completionHandler: { response in | ||
| switch response.result { | ||
| case .success(let responseObject): | ||
| WPKitLogVerbose("Received OAuth2 response: \(self.cleanedUpResponseForLogging(responseObject as AnyObject? ?? "nil" as AnyObject))") | ||
|
|
||
| guard let responseDictionary = responseObject as? [String: AnyObject] else { | ||
| return failure(.unparsableResponse(response: response.response, body: response.data)) | ||
| } | ||
| let builder = tokenRequestBuilder().body(form: form) | ||
| return await oauth2Session | ||
| .perform(request: builder) | ||
| .mapUnacceptableStatusCodeError(AuthenticationFailure.init(response:body:)) | ||
| .mapSuccess { response in | ||
| guard let responseObject = try? JSONSerialization.jsonObject(with: response.body) else { | ||
| return nil | ||
| } | ||
|
|
||
| // If we found an access_token, we are authed. | ||
| if let authToken = responseDictionary["access_token"] as? String { | ||
| return success(authToken) | ||
| } | ||
| WPKitLogVerbose("Received OAuth2 response: \(self.cleanedUpResponseForLogging(responseObject as AnyObject? ?? "nil" as AnyObject))") | ||
|
|
||
| // If there is no access token, check for a security key nonce | ||
| guard let responseData = responseDictionary["data"] as? [String: AnyObject], | ||
| let userID = responseData["user_id"] as? Int, | ||
| let _ = responseData["two_step_nonce_webauthn"] else { | ||
| return failure(.unparsableResponse(response: response.response, body: response.data)) | ||
| } | ||
| guard let responseDictionary = responseObject as? [String: AnyObject] else { | ||
| return nil | ||
| } | ||
|
|
||
| let nonceInfo = self.extractNonceInfo(data: responseData) | ||
| needsMultifactor(userID, nonceInfo) | ||
| // If we found an access_token, we are authed. | ||
| if let authToken = responseDictionary["access_token"] as? String { | ||
| return .authenticated(token: authToken) | ||
| } | ||
|
|
||
| case .failure(let error): | ||
| let nserror = self.processError(response: response, originalError: error) | ||
| WPKitLogError("Error receiving OAuth2 token: \(nserror)") | ||
| failure(nserror) | ||
| // If there is no access token, check for a security key nonce | ||
| guard let responseData = responseDictionary["data"] as? [String: AnyObject], | ||
| let userID = responseData["user_id"] as? Int, | ||
| let _ = responseData["two_step_nonce_webauthn"] else { | ||
| return nil | ||
| } | ||
| }) | ||
|
|
||
| let nonceInfo = self.extractNonceInfo(data: responseData) | ||
|
|
||
| return .needsMultiFactor(userID: userID, nonceInfo: nonceInfo) | ||
| } | ||
| } | ||
|
|
||
| /// Authenticates on WordPress.com using the OAuth endpoints. | ||
| /// | ||
| /// - Parameters: | ||
| /// - username: the account's username. | ||
| /// - password: the account's password. | ||
| /// - multifactorCode: Multifactor Authentication One-Time-Password. If not needed, can be nil | ||
| /// - needsMultifactor: @escaping (_ userID: Int, _ nonceInfo: SocialLogin2FANonceInfo) -> Void, | ||
| /// - success: block to be called if authentication was successful. The OAuth2 token is passed as a parameter. | ||
| /// - failure: block to be called if authentication failed. The error object is passed as a parameter. | ||
| public func authenticate( | ||
| username: String, | ||
| password: String, | ||
| multifactorCode: String?, | ||
| needsMultifactor: @escaping ((_ userID: Int, _ nonceInfo: SocialLogin2FANonceInfo) -> Void), | ||
| success: @escaping (_ authToken: String?) -> Void, | ||
| failure: @escaping (_ error: WordPressComOAuthError) -> Void | ||
| ) { | ||
| Task { @MainActor in | ||
| let result = await authenticate(username: username, password: password, multifactorCode: multifactorCode) | ||
| switch result { | ||
| case let .success(.authenticated(token)): | ||
| success(token) | ||
| case let .success(.needsMultiFactor(userID, nonceInfo)): | ||
| needsMultifactor(userID, nonceInfo) | ||
| case let .failure(error): | ||
| failure(error) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Requests a One Time Code, to be sent via SMS. | ||
|
|
@@ -237,35 +281,45 @@ public final class WordPressComOAuthClient: NSObject { | |
| /// - password: the account's password. | ||
| /// - success: block to be called if authentication was successful. | ||
| /// - failure: block to be called if authentication failed. The error object is passed as a parameter. | ||
| public func requestOneTimeCode(username: String, password: String) async -> WordPressAPIResult<Void, AuthenticationFailure> { | ||
| let builder = tokenRequestBuilder() | ||
| .body(form: [ | ||
| "username": username, | ||
| "password": password, | ||
| "grant_type": "password", | ||
| "client_id": clientID, | ||
| "client_secret": secret, | ||
| "wpcom_supports_2fa": "true", | ||
| "wpcom_resend_otp": "true" | ||
| ]) | ||
| return await oauth2Session | ||
| .perform(request: builder) | ||
| .mapUnacceptableStatusCodeError(AuthenticationFailure.init(response:body:)) | ||
| .mapSuccess { _ in () } | ||
| } | ||
|
|
||
| /// Requests a One Time Code, to be sent via SMS. | ||
| /// | ||
| /// - Parameters: | ||
| /// - username: the account's username. | ||
| /// - password: the account's password. | ||
| /// - success: block to be called if authentication was successful. | ||
| /// - failure: block to be called if authentication failed. The error object is passed as a parameter. | ||
| public func requestOneTimeCode( | ||
| username: String, | ||
| password: String, | ||
| success: @escaping () -> Void, | ||
| failure: @escaping (_ error: WordPressComOAuthError) -> Void | ||
| ) { | ||
| let parameters = [ | ||
| "username": username, | ||
| "password": password, | ||
| "grant_type": "password", | ||
| "client_id": clientID, | ||
| "client_secret": secret, | ||
| "wpcom_supports_2fa": true, | ||
| "wpcom_resend_otp": true | ||
| ] as [String: Any] | ||
|
|
||
| oauth2SessionManager.request(WordPressComURL.oAuthBase.url(base: wordPressComApiBaseUrl), method: .post, parameters: parameters) | ||
| .validate() | ||
| .responseJSON(completionHandler: { response in | ||
| switch response.result { | ||
| case .success: | ||
| success() | ||
| case .failure(let error): | ||
| let nserror = self.processError(response: response, originalError: error) | ||
| failure(nserror) | ||
| } | ||
| Task { @MainActor in | ||
| let result = await requestOneTimeCode(username: username, password: password) | ||
| switch result { | ||
| case .success: | ||
| success() | ||
| case let .failure(error): | ||
| failure(error) | ||
| } | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| /// Request a new SMS code to be sent during social login | ||
|
|
@@ -689,3 +743,11 @@ extension WordPressComOAuthClient { | |
| } | ||
| } | ||
| } | ||
|
|
||
| private extension WordPressComOAuthClient { | ||
| func tokenRequestBuilder() -> HTTPRequestBuilder { | ||
| HTTPRequestBuilder(url: wordPressComApiBaseUrl) | ||
| .method(.post) | ||
| .append(path: "/oauth2/token") | ||
| } | ||
|
Comment on lines
+748
to
+752
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Love to see these builders in action and to DRY things. |
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 for requiring
baseto be aURLand pushing the responsibility to create good URLs to the callers.IMHO, in the ideal world, we'd only have one point in the whole app / library where we go from
StringtoURL. The rest of the code deals with "valid" instances, without having to worry about validation.