From 619867b9e99007db7479d40de2f89eaf120cdeba Mon Sep 17 00:00:00 2001 From: Harsh <6162866+harsh62@users.noreply.github.com> Date: Wed, 2 Oct 2024 19:10:37 -0400 Subject: [PATCH 1/2] fix(auth): return configuration error upon invalid redirect URI in hosted ui (#3889) --- .../SignIn/HostedUI/ShowHostedUISignIn.swift | 2 +- .../SignIn/HostedUISignInState+Resolver.swift | 1 - .../AWSAuthHostedUISignInTests.swift | 72 +++++++++++++++++++ 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/HostedUI/ShowHostedUISignIn.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/HostedUI/ShowHostedUISignIn.swift index 02f3c992b8..559a91f2f4 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/HostedUI/ShowHostedUISignIn.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/HostedUI/ShowHostedUISignIn.swift @@ -36,7 +36,7 @@ class ShowHostedUISignIn: NSObject, Action { guard let callbackURL = URL(string: hostedUIConfig.oauth.signInRedirectURI), let callbackURLScheme = callbackURL.scheme else { - let event = SignInEvent(eventType: .throwAuthError(.hostedUI(.signInURI))) + let event = HostedUIEvent(eventType: .throwError(.hostedUI(.signInURI))) logVerbose("\(#fileID) Sending event \(event)", environment: environment) await dispatcher.send(event) return diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/SignIn/HostedUISignInState+Resolver.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/SignIn/HostedUISignInState+Resolver.swift index 8c50307e8f..6bb811523a 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/SignIn/HostedUISignInState+Resolver.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/SignIn/HostedUISignInState+Resolver.swift @@ -30,7 +30,6 @@ extension HostedUISignInState { case .showingUI: if case .throwError(let error) = event.isHostedUIEvent { - // Remove this? let action = CancelSignIn() return .init(newState: .error(error), actions: [action]) } diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/TaskTests/HostedUITests/AWSAuthHostedUISignInTests.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/TaskTests/HostedUITests/AWSAuthHostedUISignInTests.swift index 420d8f5abd..7be38b07d2 100644 --- a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/TaskTests/HostedUITests/AWSAuthHostedUISignInTests.swift +++ b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/TaskTests/HostedUITests/AWSAuthHostedUISignInTests.swift @@ -42,6 +42,43 @@ class AWSAuthHostedUISignInTests: XCTestCase { return URLSession(configuration: configuration) } + private func customPlugin(with cusotmConfiguration: HostedUIConfigurationData?) -> AWSCognitoAuthPlugin { + let plugin = AWSCognitoAuthPlugin() + mockJson = try! JSONSerialization.data(withJSONObject: mockTokenResult) + MockURLProtocol.requestHandler = { _ in + return (HTTPURLResponse(), self.mockJson) + } + + func sessionFactory() -> HostedUISessionBehavior { + MockHostedUISession(result: mockHostedUIResult) + } + + func mockRandomString() -> RandomStringBehavior { + return MockRandomStringGenerator(mockString: mockState, mockUUID: mockState) + } + + let environment = BasicHostedUIEnvironment(configuration: cusotmConfiguration ?? configuration, + hostedUISessionFactory: sessionFactory, + urlSessionFactory: urlSessionMock, + randomStringFactory: mockRandomString) + let authEnvironment = Defaults.makeDefaultAuthEnvironment( + userPoolFactory: { self.mockIdentityProvider }, + hostedUIEnvironment: environment) + let stateMachine = Defaults.authStateMachineWith( + environment: authEnvironment, + initialState: initialState + ) + + plugin.configure( + authConfiguration: Defaults.makeDefaultAuthConfigData(withHostedUI: configuration), + authEnvironment: authEnvironment, + authStateMachine: stateMachine, + credentialStoreStateMachine: Defaults.makeDefaultCredentialStateMachine(), + hubEventHandler: MockAuthHubEventBehavior(), + analyticsHandler: MockAnalyticsHandler()) + return plugin + } + override func setUp() { plugin = AWSCognitoAuthPlugin() mockJson = try! JSONSerialization.data(withJSONObject: mockTokenResult) @@ -310,6 +347,41 @@ class AWSAuthHostedUISignInTests: XCTestCase { await fulfillment(of: [expectation], timeout: networkTimeout) } + @MainActor + func testInvalidRedirectConfigurationFailure() async { + let invalidRedirectConfig = HostedUIConfigurationData(clientId: "clientId", oauth: .init( + domain: "cognitodomain", + scopes: ["name"], + signInRedirectURI: "@#$%junk1343", + signOutRedirectURI: "@3451://")) + let testPlugin = customPlugin(with: invalidRedirectConfig) + + mockHostedUIResult = .success([ + .init(name: "state", value: mockState), + .init(name: "code", value: mockProof) + ]) + mockTokenResult = [ + "refresh_token": AWSCognitoUserPoolTokens.testData.refreshToken, + "expires_in": 10] as [String: Any] + mockJson = try! JSONSerialization.data(withJSONObject: mockTokenResult) + MockURLProtocol.requestHandler = { _ in + return (HTTPURLResponse(), self.mockJson) + } + + let expectation = expectation(description: "SignIn operation should complete") + do { + _ = try await testPlugin.signInWithWebUI(presentationAnchor: ASPresentationAnchor(), options: nil) + XCTFail("Should not succeed") + } catch { + guard case AuthError.configuration = error else { + XCTFail("Should not fail with error = \(error)") + return + } + expectation.fulfill() + } + await fulfillment(of: [expectation], timeout: networkTimeout) + } + /// Test a signIn restart while another sign in is in progress From 65da44ec5b66e8069447143eb868aaed7c4e3616 Mon Sep 17 00:00:00 2001 From: Harsh <6162866+harsh62@users.noreply.github.com> Date: Fri, 18 Oct 2024 10:27:02 -0400 Subject: [PATCH 2/2] fix(Logging): adding internal configure auth hub event listener to fix logging race condition (#3899) --- .../Operations/AuthConfigureOperation.swift | 4 ++ .../AWSCognitoAuthPluginConfigTests.swift | 41 +++++++++++++++++++ .../AWSCloudWatchLoggingCategoryClient.swift | 3 +- .../AWSCloudWatchLoggingPlugin.swift | 4 -- 4 files changed, 47 insertions(+), 5 deletions(-) diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Operations/AuthConfigureOperation.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Operations/AuthConfigureOperation.swift index 49ca97e23a..376ffefc4c 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Operations/AuthConfigureOperation.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Operations/AuthConfigureOperation.swift @@ -37,6 +37,9 @@ class AuthConfigureOperation: ConfigureOperation { override public func main() { if isCancelled { finish() + dispatch(result: .failure(AuthError.configuration( + "Configuration operation was cancelled", + "", nil))) return } @@ -51,6 +54,7 @@ class AuthConfigureOperation: ConfigureOperation { for await state in stateSequences { if case .configured = state { finish() + dispatch(result: .success(())) break } } diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginConfigTests.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginConfigTests.swift index 8ba574028e..553406fbe6 100644 --- a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginConfigTests.swift +++ b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginConfigTests.swift @@ -236,4 +236,45 @@ class AWSCognitoAuthPluginConfigTests: XCTestCase { } } + /// Test that the Auth plugin emits `InternalConfigureAuth` that is used by the Logging Category + /// + /// - Given: Given a valid config + /// - When: + /// - I configure auth with the given configuration + /// - Then: + /// - I should receive `InternalConfigureAuth` Hub event + /// + func testEmittingInternalConfigureAuthHubEvent() throws { + let expectation = expectation(description: "conifguration should complete") + let subscription = Amplify.Hub.publisher(for: .auth).sink { payload in + + if payload.eventName == "InternalConfigureAuth" { + expectation.fulfill() + } + } + let plugin = AWSCognitoAuthPlugin() + try Amplify.add(plugin: plugin) + + let categoryConfig = AuthCategoryConfiguration(plugins: [ + "awsCognitoAuthPlugin": [ + "CredentialsProvider": [ + "CognitoIdentity": [ + "Default": [ + "PoolId": "cc", + "Region": "us-east-1" + ] + ] + ] + ] + ]) + let amplifyConfig = AmplifyConfiguration(auth: categoryConfig) + do { + try Amplify.configure(amplifyConfig) + } catch { + XCTFail("Should not throw error. \(error)") + } + wait(for: [expectation], timeout: 5.0) + subscription.cancel() + } + } diff --git a/AmplifyPlugins/Logging/Sources/AWSCloudWatchLoggingPlugin/AWSCloudWatchLoggingCategoryClient.swift b/AmplifyPlugins/Logging/Sources/AWSCloudWatchLoggingPlugin/AWSCloudWatchLoggingCategoryClient.swift index 9a0458f612..af7e1ace28 100644 --- a/AmplifyPlugins/Logging/Sources/AWSCloudWatchLoggingPlugin/AWSCloudWatchLoggingCategoryClient.swift +++ b/AmplifyPlugins/Logging/Sources/AWSCloudWatchLoggingPlugin/AWSCloudWatchLoggingCategoryClient.swift @@ -87,9 +87,10 @@ final class AWSCloudWatchLoggingCategoryClient { enum CognitoEventName: String { case signInAPI = "Auth.signInAPI" case signOutAPI = "Auth.signOutAPI" + case configured = "InternalConfigureAuth" } switch payload.eventName { - case HubPayload.EventName.Auth.signedIn, CognitoEventName.signInAPI.rawValue: + case HubPayload.EventName.Auth.signedIn, CognitoEventName.signInAPI.rawValue, CognitoEventName.configured.rawValue: takeUserIdentifierFromCurrentUser() case HubPayload.EventName.Auth.signedOut, CognitoEventName.signOutAPI.rawValue: self.userIdentifier = nil diff --git a/AmplifyPlugins/Logging/Sources/AWSCloudWatchLoggingPlugin/AWSCloudWatchLoggingPlugin.swift b/AmplifyPlugins/Logging/Sources/AWSCloudWatchLoggingPlugin/AWSCloudWatchLoggingPlugin.swift index ba8721681f..0d15827c21 100644 --- a/AmplifyPlugins/Logging/Sources/AWSCloudWatchLoggingPlugin/AWSCloudWatchLoggingPlugin.swift +++ b/AmplifyPlugins/Logging/Sources/AWSCloudWatchLoggingPlugin/AWSCloudWatchLoggingPlugin.swift @@ -154,10 +154,6 @@ public class AWSCloudWatchLoggingPlugin: LoggingCategoryPlugin { let localStore: LoggingConstraintsLocalStore = UserDefaults.standard localStore.reset() } - - DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(500)) { - self.loggingClient.takeUserIdentifierFromCurrentUser() - } } }