opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat: add auditing for groups (#4527)

Browse Source 
- Clean up `database.TemplateACL` implementation.
This commit is contained in:
Jon Ayers
2022-10-19 02:00:45 -05:00
committed by GitHub
parent d4585fefb8
commit 0d1096da6c
27 changed files with 405 additions and 213 deletions
Download Patch File Download Diff File Expand all files Collapse all files
enterprise/coderd/coderd.go
+1 -7
View File
@@ -22,8 +22,6 @@ import (
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/coderd/workspacequota"
"github.com/coder/coder/codersdk"
"github.com/coder/coder/enterprise/audit"
"github.com/coder/coder/enterprise/audit/backends"
"github.com/coder/coder/enterprise/coderd/license"
"github.com/coder/coder/enterprise/derpmesh"
"github.com/coder/coder/enterprise/replicasync"
@@ -241,11 +239,7 @@ func (api *API) updateEntitlements(ctx context.Context) error {
if changed, enabled := featureChanged(codersdk.FeatureAuditLog); changed {
auditor := agplaudit.NewNop()
if enabled {
auditor = audit.NewAuditor(
audit.DefaultFilter,
backends.NewPostgres(api.Database, true),
backends.NewSlog(api.Logger),
)
auditor = api.AGPL.Options.Auditor
}
api.AGPL.Auditor.Store(&auditor)
}
enterprise/coderd/coderd_test.go
+3
View File
@@ -158,6 +158,9 @@ func TestAuditLogging(t *testing.T) {
t.Parallel()
client, _, api := coderdenttest.NewWithAPI(t, &coderdenttest.Options{
AuditLogging: true,
Options: &coderdtest.Options{
Auditor: audit.NewAuditor(audit.DefaultFilter),
},
})
coderdtest.CreateFirstUser(t, client)
coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{
enterprise/coderd/groups.go
+36 -6
View File
@@ -9,6 +9,7 @@ import (
"golang.org/x/xerrors"
"github.com/coder/coder/coderd"
"github.com/coder/coder/coderd/audit"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/httpapi"
"github.com/coder/coder/coderd/httpmw"
@@ -18,9 +19,17 @@ import (
func (api *API) postGroupByOrganization(rw http.ResponseWriter, r *http.Request) {
var (
ctx = r.Context()
org = httpmw.OrganizationParam(r)
ctx = r.Context()
org = httpmw.OrganizationParam(r)
auditor = api.AGPL.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.Group](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionCreate,
})
)
defer commitAudit()
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceGroup) {
http.NotFound(rw, r)
@@ -55,15 +64,25 @@ func (api *API) postGroupByOrganization(rw http.ResponseWriter, r *http.Request)
httpapi.InternalServerError(rw, err)
return
}
aReq.New = group
httpapi.Write(ctx, rw, http.StatusCreated, convertGroup(group, nil))
}
func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
var (
ctx = r.Context()
group = httpmw.GroupParam(r)
ctx = r.Context()
group = httpmw.GroupParam(r)
auditor = api.AGPL.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.Group](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionWrite,
})
)
defer commitAudit()
aReq.Old = group
if !api.Authorize(r, rbac.ActionUpdate, group) {
http.NotFound(rw, r)
@@ -195,14 +214,25 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
return
}
aReq.New = group
httpapi.Write(ctx, rw, http.StatusOK, convertGroup(group, members))
}
func (api *API) deleteGroup(rw http.ResponseWriter, r *http.Request) {
var (
ctx = r.Context()
group = httpmw.GroupParam(r)
ctx = r.Context()
group = httpmw.GroupParam(r)
auditor = api.AGPL.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.Group](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionDelete,
})
)
defer commitAudit()
aReq.Old = group
if !api.Authorize(r, rbac.ActionDelete, group) {
httpapi.ResourceNotFound(rw)
enterprise/coderd/groups_test.go
+103
View File
@@ -8,6 +8,7 @@ import (
"github.com/stretchr/testify/require"
"k8s.io/utils/pointer"
"github.com/coder/coder/coderd/audit"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/codersdk"
@@ -39,6 +40,37 @@ func TestCreateGroup(t *testing.T) {
require.NotEqual(t, uuid.Nil.String(), group.ID.String())
})
t.Run("Audit", func(t *testing.T) {
t.Parallel()
auditor := audit.NewMock()
client := coderdenttest.New(t, &coderdenttest.Options{
AuditLogging: true,
Options: &coderdtest.Options{
IncludeProvisionerDaemon: true,
Auditor: auditor,
},
})
user := coderdtest.CreateFirstUser(t, client)
_ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{
TemplateRBAC: true,
AuditLog: true,
})
ctx, _ := testutil.Context(t)
numLogs := len(auditor.AuditLogs)
group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{
Name: "hi",
})
require.NoError(t, err)
numLogs++
require.Len(t, auditor.AuditLogs, numLogs)
require.Equal(t, database.AuditActionCreate, auditor.AuditLogs[numLogs-1].Action)
require.Equal(t, group.ID, auditor.AuditLogs[numLogs-1].ResourceID)
})
t.Run("Conflict", func(t *testing.T) {
t.Parallel()
@@ -197,6 +229,43 @@ func TestPatchGroup(t *testing.T) {
require.Contains(t, group.Members, user4)
})
t.Run("Audit", func(t *testing.T) {
t.Parallel()
auditor := audit.NewMock()
client := coderdenttest.New(t, &coderdenttest.Options{
AuditLogging: true,
Options: &coderdtest.Options{
IncludeProvisionerDaemon: true,
Auditor: auditor,
},
})
user := coderdtest.CreateFirstUser(t, client)
_ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{
TemplateRBAC: true,
AuditLog: true,
})
ctx, _ := testutil.Context(t)
group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{
Name: "hi",
})
require.NoError(t, err)
numLogs := len(auditor.AuditLogs)
group, err = client.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{
Name: "bye",
})
require.NoError(t, err)
numLogs++
require.Len(t, auditor.AuditLogs, numLogs)
require.Equal(t, database.AuditActionWrite, auditor.AuditLogs[numLogs-1].Action)
require.Equal(t, group.ID, auditor.AuditLogs[numLogs-1].ResourceID)
})
t.Run("NameConflict", func(t *testing.T) {
t.Parallel()
@@ -547,6 +616,40 @@ func TestDeleteGroup(t *testing.T) {
require.Equal(t, http.StatusNotFound, cerr.StatusCode())
})
t.Run("Audit", func(t *testing.T) {
t.Parallel()
auditor := audit.NewMock()
client := coderdenttest.New(t, &coderdenttest.Options{
AuditLogging: true,
Options: &coderdtest.Options{
IncludeProvisionerDaemon: true,
Auditor: auditor,
},
})
user := coderdtest.CreateFirstUser(t, client)
_ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{
TemplateRBAC: true,
AuditLog: true,
})
ctx, _ := testutil.Context(t)
group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{
Name: "hi",
})
require.NoError(t, err)
numLogs := len(auditor.AuditLogs)
err = client.DeleteGroup(ctx, group.ID)
require.NoError(t, err)
numLogs++
require.Len(t, auditor.AuditLogs, numLogs)
require.Equal(t, database.AuditActionDelete, auditor.AuditLogs[numLogs-1].Action)
require.Equal(t, group.ID, auditor.AuditLogs[numLogs-1].ResourceID)
})
t.Run("allUsers", func(t *testing.T) {
t.Parallel()
enterprise/coderd/templates.go
+37 -19
View File
@@ -10,6 +10,7 @@ import (
"golang.org/x/xerrors"
"github.com/coder/coder/coderd"
"github.com/coder/coder/coderd/audit"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/httpapi"
"github.com/coder/coder/coderd/httpmw"
@@ -18,8 +19,11 @@ import (
)
func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
template := httpmw.TemplateParam(r)
var (
ctx = r.Context()
template = httpmw.TemplateParam(r)
)
if !api.Authorize(r, rbac.ActionRead, template) {
httpapi.ResourceNotFound(rw)
return
@@ -90,9 +94,18 @@ func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) {
func (api *API) patchTemplateACL(rw http.ResponseWriter, r *http.Request) {
var (
ctx = r.Context()
template = httpmw.TemplateParam(r)
ctx = r.Context()
template = httpmw.TemplateParam(r)
auditor = api.AGPL.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.Template](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionWrite,
})
)
defer commitAudit()
aReq.Old = template
// Only users who are able to create templates (aka template admins)
// are able to control permissions.
@@ -119,40 +132,43 @@ func (api *API) patchTemplateACL(rw http.ResponseWriter, r *http.Request) {
}
err := api.Database.InTx(func(tx database.Store) error {
var err error
template, err = tx.GetTemplateByID(ctx, template.ID)
if err != nil {
return xerrors.Errorf("get template by ID: %w", err)
}
if len(req.UserPerms) > 0 {
userACL := template.UserACL()
for id, role := range req.UserPerms {
// A user with an empty string implies
// deletion.
if role == "" {
delete(userACL, id)
delete(template.UserACL, id)
continue
}
userACL[id] = convertSDKTemplateRole(role)
}
err := tx.UpdateTemplateUserACLByID(r.Context(), template.ID, userACL)
if err != nil {
return xerrors.Errorf("update template user ACL: %w", err)
template.UserACL[id] = convertSDKTemplateRole(role)
}
}
if len(req.GroupPerms) > 0 {
groupACL := template.GroupACL()
for id, role := range req.GroupPerms {
// An id with an empty string implies
// deletion.
if role == "" {
delete(groupACL, id)
delete(template.GroupACL, id)
continue
}
groupACL[id] = convertSDKTemplateRole(role)
template.GroupACL[id] = convertSDKTemplateRole(role)
}
}
err := tx.UpdateTemplateGroupACLByID(ctx, template.ID, groupACL)
if err != nil {
return xerrors.Errorf("update template user ACL: %w", err)
}
template, err = tx.UpdateTemplateACLByID(ctx, database.UpdateTemplateACLByIDParams{
ID: template.ID,
UserACL: template.UserACL,
GroupACL: template.GroupACL,
})
if err != nil {
return xerrors.Errorf("update template ACL by ID: %w", err)
}
return nil
})
@@ -161,6 +177,8 @@ func (api *API) patchTemplateACL(rw http.ResponseWriter, r *http.Request) {
return
}
aReq.New = template
httpapi.Write(ctx, rw, http.StatusOK, codersdk.Response{
Message: "Successfully updated template ACL list.",
})
enterprise/coderd/templates_test.go
+42
View File
@@ -8,7 +8,9 @@ import (
"github.com/google/uuid"
"github.com/stretchr/testify/require"
"github.com/coder/coder/coderd/audit"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/codersdk"
"github.com/coder/coder/enterprise/coderd/coderdenttest"
"github.com/coder/coder/provisioner/echo"
@@ -355,6 +357,46 @@ func TestUpdateTemplateACL(t *testing.T) {
require.Contains(t, acl.Users, templateUser3)
})
t.Run("Audit", func(t *testing.T) {
t.Parallel()
auditor := audit.NewMock()
client := coderdenttest.New(t, &coderdenttest.Options{
AuditLogging: true,
Options: &coderdtest.Options{
IncludeProvisionerDaemon: true,
Auditor: auditor,
},
})
user := coderdtest.CreateFirstUser(t, client)
_ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{
TemplateRBAC: true,
AuditLog: true,
})
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
ctx, _ := testutil.Context(t)
numLogs := len(auditor.AuditLogs)
req := codersdk.UpdateTemplateACL{
GroupPerms: map[string]codersdk.TemplateRole{
user.OrganizationID.String(): codersdk.TemplateRoleDeleted,
},
}
err := client.UpdateTemplateACL(ctx, template.ID, req)
require.NoError(t, err)
numLogs++
require.Len(t, auditor.AuditLogs, numLogs)
require.Equal(t, database.AuditActionWrite, auditor.AuditLogs[numLogs-1].Action)
require.Equal(t, template.ID, auditor.AuditLogs[numLogs-1].ResourceID)
})
t.Run("DeleteUser", func(t *testing.T) {
t.Parallel()