[13.x] Always validate auth token (#1769)

* fix trait * fix tests
laravel · Jul 5, 2024 · 0ee1d8b · 0ee1d8b
1 parent 4ffc542
commit 0ee1d8b
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 33 deletions.
diff --git a/src/Http/Controllers/AccessTokenController.php b/src/Http/Controllers/AccessTokenController.php
@@ -9,7 +9,7 @@
 
 class AccessTokenController
 {
-    use HandlesOAuthErrors;
+    use ConvertsPsrResponses, HandlesOAuthErrors;
 
     /**
      * The authorization server.

diff --git a/src/Http/Controllers/ApproveAuthorizationController.php b/src/Http/Controllers/ApproveAuthorizationController.php
@@ -36,8 +36,6 @@ public function __construct(AuthorizationServer $server)
      */
     public function approve(Request $request)
     {
-        $this->assertValidAuthToken($request);
-
         $authRequest = $this->getAuthRequestFromSession($request);
 
         $authRequest->setAuthorizationApproved(true);

diff --git a/src/Http/Controllers/AuthorizationController.php b/src/Http/Controllers/AuthorizationController.php
@@ -18,7 +18,7 @@
 
 class AuthorizationController
 {
-    use HandlesOAuthErrors;
+    use ConvertsPsrResponses, HandlesOAuthErrors;
 
     /**
      * The authorization server.

diff --git a/src/Http/Controllers/DenyAuthorizationController.php b/src/Http/Controllers/DenyAuthorizationController.php
@@ -36,8 +36,6 @@ public function __construct(AuthorizationServer $server)
      */
     public function deny(Request $request)
     {
-        $this->assertValidAuthToken($request);
-
         $authRequest = $this->getAuthRequestFromSession($request);
 
         $authRequest->setAuthorizationApproved(false);

diff --git a/src/Http/Controllers/RetrievesAuthRequestFromSession.php b/src/Http/Controllers/RetrievesAuthRequestFromSession.php
@@ -6,37 +6,25 @@
 use Illuminate\Http\Request;
 use Laravel\Passport\Bridge\User;
 use Laravel\Passport\Exceptions\InvalidAuthTokenException;
+use League\OAuth2\Server\RequestTypes\AuthorizationRequest;
 
 trait RetrievesAuthRequestFromSession
 {
     /**
-     * Make sure the auth token matches the one in the session.
-     *
-     * @param  \Illuminate\Http\Request  $request
-     * @return void
+     * Get the authorization request from the session.
      *
      * @throws \Laravel\Passport\Exceptions\InvalidAuthTokenException
+     * @throws \Exception
      */
-    protected function assertValidAuthToken(Request $request)
+    protected function getAuthRequestFromSession(Request $request): AuthorizationRequest
     {
-        if ($request->has('auth_token') && $request->session()->get('authToken') !== $request->get('auth_token')) {
+        if ($request->isNotFilled('auth_token') || $request->session()->pull('authToken') !== $request->get('auth_token')) {
             $request->session()->forget(['authToken', 'authRequest']);
 
             throw InvalidAuthTokenException::different();
         }
-    }
 
-    /**
-     * Get the authorization request from the session.
-     *
-     * @param  \Illuminate\Http\Request  $request
-     * @return \League\OAuth2\Server\RequestTypes\AuthorizationRequest
-     *
-     * @throws \Exception
-     */
-    protected function getAuthRequestFromSession(Request $request)
-    {
-        return tap($request->session()->get('authRequest'), function ($authRequest) use ($request) {
+        return tap($request->session()->pull('authRequest'), function ($authRequest) use ($request) {
             if (! $authRequest) {
                 throw new Exception('Authorization request was not present in the session.');
             }

diff --git a/tests/Unit/ApproveAuthorizationControllerTest.php b/tests/Unit/ApproveAuthorizationControllerTest.php
@@ -26,11 +26,11 @@ public function test_complete_authorization_request()
 
         $request = m::mock(Request::class);
         $request->shouldReceive('session')->andReturn($session = m::mock());
-        $request->shouldReceive('has')->with('auth_token')->andReturn(true);
+        $request->shouldReceive('isNotFilled')->with('auth_token')->andReturn(false);
         $request->shouldReceive('get')->with('auth_token')->andReturn('foo');
 
-        $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo');
-        $session->shouldReceive('get')
+        $session->shouldReceive('pull')->once()->with('authToken')->andReturn('foo');
+        $session->shouldReceive('pull')
             ->once()
             ->with('authRequest')
             ->andReturn($authRequest = m::mock(AuthorizationRequest::class));

diff --git a/tests/Unit/DenyAuthorizationControllerTest.php b/tests/Unit/DenyAuthorizationControllerTest.php
@@ -28,11 +28,11 @@ public function test_authorization_can_be_denied()
 
         $request->shouldReceive('session')->andReturn($session = m::mock());
         $request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser);
-        $request->shouldReceive('has')->with('auth_token')->andReturn(true);
+        $request->shouldReceive('isNotFilled')->with('auth_token')->andReturn(false);
         $request->shouldReceive('get')->with('auth_token')->andReturn('foo');
 
-        $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo');
-        $session->shouldReceive('get')
+        $session->shouldReceive('pull')->once()->with('authToken')->andReturn('foo');
+        $session->shouldReceive('pull')
             ->once()
             ->with('authRequest')
             ->andReturn($authRequest = m::mock(
@@ -65,11 +65,11 @@ public function test_auth_request_should_exist()
         $request->shouldReceive('session')->andReturn($session = m::mock());
         $request->shouldReceive('user')->never();
         $request->shouldReceive('input')->never();
-        $request->shouldReceive('has')->with('auth_token')->andReturn(true);
+        $request->shouldReceive('isNotFilled')->with('auth_token')->andReturn(false);
         $request->shouldReceive('get')->with('auth_token')->andReturn('foo');
 
-        $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo');
-        $session->shouldReceive('get')->once()->with('authRequest')->andReturnNull();
+        $session->shouldReceive('pull')->once()->with('authToken')->andReturn('foo');
+        $session->shouldReceive('pull')->once()->with('authRequest')->andReturnNull();
 
         $server->shouldReceive('completeAuthorizationRequest')->never();