Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions src/Orbit.Api/Controllers/AuthController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public async Task<IActionResult> SendCode(

if (result.IsSuccess)
{
LogVerificationCodeSent(logger, request.Email, HttpContext.GetRequestId());
LogVerificationCodeSent(logger, HttpContext.GetRequestId());
return Ok(new { message = "Verification code sent" });
}

Expand Down Expand Up @@ -89,7 +89,7 @@ public async Task<IActionResult> VerifyCode(

if (result.IsSuccess)
{
LogUserLoggedInViaCode(logger, request.Email, HttpContext.GetRequestId());
LogUserLoggedInViaCode(logger, result.Value.UserId, HttpContext.GetRequestId());
return Ok(result.Value);
}

Expand Down Expand Up @@ -350,14 +350,14 @@ public async Task<IActionResult> ConfirmDeletion(
return result.ToErrorResult();
}

[LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "Verification code sent to {Email}. RequestId={RequestId}")]
private static partial void LogVerificationCodeSent(ILogger logger, string email, string requestId);
[LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "Verification code sent. RequestId={RequestId}")]
private static partial void LogVerificationCodeSent(ILogger logger, string requestId);

[LoggerMessage(EventId = 2, Level = LogLevel.Warning, Message = "Failed to send code: {Error}. RequestId={RequestId}")]
private static partial void LogFailedToSendCode(ILogger logger, string? error, string requestId);

[LoggerMessage(EventId = 3, Level = LogLevel.Information, Message = "User logged in via code {Email}. RequestId={RequestId}")]
private static partial void LogUserLoggedInViaCode(ILogger logger, string email, string requestId);
[LoggerMessage(EventId = 3, Level = LogLevel.Information, Message = "User logged in via code. UserId={UserId} RequestId={RequestId}")]
private static partial void LogUserLoggedInViaCode(ILogger logger, Guid userId, string requestId);

[LoggerMessage(EventId = 4, Level = LogLevel.Warning, Message = "Code verification failed: {Error}. RequestId={RequestId}")]
private static partial void LogCodeVerificationFailed(ILogger logger, string? error, string requestId);
Expand Down
10 changes: 5 additions & 5 deletions src/Orbit.Application/Auth/Commands/VerifyCodeCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ private async Task<bool> HandlePostLoginAsync(
}

if (isNewUser)
SendWelcomeEmailInBackground(user.Email, user.Name, request.Language);
SendWelcomeEmailInBackground(user.Id, user.Email, user.Name, request.Language);

return wasReactivated;
}
Expand All @@ -160,7 +160,7 @@ private async Task ProcessReferralSafeAsync(Guid userId, string referralCode, Ca
}
}

private void SendWelcomeEmailInBackground(string email, string name, string language)
private void SendWelcomeEmailInBackground(Guid userId, string email, string name, string language)
{
_ = Task.Run(async () =>
{
Expand All @@ -170,14 +170,14 @@ private void SendWelcomeEmailInBackground(string email, string name, string lang
}
catch (Exception ex) when (ex is not OperationCanceledException)
{
LogWelcomeEmailFailed(logger, ex, email);
LogWelcomeEmailFailed(logger, ex, userId);
}
}, CancellationToken.None);
}

[LoggerMessage(EventId = 1, Level = LogLevel.Warning, Message = "Referral processing failed for user {UserId}")]
private static partial void LogReferralProcessingFailed(ILogger logger, Exception ex, Guid userId);

