Skip to content

Commit ab92fd0

Browse files
sweetmantechclaude
sweetmantech
and
claude
authored
feat: accept api_key for authentication in create_new_artist MCP tool (#118)
* feat: accept api_key for authentication in create_new_artist MCP tool - Add optional api_key parameter to resolve account_id automatically - Use getApiKeyDetails to validate API key and get accountId - Support account_id override for org API keys with access validation - Return clear error messages for invalid keys or access denied - Add comprehensive tests for new authentication flow Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: move API key authentication to MCP server level - Implement withMcpAuth wrapper for MCP server-level authentication - Remove api_key parameter from create_new_artist tool schema - Tool now receives accountId via extra.authInfo from the auth layer - MCP server accepts API key via Authorization: Bearer or x-api-key header - Update tests to use new auth flow with mock authInfo Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat: require API key authentication for MCP server Unauthenticated requests will now receive a 401 response. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: extract verifyApiKey to lib/mcp/verifyApiKey.ts Follow SRP - move auth verification to its own module. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: use Authorization: Bearer header for MCP API key auth Remove x-api-key header support, use only Authorization: Bearer. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: add McpAuthInfo type for typed auth info access Export McpAuthInfo and McpAuthInfoExtra types from verifyApiKey.ts for use in tool handlers. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: extract resolveAccountId to lib/mcp/resolveAccountId.ts Move account resolution logic with org access validation to its own module. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: add index signature to McpAuthInfoExtra for AuthInfo compatibility Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 7409482 commit ab92fd0

File tree

5 files changed

+302
-55
lines changed

5 files changed

+302
-55
lines changed

‎app/mcp/route.ts‎

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,23 @@
11
import { registerAllTools } from "@/lib/mcp/tools";
2-
import { createMcpHandler } from "mcp-handler";
2+
import { createMcpHandler, withMcpAuth } from "mcp-handler";
3+
import { verifyApiKey } from "@/lib/mcp/verifyApiKey";
34

4-
let handler: ReturnType<typeof createMcpHandler> | null = null;
5+
const baseHandler = createMcpHandler(
6+
server => {
7+
registerAllTools(server);
8+
},
9+
{
10+
serverInfo: {
11+
name: "recoup-mcp",
12+
version: "0.0.1",
13+
},
14+
},
15+
);
516

6-
/**
7-
* Gets the MCP handler for the API.
8-
*
9-
* @returns The MCP handler.
10-
*/
11-
async function getHandler(): Promise<ReturnType<typeof createMcpHandler>> {
12-
if (!handler) {
13-
handler = createMcpHandler(
14-
server => {
15-
registerAllTools(server);
16-
},
17-
{
18-
serverInfo: {
19-
name: "recoup-mcp",
20-
version: "0.0.1",
21-
},
22-
},
23-
);
24-
}
25-
return handler;
26-
}
17+
// Wrap with auth - API key is required for all MCP requests
18+
const handler = withMcpAuth(baseHandler, verifyApiKey, {
19+
required: true,
20+
});
2721

2822
/**
2923
* GET handler for the MCP API.
@@ -32,7 +26,6 @@ async function getHandler(): Promise<ReturnType<typeof createMcpHandler>> {
3226
* @returns The response from the MCP handler.
3327
*/
3428
export async function GET(req: Request) {
35-
const handler = await getHandler();
3629
return handler(req);
3730
}
3831

@@ -43,6 +36,5 @@ export async function GET(req: Request) {
4336
* @returns The response from the MCP handler.
4437
*/
4538
export async function POST(req: Request) {
46-
const handler = await getHandler();
4739
return handler(req);
4840
}

‎lib/mcp/resolveAccountId.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { canAccessAccount } from "@/lib/organizations/canAccessAccount";
2+
import type { McpAuthInfo } from "@/lib/mcp/verifyApiKey";
3+
4+
export interface ResolveAccountIdParams {
5+
authInfo: McpAuthInfo | undefined;
6+
accountIdOverride: string | undefined;
7+
}
8+
9+
export interface ResolveAccountIdResult {
10+
accountId: string | null;
11+
error: string | null;
12+
}
13+
14+
/**
15+
* Resolves the accountId from MCP auth info or an override parameter.
16+
* Validates access when an org API key attempts to use an account_id override.
17+
*
18+
* @param params - The auth info and optional account_id override.
19+
* @returns The resolved accountId or an error message.
20+
*/
21+
export async function resolveAccountId({
22+
authInfo,
23+
accountIdOverride,
24+
}: ResolveAccountIdParams): Promise<ResolveAccountIdResult> {
25+
const authAccountId = authInfo?.extra?.accountId;
26+
const authOrgId = authInfo?.extra?.orgId;
27+
28+
if (authAccountId) {
29+
// If account_id override is provided, validate access (for org API keys)
30+
if (accountIdOverride && accountIdOverride !== authAccountId) {
31+
const hasAccess = await canAccessAccount({
32+
orgId: authOrgId,
33+
targetAccountId: accountIdOverride,
34+
});
35+
if (!hasAccess) {
36+
return { accountId: null, error: "Access denied to specified account_id" };
37+
}
38+
return { accountId: accountIdOverride, error: null };
39+
}
40+
return { accountId: authAccountId, error: null };
41+
}
42+
43+
if (accountIdOverride) {
44+
return { accountId: accountIdOverride, error: null };
45+
}
46+
47+
return {
48+
accountId: null,
49+
error:
50+
"Authentication required. Provide an API key via Authorization: Bearer header, or provide account_id from the system prompt context.",
51+
};
52+
}

‎lib/mcp/tools/artists/__tests__/registerCreateNewArtistTool.test.ts‎

Lines changed: 166 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import { describe, it, expect, vi, beforeEach } from "vitest";
22
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
3+
import type { RequestHandlerExtra } from "@modelcontextprotocol/sdk/shared/protocol.js";
4+
import type { ServerRequest, ServerNotification } from "@modelcontextprotocol/sdk/types.js";
35

46
const mockCreateArtistInDb = vi.fn();
57
const mockCopyRoom = vi.fn();
8+
const mockCanAccessAccount = vi.fn();
69

710
vi.mock("@/lib/artists/createArtistInDb", () => ({
811
createArtistInDb: (...args: unknown[]) => mockCreateArtistInDb(...args),
@@ -12,11 +15,39 @@ vi.mock("@/lib/rooms/copyRoom", () => ({
1215
copyRoom: (...args: unknown[]) => mockCopyRoom(...args),
1316
}));
1417

18+
vi.mock("@/lib/organizations/canAccessAccount", () => ({
19+
canAccessAccount: (...args: unknown[]) => mockCanAccessAccount(...args),
20+
}));
21+
1522
import { registerCreateNewArtistTool } from "../registerCreateNewArtistTool";
1623

24+
type ServerRequestHandlerExtra = RequestHandlerExtra<ServerRequest, ServerNotification>;
25+
26+
/**
27+
* Creates a mock extra object with optional authInfo.
28+
*/
29+
function createMockExtra(authInfo?: {
30+
accountId?: string;
31+
orgId?: string | null;
32+
}): ServerRequestHandlerExtra {
33+
return {
34+
authInfo: authInfo
35+
? {
36+
token: "test-token",
37+
scopes: ["mcp:tools"],
38+
clientId: authInfo.accountId,
39+
extra: {
40+
accountId: authInfo.accountId,
41+
orgId: authInfo.orgId ?? null,
42+
},
43+
}
44+
: undefined,
45+
} as unknown as ServerRequestHandlerExtra;
46+
}
47+
1748
describe("registerCreateNewArtistTool", () => {
1849
let mockServer: McpServer;
19-
let registeredHandler: (args: unknown) => Promise<unknown>;
50+
let registeredHandler: (args: unknown, extra: ServerRequestHandlerExtra) => Promise<unknown>;
2051

2152
beforeEach(() => {
2253
vi.clearAllMocks();
@@ -40,7 +71,7 @@ describe("registerCreateNewArtistTool", () => {
4071
);
4172
});
4273

43-
it("creates an artist and returns success", async () => {
74+
it("creates an artist and returns success with account_id", async () => {
4475
const mockArtist = {
4576
id: "artist-123",
4677
account_id: "artist-123",
@@ -50,10 +81,13 @@ describe("registerCreateNewArtistTool", () => {
5081
};
5182
mockCreateArtistInDb.mockResolvedValue(mockArtist);
5283

53-
const result = await registeredHandler({
54-
name: "Test Artist",
55-
account_id: "owner-456",
56-
});
84+
const result = await registeredHandler(
85+
{
86+
name: "Test Artist",
87+
account_id: "owner-456",
88+
},
89+
createMockExtra(),
90+
);
5791

5892
expect(mockCreateArtistInDb).toHaveBeenCalledWith("Test Artist", "owner-456", undefined);
5993
expect(result).toEqual({
@@ -66,6 +100,34 @@ describe("registerCreateNewArtistTool", () => {
66100
});
67101
});
68102

103+
it("creates an artist using auth info accountId", async () => {
104+
const mockArtist = {
105+
id: "artist-123",
106+
account_id: "artist-123",
107+
name: "Test Artist",
108+
account_info: [{ image: null }],
109+
account_socials: [],
110+
};
111+
mockCreateArtistInDb.mockResolvedValue(mockArtist);
112+
113+
const result = await registeredHandler(
114+
{
115+
name: "Test Artist",
116+
},
117+
createMockExtra({ accountId: "auth-account-123" }),
118+
);
119+
120+
expect(mockCreateArtistInDb).toHaveBeenCalledWith("Test Artist", "auth-account-123", undefined);
121+
expect(result).toEqual({
122+
content: [
123+
{
124+
type: "text",
125+
text: expect.stringContaining("Successfully created artist"),
126+
},
127+
],
128+
});
129+
});
130+
69131
it("copies room when active_conversation_id is provided", async () => {
70132
const mockArtist = {
71133
id: "artist-123",
@@ -77,11 +139,14 @@ describe("registerCreateNewArtistTool", () => {
77139
mockCreateArtistInDb.mockResolvedValue(mockArtist);
78140
mockCopyRoom.mockResolvedValue("new-room-789");
79141

80-
const result = await registeredHandler({
81-
name: "Test Artist",
82-
account_id: "owner-456",
83-
active_conversation_id: "source-room-111",
84-
});
142+
const result = await registeredHandler(
143+
{
144+
name: "Test Artist",
145+
account_id: "owner-456",
146+
active_conversation_id: "source-room-111",
147+
},
148+
createMockExtra(),
149+
);
85150

86151
expect(mockCopyRoom).toHaveBeenCalledWith("source-room-111", "artist-123");
87152
expect(result).toEqual({
@@ -104,22 +169,28 @@ describe("registerCreateNewArtistTool", () => {
104169
};
105170
mockCreateArtistInDb.mockResolvedValue(mockArtist);
106171

107-
await registeredHandler({
108-
name: "Test Artist",
109-
account_id: "owner-456",
110-
organization_id: "org-999",
111-
});
172+
await registeredHandler(
173+
{
174+
name: "Test Artist",
175+
account_id: "owner-456",
176+
organization_id: "org-999",
177+
},
178+
createMockExtra(),
179+
);
112180

113181
expect(mockCreateArtistInDb).toHaveBeenCalledWith("Test Artist", "owner-456", "org-999");
114182
});
115183

116184
it("returns error when artist creation fails", async () => {
117185
mockCreateArtistInDb.mockResolvedValue(null);
118186

119-
const result = await registeredHandler({
120-
name: "Test Artist",
121-
account_id: "owner-456",
122-
});
187+
const result = await registeredHandler(
188+
{
189+
name: "Test Artist",
190+
account_id: "owner-456",
191+
},
192+
createMockExtra(),
193+
);
123194

124195
expect(result).toEqual({
125196
content: [
@@ -134,16 +205,88 @@ describe("registerCreateNewArtistTool", () => {
134205
it("returns error with message when exception is thrown", async () => {
135206
mockCreateArtistInDb.mockRejectedValue(new Error("Database connection failed"));
136207

137-
const result = await registeredHandler({
208+
const result = await registeredHandler(
209+
{
210+
name: "Test Artist",
211+
account_id: "owner-456",
212+
},
213+
createMockExtra(),
214+
);
215+
216+
expect(result).toEqual({
217+
content: [
218+
{
219+
type: "text",
220+
text: expect.stringContaining("Database connection failed"),
221+
},
222+
],
223+
});
224+
});
225+
226+
it("allows account_id override for org auth with access", async () => {
227+
mockCanAccessAccount.mockResolvedValue(true);
228+
const mockArtist = {
229+
id: "artist-123",
230+
account_id: "artist-123",
138231
name: "Test Artist",
139-
account_id: "owner-456",
232+
account_info: [{ image: null }],
233+
account_socials: [],
234+
};
235+
mockCreateArtistInDb.mockResolvedValue(mockArtist);
236+
237+
await registeredHandler(
238+
{
239+
name: "Test Artist",
240+
account_id: "target-account-456",
241+
},
242+
createMockExtra({ accountId: "org-account-id", orgId: "org-account-id" }),
243+
);
244+
245+
expect(mockCanAccessAccount).toHaveBeenCalledWith({
246+
orgId: "org-account-id",
247+
targetAccountId: "target-account-456",
140248
});
249+
expect(mockCreateArtistInDb).toHaveBeenCalledWith(
250+
"Test Artist",
251+
"target-account-456",
252+
undefined,
253+
);
254+
});
255+
256+
it("returns error when org auth lacks access to account_id", async () => {
257+
mockCanAccessAccount.mockResolvedValue(false);
258+
259+
const result = await registeredHandler(
260+
{
261+
name: "Test Artist",
262+
account_id: "target-account-456",
263+
},
264+
createMockExtra({ accountId: "org-account-id", orgId: "org-account-id" }),
265+
);
141266

142267
expect(result).toEqual({
143268
content: [
144269
{
145270
type: "text",
146-
text: expect.stringContaining("Database connection failed"),
271+
text: expect.stringContaining("Access denied to specified account_id"),
272+
},
273+
],
274+
});
275+
});
276+
277+
it("returns error when neither auth nor account_id is provided", async () => {
278+
const result = await registeredHandler(
279+
{
280+
name: "Test Artist",
281+
},
282+
createMockExtra(),
283+
);
284+
285+
expect(result).toEqual({
286+
content: [
287+
{
288+
type: "text",
289+
text: expect.stringContaining("Authentication required"),
147290
},
148291
],
149292
});

0 commit comments

Comments
 (0)