Skip to content

Commit 4c33846

Browse files
dannykoppingSasSwart
dannykopping
and
SasSwart
authored
chore: add prebuilds system user (#16916)
Pre-requisite for #16891 Closes coder/internal#515 This PR introduces a new concept of a "system" user. Our data model requires that all workspaces have an owner (a `users` relation), and prebuilds is a feature that will spin up workspaces to be claimed later by actual users - and thus needs to own the workspaces in the interim. Naturally, introducing a change like this touches a few aspects around the codebase and we've taken the approach _default hidden_ here; in other words, queries for users will by default _exclude_ all system users, but there is a flag to ensure they can be displayed. This keeps the changeset relatively small. This user has minimal permissions (it's equivalent to a `member` since it has no roles). It will be associated with the default org in the initial migration, and thereafter we'll need to somehow ensure its membership aligns with templates (which are org-scoped) for which it'll need to provision prebuilds; that's a solution we'll have in a subsequent PR. --------- Signed-off-by: Danny Kopping <dannykopping@gmail.com> Co-authored-by: Sas Swart <sas.swart.cdk@gmail.com>
1 parent 117e4c2 commit 4c33846

38 files changed

+591
-143
lines changed

‎cli/server.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1894,7 +1894,7 @@ func getGithubOAuth2ConfigParams(ctx context.Context, db database.Store, vals *c
18941894

18951895
if defaultEligibleNotSet {
18961896
// nolint:gocritic // User count requires system privileges
1897-
userCount, err := db.GetUserCount(dbauthz.AsSystemRestricted(ctx))
1897+
userCount, err := db.GetUserCount(dbauthz.AsSystemRestricted(ctx), false)
18981898
if err != nil {
18991899
return nil, xerrors.Errorf("get user count: %w", err)
19001900
}

‎coderd/database/dbauthz/dbauthz.go

+19-14
Original file line numberDiff line numberDiff line change
@@ -1057,13 +1057,13 @@ func (q *querier) ActivityBumpWorkspace(ctx context.Context, arg database.Activi
10571057
return update(q.log, q.auth, fetch, q.db.ActivityBumpWorkspace)(ctx, arg)
10581058
}
10591059

1060-
func (q *querier) AllUserIDs(ctx context.Context) ([]uuid.UUID, error) {
1060+
func (q *querier) AllUserIDs(ctx context.Context, includeSystem bool) ([]uuid.UUID, error) {
10611061
// Although this technically only reads users, only system-related functions should be
10621062
// allowed to call this.
10631063
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
10641064
return nil, err
10651065
}
1066-
return q.db.AllUserIDs(ctx)
1066+
return q.db.AllUserIDs(ctx, includeSystem)
10671067
}
10681068

10691069
func (q *querier) ArchiveUnusedTemplateVersions(ctx context.Context, arg database.ArchiveUnusedTemplateVersionsParams) ([]uuid.UUID, error) {
@@ -1316,7 +1316,11 @@ func (q *querier) DeleteOldWorkspaceAgentStats(ctx context.Context) error {
13161316

13171317
func (q *querier) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error {
13181318
return deleteQ[database.OrganizationMember](q.log, q.auth, func(ctx context.Context, arg database.DeleteOrganizationMemberParams) (database.OrganizationMember, error) {
1319-
member, err := database.ExpectOne(q.OrganizationMembers(ctx, database.OrganizationMembersParams(arg)))
1319+
member, err := database.ExpectOne(q.OrganizationMembers(ctx, database.OrganizationMembersParams{
1320+
OrganizationID: arg.OrganizationID,
1321+
UserID: arg.UserID,
1322+
IncludeSystem: false,
1323+
}))
13201324
if err != nil {
13211325
return database.OrganizationMember{}, err
13221326
}
@@ -1502,11 +1506,11 @@ func (q *querier) GetAPIKeysLastUsedAfter(ctx context.Context, lastUsed time.Tim
15021506
return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetAPIKeysLastUsedAfter)(ctx, lastUsed)
15031507
}
15041508

1505-
func (q *querier) GetActiveUserCount(ctx context.Context) (int64, error) {
1509+
func (q *querier) GetActiveUserCount(ctx context.Context, includeSystem bool) (int64, error) {
15061510
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
15071511
return 0, err
15081512
}
1509-
return q.db.GetActiveUserCount(ctx)
1513+
return q.db.GetActiveUserCount(ctx, includeSystem)
15101514
}
15111515

15121516
func (q *querier) GetActiveWorkspaceBuildsByTemplateID(ctx context.Context, templateID uuid.UUID) ([]database.WorkspaceBuild, error) {
@@ -1737,22 +1741,22 @@ func (q *querier) GetGroupByOrgAndName(ctx context.Context, arg database.GetGrou
17371741
return fetch(q.log, q.auth, q.db.GetGroupByOrgAndName)(ctx, arg)
17381742
}
17391743

1740-
func (q *querier) GetGroupMembers(ctx context.Context) ([]database.GroupMember, error) {
1744+
func (q *querier) GetGroupMembers(ctx context.Context, includeSystem bool) ([]database.GroupMember, error) {
17411745
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
17421746
return nil, err
17431747
}
1744-
return q.db.GetGroupMembers(ctx)
1748+
return q.db.GetGroupMembers(ctx, includeSystem)
17451749
}
17461750

1747-
func (q *querier) GetGroupMembersByGroupID(ctx context.Context, id uuid.UUID) ([]database.GroupMember, error) {
1748-
return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetGroupMembersByGroupID)(ctx, id)
1751+
func (q *querier) GetGroupMembersByGroupID(ctx context.Context, arg database.GetGroupMembersByGroupIDParams) ([]database.GroupMember, error) {
1752+
return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetGroupMembersByGroupID)(ctx, arg)
17491753
}
17501754

1751-
func (q *querier) GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (int64, error) {
1752-
if _, err := q.GetGroupByID(ctx, groupID); err != nil { // AuthZ check
1755+
func (q *querier) GetGroupMembersCountByGroupID(ctx context.Context, arg database.GetGroupMembersCountByGroupIDParams) (int64, error) {
1756+
if _, err := q.GetGroupByID(ctx, arg.GroupID); err != nil { // AuthZ check
17531757
return 0, err
17541758
}
1755-
memberCount, err := q.db.GetGroupMembersCountByGroupID(ctx, groupID)
1759+
memberCount, err := q.db.GetGroupMembersCountByGroupID(ctx, arg)
17561760
if err != nil {
17571761
return 0, err
17581762
}
@@ -2530,11 +2534,11 @@ func (q *querier) GetUserByID(ctx context.Context, id uuid.UUID) (database.User,
25302534
return fetch(q.log, q.auth, q.db.GetUserByID)(ctx, id)
25312535
}
25322536

2533-
func (q *querier) GetUserCount(ctx context.Context) (int64, error) {
2537+
func (q *querier) GetUserCount(ctx context.Context, includeSystem bool) (int64, error) {
25342538
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
25352539
return 0, err
25362540
}
2537-
return q.db.GetUserCount(ctx)
2541+
return q.db.GetUserCount(ctx, includeSystem)
25382542
}
25392543

25402544
func (q *querier) GetUserLatencyInsights(ctx context.Context, arg database.GetUserLatencyInsightsParams) ([]database.GetUserLatencyInsightsRow, error) {
@@ -3778,6 +3782,7 @@ func (q *querier) UpdateMemberRoles(ctx context.Context, arg database.UpdateMemb
37783782
member, err := database.ExpectOne(q.OrganizationMembers(ctx, database.OrganizationMembersParams{
37793783
OrganizationID: arg.OrgID,
37803784
UserID: arg.UserID,
3785+
IncludeSystem: false,
37813786
}))
37823787
if err != nil {
37833788
return database.OrganizationMember{}, err

‎coderd/database/dbauthz/dbauthz_test.go

+12-6
Original file line numberDiff line numberDiff line change
@@ -387,19 +387,25 @@ func (s *MethodTestSuite) TestGroup() {
387387
g := dbgen.Group(s.T(), db, database.Group{})
388388
u := dbgen.User(s.T(), db, database.User{})
389389
gm := dbgen.GroupMember(s.T(), db, database.GroupMemberTable{GroupID: g.ID, UserID: u.ID})
390-
check.Args(g.ID).Asserts(gm, policy.ActionRead)
390+
check.Args(database.GetGroupMembersByGroupIDParams{
391+
GroupID: g.ID,
392+
IncludeSystem: false,
393+
}).Asserts(gm, policy.ActionRead)
391394
}))
392395
s.Run("GetGroupMembersCountByGroupID", s.Subtest(func(db database.Store, check *expects) {
393396
dbtestutil.DisableForeignKeysAndTriggers(s.T(), db)
394397
g := dbgen.Group(s.T(), db, database.Group{})
395-
check.Args(g.ID).Asserts(g, policy.ActionRead)
398+
check.Args(database.GetGroupMembersCountByGroupIDParams{
399+
GroupID: g.ID,
400+
IncludeSystem: false,
401+
}).Asserts(g, policy.ActionRead)
396402
}))
397403
s.Run("GetGroupMembers", s.Subtest(func(db database.Store, check *expects) {
398404
dbtestutil.DisableForeignKeysAndTriggers(s.T(), db)
399405
g := dbgen.Group(s.T(), db, database.Group{})
400406
u := dbgen.User(s.T(), db, database.User{})
401407
dbgen.GroupMember(s.T(), db, database.GroupMemberTable{GroupID: g.ID, UserID: u.ID})
402-
check.Asserts(rbac.ResourceSystem, policy.ActionRead)
408+
check.Args(false).Asserts(rbac.ResourceSystem, policy.ActionRead)
403409
}))
404410
s.Run("System/GetGroups", s.Subtest(func(db database.Store, check *expects) {
405411
dbtestutil.DisableForeignKeysAndTriggers(s.T(), db)
@@ -1681,7 +1687,7 @@ func (s *MethodTestSuite) TestUser() {
16811687
s.Run("AllUserIDs", s.Subtest(func(db database.Store, check *expects) {
16821688
a := dbgen.User(s.T(), db, database.User{})
16831689
b := dbgen.User(s.T(), db, database.User{})
1684-
check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(slice.New(a.ID, b.ID))
1690+
check.Args(false).Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(slice.New(a.ID, b.ID))
16851691
}))
16861692
s.Run("CustomRoles", s.Subtest(func(db database.Store, check *expects) {
16871693
check.Args(database.CustomRolesParams{}).Asserts(rbac.ResourceAssignRole, policy.ActionRead).Returns([]database.CustomRole{})
@@ -3696,7 +3702,7 @@ func (s *MethodTestSuite) TestSystemFunctions() {
36963702
check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead)
36973703
}))
36983704
s.Run("GetActiveUserCount", s.Subtest(func(db database.Store, check *expects) {
3699-
check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(int64(0))
3705+
check.Args(false).Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(int64(0))
37003706
}))
37013707
s.Run("GetUnexpiredLicenses", s.Subtest(func(db database.Store, check *expects) {
37023708
check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead)
@@ -3739,7 +3745,7 @@ func (s *MethodTestSuite) TestSystemFunctions() {
37393745
check.Args(time.Now().Add(time.Hour*-1)).Asserts(rbac.ResourceSystem, policy.ActionRead)
37403746
}))
37413747
s.Run("GetUserCount", s.Subtest(func(db database.Store, check *expects) {
3742-
check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(int64(0))
3748+
check.Args(false).Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(int64(0))
37433749
}))
37443750
s.Run("GetTemplates", s.Subtest(func(db database.Store, check *expects) {
37453751
dbtestutil.DisableForeignKeysAndTriggers(s.T(), db)

‎coderd/database/dbauthz/groupsauth_test.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,10 @@ func TestGroupsAuth(t *testing.T) {
147147
require.Error(t, err, "group read")
148148
}
149149

150-
members, err := db.GetGroupMembersByGroupID(actorCtx, group.ID)
150+
members, err := db.GetGroupMembersByGroupID(actorCtx, database.GetGroupMembersByGroupIDParams{
151+
GroupID: group.ID,
152+
IncludeSystem: false,
153+
})
151154
if tc.ReadMembers {
152155
require.NoError(t, err, "member read")
153156
require.Len(t, members, tc.MembersExpected, "member count found does not match")

‎coderd/database/dbgen/dbgen_test.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,10 @@ func TestGenerator(t *testing.T) {
105105
gm := dbgen.GroupMember(t, db, database.GroupMemberTable{GroupID: g.ID, UserID: u.ID})
106106
exp := []database.GroupMember{gm}
107107

108-
require.Equal(t, exp, must(db.GetGroupMembersByGroupID(context.Background(), g.ID)))
108+
require.Equal(t, exp, must(db.GetGroupMembersByGroupID(context.Background(), database.GetGroupMembersByGroupIDParams{
109+
GroupID: g.ID,
110+
IncludeSystem: false,
111+
})))
109112
})
110113

111114
t.Run("Organization", func(t *testing.T) {

‎coderd/database/dbmem/dbmem.go

+54-11
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"golang.org/x/xerrors"
2424

2525
"github.com/coder/coder/v2/coderd/notifications/types"
26+
"github.com/coder/coder/v2/coderd/prebuilds"
2627

2728
"github.com/coder/coder/v2/coderd/database"
2829
"github.com/coder/coder/v2/coderd/database/dbtime"
@@ -154,6 +155,22 @@ func New() database.Store {
154155
panic(xerrors.Errorf("failed to create psk provisioner key: %w", err))
155156
}
156157

158+
q.mutex.Lock()
159+
// We can't insert this user using the interface, because it's a system user.
160+
q.data.users = append(q.data.users, database.User{
161+
ID: prebuilds.SystemUserID,
162+
Email: "prebuilds@coder.com",
163+
Username: "prebuilds",
164+
CreatedAt: dbtime.Now(),
165+
UpdatedAt: dbtime.Now(),
166+
Status: "active",
167+
LoginType: "none",
168+
HashedPassword: []byte{},
169+
IsSystem: true,
170+
Deleted: false,
171+
})
172+
q.mutex.Unlock()
173+
157174
return q
158175
}
159176

@@ -442,6 +459,7 @@ func convertUsers(users []database.User, count int64) []database.GetUsersRow {
442459
Deleted: u.Deleted,
443460
LastSeenAt: u.LastSeenAt,
444461
Count: count,
462+
IsSystem: u.IsSystem,
445463
}
446464
}
447465

@@ -1554,11 +1572,16 @@ func (q *FakeQuerier) ActivityBumpWorkspace(ctx context.Context, arg database.Ac
15541572
return sql.ErrNoRows
15551573
}
15561574

1557-
func (q *FakeQuerier) AllUserIDs(_ context.Context) ([]uuid.UUID, error) {
1575+
// nolint:revive // It's not a control flag, it's a filter.
1576+
func (q *FakeQuerier) AllUserIDs(_ context.Context, includeSystem bool) ([]uuid.UUID, error) {
15581577
q.mutex.RLock()
15591578
defer q.mutex.RUnlock()
15601579
userIDs := make([]uuid.UUID, 0, len(q.users))
15611580
for idx := range q.users {
1581+
if !includeSystem && q.users[idx].IsSystem {
1582+
continue
1583+
}
1584+
15621585
userIDs = append(userIDs, q.users[idx].ID)
15631586
}
15641587
return userIDs, nil
@@ -2649,12 +2672,17 @@ func (q *FakeQuerier) GetAPIKeysLastUsedAfter(_ context.Context, after time.Time
26492672
return apiKeys, nil
26502673
}
26512674

2652-
func (q *FakeQuerier) GetActiveUserCount(_ context.Context) (int64, error) {
2675+
// nolint:revive // It's not a control flag, it's a filter.
2676+
func (q *FakeQuerier) GetActiveUserCount(_ context.Context, includeSystem bool) (int64, error) {
26532677
q.mutex.RLock()
26542678
defer q.mutex.RUnlock()
26552679

26562680
active := int64(0)
26572681
for _, u := range q.users {
2682+
if !includeSystem && u.IsSystem {
2683+
continue
2684+
}
2685+
26582686
if u.Status == database.UserStatusActive && !u.Deleted {
26592687
active++
26602688
}
@@ -3390,14 +3418,18 @@ func (q *FakeQuerier) GetGroupByOrgAndName(_ context.Context, arg database.GetGr
33903418
return database.Group{}, sql.ErrNoRows
33913419
}
33923420

3393-
func (q *FakeQuerier) GetGroupMembers(ctx context.Context) ([]database.GroupMember, error) {
3421+
//nolint:revive // It's not a control flag, its a filter
3422+
func (q *FakeQuerier) GetGroupMembers(ctx context.Context, includeSystem bool) ([]database.GroupMember, error) {
33943423
q.mutex.RLock()
33953424
defer q.mutex.RUnlock()
33963425

33973426
members := make([]database.GroupMemberTable, 0, len(q.groupMembers))
33983427
members = append(members, q.groupMembers...)
33993428
for _, org := range q.organizations {
34003429
for _, user := range q.users {
3430+
if !includeSystem && user.IsSystem {
3431+
continue
3432+
}
34013433
members = append(members, database.GroupMemberTable{
34023434
UserID: user.ID,
34033435
GroupID: org.ID,
@@ -3420,17 +3452,17 @@ func (q *FakeQuerier) GetGroupMembers(ctx context.Context) ([]database.GroupMemb
34203452
return groupMembers, nil
34213453
}
34223454

3423-
func (q *FakeQuerier) GetGroupMembersByGroupID(ctx context.Context, id uuid.UUID) ([]database.GroupMember, error) {
3455+
func (q *FakeQuerier) GetGroupMembersByGroupID(ctx context.Context, arg database.GetGroupMembersByGroupIDParams) ([]database.GroupMember, error) {
34243456
q.mutex.RLock()
34253457
defer q.mutex.RUnlock()
34263458

3427-
if q.isEveryoneGroup(id) {
3428-
return q.getEveryoneGroupMembersNoLock(ctx, id), nil
3459+
if q.isEveryoneGroup(arg.GroupID) {
3460+
return q.getEveryoneGroupMembersNoLock(ctx, arg.GroupID), nil
34293461
}
34303462

34313463
var groupMembers []database.GroupMember
34323464
for _, member := range q.groupMembers {
3433-
if member.GroupID == id {
3465+
if member.GroupID == arg.GroupID {
34343466
groupMember, err := q.getGroupMemberNoLock(ctx, member.UserID, member.GroupID)
34353467
if errors.Is(err, errUserDeleted) {
34363468
continue
@@ -3445,8 +3477,8 @@ func (q *FakeQuerier) GetGroupMembersByGroupID(ctx context.Context, id uuid.UUID
34453477
return groupMembers, nil
34463478
}
34473479

3448-
func (q *FakeQuerier) GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (int64, error) {
3449-
users, err := q.GetGroupMembersByGroupID(ctx, groupID)
3480+
func (q *FakeQuerier) GetGroupMembersCountByGroupID(ctx context.Context, arg database.GetGroupMembersCountByGroupIDParams) (int64, error) {
3481+
users, err := q.GetGroupMembersByGroupID(ctx, database.GetGroupMembersByGroupIDParams(arg))
34503482
if err != nil {
34513483
return 0, err
34523484
}
@@ -6223,12 +6255,16 @@ func (q *FakeQuerier) GetUserByID(_ context.Context, id uuid.UUID) (database.Use
62236255
return q.getUserByIDNoLock(id)
62246256
}
62256257

6226-
func (q *FakeQuerier) GetUserCount(_ context.Context) (int64, error) {
6258+
// nolint:revive // It's not a control flag, it's a filter.
6259+
func (q *FakeQuerier) GetUserCount(_ context.Context, includeSystem bool) (int64, error) {
62276260
q.mutex.RLock()
62286261
defer q.mutex.RUnlock()
62296262

62306263
existing := int64(0)
62316264
for _, u := range q.users {
6265+
if !includeSystem && u.IsSystem {
6266+
continue
6267+
}
62326268
if !u.Deleted {
62336269
existing++
62346270
}
@@ -6580,6 +6616,12 @@ func (q *FakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams
65806616
users = usersFilteredByLastSeen
65816617
}
65826618

6619+
if !params.IncludeSystem {
6620+
users = slices.DeleteFunc(users, func(u database.User) bool {
6621+
return u.IsSystem
6622+
})
6623+
}
6624+
65836625
if params.GithubComUserID != 0 {
65846626
usersFilteredByGithubComUserID := make([]database.User, 0, len(users))
65856627
for i, user := range users {
@@ -8933,6 +8975,7 @@ func (q *FakeQuerier) InsertUser(_ context.Context, arg database.InsertUserParam
89338975
Status: status,
89348976
RBACRoles: arg.RBACRoles,
89358977
LoginType: arg.LoginType,
8978+
IsSystem: false,
89368979
}
89378980
q.users = append(q.users, user)
89388981
sort.Slice(q.users, func(i, j int) bool {
@@ -10091,7 +10134,7 @@ func (q *FakeQuerier) UpdateInactiveUsersToDormant(_ context.Context, params dat
1009110134

1009210135
var updated []database.UpdateInactiveUsersToDormantRow
1009310136
for index, user := range q.users {
10094-
if user.Status == database.UserStatusActive && user.LastSeenAt.Before(params.LastSeenAfter) {
10137+
if user.Status == database.UserStatusActive && user.LastSeenAt.Before(params.LastSeenAfter) && !user.IsSystem {
1009510138
q.users[index].Status = database.UserStatusDormant
1009610139
q.users[index].UpdatedAt = params.UpdatedAt
1009710140
updated = append(updated, database.UpdateInactiveUsersToDormantRow{

0 commit comments

Comments
 (0)