Skip to content

Commit 4b6d501

Browse files
dannykoppingdannykopping
committed
Implement and use restricted RBAC context
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
1 parent 287cb6e commit 4b6d501

File tree

5 files changed

+37
-25
lines changed

5 files changed

+37
-25
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818

1919
"cdr.dev/slog"
2020

21+
"github.com/coder/coder/v2/coderd/prebuilds"
2122
"github.com/coder/coder/v2/coderd/rbac/policy"
2223
"github.com/coder/coder/v2/coderd/rbac/rolestore"
2324

@@ -358,6 +359,25 @@ var (
358359
}),
359360
Scope: rbac.ScopeAll,
360361
}.WithCachedASTValue()
362+
363+
subjectPrebuildsOrchestrator = rbac.Subject{
364+
FriendlyName: "Prebuilds Orchestrator",
365+
ID: prebuilds.OwnerID.String(),
366+
Roles: rbac.Roles([]rbac.Role{
367+
{
368+
Identifier: rbac.RoleIdentifier{Name: "prebuilds-orchestrator"},
369+
DisplayName: "Coder",
370+
Site: rbac.Permissions(map[string][]policy.Action{
371+
// May use template, read template-related info, & insert template-related resources (preset prebuilds).
372+
rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionUse},
373+
// May CRUD workspaces, and start/stop them.
374+
rbac.ResourceWorkspace.Type: {policy.ActionCreate, policy.ActionDelete, policy.ActionRead, policy.ActionUpdate,
375+
policy.ActionWorkspaceStart, policy.ActionWorkspaceStop},
376+
}),
377+
},
378+
}),
379+
Scope: rbac.ScopeAll,
380+
}.WithCachedASTValue()
361381
)
362382

363383
// AsProvisionerd returns a context with an actor that has permissions required
@@ -412,6 +432,12 @@ func AsSystemReadProvisionerDaemons(ctx context.Context) context.Context {
412432
return context.WithValue(ctx, authContextKey{}, subjectSystemReadProvisionerDaemons)
413433
}
414434

435+
// AsPrebuildsOrchestrator returns a context with an actor that has permissions
436+
// to read orchestrator workspace prebuilds.
437+
func AsPrebuildsOrchestrator(ctx context.Context) context.Context {
438+
return context.WithValue(ctx, authContextKey{}, subjectPrebuildsOrchestrator)
439+
}
440+
415441
var AsRemoveActor = rbac.Subject{
416442
ID: "remove-actor",
417443
}

‎coderd/workspaces.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -809,16 +809,10 @@ func createWorkspace(
809809
}
810810

