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

Allow client credentials secret to be hashed #1145

Merged
merged 12 commits into from
Dec 27, 2019
17 changes: 16 additions & 1 deletion src/Bridge/ClientRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Laravel\Passport\Bridge;

use Laravel\Passport\ClientRepository as ClientModelRepository;
use Laravel\Passport\Passport;
use League\OAuth2\Server\Repositories\ClientRepositoryInterface;

class ClientRepository implements ClientRepositoryInterface
Expand Down Expand Up @@ -55,7 +56,7 @@ public function validateClient($clientIdentifier, $clientSecret, $grantType)
return false;
}

return ! $record->confidential() || hash_equals($record->secret, (string) $clientSecret);
return ! $record->confidential() || $this->verifySecret((string) $clientSecret, $record->secret);
}

/**
Expand Down Expand Up @@ -84,4 +85,18 @@ protected function handlesGrant($record, $grantType)
return true;
}
}

/**
* @param string $clientSecret
* @param string $storedHash
* @return bool
*/
protected function verifySecret($clientSecret, $storedHash)
{
if (Passport::$useHashedClientSecrets) {
$clientSecret = hash('sha256', $clientSecret);
}

return hash_equals($storedHash, $clientSecret);
}
}
37 changes: 37 additions & 0 deletions src/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ class Client extends Model
'revoked' => 'bool',
];

/**
* The temporary non-hashed client secret.
*
* @var string|null
*/
protected $plainSecret;

/**
* Get the user that the client belongs to.
*
Expand Down Expand Up @@ -73,6 +80,36 @@ public function tokens()
return $this->hasMany(Passport::tokenModel(), 'client_id');
}

/**
* The temporary non-hashed client secret.
*
* If you're using hashed client secrets, this value will only be available
* once during the request the client was created. Afterwards, it cannot
* be retrieved or decrypted anymore.
*
* @return string|null
*/
public function getPlainSecretAttribute()
{
return $this->plainSecret;
}

/**
* @param string|null $value
*/
public function setSecretAttribute($value)
{
$this->plainSecret = $value;

if ($value === null || ! Passport::$useHashedClientSecrets) {
$this->attributes['secret'] = $value;

return;
}

$this->attributes['secret'] = hash('sha256', $value);
}

/**
* Determine if the client is a "first party" client.
*
Expand Down
17 changes: 17 additions & 0 deletions src/Passport.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@ class Passport
*/
public static $unserializesCookies = false;

/**
* @var bool
*/
public static $useHashedClientSecrets = false;

/**
* Indicates the scope should inherit its parent scope.
*
Expand Down Expand Up @@ -637,6 +642,18 @@ public static function ignoreMigrations()
return new static;
}

/**
* Configure Passport to hash client credential secrets.
*
* @return static
*/
public static function useHashedClientSecrets()
{
static::$useHashedClientSecrets = true;

return new static;
}

/**
* Instruct Passport to enable cookie serialization.
*
Expand Down
29 changes: 29 additions & 0 deletions tests/BridgeClientRepositoryHashedSecretsTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

namespace Laravel\Passport\Tests;

use Laravel\Passport\Bridge\ClientRepository as BridgeClientRepository;
use Laravel\Passport\ClientRepository;
use Laravel\Passport\Passport;
use Mockery as m;

class BridgeClientRepositoryHashedSecretsTest extends BridgeClientRepositoryTest
{
protected function setUp(): void
{
Passport::useHashedClientSecrets();

$clientModelRepository = m::mock(ClientRepository::class);
$clientModelRepository->shouldReceive('findActive')
->with(1)
->andReturn(new BridgeClientRepositoryHashedTestClientStub);

$this->clientModelRepository = $clientModelRepository;
$this->repository = new BridgeClientRepository($clientModelRepository);
}
}

class BridgeClientRepositoryHashedTestClientStub extends BridgeClientRepositoryTestClientStub
{
public $secret = '2bb80d537b1da3e38bd30361aa855686bde0eacd7162fef6a25fe97bf527a25b';
}
7 changes: 5 additions & 2 deletions tests/BridgeClientRepositoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Laravel\Passport\Bridge\Client;
use Laravel\Passport\Bridge\ClientRepository as BridgeClientRepository;
use Laravel\Passport\ClientRepository;
use Laravel\Passport\Passport;
use Mockery as m;
use PHPUnit\Framework\TestCase;

Expand All @@ -13,15 +14,17 @@ class BridgeClientRepositoryTest extends TestCase
/**
* @var \Laravel\Passport\ClientRepository
*/
private $clientModelRepository;
protected $clientModelRepository;

/**
* @var \Laravel\Passport\Bridge\ClientRepository
*/
private $repository;
protected $repository;

protected function setUp(): void
{
Passport::$useHashedClientSecrets = false;

$clientModelRepository = m::mock(ClientRepository::class);
$clientModelRepository->shouldReceive('findActive')
->with(1)
Expand Down