Skip to content

Commit bc5f4f4

Browse files
dannykoppingdannykopping
committed
optionally prevent system users from counting to user count
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
1 parent 5ecc277 commit bc5f4f4

19 files changed

+105
-55
lines changed

‎cli/server.go

Lines changed: 1 addition & 1 deletion
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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1084,13 +1084,13 @@ func (q *querier) ActivityBumpWorkspace(ctx context.Context, arg database.Activi
10841084
return update(q.log, q.auth, fetch, q.db.ActivityBumpWorkspace)(ctx, arg)
10851085
}
10861086

1087-
func (q *querier) AllUserIDs(ctx context.Context) ([]uuid.UUID, error) {
1087+
func (q *querier) AllUserIDs(ctx context.Context, includeSystem bool) ([]uuid.UUID, error) {
10881088
// Although this technically only reads users, only system-related functions should be
10891089
// allowed to call this.
10901090
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
10911091
return nil, err
10921092
}
1093-
return q.db.AllUserIDs(ctx)
1093+
return q.db.AllUserIDs(ctx, includeSystem)
10941094
}
10951095

10961096
func (q *querier) ArchiveUnusedTemplateVersions(ctx context.Context, arg database.ArchiveUnusedTemplateVersionsParams) ([]uuid.UUID, error) {
@@ -1343,7 +1343,10 @@ func (q *querier) DeleteOldWorkspaceAgentStats(ctx context.Context) error {
13431343

13441344
func (q *querier) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error {
13451345
return deleteQ[database.OrganizationMember](q.log, q.auth, func(ctx context.Context, arg database.DeleteOrganizationMemberParams) (database.OrganizationMember, error) {
1346-
member, err := database.ExpectOne(q.OrganizationMembers(ctx, database.OrganizationMembersParams(arg)))
1346+
member, err := database.ExpectOne(q.OrganizationMembers(ctx, database.OrganizationMembersParams{
1347+
OrganizationID: arg.OrganizationID,
1348+
UserID: arg.UserID,
1349+
}))
13471350
if err != nil {
13481351
return database.OrganizationMember{}, err
13491352
}
@@ -1529,11 +1532,11 @@ func (q *querier) GetAPIKeysLastUsedAfter(ctx context.Context, lastUsed time.Tim
15291532
return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetAPIKeysLastUsedAfter)(ctx, lastUsed)
15301533
}
15311534

1532-
func (q *querier) GetActiveUserCount(ctx context.Context) (int64, error) {
1535+
func (q *querier) GetActiveUserCount(ctx context.Context, includeSystem bool) (int64, error) {
15331536
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
15341537
return 0, err
15351538
}
1536-
return q.db.GetActiveUserCount(ctx)
1539+
return q.db.GetActiveUserCount(ctx, includeSystem)
15371540
}
15381541

15391542
func (q *querier) GetActiveWorkspaceBuildsByTemplateID(ctx context.Context, templateID uuid.UUID) ([]database.WorkspaceBuild, error) {
@@ -2557,11 +2560,11 @@ func (q *querier) GetUserByID(ctx context.Context, id uuid.UUID) (database.User,
25572560
return fetch(q.log, q.auth, q.db.GetUserByID)(ctx, id)
25582561
}
25592562

2560-
func (q *querier) GetUserCount(ctx context.Context) (int64, error) {
2563+
func (q *querier) GetUserCount(ctx context.Context, includeSystem bool) (int64, error) {
25612564
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
25622565
return 0, err
25632566
}
2564-
return q.db.GetUserCount(ctx)
2567+
return q.db.GetUserCount(ctx, includeSystem)
25652568
}
25662569

25672570
func (q *querier) GetUserLatencyInsights(ctx context.Context, arg database.GetUserLatencyInsightsParams) ([]database.GetUserLatencyInsightsRow, error) {

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1664,7 +1664,7 @@ func (s *MethodTestSuite) TestUser() {
16641664
s.Run("AllUserIDs", s.Subtest(func(db database.Store, check *expects) {
16651665
a := dbgen.User(s.T(), db, database.User{})
16661666
b := dbgen.User(s.T(), db, database.User{})
1667-
check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(slice.New(a.ID, b.ID))
1667+
check.Args(false).Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(slice.New(a.ID, b.ID))
16681668
}))
16691669
s.Run("CustomRoles", s.Subtest(func(db database.Store, check *expects) {
16701670
check.Args(database.CustomRolesParams{}).Asserts(rbac.ResourceAssignRole, policy.ActionRead).Returns([]database.CustomRole{})
@@ -3649,7 +3649,7 @@ func (s *MethodTestSuite) TestSystemFunctions() {
36493649
check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead)
36503650
}))
36513651
s.Run("GetActiveUserCount", s.Subtest(func(db database.Store, check *expects) {
3652-
check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(int64(0))
3652+
check.Args(false).Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(int64(0))
36533653
}))
36543654
s.Run("GetUnexpiredLicenses", s.Subtest(func(db database.Store, check *expects) {
36553655
check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead)
@@ -3692,7 +3692,7 @@ func (s *MethodTestSuite) TestSystemFunctions() {
36923692
check.Args(time.Now().Add(time.Hour*-1)).Asserts(rbac.ResourceSystem, policy.ActionRead)
36933693
}))
36943694
s.Run("GetUserCount", s.Subtest(func(db database.Store, check *expects) {
3695-
check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(int64(0))
3695+
check.Args(false).Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(int64(0))
36963696
}))
36973697
s.Run("GetTemplates", s.Subtest(func(db database.Store, check *expects) {
36983698
dbtestutil.DisableForeignKeysAndTriggers(s.T(), db)

‎coderd/database/dbmem/dbmem.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1549,7 +1549,7 @@ func (q *FakeQuerier) ActivityBumpWorkspace(ctx context.Context, arg database.Ac
15491549
return sql.ErrNoRows
15501550
}
15511551

1552-
func (q *FakeQuerier) AllUserIDs(_ context.Context) ([]uuid.UUID, error) {
1552+
func (q *FakeQuerier) AllUserIDs(_ context.Context, includeSystem bool) ([]uuid.UUID, error) {
15531553
q.mutex.RLock()
15541554
defer q.mutex.RUnlock()
15551555
userIDs := make([]uuid.UUID, 0, len(q.users))
@@ -2644,7 +2644,7 @@ func (q *FakeQuerier) GetAPIKeysLastUsedAfter(_ context.Context, after time.Time
26442644
return apiKeys, nil
26452645
}
26462646

2647-
func (q *FakeQuerier) GetActiveUserCount(_ context.Context) (int64, error) {
2647+
func (q *FakeQuerier) GetActiveUserCount(_ context.Context, includeSystem bool) (int64, error) {
26482648
q.mutex.RLock()
26492649
defer q.mutex.RUnlock()
26502650

@@ -6200,7 +6200,8 @@ func (q *FakeQuerier) GetUserByID(_ context.Context, id uuid.UUID) (database.Use
62006200
return q.getUserByIDNoLock(id)
62016201
}
62026202

6203-
func (q *FakeQuerier) GetUserCount(_ context.Context) (int64, error) {
6203+
// nolint:revive // Not a control flag; used for filtering.
6204+
func (q *FakeQuerier) GetUserCount(_ context.Context, includeSystem bool) (int64, error) {
62046205
q.mutex.RLock()
62056206
defer q.mutex.RUnlock()
62066207

@@ -6209,6 +6210,10 @@ func (q *FakeQuerier) GetUserCount(_ context.Context) (int64, error) {
62096210
if !u.Deleted {
62106211
existing++
62116212
}
6213+
6214+
if !includeSystem && u.IsSystem.Valid && u.IsSystem.Bool {
6215+
continue
6216+
}
62126217
}
62136218
return existing, nil
62146219
}

‎coderd/database/dbmetrics/querymetrics.go

Lines changed: 7 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 12 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000301_system_user.down.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ DROP TRIGGER IF EXISTS prevent_system_user_deletions ON users;
99
-- Drop function
1010
DROP FUNCTION IF EXISTS prevent_system_user_changes();
1111

12+
-- Delete user status changes
13+
DELETE FROM user_status_changes
14+
WHERE user_id = 'c42fdf75-3097-471c-8c33-fb52454d81c0';
15+
1216
-- Delete system user
1317
DELETE FROM users
1418
WHERE id = 'c42fdf75-3097-471c-8c33-fb52454d81c0';

‎coderd/database/modelqueries.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,7 @@ func (q *sqlQuerier) GetAuthorizedUsers(ctx context.Context, arg GetUsersParams,
393393
arg.LastSeenAfter,
394394
arg.CreatedBefore,
395395
arg.CreatedAfter,
396+
arg.IncludeSystem,
396397
arg.OffsetOpt,
397398
arg.LimitOpt,
398399
)

‎coderd/database/querier.go

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/stretchr/testify/require"
1616

1717
"cdr.dev/slog/sloggers/slogtest"
18+
1819
"github.com/coder/coder/v2/coderd/coderdtest"
1920
"github.com/coder/coder/v2/coderd/database"
2021
"github.com/coder/coder/v2/coderd/database/db2sdk"

0 commit comments

Comments
 (0)