Let OAuth2 server handle denying the request (#1572)

laravel · Sep 7, 2022 · fb2a402 · fb2a402
1 parent 721df11
commit fb2a402
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 138 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,140 +19,44 @@ 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()
     {
         $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);
     }