From a961552c23101bcf260aaf930a8362277852f67b Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 5 Aug 2025 13:39:30 +0100 Subject: [PATCH 1/3] Commands: Updated create admin comment to accept extra flags Added flags to target changes to the first default admin user, and to generate a password. This is related to #4575. --- app/Console/Commands/CreateAdminCommand.php | 93 ++++++++++++++++----- app/Users/UserRepo.php | 19 ++++- 2 files changed, 90 insertions(+), 22 deletions(-) diff --git a/app/Console/Commands/CreateAdminCommand.php b/app/Console/Commands/CreateAdminCommand.php index 82b6e50aa52..ce619e05d1a 100644 --- a/app/Console/Commands/CreateAdminCommand.php +++ b/app/Console/Commands/CreateAdminCommand.php @@ -21,7 +21,9 @@ class CreateAdminCommand extends Command {--email= : The email address for the new admin user} {--name= : The name of the new admin user} {--password= : The password to assign to the new admin user} - {--external-auth-id= : The external authentication system id for the new admin user (SAML2/LDAP/OIDC)}'; + {--external-auth-id= : The external authentication system id for the new admin user (SAML2/LDAP/OIDC)} + {--generate-password : Generate a random password for the new admin user} + {--first-admin : Indicate if this should set/update the details of the initial admin user}'; /** * The console command description. @@ -35,23 +37,9 @@ class CreateAdminCommand extends Command */ public function handle(UserRepo $userRepo): int { - $details = $this->snakeCaseOptions(); - - if (empty($details['email'])) { - $details['email'] = $this->ask('Please specify an email address for the new admin user'); - } - - if (empty($details['name'])) { - $details['name'] = $this->ask('Please specify a name for the new admin user'); - } - - if (empty($details['password'])) { - if (empty($details['external_auth_id'])) { - $details['password'] = $this->ask('Please specify a password for the new admin user (8 characters min)'); - } else { - $details['password'] = Str::random(32); - } - } + $firstAdminOnly = $this->option('first-admin'); + $shouldGeneratePassword = $this->option('generate-password'); + $details = $this->gatherDetails($shouldGeneratePassword); $validator = Validator::make($details, [ 'email' => ['required', 'email', 'min:5', new Unique('users', 'email')], @@ -68,16 +56,81 @@ public function handle(UserRepo $userRepo): int return 1; } + $adminRole = Role::getSystemRole('admin'); + + if ($firstAdminOnly) { + $handled = $this->handleFirstAdminIfExists($userRepo, $details, $shouldGeneratePassword, $adminRole); + if ($handled) { + return 0; + } + } + $user = $userRepo->createWithoutActivity($validator->validated()); - $user->attachRole(Role::getSystemRole('admin')); + $user->attachRole($adminRole); $user->email_confirmed = true; $user->save(); - $this->info("Admin account with email \"{$user->email}\" successfully created!"); + if ($shouldGeneratePassword) { + $this->line($details['password']); + } else { + $this->info("Admin account with email \"{$user->email}\" successfully created!"); + } return 0; } + /** + * Handle updates to the first admin if exists. + * Returns true if the action has been handled (user updated or already a non-default admin user) otherwise + * returns false if no action has been taken, and we therefore need to proceed with a normal account creation. + */ + protected function handleFirstAdminIfExists(UserRepo $userRepo, array $data, bool $generatePassword, Role $adminRole): bool + { + $defaultAdmin = $userRepo->getByEmail('admin@admin.com'); + if ($defaultAdmin && $defaultAdmin->hasSystemRole('admin')) { + $userRepo->updateWithoutActivity($defaultAdmin, $data, true); + if ($generatePassword) { + $this->line($data['password']); + } else { + $this->info("The default admin user has been updated with the provided details!"); + } + + return true; + } else if ($adminRole->users()->count() > 0) { + $this->warn('Non-default admin user already exists. Skipping creation of new admin user.'); + return true; + } + + return false; + } + + protected function gatherDetails(bool $generatePassword): array + { + $details = $this->snakeCaseOptions(); + + if (empty($details['email'])) { + $details['email'] = $this->ask('Please specify an email address for the new admin user'); + } + + if (empty($details['name'])) { + $details['name'] = $this->ask('Please specify a name for the new admin user'); + } + + if (empty($details['password'])) { + if (empty($details['external_auth_id'])) { + if ($generatePassword) { + $details['password'] = Str::random(32); + } else { + $details['password'] = $this->ask('Please specify a password for the new admin user (8 characters min)'); + } + } else { + $details['password'] = Str::random(32); + } + } + + return $details; + } + protected function snakeCaseOptions(): array { $returnOpts = []; diff --git a/app/Users/UserRepo.php b/app/Users/UserRepo.php index 5c8ace8faca..d24f7002e71 100644 --- a/app/Users/UserRepo.php +++ b/app/Users/UserRepo.php @@ -100,13 +100,13 @@ public function create(array $data, bool $sendInvite = false): User } /** - * Update the given user with the given data. + * Update the given user with the given data, but do not create an activity. * * @param array{name: ?string, email: ?string, external_auth_id: ?string, password: ?string, roles: ?array, language: ?string} $data * * @throws UserUpdateException */ - public function update(User $user, array $data, bool $manageUsersAllowed): User + public function updateWithoutActivity(User $user, array $data, bool $manageUsersAllowed): User { if (!empty($data['name'])) { $user->name = $data['name']; @@ -134,6 +134,21 @@ public function update(User $user, array $data, bool $manageUsersAllowed): User } $user->save(); + + return $user; + } + + /** + * Update the given user with the given data. + * + * @param array{name: ?string, email: ?string, external_auth_id: ?string, password: ?string, roles: ?array, language: ?string} $data + * + * @throws UserUpdateException + */ + public function update(User $user, array $data, bool $manageUsersAllowed): User + { + $user = $this->updateWithoutActivity($user, $data, $manageUsersAllowed); + Activity::add(ActivityType::USER_UPDATE, $user); return $user; From 2eefbd21c1cdb6a3b32529fd6db10f9bbfd4b9da Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 5 Aug 2025 16:43:06 +0100 Subject: [PATCH 2/3] Commands: Added testing for initial admin changes - Also changed first-admin to initial. - Updated initial handling to not require email/name to be passed, using defaults instead. - Adds missing existing email use check. --- app/Console/Commands/CreateAdminCommand.php | 53 ++++-- tests/Commands/CreateAdminCommandTest.php | 185 +++++++++++++++++++- 2 files changed, 212 insertions(+), 26 deletions(-) diff --git a/app/Console/Commands/CreateAdminCommand.php b/app/Console/Commands/CreateAdminCommand.php index ce619e05d1a..520f0822a66 100644 --- a/app/Console/Commands/CreateAdminCommand.php +++ b/app/Console/Commands/CreateAdminCommand.php @@ -8,7 +8,6 @@ use Illuminate\Support\Facades\Validator; use Illuminate\Support\Str; use Illuminate\Validation\Rules\Password; -use Illuminate\Validation\Rules\Unique; class CreateAdminCommand extends Command { @@ -23,7 +22,7 @@ class CreateAdminCommand extends Command {--password= : The password to assign to the new admin user} {--external-auth-id= : The external authentication system id for the new admin user (SAML2/LDAP/OIDC)} {--generate-password : Generate a random password for the new admin user} - {--first-admin : Indicate if this should set/update the details of the initial admin user}'; + {--initial : Indicate if this should set/update the details of the initial admin user}'; /** * The console command description. @@ -37,12 +36,12 @@ class CreateAdminCommand extends Command */ public function handle(UserRepo $userRepo): int { - $firstAdminOnly = $this->option('first-admin'); + $initialAdminOnly = $this->option('initial'); $shouldGeneratePassword = $this->option('generate-password'); - $details = $this->gatherDetails($shouldGeneratePassword); + $details = $this->gatherDetails($shouldGeneratePassword, $initialAdminOnly); $validator = Validator::make($details, [ - 'email' => ['required', 'email', 'min:5', new Unique('users', 'email')], + 'email' => ['required', 'email', 'min:5'], 'name' => ['required', 'min:2'], 'password' => ['required_without:external_auth_id', Password::default()], 'external_auth_id' => ['required_without:password'], @@ -58,13 +57,20 @@ public function handle(UserRepo $userRepo): int $adminRole = Role::getSystemRole('admin'); - if ($firstAdminOnly) { - $handled = $this->handleFirstAdminIfExists($userRepo, $details, $shouldGeneratePassword, $adminRole); - if ($handled) { - return 0; + if ($initialAdminOnly) { + $handled = $this->handleInitialAdminIfExists($userRepo, $details, $shouldGeneratePassword, $adminRole); + if ($handled !== null) { + return $handled ? 0 : 1; } } + $emailUsed = $userRepo->getByEmail($details['email']) !== null; + if ($emailUsed) { + $this->error("Could not create admin account."); + $this->error("An account with the email address \"{$details['email']}\" already exists."); + return 1; + } + $user = $userRepo->createWithoutActivity($validator->validated()); $user->attachRole($adminRole); $user->email_confirmed = true; @@ -80,14 +86,19 @@ public function handle(UserRepo $userRepo): int } /** - * Handle updates to the first admin if exists. - * Returns true if the action has been handled (user updated or already a non-default admin user) otherwise - * returns false if no action has been taken, and we therefore need to proceed with a normal account creation. + * Handle updates to the original admin account if it exists. + * Returns true if it's been successfully handled, false if unsuccessful, or null if not handled. */ - protected function handleFirstAdminIfExists(UserRepo $userRepo, array $data, bool $generatePassword, Role $adminRole): bool + protected function handleInitialAdminIfExists(UserRepo $userRepo, array $data, bool $generatePassword, Role $adminRole): bool|null { $defaultAdmin = $userRepo->getByEmail('admin@admin.com'); if ($defaultAdmin && $defaultAdmin->hasSystemRole('admin')) { + if ($defaultAdmin->email !== $data['email'] && $userRepo->getByEmail($data['email']) !== null) { + $this->error("Could not create admin account."); + $this->error("An account with the email address \"{$data['email']}\" already exists."); + return false; + } + $userRepo->updateWithoutActivity($defaultAdmin, $data, true); if ($generatePassword) { $this->line($data['password']); @@ -101,19 +112,27 @@ protected function handleFirstAdminIfExists(UserRepo $userRepo, array $data, boo return true; } - return false; + return null; } - protected function gatherDetails(bool $generatePassword): array + protected function gatherDetails(bool $generatePassword, bool $initialAdmin): array { $details = $this->snakeCaseOptions(); if (empty($details['email'])) { - $details['email'] = $this->ask('Please specify an email address for the new admin user'); + if ($initialAdmin) { + $details['email'] = 'admin@example.com'; + } else { + $details['email'] = $this->ask('Please specify an email address for the new admin user'); + } } if (empty($details['name'])) { - $details['name'] = $this->ask('Please specify a name for the new admin user'); + if ($initialAdmin) { + $details['name'] = 'Admin'; + } else { + $details['name'] = $this->ask('Please specify a name for the new admin user'); + } } if (empty($details['password'])) { diff --git a/tests/Commands/CreateAdminCommandTest.php b/tests/Commands/CreateAdminCommandTest.php index 95a39c497e4..fa1329ad322 100644 --- a/tests/Commands/CreateAdminCommandTest.php +++ b/tests/Commands/CreateAdminCommandTest.php @@ -2,8 +2,11 @@ namespace Tests\Commands; +use BookStack\Users\Models\Role; use BookStack\Users\Models\User; +use Illuminate\Support\Facades\Artisan; use Illuminate\Support\Facades\Auth; +use Illuminate\Support\Facades\Hash; use Tests\TestCase; class CreateAdminCommandTest extends TestCase @@ -11,14 +14,14 @@ class CreateAdminCommandTest extends TestCase public function test_standard_command_usage() { $this->artisan('bookstack:create-admin', [ - '--email' => 'admintest@example.com', - '--name' => 'Admin Test', + '--email' => 'admintest@example.com', + '--name' => 'Admin Test', '--password' => 'testing-4', ])->assertExitCode(0); $this->assertDatabaseHas('users', [ 'email' => 'admintest@example.com', - 'name' => 'Admin Test', + 'name' => 'Admin Test', ]); /** @var User $user */ @@ -30,14 +33,14 @@ public function test_standard_command_usage() public function test_providing_external_auth_id() { $this->artisan('bookstack:create-admin', [ - '--email' => 'admintest@example.com', - '--name' => 'Admin Test', + '--email' => 'admintest@example.com', + '--name' => 'Admin Test', '--external-auth-id' => 'xX_admin_Xx', ])->assertExitCode(0); $this->assertDatabaseHas('users', [ - 'email' => 'admintest@example.com', - 'name' => 'Admin Test', + 'email' => 'admintest@example.com', + 'name' => 'Admin Test', 'external_auth_id' => 'xX_admin_Xx', ]); @@ -50,14 +53,178 @@ public function test_password_required_if_external_auth_id_not_given() { $this->artisan('bookstack:create-admin', [ '--email' => 'admintest@example.com', - '--name' => 'Admin Test', + '--name' => 'Admin Test', ])->expectsQuestion('Please specify a password for the new admin user (8 characters min)', 'hunter2000') ->assertExitCode(0); $this->assertDatabaseHas('users', [ 'email' => 'admintest@example.com', - 'name' => 'Admin Test', + 'name' => 'Admin Test', ]); $this->assertTrue(Auth::attempt(['email' => 'admintest@example.com', 'password' => 'hunter2000'])); } + + public function test_generate_password_option() + { + $this->withoutMockingConsoleOutput() + ->artisan('bookstack:create-admin', [ + '--email' => 'admintest@example.com', + '--name' => 'Admin Test', + '--generate-password' => true, + ]); + + $output = trim(Artisan::output()); + $this->assertMatchesRegularExpression('/^[a-zA-Z0-9]{32}$/', $output); + + $user = User::query()->where('email', '=', 'admintest@example.com')->first(); + $this->assertTrue(Hash::check($output, $user->password)); + } + + public function test_initial_option_updates_default_admin() + { + $defaultAdmin = User::query()->where('email', '=', 'admin@admin.com')->first(); + + $this->artisan('bookstack:create-admin', [ + '--email' => 'firstadmin@example.com', + '--name' => 'Admin Test', + '--password' => 'testing-7', + '--initial' => true, + ])->expectsOutput('The default admin user has been updated with the provided details!') + ->assertExitCode(0); + + $defaultAdmin->refresh(); + + $this->assertEquals('firstadmin@example.com', $defaultAdmin->email); + } + + public function test_initial_option_does_not_update_if_only_non_default_admin_exists() + { + $defaultAdmin = User::query()->where('email', '=', 'admin@admin.com')->first(); + $defaultAdmin->email = 'testadmin@example.com'; + $defaultAdmin->save(); + + $this->artisan('bookstack:create-admin', [ + '--email' => 'firstadmin@example.com', + '--name' => 'Admin Test', + '--password' => 'testing-7', + '--initial' => true, + ])->expectsOutput('Non-default admin user already exists. Skipping creation of new admin user.') + ->assertExitCode(0); + + $defaultAdmin->refresh(); + + $this->assertEquals('testadmin@example.com', $defaultAdmin->email); + } + + public function test_initial_option_updates_creates_new_admin_if_none_exists() + { + $adminRole = Role::getSystemRole('admin'); + $adminRole->users()->delete(); + $this->assertEquals(0, $adminRole->users()->count()); + + $this->artisan('bookstack:create-admin', [ + '--email' => 'firstadmin@example.com', + '--name' => 'My initial admin', + '--password' => 'testing-7', + '--initial' => true, + ])->expectsOutput("Admin account with email \"firstadmin@example.com\" successfully created!") + ->assertExitCode(0); + + $this->assertEquals(1, $adminRole->users()->count()); + $this->assertDatabaseHas('users', [ + 'email' => 'firstadmin@example.com', + 'name' => 'My initial admin', + ]); + } + + public function test_initial_rerun_does_not_error_but_skips() + { + $adminRole = Role::getSystemRole('admin'); + $adminRole->users()->delete(); + + $this->artisan('bookstack:create-admin', [ + '--email' => 'firstadmin@example.com', + '--name' => 'My initial admin', + '--password' => 'testing-7', + '--initial' => true, + ])->expectsOutput("Admin account with email \"firstadmin@example.com\" successfully created!") + ->assertExitCode(0); + + $this->artisan('bookstack:create-admin', [ + '--email' => 'firstadmin@example.com', + '--name' => 'My initial admin', + '--password' => 'testing-7', + '--initial' => true, + ])->expectsOutput("Non-default admin user already exists. Skipping creation of new admin user.") + ->assertExitCode(0); + } + + public function test_initial_option_creation_errors_if_email_already_exists() + { + $adminRole = Role::getSystemRole('admin'); + $adminRole->users()->delete(); + $editor = $this->users->editor(); + + $this->artisan('bookstack:create-admin', [ + '--email' => $editor->email, + '--name' => 'My initial admin', + '--password' => 'testing-7', + '--initial' => true, + ])->expectsOutput("Could not create admin account.") + ->expectsOutput("An account with the email address \"{$editor->email}\" already exists.") + ->assertExitCode(1); + } + + public function test_initial_option_updating_errors_if_email_already_exists() + { + $editor = $this->users->editor(); + $defaultAdmin = User::query()->where('email', '=', 'admin@admin.com')->first(); + $this->assertNotNull($defaultAdmin); + + $this->artisan('bookstack:create-admin', [ + '--email' => $editor->email, + '--name' => 'My initial admin', + '--password' => 'testing-7', + '--initial' => true, + ])->expectsOutput("Could not create admin account.") + ->expectsOutput("An account with the email address \"{$editor->email}\" already exists.") + ->assertExitCode(1); + } + + public function test_initial_option_does_not_require_name_or_email_to_be_passed() + { + $adminRole = Role::getSystemRole('admin'); + $adminRole->users()->delete(); + $this->assertEquals(0, $adminRole->users()->count()); + + $this->artisan('bookstack:create-admin', [ + '--generate-password' => true, + '--initial' => true, + ])->assertExitCode(0); + + $this->assertEquals(1, $adminRole->users()->count()); + $this->assertDatabaseHas('users', [ + 'email' => 'admin@example.com', + 'name' => 'Admin', + ]); + } + + public function test_initial_option_updating_existing_user_with_generate_password_only_outputs_password() + { + $defaultAdmin = User::query()->where('email', '=', 'admin@admin.com')->first(); + + $this->withoutMockingConsoleOutput() + ->artisan('bookstack:create-admin', [ + '--email' => 'firstadmin@example.com', + '--name' => 'Admin Test', + '--generate-password' => true, + '--initial' => true, + ]); + + $output = Artisan::output(); + $this->assertMatchesRegularExpression('/^[a-zA-Z0-9]{32}$/', $output); + + $defaultAdmin->refresh(); + $this->assertEquals('firstadmin@example.com', $defaultAdmin->email); + } } From f36e6fb929d50a2713d1b01f064f2f087188bb69 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 7 Aug 2025 13:16:49 +0100 Subject: [PATCH 3/3] Commands: Updated create admin skip return Return status for skipped --initial creation will now return 2, so that it can be identified seperate from a creation and from an error. --- app/Console/Commands/CreateAdminCommand.php | 12 ++++++------ tests/Commands/CreateAdminCommandTest.php | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/Console/Commands/CreateAdminCommand.php b/app/Console/Commands/CreateAdminCommand.php index 520f0822a66..bf72553f72a 100644 --- a/app/Console/Commands/CreateAdminCommand.php +++ b/app/Console/Commands/CreateAdminCommand.php @@ -60,7 +60,7 @@ public function handle(UserRepo $userRepo): int if ($initialAdminOnly) { $handled = $this->handleInitialAdminIfExists($userRepo, $details, $shouldGeneratePassword, $adminRole); if ($handled !== null) { - return $handled ? 0 : 1; + return $handled; } } @@ -87,16 +87,16 @@ public function handle(UserRepo $userRepo): int /** * Handle updates to the original admin account if it exists. - * Returns true if it's been successfully handled, false if unsuccessful, or null if not handled. + * Returns an int return status if handled, otherwise returns null if not handled (new user to be created). */ - protected function handleInitialAdminIfExists(UserRepo $userRepo, array $data, bool $generatePassword, Role $adminRole): bool|null + protected function handleInitialAdminIfExists(UserRepo $userRepo, array $data, bool $generatePassword, Role $adminRole): int|null { $defaultAdmin = $userRepo->getByEmail('admin@admin.com'); if ($defaultAdmin && $defaultAdmin->hasSystemRole('admin')) { if ($defaultAdmin->email !== $data['email'] && $userRepo->getByEmail($data['email']) !== null) { $this->error("Could not create admin account."); $this->error("An account with the email address \"{$data['email']}\" already exists."); - return false; + return 1; } $userRepo->updateWithoutActivity($defaultAdmin, $data, true); @@ -106,10 +106,10 @@ protected function handleInitialAdminIfExists(UserRepo $userRepo, array $data, b $this->info("The default admin user has been updated with the provided details!"); } - return true; + return 0; } else if ($adminRole->users()->count() > 0) { $this->warn('Non-default admin user already exists. Skipping creation of new admin user.'); - return true; + return 2; } return null; diff --git a/tests/Commands/CreateAdminCommandTest.php b/tests/Commands/CreateAdminCommandTest.php index fa1329ad322..f389dd94235 100644 --- a/tests/Commands/CreateAdminCommandTest.php +++ b/tests/Commands/CreateAdminCommandTest.php @@ -109,7 +109,7 @@ public function test_initial_option_does_not_update_if_only_non_default_admin_ex '--password' => 'testing-7', '--initial' => true, ])->expectsOutput('Non-default admin user already exists. Skipping creation of new admin user.') - ->assertExitCode(0); + ->assertExitCode(2); $defaultAdmin->refresh(); @@ -156,7 +156,7 @@ public function test_initial_rerun_does_not_error_but_skips() '--password' => 'testing-7', '--initial' => true, ])->expectsOutput("Non-default admin user already exists. Skipping creation of new admin user.") - ->assertExitCode(0); + ->assertExitCode(2); } public function test_initial_option_creation_errors_if_email_already_exists()