From 721df114d5a036d8fb3f542b83ab2879ecea7d69 Mon Sep 17 00:00:00 2001 From: driesvints Date: Tue, 6 Sep 2022 16:00:05 +0000 Subject: [PATCH 1/3] Update CHANGELOG --- CHANGELOG.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2f07ebc9..74086e1bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ # Release Notes -## [Unreleased](https://github.com/laravel/passport/compare/v11.0.1...11.x) +## [Unreleased](https://github.com/laravel/passport/compare/v11.1.0...11.x) + +## [v11.1.0](https://github.com/laravel/passport/compare/v11.0.1...v11.1.0) - 2022-09-05 + +### Added + +- Support prompting re-consent when redirecting for authorization by @hafezdivandari in https://github.com/laravel/passport/pull/1567 +- Support disabling prompt when redirecting for authorization by @hafezdivandari in https://github.com/laravel/passport/pull/1569 ## [v11.0.1](https://github.com/laravel/passport/compare/v11.0.0...v11.0.1) - 2022-08-29 From fb2a4029c6cf4df1ce0cf79ddbb874508f7a73e9 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Wed, 7 Sep 2022 13:14:45 +0000 Subject: [PATCH 2/3] Let OAuth2 server handle denying the request (#1572) --- .../ApproveAuthorizationController.php | 12 +- .../DenyAuthorizationController.php | 34 ++--- .../RetrievesAuthRequestFromSession.php | 2 - .../Unit/DenyAuthorizationControllerTest.php | 131 +++--------------- 4 files changed, 41 insertions(+), 138 deletions(-) diff --git a/src/Http/Controllers/ApproveAuthorizationController.php b/src/Http/Controllers/ApproveAuthorizationController.php index 90023d5e2..6a2d41511 100644 --- a/src/Http/Controllers/ApproveAuthorizationController.php +++ b/src/Http/Controllers/ApproveAuthorizationController.php @@ -8,7 +8,7 @@ class ApproveAuthorizationController { - use ConvertsPsrResponses, RetrievesAuthRequestFromSession; + use ConvertsPsrResponses, HandlesOAuthErrors, RetrievesAuthRequestFromSession; /** * The authorization server. @@ -40,8 +40,12 @@ public function approve(Request $request) $authRequest = $this->getAuthRequestFromSession($request); - return $this->convertResponse( - $this->server->completeAuthorizationRequest($authRequest, new Psr7Response) - ); + $authRequest->setAuthorizationApproved(true); + + return $this->withErrorHandling(function () use ($authRequest) { + return $this->convertResponse( + $this->server->completeAuthorizationRequest($authRequest, new Psr7Response) + ); + }); } } diff --git a/src/Http/Controllers/DenyAuthorizationController.php b/src/Http/Controllers/DenyAuthorizationController.php index 613dfd465..3a46e6174 100644 --- a/src/Http/Controllers/DenyAuthorizationController.php +++ b/src/Http/Controllers/DenyAuthorizationController.php @@ -2,30 +2,30 @@ namespace Laravel\Passport\Http\Controllers; -use Illuminate\Contracts\Routing\ResponseFactory; use Illuminate\Http\Request; -use Illuminate\Support\Arr; +use League\OAuth2\Server\AuthorizationServer; +use Nyholm\Psr7\Response as Psr7Response; class DenyAuthorizationController { - use RetrievesAuthRequestFromSession; + use ConvertsPsrResponses, HandlesOAuthErrors, RetrievesAuthRequestFromSession; /** - * The response factory implementation. + * The authorization server. * - * @var \Illuminate\Contracts\Routing\ResponseFactory + * @var \League\OAuth2\Server\AuthorizationServer */ - protected $response; + protected $server; /** * Create a new controller instance. * - * @param \Illuminate\Contracts\Routing\ResponseFactory $response + * @param \League\OAuth2\Server\AuthorizationServer $server * @return void */ - public function __construct(ResponseFactory $response) + public function __construct(AuthorizationServer $server) { - $this->response = $response; + $this->server = $server; } /** @@ -40,16 +40,12 @@ public function deny(Request $request) $authRequest = $this->getAuthRequestFromSession($request); - $clientUris = Arr::wrap($authRequest->getClient()->getRedirectUri()); + $authRequest->setAuthorizationApproved(false); - if (! in_array($uri = $authRequest->getRedirectUri(), $clientUris)) { - $uri = Arr::first($clientUris); - } - - $separator = $authRequest->getGrantTypeId() === 'implicit' ? '#' : (strstr($uri, '?') ? '&' : '?'); - - return $this->response->redirectTo( - $uri.$separator.'error=access_denied&state='.$request->input('state') - ); + return $this->withErrorHandling(function () use ($authRequest) { + return $this->convertResponse( + $this->server->completeAuthorizationRequest($authRequest, new Psr7Response) + ); + }); } } diff --git a/src/Http/Controllers/RetrievesAuthRequestFromSession.php b/src/Http/Controllers/RetrievesAuthRequestFromSession.php index b1296277c..0a23e1ec4 100644 --- a/src/Http/Controllers/RetrievesAuthRequestFromSession.php +++ b/src/Http/Controllers/RetrievesAuthRequestFromSession.php @@ -42,8 +42,6 @@ protected function getAuthRequestFromSession(Request $request) } $authRequest->setUser(new User($request->user()->getAuthIdentifier())); - - $authRequest->setAuthorizationApproved(true); }); } } diff --git a/tests/Unit/DenyAuthorizationControllerTest.php b/tests/Unit/DenyAuthorizationControllerTest.php index cfb5e01cc..366cc8636 100644 --- a/tests/Unit/DenyAuthorizationControllerTest.php +++ b/tests/Unit/DenyAuthorizationControllerTest.php @@ -2,12 +2,13 @@ namespace Laravel\Passport\Tests\Unit; -use Illuminate\Contracts\Routing\ResponseFactory; use Illuminate\Http\Request; use Laravel\Passport\Http\Controllers\DenyAuthorizationController; +use League\OAuth2\Server\AuthorizationServer; use League\OAuth2\Server\RequestTypes\AuthorizationRequest; use Mockery as m; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\ResponseInterface; class DenyAuthorizationControllerTest extends TestCase { @@ -18,130 +19,34 @@ protected function tearDown(): void public function test_authorization_can_be_denied() { - $response = m::mock(ResponseFactory::class); + $this->expectException('Laravel\Passport\Exceptions\OAuthServerException'); - $controller = new DenyAuthorizationController($response); + $server = m::mock(AuthorizationServer::class); + $controller = new DenyAuthorizationController($server); $request = m::mock(Request::class); $request->shouldReceive('session')->andReturn($session = m::mock()); $request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser); - $request->shouldReceive('input')->with('state')->andReturn('state'); $request->shouldReceive('has')->with('auth_token')->andReturn(true); $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); - $session->shouldReceive('get')->once()->with('authRequest')->andReturn($authRequest = m::mock( + $session->shouldReceive('get') + ->once() + ->with('authRequest') + ->andReturn($authRequest = m::mock( AuthorizationRequest::class - )); + )); $authRequest->shouldReceive('setUser')->once(); - $authRequest->shouldReceive('getGrantTypeId')->andReturn('authorization_code'); - $authRequest->shouldReceive('setAuthorizationApproved')->once()->with(true); - $authRequest->shouldReceive('getRedirectUri')->andReturn('http://localhost'); - $authRequest->shouldReceive('getClient->getRedirectUri')->andReturn('http://localhost'); + $authRequest->shouldReceive('setAuthorizationApproved')->once()->with(false); - $response->shouldReceive('redirectTo')->once()->andReturnUsing(function ($url) { - return $url; - }); + $server->shouldReceive('completeAuthorizationRequest') + ->with($authRequest, m::type(ResponseInterface::class)) + ->andThrow('League\OAuth2\Server\Exception\OAuthServerException'); - $this->assertSame('http://localhost?error=access_denied&state=state', $controller->deny($request)); - } - - public function test_authorization_can_be_denied_with_multiple_redirect_uris() - { - $response = m::mock(ResponseFactory::class); - - $controller = new DenyAuthorizationController($response); - - $request = m::mock(Request::class); - - $request->shouldReceive('session')->andReturn($session = m::mock()); - $request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser); - $request->shouldReceive('input')->with('state')->andReturn('state'); - $request->shouldReceive('has')->with('auth_token')->andReturn(true); - $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); - - $session->shouldReceive('get')->once()->with('authRequest')->andReturn($authRequest = m::mock( - AuthorizationRequest::class - )); - - $authRequest->shouldReceive('setUser')->once(); - $authRequest->shouldReceive('getGrantTypeId')->andReturn('authorization_code'); - $authRequest->shouldReceive('setAuthorizationApproved')->once()->with(true); - $authRequest->shouldReceive('getRedirectUri')->andReturn('http://localhost'); - $authRequest->shouldReceive('getClient->getRedirectUri')->andReturn(['http://localhost.localdomain', 'http://localhost']); - - $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); - $response->shouldReceive('redirectTo')->once()->andReturnUsing(function ($url) { - return $url; - }); - - $this->assertSame('http://localhost?error=access_denied&state=state', $controller->deny($request)); - } - - public function test_authorization_can_be_denied_implicit() - { - $response = m::mock(ResponseFactory::class); - - $controller = new DenyAuthorizationController($response); - - $request = m::mock(Request::class); - - $request->shouldReceive('session')->andReturn($session = m::mock()); - $request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser); - $request->shouldReceive('input')->with('state')->andReturn('state'); - $request->shouldReceive('has')->with('auth_token')->andReturn(true); - $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); - - $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); - $session->shouldReceive('get')->once()->with('authRequest')->andReturn($authRequest = m::mock( - AuthorizationRequest::class - )); - - $authRequest->shouldReceive('setUser')->once(); - $authRequest->shouldReceive('getGrantTypeId')->andReturn('implicit'); - $authRequest->shouldReceive('setAuthorizationApproved')->once()->with(true); - $authRequest->shouldReceive('getRedirectUri')->andReturn('http://localhost'); - $authRequest->shouldReceive('getClient->getRedirectUri')->andReturn('http://localhost'); - - $response->shouldReceive('redirectTo')->once()->andReturnUsing(function ($url) { - return $url; - }); - - $this->assertSame('http://localhost#error=access_denied&state=state', $controller->deny($request)); - } - - public function test_authorization_can_be_denied_with_existing_query_string() - { - $response = m::mock(ResponseFactory::class); - - $controller = new DenyAuthorizationController($response); - - $request = m::mock(Request::class); - - $request->shouldReceive('session')->andReturn($session = m::mock()); - $request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser); - $request->shouldReceive('input')->with('state')->andReturn('state'); - $request->shouldReceive('has')->with('auth_token')->andReturn(true); - $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); - - $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); - $session->shouldReceive('get')->once()->with('authRequest')->andReturn($authRequest = m::mock( - AuthorizationRequest::class - )); - - $authRequest->shouldReceive('setUser')->once(); - $authRequest->shouldReceive('getGrantTypeId')->andReturn('authorization_code'); - $authRequest->shouldReceive('setAuthorizationApproved')->once()->with(true); - $authRequest->shouldReceive('getRedirectUri')->andReturn('http://localhost?action=some_action'); - $authRequest->shouldReceive('getClient->getRedirectUri')->andReturn('http://localhost?action=some_action'); - - $response->shouldReceive('redirectTo')->once()->andReturnUsing(function ($url) { - return $url; - }); - - $this->assertSame('http://localhost?action=some_action&error=access_denied&state=state', $controller->deny($request)); + $controller->deny($request); } public function test_auth_request_should_exist() @@ -149,9 +54,9 @@ public function test_auth_request_should_exist() $this->expectException('Exception'); $this->expectExceptionMessage('Authorization request was not present in the session.'); - $response = m::mock(ResponseFactory::class); + $server = m::mock(AuthorizationServer::class); - $controller = new DenyAuthorizationController($response); + $controller = new DenyAuthorizationController($server); $request = m::mock(Request::class); @@ -164,7 +69,7 @@ public function test_auth_request_should_exist() $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); $session->shouldReceive('get')->once()->with('authRequest')->andReturnNull(); - $response->shouldReceive('redirectTo')->never(); + $server->shouldReceive('completeAuthorizationRequest')->never(); $controller->deny($request); } From 7a514ccb1ef3e83be285ad0a0a83b03d701f8c3f Mon Sep 17 00:00:00 2001 From: driesvints Date: Tue, 13 Sep 2022 15:06:14 +0000 Subject: [PATCH 3/3] Update CHANGELOG --- CHANGELOG.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74086e1bd..134492a21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ # Release Notes -## [Unreleased](https://github.com/laravel/passport/compare/v11.1.0...11.x) +## [Unreleased](https://github.com/laravel/passport/compare/v11.2.0...11.x) + +## [v11.2.0](https://github.com/laravel/passport/compare/v11.1.0...v11.2.0) - 2022-09-07 + +### Changed + +- Let OAuth2 server handle the denying response by @hafezdivandari in https://github.com/laravel/passport/pull/1572 ## [v11.1.0](https://github.com/laravel/passport/compare/v11.0.1...v11.1.0) - 2022-09-05