Skip to content
Snippets Groups Projects
Commit d87e427a authored by Neuber, Eugen Ramon's avatar Neuber, Eugen Ramon :speech_balloon:
Browse files

Add checks for expired and misused tokens. 

Add new payload field "action" which has to match HTTP method's intention (GET -> GETONE or GETALL, DELETE -> DELETEONE or DELETEALL).
parent 01ea251e
No related branches found
No related tags found
No related merge requests found
Pipeline #231176 failed
Stage: test
Hide whitespace changes
Inline Side-by-side
src/Controller/CreateFileDataAction.php
+ 17
− 1
View file @ d87e427a
......@@ -58,18 +58,33 @@ final class CreateFileDataAction extends BaseBlobController
// check if signed params aer equal to request params
if ($data['bucketID'] !== $bucketId) {
/* @noinspection ForgottenDebugOutputInspection */
dump($data['bucketID'], $bucketId);
throw ApiError::withDetails(Response::HTTP_FORBIDDEN, 'BucketId change forbidden', 'blob:bucketid-change-forbidden');
}
if ((int) $data['creationTime'] !== (int) $creationTime) {
/* @noinspection ForgottenDebugOutputInspection */
dump($data['creationTime'], $creationTime);
throw ApiError::withDetails(Response::HTTP_FORBIDDEN, 'Creation Time change forbidden', 'blob:creationtime-change-forbidden');
}
if ($data['prefix'] !== $prefix) {
/* @noinspection ForgottenDebugOutputInspection */
dump($data['prefix'], $prefix);
throw ApiError::withDetails(Response::HTTP_FORBIDDEN, 'Prefix change forbidden', 'blob:prefix-change-forbidden');
}
// TODO check if request is NOT too old
// check if request is expired
if ((int) $data['creationTime'] < $tooOld = strtotime('-5 min')) {
/* @noinspection ForgottenDebugOutputInspection */
dump((int) $data['creationTime'], $tooOld);
throw ApiError::withDetails(Response::HTTP_FORBIDDEN, 'Creation Time too old', 'blob:creationtime-too-old');
}
// check action/method
$method = $request->getMethod();
$action = $data['action'] ?? '';
echo " CreateFileDataAction::__invoke(): method=$method, action=$action\n";
if ($method !== 'POST' || $action !== 'CREATEONE') {
throw ApiError::withDetails(Response::HTTP_FORBIDDEN, 'Signature not suitable', 'blob:dataprovider-signature-not-suitable');
}
// Check retentionDuration & idleRetentionDuration valid durations
$fileData->setRetentionDuration($data['retentionDuration'] ?? '');
......@@ -96,6 +111,7 @@ final class CreateFileDataAction extends BaseBlobController
// check hash of file
if ($hash !== $data['fileHash']) {
/** @noinspection ForgottenDebugOutputInspection */
dump($data['fileHash'], $hash);
throw ApiError::withDetails(Response::HTTP_FORBIDDEN, 'File hash change forbidden', 'blob:file-hash-change-forbidden');
}
......
src/Controller/DeleteFileDatasByPrefix.php
+ 13
− 1
View file @ d87e427a
......@@ -58,7 +58,19 @@ class DeleteFileDatasByPrefix extends BaseBlobController
if ($data['prefix'] !== $prefix) {
throw ApiError::withDetails(Response::HTTP_FORBIDDEN, 'Prefix change forbidden', 'blob:prefix-change-forbidden');
}
// TODO check if request is NOT too old
// check if request is expired
if ((int) $data['creationTime'] < $tooOld = strtotime('-5 min')) {
/* @noinspection ForgottenDebugOutputInspection */
dump((int) $data['creationTime'], $tooOld);
throw ApiError::withDetails(Response::HTTP_FORBIDDEN, 'Creation Time too old', 'blob:creationtime-too-old');
}
// check action/method
$method = $request->getMethod();
$action = $data['action'] ?? '';
echo " DeleteFileDataByPrefix::__invoke(): method=$method, action=$action\n";
if (($method !== 'DELETE' || $action !== 'DELETEALL')) {
throw ApiError::withDetails(Response::HTTP_FORBIDDEN, 'Signature not suitable', 'blob:dataprovider-signature-not-suitable');
}
if (!$bucket->getService()) {
throw ApiError::withDetails(Response::HTTP_BAD_REQUEST, 'BucketService is not configured', 'blob:get-files-by-prefix-no-bucket-service');
......
src/DataProvider/FileDataDataProvider.php
+ 11
− 0
View file @ d87e427a
......@@ -9,6 +9,7 @@ use Dbp\Relay\BlobBundle\Helper\DenyAccessUnlessCheckSignature;
use Dbp\Relay\BlobBundle\Service\BlobService;
use Dbp\Relay\CoreBundle\DataProvider\AbstractDataProvider;
use Dbp\Relay\CoreBundle\Exception\ApiError;
use function PHPUnit\Framework\assertNotNull;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpFoundation\Response;
......@@ -169,5 +170,15 @@ class FileDataDataProvider extends AbstractDataProvider
dump((int) $data['creationTime'], $tooOld);
throw ApiError::withDetails(Response::HTTP_FORBIDDEN, 'Creation Time too old', 'blob:creationtime-too-old');
}
// check action/method
$method = $this->requestStack->getCurrentRequest()->getMethod();
$action = $data['action'] ?? '';
echo " FileDataProvider::checkSignature(): method=$method, action=$action\n";
if (($method === 'GET' && $action !== 'GETONE' && $action !== 'GETALL')
|| ($method === 'DELETE' && $action !== 'DELETEONE' && $action !== 'DELETEALL')
|| ($method === 'POST' && $action !== 'CREATEONE')
) {
throw ApiError::withDetails(Response::HTTP_FORBIDDEN, 'Signature not suitable', 'blob:dataprovider-signature-not-suitable');
}
}
}
tests/CurlGetTest.php
+ 294
− 12
View file @ d87e427a
......@@ -13,6 +13,7 @@ use Dbp\Relay\BlobBundle\Helper\PoliciesStruct;
use Dbp\Relay\BlobBundle\Service\BlobService;
use Dbp\Relay\BlobBundle\Service\ConfigurationService;
use Dbp\Relay\BlobBundle\Service\DatasystemProviderServiceInterface;
use Dbp\Relay\CoreBundle\Exception\ApiError;
use Dbp\Relay\CoreBundle\TestUtils\UserAuthTrait;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Tools\SchemaTool;
......@@ -137,6 +138,7 @@ class CurlGetTest extends ApiTestCase
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'action' => 'GETALL',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
......@@ -189,17 +191,17 @@ class CurlGetTest extends ApiTestCase
$bucket = $configService->getBuckets()[0];
$secret = $bucket->getPublicKey();
$bucketId = $bucket->getIdentifier();
$creationTime = date('U');
$prefix = 'playground';
$notifyEmail = 'eugen.neuber@tugraz.at';
$url = "/blob/files/?bucketID=$bucketId&prefix=$prefix&creationTime=$creationTime";
// =======================================================
// POST a file
// =======================================================
echo "POST file[0]\n";
$creationTime = date('U');
$prefix = 'playground';
$notifyEmail = 'eugen.neuber@tugraz.at';
$url = "/blob/files/?bucketID=$bucketId&prefix=$prefix&creationTime=$creationTime";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
......@@ -209,6 +211,7 @@ class CurlGetTest extends ApiTestCase
'notifyEmail' => $notifyEmail,
'retentionDuration' => $this->files[0]['retention'],
'additionalMetadata' => '',
'action' => 'CREATEONE',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
......@@ -250,6 +253,20 @@ class CurlGetTest extends ApiTestCase
// =======================================================
echo "GET all files (only 0)\n";
$creationTime = date('U');
$prefix = 'playground';
$notifyEmail = 'eugen.neuber@tugraz.at';
$url = "/blob/files/?bucketID=$bucketId&prefix=$prefix&creationTime=$creationTime";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'action' => 'GETALL',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
$options = [
'headers' => [
'HTTP_ACCEPT' => 'application/ld+json',
......@@ -284,6 +301,10 @@ class CurlGetTest extends ApiTestCase
// =======================================================
echo "POST file [1]\n";
$creationTime = date('U');
$prefix = 'playground';
$notifyEmail = 'eugen.neuber@tugraz.at';
$url = "/blob/files/?bucketID=$bucketId&prefix=$prefix&creationTime=$creationTime";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
......@@ -293,6 +314,7 @@ class CurlGetTest extends ApiTestCase
'notifyEmail' => $notifyEmail,
'retentionDuration' => $this->files[1]['retention'],
'additionalMetadata' => '',
'action' => 'CREATEONE',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
......@@ -334,6 +356,18 @@ class CurlGetTest extends ApiTestCase
// =======================================================
echo "GET all files (0 and 1)\n";
$creationTime = date('U');
$prefix = 'playground';
$url = "/blob/files/?bucketID=$bucketId&prefix=$prefix&creationTime=$creationTime";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'action' => 'GETALL',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
$options = [
'headers' => [
'HTTP_ACCEPT' => 'application/ld+json',
......@@ -379,6 +413,19 @@ class CurlGetTest extends ApiTestCase
// =======================================================
echo "DELETE all files (0 and 1)\n";
$creationTime = date('U');
$prefix = 'playground';
$url = "/blob/files/?bucketID=$bucketId&prefix=$prefix&creationTime=$creationTime";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'action' => 'DELETEALL',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
$requestDelete = Request::create($url, 'DELETE', [], [], [],
[
'HTTP_ACCEPT' => 'application/ld+json',
......@@ -407,6 +454,25 @@ class CurlGetTest extends ApiTestCase
// =======================================================
echo "GET all files (empty)\n";
$creationTime = date('U');
$prefix = 'playground';
$url = "/blob/files/?bucketID=$bucketId&prefix=$prefix&creationTime=$creationTime";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'action' => 'GETALL',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
$options = [
'headers' => [
'HTTP_ACCEPT' => 'application/ld+json',
'x-dbp-signature' => $token,
],
];
/** @var Response $response */
$response = $client->request('GET', $url, $options);
......@@ -417,7 +483,7 @@ class CurlGetTest extends ApiTestCase
$this->assertArrayHasKey('hydra:member', $data);
$this->assertCount(0, $data['hydra:member'], 'More files than expected');
} catch (\Throwable $e) {
echo $e->getTraceAsString()."\n";
// echo $e->getTraceAsString()."\n";
$this->fail($e->getMessage());
}
}
......@@ -461,6 +527,7 @@ class CurlGetTest extends ApiTestCase
'notifyEmail' => $notifyEmail,
'retentionDuration' => $this->files[0]['retention'],
'additionalMetadata' => '',
'action' => 'CREATEONE',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
......@@ -502,6 +569,15 @@ class CurlGetTest extends ApiTestCase
// =======================================================
echo "GET a file by id (0)\n";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'action' => 'GETONE',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
$options = [
'headers' => [
'HTTP_ACCEPT' => 'application/ld+json',
......@@ -524,6 +600,15 @@ class CurlGetTest extends ApiTestCase
// =======================================================
echo "DELETE a file by id\n";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'action' => 'DELETEONE',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
$options = [
'headers' => [
'Accept' => 'application/ld+json',
......@@ -557,6 +642,23 @@ class CurlGetTest extends ApiTestCase
// =======================================================
echo "GET all files\n";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'action' => 'GETALL',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
$options = [
'headers' => [
'Accept' => 'application/ld+json',
'HTTP_ACCEPT' => 'application/ld+json',
'x-dbp-signature' => $token,
],
];
/* @noinspection PhpInternalEntityUsedInspection */
$client->getKernelBrowser()->followRedirects();
......@@ -599,6 +701,7 @@ class CurlGetTest extends ApiTestCase
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'action' => 'GETALL',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
......@@ -626,7 +729,6 @@ class CurlGetTest extends ApiTestCase
}
}
/**
* Integration test: get and delete blob with unknown id.
*/
......@@ -644,19 +746,20 @@ class CurlGetTest extends ApiTestCase
$prefix = 'playground';
$uuid = Uuid::v4();
// =======================================================
// GET a file by unknown id
// =======================================================
echo "GET a file by unknown id\n";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'action' => 'GETONE',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
// =======================================================
// GET a file by unknown id
// =======================================================
echo "GET a file by unknown id\n";
$url = "/blob/files/{$uuid}?prefix=$prefix&bucketID=$bucketId&creationTime=$creationTime";
$options = [
'headers' => [
......@@ -678,6 +781,15 @@ class CurlGetTest extends ApiTestCase
// =======================================================
echo "DELETE a file by unknown id\n";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'action' => 'DELETEONE',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
$options = [
'headers' => [
'Accept' => 'application/ld+json',
......@@ -699,4 +811,174 @@ class CurlGetTest extends ApiTestCase
$this->fail($e->getMessage());
}
}
/**
* Integration test: create with wrong action.
*/
public function testPostWithWrongAction(): void
{
try {
$client = static::createClient();
/** @var BlobService $blobService */
$blobService = $client->getContainer()->get(BlobService::class);
$configService = $client->getContainer()->get(ConfigurationService::class);
$bucket = $configService->getBuckets()[0];
$secret = $bucket->getPublicKey();
$bucketId = $bucket->getIdentifier();
$creationTime = date('U');
$prefix = 'playground';
$notifyEmail = 'eugen.neuber@tugraz.at';
$url = "/blob/files/?bucketID=$bucketId&prefix=$prefix&creationTime=$creationTime";
// =======================================================
// POST a file
// =======================================================
echo "POST file 0 with wrong action\n";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'fileName' => $this->files[0]['name'],
'fileHash' => $this->files[0]['hash'],
'notifyEmail' => $notifyEmail,
'retentionDuration' => $this->files[0]['retention'],
'additionalMetadata' => '',
'action' => 'GETONE',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
$requestPost = Request::create($url, 'POST', [], [],
[
'file' => new UploadedFile($this->files[0]['path'], $this->files[0]['name'], $this->files[0]['mime']),
],
[
'HTTP_ACCEPT' => 'application/ld+json',
// 'x-dbp-signature' => $token,
'HTTP_X_DBP_SIGNATURE' => $token,
],
"HTTP_ACCEPT: application/ld+json\r\n"
."HTTP_X_DBP_SIGNATURE: $token\r\n\r\n"
.'file='.base64_encode($this->files[0]['content'])
."&fileName={$this->files[0]['name']}&prefix=$prefix&bucketID=$bucketId"
);
$c = new CreateFileDataAction($blobService);
$fileData = $c->__invoke($requestPost);
$this->fail(' FileData incorrectly saved: ' . $fileData->getIdentifier());
} catch (ApiError $e) {
$this->assertEquals($e->getStatusCode(), 403);
} catch (\Throwable $e) {
echo $e->getTraceAsString()."\n";
$this->fail($e->getMessage());
}
}
/**
* Integration test: get all with wrong action.
*/
public function testGetAllWithWrongAction(): void
{
try {
$client = static::createClient();
/** @var BlobService $blobService */
$blobService = $client->getContainer()->get(BlobService::class);
$configService = $client->getContainer()->get(ConfigurationService::class);
$bucket = $configService->getBuckets()[0];
$secret = $bucket->getPublicKey();
$bucketId = $bucket->getIdentifier();
$creationTime = date('U');
$prefix = 'playground';
// =======================================================
// GET all files
// =======================================================
echo "GET all files with wrong action\n";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'action' => 'DELETEONE',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
$url = "/blob/files/?bucketID=$bucketId&prefix=$prefix&creationTime=$creationTime";
$options = [
'headers' => [
'Accept' => 'application/ld+json',
'HTTP_ACCEPT' => 'application/ld+json',
'x-dbp-signature' => $token,
'HTTP_X_DBP_SIGNATURE' => $token,
],
];
/* @noinspection PhpInternalEntityUsedInspection */
$client->getKernelBrowser()->followRedirects();
/** @var Response $response */
$response = $client->request('GET', $url, $options);
$this->assertEquals(403, $response->getStatusCode());
} catch (\Throwable $e) {
echo $e->getTraceAsString()."\n";
$this->fail($e->getMessage());
}
}
/**
* Integration test: delete all with wrong action.
*/
public function testDeleteAllWithWrongAction(): void
{
try {
$client = static::createClient();
/** @var BlobService $blobService */
$blobService = $client->getContainer()->get(BlobService::class);
$configService = $client->getContainer()->get(ConfigurationService::class);
$bucket = $configService->getBuckets()[0];
$secret = $bucket->getPublicKey();
$bucketId = $bucket->getIdentifier();
$creationTime = date('U');
$prefix = 'playground';
// =======================================================
// DELETE all files
// =======================================================
echo "DELETE all files with wrong action\n";
$payload = [
'bucketID' => $bucketId,
'creationTime' => $creationTime,
'prefix' => $prefix,
'action' => 'GETONE',
];
$token = DenyAccessUnlessCheckSignature::create($secret, $payload);
$url = "/blob/files/?bucketID=$bucketId&prefix=$prefix&creationTime=$creationTime";
$requestDelete = Request::create($url, 'DELETE', [], [], [],
[
'HTTP_ACCEPT' => 'application/ld+json',
'HTTP_X_DBP_SIGNATURE' => $token,
],
"HTTP_ACCEPT: application/ld+json\r\n"
."HTTP_X_DBP_SIGNATURE: $token\r\n\r\n"
);
$d = new DeleteFileDatasByPrefix($blobService);
$d->__invoke($requestDelete);
$this->fail(' Delete by prefix incorrectly succeeded');
} catch (ApiError $e) {
$this->assertEquals($e->getStatusCode(), 403);
} catch (\Throwable $e) {
echo $e->getTraceAsString()."\n";
$this->fail($e->getMessage());
}
}
}
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment