Merge pull request #159 from copenhagentruckwash/fix-subuser-token-permission-bypass
Restrict replication endpoints to classic users
This commit is contained in:
@@ -16,7 +16,7 @@ class superuserReplicationRoute
|
||||
$this->get('/superuser/replication', function () {
|
||||
global $response;
|
||||
|
||||
$this->requirePermission('superuser_replication_view');
|
||||
$this->requireClassicSuperuserPermission('superuser_replication_view');
|
||||
$refresh = $this->toBool($this->getParameter('refresh'), false);
|
||||
$response->success((new replication_manager())->summary($refresh));
|
||||
}, [
|
||||
@@ -26,7 +26,7 @@ class superuserReplicationRoute
|
||||
$this->post('/superuser/replication/databases', function () {
|
||||
global $response;
|
||||
|
||||
$this->requirePermission('superuser_replication_manage');
|
||||
$this->requireClassicSuperuserPermission('superuser_replication_manage');
|
||||
$host = (new replication_manager())->addHost('database', $this->getParametersAsArray(), $this->actorUserId());
|
||||
$response->success($host, 201);
|
||||
}, [
|
||||
@@ -36,7 +36,7 @@ class superuserReplicationRoute
|
||||
$this->post('/superuser/replication/redis', function () {
|
||||
global $response;
|
||||
|
||||
$this->requirePermission('superuser_replication_manage');
|
||||
$this->requireClassicSuperuserPermission('superuser_replication_manage');
|
||||
$host = (new replication_manager())->addHost('redis', $this->getParametersAsArray(), $this->actorUserId());
|
||||
$response->success($host, 201);
|
||||
}, [
|
||||
@@ -46,7 +46,7 @@ class superuserReplicationRoute
|
||||
$this->post('/superuser/replication/minio', function () {
|
||||
global $response;
|
||||
|
||||
$this->requirePermission('superuser_replication_manage');
|
||||
$this->requireClassicSuperuserPermission('superuser_replication_manage');
|
||||
$host = (new replication_manager())->addHost('minio', $this->getParametersAsArray(), $this->actorUserId());
|
||||
$response->success($host, 201);
|
||||
}, [
|
||||
@@ -56,7 +56,7 @@ class superuserReplicationRoute
|
||||
$this->post('/superuser/replication/compose-template', function () {
|
||||
global $response;
|
||||
|
||||
$this->requirePermission('superuser_replication_manage');
|
||||
$this->requireClassicSuperuserPermission('superuser_replication_manage');
|
||||
$response->success(replication_manager::composeTemplate($this->getParametersAsArray()));
|
||||
}, [
|
||||
'superuser_replication_manage' => 'Generate Docker Compose templates for replication-ready database, Redis, and MinIO hosts',
|
||||
@@ -65,7 +65,7 @@ class superuserReplicationRoute
|
||||
$this->post('/superuser/replication/test-credentials', function () {
|
||||
global $response;
|
||||
|
||||
$this->requirePermission('superuser_replication_manage');
|
||||
$this->requireClassicSuperuserPermission('superuser_replication_manage');
|
||||
$parameters = $this->getParametersAsArray();
|
||||
$response->success((new replication_manager())->testCredentials(
|
||||
(string)($parameters['kind'] ?? ''),
|
||||
@@ -78,7 +78,7 @@ class superuserReplicationRoute
|
||||
$this->post('/superuser/replication/{kind}/{id}/test', function () {
|
||||
global $response;
|
||||
|
||||
$this->requirePermission('superuser_replication_manage');
|
||||
$this->requireClassicSuperuserPermission('superuser_replication_manage');
|
||||
$response->success((new replication_manager())->testHost(
|
||||
(string)$this->fromRoute('kind'),
|
||||
$this->routeId(),
|
||||
@@ -91,7 +91,7 @@ class superuserReplicationRoute
|
||||
$this->post('/superuser/replication/{kind}/{id}/provision', function () {
|
||||
global $response;
|
||||
|
||||
$this->requirePermission('superuser_replication_manage');
|
||||
$this->requireClassicSuperuserPermission('superuser_replication_manage');
|
||||
try {
|
||||
$result = (new replication_manager())->provisionHost(
|
||||
(string)$this->fromRoute('kind'),
|
||||
@@ -113,7 +113,7 @@ class superuserReplicationRoute
|
||||
$this->post('/superuser/replication/{kind}/{id}/promote', function () {
|
||||
global $response;
|
||||
|
||||
$this->requirePermission('superuser_replication_promote');
|
||||
$this->requireClassicSuperuserPermission('superuser_replication_promote');
|
||||
try {
|
||||
$response->success((new replication_manager())->promoteHost(
|
||||
(string)$this->fromRoute('kind'),
|
||||
@@ -130,7 +130,7 @@ class superuserReplicationRoute
|
||||
$this->patch('/superuser/replication/{kind}/{id}', function () {
|
||||
global $response;
|
||||
|
||||
$this->requirePermission('superuser_replication_manage');
|
||||
$this->requireClassicSuperuserPermission('superuser_replication_manage');
|
||||
try {
|
||||
$response->success((new replication_manager())->renameHost(
|
||||
(string)$this->fromRoute('kind'),
|
||||
@@ -148,7 +148,7 @@ class superuserReplicationRoute
|
||||
$this->delete('/superuser/replication/{kind}/{id}', function () {
|
||||
global $response;
|
||||
|
||||
$this->requirePermission('superuser_replication_remove');
|
||||
$this->requireClassicSuperuserPermission('superuser_replication_remove');
|
||||
try {
|
||||
$response->success((new replication_manager())->removeHost(
|
||||
(string)$this->fromRoute('kind'),
|
||||
@@ -163,6 +163,23 @@ class superuserReplicationRoute
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Replication controls alter infrastructure state and must only be used by
|
||||
* a classic superuser session. Subuser bearer tokens can carry a delegated
|
||||
* customer context via X-Customer-Number, so do not allow them to fall back
|
||||
* to plain string user permission checks for these routes.
|
||||
*/
|
||||
private function requireClassicSuperuserPermission(string $permission): bool
|
||||
{
|
||||
global $response;
|
||||
|
||||
if ((new authentication())->get_subuser() !== false) {
|
||||
$response->error('Subuser sessions cannot manage replication.', 403);
|
||||
}
|
||||
|
||||
return $this->requirePermission($permission);
|
||||
}
|
||||
|
||||
private function routeId(): int
|
||||
{
|
||||
$id = (int)$this->fromRoute('id');
|
||||
|
||||
@@ -14,10 +14,10 @@ it('registers superuser replication endpoints and permissions', function (): voi
|
||||
expect($content)->toContain('/superuser/replication/{kind}/{id}/provision');
|
||||
expect($content)->toContain('/superuser/replication/{kind}/{id}/promote');
|
||||
expect($content)->toContain("\$this->patch('/superuser/replication/{kind}/{id}'");
|
||||
expect($content)->toContain("requirePermission('superuser_replication_view')");
|
||||
expect($content)->toContain("requirePermission('superuser_replication_manage')");
|
||||
expect($content)->toContain("requirePermission('superuser_replication_promote')");
|
||||
expect($content)->toContain("requirePermission('superuser_replication_remove')");
|
||||
expect($content)->toContain("requireClassicSuperuserPermission('superuser_replication_view')");
|
||||
expect($content)->toContain("requireClassicSuperuserPermission('superuser_replication_manage')");
|
||||
expect($content)->toContain("requireClassicSuperuserPermission('superuser_replication_promote')");
|
||||
expect($content)->toContain("requireClassicSuperuserPermission('superuser_replication_remove')");
|
||||
});
|
||||
|
||||
it('documents replication management in openapi', function (): void {
|
||||
@@ -36,3 +36,16 @@ it('documents replication management in openapi', function (): void {
|
||||
expect($content)->toContain('SuperuserReplicationHostRenameRequest');
|
||||
expect($content)->toContain('SuperuserReplicationComposeTemplateRequest');
|
||||
});
|
||||
|
||||
it('rejects subuser sessions before checking replication permissions', function (): void {
|
||||
$content = file_get_contents(app_path('routes/superuserReplicationRoute.php'));
|
||||
|
||||
expect($content)->not->toBeFalse();
|
||||
expect($content)->toContain('private function requireClassicSuperuserPermission(string $permission): bool');
|
||||
expect($content)->toContain('get_subuser() !== false');
|
||||
expect($content)->toContain("Subuser sessions cannot manage replication.");
|
||||
expect($content)->toContain("\$response->error('Subuser sessions cannot manage replication.', 403);");
|
||||
expect($content)->toContain('return $this->requirePermission($permission);');
|
||||
expect(preg_match_all("/requireClassicSuperuserPermission\\('superuser_replication_/", $content))->toBe(10);
|
||||
expect($content)->not->toContain("requirePermission('superuser_replication_");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user