remove old root_admin column

pelican-dev · Aug 2, 2024 · ebd63d8 · ebd63d8
1 parent ac8e184
commit ebd63d8
Show file tree

Hide file tree

Showing 19 changed files with 65 additions and 56 deletions.
diff --git a/app/Filament/Resources/ServerResource/Pages/CreateServer.php b/app/Filament/Resources/ServerResource/Pages/CreateServer.php
@@ -98,21 +98,6 @@ public function form(Form $form): Form
                                         ->hintIcon('tabler-question-mark')
                                         ->hintIconTooltip('Providing a user password is optional. New user email will prompt users to create a password the first time they login.')
                                         ->password(),
-
-                                    Forms\Components\ToggleButtons::make('root_admin')
-                                        ->label('Administrator (Root)')
-                                        ->options([
-                                            false => 'No',
-                                            true => 'Admin',
-                                        ])
-                                        ->colors([
-                                            false => 'primary',
-                                            true => 'danger',
-                                        ])
-                                        ->inline()
-                                        ->required()
-                                        ->default(false)
-                                        ->hidden(),
                                 ])
                                 ->createOptionUsing(function ($data) {
                                     resolve(UserCreationService::class)->handle($data);

diff --git a/app/Http/Controllers/Api/Client/Servers/ActivityLogController.php b/app/Http/Controllers/Api/Client/Servers/ActivityLogController.php
@@ -13,6 +13,7 @@
 use App\Http\Requests\Api\Client\ClientApiRequest;
 use App\Transformers\Api\Client\ActivityLogTransformer;
 use App\Http\Controllers\Api\Client\ClientApiController;
+use App\Models\Role;
 
 class ActivityLogController extends ClientApiController
 {
@@ -32,15 +33,16 @@ public function __invoke(ClientApiRequest $request, Server $server): array
                 // We could do this with a query and a lot of joins, but that gets pretty
                 // painful so for now we'll execute a simpler query.
                 $subusers = $server->subusers()->pluck('user_id')->merge([$server->owner_id]);
+                $rootAdmins = Role::findOrCreate(Role::ROOT_ADMIN)->users()->pluck('id');
 
                 $builder->select('activity_logs.*')
                     ->leftJoin('users', function (JoinClause $join) {
                         $join->on('users.id', 'activity_logs.actor_id')
                             ->where('activity_logs.actor_type', (new User())->getMorphClass());
                     })
-                    ->where(function (Builder $builder) use ($subusers) {
+                    ->where(function (Builder $builder) use ($subusers, $rootAdmins) {
                         $builder->whereNull('users.id')
-                            ->orWhere('users.root_admin', 0)
+                            ->orWhereNotIn('users.id', $rootAdmins)
                             ->orWhereIn('users.id', $subusers);
                     });
             })

diff --git a/app/Http/Middleware/AdminAuthenticate.php b/app/Http/Middleware/AdminAuthenticate.php
@@ -14,7 +14,7 @@ class AdminAuthenticate
      */
     public function handle(Request $request, \Closure $next): mixed
     {
-        if (!$request->user() || !$request->user()->root_admin) {
+        if (!$request->user() || !$request->user()->isRootAdmin()) {
             throw new AccessDeniedHttpException();
         }
 

diff --git a/app/Http/Middleware/Api/Application/AuthenticateApplicationUser.php b/app/Http/Middleware/Api/Application/AuthenticateApplicationUser.php
@@ -15,7 +15,7 @@ public function handle(Request $request, \Closure $next): mixed
     {
         /** @var \App\Models\User|null $user */
         $user = $request->user();
-        if (!$user || !$user->root_admin) {
+        if (!$user || !$user->isRootAdmin()) {
             throw new AccessDeniedHttpException('This account does not have permission to access the API.');
         }
 

diff --git a/app/Http/Requests/Admin/NewUserFormRequest.php b/app/Http/Requests/Admin/NewUserFormRequest.php
@@ -22,7 +22,6 @@ public function rules(): array
             'name_last',
             'password',
             'language',
-            'root_admin',
         ])->toArray();
     }
 }
diff --git a/app/Http/Requests/Admin/UserFormRequest.php b/app/Http/Requests/Admin/UserFormRequest.php
@@ -22,7 +22,6 @@ public function rules(): array
             'name_last',
             'password',
             'language',
-            'root_admin',
         ])->toArray();
     }
 }
diff --git a/app/Http/Requests/Api/Application/Users/StoreUserRequest.php b/app/Http/Requests/Api/Application/Users/StoreUserRequest.php
@@ -26,7 +26,6 @@ public function rules(array $rules = null): array
             'password',
             'language',
             'timezone',
