Phone number change web service (#271)

* web service verify OTP impl * minor identity service changes --------- Co-authored-by: Roshan Piyush <piyush.roshan@gmail.com>
OWASP · Aug 29, 2024 · 7ceb7fa · 7ceb7fa
1 parent 9a061c4
commit 7ceb7fa
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 13 deletions.
diff --git a/services/identity/src/main/java/com/crapi/constant/UserMessage.java b/services/identity/src/main/java/com/crapi/constant/UserMessage.java
@@ -38,10 +38,10 @@ public class UserMessage {
   public static final String CHANGE_PHONE_MESSAGE =
       "The otp has been sent to your email. If you have used example.com email, check your email using the MailHog web portal.";
   public static final String NUMBER_CHANGE_SUCCESSFUL = "Phone number change is successful";
-  public static final String NEW_NUMBER_DOES_NOT_BELONG =
-      "Fail, new number parameter doesn’t match with OTP";
+  public static final String NEW_NUMBER_DOES_NOT_BELONG = "Fail, invalid new number";
   public static final String OLD_NUMBER_DOES_NOT_BELONG =
-      "Fail, number  parameter doesn’t belong to the user";
+      "Fail, number parameter doesn’t belong to the user";
+  public static final String INVALID_CHANGE_REQUEST = "Fail, invalid change request";
   public static final String EMAIL_ALREADY_REGISTERED = "Email already registered! Email: ";
   public static final String GIVEN_URL_ALREADY_USED =
       "Given URL is already used! Please try to login..";

diff --git a/services/identity/src/main/java/com/crapi/service/Impl/UserServiceImpl.java b/services/identity/src/main/java/com/crapi/service/Impl/UserServiceImpl.java
@@ -546,13 +546,13 @@ public CRAPIResponse verifyPhoneOTP(HttpServletRequest request, ChangePhoneForm
             userRepository.save(user);
             return new CRAPIResponse(UserMessage.NUMBER_CHANGE_SUCCESSFUL, 200);
           }
-          return new CRAPIResponse(UserMessage.NEW_NUMBER_DOES_NOT_BELONG, 500);
+          return new CRAPIResponse(UserMessage.NEW_NUMBER_DOES_NOT_BELONG, 403);
         }
-        return new CRAPIResponse(UserMessage.OLD_NUMBER_DOES_NOT_BELONG, 500);
+        return new CRAPIResponse(UserMessage.OLD_NUMBER_DOES_NOT_BELONG, 403);
       }
-      return new CRAPIResponse(UserMessage.INVALID_OTP, 500);
+      return new CRAPIResponse(UserMessage.INVALID_OTP, 400);
     }
 
-    return new CRAPIResponse(UserMessage.INVALID_CREDENTIALS, 500);
+    return new CRAPIResponse(UserMessage.INVALID_CHANGE_REQUEST, 400);
   }
 }
diff --git a/services/identity/src/test/java/com/crapi/service/Impl/UserServiceImplTest.java b/services/identity/src/test/java/com/crapi/service/Impl/UserServiceImplTest.java
@@ -563,13 +563,13 @@ public void verifyPhoneOTPSuccessful() {
   @Test
   public void verifyOTPFailWhenChangePhoneRequestIsNull() {
     User user = getDummyUser();
-    String expectedMessage = UserMessage.INVALID_CREDENTIALS;
+    String expectedMessage = UserMessage.INVALID_CHANGE_REQUEST;
     ChangePhoneForm changePhoneForm = getDummyChangePhoneForm();
     Mockito.doReturn(user).when(userService).getUserFromToken(Mockito.any());
     Mockito.when(changePhoneRepository.findByUser(user)).thenReturn(null);
     CRAPIResponse crapiResponse = userService.verifyPhoneOTP(getMockHttpRequest(), changePhoneForm);
     Assertions.assertEquals(expectedMessage, crapiResponse.getMessage());
-    Assertions.assertEquals(HttpStatus.INTERNAL_SERVER_ERROR.value(), crapiResponse.getStatus());
+    Assertions.assertEquals(HttpStatus.BAD_REQUEST.value(), crapiResponse.getStatus());
   }
 
   @Test
@@ -583,7 +583,7 @@ public void verifyOTPFailWhenOTPIsNull() {
     Mockito.when(changePhoneRepository.findByUser(user)).thenReturn(changePhoneRequest);
     CRAPIResponse crapiResponse = userService.verifyPhoneOTP(getMockHttpRequest(), changePhoneForm);
     Assertions.assertEquals(expectedMessage, crapiResponse.getMessage());
-    Assertions.assertEquals(HttpStatus.INTERNAL_SERVER_ERROR.value(), crapiResponse.getStatus());
+    Assertions.assertEquals(HttpStatus.BAD_REQUEST.value(), crapiResponse.getStatus());
   }
 
   @Test
@@ -597,7 +597,7 @@ public void verifyOTPFailWhenOTPNotMatch() {
     Mockito.when(changePhoneRepository.findByUser(user)).thenReturn(changePhoneRequest);
     CRAPIResponse crapiResponse = userService.verifyPhoneOTP(getMockHttpRequest(), changePhoneForm);
     Assertions.assertEquals(expectedMessage, crapiResponse.getMessage());
-    Assertions.assertEquals(HttpStatus.INTERNAL_SERVER_ERROR.value(), crapiResponse.getStatus());
+    Assertions.assertEquals(HttpStatus.BAD_REQUEST.value(), crapiResponse.getStatus());
   }
 
   @Test
@@ -611,7 +611,7 @@ public void verifyOTPFailWhenOldNumberNotMatch() {
     Mockito.when(changePhoneRepository.findByUser(user)).thenReturn(changePhoneRequest);
     CRAPIResponse crapiResponse = userService.verifyPhoneOTP(getMockHttpRequest(), changePhoneForm);
     Assertions.assertEquals(expectedMessage, crapiResponse.getMessage());
-    Assertions.assertEquals(HttpStatus.INTERNAL_SERVER_ERROR.value(), crapiResponse.getStatus());
+    Assertions.assertEquals(HttpStatus.FORBIDDEN.value(), crapiResponse.getStatus());
   }
 
   @Test
@@ -626,7 +626,7 @@ public void verifyOTPFailWhenNewNumberNotMatch() {
     Mockito.when(changePhoneRepository.findByUser(user)).thenReturn(changePhoneRequest);
     CRAPIResponse crapiResponse = userService.verifyPhoneOTP(getMockHttpRequest(), changePhoneForm);
     Assertions.assertEquals(expectedMessage, crapiResponse.getMessage());
-    Assertions.assertEquals(HttpStatus.INTERNAL_SERVER_ERROR.value(), crapiResponse.getStatus());
+    Assertions.assertEquals(HttpStatus.FORBIDDEN.value(), crapiResponse.getStatus());
   }
 
   private LoginWithEmailToken getDummyLoginWithEmailToken() {