Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat: add audit diffing for all user editable types#1413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
coadler merged 2 commits intomainfromcolin/audit-all-types
May 16, 2022
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletionscoderd/audit/diff.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
package audit

import (
"database/sql"
"fmt"
"reflect"

"github.com/google/uuid"
)

// TODO: this might need to be in the database package.
Expand DownExpand Up@@ -64,6 +67,11 @@ func diffValues[T any](left, right T, table Table) Map {
continue
}

// coerce struct types that would produce bad diffs.
if leftI, rightI, ok = convertDiffType(leftI, rightI); ok {
leftF, rightF = reflect.ValueOf(leftI), reflect.ValueOf(rightI)
}

// 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 {
Expand All@@ -90,6 +98,36 @@ func diffValues[T any](left, right T, table Table) Map {
return baseDiff
}

// convertDiffType converts external struct types to primitive types.
//nolint:forcetypeassert
func convertDiffType(left, right any) (newLeft, newRight any, changed bool) {
switch typed := left.(type) {
case uuid.UUID:
return typed.String(), right.(uuid.UUID).String(), true

case uuid.NullUUID:
leftStr, _ := typed.MarshalText()
rightStr, _ := right.(uuid.NullUUID).MarshalText()
return string(leftStr), string(rightStr), true

case sql.NullString:
leftStr := typed.String
if !typed.Valid {
leftStr = "null"
}

rightStr := right.(sql.NullString).String
if !right.(sql.NullString).Valid {
rightStr = "null"
}

return leftStr, rightStr, true

default:
return left, right, false
}
}

// 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.
Expand Down
218 changes: 194 additions & 24 deletionscoderd/audit/diff_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
package audit_test

import (
"database/sql"
"reflect"
"testing"
"time"

"github.com/google/uuid"
"github.com/stretchr/testify/require"

"github.com/coder/coder/coderd/audit"
Expand All@@ -12,30 +16,193 @@ import (
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{},
runDiffTests(t, []diffTest[database.GitSSHKey]{
{
name: "Create",
left: audit.Empty[database.GitSSHKey](),
right: database.GitSSHKey{
UserID: uuid.UUID{1},
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
PrivateKey: "a very secret private key",
PublicKey: "a very public public key",
},
})
exp: audit.Map{
"user_id": uuid.UUID{1}.String(),
"private_key": "",
"public_key": "a very public public key",
},
},
})

runDiffTests(t, []diffTest[database.OrganizationMember]{
{
name: "Create",
left: audit.Empty[database.OrganizationMember](),
right: database.OrganizationMember{
UserID: uuid.UUID{1},
OrganizationID: uuid.UUID{2},
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
Roles: []string{"auditor"},
},
exp: audit.Map{
"user_id": uuid.UUID{1}.String(),
"organization_id": uuid.UUID{2}.String(),
"roles": []string{"auditor"},
},
},
})

runDiffTests(t, []diffTest[database.Organization]{
{
name: "Create",
left: audit.Empty[database.Organization](),
right: database.Organization{
ID: uuid.UUID{1},
Name: "rust developers",
Description: "an organization for rust developers",
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
},
exp: audit.Map{
"id": uuid.UUID{1}.String(),
"name": "rust developers",
"description": "an organization for rust developers",
},
},
})

runDiffTests(t, []diffTest[database.Template]{
{
name: "Create",
left: audit.Empty[database.Template](),
right: database.Template{
ID: uuid.UUID{1},
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
OrganizationID: uuid.UUID{2},
Deleted: false,
Name: "rust",
Provisioner: database.ProvisionerTypeTerraform,
ActiveVersionID: uuid.UUID{3},
},
exp: audit.Map{
"id": uuid.UUID{1}.String(),
"organization_id": uuid.UUID{2}.String(),
"name": "rust",
"provisioner": database.ProvisionerTypeTerraform,
"active_version_id": uuid.UUID{3}.String(),
},
},
})

runDiffTests(t, []diffTest[database.TemplateVersion]{
{
name: "Create",
left: audit.Empty[database.TemplateVersion](),
right: database.TemplateVersion{
ID: uuid.UUID{1},
TemplateID: uuid.NullUUID{UUID: uuid.UUID{2}, Valid: true},
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
OrganizationID: uuid.UUID{3},
Name: "rust",
},
exp: audit.Map{
"id": uuid.UUID{1}.String(),
"template_id": uuid.UUID{2}.String(),
"organization_id": uuid.UUID{3}.String(),
"name": "rust",
},
},
{
name: "CreateNullTemplateID",
left: audit.Empty[database.TemplateVersion](),
right: database.TemplateVersion{
ID: uuid.UUID{1},
TemplateID: uuid.NullUUID{},
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
OrganizationID: uuid.UUID{3},
Name: "rust",
},
exp: audit.Map{
"id": uuid.UUID{1}.String(),
"organization_id": uuid.UUID{3}.String(),
"name": "rust",
},
},
})

