feat(server): accept auth settings in postIntegrations #5199
Conversation
🤖 My Senior Dev — Analysis Complete👤 For @kaposke📁 Expert in View your contributor analytics → 📊 6 files reviewed • 1 high risk • 4 need attention 🚨 High Risk:
🚀 Open Interactive Review →The full interface unlocks features not available in GitHub:
💬 Chat here: 📖 View all 12 personas & slash commandsYou can interact with me by mentioning In PR comments or on any line of code:
Slash commands:
AI Personas (mention to get their perspective):
For the best experience, view this PR on myseniordev.com — includes AI chat, file annotations, and interactive reviews. |
| // Determine unique_key | ||
| let unique_key: string; | ||
| if ('integrationId' in body && body.integrationId) { | ||
| unique_key = body.integrationId; |
There was a problem hiding this comment.
what if the passed integrationId already exist?
There was a problem hiding this comment.
I've screwed up the condition. Fixing...
| * @param clientId - Optional client ID (for MCP_OAUTH2) | ||
| * @returns A DBCreateIntegration object ready to be passed to createProviderConfig | ||
| */ | ||
| export async function buildIntegrationConfig(body: PostIntegration['Body'], environmentId: number, mcpClientId?: string): Promise<DBCreateIntegration> { |
There was a problem hiding this comment.
can this function be reused in patchIntegration?
There was a problem hiding this comment.
I've made the input format slightly different. I'm considering refactoring the patch method to be similar to this one later, just wanted to avoid work in patch and updating frontend for now.
| * @param provider - The provider configuration | ||
| * @param environmentId - The environment ID | ||
| * @param existingCount - The count of existing integrations with the same provider name (for unique_key generation) | ||
| * @param clientId - Optional client ID (for MCP_OAUTH2) |
There was a problem hiding this comment.
this is called mcpClientId below?
There was a problem hiding this comment.
also should the mcp edge case be left outside of this function?
kaposke
force-pushed
the
gui/NAN-4478/feat-post-integration-accepts-data
branch
from
34600fe to
5f7a7cf
Compare
| } else { | ||
| const exists = await configService.getIdByProviderConfigKey(environmentId, body.provider); | ||
| unique_key = !exists ? body.provider : `${body.provider}-${nanoid(4).toLocaleLowerCase()}`; | ||
| } |
There was a problem hiding this comment.
what about
const getUniqueKey(string) = async (key) => {
const exists = await configService.getIdByProviderConfigKey(environmentId, key);
return exists ? `${key}-${nanoid(4).toLocaleLowerCase()}` : ;
}
const uniqueKey = getUniqueKey(body.integrationId ? body.integrationId : body.provider)
| environment_id: environmentId, | ||
| unique_key, | ||
| provider: body.provider, | ||
| forward_webhooks: 'forward_webhooks' in body && body.forward_webhooks !== undefined ? body.forward_webhooks : true, |
There was a problem hiding this comment.
isn't it the same as:
forward_webhooks: body.forward_webhooks ?? true
Currently,
postIntegrationonly creates an empty integration. To add auth settings (like client_id and secret) we have to callpathIntegration. To the new integration creation flow, where the user fills in the data at the moment of creating the integration, we needpostIntegrationto accept auth settings from the get-go.This PR
postIntegrationsto accept auth settings, as well as general settings (integrationId, forwardWebhooks, etc).The controller now relies on shared validation and build helpers to translate provider metadata and credentials into persisted configurations, enforce auth/provider compatibility and custom identifier uniqueness, provision MCP OAuth clients when required, and extend shared credential handling and webhook secret management, all backed by integration tests covering success and failure paths.
Key Changes
• Introduced
packages/server/lib/controllers/v1/integrations/validation.tswith discriminated-union schemas reused by bothpostIntegrationandpatchIntegration• Added
packages/server/lib/controllers/v1/integrations/buildIntegrationConfig.tsto constructDBCreateIntegrationpayloads (unique key generation, credential mapping, webhook secret propagation)• Expanded
packages/server/lib/controllers/v1/integrations/postIntegration.tsto validate extended bodies, enforceintegrationIduniqueness and auth-mode compatibility, optionally register MCP OAuth clients, and create provider configs via the new helper• Updated
packages/server/lib/controllers/v1/integrations/providerConfigKey/patchIntegration.tsandpackages/types/lib/integration/api.tsto reuse shared typings/schemas for auth payloads• Added comprehensive Vitest integration coverage in
packages/server/lib/controllers/v1/integrations/postIntegration.integration.test.tsfor validation errors and success casesAffected Areas
•
packages/server/lib/controllers/v1/integrations/postIntegration.ts•
packages/server/lib/controllers/v1/integrations/buildIntegrationConfig.ts•
packages/server/lib/controllers/v1/integrations/validation.ts•
packages/server/lib/controllers/v1/integrations/providerConfigKey/patchIntegration.ts•
packages/types/lib/integration/api.ts•
packages/server/lib/controllers/v1/integrations/postIntegration.integration.test.tsThis summary was automatically generated by @propel-code-bot