RavenDB-22362 address PR comments

karmeli87 · May 15, 2024 · c0581c6 · c0581c6
1 parent efc2066
commit c0581c6
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 47 deletions.
diff --git a/src/Raven.Server/Documents/Handlers/Admin/AdminAnalyzersHandler.cs b/src/Raven.Server/Documents/Handlers/Admin/AdminAnalyzersHandler.cs
@@ -29,7 +29,7 @@ public async Task Put()
 
                     if (LoggingSource.AuditLog.IsInfoEnabled)
                     {
-                        LogAuditFor(Database.Name, $"Analyzer {analyzerDefinition.Name} PUT with definition: {analyzerToAdd}");
+                        LogAuditFor(Database.Name, $"Analyzer '{analyzerDefinition.Name}' PUT with definition: {analyzerToAdd}");
                     }
 
                     analyzerDefinition.Validate();
@@ -55,7 +55,7 @@ public async Task Delete()
 
             if (LoggingSource.AuditLog.IsInfoEnabled)
             {
-                LogAuditFor(Database.Name, $"Analyzer {name} DELETE");
+                LogAuditFor(Database.Name, $"Analyzer '{name}' DELETE");
             }
 
             var command = new DeleteAnalyzerCommand(name, Database.Name, GetRaftRequestIdFromQuery());

diff --git a/src/Raven.Server/Web/Authentication/AdminCertificatesHandler.cs b/src/Raven.Server/Web/Authentication/AdminCertificatesHandler.cs
@@ -231,7 +231,7 @@ public async Task Put()
                         : string.Empty;
 
                     LogAuditFor("Certificates",
-                        $"Add new certificate '{certificate?.Thumbprint}'. Security Clearance: {certificate?.SecurityClearance}. Permissions:{permissions}.");
+                        $"Add new certificate {certificate?.Name} ['{certificate?.Thumbprint}']. Security Clearance: {certificate?.SecurityClearance}. Permissions:{permissions}.");
                 }
 
                 try

diff --git a/src/Raven.Server/Web/Authentication/TwoFactorAuthenticationHandler.cs b/src/Raven.Server/Web/Authentication/TwoFactorAuthenticationHandler.cs
@@ -94,7 +94,7 @@ public async Task ValidateTotp()
 
             if (LoggingSource.AuditLog.IsInfoEnabled)
             {
-                LogAuditFor(nameof(TwoFactorAuthenticationHandler), $"successfully authenticated with two factor auth for {period}. Has limits: {hasLimits}");
+                LogAuditFor(nameof(TwoFactorAuthenticationHandler), $"successfully authenticated with two factor auth for {period} (until: {DateTime.UtcNow.Add(period)}). Has limits: {hasLimits}");
             }
 
             string expectedCookieValue = null;
@@ -139,7 +139,7 @@ private async Task ReplyWith(TransactionOperationContext ctx, string err, HttpSt
     {
         if (LoggingSource.AuditLog.IsInfoEnabled)
         {
-            LogAuditFor(nameof(TwoFactorAuthenticationHandler), $"Two factor auth failure, because: {err}");
+            LogAuditFor(nameof(TwoFactorAuthenticationHandler), $"Two factor auth failure: {err}");
         }
 
         HttpContext.Response.StatusCode = (int)httpStatusCode;

diff --git a/src/Raven.Server/Web/RequestHandler.Audit.cs b/src/Raven.Server/Web/RequestHandler.Audit.cs
@@ -0,0 +1,53 @@
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+using System.Net;
+using System.Text;
+using System.Threading.Tasks;
+using Sparrow.Logging;
+
+namespace Raven.Server.Web
+{
+    public abstract partial class RequestHandler
+    {
+        public bool IsLocalRequest()
+        {
+            if (HttpContext.Connection.RemoteIpAddress == null && HttpContext.Connection.LocalIpAddress == null)
+            {
+                return true;
+            }
+            if (HttpContext.Connection.RemoteIpAddress.Equals(HttpContext.Connection.LocalIpAddress))
+            {
+                return true;
+            }
+            if (IPAddress.IsLoopback(HttpContext.Connection.RemoteIpAddress))
+            {
+                return true;
+            }
+            return false;
+        }
+
+        public string RequestIp => IsLocalRequest() ? Environment.MachineName : HttpContext.Connection.RemoteIpAddress.ToString();
+
+        public void LogAuditFor(string logger, string message)
+        {
+            var auditLog = LoggingSource.AuditLog.GetLogger(logger, "Audit");
+            Debug.Assert(auditLog.IsInfoEnabled, $"auditlog info is disabled");
+
+            var clientCert = GetCurrentCertificate();
+
+            var sb = new StringBuilder();
+            sb.Append(RequestIp);
+            sb.Append(", ");
+            if (clientCert != null) 
+                sb.Append($"CN={clientCert.Subject} [{clientCert.Thumbprint}], ");
+            else
+                sb.Append("no certificate, ");
+
+            sb.Append(message);
+
+            auditLog.Info(sb.ToString());
+        }
+    }
+}
diff --git a/src/Raven.Server/Web/RequestHandler.cs b/src/Raven.Server/Web/RequestHandler.cs
@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Globalization;
 using System.IO;
 using System.IO.Compression;
@@ -26,11 +25,10 @@
 using Raven.Server.ServerWide.Context;
 using Raven.Server.TrafficWatch;
 using Sparrow;
-using Sparrow.Logging;
 
 namespace Raven.Server.Web
 {
-    public abstract class RequestHandler
+    public abstract partial class RequestHandler
     {
         public const string StartParameter = "start";
 
@@ -764,44 +762,5 @@ public void AddStringToHttpContext(string str, TrafficWatchChangeType type)
         {
             HttpContext.Items["TrafficWatch"] = (str, type);
         }
-
-        public bool IsLocalRequest()
-        {
-            if (HttpContext.Connection.RemoteIpAddress == null && HttpContext.Connection.LocalIpAddress == null)
-            {
-                return true;
-            }
-            if (HttpContext.Connection.RemoteIpAddress.Equals(HttpContext.Connection.LocalIpAddress))
-            {
-                return true;
-            }
-            if (IPAddress.IsLoopback(HttpContext.Connection.RemoteIpAddress))
-            {
-                return true;
-            }
-            return false;
-        }
-
-        public string RequestIp => IsLocalRequest() ? Environment.MachineName : HttpContext.Connection.RemoteIpAddress.ToString();
-
-        public void LogAuditFor(string logger, string message)
-        {
-            var auditLog = LoggingSource.AuditLog.GetLogger(logger, "Audit");
-            Debug.Assert(auditLog.IsInfoEnabled, $"auditlog info is disabled");
-
-            var clientCert = GetCurrentCertificate();
-
-            var sb = new StringBuilder();
-            sb.Append(RequestIp);
-            sb.Append(", ");
-            if (clientCert != null) 
-                sb.Append($"CN={clientCert.Subject} [{clientCert.Thumbprint}], ");
-            else
-                sb.Append("no certificate, ");
-
-            sb.Append(message);
-
-            auditLog.Info(sb.ToString());
-        }
     }
 }