Skip to content

Commit 71771e4

Browse files
mattcreasermattcreaser
authored
fix(auth): Fix losing session identifier when incorrect otp code is entered during confirm sign up (#3136)
1 parent 0474838 commit 71771e4

File tree

6 files changed

+149
-56
lines changed

6 files changed

+149
-56
lines changed

‎aws-auth-cognito/src/main/java/com/amplifyframework/auth/cognito/actions/SignUpCognitoActions.kt‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import com.amplifyframework.auth.result.step.AuthNextSignUpStep
2727
import com.amplifyframework.auth.result.step.AuthSignUpStep
2828
import com.amplifyframework.statemachine.Action
2929
import com.amplifyframework.statemachine.codegen.actions.SignUpActions
30-
import com.amplifyframework.statemachine.codegen.data.SignUpData
3130
import com.amplifyframework.statemachine.codegen.events.SignUpEvent
3231

3332
internal object SignUpCognitoActions : SignUpActions {
@@ -69,12 +68,9 @@ internal object SignUpCognitoActions : SignUpActions {
6968
}
7069

7170
val codeDeliveryDetails = response?.codeDeliveryDetails.toAuthCodeDeliveryDetails()
72-
val signUpData = SignUpData(
73-
username,
74-
event.signUpData.validationData,
75-
event.signUpData.clientMetadata,
76-
response?.session,
77-
response?.userSub
71+
val signUpData = event.signUpData.copy(
72+
session = response?.session,
73+
userId = response?.userSub
7874
)
7975
if (response?.userConfirmed == true) {
8076
var signUpStep = AuthSignUpStep.DONE
@@ -106,7 +102,7 @@ internal object SignUpCognitoActions : SignUpActions {
106102
SignUpEvent(SignUpEvent.EventType.InitiateSignUpComplete(signUpData, signUpResult))
107103
}
108104
} catch (e: Exception) {
109-
SignUpEvent(SignUpEvent.EventType.ThrowError(e))
105+
SignUpEvent(SignUpEvent.EventType.ThrowError(event.signUpData, e))
110106
}
111107
logger.verbose("$id Sending event ${evt.type}")
112108
dispatcher.send(evt)
@@ -136,13 +132,7 @@ internal object SignUpCognitoActions : SignUpActions {
136132
this.clientMetadata = event.signUpData.clientMetadata
137133
this.session = event.signUpData.session
138134
}
139-
val signUpData = SignUpData(
140-
username,
141-
event.signUpData.validationData,
142-
event.signUpData.clientMetadata,
143-
response?.session,
144-
event.signUpData.userId
145-
)
135+
val signUpData = event.signUpData.copy(session = response?.session)
146136
var signUpStep = AuthSignUpStep.DONE
147137
if (response?.session != null) {
148138
signUpStep = AuthSignUpStep.COMPLETE_AUTO_SIGN_IN
@@ -159,7 +149,7 @@ internal object SignUpCognitoActions : SignUpActions {
159149
)
160150
SignUpEvent(SignUpEvent.EventType.SignedUp(signUpData, signUpResult))
161151
} catch (e: Exception) {
162-
SignUpEvent(SignUpEvent.EventType.ThrowError(e))
152+
SignUpEvent(SignUpEvent.EventType.ThrowError(event.signUpData, e))
163153
}
164154
logger.verbose("$id Sending event ${evt.type}")
165155
dispatcher.send(evt)

‎aws-auth-cognito/src/main/java/com/amplifyframework/auth/cognito/usecases/ConfirmSignUpUseCase.kt‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import com.amplifyframework.auth.result.AuthSignUpResult
2424
import com.amplifyframework.statemachine.codegen.data.SignUpData
2525
import com.amplifyframework.statemachine.codegen.events.SignUpEvent
2626
import com.amplifyframework.statemachine.codegen.states.SignUpState
27+
import com.amplifyframework.statemachine.codegen.states.getSignUpData
2728
import kotlinx.coroutines.flow.drop
2829
import kotlinx.coroutines.flow.first
2930
import kotlinx.coroutines.flow.onSubscription
@@ -37,21 +38,25 @@ internal class ConfirmSignUpUseCase(private val stateMachine: AuthStateMachine)
3738
): AuthSignUpResult {
3839
stateMachine.throwIfNotConfigured()
3940

40-
val startingState = stateMachine.getCurrentState().authSignUpState
41+
val currentState = stateMachine.getCurrentState()
42+
val existingSignUpData = currentState.authSignUpState?.getSignUpData()
43+
44+
val clientMetadata = (options as? AWSCognitoAuthConfirmSignUpOptions)?.clientMetadata
45+
val signUpData = if (existingSignUpData?.username == username) {
46+
existingSignUpData.copy(clientMetadata = clientMetadata)
47+
} else {
48+
SignUpData(
49+
username = username,
50+
validationData = null,
51+
clientMetadata = clientMetadata,
52+
session = null,
53+
userId = null
54+
)
55+
}
4156

4257
val result = stateMachine.state
4358
.onSubscription {
44-
var userId: String? = null
45-
var session: String? = null
46-
if (startingState is SignUpState.AwaitingUserConfirmation &&
47-
startingState.signUpData.username == username
48-
) {
49-
session = startingState.signUpData.session
50-
userId = startingState.signUpResult.userId
51-
}
52-
val clientMetadata = (options as? AWSCognitoAuthConfirmSignUpOptions)?.clientMetadata
53-
val signupData = SignUpData(username, null, clientMetadata, session, userId)
54-
val event = SignUpEvent(SignUpEvent.EventType.ConfirmSignUp(signupData, confirmationCode))
59+
val event = SignUpEvent(SignUpEvent.EventType.ConfirmSignUp(signUpData, confirmationCode))
5560
stateMachine.send(event)
5661
}
5762
.drop(1)

‎aws-auth-cognito/src/main/java/com/amplifyframework/statemachine/codegen/events/SignUpEvent.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ internal class SignUpEvent(
3838

3939
data class SignedUp(val signUpData: SignUpData, val signUpResult: AuthSignUpResult) : EventType()
4040

41-
data class ThrowError(val exception: Exception) : EventType()
41+
data class ThrowError(val signUpData: SignUpData, val exception: Exception) : EventType()
4242
}
4343

4444
override val type: String = eventType.javaClass.simpleName

‎aws-auth-cognito/src/main/java/com/amplifyframework/statemachine/codegen/states/SignUpState.kt‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,11 @@ internal sealed class SignUpState : State {
3131
data class AwaitingUserConfirmation(val signUpData: SignUpData, val signUpResult: AuthSignUpResult) : SignUpState()
3232
data class ConfirmingSignUp(val signUpData: SignUpData) : SignUpState()
3333
data class SignedUp(val signUpData: SignUpData, val signUpResult: AuthSignUpResult) : SignUpState()
34-
data class Error(val exception: Exception, var hasNewResponse: Boolean = true) : SignUpState()
34+
data class Error(
35+
val signUpData: SignUpData,
36+
val exception: Exception,
37+
var hasNewResponse: Boolean = true
38+
) : SignUpState()
3539

3640
class Resolver(private val signUpActions: SignUpActions) :
3741
StateMachineResolver<SignUpState> {
@@ -56,7 +60,7 @@ internal sealed class SignUpState : State {
5660
)
5761
}
5862
is SignUpEvent.EventType.ThrowError -> {
59-
StateResolution(Error(signUpEvent.exception))
63+
StateResolution(Error(signUpEvent.signUpData, signUpEvent.exception))
6064
}
6165
else -> defaultResolution
6266
}
@@ -82,7 +86,7 @@ internal sealed class SignUpState : State {
8286
)
8387
}
8488
is SignUpEvent.EventType.ThrowError -> {
85-
StateResolution(Error(signUpEvent.exception))
89+
StateResolution(Error(signUpEvent.signUpData, signUpEvent.exception))
8690
}
8791
else -> defaultResolution
8892
}
@@ -100,7 +104,7 @@ internal sealed class SignUpState : State {
100104
)
101105
}
102106
is SignUpEvent.EventType.ThrowError -> {
103-
StateResolution(Error(signUpEvent.exception))
107+
StateResolution(Error(signUpEvent.signUpData, signUpEvent.exception))
104108
}
105109
else -> defaultResolution
106110
}
@@ -121,7 +125,7 @@ internal sealed class SignUpState : State {
121125
StateResolution(SignedUp(signUpEvent.signUpData, signUpEvent.signUpResult))
122126
}
123127
is SignUpEvent.EventType.ThrowError -> {
124-
StateResolution(Error(signUpEvent.exception))
128+
StateResolution(Error(signUpEvent.signUpData, signUpEvent.exception))
125129
}
126130
else -> defaultResolution
127131
}
@@ -144,3 +148,12 @@ internal sealed class SignUpState : State {
144148
}
145149
}
146150
}
151+
152+
internal fun SignUpState.getSignUpData(): SignUpData? = when (this) {
153+
is SignUpState.AwaitingUserConfirmation -> this.signUpData
154+
is SignUpState.ConfirmingSignUp -> this.signUpData
155+
is SignUpState.Error -> this.signUpData
156+
is SignUpState.InitiatingSignUp -> this.signUpData
157+
is SignUpState.NotStarted -> null
158+
is SignUpState.SignedUp -> this.signUpData
159+
}

