Fixing that a captcha is required on external login and registration - hotfix (Lombiq Technologies: GOV-44) #17489
Conversation
Piedone
changed the title
| { | ||
| if (_reCaptchaService.IsThisARobot()) | ||
| if (_reCaptchaService.IsThisARobot() && await _signInManager.GetExternalLoginInfoAsync() == null) |
There was a problem hiding this comment.
| if (_reCaptchaService.IsThisARobot() && await _signInManager.GetExternalLoginInfoAsync() == null) | |
| if (_reCaptchaService.IsThisARobot() && await _signInManager.GetExternalLoginInfoAsync() is null) |
There was a problem hiding this comment.
I still don't think that the pattern matching syntax brings any value when there is no actual pattern to match.
| { | ||
| return _reCaptchaService.ValidateCaptchaAsync(reportError); | ||
| if (await _signInManager.GetExternalLoginInfoAsync() == null) |
There was a problem hiding this comment.
| if (await _signInManager.GetExternalLoginInfoAsync() == null) | |
| if (await _signInManager.GetExternalLoginInfoAsync() is null) |
|
After looking at #17490, I think we should undo the interface rename and merge that PR instead. Then close this PR and backport that one into release/2.1 |
MikeAlhayek
dismissed
their stale review
I think we should hold off on this and take the other PR
|
I think that PR will have merge conflicts, but we can try. |
|
I'll close this if #17490 can be backported. |
|
@Piedone the backport failed due to a conflict as you expected. If you can please update the logic here along with the comments as you did in the other PR and merge this so we can release |
|
Done. |
| if (await _signInManager.GetExternalLoginInfoAsync() == null) | ||
| // When logging in via an external provider, authentication security is already handled by the provider. | ||
| // Therefore, using a CAPTCHA is unnecessary and impractical, as users wouldn't be able to complete it anyway. | ||
| if (await _signInManager.GetExternalLoginInfoAsync() != null) |
There was a problem hiding this comment.
I am wondering why this is missing the _reCaptchaService.IsThisARobot() check
There was a problem hiding this comment.
I don't know, but since that caused problems in the other case too (hence you removed it in main), I wouldn't add it.
Fixes #17422. Since a lot has changed in
mainin these classes, I started this right from the release branch. This is only a temporary hotfix, to be released in 2.1.6.The
mainversion is here: #17490.