Follow policy response status for resource authorization

[juliomotol]

• Changes have been thoroughly tested to not break existing functionality.
• New functionality has been documented or existing documentation has been updated to reflect changes.
• Visual changes are explained in the PR description using a screenshot/recording of before and after.

This PR aims to respect Laravel's Response::denyAsNotFound()usage within Policy classes.

Before, regardless of how we try to specify the status to 404, it still responds with a 403 as that is hardcoded to abort to.

[danharrin]

This is a breaking change and I don't really understand why we need to duplicate the gate code that was already in another method (can() ?). Can we not just change 403 to 404?

[juliomotol]

I don't really understand why we need to duplicate the gate code that was already in another method (can() ?)

The Resource::can() only uses Gate::check() which only returns bool. It doesn't throw an AuthorizationException (See Illuminate\Auth\Access\Response's authorize() method).

On the other hand Gate::authorize() do throw an AuthorizationException with the status code set via Response::denyAsNotFound() or Response::denyWithStatus() (See Tim MacDonald's explanation in laravel/framework#43097)

This is a breaking change

Should I introduce a new method rather than modifying the authorizeResourceAccess()?

[danharrin]

I think we should just change it to 404 all the time

[juliomotol]

Gotcha, I'll create a new PR later

[zepfietje]

I think we should just change it to 404 all the time

I don't agree. I think we should use the actual status code that's set on the auth exception from e.g. the policy.

[danharrin]

@zepfietje We can't stop using static::getResource()::canCreate() because it is a breaking change if people are overriding those methods with custom logic. I don't think it hurts to always return 404, its a good default to prevent record existence from being exposed. Also, I don't want 2 methods on a resource that access policies in slightly different ways, it feels wrong to have to override 2 methods if you want to replace policies with a different authorization mechanism.

[zepfietje]

I haven't looked into the specifics, but I think there's more use cases than just returning 404. E.g. when returning the Payment Required status, it's nice if the correct error page is rendered instead of 404. In my opinion, this is still something to be handled by the user in their policies. Though if breaking, move it to v3.

[danharrin]

Since this is a breaking change, we will have to deal with it in v3. I will do the work for that. Thanks @juliomotol!

[juliomotol]

Looking forward to see this in v3, thank you!