fix(coderd): authorize workspace start/stop/delete by transition action (#21691)

Use transition-specific actions when authorizing workspace build
parameter inserts in the database layer so start/stop/delete do not
require workspace.update.

Related to: https://github.com/coder/internal/issues/1299
This commit is contained in:
George K
2026-01-27 09:08:12 -08:00
committed by GitHub
parent 2ee3386cc5
commit c352a51b22
3 changed files with 73 additions and 13 deletions
+22 -6
View File
@@ -174,6 +174,19 @@ func (q *querier) authorizePrebuiltWorkspace(ctx context.Context, action policy.
return xerrors.Errorf("authorize context: %w", workspaceErr) return xerrors.Errorf("authorize context: %w", workspaceErr)
} }
func workspaceTransitionAction(transition database.WorkspaceTransition) (policy.Action, error) {
switch transition {
case database.WorkspaceTransitionStart:
return policy.ActionWorkspaceStart, nil
case database.WorkspaceTransitionStop:
return policy.ActionWorkspaceStop, nil
case database.WorkspaceTransitionDelete:
return policy.ActionDelete, nil
default:
return "", xerrors.Errorf("unsupported workspace transition %q", transition)
}
}
// authorizeAIBridgeInterceptionAction validates that the context's actor matches the initiator of the AIBridgeInterception. // authorizeAIBridgeInterceptionAction validates that the context's actor matches the initiator of the AIBridgeInterception.
// This is used by all of the sub-resources which fall under the [ResourceAibridgeInterception] umbrella. // This is used by all of the sub-resources which fall under the [ResourceAibridgeInterception] umbrella.
func (q *querier) authorizeAIBridgeInterceptionAction(ctx context.Context, action policy.Action, interceptionID uuid.UUID) error { func (q *querier) authorizeAIBridgeInterceptionAction(ctx context.Context, action policy.Action, interceptionID uuid.UUID) error {
@@ -4594,11 +4607,9 @@ func (q *querier) InsertWorkspaceBuild(ctx context.Context, arg database.InsertW
return xerrors.Errorf("get workspace by id: %w", err) return xerrors.Errorf("get workspace by id: %w", err)
} }
var action policy.Action = policy.ActionWorkspaceStart action, err := workspaceTransitionAction(arg.Transition)
if arg.Transition == database.WorkspaceTransitionDelete { if err != nil {
action = policy.ActionDelete return err
} else if arg.Transition == database.WorkspaceTransitionStop {
action = policy.ActionWorkspaceStop
} }
// Special handling for prebuilt workspace deletion // Special handling for prebuilt workspace deletion
@@ -4642,8 +4653,13 @@ func (q *querier) InsertWorkspaceBuildParameters(ctx context.Context, arg databa
return err return err
} }
action, err := workspaceTransitionAction(build.Transition)
if err != nil {
return err
}
// Special handling for prebuilt workspace deletion // Special handling for prebuilt workspace deletion
if err := q.authorizePrebuiltWorkspace(ctx, policy.ActionUpdate, workspace); err != nil { if err := q.authorizePrebuiltWorkspace(ctx, action, workspace); err != nil {
return err return err
} }
+41 -5
View File
@@ -2145,9 +2145,12 @@ func (s *MethodTestSuite) TestWorkspace() {
dbm.EXPECT().InsertWorkspaceBuild(gomock.Any(), arg).Return(nil).AnyTimes() dbm.EXPECT().InsertWorkspaceBuild(gomock.Any(), arg).Return(nil).AnyTimes()
check.Args(arg).Asserts(w, policy.ActionDelete) check.Args(arg).Asserts(w, policy.ActionDelete)
})) }))
s.Run("InsertWorkspaceBuildParameters", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { s.Run("Start/InsertWorkspaceBuildParameters", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
w := testutil.Fake(s.T(), faker, database.Workspace{}) w := testutil.Fake(s.T(), faker, database.Workspace{})
b := testutil.Fake(s.T(), faker, database.WorkspaceBuild{WorkspaceID: w.ID}) b := testutil.Fake(s.T(), faker, database.WorkspaceBuild{
WorkspaceID: w.ID,
Transition: database.WorkspaceTransitionStart,
})
arg := database.InsertWorkspaceBuildParametersParams{ arg := database.InsertWorkspaceBuildParametersParams{
WorkspaceBuildID: b.ID, WorkspaceBuildID: b.ID,
Name: []string{"foo", "bar"}, Name: []string{"foo", "bar"},
@@ -2156,7 +2159,39 @@ func (s *MethodTestSuite) TestWorkspace() {
dbm.EXPECT().GetWorkspaceBuildByID(gomock.Any(), b.ID).Return(b, nil).AnyTimes() dbm.EXPECT().GetWorkspaceBuildByID(gomock.Any(), b.ID).Return(b, nil).AnyTimes()
dbm.EXPECT().GetWorkspaceByID(gomock.Any(), w.ID).Return(w, nil).AnyTimes() dbm.EXPECT().GetWorkspaceByID(gomock.Any(), w.ID).Return(w, nil).AnyTimes()
dbm.EXPECT().InsertWorkspaceBuildParameters(gomock.Any(), arg).Return(nil).AnyTimes() dbm.EXPECT().InsertWorkspaceBuildParameters(gomock.Any(), arg).Return(nil).AnyTimes()
check.Args(arg).Asserts(w, policy.ActionUpdate) check.Args(arg).Asserts(w, policy.ActionWorkspaceStart)
}))
s.Run("Stop/InsertWorkspaceBuildParameters", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
w := testutil.Fake(s.T(), faker, database.Workspace{})
b := testutil.Fake(s.T(), faker, database.WorkspaceBuild{
WorkspaceID: w.ID,
Transition: database.WorkspaceTransitionStop,
})
arg := database.InsertWorkspaceBuildParametersParams{
WorkspaceBuildID: b.ID,
Name: []string{"foo", "bar"},
Value: []string{"baz", "qux"},
}
dbm.EXPECT().GetWorkspaceBuildByID(gomock.Any(), b.ID).Return(b, nil).AnyTimes()
dbm.EXPECT().GetWorkspaceByID(gomock.Any(), w.ID).Return(w, nil).AnyTimes()
dbm.EXPECT().InsertWorkspaceBuildParameters(gomock.Any(), arg).Return(nil).AnyTimes()
check.Args(arg).Asserts(w, policy.ActionWorkspaceStop)
}))
s.Run("Delete/InsertWorkspaceBuildParameters", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
w := testutil.Fake(s.T(), faker, database.Workspace{})
b := testutil.Fake(s.T(), faker, database.WorkspaceBuild{
WorkspaceID: w.ID,
Transition: database.WorkspaceTransitionDelete,
})
arg := database.InsertWorkspaceBuildParametersParams{
WorkspaceBuildID: b.ID,
Name: []string{"foo", "bar"},
Value: []string{"baz", "qux"},
}
dbm.EXPECT().GetWorkspaceBuildByID(gomock.Any(), b.ID).Return(b, nil).AnyTimes()
dbm.EXPECT().GetWorkspaceByID(gomock.Any(), w.ID).Return(w, nil).AnyTimes()
dbm.EXPECT().InsertWorkspaceBuildParameters(gomock.Any(), arg).Return(nil).AnyTimes()
check.Args(arg).Asserts(w, policy.ActionDelete)
})) }))
s.Run("UpdateWorkspace", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { s.Run("UpdateWorkspace", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
w := testutil.Fake(s.T(), faker, database.Workspace{}) w := testutil.Fake(s.T(), faker, database.Workspace{})
@@ -4407,7 +4442,7 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() {
return nil return nil
}).Asserts(w, policy.ActionDelete, w.AsPrebuild(), policy.ActionDelete) }).Asserts(w, policy.ActionDelete, w.AsPrebuild(), policy.ActionDelete)
})) }))
s.Run("PrebuildUpdate/InsertWorkspaceBuildParameters", s.Subtest(func(db database.Store, check *expects) { s.Run("PrebuildDelete/InsertWorkspaceBuildParameters", s.Subtest(func(db database.Store, check *expects) {
u := dbgen.User(s.T(), db, database.User{}) u := dbgen.User(s.T(), db, database.User{})
o := dbgen.Organization(s.T(), db, database.Organization{}) o := dbgen.Organization(s.T(), db, database.Organization{})
tpl := dbgen.Template(s.T(), db, database.Template{ tpl := dbgen.Template(s.T(), db, database.Template{
@@ -4429,6 +4464,7 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() {
}) })
wb := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{ wb := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{
JobID: pj.ID, JobID: pj.ID,
Transition: database.WorkspaceTransitionDelete,
WorkspaceID: w.ID, WorkspaceID: w.ID,
TemplateVersionID: tv.ID, TemplateVersionID: tv.ID,
}) })
@@ -4444,7 +4480,7 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() {
return xerrors.Errorf("not authorized for workspace type") return xerrors.Errorf("not authorized for workspace type")
} }
return nil return nil
}).Asserts(w, policy.ActionUpdate, w.AsPrebuild(), policy.ActionUpdate) }).Asserts(w, policy.ActionDelete, w.AsPrebuild(), policy.ActionDelete)
})) }))
} }
+10 -2
View File
@@ -1175,8 +1175,16 @@ func (b *Builder) authorize(authFunc func(action policy.Action, object rbac.Obje
switch b.trans { switch b.trans {
case database.WorkspaceTransitionDelete: case database.WorkspaceTransitionDelete:
action = policy.ActionDelete action = policy.ActionDelete
case database.WorkspaceTransitionStart, database.WorkspaceTransitionStop: case database.WorkspaceTransitionStart:
action = policy.ActionUpdate action = policy.ActionWorkspaceStart
if b.workspace.DormantAt.Valid {
// Dormant workspaces can't be started directly; they are
// first "woken" by unsetting dormancy, which makes the
// workspace.start permission apply.
action = policy.ActionUpdate
}
case database.WorkspaceTransitionStop:
action = policy.ActionWorkspaceStop
default: default:
msg := fmt.Sprintf("Transition %q not supported.", b.trans) msg := fmt.Sprintf("Transition %q not supported.", b.trans)
return BuildError{http.StatusBadRequest, msg, xerrors.New(msg)} return BuildError{http.StatusBadRequest, msg, xerrors.New(msg)}