From df1fe069fc06d0ebedd3ac73199be39118f466fd Mon Sep 17 00:00:00 2001 From: Sae126V Date: Sun, 3 Dec 2023 23:13:44 +0000 Subject: [PATCH] Update tests to address totpMfa util change --- .../DefaultIamTotpMfaService.java | 21 +++++++------ ...efaultIamTotpRecoveryCodeResetService.java | 6 ++-- .../AuthenticatorAppSettingsController.java | 6 ++-- .../RecoveryCodeManagementController.java | 5 ++- ...IamTotpMfaEncryptionAndDecryptionUtil.java | 26 ++++++++++++---- ...thenticatorAppSettingsControllerTests.java | 23 ++++++++++++++ ...tpMfaEncryptionAndDecryptionUtilTests.java | 13 ++++++++ .../IamTotpMfaServiceTests.java | 31 +++++++++++++++++++ ...RecoveryCodeManagementControllerTests.java | 16 ++++++++++ 9 files changed, 120 insertions(+), 27 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/account/multi_factor_authentication/DefaultIamTotpMfaService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/multi_factor_authentication/DefaultIamTotpMfaService.java index d58674e8f..b8e9df101 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/account/multi_factor_authentication/DefaultIamTotpMfaService.java +++ b/iam-login-service/src/main/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 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/iam-login-service/src/main/java/it/infn/mw/iam/api/account/multi_factor_authentication/DefaultIamTotpRecoveryCodeResetService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/multi_factor_authentication/DefaultIamTotpRecoveryCodeResetService.java index 241ac2ca2..22d2558ac 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/account/multi_factor_authentication/DefaultIamTotpRecoveryCodeResetService.java +++ b/iam-login-service/src/main/java/it/infn/mw/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/iam-login-service/src/main/java/it/infn/mw/iam/api/account/multi_factor_authentication/authenticator_app/AuthenticatorAppSettingsController.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/multi_factor_authentication/authenticator_app/AuthenticatorAppSettingsController.java index 013970ab5..712010e33 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/account/multi_factor_authentication/authenticator_app/AuthenticatorAppSettingsController.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/account/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/iam-login-service/src/main/java/it/infn/mw/iam/authn/multi_factor_authentication/authenticator_app/RecoveryCodeManagementController.java b/iam-login-service/src/main/java/it/infn/mw/iam/authn/multi_factor_authentication/authenticator_app/RecoveryCodeManagementController.java index 72ea2b7de..f18b9c43d 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/authn/multi_factor_authentication/authenticator_app/RecoveryCodeManagementController.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/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/iam-login-service/src/main/java/it/infn/mw/iam/util/mfa/IamTotpMfaEncryptionAndDecryptionUtil.java b/iam-login-service/src/main/java/it/infn/mw/iam/util/mfa/IamTotpMfaEncryptionAndDecryptionUtil.java index c6b1fb127..6e112e215 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/util/mfa/IamTotpMfaEncryptionAndDecryptionUtil.java +++ b/iam-login-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/iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/multi_factor_authentication/authenticator_app/AuthenticatorAppSettingsControllerTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/multi_factor_authentication/authenticator_app/AuthenticatorAppSettingsControllerTests.java index df483aa18..459e7b040 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/multi_factor_authentication/authenticator_app/AuthenticatorAppSettingsControllerTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/account/multi_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/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/IamTotpMfaEncryptionAndDecryptionUtilTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/IamTotpMfaEncryptionAndDecryptionUtilTests.java index d32f6f10c..2ff1313ed 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/IamTotpMfaEncryptionAndDecryptionUtilTests.java +++ b/iam-login-service/src/test/java/it/infn/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/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/IamTotpMfaServiceTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/IamTotpMfaServiceTests.java index 3f473621a..2f3daa5ed 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/IamTotpMfaServiceTests.java +++ b/iam-login-service/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/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/authenticator_app/RecoveryCodeManagementControllerTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/authenticator_app/RecoveryCodeManagementControllerTests.java index 4b8601b2d..1387ee951 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/multi_factor_authentication/authenticator_app/RecoveryCodeManagementControllerTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/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")); + } }