Laravel 5 Middleware "Owner"?

Try this

public function handle($request, Closure $next)
{
    $article = Article::find($request->parameter('id'));

    if ($article->user_id != Auth::user()->id)
    {
        // abort
    }

    return $next($request);
}

Thanks @luddinus !

@shako I realise you’ve already accepted an answer, but personally I think this should be in a form request and not middleware.

Middleware classes aren’t that flexible. For example, what happens if you need to verify the owner of a content type other than articles? Or if you need to change the request parameter?

In a form request, you could do something like this:

class EditArticleRequest extends FormRequest {

    public function authorize()
    {
        $article = Article::find($this->route()->parameter('id'));

        return $this->user()->id == $article->user_id;
    }

    public function rules()
    {
        // Validation rules
    }
}

If you’re using route–model binding, then the authorize() method could be simplified as thus:

public function authorize()
{
    $article = $this->route()->parameter('article');

    return $this->user()->id == $article->user_id;
}

@martinbean Using a middleware you can avoid the READ, imagine you have some "messages" which belongs to a user and the route is "messages/{message_id}"...

@luddinus I'm not sure I follow. Read of what? If you mean database, then I don't see how when you're doing a find query in your middleware example.

@martinbean You have "messages" in your application. A message belongs to an user so you can enter in "messages/{message_id}", but to ensure that only the owner of the message can view the message, you add that behaviour in the middleware.

@luddinus I would still add that logic in a form request class and not a middleware class. Middleware is for manipulating requests and responses, whereas form requests are more for authorizing and validation; checking whether a user can view a message falls under authorization.

You also save yourself a database query if you use route–model binding as previously suggested, as the model instance will already be a route parameter.

@martinbean Using Request, how can you avoid the read? FormRequests are for POST logic.

@martinbean can form requests be added to get views? I always thought they were used for checking POST data.

@clin407 A form request can be added to any controller action.

@martinbean Is it as simple as doing this?

public function show($id, SubmitForm $submitForm)
{

}

I just tried that now on show() (that form request is attached to create() and update() and when I accessed /records/1, it keeps redirect back to /records. My authorize() in my form request just returns true and my only rules are just title and body is required. I'm guessing it's checking to see if title and body is filled and since it doesn't exist on the page it tries to redirect with errors?

edit: found out it is, I just had to make it so rules arent being returned for the GET method by using this:

switch($this->method())
{

}

@clin407 I think you’re misunderstanding. You shouldn’t be re-using your form requests that you use for saving, for your show method, as like you say, the validation will kick in and redirect you back.

Instead, create an entirely new form request class for your show method, complete the authorize method but leave validation as an empty array (and thus no validation will be performed).

class ViewArticleRequest extends FormRequest
{
    public function authorize()
    {
        $article = $this->route()->parameter('article');

        return ! is_null($this->user()) && $this->user()->id == $article->author_id;
    }

    public function validate()
    {
        return [];
    }
}

1. In Middleware

class isAuthorized {
    public function handle($request, Closure $next)
    {
        if ($request->user()->isAdmin() || $request->user()->isAuthorized())
        {
            return $next($request);
        }
        throw new \Exception("Access Denied.");
    }
}

2.) In my User.php Class I add the following:

 public function isAdmin()
    {
        return $this->role->slug == 'admin';
    }

    public function isAuthorized()
    {
        $listing = Listing::find(Request::segment(3));

        return Auth::user()->id == $listing->user_id;
    }

3.)Finally in my Controller I place the following:

 public function __construct()
    {
        $this->middleware('authorize', ['only' => ['edit'], ['delete']]);
    }

Works for me.. but open to better solutions

Works for me.. but open to better solutions

Instead of the request segment, get the route parameter. That way, the route structure changing is less likely to break the middleware:

public function handle(Request $request, Closure $next)
    {
        $profileId = $request->route()->parameter('profile');
        if(!$this->auth->user()->hasAccessTo($profileId)) {
            $this->notifyError('You do not have access to edit this profile.');
            return redirect()->route('profiles.current');
        }

        return $next($request);
    }

Thanks ! This worked perfectly

What about validating against multiple models?

Example: A User can be associated with a Book and a Magazine.

How should a validation be implemented? Through a Middleware with a parameter of the current model and a switch case for the different types of model validations, or through a form request for each Model Controller (BooksController and MagazineController respectively) ?

