feat(admin): validate and document the window on the user list

.changeset/olive-swans-repeat.md

@@ -0,0 +1,27 @@
+---
+'seamless-auth-api': minor
+---
+
+Validate and document the window on `GET /admin/users`.
+
+The route has always read `limit`, `offset` and `search`, but it declared no
+query schema, so it was the one admin collection whose parameters were absent
+from `openapi.json`. A generated client could not know they existed, and a
+reader checking the document would conclude the endpoint took none. That is the
+same wrong inference that led to organization paging being reported missing.
+
+The parameters are now declared, so they appear in the generated contract
+alongside the ones on `/admin/sessions`, `/admin/auth-events` and
+`/admin/organizations`, and they are validated the same way: `limit` between 1
+and 100 defaulting to 50, `offset` from 0, and `search` trimmed.
+
+Two inputs that used to be accepted are now rejected with a `400`. A `limit`
+above 100 was previously honoured in full, so a single call could ask for every
+user in the deployment. A non-numeric `limit` reached Sequelize as `NaN` and
+failed in the database rather than at the edge. An all-whitespace `search` built
+a `%%` pattern that matched every row, so a filter that looked empty returned
+the unfiltered list.
+
+`seamless-cli` accepts `users list --limit 0` deliberately, meaning ask for
+nothing, and that value is now a `400`. The flag needs a floor of 1, or to skip
+the request when asked for zero rows.

openapi.json

@@ -1638,8 +1638,29 @@
     "/admin/users": {
       "get": {
         "summary": "List users (internal)",
+        "description": "Returns a window of users. `total` counts every user matching `search`, not the returned page.",
         "tags": ["Admin"],
         "security": [{ "bearerAuth": [] }],
+        "parameters": [
+          {
+            "schema": { "type": "number", "minimum": 1, "maximum": 100, "default": 50 },
+            "required": false,
+            "name": "limit",
+            "in": "query"
+          },
+          {
+            "schema": { "type": "number", "nullable": true, "minimum": 0, "default": 0 },
+            "required": false,
+            "name": "offset",
+            "in": "query"
+          },
+          {
+            "schema": { "type": "string", "minLength": 1, "maxLength": 120 },
+            "required": false,
+            "name": "search",
+            "in": "query"
+          }
+        ],
         "responses": {
           "200": {
             "description": "HTTP 200",
@@ -1692,6 +1713,47 @@
               }
             }
           },
+          "400": {
+            "description": "HTTP 400",
+            "content": {
+              "application/json": {
+                "example": {
+                  "error": "string",
+                  "message": "string",
+                  "details": { "issues": [null] }
+                },
+                "schema": {
+                  "type": "object",
+                  "properties": {
+                    "error": { "type": "string" },
+                    "message": { "type": "string" },
+                    "details": {
+                      "type": "object",
+                      "properties": {
+                        "issues": {
+                          "type": "array",
+                          "items": {
+                            "type": "object",
+                            "properties": {
+                              "path": {
+                                "type": "array",
+                                "items": { "anyOf": [{ "type": "string" }, { "type": "number" }] }
+                              },
+                              "code": { "type": "string" },
+                              "message": { "type": "string" }
+                            },
+                            "required": ["path", "code", "message"]
+                          }
+                        }
+                      },
+                      "required": ["issues"]
+                    }
+                  },
+                  "required": ["error"]
+                }
+              }
+            }
+          },
           "429": {
             "description": "HTTP 429",
             "content": {

src/controllers/admin.ts

@@ -15,6 +15,7 @@ import { getSequelize } from '../models/index.js';
 import { Session } from '../models/sessions.js';
 import { TotpCredential } from '../models/totpCredentials.js';
 import { User } from '../models/users.js';
+import { AdminUserListQuerySchema } from '../schemas/admin.query.js';
 import {
   CreateUserSchema,
   DeviceReplacementRecoverySchema,
@@ -82,7 +83,10 @@ function actingAdminId(req: Request): string | null {
 }
 
 export const getUsers = async (req: ServiceRequest, res: Response) => {
-  const { limit = 50, offset = 0, search } = req.query;
+  // Re-parsed rather than read straight off `req.query`: `defineRoute` has already
+  // validated it, so this cannot fail, and it is how the coerced numbers recover their
+  // types on an Express query whose values are otherwise strings.
+  const { limit, offset, search } = AdminUserListQuerySchema.parse(req.query);
 
   const where: WhereOptions<User> = search
     ? {
@@ -109,8 +113,8 @@ export const getUsers = async (req: ServiceRequest, res: Response) => {
         'createdAt',
         'updatedAt',
       ],
-      limit: Number(limit),
-      offset: Number(offset),
+      limit,
+      offset,
     }),
     User.count({ where }),
   ]);

src/generated/api.ts

@@ -1487,10 +1487,17 @@ export interface paths {
       path?: never;
       cookie?: never;
     };
-    /** List users (internal) */
+    /**
+     * List users (internal)
+     * @description Returns a window of users. `total` counts every user matching `search`, not the returned page.
+     */
     get: {
       parameters: {
-        query?: never;
+        query?: {
+          limit?: number;
+          offset?: number | null;
+          search?: string;
+        };
         header?: never;
         path?: never;
         cookie?: never;
@@ -1534,6 +1541,36 @@ export interface paths {
             };
           };
         };
+        /** @description HTTP 400 */
+        400: {
+          headers: {
+            [name: string]: unknown;
+          };
+          content: {
+            /**
+             * @example {
+             *       "error": "string",
+             *       "message": "string",
+             *       "details": {
+             *         "issues": [
+             *           null
+             *         ]
+             *       }
+             *     }
+             */
+            'application/json': {
+              error: string;
+              message?: string;
+              details?: {
+                issues: {
+                  path: (string | number)[];
+                  code: string;
+                  message: string;
+                }[];
+              };
+            };
+          };
+        };
         /** @description HTTP 429 */
         429: {
           headers: {

src/routes/admin.routes.ts

@@ -33,7 +33,7 @@ import {
 import { createRouter } from '../lib/createRouter.js';
 import { requireAdmin } from '../middleware/requireAdmin.js';
 import { requireStepUp } from '../middleware/requireStepUp.js';
-import { UserIdParamSchema } from '../schemas/admin.query.js';
+import { AdminUserListQuerySchema, UserIdParamSchema } from '../schemas/admin.query.js';
 import {
   CreateUserSchema,
   DeviceReplacementRecoverySchema,
@@ -248,10 +248,13 @@ adminRouter.get(
   {
     auth: 'access',
     summary: 'List users (internal)',
+    description:
+      'Returns a window of users. `total` counts every user matching `search`, not the returned page.',
     tags: ['Admin'],
     middleware: [requireAdmin('read')],
 
     schemas: {
+      query: AdminUserListQuerySchema,
       response: {
         200: UsersListResponseSchema,
         500: InternalErrorSchema,

src/schemas/admin.query.ts

@@ -4,4 +4,16 @@
  * See LICENSE file in the project root for full license information
  */
 
+import { PaginationQuerySchema } from '@seamless-auth/types';
+import { z } from 'zod';
+
 export { UserIdParamSchema } from '@seamless-auth/types';
+
+/**
+ * Kept local for the same reason the organization list query is: the response
+ * shape is already `{ users, total }`, so only this server's own list route
+ * needs the window.
+ */
+export const AdminUserListQuerySchema = PaginationQuerySchema.extend({
+  search: z.string().trim().min(1).max(120).optional(),
+});

tests/integration/admin/admin.spec.ts

@@ -782,6 +782,70 @@ describe('GET /admin/users (additional branches)', () => {
     expect(res.status).toBe(200);
     expect(res.body.users).toEqual([]);
   });
+
+  it('applies the default window when none is sent', async () => {
+    (User.findAll as any).mockResolvedValue([buildUser()]);
+    (User.count as any).mockResolvedValue(1);
+
+    const res = await request(app).get('/admin/users');
+
+    expect(res.status).toBe(200);
+    expect(User.findAll).toHaveBeenCalledWith(expect.objectContaining({ limit: 50, offset: 0 }));
+  });
+
+  it('applies the window that was sent', async () => {
+    (User.findAll as any).mockResolvedValue([buildUser()]);
+    (User.count as any).mockResolvedValue(400);
+
+    const res = await request(app).get('/admin/users').query({ limit: 25, offset: 100 });
+
+    expect(res.status).toBe(200);
+    // The count is of everything matching, not of the page, so the caller can
+    // tell there is more to ask for.
+    expect(res.body.total).toBe(400);
+    expect(User.findAll).toHaveBeenCalledWith(expect.objectContaining({ limit: 25, offset: 100 }));
+  });
+
+  it('trims the search term before matching', async () => {
+    (User.findAll as any).mockResolvedValue([buildUser()]);
+    (User.count as any).mockResolvedValue(1);
+
+    const res = await request(app).get('/admin/users').query({ search: '  ada  ' });
+
+    expect(res.status).toBe(200);
+    const where = (User.findAll as any).mock.calls[0][0].where;
+    expect(where[Op.or]).toEqual([
+      { email: { [Op.iLike]: '%ada%' } },
+      { phone: { [Op.iLike]: '%ada%' } },
+    ]);
+  });
+
+  // Previously unvalidated: the window went to Sequelize as it arrived, so a
+  // limit of 100000 was honoured and a non-numeric one reached the database as
+  // NaN.
+  it('rejects a window outside the allowed range', async () => {
+    const res = await request(app).get('/admin/users').query({ limit: 500 });
+
+    expect(res.status).toBe(400);
+    expect(User.findAll).not.toHaveBeenCalled();
+  });
+
+  it('rejects a non-numeric window', async () => {
+    const res = await request(app).get('/admin/users').query({ limit: 'all' });
+
+    expect(res.status).toBe(400);
+    expect(User.findAll).not.toHaveBeenCalled();
+  });
+
+  // An all-whitespace term trims to nothing. Accepting it would build `%%`,
+  // which matches every row, so a search that looks empty would silently
+  // return the unfiltered list.
+  it('rejects a search term that is only whitespace', async () => {
+    const res = await request(app).get('/admin/users').query({ search: '   ' });
+
+    expect(res.status).toBe(400);
+    expect(User.findAll).not.toHaveBeenCalled();
+  });
 });
 
 describe('POST /admin/users (additional branches)', () => {