Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to drag/drop a file to terminal with UAC turned off #11668

Closed
d0j opened this issue Nov 1, 2021 · 16 comments
Closed

Unable to drag/drop a file to terminal with UAC turned off #11668

d0j opened this issue Nov 1, 2021 · 16 comments
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. Tracking-External This bug isn't resolved, but it's following an external workitem.
Milestone

Comments

@d0j
Copy link

d0j commented Nov 1, 2021

Windows Terminal version (or Windows build number)

Windows [Version 10.0.22000.282] Terminal [Version 1.12.2931.0]

Other Software

None open at the time

Steps to reproduce

Run windows terminal as administrator.
Try to drag a file/folder into the terminal to copy its path to the terminal window.
Does not work.

Expected Behavior

I expected drag and drop for files/folders into the terminal window to behave the same as a command prompt or powershell window, both behave as expected in non-admin or administrator permission instances of the aforementioned programs.

Actual Behavior

Path does not get copied during drag and drop into the terminal window, I just get the "deny" "not allowed" cursor icon when trying, same problem as #7754 but ticket closed

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Nov 1, 2021
@zadjii-msft
Copy link
Member

To be very specific - Are you running the Terminal as your local user account, but with admin permissions, or do you have UAC disabled entirely? These are two similar sounding but totally different scenarios unfortunately.

@zadjii-msft zadjii-msft added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Product-Terminal The new Windows Terminal. labels Nov 1, 2021
@d0j
Copy link
Author

d0j commented Nov 1, 2021

To be very specific - Are you running the Terminal as your local user account, but with admin permissions, or do you have UAC disabled entirely? These are two similar sounding but totally different scenarios unfortunately.

UAC is disabled.
I am working under an administrator account.

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Nov 1, 2021
@zadjii-msft
Copy link
Member

To be totally sure I'm doing this right - what's the output of reg query HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System\ /v enablelua?

@d0j
Copy link
Author

d0j commented Nov 1, 2021

To be totally sure I'm doing this right - what's the output of reg query HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System\ /v enablelua?

`reg query HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System\ /v enablelua

HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System
enablelua REG_DWORD 0x0`

@zadjii-msft zadjii-msft added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Attention The core contributors need to come back around and look at this ASAP. labels Nov 1, 2021
@zadjii-msft
Copy link
Member

Huh okay, that's the situation that I thought I fixed. Weird.

@zadjii-msft zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Nov 3, 2021
@zadjii-msft zadjii-msft added this to the Terminal v1.13 milestone Nov 3, 2021
@zadjii-msft zadjii-msft self-assigned this Nov 3, 2021
@eryksun
Copy link

eryksun commented Dec 16, 2021

I can confirm that it isn't fixed in 1.12.2931.0. I disabled UAC and logged on with an administrator account. I verified that the access token of Explorer in this case has high mandatory integrity level and has the administrators group enabled, the same as Terminal.

@eryksun
Copy link

eryksun commented Dec 16, 2021

IMO, IsElevated() from GH-11221 is badly named. The name is misleading because the function returns false for default elevation. It also checks for administrator access, which is separate from the token's elevation level. It should be named something descriptive like IsUserALinkedAdmin().

For the elevation check, it only needs to check for the elevation type TokenElevationTypeFull, which implies that the token is an elevated token that's linked to a limited token.

The function also has a technical bug, but one that's not an issue currently. Using nullptr for the token passed to test_token_membership() uses the effective token of the current thread in the underlying CheckTokenMembership() call. But the elevation check explicitly uses the process token, i.e. GetCurrentProcessToken() (i.e. (HANDLE)(LONG_PTR)-4). For the effective token, use GetCurrentThreadEffectiveToken() (i.e. (HANDLE)(LONG_PTR)-6), which references the thread token, if the thread is impersonating, else the process token. A name like IsUserALinkedAdmin() implies it should be checking the process token in both cases. I don't think Terminal ever impersonates another user, so there's no need to worry about effective access. BTW, closing these pseudohandles is a wasted system call that does nothing. There's no need for RAII.

