Fixed some issues around creating webhooks and improved webhook validation that were introduced with the .NET 8 upgrade

niemyjski · niemyjski · commit 4649c65da49a · 2023-10-20T08:15:16.000-05:00
diff --git a/src/Exceptionless.Core/Models/WebHook.cs b/src/Exceptionless.Core/Models/WebHook.cs
@@ -24,4 +24,20 @@ public static class KnownVersions
         public const string Version1 = "v1";
         public const string Version2 = "v2";
     }
+
+    public static readonly string[] AllKnownEventTypes = new[]
+    {
+        KnownEventTypes.NewError, KnownEventTypes.CriticalError, KnownEventTypes.NewEvent,
+        KnownEventTypes.CriticalEvent, KnownEventTypes.StackRegression, KnownEventTypes.StackPromoted
+    };
+
+    public static class KnownEventTypes
+    {
+        public const string NewError = "NewError";
+        public const string CriticalError = "CriticalError";
+        public const string NewEvent = "NewEvent";
+        public const string CriticalEvent = "CriticalEvent";
+        public const string StackRegression = "StackRegression";
+        public const string StackPromoted = "StackPromoted";
+    }
 }
diff --git a/src/Exceptionless.Core/Pipeline/070_QueueNotificationAction.cs b/src/Exceptionless.Core/Pipeline/070_QueueNotificationAction.cs
@@ -80,19 +80,19 @@ private bool ShouldCallWebHook(WebHook hook, EventContext ctx)
         if (!String.IsNullOrEmpty(hook.ProjectId) && !String.Equals(ctx.Project.Id, hook.ProjectId))
             return false;
 
-        if (ctx.IsNew && ctx.Event.IsError() && hook.EventTypes.Contains(WebHookRepository.EventTypes.NewError))
+        if (ctx.IsNew && ctx.Event.IsError() && hook.EventTypes.Contains(WebHook.KnownEventTypes.NewError))
             return true;
 
-        if (ctx.Event.IsCritical() && ctx.Event.IsError() && hook.EventTypes.Contains(WebHookRepository.EventTypes.CriticalError))
+        if (ctx.Event.IsCritical() && ctx.Event.IsError() && hook.EventTypes.Contains(WebHook.KnownEventTypes.CriticalError))
             return true;
 
-        if (ctx.IsRegression && hook.EventTypes.Contains(WebHookRepository.EventTypes.StackRegression))
+        if (ctx.IsRegression && hook.EventTypes.Contains(WebHook.KnownEventTypes.StackRegression))
             return true;
 
-        if (ctx.IsNew && hook.EventTypes.Contains(WebHookRepository.EventTypes.NewEvent))
+        if (ctx.IsNew && hook.EventTypes.Contains(WebHook.KnownEventTypes.NewEvent))
             return true;
 
-        if (ctx.Event.IsCritical() && hook.EventTypes.Contains(WebHookRepository.EventTypes.CriticalEvent))
+        if (ctx.Event.IsCritical() && hook.EventTypes.Contains(WebHook.KnownEventTypes.CriticalEvent))
             return true;
 
         return false;
diff --git a/src/Exceptionless.Core/Repositories/WebHookRepository.cs b/src/Exceptionless.Core/Repositories/WebHookRepository.cs
@@ -43,16 +43,6 @@ public async Task MarkDisabledAsync(string id)
         await SaveAsync(webHook, o => o.Cache()).AnyContext();
     }
 
-    public static class EventTypes
-    {
-        // TODO: Add support for these new web hook types.
-        public const string NewError = "NewError";
-        public const string CriticalError = "CriticalError";
-        public const string NewEvent = "NewEvent";
-        public const string CriticalEvent = "CriticalEvent";
-        public const string StackRegression = "StackRegression";
-        public const string StackPromoted = "StackPromoted";
-    }
 
     protected override async Task InvalidateCacheAsync(IReadOnlyCollection<ModifiedDocument<WebHook>> documents, ChangeType? changeType = null)
     {
diff --git a/src/Exceptionless.Core/Validation/WebHookValidator.cs b/src/Exceptionless.Core/Validation/WebHookValidator.cs
@@ -12,6 +12,7 @@ public WebHookValidator()
         RuleFor(w => w.ProjectId).IsObjectId().When(p => String.IsNullOrEmpty(p.OrganizationId)).WithMessage("Please specify a valid project id.");
         RuleFor(w => w.Url).NotEmpty().WithMessage("Please specify a valid url.");
         RuleFor(w => w.EventTypes).NotEmpty().WithMessage("Please specify one or more event types.");
+        RuleForEach(w => w.EventTypes).Must(et => WebHook.AllKnownEventTypes.Contains(et)).WithMessage("Please specify a valid event type.");
         RuleFor(w => w.Version).NotEmpty().WithMessage("Please specify a valid version.");
     }
 }
