Skip to content

fix(designer): Fix OAuth adding extra parameters not specified on api registration#8798

Merged
Eric-B-Wu merged 8 commits intomainfrom
eric/fixConnection
Feb 7, 2026
Merged

fix(designer): Fix OAuth adding extra parameters not specified on api registration#8798
Eric-B-Wu merged 8 commits intomainfrom
eric/fixConnection

Conversation

@Eric-B-Wu
Copy link
Contributor

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

We were passing back client side parameters back to the api. Some connectors such as Power Platform would then run into the error:
Call to ARM failed. Status code: 'BadRequest'. Response: '{"Code":"BadRequest","Message":"Input parameters are invalid. See details for more information. Details:errorCode: ParameterNotDefined. Message: Parameter 'token:tenantId' is not allowed on the connection since it was not defined as a connection parameter when the API was registered."

Impact of Change

  • Users: Users should now be able to authenticate into previously failed connections
  • Developers: Added some tests for the new changes
  • System: No system changes

Test Plan

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

Contributors

@Eric-B-Wu

Screenshots/Videos

@Eric-B-Wu Eric-B-Wu added the risk:low Low risk change with minimal impact label Feb 7, 2026
Copilot AI review requested due to automatic review settings February 7, 2026 03:13
@github-actions
Copy link

github-actions bot commented Feb 7, 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(designer): Fix OAuth adding extra parameters not specified on api registration
  • Issue: None functional — title is descriptive and follows conventional commit style. Minor suggestion: be explicit about what parameters are being filtered or the behavior change (e.g., preventing client-side-only parameters such as token:tenantId from being sent to connector APIs).
  • Recommendation: Consider a slightly clearer title such as: fix(designer): do not send client-side connection parameters (e.g. token:tenantId) to connector APIs to make the intent explicit in search results and release notes.

Commit Type

  • Properly selected (fix).
  • Note: Only one commit type is selected which is correct for this change.

Risk Level

  • The PR body marks this as Low and the PR has the label risk:low. The code diff and scope of changes (filtering connection parameter sets, adding unit tests, and small serializer fixes/formatting) are in line with a low-risk change.

What & Why

  • Current: The body explains that client-side parameters were being passed back to connector APIs and that caused failures for some connectors (example error included).
  • Issue: None — the explanation is clear and includes the motivating example error message.
  • Recommendation: Optionally add a one-line note referencing the specific files changed (eg. CreateConnectionInternal getConnectionParameterSetValues) so reviewers can quickly map the description to the diff.

Impact of Change

  • Impact is described for Users, Developers, and System.
  • Recommendation: Good. If you want to be extra explicit, name the consumers (e.g., connectors/Power Platform) in the Users section — you already referenced Power Platform in the What & Why, which is sufficient.
    • Users: Users should no longer see connection failures caused by unexpected parameters like token:tenantId.
    • Developers: Tests were added/updated; mention of unit tests is present.
    • System: No architectural changes.

Test Plan

  • The PR body indicates unit tests were added and manual testing completed.
  • Assessment vs diff: Unit tests were indeed added/updated (two test files updated/expanded and matching tests in both libs). E2E tests are not present (which is reasonable for this backend/serializer change). Manual testing is ticked and acceptable.
  • Recommendation: If possible, call out which unit test files were added/modified (you have tests under createConnection/test and BJSWorkflow parser test updates) so reviewers can run them easily.

Contributors

  • Contributors are listed (@Eric-B-Wu). This is fine.
  • Recommendation: If product/design or other reviewers contributed, consider adding them here for credit, but not required.

⚠️ Screenshots/Videos

  • Assessment: Not applicable — no UI visual change requiring screenshots.
  • Recommendation: No screenshots necessary.

Summary Table

Section Status Recommendation
Title Consider making the title slightly more explicit about client-side params filtered (optional)
Commit Type None needed
Risk Level None needed
What & Why Optionally reference key files changed
Impact of Change None needed
Test Plan Consider calling out specific test files
Contributors Optional: add additional contributors if any
Screenshots/Videos ⚠️ Not applicable (no UI change)

Final Message

This PR passes the PR title/body checks.

Quick actionable recommendations to improve clarity:

  • Slightly rephrase the PR title to explicitly call out that client-side-only parameters (e.g. token:tenantId) are being filtered and not sent to connector APIs. Example: fix(designer): do not send client-side connection parameters (e.g. token:tenantId) to connector APIs.
  • In the What & Why or Test Plan, call out the main files changed or tests added (e.g., createConnection/createConnectionInternal.tsx getConnectionParameterSetValues, and the updated unit tests under createConnection/test). This helps reviewers map description to diff quickly.

Everything else looks correct: commit type, risk label, tests added, and the described impact align with the code changes. No risk-level mismatch detected.

Please update the PR title if you'd like to adopt the suggested phrasing; otherwise this is good to proceed. Thank you for the thorough description and tests — these make the change easy to review and verify.


Last updated: Sat, 07 Feb 2026 03:13:57 GMT

Copy link
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 an OAuth connection-creation issue in the Logic Apps Designer where client-side-only parameters could be sent to ARM/ApiHub, causing connector validation failures (e.g., ParameterNotDefined for token:tenantId).

Changes:

  • Filter connectionParametersSet values to only include keys defined in the selected parameter set (designer + designer-v2).
  • Add unit tests validating the filtering behavior (designer + designer-v2).
  • Apply minor formatting-only adjustments in a v2 workflow test fixture and the v2 BJS workflow serializer.

Reviewed changes

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

Show a summary per file
File Description
libs/designer/src/lib/ui/panel/connectionsPanel/createConnection/createConnectionInternal.tsx Filters parameter-set payload keys to avoid sending undefined/extra parameters to the API.
libs/designer/src/lib/ui/panel/connectionsPanel/createConnection/test/createConnectionWrapper.spec.tsx Adds tests to ensure invalid parameter keys are excluded when a valid-key list is provided.
libs/designer-v2/src/lib/ui/panel/connectionsPanel/createConnection/createConnectionInternal.tsx Mirrors the parameter-set payload filtering fix in designer-v2.
libs/designer-v2/src/lib/ui/panel/connectionsPanel/createConnection/test/createConnectionWrapper.spec.tsx Mirrors the unit tests for filtering behavior in designer-v2.
libs/designer-v2/src/lib/core/parsers/BJSWorkflow/test/agentMcpWorkflowDefinition.ts Formatting-only updates to the agent MCP workflow test definition/expected output.
libs/designer-v2/src/lib/core/actions/bjsworkflow/serializer.ts Formatting-only cleanup (no functional changes observed).

@github-actions
Copy link

github-actions bot commented Feb 7, 2026

📊 Coverage check completed. See workflow run for details.

@Eric-B-Wu Eric-B-Wu merged commit af1b22f into main Feb 7, 2026
22 checks passed
@Eric-B-Wu Eric-B-Wu deleted the eric/fixConnection branch February 7, 2026 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants