Update tests to address totpMfa util change

indigo-iam · Dec 4, 2023 · df1fe06 · df1fe06
1 parent 67ebfab
commit df1fe06
Show file tree

Hide file tree

Showing 9 changed files with 120 additions and 27 deletions.
diff --git a/...java/it/infn/mw/iam/api/account/multi_factor_authentication/DefaultIamTotpMfaService.java b/...java/it/infn/mw/iam/api/account/multi_factor_authentication/DefaultIamTotpMfaService.java
@@ -43,7 +43,6 @@
 import it.infn.mw.iam.persistence.model.IamTotpRecoveryCode;
 import it.infn.mw.iam.persistence.repository.IamTotpMfaRepository;
 import it.infn.mw.iam.util.mfa.IamTotpMfaEncryptionAndDecryptionUtil;
-import it.infn.mw.iam.util.mfa.IamTotpMfaInvalidArgumentError;
 
 @Service
 public class DefaultIamTotpMfaService implements IamTotpMfaService, ApplicationEventPublisherAware {
@@ -131,9 +130,9 @@ public IamTotpMfa addTotpMfaSecret(IamAccount account) {
       totpMfaRepository.save(totpMfa);
 
       return totpMfa;
-    } catch (Exception exp) {
-      throw new IamTotpMfaInvalidArgumentError(
-          "Please ensure that you either use the same password or set the password", exp);
+    } catch (Exception iamTotpMfaInvalidArgumentErrorMsg) {
+      LOG.error(iamTotpMfaInvalidArgumentErrorMsg.getMessage());
+      throw iamTotpMfaInvalidArgumentErrorMsg;
     }
   }
 
@@ -235,8 +234,9 @@ public boolean verifyTotp(IamAccount account, String totp) {
         totpVerifiedEvent(account, totpMfa);
         return true;
       }
-    } catch (Exception exp) {
-      LOG.error("Please ensure that you either use the same password or set the password", exp);
+    } catch (Exception iamTotpMfaInvalidArgumentErrorMsg) {
+      LOG.error(iamTotpMfaInvalidArgumentErrorMsg.getMessage());
+      throw iamTotpMfaInvalidArgumentErrorMsg;
     }
 
     return false;
@@ -275,8 +275,8 @@ public boolean verifyRecoveryCode(IamAccount account, String recoveryCode) {
           return true;
         }
       }
-    } catch (Exception exp) {
-      LOG.error("Please ensure that you either use the same password or set the password", exp);
+    } catch (Exception iamTotpMfaInvalidArgumentErrorMsg) {
+      throw iamTotpMfaInvalidArgumentErrorMsg;
     }
 
     return false;
@@ -295,8 +295,9 @@ private Set<IamTotpRecoveryCode> generateRecoveryCodes(IamTotpMfa totpMfa) {
             code, iamTotpMfaProperties.getPasswordToEncryptOrDecrypt()));
         recoveryCodes.add(recoveryCode);
       }
-    } catch (Exception exp) {
-      LOG.error("Please ensure that you either use the same password or set the password", exp);
+    } catch (Exception iamTotpMfaInvalidArgumentErrorMsg) {
+      LOG.error(iamTotpMfaInvalidArgumentErrorMsg.getMessage());
+      throw iamTotpMfaInvalidArgumentErrorMsg;
     }
 
     return recoveryCodes;

diff --git a/...w/iam/api/account/multi_factor_authentication/DefaultIamTotpRecoveryCodeResetService.java b/...w/iam/api/account/multi_factor_authentication/DefaultIamTotpRecoveryCodeResetService.java
@@ -36,7 +36,6 @@
 import it.infn.mw.iam.persistence.repository.IamAccountRepository;
 import it.infn.mw.iam.persistence.repository.IamTotpMfaRepository;
 import it.infn.mw.iam.util.mfa.IamTotpMfaEncryptionAndDecryptionUtil;
-import it.infn.mw.iam.util.mfa.IamTotpMfaInvalidArgumentError;
 
 @Service
 public class DefaultIamTotpRecoveryCodeResetService
@@ -101,9 +100,8 @@ public IamAccount resetRecoveryCodes(IamAccount account) {
       recoveryCodesResetEvent(account, totpMfa);
 
       return account;
-    } catch (Exception exp) {
-      throw new IamTotpMfaInvalidArgumentError(
-          "Please ensure that you either use the same password or set the password");
+    } catch (Exception iamTotpMfaInvalidArgumentErrorMsg) {
+      throw iamTotpMfaInvalidArgumentErrorMsg;
     }
   }
 }
