Skip to content

Commit

Permalink
fix PKCE for token request (#520)
Browse files Browse the repository at this point in the history
* fix PKCE for token request (follow up on #518)

* adds test for PKCE token endpoint request

* make the linter happy
  • Loading branch information
aaronpk committed Feb 12, 2021
1 parent 78463b7 commit e7561cd
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 3 deletions.
4 changes: 2 additions & 2 deletions src/Two/AbstractProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ protected function getTokenFields($code)
];

if ($this->usesPKCE()) {
$field['code_verifier'] = $this->request->session()->pull('code_verifier');
$fields['code_verifier'] = $this->request->session()->pull('code_verifier');
}

return $fields;
Expand Down Expand Up @@ -483,7 +483,7 @@ protected function getCodeVerifier()
*/
protected function getCodeChallenge()
{
$hashed = hash('sha256', $this->request->session()->pull('code_verifier'), true);
$hashed = hash('sha256', $this->request->session()->get('code_verifier'), true);

return rtrim(strtr(base64_encode($hashed), '+/', '-_'), '=');
}
Expand Down
26 changes: 25 additions & 1 deletion tests/OAuthTwoTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Illuminate\Contracts\Session\Session;
use Illuminate\Http\RedirectResponse;
use Illuminate\Http\Request;
use Illuminate\Support\Str;
use Laravel\Socialite\Tests\Fixtures\FacebookTestProviderStub;
use Laravel\Socialite\Tests\Fixtures\OAuthTwoTestProviderStub;
use Laravel\Socialite\Tests\Fixtures\OAuthTwoWithPKCETestProviderStub;
Expand Down Expand Up @@ -79,7 +80,7 @@ public function testRedirectGeneratesTheProperIlluminateRedirectResponseWithPKCE
};

$session->shouldReceive('put')->twice()->withArgs($sessionPutClosure);
$session->shouldReceive('pull')->once()->with('code_verifier')->andReturnUsing($sessionPullClosure);
$session->shouldReceive('get')->once()->with('code_verifier')->andReturnUsing($sessionPullClosure);

$provider = new OAuthTwoWithPKCETestProviderStub($request, 'client_id', 'client_secret', 'redirect');
$response = $provider->redirect();
Expand All @@ -91,6 +92,29 @@ public function testRedirectGeneratesTheProperIlluminateRedirectResponseWithPKCE
$this->assertSame('http://auth.url?client_id=client_id&redirect_uri=redirect&scope=&response_type=code&state='.$state.'&code_challenge='.$codeChallenge.'&code_challenge_method=S256', $response->getTargetUrl());
}

public function testTokenRequestIncludesPKCECodeVerifier()
{
$request = Request::create('foo', 'GET', ['state' => str_repeat('A', 40), 'code' => 'code']);
$request->setLaravelSession($session = m::mock(Session::class));
$codeVerifier = Str::random(32);
$session->shouldReceive('pull')->once()->with('state')->andReturn(str_repeat('A', 40));
$session->shouldReceive('pull')->once()->with('code_verifier')->andReturn($codeVerifier);
$provider = new OAuthTwoWithPKCETestProviderStub($request, 'client_id', 'client_secret', 'redirect_uri');
$provider->http = m::mock(stdClass::class);
$provider->http->shouldReceive('post')->once()->with('http://token.url', [
'headers' => ['Accept' => 'application/json'], 'form_params' => ['grant_type' => 'authorization_code', 'client_id' => 'client_id', 'client_secret' => 'client_secret', 'code' => 'code', 'redirect_uri' => 'redirect_uri', 'code_verifier' => $codeVerifier],
])->andReturn($response = m::mock(stdClass::class));
$response->shouldReceive('getBody')->once()->andReturn('{ "access_token" : "access_token", "refresh_token" : "refresh_token", "expires_in" : 3600 }');
$user = $provider->user();

$this->assertInstanceOf(User::class, $user);
$this->assertSame('foo', $user->id);
$this->assertSame('access_token', $user->token);
$this->assertSame('refresh_token', $user->refreshToken);
$this->assertSame(3600, $user->expiresIn);
$this->assertSame($user->id, $provider->user()->id);
}

public function testUserReturnsAUserInstanceForTheAuthenticatedRequest()
{
$request = Request::create('foo', 'GET', ['state' => str_repeat('A', 40), 'code' => 'code']);
Expand Down

0 comments on commit e7561cd

Please sign in to comment.