From 465d25e9ffb6659f6e254c87f4745142755941dc Mon Sep 17 00:00:00 2001 From: Aarav Sharma Date: Mon, 3 Aug 2026 20:09:36 -0600 Subject: [PATCH 1/2] fix(account): allow user tokens to look up accounts by social key findPersonBySocialKey is a read-only lookup used by both the front-end and CLI clients to resolve a social-key (e.g. email) to a person or account. The service-token gate it inherited was silently rejecting every regular user JWT, so every cross-workspace user lookup failed with Forbidden. Drop the gate; the underlying db.socialId.findOne / db.account.findOne reads are intentional and safe. Tests cover the user-token success path, requireAccount=true variant, empty-string rejection, and the missing-key not-found path. Signed-off-by: Aarav Sharma --- .../src/__tests__/serviceOperations.test.ts | 93 +++++++++++++++++++ server/account/src/serviceOperations.ts | 4 - 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/server/account/src/__tests__/serviceOperations.test.ts b/server/account/src/__tests__/serviceOperations.test.ts index 7d11125a32d..57de8360cdc 100644 --- a/server/account/src/__tests__/serviceOperations.test.ts +++ b/server/account/src/__tests__/serviceOperations.test.ts @@ -40,6 +40,7 @@ import { createIntegration, deleteIntegration, deleteIntegrationSecret, + findPersonBySocialKey, getIntegration, getIntegrationSecret, listIntegrations, @@ -1603,3 +1604,95 @@ describe('upsertSubscription', () => { ) }) }) + +describe('findPersonBySocialKey', () => { + const mockCtx = {} as unknown as MeasureContext + const mockBranding = null + const mockToken = 'test-token' + + function makeMockDb (socialId: { personUuid: string } | null, accountUuid: string | null = null): AccountDB { + return { + socialId: { + findOne: jest.fn().mockResolvedValue(socialId) + }, + account: { + findOne: jest.fn().mockResolvedValue(accountUuid === null ? null : { uuid: accountUuid }) + } + } as unknown as AccountDB + } + + beforeEach(() => { + jest.clearAllMocks() + }) + + test('allows a regular user token (no service claim) to look up by social key', async () => { + ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ + extra: { authMethod: 'password' }, + account: 'user-uuid' + }) + const mockDb = makeMockDb({ personUuid: 'looked-up-person' }) + + const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { + socialString: 'email:alice@example.com' + }) + + expect(result).toBe('looked-up-person') + expect(mockDb.socialId.findOne).toHaveBeenCalledWith({ key: 'email:alice@example.com' }) + }) + + test('still rejects empty socialString', async () => { + ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ + extra: { authMethod: 'password' }, + account: 'user-uuid' + }) + const mockDb = makeMockDb(null) + + await expect( + findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { socialString: '' }) + ).rejects.toThrow(/BadRequest/) + }) + + test('returns undefined when social key is not found', async () => { + ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ + extra: { authMethod: 'password' }, + account: 'user-uuid' + }) + const mockDb = makeMockDb(null) + + const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { + socialString: 'email:missing@example.com' + }) + + expect(result).toBeUndefined() + }) + + test('with requireAccount=true returns the account uuid when the person has one', async () => { + ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ + extra: { authMethod: 'password' }, + account: 'user-uuid' + }) + const mockDb = makeMockDb({ personUuid: 'person-uuid' }, 'account-uuid') + + const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { + socialString: 'email:alice@example.com', + requireAccount: true + }) + + expect(result).toBe('account-uuid') + }) + + test('with requireAccount=true returns undefined when the person has no account', async () => { + ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ + extra: { authMethod: 'password' }, + account: 'user-uuid' + }) + const mockDb = makeMockDb({ personUuid: 'person-uuid' }, null) + + const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { + socialString: 'email:alice@example.com', + requireAccount: true + }) + + expect(result).toBeUndefined() + }) +}) diff --git a/server/account/src/serviceOperations.ts b/server/account/src/serviceOperations.ts index f13af033458..5295398da95 100644 --- a/server/account/src/serviceOperations.ts +++ b/server/account/src/serviceOperations.ts @@ -997,10 +997,6 @@ export async function findPersonBySocialKey ( throw new PlatformError(new Status(Severity.ERROR, platform.status.BadRequest, {})) } - const { extra } = decodeTokenVerbose(ctx, token) - - verifyAllowedServices(['tool', 'workspace', 'aibot', ...integrationServices], extra) - const socialId = await db.socialId.findOne({ key: socialString }) if (socialId == null) { From 4ea9e8e21fbc66a9bbc6b3ea599ea5fe0211684b Mon Sep 17 00:00:00 2001 From: Aarav Sharma Date: Mon, 17 Aug 2026 21:53:43 -0600 Subject: [PATCH 2/2] fix(account): scope findPersonBySocialKey to shared workspaces for user tokens Restores the service-token gate and adds a workspace-overlap check for regular user tokens: the lookup only returns a result when the caller's account and the target account share at least one active workspace. This prevents cross-tenant user enumeration while still letting CLI and front-end clients resolve colleagues by social key. Service and admin tokens keep the unrestricted global lookup. Empty socialString still returns BadRequest. A non-shared or missing person both return undefined, so the endpoint does not leak the existence of a social key. Signed-off-by: Aarav Sharma --- .../src/__tests__/serviceOperations.test.ts | 241 ++++++++++++++++-- server/account/src/serviceOperations.ts | 39 ++- 2 files changed, 251 insertions(+), 29 deletions(-) diff --git a/server/account/src/__tests__/serviceOperations.test.ts b/server/account/src/__tests__/serviceOperations.test.ts index 57de8360cdc..f4e8a1306f9 100644 --- a/server/account/src/__tests__/serviceOperations.test.ts +++ b/server/account/src/__tests__/serviceOperations.test.ts @@ -1606,18 +1606,63 @@ describe('upsertSubscription', () => { }) describe('findPersonBySocialKey', () => { - const mockCtx = {} as unknown as MeasureContext + const mockCtx = { + warn: jest.fn(), + error: jest.fn() + } as unknown as MeasureContext const mockBranding = null const mockToken = 'test-token' + const callerAccount = 'caller-account' as AccountUuid + const callerWorkspace = 'ws-1' as WorkspaceUuid + const otherWorkspace = 'ws-2' as WorkspaceUuid + + function makeMockDb (options: { + socialId?: { personUuid: string } | null + targetAccountUuid?: string | null + callerWorkspaces?: WorkspaceUuid[] + targetWorkspaces?: WorkspaceUuid[] + }): AccountDB { + const socialId = options.socialId ?? null + const targetAccountUuid = options.targetAccountUuid ?? null + const callerWorkspaces = options.callerWorkspaces ?? [] + const targetWorkspaces = options.targetWorkspaces ?? [] + + const allWorkspaces: Record = { + [callerAccount]: callerWorkspaces, + ...(targetAccountUuid != null ? { [targetAccountUuid]: targetWorkspaces } : {}) + } + + const accountFindOne = jest.fn().mockImplementation(async (query: { uuid: AccountUuid }) => { + if (query.uuid === targetAccountUuid) { + return { uuid: targetAccountUuid } + } + return null + }) + + const getAccountWorkspaces = jest.fn().mockImplementation(async (accountId: AccountUuid) => { + const ws = allWorkspaces[accountId] ?? [] + return ws.map((uuid) => ({ + uuid, + status: { mode: 'active' as any }, + name: 'ws', + url: 'ws', + branding: null, + location: 'local', + region: 'local', + createdBy: callerAccount, + createdOn: 0, + billingAccount: null + } as any)) + }) - function makeMockDb (socialId: { personUuid: string } | null, accountUuid: string | null = null): AccountDB { return { socialId: { findOne: jest.fn().mockResolvedValue(socialId) }, account: { - findOne: jest.fn().mockResolvedValue(accountUuid === null ? null : { uuid: accountUuid }) - } + findOne: accountFindOne + }, + getAccountWorkspaces } as unknown as AccountDB } @@ -1625,68 +1670,178 @@ describe('findPersonBySocialKey', () => { jest.clearAllMocks() }) - test('allows a regular user token (no service claim) to look up by social key', async () => { + test('still rejects empty socialString', async () => { + ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ + extra: { authMethod: 'password' }, + account: callerAccount + }) + const mockDb = makeMockDb({}) + + await expect( + findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { socialString: '' }) + ).rejects.toThrow(/BadRequest/) + }) + + test('returns undefined when social key is not found', async () => { ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ extra: { authMethod: 'password' }, - account: 'user-uuid' + account: callerAccount + }) + const mockDb = makeMockDb({ socialId: null }) + + const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { + socialString: 'email:missing@example.com' + }) + + expect(result).toBeUndefined() + }) + + test('service token can look up any social key without a workspace check', async () => { + ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ + extra: { service: 'tool' } + }) + const mockDb = makeMockDb({ + socialId: { personUuid: 'looked-up-account' }, + targetAccountUuid: 'looked-up-account', + callerWorkspaces: [], + targetWorkspaces: [] }) - const mockDb = makeMockDb({ personUuid: 'looked-up-person' }) const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { socialString: 'email:alice@example.com' }) - expect(result).toBe('looked-up-person') - expect(mockDb.socialId.findOne).toHaveBeenCalledWith({ key: 'email:alice@example.com' }) + expect(result).toBe('looked-up-account') + expect(mockDb.getAccountWorkspaces).not.toHaveBeenCalled() }) - test('still rejects empty socialString', async () => { + test('admin token can look up any social key without a workspace check', async () => { + ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ + extra: { admin: 'true' }, + account: callerAccount + }) + const mockDb = makeMockDb({ + socialId: { personUuid: 'looked-up-account' }, + targetAccountUuid: 'looked-up-account' + }) + + const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { + socialString: 'email:alice@example.com' + }) + + expect(result).toBe('looked-up-account') + expect(mockDb.getAccountWorkspaces).not.toHaveBeenCalled() + }) + + test('user token returns the person when caller and target share an active workspace', async () => { ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ extra: { authMethod: 'password' }, - account: 'user-uuid' + account: callerAccount + }) + const mockDb = makeMockDb({ + socialId: { personUuid: 'looked-up-account' }, + targetAccountUuid: 'looked-up-account', + callerWorkspaces: [callerWorkspace], + targetWorkspaces: [callerWorkspace] }) - const mockDb = makeMockDb(null) - await expect( - findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { socialString: '' }) - ).rejects.toThrow(/BadRequest/) + const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { + socialString: 'email:alice@example.com' + }) + + expect(result).toBe('looked-up-account') + expect(mockDb.getAccountWorkspaces).toHaveBeenCalledWith(callerAccount) + expect(mockDb.getAccountWorkspaces).toHaveBeenCalledWith('looked-up-account') }) - test('returns undefined when social key is not found', async () => { + test('user token returns undefined when caller and target share no active workspace', async () => { ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ extra: { authMethod: 'password' }, - account: 'user-uuid' + account: callerAccount + }) + const mockDb = makeMockDb({ + socialId: { personUuid: 'looked-up-account' }, + targetAccountUuid: 'looked-up-account', + callerWorkspaces: [callerWorkspace], + targetWorkspaces: [otherWorkspace] }) - const mockDb = makeMockDb(null) const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { - socialString: 'email:missing@example.com' + socialString: 'email:alice@example.com' + }) + + expect(result).toBeUndefined() + }) + + test('user token returns undefined when caller has no active workspaces', async () => { + ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ + extra: { authMethod: 'password' }, + account: callerAccount + }) + const mockDb = makeMockDb({ + socialId: { personUuid: 'looked-up-account' }, + targetAccountUuid: 'looked-up-account', + callerWorkspaces: [], + targetWorkspaces: [callerWorkspace] + }) + + const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { + socialString: 'email:alice@example.com' }) expect(result).toBeUndefined() }) - test('with requireAccount=true returns the account uuid when the person has one', async () => { + test('user token returns undefined when the target person has no account', async () => { ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ extra: { authMethod: 'password' }, - account: 'user-uuid' + account: callerAccount + }) + const mockDb = makeMockDb({ + socialId: { personUuid: 'orphan-person' }, + targetAccountUuid: null, + callerWorkspaces: [callerWorkspace] + }) + + const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { + socialString: 'email:alice@example.com' + }) + + expect(result).toBeUndefined() + expect(mockDb.getAccountWorkspaces).not.toHaveBeenCalled() + }) + + test('user token with requireAccount=true returns the account uuid when shared workspace exists', async () => { + ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ + extra: { authMethod: 'password' }, + account: callerAccount + }) + const mockDb = makeMockDb({ + socialId: { personUuid: 'looked-up-account' }, + targetAccountUuid: 'looked-up-account', + callerWorkspaces: [callerWorkspace], + targetWorkspaces: [callerWorkspace] }) - const mockDb = makeMockDb({ personUuid: 'person-uuid' }, 'account-uuid') const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { socialString: 'email:alice@example.com', requireAccount: true }) - expect(result).toBe('account-uuid') + expect(result).toBe('looked-up-account') }) - test('with requireAccount=true returns undefined when the person has no account', async () => { + test('user token with requireAccount=true returns undefined when no shared workspace', async () => { ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ extra: { authMethod: 'password' }, - account: 'user-uuid' + account: callerAccount + }) + const mockDb = makeMockDb({ + socialId: { personUuid: 'looked-up-account' }, + targetAccountUuid: 'looked-up-account', + callerWorkspaces: [callerWorkspace], + targetWorkspaces: [otherWorkspace] }) - const mockDb = makeMockDb({ personUuid: 'person-uuid' }, null) const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { socialString: 'email:alice@example.com', @@ -1695,4 +1850,38 @@ describe('findPersonBySocialKey', () => { expect(result).toBeUndefined() }) + + test('user token throws Forbidden when the token has no account claim', async () => { + ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ + extra: { authMethod: 'password' } + }) + const mockDb = makeMockDb({ + socialId: { personUuid: 'looked-up-account' } + }) + + await expect( + findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { + socialString: 'email:alice@example.com' + }) + ).rejects.toThrow(/Forbidden/) + }) + + test('user token does not leak the existence of a non-shared person (no diff vs missing key)', async () => { + ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ + extra: { authMethod: 'password' }, + account: callerAccount + }) + const mockDb = makeMockDb({ + socialId: { personUuid: 'looked-up-account' }, + targetAccountUuid: 'looked-up-account', + callerWorkspaces: [callerWorkspace], + targetWorkspaces: [otherWorkspace] + }) + + const result = await findPersonBySocialKey(mockCtx, mockDb, mockBranding, mockToken, { + socialString: 'email:alice@example.com' + }) + + expect(result).toBeUndefined() + }) }) diff --git a/server/account/src/serviceOperations.ts b/server/account/src/serviceOperations.ts index 5295398da95..4d67e8acafa 100644 --- a/server/account/src/serviceOperations.ts +++ b/server/account/src/serviceOperations.ts @@ -997,16 +997,49 @@ export async function findPersonBySocialKey ( throw new PlatformError(new Status(Severity.ERROR, platform.status.BadRequest, {})) } + const { extra, account: callerAccount } = decodeTokenVerbose(ctx, token) + const isService = verifyAllowedServices(['tool', 'workspace', 'aibot', ...integrationServices], extra, false) + const socialId = await db.socialId.findOne({ key: socialString }) if (socialId == null) { return } - if (params.requireAccount === true) { - const account = await db.account.findOne({ uuid: socialId.personUuid as AccountUuid }) + if (isService || extra?.admin === 'true') { + if (params.requireAccount === true) { + const account = await db.account.findOne({ uuid: socialId.personUuid as AccountUuid }) + + return account?.uuid + } + + return socialId.personUuid + } + + if (callerAccount == null) { + throw new PlatformError(new Status(Severity.ERROR, platform.status.Forbidden, {})) + } + + const targetAccount = await db.account.findOne({ uuid: socialId.personUuid as AccountUuid }) - return account?.uuid + if (targetAccount == null) { + return + } + + const [callerWorkspaces, targetWorkspaces] = await Promise.all([ + db.getAccountWorkspaces(callerAccount), + db.getAccountWorkspaces(targetAccount.uuid) + ]) + + const callerActiveWs = new Set(callerWorkspaces.filter((w) => isActiveMode(w.status.mode)).map((w) => w.uuid)) + const shared = targetWorkspaces.some((w) => isActiveMode(w.status.mode) && callerActiveWs.has(w.uuid)) + + if (!shared) { + return + } + + if (params.requireAccount === true) { + return targetAccount.uuid } return socialId.personUuid