@DHowett
Copy link
Member

DHowett commented Dec 16, 2021

FWIW I believe that the implementation in IsElevated as of 1.12 does the wrong thing as well, but in a different direction.

@zadjii-msft
Copy link
Member

Huh, so observation: I don't think we can fix this currently. Seems like the OS straight up disables XAML drag/drop when EnableLUA is set to 0. I'm trying it out with Sticky Notes in a VM, and that's not letting me drag/drop anything into it. I don't think the TermControl is doing anything to suppress the Drag event - it doesn't reject drops when it's elevated:

void TermControl::_DragOverHandler(Windows::Foundation::IInspectable const& /*sender*/,
DragEventArgs const& e)
{
if (_IsClosing())
{
return;
}
// We can only handle drag/dropping StorageItems (files) and plain Text
// currently. If the format on the clipboard is anything else, returning
// early here will prevent the drag/drop from doing anything.
if (!(e.DataView().Contains(StandardDataFormats::StorageItems()) ||
e.DataView().Contains(StandardDataFormats::Text())))
{
return;
}
// Make sure to set the AcceptedOperation, so that we can later receive the path in the Drop event
e.AcceptedOperation(DataPackageOperation::Copy);

@zadjii-msft
Copy link
Member

MSFT:35616520 has more context here. Looks like this is going to be an unfortunate "no" for now. We may need to move to a future version of WinAppSDK that might support this, whenever support for elevated drag/drop is added to WASDK.

@zadjii-msft zadjii-msft modified the milestones: Terminal v1.13, Backlog Jan 6, 2022
@zadjii-msft zadjii-msft added Tracking-External This bug isn't resolved, but it's following an external workitem. Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jan 6, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jan 6, 2022
@zadjii-msft zadjii-msft removed their assignment Jan 6, 2022
@DHowett
Copy link
Member

DHowett commented Jan 6, 2022

Wow, I really don't know what I meant when I said that "1.12 had it wrong in the opposite direction" -- the code is the same, as far as I can tell.

The core issue that prevents tab reordering is that we have a process, DataExchangeHost, that is used to render the bitmap attached to the cursor and shuttle data into the receiving process. That process must be running as the same user and at the same IL (I think¹) as Terminal.

The two cases where it isn't running as the same user or at the same IL are:

  1. Linked token?
  2. Over-the-shoulder elevation. This might secretly work (?) because of how brokering works.

Since we can't detect the identity of the DataExchangeHost, we had to solve for a heuristic . . . so IsElevated was originally designed to handle (1) because we needed to use it to disable drag/drop in case (1).

It was probably a mistake to try to generalize it and use it for anything else -- and we may want to rectify that mistake.

However, the issue IN THIS THREAD is different. Terminal doesn't directly control what external applications can drag/drop to it -- this is just a platform limitation. We'll need to take it up internally.

¹ Since it's just COM, and it doesn't want to "punch down" at a lower-IL process. I think.

@zadjii-msft
Copy link
Member

Oh hey this is actually tracked elsewhere: /dup #6661

@ghost
Copy link

ghost commented Mar 4, 2022

Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!

@ghost ghost added Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. Needs-Tag-Fix Doesn't match tag requirements labels Mar 4, 2022
@he852100
Copy link

he852100 commented Jul 8, 2022

@zadjii-msft They are two different problems
拖动

@a657938016
Copy link

@zadjii-毫秒它们是两个不同的问题 拖动 拖动

麻烦问下你的这个问题解决了吗?我也遇到同样的问题

@Sincky
Copy link

Sincky commented Dec 19, 2022

it still not work in version 1.15.3465.0 when UAC disable and reg query with "enablelua REG_DWORD 0x0"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. Tracking-External This bug isn't resolved, but it's following an external workitem.
Projects
None yet
Development

No branches or pull requests

7 participants