fix: security + robustness audit (auth bypass, server-list DoS, JSON injection, input validation)

[itpick]

Audit pass over the master server turning up several correctness/security bugs, each fixed as its own commit.

Security / correctness

• Game-server ownership auth bypass — MatchmakingController update/delete/heartbeat/updatePlayers/removePlayer/changeStarted computed Unauthorized() but discarded the result (no return), so any authenticated session could modify or delete another client's game server and edit its player list.
• Server-list DoS — GameServerAttributes.ToJObject left null keys / cast by key-suffix, so one server registering an attribute whose key didn't end in _b/_i/_s (or mismatched its stored type) 500'd the whole server list. Now emits by actual stored type and skips others.
• JSON injection — CloudStorage's generated stats.json interpolated the username into raw JSON (usernames allow "/\). Built via JObject now.
• Unbounded anonymous query — RatingsController.GetRankings ([AllowAnonymous]) had no limit clamp (limit=0 = unlimited in Mongo) and passed negative skip. Clamped to skip>=0, 1<=limit<=100.
• Negative XP — ProfileController.GrantXP only bounded the upper side; a negative amount lowered any account's XP/level. Clamped to >=0.
• Forbid(message) in AdminPanelController treated the message as an auth scheme name (throws 500 instead of 403) → StatusCode(403, ...).

Robustness / resources

• AccountController.RegisterAccount created a new undisposed HttpClient per registration (socket exhaustion) and interpolated the caller's recaptcha token unescaped → shared static readonly HttpClient + Uri.EscapeDataString.
• MatchmakingWaitTimeEstimateService (singleton) mutated a Dictionary/List from concurrent requests with no locking → lock.
• StatisticBaseInputFormatter blindly chopped the last body char (empty body → 500, missing trailing NUL → truncated JSON) → TrimEnd('\0') + Failure() (400) on empty/invalid.
• ApplicationBackgroundService cleanup ran in Task.Run with no catch (any failure silently killed cleanup) → catch/log.
• RatingsService.GetSelectedRankingAsync deref'd a possibly-null SingleOrDefaultAsync account → placeholder fallback (matches GetRankingsAsync).
• Successful admin password change was logged as a Warning "not authorized" (log level never updated).
• Program.cs: UseAuthorization() ran before UseAuthentication() (benign today, ordered correctly to avoid future breakage).

Perf

• UpdateGameServerPlayers: Where(...).Any() → List.Contains, dropped redundant pre-Remove existence checks.

Reviewed against net6.0 semantics; no functional change to happy paths. Independent of #249.

[Saibamen]

Please add tests for your changes

[Copilot]

Pull request overview

This PR applies a security + robustness audit across the UT4 master server API surface, fixing several issues that could enable unauthorized game-server mutation, denial-of-service conditions in server listing, and unsafe JSON construction, while also tightening input validation and concurrency behavior.

Changes:

• Fix authorization enforcement gaps in MatchmakingController (previously computed Unauthorized() but continued execution).
• Harden several endpoints/services against malformed or abusive inputs (paging bounds, negative XP, safer request-body parsing).
• Improve robustness and resource usage (shared HttpClient, background cleanup exception handling, locking for singleton state, safer JSON emission/serialization).

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
UT4MasterServer/Program.cs	Corrects middleware ordering (UseAuthentication() before UseAuthorization()).
UT4MasterServer/Formatters/StatisticBaseInputFormatter.cs	Makes request-body parsing resilient to empty bodies / missing trailing NUL and invalid JSON.
UT4MasterServer/Controllers/UT/RatingsController.cs	Clamps anonymous paging parameters to prevent unbounded queries.
UT4MasterServer/Controllers/UT/ProfileController.cs	Prevents negative XP from reducing account XP/level.
UT4MasterServer/Controllers/UT/MatchmakingController.cs	Fixes auth bypass by returning Unauthorized(); minor player-list update perf cleanup.
UT4MasterServer/Controllers/Epic/CloudStorageController.cs	Prevents JSON injection when generating stats.json by building JSON via JObject.
UT4MasterServer/Controllers/Epic/AccountController.cs	Reuses a shared HttpClient and escapes recaptcha query params.
UT4MasterServer/Controllers/AdminPanelController.cs	Fixes incorrect Forbid(message) usage and corrects password-change log level on success.
UT4MasterServer.Services/Singleton/MatchmakingWaitTimeEstimateService.cs	Adds locking to prevent concurrent mutation of singleton-held collections.
UT4MasterServer.Services/Scoped/RatingsService.cs	Avoids null-deref when account lookup returns null; returns placeholder values.
UT4MasterServer.Services/Hosted/ApplicationBackgroundService.cs	Ensures background cleanup exceptions are caught and logged instead of silently killing cleanup.
UT4MasterServer.Models/GameServerAttributes.cs	Makes server-attribute JSON emission robust to unexpected keys/types to avoid crashing server-list serialization.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[itpick]

Addressed both the review comments and added tests.

Copilot review fixes (commit 3139c70):

• StatisticBaseInputFormatter — a body of the JSON literal null now returns InputFormatterResult.Failure() instead of Success(null) (was surfacing as a later null-ref/500).
• AccountController.RegisterAccount — the reCaptcha HttpResponseMessage is now a using declaration so it's disposed and returns the connection to the pool.

Tests (767789e extract testable helpers, fd2a6cd tests) — I focused on the changes that are unit-testable without a DI/DB/auth harness (the project has no mocking lib):

• GameServerAttributeTest — locks in the server-list-DoS fix: ToJObject() emits by actual stored type (bool/int/string), doesn't throw on a key without a _b/_i/_s suffix, and handles suffix/type mismatches — the cases that used to 500 the whole server list.
• StatisticBaseInputFormatterTest — the body parser (empty / trailing-NUL / null literal / malformed / array / string → failure; valid bodies → parsed), via a small extracted TryParse helper.
• RatingsControllerTest — ClampPaging boundaries (skip≥0, 1≤limit≤100) for the anonymous rankings endpoint.

To enable those two I extracted the pure logic into small internal static helpers (behavior unchanged) and added InternalsVisibleTo("XUnit.Tests").

Verified locally: solution builds clean and all the new tests pass. (The pre-existing EloTest.TestTeamEloBalancing statistical failures are unrelated — that test and the Elo code aren't touched by this PR.)

The remaining fixes are integration-level (Matchmaking ownership returns, ProfileController.GrantXP clamp, AdminPanel/CloudStorage, the hosted/singleton services) — they need mocked services/DB/auth, so I left them uncovered rather than add a mocking dependency in this PR. Happy to follow up if you'd prefer a test harness for those.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (1)

UT4MasterServer/Formatters/StatisticBaseInputFormatter.cs:24

• ReadRequestBodyAsync returns InputFormatterResult.Failure() on parse failure but does not add a ModelState error. In ASP.NET Core, formatters are expected to record a model-state error themselves; otherwise the request may proceed with an unbound/null body (risking a 500 in StatsController.CreateAccountStatistics) instead of producing a 400 as intended.

		if (!TryParse(rawValue, out StatisticBase? newObject))
		{
			return InputFormatterResult.Failure();
		}