Enhance test coverage & Fix small issues & Refactoring

app/Api/v1/Controllers/TwoFAccountController.php

@@ -42,10 +42,6 @@ public function index(TwoFAccountIndexRequest $request)
 
         $validated = $request->validated();
 
-        // if ($request->has('withOtp')) {
-        //     $request->merge(['at' => time()]);
-        // }
-
         return Arr::has($validated, 'ids')
             ? new TwoFAccountCollection($request->user()->twofaccounts()->whereIn('id', Helpers::commaSeparatedToArray($validated['ids']))->get()->sortBy('order_column'))
             : new TwoFAccountCollection($request->user()->twofaccounts->sortBy('order_column'));

app/Api/v1/Controllers/UserManagerController.php

@@ -4,7 +4,7 @@
 
 use App\Api\v1\Requests\UserManagerPromoteRequest;
 use App\Api\v1\Requests\UserManagerStoreRequest;
-use App\Api\v1\Resources\UserAuthentication;
+use App\Api\v1\Resources\UserAuthenticationResource;
 use App\Api\v1\Resources\UserManagerResource;
 use App\Http\Controllers\Controller;
 use App\Models\User;
@@ -231,7 +231,7 @@ public function authentications(Request $request, User $user)
         $authentications = $request->has('period') ? $user->authenticationsByPeriod($validated['period']) : $user->authentications;
         $authentications = $request->has('limit') ? $authentications->take($validated['limit']) : $authentications;
 
-        return UserAuthentication::collection($authentications);
+        return UserAuthenticationResource::collection($authentications);
     }
 
     /**

app/Api/v1/Requests/IconFetchRequest.php

@@ -25,7 +25,7 @@ public function authorize()
     public function rules()
     {
         return [
-            'service' => 'string|regex:/^[^:]+$/i',
+            'service' => 'string',
         ];
     }
 
@@ -39,7 +39,7 @@ public function rules()
     protected function prepareForValidation()
     {
         $this->merge([
-            'service' => strip_tags($this->service),
+            'service' => strip_tags(strval($this->service)),
         ]);
     }
 }

app/Api/v1/Resources/TwoFAccountCollection.php

@@ -29,8 +29,11 @@ public function toArray($request)
             $request->merge(['withSecret' => false]);
         }
 
+        // Here we add a timestamp to the request if OTPs have to be in the response.
+        // The 'at' parameter is used by the TwoFAccountReadResource class to obtain
+        // all OTPs at the same timestamps
         if ($request->has('withOtp')) {
-            $request->merge(['at' => time()]);
+            $request->merge(['at' => now()->timestamp]);
         }
 
         return $this->collection;

app/Api/v1/Resources/UserAuthentication.php → app/Api/v1/Resources/UserAuthenticationResource.php

@@ -20,7 +20,7 @@
  * @property string|null $duration
  * @property string|null $login_method
  */
