diff --git a/src/Http/Controllers/ClientController.php b/src/Http/Controllers/ClientController.php index 0365f3b34..b0f64ebde 100644 --- a/src/Http/Controllers/ClientController.php +++ b/src/Http/Controllers/ClientController.php @@ -5,6 +5,7 @@ use Illuminate\Http\Request; use Illuminate\Http\Response; use Laravel\Passport\ClientRepository; +use Laravel\Passport\Http\Rules\RedirectRule; use Illuminate\Contracts\Validation\Factory as ValidationFactory; class ClientController @@ -23,18 +24,29 @@ class ClientController */ protected $validation; + /** + * The redirect validation rule. + * + * @var \Laravel\Passport\Http\Rules\RedirectRule + */ + protected $redirectRule; + /** * Create a client controller instance. * * @param \Laravel\Passport\ClientRepository $clients * @param \Illuminate\Contracts\Validation\Factory $validation + * @param \Laravel\Passport\Http\Rules\RedirectRule $redirectRule * @return void */ - public function __construct(ClientRepository $clients, - ValidationFactory $validation) - { + public function __construct( + ClientRepository $clients, + ValidationFactory $validation, + RedirectRule $redirectRule + ) { $this->clients = $clients; $this->validation = $validation; + $this->redirectRule = $redirectRule; } /** @@ -60,7 +72,7 @@ public function store(Request $request) { $this->validation->make($request->all(), [ 'name' => 'required|max:255', - 'redirect' => 'required|url', + 'redirect' => ['required', $this->redirectRule], ])->validate(); return $this->clients->create( @@ -85,7 +97,7 @@ public function update(Request $request, $clientId) $this->validation->make($request->all(), [ 'name' => 'required|max:255', - 'redirect' => 'required|url', + 'redirect' => ['required', $this->redirectRule], ])->validate(); return $this->clients->update( diff --git a/src/Http/Rules/RedirectRule.php b/src/Http/Rules/RedirectRule.php new file mode 100644 index 000000000..bbbd9944c --- /dev/null +++ b/src/Http/Rules/RedirectRule.php @@ -0,0 +1,43 @@ +validator = $validator; + } + + /** + * {@inheritdoc} + */ + public function passes($attribute, $value) + { + foreach (explode(',', $value) as $redirect) { + $validator = $this->validator->make(['redirect' => $redirect], ['redirect' => 'url']); + + if ($validator->fails()) { + return false; + } + } + + return true; + } + + /** + * {@inheritdoc} + */ + public function message() + { + return 'One or more redirects have an invalid url format.'; + } +} diff --git a/tests/ClientControllerTest.php b/tests/ClientControllerTest.php index 54bf93b92..e7117e562 100644 --- a/tests/ClientControllerTest.php +++ b/tests/ClientControllerTest.php @@ -2,11 +2,12 @@ namespace Laravel\Passport\Tests; -use Laravel\Passport\Client; -use Laravel\Passport\Http\Controllers\ClientController; use Mockery as m; use Illuminate\Http\Request; +use Laravel\Passport\Client; use PHPUnit\Framework\TestCase; +use Laravel\Passport\Http\Rules\RedirectRule; +use Laravel\Passport\Http\Controllers\ClientController; class ClientControllerTest extends TestCase { @@ -25,7 +26,9 @@ public function test_all_the_clients_for_the_current_user_can_be_retrieved() $request->shouldReceive('user')->andReturn(new ClientControllerFakeUser); $controller = new ClientController( - $clients, m::mock('Illuminate\Contracts\Validation\Factory') + $clients, + m::mock('Illuminate\Contracts\Validation\Factory'), + m::mock(RedirectRule::class) ); $this->assertEquals($client, $controller->forUser($request)); @@ -45,17 +48,21 @@ public function test_clients_can_be_stored() ->with(1, 'client name', 'http://localhost') ->andReturn($client = new Client); + $redirectRule = m::mock(RedirectRule::class); + $validator = m::mock('Illuminate\Contracts\Validation\Factory'); $validator->shouldReceive('make')->once()->with([ 'name' => 'client name', 'redirect' => 'http://localhost', ], [ 'name' => 'required|max:255', - 'redirect' => 'required|url', + 'redirect' => ['required', $redirectRule], ])->andReturn($validator); $validator->shouldReceive('validate')->once(); - $controller = new ClientController($clients, $validator); + $controller = new ClientController( + $clients, $validator, $redirectRule + ); $this->assertEquals($client, $controller->store($request)); } @@ -79,17 +86,21 @@ public function test_clients_can_be_updated() m::type('Laravel\Passport\Client'), 'client name', 'http://localhost' )->andReturn('response'); + $redirectRule = m::mock(RedirectRule::class); + $validator = m::mock('Illuminate\Contracts\Validation\Factory'); $validator->shouldReceive('make')->once()->with([ 'name' => 'client name', 'redirect' => 'http://localhost', ], [ 'name' => 'required|max:255', - 'redirect' => 'required|url', + 'redirect' => ['required', $redirectRule], ])->andReturn($validator); $validator->shouldReceive('validate')->once(); - $controller = new ClientController($clients, $validator); + $controller = new ClientController( + $clients, $validator, $redirectRule + ); $this->assertEquals('response', $controller->update($request, 1)); } @@ -112,7 +123,9 @@ public function test_404_response_if_client_doesnt_belong_to_user() $validator = m::mock('Illuminate\Contracts\Validation\Factory'); - $controller = new ClientController($clients, $validator); + $controller = new ClientController( + $clients, $validator, m::mock(RedirectRule::class) + ); $this->assertEquals(404, $controller->update($request, 1)->status()); } @@ -138,7 +151,9 @@ public function test_clients_can_be_deleted() $validator = m::mock('Illuminate\Contracts\Validation\Factory'); - $controller = new ClientController($clients, $validator); + $controller = new ClientController( + $clients, $validator, m::mock(RedirectRule::class) + ); $controller->destroy($request, 1); } @@ -161,7 +176,9 @@ public function test_404_response_if_client_doesnt_belong_to_user_on_delete() $validator = m::mock('Illuminate\Contracts\Validation\Factory'); - $controller = new ClientController($clients, $validator); + $controller = new ClientController( + $clients, $validator, m::mock(RedirectRule::class) + ); $this->assertEquals(404, $controller->destroy($request, 1)->status()); } diff --git a/tests/RedirectRuleTest.php b/tests/RedirectRuleTest.php new file mode 100644 index 000000000..31dc7d4de --- /dev/null +++ b/tests/RedirectRuleTest.php @@ -0,0 +1,49 @@ +rule($fails = false); + + $this->assertTrue($rule->passes('redirect', 'https://example.com')); + } + + public function test_it_passes_with_multiple_valid_urls() + { + $rule = $this->rule($fails = false); + + $this->assertTrue($rule->passes('redirect', 'https://example.com,https://example2.com')); + } + + public function test_it_fails_with_a_single_invalid_url() + { + $rule = $this->rule($fails = true); + + $this->assertFalse($rule->passes('redirect', 'https://example.com,invalid')); + } + + private function rule(bool $fails): RedirectRule + { + $validator = m::mock(Validator::class); + $validator->shouldReceive('fails')->andReturn($fails); + + $factory = m::mock(Factory::class); + $factory->shouldReceive('make')->andReturn($validator); + + return new RedirectRule($factory); + } +}