diff --git a/src/Exceptionless.Web/ClientApp.angular/app/project/manage/manage-controller.js b/src/Exceptionless.Web/ClientApp.angular/app/project/manage/manage-controller.js
@@ -84,6 +84,7 @@
                     return dialogs
                         .create("components/web-hook/add-web-hook-dialog.tpl.html", "AddWebHookDialog as vm")
                         .result.then(function (data) {
+                            data.organization_id = vm.project.organization_id;
                             data.project_id = vm._projectId;
                             return createWebHook(data);
                         })
diff --git a/src/Exceptionless.Web/Controllers/StackController.cs b/src/Exceptionless.Web/Controllers/StackController.cs
@@ -392,7 +392,7 @@ public async Task<IActionResult> PromoteAsync(string id)
         if (!await _billingManager.HasPremiumFeaturesAsync(stack.OrganizationId))
             return PlanLimitReached("Promote to External is a premium feature used to promote an error stack to an external system. Please upgrade your plan to enable this feature.");
 
-        var promotedProjectHooks = (await _webHookRepository.GetByProjectIdAsync(stack.ProjectId)).Documents.Where(p => p.EventTypes.Contains(WebHookRepository.EventTypes.StackPromoted)).ToList();
+        var promotedProjectHooks = (await _webHookRepository.GetByProjectIdAsync(stack.ProjectId)).Documents.Where(p => p.EventTypes.Contains(WebHook.KnownEventTypes.StackPromoted)).ToList();
         if (!promotedProjectHooks.Any())
             return NotImplemented("No promoted web hooks are configured for this project. Please add a promoted web hook to use this feature.");
 
diff --git a/src/Exceptionless.Web/Controllers/WebHookController.cs b/src/Exceptionless.Web/Controllers/WebHookController.cs
@@ -17,7 +17,7 @@ namespace Exceptionless.App.Controllers.API;
 
 [Route(API_PREFIX + "/webhooks")]
 [Authorize(Policy = AuthorizationRoles.ClientPolicy)]
-public class WebHookController : RepositoryApiController<IWebHookRepository, WebHook, WebHook, NewWebHook, UpdateWebHook>
+public class WebHookController : RepositoryApiController<IWebHookRepository, WebHook, WebHook, NewWebHook, WebHook>
 {
     private readonly IProjectRepository _projectRepository;
     private readonly BillingManager _billingManager;
diff --git a/src/Exceptionless.Web/Models/WebHook/NewWebHook.cs b/src/Exceptionless.Web/Models/WebHook/NewWebHook.cs
@@ -12,5 +12,5 @@ public record NewWebHook : IOwnedByOrganizationAndProject
     /// <summary>
     /// The schema version that should be used.
     /// </summary>
-    public Version Version { get; set; } = null!;
+    public Version? Version { get; set; }
 }
diff --git a/src/Exceptionless.Web/Models/WebHook/UpdateWebHook.cs b/src/Exceptionless.Web/Models/WebHook/UpdateWebHook.cs
diff --git a/src/Exceptionless.Web/Utility/AutoValidationActionFilter.cs b/src/Exceptionless.Web/Utility/AutoValidationActionFilter.cs
@@ -1,3 +1,4 @@
+using System.Dynamic;
 using System.IO.Pipelines;
 using System.Security.Claims;
 using Exceptionless.Core.Extensions;
@@ -32,7 +33,7 @@ public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionE
                     continue;
 
                 // We don't support validating JSON Types
-                if (subject is Newtonsoft.Json.Linq.JToken)
+                if (subject is Newtonsoft.Json.Linq.JToken or DynamicObject)
                     continue;
 
                 (bool isValid, var errors) = await MiniValidator.TryValidateAsync(subject, _serviceProvider, recurse: true);
