Policies always results in "This action is unauthorized"

Well I register methods manually so not sure. But I noticed a typo in app\Policies\SpotPolicy.php

use App\Spot should have ;

In delete method of your spotController class you should typehint Spot next to the $spot parameter:

public function delete($spot) { // invalid
public function delete(Spot $spot) { // valid

Thans @Cruorzy ! Corrected that.

@crnkovic : for some reason this returns "Sorry, the page you are looking for could not be found."

I have this in my routes/web.php

Route::get('spots/api/{spot}/delete', 'spotController@delete');

And I'm passing

https://[domain]/spots/api/[id]/delete

If I do what you suggest the "delete" method in spotController doesn't even get reached (dd()) is not triggered)

<?php

namespace App\Http\Controllers;

use Illuminate\Http\Request;
use App\Spot;

use App\Policies\SpotPolicy;

class spotController extends Controller
{
    public function delete(Spot $spot)
    {
        dd('Stop here');
        $this->authorize('delete', $spot);

        return 'Spot gets deleted';
    }

}
,,,

Did you specify different route key in your model, other than ID? This seems very strange, make sure you have no syntax errors.

Thanks for your support @crnkovic.

I'm sorry but I don't know what you mean. What is a "route key in your model"?

This is the Spot model

<?php

namespace App;

use Illuminate\Database\Eloquent\Model;

class Spot extends Model
{

    protected $fillable = [
        'map_id',
        'place_id',
        'user_id',
        'info'
    ];

    protected $hidden = [
        'id',
    ];

    public function user() {
        return $this->belongsTo('App\User');
    }

    public function map() {
        return $this->belongsTo('App\Map');
    }
    
    public function place() {
        return $this->belongsTo('App\Place');
    }

}

I found what's causing the error. I was sending the item ID as a hashed id because I'm using Laravel component called "Hashids" from Vinkla. so the URL I was sending looked like

https://[domain]/spots/api/Zmr6kGzDE0xN9YeWgAXgApdvlBVyQPo5wj3nb2J4/delete

Surpassing the Hashid's component and sending...

https://[domain]/spots/api/1/delete

...makes my code reach the "delete" method in "SpotPolicy".

So the code works. I just need to make it work with hashed ids.

I just don't understand why the URL with the hashed ID doesn't even reach the delete() method in the spotController.

Ah, that's why I didn't do this...

public function delete(Spot $spot)

but

public function delete($spot)

I had to update my previous comment because I'm back to square one.

With the transformed hashid I'm still getting ""This action is unauthorized".

Even when I try enter the spot's id directly as an option for authorize() like this...

$this->authorize('delete', 1);

Shouldn't that option be an ID number? Apparently not?

Yeah I figured this was the issue. Okay, so look up "route model binding" on docs. Let me give you an introduction: you know, an ID is a primary key and default key that Laravel uses to search data (::find($id))and when you bind route parameter {model} in a route declaration, then resolve it in your controller as function method(Model $model), it's trying to resolve the model by name (it's automatically going to search in database and return an instance of the model). So whatever is pass as a route parameter, it's doing essentially: ->where('id', $routeParam). Let's say you have a post and a post has a slug. You can override the getRouteKeyName in Post model and simply return the column name (return 'slug';). Now, when you bind a model in your route (your example: {spot}, it's going to search by slug and not ID: ->where('slug', $routeParam). This is what has caused an issue.

Here's more: https://laravel.com/docs/5.6/routing#route-model-binding

Update: when you call $this->authorize('delete', $something), something should be an instance of the model and not an ID.

Thanks for your time @crnkovic.

While waiting for a reply I figured out that the options should be an instance of the model, not an ID number. As you pointed out in your comment.

I updated the controller to this and now it works.

<?php

namespace App\Http\Controllers;

use Illuminate\Http\Request;
use App\Spot;
use Vinkla\Hashids\Facades\Hashids;

use App\Policies\SpotPolicy;

class spotController extends Controller
{
    public function delete($spot)
    {
        $id = Hashids::connection('spots')->decode($spot)[0];
        
        $this->authorize('delete', Spot::find($id));

        return 'Spot gets deleted';
    }

}

Is this solution acceptable you think?

I have never used Hashids but this seems fine. If you're going to use this in many places, you can perhaps figure out a better solution than always trying to do Hashids::connection()..... Maybe something like Spot::byHashid($hashid) that returns an instance of model. :)

Now, let me give you a slighly better answer:

$this->authorize() has no idea on which policy should load. Until you tell it to. So, when you call $this->authorize('delete', $var), it doesn't know which policy to load delete method from. So it sees the type of $var, which in your case is App\Spot, sees that App\Spot maps to SpotPolicy and it now knows to load delete from SpotPolicy class. :)

Update: I'd personally create a custom route model binding called something like spotHash that would perform this logic, so you could simply typehint model and it would be always resolved: function delete(Spot $spot) {. (If the Hashids don't already provide a route model binding out of the box)

AppServiceProvider@boot:

Route::bind('spotHash', function ($value) {
    return App\Spot::findOrFail(Hashids::connection('spots')->decode($value)[0]);
});

Routes:

Route::get('/something/{spotHash}`)

I read somewhere that one should not pass the items ID to the client for security reasons. That's why I started using this Hash ID component. I wouldn't know how to do this "Spot::byHashid($hashid)" but that's outside of the scope of this thread, I'll look it up. Thanks for the suggestion.

Eventually I removed the method name from $this->authorize() because it is not needed in my case.

Thanks again for your time and sharing your knowledge. Didn't progress much today but I learned something new again ;-)

I usually don't expose ID to the client because I don't feel good knowing that my users know how many resources I have (if the user registers and has an ID of 1000, he will know there are 1000 registered users and can scrape them, /users/1, /users/1, ...). So I usually just create a model observer that listens to creating event and simply puts a unique ID on each model:

$model->uid = str_random(10);

Then I just set this as a route key and that works amazingly for me! :) I'm glad you learned something new!

As a matter of byHashid function, it can simply be a static method that wraps around logic you created:

public static function byHashid($hashid)
{
    return static::find(
        Hashids::connection('spots')->decode($spot)[0]
    );
}

Just keep learning! If you're interested in how Laravel works under the hood and if I'm allowed, I'd throw in a simple plug here: https://crnkovic.me/laravel-behind-the-scenes-lifecycle-container/ I go through each file loaded through the request cycle, and how from request you make you get back a response.

Estuve mucho tiempo intentando que funcionara,siempre me sacaba el mismo mensaje,observe decenas de foros y no me funcionaba, Finalmente observe que la politica esta en el servicio de autenticación,es decir que el usuario debe estar autenticado,añadí el middleware de auth y listo.