ppy · cl8n · Sep 23, 2023 · Sep 23, 2023 · Sep 23, 2023 · Sep 23, 2023
diff --git a/app/Http/Controllers/Admin/Controller.php b/app/Http/Controllers/Admin/Controller.php
@@ -15,7 +15,7 @@ public function __construct()
         $this->middleware('auth');
 
         $this->middleware(function ($request, $next) {
-            if (Auth::check() && !Auth::user()->isAdmin()) {
+            if (Auth::check() && !Auth::user()->isGroup('admin')) {
                 abort(403);
             }
 

diff --git a/app/Http/Controllers/ArtistsController.php b/app/Http/Controllers/ArtistsController.php
@@ -18,7 +18,7 @@ public function index()
         $artists = Artist::with('label')->withMax('tracks', 'created_at')->withCount('tracks')->orderBy('name', 'asc');
         $user = Auth::user();
 
-        if ($user === null || !$user->isAdmin()) {
+        if ($user === null || !$user->isGroup('admin')) {
             $artists->where('visible', true);
         }
 
@@ -32,7 +32,7 @@ public function show($id)
         $artist = Artist::with('label')->findOrFail($id);
         $user = Auth::user();
 
-        if (!$artist->visible && ($user === null || !$user->isAdmin())) {
+        if (!$artist->visible && ($user === null || !$user->isGroup('admin'))) {
             abort(404);
         }
 

diff --git a/app/Http/Controllers/ContestsController.php b/app/Http/Controllers/ContestsController.php
@@ -15,7 +15,7 @@ public function index()
     {
         $contests = Contest::orderBy('id', 'desc');
 
-        if (!Auth::check() || !Auth::user()->isAdmin()) {
+        if (!Auth::check() || !Auth::user()->isGroup('admin')) {
             $contests->where('visible', true);
         }
 
@@ -29,7 +29,7 @@ public function show($id)
         $contest = Contest::findOrFail($id);
 
         $user = Auth::user();
-        if (!$contest->visible && (!$user || !$user->isAdmin())) {
+        if (!$contest->visible && (!$user || !$user->isGroup('admin'))) {
             abort(404);
         }
 

diff --git a/app/Http/Controllers/Forum/ForumCoversController.php b/app/Http/Controllers/Forum/ForumCoversController.php
@@ -25,7 +25,7 @@ public function __construct()
         ]]);
 
         $this->middleware(function ($request, $next) {
-            if (Auth::check() && !Auth::user()->isAdmin()) {
+            if (Auth::check() && !Auth::user()->isGroup('admin')) {
                 abort(403);
             }
 

diff --git a/app/Http/Controllers/StoreController.php b/app/Http/Controllers/StoreController.php
@@ -47,7 +47,7 @@ public function getInvoice($id = null)
             ->with('items.product')
             ->findOrFail($id);
 
-        if (Auth::user()->user_id !== $order->user_id && !Auth::user()->isAdmin()) {
+        if (Auth::user()->user_id !== $order->user_id && !Auth::user()->isGroup('admin')) {
             abort(403);
         }
 

diff --git a/app/Libraries/ClientCheck.php b/app/Libraries/ClientCheck.php
@@ -11,7 +11,7 @@ class ClientCheck
 {
     public static function findBuild($user, $params): ?Build
     {
-        $assertValid = config('osu.client.check_version') && $user->findUserGroup(app('groups')->byIdentifier('admin'), true) === null;
+        $assertValid = config('osu.client.check_version') && !$user->isGroup('admin', allowOAuth: true);
 
         $clientHash = presence(get_string($params['version_hash'] ?? null));
         if ($clientHash === null) {

diff --git a/app/Libraries/OsuAuthorize.php b/app/Libraries/OsuAuthorize.php
@@ -82,7 +82,7 @@ public function doCheckUser(?User $user, string $ability, object $object = null)
         $auth = $authMap->get($cacheKey, null);
 
         if ($auth === null) {
-            if ($user !== null && $user->isAdmin() && !static::alwaysCheck($ability)) {
+            if ($user !== null && $user->isGroup('admin') && !static::alwaysCheck($ability)) {
                 $message = 'ok';
             } else {
                 $function = "check{$ability}";
@@ -141,7 +141,7 @@ public function checkBeatmapUpdateOwner(?User $user, ?Beatmapset $beatmapset): s
                 Beatmapset::STATES['graveyard'],
                 Beatmapset::STATES['loved'],
             ];
-            if ($user->isProjectLoved() && in_array($status, $lovedModifiable, true)) {
+            if ($user->isGroup('loved') && in_array($status, $lovedModifiable, true)) {
                 return 'ok';
             }
 
@@ -628,7 +628,7 @@ public function checkBeatmapsetLove(?User $user): string
     {
         $this->ensureLoggedIn($user);
 
-        if (!$user->isProjectLoved()) {
+        if (!$user->isGroup('loved')) {
             return 'unauthorized';
         }
 
@@ -648,7 +648,7 @@ public function checkBeatmapsetNominate(?User $user, Beatmapset $beatmapset): st
 
         static $prefix = 'beatmap_discussion.nominate.';
 
-        if (!$user->isBNG() && !$user->isNAT()) {
+        if (!$user->isBNG() && !$user->isGroup('nat')) {
             return 'unauthorized';
         }
 
@@ -770,7 +770,7 @@ public function checkBeatmapsetDisqualify(?User $user, Beatmapset $beatmapset):
     {
         $this->ensureLoggedIn($user);
 
-        if (!$user->isFullBN() && !$user->isModerator()) {
+        if (!$user->isGroup('bng') && !$user->isModerator()) {
             return 'unauthorized';
         }
 
@@ -841,7 +841,7 @@ public function checkBeatmapsetMetadataEdit(?User $user, Beatmapset $beatmapset)
             Beatmapset::STATES['graveyard'],
             Beatmapset::STATES['loved'],
         ];
-        if ($user->isProjectLoved() && in_array($beatmapset->approved, $lovedEditable, true)) {
+        if ($user->isGroup('loved') && in_array($beatmapset->approved, $lovedEditable, true)) {
             return 'ok';
         }
 
@@ -912,7 +912,7 @@ public function checkChatAnnounce(?User $user): string
         $this->ensureLoggedIn($user);
         $this->ensureCleanRecord($user, $prefix);
 
-        if ($user->isModerator() || $user->isChatAnnouncer()) {
+        if ($user->isModerator() || $user->isGroup('announce', allowOAuth: true)) {
             return 'ok';
         }
 
@@ -1363,7 +1363,14 @@ public function checkForumModerate(?User $user, Forum $forum): string
             return 'ok';
         }
 
-        if ($forum->moderator_groups !== null && !empty(array_intersect($user->groupIds()['active'], $forum->moderator_groups))) {
+        // TODO: If `$user` is the resource owner, authorizing with a
+        //       third-party OAuth client, `$user->groupIds()` should be empty
+        //       here to match the restrictions in `User::isGroup()`.
+        //
+        //       Some third-party clients currently rely on this mistake, so an
+        //       alternative method to request usage of group permissions needs
+        //       to be provided before fixing this.
+        if ($forum->moderator_groups !== null && !empty(array_intersect($user->groupIds(), $forum->moderator_groups))) {
             return 'ok';
         }
 
@@ -1808,8 +1815,7 @@ public function checkLegacyIrcKeyStore(?User $user): string
         $this->ensureLoggedIn($user);
         $this->ensureCleanRecord($user);
 
-        // isBot checks user primary group
-        if (!$user->isGroup(app('groups')->byIdentifier('bot')) && $user->playCount() < 100) {
+        if (!$user->isGroup('bot') && $user->playCount() < 100) {
             return 'play_more';
         }
 

diff --git a/app/Models/Beatmapset.php b/app/Models/Beatmapset.php
@@ -724,10 +724,10 @@ public function nominate(User $user, array $playmodes = [])
                 $canNominate = false;
                 $canFullNominate = false;
                 foreach ($this->playmodesStr() as $mode) {
-                    if ($user->isFullBN($mode) || $user->isNAT($mode)) {
+                    if ($user->isGroup('bng', $mode) || $user->isGroup('nat', $mode)) {
                         $canNominate = true;
                         $canFullNominate = true;
-                    } else if ($user->isLimitedBN($mode)) {
+                    } else if ($user->isGroup('bng_limited', $mode)) {
                         $canNominate = true;
                     }
                 }
@@ -747,8 +747,8 @@ public function nominate(User $user, array $playmodes = [])
                 }
 
                 foreach ($playmodes as $mode) {
-                    if (!$user->isFullBN($mode) && !$user->isNAT($mode)) {
-                        if (!$user->isLimitedBN($mode)) {
+                    if (!$user->isGroup('bng', $mode) && !$user->isGroup('nat', $mode)) {
+                        if (!$user->isGroup('bng_limited', $mode)) {
                             throw new InvariantException(osu_trans('beatmapsets.nominate.incorrect_mode', ['mode' => $mode]));
                         }
 
@@ -1270,7 +1270,7 @@ public function hasFullBNNomination($mode = null)
             ->get()
             ->pluck('user')
             ->contains(function ($user) use ($mode) {
-                return $user->isNAT($mode) || $user->isFullBN($mode);
+                return $user->isGroup('nat', $mode) || $user->isGroup('bng', $mode);
             });
     }
 

diff --git a/app/Models/Chat/Channel.php b/app/Models/Chat/Channel.php
@@ -241,7 +241,7 @@ public function isVisibleFor(User $user): bool
         return !(
             $targetUser === null
             || $user->hasBlocked($targetUser)
-            && !($targetUser->isBot() || $targetUser->isModerator() || $targetUser->isAdmin())
+            && !($targetUser->isBot() || $targetUser->isModerator() || $targetUser->isGroup('admin'))
         );
     }
 

diff --git a/app/Models/Forum/Authorize.php b/app/Models/Forum/Authorize.php
@@ -25,7 +25,7 @@ class Authorize extends Model
 
     public static function aclCheck($user, $authOption, $forum)
     {
-        $groupIds = $user->groupIds()['active'];
+        $groupIds = $user->groupIds();
         $authOptionId = AuthOption::where('auth_option', $authOption)->value('auth_option_id');
 
         // the group may contain direct acl entry
@@ -48,7 +48,7 @@ public static function aclCheck($user, $authOption, $forum)
 
     public static function aclGetAllowedForums($user, $authOption)
     {
-        $groupIds = $user->groupIds()['active'];
+        $groupIds = $user->groupIds();
         $authOptionId = AuthOption::where('auth_option', $authOption)->value('auth_option_id');
 
         $directAclForumIds = model_pluck(static::directAcl($groupIds, $authOptionId), 'forum_id');

diff --git a/app/Models/Forum/TopicCover.php b/app/Models/Forum/TopicCover.php
@@ -52,7 +52,7 @@ public static function findForUse($id, $user)
 
         $covers = static::select();
 
-        if ($user->isAdmin() === false) {
+        if ($user->isGroup('admin') === false) {
             $covers->where('user_id', $user->user_id);
         }