Skip to content

fix: Restore active desktop name check in Windows daemon#8194

Merged
nbolton merged 6 commits intomasterfrom
win-client-uac-auto
Feb 19, 2025
Merged

fix: Restore active desktop name check in Windows daemon#8194
nbolton merged 6 commits intomasterfrom
win-client-uac-auto

Conversation

@nbolton
Copy link
Copy Markdown
Member

@nbolton nbolton commented Feb 13, 2025 •
edited
Loading

Fixes: #8183

Blocks:

Restores some of the code deleted in fb686ed (#7827) which is needed to run the core process in secure desktops (UAC prompts, login, lock screen, etc). I thought I tested PR #7827 pretty thoroughly (#7827 (comment)) but it looks like I made a mistake and wasn't thorough enough.

This time we're using deskflow-core.exe deskflow-server.exe (temporarily until core bin ships) instead of deskflow-legacy.exe to find the active desktop name (which is not accessible to processes running in session 0).

  • Test with UAC dialog
  • Test with lock screen
  • Test with login screen
  • Test local built package
  • Test CI package

Testing

Use 'Automatic' elevation:
image

Right click any app and click 'Run as administrator' to show the UAC dialog.

Expect: Mouse should work on client

@sithlord48 sithlord48 mentioned this pull request Feb 14, 2025
Merged
@nbolton nbolton force-pushed the win-client-uac-auto branch 2 times, most recently from 8ef6f26 to 02507a7 Compare February 18, 2025 18:15
@nbolton nbolton requested a review from sithlord48 February 18, 2025 18:21
@nbolton nbolton marked this pull request as ready for review February 18, 2025 18:21
@nbolton nbolton force-pushed the win-client-uac-auto branch from 02507a7 to 3a2c3df Compare February 18, 2025 18:22
@nbolton nbolton mentioned this pull request Feb 18, 2025
Closed
6 tasks
sithlord48
sithlord48 requested changes Feb 18, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@sithlord48 sithlord48 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Have not tested yet on windows.

Edit: Seams to work on windows UAC prompts work but deskflow-core was not installed (if that was intended).

@nbolton nbolton force-pushed the win-client-uac-auto branch 2 times, most recently from 0e4b5f6 to bb7acdd Compare February 19, 2025 10:24
@nbolton nbolton mentioned this pull request Feb 19, 2025
Closed
@sithlord48
Copy link
Copy Markdown
Member

sithlord48 commented Feb 19, 2025

This can no land as is the installer has to be fixed to setup the core properly and we have to make sure that the core does work . I am ok with landing this but i am NOT ok with shipping the core by default yet. There is more work to make that work proper

sithlord48
sithlord48 reviewed Feb 19, 2025
View reviewed changes
@nbolton
Copy link
Copy Markdown
Member Author

nbolton commented Feb 19, 2025 •
edited
Loading

This can no land as is the installer has to be fixed to setup the core properly and we have to make sure that the core does work . I am ok with landing this but i am NOT ok with shipping the core by default yet. There is more work to make that work proper

As discussed on Matrix, we agreed that we should not ship 3 binaries as this would be confusing for people to see:

  • deskflow-core
  • deskflow-server
  • deskflow-client

If running from CLI or browsing the 'Program Files' dir, they might get confused about which to use.

For this reason, I am temporarily moving the --active-desktop arg to the deskflow-server binary (which is a hack) and will move it back to deskflow-core (where it belongs) after landing this PR:

@nbolton nbolton force-pushed the win-client-uac-auto branch from 2391983 to 32b1ccb Compare February 19, 2025 13:45
@nbolton nbolton requested a review from sithlord48 February 19, 2025 13:46
sithlord48
sithlord48 reviewed Feb 19, 2025
View reviewed changes
@nbolton
Copy link
Copy Markdown
Member Author

nbolton commented Feb 19, 2025

@sithlord48 Tested Windows CI package on client, works great for me. How about you?

@sithlord48
Copy link
Copy Markdown
Member

sithlord48 commented Feb 19, 2025

does not break linux..
on windows as a client i get nothing but ipc errors on that machine.

@nbolton
Copy link
Copy Markdown
Member Author

nbolton commented Feb 19, 2025 •
edited
Loading

on windows as a client i get nothing but ipc errors on that machine.

I can't repro that issue anymore. Did you try the latest CI package?

https://github.com/deskflow/deskflow/actions/runs/13414062492/artifacts/2616525149

I wonder if you're seeing IPC errors related to #7804? (progress on which is unfortunately blocked by this PR)

@nbolton nbolton mentioned this pull request Feb 19, 2025
Merged
9 tasks
@nbolton nbolton marked this pull request as draft February 19, 2025 16:49
@nbolton
Copy link
Copy Markdown
Member Author

nbolton commented Feb 19, 2025 •
edited
Loading

CI package works fine on dev machine, but not fresh VM :|

image

Gonna need some more work.

Edit: Repro'd it on dev machine, fixing.

This was referenced Feb 19, 2025
Closed
Closed
@nbolton
fix: Restore active desktop name check in Windows daemon
f490bc1 
- Use `PIPE_NOWAIT` to skip empty stderr output on process output reading from Windows daemon watchdog
- Use `CreateProcess` result to determine when to call `CloseHandle` in Windows daemon watchdog
- Trim output from active desktop process in Windows daemon watchdog
- Improve error logging in Windows daemon watchdog
@nbolton
fix: Remove --exit-pause to stop "press any key to exit..." showing…
bf4fbdb 
… when Windows daemon kills server/client
@nbolton
refactor: Improve logging for errors when setting/getting cursor pos …
347ff99 
…on Windows
@nbolton
chore: Improve log message for leave screen warning on Windows
0cfcb53
@nbolton
refactor: Improve log message related to desk switch on Windows
d336626
@nbolton nbolton force-pushed the win-client-uac-auto branch from 32b1ccb to fec6570 Compare February 19, 2025 17:31
@nbolton nbolton marked this pull request as ready for review February 19, 2025 17:49
@nbolton
Copy link
Copy Markdown
Member Author

nbolton commented Feb 19, 2025

@sithlord48 Figured it out. It was a segfault that only happened on fresh install (when there was no command to run).

@nbolton

This comment was marked as outdated.

Sign in to view
@nbolton nbolton force-pushed the win-client-uac-auto branch from fec6570 to abfe6bb Compare February 19, 2025 18:10
@nbolton
Copy link
Copy Markdown
Member Author

nbolton commented Feb 19, 2025 •
edited
Loading

Package: https://github.com/deskflow/deskflow/actions/runs/13419409704/artifacts/2618354728

Tested and working on fresh Windows 11 VM.

@sithlord48 sithlord48 self-requested a review February 19, 2025 18:20
sithlord48
sithlord48 approved these changes Feb 19, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@sithlord48 sithlord48 left a comment

Choose a reason for hiding this comment

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

Worked now on windows

@nbolton nbolton mentioned this pull request Feb 19, 2025
Closed
6 tasks
@nbolton nbolton merged commit e808eed into master Feb 19, 2025
@nbolton nbolton deleted the win-client-uac-auto branch February 19, 2025 18:23
This was referenced Feb 22, 2025
Closed
Closed
@nbolton nbolton mentioned this pull request Mar 3, 2025
Closed
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sithlord48 sithlord48 sithlord48 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unable to control Windows client UAC dialog or login screen with auto elevate mode

2 participants

@nbolton @sithlord48