[MOSIP-31314] otp validation security fix (#1181)

* WIP added OTP freezing logic Signed-off-by: Loganathan Sekar <[email protected]> * Added test cases Signed-off-by: Loganathan Sekar <[email protected]> * Added sendotp test cases Signed-off-by: Loganathan Sekar <[email protected]> * Updated db scripts to index refid but not otphash Signed-off-by: Loganathan Sekar <[email protected]> * Minor refectoring Signed-off-by: Loganathan Sekar <[email protected]> * Corrected otp freezing property names Signed-off-by: Loganathan Sekar <[email protected]> * Correction to the OTP frozen message Signed-off-by: Loganathan Sekar <[email protected]> * Correction to the OTP frozen message Signed-off-by: Loganathan Sekar <[email protected]> * Correction to the OTP frozen message Signed-off-by: Loganathan Sekar <[email protected]> * Added validation on frozen error message Signed-off-by: Loganathan Sekar <[email protected]> * Minor refactoring Signed-off-by: Loganathan Sekar <[email protected]> * Added javadocs Signed-off-by: Loganathan Sekar <[email protected]> * Fixed test failure Signed-off-by: Loganathan Sekar <[email protected]> * Avoided nested if elses for exceptions in else block Signed-off-by: Loganathan Sekar <[email protected]> * Fixed otp error message Signed-off-by: Loganathan Sekar <[email protected]> * Applied review comments Signed-off-by: Loganathan Sekar <[email protected]> * Minor jdoc update Signed-off-by: Loganathan Sekar <[email protected]> * Fix to saving entity Signed-off-by: Loganathan Sekar <[email protected]> --------- Signed-off-by: Loganathan Sekar <[email protected]>
mosip · Feb 9, 2024 · d8eaa5e · d8eaa5e
1 parent 2abec1a
commit d8eaa5e
Show file tree

Hide file tree

Showing 14 changed files with 805 additions and 65 deletions.
diff --git a/.../src/main/java/io/mosip/authentication/common/service/impl/match/OtpMatchingStrategy.java b/.../src/main/java/io/mosip/authentication/common/service/impl/match/OtpMatchingStrategy.java
@@ -21,11 +21,12 @@
 public enum OtpMatchingStrategy implements TextMatchingStrategy {
 
 	EXACT(MatchingStrategyType.EXACT, (Object reqInfo, Object entityInfo, Map<String, Object> props) -> {
-		if (reqInfo instanceof String && entityInfo instanceof String) {
+		Object idvidObj = props.get(IdAuthCommonConstants.IDVID);
+		if (reqInfo instanceof String && entityInfo instanceof String && idvidObj instanceof String) {
 			Object object = props.get(ValidateOtpFunction.class.getSimpleName());
 			if (object instanceof ValidateOtpFunction) {
 				ValidateOtpFunction func = (ValidateOtpFunction) object;
-				boolean otpValid = func.validateOtp((String) reqInfo, (String) entityInfo);
+				boolean otpValid = func.validateOtp((String) reqInfo, (String) entityInfo, (String) idvidObj);
 				if (!otpValid) {
 					return 0;
 				} else {

diff --git a/...n-common/src/main/java/io/mosip/authentication/common/service/impl/match/PinAuthType.java b/...n-common/src/main/java/io/mosip/authentication/common/service/impl/match/PinAuthType.java
@@ -5,6 +5,7 @@
 import java.util.Set;
 
 import io.mosip.authentication.common.service.impl.AuthTypeImpl;
+import io.mosip.authentication.core.constant.IdAuthCommonConstants;
 import io.mosip.authentication.core.indauth.dto.AuthRequestDTO;
 import io.mosip.authentication.core.spi.indauth.match.AuthType;
 import io.mosip.authentication.core.spi.indauth.match.IdInfoFetcher;
@@ -29,6 +30,7 @@ public Map<String, Object> getMatchProperties(AuthRequestDTO authRequestDTO, IdI
 			if (isAuthTypeInfoAvailable(authRequestDTO)) {
 				ValidateOtpFunction func = idInfoFetcher.getValidateOTPFunction();
 				valueMap.put(ValidateOtpFunction.class.getSimpleName(), func);
+				valueMap.put(IdAuthCommonConstants.IDVID, authRequestDTO.getIndividualId());
 			}
 			return valueMap;
 		}

diff --git a/...n-common/src/main/java/io/mosip/authentication/common/service/integration/OTPManager.java b/...n-common/src/main/java/io/mosip/authentication/common/service/integration/OTPManager.java
@@ -1,10 +1,15 @@
 package io.mosip.authentication.common.service.integration;
 
 import java.time.LocalDateTime;
-import java.util.*;
+import java.time.temporal.ChronoUnit;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.UUID;
 import java.util.stream.Collectors;
 
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.http.MediaType;
 import org.springframework.stereotype.Component;
 
@@ -38,10 +43,14 @@
  * @author Rakesh Roshan
  * @author Dinesh Karuppiah.T
  * @author Manoj SP
+ * @author Loganathan S
  */
 @Component
 public class OTPManager {
 
+	/** The Constant QUERIED_STATUS_CODES. */
+	private static final List<String> QUERIED_STATUS_CODES = List.of(IdAuthCommonConstants.ACTIVE_STATUS, IdAuthCommonConstants.FROZEN);
+
 	/** The Constant OTP_EXPIRED. */
 	private static final String OTP_EXPIRED = "OTP_EXPIRED";
 
@@ -52,21 +61,29 @@ public class OTPManager {
 	@Autowired
 	private RestHelper restHelper;
 
-	@Autowired
-	private EnvUtil environment;
-
 	/** The rest request factory. */
 	@Autowired
 	private RestRequestFactory restRequestFactory;
 
+	/** The security manager. */
 	@Autowired
 	private IdAuthSecurityManager securityManager;
 
+	/** The otp transaction repo. */
 	@Autowired
 	private OtpTxnRepository otpRepo;
 
+	/** The notification service. */
 	@Autowired
 	private NotificationService notificationService;
+
+	/** The number of validation attempts allowed. */
+	@Value("${mosip.ida.otp.validation.attempt.count.threshold:5}")
+	private int numberOfValidationAttemptsAllowed;
+
+	/** The otp frozen time minutes. */
+	@Value("${mosip.ida.otp.frozen.duration.minutes:30}")
+	private int otpFrozenTimeMinutes;
 
 	/** The logger. */
 	private static Logger logger = IdaLogger.getLogger(OTPManager.class);
@@ -76,29 +93,38 @@ public class OTPManager {
 	 * time-out.
 	 *
 	 * @param otpRequestDTO the otp request DTO
-	 * @param uin           the uin
+	 * @param idvid the idvid
+	 * @param idvidType the idvid type
 	 * @param valueMap      the value map
+	 * @param templateLanguages the template languages
 	 * @return String(otp)
 	 * @throws IdAuthenticationBusinessException the id authentication business
 	 *                                           exception
 	 */
 	public boolean sendOtp(OtpRequestDTO otpRequestDTO, String idvid, String idvidType, Map<String, String> valueMap, List<String> templateLanguages)
 			throws IdAuthenticationBusinessException {
+
+		String refIdHash = securityManager.hash(idvid);
+		Optional<OtpTransaction> otpEntityOpt = otpRepo.findFirstByRefIdInStatusCodesOrderByGeneratedDtimesDesc(refIdHash, QUERIED_STATUS_CODES);
 
+		if(otpEntityOpt.isPresent()) {
+			OtpTransaction otpEntity = otpEntityOpt.get();
+			requireOtpNotFrozen(otpEntity, false);
+		}
+
 		String otp = generateOTP(otpRequestDTO.getIndividualId());
 		LocalDateTime otpGenerationTime = DateUtils.getUTCCurrentDateTime();
 		String otpHash = IdAuthSecurityManager.digestAsPlainText((otpRequestDTO.getIndividualId()
 				+ EnvUtil.getKeySplitter() + otpRequestDTO.getTransactionID()
 				+ EnvUtil.getKeySplitter() + otp).getBytes());
-
-		Optional<OtpTransaction> otpTxnOpt = otpRepo.findByOtpHashAndStatusCode(otpHash, IdAuthCommonConstants.ACTIVE_STATUS);
-		if (otpTxnOpt.isPresent()) {
-			OtpTransaction otpTxn = otpTxnOpt.get();
+		
+		OtpTransaction otpTxn;
+		if (otpEntityOpt.isPresent()
+				&& (otpTxn = otpEntityOpt.get()).getStatusCode().equals(IdAuthCommonConstants.ACTIVE_STATUS)) {
 			otpTxn.setOtpHash(otpHash);
 			otpTxn.setUpdBy(securityManager.getUser());
 			otpTxn.setUpdDTimes(otpGenerationTime);
 			otpTxn.setExpiryDtimes(otpGenerationTime.plusSeconds(EnvUtil.getOtpExpiryTime()));
-			otpTxn.setStatusCode(IdAuthCommonConstants.ACTIVE_STATUS);
 			otpRepo.save(otpTxn);
 		} else {
 			OtpTransaction txn = new OtpTransaction();
@@ -123,6 +149,24 @@ public boolean sendOtp(OtpRequestDTO otpRequestDTO, String idvid, String idvidTy
 		return true;
 	}
 
+	/**
+	 * Creates the OTP frozen exception.
+	 *
+	 * @return the id authentication business exception
+	 */
+	private IdAuthenticationBusinessException createOTPFrozenException() {
+		return new IdAuthenticationBusinessException(IdAuthenticationErrorConstants.OTP_FROZEN.getErrorCode(),
+				String.format(IdAuthenticationErrorConstants.OTP_FROZEN.getErrorMessage(),
+						otpFrozenTimeMinutes + " seconds", numberOfValidationAttemptsAllowed));
+	}
+
+	/**
+	 * Generate OTP.
+	 *
+	 * @param uin the uin
+	 * @return the string
+	 * @throws IdAuthUncheckedException the id auth unchecked exception
+	 */
 	private String generateOTP(String uin) throws IdAuthUncheckedException {
 		try {
 			OtpGenerateRequestDto otpGenerateRequestDto = new OtpGenerateRequestDto(uin);
@@ -137,11 +181,10 @@ private String generateOTP(String uin) throws IdAuthUncheckedException {
 						IdAuthenticationErrorConstants.BLOCKED_OTP_VALIDATE.getErrorCode(), USER_BLOCKED);
 				throw new IdAuthUncheckedException(IdAuthenticationErrorConstants.BLOCKED_OTP_VALIDATE);
 			}
-			if(response !=null && response.getResponse()!=null){
-				return response.getResponse().get("otp");
-			}else{
+			if(response == null || response.getResponse() == null) {
 				throw new IdAuthUncheckedException(IdAuthenticationErrorConstants.OTP_GENERATION_FAILED);
 			}
+			return response.getResponse().get("otp");
 
 		} catch (IDDataValidationException e) {
 			logger.error(IdAuthCommonConstants.SESSION_ID, this.getClass().getSimpleName(), "generateOTP",
@@ -160,29 +203,92 @@ private String generateOTP(String uin) throws IdAuthUncheckedException {
 	 *
 	 * @param pinValue the pin value
 	 * @param otpKey   the otp key
+	 * @param individualId the individual id
 	 * @return true, if successful
 	 * @throws IdAuthenticationBusinessException the id authentication business
 	 *                                           exception
 	 */
-	public boolean validateOtp(String pinValue, String otpKey) throws IdAuthenticationBusinessException {
-		String otpHash;
-		otpHash = IdAuthSecurityManager.digestAsPlainText(
-				(otpKey + EnvUtil.getKeySplitter() + pinValue).getBytes());
-		Optional<OtpTransaction> otpTxnOpt = otpRepo.findByOtpHashAndStatusCode(otpHash, IdAuthCommonConstants.ACTIVE_STATUS);
-		if (otpTxnOpt.isPresent()) {
-			OtpTransaction otpTxn = otpTxnOpt.get();
-			//OtpTransaction otpTxn = otpRepo.findByOtpHashAndStatusCode(otpHash, IdAuthCommonConstants.ACTIVE_STATUS);
-			otpTxn.setStatusCode(IdAuthCommonConstants.USED_STATUS);
-			otpRepo.save(otpTxn);
-			if (otpTxn.getExpiryDtimes().isAfter(DateUtils.getUTCCurrentDateTime())) {
-				return true;
-			} else {
+	public boolean validateOtp(String pinValue, String otpKey, String individualId) throws IdAuthenticationBusinessException {
+		String refIdHash = securityManager.hash(individualId);
+		Optional<OtpTransaction> otpEntityOpt = otpRepo.findFirstByRefIdInStatusCodesOrderByGeneratedDtimesDesc(refIdHash, QUERIED_STATUS_CODES);
+
+		if (otpEntityOpt.isEmpty()) {
+			throw new IdAuthenticationBusinessException(IdAuthenticationErrorConstants.OTP_REQUEST_REQUIRED);
+		}
+
+		OtpTransaction otpEntity = otpEntityOpt.get();
+		requireOtpNotFrozen(otpEntity, true);
+
+		if(otpEntity.getStatusCode().equals(IdAuthCommonConstants.UNFROZEN)) {
+			throw new IdAuthenticationBusinessException(IdAuthenticationErrorConstants.OTP_REQUEST_REQUIRED);
+		}
+
+		// At this point it should be active status alone.
+		// Increment the validation attempt count.
+		int attemptCount = otpEntity.getValidationRetryCount() == null ? 1 : otpEntity.getValidationRetryCount() + 1;
+
+		String otpHash = getOtpHash(pinValue, otpKey);
+		if (otpEntity.getOtpHash().equals(otpHash)) {
+			otpEntity.setUpdDTimes(DateUtils.getUTCCurrentDateTime());
+			otpEntity.setStatusCode(IdAuthCommonConstants.USED_STATUS);
+			otpRepo.save(otpEntity);
+			if (!otpEntity.getExpiryDtimes().isAfter(DateUtils.getUTCCurrentDateTime())) {
 				logger.error(IdAuthCommonConstants.SESSION_ID, this.getClass().getSimpleName(),
 						IdAuthenticationErrorConstants.EXPIRED_OTP.getErrorCode(), OTP_EXPIRED);
 				throw new IdAuthenticationBusinessException(IdAuthenticationErrorConstants.EXPIRED_OTP);
 			}
+			return true;
 		} else {
+			//Set the incremented validation attempt count
+			otpEntity.setValidationRetryCount(attemptCount);
+			if (attemptCount >= numberOfValidationAttemptsAllowed) {
+				otpEntity.setStatusCode(IdAuthCommonConstants.FROZEN);
+			}
+			otpEntity.setUpdDTimes(DateUtils.getUTCCurrentDateTime());
+			otpRepo.save(otpEntity);
 			return false;
 		}
 	}
+
+	/**
+	 * Require otp not frozen.
+	 *
+	 * @param otpEntity the otp entity
+	 * @throws IdAuthenticationBusinessException the id authentication business exception
+	 */
+	private void requireOtpNotFrozen(OtpTransaction otpEntity, boolean saveEntity) throws IdAuthenticationBusinessException {
+		if(otpEntity.getStatusCode().equals(IdAuthCommonConstants.FROZEN)) {
+			if(!isAfterFrozenDuration(otpEntity)) {
+				throw createOTPFrozenException();
+			}
+			logger.info("OTP Frozen wait time is over. Allowing further.");
+			otpEntity.setStatusCode(IdAuthCommonConstants.UNFROZEN);
+			if(saveEntity) {
+				otpRepo.save(otpEntity);
+			}
+		}
+	}
+
+	/**
+	 * Checks if the entity is after frozen duration.
+	 *
+	 * @param otpEntity the otp entity
+	 * @return true, if is after frozen duration
+	 */
+	private boolean isAfterFrozenDuration(OtpTransaction otpEntity) {
+		return DateUtils.getUTCCurrentDateTime().isAfter(otpEntity.getUpdDTimes().plus(otpFrozenTimeMinutes, ChronoUnit.MINUTES));
+	}
+
+	/**
+	 * Gets the otp hash.
+	 *
+	 * @param pinValue the pin value
+	 * @param otpKey the otp key
+	 * @return the otp hash
+	 */
+	private String getOtpHash(String pinValue, String otpKey) {
+		return IdAuthSecurityManager.digestAsPlainText(
+				(otpKey + EnvUtil.getKeySplitter() + pinValue).getBytes());
+	}
+
 }
diff --git a/...mon/src/main/java/io/mosip/authentication/common/service/repository/OtpTxnRepository.java b/...mon/src/main/java/io/mosip/authentication/common/service/repository/OtpTxnRepository.java
@@ -1,5 +1,6 @@
 package io.mosip.authentication.common.service.repository;
 
+import java.util.List;
 import java.util.Optional;
 
 import io.mosip.authentication.common.service.entity.OtpTransaction;
@@ -13,12 +14,11 @@
 public interface OtpTxnRepository extends BaseRepository<OtpTransaction, String> {
 
 	/**
-	 * Find by otp hash and status code.
+	 * Find first element by ref_id ordered by generated_dtimes in descending order.
 	 *
-	 * @param otpHash the otp hash
-	 * @param statusCode the status code
+	 * @param refIdHash the ref id hash
 	 * @return the optional
 	 */
-	Optional<OtpTransaction> findByOtpHashAndStatusCode(String otpHash, String statusCode);
+	Optional<OtpTransaction> findFirstByRefIdInStatusCodesOrderByGeneratedDtimesDesc(String refIdHash, List<String> statusCodes);
 
 }
diff --git a/...-common/src/test/java/io/mosip/authentication/common/service/impl/OTPAuthServiceTest.java b/...-common/src/test/java/io/mosip/authentication/common/service/impl/OTPAuthServiceTest.java
@@ -139,7 +139,7 @@ public void TestValidValidateOtp() throws IdAuthenticationBusinessException {
 		List<String> valueList = new ArrayList<>();
 		valueList.add("1234567890");
 		Mockito.when(repository.findByTxnId(Mockito.anyString(), Mockito.any(), Mockito.any())).thenReturn(autntxnList);
-		Mockito.when(otpmanager.validateOtp(Mockito.anyString(), Mockito.anyString())).thenReturn(true);
+		Mockito.when(otpmanager.validateOtp(Mockito.anyString(), Mockito.anyString(), Mockito.anyString())).thenReturn(true);
 		AuthStatusInfo authStatusInfo = otpauthserviceimpl.authenticate(authreqdto, "123456", Collections.emptyMap(),
 				"PARTNER1");
 		assertNotNull(authStatusInfo);

diff --git a/.../test/java/io/mosip/authentication/common/service/impl/match/OtpMatchingStrategyTest.java b/.../test/java/io/mosip/authentication/common/service/impl/match/OtpMatchingStrategyTest.java
@@ -25,6 +25,7 @@
 import io.mosip.authentication.common.service.integration.OTPManager;
 import io.mosip.authentication.common.service.repository.OtpTxnRepository;
 import io.mosip.authentication.common.service.util.EnvUtil;
+import io.mosip.authentication.core.constant.IdAuthCommonConstants;
 import io.mosip.authentication.core.exception.IdAuthenticationBusinessException;
 import io.mosip.authentication.core.spi.indauth.match.MatchFunction;
 import io.mosip.authentication.core.spi.indauth.match.ValidateOtpFunction;
@@ -98,6 +99,7 @@ public void TestInValidOtpMatchingStrategy() throws IdAuthenticationBusinessExce
 		MatchFunction matchFunction = OtpMatchingStrategy.EXACT.getMatchFunction();
 		Map<String, Object> matchProperties = new HashMap<>();
 		matchProperties.put(ValidateOtpFunction.class.getSimpleName(), "");
+		matchProperties.put(IdAuthCommonConstants.IDVID, "");
 		int value = matchFunction.match("123456", "IDA_asdEEFAER", matchProperties);
 		assertEquals(0, value);
 	}