Skip to content

fix(MCP): Fix ManagedServiceIdentity auth for Consumption MCP connections (v5.961)#9209

Open
rllyy97 wants to merge 1 commit into
hotfix/v5.961from
rileyevans/cherry-pick/9205-to-hotfix-v5.961
Open

fix(MCP): Fix ManagedServiceIdentity auth for Consumption MCP connections (v5.961)#9209
rllyy97 wants to merge 1 commit into
hotfix/v5.961from
rileyevans/cherry-pick/9205-to-hotfix-v5.961

Conversation

@rllyy97
Copy link
Copy Markdown
Contributor

@rllyy97 rllyy97 commented May 22, 2026

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Fixes #9205 — MCP connections using ManagedServiceIdentity auth in Consumption SKU are missing the identity field in the listMcpTools API request, causing failures for user-assigned managed identity scenarios.

The Consumption connector's MCP handling had two gaps compared to the Standard connector:

  1. Managed MCP path sent a bare { connection: { id } } without connectionProperties or authentication info, so the backend had no identity context.
  2. Built-in MCP _buildMcpAuthentication only read identity from connectionProperties['identity'] (which is never populated by the MCP manifest), with no fallback to the workflow's managed identity configuration.

This PR aligns Consumption with Standard by calling WorkflowService().getAppIdentity() to derive the user-assigned identity resource ID in both paths.

Impact of Change

  • Users: Consumption workflows using MCP connectors with user-assigned managed identity will now correctly send identity in the listMcpTools request, unblocking tool discovery.
  • Developers: No API changes. WorkflowService and ResourceIdentityType are now imported in the Consumption connector service.
  • System: No architectural changes. Adds a synchronous call to WorkflowService().getAppIdentity() which is already available in all host environments.

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in:

Updated connector.spec.ts with InitWorkflowService mock for managed MCP and _buildMcpAuthentication MSI test blocks. All 68 tests pass (38 Consumption + 30 Standard).

Contributors

Screenshots/Videos

N/A — no visual changes.

…9205)

Align Consumption connector with Standard for MCP connections:

- Managed MCP path: build connectionProperties with MSI auth and user-assigned identity from WorkflowService().getAppIdentity()

- Built-in MCP _buildMcpAuthentication: add WorkflowService fallback for identity when not in parameterValues

- Remove debug console.log statements

- Update tests with WorkflowService mock initialization
Copilot AI review requested due to automatic review settings May 22, 2026 02:39
@rllyy97 rllyy97 changed the base branch from main to hotfix/v5.961 May 22, 2026 02:41
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(MCP): Fix ManagedServiceIdentity auth for Consumption MCP connections (v5.961)
  • Issue: None. The title is specific, concise, and clearly describes the change.
  • Recommendation: No change needed.

Commit Type

  • Properly selected (fix).
  • Only one commit type is checked, which is correct.

Risk Level

  • Properly selected (Medium).
  • Advised risk remains Medium based on the diff: the change is scoped to MCP authentication handling and adds/adjusts unit coverage, with limited but real user impact. I do not recommend increasing the risk level.

What & Why

  • Current: Clear and detailed explanation of the two behavior gaps, the affected SKU, and the fix approach.
  • Issue: None.
  • Recommendation: No change needed.

Impact of Change

  • The impact section is well-structured and aligned with the code change.
  • Recommendation:
    • Users: Correctly identifies the affected user group and outcome.
    • Developers: Correctly notes the internal service/import impact.
    • System: Appropriately states there is no architectural change.

Test Plan

  • Unit tests are added/updated in the diff, which satisfies the test plan requirement.
  • The absence of E2E tests is acceptable because unit tests are present.

⚠️ Contributors

  • Blank.
  • Recommendation: Optional, but if others helped with the design or implementation, please add them here for credit.

Screenshots/Videos

  • Not required; the PR has no visual changes.
  • N/A — no visual changes. is appropriate.

Summary Table

Section Status Recommendation
Title No change needed
Commit Type No change needed
Risk Level No change needed
What & Why No change needed
Impact of Change No change needed
Test Plan No change needed
Contributors ⚠️ Optional: add contributors if applicable
Screenshots/Videos No change needed

