mirror of
https://github.com/shlinkio/shlink.git
synced 2024-11-24 09:50:17 -06:00
Hash existing API keys, and do checks against the hash
This commit is contained in:
parent
9f6975119e
commit
1b9c8377ae
45
module/Core/migrations/Version20241105215309.php
Normal file
45
module/Core/migrations/Version20241105215309.php
Normal file
@ -0,0 +1,45 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace ShlinkMigrations;
|
||||
|
||||
use Doctrine\DBAL\Platforms\MySQLPlatform;
|
||||
use Doctrine\DBAL\Schema\Schema;
|
||||
use Doctrine\Migrations\AbstractMigration;
|
||||
|
||||
use function hash;
|
||||
|
||||
/**
|
||||
* Hash API keys as SHA256
|
||||
*/
|
||||
final class Version20241105215309 extends AbstractMigration
|
||||
{
|
||||
public function up(Schema $schema): void
|
||||
{
|
||||
$keyColumnName = $this->connection->quoteIdentifier('key');
|
||||
|
||||
$qb = $this->connection->createQueryBuilder();
|
||||
$qb->select($keyColumnName)
|
||||
->from('api_keys');
|
||||
$result = $qb->executeQuery();
|
||||
|
||||
$updateQb = $this->connection->createQueryBuilder();
|
||||
$updateQb
|
||||
->update('api_keys')
|
||||
->set($keyColumnName, ':encryptedKey')
|
||||
->where($updateQb->expr()->eq($keyColumnName, ':plainTextKey'));
|
||||
|
||||
while ($key = $result->fetchOne()) {
|
||||
$updateQb->setParameters([
|
||||
'encryptedKey' => hash('sha256', $key),
|
||||
'plainTextKey' => $key,
|
||||
])->executeStatement();
|
||||
}
|
||||
}
|
||||
|
||||
public function isTransactional(): bool
|
||||
{
|
||||
return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform);
|
||||
}
|
||||
}
|
@ -44,8 +44,7 @@ class ApiKey extends AbstractEntity
|
||||
*/
|
||||
public static function fromMeta(ApiKeyMeta $meta): self
|
||||
{
|
||||
// $apiKey = new self(self::hashKey($meta->key), $meta->name, $meta->expirationDate);
|
||||
$apiKey = new self($meta->key, $meta->name, $meta->expirationDate);
|
||||
$apiKey = new self(self::hashKey($meta->key), $meta->name, $meta->expirationDate);
|
||||
foreach ($meta->roleDefinitions as $roleDefinition) {
|
||||
$apiKey->registerRole($roleDefinition);
|
||||
}
|
||||
|
@ -67,8 +67,7 @@ readonly class ApiKeyService implements ApiKeyServiceInterface
|
||||
private function getByKey(string $key): ApiKey|null
|
||||
{
|
||||
return $this->em->getRepository(ApiKey::class)->findOneBy([
|
||||
// 'key' => ApiKey::hashKey($key),
|
||||
'key' => $key,
|
||||
'key' => ApiKey::hashKey($key),
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
@ -24,9 +24,9 @@ class ApiKeyRepositoryTest extends DatabaseTestCase
|
||||
self::assertCount(0, $this->repo->findAll());
|
||||
self::assertNotNull($this->repo->createInitialApiKey('initial_value'));
|
||||
self::assertCount(1, $this->repo->findAll());
|
||||
self::assertCount(1, $this->repo->findBy(['key' => 'initial_value']));
|
||||
self::assertCount(1, $this->repo->findBy(['key' => ApiKey::hashKey('initial_value')]));
|
||||
self::assertNull($this->repo->createInitialApiKey('another_one'));
|
||||
self::assertCount(1, $this->repo->findAll());
|
||||
self::assertCount(0, $this->repo->findBy(['key' => 'another_one']));
|
||||
self::assertCount(0, $this->repo->findBy(['key' => ApiKey::hashKey('another_one')]));
|
||||
}
|
||||
}
|
||||
|
@ -74,7 +74,9 @@ class ApiKeyServiceTest extends TestCase
|
||||
#[Test, DataProvider('provideInvalidApiKeys')]
|
||||
public function checkReturnsFalseForInvalidApiKeys(ApiKey|null $invalidKey): void
|
||||
{
|
||||
$this->repo->expects($this->once())->method('findOneBy')->with(['key' => '12345'])->willReturn($invalidKey);
|
||||
$this->repo->expects($this->once())->method('findOneBy')->with(['key' => ApiKey::hashKey('12345')])->willReturn(
|
||||
$invalidKey,
|
||||
);
|
||||
$this->em->method('getRepository')->with(ApiKey::class)->willReturn($this->repo);
|
||||
|
||||
$result = $this->service->check('12345');
|
||||
@ -97,7 +99,9 @@ class ApiKeyServiceTest extends TestCase
|
||||
{
|
||||
$apiKey = ApiKey::create();
|
||||
|
||||
$this->repo->expects($this->once())->method('findOneBy')->with(['key' => '12345'])->willReturn($apiKey);
|
||||
$this->repo->expects($this->once())->method('findOneBy')->with(['key' => ApiKey::hashKey('12345')])->willReturn(
|
||||
$apiKey,
|
||||
);
|
||||
$this->em->method('getRepository')->with(ApiKey::class)->willReturn($this->repo);
|
||||
|
||||
$result = $this->service->check('12345');
|
||||
@ -109,7 +113,9 @@ class ApiKeyServiceTest extends TestCase
|
||||
#[Test]
|
||||
public function disableThrowsExceptionWhenNoApiKeyIsFound(): void
|
||||
{
|
||||
$this->repo->expects($this->once())->method('findOneBy')->with(['key' => '12345'])->willReturn(null);
|
||||
$this->repo->expects($this->once())->method('findOneBy')->with(['key' => ApiKey::hashKey('12345')])->willReturn(
|
||||
null,
|
||||
);
|
||||
$this->em->method('getRepository')->with(ApiKey::class)->willReturn($this->repo);
|
||||
|
||||
$this->expectException(InvalidArgumentException::class);
|
||||
@ -121,7 +127,9 @@ class ApiKeyServiceTest extends TestCase
|
||||
public function disableReturnsDisabledApiKeyWhenFound(): void
|
||||
{
|
||||
$key = ApiKey::create();
|
||||
$this->repo->expects($this->once())->method('findOneBy')->with(['key' => '12345'])->willReturn($key);
|
||||
$this->repo->expects($this->once())->method('findOneBy')->with(['key' => ApiKey::hashKey('12345')])->willReturn(
|
||||
$key,
|
||||
);
|
||||
$this->em->method('getRepository')->with(ApiKey::class)->willReturn($this->repo);
|
||||
$this->em->expects($this->once())->method('flush');
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user