Skip to content
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.

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

[CAPTCHA] Replace ReCAPTCHA with Turnstile #3249

Merged
merged 27 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
83e1825
Replace CAPTCHA with Turnstile (with test keys)
imnasnainaec Jul 16, 2024
969ccc9
Complete CAPTCHA replacement
imnasnainaec Jul 16, 2024
80ba196
Setup secret key
imnasnainaec Jul 16, 2024
3dc89a2
Fix App tests
imnasnainaec Jul 16, 2024
eec3ba0
Merge branch 'master' into turnstile
imnasnainaec Jul 16, 2024
b9f029a
Fix failing tests
imnasnainaec Jul 16, 2024
2674e73
Fix App tests
imnasnainaec Jul 16, 2024
d805fb4
Tidy Turnstile component
imnasnainaec Jul 17, 2024
1db2d1a
Add for Turnstile: environment variables, Context, Service
imnasnainaec Jul 17, 2024
b2eab3f
Replace frontend CAPTCHA configs with Turnstile config
imnasnainaec Jul 17, 2024
0bb8630
Clean up residue
imnasnainaec Jul 18, 2024
18cf48c
Simplify bool conditional
imnasnainaec Jul 18, 2024
cef2a5e
Add turnstileSiteKey config
imnasnainaec Jul 18, 2024
4f0035b
Restore frontend Captcha configs; Add backend Enabled configs
imnasnainaec Jul 19, 2024
caeccde
Unify frontend and backend captcha/email configs
imnasnainaec Jul 19, 2024
cedee67
Clean up loose ends
imnasnainaec Jul 19, 2024
7c30de3
Clean up
imnasnainaec Jul 19, 2024
bf7ac17
Move up
imnasnainaec Jul 19, 2024
fac46f2
Update from review
imnasnainaec Jul 22, 2024
1677530
Reenforce lowercase bool config strings
imnasnainaec Jul 22, 2024
303c745
Clean up bool configs
imnasnainaec Jul 23, 2024
c510663
Deploy to QA in this branch for testing
imnasnainaec Jul 23, 2024
34e4dcb
Restore deploy_qa safeguards
imnasnainaec Jul 24, 2024
68b4a31
Make subcharts stand-alone
imnasnainaec Jul 24, 2024
003edad
None-ify the pullSecretName default
imnasnainaec Jul 24, 2024
934944b
Merge branch 'master' into turnstile
imnasnainaec Jul 24, 2024
01b67b4
Merge branch 'master' into turnstile
imnasnainaec Jul 25, 2024
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
3 changes: 2 additions & 1 deletion .github/workflows/deploy_qa.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ on:
push:
branches:
- master
#- turnstile

permissions:
contents: read
Expand Down Expand Up @@ -94,7 +95,7 @@ jobs:
deploy_update:
needs: build
# Only push to the QA server when built on the master branch
if: ${{ github.ref_name == 'master' }}
#if: ${{ github.ref_name == 'master' }}
runs-on: [self-hosted, thecombine]
steps:
- name: Harden Runner
Expand Down
1 change: 0 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
"piptools",
"Prenoun",
"Preverb",
"recaptcha",
"reportgenerator",
"sched",
"signup",
Expand Down
2 changes: 1 addition & 1 deletion Backend.Tests/Controllers/UserControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public void Setup()
_userRepo = new UserRepositoryMock();
_permissionService = new PermissionServiceMock(_userRepo);
_userController = new UserController(_userRepo, _permissionService,
new EmailServiceMock(), new PasswordResetServiceMock());
new CaptchaServiceMock(), new EmailServiceMock(), new PasswordResetServiceMock());
}

private static User RandomUser()
Expand Down
13 changes: 13 additions & 0 deletions Backend.Tests/Mocks/CaptchaServiceMock.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using System.Threading.Tasks;
using BackendFramework.Interfaces;

namespace Backend.Tests.Mocks
{
sealed internal class CaptchaServiceMock : ICaptchaService
{
public Task<bool> VerifyToken(string token)
{
return Task.FromResult(true);
}
}
}
21 changes: 21 additions & 0 deletions Backend/Contexts/CaptchaContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
using System.Diagnostics.CodeAnalysis;
using BackendFramework.Interfaces;
using Microsoft.Extensions.Options;

