Audit build outcomes/kira pilot (#5143)

* auditing failed builds

* logging workspace build successes

* remove duplicate workspace build entry

* fixed workspacebuilds_test

* PR feedback

* lint and migrations

* fix nil auditors

* workspace_build test

* fixed workspaces_teest

Co-authored-by: Colin Adler <colin1adler@gmail.com>
This commit is contained in:
Kira Pilot 2022-11-22 13:22:56 -05:00 committed by GitHub
parent 1f20cab110
commit 6786ca2854
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 184 additions and 85 deletions

View File

@ -1,6 +1,7 @@
package coderd
import (
"database/sql"
"encoding/json"
"fmt"
"net"
@ -129,7 +130,7 @@ func (api *API) generateFakeAuditLog(rw http.ResponseWriter, r *http.Request) {
Time: params.Time,
UserID: user.ID,
Ip: ipNet,
UserAgent: r.UserAgent(),
UserAgent: sql.NullString{String: r.UserAgent(), Valid: true},
ResourceType: database.ResourceType(params.ResourceType),
ResourceID: params.ResourceID,
ResourceTarget: user.Username,
@ -163,6 +164,7 @@ func convertAuditLog(dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog {
_ = json.Unmarshal(dblog.Diff, &diff)
var user *codersdk.User
if dblog.UserUsername.Valid {
user = &codersdk.User{
ID: dblog.UserID,
@ -186,7 +188,7 @@ func convertAuditLog(dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog {
Time: dblog.Time,
OrganizationID: dblog.OrganizationID,
IP: ip,
UserAgent: dblog.UserAgent,
UserAgent: dblog.UserAgent.String,
ResourceType: codersdk.ResourceType(dblog.ResourceType),
ResourceID: dblog.ResourceID,
ResourceTarget: dblog.ResourceTarget,

View File

@ -2,6 +2,7 @@ package audit
import (
"context"
"database/sql"
"encoding/json"
"fmt"
"net"
@ -32,6 +33,20 @@ type Request[T Auditable] struct {
New T
}
type BuildAuditParams[T Auditable] struct {
Audit Auditor
Log slog.Logger
UserID uuid.UUID
JobID uuid.UUID
Status int
Action database.AuditAction
AdditionalFields json.RawMessage
New T
Old T
}
func ResourceTarget[T Auditable](tgt T) string {
switch typed := any(tgt).(type) {
case database.Organization:
@ -147,7 +162,7 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
Time: database.Now(),
UserID: httpmw.APIKey(p.Request).UserID,
Ip: ip,
UserAgent: p.Request.UserAgent(),
UserAgent: sql.NullString{String: p.Request.UserAgent(), Valid: true},
ResourceType: either(req.Old, req.New, ResourceType[T]),
ResourceID: either(req.Old, req.New, ResourceID[T]),
ResourceTarget: either(req.Old, req.New, ResourceTarget[T]),
@ -164,6 +179,40 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
}
}
// BuildAudit creates an audit log for a workspace build.
// The audit log is committed upon invocation.
func BuildAudit[T Auditable](ctx context.Context, p *BuildAuditParams[T]) {
// As the audit request has not been initiated directly by a user, we omit
// certain user details.
ip := parseIP("")
// We do not show diffs for build audit logs
var diffRaw = []byte("{}")
if p.AdditionalFields == nil {
p.AdditionalFields = json.RawMessage("{}")
}
err := p.Audit.Export(ctx, database.AuditLog{
ID: uuid.New(),
Time: database.Now(),
UserID: p.UserID,
Ip: ip,
UserAgent: sql.NullString{},
ResourceType: either(p.Old, p.New, ResourceType[T]),
ResourceID: either(p.Old, p.New, ResourceID[T]),
ResourceTarget: either(p.Old, p.New, ResourceTarget[T]),
Action: p.Action,
Diff: diffRaw,
StatusCode: int32(p.Status),
RequestID: p.JobID,
AdditionalFields: p.AdditionalFields,
})
if err != nil {
p.Log.Error(ctx, "export audit log", slog.Error(err))
return
}
}
func either[T Auditable, R any](old, new T, fn func(T) R) R {
if ResourceID(new) != uuid.Nil {
return fn(new)

View File

@ -681,6 +681,7 @@ func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, debounce ti
Telemetry: api.Telemetry,
Tags: tags,
QuotaCommitter: &api.QuotaCommitter,
Auditor: &api.Auditor,
AcquireJobDebounce: debounce,
Logger: api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)),
})

View File

@ -143,8 +143,8 @@ CREATE TABLE audit_logs (
"time" timestamp with time zone NOT NULL,
user_id uuid NOT NULL,
organization_id uuid NOT NULL,
ip inet NOT NULL,
user_agent character varying(256) NOT NULL,
ip inet,
user_agent character varying(256),
resource_type resource_type NOT NULL,
resource_id uuid NOT NULL,
resource_target text NOT NULL,

View File

@ -0,0 +1,2 @@
ALTER TABLE audit_logs ALTER COLUMN ip SET NOT NULL;
ALTER TABLE audit_logs ALTER COLUMN user_agent SET NOT NULL;

View File

@ -0,0 +1,2 @@
ALTER TABLE audit_logs ALTER COLUMN ip DROP NOT NULL;
ALTER TABLE audit_logs ALTER COLUMN user_agent DROP NOT NULL;

View File

@ -411,7 +411,7 @@ type AuditLog struct {
UserID uuid.UUID `db:"user_id" json:"user_id"`
OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"`
Ip pqtype.Inet `db:"ip" json:"ip"`
UserAgent string `db:"user_agent" json:"user_agent"`
UserAgent sql.NullString `db:"user_agent" json:"user_agent"`
ResourceType ResourceType `db:"resource_type" json:"resource_type"`
ResourceID uuid.UUID `db:"resource_id" json:"resource_id"`
ResourceTarget string `db:"resource_target" json:"resource_target"`

View File

@ -455,7 +455,7 @@ type GetAuditLogsOffsetRow struct {
UserID uuid.UUID `db:"user_id" json:"user_id"`
OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"`
Ip pqtype.Inet `db:"ip" json:"ip"`
UserAgent string `db:"user_agent" json:"user_agent"`
UserAgent sql.NullString `db:"user_agent" json:"user_agent"`
ResourceType ResourceType `db:"resource_type" json:"resource_type"`
ResourceID uuid.UUID `db:"resource_id" json:"resource_id"`
ResourceTarget string `db:"resource_target" json:"resource_target"`
@ -562,7 +562,7 @@ type InsertAuditLogParams struct {
UserID uuid.UUID `db:"user_id" json:"user_id"`
OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"`
Ip pqtype.Inet `db:"ip" json:"ip"`
UserAgent string `db:"user_agent" json:"user_agent"`
UserAgent sql.NullString `db:"user_agent" json:"user_agent"`
ResourceType ResourceType `db:"resource_type" json:"resource_type"`
ResourceID uuid.UUID `db:"resource_id" json:"resource_id"`
ResourceTarget string `db:"resource_target" json:"resource_target"`

View File

@ -6,6 +6,7 @@ import (
"encoding/json"
"errors"
"fmt"
"net/http"
"net/url"
"reflect"
"sync"
@ -21,6 +22,7 @@ import (
"cdr.dev/slog"
"github.com/coder/coder/coderd/audit"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/parameter"
"github.com/coder/coder/coderd/telemetry"
@ -46,6 +48,7 @@ type Server struct {
Pubsub database.Pubsub
Telemetry telemetry.Reporter
QuotaCommitter *atomic.Pointer[proto.QuotaCommitter]
Auditor *atomic.Pointer[audit.Auditor]
AcquireJobDebounce time.Duration
}
@ -522,6 +525,43 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p
case *proto.FailedJob_TemplateImport_:
}
// if failed job is a workspace build, audit the outcome
if job.Type == database.ProvisionerJobTypeWorkspaceBuild {
auditor := server.Auditor.Load()
build, getBuildErr := server.Database.GetWorkspaceBuildByJobID(ctx, job.ID)
if getBuildErr != nil {
server.Logger.Error(ctx, "failed to create audit log - get build err", slog.Error(err))
} else {
auditAction := auditActionFromTransition(build.Transition)
workspace, getWorkspaceErr := server.Database.GetWorkspaceByID(ctx, build.WorkspaceID)
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.
workspaceResourceInfo := map[string]string{
"workspaceName": workspace.Name,
}
wriBytes, err := json.Marshal(workspaceResourceInfo)
if err != nil {
server.Logger.Error(ctx, "could not marshal workspace name", slog.Error(err))
}
audit.BuildAudit(ctx, &audit.BuildAuditParams[database.WorkspaceBuild]{
Audit: *auditor,
Log: server.Logger,
UserID: job.InitiatorID,
JobID: job.ID,
Action: auditAction,
New: build,
Status: http.StatusInternalServerError,
AdditionalFields: wriBytes,
})
}
}
}
data, err := json.Marshal(ProvisionerJobLogsNotifyMessage{EndOfLogs: true})
if err != nil {
return nil, xerrors.Errorf("marshal job log: %w", err)
@ -600,11 +640,14 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
return nil, xerrors.Errorf("get workspace build: %w", err)
}
var workspace database.Workspace
var getWorkspaceError error
err = server.Database.InTx(func(db database.Store) error {
now := database.Now()
var workspaceDeadline time.Time
workspace, err := db.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID)
if err == nil {
workspace, getWorkspaceError = db.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID)
if getWorkspaceError == nil {
if workspace.Ttl.Valid {
workspaceDeadline = now.Add(time.Duration(workspace.Ttl.Int64))
}
@ -704,6 +747,34 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
return nil, xerrors.Errorf("complete job: %w", err)
}
// audit the outcome of the workspace build
if getWorkspaceError == nil {
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.
workspaceResourceInfo := map[string]string{
"workspaceName": workspace.Name,
}
wriBytes, err := json.Marshal(workspaceResourceInfo)
if err != nil {
server.Logger.Error(ctx, "marshal resource info", slog.Error(err))
}
audit.BuildAudit(ctx, &audit.BuildAuditParams[database.WorkspaceBuild]{
Audit: *auditor,
Log: server.Logger,
UserID: job.InitiatorID,
JobID: job.ID,
Action: auditAction,
New: workspaceBuild,
Status: http.StatusOK,
AdditionalFields: wriBytes,
})
}
err = server.Pubsub.Publish(codersdk.WorkspaceNotifyChannel(workspaceBuild.WorkspaceID), []byte{})
if err != nil {
return nil, xerrors.Errorf("update workspace: %w", err)
@ -1015,6 +1086,19 @@ func convertWorkspaceTransition(transition database.WorkspaceTransition) (sdkpro
}
}
func auditActionFromTransition(transition database.WorkspaceTransition) database.AuditAction {
switch transition {
case database.WorkspaceTransitionStart:
return database.AuditActionStart
case database.WorkspaceTransitionStop:
return database.AuditActionStop
case database.WorkspaceTransitionDelete:
return database.AuditActionDelete
default:
return database.AuditActionWrite
}
}
// WorkspaceProvisionJob is the payload for the "workspace_provision" job type.
type WorkspaceProvisionJob struct {
WorkspaceBuildID uuid.UUID `json:"workspace_build_id"`

View File

@ -5,6 +5,7 @@ import (
"database/sql"
"encoding/json"
"net/url"
"sync/atomic"
"testing"
"time"
@ -12,6 +13,7 @@ import (
"github.com/stretchr/testify/require"
"cdr.dev/slog/sloggers/slogtest"
"github.com/coder/coder/coderd/audit"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/database/databasefake"
"github.com/coder/coder/coderd/provisionerdserver"
@ -21,6 +23,13 @@ import (
sdkproto "github.com/coder/coder/provisionersdk/proto"
)
func mockAuditor() *atomic.Pointer[audit.Auditor] {
ptr := &atomic.Pointer[audit.Auditor]{}
mock := audit.Auditor(audit.NewMock())
ptr.Store(&mock)
return ptr
}
func TestAcquireJob(t *testing.T) {
t.Parallel()
t.Run("Debounce", func(t *testing.T) {
@ -36,6 +45,7 @@ func TestAcquireJob(t *testing.T) {
Pubsub: pubsub,
Telemetry: telemetry.NewNoop(),
AcquireJobDebounce: time.Hour,
Auditor: mockAuditor(),
}
job, err := srv.AcquireJob(context.Background(), nil)
require.NoError(t, err)
@ -799,5 +809,6 @@ func setup(t *testing.T) *provisionerdserver.Server {
Database: db,
Pubsub: pubsub,
Telemetry: telemetry.NewNoop(),
Auditor: mockAuditor(),
}
}

View File

@ -15,8 +15,6 @@ 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"
"github.com/coder/coder/coderd/httpmw"
@ -280,58 +278,8 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
return
}
auditor := api.Auditor.Load()
// if user deletes a workspace, audit the workspace
if action == rbac.ActionDelete {
aReq, commitAudit := audit.InitRequest[database.Workspace](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionDelete,
})
defer commitAudit()
aReq.Old = workspace
}
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 {
latestBuild, latestBuildErr := api.Database.GetLatestWorkspaceBuildByWorkspaceID(ctx, workspace.ID)
if latestBuildErr != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching the latest workspace build.",

View File

@ -576,9 +576,9 @@ func TestWorkspaceBuildStatus(t *testing.T) {
numLogs := len(auditor.AuditLogs)
client, closeDaemon, api := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true, Auditor: auditor})
user := coderdtest.CreateFirstUser(t, client)
numLogs++ // add an audit log for user
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
numLogs++ // add an audit log for template version
numLogs++ // add an audit log for template version creation
numLogs++ // add an audit log for template version update
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
closeDaemon.Close()
@ -598,6 +598,8 @@ func TestWorkspaceBuildStatus(t *testing.T) {
require.NoError(t, err)
require.EqualValues(t, codersdk.WorkspaceStatusRunning, workspace.LatestBuild.Status)
numLogs++ // add an audit log for workspace_build starting
// after successful stop is "stopped"
build := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStop)
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID)
@ -616,11 +618,6 @@ func TestWorkspaceBuildStatus(t *testing.T) {
err = client.CancelWorkspaceBuild(ctx, build.ID)
require.NoError(t, err)
numLogs++ // add an audit log for workspace build start
// assert an audit log has been created workspace starting
require.Len(t, auditor.AuditLogs, numLogs)
require.Equal(t, database.AuditActionStart, auditor.AuditLogs[numLogs-1].Action)
workspace, err = client.Workspace(ctx, workspace.ID)
require.NoError(t, err)
require.EqualValues(t, codersdk.WorkspaceStatusCanceled, workspace.LatestBuild.Status)
@ -632,9 +629,4 @@ func TestWorkspaceBuildStatus(t *testing.T) {
workspace, err = client.DeletedWorkspace(ctx, workspace.ID)
require.NoError(t, err)
require.EqualValues(t, codersdk.WorkspaceStatusDeleted, workspace.LatestBuild.Status)
numLogs++ // add an audit log for workspace build deletion
// assert an audit log has been created for deletion
require.Len(t, auditor.AuditLogs, numLogs)
require.Equal(t, database.AuditActionDelete, auditor.AuditLogs[numLogs-1].Action)
}

View File

@ -1101,6 +1101,9 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
})
)
// await job to ensure audit logs for workspace_build start are created
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
// ensure test invariant: new workspaces have no autostart schedule.
require.Empty(t, workspace.AutostartSchedule, "expected newly-minted workspace to have no autostart schedule")
@ -1136,8 +1139,8 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
interval := next.Sub(testCase.at)
require.Equal(t, testCase.expectedInterval, interval, "unexpected interval")
require.Len(t, auditor.AuditLogs, 5)
assert.Equal(t, database.AuditActionWrite, auditor.AuditLogs[4].Action)
require.Len(t, auditor.AuditLogs, 6)
assert.Equal(t, database.AuditActionWrite, auditor.AuditLogs[5].Action)
})
}
@ -1245,8 +1248,8 @@ func TestWorkspaceUpdateTTL(t *testing.T) {
require.Equal(t, testCase.ttlMillis, updated.TTLMillis, "expected autostop ttl to equal requested")
require.Len(t, auditor.AuditLogs, 5)
assert.Equal(t, database.AuditActionWrite, auditor.AuditLogs[4].Action)
require.Len(t, auditor.AuditLogs, 6)
assert.Equal(t, database.AuditActionWrite, auditor.AuditLogs[5].Action)
})
}

View File

@ -1,6 +1,7 @@
package audittest
import (
"database/sql"
"net"
"net/http"
"time"
@ -22,7 +23,7 @@ func RandomLog() database.AuditLog {
IPNet: *inet,
Valid: true,
},
UserAgent: "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.127 Safari/537.36",
UserAgent: sql.NullString{String: "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.127 Safari/537.36", Valid: true},
ResourceType: database.ResourceTypeOrganization,
ResourceID: uuid.New(),
ResourceTarget: "colin's organization",

View File

@ -112,14 +112,13 @@ var AuditableResources = auditMap(map[any]map[string]Action{
"avatar_url": ActionTrack,
"quota_allowance": ActionTrack,
},
// We don't show any diff for the WorkspaceBuild resource,
// save for the template_version_id
// 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": ActionTrack,
"template_version_id": ActionIgnore,
"build_number": ActionIgnore,
"transition": ActionIgnore,
"initiator_id": ActionIgnore,

View File

@ -206,6 +206,7 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
Pubsub: api.Pubsub,
Provisioners: daemon.Provisioners,
Telemetry: api.Telemetry,
Auditor: &api.AGPL.Auditor,
Logger: api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)),
Tags: rawTags,
})

View File

@ -11,9 +11,10 @@ import (
"golang.org/x/xerrors"
"github.com/google/uuid"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/enterprise/coderd/license"
"github.com/google/uuid"
)
type request struct {

View File

@ -20,7 +20,10 @@ 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") {
if (
auditLog.resource_type === "workspace_build" &&
auditLog.additional_fields.workspaceName
) {
target = auditLog.additional_fields.workspaceName.trim()
}