From 8b912145c192b124b4f0d187eb5c007abe8e28a9 Mon Sep 17 00:00:00 2001 From: Harsh <6162866+harsh62@users.noreply.github.com> Date: Mon, 20 Nov 2023 22:31:57 -0500 Subject: [PATCH] fix(Auth): Moving HostedUI continuations to one place (#3363) * fix(Auth): Moving HostedUI continuations to one place * updating unit tests * fix tvos test --- .../SignIn/HostedUI/ShowHostedUISignIn.swift | 20 +-- .../Actions/SignOut/ShowHostedUISignOut.swift | 19 +-- .../HostedUIASWebAuthenticationSession.swift | 75 ++++++---- .../HostedUI/HostedUISessionBehavior.swift | 11 +- .../Mocks/MockHostedUISession.swift | 13 +- ...tedUIASWebAuthenticationSessionTests.swift | 141 ++++++------------ 6 files changed, 116 insertions(+), 163 deletions(-) diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/HostedUI/ShowHostedUISignIn.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/HostedUI/ShowHostedUISignIn.swift index 39894ff0ec..02f3c992b8 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/HostedUI/ShowHostedUISignIn.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/HostedUI/ShowHostedUISignIn.swift @@ -15,8 +15,6 @@ class ShowHostedUISignIn: NSObject, Action { let signingInData: HostedUISigningInState - var sessionAdapter: HostedUISessionBehavior? - init(signInData: HostedUISigningInState) { self.signingInData = signInData } @@ -48,17 +46,13 @@ class ShowHostedUISignIn: NSObject, Action { self.logVerbose("\(#fileID) Showing url \(url.absoluteString)", environment: environment) do { - let queryItems = try await withCheckedThrowingContinuation { - (continuation: CheckedContinuation<[URLQueryItem], Error>) in - sessionAdapter = hostedUIEnvironment.hostedUISessionFactory() - sessionAdapter?.showHostedUI( - url: url, - callbackScheme: callbackURLScheme, - inPrivate: signingInData.options.preferPrivateSession, - presentationAnchor: signingInData.presentationAnchor) { result in - continuation.resume(with: result) - } - } + let sessionAdapter = hostedUIEnvironment.hostedUISessionFactory() + let queryItems = try await sessionAdapter.showHostedUI( + url: url, + callbackScheme: callbackURLScheme, + inPrivate: signingInData.options.preferPrivateSession, + presentationAnchor: signingInData.presentationAnchor) + guard let code = queryItems.first(where: { $0.name == "code" })?.value, let state = queryItems.first(where: { $0.name == "state" })?.value, self.signingInData.state == state else { diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignOut/ShowHostedUISignOut.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignOut/ShowHostedUISignOut.swift index 3692b4d5e8..6d64dbf93a 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignOut/ShowHostedUISignOut.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignOut/ShowHostedUISignOut.swift @@ -16,8 +16,6 @@ class ShowHostedUISignOut: NSObject, Action { let signOutEvent: SignOutEventData let signInData: SignedInData - var sessionAdapter: HostedUISessionBehavior? - init(signOutEvent: SignOutEventData, signInData: SignedInData) { self.signInData = signInData self.signOutEvent = signOutEvent @@ -44,17 +42,12 @@ class ShowHostedUISignOut: NSObject, Action { do { let logoutURL = try HostedUIRequestHelper.createSignOutURL(configuration: hostedUIConfig) - _ = try await withCheckedThrowingContinuation { - (continuation: CheckedContinuation<[URLQueryItem], Error>) in - sessionAdapter = hostedUIEnvironment.hostedUISessionFactory() - sessionAdapter?.showHostedUI(url: logoutURL, - callbackScheme: callbackURLScheme, - inPrivate: false, - presentationAnchor: signOutEvent.presentationAnchor) { - result in - continuation.resume(with: result) - } - } + let sessionAdapter = hostedUIEnvironment.hostedUISessionFactory() + _ = try await sessionAdapter.showHostedUI( + url: logoutURL, + callbackScheme: callbackURLScheme, + inPrivate: false, + presentationAnchor: signOutEvent.presentationAnchor) await sendEvent(with: nil, dispatcher: dispatcher, environment: environment) diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Support/HostedUI/HostedUIASWebAuthenticationSession.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Support/HostedUI/HostedUIASWebAuthenticationSession.swift index 9c225ec931..c5e5b21835 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Support/HostedUI/HostedUIASWebAuthenticationSession.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Support/HostedUI/HostedUIASWebAuthenticationSession.swift @@ -15,45 +15,56 @@ class HostedUIASWebAuthenticationSession: NSObject, HostedUISessionBehavior { weak var webPresentation: AuthUIPresentationAnchor? - func showHostedUI(url: URL, - callbackScheme: String, - inPrivate: Bool, - presentationAnchor: AuthUIPresentationAnchor?, - callback: @escaping (Result<[URLQueryItem], HostedUIError>) -> Void) { + func showHostedUI( + url: URL, + callbackScheme: String, + inPrivate: Bool, + presentationAnchor: AuthUIPresentationAnchor?) async throws -> [URLQueryItem] { + #if os(iOS) || os(macOS) self.webPresentation = presentationAnchor - let aswebAuthenticationSession = createAuthenticationSession( - url: url, - callbackURLScheme: callbackScheme, - completionHandler: { url, error in - if let url = url { - let urlComponents = URLComponents(url: url, resolvingAgainstBaseURL: false) - let queryItems = urlComponents?.queryItems ?? [] - if let error = queryItems.first(where: { $0.name == "error" })?.value { - let errorDescription = queryItems.first( - where: { $0.name == "error_description" } - )?.value?.trim() ?? "" - let message = "\(error) \(errorDescription)" - callback(.failure(.serviceMessage(message))) - return - } - callback(.success(queryItems)) - } else if let error = error { - callback(.failure(self.convertHostedUIError(error))) + return try await withCheckedThrowingContinuation { + (continuation: CheckedContinuation<[URLQueryItem], Error>) in + + let aswebAuthenticationSession = createAuthenticationSession( + url: url, + callbackURLScheme: callbackScheme, + completionHandler: { url, error in - } else { - callback(.failure(.unknown)) - } - }) - aswebAuthenticationSession.presentationContextProvider = self - aswebAuthenticationSession.prefersEphemeralWebBrowserSession = inPrivate + if let url = url { + let urlComponents = URLComponents(url: url, resolvingAgainstBaseURL: false) + let queryItems = urlComponents?.queryItems ?? [] - DispatchQueue.main.async { - aswebAuthenticationSession.start() + // Validate if query items contains an error + if let error = queryItems.first(where: { $0.name == "error" })?.value { + let errorDescription = queryItems.first( + where: { $0.name == "error_description" } + )?.value?.trim() ?? "" + let message = "\(error) \(errorDescription)" + return continuation.resume( + throwing: HostedUIError.serviceMessage(message)) + } + return continuation.resume( + returning: queryItems) + } else if let error = error { + return continuation.resume( + throwing: self.convertHostedUIError(error)) + } else { + return continuation.resume( + throwing: HostedUIError.unknown) + } + }) + aswebAuthenticationSession.presentationContextProvider = self + aswebAuthenticationSession.prefersEphemeralWebBrowserSession = inPrivate + + DispatchQueue.main.async { + aswebAuthenticationSession.start() + } } + #else - callback(.failure(.serviceMessage("HostedUI is only available in iOS and macOS"))) + throw HostedUIError.serviceMessage("HostedUI is only available in iOS and macOS") #endif } diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Support/HostedUI/HostedUISessionBehavior.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Support/HostedUI/HostedUISessionBehavior.swift index 5dee83d166..016dd21038 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Support/HostedUI/HostedUISessionBehavior.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Support/HostedUI/HostedUISessionBehavior.swift @@ -11,9 +11,10 @@ import Amplify protocol HostedUISessionBehavior { - func showHostedUI(url: URL, - callbackScheme: String, - inPrivate: Bool, - presentationAnchor: AuthUIPresentationAnchor?, - callback: @escaping (Result<[URLQueryItem], HostedUIError>) -> Void) + func showHostedUI( + url: URL, + callbackScheme: String, + inPrivate: Bool, + presentationAnchor: AuthUIPresentationAnchor? + ) async throws -> [URLQueryItem] } diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Mocks/MockHostedUISession.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Mocks/MockHostedUISession.swift index ab3b2f23bd..f610a999f7 100644 --- a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Mocks/MockHostedUISession.swift +++ b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Mocks/MockHostedUISession.swift @@ -17,12 +17,13 @@ class MockHostedUISession: HostedUISessionBehavior { self.result = result } - func showHostedUI(url: URL, - callbackScheme: String, - inPrivate: Bool, - presentationAnchor: AuthUIPresentationAnchor?, - callback: @escaping (Result<[URLQueryItem], HostedUIError>) -> Void) { - callback(result) + func showHostedUI( + url: URL, + callbackScheme: String, + inPrivate: Bool, + presentationAnchor: AuthUIPresentationAnchor? + ) async throws -> [URLQueryItem] { + return try result.get() } } diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Support/HostedUIASWebAuthenticationSessionTests.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Support/HostedUIASWebAuthenticationSessionTests.swift index 3909827f36..eae166c4a6 100644 --- a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Support/HostedUIASWebAuthenticationSessionTests.swift +++ b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Support/HostedUIASWebAuthenticationSessionTests.swift @@ -29,77 +29,50 @@ class HostedUIASWebAuthenticationSessionTests: XCTestCase { /// Given: A HostedUIASWebAuthenticationSession /// When: showHostedUI is invoked and the session factory returns a URL with query items /// Then: An array of query items should be returned - func testShowHostedUI_withUrlInCallback_withQueryItems_shouldReturnQueryItems() { - let expectation = expectation(description: "showHostedUI") + func testShowHostedUI_withUrlInCallback_withQueryItems_shouldReturnQueryItems() async throws { factory.mockedURL = createURL(queryItems: [.init(name: "name", value: "value")]) - - session.showHostedUI() { result in - do { - let queryItems = try result.get() - XCTAssertEqual(queryItems.count, 1) - XCTAssertEqual(queryItems.first?.name, "name") - XCTAssertEqual(queryItems.first?.value, "value") - } catch { - XCTFail("Expected .success(queryItems), got \(result)") - } - expectation.fulfill() - } - waitForExpectations(timeout: 1) + let queryItems = try await session.showHostedUI() + XCTAssertEqual(queryItems.count, 1) + XCTAssertEqual(queryItems.first?.name, "name") + XCTAssertEqual(queryItems.first?.value, "value") } /// Given: A HostedUIASWebAuthenticationSession /// When: showHostedUI is invoked and the session factory returns a URL without query items /// Then: An empty array should be returned - func testShowHostedUI_withUrlInCallback_withoutQueryItems_shouldReturnEmptyQueryItems() { - let expectation = expectation(description: "showHostedUI") + func testShowHostedUI_withUrlInCallback_withoutQueryItems_shouldReturnEmptyQueryItems() async throws { factory.mockedURL = createURL() - - session.showHostedUI() { result in - do { - let queryItems = try result.get() - XCTAssertTrue(queryItems.isEmpty) - } catch { - XCTFail("Expected .success(queryItems), got \(result)") - } - expectation.fulfill() - } - waitForExpectations(timeout: 1) + let queryItems = try await session.showHostedUI() + XCTAssertTrue(queryItems.isEmpty) } /// Given: A HostedUIASWebAuthenticationSession /// When: showHostedUI is invoked and the session factory returns a URL with query items representing errors /// Then: A HostedUIError.serviceMessage should be returned - func testShowHostedUI_withUrlInCallback_withErrorInQueryItems_shouldReturnServiceMessageError() { - let expectation = expectation(description: "showHostedUI") + func testShowHostedUI_withUrlInCallback_withErrorInQueryItems_shouldReturnServiceMessageError() async { factory.mockedURL = createURL( queryItems: [ .init(name: "error", value: "Error."), .init(name: "error_description", value: "Something went wrong") ] ) - - session.showHostedUI() { result in - do { - _ = try result.get() - XCTFail("Expected failure(.serviceMessage), got \(result)") - } catch let error as HostedUIError { - if case .serviceMessage(let message) = error { - XCTAssertEqual(message, "Error. Something went wrong") - } else { - XCTFail("Expected HostedUIError.serviceMessage, got \(error)") - } - } catch { + do { + _ = try await session.showHostedUI() + } catch let error as HostedUIError { + if case .serviceMessage(let message) = error { + XCTAssertEqual(message, "Error. Something went wrong") + } else { XCTFail("Expected HostedUIError.serviceMessage, got \(error)") } - expectation.fulfill() + } catch { + XCTFail("Expected HostedUIError.serviceMessage, got \(error)") } - waitForExpectations(timeout: 1) } /// Given: A HostedUIASWebAuthenticationSession /// When: showHostedUI is invoked and the session factory returns ASWebAuthenticationSessionErrors /// Then: A HostedUIError corresponding to the error code should be returned - func testShowHostedUI_withASWebAuthenticationSessionErrors_shouldReturnRightError() { + func testShowHostedUI_withASWebAuthenticationSessionErrors_shouldReturnRightError() async { let errorMap: [ASWebAuthenticationSessionError.Code: HostedUIError] = [ .canceledLogin: .cancelled, .presentationContextNotProvided: .invalidContext, @@ -116,40 +89,28 @@ class HostedUIASWebAuthenticationSessionTests: XCTestCase { for code in errorCodes { factory.mockedError = ASWebAuthenticationSessionError(code) let expectedError = errorMap[code] ?? .unknown - let expectation = expectation(description: "showHostedUI for error \(code)") - session.showHostedUI() { result in - do { - _ = try result.get() - XCTFail("Expected failure(.\(expectedError)), got \(result)") - } catch let error as HostedUIError { - XCTAssertEqual(error, expectedError) - } catch { - XCTFail("Expected HostedUIError.\(expectedError), got \(error)") - } - expectation.fulfill() + do { + _ = try await session.showHostedUI() + } catch let error as HostedUIError { + XCTAssertEqual(error, expectedError) + } catch { + XCTFail("Expected HostedUIError.\(expectedError), got \(error)") } - waitForExpectations(timeout: 1) } } /// Given: A HostedUIASWebAuthenticationSession /// When: showHostedUI is invoked and the session factory returns an error /// Then: A HostedUIError.unknown should be returned - func testShowHostedUI_withOtherError_shouldReturnUnknownError() { + func testShowHostedUI_withOtherError_shouldReturnUnknownError() async { factory.mockedError = CancellationError() - let expectation = expectation(description: "showHostedUI") - session.showHostedUI() { result in - do { - _ = try result.get() - XCTFail("Expected failure(.unknown), got \(result)") - } catch let error as HostedUIError { - XCTAssertEqual(error, .unknown) - } catch { - XCTFail("Expected HostedUIError.unknown, got \(error)") - } - expectation.fulfill() + do { + _ = try await session.showHostedUI() + } catch let error as HostedUIError { + XCTAssertEqual(error, .unknown) + } catch { + XCTFail("Expected HostedUIError.unknown, got \(error)") } - waitForExpectations(timeout: 1) } private func createURL(queryItems: [URLQueryItem] = []) -> URL { @@ -203,13 +164,12 @@ class MockASWebAuthenticationSession: ASWebAuthenticationSession { } extension HostedUIASWebAuthenticationSession { - func showHostedUI(callback: @escaping (Result<[URLQueryItem], HostedUIError>) -> Void) { - showHostedUI( + func showHostedUI() async throws -> [URLQueryItem] { + return try await showHostedUI( url: URL(string: "https://test.com")!, callbackScheme: "https", inPrivate: false, - presentationAnchor: nil, - callback: callback) + presentationAnchor: nil) } } #else @@ -218,30 +178,23 @@ extension HostedUIASWebAuthenticationSession { import XCTest class HostedUIASWebAuthenticationSessionTests: XCTestCase { - func testShowHostedUI_shouldThrowServiceError() { - let expectation = expectation(description: "showHostedUI") + func testShowHostedUI_shouldThrowServiceError() async { let session = HostedUIASWebAuthenticationSession() - session.showHostedUI( - url: URL(string: "https://test.com")!, - callbackScheme: "https", - inPrivate: false, - presentationAnchor: nil - ) { result in - do { - _ = try result.get() - XCTFail("Expected failure(.serviceMessage), got \(result)") - } catch let error as HostedUIError { - if case .serviceMessage(let message) = error { - XCTAssertEqual(message, "HostedUI is only available in iOS and macOS") - } else { - XCTFail("Expected HostedUIError.serviceMessage, got \(error)") - } - } catch { + do { + _ = try await session.showHostedUI( + url: URL(string: "https://test.com")!, + callbackScheme: "https", + inPrivate: false, + presentationAnchor: nil) + } catch let error as HostedUIError { + if case .serviceMessage(let message) = error { + XCTAssertEqual(message, "HostedUI is only available in iOS and macOS") + } else { XCTFail("Expected HostedUIError.serviceMessage, got \(error)") } - expectation.fulfill() + } catch { + XCTFail("Expected HostedUIError.serviceMessage, got \(error)") } - waitForExpectations(timeout: 1) } }