Skip to content

Commit f666e13

Browse files
johnstcnjohnstcn
committed
fixup! wip: dbauthz.WithAuthorizeSystemContext -> dbauthz.AsSystem()
1 parent b509b8f commit f666e13

File tree

8 files changed

+31
-83
lines changed

8 files changed

+31
-83
lines changed

‎coderd/coderd.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,6 @@ func New(options *Options) *API {
295295
DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value,
296296
Optional: true,
297297
}),
298-
// TODO: We should remove this auth context after middleware.
299-
httpmw.SystemAuthCtx,
300298
httpmw.ExtractUserParam(api.Database, false),
301299
httpmw.ExtractWorkspaceAndAgentParam(api.Database),
302300
),
@@ -325,8 +323,6 @@ func New(options *Options) *API {
325323
DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value,
326324
Optional: true,
327325
}),
328-
// TODO: We should remove this auth context after middleware.
329-
httpmw.SystemAuthCtx,
330326
// Redirect to the login page if the user tries to open an app with
331327
// "me" as the username and they are not logged in.
332328
httpmw.ExtractUserParam(api.Database, true),

‎coderd/coderdtest/authorize.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,15 @@ import (
1212
"testing"
1313
"time"
1414

15-
"github.com/coder/coder/cryptorand"
1615
"github.com/go-chi/chi/v5"
1716
"github.com/google/uuid"
1817
"github.com/moby/moby/pkg/namesgenerator"
1918
"github.com/stretchr/testify/assert"
2019
"github.com/stretchr/testify/require"
2120
"golang.org/x/xerrors"
2221

22+
"github.com/coder/coder/cryptorand"
23+
2324
"github.com/coder/coder/coderd"
2425
"github.com/coder/coder/coderd/rbac"
2526
"github.com/coder/coder/coderd/rbac/regosql"

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -105,24 +105,10 @@ func ActorFromContext(ctx context.Context) (rbac.Subject, bool) {
105105
return a, ok
106106
}
107107

108-
// func WithAuthorizeContext(ctx context.Context, actor rbac.Subject) context.Context {
109-
// return context.WithValue(ctx, authContextKey{}, actor)
110-
// }
111-
112-
// func WithAuthorizeSystemContext(ctx context.Context, roles rbac.ExpandableRoles) context.Context {
113-
// // TODO: Add protections to search for user roles. If user roles are found,
114-
// // this should panic. That is a developer error that should be caught
115-
// // in unit tests.
116-
// return context.WithValue(ctx, authContextKey{}, rbac.Subject{
117-
// ID: uuid.Nil.String(),
118-
// Roles: roles,
119-
// Scope: rbac.ScopeAll,
120-
// Groups: []string{},
121-
// })
122-
// }
123-
124108
// AsSystem returns a context with a system actor. This is used for internal
125-
// system operations that do not require authorization.
109+
// system operations that are not tied to any particular actor.
110+
// When you use this function, be sure to add a //nolint comment
111+
// explaining why it is necessary.
126112
//
127113
// We trust you have received the usual lecture from the local System
128114
// Administrator. It usually boils down to these three things:
@@ -153,6 +139,8 @@ func AsSystem(ctx context.Context) context.Context {
153139
// As returns a context with the given actor stored in the context.
154140
// This is used for cases where the actor touching the database is not the
155141
// actor stored in the context.
142+
// When you use this function, be sure to add a //nolint comment
143+
// explaining why it is necessary.
156144
func As(ctx context.Context, actor rbac.Subject) context.Context {
157145
return context.WithValue(ctx, authContextKey{}, actor)
158146
}

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727
"github.com/coder/coder/coderd/database"
2828
"github.com/coder/coder/coderd/database/dbauthz"
2929
"github.com/coder/coder/coderd/parameter"
30-
"github.com/coder/coder/coderd/rbac"
3130
"github.com/coder/coder/coderd/telemetry"
3231
"github.com/coder/coder/codersdk"
3332
"github.com/coder/coder/provisioner"
@@ -502,7 +501,7 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p
502501
Valid: failJob.Error != "",
503502
}
504503

505-
err = server.Database.UpdateProvisionerJobWithCompleteByID(ctx, database.UpdateProvisionerJobWithCompleteByIDParams{
504+
err = server.Database.UpdateProvisionerJobWithCompleteByID(dbauthz.AsSystem(ctx), database.UpdateProvisionerJobWithCompleteByIDParams{
506505
ID: jobID,
507506
CompletedAt: job.CompletedAt,
508507
UpdatedAt: database.Now(),
@@ -525,7 +524,7 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p
525524
if err != nil {
526525
return nil, xerrors.Errorf("unmarshal workspace provision input: %w", err)
527526
}
528-
build, err := server.Database.UpdateWorkspaceBuildByID(ctx, database.UpdateWorkspaceBuildByIDParams{
527+
build, err := server.Database.UpdateWorkspaceBuildByID(dbauthz.AsSystem(ctx), database.UpdateWorkspaceBuildByIDParams{
529528
ID: input.WorkspaceBuildID,
530529
UpdatedAt: database.Now(),
531530
ProvisionerState: jobType.WorkspaceBuild.State,
@@ -544,12 +543,12 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p
544543
// if failed job is a workspace build, audit the outcome
545544
if job.Type == database.ProvisionerJobTypeWorkspaceBuild {
546545
auditor := server.Auditor.Load()
547-
build, err := server.Database.GetWorkspaceBuildByJobID(ctx, job.ID)
546+
build, err := server.Database.GetWorkspaceBuildByJobID(dbauthz.AsSystem(ctx), job.ID)
548547
if err != nil {
549548
server.Logger.Error(ctx, "audit log - get build", slog.Error(err))
550549
} else {
551550
auditAction := auditActionFromTransition(build.Transition)
552-
workspace, err := server.Database.GetWorkspaceByID(ctx, build.WorkspaceID)
551+
workspace, err := server.Database.GetWorkspaceByID(dbauthz.AsSystem(ctx), build.WorkspaceID)
553552
if err != nil {
554553
server.Logger.Error(ctx, "audit log - get workspace", slog.Error(err))
555554
} else {
@@ -605,13 +604,13 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p
605604
// CompleteJob is triggered by a provision daemon to mark a provisioner job as completed.
606605
func (server *Server) CompleteJob(ctx context.Context, completed *proto.CompletedJob) (*proto.Empty, error) {
607606
// TODO: make a provisionerd role
608-
ctx = dbauthz.WithAuthorizeSystemContext(ctx, rbac.RolesAdminSystem())
607+
// ctx = dbauthz.WithAuthorizeSystemContext(ctx, rbac.RolesAdminSystem())
609608
jobID, err := uuid.Parse(completed.JobId)
610609
if err != nil {
611610
return nil, xerrors.Errorf("parse job id: %w", err)
612611
}
613612
server.Logger.Debug(ctx, "CompleteJob starting", slog.F("job_id", jobID))
614-
job, err := server.Database.GetProvisionerJobByID(ctx, jobID)
613+
job, err := server.Database.GetProvisionerJobByID(dbauthz.AsSystem(ctx), jobID)
615614
if err != nil {
616615
return nil, xerrors.Errorf("get job by id: %w", err)
617616
}
@@ -642,7 +641,7 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
642641
slog.F("resource_type", resource.Type),
643642
slog.F("transition", transition))
644643

645-
err = InsertWorkspaceResource(ctx, server.Database, jobID, transition, resource, telemetrySnapshot)
644+
err = InsertWorkspaceResource(dbauthz.AsSystem(ctx), server.Database, jobID, transition, resource, telemetrySnapshot)
646645
if err != nil {
647646
return nil, xerrors.Errorf("insert resource: %w", err)
648647
}
@@ -658,7 +657,7 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
658657
if err != nil {
659658
return nil, xerrors.Errorf("marshal parameter options: %w", err)
660659
}
661-
_, err = server.Database.InsertTemplateVersionParameter(ctx, database.InsertTemplateVersionParameterParams{
660+
_, err = server.Database.InsertTemplateVersionParameter(dbauthz.AsSystem(ctx), database.InsertTemplateVersionParameterParams{
662661
TemplateVersionID: input.TemplateVersionID,
663662
Name: richParameter.Name,
664663
Description: richParameter.Description,
@@ -678,7 +677,7 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
678677
}
679678
}
680679

681-
err = server.Database.UpdateProvisionerJobWithCompleteByID(ctx, database.UpdateProvisionerJobWithCompleteByIDParams{
680+
err = server.Database.UpdateProvisionerJobWithCompleteByID(dbauthz.AsSystem(ctx), database.UpdateProvisionerJobWithCompleteByIDParams{
682681
ID: jobID,
683682
UpdatedAt: database.Now(),
684683
CompletedAt: sql.NullTime{
@@ -700,7 +699,7 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
700699
return nil, xerrors.Errorf("unmarshal job data: %w", err)
701700
}
702701

703-
workspaceBuild, err := server.Database.GetWorkspaceBuildByID(ctx, input.WorkspaceBuildID)
702+
workspaceBuild, err := server.Database.GetWorkspaceBuildByID(dbauthz.AsSystem(ctx), input.WorkspaceBuildID)
704703
if err != nil {
705704
return nil, xerrors.Errorf("get workspace build: %w", err)
706705
}
@@ -711,7 +710,7 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
711710
err = server.Database.InTx(func(db database.Store) error {
712711
now := database.Now()
713712
var workspaceDeadline time.Time
714-
workspace, getWorkspaceError = db.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID)
713+
workspace, getWorkspaceError = db.GetWorkspaceByID(dbauthz.AsSystem(ctx), workspaceBuild.WorkspaceID)
715714
if getWorkspaceError == nil {
716715
if workspace.Ttl.Valid {
717716
workspaceDeadline = now.Add(time.Duration(workspace.Ttl.Int64))
@@ -721,7 +720,7 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
721720
// In any case, since this is just for the TTL, try and continue anyway.
722721
server.Logger.Error(ctx, "fetch workspace for build", slog.F("workspace_build_id", workspaceBuild.ID), slog.F("workspace_id", workspaceBuild.WorkspaceID))
723722
}
724-
err = db.UpdateProvisionerJobWithCompleteByID(ctx, database.UpdateProvisionerJobWithCompleteByIDParams{
723+
err = db.UpdateProvisionerJobWithCompleteByID(dbauthz.AsSystem(ctx), database.UpdateProvisionerJobWithCompleteByIDParams{
725724
ID: jobID,
726725
UpdatedAt: database.Now(),
727726
CompletedAt: sql.NullTime{
@@ -732,7 +731,7 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
732731
if err != nil {
733732
return xerrors.Errorf("update provisioner job: %w", err)
734733
}
735-
_, err = db.UpdateWorkspaceBuildByID(ctx, database.UpdateWorkspaceBuildByIDParams{
734+
_, err = db.UpdateWorkspaceBuildByID(dbauthz.AsSystem(ctx), database.UpdateWorkspaceBuildByIDParams{
736735
ID: workspaceBuild.ID,
737736
Deadline: workspaceDeadline,
738737
ProvisionerState: jobType.WorkspaceBuild.State,
@@ -749,7 +748,7 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
749748
dur := time.Duration(protoAgent.GetConnectionTimeoutSeconds()) * time.Second
750749
agentTimeouts[dur] = true
751750
}
752-
err = InsertWorkspaceResource(ctx, db, job.ID, workspaceBuild.Transition, protoResource, telemetrySnapshot)
751+
err = InsertWorkspaceResource(dbauthz.AsSystem(ctx), db, job.ID, workspaceBuild.Transition, protoResource, telemetrySnapshot)
753752
if err != nil {
754753
return xerrors.Errorf("insert provisioner job: %w", err)
755754
}
@@ -798,7 +797,7 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
798797
return nil
799798
}
800799

801-
err = db.UpdateWorkspaceDeletedByID(ctx, database.UpdateWorkspaceDeletedByIDParams{
800+
err = db.UpdateWorkspaceDeletedByID(dbauthz.AsSystem(ctx), database.UpdateWorkspaceDeletedByIDParams{
802801
ID: workspaceBuild.WorkspaceID,
803802
Deleted: true,
804803
})

‎coderd/rbac/builtin.go

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -33,40 +33,6 @@ func (names RoleNames) Names() []string {
3333
return names
3434
}
3535

36-
// RolesAutostartSystem is the limited set of permissions required for autostart
37-
// to function.
38-
// It is EXPLICITLY NOT included in builtinRoles so that it CANNOT be assigned to a user.
39-
// func RolesAutostartSystem() Roles {
40-
// return Roles{
41-
// Role{
42-
// Name: "auto-start",
43-
// DisplayName: "Autostart",
44-
// Site: permissions(map[string][]Action{
45-
// ResourceWorkspace.Type: {ActionRead, ActionUpdate},
46-
// ResourceTemplate.Type: {ActionRead},
47-
// }),
48-
// Org: map[string][]Permission{},
49-
// User: []Permission{},
50-
// },
51-
// }
52-
// }
53-
54-
// RolesAdminSystem is an all-powerful system role. Use sparingly.
55-
// It is EXPLICITLY NOT included in builtinRoles so that it CANNOT be assigned to a user.
56-
// func RolesAdminSystem() Roles {
57-
// return Roles{
58-
// Role{
59-
// Name: "system",
60-
// DisplayName: "System",
61-
// Site: permissions(map[string][]Action{
62-
// ResourceWildcard.Type: {WildcardSymbol},
63-
// }),
64-
// Org: map[string][]Permission{},
65-
// User: []Permission{},
66-
// },
67-
// }
68-
// }
69-
7036
// The functions below ONLY need to exist for roles that are "defaulted" in some way.
7137
// Any other roles (like auditor), can be listed and let the user select/assigned.
7238
// Once we have a database implementation, the "default" roles can be defined on the

‎enterprise/coderd/coderd.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,6 @@ func New(ctx context.Context, options *Options) (*API, error) {
146146
api.AGPL.RootHandler.Route("/scim/v2", func(r chi.Router) {
147147
r.Use(
148148
api.scimEnabledMW,
149-
// TODO: Make a scim auth role.
150-
httpmw.SystemAuthCtx,
151149
)
152150
r.Post("/Users", api.scimPostUser)
153151
r.Route("/Users", func(r chi.Router) {

‎enterprise/coderd/coderd_test.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"time"
88

99
"github.com/coder/coder/coderd/database/dbauthz"
10-
"github.com/coder/coder/coderd/rbac"
1110

1211
"github.com/stretchr/testify/assert"
1312
"github.com/stretchr/testify/require"
@@ -103,7 +102,7 @@ func TestEntitlements(t *testing.T) {
103102
require.NoError(t, err)
104103
require.False(t, entitlements.HasLicense)
105104
coderdtest.CreateFirstUser(t, client)
106-
ctx := dbauthz.WithAuthorizeSystemContext(context.Background(), rbac.RolesAdminSystem())
105+
ctx := dbauthz.AsSystem(context.Background())
107106
_, err = api.Database.InsertLicense(ctx, database.InsertLicenseParams{
108107
UploadedAt: database.Now(),
109108
Exp: database.Now().AddDate(1, 0, 0),
@@ -132,8 +131,8 @@ func TestEntitlements(t *testing.T) {
132131
require.False(t, entitlements.HasLicense)
133132
coderdtest.CreateFirstUser(t, client)
134133
// Valid
135-
ctx := dbauthz.WithAuthorizeSystemContext(context.Background(), rbac.RolesAdminSystem())
136-
_, err = api.Database.InsertLicense(ctx, database.InsertLicenseParams{
134+
ctx := context.Background()
135+
_, err = api.Database.InsertLicense(dbauthz.AsSystem(ctx), database.InsertLicenseParams{
137136
UploadedAt: database.Now(),
138137
Exp: database.Now().AddDate(1, 0, 0),
139138
JWT: coderdenttest.GenerateLicense(t, coderdenttest.LicenseOptions{
@@ -144,7 +143,7 @@ func TestEntitlements(t *testing.T) {
144143
})
145144
require.NoError(t, err)
146145
// Expired
147-
_, err = api.Database.InsertLicense(ctx, database.InsertLicenseParams{
146+
_, err = api.Database.InsertLicense(dbauthz.AsSystem(ctx), database.InsertLicenseParams{
148147
UploadedAt: database.Now(),
149148
Exp: database.Now().AddDate(-1, 0, 0),
150149
JWT: coderdenttest.GenerateLicense(t, coderdenttest.LicenseOptions{
@@ -153,7 +152,7 @@ func TestEntitlements(t *testing.T) {
153152
})
154153
require.NoError(t, err)
155154
// Invalid
156-
_, err = api.Database.InsertLicense(ctx, database.InsertLicenseParams{
155+
_, err = api.Database.InsertLicense(dbauthz.AsSystem(ctx), database.InsertLicenseParams{
157156
UploadedAt: database.Now(),
158157
Exp: database.Now().AddDate(1, 0, 0),
159158
JWT: "invalid",

‎enterprise/coderd/scim.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
agpl "github.com/coder/coder/coderd"
1616
"github.com/coder/coder/coderd/database"
17+
"github.com/coder/coder/coderd/database/dbauthz"
1718
"github.com/coder/coder/coderd/httpapi"
1819
"github.com/coder/coder/codersdk"
1920
)
@@ -155,7 +156,7 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) {
155156
return
156157
}
157158

158-
user, _, err := api.AGPL.CreateUser(ctx, api.Database, agpl.CreateUserRequest{
159+
user, _, err := api.AGPL.CreateUser(dbauthz.AsSystem(ctx), api.Database, agpl.CreateUserRequest{
159160
CreateUserRequest: codersdk.CreateUserRequest{
160161
Username: sUser.UserName,
161162
Email: email,
@@ -207,7 +208,7 @@ func (api *API) scimPatchUser(rw http.ResponseWriter, r *http.Request) {
207208
return
208209
}
209210

210-
dbUser, err := api.Database.GetUserByID(ctx, uid)
211+
dbUser, err := api.Database.GetUserByID(dbauthz.AsSystem(ctx), uid)
211212
if err != nil {
212213
_ = handlerutil.WriteError(rw, err)
213214
return
@@ -220,7 +221,7 @@ func (api *API) scimPatchUser(rw http.ResponseWriter, r *http.Request) {
220221
status = database.UserStatusSuspended
221222
}
222223

223-
_, err = api.Database.UpdateUserStatus(r.Context(), database.UpdateUserStatusParams{
224+
_, err = api.Database.UpdateUserStatus(dbauthz.AsSystem(r.Context()), database.UpdateUserStatusParams{
224225
ID: dbUser.ID,
225226
Status: status,
226227
UpdatedAt: database.Now(),

0 commit comments

Comments
 (0)