diff --git a/Shared/Dependencies.cs b/Shared/Dependencies.cs index c1ee834..f4b5867 100644 --- a/Shared/Dependencies.cs +++ b/Shared/Dependencies.cs @@ -9,6 +9,7 @@ public static void Register(IServiceCollection services) { services.AddSingleton(typeof (IResponseFactory), typeof (ResponseFactory)); services.AddSingleton(typeof (IModelStateConverter), typeof (ModelStateConverter)); services.AddSingleton(typeof (IJsonSerialiser), typeof (JsonSerialiser)); + services.AddSingleton(typeof (IRequestValidator), typeof (RequestValidator)); services.AddSingleton(typeof (IEnvironment), typeof (Environment)); services.AddSingleton(typeof (IHttpClient), typeof (HttpClient)); } diff --git a/Shared/Environment.cs b/Shared/Environment.cs index d1f0f51..8161d7f 100644 --- a/Shared/Environment.cs +++ b/Shared/Environment.cs @@ -2,5 +2,6 @@ namespace GitHubAutoresponder.Shared { public class Environment : IEnvironment { public string EncodededCredentials => System.Environment.GetEnvironmentVariable("GHAR_CREDENTIALS"); + public string Secret => System.Environment.GetEnvironmentVariable("GHAR_SECRET"); } } diff --git a/Shared/IEnvironment.cs b/Shared/IEnvironment.cs index dc743c9..fb8cd1f 100644 --- a/Shared/IEnvironment.cs +++ b/Shared/IEnvironment.cs @@ -1,5 +1,6 @@ namespace GitHubAutoresponder.Shared { public interface IEnvironment { string EncodededCredentials { get; } + string Secret { get; } } } diff --git a/Responder/IJsonSerialiser.cs b/Shared/IJsonSerialiser.cs similarity index 56% rename from Responder/IJsonSerialiser.cs rename to Shared/IJsonSerialiser.cs index 3d53889..431014b 100644 --- a/Responder/IJsonSerialiser.cs +++ b/Shared/IJsonSerialiser.cs @@ -1,7 +1,8 @@ using System.IO; -namespace GitHubAutoresponder.Responder { +namespace GitHubAutoresponder.Shared { public interface IJsonSerialiser { string Serialise(object obj); + T Deserialise(string json); } } diff --git a/Responder/JsonSerialiser.cs b/Shared/JsonSerialiser.cs similarity index 73% rename from Responder/JsonSerialiser.cs rename to Shared/JsonSerialiser.cs index 0ac24a8..64ffbf9 100644 --- a/Responder/JsonSerialiser.cs +++ b/Shared/JsonSerialiser.cs @@ -1,9 +1,8 @@ using System; using System.IO; -using GitHubAutoresponder.Shared; using Newtonsoft.Json; -namespace GitHubAutoresponder.Responder { +namespace GitHubAutoresponder.Shared { public class JsonSerialiser : IJsonSerialiser { private JsonSerializerSettings settings; @@ -16,5 +15,9 @@ public JsonSerialiser() { public string Serialise(object obj) { return JsonConvert.SerializeObject(obj, this.settings); } + + public T Deserialise(string json) { + return JsonConvert.DeserializeObject(json, this.settings); + } } } diff --git a/Webhook/Tests/WebhookControllerTests.cs b/Webhook/Tests/WebhookControllerTests.cs index 0c97c98..aa8c0b2 100644 --- a/Webhook/Tests/WebhookControllerTests.cs +++ b/Webhook/Tests/WebhookControllerTests.cs @@ -10,20 +10,30 @@ using Microsoft.AspNetCore.Mvc.Controllers; using System.Net; using Microsoft.AspNetCore.Mvc.ModelBinding; +using GitHubAutoresponder.Shared; namespace GitHubAutoresponder.Webhook.Tests { public class WebhookControllerTests : IDisposable { private Mock gitHubResponder; private Mock modelStateConverter; + private Mock jsonSerialiser; + private Mock requestValidator; + private Mock environment; private WebhookController webhookController; public WebhookControllerTests() { this.gitHubResponder = new Mock(); this.modelStateConverter = new Mock(); + this.jsonSerialiser = new Mock(); + this.requestValidator = new Mock(); + this.environment = new Mock(); this.webhookController = new WebhookController( this.gitHubResponder.Object, - this.modelStateConverter.Object + this.modelStateConverter.Object, + this.jsonSerialiser.Object, + this.requestValidator.Object, + this.environment.Object ); } @@ -39,7 +49,7 @@ public async Task ItShouldForwardThePayload() { .Setup(g => g.RespondAsync(It.IsAny())) .ReturnsAsync(true); - ContentResult result = await this.webhookController.PostAsync(payload); + ContentResult result = await this.webhookController.PostAsync(); Assert.StrictEqual((int) HttpStatusCode.OK, result.StatusCode); Assert.Equal("OK", result.Content); @@ -56,7 +66,7 @@ public async Task ItShouldRespondWithBadRequestWhenPayloadIsInvalid() { this.webhookController.ModelState.AddModelError("key", "Some model error"); - ContentResult result = await this.webhookController.PostAsync(payload); + ContentResult result = await this.webhookController.PostAsync(); Assert.StrictEqual((int) HttpStatusCode.BadRequest, result.StatusCode); Assert.Equal("Model validation errors", result.Content); @@ -70,7 +80,7 @@ public async Task ItShouldRespondWithBadGatewayWhenUpstreamReturnsError() { .Setup(g => g.RespondAsync(It.IsAny())) .ReturnsAsync(false); - ContentResult result = await this.webhookController.PostAsync(payload); + ContentResult result = await this.webhookController.PostAsync(); Assert.StrictEqual((int) HttpStatusCode.BadGateway, result.StatusCode); } diff --git a/Webhook/WebhookController.cs b/Webhook/WebhookController.cs index bb5e41e..382b6b8 100644 --- a/Webhook/WebhookController.cs +++ b/Webhook/WebhookController.cs @@ -8,21 +8,54 @@ using GitHubAutoresponder.Responder; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.ModelBinding; +using GitHubAutoresponder.Shared; +using System.IO; namespace GitHubAutoresponder.Webhook { [Route("api/[controller]")] public class WebhookController : Controller { private IGitHubResponder gitHubResponder; private IModelStateConverter modelStateConverter; + private IJsonSerialiser jsonSerialiser; + private IRequestValidator requestValidator; + private IEnvironment environment; - public WebhookController(IGitHubResponder gitHubResponder, IModelStateConverter modelStateConverter) { + public WebhookController( + IGitHubResponder gitHubResponder, + IModelStateConverter modelStateConverter, + IJsonSerialiser jsonSerialiser, + IRequestValidator requestValidator, + IEnvironment environment + ) { this.gitHubResponder = gitHubResponder; this.modelStateConverter = modelStateConverter; + this.jsonSerialiser = jsonSerialiser; + this.requestValidator = requestValidator; + this.environment = environment; } [HttpPost] - public async Task PostAsync([FromBody]Payload payload) { - if (!ModelState.IsValid) { + public async Task PostAsync() { + /* We need the raw body to valid the request + * by computing a HMAC hash from it based upon + * the secret key. We then manually deserialise + * it for validation and further manipulation. + */ + string body = await this.GetBodyAsync(); + + bool isValidRequest = this.requestValidator.IsValidRequest( + Request.Headers["X-Hub-Signature"], + this.environment.Secret, + body + ); + + if (!isValidRequest) { + return this.CreateUnauthorisedResult(); + } + + Payload payload = this.jsonSerialiser.Deserialise(body); + + if (!TryValidateModel(payload)) { return this.CreateValidationErrorResult(); } @@ -31,6 +64,24 @@ public async Task PostAsync([FromBody]Payload payload) { return isSuccessful? this.CreateSuccessResult() : this.CreateUpstreamErrorResult(); } + private async Task GetBodyAsync() { + using (StreamReader reader = new StreamReader(Request.Body)) { + return await reader.ReadToEndAsync(); + } + } + + private ContentResult CreateUnauthorisedResult() { + ContentResult result = Content( + "This request lacks the required authorisation information", + MediaTypeNames.Text.Plain, + Encoding.UTF8 + ); + + result.StatusCode = (int) HttpStatusCode.Unauthorized; + + return result; + } + private ContentResult CreateValidationErrorResult() { ContentResult result = Content( modelStateConverter.AsString(ModelState),