From 166b39beeaa5c6b1e07a4c520fa3d6c705a524ed Mon Sep 17 00:00:00 2001
From: Bubka <858858+Bubka@users.noreply.github.com>
Date: Wed, 19 Feb 2025 15:46:31 +0100
Subject: [PATCH] Handle missing group during group assignment

---
 app/Api/v1/Controllers/GroupController.php       |  7 ++++++-
 app/Api/v1/Controllers/TwoFAccountController.php | 15 +++++++++++++--
 app/Services/GroupService.php                    | 10 +++++++---
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/app/Api/v1/Controllers/GroupController.php b/app/Api/v1/Controllers/GroupController.php
index 3dc24f07..4131c6d2 100644
--- a/app/Api/v1/Controllers/GroupController.php
+++ b/app/Api/v1/Controllers/GroupController.php
@@ -10,6 +10,7 @@ use App\Facades\Groups;
 use App\Http\Controllers\Controller;
 use App\Models\Group;
 use App\Models\User;
+use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Illuminate\Http\Request;
 
 class GroupController extends Controller
@@ -93,7 +94,11 @@ class GroupController extends Controller
 
         $validated = $request->validated();
 
-        Groups::assign($validated['ids'], $request->user(), $group);
+        try {
+            Groups::assign($validated['ids'], $request->user(), $group);
+        } catch (ModelNotFoundException $exc) {
+            abort(404);
+        }
 
         return new GroupResource($group);
     }
diff --git a/app/Api/v1/Controllers/TwoFAccountController.php b/app/Api/v1/Controllers/TwoFAccountController.php
index 806195f7..70997761 100644
--- a/app/Api/v1/Controllers/TwoFAccountController.php
+++ b/app/Api/v1/Controllers/TwoFAccountController.php
@@ -21,6 +21,7 @@ use App\Helpers\Helpers;
 use App\Http\Controllers\Controller;
 use App\Models\TwoFAccount;
 use App\Models\User;
+use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Illuminate\Http\Request;
 use Illuminate\Support\Arr;
 
@@ -89,7 +90,12 @@ class TwoFAccountController extends Controller
         $request->user()->twofaccounts()->save($twofaccount);
 
         // Possible group association
-        Groups::assign($twofaccount->id, $request->user(), Arr::get($validated, 'group_id', null));
+        try {
+            Groups::assign($twofaccount->id, $request->user(), Arr::get($validated, 'group_id', null));
+        } catch (\Throwable $th) {
+            // The group association might fail but we don't want the twofaccount
+            // creation to be reverted so we do nothing here.
+        }
 
         return (new TwoFAccountReadResource($twofaccount->refresh()))
             ->response()
@@ -116,7 +122,12 @@ class TwoFAccountController extends Controller
             if ((int) $groupId === 0) {
                 TwoFAccounts::withdraw($twofaccount->id);
             } else {
-                Groups::assign($twofaccount->id, $request->user(), $groupId);
+                try {
+                    Groups::assign($twofaccount->id, $request->user(), $groupId);
+                } catch (ModelNotFoundException $exc) {
+                    // The destination group no longer exists, the twofaccount is withdrawn
+                    TwoFAccounts::withdraw($twofaccount->id);
+                }
             }
             $twofaccount->refresh();
         }
diff --git a/app/Services/GroupService.php b/app/Services/GroupService.php
index 6de1444c..c319da4e 100644
--- a/app/Services/GroupService.php
+++ b/app/Services/GroupService.php
@@ -7,6 +7,7 @@ use App\Models\TwoFAccount;
 use App\Models\User;
 use Illuminate\Auth\Access\AuthorizationException;
 use Illuminate\Database\Eloquent\Collection;
+use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Log;
 
@@ -20,6 +21,7 @@ class GroupService
      * @param  mixed  $targetGroup  The group the accounts should be assigned to
      *
      * @throws \Illuminate\Auth\Access\AuthorizationException
+     * @throws \Illuminate\Database\Eloquent\ModelNotFoundException
      */
     public static function assign($ids, User $user, mixed $targetGroup = null) : void
     {
@@ -57,21 +59,23 @@ class GroupService
             $ids = is_array($ids) ? $ids : [$ids];
 
             DB::transaction(function () use ($group, $ids, $user) {
-                // Check if group still exists within transaction
                 $group        = Group::lockForUpdate()->find($group->id);
                 $twofaccounts = TwoFAccount::sharedLock()->find($ids);
+                
+                if (! $group) {
+                    throw new ModelNotFoundException('group no longer exists');
+                }
 
                 if ($user->cannot('updateEach', [(new TwoFAccount), $twofaccounts])) {
                     throw new AuthorizationException;
                 }
 
-                // Proceed with assignment
                 $group->twofaccounts()->saveMany($twofaccounts);
-                $group->loadCount('twofaccounts');
     
                 Log::info(sprintf('Twofaccounts #%s assigned to group %s (ID #%s)', implode(',', $ids), var_export($group->name, true), $group->id));
             });
 
+            $group->loadCount('twofaccounts');
         } else {
             Log::info('Cannot find a group to assign the TwoFAccounts to');
         }