From 4aeb39b494ff8191775ef01be2acd8c6ae53e7b5 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Wed, 30 Aug 2023 22:39:45 +0000 Subject: [PATCH 1/2] feat(enterprise/audit): add user object to slog exporter --- coderd/audit/request.go | 10 +++++--- enterprise/audit/audit.go | 30 +++++++++++++++++++--- enterprise/audit/audit_test.go | 16 +++++++++--- enterprise/audit/backends/postgres.go | 2 +- enterprise/audit/backends/postgres_test.go | 3 ++- enterprise/audit/backends/slog.go | 6 ++++- enterprise/audit/backends/slog_test.go | 13 +++++++--- enterprise/cli/server.go | 4 ++- enterprise/coderd/coderd_test.go | 5 ++-- 9 files changed, 69 insertions(+), 20 deletions(-) diff --git a/coderd/audit/request.go b/coderd/audit/request.go index 434ff401f3339..c472f06867c77 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -33,12 +33,14 @@ type Request[T Auditable] struct { Old T New T - // This optional field can be passed in when the userID cannot be determined from the API Key - // such as in the case of login, when the audit log is created prior the API Key's existence. + // UserID is an optional field can be passed in when the userID cannot be + // determined from the API Key such as in the case of login, when the audit + // log is created prior the API Key's existence. UserID uuid.UUID - // This optional field can be passed in if the AuditAction must be overridden - // such as in the case of new user authentication when the Audit Action is 'register', not 'login'. + // Action is an optional field can be passed in if the AuditAction must be + // overridden such as in the case of new user authentication when the Audit + // Action is 'register', not 'login'. Action database.AuditAction } diff --git a/enterprise/audit/audit.go b/enterprise/audit/audit.go index 5c745b8debc61..999923893043a 100644 --- a/enterprise/audit/audit.go +++ b/enterprise/audit/audit.go @@ -2,23 +2,37 @@ package audit import ( "context" + "database/sql" + "github.com/google/uuid" "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" ) +type BackendDetails struct { + Actor *Actor +} + +type Actor struct { + ID uuid.UUID `json:"id"` + Email string `json:"email"` + Username string `json:"username"` +} + // Backends can store or send audit logs to arbitrary locations. type Backend interface { // Decision determines the FilterDecisions that the backend tolerates. Decision() FilterDecision // Export sends an audit log to the backend. - Export(ctx context.Context, alog database.AuditLog) error + Export(ctx context.Context, alog database.AuditLog, details BackendDetails) error } -func NewAuditor(filter Filter, backends ...Backend) audit.Auditor { +func NewAuditor(db database.Store, filter Filter, backends ...Backend) audit.Auditor { return &auditor{ + db: db, filter: filter, backends: backends, Differ: audit.Differ{DiffFn: func(old, new any) audit.Map { @@ -29,6 +43,7 @@ func NewAuditor(filter Filter, backends ...Backend) audit.Auditor { // auditor is the enterprise implementation of the Auditor interface. type auditor struct { + db database.Store filter Filter backends []Backend @@ -41,12 +56,21 @@ func (a *auditor) Export(ctx context.Context, alog database.AuditLog) error { return xerrors.Errorf("filter check: %w", err) } + actor, err := a.db.GetUserByID(dbauthz.AsSystemRestricted(ctx), alog.UserID) //nolint + if err != nil && !xerrors.Is(err, sql.ErrNoRows) { + return err + } + for _, backend := range a.backends { if decision&backend.Decision() != backend.Decision() { continue } - err = backend.Export(ctx, alog) + err = backend.Export(ctx, alog, BackendDetails{Actor: &Actor{ + ID: actor.ID, + Email: actor.Email, + Username: actor.Username, + }}) if err != nil { // naively return the first error. should probably make this smarter // by returning multiple errors. diff --git a/enterprise/audit/audit_test.go b/enterprise/audit/audit_test.go index 421a637aa10c5..b4f5e0f2aab89 100644 --- a/enterprise/audit/audit_test.go +++ b/enterprise/audit/audit_test.go @@ -8,6 +8,7 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/enterprise/audit" "github.com/coder/coder/v2/enterprise/audit/audittest" ) @@ -90,6 +91,7 @@ func TestAuditor(t *testing.T) { var ( backend = &testBackend{decision: test.backendDecision, err: test.backendError} exporter = audit.NewAuditor( + dbfake.New(), audit.FilterFunc(func(_ context.Context, _ database.AuditLog) (audit.FilterDecision, error) { return test.filterDecision, test.filterError }), @@ -113,18 +115,26 @@ type testBackend struct { decision audit.FilterDecision err error - alogs []database.AuditLog + alogs []testExport +} + +type testExport struct { + alog database.AuditLog + details audit.BackendDetails } func (t *testBackend) Decision() audit.FilterDecision { return t.decision } -func (t *testBackend) Export(_ context.Context, alog database.AuditLog) error { +func (t *testBackend) Export(_ context.Context, alog database.AuditLog, details audit.BackendDetails) error { if t.err != nil { return t.err } - t.alogs = append(t.alogs, alog) + t.alogs = append(t.alogs, testExport{ + alog: alog, + details: details, + }) return nil } diff --git a/enterprise/audit/backends/postgres.go b/enterprise/audit/backends/postgres.go index 7d792dfdd0918..09f29acbe3464 100644 --- a/enterprise/audit/backends/postgres.go +++ b/enterprise/audit/backends/postgres.go @@ -30,7 +30,7 @@ func (b *postgresBackend) Decision() audit.FilterDecision { return audit.FilterDecisionExport } -func (b *postgresBackend) Export(ctx context.Context, alog database.AuditLog) error { +func (b *postgresBackend) Export(ctx context.Context, alog database.AuditLog, _ audit.BackendDetails) error { _, err := b.db.InsertAuditLog(ctx, database.InsertAuditLogParams(alog)) if err != nil { return xerrors.Errorf("insert audit log: %w", err) diff --git a/enterprise/audit/backends/postgres_test.go b/enterprise/audit/backends/postgres_test.go index c1360c6430004..b3fa1f31d0cbd 100644 --- a/enterprise/audit/backends/postgres_test.go +++ b/enterprise/audit/backends/postgres_test.go @@ -8,6 +8,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbfake" + "github.com/coder/coder/v2/enterprise/audit" "github.com/coder/coder/v2/enterprise/audit/audittest" "github.com/coder/coder/v2/enterprise/audit/backends" ) @@ -25,7 +26,7 @@ func TestPostgresBackend(t *testing.T) { ) defer cancel() - err := pgb.Export(ctx, alog) + err := pgb.Export(ctx, alog, audit.BackendDetails{}) require.NoError(t, err) got, err := db.GetAuditLogsOffset(ctx, database.GetAuditLogsOffsetParams{ diff --git a/enterprise/audit/backends/slog.go b/enterprise/audit/backends/slog.go index 4506357475626..c49ebae296ff0 100644 --- a/enterprise/audit/backends/slog.go +++ b/enterprise/audit/backends/slog.go @@ -24,7 +24,7 @@ func (*slogBackend) Decision() audit.FilterDecision { return audit.FilterDecisionExport } -func (b *slogBackend) Export(ctx context.Context, alog database.AuditLog) error { +func (b *slogBackend) Export(ctx context.Context, alog database.AuditLog, details audit.BackendDetails) error { // We don't use structs.Map because we don't want to recursively convert // fields into maps. When we keep the type information, slog can more // pleasantly format the output. For example, the clean result of @@ -35,6 +35,10 @@ func (b *slogBackend) Export(ctx context.Context, alog database.AuditLog) error fields = append(fields, b.fieldToSlog(sf)) } + if details.Actor != nil { + fields = append(fields, slog.F("actor", details.Actor)) + } + b.log.Info(ctx, "audit_log", fields...) return nil } diff --git a/enterprise/audit/backends/slog_test.go b/enterprise/audit/backends/slog_test.go index ff42779a773ab..5fe3cf70c519a 100644 --- a/enterprise/audit/backends/slog_test.go +++ b/enterprise/audit/backends/slog_test.go @@ -19,6 +19,7 @@ import ( "cdr.dev/slog" "cdr.dev/slog/sloggers/slogjson" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/enterprise/audit" "github.com/coder/coder/v2/enterprise/audit/audittest" "github.com/coder/coder/v2/enterprise/audit/backends" ) @@ -39,7 +40,7 @@ func TestSlogBackend(t *testing.T) { ) defer cancel() - err := backend.Export(ctx, alog) + err := backend.Export(ctx, alog, audit.BackendDetails{}) require.NoError(t, err) require.Len(t, sink.entries, 1) require.Equal(t, sink.entries[0].Message, "audit_log") @@ -59,7 +60,7 @@ func TestSlogBackend(t *testing.T) { _, inet, _ = net.ParseCIDR("127.0.0.1/32") alog = database.AuditLog{ ID: uuid.UUID{1}, - Time: time.Unix(1257894000, 0), + Time: time.Unix(1257894000, 0).UTC(), UserID: uuid.UUID{2}, OrganizationID: uuid.UUID{3}, Ip: pqtype.Inet{ @@ -80,7 +81,11 @@ func TestSlogBackend(t *testing.T) { ) defer cancel() - err := backend.Export(ctx, alog) + err := backend.Export(ctx, alog, audit.BackendDetails{Actor: &audit.Actor{ + ID: uuid.UUID{2}, + Username: "coadler", + Email: "doug@coder.com", + }}) require.NoError(t, err) logger.Sync() @@ -90,7 +95,7 @@ func TestSlogBackend(t *testing.T) { err = json.Unmarshal(buf.Bytes(), &s) require.NoError(t, err) - expected := `{"ID":"01000000-0000-0000-0000-000000000000","Time":"2009-11-10T23:00:00Z","UserID":"02000000-0000-0000-0000-000000000000","OrganizationID":"03000000-0000-0000-0000-000000000000","Ip":"127.0.0.1","UserAgent":"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.127 Safari/537.36","ResourceType":"organization","ResourceID":"04000000-0000-0000-0000-000000000000","ResourceTarget":"colin's organization","Action":"delete","Diff":{"1":2},"StatusCode":204,"AdditionalFields":{"name":"doug","species":"cat"},"RequestID":"05000000-0000-0000-0000-000000000000","ResourceIcon":"photo.png"}` + expected := `{"ID":"01000000-0000-0000-0000-000000000000","Time":"2009-11-10T23:00:00Z","UserID":"02000000-0000-0000-0000-000000000000","OrganizationID":"03000000-0000-0000-0000-000000000000","Ip":"127.0.0.1","UserAgent":"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.127 Safari/537.36","ResourceType":"organization","ResourceID":"04000000-0000-0000-0000-000000000000","ResourceTarget":"colin's organization","Action":"delete","Diff":{"1":2},"StatusCode":204,"AdditionalFields":{"name":"doug","species":"cat"},"RequestID":"05000000-0000-0000-0000-000000000000","ResourceIcon":"photo.png","actor":{"id":"02000000-0000-0000-0000-000000000000","email":"doug@coder.com","username":"coadler"}}` assert.Equal(t, expected, string(s.Fields)) }) } diff --git a/enterprise/cli/server.go b/enterprise/cli/server.go index 197eab61e10e9..021d767414d54 100644 --- a/enterprise/cli/server.go +++ b/enterprise/cli/server.go @@ -49,7 +49,9 @@ func (r *RootCmd) server() *clibase.Cmd { } } options.DERPServer.SetMeshKey(meshKey) - options.Auditor = audit.NewAuditor(audit.DefaultFilter, + options.Auditor = audit.NewAuditor( + options.Database, + audit.DefaultFilter, backends.NewPostgres(options.Database, true), backends.NewSlog(options.Logger), ) diff --git a/enterprise/coderd/coderd_test.go b/enterprise/coderd/coderd_test.go index 123a22938fff2..c4f3b92e8f3a2 100644 --- a/enterprise/coderd/coderd_test.go +++ b/enterprise/coderd/coderd_test.go @@ -9,6 +9,7 @@ import ( "github.com/google/uuid" "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/coderd/rbac" "github.com/stretchr/testify/assert" @@ -185,7 +186,7 @@ func TestAuditLogging(t *testing.T) { _, _, api, _ := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ AuditLogging: true, Options: &coderdtest.Options{ - Auditor: audit.NewAuditor(audit.DefaultFilter), + Auditor: audit.NewAuditor(dbfake.New(), audit.DefaultFilter), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{ @@ -194,7 +195,7 @@ func TestAuditLogging(t *testing.T) { }, }) auditor := *api.AGPL.Auditor.Load() - ea := audit.NewAuditor(audit.DefaultFilter) + ea := audit.NewAuditor(dbfake.New(), audit.DefaultFilter) t.Logf("%T = %T", auditor, ea) assert.EqualValues(t, reflect.ValueOf(ea).Type(), reflect.ValueOf(auditor).Type()) }) From 0a880c6b8d3a98aa7db61eda66c01800e3945dee Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Wed, 30 Aug 2023 22:44:32 +0000 Subject: [PATCH 2/2] fixup! feat(enterprise/audit): add user object to slog exporter --- enterprise/coderd/coderd_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/enterprise/coderd/coderd_test.go b/enterprise/coderd/coderd_test.go index c4f3b92e8f3a2..968cb0bb83946 100644 --- a/enterprise/coderd/coderd_test.go +++ b/enterprise/coderd/coderd_test.go @@ -7,11 +7,6 @@ import ( "time" "github.com/google/uuid" - - "github.com/coder/coder/v2/coderd/database/dbauthz" - "github.com/coder/coder/v2/coderd/database/dbfake" - "github.com/coder/coder/v2/coderd/rbac" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/goleak" @@ -19,6 +14,9 @@ import ( agplaudit "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbfake" + "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/audit" "github.com/coder/coder/v2/enterprise/coderd"