selfhosted
/
coder
mirror of https://github.com/coder/coder.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

feat: Auditing group members as part of group resource (#5730) 

* added AuditableGroup type

* added json tags

* Anonymizing gGroup struct

* adding support on the FE for nested group diffs

* added type for GroupMember

* Update coderd/database/modelmethods.go

Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>

* Update coderd/database/modelmethods.go

Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>

* fetching group members in group.delete

* passing through right error

* broke out into util function and added tests

Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>
This commit is contained in:
Kira Pilot 2023-01-18 15:13:39 -05:00 committed by GitHub
parent 56b996532f
commit 6b68fbbf18
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 230 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
coderd/audit.go
View File

@ -464,6 +464,8 @@ func resourceTypeFromString(resourceTypeString string) string {
return resourceTypeString
case codersdk.ResourceTypeAPIKey:
return resourceTypeString
case codersdk.ResourceTypeGroup:
return resourceTypeString
}
return ""
}

4
coderd/audit/diff.go
View File

@ -16,8 +16,8 @@ type Auditable interface {
database.User |
database.Workspace |
database.GitSSHKey |
database.Group |
database.WorkspaceBuild
database.WorkspaceBuild |
database.AuditableGroup
}
// Map is a map of changed fields in an audited resource. It maps field names to

10
coderd/audit/request.go
View File

@ -64,8 +64,8 @@ func ResourceTarget[T Auditable](tgt T) string {
return ""
case database.GitSSHKey:
return typed.PublicKey
case database.Group:
return typed.Name
case database.AuditableGroup:
return typed.Group.Name
default:
panic(fmt.Sprintf("unknown resource %T", tgt))
}
@ -87,8 +87,8 @@ func ResourceID[T Auditable](tgt T) uuid.UUID {
return typed.ID
case database.GitSSHKey:
return typed.UserID
case database.Group:
return typed.ID
case database.AuditableGroup:
return typed.Group.ID
default:
panic(fmt.Sprintf("unknown resource %T", tgt))
}
@ -110,7 +110,7 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
return database.ResourceTypeWorkspaceBuild
case database.GitSSHKey:
return database.ResourceTypeGitSshKey
case database.Group:
case database.AuditableGroup:
return database.ResourceTypeGroup
default:
panic(fmt.Sprintf("unknown resource %T", tgt))

29
coderd/database/modelmethods.go
View File

@ -1,9 +1,38 @@
package database
import (
"sort"
"github.com/coder/coder/coderd/rbac"
)
type AuditableGroup struct {
Group
Members []GroupMember `json:"members"`
}
// Auditable returns an object that can be used in audit logs.
// Covers both group and group member changes.
func (g Group) Auditable(users []User) AuditableGroup {
members := make([]GroupMember, 0, len(users))
for _, u := range users {
members = append(members, GroupMember{
UserID: u.ID,
GroupID: g.ID,
})
}
// consistent ordering
sort.Slice(members, func(i, j int) bool {
return members[i].UserID.String() < members[j].UserID.String()
})
return AuditableGroup{
Group: g,
Members: members,
}
}
const AllUsersGroup = "Everyone"
func (s APIKeyScope) ToRBAC() rbac.Scope {

15
enterprise/audit/table.go
View File

@ -105,13 +105,6 @@ var AuditableResources = auditMap(map[any]map[string]Action{
"ttl": ActionTrack,
"last_used_at": ActionIgnore,
},
&database.Group{}: {
"id": ActionTrack,
"name": ActionTrack,
"organization_id": ActionIgnore, // Never changes.
"avatar_url": ActionTrack,
"quota_allowance": ActionTrack,
},
// 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,
},
})
// auditMap converts a map of struct pointers to a map of struct names as

34
enterprise/coderd/groups.go
View File

