Skip to content

Commit 0a73f84

Browse files
stirbyjohnstcnmafredrihugodutkakacpersaw
authored
fix: merge cherry-picked items for v2.26.0 (#19678)
Co-authored-by: Cian Johnston <cian@coder.com> Co-authored-by: Mathias Fredriksson <mafredri@gmail.com> Co-authored-by: Hugo Dutka <hugo@coder.com> Co-authored-by: Kacper Sawicki <kacper@coder.com> Co-authored-by: Atif Ali <atif@coder.com> Co-authored-by: Danielle Maywood <danielle@themaywoods.com> Co-authored-by: Susana Ferreira <susana@coder.com> Co-authored-by: Brett Kolodny <brettkolodny@gmail.com> Co-authored-by: Dean Sheather <dean@deansheather.com> Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>
1 parent 8083d9d commit 0a73f84

Some content is hidden

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

51 files changed

+2407
-271
lines changed

‎.github/dependabot.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ updates:
3333
- dependency-name: "*"
3434
update-types:
3535
- version-update:semver-patch
36+
- dependency-name: "github.com/mark3labs/mcp-go"
3637

3738
# Update our Dockerfile.
3839
- package-ecosystem: "docker"

‎cli/server.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,6 @@ import (
6262
"github.com/coder/serpent"
6363
"github.com/coder/wgtunnel/tunnelsdk"
6464

65-
"github.com/coder/coder/v2/coderd/entitlements"
66-
"github.com/coder/coder/v2/coderd/notifications/reports"
67-
"github.com/coder/coder/v2/coderd/runtimeconfig"
68-
"github.com/coder/coder/v2/coderd/webpush"
69-
"github.com/coder/coder/v2/codersdk/drpcsdk"
70-
7165
"github.com/coder/coder/v2/buildinfo"
7266
"github.com/coder/coder/v2/cli/clilog"
7367
"github.com/coder/coder/v2/cli/cliui"
@@ -83,25 +77,31 @@ import (
8377
"github.com/coder/coder/v2/coderd/database/migrations"
8478
"github.com/coder/coder/v2/coderd/database/pubsub"
8579
"github.com/coder/coder/v2/coderd/devtunnel"
80+
"github.com/coder/coder/v2/coderd/entitlements"
8681
"github.com/coder/coder/v2/coderd/externalauth"
8782
"github.com/coder/coder/v2/coderd/gitsshkey"
8883
"github.com/coder/coder/v2/coderd/httpmw"
8984
"github.com/coder/coder/v2/coderd/jobreaper"
9085
"github.com/coder/coder/v2/coderd/notifications"
86+
"github.com/coder/coder/v2/coderd/notifications/reports"
9187
"github.com/coder/coder/v2/coderd/oauthpki"
9288
"github.com/coder/coder/v2/coderd/prometheusmetrics"
9389
"github.com/coder/coder/v2/coderd/prometheusmetrics/insights"
9490
"github.com/coder/coder/v2/coderd/promoauth"
91+
"github.com/coder/coder/v2/coderd/provisionerdserver"
92+
"github.com/coder/coder/v2/coderd/runtimeconfig"
9593
"github.com/coder/coder/v2/coderd/schedule"
9694
"github.com/coder/coder/v2/coderd/telemetry"
9795
"github.com/coder/coder/v2/coderd/tracing"
9896
"github.com/coder/coder/v2/coderd/updatecheck"
9997
"github.com/coder/coder/v2/coderd/util/ptr"
10098
"github.com/coder/coder/v2/coderd/util/slice"
10199
stringutil "github.com/coder/coder/v2/coderd/util/strings"
100+
"github.com/coder/coder/v2/coderd/webpush"
102101
"github.com/coder/coder/v2/coderd/workspaceapps/appurl"
103102
"github.com/coder/coder/v2/coderd/workspacestats"
104103
"github.com/coder/coder/v2/codersdk"
104+
"github.com/coder/coder/v2/codersdk/drpcsdk"
105105
"github.com/coder/coder/v2/cryptorand"
106106
"github.com/coder/coder/v2/provisioner/echo"
107107
"github.com/coder/coder/v2/provisioner/terraform"
@@ -280,6 +280,12 @@ func enablePrometheus(
280280
}
281281
}
282282

283+
provisionerdserverMetrics := provisionerdserver.NewMetrics(logger)
284+
if err := provisionerdserverMetrics.Register(options.PrometheusRegistry); err != nil {
285+
return nil, xerrors.Errorf("failed to register provisionerd_server metrics: %w", err)
286+
}
287+
options.ProvisionerdServerMetrics = provisionerdserverMetrics
288+
283289
//nolint:revive
284290
return ServeHandler(
285291
ctx, logger, promhttp.InstrumentMetricHandler(

‎coderd/aitasks.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
"github.com/go-chi/chi/v5"
1313
"github.com/google/uuid"
14-
"golang.org/x/xerrors"
1514

1615
"cdr.dev/slog"
1716

@@ -196,13 +195,6 @@ func (api *API) tasksCreate(rw http.ResponseWriter, r *http.Request) {
196195
// prompts and mapping status/state. This method enforces that only AI task
197196
// workspaces are given.
198197
func (api *API) tasksFromWorkspaces(ctx context.Context, apiWorkspaces []codersdk.Workspace) ([]codersdk.Task, error) {
199-
// Enforce that only AI task workspaces are given.
200-
for _, ws := range apiWorkspaces {
201-
if ws.LatestBuild.HasAITask == nil || !*ws.LatestBuild.HasAITask {
202-
return nil, xerrors.Errorf("workspace %s is not an AI task workspace", ws.ID)
203-
}
204-
}
205-
206198
// Fetch prompts for each workspace build and map by build ID.
207199
buildIDs := make([]uuid.UUID, 0, len(apiWorkspaces))
208200
for _, ws := range apiWorkspaces {

‎coderd/apikey.go

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
"github.com/moby/moby/pkg/namesgenerator"
1313
"golang.org/x/xerrors"
1414

15+
"cdr.dev/slog"
16+
1517
"github.com/coder/coder/v2/coderd/apikey"
1618
"github.com/coder/coder/v2/coderd/audit"
1719
"github.com/coder/coder/v2/coderd/database"
@@ -56,6 +58,14 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) {
5658
return
5759
}
5860

61+
// TODO(Cian): System users technically just have the 'member' role
62+
// and we don't want to disallow all members from creating API keys.
63+
if user.IsSystem {
64+
api.Logger.Warn(ctx, "disallowed creating api key for system user", slog.F("user_id", user.ID))
65+
httpapi.Forbidden(rw)
66+
return
67+
}
68+
5969
scope := database.APIKeyScopeAll
6070
if scope != "" {
6171
scope = database.APIKeyScope(createToken.Scope)
@@ -121,10 +131,29 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) {
121131
// @Success 201 {object} codersdk.GenerateAPIKeyResponse
122132
// @Router /users/{user}/keys [post]
123133
func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) {
124-
ctx := r.Context()
125-
user := httpmw.UserParam(r)
134+
var (
135+
ctx = r.Context()
136+
user = httpmw.UserParam(r)
137+
auditor = api.Auditor.Load()
138+
aReq, commitAudit = audit.InitRequest[database.APIKey](rw, &audit.RequestParams{
139+
Audit: *auditor,
140+
Log: api.Logger,
141+
Request: r,
142+
Action: database.AuditActionCreate,
143+
})
144+
)
145+
aReq.Old = database.APIKey{}
146+
defer commitAudit()
126147

127-
cookie, _, err := api.createAPIKey(ctx, apikey.CreateParams{
148+
// TODO(Cian): System users technically just have the 'member' role
149+
// and we don't want to disallow all members from creating API keys.
150+
if user.IsSystem {
151+
api.Logger.Warn(ctx, "disallowed creating api key for system user", slog.F("user_id", user.ID))
152+
httpapi.Forbidden(rw)
153+
return
154+
}
155+
156+
cookie, key, err := api.createAPIKey(ctx, apikey.CreateParams{
128157
UserID: user.ID,
129158
DefaultLifetime: api.DeploymentValues.Sessions.DefaultTokenDuration.Value(),
130159
LoginType: database.LoginTypePassword,
@@ -138,6 +167,7 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) {
138167
return
139168
}
140169

170+
aReq.New = *key
141171
// We intentionally do not set the cookie on the response here.
142172
// Setting the cookie will couple the browser session to the API
143173
// key we return here, meaning logging out of the website would

‎coderd/apikey_test.go

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package coderd_test
22

33
import (
44
"context"
5+
"encoding/json"
56
"net/http"
67
"strings"
78
"testing"
@@ -13,8 +14,10 @@ import (
1314
"github.com/coder/coder/v2/coderd/audit"
1415
"github.com/coder/coder/v2/coderd/coderdtest"
1516
"github.com/coder/coder/v2/coderd/database"
17+
"github.com/coder/coder/v2/coderd/database/dbgen"
1618
"github.com/coder/coder/v2/coderd/database/dbtestutil"
1719
"github.com/coder/coder/v2/coderd/database/dbtime"
20+
"github.com/coder/coder/v2/coderd/httpapi"
1821
"github.com/coder/coder/v2/codersdk"
1922
"github.com/coder/coder/v2/testutil"
2023
"github.com/coder/serpent"
@@ -301,14 +304,32 @@ func TestSessionExpiry(t *testing.T) {
301304

302305
func TestAPIKey_OK(t *testing.T) {
303306
t.Parallel()
307+
308+
// Given: a deployment with auditing enabled
304309
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
305310
defer cancel()
306-
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
307-
_ = coderdtest.CreateFirstUser(t, client)
311+
auditor := audit.NewMock()
312+
client := coderdtest.New(t, &coderdtest.Options{Auditor: auditor})
313+
owner := coderdtest.CreateFirstUser(t, client)
314+
auditor.ResetLogs()
308315

316+
// When: an API key is created
309317
res, err := client.CreateAPIKey(ctx, codersdk.Me)
310318
require.NoError(t, err)
311319
require.Greater(t, len(res.Key), 2)
320+
321+
// Then: an audit log is generated
322+
als := auditor.AuditLogs()
323+
require.Len(t, als, 1)
324+
al := als[0]
325+
assert.Equal(t, owner.UserID, al.UserID)
326+
assert.Equal(t, database.AuditActionCreate, al.Action)
327+
assert.Equal(t, database.ResourceTypeApiKey, al.ResourceType)
328+
329+
// Then: the diff MUST NOT contain the generated key.
330+
raw, err := json.Marshal(al)
331+
require.NoError(t, err)
332+
require.NotContains(t, res.Key, string(raw))
312333
}
313334

314335
func TestAPIKey_Deleted(t *testing.T) {
@@ -351,3 +372,34 @@ func TestAPIKey_SetDefault(t *testing.T) {
351372
require.NoError(t, err)
352373
require.EqualValues(t, dc.Sessions.DefaultTokenDuration.Value().Seconds(), apiKey1.LifetimeSeconds)
353374
}
375+
376+
func TestAPIKey_PrebuildsNotAllowed(t *testing.T) {
377+
t.Parallel()
378+
379+
db, pubsub := dbtestutil.NewDB(t)
380+
dc := coderdtest.DeploymentValues(t)
381+
dc.Sessions.DefaultTokenDuration = serpent.Duration(time.Hour * 12)
382+
client := coderdtest.New(t, &coderdtest.Options{
383+
Database: db,
384+
Pubsub: pubsub,
385+
DeploymentValues: dc,
386+
})
387+
388+
ctx := testutil.Context(t, testutil.WaitLong)
389+
390+
// Given: an existing api token for the prebuilds user
391+
_, prebuildsToken := dbgen.APIKey(t, db, database.APIKey{
392+
UserID: database.PrebuildsSystemUserID,
393+
})
394+
client.SetSessionToken(prebuildsToken)
395+
396+
// When: the prebuilds user tries to create an API key
397+
_, err := client.CreateAPIKey(ctx, database.PrebuildsSystemUserID.String())
398+
// Then: denied.
399+
require.ErrorContains(t, err, httpapi.ResourceForbiddenResponse.Message)
400+
401+
// When: the prebuilds user tries to create a token
402+
_, err = client.CreateToken(ctx, database.PrebuildsSystemUserID.String(), codersdk.CreateTokenRequest{})
403+
// Then: also denied.
404+
require.ErrorContains(t, err, httpapi.ResourceForbiddenResponse.Message)
405+
}

‎coderd/coderd.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,8 @@ type Options struct {
241241
UpdateAgentMetrics func(ctx context.Context, labels prometheusmetrics.AgentMetricLabels, metrics []*agentproto.Stats_Metric)
242242
StatsBatcher workspacestats.Batcher
243243

244+
ProvisionerdServerMetrics *provisionerdserver.Metrics
245+
244246
// WorkspaceAppAuditSessionTimeout allows changing the timeout for audit
245247
// sessions. Raising or lowering this value will directly affect the write
246248
// load of the audit log table. This is used for testing. Default 1 hour.
@@ -1930,6 +1932,7 @@ func (api *API) CreateInMemoryTaggedProvisionerDaemon(dialCtx context.Context, n
19301932
},
19311933
api.NotificationsEnqueuer,
19321934
&api.PrebuildsReconciler,
1935+
api.ProvisionerdServerMetrics,
19331936
)
19341937
if err != nil {
19351938
return nil, err

‎coderd/coderdtest/coderdtest.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,8 @@ type Options struct {
184184
OIDCConvertKeyCache cryptokeys.SigningKeycache
185185
Clock quartz.Clock
186186
TelemetryReporter telemetry.Reporter
187+
188+
ProvisionerdServerMetrics *provisionerdserver.Metrics
187189
}
188190

189191
// New constructs a codersdk client connected to an in-memory API instance.
@@ -604,6 +606,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
604606
Clock: options.Clock,
605607
AppEncryptionKeyCache: options.APIKeyEncryptionCache,
606608
OIDCConvertKeyCache: options.OIDCConvertKeyCache,
609+
ProvisionerdServerMetrics: options.ProvisionerdServerMetrics,
607610
}
608611
}
609612

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1777,6 +1777,13 @@ func (q *querier) EnqueueNotificationMessage(ctx context.Context, arg database.E
17771777
return q.db.EnqueueNotificationMessage(ctx, arg)
17781778
}
17791779

1780+
func (q *querier) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error {
1781+
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceApiKey); err != nil {
1782+
return err
1783+
}
1784+
return q.db.ExpirePrebuildsAPIKeys(ctx, now)
1785+
}
1786+
17801787
func (q *querier) FavoriteWorkspace(ctx context.Context, id uuid.UUID) error {
17811788
fetch := func(ctx context.Context, id uuid.UUID) (database.Workspace, error) {
17821789
return q.db.GetWorkspaceByID(ctx, id)
@@ -2242,14 +2249,6 @@ func (q *querier) GetLogoURL(ctx context.Context) (string, error) {
22422249
return q.db.GetLogoURL(ctx)
22432250
}
22442251

2245-
func (q *querier) GetManagedAgentCount(ctx context.Context, arg database.GetManagedAgentCountParams) (int64, error) {
2246-
// Must be able to read all workspaces to check usage.
2247-
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceWorkspace); err != nil {
2248-
return 0, xerrors.Errorf("authorize read all workspaces: %w", err)
2249-
}
2250-
return q.db.GetManagedAgentCount(ctx, arg)
2251-
}
2252-
22532252
func (q *querier) GetNotificationMessagesByStatus(ctx context.Context, arg database.GetNotificationMessagesByStatusParams) ([]database.NotificationMessage, error) {
22542253
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceNotificationMessage); err != nil {
22552254
return nil, err
@@ -2689,6 +2688,13 @@ func (q *querier) GetQuotaConsumedForUser(ctx context.Context, params database.G
26892688
return q.db.GetQuotaConsumedForUser(ctx, params)
26902689
}
26912690

2691+
func (q *querier) GetRegularWorkspaceCreateMetrics(ctx context.Context) ([]database.GetRegularWorkspaceCreateMetricsRow, error) {
2692+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceWorkspace.All()); err != nil {
2693+
return nil, err
2694+
}
2695+
return q.db.GetRegularWorkspaceCreateMetrics(ctx)
2696+
}
2697+
26922698
func (q *querier) GetReplicaByID(ctx context.Context, id uuid.UUID) (database.Replica, error) {
26932699
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
26942700
return database.Replica{}, err
@@ -3041,6 +3047,13 @@ func (q *querier) GetTemplatesWithFilter(ctx context.Context, arg database.GetTe
30413047
return q.db.GetAuthorizedTemplates(ctx, arg, prep)
30423048
}
30433049

3050+
func (q *querier) GetTotalUsageDCManagedAgentsV1(ctx context.Context, arg database.GetTotalUsageDCManagedAgentsV1Params) (int64, error) {
3051+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceUsageEvent); err != nil {
3052+
return 0, err
3053+
}
3054+
return q.db.GetTotalUsageDCManagedAgentsV1(ctx, arg)
3055+
}
3056+
30443057
func (q *querier) GetUnexpiredLicenses(ctx context.Context) ([]database.License, error) {
30453058
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceLicense); err != nil {
30463059
return nil, err
@@ -3711,6 +3724,14 @@ func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context, now ti
37113724
}
37123725

37133726
func (q *querier) InsertAPIKey(ctx context.Context, arg database.InsertAPIKeyParams) (database.APIKey, error) {
3727+
// TODO(Cian): ideally this would be encoded in the policy, but system users are just members and we
3728+
// don't currently have a capability to conditionally deny creating resources by owner ID in a role.
3729+
// We also need to enrich rbac.Actor with IsSystem so that we can distinguish all system users.
3730+
// For now, there is only one system user (prebuilds).
3731+
if act, ok := ActorFromContext(ctx); ok && act.ID == database.PrebuildsSystemUserID.String() {
3732+
return database.APIKey{}, logNotAuthorizedError(ctx, q.log, NotAuthorizedError{Err: xerrors.Errorf("prebuild user may not create api keys")})
3733+
}
3734+
37143735
return insert(q.log, q.auth,
37153736
rbac.ResourceApiKey.WithOwner(arg.UserID.String()),
37163737
q.db.InsertAPIKey)(ctx, arg)

0 commit comments

Comments
 (0)