I'm using Passport and Laravel's Validator.

Here is my Login Controller:

public function login (Request $request) { $validator = Validator::make($request->all(), [ 'username' => 'required|string|max:255', 'password' => 'required|string|min:6', ]); if ($validator->fails()) { return response(['errors'=>$validator->errors()->all()], 422); } $user = User::where('username', $request->username)->first(); if ($user) { if (Hash::check($request->password, $user->password)) { $token = $user->createToken('Laravel Password Grant Client')->accessToken; $response = ['token' => $token]; return response($response, 200); } else { $response = ["message" => "Password mismatch"]; return response($response, 422); } } else { $response = ["message" =>'User does not exist']; return response($response, 422); } } 

Here is my routes/api.php:

use App\Http\Controllers\Auth\ApiAuthController; use Illuminate\Http\Request; use Illuminate\Support\Facades\Route; Route::group(['middleware' => ['cors', 'json.response']], function () { Route::post('/login', [ApiAuthController::class, 'login']); Route::post('/register', [ApiAuthController::class, 'register']); Route::post('/logout', [ApiAuthController::class, 'logout']); }); 

Here is the request:

POST Accept: application/json Content-Type: application/json { "username": "sundowatch", "password": "12345678", } 

But it returns this error:

HTTP/1.1 422 Unprocessable Entity Host: 127.0.0.1:8080 Date: Sat, 24 Apr 2021 17:35:31 GMT, Sat, 24 Apr 2021 17:35:31 GMT Connection: close X-Powered-By: PHP/7.3.26 Cache-Control: no-cache, private Content-Type: application/json Access-Control-Allow-Origin: * Access-Control-Allow-Methods: GET, POST, PUT, DELETE, OPTIONS Access-Control-Allow-Headers: X-Requested-With, Content-Type, X-Token-Auth, Authorization X-RateLimit-Limit: 60 X-RateLimit-Remaining: 57 { "errors": [ "The username field is required.", "The password field is required." ] } 
4

1 Answer

I will share with you some small tips for a clearer code, so don't take this as a bad critic, just a constructive one.

Use Illuminate\Support\Facades\Validator instead of Illuminate\Validation\Validator, you will be able to use Validator::validate() instead of Validator::make() and then $validator->fails(), doing so already removes at least 3 lines of codes and it is clearer.

use Illuminate\Support\Facades\Validator; Validator::validate($request->all(), [ 'username' => 'required|string|max:255', 'password' => 'required|string|min:6', ]); 

This will throw an error (ValidationException) with all the data as you did (errors => all errors), and it will return 422. So you can completely remove if.

Validator::validate($request->all(), [ 'username' => 'required|string|max:255', 'password' => 'required|string|min:6', ]); $user = User::where('username', $request->username)->first(); if ($user) { if (Hash::check($request->password, $user->password)) { $token = $user->createToken('Laravel Password Grant Client')->accessToken; $response = ['token' => $token]; return response($response, 200); } else { $response = ["message" => "Password mismatch"]; return response($response, 422); } } else { $response = ["message" =>'User does not exist']; return response($response, 422); } 

You can still do $validated = Validator::validate(...);, and if it fails it will return with errors, but if it passes, it will ONLY store validated fields (username and password in this case, if user sent is_admin = 1, it will be ignored, so you will have $validated['username'] and $validated['password']) so this prevents us from messing up using other non-validated inputs, let's say you forget to validate an input and you do $request->is_admin, so if you only use $validated you will for sure only used fields that are present in the rules part. For the moment, I will ignore this and continue using $request.

Then we can rearrange the $user and the next if. You can use firstOrFail() instead of first(), doing so, you can then remove the else and just have a code without if-else. It would look like:

Validator::validate($request->all(), [ 'username' => 'required|string|max:255', 'password' => 'required|string|min:6', ]); $user = User::where('username', $request->input('username'))->firstOrFail(); if (Hash::check($request->password, $user->password)) { $token = $user->createToken('Laravel Password Grant Client')->accessToken; $response = ['token' => $token]; return response($response, 200); } else { $response = ['message' => 'Password mismatch']; return response($response, 422); } 

Then, you can also change the last if-else. I recommend you to read about If guard clauses here and here.

So, you can have it like this:

Validator::validate($request->all(), [ 'username' => 'required|string|max:255', 'password' => 'required|string|min:6', ]); $user = User::where('username', $request->input('username'))->firstOrFail(); if (!Hash::check($request->input('password'), $user->password)) { return response(['message' => 'Password mismatch'], 422); } $token = $user->createToken('Laravel Password Grant Client')->accessToken; return response(compact('token')); 

See that:

  1. I have changed the if logic, thanks to guard clause we are inverting the clause, so we are only going to exit if what happened/have is not desired.
  2. I have replaced $response directly with the value, I do not like creating values and then having them used in the next line when it will return that. (This is super personal, still see how we have reduced code a lot by doing so and it is still readable and understandable at first glance)
  3. I have changed $request->username and $request->password with ->input('username') and ->input('password'). You can see source code for both and it is nearly the same, I just prefer to use input as I will understand that is 100% laravel, but if you do ->username it could be something you have macroed or similar. Again, this is personal.
  4. See that, for the last line, I have:
    • Removed 200 as it is the default value and will always be for response, no need to write it again, we know it is the default as it is a good response, no errors.
    • Replaced, again, ['token => $token] but in this case with compact. This is again a personal choice, but using both ways is the same. I just wanted to change it so everyone that is not aware of compact, could learn it now and maybe use it in the future.

So, compare your original code with the new one, same functionality but smaller and more readable:

$validator = Validator::make($request->all(), [ 'username' => 'required|string|max:255', 'password' => 'required|string|min:6', ]); if ($validator->fails()) { return response(['errors'=>$validator->errors()->all()], 422); } $user = User::where('username', $request->username)->first(); if ($user) { if (Hash::check($request->password, $user->password)) { $token = $user->createToken('Laravel Password Grant Client')->accessToken; $response = ['token' => $token]; return response($response, 200); } else { $response = ["message" => "Password mismatch"]; return response($response, 422); } } else { $response = ["message" =>'User does not exist']; return response($response, 422); } 
Validator::validate($request->all(), [ 'username' => 'required|string|max:255', 'password' => 'required|string|min:6', ]); $user = User::where('username', $request->input('username'))->firstOrFail(); if (!Hash::check($request->input('password'), $user->password)) { return response(['message' => 'Password mismatch'], 422); } $token = $user->createToken('Laravel Password Grant Client')->accessToken; return response(compact('token')); 

Your Answer

Sign up or log in

Sign up using Google Sign up using Facebook Sign up using Email and Password

Post as a guest

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.