[FIX][SEC] Connettori: autorizzazione per-org + secret allowlist (findings review multi-agente)
Due vulnerabilità trovate dalla review indipendente:
1. connectorOrgGuard usava users.role (GLOBALE) invece del ruolo per-org -> la feature
era ROTTA per gli utenti reali (org_admin reale ha users.role='employee' -> 403 sulla
propria org). Ora ancora l'autorizzazione al parametro di ROUTE {id} e legge
user_organizations.role. Verificato E2E: globale=employee + per-org=org_admin -> 200;
non-membro su altra org -> 403 (no IDOR via header X-Organization-Id).
2. secret-strip era una denylist case-sensitive/non-ricorsiva aggirabile (Client_Secret,
apiKey, connection_string, segreti annidati). Sostituita con ALLOWLIST ricorsiva
(sanitizeConnectorConfig): solo campi non sensibili noti, valori forzati a stringa+troncati.
Verificato E2E: input con 11 varianti di segreti -> DB contiene solo {account_id, region}.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
ff2b5eaeeb
commit
2fd4b7ff26
@ -416,26 +416,46 @@ class OrganizationController extends BaseController
|
||||
|
||||
private const CONNECTOR_TYPES = ['m365', 'google', 'aws', 'azure', 'idp', 'edr', 'siem', 'ticketing'];
|
||||
|
||||
/**
|
||||
* Autorizza la gestione dei connettori dell'org $orgId (parametro di ROUTE).
|
||||
* L'autorizzazione è ancorata a $orgId, NON all'header X-Organization-Id,
|
||||
* e usa il ruolo PER-ORG (user_organizations.role), non users.role globale.
|
||||
*/
|
||||
private function connectorOrgGuard(int $orgId): void
|
||||
{
|
||||
$this->requireOrgAccess();
|
||||
$role = $this->currentUser['role'] ?? '';
|
||||
if ($role === 'super_admin') {
|
||||
$this->requireAuth();
|
||||
$user = $this->currentUser;
|
||||
|
||||
// super_admin: accesso pieno
|
||||
if (($user['role'] ?? '') === 'super_admin') {
|
||||
return;
|
||||
}
|
||||
if ($orgId !== $this->getCurrentOrgId()) {
|
||||
$firmId = $this->currentUser['consulting_firm_id'] ?? null;
|
||||
$owned = $firmId ? Database::fetchOne(
|
||||
|
||||
// Ruolo dell'utente NELL'org target (route {id})
|
||||
$membership = Database::fetchOne(
|
||||
'SELECT role FROM user_organizations WHERE user_id = ? AND organization_id = ?',
|
||||
[$user['id'], $orgId]
|
||||
);
|
||||
$manageRoles = ['org_admin', 'compliance_manager'];
|
||||
if ($membership && in_array($membership['role'], $manageRoles, true)) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Consulente dello studio che gestisce l'org (org.consulting_firm_id == user.consulting_firm_id)
|
||||
$firmId = $user['consulting_firm_id'] ?? null;
|
||||
if ($firmId) {
|
||||
$owned = Database::fetchOne(
|
||||
'SELECT id FROM organizations WHERE id = ? AND consulting_firm_id = ?',
|
||||
[$orgId, $firmId]
|
||||
) : null;
|
||||
if (!$owned) {
|
||||
$this->jsonError('Accesso negato a questa organizzazione', 403, 'ORG_FORBIDDEN');
|
||||
);
|
||||
// consentito se l'utente è consulente/admin del firm (membership consultant sull'org o ruolo di gestione nel firm)
|
||||
$isFirmManager = ($membership && in_array($membership['role'], ['consultant', 'org_admin', 'compliance_manager'], true));
|
||||
if ($owned && $isFirmManager) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
if (!in_array($role, ['org_admin', 'compliance_manager'], true)) {
|
||||
$this->jsonError('Ruolo non autorizzato a gestire i connettori', 403, 'ROLE_FORBIDDEN');
|
||||
}
|
||||
|
||||
$this->jsonError('Non autorizzato a gestire i connettori di questa organizzazione', 403, 'CONNECTOR_FORBIDDEN');
|
||||
}
|
||||
|
||||
/** GET /api/organizations/{id}/connectors */
|
||||
@ -472,9 +492,10 @@ class OrganizationController extends BaseController
|
||||
$config = $this->getParam('config');
|
||||
if (is_string($config)) { $config = json_decode($config, true); }
|
||||
if (!is_array($config)) { $config = []; }
|
||||
foreach (['secret', 'client_secret', 'api_key', 'password', 'private_key', 'token'] as $banned) {
|
||||
unset($config[$banned]); // difesa: mai segreti nel DB
|
||||
}
|
||||
// ALLOWLIST: persistiamo SOLO i campi non sensibili noti. Qualsiasi altra
|
||||
// chiave (inclusi segreti comunque nominati o annidati) viene scartata.
|
||||
// I segreti vivono solo nel vault — vedi cli_hint.
|
||||
$config = self::sanitizeConnectorConfig($config);
|
||||
|
||||
$alias = $this->getParam('vault_key_alias');
|
||||
if ($alias === null || $alias === '') {
|
||||
@ -521,6 +542,39 @@ class OrganizationController extends BaseController
|
||||
], 'Connettore salvato', $existing ? 200 : 201);
|
||||
}
|
||||
|
||||
/**
|
||||
* Allowlist dei soli campi di configurazione NON sensibili ammessi per i
|
||||
* connettori. Tutto ciò che non è in lista (segreti, chiavi annidate, campi
|
||||
* arbitrari) viene scartato — robusto contro denylist-bypass (case, nesting,
|
||||
* nomi alternativi). I valori sono forzati a stringa e troncati.
|
||||
*/
|
||||
private static function sanitizeConnectorConfig(array $config): array
|
||||
{
|
||||
$allowed = [
|
||||
'tenant_id', 'client_id', 'app_id', 'account_id', 'subscription_id',
|
||||
'region', 'base_url', 'endpoint', 'domain', 'workspace', 'instance_url',
|
||||
'scopes', 'project', 'directory_id', 'org_slug',
|
||||
];
|
||||
$clean = [];
|
||||
foreach ($allowed as $k) {
|
||||
if (!array_key_exists($k, $config)) {
|
||||
continue;
|
||||
}
|
||||
$v = $config[$k];
|
||||
if ($k === 'scopes' && is_array($v)) {
|
||||
// lista di stringhe scope, max 30 voci
|
||||
$clean[$k] = array_slice(array_map(
|
||||
fn($s) => substr((string) $s, 0, 120),
|
||||
array_filter($v, 'is_scalar')
|
||||
), 0, 30);
|
||||
} elseif (is_scalar($v)) {
|
||||
$clean[$k] = substr((string) $v, 0, 255);
|
||||
}
|
||||
// valori non scalari (oggetti/array annidati) su campi non-scopes: scartati
|
||||
}
|
||||
return $clean;
|
||||
}
|
||||
|
||||
/** DELETE /api/organizations/{id}/connectors?type=xxx */
|
||||
public function deleteConnector(int $id): void
|
||||
{
|
||||
|
||||
Loading…
Reference in New Issue
Block a user