Skip to content

fix: audit log broken build links #5895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions coderd/apidoc/docs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions coderd/apidoc/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 18 additions & 14 deletions coderd/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(),
Expand All @@ -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)
Expand All @@ -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)

Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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(),
)
Expand Down Expand Up @@ -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 {
Expand All @@ -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 ""
Expand All @@ -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 ""
}
Expand Down
7 changes: 7 additions & 0 deletions coderd/audit/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
}
Expand Down
48 changes: 48 additions & 0 deletions coderd/audit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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) {
Expand Down
17 changes: 9 additions & 8 deletions coderd/provisionerdserver/provisionerdserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
32 changes: 21 additions & 11 deletions coderd/workspaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 6 additions & 5 deletions codersdk/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions docs/api/audit.md
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
16 changes: 9 additions & 7 deletions docs/api/schemas.md
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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

Expand Down
1 change: 1 addition & 0 deletions site/src/api/typesGenerated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions site/src/components/AuditLogRow/AuditLogDescription.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe("AuditLogDescription", () => {
const AuditLogWithRepeat = {
...MockAuditLogWithWorkspaceBuild,
additional_fields: {
workspaceName: "workspace",
workspace_name: "workspace",
},
}
render(<AuditLogDescription auditLog={AuditLogWithRepeat} />)
Expand All @@ -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()
})
Expand Down
Loading