Don't cache tokens, additional auth validation (#1052)

Replaces Bunkum's `AuthenticationService` with an implementation which
does not cache tokens per thread, and also adds validation of tokens
received from DB to certain methods to try and avoid weird,
unreproducible issues from causing damage.

Author: Toastbrot236

Refresh.Core/Services/GameAuthenticationService.cs

@@ -0,0 +1,56 @@
+using System.Reflection;
+using Bunkum.Core;
+using Bunkum.Core.Authentication;
+using Bunkum.Core.Database;
+using Bunkum.Core.Endpoints;
+using Bunkum.Core.Responses;
+using Bunkum.Core.Services;
+using Bunkum.Listener.Protocol;
+using Bunkum.Listener.Request;
+using NotEnoughLogs;
+using Refresh.Database.Models.Authentication;
+
+namespace Refresh.Core.Services;
+
+// Referenced from https://github.com/PlanetBunkum/Bunkum/blob/main/Bunkum.Core/Services/AuthenticationService.cs
+// purposefully a less optimized implementation (as in it doesn't cache the token)
+public class GameAuthenticationService : Service
+{
+    private readonly IAuthenticationProvider<Token> _provider;
+
+    public GameAuthenticationService(Logger logger, IAuthenticationProvider<Token> provider) : base(logger)
+    {
+        this._provider = provider;
+    }
+
+    public override Response? OnRequestHandled(ListenerContext context, MethodInfo method, Lazy<IDatabaseContext> database)
+    {
+        if (!(method.GetCustomAttribute<AuthenticationAttribute>()?.Required ?? true)) return null;
+        
+        if (this.AuthenticateToken(context, database) == null)
+            return new Response("Not authenticated", ContentType.Plaintext, Forbidden);
+
+        return null;
+    }
+
+    public override object? AddParameterToEndpoint(ListenerContext context, BunkumParameterInfo parameter, Lazy<IDatabaseContext> database)
+    {
+        if (ParameterBasedFrom<IToken<IUser>>(parameter))
+        {
+            return this.AuthenticateToken(context, database);
+        }
+
+        if (ParameterBasedFrom<IUser>(parameter))
+        {
+            IToken<IUser>? token = this.AuthenticateToken(context, database);
+            if (token != null) return token.User;
+        }
+        
+        return null;
+    }
+
+    public Token? AuthenticateToken(ListenerContext context, Lazy<IDatabaseContext> database)
+    {
+        return this._provider.AuthenticateToken(context, database);
+    }
+}

Refresh.Core/Services/GameRateLimitService.cs

@@ -0,0 +1,39 @@
+using System.Reflection;
+using Bunkum.Core.Database;
+using Bunkum.Core.RateLimit;
+using Bunkum.Core.Responses;
+using Bunkum.Core.Services;
+using Bunkum.Listener.Protocol;
+using Bunkum.Listener.Request;
+using NotEnoughLogs;
+using Refresh.Database.Models.Users;
+
+namespace Refresh.Core.Services;
+
+// Referenced from https://github.com/PlanetBunkum/Bunkum/blob/main/Bunkum.Core/Services/RateLimitService.cs
+public class GameRateLimitService : Service
+{
+    private readonly IRateLimiter _rateLimiter;
+    private readonly GameAuthenticationService _authService;
+
+    internal GameRateLimitService(Logger logger, GameAuthenticationService authService, IRateLimiter rateLimiter) : base(logger)
+    {
+        this._rateLimiter = rateLimiter;
+        this._authService = authService;
+    }
+
+    public override Response? OnRequestHandled(ListenerContext context, MethodInfo method, Lazy<IDatabaseContext> database)
+    {
+        GameUser? user = this._authService.AuthenticateToken(context, database)?.User;
+
+        bool violated = false;
+
+        if (user != null)
+            violated = this._rateLimiter.UserViolatesRateLimit(context, method, user);
+        else
+            violated = this._rateLimiter.RemoteEndpointViolatesRateLimit(context, method);
+
+        if (violated) return new Response("You have been rate-limited.", ContentType.Plaintext, TooManyRequests);
+        return null;
+    }
+}

Refresh.Core/Services/RoleService.cs

@@ -12,14 +12,14 @@
 namespace Refresh.Core.Services;
 
 /// <summary>
