feat(api-service,dashboard): automate MS Teams onboarding fixes NV-7387#10958
Merged
djabarovgeorge merged 14 commits intonextfrom May 4, 2026
Merged
feat(api-service,dashboard): automate MS Teams onboarding fixes NV-7387#10958djabarovgeorge merged 14 commits intonextfrom
djabarovgeorge merged 14 commits intonextfrom
Conversation
…e flow Adds a signed ARM-template endpoint and a "Deploy to Azure" button so users can create the Azure Bot resource in one click instead of manually configuring the Azure Portal. - New GET /:integrationId/msteams-arm-template/deploy-url endpoint generates a 15-minute HMAC-signed URL that opens the Azure Portal pre-filled with the ARM template. - New GET /:integrationId/msteams-arm-template public endpoint serves the ARM template JSON, verified by sig/exp query params before returning content. - GenerateMsTeamsArmTemplate and GetMsTeamsArmTemplate usecases added and registered. - TeamsSetupGuide refactored: redirect URI moved to step 0 (App Registration), old "Add redirect URI" step removed, new "Deploy to Azure" step added at step 3 replacing the manual webhook/channel setup step. - WebhookUrlSection and buildWebhookUrl helpers removed (superseded by ARM template deployment). - getMsTeamsArmTemplateDeployUrl API helper added to the dashboard. Made-with: Cursor
Updated the bot name sanitization logic to ensure that names are between 2 and 64 characters, replacing invalid characters with hyphens. If the sanitized name is less than 2 characters, it defaults to 'bot'. This change enhances compliance with Azure Bot resource naming conventions.
…ation Added new endpoints and use cases to facilitate Azure AD OAuth setup for MS Teams. This includes: - GET /:integrationId/msteams-azure-setup/oauth-url to generate an Azure AD OAuth URL for quick setup. - GET /chat/oauth/azure-setup/callback to handle the OAuth callback from Azure AD. - Integrated new use cases: AzureSetupOauthCallback and GenerateAzureSetupOauthUrl. - Updated the IntegrationsController to include the new functionality and handle OAuth flow. - Adjusted related components in the dashboard to support the new Azure setup process. This enhancement streamlines the integration process for users, allowing for easier setup of MS Teams bots.
… endpoint - Introduced a new health-check endpoint for MS Teams integration at GET /:integrationId/msteams-health, allowing users to verify the readiness of their MS Teams credentials and app catalog entry after the OAuth setup. - Updated the IntegrationsController to include the new health check functionality. - Added necessary use cases and types to support the health check feature, improving the overall integration experience. - Refactored existing code to streamline the MS Teams setup process and ensure compliance with Azure requirements.
- Replaced the exclusion of the TeamsAppInstallation.ReadWriteSelfForUser.All permission with its inclusion, reflecting changes in the bot installation process. - Enhanced the MsTeamsOauthCallback to handle errors during the link_user mode, providing better feedback on bot installation failures. - Updated tests to accommodate the new bot installation logic and ensure proper functionality. - Introduced a feature flag for enabling quick setup in the Teams setup guide, improving user experience during integration.
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
github-actions
Bot
added
@novu/dal
@novu/shared
@novu/dashboard
@novu/api-service
labels
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds MS Teams Azure Quick Setup (OAuth-driven app registration, Teams catalog upload, and optional ARM Bot provisioning), ARM template generation/serving with HMAC+expiry, MS Teams health checks, dashboard UI for quick/manual flows and ZIP packaging utilities, plus a small webhook controller error-handling change. ChangesAzure Setup & MS Teams Integration
Webhook Error Handling
Sequence Diagram(s)sequenceDiagram
participant Browser
participant DashboardAPI as IntegrationsController
participant UseCase as AzureSetupOauthCallback
participant Graph as Microsoft Graph
participant AzureARM as management.azure.com
participant DB as IntegrationRepository
Browser->>DashboardAPI: GET /msteams-azure-setup/oauth-url
DashboardAPI->>UseCase: GenerateAzureSetupOauthUrl.execute(cmd)
UseCase-->>DashboardAPI: { url }
DashboardAPI-->>Browser: redirect to Azure authorize (state signed)
Browser->>DashboardAPI: GET /chat/oauth/azure-setup/callback?code&state
DashboardAPI->>UseCase: AzureSetupOauthCallback.execute(command)
UseCase->>Graph: Exchange code -> tokens
UseCase->>Graph: Create app registration / service principal / appRoleAssignments
UseCase->>DB: save encrypted credentials + provisioning.pending
UseCase->>Graph: upload Teams app zip (best-effort)
UseCase->>AzureARM: (fire-and-forget) deploy Bot Service using refresh token
UseCase-->>DashboardAPI: popup HTML
DashboardAPI-->>Browser: popup HTML (closes window)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
djabarovgeorge
commented
May 3, 2026
Contributor
Author
There was a problem hiding this comment.
by this direct return the nest js filter were not triggered and that caused missing logs for the 500 errors
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-msteams-oath-url/generate-msteams-oauth-url.usecase.ts (1)
190-199:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: personal
ngrok.appURL hardcoded as OAuth redirect — must revert before merge
buildRedirectUri()returnshttps://gosha.ngrok.app${CHAT_OAUTH_CALLBACK_PATH}(a developer's local tunnel) while the correctAPI_ROOT_URL-based logic is commented out. Every OAuth admin-consent and link-user redirect in staging and production will be sent to this personal ngrok tunnel, which is ephemeral, unavailable to Azure AD, and publicly unknown — completely breaking the MS Teams OAuth flow outside of one developer's laptop.Additionally,
const baseUrlon Line 195 is dead code: the value is computed and then never referenced.🐛 Proposed fix
static buildRedirectUri(): string { if (!process.env.API_ROOT_URL) { throw new Error('API_ROOT_URL environment variable is required'); } const baseUrl = process.env.API_ROOT_URL.replace(/\/$/, ''); // Remove trailing slash - - return `https://gosha.ngrok.app${CHAT_OAUTH_CALLBACK_PATH}`; - // return `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}`; + return `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-msteams-oath-url/generate-msteams-oauth-url.usecase.ts` around lines 190 - 199, In buildRedirectUri() replace the hardcoded "https://gosha.ngrok.app" return with the intended API_ROOT_URL-based redirect by returning `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}` (preserving the existing trailing-slash trimming logic) and remove the unused debug/commented return; ensure the function still throws when process.env.API_ROOT_URL is missing and that CHAT_OAUTH_CALLBACK_PATH is concatenated as before so no personal ngrok URL remains in production code.
🧹 Nitpick comments (5)
libs/dal/src/repositories/integration/integration.schema.ts (1)
82-88: ⚡ Quick winConsider adding Mongoose-level
enumvalidation forstatusThe
statusfield is constrained to'pending' | 'ready' | 'failed'at the TypeScript layer but stored as a plainSchema.Types.String, so Mongoose won't reject an out-of-range value (e.g., a typo) at the persistence layer.♻️ Proposed refactor
provisioning: { - status: Schema.Types.String, + status: { type: Schema.Types.String, enum: ['pending', 'ready', 'failed'] }, startedAt: Schema.Types.String, completedAt: Schema.Types.String, errorMessage: Schema.Types.String, teamsAppCatalogId: Schema.Types.String, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/dal/src/repositories/integration/integration.schema.ts` around lines 82 - 88, The provisioning.status field in the Mongoose schema is currently Schema.Types.String and needs enum validation to match the TypeScript union; update the provisioning object in integration.schema.ts so that the status property uses type: String and an enum: ['pending','ready','failed'] (and optionally a default), ensuring the Mongoose schema for provisioning.status enforces the same allowed values as the TypeScript type.apps/api/src/app/integrations/usecases/generate-msteams-arm-template/generate-msteams-arm-template.usecase.ts (1)
67-80: ⚡ Quick winRemove unnecessary
asyncmodifier fromverifySignature.This static method performs only synchronous operations (timestamp comparison and HMAC verification) but is declared
async. This forces callers toawaitunnecessarily.♻️ Make the method synchronous
- static async verifySignature(integrationId: string, sig: string, exp: string, signingKey: string): Promise<void> { + static verifySignature(integrationId: string, sig: string, exp: string, signingKey: string): void { const expMs = Number(exp); if (!Number.isFinite(expMs) || Date.now() > expMs) { throw new UnauthorizedException('ARM template link has expired'); } const payload = `${integrationId}:${expMs}`; const expected = createHmac('sha256', signingKey).update(payload).digest('hex'); if (sig !== expected) { throw new UnauthorizedException('Invalid ARM template signature'); } }Then update the caller in
get-msteams-arm-template.usecase.ts:- await GenerateMsTeamsArmTemplate.verifySignature(integrationId, sig, exp, apiKeys[0].key); + GenerateMsTeamsArmTemplate.verifySignature(integrationId, sig, exp, apiKeys[0].key);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/usecases/generate-msteams-arm-template/generate-msteams-arm-template.usecase.ts` around lines 67 - 80, The static method verifySignature is declared async but only does synchronous work; remove the async modifier and change its return type from Promise<void> to void in generate-msteams-arm-template.usecase.ts, keeping the logic (exp parsing, HMAC compare, throwing UnauthorizedException) identical so it remains synchronous; then update its caller in get-msteams-arm-template.usecase.ts to stop awaiting the call (remove any await and treat exceptions as thrown synchronously or propagate as before).apps/api/src/app/integrations/usecases/chat-oauth-callback/msteams-oauth-callback/msteams-oauth-callback.usecase.spec.ts (1)
91-109: 💤 Low valueThe
Reflect.set/Reflect.deletePropertyapproach forprocess.envis unconventional.Direct assignment (
process.env.API_ROOT_URL = valueanddelete process.env.API_ROOT_URL) is more idiomatic for Node.js environment variable manipulation in tests. The current approach works but may confuse readers.♻️ Optional: Use idiomatic env var assignment
originalApiRootUrl = process.env.API_ROOT_URL; - Reflect.set(process.env, 'API_ROOT_URL', MOCK_API_ROOT_URL); + process.env.API_ROOT_URL = MOCK_API_ROOT_URL; // ... afterEach(() => { sinon.restore(); if (originalApiRootUrl === undefined) { - Reflect.deleteProperty(process.env, 'API_ROOT_URL'); + delete process.env.API_ROOT_URL; } else { - Reflect.set(process.env, 'API_ROOT_URL', originalApiRootUrl); + process.env.API_ROOT_URL = originalApiRootUrl; } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/usecases/chat-oauth-callback/msteams-oauth-callback/msteams-oauth-callback.usecase.spec.ts` around lines 91 - 109, The test currently uses Reflect.set/Reflect.deleteProperty to manage API_ROOT_URL via originalApiRootUrl and process.env; change these to idiomatic direct assignments by setting process.env.API_ROOT_URL = MOCK_API_ROOT_URL in the setup and restoring with either process.env.API_ROOT_URL = originalApiRootUrl (when defined) or delete process.env.API_ROOT_URL in the afterEach; update the variables and the afterEach/ beforeEach references (originalApiRootUrl, process.env, MOCK_API_ROOT_URL) accordingly to ensure the environment is restored the same way but using direct assignment/deletion.apps/api/src/app/integrations/usecases/msteams-health-check/msteams-health-check.usecase.ts (1)
158-165: 💤 Low valueConsider escaping the
clientIdin the OData filter.While
clientIdoriginates from stored credentials (not direct user input), defensive encoding would protect against edge cases where a clientId might contain special characters.♻️ Optional: Escape single quotes in OData filter
+ const escapedClientId = clientId.replace(/'/g, "''"); - const filter = encodeURIComponent(`externalId eq '${clientId}' and distributionMethod eq 'organization'`); + const filter = encodeURIComponent(`externalId eq '${escapedClientId}' and distributionMethod eq 'organization'`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/usecases/msteams-health-check/msteams-health-check.usecase.ts` around lines 158 - 165, The OData filter construction uses clientId directly before encodeURIComponent; escape single quotes in clientId first to avoid breaking the OData string. In the method that builds the filter (where clientId is used with encodeURIComponent and assigned to filter before calling `${this.MS_GRAPH_BASE_URL}/appCatalogs/teamsApps?$filter=${filter}`), replace raw clientId with an escaped version (e.g., replace each single quote with two single quotes) and then call encodeURIComponent on the resulting filter so the request to MS_GRAPH_BASE_URL with graphToken is safe.apps/api/src/app/integrations/usecases/msteams-health-check/msteams-health-check.command.ts (1)
9-19: 💤 Low valueConsider adding
@IsIn()validation for thechecksarray values.The JSDoc documents valid values (
'appRegistration' | 'azureBotCreated' | 'teamsAppCatalog' | 'permissions'), but the runtime validation accepts any string. Adding@IsIn()would catch invalid check names early.♻️ Optional: Add `@IsIn` validation
+const VALID_CHECKS = ['appRegistration', 'azureBotCreated', 'teamsAppCatalog', 'permissions'] as const; + export class MsTeamsHealthCheckCommand extends EnvironmentCommand { // ... `@IsOptional`() `@IsArray`() `@IsString`({ each: true }) + `@IsIn`(VALID_CHECKS, { each: true }) readonly checks?: string[]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/usecases/msteams-health-check/msteams-health-check.command.ts` around lines 9 - 19, The checks property accepts any string but should be restricted to the documented set; update the checks decorator list on the checks property in the MsteamsHealthCheckCommand class to include `@IsIn`(['appRegistration','azureBotCreated','teamsAppCatalog','permissions'], { each: true }), ensure IsIn is imported from class-validator, and keep `@IsOptional`(), `@IsArray`(), `@IsString`({ each: true }) in place so each element is validated against the allowed values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/integrations/integrations.controller.ts`:
- Around line 487-513: The getAzureSetupOauthUrl controller currently always
returns a signed OAuth URL; gate this endpoint with the same rollout flag used
in the UI by checking the feature flag (IS_MSTEAMS_QUICK_SETUP_ENABLED) at the
start of the getAzureSetupOauthUrl method and reject requests when it is
disabled (e.g., throw a 403/404 or use existing feature-flag guard logic).
Modify getAzureSetupOauthUrl to read the flag (via your feature-flag/service or
environment helper) and short-circuit before calling
generateAzureSetupOauthUrlUsecase.execute when the flag is false, returning the
appropriate error response so the endpoint behavior matches the UI rollout.
Ensure you reference the existing method getAzureSetupOauthUrl and the
constant/name IS_MSTEAMS_QUICK_SETUP_ENABLED when implementing the check.
In
`@apps/api/src/app/integrations/usecases/azure-setup-oauth-callback/azure-setup-oauth-callback.usecase.ts`:
- Around line 877-887: The manifest's developer block currently contains
placeholder values ("Your Company", "your-domain.com") in the object created in
azure-setup-oauth-callback.usecase.ts (the manifest/manifest object around id
appId); update that code to pull real values from configuration or tenant
metadata (e.g., env vars or the existing tenant/org record) and populate name,
websiteUrl, privacyUrl, and termsOfUseUrl accordingly before upload, and
validate/throw if any required URL or name is missing so the auto-upload won't
send placeholder links.
- Around line 132-137: The returned HTML embeds messageJson directly into a
<script> which allows a crafted payload (e.g., containing </script>,
U+2028/U+2029) to break out and execute code; in
azure-setup-oauth-callback.usecase.ts sanitize/escape the serialized payload
before embedding by transforming messageJson (the JSON.stringify(message)
result) to a JS-safe literal (e.g., replace "</" with "<\\/” and escape
U+2028/U+2029) or alternatively encode it (encodeURIComponent) and decode in the
opener, then use that escaped/encoded value in the template returned by the
function instead of the raw messageJson to neutralize any script-breaking
sequences.
- Around line 676-701: The resolveWebhookEndpoint function is hardcoded to an
ngrok dev URL and silently returns an "/agents/unknown/..." fallback when agent
link is missing; change it to use the production-safe base URL from
process.env.API_ROOT_URL (trim trailing slash) instead of the hardcoded
'https://gosha.ngrok.app', and when agentIntegrationRepository.findOne returns
no link, do not construct a webhook to an unknown agent — instead throw or
return an error/undefined to stop Azure bot creation (update callers
accordingly), and ensure any logging includes context (integrationId,
environmentId) so callers like resolveWebhookEndpoint,
agentIntegrationRepository.findOne, and integrationRepository.findOne do not
deploy bots against developer tunnels.
In
`@apps/api/src/app/integrations/usecases/generate-azure-setup-oauth-url/generate-azure-setup-oauth-url.usecase.ts`:
- Around line 89-98: In buildRedirectUri(), remove the hardcoded
'https://gosha.ngrok.app' and use the validated process.env.API_ROOT_URL instead
(trim any trailing slash) so the function returns
`${base}/v1/integrations/chat/oauth/azure-setup/callback`; ensure the existing
check that throws when API_ROOT_URL is missing remains and replace the commented
replace line with active trimming logic to normalize the base URL in the
buildRedirectUri method.
In
`@apps/api/src/app/integrations/usecases/generate-msteams-arm-template/generate-msteams-arm-template.command.ts`:
- Around line 1-8: Replace the current validation and mutability for
integrationId in the GenerateMsTeamsArmTemplateCommand: change the decorator
from `@IsDefined`() to `@IsNotEmpty`() so blank strings are rejected, keep
`@IsString`(), and mark the property as readonly (integrationId) to enforce
immutability; update the imports to include IsNotEmpty from 'class-validator'
and remove IsDefined if no longer used.
In
`@apps/api/src/app/integrations/usecases/generate-msteams-arm-template/generate-msteams-arm-template.usecase.ts`:
- Around line 82-87: The buildTemplateApiUrl function currently uses a hardcoded
ngrok URL; replace that with the environment variable approach used elsewhere:
read process.env.API_ROOT_URL (with a safe fallback), strip any trailing slash,
and use that as base; update buildTemplateApiUrl to compute base from
API_ROOT_URL (or '' fallback) and then return the same path including
integrationId, sig and exp so there is no hardcoded dev URL in
buildTemplateApiUrl.
In
`@apps/api/src/app/integrations/usecases/generate-msteams-arm-template/get-msteams-arm-template.usecase.ts`:
- Around line 98-107: The buildWebhookUrl function currently uses a hardcoded
ngrok URL; replace that with the runtime API root by reading
process.env.API_ROOT_URL (or another appropriate env var), trim any trailing
slash (e.g., using .replace(/\/$/, '') or equivalent) to form base, and then
construct the webhook path using the existing agentId and integrationIdentifier
logic in buildWebhookUrl so the URL is environment-driven rather than hardcoded.
In `@apps/dashboard/src/components/agents/teams-setup-guide.tsx`:
- Around line 617-646: The poll currently ignores azureBotCreated (setting it to
null) and advances when only teamsAppCatalog and permissions are ready; change
useManualHealthPoll to include 'azureBotCreated' in MANUAL_HEALTH_CHECKS (or add
it where those arrays are built), populate ManualHealthState.azureBotCreated
from the API result instead of null in the poll() function, and only consider
the manual flow ready when azureBotCreated === 'ready' AND teamsAppCatalog ===
'ready' AND permissions === 'ready'; apply the same update to the other poll
implementations mentioned (the other useManualHealthPoll usages/variants around
the indicated ranges) so all checks block advancing until azureBotCreated is
actually ready.
- Around line 778-797: The message handler (handleMessage in the useEffect)
trusts any postMessage with type 'novu:azure-setup-complete'; tighten it by
verifying the message source and origin before mutating state: confirm
event.source === azurePopupRef.current and that event.origin matches the
expected Azure popup origin (or derive it from
azurePopupRef.current.location.origin after checking popup is non-null), and
only then call setTeamsAppUploaded, setShowHealthCheck and invalidate queries
via queryClient.invalidateQueries([QueryKeys.fetchIntegrations]). Also ensure
azurePopupRef.current is checked for null before closing to avoid race
conditions.
---
Outside diff comments:
In
`@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-msteams-oath-url/generate-msteams-oauth-url.usecase.ts`:
- Around line 190-199: In buildRedirectUri() replace the hardcoded
"https://gosha.ngrok.app" return with the intended API_ROOT_URL-based redirect
by returning `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}` (preserving the existing
trailing-slash trimming logic) and remove the unused debug/commented return;
ensure the function still throws when process.env.API_ROOT_URL is missing and
that CHAT_OAUTH_CALLBACK_PATH is concatenated as before so no personal ngrok URL
remains in production code.
---
Nitpick comments:
In
`@apps/api/src/app/integrations/usecases/chat-oauth-callback/msteams-oauth-callback/msteams-oauth-callback.usecase.spec.ts`:
- Around line 91-109: The test currently uses Reflect.set/Reflect.deleteProperty
to manage API_ROOT_URL via originalApiRootUrl and process.env; change these to
idiomatic direct assignments by setting process.env.API_ROOT_URL =
MOCK_API_ROOT_URL in the setup and restoring with either
process.env.API_ROOT_URL = originalApiRootUrl (when defined) or delete
process.env.API_ROOT_URL in the afterEach; update the variables and the
afterEach/ beforeEach references (originalApiRootUrl, process.env,
MOCK_API_ROOT_URL) accordingly to ensure the environment is restored the same
way but using direct assignment/deletion.
In
`@apps/api/src/app/integrations/usecases/generate-msteams-arm-template/generate-msteams-arm-template.usecase.ts`:
- Around line 67-80: The static method verifySignature is declared async but
only does synchronous work; remove the async modifier and change its return type
from Promise<void> to void in generate-msteams-arm-template.usecase.ts, keeping
the logic (exp parsing, HMAC compare, throwing UnauthorizedException) identical
so it remains synchronous; then update its caller in
get-msteams-arm-template.usecase.ts to stop awaiting the call (remove any await
and treat exceptions as thrown synchronously or propagate as before).
In
`@apps/api/src/app/integrations/usecases/msteams-health-check/msteams-health-check.command.ts`:
- Around line 9-19: The checks property accepts any string but should be
restricted to the documented set; update the checks decorator list on the checks
property in the MsteamsHealthCheckCommand class to include
`@IsIn`(['appRegistration','azureBotCreated','teamsAppCatalog','permissions'], {
each: true }), ensure IsIn is imported from class-validator, and keep
`@IsOptional`(), `@IsArray`(), `@IsString`({ each: true }) in place so each element is
validated against the allowed values.
In
`@apps/api/src/app/integrations/usecases/msteams-health-check/msteams-health-check.usecase.ts`:
- Around line 158-165: The OData filter construction uses clientId directly
before encodeURIComponent; escape single quotes in clientId first to avoid
breaking the OData string. In the method that builds the filter (where clientId
is used with encodeURIComponent and assigned to filter before calling
`${this.MS_GRAPH_BASE_URL}/appCatalogs/teamsApps?$filter=${filter}`), replace
raw clientId with an escaped version (e.g., replace each single quote with two
single quotes) and then call encodeURIComponent on the resulting filter so the
request to MS_GRAPH_BASE_URL with graphToken is safe.
In `@libs/dal/src/repositories/integration/integration.schema.ts`:
- Around line 82-88: The provisioning.status field in the Mongoose schema is
currently Schema.Types.String and needs enum validation to match the TypeScript
union; update the provisioning object in integration.schema.ts so that the
status property uses type: String and an enum: ['pending','ready','failed'] (and
optionally a default), ensuring the Mongoose schema for provisioning.status
enforces the same allowed values as the TypeScript type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ce1d61df-96b3-4e3d-8554-a24c25a9e796
📒 Files selected for processing (24)
apps/api/src/app/agents/agents-webhook.controller.tsapps/api/src/app/integrations/integrations.controller.tsapps/api/src/app/integrations/usecases/azure-setup-oauth-callback/azure-setup-oauth-callback.command.tsapps/api/src/app/integrations/usecases/azure-setup-oauth-callback/azure-setup-oauth-callback.usecase.tsapps/api/src/app/integrations/usecases/chat-oauth-callback/msteams-oauth-callback/msteams-oauth-callback.usecase.spec.tsapps/api/src/app/integrations/usecases/chat-oauth-callback/msteams-oauth-callback/msteams-oauth-callback.usecase.tsapps/api/src/app/integrations/usecases/generate-azure-setup-oauth-url/generate-azure-setup-oauth-url.command.tsapps/api/src/app/integrations/usecases/generate-azure-setup-oauth-url/generate-azure-setup-oauth-url.usecase.tsapps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-msteams-oath-url/generate-msteams-oauth-url.usecase.tsapps/api/src/app/integrations/usecases/generate-msteams-arm-template/generate-msteams-arm-template.command.tsapps/api/src/app/integrations/usecases/generate-msteams-arm-template/generate-msteams-arm-template.usecase.tsapps/api/src/app/integrations/usecases/generate-msteams-arm-template/get-msteams-arm-template.usecase.tsapps/api/src/app/integrations/usecases/index.tsapps/api/src/app/integrations/usecases/msteams-health-check/msteams-health-check.command.tsapps/api/src/app/integrations/usecases/msteams-health-check/msteams-health-check.usecase.tsapps/dashboard/src/api/integrations.tsapps/dashboard/src/components/agents/agent-integrations-tab.tsxapps/dashboard/src/components/agents/agent-setup-guide.tsxapps/dashboard/src/components/agents/provider-dropdown.tsxapps/dashboard/src/components/agents/teams-setup-guide.tsxlibs/application-generic/src/services/ms-teams-token.service.tslibs/dal/src/repositories/integration/integration.entity.tslibs/dal/src/repositories/integration/integration.schema.tspackages/shared/src/types/feature-flags.ts
Comment on lines
+487
to
+513
| @Get('/:integrationId/msteams-azure-setup/oauth-url') | ||
| @ApiOkResponse({ | ||
| description: 'Azure AD OAuth URL for the Quick Setup flow (Novu creates the app registration).', | ||
| }) | ||
| @ApiOperation({ | ||
| summary: 'Get Azure Quick Setup OAuth URL', | ||
| description: | ||
| 'Returns an Azure AD OAuth URL that authorizes Novu to create an App Registration and client secret on your behalf via Microsoft Graph.', | ||
| }) | ||
| @ApiExcludeEndpoint() | ||
| @RequireAuthentication() | ||
| @RequirePermissions(PermissionsEnum.INTEGRATION_WRITE) | ||
| async getAzureSetupOauthUrl( | ||
| @UserSession() user: UserSessionData, | ||
| @Param('integrationId') integrationId: string | ||
| ): Promise<{ url: string }> { | ||
| const url = await this.generateAzureSetupOauthUrlUsecase.execute( | ||
| GenerateAzureSetupOauthUrlCommand.create({ | ||
| userId: user._id, | ||
| organizationId: user.organizationId, | ||
| environmentId: user.environmentId, | ||
| integrationId, | ||
| }) | ||
| ); | ||
|
|
||
| return { url }; | ||
| } |
Contributor
There was a problem hiding this comment.
Gate the Azure Quick Setup entrypoint with the rollout flag.
The dashboard hides Quick Setup behind IS_MSTEAMS_QUICK_SETUP_ENABLED, but this endpoint still issues a usable signed OAuth URL to any caller with integration-write access. That makes the rollout flag UI-only.
💡 Suggested fix
async getAzureSetupOauthUrl(
`@UserSession`() user: UserSessionData,
`@Param`('integrationId') integrationId: string
): Promise<{ url: string }> {
+ const isEnabled = await this.featureFlagsService.getFlag({
+ key: FeatureFlagsKeysEnum.IS_MSTEAMS_QUICK_SETUP_ENABLED,
+ defaultValue: false,
+ organization: { _id: user.organizationId },
+ });
+
+ if (!isEnabled) {
+ throw new NotFoundException('Feature not enabled');
+ }
+
const url = await this.generateAzureSetupOauthUrlUsecase.execute(
GenerateAzureSetupOauthUrlCommand.create({
userId: user._id,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Get('/:integrationId/msteams-azure-setup/oauth-url') | |
| @ApiOkResponse({ | |
| description: 'Azure AD OAuth URL for the Quick Setup flow (Novu creates the app registration).', | |
| }) | |
| @ApiOperation({ | |
| summary: 'Get Azure Quick Setup OAuth URL', | |
| description: | |
| 'Returns an Azure AD OAuth URL that authorizes Novu to create an App Registration and client secret on your behalf via Microsoft Graph.', | |
| }) | |
| @ApiExcludeEndpoint() | |
| @RequireAuthentication() | |
| @RequirePermissions(PermissionsEnum.INTEGRATION_WRITE) | |
| async getAzureSetupOauthUrl( | |
| @UserSession() user: UserSessionData, | |
| @Param('integrationId') integrationId: string | |
| ): Promise<{ url: string }> { | |
| const url = await this.generateAzureSetupOauthUrlUsecase.execute( | |
| GenerateAzureSetupOauthUrlCommand.create({ | |
| userId: user._id, | |
| organizationId: user.organizationId, | |
| environmentId: user.environmentId, | |
| integrationId, | |
| }) | |
| ); | |
| return { url }; | |
| } | |
| `@Get`('/:integrationId/msteams-azure-setup/oauth-url') | |
| `@ApiOkResponse`({ | |
| description: 'Azure AD OAuth URL for the Quick Setup flow (Novu creates the app registration).', | |
| }) | |
| `@ApiOperation`({ | |
| summary: 'Get Azure Quick Setup OAuth URL', | |
| description: | |
| 'Returns an Azure AD OAuth URL that authorizes Novu to create an App Registration and client secret on your behalf via Microsoft Graph.', | |
| }) | |
| `@ApiExcludeEndpoint`() | |
| `@RequireAuthentication`() | |
| `@RequirePermissions`(PermissionsEnum.INTEGRATION_WRITE) | |
| async getAzureSetupOauthUrl( | |
| `@UserSession`() user: UserSessionData, | |
| `@Param`('integrationId') integrationId: string | |
| ): Promise<{ url: string }> { | |
| const isEnabled = await this.featureFlagsService.getFlag({ | |
| key: FeatureFlagsKeysEnum.IS_MSTEAMS_QUICK_SETUP_ENABLED, | |
| defaultValue: false, | |
| organization: { _id: user.organizationId }, | |
| }); | |
| if (!isEnabled) { | |
| throw new NotFoundException('Feature not enabled'); | |
| } | |
| const url = await this.generateAzureSetupOauthUrlUsecase.execute( | |
| GenerateAzureSetupOauthUrlCommand.create({ | |
| userId: user._id, | |
| organizationId: user.organizationId, | |
| environmentId: user.environmentId, | |
| integrationId, | |
| }) | |
| ); | |
| return { url }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/app/integrations/integrations.controller.ts` around lines 487 -
513, The getAzureSetupOauthUrl controller currently always returns a signed
OAuth URL; gate this endpoint with the same rollout flag used in the UI by
checking the feature flag (IS_MSTEAMS_QUICK_SETUP_ENABLED) at the start of the
getAzureSetupOauthUrl method and reject requests when it is disabled (e.g.,
throw a 403/404 or use existing feature-flag guard logic). Modify
getAzureSetupOauthUrl to read the flag (via your feature-flag/service or
environment helper) and short-circuit before calling
generateAzureSetupOauthUrlUsecase.execute when the flag is false, returning the
appropriate error response so the endpoint behavior matches the UI rollout.
Ensure you reference the existing method getAzureSetupOauthUrl and the
constant/name IS_MSTEAMS_QUICK_SETUP_ENABLED when implementing the check.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…sed code - Updated OAuth URL generation in multiple use cases to utilize the API_ROOT_URL environment variable instead of hardcoded URLs. - Removed redundant ZIP building logic from the AzureSetupOauthCallback and teams-app-package components, leveraging a shared utility function instead. - Cleaned up commented-out code and improved overall readability of the affected files.
…rom AzureSetupOauthCallback - Eliminated unused commented-out code related to state verification in the AzureSetupOauthCallback use case, improving code clarity and maintainability.
- Updated the message JSON serialization in the AzureSetupOauthCallback use case to escape special characters, ensuring proper handling of HTML entities and line breaks. This change enhances security and prevents potential issues with message formatting during the OAuth callback process.
…lidation in GenerateMsTeamsArmTemplateCommand - Removed unnecessary variable for teams app upload status in AzureSetupOauthCallback, streamlining the return value. - Updated the validation decorator in GenerateMsTeamsArmTemplateCommand from @isdefined to @isnotempty for better clarity and consistency in input validation. - Cleaned up the TeamsSetupGuide component by removing the message event listener, enhancing performance and reducing complexity.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-msteams-oath-url/generate-msteams-oauth-url.usecase.ts (1)
190-199: 💤 Low value
buildRedirectUri()change looks correct; remove the dead commented-out line (Line 198).The implementation now matches the Slack pattern exactly (Context snippet 3), which is good for consistency. However, Line 198 is a commented-out copy of the active
returnon Line 197 and was introduced as a new line in this diff — it should be removed.🧹 Proposed cleanup
const baseUrl = process.env.API_ROOT_URL.replace(/\/$/, ''); // Remove trailing slash - return `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}`; - // return `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}`; + return `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-msteams-oath-url/generate-msteams-oauth-url.usecase.ts` around lines 190 - 199, Remove the dead commented line in buildRedirectUri(): delete the redundant commented return statement so the method only contains the active logic that reads process.env.API_ROOT_URL, normalizes it and returns `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}`; refer to buildRedirectUri(), API_ROOT_URL and CHAT_OAUTH_CALLBACK_PATH to locate the code to edit.apps/api/src/app/integrations/usecases/azure-setup-oauth-callback/azure-setup-oauth-callback.usecase.ts (1)
35-43: ⚡ Quick winUse interfaces for these backend DTO shapes.
AzureSetupResultandTokenResponseare backend object shapes, so this file should follow the repo rule and model them asinterfaces instead oftypealiases.Suggested change
-export type AzureSetupResult = { +export interface AzureSetupResult { /** Script response for the browser popup. Posts a message to the opener and closes the tab. */ html: string; -}; +} -type TokenResponse = { +interface TokenResponse { accessToken: string; refreshToken: string | null; -}; +}As per coding guidelines, "On the backend: use
interfacefor type definitions".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/usecases/azure-setup-oauth-callback/azure-setup-oauth-callback.usecase.ts` around lines 35 - 43, Replace the backend DTO type aliases with interfaces: change the exported AzureSetupResult type alias to an exported interface named AzureSetupResult (preserving the html string JSDoc comment) and change the TokenResponse type alias to an interface named TokenResponse with accessToken: string and refreshToken: string | null; keep existing nullability and export status as in the diff and update any references/imports if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api/src/app/integrations/usecases/azure-setup-oauth-callback/azure-setup-oauth-callback.usecase.ts`:
- Around line 97-108: The code marks provisioning failed via
writeProvisioning(...) but still returns
AzureSetupOauthCallback.buildPopupHtml({ success: true }), which incorrectly
signals success; update the else branch so the returned popup reflects failure
(e.g., call AzureSetupOauthCallback.buildPopupHtml({ success: false,
errorMessage: 'No refresh token available — ARM deployment was skipped.' }) or
at minimum { success: false }) after writeProvisioning; locate the else branch
around the writeProvisioning call (references: writeProvisioning,
AzureSetupOauthCallback.buildPopupHtml, stateData.integrationId) and change the
returned payload to indicate failure instead of success.
- Around line 374-383: getTenantId currently swallows errors and returns an
empty string which lets saveCredentials persist unusable creds and an empty
msaAppTenantId; change getTenantId (in azure-setup-oauth-callback.usecase.ts) to
throw a descriptive Error when tenant resolution fails (include the accessToken
context minimally or the original error message) instead of returning ''. Also
ensure the caller path that invokes getTenantId (the flow that calls
saveCredentials()) does not proceed on a thrown error — i.e., remove any logic
that persists credentials when tenantId is falsy and let the thrown error abort
the setup so saveCredentials() is never called with an empty msaAppTenantId.
In `@apps/dashboard/src/utils/build-zip.ts`:
- Around line 27-53: The local and central directory headers created in
build-zip.ts encode filenames with TextEncoder into nameBytes but never set the
UTF-8 language encoding flag, so non-ASCII names are misinterpreted; update the
general-purpose bit flag in both the local header (DataView lv) and the central
header (DataView cv) to include the UTF-8 bit (bit 11 / 0x0800) — preserving any
existing flags (OR the value) — so the encoded names are marked as UTF-8 for ZIP
readers.
In `@libs/application-generic/src/utils/build-zip.ts`:
- Around line 31-68: The ZIP entry names are written as UTF-8 but the UTF-8
general-purpose bit (bit 11) is left cleared; update the local and central
header flag fields to set 0x0800 so filenames decode as UTF-8: in build-zip.ts
modify the Buffer.writeUInt16LE call that writes the general-purpose bit for the
local header (the writeUInt16LE at offset 6 on the Buffer named local) and the
corresponding call for the central directory header (the writeUInt16LE at offset
8 on the Buffer named central) to OR in 0x0800 instead of 0.
---
Nitpick comments:
In
`@apps/api/src/app/integrations/usecases/azure-setup-oauth-callback/azure-setup-oauth-callback.usecase.ts`:
- Around line 35-43: Replace the backend DTO type aliases with interfaces:
change the exported AzureSetupResult type alias to an exported interface named
AzureSetupResult (preserving the html string JSDoc comment) and change the
TokenResponse type alias to an interface named TokenResponse with accessToken:
string and refreshToken: string | null; keep existing nullability and export
status as in the diff and update any references/imports if needed.
In
`@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-msteams-oath-url/generate-msteams-oauth-url.usecase.ts`:
- Around line 190-199: Remove the dead commented line in buildRedirectUri():
delete the redundant commented return statement so the method only contains the
active logic that reads process.env.API_ROOT_URL, normalizes it and returns
`${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}`; refer to buildRedirectUri(),
API_ROOT_URL and CHAT_OAUTH_CALLBACK_PATH to locate the code to edit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 16653e43-5133-4de5-adde-7e54dbc282ff
📒 Files selected for processing (11)
apps/api/src/app/integrations/usecases/azure-setup-oauth-callback/azure-setup-oauth-callback.usecase.tsapps/api/src/app/integrations/usecases/generate-azure-setup-oauth-url/generate-azure-setup-oauth-url.usecase.tsapps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-msteams-oath-url/generate-msteams-oauth-url.usecase.tsapps/api/src/app/integrations/usecases/generate-msteams-arm-template/generate-msteams-arm-template.command.tsapps/api/src/app/integrations/usecases/generate-msteams-arm-template/generate-msteams-arm-template.usecase.tsapps/api/src/app/integrations/usecases/generate-msteams-arm-template/get-msteams-arm-template.usecase.tsapps/dashboard/src/components/agents/teams-app-package.tsapps/dashboard/src/components/agents/teams-setup-guide.tsxapps/dashboard/src/utils/build-zip.tslibs/application-generic/src/utils/build-zip.tslibs/application-generic/src/utils/index.ts
✅ Files skipped from review due to trivial changes (2)
- libs/application-generic/src/utils/index.ts
- apps/api/src/app/integrations/usecases/generate-msteams-arm-template/get-msteams-arm-template.usecase.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/api/src/app/integrations/usecases/generate-msteams-arm-template/generate-msteams-arm-template.command.ts
- apps/api/src/app/integrations/usecases/generate-msteams-arm-template/generate-msteams-arm-template.usecase.ts
- apps/api/src/app/integrations/usecases/generate-azure-setup-oauth-url/generate-azure-setup-oauth-url.usecase.ts
- apps/dashboard/src/components/agents/teams-setup-guide.tsx
Comment on lines
+97
to
+108
| } else { | ||
| this.logger.warn( | ||
| `Azure setup: no refresh token available, skipping ARM deployment integrationId=${stateData.integrationId}` | ||
| ); | ||
| void this.writeProvisioning(stateData, { | ||
| status: 'failed', | ||
| completedAt: new Date().toISOString(), | ||
| errorMessage: 'No refresh token available — ARM deployment was skipped.', | ||
| }); | ||
| } | ||
|
|
||
| return { html: AzureSetupOauthCallback.buildPopupHtml({ success: true }) }; |
Contributor
There was a problem hiding this comment.
Return a failure popup when deployment cannot start.
In the no-refresh-token branch, provisioning is marked failed, but Line 108 still returns success: true. That tells the user setup completed even though bot deployment was skipped.
Suggested change
if (refreshToken) {
void this.tryDeployBotService(refreshToken, appId, tenantId, stateData).catch((err) => {
this.logger.warn(
`Azure setup: ARM deployment failed (non-fatal, health check will reflect) integrationId=${stateData.integrationId} error="${this.axiosErrorMessage(err)}" responseBody=${JSON.stringify((err as AxiosError)?.response?.data ?? null)}`
);
});
} else {
this.logger.warn(
`Azure setup: no refresh token available, skipping ARM deployment integrationId=${stateData.integrationId}`
);
void this.writeProvisioning(stateData, {
status: 'failed',
completedAt: new Date().toISOString(),
errorMessage: 'No refresh token available — ARM deployment was skipped.',
});
+
+ return {
+ html: AzureSetupOauthCallback.buildPopupHtml({
+ success: false,
+ errorMessage: 'Azure did not return a refresh token, so bot deployment could not start.',
+ }),
+ };
}
return { html: AzureSetupOauthCallback.buildPopupHtml({ success: true }) };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/api/src/app/integrations/usecases/azure-setup-oauth-callback/azure-setup-oauth-callback.usecase.ts`
around lines 97 - 108, The code marks provisioning failed via
writeProvisioning(...) but still returns
AzureSetupOauthCallback.buildPopupHtml({ success: true }), which incorrectly
signals success; update the else branch so the returned popup reflects failure
(e.g., call AzureSetupOauthCallback.buildPopupHtml({ success: false,
errorMessage: 'No refresh token available — ARM deployment was skipped.' }) or
at minimum { success: false }) after writeProvisioning; locate the else branch
around the writeProvisioning call (references: writeProvisioning,
AzureSetupOauthCallback.buildPopupHtml, stateData.integrationId) and change the
returned payload to indicate failure instead of success.
Comment on lines
+374
to
+383
| private async getTenantId(accessToken: string): Promise<string> { | ||
| try { | ||
| const response = await axios.get<{ value: Array<{ id: string }> }>(`${MS_GRAPH_BASE_URL}/organization`, { | ||
| headers: this.graphHeaders(accessToken), | ||
| }); | ||
|
|
||
| return response.data.value[0]?.id ?? ''; | ||
| } catch { | ||
| return ''; | ||
| } |
Contributor
There was a problem hiding this comment.
Don't continue with an empty tenant ID.
Returning '' here lets saveCredentials() persist unusable credentials and later sends an empty msaAppTenantId into ARM deployment. If tenant resolution fails, abort the setup instead of continuing with a partially configured integration.
Suggested change
private async getTenantId(accessToken: string): Promise<string> {
try {
const response = await axios.get<{ value: Array<{ id: string }> }>(`${MS_GRAPH_BASE_URL}/organization`, {
headers: this.graphHeaders(accessToken),
});
- return response.data.value[0]?.id ?? '';
- } catch {
- return '';
+ const tenantId = response.data.value[0]?.id;
+ if (!tenantId) {
+ throw new Error('Microsoft Graph did not return an organization id');
+ }
+
+ return tenantId;
+ } catch (error) {
+ throw new BadRequestException(`Failed to determine tenant id: ${this.axiosErrorMessage(error)}`);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/api/src/app/integrations/usecases/azure-setup-oauth-callback/azure-setup-oauth-callback.usecase.ts`
around lines 374 - 383, getTenantId currently swallows errors and returns an
empty string which lets saveCredentials persist unusable creds and an empty
msaAppTenantId; change getTenantId (in azure-setup-oauth-callback.usecase.ts) to
throw a descriptive Error when tenant resolution fails (include the accessToken
context minimally or the original error message) instead of returning ''. Also
ensure the caller path that invokes getTenantId (the flow that calls
saveCredentials()) does not proceed on a thrown error — i.e., remove any logic
that persists credentials when tenantId is falsy and let the thrown error abort
the setup so saveCredentials() is never called with an empty msaAppTenantId.
Comment on lines
+27
to
+53
| const nameBytes = new TextEncoder().encode(file.name); | ||
| const crc = crc32(file.data); | ||
|
|
||
| const local = new ArrayBuffer(30 + nameBytes.length); | ||
| const lv = new DataView(local); | ||
| lv.setUint32(0, 0x04034b50, true); | ||
| lv.setUint16(4, 20, true); | ||
| lv.setUint16(8, 0, true); | ||
| lv.setUint32(14, crc, true); | ||
| lv.setUint32(18, file.data.length, true); | ||
| lv.setUint32(22, file.data.length, true); | ||
| lv.setUint16(26, nameBytes.length, true); | ||
| new Uint8Array(local).set(nameBytes, 30); | ||
|
|
||
| parts.push(new Uint8Array(local), file.data); | ||
|
|
||
| const central = new ArrayBuffer(46 + nameBytes.length); | ||
| const cv = new DataView(central); | ||
| cv.setUint32(0, 0x02014b50, true); | ||
| cv.setUint16(4, 20, true); | ||
| cv.setUint16(6, 20, true); | ||
| cv.setUint32(16, crc, true); | ||
| cv.setUint32(20, file.data.length, true); | ||
| cv.setUint32(24, file.data.length, true); | ||
| cv.setUint16(28, nameBytes.length, true); | ||
| cv.setUint32(42, offset, true); | ||
| new Uint8Array(central).set(nameBytes, 46); |
Contributor
There was a problem hiding this comment.
Mirror the ZIP UTF-8 filename flag in the browser builder too.
Line 27 encodes names with TextEncoder, but the local and central headers never set the UTF-8 language encoding flag, so ZIP readers will interpret non-ASCII filenames with the wrong code page. The current Teams package uses ASCII names, but this shared utility is still incorrect for localized filenames.
Proposed fix
export function buildZip(files: BrowserZipEntry[]): Blob {
const parts: Uint8Array[] = [];
const centralEntries: Uint8Array[] = [];
let offset = 0;
for (const file of files) {
const nameBytes = new TextEncoder().encode(file.name);
const crc = crc32(file.data);
+ const generalPurposeBitFlag = 0x0800;
const local = new ArrayBuffer(30 + nameBytes.length);
const lv = new DataView(local);
lv.setUint32(0, 0x04034b50, true);
lv.setUint16(4, 20, true);
+ lv.setUint16(6, generalPurposeBitFlag, true);
lv.setUint16(8, 0, true);
@@
const central = new ArrayBuffer(46 + nameBytes.length);
const cv = new DataView(central);
cv.setUint32(0, 0x02014b50, true);
cv.setUint16(4, 20, true);
cv.setUint16(6, 20, true);
+ cv.setUint16(8, generalPurposeBitFlag, true);
cv.setUint32(16, crc, true);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const nameBytes = new TextEncoder().encode(file.name); | |
| const crc = crc32(file.data); | |
| const local = new ArrayBuffer(30 + nameBytes.length); | |
| const lv = new DataView(local); | |
| lv.setUint32(0, 0x04034b50, true); | |
| lv.setUint16(4, 20, true); | |
| lv.setUint16(8, 0, true); | |
| lv.setUint32(14, crc, true); | |
| lv.setUint32(18, file.data.length, true); | |
| lv.setUint32(22, file.data.length, true); | |
| lv.setUint16(26, nameBytes.length, true); | |
| new Uint8Array(local).set(nameBytes, 30); | |
| parts.push(new Uint8Array(local), file.data); | |
| const central = new ArrayBuffer(46 + nameBytes.length); | |
| const cv = new DataView(central); | |
| cv.setUint32(0, 0x02014b50, true); | |
| cv.setUint16(4, 20, true); | |
| cv.setUint16(6, 20, true); | |
| cv.setUint32(16, crc, true); | |
| cv.setUint32(20, file.data.length, true); | |
| cv.setUint32(24, file.data.length, true); | |
| cv.setUint16(28, nameBytes.length, true); | |
| cv.setUint32(42, offset, true); | |
| new Uint8Array(central).set(nameBytes, 46); | |
| const nameBytes = new TextEncoder().encode(file.name); | |
| const crc = crc32(file.data); | |
| const generalPurposeBitFlag = 0x0800; | |
| const local = new ArrayBuffer(30 + nameBytes.length); | |
| const lv = new DataView(local); | |
| lv.setUint32(0, 0x04034b50, true); | |
| lv.setUint16(4, 20, true); | |
| lv.setUint16(6, generalPurposeBitFlag, true); | |
| lv.setUint16(8, 0, true); | |
| lv.setUint32(14, crc, true); | |
| lv.setUint32(18, file.data.length, true); | |
| lv.setUint32(22, file.data.length, true); | |
| lv.setUint16(26, nameBytes.length, true); | |
| new Uint8Array(local).set(nameBytes, 30); | |
| parts.push(new Uint8Array(local), file.data); | |
| const central = new ArrayBuffer(46 + nameBytes.length); | |
| const cv = new DataView(central); | |
| cv.setUint32(0, 0x02014b50, true); | |
| cv.setUint16(4, 20, true); | |
| cv.setUint16(6, 20, true); | |
| cv.setUint16(8, generalPurposeBitFlag, true); | |
| cv.setUint32(16, crc, true); | |
| cv.setUint32(20, file.data.length, true); | |
| cv.setUint32(24, file.data.length, true); | |
| cv.setUint16(28, nameBytes.length, true); | |
| cv.setUint32(42, offset, true); | |
| new Uint8Array(central).set(nameBytes, 46); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/utils/build-zip.ts` around lines 27 - 53, The local and
central directory headers created in build-zip.ts encode filenames with
TextEncoder into nameBytes but never set the UTF-8 language encoding flag, so
non-ASCII names are misinterpreted; update the general-purpose bit flag in both
the local header (DataView lv) and the central header (DataView cv) to include
the UTF-8 bit (bit 11 / 0x0800) — preserving any existing flags (OR the value) —
so the encoded names are marked as UTF-8 for ZIP readers.
Comment on lines
+31
to
+68
| const nameBytes = Buffer.from(file.name, 'utf-8'); | ||
| const crc = crc32(file.data); | ||
|
|
||
| const local = Buffer.allocUnsafe(30 + nameBytes.length); | ||
| local.writeUInt32LE(0x04034b50, 0); | ||
| local.writeUInt16LE(20, 4); | ||
| local.writeUInt16LE(0, 6); | ||
| local.writeUInt16LE(0, 8); | ||
| local.writeUInt16LE(0, 10); | ||
| local.writeUInt16LE(0, 12); | ||
| local.writeUInt32LE(crc, 14); | ||
| local.writeUInt32LE(file.data.length, 18); | ||
| local.writeUInt32LE(file.data.length, 22); | ||
| local.writeUInt16LE(nameBytes.length, 26); | ||
| local.writeUInt16LE(0, 28); | ||
| nameBytes.copy(local, 30); | ||
|
|
||
| parts.push(local, file.data); | ||
|
|
||
| const central = Buffer.allocUnsafe(46 + nameBytes.length); | ||
| central.writeUInt32LE(0x02014b50, 0); | ||
| central.writeUInt16LE(20, 4); | ||
| central.writeUInt16LE(20, 6); | ||
| central.writeUInt16LE(0, 8); | ||
| central.writeUInt16LE(0, 10); | ||
| central.writeUInt16LE(0, 12); | ||
| central.writeUInt16LE(0, 14); | ||
| central.writeUInt32LE(crc, 16); | ||
| central.writeUInt32LE(file.data.length, 20); | ||
| central.writeUInt32LE(file.data.length, 24); | ||
| central.writeUInt16LE(nameBytes.length, 28); | ||
| central.writeUInt16LE(0, 30); | ||
| central.writeUInt16LE(0, 32); | ||
| central.writeUInt16LE(0, 34); | ||
| central.writeUInt16LE(0, 36); | ||
| central.writeUInt32LE(0, 38); | ||
| central.writeUInt32LE(offset, 42); | ||
| nameBytes.copy(central, 46); |
Contributor
There was a problem hiding this comment.
Set the ZIP UTF-8 filename flag when writing UTF-8 names.
Line 31 encodes entry names as UTF-8, but Lines 37 and 54 leave the general-purpose bit flag at 0. That produces incorrectly decoded filenames for any non-ASCII entry name even though the file data is otherwise valid. Current Teams assets are ASCII-only, but this exported helper will break as soon as a caller passes a localized filename.
Proposed fix
export function buildZip(files: ZipEntry[]): Buffer {
const parts: Buffer[] = [];
const centralEntries: Buffer[] = [];
let offset = 0;
for (const file of files) {
const nameBytes = Buffer.from(file.name, 'utf-8');
const crc = crc32(file.data);
+ const generalPurposeBitFlag = 0x0800;
const local = Buffer.allocUnsafe(30 + nameBytes.length);
local.writeUInt32LE(0x04034b50, 0);
local.writeUInt16LE(20, 4);
- local.writeUInt16LE(0, 6);
+ local.writeUInt16LE(generalPurposeBitFlag, 6);
local.writeUInt16LE(0, 8);
@@
const central = Buffer.allocUnsafe(46 + nameBytes.length);
central.writeUInt32LE(0x02014b50, 0);
central.writeUInt16LE(20, 4);
central.writeUInt16LE(20, 6);
- central.writeUInt16LE(0, 8);
+ central.writeUInt16LE(generalPurposeBitFlag, 8);
central.writeUInt16LE(0, 10);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const nameBytes = Buffer.from(file.name, 'utf-8'); | |
| const crc = crc32(file.data); | |
| const local = Buffer.allocUnsafe(30 + nameBytes.length); | |
| local.writeUInt32LE(0x04034b50, 0); | |
| local.writeUInt16LE(20, 4); | |
| local.writeUInt16LE(0, 6); | |
| local.writeUInt16LE(0, 8); | |
| local.writeUInt16LE(0, 10); | |
| local.writeUInt16LE(0, 12); | |
| local.writeUInt32LE(crc, 14); | |
| local.writeUInt32LE(file.data.length, 18); | |
| local.writeUInt32LE(file.data.length, 22); | |
| local.writeUInt16LE(nameBytes.length, 26); | |
| local.writeUInt16LE(0, 28); | |
| nameBytes.copy(local, 30); | |
| parts.push(local, file.data); | |
| const central = Buffer.allocUnsafe(46 + nameBytes.length); | |
| central.writeUInt32LE(0x02014b50, 0); | |
| central.writeUInt16LE(20, 4); | |
| central.writeUInt16LE(20, 6); | |
| central.writeUInt16LE(0, 8); | |
| central.writeUInt16LE(0, 10); | |
| central.writeUInt16LE(0, 12); | |
| central.writeUInt16LE(0, 14); | |
| central.writeUInt32LE(crc, 16); | |
| central.writeUInt32LE(file.data.length, 20); | |
| central.writeUInt32LE(file.data.length, 24); | |
| central.writeUInt16LE(nameBytes.length, 28); | |
| central.writeUInt16LE(0, 30); | |
| central.writeUInt16LE(0, 32); | |
| central.writeUInt16LE(0, 34); | |
| central.writeUInt16LE(0, 36); | |
| central.writeUInt32LE(0, 38); | |
| central.writeUInt32LE(offset, 42); | |
| nameBytes.copy(central, 46); | |
| const nameBytes = Buffer.from(file.name, 'utf-8'); | |
| const crc = crc32(file.data); | |
| const generalPurposeBitFlag = 0x0800; | |
| const local = Buffer.allocUnsafe(30 + nameBytes.length); | |
| local.writeUInt32LE(0x04034b50, 0); | |
| local.writeUInt16LE(20, 4); | |
| local.writeUInt16LE(generalPurposeBitFlag, 6); | |
| local.writeUInt16LE(0, 8); | |
| local.writeUInt16LE(0, 10); | |
| local.writeUInt16LE(0, 12); | |
| local.writeUInt32LE(crc, 14); | |
| local.writeUInt32LE(file.data.length, 18); | |
| local.writeUInt32LE(file.data.length, 22); | |
| local.writeUInt16LE(nameBytes.length, 26); | |
| local.writeUInt16LE(0, 28); | |
| nameBytes.copy(local, 30); | |
| parts.push(local, file.data); | |
| const central = Buffer.allocUnsafe(46 + nameBytes.length); | |
| central.writeUInt32LE(0x02014b50, 0); | |
| central.writeUInt16LE(20, 4); | |
| central.writeUInt16LE(20, 6); | |
| central.writeUInt16LE(generalPurposeBitFlag, 8); | |
| central.writeUInt16LE(0, 10); | |
| central.writeUInt16LE(0, 12); | |
| central.writeUInt16LE(0, 14); | |
| central.writeUInt32LE(crc, 16); | |
| central.writeUInt32LE(file.data.length, 20); | |
| central.writeUInt32LE(file.data.length, 24); | |
| central.writeUInt16LE(nameBytes.length, 28); | |
| central.writeUInt16LE(0, 30); | |
| central.writeUInt16LE(0, 32); | |
| central.writeUInt16LE(0, 34); | |
| central.writeUInt16LE(0, 36); | |
| central.writeUInt32LE(0, 38); | |
| central.writeUInt32LE(offset, 42); | |
| nameBytes.copy(central, 46); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/application-generic/src/utils/build-zip.ts` around lines 31 - 68, The
ZIP entry names are written as UTF-8 but the UTF-8 general-purpose bit (bit 11)
is left cleared; update the local and central header flag fields to set 0x0800
so filenames decode as UTF-8: in build-zip.ts modify the Buffer.writeUInt16LE
call that writes the general-purpose bit for the local header (the writeUInt16LE
at offset 6 on the Buffer named local) and the corresponding call for the
central directory header (the writeUInt16LE at offset 8 on the Buffer named
central) to OR in 0x0800 instead of 0.
Contributor
LaunchDarkly flag references🔍 1 flag added or modified
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
libs/dal/src/repositories/integration/integration.schema.ts (1)
84-85: 🏗️ Heavy liftConsider
Datetypes for provisioning timestamps instead ofString.Lines 84-85 as strings permit inconsistent formats and weaker validation.
Schema.Types.Dateis safer for integrity and future date-based queries; this should be coordinated with entity typing and a migration/backfill plan.Schema-side change (requires coordinated follow-up)
- startedAt: Schema.Types.String, - completedAt: Schema.Types.String, + startedAt: Schema.Types.Date, + completedAt: Schema.Types.Date,As per coding guidelines,
libs/dal/**: “Flag any schema changes that need migrations.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/dal/src/repositories/integration/integration.schema.ts` around lines 84 - 85, Change the integration schema fields startedAt and completedAt from Schema.Types.String to Schema.Types.Date in integration.schema.ts (replace Schema.Types.String usages for startedAt and completedAt with Schema.Types.Date), update the corresponding entity/TypeScript types to use Date, and add a note or TODO in the DAL indicating this is a breaking schema change that requires a migration/backfill plan to convert existing string timestamps to real Date values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/integrations/integrations.controller.ts`:
- Around line 442-454: The endpoint getMsTeamsArmTemplateDeployUrl is currently
protected with `@RequirePermissions`(PermissionsEnum.INTEGRATION_READ) but should
require write access; update the decorator on getMsTeamsArmTemplateDeployUrl to
use `@RequirePermissions`(PermissionsEnum.INTEGRATION_WRITE) (keep other
decorators like `@RequireAuthentication` and the GenerateMsTeamsArmTemplateUsecase
call unchanged) so only users with INTEGRATION_WRITE can mint the Azure deploy
URL.
- Around line 596-602: The catch block in integrations.controller.ts currently
forwards raw err.message into AzureSetupOauthCallback.buildPopupHtml (via
errorMessage) which can create an XSS sink; change the logic in the catch (err:
unknown) handler to pass a fixed, generic user-facing string (e.g. "An
unexpected error occurred while completing Azure setup.") to
AzureSetupOauthCallback.buildPopupHtml instead of err.message, and move the
original err (or err.message/stack) into a server-side log call (e.g.
processLogger.error or similar) so the real error is recorded but never rendered
into the HTML response.
In `@libs/dal/src/repositories/integration/integration.schema.ts`:
- Line 83: The provisioning.status field is currently an unconstrained string
which permits invalid persisted states; update the integration schema
(provisioning.status) to restrict values to the allowed set ("pending", "ready",
"failed") by adding an enum constraint on the Schema type (and optionally set
required/default if desired) so the repository enforces only supported states at
the model level.
---
Nitpick comments:
In `@libs/dal/src/repositories/integration/integration.schema.ts`:
- Around line 84-85: Change the integration schema fields startedAt and
completedAt from Schema.Types.String to Schema.Types.Date in
integration.schema.ts (replace Schema.Types.String usages for startedAt and
completedAt with Schema.Types.Date), update the corresponding entity/TypeScript
types to use Date, and add a note or TODO in the DAL indicating this is a
breaking schema change that requires a migration/backfill plan to convert
existing string timestamps to real Date values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0715894a-7819-42ac-9e87-0ab97d7df934
📒 Files selected for processing (7)
apps/api/src/app/integrations/integrations.controller.tsapps/dashboard/src/components/agents/agent-integrations-tab.tsxapps/dashboard/src/components/agents/teams-setup-guide.tsxlibs/application-generic/src/utils/index.tslibs/dal/src/repositories/integration/integration.entity.tslibs/dal/src/repositories/integration/integration.schema.tspackages/shared/src/types/feature-flags.ts
✅ Files skipped from review due to trivial changes (2)
- apps/dashboard/src/components/agents/agent-integrations-tab.tsx
- libs/dal/src/repositories/integration/integration.entity.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/shared/src/types/feature-flags.ts
- libs/application-generic/src/utils/index.ts
- apps/dashboard/src/components/agents/teams-setup-guide.tsx
… documentation - Simplified the OAuth URL generation in GenerateMsTeamsOauthUrl by removing commented-out code. - Corrected the required permissions documentation in MsTeamsTokenService to reflect the correct scope for user-level installations.
…uthCallback - Removed unnecessary parameters and streamlined the buildPopupHtml method to focus solely on closing the popup window, enhancing code clarity and maintainability.
- Added PinoLogger to the IntegrationsController for improved error logging during Azure OAuth callback failures.
- Updated the integration schema to enforce an enum for the provisioning status, ensuring only valid states ('pending', 'ready', 'failed') are accepted.
…lateDeployUrl method - Changed the permission requirement for the getMsTeamsArmTemplateDeployUrl method from INTEGRATION_READ to INTEGRATION_WRITE, ensuring proper access control for deployment actions.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/api/src/app/integrations/integrations.controller.ts (1)
490-516:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFeature-flag gate for
getAzureSetupOauthUrlis still absent.The dashboard hides Quick Setup behind
IS_MSTEAMS_QUICK_SETUP_ENABLED, but this endpoint still issues a live signed OAuth URL to any caller withINTEGRATION_WRITE. The rollout flag is enforced only in the UI, leaving the API surface fully open.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/integrations.controller.ts` around lines 490 - 516, Add a feature-flag gate at the start of the getAzureSetupOauthUrl handler to prevent issuing OAuth URLs when the rollout is disabled: check the IS_MSTEAMS_QUICK_SETUP_ENABLED flag (via your feature flag/config service using the user.organizationId/environmentId or global config) and if it is false throw a 403/Forbidden (or appropriate NestJS HttpException) before calling generateAzureSetupOauthUrlUsecase or creating GenerateAzureSetupOauthUrlCommand; implement this check inside the getAzureSetupOauthUrl method so the API surface matches the dashboard behavior.
🧹 Nitpick comments (1)
apps/api/src/app/integrations/integrations.controller.ts (1)
575-612: 💤 Low valueInconsistent HTTP status codes for error HTML responses within the same callback handler.
The missing-
statebranch sends HTTP 400 with the HTML popup (line 577), while thecatchblock sends HTTP 200 (line 604). Both paths render the same self-closing popup HTML and browsers execute the inline script regardless of status, but the inconsistency can confuse error monitoring and any upstream proxy that logs non-2xx responses from the OAuth redirect.Pick one and apply it uniformly — HTTP 200 is conventional for HTML popup handlers since the semantic response is the page content, not a REST error:
♻️ Proposed fix
if (!state) { res - .status(400) + .status(200) .type('html') .send( AzureSetupOauthCallback.buildPopupHtml({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/integrations.controller.ts` around lines 575 - 612, The handler uses two different HTTP status codes for error HTML: change the missing-state branch to return status 200 (instead of 400) to match the catch block so error popup HTML responses are consistent; update the response in the branch that builds AzureSetupOauthCallback.buildPopupHtml({ success: false, ... }) where state is falsy so it calls res.status(200).type('html').send(...), leaving the try/catch and calls to this.azureSetupOauthCallbackUsecase.execute and AzureSetupOauthCallbackCommand.create unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/api/src/app/integrations/integrations.controller.ts`:
- Around line 490-516: Add a feature-flag gate at the start of the
getAzureSetupOauthUrl handler to prevent issuing OAuth URLs when the rollout is
disabled: check the IS_MSTEAMS_QUICK_SETUP_ENABLED flag (via your feature
flag/config service using the user.organizationId/environmentId or global
config) and if it is false throw a 403/Forbidden (or appropriate NestJS
HttpException) before calling generateAzureSetupOauthUrlUsecase or creating
GenerateAzureSetupOauthUrlCommand; implement this check inside the
getAzureSetupOauthUrl method so the API surface matches the dashboard behavior.
---
Nitpick comments:
In `@apps/api/src/app/integrations/integrations.controller.ts`:
- Around line 575-612: The handler uses two different HTTP status codes for
error HTML: change the missing-state branch to return status 200 (instead of
400) to match the catch block so error popup HTML responses are consistent;
update the response in the branch that builds
AzureSetupOauthCallback.buildPopupHtml({ success: false, ... }) where state is
falsy so it calls res.status(200).type('html').send(...), leaving the try/catch
and calls to this.azureSetupOauthCallbackUsecase.execute and
AzureSetupOauthCallbackCommand.create unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 784e24e4-e6f4-4733-992d-92a9200bcb90
📒 Files selected for processing (3)
apps/api/src/app/integrations/integrations.controller.tsapps/api/src/app/integrations/usecases/azure-setup-oauth-callback/azure-setup-oauth-callback.usecase.tslibs/dal/src/repositories/integration/integration.schema.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/src/app/integrations/usecases/azure-setup-oauth-callback/azure-setup-oauth-callback.usecase.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Related Linear Issue
Flow
Breaking Changes
None.
Screenshots
Not included. This updates an existing setup guide flow and API-backed state, but no screenshot was captured in this pass.
Test Plan
Made with Cursor
What changed
Automates Microsoft Teams onboarding by adding Azure deployment and OAuth flows plus health verification. The API now generates signed ARM template deploy URLs and serves ARM templates, issues Azure AD OAuth URLs and handles callbacks to create app registrations/service principals and encrypt credentials, and exposes a health-check endpoint to validate Graph tokens, provisioning state, app catalog presence, and permissions. The dashboard guides users through quick and manual setup flows, polls health status, and surfaces deployment progress. Integration metadata now tracks provisioning status, timestamps, errors, and Teams app catalog IDs.
Affected areas
api-service: New endpoints for ARM deploy URL, ARM template JSON (signed sig/exp), Azure OAuth URL, OAuth callback handling (creates app registration, service principal, client secret, best-effort Teams app catalog upload, optional ARM provisioning), and a multi-check Teams health endpoint; added PinoLogger to controllers and stricter provisioning enum in the integration schema; popup HTML simplified to close window; fixed permission for deploy-URL endpoint to require write.
dashboard: Reworked TeamsSetupGuide to support feature-flagged Quick vs Manual flows, added health-check polling and UI checkpoints, integrated ARM deploy button, manual fallback steps, Connect-and-Link polling, and Teams app package download (now uses shared ZIP builder).
dal: Integration entity/schema extended with provisioning subdocument (status: 'pending'|'ready'|'failed', timestamps, errorMessage, teamsAppCatalogId).
shared: Added IS_MSTEAMS_QUICK_SETUP_ENABLED feature flag key.
application-generic & dashboard utils: Added uncompressed ZIP builders for Node/browser and re-used buildZip for Teams app packages.
Key technical decisions
Testing
Updated unit tests for MS Teams OAuth callback now assert thrown exceptions for error conditions; manual end-to-end verification is required for Azure deployment/OAuth flow and health-check behavior; dashboard UI behavior covered by manual interaction checks.