feat: add auditing for groups

coder · sreya · Oct 19, 2022 · Oct 13, 2022 · Oct 13, 2022 · Oct 13, 2022
commit c59b73cc1b4df6c2c8d4df298a4e70825d39d1c8
diff --git a/coderd/audit/diff.go b/coderd/audit/diff.go
@@ -15,7 +15,8 @@ type Auditable interface {
 		database.TemplateVersion |
 		database.User |
 		database.Workspace |
-		database.GitSSHKey
+		database.GitSSHKey |
+		database.Group
 }
 
 // Map is a map of changed fields in an audited resource. It maps field names to

diff --git a/coderd/audit/request.go b/coderd/audit/request.go
@@ -45,6 +45,8 @@ func ResourceTarget[T Auditable](tgt T) string {
 		return typed.Name
 	case database.GitSSHKey:
 		return typed.PublicKey
+	case database.Group:
+		return typed.Name
 	default:
 		panic(fmt.Sprintf("unknown resource %T", tgt))
 	}
@@ -64,6 +66,8 @@ func ResourceID[T Auditable](tgt T) uuid.UUID {
 		return typed.ID
 	case database.GitSSHKey:
 		return typed.UserID
+	case database.Group:
+		return typed.ID
 	default:
 		panic(fmt.Sprintf("unknown resource %T", tgt))
 	}
@@ -83,6 +87,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
 		return database.ResourceTypeWorkspace
 	case database.GitSSHKey:
 		return database.ResourceTypeGitSshKey
+	case database.Group:
+		return database.ResourceTypeGroup
 	default:
 		panic(fmt.Sprintf("unknown resource %T", tgt))
 	}

diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql
diff --git a/coderd/database/migrations/000059_resource_type_group.down.sql b/coderd/database/migrations/000059_resource_type_group.down.sql
@@ -0,0 +1,2 @@
+-- You cannot safely remove values from enums https://www.postgresql.org/docs/current/datatype-enum.html
+-- You cannot create a new type and do a rename because objects depend on this type now.
diff --git a/coderd/database/migrations/000059_resource_type_group.up.sql b/coderd/database/migrations/000059_resource_type_group.up.sql
@@ -0,0 +1,5 @@
+BEGIN;
+
+ALTER TYPE resource_type ADD VALUE 'group'; 
+
+COMMIT;
diff --git a/coderd/database/models.go b/coderd/database/models.go
diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go
@@ -101,6 +101,11 @@ var AuditableResources = auditMap(map[any]map[string]Action{
 		"ttl":                ActionTrack,
 		"last_used_at":       ActionIgnore,
 	},
+	&database.Group{}: {
+		"id":         ActionTrack,
+		"name":       ActionTrack,
+		"avatar_url": ActionTrack,
+	},
 })
 
 // auditMap converts a map of struct pointers to a map of struct names as

diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go
@@ -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,16 @@ 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)
+		aReq, commitAudit = audit.InitRequest[database.Group](rw, &audit.RequestParams{
+			Audit:   api.Auditor,
+			Log:     api.Logger,
+			Request: r,
+			Action:  database.AuditActionCreate,
+		})
 	)
+	defer commitAudit()
 
 	if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceGroup) {
 		http.NotFound(rw, r)
@@ -54,15 +62,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.Auditor
+		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)
@@ -175,14 +193,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.Auditor
+		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)

diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go
@@ -7,6 +7,7 @@ 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"
@@ -36,6 +37,34 @@ 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{
+			Options: &coderdtest.Options{
+				IncludeProvisionerDaemon: true,
+				Auditor:                  auditor,
+			},
+		})
+		user := coderdtest.CreateFirstUser(t, client)
+
+		_ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{
+			TemplateRBACEnabled: 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()
 
@@ -166,6 +195,40 @@ 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{
+			Options: &coderdtest.Options{
+				IncludeProvisionerDaemon: true,
+				Auditor:                  auditor,
+			},
+		})
+		user := coderdtest.CreateFirstUser(t, client)
+
+		_ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{
+			TemplateRBACEnabled: 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("UserNotExist", func(t *testing.T) {
 		t.Parallel()
 
@@ -485,6 +548,38 @@ 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{
+			Options: &coderdtest.Options{
+				IncludeProvisionerDaemon: true,
+				Auditor:                  auditor,
+			},
+		})
+		user := coderdtest.CreateFirstUser(t, client)
+
+		_ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{
+			TemplateRBACEnabled: 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()
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		-- You cannot safely remove values from enums https://www.postgresql.org/docs/current/datatype-enum.html
		-- You cannot create a new type and do a rename because objects depend on this type now.