From 685742012c560e72189296d1e5ce609f280bf7dd Mon Sep 17 00:00:00 2001
From: Dominion <Cyberboss@users.noreply.github.com>
Date: Thu, 8 Jun 2023 04:02:01 -0400
Subject: [PATCH] Clean up side effects of #1528

---
 .../Controllers/HomeController.cs             | 37 ++++++++++++++-----
 .../Controllers/UserController.cs             | 13 ++++++-
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/src/Tgstation.Server.Host/Controllers/HomeController.cs b/src/Tgstation.Server.Host/Controllers/HomeController.cs
index 0bce69891c..731be3182f 100644
--- a/src/Tgstation.Server.Host/Controllers/HomeController.cs
+++ b/src/Tgstation.Server.Host/Controllers/HomeController.cs
@@ -297,6 +297,7 @@ namespace Tgstation.Server.Host.Controllers
 						PasswordHash = x.PasswordHash,
 						Enabled = x.Enabled,
 						Name = x.Name,
+						SystemIdentifier = x.SystemIdentifier,
 					})
 					.ToListAsync(cancellationToken);
 
@@ -315,13 +316,13 @@ namespace Tgstation.Server.Host.Controllers
 				// FALLBACK TO THE DB USER HERE, DO NOT REVEAL A SYSTEM LOGIN!!!
 				// This of course, allows system users to discover TGS users in this (HIGHLY IMPROBABLE) case but that is not our fault
 				var originalHash = user.PasswordHash;
-				var isDbUser = originalHash != null;
-				bool usingSystemIdentity = systemIdentity != null && !isDbUser;
+				var isLikelyDbUser = originalHash != null;
+				bool usingSystemIdentity = systemIdentity != null && !isLikelyDbUser;
 				if (!oAuthLogin)
 					if (!usingSystemIdentity)
 					{
 						// DB User password check and update
-						if (originalHash == null || !cryptographySuite.CheckUserPassword(user, ApiHeaders.Password))
+						if (!isLikelyDbUser || !cryptographySuite.CheckUserPassword(user, ApiHeaders.Password))
 							return Unauthorized();
 						if (user.PasswordHash != originalHash)
 						{
@@ -335,14 +336,30 @@ namespace Tgstation.Server.Host.Controllers
 							await DatabaseContext.Save(cancellationToken);
 						}
 					}
-					else if (systemIdentity.Username != user.Name)
+					else
 					{
-						// System identity username change update
-						Logger.LogDebug("User ID {userId}'s system identity needs a refresh, updating database.", user.Id);
-						DatabaseContext.Users.Attach(user);
-						user.Name = systemIdentity.Username;
-						user.CanonicalName = Models.User.CanonicalizeName(user.Name);
-						await DatabaseContext.Save(cancellationToken);
+						var usernameMismatch = systemIdentity.Username != user.Name;
+						if (isLikelyDbUser || usernameMismatch)
+						{
+							DatabaseContext.Users.Attach(user);
+							if (isLikelyDbUser)
+							{
+								// cleanup from https://github.com/tgstation/tgstation-server/issues/1528
+								Logger.LogDebug("System user ID {userId}'s PasswordHash is polluted, updating database.", user.Id);
+								user.PasswordHash = null;
+								user.LastPasswordUpdate = DateTimeOffset.UtcNow;
+							}
+
+							if (usernameMismatch)
+							{
+								// System identity username change update
+								Logger.LogDebug("User ID {userId}'s system identity needs a refresh, updating database.", user.Id);
+								user.Name = systemIdentity.Username;
+								user.CanonicalName = Models.User.CanonicalizeName(user.Name);
+							}
+
+							await DatabaseContext.Save(cancellationToken);
+						}
 					}
 
 				// Now that the bookeeping is done, tell them to fuck off if necessary
diff --git a/src/Tgstation.Server.Host/Controllers/UserController.cs b/src/Tgstation.Server.Host/Controllers/UserController.cs
index d5769889da..6b4dee0594 100644
--- a/src/Tgstation.Server.Host/Controllers/UserController.cs
+++ b/src/Tgstation.Server.Host/Controllers/UserController.cs
@@ -144,7 +144,7 @@ namespace Tgstation.Server.Host.Controllers
 
 			await DatabaseContext.Save(cancellationToken);
 
-			Logger.LogInformation("Created new user {0} ({1})", dbUser.Name, dbUser.Id);
+			Logger.LogInformation("Created new user {name} ({id})", dbUser.Name, dbUser.Id);
 
 			return Created(dbUser.ToApi());
 		}
@@ -214,12 +214,21 @@ namespace Tgstation.Server.Host.Controllers
 				|| (!oAuthEdit && model.OAuthConnections != null))
 				return Forbid();
 
+			var originalUserHasSid = originalUser.SystemIdentifier != null;
+			if (originalUserHasSid && originalUser.PasswordHash != null)
+			{
+				// cleanup from https://github.com/tgstation/tgstation-server/issues/1528
+				Logger.LogDebug("System user ID {userId}'s PasswordHash is polluted, updating database.", originalUser.Id);
+				originalUser.PasswordHash = null;
+				originalUser.LastPasswordUpdate = DateTimeOffset.UtcNow;
+			}
+
 			if (model.SystemIdentifier != null && model.SystemIdentifier != originalUser.SystemIdentifier)
 				return BadRequest(new ErrorMessageResponse(ErrorCode.UserSidChange));
 
 			if (model.Password != null)
 			{
-				if (originalUser.SystemIdentifier != null)
+				if (originalUserHasSid)
 					return BadRequest(new ErrorMessageResponse(ErrorCode.UserMismatchPasswordSid));
 
 				var result = TrySetPassword(originalUser, model.Password, false);