@@ -55,28 +56,28 @@ public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionE
 
             if (hasErrors)
             {
-                var validationProblem = controllerBase.ProblemDetailsFactory.CreateValidationProblemDetails(context.HttpContext, context.ModelState, 422);
-                context.Result = new UnprocessableEntityObjectResult(validationProblem);
+            var validationProblem = controllerBase.ProblemDetailsFactory.CreateValidationProblemDetails(context.HttpContext, context.ModelState, 422);
+            context.Result = new UnprocessableEntityObjectResult(validationProblem);
 
-                return;
-            }
+            return;
         }
-
-        await next();
     }
 
-    private static bool ShouldValidate(Type type, IServiceProviderIsService? isService = null) =>
-        !IsNonValidatedType(type, isService) && MiniValidator.RequiresValidation(type);
+    await next();
+}
+
+private static bool ShouldValidate(Type type, IServiceProviderIsService? isService = null) =>
+    !IsNonValidatedType(type, isService) && MiniValidator.RequiresValidation(type);
 
-    private static bool IsNonValidatedType(Type type, IServiceProviderIsService? isService) =>
-        typeof(HttpContext) == type
-        || typeof(HttpRequest) == type
-        || typeof(HttpResponse) == type
-        || typeof(ClaimsPrincipal) == type
-        || typeof(CancellationToken) == type
-        || typeof(IFormFileCollection) == type
-        || typeof(IFormFile) == type
-        || typeof(Stream) == type
-        || typeof(PipeReader) == type
-        || isService?.IsService(type) == true;
+private static bool IsNonValidatedType(Type type, IServiceProviderIsService? isService) =>
+    typeof(HttpContext) == type
+    || typeof(HttpRequest) == type
+    || typeof(HttpResponse) == type
+    || typeof(ClaimsPrincipal) == type
+    || typeof(CancellationToken) == type
+    || typeof(IFormFileCollection) == type
+    || typeof(IFormFile) == type
+    || typeof(Stream) == type
+    || typeof(PipeReader) == type
+    || isService?.IsService(type) == true;
 }
diff --git a/tests/Exceptionless.Tests/Controllers/ProjectControllerTests.cs b/tests/Exceptionless.Tests/Controllers/ProjectControllerTests.cs
@@ -24,6 +24,66 @@ protected override async Task ResetDataAsync()
         await service.CreateDataAsync();
     }
 