‎aws-auth-cognito/src/test/java/com/amplifyframework/auth/cognito/usecases/ConfirmSignUpUseCaseTest.kt‎

Lines changed: 106 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,24 @@ import com.amplifyframework.auth.cognito.testUtil.withSignUpEvent
2222
import com.amplifyframework.auth.result.AuthSignUpResult
2323
import com.amplifyframework.auth.result.step.AuthNextSignUpStep
2424
import com.amplifyframework.auth.result.step.AuthSignUpStep
25+
import com.amplifyframework.statemachine.codegen.data.SignUpData
2526
import com.amplifyframework.statemachine.codegen.events.SignUpEvent
2627
import com.amplifyframework.statemachine.codegen.states.AuthState
2728
import com.amplifyframework.statemachine.codegen.states.AuthenticationState
2829
import com.amplifyframework.statemachine.codegen.states.SignUpState
2930
import io.kotest.assertions.throwables.shouldThrowAny
31+
import io.kotest.matchers.nulls.shouldBeNull
3032
import io.kotest.matchers.shouldBe
3133
import io.mockk.coEvery
3234
import io.mockk.coVerify
3335
import io.mockk.every
3436
import io.mockk.justRun
3537
import io.mockk.mockk
3638
import kotlinx.coroutines.ExperimentalCoroutinesApi
39+
import kotlinx.coroutines.async
3740
import kotlinx.coroutines.flow.MutableStateFlow
3841
import kotlinx.coroutines.launch
42+
import kotlinx.coroutines.test.TestScope
3943
import kotlinx.coroutines.test.runCurrent
4044
import kotlinx.coroutines.test.runTest
4145
import org.junit.Test
@@ -50,6 +54,14 @@ class ConfirmSignUpUseCaseTest {
5054
}
5155
private val useCase = ConfirmSignUpUseCase(stateMachine = stateMachine)
5256

