Skip to content

Commit d80b6dd

Browse files
hafezdivandarihafezdivandari
authored
[13.x] Enhance error responses (#1791)
* enhance error responses * add feature tests * formatting
1 parent 0c33c4c commit d80b6dd

13 files changed

+713
-115
lines changed

‎database/factories/ClientFactory.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,16 @@ public function definition(): array
3838
];
3939
}
4040

41+
/**
42+
* Use as a public client.
43+
*/
44+
public function asPublic(): static
45+
{
46+
return $this->state([
47+
'secret' => null,
48+
]);
49+
}
50+
4151
/**
4252
* Use as a Password client.
4353
*/
@@ -67,6 +77,7 @@ public function asImplicitClient(): static
6777
{
6878
return $this->state([
6979
'grant_types' => ['implicit'],
80+
'secret' => null,
7081
]);
7182
}
7283

‎src/Exceptions/OAuthServerException.php‎

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,65 @@
33
namespace Laravel\Passport\Exceptions;
44

55
use Illuminate\Http\Exceptions\HttpResponseException;
6-
use Illuminate\Http\Response;
6+
use Illuminate\Support\Arr;
7+
use Laravel\Passport\Http\Controllers\ConvertsPsrResponses;
78
use League\OAuth2\Server\Exception\OAuthServerException as LeagueException;
9+
use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
10+
use Nyholm\Psr7\Response as Psr7Response;
811

912
class OAuthServerException extends HttpResponseException
1013
{
14+
use ConvertsPsrResponses;
15+
1116
/**
1217
* Create a new OAuthServerException.
1318
*/
14-
public function __construct(LeagueException $e, Response $response)
19+
public function __construct(LeagueException $e, bool $useFragment = false)
20+
{
21+
parent::__construct($this->convertResponse($e->generateHttpResponse(new Psr7Response, $useFragment)), $e);
22+
}
23+
24+
/**
25+
* Create a new OAuthServerException for when login is required.
26+
*/
27+
public static function loginRequired(AuthorizationRequestInterface $authRequest): static
1528
{
16-
parent::__construct($response, $e);
29+
$exception = new LeagueException(
30+
'The authorization server requires end-user authentication.',
31+
9,
32+
'login_required',
33+
401,
34+
'The user is not authenticated',
35+
$authRequest->getRedirectUri() ?? Arr::wrap($authRequest->getClient()->getRedirectUri())[0]
36+
);
37+
38+
$exception->setPayload([
39+
'state' => $authRequest->getState(),
40+
...$exception->getPayload(),
41+
]);
42+
43+
return new static($exception, $authRequest->getGrantTypeId() === 'implicit');
44+
}
45+
46+
/**
47+
* Create a new OAuthServerException for when consent is required.
48+
*/
49+
public static function consentRequired(AuthorizationRequestInterface $authRequest): static
50+
{
51+
$exception = new LeagueException(
52+
'The authorization server requires end-user consent.',
53+
9,
54+
'consent_required',
55+
401,
56+
null,
57+
$authRequest->getRedirectUri() ?? Arr::wrap($authRequest->getClient()->getRedirectUri())[0]
58+
);
59+
60+
$exception->setPayload([
61+
'state' => $authRequest->getState(),
62+
...$exception->getPayload(),
63+
]);
64+
65+
return new static($exception, $authRequest->getGrantTypeId() === 'implicit');
1766
}
1867
}

‎src/Http/Controllers/ApproveAuthorizationController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,6 @@ public function approve(Request $request): Response
3030

3131
return $this->withErrorHandling(fn () => $this->convertResponse(
3232
$this->server->completeAuthorizationRequest($authRequest, new Psr7Response)
33-
));
33+
), $authRequest->getGrantTypeId() === 'implicit');
3434
}
3535
}

‎src/Http/Controllers/AuthorizationController.php‎

