From 6a2ebf709de0c9169aa70cac0e8da66f01952499 Mon Sep 17 00:00:00 2001 From: Claudio Dekker Date: Mon, 1 Nov 2021 20:32:55 +0100 Subject: [PATCH 1/3] Ability to enforce implicit scoping --- .../Routing/ImplicitRouteBinding.php | 2 +- src/Illuminate/Routing/Route.php | 10 +++ tests/Routing/ImplicitRouteBindingTest.php | 86 ++++++++++++++++++- 3 files changed, 95 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Routing/ImplicitRouteBinding.php b/src/Illuminate/Routing/ImplicitRouteBinding.php index 990a6466dbc0..f9f7885838da 100644 --- a/src/Illuminate/Routing/ImplicitRouteBinding.php +++ b/src/Illuminate/Routing/ImplicitRouteBinding.php @@ -41,7 +41,7 @@ public static function resolveForRoute($container, $route) ? 'resolveSoftDeletableRouteBinding' : 'resolveRouteBinding'; - if ($parent instanceof UrlRoutable && in_array($parameterName, array_keys($route->bindingFields()))) { + if ($parent instanceof UrlRoutable && ($route->enforcesScoping() || array_key_exists($parameterName, $route->bindingFields()))) { $childRouteBindingMethod = $route->allowsTrashedBindings() ? 'resolveSoftDeletableChildRouteBinding' : 'resolveChildRouteBinding'; diff --git a/src/Illuminate/Routing/Route.php b/src/Illuminate/Routing/Route.php index a6871521fcd1..bbb66464e96b 100755 --- a/src/Illuminate/Routing/Route.php +++ b/src/Illuminate/Routing/Route.php @@ -1077,6 +1077,16 @@ public function excludedMiddleware() return (array) ($this->action['excluded_middleware'] ?? []); } + /** + * Determine if the route should enforce scoping of multiple Eloquent bindings. + * + * @return bool + */ + public function enforcesScoping() + { + return (bool) ($this->action['scoping'] ?? false); + } + /** * Specify that the route should not allow concurrent requests from the same session. * diff --git a/tests/Routing/ImplicitRouteBindingTest.php b/tests/Routing/ImplicitRouteBindingTest.php index d4acfa63cf82..0d1f3144c223 100644 --- a/tests/Routing/ImplicitRouteBindingTest.php +++ b/tests/Routing/ImplicitRouteBindingTest.php @@ -3,7 +3,9 @@ namespace Illuminate\Tests\Routing; use Illuminate\Container\Container; -use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Capsule\Manager as DB; +use Illuminate\Database\Eloquent\Model as Eloquent; +use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Routing\ImplicitRouteBinding; use Illuminate\Routing\Route; use PHPUnit\Framework\TestCase; @@ -27,9 +29,89 @@ public function test_it_can_resolve_the_implicit_route_bindings_for_the_given_ro ImplicitRouteBinding::resolveForRoute($container, $route); } + + /** @test */ + public function it_enforces_scoping_of_implicit_route_bindings(): void + { + $this->prepareEloquent(); + + $action = ['uses' => function (ImplicitRouteBindingUser $user, ImplicitRouteBindingPost $post) { + return $user; + }]; + + $route = new Route('GET', '/users/{user}/posts/{post}', $action); + $route->action['scoping'] = true; + $route->parameters = ['user' => 1, 'post' => 1]; + + $container = Container::getInstance(); + + $this->expectException(ModelNotFoundException::class); + ImplicitRouteBinding::resolveForRoute($container, $route); + } + + /** @test */ + public function it_does_not_enforce_scoping_of_implicit_route_bindings(): void + { + $this->prepareEloquent(); + + $action = ['uses' => function (ImplicitRouteBindingUser $user, ImplicitRouteBindingPost $post) { + return $user; + }]; + + $route = new Route('GET', '/users/{user}/posts/{post}', $action); + $route->parameters = ['user' => 1, 'post' => 1]; + + $container = Container::getInstance(); + + ImplicitRouteBinding::resolveForRoute($container, $route); + + $this->assertTrue($route->parameter('user')->is(ImplicitRouteBindingUser::findOrFail(1))); + $this->assertTrue($route->parameter('post')->is(ImplicitRouteBindingPost::findOrFail(1))); + } + + protected function prepareEloquent() + { + $db = new DB; + $db->addConnection([ + 'driver' => 'sqlite', + 'database' => ':memory:', + ]); + + $db->bootEloquent(); + $db->setAsGlobal(); + + $schema = Eloquent::getConnectionResolver()->connection()->getSchemaBuilder(); + $schema->create('users', function ($table) { + $table->increments('id'); + }); + + $schema->create('posts', function ($table) { + $table->increments('id'); + $table->integer('user_id'); + }); + + ImplicitRouteBindingUser::create(['id' => 1]); + ImplicitRouteBindingPost::create(['id' => 1, 'user_id' => 2]); + } +} + +class Model extends Eloquent +{ + public $timestamps = false; + protected $guarded = false; } class ImplicitRouteBindingUser extends Model { - // + protected $table = 'users'; + + public function posts() + { + return $this->hasMany(ImplicitRouteBindingPost::class, 'user_id'); + } +} + +class ImplicitRouteBindingPost extends Model +{ + protected $table = 'posts'; } From 2bdc227aa3f9767620cc6e28171e74927313d423 Mon Sep 17 00:00:00 2001 From: Claudio Dekker Date: Tue, 2 Nov 2021 12:38:42 +0100 Subject: [PATCH 2/3] Change to Integration Test --- .../Routing/ImplicitRouteBindingTest.php | 110 ++++++++++++++++-- tests/Routing/ImplicitRouteBindingTest.php | 86 +------------- 2 files changed, 102 insertions(+), 94 deletions(-) diff --git a/tests/Integration/Routing/ImplicitRouteBindingTest.php b/tests/Integration/Routing/ImplicitRouteBindingTest.php index 987bb0fbad36..850862960162 100644 --- a/tests/Integration/Routing/ImplicitRouteBindingTest.php +++ b/tests/Integration/Routing/ImplicitRouteBindingTest.php @@ -50,8 +50,15 @@ protected function defineDatabaseMigrations(): void $table->softDeletes(); }); + Schema::create('posts', function (Blueprint $table) { + $table->increments('id'); + $table->integer('user_id'); + $table->timestamps(); + }); + $this->beforeApplicationDestroyed(function () { Schema::dropIfExists('users'); + Schema::dropIfExists('posts'); }); } @@ -60,14 +67,14 @@ public function testWithRouteCachingEnabled() $this->defineCacheRoutes(<<middleware('web'); PHP); - $user = ImplicitBindingModel::create(['name' => 'Dries']); + $user = ImplicitBindingUser::create(['name' => 'Dries']); $response = $this->postJson("/user/{$user->id}"); @@ -79,11 +86,11 @@ public function testWithRouteCachingEnabled() public function testWithoutRouteCachingEnabled() { - $user = ImplicitBindingModel::create(['name' => 'Dries']); + $user = ImplicitBindingUser::create(['name' => 'Dries']); config(['app.key' => str_repeat('a', 32)]); - Route::post('/user/{user}', function (ImplicitBindingModel $user) { + Route::post('/user/{user}', function (ImplicitBindingUser $user) { return $user; })->middleware(['web']); @@ -97,13 +104,13 @@ public function testWithoutRouteCachingEnabled() public function testSoftDeletedModelsAreNotRetrieved() { - $user = ImplicitBindingModel::create(['name' => 'Dries']); + $user = ImplicitBindingUser::create(['name' => 'Dries']); $user->delete(); config(['app.key' => str_repeat('a', 32)]); - Route::post('/user/{user}', function (ImplicitBindingModel $user) { + Route::post('/user/{user}', function (ImplicitBindingUser $user) { return $user; })->middleware(['web']); @@ -114,13 +121,13 @@ public function testSoftDeletedModelsAreNotRetrieved() public function testSoftDeletedModelsCanBeRetrievedUsingWithTrashedMethod() { - $user = ImplicitBindingModel::create(['name' => 'Dries']); + $user = ImplicitBindingUser::create(['name' => 'Dries']); $user->delete(); config(['app.key' => str_repeat('a', 32)]); - Route::post('/user/{user}', function (ImplicitBindingModel $user) { + Route::post('/user/{user}', function (ImplicitBindingUser $user) { return $user; })->middleware(['web'])->withTrashed(); @@ -131,13 +138,96 @@ public function testSoftDeletedModelsCanBeRetrievedUsingWithTrashedMethod() 'name' => $user->name, ]); } + + public function testEnforceScopingImplicitRouteBindings() + { + $user = ImplicitBindingUser::create(['name' => 'Dries']); + $post = ImplicitBindingPost::create(['user_id' => 2]); + $this->assertEmpty($user->posts); + + config(['app.key' => str_repeat('a', 32)]); + + Route::group(['scoping' => true], function () { + Route::get('/user/{user}/post/{post}', function (ImplicitBindingUser $user, ImplicitBindingPost $post) { + return [$user, $post]; + })->middleware(['web']); + }); + + $response = $this->getJson("/user/{$user->id}/post/{$post->id}"); + + $response->assertNotFound(); + } + + public function testEnforceScopingImplicitRouteBindingsWithRouteCachingEnabled() + { + $user = ImplicitBindingUser::create(['name' => 'Dries']); + $post = ImplicitBindingPost::create(['user_id' => 2]); + $this->assertEmpty($user->posts); + + $this->defineCacheRoutes(<< true], function () { + Route::get('/user/{user}/post/{post}', function (ImplicitBindingUser \$user, ImplicitBindingPost \$post) { + return [\$user, \$post]; + })->middleware(['web']); +}); +PHP); + + $response = $this->getJson("/user/{$user->id}/post/{$post->id}"); + + $response->assertNotFound(); + } + + public function testWithoutEnforceScopingImplicitRouteBindings() + { + $user = ImplicitBindingUser::create(['name' => 'Dries']); + $post = ImplicitBindingPost::create(['user_id' => 2]); + $this->assertEmpty($user->posts); + + config(['app.key' => str_repeat('a', 32)]); + + Route::group(['scoping' => false], function () { + Route::get('/user/{user}/post/{post}', function (ImplicitBindingUser $user, ImplicitBindingPost $post) { + return [$user, $post]; + })->middleware(['web']); + }); + + $response = $this->getJson("/user/{$user->id}/post/{$post->id}"); + $response->assertOk(); + $response->assertJson([ + [ + 'id' => $user->id, + 'name' => $user->name, + ], + [ + 'id' => 1, + 'user_id' => 2, + ], + ]); + } } -class ImplicitBindingModel extends Model +class ImplicitBindingUser extends Model { use SoftDeletes; public $table = 'users'; protected $fillable = ['name']; + + public function posts() + { + return $this->hasMany(ImplicitBindingPost::class, 'user_id'); + } +} + +class ImplicitBindingPost extends Model +{ + public $table = 'posts'; + + protected $fillable = ['user_id']; } diff --git a/tests/Routing/ImplicitRouteBindingTest.php b/tests/Routing/ImplicitRouteBindingTest.php index 0d1f3144c223..d4acfa63cf82 100644 --- a/tests/Routing/ImplicitRouteBindingTest.php +++ b/tests/Routing/ImplicitRouteBindingTest.php @@ -3,9 +3,7 @@ namespace Illuminate\Tests\Routing; use Illuminate\Container\Container; -use Illuminate\Database\Capsule\Manager as DB; -use Illuminate\Database\Eloquent\Model as Eloquent; -use Illuminate\Database\Eloquent\ModelNotFoundException; +use Illuminate\Database\Eloquent\Model; use Illuminate\Routing\ImplicitRouteBinding; use Illuminate\Routing\Route; use PHPUnit\Framework\TestCase; @@ -29,89 +27,9 @@ public function test_it_can_resolve_the_implicit_route_bindings_for_the_given_ro ImplicitRouteBinding::resolveForRoute($container, $route); } - - /** @test */ - public function it_enforces_scoping_of_implicit_route_bindings(): void - { - $this->prepareEloquent(); - - $action = ['uses' => function (ImplicitRouteBindingUser $user, ImplicitRouteBindingPost $post) { - return $user; - }]; - - $route = new Route('GET', '/users/{user}/posts/{post}', $action); - $route->action['scoping'] = true; - $route->parameters = ['user' => 1, 'post' => 1]; - - $container = Container::getInstance(); - - $this->expectException(ModelNotFoundException::class); - ImplicitRouteBinding::resolveForRoute($container, $route); - } - - /** @test */ - public function it_does_not_enforce_scoping_of_implicit_route_bindings(): void - { - $this->prepareEloquent(); - - $action = ['uses' => function (ImplicitRouteBindingUser $user, ImplicitRouteBindingPost $post) { - return $user; - }]; - - $route = new Route('GET', '/users/{user}/posts/{post}', $action); - $route->parameters = ['user' => 1, 'post' => 1]; - - $container = Container::getInstance(); - - ImplicitRouteBinding::resolveForRoute($container, $route); - - $this->assertTrue($route->parameter('user')->is(ImplicitRouteBindingUser::findOrFail(1))); - $this->assertTrue($route->parameter('post')->is(ImplicitRouteBindingPost::findOrFail(1))); - } - - protected function prepareEloquent() - { - $db = new DB; - $db->addConnection([ - 'driver' => 'sqlite', - 'database' => ':memory:', - ]); - - $db->bootEloquent(); - $db->setAsGlobal(); - - $schema = Eloquent::getConnectionResolver()->connection()->getSchemaBuilder(); - $schema->create('users', function ($table) { - $table->increments('id'); - }); - - $schema->create('posts', function ($table) { - $table->increments('id'); - $table->integer('user_id'); - }); - - ImplicitRouteBindingUser::create(['id' => 1]); - ImplicitRouteBindingPost::create(['id' => 1, 'user_id' => 2]); - } -} - -class Model extends Eloquent -{ - public $timestamps = false; - protected $guarded = false; } class ImplicitRouteBindingUser extends Model { - protected $table = 'users'; - - public function posts() - { - return $this->hasMany(ImplicitRouteBindingPost::class, 'user_id'); - } -} - -class ImplicitRouteBindingPost extends Model -{ - protected $table = 'posts'; + // } From 37f4dcfaacd2c195c5ce7d99c92b9966257fb5e2 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 4 Nov 2021 15:55:45 -0500 Subject: [PATCH 3/3] allow fluent scoping method --- .../Routing/ImplicitRouteBinding.php | 2 +- src/Illuminate/Routing/Route.php | 18 +++++++++++++++--- src/Illuminate/Routing/RouteRegistrar.php | 12 ++++++++++-- src/Illuminate/Routing/Router.php | 2 +- .../Routing/ImplicitRouteBindingTest.php | 2 +- 5 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/Illuminate/Routing/ImplicitRouteBinding.php b/src/Illuminate/Routing/ImplicitRouteBinding.php index f9f7885838da..d5f9488df3e5 100644 --- a/src/Illuminate/Routing/ImplicitRouteBinding.php +++ b/src/Illuminate/Routing/ImplicitRouteBinding.php @@ -41,7 +41,7 @@ public static function resolveForRoute($container, $route) ? 'resolveSoftDeletableRouteBinding' : 'resolveRouteBinding'; - if ($parent instanceof UrlRoutable && ($route->enforcesScoping() || array_key_exists($parameterName, $route->bindingFields()))) { + if ($parent instanceof UrlRoutable && ($route->enforcesScopedBindings() || array_key_exists($parameterName, $route->bindingFields()))) { $childRouteBindingMethod = $route->allowsTrashedBindings() ? 'resolveSoftDeletableChildRouteBinding' : 'resolveChildRouteBinding'; diff --git a/src/Illuminate/Routing/Route.php b/src/Illuminate/Routing/Route.php index bbb66464e96b..5de2f4004604 100755 --- a/src/Illuminate/Routing/Route.php +++ b/src/Illuminate/Routing/Route.php @@ -1078,13 +1078,25 @@ public function excludedMiddleware() } /** - * Determine if the route should enforce scoping of multiple Eloquent bindings. + * Indicate that the route should enforce scoping of multiple implicit Eloquent bindings. * * @return bool */ - public function enforcesScoping() + public function scopeBindings() { - return (bool) ($this->action['scoping'] ?? false); + $this->action['scope_bindings'] = true; + + return $this; + } + + /** + * Determine if the route should enforce scoping of multiple implicit Eloquent bindings. + * + * @return bool + */ + public function enforcesScopedBindings() + { + return (bool) ($this->action['scope_bindings'] ?? false); } /** diff --git a/src/Illuminate/Routing/RouteRegistrar.php b/src/Illuminate/Routing/RouteRegistrar.php index ad7f6c0ccafa..d89737497262 100644 --- a/src/Illuminate/Routing/RouteRegistrar.php +++ b/src/Illuminate/Routing/RouteRegistrar.php @@ -55,7 +55,14 @@ class RouteRegistrar * @var string[] */ protected $allowedAttributes = [ - 'as', 'domain', 'middleware', 'name', 'namespace', 'prefix', 'where', + 'as', + 'domain', + 'middleware', + 'name', + 'namespace', + 'prefix', + 'scopeBindings', + 'where', ]; /** @@ -65,6 +72,7 @@ class RouteRegistrar */ protected $aliases = [ 'name' => 'as', + 'scopeBindings' => 'scope_bindings', ]; /** @@ -216,7 +224,7 @@ public function __call($method, $parameters) return $this->attribute($method, is_array($parameters[0]) ? $parameters[0] : $parameters); } - return $this->attribute($method, $parameters[0]); + return $this->attribute($method, $parameters[0] ?? true); } throw new BadMethodCallException(sprintf( diff --git a/src/Illuminate/Routing/Router.php b/src/Illuminate/Routing/Router.php index eaecb4873687..241cfdb63e6d 100644 --- a/src/Illuminate/Routing/Router.php +++ b/src/Illuminate/Routing/Router.php @@ -1326,6 +1326,6 @@ public function __call($method, $parameters) return (new RouteRegistrar($this))->attribute($method, is_array($parameters[0]) ? $parameters[0] : $parameters); } - return (new RouteRegistrar($this))->attribute($method, $parameters[0]); + return (new RouteRegistrar($this))->attribute($method, $parameters[0] ?? true); } } diff --git a/tests/Integration/Routing/ImplicitRouteBindingTest.php b/tests/Integration/Routing/ImplicitRouteBindingTest.php index 850862960162..a930e7fc5115 100644 --- a/tests/Integration/Routing/ImplicitRouteBindingTest.php +++ b/tests/Integration/Routing/ImplicitRouteBindingTest.php @@ -147,7 +147,7 @@ public function testEnforceScopingImplicitRouteBindings() config(['app.key' => str_repeat('a', 32)]); - Route::group(['scoping' => true], function () { + Route::scopeBindings()->group(function () { Route::get('/user/{user}/post/{post}', function (ImplicitBindingUser $user, ImplicitBindingPost $post) { return [$user, $post]; })->middleware(['web']);