Skip to content

Commit 864052c

Browse files
mtojekmtojek
committed
Merge branch 'main' into 5980-manage-temp-variables
2 parents af4adec + 51f17b1 commit 864052c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

69 files changed

+5307
-199
lines changed

‎agent/apphealth_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func TestAppHealth_NotSpamming(t *testing.T) {
158158
// Ensure we haven't made more than 2 (expected 1 + 1 for buffer) requests in the last second.
159159
// if there is a bug where we are spamming the healthcheck route this will catch it.
160160
time.Sleep(time.Second)
161-
require.LessOrEqual(t, *counter, int32(2))
161+
require.LessOrEqual(t, atomic.LoadInt32(counter), int32(2))
162162
}
163163

164164
func setupAppReporter(ctx context.Context, t *testing.T, apps []codersdk.WorkspaceApp, handlers []http.Handler) (agent.WorkspaceAgentApps, func()) {

‎coderd/activitybump.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ import (
1515

1616
// activityBumpWorkspace automatically bumps the workspace's auto-off timer
1717
// if it is set to expire soon.
18-
func activityBumpWorkspace(log slog.Logger, db database.Store, workspaceID uuid.UUID) {
18+
func activityBumpWorkspace(ctx context.Context, log slog.Logger, db database.Store, workspaceID uuid.UUID) {
1919
// We set a short timeout so if the app is under load, these
2020
// low priority operations fail first.
21-
ctx, cancel := context.WithTimeout(context.Background(), time.Second*15)
21+
ctx, cancel := context.WithTimeout(ctx, time.Second*15)
2222
defer cancel()
2323

2424
err := db.InTx(func(s database.Store) error {
@@ -82,9 +82,12 @@ func activityBumpWorkspace(log slog.Logger, db database.Store, workspaceID uuid.
8282
return nil
8383
}, nil)
8484
if err != nil {
85-
log.Error(ctx, "bump failed", slog.Error(err),
86-
slog.F("workspace_id", workspaceID),
87-
)
85+
if !xerrors.Is(err, context.Canceled) {
86+
// Bump will fail if the context is cancelled, but this is ok.
87+
log.Error(ctx, "bump failed", slog.Error(err),
88+
slog.F("workspace_id", workspaceID),
89+
)
90+
}
8891
return
8992
}
9093

‎coderd/authorize.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,28 @@ type HTTPAuthorizer struct {
5151
// return
5252
// }
5353
func (api *API) Authorize(r *http.Request, action rbac.Action, object rbac.Objecter) bool {
54+
// The experiment does not replace ALL rbac checks, but does replace most.
55+
// This statement aborts early on the checks that will be removed in the
56+
// future when this experiment is default.
57+
if api.Experiments.Enabled(codersdk.ExperimentAuthzQuerier) {
58+
// Some resource types do not interact with the persistent layer and
59+
// we need to keep these checks happening in the API layer.
60+
switch object.RBACObject().Type {
61+
case rbac.ResourceWorkspaceExecution.Type:
62+
// This is not a db resource, always in API layer
63+
case rbac.ResourceDeploymentConfig.Type:
64+
// For metric cache items like DAU, we do not hit the DB.
65+
// Some db actions are in asserted in the authz layer.
66+
case rbac.ResourceReplicas.Type:
67+
// Replica rbac is checked for adding and removing replicas.
68+
case rbac.ResourceProvisionerDaemon.Type:
69+
// Provisioner rbac is checked for adding and removing provisioners.
70+
case rbac.ResourceDebugInfo.Type:
71+
// This is not a db resource, always in API layer.
72+
default:
73+
return true
74+
}
75+
}
5476
return api.HTTPAuth.Authorize(r, action, object)
5577
}
5678

‎coderd/autobuild/executor/lifecycle_executor.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"cdr.dev/slog"
1313
"github.com/coder/coder/coderd/autobuild/schedule"
1414
"github.com/coder/coder/coderd/database"
15+
"github.com/coder/coder/coderd/database/dbauthz"
1516
"github.com/coder/coder/coderd/provisionerdserver"
1617
)
1718

@@ -34,7 +35,8 @@ type Stats struct {
3435
// New returns a new autobuild executor.
3536
func New(ctx context.Context, db database.Store, log slog.Logger, tick <-chan time.Time) *Executor {
3637
le := &Executor{
37-
ctx: ctx,
38+
//nolint:gocritic // TODO: make an autostart role instead of using System
39+
ctx: dbauthz.AsSystem(ctx),
3840
db: db,
3941
tick: tick,
4042
log: log,

‎coderd/coderd.go

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242
"github.com/coder/coder/coderd/audit"
4343
"github.com/coder/coder/coderd/awsidentity"
4444
"github.com/coder/coder/coderd/database"
45+
"github.com/coder/coder/coderd/database/dbauthz"
4546
"github.com/coder/coder/coderd/database/dbtype"
4647
"github.com/coder/coder/coderd/gitauth"
4748
"github.com/coder/coder/coderd/gitsshkey"
@@ -157,13 +158,6 @@ func New(options *Options) *API {
157158
options = &Options{}
158159
}
159160
experiments := initExperiments(options.Logger, options.DeploymentConfig.Experiments.Value, options.DeploymentConfig.Experimental.Value)
160-
// TODO: remove this once we promote authz_querier out of experiments.
161-
if experiments.Enabled(codersdk.ExperimentAuthzQuerier) {
162-
panic("Coming soon!")
163-
// if _, ok := (options.Database).(*authzquery.AuthzQuerier); !ok {
164-
// options.Database = authzquery.NewAuthzQuerier(options.Database, options.Authorizer)
165-
// }
166-
}
167161
if options.AppHostname != "" && options.AppHostnameRegex == nil || options.AppHostname == "" && options.AppHostnameRegex != nil {
168162
panic("coderd: both AppHostname and AppHostnameRegex must be set or unset")
169163
}
@@ -204,6 +198,14 @@ func New(options *Options) *API {
204198
if options.Auditor == nil {
205199
options.Auditor = audit.NewNop()
206200
}
201+
// TODO: remove this once we promote authz_querier out of experiments.
202+
if experiments.Enabled(codersdk.ExperimentAuthzQuerier) {
203+
options.Database = dbauthz.New(
204+
options.Database,
205+
options.Authorizer,
206+
options.Logger.Named("authz_querier"),
207+
)
208+
}
207209
if options.SetUserGroups == nil {
208210
options.SetUserGroups = func(context.Context, database.Store, uuid.UUID, []string) error { return nil }
209211
}
@@ -304,8 +306,10 @@ func New(options *Options) *API {
304306
DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value,
305307
Optional: true,
306308
}),
307-
httpmw.ExtractUserParam(api.Database, false),
308-
httpmw.ExtractWorkspaceAndAgentParam(api.Database),
309+
httpmw.AsAuthzSystem(
310+
httpmw.ExtractUserParam(api.Database, false),
311+
httpmw.ExtractWorkspaceAndAgentParam(api.Database),
312+
),
309313
),
310314
// Build-Version is helpful for debugging.
311315
func(next http.Handler) http.Handler {
@@ -332,11 +336,13 @@ func New(options *Options) *API {
332336
DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value,
333337
Optional: true,
334338
}),
335-
// Redirect to the login page if the user tries to open an app with
336-
// "me" as the username and they are not logged in.
337-
httpmw.ExtractUserParam(api.Database, true),
338-
// Extracts the <workspace.agent> from the url
339-
httpmw.ExtractWorkspaceAndAgentParam(api.Database),
339+
httpmw.AsAuthzSystem(
340+
// Redirect to the login page if the user tries to open an app with
341+
// "me" as the username and they are not logged in.
342+
httpmw.ExtractUserParam(api.Database, true),
343+
// Extracts the <workspace.agent> from the url
344+
httpmw.ExtractWorkspaceAndAgentParam(api.Database),
345+
),
340346
)
341347
r.HandleFunc("/*", api.workspaceAppsProxyPath)
342348
}

‎coderd/coderdtest/authorize.go

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,16 @@ 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"
24-
"github.com/coder/coder/coderd/database/dbfake"
2525
"github.com/coder/coder/coderd/rbac"
2626
"github.com/coder/coder/coderd/rbac/regosql"
2727
"github.com/coder/coder/codersdk"
@@ -30,12 +30,6 @@ import (
3030
)
3131

3232
func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
33-
// For any route using SQL filters, we need to know if the database is an
34-
// in memory fake. This is because the in memory fake does not use SQL, and
35-
// still uses rego. So this boolean indicates how to assert the expected
36-
// behavior.
37-
_, isMemoryDB := a.api.Database.(dbfake.FakeDatabase)
38-
3933
// Some quick reused objects
4034
workspaceRBACObj := rbac.ResourceWorkspace.WithID(a.Workspace.ID).InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String())
4135
workspaceExecObj := rbac.ResourceWorkspaceExecution.WithID(a.Workspace.ID).InOrg(a.Organization.ID).WithOwner(a.Workspace.OwnerID.String())
@@ -269,16 +263,17 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
269263
"POST:/api/v2/workspaces/{workspace}/builds": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
270264
"POST:/api/v2/organizations/{organization}/templateversions": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
271265

272-
// Endpoints that use the SQLQuery filter.
266+
// For any route using SQL filters, we do not check authorization.
267+
// This is because the in memory fake does not use SQL.
273268
"GET:/api/v2/workspaces/": {
274269
StatusCode: http.StatusOK,
275-
NoAuthorize: !isMemoryDB,
270+
NoAuthorize: true,
276271
AssertAction: rbac.ActionRead,
277272
AssertObject: rbac.ResourceWorkspace,
278273
},
279274
"GET:/api/v2/organizations/{organization}/templates": {
280275
StatusCode: http.StatusOK,
281-
NoAuthorize: !isMemoryDB,
276+
NoAuthorize: true,
282277
AssertAction: rbac.ActionRead,
283278
AssertObject: rbac.ResourceTemplate,
284279
},

‎coderd/coderdtest/authorize_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,21 @@ package coderdtest_test
22

33
import (
44
"context"
5+
"os"
6+
"strings"
57
"testing"
68

79
"github.com/stretchr/testify/require"
810

911
"github.com/coder/coder/coderd/coderdtest"
1012
"github.com/coder/coder/coderd/rbac"
13+
"github.com/coder/coder/codersdk"
1114
)
1215

1316
func TestAuthorizeAllEndpoints(t *testing.T) {
17+
if strings.Contains(os.Getenv("CODER_EXPERIMENTS_TEST"), string(codersdk.ExperimentAuthzQuerier)) {
18+
t.Skip("Skipping TestAuthorizeAllEndpoints for authz_querier experiment")
19+
}
1420
t.Parallel()
1521
client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{
1622
// Required for any subdomain-based proxy tests to pass.

‎coderd/coderdtest/coderdtest.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"github.com/golang-jwt/jwt"
3636
"github.com/google/uuid"
3737
"github.com/moby/moby/pkg/namesgenerator"
38+
"github.com/prometheus/client_golang/prometheus"
3839
"github.com/spf13/afero"
3940
"github.com/spf13/pflag"
4041
"github.com/stretchr/testify/assert"
@@ -58,6 +59,7 @@ import (
5859
"github.com/coder/coder/coderd/autobuild/executor"
5960
"github.com/coder/coder/coderd/awsidentity"
6061
"github.com/coder/coder/coderd/database"
62+
"github.com/coder/coder/coderd/database/dbauthz"
6163
"github.com/coder/coder/coderd/database/dbtestutil"
6264
"github.com/coder/coder/coderd/gitauth"
6365
"github.com/coder/coder/coderd/gitsshkey"
@@ -179,12 +181,13 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can
179181
options.Database, options.Pubsub = dbtestutil.NewDB(t)
180182
}
181183
// TODO: remove this once we're ready to enable authz querier by default.
182-
if strings.Contains(os.Getenv("CODER_EXPERIMENTS_TEST"), "authz_querier") {
183-
panic("Coming soon!")
184-
// if options.Authorizer != nil {
185-
// options.Authorizer = &RecordingAuthorizer{}
186-
// }
187-
// options.Database = authzquery.NewAuthzQuerier(options.Database, options.Authorizer)
184+
if strings.Contains(os.Getenv("CODER_EXPERIMENTS_TEST"), string(codersdk.ExperimentAuthzQuerier)) {
185+
if options.Authorizer == nil {
186+
options.Authorizer = &RecordingAuthorizer{
187+
Wrapped: rbac.NewAuthorizer(prometheus.NewRegistry()),
188+
}
189+
}
190+
options.Database = dbauthz.New(options.Database, options.Authorizer, slogtest.Make(t, nil).Leveled(slog.LevelDebug))
188191
}
189192
if options.DeploymentConfig == nil {
190193
options.DeploymentConfig = DeploymentConfig(t)
@@ -393,13 +396,16 @@ func NewProvisionerDaemon(t *testing.T, coderAPI *coderd.API) io.Closer {
393396
func NewExternalProvisionerDaemon(t *testing.T, client *codersdk.Client, org uuid.UUID, tags map[string]string) io.Closer {
394397
echoClient, echoServer := provisionersdk.MemTransportPipe()
395398
ctx, cancelFunc := context.WithCancel(context.Background())
399+
serveDone := make(chan struct{})
396400
t.Cleanup(func() {
397401
_ = echoClient.Close()
398402
_ = echoServer.Close()
399403
cancelFunc()
404+
<-serveDone
400405
})
401406
fs := afero.NewMemMapFs()
402407
go func() {
408+
defer close(serveDone)
403409
err := echo.Serve(ctx, fs, &provisionersdk.ServeOptions{
404410
Listener: echoServer,
405411
})

0 commit comments

Comments
 (0)