namespace BackendFramework.Contexts
{
[ExcludeFromCodeCoverage]
public class CaptchaContext : ICaptchaContext
{
public bool CaptchaEnabled { get; }
public string? CaptchaSecretKey { get; }
public string? CaptchaVerifyUrl { get; }

public CaptchaContext(IOptions<Startup.Settings> options)
{
CaptchaEnabled = options.Value.CaptchaEnabled;
CaptchaSecretKey = options.Value.CaptchaSecretKey;
CaptchaVerifyUrl = options.Value.CaptchaVerifyUrl;
}
}
}
2 changes: 2 additions & 0 deletions Backend/Contexts/EmailContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ namespace BackendFramework.Contexts
[ExcludeFromCodeCoverage]
public class EmailContext : IEmailContext
{
public bool EmailEnabled { get; }
public string? SmtpServer { get; }
public int SmtpPort { get; }
public string? SmtpUsername { get; }
Expand All @@ -16,6 +17,7 @@ public class EmailContext : IEmailContext

public EmailContext(IOptions<Startup.Settings> options)
{
EmailEnabled = options.Value.EmailEnabled;
SmtpServer = options.Value.SmtpServer;
SmtpPort = options.Value.SmtpPort ?? IEmailContext.InvalidPort;
SmtpUsername = options.Value.SmtpUsername;
Expand Down
13 changes: 12 additions & 1 deletion Backend/Controllers/UserController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,30 @@ namespace BackendFramework.Controllers
public class UserController : Controller
{
private readonly IUserRepository _userRepo;
private readonly ICaptchaService _captchaService;
private readonly IEmailService _emailService;
private readonly IPasswordResetService _passwordResetService;
private readonly IPermissionService _permissionService;

public UserController(IUserRepository userRepo, IPermissionService permissionService,
IEmailService emailService, IPasswordResetService passwordResetService)
ICaptchaService captchaService, IEmailService emailService, IPasswordResetService passwordResetService)
{
_userRepo = userRepo;
_captchaService = captchaService;
_emailService = emailService;
_passwordResetService = passwordResetService;
_permissionService = permissionService;
}

/// <summary> Verifies a CAPTCHA token </summary>
[AllowAnonymous]
[HttpGet("captcha/{token}", Name = "VerifyCaptchaToken")]
[ProducesResponseType(StatusCodes.Status200OK)]
public async Task<IActionResult> VerifyCaptchaToken(string token)
{
return await _captchaService.VerifyToken(token) ? Ok() : BadRequest();
}

/// <summary> Sends a password reset request </summary>
[AllowAnonymous]
[HttpPost("forgot", Name = "ResetPasswordRequest")]
Expand Down
9 changes: 9 additions & 0 deletions Backend/Interfaces/ICaptchaContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
namespace BackendFramework.Interfaces
{
public interface ICaptchaContext
{
bool CaptchaEnabled { get; }
string? CaptchaSecretKey { get; }
string? CaptchaVerifyUrl { get; }
}
}
9 changes: 9 additions & 0 deletions Backend/Interfaces/ICaptchaService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using System.Threading.Tasks;

namespace BackendFramework.Interfaces
{
public interface ICaptchaService
{
Task<bool> VerifyToken(string token);
}
}
1 change: 1 addition & 0 deletions Backend/Interfaces/IEmailContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ public interface IEmailContext
/// This is value is set if the user does not supply an SMTP port number.
public const int InvalidPort = -1;

bool EmailEnabled { get; }
string? SmtpServer { get; }
int SmtpPort { get; }
string? SmtpUsername { get; }
Expand Down
12 changes: 10 additions & 2 deletions Backend/Properties/launchSettings.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
"launchBrowser": true,
"launchUrl": "v1",
"environmentVariables": {
"ASPNETCORE_ENVIRONMENT": "Development"
"ASPNETCORE_ENVIRONMENT": "Development",
"COMBINE_CAPTCHA_REQUIRED": "True",
"COMBINE_CAPTCHA_SECRET_KEY": "1x0000000000000000000000000000000AA",
"COMBINE_CAPTCHA_VERIFY_URL": "https://challenges.cloudflare.com/turnstile/v0/siteverify",
"COMBINE_JWT_SECRET_KEY": "0123456789abcdefghijklmnopqrstuvwxyz"
}
},
"BackendFramework": {
Expand All @@ -23,7 +27,11 @@
"launchUrl": "v1/",
"environmentVariables": {
"Key": "Value",
"ASPNETCORE_ENVIRONMENT": "Development"
"ASPNETCORE_ENVIRONMENT": "Development",
"COMBINE_CAPTCHA_REQUIRED": "True",
"COMBINE_CAPTCHA_SECRET_KEY": "1x0000000000000000000000000000000AA",
"COMBINE_CAPTCHA_VERIFY_URL": "https://challenges.cloudflare.com/turnstile/v0/siteverify",
"COMBINE_JWT_SECRET_KEY": "0123456789abcdefghijklmnopqrstuvwxyz"
},
"applicationUrl": "http://localhost:5000",
"hotReloadProfile": "aspnetcore"
Expand Down
44 changes: 44 additions & 0 deletions Backend/Services/CaptchaService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Net.Http;
using System.Threading.Tasks;
using BackendFramework.Interfaces;

namespace BackendFramework.Services
{
[ExcludeFromCodeCoverage]
public class CaptchaService : ICaptchaService
{
private readonly ICaptchaContext _captchaContext;

public CaptchaService(ICaptchaContext captchaContext)
{
_captchaContext = captchaContext;
}

public async Task<bool> VerifyToken(string token)
{
if (!_captchaContext.CaptchaEnabled)
{
throw new CaptchaNotEnabledException();
}

var secret = _captchaContext.CaptchaSecretKey;
var verifyUrl = _captchaContext.CaptchaVerifyUrl;
if (string.IsNullOrEmpty(secret) || string.IsNullOrEmpty(verifyUrl))
{
return false;
}
var httpContent = new FormUrlEncodedContent(new Dictionary<string, string>{
{"response", token},
{"secret", secret},
});
using var result = await new HttpClient().PostAsync(verifyUrl, httpContent);
var contentString = await result.Content.ReadAsStringAsync();
return contentString.Contains("\"success\":true");
}

private sealed class CaptchaNotEnabledException : Exception { }
}
}
10 changes: 9 additions & 1 deletion Backend/Services/EmailService.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Diagnostics.CodeAnalysis;
using BackendFramework.Interfaces;
using System.Threading.Tasks;
using BackendFramework.Interfaces;
using MimeKit;

namespace BackendFramework.Services
Expand All @@ -17,6 +18,11 @@ public EmailService(IEmailContext emailContext)

public async Task<bool> SendEmail(MimeMessage message)
{
if (!_emailContext.EmailEnabled)
{
throw new EmailNotEnabledException();
}

using var client = new MailKit.Net.Smtp.SmtpClient();

await client.ConnectAsync(_emailContext.SmtpServer, _emailContext.SmtpPort);
Expand All @@ -33,5 +39,7 @@ public async Task<bool> SendEmail(MimeMessage message)
await client.DisconnectAsync(true);
return true;
}

private sealed class EmailNotEnabledException : Exception { }
}
}
94 changes: 68 additions & 26 deletions Backend/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,33 +41,46 @@

