fix: audit log broken build links (#5895)

* pushing for guidance

* added test

* PR feedback

* fixed tests

* Update coderd/audit.go

Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>

* runnig make gen

---------

Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>
This commit is contained in:
Kira Pilot
2023-01-30 18:43:29 +00:00
committed by GitHub
co-authored by Steven Masley
parent 88b5d42967
commit b31b0fd189
15 changed files with 145 additions and 58 deletions
+6
View File
@@ -5734,6 +5734,12 @@ const docTemplate = `{
}
]
},
"additional_fields": {
"type": "array",
"items": {
"type": "integer"
}
},
"build_reason": {
"enum": [
"autostart",
+6
View File
@@ -5084,6 +5084,12 @@
}
]
},
"additional_fields": {
"type": "array",
"items": {
"type": "integer"
}
},
"build_reason": {
"enum": ["autostart", "autostop", "initiator"],
"allOf": [
+18 -14
View File
@@ -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 ""
}
+7
View File
@@ -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{}
}
+48
View File
@@ -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) {
@@ -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)
+21 -11
View File
@@ -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,
+6 -5
View File
@@ -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.
+1
View File
@@ -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",
+9 -7
View File
@@ -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
+1
View File
@@ -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
}
@@ -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()
})
@@ -16,11 +16,11 @@ export const AuditLogDescription: FC<{ auditLog: AuditLog }> = ({
if (auditLog.resource_type === "workspace_build") {
// audit logs with a resource_type of workspace build use workspace name as a target
target = auditLog.additional_fields.workspaceName.trim()
target = auditLog.additional_fields?.workspace_name?.trim()
// workspaces can be started/stopped by a user, or kicked off automatically by Coder
user =
auditLog.additional_fields.buildReason &&
auditLog.additional_fields.buildReason !== "initiator"
auditLog.additional_fields?.build_reason &&
auditLog.additional_fields?.build_reason !== "initiator"
? t("auditLog:table.logRow.buildReason")
: auditLog.user?.username.trim()
}
@@ -56,12 +56,12 @@ export const AuditLogDescription: FC<{ auditLog: AuditLog }> = ({
</span>
)}
{/* logs for workspaces created on behalf of other users indicate ownership in the description */}
{auditLog.additional_fields.workspaceOwner &&
auditLog.additional_fields.workspaceOwner !== "unknown" && (
{auditLog.additional_fields.workspace_owner &&
auditLog.additional_fields.workspace_owner !== "unknown" && (
<span>
<>
{t("auditLog:table.logRow.onBehalfOf", {
owner: auditLog.additional_fields.workspaceOwner,
owner: auditLog.additional_fields.workspace_owner,
})}
</>
</span>
@@ -14,13 +14,13 @@ export const determineGroupDiff = (auditLogDiff: AuditDiff): AuditDiff => {
return {
...auditLogDiff,
members: {
old: auditLogDiff.members.old?.map(
old: auditLogDiff.members?.old?.map(
(groupMember: GroupMember) => groupMember.user_id,
),
new: auditLogDiff.members.new?.map(
new: auditLogDiff.members?.new?.map(
(groupMember: GroupMember) => groupMember.user_id,
),
secret: auditLogDiff.members.secret,
secret: auditLogDiff.members?.secret,
},
}
}
+2 -2
View File
@@ -1080,7 +1080,7 @@ export const MockAuditLog2: TypesGen.AuditLog = {
export const MockWorkspaceCreateAuditLogForDifferentOwner = {
...MockAuditLog,
additional_fields: {
workspaceOwner: "Member",
workspace_owner: "Member",
},
}
@@ -1092,7 +1092,7 @@ export const MockAuditLogWithWorkspaceBuild: TypesGen.AuditLog = {
resource_type: "workspace_build",
description: "{user} stopped build for workspace {target}",
additional_fields: {
workspaceName: "test2",
workspace_name: "test2",
},
}