Skip to content

Commit f4f85e3

Browse files
hafezdivandarihafezdivandari
authored
[13.x] Fix session JSON serialization (#1905)
* fix json serialization * add tests * fix tests
1 parent 27ba0a8 commit f4f85e3

9 files changed

+104
-40
lines changed

‎src/Http/Controllers/AuthorizationController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public function authorize(
8282
}
8383

8484
$request->session()->put('authToken', $authToken = Str::random());
85-
$request->session()->put('authRequest', $authRequest);
85+
$request->session()->put('authRequest', serialize($authRequest));
8686

8787
return $viewResponse->withParameters([
8888
'client' => $client,

‎src/Http/Controllers/DeviceAuthorizationController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public function __invoke(
5555
$client = $this->clients->find($deviceCode->getClient()->getIdentifier());
5656

5757
$request->session()->put('authToken', $authToken = Str::random());
58-
$request->session()->put('deviceCode', $deviceCode);
58+
$request->session()->put('deviceCode', serialize($deviceCode));
5959

6060
return $viewResponse->withParameters([
6161
'client' => $client,

‎src/Http/Controllers/RetrievesAuthRequestFromSession.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,14 @@ protected function getAuthRequestFromSession(Request $request): AuthorizationReq
2424
throw InvalidAuthTokenException::different();
2525
}
2626

27-
return $request->session()->pull('authRequest')
27+
$authRequest = $request->session()->pull('authRequest')
2828
?? throw new Exception('Authorization request was not present in the session.');
29+
30+
return unserialize($authRequest, ['allowed_classes' => [
31+
\League\OAuth2\Server\RequestTypes\AuthorizationRequest::class,
32+
\Laravel\Passport\Bridge\Client::class,
33+
\Laravel\Passport\Bridge\Scope::class,
34+
\Laravel\Passport\Bridge\User::class,
35+
]]);
2936
}
3037
}

‎src/Http/Controllers/RetrievesDeviceCodeFromSession.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,14 @@ protected function getDeviceCodeFromSession(Request $request): DeviceCodeEntityI
2424
throw InvalidAuthTokenException::different();
2525
}
2626

27-
return $request->session()->pull('deviceCode')
27+
$deviceCode = $request->session()->pull('deviceCode')
2828
?? throw new Exception('Device code was not present in the session.');
29+
30+
return unserialize($deviceCode, ['allowed_classes' => [
31+
\Laravel\Passport\Bridge\DeviceCode::class,
32+
\Laravel\Passport\Bridge\Client::class,
33+
\Laravel\Passport\Bridge\Scope::class,
34+
\DateTimeImmutable::class,
35+
]]);
2936
}
3037
}

‎tests/Feature/AuthorizationCodeGrantTest.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Illuminate\Support\Str;
88
use Laravel\Passport\Database\Factories\ClientFactory;
99
use Laravel\Passport\Passport;
10+
use Orchestra\Testbench\Attributes\WithConfig;
1011
use Orchestra\Testbench\Concerns\WithLaravelMigrations;
1112
use Workbench\Database\Factories\UserFactory;
1213

@@ -254,6 +255,35 @@ public function testPromptConsentForNewScope()
254255
$this->assertSame(collect(Passport::scopesFor(['create', 'update']))->toArray(), $json['scopes']);
255256
}
256257

258+
#[WithConfig('session.serialization', 'json')]
259+
public function testAuthRequestSerialization()
260+
{
261+
$client = ClientFactory::new()->create();
262+
263+
$query = http_build_query([
264+
'client_id' => $client->getKey(),
265+
'redirect_uri' => $client->redirect_uris[0],
266+
'response_type' => 'code',
267+
'scope' => 'create read',
268+
'state' => Str::random(40),
269+
]);
270+
271+
$this->actingAs(UserFactory::new()->create(), 'web');
272+
273+
$json = $this->get('/oauth/authorize?'.$query)
274+
->assertOk()
275+
->assertSessionHas('authRequest')
276+
->assertSessionHas('authToken')
277+
->json();
278+
279+
$this->assertEqualsCanonicalizing(['client', 'user', 'scopes', 'request', 'authToken'], array_keys($json));
280+
$this->assertSame(collect(Passport::scopesFor(['create', 'read']))->toArray(), $json['scopes']);
281+
282+
$this->post('/oauth/authorize', ['auth_token' => $json['authToken']])
283+
->assertRedirect()
284+
->assertSessionMissing(['authRequest', 'authToken']);
285+
}
286+
257287
public function testValidateAuthorizationRequest()
258288
{
259289
$client = ClientFactory::new()->create();

‎tests/Feature/DeviceAuthorizationGrantTest.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Illuminate\Support\Str;
88
use Laravel\Passport\Database\Factories\ClientFactory;
99
use Laravel\Passport\Passport;
10+
use Orchestra\Testbench\Attributes\WithConfig;
1011
use Orchestra\Testbench\Concerns\WithLaravelMigrations;
1112
use Workbench\Database\Factories\UserFactory;
1213

@@ -226,6 +227,33 @@ public function testDenyAuthorization()
226227
$this->assertSame('access_denied', $json['error']);
227228
}
228229

