[ES-1601] Captcha validation support during send-binding-otp #961
Conversation
Signed-off-by: Md-Humair-KK <[email protected]>
Signed-off-by: Md-Humair-KK <[email protected]>
Signed-off-by: Md-Humair-KK <[email protected]>
Signed-off-by: Md-Humair-KK <[email protected]>
Signed-off-by: Md-Humair-KK <[email protected]>
| <dependency> | ||
| <groupId>io.mosip.esignet</groupId> | ||
| <artifactId>oidc-service-impl</artifactId> | ||
| <version>${project.version}</version> | ||
| <scope>compile</scope> | ||
| </dependency> |
There was a problem hiding this comment.
If CacheUtilService is the only dependent service class, we should move CacheUtilService to esignet-core instead of adding oidc-service-impl as a dependency to binding-service-impl. It's important to keep them as separate services to maintain the "separation of concerns."
If CacheUtilService cannot be moved to esignet-core, we can create new Cache service in binding-service-impl module.
There was a problem hiding this comment.
Also, can we limit this PR only to captcha validation? API rate limit can be a separate PR. Let us explore bucket4j for the API rate limit.
| @@ -119,6 +126,19 @@ public void sendBindingOtp_withInvalidRequest_thenFail() throws SendOtpException | |||
| keyBindingService.sendBindingOtp(otpRequest, headers); | |||
| } | |||
|
|
|||
| @Test(expected = EsignetException.class) | |||
There was a problem hiding this comment.
Adding the assertion on the errorCode is better.
| import io.mosip.esignet.core.constants.ErrorConstants; | ||
| import io.mosip.esignet.core.validator.OtpChannel; | ||
| import lombok.Data; | ||
|
|
||
| @Data | ||
| @JsonInclude(JsonInclude.Include.NON_NULL) |
There was a problem hiding this comment.
Why is this annotation added @Md-Humair-KK?
| @@ -24,4 +26,6 @@ public class BindingOtpRequest { | |||
| @NotNull(message = ErrorConstants.INVALID_OTP_CHANNEL) | |||
| @Size(min = 1, message = ErrorConstants.INVALID_OTP_CHANNEL) | |||
| private List<@OtpChannel String> otpChannels; | |||
|
|
|||
| private String captcha; | |||
There was a problem hiding this comment.
This will hold the captcha token, so I think it's apt to rename this as captchaToken
| public CaptchaHelper(RestTemplate restTemplate, String validatorUrl, String moduleName) { | ||
| this.restTemplate = restTemplate; | ||
| this.validatorUrl = validatorUrl; | ||
| this.moduleName = moduleName; | ||
| } |
There was a problem hiding this comment.
Can we remove this old constructor? so it will not confuse in the future.
ES-1601 Captcha validation support during send-binding-otp
Captcha should be used when requesting to send-otp.
2. In esignet binding-otp must validate the captcha token passed in the request.
3. A limit should apply to how often binding-otp can be invoked.