-/// A service that hooks into the AuthenticationService, adding extra checks for roles.
+/// A service that hooks into the GameAuthenticationService, adding extra checks for roles.
 /// </summary>
 public class RoleService : Service
 {
-    private readonly AuthenticationService _authService;
+    private readonly GameAuthenticationService _authService;
     private readonly GameServerConfig _config;
 
-    internal RoleService(AuthenticationService authService, GameServerConfig config, Logger logger) : base(logger)
+    internal RoleService(GameAuthenticationService authService, GameServerConfig config, Logger logger) : base(logger)
     {
         this._authService = authService;
         this._config = config;
@@ -39,14 +39,12 @@ internal RoleService(AuthenticationService authService, GameServerConfig config,
         // if the user's role is lower than the minimum role for this endpoint, then return unauthorized
         if (user.Role < minimumRole)
         {
-            this._authService.RemoveTokenFromCache();
             return Unauthorized;
         }
 
         RequireEmailVerifiedAttribute? emailAttrib = method.GetCustomAttribute<RequireEmailVerifiedAttribute>();
         if (emailAttrib != null && !user.EmailAddressVerified)
         {
-            this._authService.RemoveTokenFromCache();
             return Unauthorized;
         }
 
@@ -64,7 +62,6 @@ internal RoleService(AuthenticationService authService, GameServerConfig config,
         // If user isn't an admin, then stop the request here, ignoring all
         if (user.Role != GameUserRole.Admin)
         {
-            this._authService.RemoveTokenFromCache();
             return Forbidden;
         }
 

Refresh.Core/Types/Data/DataContextService.cs

@@ -14,11 +14,11 @@ public class DataContextService : Service
 {
     private readonly StorageService _storageService;
     private readonly MatchService _matchService;
-    private readonly AuthenticationService _authService;
+    private readonly GameAuthenticationService _authService;
     private readonly GuidCheckerService _guidCheckerService;
     private readonly CacheService _cacheService;
 
-    public DataContextService(StorageService storage, MatchService match, AuthenticationService auth, Logger logger, GuidCheckerService guidChecker, CacheService cache) : base(logger)
+    public DataContextService(StorageService storage, MatchService match, GameAuthenticationService auth, Logger logger, GuidCheckerService guidChecker, CacheService cache) : base(logger)
     {
         this._storageService = storage;
         this._matchService = match;

Refresh.Database/GameDatabaseContext.Tokens.cs

@@ -4,6 +4,7 @@
 using Refresh.Database.Models.Authentication;
 using Refresh.Database.Models.Users;
 using Refresh.Database.Models.Relations;
+using System.Diagnostics;
 
 namespace Refresh.Database;
 
@@ -84,6 +85,14 @@ public Token GenerateTokenForUser(GameUser user, TokenType type, TokenGame game,
             return null;
         }
 
+        if (token.TokenData != tokenData || token.TokenType != type)
+        {
+#if DEBUG
+            if(Debugger.IsAttached) Debugger.Break();
+#endif
+            throw new InvalidDataException($"GetTokenFromTokenData - Token data or type does not match!");
+        }
+
         return token;
     }
 

Refresh.Database/GameDatabaseContext.Users.cs

@@ -9,6 +9,7 @@
 using Refresh.Database.Models.Levels;
 using Refresh.Database.Models.Photos;
 using Refresh.Database.Models.Assets;
+using System.Diagnostics;
 
 namespace Refresh.Database;
 
@@ -64,8 +65,20 @@ public partial class GameDatabaseContext // Users
     public GameUser? GetUserByEmailAddress(string? emailAddress)
     {
         if (emailAddress == null) return null;
+
         emailAddress = emailAddress.ToLowerInvariant();
-        return this.GameUsersIncluded.FirstOrDefault(u => u.EmailAddress == emailAddress);
+        GameUser? user = this.GameUsersIncluded.FirstOrDefault(u => u.EmailAddress == emailAddress);
+        if (user == null) return null;
+
+        if (user.EmailAddress != emailAddress)
+        {
+#if DEBUG
+            if(Debugger.IsAttached) Debugger.Break();
+#endif
+            throw new InvalidDataException($"GetUserByEmailAddress - Found user's email does not match given email!");
+        }
+
+        return user;
     }
 
     [Pure]

Refresh.Database/Models/Authentication/Token.cs

@@ -28,8 +28,9 @@ public partial class Token : IToken<GameUser>
     [Required]
     public string IpAddress { get; set; }
 
-    [Required]
-    public GameUser User { get; set; }
+    [ForeignKey(nameof(UserId))]
+    [Required] public GameUser User { get; set; }
+    [Required] public ObjectId UserId { get; set; }
 
     /// <summary>
     /// The digest key to use with this token, determined from the first game request created by this token

Refresh.GameServer/Authentication/GameAuthenticationProvider.cs

@@ -9,16 +9,20 @@
 using Refresh.Interfaces.APIv3;
 using Refresh.Interfaces.Game;
 using Refresh.Interfaces.Internal;
+using NotEnoughLogs;
+using Bunkum.Core;
 
 namespace Refresh.GameServer.Authentication;
 
 public class GameAuthenticationProvider : IAuthenticationProvider<Token>
 {
     private readonly GameServerConfig? _config;
+    private readonly Logger _logger;
 
-    public GameAuthenticationProvider(GameServerConfig? config)
+    public GameAuthenticationProvider(GameServerConfig? config, Logger logger)
     {
         this._config = config;
+        this._logger = logger;
     }
 
     public Token? AuthenticateToken(ListenerContext request, Lazy<IDatabaseContext> db)
@@ -71,6 +75,23 @@ public GameAuthenticationProvider(GameServerConfig? config)
         // we don't actually receive tokens in endpoints (except during logout, aka token revocation)
         if ((this._config?.MaintenanceMode ?? false) && user.Role != GameUserRole.Admin)
             return null;
+        
+        // Additional validation of the token gotten from DB. Exceptions will be caught, logged and InternalServerError will be returned automatically.
+        if (token.TokenData != tokenData)
+        {
+#if DEBUG
+            if(Debugger.IsAttached) Debugger.Break();
+#endif
+            throw new InvalidDataException($"{typeof(GameAuthenticationProvider)} - Token from DB does not match token received from client!");
+        }
+
+        if (token.User.UserId != token.UserId)
+        {
+#if DEBUG
+            if(Debugger.IsAttached) Debugger.Break();
+#endif
+            throw new InvalidDataException($"{typeof(GameAuthenticationProvider)} - GameUser included with token is not the token owner!");
+        }
 
         return token;
     }

Refresh.GameServer/RefreshGameServer.cs

@@ -93,7 +93,7 @@ public RefreshGameServer(
 
             this.WorkerManager?.Stop();
 
-            authProvider ??= new GameAuthenticationProvider(this._configStore.GameServer);
+            authProvider ??= new GameAuthenticationProvider(this._configStore.GameServer, this.Logger);
 
             this.InjectBaseServices(provider, authProvider, this._dataStore);
         });
@@ -107,7 +107,7 @@ protected virtual ConfigStore CreateConfigStore()
     private void InjectBaseServices(GameDatabaseProvider databaseProvider, IAuthenticationProvider<Token> authProvider, IDataStore dataStore)
     {
         this.Server.UseDatabaseProvider(databaseProvider);
-        this.Server.AddAuthenticationService(authProvider, true);
+        this.Server.AddService(new GameAuthenticationService(this.Server.Logger, authProvider));
         this.Server.AddStorageService(dataStore);
     }
 
@@ -141,7 +141,7 @@ protected override void SetupConfiguration()
     protected override void SetupServices()
     {
         this.Server.AddService<TimeProviderService>(this.GetTimeProvider());
-        this.Server.AddRateLimitService(new RateLimitSettings(90, 380, 45, "global"));
+        this.Server.AddService<GameRateLimitService>(new RateLimiter(new RateLimitSettings(90, 380, 45, "global")));
         this.Server.AddService<CategoryService>();
         this.Server.AddService(new MatchService(this.Server.Logger, this._configStore.GameServer));
         this.Server.AddService<ImportService>();

Refresh.Interfaces.APIv3/Endpoints/AuthenticationApiEndpoints.cs

@@ -1,3 +1,4 @@
+using System.Diagnostics;
 using System.Net;
 using AttribDoc.Attributes;
 using Bunkum.Core;
@@ -105,6 +106,21 @@ public ApiResponse<IApiAuthenticationResponse> Authenticate(RequestContext conte
 
         Token token = database.GenerateTokenForUser(user, TokenType.Api, TokenGame.Website, TokenPlatform.Website, ipAddress);
         Token refreshToken = database.GenerateTokenForUser(user, TokenType.ApiRefresh, TokenGame.Website, TokenPlatform.Website, ipAddress, GameDatabaseContext.RefreshTokenExpirySeconds);
+
+        if (user.UserId != token.UserId)
+        {
+#if DEBUG
+            if(Debugger.IsAttached) Debugger.Break();
+#endif
+            throw new InvalidDataException($"API login - API token owner ({token.User}) does not match user received from DB ({user})!");
+        }
+        if (user.UserId != refreshToken.UserId)
+        {
+#if DEBUG
+            if(Debugger.IsAttached) Debugger.Break();
+#endif
+            throw new InvalidDataException($"API login - Refresh token owner ({refreshToken.User}) does not match user received from DB ({user})!");
+        }
 
         context.Logger.LogInfo(BunkumCategory.Authentication, $"{user} successfully logged in through the API");
 
@@ -131,8 +147,15 @@ public ApiResponse<IApiAuthenticationResponse> RefreshToken(RequestContext conte
         GameUser user = refreshToken.User;
 
         Token token = database.GenerateTokenForUser(user, TokenType.Api, TokenGame.Website, TokenPlatform.Website, context.RemoteIp());
+        if (token.UserId != refreshToken.UserId)
+        {
+#if DEBUG
+            if(Debugger.IsAttached) Debugger.Break();
+#endif
+            throw new InvalidDataException($"RefreshToken - Owner of new token ({token.User}) does not match owner of refresh token ({refreshToken.User})!");
+        }
+
         database.ResetApiRefreshTokenExpiry(refreshToken);
-        
         context.Logger.LogInfo(BunkumCategory.Authentication, $"{user} successfully refreshed their token through the API");
 
         return new ApiAuthenticationResponse