User Management API

Question

I got as task to build a simple Laravel API that should:

1 - Create User API
2 - Get Users API

So I run the artisan command to create routes/api.php. And in it I added:

Route::post('/users', [UserController::class, 'store']);
Route::get('/users', [UserController::class, 'get']);

Table "users" has to be created. However, since Laravel already ships a "create_user_table"-migration I decided to only create another migration containing aspects that match the requirements:

//add_role_and_active_to_users_table.php
Schema::table('users', function (Blueprint $table) {
    $table->enum('role',['user', 'administrator', 'manager'])->default('user');
    $table->boolean('active')->default(true);
    $table->timestamp('created_at')->default(DB::raw('current_timestamp'))->change();
});

Finally, I created the store method:

//app/Http/Controllers/UserController
$request->validate([
    'email' => 'required|email',
    'password' => 'required|min:8',
    'name' => 'required|min:3|max:50',
]);    
try{
    $user = new User;
    $user->name = $request->input('name');
    $user->email = $request->input('email');
    $user->password = $request->input('password');
    $user->save();
    ProcessAccountCreated::dispatch($user);
    return response()->json(
        ['id' => $user->id,'email' => $user->email,'name' => $user->name,'created_at' => $user->created_at]
        , 201);
}
catch(\Exception $e){
    return response()->json(['error' => $e->getMessage()], 500);
}

Also, an email about new users has to be sent to an administrator. And another to the user itself. I fixed it with the following:

//app/Jobs/ProcessAccountCreated
public function __construct(User $user){$this->user = $user;}    
public function handle(): void
{
    Mail::to($this->user->email)->send(new MailToUser($this->user));
    Mail::to('root')->send(new MailToAdmin($this->user));
}

//app/Mail/MailToAdmin (MailToUser is allmost the same)
public function content(): Content
{
    return new Content(view: 'emails.mail_to_admin',);
}

To the users, I had to take care of optional parameters (to filter and sort). And also needed this table:

//database/migrations/create_orders_table
Schema::create('orders', function (Blueprint $table) {
    $table->id();
    $table->foreignId('user_id')
        ->constrained('users')
        ->cascadeOnDelete();
    $table->timestamp('created_at')->useCurrent();
});    

Finally I could write the get method inside the controller:

$users_per_page = 10;
$search = $request->query('search') ? $request->query('search') : '';
$page = $request->query('page') ? $request->query('page') : 1;
$sortBy = $request->query('sortBy') ? $request->query('sortBy') : 'created_at';

$query = DB::table('users as u')
    ->select('u.id', 'u.email', 'u.name', 'u.role', 'u.created_at', DB::raw('COUNT(o.id) AS orders_count'))
    ->leftJoin('orders as o', 'u.id', '=', 'o.user_id')
    ->where('u.active', true);
if($search){
    $query = $query->whereRaw(" (name = '$search' OR email = '$search')");
}
$query = $query->groupBy('u.id', 'u.name', 'u.email', 'u.role', 'u.created_at');
$query = $query->orderBy($sortBy);
$from = ($page*$users_per_page)-$users_per_page;
$query = $query->skip($from)->take($users_per_page);
$result = $query->get();
$users = [];
foreach ($result as $user){
    $u = [
        'id' => $user->id
        ,'email' => $user->email
        ,'name' => $user->name
        ,'role' => $user->role
        ,'created_at' => $user->created_at
        ,'orders_count' => $user->orders_count
    ];
    $users[] = $u;
}

return response()->json(['page' => $page, 'users' => $users]);

Can anyone have a look at this and come up with improvements? Is there something important that I could improve? The code is obviously working, as far as I could see.

Answer 1

General review

There are numerous Laravel features which could be used to simplify the controller methods - e.g. the base User model (which ships with the framework and can be modified) could have a relationship defined which would utilize a new model for orders, then the count of orders could be added via the withCount() method.

Laravel's query builder offers a paginate() method, which can likely be used to simplify the query to get users based on the page.

The arrays pushed into $users could be simplified by defining an API resource subclass.

Security concern - Avoid SQL injection attacks

Like Bobby Tables can likely describe SQL injection attacks should be taken seriously. This line is concerning:

$query = $query->whereRaw(" (name = '$search' OR email = '$search')");

Try requesting the API route with a value for search containing a single quote - it will likely lead to a SQL error along the lines of Incorrect syntax near.

Laravel typically handles SQL injection attacks out of the box, and in order to group "or" conditions one can pass a closure to a call to the where() method, as described in the Or Where Clauses section of the Query builder documentation.

$query->where(fn(Builder $orQuery) => $orQuery->where('name', $searchTerm)
                                      ->orWhere('email', $searchTerm));