Merge pull request #2625 from gterdem/pr/2277

Allow to write audit logs on exceptions even if disabled for GET requests

framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/Auditing/AbpAuditingMiddleware.cs

@@ -28,38 +28,49 @@ namespace Volo.Abp.AspNetCore.Auditing
 
         public async Task InvokeAsync(HttpContext context, RequestDelegate next)
         {
-            if (!ShouldWriteAuditLog(context))
+            bool hasError = false;
-            {
-                await next(context);
-                return;
-            }
-
             using (var scope = _auditingManager.BeginScope())
             {
                 try
                 {
                     await next(context);
                 }
+                catch (Exception)
+                {
+                    hasError = true;
+                    if (!Options.HideErrors)
+                    {
+                        throw;
+                    }
+                }
                 finally
                 {
-                    await scope.SaveAsync();
+                    if (ShouldWriteAuditLog(context, hasError))
+                    {
+                        await scope.SaveAsync();
+                    }
                 }
             }
         }
 
-        private bool ShouldWriteAuditLog(HttpContext httpContext)
+        private bool ShouldWriteAuditLog(HttpContext httpContext, bool hasError = false)
         {
             if (!Options.IsEnabled)
             {
                 return false;
             }
 
+            if (Options.AlwaysLogOnException && hasError)
+            {
+                return true;
+            }
+
             if (!Options.IsEnabledForAnonymousUsers && !CurrentUser.IsAuthenticated)
             {
                 return false;
             }
 
-            if (!Options.IsEnabledForGetRequests && 
+            if (!Options.IsEnabledForGetRequests &&
                 string.Equals(httpContext.Request.Method, HttpMethods.Get, StringComparison.OrdinalIgnoreCase))
             {
                 return false;

framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/AbpAuditingOptions.cs

@@ -30,6 +30,12 @@ namespace Volo.Abp.Auditing
         /// Default: true.
         /// </summary>
         public bool IsEnabledForAnonymousUsers { get; set; }
+
+        /// <summary>
+        /// Audit log on exceptions.
+        /// Default: true.
+        /// </summary>
+        public bool AlwaysLogOnException { get; set; }
 
         public List<AuditLogContributor> Contributors { get; }
 
@@ -48,6 +54,7 @@ namespace Volo.Abp.Auditing
             IsEnabled = true;
             IsEnabledForAnonymousUsers = true;
             HideErrors = true;
+            AlwaysLogOnException = true;
 
             Contributors = new List<AuditLogContributor>();
 

framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/AuditingManager.cs

@@ -23,8 +23,8 @@ namespace Volo.Abp.Auditing
         private readonly IAuditingStore _auditingStore;
 
         public AuditingManager(
-            IAmbientScopeProvider<IAuditLogScope> ambientScopeProvider, 
+            IAmbientScopeProvider<IAuditLogScope> ambientScopeProvider,
-            IAuditingHelper auditingHelper, 
+            IAuditingHelper auditingHelper,
             IAuditingStore auditingStore,
             IServiceProvider serviceProvider,
             IOptions<AbpAuditingOptions> options)
@@ -84,7 +84,7 @@ namespace Volo.Abp.Auditing
         {
             var changeGroups = auditLog.EntityChanges
                 .Where(e => e.ChangeType == EntityChangeType.Updated)
-                .GroupBy(e => new {e.EntityTypeFullName, e.EntityId})
+                .GroupBy(e => new { e.EntityTypeFullName, e.EntityId })
                 .ToList();
 
             foreach (var changeGroup in changeGroups)
@@ -141,7 +141,7 @@ namespace Volo.Abp.Auditing
             public DisposableSaveHandle(
                 AuditingManager auditingManager,
                 IDisposable scope,
-                AuditLogInfo auditLog, 
+                AuditLogInfo auditLog,
                 Stopwatch stopWatch)
             {
                 _auditingManager = auditingManager;
@@ -154,6 +154,11 @@ namespace Volo.Abp.Auditing
             {
                 await _auditingManager.SaveAsync(this);
             }
+            
+            public void AddException(Exception exception)
+            {
+                this.AuditLog.Exceptions.Add(exception);
+            }
 
             public void Dispose()
             {

framework/src/Volo.Abp.Auditing/Volo/Abp/Auditing/IAuditLogSaveHandle.cs

@@ -6,5 +6,6 @@ namespace Volo.Abp.Auditing
     public interface IAuditLogSaveHandle : IDisposable
     {
         Task SaveAsync();
+        void AddException(Exception exception);
     }
 }

framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Auditing/AuditTestController.cs

@@ -0,0 +1,29 @@
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.Extensions.Options;
+using Volo.Abp.Auditing;
+
+namespace Volo.Abp.AspNetCore.Mvc.Auditing
+{
+    [Route("api/audit-test")]
+    public class AuditTestController : AbpController
+    {
+        private readonly AbpAuditingOptions _options;
+
+        public AuditTestController(IOptions<AbpAuditingOptions> options)
+        {
+            _options = options.Value;
+        }
+
+        [Route("audit-success")]
+        public IActionResult AuditSuccessForGetRequests()
+        {
+            return Ok();
+        }
+
+        [Route("audit-fail")]
+        public IActionResult AuditFailForGetRequests()
+        {
+            throw new UserFriendlyException("Exception occurred!");
+        }
+    }
+}

framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/Auditing/AuditTestController_Tests.cs

@@ -0,0 +1,48 @@
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Options;
+using NSubstitute;
+using System.Threading.Tasks;
+using Volo.Abp.Auditing;
+using Xunit;
+
+namespace Volo.Abp.AspNetCore.Mvc.Auditing
+{
+    public class AuditTestController_Tests : AspNetCoreMvcTestBase
+    {
+        private readonly AbpAuditingOptions _options;
+        private IAuditingStore _auditingStore;
+
+        public AuditTestController_Tests()
+        {
+            _options = ServiceProvider.GetRequiredService<IOptions<AbpAuditingOptions>>().Value;
+            _auditingStore = ServiceProvider.GetRequiredService<IAuditingStore>();
+        }
+
+        protected override void ConfigureServices(HostBuilderContext context, IServiceCollection services)
+        {
+            _auditingStore = Substitute.For<IAuditingStore>();
+            services.Replace(ServiceDescriptor.Singleton(_auditingStore));
+            base.ConfigureServices(context, services);
+        }
+
+        [Fact]
+        public async Task Should_Trigger_Middleware_And_AuditLog_Success_For_GetRequests()
+        {
+            _options.IsEnabledForGetRequests = true;
+            _options.AlwaysLogOnException = false;
+            await GetResponseAsync("api/audit-test/audit-success");
+            //await _auditingStore.Received().SaveAsync(Arg.Any<AuditLogInfo>()); //Won't work, save happens out of scope
+        }
+
+        [Fact]
+        public async Task Should_Trigger_Middleware_And_AuditLog_Exception_Always()
+        {
+            _options.IsEnabled = true;
+            _options.AlwaysLogOnException = true;
+            await GetResponseAsync("api/audit-test/audit-fail", System.Net.HttpStatusCode.BadRequest);
+            //await _auditingStore.Received().SaveAsync(Arg.Any<AuditLogInfo>()); //Won't work, save happens out of scope
+        }
+    }
+}