From ed862618f7a2aa79769b1b371cb4232a6ef45258 Mon Sep 17 00:00:00 2001 From: mdmathias Date: Thu, 4 May 2023 13:49:46 -0700 Subject: [PATCH] Add a method taking a completion argument for asynchronous error updates (#229) --- GTMAppAuth/Sources/AuthSession.swift | 38 ++++++++------- GTMAppAuth/Sources/AuthSessionDelegate.swift | 13 +++--- .../Helpers/AuthorizationTestingHelp.swift | 46 +++++++++++++++---- GTMAppAuth/Tests/Unit/AuthSessionTests.swift | 29 ++++++++++++ 4 files changed, 95 insertions(+), 31 deletions(-) diff --git a/GTMAppAuth/Sources/AuthSession.swift b/GTMAppAuth/Sources/AuthSession.swift index 1374c828..de93bff9 100644 --- a/GTMAppAuth/Sources/AuthSession.swift +++ b/GTMAppAuth/Sources/AuthSession.swift @@ -285,26 +285,30 @@ public final class AuthSession: NSObject, GTMSessionFetcherAuthorizer, NSSecureC } let callbackQueue = fetcherService?.callbackQueue ?? DispatchQueue.main - if let error = args.error, let delegate = self.delegate { - // If there is an updated error, use that; otherwise, use whatever is already in `args.error` - let newError = delegate.updatedError?(forAuthSession: self, originalError: error) - args.error = newError ?? error + let callback: () -> Void = { + callbackQueue.async { + switch args.callbackStyle { + case .completion(let callback): + self.invokeCompletionCallback(with: callback, error: args.error) + case .delegate(let delegate, let selector): + self.invokeCallback( + withDelegate: delegate, + selector: selector, + request: request, + error: args.error + ) + } + } } - callbackQueue.async { [weak self] in - guard let self = self else { return } - - switch args.callbackStyle { - case .completion(let callback): - self.invokeCompletionCallback(with: callback, error: args.error) - case .delegate(let delegate, let selector): - self.invokeCallback( - withDelegate: delegate, - selector: selector, - request: request, - error: args.error - ) + if let error = args.error, let delegate = self.delegate, delegate.updateError != nil { + // Use updated error if exists; otherwise, use whatever is already in `args.error` + delegate.updateError?(forAuthSession: self, originalError: error) { updatedError in + args.error = updatedError ?? error + callback() } + } else { + callback() } } diff --git a/GTMAppAuth/Sources/AuthSessionDelegate.swift b/GTMAppAuth/Sources/AuthSessionDelegate.swift index c3d40ca6..3e24a71d 100644 --- a/GTMAppAuth/Sources/AuthSessionDelegate.swift +++ b/GTMAppAuth/Sources/AuthSessionDelegate.swift @@ -30,15 +30,16 @@ public protocol AuthSessionDelegate { /// A method notifying the delegate that the authorization request failed. /// - /// Use this method to examine the error behind the failed authorization request and supply a more - /// custom error specifying whatever context is needed. + /// Use this method to examine the error behind the failed authorization request and supply a + /// customized error created asynchronously that specifies whatever context is needed. /// /// - Parameters: /// - authSession: The `AuthSession` whose authorization request failed. /// - originalError: The original `Error` associated with the failure. - /// - Returns: The new error for `AuthSession` to send back or nil if no error should be sent. - @objc optional func updatedError( + /// - completion: An escaping closure to pass back the updated error. + @objc optional func updateError( forAuthSession authSession: AuthSession, - originalError: Error - ) -> Error? + originalError: Error, + completion: @escaping (Error?) -> Void + ) } diff --git a/GTMAppAuth/Tests/Helpers/AuthorizationTestingHelp.swift b/GTMAppAuth/Tests/Helpers/AuthorizationTestingHelp.swift index 42cefca9..3387d528 100644 --- a/GTMAppAuth/Tests/Helpers/AuthorizationTestingHelp.swift +++ b/GTMAppAuth/Tests/Helpers/AuthorizationTestingHelp.swift @@ -63,10 +63,42 @@ public class AuthorizationTestDelegate: NSObject { } } +/// A testing helper that does not implement the update error method in `AuthSessionDelegate`. +@objc(GTMAuthSessionDelegateProvideMissingEMMErrorHandling) +public class AuthSessionDelegateProvideMissingEMMErrorHandling: NSObject, AuthSessionDelegate { + /// The `AuthSession` to which this delegate was given. + @objc public var originalAuthSession: AuthSession? + + /// Whether or not the delegate callback for additional refresh parameters was called. + /// + /// - Note: Defaults to `false`. + @objc public var additionalRefreshParametersCalled = false + + /// Whether or not the delegate callback for authorization request failure was called. + /// + /// - Note: Defaults to `false`. + @objc public var updatedErrorCalled = false + + /// The expected error from the delegate callback. + @objc public let expectedError: NSError? + + @objc public init(originalAuthSession: AuthSession, expectedError: NSError? = nil) { + self.originalAuthSession = originalAuthSession + self.expectedError = expectedError + } + + public func additionalTokenRefreshParameters( + forAuthSession authSession: AuthSession + ) -> [String : String]? { + XCTAssertEqual(authSession, originalAuthSession) + additionalRefreshParametersCalled = true + return [:] + } +} + /// A testing helper given to `AuthSession`'s `delegate` to verify delegate callbacks. @objc(GTMAuthSessionDelegateProvider) public class AuthSessionDelegateProvider: NSObject, AuthSessionDelegate { - /// The `AuthSession` to which this delegate was given. @objc public var originalAuthSession: AuthSession? @@ -96,15 +128,13 @@ public class AuthSessionDelegateProvider: NSObject, AuthSessionDelegate { return [:] } - public func updatedError( + public func updateError( forAuthSession authSession: AuthSession, - originalError: Error - ) -> Error? { + originalError: Error, + completion: @escaping (Error?) -> Void + ) { XCTAssertEqual(authSession, originalAuthSession) updatedErrorCalled = true - guard let expectedError = expectedError else { - return nil - } - return expectedError + completion(expectedError) } } diff --git a/GTMAppAuth/Tests/Unit/AuthSessionTests.swift b/GTMAppAuth/Tests/Unit/AuthSessionTests.swift index fabfbda7..9e3b81d8 100644 --- a/GTMAppAuth/Tests/Unit/AuthSessionTests.swift +++ b/GTMAppAuth/Tests/Unit/AuthSessionTests.swift @@ -268,6 +268,35 @@ class AuthSessionTests: XCTestCase { XCTAssertTrue(authSessionDelegate.updatedErrorCalled) } + func testAuthSessionAuthorizeRequestWithCompletionFailDelegateNoImplementUpdateErrorCallback() { + let authorizeSecureRequestExpectation = expectation( + description: "Authorize with completion expectation" + ) + let authSession = AuthSession( + authState: OIDAuthState.testInstance() + ) + + // There must be an error here for the delegate to get the `updatedError` callback + let insecureRequest = NSMutableURLRequest(url: insecureFakeURL) + let expectedError = AuthSession.Error.cannotAuthorizeRequest(insecureRequest as URLRequest) + let authSessionDelegate = AuthSessionDelegateProvideMissingEMMErrorHandling( + originalAuthSession: authSession + ) + authSession.delegate = authSessionDelegate + + authSession.authorizeRequest(insecureRequest) { error in + guard let error = error else { + return XCTFail("There should be an `NSError` authorizing \(insecureRequest)") + } + XCTAssertEqual(error as? AuthSession.Error, expectedError) + authorizeSecureRequestExpectation.fulfill() + } + XCTAssertTrue(authSession.isAuthorizingRequest(insecureRequest as URLRequest)) + waitForExpectations(timeout: expectationTimeout) + XCTAssertFalse(authSession.isAuthorizedRequest(insecureRequest as URLRequest)) + XCTAssertFalse(authSessionDelegate.updatedErrorCalled) + } + func testAuthSessionAuthorizeRequestWithCompletionDidFailButDelegateDoesntUpdateError() { let authorizeSecureRequestExpectation = expectation( description: "Authorize with completion expectation"