diff --git a/GTMAppAuth/Sources/AuthSession.swift b/GTMAppAuth/Sources/AuthSession.swift index 9d2eeecc..1374c828 100644 --- a/GTMAppAuth/Sources/AuthSession.swift +++ b/GTMAppAuth/Sources/AuthSession.swift @@ -29,7 +29,7 @@ import GTMSessionFetcher /// /// Enables you to use AppAuth with the GTM Session Fetcher library. @objc(GTMAuthSession) -open class AuthSession: NSObject, GTMSessionFetcherAuthorizer, NSSecureCoding { +public final class AuthSession: NSObject, GTMSessionFetcherAuthorizer, NSSecureCoding { /// The legacy name for this type used while archiving and unarchiving an instance. static let legacyArchiveName = "GTMAppAuthFetcherAuthorization" @@ -69,8 +69,8 @@ open class AuthSession: NSObject, GTMSessionFetcherAuthorizer, NSSecureCoding { /// bearer token unencrypted. @objc public var shouldAuthorizeAllRequests = false - /// Delegate of the `AuthSession` used to supply additional parameters on token refresh. - @objc public weak var tokenRefreshDelegate: AuthSessionTokenRefreshDelegate? + /// Delegate of the `AuthSession`. + @objc public weak var delegate: AuthSessionDelegate? /// The fetcher service. @objc public weak var fetcherService: GTMSessionFetcherServiceProtocol? = nil @@ -225,13 +225,18 @@ open class AuthSession: NSObject, GTMSessionFetcherAuthorizer, NSSecureCoding { serialAuthArgsQueue.sync { authorizationArgs.append(args) } - let additionalRefreshParameters = tokenRefreshDelegate? - .additionalRefreshParameters(authSession: self) + let additionalRefreshParameters = delegate?.additionalTokenRefreshParameters?( + forAuthSession: self + ) let authStateAction = { (accessToken: String?, idToken: String?, error: Swift.Error?) in self.serialAuthArgsQueue.sync { [weak self] in guard let self = self else { return } - for queuedArgs in self.authorizationArgs { + for var queuedArgs in self.authorizationArgs { + if let error = error { + // Give `queuedArgs` most recent error from AppAuth + queuedArgs.error = error + } self.authorizeRequestImmediately(args: queuedArgs, accessToken: accessToken) } self.authorizationArgs.removeAll() @@ -243,10 +248,7 @@ open class AuthSession: NSObject, GTMSessionFetcherAuthorizer, NSSecureCoding { ) } - private func authorizeRequestImmediately( - args: AuthorizationArguments, - accessToken: String? - ) { + private func authorizeRequestImmediately(args: AuthorizationArguments, accessToken: String?) { var args = args let request = args.request let requestURL = request.url @@ -256,7 +258,6 @@ open class AuthSession: NSObject, GTMSessionFetcherAuthorizer, NSSecureCoding { || requestURL?.isFileURL ?? false || shouldAuthorizeAllRequests if !isAuthorizableRequest { - // #if DEBUG print( """ @@ -265,7 +266,7 @@ open class AuthSession: NSObject, GTMSessionFetcherAuthorizer, NSSecureCoding { ) #endif } - if isAuthorizableRequest, + authorizeRequestControlFlow: if isAuthorizableRequest, let accessToken = accessToken, !accessToken.isEmpty { request.setValue( @@ -274,14 +275,25 @@ open class AuthSession: NSObject, GTMSessionFetcherAuthorizer, NSSecureCoding { ) // `request` is authorized even if previous refreshes produced an error args.error = nil + } else if args.error != nil { + // Keep error received from AppAuth + break authorizeRequestControlFlow } else if accessToken?.isEmpty ?? true { args.error = Error.accessTokenEmptyForRequest(request as URLRequest) } else { args.error = Error.cannotAuthorizeRequest(request as URLRequest) } 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 + } + 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) @@ -302,8 +314,7 @@ open class AuthSession: NSObject, GTMSessionFetcherAuthorizer, NSSecureCoding { request: NSMutableURLRequest, error: Swift.Error? ) { - guard let delegate = delegate as? NSObject, - delegate.responds(to: selector) else { + guard let delegate = delegate as? NSObject, delegate.responds(to: selector) else { return } let authorization = self @@ -431,14 +442,6 @@ extension AuthorizationArguments { } } -/// Delegate of the `AuthSession` used to supply additional parameters on token refresh. -@objc(GTMAuthSessionTokenRefreshDelegate) -public protocol AuthSessionTokenRefreshDelegate: NSObjectProtocol { - func additionalRefreshParameters( - authSession: AuthSession - ) -> [String: String]? -} - public extension AuthSession { // MARK: - Keys diff --git a/GTMAppAuth/Sources/AuthSessionDelegate.swift b/GTMAppAuth/Sources/AuthSessionDelegate.swift new file mode 100644 index 00000000..c3d40ca6 --- /dev/null +++ b/GTMAppAuth/Sources/AuthSessionDelegate.swift @@ -0,0 +1,44 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import Foundation + +/// Methods defining the `AuthSession`'s delegate. +@objc(GTMAuthSessionDelegate) +public protocol AuthSessionDelegate { + /// Used to supply additional parameters on token refresh. + /// + /// - Parameters: + /// - authSession: The `AuthSession` needing additional token refresh parameters. + /// - Returns: An optional `[String: String]` supplying the additional token refresh parameters. + @objc optional func additionalTokenRefreshParameters( + forAuthSession authSession: AuthSession + ) -> [String: String]? + + /// 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. + /// + /// - 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( + forAuthSession authSession: AuthSession, + originalError: Error + ) -> Error? +} diff --git a/GTMAppAuth/Sources/KeychainStore/KeychainStore.swift b/GTMAppAuth/Sources/KeychainStore/KeychainStore.swift index b94c9b66..63af95af 100644 --- a/GTMAppAuth/Sources/KeychainStore/KeychainStore.swift +++ b/GTMAppAuth/Sources/KeychainStore/KeychainStore.swift @@ -29,7 +29,8 @@ import GTMSessionFetcher /// A helper providing a concrete implementation for saving and loading auth data to the keychain. @objc(GTMKeychainStore) public final class KeychainStore: NSObject, AuthSessionStore { - private let keychainHelper: KeychainHelper + /// The helper wrapping keychain access. + @objc public let keychainHelper: KeychainHelper /// The last used `NSKeyedArchiver` used in tests to ensure that the class name mapping worked. private(set) var lastUsedKeyedArchiver: NSKeyedArchiver? /// The last used `NSKeyedUnarchiver` used in tests to ensure that the class name mapping worked. diff --git a/GTMAppAuth/Tests/Helpers/AuthorizationTestingHelp.swift b/GTMAppAuth/Tests/Helpers/AuthorizationTestingHelp.swift index 3282837c..42cefca9 100644 --- a/GTMAppAuth/Tests/Helpers/AuthorizationTestingHelp.swift +++ b/GTMAppAuth/Tests/Helpers/AuthorizationTestingHelp.swift @@ -25,11 +25,7 @@ import AppAuth import XCTest import GTMAppAuth -/// A subclass of `AuthSession` to use in tests. -@objc(GTMAuthorizationTestingHelper) -public class AuthorizationTestingHelper: AuthSession {} - -/// The delegate object passed to `AuthorizationTestingHelper`. +/// The delegate object passed to selector-based callback to authorize requests. @objc(GTMAuthorizationTestDelegate) public class AuthorizationTestDelegate: NSObject { /// The authorization passed back to this delegate. @@ -38,8 +34,7 @@ public class AuthorizationTestDelegate: NSObject { @objc public var passedRequest: NSMutableURLRequest? /// The error passed back to this delegate. @objc public var passedError: NSError? - /// The expectation needing fulfillment when this delegate receives its - /// callback. + /// The expectation needing fulfillment when this delegate receives its callback. @objc public let expectation: XCTestExpectation /// An initializer creating the delegate. @@ -67,3 +62,49 @@ public class AuthorizationTestDelegate: NSObject { expectation.fulfill() } } + +/// 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? + + /// 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 [:] + } + + public func updatedError( + forAuthSession authSession: AuthSession, + originalError: Error + ) -> Error? { + XCTAssertEqual(authSession, originalAuthSession) + updatedErrorCalled = true + guard let expectedError = expectedError else { + return nil + } + return expectedError + } +} diff --git a/GTMAppAuth/Tests/ObjCIntegration/GTMAuthSessionTests.m b/GTMAppAuth/Tests/ObjCIntegration/GTMAuthSessionTests.m index dec4044d..092ee7b6 100644 --- a/GTMAppAuth/Tests/ObjCIntegration/GTMAuthSessionTests.m +++ b/GTMAppAuth/Tests/ObjCIntegration/GTMAuthSessionTests.m @@ -156,8 +156,8 @@ - (void)testAuthorizeSecureRequestWithDelegate { [[XCTestExpectation alloc] initWithDescription:@"Authorize with delegate"]; OIDAuthState *authState = OIDAuthState.testInstance; - GTMAuthorizationTestingHelper *originalAuthorization = - [[GTMAuthorizationTestingHelper alloc] initWithAuthState:authState]; + GTMAuthSession *originalAuthorization = + [[GTMAuthSession alloc] initWithAuthState:authState]; GTMAuthorizationTestDelegate *testingDelegate = [[GTMAuthorizationTestDelegate alloc] initWithExpectation:delegateExpectation]; diff --git a/GTMAppAuth/Tests/Unit/AuthSessionTests.swift b/GTMAppAuth/Tests/Unit/AuthSessionTests.swift index 58c21755..fabfbda7 100644 --- a/GTMAppAuth/Tests/Unit/AuthSessionTests.swift +++ b/GTMAppAuth/Tests/Unit/AuthSessionTests.swift @@ -75,6 +75,228 @@ class AuthSessionTests: XCTestCase { XCTAssertTrue(authSession.isAuthorizedRequest(request as URLRequest)) } + func testAuthSessionAuthorizeRequestWithCompletionAdditionalParametersDelegateCallback() { + let authorizeSecureRequestExpectation = expectation( + description: "Authorize with completion expectation" + ) + let authSession = AuthSession( + authState: OIDAuthState.testInstance() + ) + let authSessionDelegate = AuthSessionDelegateProvider(originalAuthSession: authSession) + authSession.delegate = authSessionDelegate + + let request = NSMutableURLRequest(url: secureFakeURL) + authSession.authorizeRequest(request) { error in + XCTAssertNil(error) + authorizeSecureRequestExpectation.fulfill() + } + XCTAssertTrue(authSession.isAuthorizingRequest(request as URLRequest)) + waitForExpectations(timeout: expectationTimeout) + XCTAssertTrue(authSession.isAuthorizedRequest(request as URLRequest)) + XCTAssertTrue(authSessionDelegate.additionalRefreshParametersCalled) + } + + func testAuthSessionSelectorCallbackAdditionalParametersDelegateCallback() { + let delegateExpectation = expectation( + description: "Delegate callback expectation" + ) + + let originalAuthorization = AuthSession(authState: OIDAuthState.testInstance()) + let testingDelegate = AuthorizationTestDelegate( + expectation: delegateExpectation + ) + let authSessionDelegate = AuthSessionDelegateProvider( + originalAuthSession: originalAuthorization + ) + originalAuthorization.delegate = authSessionDelegate + + let originalRequest = NSMutableURLRequest(url: self.secureFakeURL) + originalAuthorization.authorizeRequest( + originalRequest, + delegate: testingDelegate, + didFinish: #selector( + AuthorizationTestDelegate.authentication(_:request:finishedWithError:) + ) + ) + + XCTAssertTrue(originalAuthorization.isAuthorizingRequest(originalRequest as URLRequest)) + waitForExpectations(timeout: expectationTimeout) + + guard let receivedRequest = testingDelegate.passedRequest else { + return XCTFail("Testing delegate did not receive the request") + } + XCTAssertEqual(originalRequest, receivedRequest) + + guard let receivedAuthorization = testingDelegate.passedAuthorization else { + return XCTFail("Testing delegate did not receive the authorization") + } + XCTAssertEqual(originalAuthorization, receivedAuthorization) + + XCTAssertNil(testingDelegate.passedError) + + XCTAssertTrue(originalAuthorization.isAuthorizedRequest(originalRequest as URLRequest)) + XCTAssertTrue(authSessionDelegate.additionalRefreshParametersCalled) + } + + func testAuthSessionSelectorCallbackUpdatedErrorDelegateCallback() { + let delegateExpectation = expectation( + description: "Delegate callback expectation" + ) + + let originalAuthorization = AuthSession(authState: OIDAuthState.testInstance()) + let testingDelegate = AuthorizationTestDelegate( + expectation: delegateExpectation + ) + + // The error we expect from the `AuthSessionDelegate` + let expectedCustomErrorDomain = "SomeCustomDomain" + let expectedError = NSError(domain: expectedCustomErrorDomain, code: 4) + let authSessionDelegate = AuthSessionDelegateProvider( + originalAuthSession: originalAuthorization, + expectedError: expectedError + ) + originalAuthorization.delegate = authSessionDelegate + + // There must be an error here for the delegate to get the `updatedError` callback + let originalRequest = NSMutableURLRequest(url: self.insecureFakeURL) + originalAuthorization.authorizeRequest( + originalRequest, + delegate: testingDelegate, + didFinish: #selector( + AuthorizationTestDelegate.authentication(_:request:finishedWithError:) + ) + ) + + XCTAssertTrue(originalAuthorization.isAuthorizingRequest(originalRequest as URLRequest)) + waitForExpectations(timeout: expectationTimeout) + + guard let receivedRequest = testingDelegate.passedRequest else { + return XCTFail("Testing delegate did not receive the request") + } + XCTAssertEqual(originalRequest, receivedRequest) + + guard let receivedAuthorization = testingDelegate.passedAuthorization else { + return XCTFail("Testing delegate did not receive the authorization") + } + XCTAssertEqual(originalAuthorization, receivedAuthorization) + + XCTAssertNotNil(testingDelegate.passedError) + XCTAssertEqual(testingDelegate.passedError, expectedError) + XCTAssertFalse(originalAuthorization.isAuthorizedRequest(originalRequest as URLRequest)) + XCTAssertTrue(authSessionDelegate.updatedErrorCalled) + } + + func testAuthSessionSelectorCallbackNoUpdatedErrorDelegateCallback() { + let delegateExpectation = expectation( + description: "Delegate callback expectation" + ) + + let originalAuthorization = AuthSession(authState: OIDAuthState.testInstance()) + let testingDelegate = AuthorizationTestDelegate( + expectation: delegateExpectation + ) + + let authSessionDelegate = AuthSessionDelegateProvider( + originalAuthSession: originalAuthorization, + expectedError: nil + ) + originalAuthorization.delegate = authSessionDelegate + + // There must be an error here for the delegate to get the `updatedError` callback + let originalRequest = NSMutableURLRequest(url: self.insecureFakeURL) + originalAuthorization.authorizeRequest( + originalRequest, + delegate: testingDelegate, + didFinish: #selector( + AuthorizationTestDelegate.authentication(_:request:finishedWithError:) + ) + ) + + XCTAssertTrue(originalAuthorization.isAuthorizingRequest(originalRequest as URLRequest)) + waitForExpectations(timeout: expectationTimeout) + + guard let receivedRequest = testingDelegate.passedRequest else { + return XCTFail("Testing delegate did not receive the request") + } + XCTAssertEqual(originalRequest, receivedRequest) + + guard let receivedAuthorization = testingDelegate.passedAuthorization else { + return XCTFail("Testing delegate did not receive the authorization") + } + XCTAssertEqual(originalAuthorization, receivedAuthorization) + + XCTAssertNotNil(testingDelegate.passedError) + XCTAssertEqual( + testingDelegate.passedError as? FetcherAuthError, + FetcherAuthError.cannotAuthorizeRequest(originalRequest as URLRequest) + ) + XCTAssertFalse(originalAuthorization.isAuthorizedRequest(originalRequest as URLRequest)) + XCTAssertTrue(authSessionDelegate.updatedErrorCalled) + } + + func testAuthSessionAuthorizeRequestWithCompletionDidFailDelegateUpdatedErrorCallback() { + let authorizeSecureRequestExpectation = expectation( + description: "Authorize with completion expectation" + ) + let authSession = AuthSession( + authState: OIDAuthState.testInstance() + ) + + // The error we expect from the + // `AuthSessionDelegate.updatedError(forAuthSession:error:)` callback + let expectedCustomErrorDomain = "SomeCustomDomain" + let expectedError = NSError(domain: expectedCustomErrorDomain, code: 4) + // There must be an error here for the delegate to get the `updatedError` callback + let request = NSMutableURLRequest(url: insecureFakeURL) + let authSessionDelegate = AuthSessionDelegateProvider( + originalAuthSession: authSession, + expectedError: expectedError + ) + authSession.delegate = authSessionDelegate + + authSession.authorizeRequest(request) { error in + guard let error = error else { + return XCTFail("There should be an `NSError` authorizing \(request)") + } + let nsError = error as NSError + XCTAssertEqual(nsError, expectedError) + authorizeSecureRequestExpectation.fulfill() + } + XCTAssertTrue(authSession.isAuthorizingRequest(request as URLRequest)) + waitForExpectations(timeout: expectationTimeout) + XCTAssertFalse(authSession.isAuthorizedRequest(request as URLRequest)) + XCTAssertTrue(authSessionDelegate.updatedErrorCalled) + } + + func testAuthSessionAuthorizeRequestWithCompletionDidFailButDelegateDoesntUpdateError() { + 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 request = NSMutableURLRequest(url: insecureFakeURL) + let authSessionDelegate = AuthSessionDelegateProvider( + originalAuthSession: authSession, + expectedError: nil + ) + authSession.delegate = authSessionDelegate + + authSession.authorizeRequest(request) { error in + guard let error = error as? FetcherAuthError else { + return XCTFail("There should be an `NSError` authorizing \(request)") + } + XCTAssertEqual(error, FetcherAuthError.cannotAuthorizeRequest(request as URLRequest)) + authorizeSecureRequestExpectation.fulfill() + } + XCTAssertTrue(authSession.isAuthorizingRequest(request as URLRequest)) + waitForExpectations(timeout: expectationTimeout) + XCTAssertFalse(authSession.isAuthorizedRequest(request as URLRequest)) + XCTAssertTrue(authSessionDelegate.updatedErrorCalled) + } + func testAuthorizeInsecureRequestWithCompletion() { let authorizeInsecureRequestExpectation = expectation( description: "Authorize with completion expectation" @@ -118,9 +340,8 @@ class AuthSessionTests: XCTestCase { let delegateExpectation = expectation( description: "Delegate callback expectation" ) - let originalAuthorization = AuthorizationTestingHelper( - authState: OIDAuthState.testInstance() - ) + + let originalAuthorization = AuthSession(authState: OIDAuthState.testInstance()) let testingDelegate = AuthorizationTestDelegate( expectation: delegateExpectation ) @@ -153,9 +374,8 @@ class AuthSessionTests: XCTestCase { let delegateExpectation = expectation( description: "Delegate callback expectation" ) - let originalAuthorization = AuthorizationTestingHelper( - authState: OIDAuthState.testInstance() - ) + + let originalAuthorization = AuthSession(authState: OIDAuthState.testInstance()) let testingDelegate = AuthorizationTestDelegate( expectation: delegateExpectation )