811811
func claimPrebuild(ctx context.Context, claimer prebuilds.Claimer, db database.Store, logger slog.Logger, req codersdk.CreateWorkspaceRequest, owner workspaceOwner) (*database.Workspace, error) {
812-
// TODO: authz // Can't use existing profiles (i.e. AsSystemRestricted) because of dbauthz rules
813-
ownerCtx := dbauthz.As(ctx, rbac.Subject{
814-
ID: "owner",
815-
Roles: rbac.RoleIdentifiers{rbac.RoleOwner()},
816-
Groups: []string{},
817-
Scope: rbac.ExpandableScope(rbac.ScopeAll),
818-
})
812+
prebuildsCtx := dbauthz.AsPrebuildsOrchestrator(ctx)
819813

820814
// TODO: do we need a timeout here?
821-
claimCtx, cancel := context.WithTimeout(ownerCtx, time.Second*10) // TODO: don't use elevated authz context
815+
claimCtx, cancel := context.WithTimeout(prebuildsCtx, time.Second*10)
822816
defer cancel()
823817

824818
claimedID, err := claimer.Claim(claimCtx, db, owner.ID, req.Name, req.TemplateVersionPresetID)
@@ -832,7 +826,7 @@ func claimPrebuild(ctx context.Context, claimer prebuilds.Claimer, db database.S
832826
return nil, nil
833827
}
834828

835-
lookup, err := db.GetWorkspaceByID(ownerCtx, *claimedID) // TODO: don't use elevated authz context
829+
lookup, err := db.GetWorkspaceByID(prebuildsCtx, *claimedID)
836830
if err != nil {
837831
logger.Error(ctx, "unable to find claimed workspace by ID", slog.Error(err), slog.F("claimed_prebuild_id", (*claimedID).String()))
838832
return nil, xerrors.Errorf("find claimed workspace by ID %q: %w", (*claimedID).String(), err)

‎enterprise/coderd/prebuilds/claim_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func TestClaimPrebuild(t *testing.T) {
175175

176176
userClient, user := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleMember())
177177

178-
ctx = dbauthz.AsSystemRestricted(ctx)
178+
ctx = dbauthz.AsPrebuildsOrchestrator(ctx)
179179

180180
// Given: the reconciliation state is snapshot.
181181
state, err := reconciler.SnapshotState(ctx, spy)

‎enterprise/coderd/prebuilds/metricscollector.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,10 @@ func (*MetricsCollector) Describe(descCh chan<- *prometheus.Desc) {
5050
}
5151

5252
func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) {
53-
// TODO (sasswart): get a proper actor in here, to deescalate from system
54-
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
53+
ctx, cancel := context.WithTimeout(dbauthz.AsPrebuildsOrchestrator(context.Background()), 10*time.Second)
5554
defer cancel()
5655
// nolint:gocritic // just until we get back to this
57-
prebuildMetrics, err := mc.database.GetPrebuildMetrics(dbauthz.AsSystemRestricted(ctx))
56+
prebuildMetrics, err := mc.database.GetPrebuildMetrics(ctx)
5857
if err != nil {
5958
mc.logger.Error(ctx, "failed to get prebuild metrics", slog.Error(err))
6059
return
@@ -66,7 +65,7 @@ func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) {
6665
metricsCh <- prometheus.MustNewConstMetric(claimedPrebuildsDesc, prometheus.CounterValue, float64(metric.ClaimedCount), metric.TemplateName, metric.PresetName)
6766
}
6867

69-
state, err := mc.reconciler.SnapshotState(dbauthz.AsSystemRestricted(ctx), mc.database)
68+
state, err := mc.reconciler.SnapshotState(ctx, mc.database)
7069
if err != nil {
7170
mc.logger.Error(ctx, "failed to get latest prebuild state", slog.Error(err))
7271
return

‎enterprise/coderd/prebuilds/reconcile.go

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,7 @@ func (c *StoreReconciler) RunLoop(ctx context.Context) {
7070
c.done <- struct{}{}
7171
}()
7272

73-
// nolint:gocritic // TODO: create a new authz role
74-
ctx, cancel := context.WithCancelCause(dbauthz.AsSystemRestricted(ctx))
73+
ctx, cancel := context.WithCancelCause(dbauthz.AsPrebuildsOrchestrator(ctx))
7574
c.cancelFn = cancel
7675

7776
for {
@@ -296,13 +295,7 @@ func (c *StoreReconciler) Reconcile(ctx context.Context, ps prebuilds.PresetStat
296295
vlogger := logger.With(slog.F("template_version_id", ps.Preset.TemplateVersionID), slog.F("template_version_name", ps.Preset.TemplateVersionName),
297296
slog.F("preset_id", ps.Preset.PresetID), slog.F("preset_name", ps.Preset.Name))
298297

299-
// TODO: authz // Can't use existing profiles (i.e. AsSystemRestricted) because of dbauthz rules
300-
ownerCtx := dbauthz.As(ctx, rbac.Subject{
301-
ID: "owner",
302-
Roles: rbac.RoleIdentifiers{rbac.RoleOwner()},
303-
Groups: []string{},
304-
Scope: rbac.ExpandableScope(rbac.ScopeAll),
305-
})
298+
prebuildsCtx := dbauthz.AsPrebuildsOrchestrator(ctx)
306299

307300
levelFn := vlogger.Debug
308301
if actions.Create > 0 || len(actions.DeleteIDs) > 0 {
@@ -344,14 +337,14 @@ func (c *StoreReconciler) Reconcile(ctx context.Context, ps prebuilds.PresetStat
344337

345338
// TODO: i've removed the surrounding tx, but if we restore it then we need to pass down the store to these funcs.
346339
for range actions.Create {
347-
if err := c.createPrebuild(ownerCtx, uuid.New(), ps.Preset.TemplateID, ps.Preset.PresetID); err != nil {
340+
if err := c.createPrebuild(prebuildsCtx, uuid.New(), ps.Preset.TemplateID, ps.Preset.PresetID); err != nil {
348341
vlogger.Error(ctx, "failed to create prebuild", slog.Error(err))
349342
lastErr.Errors = append(lastErr.Errors, err)
350343
}
351344
}
352345

353346
for _, id := range actions.DeleteIDs {
354-
if err := c.deletePrebuild(ownerCtx, id, ps.Preset.TemplateID, ps.Preset.PresetID); err != nil {
347+
if err := c.deletePrebuild(prebuildsCtx, id, ps.Preset.TemplateID, ps.Preset.PresetID); err != nil {
355348
vlogger.Error(ctx, "failed to delete prebuild", slog.Error(err))
356349
lastErr.Errors = append(lastErr.Errors, err)
357350
}

0 commit comments

Comments
 (0)