Skip to content

Commit 2c34f6d

Browse files
committed
fix linter errors
1 parent 2724dfd commit 2c34f6d

File tree

17 files changed

+69
-26
lines changed

17 files changed

+69
-26
lines changed

coderd/autobuild/executor/lifecycle_executor.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ type Stats struct {
3434
// New returns a new autobuild executor.
3535
func New(ctx context.Context, db database.Store, log slog.Logger, tick <-chan time.Time) *Executor {
3636
le := &Executor{
37-
// Use an authorized context
37+
//nolint:gocritic // TODO: make an autostart role instead of using System
3838
ctx: dbauthz.AsSystem(ctx),
3939
db: db,
4040
tick: tick,

coderd/database/dbauthz/dbauthz.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@ import (
88
"github.com/google/uuid"
99
"golang.org/x/xerrors"
1010

11+
"github.com/open-policy-agent/opa/topdown"
12+
1113
"cdr.dev/slog"
1214
"github.com/coder/coder/coderd/database"
1315
"github.com/coder/coder/coderd/rbac"
14-
"github.com/open-policy-agent/opa/topdown"
1516
)
1617

1718
var _ database.Store = (*querier)(nil)

coderd/database/dbauthz/setup_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,7 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec
151151
"GetAuthorizedWorkspaces",
152152
"GetAuthorizedTemplates",
153153
}, methodName) {
154-
155-
// Some methods do no make rbac assertions because they use
154+
// Some methods do not make RBAC assertions because they use
156155
// SQL. We still want to test that they return an error if the
157156
// actor is not set.
158157
s.NoActorErrorTest(callMethod)

coderd/httpmw/apikey.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,6 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
116116
return func(next http.Handler) http.Handler {
117117
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
118118
ctx := r.Context()
119-
// systemCtx := dbauthz.WithAuthorizeSystemContext(ctx, rbac.RolesAdminSystem())
120119
// Write wraps writing a response to redirect if the handler
121120
// specified it should. This redirect is used for user-facing pages
122121
// like workspace applications.
@@ -161,6 +160,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
161160
return
162161
}
163162

163+
//nolint:gocritic // System needs to fetch API key to check if it's valid.
164164
key, err := cfg.DB.GetAPIKeyByID(dbauthz.AsSystem(ctx), keyID)
165165
if err != nil {
166166
if errors.Is(err, sql.ErrNoRows) {
@@ -194,6 +194,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
194194
changed = false
195195
)
196196
if key.LoginType == database.LoginTypeGithub || key.LoginType == database.LoginTypeOIDC {
197+
//nolint:gocritic // System needs to fetch UserLink to check if it's valid.
197198
link, err = cfg.DB.GetUserLinkByUserIDLoginType(dbauthz.AsSystem(ctx), database.GetUserLinkByUserIDLoginTypeParams{
198199
UserID: key.UserID,
199200
LoginType: key.LoginType,
@@ -277,6 +278,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
277278
}
278279
}
279280
if changed {
281+
//nolint:gocritic // System needs to update API Key LastUsed
280282
err := cfg.DB.UpdateAPIKeyByID(dbauthz.AsSystem(ctx), database.UpdateAPIKeyByIDParams{
281283
ID: key.ID,
282284
LastUsed: key.LastUsed,
@@ -293,6 +295,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
293295
// If the API Key is associated with a user_link (e.g. Github/OIDC)
294296
// then we want to update the relevant oauth fields.
295297
if link.UserID != uuid.Nil {
298+
// nolint:gocritic
296299
link, err = cfg.DB.UpdateUserLink(dbauthz.AsSystem(ctx), database.UpdateUserLinkParams{
297300
UserID: link.UserID,
298301
LoginType: link.LoginType,
@@ -312,6 +315,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
312315
// We only want to update this occasionally to reduce DB write
313316
// load. We update alongside the UserLink and APIKey since it's
314317
// easier on the DB to colocate writes.
318+
// nolint:gocritic
315319
_, err = cfg.DB.UpdateUserLastSeenAt(dbauthz.AsSystem(ctx), database.UpdateUserLastSeenAtParams{
316320
ID: key.UserID,
317321
LastSeenAt: database.Now(),
@@ -329,6 +333,7 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
329333
// If the key is valid, we also fetch the user roles and status.
330334
// The roles are used for RBAC authorize checks, and the status
331335
// is to block 'suspended' users from accessing the platform.
336+
// nolint:gocritic
332337
roles, err := cfg.DB.GetAuthorizationUserRoles(dbauthz.AsSystem(ctx), key.UserID)
333338
if err != nil {
334339
write(http.StatusUnauthorized, codersdk.Response{

coderd/httpmw/authz.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ import (
88
"github.com/go-chi/chi/v5"
99
)
1010

11-
// AsAuthzSystem is a bit of a kludge for now. Some middleware functions require
12-
// usage as a system user in some cases, but not all cases. To avoid large
13-
// refactors, we use this middleware to temporarily set the context to a system.
11+
// AsAuthzSystem is a chained handler that temporarily sets the dbauthz context
12+
// to System for the inner handlers, and resets the context afterwards.
1413
//
1514
// TODO: Refactor the middleware functions to not require this.
15+
// This is a bit of a kludge for now as some middleware functions require
16+
// usage as a system user in some cases, but not all cases. To avoid large
17+
// refactors, we use this middleware to temporarily set the context to a system.
1618
func AsAuthzSystem(mws ...func(http.Handler) http.Handler) func(http.Handler) http.Handler {
1719
chain := chi.Chain(mws...)
1820
return func(next http.Handler) http.Handler {
@@ -24,6 +26,7 @@ func AsAuthzSystem(mws ...func(http.Handler) http.Handler) func(http.Handler) ht
2426
before = dbauthz.AsRemoveActor
2527
}
2628

29+
// nolint:gocritic // AsAuthzSystem needs to do this.
2730
r = r.WithContext(dbauthz.AsSystem(ctx))
2831
chain.Handler(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
2932
r = r.WithContext(dbauthz.As(r.Context(), before))

coderd/httpmw/authz_test.go

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

1616
func TestAsAuthzSystem(t *testing.T) {
17+
t.Parallel()
1718
userActor := coderdtest.RandomRBACSubject()
1819

1920
base := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
@@ -62,6 +63,7 @@ func TestAsAuthzSystem(t *testing.T) {
6263
mwAssertSystem,
6364
mwAssertSystem,
6465
),
66+
// Assert no user present outside of the AsAuthzSystem chain
6567
mwAssertNoUser,
6668
// ----
6769
// Set to the user actor
@@ -85,10 +87,10 @@ func TestAsAuthzSystem(t *testing.T) {
8587
handler.ServeHTTP(res, req)
8688
}
8789

88-
func mwAssert(assert func(req *http.Request)) func(next http.Handler) http.Handler {
90+
func mwAssert(assertF func(req *http.Request)) func(next http.Handler) http.Handler {
8991
return func(next http.Handler) http.Handler {
9092
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
91-
assert(r)
93+
assertF(r)
9294
next.ServeHTTP(rw, r)
9395
})
9496
}

coderd/httpmw/userparam.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ func ExtractUserParam(db database.Store, redirectToLoginOnMe bool) func(http.Han
6969
})
7070
return
7171
}
72+
//nolint:gocritic // System needs to be able to get user from param.
7273
user, err = db.GetUserByID(dbauthz.AsSystem(ctx), apiKey.UserID)
7374
if xerrors.Is(err, sql.ErrNoRows) {
7475
httpapi.ResourceNotFound(rw)
@@ -82,7 +83,7 @@ func ExtractUserParam(db database.Store, redirectToLoginOnMe bool) func(http.Han
8283
return
8384
}
8485
} else if userID, err := uuid.Parse(userQuery); err == nil {
85-
// If the userQuery is a valid uuid
86+
//nolint:gocritic // If the userQuery is a valid uuid
8687
user, err = db.GetUserByID(dbauthz.AsSystem(ctx), userID)
8788
if err != nil {
8889
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
@@ -91,7 +92,7 @@ func ExtractUserParam(db database.Store, redirectToLoginOnMe bool) func(http.Han
9192
return
9293
}
9394
} else {
94-
// Try as a username last
95+
// nolint:gocritic // Try as a username last
9596
user, err = db.GetUserByEmailOrUsername(dbauthz.AsSystem(ctx), database.GetUserByEmailOrUsernameParams{
9697
Username: userQuery,
9798
})

coderd/httpmw/workspaceagent.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ func ExtractWorkspaceAgent(db database.Store) func(http.Handler) http.Handler {
3232
return func(next http.Handler) http.Handler {
3333
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
3434
ctx := r.Context()
35-
// dbauthz.AsSystem(ctx) := dbauthz.WithAuthorizeSystemContext(ctx, rbac.RolesAdminSystem())
3635
tokenValue := apiTokenFromRequest(r)
3736
if tokenValue == "" {
3837
httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
@@ -48,6 +47,7 @@ func ExtractWorkspaceAgent(db database.Store) func(http.Handler) http.Handler {
4847
})
4948
return
5049
}
50+
//nolint:gocritic // System needs to be able to get workspace agents.
5151
agent, err := db.GetWorkspaceAgentByAuthToken(dbauthz.AsSystem(ctx), token)
5252
if err != nil {
5353
if errors.Is(err, sql.ErrNoRows) {
@@ -65,6 +65,7 @@ func ExtractWorkspaceAgent(db database.Store) func(http.Handler) http.Handler {
6565
return
6666
}
6767

68+
//nolint:gocritic // System needs to be able to get workspace agents.
6869
subject, err := getAgentSubject(dbauthz.AsSystem(ctx), db, agent)
6970
if err != nil {
7071
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{

coderd/metricscache/metricscache.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,9 @@ func countUniqueUsers(rows []database.GetTemplateDAUsRow) int {
143143
}
144144

145145
func (c *Cache) refresh(ctx context.Context) error {
146-
// dbauthz.AsSystem(ctx) := dbauthz.WithAuthorizeSystemContext(ctx, rbac.RolesAdminSystem())
147-
err := c.database.DeleteOldAgentStats(dbauthz.AsSystem(ctx))
146+
//nolint:gocritic // This is a system service.
147+
ctx = dbauthz.AsSystem(ctx)
148+
err := c.database.DeleteOldAgentStats(ctx)
148149
if err != nil {
149150
return xerrors.Errorf("delete old stats: %w", err)
150151
}
@@ -161,22 +162,22 @@ func (c *Cache) refresh(ctx context.Context) error {
161162
templateAverageBuildTimes = make(map[uuid.UUID]database.GetTemplateAverageBuildTimeRow)
162163
)
163164

164-
rows, err := c.database.GetDeploymentDAUs(dbauthz.AsSystem(ctx))
165+
rows, err := c.database.GetDeploymentDAUs(ctx)
165166
if err != nil {
166167
return err
167168
}
168169
deploymentDAUs = convertDeploymentDAUResponse(rows)
169170
c.deploymentDAUResponses.Store(&deploymentDAUs)
170171

171172
for _, template := range templates {
172-
rows, err := c.database.GetTemplateDAUs(dbauthz.AsSystem(ctx), template.ID)
173+
rows, err := c.database.GetTemplateDAUs(ctx, template.ID)
173174
if err != nil {
174175
return err
175176
}
176177
templateDAUs[template.ID] = convertDAUResponse(rows)
177178
templateUniqueUsers[template.ID] = countUniqueUsers(rows)
178179

179-
templateAvgBuildTime, err := c.database.GetTemplateAverageBuildTime(dbauthz.AsSystem(ctx), database.GetTemplateAverageBuildTimeParams{
180+
templateAvgBuildTime, err := c.database.GetTemplateAverageBuildTime(ctx, database.GetTemplateAverageBuildTimeParams{
180181
TemplateID: uuid.NullUUID{
181182
UUID: template.ID,
182183
Valid: true,

coderd/provisionerdserver/provisionerdserver.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ type Server struct {
5757

5858
// AcquireJob queries the database to lock a job.
5959
func (server *Server) AcquireJob(ctx context.Context, _ *proto.Empty) (*proto.AcquiredJob, error) {
60-
// TODO: make a provisionerd role
60+
//nolint:gocritic //TODO: make a provisionerd role
6161
ctx = dbauthz.AsSystem(ctx)
6262
// This prevents loads of provisioner daemons from consistently
6363
// querying the database when no jobs are available.
@@ -273,6 +273,7 @@ func (server *Server) AcquireJob(ctx context.Context, _ *proto.Empty) (*proto.Ac
273273
}
274274

275275
func (server *Server) CommitQuota(ctx context.Context, request *proto.CommitQuotaRequest) (*proto.CommitQuotaResponse, error) {
276+
//nolint:gocritic //TODO: make a provisionerd role
276277
ctx = dbauthz.AsSystem(ctx)
277278
jobID, err := uuid.Parse(request.JobId)
278279
if err != nil {
@@ -303,7 +304,7 @@ func (server *Server) CommitQuota(ctx context.Context, request *proto.CommitQuot
303304
}
304305

305306
func (server *Server) UpdateJob(ctx context.Context, request *proto.UpdateJobRequest) (*proto.UpdateJobResponse, error) {
306-
// TODO: make a provisionerd role
307+
//nolint:gocritic //TODO: make a provisionerd role
307308
ctx = dbauthz.AsSystem(ctx)
308309
parsedID, err := uuid.Parse(request.JobId)
309310
if err != nil {
@@ -351,6 +352,7 @@ func (server *Server) UpdateJob(ctx context.Context, request *proto.UpdateJobReq
351352
slog.F("stage", log.Stage),
352353
slog.F("output", log.Output))
353354
}
355+
//nolint:gocritic //TODO: make a provisionerd role
354356
logs, err := server.Database.InsertProvisionerJobLogs(dbauthz.AsSystem(context.Background()), insertParams)
355357
if err != nil {
356358
server.Logger.Error(ctx, "failed to insert job logs", slog.F("job_id", parsedID), slog.Error(err))
@@ -476,7 +478,7 @@ func (server *Server) UpdateJob(ctx context.Context, request *proto.UpdateJobReq
476478
}
477479

478480
func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*proto.Empty, error) {
479-
// TODO: make a provisionerd role
481+
//nolint:gocritic // TODO: make a provisionerd role
480482
ctx = dbauthz.AsSystem(ctx)
481483
jobID, err := uuid.Parse(failJob.JobId)
482484
if err != nil {
@@ -604,7 +606,7 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p
604606

605607
// CompleteJob is triggered by a provision daemon to mark a provisioner job as completed.
606608
func (server *Server) CompleteJob(ctx context.Context, completed *proto.CompletedJob) (*proto.Empty, error) {
607-
// TODO: make a provisionerd role
609+
//nolint:gocritic // TODO: make a provisionerd role
608610
ctx = dbauthz.AsSystem(ctx)
609611
jobID, err := uuid.Parse(completed.JobId)
610612
if err != nil {

coderd/userauth.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ import (
4040
// @Router /users/login [post]
4141
func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) {
4242
var (
43-
ctx = r.Context()
44-
// dbauthz.AsSystem(ctx) = dbauthz.WithAuthorizeSystemContext(ctx, rbac.RolesAdminSystem())
43+
ctx = r.Context()
4544
auditor = api.Auditor.Load()
4645
aReq, commitAudit = audit.InitRequest[database.APIKey](rw, &audit.RequestParams{
4746
Audit: *auditor,
@@ -58,6 +57,7 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) {
5857
return
5958
}
6059

60+
//nolint:gocritic // In order to login, we need to get the user first!
6161
user, err := api.Database.GetUserByEmailOrUsername(dbauthz.AsSystem(ctx), database.GetUserByEmailOrUsernameParams{
6262
Email: loginWithPassword.Email,
6363
})
@@ -732,8 +732,7 @@ func (e httpError) Error() string {
732732

733733
func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cookie, database.APIKey, error) {
734734
var (
735-
ctx = r.Context()
736-
// dbauthz.AsSystem(ctx) = dbauthz.WithAuthorizeSystemContext(ctx, rbac.RolesAdminSystem())
735+
ctx = r.Context()
737736
user database.User
738737
)
739738

@@ -767,6 +766,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
767766
// with OIDC for the first time.
768767
if user.ID == uuid.Nil {
769768
var organizationID uuid.UUID
769+
//nolint:gocritic
770770
organizations, _ := tx.GetOrganizations(dbauthz.AsSystem(ctx))
771771
if len(organizations) > 0 {
772772
// Add the user to the first organization. Once multi-organization
@@ -775,6 +775,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
775775
organizationID = organizations[0].ID
776776
}
777777

778+
//nolint:gocritic
778779
_, err := tx.GetUserByEmailOrUsername(dbauthz.AsSystem(ctx), database.GetUserByEmailOrUsernameParams{
779780
Username: params.Username,
780781
})
@@ -788,6 +789,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
788789

789790
params.Username = httpapi.UsernameFrom(alternate)
790791

792+
//nolint:gocritic
791793
_, err := tx.GetUserByEmailOrUsername(dbauthz.AsSystem(ctx), database.GetUserByEmailOrUsernameParams{
792794
Username: params.Username,
793795
})
@@ -807,6 +809,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
807809
}
808810
}
809811

812+
//nolint:gocritic
810813
user, _, err = api.CreateUser(dbauthz.AsSystem(ctx), tx, CreateUserRequest{
811814
CreateUserRequest: codersdk.CreateUserRequest{
812815
Email: params.Email,
@@ -821,6 +824,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
821824
}
822825

823826
if link.UserID == uuid.Nil {
827+
//nolint:gocritic
824828
link, err = tx.InsertUserLink(dbauthz.AsSystem(ctx), database.InsertUserLinkParams{
825829
UserID: user.ID,
826830
LoginType: params.LoginType,
@@ -835,6 +839,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
835839
}
836840

837841
if link.UserID != uuid.Nil {
842+
//nolint:gocritic
838843
link, err = tx.UpdateUserLink(dbauthz.AsSystem(ctx), database.UpdateUserLinkParams{
839844
UserID: user.ID,
840845
LoginType: params.LoginType,
@@ -849,6 +854,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
849854

850855
// Ensure groups are correct.
851856
if len(params.Groups) > 0 {
857+
//nolint:gocritic
852858
err := api.Options.SetUserGroups(dbauthz.AsSystem(ctx), tx, user.ID, params.Groups)
853859
if err != nil {
854860
return xerrors.Errorf("set user groups: %w", err)
@@ -882,6 +888,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
882888
// In such cases in the current implementation this user can now no
883889
// longer sign in until an administrator finds the offending built-in
884890
// user and changes their username.
891+
//nolint:gocritic
885892
user, err = tx.UpdateUserProfile(dbauthz.AsSystem(ctx), database.UpdateUserProfileParams{
886893
ID: user.ID,
887894
Email: user.Email,
@@ -900,6 +907,7 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
900907
return nil, database.APIKey{}, xerrors.Errorf("in tx: %w", err)
901908
}
902909

910+
//nolint:gocritic
903911
cookie, key, err := api.createAPIKey(dbauthz.AsSystem(ctx), createAPIKeyParams{
904912
UserID: user.ID,
905913
LoginType: params.LoginType,

0 commit comments

Comments
 (0)