From a6309822feb8ec1a016cf1809ba5c948631b3252 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 23 Nov 2022 02:29:44 +0000 Subject: [PATCH 01/12] got links working --- coderd/audit.go | 57 +++++++++++++++--- .../provisionerdserver/provisionerdserver.go | 3 + codersdk/audit.go | 1 + site/src/api/typesGenerated.ts | 1 + .../components/AuditLogRow/AuditLogRow.tsx | 58 +++++++++++++++---- 5 files changed, 102 insertions(+), 18 deletions(-) diff --git a/coderd/audit.go b/coderd/audit.go index e177494d6d934..812c0825f7866 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -1,6 +1,7 @@ package coderd import ( + "context" "database/sql" "encoding/json" "fmt" @@ -14,6 +15,7 @@ import ( "github.com/google/uuid" "github.com/tabbed/pqtype" + "cdr.dev/slog" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" @@ -69,7 +71,7 @@ func (api *API) auditLogs(rw http.ResponseWriter, r *http.Request) { } httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogResponse{ - AuditLogs: convertAuditLogs(dblogs), + AuditLogs: api.convertAuditLogs(ctx, dblogs), Count: dblogs[0].Count, }) } @@ -147,17 +149,17 @@ func (api *API) generateFakeAuditLog(rw http.ResponseWriter, r *http.Request) { rw.WriteHeader(http.StatusNoContent) } -func convertAuditLogs(dblogs []database.GetAuditLogsOffsetRow) []codersdk.AuditLog { +func (api *API) convertAuditLogs(ctx context.Context, dblogs []database.GetAuditLogsOffsetRow) []codersdk.AuditLog { alogs := make([]codersdk.AuditLog, 0, len(dblogs)) for _, dblog := range dblogs { - alogs = append(alogs, convertAuditLog(dblog)) + alogs = append(alogs, api.convertAuditLog(ctx, dblog)) } return alogs } -func convertAuditLog(dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog { +func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog { ip, _ := netip.AddrFromSlice(dblog.Ip.IPNet.IP) diff := codersdk.AuditDiff{} @@ -199,21 +201,60 @@ func convertAuditLog(dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog { AdditionalFields: dblog.AdditionalFields, Description: auditLogDescription(dblog), User: user, + ResourceLink: api.auditLogResourceLink(ctx, dblog), + } +} + +type AdditionalFields struct { + WorkspaceName string + BuildNumber string +} + +func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAuditLogsOffsetRow) string { + switch alog.ResourceType { + case database.ResourceTypeOrganization: + return "" + case database.ResourceTypeTemplate: + return "" + case database.ResourceTypeTemplateVersion: + return "" + case database.ResourceTypeUser: + return "" + case database.ResourceTypeWorkspace: + return fmt.Sprintf("/@%s/%s", + alog.UserUsername.String, alog.ResourceTarget) + case database.ResourceTypeWorkspaceBuild: + additionalFieldsBytes := []byte(alog.AdditionalFields) + var additionalFields AdditionalFields + err := json.Unmarshal(additionalFieldsBytes, &additionalFields) + if err != nil { + api.Logger.Error(ctx, "could not unmarshal workspace name for friendly string", slog.Error(err)) + } + return fmt.Sprintf("/@%s/%s/builds/%s", + alog.UserUsername.String, additionalFields.WorkspaceName, additionalFields.BuildNumber) + case database.ResourceTypeGitSshKey: + return "" + case database.ResourceTypeApiKey: + return "" + case database.ResourceTypeGroup: + return "" + default: + return "" } } func auditLogDescription(alog database.GetAuditLogsOffsetRow) string { - str := fmt.Sprintf("{user} %s %s", + str := fmt.Sprintf("{user} %s", codersdk.AuditAction(alog.Action).FriendlyString(), - codersdk.ResourceType(alog.ResourceType).FriendlyString(), ) // Strings for workspace_builds follow the below format: // "{user} started workspace build for {target}" // where target is a workspace instead of the workspace build, // passed in on the FE via AuditLog.AdditionalFields rather than derived in request.go:35 - if alog.ResourceType == database.ResourceTypeWorkspaceBuild { - str += " for" + if alog.ResourceType != database.ResourceTypeWorkspaceBuild { + str += fmt.Sprintf(" %s", + codersdk.ResourceType(alog.ResourceType).FriendlyString()) } // We don't display the name for git ssh keys. It's fairly long and doesn't diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index f3ecba76e6f47..51c622f7f172f 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -9,6 +9,7 @@ import ( "net/http" "net/url" "reflect" + "strconv" "sync" "sync/atomic" "time" @@ -541,6 +542,7 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p // can form a friendly string for the user. workspaceResourceInfo := map[string]string{ "workspaceName": workspace.Name, + "buildNumber": strconv.FormatInt(int64(build.BuildNumber), 10), } wriBytes, err := json.Marshal(workspaceResourceInfo) @@ -756,6 +758,7 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete // can form a friendly string for the user. workspaceResourceInfo := map[string]string{ "workspaceName": workspace.Name, + "buildNumber": strconv.FormatInt(int64(workspaceBuild.BuildNumber), 10), } wriBytes, err := json.Marshal(workspaceResourceInfo) diff --git a/codersdk/audit.go b/codersdk/audit.go index f958411501599..2b8e3ad3b3a45 100644 --- a/codersdk/audit.go +++ b/codersdk/audit.go @@ -102,6 +102,7 @@ type AuditLog struct { StatusCode int32 `json:"status_code"` AdditionalFields json.RawMessage `json:"additional_fields"` Description string `json:"description"` + ResourceLink string `json:"resource_link"` User *User `json:"user"` } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 2347d8f680369..5ecb29a1c7ed9 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -67,6 +67,7 @@ export interface AuditLog { readonly status_code: number readonly additional_fields: Record readonly description: string + readonly resource_link: string readonly user?: User } diff --git a/site/src/components/AuditLogRow/AuditLogRow.tsx b/site/src/components/AuditLogRow/AuditLogRow.tsx index aa51fd6f01734..f9caac9232ac0 100644 --- a/site/src/components/AuditLogRow/AuditLogRow.tsx +++ b/site/src/components/AuditLogRow/AuditLogRow.tsx @@ -15,21 +15,45 @@ import { PaletteIndex } from "theme/palettes" import userAgentParser from "ua-parser-js" import { combineClasses } from "util/combineClasses" import { AuditLogDiff } from "./AuditLogDiff" +import { Link } from "react-router-dom" -export const readableActionMessage = (auditLog: AuditLog): string => { +// const determineInitiator = (auditLog: AuditLog): string => { +// return auditLog. +// } + +const determineResourceTarget = (auditLog: AuditLog): string => { let target = auditLog.resource_target.trim() // audit logs with a resource_type of workspace build use workspace name as a target - if ( - auditLog.resource_type === "workspace_build" && - auditLog.additional_fields.workspaceName - ) { - target = auditLog.additional_fields.workspaceName.trim() + if (auditLog.resource_type === "workspace_build") { + // target = auditLog.additional_fields.workspaceName.trim() + target = "build" } + return target +} + +export const readableActionMessage = (auditLog: AuditLog): string => { + // let target = auditLog.resource_target.trim() + + // audit logs with a resource_type of workspace build use workspace name as a target + // if ( + // auditLog.resource_type === "workspace_build" && + // auditLog.additional_fields.workspaceName + // ) { + // target = auditLog.additional_fields.workspaceName.trim() + // } + return auditLog.description - .replace("{user}", `${auditLog.user?.username.trim()}`) - .replace("{target}", `${target}`) + .replace("{user}", `${auditLog.user?.username.trim()}`) + .replace("{target}", "") + + // return auditLog.description + // .replace("{user}", `${auditLog.user?.username.trim()}`) + // .replace( + // "{target}", + // `${target}`, + // ) } const httpStatusColor = (httpStatus: number): PaletteIndex => { @@ -119,11 +143,25 @@ export const AuditLogRow: React.FC = ({ alignItems="baseline" spacing={1} > - + {readableActionMessage(auditLog)}{" "} + + {determineResourceTarget(auditLog)} + + {auditLog.resource_type === "workspace_build" && + auditLog.additional_fields.workspaceName && ( + + {" "} + for workspace{" "} + {auditLog.additional_fields.workspaceName} + + )} + + {/* + /> */} {new Date(auditLog.time).toLocaleTimeString()} From c988b1a8590bc5bb192d68e04f317827cc0d0bb8 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 23 Nov 2022 18:20:03 +0000 Subject: [PATCH 02/12] added translations --- .../components/AuditLogRow/AuditLogRow.tsx | 66 +++++++------------ site/src/i18n/en/auditLog.json | 9 ++- 2 files changed, 31 insertions(+), 44 deletions(-) diff --git a/site/src/components/AuditLogRow/AuditLogRow.tsx b/site/src/components/AuditLogRow/AuditLogRow.tsx index f9caac9232ac0..b33dc2853128a 100644 --- a/site/src/components/AuditLogRow/AuditLogRow.tsx +++ b/site/src/components/AuditLogRow/AuditLogRow.tsx @@ -15,45 +15,25 @@ import { PaletteIndex } from "theme/palettes" import userAgentParser from "ua-parser-js" import { combineClasses } from "util/combineClasses" import { AuditLogDiff } from "./AuditLogDiff" -import { Link } from "react-router-dom" +import { Link as RouterLink } from "react-router-dom" +import i18next from "i18next" +import Link from "@material-ui/core/Link" -// const determineInitiator = (auditLog: AuditLog): string => { -// return auditLog. -// } +const determineResourceLink = (auditLog: AuditLog): string => { + const { t } = i18next + let linkTarget = auditLog.resource_target.trim() -const determineResourceTarget = (auditLog: AuditLog): string => { - let target = auditLog.resource_target.trim() - - // audit logs with a resource_type of workspace build use workspace name as a target if (auditLog.resource_type === "workspace_build") { - // target = auditLog.additional_fields.workspaceName.trim() - target = "build" + linkTarget = t("auditLog:table.logRow.buildTarget") } - return target + return linkTarget } export const readableActionMessage = (auditLog: AuditLog): string => { - // let target = auditLog.resource_target.trim() - - // audit logs with a resource_type of workspace build use workspace name as a target - // if ( - // auditLog.resource_type === "workspace_build" && - // auditLog.additional_fields.workspaceName - // ) { - // target = auditLog.additional_fields.workspaceName.trim() - // } - return auditLog.description .replace("{user}", `${auditLog.user?.username.trim()}`) .replace("{target}", "") - - // return auditLog.description - // .replace("{user}", `${auditLog.user?.username.trim()}`) - // .replace( - // "{target}", - // `${target}`, - // ) } const httpStatusColor = (httpStatus: number): PaletteIndex => { @@ -79,6 +59,7 @@ export const AuditLogRow: React.FC = ({ defaultIsDiffOpen = false, }) => { const styles = useStyles() + const { t } = i18next const [isDiffOpen, setIsDiffOpen] = useState(defaultIsDiffOpen) const diffs = Object.entries(auditLog.diff) const shouldDisplayDiff = diffs.length > 0 @@ -145,23 +126,19 @@ export const AuditLogRow: React.FC = ({ > {readableActionMessage(auditLog)}{" "} - - {determineResourceTarget(auditLog)} - + {auditLog.resource_link && ( + + {determineResourceLink(auditLog)} + + )} {auditLog.resource_type === "workspace_build" && auditLog.additional_fields.workspaceName && ( - - {" "} - for workspace{" "} + <> + {t("auditLog:table.logRow.buildFriendlyString")} {auditLog.additional_fields.workspaceName} - + )} - {/* */} {new Date(auditLog.time).toLocaleTimeString()} @@ -170,15 +147,18 @@ export const AuditLogRow: React.FC = ({ - IP: {auditLog.ip ?? notAvailableLabel} + <>{t("auditLog:table.logRow.ip")} + {auditLog.ip ?? notAvailableLabel} - OS: {os.name ?? notAvailableLabel} + <>{t("auditLog:table.logRow.os")} + {os.name ?? notAvailableLabel} - Browser: {displayBrowserInfo} + <>{t("auditLog:table.logRow.browser")} + {displayBrowserInfo} diff --git a/site/src/i18n/en/auditLog.json b/site/src/i18n/en/auditLog.json index 84be1ec8adbec..503fadc272ff1 100644 --- a/site/src/i18n/en/auditLog.json +++ b/site/src/i18n/en/auditLog.json @@ -6,6 +6,13 @@ }, "table": { "emptyPage": "No audit logs available on this page", - "noLogs": "No audit logs available" + "noLogs": "No audit logs available", + "logRow": { + "buildTarget": "build", + "buildFriendlyString": " for workspace ", + "ip": "IP: ", + "os": "OS: ", + "Browser": "Browser: " + } } } From 34a527fcc35de2e060810b390bf28135ff2ab175 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 23 Nov 2022 18:21:59 +0000 Subject: [PATCH 03/12] fixed translation --- site/src/i18n/en/auditLog.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/i18n/en/auditLog.json b/site/src/i18n/en/auditLog.json index 503fadc272ff1..441c091e21e7f 100644 --- a/site/src/i18n/en/auditLog.json +++ b/site/src/i18n/en/auditLog.json @@ -12,7 +12,7 @@ "buildFriendlyString": " for workspace ", "ip": "IP: ", "os": "OS: ", - "Browser": "Browser: " + "browser": "Browser: " } } } From b10141756befe068bd1655f5ab0ea1d987e11f98 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Mon, 28 Nov 2022 17:03:53 +0000 Subject: [PATCH 04/12] added translation for unavailable ip --- site/src/components/AuditLogRow/AuditLogRow.tsx | 16 ++++++++++++---- site/src/i18n/en/auditLog.json | 3 ++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/site/src/components/AuditLogRow/AuditLogRow.tsx b/site/src/components/AuditLogRow/AuditLogRow.tsx index b33dc2853128a..943c98fffc181 100644 --- a/site/src/components/AuditLogRow/AuditLogRow.tsx +++ b/site/src/components/AuditLogRow/AuditLogRow.tsx @@ -64,10 +64,9 @@ export const AuditLogRow: React.FC = ({ const diffs = Object.entries(auditLog.diff) const shouldDisplayDiff = diffs.length > 0 const { os, browser } = userAgentParser(auditLog.user_agent) - const notAvailableLabel = "Not available" const displayBrowserInfo = browser.name ? `${browser.name} ${browser.version}` - : notAvailableLabel + : t("auditLog:table.logRow.notAvailable") const toggle = () => { if (shouldDisplayDiff) { @@ -148,12 +147,21 @@ export const AuditLogRow: React.FC = ({ <>{t("auditLog:table.logRow.ip")} - {auditLog.ip ?? notAvailableLabel} + + {auditLog.ip + ? auditLog.ip + : t("auditLog:table.logRow.notAvailable")} + <>{t("auditLog:table.logRow.os")} - {os.name ?? notAvailableLabel} + + {os.name + ? os.name + : // https://github.com/i18next/next-i18next/issues/1795 + t("auditLog:table.logRow.notAvailable")} + diff --git a/site/src/i18n/en/auditLog.json b/site/src/i18n/en/auditLog.json index 441c091e21e7f..352b9ce93c20b 100644 --- a/site/src/i18n/en/auditLog.json +++ b/site/src/i18n/en/auditLog.json @@ -12,7 +12,8 @@ "buildFriendlyString": " for workspace ", "ip": "IP: ", "os": "OS: ", - "browser": "Browser: " + "browser": "Browser: ", + "notAvailable": "Not available" } } } From ddc0dc04b90e90be62158837389e551d7350a10f Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Mon, 28 Nov 2022 19:21:32 +0000 Subject: [PATCH 05/12] added support for group, template, user links --- coderd/audit.go | 17 ++++++---------- .../components/AuditLogRow/AuditLogRow.tsx | 20 ++++++++++++------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/coderd/audit.go b/coderd/audit.go index 812c0825f7866..9cee2970b4500 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -212,14 +212,12 @@ type AdditionalFields struct { func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAuditLogsOffsetRow) string { switch alog.ResourceType { - case database.ResourceTypeOrganization: - return "" case database.ResourceTypeTemplate: - return "" - case database.ResourceTypeTemplateVersion: - return "" + return fmt.Sprintf("/templates/%s", + alog.ResourceTarget) case database.ResourceTypeUser: - return "" + return fmt.Sprintf("/users?filter=%s", + alog.ResourceTarget) case database.ResourceTypeWorkspace: return fmt.Sprintf("/@%s/%s", alog.UserUsername.String, alog.ResourceTarget) @@ -232,12 +230,9 @@ func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAudit } return fmt.Sprintf("/@%s/%s/builds/%s", alog.UserUsername.String, additionalFields.WorkspaceName, additionalFields.BuildNumber) - case database.ResourceTypeGitSshKey: - return "" - case database.ResourceTypeApiKey: - return "" case database.ResourceTypeGroup: - return "" + return fmt.Sprintf("/groups/%s", + alog.ResourceID) default: return "" } diff --git a/site/src/components/AuditLogRow/AuditLogRow.tsx b/site/src/components/AuditLogRow/AuditLogRow.tsx index 943c98fffc181..94bebca689ada 100644 --- a/site/src/components/AuditLogRow/AuditLogRow.tsx +++ b/site/src/components/AuditLogRow/AuditLogRow.tsx @@ -19,15 +19,17 @@ import { Link as RouterLink } from "react-router-dom" import i18next from "i18next" import Link from "@material-ui/core/Link" -const determineResourceLink = (auditLog: AuditLog): string => { +const determineResourceLink = (auditLog: AuditLog): JSX.Element | undefined => { const { t } = i18next - let linkTarget = auditLog.resource_target.trim() + const linkTarget = auditLog.resource_target.trim() if (auditLog.resource_type === "workspace_build") { - linkTarget = t("auditLog:table.logRow.buildTarget") + return <>{t("auditLog:table.logRow.buildTarget")} + } else if (auditLog.resource_type === "git_ssh_key") { + return + } else { + return {linkTarget} } - - return linkTarget } export const readableActionMessage = (auditLog: AuditLog): string => { @@ -125,16 +127,20 @@ export const AuditLogRow: React.FC = ({ > {readableActionMessage(auditLog)}{" "} - {auditLog.resource_link && ( + {auditLog.resource_link ? ( {determineResourceLink(auditLog)} + ) : ( + {determineResourceLink(auditLog)} )} {auditLog.resource_type === "workspace_build" && auditLog.additional_fields.workspaceName && ( <> {t("auditLog:table.logRow.buildFriendlyString")} - {auditLog.additional_fields.workspaceName} + + {auditLog.additional_fields.workspaceName} + )} From 709e42e7c11c6ddf980f37056d7c81c921db8be5 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 30 Nov 2022 15:52:10 +0000 Subject: [PATCH 06/12] cleaned up string --- coderd/audit.go | 84 ++++++++++++++++--- .../provisionerdserver/provisionerdserver.go | 12 ++- codersdk/audit.go | 4 +- .../AuditLogRow/AuditLogDescription.tsx | 46 ++++++++++ .../components/AuditLogRow/AuditLogRow.tsx | 42 +--------- site/src/i18n/en/auditLog.json | 2 - 6 files changed, 130 insertions(+), 60 deletions(-) create mode 100644 site/src/components/AuditLogRow/AuditLogDescription.tsx diff --git a/coderd/audit.go b/coderd/audit.go index 9cee2970b4500..aac99f85d45cf 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -207,32 +207,84 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs type AdditionalFields struct { WorkspaceName string + WorkspaceID string BuildNumber string } +func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.GetAuditLogsOffsetRow) bool { + switch alog.ResourceType { + case database.ResourceTypeTemplate: + template, err := api.Database.GetTemplateByID(ctx, alog.ResourceID) + if err != nil { + api.Logger.Error(ctx, "could not get template", slog.Error(err)) + } + return template.Deleted + case database.ResourceTypeUser: + user, err := api.Database.GetUserByID(ctx, alog.ResourceID) + if err != nil { + api.Logger.Error(ctx, "could not get user", slog.Error(err)) + } + return user.Deleted + case database.ResourceTypeWorkspace: + workspace, err := api.Database.GetWorkspaceByID(ctx, alog.ResourceID) + if err != nil { + api.Logger.Error(ctx, "could not get workspace", slog.Error(err)) + } + return workspace.Deleted + case database.ResourceTypeWorkspaceBuild: + additionalFieldsBytes := []byte(alog.AdditionalFields) + var additionalFields AdditionalFields + err := json.Unmarshal(additionalFieldsBytes, &additionalFields) + if err != nil { + api.Logger.Error(ctx, "could not unmarshal workspace ID", slog.Error(err)) + } + // if we don't have a WorkspaceID, we return true so as to hide the link in the UI + if len(additionalFields.WorkspaceID) < 1 { + return true + } + // We use workspace as a proxy for workspace build here + workspace, err := api.Database.GetWorkspaceByID(ctx, uuid.MustParse(additionalFields.WorkspaceID)) + if err != nil { + api.Logger.Error(ctx, "could not get workspace", slog.Error(err)) + } + return workspace.Deleted + default: + return false + } +} + func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAuditLogsOffsetRow) string { switch alog.ResourceType { case database.ResourceTypeTemplate: + if api.auditLogIsResourceDeleted(ctx, alog) { + return "" + } return fmt.Sprintf("/templates/%s", alog.ResourceTarget) case database.ResourceTypeUser: + if api.auditLogIsResourceDeleted(ctx, alog) { + return "" + } return fmt.Sprintf("/users?filter=%s", alog.ResourceTarget) case database.ResourceTypeWorkspace: + if api.auditLogIsResourceDeleted(ctx, alog) { + return "" + } return fmt.Sprintf("/@%s/%s", alog.UserUsername.String, alog.ResourceTarget) case database.ResourceTypeWorkspaceBuild: + if api.auditLogIsResourceDeleted(ctx, alog) { + return "" + } additionalFieldsBytes := []byte(alog.AdditionalFields) var additionalFields AdditionalFields err := json.Unmarshal(additionalFieldsBytes, &additionalFields) if err != nil { - api.Logger.Error(ctx, "could not unmarshal workspace name for friendly string", slog.Error(err)) + api.Logger.Error(ctx, "could not unmarshal workspace name", slog.Error(err)) } return fmt.Sprintf("/@%s/%s/builds/%s", alog.UserUsername.String, additionalFields.WorkspaceName, additionalFields.BuildNumber) - case database.ResourceTypeGroup: - return fmt.Sprintf("/groups/%s", - alog.ResourceID) default: return "" } @@ -243,21 +295,27 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow) string { codersdk.AuditAction(alog.Action).FriendlyString(), ) - // Strings for workspace_builds follow the below format: - // "{user} started workspace build for {target}" - // where target is a workspace instead of the workspace build, + // Strings for starting/stopping workspace builds follow the below format: + // "{user} started build for workspace {target}" + // where target is a workspace instead of a workspace build // passed in on the FE via AuditLog.AdditionalFields rather than derived in request.go:35 - if alog.ResourceType != database.ResourceTypeWorkspaceBuild { - str += fmt.Sprintf(" %s", - codersdk.ResourceType(alog.ResourceType).FriendlyString()) + if alog.ResourceType == database.ResourceTypeWorkspaceBuild && alog.Action != database.AuditActionDelete { + str += " build for" } - // We don't display the name for git ssh keys. It's fairly long and doesn't + // We don't display the name (target) for git ssh keys. It's fairly long and doesn't // make too much sense to display. - if alog.ResourceType != database.ResourceTypeGitSshKey { - str += " {target}" + if alog.ResourceType == database.ResourceTypeGitSshKey { + str += fmt.Sprintf(" the %s", + codersdk.ResourceType(alog.ResourceType).FriendlyString()) + return str } + str += fmt.Sprintf(" %s", + codersdk.ResourceType(alog.ResourceType).FriendlyString()) + + str += " {target}" + return str } diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 51c622f7f172f..563af7b809e64 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -538,10 +538,12 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p if getWorkspaceErr != nil { server.Logger.Error(ctx, "failed to create audit log - get workspace err", slog.Error(err)) } else { - // We pass the workspace name to the Auditor so that it - // can form a friendly string for the user. + fmt.Println("Hello World! 1", workspace.ID.String()) + // We pass the below information to the Auditor so that it + // can form a friendly string for the user to view in the UI. workspaceResourceInfo := map[string]string{ "workspaceName": workspace.Name, + "workspaceId": workspace.ID.String(), "buildNumber": strconv.FormatInt(int64(build.BuildNumber), 10), } @@ -753,11 +755,13 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete if getWorkspaceError == nil { auditor := server.Auditor.Load() auditAction := auditActionFromTransition(workspaceBuild.Transition) + fmt.Println("Hello World! 2", workspace.ID.String()) - // We pass the workspace name to the Auditor so that it - // can form a friendly string for the user. + // We pass the below information to the Auditor so that it + // can form a friendly string for the user to view in the UI. workspaceResourceInfo := map[string]string{ "workspaceName": workspace.Name, + "workspaceId": workspace.ID.String(), "buildNumber": strconv.FormatInt(int64(workspaceBuild.BuildNumber), 10), } diff --git a/codersdk/audit.go b/codersdk/audit.go index 2b8e3ad3b3a45..a3693e798410a 100644 --- a/codersdk/audit.go +++ b/codersdk/audit.go @@ -38,7 +38,9 @@ func (r ResourceType) FriendlyString() string { case ResourceTypeWorkspace: return "workspace" case ResourceTypeWorkspaceBuild: - return "workspace build" + // workspace builds have a unique friendly string + // see coderd/audit.go:298 for explanation + return "workspace" case ResourceTypeGitSSHKey: return "git ssh key" case ResourceTypeAPIKey: diff --git a/site/src/components/AuditLogRow/AuditLogDescription.tsx b/site/src/components/AuditLogRow/AuditLogDescription.tsx new file mode 100644 index 0000000000000..6d9a7429147d5 --- /dev/null +++ b/site/src/components/AuditLogRow/AuditLogDescription.tsx @@ -0,0 +1,46 @@ +import { FC } from "react" +import { AuditLog } from "api/typesGenerated" +import { Link as RouterLink } from "react-router-dom" +import Link from "@material-ui/core/Link" + +export const AuditLogDescription: FC<{ auditLog: AuditLog }> = ({ + auditLog, +}): JSX.Element => { + let target = auditLog.resource_target.trim() + + // audit logs with a resource_type of workspace build use workspace name as a target + if ( + auditLog.resource_type === "workspace_build" && + auditLog.additional_fields.workspaceName + ) { + target = auditLog.additional_fields.workspaceName.trim() + } + + // SSH key entries have no links + if (auditLog.resource_type === "git_ssh_key") { + return ( + + {auditLog.description + .replace("{user}", `${auditLog.user?.username.trim()}`) + .replace("{target}", `${target}`)} + + ) + } + + const truncatedDescription = auditLog.description + .replace("{user}", `${auditLog.user?.username.trim()}`) + .replace("{target}", "") + + return ( + + {truncatedDescription} + {auditLog.resource_link ? ( + + {target} + + ) : ( + {target} + )} + + ) +} diff --git a/site/src/components/AuditLogRow/AuditLogRow.tsx b/site/src/components/AuditLogRow/AuditLogRow.tsx index 94bebca689ada..07c7a6aa5fd6a 100644 --- a/site/src/components/AuditLogRow/AuditLogRow.tsx +++ b/site/src/components/AuditLogRow/AuditLogRow.tsx @@ -15,28 +15,8 @@ import { PaletteIndex } from "theme/palettes" import userAgentParser from "ua-parser-js" import { combineClasses } from "util/combineClasses" import { AuditLogDiff } from "./AuditLogDiff" -import { Link as RouterLink } from "react-router-dom" import i18next from "i18next" -import Link from "@material-ui/core/Link" - -const determineResourceLink = (auditLog: AuditLog): JSX.Element | undefined => { - const { t } = i18next - const linkTarget = auditLog.resource_target.trim() - - if (auditLog.resource_type === "workspace_build") { - return <>{t("auditLog:table.logRow.buildTarget")} - } else if (auditLog.resource_type === "git_ssh_key") { - return - } else { - return {linkTarget} - } -} - -export const readableActionMessage = (auditLog: AuditLog): string => { - return auditLog.description - .replace("{user}", `${auditLog.user?.username.trim()}`) - .replace("{target}", "") -} +import { AuditLogDescription } from "./AuditLogDescription" const httpStatusColor = (httpStatus: number): PaletteIndex => { if (httpStatus >= 300 && httpStatus < 500) { @@ -125,25 +105,7 @@ export const AuditLogRow: React.FC = ({ alignItems="baseline" spacing={1} > - - {readableActionMessage(auditLog)}{" "} - {auditLog.resource_link ? ( - - {determineResourceLink(auditLog)} - - ) : ( - {determineResourceLink(auditLog)} - )} - {auditLog.resource_type === "workspace_build" && - auditLog.additional_fields.workspaceName && ( - <> - {t("auditLog:table.logRow.buildFriendlyString")} - - {auditLog.additional_fields.workspaceName} - - - )} - + {new Date(auditLog.time).toLocaleTimeString()} diff --git a/site/src/i18n/en/auditLog.json b/site/src/i18n/en/auditLog.json index 352b9ce93c20b..393a862ad50d9 100644 --- a/site/src/i18n/en/auditLog.json +++ b/site/src/i18n/en/auditLog.json @@ -8,8 +8,6 @@ "emptyPage": "No audit logs available on this page", "noLogs": "No audit logs available", "logRow": { - "buildTarget": "build", - "buildFriendlyString": " for workspace ", "ip": "IP: ", "os": "OS: ", "browser": "Browser: ", From d5eb06a13867370ef8f729cf9ebf88af8948a636 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 30 Nov 2022 16:26:32 +0000 Subject: [PATCH 07/12] added deleted label --- coderd/audit.go | 85 +++++++++---------- codersdk/audit.go | 1 + site/src/api/typesGenerated.ts | 1 + .../AuditLogRow/AuditLogDescription.tsx | 17 ++++ site/src/i18n/en/auditLog.json | 1 + 5 files changed, 59 insertions(+), 46 deletions(-) diff --git a/coderd/audit.go b/coderd/audit.go index aac99f85d45cf..d93038796626a 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -199,10 +199,40 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs Diff: diff, StatusCode: dblog.StatusCode, AdditionalFields: dblog.AdditionalFields, - Description: auditLogDescription(dblog), User: user, + Description: auditLogDescription(dblog), ResourceLink: api.auditLogResourceLink(ctx, dblog), + IsDeleted: api.auditLogIsResourceDeleted(ctx, dblog), + } +} + +func auditLogDescription(alog database.GetAuditLogsOffsetRow) string { + str := fmt.Sprintf("{user} %s", + codersdk.AuditAction(alog.Action).FriendlyString(), + ) + + // Strings for starting/stopping workspace builds follow the below format: + // "{user} started build for workspace {target}" + // where target is a workspace instead of a workspace build + // passed in on the FE via AuditLog.AdditionalFields rather than derived in request.go:35 + if alog.ResourceType == database.ResourceTypeWorkspaceBuild && alog.Action != database.AuditActionDelete { + str += " build for" + } + + // We don't display the name (target) for git ssh keys. It's fairly long and doesn't + // make too much sense to display. + if alog.ResourceType == database.ResourceTypeGitSshKey { + str += fmt.Sprintf(" the %s", + codersdk.ResourceType(alog.ResourceType).FriendlyString()) + return str } + + str += fmt.Sprintf(" %s", + codersdk.ResourceType(alog.ResourceType).FriendlyString()) + + str += " {target}" + + return str } type AdditionalFields struct { @@ -216,19 +246,19 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get case database.ResourceTypeTemplate: template, err := api.Database.GetTemplateByID(ctx, alog.ResourceID) if err != nil { - api.Logger.Error(ctx, "could not get template", slog.Error(err)) + api.Logger.Error(ctx, "could not fetch template", slog.Error(err)) } return template.Deleted case database.ResourceTypeUser: user, err := api.Database.GetUserByID(ctx, alog.ResourceID) if err != nil { - api.Logger.Error(ctx, "could not get user", slog.Error(err)) + api.Logger.Error(ctx, "could not fetch user", slog.Error(err)) } return user.Deleted case database.ResourceTypeWorkspace: workspace, err := api.Database.GetWorkspaceByID(ctx, alog.ResourceID) if err != nil { - api.Logger.Error(ctx, "could not get workspace", slog.Error(err)) + api.Logger.Error(ctx, "could not fetch workspace", slog.Error(err)) } return workspace.Deleted case database.ResourceTypeWorkspaceBuild: @@ -245,7 +275,7 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get // We use workspace as a proxy for workspace build here workspace, err := api.Database.GetWorkspaceByID(ctx, uuid.MustParse(additionalFields.WorkspaceID)) if err != nil { - api.Logger.Error(ctx, "could not get workspace", slog.Error(err)) + api.Logger.Error(ctx, "could not fetch workspace", slog.Error(err)) } return workspace.Deleted default: @@ -254,29 +284,21 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get } func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAuditLogsOffsetRow) string { + if api.auditLogIsResourceDeleted(ctx, alog) { + return "" + } + switch alog.ResourceType { case database.ResourceTypeTemplate: - if api.auditLogIsResourceDeleted(ctx, alog) { - return "" - } return fmt.Sprintf("/templates/%s", alog.ResourceTarget) case database.ResourceTypeUser: - if api.auditLogIsResourceDeleted(ctx, alog) { - return "" - } return fmt.Sprintf("/users?filter=%s", alog.ResourceTarget) case database.ResourceTypeWorkspace: - if api.auditLogIsResourceDeleted(ctx, alog) { - return "" - } return fmt.Sprintf("/@%s/%s", alog.UserUsername.String, alog.ResourceTarget) case database.ResourceTypeWorkspaceBuild: - if api.auditLogIsResourceDeleted(ctx, alog) { - return "" - } additionalFieldsBytes := []byte(alog.AdditionalFields) var additionalFields AdditionalFields err := json.Unmarshal(additionalFieldsBytes, &additionalFields) @@ -290,35 +312,6 @@ func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAudit } } -func auditLogDescription(alog database.GetAuditLogsOffsetRow) string { - str := fmt.Sprintf("{user} %s", - codersdk.AuditAction(alog.Action).FriendlyString(), - ) - - // Strings for starting/stopping workspace builds follow the below format: - // "{user} started build for workspace {target}" - // where target is a workspace instead of a workspace build - // passed in on the FE via AuditLog.AdditionalFields rather than derived in request.go:35 - if alog.ResourceType == database.ResourceTypeWorkspaceBuild && alog.Action != database.AuditActionDelete { - str += " build for" - } - - // We don't display the name (target) for git ssh keys. It's fairly long and doesn't - // make too much sense to display. - if alog.ResourceType == database.ResourceTypeGitSshKey { - str += fmt.Sprintf(" the %s", - codersdk.ResourceType(alog.ResourceType).FriendlyString()) - return str - } - - str += fmt.Sprintf(" %s", - codersdk.ResourceType(alog.ResourceType).FriendlyString()) - - str += " {target}" - - return str -} - // auditSearchQuery takes a query string and returns the auditLog filter. // It also can return the list of validation errors to return to the api. func auditSearchQuery(query string) (database.GetAuditLogsOffsetParams, []codersdk.ValidationError) { diff --git a/codersdk/audit.go b/codersdk/audit.go index a3693e798410a..e3d25851ea89f 100644 --- a/codersdk/audit.go +++ b/codersdk/audit.go @@ -105,6 +105,7 @@ type AuditLog struct { AdditionalFields json.RawMessage `json:"additional_fields"` Description string `json:"description"` ResourceLink string `json:"resource_link"` + IsDeleted bool `json:"is_deleted"` User *User `json:"user"` } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 5ecb29a1c7ed9..d0b7b42d47d5c 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -68,6 +68,7 @@ export interface AuditLog { readonly additional_fields: Record readonly description: string readonly resource_link: string + readonly is_deleted: boolean readonly user?: User } diff --git a/site/src/components/AuditLogRow/AuditLogDescription.tsx b/site/src/components/AuditLogRow/AuditLogDescription.tsx index 6d9a7429147d5..240f40acea552 100644 --- a/site/src/components/AuditLogRow/AuditLogDescription.tsx +++ b/site/src/components/AuditLogRow/AuditLogDescription.tsx @@ -2,10 +2,15 @@ import { FC } from "react" import { AuditLog } from "api/typesGenerated" import { Link as RouterLink } from "react-router-dom" import Link from "@material-ui/core/Link" +import { makeStyles } from "@material-ui/core/styles" +import i18next from "i18next" export const AuditLogDescription: FC<{ auditLog: AuditLog }> = ({ auditLog, }): JSX.Element => { + const classes = useStyles() + const { t } = i18next + let target = auditLog.resource_target.trim() // audit logs with a resource_type of workspace build use workspace name as a target @@ -41,6 +46,18 @@ export const AuditLogDescription: FC<{ auditLog: AuditLog }> = ({ ) : ( {target} )} + {auditLog.is_deleted && ( + + <> {t("auditLog:table.logRow.deletedLabel")} + + )} ) } + +const useStyles = makeStyles((theme) => ({ + deletedLabel: { + ...theme.typography.caption, + color: theme.palette.text.secondary, + }, +})) diff --git a/site/src/i18n/en/auditLog.json b/site/src/i18n/en/auditLog.json index 393a862ad50d9..b68d33ab5fcb1 100644 --- a/site/src/i18n/en/auditLog.json +++ b/site/src/i18n/en/auditLog.json @@ -8,6 +8,7 @@ "emptyPage": "No audit logs available on this page", "noLogs": "No audit logs available", "logRow": { + "deletedLabel": "(deleted)", "ip": "IP: ", "os": "OS: ", "browser": "Browser: ", From 221f089e6193356f5261a7fb35e492ccf5d103c3 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 30 Nov 2022 16:48:32 +0000 Subject: [PATCH 08/12] querying for workspace id --- coderd/audit.go | 23 +++++++------------ .../provisionerdserver/provisionerdserver.go | 2 -- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/coderd/audit.go b/coderd/audit.go index d93038796626a..d2064418b9e94 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -235,12 +235,6 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow) string { return str } -type AdditionalFields struct { - WorkspaceName string - WorkspaceID string - BuildNumber string -} - func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.GetAuditLogsOffsetRow) bool { switch alog.ResourceType { case database.ResourceTypeTemplate: @@ -262,18 +256,12 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get } return workspace.Deleted case database.ResourceTypeWorkspaceBuild: - additionalFieldsBytes := []byte(alog.AdditionalFields) - var additionalFields AdditionalFields - err := json.Unmarshal(additionalFieldsBytes, &additionalFields) + workspaceBuild, err := api.Database.GetWorkspaceBuildByID(ctx, alog.ResourceID) if err != nil { - api.Logger.Error(ctx, "could not unmarshal workspace ID", slog.Error(err)) - } - // if we don't have a WorkspaceID, we return true so as to hide the link in the UI - if len(additionalFields.WorkspaceID) < 1 { - return true + api.Logger.Error(ctx, "could not fetch workspace build", slog.Error(err)) } // We use workspace as a proxy for workspace build here - workspace, err := api.Database.GetWorkspaceByID(ctx, uuid.MustParse(additionalFields.WorkspaceID)) + workspace, err := api.Database.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID) if err != nil { api.Logger.Error(ctx, "could not fetch workspace", slog.Error(err)) } @@ -283,6 +271,11 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get } } +type AdditionalFields struct { + WorkspaceName string + BuildNumber string +} + func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAuditLogsOffsetRow) string { if api.auditLogIsResourceDeleted(ctx, alog) { return "" diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 563af7b809e64..a5ef6a0a130dc 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -543,7 +543,6 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p // can form a friendly string for the user to view in the UI. workspaceResourceInfo := map[string]string{ "workspaceName": workspace.Name, - "workspaceId": workspace.ID.String(), "buildNumber": strconv.FormatInt(int64(build.BuildNumber), 10), } @@ -761,7 +760,6 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete // can form a friendly string for the user to view in the UI. workspaceResourceInfo := map[string]string{ "workspaceName": workspace.Name, - "workspaceId": workspace.ID.String(), "buildNumber": strconv.FormatInt(int64(workspaceBuild.BuildNumber), 10), } From 8177557a58a5abde126315c954c1e9b632ad4ce2 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 30 Nov 2022 19:51:50 +0000 Subject: [PATCH 09/12] remove prints --- coderd/provisionerdserver/provisionerdserver.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index a5ef6a0a130dc..1795ef4bf8d3e 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -538,7 +538,6 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p if getWorkspaceErr != nil { server.Logger.Error(ctx, "failed to create audit log - get workspace err", slog.Error(err)) } else { - fmt.Println("Hello World! 1", workspace.ID.String()) // We pass the below information to the Auditor so that it // can form a friendly string for the user to view in the UI. workspaceResourceInfo := map[string]string{ @@ -754,7 +753,6 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete if getWorkspaceError == nil { auditor := server.Auditor.Load() auditAction := auditActionFromTransition(workspaceBuild.Transition) - fmt.Println("Hello World! 2", workspace.ID.String()) // We pass the below information to the Auditor so that it // can form a friendly string for the user to view in the UI. From d1ba2b92989a66d5bbec584464aef467bb59ffba Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 30 Nov 2022 20:59:37 +0000 Subject: [PATCH 10/12] fix/write tests --- coderd/audit_test.go | 25 ++++++---- .../AuditLogRow/AuditLogDescription.test.tsx | 49 +++++++++++++++++++ .../AuditLogRow/AuditLogRow.test.tsx | 41 ---------------- site/src/testHelpers/entities.ts | 4 +- 4 files changed, 67 insertions(+), 52 deletions(-) create mode 100644 site/src/components/AuditLogRow/AuditLogDescription.test.tsx delete mode 100644 site/src/components/AuditLogRow/AuditLogRow.test.tsx diff --git a/coderd/audit_test.go b/coderd/audit_test.go index 1fbcf7814f8e0..5495235d954e5 100644 --- a/coderd/audit_test.go +++ b/coderd/audit_test.go @@ -5,7 +5,6 @@ import ( "testing" "time" - "github.com/google/uuid" "github.com/stretchr/testify/require" "github.com/coder/coder/coderd/coderdtest" @@ -20,9 +19,11 @@ func TestAuditLogs(t *testing.T) { ctx := context.Background() client := coderdtest.New(t, nil) - _ = coderdtest.CreateFirstUser(t, client) + user := coderdtest.CreateFirstUser(t, client) - err := client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{}) + err := client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{ + ResourceID: user.UserID, + }) require.NoError(t, err) alogs, err := client.AuditLogs(ctx, codersdk.AuditLogsRequest{ @@ -43,22 +44,26 @@ func TestAuditLogsFilter(t *testing.T) { t.Run("Filter", func(t *testing.T) { t.Parallel() - ctx := context.Background() - client := coderdtest.New(t, nil) - _ = coderdtest.CreateFirstUser(t, client) - userResourceID := uuid.New() + var ( + ctx = context.Background() + client = coderdtest.New(t, nil) + user = coderdtest.CreateFirstUser(t, client) + version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + template = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + ) // Create two logs with "Create" err := client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{ Action: codersdk.AuditActionCreate, ResourceType: codersdk.ResourceTypeTemplate, + ResourceID: template.ID, Time: time.Date(2022, 8, 15, 14, 30, 45, 100, time.UTC), // 2022-8-15 14:30:45 }) require.NoError(t, err) err = client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{ Action: codersdk.AuditActionCreate, ResourceType: codersdk.ResourceTypeUser, - ResourceID: userResourceID, + ResourceID: user.UserID, Time: time.Date(2022, 8, 16, 14, 30, 45, 100, time.UTC), // 2022-8-16 14:30:45 }) require.NoError(t, err) @@ -67,7 +72,7 @@ func TestAuditLogsFilter(t *testing.T) { err = client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{ Action: codersdk.AuditActionDelete, ResourceType: codersdk.ResourceTypeUser, - ResourceID: userResourceID, + ResourceID: user.UserID, Time: time.Date(2022, 8, 15, 14, 30, 45, 100, time.UTC), // 2022-8-15 14:30:45 }) require.NoError(t, err) @@ -110,7 +115,7 @@ func TestAuditLogsFilter(t *testing.T) { }, { Name: "FilterByResourceID", - SearchQuery: "resource_id:" + userResourceID.String(), + SearchQuery: "resource_id:" + user.UserID.String(), ExpectedResult: 2, }, { diff --git a/site/src/components/AuditLogRow/AuditLogDescription.test.tsx b/site/src/components/AuditLogRow/AuditLogDescription.test.tsx new file mode 100644 index 0000000000000..cf8801678cca6 --- /dev/null +++ b/site/src/components/AuditLogRow/AuditLogDescription.test.tsx @@ -0,0 +1,49 @@ +import { + MockAuditLog, + MockAuditLogWithWorkspaceBuild, +} from "testHelpers/entities" +import { AuditLogDescription } from "./AuditLogDescription" +import { render } from "../../testHelpers/renderHelpers" +import { screen } from "@testing-library/react" + +const getByTextContent = (text: string) => { + return screen.getByText((_, element) => { + const hasText = (element: Element | null) => element?.textContent === text + const elementHasText = hasText(element) + const childrenDontHaveText = Array.from(element?.children || []).every( + (child) => !hasText(child), + ) + return elementHasText && childrenDontHaveText + }) +} +describe("AuditLogDescription", () => { + it("renders the correct string for a workspace create audit log", async () => { + render() + + expect( + getByTextContent("TestUser created workspace bruno-dev"), + ).toBeDefined() + }) + + it("renders the correct string for a workspace_build stop audit log", async () => { + render() + + expect( + getByTextContent("TestUser stopped build for workspace test2"), + ).toBeDefined() + }) + + it("renders the correct string for a workspace_build audit log with a duplicate word", async () => { + const AuditLogWithRepeat = { + ...MockAuditLogWithWorkspaceBuild, + additional_fields: { + workspaceName: "workspace", + }, + } + render() + + expect( + getByTextContent("TestUser stopped build for workspace workspace"), + ).toBeDefined() + }) +}) diff --git a/site/src/components/AuditLogRow/AuditLogRow.test.tsx b/site/src/components/AuditLogRow/AuditLogRow.test.tsx deleted file mode 100644 index ab0152730161a..0000000000000 --- a/site/src/components/AuditLogRow/AuditLogRow.test.tsx +++ /dev/null @@ -1,41 +0,0 @@ -import { readableActionMessage } from "./AuditLogRow" -import { - MockAuditLog, - MockAuditLogWithWorkspaceBuild, -} from "testHelpers/entities" - -describe("readableActionMessage()", () => { - it("renders the correct string for a workspaceBuild audit log", async () => { - // When - const friendlyString = readableActionMessage(MockAuditLogWithWorkspaceBuild) - - // Then - expect(friendlyString).toBe( - "TestUser stopped workspace build for test2", - ) - }) - it("renders the correct string for a workspaceBuild audit log with a duplicate word", async () => { - // When - const AuditLogWithRepeat = { - ...MockAuditLogWithWorkspaceBuild, - additional_fields: { - workspaceName: "workspace", - }, - } - const friendlyString = readableActionMessage(AuditLogWithRepeat) - - // Then - expect(friendlyString).toBe( - "TestUser stopped workspace build for workspace", - ) - }) - it("renders the correct string for a workspace audit log", async () => { - // When - const friendlyString = readableActionMessage(MockAuditLog) - - // Then - expect(friendlyString).toBe( - "TestUser created workspace bruno-dev", - ) - }) -}) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 482f6e1d47b29..efcf975d4599a 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -961,6 +961,8 @@ export const MockAuditLog: TypesGen.AuditLog = { additional_fields: {}, description: "{user} created workspace {target}", user: MockUser, + resource_link: "", + is_deleted: false, } export const MockAuditLog2: TypesGen.AuditLog = { @@ -999,7 +1001,7 @@ export const MockAuditLogWithWorkspaceBuild: TypesGen.AuditLog = { request_id: "61555889-2875-475c-8494-f7693dd5d75b", action: "stop", resource_type: "workspace_build", - description: "{user} stopped workspace build for {target}", + description: "{user} stopped build for workspace {target}", additional_fields: { workspaceName: "test2", }, From 93533422db8f83803126a44c08dee9a08efaa316 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Thu, 1 Dec 2022 19:14:41 +0000 Subject: [PATCH 11/12] PR feedback pt 1 --- coderd/audit.go | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/coderd/audit.go b/coderd/audit.go index d2064418b9e94..e6a617fbcf868 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -14,6 +14,7 @@ import ( "github.com/google/uuid" "github.com/tabbed/pqtype" + "golang.org/x/xerrors" "cdr.dev/slog" "github.com/coder/coder/coderd/database" @@ -240,30 +241,50 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get case database.ResourceTypeTemplate: template, err := api.Database.GetTemplateByID(ctx, alog.ResourceID) if err != nil { - api.Logger.Error(ctx, "could not fetch template", slog.Error(err)) + if xerrors.Is(err, sql.ErrNoRows) { + return true + } else { + api.Logger.Error(ctx, "fetch template", slog.Error(err)) + } } return template.Deleted case database.ResourceTypeUser: user, err := api.Database.GetUserByID(ctx, alog.ResourceID) if err != nil { - api.Logger.Error(ctx, "could not fetch user", slog.Error(err)) + if xerrors.Is(err, sql.ErrNoRows) { + return true + } else { + api.Logger.Error(ctx, "fetch user", slog.Error(err)) + } } return user.Deleted case database.ResourceTypeWorkspace: workspace, err := api.Database.GetWorkspaceByID(ctx, alog.ResourceID) if err != nil { - api.Logger.Error(ctx, "could not fetch workspace", slog.Error(err)) + if xerrors.Is(err, sql.ErrNoRows) { + return true + } else { + api.Logger.Error(ctx, "fetch workspace", slog.Error(err)) + } } return workspace.Deleted case database.ResourceTypeWorkspaceBuild: workspaceBuild, err := api.Database.GetWorkspaceBuildByID(ctx, alog.ResourceID) if err != nil { - api.Logger.Error(ctx, "could not fetch workspace build", slog.Error(err)) + if xerrors.Is(err, sql.ErrNoRows) { + return true + } else { + api.Logger.Error(ctx, "fetch workspace build", slog.Error(err)) + } } // We use workspace as a proxy for workspace build here workspace, err := api.Database.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID) if err != nil { - api.Logger.Error(ctx, "could not fetch workspace", slog.Error(err)) + if xerrors.Is(err, sql.ErrNoRows) { + return true + } else { + api.Logger.Error(ctx, "fetch workspace", slog.Error(err)) + } } return workspace.Deleted default: @@ -296,7 +317,7 @@ func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAudit var additionalFields AdditionalFields err := json.Unmarshal(additionalFieldsBytes, &additionalFields) if err != nil { - api.Logger.Error(ctx, "could not unmarshal workspace name", slog.Error(err)) + api.Logger.Error(ctx, "unmarshal workspace name", slog.Error(err)) } return fmt.Sprintf("/@%s/%s/builds/%s", alog.UserUsername.String, additionalFields.WorkspaceName, additionalFields.BuildNumber) From 6465f3e1d425d4aa7f59e0cfc09874022bb0f6a7 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Fri, 2 Dec 2022 18:55:30 +0000 Subject: [PATCH 12/12] PR feedback part 2 --- coderd/audit.go | 31 +++++++++---------- .../AuditLogRow/AuditLogRow.stories.tsx | 6 ++++ site/src/testHelpers/entities.ts | 7 ++++- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/coderd/audit.go b/coderd/audit.go index e6a617fbcf868..2228af28c43e4 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -185,6 +185,14 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs } } + isDeleted := api.auditLogIsResourceDeleted(ctx, dblog) + var resourceLink string + if isDeleted { + resourceLink = "" + } else { + resourceLink = api.auditLogResourceLink(ctx, dblog) + } + return codersdk.AuditLog{ ID: dblog.ID, RequestID: dblog.RequestID, @@ -202,8 +210,8 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs AdditionalFields: dblog.AdditionalFields, User: user, Description: auditLogDescription(dblog), - ResourceLink: api.auditLogResourceLink(ctx, dblog), - IsDeleted: api.auditLogIsResourceDeleted(ctx, dblog), + ResourceLink: resourceLink, + IsDeleted: isDeleted, } } @@ -243,9 +251,8 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get if err != nil { if xerrors.Is(err, sql.ErrNoRows) { return true - } else { - api.Logger.Error(ctx, "fetch template", slog.Error(err)) } + api.Logger.Error(ctx, "fetch template", slog.Error(err)) } return template.Deleted case database.ResourceTypeUser: @@ -253,9 +260,8 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get if err != nil { if xerrors.Is(err, sql.ErrNoRows) { return true - } else { - api.Logger.Error(ctx, "fetch user", slog.Error(err)) } + api.Logger.Error(ctx, "fetch user", slog.Error(err)) } return user.Deleted case database.ResourceTypeWorkspace: @@ -263,9 +269,8 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get if err != nil { if xerrors.Is(err, sql.ErrNoRows) { return true - } else { - api.Logger.Error(ctx, "fetch workspace", slog.Error(err)) } + api.Logger.Error(ctx, "fetch workspace", slog.Error(err)) } return workspace.Deleted case database.ResourceTypeWorkspaceBuild: @@ -273,18 +278,16 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get if err != nil { if xerrors.Is(err, sql.ErrNoRows) { return true - } else { - api.Logger.Error(ctx, "fetch workspace build", slog.Error(err)) } + api.Logger.Error(ctx, "fetch workspace build", slog.Error(err)) } // We use workspace as a proxy for workspace build here workspace, err := api.Database.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID) if err != nil { if xerrors.Is(err, sql.ErrNoRows) { return true - } else { - api.Logger.Error(ctx, "fetch workspace", slog.Error(err)) } + api.Logger.Error(ctx, "fetch workspace", slog.Error(err)) } return workspace.Deleted default: @@ -298,10 +301,6 @@ type AdditionalFields struct { } func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAuditLogsOffsetRow) string { - if api.auditLogIsResourceDeleted(ctx, alog) { - return "" - } - switch alog.ResourceType { case database.ResourceTypeTemplate: return fmt.Sprintf("/templates/%s", diff --git a/site/src/components/AuditLogRow/AuditLogRow.stories.tsx b/site/src/components/AuditLogRow/AuditLogRow.stories.tsx index 8594a1a06b4a4..7d73efd47b3e5 100644 --- a/site/src/components/AuditLogRow/AuditLogRow.stories.tsx +++ b/site/src/components/AuditLogRow/AuditLogRow.stories.tsx @@ -9,6 +9,7 @@ import { MockAuditLog, MockAuditLog2, MockAuditLogWithWorkspaceBuild, + MockAuditLogWithDeletedResource, } from "testHelpers/entities" import { AuditLogRow, AuditLogRowProps } from "./AuditLogRow" @@ -66,3 +67,8 @@ export const WithWorkspaceBuild = Template.bind({}) WithWorkspaceBuild.args = { auditLog: MockAuditLogWithWorkspaceBuild, } + +export const DeletedResource = Template.bind({}) +DeletedResource.args = { + auditLog: MockAuditLogWithDeletedResource, +} diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index efcf975d4599a..dfa73911085bb 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -961,7 +961,7 @@ export const MockAuditLog: TypesGen.AuditLog = { additional_fields: {}, description: "{user} created workspace {target}", user: MockUser, - resource_link: "", + resource_link: "/@admin/bruno-dev", is_deleted: false, } @@ -1007,6 +1007,11 @@ export const MockAuditLogWithWorkspaceBuild: TypesGen.AuditLog = { }, } +export const MockAuditLogWithDeletedResource: TypesGen.AuditLog = { + ...MockAuditLog, + is_deleted: true, +} + export const MockWorkspaceQuota: TypesGen.WorkspaceQuota = { credits_consumed: 0, budget: 100,