-            'root_admin',
         ])->toArray();
 
         $response['first_name'] = $rules['name_first'];
@@ -56,7 +55,6 @@ public function attributes(): array
             'external_id' => 'Third Party Identifier',
             'name_first' => 'First Name',
             'name_last' => 'Last Name',
-            'root_admin' => 'Root Administrator Status',
         ];
     }
 }
diff --git a/app/Models/User.php b/app/Models/User.php
@@ -41,7 +41,6 @@
  * @property string|null $remember_token
  * @property string $language
  * @property string $timezone
- * @property bool $root_admin
  * @property bool $use_totp
  * @property string|null $totp_secret
  * @property \Illuminate\Support\Carbon|null $totp_authenticated_at
@@ -78,7 +77,6 @@
  * @method static Builder|User whereNameLast($value)
  * @method static Builder|User wherePassword($value)
  * @method static Builder|User whereRememberToken($value)
- * @method static Builder|User whereRootAdmin($value)
  * @method static Builder|User whereTotpAuthenticatedAt($value)
  * @method static Builder|User whereTotpSecret($value)
  * @method static Builder|User whereUpdatedAt($value)
@@ -133,7 +131,6 @@ class User extends Model implements AuthenticatableContract, AuthorizableContrac
         'totp_secret',
         'totp_authenticated_at',
         'gravatar',
-        'root_admin',
         'oauth',
     ];
 
@@ -147,7 +144,6 @@ class User extends Model implements AuthenticatableContract, AuthorizableContrac
      */
     protected $attributes = [
         'external_id' => null,
-        'root_admin' => false,
         'language' => 'en',
         'timezone' => 'UTC',
         'use_totp' => false,
@@ -168,7 +164,6 @@ class User extends Model implements AuthenticatableContract, AuthorizableContrac
         'name_first' => 'nullable|string|between:0,255',
         'name_last' => 'nullable|string|between:0,255',
         'password' => 'sometimes|nullable|string',
-        'root_admin' => 'boolean',
         'language' => 'string',
         'timezone' => 'string',
         'use_totp' => 'boolean',
@@ -179,7 +174,6 @@ class User extends Model implements AuthenticatableContract, AuthorizableContrac
     protected function casts(): array
     {
         return [
-            'root_admin' => 'boolean',
             'use_totp' => 'boolean',
             'gravatar' => 'boolean',
             'totp_authenticated_at' => 'datetime',
@@ -360,7 +354,7 @@ public function isLastRootAdmin(): bool
 
     public function isRootAdmin(): bool
     {
-        return $this->root_admin || $this->hasRole(Role::ROOT_ADMIN);
+        return $this->hasRole(Role::ROOT_ADMIN);
     }
 
     public function canAccessPanel(Panel $panel): bool

diff --git a/database/Factories/UserFactory.php b/database/Factories/UserFactory.php
@@ -33,19 +33,10 @@ public function definition(): array
             'name_last' => $this->faker->lastName(),
             'password' => $password ?: $password = bcrypt('password'),
             'language' => 'en',
-            'root_admin' => false,
             'use_totp' => false,
             'oauth' => [],
             'created_at' => Carbon::now(),
             'updated_at' => Carbon::now(),
         ];
     }
-
-    /**
-     * Indicate that the user is an admin.
-     */
-    public function admin(): static
-    {
-        return $this->state(['root_admin' => true]);
-    }
 }
