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

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`

src/cascadia/TerminalApp/AppLogic.cpp

@@ -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

src/cascadia/TerminalApp/AppLogic.h

@@ -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 };
 

src/cascadia/TerminalApp/AppLogic.idl

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

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -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)

src/cascadia/TerminalApp/TerminalPage.h

@@ -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);

src/cascadia/TerminalApp/TerminalWindow.cpp

@@ -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

src/cascadia/TerminalApp/TerminalWindow.h

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

src/cascadia/TerminalApp/TerminalWindow.idl

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

src/cascadia/TerminalSettingsModel/ApplicationState.cpp

@@ -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

src/types/inc/utils.hpp

@@ -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

src/types/utils.cpp

@@ -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 (...)