Lines changed: 15 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@
1313
use Laravel\Passport\ClientRepository;
1414
use Laravel\Passport\Contracts\AuthorizationViewResponse;
1515
use Laravel\Passport\Exceptions\AuthenticationException;
16+
use Laravel\Passport\Exceptions\OAuthServerException;
1617
use Laravel\Passport\Passport;
1718
use League\OAuth2\Server\AuthorizationServer;
1819
use League\OAuth2\Server\Entities\ScopeEntityInterface;
19-
use League\OAuth2\Server\Exception\OAuthServerException;
2020
use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
2121
use Nyholm\Psr7\Response as Psr7Response;
2222
use Psr\Http\Message\ServerRequestInterface;
@@ -41,14 +41,15 @@ public function __construct(
4141
*/
4242
public function authorize(ServerRequestInterface $psrRequest, Request $request): Response|AuthorizationViewResponse
4343
{
44-
$authRequest = $this->withErrorHandling(fn () => $this->server->validateAuthorizationRequest($psrRequest));
44+
$authRequest = $this->withErrorHandling(
45+
fn () => $this->server->validateAuthorizationRequest($psrRequest),
46+
($psrRequest->getQueryParams()['response_type'] ?? null) === 'token'
47+
);
4548

4649
if ($this->guard->guest()) {
47-
if ($request->get('prompt') === 'none') {
48-
return $this->denyRequest($authRequest);
49-
}
50-
51-
$this->promptForLogin($request);
50+
$request->get('prompt') === 'none'
51+
? throw OAuthServerException::loginRequired($authRequest)
52+
: $this->promptForLogin($request);
5253
}
5354

5455
if ($request->get('prompt') === 'login' &&
@@ -62,17 +63,19 @@ public function authorize(ServerRequestInterface $psrRequest, Request $request):
6263

6364
$request->session()->forget('promptedForLogin');
6465

65-
$scopes = $this->parseScopes($authRequest);
6666
$user = $this->guard->user();
67+
$authRequest->setUser(new User($user->getAuthIdentifier()));
68+
69+
$scopes = $this->parseScopes($authRequest);
6770
$client = $this->clients->find($authRequest->getClient()->getIdentifier());
6871

6972
if ($request->get('prompt') !== 'consent' &&
7073
($client->skipsAuthorization($user, $scopes) || $this->hasGrantedScopes($user, $client, $scopes))) {
71-
return $this->approveRequest($authRequest, $user);
74+
return $this->approveRequest($authRequest);
7275
}
7376

7477
if ($request->get('prompt') === 'none') {
75-
return $this->denyRequest($authRequest, $user);
78+
throw OAuthServerException::consentRequired($authRequest);
7679
}
7780

7881
$request->session()->put('authToken', $authToken = Str::random());
@@ -121,44 +124,13 @@ protected function hasGrantedScopes(Authenticatable $user, Client $client, array
121124
/**
122125
* Approve the authorization request.
123126
*/
124-
protected function approveRequest(AuthorizationRequestInterface $authRequest, Authenticatable $user): Response
127+
protected function approveRequest(AuthorizationRequestInterface $authRequest): Response
125128
{
126-
$authRequest->setUser(new User($user->getAuthIdentifier()));
127-
128129
$authRequest->setAuthorizationApproved(true);
129130

130131
return $this->withErrorHandling(fn () => $this->convertResponse(
131132
$this->server->completeAuthorizationRequest($authRequest, new Psr7Response)
132-
));
133-
}
134-
135-
/**
136-
* Deny the authorization request.
137-
*/
138-
protected function denyRequest(AuthorizationRequestInterface $authRequest, ?Authenticatable $user = null): Response
139-
{
140-
if (is_null($user)) {
141-
$uri = $authRequest->getRedirectUri()
142-
?? (is_array($authRequest->getClient()->getRedirectUri())
143-
? $authRequest->getClient()->getRedirectUri()[0]
144-
: $authRequest->getClient()->getRedirectUri());
145-
146-
$separator = $authRequest->getGrantTypeId() === 'implicit' ? '#' : '?';
147-
148-
$uri = $uri.(str_contains($uri, $separator) ? '&' : $separator).'state='.$authRequest->getState();
149-
150-
return $this->withErrorHandling(function () use ($uri) {
151-
throw OAuthServerException::accessDenied('Unauthenticated', $uri);
152-
});
153-
}
154-
155-
$authRequest->setUser(new User($user->getAuthIdentifier()));
156-
157-
$authRequest->setAuthorizationApproved(false);
158-
159-
return $this->withErrorHandling(fn () => $this->convertResponse(
160-
$this->server->completeAuthorizationRequest($authRequest, new Psr7Response)
161-
));
133+
), $authRequest->getGrantTypeId() === 'implicit');
162134
}
163135

164136
/**

‎src/Http/Controllers/DenyAuthorizationController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,6 @@ public function deny(Request $request): Response
3030

3131
return $this->withErrorHandling(fn () => $this->convertResponse(
3232
$this->server->completeAuthorizationRequest($authRequest, new Psr7Response)
33-
));
33+
), $authRequest->getGrantTypeId() === 'implicit');
3434
}
3535
}

‎src/Http/Controllers/HandlesOAuthErrors.php‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,9 @@
55
use Closure;
66
use Laravel\Passport\Exceptions\OAuthServerException;
77
use League\OAuth2\Server\Exception\OAuthServerException as LeagueException;
8-
use Nyholm\Psr7\Response as Psr7Response;
98

109
trait HandlesOAuthErrors
1110
{
12-
use ConvertsPsrResponses;
13-
1411
/**
1512
* Perform the given callback with exception handling.
1613
*
@@ -21,15 +18,12 @@ trait HandlesOAuthErrors
2118
*
2219
* @throws \Laravel\Passport\Exceptions\OAuthServerException
2320
*/
24-
protected function withErrorHandling(Closure $callback)
21+
protected function withErrorHandling(Closure $callback, bool $useFragment = false)
2522
{
2623
try {
2724
return $callback();
2825
} catch (LeagueException $e) {
29-
throw new OAuthServerException(
30-
$e,
31-
$this->convertResponse($e->generateHttpResponse(new Psr7Response))
32-
);
26+
throw new OAuthServerException($e, $useFragment);
3327
}
3428
}
3529
}

‎src/Http/Controllers/RetrievesAuthRequestFromSession.php‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
use Exception;
66
use Illuminate\Http\Request;
7-
use Laravel\Passport\Bridge\User;
87
use Laravel\Passport\Exceptions\InvalidAuthTokenException;
98
use League\OAuth2\Server\RequestTypes\AuthorizationRequest;
109

@@ -18,18 +17,14 @@ trait RetrievesAuthRequestFromSession
1817
*/
1918
protected function getAuthRequestFromSession(Request $request): AuthorizationRequest
2019
{
21-
if ($request->isNotFilled('auth_token') || $request->session()->pull('authToken') !== $request->get('auth_token')) {
20+
if ($request->isNotFilled('auth_token') ||
21+
$request->session()->pull('authToken') !== $request->get('auth_token')) {
2222
$request->session()->forget(['authToken', 'authRequest']);
2323

2424
throw InvalidAuthTokenException::different();
2525
}
2626

27-
return tap($request->session()->pull('authRequest'), function ($authRequest) use ($request) {
28-
if (! $authRequest) {
29-
throw new Exception('Authorization request was not present in the session.');
30-
}
31-
32-
$authRequest->setUser(new User($request->user()->getAuthIdentifier()));
33-
});
27+
return $request->session()->pull('authRequest')
28+
?? throw new Exception('Authorization request was not present in the session.');
3429
}
3530
}

0 commit comments

Comments
 (0)