diff --git a/coderd/audit.go b/coderd/audit.go index e177494d6d934..2228af28c43e4 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -1,6 +1,7 @@ package coderd import ( + "context" "database/sql" "encoding/json" "fmt" @@ -13,7 +14,9 @@ import ( "github.com/google/uuid" "github.com/tabbed/pqtype" + "golang.org/x/xerrors" + "cdr.dev/slog" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" @@ -69,7 +72,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 +150,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{} @@ -182,6 +185,14 @@ func convertAuditLog(dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog { } } + 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, @@ -197,34 +208,123 @@ func convertAuditLog(dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog { Diff: diff, StatusCode: dblog.StatusCode, AdditionalFields: dblog.AdditionalFields, - Description: auditLogDescription(dblog), User: user, + Description: auditLogDescription(dblog), + ResourceLink: resourceLink, + IsDeleted: isDeleted, } } 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, + // 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 += " for" + 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 } +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 { + if xerrors.Is(err, sql.ErrNoRows) { + return true + } + 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 { + if xerrors.Is(err, sql.ErrNoRows) { + return true + } + 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 { + if xerrors.Is(err, sql.ErrNoRows) { + return true + } + 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 { + if xerrors.Is(err, sql.ErrNoRows) { + return true + } + 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 + } + api.Logger.Error(ctx, "fetch workspace", slog.Error(err)) + } + return workspace.Deleted + default: + return false + } +} + +type AdditionalFields struct { + WorkspaceName string + BuildNumber string +} + +func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAuditLogsOffsetRow) string { + switch alog.ResourceType { + case database.ResourceTypeTemplate: + return fmt.Sprintf("/templates/%s", + alog.ResourceTarget) + case database.ResourceTypeUser: + return fmt.Sprintf("/users?filter=%s", + alog.ResourceTarget) + 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, "unmarshal workspace name", slog.Error(err)) + } + return fmt.Sprintf("/@%s/%s/builds/%s", + alog.UserUsername.String, additionalFields.WorkspaceName, additionalFields.BuildNumber) + default: + return "" + } +} + // 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/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/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 0c3705f719c81..0ee94d2f96ab7 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" @@ -537,10 +538,11 @@ 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. + // 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, + "buildNumber": strconv.FormatInt(int64(build.BuildNumber), 10), } wriBytes, err := json.Marshal(workspaceResourceInfo) @@ -752,10 +754,11 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete auditor := server.Auditor.Load() auditAction := auditActionFromTransition(workspaceBuild.Transition) - // 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, + "buildNumber": strconv.FormatInt(int64(workspaceBuild.BuildNumber), 10), } wriBytes, err := json.Marshal(workspaceResourceInfo) diff --git a/codersdk/audit.go b/codersdk/audit.go index f958411501599..e3d25851ea89f 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: @@ -102,6 +104,8 @@ type AuditLog struct { StatusCode int32 `json:"status_code"` 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 4ff9b8e875cf6..0e9369e0e08bb 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -67,6 +67,8 @@ export interface AuditLog { readonly status_code: number 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.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/AuditLogDescription.tsx b/site/src/components/AuditLogRow/AuditLogDescription.tsx new file mode 100644 index 0000000000000..240f40acea552 --- /dev/null +++ b/site/src/components/AuditLogRow/AuditLogDescription.tsx @@ -0,0 +1,63 @@ +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 + 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} + )} + {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/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/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/components/AuditLogRow/AuditLogRow.tsx b/site/src/components/AuditLogRow/AuditLogRow.tsx index dcea7193e04e7..991f0e5303bae 100644 --- a/site/src/components/AuditLogRow/AuditLogRow.tsx +++ b/site/src/components/AuditLogRow/AuditLogRow.tsx @@ -14,22 +14,8 @@ import { useState } from "react" import { PaletteIndex } from "theme/palettes" import userAgentParser from "ua-parser-js" import { AuditLogDiff } from "./AuditLogDiff" - -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}`) -} +import i18next from "i18next" +import { AuditLogDescription } from "./AuditLogDescription" const httpStatusColor = (httpStatus: number): PaletteIndex => { if (httpStatus >= 300 && httpStatus < 500) { @@ -54,14 +40,14 @@ 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 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) { @@ -115,11 +101,7 @@ export const AuditLogRow: React.FC = ({ alignItems="baseline" spacing={1} > - + {new Date(auditLog.time).toLocaleTimeString()} @@ -128,15 +110,27 @@ export const AuditLogRow: React.FC = ({ - IP: {auditLog.ip ?? notAvailableLabel} + <>{t("auditLog:table.logRow.ip")} + + {auditLog.ip + ? auditLog.ip + : t("auditLog:table.logRow.notAvailable")} + - OS: {os.name ?? notAvailableLabel} + <>{t("auditLog:table.logRow.os")} + + {os.name + ? os.name + : // https://github.com/i18next/next-i18next/issues/1795 + t("auditLog:table.logRow.notAvailable")} + - 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..b68d33ab5fcb1 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": { + "deletedLabel": "(deleted)", + "ip": "IP: ", + "os": "OS: ", + "browser": "Browser: ", + "notAvailable": "Not available" + } } } diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 3ee8993068408..725383b3f3c98 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -986,6 +986,8 @@ export const MockAuditLog: TypesGen.AuditLog = { additional_fields: {}, description: "{user} created workspace {target}", user: MockUser, + resource_link: "/@admin/bruno-dev", + is_deleted: false, } export const MockAuditLog2: TypesGen.AuditLog = { @@ -1024,12 +1026,17 @@ 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", }, } +export const MockAuditLogWithDeletedResource: TypesGen.AuditLog = { + ...MockAuditLog, + is_deleted: true, +} + export const MockWorkspaceQuota: TypesGen.WorkspaceQuota = { credits_consumed: 0, budget: 100,