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
2 changes: 1 addition & 1 deletion .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
sed -i 's/"__DATABASE_CONNECTION_STRING",/"${{ vars.DATABASE_CONNECTION_STRING }}",/g' ${{env.PRODUCTION_SETTINGS_FILE}}
sed -i 's/"__TELEGRAM_BOT_KEY",/"${{ secrets.TELEGRAM_BOT_KEY }}",/g' ${{env.PRODUCTION_SETTINGS_FILE}}
sed -i 's/"__PROFILE_GITHUB_BOT_KEY",/"${{ secrets.PROFILE_GITHUB_BOT_KEY }}",/g' ${{env.PRODUCTION_SETTINGS_FILE}}
sed -i 's/"__SENDGRID_API_KEY",/"${{ secrets.SENDGRID_API_KEY }}",/g' ${{env.PRODUCTION_SETTINGS_FILE}}
sed -i 's/"__RESEND_COM_API_KEY",/"${{ secrets.RESEND_COM_API_KEY }}",/g' ${{env.PRODUCTION_SETTINGS_FILE}}
sed -i 's/"__SENDGRID_WEBHOOK_SIGNATURE",/"${{ secrets.SENDGRID_WEBHOOK_SIGNATURE }}",/g' ${{env.PRODUCTION_SETTINGS_FILE}}
sed -i 's/"__OPEN_AI_SECRET",/"${{ secrets.OPEN_AI_SECRET }}",/g' ${{env.PRODUCTION_SETTINGS_FILE}}
sed -i "s|__OPENAI_API_KEY|${{ secrets.OPENAI_API_KEY }}|g" ${{env.PRODUCTION_SETTINGS_FILE}}
Expand Down
77 changes: 77 additions & 0 deletions src/Infrastructure/Emails/ResendEmailApiSender.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
using Domain.Validation;
using Infrastructure.Emails.Contracts;
using Infrastructure.Emails.Contracts.Requests;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Logging;
using Resend;

namespace Infrastructure.Emails;