[LoggerMessage(EventId = 2, Level = LogLevel.Warning, Message = "Welcome email failed for user {Email}")]
private static partial void LogWelcomeEmailFailed(ILogger logger, Exception ex, string email);
[LoggerMessage(EventId = 2, Level = LogLevel.Warning, Message = "Welcome email failed for user {UserId}")]
private static partial void LogWelcomeEmailFailed(ILogger logger, Exception ex, Guid userId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public async Task<Result<MarketingBroadcastResult>> Handle(
await emailService.SendMarketingEmailAsync(
request.TestEmail, preview.Subject, preview.BodyHtml, "en", previewUrl, cancellationToken);

LogPreviewSent(logger, request.TestEmail);
LogPreviewSent(logger, request.SubjectEn);
return Result.Success(new MarketingBroadcastResult(RecipientCount: 1, WasTest: true));
}

Expand All @@ -50,7 +50,7 @@ await emailService.SendMarketingEmailAsync(
.Select(user => new MarketingRecipient(user.Id, user.Email, user.Language ?? "en"))
.ToList();

LogBroadcastQueued(logger, recipients.Count, request.SubjectEn, DateTime.UtcNow);
LogBroadcastQueued(logger, recipients.Count, request.SubjectEn);
FanOutInBackground(request, recipients);

return Result.Success(new MarketingBroadcastResult(recipients.Count, WasTest: false));
Expand Down Expand Up @@ -109,11 +109,11 @@ private sealed record MarketingRecipient(Guid UserId, string Email, string Langu

private sealed record RenderedContent(string Subject, string BodyHtml);

[LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "Marketing broadcast preview sent to {TestEmail}")]
private static partial void LogPreviewSent(ILogger logger, string testEmail);
[LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "Marketing broadcast preview sent; subject={Subject}")]
private static partial void LogPreviewSent(ILogger logger, string subject);

[LoggerMessage(EventId = 2, Level = LogLevel.Information, Message = "Marketing broadcast queued: {RecipientCount} recipients, subject={Subject}, at {QueuedAtUtc:o}")]
private static partial void LogBroadcastQueued(ILogger logger, int recipientCount, string subject, DateTime queuedAtUtc);
[LoggerMessage(EventId = 2, Level = LogLevel.Information, Message = "Marketing broadcast queued: {RecipientCount} recipients, subject={Subject}")]
private static partial void LogBroadcastQueued(ILogger logger, int recipientCount, string subject);

[LoggerMessage(EventId = 3, Level = LogLevel.Information, Message = "Marketing broadcast fan-out completed: {SentCount} sent, {FailedCount} failed")]
private static partial void LogBroadcastCompleted(ILogger logger, int sentCount, int failedCount);
Expand Down
40 changes: 19 additions & 21 deletions src/Orbit.Infrastructure/Services/ResendEmailService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@

var tokens = new Dictionary<string, string>
{
["heading"] = copy.Heading,

Check warning on line 47 in src/Orbit.Infrastructure/Services/ResendEmailService.cs

View workflow job for this annotation

GitHub Actions / SonarCloud Analysis

Define a constant instead of using this literal 'heading' 4 times.
["intro"] = copy.Intro,

Check warning on line 48 in src/Orbit.Infrastructure/Services/ResendEmailService.cs

View workflow job for this annotation

GitHub Actions / SonarCloud Analysis

Define a constant instead of using this literal 'intro' 4 times.
["code"] = code,
["cta"] = copy.Cta,
["signInUrl"] = signInUrl,
["warning"] = copy.Warning,
["footer"] = copy.Footer,

Check warning on line 53 in src/Orbit.Infrastructure/Services/ResendEmailService.cs

View workflow job for this annotation

GitHub Actions / SonarCloud Analysis

Define a constant instead of using this literal 'footer' 4 times.
};

var layout = new EmailLayout(LangCode(isPtBr), copy.Preheader, copy.Footer, LogoUrl, GradientHeader: false);
Expand Down Expand Up @@ -145,12 +145,12 @@
$"<a href=\"{encodedUrl}\" style=\"color: #90A1B9; text-decoration: underline;\">{unsubscribeLabel}</a>";
}

private async Task SendMarketingWithBackoffAsync(string to, string subject, string serializedPayload, CancellationToken cancellationToken)

Check warning on line 148 in src/Orbit.Infrastructure/Services/ResendEmailService.cs

View workflow job for this annotation

GitHub Actions / SonarCloud Analysis

Refactor this method to reduce its Cognitive Complexity from 22 to the 15 allowed.
{
if (IsTestAccount(to))
{
if (logger.IsEnabled(LogLevel.Information))
LogSkippingTestEmail(logger, to, subject);
LogSkippingTestEmail(logger, subject);
return;
}

Expand All @@ -170,7 +170,7 @@
}
catch (Exception ex)
{
LogEmailSendException(logger, ex, to);
LogEmailSendException(logger, ex);
return;
}

Expand All @@ -179,23 +179,22 @@
if (response.IsSuccessStatusCode)
{
if (logger.IsEnabled(LogLevel.Information))
LogEmailSent(logger, to, subject);
LogEmailSent(logger, subject);
return;
}

var isRetriable = response.StatusCode == HttpStatusCode.TooManyRequests || (int)response.StatusCode >= 500;
if (!isRetriable || attempt == MaxMarketingRetries)
{
var body = await response.Content.ReadAsStringAsync(cancellationToken);
if (logger.IsEnabled(LogLevel.Error))
LogEmailFailed(logger, to, response.StatusCode, body);
LogEmailFailed(logger, subject, response.StatusCode);
return;
}
}

var backoff = TimeSpan.FromMilliseconds(_settings.MarketingRetryBaseDelayMs * Math.Pow(2, attempt));
if (logger.IsEnabled(LogLevel.Warning))
LogMarketingRetry(logger, to, attempt + 1, backoff.TotalMilliseconds);
LogMarketingRetry(logger, attempt + 1, backoff.TotalMilliseconds);
await Task.Delay(backoff, cancellationToken);
}
}
Expand Down Expand Up @@ -247,7 +246,7 @@
if (IsTestAccount(to))
{
if (logger.IsEnabled(LogLevel.Information))
LogSkippingTestEmail(logger, to, subject);
LogSkippingTestEmail(logger, subject);
return;
}

Expand All @@ -268,13 +267,12 @@
if (response.IsSuccessStatusCode)
{
if (logger.IsEnabled(LogLevel.Information))
LogEmailSent(logger, to, subject);
LogEmailSent(logger, subject);
}
else
{
var body = await response.Content.ReadAsStringAsync(cancellationToken);
if (logger.IsEnabled(LogLevel.Error))
LogEmailFailed(logger, to, response.StatusCode, body);
LogEmailFailed(logger, subject, response.StatusCode);
}
}
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
Expand All @@ -283,7 +281,7 @@
}
catch (Exception ex)
{
LogEmailSendException(logger, ex, to);
LogEmailSendException(logger, ex);
}
}

Expand All @@ -304,19 +302,19 @@
return false;
}

[LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "Skipping email to test account {To} subject={Subject}")]
private static partial void LogSkippingTestEmail(ILogger logger, string to, string subject);
[LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "Skipping email to test account; subject={Subject}")]
private static partial void LogSkippingTestEmail(ILogger logger, string subject);

[LoggerMessage(EventId = 2, Level = LogLevel.Information, Message = "Email sent to {To} subject={Subject}")]
private static partial void LogEmailSent(ILogger logger, string to, string subject);
[LoggerMessage(EventId = 2, Level = LogLevel.Information, Message = "Email sent; subject={Subject}")]
private static partial void LogEmailSent(ILogger logger, string subject);

[LoggerMessage(EventId = 3, Level = LogLevel.Error, Message = "Email failed to {To} status={Status} body={Body}")]
private static partial void LogEmailFailed(ILogger logger, string to, System.Net.HttpStatusCode status, string body);
[LoggerMessage(EventId = 3, Level = LogLevel.Error, Message = "Email failed; subject={Subject} status={Status}")]
private static partial void LogEmailFailed(ILogger logger, string subject, System.Net.HttpStatusCode status);

[LoggerMessage(EventId = 4, Level = LogLevel.Error, Message = "Email send exception to {To}")]
private static partial void LogEmailSendException(ILogger logger, Exception ex, string to);
[LoggerMessage(EventId = 4, Level = LogLevel.Error, Message = "Email send exception")]
private static partial void LogEmailSendException(ILogger logger, Exception ex);

[LoggerMessage(EventId = 5, Level = LogLevel.Warning, Message = "Marketing email to {To} rate-limited; retry {Attempt} after {BackoffMs}ms")]
private static partial void LogMarketingRetry(ILogger logger, string to, int attempt, double backoffMs);
[LoggerMessage(EventId = 5, Level = LogLevel.Warning, Message = "Marketing email rate-limited; retry {Attempt} after {BackoffMs}ms")]
private static partial void LogMarketingRetry(ILogger logger, int attempt, double backoffMs);

}
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,25 @@ public async Task Handle_NoConsentingUsers_QueuesZeroAndSendsNothing()
_emailService.MarketingSends.Should().BeEmpty();
}

[Fact]
public async Task Handle_TestEmail_PreviewLogDoesNotLeakTestEmail()
{
var logger = new CollectingLogger<SendMarketingBroadcastCommandHandler>();
var handler = new SendMarketingBroadcastCommandHandler(
_userRepo, _emailService, _tokenService, Substitute.For<IServiceScopeFactory>(),
Options.Create(new MarketingSettings { ApiBaseUrl = "https://api.useorbit.org", SendDelayMilliseconds = 0 }),
logger);

var command = new SendMarketingBroadcastCommand(
"EN Subject", "PT Assunto", "<p>EN body</p>", "<p>PT corpo</p>", "preview-pii@example.com");
await handler.Handle(command, CancellationToken.None);

var previewLog = logger.Entries.Should()
.ContainSingle(entry => entry.Contains("preview sent")).Subject;
previewLog.Should().NotContain("preview-pii@example.com");
previewLog.Should().Contain("EN Subject");
}

[Fact]
public async Task Handle_OneRecipientSendFails_ContinuesWithTheRest()
{
Expand All @@ -146,6 +165,22 @@ public async Task Handle_OneRecipientSendFails_ContinuesWithTheRest()
_emailService.MarketingSends.Should().ContainSingle(send => send.To == "second@example.com");
}

private sealed class CollectingLogger<T> : ILogger<T>
{
public List<string> Entries { get; } = [];

public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;

public bool IsEnabled(LogLevel logLevel) => true;

public void Log<TState>(
LogLevel logLevel,
EventId eventId,
TState state,
Exception? exception,
Func<TState, Exception?, string> formatter) => Entries.Add(formatter(state, exception));
}

private sealed class RecordingEmailService : IEmailService
{
public ConcurrentBag<(string To, string Subject, string Body, string Language, string UnsubscribeUrl)> MarketingSends { get; } = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public async Task SendCode_Failure_ReturnsBadRequest()
public async Task VerifyCode_Success_ReturnsOk()
{
_mediator.Send(Arg.Any<VerifyCodeCommand>(), Arg.Any<CancellationToken>())
.Returns(Result.Success(default(LoginResponse)!));
.Returns(Result.Success(new LoginResponse(UserId, "token", "Name", "test@example.com", false, "refresh")));

var request = new AuthController.VerifyCodeRequest("test@example.com", "123456");
var result = await _controller.VerifyCode(request, CancellationToken.None);
Expand Down Expand Up @@ -351,4 +351,59 @@ public async Task LogoutOperation_Failure_ReturnsUnauthorized()

result.Should().BeOfType<UnauthorizedObjectResult>();
}

[Fact]
public async Task SendCode_Success_LogDoesNotLeakEmail()
{
_mediator.Send(Arg.Any<SendCodeCommand>(), Arg.Any<CancellationToken>())
.Returns(Result.Success());
var (controller, logger) = BuildWithCapturingLogger();

await controller.SendCode(new AuthController.SendCodeRequest("pii-user@example.com"), CancellationToken.None);

logger.Entries.Should().NotBeEmpty();
logger.Entries.Should().OnlyContain(entry => !entry.Contains("pii-user@example.com"));
}

[Fact]
public async Task VerifyCode_Success_LogsUserIdInsteadOfEmail()
{
var userId = Guid.NewGuid();
_mediator.Send(Arg.Any<VerifyCodeCommand>(), Arg.Any<CancellationToken>())
.Returns(Result.Success(new LoginResponse(userId, "token", "Name", "pii-user@example.com", false, "refresh")));
var (controller, logger) = BuildWithCapturingLogger();

await controller.VerifyCode(
new AuthController.VerifyCodeRequest("pii-user@example.com", "123456"), CancellationToken.None);

var loginLog = logger.Entries.Should().ContainSingle(entry => entry.Contains("logged in via code")).Subject;
loginLog.Should().NotContain("pii-user@example.com");
loginLog.Should().Contain(userId.ToString());
}

private (AuthController Controller, CollectingLogger<AuthController> Logger) BuildWithCapturingLogger()
{
var logger = new CollectingLogger<AuthController>();
var controller = new AuthController(_mediator, _auditService, logger)
{
ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() }
};
return (controller, logger);
}

private sealed class CollectingLogger<T> : ILogger<T>
{
public List<string> Entries { get; } = [];

public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;

public bool IsEnabled(LogLevel logLevel) => true;

public void Log<TState>(
LogLevel logLevel,
EventId eventId,
TState state,
Exception? exception,
Func<TState, Exception?, string> formatter) => Entries.Add(formatter(state, exception));
}
}
Loading
Loading