Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: ResourceQuery::authorizeResource must return bool but Authorizer::index returns Response #299

Closed
paul-thebaud opened this issue Nov 28, 2024 · 5 comments

Comments

@paul-thebaud
Copy link

Description

There is currently a bug with ResourceQuery::authorizeResource(), which is strict typed to return a bool value.
But, if we check the Authorizer::index() method (and other methods like it), it can return bool or Response.

Since Laravel default gate always returns a Response object, this results in a 500 error.

Env

  • Laravel version: v11.34.2
  • laravel-json-api/core version: v4.3.0

Notes

This is due to the updated Authorizer implementation in the core package, which now uses inspect instead of check. IMO, ResourceQuery and any other code which uses Authorizer methods should be updated in consequence.

@paul-thebaud
Copy link
Author

Another note: I cannot currently upgrade laravel-json-api/core to latest v4.3.0, because most of my API routes results in 500s. I'm locking this dependency to v4.2.0 for now as a workarround.

@lindyhopchris
Copy link
Contributor

Ugh yeah sorry, this is my fault for doing something in a rush the other day. I can't fix today but I can sort out on Friday afternoon UK time.

@paul-thebaud
Copy link
Author

Thanks for this quick answer! I'll look into when it is ready 🎉

@haddowg
Copy link

haddowg commented Nov 29, 2024

yeah core change was technically breaking without #298 being merged at the same time 😬

@lindyhopchris
Copy link
Contributor

So the mistake here was I should never have tagged those changes as 4.3.0. Adding an additional return type to the interface methods was breaking, so needs to be released as 5.0.0.

I've therefore reverted the changes in the core package and tagged as 4.3.1. I'll re-apply the changes and tag those as 5.0.0. That does mean I need to go through all the packages and ensure they can use core ^4.0|^5.0. Which normally would mean lots of extra work - but actually I have to go through all the packages today anyway to remove PHP 8.4 deprecation notices. So can just sort both things out at once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants