From 2799228e9e7fdd7200ee951bad3c95f365225c7f Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 4 Jun 2025 07:46:55 +0000 Subject: [PATCH 1/2] chore(coderd/database/dbauthz): update RBAC for InsertWorkspaceApp Instead of using `ResourceSystem` as the resource for `InsertWorkspaceApp`, we instead use the associated workspace (if it exists), with the action `ActionCreateAgent`. The decision to use this action is because everywhere where `InsertWorkspaceApp` is called is also where `InsertWorkspaceAgent` is called. I don't think adding an extra action makes sense, and adding an extra resource also doesn't make sense. --- coderd/database/dbauthz/dbauthz.go | 12 +++++++++++- coderd/database/dbauthz/dbauthz_test.go | 19 +++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index c64701fde482a..644a8fa853f8b 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -3851,9 +3851,19 @@ func (q *querier) InsertWorkspaceAgentStats(ctx context.Context, arg database.In } func (q *querier) InsertWorkspaceApp(ctx context.Context, arg database.InsertWorkspaceAppParams) (database.WorkspaceApp, error) { - if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil { + // NOTE(DanielleMaywood): + // It is possible for there to exist an agent without a workspace. + // This means that we want to allow execution to continue if + // there isn't a workspace found to allow this behavior to continue. + workspace, err := q.db.GetWorkspaceByAgentID(ctx, arg.AgentID) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return database.WorkspaceApp{}, err + } + + if err := q.authorizeContext(ctx, policy.ActionCreateAgent, workspace); err != nil { return database.WorkspaceApp{}, err } + return q.db.InsertWorkspaceApp(ctx, arg) } diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index db3c1d9f861ad..5e3edb61c77bb 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -4093,13 +4093,28 @@ func (s *MethodTestSuite) TestSystemFunctions() { }).Asserts(ws, policy.ActionCreateAgent) })) s.Run("InsertWorkspaceApp", s.Subtest(func(db database.Store, check *expects) { - dbtestutil.DisableForeignKeysAndTriggers(s.T(), db) + _ = dbgen.User(s.T(), db, database.User{}) + u := dbgen.User(s.T(), db, database.User{}) + o := dbgen.Organization(s.T(), db, database.Organization{}) + j := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{Type: database.ProvisionerJobTypeWorkspaceBuild}) + tpl := dbgen.Template(s.T(), db, database.Template{CreatedBy: u.ID, OrganizationID: o.ID}) + tv := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: tpl.ID, Valid: true}, + JobID: j.ID, + OrganizationID: o.ID, + CreatedBy: u.ID, + }) + ws := dbgen.Workspace(s.T(), db, database.WorkspaceTable{OwnerID: u.ID, TemplateID: tpl.ID, OrganizationID: o.ID}) + _ = dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: j.ID, TemplateVersionID: tv.ID}) + res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: j.ID}) + agent := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID}) check.Args(database.InsertWorkspaceAppParams{ ID: uuid.New(), + AgentID: agent.ID, Health: database.WorkspaceAppHealthDisabled, SharingLevel: database.AppSharingLevelOwner, OpenIn: database.WorkspaceAppOpenInSlimWindow, - }).Asserts(rbac.ResourceSystem, policy.ActionCreate) + }).Asserts(ws, policy.ActionCreateAgent) })) s.Run("InsertWorkspaceResourceMetadata", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertWorkspaceResourceMetadataParams{ From 381c5ca60bb05c2290434e45e522f1f5110bd35b Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 4 Jun 2025 11:06:53 +0000 Subject: [PATCH 2/2] chore: ActionCreateAgent -> ActionUpdate --- coderd/database/dbauthz/dbauthz.go | 2 +- coderd/database/dbauthz/dbauthz_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 644a8fa853f8b..36e7315b6d6c9 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -3860,7 +3860,7 @@ func (q *querier) InsertWorkspaceApp(ctx context.Context, arg database.InsertWor return database.WorkspaceApp{}, err } - if err := q.authorizeContext(ctx, policy.ActionCreateAgent, workspace); err != nil { + if err := q.authorizeContext(ctx, policy.ActionUpdate, workspace); err != nil { return database.WorkspaceApp{}, err } diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 5e3edb61c77bb..50373fbeb72e6 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -4114,7 +4114,7 @@ func (s *MethodTestSuite) TestSystemFunctions() { Health: database.WorkspaceAppHealthDisabled, SharingLevel: database.AppSharingLevelOwner, OpenIn: database.WorkspaceAppOpenInSlimWindow, - }).Asserts(ws, policy.ActionCreateAgent) + }).Asserts(ws, policy.ActionUpdate) })) s.Run("InsertWorkspaceResourceMetadata", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertWorkspaceResourceMetadataParams{