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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"require": {
"php": "^8.2",
"ext-json": "*",
"laravel-json-api/core": "^4.3.2",
"laravel-json-api/core": "^4.3.2|^5.0.1",
"laravel-json-api/eloquent": "^4.4",
"laravel-json-api/encoder-neomerx": "^4.1",
"laravel-json-api/exceptions": "^3.1",
Expand Down
62 changes: 38 additions & 24 deletions src/Http/Requests/FormRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace LaravelJsonApi\Laravel\Http\Requests;

use Illuminate\Auth\Access\AuthorizationException;
use Illuminate\Auth\Access\Response;
use Illuminate\Auth\AuthenticationException;
use Illuminate\Contracts\Auth\Guard;
use Illuminate\Foundation\Http\FormRequest as BaseFormRequest;
Expand Down Expand Up @@ -226,42 +228,54 @@ public function schema(): Schema
*/
protected function passesAuthorization()
{
/**
* If the developer has implemented the `authorize` method, we
* will return the result if it is a boolean. This allows
* the developer to return a null value to indicate they want
* the default authorization to run.
*/
if (method_exists($this, 'authorize')) {
if (is_bool($passes = $this->container->call([$this, 'authorize']))) {
return $passes;
try {
/**
* If the developer has implemented the `authorize` method, we
* will return the result if it is a boolean. This allows
* the developer to return a null value to indicate they want
* the default authorization to run.
*/
if (method_exists($this, 'authorize')) {
$result = $this->container->call([$this, 'authorize']);
if ($result !== null) {
return $result instanceof Response ? $result->authorize() : $result;
}
}
}

/**
* If the developer has not authorized the request themselves,
* we run our default authorization as long as authorization is
* enabled for both the server and the schema (checked via the
* `mustAuthorize()` method).
*/
if (method_exists($this, 'authorizeResource')) {
return $this->container->call([$this, 'authorizeResource']);
}
/**
* If the developer has not authorized the request themselves,
* we run our default authorization as long as authorization is
* enabled for both the server and the schema (checked via the
* `mustAuthorize()` method).
*/
if (method_exists($this, 'authorizeResource')) {
$result = $this->container->call([$this, 'authorizeResource']);
return $result instanceof Response ? $result->authorize() : $result;
}

} catch (AuthorizationException $ex) {
$this->failIfUnauthenticated();
throw $ex;
}
return true;
}

/**
* @inheritDoc
*/
protected function failedAuthorization()
protected function failIfUnauthenticated()
{
/** @var Guard $auth */
/** @var Guard $auth */
$auth = $this->container->make(Guard::class);

if ($auth->guest()) {
throw new AuthenticationException();
}
}

/**
* @inheritDoc
*/
protected function failedAuthorization()
{
$this->failIfUnauthenticated();

parent::failedAuthorization();
}
Expand Down
5 changes: 3 additions & 2 deletions src/Http/Requests/ResourceQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace LaravelJsonApi\Laravel\Http\Requests;

use Illuminate\Auth\Access\Response;
use Illuminate\Contracts\Validation\Validator;
use Illuminate\Database\Eloquent\Model;
use LaravelJsonApi\Contracts\Auth\Authorizer;
Expand Down Expand Up @@ -104,9 +105,9 @@ public static function queryOne(string $resourceType): QueryParameters
* Perform resource authorization.
*
* @param Authorizer $authorizer
* @return bool
* @return bool|Response
*/
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.

{
if ($this->isViewingAny()) {
return $authorizer->index(
Expand Down
5 changes: 3 additions & 2 deletions src/Http/Requests/ResourceRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace LaravelJsonApi\Laravel\Http\Requests;

use Illuminate\Auth\Access\Response;
use Illuminate\Contracts\Validation\Factory as ValidationFactory;
use Illuminate\Contracts\Validation\Validator;
use Illuminate\Database\Eloquent\Model;
Expand Down Expand Up @@ -150,9 +151,9 @@ public function toMany(): Collection
* Perform resource authorization.
*
* @param Authorizer $authorizer
* @return bool
* @return bool|Response
*/
public function authorizeResource(Authorizer $authorizer): bool
public function authorizeResource(Authorizer $authorizer): bool|Response
{
if ($this->isCreating()) {
return $authorizer->store(
Expand Down
1 change: 1 addition & 0 deletions tests/dummy/app/Http/Controllers/Api/V1/UserController.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
class UserController extends Controller
{
use Actions\FetchOne;
use Actions\Destroy;
use Actions\FetchRelated;
use Actions\FetchRelationship;
use Actions\UpdateRelationship;
Expand Down
14 changes: 14 additions & 0 deletions tests/dummy/app/Policies/UserPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace App\Policies;

use App\Models\User;
use Illuminate\Auth\Access\Response;

class UserPolicy
{
Expand Down Expand Up @@ -50,4 +51,17 @@ public function updatePhone(User $user, User $other): bool
{
return $user->is($other);
}

/**
* Determine if the user can delete the other user.
*
* @param User $user
* @param User $other
* @return bool|Response
*/
public function delete(User $user, User $other)
{
return $user->is($other) ? true : Response::denyAsNotFound('not found message');
}

}
2 changes: 1 addition & 1 deletion tests/dummy/routes/api.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
});

/** Users */
$server->resource('users')->only('show')->relationships(function ($relationships) {
$server->resource('users')->only('show','destroy')->relationships(function ($relationships) {
$relationships->hasOne('phone');
})->actions(function ($actions) {
$actions->get('me');
Expand Down
38 changes: 38 additions & 0 deletions tests/dummy/tests/Api/V1/Users/DeleteTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php
/*
* Copyright 2024 Cloud Creativity Limited
*
* Use of this source code is governed by an MIT-style
* license that can be found in the LICENSE file or at
* https://opensource.org/licenses/MIT.
*/

declare(strict_types=1);

namespace App\Tests\Api\V1\Users;

use App\Models\User;
use App\Tests\Api\V1\TestCase;

class DeleteTest extends TestCase
{

public function test(): void
{
$user = User::factory()->createOne();

$expected = $this->serializer
->user($user);
$response = $this
->actingAs(User::factory()->createOne())
->jsonApi('users')
->delete(url('/api/v1/users', $expected['id']));

$response->assertNotFound()
->assertHasError(404, [
'detail' => 'not found message',
'status' => '404',
'title' => 'Not Found',
]);
}
}