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." ] } 41 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:
- I have changed the
iflogic, thanks toguard clausewe are inverting the clause, so we are only going to exit if what happened/have is not desired. - I have replaced
$responsedirectly 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) - I have changed
$request->usernameand$request->passwordwith->input('username')and->input('password'). You can see source code for both and it is nearly the same, I just prefer to useinputas I will understand that is 100% laravel, but if you do->usernameit could be something you have macroed or similar. Again, this is personal. - See that, for the last line, I have:
- Removed
200as it is the default value and will always be forresponse, 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 withcompact. 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 ofcompact, could learn it now and maybe use it in the future.
- Removed
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'));