diff --git a/...unt/multi_factor_authentication/authenticator_app/AuthenticatorAppSettingsController.java b/...unt/multi_factor_authentication/authenticator_app/AuthenticatorAppSettingsController.java
@@ -51,7 +51,6 @@
 import it.infn.mw.iam.persistence.model.IamTotpMfa;
 import it.infn.mw.iam.persistence.repository.IamAccountRepository;
 import it.infn.mw.iam.util.mfa.IamTotpMfaEncryptionAndDecryptionUtil;
-import it.infn.mw.iam.util.mfa.IamTotpMfaInvalidArgumentError;
 
 /**
  * Controller for customising user's authenticator app MFA settings Can enable or disable the
@@ -102,9 +101,8 @@ public SecretAndDataUriDTO addSecret() {
     try {
       mfaSecret = IamTotpMfaEncryptionAndDecryptionUtil.decryptSecretOrRecoveryCode(
           totpMfa.getSecret(), iamTotpMfaProperties.getPasswordToEncryptOrDecrypt());
-    } catch (Exception exp) {
-      throw new IamTotpMfaInvalidArgumentError(
-          "Please use the same password which is used for encryption");
+    } catch (Exception iamTotpMfaInvalidArgumentErrorMsg) {
+      throw iamTotpMfaInvalidArgumentErrorMsg;
     }
 
     try {

diff --git a/...authn/multi_factor_authentication/authenticator_app/RecoveryCodeManagementController.java b/...authn/multi_factor_authentication/authenticator_app/RecoveryCodeManagementController.java
@@ -43,7 +43,6 @@
 import it.infn.mw.iam.persistence.model.IamTotpRecoveryCode;
 import it.infn.mw.iam.persistence.repository.IamTotpMfaRepository;
 import it.infn.mw.iam.util.mfa.IamTotpMfaEncryptionAndDecryptionUtil;
-import it.infn.mw.iam.util.mfa.IamTotpMfaInvalidArgumentError;
 
 /**
  * Provides webpages related to recovery codes. Most of this appears if the user chooses to use a
@@ -130,8 +129,8 @@ public String viewRecoveryCodes(ModelMap model) {
       }
 
       return codes;
-    } catch (Exception exp) {
-      throw new IamTotpMfaInvalidArgumentError("Please use the same password which is used for encryption");
+    } catch (Exception iamTotpMfaInvalidArgumentErrorMsg) {
+      throw iamTotpMfaInvalidArgumentErrorMsg;
     }
   }
 

diff --git a/...-service/src/main/java/it/infn/mw/iam/util/mfa/IamTotpMfaEncryptionAndDecryptionUtil.java b/...-service/src/main/java/it/infn/mw/iam/util/mfa/IamTotpMfaEncryptionAndDecryptionUtil.java
@@ -43,19 +43,24 @@ private IamTotpMfaEncryptionAndDecryptionUtil() {
    * This process requires a password for encrypting the plaintext. Ensure to use
    * the same password for decryption as well.
    *
-   * @param input    plaintext to encrypt.
-   * @param password Provided by the admin through the environment
-   *                 variable.
+   * @param plaintext plaintext to encrypt.
+   * @param password  Provided by the admin through the environment
+   *                  variable.
    *
    * @return String If encryption is successful, the cipherText would be returned.
    *
    * @throws IamTotpMfaInvalidArgumentError
    */
-  public static String encryptSecretOrRecoveryCode(String input, String password)
+  public static String encryptSecretOrRecoveryCode(String plaintext, String password)
       throws IamTotpMfaInvalidArgumentError {
     byte[] salt = generateSalt();
     String modeOfOperation = defaultModel.getModeOfOperation();
 
+    if (validatePlaintext(plaintext) || validatePlaintext(password)) {
+      throw new IamTotpMfaInvalidArgumentError(
+          "Please ensure that you provide plaintext and the password");
+    }
+
     try {
       Key key = getKeyFromPassword(password, salt, defaultModel.getEncryptionAlgorithm());
       IvParameterSpec iv = getIVSecureRandom(defaultModel.getModeOfOperation());
@@ -64,7 +69,7 @@ public static String encryptSecretOrRecoveryCode(String input, String password)
       cipher1.init(Cipher.ENCRYPT_MODE, key, iv);
 
       byte[] ivBytes = cipher1.getIV();
-      byte[] cipherText = cipher1.doFinal(input.getBytes());
+      byte[] cipherText = cipher1.doFinal(plaintext.getBytes());
       byte[] encryptedData = new byte[salt.length + ivBytes.length + cipherText.length];
 
       // Append salt, IV, and cipherText into `encryptedData`.
@@ -76,7 +81,7 @@ public static String encryptSecretOrRecoveryCode(String input, String password)
           .encodeToString(encryptedData);
     } catch (Exception exp) {
       throw new IamTotpMfaInvalidArgumentError(
-          "Please ensure that you either use the same password or set the password", exp);
+          "Please ensure that you provide plaintext and the password", exp);
     }
   }
 
