Skip to content

Fixing that a captcha is required on external login and registration - hotfix (Lombiq Technologies: GOV-44) #17489

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 18, 2025
Merged

Fixing that a captcha is required on external login and registration - hotfix (Lombiq Technologies: GOV-44) #17489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
687ba87
Fixing that a captcha is required on external login an registration
Piedone Feb 16, 2025
f8b077d
Better fix
Piedone Feb 17, 2025
fb27abc
Applying logic changes from the main PR
Piedone Feb 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions src/OrchardCore.Modules/OrchardCore.ReCaptcha/Users/Handlers/LoginFormEventEventHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Microsoft.AspNetCore.Identity;
using OrchardCore.ReCaptcha.Services;
using OrchardCore.Users;
using OrchardCore.Users.Events;
Expand All @@ -7,10 +8,12 @@ namespace OrchardCore.ReCaptcha.Users.Handlers;
public class LoginFormEventEventHandler : ILoginFormEvent
{
private readonly ReCaptchaService _reCaptchaService;
private readonly SignInManager<IUser> _signInManager;

public LoginFormEventEventHandler(ReCaptchaService reCaptchaService)
public LoginFormEventEventHandler(ReCaptchaService reCaptchaService, SignInManager<IUser> signInManager)
{
_reCaptchaService = reCaptchaService;
_signInManager = signInManager;
}

public Task IsLockedOutAsync(IUser user)
Expand All @@ -23,14 +26,16 @@ public Task LoggedInAsync(IUser user)
return Task.CompletedTask;
}

public Task LoggingInAsync(string userName, Action<string, string> reportError)
public async Task LoggingInAsync(string userName, Action<string, string> reportError)
{
if (_reCaptchaService.IsThisARobot())
// 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 (!_reCaptchaService.IsThisARobot() || await _signInManager.GetExternalLoginInfoAsync() != null)
{
return _reCaptchaService.ValidateCaptchaAsync(reportError);
return;
}

return Task.CompletedTask;
await _reCaptchaService.ValidateCaptchaAsync(reportError);
}

public Task LoggingInFailedAsync(string userName)
Expand Down
18 changes: 14 additions & 4 deletions src/OrchardCore.Modules/OrchardCore.ReCaptcha/Users/Handlers/RegistrationFormEventHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Microsoft.AspNetCore.Identity;
using OrchardCore.ReCaptcha.Services;
using OrchardCore.Users;
using OrchardCore.Users.Events;
Expand All @@ -7,19 +8,28 @@ namespace OrchardCore.ReCaptcha.Users.Handlers;
public class RegistrationFormEventHandler : IRegistrationFormEvents
{
private readonly ReCaptchaService _reCaptchaService;
private readonly SignInManager<IUser> _signInManager;

public RegistrationFormEventHandler(ReCaptchaService recaptchaService)
public RegistrationFormEventHandler(ReCaptchaService reCaptchaService, SignInManager<IUser> signInManager)
{
_reCaptchaService = recaptchaService;
_reCaptchaService = reCaptchaService;
_signInManager = signInManager;
}

public Task RegisteredAsync(IUser user)
{
return Task.CompletedTask;
}

public Task RegistrationValidationAsync(Action<string, string> reportError)
public async Task RegistrationValidationAsync(Action<string, string> reportError)
{
return _reCaptchaService.ValidateCaptchaAsync(reportError);
// 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering why this is missing the _reCaptchaService.IsThisARobot() check

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
return;
}

await _reCaptchaService.ValidateCaptchaAsync(reportError);
}
}