[13.x] Add new EnsureClientIsResourceOwner middleware (#1794)

* add new middleware and rename old ones * fix tests * upgrade guide * formatting
laravel · Oct 9, 2024 · 6fafccc · 6fafccc
1 parent ef122ad
commit 6fafccc
Show file tree

Hide file tree

Showing 14 changed files with 218 additions and 301 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -66,10 +66,12 @@ When authenticating users via bearer tokens, the `User` model's `token` method n
 
 ### Renamed Middlewares
 
-PR: https://github.com/laravel/passport/pull/1792
+PR: https://github.com/laravel/passport/pull/1792, https://github.com/laravel/passport/pull/1794
 
-Passport's `CheckClientCredentials` and `CheckClientCredentialsForAnyScope` middleware have been renamed to better reflect their functionality:
+The following Passport's middlewares have been renamed to better reflect their functionality:
 
+* `Laravel\Passport\Http\Middleware\CheckScopes` class has been renamed to `CheckToken`.
+* `Laravel\Passport\Http\Middleware\CheckForAnyScope` class has been renamed to `CheckTokenForAnyScope`.
 * `Laravel\Passport\Http\Middleware\CheckClientCredentials` class has been renamed to `CheckToken`.
 * `Laravel\Passport\Http\Middleware\CheckClientCredentialsForAnyScope` class has been renamed to `CheckTokenForAnyScope`.
 * `Laravel\Passport\Http\Middleware\CheckCredentials` abstract class has been renamed to `ValidateToken`.

diff --git a/src/Http/Middleware/CheckForAnyScope.php b/src/Http/Middleware/CheckForAnyScope.php
diff --git a/src/Http/Middleware/CheckScopes.php b/src/Http/Middleware/CheckScopes.php
diff --git a/src/Http/Middleware/CheckToken.php b/src/Http/Middleware/CheckToken.php
@@ -10,17 +10,11 @@ class CheckToken extends ValidateToken
     /**
      * Determine if the token has all the given scopes.
      *
-     * @param  string[]  $scopes
-     *
      * @throws \Laravel\Passport\Exceptions\MissingScopeException
      */
-    protected function hasScopes(AccessToken $token, array $scopes): void
+    protected function validate(AccessToken $token, string ...$params): void
     {
-        if (in_array('*', $token->oauth_scopes)) {
-            return;
-        }
-
-        foreach ($scopes as $scope) {
+        foreach ($params as $scope) {
             if ($token->cant($scope)) {
                 throw new MissingScopeException($scope);
             }

diff --git a/src/Http/Middleware/CheckTokenForAnyScope.php b/src/Http/Middleware/CheckTokenForAnyScope.php
@@ -10,22 +10,16 @@ class CheckTokenForAnyScope extends ValidateToken
     /**
      * Determine if the token has at least one of the given scopes.
      *
-     * @param  string[]  $scopes
-     *
      * @throws \Laravel\Passport\Exceptions\MissingScopeException
      */
-    protected function hasScopes(AccessToken $token, array $scopes): void
+    protected function validate(AccessToken $token, string ...$params): void
     {
-        if (in_array('*', $token->oauth_scopes)) {
-            return;
-        }
-
-        foreach ($scopes as $scope) {
+        foreach ($params as $scope) {
             if ($token->can($scope)) {
                 return;
             }
         }
 
-        throw new MissingScopeException($scopes);
+        throw new MissingScopeException($params);
     }
 }
diff --git a/src/Http/Middleware/EnsureClientIsResourceOwner.php b/src/Http/Middleware/EnsureClientIsResourceOwner.php
@@ -0,0 +1,28 @@
+<?php
+
+namespace Laravel\Passport\Http\Middleware;
+
+use Illuminate\Auth\AuthenticationException;
+use Laravel\Passport\AccessToken;
+use Laravel\Passport\Exceptions\MissingScopeException;
+
+class EnsureClientIsResourceOwner extends ValidateToken
+{
+    /**
+     * Determine if the token's client is the resource owner and has all the given scopes.
+     *
+     * @throws \Exception
+     */
+    protected function validate(AccessToken $token, string ...$params): void
+    {
+        if ($token->oauth_user_id !== $token->oauth_client_id) {
+            throw new AuthenticationException;
+        }
+
+        foreach ($params as $scope) {
+            if ($token->cant($scope)) {
+                throw new MissingScopeException($scope);
+            }
+        }
+    }
+}
diff --git a/src/Http/Middleware/ValidateToken.php b/src/Http/Middleware/ValidateToken.php
@@ -22,48 +22,66 @@ public function __construct(
     }
 
     /**
-     * Specify the scopes for the middleware.
+     * Specify the parameters for the middleware.
      *
-     * @param  string[]|string  ...$scopes
+     * @param  string[]|string  ...$params
      */
-    public static function using(...$scopes): string
+    public static function using(...$params): string
     {
-        if (is_array($scopes[0])) {
-            return static::class.':'.implode(',', $scopes[0]);
+        if (is_array($params[0])) {
+            return static::class.':'.implode(',', $params[0]);
         }
 
-        return static::class.':'.implode(',', $scopes);
+        return static::class.':'.implode(',', $params);
     }
 
     /**
      * Handle an incoming request.
      *
      * @param  \Closure(\Illuminate\Http\Request): (\Symfony\Component\HttpFoundation\Response)  $next
-     * @param  string[]|string  ...$scopes
+     * @param  string[]|string  ...$params
+     */
+    public function handle(Request $request, Closure $next, string ...$params): Response
+    {
+        $token = $this->validateToken($request);
+
+        $this->validate($token, ...$params);
+
+        return $next($request);
+    }
+
+    /**
+     * Validate and get the request's access token.
      *
      * @throws \Laravel\Passport\Exceptions\AuthenticationException
      */
-    public function handle(Request $request, Closure $next, string ...$scopes): Response
+    protected function validateToken(Request $request): AccessToken
     {
-        $psr = (new PsrHttpFactory())->createRequest($request);
+        // If the user is authenticated and already has an access token set via
+        // the token guard, there's no need to validate the request's bearer
+        // token again, so we'll return the access token as the valid one.
+        if ($request->user()?->token()) {
+            return $request->user()->token();
+        }
+
+        // Otherwise, we will convert the request to a PSR-7 implementation and
+        // pass it to the OAuth2 server to be validated. If the bearer token
+        // passed the validation, we will return an access token instance.
+        $psrRequest = (new PsrHttpFactory())->createRequest($request);
 
         try {
-            $psr = $this->server->validateAuthenticatedRequest($psr);
+            $psrRequest = $this->server->validateAuthenticatedRequest($psrRequest);
         } catch (OAuthServerException) {
             throw new AuthenticationException;
         }
 
-        $this->hasScopes(AccessToken::fromPsrRequest($psr), $scopes);
-
-        return $next($request);
+        return AccessToken::fromPsrRequest($psrRequest);
     }
 
     /**
-     * Determine if the token has the given scopes.
-     *
-     * @param  string[]  $scopes
+     * Validate the given access token.
      *
      * @throws \Laravel\Passport\Exceptions\MissingScopeException
      */
-    abstract protected function hasScopes(AccessToken $token, array $scopes): void;
+    abstract protected function validate(AccessToken $token, string ...$params): void;
 }
diff --git a/tests/Feature/ActingAsTest.php b/tests/Feature/ActingAsTest.php
@@ -4,8 +4,8 @@
 
 use Illuminate\Contracts\Routing\Registrar;
 use Illuminate\Support\Facades\Route;
-use Laravel\Passport\Http\Middleware\CheckForAnyScope;
-use Laravel\Passport\Http\Middleware\CheckScopes;
+use Laravel\Passport\Http\Middleware\CheckToken;
+use Laravel\Passport\Http\Middleware\CheckTokenForAnyScope;
 use Laravel\Passport\Passport;
 use Workbench\App\Models\User;
 
@@ -38,7 +38,7 @@ public function testActingAsWhenTheRouteIsProtectedByCheckScopesMiddleware()
 
         $router->get('/foo', function () {
             return 'bar';
-        })->middleware(CheckScopes::class.':admin,footest');
+        })->middleware(CheckToken::class.':admin,footest');
 
         Passport::actingAs(new User(), ['admin', 'footest']);
 
@@ -49,23 +49,23 @@ public function testActingAsWhenTheRouteIsProtectedByCheckScopesMiddleware()
 
     public function testItCanGenerateDefinitionViaStaticMethod()
     {
-        $signature = (string) CheckScopes::using('admin');
-        $this->assertSame('Laravel\Passport\Http\Middleware\CheckScopes:admin', $signature);
+        $signature = (string) CheckToken::using('admin');
+        $this->assertSame('Laravel\Passport\Http\Middleware\CheckToken:admin', $signature);
 
-        $signature = (string) CheckScopes::using('admin', 'footest');
-        $this->assertSame('Laravel\Passport\Http\Middleware\CheckScopes:admin,footest', $signature);
+        $signature = (string) CheckToken::using('admin', 'footest');
+        $this->assertSame('Laravel\Passport\Http\Middleware\CheckToken:admin,footest', $signature);
 
-        $signature = (string) CheckScopes::using(['admin', 'footest']);
-        $this->assertSame('Laravel\Passport\Http\Middleware\CheckScopes:admin,footest', $signature);
+        $signature = (string) CheckToken::using(['admin', 'footest']);
+        $this->assertSame('Laravel\Passport\Http\Middleware\CheckToken:admin,footest', $signature);
 
-        $signature = (string) CheckForAnyScope::using('admin');
-        $this->assertSame('Laravel\Passport\Http\Middleware\CheckForAnyScope:admin', $signature);
+        $signature = (string) CheckTokenForAnyScope::using('admin');
+        $this->assertSame('Laravel\Passport\Http\Middleware\CheckTokenForAnyScope:admin', $signature);
 
-        $signature = (string) CheckForAnyScope::using('admin', 'footest');
-        $this->assertSame('Laravel\Passport\Http\Middleware\CheckForAnyScope:admin,footest', $signature);
+        $signature = (string) CheckTokenForAnyScope::using('admin', 'footest');
+        $this->assertSame('Laravel\Passport\Http\Middleware\CheckTokenForAnyScope:admin,footest', $signature);
 
-        $signature = (string) CheckForAnyScope::using(['admin', 'footest']);
-        $this->assertSame('Laravel\Passport\Http\Middleware\CheckForAnyScope:admin,footest', $signature);
+        $signature = (string) CheckTokenForAnyScope::using(['admin', 'footest']);
+        $this->assertSame('Laravel\Passport\Http\Middleware\CheckTokenForAnyScope:admin,footest', $signature);
     }
 
     public function testActingAsWhenTheRouteIsProtectedByCheckForAnyScopeMiddleware()
@@ -77,7 +77,7 @@ public function testActingAsWhenTheRouteIsProtectedByCheckForAnyScopeMiddleware(
 
         $router->get('/foo', function () {
             return 'bar';
-        })->middleware(CheckForAnyScope::class.':admin,footest');
+        })->middleware(CheckTokenForAnyScope::class.':admin,footest');
 
         Passport::actingAs(new User(), ['footest']);
 
@@ -92,7 +92,7 @@ public function testActingAsWhenTheRouteIsProtectedByCheckScopesMiddlewareWithIn
 
         $this->withoutExceptionHandling();
 
-        Route::middleware(CheckScopes::class.':foo:bar,baz:qux')->get('/foo', function () {
+        Route::middleware(CheckToken::class.':foo:bar,baz:qux')->get('/foo', function () {
             return 'bar';
         });
 
@@ -109,7 +109,7 @@ public function testActingAsWhenTheRouteIsProtectedByCheckForAnyScopeMiddlewareW
 
         $this->withoutExceptionHandling();
 
-        Route::middleware(CheckForAnyScope::class.':foo:baz,baz:qux')->get('/foo', function () {
+        Route::middleware(CheckTokenForAnyScope::class.':foo:baz,baz:qux')->get('/foo', function () {
             return 'bar';
         });
 

diff --git a/tests/Feature/AuthorizationCodeGrantTest.php b/tests/Feature/AuthorizationCodeGrantTest.php
@@ -142,7 +142,7 @@ public function testSkipsAuthorizationWhenHasGrantedScopes()
             'client_secret' => $client->plainSecret,
             'redirect_uri' => $redirect,
             'code' => $params['code'],
-        ]);
+        ])->assertOk();
 
         $query = http_build_query([
             'client_id' => $client->getKey(),