Overall: this PR passes review for title/body compliance. The advised risk level is Medium and matches the submitter's selection.


Last updated: Fri, 22 May 2026 02:46:25 GMT

@github-actions
Copy link
Copy Markdown
Contributor

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(MCP): Fix ManagedServiceIdentity auth for Consumption MCP connections (v5.961)
  • Issue: No blocking issue. The title is specific and describes the affected area and behavior.
  • Recommendation: Consider shortening slightly if you want to match a more concise conventional style, but it is already high quality.

Commit Type

  • Properly selected (fix).
  • Only one commit type is checked, which is correct.

⚠️ Risk Level

  • The PR body marks Medium, which is reasonable for a targeted auth flow fix with user impact.
  • Advised risk from the diff: still medium. I do not see evidence that this should be escalated to high.

What & Why

  • Current: Clear explanation of the bug, affected SKU, and why the fix is needed.
  • Issue: None blocking.
  • Recommendation: You could make the opening sentence even shorter, but the current detail is strong and actionable.

Impact of Change

  • Impact is described clearly and maps to the code change.
  • Recommendation:
    • Users: Correctly calls out Consumption workflows using user-assigned managed identity.
    • Developers: Good note that there are no API changes.
    • System: Good note that the change is localized and uses an existing service call.

Test Plan

  • Unit tests are added/updated in the diff, which satisfies the test-plan requirement.
  • The absence of E2E tests is acceptable because unit tests are present.
  • Recommendation: If you want to make this even stronger, you could mention the specific behavior verified by the updated tests (managed MCP payload and _buildMcpAuthentication identity fallback).

⚠️ Contributors

  • Blank, but this is not required.
  • Recommendation: Add contributors only if there were PM/design/peer contributors worth crediting.

Screenshots/Videos

  • N/A is appropriate because this is a non-visual change.

Summary Table

Section Status Recommendation
Title
Commit Type
Risk Level
What & Why
Impact of Change
Test Plan Mention the specific unit-test coverage if desired
Contributors ⚠️ Optional; add only if you want to credit collaborators
Screenshots/Videos

Overall: this PR passes. The body is compliant, the selected risk level matches the scope of the diff, and the unit test updates back the test-plan checkbox. The advised risk level remains medium, which matches the submitter's estimate.


Last updated: Fri, 22 May 2026 02:42:43 GMT

@github-actions
Copy link
Copy Markdown
Contributor

📊 Coverage Check

The following changed files need attention:

⚠️ libs/designer-v2/src/lib/core/state/designerView/designerViewSelectors.ts - 61% covered (needs improvement)
⚠️ libs/designer-v2/src/lib/core/state/panel/panelSelectors.ts - 64% covered (needs improvement)
⚠️ libs/designer-v2/src/lib/core/state/panel/panelSlice.ts - 11% covered (needs improvement)
⚠️ libs/logic-apps-shared/src/designer-client-services/lib/consumption/connector.ts - 70% covered (needs improvement)

Please add tests for the uncovered files before merging.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Consumption-SKU MCP listMcpTools requests for Managed Service Identity (MSI) authentication by ensuring the user-assigned managed identity (UAMI) resource ID is included when required (aligning behavior with the Standard connector’s approach of deriving identity from WorkflowService().getAppIdentity()).

Changes:

  • Consumption MCP listMcpTools request building now derives UAMI from WorkflowService().getAppIdentity() and includes it in MSI authentication payloads.
  • Managed MCP connection path now adds connectionProperties.authentication in the request (previously only referenced { connection: { id } }).
  • Consumption connector unit tests updated to initialize WorkflowService to support the new identity-derivation behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
libs/logic-apps-shared/src/designer-client-services/lib/consumption/connector.ts Adds workflow-identity-derived UAMI support for MSI in Consumption MCP listMcpTools payload construction.
libs/logic-apps-shared/src/designer-client-services/lib/consumption/tests/connector.spec.ts Updates Consumption connector tests to initialize WorkflowService for MCP scenarios and validates managed MCP request shape.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

user managed identity didn't send part of the list mcp tool API in new designer

3 participants