Courses

Laravel API Code Review and Refactor

Handling AuthorizationException Globally

We continue shortening the OrderController, and now I want to remove the try-catch with AuthorizationException.

app/Http/Controllers/Api/V1/OrderController.php

public function show(Order $order)
{
try {
$this->isAble('view', $order); // policy
 
// ...
 
return new OrderResource($order);
} catch (AuthorizationException $eAuthorizationException) {
return $this->responseNotAuthorized();
}
}

There is no need to do this manually in every Controller method. That Exception can also be handled automatically by Laravel.


Controller Cleanup

So, we will remove this try-catch in THREE methods of this Controller.

BEFORE:

app/Http/Controllers/Api/V1/OrderController.php

class OrderController extends ApiController
{
// ...
 
public function show(Order $order)
{
try {
$this->isAble('view', $order); // policy
 
if ($this->include('user')) {
$order->load('user');
}
 
$order->load('products');
 
return new OrderResource($order);
} catch (AuthorizationException $eAuthorizationException) {
return $this->responseNotAuthorized();
}
}
 
public function update(UpdateOrderRequest $request, Order $order)
{
try {
$this->isAble('update', $order); // policy
$this->orderService->updateOrderHandleProducts($request, $order);
 
return response()->json(new OrderResource($order), Response::HTTP_OK);
} catch (AuthorizationException $eAuthorizationException) {
return $this->responseNotAuthorized();
} catch (QueryException $eQueryException) {
DB::rollback(); // Rollback transaction on database error
 
return $this->responseDbError();
} catch (Throwable $eTh) {
DB::rollback(); // Rollback transaction on any other error
 
return $this->responseUnexpectedError();
}
}
 
public function destroy(Order $order)
{
try {
$this->isAble('delete', $order); // policy
$this->orderService->deleteOrderHandleProducts($order);
 
return $this->responseSuccess('Order deleted successfully');
} catch (AuthorizationException $eAuthorizationException) {
return $this->responseNotAuthorized();
}
}
}

AFTER:

class OrderController extends ApiController
{
// ...
 
public function show(Order $order)
{
$this->isAble('view', $order); // policy
 
if ($this->include('user')) {
$order->load('user');
}
 
$order->load('products');
 
return new OrderResource($order);
}
 
public function update(UpdateOrderRequest $request, Order $order)
{
try {
$this->isAble('update', $order); // policy
$this->orderService->updateOrderHandleProducts($request, $order);
 
return response()->json(new OrderResource($order), Response::HTTP_OK);
} catch (QueryException $eQueryException) {
DB::rollback(); // Rollback transaction on database error
 
return $this->responseDbError();
} catch (Throwable $eTh) {
DB::rollback(); // Rollback transaction on any other error
 
return $this->responseUnexpectedError();
}
}
 
public function destroy(Order $order)
{
$this->isAble('delete', $order); // policy
$this->orderService->deleteOrderHandleProducts($order);
 
return $this->responseSuccess('Order deleted successfully');
}
}

It looks a bit shorter, doesn't it? Especially for show() and destroy(), where we remove all the try-catch structures.

Notice: For whatever reason, the store() method didn't have that Authorization check. We will get to that in the future lesson.


Trait Response: Fix the BUG with the Status Code

The next step is to return the same response as the responseNotAuthorized() method globally, catching the Exception in the bootstrap/app.php.

But there's a problem with what that method returns.

Here's the current code:

app/Traits/V1/ApiResponses.php:

public function responseNotAuthorized(string $message = 'You are not authorized.'): JsonResponse
{
return $this->responseError($message, Response::HTTP_UNAUTHORIZED);
}
 
protected function responseError($message, int $statusCode = 500): JsonResponse
{
return response()->json([
'errors' => $message,
'status' => $statusCode
], $statusCode);
}

We're talking about Authorization, and you would expect that Symfony constant Response::HTTP_UNAUTHORIZED returns the 403 HTTP Status Code?

Wrong. It returns 401.

Looking outside of Laravel or Symfony, these HTTP status codes mean different things:

  • 401 means the user is not logged-in (unauthenticated)
  • 403 means user is logged in but doesn't have permissions for this (unauthorized)

The correct Symfony constant for 403 Forbidden is this:

return $this->responseError($message, Response::HTTP_UNAUTHORIZED);
return $this->responseError($message, Response::HTTP_FORBIDDEN);

You can take a look at the complete list of Symfony HTTP status codes here on GitHub.

Important: here, we have a rare case when we do introduce a breaking change in the API. But in my opinion, it was a bug that was left untested. The codebase didn't have any tests for roles/permissions. We will introduce them at the end of this lesson.


Catch the Exception Globally

Now, similarly to what we did with the Route Model Binding a few lessons ago, we need to catch that AuthorizationException globally in the bootstrap/app.php.

bootstrap/app.php:

use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
 
// ...
 
->withExceptions(function (Exceptions $exceptions) {
// ...
 
$exceptions->renderable(function (AccessDeniedHttpException $e) {
return response()->json([
'errors' => 'You are not authorized.',
'status' => Response::HTTP_FORBIDDEN,
], Response::HTTP_FORBIDDEN);
});
})->create();

Similarly to the Route Model Binding with ModelNotFoundException, Laravel didn't catch its own Laravel Exception, I don't know why. But it did work with the underlying Symfony AccessDeniedHttpException, so I used that one.


Automated Tests for Deleting Orders

We will add a few new methods to the tests. First, the codebase didn't have the tests for deleting the orders. Let's add two cases: successful deletion and forbidden response.

tests/Feature/OrderCreateTest.php

public function test_delete_order_successfully()
{
$user = User::factory()->create();
 
$order = Order::factory()->create();
$order->update(['user_id' => $user->id]);
 
$response = $this
->actingAs($user)
->deleteJson("/api/v1/orders/{$order->id}");
 
$response->assertStatus(200);
 
$this->assertDatabaseCount('orders', 0);
}
 
public function test_delete_fails_for_order_by_other_user()
{
$user1 = User::factory()->create();
 
$order = Order::factory()->create();
$order->update(['user_id' => $user1->id]);
 
$user2 = User::factory()->create();
 
$response = $this
->actingAs($user2)
->deleteJson("/api/v1/orders/{$order->id}");
 
$response->assertStatus(403)
->assertJson([
'errors' => 'You are not authorized.'
]);
 
$this->assertDatabaseCount('orders', 1);
}

Automated Test for Showing Forbidden Orders

Let's also add a method to test if someone wants to look at someone else's order without permission.

tests/Feature/OrdersShowTest.php

public function test_returns_403_for_order_by_other_user()
{
$user1 = User::factory()->create();
$order = Order::factory()->create(['user_id' => $user1->id]);
 
$user2 = User::factory()->create();
$response = $this
->actingAs($user2)
->getJson('/api/v1/orders/' . $order->id);
 
$response->assertStatus(403)
->assertJson([
'errors' => 'You are not authorized.'
]);
}

We run the tests... success!

Notice that we are adding more test methods to our codebase each step of the way. It feels good.


Here's the GitHub commit for all the changes from this lesson.

Previous: Static String Letters to PHP Enum

No comments yet…

avatar
You can use Markdown