diff --git a/database/migrations/2024_08_01_114538_remove_root_admin_column.php b/database/migrations/2024_08_01_114538_remove_root_admin_column.php
@@ -0,0 +1,35 @@
+<?php
+
+use App\Models\Role;
+use App\Models\User;
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+return new class extends Migration
+{
+    /**
+     * Run the migrations.
+     */
+    public function up(): void
+    {
+        $adminUsers = User::whereRootAdmin(1)->get();
+        foreach ($adminUsers as $adminUser) {
+            $adminUser->syncRoles(Role::findOrCreate(Role::ROOT_ADMIN));
+        }
+
+        Schema::table('users', function (Blueprint $table) {
+            $table->dropColumn('root_admin');
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     */
+    public function down(): void
+    {
+        Schema::table('users', function (Blueprint $table) {
+            $table->tinyInteger('root_admin')->unsigned()->default(0);
+        });
+    }
+};
diff --git a/database/schema/mysql-schema.sql b/database/schema/mysql-schema.sql
@@ -630,7 +630,6 @@ CREATE TABLE `users` (
   `password` text COLLATE utf8mb4_unicode_ci NOT NULL,
   `remember_token` varchar(255) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
   `language` char(5) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT 'en',
-  `root_admin` tinyint unsigned NOT NULL DEFAULT '0',
   `use_totp` tinyint unsigned NOT NULL,
   `totp_secret` text COLLATE utf8mb4_unicode_ci,
   `totp_authenticated_at` timestamp NULL DEFAULT NULL,

diff --git a/lang/en/admin/user.php b/lang/en/admin/user.php
@@ -13,7 +13,6 @@
         'hint' => 'This is the last root administrator!',
         'helper_text' => 'You must have at least one root administrator in your system.',
     ],
-    'root_admin' => 'Administrator (Root)',
     'language' => [
         'helper_text1' => 'Your language (:state) has not been translated yet!\nBut never fear, you can help fix that by',
         'helper_text2' => 'contributing directly here',

diff --git a/tests/Integration/Api/Application/ApplicationApiIntegrationTestCase.php b/tests/Integration/Api/Application/ApplicationApiIntegrationTestCase.php
@@ -6,6 +6,7 @@
 use App\Models\User;
 use PHPUnit\Framework\Assert;
 use App\Models\ApiKey;
+use App\Models\Role;
 use App\Services\Acl\Api\AdminAcl;
 use App\Tests\Integration\IntegrationTestCase;
 use Illuminate\Foundation\Testing\DatabaseTransactions;
@@ -67,9 +68,10 @@ protected function createNewDefaultApiKey(User $user, array $permissions = []):
      */
     protected function createApiUser(): User
     {
-        return User::factory()->create([
-            'root_admin' => true,
-        ]);
+        $user = User::factory()->create();
+        $user->syncRoles(Role::findOrCreate(Role::ROOT_ADMIN));
+
+        return $user;
     }
 
     /**

diff --git a/tests/Integration/Api/Application/Users/ExternalUserControllerTest.php b/tests/Integration/Api/Application/Users/ExternalUserControllerTest.php
@@ -38,7 +38,7 @@ public function testGetRemoteUser(): void
                 'first_name' => $user->name_first,
                 'last_name' => $user->name_last,
                 'language' => $user->language,
-                'root_admin' => (bool) $user->root_admin,
+                'root_admin' => (bool) $user->isRootAdmin(),
                 '2fa' => (bool) $user->totp_enabled,
                 'created_at' => $this->formatTimestamp($user->created_at),
                 'updated_at' => $this->formatTimestamp($user->updated_at),

diff --git a/tests/Integration/Api/Application/Users/UserControllerTest.php b/tests/Integration/Api/Application/Users/UserControllerTest.php
@@ -55,7 +55,7 @@ public function testGetUsers(): void
                     'first_name' => $this->getApiUser()->name_first,
                     'last_name' => $this->getApiUser()->name_last,
                     'language' => $this->getApiUser()->language,
-                    'root_admin' => $this->getApiUser()->root_admin,
+                    'root_admin' => $this->getApiUser()->isRootAdmin(),
                     '2fa_enabled' => (bool) $this->getApiUser()->totp_enabled,
                     '2fa' => (bool) $this->getApiUser()->totp_enabled,
                     'created_at' => $this->formatTimestamp($this->getApiUser()->created_at),
@@ -73,7 +73,7 @@ public function testGetUsers(): void
                     'first_name' => $user->name_first,
                     'last_name' => $user->name_last,
                     'language' => $user->language,
-                    'root_admin' => (bool) $user->root_admin,
+                    'root_admin' => (bool) $user->isRootAdmin(),
                     '2fa_enabled' => (bool) $user->totp_enabled,
                     '2fa' => (bool) $user->totp_enabled,
                     'created_at' => $this->formatTimestamp($user->created_at),

diff --git a/tests/Integration/Api/Client/ClientControllerTest.php b/tests/Integration/Api/Client/ClientControllerTest.php
@@ -7,6 +7,7 @@
 use App\Models\Subuser;
 use App\Models\Allocation;
 use App\Models\Permission;
+use App\Models\Role;
 
 class ClientControllerTest extends ClientApiIntegrationTestCase
 {
@@ -47,7 +48,7 @@ public function testServersAreFilteredUsingNameAndUuidInformation(): void
     {
         /** @var \App\Models\User[] $users */
         $users = User::factory()->times(2)->create();
-        $users[0]->update(['root_admin' => true]);
+        $users[0]->syncRoles(Role::findOrCreate(Role::ROOT_ADMIN));
 
         /** @var \App\Models\Server[] $servers */
         $servers = [
@@ -225,7 +226,7 @@ public function testOnlyAdminLevelServersAreReturned(): void
     {
         /** @var \App\Models\User[] $users */
         $users = User::factory()->times(4)->create();
-        $users[0]->update(['root_admin' => true]);
+        $users[0]->syncRoles(Role::findOrCreate(Role::ROOT_ADMIN));
 
         $servers = [
             $this->createServerModel(['user_id' => $users[0]->id]),
@@ -260,7 +261,7 @@ public function testAllServersAreReturnedToAdmin(): void
     {
         /** @var \App\Models\User[] $users */
         $users = User::factory()->times(4)->create();
-        $users[0]->update(['root_admin' => true]);
+        $users[0]->syncRoles(Role::findOrCreate(Role::ROOT_ADMIN));
 
         $servers = [
             $this->createServerModel(['user_id' => $users[0]->id]),

diff --git a/tests/Integration/Api/Remote/SftpAuthenticationControllerTest.php b/tests/Integration/Api/Remote/SftpAuthenticationControllerTest.php
@@ -7,6 +7,7 @@
 use App\Models\User;
 use App\Models\Server;
 use App\Models\Permission;
+use App\Models\Role;
 use App\Models\UserSSHKey;
 use App\Tests\Integration\IntegrationTestCase;
 
@@ -180,7 +181,7 @@ public function testUserPermissionsAreReturnedCorrectly(): void
             ->assertOk()
             ->assertJsonPath('permissions', [Permission::ACTION_FILE_READ, Permission::ACTION_FILE_SFTP]);
 
-        $user->update(['root_admin' => true]);
+        $user->syncRoles(Role::findOrCreate(Role::ROOT_ADMIN));
 
         $this->postJson('/api/remote/sftp/auth', $data)
             ->assertOk()
@@ -193,7 +194,7 @@ public function testUserPermissionsAreReturnedCorrectly(): void
             ->assertOk()
             ->assertJsonPath('permissions.0', '*');
 
-        $user->update(['root_admin' => false]);
+        $user->syncRoles();
         $this->post('/api/remote/sftp/auth', $data)->assertForbidden();
     }
 

diff --git a/tests/Unit/Http/Middleware/AdminAuthenticateTest.php b/tests/Unit/Http/Middleware/AdminAuthenticateTest.php
@@ -4,6 +4,7 @@
 
 use App\Models\User;
 use App\Http\Middleware\AdminAuthenticate;
+use App\Models\Role;
 use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
 
 class AdminAuthenticateTest extends MiddlewareTestCase
@@ -13,7 +14,8 @@ class AdminAuthenticateTest extends MiddlewareTestCase
      */
     public function testAdminsAreAuthenticated(): void
     {
-        $user = User::factory()->make(['root_admin' => 1]);
+        $user = User::factory()->create();
+        $user->syncRoles(Role::findOrCreate(Role::ROOT_ADMIN));
 
         $this->request->shouldReceive('user')->withNoArgs()->twice()->andReturn($user);
 
@@ -39,7 +41,7 @@ public function testExceptionIsThrownIfUserIsNotAnAdmin(): void
     {
         $this->expectException(AccessDeniedHttpException::class);
 
-        $user = User::factory()->make(['root_admin' => 0]);
+        $user = User::factory()->create();
 
         $this->request->shouldReceive('user')->withNoArgs()->twice()->andReturn($user);
 

diff --git a/tests/Unit/Http/Middleware/Api/Application/AuthenticateUserTest.php b/tests/Unit/Http/Middleware/Api/Application/AuthenticateUserTest.php
@@ -5,6 +5,7 @@
 use App\Tests\Unit\Http\Middleware\MiddlewareTestCase;
 use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
 use App\Http\Middleware\Api\Application\AuthenticateApplicationUser;
+use App\Models\Role;
 
 class AuthenticateUserTest extends MiddlewareTestCase
 {
@@ -27,7 +28,7 @@ public function testNonAdminUser(): void
     {
         $this->expectException(AccessDeniedHttpException::class);
 
-        $this->generateRequestUserModel(['root_admin' => false]);
+        $this->generateRequestUserModel();
 
         $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
     }
@@ -37,7 +38,8 @@ public function testNonAdminUser(): void
      */
     public function testAdminUser(): void
     {
-        $this->generateRequestUserModel(['root_admin' => true]);
+        $user = $this->generateRequestUserModel();
+        $user->syncRoles(Role::findOrCreate(Role::ROOT_ADMIN));
 
         $this->getMiddleware()->handle($this->request, $this->getClosureAssertions());
     }