From fd08d5660a9d995ec38edb9882cac68d5cd3c38e Mon Sep 17 00:00:00 2001 From: Jay Malhotra <5047192+SapiensAnatis@users.noreply.github.com> Date: Thu, 31 Oct 2024 19:58:51 +0000 Subject: [PATCH] Prevent access to /tool/auth with valid session ID and invalid token (#1137) After #1135 I noticed from the logs that SessionAuthentication runs when accessing `/tool/auth`, `/tool/reauth`, `/tool/signup`, etc. This allows you to access the controller with a valid session ID but invalid token, which causes `DoLogin` to crash on failing to retrieve the subject, as you have no JWT identity. Fix this by removing the implicit SessionAuthentication from DragaliaControllerBaseCore. --- .../Features/Tool/ToolTest.cs | 1 - .../Dragalia/TransitionController.cs | 3 +-- .../Controllers/DragaliaControllerBase.cs | 2 +- .../Features/Tool/ToolController.cs | 7 ++++--- .../Features/Web/FeatureExtensions.cs | 2 +- .../Features/Web/Users/UserController.cs | 2 +- .../Authentication/AuthConstants.cs | 6 +++--- .../DragaliaAPI/ServiceConfiguration.cs | 19 ++++--------------- 8 files changed, 15 insertions(+), 27 deletions(-) diff --git a/DragaliaAPI/DragaliaAPI.Integration.Test/Features/Tool/ToolTest.cs b/DragaliaAPI/DragaliaAPI.Integration.Test/Features/Tool/ToolTest.cs index 964550222..886292b41 100644 --- a/DragaliaAPI/DragaliaAPI.Integration.Test/Features/Tool/ToolTest.cs +++ b/DragaliaAPI/DragaliaAPI.Integration.Test/Features/Tool/ToolTest.cs @@ -16,7 +16,6 @@ public class ToolTest : TestFixture public ToolTest(CustomWebApplicationFactory factory, ITestOutputHelper outputHelper) : base(factory, outputHelper) { - this.Client.DefaultRequestHeaders.Remove("SID"); this.SetupSaveImport(); } diff --git a/DragaliaAPI/DragaliaAPI/Controllers/Dragalia/TransitionController.cs b/DragaliaAPI/DragaliaAPI/Controllers/Dragalia/TransitionController.cs index 825809ae5..faafba872 100644 --- a/DragaliaAPI/DragaliaAPI/Controllers/Dragalia/TransitionController.cs +++ b/DragaliaAPI/DragaliaAPI/Controllers/Dragalia/TransitionController.cs @@ -8,8 +8,7 @@ namespace DragaliaAPI.Controllers.Dragalia; [Route("transition")] -[AllowAnonymous] -internal sealed class TransitionController : DragaliaControllerBase +internal sealed class TransitionController : DragaliaControllerBaseCore { private readonly IAuthService authService; diff --git a/DragaliaAPI/DragaliaAPI/Controllers/DragaliaControllerBase.cs b/DragaliaAPI/DragaliaAPI/Controllers/DragaliaControllerBase.cs index a9eefcce0..5740b0ed1 100644 --- a/DragaliaAPI/DragaliaAPI/Controllers/DragaliaControllerBase.cs +++ b/DragaliaAPI/DragaliaAPI/Controllers/DragaliaControllerBase.cs @@ -17,7 +17,6 @@ namespace DragaliaAPI.Controllers; /// should be used. /// [ApiController] -[Authorize(AuthenticationSchemes = AuthConstants.SchemeNames.Session)] [Consumes("application/octet-stream")] [Produces("application/x-msgpack")] [ServiceFilter(Order = 2)] @@ -66,6 +65,7 @@ public OkObjectResult Code(ResultCode code) /// /// Not to be used for endpoints that make up the title screen (/tool/*, /version/*, etc.) to prevent infinite loops. /// +[Authorize(AuthenticationSchemes = AuthConstants.SchemeNames.Session)] [ServiceFilter(Order = 1)] [ServiceFilter(Order = 1)] public abstract class DragaliaControllerBase : DragaliaControllerBaseCore { } diff --git a/DragaliaAPI/DragaliaAPI/Features/Tool/ToolController.cs b/DragaliaAPI/DragaliaAPI/Features/Tool/ToolController.cs index f6e48391d..c5e9bef82 100644 --- a/DragaliaAPI/DragaliaAPI/Features/Tool/ToolController.cs +++ b/DragaliaAPI/DragaliaAPI/Features/Tool/ToolController.cs @@ -7,6 +7,7 @@ using DragaliaAPI.Shared.PlayerDetails; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; +using static DragaliaAPI.Infrastructure.Authentication.AuthConstants; namespace DragaliaAPI.Features.Tool; @@ -22,7 +23,7 @@ public ActionResult GetServiceStatus() } [HttpPost("signup")] - [Authorize(AuthenticationSchemes = AuthConstants.SchemeNames.GameJwt)] + [Authorize(AuthenticationSchemes = SchemeNames.GameJwt)] public async Task Signup() { if (this.User.HasDawnshardIdentity()) @@ -45,7 +46,7 @@ public async Task Signup() } [HttpPost("auth")] - [Authorize(AuthenticationSchemes = AuthConstants.SchemeNames.GameJwt)] + [Authorize(AuthenticationSchemes = SchemeNames.GameJwt)] public async Task Auth() { if (!this.User.HasDawnshardIdentity()) @@ -71,7 +72,7 @@ public async Task Auth() } [HttpPost("reauth")] - [Authorize(AuthenticationSchemes = AuthConstants.SchemeNames.GameJwt)] + [Authorize(AuthenticationSchemes = SchemeNames.GameJwt)] public async Task Reauth() { (long viewerId, string sessionId) = await authService.DoLogin(this.User); diff --git a/DragaliaAPI/DragaliaAPI/Features/Web/FeatureExtensions.cs b/DragaliaAPI/DragaliaAPI/Features/Web/FeatureExtensions.cs index a7573c580..f3e1057ef 100644 --- a/DragaliaAPI/DragaliaAPI/Features/Web/FeatureExtensions.cs +++ b/DragaliaAPI/DragaliaAPI/Features/Web/FeatureExtensions.cs @@ -40,7 +40,7 @@ public static IServiceCollection AddWebFeature(this IServiceCollection serviceCo serviceCollection .AddAuthorizationBuilder() .AddPolicy( - PolicyNames.RequireValidJwt, + PolicyNames.RequireValidWebJwt, builder => builder.RequireAuthenticatedUser().AddAuthenticationSchemes(SchemeNames.WebJwt) ) diff --git a/DragaliaAPI/DragaliaAPI/Features/Web/Users/UserController.cs b/DragaliaAPI/DragaliaAPI/Features/Web/Users/UserController.cs index 736a83401..d1b913900 100644 --- a/DragaliaAPI/DragaliaAPI/Features/Web/Users/UserController.cs +++ b/DragaliaAPI/DragaliaAPI/Features/Web/Users/UserController.cs @@ -11,7 +11,7 @@ public class UserController(UserService userService, ILogger log : ControllerBase { [HttpGet("me")] - [Authorize(Policy = PolicyNames.RequireValidJwt)] + [Authorize(Policy = PolicyNames.RequireValidWebJwt)] public async Task> GetSelf(CancellationToken cancellationToken) { if (!this.User.HasDawnshardIdentity()) diff --git a/DragaliaAPI/DragaliaAPI/Infrastructure/Authentication/AuthConstants.cs b/DragaliaAPI/DragaliaAPI/Infrastructure/Authentication/AuthConstants.cs index d31ab076c..fee5a0796 100644 --- a/DragaliaAPI/DragaliaAPI/Infrastructure/Authentication/AuthConstants.cs +++ b/DragaliaAPI/DragaliaAPI/Infrastructure/Authentication/AuthConstants.cs @@ -4,14 +4,14 @@ public static class AuthConstants { public static class PolicyNames { - public const string RequireValidJwt = nameof(RequireValidJwt); + public const string RequireValidWebJwt = "RequireValidWebJwt"; - public const string RequireDawnshardIdentity = nameof(RequireDawnshardIdentity); + public const string RequireDawnshardIdentity = "RequireDawnshardIdentity"; } public static class IdentityLabels { - public const string Dawnshard = nameof(Dawnshard); + public const string Dawnshard = "Dawnshard"; } public static class SchemeNames diff --git a/DragaliaAPI/DragaliaAPI/ServiceConfiguration.cs b/DragaliaAPI/DragaliaAPI/ServiceConfiguration.cs index c59309451..ff6e98284 100644 --- a/DragaliaAPI/DragaliaAPI/ServiceConfiguration.cs +++ b/DragaliaAPI/DragaliaAPI/ServiceConfiguration.cs @@ -25,9 +25,6 @@ using DragaliaAPI.Features.Tool; using DragaliaAPI.Features.Trade; using DragaliaAPI.Features.Version; -using DragaliaAPI.Features.Web; -using DragaliaAPI.Features.Zena; -using DragaliaAPI.Infrastructure; using DragaliaAPI.Infrastructure.Authentication; using DragaliaAPI.Infrastructure.Middleware; using DragaliaAPI.Models.Options; @@ -38,14 +35,12 @@ using DragaliaAPI.Services.Photon; using Hangfire; using Hangfire.PostgreSql; -using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.Extensions.Diagnostics.HealthChecks; using Microsoft.Extensions.Options; -using OpenTelemetry.Metrics; using OpenTelemetry.Resources; using OpenTelemetry.Trace; -using AuthService = DragaliaAPI.Features.Tool.AuthService; +using static DragaliaAPI.Infrastructure.Authentication.AuthConstants; namespace DragaliaAPI; @@ -236,21 +231,15 @@ public static IServiceCollection ConfigureAuthentication(this IServiceCollection services .AddAuthentication(opts => { - opts.AddScheme( - AuthConstants.SchemeNames.Session, - null - ); - opts.AddScheme( - AuthConstants.SchemeNames.Developer, - null - ); + opts.AddScheme(SchemeNames.Session, null); + opts.AddScheme(SchemeNames.Developer, null); opts.AddScheme( nameof(PhotonAuthenticationHandler), null ); }) .AddJwtBearer( - AuthConstants.SchemeNames.GameJwt, + SchemeNames.GameJwt, options => { options.Events = new()