Skip to content

Commit a7452c5

Browse files
authored
fix(auth): Remove parallel event that move auth plugin to an error state and other cleanups (#2612)
- When an error happens in Auth signIn flow, we return the statemachine with an error and sends two parallel events: one for cancelling signIn flow and another to update authentication state to error. Both of these could be processed after the running signIn flow is completed, which can affect a new signIn flow that immediately follow. This PR removes the parallel events and moves the cancel signIn flow to the Task so that the current signIn flow is complete only when the state machine is cleaned up. - The line number and file info passed in recovery suggestion is not giving any useful information. This PR removes those details from the message. - Masked device meta data information
1 parent 2e0e45e commit a7452c5

File tree

11 files changed

+88
-19
lines changed

11 files changed

+88
-19
lines changed

Amplify/Categories/Auth/Error/AuthError.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ extension AuthError: AmplifyError {
7777
.invalidState(_, let recoverySuggestion, _):
7878
return recoverySuggestion
7979
case .unknown:
80-
return AmplifyErrorMessages.shouldNotHappenReportBugToAWS()
80+
return AmplifyErrorMessages.shouldNotHappenReportBugToAWSWithoutLineInfo()
8181
}
8282
}
8383

Amplify/Core/Support/AmplifyErrorMessages.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,13 @@ public struct AmplifyErrorMessages {
2727
"This should not happen. \(reportBugToAWS(file: file, function: function, line: line))"
2828
}
2929

30+
public static func shouldNotHappenReportBugToAWSWithoutLineInfo() -> String {
31+
"""
32+
This should not happen. There is a possibility that there is a bug if this error persists. \
33+
Please take a look at https://github.com/aws-amplify/amplify-swift/issues to see if there \
34+
are any existing issues that match your scenario, and file an issue with the details of \
35+
the bug if there isn't.
36+
"""
37+
}
38+
3039
}

AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Operations/Helpers/HostedUISignInHelper.swift

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,19 @@ struct HostedUISignInHelper {
8181
}
8282

8383
case .error(let error):
84-
throw AuthError.unknown("Sign in reached an error state", error)
84+
await waitForSignInCancel()
85+
throw error.authError
8586

8687
case .signingIn(let signInState):
87-
guard let result = try UserPoolSignInHelper.checkNextStep(signInState) else {
88-
continue
88+
do {
89+
guard let result = try UserPoolSignInHelper.checkNextStep(signInState) else {
90+
continue
91+
}
92+
return result
93+
} catch {
94+
await waitForSignInCancel()
95+
throw error
8996
}
90-
return result
9197
default:
9298
continue
9399
}
@@ -121,4 +127,19 @@ struct HostedUISignInHelper {
121127
await authStateMachine.send(event)
122128
}
123129

130+
private func waitForSignInCancel() async {
131+
await sendCancelSignInEvent()
132+
let stateSequences = await authStateMachine.listen()
133+
for await state in stateSequences {
134+
guard case .configured(let authenticationState, _) = state else {
135+
continue
136+
}
137+
138+
switch authenticationState {
139+
case .signedOut:
140+
return
141+
default: continue
142+
}
143+
}
144+
}
124145
}

AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/CodeGen/Data/DeviceMetadata.swift

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,29 @@ extension DeviceMetadata: Codable { }
3333

3434
extension DeviceMetadata: Equatable { }
3535

