Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[13.x] Improve resolving and converting PSR responses #1793

Merged
merged 3 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"nyholm/psr7": "^1.5",
"phpseclib/phpseclib": "^3.0",
"symfony/console": "^7.0",
"symfony/psr-http-message-bridge": "^7.0"
"symfony/psr-http-message-bridge": "^7.1"
},
"require-dev": {
"mockery/mockery": "^1.0",
Expand Down
6 changes: 4 additions & 2 deletions src/Exceptions/OAuthServerException.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use Laravel\Passport\Http\Controllers\ConvertsPsrResponses;
use League\OAuth2\Server\Exception\OAuthServerException as LeagueException;
use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
use Nyholm\Psr7\Response as Psr7Response;
use Psr\Http\Message\ResponseInterface;

class OAuthServerException extends HttpResponseException
{
Expand All @@ -18,7 +18,9 @@ class OAuthServerException extends HttpResponseException
*/
public function __construct(LeagueException $e, bool $useFragment = false)
{
parent::__construct($this->convertResponse($e->generateHttpResponse(new Psr7Response, $useFragment)), $e);
parent::__construct($this->convertResponse(
$e->generateHttpResponse(app(ResponseInterface::class), $useFragment)
), $e);
}

/**
Expand Down
10 changes: 2 additions & 8 deletions src/Guards/TokenGuard.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
use Laravel\Passport\TransientToken;
use League\OAuth2\Server\Exception\OAuthServerException;
use League\OAuth2\Server\ResourceServer;
use Nyholm\Psr7\Factory\Psr17Factory;
use Psr\Http\Message\ServerRequestInterface;
use Symfony\Bridge\PsrHttpMessage\Factory\PsrHttpFactory;

Expand Down Expand Up @@ -161,13 +160,8 @@ protected function getPsrRequestViaBearerToken(): ?ServerRequestInterface
{
// First, we will convert the Symfony request to a PSR-7 implementation which will
// be compatible with the base OAuth2 library. The Symfony bridge can perform a
// conversion for us to a new Nyholm implementation of this PSR-7 request.
$psr = (new PsrHttpFactory(
new Psr17Factory,
new Psr17Factory,
new Psr17Factory,
new Psr17Factory
))->createRequest($this->request);
// conversion for us to a new PSR-7 implementation from this Symfony request.
$psr = (new PsrHttpFactory())->createRequest($this->request);

try {
return $this->server->validateAuthenticatedRequest($psr);
Expand Down
12 changes: 6 additions & 6 deletions src/Http/Controllers/AccessTokenController.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

namespace Laravel\Passport\Http\Controllers;

use Illuminate\Http\Response;
use League\OAuth2\Server\AuthorizationServer;
use League\OAuth2\Server\Exception\OAuthServerException;
use Nyholm\Psr7\Response as Psr7Response;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Symfony\Component\HttpFoundation\Response;

class AccessTokenController
{
Expand All @@ -23,16 +23,16 @@ public function __construct(
/**
* Issue an access token.
*/
public function issueToken(ServerRequestInterface $request): Response
public function issueToken(ServerRequestInterface $psrRequest, ResponseInterface $psrResponse): Response
{
return $this->withErrorHandling(function () use ($request) {
if (array_key_exists('grant_type', $attributes = (array) $request->getParsedBody()) &&
return $this->withErrorHandling(function () use ($psrRequest, $psrResponse) {
if (array_key_exists('grant_type', $attributes = (array) $psrRequest->getParsedBody()) &&
$attributes['grant_type'] === 'personal_access') {
throw OAuthServerException::unsupportedGrantType();
}

return $this->convertResponse(
$this->server->respondToAccessTokenRequest($request, new Psr7Response)
$this->server->respondToAccessTokenRequest($psrRequest, $psrResponse)
);
});
}
Expand Down
8 changes: 4 additions & 4 deletions src/Http/Controllers/ApproveAuthorizationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
namespace Laravel\Passport\Http\Controllers;

use Illuminate\Http\Request;
use Illuminate\Http\Response;
use League\OAuth2\Server\AuthorizationServer;
use Nyholm\Psr7\Response as Psr7Response;
use Psr\Http\Message\ResponseInterface;
use Symfony\Component\HttpFoundation\Response;

class ApproveAuthorizationController
{
Expand All @@ -22,14 +22,14 @@ public function __construct(
/**
* Approve the authorization request.
*/
public function approve(Request $request): Response
public function approve(Request $request, ResponseInterface $psrResponse): Response
{
$authRequest = $this->getAuthRequestFromSession($request);

$authRequest->setAuthorizationApproved(true);

return $this->withErrorHandling(fn () => $this->convertResponse(
$this->server->completeAuthorizationRequest($authRequest, new Psr7Response)
$this->server->completeAuthorizationRequest($authRequest, $psrResponse)
), $authRequest->getGrantTypeId() === 'implicit');
}
}
21 changes: 12 additions & 9 deletions src/Http/Controllers/AuthorizationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use Illuminate\Contracts\Auth\Authenticatable;
use Illuminate\Contracts\Auth\StatefulGuard;
use Illuminate\Http\Request;
use Illuminate\Http\Response;
use Illuminate\Support\Facades\Date;
use Illuminate\Support\Str;
use Laravel\Passport\Bridge\User;
Expand All @@ -18,8 +17,9 @@
use League\OAuth2\Server\AuthorizationServer;
use League\OAuth2\Server\Entities\ScopeEntityInterface;
use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
use Nyholm\Psr7\Response as Psr7Response;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Symfony\Component\HttpFoundation\Response;

class AuthorizationController
{
Expand All @@ -31,16 +31,19 @@ class AuthorizationController
public function __construct(
protected AuthorizationServer $server,
protected StatefulGuard $guard,
protected AuthorizationViewResponse $response,
protected ClientRepository $clients
) {
}

/**
* Authorize a client to access the user's account.
*/
public function authorize(ServerRequestInterface $psrRequest, Request $request): Response|AuthorizationViewResponse
{
public function authorize(
ServerRequestInterface $psrRequest,
Request $request,
ResponseInterface $psrResponse,
AuthorizationViewResponse $viewResponse
): Response|AuthorizationViewResponse {
$authRequest = $this->withErrorHandling(
fn () => $this->server->validateAuthorizationRequest($psrRequest),
($psrRequest->getQueryParams()['response_type'] ?? null) === 'token'
Expand Down Expand Up @@ -71,7 +74,7 @@ public function authorize(ServerRequestInterface $psrRequest, Request $request):

if ($request->get('prompt') !== 'consent' &&
($client->skipsAuthorization($user, $scopes) || $this->hasGrantedScopes($user, $client, $scopes))) {
return $this->approveRequest($authRequest);
return $this->approveRequest($authRequest, $psrResponse);
}

if ($request->get('prompt') === 'none') {
Expand All @@ -81,7 +84,7 @@ public function authorize(ServerRequestInterface $psrRequest, Request $request):
$request->session()->put('authToken', $authToken = Str::random());
$request->session()->put('authRequest', $authRequest);

return $this->response->withParameters([
return $viewResponse->withParameters([
'client' => $client,
'user' => $user,
'scopes' => $scopes,
Expand Down Expand Up @@ -124,12 +127,12 @@ protected function hasGrantedScopes(Authenticatable $user, Client $client, array
/**
* Approve the authorization request.
*/
protected function approveRequest(AuthorizationRequestInterface $authRequest): Response
protected function approveRequest(AuthorizationRequestInterface $authRequest, ResponseInterface $psrResponse): Response
{
$authRequest->setAuthorizationApproved(true);

return $this->withErrorHandling(fn () => $this->convertResponse(
$this->server->completeAuthorizationRequest($authRequest, new Psr7Response)
$this->server->completeAuthorizationRequest($authRequest, $psrResponse)
), $authRequest->getGrantTypeId() === 'implicit');
}

Expand Down
9 changes: 3 additions & 6 deletions src/Http/Controllers/ConvertsPsrResponses.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

namespace Laravel\Passport\Http\Controllers;

use Illuminate\Http\Response;
use Psr\Http\Message\ResponseInterface;
use Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory;
use Symfony\Component\HttpFoundation\Response;

trait ConvertsPsrResponses
{
Expand All @@ -12,10 +13,6 @@ trait ConvertsPsrResponses
*/
public function convertResponse(ResponseInterface $psrResponse): Response
{
return new Response(
$psrResponse->getBody(),
$psrResponse->getStatusCode(),
$psrResponse->getHeaders()
);
return (new HttpFoundationFactory())->createResponse($psrResponse);
}
}
8 changes: 4 additions & 4 deletions src/Http/Controllers/DenyAuthorizationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
namespace Laravel\Passport\Http\Controllers;

use Illuminate\Http\Request;
use Illuminate\Http\Response;
use League\OAuth2\Server\AuthorizationServer;
use Nyholm\Psr7\Response as Psr7Response;
use Psr\Http\Message\ResponseInterface;
use Symfony\Component\HttpFoundation\Response;

class DenyAuthorizationController
{
Expand All @@ -22,14 +22,14 @@ public function __construct(
/**
* Deny the authorization request.
*/
public function deny(Request $request): Response
public function deny(Request $request, ResponseInterface $psrResponse): Response
{
$authRequest = $this->getAuthRequestFromSession($request);

$authRequest->setAuthorizationApproved(false);

return $this->withErrorHandling(fn () => $this->convertResponse(
$this->server->completeAuthorizationRequest($authRequest, new Psr7Response)
$this->server->completeAuthorizationRequest($authRequest, $psrResponse)
), $authRequest->getGrantTypeId() === 'implicit');
}
}
8 changes: 1 addition & 7 deletions src/Http/Middleware/ValidateToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use Laravel\Passport\Exceptions\AuthenticationException;
use League\OAuth2\Server\Exception\OAuthServerException;
use League\OAuth2\Server\ResourceServer;
use Nyholm\Psr7\Factory\Psr17Factory;
use Symfony\Bridge\PsrHttpMessage\Factory\PsrHttpFactory;
use Symfony\Component\HttpFoundation\Response;

Expand Down Expand Up @@ -46,12 +45,7 @@ public static function using(...$scopes): string
*/
public function handle(Request $request, Closure $next, string ...$scopes): Response
{
$psr = (new PsrHttpFactory(
new Psr17Factory,
new Psr17Factory,
new Psr17Factory,
new Psr17Factory
))->createRequest($request);
$psr = (new PsrHttpFactory())->createRequest($request);

try {
$psr = $this->server->validateAuthenticatedRequest($psr);
Expand Down
11 changes: 6 additions & 5 deletions src/PersonalAccessTokenFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@

use Lcobucci\JWT\Parser as JwtParser;
use League\OAuth2\Server\AuthorizationServer;
use Nyholm\Psr7\Response;
use Nyholm\Psr7\ServerRequest;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Symfony\Bridge\PsrHttpMessage\Factory\PsrHttpFactory;
use Symfony\Component\HttpFoundation\Request;

class PersonalAccessTokenFactory
{
Expand Down Expand Up @@ -48,12 +49,12 @@ public function make(string|int $userId, string $name, array $scopes, string $pr
*/
protected function createRequest(string|int $userId, array $scopes, string $provider): ServerRequestInterface
{
return (new ServerRequest('POST', 'not-important'))->withParsedBody([
return (new PsrHttpFactory())->createRequest(Request::create('not-important', 'POST', [
'grant_type' => 'personal_access',
'provider' => $provider,
'user_id' => $userId,
'scope' => implode(' ', $scopes),
]);
]));
}

/**
Expand All @@ -64,7 +65,7 @@ protected function createRequest(string|int $userId, array $scopes, string $prov
protected function dispatchRequestToAuthorizationServer(ServerRequestInterface $request): array
{
return json_decode($this->server->respondToAccessTokenRequest(
$request, new Response
$request, app(ResponseInterface::class)
)->getBody()->__toString(), true);
}

Expand Down
6 changes: 4 additions & 2 deletions tests/Unit/AccessTokenControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,16 @@ public function test_a_token_can_be_issued()

$controller = new AccessTokenController($server);

$this->assertSame('{"access_token":"access-token"}', $controller->issueToken($request)->getContent());
$this->assertSame('{"access_token":"access-token"}', $controller->issueToken($request, $psrResponse)->getContent());
}

public function test_exceptions_are_handled()
{
$request = m::mock(ServerRequestInterface::class);
$request->shouldReceive('getParsedBody')->once()->andReturn([]);

app()->instance(ResponseInterface::class, new Response);

$server = m::mock(AuthorizationServer::class);
$server->shouldReceive('respondToAccessTokenRequest')->with(
$request, m::type(ResponseInterface::class)
Expand All @@ -51,7 +53,7 @@ public function test_exceptions_are_handled()

$this->expectException(OAuthServerException::class);

$controller->issueToken($request);
$controller->issueToken($request, m::mock(ResponseInterface::class));
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/Unit/ApproveAuthorizationControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function test_complete_authorization_request()
->with($authRequest, m::type(ResponseInterface::class))
->andReturn($psrResponse);

$this->assertSame('response', $controller->approve($request)->getContent());
$this->assertSame('response', $controller->approve($request, $psrResponse)->getContent());
}
}

Expand Down
Loading