From 059ec24a0e3150c393bd79285732253432fe626b Mon Sep 17 00:00:00 2001 From: Mohammad Alavi Date: Wed, 20 Apr 2022 17:17:37 +0430 Subject: [PATCH] fix(refresh_token): given `refresh_token` not provided, now throws proper validation errors instead of 500 --- .../ApiRefreshProxyForWebClientAction.php | 12 ++--- ...n.php => RefreshTokenMissingException.php} | 2 +- .../RefreshProxyForWebClientController.php | 4 +- .../UI/API/Requests/RefreshProxyRequest.php | 2 +- .../ApiRefreshProxyForWebClientTest.php | 54 +++++++++++++------ 5 files changed, 49 insertions(+), 25 deletions(-) rename app/Containers/AppSection/Authentication/Exceptions/{RefreshTokenMissedException.php => RefreshTokenMissingException.php} (85%) diff --git a/app/Containers/AppSection/Authentication/Actions/ApiRefreshProxyForWebClientAction.php b/app/Containers/AppSection/Authentication/Actions/ApiRefreshProxyForWebClientAction.php index faecaccfe..3b810e8eb 100644 --- a/app/Containers/AppSection/Authentication/Actions/ApiRefreshProxyForWebClientAction.php +++ b/app/Containers/AppSection/Authentication/Actions/ApiRefreshProxyForWebClientAction.php @@ -4,7 +4,7 @@ use Apiato\Core\Exceptions\IncorrectIdException; use App\Containers\AppSection\Authentication\Exceptions\LoginFailedException; -use App\Containers\AppSection\Authentication\Exceptions\RefreshTokenMissedException; +use App\Containers\AppSection\Authentication\Exceptions\RefreshTokenMissingException; use App\Containers\AppSection\Authentication\Tasks\CallOAuthServerTask; use App\Containers\AppSection\Authentication\Tasks\MakeRefreshCookieTask; use App\Containers\AppSection\Authentication\UI\API\Requests\RefreshProxyRequest; @@ -17,7 +17,7 @@ class ApiRefreshProxyForWebClientAction extends Action * @param RefreshProxyRequest $request * @return array * @throws LoginFailedException - * @throws RefreshTokenMissedException + * @throws RefreshTokenMissingException * @throws IncorrectIdException */ public function run(RefreshProxyRequest $request): array @@ -26,16 +26,16 @@ public function run(RefreshProxyRequest $request): array 'refresh_token', ]); + if (!array_key_exists('refresh_token', $sanitizedData) && is_null(Request::cookie('refreshToken'))) { + throw new RefreshTokenMissingException(); + } + $sanitizedData['refresh_token'] = $sanitizedData['refresh_token'] ?: Request::cookie('refreshToken'); $sanitizedData['client_id'] = config('appSection-authentication.clients.web.id'); $sanitizedData['client_secret'] = config('appSection-authentication.clients.web.secret'); $sanitizedData['grant_type'] = 'refresh_token'; $sanitizedData['scope'] = ''; - if (!$sanitizedData['refresh_token']) { - throw new RefreshTokenMissedException(); - } - $responseContent = app(CallOAuthServerTask::class)->run($sanitizedData, $request->headers->get('accept-language')); $refreshCookie = app(MakeRefreshCookieTask::class)->run($responseContent['refresh_token']); diff --git a/app/Containers/AppSection/Authentication/Exceptions/RefreshTokenMissedException.php b/app/Containers/AppSection/Authentication/Exceptions/RefreshTokenMissingException.php similarity index 85% rename from app/Containers/AppSection/Authentication/Exceptions/RefreshTokenMissedException.php rename to app/Containers/AppSection/Authentication/Exceptions/RefreshTokenMissingException.php index cb5dbcae2..f85cce323 100644 --- a/app/Containers/AppSection/Authentication/Exceptions/RefreshTokenMissedException.php +++ b/app/Containers/AppSection/Authentication/Exceptions/RefreshTokenMissingException.php @@ -5,7 +5,7 @@ use App\Ship\Parents\Exceptions\Exception; use Symfony\Component\HttpFoundation\Response; -class RefreshTokenMissedException extends Exception +class RefreshTokenMissingException extends Exception { protected $code = Response::HTTP_BAD_REQUEST; protected $message = 'We could not find the Refresh Token. Maybe none is provided?'; diff --git a/app/Containers/AppSection/Authentication/UI/API/Controllers/RefreshProxyForWebClientController.php b/app/Containers/AppSection/Authentication/UI/API/Controllers/RefreshProxyForWebClientController.php index a2fd0ed7c..806967d7a 100644 --- a/app/Containers/AppSection/Authentication/UI/API/Controllers/RefreshProxyForWebClientController.php +++ b/app/Containers/AppSection/Authentication/UI/API/Controllers/RefreshProxyForWebClientController.php @@ -5,7 +5,7 @@ use Apiato\Core\Exceptions\IncorrectIdException; use App\Containers\AppSection\Authentication\Actions\ApiRefreshProxyForWebClientAction; use App\Containers\AppSection\Authentication\Exceptions\LoginFailedException; -use App\Containers\AppSection\Authentication\Exceptions\RefreshTokenMissedException; +use App\Containers\AppSection\Authentication\Exceptions\RefreshTokenMissingException; use App\Containers\AppSection\Authentication\UI\API\Requests\RefreshProxyRequest; use App\Ship\Parents\Controllers\ApiController; use Illuminate\Http\JsonResponse; @@ -23,7 +23,7 @@ class RefreshProxyForWebClientController extends ApiController * @param RefreshProxyRequest $request * @return JsonResponse * @throws LoginFailedException - * @throws RefreshTokenMissedException + * @throws RefreshTokenMissingException * @throws IncorrectIdException */ public function refreshProxyForWebClient(RefreshProxyRequest $request): JsonResponse diff --git a/app/Containers/AppSection/Authentication/UI/API/Requests/RefreshProxyRequest.php b/app/Containers/AppSection/Authentication/UI/API/Requests/RefreshProxyRequest.php index cc2581e18..80bc9d2e8 100644 --- a/app/Containers/AppSection/Authentication/UI/API/Requests/RefreshProxyRequest.php +++ b/app/Containers/AppSection/Authentication/UI/API/Requests/RefreshProxyRequest.php @@ -35,7 +35,7 @@ class RefreshProxyRequest extends Request public function rules(): array { return [ - + 'refresh_token' => 'string', ]; } diff --git a/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/ApiRefreshProxyForWebClientTest.php b/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/ApiRefreshProxyForWebClientTest.php index 3fd2310c8..6887eafc2 100644 --- a/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/ApiRefreshProxyForWebClientTest.php +++ b/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/ApiRefreshProxyForWebClientTest.php @@ -2,8 +2,9 @@ namespace App\Containers\AppSection\Authentication\UI\API\Tests\Functional; -use App\Containers\AppSection\Authentication\Exceptions\RefreshTokenMissedException; +use App\Containers\AppSection\Authentication\Exceptions\RefreshTokenMissingException; use App\Containers\AppSection\Authentication\UI\API\Tests\ApiTestCase; +use Illuminate\Testing\Fluent\AssertableJson; /** * Class ApiRefreshProxyForWebClientTest @@ -17,30 +18,37 @@ class ApiRefreshProxyForWebClientTest extends ApiTestCase private array $data; - protected function setUp(): void + public function testRequestingRefreshTokenWithoutPassingARefreshTokenShouldThrowAnException(): void { - parent::setUp(); + $data = []; - $this->data = [ - 'email' => 'testing@mail.com', - 'password' => 'testing_pass', - ]; + $response = $this->makeCall($data); - $this->getTestingUser($this->data); - $this->actingAs($this->testingUser, 'web'); + $response->assertStatus(400); + $message = (new RefreshTokenMissingException())->getMessage(); + $response->assertJson( + fn (AssertableJson $json) => $json->has('message') + ->where('message', $message) + ->etc() + ); } - public function testRequestingRefreshTokenWithoutPassingARefreshTokenShouldThrowAnException(): void + public function testGivenRefreshTokenPassedAsParameter_ItShouldBeString(): void { $data = [ - 'refresh_token' => null, + 'refresh_token' => '', // empty equals `not string` ]; $response = $this->makeCall($data); - $response->assertStatus(400); - $message = (new RefreshTokenMissedException())->getMessage(); - $this->assertResponseContainKeyValue(['message' => $message]); + $response->assertStatus(422); + $response->assertJson( + fn (AssertableJson $json) => $json->hasAll(['message', 'errors' => 1]) + ->has( + 'errors', + fn (AssertableJson $json) => $json->where('refresh_token.0', 'The refresh token must be a string.') + ) + ); } public function testOnSuccessfulRefreshTokenRequestEnsureValuesAreSetProperly(): void @@ -57,6 +65,22 @@ public function testOnSuccessfulRefreshTokenRequestEnsureValuesAreSetProperly(): $this->assertResponseContainKeyValue([ 'token_type' => 'Bearer', ]); - $this->assertResponseContainKeys(['expires_in', 'access_token']); + $response->assertJson( + fn (AssertableJson $json) => $json->hasAll(['expires_in', 'access_token']) + ->etc() + ); + } + + protected function setUp(): void + { + parent::setUp(); + + $this->data = [ + 'email' => 'testing@mail.com', + 'password' => 'testing_pass', + ]; + + $this->getTestingUser($this->data); + $this->actingAs($this->testingUser, 'web'); } }