Unhandled Exception on CRUD::setAccessCondition()

[mariovillani]

trafficstars

class CompanyCrudController extends CrudController
{
    use \Backpack\CRUD\app\Http\Controllers\Operations\ListOperation;
    use \Backpack\CRUD\app\Http\Controllers\Operations\CreateOperation;
    use \Backpack\CRUD\app\Http\Controllers\Operations\UpdateOperation;
    use \Backpack\CRUD\app\Http\Controllers\Operations\DeleteOperation { destroy as traitDestroy;
    }
    use \Backpack\CRUD\app\Http\Controllers\Operations\ShowOperation;
    use \Backpack\ReviseOperation\ReviseOperation;

    protected $setupDetailsRowRoute = false;
    /**
     * Configure the CrudPanel object. Apply settings to all operations.
     * 
     * @return void
     */
    public function setup()
    {
        // Remove all permissions
        $this->crud->denyAccess(['list', 'create', 'update', 'delete', 'show', 'revise']);

        CRUD::setAccessCondition(['show', 'update'], function ($entry) {
            if(isset($entry)) {
                if (backpack_user()->hasRole('Reseller'))
                    return ($entry->created_by === backpack_user()->id) || ($entry->created_by_company === backpack_user()->id_company);
            }
            return false;
        });

If a reseller tries to open up a Company entity without having permissions, an exception is thrown and logged in laravel.log:

Accesso non autorizzato - non hai i permessi necessari per vedere questa pagina. {"userId":32,"exception":"[object] (Backpack\\CRUD\\app\\Exceptions\\AccessDeniedException(code: 0): Accesso non autorizzato - non hai i permessi necessari per vedere questa pagina. at /myapp/vendor/backpack/crud/src/app/Library/CrudPanel/Traits/Access.php:88)

Docs do not mention that exceptions are thrown by CRUD::setAccessCondition()(related section: https://backpackforlaravel.com/docs/6.x/crud-operations#handling-access-to-operations). I think it would be better to show how to correctly handle that. Thank you in advance!

[karandatwani92]

Hey @pxpm

An exception is being logged whenever the user doesn't have access. It is not only limited to setAccessCondition. Crud::denyAccess does the same.

I don't think that is intentional; even if it is, it should be a warning or info, not an error.

[pxpm]

Hey @pxpm

An exception is being logged whenever the user doesn't have access. It is not only limited to setAccessCondition. Crud::denyAccess does the same.

I don't think that is intentional; even if it is, it should be a warning or info, not an error.

It throws a http unauthorized exception with forbidden (403) status: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/403

Similar if you navigate to a non-existent page you get a 404.

If you think it's worth adding to the docs a description of what we talked here, go ahead and add a note in the docs about denyAccess throwing a 403 http error forbidden.

Cheers

[karandatwani92]

Hey @pxpm IDK if we are on the same page. I mean, this logs an error for each attempt, and we both assume it should not be saved in the log file:

[pxpm]

@karandatwani92 that depends on your log level settings I guess. In Backpack we are not doing Log::error() or something similar. Like I told you, you are trying to access a resource on server, say user/1/edit and you don't have access to that resource, we throw an http 403 exception. (abort(403)).

Maybe you are looking to don't report some exceptions? https://laravel.com/docs/11.x/errors#ignoring-exceptions-by-type

[munjaldevelopment]

Hey @pxpm IDK if we are on the same page. I mean, this logs an error for each attempt, and we both assume it should not be saved in the log file:

Hi @karandatwani92, This is functionality of "Backpack" I guess. In log once any page is 403 forbidden for our reference,,

Please check this error on one of our LIVE project.

[pxpm]

Hey @pxpm IDK if we are on the same page. I mean, this logs an error for each attempt, and we both assume it should not be saved in the log file:

Hi @karandatwani92, This is functionality of "Backpack" I guess. In log once any page is 403 forbidden for our reference,,

Please check this error on one of our LIVE project.

Hey @munjaldevelopment . We use abort(403). Cheers

[pxpm]

I got back here with a fresh perspective, and I think I got what you guys where trying to say.

I did some digging and I found that Laravel itself already excludes some status codes from being reported like 419 for eg. In that regard I think we can make our exception extend one that Laravel already don't report on, so it will not be logged like before.

The @munjaldevelopment screenshot was very helpful, if it had some description of where I should look for it would have helped more from the start, but it is what it is.

Thanks for the report and for sticking with us 🙏

I will tag a new version later today that will have https://github.com/Laravel-Backpack/CRUD/pull/5642 merged.

Cheers