Skip to content

fix(gui-client/windows): allow GUI to run as admin again#6308

Merged
ReactorScram merged 6 commits intomainfrom
fix/windows-allow-gui-as-admin
Aug 15, 2024
Merged

fix(gui-client/windows): allow GUI to run as admin again#6308
ReactorScram merged 6 commits intomainfrom
fix/windows-allow-gui-as-admin

Conversation

@ReactorScram
Copy link
Contributor

@ReactorScram ReactorScram commented Aug 15, 2024

Closes #6305 too

I couldn't find the ticket for this so I'm not sure which customers are affected.

@ReactorScram ReactorScram self-assigned this Aug 15, 2024
@vercel
Copy link

vercel bot commented Aug 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
firezone ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 15, 2024 9:47pm


# Permissions

- [ ] Given a production exe, when you run it normally, then it will ask to escalate to Admin privileges ([#2751](https://github.com/firezone/firezone/issues/2751))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were all out of date too

const SERVICE_NAME: &str = "firezone_client_ipc";
const SERVICE_TYPE: ServiceType = ServiceType::OWN_PROCESS;

/// Returns true if the IPC service can run properly
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All this makes the file really long, but adding a refactoring would have made the diff huge.

There's too much IPC service code in firezone-headless-client so I think after this merges we should split that up.

@ReactorScram ReactorScram marked this pull request as ready for review August 15, 2024 16:21
Copy link
Member

@jamilbk jamilbk left a comment

Choose a reason for hiding this comment

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

I will defer to @thomaseizinger since this is a bit over my head. But does it basically revert the change that was rolled out in 1.1.10?

@ReactorScram
Copy link
Contributor Author

It partially reverts #6176, only affecting Windows. Linux will still get an error in stdout/stderr if you launch the GUI with sudo.

It also throws an error if the IPC service tries to run as a normal user, since it won't be able to control DNS.

Wasn't there one customer who ran into this issue before Thomas did?

@ReactorScram
Copy link
Contributor Author

And yeah judging from the dates, that did go into 1.1.10

@jamilbk
Copy link
Member

jamilbk commented Aug 15, 2024

Wasn't there one customer who ran into this issue before Thomas did?

Yeah - https://firezonehq.slack.com/archives/C07CL0LSSV8/p1723170052319419

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
@ReactorScram ReactorScram enabled auto-merge August 15, 2024 21:46
@ReactorScram ReactorScram added this pull request to the merge queue Aug 15, 2024
Merged via the queue into main with commit 4ddec81 Aug 15, 2024
@ReactorScram ReactorScram deleted the fix/windows-allow-gui-as-admin branch August 15, 2024 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delayed failure on IPC service when run without elevation

3 participants