diff --git a/src/app/api/connections/[id]/route.ts b/src/app/api/connections/[id]/route.ts index 015e159..e1a209a 100644 --- a/src/app/api/connections/[id]/route.ts +++ b/src/app/api/connections/[id]/route.ts @@ -1,22 +1,34 @@ import { NextResponse } from "next/server" import { auth } from "@/auth" +import { requireSessionUserId } from "@/lib/api/require-session-user-id" import { connectorStore } from "@/lib/connector-store" +import type { Connection } from "@/lib/connector-types" -export async function GET(request: Request, { params }: { params: Promise<{ id: string }> }) { +type ConnectionUpdate = Partial> + +export async function GET(_request: Request, { params }: { params: Promise<{ id: string }> }) { const session = await auth() - if (!session) return NextResponse.json({ error: "Unauthorized" }, { status: 401 }) + const userIdOrError = requireSessionUserId(session) + if (userIdOrError instanceof NextResponse) return userIdOrError + const { id } = await params - const connections = connectorStore.getConnectionsByConnector(id) + // Path param is connectorId (legacy route shape). + const connections = connectorStore.getConnectionsByConnector(id, userIdOrError) return NextResponse.json(connections) } export async function PATCH(request: Request, { params }: { params: Promise<{ id: string }> }) { const session = await auth() - if (!session) return NextResponse.json({ error: "Unauthorized" }, { status: 401 }) + const userIdOrError = requireSessionUserId(session) + if (userIdOrError instanceof NextResponse) return userIdOrError const { id } = await params - const body = await request.json() - const connection = connectorStore.updateConnection(id, body) + const body = (await request.json()) as ConnectionUpdate & { + ownerUserId?: string + id?: string + } + const { ownerUserId: _ignoredOwner, id: _ignoredId, ...updates } = body + const connection = connectorStore.updateConnection(id, userIdOrError, updates) if (!connection) { return NextResponse.json({ error: "Connection not found" }, { status: 404 }) @@ -25,12 +37,13 @@ export async function PATCH(request: Request, { params }: { params: Promise<{ id return NextResponse.json(connection) } -export async function DELETE(request: Request, { params }: { params: Promise<{ id: string }> }) { +export async function DELETE(_request: Request, { params }: { params: Promise<{ id: string }> }) { const session = await auth() - if (!session) return NextResponse.json({ error: "Unauthorized" }, { status: 401 }) + const userIdOrError = requireSessionUserId(session) + if (userIdOrError instanceof NextResponse) return userIdOrError const { id } = await params - const success = connectorStore.deleteConnection(id) + const success = connectorStore.deleteConnection(id, userIdOrError) if (!success) { return NextResponse.json({ error: "Connection not found" }, { status: 404 }) diff --git a/src/app/api/connections/[id]/test/route.ts b/src/app/api/connections/[id]/test/route.ts index 7b965a7..e8db8af 100644 --- a/src/app/api/connections/[id]/test/route.ts +++ b/src/app/api/connections/[id]/test/route.ts @@ -1,12 +1,17 @@ import { NextResponse } from "next/server" import { auth } from "@/auth" +import { requireSessionUserId } from "@/lib/api/require-session-user-id" import { connectorStore } from "@/lib/connector-store" -export async function POST(request: Request, { params }: { params: Promise<{ id: string }> }) { +export async function POST(_request: Request, { params }: { params: Promise<{ id: string }> }) { const session = await auth() - if (!session) return NextResponse.json({ error: "Unauthorized" }, { status: 401 }) + const userIdOrError = requireSessionUserId(session) + if (userIdOrError instanceof NextResponse) return userIdOrError const { id } = await params - const result = await connectorStore.testConnection(id) + const result = await connectorStore.testConnection(id, userIdOrError) + if (!result.success && result.message === "Connection not found") { + return NextResponse.json({ error: "Connection not found" }, { status: 404 }) + } return NextResponse.json(result) } diff --git a/src/app/api/connections/route.ts b/src/app/api/connections/route.ts index 4d4d4e1..ee2a48f 100644 --- a/src/app/api/connections/route.ts +++ b/src/app/api/connections/route.ts @@ -1,20 +1,33 @@ import { NextResponse } from "next/server" import { auth } from "@/auth" +import { requireSessionUserId } from "@/lib/api/require-session-user-id" import { connectorStore } from "@/lib/connector-store" +import type { Connection } from "@/lib/connector-types" + +type ConnectionCreateBody = Omit export async function GET() { const session = await auth() - if (!session) return NextResponse.json({ error: "Unauthorized" }, { status: 401 }) + const userIdOrError = requireSessionUserId(session) + if (userIdOrError instanceof NextResponse) return userIdOrError - const connections = connectorStore.getConnections() + const connections = connectorStore.getConnections(userIdOrError) return NextResponse.json(connections) } export async function POST(request: Request) { const session = await auth() - if (!session) return NextResponse.json({ error: "Unauthorized" }, { status: 401 }) + const userIdOrError = requireSessionUserId(session) + if (userIdOrError instanceof NextResponse) return userIdOrError - const body = await request.json() - const connection = connectorStore.addConnection(body) + const body = (await request.json()) as Partial & { + ownerUserId?: string + } + // Ignore any client-supplied ownerUserId — ownership always comes from the session. + const { ownerUserId: _ignored, ...rest } = body + const connection = connectorStore.addConnection({ + ...(rest as ConnectionCreateBody), + ownerUserId: userIdOrError, + }) return NextResponse.json(connection, { status: 201 }) } diff --git a/src/app/api/oauth/callback/route.ts b/src/app/api/oauth/callback/route.ts index e722b3d..ffe6c27 100644 --- a/src/app/api/oauth/callback/route.ts +++ b/src/app/api/oauth/callback/route.ts @@ -1,5 +1,7 @@ import { cookies } from "next/headers" import { NextResponse } from "next/server" +import { auth } from "@/auth" +import { requireSessionUserId } from "@/lib/api/require-session-user-id" import { OAUTH_PKCE_COOKIE_NAME, getOAuthPkceCookieClearOptions, @@ -30,6 +32,15 @@ export async function GET(request: Request) { ) } + const session = await auth() + const userIdOrError = requireSessionUserId(session) + if (userIdOrError instanceof NextResponse) { + return withClearedPkceCookie( + NextResponse.redirect(new URL("/connectors?error=unauthorized", request.url)), + ) + } + const userId = userIdOrError + const cookieStore = await cookies() const pkceCookie = cookieStore.get(OAUTH_PKCE_COOKIE_NAME)?.value @@ -41,7 +52,8 @@ export async function GET(request: Request) { const tokens = await oauthManager.exchangeCodeForToken(code, oauthState) - const _connection = connectorStore.addConnection({ + connectorStore.addConnection({ + ownerUserId: userId, connectorId: oauthState.connectorId, name: `${oauthState.connectorId} Connection`, status: "connected", diff --git a/src/app/api/oauth/refresh/route.ts b/src/app/api/oauth/refresh/route.ts index 32f907b..cfb5622 100644 --- a/src/app/api/oauth/refresh/route.ts +++ b/src/app/api/oauth/refresh/route.ts @@ -1,18 +1,23 @@ import { NextResponse } from "next/server" import { auth } from "@/auth" +import { requireSessionUserId } from "@/lib/api/require-session-user-id" import { oauthManager } from "@/lib/oauth-manager" import { connectorStore } from "@/lib/connector-store" export async function POST(request: Request) { const session = await auth() - if (!session) return NextResponse.json({ error: "Unauthorized" }, { status: 401 }) + const userIdOrError = requireSessionUserId(session) + if (userIdOrError instanceof NextResponse) return userIdOrError const body = await request.json() const { connectionId } = body + if (typeof connectionId !== "string" || connectionId.length === 0) { + return NextResponse.json({ error: "connectionId must be a non-empty string" }, { status: 400 }) + } + try { - const connections = connectorStore.getConnections() - const connection = connections.find((c) => c.id === connectionId) + const connection = connectorStore.getConnectionById(connectionId, userIdOrError) if (!connection) { return NextResponse.json({ error: "Connection not found" }, { status: 404 }) @@ -27,7 +32,7 @@ export async function POST(request: Request) { connection.config.credentials.refreshToken, ) - connectorStore.updateConnection(connectionId, { + connectorStore.updateConnection(connectionId, userIdOrError, { config: { ...connection.config, credentials: { diff --git a/src/lib/api/require-session-user-id.ts b/src/lib/api/require-session-user-id.ts new file mode 100644 index 0000000..b017c9c --- /dev/null +++ b/src/lib/api/require-session-user-id.ts @@ -0,0 +1,15 @@ +import { NextResponse } from "next/server" + +/** + * Require an authenticated session with a non-empty user id. + * Callers must not trust client-supplied owner ids for connection ownership. + */ +export function requireSessionUserId( + session: { user?: { id?: string | null } } | null, +): string | NextResponse { + const userId = session?.user?.id + if (!session || typeof userId !== "string" || userId.length === 0) { + return NextResponse.json({ error: "Unauthorized" }, { status: 401 }) + } + return userId +} diff --git a/src/lib/connector-store.ts b/src/lib/connector-store.ts index f370234..9f86dd3 100644 --- a/src/lib/connector-store.ts +++ b/src/lib/connector-store.ts @@ -129,15 +129,25 @@ class ConnectorStore { return this.connectors.filter((c) => c.category === category) } - getConnections(): Connection[] { - return this.connections + getConnections(ownerUserId: string): Connection[] { + return this.connections.filter((c) => c.ownerUserId === ownerUserId) } - getConnectionsByConnector(connectorId: string): Connection[] { - return this.connections.filter((c) => c.connectorId === connectorId) + getConnectionsByConnector(connectorId: string, ownerUserId: string): Connection[] { + return this.connections.filter( + (c) => c.connectorId === connectorId && c.ownerUserId === ownerUserId, + ) + } + + getConnectionById(id: string, ownerUserId: string): Connection | undefined { + return this.connections.find((c) => c.id === id && c.ownerUserId === ownerUserId) } addConnection(connection: Omit): Connection { + if (!connection.ownerUserId) { + throw new Error("ownerUserId is required when adding a connection") + } + const newConnection: Connection = { ...connection, id: `conn-${Date.now()}-${Math.random().toString(36).substring(7)}`, @@ -153,16 +163,20 @@ class ConnectorStore { return newConnection } - updateConnection(id: string, updates: Partial): Connection | null { - const index = this.connections.findIndex((c) => c.id === id) + updateConnection( + id: string, + ownerUserId: string, + updates: Partial>, + ): Connection | null { + const index = this.connections.findIndex((c) => c.id === id && c.ownerUserId === ownerUserId) if (index === -1) return null this.connections[index] = { ...this.connections[index], ...updates } return this.connections[index] } - deleteConnection(id: string): boolean { - const connection = this.connections.find((c) => c.id === id) + deleteConnection(id: string, ownerUserId: string): boolean { + const connection = this.connections.find((c) => c.id === id && c.ownerUserId === ownerUserId) if (!connection) return false this.connections = this.connections.filter((c) => c.id !== id) @@ -180,7 +194,12 @@ class ConnectorStore { return true } - testConnection(_id: string): Promise<{ success: boolean; message: string }> { + testConnection(id: string, ownerUserId: string): Promise<{ success: boolean; message: string }> { + const owned = this.getConnectionById(id, ownerUserId) + if (!owned) { + return Promise.resolve({ success: false, message: "Connection not found" }) + } + return new Promise((resolve) => { setTimeout(() => { resolve({ @@ -190,6 +209,11 @@ class ConnectorStore { }, 1500) }) } + + /** Test-only: drop all stored connections between cases. */ + clearConnections(): void { + this.connections = [] + } } export const connectorStore = new ConnectorStore() diff --git a/src/lib/connector-types.ts b/src/lib/connector-types.ts index 2cbb2cf..f392213 100644 --- a/src/lib/connector-types.ts +++ b/src/lib/connector-types.ts @@ -26,6 +26,8 @@ export interface Connector { export interface Connection { id: string + /** Authenticated user that owns this connection; required for tenant isolation. */ + ownerUserId: string connectorId: string name: string status: ConnectionStatus diff --git a/tests/api/connections-route.test.ts b/tests/api/connections-route.test.ts new file mode 100644 index 0000000..30bbb55 --- /dev/null +++ b/tests/api/connections-route.test.ts @@ -0,0 +1,112 @@ +import { beforeEach, describe, expect, it, vi } from "vitest" +import { GET, POST } from "@/app/api/connections/route" +import { DELETE, PATCH } from "@/app/api/connections/[id]/route" +import { connectorStore } from "@/lib/connector-store" + +const authMock = vi.fn() + +vi.mock("@/auth", () => ({ + auth: () => authMock(), +})) + +describe("api/connections ownership", () => { + beforeEach(() => { + vi.clearAllMocks() + connectorStore.clearConnections() + }) + + it("returns 401 when unauthenticated", async () => { + authMock.mockResolvedValue(null) + const res = await GET() + expect(res.status).toBe(401) + }) + + it("does not leak another user's credentials on GET", async () => { + connectorStore.addConnection({ + ownerUserId: "user-a", + connectorId: "openai", + name: "A OpenAI", + status: "connected", + config: { + authType: "api_key", + credentials: { apiKey: "sk-secret-a" }, + }, + }) + connectorStore.addConnection({ + ownerUserId: "user-b", + connectorId: "openai", + name: "B OpenAI", + status: "connected", + config: { + authType: "api_key", + credentials: { apiKey: "sk-secret-b" }, + }, + }) + + authMock.mockResolvedValue({ user: { id: "user-b" } }) + const res = await GET() + expect(res.status).toBe(200) + const body = await res.json() + expect(body).toHaveLength(1) + expect(body[0].config.credentials.apiKey).toBe("sk-secret-b") + expect(body[0].ownerUserId).toBe("user-b") + }) + + it("ignores client-supplied ownerUserId on POST", async () => { + authMock.mockResolvedValue({ user: { id: "user-real" } }) + const res = await POST( + new Request("http://localhost/api/connections", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + ownerUserId: "user-attacker", + connectorId: "openai", + name: "Spoofed", + status: "connected", + config: { + authType: "api_key", + credentials: { apiKey: "sk-new" }, + }, + }), + }), + ) + expect(res.status).toBe(201) + const body = await res.json() + expect(body.ownerUserId).toBe("user-real") + expect(connectorStore.getConnections("user-attacker")).toHaveLength(0) + expect(connectorStore.getConnections("user-real")).toHaveLength(1) + }) + + it("returns 404 when a non-owner tries to PATCH or DELETE", async () => { + const owned = connectorStore.addConnection({ + ownerUserId: "user-a", + connectorId: "openai", + name: "A OpenAI", + status: "connected", + config: { + authType: "api_key", + credentials: { apiKey: "sk-secret-a" }, + }, + }) + + authMock.mockResolvedValue({ user: { id: "user-b" } }) + const params = Promise.resolve({ id: owned.id }) + + const patchRes = await PATCH( + new Request(`http://localhost/api/connections/${owned.id}`, { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ name: "Hijacked" }), + }), + { params }, + ) + expect(patchRes.status).toBe(404) + + const deleteRes = await DELETE( + new Request(`http://localhost/api/connections/${owned.id}`, { method: "DELETE" }), + { params }, + ) + expect(deleteRes.status).toBe(404) + expect(connectorStore.getConnectionById(owned.id, "user-a")?.name).toBe("A OpenAI") + }) +}) diff --git a/tests/api/oauth-refresh-route.test.ts b/tests/api/oauth-refresh-route.test.ts new file mode 100644 index 0000000..ac1e8de --- /dev/null +++ b/tests/api/oauth-refresh-route.test.ts @@ -0,0 +1,84 @@ +import { beforeEach, describe, expect, it, vi } from "vitest" +import { POST } from "@/app/api/oauth/refresh/route" +import { connectorStore } from "@/lib/connector-store" + +const authMock = vi.fn() +const refreshAccessToken = vi.fn() + +vi.mock("@/auth", () => ({ + auth: () => authMock(), +})) + +vi.mock("@/lib/oauth-manager", () => ({ + oauthManager: { + refreshAccessToken: (...args: unknown[]) => refreshAccessToken(...args), + }, +})) + +describe("api/oauth/refresh ownership", () => { + beforeEach(() => { + vi.clearAllMocks() + connectorStore.clearConnections() + }) + + it("returns 404 when refreshing another user's connection", async () => { + const owned = connectorStore.addConnection({ + ownerUserId: "user-a", + connectorId: "github", + name: "A GitHub", + status: "connected", + config: { + authType: "oauth2", + credentials: { + accessToken: "access-a", + refreshToken: "refresh-a", + }, + }, + }) + + authMock.mockResolvedValue({ user: { id: "user-b" } }) + const res = await POST( + new Request("http://localhost/api/oauth/refresh", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ connectionId: owned.id }), + }), + ) + + expect(res.status).toBe(404) + expect(refreshAccessToken).not.toHaveBeenCalled() + }) + + it("refreshes tokens for the owning user", async () => { + const owned = connectorStore.addConnection({ + ownerUserId: "user-a", + connectorId: "github", + name: "A GitHub", + status: "connected", + config: { + authType: "oauth2", + credentials: { + accessToken: "access-a", + refreshToken: "refresh-a", + }, + }, + }) + + authMock.mockResolvedValue({ user: { id: "user-a" } }) + refreshAccessToken.mockResolvedValue({ accessToken: "access-new" }) + + const res = await POST( + new Request("http://localhost/api/oauth/refresh", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ connectionId: owned.id }), + }), + ) + + expect(res.status).toBe(200) + expect(refreshAccessToken).toHaveBeenCalledWith("github", "refresh-a") + expect( + connectorStore.getConnectionById(owned.id, "user-a")?.config.credentials?.accessToken, + ).toBe("access-new") + }) +}) diff --git a/tests/lib/connector-store.test.ts b/tests/lib/connector-store.test.ts new file mode 100644 index 0000000..fe31102 --- /dev/null +++ b/tests/lib/connector-store.test.ts @@ -0,0 +1,70 @@ +import { beforeEach, describe, expect, it } from "vitest" +import { connectorStore } from "@/lib/connector-store" + +describe("connectorStore ownership isolation", () => { + beforeEach(() => { + connectorStore.clearConnections() + }) + + it("does not return another user's connections on list", () => { + connectorStore.addConnection({ + ownerUserId: "user-a", + connectorId: "openai", + name: "A OpenAI", + status: "connected", + config: { + authType: "api_key", + credentials: { apiKey: "sk-secret-a" }, + }, + }) + connectorStore.addConnection({ + ownerUserId: "user-b", + connectorId: "anthropic", + name: "B Anthropic", + status: "connected", + config: { + authType: "api_key", + credentials: { apiKey: "sk-secret-b" }, + }, + }) + + const forA = connectorStore.getConnections("user-a") + expect(forA).toHaveLength(1) + expect(forA[0]?.config.credentials?.apiKey).toBe("sk-secret-a") + expect(connectorStore.getConnections("user-b")).toHaveLength(1) + expect(connectorStore.getConnections("user-c")).toHaveLength(0) + }) + + it("rejects update and delete from a non-owner", () => { + const owned = connectorStore.addConnection({ + ownerUserId: "user-a", + connectorId: "openai", + name: "A OpenAI", + status: "connected", + config: { + authType: "api_key", + credentials: { apiKey: "sk-secret-a" }, + }, + }) + + expect( + connectorStore.updateConnection(owned.id, "user-b", { + name: "Hijacked", + }), + ).toBeNull() + expect(connectorStore.deleteConnection(owned.id, "user-b")).toBe(false) + expect(connectorStore.getConnectionById(owned.id, "user-a")?.name).toBe("A OpenAI") + }) + + it("requires ownerUserId when adding a connection", () => { + expect(() => + connectorStore.addConnection({ + ownerUserId: "", + connectorId: "openai", + name: "Missing owner", + status: "connected", + config: { authType: "api_key" }, + }), + ).toThrow(/ownerUserId/) + }) +})