@@ -96,6 +101,11 @@ public static String decryptSecretOrRecoveryCode(String cipherText, String passw
       throws IamTotpMfaInvalidArgumentError {
     String modeOfOperation = defaultModel.getModeOfOperation();
 
+    if (validatePlaintext(cipherText) || validatePlaintext(password)) {
+      throw new IamTotpMfaInvalidArgumentError(
+          "Please ensure that you provide cipherText and the password");
+    }
+
     try {
       byte[] encryptedData = Base64.getDecoder().decode(cipherText);
 
@@ -178,4 +188,8 @@ private static byte[] generateSalt() {
 
     return salt;
   }
+
+  private static boolean validatePlaintext(String text) {
+    return (text == null || text.isEmpty());
+  }
 }
diff --git a/...ulti_factor_authentication/authenticator_app/AuthenticatorAppSettingsControllerTests.java b/...ulti_factor_authentication/authenticator_app/AuthenticatorAppSettingsControllerTests.java
@@ -18,6 +18,8 @@
 import static it.infn.mw.iam.api.account.multi_factor_authentication.authenticator_app.AuthenticatorAppSettingsController.ADD_SECRET_URL;
 import static it.infn.mw.iam.api.account.multi_factor_authentication.authenticator_app.AuthenticatorAppSettingsController.ENABLE_URL;
 import static it.infn.mw.iam.api.account.multi_factor_authentication.authenticator_app.AuthenticatorAppSettingsController.DISABLE_URL;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -41,6 +43,7 @@
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.setup.MockMvcBuilders;
 import org.springframework.web.context.WebApplicationContext;
+import org.springframework.web.util.NestedServletException;
 
 import it.infn.mw.iam.api.account.multi_factor_authentication.IamTotpMfaService;
 import it.infn.mw.iam.config.mfa.IamTotpMfaProperties;
@@ -107,6 +110,26 @@ public void testAddSecret() throws Exception {
     verify(totpMfaService, times(1)).addTotpMfaSecret(account);
   }
 
+  @Test
+  @WithMockUser(username = TEST_USERNAME)
+  public void testAddSecret_withDifferentPassword() throws Exception {
+    IamAccount account = cloneAccount(TEST_ACCOUNT);
+    IamTotpMfa totpMfa = cloneTotpMfa(TOTP_MFA);
+    totpMfa.setActive(false);
+    totpMfa.setAccount(null);
+    totpMfa.setSecret(
+        IamTotpMfaEncryptionAndDecryptionUtil.encryptSecretOrRecoveryCode("secret",
+            iamTotpMfaProperties.getPasswordToEncryptOrDecrypt()));
+    when(totpMfaService.addTotpMfaSecret(account)).thenReturn(totpMfa);
+    when(iamTotpMfaProperties.getPasswordToEncryptOrDecrypt()).thenReturn("");
+
+    NestedServletException thrownException = assertThrows(NestedServletException.class, () -> {
+      mvc.perform(put(ADD_SECRET_URL));
+    });
+
+    assertTrue(thrownException.getCause().getMessage().startsWith("Please ensure that you provide"));
+  }
+
   @Test
   @WithAnonymousUser
   public void testAddSecretNoAuthenticationIsUnauthorized() throws Exception {

diff --git a/...n/mw/iam/test/multi_factor_authentication/IamTotpMfaEncryptionAndDecryptionUtilTests.java b/...n/mw/iam/test/multi_factor_authentication/IamTotpMfaEncryptionAndDecryptionUtilTests.java
@@ -68,4 +68,17 @@ public void testEncryptSecretOrRecoveryCodeWithTamperedCipher() throws IamTotpMf
       assertEquals(TOTP_MFA_SECRET, plainText);
     }
   }
+
+  @Test
+  public void testEncryptSecretOrRecoveryCodeWithEmptyPlainText() throws IamTotpMfaInvalidArgumentError {
+
+    IamTotpMfaInvalidArgumentError thrownException = assertThrows(IamTotpMfaInvalidArgumentError.class, () -> {
+      // Try to encrypt the empty plainText
+      IamTotpMfaEncryptionAndDecryptionUtil.encryptSecretOrRecoveryCode(null, DEFAULT_KEY);
+    });
+
+    // Always throws an error because we have passed empty plaintext.
+    assertTrue(thrownException.getMessage().startsWith("Please ensure that you provide"));
+
+  }
 }
