From 749fe0586d7e58cb7294161eb912d921ccc3fc95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Jaussoin?= Date: Mon, 9 Dec 2024 15:52:37 +0100 Subject: [PATCH] Fix FLEXIAPI-242 Add stricter validation for the AccountCreationToken Push Notification endpoint --- .gitlab-ci-files/deploy.yml | 14 +++++------ .gitlab-ci-files/package.yml | 6 ++--- .gitlab-ci-files/test.yml | 10 ++++---- CHANGELOG.md | 4 +--- flexiapi/app/AccountCreationToken.php | 1 + .../Api/Account/CreationTokenController.php | 9 +++++--- flexiapi/app/Rules/PnParam.php | 19 +++++++++++++++ flexiapi/app/Rules/PnPrid.php | 19 +++++++++++++++ flexiapi/app/Rules/PnProvider.php | 21 +++++++++++++++++ .../api/documentation_markdown.blade.php | 6 ++--- .../Feature/ApiAccountCreationTokenTest.php | 23 ++++++++++++++++++- 11 files changed, 107 insertions(+), 25 deletions(-) create mode 100644 flexiapi/app/Rules/PnParam.php create mode 100644 flexiapi/app/Rules/PnPrid.php create mode 100644 flexiapi/app/Rules/PnProvider.php diff --git a/.gitlab-ci-files/deploy.yml b/.gitlab-ci-files/deploy.yml index b6b277b..8be5fcd 100644 --- a/.gitlab-ci-files/deploy.yml +++ b/.gitlab-ci-files/deploy.yml @@ -14,13 +14,13 @@ rocky9-deploy: - rocky9-package - rocky9-test -debian11-deploy: - extends: .deploy - script: - - ./deploy_packages.sh debian bullseye - needs: - - debian11-package - - debian11-test +#debian11-deploy: +# extends: .deploy +# script: +# - ./deploy_packages.sh debian bullseye +# needs: +# - debian11-package +# - debian11-test debian12-deploy: extends: .deploy diff --git a/.gitlab-ci-files/package.yml b/.gitlab-ci-files/package.yml index 6e5cc77..a697b33 100644 --- a/.gitlab-ci-files/package.yml +++ b/.gitlab-ci-files/package.yml @@ -16,9 +16,9 @@ rocky9-package: script: - make rpm-el9 -debian11-package: - extends: .debian_package - image: gitlab.linphone.org:4567/bc/public/docker/debian11-php:$DEBIAN_11_IMAGE_VERSION +#debian11-package: +# extends: .debian_package +# image: gitlab.linphone.org:4567/bc/public/docker/debian11-php:$DEBIAN_11_IMAGE_VERSION debian12-package: extends: .debian_package diff --git a/.gitlab-ci-files/test.yml b/.gitlab-ci-files/test.yml index eb8a3e4..09b7730 100644 --- a/.gitlab-ci-files/test.yml +++ b/.gitlab-ci-files/test.yml @@ -21,11 +21,11 @@ rocky9-test: - php artisan key:generate - vendor/bin/phpunit --log-junit $CI_PROJECT_DIR/flexiapi_phpunit.log -debian11-test: - extends: .debian-test - image: gitlab.linphone.org:4567/bc/public/docker/debian11-php:$DEBIAN_11_IMAGE_VERSION - needs: - - debian11-package +#debian11-test: +# extends: .debian-test +# image: gitlab.linphone.org:4567/bc/public/docker/debian11-php:$DEBIAN_11_IMAGE_VERSION +# needs: +# - debian11-package debian12-test: extends: .debian-test diff --git a/CHANGELOG.md b/CHANGELOG.md index fdadb45..daa973c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,5 @@ # Flexisip Account Manager Changelog -v1.6 (master) -------------- - v1.5 --- - Fix FLEXIAPI-202 Add account parameter to the redirection in the destroy admin route @@ -66,6 +63,7 @@ v1.5 - Fix #133 Make the MySQL connection unstrict - Fix #132 Move the provisioning_tokens and recovery_codes to dedicated table - Fix #130 Drop the group column in the Accounts table +- Fix FLEXIAPI-242 Add stricter validation for the AccountCreationToken Push Notification endpoint v1.4.9 ------ diff --git a/flexiapi/app/AccountCreationToken.php b/flexiapi/app/AccountCreationToken.php index 1105830..1ed48c7 100644 --- a/flexiapi/app/AccountCreationToken.php +++ b/flexiapi/app/AccountCreationToken.php @@ -19,6 +19,7 @@ namespace App; +use Illuminate\Validation\Rule; use Illuminate\Database\Eloquent\Factories\HasFactory; class AccountCreationToken extends Consommable diff --git a/flexiapi/app/Http/Controllers/Api/Account/CreationTokenController.php b/flexiapi/app/Http/Controllers/Api/Account/CreationTokenController.php index 57db8cf..0e6e00a 100644 --- a/flexiapi/app/Http/Controllers/Api/Account/CreationTokenController.php +++ b/flexiapi/app/Http/Controllers/Api/Account/CreationTokenController.php @@ -26,6 +26,9 @@ use Carbon\Carbon; use App\AccountCreationToken; use App\AccountCreationRequestToken; +use App\Rules\PnParam; +use App\Rules\PnPrid; +use App\Rules\PnProvider; use App\Http\Controllers\Controller; use App\Http\Controllers\Account\AuthenticateController as WebAuthenticateController; use App\Libraries\FlexisipPusherConnector; @@ -36,9 +39,9 @@ class CreationTokenController extends Controller public function sendByPush(Request $request) { $request->validate([ - 'pn_provider' => 'required', - 'pn_param' => 'required', - 'pn_prid' => 'required', + 'pn_provider' => ['required', new PnProvider], + 'pn_param' => [new PnParam], + 'pn_prid' => [new PnPrid], ]); $last = AccountCreationToken::where('pn_provider', $request->get('pn_provider')) diff --git a/flexiapi/app/Rules/PnParam.php b/flexiapi/app/Rules/PnParam.php new file mode 100644 index 0000000..10a3539 --- /dev/null +++ b/flexiapi/app/Rules/PnParam.php @@ -0,0 +1,19 @@ +validate($value); + } + + public function message() + { + return 'The :attribute should be null or contain only alphanumeric and underscore characters'; + } +} diff --git a/flexiapi/app/Rules/PnPrid.php b/flexiapi/app/Rules/PnPrid.php new file mode 100644 index 0000000..b7fd8ba --- /dev/null +++ b/flexiapi/app/Rules/PnPrid.php @@ -0,0 +1,19 @@ +validate($value); + } + + public function message() + { + return 'The :attribute should be null or contain only alphanumeric, dashes and colon characters'; + } +} diff --git a/flexiapi/app/Rules/PnProvider.php b/flexiapi/app/Rules/PnProvider.php new file mode 100644 index 0000000..e88d05d --- /dev/null +++ b/flexiapi/app/Rules/PnProvider.php @@ -0,0 +1,21 @@ +values); + } + + public function message() + { + return 'The :attribute should be in ' . implode(', ', $this->values); + } +} diff --git a/flexiapi/resources/views/api/documentation_markdown.blade.php b/flexiapi/resources/views/api/documentation_markdown.blade.php index 5976892..9c58195 100644 --- a/flexiapi/resources/views/api/documentation_markdown.blade.php +++ b/flexiapi/resources/views/api/documentation_markdown.blade.php @@ -179,9 +179,9 @@ Return `503` if the token was not successfully sent. JSON parameters: -* `pn_provider` the push notification provider -* `pn_param` the push notification parameter -* `pn_prid` the push notification unique id +* `pn_provider` **required**, the push notification provider, must be in apns.dev, apns or fcm +* `pn_param` the push notification parameter, can be null or contain only alphanumeric and underscore characters +* `pn_prid` the push notification unique id, can be null or contain only alphanumeric, dashes and colon characters ### `POST /account_creation_tokens/using-account-creation-request-token` Public diff --git a/flexiapi/tests/Feature/ApiAccountCreationTokenTest.php b/flexiapi/tests/Feature/ApiAccountCreationTokenTest.php index 0d50360..984c7f1 100644 --- a/flexiapi/tests/Feature/ApiAccountCreationTokenTest.php +++ b/flexiapi/tests/Feature/ApiAccountCreationTokenTest.php @@ -35,12 +35,33 @@ class ApiAccountCreationTokenTest extends TestCase protected $adminRoute = '/api/account_creation_tokens'; protected $method = 'POST'; - protected $pnProvider = 'provider'; + protected $pnProvider = 'fcm'; protected $pnParam = 'param'; protected $pnPrid = 'id'; public function testCorrectParameters() { + $this->assertSame(AccountCreationToken::count(), 0); + $this->json($this->method, $this->tokenRoute, [ + 'pn_provider' => 'wrong', + 'pn_param' => $this->pnParam, + 'pn_prid' => $this->pnPrid, + ])->assertJsonValidationErrors(['pn_provider']); + + $this->assertSame(AccountCreationToken::count(), 0); + $this->json($this->method, $this->tokenRoute, [ + 'pn_provider' => $this->pnProvider, + 'pn_param' => '@wrong', + 'pn_prid' => $this->pnPrid, + ])->assertJsonValidationErrors(['pn_param']); + + $this->assertSame(AccountCreationToken::count(), 0); + $this->json($this->method, $this->tokenRoute, [ + 'pn_provider' => $this->pnProvider, + 'pn_param' => $this->pnParam, + 'pn_prid' => '@wrong', + ])->assertJsonValidationErrors(['pn_prid']); + $this->assertSame(AccountCreationToken::count(), 0); $this->json($this->method, $this->tokenRoute, [ 'pn_provider' => $this->pnProvider,