Skip to content

feat: add workspace build start/stop to audit log #4744

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 14 commits into from
Oct 25, 2022
Merged
16 changes: 16 additions & 0 deletions coderd/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,12 +219,26 @@ func convertAuditLog(dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog {
}
}

type WorkspaceResourceInfo struct {
WorkspaceName string
}

func auditLogDescription(alog database.GetAuditLogsOffsetRow) string {
str := fmt.Sprintf("{user} %s %s",
codersdk.AuditAction(alog.Action).FriendlyString(),
codersdk.ResourceType(alog.ResourceType).FriendlyString(),
)

// Strings for build updates follow the below format:
// "{user} started workspace build for workspace {target}"
// where target is a workspace instead of the workspace build
if alog.ResourceType == database.ResourceTypeWorkspaceBuild {
workspaceBytes := []byte(alog.AdditionalFields)
var workspaceResourceInfo WorkspaceResourceInfo
_ = json.Unmarshal(workspaceBytes, &workspaceResourceInfo)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this error be worth surfacing (even just through the logger) or is it expected that this might sometimes not contain workspace info?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

str += " for workspace " + workspaceResourceInfo.WorkspaceName
}

// We don't display the name for git ssh keys. It's fairly long and doesn't
// make too much sense to display.
if alog.ResourceType != database.ResourceTypeGitSshKey {
Expand Down Expand Up @@ -288,6 +302,8 @@ func resourceTypeFromString(resourceTypeString string) string {
return resourceTypeString
case codersdk.ResourceTypeWorkspace:
return resourceTypeString
case codersdk.ResourceTypeWorkspaceBuild:
return resourceTypeString
case codersdk.ResourceTypeGitSSHKey:
return resourceTypeString
case codersdk.ResourceTypeAPIKey:
Expand Down
3 changes: 2 additions & 1 deletion coderd/audit/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ type Auditable interface {
database.User |
database.Workspace |
database.GitSSHKey |
database.Group
database.Group |
database.WorkspaceBuild
}

// Map is a map of changed fields in an audited resource. It maps field names to
Expand Down
18 changes: 15 additions & 3 deletions coderd/audit/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ type RequestParams struct {
Audit Auditor
Log slog.Logger

Request *http.Request
Action database.AuditAction
Request *http.Request
Action database.AuditAction
AdditionalFields json.RawMessage
}

type Request[T Auditable] struct {
Expand All @@ -43,6 +44,9 @@ func ResourceTarget[T Auditable](tgt T) string {
return typed.Username
case database.Workspace:
return typed.Name
case database.WorkspaceBuild:
// this isn't used
return string(typed.BuildNumber)
case database.GitSSHKey:
return typed.PublicKey
case database.Group:
Expand All @@ -64,6 +68,8 @@ func ResourceID[T Auditable](tgt T) uuid.UUID {
return typed.ID
case database.Workspace:
return typed.ID
case database.WorkspaceBuild:
return typed.ID
case database.GitSSHKey:
return typed.UserID
case database.Group:
Expand All @@ -85,6 +91,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
return database.ResourceTypeUser
case database.Workspace:
return database.ResourceTypeWorkspace
case database.WorkspaceBuild:
return database.ResourceTypeWorkspaceBuild
case database.GitSSHKey:
return database.ResourceTypeGitSshKey
case database.Group:
Expand Down Expand Up @@ -129,6 +137,10 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
}
}

if p.AdditionalFields == nil {
p.AdditionalFields = json.RawMessage("{}")
}

ip := parseIP(p.Request.RemoteAddr)
err := p.Audit.Export(ctx, database.AuditLog{
ID: uuid.New(),
Expand All @@ -143,7 +155,7 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
Diff: diffRaw,
StatusCode: int32(sw.Status),
RequestID: httpmw.RequestID(p.Request),
AdditionalFields: json.RawMessage("{}"),
AdditionalFields: p.AdditionalFields,
})
if err != nil {
p.Log.Error(logCtx, "export audit log", slog.Error(err))
Expand Down
7 changes: 5 additions & 2 deletions coderd/database/dump.sql

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

2 changes: 2 additions & 0 deletions coderd/database/migrations/000065_add_audit_enums.down.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- It's not possible to drop enum values from enum types, so the UP has "IF NOT
-- EXISTS".
4 changes: 4 additions & 0 deletions coderd/database/migrations/000065_add_audit_enums.up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
ALTER TYPE audit_action ADD VALUE IF NOT EXISTS 'start';
ALTER TYPE audit_action ADD VALUE IF NOT EXISTS 'stop';

ALTER TYPE resource_type ADD VALUE IF NOT EXISTS 'workspace_build';
3 changes: 3 additions & 0 deletions coderd/database/models.go

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

61 changes: 48 additions & 13 deletions coderd/workspacebuilds.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"golang.org/x/exp/slices"
"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"
Expand Down Expand Up @@ -278,28 +279,62 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
return
}

// we only want to create audit logs for delete builds right now
auditor := api.Auditor.Load()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really related to your PR since this was already here so feel free to ignore but I wonder if the auditor should be set up before the authorization check so we can log unauthorized request attempts.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good eye! We have a separate ticket for this!


// if user deletes a workspace, audit the workspace
if action == rbac.ActionDelete {
var (
auditor = api.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.Workspace](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionDelete,
})
)
aReq, commitAudit := audit.InitRequest[database.Workspace](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionDelete,
})

defer commitAudit()
aReq.Old = workspace
}

if createBuild.TemplateVersionID == uuid.Nil {
latestBuild, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(ctx, workspace.ID)
latestBuild, latestBuildErr := api.Database.GetLatestWorkspaceBuildByWorkspaceID(ctx, workspace.ID)

// if a user starts/stops a workspace, audit the workspace build
if action == rbac.ActionUpdate {
var auditAction database.AuditAction
if createBuild.Transition == codersdk.WorkspaceTransitionStart {
auditAction = database.AuditActionStart
} else if createBuild.Transition == codersdk.WorkspaceTransitionStop {
auditAction = database.AuditActionStop
} else {
auditAction = database.AuditActionWrite
}

// We pass the workspace name to the Auditor so that it
// can form a friendly string for the user.
workspaceResourceInfo := map[string]string{
"workspaceName": workspace.Name,
}

wriBytes, err := json.Marshal(workspaceResourceInfo)
if err != nil {
api.Logger.Error(ctx, "could not marshal workspace name", slog.Error(err))
}

aReq, commitAudit := audit.InitRequest[database.WorkspaceBuild](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: auditAction,
AdditionalFields: wriBytes,
})

defer commitAudit()
aReq.Old = latestBuild
}

if createBuild.TemplateVersionID == uuid.Nil {
if latestBuildErr != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching the latest workspace build.",
Detail: err.Error(),
Detail: latestBuildErr.Error(),
})
return
}
Expand Down
4 changes: 2 additions & 2 deletions coderd/workspacebuilds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,6 @@ func TestWorkspaceBuildStatus(t *testing.T) {
require.EqualValues(t, codersdk.WorkspaceStatusDeleted, workspace.LatestBuild.Status)

// assert an audit log has been created for deletion
require.Len(t, auditor.AuditLogs, 5)
assert.Equal(t, database.AuditActionDelete, auditor.AuditLogs[4].Action)
require.Len(t, auditor.AuditLogs, 7)
assert.Equal(t, database.AuditActionDelete, auditor.AuditLogs[6].Action)
}
9 changes: 9 additions & 0 deletions codersdk/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const (
ResourceTypeTemplateVersion ResourceType = "template_version"
ResourceTypeUser ResourceType = "user"
ResourceTypeWorkspace ResourceType = "workspace"
ResourceTypeWorkspaceBuild ResourceType = "workspace_build"
ResourceTypeGitSSHKey ResourceType = "git_ssh_key"
ResourceTypeAPIKey ResourceType = "api_key"
ResourceTypeGroup ResourceType = "group"
Expand All @@ -36,6 +37,8 @@ func (r ResourceType) FriendlyString() string {
return "user"
case ResourceTypeWorkspace:
return "workspace"
case ResourceTypeWorkspaceBuild:
return "workspace build"
case ResourceTypeGitSSHKey:
return "git ssh key"
case ResourceTypeAPIKey:
Expand All @@ -53,6 +56,8 @@ const (
AuditActionCreate AuditAction = "create"
AuditActionWrite AuditAction = "write"
AuditActionDelete AuditAction = "delete"
AuditActionStart AuditAction = "start"
AuditActionStop AuditAction = "stop"
)

func (a AuditAction) FriendlyString() string {
Expand All @@ -63,6 +68,10 @@ func (a AuditAction) FriendlyString() string {
return "updated"
case AuditActionDelete:
return "deleted"
case AuditActionStart:
return "started"
case AuditActionStop:
return "stopped"
default:
return "unknown"
}
Expand Down
15 changes: 15 additions & 0 deletions enterprise/audit/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,21 @@ var AuditableResources = auditMap(map[any]map[string]Action{
"organization_id": ActionIgnore, // Never changes.
"avatar_url": ActionTrack,
},
// We don't show any diff for the WorkspaceBuild resource
&database.WorkspaceBuild{}: {
"id": ActionIgnore,
"created_at": ActionIgnore,
"updated_at": ActionIgnore,
"workspace_id": ActionIgnore,
"template_version_id": ActionIgnore,
"build_number": ActionIgnore,
"transition": ActionIgnore,
"initiator_id": ActionIgnore,
"provisioner_state": ActionIgnore,
"job_id": ActionIgnore,
"deadline": ActionIgnore,
"reason": ActionIgnore,
},
})

// auditMap converts a map of struct pointers to a map of struct names as
Expand Down
3 changes: 2 additions & 1 deletion site/src/api/typesGenerated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,7 @@ export interface WorkspacesRequest extends Pagination {
export type APIKeyScope = "all" | "application_connect"

// From codersdk/audit.go
export type AuditAction = "create" | "delete" | "write"
export type AuditAction = "create" | "delete" | "start" | "stop" | "write"

// From codersdk/workspacebuilds.go
export type BuildReason = "autostart" | "autostop" | "initiator"
Expand Down Expand Up @@ -975,6 +975,7 @@ export type ResourceType =
| "template_version"
| "user"
| "workspace"
| "workspace_build"

// From codersdk/sse.go
export type ServerSentEventType = "data" | "error" | "ping"
Expand Down
18 changes: 8 additions & 10 deletions site/src/components/AuditLogRow/AuditLogRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,11 @@ export const AuditLogRow: React.FC<AuditLogRowProps> = ({
</Stack>
</Stack>

<div
className={
shouldDisplayDiff ? undefined : styles.disabledDropdownIcon
}
>
{isDiffOpen ? <CloseDropdown /> : <OpenDropdown />}
</div>
{shouldDisplayDiff ? (
<div> {isDiffOpen ? <CloseDropdown /> : <OpenDropdown />}</div>
) : (
<div className={styles.columnWithoutDiff}></div>
)}
</Stack>

{shouldDisplayDiff && (
Expand Down Expand Up @@ -190,8 +188,8 @@ const useStyles = makeStyles((theme) => ({
color: theme.palette.text.secondary,
whiteSpace: "nowrap",
},

disabledDropdownIcon: {
opacity: 0.5,
// offset the absence of the arrow icon on diff-less logs
columnWithoutDiff: {
marginLeft: "24px",
},
}))