Merge pull request #261 from VAuthenticator/mfa-device-id-and-key-id-…

…shoyld-be-immutable-acrose-multiple-save-request Mfa device id and key id should be immutable across multiple save request
VAuthenticator · Nov 25, 2024 · 907c0d8 · 907c0d8
2 parents 2fcafdf + 0ebdb61
commit 907c0d8
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 13 deletions.
diff --git a/...otlin/com/vauthenticator/server/mfa/adapter/dynamodb/DynamoMfaAccountMethodsRepository.kt b/...otlin/com/vauthenticator/server/mfa/adapter/dynamodb/DynamoMfaAccountMethodsRepository.kt
@@ -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)
     }
 

diff --git a/...main/kotlin/com/vauthenticator/server/mfa/adapter/jdbc/JdbcMfaAccountMethodsRepository.kt b/...main/kotlin/com/vauthenticator/server/mfa/adapter/jdbc/JdbcMfaAccountMethodsRepository.kt
@@ -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)
     }

diff --git a/...t/kotlin/com/vauthenticator/server/mfa/adapter/AbstractMfaAccountMethodsRepositoryTest.kt b/...t/kotlin/com/vauthenticator/server/mfa/adapter/AbstractMfaAccountMethodsRepositoryTest.kt
@@ -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(
@@ -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)
@@ -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)
+    }
+
 }