From 37fb872460d5c8ae237335ac4e74755b19ad9d6e Mon Sep 17 00:00:00 2001 From: man4ish Date: Sun, 4 Oct 2026 10:50:01 -0500 Subject: [PATCH] fix: authoritativeIdentity rejected real, valid numeric user/org ids omnibioai-auth's own /auth/validate returns user_id/org_id as JSON numbers (plain SQL integer primary keys, never stored as strings anywhere) -- but authoritativeIdentity required typeof === 'string' for both, so every real account with actual organization membership was rejected with "authenticated IAM identity lacks user or organization context", even though the identity was completely valid. This repo's own tests never caught it because every iamReply fixture mocks user_id/organization_id as hand-written strings, never the numeric shape the real upstream service actually returns -- found while testing a real account's first-ever use of Launcher against a live stack (every previous account tested here had no organization membership at all, which fails this same check for an unrelated, genuine reason, so the type bug never had a chance to surface). Now accepts either a string or a number for both ids, normalizing to a string (0 is a legitimate id, so the emptiness check is on the normalized string, not numeric truthiness). Co-Authored-By: Claude Sonnet 5 --- server.js | 21 +++++++++++++++++---- src/server.test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/server.js b/server.js index 4219dca..6d10038 100644 --- a/server.js +++ b/server.js @@ -37,10 +37,23 @@ function manifestKey(identity, workspaceId) { } function authoritativeIdentity(identity) { - const userId = identity?.user_id || identity?.sub || identity?.id; - const organizationId = identity?.organization_id || identity?.org_id || identity?.tenant_id; - if (typeof userId !== 'string' || !userId || typeof organizationId !== 'string' || !organizationId) return null; - return { user_id: userId, organization_id: organizationId }; + const userId = identity?.user_id ?? identity?.sub ?? identity?.id; + const organizationId = identity?.organization_id ?? identity?.org_id ?? identity?.tenant_id; + // omnibioai-auth's own /auth/validate returns user_id/org_id as JSON + // numbers (they're plain SQL integer primary keys, not stored as + // strings anywhere) -- accept either a string or a number here rather + // than demanding the caller's own type already be a string, then + // normalize to a string for manifestKey's template-literal key below + // (and everything else downstream that was already written assuming + // a string). 0 is a legitimate id and must not be treated as missing, + // so the emptiness check below is explicitly on the normalized string + // being non-empty, not on numeric truthiness. + const isIdLike = (v) => typeof v === 'string' || typeof v === 'number'; + if (!isIdLike(userId) || !isIdLike(organizationId)) return null; + const normalizedUserId = String(userId); + const normalizedOrganizationId = String(organizationId); + if (!normalizedUserId || !normalizedOrganizationId) return null; + return { user_id: normalizedUserId, organization_id: normalizedOrganizationId }; } async function verifyIdentity(authorization) { diff --git a/src/server.test.js b/src/server.test.js index 96041f7..09e7077 100644 --- a/src/server.test.js +++ b/src/server.test.js @@ -237,5 +237,32 @@ describe('launcher Express API', () => { expect(await requestApp('GET', '/api/launcher/v1/workspaces/private-workspace/manifest', GOOD)).toMatchObject({ status: 200 }); serverModule.manifests.delete(key); }); + + test('a numeric user_id/org_id (the real shape omnibioai-auth/validate returns -- plain SQL integer primary keys, never stored as strings) is accepted, not rejected as lacking org context', async () => { + // Reproduces a real production bug: authoritativeIdentity used to + // require typeof === 'string' for both ids, which every real + // account with actual org membership fails (the field is also + // literally named org_id in the real response, not + // organization_id, but the fallback chain already covers that -- + // this test is specifically about the numeric-vs-string type gap). + // platform.manage_infra so this request reaches + // requireAuthoritativeIdentity at all, rather than being rejected + // earlier for insufficient permissions and trivially passing this + // assertion for the wrong reason. + iamReply({ ...ADMIN, user_id: 6, org_id: 1 }); + dockerReply(200, { Id: 'container-1' }); + const response = await requestApp('POST', '/api/launcher/v1/workspaces', GOOD); + expect(response.body).not.toMatchObject({ error: 'authenticated IAM identity lacks user or organization context' }); + expect(response.status).not.toBe(403); + }); + + test('authoritativeIdentity normalizes numeric ids to strings and still rejects 0/absent ids', () => { + expect(serverModule.authoritativeIdentity({ user_id: 6, org_id: 1 })).toEqual({ user_id: '6', organization_id: '1' }); + expect(serverModule.authoritativeIdentity({ user_id: 'u1', organization_id: 'org-a' })).toEqual({ user_id: 'u1', organization_id: 'org-a' }); + expect(serverModule.authoritativeIdentity({ user_id: 6 })).toBeNull(); + expect(serverModule.authoritativeIdentity({ org_id: 1 })).toBeNull(); + expect(serverModule.authoritativeIdentity({})).toBeNull(); + expect(serverModule.authoritativeIdentity(null)).toBeNull(); + }); }); });