From c59b73cc1b4df6c2c8d4df298a4e70825d39d1c8 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 13 Oct 2022 01:37:50 +0000 Subject: [PATCH 01/14] feat: add auditing for groups --- coderd/audit/diff.go | 3 +- coderd/audit/request.go | 6 ++ coderd/database/dump.sql | 3 +- .../000059_resource_type_group.down.sql | 2 + .../000059_resource_type_group.up.sql | 5 + coderd/database/models.go | 1 + enterprise/audit/table.go | 5 + enterprise/coderd/groups.go | 41 ++++++-- enterprise/coderd/groups_test.go | 95 +++++++++++++++++++ 9 files changed, 153 insertions(+), 8 deletions(-) create mode 100644 coderd/database/migrations/000059_resource_type_group.down.sql create mode 100644 coderd/database/migrations/000059_resource_type_group.up.sql diff --git a/coderd/audit/diff.go b/coderd/audit/diff.go index 8d0b5494568f8..42a67e92a8559 100644 --- 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 index c23ad2b1f7339..c658a09038569 100644 --- 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 index eb16074e90525..811b41f260733 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -81,7 +81,8 @@ CREATE TYPE resource_type AS ENUM ( 'user', 'workspace', 'git_ssh_key', - 'api_key' + 'api_key', + 'group' ); CREATE TYPE user_status AS ENUM ( diff --git a/coderd/database/migrations/000059_resource_type_group.down.sql b/coderd/database/migrations/000059_resource_type_group.down.sql new file mode 100644 index 0000000000000..0d2e97aef130e --- /dev/null +++ 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 new file mode 100644 index 0000000000000..ffd5ee9d2ea66 --- /dev/null +++ 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 index f669b5e618138..4bae9f76b8b4c 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -281,6 +281,7 @@ const ( ResourceTypeWorkspace ResourceType = "workspace" ResourceTypeGitSshKey ResourceType = "git_ssh_key" ResourceTypeApiKey ResourceType = "api_key" + ResourceTypeGroup ResourceType = "group" ) func (e *ResourceType) Scan(src interface{}) error { diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 0a0ebeca7304f..6d702f6aa569e 100644 --- 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 index 4c81c4a5efaf9..f27ac5d9ac33b 100644 --- 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 index 2661da6bcc29f..9cdca99d99dcf 100644 --- 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() From 1d71c29548e86c81a8969e03c60d5c81b77530b3 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 13 Oct 2022 02:03:23 +0000 Subject: [PATCH 02/14] fix auditing --- coderd/audit/audit.go | 2 ++ coderd/audit/request.go | 1 + coderd/coderd.go | 4 ++++ enterprise/coderd/coderd.go | 8 +------- enterprise/coderd/groups.go | 11 ++++++----- enterprise/coderd/groups_test.go | 9 +++++++++ 6 files changed, 23 insertions(+), 12 deletions(-) diff --git a/coderd/audit/audit.go b/coderd/audit/audit.go index 92f447130503c..9ca3b85fa65bd 100644 --- a/coderd/audit/audit.go +++ b/coderd/audit/audit.go @@ -2,6 +2,7 @@ package audit import ( "context" + "fmt" "github.com/coder/coder/coderd/database" ) @@ -33,6 +34,7 @@ type MockAuditor struct { func (a *MockAuditor) Export(_ context.Context, alog database.AuditLog) error { a.AuditLogs = append(a.AuditLogs, alog) + fmt.Println("WE APPENDED len: ", len(a.AuditLogs)) return nil } diff --git a/coderd/audit/request.go b/coderd/audit/request.go index c658a09038569..016c06c952562 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -113,6 +113,7 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request // If no resources were provided, there's nothing we can audit. if ResourceID(req.Old) == uuid.Nil && ResourceID(req.New) == uuid.Nil { + fmt.Println("FUCK") return } diff --git a/coderd/coderd.go b/coderd/coderd.go index 29439a2001a99..674a07296cfa0 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -2,6 +2,7 @@ package coderd import ( "crypto/x509" + "fmt" "io" "net/http" "net/url" @@ -128,6 +129,7 @@ func New(options *Options) *API { options.WorkspaceQuotaEnforcer = workspacequota.NewNop() } + fmt.Printf("options.Auditor: %T\n", options.Auditor) siteCacheDir := options.CacheDir if siteCacheDir != "" { siteCacheDir = filepath.Join(siteCacheDir, "site") @@ -165,6 +167,8 @@ func New(options *Options) *API { OIDC: options.OIDCConfig, } + fmt.Printf("load it: %T\n", *api.Auditor.Load()) + apiKeyMiddleware := httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ DB: options.Database, OAuth2Configs: oauthConfigs, diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 488d5dc033956..5c1e3809b0ea6 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -20,8 +20,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" ) @@ -178,11 +176,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) } diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index f27ac5d9ac33b..0e52f0a26a5b5 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -21,8 +21,9 @@ func (api *API) postGroupByOrganization(rw http.ResponseWriter, r *http.Request) var ( ctx = r.Context() org = httpmw.OrganizationParam(r) + auditor = api.AGPL.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.Group](rw, &audit.RequestParams{ - Audit: api.Auditor, + Audit: *auditor, Log: api.Logger, Request: r, Action: database.AuditActionCreate, @@ -71,9 +72,9 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { var ( ctx = r.Context() group = httpmw.GroupParam(r) - auditor = api.Auditor + auditor = api.AGPL.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.Group](rw, &audit.RequestParams{ - Audit: auditor, + Audit: *auditor, Log: api.Logger, Request: r, Action: database.AuditActionWrite, @@ -202,9 +203,9 @@ func (api *API) deleteGroup(rw http.ResponseWriter, r *http.Request) { var ( ctx = r.Context() group = httpmw.GroupParam(r) - auditor = api.Auditor + auditor = api.AGPL.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.Group](rw, &audit.RequestParams{ - Audit: auditor, + Audit: *auditor, Log: api.Logger, Request: r, Action: database.AuditActionDelete, diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go index 9cdca99d99dcf..c56c78eb73d83 100644 --- a/enterprise/coderd/groups_test.go +++ b/enterprise/coderd/groups_test.go @@ -42,6 +42,7 @@ func TestCreateGroup(t *testing.T) { auditor := audit.NewMock() client := coderdenttest.New(t, &coderdenttest.Options{ + AuditLogging: true, Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, Auditor: auditor, @@ -51,7 +52,9 @@ func TestCreateGroup(t *testing.T) { _ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ TemplateRBACEnabled: true, + AuditLog: true, }) + ctx, _ := testutil.Context(t) numLogs := len(auditor.AuditLogs) @@ -200,16 +203,20 @@ func TestPatchGroup(t *testing.T) { 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{ TemplateRBACEnabled: true, + AuditLog: true, }) + ctx, _ := testutil.Context(t) group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ @@ -553,6 +560,7 @@ func TestDeleteGroup(t *testing.T) { auditor := audit.NewMock() client := coderdenttest.New(t, &coderdenttest.Options{ + AuditLogging: true, Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, Auditor: auditor, @@ -562,6 +570,7 @@ func TestDeleteGroup(t *testing.T) { _ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ TemplateRBACEnabled: true, + AuditLog: true, }) ctx, _ := testutil.Context(t) From c3c17268d825a70652a96eda1319942ac6f4019a Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 13 Oct 2022 20:35:42 +0000 Subject: [PATCH 03/14] wip auditing --- coderd/audit/audit.go | 2 -- coderd/coderd.go | 4 ---- enterprise/cli/server.go | 3 +++ 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/coderd/audit/audit.go b/coderd/audit/audit.go index 9ca3b85fa65bd..92f447130503c 100644 --- a/coderd/audit/audit.go +++ b/coderd/audit/audit.go @@ -2,7 +2,6 @@ package audit import ( "context" - "fmt" "github.com/coder/coder/coderd/database" ) @@ -34,7 +33,6 @@ type MockAuditor struct { func (a *MockAuditor) Export(_ context.Context, alog database.AuditLog) error { a.AuditLogs = append(a.AuditLogs, alog) - fmt.Println("WE APPENDED len: ", len(a.AuditLogs)) return nil } diff --git a/coderd/coderd.go b/coderd/coderd.go index 674a07296cfa0..29439a2001a99 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -2,7 +2,6 @@ package coderd import ( "crypto/x509" - "fmt" "io" "net/http" "net/url" @@ -129,7 +128,6 @@ func New(options *Options) *API { options.WorkspaceQuotaEnforcer = workspacequota.NewNop() } - fmt.Printf("options.Auditor: %T\n", options.Auditor) siteCacheDir := options.CacheDir if siteCacheDir != "" { siteCacheDir = filepath.Join(siteCacheDir, "site") @@ -167,8 +165,6 @@ func New(options *Options) *API { OIDC: options.OIDCConfig, } - fmt.Printf("load it: %T\n", *api.Auditor.Load()) - apiKeyMiddleware := httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{ DB: options.Database, OAuth2Configs: oauthConfigs, diff --git a/enterprise/cli/server.go b/enterprise/cli/server.go index 62af6f2888373..b973ddce11146 100644 --- a/enterprise/cli/server.go +++ b/enterprise/cli/server.go @@ -6,6 +6,8 @@ import ( "github.com/spf13/cobra" "github.com/coder/coder/cli/deployment" + "github.com/coder/coder/enterprise/audit" + "github.com/coder/coder/enterprise/audit/backends" "github.com/coder/coder/enterprise/coderd" agpl "github.com/coder/coder/cli" @@ -27,6 +29,7 @@ func server() *cobra.Command { if err != nil { return nil, err } + api.Auditor = audit.NewAuditor(audit.DefaultFilter, backends.NewPostgres(api.Database, true), backends.NewSlog(api.AGPL.Logger)) return api.AGPL, nil }) From 374521b2cdda64032bafb1b2b82cbe8a64d1ee49 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Fri, 14 Oct 2022 00:15:01 +0000 Subject: [PATCH 04/14] update enterprise server initialization --- enterprise/cli/server.go | 11 ++++++++++- enterprise/coderd/coderd.go | 3 ++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/enterprise/cli/server.go b/enterprise/cli/server.go index b973ddce11146..5091528c766f0 100644 --- a/enterprise/cli/server.go +++ b/enterprise/cli/server.go @@ -17,6 +17,15 @@ import ( func server() *cobra.Command { dflags := deployment.Flags() cmd := agpl.Server(dflags, func(ctx context.Context, options *agplcoderd.Options) (*agplcoderd.API, error) { + if dflags.AuditLogging.Value { + options.Auditor = audit.NewAuditor(audit.DefaultFilter, + backends.NewPostgres(options.Database, true), + backends.NewSlog(options.Logger), + ) + } + + options.WorkspaceQuotaEnforcer = coderd.NewEnforcer(dflags.UserWorkspaceQuota.Value) + o := &coderd.Options{ AuditLogging: dflags.AuditLogging.Value, BrowserOnly: dflags.BrowserOnly.Value, @@ -25,11 +34,11 @@ func server() *cobra.Command { RBACEnabled: true, Options: options, } + api, err := coderd.New(ctx, o) if err != nil { return nil, err } - api.Auditor = audit.NewAuditor(audit.DefaultFilter, backends.NewPostgres(api.Database, true), backends.NewSlog(api.AGPL.Logger)) return api.AGPL, nil }) diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 5c1e3809b0ea6..c0df7653fe758 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -39,6 +39,7 @@ func New(ctx context.Context, options *Options) (*API, error) { Options: options, cancelEntitlementsLoop: cancelFunc, } + oauthConfigs := &httpmw.OAuth2Configs{ Github: options.GithubOAuth2Config, OIDC: options.OIDCConfig, @@ -192,7 +193,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { if changed, enabled := featureChanged(codersdk.FeatureWorkspaceQuota); changed { enforcer := workspacequota.NewNop() if enabled { - enforcer = NewEnforcer(api.Options.UserWorkspaceQuota) + enforcer = api.WorkspaceQuotaEnforcer } api.AGPL.WorkspaceQuotaEnforcer.Store(&enforcer) } From ac72abf26623cd136c18855c2b0c6a3868471349 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Fri, 14 Oct 2022 01:21:56 +0000 Subject: [PATCH 05/14] fix some stuff --- coderd/audit/request.go | 1 - codersdk/audit.go | 3 +++ enterprise/audit/table.go | 6 +++--- site/src/api/typesGenerated.ts | 1 + 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/coderd/audit/request.go b/coderd/audit/request.go index 016c06c952562..c658a09038569 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -113,7 +113,6 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request // If no resources were provided, there's nothing we can audit. if ResourceID(req.Old) == uuid.Nil && ResourceID(req.New) == uuid.Nil { - fmt.Println("FUCK") return } diff --git a/codersdk/audit.go b/codersdk/audit.go index 068d8d63c9b3d..1452af0c0f6b9 100644 --- a/codersdk/audit.go +++ b/codersdk/audit.go @@ -21,6 +21,7 @@ const ( ResourceTypeWorkspace ResourceType = "workspace" ResourceTypeGitSSHKey ResourceType = "git_ssh_key" ResourceTypeAPIKey ResourceType = "api_key" + ResourceTypeGroup ResourceType = "group" ) func (r ResourceType) FriendlyString() string { @@ -39,6 +40,8 @@ func (r ResourceType) FriendlyString() string { return "git ssh key" case ResourceTypeAPIKey: return "api key" + case ResourceTypeGroup: + return "group" default: return "unknown" } diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 6d702f6aa569e..e861c7f7afdc2 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -102,9 +102,9 @@ var AuditableResources = auditMap(map[any]map[string]Action{ "last_used_at": ActionIgnore, }, &database.Group{}: { - "id": ActionTrack, - "name": ActionTrack, - "avatar_url": ActionTrack, + "id": ActionTrack, + "name": ActionTrack, + "organization_id": ActionTrack, }, }) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 92088b978f77a..1bedc9be1a1e7 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -906,6 +906,7 @@ export type ProvisionerType = "echo" | "terraform" export type ResourceType = | "api_key" | "git_ssh_key" + | "group" | "organization" | "template" | "template_version" From 0144a10140c50abdab7a0163ea0d62500ce79e63 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 18 Oct 2022 21:40:52 +0000 Subject: [PATCH 06/14] go.mod --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 8fec641bc9ca1..758f1d6a4fff9 100644 --- a/go.mod +++ b/go.mod @@ -194,7 +194,7 @@ require ( github.com/fxamacker/cbor/v2 v2.4.0 // indirect github.com/ghodss/yaml v1.0.0 // indirect github.com/gin-gonic/gin v1.7.0 // indirect - github.com/go-logr/logr v1.2.3 // indirect + github.com/go-logr/logr v1.2.3 github.com/go-logr/stdr v1.2.2 // indirect github.com/go-ole/go-ole v1.2.6 // indirect github.com/go-playground/locales v0.14.0 // indirect From 6f4dcf7e549e81ef6dfbcfa4387361bb3f78018b Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 18 Oct 2022 22:05:50 +0000 Subject: [PATCH 07/14] update migration file --- ...ce_type_group.down.sql => 000063_resource_type_group.down.sql} | 0 ...source_type_group.up.sql => 000063_resource_type_group.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000059_resource_type_group.down.sql => 000063_resource_type_group.down.sql} (100%) rename coderd/database/migrations/{000059_resource_type_group.up.sql => 000063_resource_type_group.up.sql} (100%) diff --git a/coderd/database/migrations/000059_resource_type_group.down.sql b/coderd/database/migrations/000063_resource_type_group.down.sql similarity index 100% rename from coderd/database/migrations/000059_resource_type_group.down.sql rename to coderd/database/migrations/000063_resource_type_group.down.sql diff --git a/coderd/database/migrations/000059_resource_type_group.up.sql b/coderd/database/migrations/000063_resource_type_group.up.sql similarity index 100% rename from coderd/database/migrations/000059_resource_type_group.up.sql rename to coderd/database/migrations/000063_resource_type_group.up.sql From 3365beaf0689b7dc9391d1ab3b4547dff797d1e8 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 18 Oct 2022 23:05:05 +0000 Subject: [PATCH 08/14] cleanup template acl implementation --- coderd/database/databasefake/databasefake.go | 63 ++++++---------- coderd/database/drivers.go | 19 +++++ coderd/database/modelmethods.go | 58 +-------------- coderd/database/modelqueries.go | 47 ------------ coderd/database/models.go | 4 +- coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 75 ++++++++++++++++---- coderd/database/queries/templates.sql | 17 ++++- coderd/database/sqlc.yaml | 10 ++- coderd/templates.go | 22 +++--- enterprise/coderd/templates.go | 35 +++++---- 11 files changed, 159 insertions(+), 192 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 6f809ad117226..ad839dd08ed2a 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1457,34 +1457,6 @@ func (q *fakeQuerier) GetTemplates(_ context.Context) ([]database.Template, erro return templates, nil } -func (q *fakeQuerier) UpdateTemplateUserACLByID(_ context.Context, id uuid.UUID, acl database.TemplateACL) error { - q.mutex.RLock() - defer q.mutex.RUnlock() - - for i, t := range q.templates { - if t.ID == id { - t = t.SetUserACL(acl) - q.templates[i] = t - return nil - } - } - return sql.ErrNoRows -} - -func (q *fakeQuerier) UpdateTemplateGroupACLByID(_ context.Context, id uuid.UUID, acl database.TemplateACL) error { - q.mutex.RLock() - defer q.mutex.RUnlock() - - for i, t := range q.templates { - if t.ID == id { - t = t.SetGroupACL(acl) - q.templates[i] = t - return nil - } - } - return sql.ErrNoRows -} - func (q *fakeQuerier) GetTemplateUserRoles(_ context.Context, id uuid.UUID) ([]database.TemplateUser, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -1501,10 +1473,8 @@ func (q *fakeQuerier) GetTemplateUserRoles(_ context.Context, id uuid.UUID) ([]d return nil, sql.ErrNoRows } - acl := template.UserACL() - - users := make([]database.TemplateUser, 0, len(acl)) - for k, v := range acl { + users := make([]database.TemplateUser, 0, len(template.UserACL)) + for k, v := range template.UserACL { user, err := q.GetUserByID(context.Background(), uuid.MustParse(k)) if err != nil && xerrors.Is(err, sql.ErrNoRows) { return nil, xerrors.Errorf("get user by ID: %w", err) @@ -1544,10 +1514,8 @@ func (q *fakeQuerier) GetTemplateGroupRoles(_ context.Context, id uuid.UUID) ([] return nil, sql.ErrNoRows } - acl := template.GroupACL() - - groups := make([]database.TemplateGroup, 0, len(acl)) - for k, v := range acl { + groups := make([]database.TemplateGroup, 0, len(template.GroupACL)) + for k, v := range template.GroupACL { group, err := q.GetGroupByID(context.Background(), uuid.MustParse(k)) if err != nil && !xerrors.Is(err, sql.ErrNoRows) { return nil, xerrors.Errorf("get group by ID: %w", err) @@ -2047,11 +2015,9 @@ func (q *fakeQuerier) InsertTemplate(_ context.Context, arg database.InsertTempl MaxTtl: arg.MaxTtl, MinAutostartInterval: arg.MinAutostartInterval, CreatedBy: arg.CreatedBy, + UserACL: arg.UserACL, + GroupACL: arg.GroupACL, } - template = template.SetUserACL(database.TemplateACL{}) - template = template.SetGroupACL(database.TemplateACL{ - arg.OrganizationID.String(): []rbac.Action{rbac.ActionRead}, - }) q.templates = append(q.templates, template) return template, nil } @@ -2470,6 +2436,23 @@ func (q *fakeQuerier) UpdateTemplateDeletedByID(_ context.Context, arg database. return sql.ErrNoRows } +func (q *fakeQuerier) UpdateTemplateACLByID(_ context.Context, arg database.UpdateTemplateACLByIDParams) (database.Template, error) { + q.mutex.Lock() + defer q.mutex.Unlock() + + for i, template := range q.templates { + if template.ID == arg.ID { + template.GroupACL = arg.GroupACL + template.UserACL = arg.UserACL + + q.templates[i] = template + return template, nil + } + } + + return database.Template{}, sql.ErrNoRows +} + func (q *fakeQuerier) UpdateTemplateVersionByID(_ context.Context, arg database.UpdateTemplateVersionByIDParams) error { q.mutex.Lock() defer q.mutex.Unlock() diff --git a/coderd/database/drivers.go b/coderd/database/drivers.go index b0bf0f79612c8..1ab631077a6d3 100644 --- a/coderd/database/drivers.go +++ b/coderd/database/drivers.go @@ -24,3 +24,22 @@ func (a *Actions) Scan(src interface{}) error { func (a *Actions) Value() (driver.Value, error) { return json.Marshal(a) } + +// TemplateACL is a map of user_ids to permissions. +type TemplateACL map[string][]rbac.Action + +func (t *TemplateACL) Scan(src interface{}) error { + switch v := src.(type) { + case string: + return json.Unmarshal([]byte(v), &t) + case []byte, json.RawMessage: + //nolint + return json.Unmarshal(v.([]byte), &t) + } + + return xerrors.Errorf("unexpected type %T", src) +} + +func (t TemplateACL) Value() (driver.Value, error) { + return json.Marshal(t) +} diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index f12d8b7f2dceb..523d6e0afea25 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -1,65 +1,11 @@ package database import ( - "encoding/json" - "fmt" - "github.com/coder/coder/coderd/rbac" ) const AllUsersGroup = "Everyone" -// TemplateACL is a map of user_ids to permissions. -type TemplateACL map[string][]rbac.Action - -func (t Template) UserACL() TemplateACL { - var acl TemplateACL - if len(t.userACL) == 0 { - return acl - } - - err := json.Unmarshal(t.userACL, &acl) - if err != nil { - panic(fmt.Sprintf("failed to unmarshal template.userACL: %v", err.Error())) - } - - return acl -} - -func (t Template) GroupACL() TemplateACL { - var acl TemplateACL - if len(t.groupACL) == 0 { - return acl - } - - err := json.Unmarshal(t.groupACL, &acl) - if err != nil { - panic(fmt.Sprintf("failed to unmarshal template.userACL: %v", err.Error())) - } - - return acl -} - -func (t Template) SetGroupACL(acl TemplateACL) Template { - raw, err := json.Marshal(acl) - if err != nil { - panic(fmt.Sprintf("marshal user acl: %v", err)) - } - - t.groupACL = raw - return t -} - -func (t Template) SetUserACL(acl TemplateACL) Template { - raw, err := json.Marshal(acl) - if err != nil { - panic(fmt.Sprintf("marshal user acl: %v", err)) - } - - t.userACL = raw - return t -} - func (s APIKeyScope) ToRBAC() rbac.Scope { switch s { case APIKeyScopeAll: @@ -74,8 +20,8 @@ func (s APIKeyScope) ToRBAC() rbac.Scope { func (t Template) RBACObject() rbac.Object { obj := rbac.ResourceTemplate return obj.InOrg(t.OrganizationID). - WithACLUserList(t.UserACL()). - WithGroupACL(t.GroupACL()) + WithACLUserList(t.UserACL). + WithGroupACL(t.GroupACL) } func (TemplateVersion) RBACObject(template Template) rbac.Object { diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index 3383b6af96e39..8ccbe5f14f6ca 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -2,7 +2,6 @@ package database import ( "context" - "encoding/json" "fmt" "strings" @@ -23,8 +22,6 @@ type customQuerier interface { } type templateQuerier interface { - UpdateTemplateUserACLByID(ctx context.Context, id uuid.UUID, acl TemplateACL) error - UpdateTemplateGroupACLByID(ctx context.Context, id uuid.UUID, acl TemplateACL) error GetTemplateGroupRoles(ctx context.Context, id uuid.UUID) ([]TemplateGroup, error) GetTemplateUserRoles(ctx context.Context, id uuid.UUID) ([]TemplateUser, error) } @@ -34,28 +31,6 @@ type TemplateUser struct { Actions Actions `db:"actions"` } -func (q *sqlQuerier) UpdateTemplateUserACLByID(ctx context.Context, id uuid.UUID, acl TemplateACL) error { - raw, err := json.Marshal(acl) - if err != nil { - return xerrors.Errorf("marshal user acl: %w", err) - } - - const query = ` -UPDATE - templates -SET - user_acl = $2 -WHERE - id = $1` - - _, err = q.db.ExecContext(ctx, query, id.String(), raw) - if err != nil { - return xerrors.Errorf("update user acl: %w", err) - } - - return nil -} - func (q *sqlQuerier) GetTemplateUserRoles(ctx context.Context, id uuid.UUID) ([]TemplateUser, error) { const query = ` SELECT @@ -100,28 +75,6 @@ type TemplateGroup struct { Actions Actions `db:"actions"` } -func (q *sqlQuerier) UpdateTemplateGroupACLByID(ctx context.Context, id uuid.UUID, acl TemplateACL) error { - raw, err := json.Marshal(acl) - if err != nil { - return xerrors.Errorf("marshal user acl: %w", err) - } - - const query = ` -UPDATE - templates -SET - group_acl = $2 -WHERE - id = $1` - - _, err = q.db.ExecContext(ctx, query, id.String(), raw) - if err != nil { - return xerrors.Errorf("update user acl: %w", err) - } - - return nil -} - func (q *sqlQuerier) GetTemplateGroupRoles(ctx context.Context, id uuid.UUID) ([]TemplateGroup, error) { const query = ` SELECT diff --git a/coderd/database/models.go b/coderd/database/models.go index fdb5cc5229da5..dd0bce6064514 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -574,8 +574,8 @@ type Template struct { MinAutostartInterval int64 `db:"min_autostart_interval" json:"min_autostart_interval"` CreatedBy uuid.UUID `db:"created_by" json:"created_by"` Icon string `db:"icon" json:"icon"` - userACL json.RawMessage `db:"user_acl" json:"user_acl"` - groupACL json.RawMessage `db:"group_acl" json:"group_acl"` + UserACL TemplateACL `db:"user_acl" json:"user_acl"` + GroupACL TemplateACL `db:"group_acl" json:"group_acl"` } type TemplateVersion struct { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 393ab81fdd347..16cb6f3151653 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -162,6 +162,7 @@ type sqlcQuerier interface { UpdateProvisionerJobWithCancelByID(ctx context.Context, arg UpdateProvisionerJobWithCancelByIDParams) error UpdateProvisionerJobWithCompleteByID(ctx context.Context, arg UpdateProvisionerJobWithCompleteByIDParams) error UpdateReplica(ctx context.Context, arg UpdateReplicaParams) (Replica, error) + UpdateTemplateACLByID(ctx context.Context, arg UpdateTemplateACLByIDParams) (Template, error) UpdateTemplateActiveVersionByID(ctx context.Context, arg UpdateTemplateActiveVersionByIDParams) error UpdateTemplateDeletedByID(ctx context.Context, arg UpdateTemplateDeletedByIDParams) error UpdateTemplateMetaByID(ctx context.Context, arg UpdateTemplateMetaByIDParams) (Template, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index cb4b43591a41e..16a1109636a53 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2899,8 +2899,8 @@ func (q *sqlQuerier) GetTemplateByID(ctx context.Context, id uuid.UUID) (Templat &i.MinAutostartInterval, &i.CreatedBy, &i.Icon, - &i.userACL, - &i.groupACL, + &i.UserACL, + &i.GroupACL, ) return i, err } @@ -2941,8 +2941,8 @@ func (q *sqlQuerier) GetTemplateByOrganizationAndName(ctx context.Context, arg G &i.MinAutostartInterval, &i.CreatedBy, &i.Icon, - &i.userACL, - &i.groupACL, + &i.UserACL, + &i.GroupACL, ) return i, err } @@ -2975,8 +2975,8 @@ func (q *sqlQuerier) GetTemplates(ctx context.Context) ([]Template, error) { &i.MinAutostartInterval, &i.CreatedBy, &i.Icon, - &i.userACL, - &i.groupACL, + &i.UserACL, + &i.GroupACL, ); err != nil { return nil, err } @@ -3055,8 +3055,8 @@ func (q *sqlQuerier) GetTemplatesWithFilter(ctx context.Context, arg GetTemplate &i.MinAutostartInterval, &i.CreatedBy, &i.Icon, - &i.userACL, - &i.groupACL, + &i.UserACL, + &i.GroupACL, ); err != nil { return nil, err } @@ -3085,10 +3085,12 @@ INSERT INTO max_ttl, min_autostart_interval, created_by, - icon + icon, + user_acl, + group_acl ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by, icon, user_acl, group_acl + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by, icon, user_acl, group_acl ` type InsertTemplateParams struct { @@ -3104,6 +3106,8 @@ type InsertTemplateParams struct { MinAutostartInterval int64 `db:"min_autostart_interval" json:"min_autostart_interval"` CreatedBy uuid.UUID `db:"created_by" json:"created_by"` Icon string `db:"icon" json:"icon"` + UserACL TemplateACL `db:"user_acl" json:"user_acl"` + GroupACL TemplateACL `db:"group_acl" json:"group_acl"` } func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParams) (Template, error) { @@ -3120,6 +3124,8 @@ func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParam arg.MinAutostartInterval, arg.CreatedBy, arg.Icon, + arg.UserACL, + arg.GroupACL, ) var i Template err := row.Scan( @@ -3136,8 +3142,49 @@ func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParam &i.MinAutostartInterval, &i.CreatedBy, &i.Icon, - &i.userACL, - &i.groupACL, + &i.UserACL, + &i.GroupACL, + ) + return i, err +} + +const updateTemplateACLByID = `-- name: UpdateTemplateACLByID :one +UPDATE + templates +SET + group_acl = $1, + user_acl = $2 +WHERE + id = $3 +RETURNING + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by, icon, user_acl, group_acl +` + +type UpdateTemplateACLByIDParams struct { + GroupACL TemplateACL `db:"group_acl" json:"group_acl"` + UserACL TemplateACL `db:"user_acl" json:"user_acl"` + ID uuid.UUID `db:"id" json:"id"` +} + +func (q *sqlQuerier) UpdateTemplateACLByID(ctx context.Context, arg UpdateTemplateACLByIDParams) (Template, error) { + row := q.db.QueryRowContext(ctx, updateTemplateACLByID, arg.GroupACL, arg.UserACL, arg.ID) + var i Template + err := row.Scan( + &i.ID, + &i.CreatedAt, + &i.UpdatedAt, + &i.OrganizationID, + &i.Deleted, + &i.Name, + &i.Provisioner, + &i.ActiveVersionID, + &i.Description, + &i.MaxTtl, + &i.MinAutostartInterval, + &i.CreatedBy, + &i.Icon, + &i.UserACL, + &i.GroupACL, ) return i, err } @@ -3235,8 +3282,8 @@ func (q *sqlQuerier) UpdateTemplateMetaByID(ctx context.Context, arg UpdateTempl &i.MinAutostartInterval, &i.CreatedBy, &i.Icon, - &i.userACL, - &i.groupACL, + &i.UserACL, + &i.GroupACL, ) return i, err } diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql index 06570fffb0998..196355ad72404 100644 --- a/coderd/database/queries/templates.sql +++ b/coderd/database/queries/templates.sql @@ -68,10 +68,12 @@ INSERT INTO max_ttl, min_autostart_interval, created_by, - icon + icon, + user_acl, + group_acl ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) RETURNING *; + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14) RETURNING *; -- name: UpdateTemplateActiveVersionByID :exec UPDATE @@ -106,6 +108,17 @@ WHERE RETURNING *; +-- name: UpdateTemplateACLByID :one +UPDATE + templates +SET + group_acl = $1, + user_acl = $2 +WHERE + id = $3 +RETURNING + *; + -- name: GetTemplateAverageBuildTime :one WITH build_times AS ( SELECT diff --git a/coderd/database/sqlc.yaml b/coderd/database/sqlc.yaml index 24df14057fcc9..3a6d6e55b6853 100644 --- a/coderd/database/sqlc.yaml +++ b/coderd/database/sqlc.yaml @@ -19,6 +19,12 @@ packages: overrides: - column: "users.rbac_roles" go_type: "github.com/lib/pq.StringArray" + - column: "templates.user_acl" + go_type: + type: "TemplateACL" + - column: "templates.group_acl" + go_type: + type: "TemplateACL" rename: api_key: APIKey @@ -39,5 +45,5 @@ rename: ip_addresses: IPAddresses ids: IDs jwt: JWT - user_acl: userACL - group_acl: groupACL + user_acl: UserACL + group_acl: GroupACL diff --git a/coderd/templates.go b/coderd/templates.go index 7b08303dea096..a1108dfc43082 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -257,6 +257,10 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque MaxTtl: int64(maxTTL), MinAutostartInterval: int64(minAutostartInterval), CreatedBy: apiKey.UserID, + UserACL: database.TemplateACL{}, + GroupACL: database.TemplateACL{ + organization.ID.String(): []rbac.Action{rbac.ActionRead}, + }, }) if err != nil { return xerrors.Errorf("insert template: %s", err) @@ -299,13 +303,6 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque } } - err = tx.UpdateTemplateGroupACLByID(ctx, dbTemplate.ID, database.TemplateACL{ - dbTemplate.OrganizationID.String(): []rbac.Action{rbac.ActionRead}, - }) - if err != nil { - return xerrors.Errorf("update template group acl: %w", err) - } - createdByNameMap, err := getCreatedByNamesByTemplateIDs(ctx, tx, []database.Template{dbTemplate}) if err != nil { return xerrors.Errorf("get creator name: %w", err) @@ -683,6 +680,10 @@ func (api *API) autoImportTemplate(ctx context.Context, opts autoImportTemplateO MaxTtl: int64(maxTTLDefault), MinAutostartInterval: int64(minAutostartIntervalDefault), CreatedBy: opts.userID, + UserACL: database.TemplateACL{}, + GroupACL: database.TemplateACL{ + opts.orgID.String(): []rbac.Action{rbac.ActionRead}, + }, }) if err != nil { return xerrors.Errorf("insert template: %w", err) @@ -718,13 +719,6 @@ func (api *API) autoImportTemplate(ctx context.Context, opts autoImportTemplateO } } - err = tx.UpdateTemplateGroupACLByID(ctx, template.ID, database.TemplateACL{ - opts.orgID.String(): []rbac.Action{rbac.ActionRead}, - }) - if err != nil { - return xerrors.Errorf("update template group acl: %w", err) - } - return nil }) diff --git a/enterprise/coderd/templates.go b/enterprise/coderd/templates.go index 245f8d1e1dbd6..07139ec5a413f 100644 --- a/enterprise/coderd/templates.go +++ b/enterprise/coderd/templates.go @@ -31,6 +31,8 @@ func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) { return } + fmt.Printf("group_acl: %+v\n", template.GroupACL) + dbGroups, err := api.Database.GetTemplateGroupRoles(ctx, template.ID) if err != nil { httpapi.InternalServerError(rw, err) @@ -119,40 +121,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 }) From 315817679af6160a647759ae0bb85ca2371119e4 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 18 Oct 2022 23:24:58 +0000 Subject: [PATCH 09/14] add audit to template acl --- coderd/database/drivers.go | 2 +- enterprise/audit/table.go | 2 ++ enterprise/coderd/templates.go | 23 +++++++++++++--- enterprise/coderd/templates_test.go | 42 +++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 5 deletions(-) diff --git a/coderd/database/drivers.go b/coderd/database/drivers.go index 1ab631077a6d3..a1084d1229662 100644 --- a/coderd/database/drivers.go +++ b/coderd/database/drivers.go @@ -25,7 +25,7 @@ func (a *Actions) Value() (driver.Value, error) { return json.Marshal(a) } -// TemplateACL is a map of user_ids to permissions. +// TemplateACL is a map of ids to permissions. type TemplateACL map[string][]rbac.Action func (t *TemplateACL) Scan(src interface{}) error { diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index e861c7f7afdc2..55e81ca8e8ae4 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -62,6 +62,8 @@ var AuditableResources = auditMap(map[any]map[string]Action{ "min_autostart_interval": ActionTrack, "created_by": ActionTrack, "is_private": ActionTrack, + "group_acl": ActionTrack, + "user_acl": ActionTrack, }, &database.TemplateVersion{}: { "id": ActionTrack, diff --git a/enterprise/coderd/templates.go b/enterprise/coderd/templates.go index 07139ec5a413f..7d2d7f7049e9f 100644 --- a/enterprise/coderd/templates.go +++ b/enterprise/coderd/templates.go @@ -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 @@ -92,9 +96,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. @@ -166,6 +179,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.", }) diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index 87aa5a4ca83d8..ee8003c5532a5 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -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() From bc5e95d9e719b504d1ea3300a60b04e65071a884 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Wed, 19 Oct 2022 00:21:21 +0000 Subject: [PATCH 10/14] support templateacl --- enterprise/audit/diff.go | 4 +++- enterprise/audit/table.go | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/enterprise/audit/diff.go b/enterprise/audit/diff.go index dac3ab437bdcf..05d46499b525a 100644 --- a/enterprise/audit/diff.go +++ b/enterprise/audit/diff.go @@ -8,6 +8,7 @@ import ( "github.com/google/uuid" "github.com/coder/coder/coderd/audit" + "github.com/coder/coder/coderd/database" ) func structName(t reflect.Type) string { @@ -137,7 +138,8 @@ func convertDiffType(left, right any) (newLeft, newRight any, changed bool) { } return leftInt64Ptr, rightInt64Ptr, true - + case database.TemplateACL: + return fmt.Sprintf("%+v", left), fmt.Sprintf("%+v", right), true default: return left, right, false } diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 55e81ca8e8ae4..54a21a986200a 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -107,6 +107,7 @@ var AuditableResources = auditMap(map[any]map[string]Action{ "id": ActionTrack, "name": ActionTrack, "organization_id": ActionTrack, + "avatar_url": ActionTrack, }, }) From da108382841f557f7351e175293678fbef0c3b7e Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Wed, 19 Oct 2022 00:34:46 +0000 Subject: [PATCH 11/14] reduce deltas --- enterprise/cli/server.go | 2 -- enterprise/coderd/coderd.go | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/enterprise/cli/server.go b/enterprise/cli/server.go index 1aa59213edd1b..9459cc6906a84 100644 --- a/enterprise/cli/server.go +++ b/enterprise/cli/server.go @@ -57,8 +57,6 @@ func server() *cobra.Command { ) } - options.WorkspaceQuotaEnforcer = coderd.NewEnforcer(dflags.UserWorkspaceQuota.Value) - o := &coderd.Options{ AuditLogging: dflags.AuditLogging.Value, BrowserOnly: dflags.BrowserOnly.Value, diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 7c929af548d8b..b9d095883824e 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -255,7 +255,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { if changed, enabled := featureChanged(codersdk.FeatureWorkspaceQuota); changed { enforcer := workspacequota.NewNop() if enabled { - enforcer = api.WorkspaceQuotaEnforcer + enforcer = NewEnforcer(api.Options.UserWorkspaceQuota) } api.AGPL.WorkspaceQuotaEnforcer.Store(&enforcer) } From 97867bce00388817320098f6cb2e7ad66470255c Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Wed, 19 Oct 2022 00:36:13 +0000 Subject: [PATCH 12/14] debug line --- enterprise/coderd/templates.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/enterprise/coderd/templates.go b/enterprise/coderd/templates.go index 7d2d7f7049e9f..9fbcb403735e8 100644 --- a/enterprise/coderd/templates.go +++ b/enterprise/coderd/templates.go @@ -35,8 +35,6 @@ func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) { return } - fmt.Printf("group_acl: %+v\n", template.GroupACL) - dbGroups, err := api.Database.GetTemplateGroupRoles(ctx, template.ID) if err != nil { httpapi.InternalServerError(rw, err) From 05861d5b63ace13e05d6add6d816d16bfc4d50e5 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Wed, 19 Oct 2022 00:37:58 +0000 Subject: [PATCH 13/14] fix tests --- enterprise/coderd/coderd_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/enterprise/coderd/coderd_test.go b/enterprise/coderd/coderd_test.go index 7b51845ff3986..a6c3c6d86973b 100644 --- a/enterprise/coderd/coderd_test.go +++ b/enterprise/coderd/coderd_test.go @@ -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{ From 94200c2bde624783f7cc47cb2fb00c6c9164a45e Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Wed, 19 Oct 2022 06:41:59 +0000 Subject: [PATCH 14/14] pr comments --- coderd/database/migrations/000063_resource_type_group.up.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/migrations/000063_resource_type_group.up.sql b/coderd/database/migrations/000063_resource_type_group.up.sql index ffd5ee9d2ea66..3234c61bb7ca1 100644 --- a/coderd/database/migrations/000063_resource_type_group.up.sql +++ b/coderd/database/migrations/000063_resource_type_group.up.sql @@ -1,5 +1,5 @@ BEGIN; -ALTER TYPE resource_type ADD VALUE 'group'; +ALTER TYPE resource_type ADD VALUE IF NOT EXISTS 'group'; COMMIT;