Added GATED_AUTH_FIELDS to keep auth gated fields consistent

app/Http/Controllers/Api/UsersController.php

@@ -584,10 +584,38 @@ class UsersController extends Controller
             return response()->json(Helper::formatStandardApiResponse('error', null, 'Permission denied. You cannot update user information via API on the demo.'));
         }
 
-        // Pull out sensitive fields that require extra permission
-        $user->fill($request->except(['password', 'username', 'email', 'activated', 'permissions', 'activation_code', 'remember_token', 'two_factor_secret', 'two_factor_enrolled', 'two_factor_optin']));
+        // User::GATED_AUTH_FIELDS enter through the canEditAuthFields branch
+        // below. If the caller does not hold that gate against this target
+        // but their request nevertheless carries any of those fields, fail
+        // loud rather than persisting a partial write while returning a
+        // success response. Prior behavior silently dropped the auth-field
+        // writes and returned `success`, which misrepresented what
+        // actually persisted.
+        $requestedAuthFields = array_values(array_intersect(User::GATED_AUTH_FIELDS, array_keys($request->all())));
+        $canEditAuthFields = auth()->user()->can('canEditAuthFields', $user) && auth()->user()->can('editableOnDemo');
 
-        if (auth()->user()->can('canEditAuthFields', $user) && auth()->user()->can('editableOnDemo')) {
+        if (! empty($requestedAuthFields) && ! $canEditAuthFields) {
+            return response()->json(Helper::formatStandardApiResponse(
+                'error',
+                null,
+                trans('admin/users/message.auth_fields_denied', ['fields' => implode(', ', $requestedAuthFields)]),
+            ));
+        }
+
+        // Pull out sensitive fields that require extra permission. The
+        // GATED_AUTH_FIELDS constant covers user-editable secrets; the
+        // additional keys below are internal state (2FA secrets, remember
+        // tokens, activation codes) that must never be settable from a
+        // request payload regardless of caller privilege.
+        $user->fill($request->except(array_merge(User::GATED_AUTH_FIELDS, [
+            'activation_code',
+            'remember_token',
+            'two_factor_secret',
+            'two_factor_enrolled',
+            'two_factor_optin',
+        ])));
+
+        if ($canEditAuthFields) {
 
             if ($request->filled('password')) {
                 $user->password = bcrypt($request->input('password'));

app/Importer/UserImporter.php

@@ -141,10 +141,25 @@ class UserImporter extends ItemImporter
             // CLI imports run unauthenticated and are fully trusted; only restrict web-initiated imports.
             // Note: unset must target $this->item, not the model — sanitizeItemForUpdating() reads from $this->item.
             if (Auth::check() && (! Auth::user()->hasAccess('users.edit') || ! Gate::allows('canEditAuthFields', $user))) {
-                unset($this->item['username']);
-                unset($this->item['email']);
-                unset($this->item['password']);
-                unset($this->item['activated']);
+                // GATED_AUTH_FIELDS is the shared list across the API,
+                // web-UI, and importer paths. The importer naturally
+                // filters out fields that are not present in the CSV
+                // via array_intersect, so `permissions` (which the
+                // importer never processes) drops out on its own.
+                $deniedAuthFields = array_values(array_intersect(User::GATED_AUTH_FIELDS, array_keys($this->item)));
+                foreach ($deniedAuthFields as $field) {
+                    unset($this->item[$field]);
+                }
+                if (! empty($deniedAuthFields)) {
+                    // Surface the skip in the import summary rather than
+                    // silently persisting a partial row. Halting the whole
+                    // import on the first affected row would be worse UX.
+                    $this->log(sprintf(
+                        'Skipped auth fields (%s) on user %s: caller lacks canEditAuthFields on this target.',
+                        implode(', ', $deniedAuthFields),
+                        $user->username,
+                    ));
+                }
             }
 
             if (! $this->validateFmcsLocation($this->item['location_id'] ?? null, $companyIds)) {

app/Models/User.php

@@ -47,6 +47,22 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
     use Searchable;
     use UniqueUndeletedTrait;
 
+    /**
+     * Fields governed by the `canEditAuthFields` gate: credentials (username,
+     * email, password), the activation flag, and the permission blob. Any
+     * controller / importer / job that mass-assigns from user input must gate
+     * writes to these fields on `canEditAuthFields` against the target, and
+     * signal denial rather than silently dropping them. Add to this list to
+     * bring a new field under the same gate everywhere at once.
+     */
+    public const GATED_AUTH_FIELDS = [
+        'password',
+        'username',
+        'email',
+        'activated',
+        'permissions',
+    ];
+
     protected $hidden = [
         'password',
         'remember_token',

resources/lang/en-US/admin/users/message.php

@@ -13,6 +13,7 @@ return [
     'nothing_currently_assigned' => 'Nothing currently assigned.',
     'user_password_required' => 'The password is required.',
     'insufficient_permissions' => 'Insufficient Permissions.',
+    'auth_fields_denied' => 'You do not have permission to modify credential or activation fields on this user. Requested fields not updated: :fields',
     'user_deleted_warning' => 'This user has been deleted. You will have to restore this user to edit them or assign them new assets.',
     'ldap_not_configured' => 'LDAP integration has not been configured for this installation.',
     'password_resets_sent' => 'The selected users who are activated and have a valid email addresses have been sent a password reset link.',

tests/Feature/Users/Api/UpdateUserTest.php

@@ -237,6 +237,74 @@ class UpdateUserTest extends TestCase
         $this->assertSame(1, (int) $admin->fresh()->activated, 'Non-admin actor must not be able to deactivate an admin via API.');
     }
 
+    /**
+     * When a caller who cannot pass canEditAuthFields on the target sends any
+     * User::GATED_AUTH_FIELDS in the request payload, the API must return an
+     * error status naming the denied fields rather than silently dropping
+     * them and returning `success`. Prior behavior returned
+     * `{"status":"success", "messages":"User was successfully updated."}`
+     * even when the password / permissions / activated / username / email
+     * write in the payload never persisted, misrepresenting the actual
+     * outcome to API clients.
+     */
+    public function test_api_returns_error_when_auth_fields_requested_without_permission(): void
+    {
+        $editing_user = User::factory()->editUsers()->create();
+        $admin = User::factory()->admin()->create([
+            'username' => 'api_admin_authfield_target',
+            'email' => 'api-admin-authfield-target@example.test',
+            'first_name' => 'Original',
+            'last_name' => 'Name',
+        ]);
+
+        $originalPasswordHash = $admin->password;
+
+        $response = $this->actingAsForApi($editing_user)
+            ->patch(route('api.users.update', $admin), [
+                'first_name' => 'Tampered',
+                'password' => 'attempted-new-password',
+                'permissions' => ['licenses.keys' => '1'],
+            ])
+            ->assertOk();
+
+        $response->assertJson([
+            'status' => 'error',
+            'messages' => trans('admin/users/message.auth_fields_denied', ['fields' => 'password, permissions']),
+        ]);
+
+        $fresh = $admin->fresh();
+        $this->assertSame('Original', $fresh->first_name, 'Non-auth fields must not persist when the request is rejected for auth-field denial.');
+        $this->assertSame($originalPasswordHash, $fresh->password, 'Password must not change when the caller cannot canEditAuthFields on the target.');
+    }
+
+    /**
+     * The inverse contract: when the request carries no GATED_AUTH_FIELDS,
+     * the response stays `success` and non-auth fields persist as normal.
+     * Pins that the new loud-fail path is entered only when the payload
+     * actually asks for gated fields.
+     */
+    public function test_api_returns_success_when_no_auth_fields_are_requested(): void
+    {
+        $editing_user = User::factory()->editUsers()->create();
+        $admin = User::factory()->admin()->create([
+            'first_name' => 'Original',
+            'last_name' => 'Name',
+            'jobtitle' => 'Previous Title',
+        ]);
+
+        $this->actingAsForApi($editing_user)
+            ->patch(route('api.users.update', $admin), [
+                'first_name' => 'Updated',
+                'jobtitle' => 'New Title',
+            ])
+            ->assertOk()
+            ->assertJson(['status' => 'success']);
+
+        $fresh = $admin->fresh();
+        $this->assertSame('Updated', $fresh->first_name);
+        $this->assertSame('New Title', $fresh->jobtitle);
+    }
+
     public function test_api_users_can_be_deactivated_with_number()
     {
         $admin = User::factory()->editUsers()->create();