diff --git a/app/Providers/ValidationServiceProvider.php b/app/Providers/ValidationServiceProvider.php index 3926e68a0e..ba699ab1ca 100644 --- a/app/Providers/ValidationServiceProvider.php +++ b/app/Providers/ValidationServiceProvider.php @@ -461,20 +461,20 @@ class ValidationServiceProvider extends ServiceProvider // - FMCS is off (nothing to enforce) // - null_company_is_floater is on (nulls are legal floaters) // - value is present (form was filled in) - // - no auth context (CLI / seeders / importers bypass — same - // posture as SaveUserRequest's cannot_make_floater gate) - // - acting user is a superuser (they see everything; a null is - // an explicit choice, not an accident) + // - no auth context (CLI, seeders, or importers bypass, matching + // the SaveUserRequest cannot_make_floater gate posture) + // - acting user is a superuser (they see everything, so a null + // is an explicit choice, not an accident) // - acting user has NO company memberships. In strict mode // such users legitimately operate in the null "pseudo-company" - // namespace — Company::scopeCompanyablesDirectly scopes them - // to whereNull($company_id), so null IS a valid company id - // for them. Forcing them to pick a non-null company would + // namespace, where Company::scopeCompanyablesDirectly scopes + // them to whereNull($company_id) and null IS a valid company + // id for them. Forcing them to pick a non-null company would // both lock them out of their normal workflow and produce a // row they wouldn't be able to see afterward. - // extendImplicit (not extend) — Laravel skips "explicit" rules - // when the field is null/absent. Since the whole point of - // fmcs_company is to fire ON blank submissions, it must run + // extendImplicit (not extend) because Laravel skips "explicit" + // rules when the field is null or absent. Since the whole point + // of fmcs_company is to fire ON blank submissions, it must run // implicitly. Same reason built-in rules like `required`, // `filled`, `present`, `accepted` are all registered implicit. Validator::extendImplicit('fmcs_company', function ($attribute, $value, $parameters, $validator) { @@ -502,7 +502,7 @@ class ValidationServiceProvider extends ServiceProvider return false; }); - Validator::replacer('fmcs_company', function ($message, $attribute, $rule, $parameters) { + Validator::replacer('fmcs_company', function ($message) { return str_replace(':attribute', trans('general.company'), $message); }); diff --git a/tests/Feature/Fmcs/FmcsCompanyRuleWiringTest.php b/tests/Feature/Fmcs/FmcsCompanyRuleWiringTest.php new file mode 100644 index 0000000000..3571f0190f --- /dev/null +++ b/tests/Feature/Fmcs/FmcsCompanyRuleWiringTest.php @@ -0,0 +1,54 @@ +assertArrayHasKey('company_id', $rules, $modelClass.' should declare a company_id rule'); + + $companyRule = $rules['company_id']; + $ruleString = is_array($companyRule) ? implode('|', $companyRule) : $companyRule; + + $this->assertStringContainsString( + 'fmcs_company', + $ruleString, + $modelClass.'::rules()[company_id] must include the fmcs_company validator so strict-FMCS mode rejects blank submissions', + ); + } + + public static function companyableModelProvider(): array + { + return [ + 'Asset' => [Asset::class], + 'License' => [License::class], + 'Accessory' => [Accessory::class], + 'Consumable' => [Consumable::class], + 'Component' => [Component::class], + 'Department' => [Department::class], + 'Location' => [Location::class], + ]; + } +} diff --git a/tests/Feature/Fmcs/FmcsCompanyValidatorTest.php b/tests/Feature/Fmcs/FmcsCompanyValidatorTest.php new file mode 100644 index 0000000000..6c07282a3e --- /dev/null +++ b/tests/Feature/Fmcs/FmcsCompanyValidatorTest.php @@ -0,0 +1,132 @@ +settings->enableMultipleFullCompanySupport(); + $this->settings->disableFloaterMode(); + auth()->login(User::factory()->create()); + + $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); + + $this->assertTrue($validator->fails()); + $this->assertArrayHasKey('company_id', $validator->errors()->toArray()); + } + + public function test_rule_accepts_null_in_strict_fmcs_for_superuser() + { + $this->settings->enableMultipleFullCompanySupport(); + $this->settings->disableFloaterMode(); + auth()->login(User::factory()->superuser()->create()); + + $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); + + $this->assertFalse($validator->fails()); + } + + public function test_rule_accepts_null_when_floater_mode_enabled() + { + $this->settings->enableFloaterMode(); + auth()->login(User::factory()->create()); + + $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); + + $this->assertFalse($validator->fails()); + } + + public function test_rule_accepts_null_when_fmcs_off() + { + $this->settings->disableMultipleFullCompanySupport(); + auth()->login(User::factory()->create()); + + $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); + + $this->assertFalse($validator->fails()); + } + + public function test_rule_accepts_non_null_in_strict_fmcs_for_non_superuser() + { + $this->settings->enableMultipleFullCompanySupport(); + $this->settings->disableFloaterMode(); + auth()->login(User::factory()->create()); + $company = Company::factory()->create(); + + $validator = Validator::make(['company_id' => $company->id], ['company_id' => 'fmcs_company']); + + $this->assertFalse($validator->fails()); + } + + public function test_rule_accepts_null_when_no_auth_context() + { + // CLI, seeders, and importers deliberately bypass, matching the + // SaveUserRequest cannot_make_floater gate's posture. + $this->settings->enableMultipleFullCompanySupport(); + $this->settings->disableFloaterMode(); + auth()->logout(); + + $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); + + $this->assertFalse($validator->fails()); + } + + public function test_rule_accepts_null_for_uncompanied_non_superuser_in_strict_mode() + { + // Regression guard for the pseudo-company workflow. Under + // Company::scopeCompanyablesDirectly in strict mode, actors + // with no company memberships are scoped to null-company rows + // (whereNull($column)). Null IS a valid company id for them. + // Forcing them to pick a non-null company would both lock them + // out of their normal workflow AND produce a row they wouldn't + // be able to see afterward. + $this->settings->enableMultipleFullCompanySupport(); + $this->settings->disableFloaterMode(); + + $actor = User::factory()->withoutCompany()->create(); + $this->assertFalse($actor->companies()->exists(), 'test precondition: actor is uncompanied'); + auth()->login($actor); + + $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); + + $this->assertFalse($validator->fails()); + } + + public function test_rule_still_rejects_null_for_companied_non_superuser_in_strict_mode() + { + // Reporter's #19192 case. A non-superuser WITH company + // memberships submitting a null company_id would land an + // invisible row. That must still fail. + $this->settings->enableMultipleFullCompanySupport(); + $this->settings->disableFloaterMode(); + + $company = Company::factory()->create(); + $actor = $company->users()->save(User::factory()->create()); + $this->assertTrue($actor->companies()->exists(), 'test precondition: actor has memberships'); + auth()->login($actor); + + $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); + + $this->assertTrue($validator->fails()); + } +} diff --git a/tests/Feature/Fmcs/FmcsStrictBulkAssetEditTest.php b/tests/Feature/Fmcs/FmcsStrictBulkAssetEditTest.php new file mode 100644 index 0000000000..4e691c27cc --- /dev/null +++ b/tests/Feature/Fmcs/FmcsStrictBulkAssetEditTest.php @@ -0,0 +1,110 @@ +update($updateArray) + * per row, so the model-level fmcs_company rule reaches the bulk path + * via ValidatingTrait. These tests pin the behavior on both the + * "clearing Company should fail" and the "clearing Company is fine for + * uncompanied actors working in the pseudo-company namespace" paths, + * plus a sanity check that ordinary bulk edits touching only unrelated + * fields still succeed. + */ +class FmcsStrictBulkAssetEditTest extends TestCase +{ + public function test_clearing_company_is_rejected_for_companied_non_superuser() + { + // Locked in so a future refactor of the bulk controller cannot + // quietly bypass the gate. + $this->settings->enableMultipleFullCompanySupport(); + $this->settings->disableFloaterMode(); + + $company = Company::factory()->create(); + $actor = $company->users()->save(User::factory()->editAssets()->create()); + $target = Asset::factory()->create(['company_id' => $company->id]); + + $this->actingAs($actor) + ->post(route('hardware/bulksave'), [ + 'ids' => [$target->id => '1'], + 'company_id' => 'clear', + 'bulk_actions' => 'edit', + ]); + + $this->assertEquals($company->id, $target->fresh()->company_id, 'Row company should not have been cleared'); + } + + public function test_clearing_company_is_allowed_for_uncompanied_non_superuser() + { + // Uncompanied non-superusers work in the null pseudo-company + // namespace under strict mode. Bulk-clearing Company on rows + // they own is a legitimate operation for them and the gate + // steps aside. + $this->settings->enableMultipleFullCompanySupport(); + $this->settings->disableFloaterMode(); + + $actor = User::factory()->withoutCompany()->editAssets()->create(); + $target = Asset::factory()->create(['company_id' => null]); + + $this->actingAs($actor) + ->post(route('hardware/bulksave'), [ + 'ids' => [$target->id => '1'], + 'company_id' => 'clear', + 'bulk_actions' => 'edit', + ]); + + $this->assertNull($target->fresh()->company_id); + } + + public function test_editing_unrelated_field_still_works_for_companied_non_superuser() + { + // If the bulk edit doesn't touch Company at all, ValidatingTrait + // sees the existing non-null company_id on each row and passes. + // This is the "make sure we haven't broken ordinary bulk edits" + // sanity guard. + // + // Defensive Setting cache flush. Some upstream tests in the full + // MySQL sweep can leave the memoized Setting instance in a state + // where full_multiple_companies_support looks enabled at the + // moment auth loads but disabled by the time the fmcs_company + // validator reads it, or vice versa. The Support helper's update() + // clears the cache too, but only after both writes have landed. + // Clearing here first pins the pre-state so the two writes below + // are the only source of truth for this test's fmcs_company check. + Setting::$_cache = null; + $this->settings->enableMultipleFullCompanySupport(); + $this->settings->disableFloaterMode(); + + $company = Company::factory()->create(); + $actor = $company->users()->save(User::factory()->editAssets()->create()); + $target = Asset::factory()->create([ + 'company_id' => $company->id, + 'notes' => 'before', + ]); + + $response = $this->actingAs($actor) + ->post(route('hardware/bulksave'), [ + 'ids' => [$target->id => '1'], + 'notes' => 'after', + 'bulk_actions' => 'edit', + ]); + + // Fail loudly if the controller redirected back with a flash + // error (the notes assertion below is a downstream symptom and + // hides the real cause). Seen on MySQL CI as a full-suite flake. + // Pinning the response state up front turns any recurrence into + // an actionable diagnostic. + $response->assertSessionMissing('error'); + + $this->assertEquals('after', $target->fresh()->notes); + $this->assertEquals($company->id, $target->fresh()->company_id); + } +} diff --git a/tests/Feature/Fmcs/FmcsStrictUsersHttpTest.php b/tests/Feature/Fmcs/FmcsStrictUsersHttpTest.php new file mode 100644 index 0000000000..6b1d5a362c --- /dev/null +++ b/tests/Feature/Fmcs/FmcsStrictUsersHttpTest.php @@ -0,0 +1,62 @@ +settings->enableMultipleFullCompanySupport(); + $this->settings->disableFloaterMode(); + + $actor = User::factory()->create(); + $username = 'strict-null-target-'.uniqid(); + + $this->actingAs($actor) + ->post(route('users.store'), [ + 'first_name' => 'Test', + 'last_name' => 'User', + 'username' => $username, + 'email' => $username.'@example.com', + 'password' => 'SomeGreatPassword-123', + 'password_confirmation' => 'SomeGreatPassword-123', + // No company_ids submitted. + ]) + ->assertSessionHasErrors('company_ids'); + + $this->assertDatabaseMissing('users', ['username' => $username]); + } + + public function test_strict_fmcs_allows_empty_company_ids_for_superuser() + { + $this->settings->enableMultipleFullCompanySupport(); + $this->settings->disableFloaterMode(); + + $actor = User::factory()->superuser()->create(); + $username = 'super-null-'.uniqid(); + + $this->actingAs($actor) + ->post(route('users.store'), [ + 'first_name' => 'Superuser-Created', + 'last_name' => 'User', + 'username' => $username, + 'email' => $username.'@example.com', + 'password' => 'SomeGreatPassword-123', + 'password_confirmation' => 'SomeGreatPassword-123', + ]) + ->assertSessionHasNoErrors('company_ids'); + } +} diff --git a/tests/Feature/Fmcs/StrictModeRequiresCompanyOnCreateTest.php b/tests/Feature/Fmcs/StrictModeRequiresCompanyOnCreateTest.php deleted file mode 100644 index f73cd20c00..0000000000 --- a/tests/Feature/Fmcs/StrictModeRequiresCompanyOnCreateTest.php +++ /dev/null @@ -1,321 +0,0 @@ -settings->enableMultipleFullCompanySupport(); - $this->settings->disableFloaterMode(); - auth()->login(User::factory()->create()); - - $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); - - $this->assertTrue($validator->fails()); - $this->assertArrayHasKey('company_id', $validator->errors()->toArray()); - } - - public function test_rule_accepts_null_in_strict_fmcs_for_superuser() - { - $this->settings->enableMultipleFullCompanySupport(); - $this->settings->disableFloaterMode(); - auth()->login(User::factory()->superuser()->create()); - - $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); - - $this->assertFalse($validator->fails()); - } - - public function test_rule_accepts_null_when_floater_mode_enabled() - { - $this->settings->enableFloaterMode(); - auth()->login(User::factory()->create()); - - $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); - - $this->assertFalse($validator->fails()); - } - - public function test_rule_accepts_null_when_fmcs_off() - { - $this->settings->disableMultipleFullCompanySupport(); - auth()->login(User::factory()->create()); - - $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); - - $this->assertFalse($validator->fails()); - } - - public function test_rule_accepts_non_null_in_strict_fmcs_for_non_superuser() - { - $this->settings->enableMultipleFullCompanySupport(); - $this->settings->disableFloaterMode(); - auth()->login(User::factory()->create()); - $company = Company::factory()->create(); - - $validator = Validator::make(['company_id' => $company->id], ['company_id' => 'fmcs_company']); - - $this->assertFalse($validator->fails()); - } - - public function test_rule_accepts_null_when_no_auth_context() - { - // CLI / seeders / importers deliberately bypass — same posture - // as the SaveUserRequest cannot_make_floater gate. - $this->settings->enableMultipleFullCompanySupport(); - $this->settings->disableFloaterMode(); - auth()->logout(); - - $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); - - $this->assertFalse($validator->fails()); - } - - public function test_rule_accepts_null_for_uncompanied_non_superuser_in_strict_mode() - { - // Regression guard for the pseudo-company workflow. Under - // Company::scopeCompanyablesDirectly in strict mode, actors - // with no company memberships are scoped to null-company rows - // (whereNull($column)). Null IS a valid company id for them — - // forcing them to pick a non-null company would both lock them - // out of their normal workflow AND produce a row they wouldn't - // be able to see afterward. - $this->settings->enableMultipleFullCompanySupport(); - $this->settings->disableFloaterMode(); - - $actor = User::factory()->withoutCompany()->create(); - $this->assertFalse($actor->companies()->exists(), 'test precondition: actor is uncompanied'); - auth()->login($actor); - - $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); - - $this->assertFalse($validator->fails()); - } - - public function test_rule_still_rejects_null_for_companied_non_superuser_in_strict_mode() - { - // Reporter's #19192 case: a non-superuser WITH company - // memberships submitting a null company_id would land an - // invisible row. That must still fail. - $this->settings->enableMultipleFullCompanySupport(); - $this->settings->disableFloaterMode(); - - $company = Company::factory()->create(); - $actor = $company->users()->save(User::factory()->create()); - $this->assertTrue($actor->companies()->exists(), 'test precondition: actor has memberships'); - auth()->login($actor); - - $validator = Validator::make(['company_id' => null], ['company_id' => 'fmcs_company']); - - $this->assertTrue($validator->fails()); - } - - // ------------------------------------------------------------------ - // Sanity: every model the reporter listed has the rule wired - // ------------------------------------------------------------------ - - /** - * @dataProvider companyableModelProvider - */ - public function test_model_rules_include_fmcs_company_for_company_id(string $modelClass) - { - $rules = $modelClass::rules(); - $this->assertArrayHasKey('company_id', $rules, $modelClass.' should declare a company_id rule'); - - $companyRule = $rules['company_id']; - $ruleString = is_array($companyRule) ? implode('|', $companyRule) : $companyRule; - - $this->assertStringContainsString( - 'fmcs_company', - $ruleString, - $modelClass.'::rules()[company_id] must include the fmcs_company validator so strict-FMCS mode rejects blank submissions', - ); - } - - public static function companyableModelProvider(): array - { - return [ - 'Asset' => [Asset::class], - 'License' => [License::class], - 'Accessory' => [Accessory::class], - 'Consumable' => [Consumable::class], - 'Component' => [Component::class], - 'Department' => [Department::class], - 'Location' => [Location::class], - ]; - } - - // ------------------------------------------------------------------ - // Users: gate lives in SaveUserRequest, not model $rules - // ------------------------------------------------------------------ - - public function test_users_strict_fmcs_rejects_empty_company_ids_for_non_superuser() - { - $this->settings->enableMultipleFullCompanySupport(); - $this->settings->disableFloaterMode(); - - $actor = User::factory()->create(); - $username = 'strict-null-target-'.uniqid(); - - $this->actingAs($actor) - ->post(route('users.store'), [ - 'first_name' => 'Test', - 'last_name' => 'User', - 'username' => $username, - 'email' => $username.'@example.com', - 'password' => 'SomeGreatPassword-123', - 'password_confirmation' => 'SomeGreatPassword-123', - // No company_ids submitted. - ]) - ->assertSessionHasErrors('company_ids'); - - $this->assertDatabaseMissing('users', ['username' => $username]); - } - - // ------------------------------------------------------------------ - // Bulk asset edit: same gate reaches through ValidatingTrait - // ------------------------------------------------------------------ - - public function test_bulk_asset_edit_clear_company_is_rejected_for_companied_non_superuser() - { - // BulkAssetsController::update() calls $asset->update($updateArray) - // per row; the model-level fmcs_company rule fires when - // company_id gets filled to null via the 'clear' bulk option. - // Locked in here so a future refactor of the bulk controller - // can't quietly bypass the gate. - $this->settings->enableMultipleFullCompanySupport(); - $this->settings->disableFloaterMode(); - - $company = \App\Models\Company::factory()->create(); - $actor = $company->users()->save(User::factory()->editAssets()->create()); - $target = \App\Models\Asset::factory()->create(['company_id' => $company->id]); - - $this->actingAs($actor) - ->post(route('hardware/bulksave'), [ - 'ids' => [$target->id => '1'], - 'company_id' => 'clear', - 'bulk_actions' => 'edit', - ]); - - // Row's company should NOT have been cleared. - $this->assertEquals($company->id, $target->fresh()->company_id); - } - - public function test_bulk_asset_edit_clear_company_is_allowed_for_uncompanied_non_superuser() - { - // Uncompanied non-superusers work in the null pseudo-company - // namespace under strict mode. Bulk-clearing Company on rows - // they own is a legitimate operation for them and the gate - // steps aside. - $this->settings->enableMultipleFullCompanySupport(); - $this->settings->disableFloaterMode(); - - $actor = User::factory()->withoutCompany()->editAssets()->create(); - $target = \App\Models\Asset::factory()->create(['company_id' => null]); - - $this->actingAs($actor) - ->post(route('hardware/bulksave'), [ - 'ids' => [$target->id => '1'], - 'company_id' => 'clear', - 'bulk_actions' => 'edit', - ]); - - $this->assertNull($target->fresh()->company_id); - } - - public function test_bulk_asset_edit_unrelated_field_still_works_for_companied_non_superuser() - { - // If the bulk edit doesn't touch Company at all, ValidatingTrait - // sees the existing non-null company_id on each row and passes. - // This is the "make sure we haven't broken ordinary bulk edits" - // sanity guard. - // - // Defensive Setting cache flush. Some upstream tests in the full - // MySQL sweep can leave the memoized Setting instance in a state - // where full_multiple_companies_support looks enabled at the - // moment auth loads but disabled by the time the fmcs_company - // validator reads it, or vice versa. The Support helper's update() - // clears the cache too, but only after both writes have landed. - // Clearing here first pins the pre-state so the two writes below - // are the only source of truth for this test's fmcs_company check. - \App\Models\Setting::$_cache = null; - $this->settings->enableMultipleFullCompanySupport(); - $this->settings->disableFloaterMode(); - - $company = \App\Models\Company::factory()->create(); - $actor = $company->users()->save(User::factory()->editAssets()->create()); - $target = \App\Models\Asset::factory()->create([ - 'company_id' => $company->id, - 'notes' => 'before', - ]); - - $response = $this->actingAs($actor) - ->post(route('hardware/bulksave'), [ - 'ids' => [$target->id => '1'], - 'notes' => 'after', - 'bulk_actions' => 'edit', - ]); - - // Fail loudly if the controller redirected back with a flash - // error (the notes assertion below is a downstream symptom and - // hides the real cause). Seen on MySQL CI as a full-suite flake - // — pinning the response state up front turns any recurrence - // into an actionable diagnostic. - $response->assertSessionMissing('error'); - - $this->assertEquals('after', $target->fresh()->notes); - $this->assertEquals($company->id, $target->fresh()->company_id); - } - - public function test_users_strict_fmcs_allows_empty_company_ids_for_superuser() - { - $this->settings->enableMultipleFullCompanySupport(); - $this->settings->disableFloaterMode(); - - $actor = User::factory()->superuser()->create(); - $username = 'super-null-'.uniqid(); - - $this->actingAs($actor) - ->post(route('users.store'), [ - 'first_name' => 'Superuser-Created', - 'last_name' => 'User', - 'username' => $username, - 'email' => $username.'@example.com', - 'password' => 'SomeGreatPassword-123', - 'password_confirmation' => 'SomeGreatPassword-123', - ]) - ->assertSessionHasNoErrors('company_ids'); - } -}