misc: addressed comments

This commit is contained in:
Sheen Capadngan
2025-12-19 18:06:51 +08:00
parent d50ef8cb64
commit 474931d029
9 changed files with 112 additions and 78 deletions

View File

@@ -40,7 +40,7 @@ const requireMcpAuthHook = (
return; return;
} }
if (auth.authMode === AuthMode.MCP_JWT && !req.permission.orgId) { if (!req.permission.orgId) {
void reply.status(401).send({ message: "Unauthorized: organization context required" }); void reply.status(401).send({ message: "Unauthorized: organization context required" });
return; return;
} }
@@ -56,7 +56,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1) endpointId: z.string().uuid().trim().min(1)
}) })
}, },
url: "/:endpointId/connect", url: "/:endpointId/connect",
@@ -64,11 +64,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
handler: async (req, res) => { handler: async (req, res) => {
await res.hijack(); // allow manual control of the underlying res await res.hijack(); // allow manual control of the underlying res
if (req.auth.authMode !== AuthMode.MCP_JWT) { if (req.auth.authMode === AuthMode.MCP_JWT && req.params.endpointId !== req.auth.token.mcp?.endpointId) {
throw new UnauthorizedError({ message: "Unauthorized" });
}
if (req.params.endpointId !== req.auth.token.mcp?.endpointId) {
throw new UnauthorizedError({ message: "Unauthorized" }); throw new UnauthorizedError({ message: "Unauthorized" });
} }
@@ -120,7 +116,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1) endpointId: z.string().uuid().trim().min(1)
}) })
}, },
onRequest: (req, reply, done) => requireMcpAuthHook(req, reply, done, req.params.endpointId), onRequest: (req, reply, done) => requireMcpAuthHook(req, reply, done, req.params.endpointId),
@@ -147,7 +143,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
body: z.object({ body: z.object({
projectId: z.string().trim().min(1), projectId: z.string().uuid().trim().min(1),
name: z.string().trim().min(1).max(64), name: z.string().trim().min(1).max(64),
description: z.string().trim().max(256).optional(), description: z.string().trim().max(256).optional(),
serverIds: z.array(z.string().uuid()).default([]) serverIds: z.array(z.string().uuid()).default([])
@@ -197,7 +193,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
querystring: z.object({ querystring: z.object({
projectId: z.string().trim().min(1) projectId: z.string().uuid().trim().min(1)
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -248,7 +244,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1) endpointId: z.string().uuid().trim().min(1)
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -295,7 +291,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1) endpointId: z.string().uuid().trim().min(1)
}), }),
body: z.object({ body: z.object({
name: z.string().trim().min(1).max(64).optional(), name: z.string().trim().min(1).max(64).optional(),
@@ -347,7 +343,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1) endpointId: z.string().uuid().trim().min(1)
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -432,8 +428,8 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1), endpointId: z.string().uuid().trim().min(1),
serverToolId: z.string().trim().min(1) serverToolId: z.string().uuid().trim().min(1)
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -478,8 +474,8 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1), endpointId: z.string().uuid().trim().min(1),
serverToolId: z.string().trim().min(1) serverToolId: z.string().uuid().trim().min(1)
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -524,7 +520,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1) endpointId: z.string().uuid().trim().min(1)
}), }),
body: z.object({ body: z.object({
tools: z.array( tools: z.array(
@@ -620,10 +616,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
} }
}); });
// Return without projectId/endpointName in response (not part of OAuth spec) return payload;
// eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/naming-convention
const { projectId: __projectId, endpointName: __endpointName, ...responsePayload } = payload;
return responsePayload;
} }
}); });
@@ -636,7 +629,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1) endpointId: z.string().uuid().trim().min(1)
}), }),
querystring: z.object({ querystring: z.object({
response_type: z.string(), response_type: z.string(),
@@ -670,7 +663,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1) endpointId: z.string().uuid().trim().min(1)
}), }),
body: z.object({ body: z.object({
response_type: z.string(), response_type: z.string(),
@@ -738,7 +731,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1) endpointId: z.string().uuid().trim().min(1)
}), }),
body: z.object({ body: z.object({
grant_type: z.literal("authorization_code"), grant_type: z.literal("authorization_code"),
@@ -774,7 +767,7 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1) endpointId: z.string().uuid().trim().min(1)
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -812,8 +805,8 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1), endpointId: z.string().uuid().trim().min(1),
serverId: z.string().trim().min(1) serverId: z.string().uuid().trim().min(1)
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -846,8 +839,8 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
schema: { schema: {
params: z.object({ params: z.object({
endpointId: z.string().trim().min(1), endpointId: z.string().uuid().trim().min(1),
serverId: z.string().trim().min(1) serverId: z.string().uuid().trim().min(1)
}), }),
body: z.object({ body: z.object({
accessToken: z.string().min(1), accessToken: z.string().min(1),
@@ -863,17 +856,32 @@ export const registerAiMcpEndpointRouter = async (server: FastifyZodProvider) =>
}, },
onRequest: verifyAuth([AuthMode.JWT]), onRequest: verifyAuth([AuthMode.JWT]),
handler: async (req) => { handler: async (req) => {
const result = await server.services.aiMcpEndpoint.saveUserServerCredential({ const { success, projectId, endpointName, serverName } =
endpointId: req.params.endpointId, await server.services.aiMcpEndpoint.saveUserServerCredential({
serverId: req.params.serverId, endpointId: req.params.endpointId,
...req.body, serverId: req.params.serverId,
actor: req.permission.type, ...req.body,
actorId: req.permission.id, actor: req.permission.type,
actorAuthMethod: req.permission.authMethod, actorId: req.permission.id,
actorOrgId: req.permission.orgId actorAuthMethod: req.permission.authMethod,
actorOrgId: req.permission.orgId
});
await server.services.auditLog.createAuditLog({
...req.auditLogInfo,
projectId,
event: {
type: EventType.MCP_ENDPOINT_SAVE_USER_CREDENTIAL,
metadata: {
endpointId: req.params.endpointId,
endpointName,
serverId: req.params.serverId,
serverName
}
}
}); });
return result; return { success };
} }
}); });
}; };

