mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(audit): keep personal data and credentials out of audit events
Anonymize the audit events of a deleted user, store only the ID and name of other users, and drop the metadata copy of actor_email. Store URLs without query strings, registries without user info, and the exception class instead of raw SSH errors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
b7d0e6be2a
commit
3379fa448a
@@ -110,8 +110,8 @@ class UpdateIntegrationToken
|
||||
'integration_token_uuid' => $integrationToken->uuid,
|
||||
'integration_token_name' => $integrationToken->name,
|
||||
'provider' => $provider,
|
||||
'previous_base_url' => $this->baseUrlForAudit(data_get($storedMetadata, 'base_url')),
|
||||
'base_url' => $this->baseUrlForAudit(data_get($metadata, 'base_url')),
|
||||
'previous_base_url' => auditUrl(data_get($storedMetadata, 'base_url')),
|
||||
'base_url' => auditUrl(data_get($metadata, 'base_url')),
|
||||
], 'warning');
|
||||
}
|
||||
|
||||
@@ -146,25 +146,6 @@ class UpdateIntegrationToken
|
||||
));
|
||||
}
|
||||
|
||||
/**
|
||||
* Base URL without user info, query, or fragment, which can carry credentials.
|
||||
*/
|
||||
private function baseUrlForAudit(mixed $url): ?string
|
||||
{
|
||||
if (! is_string($url) || $url === '') {
|
||||
return null;
|
||||
}
|
||||
|
||||
$parts = parse_url($url);
|
||||
if (! is_array($parts) || ! isset($parts['scheme'], $parts['host'])) {
|
||||
return '[invalid-url]';
|
||||
}
|
||||
|
||||
return $parts['scheme'].'://'.$parts['host']
|
||||
.(isset($parts['port']) ? ':'.$parts['port'] : '')
|
||||
.($parts['path'] ?? '');
|
||||
}
|
||||
|
||||
private function reenterSecretMessage(string $provider): string
|
||||
{
|
||||
return match ($provider) {
|
||||
|
||||
@@ -112,7 +112,7 @@ class ServerTransferController extends Controller
|
||||
auditLog('api.server.migrate', [
|
||||
'team_id' => $teamId,
|
||||
'server_uuid' => $server->uuid,
|
||||
'target_url' => data_get($server->server_metadata, 'migration.target_url'),
|
||||
'target_url' => auditUrl(data_get($server->server_metadata, 'migration.target_url')),
|
||||
]);
|
||||
|
||||
return response()->json([
|
||||
|
||||
@@ -279,7 +279,6 @@ class Controller extends BaseController
|
||||
'already_member' => $alreadyMember,
|
||||
'member_id' => $user->id,
|
||||
'member_name' => $user->name,
|
||||
'member_email' => $user->email,
|
||||
'actor_id' => $user->id,
|
||||
'actor_name' => $user->name,
|
||||
'actor_email' => $user->email,
|
||||
|
||||
@@ -75,7 +75,6 @@ class Index extends Component
|
||||
'resource' => 'user',
|
||||
'user_name' => $user->name,
|
||||
'target_user_id' => $user->id,
|
||||
'target_user_email' => $user->email,
|
||||
'target_team_id' => $team_to_switch_to?->id,
|
||||
]);
|
||||
Auth::login($user);
|
||||
|
||||
@@ -125,7 +125,7 @@ class Patches extends Component
|
||||
try {
|
||||
RebootServer::run($this->server);
|
||||
} catch (\Throwable $e) {
|
||||
$this->auditPatchAction('reboot_requested', ['result' => 'failed', 'error' => $e->getMessage()], 'error');
|
||||
$this->auditPatchAction('reboot_requested', ['result' => 'failed', 'error' => class_basename($e)], 'error');
|
||||
|
||||
throw $e;
|
||||
}
|
||||
@@ -212,7 +212,7 @@ class Patches extends Component
|
||||
try {
|
||||
ConfigureAutomaticSecurityUpdates::run($this->server, $packageManager, $enabled, $time);
|
||||
} catch (\Throwable $e) {
|
||||
$this->auditPatchAction('auto_security_updates_updated', [...$auditContext, 'result' => 'failed', 'error' => $e->getMessage()], 'error');
|
||||
$this->auditPatchAction('auto_security_updates_updated', [...$auditContext, 'result' => 'failed', 'error' => class_basename($e)], 'error');
|
||||
|
||||
throw $e;
|
||||
}
|
||||
|
||||
@@ -116,7 +116,7 @@ class Transfer extends Component
|
||||
auditLog('ui.server.migrate', [
|
||||
'team_id' => $this->server->team_id,
|
||||
'server_uuid' => $this->server->uuid,
|
||||
'target_url' => $this->targetUrl,
|
||||
'target_url' => auditUrl($this->targetUrl),
|
||||
]);
|
||||
|
||||
$this->server->refresh();
|
||||
@@ -141,7 +141,7 @@ class Transfer extends Component
|
||||
auditLog('ui.server.transfer_back', [
|
||||
'team_id' => $this->server->team_id,
|
||||
'server_uuid' => $this->server->uuid,
|
||||
'from' => $this->managerUrl,
|
||||
'from' => auditUrl($this->managerUrl),
|
||||
]);
|
||||
|
||||
$this->server->refresh();
|
||||
|
||||
@@ -89,9 +89,7 @@ class AdminView extends Component
|
||||
auditLog('ui.user.deleted', [
|
||||
'team_id' => currentTeam()?->id,
|
||||
'resource' => 'user',
|
||||
'user_name' => $user->name,
|
||||
'deleted_user_id' => $user->id,
|
||||
'deleted_user_email' => $user->email,
|
||||
]);
|
||||
$this->resetPage();
|
||||
|
||||
|
||||
@@ -83,7 +83,6 @@ class Member extends Component
|
||||
'team_id' => $teamId,
|
||||
'member_id' => $this->member->id,
|
||||
'member_name' => $this->member->name,
|
||||
'member_email' => $this->member->email,
|
||||
]);
|
||||
// Clear cache for the removed user - both old and new key formats
|
||||
Cache::forget("team:{$this->member->id}");
|
||||
@@ -117,7 +116,6 @@ class Member extends Component
|
||||
'team_id' => $teamId,
|
||||
'member_id' => $this->member->id,
|
||||
'member_name' => $this->member->name,
|
||||
'member_email' => $this->member->email,
|
||||
'role' => $role->value,
|
||||
]);
|
||||
}
|
||||
|
||||
@@ -122,7 +122,6 @@ class Access extends Component
|
||||
'team_id' => $this->teamId,
|
||||
'member_id' => $member->id,
|
||||
'member_name' => $member->name,
|
||||
'member_email' => $member->email,
|
||||
'access' => Grant::listToArray($grants),
|
||||
]);
|
||||
|
||||
|
||||
@@ -203,7 +203,7 @@ class AuditEvent extends Model
|
||||
'resource_name' => $resourceName,
|
||||
'description' => data_get($context, 'audit_description')
|
||||
?? trim(($resourceName ?? Str::headline((string) $resourceType)).' '.Str::headline($action)),
|
||||
'metadata' => self::redact(Arr::except($context, ['audit_changes'])),
|
||||
'metadata' => self::redact(Arr::except($context, ['audit_changes', 'actor_email'])),
|
||||
'changes' => $changes,
|
||||
'ip_address' => app()->bound('request') ? (client_ip(request()) ?: null) : null,
|
||||
'user_agent' => app()->bound('request') ? Str::limit((string) request()->userAgent(), 200, '') : null,
|
||||
@@ -235,6 +235,53 @@ class AuditEvent extends Model
|
||||
->first()?->team()?->id;
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes the personal data of a deleted user: from the events that the user did,
|
||||
* and from the events about the user (team member, impersonation, deletion).
|
||||
*/
|
||||
public static function anonymizeUser(User $user): void
|
||||
{
|
||||
$replacement = '[deleted user]';
|
||||
$personalValues = array_filter([Str::lower((string) $user->email), Str::lower((string) $user->name)]);
|
||||
|
||||
self::query()
|
||||
->where(fn (Builder $query) => $query
|
||||
->where(fn (Builder $query) => $query->where('actor_type', 'user')->where('actor_id', $user->id))
|
||||
->orWhereJsonContains('metadata->member_id', $user->id)
|
||||
->orWhereJsonContains('metadata->target_user_id', $user->id)
|
||||
->orWhereJsonContains('metadata->deleted_user_id', $user->id))
|
||||
->chunkById(self::PRUNE_BATCH_SIZE, function ($events) use ($personalValues, $replacement, $user): void {
|
||||
foreach ($events as $event) {
|
||||
if ($event->actor_type === 'user' && (int) $event->actor_id === (int) $user->id) {
|
||||
$event->actor_name = $replacement;
|
||||
$event->actor_email = null;
|
||||
$event->ip_address = null;
|
||||
$event->user_agent = null;
|
||||
}
|
||||
|
||||
if ($event->resource_name !== null && in_array(Str::lower($event->resource_name), $personalValues, true)) {
|
||||
$event->description = $replacement.substr($event->description, strlen($event->resource_name));
|
||||
$event->resource_name = $replacement;
|
||||
}
|
||||
|
||||
$event->metadata = self::replacePersonalValues($event->metadata, $personalValues, $replacement);
|
||||
$event->save();
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<int, string> $personalValues
|
||||
*/
|
||||
private static function replacePersonalValues(mixed $value, array $personalValues, string $replacement): mixed
|
||||
{
|
||||
if (is_array($value)) {
|
||||
return array_map(fn (mixed $item): mixed => self::replacePersonalValues($item, $personalValues, $replacement), $value);
|
||||
}
|
||||
|
||||
return is_string($value) && in_array(Str::lower($value), $personalValues, true) ? $replacement : $value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Deletes in batches, so a large table is not locked by one long DELETE.
|
||||
*/
|
||||
|
||||
@@ -186,6 +186,8 @@ class User extends Authenticatable implements PasskeyUser, SendsEmail
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
static::deleted(fn (User $user) => AuditEvent::anonymizeUser($user));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -255,7 +255,7 @@ class DockerRegistryLogins
|
||||
'server_uuid' => $server->uuid,
|
||||
'server_name' => $server->name,
|
||||
'team_id' => $server->team_id,
|
||||
'registry' => $registry,
|
||||
'registry' => preg_replace('#^[^/]*@#', '', $registry),
|
||||
'outcome' => $succeeded ? 'success' : 'failed',
|
||||
], $succeeded ? 'info' : 'warning');
|
||||
}
|
||||
|
||||
@@ -53,6 +53,27 @@ if (! function_exists('auditLog')) {
|
||||
}
|
||||
}
|
||||
|
||||
if (! function_exists('auditUrl')) {
|
||||
/**
|
||||
* Scheme, host, port and path of a URL, without user info, query string or fragment.
|
||||
*/
|
||||
function auditUrl(mixed $url): ?string
|
||||
{
|
||||
if (! is_string($url) || $url === '') {
|
||||
return null;
|
||||
}
|
||||
|
||||
$parts = parse_url($url);
|
||||
if (! is_array($parts) || ! isset($parts['scheme'], $parts['host'])) {
|
||||
return '[invalid-url]';
|
||||
}
|
||||
|
||||
return $parts['scheme'].'://'.$parts['host']
|
||||
.(isset($parts['port']) ? ':'.$parts['port'] : '')
|
||||
.($parts['path'] ?? '');
|
||||
}
|
||||
}
|
||||
|
||||
if (! function_exists('auditChangedFields')) {
|
||||
/**
|
||||
* Names of the attributes that the next save() will really change, without timestamps.
|
||||
|
||||
@@ -140,7 +140,8 @@ test('records an accepted team invitation', function () {
|
||||
->and($event->actor_id)->toBe($this->user->id)
|
||||
->and($event->resource_uuid)->toBe('account-audit-invitation')
|
||||
->and(data_get($event->metadata, 'role'))->toBe('admin')
|
||||
->and(data_get($event->metadata, 'member_email'))->toBe($this->user->email)
|
||||
->and(data_get($event->metadata, 'member_id'))->toBe($this->user->id)
|
||||
->and($event->metadata)->not->toHaveKeys(['member_email', 'actor_email'])
|
||||
->and(data_get($event->metadata, 'already_member'))->toBeFalse();
|
||||
});
|
||||
|
||||
@@ -195,7 +196,53 @@ test('records a user deleted by an instance admin', function () {
|
||||
->and($event->team_id)->toBe($this->team->id)
|
||||
->and($event->actor_id)->toBe($this->user->id)
|
||||
->and(data_get($event->metadata, 'deleted_user_id'))->toBe($deletedUser->id)
|
||||
->and(data_get($event->metadata, 'deleted_user_email'))->toBe('deleted-user@example.com');
|
||||
->and(accountAuditSerialized($event))->not->toContain('deleted-user@example.com');
|
||||
});
|
||||
|
||||
test('deleting a user removes their personal data from earlier audit events', function () {
|
||||
$rootTeam = Team::factory()->create(['id' => 0]);
|
||||
$rootTeam->members()->attach($this->user->id, ['role' => 'admin']);
|
||||
$deletedUser = User::factory()->create(['name' => 'Jane Doe', 'email' => 'jane@example.com']);
|
||||
$ownEvent = AuditEvent::factory()->create([
|
||||
'event' => 'auth.user.login',
|
||||
'actor_type' => 'user',
|
||||
'actor_id' => $deletedUser->id,
|
||||
'actor_name' => 'Jane Doe',
|
||||
'actor_email' => 'jane@example.com',
|
||||
'resource_type' => 'user',
|
||||
'resource_name' => 'Jane Doe',
|
||||
'description' => 'Jane Doe Login',
|
||||
'metadata' => ['user_name' => 'Jane Doe'],
|
||||
'ip_address' => '203.0.113.7',
|
||||
'user_agent' => 'Jane Browser',
|
||||
]);
|
||||
$aboutEvent = AuditEvent::factory()->create([
|
||||
'event' => 'ui.team_member.removed',
|
||||
'actor_type' => 'user',
|
||||
'actor_id' => $this->user->id,
|
||||
'actor_email' => $this->user->email,
|
||||
'metadata' => ['member_id' => $deletedUser->id, 'member_name' => 'Jane Doe', 'member_email' => 'JANE@example.com'],
|
||||
]);
|
||||
$unrelatedEvent = AuditEvent::factory()->create([
|
||||
'event' => 'ui.team_member.removed',
|
||||
'metadata' => ['member_id' => $deletedUser->id + 1000, 'member_name' => 'Jane Doe'],
|
||||
]);
|
||||
|
||||
Livewire::test(AdminView::class)
|
||||
->call('delete', $deletedUser->id, 'password')
|
||||
->assertReturned(true);
|
||||
|
||||
$ownEvent->refresh();
|
||||
$aboutEvent->refresh();
|
||||
|
||||
expect(accountAuditSerialized($ownEvent))->not->toContain('Jane Doe')
|
||||
->not->toContain('jane@example.com')
|
||||
->not->toContain('203.0.113.7')
|
||||
->not->toContain('Jane Browser')
|
||||
->and($ownEvent->description)->toBe('[deleted user] Login')
|
||||
->and($aboutEvent->metadata)->toBe(['member_id' => $deletedUser->id, 'member_name' => '[deleted user]', 'member_email' => '[deleted user]'])
|
||||
->and($aboutEvent->actor_email)->toBe($this->user->email)
|
||||
->and($unrelatedEvent->refresh()->metadata['member_name'])->toBe('Jane Doe');
|
||||
});
|
||||
|
||||
test('does not record a user deletion when the password is wrong', function () {
|
||||
@@ -229,7 +276,7 @@ test('records impersonation with the impersonating root user as actor', function
|
||||
expect($event->actor_id)->toBe(0)
|
||||
->and($event->team_id)->toBe(0)
|
||||
->and(data_get($event->metadata, 'target_user_id'))->toBe($targetUser->id)
|
||||
->and(data_get($event->metadata, 'target_user_email'))->toBe('impersonated@example.com');
|
||||
->and(accountAuditSerialized($event))->not->toContain('impersonated@example.com');
|
||||
});
|
||||
|
||||
test('records an oauth identity linked to an existing user', function () {
|
||||
|
||||
Reference in New Issue
Block a user