- Notifications
You must be signed in to change notification settings - Fork927
feat: Auditing group members as part of group resource#5730
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
Uh oh!
There was an error while loading.Please reload this page.
Changes fromall commits
6c73cf3
40842b9
f4e5801
c478e9a
ed17aa8
25bbb3a
6b5f134
d33b330
240c004
921107c
2ffcab8
66acf51
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,38 @@ | ||
package database | ||
import ( | ||
"sort" | ||
"github.com/coder/coder/coderd/rbac" | ||
) | ||
typeAuditableGroupstruct { | ||
Group | ||
Members []GroupMember`json:"members"` | ||
} | ||
// Auditable returns an object that can be used in audit logs. | ||
// Covers both group and group member changes. | ||
func (gGroup)Auditable(users []User)AuditableGroup { | ||
Kira-Pilot marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
members:=make([]GroupMember,0,len(users)) | ||
for_,u:=rangeusers { | ||
members=append(members,GroupMember{ | ||
UserID:u.ID, | ||
GroupID:g.ID, | ||
}) | ||
} | ||
// consistent ordering | ||
sort.Slice(members,func(i,jint)bool { | ||
returnmembers[i].UserID.String()<members[j].UserID.String() | ||
}) | ||
returnAuditableGroup{ | ||
Group:g, | ||
Members:members, | ||
} | ||
} | ||
constAllUsersGroup="Everyone" | ||
func (sAPIKeyScope)ToRBAC() rbac.Scope { | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -105,13 +105,6 @@ var AuditableResources = auditMap(map[any]map[string]Action{ | ||
"ttl":ActionTrack, | ||
"last_used_at":ActionIgnore, | ||
}, | ||
// We don't show any diff for the WorkspaceBuild resource | ||
&database.WorkspaceBuild{}: { | ||
"id":ActionIgnore, | ||
@@ -128,6 +121,14 @@ var AuditableResources = auditMap(map[any]map[string]Action{ | ||
"reason":ActionIgnore, | ||
"daily_cost":ActionIgnore, | ||
}, | ||
&database.AuditableGroup{}: { | ||
"id":ActionTrack, | ||
"name":ActionTrack, | ||
"organization_id":ActionIgnore,// Never changes. | ||
"avatar_url":ActionTrack, | ||
"quota_allowance":ActionTrack, | ||
"members":ActionTrack, | ||
Kira-Pilot marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
}, | ||
}) | ||
// auditMap converts a map of struct pointers to a map of struct names as | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -16,6 +16,7 @@ import userAgentParser from "ua-parser-js" | ||
import{AuditLogDiff}from"./AuditLogDiff" | ||
importi18nextfrom"i18next" | ||
import{AuditLogDescription}from"./AuditLogDescription" | ||
import{determineGroupDiff}from"./auditUtils" | ||
consthttpStatusColor=(httpStatus:number):PaletteIndex=>{ | ||
if(httpStatus>=300&&httpStatus<500){ | ||
@@ -49,6 +50,13 @@ export const AuditLogRow: React.FC<AuditLogRowProps> = ({ | ||
?`${browser.name}${browser.version}` | ||
:t("auditLog:table.logRow.notAvailable") | ||
letauditDiff=auditLog.diff | ||
// groups have nested diffs (group members) | ||
if(auditLog.resource_type==="group"){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Could you pull this out into a util function, maybe with a unit test? | ||
auditDiff=determineGroupDiff(auditLog.diff) | ||
} | ||
consttoggle=()=>{ | ||
if(shouldDisplayDiff){ | ||
setIsDiffOpen((v)=>!v) | ||
@@ -153,7 +161,7 @@ export const AuditLogRow: React.FC<AuditLogRowProps> = ({ | ||
{shouldDisplayDiff&&( | ||
<Collapsein={isDiffOpen}> | ||
<AuditLogDiffdiff={auditDiff}/> | ||
</Collapse> | ||
)} | ||
</TableCell> | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
import { determineGroupDiff } from "./auditUtils" | ||
const auditDiffForNewGroup = { | ||
id: { | ||
old: "", | ||
new: "e22e0eb9-625a-468b-b962-269b19473789", | ||
secret: false, | ||
}, | ||
members: { | ||
new: [], | ||
secret: false, | ||
}, | ||
name: { | ||
old: "", | ||
new: "another-test-group", | ||
secret: false, | ||
}, | ||
} | ||
const auditDiffForAddedGroupMember = { | ||
members: { | ||
old: [], | ||
new: [ | ||
{ | ||
group_id: "e22e0eb9-625a-468b-b962-269b19473789", | ||
user_id: "cea4c2b0-6373-4858-b26a-df3cbfce8845", | ||
}, | ||
], | ||
secret: false, | ||
}, | ||
} | ||
const auditDiffForRemovedGroupMember = { | ||
members: { | ||
old: [ | ||
{ | ||
group_id: "25793395-b093-4a3c-a473-9ecf9b243478", | ||
user_id: "84d1cd5a-17e1-4022-898c-52e64256e737", | ||
}, | ||
{ | ||
group_id: "25793395-b093-4a3c-a473-9ecf9b243478", | ||
user_id: "cea4c2b0-6373-4858-b26a-df3cbfce8845", | ||
}, | ||
], | ||
new: [ | ||
{ | ||
group_id: "25793395-b093-4a3c-a473-9ecf9b243478", | ||
user_id: "84d1cd5a-17e1-4022-898c-52e64256e737", | ||
}, | ||
], | ||
secret: false, | ||
}, | ||
} | ||
const AuditDiffForDeletedGroup = { | ||
id: { | ||
old: "25793395-b093-4a3c-a473-9ecf9b243478", | ||
new: "", | ||
secret: false, | ||
}, | ||
members: { | ||
old: [ | ||
{ | ||
group_id: "25793395-b093-4a3c-a473-9ecf9b243478", | ||
user_id: "84d1cd5a-17e1-4022-898c-52e64256e737", | ||
}, | ||
], | ||
secret: false, | ||
}, | ||
name: { | ||
old: "test-group", | ||
new: "", | ||
secret: false, | ||
}, | ||
} | ||
describe("determineAuditDiff", () => { | ||
it("auditDiffForNewGroup", () => { | ||
// there should be no change as members are not added when a group is created | ||
expect(determineGroupDiff(auditDiffForNewGroup)).toEqual( | ||
auditDiffForNewGroup, | ||
) | ||
}) | ||
it("auditDiffForAddedGroupMember", () => { | ||
const result = { | ||
members: { | ||
...auditDiffForAddedGroupMember.members, | ||
new: ["cea4c2b0-6373-4858-b26a-df3cbfce8845"], | ||
}, | ||
} | ||
expect(determineGroupDiff(auditDiffForAddedGroupMember)).toEqual(result) | ||
}) | ||
it("auditDiffForRemovedGroupMember", () => { | ||
const result = { | ||
members: { | ||
...auditDiffForRemovedGroupMember.members, | ||
old: [ | ||
"84d1cd5a-17e1-4022-898c-52e64256e737", | ||
"cea4c2b0-6373-4858-b26a-df3cbfce8845", | ||
], | ||
new: ["84d1cd5a-17e1-4022-898c-52e64256e737"], | ||
}, | ||
} | ||
expect(determineGroupDiff(auditDiffForRemovedGroupMember)).toEqual(result) | ||
}) | ||
it("AuditDiffForDeletedGroup", () => { | ||
const result = { | ||
...AuditDiffForDeletedGroup, | ||
members: { | ||
...AuditDiffForDeletedGroup.members, | ||
old: ["84d1cd5a-17e1-4022-898c-52e64256e737"], | ||
}, | ||
} | ||
expect(determineGroupDiff(AuditDiffForDeletedGroup)).toEqual(result) | ||
}) | ||
}) |
Uh oh!
There was an error while loading.Please reload this page.