From 1fed081de5eb06b45f59670972e68684065edb22 Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Sun, 17 May 2026 21:02:09 -0500 Subject: [PATCH] =?UTF-8?q?feat(api):=20GET=20/v1/customer/search=20?= =?UTF-8?q?=E2=80=94=20company-scoped=20substring=20lookup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Search by substring across custCompanyName / custFName / custLName (case-insensitive ILIKE). Common SDK pattern is "user types 'acm' in the customer autocomplete," and the existing GET /v1/customer/bycompany/:id paginated list doesn't help unless the caller already knows the company id AND fetches the whole table to filter client-side. Search closes that gap. Auth shape (deliberately stricter than the existing list endpoint): - missing authKey -> 403 - non-master + companyId mismatching auth scope -> 403 - non-master without companyId -> auto-scope to own - master without companyId -> 400 The "master must specify companyId" requirement is intentional — a global cross-tenant substring search is a footgun (latency on huge tables; accidental data exposure if the master key wasn't authorized to read every tenant's data). Forcing the explicit scope keeps the surface predictable. `q` enforces a 2-char minimum at the zod boundary so a dropdown that fires on every keystroke doesn't full-scan the table on the first letter. Route declared before /:id so express doesn't treat "search" as a customer id and route to getCustomerById. OpenAPI gets the full path entry with parameter docs + the response envelope schema. Tests cover the auth contract (403), zod validation paths (q required, min length, strict() rejects unknown params, limit cap), and route mounting (verifies the search-before-:id ordering). Tests: 32 files / 234 passing + 4 integration skipped (was 31 / 227). Co-Authored-By: Claude Opus 4.7 (1M context) --- app/config/openapi.js | 42 ++++++++++ app/controllers/customercontroller.js | 110 ++++++++++++++++++++++++++ app/routers/router.js | 7 ++ app/schemas/customer.schema.js | 21 +++++ tests/api/customer-search.test.js | 107 +++++++++++++++++++++++++ 5 files changed, 287 insertions(+) create mode 100644 tests/api/customer-search.test.js diff --git a/app/config/openapi.js b/app/config/openapi.js index 5c91b40..6dd19f0 100644 --- a/app/config/openapi.js +++ b/app/config/openapi.js @@ -344,6 +344,48 @@ const spec = { }, }, }, + '/v1/customer/search': { + get: { + summary: 'Search customers by substring (company-scoped)', + description: + 'Case-insensitive ILIKE match on custCompanyName / custFName / custLName. ' + + 'Non-master keys are auto-scoped to their authKey company; master keys ' + + 'must pass companyId explicitly (no global cross-tenant search).', + security: [{ authKey: [] }], + parameters: [ + { name: 'q', in: 'query', required: true, schema: { type: 'string', minLength: 2, maxLength: 255 } }, + { name: 'companyId', in: 'query', schema: { type: 'integer' }, description: 'Required for master keys. Forbidden when it does not match a non-master authKey\'s own company.' }, + { name: 'limit', in: 'query', schema: { type: 'integer', default: 100, maximum: 500 } }, + { name: 'offset', in: 'query', schema: { type: 'integer', default: 0 } }, + ], + responses: { + 200: { + description: 'OK — search results', + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + message: { type: 'string' }, + q: { type: 'string' }, + companyId: { type: 'integer' }, + count: { type: 'integer' }, + limit: { type: 'integer' }, + offset: { type: 'integer' }, + customers: { + type: 'array', + items: { $ref: '#/components/schemas/Customer' }, + }, + }, + }, + }, + }, + }, + 400: { description: 'Bad request — q missing/too short, or master without companyId' }, + 403: { description: 'Missing authKey, or cross-tenant search attempt' }, + }, + }, + }, '/v1/customer/{id}': { get: { summary: 'Get one customer by id', diff --git a/app/controllers/customercontroller.js b/app/controllers/customercontroller.js index 739e428..6b12992 100644 --- a/app/controllers/customercontroller.js +++ b/app/controllers/customercontroller.js @@ -251,6 +251,116 @@ exports.getAllByCompanyId = async (req, res) => { } }; +/** + * GET /v1/customer/search + * + * Substring search over custCompanyName / custFName / custLName. + * + * Auth contract: + * - missing authKey -> 403 + * - non-master + companyId in query NOT matching auth scope -> 403 + * - non-master without companyId -> auto-scope to own + * - master without companyId -> 400 (companyId required) + * + * The companyId requirement for master keys is deliberate: a + * global substring search across every tenant's customer table + * is a big footgun (latency + accidental data exposure if the + * caller wasn't authorized to see all companies' data). Master + * keys must be explicit about the scope. + */ +exports.search = async (req, res) => { + const authKey = req.get('authKey'); + if (!authKey) { + return res.status(403).json({ message: "Authorization key not sent." }); + } + + let isAuthKeyMasterKey; + try { + isAuthKeyMasterKey = await IsMaster(authKey); + } catch (error) { + log.error({ err: error }, 'IsMaster failed'); + return res.status(500).json({ message: "Error!", error: String(error) }); + } + + // Resolve effective companyId from auth + query + let effectiveCompanyId; + if (isAuthKeyMasterKey) { + const qCompanyId = Number(req.query.companyId); + if (!Number.isInteger(qCompanyId) || qCompanyId <= 0) { + return res.status(400).json({ + message: "Master keys must specify companyId on search.", + }); + } + effectiveCompanyId = qCompanyId; + } else { + let authKeyCompanyId; + try { + authKeyCompanyId = await GetCompanyId(authKey); + } catch (error) { + log.error({ err: error }, 'GetCompanyId failed'); + return res.status(500).json({ message: "Error!", error: String(error) }); + } + if (authKeyCompanyId === -1) { + return res.status(403).json({ message: "Invalid Authorization Key." }); + } + const qCompanyId = req.query.companyId !== undefined ? Number(req.query.companyId) : null; + if (qCompanyId !== null && qCompanyId !== authKeyCompanyId) { + return res.status(403).json({ + message: "Cannot search customers in a company you do not belong to.", + }); + } + effectiveCompanyId = authKeyCompanyId; + } + + const q = String(req.query.q || ''); + const requestedLimit = parseInt(req.query.limit, 10); + const limit = Number.isInteger(requestedLimit) && requestedLimit > 0 + ? Math.min(requestedLimit, 500) + : 100; + const requestedOffset = parseInt(req.query.offset, 10); + const offset = Number.isInteger(requestedOffset) && requestedOffset >= 0 + ? requestedOffset + : 0; + + const Op = db.Sequelize && db.Sequelize.Op; + if (!Op) { + return res.status(500).json({ message: "Error!", error: "Sequelize Op not available" }); + } + + // ILIKE on Postgres; the `%` wildcards come from us, not the user. + const pattern = `%${q}%`; + const where = { + custCompId: effectiveCompanyId, + custArch: false, + [Op.or]: [ + { custCompanyName: { [Op.iLike]: pattern } }, + { custFName: { [Op.iLike]: pattern } }, + { custLName: { [Op.iLike]: pattern } }, + ], + }; + + try { + const { count, rows } = await Customer.findAndCountAll({ + where, + limit, + offset, + order: [['custId', 'ASC']], + }); + return res.status(200).json({ + message: `Found ${count} customer(s) matching ${JSON.stringify(q)} in company ${effectiveCompanyId}`, + q, + companyId: effectiveCompanyId, + count, + limit, + offset, + customers: rows, + }); + } catch (error) { + log.error({ err: error }, 'Customer.search findAndCountAll failed'); + return res.status(500).json({ message: "Error!", error: String(error) }); + } +}; + // ---- helpers ---- async function findAndRespond(customerId, res) { diff --git a/app/routers/router.js b/app/routers/router.js index 095ddf3..c194bbd 100644 --- a/app/routers/router.js +++ b/app/routers/router.js @@ -61,6 +61,13 @@ router.use('/docs', swaggerUi.serve, swaggerUi.setup(openapiSpec, { })); // v1 customer routes. +// /search MUST be declared before /:id so express doesn't treat +// "search" as an :id param (which intIdParam would 400 on). +router.get( + '/v1/customer/search', + v.query(customerSchemas.searchQuery), + customer.search, +); router.get( '/v1/customer/:id', v.params(customerSchemas.intIdParam), diff --git a/app/schemas/customer.schema.js b/app/schemas/customer.schema.js index c1761bc..df15ac8 100644 --- a/app/schemas/customer.schema.js +++ b/app/schemas/customer.schema.js @@ -44,8 +44,29 @@ const listByCompanyQuery = z.object({ message: 'Unexpected query parameter. Allowed: limit, offset.', }); +/** + * GET /v1/customer/search query schema. + * + * `q` is the substring to match against custCompanyName, custFName, + * and custLName (case-insensitive ILIKE). 2-char minimum so we don't + * page-scan the table on every dropdown keystroke. + * + * `companyId` is master-only — non-master keys are auto-scoped to + * their authKey's owning company and a `companyId` param that + * doesn't match returns 403. + */ +const searchQuery = z.object({ + q: z.string().min(2).max(255), + companyId: z.coerce.number().int().positive().optional(), + limit: z.coerce.number().int().positive().max(500).optional(), + offset: z.coerce.number().int().nonnegative().optional(), +}).strict({ + message: 'Unexpected query parameter. Allowed: q (>= 2 chars), companyId, limit, offset.', +}); + module.exports = { intIdParam, createCustomerBody, listByCompanyQuery, + searchQuery, }; diff --git a/tests/api/customer-search.test.js b/tests/api/customer-search.test.js new file mode 100644 index 0000000..b1883c8 --- /dev/null +++ b/tests/api/customer-search.test.js @@ -0,0 +1,107 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Aaron K. Clark +// +// HTTP tests for GET /v1/customer/search. Same constraint as the +// other API tests: vi.mock on db.config.js doesn't intercept the +// nested CJS require chain, so behavioral testing (real ILIKE +// matching) lives in tests/integration. What this file asserts: +// +// - auth contract (403 / 400 paths the test env can drive +// without a working DB mock) +// - query-param validation via zod middleware +// - route is mounted (not 404, no double-response) + +import { describe, test, expect, vi, beforeAll } from 'vitest'; +import request from 'supertest'; +import express from 'express'; + +vi.mock('../../app/config/db.config.js', () => ({ + sequelize: { + query: vi.fn().mockResolvedValue([]), + QueryTypes: { SELECT: 'SELECT' }, + }, + Sequelize: { Op: { or: Symbol('or'), iLike: Symbol('iLike') } }, + Customer: { + findAndCountAll: vi.fn().mockResolvedValue({ count: 0, rows: [] }), + }, + TimeEntry: {}, Worker: {}, BillingType: {}, InventoryItem: {}, + Company: {}, Job: {}, Invoice: {}, CustomerPayment: {}, + InvoiceJob: {}, ProductEntry: {}, VersionInfo: {}, + PurchaseOrderVendor: {}, PurchaseOrderHeader: {}, PurchaseOrderLine: {}, + InventoryTransaction: {}, + ApiKey: {}, ApiMaster: {}, +})); + +let app; + +beforeAll(async () => { + const router = (await import('../../app/routers/router.js')).default + || require('../../app/routers/router.js'); + app = express(); + app.use(express.json()); + app.use('/', router); +}); + +describe('GET /v1/customer/search auth contract', () => { + test('returns 403 when authKey header is missing', async () => { + const res = await request(app).get('/v1/customer/search?q=acme'); + expect(res.status).toBe(403); + expect(res.body.message).toMatch(/Authorization key not sent/i); + }); +}); + +describe('GET /v1/customer/search query validation', () => { + test('q is required (400 when missing)', async () => { + const res = await request(app) + .get('/v1/customer/search') + .set('authKey', 'any'); + expect(res.status).toBe(400); + }); + + test('q is too short — < 2 chars rejected (400)', async () => { + const res = await request(app) + .get('/v1/customer/search?q=a') + .set('authKey', 'any'); + expect(res.status).toBe(400); + }); + + test('q at the 2-char minimum is accepted (passes validation)', async () => { + const res = await request(app) + .get('/v1/customer/search?q=ab') + .set('authKey', 'any'); + // 400 would mean schema rejected; anything else means schema passed. + expect(res.status).not.toBe(400); + }); + + test('unknown query param is rejected via .strict()', async () => { + const res = await request(app) + .get('/v1/customer/search?q=acme&bogus=1') + .set('authKey', 'any'); + expect(res.status).toBe(400); + }); + + test('limit cap enforced — > 500 rejected', async () => { + const res = await request(app) + .get('/v1/customer/search?q=acme&limit=10000') + .set('authKey', 'any'); + expect(res.status).toBe(400); + }); +}); + +describe('GET /v1/customer/search route mounting', () => { + test('route is mounted; not treated as /v1/customer/:id', async () => { + // If the route ordering were wrong, "search" would be parsed as + // an :id param and intIdParam would 400 with a different message. + const res = await request(app) + .get('/v1/customer/search?q=acme') + .set('authKey', 'any'); + expect(res.body).toBeTypeOf('object'); + expect(res.body.message).toBeDefined(); + // 400 from intIdParam would say "expected positive integer"; + // search's own body errors are different (and we pass validation + // with q=acme anyway). + if (res.status === 400) { + expect(res.body.message).not.toMatch(/positive/i); + } + }); +});