Skip to content

Commit de41bd6

Browse files
mafredrimafredri
authored
feat: add support for workspace app audit (#16801)
This change adds support for workspace app auditing. To avoid audit log spam, we introduce the concept of app audit sessions. An audit session is unique per workspace app, user, ip, user agent and http status code. The sessions are stored in a separate table from audit logs to allow use-case specific optimizations. Sessions are ephemeral and the table does not function as a log. The logic for auditing is placed in the DBTokenProvider for workspace apps so that wsproxies are included. This is the final change affecting the API fo #15139. Updates #15139
1 parent 3ae55bb commit de41bd6

25 files changed

+1042
-159
lines changed

‎coderd/audit.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -282,10 +282,14 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow) string {
282282
_, _ = b.WriteString("{user} ")
283283
}
284284

285-
if alog.AuditLog.StatusCode >= 400 {
285+
switch {
286+
case alog.AuditLog.StatusCode == int32(http.StatusSeeOther):
287+
_, _ = b.WriteString("was redirected attempting to ")
288+
_, _ = b.WriteString(string(alog.AuditLog.Action))
289+
case alog.AuditLog.StatusCode >= 400:
286290
_, _ = b.WriteString("unsuccessfully attempted to ")
287291
_, _ = b.WriteString(string(alog.AuditLog.Action))
288-
} else {
292+
default:
289293
_, _ = b.WriteString(codersdk.AuditAction(alog.AuditLog.Action).Friendly())
290294
}
291295

‎coderd/audit/audit.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func (a *MockAuditor) Contains(t testing.TB, expected database.AuditLog) bool {
9393
t.Logf("audit log %d: expected UserID %s, got %s", idx+1, expected.UserID, al.UserID)
9494
continue
9595
}
96-
if expected.OrganizationID != uuid.Nil && al.UserID != expected.UserID {
96+
if expected.OrganizationID != uuid.Nil && al.OrganizationID != expected.OrganizationID {
9797
t.Logf("audit log %d: expected OrganizationID %s, got %s", idx+1, expected.OrganizationID, al.OrganizationID)
9898
continue
9999
}

‎coderd/audit/request.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ type BackgroundAuditParams[T Auditable] struct {
7171
Action database.AuditAction
7272
OrganizationID uuid.UUID
7373
IP string
74+
UserAgent string
7475
// todo: this should automatically marshal an interface{} instead of accepting a raw message.
7576
AdditionalFields json.RawMessage
7677

@@ -422,7 +423,7 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
422423
action = req.Action
423424
}
424425

425-
ip := parseIP(p.Request.RemoteAddr)
426+
ip := ParseIP(p.Request.RemoteAddr)
426427
auditLog := database.AuditLog{
427428
ID: uuid.New(),
428429
Time: dbtime.Now(),
@@ -453,7 +454,7 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
453454
// BackgroundAudit creates an audit log for a background event.
454455
// The audit log is committed upon invocation.
455456
func BackgroundAudit[T Auditable](ctx context.Context, p *BackgroundAuditParams[T]) {
456-
ip := parseIP(p.IP)
457+
ip := ParseIP(p.IP)
457458

458459
diff := Diff(p.Audit, p.Old, p.New)
459460
var err error
@@ -479,7 +480,7 @@ func BackgroundAudit[T Auditable](ctx context.Context, p *BackgroundAuditParams[
479480
UserID: p.UserID,
480481
OrganizationID: requireOrgID[T](ctx, p.OrganizationID, p.Log),
481482
Ip: ip,
482-
UserAgent: sql.NullString{},
483+
UserAgent: sql.NullString{Valid: p.UserAgent != "", String: p.UserAgent},
483484
ResourceType: either(p.Old, p.New, ResourceType[T], p.Action),
484485
ResourceID: either(p.Old, p.New, ResourceID[T], p.Action),
485486
ResourceTarget: either(p.Old, p.New, ResourceTarget[T], p.Action),
@@ -566,7 +567,7 @@ func either[T Auditable, R any](old, new T, fn func(T) R, auditAction database.A
566567
panic("both old and new are nil")
567568
}
568569

569-
func parseIP(ipStr string) pqtype.Inet {
570+
func ParseIP(ipStr string) pqtype.Inet {
570571
ip := net.ParseIP(ipStr)
571572
ipNet := net.IPNet{}
572573
if ip != nil {

‎coderd/coderd.go

+16-10
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,10 @@ type Options struct {
226226
UpdateAgentMetrics func(ctx context.Context, labels prometheusmetrics.AgentMetricLabels, metrics []*agentproto.Stats_Metric)
227227
StatsBatcher workspacestats.Batcher
228228

229+
// WorkspaceAppAuditSessionTimeout allows changing the timeout for audit
230+
// sessions. Raising or lowering this value will directly affect the write
231+
// load of the audit log table. This is used for testing. Default 1 hour.
232+
WorkspaceAppAuditSessionTimeout time.Duration
229233
WorkspaceAppsStatsCollectorOptions workspaceapps.StatsCollectorOptions
230234

231235
// This janky function is used in telemetry to parse fields out of the raw
@@ -534,16 +538,6 @@ func New(options *Options) *API {
534538
Authorizer: options.Authorizer,
535539
Logger: options.Logger,
536540
},
537-
WorkspaceAppsProvider: workspaceapps.NewDBTokenProvider(
538-
options.Logger.Named("workspaceapps"),
539-
options.AccessURL,
540-
options.Authorizer,
541-
options.Database,
542-
options.DeploymentValues,
543-
oauthConfigs,
544-
options.AgentInactiveDisconnectTimeout,
545-
options.AppSigningKeyCache,
546-
),
547541
metricsCache: metricsCache,
548542
Auditor: atomic.Pointer[audit.Auditor]{},
549543
TailnetCoordinator: atomic.Pointer[tailnet.Coordinator]{},
@@ -561,6 +555,18 @@ func New(options *Options) *API {
561555
),
562556
dbRolluper: options.DatabaseRolluper,
563557
}
558+
api.WorkspaceAppsProvider = workspaceapps.NewDBTokenProvider(
559+
options.Logger.Named("workspaceapps"),
560+
options.AccessURL,
561+
options.Authorizer,
562+
&api.Auditor,
563+
options.Database,
564+
options.DeploymentValues,
565+
oauthConfigs,
566+
options.AgentInactiveDisconnectTimeout,
567+
options.WorkspaceAppAuditSessionTimeout,
568+
options.AppSigningKeyCache,
569+
)
564570

565571
f := appearance.NewDefaultFetcher(api.DeploymentValues.DocsURL.String())
566572
api.AppearanceFetcher.Store(&f)

‎coderd/database/dbauthz/dbauthz.go

+7
Original file line numberDiff line numberDiff line change
@@ -4615,6 +4615,13 @@ func (q *querier) UpsertWorkspaceAgentPortShare(ctx context.Context, arg databas
46154615
return q.db.UpsertWorkspaceAgentPortShare(ctx, arg)
46164616
}
46174617

4618+
func (q *querier) UpsertWorkspaceAppAuditSession(ctx context.Context, arg database.UpsertWorkspaceAppAuditSessionParams) (time.Time, error) {
4619+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil {
4620+
return time.Time{}, err
4621+
}
4622+
return q.db.UpsertWorkspaceAppAuditSession(ctx, arg)
4623+
}
4624+
46184625
func (q *querier) GetAuthorizedTemplates(ctx context.Context, arg database.GetTemplatesWithFilterParams, _ rbac.PreparedAuthorized) ([]database.Template, error) {
46194626
// TODO Delete this function, all GetTemplates should be authorized. For now just call getTemplates on the authz querier.
46204627
return q.GetTemplatesWithFilter(ctx, arg)

‎coderd/database/dbauthz/dbauthz_test.go

+13
Original file line numberDiff line numberDiff line change
@@ -4065,6 +4065,19 @@ func (s *MethodTestSuite) TestSystemFunctions() {
40654065
s.Run("InsertWorkspaceAppStats", s.Subtest(func(db database.Store, check *expects) {
40664066
check.Args(database.InsertWorkspaceAppStatsParams{}).Asserts(rbac.ResourceSystem, policy.ActionCreate)
40674067
}))
4068+
s.Run("UpsertWorkspaceAppAuditSession", s.Subtest(func(db database.Store, check *expects) {
4069+
u := dbgen.User(s.T(), db, database.User{})
4070+
pj := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{})
4071+
res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: pj.ID})
4072+
agent := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID})
4073+
app := dbgen.WorkspaceApp(s.T(), db, database.WorkspaceApp{AgentID: agent.ID})
4074+
check.Args(database.UpsertWorkspaceAppAuditSessionParams{
4075+
AgentID: agent.ID,
4076+
AppID: app.ID,
4077+
UserID: u.ID,
4078+
Ip: "127.0.0.1",
4079+
}).Asserts(rbac.ResourceSystem, policy.ActionUpdate)
4080+
}))
40684081
s.Run("InsertWorkspaceAgentScriptTimings", s.Subtest(func(db database.Store, check *expects) {
40694082
dbtestutil.DisableForeignKeysAndTriggers(s.T(), db)
40704083
check.Args(database.InsertWorkspaceAgentScriptTimingsParams{

‎coderd/database/dbmem/dbmem.go

+59
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ func New() database.Store {
9292
workspaceAgentLogs: make([]database.WorkspaceAgentLog, 0),
9393
workspaceBuilds: make([]database.WorkspaceBuild, 0),
9494
workspaceApps: make([]database.WorkspaceApp, 0),
95+
workspaceAppAuditSessions: make([]database.WorkspaceAppAuditSession, 0),
9596
workspaces: make([]database.WorkspaceTable, 0),
9697
workspaceProxies: make([]database.WorkspaceProxy, 0),
9798
},
@@ -237,6 +238,7 @@ type data struct {
237238
workspaceAgentMemoryResourceMonitors []database.WorkspaceAgentMemoryResourceMonitor
238239
workspaceAgentVolumeResourceMonitors []database.WorkspaceAgentVolumeResourceMonitor
239240
workspaceApps []database.WorkspaceApp
241+
workspaceAppAuditSessions []database.WorkspaceAppAuditSession
240242
workspaceAppStatsLastInsertID int64
241243
workspaceAppStats []database.WorkspaceAppStat
242244
workspaceBuilds []database.WorkspaceBuild
@@ -12281,6 +12283,63 @@ func (q *FakeQuerier) UpsertWorkspaceAgentPortShare(_ context.Context, arg datab
1228112283
return psl, nil
1228212284
}
1228312285

12286+
func (q *FakeQuerier) UpsertWorkspaceAppAuditSession(_ context.Context, arg database.UpsertWorkspaceAppAuditSessionParams) (time.Time, error) {
12287+
err := validateDatabaseType(arg)
12288+
if err != nil {
12289+
return time.Time{}, err
12290+
}
12291+
12292+
q.mutex.Lock()
12293+
defer q.mutex.Unlock()
12294+
12295+
for i, s := range q.workspaceAppAuditSessions {
12296+
if s.AgentID != arg.AgentID {
12297+
continue
12298+
}
12299+
if s.AppID != arg.AppID {
12300+
continue
12301+
}
12302+
if s.UserID != arg.UserID {
12303+
continue
12304+
}
12305+
if s.Ip != arg.Ip {
12306+
continue
12307+
}
12308+
if s.UserAgent != arg.UserAgent {
12309+
continue
12310+
}
12311+
if s.SlugOrPort != arg.SlugOrPort {
12312+
continue
12313+
}
12314+
if s.StatusCode != arg.StatusCode {
12315+
continue
12316+
}
12317+
12318+
staleTime := dbtime.Now().Add(-(time.Duration(arg.StaleIntervalMS) * time.Millisecond))
12319+
fresh := s.UpdatedAt.After(staleTime)
12320+
12321+
q.workspaceAppAuditSessions[i].UpdatedAt = arg.UpdatedAt
12322+
if !fresh {
12323+
q.workspaceAppAuditSessions[i].StartedAt = arg.StartedAt
12324+
return arg.StartedAt, nil
12325+
}
12326+
return s.StartedAt, nil
12327+
}
12328+
12329+
q.workspaceAppAuditSessions = append(q.workspaceAppAuditSessions, database.WorkspaceAppAuditSession{
12330+
AgentID: arg.AgentID,
12331+
AppID: arg.AppID,
12332+
UserID: arg.UserID,
12333+
Ip: arg.Ip,
12334+
UserAgent: arg.UserAgent,
12335+
SlugOrPort: arg.SlugOrPort,
12336+
StatusCode: arg.StatusCode,
12337+
StartedAt: arg.StartedAt,
12338+
UpdatedAt: arg.UpdatedAt,
12339+
})
12340+
return arg.StartedAt, nil
12341+
}
12342+
1228412343
func (q *FakeQuerier) GetAuthorizedTemplates(ctx context.Context, arg database.GetTemplatesWithFilterParams, prepared rbac.PreparedAuthorized) ([]database.Template, error) {
1228512344
if err := validateDatabaseType(arg); err != nil {
1228612345
return nil, err

‎coderd/database/dbmetrics/querymetrics.go

+7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

+15
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dump.sql

+42
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/foreign_key_constraint.go

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000301_add_workspace_app_audit_sessions.down.sql

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
DROP TABLE workspace_app_audit_sessions;

0 commit comments

Comments
 (0)