runDiffTests(t, []diffTest[database.User]{
{
name: "Create",
left: audit.Empty[database.User](),
right: database.User{
ID: uuid.UUID{1},
Email: "colin@coder.com",
Username: "colin",
HashedPassword: []byte("hunter2ButHashed"),
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
Status: database.UserStatusActive,
RBACRoles: []string{"omega admin"},
},
exp: audit.Map{
"id": uuid.UUID{1}.String(),
"email": "colin@coder.com",
"username": "colin",
"hashed_password": ([]byte)(nil),
"status": database.UserStatusActive,
"rbac_roles": []string{"omega admin"},
},
},
})

runDiffTests(t, []diffTest[database.Workspace]{
{
name: "Create",
left: audit.Empty[database.Workspace](),
right: database.Workspace{
ID: uuid.UUID{1},
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
OwnerID: uuid.UUID{2},
TemplateID: uuid.UUID{3},
Name: "rust workspace",
AutostartSchedule: sql.NullString{String: "0 12 * * 1-5", Valid: true},
AutostopSchedule: sql.NullString{String: "0 2 * * 2-6", Valid: true},
},
exp: audit.Map{
"id": uuid.UUID{1}.String(),
"owner_id": uuid.UUID{2}.String(),
"template_id": uuid.UUID{3}.String(),
"name": "rust workspace",
"autostart_schedule": "0 12 * * 1-5",
"autostop_schedule": "0 2 * * 2-6",
},
},
{
name: "NullSchedules",
left: audit.Empty[database.Workspace](),
right: database.Workspace{
ID: uuid.UUID{1},
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
OwnerID: uuid.UUID{2},
TemplateID: uuid.UUID{3},
Name: "rust workspace",
AutostartSchedule: sql.NullString{},
AutostopSchedule: sql.NullString{},
},
exp: audit.Map{
"id": uuid.UUID{1}.String(),
"owner_id": uuid.UUID{2}.String(),
"template_id": uuid.UUID{3}.String(),
"name": "rust workspace",
},
},
})
}

Expand All@@ -48,8 +215,11 @@ type diffTest[T audit.Auditable] struct {
func runDiffTests[T audit.Auditable](t *testing.T, tests []diffTest[T]) {
t.Helper()

var typ T
typName := reflect.TypeOf(typ).Name()

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Run(typName+"/"+test.name, func(t *testing.T) {
require.Equal(t,
test.exp,
audit.Diff(test.left, test.right),
Expand Down
73 changes: 60 additions & 13 deletionscoderd/audit/table.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,12 @@ import (
// 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.GitSSHKey |
database.OrganizationMember |
database.Organization |
database.Template |
database.TemplateVersion |
database.User |
database.Workspace
}

Expand All@@ -34,26 +39,68 @@ 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.GitSSHKey{}: {
"user_id": ActionTrack,
"created_at": ActionIgnore, // Never changes, but is implicit and not helpful in a diff.
"updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff.
"private_key": ActionSecret, // We don't want to expose private keys in diffs.
"public_key": ActionTrack, // Public keys are ok to expose in a diff.
},
&database.OrganizationMember{}: {
"user_id": ActionTrack,
"organization_id": ActionTrack,
"created_at": ActionIgnore, // Never changes, but is implicit and not helpful in a diff.
"updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff.
"roles": ActionTrack,
},
&database.Organization{}: {
"id": ActionTrack,
"name": ActionTrack,
"description": ActionTrack,
"created_at": ActionIgnore, // Never changes, but is implicit and not helpful in a diff.
"updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff.
},
&database.Template{}: {
"id": ActionTrack,
"created_at": ActionIgnore, // Never changes, but is implicit and not helpful in a diff.
"updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff.
"organization_id": ActionTrack,
"deleted": ActionIgnore, // Changes, but is implicit when a delete event is fired.
"name": ActionTrack,
"provisioner": ActionTrack,
"active_version_id": ActionTrack,
},
&database.TemplateVersion{}: {
"id": ActionTrack,
"template_id": ActionTrack,
"organization_id": ActionTrack,
"created_at": ActionIgnore, // Never changes, but is implicit and not helpful in a diff.
"updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff.
"name": ActionTrack,
"description": ActionTrack,
"job_id": ActionIgnore, // Not helpful in a diff because jobs aren't tracked in audit logs.
},
&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.
"id":ActionTrack,
"email": ActionTrack,
"username":ActionTrack,
"hashed_password": ActionSecret, //Do not expose a users hashed password.
"created_at": ActionIgnore, // Never changes.
"updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff.
"status": ActionTrack, // A user can update another user status
"rbac_roles": ActionTrack, // A user's roles are mutable
"status": ActionTrack,
"rbac_roles": ActionTrack,
},
&database.Workspace{}: {
"id":ActionIgnore, // Never changes.
"id":ActionTrack,
"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.
"owner_id": ActionTrack,
"organization_id": ActionTrack,
"template_id": ActionTrack,
"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.
"name":ActionTrack,
"autostart_schedule": ActionTrack,
"autostop_schedule": ActionTrack,
},
})

Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp