From 5bacdc6a95676eb20300ce65ebb61caa08377a1e Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 10 Dec 2024 15:36:51 +0900 Subject: [PATCH 01/13] Add `beatmapset_id` to multiplayer playlist items --- app/Models/Multiplayer/PlaylistItem.php | 2 ++ .../Multiplayer/PlaylistItemTransformer.php | 1 + ...apset_id_to_multiplayer_playlist_items.php | 29 +++++++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 database/migrations/2024_12_10_035240_add_beatmapset_id_to_multiplayer_playlist_items.php diff --git a/app/Models/Multiplayer/PlaylistItem.php b/app/Models/Multiplayer/PlaylistItem.php index 4c8e6175505..73d70ae77ba 100644 --- a/app/Models/Multiplayer/PlaylistItem.php +++ b/app/Models/Multiplayer/PlaylistItem.php @@ -16,6 +16,7 @@ * @property json|null $allowed_mods * @property Beatmap $beatmap * @property int $beatmap_id + * @property int|null $beatmapset_id * @property \Carbon\Carbon|null $created_at * @property int $id * @property int $owner_id @@ -64,6 +65,7 @@ public static function fromJsonParams(User $owner, $json) $obj->$field = $value; } + $obj->beatmapset_id = get_int($json['beatmapset_id'] ?? null); $obj->max_attempts = get_int($json['max_attempts'] ?? null); $modsHelper = app('mods'); diff --git a/app/Transformers/Multiplayer/PlaylistItemTransformer.php b/app/Transformers/Multiplayer/PlaylistItemTransformer.php index 99cd7f98d83..7caf2ab77ab 100644 --- a/app/Transformers/Multiplayer/PlaylistItemTransformer.php +++ b/app/Transformers/Multiplayer/PlaylistItemTransformer.php @@ -21,6 +21,7 @@ public function transform(PlaylistItem $item) 'id' => $item->id, 'room_id' => $item->room_id, 'beatmap_id' => $item->beatmap_id, + 'beatmapset_id' => $item->beatmapset_id, 'ruleset_id' => $item->ruleset_id, 'allowed_mods' => $item->allowed_mods, 'required_mods' => $item->required_mods, diff --git a/database/migrations/2024_12_10_035240_add_beatmapset_id_to_multiplayer_playlist_items.php b/database/migrations/2024_12_10_035240_add_beatmapset_id_to_multiplayer_playlist_items.php new file mode 100644 index 00000000000..70106407a55 --- /dev/null +++ b/database/migrations/2024_12_10_035240_add_beatmapset_id_to_multiplayer_playlist_items.php @@ -0,0 +1,29 @@ +unsignedMediumInteger('beatmapset_id')->nullable()->after('beatmap_id'); + }); + + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('multiplayer_playlist_items', function (Blueprint $table) { + $table->dropColumn('beatmapset_id'); + }); + } +}; From 68c26056d2efa5ae988eca8259720e82e4d9aa07 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 23 Dec 2024 20:11:26 +0900 Subject: [PATCH 02/13] Allow multiplayer scores with arbitrary beatmap/ruleset combinations --- .../Multiplayer/Rooms/Playlist/ScoresController.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php b/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php index ea1f7321d1f..858305be77f 100644 --- a/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php +++ b/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php @@ -8,6 +8,7 @@ use App\Exceptions\InvariantException; use App\Http\Controllers\Controller as BaseController; use App\Libraries\ClientCheck; +use App\Models\Beatmap; use App\Models\Multiplayer\PlaylistItem; use App\Models\Multiplayer\PlaylistItemUserHighScore; use App\Models\Multiplayer\Room; @@ -184,8 +185,15 @@ public function store($roomId, $playlistId) $request = \Request::instance(); $params = $request->all(); - if (get_string($params['beatmap_hash'] ?? null) !== $playlistItem->beatmap->checksum) { - throw new InvariantException(osu_trans('score_tokens.create.beatmap_hash_invalid')); + if ($playlistItem->beatmapset_id === null) { + if (get_string($params['beatmap_hash'] ?? null) !== $playlistItem->beatmap->checksum) { + throw new InvariantException(osu_trans('score_tokens.create.beatmap_hash_invalid')); + } + } else { + // Todo: Validate beatmap_hash param matches any checksum from the playlist item's beatmap set. + // Todo: Modifying the playlist item looks dodgy to me and is likely failing some internal validations. + $playlistItem->beatmap_id = Beatmap::firstWhere('checksum', get_string($params['beatmap_hash'] ?? null))->beatmap_id; + $playlistItem->ruleset_id = get_int($params['ruleset_id'] ?? null); } $buildId = ClientCheck::parseToken($request)['buildId']; From fa578457f300a9cddfbadd5e85131542ebd0b3e0 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 8 Jan 2025 15:37:27 +0900 Subject: [PATCH 03/13] Modify to boolean 'freestyle' parameter instead --- .../Multiplayer/Rooms/Playlist/ScoresController.php | 13 +++++++------ app/Models/Multiplayer/PlaylistItem.php | 4 ++-- .../Multiplayer/PlaylistItemTransformer.php | 2 +- ...add_freestyle_to_multiplayer_playlist_items.php} | 4 ++-- 4 files changed, 12 insertions(+), 11 deletions(-) rename database/migrations/{2024_12_10_035240_add_beatmapset_id_to_multiplayer_playlist_items.php => 2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php} (79%) diff --git a/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php b/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php index 858305be77f..daf52f82020 100644 --- a/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php +++ b/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php @@ -185,15 +185,16 @@ public function store($roomId, $playlistId) $request = \Request::instance(); $params = $request->all(); - if ($playlistItem->beatmapset_id === null) { + if ($playlistItem->freestyle) { + // Todo: Ensure beatmap_hash matches any beatmap from the playlist item's beatmap set. + // Todo: Ensure ruleset_id is valid (converts only allowed for id=0 otherwise must match beatmap playmode, and value within 0..3). + // Todo: Modifying the playlist item looks dodgy to me :)! + $playlistItem->beatmap_id = Beatmap::firstWhere('checksum', get_string($params['beatmap_hash'] ?? null))->beatmap_id; + $playlistItem->ruleset_id = get_int($params['ruleset_id'] ?? null); + } else { if (get_string($params['beatmap_hash'] ?? null) !== $playlistItem->beatmap->checksum) { throw new InvariantException(osu_trans('score_tokens.create.beatmap_hash_invalid')); } - } else { - // Todo: Validate beatmap_hash param matches any checksum from the playlist item's beatmap set. - // Todo: Modifying the playlist item looks dodgy to me and is likely failing some internal validations. - $playlistItem->beatmap_id = Beatmap::firstWhere('checksum', get_string($params['beatmap_hash'] ?? null))->beatmap_id; - $playlistItem->ruleset_id = get_int($params['ruleset_id'] ?? null); } $buildId = ClientCheck::parseToken($request)['buildId']; diff --git a/app/Models/Multiplayer/PlaylistItem.php b/app/Models/Multiplayer/PlaylistItem.php index 73d70ae77ba..b8ab7d8b56b 100644 --- a/app/Models/Multiplayer/PlaylistItem.php +++ b/app/Models/Multiplayer/PlaylistItem.php @@ -16,12 +16,12 @@ * @property json|null $allowed_mods * @property Beatmap $beatmap * @property int $beatmap_id - * @property int|null $beatmapset_id * @property \Carbon\Carbon|null $created_at * @property int $id * @property int $owner_id * @property int|null $playlist_order * @property json|null $required_mods + * @property bool $freestyle * @property Room $room * @property int $room_id * @property int|null $ruleset_id @@ -65,7 +65,7 @@ public static function fromJsonParams(User $owner, $json) $obj->$field = $value; } - $obj->beatmapset_id = get_int($json['beatmapset_id'] ?? null); + $obj->freestyle = get_bool($json['freestyle'] ?? false); $obj->max_attempts = get_int($json['max_attempts'] ?? null); $modsHelper = app('mods'); diff --git a/app/Transformers/Multiplayer/PlaylistItemTransformer.php b/app/Transformers/Multiplayer/PlaylistItemTransformer.php index 7caf2ab77ab..f7b331b05e9 100644 --- a/app/Transformers/Multiplayer/PlaylistItemTransformer.php +++ b/app/Transformers/Multiplayer/PlaylistItemTransformer.php @@ -21,10 +21,10 @@ public function transform(PlaylistItem $item) 'id' => $item->id, 'room_id' => $item->room_id, 'beatmap_id' => $item->beatmap_id, - 'beatmapset_id' => $item->beatmapset_id, 'ruleset_id' => $item->ruleset_id, 'allowed_mods' => $item->allowed_mods, 'required_mods' => $item->required_mods, + 'freestyle' => $item->freestyle, 'expired' => $item->expired, 'owner_id' => $item->owner_id, 'playlist_order' => $item->playlist_order, diff --git a/database/migrations/2024_12_10_035240_add_beatmapset_id_to_multiplayer_playlist_items.php b/database/migrations/2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php similarity index 79% rename from database/migrations/2024_12_10_035240_add_beatmapset_id_to_multiplayer_playlist_items.php rename to database/migrations/2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php index 70106407a55..d8c3d6bd5c9 100644 --- a/database/migrations/2024_12_10_035240_add_beatmapset_id_to_multiplayer_playlist_items.php +++ b/database/migrations/2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php @@ -12,7 +12,7 @@ public function up(): void { Schema::table('multiplayer_playlist_items', function (Blueprint $table) { - $table->unsignedMediumInteger('beatmapset_id')->nullable()->after('beatmap_id'); + $table->boolean('freestyle')->after('required_mods')->default(false); }); } @@ -23,7 +23,7 @@ public function up(): void public function down(): void { Schema::table('multiplayer_playlist_items', function (Blueprint $table) { - $table->dropColumn('beatmapset_id'); + $table->dropColumn('freestyle'); }); } }; From 4699f50af05ab3908e9f33022eee0a6dc9e85ed9 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 8 Jan 2025 19:01:00 +0900 Subject: [PATCH 04/13] Require strict types + add license header --- ...10_035240_add_freestyle_to_multiplayer_playlist_items.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/database/migrations/2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php b/database/migrations/2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php index d8c3d6bd5c9..50b6076cbab 100644 --- a/database/migrations/2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php +++ b/database/migrations/2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php @@ -1,5 +1,10 @@ . Licensed under the GNU Affero General Public License v3.0. +// See the LICENCE file in the repository root for full licence text. + +declare(strict_types=1); + use Illuminate\Database\Migrations\Migration; use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Schema; From 2ecb1f04665aa95737c66895e943a41471384727 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 8 Jan 2025 19:14:12 +0900 Subject: [PATCH 05/13] Fix inspection --- ..._12_10_035240_add_freestyle_to_multiplayer_playlist_items.php | 1 - 1 file changed, 1 deletion(-) diff --git a/database/migrations/2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php b/database/migrations/2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php index 50b6076cbab..9136afe63b2 100644 --- a/database/migrations/2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php +++ b/database/migrations/2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php @@ -19,7 +19,6 @@ public function up(): void Schema::table('multiplayer_playlist_items', function (Blueprint $table) { $table->boolean('freestyle')->after('required_mods')->default(false); }); - } /** From 87db8f489caf8e2f153d08f7a68d2f8cb3f862f1 Mon Sep 17 00:00:00 2001 From: nanaya Date: Wed, 15 Jan 2025 21:29:32 +0900 Subject: [PATCH 06/13] Readd beatmap_id validation Ruleset id validation is done in ScoreToken. --- app/Models/Multiplayer/Room.php | 37 ++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/app/Models/Multiplayer/Room.php b/app/Models/Multiplayer/Room.php index 30041b14c4b..8dcf6d088ed 100644 --- a/app/Models/Multiplayer/Room.php +++ b/app/Models/Multiplayer/Room.php @@ -662,9 +662,21 @@ public function startPlay(User $user, PlaylistItem $playlistItem, array $rawPara { priv_check_user($user, 'MultiplayerScoreSubmit', $this)->ensureCan(); - $this->assertValidStartPlay($user, $playlistItem); + $params = get_params($rawParams, null, [ + 'beatmap_hash', + 'beatmap_id:int', + 'build_id', + 'ruleset_id:int', + ], ['null_missing' => true]); + + if (!$playlistItem->freestyle) { + $params['beatmap_id'] = $playlistItem->beatmap_id; + $params['ruleset_id'] = $playlistItem->ruleset_id; + } - return $this->getConnection()->transaction(function () use ($playlistItem, $rawParams, $user) { + $this->assertValidStartPlay($user, $playlistItem, $params); + + return $this->getConnection()->transaction(function () use ($params, $playlistItem, $user) { $agg = UserScoreAggregate::new($user, $this); if ($agg->wasRecentlyCreated) { $this->incrementInstance('participant_count'); @@ -676,11 +688,11 @@ public function startPlay(User $user, PlaylistItem $playlistItem, array $rawPara $playlistItemAgg->updateUserAttempts(); return ScoreToken::create([ - 'beatmap_hash' => get_string($rawParams['beatmap_hash'] ?? null), - 'beatmap_id' => $playlistItem->beatmap_id, - 'build_id' => $rawParams['build_id'], + 'beatmap_hash' => $params['beatmap_hash'], + 'beatmap_id' => $params['beatmap_id'], + 'build_id' => $params['build_id'], 'playlist_item_id' => $playlistItem->getKey(), - 'ruleset_id' => $playlistItem->ruleset_id, + 'ruleset_id' => $params['ruleset_id'], 'user_id' => $user->getKey(), ]); }); @@ -741,7 +753,7 @@ private function assertValidStartGame() } } - private function assertValidStartPlay(User $user, PlaylistItem $playlistItem) + private function assertValidStartPlay(User $user, PlaylistItem $playlistItem, array $params): void { // todo: check against room's end time (to see if player has enough time to play this beatmap) and is under the room's max attempts limit @@ -749,6 +761,17 @@ private function assertValidStartPlay(User $user, PlaylistItem $playlistItem) throw new InvariantException('Room has already ended.'); } + if ($playlistItem->freestyle) { + // assert the beatmap_id is part of playlist item's beatmapset + $beatmapsetIdCount = Beatmap + ::whereKey([$playlistItem->beatmap_id, $params['beatmap_id']]) + ->distinct('beatmapset_id') + ->count(); + if ($beatmapsetIdCount !== 1) { + throw new InvariantException('Specified beatmap_id is not allowed'); + } + } + $userId = $user->getKey(); if ($this->max_attempts !== null) { $roomStats = $this->userHighScores()->where('user_id', $userId)->first(); From 0d393a8e1f343eeea4abc457b8eddc8d91d10840 Mon Sep 17 00:00:00 2001 From: nanaya Date: Wed, 15 Jan 2025 21:34:52 +0900 Subject: [PATCH 07/13] Disallow mod on freestyle --- app/Models/Multiplayer/PlaylistItem.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/Models/Multiplayer/PlaylistItem.php b/app/Models/Multiplayer/PlaylistItem.php index b8ab7d8b56b..37ca9b1a653 100644 --- a/app/Models/Multiplayer/PlaylistItem.php +++ b/app/Models/Multiplayer/PlaylistItem.php @@ -171,6 +171,10 @@ private function assertValidRuleset() private function assertValidMods() { + if ($this->freestyle && (count($this->allowed_mods) === 0 || count($this->required_mods) === 0)) { + throw new InvariantException("mod isn't allowed in freestyle"); + } + $allowedModIds = array_column($this->allowed_mods, 'acronym'); $requiredModIds = array_column($this->required_mods, 'acronym'); From adecc03f8e59ef424221b9b6f31bd1c52943385e Mon Sep 17 00:00:00 2001 From: nanaya Date: Wed, 15 Jan 2025 21:37:44 +0900 Subject: [PATCH 08/13] Lint fix --- .../Controllers/Multiplayer/Rooms/Playlist/ScoresController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php b/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php index 9dd6d4ce9ea..19b79dde254 100644 --- a/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php +++ b/app/Http/Controllers/Multiplayer/Rooms/Playlist/ScoresController.php @@ -7,7 +7,6 @@ use App\Http\Controllers\Controller as BaseController; use App\Libraries\ClientCheck; -use App\Models\Beatmap; use App\Models\Multiplayer\PlaylistItem; use App\Models\Multiplayer\PlaylistItemUserHighScore; use App\Models\Multiplayer\Room; From ac1b77b7ee6ef2ddc2be44aab23574e26b30d83e Mon Sep 17 00:00:00 2001 From: nanaya Date: Wed, 15 Jan 2025 21:42:04 +0900 Subject: [PATCH 09/13] No smarts allowed Easier to understand. --- app/Models/Multiplayer/Room.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/Models/Multiplayer/Room.php b/app/Models/Multiplayer/Room.php index 8dcf6d088ed..19e9d2c7d8b 100644 --- a/app/Models/Multiplayer/Room.php +++ b/app/Models/Multiplayer/Room.php @@ -763,11 +763,7 @@ private function assertValidStartPlay(User $user, PlaylistItem $playlistItem, ar if ($playlistItem->freestyle) { // assert the beatmap_id is part of playlist item's beatmapset - $beatmapsetIdCount = Beatmap - ::whereKey([$playlistItem->beatmap_id, $params['beatmap_id']]) - ->distinct('beatmapset_id') - ->count(); - if ($beatmapsetIdCount !== 1) { + if ($playlistItem->beatmap->beatmapset_id !== Beatmap::find($params['beatmap_id'])?->getKey()) { throw new InvariantException('Specified beatmap_id is not allowed'); } } From 2b05d0980cd8ce3655ebd41c9c2624425f5ad479 Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 16 Jan 2025 15:34:58 +0900 Subject: [PATCH 10/13] Fix incorrect check and short circuit if passed --- app/Models/Multiplayer/PlaylistItem.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/Models/Multiplayer/PlaylistItem.php b/app/Models/Multiplayer/PlaylistItem.php index 37ca9b1a653..4751a5eeff6 100644 --- a/app/Models/Multiplayer/PlaylistItem.php +++ b/app/Models/Multiplayer/PlaylistItem.php @@ -171,8 +171,11 @@ private function assertValidRuleset() private function assertValidMods() { - if ($this->freestyle && (count($this->allowed_mods) === 0 || count($this->required_mods) === 0)) { - throw new InvariantException("mod isn't allowed in freestyle"); + if ($this->freestyle) { + if (count($this->allowed_mods) !== 0 || count($this->required_mods) !== 0) { + throw new InvariantException("mod isn't allowed in freestyle"); + } + return; } $allowedModIds = array_column($this->allowed_mods, 'acronym'); From 6f053b369d2ba5eeb3b7cf699b3a617bbe242c15 Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 16 Jan 2025 15:48:43 +0900 Subject: [PATCH 11/13] Wrong id --- app/Models/Multiplayer/Room.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Models/Multiplayer/Room.php b/app/Models/Multiplayer/Room.php index 19e9d2c7d8b..06088752613 100644 --- a/app/Models/Multiplayer/Room.php +++ b/app/Models/Multiplayer/Room.php @@ -763,7 +763,7 @@ private function assertValidStartPlay(User $user, PlaylistItem $playlistItem, ar if ($playlistItem->freestyle) { // assert the beatmap_id is part of playlist item's beatmapset - if ($playlistItem->beatmap->beatmapset_id !== Beatmap::find($params['beatmap_id'])?->getKey()) { + if ($playlistItem->beatmap->beatmapset_id !== Beatmap::find($params['beatmap_id'])?->beatmapset_id) { throw new InvariantException('Specified beatmap_id is not allowed'); } } From 72baade57f954423c98614321566304ef7f98c6c Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 16 Jan 2025 20:51:22 +0900 Subject: [PATCH 12/13] Correctly cast the field --- app/Models/Multiplayer/PlaylistItem.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/Models/Multiplayer/PlaylistItem.php b/app/Models/Multiplayer/PlaylistItem.php index 4751a5eeff6..4782c1714a5 100644 --- a/app/Models/Multiplayer/PlaylistItem.php +++ b/app/Models/Multiplayer/PlaylistItem.php @@ -36,6 +36,7 @@ class PlaylistItem extends Model protected $casts = [ 'allowed_mods' => 'object', 'expired' => 'boolean', + 'freestyle' => 'boolean', 'required_mods' => 'object', ]; From 6e84fed8bc095c763f123912a070004f21366d81 Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 16 Jan 2025 20:52:40 +0900 Subject: [PATCH 13/13] Redate migration --- ..._01_16_000000_add_freestyle_to_multiplayer_playlist_items.php} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename database/migrations/{2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php => 2025_01_16_000000_add_freestyle_to_multiplayer_playlist_items.php} (100%) diff --git a/database/migrations/2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php b/database/migrations/2025_01_16_000000_add_freestyle_to_multiplayer_playlist_items.php similarity index 100% rename from database/migrations/2024_12_10_035240_add_freestyle_to_multiplayer_playlist_items.php rename to database/migrations/2025_01_16_000000_add_freestyle_to_multiplayer_playlist_items.php