Coder's privilege escalation vulnerability could lead to a cross workspace compromise
Description
Coder allows organizations to provision remote development environments via Terraform. In versions 2.22.0 through 2.24.3, 2.25.0 and 2.25.1, Coder can be compromised through insecure session handling in prebuilt workspaces. Coder automatically generates a session token for a user when a workspace is started. It is automatically exposed via coder_workspace_owner.session_token. Prebuilt workspaces are initially owned by a built-in prebuilds system user. When a prebuilt workspace is claimed, a new session token is generated for the user that claimed the workspace, but the previous session token for the prebuilds user was not expired. Any Coder workspace templates that persist this automatically generated session token are potentially impacted. This is fixed in versions 2.24.4 and 2.25.2.
AI Insight
LLM-synthesized narrative grounded in this CVE's description and references.
Coder 2.22.0 through 2.25.1 fails to expire session tokens for the prebuilds system user when a prebuilt workspace is claimed, allowing persistent unauthorized access.
Vulnerability
Overview Coder versions 2.22.0 through 2.24.3, 2.25.0, and 2.25.1 are vulnerable to a session handling flaw related to prebuilt workspaces. When a prebuilt workspace (initially owned by a built-in prebuilds system user) is claimed by a regular user, Coder generates a new session token for the claiming user but fails to expire the original session token belonging to the prebuilds user [2]. This means the pre-existing token remains valid, potentially granting persistent access to the workspace or associated resources through templates that persist the automatically generated session token from coder_workspace_owner.session_token [2].
Attack
Vector and Requirements Exploitation does not require authentication as the vulnerable user; rather, any user who has access to a prebuilt workspace template that stores the session_token can utilize the stale prebuilds user token [2]. The attacker would need the privilege to start or use a prebuilt workspace that saves the token. The vulnerability is inherent in the workspace lifecycle: the token is generated at workspace start and not invalidated on claim, so if a template persists that token, the stale token remains usable [2]. No additional authentication is needed once the token is obtained.
Impact
An attacker who successfully obtains the stale session token can impersonate the prebuilds system user. Depending on the permissions of that built-in user within the organization, this could lead to unauthorized access to workspaces, code, or infrastructure managed by Coder [2]. The severity is reflected in the CVSS 4.0 score of 8.2 (high) [2].
Mitigation
The vulnerability is fixed in versions 2.24.4 and 2.25.2 [2]. The fix ensures that when a new session token is generated for the claiming user, the previous token for the prebuilds user is explicitly expired [1][4]. Administrators should update Coder immediately; there is no viable workaround aside from patching, as the token expiration logic is core to the fix [2].
AI Insight generated on May 19, 2026. Synthesized from this CVE's description and the cited reference URLs; citations are validated against the source bundle.
Affected packages
Versions sourced from the GitHub Security Advisory.
| Package | Affected versions | Patched versions |
|---|---|---|
github.com/coder/coder/v2Go | >= 2.22.0, < 2.24.4 | 2.24.4 |
github.com/coder/coder/v2Go | >= 2.25.0, < 2.25.2 | 2.25.2 |
Affected products
2- coder/coderv5Range: >= 2.22.0, < 2.24.4
Patches
3ec660907faa0fix: expire token for prebuilds user when regenerating session token (#19667) (#19668)
14 files changed · +344 −8
coderd/apikey.go+18 −0 modified@@ -12,6 +12,8 @@ import ( "github.com/moby/moby/pkg/namesgenerator" "golang.org/x/xerrors" + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" @@ -56,6 +58,14 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) { return } + // TODO(Cian): System users technically just have the 'member' role + // and we don't want to disallow all members from creating API keys. + if user.IsSystem { + api.Logger.Warn(ctx, "disallowed creating api key for system user", slog.F("user_id", user.ID)) + httpapi.Forbidden(rw) + return + } + scope := database.APIKeyScopeAll if scope != "" { scope = database.APIKeyScope(createToken.Scope) @@ -124,6 +134,14 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() user := httpmw.UserParam(r) + // TODO(Cian): System users technically just have the 'member' role + // and we don't want to disallow all members from creating API keys. + if user.IsSystem { + api.Logger.Warn(ctx, "disallowed creating api key for system user", slog.F("user_id", user.ID)) + httpapi.Forbidden(rw) + return + } + cookie, _, err := api.createAPIKey(ctx, apikey.CreateParams{ UserID: user.ID, DefaultLifetime: api.DeploymentValues.Sessions.DefaultTokenDuration.Value(),
coderd/apikey_test.go+33 −0 modified@@ -13,8 +13,10 @@ import ( "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" "github.com/coder/serpent" @@ -351,3 +353,34 @@ func TestAPIKey_SetDefault(t *testing.T) { require.NoError(t, err) require.EqualValues(t, dc.Sessions.DefaultTokenDuration.Value().Seconds(), apiKey1.LifetimeSeconds) } + +func TestAPIKey_PrebuildsNotAllowed(t *testing.T) { + t.Parallel() + + db, pubsub := dbtestutil.NewDB(t) + dc := coderdtest.DeploymentValues(t) + dc.Sessions.DefaultTokenDuration = serpent.Duration(time.Hour * 12) + client := coderdtest.New(t, &coderdtest.Options{ + Database: db, + Pubsub: pubsub, + DeploymentValues: dc, + }) + + ctx := testutil.Context(t, testutil.WaitLong) + + // Given: an existing api token for the prebuilds user + _, prebuildsToken := dbgen.APIKey(t, db, database.APIKey{ + UserID: database.PrebuildsSystemUserID, + }) + client.SetSessionToken(prebuildsToken) + + // When: the prebuilds user tries to create an API key + _, err := client.CreateAPIKey(ctx, database.PrebuildsSystemUserID.String()) + // Then: denied. + require.ErrorContains(t, err, httpapi.ResourceForbiddenResponse.Message) + + // When: the prebuilds user tries to create a token + _, err = client.CreateToken(ctx, database.PrebuildsSystemUserID.String(), codersdk.CreateTokenRequest{}) + // Then: also denied. + require.ErrorContains(t, err, httpapi.ResourceForbiddenResponse.Message) +}
coderd/database/dbauthz/dbauthz.go+15 −0 modified@@ -1725,6 +1725,13 @@ func (q *querier) EnqueueNotificationMessage(ctx context.Context, arg database.E return q.db.EnqueueNotificationMessage(ctx, arg) } +func (q *querier) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceApiKey); err != nil { + return err + } + return q.db.ExpirePrebuildsAPIKeys(ctx, now) +} + func (q *querier) FavoriteWorkspace(ctx context.Context, id uuid.UUID) error { fetch := func(ctx context.Context, id uuid.UUID) (database.Workspace, error) { return q.db.GetWorkspaceByID(ctx, id) @@ -3623,6 +3630,14 @@ func (q *querier) HasTemplateVersionsWithAITask(ctx context.Context) (bool, erro } func (q *querier) InsertAPIKey(ctx context.Context, arg database.InsertAPIKeyParams) (database.APIKey, error) { + // TODO(Cian): ideally this would be encoded in the policy, but system users are just members and we + // don't currently have a capability to conditionally deny creating resources by owner ID in a role. + // We also need to enrich rbac.Actor with IsSystem so that we can distinguish all system users. + // For now, there is only one system user (prebuilds). + if act, ok := ActorFromContext(ctx); ok && act.ID == database.PrebuildsSystemUserID.String() { + return database.APIKey{}, logNotAuthorizedError(ctx, q.log, NotAuthorizedError{Err: xerrors.Errorf("prebuild user may not create api keys")}) + } + return insert(q.log, q.auth, rbac.ResourceApiKey.WithOwner(arg.UserID.String()), q.db.InsertAPIKey)(ctx, arg)
coderd/database/dbauthz/dbauthz_test.go+21 −0 modified@@ -14,14 +14,17 @@ import ( "github.com/google/uuid" "github.com/sqlc-dev/pqtype" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" "golang.org/x/xerrors" "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbmock" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/notifications" @@ -1681,6 +1684,9 @@ func (s *MethodTestSuite) TestUser() { u := dbgen.User(s.T(), db, database.User{}) check.Args(u.ID).Asserts(rbac.ResourceApiKey.WithOwner(u.ID.String()), policy.ActionDelete).Returns() })) + s.Run("ExpirePrebuildsAPIKeys", s.Subtest(func(db database.Store, check *expects) { + check.Args(dbtime.Now()).Asserts(rbac.ResourceApiKey, policy.ActionDelete).Returns() + })) s.Run("GetQuotaAllowanceForUser", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) check.Args(database.GetQuotaAllowanceForUserParams{ @@ -5845,3 +5851,18 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() { }).Asserts(w, policy.ActionUpdate, w.AsPrebuild(), policy.ActionUpdate) })) } + +// Ensures that the prebuilds actor may never insert an api key. +func TestInsertAPIKey_AsPrebuildsUser(t *testing.T) { + t.Parallel() + prebuildsSubj := rbac.Subject{ + ID: database.PrebuildsSystemUserID.String(), + } + ctx := dbauthz.As(testutil.Context(t, testutil.WaitShort), prebuildsSubj) + mDB := dbmock.NewMockStore(gomock.NewController(t)) + log := slogtest.Make(t, nil) + mDB.EXPECT().Wrappers().Times(1).Return([]string{}) + dbz := dbauthz.New(mDB, nil, log, nil) + _, err := dbz.InsertAPIKey(ctx, database.InsertAPIKeyParams{}) + require.True(t, dbauthz.IsNotAuthorizedError(err)) +}
coderd/database/dbgen/dbgen.go+7 −3 modified@@ -156,7 +156,7 @@ func Template(t testing.TB, db database.Store, seed database.Template) database. return template } -func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database.APIKey, token string) { +func APIKey(t testing.TB, db database.Store, seed database.APIKey, munge ...func(*database.InsertAPIKeyParams)) (key database.APIKey, token string) { id, _ := cryptorand.String(10) secret, _ := cryptorand.String(22) hashed := sha256.Sum256([]byte(secret)) @@ -172,7 +172,7 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database } } - key, err := db.InsertAPIKey(genCtx, database.InsertAPIKeyParams{ + params := database.InsertAPIKeyParams{ ID: takeFirst(seed.ID, id), // 0 defaults to 86400 at the db layer LifetimeSeconds: takeFirst(seed.LifetimeSeconds, 0), @@ -186,7 +186,11 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database LoginType: takeFirst(seed.LoginType, database.LoginTypePassword), Scope: takeFirst(seed.Scope, database.APIKeyScopeAll), TokenName: takeFirst(seed.TokenName), - }) + } + for _, fn := range munge { + fn(¶ms) + } + key, err := db.InsertAPIKey(genCtx, params) require.NoError(t, err, "insert api key") return key, fmt.Sprintf("%s-%s", key.ID, secret) }
coderd/database/dbmetrics/querymetrics.go+7 −0 modified@@ -509,6 +509,13 @@ func (m queryMetricsStore) EnqueueNotificationMessage(ctx context.Context, arg d return r0 } +func (m queryMetricsStore) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + start := time.Now() + r0 := m.s.ExpirePrebuildsAPIKeys(ctx, now) + m.queryLatencies.WithLabelValues("ExpirePrebuildsAPIKeys").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) FavoriteWorkspace(ctx context.Context, arg uuid.UUID) error { start := time.Now() r0 := m.s.FavoriteWorkspace(ctx, arg)
coderd/database/dbmock/dbmock.go+14 −0 modified@@ -933,6 +933,20 @@ func (mr *MockStoreMockRecorder) EnqueueNotificationMessage(ctx, arg any) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnqueueNotificationMessage", reflect.TypeOf((*MockStore)(nil).EnqueueNotificationMessage), ctx, arg) } +// ExpirePrebuildsAPIKeys mocks base method. +func (m *MockStore) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ExpirePrebuildsAPIKeys", ctx, now) + ret0, _ := ret[0].(error) + return ret0 +} + +// ExpirePrebuildsAPIKeys indicates an expected call of ExpirePrebuildsAPIKeys. +func (mr *MockStoreMockRecorder) ExpirePrebuildsAPIKeys(ctx, now any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ExpirePrebuildsAPIKeys", reflect.TypeOf((*MockStore)(nil).ExpirePrebuildsAPIKeys), ctx, now) +} + // FavoriteWorkspace mocks base method. func (m *MockStore) FavoriteWorkspace(ctx context.Context, id uuid.UUID) error { m.ctrl.T.Helper()
coderd/database/dbpurge/dbpurge.go+3 −0 modified@@ -67,6 +67,9 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, clk quartz. if err := tx.DeleteOldNotificationMessages(ctx); err != nil { return xerrors.Errorf("failed to delete old notification messages: %w", err) } + if err := tx.ExpirePrebuildsAPIKeys(ctx, dbtime.Time(start)); err != nil { + return xerrors.Errorf("failed to expire prebuilds user api keys: %w", err) + } deleteOldAuditLogConnectionEventsBefore := start.Add(-maxAuditLogConnectionEventAge) if err := tx.DeleteOldAuditLogConnectionEvents(ctx, database.DeleteOldAuditLogConnectionEventsParams{
coderd/database/dbpurge/dbpurge_test.go+66 −0 modified@@ -25,6 +25,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbrollup" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisionerd/proto" "github.com/coder/coder/v2/provisionersdk" @@ -635,3 +636,68 @@ func TestDeleteOldAuditLogConnectionEventsLimit(t *testing.T) { require.Len(t, logs, 0) } + +func TestExpireOldAPIKeys(t *testing.T) { + t.Parallel() + + // Given: a number of workspaces and API keys owned by a regular user and the prebuilds system user. + var ( + ctx = testutil.Context(t, testutil.WaitShort) + now = dbtime.Now() + db, _ = dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure()) + org = dbgen.Organization(t, db, database.Organization{}) + user = dbgen.User(t, db, database.User{}) + tpl = dbgen.Template(t, db, database.Template{OrganizationID: org.ID, CreatedBy: user.ID}) + userWs = dbgen.Workspace(t, db, database.WorkspaceTable{ + OwnerID: user.ID, + TemplateID: tpl.ID, + }) + prebuildsWs = dbgen.Workspace(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + TemplateID: tpl.ID, + }) + createAPIKey = func(userID uuid.UUID, name string) database.APIKey { + k, _ := dbgen.APIKey(t, db, database.APIKey{UserID: userID, TokenName: name, ExpiresAt: now.Add(time.Hour)}, func(iap *database.InsertAPIKeyParams) { + iap.TokenName = name + }) + return k + } + assertKeyActive = func(kid string) { + k, err := db.GetAPIKeyByID(ctx, kid) + require.NoError(t, err) + assert.True(t, k.ExpiresAt.After(now)) + } + assertKeyExpired = func(kid string) { + k, err := db.GetAPIKeyByID(ctx, kid) + require.NoError(t, err) + assert.True(t, k.ExpiresAt.Equal(now)) + } + unnamedUserAPIKey = createAPIKey(user.ID, "") + unnamedPrebuildsAPIKey = createAPIKey(database.PrebuildsSystemUserID, "") + namedUserAPIKey = createAPIKey(user.ID, "my-token") + namedPrebuildsAPIKey = createAPIKey(database.PrebuildsSystemUserID, "also-my-token") + userWorkspaceAPIKey1 = createAPIKey(user.ID, provisionerdserver.WorkspaceSessionTokenName(user.ID, userWs.ID)) + userWorkspaceAPIKey2 = createAPIKey(user.ID, provisionerdserver.WorkspaceSessionTokenName(user.ID, prebuildsWs.ID)) + prebuildsWorkspaceAPIKey1 = createAPIKey(database.PrebuildsSystemUserID, provisionerdserver.WorkspaceSessionTokenName(database.PrebuildsSystemUserID, prebuildsWs.ID)) + prebuildsWorkspaceAPIKey2 = createAPIKey(database.PrebuildsSystemUserID, provisionerdserver.WorkspaceSessionTokenName(database.PrebuildsSystemUserID, userWs.ID)) + ) + + // When: we call ExpirePrebuildsAPIKeys + err := db.ExpirePrebuildsAPIKeys(ctx, now) + // Then: no errors is reported. + require.NoError(t, err) + + // We do not touch user API keys. + assertKeyActive(unnamedUserAPIKey.ID) + assertKeyActive(namedUserAPIKey.ID) + assertKeyActive(userWorkspaceAPIKey1.ID) + assertKeyActive(userWorkspaceAPIKey2.ID) + // Unnamed prebuilds API keys get expired. + assertKeyExpired(unnamedPrebuildsAPIKey.ID) + // API keys for workspaces still owned by prebuilds user remain active until claimed. + assertKeyActive(prebuildsWorkspaceAPIKey1.ID) + // API keys for workspaces no longer owned by prebuilds user get expired. + assertKeyExpired(prebuildsWorkspaceAPIKey2.ID) + // Out of an abundance of caution, we do not expire explicitly named prebuilds API keys. + assertKeyActive(namedPrebuildsAPIKey.ID) +}
coderd/database/querier.go+5 −0 modified@@ -128,6 +128,11 @@ type sqlcQuerier interface { // of the test-only in-memory database. Do not use this in new code. DisableForeignKeysAndTriggers(ctx context.Context) error EnqueueNotificationMessage(ctx context.Context, arg EnqueueNotificationMessageParams) error + // Firstly, collect api_keys owned by the prebuilds user that correlate + // to workspaces no longer owned by the prebuilds user. + // Next, collect api_keys that belong to the prebuilds user but have no token name. + // These were most likely created via 'coder login' as the prebuilds user. + ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error FavoriteWorkspace(ctx context.Context, id uuid.UUID) error FetchMemoryResourceMonitorsByAgentID(ctx context.Context, agentID uuid.UUID) (WorkspaceAgentMemoryResourceMonitor, error) FetchMemoryResourceMonitorsUpdatedAfter(ctx context.Context, updatedAt time.Time) ([]WorkspaceAgentMemoryResourceMonitor, error)
coderd/database/queries/apikeys.sql+34 −0 modified@@ -83,3 +83,37 @@ DELETE FROM api_keys WHERE user_id = $1; + +-- name: ExpirePrebuildsAPIKeys :exec +-- Firstly, collect api_keys owned by the prebuilds user that correlate +-- to workspaces no longer owned by the prebuilds user. +WITH unexpired_prebuilds_workspace_session_tokens AS ( + SELECT id, SUBSTRING(token_name FROM 38 FOR 36)::uuid AS workspace_id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND expires_at > @now::timestamptz + AND token_name SIMILAR TO 'c42fdf75-3097-471c-8c33-fb52454d81c0_[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}_session_token' +), +stale_prebuilds_workspace_session_tokens AS ( + SELECT upwst.id + FROM unexpired_prebuilds_workspace_session_tokens upwst + LEFT JOIN workspaces w + ON w.id = upwst.workspace_id + WHERE w.owner_id <> 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid +), +-- Next, collect api_keys that belong to the prebuilds user but have no token name. +-- These were most likely created via 'coder login' as the prebuilds user. +unnamed_prebuilds_api_keys AS ( + SELECT id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND token_name = '' + AND expires_at > @now::timestamptz +) +UPDATE api_keys +SET expires_at = @now::timestamptz +WHERE id IN ( + SELECT id FROM stale_prebuilds_workspace_session_tokens + UNION + SELECT id FROM unnamed_prebuilds_api_keys +);
coderd/database/queries.sql.go+40 −0 modified@@ -144,6 +144,46 @@ func (q *sqlQuerier) DeleteApplicationConnectAPIKeysByUserID(ctx context.Context return err } +const expirePrebuildsAPIKeys = `-- name: ExpirePrebuildsAPIKeys :exec +WITH unexpired_prebuilds_workspace_session_tokens AS ( + SELECT id, SUBSTRING(token_name FROM 38 FOR 36)::uuid AS workspace_id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND expires_at > $1::timestamptz + AND token_name SIMILAR TO 'c42fdf75-3097-471c-8c33-fb52454d81c0_[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}_session_token' +), +stale_prebuilds_workspace_session_tokens AS ( + SELECT upwst.id + FROM unexpired_prebuilds_workspace_session_tokens upwst + LEFT JOIN workspaces w + ON w.id = upwst.workspace_id + WHERE w.owner_id <> 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid +), +unnamed_prebuilds_api_keys AS ( + SELECT id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND token_name = '' + AND expires_at > $1::timestamptz +) +UPDATE api_keys +SET expires_at = $1::timestamptz +WHERE id IN ( + SELECT id FROM stale_prebuilds_workspace_session_tokens + UNION + SELECT id FROM unnamed_prebuilds_api_keys +) +` + +// Firstly, collect api_keys owned by the prebuilds user that correlate +// to workspaces no longer owned by the prebuilds user. +// Next, collect api_keys that belong to the prebuilds user but have no token name. +// These were most likely created via 'coder login' as the prebuilds user. +func (q *sqlQuerier) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + _, err := q.db.ExecContext(ctx, expirePrebuildsAPIKeys, now) + return err +} + const getAPIKeyByID = `-- name: GetAPIKeyByID :one SELECT id, hashed_secret, user_id, last_used, expires_at, created_at, updated_at, login_type, lifetime_seconds, ip_address, scope, token_name
coderd/provisionerdserver/provisionerdserver.go+17 −5 modified@@ -2711,15 +2711,23 @@ func InsertWorkspaceResource(ctx context.Context, db database.Store, jobID uuid. return nil } -func workspaceSessionTokenName(workspace database.Workspace) string { - return fmt.Sprintf("%s_%s_session_token", workspace.OwnerID, workspace.ID) +func WorkspaceSessionTokenName(ownerID, workspaceID uuid.UUID) string { + return fmt.Sprintf("%s_%s_session_token", ownerID, workspaceID) } func (s *server) regenerateSessionToken(ctx context.Context, user database.User, workspace database.Workspace) (string, error) { + // NOTE(Cian): Once a workspace is claimed, there's no reason for the session token to be valid any longer. + // Not generating any session token at all for a system user may unintentionally break existing templates, + // which we want to avoid. If there's no session token for the workspace belonging to the prebuilds user, + // then there's nothing for us to worry about here. + // TODO(Cian): Update this to handle _all_ system users. At the time of writing, only one system user exists. + if err := deleteSessionTokenForUserAndWorkspace(ctx, s.Database, database.PrebuildsSystemUserID, workspace.ID); err != nil && !errors.Is(err, sql.ErrNoRows) { + s.Logger.Error(ctx, "failed to delete prebuilds session token", slog.Error(err), slog.F("workspace_id", workspace.ID)) + } newkey, sessionToken, err := apikey.Generate(apikey.CreateParams{ UserID: user.ID, LoginType: user.LoginType, - TokenName: workspaceSessionTokenName(workspace), + TokenName: WorkspaceSessionTokenName(workspace.OwnerID, workspace.ID), DefaultLifetime: s.DeploymentValues.Sessions.DefaultTokenDuration.Value(), LifetimeSeconds: int64(s.DeploymentValues.Sessions.MaximumTokenDuration.Value().Seconds()), }) @@ -2747,10 +2755,14 @@ func (s *server) regenerateSessionToken(ctx context.Context, user database.User, } func deleteSessionToken(ctx context.Context, db database.Store, workspace database.Workspace) error { + return deleteSessionTokenForUserAndWorkspace(ctx, db, workspace.OwnerID, workspace.ID) +} + +func deleteSessionTokenForUserAndWorkspace(ctx context.Context, db database.Store, userID, workspaceID uuid.UUID) error { err := db.InTx(func(tx database.Store) error { key, err := tx.GetAPIKeyByName(ctx, database.GetAPIKeyByNameParams{ - UserID: workspace.OwnerID, - TokenName: workspaceSessionTokenName(workspace), + UserID: userID, + TokenName: WorkspaceSessionTokenName(userID, workspaceID), }) if err == nil { err = tx.DeleteAPIKeyByID(ctx, key.ID)
coderd/provisionerdserver/provisionerdserver_test.go+64 −0 modified@@ -3576,6 +3576,70 @@ func TestNotifications(t *testing.T) { }) } +func TestServer_ExpirePrebuildsSessionToken(t *testing.T) { + t.Parallel() + + // Given: a prebuilt workspace where an API key was previously created for the prebuilds user. + var ( + ctx = testutil.Context(t, testutil.WaitShort) + srv, db, ps, pd = setup(t, false, nil) + user = dbgen.User(t, db, database.User{}) + template = dbgen.Template(t, db, database.Template{ + OrganizationID: pd.OrganizationID, + CreatedBy: user.ID, + }) + version = dbgen.TemplateVersion(t, db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: template.ID, Valid: true}, + OrganizationID: pd.OrganizationID, + CreatedBy: user.ID, + }) + workspace = dbgen.Workspace(t, db, database.WorkspaceTable{ + OrganizationID: pd.OrganizationID, + TemplateID: template.ID, + OwnerID: database.PrebuildsSystemUserID, + }) + workspaceBuildID = uuid.New() + buildJob = dbgen.ProvisionerJob(t, db, ps, database.ProvisionerJob{ + OrganizationID: pd.OrganizationID, + FileID: dbgen.File(t, db, database.File{CreatedBy: user.ID}).ID, + Type: database.ProvisionerJobTypeWorkspaceBuild, + Input: must(json.Marshal(provisionerdserver.WorkspaceProvisionJob{ + WorkspaceBuildID: workspaceBuildID, + })), + InitiatorID: database.PrebuildsSystemUserID, + Tags: pd.Tags, + }) + _ = dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + ID: workspaceBuildID, + WorkspaceID: workspace.ID, + TemplateVersionID: version.ID, + JobID: buildJob.ID, + Transition: database.WorkspaceTransitionStart, + InitiatorID: database.PrebuildsSystemUserID, + }) + existingKey, _ = dbgen.APIKey(t, db, database.APIKey{ + UserID: database.PrebuildsSystemUserID, + TokenName: provisionerdserver.WorkspaceSessionTokenName(database.PrebuildsSystemUserID, workspace.ID), + }) + ) + + // When: the prebuild claim job is acquired + fs := newFakeStream(ctx) + err := srv.AcquireJobWithCancel(fs) + require.NoError(t, err) + job, err := fs.waitForJob() + require.NoError(t, err) + require.NotNil(t, job) + workspaceBuildJob := job.Type.(*proto.AcquiredJob_WorkspaceBuild_).WorkspaceBuild + require.NotNil(t, workspaceBuildJob.Metadata) + + // Assert test invariant: we acquired the expected build job + require.Equal(t, workspaceBuildID.String(), workspaceBuildJob.WorkspaceBuildId) + // Then: The session token should be deleted + _, err = db.GetAPIKeyByID(ctx, existingKey.ID) + require.ErrorIs(t, err, sql.ErrNoRows, "api key for prebuilds user should be deleted") +} + type overrides struct { ctx context.Context deploymentValues *codersdk.DeploymentValues
20d67d7d7191fix: expire token for prebuilds user when regenerating session token (#19667) (#19669)
15 files changed · +360 −14
coderd/apikey.go+18 −0 modified@@ -12,6 +12,8 @@ import ( "github.com/moby/moby/pkg/namesgenerator" "golang.org/x/xerrors" + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" @@ -56,6 +58,14 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) { return } + // TODO(Cian): System users technically just have the 'member' role + // and we don't want to disallow all members from creating API keys. + if user.IsSystem { + api.Logger.Warn(ctx, "disallowed creating api key for system user", slog.F("user_id", user.ID)) + httpapi.Forbidden(rw) + return + } + scope := database.APIKeyScopeAll if scope != "" { scope = database.APIKeyScope(createToken.Scope) @@ -124,6 +134,14 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() user := httpmw.UserParam(r) + // TODO(Cian): System users technically just have the 'member' role + // and we don't want to disallow all members from creating API keys. + if user.IsSystem { + api.Logger.Warn(ctx, "disallowed creating api key for system user", slog.F("user_id", user.ID)) + httpapi.Forbidden(rw) + return + } + cookie, _, err := api.createAPIKey(ctx, apikey.CreateParams{ UserID: user.ID, DefaultLifetime: api.DeploymentValues.Sessions.DefaultTokenDuration.Value(),
coderd/apikey_test.go+33 −0 modified@@ -13,8 +13,10 @@ import ( "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" "github.com/coder/serpent" @@ -351,3 +353,34 @@ func TestAPIKey_SetDefault(t *testing.T) { require.NoError(t, err) require.EqualValues(t, dc.Sessions.DefaultTokenDuration.Value().Seconds(), apiKey1.LifetimeSeconds) } + +func TestAPIKey_PrebuildsNotAllowed(t *testing.T) { + t.Parallel() + + db, pubsub := dbtestutil.NewDB(t) + dc := coderdtest.DeploymentValues(t) + dc.Sessions.DefaultTokenDuration = serpent.Duration(time.Hour * 12) + client := coderdtest.New(t, &coderdtest.Options{ + Database: db, + Pubsub: pubsub, + DeploymentValues: dc, + }) + + ctx := testutil.Context(t, testutil.WaitLong) + + // Given: an existing api token for the prebuilds user + _, prebuildsToken := dbgen.APIKey(t, db, database.APIKey{ + UserID: database.PrebuildsSystemUserID, + }) + client.SetSessionToken(prebuildsToken) + + // When: the prebuilds user tries to create an API key + _, err := client.CreateAPIKey(ctx, database.PrebuildsSystemUserID.String()) + // Then: denied. + require.ErrorContains(t, err, httpapi.ResourceForbiddenResponse.Message) + + // When: the prebuilds user tries to create a token + _, err = client.CreateToken(ctx, database.PrebuildsSystemUserID.String(), codersdk.CreateTokenRequest{}) + // Then: also denied. + require.ErrorContains(t, err, httpapi.ResourceForbiddenResponse.Message) +}
coderd/database/dbauthz/dbauthz.go+15 −0 modified@@ -1652,6 +1652,13 @@ func (q *querier) EnqueueNotificationMessage(ctx context.Context, arg database.E return q.db.EnqueueNotificationMessage(ctx, arg) } +func (q *querier) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceApiKey); err != nil { + return err + } + return q.db.ExpirePrebuildsAPIKeys(ctx, now) +} + func (q *querier) FavoriteWorkspace(ctx context.Context, id uuid.UUID) error { fetch := func(ctx context.Context, id uuid.UUID) (database.Workspace, error) { return q.db.GetWorkspaceByID(ctx, id) @@ -3507,6 +3514,14 @@ func (q *querier) HasTemplateVersionsWithAITask(ctx context.Context) (bool, erro } func (q *querier) InsertAPIKey(ctx context.Context, arg database.InsertAPIKeyParams) (database.APIKey, error) { + // TODO(Cian): ideally this would be encoded in the policy, but system users are just members and we + // don't currently have a capability to conditionally deny creating resources by owner ID in a role. + // We also need to enrich rbac.Actor with IsSystem so that we can distinguish all system users. + // For now, there is only one system user (prebuilds). + if act, ok := ActorFromContext(ctx); ok && act.ID == database.PrebuildsSystemUserID.String() { + return database.APIKey{}, logNotAuthorizedError(ctx, q.log, NotAuthorizedError{Err: xerrors.Errorf("prebuild user may not create api keys")}) + } + return insert(q.log, q.auth, rbac.ResourceApiKey.WithOwner(arg.UserID.String()), q.db.InsertAPIKey)(ctx, arg)
coderd/database/dbauthz/dbauthz_test.go+26 −6 modified@@ -14,24 +14,26 @@ import ( "github.com/google/uuid" "github.com/sqlc-dev/pqtype" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" "golang.org/x/xerrors" "cdr.dev/slog" - - "github.com/coder/coder/v2/coderd/database/db2sdk" - "github.com/coder/coder/v2/coderd/database/dbmem" - "github.com/coder/coder/v2/coderd/notifications" - "github.com/coder/coder/v2/coderd/rbac/policy" - "github.com/coder/coder/v2/codersdk" + "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbmem" + "github.com/coder/coder/v2/coderd/database/dbmock" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/coderd/util/slice" + "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisionersdk" "github.com/coder/coder/v2/testutil" ) @@ -1573,6 +1575,9 @@ func (s *MethodTestSuite) TestUser() { UserID: u.ID, OrganizationID: uuid.New(), }).Asserts(u, policy.ActionRead).Returns(int64(0)) + s.Run("ExpirePrebuildsAPIKeys", s.Subtest(func(db database.Store, check *expects) { + check.Args(dbtime.Now()).Asserts(rbac.ResourceApiKey, policy.ActionDelete).Returns() + })) })) s.Run("GetQuotaConsumedForUser", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) @@ -5637,3 +5642,18 @@ func (s *MethodTestSuite) TestAuthorizePrebuiltWorkspace() { }).Asserts(w, policy.ActionUpdate, w.AsPrebuild(), policy.ActionUpdate) })) } + +// Ensures that the prebuilds actor may never insert an api key. +func TestInsertAPIKey_AsPrebuildsUser(t *testing.T) { + t.Parallel() + prebuildsSubj := rbac.Subject{ + ID: database.PrebuildsSystemUserID.String(), + } + ctx := dbauthz.As(testutil.Context(t, testutil.WaitShort), prebuildsSubj) + mDB := dbmock.NewMockStore(gomock.NewController(t)) + log := slogtest.Make(t, nil) + mDB.EXPECT().Wrappers().Times(1).Return([]string{}) + dbz := dbauthz.New(mDB, nil, log, nil) + _, err := dbz.InsertAPIKey(ctx, database.InsertAPIKeyParams{}) + require.True(t, dbauthz.IsNotAuthorizedError(err)) +}
coderd/database/dbgen/dbgen.go+7 −3 modified@@ -108,7 +108,7 @@ func Template(t testing.TB, db database.Store, seed database.Template) database. return template } -func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database.APIKey, token string) { +func APIKey(t testing.TB, db database.Store, seed database.APIKey, munge ...func(*database.InsertAPIKeyParams)) (key database.APIKey, token string) { id, _ := cryptorand.String(10) secret, _ := cryptorand.String(22) hashed := sha256.Sum256([]byte(secret)) @@ -124,7 +124,7 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database } } - key, err := db.InsertAPIKey(genCtx, database.InsertAPIKeyParams{ + params := database.InsertAPIKeyParams{ ID: takeFirst(seed.ID, id), // 0 defaults to 86400 at the db layer LifetimeSeconds: takeFirst(seed.LifetimeSeconds, 0), @@ -138,7 +138,11 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database LoginType: takeFirst(seed.LoginType, database.LoginTypePassword), Scope: takeFirst(seed.Scope, database.APIKeyScopeAll), TokenName: takeFirst(seed.TokenName), - }) + } + for _, fn := range munge { + fn(¶ms) + } + key, err := db.InsertAPIKey(genCtx, params) require.NoError(t, err, "insert api key") return key, fmt.Sprintf("%s-%s", key.ID, secret) }
coderd/database/dbmem/dbmem.go+5 −0 modified@@ -2597,6 +2597,11 @@ func (q *FakeQuerier) EnqueueNotificationMessage(_ context.Context, arg database return err } +func (*FakeQuerier) ExpirePrebuildsAPIKeys(_ context.Context, _ time.Time) error { + // Implemented in postgres. + return nil +} + func (q *FakeQuerier) FavoriteWorkspace(_ context.Context, arg uuid.UUID) error { err := validateDatabaseType(arg) if err != nil {
coderd/database/dbmetrics/querymetrics.go+7 −0 modified@@ -487,6 +487,13 @@ func (m queryMetricsStore) EnqueueNotificationMessage(ctx context.Context, arg d return r0 } +func (m queryMetricsStore) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + start := time.Now() + r0 := m.s.ExpirePrebuildsAPIKeys(ctx, now) + m.queryLatencies.WithLabelValues("ExpirePrebuildsAPIKeys").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) FavoriteWorkspace(ctx context.Context, arg uuid.UUID) error { start := time.Now() r0 := m.s.FavoriteWorkspace(ctx, arg)
coderd/database/dbmock/dbmock.go+14 −0 modified@@ -874,6 +874,20 @@ func (mr *MockStoreMockRecorder) EnqueueNotificationMessage(ctx, arg any) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnqueueNotificationMessage", reflect.TypeOf((*MockStore)(nil).EnqueueNotificationMessage), ctx, arg) } +// ExpirePrebuildsAPIKeys mocks base method. +func (m *MockStore) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ExpirePrebuildsAPIKeys", ctx, now) + ret0, _ := ret[0].(error) + return ret0 +} + +// ExpirePrebuildsAPIKeys indicates an expected call of ExpirePrebuildsAPIKeys. +func (mr *MockStoreMockRecorder) ExpirePrebuildsAPIKeys(ctx, now any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ExpirePrebuildsAPIKeys", reflect.TypeOf((*MockStore)(nil).ExpirePrebuildsAPIKeys), ctx, now) +} + // FavoriteWorkspace mocks base method. func (m *MockStore) FavoriteWorkspace(ctx context.Context, id uuid.UUID) error { m.ctrl.T.Helper()
coderd/database/dbpurge/dbpurge.go+3 −0 modified@@ -62,6 +62,9 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, clk quartz. if err := tx.DeleteOldNotificationMessages(ctx); err != nil { return xerrors.Errorf("failed to delete old notification messages: %w", err) } + if err := tx.ExpirePrebuildsAPIKeys(ctx, dbtime.Time(start)); err != nil { + return xerrors.Errorf("failed to expire prebuilds user api keys: %w", err) + } logger.Debug(ctx, "purged old database entries", slog.F("duration", clk.Since(start)))
coderd/database/dbpurge/dbpurge_test.go+72 −0 modified@@ -25,6 +25,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbrollup" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisionerd/proto" "github.com/coder/coder/v2/provisionersdk" @@ -40,6 +41,9 @@ func TestMain(m *testing.M) { // //nolint:paralleltest // It uses LockIDDBPurge. func TestPurge(t *testing.T) { + if !dbtestutil.WillUsePostgres() { + t.Skip("requires postgres") + } ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) defer cancel() @@ -490,3 +494,71 @@ func containsProvisionerDaemon(daemons []database.ProvisionerDaemon, name string return d.Name == name }) } + +func TestExpireOldAPIKeys(t *testing.T) { + t.Parallel() + if !dbtestutil.WillUsePostgres() { + t.Skip("only implemented in postgres") + } + + // Given: a number of workspaces and API keys owned by a regular user and the prebuilds system user. + var ( + ctx = testutil.Context(t, testutil.WaitShort) + now = dbtime.Now() + db, _ = dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure()) + org = dbgen.Organization(t, db, database.Organization{}) + user = dbgen.User(t, db, database.User{}) + tpl = dbgen.Template(t, db, database.Template{OrganizationID: org.ID, CreatedBy: user.ID}) + userWs = dbgen.Workspace(t, db, database.WorkspaceTable{ + OwnerID: user.ID, + TemplateID: tpl.ID, + }) + prebuildsWs = dbgen.Workspace(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + TemplateID: tpl.ID, + }) + createAPIKey = func(userID uuid.UUID, name string) database.APIKey { + k, _ := dbgen.APIKey(t, db, database.APIKey{UserID: userID, TokenName: name, ExpiresAt: now.Add(time.Hour)}, func(iap *database.InsertAPIKeyParams) { + iap.TokenName = name + }) + return k + } + assertKeyActive = func(kid string) { + k, err := db.GetAPIKeyByID(ctx, kid) + require.NoError(t, err) + assert.True(t, k.ExpiresAt.After(now)) + } + assertKeyExpired = func(kid string) { + k, err := db.GetAPIKeyByID(ctx, kid) + require.NoError(t, err) + assert.True(t, k.ExpiresAt.Equal(now)) + } + unnamedUserAPIKey = createAPIKey(user.ID, "") + unnamedPrebuildsAPIKey = createAPIKey(database.PrebuildsSystemUserID, "") + namedUserAPIKey = createAPIKey(user.ID, "my-token") + namedPrebuildsAPIKey = createAPIKey(database.PrebuildsSystemUserID, "also-my-token") + userWorkspaceAPIKey1 = createAPIKey(user.ID, provisionerdserver.WorkspaceSessionTokenName(user.ID, userWs.ID)) + userWorkspaceAPIKey2 = createAPIKey(user.ID, provisionerdserver.WorkspaceSessionTokenName(user.ID, prebuildsWs.ID)) + prebuildsWorkspaceAPIKey1 = createAPIKey(database.PrebuildsSystemUserID, provisionerdserver.WorkspaceSessionTokenName(database.PrebuildsSystemUserID, prebuildsWs.ID)) + prebuildsWorkspaceAPIKey2 = createAPIKey(database.PrebuildsSystemUserID, provisionerdserver.WorkspaceSessionTokenName(database.PrebuildsSystemUserID, userWs.ID)) + ) + + // When: we call ExpirePrebuildsAPIKeys + err := db.ExpirePrebuildsAPIKeys(ctx, now) + // Then: no errors is reported. + require.NoError(t, err) + + // We do not touch user API keys. + assertKeyActive(unnamedUserAPIKey.ID) + assertKeyActive(namedUserAPIKey.ID) + assertKeyActive(userWorkspaceAPIKey1.ID) + assertKeyActive(userWorkspaceAPIKey2.ID) + // Unnamed prebuilds API keys get expired. + assertKeyExpired(unnamedPrebuildsAPIKey.ID) + // API keys for workspaces still owned by prebuilds user remain active until claimed. + assertKeyActive(prebuildsWorkspaceAPIKey1.ID) + // API keys for workspaces no longer owned by prebuilds user get expired. + assertKeyExpired(prebuildsWorkspaceAPIKey2.ID) + // Out of an abundance of caution, we do not expire explicitly named prebuilds API keys. + assertKeyActive(namedPrebuildsAPIKey.ID) +}
coderd/database/querier.go+5 −0 modified@@ -124,6 +124,11 @@ type sqlcQuerier interface { // of the test-only in-memory database. Do not use this in new code. DisableForeignKeysAndTriggers(ctx context.Context) error EnqueueNotificationMessage(ctx context.Context, arg EnqueueNotificationMessageParams) error + // Firstly, collect api_keys owned by the prebuilds user that correlate + // to workspaces no longer owned by the prebuilds user. + // Next, collect api_keys that belong to the prebuilds user but have no token name. + // These were most likely created via 'coder login' as the prebuilds user. + ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error FavoriteWorkspace(ctx context.Context, id uuid.UUID) error FetchMemoryResourceMonitorsByAgentID(ctx context.Context, agentID uuid.UUID) (WorkspaceAgentMemoryResourceMonitor, error) FetchMemoryResourceMonitorsUpdatedAfter(ctx context.Context, updatedAt time.Time) ([]WorkspaceAgentMemoryResourceMonitor, error)
coderd/database/queries/apikeys.sql+34 −0 modified@@ -83,3 +83,37 @@ DELETE FROM api_keys WHERE user_id = $1; + +-- name: ExpirePrebuildsAPIKeys :exec +-- Firstly, collect api_keys owned by the prebuilds user that correlate +-- to workspaces no longer owned by the prebuilds user. +WITH unexpired_prebuilds_workspace_session_tokens AS ( + SELECT id, SUBSTRING(token_name FROM 38 FOR 36)::uuid AS workspace_id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND expires_at > @now::timestamptz + AND token_name SIMILAR TO 'c42fdf75-3097-471c-8c33-fb52454d81c0_[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}_session_token' +), +stale_prebuilds_workspace_session_tokens AS ( + SELECT upwst.id + FROM unexpired_prebuilds_workspace_session_tokens upwst + LEFT JOIN workspaces w + ON w.id = upwst.workspace_id + WHERE w.owner_id <> 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid +), +-- Next, collect api_keys that belong to the prebuilds user but have no token name. +-- These were most likely created via 'coder login' as the prebuilds user. +unnamed_prebuilds_api_keys AS ( + SELECT id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND token_name = '' + AND expires_at > @now::timestamptz +) +UPDATE api_keys +SET expires_at = @now::timestamptz +WHERE id IN ( + SELECT id FROM stale_prebuilds_workspace_session_tokens + UNION + SELECT id FROM unnamed_prebuilds_api_keys +);
coderd/database/queries.sql.go+40 −0 modified@@ -144,6 +144,46 @@ func (q *sqlQuerier) DeleteApplicationConnectAPIKeysByUserID(ctx context.Context return err } +const expirePrebuildsAPIKeys = `-- name: ExpirePrebuildsAPIKeys :exec +WITH unexpired_prebuilds_workspace_session_tokens AS ( + SELECT id, SUBSTRING(token_name FROM 38 FOR 36)::uuid AS workspace_id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND expires_at > $1::timestamptz + AND token_name SIMILAR TO 'c42fdf75-3097-471c-8c33-fb52454d81c0_[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}_session_token' +), +stale_prebuilds_workspace_session_tokens AS ( + SELECT upwst.id + FROM unexpired_prebuilds_workspace_session_tokens upwst + LEFT JOIN workspaces w + ON w.id = upwst.workspace_id + WHERE w.owner_id <> 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid +), +unnamed_prebuilds_api_keys AS ( + SELECT id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND token_name = '' + AND expires_at > $1::timestamptz +) +UPDATE api_keys +SET expires_at = $1::timestamptz +WHERE id IN ( + SELECT id FROM stale_prebuilds_workspace_session_tokens + UNION + SELECT id FROM unnamed_prebuilds_api_keys +) +` + +// Firstly, collect api_keys owned by the prebuilds user that correlate +// to workspaces no longer owned by the prebuilds user. +// Next, collect api_keys that belong to the prebuilds user but have no token name. +// These were most likely created via 'coder login' as the prebuilds user. +func (q *sqlQuerier) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + _, err := q.db.ExecContext(ctx, expirePrebuildsAPIKeys, now) + return err +} + const getAPIKeyByID = `-- name: GetAPIKeyByID :one SELECT id, hashed_secret, user_id, last_used, expires_at, created_at, updated_at, login_type, lifetime_seconds, ip_address, scope, token_name
coderd/provisionerdserver/provisionerdserver.go+17 −5 modified@@ -2708,15 +2708,23 @@ func InsertWorkspaceResource(ctx context.Context, db database.Store, jobID uuid. return nil } -func workspaceSessionTokenName(workspace database.Workspace) string { - return fmt.Sprintf("%s_%s_session_token", workspace.OwnerID, workspace.ID) +func WorkspaceSessionTokenName(ownerID, workspaceID uuid.UUID) string { + return fmt.Sprintf("%s_%s_session_token", ownerID, workspaceID) } func (s *server) regenerateSessionToken(ctx context.Context, user database.User, workspace database.Workspace) (string, error) { + // NOTE(Cian): Once a workspace is claimed, there's no reason for the session token to be valid any longer. + // Not generating any session token at all for a system user may unintentionally break existing templates, + // which we want to avoid. If there's no session token for the workspace belonging to the prebuilds user, + // then there's nothing for us to worry about here. + // TODO(Cian): Update this to handle _all_ system users. At the time of writing, only one system user exists. + if err := deleteSessionTokenForUserAndWorkspace(ctx, s.Database, database.PrebuildsSystemUserID, workspace.ID); err != nil && !errors.Is(err, sql.ErrNoRows) { + s.Logger.Error(ctx, "failed to delete prebuilds session token", slog.Error(err), slog.F("workspace_id", workspace.ID)) + } newkey, sessionToken, err := apikey.Generate(apikey.CreateParams{ UserID: user.ID, LoginType: user.LoginType, - TokenName: workspaceSessionTokenName(workspace), + TokenName: WorkspaceSessionTokenName(workspace.OwnerID, workspace.ID), DefaultLifetime: s.DeploymentValues.Sessions.DefaultTokenDuration.Value(), LifetimeSeconds: int64(s.DeploymentValues.Sessions.MaximumTokenDuration.Value().Seconds()), }) @@ -2744,10 +2752,14 @@ func (s *server) regenerateSessionToken(ctx context.Context, user database.User, } func deleteSessionToken(ctx context.Context, db database.Store, workspace database.Workspace) error { + return deleteSessionTokenForUserAndWorkspace(ctx, db, workspace.OwnerID, workspace.ID) +} + +func deleteSessionTokenForUserAndWorkspace(ctx context.Context, db database.Store, userID, workspaceID uuid.UUID) error { err := db.InTx(func(tx database.Store) error { key, err := tx.GetAPIKeyByName(ctx, database.GetAPIKeyByNameParams{ - UserID: workspace.OwnerID, - TokenName: workspaceSessionTokenName(workspace), + UserID: userID, + TokenName: WorkspaceSessionTokenName(userID, workspaceID), }) if err == nil { err = tx.DeleteAPIKeyByID(ctx, key.ID)
coderd/provisionerdserver/provisionerdserver_test.go+64 −0 modified@@ -3576,6 +3576,70 @@ func TestNotifications(t *testing.T) { }) } +func TestServer_ExpirePrebuildsSessionToken(t *testing.T) { + t.Parallel() + + // Given: a prebuilt workspace where an API key was previously created for the prebuilds user. + var ( + ctx = testutil.Context(t, testutil.WaitShort) + srv, db, ps, pd = setup(t, false, nil) + user = dbgen.User(t, db, database.User{}) + template = dbgen.Template(t, db, database.Template{ + OrganizationID: pd.OrganizationID, + CreatedBy: user.ID, + }) + version = dbgen.TemplateVersion(t, db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: template.ID, Valid: true}, + OrganizationID: pd.OrganizationID, + CreatedBy: user.ID, + }) + workspace = dbgen.Workspace(t, db, database.WorkspaceTable{ + OrganizationID: pd.OrganizationID, + TemplateID: template.ID, + OwnerID: database.PrebuildsSystemUserID, + }) + workspaceBuildID = uuid.New() + buildJob = dbgen.ProvisionerJob(t, db, ps, database.ProvisionerJob{ + OrganizationID: pd.OrganizationID, + FileID: dbgen.File(t, db, database.File{CreatedBy: user.ID}).ID, + Type: database.ProvisionerJobTypeWorkspaceBuild, + Input: must(json.Marshal(provisionerdserver.WorkspaceProvisionJob{ + WorkspaceBuildID: workspaceBuildID, + })), + InitiatorID: database.PrebuildsSystemUserID, + Tags: pd.Tags, + }) + _ = dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + ID: workspaceBuildID, + WorkspaceID: workspace.ID, + TemplateVersionID: version.ID, + JobID: buildJob.ID, + Transition: database.WorkspaceTransitionStart, + InitiatorID: database.PrebuildsSystemUserID, + }) + existingKey, _ = dbgen.APIKey(t, db, database.APIKey{ + UserID: database.PrebuildsSystemUserID, + TokenName: provisionerdserver.WorkspaceSessionTokenName(database.PrebuildsSystemUserID, workspace.ID), + }) + ) + + // When: the prebuild claim job is acquired + fs := newFakeStream(ctx) + err := srv.AcquireJobWithCancel(fs) + require.NoError(t, err) + job, err := fs.waitForJob() + require.NoError(t, err) + require.NotNil(t, job) + workspaceBuildJob := job.Type.(*proto.AcquiredJob_WorkspaceBuild_).WorkspaceBuild + require.NotNil(t, workspaceBuildJob.Metadata) + + // Assert test invariant: we acquired the expected build job + require.Equal(t, workspaceBuildID.String(), workspaceBuildJob.WorkspaceBuildId) + // Then: The session token should be deleted + _, err = db.GetAPIKeyByID(ctx, existingKey.ID) + require.ErrorIs(t, err, sql.ErrNoRows, "api key for prebuilds user should be deleted") +} + type overrides struct { ctx context.Context deploymentValues *codersdk.DeploymentValues
06cbb2890f45fix: expire token for prebuilds user when regenerating session token (#19667)
14 files changed · +344 −8
coderd/apikey.go+18 −0 modified@@ -12,6 +12,8 @@ import ( "github.com/moby/moby/pkg/namesgenerator" "golang.org/x/xerrors" + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/apikey" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" @@ -56,6 +58,14 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) { return } + // TODO(Cian): System users technically just have the 'member' role + // and we don't want to disallow all members from creating API keys. + if user.IsSystem { + api.Logger.Warn(ctx, "disallowed creating api key for system user", slog.F("user_id", user.ID)) + httpapi.Forbidden(rw) + return + } + scope := database.APIKeyScopeAll if scope != "" { scope = database.APIKeyScope(createToken.Scope) @@ -124,6 +134,14 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() user := httpmw.UserParam(r) + // TODO(Cian): System users technically just have the 'member' role + // and we don't want to disallow all members from creating API keys. + if user.IsSystem { + api.Logger.Warn(ctx, "disallowed creating api key for system user", slog.F("user_id", user.ID)) + httpapi.Forbidden(rw) + return + } + cookie, _, err := api.createAPIKey(ctx, apikey.CreateParams{ UserID: user.ID, DefaultLifetime: api.DeploymentValues.Sessions.DefaultTokenDuration.Value(),
coderd/apikey_test.go+33 −0 modified@@ -13,8 +13,10 @@ import ( "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" "github.com/coder/serpent" @@ -351,3 +353,34 @@ func TestAPIKey_SetDefault(t *testing.T) { require.NoError(t, err) require.EqualValues(t, dc.Sessions.DefaultTokenDuration.Value().Seconds(), apiKey1.LifetimeSeconds) } + +func TestAPIKey_PrebuildsNotAllowed(t *testing.T) { + t.Parallel() + + db, pubsub := dbtestutil.NewDB(t) + dc := coderdtest.DeploymentValues(t) + dc.Sessions.DefaultTokenDuration = serpent.Duration(time.Hour * 12) + client := coderdtest.New(t, &coderdtest.Options{ + Database: db, + Pubsub: pubsub, + DeploymentValues: dc, + }) + + ctx := testutil.Context(t, testutil.WaitLong) + + // Given: an existing api token for the prebuilds user + _, prebuildsToken := dbgen.APIKey(t, db, database.APIKey{ + UserID: database.PrebuildsSystemUserID, + }) + client.SetSessionToken(prebuildsToken) + + // When: the prebuilds user tries to create an API key + _, err := client.CreateAPIKey(ctx, database.PrebuildsSystemUserID.String()) + // Then: denied. + require.ErrorContains(t, err, httpapi.ResourceForbiddenResponse.Message) + + // When: the prebuilds user tries to create a token + _, err = client.CreateToken(ctx, database.PrebuildsSystemUserID.String(), codersdk.CreateTokenRequest{}) + // Then: also denied. + require.ErrorContains(t, err, httpapi.ResourceForbiddenResponse.Message) +}
coderd/database/dbauthz/dbauthz.go+15 −0 modified@@ -1787,6 +1787,13 @@ func (q *querier) EnqueueNotificationMessage(ctx context.Context, arg database.E return q.db.EnqueueNotificationMessage(ctx, arg) } +func (q *querier) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceApiKey); err != nil { + return err + } + return q.db.ExpirePrebuildsAPIKeys(ctx, now) +} + func (q *querier) FavoriteWorkspace(ctx context.Context, id uuid.UUID) error { fetch := func(ctx context.Context, id uuid.UUID) (database.Workspace, error) { return q.db.GetWorkspaceByID(ctx, id) @@ -3727,6 +3734,14 @@ func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context, now ti } func (q *querier) InsertAPIKey(ctx context.Context, arg database.InsertAPIKeyParams) (database.APIKey, error) { + // TODO(Cian): ideally this would be encoded in the policy, but system users are just members and we + // don't currently have a capability to conditionally deny creating resources by owner ID in a role. + // We also need to enrich rbac.Actor with IsSystem so that we can distinguish all system users. + // For now, there is only one system user (prebuilds). + if act, ok := ActorFromContext(ctx); ok && act.ID == database.PrebuildsSystemUserID.String() { + return database.APIKey{}, logNotAuthorizedError(ctx, q.log, NotAuthorizedError{Err: xerrors.Errorf("prebuild user may not create api keys")}) + } + return insert(q.log, q.auth, rbac.ResourceApiKey.WithOwner(arg.UserID.String()), q.db.InsertAPIKey)(ctx, arg)
coderd/database/dbauthz/dbauthz_test.go+21 −0 modified@@ -18,6 +18,7 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" @@ -1297,6 +1298,10 @@ func (s *MethodTestSuite) TestUser() { dbm.EXPECT().DeleteAPIKeysByUserID(gomock.Any(), key.UserID).Return(nil).AnyTimes() check.Args(key.UserID).Asserts(rbac.ResourceApiKey.WithOwner(key.UserID.String()), policy.ActionDelete).Returns() })) + s.Run("ExpirePrebuildsAPIKeys", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + dbm.EXPECT().ExpirePrebuildsAPIKeys(gomock.Any(), gomock.Any()).Times(1).Return(nil) + check.Args(dbtime.Now()).Asserts(rbac.ResourceApiKey, policy.ActionDelete).Returns() + })) s.Run("GetQuotaAllowanceForUser", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { u := testutil.Fake(s.T(), faker, database.User{}) arg := database.GetQuotaAllowanceForUserParams{UserID: u.ID, OrganizationID: uuid.New()} @@ -4287,3 +4292,19 @@ func (s *MethodTestSuite) TestUsageEvents() { }).Asserts(rbac.ResourceUsageEvent, policy.ActionRead) })) } + +// Ensures that the prebuilds actor may never insert an api key. +func TestInsertAPIKey_AsPrebuildsUser(t *testing.T) { + t.Parallel() + prebuildsSubj := rbac.Subject{ + ID: database.PrebuildsSystemUserID.String(), + } + ctx := dbauthz.As(testutil.Context(t, testutil.WaitShort), prebuildsSubj) + mDB := dbmock.NewMockStore(gomock.NewController(t)) + log := slogtest.Make(t, nil) + mDB.EXPECT().Wrappers().Times(1).Return([]string{}) + dbz := dbauthz.New(mDB, nil, log, nil) + faker := gofakeit.New(0) + _, err := dbz.InsertAPIKey(ctx, testutil.Fake(t, faker, database.InsertAPIKeyParams{})) + require.True(t, dbauthz.IsNotAuthorizedError(err)) +}
coderd/database/dbgen/dbgen.go+7 −3 modified@@ -157,7 +157,7 @@ func Template(t testing.TB, db database.Store, seed database.Template) database. return template } -func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database.APIKey, token string) { +func APIKey(t testing.TB, db database.Store, seed database.APIKey, munge ...func(*database.InsertAPIKeyParams)) (key database.APIKey, token string) { id, _ := cryptorand.String(10) secret, _ := cryptorand.String(22) hashed := sha256.Sum256([]byte(secret)) @@ -173,7 +173,7 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database } } - key, err := db.InsertAPIKey(genCtx, database.InsertAPIKeyParams{ + params := database.InsertAPIKeyParams{ ID: takeFirst(seed.ID, id), // 0 defaults to 86400 at the db layer LifetimeSeconds: takeFirst(seed.LifetimeSeconds, 0), @@ -187,7 +187,11 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database LoginType: takeFirst(seed.LoginType, database.LoginTypePassword), Scope: takeFirst(seed.Scope, database.APIKeyScopeAll), TokenName: takeFirst(seed.TokenName), - }) + } + for _, fn := range munge { + fn(¶ms) + } + key, err := db.InsertAPIKey(genCtx, params) require.NoError(t, err, "insert api key") return key, fmt.Sprintf("%s-%s", key.ID, secret) }
coderd/database/dbmetrics/querymetrics.go+7 −0 modified@@ -523,6 +523,13 @@ func (m queryMetricsStore) EnqueueNotificationMessage(ctx context.Context, arg d return r0 } +func (m queryMetricsStore) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + start := time.Now() + r0 := m.s.ExpirePrebuildsAPIKeys(ctx, now) + m.queryLatencies.WithLabelValues("ExpirePrebuildsAPIKeys").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) FavoriteWorkspace(ctx context.Context, arg uuid.UUID) error { start := time.Now() r0 := m.s.FavoriteWorkspace(ctx, arg)
coderd/database/dbmock/dbmock.go+14 −0 modified@@ -962,6 +962,20 @@ func (mr *MockStoreMockRecorder) EnqueueNotificationMessage(ctx, arg any) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnqueueNotificationMessage", reflect.TypeOf((*MockStore)(nil).EnqueueNotificationMessage), ctx, arg) } +// ExpirePrebuildsAPIKeys mocks base method. +func (m *MockStore) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ExpirePrebuildsAPIKeys", ctx, now) + ret0, _ := ret[0].(error) + return ret0 +} + +// ExpirePrebuildsAPIKeys indicates an expected call of ExpirePrebuildsAPIKeys. +func (mr *MockStoreMockRecorder) ExpirePrebuildsAPIKeys(ctx, now any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ExpirePrebuildsAPIKeys", reflect.TypeOf((*MockStore)(nil).ExpirePrebuildsAPIKeys), ctx, now) +} + // FavoriteWorkspace mocks base method. func (m *MockStore) FavoriteWorkspace(ctx context.Context, id uuid.UUID) error { m.ctrl.T.Helper()
coderd/database/dbpurge/dbpurge.go+3 −0 modified@@ -68,6 +68,9 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, clk quartz. if err := tx.DeleteOldNotificationMessages(ctx); err != nil { return xerrors.Errorf("failed to delete old notification messages: %w", err) } + if err := tx.ExpirePrebuildsAPIKeys(ctx, dbtime.Time(start)); err != nil { + return xerrors.Errorf("failed to expire prebuilds user api keys: %w", err) + } deleteOldAuditLogConnectionEventsBefore := start.Add(-maxAuditLogConnectionEventAge) if err := tx.DeleteOldAuditLogConnectionEvents(ctx, database.DeleteOldAuditLogConnectionEventsParams{
coderd/database/dbpurge/dbpurge_test.go+66 −0 modified@@ -27,6 +27,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbrollup" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisionerd/proto" "github.com/coder/coder/v2/provisionersdk" @@ -638,3 +639,68 @@ func TestDeleteOldAuditLogConnectionEventsLimit(t *testing.T) { require.Len(t, logs, 0) } + +func TestExpireOldAPIKeys(t *testing.T) { + t.Parallel() + + // Given: a number of workspaces and API keys owned by a regular user and the prebuilds system user. + var ( + ctx = testutil.Context(t, testutil.WaitShort) + now = dbtime.Now() + db, _ = dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure()) + org = dbgen.Organization(t, db, database.Organization{}) + user = dbgen.User(t, db, database.User{}) + tpl = dbgen.Template(t, db, database.Template{OrganizationID: org.ID, CreatedBy: user.ID}) + userWs = dbgen.Workspace(t, db, database.WorkspaceTable{ + OwnerID: user.ID, + TemplateID: tpl.ID, + }) + prebuildsWs = dbgen.Workspace(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + TemplateID: tpl.ID, + }) + createAPIKey = func(userID uuid.UUID, name string) database.APIKey { + k, _ := dbgen.APIKey(t, db, database.APIKey{UserID: userID, TokenName: name, ExpiresAt: now.Add(time.Hour)}, func(iap *database.InsertAPIKeyParams) { + iap.TokenName = name + }) + return k + } + assertKeyActive = func(kid string) { + k, err := db.GetAPIKeyByID(ctx, kid) + require.NoError(t, err) + assert.True(t, k.ExpiresAt.After(now)) + } + assertKeyExpired = func(kid string) { + k, err := db.GetAPIKeyByID(ctx, kid) + require.NoError(t, err) + assert.True(t, k.ExpiresAt.Equal(now)) + } + unnamedUserAPIKey = createAPIKey(user.ID, "") + unnamedPrebuildsAPIKey = createAPIKey(database.PrebuildsSystemUserID, "") + namedUserAPIKey = createAPIKey(user.ID, "my-token") + namedPrebuildsAPIKey = createAPIKey(database.PrebuildsSystemUserID, "also-my-token") + userWorkspaceAPIKey1 = createAPIKey(user.ID, provisionerdserver.WorkspaceSessionTokenName(user.ID, userWs.ID)) + userWorkspaceAPIKey2 = createAPIKey(user.ID, provisionerdserver.WorkspaceSessionTokenName(user.ID, prebuildsWs.ID)) + prebuildsWorkspaceAPIKey1 = createAPIKey(database.PrebuildsSystemUserID, provisionerdserver.WorkspaceSessionTokenName(database.PrebuildsSystemUserID, prebuildsWs.ID)) + prebuildsWorkspaceAPIKey2 = createAPIKey(database.PrebuildsSystemUserID, provisionerdserver.WorkspaceSessionTokenName(database.PrebuildsSystemUserID, userWs.ID)) + ) + + // When: we call ExpirePrebuildsAPIKeys + err := db.ExpirePrebuildsAPIKeys(ctx, now) + // Then: no errors is reported. + require.NoError(t, err) + + // We do not touch user API keys. + assertKeyActive(unnamedUserAPIKey.ID) + assertKeyActive(namedUserAPIKey.ID) + assertKeyActive(userWorkspaceAPIKey1.ID) + assertKeyActive(userWorkspaceAPIKey2.ID) + // Unnamed prebuilds API keys get expired. + assertKeyExpired(unnamedPrebuildsAPIKey.ID) + // API keys for workspaces still owned by prebuilds user remain active until claimed. + assertKeyActive(prebuildsWorkspaceAPIKey1.ID) + // API keys for workspaces no longer owned by prebuilds user get expired. + assertKeyExpired(prebuildsWorkspaceAPIKey2.ID) + // Out of an abundance of caution, we do not expire explicitly named prebuilds API keys. + assertKeyActive(namedPrebuildsAPIKey.ID) +}
coderd/database/querier.go+5 −0 modified@@ -130,6 +130,11 @@ type sqlcQuerier interface { // of the test-only in-memory database. Do not use this in new code. DisableForeignKeysAndTriggers(ctx context.Context) error EnqueueNotificationMessage(ctx context.Context, arg EnqueueNotificationMessageParams) error + // Firstly, collect api_keys owned by the prebuilds user that correlate + // to workspaces no longer owned by the prebuilds user. + // Next, collect api_keys that belong to the prebuilds user but have no token name. + // These were most likely created via 'coder login' as the prebuilds user. + ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error FavoriteWorkspace(ctx context.Context, id uuid.UUID) error FetchMemoryResourceMonitorsByAgentID(ctx context.Context, agentID uuid.UUID) (WorkspaceAgentMemoryResourceMonitor, error) FetchMemoryResourceMonitorsUpdatedAfter(ctx context.Context, updatedAt time.Time) ([]WorkspaceAgentMemoryResourceMonitor, error)
coderd/database/queries/apikeys.sql+34 −0 modified@@ -83,3 +83,37 @@ DELETE FROM api_keys WHERE user_id = $1; + +-- name: ExpirePrebuildsAPIKeys :exec +-- Firstly, collect api_keys owned by the prebuilds user that correlate +-- to workspaces no longer owned by the prebuilds user. +WITH unexpired_prebuilds_workspace_session_tokens AS ( + SELECT id, SUBSTRING(token_name FROM 38 FOR 36)::uuid AS workspace_id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND expires_at > @now::timestamptz + AND token_name SIMILAR TO 'c42fdf75-3097-471c-8c33-fb52454d81c0_[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}_session_token' +), +stale_prebuilds_workspace_session_tokens AS ( + SELECT upwst.id + FROM unexpired_prebuilds_workspace_session_tokens upwst + LEFT JOIN workspaces w + ON w.id = upwst.workspace_id + WHERE w.owner_id <> 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid +), +-- Next, collect api_keys that belong to the prebuilds user but have no token name. +-- These were most likely created via 'coder login' as the prebuilds user. +unnamed_prebuilds_api_keys AS ( + SELECT id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND token_name = '' + AND expires_at > @now::timestamptz +) +UPDATE api_keys +SET expires_at = @now::timestamptz +WHERE id IN ( + SELECT id FROM stale_prebuilds_workspace_session_tokens + UNION + SELECT id FROM unnamed_prebuilds_api_keys +);
coderd/database/queries.sql.go+40 −0 modified@@ -148,6 +148,46 @@ func (q *sqlQuerier) DeleteApplicationConnectAPIKeysByUserID(ctx context.Context return err } +const expirePrebuildsAPIKeys = `-- name: ExpirePrebuildsAPIKeys :exec +WITH unexpired_prebuilds_workspace_session_tokens AS ( + SELECT id, SUBSTRING(token_name FROM 38 FOR 36)::uuid AS workspace_id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND expires_at > $1::timestamptz + AND token_name SIMILAR TO 'c42fdf75-3097-471c-8c33-fb52454d81c0_[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}_session_token' +), +stale_prebuilds_workspace_session_tokens AS ( + SELECT upwst.id + FROM unexpired_prebuilds_workspace_session_tokens upwst + LEFT JOIN workspaces w + ON w.id = upwst.workspace_id + WHERE w.owner_id <> 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid +), +unnamed_prebuilds_api_keys AS ( + SELECT id + FROM api_keys + WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0'::uuid + AND token_name = '' + AND expires_at > $1::timestamptz +) +UPDATE api_keys +SET expires_at = $1::timestamptz +WHERE id IN ( + SELECT id FROM stale_prebuilds_workspace_session_tokens + UNION + SELECT id FROM unnamed_prebuilds_api_keys +) +` + +// Firstly, collect api_keys owned by the prebuilds user that correlate +// to workspaces no longer owned by the prebuilds user. +// Next, collect api_keys that belong to the prebuilds user but have no token name. +// These were most likely created via 'coder login' as the prebuilds user. +func (q *sqlQuerier) ExpirePrebuildsAPIKeys(ctx context.Context, now time.Time) error { + _, err := q.db.ExecContext(ctx, expirePrebuildsAPIKeys, now) + return err +} + const getAPIKeyByID = `-- name: GetAPIKeyByID :one SELECT id, hashed_secret, user_id, last_used, expires_at, created_at, updated_at, login_type, lifetime_seconds, ip_address, scope, token_name
coderd/provisionerdserver/provisionerdserver.go+17 −5 modified@@ -2955,15 +2955,23 @@ func InsertWorkspaceResource(ctx context.Context, db database.Store, jobID uuid. return nil } -func workspaceSessionTokenName(workspace database.Workspace) string { - return fmt.Sprintf("%s_%s_session_token", workspace.OwnerID, workspace.ID) +func WorkspaceSessionTokenName(ownerID, workspaceID uuid.UUID) string { + return fmt.Sprintf("%s_%s_session_token", ownerID, workspaceID) } func (s *server) regenerateSessionToken(ctx context.Context, user database.User, workspace database.Workspace) (string, error) { + // NOTE(Cian): Once a workspace is claimed, there's no reason for the session token to be valid any longer. + // Not generating any session token at all for a system user may unintentionally break existing templates, + // which we want to avoid. If there's no session token for the workspace belonging to the prebuilds user, + // then there's nothing for us to worry about here. + // TODO(Cian): Update this to handle _all_ system users. At the time of writing, only one system user exists. + if err := deleteSessionTokenForUserAndWorkspace(ctx, s.Database, database.PrebuildsSystemUserID, workspace.ID); err != nil && !errors.Is(err, sql.ErrNoRows) { + s.Logger.Error(ctx, "failed to delete prebuilds session token", slog.Error(err), slog.F("workspace_id", workspace.ID)) + } newkey, sessionToken, err := apikey.Generate(apikey.CreateParams{ UserID: user.ID, LoginType: user.LoginType, - TokenName: workspaceSessionTokenName(workspace), + TokenName: WorkspaceSessionTokenName(workspace.OwnerID, workspace.ID), DefaultLifetime: s.DeploymentValues.Sessions.DefaultTokenDuration.Value(), LifetimeSeconds: int64(s.DeploymentValues.Sessions.MaximumTokenDuration.Value().Seconds()), }) @@ -2991,10 +2999,14 @@ func (s *server) regenerateSessionToken(ctx context.Context, user database.User, } func deleteSessionToken(ctx context.Context, db database.Store, workspace database.Workspace) error { + return deleteSessionTokenForUserAndWorkspace(ctx, db, workspace.OwnerID, workspace.ID) +} + +func deleteSessionTokenForUserAndWorkspace(ctx context.Context, db database.Store, userID, workspaceID uuid.UUID) error { err := db.InTx(func(tx database.Store) error { key, err := tx.GetAPIKeyByName(ctx, database.GetAPIKeyByNameParams{ - UserID: workspace.OwnerID, - TokenName: workspaceSessionTokenName(workspace), + UserID: userID, + TokenName: WorkspaceSessionTokenName(userID, workspaceID), }) if err == nil { err = tx.DeleteAPIKeyByID(ctx, key.ID)
coderd/provisionerdserver/provisionerdserver_test.go+64 −0 modified@@ -3999,6 +3999,70 @@ func TestNotifications(t *testing.T) { }) } +func TestServer_ExpirePrebuildsSessionToken(t *testing.T) { + t.Parallel() + + // Given: a prebuilt workspace where an API key was previously created for the prebuilds user. + var ( + ctx = testutil.Context(t, testutil.WaitShort) + srv, db, ps, pd = setup(t, false, nil) + user = dbgen.User(t, db, database.User{}) + template = dbgen.Template(t, db, database.Template{ + OrganizationID: pd.OrganizationID, + CreatedBy: user.ID, + }) + version = dbgen.TemplateVersion(t, db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: template.ID, Valid: true}, + OrganizationID: pd.OrganizationID, + CreatedBy: user.ID, + }) + workspace = dbgen.Workspace(t, db, database.WorkspaceTable{ + OrganizationID: pd.OrganizationID, + TemplateID: template.ID, + OwnerID: database.PrebuildsSystemUserID, + }) + workspaceBuildID = uuid.New() + buildJob = dbgen.ProvisionerJob(t, db, ps, database.ProvisionerJob{ + OrganizationID: pd.OrganizationID, + FileID: dbgen.File(t, db, database.File{CreatedBy: user.ID}).ID, + Type: database.ProvisionerJobTypeWorkspaceBuild, + Input: must(json.Marshal(provisionerdserver.WorkspaceProvisionJob{ + WorkspaceBuildID: workspaceBuildID, + })), + InitiatorID: database.PrebuildsSystemUserID, + Tags: pd.Tags, + }) + _ = dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + ID: workspaceBuildID, + WorkspaceID: workspace.ID, + TemplateVersionID: version.ID, + JobID: buildJob.ID, + Transition: database.WorkspaceTransitionStart, + InitiatorID: database.PrebuildsSystemUserID, + }) + existingKey, _ = dbgen.APIKey(t, db, database.APIKey{ + UserID: database.PrebuildsSystemUserID, + TokenName: provisionerdserver.WorkspaceSessionTokenName(database.PrebuildsSystemUserID, workspace.ID), + }) + ) + + // When: the prebuild claim job is acquired + fs := newFakeStream(ctx) + err := srv.AcquireJobWithCancel(fs) + require.NoError(t, err) + job, err := fs.waitForJob() + require.NoError(t, err) + require.NotNil(t, job) + workspaceBuildJob := job.Type.(*proto.AcquiredJob_WorkspaceBuild_).WorkspaceBuild + require.NotNil(t, workspaceBuildJob.Metadata) + + // Assert test invariant: we acquired the expected build job + require.Equal(t, workspaceBuildID.String(), workspaceBuildJob.WorkspaceBuildId) + // Then: The session token should be deleted + _, err = db.GetAPIKeyByID(ctx, existingKey.ID) + require.ErrorIs(t, err, sql.ErrNoRows, "api key for prebuilds user should be deleted") +} + type overrides struct { ctx context.Context deploymentValues *codersdk.DeploymentValues
Vulnerability mechanics
Generated on May 9, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.
References
9- github.com/advisories/GHSA-j6xf-jwrj-v5qpghsaADVISORY
- nvd.nist.gov/vuln/detail/CVE-2025-58437ghsaADVISORY
- github.com/coder/coder/commit/06cbb2890f453cd522bb2158a6549afa3419c276ghsax_refsource_MISCWEB
- github.com/coder/coder/commit/20d67d7d7191a4fd5d36a61c6fc1e23ab59befc0ghsax_refsource_MISCWEB
- github.com/coder/coder/commit/ec660907faa0b0eae20fa2ba58ce1733f5f4b35aghsax_refsource_MISCWEB
- github.com/coder/coder/pull/19667ghsax_refsource_MISCWEB
- github.com/coder/coder/pull/19668ghsax_refsource_MISCWEB
- github.com/coder/coder/pull/19669ghsax_refsource_MISCWEB
- github.com/coder/coder/security/advisories/GHSA-j6xf-jwrj-v5qpghsax_refsource_CONFIRMWEB
News mentions
0No linked articles in our index yet.