fix: expire token for prebuilds user when regenerating session token (#19667)

* provisionerdserver: Expires prebuild user token for workspace, if it
exists, when regenerating session token.
* dbauthz: disallow prebuilds user from creating api keys
* dbpurge: added functionality to expire stale api keys owned by the
prebuilds user

coderd/database/dbauthz/dbauthz.go

@@ -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

@@ -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

@@ -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(&params)
+	}
+	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

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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.sql.go

@@ -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/database/queries/apikeys.sql

@@ -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
+);