230+
#[WithConfig('session.serialization', 'json')]
231+
public function testDeviceCodeSerialization()
232+
{
233+
$client = ClientFactory::new()->asDeviceCodeClient()->create();
234+
235+
['user_code' => $userCode] = $this->post('/oauth/device/code', [
236+
'client_id' => $client->getKey(),
237+
'scope' => 'create read',
238+
])->assertOk()->json();
239+
240+
$user = UserFactory::new()->create();
241+
$this->actingAs($user, 'web');
242+
243+
$json = $this->get('/oauth/device/authorize?user_code='.$userCode)
244+
->assertOk()
245+
->assertSessionHas('deviceCode')
246+
->assertSessionHas('authToken')
247+
->json();
248+
$this->assertEqualsCanonicalizing(['client', 'user', 'scopes', 'request', 'authToken'], array_keys($json));
249+
$this->assertSame(collect(Passport::scopesFor(['create', 'read']))->toArray(), $json['scopes']);
250+
251+
$this->post('/oauth/device/authorize', ['auth_token' => $json['authToken']])
252+
->assertRedirectToRoute('passport.device')
253+
->assertSessionHas('status', 'authorization-approved')
254+
->assertSessionMissing(['deviceCode', 'authToken']);
255+
}
256+
229257
public function testPublicClient()
230258
{
231259
$client = ClientFactory::new()->asDeviceCodeClient()->asPublic()->create();

‎tests/Unit/ApproveAuthorizationControllerTest.php‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,22 +28,25 @@ public function test_complete_authorization_request()
2828
$request->shouldReceive('isNotFilled')->with('auth_token')->andReturn(false);
2929
$request->shouldReceive('input')->with('auth_token')->andReturn('foo');
3030

31+
$authRequest = new AuthorizationRequest;
32+
$authRequest->setGrantTypeId('authorization_code');
33+
3134
$session->shouldReceive('pull')->once()->with('authToken')->andReturn('foo');
3235
$session->shouldReceive('pull')
3336
->once()
3437
->with('authRequest')
35-
->andReturn($authRequest = m::mock(AuthorizationRequest::class));
38+
->andReturn(serialize($authRequest));
3639

3740
$request->shouldReceive('user')->andReturn(new ApproveAuthorizationControllerFakeUser);
3841

39-
$authRequest->shouldReceive('getGrantTypeId')->once()->andReturn('authorization_code');
40-
$authRequest->shouldReceive('setAuthorizationApproved')->once()->with(true);
41-
4242
$psrResponse = (new PsrHttpFactory)->createResponse(new Response);
4343
$psrResponse->getBody()->write('response');
4444

4545
$server->shouldReceive('completeAuthorizationRequest')
46-
->with($authRequest, m::type(ResponseInterface::class))
46+
->with(
47+
m::on(fn (AuthorizationRequest $request) => $request->isAuthorizationApproved()),
48+
m::type(ResponseInterface::class)
49+
)
4750
->andReturn($psrResponse);
4851

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

‎tests/Unit/AuthorizationControllerTest.php‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
use League\OAuth2\Server\AuthorizationServer;
1717
use League\OAuth2\Server\Exception\OAuthServerException as LeagueException;
1818
use League\OAuth2\Server\RequestTypes\AuthorizationRequest;
19-
use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
2019
use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
2120
use Mockery as m;
2221
use PHPUnit\Framework\TestCase;
@@ -39,24 +38,24 @@ public function test_authorization_view_is_presented()
3938
$response = m::mock(AuthorizationViewResponse::class);
4039
$guard = m::mock(StatefulGuard::class);
4140

41+
$authRequest = new AuthorizationRequest;
42+
$authRequest->setClient(new \Laravel\Passport\Bridge\Client('1', 'Test Client'));
43+
$authRequest->setScopes([new Scope('scope-1')]);
44+
4245
$guard->shouldReceive('guest')->andReturn(false);
4346
$guard->shouldReceive('user')->andReturn($user = m::mock(Authenticatable::class));
44-
$server->shouldReceive('validateAuthorizationRequest')->andReturn($authRequest = m::mock(AuthorizationRequestInterface::class));
47+
$server->shouldReceive('validateAuthorizationRequest')->andReturn($authRequest);
4548

4649
$psrRequest = m::mock(ServerRequestInterface::class);
4750
$psrRequest->shouldReceive('getQueryParams')->andReturn([]);
4851

4952
$request = m::mock(Request::class);
5053
$request->shouldReceive('session')->andReturn($session = m::mock());
5154
$session->shouldReceive('put')->withSomeOfArgs('authToken');
52-
$session->shouldReceive('put')->with('authRequest', $authRequest);
55+
$session->shouldReceive('put')->with('authRequest', m::on(fn ($value) => is_string($value)))->once();
5356
$session->shouldReceive('forget')->with('promptedForLogin')->once();
5457
$request->shouldReceive('input')->with('prompt')->andReturn(null);
5558

56-
$authRequest->shouldReceive('getClient->getIdentifier')->andReturn(1);
57-
$authRequest->shouldReceive('getScopes')->andReturn([new Scope('scope-1')]);
58-
$authRequest->shouldReceive('setUser')->once();
59-
6059
$clients = m::mock(ClientRepository::class);
6160
$clients->shouldReceive('find')->with(1)->andReturn($client = m::mock(Client::class));
6261
$client->shouldReceive('skipsAuthorization')->andReturn(false);
@@ -210,11 +209,14 @@ public function test_authorization_view_is_presented_if_request_has_prompt_equal
210209
$response = m::mock(AuthorizationViewResponse::class);
211210
$guard = m::mock(StatefulGuard::class);
212211

212+
$authRequest = new AuthorizationRequest;
213+
$authRequest->setClient(new \Laravel\Passport\Bridge\Client('1', 'Test Client'));
214+
$authRequest->setScopes([new Scope('scope-1')]);
215+
213216
$guard->shouldReceive('guest')->andReturn(false);
214217
$guard->shouldReceive('user')->andReturn($user = m::mock(Authenticatable::class));
215218
$user->shouldReceive('getAuthIdentifier')->andReturn(1);
216-
$server->shouldReceive('validateAuthorizationRequest')
217-
->andReturn($authRequest = m::mock(AuthorizationRequest::class));
219+
$server->shouldReceive('validateAuthorizationRequest')->andReturn($authRequest);
218220

219221
$psrRequest = m::mock(ServerRequestInterface::class);
220222
$psrRequest->shouldReceive('getQueryParams')->andReturn([]);
@@ -224,14 +226,10 @@ public function test_authorization_view_is_presented_if_request_has_prompt_equal
224226
$request = m::mock(Request::class);
225227
$request->shouldReceive('session')->andReturn($session = m::mock());
226228
$session->shouldReceive('put')->withSomeOfArgs('authToken');
227-
$session->shouldReceive('put')->with('authRequest', $authRequest);
229+
$session->shouldReceive('put')->with('authRequest', m::on(fn ($value) => is_string($value)))->once();
228230
$session->shouldReceive('forget')->with('promptedForLogin')->once();
229231
$request->shouldReceive('input')->with('prompt')->andReturn('consent');
230232

231-
$authRequest->shouldReceive('getClient->getIdentifier')->once()->andReturn(1);
232-
$authRequest->shouldReceive('getScopes')->once()->andReturn([new Scope('scope-1')]);
233-
$authRequest->shouldReceive('setUser')->once()->andReturnNull();
234-
235233
$clients = m::mock(ClientRepository::class);
236234
$clients->shouldReceive('find')->with(1)->andReturn($client = m::mock(Client::class));
237235
$client->shouldReceive('skipsAuthorization')->andReturn(false);

‎tests/Unit/DenyAuthorizationControllerTest.php‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,22 +30,23 @@ public function test_authorization_can_be_denied()
3030
$request->shouldReceive('isNotFilled')->with('auth_token')->andReturn(false);
3131
$request->shouldReceive('input')->with('auth_token')->andReturn('foo');
3232

33+
$authRequest = new AuthorizationRequest;
34+
$authRequest->setGrantTypeId('authorization_code');
35+
3336
$session->shouldReceive('pull')->once()->with('authToken')->andReturn('foo');
3437
$session->shouldReceive('pull')
3538
->once()
3639
->with('authRequest')
37-
->andReturn($authRequest = m::mock(
38-
AuthorizationRequest::class
39-
));
40-
41-
$authRequest->shouldReceive('getGrantTypeId')->once()->andReturn('authorization_code');
42-
$authRequest->shouldReceive('setAuthorizationApproved')->once()->with(false);
40+
->andReturn(serialize($authRequest));
4341

4442
$psrResponse = m::mock(ResponseInterface::class);
4543
app()->instance(ResponseInterface::class, (new PsrHttpFactory)->createResponse(new Response));
4644

4745
$server->shouldReceive('completeAuthorizationRequest')
48-
->with($authRequest, m::type(ResponseInterface::class))
46+
->with(
47+
m::on(fn (AuthorizationRequest $request) => ! $request->isAuthorizationApproved()),
48+
m::type(ResponseInterface::class)
49+
)
4950
->andReturnUsing(function () {
5051
throw new \League\OAuth2\Server\Exception\OAuthServerException('', 0, '');
5152
});
@@ -80,13 +81,3 @@ public function test_auth_request_should_exist()
8081
$controller->deny($request, $psrResponse);
8182
}
8283
}
83-
84-
class DenyAuthorizationControllerFakeUser
85-
{
86-
public $id = 1;
87-
88-
public function getAuthIdentifier()
89-
{
90-
return $this->id;
91-
}
92-
}

0 commit comments

Comments
 (0)
⚡