+    [Fact]
+    public async Task CanUpdateProject()
+    {
+        var project = await SendRequestAsAsync<ViewProject>(r => r
+            .AsTestOrganizationUser()
+            .Post()
+            .AppendPath("projects")
+            .Content(new NewProject
+            {
+                OrganizationId = SampleDataService.TEST_ORG_ID,
+                Name = "Test Project",
+                DeleteBotDataEnabled = true
+            })
+            .StatusCodeShouldBeCreated()
+        );
+
+        var updatedProject = await SendRequestAsAsync<ViewProject>(r => r
+            .AsTestOrganizationUser()
+            .Patch()
+            .AppendPath("projects", project.Id)
+            .Content(new UpdateProject
+            {
+                Name = "Test Project 2",
+                DeleteBotDataEnabled = true
+            })
+            .StatusCodeShouldBeOk()
+        );
+
+        Assert.NotEqual(project.Name, updatedProject.Name);
+    }
+
+
+    [Fact]
+    public async Task CanUpdateProjectWithExtraPayloadProperties()
+    {
+        var project = await SendRequestAsAsync<ViewProject>(r => r
+            .AsTestOrganizationUser()
+            .Post()
+            .AppendPath("projects")
+            .Content(new NewProject
+            {
+                OrganizationId = SampleDataService.TEST_ORG_ID,
+                Name = "Test Project",
+                DeleteBotDataEnabled = true
+            })
+            .StatusCodeShouldBeCreated()
+        );
+
+        project.Name = "Updated";
+        var updatedProject = await SendRequestAsAsync<ViewProject>(r => r
+            .AsTestOrganizationUser()
+            .Patch()
+            .AppendPath("projects", project.Id)
+            .Content(project)
+            .StatusCodeShouldBeOk()
+        );
+
+        Assert.Equal("Updated", updatedProject.Name);
+    }
+
     [Fact]
     public async Task CanGetProjectConfiguration()
     {
diff --git a/tests/Exceptionless.Tests/Controllers/WebHookControllerTests.cs b/tests/Exceptionless.Tests/Controllers/WebHookControllerTests.cs
@@ -0,0 +1,56 @@
+using Exceptionless.Core.Models;
+using Exceptionless.Core.Utility;
+using Exceptionless.Tests.Extensions;
+using Exceptionless.Web.Models;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Exceptionless.Tests.Controllers;
+
+public sealed class WebHookControllerTests : IntegrationTestsBase
+{
+    public WebHookControllerTests(ITestOutputHelper output, AppWebHostFactory factory) : base(output, factory) { }
+
+    protected override async Task ResetDataAsync()
+    {
+        await base.ResetDataAsync();
+        var service = GetService<SampleDataService>();
+        await service.CreateDataAsync();
+    }
+
+    [Fact]
+    public Task CanCreateNewWebHook()
+    {
+        return SendRequestAsync(r => r
+           .Post()
+           .AsTestOrganizationUser()
+           .AppendPath("webhooks")
+           .Content(new NewWebHook
+           {
+               EventTypes = new[] { WebHook.KnownEventTypes.StackPromoted },
+               OrganizationId = SampleDataService.TEST_ORG_ID,
+               ProjectId = SampleDataService.TEST_PROJECT_ID,
+               Url = "https://localhost/test"
+           })
+           .StatusCodeShouldBeCreated()
+        );
+    }
+
+    [Fact]
+    public Task CreateNewWebHookWithInvalidEventTypeFails()
+    {
+        return SendRequestAsync(r => r
+            .Post()
+            .AsTestOrganizationUser()
+            .AppendPath("webhooks")
+            .Content(new NewWebHook
+            {
+                EventTypes = new[] { "Invalid" },
+                OrganizationId = SampleDataService.TEST_ORG_ID,
+                ProjectId = SampleDataService.TEST_PROJECT_ID,
+                Url = "https://localhost/test"
+            })
+            .StatusCodeShouldBeBadRequest()
+        );
+    }
+}
diff --git a/tests/Exceptionless.Tests/Plugins/WebHookDataTests.cs b/tests/Exceptionless.Tests/Plugins/WebHookDataTests.cs
@@ -82,7 +82,7 @@ private WebHookDataContext GetWebHookDataContext(string version)
             OrganizationId = TestConstants.OrganizationId,
             ProjectId = TestConstants.ProjectId,
             Url = "http://localhost:40000/test",
-            EventTypes = new[] { WebHookRepository.EventTypes.StackPromoted },
+            EventTypes = new[] { WebHook.KnownEventTypes.StackPromoted },
             Version = version,
             CreatedUtc = SystemClock.UtcNow
         };