public string ConnectionString { get; set; }
public string CombineDatabase { get; set; }
public bool EmailEnabled { get; set; }
public string? SmtpServer { get; set; }
public int? SmtpPort { get; set; }
public string? SmtpUsername { get; set; }
public string? SmtpPassword { get; set; }
public string? SmtpAddress { get; set; }
public string? SmtpFrom { get; set; }
public int PassResetExpireTime { get; set; }
public bool CaptchaEnabled { get; set; }
public string? CaptchaSecretKey { get; set; }
public string? CaptchaVerifyUrl { get; set; }

public Settings()
{
ConnectionString = "";
CombineDatabase = "";
EmailEnabled = false;
PassResetExpireTime = DefaultPasswordResetExpireTime;
CaptchaEnabled = true;
}
}

private sealed class EnvironmentNotConfiguredException : Exception { }

private string? CheckedEnvironmentVariable(string name, string? defaultValue, string error = "")
private string? CheckedEnvironmentVariable(string name, string? defaultValue, string error = "", bool info = false)
{
var contents = Environment.GetEnvironmentVariable(name);
if (contents is not null)
{
return contents;
}

_logger.LogError("Environment variable: {Name} is not defined. {Error}", name, error);
if (info)
{
_logger.LogInformation("Environment variable: {Name} is not defined. {Error}", name, error);
Dismissed Show dismissed Hide dismissed
Dismissed Show dismissed Hide dismissed
}
else
{
_logger.LogError("Environment variable: {Name} is not defined. {Error}", name, error);
Dismissed Show dismissed Hide dismissed
Dismissed Show dismissed Hide dismissed
}
return defaultValue;
}

