Skip to content

Commit

Permalink
Merge pull request #4910 from systeminit/nick/eng-2826
Browse files Browse the repository at this point in the history
Solidify audit log structure in the UI
  • Loading branch information
stack72 authored Nov 1, 2024
2 parents 5c10cdd + b116da3 commit 929d37d
Show file tree
Hide file tree
Showing 14 changed files with 300 additions and 151 deletions.
3 changes: 3 additions & 0 deletions app/web/src/components/AuditLogCell.vue
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,14 @@ const props = defineProps({
type: Object as PropType<
Cell<
{
displayName: string;
userName: string;
userId?: string;
userEmail?: string;
kind: string;
timestamp: string;
entityType: string;
entityName: string;
changeSetId?: string;
changeSetName?: string;
metadata: Record<string, unknown>;
Expand Down
3 changes: 3 additions & 0 deletions app/web/src/components/AuditLogHeader.vue
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,13 @@ const props = defineProps({
type: Object as PropType<
Header<
{
displayName: string;
userName: string;
userId?: string;
userEmail?: string;
kind: string;
entityType: string;
entityName: string;
timestamp: string;
changeSetId?: string;
changeSetName?: string;
Expand Down
77 changes: 46 additions & 31 deletions app/web/src/components/Workspace/WorkspaceAuditLog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@
class="flex items-center gap-2xs pr-xs whitespace-nowrap flex-none"
>
<div>Page</div>
<div class="font-bold">
{{ currentFilters.page }} of {{ totalPages }}
</div>
<div class="font-bold">{{ currentPage }} of {{ totalPages }}</div>
</div>
<IconButton
v-tooltip="
Expand Down Expand Up @@ -106,11 +104,13 @@
<tr
:class="
clsx(
'h-md text-sm',
themeClasses(
'odd:bg-neutral-200 even:bg-neutral-100',
'odd:bg-neutral-700 even:bg-neutral-800',
),
'h-lg text-sm',
rowCollapseState[Number(row.id)]
? 'bg-action-300'
: themeClasses(
'odd:bg-neutral-200 even:bg-neutral-100 hover:bg-action-200',
'odd:bg-neutral-700 even:bg-neutral-800 hover:bg-action-200',
),
)
"
>
Expand Down Expand Up @@ -157,7 +157,7 @@ import {
createColumnHelper,
} from "@tanstack/vue-table";
import clsx from "clsx";
import { h, computed, ref, withDirectives, resolveDirective } from "vue";
import { h, computed, ref } from "vue";
import { trackEvent } from "@/utils/tracking";
import { AuditLogDisplay, LogFilters, useLogsStore } from "@/store/logs.store";
import { AdminUser } from "@/store/admin.store";
Expand Down Expand Up @@ -213,38 +213,51 @@ const columns = [
header: "",
cell: "",
},
columnHelper.accessor("displayName", {
header: "Event",
cell: (info) => info.getValue(),
}),
columnHelper.accessor("entityType", {
header: "Entity Type",
cell: (info) => info.getValue(),
}),
columnHelper.accessor("entityName", {
header: "Entity Name",
cell: (info) => info.getValue(),
}),
columnHelper.accessor("changeSetName", {
header: "Change Set",
cell: (info) => info.getValue(),
}),
// TODO(nick): restore change set filtering.
// columnHelper.accessor("changeSetName", {
// header: "Change Set",
// cell: (info) =>
// withDirectives(
// h("div", {
// innerText: info.getValue(),
// class: "hover:underline cursor-pointer",
// }),
// [[resolveDirective("tooltip"), info.row.getValue("changeSetName")]],
// ),
// }),
columnHelper.accessor("userName", {
header: "User",
cell: (info) => info.getValue(),
}),
columnHelper.accessor("timestamp", {
header: "Timestamp",
header: "Time",
cell: (info) =>
h(Timestamp, {
date: info.getValue(),
relative: true,
enableDetailTooltip: true,
}),
}),
columnHelper.accessor("changeSetName", {
header: "Change Set",
cell: (info) =>
withDirectives(
h("div", {
innerText: info.getValue(),
class: "hover:underline cursor-pointer",
}),
[[resolveDirective("tooltip"), info.row.getValue("changeSetId")]],
),
}),
columnHelper.accessor("changeSetId", {
header: "Change Set Id",
cell: (info) => info.getValue(),
}),
columnHelper.accessor("kind", {
header: "Kind",
cell: (info) => info.getValue(),
}),
columnHelper.accessor("userName", {
header: "User",
cell: (info) => info.getValue(),
}),
];
const table = useVueTable({
Expand Down Expand Up @@ -295,8 +308,6 @@ const toggleFilter = (id: string, filterId: string) => {
const clearFilters = (id: string) => {
if (id === "kind") {
currentFilters.value.kindFilter = [];
} else if (id === "service") {
currentFilters.value.serviceFilter = [];
} else if (id === "changeSetName") {
currentFilters.value.changeSetFilter = [];
} else if (id === "userName") {
Expand Down Expand Up @@ -327,4 +338,8 @@ const previousPage = () => {
currentFilters.value.page--;
loadLogs();
};
const currentPage = computed(() =>
totalPages.value === 0 ? 0 : currentFilters.value.page,
);
</script>
30 changes: 15 additions & 15 deletions app/web/src/store/logs.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,34 +12,31 @@ export type LogFilters = {
sortTimestampAscending: boolean;
excludeSystemUser: boolean;
kindFilter: string[];
serviceFilter: string[];
changeSetFilter: ChangeSetId[];
userFilter: UserId[];
};

export type AuditLog = {
userName?: string;

interface AuditLogCommon {
displayName: string;
userId?: UserId;
userEmail?: string;
kind: string;
entityType: string;
timestamp: string;
changeSetId?: ChangeSetId;
changeSetName?: string;
metadata: Record<string, unknown>;
};
}

export type AuditLogDisplay = {
userName: string;
export interface AuditLog extends AuditLogCommon {
userName?: string;
entityName?: string;
}

userId?: string;
userEmail?: string;
kind: string;
timestamp: string;
changeSetId?: string;
changeSetName?: string;
metadata: Record<string, unknown>;
};
export interface AuditLogDisplay extends AuditLogCommon {
userName: string;
entityName: string;
}

export const useLogsStore = (forceChangeSetId?: ChangeSetId) => {
// this needs some work... but we'll probably want a way to force using HEAD
Expand Down Expand Up @@ -92,10 +89,13 @@ export const useLogsStore = (forceChangeSetId?: ChangeSetId) => {
this.logs = response.logs.map(
(log: AuditLog) =>
({
displayName: log.displayName,
userName: log.userName ?? "System",
userId: log.userId,
userEmail: log.userEmail,
kind: log.kind,
entityType: log.entityType,
entityName: log.entityName ?? "-",
metadata: log.metadata,
timestamp: log.timestamp,
changeSetId: log.changeSetId,
Expand Down
129 changes: 93 additions & 36 deletions lib/dal/src/audit_logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ use telemetry::prelude::*;
use thiserror::Error;
use tokio_util::task::TaskTracker;

use crate::ChangeSet;
use crate::ChangeSetError;
use crate::ChangeSetId;
use crate::ChangeSetStatus;
use crate::DalContext;
use crate::TenancyError;
use crate::TransactionsError;
Expand All @@ -35,6 +39,10 @@ pub enum AuditLoggingError {
AsyncNatsConsumer(#[from] async_nats::error::Error<ConsumerErrorKind>),
#[error("audit logs error: {0}")]
AuditLogs(#[from] AuditLogsError),
#[error("change set error: {0}")]
ChangeSet(#[from] Box<ChangeSetError>),
#[error("change set not found by id: {0}")]
ChangeSetNotFound(ChangeSetId),
#[error("pending events error: {0}")]
PendingEventsError(#[from] PendingEventsError),
#[error("serde json error: {0}")]
Expand Down Expand Up @@ -126,7 +134,7 @@ pub(crate) async fn publish_pending(
}

#[instrument(name = "audit_logging.write", level = "debug", skip_all, fields(kind))]
pub(crate) async fn write(ctx: &DalContext, kind: AuditLogKind) -> Result<()> {
pub(crate) async fn write(ctx: &DalContext, kind: AuditLogKind, entity_name: String) -> Result<()> {
// TODO(nick): nuke this from intergalactic orbit. Then do it again.
let workspace_id = match ctx.workspace_pk() {
Ok(workspace_id) => workspace_id,
Expand All @@ -140,7 +148,13 @@ pub(crate) async fn write(ctx: &DalContext, kind: AuditLogKind) -> Result<()> {
workspace_id.into(),
ctx.change_set_id().into(),
ctx.event_session_id(),
&AuditLog::new(ctx.events_actor(), kind, ctx.change_set_id().into()),
&AuditLog::new(
ctx.events_actor(),
kind,
// NOTE(nick): for now, let's make this mandatory, but I think we'll learn that it shouldn't be.
Some(entity_name),
ctx.change_set_id().into(),
),
)
.await?;
Ok(())
Expand Down Expand Up @@ -184,50 +198,93 @@ pub async fn list(ctx: &DalContext) -> Result<Vec<FrontendAuditLog>> {
})
.await?;

// TODO(nick): remove hard-coded max messages value.
let mut messages = consumer.fetch().max_messages(200).messages().await?;
// TODO(nick): replace hard-coded max messages value with proper pagination.
let mut messages = consumer.fetch().max_messages(1000).messages().await?;
let mut frontend_audit_logs = Vec::new();

while let Some(Ok(message)) = messages.next().await {
let audit_log: AuditLog = serde_json::from_slice(&message.payload)?;
match audit_log {
AuditLog::V2(inner) => {
// TODO(nick): cache users.
let (user_id, user_email, user_name) = match inner.actor {
Actor::System => (None, None, None),
Actor::User(user_id) => {
let user = User::get_by_pk(ctx, user_id.into())
.await
.map_err(Box::new)?
.ok_or(AuditLoggingError::UserNotFound(user_id.into()))?;
(
Some(user_id),
Some(user.email().to_owned()),
Some(user.name().to_owned()),
)
}
};
frontend_audit_logs.push(FrontendAuditLog {
user_id,
user_email,
user_name,
kind: inner.kind.to_string(),
timestamp: inner.timestamp,
change_set_id: inner.change_set_id,
metadata: serde_json::to_value(FrontendAuditLogDeserializedMetadata::from(
inner.kind,
))?,
});
}
AuditLog::V1(_) => {
debug!("skipping older audit logs in beta...");
}
if let Some(frontend_audit_log) = assemble_for_list(ctx, audit_log).await? {
frontend_audit_logs.push(frontend_audit_log);
}
}

Ok(frontend_audit_logs)
}

async fn assemble_for_list(
ctx: &DalContext,
audit_log: AuditLog,
) -> Result<Option<FrontendAuditLog>> {
match audit_log {
AuditLog::V3(inner) => {
// TODO(nick): cache change sets.
let (change_set_id, change_set_name) = match inner.change_set_id {
Some(change_set_id) => {
let change_set = ChangeSet::find(ctx, change_set_id.into())
.await
.map_err(Box::new)?
.ok_or(AuditLoggingError::ChangeSetNotFound(change_set_id.into()))?;
match change_set.status {
ChangeSetStatus::Abandoned
| ChangeSetStatus::Failed
| ChangeSetStatus::Rejected => {
trace!(?change_set.status, ?change_set.id, "skipping change set for audit log assembly due to status");
return Ok(None);
}
ChangeSetStatus::Applied
| ChangeSetStatus::Approved
| ChangeSetStatus::NeedsAbandonApproval
| ChangeSetStatus::NeedsApproval
| ChangeSetStatus::Open => {
(Some(change_set_id), Some(change_set.name.to_owned()))
}
}
}
None => (None, None),
};

// TODO(nick): cache users.
let (user_id, user_email, user_name) = match inner.actor {
Actor::System => (None, None, None),
Actor::User(user_id) => {
let user = User::get_by_pk(ctx, user_id.into())
.await
.map_err(Box::new)?
.ok_or(AuditLoggingError::UserNotFound(user_id.into()))?;
(
Some(user_id),
Some(user.email().to_owned()),
Some(user.name().to_owned()),
)
}
};

let kind = inner.kind.to_string();
let deserialized_metadata = FrontendAuditLogDeserializedMetadata::from(inner.kind);
let (display_name, entity_type) = deserialized_metadata.display_name_and_entity_type();

Ok(Some(FrontendAuditLog {
display_name: display_name.to_owned(),
user_id,
user_email,
user_name,
kind,
entity_name: inner.entity_name,
entity_type: entity_type.to_owned(),
timestamp: inner.timestamp,
change_set_id,
change_set_name,
metadata: serde_json::to_value(deserialized_metadata)?,
}))
}
AuditLog::V2(_) | AuditLog::V1(_) => {
debug!("skipping older audit logs in beta...");
Ok(None)
}
}
}

pub mod temporary {
use std::collections::HashSet;

Expand Down
Loading

0 comments on commit 929d37d

Please sign in to comment.