diff --git a/tests/Exceptionless.Tests/Repositories/WebHookRepositoryTests.cs b/tests/Exceptionless.Tests/Repositories/WebHookRepositoryTests.cs
@@ -18,9 +18,9 @@ public WebHookRepositoryTests(ITestOutputHelper output, AppWebHostFactory factor
     [Fact]
     public async Task GetByOrganizationIdOrProjectIdAsync()
     {
-        await _repository.AddAsync(new WebHook { OrganizationId = TestConstants.OrganizationId, Url = "http://localhost:40000/test", EventTypes = new[] { WebHookRepository.EventTypes.StackPromoted }, Version = WebHook.KnownVersions.Version2 });
-        await _repository.AddAsync(new WebHook { OrganizationId = TestConstants.OrganizationId, ProjectId = TestConstants.ProjectId, Url = "http://localhost:40000/test1", EventTypes = new[] { WebHookRepository.EventTypes.StackPromoted }, Version = WebHook.KnownVersions.Version2 });
-        await _repository.AddAsync(new WebHook { OrganizationId = TestConstants.OrganizationId, ProjectId = TestConstants.ProjectIdWithNoRoles, Url = "http://localhost:40000/test1", EventTypes = new[] { WebHookRepository.EventTypes.StackPromoted }, Version = WebHook.KnownVersions.Version2 });
+        await _repository.AddAsync(new WebHook { OrganizationId = TestConstants.OrganizationId, Url = "http://localhost:40000/test", EventTypes = new[] { WebHook.KnownEventTypes.StackPromoted }, Version = WebHook.KnownVersions.Version2 });
+        await _repository.AddAsync(new WebHook { OrganizationId = TestConstants.OrganizationId, ProjectId = TestConstants.ProjectId, Url = "http://localhost:40000/test1", EventTypes = new[] { WebHook.KnownEventTypes.StackPromoted }, Version = WebHook.KnownVersions.Version2 });
+        await _repository.AddAsync(new WebHook { OrganizationId = TestConstants.OrganizationId, ProjectId = TestConstants.ProjectIdWithNoRoles, Url = "http://localhost:40000/test1", EventTypes = new[] { WebHook.KnownEventTypes.StackPromoted }, Version = WebHook.KnownVersions.Version2 });
 
         await RefreshDataAsync();
         Assert.Equal(3, (await _repository.GetByOrganizationIdAsync(TestConstants.OrganizationId)).Total);
@@ -32,8 +32,8 @@ public async Task GetByOrganizationIdOrProjectIdAsync()
     [Fact]
     public async Task CanSaveWebHookVersionAsync()
     {
-        await _repository.AddAsync(new WebHook { OrganizationId = TestConstants.OrganizationId, ProjectId = TestConstants.ProjectId, Url = "http://localhost:40000/test", EventTypes = new[] { WebHookRepository.EventTypes.StackPromoted }, Version = WebHook.KnownVersions.Version1 });
-        await _repository.AddAsync(new WebHook { OrganizationId = TestConstants.OrganizationId, ProjectId = TestConstants.ProjectIdWithNoRoles, Url = "http://localhost:40000/test1", EventTypes = new[] { WebHookRepository.EventTypes.StackPromoted }, Version = WebHook.KnownVersions.Version2 });
+        await _repository.AddAsync(new WebHook { OrganizationId = TestConstants.OrganizationId, ProjectId = TestConstants.ProjectId, Url = "http://localhost:40000/test", EventTypes = new[] { WebHook.KnownEventTypes.StackPromoted }, Version = WebHook.KnownVersions.Version1 });
+        await _repository.AddAsync(new WebHook { OrganizationId = TestConstants.OrganizationId, ProjectId = TestConstants.ProjectIdWithNoRoles, Url = "http://localhost:40000/test1", EventTypes = new[] { WebHook.KnownEventTypes.StackPromoted }, Version = WebHook.KnownVersions.Version2 });
 
         await RefreshDataAsync();
         Assert.Equal(WebHook.KnownVersions.Version1, (await _repository.GetByProjectIdAsync(TestConstants.ProjectId)).Documents.First().Version);

Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,7 @@ public WebHookValidator()`
`12`	`12`	`RuleFor(w => w.ProjectId).IsObjectId().When(p => String.IsNullOrEmpty(p.OrganizationId)).WithMessage("Please specify a valid project id.");`
`13`	`13`	`RuleFor(w => w.Url).NotEmpty().WithMessage("Please specify a valid url.");`
`14`	`14`	`RuleFor(w => w.EventTypes).NotEmpty().WithMessage("Please specify one or more event types.");`
	`15`	`+ RuleForEach(w => w.EventTypes).Must(et => WebHook.AllKnownEventTypes.Contains(et)).WithMessage("Please specify a valid event type.");`
`15`	`16`	`RuleFor(w => w.Version).NotEmpty().WithMessage("Please specify a valid version.");`
`16`	`17`	`}`
`17`	`18`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ namespace Exceptionless.App.Controllers.API;`
`17`	`17`
`18`	`18`	`[Route(API_PREFIX + "/webhooks")]`
`19`	`19`	`[Authorize(Policy = AuthorizationRoles.ClientPolicy)]`
`20`		`-public class WebHookController : RepositoryApiController<IWebHookRepository, WebHook, WebHook, NewWebHook, UpdateWebHook>`
	`20`	`+public class WebHookController : RepositoryApiController<IWebHookRepository, WebHook, WebHook, NewWebHook, WebHook>`
`21`	`21`	`{`
`22`	`22`	`private readonly IProjectRepository _projectRepository;`
`23`	`23`	`private readonly BillingManager _billingManager;`
Original file line number	Diff line number	Diff line change
`@@ -12,5 +12,5 @@ public record NewWebHook : IOwnedByOrganizationAndProject`
`12`	`12`	`/// <summary>`
`13`	`13`	`/// The schema version that should be used.`
`14`	`14`	`/// </summary>`
`15`		`- public Version Version { get; set; } = null!;`
	`15`	`+ public Version? Version { get; set; }`
`16`	`16`	`}`