Skip to content

Commit

Permalink
mfa association bug: mfa channel was hardcoded as main email
Browse files Browse the repository at this point in the history
  • Loading branch information
@mrFlick72
mrFlick72 committed Jul 27, 2024
1 parent 4b04cd9 commit ad0e766
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 25 deletions.
4 changes: 2 additions & 2 deletions src/main/kotlin/com/vauthenticator/server/mfa/domain/MfaSenderAndVerifier.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class AccountAwareOtpMfaVerifier(
mfaChannel: String,
challenge: MfaChallenge
) {
mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName)
mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, mfaChannel)
.map {
val account = accountRepository.accountFor(userName).get()
if (!it.associated) {
Expand All @@ -68,7 +68,7 @@ class AccountAwareOtpMfaVerifier(
mfaChannel: String,
challenge: MfaChallenge
) {
mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName)
mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, mfaChannel)
.map {
val account = accountRepository.accountFor(userName).get()
if (it.associated) {
Expand Down
12 changes: 11 additions & 1 deletion ...main/kotlin/com/vauthenticator/server/mfa/repository/DynamoMfaAccountMethodsRepository.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class DynamoMfaAccountMethodsRepository(
mfaMfaMethod: MfaMethod,
mfaChannel: String
): Optional<MfaAccountMethod> =
Optional.ofNullable(findAll(userName).find { it.method == mfaMfaMethod })
Optional.ofNullable(findAll(userName).find { it.method == mfaMfaMethod && it.mfaChannel == mfaChannel})


override fun findAll(userName: String): List<MfaAccountMethod> =
Expand All @@ -42,6 +42,16 @@ class DynamoMfaAccountMethodsRepository(
QueryRequest.builder().tableName(tableName).keyConditionExpression("user_name=:email")
.expressionAttributeValues(mapOf(":email" to email.asDynamoAttribute())).build()
).items()
private fun getFromDynamo(email: String, mfaChannel : String) = dynamoDbClient.query(
QueryRequest.builder().tableName(tableName).keyConditionExpression("user_name=:email AND mfa_channel=:mfaChannel")
.expressionAttributeValues(
mapOf(
":email" to email.asDynamoAttribute(),
":mfaChannel" to mfaChannel.asDynamoAttribute(),
)
)
.build()
).items()

override fun save(
userName: String,
Expand Down
44 changes: 22 additions & 22 deletions src/test/kotlin/com/vauthenticator/server/mfa/domain/AccountAwareOtpMfaVerifierTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ internal class AccountAwareOtpMfaVerifierTest {

private val account = anAccount()
private val userName = account.email
private val email = userName
private val email = "[email protected]"
private val challenge = MfaChallenge("AN_MFA_CHALLENGE")


Expand All @@ -45,7 +45,7 @@ internal class AccountAwareOtpMfaVerifierTest {
internal fun `when associated mfa challenge is successfully verified`() {
val challenge = MfaChallenge("AN_MFA_CHALLENGE")

every { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) } returns
every { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, email) } returns
Optional.of(
MfaAccountMethod(
userName,
Expand All @@ -56,16 +56,16 @@ internal class AccountAwareOtpMfaVerifierTest {
)
)
every { accountRepository.accountFor(userName) } returns Optional.of(account)
every { otpMfa.verify(account, MfaMethod.EMAIL_MFA_METHOD, userName, challenge) } just runs
every { otpMfa.verify(account, MfaMethod.EMAIL_MFA_METHOD, email, challenge) } just runs

underTest.verifyAssociatedMfaChallengeFor(userName, MfaMethod.EMAIL_MFA_METHOD, userName, challenge)
underTest.verifyAssociatedMfaChallengeFor(userName, MfaMethod.EMAIL_MFA_METHOD, email, challenge)

verify { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) }
verify { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, email) }
}

@Test
internal fun `when not associated mfa challenge fails on verification`() {
every { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) } returns
every { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, email) } returns
Optional.of(
MfaAccountMethod(
userName,
Expand All @@ -76,25 +76,25 @@ internal class AccountAwareOtpMfaVerifierTest {
)
)
every { accountRepository.accountFor(userName) } returns Optional.of(account)
every { otpMfa.verify(account, MfaMethod.EMAIL_MFA_METHOD, userName, challenge) } just runs
every { otpMfa.verify(account, MfaMethod.EMAIL_MFA_METHOD, email, challenge) } just runs

assertThrows(UnAssociatedMfaVerificationException::class.java) {
underTest.verifyAssociatedMfaChallengeFor(
userName,
MfaMethod.EMAIL_MFA_METHOD,
userName,
email,
challenge
)
}

verify { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) }
verify { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, email) }
}

@Test
internal fun `when associated mfa challenge fails on verification`() {
every { accountRepository.accountFor(account.email) } returns Optional.of(account)
every { otpMfa.verify(account, MfaMethod.EMAIL_MFA_METHOD, account.email, challenge) } throws MfaException("")
every { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) } returns
every { otpMfa.verify(account, MfaMethod.EMAIL_MFA_METHOD, email, challenge) } throws MfaException("")
every { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, email) } returns
Optional.of(
MfaAccountMethod(
userName,
Expand All @@ -106,23 +106,23 @@ internal class AccountAwareOtpMfaVerifierTest {
)
assertThrows(MfaException::class.java) {
underTest.verifyAssociatedMfaChallengeFor(
account.email,
userName,
MfaMethod.EMAIL_MFA_METHOD,
account.email,
email,
challenge
)
}

verify { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) }
verify { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, email) }
}



@Test
internal fun `when not associated mfa verification succeed on association verification`() {
every { accountRepository.accountFor(account.email) } returns Optional.of(account)
every { otpMfa.verify(account, MfaMethod.EMAIL_MFA_METHOD, account.email, challenge) } throws MfaException("")
every { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) } returns
every { otpMfa.verify(account, MfaMethod.EMAIL_MFA_METHOD, email, challenge) } throws MfaException("")
every { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, email) } returns
Optional.of(
MfaAccountMethod(
userName,
Expand All @@ -136,19 +136,19 @@ internal class AccountAwareOtpMfaVerifierTest {
underTest.verifyMfaChallengeToBeAssociatedFor(
account.email,
MfaMethod.EMAIL_MFA_METHOD,
account.email,
email,
challenge
)
}

verify { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) }
verify { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, email) }
}

@Test
internal fun `when associated mfa verification succeed on association verification`() {
every { accountRepository.accountFor(account.email) } returns Optional.of(account)
every { otpMfa.verify(account, MfaMethod.EMAIL_MFA_METHOD, account.email, challenge) } throws MfaException("")
every { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) } returns
every { otpMfa.verify(account, MfaMethod.EMAIL_MFA_METHOD, email, challenge) } throws MfaException("")
every { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, email) } returns
Optional.of(
MfaAccountMethod(
userName,
Expand All @@ -162,11 +162,11 @@ internal class AccountAwareOtpMfaVerifierTest {
underTest.verifyMfaChallengeToBeAssociatedFor(
account.email,
MfaMethod.EMAIL_MFA_METHOD,
account.email,
email,
challenge
)
}

verify { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) }
verify { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, email) }
}
}

0 comments on commit ad0e766

Please sign in to comment.