diff --git a/UPGRADE.md b/UPGRADE.md index 366c709b..a1c3b1ba 100644 --- 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 deleted file mode 100644 index b7882a78..00000000 --- a/src/Http/Middleware/CheckForAnyScope.php +++ /dev/null @@ -1,48 +0,0 @@ -user() || ! $request->user()->token()) { - throw new AuthenticationException; - } - - foreach ($scopes as $scope) { - if ($request->user()->tokenCan($scope)) { - return $next($request); - } - } - - throw new MissingScopeException($scopes); - } -} diff --git a/src/Http/Middleware/CheckScopes.php b/src/Http/Middleware/CheckScopes.php deleted file mode 100644 index 4d00eb96..00000000 --- a/src/Http/Middleware/CheckScopes.php +++ /dev/null @@ -1,48 +0,0 @@ -user() || ! $request->user()->token()) { - throw new AuthenticationException; - } - - foreach ($scopes as $scope) { - if (! $request->user()->tokenCan($scope)) { - throw new MissingScopeException($scope); - } - } - - return $next($request); - } -} diff --git a/src/Http/Middleware/CheckToken.php b/src/Http/Middleware/CheckToken.php index 552890f7..f7e0c1eb 100644 --- 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 index b05e142a..0386f17d 100644 --- 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 new file mode 100644 index 00000000..a552002c --- /dev/null +++ b/src/Http/Middleware/EnsureClientIsResourceOwner.php @@ -0,0 +1,28 @@ +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 index d9b5f2cf..06fae3a2 100644 --- 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 index 62617bc9..8860a5be 100644 --- 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 index 6f297849..60121372 100644 --- 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(), diff --git a/tests/Feature/ClientCredentialsGrantTest.php b/tests/Feature/ClientCredentialsGrantTest.php new file mode 100644 index 00000000..b0711e09 --- /dev/null +++ b/tests/Feature/ClientCredentialsGrantTest.php @@ -0,0 +1,57 @@ + 'Create', + 'read' => 'Read', + 'update' => 'Update', + 'delete' => 'Delete', + ]); + } + + public function testIssueAccessToken() + { + $client = ClientFactory::new()->asClientCredentials()->create(); + + $json = $this->post('/oauth/token', [ + 'grant_type' => 'client_credentials', + 'client_id' => $client->getKey(), + 'client_secret' => $client->plainSecret, + 'scope' => 'create read delete', + ])->assertOk()->json(); + + $this->assertArrayHasKey('access_token', $json); + $this->assertSame('Bearer', $json['token_type']); + $this->assertSame(31536000, $json['expires_in']); + + Route::get('/foo', fn (Request $request) => response('response')) + ->middleware([EnsureClientIsResourceOwner::using(['create', 'delete'])]); + + $response = $this->withToken($json['access_token'], $json['token_type'])->get('/foo'); + $response->assertOk(); + $this->assertSame('response', $response->content()); + + Route::get('/bar', fn (Request $request) => response('response')) + ->middleware(CheckToken::using(['create', 'update'])); + + $response = $this->withToken($json['access_token'], $json['token_type'])->get('/bar'); + $response->assertForbidden(); + } +} diff --git a/tests/Unit/CheckForAnyScopeTest.php b/tests/Unit/CheckForAnyScopeTest.php deleted file mode 100644 index 348b653c..00000000 --- a/tests/Unit/CheckForAnyScopeTest.php +++ /dev/null @@ -1,75 +0,0 @@ -shouldReceive('user')->andReturn($user = m::mock()); - $user->shouldReceive('token')->andReturn($token = m::mock()); - $user->shouldReceive('tokenCan')->with('foo')->andReturn(true); - $user->shouldReceive('tokenCan')->with('bar')->andReturn(false); - - $response = $middleware->handle($request, function () { - return new Response('response'); - }, 'foo', 'bar'); - - $this->assertSame('response', $response->getContent()); - } - - public function test_exception_is_thrown_if_token_doesnt_have_scope() - { - $this->expectException('Laravel\Passport\Exceptions\MissingScopeException'); - - $middleware = new CheckScopes; - $request = m::mock(Request::class); - $request->shouldReceive('user')->andReturn($user = m::mock()); - $user->shouldReceive('token')->andReturn($token = m::mock()); - $user->shouldReceive('tokenCan')->with('foo')->andReturn(false); - $user->shouldReceive('tokenCan')->with('bar')->andReturn(false); - - $middleware->handle($request, function () { - return new Response('response'); - }, 'foo', 'bar'); - } - - public function test_exception_is_thrown_if_no_authenticated_user() - { - $this->expectException(AuthenticationException::class); - - $middleware = new CheckScopes; - $request = m::mock(Request::class); - $request->shouldReceive('user')->once()->andReturn(null); - - $middleware->handle($request, function () { - return new Response('response'); - }, 'foo', 'bar'); - } - - public function test_exception_is_thrown_if_no_token() - { - $this->expectException(AuthenticationException::class); - - $middleware = new CheckScopes; - $request = m::mock(Request::class); - $request->shouldReceive('user')->andReturn($user = m::mock()); - $user->shouldReceive('token')->andReturn(null); - - $middleware->handle($request, function () { - return new Response('response'); - }, 'foo', 'bar'); - } -} diff --git a/tests/Unit/CheckScopesTest.php b/tests/Unit/CheckScopesTest.php deleted file mode 100644 index 5531717d..00000000 --- a/tests/Unit/CheckScopesTest.php +++ /dev/null @@ -1,74 +0,0 @@ -shouldReceive('user')->andReturn($user = m::mock()); - $user->shouldReceive('token')->andReturn($token = m::mock()); - $user->shouldReceive('tokenCan')->with('foo')->andReturn(true); - $user->shouldReceive('tokenCan')->with('bar')->andReturn(true); - - $response = $middleware->handle($request, function () { - return new Response('response'); - }, 'foo', 'bar'); - - $this->assertSame('response', $response->getContent()); - } - - public function test_exception_is_thrown_if_token_doesnt_have_scope() - { - $this->expectException('Laravel\Passport\Exceptions\MissingScopeException'); - - $middleware = new CheckScopes; - $request = m::mock(Request::class); - $request->shouldReceive('user')->andReturn($user = m::mock()); - $user->shouldReceive('token')->andReturn($token = m::mock()); - $user->shouldReceive('tokenCan')->with('foo')->andReturn(false); - - $middleware->handle($request, function () { - return new Response('response'); - }, 'foo', 'bar'); - } - - public function test_exception_is_thrown_if_no_authenticated_user() - { - $this->expectException(AuthenticationException::class); - - $middleware = new CheckScopes; - $request = m::mock(Request::class); - $request->shouldReceive('user')->once()->andReturn(null); - - $middleware->handle($request, function () { - return new Response('response'); - }, 'foo', 'bar'); - } - - public function test_exception_is_thrown_if_no_token() - { - $this->expectException(AuthenticationException::class); - - $middleware = new CheckScopes; - $request = m::mock(Request::class); - $request->shouldReceive('user')->andReturn($user = m::mock()); - $user->shouldReceive('token')->andReturn(null); - - $middleware->handle($request, function () { - return new Response('response'); - }, 'foo', 'bar'); - } -} diff --git a/tests/Unit/CheckTokenForAnyScopeTest.php b/tests/Unit/CheckTokenForAnyScopeTest.php index da5ef7d3..22fe874c 100644 --- a/tests/Unit/CheckTokenForAnyScopeTest.php +++ b/tests/Unit/CheckTokenForAnyScopeTest.php @@ -4,6 +4,7 @@ use Illuminate\Http\Request; use Illuminate\Http\Response; +use Laravel\Passport\AccessToken; use Laravel\Passport\Exceptions\AuthenticationException; use Laravel\Passport\Http\Middleware\CheckTokenForAnyScope; use League\OAuth2\Server\Exception\OAuthServerException; @@ -35,7 +36,7 @@ public function test_request_is_passed_along_if_token_is_valid() $response = $middleware->handle($request, function () { return new Response('response'); - }); + }, 'notfoo'); $this->assertSame('response', $response->getContent()); } @@ -104,4 +105,38 @@ public function test_exception_is_thrown_if_token_does_not_have_required_scope() return 'response'; }, 'baz', 'notbar'); } + + public function test_request_is_passed_along_if_scopes_are_present_on_token() + { + $resourceServer = m::mock(ResourceServer::class); + $middleware = new CheckTokenForAnyScope($resourceServer); + $request = m::mock(Request::class); + $request->shouldReceive('user')->andReturn($user = m::mock()); + $user->shouldReceive('token')->andReturn($token = m::mock(AccessToken::class)); + $token->shouldReceive('can')->with('foo')->andReturn(true); + $token->shouldReceive('can')->with('bar')->andReturn(false); + + $response = $middleware->handle($request, function () { + return new Response('response'); + }, 'foo', 'bar'); + + $this->assertSame('response', $response->getContent()); + } + + public function test_exception_is_thrown_if_token_doesnt_have_scope() + { + $this->expectException('Laravel\Passport\Exceptions\MissingScopeException'); + + $resourceServer = m::mock(ResourceServer::class); + $middleware = new CheckTokenForAnyScope($resourceServer); + $request = m::mock(Request::class); + $request->shouldReceive('user')->andReturn($user = m::mock()); + $user->shouldReceive('token')->andReturn($token = m::mock(AccessToken::class)); + $token->shouldReceive('can')->with('foo')->andReturn(false); + $token->shouldReceive('can')->with('bar')->andReturn(false); + + $middleware->handle($request, function () { + return new Response('response'); + }, 'foo', 'bar'); + } } diff --git a/tests/Unit/CheckTokenTest.php b/tests/Unit/CheckTokenTest.php index 555293e0..ce234c20 100644 --- a/tests/Unit/CheckTokenTest.php +++ b/tests/Unit/CheckTokenTest.php @@ -4,6 +4,7 @@ use Illuminate\Http\Request; use Illuminate\Http\Response; +use Laravel\Passport\AccessToken; use Laravel\Passport\Exceptions\AuthenticationException; use Laravel\Passport\Http\Middleware\CheckToken; use League\OAuth2\Server\Exception\OAuthServerException; @@ -104,4 +105,37 @@ public function test_exception_is_thrown_if_token_does_not_have_required_scopes( return 'response'; }, 'foo', 'bar'); } + + public function test_request_is_passed_along_if_scopes_are_present_on_token() + { + $resourceServer = m::mock(ResourceServer::class); + $middleware = new CheckToken($resourceServer); + $request = m::mock(Request::class); + $request->shouldReceive('user')->andReturn($user = m::mock()); + $user->shouldReceive('token')->andReturn($token = m::mock(AccessToken::class)); + $token->shouldReceive('cant')->with('foo')->andReturn(false); + $token->shouldReceive('cant')->with('bar')->andReturn(false); + + $response = $middleware->handle($request, function () { + return new Response('response'); + }, 'foo', 'bar'); + + $this->assertSame('response', $response->getContent()); + } + + public function test_exception_is_thrown_if_token_doesnt_have_scope() + { + $this->expectException('Laravel\Passport\Exceptions\MissingScopeException'); + + $resourceServer = m::mock(ResourceServer::class); + $middleware = new CheckToken($resourceServer); + $request = m::mock(Request::class); + $request->shouldReceive('user')->andReturn($user = m::mock()); + $user->shouldReceive('token')->andReturn($token = m::mock(AccessToken::class)); + $token->shouldReceive('cant')->with('foo')->andReturn(true); + + $middleware->handle($request, function () { + return new Response('response'); + }, 'foo', 'bar'); + } }