Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mfa device id and key id should be immutable across multiple save request #261

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,16 @@ class DynamoMfaAccountMethodsRepository(
mfaChannel: String,
associated: Boolean
): MfaAccountMethod {
//todo kid and device id should be the same across save request

val kid = keyRepository.createKeyFrom(masterKid, KeyType.SYMMETRIC, KeyPurpose.MFA)
val mfaDeviceId = mfaDeviceIdGenerator.invoke()
storeOnDynamo(userName, mfaMfaMethod, mfaChannel, mfaDeviceId, kid, associated)
val (kid, mfaDeviceId) = findBy(userName, mfaMfaMethod, mfaChannel)
.map {
listOf(it.key, it.mfaDeviceId)
}.orElseGet {
val kid = keyRepository.createKeyFrom(masterKid, KeyType.SYMMETRIC, KeyPurpose.MFA)
val mfaDeviceId = mfaDeviceIdGenerator.invoke()
listOf(kid, mfaDeviceId)
}

storeOnDynamo(userName, mfaMfaMethod, mfaChannel, mfaDeviceId as MfaDeviceId, kid as Kid, associated)
return MfaAccountMethod(userName, mfaDeviceId, kid, mfaMfaMethod, mfaChannel, associated)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,34 @@ class JdbcMfaAccountMethodsRepository(
mfaChannel: String,
associated: Boolean
): MfaAccountMethod {
val kid = keyRepository.createKeyFrom(masterKid, KeyType.SYMMETRIC, KeyPurpose.MFA)
val mfaDeviceId = mfaDeviceIdGenerator.invoke()
val (kid, mfaDeviceId) = findBy(userName, mfaMfaMethod, mfaChannel)
.map {
listOf(it.key, it.mfaDeviceId)
}.orElseGet {
val kid = keyRepository.createKeyFrom(masterKid, KeyType.SYMMETRIC, KeyPurpose.MFA)
val mfaDeviceId = mfaDeviceIdGenerator.invoke()
listOf(kid, mfaDeviceId)
}

jdbcTemplate.update(
"INSERT INTO MFA_ACCOUNT_METHODS (user_name, mfa_device_id, mfa_method, mfa_channel, key_id, associated) VALUES (?,?,?,?,?,?)",
userName, mfaDeviceId.content, mfaMfaMethod.name, mfaChannel, kid.content(), associated
)
"""INSERT INTO MFA_ACCOUNT_METHODS (user_name, mfa_device_id, mfa_method, mfa_channel, key_id, associated) VALUES (?,?,?,?,?,?)
ON CONFLICT(user_name, mfa_channel)
DO UPDATE SET mfa_device_id=?,
mfa_method=?,
key_id=?,
associated=?
""",
userName,
(mfaDeviceId as MfaDeviceId).content,
mfaMfaMethod.name,
mfaChannel,
(kid as Kid).content(),
associated,
mfaDeviceId.content,
mfaMfaMethod.name,
kid.content(),
associated
)

return MfaAccountMethod(userName, mfaDeviceId, kid, mfaMfaMethod, mfaChannel, associated)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,17 @@ abstract class AbstractMfaAccountMethodsRepositoryTest {
lateinit var uut: MfaAccountMethodsRepository
abstract fun initMfaAccountMethodsRepository(): MfaAccountMethodsRepository
abstract fun resetDatabase()

@BeforeEach
fun setUp() {
every { keyRepository.createKeyFrom(masterKid, KeyType.SYMMETRIC, KeyPurpose.MFA) } returns key

resetDatabase()
uut = initMfaAccountMethodsRepository()
}

@Test
fun `when a mfa account method is stored`() {
every { keyRepository.createKeyFrom(masterKid, KeyType.SYMMETRIC, KeyPurpose.MFA) } returns key

uut.save(email, MfaMethod.EMAIL_MFA_METHOD, email, true)
val mfaAccountMethods = uut.findAll(email)
assertEquals(
Expand Down Expand Up @@ -86,7 +86,6 @@ abstract class AbstractMfaAccountMethodsRepositoryTest {

@Test
fun `when decide what mfa use as default`() {
every { keyRepository.createKeyFrom(masterKid, KeyType.SYMMETRIC, KeyPurpose.MFA) } returns key
uut.save(email, MfaMethod.EMAIL_MFA_METHOD, email, true)

val expected = Optional.of(mfaDeviceId)
Expand All @@ -95,4 +94,23 @@ abstract class AbstractMfaAccountMethodsRepositoryTest {

assertEquals(expected, defaultDevice)
}

@Test
fun `when a mfa account method is stored and then enabled`() {
uut.save(email, MfaMethod.EMAIL_MFA_METHOD, email, false)
val beforeToBeAssociated = uut.findBy(email, MfaMethod.EMAIL_MFA_METHOD, email).get()

uut.save(email, MfaMethod.EMAIL_MFA_METHOD, email, true)
val afterAssociated = uut.findBy(email, MfaMethod.EMAIL_MFA_METHOD, email).get()


assertEquals(afterAssociated.mfaDeviceId, beforeToBeAssociated.mfaDeviceId)
assertEquals(afterAssociated.mfaChannel, beforeToBeAssociated.mfaChannel)
assertEquals(afterAssociated.mfaMethod, beforeToBeAssociated.mfaMethod)
assertEquals(beforeToBeAssociated.associated, false)
assertEquals(afterAssociated.associated, true)
assertEquals(afterAssociated.key, beforeToBeAssociated.key)
assertEquals(afterAssociated.userName, beforeToBeAssociated.userName)
}

}