Fix MCP UAMI persistence and dynamic-values identity threading#9265
Fix MCP UAMI persistence and dynamic-values identity threading#9265AbodeSaafan wants to merge 5 commits into
Conversation
…mption) Bug 1: After picking a UAMI in the MCP connection creation panel, the MCP server node on the canvas reverted to system-assigned identity. The connection-update thunk re-derived identity from the Logic App's identity config, which failed for hybrid 'SystemAssigned, UserAssigned' apps. Now reads the identity stored on the connection first. Bug 2: listMcpTools (and other dynamic-value calls) in the v1 designer ran without the selected identity. Now threads the identity from connectionReference.connectionProperties.authentication.identity through getListDynamicValues, and _buildMcpAuthentication prefers an explicit override over the Logic App identity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔒 AI Validation PendingThis PR is from an external contributor. AI validation will be performed after manual review by a maintainer. Maintainers: Add the |
There was a problem hiding this comment.
Pull request overview
This PR fixes managed identity (UAMI) propagation for MCP server connections and MCP dynamic-values in the v1 designer by preferring the identity stored on the connection (across both single-auth and multi-auth storage shapes) and threading the selected identity through dynamic-values calls and service-layer authentication building.
Changes:
- Add
getManagedIdentityFromConnectionhelper and use it to preserve a user-selected UAMI when re-hydrating connection properties. - Thread an optional
identitythroughgetDynamicValues→getListDynamicValues(query + cache key) → connector service calls. - Add/extend unit tests validating identity persistence, cache separation by identity, and service-layer MSI auth priority behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/connector.ts | Adds identity override/effective identity selection so MCP MSI auth keeps the selected UAMI (including hybrid identity apps). |
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/tests/connector.spec.ts | Adds tests covering identity propagation and MSI identity resolution priority for MCP auth. |
| libs/designer/src/lib/core/utils/parameters/dynamicdata.ts | Extracts selected identity from connectionReference and forwards it into dynamic-values fetching. |
| libs/designer/src/lib/core/utils/connectors/connections.ts | Introduces getManagedIdentityFromConnection helper to read identity from both connection parameter storage shapes. |
| libs/designer/src/lib/core/utils/connectors/test/connections.spec.ts | New unit tests validating the helper across multi-auth/single-auth shapes and precedence. |
| libs/designer/src/lib/core/queries/connector.ts | Extends getListDynamicValues to accept identity, include it in the cache key, and forward it to the connector service. |
| [ | ||
| 'listdynamicvalues', | ||
| (connectionId ?? '').toLowerCase(), | ||
| connectorId.toLowerCase(), | ||
| operationId.toLowerCase(), | ||
| dynamicState.operationId?.toLowerCase(), | ||
| getParametersKey({ ...dynamicState.parameters, ...parameters }), | ||
| identity ?? '', | ||
| ], |
There was a problem hiding this comment.
Good catch normalized to lowercase in the cache key (a85b8e5) so case-variant ARM IDs collapse to the same entry. The service call still passes the raw identity to preserve the canonical ID. Added a test in connector.spec.ts that asserts a case-variant second call is a cache hit.
|
📊 Coverage check completed. See workflow run for details. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three v2-specific gaps still left UAMI broken after publish: - mcpToolWizard.tsx: getListDynamicValues was missing the identity arg, so listMcpTools fetched tools without the selected UAMI. - consumption connector managed branch used bare identity instead of the effective identity resolved from parameter values/connection. - ParseReduxAction reconstructed connectionReferences from $connections but dropped connectionProperties, connectionRuntimeUrl, authentication, and impersonation. This explained the 'works only after publish' symptom: dynamic data could not see the chosen identity until a fresh load via the API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… in v2 Built-in MCP connections returned from Azure store their parameters in parameterValueSet.values.<key>.value when a non-default authentication set is selected (e.g. managedServiceIdentity). The consumption connector's getListDynamicValues branch only checked the flat parameterValues shape, so UAMI connections fell through to the managed-MCP path and listMcpTools was called without identity. Add a _flattenConnectionParameters helper that merges both shapes (parameterValueSet first, then parameterValues for any keys not already set) and use it for branch detection and authentication building. Updated tests exercise the real parameterValueSet-only Azure shape instead of the redundant dual-shape fixture that previously masked the bug. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Found another root cause for the v2 issue you hit at https://localhost:4200/v2 pushed as Root cause: Built-in MCP connections that use UAMI are returned by Azure in the Fix: Added No changes needed to the v2 |
Managed MCP connections persist UAMI at properties.parameterValues.authentication.identity, which the existing reader helpers (flat parameterValueSet / parameterValues paths only) missed. As a result, listMcpTools was posted with no identity even after the user had explicitly picked a UAMI on the existing connection. Extends both getManagedIdentityFromConnection (v1 + v2) and the consumption connector's listMcpTools identity resolution with a third fallback that reads parameterValues.authentication.identity, gated on authentication.type === 'ManagedServiceIdentity' to avoid misreading other auth shapes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pushed 313a51d with a fix for the v2 issue you reported (calling Root cause: Fix: added a third fallback that reads
Added 5 tests (positive nested-shape + negative type-guard in v1/v2 helpers, plus a connector test mirroring the existing managed-MCP UAMI test with the nested shape). All 40 tests in the 3 specs pass. Out of scope: the pre-publish 401 you saw when manually injecting the identity is a backend requirement (workflow must be published before |
Commit Type
Risk Level
What & Why
Two MCP connection bugs in the Consumption designer:
Bug 1: MCP server node reverts to system-assigned after saving a UAMI connection. The connection thunk re-derived the identity from the Logic App via
WorkflowService().getAppIdentity()with a strictequals(type, 'UserAssigned')check, dropping the user's UAMI selection (and failing entirely for hybridSystemAssigned, UserAssignedidentities), so the canvas node fell back to system-assigned.Bug 2:
listMcpToolswas called without the UAMI identity in v1.getListDynamicValueshardcoded the identity toundefined, so both the cache key and the service call omitted the user's selected identity, causing tool listings to use the wrong auth.Fix:
getManagedIdentityFromConnectionhelper reads the stored identity from both connection-storage shapes (multi-authparameterValueSet.values.identity.valueand single-authparameterValues.identity).getConnectionPropertiesIfRequiredprefers the stored connection identity, falling back to the Logic App identity only when none is stored.getListDynamicValuesaccepts an optionalidentityarg, includes it (lowercased) in the cache key, and forwards it to the service.dynamicdata.tsextracts the identity fromconnectionReference.connectionProperties.authentication.identityand threads it through._buildMcpAuthenticationaccepts anidentityOverrideand the caller computes aneffectiveIdentitypriority chain.Impact of Change
listMcpToolscalls use the right identity.getManagedIdentityFromConnectionhelper.getListDynamicValuesand_buildMcpAuthenticationgained an optional identity arg/override.Test Plan
13 new unit tests across 3 files (1 new spec), all passing:
connections.spec.ts(new) — 5 tests for the helper across both storage shapesconnector.spec.ts(designer) — 3 tests for identity threading, cache-key separation, and case-insensitive cache reuseconnector.spec.ts(shared) — 5 tests for_buildMcpAuthenticationpriority and caller identity selectionFull targeted runs green. Full suites (873 designer + 2105 shared) still pass.
Contributors
Screenshots/Videos