microsoft/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2025-12-10 00:48:23 -06:00
Code Issues Packages Projects Releases 138 Wiki Activity

Clearly differentiate running elevated vs. drag/drop breaking (#14946)

Browse Source 
Credit where credit is due - @jboelter did literally all the hard work.

I just separated this out to two elements:
* Are we running elevated?
* Can we drag drop?

As we learned in #7754, the builtin administrator _can_ drag drop. But
critically, they are also running as admin! The way we had this logic
before, we're treat them as unelevated, because we had been overloading
the meaning here.

This splits these into two separate functions. Comes with the added
benefit of re-adding the elevation shield to the Terminal window for
users with UAC disabled (which was missing before) (and can _still_ be
disabled).

Closes #13928

Tested on a Win10 VM with `EnableLua=0`
This commit is contained in:
Mike Griese 2023-03-17 17:01:37 -05:00 committed by GitHub
parent 00af187a97
commit c5c15e86f3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 84 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/cascadia/TerminalApp/AppLogic.cpp
View File

@ -132,7 +132,12 @@ namespace winrt::TerminalApp::implementation
// cause you to chase down the rabbit hole of "why is App not
// registered?" when it definitely is.
_isElevated = ::Microsoft::Console::Utils::IsElevated();
// The TerminalPage has to be constructed during our construction, to
// make sure that there's a terminal page for callers of
// SetTitleBarContent
_isElevated = ::Microsoft::Console::Utils::IsRunningElevated();
_canDragDrop = ::Microsoft::Console::Utils::CanUwpDragDrop();
_reloadSettings = std::make_shared<ThrottledFuncTrailing<>>(winrt::Windows::System::DispatcherQueue::GetForCurrentThread(), std::chrono::milliseconds(100), [weakSelf = get_weak()]() {
if (auto self{ weakSelf.get() })
@ -163,10 +168,14 @@ namespace winrt::TerminalApp::implementation
// - <none> - reports internal state
// Return Value:
// - True if elevated, false otherwise.
bool AppLogic::IsElevated() const noexcept
bool AppLogic::IsRunningElevated() const noexcept
{
return _isElevated;
}
bool AppLogic::CanDragDrop() const noexcept
{
return _canDragDrop;
}
// Method Description:
// - Called by UWP context invoker to let us know that we may have to change some of our behaviors

4
src/cascadia/TerminalApp/AppLogic.h
View File

@ -49,7 +49,8 @@ namespace winrt::TerminalApp::implementation
void Create();
bool IsUwp() const noexcept;
void RunAsUwp();
bool IsElevated() const noexcept;
bool IsRunningElevated() const noexcept;
bool CanDragDrop() const noexcept;
void ReloadSettings();
bool HasSettingsStartupActions() const noexcept;
@ -72,6 +73,7 @@ namespace winrt::TerminalApp::implementation
private:
bool _isUwp{ false };
bool _isElevated{ false };
bool _canDragDrop{ false };
Microsoft::Terminal::Settings::Model::CascadiaSettings _settings{ nullptr };

3
src/cascadia/TerminalApp/AppLogic.idl
View File

@ -25,7 +25,8 @@ namespace TerminalApp
Boolean IsUwp();
void RunAsUwp();
Boolean IsElevated();
Boolean IsRunningElevated();
Boolean CanDragDrop();
Boolean HasSettingsStartupActions();

55
src/cascadia/TerminalApp/TerminalPage.cpp
View File

@ -153,27 +153,26 @@ namespace winrt::TerminalApp::implementation
_systemRowsToScroll = _ReadSystemRowsToScroll();
}
bool TerminalPage::IsElevated() const noexcept
bool TerminalPage::IsRunningElevated() const noexcept
{
// use C++11 magic statics to make sure we only do this once.
// This won't change over the lifetime of the application
static const auto isElevated = []() {
// *** THIS IS A SINGLETON ***
auto result = false;
// GH#2455 - Make sure to try/catch calls to Application::Current,
// because that _won't_ be an instance of TerminalApp::App in the
// LocalTests
try
{
result = ::winrt::Windows::UI::Xaml::Application::Current().as<::winrt::TerminalApp::App>().Logic().IsElevated();
}
CATCH_LOG();
return result;
}();
return isElevated;
// GH#2455 - Make sure to try/catch calls to Application::Current,
// because that _won't_ be an instance of TerminalApp::App in the
// LocalTests
try
{
return Application::Current().as<TerminalApp::App>().Logic().IsRunningElevated();
}
CATCH_LOG();
return false;
}
bool TerminalPage::CanDragDrop() const noexcept
{
try
{
return Application::Current().as<TerminalApp::App>().Logic().CanDragDrop();
}
CATCH_LOG();
return true;
}
void TerminalPage::Create()
@ -186,11 +185,11 @@ namespace winrt::TerminalApp::implementation
_tabView = _tabRow.TabView();
_rearranging = false;
const auto isElevated = IsElevated();
const auto canDragDrop = CanDragDrop();
_tabRow.PointerMoved({ get_weak(), &TerminalPage::_RestorePointerCursorHandler });
_tabView.CanReorderTabs(!isElevated);
_tabView.CanDragTabs(!isElevated);
_tabView.CanReorderTabs(canDragDrop);
_tabView.CanDragTabs(canDragDrop);
_tabView.TabDragStarting({ get_weak(), &TerminalPage::_TabDragStarted });
_tabView.TabDragCompleted({ get_weak(), &TerminalPage::_TabDragCompleted });
@ -312,7 +311,7 @@ namespace winrt::TerminalApp::implementation
// Setup mouse vanish attributes
SystemParametersInfoW(SPI_GETMOUSEVANISH, 0, &_shouldMouseVanish, false);
_tabRow.ShowElevationShield(IsElevated() && _settings.GlobalSettings().ShowAdminShield());
_tabRow.ShowElevationShield(IsRunningElevated() && _settings.GlobalSettings().ShowAdminShield());
// Store cursor, so we can restore it, e.g., after mouse vanishing
// (we'll need to adapt this logic once we make cursor context aware)
@ -339,7 +338,7 @@ namespace winrt::TerminalApp::implementation
// GH#12267: Don't forget about defterm handoff here. If we're being
// created for embedding, then _yea_, we don't need to handoff to an
// elevated window.
if (!_startupActions || IsElevated() || _shouldStartInboundListener || _startupActions.Size() == 0)
if (!_startupActions || IsRunningElevated() || _shouldStartInboundListener || _startupActions.Size() == 0)
{
// there aren't startup actions, or we're elevated. In that case, go for it.
return false;
@ -1128,7 +1127,7 @@ namespace winrt::TerminalApp::implementation
WI_IsFlagSet(lAltState, CoreVirtualKeyStates::Down) &&
WI_IsFlagSet(rAltState, CoreVirtualKeyStates::Down);
const auto dispatchToElevatedWindow = ctrlPressed && !IsElevated();
const auto dispatchToElevatedWindow = ctrlPressed && !IsRunningElevated();
if ((shiftPressed || dispatchToElevatedWindow) && !debugTap)
{
@ -2894,7 +2893,7 @@ namespace winrt::TerminalApp::implementation
// want to create an animation.
WUX::Media::Animation::Timeline::AllowDependentAnimations(!_settings.GlobalSettings().DisableAnimations());
_tabRow.ShowElevationShield(IsElevated() && _settings.GlobalSettings().ShowAdminShield());
_tabRow.ShowElevationShield(IsRunningElevated() && _settings.GlobalSettings().ShowAdminShield());
Media::SolidColorBrush transparent{ Windows::UI::Colors::Transparent() };
_tabView.Background(transparent);
@ -4034,7 +4033,7 @@ namespace winrt::TerminalApp::implementation
{
// Try to handle auto-elevation
const auto requestedElevation = controlSettings.DefaultSettings().Elevate();
const auto currentlyElevated = IsElevated();
const auto currentlyElevated = IsRunningElevated();
// We aren't elevated, but we want to be.
if (requestedElevation && !currentlyElevated)

3
src/cascadia/TerminalApp/TerminalPage.h
View File

@ -126,7 +126,8 @@ namespace winrt::TerminalApp::implementation
TerminalApp::WindowProperties WindowProperties() const noexcept { return _WindowProperties; };
bool IsElevated() const noexcept;
bool CanDragDrop() const noexcept;
bool IsRunningElevated() const noexcept;
void OpenSettingsUI();
void WindowActivated(const bool activated);

29
src/cascadia/TerminalApp/TerminalWindow.cpp
View File

@ -201,35 +201,6 @@ namespace winrt::TerminalApp::implementation
return isUwp;
}
// Method Description:
// - Called around the codebase to discover if Terminal is running elevated
// Arguments:
// - <none> - reports internal state
// Return Value:
// - True if elevated, false otherwise.
bool TerminalWindow::IsElevated() const noexcept
{
// use C++11 magic statics to make sure we only do this once.
// This won't change over the lifetime of the application
static const auto isElevated = []() {
// *** THIS IS A SINGLETON ***
auto result = false;
// GH#2455 - Make sure to try/catch calls to Application::Current,
// because that _won't_ be an instance of TerminalApp::App in the
// LocalTests
try
{
result = ::winrt::Windows::UI::Xaml::Application::Current().as<::winrt::TerminalApp::App>().Logic().IsElevated();
}
CATCH_LOG();
return result;
}();
return isElevated;
}
// Method Description:
// - Build the UI for the terminal app. Before this method is called, it
// should not be assumed that the TerminalApp is usable. The Settings

1
src/cascadia/TerminalApp/TerminalWindow.h
View File

@ -65,7 +65,6 @@ namespace winrt::TerminalApp::implementation
void Create();
bool IsUwp() const noexcept;
bool IsElevated() const noexcept;
void Quit();

2
src/cascadia/TerminalApp/TerminalWindow.idl
View File

@ -51,8 +51,6 @@ namespace TerminalApp
// registered?" when it definitely is.
void Create();
Boolean IsElevated();
Boolean HasCommandlineArguments();
Int32 SetStartupCommandline(String[] commands);

8
src/cascadia/TerminalSettingsModel/ApplicationState.cpp
View File

@ -177,7 +177,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// from state.json. We'll then load the Local props from
// `elevated-state.json`
// - If we're unelevated, then load _everything_ from state.json.
if (::Microsoft::Console::Utils::IsElevated())
if (::Microsoft::Console::Utils::IsRunningElevated())
{
// Only load shared properties if we're elevated
FromJson(root, FileSource::Shared);
@ -225,7 +225,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
//
// After that's done, we'll write our Local properties into
// elevated-state.json.
if (::Microsoft::Console::Utils::IsElevated())
if (::Microsoft::Console::Utils::IsRunningElevated())
{
std::string errs;
std::unique_ptr<Json::CharReader> reader{ Json::CharReaderBuilder{}.newCharReader() };
@ -353,7 +353,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// permissions, so we don't potentially read malicious data.
std::optional<std::string> ApplicationState::_readLocalContents() const
{
return ::Microsoft::Console::Utils::IsElevated() ?
return ::Microsoft::Console::Utils::IsRunningElevated() ?
ReadUTF8FileIfExists(_elevatedPath, true) :
ReadUTF8FileIfExists(_sharedPath, false);
}
@ -374,7 +374,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// will atomically write to `user-state.json`
void ApplicationState::_writeLocalContents(const std::string_view content) const
{
if (::Microsoft::Console::Utils::IsElevated())
if (::Microsoft::Console::Utils::IsRunningElevated())
{
// DON'T use WriteUTF8FileAtomic, which will write to a temporary file
// then rename that file to the final filename. That actually lets us

3
src/types/inc/utils.hpp
View File

@ -96,7 +96,8 @@ namespace Microsoft::Console::Utils
GUID CreateV5Uuid(const GUID& namespaceGuid, const std::span<const std::byte> name);
bool IsElevated();
bool CanUwpDragDrop();
bool IsRunningElevated();
// This function is only ever used by the ConPTY connection in
// TerminalConnection. However, that library does not have a good system of

35
src/types/utils.cpp
View File

@ -653,9 +653,18 @@ GUID Utils::CreateV5Uuid(const GUID& namespaceGuid, const std::span<const std::b
return EndianSwap(newGuid);
}
bool Utils::IsElevated()
// * Elevated users cannot use the modern drag drop experience. This is
// specifically normal users running the Terminal as admin
// * The Default Administrator, who does not have a split token, CAN drag drop
// perfectly fine. So in that case, we want to return false.
// * This has to be kept separate from IsRunningElevated, which is exclusively
// used for "is this instance running as admin".
bool Utils::CanUwpDragDrop()
{
static auto isElevated = []() {
// There's a lot of wacky double negatives here so that the logic is
// basically the same as IsRunningElevated, but the end result semantically
// makes sense as "CanDragDrop".
static auto isDragDropBroken = []() {
try
{
wil::unique_handle processToken{ GetCurrentProcessToken() };
@ -670,8 +679,30 @@ bool Utils::IsElevated()
//
// See GH#7754, GH#11096
return false;
// drag drop is _not_ broken -> they _can_ drag drop
}
// If they are running admin, they cannot drag drop.
return wil::test_token_membership(nullptr, SECURITY_NT_AUTHORITY, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS);
}
catch (...)
{
LOG_CAUGHT_EXCEPTION();
// This failed? That's very peculiar indeed. Let's err on the side
// of "drag drop is broken", just in case.
return true;
}
}();
return !isDragDropBroken;
}
// See CanUwpDragDrop, GH#13928 for why this is different.
bool Utils::IsRunningElevated()
{
static auto isElevated = []() {
try
{
return wil::test_token_membership(nullptr, SECURITY_NT_AUTHORITY, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS);
}
catch (...)