Audit events UI shows "(removed)" next to system authors like "(System)"

Illya Klymovrequested to merge
xanf/ai/run-heg73j-audit-events-ui-shows-removed-next-to-sy into master

What does this MR do and why?

The audit events table appends a "(removed)" hint to any author whose name isn't a link, on the assumption that a missing profile URL means the user was deleted. That assumption doesn't hold for system-generated events: Gitlab::Audit::UnauthenticatedAuthor (author_id -1, display name (System)) and similar non-user authors (deploy tokens, unauthenticated requests) never had a profile URL to begin with, so every one of these events renders as (System) (removed) in the UI, even though nothing was ever removed. This affects 10+ emitters across the codebase (member management, container registry cleanup, webhook admin destroy, compliance controls timeout, Duo stuck session cleanup, and others), and it's misleading on a page whose purpose is forensic review — it implies the acting principal was deleted when it was never a user account at all. The underlying data is unaffected: the REST API, GraphQL, CSV export, and streamed audit payloads already carry the clean author_name with no suffix; the issue is purely in how the frontend decides when to append the hint.

The fix threads an explicit signal through from backend to frontend instead of inferring "removed" from the absence of a URL. Gitlab::Audit::NullAuthor and Gitlab::Audit::DeletedAuthor now expose whether the author actually represents a deleted user, AuditEventPresenter surfaces that flag, and AuditEventEntity serializes it alongside the existing author fields. On the frontend, audit_events_table.vue and table_cells/url_table_cell.vue use that flag to decide whether to render the "(removed)" suffix, rather than keying off target_details/URL presence. As a result, only authors that were once real users and have since been deleted show the hint; system authors like (System) and other non-user authors render cleanly with no suffix, matching what the API and other export surfaces already show.

Closes #606991

References

Closes #606991

Screenshots or screen recordings

Before After

How to set up and validate locally

  1. In a rails console (bin/rails runner), run the fixture setup to create a repro group/project with a system-authored event ((System), author_id: -1) and a genuinely-deleted-user event (contrast case): 
    # frozen_string_literal: true
#
# Fixture for https://gitlab.com/gitlab-org/gitlab/-/issues/606991
# "(removed)" wrongly appended to system-authored audit events.
#
# Mode: self-managed (sm) - issue is silent on SaaS vs SM, current GDK mode kept as-is.
# Reset-first + idempotent: destroys any prior run's repro group/project before recreating,
# so re-running always yields the identical starting state.

GROUP_PATH = 'cockpit-repro-audit-events-group'
PROJECT_PATH = 'cockpit-repro-audit-events'

existing_project = Project.find_by_full_path("#{GROUP_PATH}/#{PROJECT_PATH}")
existing_project&.destroy!

existing_group = Group.find_by_full_path(GROUP_PATH)
existing_group&.destroy!

root = User.find_by(username: 'root')
organization = Organizations::Organization.first

group = Group.new(name: 'Cockpit Repro Audit Events', path: GROUP_PATH, organization: organization)
group.save!(validate: false)
group.add_owner(root)

project = ::Projects::CreateService.new(
  root,
  name: 'Cockpit Repro Audit Events',
  path: PROJECT_PATH,
  namespace_id: group.id,
  organization_id: organization.id,
  visibility_level: Gitlab::VisibilityLevel::PRIVATE
).execute

raise "project creation failed: #{project.errors.full_messages}" unless project.persisted?

project.add_maintainer(root)

common_attrs = {
  project_id: project.id,
  entity_path: project.full_path,
  target_details: project.name,
  ip_address: '127.0.0.1'
}

# System author: Gitlab::Audit::UnauthenticatedAuthor (author_id -1), display name "(System)" -
# the exact pattern used by cron/system emitters (e.g. duo_session_failed stuck-session cleanup).
# NullAuthor#full_path is always nil, which is what url_table_cell.vue currently misreads as "removed".
AuditEvents::ProjectAuditEvent.create!(
  common_attrs.merge(
    author_id: -1,
    author_name: '(System)',
    event_name: 'duo_session_failed',
    details: {
      custom_message: 'Duo session failed: stuck session cleaned up after timeout',
      author_name: '(System)',
      author_class: 'Gitlab::Audit::UnauthenticatedAuthor',
      target_id: project.id,
      target_type: 'Project',
      target_details: project.name,
      ip_address: '127.0.0.1',
      entity_path: project.full_path
    }
  )
)

# Contrast case: a genuinely deleted user must keep showing "(removed)" - non-triviality guard,
# the fix must not suppress the suffix everywhere, only for NullAuthor subclasses.
AuditEvents::ProjectAuditEvent.create!(
  common_attrs.merge(
    author_id: User.maximum(:id).to_i + 1000,
    author_name: 'A deleted user',
    event_name: 'project_name_updated',
    details: {
      change: 'name',
      from: 'Old Name',
      to: 'New Name',
      author_name: 'A deleted user',
      target_id: project.id,
      target_type: 'Project',
      target_details: project.name,
      ip_address: '127.0.0.1',
      entity_path: project.full_path
    }
  )
)

puts "OK project_full_path=#{project.full_path}"
  2. Visit /cockpit-repro-audit-events-group/cockpit-repro-audit-events/-/audit_events.
  3. Before the fix: the duo_session_failed row shows (System) (removed) in the Author column. After the fix: it shows (System) with no suffix.
  4. Confirm the contrast case still behaves correctly: the project_name_updated row (author A deleted user) still shows A deleted user (removed), proving the fix only suppresses the suffix for NullAuthor subclasses (system/deploy token/deploy key/orbit indexer), not for genuinely deleted users.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Merge request reports