Skip to content

Commit 49e893b

Browse files
EmyrkEmyrk
committed
Remove GetAuthorizedUserFilter
1 parent 634f4ca commit 49e893b

File tree

7 files changed

+61
-145
lines changed

7 files changed

+61
-145
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -615,12 +615,9 @@ func (q *querier) GetTemplateUserRoles(ctx context.Context, id uuid.UUID) ([]dat
615615
return q.db.GetTemplateUserRoles(ctx, id)
616616
}
617617

618-
func (q *querier) GetAuthorizedUserCount(ctx context.Context, arg database.GetFilteredUserCountParams, prepared rbac.PreparedAuthorized) (int64, error) {
619-
return q.db.GetAuthorizedUserCount(ctx, arg, prepared)
620-
}
621-
622618
func (q *querier) GetUsersWithCount(ctx context.Context, arg database.GetUsersParams) ([]database.User, int64, error) {
623-
rowUsers, err := q.db.GetUsers(ctx, arg)
619+
// q.GetUsers only returns authorized users
620+
rowUsers, err := q.GetUsers(ctx, arg)
624621
if err != nil {
625622
return nil, -1, err
626623
}
@@ -939,12 +936,10 @@ func (q *querier) GetFileTemplates(ctx context.Context, fileID uuid.UUID) ([]dat
939936
}
940937

941938
func (q *querier) GetFilteredUserCount(ctx context.Context, arg database.GetFilteredUserCountParams) (int64, error) {
942-
prep, err := prepareSQLFilter(ctx, q.auth, rbac.ActionRead, rbac.ResourceUser.Type)
943-
if err != nil {
944-
return -1, xerrors.Errorf("(dev error) prepare sql filter: %w", err)
939+
if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil {
940+
return -1, err
945941
}
946-
// TODO: This should be the only implementation.
947-
return q.GetAuthorizedUserCount(ctx, arg, prep)
942+
return q.db.GetFilteredUserCount(ctx, arg)
948943
}
949944

950945
func (q *querier) GetGitAuthLink(ctx context.Context, arg database.GetGitAuthLinkParams) (database.GitAuthLink, error) {

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -869,19 +869,17 @@ func (s *MethodTestSuite) TestUser() {
869869
Asserts(a, rbac.ActionRead, b, rbac.ActionRead).
870870
Returns(slice.New(a, b))
871871
}))
872-
s.Run("GetAuthorizedUserCount", s.Subtest(func(db database.Store, check *expects) {
873-
_ = dbgen.User(s.T(), db, database.User{})
874-
check.Args(database.GetFilteredUserCountParams{}, emptyPreparedAuthorized{}).Asserts().Returns(int64(1))
875-
}))
876872
s.Run("GetFilteredUserCount", s.Subtest(func(db database.Store, check *expects) {
877873
_ = dbgen.User(s.T(), db, database.User{})
878-
check.Args(database.GetFilteredUserCountParams{}).Asserts().Returns(int64(1))
874+
check.Args(database.GetFilteredUserCountParams{}).Asserts(
875+
rbac.ResourceSystem, rbac.ActionRead).Returns(int64(1))
879876
}))
880877
s.Run("GetUsers", s.Subtest(func(db database.Store, check *expects) {
881-
a := dbgen.User(s.T(), db, database.User{Username: "GetUsers-a-user"})
882-
b := dbgen.User(s.T(), db, database.User{Username: "GetUsers-b-user"})
878+
dbgen.User(s.T(), db, database.User{Username: "GetUsers-a-user"})
879+
dbgen.User(s.T(), db, database.User{Username: "GetUsers-b-user"})
883880
check.Args(database.GetUsersParams{}).
884-
Asserts(a, rbac.ActionRead, b, rbac.ActionRead)
881+
// Asserts are done in a SQL filter
882+
Asserts()
885883
}))
886884
s.Run("GetUsersWithCount", s.Subtest(func(db database.Store, check *expects) {
887885
a := dbgen.User(s.T(), db, database.User{Username: "GetUsersWithCount-a-user"})

‎coderd/database/dbfake/dbfake.go

Lines changed: 50 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -267,80 +267,6 @@ func (q *FakeQuerier) getUserByIDNoLock(id uuid.UUID) (database.User, error) {
267267
return database.User{}, sql.ErrNoRows
268268
}
269269

270-
func (q *FakeQuerier) GetAuthorizedUserCount(ctx context.Context, params database.GetFilteredUserCountParams, prepared rbac.PreparedAuthorized) (int64, error) {
271-
if err := validateDatabaseType(params); err != nil {
272-
return 0, err
273-
}
274-
275-
q.mutex.RLock()
276-
defer q.mutex.RUnlock()
277-
278-
// Call this to match the same function calls as the SQL implementation.
279-
if prepared != nil {
280-
_, err := prepared.CompileToSQL(ctx, rbac.ConfigWithoutACL())
281-
if err != nil {
282-
return -1, err
283-
}
284-
}
285-
286-
users := make([]database.User, 0, len(q.users))
287-
288-
for _, user := range q.users {
289-
// If the filter exists, ensure the object is authorized.
290-
if prepared != nil && prepared.Authorize(ctx, user.RBACObject()) != nil {
291-
continue
292-
}
293-
294-
users = append(users, user)
295-
}
296-
297-
// Filter out deleted since they should never be returned..
298-
tmp := make([]database.User, 0, len(users))
299-
for _, user := range users {
300-
if !user.Deleted {
301-
tmp = append(tmp, user)
302-
}
303-
}
304-
users = tmp
305-
306-
if params.Search != "" {
307-
tmp := make([]database.User, 0, len(users))
308-
for i, user := range users {
309-
if strings.Contains(strings.ToLower(user.Email), strings.ToLower(params.Search)) {
310-
tmp = append(tmp, users[i])
311-
} else if strings.Contains(strings.ToLower(user.Username), strings.ToLower(params.Search)) {
312-
tmp = append(tmp, users[i])
313-
}
314-
}
315-
users = tmp
316-
}
317-
318-
if len(params.Status) > 0 {
319-
usersFilteredByStatus := make([]database.User, 0, len(users))
320-
for i, user := range users {
321-
if slice.ContainsCompare(params.Status, user.Status, func(a, b database.UserStatus) bool {
322-
return strings.EqualFold(string(a), string(b))
323-
}) {
324-
usersFilteredByStatus = append(usersFilteredByStatus, users[i])
325-
}
326-
}
327-
users = usersFilteredByStatus
328-
}
329-
330-
if len(params.RbacRole) > 0 && !slice.Contains(params.RbacRole, rbac.RoleMember()) {
331-
usersFilteredByRole := make([]database.User, 0, len(users))
332-
for i, user := range users {
333-
if slice.OverlapCompare(params.RbacRole, user.RBACRoles, strings.EqualFold) {
334-
usersFilteredByRole = append(usersFilteredByRole, users[i])
335-
}
336-
}
337-
338-
users = usersFilteredByRole
339-
}
340-
341-
return int64(len(users)), nil
342-
}
343-
344270
func convertUsers(users []database.User, count int64) []database.GetUsersRow {
345271
rows := make([]database.GetUsersRow, len(users))
346272
for i, u := range users {
@@ -1673,12 +1599,58 @@ func (q *FakeQuerier) GetFileTemplates(_ context.Context, id uuid.UUID) ([]datab
16731599
return rows, nil
16741600
}
16751601

1676-
func (q *FakeQuerier) GetFilteredUserCount(ctx context.Context, arg database.GetFilteredUserCountParams) (int64, error) {
1677-
if err := validateDatabaseType(arg); err != nil {
1602+
func (q *FakeQuerier) GetFilteredUserCount(ctx context.Context, params database.GetFilteredUserCountParams) (int64, error) {
1603+
if err := validateDatabaseType(params); err != nil {
16781604
return 0, err
16791605
}
1680-
count, err := q.GetAuthorizedUserCount(ctx, arg, nil)
1681-
return count, err
1606+
1607+
q.mutex.RLock()
1608+
defer q.mutex.RUnlock()
1609+
1610+
// Filter out deleted since they should never be returned..
1611+
users := make([]database.User, 0, len(q.users))
1612+
for _, user := range q.users {
1613+
if !user.Deleted {
1614+
users = append(users, user)
1615+
}
1616+
}
1617+
1618+
if params.Search != "" {
1619+
tmp := make([]database.User, 0, len(users))
1620+
for i, user := range users {
1621+
if strings.Contains(strings.ToLower(user.Email), strings.ToLower(params.Search)) {
1622+
tmp = append(tmp, users[i])
1623+
} else if strings.Contains(strings.ToLower(user.Username), strings.ToLower(params.Search)) {
1624+
tmp = append(tmp, users[i])
1625+
}
1626+
}
1627+
users = tmp
1628+
}
1629+
1630+
if len(params.Status) > 0 {
1631+
usersFilteredByStatus := make([]database.User, 0, len(users))
1632+
for i, user := range users {
1633+
if slice.ContainsCompare(params.Status, user.Status, func(a, b database.UserStatus) bool {
1634+
return strings.EqualFold(string(a), string(b))
1635+
}) {
1636+
usersFilteredByStatus = append(usersFilteredByStatus, users[i])
1637+
}
1638+
}
1639+
users = usersFilteredByStatus
1640+
}
1641+
1642+
if len(params.RbacRole) > 0 && !slice.Contains(params.RbacRole, rbac.RoleMember()) {
1643+
usersFilteredByRole := make([]database.User, 0, len(users))
1644+
for i, user := range users {
1645+
if slice.OverlapCompare(params.RbacRole, user.RBACRoles, strings.EqualFold) {
1646+
usersFilteredByRole = append(usersFilteredByRole, users[i])
1647+
}
1648+
}
1649+
1650+
users = usersFilteredByRole
1651+
}
1652+
1653+
return int64(len(users)), nil
16821654
}
16831655

16841656
func (q *FakeQuerier) GetGitAuthLink(_ context.Context, arg database.GetGitAuthLinkParams) (database.GitAuthLink, error) {

‎coderd/database/dbmetrics/dbmetrics.go

Lines changed: 0 additions & 7 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: 0 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/modelqueries.go

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,6 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa
255255
}
256256

257257
type userQuerier interface {
258-
GetAuthorizedUserCount(ctx context.Context, arg GetFilteredUserCountParams, prepared rbac.PreparedAuthorized) (int64, error)
259258
GetAuthorizedUsers(ctx context.Context, arg GetUsersParams, prepared rbac.PreparedAuthorized) ([]GetUsersRow, error)
260259
}
261260

@@ -319,30 +318,6 @@ func (q *sqlQuerier) GetAuthorizedUsers(ctx context.Context, arg GetUsersParams,
319318
return items, nil
320319
}
321320

322-
func (q *sqlQuerier) GetAuthorizedUserCount(ctx context.Context, arg GetFilteredUserCountParams, prepared rbac.PreparedAuthorized) (int64, error) {
323-
authorizedFilter, err := prepared.CompileToSQL(ctx, regosql.ConvertConfig{
324-
VariableConverter: regosql.UserConverter(),
325-
})
326-
if err != nil {
327-
return -1, xerrors.Errorf("compile authorized filter: %w", err)
328-
}
329-
330-
filtered, err := insertAuthorizedFilter(getFilteredUserCount, fmt.Sprintf(" AND %s", authorizedFilter))
331-
if err != nil {
332-
return -1, xerrors.Errorf("insert authorized filter: %w", err)
333-
}
334-
335-
query := fmt.Sprintf("-- name: GetAuthorizedUserCount :one\n%s", filtered)
336-
row := q.db.QueryRowContext(ctx, query,
337-
arg.Search,
338-
pq.Array(arg.Status),
339-
pq.Array(arg.RbacRole),
340-
)
341-
var count int64
342-
err = row.Scan(&count)
343-
return count, err
344-
}
345-
346321
func insertAuthorizedFilter(query string, replaceWith string) (string, error) {
347322
if !strings.Contains(query, authorizedQueryPlaceholder) {
348323
return "", xerrors.Errorf("query does not contain authorized replace string, this is not an authorized query")

‎coderd/database/queries.sql.go

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

0 commit comments

Comments
 (0)