Skip to content

Commit 2dcfca1

Browse files
murrantStyleCIBot
murrant
and
StyleCIBot
authored
Validate passwords against compromised password lists (librenms#18380)
* Validate passwords against compromised password lists * Revamp the user:add command WIP * Working, but not a form * Actual good implementation * Improve validatePromptInput by returning a callable * Apply fixes from StyleCI * Add password.uncompromised setting --------- Co-authored-by: StyleCI Bot <[email protected]>
1 parent e20ec47 commit 2dcfca1

File tree

9 files changed

+157
-42
lines changed

9 files changed

+157
-42
lines changed

‎app/Console/Commands/AddUserCommand.php‎

Lines changed: 106 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -29,76 +29,148 @@
2929
use App\Console\LnmsCommand;
3030
use App\Facades\LibrenmsConfig;
3131
use App\Models\User;
32+
use Illuminate\Support\Facades\Validator;
3233
use Illuminate\Validation\Rule;
33-
use LibreNMS\Authentication\LegacyAuth;
34+
use Illuminate\Validation\Rules\Password;
35+
use Illuminate\Validation\ValidationException;
3436
use Spatie\Permission\Models\Role;
3537
use Symfony\Component\Console\Input\InputArgument;
3638
use Symfony\Component\Console\Input\InputOption;
3739

40+
use function Laravel\Prompts\form;
41+
3842
class AddUserCommand extends LnmsCommand
3943
{
4044
protected $name = 'user:add';
4145

42-
/**
43-
* Create a new command instance.
44-
*
45-
* @return void
46-
*/
4746
public function __construct()
4847
{
4948
parent::__construct();
5049

5150
$this->setDescription(__('commands.user:add.description'));
52-
53-
$this->addArgument('username', InputArgument::REQUIRED);
51+
$this->addArgument('username', InputArgument::OPTIONAL);
5452
$this->addOption('password', 'p', InputOption::VALUE_REQUIRED);
55-
$this->addOption('role', 'r', InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, __('commands.user:add.options.role', ['roles' => '[user, global-read, admin]']), ['user']);
53+
$this->addOption('role', 'r', InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY,
54+
__('commands.user:add.options.role', ['roles' => '[user, global-read, admin]']), default: ['user']);
5655
$this->addOption('email', 'e', InputOption::VALUE_REQUIRED);
5756
$this->addOption('full-name', 'l', InputOption::VALUE_REQUIRED);
5857
$this->addOption('descr', 's', InputOption::VALUE_REQUIRED);
5958
}
6059

61-
/**
62-
* Execute the console command.
63-
*/
6460
public function handle(): int
6561
{
66-
if (LibrenmsConfig::get('auth_mechanism') != 'mysql') {
62+
if (LibrenmsConfig::get('auth_mechanism') !== 'mysql') {
6763
$this->warn(__('commands.user:add.wrong-auth'));
6864
}
6965

70-
$roles = Role::query()->pluck('name')
71-
->whenEmpty(fn () => collect(['admin', 'global-read', 'user']));
66+
$availableRoles = Role::pluck('name')->whenEmpty(fn () => collect(['admin', 'global-read', 'user']))->all();
7267

73-
$this->validate([
74-
'username' => ['required', Rule::unique('users', 'username')->where('auth_type', 'mysql')],
75-
'email' => 'nullable|email',
76-
'role.*' => Rule::in($roles),
77-
]);
78-
79-
// set get password
68+
$username = $this->argument('username');
8069
$password = $this->option('password');
81-
if (! $password) {
82-
$password = $this->secret(__('commands.user:add.password-request'));
70+
$roles = $this->option('role');
71+
$roles = empty($roles) ? ['user'] : $roles;
72+
$email = $this->option('email');
73+
$fullName = $this->option('full-name');
74+
$descr = $this->option('descr');
75+
76+
if ($username && $password) {
77+
// cli input method
78+
try {
79+
Validator::make(['username' => $username], ['username' => $this->usernameRules()])->validate();
80+
Validator::make(['password' => $password], ['password' => $this->passwordRules()])->validate();
81+
Validator::make(['roles' => $roles],
82+
['roles' => ['required', 'array', Rule::in($availableRoles)]])->validate();
83+
Validator::make(['email' => $email], ['email' => ['nullable', 'email']])->validate();
84+
} catch (ValidationException $e) {
85+
$this->error($e->getMessage());
86+
87+
return 1;
88+
}
89+
90+
$this->makeUser(
91+
$username,
92+
$password,
93+
$roles,
94+
$email,
95+
$fullName,
96+
$descr,
97+
);
98+
99+
return 0;
83100
}
84101

102+
// interactive input
103+
$data = form()
104+
->text(
105+
label: __('commands.user:add.form.username'),
106+
default: $username ?? '',
107+
required: true,
108+
validate: $this->validatePromptInput('username', $this->usernameRules())
109+
)
110+
->password(
111+
label: __('commands.user:add.form.password'),
112+
required: true,
113+
validate: $this->validatePromptInput('password', $this->passwordRules())
114+
)
115+
->multiselect(
116+
label: __('commands.user:add.form.roles'),
117+
options: $availableRoles,
118+
default: $roles,
119+
required: true,
120+
)
121+
->text(
122+
label: __('commands.user:add.form.email'),
123+
default: $email ?? '',
124+
validate: $this->validatePromptInput('email', ['nullable', 'email'])
125+
)
126+
->text(__('commands.user:add.form.full-name'), default: $fullName ?? '')
127+
->text(__('commands.user:add.form.descr'), default: $descr ?? '')
128+
->submit();
129+
130+
$this->makeUser(...$data);
131+
132+
return 0;
133+
}
134+
135+
private function usernameRules(): array
136+
{
137+
return [
138+
'required',
139+
'string',
140+
'max:255',
141+
Rule::unique('users', 'username')->where('auth_type', 'mysql'),
142+
];
143+
}
144+
145+
private function passwordRules(): array
146+
{
147+
return [
148+
'required',
149+
Password::defaults(),
150+
];
151+
}
152+
153+
private function makeUser(
154+
string $username,
155+
string $password,
156+
array $roles,
157+
?string $email,
158+
?string $fullName,
159+
?string $descr,
160+
): void {
85161
$user = new User([
86-
'username' => $this->argument('username'),
87-
'descr' => $this->option('descr'),
88-
'email' => $this->option('email'),
89-
'realname' => $this->option('full-name'),
162+
'username' => $username,
163+
'realname' => $fullName,
164+
'email' => $email,
165+
'descr' => $descr,
90166
'auth_type' => 'mysql',
91167
]);
92168

93169
$user->setPassword($password);
94-
$user->save();
95-
$user->assignRole($this->option('role'));
96-
97-
$user->auth_id = (string) LegacyAuth::get()->getUserid($user->username) ?: $user->user_id;
170+
$user->save(); // assign roles requires a user_id
171+
$user->assignRole($roles);
98172
$user->save();
99173

100174
$this->info(__('commands.user:add.success', ['username' => $user->username]));
101-
102-
return 0;
103175
}
104176
}

‎app/Console/LnmsCommand.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,19 @@ protected function configureOutputOptions(): void
185185
}
186186
}
187187

188+
protected function validatePromptInput(string $attributeName, string|array $rules): callable
189+
{
190+
return function (string|array $value) use ($attributeName, $rules): ?string {
191+
$validator = Validator::make([$attributeName => $value], [$attributeName => $rules]);
192+
193+
if ($validator->fails()) {
194+
return $validator->errors()->first($attributeName);
195+
}
196+
197+
return null;
198+
};
199+
}
200+
188201
private function getCallable(string $type, string $name): ?callable
189202
{
190203
if (empty($this->{'option' . $type}[$name])) {

‎app/Http/Controllers/Install/MakeUserController.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
use Illuminate\Database\QueryException;
3131
use Illuminate\Http\Request;
3232
use Illuminate\Support\Arr;
33+
use Illuminate\Validation\Rules\Password;
3334
use LibreNMS\Interfaces\InstallerStep;
3435

3536
class MakeUserController extends InstallationController implements InstallerStep
@@ -63,7 +64,7 @@ public function create(Request $request)
6364
{
6465
$this->validate($request, [
6566
'username' => 'required',
66-
'password' => 'required',
67+
'password' => ['required', Password::defaults()],
6768
]);
6869

6970
$message = trans('install.user.failure');

‎app/Http/Requests/StoreUserRequest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22

33
namespace App\Http\Requests;
44

5-
use App\Facades\LibrenmsConfig;
65
use App\Models\User;
76
use Illuminate\Foundation\Http\FormRequest;
87
use Illuminate\Validation\Rule;
8+
use Illuminate\Validation\Rules\Password;
99
use LibreNMS\Authentication\LegacyAuth;
1010
use Spatie\Permission\Models\Role;
1111

@@ -48,7 +48,7 @@ public function rules(): array
4848
'descr' => 'nullable|max:30|alpha_space',
4949
'roles' => 'array',
5050
'roles.*' => 'exists:roles,name',
51-
'new_password' => 'required|confirmed|min:' . LibrenmsConfig::get('password.min_length', 8),
51+
'new_password' => ['required', 'confirmed', Password::defaults()],
5252
'dashboard' => 'int',
5353
];
5454
}

‎app/Http/Requests/UpdateUserRequest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22

33
namespace App\Http\Requests;
44

5-
use App\Facades\LibrenmsConfig;
65
use App\Models\User;
76
use Hash;
87
use Illuminate\Foundation\Http\FormRequest;
98
use Illuminate\Validation\Rule;
9+
use Illuminate\Validation\Rules\Password;
1010
use Spatie\Permission\Models\Role;
1111

1212
class UpdateUserRequest extends FormRequest
@@ -48,7 +48,7 @@ public function rules(): array
4848
'realname' => 'nullable|max:64|alpha_space',
4949
'email' => 'nullable|email|max:64',
5050
'descr' => 'nullable|max:30|alpha_space',
51-
'new_password' => 'nullable|confirmed|min:' . LibrenmsConfig::get('password.min_length', 8),
51+
'new_password' => ['nullable', 'confirmed', Password::defaults()],
5252
'new_password_confirmation' => 'nullable|same:new_password',
5353
'dashboard' => 'int',
5454
'roles' => 'array',
@@ -63,7 +63,7 @@ public function rules(): array
6363
'email' => 'nullable|email|max:64',
6464
'descr' => 'nullable|max:30|alpha_space',
6565
'old_password' => 'nullable|string',
66-
'new_password' => 'nullable|confirmed|min:' . LibrenmsConfig::get('password.min_length', 8),
66+
'new_password' => ['nullable', 'confirmed', Password::defaults()],
6767
'new_password_confirmation' => 'nullable|same:new_password',
6868
'dashboard' => 'int',
6969
];

‎app/Providers/AppServiceProvider.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use Illuminate\Support\Facades\Gate;
1414
use Illuminate\Support\Facades\Log;
1515
use Illuminate\Support\ServiceProvider;
16+
use Illuminate\Validation\Rules\Password;
1617
use LibreNMS\Cache\PermissionsCache;
1718
use LibreNMS\Util\IP;
1819
use LibreNMS\Util\Validate;
@@ -77,6 +78,16 @@ public function boot(): void
7778
$this->bootObservers();
7879
Version::registerAboutCommand();
7980

81+
Password::defaults(function () {
82+
$validation = Password::min(LibrenmsConfig::get('password.min_length', 8));
83+
84+
if (LibrenmsConfig::get('password.uncompromised', true)) {
85+
$validation->uncompromised();
86+
}
87+
88+
return $validation;
89+
});
90+
8091
$this->bootAuth();
8192
}
8293

‎lang/en/commands.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,14 @@
300300
'full-name' => 'Full name for the user',
301301
'role' => 'Set the user to the desired role :roles',
302302
],
303-
'password-request' => "Please enter the user's password",
303+
'form' => [
304+
'username' => 'Username',
305+
'password' => 'Password',
306+
'roles' => 'Select user role(s)',
307+
'email' => 'Email (optional)',
308+
'full-name' => 'Full name (optional)',
309+
'descr' => 'Description (optional)',
310+
],
304311
'success' => 'Successfully added user: :username',
305312
'wrong-auth' => 'Warning! You will not be able to log in with this user because you are not using MySQL auth',
306313
],

‎lang/en/settings.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1588,6 +1588,10 @@
15881588
'description' => 'Minimum password length',
15891589
'help' => 'Passwords shorter than the given length will be rejected',
15901590
],
1591+
'uncompromised' => [
1592+
'description' => 'Require password to be uncompromised',
1593+
'help' => 'Checks password against HaveIBeenPwned database using k-anonymity',
1594+
],
15911595
],
15921596
'peeringdb' => [
15931597
'enabled' => [

‎resources/definitions/config_definitions.json‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5836,6 +5836,13 @@
58365836
"section": "general",
58375837
"order": 6
58385838
},
5839+
"password.uncompromised": {
5840+
"default": true,
5841+
"type": "boolean",
5842+
"group": "auth",
5843+
"section": "general",
5844+
"order": 7
5845+
},
58395846
"peering_descr": {
58405847
"default": [
58415848
"peering"
@@ -6382,7 +6389,7 @@
63826389
"default": false,
63836390
"group": "auth",
63846391
"section": "general",
6385-
"order": 7,
6392+
"order": 20,
63866393
"type": "boolean"
63876394
},
63886395
"radius.default_roles": {

0 commit comments

Comments
 (0)