-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Closed
Labels
Area-UserInterfaceIssues pertaining to the user interface of the Console or TerminalIssues pertaining to the user interface of the Console or TerminalIssue-TaskIt's a feature request, but it doesn't really need a major design.It's a feature request, but it doesn't really need a major design.Product-TerminalThe new Windows Terminal.The new Windows Terminal.Resolution-Won't-FixWe're just really obstinate about this. There's probably a good reason.We're just really obstinate about this. There's probably a good reason.
Description
This is part of https://github.com/microsoft/terminal/projects/5, and a pre-req for #632. I'm promoting this to a full issue since I've got a lot of edge-case-y work that needs doing and tracking somewhere.
- Hot reload
elevated-state.json - Find the right place to store the file
- Figure out the right way to do the permissions on this file.
- SYSTEM:
GENERIC_ALL, Everyone:GENERIC_READ- Didn't work. Elevated Terminal couldn't write the file. - Admins:
GENERIC_ALL, Everyone:GENERIC_READ- Didn't work. Sublime could just delete the file and write something else in it's place. Un-elevated Terminal could even write the file! -
DOMAIN_USER_RID_ADMIN"Admin", not "Admins"? - Manual experimentation says "MIGRIE-HOME\Administrators": Full Control + "Everyone": Read seems to work as I want. Now how do I get that?
- Something else?
- IT WAS SOMETHING ELSE.
WriteFileAtomicwas the problem - the rename would discard the elevated only version of the file, then write a new one in it's place. Presumably, this doesn't really matter. We should only be calling into that function from an elevated window, but it's unfortunate that it doesn't prevent it from working when unelevated.
- SYSTEM:
- Maybe decide to still do
WriteUTF8FileAtomic. We don't need the link handling, so I'm not worried about that... - Move the
isElevated()logic to a common place to make sure we have one implementation - Make a call to the common
isElevated()inWriteUTF8File*(), so that we can bail early if we're unelevated. - Check that the file is only writable by admins when opening. If it isn't, blow it away and recreate it.
- Use
unique_sidand in general, make sure to clean up the things allocated for us. - Only pop the dialog when running elevated
- Don't pop the dialog if we're elevated (but UAC is disabled) (this is probably just Unable to drag/drop a file to terminal with UAC turned off #7754)
- Don't pop the dialog for commandlines that are literally just a path to a file in
C:\windows\system32.- The file must be a full path
- The file must be in
C:\windows\system32
- UHG also resolve environment variables, so
%SystemRoot%\\System32\\WindowsPowerShell\\v1.0\\powershell.exewill work. - UHG WSL distros too
- Pop the dialog for panes too.
- Actually format the dialog message with the commandline
- Resolve what happens when multiple controls should be added at once (See Ideas A and B below)
- What happens when you reject the creation of a pane? Probably should just close
- What happens during startup for this dialog? Do we just immediately go to the
Completestate, then process the actions one at a time? - What happens when running a
wtaction in an elevated window? - Rejecting the creation of the first&only (or all) tabs should just close the window.
- What happens if you
wt nt ; sp, then reject the first tab? There are no tabs now to split! - Approving a commandline in one pane should automatically update the other ones.
- Don't add the same commandline multiple times
- There's some weird focus issues - Replacing the content of the pane doesn't update the tab title (and presumably other properties as well)
- What happens when the user isn't an admin, but runs the terminal as another user that's an admin (that also has the Terminal installed)?
- Make sure to add a "why am I seeing this" link
- Add the TermControl's background to the admin warning placeholder's background.
- After 64d02f2, fix the hot-reloading for elevated windows
Engineering deets
- Break into two PRs:
- Elevation State
- Warning Dialog
- Fix Unable to drag/drop a file to terminal with UAC turned off #7754 in it's own PR
- send
dev/migrie/uac-shield - send
dev/migrie/f/632-attempt-2
what happens when multiple controls should be added at once
Idea A: for scenarios where we're going to perform multiple actions, check all of them for commandlines. If we find one action that has an unapproved commandline, then ask the user if they want to run all of the actions. They either approve them all, or none of them.
Idea B: Use a fake content dialog as a placeholder control, until the connection is approved.
- B.1: We use non-terminal content in the Pane. We use a fake ContentDialog as a placeholder for the TermControl, then swap it out with the TermControl if the control is allowed.
- This is in the branch from c66a566 onwards
- How do we handle closing? I think in my initial implementation, there was a
IPaneContentinterface that exposed all the things a TermControl might do. - Like, what happens when the dialog is rejected. It raises a
Closed()? How does the pane listen for that... - I suppose the
AdminWarningPlaceholderwill be holding the Pane, the - NO it won't hold the pane. That's A: bad circular refs; and B: the Panes will toss the controls around to other panes. So that's bad.
- B.2: We tell the TermControl to display the dialog itself. It doesn't Start() the connection immediately, instead it starts with it's fake contentdialog.
- How does this work with oop controls? It wouldn't. The ContentProcess needs to be started before the TermControl. So even if we told the TermControl "you need to ask permission", we'd have already started the content process. It would be harder for the Control to then tell the app "yes I have permission for the thing you asked", then pass it the ContentProcess.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
Area-UserInterfaceIssues pertaining to the user interface of the Console or TerminalIssues pertaining to the user interface of the Console or TerminalIssue-TaskIt's a feature request, but it doesn't really need a major design.It's a feature request, but it doesn't really need a major design.Product-TerminalThe new Windows Terminal.The new Windows Terminal.Resolution-Won't-FixWe're just really obstinate about this. There's probably a good reason.We're just really obstinate about this. There's probably a good reason.