From 4ba75f09a9c16047d6cdc48a2fae5b84dbaf37bc Mon Sep 17 00:00:00 2001 From: jvyden Date: Fri, 29 Jul 2022 15:08:41 -0400 Subject: [PATCH] Make all tokens expire Closes #335 --- .config/dotnet-tools.json | 2 +- .../Controllers/LoginController.cs | 8 +- .../Controllers/MessageController.cs | 2 + .../Pages/LoginForm.cshtml.cs | 2 + .../Pages/RegisterForm.cshtml.cs | 1 + .../Pages/SetEmailForm.cshtml.cs | 3 + .../Tests/AdminTests.cs | 2 + .../Tests/AuthenticationTests.cs | 1 + ProjectLighthouse/Database.cs | 60 +++++++++++++- .../Extensions/DatabaseExtensions.cs | 7 +- .../20220729182709_AddExpiryTimesToTokens.cs | 80 +++++++++++++++++++ .../Migrations/DatabaseModelSnapshot.cs | 18 +++++ ProjectLighthouse/PlayerData/GameToken.cs | 3 + .../Profiles/Email/EmailSetToken.cs | 3 + .../Profiles/Email/EmailVerificationToken.cs | 3 + ProjectLighthouse/PlayerData/WebToken.cs | 3 + 16 files changed, 188 insertions(+), 10 deletions(-) create mode 100644 ProjectLighthouse/Migrations/20220729182709_AddExpiryTimesToTokens.cs diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index e15fce7d..284937e9 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -3,7 +3,7 @@ "isRoot": true, "tools": { "dotnet-ef": { - "version": "6.0.5", + "version": "6.0.7", "commands": [ "dotnet-ef" ] diff --git a/ProjectLighthouse.Servers.GameServer/Controllers/LoginController.cs b/ProjectLighthouse.Servers.GameServer/Controllers/LoginController.cs index 056a5cfe..8273c408 100644 --- a/ProjectLighthouse.Servers.GameServer/Controllers/LoginController.cs +++ b/ProjectLighthouse.Servers.GameServer/Controllers/LoginController.cs @@ -57,9 +57,10 @@ public class LoginController : ControllerBase string ipAddress = remoteIpAddress.ToString(); + await this.database.RemoveExpiredTokens(); + // Get an existing token from the IP & username - GameToken? token = await this.database.GameTokens.Include - (t => t.User) + GameToken? token = await this.database.GameTokens.Include(t => t.User) .FirstOrDefaultAsync(t => t.UserLocation == ipAddress && t.User.Username == npTicket.Username && !t.Used); if (token == null) // If we cant find an existing token, try to generate a new one @@ -67,7 +68,8 @@ public class LoginController : ControllerBase token = await this.database.AuthenticateUser(npTicket, ipAddress); if (token == null) { - Logger.Warn($"Unable to find/generate a token for username {npTicket.Username}", LogArea.Login); + Logger.Warn($"Unable to " + + $"find/generate a token for username {npTicket.Username}", LogArea.Login); return this.StatusCode(403, ""); // If not, then 403. } } diff --git a/ProjectLighthouse.Servers.GameServer/Controllers/MessageController.cs b/ProjectLighthouse.Servers.GameServer/Controllers/MessageController.cs index 63067c7a..745ec299 100644 --- a/ProjectLighthouse.Servers.GameServer/Controllers/MessageController.cs +++ b/ProjectLighthouse.Servers.GameServer/Controllers/MessageController.cs @@ -1,4 +1,5 @@ #nullable enable +using System.Globalization; using LBPUnion.ProjectLighthouse.Configuration; using LBPUnion.ProjectLighthouse.Helpers; using LBPUnion.ProjectLighthouse.Logging; @@ -75,6 +76,7 @@ along with this program. If not, see ."; $"token.Used: {gameToken.Used}\n" + $"token.UserLocation: {gameToken.UserLocation}\n" + $"token.GameVersion: {gameToken.GameVersion}\n" + + $"token.ExpiresAt: {gameToken.ExpiresAt.ToString(CultureInfo.CurrentCulture)}\n" + "---DEBUG INFO---" + #endif "\n" diff --git a/ProjectLighthouse.Servers.Website/Pages/LoginForm.cshtml.cs b/ProjectLighthouse.Servers.Website/Pages/LoginForm.cshtml.cs index 662f1f5b..5c6ea815 100644 --- a/ProjectLighthouse.Servers.Website/Pages/LoginForm.cshtml.cs +++ b/ProjectLighthouse.Servers.Website/Pages/LoginForm.cshtml.cs @@ -73,6 +73,7 @@ public class LoginForm : BaseLayout UserId = user.UserId, User = user, EmailToken = CryptoHelper.GenerateAuthToken(), + ExpiresAt = DateTime.Now + TimeSpan.FromHours(6), }; this.Database.EmailSetTokens.Add(emailSetToken); @@ -85,6 +86,7 @@ public class LoginForm : BaseLayout { UserId = user.UserId, UserToken = CryptoHelper.GenerateAuthToken(), + ExpiresAt = DateTime.Now + TimeSpan.FromDays(7), }; this.Database.WebTokens.Add(webToken); diff --git a/ProjectLighthouse.Servers.Website/Pages/RegisterForm.cshtml.cs b/ProjectLighthouse.Servers.Website/Pages/RegisterForm.cshtml.cs index 4bbe3efb..8789285c 100644 --- a/ProjectLighthouse.Servers.Website/Pages/RegisterForm.cshtml.cs +++ b/ProjectLighthouse.Servers.Website/Pages/RegisterForm.cshtml.cs @@ -94,6 +94,7 @@ public class RegisterForm : BaseLayout { UserId = user.UserId, UserToken = CryptoHelper.GenerateAuthToken(), + ExpiresAt = DateTime.Now + TimeSpan.FromDays(7), }; this.Database.WebTokens.Add(webToken); diff --git a/ProjectLighthouse.Servers.Website/Pages/SetEmailForm.cshtml.cs b/ProjectLighthouse.Servers.Website/Pages/SetEmailForm.cshtml.cs index 05d57ee3..01ca1262 100644 --- a/ProjectLighthouse.Servers.Website/Pages/SetEmailForm.cshtml.cs +++ b/ProjectLighthouse.Servers.Website/Pages/SetEmailForm.cshtml.cs @@ -49,15 +49,18 @@ public class SetEmailForm : BaseLayout UserId = user.UserId, User = user, EmailToken = CryptoHelper.GenerateAuthToken(), + ExpiresAt = DateTime.Now + TimeSpan.FromHours(6), }; this.Database.EmailVerificationTokens.Add(emailVerifyToken); // The user just set their email address. Now, let's grant them a token to proceed with verifying the email. + // TODO: insecure WebToken webToken = new() { UserId = user.UserId, UserToken = CryptoHelper.GenerateAuthToken(), + ExpiresAt = DateTime.Now + TimeSpan.FromDays(7), }; this.Response.Cookies.Append diff --git a/ProjectLighthouse.Tests.WebsiteTests/Tests/AdminTests.cs b/ProjectLighthouse.Tests.WebsiteTests/Tests/AdminTests.cs index 9c137086..0b38cdbc 100644 --- a/ProjectLighthouse.Tests.WebsiteTests/Tests/AdminTests.cs +++ b/ProjectLighthouse.Tests.WebsiteTests/Tests/AdminTests.cs @@ -25,6 +25,7 @@ public class AdminTests : LighthouseWebTest { UserId = user.UserId, UserToken = CryptoHelper.GenerateAuthToken(), + ExpiresAt = DateTime.Now + TimeSpan.FromHours(1), }; database.WebTokens.Add(webToken); @@ -49,6 +50,7 @@ public class AdminTests : LighthouseWebTest { UserId = user.UserId, UserToken = CryptoHelper.GenerateAuthToken(), + ExpiresAt = DateTime.Now + TimeSpan.FromHours(1), }; database.WebTokens.Add(webToken); diff --git a/ProjectLighthouse.Tests.WebsiteTests/Tests/AuthenticationTests.cs b/ProjectLighthouse.Tests.WebsiteTests/Tests/AuthenticationTests.cs index af4c9af2..f85d0c99 100644 --- a/ProjectLighthouse.Tests.WebsiteTests/Tests/AuthenticationTests.cs +++ b/ProjectLighthouse.Tests.WebsiteTests/Tests/AuthenticationTests.cs @@ -88,6 +88,7 @@ public class AuthenticationTests : LighthouseWebTest { UserId = user.UserId, UserToken = CryptoHelper.GenerateAuthToken(), + ExpiresAt = DateTime.Now + TimeSpan.FromHours(1), }; database.WebTokens.Add(webToken); diff --git a/ProjectLighthouse/Database.cs b/ProjectLighthouse/Database.cs index f6fad8b5..f3dd283a 100644 --- a/ProjectLighthouse/Database.cs +++ b/ProjectLighthouse/Database.cs @@ -6,6 +6,7 @@ using System.Threading.Tasks; using LBPUnion.ProjectLighthouse.Administration; using LBPUnion.ProjectLighthouse.Administration.Reports; using LBPUnion.ProjectLighthouse.Configuration; +using LBPUnion.ProjectLighthouse.Extensions; using LBPUnion.ProjectLighthouse.Helpers; using LBPUnion.ProjectLighthouse.Levels; using LBPUnion.ProjectLighthouse.Levels.Categories; @@ -111,6 +112,8 @@ public class Database : DbContext UserLocation = userLocation, GameVersion = npTicket.GameVersion, Platform = npTicket.Platform, + // we can get away with a low expiry here since LBP will just get a new token everytime it gets 403'd + ExpiresAt = DateTime.Now + TimeSpan.FromHours(1), }; this.GameTokens.Add(gameToken); @@ -289,6 +292,13 @@ public class Database : DbContext if (token == null) return null; if (!allowUnapproved && !token.Approved) return null; + if (DateTime.Now > token.ExpiresAt) + { + this.Remove(token); + await this.SaveChangesAsync(); + return null; + } + return await this.Users.Include(u => u.Location).FirstOrDefaultAsync(u => u.UserId == token.UserId); } @@ -314,6 +324,13 @@ public class Database : DbContext if (token == null) return null; if (!allowUnapproved && !token.Approved) return null; + if (DateTime.Now > token.ExpiresAt) + { + this.Remove(token); + await this.SaveChangesAsync(); + return null; + } + return token; } @@ -326,6 +343,13 @@ public class Database : DbContext if (token == null) return null; if (!allowUnapproved && !token.Approved) return null; + if (DateTime.Now > token.ExpiresAt) + { + this.Remove(token); + await this.SaveChangesAsync(); + return null; + } + User? user = await this.UserFromGameToken(token); if (user == null) return null; @@ -342,6 +366,13 @@ public class Database : DbContext WebToken? token = this.WebTokens.FirstOrDefault(t => t.UserToken == lighthouseToken); if (token == null) return null; + if (DateTime.Now > token.ExpiresAt) + { + this.Remove(token); + this.SaveChanges(); + return null; + } + return this.Users.Include(u => u.Location).FirstOrDefault(u => u.UserId == token.UserId); } @@ -356,12 +387,21 @@ public class Database : DbContext { if (!request.Cookies.TryGetValue("LighthouseToken", out string? lighthouseToken) || lighthouseToken == null) return null; - return this.WebTokens.FirstOrDefault(t => t.UserToken == lighthouseToken); + WebToken? token = this.WebTokens.FirstOrDefault(t => t.UserToken == lighthouseToken); + if (token == null) return null; + + if (DateTime.Now > token.ExpiresAt) + { + this.Remove(token); + this.SaveChanges(); + return null; + } + + return token; } public async Task UserFromPasswordResetToken(string resetToken) { - PasswordResetToken? token = await this.PasswordResetTokens.FirstOrDefaultAsync(token => token.ResetToken == resetToken); if (token == null) { @@ -371,8 +411,10 @@ public class Database : DbContext if (token.Created < DateTime.Now.AddHours(-1)) // if token is expired { this.PasswordResetTokens.Remove(token); + await this.SaveChangesAsync(); return null; } + return await this.Users.FirstOrDefaultAsync(user => user.UserId == token.UserId); } @@ -385,12 +427,23 @@ public class Database : DbContext if (token.Created < DateTime.Now.AddDays(-7)) // if token is expired { this.RegistrationTokens.Remove(token); + this.SaveChanges(); return false; } return true; } + public async Task RemoveExpiredTokens() + { + this.GameTokens.RemoveWhere(t => DateTime.Now > t.ExpiresAt); + this.WebTokens.RemoveWhere(t => DateTime.Now > t.ExpiresAt); + this.EmailVerificationTokens.RemoveWhere(t => DateTime.Now > t.ExpiresAt); + this.EmailSetTokens.RemoveWhere(t => DateTime.Now > t.ExpiresAt); + + await this.SaveChangesAsync(); + } + public async Task RemoveRegistrationToken(string tokenString) { RegistrationToken? token = await this.RegistrationTokens.FirstOrDefaultAsync(t => t.Token == tokenString); @@ -398,7 +451,6 @@ public class Database : DbContext if (token == null) return; this.RegistrationTokens.Remove(token); - await this.SaveChangesAsync(); } @@ -426,10 +478,10 @@ public class Database : DbContext this.RatedLevels.RemoveRange(this.RatedLevels.Where(r => r.UserId == user.UserId)); this.GameTokens.RemoveRange(this.GameTokens.Where(t => t.UserId == user.UserId)); this.WebTokens.RemoveRange(this.WebTokens.Where(t => t.UserId == user.UserId)); + this.Reactions.RemoveRange(this.Reactions.Where(p => p.UserId == user.UserId)); this.Comments.RemoveRange(this.Comments.Where(c => c.PosterUserId == user.UserId)); this.Reviews.RemoveRange(this.Reviews.Where(r => r.ReviewerId == user.UserId)); this.Photos.RemoveRange(this.Photos.Where(p => p.CreatorId == user.UserId)); - this.Reactions.RemoveRange(this.Reactions.Where(p => p.UserId == user.UserId)); this.Users.Remove(user); diff --git a/ProjectLighthouse/Extensions/DatabaseExtensions.cs b/ProjectLighthouse/Extensions/DatabaseExtensions.cs index a7e27782..67748eff 100644 --- a/ProjectLighthouse/Extensions/DatabaseExtensions.cs +++ b/ProjectLighthouse/Extensions/DatabaseExtensions.cs @@ -56,6 +56,9 @@ public static class DatabaseExtensions return query; } - public static async Task Has(this IQueryable queryable, Expression> predicate) => - await queryable.FirstOrDefaultAsync(predicate) != null; + public static async Task Has(this IQueryable queryable, Expression> predicate) + => await queryable.FirstOrDefaultAsync(predicate) != null; + + public static void RemoveWhere(this DbSet queryable, Expression> predicate) where T : class + => queryable.RemoveRange(queryable.Where(predicate)); } \ No newline at end of file diff --git a/ProjectLighthouse/Migrations/20220729182709_AddExpiryTimesToTokens.cs b/ProjectLighthouse/Migrations/20220729182709_AddExpiryTimesToTokens.cs new file mode 100644 index 00000000..1560ad99 --- /dev/null +++ b/ProjectLighthouse/Migrations/20220729182709_AddExpiryTimesToTokens.cs @@ -0,0 +1,80 @@ +using System; +using LBPUnion.ProjectLighthouse; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace ProjectLighthouse.Migrations +{ + [DbContext(typeof(Database))] + [Migration("20220729182709_AddExpiryTimesToTokens")] + public partial class AddExpiryTimesToTokens : Migration + { + protected override void Up(MigrationBuilder migrationBuilder) + { + // Remove existing tokens + migrationBuilder.Sql("DELETE FROM GameTokens;"); + migrationBuilder.Sql("DELETE FROM WebTokens;"); + migrationBuilder.Sql("DELETE FROM EmailSetTokens;"); + migrationBuilder.Sql("DELETE FROM EmailVerificationTokens;"); + migrationBuilder.Sql("DELETE FROM PasswordResetTokens;"); + migrationBuilder.Sql("DELETE FROM RegistrationTokens;"); + + migrationBuilder.AddColumn( + name: "ExpiresAt", + table: "WebTokens", + type: "datetime(6)", + nullable: false, + defaultValue: new DateTime(1, 1, 1, 0, 0, 0, 0, DateTimeKind.Unspecified)); + + migrationBuilder.AddColumn( + name: "ExpiresAt", + table: "GameTokens", + type: "datetime(6)", + nullable: false, + defaultValue: new DateTime(1, 1, 1, 0, 0, 0, 0, DateTimeKind.Unspecified)); + + migrationBuilder.AddColumn( + name: "ExpiresAt", + table: "EmailVerificationTokens", + type: "datetime(6)", + nullable: false, + defaultValue: new DateTime(1, 1, 1, 0, 0, 0, 0, DateTimeKind.Unspecified)); + + migrationBuilder.AddColumn( + name: "ExpiresAt", + table: "EmailSetTokens", + type: "datetime(6)", + nullable: false, + defaultValue: new DateTime(1, 1, 1, 0, 0, 0, 0, DateTimeKind.Unspecified)); + } + + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropColumn( + name: "ExpiresAt", + table: "WebTokens"); + + migrationBuilder.DropColumn( + name: "ExpiresAt", + table: "RegistrationTokens"); + + migrationBuilder.DropColumn( + name: "ExpiresAt", + table: "PasswordResetTokens"); + + migrationBuilder.DropColumn( + name: "ExpiresAt", + table: "GameTokens"); + + migrationBuilder.DropColumn( + name: "ExpiresAt", + table: "EmailVerificationTokens"); + + migrationBuilder.DropColumn( + name: "ExpiresAt", + table: "EmailSetTokens"); + } + } +} diff --git a/ProjectLighthouse/Migrations/DatabaseModelSnapshot.cs b/ProjectLighthouse/Migrations/DatabaseModelSnapshot.cs index 53cd19d1..9a2d1729 100644 --- a/ProjectLighthouse/Migrations/DatabaseModelSnapshot.cs +++ b/ProjectLighthouse/Migrations/DatabaseModelSnapshot.cs @@ -385,6 +385,9 @@ namespace ProjectLighthouse.Migrations b.Property("Approved") .HasColumnType("tinyint(1)"); + b.Property("ExpiresAt") + .HasColumnType("datetime(6)"); + b.Property("GameVersion") .HasColumnType("int"); @@ -419,6 +422,9 @@ namespace ProjectLighthouse.Migrations b.Property("Created") .HasColumnType("datetime(6)"); + b.Property("ExpiresAt") + .HasColumnType("datetime(6)"); + b.Property("ResetToken") .HasColumnType("longtext"); @@ -540,6 +546,9 @@ namespace ProjectLighthouse.Migrations b.Property("EmailToken") .HasColumnType("longtext"); + b.Property("ExpiresAt") + .HasColumnType("datetime(6)"); + b.Property("UserId") .HasColumnType("int"); @@ -559,6 +568,9 @@ namespace ProjectLighthouse.Migrations b.Property("EmailToken") .HasColumnType("longtext"); + b.Property("ExpiresAt") + .HasColumnType("datetime(6)"); + b.Property("UserId") .HasColumnType("int"); @@ -731,6 +743,9 @@ namespace ProjectLighthouse.Migrations b.Property("Created") .HasColumnType("datetime(6)"); + b.Property("ExpiresAt") + .HasColumnType("datetime(6)"); + b.Property("Token") .HasColumnType("longtext"); @@ -841,6 +856,9 @@ namespace ProjectLighthouse.Migrations .ValueGeneratedOnAdd() .HasColumnType("int"); + b.Property("ExpiresAt") + .HasColumnType("datetime(6)"); + b.Property("UserId") .HasColumnType("int"); diff --git a/ProjectLighthouse/PlayerData/GameToken.cs b/ProjectLighthouse/PlayerData/GameToken.cs index dd83924a..1317b34f 100644 --- a/ProjectLighthouse/PlayerData/GameToken.cs +++ b/ProjectLighthouse/PlayerData/GameToken.cs @@ -1,3 +1,4 @@ +using System; using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; using LBPUnion.ProjectLighthouse.PlayerData.Profiles; @@ -28,4 +29,6 @@ public class GameToken // Set to true on login public bool Used { get; set; } + + public DateTime ExpiresAt { get; set; } } \ No newline at end of file diff --git a/ProjectLighthouse/PlayerData/Profiles/Email/EmailSetToken.cs b/ProjectLighthouse/PlayerData/Profiles/Email/EmailSetToken.cs index 152e42ed..879efce9 100644 --- a/ProjectLighthouse/PlayerData/Profiles/Email/EmailSetToken.cs +++ b/ProjectLighthouse/PlayerData/Profiles/Email/EmailSetToken.cs @@ -1,3 +1,4 @@ +using System; using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; @@ -14,4 +15,6 @@ public class EmailSetToken public User User { get; set; } public string EmailToken { get; set; } + + public DateTime ExpiresAt { get; set; } } \ No newline at end of file diff --git a/ProjectLighthouse/PlayerData/Profiles/Email/EmailVerificationToken.cs b/ProjectLighthouse/PlayerData/Profiles/Email/EmailVerificationToken.cs index 61559cce..d2c9e866 100644 --- a/ProjectLighthouse/PlayerData/Profiles/Email/EmailVerificationToken.cs +++ b/ProjectLighthouse/PlayerData/Profiles/Email/EmailVerificationToken.cs @@ -1,3 +1,4 @@ +using System; using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; @@ -14,4 +15,6 @@ public class EmailVerificationToken public User User { get; set; } public string EmailToken { get; set; } + + public DateTime ExpiresAt { get; set; } } \ No newline at end of file diff --git a/ProjectLighthouse/PlayerData/WebToken.cs b/ProjectLighthouse/PlayerData/WebToken.cs index 7860a418..9ad0f5be 100644 --- a/ProjectLighthouse/PlayerData/WebToken.cs +++ b/ProjectLighthouse/PlayerData/WebToken.cs @@ -1,3 +1,4 @@ +using System; using System.ComponentModel.DataAnnotations; namespace LBPUnion.ProjectLighthouse.PlayerData; @@ -11,4 +12,6 @@ public class WebToken public int UserId { get; set; } public string UserToken { get; set; } + + public DateTime ExpiresAt { get; set; } } \ No newline at end of file