Merge branch 'master' into form-param

ppy · Nov 21, 2024 · 693fd4a · 693fd4a
2 parents ca8ce96 + cd26bb5
commit 693fd4a
Show file tree

Hide file tree

Showing 12 changed files with 235 additions and 26 deletions.
diff --git a/.env.example b/.env.example
@@ -229,6 +229,7 @@ CLIENT_CHECK_VERSION=false
 # USER_PROFILE_SCORES_NOTICE=
 
 # MULTIPLAYER_MAX_ATTEMPTS_LIMIT=128
+# MULTIPLAYER_ROOM_CLOSE_GRACE_PERIOD_MINUTES=5
 
 # NOTIFICATION_QUEUE=notification
 # NOTIFICATION_REDIS_HOST=127.0.0.1

diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -137,11 +137,16 @@ jobs:
           key: ${{ runner.os }}-composer-${{ hashFiles('composer.lock') }}
           restore-keys: ${{ runner.os }}-composer-
 
+      - name: Get current date
+        id: current-date
+        run: echo "today=$(date '+%Y%m%d')" >> "$GITHUB_OUTPUT"
+
       - name: Cache ip2asn
         uses: actions/cache@v4
         with:
           path: database/ip2asn/
-          key: ip2asn
+          key: ip2asn-${{ steps.current-date.outputs.today }}
+          restore-keys: ip2asn-
 
       - run: ./build.sh
 

diff --git a/app/Http/Controllers/Multiplayer/RoomsController.php b/app/Http/Controllers/Multiplayer/RoomsController.php
@@ -20,6 +20,13 @@ public function __construct()
         $this->middleware('require-scopes:public', ['only' => ['index', 'leaderboard', 'show']]);
     }
 
+    public function destroy($id)
+    {
+        Room::findOrFail($id)->endGame(\Auth::user());
+
+        return response(null, 204);
+    }
+
     /**
      * Get Multiplayer Rooms
      *

diff --git a/app/Http/Controllers/RankingController.php b/app/Http/Controllers/RankingController.php
@@ -212,8 +212,8 @@ public function index($mode, $type)
 
         $maxResults = $this->maxResults($modeInt, $stats);
         $maxPages = ceil($maxResults / static::PAGE_SIZE);
-        // TODO: less repeatedly getting params out of request.
-        $page = \Number::clamp(get_int(request('cursor.page') ?? request('page') ?? 1), 1, $maxPages);
+        $params = get_params(\Request::all(), null, ['cursor.page:int', 'page:int']);
+        $page = \Number::clamp($params['cursor']['page'] ?? $params['page'] ?? 1, 1, $maxPages);
 
         $stats = $stats->limit(static::PAGE_SIZE)
             ->offset(static::PAGE_SIZE * ($page - 1))

diff --git a/app/Models/Multiplayer/Room.php b/app/Models/Multiplayer/Room.php
@@ -419,7 +419,7 @@ public function calculateMissingTopScores()
 
     public function completePlay(ScoreToken $scoreToken, array $params): ScoreLink
     {
-        priv_check_user($scoreToken->user, 'MultiplayerScoreSubmit')->ensureCan();
+        priv_check_user($scoreToken->user, 'MultiplayerScoreSubmit', $this)->ensureCan();
 
         $this->assertValidCompletePlay();
 
@@ -622,9 +622,29 @@ public function startGame(User $host, array $rawParams, array $extraParams = [])
         return $this->fresh();
     }
 
+    /**
+     * @throws InvariantException
+     */
+    public function endGame(User $requestingUser)
+    {
+        priv_check_user($requestingUser, 'MultiplayerRoomDestroy', $this)->ensureCan();
+
+        if ($this->isRealtime()) {
+            throw new InvariantException('Realtime rooms cannot be closed.');
+        }
+
+        $gracePeriodMinutes = $GLOBALS['cfg']['osu']['multiplayer']['room_close_grace_period_minutes'];
+        if ($this->starts_at->addMinutes($gracePeriodMinutes)->isPast()) {
+            throw new InvariantException('The grace period for closing this room has expired.');
+        }
+
+        $this->ends_at = now();
+        $this->save();
+    }
+
     public function startPlay(User $user, PlaylistItem $playlistItem, int $buildId)
     {
-        priv_check_user($user, 'MultiplayerScoreSubmit')->ensureCan();
+        priv_check_user($user, 'MultiplayerScoreSubmit', $this)->ensureCan();
 
         $this->assertValidStartPlay($user, $playlistItem);
 

diff --git a/app/Singletons/OsuAuthorize.php b/app/Singletons/OsuAuthorize.php
@@ -976,6 +976,7 @@ public function checkChatChannelSend(?User $user, Channel $channel): string
     {
         $prefix = 'chat.';
 
+        $this->ensureLoggedIn($user);
         $this->ensureSessionVerified($user);
         $this->ensureCleanRecord($user, $prefix);
         // This check becomes useless when min_plays_allow_verified_bypass is enabled.
@@ -1836,14 +1837,41 @@ public function checkMultiplayerRoomCreate(?User $user): string
 
     /**
      * @param User|null $user
+     * @param Room $room
      * @return string
      * @throws AuthorizationCheckException
      */
-    public function checkMultiplayerScoreSubmit(?User $user): string
+    public function checkMultiplayerRoomDestroy(?User $user, Room $room): string
     {
+        $prefix = 'room.destroy.';
+
         $this->ensureLoggedIn($user);
         $this->ensureCleanRecord($user);
 
+        if ($room->user_id !== $user->getKey()) {
+            return $prefix.'not_owner';
+        }
+
+        return 'ok';
+    }
+
+    /**
+     * @param User|null $user
+     * @return string
+     * @throws AuthorizationCheckException
+     */
+    public function checkMultiplayerScoreSubmit(?User $user, Room $room): string
+    {
+        $this->ensureLoggedIn($user);
+
+        if ($room->isRealtime()) {
+            $this->ensureCleanRecord($user);
+        } else {
+            if ($user->isRestricted()) {
+                throw new AuthorizationCheckException('restricted');
+            }
+        }
+
         return 'ok';
     }
 
@@ -2023,8 +2051,6 @@ public function checkWikiPageRefresh(?User $user): string
     }
 
     /**
-     * @param User|null $user
-     * @param string $prefix
      * @throws AuthorizationCheckException
      */
     public function ensureLoggedIn(?User $user, string $prefix = ''): void
@@ -2035,35 +2061,25 @@ public function ensureLoggedIn(?User $user, string $prefix = ''): void
     }
 
     /**
-     * @param User|null $user
-     * @param string $prefix
-     * @return string
      * @throws AuthorizationCheckException
      */