-class UserAuthentication extends JsonResource
+class UserAuthenticationResource extends JsonResource
 {
     /**
      * A user agent parser instance.

app/Listeners/Authentication/FailedLoginListener.php

@@ -24,7 +24,7 @@
 
 namespace App\Listeners\Authentication;
 
-use App\Notifications\FailedLogin;
+use App\Notifications\FailedLoginNotification;
 use Illuminate\Auth\Events\Failed;
 
 class FailedLoginListener extends AbstractAccessListener
@@ -56,7 +56,7 @@ public function handle(mixed $event) : void
             ]);
 
             if ($user->preferences['notifyOnFailedLogin'] == true) {
-                $user->notify(new FailedLogin($log));
+                $user->notify(new FailedLoginNotification($log));
             }
         }
     }

app/Listeners/Authentication/LoginListener.php

@@ -24,7 +24,7 @@
 
 namespace App\Listeners\Authentication;
 
-use App\Notifications\SignedInWithNewDevice;
+use App\Notifications\SignedInWithNewDeviceNotification;
 use Illuminate\Auth\Events\Login;
 use Illuminate\Support\Carbon;
 
@@ -59,7 +59,7 @@ public function handle(mixed $event) : void
         ]);
 
         if (! $known && ! $newUser && $user->preferences['notifyOnNewAuthDevice'] == true) {
-            $user->notify(new SignedInWithNewDevice($log));
+            $user->notify(new SignedInWithNewDeviceNotification($log));
         }
     }
 }

app/Listeners/Authentication/VisitedByProxyUserListener.php

@@ -4,7 +4,7 @@
 
 use App\Events\VisitedByProxyUser;
 use App\Extensions\RemoteUserProvider;
-use App\Notifications\SignedInWithNewDevice;
+use App\Notifications\SignedInWithNewDeviceNotification;
 use Illuminate\Support\Carbon;
 
 class VisitedByProxyUserListener extends AbstractAccessListener
@@ -37,7 +37,7 @@ public function handle(mixed $event) : void
         ]);
 
         if (! $known && ! $newUser && ! str_ends_with($user->email, RemoteUserProvider::FAKE_REMOTE_DOMAIN) && $user->preferences['notifyOnNewAuthDevice']) {
-            $user->notify(new SignedInWithNewDevice($log));
+            $user->notify(new SignedInWithNewDeviceNotification($log));
         }
     }
 }

app/Listeners/LogNotification.php → app/Listeners/LogNotificationListener.php

@@ -5,7 +5,7 @@
 use Illuminate\Notifications\Events\NotificationSent;
 use Illuminate\Support\Facades\Log;
 
-class LogNotification
+class LogNotificationListener
 {
     /**
      * Create the event listener.

app/Notifications/FailedLogin.php → app/Notifications/FailedLoginNotification.php

@@ -9,7 +9,7 @@
 use Illuminate\Notifications\Notification;
 use Jenssegers\Agent\Agent;
 
-class FailedLogin extends Notification implements ShouldQueue
+class FailedLoginNotification extends Notification implements ShouldQueue
 {
     use Queueable;
 
@@ -26,7 +26,7 @@ class FailedLogin extends Notification implements ShouldQueue
     public AuthLog $authLog;
 
     /**
-     * Create a new FailedLogin instance
+     * Create a new FailedLoginNotification instance
      */
     public function __construct(AuthLog $authLog)
     {

app/Notifications/SignedInWithNewDevice.php → app/Notifications/SignedInWithNewDeviceNotification.php

@@ -9,7 +9,7 @@
 use Illuminate\Notifications\Notification;
 use Jenssegers\Agent\Agent;
 
-class SignedInWithNewDevice extends Notification implements ShouldQueue
+class SignedInWithNewDeviceNotification extends Notification implements ShouldQueue
 {
     use Queueable;
 
@@ -26,7 +26,7 @@ class SignedInWithNewDevice extends Notification implements ShouldQueue
     protected $agent;
 
     /**
-     * Create a new SignedInWithNewDevice instance
+     * Create a new SignedInWithNewDeviceNotification instance
      */
     public function __construct(AuthLog $authLog)
     {
@@ -47,7 +47,7 @@ public function toMail(mixed $notifiable) : MailMessage
     {
         return (new MailMessage())
             ->subject(__('notifications.new_device.subject'))
-            ->markdown('emails.SignedInWithNewDevice', [
+            ->markdown('emails.signedInWithNewDevice', [
                 'account'   => $notifiable,
                 'time'      => $this->authLog->login_at,
                 'ipAddress' => $this->authLog->ip_address,

app/Policies/UserPolicy.php

@@ -24,13 +24,16 @@ public function before(User $user, string $ability) : ?bool
     /**
      * Determine whether the user can view any models.
      */
-    public function viewAny(User $user) : bool
-    {
-        return false;
-    }
+    // public function viewAny(User $user) : bool
+    // {
+    //     return false;
+    // }
 
     /**
      * Determine whether the user can view the model.
+     * 
+     * @codeCoverageIgnore
+     * Ignored as long as the before() method restrict the access to admins only
      */
     public function view(User $user, User $model) : bool
     {
@@ -45,6 +48,9 @@ public function view(User $user, User $model) : bool
 
     /**
      * Determine whether the user can create models.
+     * 
+     * @codeCoverageIgnore
+     * Ignored as long as the before() method restrict the access to admins only
      */
     public function create(?User $user) : bool
     {
@@ -53,6 +59,8 @@ public function create(?User $user) : bool
 
     /**
      * Determine whether the user can update the model.
+     * 
+     * Not ignored because the user can update itself
      */
     public function update(User $user, User $model) : bool
     {
@@ -67,6 +75,9 @@ public function update(User $user, User $model) : bool
 
     /**
      * Determine whether the user can delete the model.
+     * 
+     * @codeCoverageIgnore
+     * Ignored as long as the before() method restrict the access to admins only
      */
     public function delete(User $user, User $model) : bool
     {
@@ -81,6 +92,9 @@ public function delete(User $user, User $model) : bool
 
     /**
      * Determine whether the user can promote the model.
+     * 
+     * @codeCoverageIgnore
+     * Ignored as long as the before() method restrict the access to admins only
      */
     public function promote(User $user) : bool
     {

app/Providers/EventServiceProvider.php

@@ -13,7 +13,7 @@
 use App\Listeners\Authentication\VisitedByProxyUserListener;
 use App\Listeners\CleanIconStorage;
 use App\Listeners\DissociateTwofaccountFromGroup;
-use App\Listeners\LogNotification;
+use App\Listeners\LogNotificationListener;
 use App\Listeners\RegisterOpenId;
 use App\Listeners\ReleaseRadar;
 use App\Listeners\ResetUsersPreference;
@@ -55,7 +55,7 @@ class EventServiceProvider extends ServiceProvider
             RegisterOpenId::class,
         ],
         NotificationSent::class => [
-            LogNotification::class,
+            LogNotificationListener::class,
         ],
         Login::class => [
             LoginListener::class,

docker/entrypoint.sh

@@ -60,9 +60,11 @@ fi
 
 echo "${COMMIT}" > /2fauth/installed
 php artisan storage:link --quiet
-php artisan optimize:clear
-php artisan config:cache
-php artisan route:cache
-php artisan view:cache
+
+# Clearing compiled, cache has already been cleared
+php artisan clear-compiled
+
+# Clearing and Caching config, events, routes, views
+php artisan optimize
 
 supervisord

routes/web.php

@@ -23,7 +23,7 @@
 use Laravel\Passport\Http\Controllers\PersonalAccessTokenController;
 
 // use App\Models\User;
-// use App\Notifications\SignedInWithNewDevice;
+// use App\Notifications\SignedInWithNewDeviceNotification;
 // use App\Models\AuthLog;
 
 /*
@@ -109,7 +109,7 @@
 
 // Route::get('/notification', function () {
 //     $user = User::find(1);
-//     return (new SignedInWithNewDevice(AuthLog::find(9)))
+//     return (new SignedInWithNewDeviceNotification(AuthLog::find(9)))
 //         ->toMail($user);
 // });
 

tests/Api/v1/Controllers/GroupControllerTest.php

@@ -4,6 +4,7 @@
 
 use App\Api\v1\Controllers\GroupController;
 use App\Api\v1\Resources\GroupResource;
+use App\Listeners\DissociateTwofaccountFromGroup;
 use App\Listeners\ResetUsersPreference;
 use App\Models\Group;
 use App\Models\TwoFAccount;
@@ -18,6 +19,7 @@
 #[CoversClass(ResetUsersPreference::class)]
 #[CoversClass(GroupPolicy::class)]
 #[CoversClass(Group::class)]
+#[CoversClass(DissociateTwofaccountFromGroup::class)]
 class GroupControllerTest extends FeatureTestCase
 {
     /**
@@ -103,6 +105,27 @@ public function test_index_returns_user_groups_only_with_pseudo_group()
             ]);
     }
 
+    #[Test]
+    public function test_orphan_groups_are_reassign_to_the_only_user()
+    {
+        config(['auth.defaults.guard' => 'reverse-proxy-guard']);
+
+        $this->anotherUser->delete();
+        $this->userGroupA->user_id = null;
+        $this->userGroupA->save();
+
+        $this->assertCount(1, User::all());
+        $this->assertNull($this->userGroupA->user_id);
+
+        $this->actingAs($this->user, 'reverse-proxy-guard')
+            ->json('GET', '/api/v1/groups')
+            ->assertOk();
+
+        $this->userGroupA->refresh();
+
+        $this->assertNotNull($this->userGroupA->user_id);
+    }
+
     #[Test]
     public function test_store_returns_created_group_resource()
     {

tests/Api/v1/Controllers/SettingControllerTest.php

@@ -3,19 +3,22 @@
 namespace Tests\Api\v1\Controllers;
 
 use App\Api\v1\Controllers\SettingController;
+use App\Api\v1\Requests\SettingUpdateRequest;
 use App\Facades\Settings;
 use App\Models\User;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Facades\Route;
 use Illuminate\Support\Str;
 use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\CoversMethod;
 use PHPUnit\Framework\Attributes\Test;
 use Tests\FeatureTestCase;
 
 /**
  * SettingController test class
  */
 #[CoversClass(SettingController::class)]
+#[CoversMethod(SettingUpdateRequest::class, 'rules')]
 class SettingControllerTest extends FeatureTestCase
 {
     /**
@@ -230,6 +233,16 @@ public function test_update_missing_user_setting_returns_created_setting()
             ]);
     }
 
+    #[Test]
+    public function test_update_restrictList_setting_rejects_invalid_email_list()
+    {
+        $response = $this->actingAs($this->admin, 'api-guard')
+            ->json('PUT', '/api/v1/settings/restrictList', [
+                'value' => '[email protected]|janedoeexamplecom',
+            ])
+            ->assertJsonValidationErrorFor('value');
+    }
+
     #[Test]
     public function test_destroy_user_setting_returns_success()
     {