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

Commita17dafc

Browse files
committed
feat: add audit diffing for all user editable types
1 parent2a278b8 commita17dafc

File tree

4 files changed

+294
-39
lines changed

4 files changed

+294
-39
lines changed

‎coderd/audit/diff.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
package audit
22

33
import (
4+
"database/sql"
45
"fmt"
56
"reflect"
7+
8+
"github.com/google/uuid"
69
)
710

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

70+
// coerce struct types that would produce bad diffs.
71+
ifleftI,rightI,ok=convertDiffType(leftI,rightI);ok {
72+
leftF,rightF=reflect.ValueOf(leftI),reflect.ValueOf(rightI)
73+
}
74+
6775
// If the field is a pointer, dereference it. Nil pointers are coerced
6876
// to the zero value of their underlying type.
6977
ifleftF.Kind()==reflect.Ptr&&rightF.Kind()==reflect.Ptr {
@@ -90,6 +98,36 @@ func diffValues[T any](left, right T, table Table) Map {
9098
returnbaseDiff
9199
}
92100

101+
// convertDiffType converts external struct types to primitive types.
102+
//nolint:forcetypeassert
103+
funcconvertDiffType(left,rightany) (newLeft,newRightany,changedbool) {
104+
switchtyped:=left.(type) {
105+
case uuid.UUID:
106+
returntyped.String(),right.(uuid.UUID).String(),true
107+
108+
case uuid.NullUUID:
109+
leftStr,_:=typed.MarshalText()
110+
rightStr,_:=right.(uuid.NullUUID).MarshalText()
111+
returnstring(leftStr),string(rightStr),true
112+
113+
case sql.NullString:
114+
leftStr:=typed.String
115+
if!typed.Valid {
116+
leftStr="null"
117+
}
118+
119+
rightStr:=right.(sql.NullString).String
120+
if!right.(sql.NullString).Valid {
121+
rightStr="null"
122+
}
123+
124+
returnleftStr,rightStr,true
125+
126+
default:
127+
returnleft,right,false
128+
}
129+
}
130+
93131
// derefPointer deferences a reflect.Value that is a pointer to its underlying
94132
// value. It dereferences recursively until it finds a non-pointer value. If the
95133
// pointer is nil, it will be coerced to the zero value of the underlying type.

‎coderd/audit/diff_test.go

Lines changed: 194 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package audit_test
22

33
import (
4+
"database/sql"
5+
"reflect"
46
"testing"
7+
"time"
58

9+
"github.com/google/uuid"
610
"github.com/stretchr/testify/require"
711

812
"github.com/coder/coder/coderd/audit"
@@ -12,30 +16,193 @@ import (
1216
funcTestDiff(t*testing.T) {
1317
t.Parallel()
1418

15-
t.Run("Normal",func(t*testing.T) {
16-
t.Parallel()
17-
18-
runDiffTests(t, []diffTest[database.User]{
19-
{
20-
name:"LeftEmpty",
21-
left:audit.Empty[database.User](),right: database.User{Username:"colin",Email:"colin@coder.com"},
22-
exp: audit.Map{
23-
"email":"colin@coder.com",
24-
},
25-
},
26-
{
27-
name:"RightEmpty",
28-
left: database.User{Username:"colin",Email:"colin@coder.com"},right:audit.Empty[database.User](),
29-
exp: audit.Map{
30-
"email":"",
31-
},
32-
},
33-
{
34-
name:"NoChange",
35-
left:audit.Empty[database.User](),right:audit.Empty[database.User](),
36-
exp: audit.Map{},
19+
runDiffTests(t, []diffTest[database.GitSSHKey]{
20+
{
21+
name:"Create",
22+
left:audit.Empty[database.GitSSHKey](),
23+
right: database.GitSSHKey{
24+
UserID: uuid.UUID{1},
25+
CreatedAt:time.Now(),
26+
UpdatedAt:time.Now(),
27+
PrivateKey:"a very secret private key",
28+
PublicKey:"a very public public key",
3729
},
38-
})
30+
exp: audit.Map{
31+
"user_id": uuid.UUID{1}.String(),
32+
"private_key":"",
33+
"public_key":"a very public public key",
34+
},
35+
},
36+
})
37+
38+
runDiffTests(t, []diffTest[database.OrganizationMember]{
39+
{
40+
name:"Create",
41+
left:audit.Empty[database.OrganizationMember](),
42+
right: database.OrganizationMember{
43+
UserID: uuid.UUID{1},
44+
OrganizationID: uuid.UUID{2},
45+
CreatedAt:time.Now(),
46+
UpdatedAt:time.Now(),
47+
Roles: []string{"auditor"},
48+
},
49+
exp: audit.Map{
50+
"user_id": uuid.UUID{1}.String(),
51+
"organization_id": uuid.UUID{2}.String(),
52+
"roles": []string{"auditor"},
53+
},
54+
},
55+
})
56+
57+
runDiffTests(t, []diffTest[database.Organization]{
58+
{
59+
name:"Create",
60+
left:audit.Empty[database.Organization](),
61+
right: database.Organization{
62+
ID: uuid.UUID{1},
63+
Name:"rust developers",
64+
Description:"an organization for rust developers",
65+
CreatedAt:time.Now(),
66+
UpdatedAt:time.Now(),
67+
},
68+
exp: audit.Map{
69+
"id": uuid.UUID{1}.String(),
70+
"name":"rust developers",
71+
"description":"an organization for rust developers",
72+
},
73+
},
74+
})
75+
76+
runDiffTests(t, []diffTest[database.Template]{
77+
{
78+
name:"Create",
79+
left:audit.Empty[database.Template](),
80+
right: database.Template{
81+
ID: uuid.UUID{1},
82+
CreatedAt:time.Now(),
83+
UpdatedAt:time.Now(),
84+
OrganizationID: uuid.UUID{2},
85+
Deleted:false,
86+
Name:"rust",
87+
Provisioner:database.ProvisionerTypeTerraform,
88+
ActiveVersionID: uuid.UUID{3},
89+
},
90+
exp: audit.Map{
91+
"id": uuid.UUID{1}.String(),
92+
"organization_id": uuid.UUID{2}.String(),
93+
"name":"rust",
94+
"provisioner":database.ProvisionerTypeTerraform,
95+
"active_version_id": uuid.UUID{3}.String(),
96+
},
97+
},
98+
})
99+
100+
runDiffTests(t, []diffTest[database.TemplateVersion]{
101+
{
102+
name:"Create",
103+
left:audit.Empty[database.TemplateVersion](),
104+
right: database.TemplateVersion{
105+
ID: uuid.UUID{1},
106+
TemplateID: uuid.NullUUID{UUID: uuid.UUID{2},Valid:true},
107+
CreatedAt:time.Now(),
108+
UpdatedAt:time.Now(),
109+
OrganizationID: uuid.UUID{3},
110+
Name:"rust",
111+
},
112+
exp: audit.Map{
113+
"id": uuid.UUID{1}.String(),
114+
"template_id": uuid.UUID{2}.String(),
115+
"organization_id": uuid.UUID{3}.String(),
116+
"name":"rust",
117+
},
118+
},
119+
{
120+
name:"CreateNullTemplateID",
121+
left:audit.Empty[database.TemplateVersion](),
122+
right: database.TemplateVersion{
123+
ID: uuid.UUID{1},
124+
TemplateID: uuid.NullUUID{},
125+
CreatedAt:time.Now(),
126+
UpdatedAt:time.Now(),
127+
OrganizationID: uuid.UUID{3},
128+
Name:"rust",
129+
},
130+
exp: audit.Map{
131+
"id": uuid.UUID{1}.String(),
132+
"organization_id": uuid.UUID{3}.String(),
133+
"name":"rust",
134+
},
135+
},
136+
})
137+
138+
runDiffTests(t, []diffTest[database.User]{
139+
{
140+
name:"Create",
141+
left:audit.Empty[database.User](),
142+
right: database.User{
143+
ID: uuid.UUID{1},
144+
Email:"colin@coder.com",
145+
Username:"colin",
146+
HashedPassword: []byte("hunter2ButHashed"),
147+
CreatedAt:time.Now(),
148+
UpdatedAt:time.Now(),
149+
Status:database.UserStatusActive,
150+
RBACRoles: []string{"omega admin"},
151+
},
152+
exp: audit.Map{
153+
"id": uuid.UUID{1}.String(),
154+
"email":"colin@coder.com",
155+
"username":"colin",
156+
"hashed_password": ([]byte)(nil),
157+
"status":database.UserStatusActive,
158+
"rbac_roles": []string{"omega admin"},
159+
},
160+
},
161+
})
162+
163+
runDiffTests(t, []diffTest[database.Workspace]{
164+
{
165+
name:"Create",
166+
left:audit.Empty[database.Workspace](),
167+
right: database.Workspace{
168+
ID: uuid.UUID{1},
169+
CreatedAt:time.Now(),
170+
UpdatedAt:time.Now(),
171+
OwnerID: uuid.UUID{2},
172+
TemplateID: uuid.UUID{3},
173+
Name:"rust workspace",
174+
AutostartSchedule: sql.NullString{String:"0 12 * * 1-5",Valid:true},
175+
AutostopSchedule: sql.NullString{String:"0 2 * * 2-6",Valid:true},
176+
},
177+
exp: audit.Map{
178+
"id": uuid.UUID{1}.String(),
179+
"owner_id": uuid.UUID{2}.String(),
180+
"template_id": uuid.UUID{3}.String(),
181+
"name":"rust workspace",
182+
"autostart_schedule":"0 12 * * 1-5",
183+
"autostop_schedule":"0 2 * * 2-6",
184+
},
185+
},
186+
{
187+
name:"NullSchedules",
188+
left:audit.Empty[database.Workspace](),
189+
right: database.Workspace{
190+
ID: uuid.UUID{1},
191+
CreatedAt:time.Now(),
192+
UpdatedAt:time.Now(),
193+
OwnerID: uuid.UUID{2},
194+
TemplateID: uuid.UUID{3},
195+
Name:"rust workspace",
196+
AutostartSchedule: sql.NullString{},
197+
AutostopSchedule: sql.NullString{},
198+
},
199+
exp: audit.Map{
200+
"id": uuid.UUID{1}.String(),
201+
"owner_id": uuid.UUID{2}.String(),
202+
"template_id": uuid.UUID{3}.String(),
203+
"name":"rust workspace",
204+
},
205+
},
39206
})
40207
}
41208

@@ -48,8 +215,11 @@ type diffTest[T audit.Auditable] struct {
48215
funcrunDiffTests[T audit.Auditable](t*testing.T,tests []diffTest[T]) {
49216
t.Helper()
50217

218+
vartypT
219+
typName:=reflect.TypeOf(typ).Name()
220+
51221
for_,test:=rangetests {
52-
t.Run(test.name,func(t*testing.T) {
222+
t.Run(typName+"/"+test.name,func(t*testing.T) {
53223
require.Equal(t,
54224
test.exp,
55225
audit.Diff(test.left,test.right),

‎coderd/audit/table.go

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,12 @@ import (
1010
// auditable types. If you want to audit a new type, first define it in
1111
// AuditableResources, then add it to this interface.
1212
typeAuditableinterface {
13-
database.User|
13+
database.GitSSHKey|
14+
database.OrganizationMember|
15+
database.Organization|
16+
database.Template|
17+
database.TemplateVersion|
18+
database.User|
1419
database.Workspace
1520
}
1621

@@ -34,26 +39,68 @@ type Table map[string]map[string]Action
3439
// AuditableResources contains a definitive list of all auditable resources and
3540
// which fields are auditable.
3641
varAuditableResources=auditMap(map[any]map[string]Action{
42+
&database.GitSSHKey{}: {
43+
"user_id":ActionTrack,
44+
"created_at":ActionIgnore,// Never changes, but is implicit and not helpful in a diff.
45+
"updated_at":ActionIgnore,// Changes, but is implicit and not helpful in a diff.
46+
"private_key":ActionSecret,// We don't want to expose private keys in diffs.
47+
"public_key":ActionTrack,// Public keys are ok to expose in a diff.
48+
},
49+
&database.OrganizationMember{}: {
50+
"user_id":ActionTrack,
51+
"organization_id":ActionTrack,
52+
"created_at":ActionIgnore,// Never changes, but is implicit and not helpful in a diff.
53+
"updated_at":ActionIgnore,// Changes, but is implicit and not helpful in a diff.
54+
"roles":ActionTrack,
55+
},
56+
&database.Organization{}: {
57+
"id":ActionTrack,
58+
"name":ActionTrack,
59+
"description":ActionTrack,
60+
"created_at":ActionIgnore,// Never changes, but is implicit and not helpful in a diff.
61+
"updated_at":ActionIgnore,// Changes, but is implicit and not helpful in a diff.
62+
},
63+
&database.Template{}: {
64+
"id":ActionTrack,
65+
"created_at":ActionIgnore,// Never changes, but is implicit and not helpful in a diff.
66+
"updated_at":ActionIgnore,// Changes, but is implicit and not helpful in a diff.
67+
"organization_id":ActionTrack,
68+
"deleted":ActionIgnore,// Changes, but is implicit when a delete event is fired.
69+
"name":ActionTrack,
70+
"provisioner":ActionTrack,
71+
"active_version_id":ActionTrack,
72+
},
73+
&database.TemplateVersion{}: {
74+
"id":ActionTrack,
75+
"template_id":ActionTrack,
76+
"organization_id":ActionTrack,
77+
"created_at":ActionIgnore,// Never changes, but is implicit and not helpful in a diff.
78+
"updated_at":ActionIgnore,// Changes, but is implicit and not helpful in a diff.
79+
"name":ActionTrack,
80+
"description":ActionTrack,
81+
"job_id":ActionIgnore,// Not helpful in a diff because jobs aren't tracked in audit logs.
82+
},
3783
&database.User{}: {
38-
"id":ActionIgnore,// Never changes.
39-
"email":ActionTrack,// A user can edit their email.
40-
"username":ActionIgnore,// A user cannot change their username.
41-
"hashed_password":ActionSecret,//A user can change their own password.
84+
"id":ActionTrack,
85+
"email":ActionTrack,
86+
"username":ActionTrack,
87+
"hashed_password":ActionSecret,//Do not expose a users hashed password.
4288
"created_at":ActionIgnore,// Never changes.
4389
"updated_at":ActionIgnore,// Changes, but is implicit and not helpful in a diff.
44-
"status":ActionTrack,// A user can update another user status
45-
"rbac_roles":ActionTrack,// A user's roles are mutable
90+
"status":ActionTrack,
91+
"rbac_roles":ActionTrack,
4692
},
4793
&database.Workspace{}: {
48-
"id":ActionIgnore,// Never changes.
94+
"id":ActionTrack,
4995
"created_at":ActionIgnore,// Never changes.
5096
"updated_at":ActionIgnore,// Changes, but is implicit and not helpful in a diff.
51-
"owner_id":ActionIgnore,// We don't allow workspaces to change ownership.
52-
"template_id":ActionIgnore,// We don't allow workspaces to change templates.
97+
"owner_id":ActionTrack,
98+
"organization_id":ActionTrack,
99+
"template_id":ActionTrack,
53100
"deleted":ActionIgnore,// Changes, but is implicit when a delete event is fired.
54-
"name":ActionIgnore,// We don't allow workspaces to change names.
55-
"autostart_schedule":ActionTrack,// Autostart schedules are directly editable by users.
56-
"autostop_schedule":ActionTrack,// Autostart schedules are directly editable by users.
101+
"name":ActionTrack,
102+
"autostart_schedule":ActionTrack,
103+
"autostop_schedule":ActionTrack,
57104
},
58105
})
59106

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp