From 23bd9206a16c9d3c0221a1a15b578a5abf703f66 Mon Sep 17 00:00:00 2001 From: mengleang-0090 <148198934+mengleang-0090@users.noreply.github.com> Date: Fri, 16 Feb 2024 19:58:16 +0700 Subject: [PATCH] DSD-4457 (#72) * fix cookie without secure code Signed-off-by: mengleang-ngoun * fix code bug Signed-off-by: mengleang-ngoun * update sonar ignore on getChache Signed-off-by: mengleang-ngoun * fix null on message Signed-off-by: mengleang-ngoun --------- Signed-off-by: mengleang-ngoun --- .../signup/helper/NotificationHelper.java | 2 +- .../signup/services/CacheUtilService.java | 17 ++++++----- .../services/ChallengeManagerService.java | 5 ++-- .../GoogleRecaptchaValidatorService.java | 1 - .../signup/services/RegistrationService.java | 30 +++++++++++-------- .../signup/validator/LanguageValidator.java | 2 +- 6 files changed, 31 insertions(+), 26 deletions(-) diff --git a/signup-service/src/main/java/io/mosip/signup/helper/NotificationHelper.java b/signup-service/src/main/java/io/mosip/signup/helper/NotificationHelper.java index 7733131a..74a3fde7 100644 --- a/signup-service/src/main/java/io/mosip/signup/helper/NotificationHelper.java +++ b/signup-service/src/main/java/io/mosip/signup/helper/NotificationHelper.java @@ -43,7 +43,7 @@ public class NotificationHelper { environment.getProperty(templateKey + "." + locale) : new String(Base64.getDecoder().decode(environment.getProperty(templateKey + "." + locale))); - if(params != null){ + if(params != null && message != null){ for (Map.Entry entry: params.entrySet()){ message = message.replace(entry.getKey(), entry.getValue()); } diff --git a/signup-service/src/main/java/io/mosip/signup/services/CacheUtilService.java b/signup-service/src/main/java/io/mosip/signup/services/CacheUtilService.java index 8cb2810e..aa673c2d 100644 --- a/signup-service/src/main/java/io/mosip/signup/services/CacheUtilService.java +++ b/signup-service/src/main/java/io/mosip/signup/services/CacheUtilService.java @@ -13,6 +13,7 @@ import org.springframework.stereotype.Service; import java.util.Locale; +import java.util.Objects; @Slf4j @Service @@ -59,39 +60,39 @@ public String setActiveKeyAlias(String key, String alias) { public RegistrationTransaction createUpdateChallengeGeneratedTransaction(String transactionId, RegistrationTransaction registrationTransaction) { - cacheManager.getCache(SignUpConstants.CHALLENGE_GENERATED).put(transactionId, registrationTransaction); + cacheManager.getCache(SignUpConstants.CHALLENGE_GENERATED).put(transactionId, registrationTransaction); //NOSONAR getCache() will not be returning null here. return registrationTransaction; } public void updateStatusCheckTransaction(String transactionId, RegistrationTransaction registrationTransaction) { - cacheManager.getCache(SignUpConstants.STATUS_CHECK).put(transactionId, registrationTransaction); + cacheManager.getCache(SignUpConstants.STATUS_CHECK).put(transactionId, registrationTransaction); //NOSONAR getCache() will not be returning null here. } //---Getter--- public RegistrationTransaction getChallengeGeneratedTransaction(String transactionId) { - return cacheManager.getCache(SignUpConstants.CHALLENGE_GENERATED).get(transactionId, RegistrationTransaction.class); +return cacheManager.getCache(SignUpConstants.CHALLENGE_GENERATED).get(transactionId, RegistrationTransaction.class); //NOSONAR getCache() will not be returning null here. } public RegistrationTransaction getChallengeVerifiedTransaction(String transactionId) { - return cacheManager.getCache(SignUpConstants.CHALLENGE_VERIFIED).get(transactionId, RegistrationTransaction.class); + return cacheManager.getCache(SignUpConstants.CHALLENGE_VERIFIED).get(transactionId, RegistrationTransaction.class); //NOSONAR getCache() will not be returning null here. } public RegistrationTransaction getStatusCheckTransaction(String transactionId) { - return cacheManager.getCache(SignUpConstants.STATUS_CHECK).get(transactionId, RegistrationTransaction.class); + return cacheManager.getCache(SignUpConstants.STATUS_CHECK).get(transactionId, RegistrationTransaction.class); //NOSONAR getCache() will not be returning null here. } public boolean isIdentifierBlocked(String identifier) { String identifierHash = IdentityProviderUtil.generateB64EncodedHash(IdentityProviderUtil.ALGO_SHA3_256, identifier.toLowerCase(Locale.ROOT)); - return cacheManager.getCache(SignUpConstants.BLOCKED_IDENTIFIER).get(identifierHash, String.class) != null; + return cacheManager.getCache(SignUpConstants.BLOCKED_IDENTIFIER).get(identifierHash, String.class) != null; //NOSONAR getCache() will not be returning null here. } public String getSecretKey(String keyAlias) { - return cacheManager.getCache(SignUpConstants.KEYSTORE).get(keyAlias, String.class); + return cacheManager.getCache(SignUpConstants.KEYSTORE).get(keyAlias, String.class); //NOSONAR getCache() will not be returning null here. } public String getActiveKeyAlias() { - return cacheManager.getCache(SignUpConstants.KEY_ALIAS).get(CryptoHelper.ALIAS_CACHE_KEY, String.class); + return cacheManager.getCache(SignUpConstants.KEY_ALIAS).get(CryptoHelper.ALIAS_CACHE_KEY, String.class); //NOSONAR getCache() will not be returning null here. } } diff --git a/signup-service/src/main/java/io/mosip/signup/services/ChallengeManagerService.java b/signup-service/src/main/java/io/mosip/signup/services/ChallengeManagerService.java index 0d98e698..94571183 100644 --- a/signup-service/src/main/java/io/mosip/signup/services/ChallengeManagerService.java +++ b/signup-service/src/main/java/io/mosip/signup/services/ChallengeManagerService.java @@ -33,9 +33,8 @@ public class ChallengeManagerService { public String generateChallenge(RegistrationTransaction transaction) throws SignUpException { - switch (supportedGenerateChallengeType) { - case "OTP" : - return generateOTPChallenge(transaction.getChallengeTransactionId()); + if (supportedGenerateChallengeType.equals("OTP")) { + return generateOTPChallenge(transaction.getChallengeTransactionId()); } throw new SignUpException(ErrorConstants.UNSUPPORTED_CHALLENGE_TYPE); } diff --git a/signup-service/src/main/java/io/mosip/signup/services/GoogleRecaptchaValidatorService.java b/signup-service/src/main/java/io/mosip/signup/services/GoogleRecaptchaValidatorService.java index fdbb9474..dbbf1f32 100644 --- a/signup-service/src/main/java/io/mosip/signup/services/GoogleRecaptchaValidatorService.java +++ b/signup-service/src/main/java/io/mosip/signup/services/GoogleRecaptchaValidatorService.java @@ -7,7 +7,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.context.annotation.Bean; import org.springframework.stereotype.Component; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; diff --git a/signup-service/src/main/java/io/mosip/signup/services/RegistrationService.java b/signup-service/src/main/java/io/mosip/signup/services/RegistrationService.java index c7979b66..a5687090 100644 --- a/signup-service/src/main/java/io/mosip/signup/services/RegistrationService.java +++ b/signup-service/src/main/java/io/mosip/signup/services/RegistrationService.java @@ -118,6 +118,8 @@ public class RegistrationService { @Value("${mosip.signup.get-registration-status.endpoint}") private String getRegistrationStatusEndpoint; + private final String notificationLogging = "Notification response -> {}"; + /** * Generate and regenerate challenge based on the "regenerate" flag in the request. * if regenerate is false - always creates a new transaction and set-cookie header is sent in the response. @@ -139,7 +141,7 @@ public GenerateChallengeResponse generateChallenge(GenerateChallengeRequest gene if(cacheUtilService.isIdentifierBlocked(identifier)) throw new SignUpException(ErrorConstants.IDENTIFIER_BLOCKED); - if(generateChallengeRequest.isRegenerate() == false) { + if(!generateChallengeRequest.isRegenerate()) { transactionId = IdentityProviderUtil.createTransactionId(null); transaction = new RegistrationTransaction(identifier, generateChallengeRequest.getPurpose()); //Need to set cookie only when regenerate is false. @@ -162,11 +164,13 @@ public GenerateChallengeResponse generateChallenge(GenerateChallengeRequest gene if(transaction.getChallengeRetryAttempts() > resendAttempts) cacheUtilService.blockIdentifier(transactionId, transaction.getIdentifier(), "blocked"); + HashMap hashMap = new LinkedHashMap<>(); + hashMap.put("{challenge}", challenge); notificationHelper.sendSMSNotificationAsync(generateChallengeRequest.getIdentifier(), transaction.getLocale(), - SEND_OTP_SMS_NOTIFICATION_TEMPLATE_KEY, new HashMap<>(){{put("{challenge}", challenge);}}) - .thenAccept(notificationResponseRestResponseWrapper -> { - log.debug("Notification response -> {}", notificationResponseRestResponseWrapper); - }); + SEND_OTP_SMS_NOTIFICATION_TEMPLATE_KEY, hashMap) + .thenAccept(notificationResponseRestResponseWrapper -> + log.debug(notificationLogging, notificationResponseRestResponseWrapper) + ); return new GenerateChallengeResponse(ActionStatus.SUCCESS); } @@ -245,9 +249,9 @@ public RegisterResponse register(RegisterRequest registerRequest, String transac notificationHelper.sendSMSNotificationAsync(registerRequest.getUserInfo().getPhone(), transaction.getLocale(), REGISTRATION_SMS_NOTIFICATION_TEMPLATE_KEY, null) - .thenAccept(notificationResponseRestResponseWrapper -> { - log.debug("Notification response -> {}", notificationResponseRestResponseWrapper); - }); + .thenAccept(notificationResponseRestResponseWrapper -> + log.debug(notificationLogging, notificationResponseRestResponseWrapper) + ); RegisterResponse registration = new RegisterResponse(); registration.setStatus(ActionStatus.PENDING); @@ -318,9 +322,9 @@ public RegistrationStatusResponse updatePassword(ResetPasswordRequest resetPassw notificationHelper.sendSMSNotificationAsync(resetPasswordRequest.getIdentifier(), transaction.getLocale(), FORGOT_PASSWORD_SMS_NOTIFICATION_TEMPLATE_KEY, null) - .thenAccept(notificationResponseRestResponseWrapper -> { - log.debug("Notification response -> {}", notificationResponseRestResponseWrapper); - }); + .thenAccept(notificationResponseRestResponseWrapper -> + log.debug(notificationLogging, notificationResponseRestResponseWrapper) + ); RegistrationStatusResponse resetPassword = new RegistrationStatusResponse(); resetPassword.setStatus(RegistrationStatus.PENDING); @@ -445,7 +449,7 @@ private void saveIdentityData(RegisterRequest registerRequest, String transactio identity.setPassword(password); //By default, phone is set as the selected handle. - identity.setSelectedHandles(Arrays.asList("phone")); + identity.setSelectedHandles(List.of("phone")); transaction.getHandlesStatus().put(getHandleRequestId(transaction.getApplicationId(), "phone", userInfoMap.getPhone()), RegistrationStatus.PENDING); @@ -587,6 +591,8 @@ private void addVerifiedCookieInResponse(String transactionId, int maxAge) { Cookie unsetCookie = new Cookie(SignUpConstants.TRANSACTION_ID, ""); unsetCookie.setMaxAge(0); + cookie.setHttpOnly(true); + cookie.setSecure(true); response.addCookie(unsetCookie); } diff --git a/signup-service/src/main/java/io/mosip/signup/validator/LanguageValidator.java b/signup-service/src/main/java/io/mosip/signup/validator/LanguageValidator.java index 721c0f24..ab27e5e5 100644 --- a/signup-service/src/main/java/io/mosip/signup/validator/LanguageValidator.java +++ b/signup-service/src/main/java/io/mosip/signup/validator/LanguageValidator.java @@ -22,7 +22,7 @@ public void initialize(Language constraintAnnotation) { @Override public boolean isValid(String value, ConstraintValidatorContext constraintValidatorContext) { if(value == null) - return this.required ? false : true; + return !this.required; return supportedLanguages.contains(value); }