Skip to content

Commit

Permalink
split mfa verification for mfa association and associated mfa
Browse files Browse the repository at this point in the history
  • Loading branch information
mrFlick72 committed Jul 27, 2024
1 parent 57d216a commit c4c2226
Show file tree
Hide file tree
Showing 14 changed files with 204 additions and 41 deletions.
5 changes: 3 additions & 2 deletions src/main/kotlin/com/vauthenticator/server/mfa/MfaConfig.kt
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@ class MfaConfig {

@Bean
fun otpMfaVerifier(
otpMfa: OtpMfa,
accountRepository: AccountRepository,
otpMfa: OtpMfa
) = AccountAwareOtpMfaVerifier(accountRepository, otpMfa)
mfaAccountMethodsRepository : MfaAccountMethodsRepository,
) = AccountAwareOtpMfaVerifier(accountRepository, otpMfa, mfaAccountMethodsRepository)

@Bean
fun mfaMailSender(
Expand Down
15 changes: 12 additions & 3 deletions src/main/kotlin/com/vauthenticator/server/mfa/domain/Mfa.kt
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ sealed class MfaDevice(val mfaMethod: MfaMethod)
class EmailMfaDevice(val email: String, mfaMethod: MfaMethod) : MfaDevice(mfaMethod)


class MfaException(message: String) : AuthenticationException(message)

class MfaFailureEvent(authentication: Authentication, exception: AuthenticationException) :
AbstractAuthenticationFailureEvent(authentication, exception) {}

Expand All @@ -29,5 +27,16 @@ value class MfaChallenge(private val content: String) {
}

enum class MfaMethod { EMAIL_MFA_METHOD, SMS_MFA_METHOD, OTP_MFA_METHOD }
data class MfaAccountMethod(
val userName: String,
val key: Kid,
val method: MfaMethod,
val mfaChannel: String,
val associated: Boolean
)

class MfaException(message: String) : AuthenticationException(message)

data class MfaAccountMethod(val userName: String, val key: Kid, val method: MfaMethod, val mfaChannel : String)
// todo
class UnAssociatedMfaVerificationException(message: String) : AuthenticationException(message)
class AssociatedMfaVerificationException(message: String) : AuthenticationException(message)
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class MfaMethodsEnrollmentAssociation(
associate(
ticketId,
) {
otpMfaVerifier.verifyMfaChallengeFor(
otpMfaVerifier.verifyMfaChallengeToBeAssociatedFor(
it.userName,
it.context.mfaMethod(),
it.context.mfaChannel(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,22 @@ package com.vauthenticator.server.mfa.domain

import com.vauthenticator.server.account.repository.AccountRepository
import com.vauthenticator.server.email.EMailSenderService
import com.vauthenticator.server.mfa.repository.MfaAccountMethodsRepository


interface OtpMfaSender {
fun sendMfaChallenge(userName: String, mfaMethod: MfaMethod, mfaChannel: String)
}

interface OtpMfaVerifier {
fun verifyMfaChallengeFor(
fun verifyMfaChallengeToBeAssociatedFor(
userName: String,
mfaMethod: MfaMethod,
mfaChannel: String,
challenge: MfaChallenge
)

fun verifyAssociatedMfaChallengeFor(
userName: String,
mfaMethod: MfaMethod,
mfaChannel: String,
Expand All @@ -33,16 +41,42 @@ class OtpMfaEmailSender(

class AccountAwareOtpMfaVerifier(
private val accountRepository: AccountRepository,
private val otpMfa: OtpMfa
private val otpMfa: OtpMfa,
private val mfaAccountMethodsRepository: MfaAccountMethodsRepository
) : OtpMfaVerifier {
override fun verifyMfaChallengeFor(

override fun verifyMfaChallengeToBeAssociatedFor(
userName: String,
mfaMethod: MfaMethod,
mfaChannel: String,
challenge: MfaChallenge
) {
val account = accountRepository.accountFor(userName).get()
otpMfa.verify(account, mfaMethod, mfaChannel, challenge)
mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName)
.map {
val account = accountRepository.accountFor(userName).get()
if (!it.associated) {
otpMfa.verify(account, mfaMethod, mfaChannel, challenge)
} else {
throw AssociatedMfaVerificationException("Mfa Challenge verification failed: this mfa method is already associated")
}
}
}

override fun verifyAssociatedMfaChallengeFor(
userName: String,
mfaMethod: MfaMethod,
mfaChannel: String,
challenge: MfaChallenge
) {
mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName)
.map {
val account = accountRepository.accountFor(userName).get()
if (it.associated) {
otpMfa.verify(account, mfaMethod, mfaChannel, challenge)
} else {
throw UnAssociatedMfaVerificationException("Mfa Challenge verification failed: this mfa method has to be associated")
}
}
}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.vauthenticator.server.mfa.repository

import com.vauthenticator.server.extentions.asDynamoAttribute
import com.vauthenticator.server.extentions.valueAsBoolFor
import com.vauthenticator.server.extentions.valueAsStringFor
import com.vauthenticator.server.keys.*
import com.vauthenticator.server.mfa.domain.MfaAccountMethod
Expand Down Expand Up @@ -32,7 +33,8 @@ class DynamoMfaAccountMethodsRepository(
userName,
Kid(it.valueAsStringFor("key_id")),
valueOf(it.valueAsStringFor("mfa_method")),
it.valueAsStringFor("mfa_channel")
it.valueAsStringFor("mfa_channel"),
it.valueAsBoolFor("associated")
)
}

Expand All @@ -49,7 +51,7 @@ class DynamoMfaAccountMethodsRepository(
): MfaAccountMethod {
val kid = keyRepository.createKeyFrom(masterKid, KeyType.SYMMETRIC, KeyPurpose.MFA)
storeOnDynamo(userName, mfaMfaMethod, mfaChannel, kid, associated)
return MfaAccountMethod(userName, kid, mfaMfaMethod, mfaChannel)
return MfaAccountMethod(userName, kid, mfaMfaMethod, mfaChannel, associated)
}

private fun storeOnDynamo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,10 @@ import com.vauthenticator.server.mfa.domain.MfaAccountMethod
import com.vauthenticator.server.mfa.domain.MfaMethod
import java.util.*

//todo the interface has to take in account the enrolled method

interface MfaAccountMethodsRepository {

// fun findOne(userName: String, mfaChannel : String): Optional<MfaAccountMethod>
fun findOne(userName: String, mfaMfaMethod: MfaMethod, mfaChannel : String): Optional<MfaAccountMethod>
fun findOne(userName: String, mfaMfaMethod: MfaMethod, mfaChannel: String): Optional<MfaAccountMethod>
fun findAll(userName: String): List<MfaAccountMethod>
fun save(userName: String, mfaMfaMethod: MfaMethod, mfaChannel : String, associated : Boolean): MfaAccountMethod
fun save(userName: String, mfaMfaMethod: MfaMethod, mfaChannel: String, associated: Boolean): MfaAccountMethod
}

Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class MfaController(
try {
val defaultMfaChannel = mfaChannel.orElseGet { authentication.name }

otpMfaVerifier.verifyMfaChallengeFor(authentication.name, mfaMethod, defaultMfaChannel, MfaChallenge(mfaCode))
otpMfaVerifier.verifyAssociatedMfaChallengeFor(authentication.name, mfaMethod, defaultMfaChannel, MfaChallenge(mfaCode))

publisher.publishEvent(MfaSuccessEvent(authentication))
nextHopeLoginWorkflowSuccessHandler.onAuthenticationSuccess(request, response, authentication)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,55 +1,172 @@
package com.vauthenticator.server.mfa.domain

import com.vauthenticator.server.account.repository.AccountRepository
import com.vauthenticator.server.keys.Kid
import com.vauthenticator.server.mfa.repository.MfaAccountMethodsRepository
import com.vauthenticator.server.support.AccountTestFixture.anAccount
import io.mockk.every
import io.mockk.impl.annotations.MockK
import io.mockk.junit5.MockKExtension
import io.mockk.just
import io.mockk.runs
import io.mockk.verify
import org.junit.jupiter.api.Assertions.assertThrows
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import java.util.*

@ExtendWith(MockKExtension::class)
internal class AccountAwareOtpMfaVerifierTest {

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


@MockK
lateinit var accountRepository: AccountRepository

@MockK
lateinit var mfaAccountMethodsRepository: MfaAccountMethodsRepository

@MockK
lateinit var otpMfa: OtpMfa

lateinit var underTest: OtpMfaVerifier

@BeforeEach
fun setUp() {
underTest = AccountAwareOtpMfaVerifier(accountRepository, otpMfa, mfaAccountMethodsRepository)
}

@Test
internal fun `when a challenge is successfully verified`() {
val account = anAccount()
internal fun `when associated mfa challenge is successfully verified`() {
val challenge = MfaChallenge("AN_MFA_CHALLENGE")
val underTest = AccountAwareOtpMfaVerifier(accountRepository, otpMfa)

every { accountRepository.accountFor(account.email) } returns Optional.of(account)
every { otpMfa.verify(account, MfaMethod.EMAIL_MFA_METHOD, account.email, challenge) } just runs
every { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) } returns
Optional.of(
MfaAccountMethod(
userName,
Kid("A_KID"),
MfaMethod.EMAIL_MFA_METHOD,
email,
true
)
)
every { accountRepository.accountFor(userName) } returns Optional.of(account)
every { otpMfa.verify(account, MfaMethod.EMAIL_MFA_METHOD, userName, challenge) } just runs

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

underTest.verifyMfaChallengeFor(account.email, MfaMethod.EMAIL_MFA_METHOD, account.email, challenge)
verify { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) }
}

@Test
internal fun `when a challenge fails on verification`() {
val account = anAccount()
val challenge = MfaChallenge("AN_MFA_CHALLENGE")
internal fun `when not associated mfa challenge fails on verification`() {
every { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) } returns
Optional.of(
MfaAccountMethod(
userName,
Kid("A_KID"),
MfaMethod.EMAIL_MFA_METHOD,
email,
false
)
)
every { accountRepository.accountFor(userName) } returns Optional.of(account)
every { otpMfa.verify(account, MfaMethod.EMAIL_MFA_METHOD, userName, challenge) } just runs

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

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

@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
Optional.of(
MfaAccountMethod(
userName,
Kid("A_KID"),
MfaMethod.EMAIL_MFA_METHOD,
email,
true
)
)
assertThrows(MfaException::class.java) {
underTest.verifyAssociatedMfaChallengeFor(
account.email,
MfaMethod.EMAIL_MFA_METHOD,
account.email,
challenge
)
}

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


val underTest = AccountAwareOtpMfaVerifier(accountRepository, otpMfa)

@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
Optional.of(
MfaAccountMethod(
userName,
Kid("A_KID"),
MfaMethod.EMAIL_MFA_METHOD,
email,
false
)
)
assertThrows(MfaException::class.java) {
underTest.verifyMfaChallengeFor(
underTest.verifyMfaChallengeToBeAssociatedFor(
account.email,
MfaMethod.EMAIL_MFA_METHOD,
account.email,
challenge
)
}

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

@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
Optional.of(
MfaAccountMethod(
userName,
Kid("A_KID"),
MfaMethod.EMAIL_MFA_METHOD,
email,
true
)
)
assertThrows(AssociatedMfaVerificationException::class.java) {
underTest.verifyMfaChallengeToBeAssociatedFor(
account.email,
MfaMethod.EMAIL_MFA_METHOD,
account.email,
challenge
)
}

verify { mfaAccountMethodsRepository.findOne(userName, MfaMethod.EMAIL_MFA_METHOD, userName) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ class MfaMethodsEnrollmentTest {
account.email,
Kid("A_KID"),
EMAIL_MFA_METHOD,
emailMfaChannel
emailMfaChannel,
true
)

@BeforeEach
Expand Down Expand Up @@ -90,7 +91,7 @@ class MfaMethodsEnrollmentTest {
)
} returns Optional.of(emailMfaAccountMethod)
every { ticketCreator.createTicketFor(account, clientAppId, ticketContext(emailMfaChannel)) } returns ticketId
every { mfaSender.sendMfaChallenge(account.email, EMAIL_MFA_METHOD,emailMfaChannel) } just runs
every { mfaSender.sendMfaChallenge(account.email, EMAIL_MFA_METHOD, emailMfaChannel) } just runs

val actual = uut.enroll(account, EMAIL_MFA_METHOD, emailMfaChannel, clientAppId, true)

Expand All @@ -102,7 +103,7 @@ class MfaMethodsEnrollmentTest {
)
}
verify { ticketCreator.createTicketFor(account, clientAppId, ticketContext(emailMfaChannel)) }
verify { mfaSender.sendMfaChallenge(account.email, EMAIL_MFA_METHOD,emailMfaChannel) }
verify { mfaSender.sendMfaChallenge(account.email, EMAIL_MFA_METHOD, emailMfaChannel) }

assertEquals(ticketId, actual)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class TaimosOtpMfaTest {
0L
)
every { mfaAccountMethodsRepository.findOne(email, MfaMethod.EMAIL_MFA_METHOD, email) } returns
of(MfaAccountMethod(email, Kid("A_KID"), MfaMethod.EMAIL_MFA_METHOD, email))
of(MfaAccountMethod(email, Kid("A_KID"), MfaMethod.EMAIL_MFA_METHOD, email, true))

every { keyRepository.keyFor(Kid("A_KID"), KeyPurpose.MFA) } returns key
every { keyDecrypter.decryptKey("QV9FTkNSWVBURURfS0VZ") } returns "QV9ERUNSWVBURURfU1lNTUVUUklDX0tFWQ=="
Expand Down
Loading

0 comments on commit c4c2226

Please sign in to comment.