Skip to content

Conversation

@DhariniJeeva
Copy link
Collaborator

@DhariniJeeva DhariniJeeva commented Nov 10, 2025

Description

This PR contains all the commits for RDP client launch work.

Screenshots (if appropriate)

How to Test

DM me or @priya-patel04 for cluster url

To test setting page functionality:

Navigate to Settings page and verify a new Preferred Clients section is rendered

  • Mac: Windows App should be selected by default as your preferred client
  • Windows: Mstc should be selected by default as your preferred client
    Test updating your preferred client to None. Reopen the app and verify None selection persists.
  • Verfiy electron stores/update preferred client selection in the config:
  • Mac: /Library/Application Support/Boundary/config.json
  • Windows: \AppData\Roaming\Boundary\config.json

To test Target and details page

  • Navigate to Settings page and make sure you have selected either Windows App for mac or Remote Desktop Connection for Windows.
  • Go to Targets list view and click Open for one of the RDP targets, it should launch Windows App for Mac and Mstc if you are testing this on Windows os
  • On targets details page, verify Open button is visible.
  • should see connect for non rdp targets

To test Session details page

  • Navigate to Settings page and make sure you have selected either Windows App for mac or Remote Desktop Connection for Windows.
  • Go to Targets List view page and click Open for one of the RDP targets.
  • Click Continue on the certificate modal to launch the rdp client
  • On Session details page, verify Open button is visible.
  • Click Open button on session details page and it should open the existing rdp session
  • It should not show Open button:
    • for non-rdp targets
    • when preferred client is selected as None

Checklist

  • I have added before and after screenshots for UI changes
  • I have added JSON response output for API changes
  • I have added steps to reproduce and test for bug fixes in the description
  • I have commented on my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have added a11y-tests label to run a11y audit tests if needed

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.
  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
  • If applicable, I've documented the impact of any changes to security controls.
    Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

@vercel
Copy link

vercel bot commented Nov 10, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
boundary-ui Ready Ready Preview Comment Nov 17, 2025 11:26pm
boundary-ui-desktop Ready Ready Preview Comment Nov 17, 2025 11:26pm

Copy link
Collaborator

@priya-patel04 priya-patel04 left a comment

Choose a reason for hiding this comment

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

Everything looks good! Excited to get this in 🎉
I missed updating few magic strings - I have small PR covering it

priya-patel04
priya-patel04 previously approved these changes Nov 13, 2025
priya-patel04 and others added 8 commits November 17, 2025 15:24
* feat: 🎸 added a new RDP service

* fix: 🐛 rdp test

* refactor: 💡 update error handling for fetching rdp clients

* refactor: 💡 addressed comments

* fix: 🐛 revert private field

* refactor: 💡 addressed comments

* refactor: 💡 updated rdp clients to array of strings
* feat: 🎸 preferred clients section on settings page

* chore: 🤖 update mock ipc with new handlers

* refactor: 💡 moved windows rdp protocol to const

* refactor: 💡 moved rdp calls to app route

* test: 💍 fixed failings tests

* refactor: 💡 addressed comments

* fix: 🐛 fixed failing tests

* refactor: 💡 add mock rdp service calls to tests

* refactor: 💡 addressed comments

* test: 💍 add additional tests

* refactor: 💡 fixed test failure
* feat: 🎸 preferred clients section on settings page

* refactor: 💡 moved rdp calls to app route

* test: 💍 fixed failings tests

* refactor: 💡 addressed comments

* fix: 🐛 fixed failing tests

* refactor: 💡 add mock rdp service calls to tests

* feat: 🎸 adds open button to session details page

* fix: 🐛 translation string

* refactor: 💡 address feedback

* refactor: 💡 address comment

* fix: 🐛 test failure
* test: 💍 e2e test for rdp client launch

* test: 💍 added rdp test helper

* refactor: 💡 misc

* refactor: 💡 addressed comments

* refactor: 💡 removed similar test
* refactor: 💡 updated logic to store preferred rdp client

* refactor: 💡 simplified preferred client logic
Copy link
Collaborator

@lisbet-alvarez lisbet-alvarez left a comment

Choose a reason for hiding this comment

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

Looks good! Also tested in mac + windows! 🎉

app.on('quit', () => {
cacheDaemonManager.stop();
// we should stop any active RDP client processes
rdpClientManager.stopAll();
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like we don't stop the processes when we logout but still kill the sessions, should we have killed the RDP processes as well on logout?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, I think we should do the same for logout as well. I will raise a pr to address this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

addressed here

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.

4 participants