@ -32,7 +32,7 @@ func (api *API) postGroupByOrganization(rw http.ResponseWriter, r *http.Request)
ctx = r.Context()
org = httpmw.OrganizationParam(r)
auditor = api.AGPL.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.Group](rw, &audit.RequestParams{
aReq, commitAudit = audit.InitRequest[database.AuditableGroup](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
@ -75,7 +75,9 @@ func (api *API) postGroupByOrganization(rw http.ResponseWriter, r *http.Request)
httpapi.InternalServerError(rw, err)
return
}
aReq.New = group
var emptyUsers []database.User
aReq.New = group.Auditable(emptyUsers)
httpapi.Write(ctx, rw, http.StatusCreated, convertGroup(group, nil))
}
@ -93,7 +95,7 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
ctx = r.Context()
group = httpmw.GroupParam(r)
auditor = api.AGPL.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.Group](rw, &audit.RequestParams{
aReq, commitAudit = audit.InitRequest[database.AuditableGroup](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
@ -101,7 +103,14 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
})
)
defer commitAudit()
aReq.Old = group
currentMembers, currentMembersErr := api.Database.GetGroupMembers(ctx, group.ID)
if currentMembersErr != nil {
httpapi.InternalServerError(rw, currentMembersErr)
return
}
aReq.Old = group.Auditable(currentMembers)
if !api.Authorize(r, rbac.ActionUpdate, group) {
http.NotFound(rw, r)
@ -233,15 +242,15 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
return
}
members, err := api.Database.GetGroupMembers(ctx, group.ID)
patchedMembers, err := api.Database.GetGroupMembers(ctx, group.ID)
if err != nil {
httpapi.InternalServerError(rw, err)
return
}
aReq.New = group
aReq.New = group.Auditable(patchedMembers)
httpapi.Write(ctx, rw, http.StatusOK, convertGroup(group, members))
httpapi.Write(ctx, rw, http.StatusOK, convertGroup(group, patchedMembers))
}
// @Summary Delete group by name
@ -257,7 +266,7 @@ func (api *API) deleteGroup(rw http.ResponseWriter, r *http.Request) {
ctx = r.Context()
group = httpmw.GroupParam(r)
auditor = api.AGPL.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.Group](rw, &audit.RequestParams{
aReq, commitAudit = audit.InitRequest[database.AuditableGroup](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
@ -265,7 +274,14 @@ func (api *API) deleteGroup(rw http.ResponseWriter, r *http.Request) {
})
)
defer commitAudit()
aReq.Old = group
groupMembers, getMembersErr := api.Database.GetGroupMembers(ctx, group.ID)
if getMembersErr != nil {
httpapi.InternalServerError(rw, getMembersErr)
return
}
aReq.Old = group.Auditable(groupMembers)
if !api.Authorize(r, rbac.ActionDelete, group) {
httpapi.ResourceNotFound(rw)

5
site/src/components/AuditLogRow/AuditLogDiff.tsx
View File

@ -3,6 +3,7 @@ import { AuditLog } from "api/typesGenerated"
import { colors } from "theme/colors"
import { MONOSPACE_FONT_FAMILY } from "theme/constants"
import { combineClasses } from "util/combineClasses"
import { FC } from "react"
const getDiffValue = (value: unknown): string => {
if (typeof value === "string") {
@ -21,9 +22,7 @@ const getDiffValue = (value: unknown): string => {
return value.toString()
}
export const AuditLogDiff: React.FC<{ diff: AuditLog["diff"] }> = ({
diff,
}) => {
export const AuditLogDiff: FC<{ diff: AuditLog["diff"] }> = ({ diff }) => {
const styles = useStyles()
const diffEntries = Object.entries(diff)

10
site/src/components/AuditLogRow/AuditLogRow.tsx
View File

@ -16,6 +16,7 @@ import userAgentParser from "ua-parser-js"
import { AuditLogDiff } from "./AuditLogDiff"
import i18next from "i18next"
import { AuditLogDescription } from "./AuditLogDescription"
import { determineGroupDiff } from "./auditUtils"
const httpStatusColor = (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")
let auditDiff = auditLog.diff
// groups have nested diffs (group members)
if (auditLog.resource_type === "group") {
auditDiff = determineGroupDiff(auditLog.diff)
}
const toggle = () => {
if (shouldDisplayDiff) {
setIsDiffOpen((v) => !v)
@ -153,7 +161,7 @@ export const AuditLogRow: React.FC<AuditLogRowProps> = ({
{shouldDisplayDiff && (
<Collapse in={isDiffOpen}>
<AuditLogDiff diff={auditLog.diff} />
<AuditLogDiff diff={auditDiff} />
</Collapse>
)}
</TableCell>

122
site/src/components/AuditLogRow/auditUtils.test.ts Normal file
View File

@ -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)
})
})

26
site/src/components/AuditLogRow/auditUtils.ts Normal file
View File

@ -0,0 +1,26 @@
import { AuditDiff } from "api/typesGenerated"
interface GroupMember {
user_id: string
group_id: string
}
/**
*
* @param auditLogDiff
* @returns a diff with the 'members' key flattened to be an array of user_ids
*/
export const determineGroupDiff = (auditLogDiff: AuditDiff): AuditDiff => {
return {
...auditLogDiff,
members: {
old: auditLogDiff.members.old?.map(
(groupMember: GroupMember) => groupMember.user_id,
),
new: auditLogDiff.members.new?.map(
(groupMember: GroupMember) => groupMember.user_id,
),
secret: auditLogDiff.members.secret,
},
}
}