Search code examples
laravellaravel-4routesmodelsrelationships

Best way of protecting the show() method against users accessing other users messages


Ok, so I have a basic messaging system, I have a relationship set up so I can just call $user->messages to retrieve an array of the users inbox messages. I also have a very simple show method that currently only grabs the message with id passed to the show() function.

The question is what would be the best way to protect that messages/2 URL so a user couldn't just type any number in the URL and access another users messages.

Should I use a route filter and basically run another query to ensure the message id is accessible by the user or is there something I can do with the relationship, perhaps check the id against the messages array and if it exists then the user must have access?

public function up()
    {
        Schema::create('messages', function(Blueprint $table) {
            $table->increments('id');
            $table->mediumText('subject');
            $table->text('message');
            $table->boolean('draft');
            $table->integer('sender_id')->unsigned();
            $table->softDeletes();
            $table->timestamps();

            $table->foreign('sender_id')->references('id')->on('users')->onUpdate('cascade');
        });

        Schema::create('message_assets', function(Blueprint $table) {
            $table->increments('id');
            $table->integer('message_id')->unsigned();
            $table->string('filename', 255);
            $table->softDeletes();

            $table->foreign('message_id')->references('id')->on('messages')->onUpdate('cascade');
        });

        Schema::create('message_users', function(Blueprint $table) {
            $table->increments('id');
            $table->integer('message_id')->unsigned();
            $table->integer('user_id')->unsigned();
            $table->integer('read')->default(0);
            $table->string('folder', 255)->nullable();
            $table->softDeletes();

            $table->foreign('message_id')->references('id')->on('messages')->onUpdate('cascade');
            $table->foreign('user_id')->references('id')->on('users')->onUpdate('cascade');
        });
    }

Solution

  • in the simplest form, in your MessagesController in the show method, you can add an additional parameter to your query and get the record where message_id = the paramater from the url and the user_id on that message is the authenticated user's ID.... do something like

    $message = App\Message::where('id', '=', $id)
                          ->where('user_id', '=', Auth::user()->id)
                          ->first();
    

    if you are doing a more advanced design and have a MessageRepository, this logic could be extracted there so in your controller you do something like

    $this->repository->getById($id);
    

    and in the messages repository the getById() method would do something similar to the above code example using an eloquent model. this method allows the controller to be clean and the logic to be re-used elsewhere in the app if needed

    ADDED to work with the pivot table specified above. this will only work for a user that has the message in the inbox:

    DB::table('messages')
        ->join('message_users', 'messages.id', '=', 'message_users.message_id')
        ->where('message_users.message_id', $id)
        ->where('message_users.user_id', $userId)
        ->where('message_users.deleted_at', null)
        ->select('messages.*')
        ->first();