Expand Down Expand Up @@ -153,31 +166,56 @@
options.CombineDatabase = Configuration["MongoDB:CombineDatabase"]
?? throw new EnvironmentNotConfiguredException();

options.CaptchaEnabled = bool.Parse(CheckedEnvironmentVariable(
"COMBINE_CAPTCHA_REQUIRED",
bool.TrueString, // "True"
"CAPTCHA should be explicitly required or not required.")!);
if (options.CaptchaEnabled)
{
options.CaptchaSecretKey = CheckedEnvironmentVariable(
"COMBINE_CAPTCHA_SECRET_KEY",
null,
"CAPTCHA secret key required.");
options.CaptchaVerifyUrl = CheckedEnvironmentVariable(
"COMBINE_CAPTCHA_VERIFY_URL",
null,
"CAPTCHA verification URL required.");
}

const string emailServiceFailureMessage = "Email services will not work.";
options.SmtpServer = CheckedEnvironmentVariable(
"COMBINE_SMTP_SERVER",
null,
emailServiceFailureMessage);
options.SmtpPort = int.Parse(CheckedEnvironmentVariable(
"COMBINE_SMTP_PORT",
IEmailContext.InvalidPort.ToString(),
emailServiceFailureMessage)!);
options.SmtpUsername = CheckedEnvironmentVariable(
"COMBINE_SMTP_USERNAME",
null,
emailServiceFailureMessage);
options.SmtpPassword = CheckedEnvironmentVariable(
"COMBINE_SMTP_PASSWORD",
null,
emailServiceFailureMessage);
options.SmtpAddress = CheckedEnvironmentVariable(
"COMBINE_SMTP_ADDRESS",
null,
emailServiceFailureMessage);
options.SmtpFrom = CheckedEnvironmentVariable(
"COMBINE_SMTP_FROM",
null,
emailServiceFailureMessage);
options.EmailEnabled = bool.Parse(CheckedEnvironmentVariable(
"COMBINE_EMAIL_ENABLED",
bool.FalseString, // "False"
emailServiceFailureMessage,
true)!);
if (options.EmailEnabled)
{
options.SmtpServer = CheckedEnvironmentVariable(
"COMBINE_SMTP_SERVER",
null,
emailServiceFailureMessage);
options.SmtpPort = int.Parse(CheckedEnvironmentVariable(
"COMBINE_SMTP_PORT",
IEmailContext.InvalidPort.ToString(),
emailServiceFailureMessage)!);
options.SmtpUsername = CheckedEnvironmentVariable(
"COMBINE_SMTP_USERNAME",
null,
emailServiceFailureMessage);
options.SmtpPassword = CheckedEnvironmentVariable(
"COMBINE_SMTP_PASSWORD",
null,
emailServiceFailureMessage);
options.SmtpAddress = CheckedEnvironmentVariable(
"COMBINE_SMTP_ADDRESS",
null,
emailServiceFailureMessage);
options.SmtpFrom = CheckedEnvironmentVariable(
"COMBINE_SMTP_FROM",
null,
emailServiceFailureMessage);
}

options.PassResetExpireTime = int.Parse(CheckedEnvironmentVariable(
"COMBINE_PASSWORD_RESET_EXPIRE_TIME",
Settings.DefaultPasswordResetExpireTime.ToString(),
Expand All @@ -190,6 +228,10 @@
services.AddTransient<IBannerContext, BannerContext>();
services.AddTransient<IBannerRepository, BannerRepository>();

// CAPTCHA types
services.AddTransient<ICaptchaContext, CaptchaContext>();
services.AddTransient<ICaptchaService, CaptchaService>();

// Email types
services.AddTransient<IEmailContext, EmailContext>();
services.AddTransient<IEmailService, EmailService>();
Expand Down
Loading
Loading