diff --git a/...src/test/java/it/infn/mw/iam/test/multi_factor_authentication/IamTotpMfaServiceTests.java b/...src/test/java/it/infn/mw/iam/test/multi_factor_authentication/IamTotpMfaServiceTests.java
@@ -22,6 +22,7 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.reset;
@@ -60,6 +61,7 @@
 import it.infn.mw.iam.persistence.model.IamTotpRecoveryCode;
 import it.infn.mw.iam.persistence.repository.IamTotpMfaRepository;
 import it.infn.mw.iam.util.mfa.IamTotpMfaEncryptionAndDecryptionUtil;
+import it.infn.mw.iam.util.mfa.IamTotpMfaInvalidArgumentError;
 
 @RunWith(MockitoJUnitRunner.class)
 public class IamTotpMfaServiceTests extends IamTotpMfaServiceTestSupport {
@@ -172,6 +174,35 @@ public void testAddsMfaRecoveryCode_whenNoMfaSecretAssignedFails() {
     }
   }
 
+  @Test
+  public void testAddsMfaRecoveryCode_whenPasswordIsEmpty() {
+    when(repository.findByAccount(TOTP_MFA_ACCOUNT)).thenReturn(Optional.empty());
+    when(iamTotpMfaProperties.getPasswordToEncryptOrDecrypt()).thenReturn("");
+
+    IamAccount account = cloneAccount(TOTP_MFA_ACCOUNT);
+
+    IamTotpMfaInvalidArgumentError thrownException = assertThrows(IamTotpMfaInvalidArgumentError.class, () -> {
+      // Decrypt the cipherText with a different key
+      service.addTotpMfaSecret(account);
+    });
+
+    assertTrue(thrownException.getMessage().startsWith("Please ensure that you provide"));
+  }
+
+  @Test
+  public void testAddsMfaRecoveryCodes_whenPasswordIsEmpty() {
+    when(iamTotpMfaProperties.getPasswordToEncryptOrDecrypt()).thenReturn("");
+
+    IamAccount account = cloneAccount(TOTP_MFA_ACCOUNT);
+
+    IamTotpMfaInvalidArgumentError thrownException = assertThrows(IamTotpMfaInvalidArgumentError.class, () -> {
+      // Decrypt the cipherText with a different key
+      service.addTotpMfaRecoveryCodes(account);
+    });
+
+    assertTrue(thrownException.getMessage().startsWith("Please ensure that you provide"));
+  }
+
   @Test
   public void testEnablesTotpMfa() throws Exception {
     IamAccount account = cloneAccount(TOTP_MFA_ACCOUNT);

diff --git a/.../multi_factor_authentication/authenticator_app/RecoveryCodeManagementControllerTests.java b/.../multi_factor_authentication/authenticator_app/RecoveryCodeManagementControllerTests.java
@@ -20,6 +20,8 @@
 import static it.infn.mw.iam.authn.multi_factor_authentication.authenticator_app.RecoveryCodeManagementController.RECOVERY_CODE_VIEW_URL;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -51,6 +53,7 @@
 import org.springframework.test.web.servlet.MvcResult;
 import org.springframework.test.web.servlet.setup.MockMvcBuilders;
 import org.springframework.web.context.WebApplicationContext;
+import org.springframework.web.util.NestedServletException;
 
 import dev.samstevens.totp.recovery.RecoveryCodeGenerator;
 import it.infn.mw.iam.api.account.AccountUtils;
@@ -212,4 +215,17 @@ public void testGetRecoveryCodesNoAuthenticationFails() throws Exception {
   public void testGetRecoveryCodesWithPreAuthenticationFails() throws Exception {
     mvc.perform(get(RECOVERY_CODE_GET_URL)).andExpect(status().isUnauthorized());
   }
+
+  @Test
+  @WithMockMfaUser
+  public void testGetRecoveryCodes_WithDifferentPassword() throws Exception {
+    when(iamTotpMfaProperties.getPasswordToEncryptOrDecrypt()).thenReturn("Different_Password");
+
+    // Test with different password for decryption to get the plaintext.
+    NestedServletException thrownException = assertThrows(NestedServletException.class, () -> {
+      mvc.perform(get(RECOVERY_CODE_GET_URL));
+    });
+
+    assertTrue(thrownException.getCause().getMessage().startsWith("Please use the same password which"));
+  }
 }