From 2a57ea757a8129510a6187188a460f69c8f8527f Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Mon, 25 Apr 2022 13:57:59 -0500 Subject: [PATCH] feat: add audit package (#1046) --- coderd/audit/diff.go | 111 ++++++++++++++++++++ coderd/audit/diff_internal_test.go | 163 +++++++++++++++++++++++++++++ coderd/audit/diff_test.go | 59 +++++++++++ coderd/audit/table.go | 73 +++++++++++++ coderd/coderd.go | 2 +- coderd/users.go | 2 +- go.mod | 1 + go.sum | 2 + 8 files changed, 411 insertions(+), 2 deletions(-) create mode 100644 coderd/audit/diff.go create mode 100644 coderd/audit/diff_internal_test.go create mode 100644 coderd/audit/diff_test.go create mode 100644 coderd/audit/table.go diff --git a/coderd/audit/diff.go b/coderd/audit/diff.go new file mode 100644 index 0000000000..ffd72df25d --- /dev/null +++ b/coderd/audit/diff.go @@ -0,0 +1,111 @@ +package audit + +import ( + "fmt" + "reflect" +) + +// TODO: this might need to be in the database package. +type Map map[string]interface{} + +func Empty[T Auditable]() T { + var t T + return t +} + +// Diff compares two auditable resources and produces a Map of the changed +// values. +func Diff[T Auditable](left, right T) Map { + // Values are equal, return an empty diff. + if reflect.DeepEqual(left, right) { + return Map{} + } + + return diffValues(left, right, AuditableResources) +} + +func structName(t reflect.Type) string { + return t.PkgPath() + "." + t.Name() +} + +func diffValues[T any](left, right T, table Table) Map { + var ( + baseDiff = Map{} + + leftV = reflect.ValueOf(left) + + rightV = reflect.ValueOf(right) + rightT = reflect.TypeOf(right) + + diffKey = table[structName(rightT)] + ) + + if diffKey == nil { + panic(fmt.Sprintf("dev error: type %q (type %T) attempted audit but not auditable", rightT.Name(), right)) + } + + for i := 0; i < rightT.NumField(); i++ { + var ( + leftF = leftV.Field(i) + rightF = rightV.Field(i) + + leftI = leftF.Interface() + rightI = rightF.Interface() + + diffName = rightT.Field(i).Tag.Get("json") + ) + + atype, ok := diffKey[diffName] + if !ok { + panic(fmt.Sprintf("dev error: field %q lacks audit information", diffName)) + } + + if atype == ActionIgnore { + continue + } + + // If the field is a pointer, dereference it. Nil pointers are coerced + // to the zero value of their underlying type. + if leftF.Kind() == reflect.Ptr && rightF.Kind() == reflect.Ptr { + leftF, rightF = derefPointer(leftF), derefPointer(rightF) + leftI, rightI = leftF.Interface(), rightF.Interface() + } + + // Recursively walk up nested structs. + if rightF.Kind() == reflect.Struct { + baseDiff[diffName] = diffValues(leftI, rightI, table) + continue + } + + if !reflect.DeepEqual(leftI, rightI) { + switch atype { + case ActionTrack: + baseDiff[diffName] = rightI + case ActionSecret: + baseDiff[diffName] = reflect.Zero(rightF.Type()).Interface() + } + } + } + + return baseDiff +} + +// derefPointer deferences a reflect.Value that is a pointer to its underlying +// value. It dereferences recursively until it finds a non-pointer value. If the +// pointer is nil, it will be coerced to the zero value of the underlying type. +func derefPointer(ptr reflect.Value) reflect.Value { + if !ptr.IsNil() { + // Grab the value the pointer references. + ptr = ptr.Elem() + } else { + // Coerce nil ptrs to zero'd values of their underlying type. + ptr = reflect.Zero(ptr.Type().Elem()) + } + + // Recursively deref nested pointers. + if ptr.Kind() == reflect.Ptr { + return derefPointer(ptr) + } + + return ptr +} diff --git a/coderd/audit/diff_internal_test.go b/coderd/audit/diff_internal_test.go new file mode 100644 index 0000000000..88d477a727 --- /dev/null +++ b/coderd/audit/diff_internal_test.go @@ -0,0 +1,163 @@ +package audit + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/utils/pointer" +) + +func Test_diffValues(t *testing.T) { + t.Parallel() + + t.Run("Normal", func(t *testing.T) { + t.Parallel() + + type foo struct { + Bar string `json:"bar"` + Baz int64 `json:"baz"` + } + + table := auditMap(map[any]map[string]Action{ + &foo{}: { + "bar": ActionTrack, + "baz": ActionTrack, + }, + }) + + runDiffTests(t, table, []diffTest{ + { + name: "LeftEmpty", + left: foo{Bar: "", Baz: 0}, right: foo{Bar: "bar", Baz: 10}, + exp: Map{ + "bar": "bar", + "baz": int64(10), + }, + }, + { + name: "RightEmpty", + left: foo{Bar: "Bar", Baz: 10}, right: foo{Bar: "", Baz: 0}, + exp: Map{ + "bar": "", + "baz": int64(0), + }, + }, + { + name: "NoChange", + left: foo{Bar: "", Baz: 0}, right: foo{Bar: "", Baz: 0}, + exp: Map{}, + }, + { + name: "SingleFieldChange", + left: foo{Bar: "", Baz: 0}, right: foo{Bar: "Bar", Baz: 0}, + exp: Map{ + "bar": "Bar", + }, + }, + }) + }) + + t.Run("PointerField", func(t *testing.T) { + t.Parallel() + + type foo struct { + Bar *string `json:"bar"` + } + + table := auditMap(map[any]map[string]Action{ + &foo{}: { + "bar": ActionTrack, + }, + }) + + runDiffTests(t, table, []diffTest{ + { + name: "LeftNil", + left: foo{Bar: nil}, right: foo{Bar: pointer.StringPtr("baz")}, + exp: Map{"bar": "baz"}, + }, + { + name: "RightNil", + left: foo{Bar: pointer.StringPtr("baz")}, right: foo{Bar: nil}, + exp: Map{"bar": ""}, + }, + }) + }) + + t.Run("NestedStruct", func(t *testing.T) { + t.Parallel() + + type bar struct { + Baz string `json:"baz"` + } + + type foo struct { + Bar *bar `json:"bar"` + } + + table := auditMap(map[any]map[string]Action{ + &foo{}: { + "bar": ActionTrack, + }, + &bar{}: { + "baz": ActionTrack, + }, + }) + + runDiffTests(t, table, []diffTest{ + { + name: "LeftEmpty", + left: foo{Bar: &bar{}}, right: foo{Bar: &bar{Baz: "baz"}}, + exp: Map{ + "bar": Map{ + "baz": "baz", + }, + }, + }, + { + name: "RightEmpty", + left: foo{Bar: &bar{Baz: "baz"}}, right: foo{Bar: &bar{}}, + exp: Map{ + "bar": Map{ + "baz": "", + }, + }, + }, + { + name: "LeftNil", + left: foo{Bar: nil}, right: foo{Bar: &bar{}}, + exp: Map{ + "bar": Map{}, + }, + }, + { + name: "RightNil", + left: foo{Bar: &bar{Baz: "baz"}}, right: foo{Bar: nil}, + exp: Map{ + "bar": Map{ + "baz": "", + }, + }, + }, + }) + }) +} + +type diffTest struct { + name string + left, right any + exp any +} + +func runDiffTests(t *testing.T, table Table, tests []diffTest) { + t.Helper() + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, + test.exp, + diffValues(test.left, test.right, table), + ) + }) + } +} diff --git a/coderd/audit/diff_test.go b/coderd/audit/diff_test.go new file mode 100644 index 0000000000..93aec75913 --- /dev/null +++ b/coderd/audit/diff_test.go @@ -0,0 +1,59 @@ +package audit_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/coderd/audit" + "github.com/coder/coder/coderd/database" +) + +func TestDiff(t *testing.T) { + t.Parallel() + + t.Run("Normal", func(t *testing.T) { + t.Parallel() + + runDiffTests(t, []diffTest[database.User]{ + { + name: "LeftEmpty", + left: audit.Empty[database.User](), right: database.User{Username: "colin", Email: "colin@coder.com"}, + exp: audit.Map{ + "email": "colin@coder.com", + }, + }, + { + name: "RightEmpty", + left: database.User{Username: "colin", Email: "colin@coder.com"}, right: audit.Empty[database.User](), + exp: audit.Map{ + "email": "", + }, + }, + { + name: "NoChange", + left: audit.Empty[database.User](), right: audit.Empty[database.User](), + exp: audit.Map{}, + }, + }) + }) +} + +type diffTest[T audit.Auditable] struct { + name string + left, right T + exp audit.Map +} + +func runDiffTests[T audit.Auditable](t *testing.T, tests []diffTest[T]) { + t.Helper() + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, + test.exp, + audit.Diff(test.left, test.right), + ) + }) + } +} diff --git a/coderd/audit/table.go b/coderd/audit/table.go new file mode 100644 index 0000000000..1b4cf2a92f --- /dev/null +++ b/coderd/audit/table.go @@ -0,0 +1,73 @@ +package audit + +import ( + "reflect" + + "github.com/coder/coder/coderd/database" +) + +// Auditable is mostly a marker interface. It contains a definitive list of all +// auditable types. If you want to audit a new type, first define it in +// AuditableResources, then add it to this interface. +type Auditable interface { + database.User | + database.Workspace +} + +type Action string + +const ( + // ActionIgnore ignores diffing for the field. + ActionIgnore = "ignore" + // ActionTrack includes the value in the diff if the value changed. + ActionTrack = "track" + // ActionSecret includes a zero value of the same type if the value changed. + // It lets you indicate that a value changed, but without leaking its + // contents. + ActionSecret = "secret" +) + +// Table is a map of struct names to a map of field names that indicate that +// field's AuditType. +type Table map[string]map[string]Action + +// AuditableResources contains a definitive list of all auditable resources and +// which fields are auditable. +var AuditableResources = auditMap(map[any]map[string]Action{ + &database.User{}: { + "id": ActionIgnore, // Never changes. + "email": ActionTrack, // A user can edit their email. + "username": ActionIgnore, // A user cannot change their username. + "hashed_password": ActionSecret, // A user can change their own password. + "created_at": ActionIgnore, // Never changes. + "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. + }, + &database.Workspace{}: { + "id": ActionIgnore, // Never changes. + "created_at": ActionIgnore, // Never changes. + "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. + "owner_id": ActionIgnore, // We don't allow workspaces to change ownership. + "template_id": ActionIgnore, // We don't allow workspaces to change templates. + "deleted": ActionIgnore, // Changes, but is implicit when a delete event is fired. + "name": ActionIgnore, // We don't allow workspaces to change names. + "autostart_schedule": ActionTrack, // Autostart schedules are directly editable by users. + "autostop_schedule": ActionTrack, // Autostart schedules are directly editable by users. + }, +}) + +// auditMap converts a map of struct pointers to a map of struct names as +// strings. It's a convenience wrapper so that structs can be passed in by value +// instead of manually typing struct names as strings. +func auditMap(m map[any]map[string]Action) Table { + out := make(Table, len(m)) + + for k, v := range m { + out[structName(reflect.TypeOf(k).Elem())] = v + } + + return out +} + +func (t Action) String() string { + return string(t) +} diff --git a/coderd/coderd.go b/coderd/coderd.go index 8679200a12..f0efad8013 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -167,7 +167,7 @@ func New(options *Options) (http.Handler, func()) { }) r.Group(func(r chi.Router) { r.Use(apiKeyMiddleware) - r.Post("/", api.postUsers) + r.Post("/", api.postUser) r.Get("/", api.users) r.Route("/{user}", func(r chi.Router) { r.Use(httpmw.ExtractUserParam(options.Database)) diff --git a/coderd/users.go b/coderd/users.go index 579fcffd7e..e5f78325d3 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -152,7 +152,7 @@ func (api *api) users(rw http.ResponseWriter, r *http.Request) { } // Creates a new user. -func (api *api) postUsers(rw http.ResponseWriter, r *http.Request) { +func (api *api) postUser(rw http.ResponseWriter, r *http.Request) { apiKey := httpmw.APIKey(r) var createUser codersdk.CreateUserRequest diff --git a/go.mod b/go.mod index 81d49d35fc..90ea7cc1ae 100644 --- a/go.mod +++ b/go.mod @@ -113,6 +113,7 @@ require ( google.golang.org/api v0.75.0 google.golang.org/protobuf v1.28.0 gopkg.in/DataDog/dd-trace-go.v1 v1.38.0 + k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 nhooyr.io/websocket v1.8.7 storj.io/drpc v0.0.30 ) diff --git a/go.sum b/go.sum index 5839d3054f..822bdd8dea 100644 --- a/go.sum +++ b/go.sum @@ -2469,6 +2469,8 @@ k8s.io/kube-openapi v0.0.0-20210305001622-591a79e4bda7/go.mod h1:wXW5VT87nVfh/iL k8s.io/kubernetes v1.13.0/go.mod h1:ocZa8+6APFNC2tX1DZASIbocyYT5jHzqFVsY5aoB7Jk= k8s.io/utils v0.0.0-20191114184206-e782cd3c129f/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew= k8s.io/utils v0.0.0-20201110183641-67b214c5f920/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= +k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 h1:HNSDgDCrr/6Ly3WEGKZftiE7IY19Vz2GdbOCyI4qqhc= +k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= mellium.im/sasl v0.2.1/go.mod h1:ROaEDLQNuf9vjKqE1SrAfnsobm2YKXT1gnN1uDp1PjQ= modernc.org/b v1.0.0/go.mod h1:uZWcZfRj1BpYzfN9JTerzlNUnnPsV9O2ZA8JsRcubNg= modernc.org/cc/v3 v3.32.4/go.mod h1:0R6jl1aZlIl2avnYfbfHBS1QB6/f+16mihBObaBC878=