View File

@@ -10,7 +10,7 @@ import { AuthMode } from "@app/services/auth/auth-type";
// Common fields for MCP server creation // Common fields for MCP server creation
const CreateMcpServerBaseSchema = z.object({ const CreateMcpServerBaseSchema = z.object({
projectId: z.string().trim().min(1), projectId: z.string().uuid().trim().min(1),
name: z.string().trim().min(1).max(64), name: z.string().trim().min(1).max(64),
url: z.string().trim().url(), url: z.string().trim().url(),
description: z.string().trim().max(256).optional(), description: z.string().trim().max(256).optional(),
@@ -53,7 +53,7 @@ export const registerAiMcpServerRouter = async (server: FastifyZodProvider) => {
}, },
schema: { schema: {
body: z.object({ body: z.object({
projectId: z.string().trim().min(1), projectId: z.string().uuid().trim().min(1),
url: z.string().trim().url(), url: z.string().trim().url(),
clientId: z.string().trim().min(1).optional(), clientId: z.string().trim().min(1).optional(),
clientSecret: z.string().trim().min(1).optional() clientSecret: z.string().trim().min(1).optional()
@@ -137,7 +137,7 @@ export const registerAiMcpServerRouter = async (server: FastifyZodProvider) => {
}, },
schema: { schema: {
params: z.object({ params: z.object({
sessionId: z.string() sessionId: z.string().uuid().trim().min(1)
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -217,9 +217,7 @@ export const registerAiMcpServerRouter = async (server: FastifyZodProvider) => {
}, },
schema: { schema: {
querystring: z.object({ querystring: z.object({
projectId: z.string().trim().min(1), projectId: z.string().uuid().trim().min(1)
limit: z.coerce.number().min(1).max(100).default(100),
offset: z.coerce.number().min(0).default(0)
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -264,7 +262,7 @@ export const registerAiMcpServerRouter = async (server: FastifyZodProvider) => {
}, },
schema: { schema: {
params: z.object({ params: z.object({
serverId: z.string().trim().min(1) serverId: z.string().uuid().trim().min(1)
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -306,7 +304,7 @@ export const registerAiMcpServerRouter = async (server: FastifyZodProvider) => {
}, },
schema: { schema: {
params: z.object({ params: z.object({
serverId: z.string().trim().min(1) serverId: z.string().uuid().trim().min(1)
}), }),
body: z.object({ body: z.object({
name: z.string().trim().min(1).max(64).optional(), name: z.string().trim().min(1).max(64).optional(),
@@ -353,7 +351,7 @@ export const registerAiMcpServerRouter = async (server: FastifyZodProvider) => {
}, },
schema: { schema: {
params: z.object({ params: z.object({
serverId: z.string().trim().min(1) serverId: z.string().uuid().trim().min(1)
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -396,7 +394,7 @@ export const registerAiMcpServerRouter = async (server: FastifyZodProvider) => {
}, },
schema: { schema: {
params: z.object({ params: z.object({
serverId: z.string().trim().min(1) serverId: z.string().uuid().trim().min(1)
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -439,7 +437,7 @@ export const registerAiMcpServerRouter = async (server: FastifyZodProvider) => {
}, },
schema: { schema: {
params: z.object({ params: z.object({
serverId: z.string().trim().min(1) serverId: z.string().uuid().trim().min(1)
}), }),
response: { response: {
200: z.object({ 200: z.object({

View File

@@ -7,5 +7,10 @@ export type TAiMcpEndpointServerDALFactory = ReturnType<typeof aiMcpEndpointServ
export const aiMcpEndpointServerDALFactory = (db: TDbClient) => { export const aiMcpEndpointServerDALFactory = (db: TDbClient) => {
const aiMcpEndpointServerOrm = ormify(db, TableName.AiMcpEndpointServer); const aiMcpEndpointServerOrm = ormify(db, TableName.AiMcpEndpointServer);
return aiMcpEndpointServerOrm; const countByEndpointId = async (aiMcpEndpointId: string) => {
const result = await db.replicaNode()(TableName.AiMcpEndpointServer).where({ aiMcpEndpointId }).count().first();
return Number(result?.count ?? 0);
};
return { ...aiMcpEndpointServerOrm, countByEndpointId };
}; };

View File

@@ -7,5 +7,10 @@ export type TAiMcpEndpointServerToolDALFactory = ReturnType<typeof aiMcpEndpoint
export const aiMcpEndpointServerToolDALFactory = (db: TDbClient) => { export const aiMcpEndpointServerToolDALFactory = (db: TDbClient) => {
const aiMcpEndpointServerToolOrm = ormify(db, TableName.AiMcpEndpointServerTool); const aiMcpEndpointServerToolOrm = ormify(db, TableName.AiMcpEndpointServerTool);
return aiMcpEndpointServerToolOrm; const countByEndpointId = async (aiMcpEndpointId: string) => {
const result = await db.replicaNode()(TableName.AiMcpEndpointServerTool).where({ aiMcpEndpointId }).count().first();
return Number(result?.count ?? 0);
};
return { ...aiMcpEndpointServerToolOrm, countByEndpointId };
}; };

View File

@@ -117,7 +117,6 @@ const computePkceChallenge = (codeVerifier: string) => {
export type TAiMcpEndpointServiceFactory = ReturnType<typeof aiMcpEndpointServiceFactory>; export type TAiMcpEndpointServiceFactory = ReturnType<typeof aiMcpEndpointServiceFactory>;
/* eslint-disable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-argument */
export const aiMcpEndpointServiceFactory = ({ export const aiMcpEndpointServiceFactory = ({
aiMcpEndpointDAL, aiMcpEndpointDAL,
aiMcpEndpointServerDAL, aiMcpEndpointServerDAL,
@@ -335,19 +334,15 @@ export const aiMcpEndpointServiceFactory = ({
// Log failed activity with full error details for user visibility in activity logs // Log failed activity with full error details for user visibility in activity logs
const errorMessage = error instanceof Error ? error.message : String(error); const errorMessage = error instanceof Error ? error.message : String(error);
await aiMcpActivityLogService await aiMcpActivityLogService.createActivityLog({
.createActivityLog({ endpointName: endpoint.name,
endpointName: endpoint.name, serverName: selectedMcpClient.server.name,
serverName: selectedMcpClient.server.name, toolName: name,
toolName: name, actor: user.email || "",
actor: user.email || "", request: args,
request: args, response: { error: errorMessage },
response: { error: errorMessage }, projectId: endpoint.projectId
projectId: endpoint.projectId });
})
.catch((logError) => {
logger.error({ error: logError }, "Failed to log tool call error activity");
});
// Return generic error to client to avoid information leakage // Return generic error to client to avoid information leakage
return { return {
@@ -463,13 +458,15 @@ export const aiMcpEndpointServiceFactory = ({
// Get connected servers count and tools count for each endpoint // Get connected servers count and tools count for each endpoint
const endpointsWithStats = await Promise.all( const endpointsWithStats = await Promise.all(
endpoints.map(async (endpoint) => { endpoints.map(async (endpoint) => {
const connectedServers = await aiMcpEndpointServerDAL.find({ aiMcpEndpointId: endpoint.id }); const [connectedServersCount, activeToolsCount] = await Promise.all([
const tools = await aiMcpEndpointServerToolDAL.find({ aiMcpEndpointId: endpoint.id }); aiMcpEndpointServerDAL.countByEndpointId(endpoint.id),
aiMcpEndpointServerToolDAL.countByEndpointId(endpoint.id)
]);
return { return {
...endpoint, ...endpoint,
connectedServers: connectedServers.length, connectedServers: connectedServersCount,
activeTools: tools.length activeTools: activeToolsCount
}; };
}) })
); );
@@ -503,13 +500,15 @@ export const aiMcpEndpointServiceFactory = ({
ProjectPermissionSub.McpEndpoints ProjectPermissionSub.McpEndpoints
); );
const connectedServers = await aiMcpEndpointServerDAL.find({ aiMcpEndpointId: endpointId }); const [connectedServers, activeToolsCount] = await Promise.all([
const tools = await aiMcpEndpointServerToolDAL.find({ aiMcpEndpointId: endpointId }); aiMcpEndpointServerDAL.find({ aiMcpEndpointId: endpointId }),
aiMcpEndpointServerToolDAL.countByEndpointId(endpointId)
]);
return { return {
...endpoint, ...endpoint,
connectedServers: connectedServers.length, connectedServers: connectedServers.length,
activeTools: tools.length, activeTools: activeToolsCount,
serverIds: connectedServers.map((s) => s.aiMcpServerId) serverIds: connectedServers.map((s) => s.aiMcpServerId)
}; };
}; };
@@ -1194,7 +1193,12 @@ export const aiMcpEndpointServiceFactory = ({
encryptedCredentials encryptedCredentials
}); });
return { success: true }; return {
success: true,
projectId: endpoint.projectId,
endpointName: endpoint.name,
serverName: server.name
};
}; };
return { return {

View File

@@ -72,10 +72,9 @@ const refreshOAuthToken = async (
clientId: string, clientId: string,
clientSecret?: string clientSecret?: string
): Promise<{ accessToken: string; refreshToken?: string; expiresAt?: number }> => { ): Promise<{ accessToken: string; refreshToken?: string; expiresAt?: number }> => {
const issuer = new URL(serverUrl).origin; const serverUrlObj = new URL(serverUrl);
const { data: serverMetadata } = await request.get<TOAuthAuthorizationServerMetadata>( const metadataUrl = `${serverUrlObj.origin}/.well-known/oauth-authorization-server${serverUrlObj.pathname !== "/" ? serverUrlObj.pathname : ""}`;
`${issuer}/.well-known/oauth-authorization-server` const { data: serverMetadata } = await request.get<TOAuthAuthorizationServerMetadata>(metadataUrl);
);
const tokenParams: Record<string, string> = { const tokenParams: Record<string, string> = {
grant_type: "refresh_token", grant_type: "refresh_token",

View File

@@ -603,6 +603,7 @@ export enum EventType {
MCP_ENDPOINT_OAUTH_CLIENT_REGISTER = "mcp-endpoint-oauth-client-register", MCP_ENDPOINT_OAUTH_CLIENT_REGISTER = "mcp-endpoint-oauth-client-register",
MCP_ENDPOINT_OAUTH_AUTHORIZE = "mcp-endpoint-oauth-authorize", MCP_ENDPOINT_OAUTH_AUTHORIZE = "mcp-endpoint-oauth-authorize",
MCP_ENDPOINT_CONNECT = "mcp-endpoint-connect", MCP_ENDPOINT_CONNECT = "mcp-endpoint-connect",
MCP_ENDPOINT_SAVE_USER_CREDENTIAL = "mcp-endpoint-save-user-credential",
// MCP Servers // MCP Servers
MCP_SERVER_CREATE = "mcp-server-create", MCP_SERVER_CREATE = "mcp-server-create",
@@ -4623,6 +4624,16 @@ interface McpEndpointConnectEvent {
}; };
} }
interface McpEndpointSaveUserCredentialEvent {
type: EventType.MCP_ENDPOINT_SAVE_USER_CREDENTIAL;
metadata: {
endpointId: string;
endpointName: string;
serverId: string;
serverName: string;
};
}
interface McpServerCreateEvent { interface McpServerCreateEvent {
type: EventType.MCP_SERVER_CREATE; type: EventType.MCP_SERVER_CREATE;
metadata: { metadata: {
@@ -5112,6 +5123,7 @@ export type Event =
| McpEndpointOAuthClientRegisterEvent | McpEndpointOAuthClientRegisterEvent
| McpEndpointOAuthAuthorizeEvent | McpEndpointOAuthAuthorizeEvent
| McpEndpointConnectEvent | McpEndpointConnectEvent
| McpEndpointSaveUserCredentialEvent
| McpServerCreateEvent | McpServerCreateEvent
| McpServerUpdateEvent | McpServerUpdateEvent
| McpServerDeleteEvent | McpServerDeleteEvent

View File

@@ -321,6 +321,7 @@ export const eventToNameMap: { [K in EventType]: string } = {
[EventType.MCP_ENDPOINT_OAUTH_CLIENT_REGISTER]: "Register MCP OAuth Client", [EventType.MCP_ENDPOINT_OAUTH_CLIENT_REGISTER]: "Register MCP OAuth Client",
[EventType.MCP_ENDPOINT_OAUTH_AUTHORIZE]: "Authorize MCP OAuth Client", [EventType.MCP_ENDPOINT_OAUTH_AUTHORIZE]: "Authorize MCP OAuth Client",
[EventType.MCP_ENDPOINT_CONNECT]: "Connect to MCP Endpoint", [EventType.MCP_ENDPOINT_CONNECT]: "Connect to MCP Endpoint",
[EventType.MCP_ENDPOINT_SAVE_USER_CREDENTIAL]: "Save MCP Server User Credential",
// MCP Servers // MCP Servers
[EventType.MCP_SERVER_CREATE]: "Create MCP Server", [EventType.MCP_SERVER_CREATE]: "Create MCP Server",
@@ -405,6 +406,7 @@ export const projectToEventsMap: Partial<Record<ProjectType, EventType[]>> = {
EventType.MCP_ENDPOINT_OAUTH_CLIENT_REGISTER, EventType.MCP_ENDPOINT_OAUTH_CLIENT_REGISTER,
EventType.MCP_ENDPOINT_OAUTH_AUTHORIZE, EventType.MCP_ENDPOINT_OAUTH_AUTHORIZE,
EventType.MCP_ENDPOINT_CONNECT, EventType.MCP_ENDPOINT_CONNECT,
EventType.MCP_ENDPOINT_SAVE_USER_CREDENTIAL,
// MCP Servers // MCP Servers
EventType.MCP_SERVER_CREATE, EventType.MCP_SERVER_CREATE,
EventType.MCP_SERVER_UPDATE, EventType.MCP_SERVER_UPDATE,

View File

@@ -314,6 +314,7 @@ export enum EventType {
MCP_ENDPOINT_OAUTH_CLIENT_REGISTER = "mcp-endpoint-oauth-client-register", MCP_ENDPOINT_OAUTH_CLIENT_REGISTER = "mcp-endpoint-oauth-client-register",
MCP_ENDPOINT_OAUTH_AUTHORIZE = "mcp-endpoint-oauth-authorize", MCP_ENDPOINT_OAUTH_AUTHORIZE = "mcp-endpoint-oauth-authorize",
MCP_ENDPOINT_CONNECT = "mcp-endpoint-connect", MCP_ENDPOINT_CONNECT = "mcp-endpoint-connect",
MCP_ENDPOINT_SAVE_USER_CREDENTIAL = "mcp-endpoint-save-user-credential",
// MCP Servers // MCP Servers
MCP_SERVER_CREATE = "mcp-server-create", MCP_SERVER_CREATE = "mcp-server-create",