public class ResendEmailApiSender : IEmailApiSender
{
private readonly IResend _client;
private readonly ILogger<ResendEmailApiSender> _logger;

public ResendEmailApiSender(
IConfiguration configuration,
ILogger<ResendEmailApiSender> logger)
{
_logger = logger;
var apiKey = configuration["ResendComApiKey"]
?? throw new InvalidOperationException("ResendComApiKey configuration is missing");

_client = ResendClient.Create(apiKey);
}

public async Task SendAsync(
EmailContent email,
CancellationToken cancellationToken)
{
email.ThrowIfNull(nameof(email));

try
{
var message = new EmailMessage
{
From = email.From,
Subject = email.Subject,
HtmlBody = email.HtmlBody
};

foreach (var recipient in email.Recipients)
{
message.To.Add(recipient);
}
Comment on lines +41 to +44

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate and examine the ResendEmailApiSender.cs file
find . -name "ResendEmailApiSender.cs" -type f

Repository: Techinterview-space/web-api

Length of output: 122


🏁 Script executed:

# Also search for the EmailContent class definition
fd "EmailContent" --type f -e cs

Repository: Techinterview-space/web-api

Length of output: 131


🏁 Script executed:

# Check for any file that might contain the email-related models
fd -e cs -x grep -l "Recipients" {} \;

Repository: Techinterview-space/web-api

Length of output: 372


🏁 Script executed:

# Read the ResendEmailApiSender.cs file to see the context around lines 41-44
cat -n ./src/Infrastructure/Emails/ResendEmailApiSender.cs

Repository: Techinterview-space/web-api

Length of output: 2747


🏁 Script executed:

# Read the EmailContent.cs file to understand Recipients property
cat -n src/Infrastructure/Emails/Contracts/Requests/EmailContent.cs

Repository: Techinterview-space/web-api

Length of output: 1196


🏁 Script executed:

# Check if the project has nullable reference types enabled
cat -n src/Infrastructure/Emails/ResendEmailApiSender.cs | head -20

Repository: Techinterview-space/web-api

Length of output: 805


🏁 Script executed:

# Check if there are any usage patterns or prior validation for Recipients
rg -n "Recipients" --type cs -B 2 -A 2 src/Web.Api/Services/Emails/TechInterviewerEmailService.cs

Repository: Techinterview-space/web-api

Length of output: 303


🏁 Script executed:

# Check the standard.ruleset to understand linting rules
find . -name "standard.ruleset" -o -name "*.ruleset" | head -5

Repository: Techinterview-space/web-api

Length of output: 93


Add null/empty validation for Recipients to prevent NullReferenceException.

The Recipients collection can be null when EmailContent is instantiated via the parameterless constructor (e.g., ASP.NET model binding), and iterating over it without checking will throw. While Cc and HiddenCc are defensively validated with .Any() checks before iteration (lines 46, 55), Recipients lacks this protection.

Proposed fix
+        if (email.Recipients == null || !email.Recipients.Any())
+        {
+            throw new ArgumentException("Email must have at least one recipient", nameof(email));
+        }
+
         foreach (var recipient in email.Recipients)
         {
             message.To.Add(recipient);
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
foreach (var recipient in email.Recipients)
{
message.To.Add(recipient);
}
if (email.Recipients == null || !email.Recipients.Any())
{
throw new ArgumentException("Email must have at least one recipient", nameof(email));
}
foreach (var recipient in email.Recipients)
{
message.To.Add(recipient);
}
🤖 Prompt for AI Agents
In `@src/Infrastructure/Emails/ResendEmailApiSender.cs` around lines 41 - 44, The
loop that adds recipients to the MailMessage (the block containing
message.To.Add(recipient) in ResendEmailApiSender.cs) can throw a
NullReferenceException because EmailContent.Recipients may be null; add a
defensive null/empty check before iterating (e.g., guard with email.Recipients
!= null && email.Recipients.Any()) or iterate over email.Recipients ??
Enumerable.Empty<string>() so the foreach only runs when recipients exist;
ensure any needed System.Linq/System.Collections.Generic imports are present and
follow the same pattern used for Cc and HiddenCc.


if (email.Cc.Any())
{
message.Cc ??= new EmailAddressList();
foreach (var cc in email.Cc)
{
message.Cc.Add(cc);
}
}

if (email.HiddenCc.Any())
{
message.Bcc ??= new EmailAddressList();
foreach (var bcc in email.HiddenCc)
{
message.Bcc.Add(bcc);
}
}
Comment on lines +46 to +62

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential NullReferenceException when calling .Any() on null collections.

If email.Cc or email.HiddenCc are null, calling .Any() will throw. Consider using null-conditional or null-coalescing patterns.

🐛 Proposed fix
-        if (email.Cc.Any())
+        if (email.Cc?.Any() == true)
         {
             message.Cc ??= new EmailAddressList();
             foreach (var cc in email.Cc)
             {
                 message.Cc.Add(cc);
             }
         }

-        if (email.HiddenCc.Any())
+        if (email.HiddenCc?.Any() == true)
         {
             message.Bcc ??= new EmailAddressList();
             foreach (var bcc in email.HiddenCc)
             {
                 message.Bcc.Add(bcc);
             }
         }
🤖 Prompt for AI Agents
In `@src/Infrastructure/Emails/ResendEmailApiSender.cs` around lines 46 - 62, In
ResendEmailApiSender.cs the code calls email.Cc.Any() and email.HiddenCc.Any()
which can throw if those lists are null; update the checks to guard for null
(e.g., use email.Cc != null && email.Cc.Any() or email.Cc?.Any() == true) and
when iterating use a null-safe source (e.g., iterate over email.Cc ??
Enumerable.Empty<...>() or only enter the foreach after the null check), keeping
the existing assignments to message.Cc and message.Bcc; reference symbols:
email.Cc, email.HiddenCc, message.Cc, message.Bcc in the sending method of the
ResendEmailApiSender class.


var response = await _client.EmailSendAsync(message, cancellationToken);

_logger.LogInformation(
"Email was sent successfully via Resend. MessageId: {MessageId}",
response.Content);
}
catch (Exception e)
{
_logger.LogError(
e,
"Could not send email via Resend");
}
Comment on lines +70 to +75

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Swallowing exceptions silently hides email delivery failures.

The catch block logs the error but doesn't propagate it. Callers will have no way to know the email failed to send, which could lead to silent failures in critical notification flows (e.g., password resets, interview confirmations).

🐛 Proposed fix - rethrow or return a result

Option 1: Rethrow after logging:

         catch (Exception e)
         {
             _logger.LogError(
                 e,
                 "Could not send email via Resend");
+            throw;
         }

Option 2: Return a result type that indicates success/failure (requires interface change).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
catch (Exception e)
{
_logger.LogError(
e,
"Could not send email via Resend");
}
catch (Exception e)
{
_logger.LogError(
e,
"Could not send email via Resend");
throw;
}
🤖 Prompt for AI Agents
In `@src/Infrastructure/Emails/ResendEmailApiSender.cs` around lines 70 - 75, The
catch in ResendEmailApiSender (inside the SendEmailAsync method) currently logs
exceptions but swallows them; update the method to surface failures by either
rethrowing the exception after logging (use "throw;" to preserve stack trace) or
change the method signature and related interface to return a result/boolean
indicating success and return a failure result after logging; ensure all callers
are updated to handle the rethrown exception or the new result type accordingly.

}
}
78 changes: 0 additions & 78 deletions src/Infrastructure/Emails/SendgridEmailApiSender.cs

This file was deleted.

3 changes: 1 addition & 2 deletions src/Infrastructure/Infrastructure.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,12 @@
<PackageReference Include="QRCoder" Version="1.7.0" />
<PackageReference Include="QuestPDF" Version="2025.12.0" />
<PackageReference Include="QuestPDF.Markdown" Version="1.44.0" />
<PackageReference Include="SendGrid" Version="9.29.3" />
<PackageReference Include="Resend" Version="0.2.1" />
<PackageReference Include="Serilog" Version="3.1.1" />
<PackageReference Include="Serilog.AspNetCore" Version="8.0.1" />
<PackageReference Include="Serilog.Extensions.Logging" Version="8.0.0" />
<PackageReference Include="Serilog.Sinks.Console" Version="5.0.1" />
<PackageReference Include="Serilog.Sinks.Elasticsearch" Version="9.0.3" />
<PackageReference Include="StrongGrid" Version="0.114.0" />
<PackageReference Include="System.IdentityModel.Tokens.Jwt" Version="8.15.0" />
<PackageReference Include="System.Net.Http" Version="4.3.4" NoWarn="NU1510" />
<PackageReference Include="System.Text.RegularExpressions" Version="4.3.1" NoWarn="NU1510" />
Expand Down
78 changes: 78 additions & 0 deletions src/Web.Api/Features/Admin/AdminEmailController.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Threading;
using System.Threading.Tasks;
using Domain.Enums;
using Infrastructure.Emails.Contracts;
using Infrastructure.Emails.Contracts.Requests;
using Microsoft.AspNetCore.Mvc;
using Web.Api.Setup.Attributes;

namespace Web.Api.Features.Admin;

[ApiController]
[Route("api/admin/emails")]
[HasAnyRole(Role.Admin)]
public class AdminEmailController : ControllerBase
{
private readonly IEmailApiSender _emailApiSender;

public AdminEmailController(IEmailApiSender emailApiSender)
{
_emailApiSender = emailApiSender;
}

[HttpPost("send")]
public async Task<IActionResult> SendCustomEmailAsync(
[FromBody] SendCustomEmailRequest request,
CancellationToken cancellationToken)
{
var recipients = new List<string> { request.To };

var cc = !string.IsNullOrWhiteSpace(request.Cc)
? new List<string> { request.Cc }
: new List<string>();

await _emailApiSender.SendAsync(
new EmailContent(
request.From,
request.Subject,
request.HtmlBody,
recipients,
cc),
cancellationToken);

return Ok(new SendCustomEmailResponse
{
Success = true,
Message = "Email sent successfully"
});
Comment on lines +25 to +49

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don’t return success when the send fails.
ResendEmailApiSender currently swallows exceptions and only logs them, so this action will always return Success=true even when the email was not sent. Please propagate failures (throw or return a result) and surface a non‑200 response when sending fails.

🔧 Proposed controller-side handling (pairs with making sender throw or return failure)
-        await _emailApiSender.SendAsync(
-            new EmailContent(
-                request.From,
-                request.Subject,
-                request.HtmlBody,
-                recipients,
-                cc),
-            cancellationToken);
-
-        return Ok(new SendCustomEmailResponse
-        {
-            Success = true,
-            Message = "Email sent successfully"
-        });
+        try
+        {
+            await _emailApiSender.SendAsync(
+                new EmailContent(
+                    request.From,
+                    request.Subject,
+                    request.HtmlBody,
+                    recipients,
+                    cc),
+                cancellationToken);
+
+            return Ok(new SendCustomEmailResponse
+            {
+                Success = true,
+                Message = "Email sent successfully"
+            });
+        }
+        catch (Exception)
+        {
+            return StatusCode(502, new SendCustomEmailResponse
+            {
+                Success = false,
+                Message = "Email send failed"
+            });
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[HttpPost("send")]
public async Task<IActionResult> SendCustomEmailAsync(
[FromBody] SendCustomEmailRequest request,
CancellationToken cancellationToken)
{
var recipients = new List<string> { request.To };
var cc = !string.IsNullOrWhiteSpace(request.Cc)
? new List<string> { request.Cc }
: new List<string>();
await _emailApiSender.SendAsync(
new EmailContent(
request.From,
request.Subject,
request.HtmlBody,
recipients,
cc),
cancellationToken);
return Ok(new SendCustomEmailResponse
{
Success = true,
Message = "Email sent successfully"
});
[HttpPost("send")]
public async Task<IActionResult> SendCustomEmailAsync(
[FromBody] SendCustomEmailRequest request,
CancellationToken cancellationToken)
{
var recipients = new List<string> { request.To };
var cc = !string.IsNullOrWhiteSpace(request.Cc)
? new List<string> { request.Cc }
: new List<string>();
try
{
await _emailApiSender.SendAsync(
new EmailContent(
request.From,
request.Subject,
request.HtmlBody,
recipients,
cc),
cancellationToken);
return Ok(new SendCustomEmailResponse
{
Success = true,
Message = "Email sent successfully"
});
}
catch (Exception)
{
return StatusCode(502, new SendCustomEmailResponse
{
Success = false,
Message = "Email send failed"
});
}
}
🤖 Prompt for AI Agents
In `@src/Web.Api/Features/Admin/AdminEmailController.cs` around lines 25 - 49,
SendCustomEmailAsync currently assumes _emailApiSender.SendAsync always
succeeds, but ResendEmailApiSender swallows exceptions so the controller returns
Success=true even on failure; update the flow so send failures are propagated
and surfaced: modify ResendEmailApiSender to either throw the exception on
failure or return a failure result, then update SendCustomEmailAsync to await
SendAsync and either catch exceptions and return an error IActionResult (e.g.,
BadRequest or StatusCode(500) with Success=false and a helpful Message) or
inspect the returned result and return Success=false when appropriate; reference
SendCustomEmailAsync, _emailApiSender.SendAsync, ResendEmailApiSender, and
SendCustomEmailResponse when making the changes.

}

public record SendCustomEmailRequest
{
[Required]
[EmailAddress]
public string To { get; init; }

[EmailAddress]
public string Cc { get; init; }

[Required]
[EmailAddress]
public string From { get; init; }

[Required]
public string Subject { get; init; }

[Required]
public string HtmlBody { get; init; }
}

public record SendCustomEmailResponse
{
public bool Success { get; init; }

public string Message { get; init; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
using System.Threading.Tasks;
using Domain.Entities.Salaries;
using Domain.Enums;
using Domain.Validation.Exceptions;
using Infrastructure.Authentication.Contracts;
using Infrastructure.Database;
using Infrastructure.Salaries;
using Microsoft.EntityFrameworkCore;
using SendGrid.Helpers.Errors.Model;
using Web.Api.Features.Salaries.Models;
using NotFoundException = Domain.Validation.Exceptions.NotFoundException;

Expand Down Expand Up @@ -34,12 +34,12 @@ public async Task<CreateOrEditSalaryRecordResponse> Handle(
.FirstOrDefaultAsync(x => x.Id == request.Id, cancellationToken)
?? throw new NotFoundException("Salary record not found");

var currentUser = await _auth.GetCurrentUserOrNullAsync();
var currentUser = await _auth.GetCurrentUserOrNullAsync(cancellationToken);

if (!currentUser.Has(Role.Admin) &&
salary.UserId != currentUser.Id)
{
throw new ForbiddenException("You can only edit your own salary records");
throw new NoPermissionsException("You can only edit your own salary records");
}
Comment on lines +37 to 43

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Potential NullReferenceException when accessing currentUser.

The method GetCurrentUserOrNullAsync (by naming convention) can return null, but currentUser is used directly on lines 39-40 without a null check. If no authenticated user exists, this will throw a NullReferenceException.

🐛 Proposed fix: Add null check or throw appropriate exception
 var currentUser = await _auth.GetCurrentUserOrNullAsync(cancellationToken);
+if (currentUser is null)
+{
+    throw new NoPermissionsException("Authentication required");
+}

 if (!currentUser.Has(Role.Admin) &&
     salary.UserId != currentUser.Id)
 {
     throw new NoPermissionsException("You can only edit your own salary records");
 }

Alternatively, if the endpoint is already protected by an [Authorize] attribute and currentUser is guaranteed to be non-null at this point, consider using a method like GetCurrentUserAsync that throws if the user is not authenticated, to make this expectation explicit and avoid the misleading OrNull suffix.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var currentUser = await _auth.GetCurrentUserOrNullAsync(cancellationToken);
if (!currentUser.Has(Role.Admin) &&
salary.UserId != currentUser.Id)
{
throw new ForbiddenException("You can only edit your own salary records");
throw new NoPermissionsException("You can only edit your own salary records");
}
var currentUser = await _auth.GetCurrentUserOrNullAsync(cancellationToken);
if (currentUser is null)
{
throw new NoPermissionsException("Authentication required");
}
if (!currentUser.Has(Role.Admin) &&
salary.UserId != currentUser.Id)
{
throw new NoPermissionsException("You can only edit your own salary records");
}
🤖 Prompt for AI Agents
In `@src/Web.Api/Features/Salaries/UpdateSalary/UpdateSalaryHandler.cs` around
lines 37 - 43, currentUser may be null when calling
_auth.GetCurrentUserOrNullAsync, so update the logic in UpdateSalaryHandler to
handle that: either replace GetCurrentUserOrNullAsync with a non-null-returning
method like GetCurrentUserAsync (so the intent is explicit) or add an explicit
null check after calling GetCurrentUserOrNullAsync and throw an appropriate
authentication exception (e.g., UnauthorizedAccessException or a custom
NotAuthenticatedException) before evaluating currentUser.Has(Role.Admin) or
accessing currentUser.Id; ensure the check occurs immediately after the call to
avoid a NullReferenceException.


Skill skill = null;
Expand Down
Loading