Skip to content
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@
/// </summary>
/// <returns>The user and their roles if successful, <see cref="AuthenticateResult.Fail(string)" /> otherwise.</returns>
protected override async Task<AuthenticateResult> HandleAuthenticateAsync() {
// Skip options requests
if (Request.Method == "OPTIONS") {

Check failure

Code scanning / CodeQL

User-controlled bypass of sensitive method High

This condition guards a sensitive
action
, but a
user-provided value
controls it.

Copilot Autofix

AI about 2 months ago

To reduce the risk of user-controlled bypass of authentication, we need to remove or properly handle the condition that allows OPTIONS requests to skip authentication. The best practice is to handle OPTIONS requests at a different (earlier) layer, outside of the authentication handler—for example, by middleware that handles OPTIONS/CORS specifically, or by configuring endpoint authorization policies. If OPTIONS requests genuinely should always pass through unauthenticated, this should be enforced globally and securely. Otherwise, a malicious user can simply send OPTIONS requests to protected endpoints.
Steps:

  • In BasicAuthenticationHandler.cs, within HandleAuthenticateAsync, remove the check for Request.Method == "OPTIONS".
  • If OPTIONS requests must be handled differently, this should be set up elsewhere (such as globally allowing OPTIONS to bypass authentication using ASP.NET Core middleware or endpoint configuration).
  • No additional methods, imports, or definitions are required for this change.
Suggested changeset 1
src/Nullinside.Api.Common.AspNetCore/Middleware/BasicAuthenticationHandler.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/Nullinside.Api.Common.AspNetCore/Middleware/BasicAuthenticationHandler.cs b/src/Nullinside.Api.Common.AspNetCore/Middleware/BasicAuthenticationHandler.cs
--- a/src/Nullinside.Api.Common.AspNetCore/Middleware/BasicAuthenticationHandler.cs
+++ b/src/Nullinside.Api.Common.AspNetCore/Middleware/BasicAuthenticationHandler.cs
@@ -45,11 +45,6 @@
   /// </summary>
   /// <returns>The user and their roles if successful, <see cref="AuthenticateResult.Fail(string)" /> otherwise.</returns>
   protected override async Task<AuthenticateResult> HandleAuthenticateAsync() {
-    // Skip options requests
-    if (Request.Method == "OPTIONS") {
-      return AuthenticateResult.NoResult();
-    }
-
     // Read token from HTTP request header
     string? authorizationHeader = Request.Headers.Authorization;
     if (string.IsNullOrEmpty(authorizationHeader) || !authorizationHeader.StartsWith("Bearer ")) {
EOF
@@ -45,11 +45,6 @@
/// </summary>
/// <returns>The user and their roles if successful, <see cref="AuthenticateResult.Fail(string)" /> otherwise.</returns>
protected override async Task<AuthenticateResult> HandleAuthenticateAsync() {
// Skip options requests
if (Request.Method == "OPTIONS") {
return AuthenticateResult.NoResult();
}

// Read token from HTTP request header
string? authorizationHeader = Request.Headers.Authorization;
if (string.IsNullOrEmpty(authorizationHeader) || !authorizationHeader.StartsWith("Bearer ")) {
Copilot is powered by AI and may make mistakes. Always verify output.
return AuthenticateResult.NoResult();
}

// Read token from HTTP request header
string? authorizationHeader = Request.Headers.Authorization;
if (string.IsNullOrEmpty(authorizationHeader) || !authorizationHeader.StartsWith("Bearer ")) {
Expand Down Expand Up @@ -87,18 +92,27 @@
}
}

ClaimsIdentity identity = new(claims, "BasicBearerToken");
ClaimsPrincipal user = new(identity);
AuthenticationProperties authProperties = new() {
IsPersistent = true
};

AuthenticationTicket ticket = new(user, authProperties, "BasicBearerToken");
return AuthenticateResult.Success(ticket);
return CreateAuth(claims);
}
catch (Exception ex) {
_logger.Error("Failed to create an auth ticket after successful token validation", ex);
return AuthenticateResult.Fail(ex);
}
}

/// <summary>
/// Creates the authorization ticket.
/// </summary>
/// <param name="claims">The list of claims to provide.</param>
/// <returns>The authorization ticket.</returns>
private static AuthenticateResult CreateAuth(List<Claim> claims) {
ClaimsIdentity identity = new(claims, "BasicBearerToken");
ClaimsPrincipal user = new(identity);
AuthenticationProperties authProperties = new() {
IsPersistent = true
};

AuthenticationTicket ticket = new(user, authProperties, "BasicBearerToken");
return AuthenticateResult.Success(ticket);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
<PrivateAssets>all</PrivateAssets>
</PackageReference>
<PackageReference Include="Nullinside.MySql.EntityFrameworkCore" Version="9.0.3"/>
<PackageReference Include="Swashbuckle.AspNetCore" Version="9.0.5" />
<PackageReference Include="System.Text.Json" Version="9.0.9" />
<PackageReference Include="Swashbuckle.AspNetCore" Version="9.0.5"/>
<PackageReference Include="System.Text.Json" Version="9.0.9"/>
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<wpf:ResourceDictionary xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib"
<wpf:ResourceDictionary xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:s="clr-namespace:System;assembly=mscorlib"
xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xml:space="preserve">
<s:String x:Key="/Default/CodeInspection/Daemon/ConfigureAwaitAnalysisMode/@EntryValue">Library</s:String></wpf:ResourceDictionary>
6 changes: 3 additions & 3 deletions src/Nullinside.Api.Common/Nullinside.Api.Common.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="log4net" Version="3.2.0" />
<PackageReference Include="Microsoft.Extensions.Logging" Version="9.0.9" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.4" />
<PackageReference Include="log4net" Version="3.2.0"/>
<PackageReference Include="Microsoft.Extensions.Logging" Version="9.0.9"/>
<PackageReference Include="Newtonsoft.Json" Version="13.0.4"/>
<PackageReference Include="SSH.NET" Version="2025.0.0"/>
<PackageReference Include="TwitchLib.Api" Version="3.9.0"/>
<PackageReference Include="TwitchLib.Client" Version="3.4.0"/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<wpf:ResourceDictionary xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib"
<wpf:ResourceDictionary xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:s="clr-namespace:System;assembly=mscorlib"
xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xml:space="preserve">
<s:String x:Key="/Default/CodeInspection/Daemon/ConfigureAwaitAnalysisMode/@EntryValue">Library</s:String></wpf:ResourceDictionary>
2 changes: 1 addition & 1 deletion src/Nullinside.Api.Model/Ddl/ITableModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ public interface ITableModel {
/// The method used to configure the POCOs of the table.
/// </summary>
/// <param name="modelBuilder">The model builder.</param>
public void OnModelCreating(ModelBuilder modelBuilder);
void OnModelCreating(ModelBuilder modelBuilder);
}
4 changes: 2 additions & 2 deletions src/Nullinside.Api.Model/Ddl/User.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ public class User : ITableModel {
/// The user's auth token for interacting with the site's API.
/// </summary>
public string? Token { get; set; }

/// <summary>
/// The user's auth token for interacting with the site's API.
/// </summary>
public string? RefreshToken { get; set; }

/// <summary>
/// The user's auth token for interacting with the site's API.
/// </summary>
Expand Down
4 changes: 2 additions & 2 deletions src/Nullinside.Api.Model/Nullinside.Api.Model.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.EntityFrameworkCore" Version="9.0.9" />
<PackageReference Include="Microsoft.EntityFrameworkCore" Version="9.0.9"/>
<PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="9.0.9">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
<PackageReference Include="Nullinside.MySql.EntityFrameworkCore" Version="9.0.3"/>
<PackageReference Include="System.Text.Json" Version="9.0.9" />
<PackageReference Include="System.Text.Json" Version="9.0.9"/>
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<wpf:ResourceDictionary xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib"
<wpf:ResourceDictionary xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:s="clr-namespace:System;assembly=mscorlib"
xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xml:space="preserve">
<s:String x:Key="/Default/CodeInspection/Daemon/ConfigureAwaitAnalysisMode/@EntryValue">Library</s:String></wpf:ResourceDictionary>
8 changes: 4 additions & 4 deletions src/Nullinside.Api.Model/Shared/UserHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public static class UserHelpers {
/// <param name="twitchUsername">The username of the user on twitch.</param>
/// <param name="twitchId">The id of the user on twitch.</param>
/// <returns>The bearer token if successful, null otherwise.</returns>
public static async Task<OAuthToken?> GenerateTokenAndSaveToDatabase(INullinsideContext dbContext, string email,
public static async Task<OAuthToken?> GenerateTokenAndSaveToDatabase(INullinsideContext dbContext, string email,
TimeSpan tokenExpires, string? authToken = null, string? refreshToken = null, DateTime? expires = null,
string? twitchUsername = null, string? twitchId = null, CancellationToken cancellationToken = new()) {
string bearerToken = AuthUtils.GenerateToken();
Expand Down Expand Up @@ -76,9 +76,9 @@ public static class UserHelpers {
}

await dbContext.SaveChangesAsync(cancellationToken).ConfigureAwait(false);
return new() {
AccessToken = bearerToken,
RefreshToken = bearerRefreshToken,
return new OAuthToken {
AccessToken = bearerToken,
RefreshToken = bearerRefreshToken,
ExpiresUtc = expiresUtc
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Nullinside.Api.Common.Auth;
using Nullinside.Api.Model.Ddl;
using Nullinside.Api.Model.Shared;

Expand All @@ -23,7 +24,7 @@ public async Task GenerateTokenForExistingUser() {
Assert.That(_db.Users.Count(), Is.EqualTo(1));

// Generate a new token
var token = await UserHelpers.GenerateTokenAndSaveToDatabase(_db, "email", Constants.OAUTH_TOKEN_TIME_LIMIT).ConfigureAwait(false);
OAuthToken? token = await UserHelpers.GenerateTokenAndSaveToDatabase(_db, "email", Constants.OAUTH_TOKEN_TIME_LIMIT).ConfigureAwait(false);
Assert.That(token, Is.Not.Null);

// Verify we still only have one user
Expand All @@ -48,7 +49,7 @@ public async Task GenerateTokenForNewUser() {
Assert.That(_db.Users.Count(), Is.EqualTo(1));

// Generate a new token
var token = await UserHelpers.GenerateTokenAndSaveToDatabase(_db, "email", Constants.OAUTH_TOKEN_TIME_LIMIT).ConfigureAwait(false);
OAuthToken? token = await UserHelpers.GenerateTokenAndSaveToDatabase(_db, "email", Constants.OAUTH_TOKEN_TIME_LIMIT).ConfigureAwait(false);
Assert.That(token, Is.Not.Null);

// Verify we have a new user
Expand All @@ -65,7 +66,7 @@ public async Task GenerateTokenForNewUser() {
[Test]
public async Task HandleUnexpectedErrors() {
// Force an error to occur.
var token = await UserHelpers.GenerateTokenAndSaveToDatabase(null!, "email", Constants.OAUTH_TOKEN_TIME_LIMIT).ConfigureAwait(false);
OAuthToken? token = await UserHelpers.GenerateTokenAndSaveToDatabase(null!, "email", Constants.OAUTH_TOKEN_TIME_LIMIT).ConfigureAwait(false);
Assert.That(token, Is.Null);
}
}
10 changes: 5 additions & 5 deletions src/Nullinside.Api.Tests/Nullinside.Api.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.EntityFrameworkCore.InMemory" Version="9.0.9" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Relational" Version="9.0.9" />
<PackageReference Include="Microsoft.EntityFrameworkCore.InMemory" Version="9.0.9"/>
<PackageReference Include="Microsoft.EntityFrameworkCore.Relational" Version="9.0.9"/>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.14.1"/>
<PackageReference Include="Moq" Version="4.20.72"/>
<PackageReference Include="Newtonsoft.Json" Version="13.0.4" />
<PackageReference Include="NUnit" Version="4.4.0" />
<PackageReference Include="NUnit3TestAdapter" Version="5.1.0" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.4"/>
<PackageReference Include="NUnit" Version="4.4.0"/>
<PackageReference Include="NUnit3TestAdapter" Version="5.1.0"/>
<PackageReference Include="NUnit.Analyzers" Version="4.10.0">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<wpf:ResourceDictionary xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib"
<wpf:ResourceDictionary xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:s="clr-namespace:System;assembly=mscorlib"
xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xml:space="preserve">
<s:String x:Key="/Default/CodeInspection/Daemon/ConfigureAwaitAnalysisMode/@EntryValue">Library</s:String></wpf:ResourceDictionary>
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System.Security.Claims;
using System.Text;
using System.Text.Unicode;

using Google.Apis.Auth;

Expand All @@ -21,8 +20,6 @@
using Nullinside.Api.Shared;
using Nullinside.Api.Shared.Json;

using Org.BouncyCastle.Utilities.Encoders;

namespace Nullinside.Api.Tests.Nullinside.Api.Controllers;

/// <summary>
Expand Down Expand Up @@ -81,13 +78,13 @@ public async Task PerformGoogleLoginExisting() {

// We should have been redirected to the successful route.
Assert.That(obj.Url.StartsWith("/user/login?token="), Is.True);
var queryParam = obj.Url["/user/login?token=".Length..];
string queryParam = obj.Url["/user/login?token=".Length..];

// No additional users should have been created.
Assert.That(_db.Users.Count(), Is.EqualTo(1));

// We should have saved the token in the existing user's database.
var json = Encoding.UTF8.GetString(Convert.FromBase64String(queryParam));
string json = Encoding.UTF8.GetString(Convert.FromBase64String(queryParam));
var oauth = JsonConvert.DeserializeObject<OAuthToken>(json);
Assert.That(oauth?.AccessToken!, Is.EqualTo(_db.Users.First().Token!));
}
Expand All @@ -104,13 +101,13 @@ public async Task PerformGoogleLoginNewUser() {

// We should have been redirected to the successful route.
Assert.That(obj.Url.StartsWith("/user/login?token="), Is.True);
var queryParam = obj.Url["/user/login?token=".Length..];
string queryParam = obj.Url["/user/login?token=".Length..];

// No additional users should have been created.
Assert.That(_db.Users.Count(), Is.EqualTo(1));

// We should have saved the token in the existing user's database.
var json = Encoding.UTF8.GetString(Convert.FromBase64String(queryParam));
string json = Encoding.UTF8.GetString(Convert.FromBase64String(queryParam));
var oauth = JsonConvert.DeserializeObject<OAuthToken>(json);
Assert.That(oauth?.AccessToken!, Is.EqualTo(_db.Users.First().Token!));
}
Expand Down Expand Up @@ -172,13 +169,13 @@ public async Task PerformTwitchLoginExisting() {

// We should have been redirected to the successful route.
Assert.That(obj.Url.StartsWith("/user/login?token="), Is.True);
var queryParam = obj.Url["/user/login?token=".Length..];
string queryParam = obj.Url["/user/login?token=".Length..];

// No additional users should have been created.
Assert.That(_db.Users.Count(), Is.EqualTo(1));

// We should have saved the token in the existing user's database.
var json = Encoding.UTF8.GetString(Convert.FromBase64String(queryParam));
string json = Encoding.UTF8.GetString(Convert.FromBase64String(queryParam));
var oauth = JsonConvert.DeserializeObject<OAuthToken>(json);
Assert.That(oauth?.AccessToken!, Is.EqualTo(_db.Users.First().Token!));
}
Expand All @@ -202,13 +199,13 @@ public async Task PerformTwitchLoginNewUser() {

// We should have been redirected to the successful route.
Assert.That(obj.Url.StartsWith("/user/login?token="), Is.True);
var queryParam = obj.Url["/user/login?token=".Length..];
string queryParam = obj.Url["/user/login?token=".Length..];

// No additional users should have been created.
Assert.That(_db.Users.Count(), Is.EqualTo(1));

// We should have saved the token in the existing user's database.
var json = Encoding.UTF8.GetString(Convert.FromBase64String(queryParam));
string json = Encoding.UTF8.GetString(Convert.FromBase64String(queryParam));
var oauth = JsonConvert.DeserializeObject<OAuthToken>(json);
Assert.That(oauth?.AccessToken!, Is.EqualTo(_db.Users.First().Token!));
}
Expand Down
4 changes: 2 additions & 2 deletions src/Nullinside.Api/Constants.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
namespace Nullinside.Api;

/// <summary>
/// Constants used throughout the application.
/// Constants used throughout the application.
/// </summary>
public static class Constants {
/// <summary>
/// The amount of time a token is valid for.
/// The amount of time a token is valid for.
/// </summary>
public static readonly TimeSpan OAUTH_TOKEN_TIME_LIMIT = TimeSpan.FromHours(1);
}
22 changes: 10 additions & 12 deletions src/Nullinside.Api/Controllers/UserController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@
using Nullinside.Api.Shared;
using Nullinside.Api.Shared.Json;

using Org.BouncyCastle.Utilities.Encoders;

namespace Nullinside.Api.Controllers;

/// <summary>
Expand Down Expand Up @@ -81,19 +79,19 @@ public UserController(IConfiguration configuration, INullinsideContext dbContext
return Redirect($"{siteUrl}/user/login?error=1");
}

var bearerToken = await UserHelpers.GenerateTokenAndSaveToDatabase(_dbContext, credentials.Email, Constants.OAUTH_TOKEN_TIME_LIMIT, cancellationToken: token).ConfigureAwait(false);
OAuthToken? bearerToken = await UserHelpers.GenerateTokenAndSaveToDatabase(_dbContext, credentials.Email, Constants.OAUTH_TOKEN_TIME_LIMIT, cancellationToken: token).ConfigureAwait(false);
if (null == bearerToken) {
return Redirect($"{siteUrl}/user/login?error=2");
}
var json = JsonConvert.SerializeObject(bearerToken);

string json = JsonConvert.SerializeObject(bearerToken);
return Redirect($"{siteUrl}/user/login?token={Convert.ToBase64String(Encoding.UTF8.GetBytes(json))}");
}
catch (InvalidJwtException) {
return Redirect($"{siteUrl}/user/login?error=1");
}
}

/// <summary>
/// Called to generate a new oauth token using the refresh token we previously provided.
/// </summary>
Expand All @@ -104,16 +102,16 @@ public UserController(IConfiguration configuration, INullinsideContext dbContext
[HttpPost]
[Route("token/refresh")]
public async Task<ActionResult> Refresh(AuthToken token, CancellationToken cancellationToken = new()) {
var user = await _dbContext.Users.FirstOrDefaultAsync(u => u.RefreshToken == token.Token, cancellationToken).ConfigureAwait(false);
User? user = await _dbContext.Users.FirstOrDefaultAsync(u => u.RefreshToken == token.Token, cancellationToken).ConfigureAwait(false);
if (null == user?.Email) {
return Unauthorized();
}
var bearerToken = await UserHelpers.GenerateTokenAndSaveToDatabase(_dbContext, user.Email, Constants.OAUTH_TOKEN_TIME_LIMIT, cancellationToken: cancellationToken).ConfigureAwait(false);

OAuthToken? bearerToken = await UserHelpers.GenerateTokenAndSaveToDatabase(_dbContext, user.Email, Constants.OAUTH_TOKEN_TIME_LIMIT, cancellationToken: cancellationToken).ConfigureAwait(false);
if (null == bearerToken) {
return StatusCode(500);
}

return Ok(bearerToken);
}

Expand Down Expand Up @@ -155,12 +153,12 @@ public async Task<RedirectResult> TwitchLogin([FromQuery] string code, [FromServ
return Redirect($"{siteUrl}/user/login?error=4");
}

var bearerToken = await UserHelpers.GenerateTokenAndSaveToDatabase(_dbContext, email, Constants.OAUTH_TOKEN_TIME_LIMIT, cancellationToken: token).ConfigureAwait(false);
OAuthToken? bearerToken = await UserHelpers.GenerateTokenAndSaveToDatabase(_dbContext, email, Constants.OAUTH_TOKEN_TIME_LIMIT, cancellationToken: token).ConfigureAwait(false);
if (null == bearerToken) {
return Redirect($"{siteUrl}/user/login?error=2");
}

var json = JsonConvert.SerializeObject(bearerToken);
string json = JsonConvert.SerializeObject(bearerToken);
return Redirect($"{siteUrl}/user/login?token={Convert.ToBase64String(Encoding.UTF8.GetBytes(json))}");
}

Expand Down
Loading