From 43fabfa6c3d8af4f0c0b4cfaf08d96d40c09bd3b Mon Sep 17 00:00:00 2001
From: Jakub Zych <jakub@keios.eu>
Date: Mon, 5 Oct 2026 15:15:28 +0200
Subject: [PATCH] fix: a group filter value that is not a group id lists nobody
 (D-23)

User.FilterScope handed the filter value to the database as it came. A
value such as abc failed the query, so the Users list answered 500.

- groupFilterID reads a positive whole number, as a number or its text
- any other value narrows the list to nothing
- TestAdminUsersGroupFilterValue fails with a 500 without the change
---
 admin_users_edge_test.go | 49 ++++++++++++++++++++++++++++++++++++++++
 models/user.go           | 26 +++++++++++++++++++--
 2 files changed, 73 insertions(+), 2 deletions(-)
 create mode 100644 admin_users_edge_test.go

diff --git a/admin_users_edge_test.go b/admin_users_edge_test.go
new file mode 100644
index 0000000..0c80510
--- /dev/null
+++ b/admin_users_edge_test.go
@@ -0,0 +1,49 @@
+package user
+
+import (
+	"net/http"
+	"strconv"
+	"testing"
+
+	"git.golem15.com/golem15/sm-user-plugin/models"
+)
+
+// TestAdminUsersGroupFilterValue (D-23): the groups filter takes a group id.
+// A value that is not one matches nobody (or is refused with 422 by the
+// framework); it must not reach the database as it is, where a non-numeric
+// text fails the whole list with a 500.
+func TestAdminUsersGroupFilterValue(t *testing.T) {
+	env := newAdminEnv(t)
+	token := env.admin(t, permAccessUsers)
+	marker := "gfilter" + adminStamp()
+	group := env.seedGroup(t, "Filter "+marker, "")
+	member := env.seedUser(t, marker, nil)
+	other := env.seedUser(t, marker, nil)
+	env.join(t, member, group)
+
+	gid := strconv.FormatUint(uint64(group.ID), 10)
+	if page := env.users(t, token, "search="+marker+"&filter[groups]="+gid); page.Total != 1 || !page.has(member.ID) || page.has(other.ID) {
+		t.Fatalf("group filter = %s", page.Raw)
+	}
+	for _, value := range []string{"abc", "1%20OR%201=1", "-1", "0", "1.5", "99999999999999999999", gid + "x"} {
+		rec := env.call(t, http.MethodGet, adminAPI(adminUsersPath)+"?search="+marker+"&filter[groups]="+value, token, nil)
+		// The framework refuses a value it cannot read as a scope argument
+		// with 422; whatever it lets through must list nobody.
+		if rec.Code == http.StatusUnprocessableEntity {
+			continue
+		}
+		if rec.Code != http.StatusOK {
+			t.Fatalf("filter[groups]=%s answers %d: %s", value, rec.Code, rec.Body.String())
+		}
+		var rows []map[string]any
+		adminInto(t, rec, &rows)
+		if len(rows) != 0 {
+			t.Fatalf("filter[groups]=%s lists %d users, want none", value, len(rows))
+		}
+	}
+	// The list without the filter is unaffected.
+	if page := env.users(t, token, "search="+marker); page.Total != 2 {
+		t.Fatalf("unfiltered list = %s", page.Raw)
+	}
+	_ = models.FilterByGroup
+}
diff --git a/models/user.go b/models/user.go
index 579dab0..4886905 100644
--- a/models/user.go
+++ b/models/user.go
@@ -1,6 +1,9 @@
 package models
 
 import (
+	"fmt"
+	"strconv"
+	"strings"
 	"time"
 
 	"git.golem15.com/golem15/summercms/modules/lagoon/attach"
@@ -99,7 +102,7 @@ const FilterByGroup = "filterByGroup"
 func (User) FilterScopes() []string { return []string{FilterByGroup} }
 
 // FilterScope narrows db to the members of the group whose id is value. Any
-// other scope name matches nothing.
+// other scope name, and a value that is not a group id, match nothing.
 func (User) FilterScope(name string, db *gorm.DB, value any) *gorm.DB {
 	if db == nil {
 		return db
@@ -107,7 +110,26 @@ func (User) FilterScope(name string, db *gorm.DB, value any) *gorm.DB {
 	if name != FilterByGroup {
 		return db.Where("1 = 0")
 	}
-	return db.Where("users.id IN (SELECT user_id FROM users_groups WHERE user_group_id = ?)", value)
+	id, ok := groupFilterID(value)
+	if !ok {
+		return db.Where("1 = 0")
+	}
+	return db.Where("users.id IN (SELECT user_id FROM users_groups WHERE user_group_id = ?)", id)
+}
+
+// groupFilterID reads the value of the group filter: a positive whole number,
+// as a number or as its decimal text. Anything else is not a group id, and
+// handing it to the database would fail the whole list.
+func groupFilterID(value any) (uint64, bool) {
+	text := strings.TrimSpace(fmt.Sprint(value))
+	if value == nil || text == "" {
+		return 0, false
+	}
+	id, err := strconv.ParseUint(text, 10, 32)
+	if err != nil || id == 0 {
+		return 0, false
+	}
+	return id, true
 }
 
 func init() {