Nice question that I asked my self recently and didnt quite get an answer on my own.

@dhristov A form request is made to validate a form not a model. Even if in most cases a form is made to add one particular model. So use a form request to validate input for the form allowing to add book and magazine.

Exactly my thought. So a Middleware it is then. With parameters in 5.1 they are really usable for this situation. Hope this helps someone else.

So a Middleware it is then

Whaaaaaat ? This is not what I said. Don't use middleware for validating stuff !

@pmall what do you suggest for this owner check before showing the item then?

@polarcubs I totally agree with @martinbean on this topic. Check his first post on this thread.

I see @pmall, so form request is recommended more for owner checking.etc before the show($id) method. Am I right to say that?

It does sound weird though, probably because of the word Form in FormRequest.

You can make something like this for multiple models:

Route:

Route::get('job/{id}', [
        'middleware' => 'authorized:admin|owner,jobs',
        'uses'       => 'JobsController@show',
        'as'         => 'jobs.view',
    ]);

Middleware:

class IsAuthorized
    {
        private $jobs;

        public function __construct(Guard $auth, DesignJobs $jobs)
        {
            $this->auth = $auth;
            $this->jobs = $jobs;
        }

        public function handle($request, Closure $next, $roles, $model)
        {
            if ($this->auth->user()->is($roles) || $this->$model->find($request->route()->parameter('id'))->user->id == $this->auth->user()->id) {
                return $next($request);
            } else {
                return response('Unauthorized.', 401);
            }

        }
    }

If you use route model binding you may not need to pass through the model as a parameter. Perhaps if you had an interface called Ownable and you do something like this:

public function handle($request, Closure $next, $roles)
{
    $isOwner = true;
    foreach ($request->route()->parameters() as $model) {
        if ($model instanceof Ownable && $model->getOwnerKey() != $this->auth->user()->getKey()) {
            $isOwner = false;
        }
    }

    if ($isOwner || $this->auth->user()->is($roles)) {
        return $next($request);
    }

    return response('Unauthorized.', 401);
}

It would probably need some refining but would make it a flexible middleware.

@Corez64 How that interface will sound like?

Possibly something like this:

interface Ownable
{
    /**
     * Get the value of the owner's foreign key.
     */
    public function getOwnerKey();
}

And something that would implement it might look something like this:

class Post extends Eloquent implements Ownable
{
    /**
     * Relationship with the author of this post.
    * @return \Illuminate\Database\Eloquent\Relations\BelongsTo
     */
    public function author()
    {
        return $this->belongsTo(User::class, 'author_id');
    }

    /**
     * Get the value of the owner's foreign key.
     * @return int
     */
    public function getOwnerKey()
    {
        return $this->attributes['author_id'];
    }
}

@Corez64 I've already tried this but dose not work for me, is not entering in the if statement, and is giving me that is not an instance of Ownable !

My route:

Route::get('jobs/{jobRequest}', [
        'middleware' =>'authorized:admin|owner',
        'uses' => 'JobsController@show',
        'as'   => 'jobs.view',
    ]);

My model binding:

 public function boot(Router $router)
    {
        parent::boot($router);
        $router->model('jobRequest', 'App\Models\DesignJobs');
    }

My model:

 class DesignJobs extends Model implements Ownable{
etc...
public function getOwnerKey(){
            return 2;
        }

}

is not entering in the if statement, and is giving me that is not an instance of Ownable !

Be careful with ```instanceOf```` you have to be sure the full namespaces of the things you compare are correct (be sure to import the full namespace of Ownable)

@pmall Already done that, the import is corect but the reality is :

Changes for tests:

public function handle($request, Closure $next)
        {
            //$isOwner = true;
            foreach ($request as $model) {
                if ($model instanceof Ownable) {
                    print_r("Is an instance!") ;
                } else {
                    print_r("Is not an instance!") ;
                }
            }
        }

The page output:

Is not an instance!
Is not an instance!
Is not an instance!
Is not an instance!
Is not an instance!
Is not an instance!
Is not an instance!

Any advice?

I was doing this completely from an idea that popped into my head as I read this thread so I have never tried running this code. I am thinking this problem might be to do with the foreach loop in the handler. Try changing the $request to $request->route()->parameters() and see if that does the trick. I have edited the original post to reflect this.