Browse Source

Handle missing group during group assignment

Bubka 5 months ago
parent
commit
166b39beea

+ 6 - 1
app/Api/v1/Controllers/GroupController.php

@@ -10,6 +10,7 @@ use App\Facades\Groups;
 use App\Http\Controllers\Controller;
 use App\Http\Controllers\Controller;
 use App\Models\Group;
 use App\Models\Group;
 use App\Models\User;
 use App\Models\User;
+use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Illuminate\Http\Request;
 use Illuminate\Http\Request;
 
 
 class GroupController extends Controller
 class GroupController extends Controller
@@ -93,7 +94,11 @@ class GroupController extends Controller
 
 
         $validated = $request->validated();
         $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);
         return new GroupResource($group);
     }
     }

+ 13 - 2
app/Api/v1/Controllers/TwoFAccountController.php

@@ -21,6 +21,7 @@ use App\Helpers\Helpers;
 use App\Http\Controllers\Controller;
 use App\Http\Controllers\Controller;
 use App\Models\TwoFAccount;
 use App\Models\TwoFAccount;
 use App\Models\User;
 use App\Models\User;
+use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Illuminate\Http\Request;
 use Illuminate\Http\Request;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Arr;
 
 
@@ -89,7 +90,12 @@ class TwoFAccountController extends Controller
         $request->user()->twofaccounts()->save($twofaccount);
         $request->user()->twofaccounts()->save($twofaccount);
 
 
         // Possible group association
         // 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()))
         return (new TwoFAccountReadResource($twofaccount->refresh()))
             ->response()
             ->response()
@@ -116,7 +122,12 @@ class TwoFAccountController extends Controller
             if ((int) $groupId === 0) {
             if ((int) $groupId === 0) {
                 TwoFAccounts::withdraw($twofaccount->id);
                 TwoFAccounts::withdraw($twofaccount->id);
             } else {
             } 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();
             $twofaccount->refresh();
         }
         }

+ 7 - 3
app/Services/GroupService.php

@@ -7,6 +7,7 @@ use App\Models\TwoFAccount;
 use App\Models\User;
 use App\Models\User;
 use Illuminate\Auth\Access\AuthorizationException;
 use Illuminate\Auth\Access\AuthorizationException;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Database\Eloquent\Collection;
+use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Log;
 
 
@@ -20,6 +21,7 @@ class GroupService
      * @param  mixed  $targetGroup  The group the accounts should be assigned to
      * @param  mixed  $targetGroup  The group the accounts should be assigned to
      *
      *
      * @throws \Illuminate\Auth\Access\AuthorizationException
      * @throws \Illuminate\Auth\Access\AuthorizationException
+     * @throws \Illuminate\Database\Eloquent\ModelNotFoundException
      */
      */
     public static function assign($ids, User $user, mixed $targetGroup = null) : void
     public static function assign($ids, User $user, mixed $targetGroup = null) : void
     {
     {
@@ -57,21 +59,23 @@ class GroupService
             $ids = is_array($ids) ? $ids : [$ids];
             $ids = is_array($ids) ? $ids : [$ids];
 
 
             DB::transaction(function () use ($group, $ids, $user) {
             DB::transaction(function () use ($group, $ids, $user) {
-                // Check if group still exists within transaction
                 $group        = Group::lockForUpdate()->find($group->id);
                 $group        = Group::lockForUpdate()->find($group->id);
                 $twofaccounts = TwoFAccount::sharedLock()->find($ids);
                 $twofaccounts = TwoFAccount::sharedLock()->find($ids);
+                
+                if (! $group) {
+                    throw new ModelNotFoundException('group no longer exists');
+                }
 
 
                 if ($user->cannot('updateEach', [(new TwoFAccount), $twofaccounts])) {
                 if ($user->cannot('updateEach', [(new TwoFAccount), $twofaccounts])) {
                     throw new AuthorizationException;
                     throw new AuthorizationException;
                 }
                 }
 
 
-                // Proceed with assignment
                 $group->twofaccounts()->saveMany($twofaccounts);
                 $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));
                 Log::info(sprintf('Twofaccounts #%s assigned to group %s (ID #%s)', implode(',', $ids), var_export($group->name, true), $group->id));
             });
             });
 
 
+            $group->loadCount('twofaccounts');
         } else {
         } else {
             Log::info('Cannot find a group to assign the TwoFAccounts to');
             Log::info('Cannot find a group to assign the TwoFAccounts to');
         }
         }