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

feat: support Illuminate\Auth\Access\Response from authorizer #298

Merged

Conversation

haddowg
Copy link

@haddowg haddowg commented Nov 22, 2024

This PR is dependent on laravel-json-api/core#19 and adds support for gates and policies with Illuminate\Auth\Access\Response return values.

I have not modified the composer.json to pull this branch from core but have tested it with it locally.

@lindyhopchris
Copy link
Contributor

@haddowg I've just tagged v4.3.0 of the core package, so you can pull that in now.

@haddowg haddowg force-pushed the feat/authorizer-response branch from 26f46bd to 8a6b0d3 Compare November 27, 2024 08:57
@haddowg
Copy link
Author

haddowg commented Nov 27, 2024

@haddowg I've just tagged v4.3.0 of the core package, so you can pull that in now.

@lindyhopchris updated

@lindyhopchris
Copy link
Contributor

@haddowg one final change required, you'll need to swap the laravel-json-api/core dependency to ^5.0.1

@lindyhopchris
Copy link
Contributor

Actually, might be best to put it to ^4.3.2|^5.0.1 as technically your change will work with both versions.

@haddowg haddowg force-pushed the feat/authorizer-response branch 4 times, most recently from ab45f5c to 76bfdb6 Compare November 30, 2024 19:27
@haddowg haddowg force-pushed the feat/authorizer-response branch from 76bfdb6 to 29b8ac7 Compare November 30, 2024 21:49
@haddowg
Copy link
Author

haddowg commented Nov 30, 2024

done

*/
public function authorizeResource(Authorizer $authorizer): bool
public function authorizeResource(Authorizer $authorizer): bool|Response
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is technically breaking. As the method is public, anyone could be calling it - and if they're relying on it returing a boolean it'll break their implementation.

So I'll need to tag this as a major release.

@lindyhopchris lindyhopchris merged commit e8d4e19 into laravel-json-api:develop Dec 1, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants