feature/set-password-on-org-sign-up (#257)

* Updated README.md * Added password field to API docs * Added tests and logic for requiring user password * Hashed password removed from responses * Updated failing test
RoyalBoroughKingston · Nov 3, 2019 · c09fe36 · c09fe36
1 parent 45442c4
commit c09fe36
Show file tree

Hide file tree

Showing 15 changed files with 122 additions and 8 deletions.
diff --git a/README.md b/README.md
@@ -64,13 +64,19 @@ script:
 
 ## Running the tests
 
-To run the PHPUnit tests:
+To run all tests:
 
 ```bash
-./develop phpunit
+./develop composer test
 ```
 
-To run the code style tests:
+To run only the PHPUnit tests:
+
+```bash
+./develop composer test:unit
+```
+
+To run only the code style tests:
 
 ```bash
 # Run linter.

diff --git a/app/Docs/Operations/OrganisationSignUpForms/StoreOrganisationSignUpFormOperation.php b/app/Docs/Operations/OrganisationSignUpForms/StoreOrganisationSignUpFormOperation.php
@@ -9,6 +9,7 @@
 use GoldSpecDigital\ObjectOrientedOAS\Objects\MediaType;
 use GoldSpecDigital\ObjectOrientedOAS\Objects\Operation;
 use GoldSpecDigital\ObjectOrientedOAS\Objects\RequestBody;
+use GoldSpecDigital\ObjectOrientedOAS\Objects\Schema;
 
 class StoreOrganisationSignUpFormOperation extends Operation
 {
@@ -35,7 +36,28 @@ public static function create(string $objectId = null): BaseObject
                 )
             )
             ->responses(
-                UpdateRequestReceivedResponse::create(null, StoreOrganisationSignUpFormSchema::create())
+                UpdateRequestReceivedResponse::create(
+                    null,
+                    StoreOrganisationSignUpFormSchema::create()->properties(
+                        ...array_map(
+                            function (Schema $schema): Schema {
+                                if ($schema->objectId === 'user') {
+                                    return $schema->properties(
+                                        ...array_filter(
+                                            $schema->properties,
+                                            function (Schema $schema): bool {
+                                                return $schema->objectId !== 'password';
+                                            }
+                                        )
+                                    );
+                                }
+
+                                return $schema;
+                            },
+                            StoreOrganisationSignUpFormSchema::create()->properties
+                        )
+                    )
+                )
             );
     }
 }
diff --git a/app/Docs/Schemas/OrganisationSignUpForm/StoreOrganisationSignUpFormSchema.php b/app/Docs/Schemas/OrganisationSignUpForm/StoreOrganisationSignUpFormSchema.php
@@ -27,6 +27,7 @@ public static function create(string $objectId = null): BaseObject
             ->properties(
                 StoreUserSchema::create('user')
                     ->required(
+                        'password',
                         ...array_filter(
                             StoreUserSchema::create()->required,
                             function (string $required): bool {
@@ -35,6 +36,8 @@ function (string $required): bool {
                         )
                     )
                     ->properties(
+                        Schema::string('password')
+                            ->format(Schema::FORMAT_PASSWORD),
                         ...array_filter(
                             StoreUserSchema::create()->properties,
                             function (Schema $property): bool {

diff --git a/app/Http/Controllers/Core/V1/OrganisationSignUpFormController.php b/app/Http/Controllers/Core/V1/OrganisationSignUpFormController.php
@@ -37,6 +37,7 @@ public function store(StoreRequest $request)
                         'last_name' => $request->input('user.last_name'),
                         'email' => $request->input('user.email'),
                         'phone' => $request->input('user.phone'),
+                        'password' => bcrypt($request->input('user.password')),
                     ],
                     'organisation' => [
                         'slug' => $request->input('organisation.slug'),

diff --git a/app/Http/Requests/OrganisationSignUpForm/StoreRequest.php b/app/Http/Requests/OrganisationSignUpForm/StoreRequest.php
@@ -8,6 +8,7 @@
 use App\Rules\InOrder;
 use App\Rules\MarkdownMaxLength;
 use App\Rules\MarkdownMinLength;
+use App\Rules\Password;
 use App\Rules\Slug;
 use App\Rules\UkPhoneNumber;
 use App\Rules\UserEmailNotTaken;
@@ -40,6 +41,7 @@ public function rules()
             'user.last_name' => ['required', 'string', 'min:1', 'max:255'],
             'user.email' => ['required', 'email', 'max:255', new UserEmailNotTaken()],
             'user.phone' => ['required', 'string', 'min:1', 'max:255', new UkPhoneNumber()],
+            'user.password' => ['required', 'string', 'min:8', 'max:255', new Password()],
 
             'organisation' => ['required', 'array'],
             'organisation.slug' => [

diff --git a/app/Http/Resources/UpdateRequestResource.php b/app/Http/Resources/UpdateRequestResource.php
@@ -21,7 +21,7 @@ public function toArray($request)
             'actioning_user_id' => $this->actioning_user_id,
             'updateable_type' => $this->updateable_type,
             'updateable_id' => $this->updateable_id,
-            'data' => $this->data,
+            'data' => $this->getUpdateable()->getData($this->data),
             'created_at' => $this->created_at->format(CarbonImmutable::ISO8601),
             'updated_at' => $this->updated_at->format(CarbonImmutable::ISO8601),
             'approved_at' => optional($this->approved_at)->format(CarbonImmutable::ISO8601),

diff --git a/app/Http/Responses/UpdateRequestReceived.php b/app/Http/Responses/UpdateRequestReceived.php
@@ -41,7 +41,9 @@ public function toResponse($request)
         return response()->json([
             'message' => 'The update request has been received and needs to be reviewed',
             'id' => $this->updateRequest->id,
-            'data' => $this->updateRequest->data,
+            'data' => $this->updateRequest->getUpdateable()->getData(
+                $this->updateRequest->data
+            ),
         ], $this->code);
     }
 }
diff --git a/app/Models/Location.php b/app/Models/Location.php
@@ -106,6 +106,18 @@ public function applyUpdateRequest(UpdateRequest $updateRequest): UpdateRequest
         return $updateRequest;
     }
 
+    /**
+     * Custom logic for returning the data. Useful when wanting to transform
+     * or modify the data before returning it, e.g. removing passwords.
+     *
+     * @param array $data
+     * @return array
+     */
+    public function getData(array $data): array
+    {
+        return $data;
+    }
+
     /**
      * @return \App\Models\Location
      */

diff --git a/app/Models/Organisation.php b/app/Models/Organisation.php
@@ -68,6 +68,18 @@ public function applyUpdateRequest(UpdateRequest $updateRequest): UpdateRequest
         return $updateRequest;
     }
 
+    /**
+     * Custom logic for returning the data. Useful when wanting to transform
+     * or modify the data before returning it, e.g. removing passwords.
+     *
+     * @param array $data
+     * @return array
+     */
+    public function getData(array $data): array
+    {
+        return $data;
+    }
+
     /**
      * @return \App\Models\Organisation
      */

diff --git a/app/Models/Service.php b/app/Models/Service.php
@@ -321,6 +321,18 @@ public function applyUpdateRequest(UpdateRequest $updateRequest): UpdateRequest
         return $updateRequest;
     }
 
+    /**
+     * Custom logic for returning the data. Useful when wanting to transform
+     * or modify the data before returning it, e.g. removing passwords.
+     *
+     * @param array $data
+     * @return array
+     */
+    public function getData(array $data): array
+    {
+        return $data;
+    }
+
     /**
      * Ensures conditional fields are reset to expected values.
      *

diff --git a/app/Models/ServiceLocation.php b/app/Models/ServiceLocation.php
@@ -205,6 +205,18 @@ public function applyUpdateRequest(UpdateRequest $updateRequest): UpdateRequest
         return $updateRequest;
     }
 
+    /**
+     * Custom logic for returning the data. Useful when wanting to transform
+     * or modify the data before returning it, e.g. removing passwords.
+     *
+     * @param array $data
+     * @return array
+     */
+    public function getData(array $data): array
+    {
+        return $data;
+    }
+
     /**
      * @return \App\Models\ServiceLocation
      */

diff --git a/app/UpdateRequest/AppliesUpdateRequests.php b/app/UpdateRequest/AppliesUpdateRequests.php
@@ -22,4 +22,13 @@ public function validateUpdateRequest(UpdateRequest $updateRequest): Validator;
      * @return \App\Models\UpdateRequest
      */
     public function applyUpdateRequest(UpdateRequest $updateRequest): UpdateRequest;
+
+    /**
+     * Custom logic for returning the data. Useful when wanting to transform
+     * or modify the data before returning it, e.g. removing passwords.
+     *
+     * @param array $data
+     * @return array
+     */
+    public function getData(array $data): array;
 }
diff --git a/app/UpdateRequest/OrganisationSignUpForm.php b/app/UpdateRequest/OrganisationSignUpForm.php
@@ -8,9 +8,9 @@
 use App\Models\UpdateRequest;
 use App\Models\User;
 use Illuminate\Contracts\Validation\Validator;
+use Illuminate\Support\Arr;
 use Illuminate\Support\Facades\Date;
 use Illuminate\Support\Facades\Validator as ValidatorFacade;
-use Illuminate\Support\Str;
 
 class OrganisationSignUpForm implements AppliesUpdateRequests
 {
@@ -26,6 +26,9 @@ public function validateUpdateRequest(UpdateRequest $updateRequest): Validator
             ->merge($updateRequest->data)
             ->rules();
 
+        // Update rules for hashed password instead of raw.
+        $rules['user.password'] = ['required', 'string'];
+
         return ValidatorFacade::make($updateRequest->data, $rules);
     }
 
@@ -43,7 +46,7 @@ public function applyUpdateRequest(UpdateRequest $updateRequest): UpdateRequest
             'last_name' => $updateRequest->getFromData('user.last_name'),
             'email' => $updateRequest->getFromData('user.email'),
             'phone' => $updateRequest->getFromData('user.phone'),
-            'password' => bcrypt(Str::random()),
+            'password' => $updateRequest->getFromData('user.password'),
         ]);
 
         /** @var \App\Models\Organisation $organisation */
@@ -129,4 +132,18 @@ public function applyUpdateRequest(UpdateRequest $updateRequest): UpdateRequest
 
         return $updateRequest;
     }
+
+    /**
+     * Custom logic for returning the data. Useful when wanting to transform
+     * or modify the data before returning it, e.g. removing passwords.
+     *
+     * @param array $data
+     * @return array
+     */
+    public function getData(array $data): array
+    {
+        Arr::forget($data, ['user.password']);
+
+        return $data;
+    }
 }
diff --git a/tests/Feature/OrganisationSignUpFormTest.php b/tests/Feature/OrganisationSignUpFormTest.php
@@ -26,6 +26,7 @@ public function test_guest_can_create_one()
                 'last_name' => $this->faker->lastName,
                 'email' => $this->faker->safeEmail,
                 'phone' => random_uk_phone(),
+                'password' => 'P@55w0rd.',
             ],
             'organisation' => [
                 'slug' => 'test-org',
@@ -94,6 +95,7 @@ public function test_guest_can_create_one_with_single_form_of_contact()
                 'last_name' => $this->faker->lastName,
                 'email' => $this->faker->safeEmail,
                 'phone' => random_uk_phone(),
+                'password' => 'P@55w0rd.',
             ],
             'organisation' => [
                 'slug' => 'test-org',
@@ -239,6 +241,7 @@ public function test_audit_created_when_created()
                 'last_name' => $this->faker->lastName,
                 'email' => $this->faker->safeEmail,
                 'phone' => random_uk_phone(),
+                'password' => 'P@55w0rd.',
             ],
             'organisation' => [
                 'slug' => 'test-org',

diff --git a/tests/Feature/UpdateRequestsTest.php b/tests/Feature/UpdateRequestsTest.php
@@ -739,6 +739,7 @@ public function test_global_admin_can_approve_one_for_organisation_sign_up_form(
                     'last_name' => 'Doe',
                     'email' => 'john.doe@example.com',
                     'phone' => '07700000000',
+                    'password' => 'P@55w0rd.',
                 ],
                 'organisation' => [
                     'slug' => 'test-org',