Skip to content

Commit

Permalink
Prevent access to /tool/auth with valid session ID and invalid token (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
SapiensAnatis authored Oct 31, 2024
1 parent 7ac43e1 commit fd08d56
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ public class ToolTest : TestFixture
public ToolTest(CustomWebApplicationFactory factory, ITestOutputHelper outputHelper)
: base(factory, outputHelper)
{
this.Client.DefaultRequestHeaders.Remove("SID");
this.SetupSaveImport();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ namespace DragaliaAPI.Controllers;
/// should be used.
/// </remarks>
[ApiController]
[Authorize(AuthenticationSchemes = AuthConstants.SchemeNames.Session)]
[Consumes("application/octet-stream")]
[Produces("application/x-msgpack")]
[ServiceFilter<SetResultCodeActionFilter>(Order = 2)]
Expand Down Expand Up @@ -66,6 +65,7 @@ public OkObjectResult Code(ResultCode code)
/// <remarks>
/// Not to be used for endpoints that make up the title screen (/tool/*, /version/*, etc.) to prevent infinite loops.
/// </remarks>
[Authorize(AuthenticationSchemes = AuthConstants.SchemeNames.Session)]
[ServiceFilter<ResourceVersionActionFilter>(Order = 1)]
[ServiceFilter<MaintenanceActionFilter>(Order = 1)]
public abstract class DragaliaControllerBase : DragaliaControllerBaseCore { }
7 changes: 4 additions & 3 deletions DragaliaAPI/DragaliaAPI/Features/Tool/ToolController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -22,7 +23,7 @@ public ActionResult<DragaliaResult> GetServiceStatus()
}

[HttpPost("signup")]
[Authorize(AuthenticationSchemes = AuthConstants.SchemeNames.GameJwt)]
[Authorize(AuthenticationSchemes = SchemeNames.GameJwt)]
public async Task<DragaliaResult> Signup()
{
if (this.User.HasDawnshardIdentity())
Expand All @@ -45,7 +46,7 @@ public async Task<DragaliaResult> Signup()
}

[HttpPost("auth")]
[Authorize(AuthenticationSchemes = AuthConstants.SchemeNames.GameJwt)]
[Authorize(AuthenticationSchemes = SchemeNames.GameJwt)]
public async Task<DragaliaResult> Auth()
{
if (!this.User.HasDawnshardIdentity())
Expand All @@ -71,7 +72,7 @@ public async Task<DragaliaResult> Auth()
}

[HttpPost("reauth")]
[Authorize(AuthenticationSchemes = AuthConstants.SchemeNames.GameJwt)]
[Authorize(AuthenticationSchemes = SchemeNames.GameJwt)]
public async Task<DragaliaResult> Reauth()
{
(long viewerId, string sessionId) = await authService.DoLogin(this.User);
Expand Down
2 changes: 1 addition & 1 deletion DragaliaAPI/DragaliaAPI/Features/Web/FeatureExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public static IServiceCollection AddWebFeature(this IServiceCollection serviceCo
serviceCollection
.AddAuthorizationBuilder()
.AddPolicy(
PolicyNames.RequireValidJwt,
PolicyNames.RequireValidWebJwt,
builder =>
builder.RequireAuthenticatedUser().AddAuthenticationSchemes(SchemeNames.WebJwt)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class UserController(UserService userService, ILogger<UserController> log
: ControllerBase
{
[HttpGet("me")]
[Authorize(Policy = PolicyNames.RequireValidJwt)]
[Authorize(Policy = PolicyNames.RequireValidWebJwt)]
public async Task<ActionResult<User>> GetSelf(CancellationToken cancellationToken)
{
if (!this.User.HasDawnshardIdentity())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 4 additions & 15 deletions DragaliaAPI/DragaliaAPI/ServiceConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -236,21 +231,15 @@ public static IServiceCollection ConfigureAuthentication(this IServiceCollection
services
.AddAuthentication(opts =>
{
opts.AddScheme<SessionAuthenticationHandler>(
AuthConstants.SchemeNames.Session,
null
);
opts.AddScheme<DeveloperAuthenticationHandler>(
AuthConstants.SchemeNames.Developer,
null
);
opts.AddScheme<SessionAuthenticationHandler>(SchemeNames.Session, null);
opts.AddScheme<DeveloperAuthenticationHandler>(SchemeNames.Developer, null);
opts.AddScheme<PhotonAuthenticationHandler>(
nameof(PhotonAuthenticationHandler),
null
);
})
.AddJwtBearer(
AuthConstants.SchemeNames.GameJwt,
SchemeNames.GameJwt,
options =>
{
options.Events = new()
Expand Down

0 comments on commit fd08d56

Please sign in to comment.