SMSOTP Improvements with new features #25
Conversation
| + e.getMessage(), e); | ||
| } | ||
| } | ||
| if (!SMSOTPConstants.UNABLE_SEND_CODE.equals(statusCode)) { |
There was a problem hiding this comment.
Include this as else if. Avoid nested if since it's not required.
| } | ||
| if (userToken.equals(contextToken)) { | ||
| context.setSubject(authenticatedUser); | ||
| } else if (SMSOTPUtils.getBackupCode(context, getName()).equals("false")) { |
There was a problem hiding this comment.
Why we have two checks for backup codes?
There was a problem hiding this comment.
Checking with backup code also configurable. If that property false, then no need to check with backup codes, it will allowed to retry with another code.
| private void updateMobileNumberForUsername(AuthenticationContext context, HttpServletRequest request, | ||
| String username, String tenantDomain) | ||
| throws SMSOTPException, AuthenticationFailedException { | ||
| if (username != null && !context.isRetrying()) { |
There was a problem hiding this comment.
We need to validate if a mobile number already exists before updating the mobile claim.
There was a problem hiding this comment.
In getMobileNumber() method, first we have checked the mobile number whether it is already exist or not. If it is empty only, we proceed with the redirection to mobile number request page and update the mobile claim
| smsUrl = smsUrl.replaceAll("\\$ctx.num", encodedMobileNo).replaceAll("\\$ctx.msg", | ||
| smsMessage.replaceAll("\\s", "+") + otpToken); | ||
| URL smsProviderUrl = new URL(smsUrl); | ||
| HttpURLConnection httpConnection = (HttpURLConnection) smsProviderUrl.openConnection(); |
There was a problem hiding this comment.
Move this to else block. You are opening the connection two times for https flow.
No description provided.