fix: audit log broken build links (coder#5895)

Kira-Pilot · Emyrk · web-flow · commit b31b0fd189da · 2023-01-30T18:43:29.000Z
* pushing for guidance

* added test

* PR feedback

* fixed tests

* Update coderd/audit.go

Co-authored-by: Steven Masley &lt;Emyrk@users.noreply.github.com&gt;

* runnig make gen

---------

Co-authored-by: Steven Masley &lt;Emyrk@users.noreply.github.com&gt;
diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go
diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json
diff --git a/coderd/audit.go b/coderd/audit.go
@@ -17,6 +17,7 @@ import (
 	"golang.org/x/xerrors"
 
 	"cdr.dev/slog"
+	"github.com/coder/coder/coderd/audit"
 	"github.com/coder/coder/coderd/database"
 	"github.com/coder/coder/coderd/httpapi"
 	"github.com/coder/coder/coderd/httpmw"
@@ -147,6 +148,9 @@ func (api *API) generateFakeAuditLog(rw http.ResponseWriter, r *http.Request) {
 	if params.Time.IsZero() {
 		params.Time = time.Now()
 	}
+	if len(params.AdditionalFields) == 0 {
+		params.AdditionalFields = json.RawMessage("{}")
+	}
 
 	_, err = api.Database.InsertAuditLog(ctx, database.InsertAuditLogParams{
 		ID:               uuid.New(),
@@ -160,7 +164,7 @@ func (api *API) generateFakeAuditLog(rw http.ResponseWriter, r *http.Request) {
 		Action:           database.AuditAction(params.Action),
 		Diff:             diff,
 		StatusCode:       http.StatusOK,
-		AdditionalFields: []byte("{}"),
+		AdditionalFields: params.AdditionalFields,
 	})
 	if err != nil {
 		httpapi.InternalServerError(rw, err)
@@ -180,12 +184,6 @@ func (api *API) convertAuditLogs(ctx context.Context, dblogs []database.GetAudit
 	return alogs
 }
 
-type AdditionalFields struct {
-	WorkspaceName string               `json:"workspace_name"`
-	BuildNumber   string               `json:"build_number"`
-	BuildReason   database.BuildReason `json:"build_reason"`
-}
-
 func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog {
 	ip, _ := netip.AddrFromSlice(dblog.Ip.IPNet.IP)
 
@@ -213,16 +211,18 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs
 
 	var (
 		additionalFieldsBytes = []byte(dblog.AdditionalFields)
-		additionalFields      AdditionalFields
+		additionalFields      audit.AdditionalFields
 		err                   = json.Unmarshal(additionalFieldsBytes, &additionalFields)
 	)
 	if err != nil {
 		api.Logger.Error(ctx, "unmarshal additional fields", slog.Error(err))
-		resourceInfo := map[string]string{
-			"workspaceName": "unknown",
-			"buildNumber":   "unknown",
-			"buildReason":   "unknown",
+		resourceInfo := audit.AdditionalFields{
+			WorkspaceName:  "unknown",
+			BuildNumber:    "unknown",
+			BuildReason:    "unknown",
+			WorkspaceOwner: "unknown",
 		}
+
 		dblog.AdditionalFields, err = json.Marshal(resourceInfo)
 		api.Logger.Error(ctx, "marshal additional fields", slog.Error(err))
 	}
@@ -259,7 +259,7 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs
 	}
 }
 
-func auditLogDescription(alog database.GetAuditLogsOffsetRow, additionalFields AdditionalFields) string {
+func auditLogDescription(alog database.GetAuditLogsOffsetRow, additionalFields audit.AdditionalFields) string {
 	str := fmt.Sprintf("{user} %s",
 		codersdk.AuditAction(alog.Action).Friendly(),
 	)
@@ -344,14 +344,16 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get
 	}
 }
 
-func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAuditLogsOffsetRow, additionalFields AdditionalFields) string {
+func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAuditLogsOffsetRow, additionalFields audit.AdditionalFields) 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:
 		workspace, getWorkspaceErr := api.Database.GetWorkspaceByID(ctx, alog.ResourceID)
 		if getWorkspaceErr != nil {
@@ -363,6 +365,7 @@ func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAudit
 		}
 		return fmt.Sprintf("/@%s/%s",
 			workspaceOwner.Username, alog.ResourceTarget)
+
 	case database.ResourceTypeWorkspaceBuild:
 		if len(additionalFields.WorkspaceName) == 0 || len(additionalFields.BuildNumber) == 0 {
 			return ""
@@ -381,6 +384,7 @@ func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAudit
 		}
 		return fmt.Sprintf("/@%s/%s/builds/%s",
 			workspaceOwner.Username, additionalFields.WorkspaceName, additionalFields.BuildNumber)
+
 	default:
 		return ""
 	}
diff --git a/coderd/audit/audit.go b/coderd/audit/audit.go
@@ -12,6 +12,13 @@ type Auditor interface {
 	diff(old, new any) Map
 }
 
+type AdditionalFields struct {
+	WorkspaceName  string               `json:"workspace_name"`
+	BuildNumber    string               `json:"build_number"`
+	BuildReason    database.BuildReason `json:"build_reason"`
+	WorkspaceOwner string               `json:"workspace_owner"`
+}
+
 func NewNop() Auditor {
 	return nop{}
 }
diff --git a/coderd/audit_test.go b/coderd/audit_test.go
@@ -2,12 +2,17 @@ package coderd_test
 
 import (
 	"context"
+	"encoding/json"
+	"fmt"
+	"strconv"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/coder/coder/coderd/audit"
 	"github.com/coder/coder/coderd/coderdtest"
+	"github.com/coder/coder/coderd/database"
 	"github.com/coder/coder/codersdk"
 )
 
@@ -36,6 +41,49 @@ func TestAuditLogs(t *testing.T) {
 		require.Equal(t, int64(1), alogs.Count)
 		require.Len(t, alogs.AuditLogs, 1)
 	})
+
+	t.Run("WorkspaceBuildAuditLink", func(t *testing.T) {
+		t.Parallel()
+
+		var (
+			ctx      = context.Background()
+			client   = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
+			user     = coderdtest.CreateFirstUser(t, client)
+			version  = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+			template = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
+		)
+
+		coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
+		workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
+		coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
+
+		buildResourceInfo := audit.AdditionalFields{
+			WorkspaceName: workspace.Name,
+			BuildNumber:   strconv.FormatInt(int64(workspace.LatestBuild.BuildNumber), 10),
+			BuildReason:   database.BuildReason(string(workspace.LatestBuild.Reason)),
+		}
+
+		wriBytes, err := json.Marshal(buildResourceInfo)
+		require.NoError(t, err)
+
+		err = client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{
+			Action:           codersdk.AuditActionStop,
+			ResourceType:     codersdk.ResourceTypeWorkspaceBuild,
+			ResourceID:       workspace.LatestBuild.ID,
+			AdditionalFields: wriBytes,
+		})
+		require.NoError(t, err)
+
+		auditLogs, err := client.AuditLogs(ctx, codersdk.AuditLogsRequest{
+			Pagination: codersdk.Pagination{
+				Limit: 1,
+			},
+		})
+		require.NoError(t, err)
+		buildNumberString := strconv.FormatInt(int64(workspace.LatestBuild.BuildNumber), 10)
+		require.Equal(t, auditLogs.AuditLogs[0].ResourceLink, fmt.Sprintf("/@%s/%s/builds/%s",
+			workspace.OwnerName, workspace.Name, buildNumberString))
+	})
 }
 
 func TestAuditLogsFilter(t *testing.T) {
diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go
@@ -553,12 +553,13 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p
 				if prevBuildErr != nil {
 					previousBuild = database.WorkspaceBuild{}
 				}
+
 				// We pass the below information to the Auditor so that it
 				// can form a friendly string for the user to view in the UI.
-				buildResourceInfo := map[string]string{
-					"workspaceName": workspace.Name,
-					"buildNumber":   strconv.FormatInt(int64(build.BuildNumber), 10),
-					"buildReason":   fmt.Sprintf("%v", build.Reason),
+				buildResourceInfo := audit.AdditionalFields{
+					WorkspaceName: workspace.Name,
+					BuildNumber:   strconv.FormatInt(int64(build.BuildNumber), 10),
+					BuildReason:   database.BuildReason(string(build.Reason)),
 				}
 
 				wriBytes, err := json.Marshal(buildResourceInfo)
@@ -816,10 +817,10 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
 
 			// We pass the below information to the Auditor so that it
 			// can form a friendly string for the user to view in the UI.
-			buildResourceInfo := map[string]string{
-				"workspaceName": workspace.Name,
-				"buildNumber":   strconv.FormatInt(int64(workspaceBuild.BuildNumber), 10),
-				"buildReason":   fmt.Sprintf("%v", workspaceBuild.Reason),
+			buildResourceInfo := audit.AdditionalFields{
+				WorkspaceName: workspace.Name,
+				BuildNumber:   strconv.FormatInt(int64(workspaceBuild.BuildNumber), 10),
+				BuildReason:   database.BuildReason(string(workspaceBuild.Reason)),
 			}
 
 			wriBytes, err := json.Marshal(buildResourceInfo)
diff --git a/coderd/workspaces.go b/coderd/workspaces.go
@@ -279,19 +279,29 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
 // @Router /organizations/{organization}/members/{user}/workspaces [post]
 func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Request) {
 	var (
-		ctx               = r.Context()
-		organization      = httpmw.OrganizationParam(r)
-		apiKey            = httpmw.APIKey(r)
-		auditor           = api.Auditor.Load()
-		user              = httpmw.UserParam(r)
-		aReq, commitAudit = audit.InitRequest[database.Workspace](rw, &audit.RequestParams{
-			Audit:   *auditor,
-			Log:     api.Logger,
-			Request: r,
-			Action:  database.AuditActionCreate,
-		})
+		ctx                   = r.Context()
+		organization          = httpmw.OrganizationParam(r)
+		apiKey                = httpmw.APIKey(r)
+		auditor               = api.Auditor.Load()
+		user                  = httpmw.UserParam(r)
+		workspaceResourceInfo = audit.AdditionalFields{
+			WorkspaceOwner: user.Username,
+		}
 	)
 
+	wriBytes, err := json.Marshal(workspaceResourceInfo)
+	if err != nil {
+		api.Logger.Warn(ctx, "marshal workspace owner name")
+	}
+
+	aReq, commitAudit := audit.InitRequest[database.Workspace](rw, &audit.RequestParams{
+		Audit:            *auditor,
+		Log:              api.Logger,
+		Request:          r,
+		Action:           database.AuditActionCreate,
+		AdditionalFields: wriBytes,
+	})
+
 	defer commitAudit()
 
 	if !api.Authorize(r, rbac.ActionCreate,
diff --git a/codersdk/audit.go b/codersdk/audit.go
@@ -118,11 +118,12 @@ type AuditLogResponse struct {
 }
 
 type CreateTestAuditLogRequest struct {
-	Action       AuditAction  `json:"action,omitempty" enums:"create,write,delete,start,stop"`
-	ResourceType ResourceType `json:"resource_type,omitempty" enums:"template,template_version,user,workspace,workspace_build,git_ssh_key,auditable_group"`
-	ResourceID   uuid.UUID    `json:"resource_id,omitempty" format:"uuid"`
-	Time         time.Time    `json:"time,omitempty" format:"date-time"`
-	BuildReason  BuildReason  `json:"build_reason,omitempty" enums:"autostart,autostop,initiator"`
+	Action           AuditAction     `json:"action,omitempty" enums:"create,write,delete,start,stop"`
+	ResourceType     ResourceType    `json:"resource_type,omitempty" enums:"template,template_version,user,workspace,workspace_build,git_ssh_key,auditable_group"`
+	ResourceID       uuid.UUID       `json:"resource_id,omitempty" format:"uuid"`
+	AdditionalFields json.RawMessage `json:"additional_fields,omitempty"`
+	Time             time.Time       `json:"time,omitempty" format:"date-time"`
+	BuildReason      BuildReason     `json:"build_reason,omitempty" enums:"autostart,autostop,initiator"`
 }
 
 // AuditLogs retrieves audit logs from the given page.
diff --git a/docs/api/audit.md b/docs/api/audit.md
@@ -106,6 +106,7 @@ curl -X POST http://coder-server:8080/api/v2/audit/testgenerate \
 ```json
 {
   "action": "create",
+  "additional_fields": [0],
   "build_reason": "autostart",
   "resource_id": "4d5215ed-38bb-48ed-879a-fdb9ca58522f",
   "resource_type": "template",
diff --git a/docs/api/schemas.md b/docs/api/schemas.md
@@ -991,6 +991,7 @@ CreateParameterRequest is a structure used to create a new parameter value for a
 ```json
 {
   "action": "create",
+  "additional_fields": [0],
   "build_reason": "autostart",
   "resource_id": "4d5215ed-38bb-48ed-879a-fdb9ca58522f",
   "resource_type": "template",
@@ -1000,13 +1001,14 @@ CreateParameterRequest is a structure used to create a new parameter value for a
 
 ### Properties
 
-| Name            | Type                                           | Required | Restrictions | Description |
-| --------------- | ---------------------------------------------- | -------- | ------------ | ----------- |
-| `action`        | [codersdk.AuditAction](#codersdkauditaction)   | false    |              |             |
-| `build_reason`  | [codersdk.BuildReason](#codersdkbuildreason)   | false    |              |             |
-| `resource_id`   | string                                         | false    |              |             |
-| `resource_type` | [codersdk.ResourceType](#codersdkresourcetype) | false    |              |             |
-| `time`          | string                                         | false    |              |             |
+| Name                | Type                                           | Required | Restrictions | Description |
+| ------------------- | ---------------------------------------------- | -------- | ------------ | ----------- |
+| `action`            | [codersdk.AuditAction](#codersdkauditaction)   | false    |              |             |
+| `additional_fields` | array of integer                               | false    |              |             |
+| `build_reason`      | [codersdk.BuildReason](#codersdkbuildreason)   | false    |              |             |
+| `resource_id`       | string                                         | false    |              |             |
+| `resource_type`     | [codersdk.ResourceType](#codersdkresourcetype) | false    |              |             |
+| `time`              | string                                         | false    |              |             |
 
 #### Enumerated Values
 
diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts
@@ -201,6 +201,7 @@ export interface CreateTestAuditLogRequest {
   readonly action?: AuditAction
   readonly resource_type?: ResourceType
   readonly resource_id?: string
+  readonly additional_fields?: Record<string, string>
   readonly time?: string
   readonly build_reason?: BuildReason
 }
diff --git a/site/src/components/AuditLogRow/AuditLogDescription.test.tsx b/site/src/components/AuditLogRow/AuditLogDescription.test.tsx
@@ -38,7 +38,7 @@ describe("AuditLogDescription", () => {
     const AuditLogWithRepeat = {
       ...MockAuditLogWithWorkspaceBuild,
       additional_fields: {
-        workspaceName: "workspace",
+        workspace_name: "workspace",
       },
     }
     render(<AuditLogDescription auditLog={AuditLogWithRepeat} />)
@@ -55,7 +55,7 @@ describe("AuditLogDescription", () => {
     )
     expect(
       getByTextContent(
-        `TestUser created workspace bruno-dev on behalf of ${MockWorkspaceCreateAuditLogForDifferentOwner.additional_fields.workspaceOwner}`,
+        `TestUser created workspace bruno-dev on behalf of ${MockWorkspaceCreateAuditLogForDifferentOwner.additional_fields.workspace_owner}`,
       ),
     ).toBeDefined()
   })
diff --git a/site/src/components/AuditLogRow/AuditLogDescription.tsx b/site/src/components/AuditLogRow/AuditLogDescription.tsx
diff --git a/site/src/components/AuditLogRow/auditUtils.ts b/site/src/components/AuditLogRow/auditUtils.ts
diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts

Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,13 @@ type Auditor interface {`
`12`	`12`	`diff(old, new any) Map`
`13`	`13`	`}`
`14`	`14`
	`15`	`+type AdditionalFields struct {`
	`16`	+ WorkspaceName string `json:"workspace_name"`
	`17`	+ BuildNumber string `json:"build_number"`
	`18`	+ BuildReason database.BuildReason `json:"build_reason"`
	`19`	+ WorkspaceOwner string `json:"workspace_owner"`
	`20`	`+}`
	`21`	`+`
`15`	`22`	`func NewNop() Auditor {`
`16`	`23`	`return nop{}`
`17`	`24`	`}`
Original file line number	Diff line number	Diff line change
`@@ -106,6 +106,7 @@ curl -X POST http://coder-server:8080/api/v2/audit/testgenerate \`
`106`	`106`	```json
`107`	`107`	`{`
`108`	`108`	`"action": "create",`
	`109`	`+ "additional_fields": [0],`
`109`	`110`	`"build_reason": "autostart",`
`110`	`111`	`"resource_id": "4d5215ed-38bb-48ed-879a-fdb9ca58522f",`
`111`	`112`	`"resource_type": "template",`
Original file line number	Diff line number	Diff line change
`@@ -201,6 +201,7 @@ export interface CreateTestAuditLogRequest {`
`201`	`201`	`readonly action?: AuditAction`
`202`	`202`	`readonly resource_type?: ResourceType`
`203`	`203`	`readonly resource_id?: string`
	`204`	`+ readonly additional_fields?: Record<string, string>`
`204`	`205`	`readonly time?: string`
`205`	`206`	`readonly build_reason?: BuildReason`
`206`	`207`	`}`