coder · Emyrk · Jul 11, 2023 · Jul 12, 2023 · Jul 12, 2023 · Jul 12, 2023
diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go
@@ -116,7 +116,7 @@ func (RBACAsserter) convertObjects(t *testing.T, objs ...interface{}) []rbac.Obj
 		case codersdk.TemplateVersion:
 			robj = rbac.ResourceTemplate.InOrg(obj.OrganizationID)
 		case codersdk.User:
-			robj = rbac.ResourceUser.WithID(obj.ID)
+			robj = rbac.ResourceUser.WithID(obj.ID).WithOwner(obj.ID.String())
 		case codersdk.Workspace:
 			robj = rbac.ResourceWorkspace.WithID(obj.ID).InOrg(obj.OrganizationID).WithOwner(obj.OwnerID.String())
 		default:

diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -575,11 +575,6 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
 	return nil
 }
 
-func (q *querier) GetAuthorizedTemplates(ctx context.Context, arg database.GetTemplatesWithFilterParams, _ rbac.PreparedAuthorized) ([]database.Template, error) {
-	// TODO Delete this function, all GetTemplates should be authorized. For now just call getTemplates on the authz querier.
-	return q.GetTemplatesWithFilter(ctx, arg)
-}
-
 func (q *querier) SoftDeleteTemplateByID(ctx context.Context, id uuid.UUID) error {
 	deleteF := func(ctx context.Context, id uuid.UUID) error {
 		return q.db.UpdateTemplateDeletedByID(ctx, database.UpdateTemplateDeletedByIDParams{
@@ -591,37 +586,13 @@ 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) GetTemplateGroupRoles(ctx context.Context, id uuid.UUID) ([]database.TemplateGroup, error) {
-	// An actor is authorized to read template group roles if they are authorized to read the template.
-	template, err := q.db.GetTemplateByID(ctx, id)
+func (q *querier) GetUsersWithCount(ctx context.Context, arg database.GetUsersParams) ([]database.User, int64, error) {
+	prep, err := prepareSQLFilter(ctx, q.auth, rbac.ActionRead, rbac.ResourceUser.Type)
 	if err != nil {
-		return nil, err
+		return nil, -1, xerrors.Errorf("failed to prepare sql filter: %w", err)
 	}
-	if err := q.authorizeContext(ctx, rbac.ActionRead, template); err != nil {
-		return nil, err
-	}
-	return q.db.GetTemplateGroupRoles(ctx, id)
-}
 
-func (q *querier) GetTemplateUserRoles(ctx context.Context, id uuid.UUID) ([]database.TemplateUser, error) {
-	// An actor is authorized to query template user roles if they are authorized to read the template.
-	template, err := q.db.GetTemplateByID(ctx, id)
-	if err != nil {
-		return nil, err
-	}
-	if err := q.authorizeContext(ctx, rbac.ActionRead, template); err != nil {
-		return nil, err
-	}
-	return q.db.GetTemplateUserRoles(ctx, id)
-}
-
-func (q *querier) GetAuthorizedUserCount(ctx context.Context, arg database.GetFilteredUserCountParams, prepared rbac.PreparedAuthorized) (int64, error) {
-	return q.db.GetAuthorizedUserCount(ctx, arg, prepared)
-}
-
-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)
+	rowUsers, err := q.db.GetAuthorizedUsers(ctx, arg, prep)
 	if err != nil {
 		return nil, -1, err
 	}
@@ -630,18 +601,8 @@ func (q *querier) GetUsersWithCount(ctx context.Context, arg database.GetUsersPa
 		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
 }
 
@@ -655,11 +616,6 @@ func (q *querier) SoftDeleteUserByID(ctx context.Context, id uuid.UUID) error {
 	return deleteQ(q.log, q.auth, q.db.GetUserByID, deleteF)(ctx, id)
 }
 
-func (q *querier) GetAuthorizedWorkspaces(ctx context.Context, arg database.GetWorkspacesParams, _ rbac.PreparedAuthorized) ([]database.GetWorkspacesRow, error) {
-	// TODO Delete this function, all GetWorkspaces should be authorized. For now just call GetWorkspaces on the authz querier.
-	return q.GetWorkspaces(ctx, arg)
-}
-
 func (q *querier) SoftDeleteWorkspaceByID(ctx context.Context, id uuid.UUID) error {
 	return deleteQ(q.log, q.auth, q.db.GetWorkspaceByID, func(ctx context.Context, id uuid.UUID) error {
 		return q.db.UpdateWorkspaceDeletedByID(ctx, database.UpdateWorkspaceDeletedByIDParams{
@@ -1181,15 +1137,15 @@ func (q *querier) GetProvisionerLogsAfterID(ctx context.Context, arg database.Ge
 }
 
 func (q *querier) GetQuotaAllowanceForUser(ctx context.Context, userID uuid.UUID) (int64, error) {
-	err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(userID))
+	err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(userID).WithOwner(userID.String()))
 	if err != nil {
 		return -1, err
 	}
 	return q.db.GetQuotaAllowanceForUser(ctx, userID)
 }
 
 func (q *querier) GetQuotaConsumedForUser(ctx context.Context, userID uuid.UUID) (int64, error) {
-	err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(userID))
+	err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(userID).WithOwner(userID.String()))
 	if err != nil {
 		return -1, err
 	}
@@ -1427,16 +1383,20 @@ 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.
 // This function should be replaced by joining this data to the workspace query
 // itself.
 func (q *querier) GetUsersByIDs(ctx context.Context, ids []uuid.UUID) ([]database.User, error) {
 	for _, uid := range ids {
-		if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(uid)); err != nil {
+		if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(uid).WithOwner(uid.String())); err != nil {
 			return nil, err
 		}
 	}
@@ -1942,7 +1902,7 @@ func (q *querier) InsertUserGroupsByName(ctx context.Context, arg database.Inser
 
 // TODO: Should this be in system.go?
 func (q *querier) InsertUserLink(ctx context.Context, arg database.InsertUserLinkParams) (database.UserLink, error) {
-	if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceUser.WithID(arg.UserID)); err != nil {
+	if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceUser.WithID(arg.UserID).WithOwner(arg.UserID.String())); err != nil {
 		return database.UserLink{}, err
 	}
 	return q.db.InsertUserLink(ctx, arg)
@@ -2642,3 +2602,48 @@ func (q *querier) UpsertTailnetCoordinator(ctx context.Context, id uuid.UUID) (d
 	}
 	return q.db.UpsertTailnetCoordinator(ctx, id)
 }
+
+func (q *querier) GetAuthorizedTemplates(ctx context.Context, arg database.GetTemplatesWithFilterParams, _ rbac.PreparedAuthorized) ([]database.Template, error) {
+	// TODO Delete this function, all GetTemplates should be authorized. For now just call getTemplates on the authz querier.
+	return q.GetTemplatesWithFilter(ctx, arg)
+}
+
+func (q *querier) GetTemplateGroupRoles(ctx context.Context, id uuid.UUID) ([]database.TemplateGroup, error) {
+	// An actor is authorized to read template group roles if they are authorized to read the template.
+	template, err := q.db.GetTemplateByID(ctx, id)
+	if err != nil {
+		return nil, err
+	}
+	if err := q.authorizeContext(ctx, rbac.ActionRead, template); err != nil {
+		return nil, err
+	}
+	return q.db.GetTemplateGroupRoles(ctx, id)
+}
+
+func (q *querier) GetTemplateUserRoles(ctx context.Context, id uuid.UUID) ([]database.TemplateUser, error) {
+	// An actor is authorized to query template user roles if they are authorized to read the template.
+	template, err := q.db.GetTemplateByID(ctx, id)
+	if err != nil {
+		return nil, err
+	}
+	if err := q.authorizeContext(ctx, rbac.ActionRead, template); err != nil {
+		return nil, err
+	}
+	return q.db.GetTemplateUserRoles(ctx, id)
+}
+
+func (q *querier) GetAuthorizedWorkspaces(ctx context.Context, arg database.GetWorkspacesParams, _ rbac.PreparedAuthorized) ([]database.GetWorkspacesRow, error) {
+	// TODO Delete this function, all GetWorkspaces should be authorized. For now just call GetWorkspaces on the authz querier.
+	return q.GetWorkspaces(ctx, arg)
+}
+
+// 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)
+}
+
+func (q *querier) GetAuthorizedUserCount(ctx context.Context, arg database.GetFilteredUserCountParams, prepared rbac.PreparedAuthorized) (int64, error) {
+	return q.db.GetAuthorizedUserCount(ctx, arg, prepared)
+}
diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go
@@ -521,7 +521,7 @@ func (s *MethodTestSuite) TestOrganization() {
 		ma := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{OrganizationID: oa.ID})
 		mb := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{OrganizationID: ob.ID})
 		check.Args([]uuid.UUID{ma.UserID, mb.UserID}).
-			Asserts(rbac.ResourceUser.WithID(ma.UserID), rbac.ActionRead, rbac.ResourceUser.WithID(mb.UserID), rbac.ActionRead)
+			Asserts(rbac.ResourceUser.WithID(ma.UserID).WithOwner(ma.UserID.String()), rbac.ActionRead, rbac.ResourceUser.WithID(mb.UserID).WithOwner(mb.UserID.String()), rbac.ActionRead)
 	}))
 	s.Run("GetOrganizationMemberByUserID", s.Subtest(func(db database.Store, check *expects) {
 		mem := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{})

diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go
@@ -5437,3 +5437,38 @@ func (*FakeQuerier) UpsertTailnetClient(context.Context, database.UpsertTailnetC
 func (*FakeQuerier) UpsertTailnetCoordinator(context.Context, uuid.UUID) (database.TailnetCoordinator, error) {
 	return database.TailnetCoordinator{}, ErrUnimplemented
 }
+
+func (q *fakeQuerier) GetAuthorizedUsers(ctx context.Context, arg database.GetUsersParams, prepared rbac.PreparedAuthorized) ([]database.GetUsersRow, error) {
+	if err := validateDatabaseType(arg); err != nil {
+		return nil, err
+	}
+
+	// Call this to match the same function calls as the SQL implementation.
+	if prepared != nil {
+		_, err := prepared.CompileToSQL(ctx, regosql.ConvertConfig{
+			VariableConverter: regosql.UserConverter(),
+		})
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	users, err := q.GetUsers(ctx, arg)
+	if err != nil {
+		return nil, err
+	}
+
+	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
+		}
+
+		filteredUsers = append(filteredUsers, user)
+	}
+	return filteredUsers, nil
+}
diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go
diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go
diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go
@@ -201,7 +201,7 @@ func (m GetOrganizationIDsByMemberIDsRow) RBACObject() rbac.Object {
 	// TODO: This feels incorrect as we are really returning a list of orgmembers.
 	// This return type should be refactored to return a list of orgmembers, not this
 	// special type.
-	return rbac.ResourceUser.WithID(m.UserID)
+	return rbac.ResourceUser.WithID(m.UserID).WithOwner(m.UserID.String())
 }
 
 func (o Organization) RBACObject() rbac.Object {
@@ -233,15 +233,15 @@ func (f File) RBACObject() rbac.Object {
 // If you are trying to get the RBAC object for the UserData, use
 // u.UserDataRBACObject() instead.
 func (u User) RBACObject() rbac.Object {
-	return rbac.ResourceUser.WithID(u.ID)
+	return rbac.ResourceUser.WithID(u.ID).WithOwner(u.ID.String())
 }
 
 func (u User) UserDataRBACObject() rbac.Object {
 	return rbac.ResourceUserData.WithID(u.ID).WithOwner(u.ID.String())
 }
 
 func (u GetUsersRow) RBACObject() rbac.Object {
-	return rbac.ResourceUser.WithID(u.ID)
+	return rbac.ResourceUser.WithID(u.ID).WithOwner(u.ID.String())
 }
 
 func (u GitSSHKey) RBACObject() rbac.Object {