Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat: push GetUsers authorization filter to SQL#8497

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Emyrk merged 8 commits intomainfromstevenmasley/users_sql_filter
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 11 additions & 39 deletionscoderd/database/dbauthz/dbauthz.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -586,32 +586,6 @@ func (q *querier) SoftDeleteTemplateByID(ctx context.Context, id uuid.UUID) erro
return deleteQ(q.log, q.auth, q.db.GetTemplateByID, deleteF)(ctx, id)
}

func (q *querier) GetUsersWithCount(ctx context.Context, arg database.GetUsersParams) ([]database.User, int64, error) {
// TODO Implement this with a SQL filter. The count is incorrect without it.
rowUsers, err := q.db.GetUsers(ctx, arg)
if err != nil {
return nil, -1, err
}

if len(rowUsers) == 0 {
return []database.User{}, 0, nil
}

act, ok := ActorFromContext(ctx)
if !ok {
return nil, -1, NoActorError
}

// TODO: Is this correct? Should we return a restricted user?
users := database.ConvertUserRows(rowUsers)
users, err = rbac.Filter(ctx, q.auth, act, rbac.ActionRead, users)
if err != nil {
return nil, -1, err
}

return users, rowUsers[0].Count, nil
}

func (q *querier) SoftDeleteUserByID(ctx context.Context, id uuid.UUID) error {
deleteF := func(ctx context.Context, id uuid.UUID) error {
return q.db.UpdateUserDeletedByID(ctx, database.UpdateUserDeletedByIDParams{
Expand DownExpand Up@@ -904,15 +878,6 @@ func (q *querier) GetFileTemplates(ctx context.Context, fileID uuid.UUID) ([]dat
return q.db.GetFileTemplates(ctx, fileID)
}

func (q *querier) GetFilteredUserCount(ctx context.Context, arg database.GetFilteredUserCountParams) (int64, error) {
prep, err := prepareSQLFilter(ctx, q.auth, rbac.ActionRead, rbac.ResourceUser.Type)
if err != nil {
return -1, xerrors.Errorf("(dev error) prepare sql filter: %w", err)
}
// TODO: This should be the only implementation.
return q.GetAuthorizedUserCount(ctx, arg, prep)
}

func (q *querier) GetGitAuthLink(ctx context.Context, arg database.GetGitAuthLinkParams) (database.GitAuthLink, error) {
return fetch(q.log, q.auth, q.db.GetGitAuthLink)(ctx, arg)
}
Expand DownExpand Up@@ -1389,8 +1354,12 @@ func (q *querier) GetUserLinkByUserIDLoginType(ctx context.Context, arg database
}

func (q *querier) GetUsers(ctx context.Context, arg database.GetUsersParams) ([]database.GetUsersRow, error) {
// TODO: We should use GetUsersWithCount with a better method signature.
return fetchWithPostFilter(q.auth, q.db.GetUsers)(ctx, arg)
// This does the filtering in SQL.
prep, err := prepareSQLFilter(ctx, q.auth, rbac.ActionRead, rbac.ResourceUser.Type)
if err != nil {
return nil, xerrors.Errorf("(dev error) prepare sql filter: %w", err)
}
return q.db.GetAuthorizedUsers(ctx, arg, prep)
}

// GetUsersByIDs is only used for usernames on workspace return data.
Expand DownExpand Up@@ -2639,6 +2608,9 @@ func (q *querier) GetAuthorizedWorkspaces(ctx context.Context, arg database.GetW
return q.GetWorkspaces(ctx, arg)
}

func (q *querier) GetAuthorizedUserCount(ctx context.Context, arg database.GetFilteredUserCountParams, prepared rbac.PreparedAuthorized) (int64, error) {
return q.db.GetAuthorizedUserCount(ctx, arg, prepared)
// GetAuthorizedUsers is not required for dbauthz since GetUsers is already
// authenticated.
func (q *querier) GetAuthorizedUsers(ctx context.Context, arg database.GetUsersParams, _ rbac.PreparedAuthorized) ([]database.GetUsersRow, error) {
// GetUsers is authenticated.
return q.GetUsers(ctx, arg)
}
20 changes: 4 additions & 16 deletionscoderd/database/dbauthz/dbauthz_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -869,24 +869,12 @@ func (s *MethodTestSuite) TestUser() {
Asserts(a, rbac.ActionRead, b, rbac.ActionRead).
Returns(slice.New(a, b))
}))
s.Run("GetAuthorizedUserCount", s.Subtest(func(db database.Store, check *expects) {
_ = dbgen.User(s.T(), db, database.User{})
check.Args(database.GetFilteredUserCountParams{}, emptyPreparedAuthorized{}).Asserts().Returns(int64(1))
}))
s.Run("GetFilteredUserCount", s.Subtest(func(db database.Store, check *expects) {
_ = dbgen.User(s.T(), db, database.User{})
check.Args(database.GetFilteredUserCountParams{}).Asserts().Returns(int64(1))
}))
s.Run("GetUsers", s.Subtest(func(db database.Store, check *expects) {
a :=dbgen.User(s.T(), db, database.User{Username: "GetUsers-a-user"})
b :=dbgen.User(s.T(), db, database.User{Username: "GetUsers-b-user"})
dbgen.User(s.T(), db, database.User{Username: "GetUsers-a-user"})
dbgen.User(s.T(), db, database.User{Username: "GetUsers-b-user"})
check.Args(database.GetUsersParams{}).
Asserts(a, rbac.ActionRead, b, rbac.ActionRead)
}))
s.Run("GetUsersWithCount", s.Subtest(func(db database.Store, check *expects) {
a := dbgen.User(s.T(), db, database.User{Username: "GetUsersWithCount-a-user"})
b := dbgen.User(s.T(), db, database.User{Username: "GetUsersWithCount-b-user"})
check.Args(database.GetUsersParams{}).Asserts(a, rbac.ActionRead, b, rbac.ActionRead)
// Asserts are done in a SQL filter
Asserts()
}))
s.Run("InsertUser", s.Subtest(func(db database.Store, check *expects) {
check.Args(database.InsertUserParams{
Expand Down
84 changes: 19 additions & 65 deletionscoderd/database/dbfake/dbfake.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@ import (
"github.com/coder/coder/coderd/database/db2sdk"
"github.com/coder/coder/coderd/httpapi"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/coderd/rbac/regosql"
"github.com/coder/coder/coderd/util/slice"
"github.com/coder/coder/codersdk"
)
Expand DownExpand Up@@ -1207,14 +1208,6 @@ func (q *FakeQuerier) GetFileTemplates(_ context.Context, id uuid.UUID) ([]datab
return rows, nil
}

func (q *FakeQuerier) GetFilteredUserCount(ctx context.Context, arg database.GetFilteredUserCountParams) (int64, error) {
if err := validateDatabaseType(arg); err != nil {
return 0, err
}
count, err := q.GetAuthorizedUserCount(ctx, arg, nil)
return count, err
}

func (q *FakeQuerier) GetGitAuthLink(_ context.Context, arg database.GetGitAuthLinkParams) (database.GitAuthLink, error) {
if err := validateDatabaseType(arg); err != nil {
return database.GitAuthLink{}, err
Expand DownExpand Up@@ -5365,76 +5358,37 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database.
return q.convertToWorkspaceRowsNoLock(ctx, workspaces, int64(beforePageCount)), nil
}

func (q *FakeQuerier)GetAuthorizedUserCount(ctx context.Context,params database.GetFilteredUserCountParams, prepared rbac.PreparedAuthorized) (int64, error) {
if err := validateDatabaseType(params); err != nil {
return0, err
func (q *FakeQuerier)GetAuthorizedUsers(ctx context.Context,arg database.GetUsersParams, prepared rbac.PreparedAuthorized) ([]database.GetUsersRow, error) {
if err := validateDatabaseType(arg); err != nil {
returnnil, err
}

q.mutex.RLock()
defer q.mutex.RUnlock()

// Call this to match the same function calls as the SQL implementation.
if prepared != nil {
_, err := prepared.CompileToSQL(ctx, rbac.ConfigWithoutACL())
_, err := prepared.CompileToSQL(ctx, regosql.ConvertConfig{
VariableConverter: regosql.UserConverter(),
})
if err != nil {
return-1, err
returnnil, err
}
}

users := make([]database.User, 0, len(q.users))
users, err := q.GetUsers(ctx, arg)
if err != nil {
return nil, err
}

for _, user := range q.users {
q.mutex.RLock()
defer q.mutex.RUnlock()

filteredUsers := make([]database.GetUsersRow, 0, len(users))
for _, user := range users {
// If the filter exists, ensure the object is authorized.
if prepared != nil && prepared.Authorize(ctx, user.RBACObject()) != nil {
continue
}

users = append(users, user)
}

// Filter out deleted since they should never be returned..
tmp := make([]database.User, 0, len(users))
for _, user := range users {
if !user.Deleted {
tmp = append(tmp, user)
}
}
users = tmp

if params.Search != "" {
tmp := make([]database.User, 0, len(users))
for i, user := range users {
if strings.Contains(strings.ToLower(user.Email), strings.ToLower(params.Search)) {
tmp = append(tmp, users[i])
} else if strings.Contains(strings.ToLower(user.Username), strings.ToLower(params.Search)) {
tmp = append(tmp, users[i])
}
}
users = tmp
}

if len(params.Status) > 0 {
usersFilteredByStatus := make([]database.User, 0, len(users))
for i, user := range users {
if slice.ContainsCompare(params.Status, user.Status, func(a, b database.UserStatus) bool {
return strings.EqualFold(string(a), string(b))
}) {
usersFilteredByStatus = append(usersFilteredByStatus, users[i])
}
}
users = usersFilteredByStatus
}

if len(params.RbacRole) > 0 && !slice.Contains(params.RbacRole, rbac.RoleMember()) {
usersFilteredByRole := make([]database.User, 0, len(users))
for i, user := range users {
if slice.OverlapCompare(params.RbacRole, user.RBACRoles, strings.EqualFold) {
usersFilteredByRole = append(usersFilteredByRole, users[i])
}
}

users = usersFilteredByRole
Comment on lines -5392 to -5436
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

All this is done by theGetUsers call

filteredUsers = append(filteredUsers, user)
}

return int64(len(users)), nil
return filteredUsers, nil
}
15 changes: 4 additions & 11 deletionscoderd/database/dbmetrics/dbmetrics.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

29 changes: 7 additions & 22 deletionscoderd/database/dbmock/dbmock.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

59 changes: 48 additions & 11 deletionscoderd/database/modelqueries.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -255,29 +255,66 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa
}

type userQuerier interface {
GetAuthorizedUserCount(ctx context.Context, argGetFilteredUserCountParams, prepared rbac.PreparedAuthorized) (int64, error)
GetAuthorizedUsers(ctx context.Context, argGetUsersParams, prepared rbac.PreparedAuthorized) ([]GetUsersRow, error)
}

func (q *sqlQuerier) GetAuthorizedUserCount(ctx context.Context, arg GetFilteredUserCountParams, prepared rbac.PreparedAuthorized) (int64, error) {
authorizedFilter, err := prepared.CompileToSQL(ctx, rbac.ConfigWithoutACL())
func (q *sqlQuerier) GetAuthorizedUsers(ctx context.Context, arg GetUsersParams, prepared rbac.PreparedAuthorized) ([]GetUsersRow, error) {
authorizedFilter, err := prepared.CompileToSQL(ctx, regosql.ConvertConfig{
VariableConverter: regosql.UserConverter(),
})
if err != nil {
return-1, xerrors.Errorf("compile authorized filter: %w", err)
returnnil, xerrors.Errorf("compile authorized filter: %w", err)
}

filtered, err := insertAuthorizedFilter(getFilteredUserCount, fmt.Sprintf(" AND %s", authorizedFilter))
filtered, err := insertAuthorizedFilter(getUsers, fmt.Sprintf(" AND %s", authorizedFilter))
if err != nil {
return-1, xerrors.Errorf("insert authorized filter: %w", err)
returnnil, xerrors.Errorf("insert authorized filter: %w", err)
}

query := fmt.Sprintf("-- name: GetAuthorizedUserCount :one\n%s", filtered)
row := q.db.QueryRowContext(ctx, query,
query := fmt.Sprintf("-- name: GetAuthorizedUsers :many\n%s", filtered)
rows, err := q.db.QueryContext(ctx, query,
arg.AfterID,
arg.Search,
pq.Array(arg.Status),
pq.Array(arg.RbacRole),
arg.LastSeenBefore,
arg.LastSeenAfter,
arg.OffsetOpt,
arg.LimitOpt,
)
var count int64
err = row.Scan(&count)
return count, err
if err != nil {
return nil, err
}
defer rows.Close()
var items []GetUsersRow
for rows.Next() {
var i GetUsersRow
if err := rows.Scan(
&i.ID,
&i.Email,
&i.Username,
&i.HashedPassword,
&i.CreatedAt,
&i.UpdatedAt,
&i.Status,
&i.RBACRoles,
&i.LoginType,
&i.AvatarURL,
&i.Deleted,
&i.LastSeenAt,
&i.Count,
); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Close(); err != nil {
return nil, err
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}

func insertAuthorizedFilter(query string, replaceWith string) (string, error) {
Expand Down
2 changes: 0 additions & 2 deletionscoderd/database/querier.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

Loading
[8]ページ先頭
©2009-2025 Movatter.jp