From 55c13c8ff98ea5abf7286486214e6811bb92eaa2 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Fri, 2 Sep 2022 11:42:28 -0500 Subject: [PATCH] chore: fully implement enterprise audit pkg (#3821) --- coderd/audit/exporter.go | 55 --------------- coderd/audit/filter.go | 42 ------------ coderd/audit/request.go | 65 +++++++++++++++--- coderd/database/time.go | 8 ++- enterprise/audit/audit.go | 33 +++++++-- .../audit/audit_test.go | 67 +++++++++---------- enterprise/audit/audittest/rand.go | 33 +++++++++ .../audit/backends/postgres.go | 2 +- .../audit/backends/postgres_test.go | 29 +------- {coderd => enterprise}/audit/backends/slog.go | 2 +- .../audit/backends/slog_test.go | 5 +- {coderd => enterprise}/audit/generate.sh | 0 12 files changed, 164 insertions(+), 177 deletions(-) delete mode 100644 coderd/audit/exporter.go delete mode 100644 coderd/audit/filter.go rename coderd/audit/exporter_test.go => enterprise/audit/audit_test.go (68%) create mode 100644 enterprise/audit/audittest/rand.go rename {coderd => enterprise}/audit/backends/postgres.go (95%) rename {coderd => enterprise}/audit/backends/postgres_test.go (50%) rename {coderd => enterprise}/audit/backends/slog.go (93%) rename {coderd => enterprise}/audit/backends/slog_test.go (85%) rename {coderd => enterprise}/audit/generate.sh (100%) diff --git a/coderd/audit/exporter.go b/coderd/audit/exporter.go deleted file mode 100644 index c9b37aec5f..0000000000 --- a/coderd/audit/exporter.go +++ /dev/null @@ -1,55 +0,0 @@ -package audit - -import ( - "context" - - "golang.org/x/xerrors" - - "github.com/coder/coder/coderd/database" -) - -// 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 -} - -// Exporter exports audit logs to an arbitrary list of backends. -type Exporter struct { - filter Filter - backends []Backend -} - -// NewExporter creates an exporter from the given filter and backends. -func NewExporter(filter Filter, backends ...Backend) *Exporter { - return &Exporter{ - filter: filter, - backends: backends, - } -} - -// Export exports and audit log. Before exporting to a backend, it uses the -// filter to determine if the backend tolerates the audit log. If not, it is -// dropped. -func (e *Exporter) Export(ctx context.Context, alog database.AuditLog) error { - decision, err := e.filter.Check(ctx, alog) - if err != nil { - return xerrors.Errorf("filter check: %w", err) - } - - for _, backend := range e.backends { - if decision&backend.Decision() != backend.Decision() { - continue - } - - err = backend.Export(ctx, alog) - if err != nil { - // naively return the first error. should probably make this smarter - // by returning multiple errors. - return xerrors.Errorf("export audit log to backend: %w", err) - } - } - return nil -} diff --git a/coderd/audit/filter.go b/coderd/audit/filter.go deleted file mode 100644 index 868d5bb7d7..0000000000 --- a/coderd/audit/filter.go +++ /dev/null @@ -1,42 +0,0 @@ -package audit - -import ( - "context" - - "github.com/coder/coder/coderd/database" -) - -// FilterDecision is a bitwise flag describing the actions a given filter allows -// for a given audit log. -type FilterDecision uint8 - -const ( - // FilterDecisionDrop indicates that the audit log should be dropped. It - // should not be stored or exported anywhere. - FilterDecisionDrop FilterDecision = 0 - // FilterDecisionStore indicates that the audit log should be allowed to be - // stored in the Coder database. - FilterDecisionStore FilterDecision = 1 << iota - // FilterDecisionExport indicates that the audit log should be exported - // externally of Coder. - FilterDecisionExport -) - -// Filters produce a FilterDecision for a given audit log. -type Filter interface { - Check(ctx context.Context, alog database.AuditLog) (FilterDecision, error) -} - -// DefaultFilter is the default filter used when exporting audit logs. It allows -// storage and exporting for all audit logs. -var DefaultFilter Filter = FilterFunc(func(ctx context.Context, alog database.AuditLog) (FilterDecision, error) { - // Store and export all audit logs for now. - return FilterDecisionStore | FilterDecisionExport, nil -}) - -// FilterFunc constructs a Filter from a simple function. -type FilterFunc func(ctx context.Context, alog database.AuditLog) (FilterDecision, error) - -func (f FilterFunc) Check(ctx context.Context, alog database.AuditLog) (FilterDecision, error) { - return f(ctx, alog) -} diff --git a/coderd/audit/request.go b/coderd/audit/request.go index aa3520b847..d547a2d3f4 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -2,22 +2,28 @@ package audit import ( "context" + "encoding/json" + "net" "net/http" - chimw "github.com/go-chi/chi/v5/middleware" "github.com/google/uuid" + "github.com/tabbed/pqtype" "cdr.dev/slog" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/httpapi" ) type RequestParams struct { Audit Auditor Log slog.Logger - Action database.AuditAction - ResourceType database.ResourceType - Actor uuid.UUID + Request *http.Request + ResourceID uuid.UUID + ResourceTarget string + Action database.AuditAction + ResourceType database.ResourceType + Actor uuid.UUID } type Request[T Auditable] struct { @@ -31,9 +37,9 @@ type Request[T Auditable] struct { // that should be deferred, causing the audit log to be committed when the // handler returns. func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request[T], func()) { - sw, ok := w.(chimw.WrapResponseWriter) + sw, ok := w.(*httpapi.StatusWriter) if !ok { - panic("dev error: http.ResponseWriter is not chimw.WrapResponseWriter") + panic("dev error: http.ResponseWriter is not *httpapi.StatusWriter") } req := &Request[T]{ @@ -42,11 +48,54 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request return req, func() { ctx := context.Background() - code := sw.Status() - err := p.Audit.Export(ctx, database.AuditLog{StatusCode: int32(code)}) + diff := Diff(p.Audit, req.Old, req.New) + diffRaw, _ := json.Marshal(diff) + + ip, err := parseIP(p.Request.RemoteAddr) + if err != nil { + p.Log.Warn(ctx, "parse ip", slog.Error(err)) + } + + err = p.Audit.Export(ctx, database.AuditLog{ + ID: uuid.New(), + Time: database.Now(), + UserID: p.Actor, + Ip: ip, + UserAgent: p.Request.UserAgent(), + ResourceType: p.ResourceType, + ResourceID: p.ResourceID, + ResourceTarget: p.ResourceTarget, + Action: p.Action, + Diff: diffRaw, + StatusCode: int32(sw.Status), + }) if err != nil { p.Log.Error(ctx, "export audit log", slog.Error(err)) } } } + +func parseIP(ipStr string) (pqtype.Inet, error) { + var err error + + ipStr, _, err = net.SplitHostPort(ipStr) + if err != nil { + return pqtype.Inet{}, err + } + + ip := net.ParseIP(ipStr) + + ipNet := net.IPNet{} + if ip != nil { + ipNet = net.IPNet{ + IP: ip, + Mask: net.CIDRMask(len(ip)*8, len(ip)*8), + } + } + + return pqtype.Inet{ + IPNet: ipNet, + Valid: ip != nil, + }, nil +} diff --git a/coderd/database/time.go b/coderd/database/time.go index 404a14fc96..290ddf228f 100644 --- a/coderd/database/time.go +++ b/coderd/database/time.go @@ -4,5 +4,11 @@ import "time" // Now returns a standardized timezone used for database resources. func Now() time.Time { - return time.Now().UTC() + return Time(time.Now().UTC()) +} + +// Time returns a time compatible with Postgres. Postgres only stores dates with +// microsecond precision. +func Time(t time.Time) time.Time { + return t.Round(time.Microsecond) } diff --git a/enterprise/audit/audit.go b/enterprise/audit/audit.go index f4a2796210..7b345f22f8 100644 --- a/enterprise/audit/audit.go +++ b/enterprise/audit/audit.go @@ -3,6 +3,8 @@ package audit import ( "context" + "golang.org/x/xerrors" + "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/database" ) @@ -15,8 +17,10 @@ type Backend interface { Export(ctx context.Context, alog database.AuditLog) error } -func NewAuditor() audit.Auditor { +func NewAuditor(filter Filter, backends ...Backend) audit.Auditor { return &auditor{ + filter: filter, + backends: backends, Differ: audit.Differ{DiffFn: func(old, new any) audit.Map { return diffValues(old, new, AuditableResources) }}, @@ -25,15 +29,30 @@ func NewAuditor() audit.Auditor { // auditor is the enterprise implementation of the Auditor interface. type auditor struct { - //nolint:unused - filter Filter - //nolint:unused + filter Filter backends []Backend audit.Differ } -//nolint:unused -func (*auditor) Export(context.Context, database.AuditLog) error { - panic("not implemented") // TODO: Implement +func (a *auditor) Export(ctx context.Context, alog database.AuditLog) error { + decision, err := a.filter.Check(ctx, alog) + if err != nil { + return xerrors.Errorf("filter check: %w", err) + } + + for _, backend := range a.backends { + if decision&backend.Decision() != backend.Decision() { + continue + } + + err = backend.Export(ctx, alog) + if err != nil { + // naively return the first error. should probably make this smarter + // by returning multiple errors. + return xerrors.Errorf("export audit log to backend: %w", err) + } + } + + return nil } diff --git a/coderd/audit/exporter_test.go b/enterprise/audit/audit_test.go similarity index 68% rename from coderd/audit/exporter_test.go rename to enterprise/audit/audit_test.go index 39376b4775..a4f76f2ec4 100644 --- a/coderd/audit/exporter_test.go +++ b/enterprise/audit/audit_test.go @@ -2,26 +2,25 @@ package audit_test import ( "context" - "net" - "net/http" "testing" - "time" - "github.com/google/uuid" "github.com/stretchr/testify/require" - "github.com/tabbed/pqtype" + "golang.org/x/xerrors" - "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/enterprise/audit" + "github.com/coder/coder/enterprise/audit/audittest" ) -func TestExporter(t *testing.T) { +func TestAuditor(t *testing.T) { t.Parallel() var tests = []struct { name string filterDecision audit.FilterDecision + filterError error backendDecision audit.FilterDecision + backendError error shouldExport bool }{ { @@ -60,11 +59,22 @@ func TestExporter(t *testing.T) { backendDecision: audit.FilterDecisionExport, shouldExport: true, }, + { + name: "FilterError", + filterError: xerrors.New("filter errored"), + backendDecision: audit.FilterDecisionExport, + shouldExport: false, + }, + { + name: "BackendError", + backendError: xerrors.New("backend errored"), + shouldExport: false, + }, // When more filters are written they should have their own tests. { name: "DefaultFilter", filterDecision: func() audit.FilterDecision { - decision, _ := audit.DefaultFilter.Check(context.Background(), randomAuditLog()) + decision, _ := audit.DefaultFilter.Check(context.Background(), audittest.RandomLog()) return decision }(), backendDecision: audit.FilterDecisionExport, @@ -78,45 +88,30 @@ func TestExporter(t *testing.T) { t.Parallel() var ( - backend = &testBackend{decision: test.backendDecision} - exporter = audit.NewExporter( + backend = &testBackend{decision: test.backendDecision, err: test.backendError} + exporter = audit.NewAuditor( audit.FilterFunc(func(_ context.Context, _ database.AuditLog) (audit.FilterDecision, error) { - return test.filterDecision, nil + return test.filterDecision, test.filterError }), backend, ) ) - err := exporter.Export(context.Background(), randomAuditLog()) - require.NoError(t, err) + err := exporter.Export(context.Background(), audittest.RandomLog()) + if test.filterError != nil { + require.ErrorIs(t, err, test.filterError) + } else if test.backendError != nil { + require.ErrorIs(t, err, test.backendError) + } + require.Equal(t, len(backend.alogs) > 0, test.shouldExport) }) } } -func randomAuditLog() database.AuditLog { - _, inet, _ := net.ParseCIDR("127.0.0.1/32") - return database.AuditLog{ - ID: uuid.New(), - Time: time.Now(), - UserID: uuid.New(), - OrganizationID: uuid.New(), - Ip: pqtype.Inet{ - 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", - ResourceType: database.ResourceTypeOrganization, - ResourceID: uuid.New(), - ResourceTarget: "colin's organization", - Action: database.AuditActionDelete, - Diff: []byte{}, - StatusCode: http.StatusNoContent, - } -} - type testBackend struct { decision audit.FilterDecision + err error alogs []database.AuditLog } @@ -126,6 +121,10 @@ func (t *testBackend) Decision() audit.FilterDecision { } func (t *testBackend) Export(_ context.Context, alog database.AuditLog) error { + if t.err != nil { + return t.err + } + t.alogs = append(t.alogs, alog) return nil } diff --git a/enterprise/audit/audittest/rand.go b/enterprise/audit/audittest/rand.go new file mode 100644 index 0000000000..ea44c8f8e6 --- /dev/null +++ b/enterprise/audit/audittest/rand.go @@ -0,0 +1,33 @@ +package audittest + +import ( + "net" + "net/http" + "time" + + "github.com/google/uuid" + "github.com/tabbed/pqtype" + + "github.com/coder/coder/coderd/database" +) + +func RandomLog() database.AuditLog { + _, inet, _ := net.ParseCIDR("127.0.0.1/32") + return database.AuditLog{ + ID: uuid.New(), + Time: time.Now(), + UserID: uuid.New(), + OrganizationID: uuid.New(), + Ip: pqtype.Inet{ + 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", + ResourceType: database.ResourceTypeOrganization, + ResourceID: uuid.New(), + ResourceTarget: "colin's organization", + Action: database.AuditActionDelete, + Diff: []byte("{}"), + StatusCode: http.StatusNoContent, + } +} diff --git a/coderd/audit/backends/postgres.go b/enterprise/audit/backends/postgres.go similarity index 95% rename from coderd/audit/backends/postgres.go rename to enterprise/audit/backends/postgres.go index 631fcb0d01..a0cadd2899 100644 --- a/coderd/audit/backends/postgres.go +++ b/enterprise/audit/backends/postgres.go @@ -5,8 +5,8 @@ import ( "golang.org/x/xerrors" - "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/enterprise/audit" ) type postgresBackend struct { diff --git a/coderd/audit/backends/postgres_test.go b/enterprise/audit/backends/postgres_test.go similarity index 50% rename from coderd/audit/backends/postgres_test.go rename to enterprise/audit/backends/postgres_test.go index 952443cd67..ceddbe3958 100644 --- a/coderd/audit/backends/postgres_test.go +++ b/enterprise/audit/backends/postgres_test.go @@ -2,18 +2,16 @@ package backends_test import ( "context" - "net" - "net/http" "testing" "time" "github.com/google/uuid" "github.com/stretchr/testify/require" - "github.com/tabbed/pqtype" - "github.com/coder/coder/coderd/audit/backends" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/database/databasefake" + "github.com/coder/coder/enterprise/audit/audittest" + "github.com/coder/coder/enterprise/audit/backends" ) func TestPostgresBackend(t *testing.T) { @@ -25,7 +23,7 @@ func TestPostgresBackend(t *testing.T) { ctx, cancel = context.WithCancel(context.Background()) db = databasefake.New() pgb = backends.NewPostgres(db, true) - alog = randomAuditLog() + alog = audittest.RandomLog() ) defer cancel() @@ -42,24 +40,3 @@ func TestPostgresBackend(t *testing.T) { require.Equal(t, alog, got[0]) }) } - -func randomAuditLog() database.AuditLog { - _, inet, _ := net.ParseCIDR("127.0.0.1/32") - return database.AuditLog{ - ID: uuid.New(), - Time: time.Now(), - UserID: uuid.New(), - OrganizationID: uuid.New(), - Ip: pqtype.Inet{ - 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", - ResourceType: database.ResourceTypeOrganization, - ResourceID: uuid.New(), - ResourceTarget: "colin's organization", - Action: database.AuditActionDelete, - Diff: []byte{}, - StatusCode: http.StatusNoContent, - } -} diff --git a/coderd/audit/backends/slog.go b/enterprise/audit/backends/slog.go similarity index 93% rename from coderd/audit/backends/slog.go rename to enterprise/audit/backends/slog.go index ad9191a0c6..c4c67c0932 100644 --- a/coderd/audit/backends/slog.go +++ b/enterprise/audit/backends/slog.go @@ -6,8 +6,8 @@ import ( "github.com/fatih/structs" "cdr.dev/slog" - "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/enterprise/audit" ) type slogBackend struct { diff --git a/coderd/audit/backends/slog_test.go b/enterprise/audit/backends/slog_test.go similarity index 85% rename from coderd/audit/backends/slog_test.go rename to enterprise/audit/backends/slog_test.go index 428637dd4b..c963746bf2 100644 --- a/coderd/audit/backends/slog_test.go +++ b/enterprise/audit/backends/slog_test.go @@ -8,7 +8,8 @@ import ( "github.com/stretchr/testify/require" "cdr.dev/slog" - "github.com/coder/coder/coderd/audit/backends" + "github.com/coder/coder/enterprise/audit/audittest" + "github.com/coder/coder/enterprise/audit/backends" ) func TestSlogBackend(t *testing.T) { @@ -23,7 +24,7 @@ func TestSlogBackend(t *testing.T) { logger = slog.Make(sink) backend = backends.NewSlog(logger) - alog = randomAuditLog() + alog = audittest.RandomLog() ) defer cancel() diff --git a/coderd/audit/generate.sh b/enterprise/audit/generate.sh similarity index 100% rename from coderd/audit/generate.sh rename to enterprise/audit/generate.sh