Make the system users list searchable

enferno/admin/templates/admin/users.html

@@ -22,6 +22,17 @@
                             <v-toolbar flat color="white">
                                 <v-toolbar-title>{{ _('System Users') }}</v-toolbar-title>
 
+                                <v-text-field
+                                        variant="outlined"
+                                        class="mt-6"
+                                        density="compact"
+                                        clearable
+                                        @click:clear="resetSearch"
+                                        v-model="q"
+                                        label="{{ _('Search') }}"
+                                        @keydown.enter="refresh()"
+                                ></v-text-field>
+
                                 <v-spacer></v-spacer>
 
                                 <v-btn @click.once="forceGlobalReset" class="ma-2" color="error" variant="elevated">
@@ -350,6 +361,7 @@
                 translations: window.translations,
                 validationRules: validationRules,
                 options: {},
+                q: '',
 
                 headers: [
                     {title: "{{_('ID')}}", value: "id"},
@@ -611,7 +623,13 @@
                 refresh(options) {
                     this.options = options || { ...this.options, page: 1 };
                     this.loading = true;
-                    api.get(`/admin/api/users/?page=${this.options.page}&per_page=${this.options.itemsPerPage}`, {search: this.search}).then(res => {
+                    api.get('/admin/api/users/', {
+                        params: {
+                            page: this.options.page,
+                            per_page: this.options.itemsPerPage,
+                            q: this.q || undefined,
+                        }
+                    }).then(res => {
                         this.items = res.data.items;
                         this.itemsLength = res.data.total;
                     }).catch(err => {
@@ -621,6 +639,11 @@
                     });
                 },
 
+                resetSearch() {
+                    this.q = '';
+                    this.refresh();
+                },
+
                 createItem() {
                     this.editedItem = {...this.defaultItem};
                     // enable fields

enferno/admin/views/users.py

@@ -46,8 +46,20 @@ def api_users() -> Response:
     per_page = request.args.get("per_page", PER_PAGE, int)
     q = request.args.get("q")
     query = []
-    if q is not None:
-        query.append(User.name.ilike("%" + q + "%"))
+    if q:
+        term = f"%{q}%"
+        # username and email are masked for users who cannot view them, so
+        # searching them would leak values the caller is not allowed to see
+        if current_user.has_role("Admin") or current_user.view_usernames:
+            query.append(
+                or_(
+                    User.name.ilike(term),
+                    User.username.ilike(term),
+                    User.email.ilike(term),
+                )
+            )
+        else:
+            query.append(User.name.ilike(term))
     result = (
         User.query.filter(*query)
         .order_by(User.username)

tests/test_pentest_fixes.py

@@ -826,3 +826,71 @@ def has_role(self, r):
             assert mu.can_view_media() is True
         with patch.object(mu, "current_user", _Admin()):
             assert mu.can_view_media() is True
+
+
+# ---------------------------------------------------------------------------
+# BAY-01-021 (extension)  The users list gained a search box. Masking a value in
+# the response is not enough on its own: a filter over that same value turns the
+# list into an oracle, where a hit or miss reveals an identifier the caller is
+# not allowed to read. Search over username/email must therefore be gated on the
+# same condition to_compact() masks on.
+# ---------------------------------------------------------------------------
+
+
+@pytest.fixture
+def blind_mod(app, session, isolated_session_store):
+    """Mod without view_usernames. Built fresh: current_user does not observe
+    test-session edits, so mutating a shared fixture would silently pass."""
+    from enferno.admin.models import Activity
+    from enferno.user.models import Role, User
+
+    u = User(username=f"bm-{uuid4().hex[:8]}", password=hash_password("password"), active=1)
+    u.name = "Blind Mod"
+    u.view_usernames = False
+    u.fs_uniquifier = uuid4().hex
+    u.roles.append(session.query(Role).filter(Role.name == "Mod").first())
+    session.add(u)
+    session.commit()
+    user_id = u.id
+    with app.app_context():
+        with app.test_client(user=u) as client:
+            yield client
+    session.query(Activity).filter(Activity.user_id == user_id).delete(synchronize_session=False)
+    session.delete(u)
+    session.commit()
+
+
+@pytest.fixture
+def secret_identity(session):
+    from enferno.user.models import User
+
+    u = User(username="si-secret", password=hash_password("password"), active=1)
+    u.name = "Hidden Person"
+    u.email = "hidden@example.org"
+    u.fs_uniquifier = uuid4().hex
+    session.add(u)
+    session.commit()
+    yield u
+    session.delete(u)
+    session.commit()
+
+
+@pytest.mark.parametrize("probe", ["si-secret", "hidden@example.org"])
+def test_bay_01_021_search_is_not_an_identifier_oracle(blind_mod, secret_identity, probe):
+    resp = blind_mod.get(
+        f"/admin/api/users/?q={probe}", headers={"Content-Type": "application/json"}
+    )
+    assert resp.status_code == 200
+    assert resp.json["data"]["items"] == [], f"search leaked existence of {probe}"
+
+
+def test_bay_01_021_search_still_masks_what_it_returns(blind_mod, secret_identity):
+    """Even on a permitted match the payload stays masked."""
+    resp = blind_mod.get(
+        "/admin/api/users/?q=Hidden Person", headers={"Content-Type": "application/json"}
+    )
+    assert resp.status_code == 200
+    items = resp.json["data"]["items"]
+    assert len(items) == 1
+    assert items[0]["username"] == f"user-{secret_identity.id}"
+    assert "si-secret" not in str(items[0])

tests/test_users.py

@@ -728,3 +728,78 @@ def test_revoke_2fa(self, request, session, client_fixture, expected):
             assert found_user.tf_phone_number == target.tf_phone_number
             assert found_user.tf_primary_method == target.tf_primary_method
             assert len(new_wa) == 1
+
+
+# =========================================================================
+# GET /admin/api/users/ - search
+# =========================================================================
+
+
+class TestUserSearch:
+    @pytest.fixture
+    def searchable(self, session):
+        user = UserFactory()
+        user.fs_uniquifier = uuid4().hex
+        user.name = "Zainab Haddad"
+        user.username = "zhaddad"
+        user.email = "zainab@example.org"
+        session.add(user)
+        session.commit()
+        yield user
+        session.delete(user)
+        session.commit()
+
+    def _usernames(self, resp):
+        return [item["username"] for item in resp.json["data"]["items"]]
+
+    @pytest.mark.parametrize("term", ["Zainab", "zhaddad", "zainab@example.org"])
+    def test_admin_searches_name_username_and_email(self, admin_client, searchable, term):
+        resp = admin_client.get(f"/admin/api/users/?q={term}", headers=HEADERS)
+        assert resp.status_code == 200
+        assert self._usernames(resp) == ["zhaddad"]
+
+    def test_search_is_case_insensitive_and_partial(self, admin_client, searchable):
+        resp = admin_client.get("/admin/api/users/?q=HADD", headers=HEADERS)
+        assert resp.status_code == 200
+        assert "zhaddad" in self._usernames(resp)
+
+    def test_no_match_returns_empty(self, admin_client, searchable):
+        resp = admin_client.get("/admin/api/users/?q=nobodyhere", headers=HEADERS)
+        assert resp.status_code == 200
+        assert resp.json["data"]["items"] == []
+        assert resp.json["data"]["total"] == 0
+
+    def test_blank_query_does_not_filter(self, admin_client, searchable):
+        resp = admin_client.get("/admin/api/users/?q=", headers=HEADERS)
+        assert resp.status_code == 200
+        assert resp.json["data"]["total"] > 1
+
+    @pytest.fixture
+    def blind_mod_client(self, app, session, isolated_session_store):
+        """A Mod who cannot view usernames. Built fresh rather than by mutating
+        the shared fixture, since current_user does not see test-session edits."""
+        from enferno.user.models import Role
+
+        mod = User(username="TestBlindMod", password="password", active=1)
+        mod.name = "BlindMod"
+        mod.fs_uniquifier = uuid4().hex
+        mod.view_usernames = False
+        mod.roles.append(Role.query.filter(Role.name == "Mod").first())
+        session.add(mod)
+        session.commit()
+        with app.app_context():
+            with app.test_client(user=mod) as client:
+                yield client
+        session.delete(mod)
+        session.commit()
+
+    def test_hidden_identifiers_are_not_searchable(self, blind_mod_client, searchable):
+        """A user who cannot see usernames must not be able to probe them."""
+        resp = blind_mod_client.get("/admin/api/users/?q=zainab@example.org", headers=HEADERS)
+        assert resp.status_code == 200
+        assert resp.json["data"]["items"] == []
+
+        # the name is still searchable, as it was before
+        resp = blind_mod_client.get("/admin/api/users/?q=Zainab", headers=HEADERS)
+        assert resp.status_code == 200
+        assert resp.json["data"]["total"] == 1