Skip to content

Commit bb33b4f

Browse files
committed
Refactor AuditSignInManager and enhance logging
Refactored the constructor of `AuditSignInManager<TUser>` for improved readability. Enhanced the `LogLoginAuditAsync` method to include `SignInResult? result` for detailed logging. Updated all method calls to pass the new parameter. Simplified client IP address retrieval by replacing legacy parsing logic with a streamlined approach. Added input sanitization and introduced a new method to extract browser information from the `User-Agent` header. Improved exception handling with better logging and ensured graceful handling of errors. Cleaned up unused code, removed redundant methods, and improved overall code formatting for maintainability.
1 parent b9504d1 commit bb33b4f

1 file changed

Lines changed: 48 additions & 87 deletions

File tree

src/Infrastructure/Services/Identity/AuditSignInManager.cs

Lines changed: 48 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -4,28 +4,27 @@
44
using CleanArchitecture.Blazor.Domain.Identity;
55
using Microsoft.AspNetCore.Authentication;
66
using Microsoft.AspNetCore.Http;
7-
using System.Net; // added for IPAddress parsing
87

98
namespace CleanArchitecture.Blazor.Infrastructure.Services;
109

1110
public class AuditSignInManager<TUser> : SignInManager<TUser>
12-
where TUser : class
11+
where TUser : class
1312
{
1413
private readonly IApplicationDbContextFactory _dbContextFactory;
1514
private readonly IHttpContextAccessor _httpContextAccessor;
1615
private readonly ILogger<AuditSignInManager<TUser>> _logger;
1716

1817
public AuditSignInManager(
19-
UserManager<TUser> userManager,
20-
IHttpContextAccessor contextAccessor,
21-
IUserClaimsPrincipalFactory<TUser> claimsFactory,
22-
IOptions<IdentityOptions> optionsAccessor,
23-
ILogger<SignInManager<TUser>> logger,
24-
IAuthenticationSchemeProvider schemes,
25-
IUserConfirmation<TUser> confirmation,
26-
IApplicationDbContextFactory dbContextFactory,
27-
ILogger<AuditSignInManager<TUser>> auditLogger)
28-
: base(userManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation)
18+
UserManager<TUser> userManager,
19+
IHttpContextAccessor contextAccessor,
20+
IUserClaimsPrincipalFactory<TUser> claimsFactory,
21+
IOptions<IdentityOptions> optionsAccessor,
22+
ILogger<SignInManager<TUser>> logger,
23+
IAuthenticationSchemeProvider schemes,
24+
IUserConfirmation<TUser> confirmation,
25+
IApplicationDbContextFactory dbContextFactory,
26+
ILogger<AuditSignInManager<TUser>> auditLogger)
27+
: base(userManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation)
2928
{
3029
_dbContextFactory = dbContextFactory;
3130
_httpContextAccessor = contextAccessor;
@@ -44,7 +43,7 @@ public override async Task<SignInResult> PasswordSignInAsync(TUser user, string
4443
var result = await base.PasswordSignInAsync(user, password, isPersistent, lockoutOnFailure);
4544
var userName = await UserManager.GetUserNameAsync(user) ?? "Unknown";
4645
var userId = await UserManager.GetUserIdAsync(user) ?? "Unknown";
47-
await LogLoginAuditAsync(userId, userName, result.Succeeded, "Local");
46+
await LogLoginAuditAsync(userId, userName, result.Succeeded, "Local", result);
4847
return result;
4948
}
5049

@@ -56,7 +55,7 @@ public override async Task<SignInResult> ExternalLoginSignInAsync(string loginPr
5655
var info = await GetExternalLoginInfoAsync();
5756
var userName = info?.Principal?.Identity?.Name ?? "External User";
5857
var userId = info?.Principal?.FindFirstValue(ClaimTypes.NameIdentifier) ?? "Unknown";
59-
await LogLoginAuditAsync(userId, userName, result.Succeeded, loginProvider);
58+
await LogLoginAuditAsync(userId, userName, result.Succeeded, loginProvider, result);
6059
return result;
6160
}
6261

@@ -65,15 +64,15 @@ public override async Task SignInAsync(TUser user, bool isPersistent, string? au
6564
await base.SignInAsync(user, isPersistent, authenticationMethod);
6665
var userName = await UserManager.GetUserNameAsync(user) ?? "Unknown";
6766
var userId = await UserManager.GetUserIdAsync(user) ?? "Unknown";
68-
await LogLoginAuditAsync(userId, userName, true, authenticationMethod ?? "Direct");
67+
await LogLoginAuditAsync(userId, userName, true, authenticationMethod ?? "Direct", null);
6968
}
7069

7170
public override async Task SignInAsync(TUser user, AuthenticationProperties authenticationProperties, string? authenticationMethod = null)
7271
{
7372
await base.SignInAsync(user, authenticationProperties, authenticationMethod);
7473
var userName = await UserManager.GetUserNameAsync(user) ?? "Unknown";
7574
var userId = await UserManager.GetUserIdAsync(user) ?? "Unknown";
76-
await LogLoginAuditAsync(userId, userName, true, authenticationMethod ?? "Direct");
75+
await LogLoginAuditAsync(userId, userName, true, authenticationMethod ?? "Direct", null);
7776
}
7877

7978
public override async Task<SignInResult> TwoFactorSignInAsync(string provider, string code, bool isPersistent, bool rememberClient)
@@ -90,11 +89,11 @@ public override async Task<SignInResult> TwoFactorSignInAsync(string provider, s
9089
userId = await UserManager.GetUserIdAsync(user) ?? "Unknown";
9190
}
9291

93-
await LogLoginAuditAsync(userId, userName, result.Succeeded, $"2FA-{provider}");
92+
await LogLoginAuditAsync(userId, userName, result.Succeeded, $"2FA-{provider}", result);
9493
return result;
9594
}
9695