36+
extension DeviceMetadata: CustomDebugDictionaryConvertible {
37+
38+
var debugDictionary: [String: Any] {
39+
switch self {
40+
case .noData:
41+
return ["noData": "noData"]
42+
case .metadata(let data):
43+
return [
44+
"deviceKey": data.deviceKey.masked(interiorCount: 5),
45+
"deviceGroupKey": data.deviceGroupKey.masked(interiorCount: 5),
46+
"deviceSecret": data.deviceSecret.masked(interiorCount: 5)
47+
]
48+
}
49+
}
50+
}
51+
52+
extension DeviceMetadata: CustomDebugStringConvertible {
53+
54+
var debugDescription: String {
55+
debugDictionary.debugDescription
56+
}
57+
}
58+
3659
extension CognitoIdentityProviderClientTypes.AuthenticationResultType {
3760

3861
var deviceMetadata: DeviceMetadata {

AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/Authentication/AuthenticationState+Resolver.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,7 @@ extension AuthenticationState {
221221
event: StateMachineEvent) -> StateResolution<StateType> {
222222
if let authEvent = event as? AuthenticationEvent,
223223
case .error(let error) = authEvent.eventType {
224-
let action = CancelSignIn()
225-
return .init(newState: .error(error), actions: [action])
224+
return .init(newState: .error(error))
226225
}
227226
/// Move to signedOut state if cancelSignIn
228227
if let authEvent = event as? AuthenticationEvent,

AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/CustomAuth/CustomSignInState+Resolver.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,8 @@ extension CustomSignInState {
7474

7575
private func errorStateWithCancelSignIn(_ error: SignInError)
7676
-> StateResolution<CustomSignInState> {
77-
let action = CancelSignIn()
7877
return StateResolution(
79-
newState: CustomSignInState.error(error),
80-
actions: [action]
78+
newState: CustomSignInState.error(error)
8179
)
8280
}
8381
}

AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/MigrateAuth/MigrateSignInState+Resolver.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,8 @@ extension MigrateSignInState {
8383

8484
private func errorStateWithCancelSignIn(_ error: SignInError)
8585
-> StateResolution<MigrateSignInState> {
86-
let action = CancelSignIn()
8786
return StateResolution(
88-
newState: MigrateSignInState.error(error),
89-
actions: [action]
87+
newState: MigrateSignInState.error(error)
9088
)
9189
}
9290
}

AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/SRP/SRPSignInState+Resolver.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,8 @@ extension SRPSignInState {
114114

115115
private func errorState(_ error: SignInError)
116116
-> StateResolution<SRPSignInState> {
117-
let action = ThrowSignInError(error: error)
118117
return StateResolution(
119-
newState: SRPSignInState.error(error),
120-
actions: [action]
118+
newState: SRPSignInState.error(error)
121119
)
122120
}
123121

AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/SignIn/HostedUISignInState+Resolver.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ extension HostedUISignInState {
3030

3131
case .showingUI:
3232
if case .throwError(let error) = event.isHostedUIEvent {
33+
// Remove this?
3334
let action = CancelSignIn()
3435
return .init(newState: .error(error), actions: [action])
3536
}
@@ -42,6 +43,7 @@ extension HostedUISignInState {
4243

4344
case .fetchingToken:
4445
if case .throwError(let error) = event.isHostedUIEvent {
46+
// Remove this?
4547
let action = CancelSignIn()
4648
return .init(newState: .error(error), actions: [action])
4749
}

AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Task/AWSAuthSignInTask.swift

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,12 @@ class AWSAuthSignInTask: AuthSignInTask {
3939
try await validateCurrentState()
4040

4141
let authflowType = authFlowType(userPoolConfiguration: userPoolConfiguration)
42-
return try await doSignIn(authflowType: authflowType)
42+
do {
43+
return try await doSignIn(authflowType: authflowType)
44+
} catch {
45+
await waitForSignInCancel()
46+
throw error
47+
}
4348
}
4449

4550
private func validateCurrentState() async throws {
@@ -101,6 +106,22 @@ class AWSAuthSignInTask: AuthSignInTask {
101106
await authStateMachine.send(event)
102107
}
103108

109+
private func waitForSignInCancel() async {
110+
await sendCancelSignInEvent()
111+
let stateSequences = await authStateMachine.listen()
112+
for await state in stateSequences {
113+
guard case .configured(let authenticationState, _) = state else {
114+
continue
115+
}
116+
117+
switch authenticationState {
118+
case .signedOut:
119+
return
120+
default: continue
121+
}
122+
}
123+
}
124+
104125
private func sendSignInEvent(authflowType: AuthFlowType) async {
105126
let signInData = SignInEventData(
106127
username: request.username,

0 commit comments

Comments
 (0)