Merge f98f978502 into c9f55bfd94

app/Http/Requests/SaveUserRequest.php

@@ -7,6 +7,7 @@ use Illuminate\Contracts\Validation\Validator;
 use Illuminate\Foundation\Http\FormRequest;
 use Illuminate\Http\Exceptions\HttpResponseException;
 use App\Rules\UserCannotSwitchCompaniesIfItemsAssigned;
+use Illuminate\Support\Facades\Gate;
 
 class SaveUserRequest extends FormRequest
 {
@@ -17,7 +18,7 @@ class SaveUserRequest extends FormRequest
      */
     public function authorize()
     {
-        return true;
+        return (Gate::allows('users.create') || Gate::allows('users.edit'));
     }
 
     public function response(array $errors)
@@ -35,7 +36,8 @@ class SaveUserRequest extends FormRequest
         $rules = [
             'department_id' => 'nullable|exists:departments,id',
             'manager_id' => 'nullable|exists:users,id',
-            'company_id' => ['nullable','exists:companies,id']
+            'company_id' => ['nullable','exists:companies,id'],
+            'groups' => ['nullable','exists:permission_groups,id']
         ];
 
         switch ($this->method()) {

app/Policies/SnipePermissionsPolicy.php

@@ -23,7 +23,7 @@ use Illuminate\Auth\Access\HandlesAuthorization;
 abstract class SnipePermissionsPolicy
 {
     /**
-     * This should return the key of the model in the users json permission string.
+     * This should return the key of the model in the user's JSON permission string.
      *
      * @return bool
      */
@@ -37,11 +37,7 @@ abstract class SnipePermissionsPolicy
     {
         /**
          * If an admin, they can do all item related tasks, but ARE constrained by FMCSA company access.
-         * That scoping happens on the model level (except for the Users model) via the Companyable trait.
+         * That scoping happens on the model level via the Companyable trait.
-         *
-         * This does lead to some inconsistencies in the responses, since attempting to edit assets,
-         * accessories, etc (anything other than users) will result in a Forbidden error, whereas the users
-         * area will redirect with "That user doesn't exist" since the scoping is handled directly on those queries.
          *
          * The *superuser* global permission gets handled in the AuthServiceProvider before() method.
          *
@@ -53,7 +49,7 @@ abstract class SnipePermissionsPolicy
         }
 
         /**
-         * If we got here by $this→authorize('something', $actualModel) then we can continue on Il but if we got here
+         * If we got here by $this→authorize('something', $actualModel) then we can continue on, but if we got here
          * via $this→authorize('something', Model::class) then calling Company:: isCurrentUserHasAccess($item) gets weird.
          * Bail out here by returning "nothing" and allow the relevant method lower in this class to be called and handle authorization.
          */
@@ -85,7 +81,7 @@ abstract class SnipePermissionsPolicy
     }
 
     /**
-     * Determine whether the user can view the accessory.
+     * Determine whether the user can view the item.
      *
      * @param  \App\Models\User  $user
      * @return mixed
@@ -112,7 +108,7 @@ abstract class SnipePermissionsPolicy
     }
 
     /**
-     * Determine whether the user can update the accessory.
+     * Determine whether the user can update the item.
      *
      * @param  \App\Models\User  $user
      * @return mixed
@@ -124,7 +120,7 @@ abstract class SnipePermissionsPolicy
 
 
     /**
-     * Determine whether the user can update the accessory.
+     * Determine whether the user can checkout the item.
      *
      * @param  \App\Models\User  $user
      * @return mixed
@@ -135,7 +131,7 @@ abstract class SnipePermissionsPolicy
     }
 
     /**
-     * Determine whether the user can delete the accessory.
+     * Determine whether the user can delete the item.
      *
      * @param  \App\Models\User  $user
      * @return mixed
@@ -151,7 +147,7 @@ abstract class SnipePermissionsPolicy
     }
 
     /**
-     * Determine whether the user can manage the accessory.
+     * Determine whether the user can manage the item.
      *
      * @param  \App\Models\User  $user
      * @return mixed

tests/Feature/Users/Api/StoreUserTest.php

@@ -0,0 +1,91 @@
+<?php
+
+namespace Tests\Feature\Users\Api;
+
+use App\Models\Company;
+use App\Models\Department;
+use App\Models\Group;
+use App\Models\Location;
+use App\Models\User;
+use Tests\TestCase;
+
+class StoreUserTest extends TestCase {
+    public function testRequiresPermission()
+    {
+
+        $request = $this->actingAsForApi(User::factory()->create())
+            ->postJson(route('api.users.store'))
+            ->assertForbidden()
+            ->json();
+    }
+
+    public function testCanSaveUserViaPost()
+    {
+        $admin = User::factory()->superuser()->create();
+        $manager = User::factory()->create();
+        $company = Company::factory()->create();
+        $department = Department::factory()->create();
+        $location = Location::factory()->create();
+        $group = Group::factory()->create();
+
+
+        $response = $this->actingAsForApi($admin)
+            ->postJson(route('api.users.store'), [
+                'first_name' => 'Mabel',
+                'last_name' => 'Mora',
+                'username' => 'mabel',
+                'password' => 'super-secret',
+                'password_confirmation' => 'super-secret',
+                'email' => 'mabel@example.com',
+                'permissions' => '{"a.new.permission":"1"}',
+                'activated' => true,
+                'phone' => '619-555-5555',
+                'jobtitle' => 'Host',
+                'manager_id' => $manager->id,
+                'employee_num' => '1111',
+                'notes' => 'Pretty good artist',
+                'company_id' => $company->id,
+                'department_id' => $department->id,
+                'location_id' => $location->id,
+                'remote' => true,
+                'groups' => $group->id,
+                'vip' => true,
+                'start_date' => '2021-08-01',
+                'end_date' => '2025-12-31',
+            ])
+            ->assertOk()
+            ->assertStatus(200)
+            ->assertStatusMessageIs('success');
+
+        $user = User::find($response['payload']['id']);
+        $this->assertEquals($admin->id, $user->created_by, 'Created by was not saved');
+    }
+
+    public function testDoesNotAcceptBogusGroupData()
+    {
+        $admin = User::factory()->superuser()->create();
+
+        $this->actingAsForApi($admin)
+            ->postJson(route('api.users.store'), [
+                'first_name' => 'Mabel',
+                'username' => 'mabel',
+                'password' => 'super-secret',
+                'password_confirmation' => 'super-secret',
+                'groups' => ['blah'],
+            ])
+            ->assertOk()
+            ->assertStatus(200)
+            ->assertStatusMessageIs('error')
+            ->assertJson(
+                [
+                    'messages' =>  [
+                        'groups' =>
+                            [0 => trans('The selected groups is invalid.')]
+                        ]
+                ])
+            ->json();
+
+    }
+
+
+}