97-
private async Task LogLoginAuditAsync(string userId, string userName, bool success, string provider)
96+
private async Task LogLoginAuditAsync(string userId, string userName, bool success, string provider, SignInResult? result)
9897
{
9998
try
10099
{
@@ -105,6 +104,8 @@ private async Task LogLoginAuditAsync(string userId, string userName, bool succe
105104
return;
106105
}
107106

107+
108+
108109
// Extract client information
109110
var ipAddress = GetClientIpAddress(httpContext);
110111
var browserInfo = GetBrowserInfo(httpContext);
@@ -137,88 +138,48 @@ private async Task LogLoginAuditAsync(string userId, string userName, bool succe
137138
{
138139
try
139140
{
140-
// Simple & safe: only examine a short list of common headers, validate each with IPAddress.TryParse.
141-
// Order: CF-Connecting-IP -> X-Forwarded-For (first) -> True-Client-IP -> X-Real-IP -> fallback RemoteIpAddress
142-
if (TryGetSingleHeaderIp(httpContext, "CF-Connecting-IP", out var ip)) return ip;
143-
if (TryGetXForwardedFor(httpContext, out ip)) return ip;
144-
if (TryGetSingleHeaderIp(httpContext, "True-Client-IP", out ip)) return ip;
145-
if (TryGetSingleHeaderIp(httpContext, "X-Real-IP", out ip)) return ip;
146-
147-
var remote = httpContext.Connection.RemoteIpAddress;
148-
return NormalizeLoopback(remote);
149-
}
150-
catch (Exception ex)
151-
{
152-
_logger.LogWarning(ex, "Failed to get client IP address");
153-
return null;
154-
}
155-
}
156-
157-
private bool TryGetSingleHeaderIp(HttpContext ctx, string headerName, out string? ip)
158-
{
159-
ip = null;
160-
var raw = ctx.Request.Headers[headerName].FirstOrDefault();
161-
if (string.IsNullOrWhiteSpace(raw)) return false;
162-
raw = raw.Split(',')[0].Trim(); // if multiple, only first
163-
raw = StripPortAndBrackets(raw);
164-
if (IPAddress.TryParse(raw, out var parsed))
165-
{
166-
ip = NormalizeLoopback(parsed);
167-
return true;
168-
}
169-
return false;
170-
}
171-
172-
private bool TryGetXForwardedFor(HttpContext ctx, out string? ip)
173-
{
174-
ip = null;
175-
var raw = ctx.Request.Headers["X-Forwarded-For"].FirstOrDefault();
176-
if (string.IsNullOrWhiteSpace(raw)) return false;
177-
// Split chain client, proxy1, proxy2 ... choose first non-empty candidate that parses
178-
foreach (var candidate in raw.Split(',').Select(s => s.Trim()))
179-
{
180-
if (string.IsNullOrEmpty(candidate)) continue;
181-
var cleaned = StripPortAndBrackets(candidate);
182-
if (IPAddress.TryParse(cleaned, out var parsed))
141+
// Check for forwarded IP first (when behind proxy/load balancer)
142+
var forwardedFor = httpContext.Request.Headers["X-Forwarded-For"].FirstOrDefault();
143+
if (!string.IsNullOrEmpty(forwardedFor))
183144
{
184-
ip = NormalizeLoopback(parsed);
185-
return true;
145+
// Take the first IP if there are multiple
146+
var firstIp = forwardedFor.Split(',').FirstOrDefault()?.Trim();
147+
if (!string.IsNullOrEmpty(firstIp))
148+
return firstIp;
186149
}
187-
}
188-
return false;
189-
}
190150

191-
private string StripPortAndBrackets(string value)
192-
{
193-
if (string.IsNullOrEmpty(value)) return value;
194-
value = value.Trim('"');
195-
// IPv6 in brackets: [2001:db8::1]:443
196-
if (value.StartsWith("[") && value.Contains("]"))
197-
{
198-
var end = value.IndexOf(']');
199-
if (end > 0)
151+
// Check for real IP header
152+
var realIp = httpContext.Request.Headers["X-Real-IP"].FirstOrDefault();
153+
if (!string.IsNullOrEmpty(realIp))
154+
return realIp;
155+
156+
// Fall back to remote IP
157+
var remoteIp = httpContext.Connection.RemoteIpAddress?.ToString();
158+
159+
// Handle localhost scenarios
160+
if (remoteIp == "::1" || remoteIp == "127.0.0.1")
200161
{
201-
var core = value.Substring(1, end - 1);
202-
// ignore trailing :port
203-
return core;
162+
return "127.0.0.1"; // Normalize localhost
204163
}
164+
165+
return SanitizeInput(remoteIp);
205166
}
206-
// IPv4:port
207-
var colonIndex = value.LastIndexOf(':');
208-
if (colonIndex > -1 && value.Count(c => c == ':') == 1 && value.Contains('.'))
167+
catch (Exception ex)
209168
{
210-
return value.Substring(0, colonIndex);
169+
_logger.LogWarning(ex, "Failed to get client IP address");
170+
return null;
211171
}
212-
return value;
213172
}
214173

215-
private string? NormalizeLoopback(IPAddress? ip)
174+
private string SanitizeInput(string? input)
216175
{
217-
if (ip == null) return null;
218-
if (IPAddress.IsLoopback(ip)) return "127.0.0.1"; // unify
219-
return ip.ToString();
176+
if (string.IsNullOrEmpty(input))
177+
return string.Empty;
178+
// Remove newline characters and trim whitespace
179+
return input.Replace("\r", "").Replace("\n", "").Trim();
220180
}
221181

182+
222183
private string? GetBrowserInfo(HttpContext httpContext)
223184
{
224185
try

0 commit comments

Comments
 (0)