trjhtr

I want to make my API unavailable to every client who doesn't have the token to access. This means the Android app will send a client as Android and token as token string in the header with keys client and token.

Now in middleware, I am checking it with my table fields to pass through authorization. If both match, then I will authorize and if don't then it will send a 403 response.

I am aware of Passport but it is not what I am looking for. In fact, consider it as a first layer of security and then use Passport as a second layer of security to authorize the API.

Is this code correct?

As I am not so familiar with Laravel - Middleware I just want to get some feedback from experts whether the code I have written is accurate and up to the standard. If not, I would appreciate your suggestion and help to make it better.

Middleware

namespace AppHttpMiddleware;

use AppApiToken;
use Closure;
use function response;

class ApiAccess

 /**
 * Handle an incoming request.
 *
 * @param IlluminateHttpRequest $request
 * @param Closure $next
 *
 * @return mixed
 */
 public function handle( $request, Closure $next ) 

 if ( $this->checkToken( $request ) ) 
 return $next( $request );
 

 return response()->json( [ 'error' => 'Unauthorized' ], 403 );


 

 public function checkToken( $request ) 

 $client = $request->header( 'client' );
 $token = $request->header( 'token' );

 $checkToken = ApiToken::where( 'client', $client )
 ->where( 'token', $token )->first();

 return $checkToken;
 

API Route

I am fetching results from the ApiToken table just to check:

Route::get('/', function(Request $request) 
 return ApiToken::all();
)->middleware('apiAccess');

Optimized with Muhammad Nauman's answer here

public function checkToken( $request ) 

 $client = $request->header( 'client' );
 $token = $request->header( 'token' );

 return ApiToken::where( 'client', $client )
 ->where( 'token', $token )->exists();
// Nicer, and it will return true and false based on the existence of the token and client.

My take on this is the following:

1. rename the middleware to something meaningful (I took example from VerifyCsrfToken middleware)


2. throw exception if token is mismatched


3. rename checkToken to verify, reader of the code knows that code checks/verifies token because thats the point of the middleware


4. simplify even more verify function and add docblock


5. optimize the query to the following
   
   

   
   

   select exists(select `id` from `tokens` where (`client` = 'Android' and `token` = 'OuK0ELzYkN3Ss9Zf')) as `exists`
   

   
   

   
   

So ApiAccess becomes following:

<?php

namespace AppHttpMiddleware;

use AppExceptionsTokenMismatchException;
use AppApiToken;
use Closure;

class VerifyApiToken


 /**
 * Handle an incoming request.
 *
 * @param IlluminateHttpRequest $request
 * @param Closure $next
 *
 * @return mixed
 * @throws AppExceptionsTokenMismatchException
 */
 public function handle(Request $request, Closure $next)
 
 if ($this->verify($request)) 
 return $next($request);
 

 throw new TokenMismatchException;
 


 /**
 * Verify token by querying database for existence of the client:token pair specified in headers.
 *
 * @param IlluminateHttpRequest $request
 *
 * @return bool
 */
 public function verify($request): bool //optional return types
 
 return ApiToken::select('id')->where([ // add select so Eloquent does not query for all fields
 'client' => $request->header('client'), // remove variable that is used only once
 'token' => $request->header('token'), // remove variable that is used only once
 ])->exists();
 

Create new Exception, php artisan make:exception TokenMismatchException - yes kind of same as Laravel's stock one used when CSRF token is mismatched.

With body:

<?php

namespace AppExceptions;

use Exception;

class TokenMismatchException extends Exception


 /**
 * Report the exception.
 *
 * @return void
 */
 public function report()
 
 //
 


 /**
 * Render the exception into an HTTP response.
 *
 * @param IlluminateHttpRequest $request
 *
 * @return IlluminateHttpResponse

Note: remove use function response; statements because helpers are auto-loaded from Laravel's helpers