Fixed #19131 - tighter validation for company_id/company_ids
This commit is contained in:
@@ -34,6 +34,8 @@ class SaveUserRequest extends FormRequest
|
||||
'department_id' => 'nullable|integer|exists:departments,id',
|
||||
'manager_id' => 'nullable|integer|exists:users,id',
|
||||
'company_id' => ['nullable', 'integer', 'exists:companies,id'],
|
||||
'company_ids' => 'nullable|array',
|
||||
'company_ids.*' => 'integer|exists:companies,id',
|
||||
];
|
||||
|
||||
switch ($this->method()) {
|
||||
@@ -52,13 +54,13 @@ class SaveUserRequest extends FormRequest
|
||||
$rules['first_name'] = 'required|string|min:1';
|
||||
$rules['username'] = 'required_unless:ldap_import,1|string|min:1';
|
||||
$rules['password'] = Setting::passwordComplexityRulesSaving('update').'|confirmed';
|
||||
$rules['company_id'] = [new UserCannotSwitchCompaniesIfItemsAssigned];
|
||||
$rules['company_id'] = ['nullable', 'integer', 'exists:companies,id', new UserCannotSwitchCompaniesIfItemsAssigned];
|
||||
break;
|
||||
|
||||
// Save only what's passed
|
||||
case 'PATCH':
|
||||
$rules['password'] = Setting::passwordComplexityRulesSaving('update');
|
||||
$rules['company_id'] = [new UserCannotSwitchCompaniesIfItemsAssigned];
|
||||
$rules['company_id'] = ['nullable', 'integer', 'exists:companies,id', new UserCannotSwitchCompaniesIfItemsAssigned];
|
||||
break;
|
||||
|
||||
default:
|
||||
|
||||
@@ -153,4 +153,83 @@ class UserCompanyMembershipTest extends TestCase
|
||||
$this->assertTrue($ids->contains($unassignedUser->id), 'Should see user with no company');
|
||||
$this->assertTrue($ids->contains($actor->id), 'Should see self');
|
||||
}
|
||||
|
||||
public function test_patch_with_invalid_company_id_returns_error()
|
||||
{
|
||||
$company = Company::factory()->create();
|
||||
$user = User::factory()->create(['company_id' => $company->id]);
|
||||
$user->companies()->sync([$company->id]);
|
||||
|
||||
$actor = User::factory()->superuser()->create();
|
||||
|
||||
$this->actingAsForApi($actor)
|
||||
->patchJson(route('api.users.update', $user), [
|
||||
'company_id' => 99999999,
|
||||
])
|
||||
->assertStatus(200)
|
||||
->assertStatusMessageIs('error');
|
||||
|
||||
$user->refresh();
|
||||
$this->assertEquals($company->id, $user->company_id, 'company_id should not be changed on invalid input');
|
||||
}
|
||||
|
||||
public function test_put_with_invalid_company_id_returns_error()
|
||||
{
|
||||
$company = Company::factory()->create();
|
||||
$user = User::factory()->create(['company_id' => $company->id]);
|
||||
|
||||
$actor = User::factory()->superuser()->create();
|
||||
|
||||
$this->actingAsForApi($actor)
|
||||
->putJson(route('api.users.update', $user), [
|
||||
'first_name' => $user->first_name,
|
||||
'last_name' => $user->last_name,
|
||||
'username' => $user->username,
|
||||
'company_id' => 99999999,
|
||||
])
|
||||
->assertStatus(200)
|
||||
->assertStatusMessageIs('error');
|
||||
|
||||
$user->refresh();
|
||||
$this->assertEquals($company->id, $user->company_id, 'company_id should not be changed on invalid input');
|
||||
}
|
||||
|
||||
public function test_patch_with_invalid_company_ids_returns_error()
|
||||
{
|
||||
$company = Company::factory()->create();
|
||||
$user = User::factory()->create(['company_id' => $company->id]);
|
||||
$user->companies()->sync([$company->id]);
|
||||
|
||||
$actor = User::factory()->superuser()->create();
|
||||
|
||||
$this->actingAsForApi($actor)
|
||||
->patchJson(route('api.users.update', $user), [
|
||||
'company_ids' => [99999999, 88888888],
|
||||
])
|
||||
->assertStatus(200)
|
||||
->assertStatusMessageIs('error');
|
||||
|
||||
$user->refresh();
|
||||
$this->assertCount(1, $user->companies, 'Company pivot should not be changed on invalid input');
|
||||
$this->assertTrue($user->companies->contains($company));
|
||||
}
|
||||
|
||||
public function test_post_with_invalid_company_ids_returns_error()
|
||||
{
|
||||
$actor = User::factory()->superuser()->create();
|
||||
|
||||
$this->actingAsForApi($actor)
|
||||
->postJson(route('api.users.store'), [
|
||||
'first_name' => 'Test',
|
||||
'last_name' => 'User',
|
||||
'username' => 'testuser_invalid_companies',
|
||||
'password' => 'secret123456',
|
||||
'password_confirmation' => 'secret123456',
|
||||
'company_ids' => [99999999],
|
||||
])
|
||||
->assertStatus(200)
|
||||
->assertStatusMessageIs('error');
|
||||
|
||||
$this->assertNull(User::where('username', 'testuser_invalid_companies')->first());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user