57+
private val signUpData = SignUpData(
58+
username = "username",
59+
validationData = mapOf("key" to "value"),
60+
clientMetadata = mapOf("meta" to "data"),
61+
session = "session",
62+
userId = "userId"
63+
)
64+
5365
@Test
5466
fun `fails if not configured`() = runTest {
5567
val expectedAuthError = InvalidUserPoolConfigurationException()
@@ -75,30 +87,96 @@ class ConfirmSignUpUseCaseTest {
7587

7688
runCurrent()
7789
stateFlow.emit(mockAuthState(SignUpState.ConfirmingSignUp(mockk())))
78-
stateFlow.emit(mockAuthState(SignUpState.Error(exception)))
90+
stateFlow.emit(mockAuthState(SignUpState.Error(mockk(), exception)))
7991
}
8092

8193
@Test
8294
fun `sends expected event`() = runTest {
8395
coEvery { stateMachine.getCurrentState().authNState } returns AuthenticationState.Configured()
8496
coEvery { stateMachine.getCurrentState().authSignUpState } returns null
8597

86-
launch {
87-
useCase.execute("user", "pass")
98+
executeUseCaseToCompletion()
99+
100+
coVerify {
101+
stateMachine.send(
102+
withSignUpEvent<SignUpEvent.EventType.ConfirmSignUp> { event ->
103+
event.signUpData.username shouldBe "user"
104+
event.confirmationCode shouldBe "pass"
105+
}
106+
)
107+
}
108+
}
88109

89-
coVerify {
90-
stateMachine.send(
91-
withSignUpEvent<SignUpEvent.EventType.ConfirmSignUp> { event ->
92-
event.signUpData.username shouldBe "user"
93-
event.confirmationCode shouldBe "pass"
110+
@Test
111+
fun `uses signUpData values from existing error state`() = runTest {
112+
coEvery { stateMachine.getCurrentState().authNState } returns AuthenticationState.Configured()
113+
coEvery { stateMachine.getCurrentState().authSignUpState } returns
114+
SignUpState.Error(signUpData = signUpData, exception = mockk())
115+
116+
executeUseCaseToCompletion(username = signUpData.username)
117+
118+
coVerify {
119+
stateMachine.send(
120+
withSignUpEvent<SignUpEvent.EventType.ConfirmSignUp> { event ->
121+
event.signUpData.run {
122+
username shouldBe signUpData.username
123+
validationData shouldBe signUpData.validationData
124+
clientMetadata.shouldBeNull() // was not passed in options
125+
session shouldBe signUpData.session
126+
userId shouldBe signUpData.userId
94127
}
95-
)
96-
}
128+
event.confirmationCode shouldBe "pass"
129+
}
130+
)
97131
}
132+
}
98133

99-
runCurrent()
100-
stateFlow.emit(mockAuthState(SignUpState.ConfirmingSignUp(mockk())))
101-
stateFlow.emit(mockAuthState(SignUpState.SignedUp(mockk(), mockk())))
134+
@Test
135+
fun `uses session value from existing AwaitingConfirmation state`() = runTest {
136+
coEvery { stateMachine.getCurrentState().authNState } returns AuthenticationState.Configured()
137+
coEvery { stateMachine.getCurrentState().authSignUpState } returns
138+
SignUpState.AwaitingUserConfirmation(signUpData = signUpData, signUpResult = mockk())
139+
140+
executeUseCaseToCompletion(username = signUpData.username)
141+
142+
coVerify {
143+
stateMachine.send(
144+
withSignUpEvent<SignUpEvent.EventType.ConfirmSignUp> { event ->
145+
event.signUpData.run {
146+
username shouldBe signUpData.username
147+
validationData shouldBe signUpData.validationData
148+
clientMetadata.shouldBeNull() // was not passed in options
149+
session shouldBe signUpData.session
150+
userId shouldBe signUpData.userId
151+
}
152+
event.confirmationCode shouldBe "pass"
153+
}
154+
)
155+
}
156+
}
157+
158+
@Test
159+
fun `does not use values from existing state if username does not match`() = runTest {
160+
coEvery { stateMachine.getCurrentState().authNState } returns AuthenticationState.Configured()
161+
coEvery { stateMachine.getCurrentState().authSignUpState } returns
162+
SignUpState.AwaitingUserConfirmation(signUpData = signUpData, signUpResult = mockk())
163+
164+
executeUseCaseToCompletion(username = "bob")
165+
166+
coVerify {
167+
stateMachine.send(
168+
withSignUpEvent<SignUpEvent.EventType.ConfirmSignUp> { event ->
169+
event.signUpData.run {
170+
username shouldBe "bob"
171+
validationData.shouldBeNull()
172+
clientMetadata.shouldBeNull() // was not passed in options
173+
session.shouldBeNull()
174+
userId.shouldBeNull()
175+
}
176+
event.confirmationCode shouldBe "pass"
177+
}
178+
)
179+
}
102180
}
103181

104182
@Test
@@ -116,17 +194,24 @@ class ConfirmSignUpUseCaseTest {
116194
coEvery { stateMachine.getCurrentState().authNState } returns AuthenticationState.Configured()
117195
coEvery { stateMachine.getCurrentState().authSignUpState } returns null
118196

119-
launch {
120-
val result = useCase.execute("user", "pass")
121-
result shouldBe expectedResult
122-
}
123-
124-
runCurrent()
125-
stateFlow.emit(mockAuthState(SignUpState.ConfirmingSignUp(mockk())))
126-
stateFlow.emit(mockAuthState(SignUpState.SignedUp(mockk(), expectedResult)))
197+
val result = executeUseCaseToCompletion(signUpResult = expectedResult)
198+
result shouldBe expectedResult
127199
}
128200

129201
private fun mockAuthState(signUpState: SignUpState): AuthState = mockk {
130202
coEvery { authSignUpState } returns signUpState
131203
}
204+
205+
@Suppress("SuspendFunctionOnCoroutineScope")
206+
private suspend fun TestScope.executeUseCaseToCompletion(
207+
username: String = "user",
208+
confirmationCode: String = "pass",
209+
signUpResult: AuthSignUpResult = mockk()
210+
): AuthSignUpResult {
211+
val result = async { useCase.execute(username, confirmationCode) }
212+
runCurrent()
213+
stateFlow.emit(mockAuthState(SignUpState.ConfirmingSignUp(mockk())))
214+
stateFlow.emit(mockAuthState(SignUpState.SignedUp(mockk(), signUpResult)))
215+
return result.await()
216+
}
132217
}

‎aws-auth-cognito/src/test/java/com/amplifyframework/auth/cognito/usecases/SignUpUseCaseTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ class SignUpUseCaseTest {
7474
}
7575

7676
runCurrent()
77-
stateFlow.emit(mockAuthState(SignUpState.Error(exception)))
77+
stateFlow.emit(mockAuthState(SignUpState.Error(mockk(), exception)))
7878
}
7979

8080
@Test

0 commit comments

Comments
 (0)