-    public function ensureCleanRecord(?User $user, string $prefix = ''): string
+    private function ensureCleanRecord(User $user, string $prefix = ''): void
     {
-        if ($user === null) {
-            return 'unauthorized';
-        }
-
         if ($user->isRestricted()) {
             throw new AuthorizationCheckException($prefix.'restricted');
         }
 
         if ($user->isSilenced()) {
             throw new AuthorizationCheckException($prefix.'silenced');
         }
-
-        return 'ok';
     }
 
     /**
-     * @param User|null $user
      * @throws AuthorizationCheckException
      */
-    public function ensureHasPlayed(?User $user): void
+    private function ensureHasPlayed(User $user): void
     {
-        if ($user === null || $user->isBot()) {
+        if ($user->isBot()) {
             return;
         }
 
@@ -2087,13 +2103,10 @@ public function ensureHasPlayed(?User $user): void
     /**
      * Ensure User is logged in and verified.
      *
-     * @param User|null $user
      * @throws AuthorizationCheckException
      */
-    public function ensureSessionVerified(?User $user)
+    private function ensureSessionVerified(User $user)
     {
-        $this->ensureLoggedIn($user);
-
         if (!$user->isSessionVerified()) {
             throw new AuthorizationCheckException('require_verification');
         }

diff --git a/config/osu.php b/config/osu.php
@@ -152,6 +152,7 @@
     ],
     'multiplayer' => [
         'max_attempts_limit' => get_int(env('MULTIPLAYER_MAX_ATTEMPTS_LIMIT')) ?? 128,
+        'room_close_grace_period_minutes' => get_int(env('MULTIPLAYER_ROOM_CLOSE_GRACE_PERIOD_MINUTES')) ?? 5,
     ],
     'notification' => [
         'endpoint' => presence(env('NOTIFICATION_ENDPOINT'), '/home/notifications/feed'),

diff --git a/database/migrations/2024_11_19_130731_create_beatmap_versioning_tables.php b/database/migrations/2024_11_19_130731_create_beatmap_versioning_tables.php
@@ -0,0 +1,55 @@
+<?php
+
+// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the GNU Affero General Public License v3.0.
+// See the LICENCE file in the repository root for full licence text.
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+
+return new class extends Migration
+{
+    /**
+     * Run the migrations.
+     */
+    public function up(): void
+    {
+        // raw statement used as laravel syntax doesn't support a fixed size BINARY column
+        DB::statement('CREATE TABLE `beatmapset_files` (
+            `file_id` BIGINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
+            `sha2_hash` BINARY(32) NOT NULL,
+            `file_size` INT UNSIGNED NOT NULL,
+
+            UNIQUE (`sha2_hash`)
+        )');
+
+        Schema::create('beatmapset_versions', function (Blueprint $table) {
+            $table->bigIncrements('version_id');
+            $table->mediumInteger('beatmapset_id')->unsigned();
+            $table->timestamp('created_at')->useCurrent();
+            $table->bigInteger('previous_version_id')->unsigned()->nullable();
+
+            $table->index('beatmapset_id');
+            $table->index('previous_version_id');
+        });
+
+        Schema::create('beatmapset_version_files', function (Blueprint $table) {
+            $table->bigInteger('file_id')->unsigned();
+            $table->bigInteger('version_id')->unsigned();
+            $table->string('filename', length: 500);
+
+            $table->primary(['file_id', 'version_id']);
+            $table->index('file_id');
+            $table->index('version_id');
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     */
+    public function down(): void
+    {
+        Schema::dropIfExists('beatmapset_version_files');
+        Schema::dropIfExists('beatmapset_versions');
+        Schema::dropIfExists('beatmapset_files');
+    }
+};
diff --git a/resources/lang/en/authorization.php b/resources/lang/en/authorization.php
@@ -170,6 +170,12 @@
         ],
     ],
 
+    'room' => [
+        'destroy' => [
+            'not_owner' => 'Only room owner can close it.',
+        ],
+    ],
+
     'score' => [
         'pin' => [
             'disabled_type' => "Can't pin this type of score",

diff --git a/routes/web.php b/routes/web.php
@@ -493,7 +493,7 @@
             });
         });
 
-        Route::apiResource('rooms', 'Multiplayer\RoomsController', ['only' => ['index', 'show', 'store']]);
+        Route::apiResource('rooms', 'Multiplayer\RoomsController', ['only' => ['index', 'show', 'store', 'destroy']]);
 
         Route::apiResource('seasonal-backgrounds', 'SeasonalBackgroundsController', ['only' => ['index']]);
 

diff --git a/tests/Controllers/Multiplayer/RoomsControllerTest.php b/tests/Controllers/Multiplayer/RoomsControllerTest.php
@@ -429,6 +429,94 @@ public function testJoinWithPassword()
         $this->assertSame($initialUserChannelCount + 1, UserChannel::count());
     }
 
+    public function testDestroy()
+    {
+        $start = now();
+        $end = $start->clone()->addMinutes(60);
+        $room = Room::factory()->create([
+            'starts_at' => $start,
+            'ends_at' => $end,
+            'type' => Room::PLAYLIST_TYPE,
+        ]);
+        $end = $room->ends_at; // assignment truncates fractional second part, so refetch here
+        $url = route('api.rooms.destroy', ['room' => $room]);
+
+        $this->actAsScopedUser($room->host);
+        $this
+            ->delete($url)
+            ->assertSuccessful();
+
+        $room->refresh();
+        $this->assertLessThan($end, $room->ends_at);
+    }
+
+    public function testDestroyCannotBeCalledOnRealtimeRoom()
+    {
+        $start = now();
+        $end = $start->clone()->addMinutes(60);
+        $room = Room::factory()->create([
+            'starts_at' => $start,
+            'ends_at' => $end,
+            'type' => Room::REALTIME_DEFAULT_TYPE,
+        ]);
+        $end = $room->ends_at; // assignment truncates fractional second part, so refetch here
+        $url = route('api.rooms.destroy', ['room' => $room]);
+
+        $this->actAsScopedUser($room->host);
+        $this
+            ->delete($url)
+            ->assertStatus(422);
+
+        $room->refresh();
+        $this->assertEquals($end, $room->ends_at);
+    }
+
+    public function testDestroyCannotBeCalledByAnotherUser()
+    {
+        $requester = User::factory()->create();
+        $owner = User::factory()->create();
+        $start = now();
+        $end = $start->clone()->addMinutes(60);
+        $room = Room::factory()->create([
+            'user_id' => $owner->getKey(),
+            'starts_at' => $start,
+            'ends_at' => $end,
+            'type' => Room::PLAYLIST_TYPE,
+        ]);
+        $url = route('api.rooms.destroy', ['room' => $room]);
+        $end = $room->ends_at; // assignment truncates fractional second part, so refetch here
+
+        $this->actAsScopedUser($requester);
+        $this
+            ->delete($url)
+            ->assertStatus(403);
+
+        $room->refresh();
+        $this->assertEquals($end, $room->ends_at);
+    }
+
+    public function testDestroyCannotBeCalledAfterGracePeriod()
+    {
+        $start = now();
+        $end = $start->clone()->addMinutes(60);
+        $room = Room::factory()->create([
+            'starts_at' => $start,
+            'ends_at' => $end,
+            'type' => Room::PLAYLIST_TYPE,
+        ]);
+        $url = route('api.rooms.destroy', ['room' => $room]);
+        $end = $room->ends_at; // assignment truncates fractional second part, so refetch here
+
+        $this->actAsScopedUser($room->host);
+        $this->travelTo($start->addMinutes(6));
+        $this
+            ->delete($url)
+            ->assertStatus(422);
+
+        $room->refresh();
+        $this->assertEquals($end, $room->ends_at);
+    }
+
     public static function dataProviderForTestStoreWithInvalidPlayableMods(): array
     {
         $ret = [];