Skip to content

fix authentication #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions rubberduckvba.Server/Api/Admin/AdminController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public class AdminController(ConfigurationOptions options, HangfireLauncherServi
/// Enqueues a job that updates xmldoc content from the latest release/pre-release tags.
/// </summary>
/// <returns>The unique identifier of the enqueued job.</returns>
[Authorize("github", AuthenticationSchemes = "github")]
[Authorize("github")]
[HttpPost("admin/update/xmldoc")]
public IActionResult UpdateXmldocContent()
{
Expand All @@ -24,7 +24,7 @@ public IActionResult UpdateXmldocContent()
/// Enqueues a job that gets the latest release/pre-release tags and their respective assets, and updates the installer download stats.
/// </summary>
/// <returns>The unique identifier of the enqueued job.</returns>
[Authorize("github", AuthenticationSchemes = "github")]
[Authorize("github")]
[HttpPost("admin/update/tags")]
public IActionResult UpdateTagMetadata()
{
Expand Down
32 changes: 32 additions & 0 deletions rubberduckvba.Server/Api/Admin/PushWebhookPayload.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
namespace rubberduckvba.Server.Api.Admin;

public record class PushWebhookPayload
{
public string Ref { get; set; }

public bool Created { get; set; }
public bool Deleted { get; set; }
public bool Forced { get; set; }

public WebhookPayloadSender Sender { get; set; }
public WebhookPayloadRepository Repository { get; set; }

public record class WebhookPayloadSender
{
public string Login { get; set; }
}

public record class WebhookPayloadRepository
{
public int Id { get; set; }
public string Name { get; set; }
public WebhookPayloadRepositoryOwner Owner { get; set; }

public record class WebhookPayloadRepositoryOwner
{
public int Id { get; set; }
public string Login { get; set; }
public string Type { get; set; }
}
}
}
10 changes: 7 additions & 3 deletions rubberduckvba.Server/Api/Admin/WebhookController.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using Newtonsoft.Json.Linq;

namespace rubberduckvba.Server.Api.Admin;

Expand All @@ -20,9 +19,9 @@ public WebhookController(
_hangfire = hangfire;
}

[Authorize("webhook", AuthenticationSchemes = "webhook-signature")]
[Authorize("webhook")]
[HttpPost("webhook/github")]
public IActionResult GitHub([FromBody] JToken payload)
public IActionResult GitHub([FromBody] PushWebhookPayload payload)
{
var eventType = _validator.Validate(payload, Request.Headers, out var content, out var gitref);

Expand All @@ -34,6 +33,11 @@ public IActionResult GitHub([FromBody] JToken payload)
Logger.LogInformation(message);
return Ok(message);
}
else if (eventType == WebhookPayloadType.Ping)
{
Logger.LogInformation("Webhook ping event was accepted; nothing to process.");
return Ok();
}
else if (eventType == WebhookPayloadType.Greeting)
{
Logger.LogInformation("Webhook push event was accepted; nothing to process. {content}", content);
Expand Down
1 change: 1 addition & 0 deletions rubberduckvba.Server/Api/Admin/WebhookPayloadType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ public enum WebhookPayloadType
{
Unsupported,
Greeting,
Ping,
Push
}
37 changes: 12 additions & 25 deletions rubberduckvba.Server/Api/Admin/WebhookPayloadValidationService.cs
Original file line number Diff line number Diff line change
@@ -1,39 +1,26 @@
using Newtonsoft.Json.Linq;

namespace rubberduckvba.Server.Api.Admin;
namespace rubberduckvba.Server.Api.Admin;

public class WebhookPayloadValidationService(ConfigurationOptions options)
{
public WebhookPayloadType Validate(JToken payload, IHeaderDictionary headers, out string? content, out GitRef? gitref)
public WebhookPayloadType Validate(PushWebhookPayload payload, IHeaderDictionary headers, out string? content, out GitRef? gitref)
{
content = default;
gitref = default;

if (!IsValidHeaders(headers) || !IsValidSource(payload) || !IsValidEvent(payload))
gitref = new GitRef(payload.Ref);
if (headers["X-GitHub-Event"].FirstOrDefault() == "ping")
{
return WebhookPayloadType.Unsupported;
if (!(payload.Created && gitref.Value.IsTag))
{
return WebhookPayloadType.Greeting;
}
return WebhookPayloadType.Ping;
}

gitref = new GitRef(payload.Value<string>("ref"));
if (!(payload.Value<bool>("created") && gitref.HasValue && gitref.Value.IsTag))
if (headers["X-GitHub-Event"].FirstOrDefault() == "push")
{
content = payload.Value<string>("zen");
return WebhookPayloadType.Greeting;
return WebhookPayloadType.Push;
}

return WebhookPayloadType.Push;
}

private bool IsValidHeaders(IHeaderDictionary headers) =>
headers.TryGetValue("X-GitHub-Event", out Microsoft.Extensions.Primitives.StringValues values) && values.Contains("push");

private bool IsValidSource(JToken payload) =>
payload["repository"].Value<string>("name") == options.GitHubOptions.Value.Rubberduck &&
payload["owner"].Value<int>("id") == options.GitHubOptions.Value.RubberduckOrgId;

private bool IsValidEvent(JToken payload)
{
var ev = payload["hook"]?["events"]?.Values<string>() ?? [];
return ev.Contains("push");
return WebhookPayloadType.Unsupported;
}
}
15 changes: 7 additions & 8 deletions rubberduckvba.Server/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
using rubberduckvba.Server.Services.rubberduckdb;
using System.Diagnostics;
using System.Reflection;
using System.Security.Claims;

namespace rubberduckvba.Server;

Expand All @@ -43,12 +42,12 @@ public static void Main(string[] args)
builder.Services.Configure<ApiSettings>(options => builder.Configuration.GetSection("Api").Bind(options));
builder.Services.Configure<HangfireSettings>(options => builder.Configuration.GetSection("Hangfire").Bind(options));


builder.Services.AddAuthentication(options =>
{
options.RequireAuthenticatedSignIn = false;

options.DefaultAuthenticateScheme = "github";
options.DefaultChallengeScheme = "github";
options.DefaultScheme = "anonymous";

options.AddScheme("github", builder =>
{
Expand All @@ -59,18 +58,16 @@ public static void Main(string[] args)
builder.HandlerType = typeof(WebhookAuthenticationHandler);
});
});

builder.Services.AddAuthorization(options =>
{
options.AddPolicy("github", builder =>
{
builder.RequireAuthenticatedUser();
builder.RequireAuthenticatedUser().AddAuthenticationSchemes("github");
});
options.AddPolicy("webhook", builder =>
{
builder.RequireAuthenticatedUser()
.RequireClaim(ClaimTypes.Authentication, "webhook-signature")
.RequireClaim(ClaimTypes.Role, "rubberduck-webhook")
.RequireClaim(ClaimTypes.Name, "rubberduck-vba-releasebot");
builder.RequireAuthenticatedUser().AddAuthenticationSchemes("webhook-signature");
});
});

Expand Down Expand Up @@ -164,6 +161,8 @@ private static void ConfigureServices(IServiceCollection services)
services.AddSingleton<IMarkdownFormattingService, MarkdownFormattingService>();
services.AddSingleton<ISyntaxHighlighterService, SyntaxHighlighterService>();
services.AddSingleton<WebhookSignatureValidationService>();
services.AddSingleton<WebhookPayloadValidationService>();
services.AddSingleton<HangfireLauncherService>();

services.AddSingleton<IRubberduckDbService, RubberduckDbService>();
services.AddSingleton<TagServices>();
Expand Down
8 changes: 7 additions & 1 deletion rubberduckvba.Server/WebhookSignatureValidationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ string[] xHubSignature256

if (!xGitHubEvent.Contains("push"))
{
// only authenticate push events
if (xGitHubEvent.Contains("ping"))
{
// no harm just returning 200-OK on ping
return true;
}

// only authenticate ping and push events
return false;
}

Expand Down