From b6b58d0659fc8caee68a65e112780a1122d55907 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Thu, 14 May 2026 20:53:10 +0200 Subject: [PATCH] fix(admin-users): normalise date fields to ISO across DB drivers (#485) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Admin > Users page crashed with "TypeError: e.split is not a function" on native installs (SQLite default). Reported by @blazmaric in #485 with a clean diagnosis: SQLite returns lastLogin / createdAt / updatedAt as integer milliseconds since epoch, while Postgres returns ISO strings via the standard JSON serialiser. The page used parseISO() on the raw value and parseISO trips on numbers. Fix at both layers — defence in depth: - backend/src/routes/adminUsers.js: new toIso() helper applied in transformUser + transformInvitation. Coerces Date / number / numeric-string / null to a single ISO 8601 string contract before the response leaves the API. Protects every consumer (frontend AND external API tokens / n8n) regardless of which DB driver is underneath. - frontend/src/services/userManagement.service.ts: same helper as defence-in-depth for stale backends mid-deploy and any cached pre-fix response shape. Also surfaced an existing transformInvitation gap — invitations endpoints were returning raw response.data.invitations without going through the transformer. 10 unit tests pin the toIso contract: all known driver shapes (Date, number, numeric-string, ISO-string, null/undefined/empty) plus the full transformer paths for transformUser and transformInvitation. Out of scope: same epoch-ms surface may exist on other admin pages that were never tested against SQLite (events list, customers, webhooks, api tokens, activity log). Worth a follow-up audit pass to apply toIso() in every snake_case→camelCase transformer the admin routes use, but the immediate Users-page crash is the only reported one and shipping that fix unblocks @blazmaric. --- .../routes/adminUsers.dateNormalize.test.js | 123 ++++++++++++++++++ backend/src/routes/adminUsers.js | 44 ++++++- .../src/services/userManagement.service.ts | 49 ++++++- 3 files changed, 206 insertions(+), 10 deletions(-) create mode 100644 backend/__tests__/routes/adminUsers.dateNormalize.test.js diff --git a/backend/__tests__/routes/adminUsers.dateNormalize.test.js b/backend/__tests__/routes/adminUsers.dateNormalize.test.js new file mode 100644 index 00000000..d30d1320 --- /dev/null +++ b/backend/__tests__/routes/adminUsers.dateNormalize.test.js @@ -0,0 +1,123 @@ +/** + * Pin the date-field normalisation in adminUsers transformer (#485). + * + * The Users page crashed on native/SQLite installs because Postgres + * returned ISO strings while SQLite returned epoch-millisecond + * integers, and the frontend `parseISO()` blew up on numbers with + * "e.split is not a function". The transformer now coerces every + * shape to an ISO 8601 string before serialising. + * + * These tests guard the contract so a future refactor can't quietly + * regress and re-break the same page on the same DB. + */ + +const adminUsersRoute = require('../../src/routes/adminUsers'); +const { toIso, transformUser, transformInvitation } = adminUsersRoute.__test; + +describe('toIso', () => { + it('passes null and undefined through unchanged', () => { + expect(toIso(null)).toBeNull(); + expect(toIso(undefined)).toBeUndefined(); + // Empty string also short-circuits — important so an unset + // last_login renders as "Never" instead of 1970-01-01T00:00:00Z. + expect(toIso('')).toBe(''); + }); + + it('coerces an integer epoch (SQLite shape) to an ISO 8601 string', () => { + // 2026-05-14T10:00:00.000Z, in epoch ms. + const epochMs = 1778752800000; + expect(toIso(epochMs)).toBe('2026-05-14T10:00:00.000Z'); + }); + + it('coerces a stringified large integer to an ISO 8601 string', () => { + // Some SQLite drivers stringify large integers because they + // overflow JS safe-integer in the driver's serialiser. Re-coerce + // so the frontend doesn't try to parseISO('1778752800000'). + expect(toIso('1778752800000')).toBe('2026-05-14T10:00:00.000Z'); + }); + + it('coerces a Date instance via toISOString', () => { + const d = new Date('2026-01-01T12:34:56.000Z'); + expect(toIso(d)).toBe('2026-01-01T12:34:56.000Z'); + }); + + it('passes an existing ISO string through unchanged', () => { + const iso = '2026-05-14T10:00:00.000Z'; + expect(toIso(iso)).toBe(iso); + }); + + it('passes a non-numeric short string (e.g. truncated date) through unchanged', () => { + // Defensive: anything that isn't a 10+ digit integer string is + // treated as already-stringified — the date library will surface + // the failure cleanly if it's malformed, rather than the + // transformer silently rewriting it. + expect(toIso('2026-05-14')).toBe('2026-05-14'); + }); +}); + +describe('transformUser', () => { + it('normalises last_login, created_at, updated_at coming from SQLite', () => { + const sqliteRow = { + id: 1, + username: 'admin', + email: 'admin@example.com', + is_active: 1, + last_login: 1778752800000, // epoch ms + last_login_ip: '127.0.0.1', + created_at: 1778751144600, // epoch ms + updated_at: 1778751242320, // epoch ms + role_id: 1, + role_name: 'super_admin', + role_display_name: 'Super Admin', + created_by_username: null, + }; + + const out = transformUser(sqliteRow); + + expect(out.lastLogin).toBe('2026-05-14T10:00:00.000Z'); + expect(out.createdAt).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/); + expect(out.updatedAt).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/); + // Other fields untouched. + expect(out.username).toBe('admin'); + expect(out.lastLoginIp).toBe('127.0.0.1'); + }); + + it('leaves Postgres ISO strings intact', () => { + const pgRow = { + id: 2, + username: 'second', + email: 'second@example.com', + is_active: true, + last_login: '2026-05-14T10:00:00.000Z', + created_at: '2026-05-13T08:00:00.000Z', + updated_at: '2026-05-14T09:00:00.000Z', + }; + const out = transformUser(pgRow); + expect(out.lastLogin).toBe('2026-05-14T10:00:00.000Z'); + expect(out.createdAt).toBe('2026-05-13T08:00:00.000Z'); + expect(out.updatedAt).toBe('2026-05-14T09:00:00.000Z'); + }); + + it('keeps last_login null when the user has never logged in', () => { + const out = transformUser({ + id: 3, username: 'fresh', email: 'fresh@example.com', + is_active: 1, last_login: null, + }); + expect(out.lastLogin).toBeNull(); + }); +}); + +describe('transformInvitation', () => { + it('normalises expires_at and created_at from SQLite epoch-ms', () => { + const out = transformInvitation({ + id: 9, + email: 'invitee@example.com', + expires_at: 1779357600000, + created_at: 1778752800000, + role_name: 'admin', + invited_by: 'admin', + }); + expect(out.expiresAt).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/); + expect(out.createdAt).toBe('2026-05-14T10:00:00.000Z'); + }); +}); diff --git a/backend/src/routes/adminUsers.js b/backend/src/routes/adminUsers.js index a236b175..1852f903 100644 --- a/backend/src/routes/adminUsers.js +++ b/backend/src/routes/adminUsers.js @@ -11,6 +11,37 @@ const { handleAsync, validateRequest, successResponse } = require('../utils/rout const userManagementService = require('../services/userManagementService'); const router = express.Router(); +/** + * Coerce any of the shapes a TIMESTAMP column produces across our + * supported drivers into a single ISO 8601 string the frontend (and + * any external API consumer) can safely pass to date-fns / new Date. + * + * Postgres → Date object (becomes ISO via JSON.stringify anyway, but + * pinning the format defends against driver-side surprises). + * SQLite → integer milliseconds since epoch (the surface that crashed + * the admin Users page in #485 — `parseISO(123456789)` blows up + * with "e.split is not a function"). Native installs default to + * SQLite, so this path matters every release. + * Already a string → assume it's a parseable ISO/RFC3339 (Postgres + * driver may stringify under JSON serialization mid-pipeline). + * + * Returns null/undefined unchanged so an unset last_login surfaces as + * "Never" in the UI rather than 1970-01-01T00:00:00Z. + */ +function toIso(value) { + if (value === null || value === undefined || value === '') return value; + if (value instanceof Date) return value.toISOString(); + if (typeof value === 'number') return new Date(value).toISOString(); + if (typeof value === 'string') { + // Numeric-as-string ("1778752458666") happens when the SQLite + // driver stringifies large integers — re-coerce so the frontend + // doesn't try to parseISO('1778752458666'). + if (/^\d{10,}$/.test(value)) return new Date(Number(value)).toISOString(); + return value; + } + return value; +} + /** * Transform user object from snake_case (DB) to camelCase (API) */ @@ -20,10 +51,10 @@ function transformUser(user) { username: user.username, email: user.email, isActive: user.is_active, - lastLogin: user.last_login, + lastLogin: toIso(user.last_login), lastLoginIp: user.last_login_ip, - createdAt: user.created_at, - updatedAt: user.updated_at, + createdAt: toIso(user.created_at), + updatedAt: toIso(user.updated_at), roleId: user.role_id, roleName: user.role_name, roleDisplayName: user.role_display_name, @@ -52,8 +83,8 @@ function transformInvitation(invitation) { return { id: invitation.id, email: invitation.email, - expiresAt: invitation.expires_at, - createdAt: invitation.created_at, + expiresAt: toIso(invitation.expires_at), + createdAt: toIso(invitation.created_at), roleName: invitation.role_name, invitedBy: invitation.invited_by }; @@ -212,4 +243,7 @@ router.post('/:id/reset-password', [ successResponse(res, { message: 'Password reset email sent', ...result }); })); +// Test surface: expose the date normaliser so the unit test can pin +// the contract without spinning up the full router. module.exports = router; +module.exports.__test = { toIso, transformUser, transformInvitation }; diff --git a/frontend/src/services/userManagement.service.ts b/frontend/src/services/userManagement.service.ts index 3f62afd9..76087d7e 100644 --- a/frontend/src/services/userManagement.service.ts +++ b/frontend/src/services/userManagement.service.ts @@ -1,6 +1,33 @@ import { api } from '../config/api'; import type { AdminUser, AdminRole, AdminInvitation } from '../types'; +/** + * Defence-in-depth normalisation for date fields (#485). The backend + * now always returns these as ISO strings via toIso() in adminUsers.js, + * but this wrapper: + * + * - Lets the page survive a stale backend during a mid-deploy window + * (older backend still returns SQLite epoch-ms numbers). + * - Lets the page survive an external API consumer's response cache + * that captured the pre-fix shape. + * + * The original crash was `parseISO(123456789)` in + * AdminUsersPage → `e.split is not a function`. Coercing here means + * the next consumer of the AdminUser type can rely on the field + * being a string regardless of how it landed. + */ +function normalizeDateValue(value: unknown): string | null | undefined { + if (value === null || value === undefined || value === '') return value as null | undefined; + if (value instanceof Date) return value.toISOString(); + if (typeof value === 'number') return new Date(value).toISOString(); + if (typeof value === 'string') { + // SQLite via some drivers stringifies large integers — coerce back. + if (/^\d{10,}$/.test(value)) return new Date(Number(value)).toISOString(); + return value; + } + return undefined; +} + // Transform snake_case API response to camelCase for frontend // eslint-disable-next-line @typescript-eslint/no-explicit-any function transformUser(user: any): AdminUser { @@ -9,10 +36,10 @@ function transformUser(user: any): AdminUser { username: user.username, email: user.email, isActive: user.isActive ?? user.is_active, - lastLogin: user.lastLogin ?? user.last_login, + lastLogin: normalizeDateValue(user.lastLogin ?? user.last_login), lastLoginIp: user.lastLoginIp ?? user.last_login_ip, - createdAt: user.createdAt ?? user.created_at, - updatedAt: user.updatedAt ?? user.updated_at, + createdAt: normalizeDateValue(user.createdAt ?? user.created_at), + updatedAt: normalizeDateValue(user.updatedAt ?? user.updated_at), roleId: user.roleId ?? user.role_id, roleName: user.roleName ?? user.role_name, roleDisplayName: user.roleDisplayName ?? user.role_display_name, @@ -20,6 +47,18 @@ function transformUser(user: any): AdminUser { }; } +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function transformInvitation(invitation: any): AdminInvitation { + return { + id: invitation.id, + email: invitation.email, + roleName: invitation.roleName ?? invitation.role_name, + invitedBy: invitation.invitedBy ?? invitation.invited_by, + expiresAt: normalizeDateValue(invitation.expiresAt ?? invitation.expires_at) as string, + createdAt: normalizeDateValue(invitation.createdAt ?? invitation.created_at) as string, + }; +} + // eslint-disable-next-line @typescript-eslint/no-explicit-any function transformRole(role: any): AdminRole { return { @@ -127,7 +166,7 @@ export const userManagementService = { */ async getInvitations(): Promise { const response = await api.get('/admin/users/invitations'); - return response.data.invitations; + return response.data.invitations.map(transformInvitation); }, /** @@ -135,7 +174,7 @@ export const userManagementService = { */ async createInvitation(data: CreateInvitationData): Promise { const response = await api.post('/admin/users/invite', data); - return response.data.invitation; + return transformInvitation(response.data.invitation); }, /**